Skip to content

Commit 06973be

Browse files
committed
chore(hooks): rename logger-guard marker to allow console; allow pnpm dlx in npx scanner
- logger-guard: canonical marker `# socket-hook: allow console` (was `allow logger`); the construct being suppressed is the console / direct-stream write, not the recommended replacement. Legacy `allow logger` accepted via a one-cycle alias. - _helpers.mts: introduce RULE_ALIASES + aliasMatches() so future rule-marker renames don't break existing comments mid-flight. - npx-dlx scanner: drop `pnpm dlx` from the offending forms (it's the fleet-canonical fetch-and-run for documentation lines that describe ad-hoc CLI usage where the consumer doesn't have the package pinned). suggestNpxReplacement now suggests `pnpm dlx` for runtime npx swaps that need fetch semantics. - pre-commit.mts: route the npx loop through shouldSkipFile so tests, fixtures, and .git-hooks themselves are skipped; covers cases like socket-lib's bin.test.mts which legitimately mention npx in resolution-logic test fixtures.
1 parent 3cb9139 commit 06973be

5 files changed

Lines changed: 84 additions & 30 deletions

File tree

.claude/hooks/logger-guard/README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,11 @@ The hook is intentionally narrow:
4444
- **Exempts** `.claude/hooks/`, `.git-hooks/`, `scripts/`, tests,
4545
fixtures, and external/vendored code — those have legitimate
4646
reasons to write directly.
47-
- **Exempts** lines tagged `# socket-hook: allow logger` (canonical
48-
per-line opt-out). The bare form `# socket-hook: allow` also
49-
works for blanket suppression.
47+
- **Exempts** lines tagged `# socket-hook: allow console` (canonical
48+
per-line opt-out — names the construct being allowed, not the
49+
recommended replacement). The bare form `# socket-hook: allow`
50+
also works for blanket suppression. Legacy `allow logger` is
51+
accepted as an alias for one deprecation cycle.
5052
- **Exempts** lines that look like documentation: lines starting
5153
with `*`, `//`, or `#`; JSDoc tags; fully-backticked code spans.
5254

