Skip to content

guard-main-checkout-bash.sh reads a > inside a shell COMMENT as a redirect and false-blocks the command #10570

Closed

Description

@claude

Found while working #9830 (PR #10569). Out of that card's scope — it edits one workflow file and touches nothing under .claude/. Filed unassigned.

What was observed

guard-main-checkout-bash.sh does not recognise shell comments, so a > inside a # comment is read as a redirection and the whole command is BLOCKED. A comment cannot write anything, so this is a false block — the fail-closed direction, opposite to the fail-open concern recorded on #10409.

Reproduced against the committed hook, with the control that isolates the comment as the cause:

$ python3 -c "import json; print(json.dumps({'cwd':'/home/user/objectstack','tool_input':{'command':'# rename foo -> bar\necho hello'}}))" \
    | bash .claude/hooks/guard-main-checkout-bash.sh
⛔ Blocked: this Bash command WRITES into the shared PRIMARY checkout, not a worktree.
   command: # rename foo -> bar
   target:  bar
   repo:    /home/user/objectstack
hook-exit=2

$ python3 -c "import json; print(json.dumps({'cwd':'/home/user/objectstack','tool_input':{'command':'echo hello'}}))" \
    | bash .claude/hooks/guard-main-checkout-bash.sh
hook-exit=0

Same command, same cwd; the only difference is the comment line. grep -n "comment\|'#'" .claude/hooks/guard-main-checkout-bash.sh and the same grep over guard-main-checkout-bash.selftest.sh both return nothing — there is no # handling in either the tokenizer or the segmenter, and no self-test probe for the shape.

How it actually bit

A single Bash call carrying three labelled sub-runs, each preceded by an explanatory comment. One comment read # (c) a path already in the INDEX that the allowlist does not name -> the BAD re-check. The hook reported target: the and repo: /home/user/objectstack, blocking a command whose only file writes were into $TMPDIR and a scratchpad. Cost: one retry with the arrow removed from the prose. It looks exactly like the guard catching a real violation, which is what makes it worth a card rather than a shrug — the message names the primary checkout and a plausible-looking target.

Why it matters, and the honest severity

Low, and self-announcing. The failure is loud, and the workaround (reword the comment) is immediate. It never lets a write through.

The reason to fix it anyway is the hook's own stated philosophy, quoted from its header:

PRECISION OVER RECALL. Recognising a write target inside an arbitrary shell command is heuristic in a way that Edit's file_path is not, and a guard that blocks work it does not understand gets switched off — after which it guards nothing. So this hook only claims shapes it can read with confidence, and everything else fails OPEN.

A comment is not a shape the hook reads with low confidence — it is a shape it does not model at all, and it currently claims one with false confidence. Prose containing ->, =>, 2> or N > M is ordinary in explanatory comments, so the shape recurs.

Shape of a fix (not a ruling)

Give both quote-aware passes (tokenize() and split_segments()) the shell's comment rule: outside quotes, an unquoted # that starts a word begins a comment that runs to the next newline. Note the word-start condition — foo#bar and ${x#y} are not comments — so this is not a bare "strip from the first #".

Then extend guard-main-checkout-bash.selftest.sh in both directions, the discipline #10409 already argues for on this hook: a comment containing > must NOT block, and a real redirect on a line after a comment must still block.

Relationship to #10409

Same script, same two passes, different defect and opposite failure direction. #10409 is about a backslash outside quotes making the two passes disagree, and its direction is a missed write (fail-open). This one is a false block (fail-closed) from a construct neither pass models. Fixing #10409's backslash case does not address comments, so this is a sibling rather than a sub-issue — but the two land in the same function pair and would be cheaper to fix and self-test together.

Refs: #10409 (the sibling observation on this script) · #10406 / #10247 (the last change to these passes) · #9830 / PR #10569 (found here).


Generated by Claude Code

