Skip to content

Commit 0017635

Browse files
committed
fix: stop an = in a flag from disabling the install gate
The env-assignment strip used an unanchored `[A-Za-z_]*=*` case glob, which matches the WHOLE segment whenever any LATER token carries an `=`. It then ate the leading words, so `npm install --omit=dev`, `npm ci --loglevel=error` and `npm install --workspace=packages/core` were all ALLOWED. That is the one direction that matters, since the gate exists to stop a write, and the last is an ordinary command in this monorepo. It also made the `--prefix=` branch of the regexes dead while the space-form test stayed green over it. The strip is token-wise now, and the assignment test is anchored to the first token alone. Walking past arbitrary tokens after a wrapper turned out to re-create the token-anywhere class one level in: with `command` and `bash` treated as wrappers, `command -v yarn` blocked and `bash -c "echo yarn"` would have. So the walk covers a wrapper's own flags and their values only, and `command`, `exec`, `bash` and `sh` are not wrappers. `bash -c "npm ci"` is a documented accepted gap rather than a parser for nested shells.
1 parent 4c6b094 commit 0017635

2 files changed

Lines changed: 91 additions & 12 deletions

File tree

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

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,22 +69,58 @@ GLOBAL='(^|[[:space:]])(-g|--global)([[:space:]]|$)'
6969
eff="$PWD"
7070
target=""
7171
while IFS= read -r seg; do
72+
# Quotes are dropped so a quoted command body tokenizes (`bash -c "npm ci"`).
73+
# This is safe: what keeps `git commit -m "npm ci"` out is the command-position
74+
# rule below, never the quoting.
75+
seg=$(printf '%s' "$seg" | tr -d '"'"'"'')
7276
seg="${seg#"${seg%%[![:space:]]*}"}"
7377
[ -z "$seg" ] && continue
7478

75-
# Strip leading env assignments and benign wrappers, so `FOO=1 npm ci` and
76-
# `sudo npm ci` are still judged on the manager that follows them.
77-
while :; do
78-
case "$seg" in
79-
[A-Za-z_]*=*)
80-
rest="${seg#* }"; [ "$rest" = "$seg" ] && break
81-
seg="${rest#"${rest%%[![:space:]]*}"}" ;;
82-
sudo\ *|env\ *|time\ *|nice\ *)
83-
rest="${seg#* }"
84-
seg="${rest#"${rest%%[![:space:]]*}"}" ;;
85-
*) break ;;
79+
# Strip leading env assignments and benign wrappers, so `FOO=1 npm ci`,
80+
# `sudo npm ci` and `bash -c "npm ci"` are still judged on the manager that
81+
# follows them.
82+
#
83+
# This is done TOKEN BY TOKEN, and the env-assignment test is anchored to the
84+
# first token alone. An unanchored `[A-Za-z_]*=*` case glob matches the WHOLE
85+
# segment whenever any LATER token contains `=`, so it ate leading words and
86+
# `npm install --omit=dev`, `npm ci --loglevel=error` and
87+
# `npm install --workspace=packages/core` all failed OPEN. Failing open is the
88+
# one direction that matters here, since the whole point is to stop a write.
89+
wrapper_seen=0
90+
prev_flag=0
91+
for _ in 1 2 3 4 5 6 7 8 9 10; do
92+
first="${seg%%[[:space:]]*}"
93+
[ -n "$first" ] || break
94+
strip=0
95+
case "$first" in
96+
*=*)
97+
# A real env assignment: NAME=..., NAME being a valid shell identifier.
98+
name="${first%%=*}"
99+
case "$name" in
100+
''|*[!A-Za-z0-9_]*|[0-9]*) ;;
101+
*) strip=1 ;;
102+
esac ;;
103+
sudo|env|time|nice) wrapper_seen=1; prev_flag=0; strip=1 ;;
104+
npm|bun|pnpm|yarn|yarnpkg) ;;
105+
-*)
106+
# A wrapper's OWN flag, e.g. `sudo -u foo npm ci`.
107+
[ "$wrapper_seen" = "1" ] && { prev_flag=1; strip=1; } ;;
108+
*)
109+
# The VALUE of the wrapper flag just stripped, e.g. the `foo` in `-u foo`
110+
# or the `10` in `nice -n 10`. Deliberately narrow: walking past ARBITRARY
111+
# tokens after a wrapper re-creates the token-anywhere class one level in,
112+
# where `bash -c "echo yarn"` would reach the bare-yarn branch and block.
113+
# `command`, `exec`, `bash` and `sh` are NOT wrappers here for that reason,
114+
# so `command -v yarn` stays allowed and `bash -c "npm ci"` is a known,
115+
# accepted gap rather than a parser for nested shells.
116+
if [ "$wrapper_seen" = "1" ] && [ "$prev_flag" = "1" ]; then prev_flag=0; strip=1; fi ;;
86117
esac
118+
[ "$strip" = "1" ] || break
119+
rest="${seg#*[[:space:]]}"
120+
[ "$rest" = "$seg" ] && { seg=""; break; }
121+
seg="${rest#"${rest%%[![:space:]]*}"}"
87122
done
123+
[ -n "$seg" ] || continue
88124