.claude/hooks/logger-guard/index.mts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@
2727
// extensions. Hooks (.claude/hooks/), git-hooks (.git-hooks/),
2828
// scripts (scripts/), tests, fixtures, and external/ vendored code
2929
// are exempt — see EXEMPT_PATH_PATTERNS.
30-
// - Lines marked `# socket-hook: allow logger` are exempt (canonical
30+
// - Lines marked `# socket-hook: allow console` are exempt (canonical
3131
// opt-out marker, same as path-guard / token-guard / npx-guard).
32+
// The legacy spelling `allow logger` is accepted as an alias for
33+
// one cycle (deprecation grace period).
3234
// - Lines that look like documentation (comment lines, JSDoc tags,
3335
// fully backticked code spans) are exempt — handled by the shared
3436
// `looksLikeDocumentation` heuristic in `_helpers.mts`.
@@ -70,7 +72,7 @@ const COMMENT_LINE_RE = /^\s*(\*|\/\/|#)/
7072
const JSDOC_TAG_RE = /@(example|param|returns?|see|link)\b/
7173
// Accept `#`, `//`, or `/*` comment prefixes — same as the git pre-
7274
// commit/pre-push scanners. This hook is invoked on TS/JS edits where
73-
// `// socket-hook: allow logger` is the only natural spelling.
75+
// `// socket-hook: allow console` is the natural spelling.
7476
const SOCKET_HOOK_MARKER_RE =
7577
/(?:#|\/\/|\/\*)\s*socket-hook:\s*allow(?:\s+([\w-]+))?/
7678

@@ -79,9 +81,10 @@ function isMarkerSuppressed(line: string): boolean {
7981
if (!m) {
8082
return false
8183
}
82-
// No specific rule named → blanket allow. Targeted form must name
83-
// 'logger' to suppress this scanner.
84-
return !m[1] || m[1] === 'logger'
84+
// No specific rule named → blanket allow. Targeted form names what
85+
// the line is doing ('console' for direct stream writes); 'logger'
86+
// is accepted as a one-cycle alias for the legacy spelling.
87+
return !m[1] || m[1] === 'console' || m[1] === 'logger'
8588
}
8689

8790
function isInsideBackticks(line: string): boolean {
@@ -220,7 +223,7 @@ function emitBlock(filePath: string, hits: Hit[]): void {
220223
out.push(` …and ${hits.length - 3} more.`)
221224
}
222225
out.push(
223-
' Opt-out for one line (rare): append `// socket-hook: allow logger`.',
226+
' Opt-out for one line (rare): append `// socket-hook: allow console`.',
224227
)
225228
out.push('')
226229
process.stderr.write(out.join('\n'))

.claude/hooks/logger-guard/test/logger-guard.test.mts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,20 @@ test('allows tests to use console.log', async () => {
9292
assert.equal(code, 0)
9393
})
9494

95-
test('respects # socket-hook: allow logger marker', async () => {
95+
test('respects # socket-hook: allow console marker', async () => {
96+
const { code } = await runHook({
97+
tool_name: 'Edit',
98+
tool_input: {
99+
file_path: 'src/foo.ts',
100+
new_string:
101+
'const x = 1; console.error("a") // # socket-hook: allow console',
102+
},
103+
})
104+
assert.equal(code, 0)
105+
})
106+
107+
// Legacy spelling — accepted as alias for one deprecation cycle.
108+
test('respects # socket-hook: allow logger marker (legacy alias)', async () => {
96109
const { code } = await runHook({
97110
tool_name: 'Edit',
98111
tool_input: {
@@ -115,23 +128,23 @@ test('respects bare # socket-hook: allow marker', async () => {
115128
assert.equal(code, 0)
116129
})
117130

118-
test('respects // socket-hook: allow logger marker (slash-slash prefix)', async () => {
131+
test('respects // socket-hook: allow console marker (slash-slash prefix)', async () => {
119132
const { code } = await runHook({
120133
tool_name: 'Edit',
121134
tool_input: {
122135
file_path: 'src/foo.ts',
123-
new_string: 'process.stderr.write(buf) // socket-hook: allow logger',
136+
new_string: 'process.stderr.write(buf) // socket-hook: allow console',
124137
},
125138
})
126139
assert.equal(code, 0)
127140
})
128141

129-
test('respects /* socket-hook: allow logger */ marker (block-comment prefix)', async () => {
142+
test('respects /* socket-hook: allow console */ marker (block-comment prefix)', async () => {
130143
const { code } = await runHook({
131144
tool_name: 'Edit',
132145
tool_input: {
133146
file_path: 'src/foo.ts',
134-
new_string: 'console.error("a") /* socket-hook: allow logger */',
147+
new_string: 'console.error("a") /* socket-hook: allow console */',
135148
},
136149
})
137150
assert.equal(code, 0)

.git-hooks/_helpers.mts

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,27 @@ export const socketHookMarkerFor = (filePath: string, rule: string): string =>
167167
: `# socket-hook: allow ${rule}`
168168
const LEGACY_ZIZMOR_MARKER_RE = /(?:#|\/\/|\/\*)\s*zizmor:\s*[\w-]+/
169169

170+
// Aliases: legacy marker names recognized as equivalent to a current
171+
// rule for one deprecation cycle, so callers can rename the canonical
172+
// rule without breaking files that still carry the old marker.
173+
//
174+
// Add entries as `<alias>: <canonical>`; both directions match in the
175+
// comparison below.
176+
const RULE_ALIASES: { [k: string]: string | undefined } = {
177+
__proto__: null,
178+
// 'logger' was the original name when the scanner only flagged
179+
// process.std{out,err}.write; it now flags console.* too, so the
180+
// canonical marker is 'console'. Keep 'logger' for one cycle.
181+
logger: 'console',
182+
}
183+
184+
function aliasMatches(marker: string, rule: string): boolean {
185+
if (marker === rule) {
186+
return true
187+
}
188+
return RULE_ALIASES[marker] === rule || RULE_ALIASES[rule] === marker
189+
}
190+
170191
function lineIsSuppressed(line: string, rule?: string): boolean {
171192
if (LEGACY_ZIZMOR_MARKER_RE.test(line)) {
172193
return true
@@ -179,8 +200,9 @@ function lineIsSuppressed(line: string, rule?: string): boolean {
179200
if (!m[1]) {
180201
return true
181202
}
182-
// Marker named a specific rule → only suppress that rule.
183-
return rule === undefined || m[1] === rule
203+
// Marker named a specific rule → suppress when the names match
204+
// directly OR through an alias.
205+
return rule === undefined || aliasMatches(m[1], rule)
184206
}
185207

186208
// Heuristic context flags: lines that look like "this is a doc example"
@@ -354,30 +376,39 @@ export const scanPrivateKeys = (text: string): LineHit[] => {
354376

355377
// ── npx/dlx scanner ────────────────────────────────────────────────
356378
//
357-
// Match `npx` / `pnpm dlx` / `yarn dlx` only when the token sits at a
358-
// command position — preceded by start-of-line / whitespace / shell
359-
// separator (`&&`, `||`, `;`, `|`, `(`, backtick), or directly after a
360-
// PowerShell `& ` invoke. Exclude JSON-key, env-value, and identifier
361-
// suffix contexts where `npx` shows up as an embedded substring:
379+
// Match `npx` / `yarn dlx` only when the token sits at a command
380+
// position — preceded by start-of-line / whitespace / shell separator
381+
// (`&&`, `||`, `;`, `|`, `(`, backtick), or directly after a PowerShell
382+
// `& ` invoke. Exclude JSON-key, env-value, and identifier suffix
383+
// contexts where `npx` shows up as an embedded substring:
362384
// - `"socket-npx": …` (bin-name suffix)
363385
// - `"dev:npx": "…SOCKET_CLI_MODE=npx node …"` (script key + env value)
364386
// - `cmd-npx-helper` (identifier interior)
365387
// The negative lookbehind catches hyphen / colon / equals / underscore /
366388
// dot prefixes; the negative lookahead catches the same followed forms
367389
// (`npx-helper`, `npx:foo`).
390+
//
391+
// **Allowed:** `pnpm dlx` / `pnpm exec` / `pn dlx` / `pn exec` / `pnx`
392+
// (the pnpm v11 shorthands for `pnpm dlx`). `pnpm dlx` is the
393+
// fleet-canonical fetch-and-run form for documentation lines that
394+
// describe ad-hoc CLI usage (where the consumer doesn't have the
395+
// package pinned in their workspace). `pnx` is the v11 shorthand and
396+
// is equally allowed.
368397

369-
const NPX_DLX_RE = /(?<![\w\-:=.])\b(npx|pnpm dlx|yarn dlx)\b(?![\w\-:=.])/
398+
const NPX_DLX_RE = /(?<![\w\-:=.])\b(npx|yarn dlx)\b(?![\w\-:=.])/
370399

371400
// Suggest the canonical replacement for a runtime npx/dlx call.
372401
// Documentation contexts (comments, JSDoc) are exempt via
373402
// looksLikeDocumentation(); we only ever land here for code lines, where
374403
// the right swap is `pnpm exec` (since `pnpm` is the fleet's package
375-
// manager) or `pnpm run` for script entries.
404+
// manager) or `pnpm run` for script entries. For documentation lines
405+
// that legitimately need a fetch-and-run command (user-facing
406+
// instructions where the consumer doesn't have the package pinned),
407+
// use `pnpm dlx` or its pnpm v11 shorthand `pnx` instead of `npx`.
376408
function suggestNpxReplacement(line: string): string {
377409
return line
378-
.replace(/\bpnpm dlx\b/g, 'pnpm exec')
379410
.replace(/\byarn dlx\b/g, 'pnpm exec')
380-
.replace(/\bnpx\b/g, 'pnpm exec')
411+
.replace(/\bnpx\b/g, 'pnpm dlx')
381412
}
382413

383414
export const scanNpxDlx = (text: string): LineHit[] => {
@@ -406,8 +437,9 @@ export const scanNpxDlx = (text: string): LineHit[] => {
406437
// `@socketsecurity/lib/logger`. Direct calls to `process.stderr.write`,
407438
// `process.stdout.write`, `console.log`, `console.error`, `console.warn`,
408439
// `console.info`, `console.debug` are blocked. Doc-context lines are
409-
// exempt; lines carrying `// socket-hook: allow logger` (or `#` in
410-
// non-TS files) are exempt too.
440+
// exempt; lines carrying `// socket-hook: allow console` (or `#` in
441+
// non-TS files) are exempt too. Legacy `allow logger` is accepted as
442+
// an alias for one deprecation cycle.
411443

412444
const LOGGER_LEAK_RE =
413445
/\b(process\.std(?:err|out)\.write|console\.(?:log|error|warn|info|debug))\s*\(/
@@ -435,7 +467,7 @@ export const scanLoggerLeaks = (text: string): LineHit[] => {
435467
if (!LOGGER_LEAK_RE.test(line)) {
436468
continue
437469
}
438-
if (looksLikeDocumentation(line, LOGGER_LEAK_RE, 'logger')) {
470+
if (looksLikeDocumentation(line, LOGGER_LEAK_RE, 'console')) {
439471
continue
440472
}
441473
hits.push({

.git-hooks/pre-commit.mts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,14 @@ const main = (): number => {
179179
// npx/dlx usage.
180180
logger.info('Checking for npx/dlx usage...')
181181
for (const file of stagedFiles) {
182+
// shouldSkipFile covers tests, fixtures, .git-hooks, etc. — test
183+
// files frequently mention `npx` as part of fixture paths or
184+
// resolution-logic test cases (see socket-lib/test/unit/bin.test.mts).
185+
if (shouldSkipFile(file)) {
186+
continue
187+
}
182188
if (
183-
file.includes('node_modules/') ||
184189
file.endsWith('pnpm-lock.yaml') ||
185-
file.includes('.git-hooks/') ||
186190
// CHANGELOG entries discuss npx ecosystem *behavior* (cache
187191
// semantics, naming conventions) as historical documentation —
188192
// they're not commands. Skip the npx/dlx scan for changelogs.

0 commit comments

Comments
 (0)