Activity

  1. os-zhuang commented on Aug 21, 2026

    @os-zhuang
    Contributor

    Triage: .claude/hooks/guard-main-checkout-bash.sh ⇒ domain:skills + finding, following the routing of its sibling #10409 (same script, same two passes). Skills-lane findings are graded by that seat (standing exception to the repo-wide first-touch round). Note for the grading seat: the card itself argues this and #10409 are cheaper fixed and self-tested together — fold-or-serial is yours to answer.


    Generated by Claude Code

  2. os-zhuang commented on Aug 21, 2026

    @os-zhuang
    Contributor

    Triage (dedup cross-link): #10720 was a later duplicate of this card, now closed as such. Its body carries the richer record — full mechanism walkthrough (split_segments on the comment's ;, no # stripping, -> tokenized as redirect), the harm argument (false positives train operators toward the OS_ALLOW_MAIN_EDITS=1 total override), and a concrete fix direction (strip # comments with the same quote/heredoc care strip_heredocs() already takes). Read it alongside this card at grading time.


    Generated by Claude Code

  3. os-zhuang commented on Aug 22, 2026

    @os-zhuang
    Contributor

    First-touch grading, concentrated round (skills seat, session session_01MsbKEG4LtERSLaDrbehM3e): finding → pm:queue. Low severity, self-announcing, but worth fixing on the hook's own stated philosophy: a comment is a shape it does not model yet claims with false confidence, and prose arrows recur. Scope: the shell comment rule (word-start unquoted # to newline — foo#bar/${x#y} excluded) in BOTH quote-aware passes, with self-test cases in both directions; check the sibling backslash observation on the same function pair before dispatch — if still open, the two are cheaper fixed and self-tested together. Governed face (.claude/hooks/**) — draft PR, human merge; re-run the hook's own selftest per its header.


    Generated by Claude Code

  4. self-assigned this
    on Aug 22, 2026
  5. os-zhuang commented on Aug 22, 2026

    @os-zhuang
    Contributor

    Claim: session_01MsbKEG4LtERSLaDrbehM3e branch=claude/issue-10570-hook-comment-rule

    skills 车道执行席认领,单卡派发。裁定重读完毕:定级评论(2026-08-22 15:35)即边界——shell 注释规则(词首未引 # 至换行;foo#bar/${x#y} 排除)入两个 quote-aware pass,自测双向扩展,治理面 draft + 人工合并。折叠判定结果:单卡派发——定级要求核姊妹反斜杠观察卡(fail-open 方向)状态,实测该卡(旧号 10409)API 与网页双 404、搜索不可见(已删除或转移),无可折叠对象;若 dev 在同一函数对上顺带核实反斜杠缺陷仍在,按发现纪律另立卡(先查重),⛔ 不在本单内修未裁定范围。补充输入:关闭的重复卡 #10720 载有更全机制走查(split_segments 在注释 ; 上切段、无 # 剥除、strip_heredocs 同款处理方向)与危害论证(假阳性把操作者推向 OS_ALLOW_MAIN_EDITS=1 总开关),随卡读。


    Generated by Claude Code

  6. added a commit that references this issue on Aug 22, 2026
    5f52070
  7. os-zhuang commented on Aug 22, 2026

    @os-zhuang
    Contributor

    os-dev-report

    (This comment was edited to repair sanitizer damage: the first posting contained a literal heredoc introducer, whose two less-than characters opened a tag-like span that swallowed the rest of that finding, the third finding and most of the summary. No raw less-than character appears below; angle brackets are spelled out.)

    {
      "issue": 10570,
      "status": "done",
      "branch": "claude/issue-10570-hook-comment-rule",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/11129",
      "premise_still_valid": true,
      "commits": ["5f520708c fix(hooks): guard-main-checkout-bash reads shell comments as text, not commands"],
      "gates": [
        "node scripts/pm/dispatch-gates.mjs, no hand-fed paths, run AFTER the final commit: 'change set derived from git — 2 path(s) vs merge base 95437e7d2' — 4 families named, all 4 run",
        "check:doc-authoring — verdict line: '✓ doc authoring guard: 389 files clean — no bare metadata literals.' (RC_doc_authoring=0)",
        "check:doc-formula-expressions — verdict lines: '✓ check:doc-formula-expressions self-test: 30 cases passed' and '✓ check:doc-formula-expressions: 22 record-scoped formula example(s) across 416 files / 1447 TS blocks judged clean by @objectstack/formula.' (first run RED for a missing dependency closure — I had skipped pnpm install in the worktree; re-run green after pnpm install plus the lint dependency-closure build)",
        "check:pm-governed-merges — verdict line: '✓ check-governed-merges --self-test: 119 assertions' (RC_pm_governed=0)",
        "check:skill-frame-sync — verdict line: '✓ check-skill-frame-sync: 4 copies of the decision frame are structurally isomorphic across 3 files'",
        "check:nul-bytes (any-edit family, not named by dispatch-gates) — verdict line: '✓ check-nul-bytes --self-test: 75 assertions over a temp git repo (real scan() path)'; direct control-character scan over both changed files returned no matches (grep exit 1)",
        "all run under scripts/pm/os-verify-lock.sh; longest hold was the lint dependency-closure build, 'VERDICT command-exit 0 · held the lock 137s (2m17s) · waited 0s'",
        "DECLARED NARROWING — repo-wide pnpm lint not run (CI owns it). Measured, not skipped: eslint's own config supplies the population and reports 'File ignored because no matching configuration was supplied' for BOTH changed paths; file count 2 read from --format json; invariance = every files: glob in the flat config is a TS/JS extension list and a scan of the config for any shell population returns nothing (exit 1), so an all-.sh diff cannot move any untouched file's verdict",
        "CI not waited on per the draft-PR contract"
      ],
      "tests": [
        "Hook self-test .claude/hooks/guard-main-checkout-bash.selftest.sh, per the hook header's own discipline: BEFORE '82 passed, 0 failed' then AFTER '100 passed, 0 failed' (18 new cases). Re-run at the final commit 5f520708c after the ablation restore: '100 passed, 0 failed'.",
        "Baseline repro of the card, against the committed hook before any edit: repro-a (a comment reading 'rename foo arrow bar' plus 'echo hello') hook-exit=2 with 'target: bar'; repro-c (the #10720 shape, a comment carrying both a semicolon and an arrow ahead of pure-curl calls) hook-exit=2 with 'target: pm:dispatched'; control repro-b (same command, no comment) hook-exit=0. After the fix all three exit 0.",
        "ABLATION, run from the COMMITTED state by a script whose trap on EXIT INT TERM owns the restore. No build step exists for a shell hook — the hook file IS the artefact the payload runs against — so 'reached disk' is the entire proof chain and it was asserted, never inferred from an editor's exit code.",
        "  on-disk proof, pre-ablation: marker occurrences 3, word-start guard lines 2.",
        "  on-disk proof, mutated: marker occurrences 0, guard lines 0, file 20702 down to 18156 bytes (delta -2546); the script hard-exits 9 if any of those three fails, so a no-op mutation could not have been read as a result.",
        "  on-disk proof, restored under trap: marker occurrences 3, guard lines 2, git status --porcelain on the hook = 0 lines.",
        "  measured with the mutation in place: the card's EXACT repro JSON blocks again — repro-a and repro-c both hook-exit=2, control repro-b still 0 — and the self-test reports '96 passed, 4 failed'.",
        "  DIRECTION as observed, not as templated: exactly the 4 allow-direction comment cases flip; the 14 recall and non-comment cases stay green in BOTH states. That is the expected asymmetry — the ablation removes only the false-block fix, so a case blocking for reasons the fix never touched cannot flip. Those 14 are regression pins, not discriminators for this change."
      ],
      "deviations": [
        "None on the adjudicated scope. One judgement call inside it, stated because it shapes the diff: in split_segments the word-state reset set is start-of-input, blank and the metacharacters semicolon, pipe, ampersand, parentheses, newline, and the two redirection operators — but NOT the braces, which are reserved words rather than metacharacters. This pass splits on braces anyway, so resetting word state there would make the hash of a dollar-brace-hash parameter expansion read as a comment and swallow the rest of the line INCLUDING a real redirect behind it. That would have been a recall regression, which the scope forbids. Pinned by two negative twins in the self-test: an array-length expansion and a prefix-strip expansion, each followed by a real redirect into pkg/x.ts, both still BLOCK.",
        "Procedural: I ran the first gate batch before pnpm install in the fresh worktree; one gate failed on a missing dependency and was re-run green after install plus the dependency-closure build. Recorded rather than quietly re-run.",
        "Procedural: this report comment was posted once and then edited to repair the sanitizer damage described at the top."
      ],
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #11131: an unquoted backslash still makes tokenize() and split_segments() disagree — measured, a real 'sed -i' into the shared checkout is ALLOWED (exit 0) when an escaped double quote precedes it, while the same sed -i alone exits 2. This is the sibling the dispatch asked me to cheaply verify: the old card is gone (404), so it is filed FRESH after dedup (search returned only #10570, closed #10720, #5790). Fail-OPEN direction, unadjudicated, NOT fixed in this PR.",
        "filed as #11133: a heredoc introducer NAMED inside a comment makes strip_heredocs() swallow the real commands after it — a comment that merely mentions a cat redirect with a heredoc introducer, followed on the next line by a real 'sed -i s/a/b/ pkg/x.ts', exits 0, while the same write under a plain prose comment exits 2. Confirmed pre-existing at origin/main 95437e7d2 and unchanged by this PR. Lives in the THIRD pass, outside this card's two-pass scope, and carries a real ordering design question (comments cannot simply be stripped before heredocs — an unbalanced apostrophe in a heredoc body would desynchronise a quote-aware comment scanner), which is why it is a separate card rather than a rider.",
        "filed as objectstack-ai/objectui#5712: objectui's copy of this hook, declared 'case-for-case identical', now carries a SECOND drift item — it still reads a shell comment as a command. Filed at the repo where the fix lands, referencing the existing open port card objectui#5459 (scoped to the quote-escape fix, so this is a sibling to fold into one port PR, not a duplicate). Dedup done first; objectui's 'finding' label verified to exist before use."
      ],
      "summary": "Both quote-aware passes of .claude/hooks/guard-main-checkout-bash.sh now apply the shell's comment rule: outside quotes and heredoc bodies, an unquoted hash that starts a WORD begins a comment running to the next newline. split_segments() gains a word-state flag so a separator inside a comment never splits; tokenize() keys the same rule on its existing 'have' flag, which already means 'a word is open'. Nothing changes about what the hook blocks once it has a real command — the word-start condition is the whole rule, and foo#bar, parameter expansions, quoted hashes, URL fragments and sed 's/#//' keep every verdict they had. Self-test extended in both directions, 82 to 100 cases, with the recall side pinned by negative twins. Draft PR #11129, assigned to os-zhuang, skip-changeset applied and read back as a union with the bot's size/s."
    }

    Generated by Claude Code


    Generated by Claude Code

  8. added 2 commits that reference this issue on Aug 23, 2026
    5fdd463
    764dbbc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions