Skip to content

Commit 41c562c

Browse files
committed
fix: keep quoted paths, drop heredoc bodies, split the verb tables
The previous commit's quote handling DELETED quoted spans, which took the path with them. `cd "<worktree>" && npm ci` left `cd` with no argument, so the install was judged against the session cwd and allowed. That is the arrival shape this hook exists for, and quoting a path is the ordinary spelling, so the rewrite regressed the headline case. Quoted spans are now neutralised rather than removed: the quote characters go, the content stays, and only the separators inside them are defused. It is a character-by-character state machine because quote nesting has to be tracked, and a single-quote sed pass running first paired the apostrophe in `can't` with the next quote in the line and swallowed a real install between them. Heredoc bodies are dropped. This repo's docs are full of `npm install` lines and a newline is a command separator here. The merged verb table is per manager again: `a` is a BUN alias, so merging it blocked `npm --workspace a run build`, and `bun upgrade` upgrades the Bun binary rather than node_modules. `npm audit fix` joins the block list on the same rationale as `link`, `rebuild` and `prune`, `--cwd` and `--dir` are mined as target directories like `--prefix`, and an informational `yarn --version` is no longer read as a bare install.
1 parent 92e8295 commit 41c562c

3 files changed

Lines changed: 171 additions & 14 deletions

File tree

.claude/hooks/block-install-in-linked-worktree.sh

Lines changed: 76 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,74 @@ if [ -z "$cmd" ]; then exit 0; fi
5353
# THROUGH the link rather than replacing it. `link`, `rebuild` and `prune` are
5454
# here for the same reason as the remove verbs: they all mutate the tree that
5555
# the symlink points at.
56-
INSTALL_VERBS='install-ci-test|clean-install-test|install-clean|clean-install|install-test|install|isntall|isntal|isnta|isnt|instal|insta|inst|ins|in|i|add|a|ci|cit|sit|it|ic|update|upgrade|udpate|up|dedupe|ddp|uninstall|unlink|un|remove|rm|r|link|ln|rebuild|rb|prune'
56+
# PER MANAGER, not one merged list. Merging them blocked `npm --workspace a run
57+
# build`, because `a` is a BUN alias for `add`, and blocked `bun upgrade`, which
58+
# upgrades the Bun BINARY and never touches node_modules.
59+
#
60+
# The one-letter aliases `a` and `r` are deliberately omitted from npm's list.
61+
# They are rare as commands and common as flag VALUES, and the scan cannot tell
62+
# the two apart, so admitting them blocks `npm -w a run build`. KNOWN GAP: `npm
63+
# r <pkg>` and `npm a <pkg>` are therefore not blocked. That is the deliberate
64+
# trade, because the false positive lands on an ordinary command while the false
65+
# negative lands on a spelling almost nobody types, and the repair, report and
66+
# doctor layers still catch the damage after the fact.
67+
NPM_INSTALL='install-ci-test|clean-install-test|install-clean|clean-install|install-test|install|isntall|isntal|isnta|isnt|instal|insta|inst|ins|in|i|add|ci|cit|sit|it|ic|update|upgrade|udpate|up|dedupe|ddp|uninstall|unlink|un|remove|rm|link|ln|rebuild|rb|prune'
68+
BUN_INSTALL='install|i|add|a|remove|rm|link|unlink|update|pm'
69+
PNPM_INSTALL='install|i|add|update|upgrade|up|dedupe|remove|rm|uninstall|un|link|unlink|prune|rebuild'
70+
YARN_INSTALL='install|add|upgrade|up|dedupe|remove|link|unlink'
5771
# Verbs that do NOT touch node_modules. Listed explicitly so the scan can STOP:
5872
# without them, `npm run test -- --grep add` would keep scanning and hit `add`.
5973
SAFE_VERBS='run|run-script|rum|urn|test|tst|t|start|stop|restart|exec|x|ls|list|la|ll|init|innit|create|publish|pack|version|view|v|info|show|why|ping|config|c|get|set|docs|home|repo|bugs|audit|fund|outdated|prefix|root|bin|whoami|token|team|org|access|star|unstar|search|s|se|find|help|doctor|explain|edit|deprecate|dist-tag|hook|login|logout|adduser|owner|profile|shrinkwrap|unpublish|completion|diff|query|sbom'
74+
# `npm audit` reports and is safe; `npm audit fix` INSTALLS revised versions
75+
# straight through the link, so it is matched ahead of the safe-verb scan.
76+
AUDIT_FIX='(^|[[:space:]])audit([[:space:]]+-[^[:space:]]+)*[[:space:]]+fix([[:space:]]|$)'
6077
# A GLOBAL install writes to the npm prefix, never through the local link, and
6178
# `npm update -g webjsdev` is this repo's documented post-release step.
6279
GLOBAL='(^|[[:space:]])(-g|--global)([[:space:]]|$)'
6380

64-
# STAGE 1: remove quoted spans. See the docblock; this is what keeps a shell
65-
# metacharacter inside a commit message from being read as a command boundary.
66-
scrubbed=$(printf '%s' "$cmd" | sed -e "s/'[^']*'/ /g" -e 's/"[^"]*"/ /g')
81+
# STAGE 1: neutralise quoted spans and drop heredoc bodies.
82+
#
83+
# A quoted span must keep its CONTENT (a quoted path is the ordinary defensive
84+
# spelling of `cd "<worktree>" && npm ci`, which is the arrival shape this hook
85+
# exists for) while losing its power to look like a command boundary. So the
86+
# quote characters are removed and only the SEPARATORS inside them are
87+
# neutralised. Deleting the whole span instead loses the path and fails open.
88+
#
89+
# It is a character-by-character state machine rather than a pair of seds
90+
# because quote nesting has to be tracked: an apostrophe inside a double-quoted
91+
# string is literal, and a sed pass over `'...'` first would pair it with the
92+
# next single quote in the line and swallow whatever sat between.
93+
#
94+
# A heredoc BODY is not commands. This repo's docs are full of `npm install`
95+
# lines, and `cat > doc.md <<'EOF'` ... `EOF` must not read as an install.
96+
scrubbed=$(printf '%s' "$cmd" | awk '
97+
function flushline(l) { print l }
98+
BEGIN { heredoc = "" }
99+
{
100+
if (heredoc != "") {
101+
line = $0
102+
sub(/[[:space:]]+$/, "", line)
103+
if (line == heredoc) heredoc = ""
104+
next
105+
}
106+
out = ""; inS = 0; inD = 0
107+
n = length($0)
108+
for (i = 1; i <= n; i++) {
109+
c = substr($0, i, 1)
110+
if (!inD && c == "\047") { inS = !inS; continue }
111+
if (!inS && c == "\042") { inD = !inD; continue }
112+
if ((inS || inD) && (c == "&" || c == "|" || c == ";" || c == "(" || c == ")")) { out = out "\001"; continue }
113+
out = out c
114+
}
115+
if (match(out, /<<-?[[:space:]]*[A-Za-z_][A-Za-z0-9_]*/)) {
116+
tag = substr(out, RSTART, RLENGTH)
117+
sub(/^<<-?[[:space:]]*/, "", tag)
118+
heredoc = tag
119+
sub(/<<-?[[:space:]]*[A-Za-z_][A-Za-z0-9_]*.*$/, "", out)
120+
}
121+
flushline(out)
122+
}
123+
')
67124

68125
eff="$PWD"
69126
target=""
@@ -133,7 +190,10 @@ while IFS= read -r seg; do
133190

134191
# STAGE 4: only a manager-led segment can be an install.
135192
case "$head_tok" in
136-
npm|bun|pnpm|yarn|yarnpkg) ;;
193+
npm) verbs="$NPM_INSTALL" ;;
194+
bun) verbs="$BUN_INSTALL" ;;
195+
pnpm) verbs="$PNPM_INSTALL" ;;
196+
yarn|yarnpkg) verbs="$YARN_INSTALL" ;;
137197
*) continue ;;
138198
esac
139199
shift
@@ -142,13 +202,14 @@ while IFS= read -r seg; do
142202
printf '%s' "$seg" | grep -Eq "$GLOBAL" && continue
143203

144204
verdict=""
205+
if printf '%s' "$seg" | grep -Eq "$AUDIT_FIX"; then verdict="install"; fi
145206
prefix_dir=""
146207
pending_prefix=0
147208
while [ $# -gt 0 ]; do
148209
tok="$1"; shift
149210
case "$tok" in
150-
--prefix=*|-C=*) prefix_dir="${tok#*=}"; continue ;;
151-
--prefix|-C) pending_prefix=1; continue ;;
211+
--prefix=*|-C=*|--cwd=*|--dir=*) prefix_dir="${tok#*=}"; continue ;;
212+
--prefix|-C|--cwd|--dir) pending_prefix=1; continue ;;
152213
-*) continue ;;
153214
esac
154215
if [ "$pending_prefix" = "1" ]; then prefix_dir="$tok"; pending_prefix=0; continue; fi
@@ -159,13 +220,19 @@ while IFS= read -r seg; do
159220
# judged against the primary's own real node_modules and allowed. The FIRST
160221
# verdict wins; later tokens are only mined for the prefix.
161222
[ -n "$verdict" ] && continue
162-
if printf '%s' "$tok" | grep -Eq "^(${INSTALL_VERBS})$"; then verdict="install"; continue; fi
223+
if printf '%s' "$tok" | grep -Eq "^(${verbs})$"; then verdict="install"; continue; fi
163224
if printf '%s' "$tok" | grep -Eq "^(${SAFE_VERBS})$"; then verdict="safe"; continue; fi
164225
done
165226

