Repository navigation
Commit 338297e
fix(hooks): read $(…), backticks and ${…} inside "…" as their own quoting context in every Bash guard (#25025)
Fixes #24901
Clause-②: no
## What
Every quote-aware segmenter in `.claude/hooks/` now reads a `$(…)`, a
backtick or a `${…}` met inside a double-quoted word as a quoting
context of its own, through one function, `subst_end()`. A `"` inside
such a body nests; it no longer closes the outer string. Before this,
the inner `"` was read as the closer, the rest of the body became bare
shell, and a pure read was blocked.
- `guard-main-checkout-bash.sh`: `subst_end()` (added by PR #24886 for
`$(…)` and backticks) now also answers where a quoted `${…}` ends, and
decides by itself whether the character in front of it opens such a
context. Its three call sites (the heredoc comment scan, segmentation,
tokenisation) now share that trigger too, so the passes cannot disagree
on which characters open a context.
- `guard-shared-stash.sh`, `guard-process-kill.sh`,
`guard-tree-enum.sh`: each carries a byte-identical copy of
`subst_end()`, and `split_segments()` delegates to it from its in-quote
branch. The three `split_segments()` are now byte-identical as well.
- `guard-tree-enum.sh` also gains the in-quote backslash branch (`\"`
inside "…" leaves the region open) that its two siblings already had.
Its header already said its pass was "kept case-for-case identical to
guard-shared-stash.sh's", but it was not. This is the in-place fix
described under Acceptance notes.
Diff: 8 files, +392 / -34, all under `.claude/hooks/` (Tier S).
## Measurement: every row of the card's enumeration table
On `62bc12930`, the dispatch base (pre-fix), and on this PR's head. Exit
2 = blocked, 0 = allowed. Every command below is a pure read.
| hook | command | pre-fix | this PR |
|:--|:--|:--|:--|
| main-checkout-bash | `echo "${x:-"a > b"}"` | 2, `target: b}` | 0 |
| main-checkout-bash | `echo "${x#"a > b"}"` | 2, `target: b}` | 0 |
| main-checkout-bash | `echo "${x:-"a; sed -i s/a/b/ pkg/x.ts"}"` | 2 |
0 |
| main-checkout-bash (heredoc scan) | a quoted `${x:-" #"}` on a
heredoc's introducing line | 2 (the ` #` read as a comment, so the
heredoc body was analysed) | 0 |
| shared-stash | `echo "a $(echo "x; git stash pop")"` (card position 1)
| 2 | 0 |
| shared-stash | ``echo "a `echo "x; git stash pop"`"`` | 2 | 0 |
| shared-stash | `echo "${x:-"a; git stash pop"}"` | 2 | 0 |
| process-kill | `echo "a $(echo "x; pkill -f node")"` | 2 | 0 |
| process-kill | `echo "${x:-"a; pkill -f node"}"` | 2 | 0 |
| tree-enum | `echo "a $(echo "x; ls *.yml")" && git show
origin/main:README.md` | 2 | 0 |
| tree-enum | `echo "${x:-"; ls *.yml"}" && git show
origin/main:README.md` | 2 | 0 |
| tree-enum | `echo "say \"; ls *.yml; git show origin/main:x\" ok"` | 2
| 0 |
So H4 is answered: both NOT MEASURED rows (`guard-process-kill.sh`,
`guard-tree-enum.sh`) carried the defect, and both are fixed here rather
than pinned clean.
Recall controls, unchanged (each blocked by its own hook before and
after): `echo hi > out.txt`, `sed -i "s/a/${x}/" pkg/x.ts`, `sed -i
"s/a/${x:-"b"}/" pkg/x.ts`, `echo "${HOME}" > pkg/x.ts`, `echo
"${x:-'a'}" ; sed -i s/a/b/ pkg/x.ts`, `X="${Y:-"z"}" && git stash push
-m wip`, `git stash push -m "${msg:-"wip; x"}"`, `kill $(pgrep -f
"${pat:-"node"}")`, `pkill -f "${pat:-"vitest; x"}"`, `for f in
"${dir:-"docs"}"/*.md; do git show "origin/main:$f"; done`. Every
pre-existing case in all five matrices passes unchanged.
Four fail-open holes close as a side effect, because the outer string no
longer hangs open. Each was ALLOWED before and is blocked now: `echo
"$(printf '"')" ; git stash pop`, `echo "${x:-$(printf '"')}" ; git
stash pop`, `echo "$(printf '"')" ; pkill -f vitest`, and in tree-enum
`echo "he said \"x\"" ; for f in *.yml; do git show "origin/main:$f";
done`.
## H2: how one rule lives in four hooks, an identical copy pinned
identical
The hooks share no code today. Each one is a standalone script
registered on its own in `.claude/settings.json`. The sharing that
exists is by copy: `guard-process-kill.sh` says its pass is "Copied
case-for-case from guard-shared-stash.sh's split_segments()", and
`guard-tree-enum.sh` says its pass is "Kept case-for-case identical". So
this PR keeps the copy form and adds the missing half, a pin. Each
copying hook's self-test extracts `subst_end()` (and `split_segments()`
where it is shared) from its hook and from the canonical file, and fails
unless the two are byte-identical and non-empty. A sourced helper file
was the alternative. It was not chosen, for three reasons:
1. It would make one file a single point of failure for all four guards
at once, the same reason AGENTS.md gives against a shared self-test
module.
2. It would add a failure mode the hooks do not have today: a missing or
unreadable helper would leave `SUBST_END` at 0 inside a `for` loop, so
it would need its own fail-open guard.
3. It would be a new file for the objectui mirror to carry, while the
copies already mirror one-for-one.
The prose claim alone had already drifted: `guard-tree-enum.sh` lacked
the in-quote escape branch while its header called the pass identical.
The pin catches exactly that, and it is ablated below.
## H3: a rule of its own for `${…}`, not failing open
The two routes the card allows, measured on
`guard-main-checkout-bash.sh`:
| command (a real write into the primary checkout) | pre-fix | route A
(this PR: track `${…}`) | route B (a quoted `${` makes the rest inert) |
|:--|:--|:--|:--|
| `sed -i "s/a/${x}/" pkg/x.ts` | 2 | 2 | 0 |
| `sed -i "s/a/${x:-"b"}/" pkg/x.ts` | 2 | 2 | 0 |
| `echo "${HOME}" > pkg/x.ts` | 2 | 2 | 0 |
| `echo "${x:-"a"}" ; sed -i s/a/b/ pkg/x.ts` | 2 | 2 | 0 |
| `echo "${x:-'a'}" ; sed -i s/a/b/ pkg/x.ts` | 2 | 2 | 0 |
| `cp "${SRC}" pkg/x.ts` | 2 | 2 | 0 |
Route B was prototyped as a one-line change, the `${` arm returning
"rest inert". It gives up the reflexive in-place edit with a variable in
its script, which is what this guard exists for. To narrow it to "only a
`${…}` whose body holds a `"`", it would first have to find where the
body ends, and that is route A's parse. Route A costs one more closer on
the existing stack in `subst_end()`, about 15 lines in one function that
all three passes already call. That is the least new parser surface on
which both passes agree.
**H3 premise, partly disproved by measurement.** The card says a `'`
inside `"${…}"` is literal. That holds for the expansion's output, and
for `bash --posix` and dash when finding the extent. It does not hold
for bash 5.2's default mode, which reads `'…'` as a quoted run while
looking for the closing brace: `x=; echo "${x:-'}'}"` prints `'}'` under
bash 5.2 and `''}` under `bash --posix` and dash. In those two, the
first `}` closes the expansion and `'}` is literal text after it. The
two readings agree only when the run holds none of `"` `\` `$` `}` or a
backtick. So such a run is skipped, and any other `'` there makes the
rest of the input one inert run (fail open). That is the header's own
contract: an extent that depends on the shell is not one the guard can
read with confidence. Both one-reading-only variants were ablated
(below), and each goes red.
**What route A gives up.** The PM asked that a real write the pre-fix
hook blocks stay blocked. I measured a 41-command corpus across the four
hooks, pre-fix and with this PR. Every write, stash or kill the pre-fix
hooks blocked stays blocked, except shapes the pre-fix reached only
through the misreading itself:
- An action nested inside a QUOTED substitution, reached through an
inner string: `echo "${x:-"$(sed -i s/a/b/ pkg/x.ts)"}"`, `echo
"${x:-"$(git stash pop)"}"`, `echo "a $(echo "$(git stash pop)")"`, and
the `pkill` twins. The pre-fix blocked them only because the inner `"`
exposed the body as bare shell, and the same misreading is what blocked
the pure reads above. `echo "$(git stash pop)"` was allowed before and
after. This is the class PR #24886 already made a deliberate fail-open
in `guard-main-checkout-bash.sh` ("a command inside a QUOTED $(…)"). The
`sed` shape is pinned in that hook's matrix. In the stash and kill
matrices the class is pinned by `echo "$(git stash pop)"` and `echo
"$(pkill -f node)"`.
- A target that holds an expansion: `echo hi > "${x:-"a > b"}"`. The
pre-fix blocked it on the wrong target, `b}`; it now fails open on its
`$`, which is the header's existing promise.
- A `'…'` run holding `}` or `"` inside a quoted `${…}`: `echo
"${x:-'}'}" ; sed -i s/a/b/ pkg/x.ts`. Bash and dash end the expansion
in different places. Its twin `echo "${x:-'}"; sed -i s/a/b/
pkg/x.ts'}"` is a pure read under bash and was a false block before.
Keeping the first group blocked would mean analysing quoted substitution
bodies as commands. That is a new claim with recursive parser surface,
and it would diverge from `guard-main-checkout-bash.sh`'s documented
fail-open. This PR does not do it, and the report puts it to the seat as
an open question.
**Boundary, written into the helper's header.** An unquoted `${…}`
holding a `)` inside a quoted substitution (`echo "$(echo ${x:-a)b})"`,
which bash prints as `a)b`) ends the body too early. The rest is then
read as it was before PR #24886. This is the same direction as the
existing `case`-pattern and comment boundary.
## H5: objectui port
This PR changes `guard-main-checkout-bash.sh` again. `subst_end()` gains
the `${…}` arm and the `'` rule, and now returns 1 for a non-opener. Its
three call sites change from testing two-character prefixes to the shape
`` { [ "$ch" = '$' ] || [ "$ch" = '`' ]; } && subst_end "$s" "$i" ``.
objectui#13260, the port of PR #24886 that mirrors this hook
case-for-case, should carry this delta too, along with the stash,
process-kill and tree-enum copies and their self-test pins. This PR
changes no objectui file.
## Tests, ablations and gates
Matrices and gates ran at head `1df8c6399`. The ablations ran at
`d83fca2fb`, whose eight files are byte-identical to the head's: the
merge touched nothing under `.claude/hooks/`.
Hook matrices (`bash .claude/hooks/X.selftest.sh`):
`guard-main-checkout-bash` 157 passed / 0 failed (141 on main, +16),
`guard-shared-stash` 65/0 (53, +12), `guard-process-kill` 80/0 (69,
+11), `guard-tree-enum` 50/0 (38, +12), `guard-main-checkout` 120/0
(unchanged).
The new self-tests against the pre-fix hooks (each base hook copied
beside its new matrix): main-checkout-bash 149 passed / 8 failed,
shared-stash 59/6, process-kill 74/6, tree-enum 43/7. In
main-checkout-bash the 8 are the `${…}` false blocks plus the four
documented fail-open pins that the pre-fix blocked. In the three copies
they are the false blocks, the fail-open mirrors and the identity pins.
Ablations went through `scripts/ablation-replace.mjs` in WRAP mode, with
the hook's matrix as the child. Each mutation was proven on disk (anchor
1 to 0, blob changed), and each restore was proven by blob equal to HEAD
and an empty `git diff HEAD`:
- A1 `${` arm removed from `subst_end()`: 8 red (every `${` form).
- A2 heredoc comment scan delegation only: 2 red (the `$(…)` and `${…}`
heredoc pins).
- A3 segmentation delegation only: 8 red.
- A4 tokenisation delegation only: 5 red (including `echo "${x:-"a >
b"}"` and `echo "${x#"a > b"}"`). So no single pass fixes `${…}` alone.
- A5a bash-only reading of `'` (the special-character check deleted): 1
red. A5b POSIX-only reading (the `'` arm deleted): 2 red. A5c a `'`
always fails open: 1 red (`echo "${x:-'a'}" ; sed …`, the recall case).
- A6 the stash guard's delegation removed: 5 red. A7 a behaviour-neutral
edit to the stash copy of `subst_end()` (locals reordered): exactly 1
red, the identity pin. A8 the tree-enum in-quote escape branch deleted:
3 red (both `\"` cases and the `split_segments()` pin).
Gates: `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` (no paths) derived 16 families, the same 16
as at dispatch. All 16 exited 0, each exit captured before any pipe.
`--ran` printed: "16 derived, 16 run, 0 NOT-MEASURED, 0 UNRUN". `pnpm
check:bash32-floor`: "19 constructs checked, floor bash 3.2".
`check:doc-formula-expressions` first answered PREREQUISITE NOT MET
(exit 3, unbuilt `@objectstack/formula` and `@objectstack/lint`). I
built both under the verify lock, and it then exited 0. `pnpm
check:pm-settings-deny-roster`: exit 0. Control-byte self-scan of the
eight files: no match.
Cost on a 1.9 KB, 25-line command of the card's probe shape:
main-checkout-bash took 146 ms pre-fix and 163 ms with this PR,
shared-stash 46 and 53 ms, process-kill 63 and 61 ms, tree-enum 63 and
55 ms.
No changeset: `.claude/**` publishes nothing (`skip-changeset`).
## Acceptance notes
- **In-place fix under the bounded exemption: the tree-enum in-quote
escape branch.** It is the same defect class as this card: a `"` that
bash does not read as closing the outer string was read as closing it.
The fix is mechanical, a verbatim copy of the branch its two siblings
carry, with its shape already pinned in their matrices. The file is
inside this card's declared surface, and the change is in the same gate
family. Measured before: `echo "say \"; ls *.yml; git show
origin/main:x\" ok"` exit 2 (a pure read), and `echo "he said \"x\"" ;
for f in *.yml; do git show "origin/main:$f"; done` exit 0 (a real sweep
let through).
- `guard-shared-stash.sh`'s header still says "Self-test (51 cases…)".
It was 53 on main and is 65 now. That is a stale count in prose, left as
is.
- Merged `origin/main` (`05b28c199`, 4 commits, none under
`.claude/hooks/`) before opening, per AGENTS.md §10. The union above was
re-run on the merge head.
## 维护者速读(草稿)
### 改了什么
修了四个 Bash 守卫钩子的同一类误判:命令里双引号包着的 `$(…)`、反引号或 `${…}`
里如果又带了引号,钩子会把内层引号当成外层字符串的结束,结果把纯只读命令误拦下来(比如 `echo "${x:-"a > b"}"`
被当成写文件)。现在四个钩子共用同一个判断函数,各自持有一份逐字节相同的副本,并由自测锁定"副本必须与原件相同"。顺带补上
`guard-tree-enum.sh` 漏掉的一条转义引号规则,它的注释早就声称与兄弟钩子一致,实际并不一致。
### 为什么改
这是 #24812 那次修复的同族收尾:上次只修了写保护钩子里的 `$(…)`,stash、杀进程、目录枚举三个钩子和 `${…}`
写法仍会误拦只读命令。钩子头部承诺"看不懂的一律放行,绝不误拦只读命令",误拦违背这个承诺,会诱使人把钩子关掉。
### 风险与代价(含回滚)
真正的写入、stash、按名杀进程、目录枚举误用仍全部拦截(原有用例全部通过)。代价:少数原本"碰巧被拦"的形状现在放行,即藏在双引号子命令里的写操作,以及
bash 与 POSIX shell 对引号理解不一致的极端写法;这与上次修复时接受的取舍一致,并已写入自测。回滚就是 revert 本
PR,只涉及 `.claude/hooks/` 下 8 个文件。
### 席位意见
### 你要做的
无需操作:这是 Tier S,席位按契约档复核后自行落地。objectui 的同名钩子由另一张卡(objectui#13260)跟进,本 PR
不改 objectui。
---
_Generated by [Claude
Code](https://claude.ai/code/session_01M82feuetLzrCiBz41BJb7L)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent c0cde8b commit 338297e
8 files changed
Lines changed: 392 additions & 34 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
326 | 326 | | |
327 | 327 | | |
328 | 328 | | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
329 | 353 | | |
330 | 354 | | |
331 | 355 | | |
332 | 356 | | |
333 | 357 | | |
334 | 358 | | |
335 | 359 | | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
336 | 367 | | |
337 | 368 | | |
338 | 369 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
47 | 50 | | |
48 | 51 | | |
49 | 52 | | |
| |||
52 | 55 | | |
53 | 56 | | |
54 | 57 | | |
55 | | - | |
56 | | - | |
| 58 | + | |
| 59 | + | |
57 | 60 | | |
58 | 61 | | |
59 | 62 | | |
| |||
121 | 124 | | |
122 | 125 | | |
123 | 126 | | |
124 | | - | |
125 | | - | |
126 | | - | |
127 | | - | |
128 | | - | |
129 | | - | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
130 | 136 | | |
131 | | - | |
132 | | - | |
133 | | - | |
134 | | - | |
135 | | - | |
136 | | - | |
137 | | - | |
138 | | - | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
139 | 154 | | |
140 | 155 | | |
141 | | - | |
142 | | - | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
143 | 163 | | |
144 | 164 | | |
145 | 165 | | |
146 | 166 | | |
147 | | - | |
| 167 | + | |
148 | 168 | | |
149 | | - | |
| 169 | + | |
150 | 170 | | |
151 | | - | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
152 | 185 | | |
| 186 | + | |
153 | 187 | | |
154 | 188 | | |
155 | 189 | | |
| |||
158 | 192 | | |
159 | 193 | | |
160 | 194 | | |
161 | | - | |
| 195 | + | |
162 | 196 | | |
163 | 197 | | |
| 198 | + | |
164 | 199 | | |
165 | 200 | | |
166 | 201 | | |
| |||
193 | 228 | | |
194 | 229 | | |
195 | 230 | | |
196 | | - | |
197 | | - | |
| 231 | + | |
| 232 | + | |
198 | 233 | | |
199 | 234 | | |
200 | 235 | | |
| |||
265 | 300 | | |
266 | 301 | | |
267 | 302 | | |
268 | | - | |
269 | | - | |
| 303 | + | |
| 304 | + | |
270 | 305 | | |
271 | 306 | | |
272 | 307 | | |
| |||
325 | 360 | | |
326 | 361 | | |
327 | 362 | | |
328 | | - | |
329 | | - | |
| 363 | + | |
| 364 | + | |
330 | 365 | | |
331 | 366 | | |
332 | 367 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
176 | 176 | | |
177 | 177 | | |
178 | 178 | | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
179 | 214 | | |
180 | 215 | | |
181 | 216 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
142 | 142 | | |
143 | 143 | | |
144 | 144 | | |
145 | | - | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
146 | 150 | | |
147 | 151 | | |
148 | | - | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
149 | 202 | | |
150 | 203 | | |
151 | 204 | | |
| |||
158 | 211 | | |
159 | 212 | | |
160 | 213 | | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
161 | 217 | | |
162 | 218 | | |
163 | 219 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
158 | 158 | | |
159 | 159 | | |
160 | 160 | | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
161 | 198 | | |
162 | 199 | | |
163 | 200 | | |
| |||
0 commit comments