89125
case "$seg" in
90126
cd|cd\ *)

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

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,55 @@ test('never blocks a GLOBAL install, which writes to the npm prefix not the link
154154
test('still sees the install through env assignments and benign wrappers', () => {
155155
const { root, worktree } = makeLinkedPair();
156156
try {
157-
for (const cmd of ['WEBJS_X=1 npm ci', 'sudo npm ci', 'time npm install', 'FOO=a BAR=b bun install']) {
157+
for (const cmd of [
158+
'WEBJS_X=1 npm ci', 'sudo npm ci', 'time npm install', 'FOO=a BAR=b bun install',
159+
// A wrapper carrying its OWN flags, which the strip must walk past.
160+
'sudo -u foo npm ci', 'nice -n 10 npm install',
161+
]) {
162+
assert.equal(runHook(cmd, worktree).status, 2, `expected block for \`${cmd}\``);
163+
}
164+
// `command`, `exec`, `bash` and `sh` are deliberately NOT wrappers. Walking
165+
// into a nested shell command re-creates the token-anywhere class one level
166+
// in, where `bash -c "echo yarn"` reaches the bare-yarn branch. So a lookup
167+
// stays allowed, and `bash -c "npm ci"` is a known accepted gap.
168+
assert.equal(runHook('command -v yarn', worktree).status, 0);
169+
assert.equal(runHook('bash -c "echo yarn"', worktree).status, 0);
170+
} finally { rmSync(root, { recursive: true, force: true }); }
171+
});
172+
173+
test('an `=` in a FLAG does not disable the gate (fail-open regression)', () => {
174+
// The env-assignment strip is anchored to the FIRST token. An unanchored
175+
// `[A-Za-z_]*=*` case glob matches the WHOLE segment whenever any LATER token
176+
// carries an `=`, so it ate the leading words and every one of these was
177+
// ALLOWED, which is the one direction that actually matters: the gate exists
178+
// to stop a write, and `npm install --workspace=packages/core` is an ordinary
179+
// command in this monorepo that corrupts the primary.
180+
const { root, worktree } = makeLinkedPair();
181+
try {
182+
for (const cmd of [
183+
'npm install --omit=dev',
184+
'npm ci --loglevel=error',
185+
'npm install --workspace=packages/core',
186+
'bun install --backend=hardlink',
187+
'yarn add x --registry=https://r',
188+
'pnpm add x --dir=/y',
189+
'npm i -D esbuild --foreground-scripts=true',
190+
]) {
158191
assert.equal(runHook(cmd, worktree).status, 2, `expected block for \`${cmd}\``);
159192
}
160193
} finally { rmSync(root, { recursive: true, force: true }); }
161194
});
162195

196+
test('the --prefix= EQUALS form is reachable, not just the space form', () => {
197+
// The regexes carry `[[:space:]=]+` deliberately, and the pre-existing prefix
198+
// test used only the space form, so the equals branch was dead while green.
199+
const { root, primary, worktree } = makeLinkedPair();
200+
try {
201+
assert.equal(runHook(`npm --prefix=${worktree} install`, primary).status, 2);
202+
assert.equal(runHook(`npm install --prefix=${worktree}`, primary).status, 2);
203+
} finally { rmSync(root, { recursive: true, force: true }); }
204+
});
205+
163206
test('bare `yarn` blocks only in COMMAND position, never as a trailing word', () => {
164207
// The bare-yarn branch first lived inside the generic VERBS pattern, whose
165208
// prefix any space satisfies, so it matched the token ANYWHERE and blocked

0 commit comments

Comments
 (0)