166227
# A bare `yarn` (only flags, no verb) IS an install in yarn classic.
228+
# A flags-only `yarn` IS an install in yarn classic, but `yarn --version` and
229+
# `yarn --help` only print, so they must not be read as one.
167230
if [ -z "$verdict" ]; then
168-
case "$head_tok" in yarn|yarnpkg) verdict="install" ;; esac
231+
case "$head_tok" in
232+
yarn|yarnpkg)
233+
if printf '%s' "$seg" | grep -Eq '(^|[[:space:]])(--version|-v|-V|--help|-h)([[:space:]]|$)'; then :
234+
else verdict="install"; fi ;;
235+
esac
169236
fi
170237
[ "$verdict" = "install" ] || continue
171238

framework-dev.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,9 @@ Measured on npm 11.19.0 and bun 1.3.14:
162162

163163
So prevention lives one layer up, and the rest is repair:
164164

165-
- **Block.** `.claude/hooks/block-install-in-linked-worktree.sh` is a `PreToolUse` (Bash) hook, the only layer that sees the state before the package manager starts. It refuses an install verb whose target directory has a symlinked `node_modules`, covering every manager's documented aliases (`bun i` matters most, since Bun writes THROUGH the link) and the REMOVE verbs too, because `npm rm` in a linked worktree deletes from the checkout that owns the tree. Escape hatch `WEBJS_NO_WORKTREE_INSTALL_GATE=1`.
165+
- **Block.** `.claude/hooks/block-install-in-linked-worktree.sh` is a `PreToolUse` (Bash) hook, the only layer that sees the state before the package manager starts. It refuses an install verb whose target directory has a symlinked `node_modules`, covering every manager's documented aliases (`bun i` matters most, since Bun writes THROUGH the link), the REMOVE verbs, and `link` / `rebuild` / `prune` / `audit fix`, all of which mutate the tree the symlink points at. The tables are per manager, so `bun upgrade`, which upgrades the Bun binary, stays allowed while `pnpm upgrade` does not. Escape hatch `WEBJS_NO_WORKTREE_INSTALL_GATE=1`.
166166

167-
It judges a COMMAND, never a token. The command is split on `&&`, `||`, `;`, `|`, `(` and `)`, and each segment is judged only by what it STARTS with, after leading env assignments and wrappers like `sudo` are stripped. Matching the manager token anywhere in the line is the obvious shortcut and it is badly wrong: it blocks `git commit -m "fix: npm install ..."`, `grep -rn "npm ci" AGENTS.md` and `git log --grep "npm install"`. A linked worktree is the mandated working state here, so that fires on ordinary commands constantly, and a gate that cries wolf is a gate someone turns off. `npm test`, `npm run <script>`, `npx ...`, `npm init` and `yarn test` all pass, and so does a GLOBAL install (`-g` / `--global`), which writes to the npm prefix rather than through the link and is this repo's documented post-release step.
167+
It judges a COMMAND, never a token, in four stages. Quoted spans are neutralised FIRST, keeping their content but stripping the separators inside them, and heredoc bodies are dropped. The remainder is split on `&&`, `||`, `;`, `|`, `(`, `)` and newlines. Each segment is then judged by its FIRST token, after leading env assignments and wrappers like `sudo` are stripped. Only inside a manager-led segment are the remaining tokens scanned, for the first one recognised as either an install verb or a known safe verb, so a flag sitting before the verb does not hide it. Matching the manager token anywhere in the line is the obvious shortcut and it is badly wrong: it blocks `git commit -m "fix: npm install ..."`, `grep -rn "npm ci" AGENTS.md` and `git log --grep "npm install"`. A linked worktree is the mandated working state here, so that fires on ordinary commands constantly, and a gate that cries wolf is a gate someone turns off. `npm test`, `npm run <script>`, `npx ...`, `npm init` and `yarn test` all pass, and so does a GLOBAL install (`-g` / `--global`), which writes to the npm prefix rather than through the link and is this repo's documented post-release step.
168168
- **Report.** The root `preinstall` runs `scripts/warn-worktree-install.mjs`, which ALWAYS exits 0 and returns immediately unless `.git` is a FILE, so a normal clone and CI never see it. It names whichever of the three states it landed in and prints the repair.
169169
- **Repoint on teardown.** `.claude/hooks/cleanup-merged-worktree.sh` repoints any `<primary>/node_modules/@webjsdev/*` link targeting a worktree it is about to remove, before removing it.
170170
- **Repair on demand.** `npm run worktree:link` repairs the primary's `@webjsdev/*` scope: a dangling link, a link into a live foreign checkout, and an absolute in-primary link all become the relative form, and a DANGLING `.name-HASH` npm staging entry is dropped. A LIVE staging entry is left strictly alone. `npm run check:worktree-links` reports without writing and exits non-zero when there is work. Escape hatch `WEBJS_NO_WORKTREE_REPAIR=1` suppresses the repair WRITE only, so `--check` ignores it and still inspects; the `defaultPrimary()` test needs the hatch for the same reason it needs `WEBJS_NO_WORKTREE_SEED=1`: the repair pass sits ABOVE the primary-checkout guard by design, so it runs in both positions and would otherwise rewrite the real checkout during `npm test`.

test/hooks/block-install-in-linked-worktree.test.mjs

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,10 @@ test('blocks the HYPHENATED npm verbs, which the short aliases do not cover', ()
110110
test('blocks the REMOVE verbs, which delete from the owning checkout', () => {
111111
const { root, worktree } = makeLinkedPair();
112112
try {
113-
for (const cmd of ['npm uninstall x', 'npm rm x', 'npm r x', 'bun rm x', 'pnpm rm x', 'yarn remove x', 'pnpm upgrade', 'pnpm dedupe', 'yarn dedupe']) {
113+
// `npm r` and `npm a` are deliberately NOT in npm's table: they are rare as
114+
// commands and common as flag VALUES, and the scan cannot tell the two
115+
// apart, so admitting them blocks `npm -w a run build`. A documented gap.
116+
for (const cmd of ['npm uninstall x', 'npm rm x', 'npm unlink x', 'bun rm x', 'pnpm rm x', 'yarn remove x', 'pnpm upgrade', 'pnpm dedupe', 'yarn dedupe']) {
114117
assert.equal(runHook(cmd, worktree).status, 2, `expected block for \`${cmd}\``);
115118
}
116119
} finally { rmSync(root, { recursive: true, force: true }); }
@@ -193,7 +196,6 @@ test('an `=` in a FLAG does not disable the gate (fail-open regression)', () =>
193196
'npm install --workspace=packages/core',
194197
'bun install --backend=hardlink',
195198
'yarn add x --registry=https://r',
196-
'pnpm add x --dir=/y',
197199
'npm i -D esbuild --foreground-scripts=true',
198200
]) {
199201
assert.equal(runHook(cmd, worktree).status, 2, `expected block for \`${cmd}\``);
@@ -317,7 +319,7 @@ test('a flag VALUE never hides the verb, and a safe verb still stops the scan',
317319
} finally { rmSync(root, { recursive: true, force: true }); }
318320
});
319321

320-
test('follows every `cd` spelling, including ~, --, and pushd', () => {
322+
test('follows the `cd --`, `cd -P` and `pushd` spellings', () => {
321323
const { root, worktree } = makeLinkedPair();
322324
try {
323325
for (const cmd of [`cd -- ${worktree} && npm ci`, `pushd ${worktree} && npm ci`, `cd -P ${worktree} && npm ci`]) {
@@ -357,3 +359,91 @@ test('escalates to the git toplevel, so an install from a SUBDIRECTORY blocks',
357359
assert.equal(runHook('npm install', sub).status, 2, 'a subdirectory install still reaches the linked root');
358360
} finally { rmSync(root, { recursive: true, force: true }); }
359361
});
362+
363+
test('a QUOTED directory argument survives the quote handling', () => {
364+
// Quoted spans must lose their power to look like a command boundary while
365+
// KEEPING their content. Deleting the span instead leaves `cd` with no
366+
// argument, so the install is judged against the session cwd (the primary,
367+
// whose node_modules is real) and the headline scenario fails open. Quoting a
368+
// path is the ordinary defensive spelling.
369+
const { root, primary, worktree } = makeLinkedPair();
370+
try {
371+
for (const cmd of [
372+
`cd "${worktree}" && npm ci`,
373+
`cd '${worktree}' && npm ci`,
374+
`cd "${worktree}" && bun install`,
375+
`npm --prefix "${worktree}" install`,
376+
`npm install --prefix '${worktree}'`,
377+
]) {
378+
assert.equal(runHook(cmd, primary).status, 2, `expected block for \`${cmd}\``);
379+
}
380+
} finally { rmSync(root, { recursive: true, force: true }); }
381+
});
382+
383+
test('an apostrophe inside a double-quoted string does not swallow a later install', () => {
384+
// Running a single-quote pass before a double-quote pass pairs the apostrophe
385+
// in `can't` with the next single quote anywhere in the line and deletes
386+
// everything between, taking a real install with it.
387+
const { root, worktree } = makeLinkedPair();
388+
try {
389+
assert.equal(runHook(`echo "can't resolve" && npm i -D esbuild`, worktree).status, 2);
390+
assert.equal(runHook(`git commit -m "don't ship" && npm ci && echo "it's done"`, worktree).status, 2);
391+
} finally { rmSync(root, { recursive: true, force: true }); }
392+
});
393+
394+
test('a heredoc BODY is content, not commands', () => {
395+
// This repo's docs are full of `npm install` lines.
396+
const { root, worktree } = makeLinkedPair();
397+
try {
398+
assert.equal(runHook("cat > doc.md <<'EOF'\nnpm install\nEOF", worktree).status, 0);
399+
assert.equal(runHook('cat > doc.md <<EOF\nnpm ci\nEOF', worktree).status, 0);
400+
// ...but a real newline-separated install still blocks.
401+
assert.equal(runHook('echo hi\nnpm ci', worktree).status, 2);
402+
} finally { rmSync(root, { recursive: true, force: true }); }
403+
});
404+
405+
test('`npm audit fix` blocks while a bare `npm audit` does not', () => {
406+
// `audit fix` installs revised versions straight through the link, which is
407+
// the same test `rebuild`, `prune` and `link` pass.
408+
const { root, worktree } = makeLinkedPair();
409+
try {
410+
assert.equal(runHook('npm audit fix', worktree).status, 2);
411+
assert.equal(runHook('npm audit fix --force', worktree).status, 2);
412+
assert.equal(runHook('npm audit', worktree).status, 0);
413+
assert.equal(runHook('npm audit --json', worktree).status, 0);
414+
} finally { rmSync(root, { recursive: true, force: true }); }
415+
});
416+
417+
test('the verb tables are PER MANAGER, so one manager alias cannot fire for another', () => {
418+
const { root, worktree } = makeLinkedPair();
419+
try {
420+
// `a` is a BUN alias for add. As an npm flag VALUE it must mean nothing.
421+
assert.equal(runHook('npm --workspace a run build', worktree).status, 0);
422+
assert.equal(runHook('npm -w a run build', worktree).status, 0);
423+
assert.equal(runHook('bun a nanoid', worktree).status, 2, 'but it IS an install for bun');
424+
// `bun upgrade` upgrades the Bun BINARY and never touches node_modules.
425+
assert.equal(runHook('bun upgrade', worktree).status, 0);
426+
assert.equal(runHook('pnpm upgrade', worktree).status, 2, 'while pnpm upgrade DOES install');
427+
} finally { rmSync(root, { recursive: true, force: true }); }
428+
});
429+
430+
test('an informational flags-only yarn is not read as a bare install', () => {
431+
const { root, worktree } = makeLinkedPair();
432+
try {
433+
for (const cmd of ['yarn --version', 'yarn -v', 'yarnpkg --help', 'yarn -h']) {
434+
assert.equal(runHook(cmd, worktree).status, 0, `expected allow for \`${cmd}\``);
435+
}
436+
assert.equal(runHook('yarn', worktree).status, 2, 'a truly bare yarn still installs');
437+
assert.equal(runHook('yarn --frozen-lockfile', worktree).status, 2);
438+
} finally { rmSync(root, { recursive: true, force: true }); }
439+
});
440+
441+
test('mines --cwd and --dir as target directories, not just --prefix', () => {
442+
// Bun is the manager that writes THROUGH the link, so `bun --cwd` matters most.
443+
const { root, primary, worktree } = makeLinkedPair();
444+
try {
445+
assert.equal(runHook(`bun --cwd ${worktree} install`, primary).status, 2);
446+
assert.equal(runHook(`pnpm --dir ${worktree} install`, primary).status, 2);
447+
assert.equal(runHook(`yarn --cwd ${worktree} install`, primary).status, 2);
448+
} finally { rmSync(root, { recursive: true, force: true }); }
449+
});

0 commit comments

Comments
 (0)