Skip to content

[finding] guard-main-checkout-bash.sh: an unquoted backslash still makes tokenize() and split_segments() disagree, and a real sed -i into the shared checkout is allowed through #11131

Closed

Description

@os-zhuang

Measured while implementing #10570 (PR #11129) in the same function pair. Out of that card's adjudicated scope — #10570 is the comment rule and its direction is a false BLOCK; this is the opposite direction, a missed write. Filed unassigned, and NOT fixed in that PR.

An older observation card on this same shape existed and is gone (404 on both API and web, invisible to search), so this is filed fresh rather than folded. Dedup performed first: a search for the guard plus backslash returns only #10570, the closed duplicate #10720, and #5790.

What was measured

Against the committed hook (origin/main at 95437e7d2, and unchanged by PR #11129), from a cwd inside the shared primary checkout:

command: echo \" ; sed -i s/a/b/ pkg/x.ts      -> hook-exit=0   (ALLOWED)
command: sed -i s/a/b/ pkg/x.ts                -> hook-exit=2   (blocked)

Same write, same cwd. The only difference is the escaped double quote ahead of it, and it is enough to let a real in-place edit of the shared checkout through in silence.

Mechanism

The two quote-aware passes handle an unquoted backslash differently:

  • tokenize() has a backslash branch outside quotes: it consumes the next character as a literal, so an escaped quote never opens a quoted region.
  • split_segments() has no such branch. Outside quotes a backslash is an ordinary character, so the " that follows it is read as OPENING a double-quoted region. Quote state then never closes, every separator after it becomes inert, and the whole command collapses into one segment whose command name is echo — after which the sed -i is just another argument and collect_targets() has nothing to report.

Layer 3 of the hook's header documents the mirror-image case (an escaped quote INSIDE a double-quoted word, #10247, which was a false block). This is the case outside quotes, and it fails the other way.

Why it is worth a card

The header's precision-over-recall philosophy explicitly accepts fail-open holes, and each accepted hole is enumerated there: wrapped invocations, interpreters whose writes live in their own source text, targets carrying expansions or globs, relative targets with no cwd. This shape is not one of them — the write target is a plain shell token in plain sight, and the guard would name it correctly if the two passes agreed. The hole is a disagreement between passes rather than a decision, which is exactly the class the hook's own self-test discipline exists to pin.

Severity is bounded by the same thing that bounds every hole here: the guard targets the reflexive sed -i an agent reaches for mid-task, not a determined evader, and nobody writes an escaped quote on purpose to slip past it. The realistic path in is incidental — a command that legitimately carries an escaped quote and also writes.

Direction (not a decision)

Give split_segments() the backslash branch tokenize() already has, so the two passes agree on what a quote is; then pin both directions in guard-main-checkout-bash.selftest.sh, whose matrix already carries the #10247 twins next door.

Refs

.claude/hooks/guard-main-checkout-bash.sh — split_segments() and tokenize() · #10570 / PR #11129 (found here) · #10247 (the inside-quotes mirror image) · #5790 (why the Bash-side guard exists) · governed surface


Generated by Claude Code

Activity

  1. self-assigned this
    on Aug 23, 2026
  2. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    Claim: PM loop round 1 — family dispatch anchor (members: #11131, #11133, #11234; graded → dispatched in the same round's concentrated grading)
    Session: session_01RMTpSRF5CjMmQBFfPtPCwJ
    Branch: claude/issue-11131-guard-bash-pass-agreement (shared, named for the chain head; member cards carry no own branch by design)
    Worktree: objectstack-issue-11131
    Domain: domain:skills
    File surface: .claude/hooks/guard-main-checkout-bash.sh + .claude/hooks/guard-main-checkout-bash.selftest.sh (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus — dispatch-gates --tier: no path-derived mandate; parser logic + selftest matrix = judgment work
    Clause-②: no
    Serial constraints cleared: none in flight on .claude/hooks/** — scanned all 9 open PRs; last churn is merged PR #11129 (#10570 comment rule), which all three cards already account for.

    Fold answer (five gates): ① #11131+#11133 are one defect shape — an analysis pass disagreeing with its siblings/contract producing a false ALLOW, fixed by making the passes agree + pinning both directions; #11234 is declared a boundary-case rider (header enumeration count, one word, same file) folded deliberately to avoid a second serialized governed PR for one word — recorded as a deviation from strict shape-sameness. ② One file pair, one worktree, one changeset-exempt PR, one queue slot. ③ All three graded this round (concentrated grading, this session). ④ Per-member criteria: #11131 = the measured echo \" ; sed -i … probe flips ALLOWED→blocked with both directions pinned in the selftest; #11133 = the comment-named heredoc probe flips (real command after it analysed), pinned both directions; #11234 = header reads Four layers:. One commit per member, PR body carries one Fixes #n line per member. ⑤ Exclusions (look-alike, NOT in this family): the #10570/PR #11129 comment rule (landed — do not re-touch its behavior); guard-shared-stash.sh (different hook); the objectui ported copy (port follows AFTER this merges, filed as a follow-up card at ACCEPT per the declared objectstack→objectui direction of travel).


    Generated by Claude Code

  3. claude commented on Aug 23, 2026

    @claude
    Contributor
    {
      "issue": 11131,
      "status": "done",
      "branch": "claude/issue-11131-guard-bash-pass-agreement",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/11278",
      "premise_still_valid": true,
      "family": {
        "anchor": 11131,
        "members": [
          11131,
          11133,
          11234
        ],
        "commits": {
          "11131": "6bbc96e9 fix(hooks): give split_segments() the backslash branch tokenize() already had",
          "11133": "389fddcc fix(hooks): a heredoc introducer named inside a comment introduces nothing",
          "11234": "1a619db2 docs(hooks): the header enumerates four layers, not three"
        },
        "fixes_lines_in_pr_body": [
          "Fixes #11131",
          "Fixes #11133",
          "Fixes #11234"
        ]
      },
      "premise_recheck": {
        "method": "Cards were measured at 95437e7d2. Re-derived against current main a2ec377 (my BASE), not the measurement commit. Only churn on the two files since 95437e7d2 is merged PR #11129 (5fdd4637), exactly as all three cards state. All three probes re-measured on a2ec377 before any edit.",
        "11131": "VALID - probe reproduced fail-open on a2ec377",
        "11133": "VALID - probe reproduced fail-open on a2ec377",
        "11234": "VALID - header line 51 read 'Three layers:' above exactly 4 enumerated items"
      },
      "probe_readings": {
        "note": "hook exit code, cwd = /home/user/objectstack (the shared PRIMARY checkout, git-dir '.git'); the hook only inspects the payload, nothing is executed",
        "11131": {
          "probe": "echo \\\" ; sed -i s/a/b/ pkg/x.ts",
          "before": "hook-exit=0 ALLOWED",
          "after": "hook-exit=2 blocked",
          "control": "sed -i s/a/b/ pkg/x.ts -> hook-exit=2 blocked before AND after",
          "mirror_10247_must_stay_green": {
            "node -e with escaped quotes + st=>st.a": "hook-exit=0 ALLOWED before AND after",
            "grep -rn \"he said \\\"sed -i\\\" once\" .claude/": "hook-exit=0 ALLOWED before AND after"
          }
        },
        "11133": {
          "probe": "'# use cat > /tmp/n <<EOF for notes' NEWLINE 'sed -i s/a/b/ pkg/x.ts'",
          "before": "hook-exit=0 ALLOWED",
          "after": "hook-exit=2 blocked",
          "control": "'# plain prose comment' + same write -> hook-exit=2 blocked before AND after",
          "neighbours_unchanged": {
            "real heredoc body is prose": "hook-exit=0 ALLOWED before AND after",
            "introducing line's own redirect": "hook-exit=2 blocked before AND after",
            "heredoc + trailing inline comment, then a real write": "hook-exit=2 blocked before AND after"
          }
        },
        "11234": {
          "before": "line 51: '# Writing ABOUT the ban must never trip the ban (#4890's lesson). Three layers:' with 4 enumerated items",
          "after": "'Four layers:'; enumerated item count measured = 4; count line and items now agree"
        }
      },
      "selftest_counts": {
        "file": ".claude/hooks/guard-main-checkout-bash.selftest.sh",
        "before_BASE_a2ec377": "100 passed, 0 failed (exit 0)",
        "after_11131_cases": "110 passed, 0 failed (exit 0)",
        "after_11133_cases": "121 passed, 0 failed (exit 0)",
        "final_union_at_1a619db2": "121 passed, 0 failed (exit 0)",
        "cases_added": 21
      },
      "reverse_verification": {
        "method": "each fix reverted from its OWN commit via git checkout HEAD~1 -- <hook>, under a trap '<restore>' EXIT INT TERM; mutation proven absent on disk by grep -c on the injected anchors BEFORE reading any run result; restore leg proven byte-identical by an empty 'git diff --stat HEAD'",
        "11131": {
          "disk_proof": "grep -c 'the disagreement was a fail-OPEN hole' = 0; grep -c 'escapes the NEXT character' = 0",
          "direction_predicted": "turn red",
          "direction_observed": "turn red - 106 passed, 4 failed (selftest exit 1)",
          "reds": [
            "echo \\\" ; sed -i s/a/b/ pkg/x.ts",
            "echo \\\" ; rm -rf pkg/x.ts",
            "printf \\\" ; tee pkg/a.ts",
            "echo \\\" NEWLINE sed -i s/a/b/ pkg/x.ts"
          ]
        },
        "11133": {
          "disk_proof": "grep -c '^strip_line_comment()' = 0; grep -c on the scan call site = 0; grep -c on the OLD raw scan line = 1",
          "direction_predicted": "turn red",
          "direction_observed": "turn red - 117 passed, 4 failed (selftest exit 1)",
          "reds": [
            "# use cat > /tmp/n <<EOF for notes + sed -i",
            "# see the <<EOF trick + echo x >>",
            "git status   # cat <<MARKER writes notes + rm -rf",
            "# heredocs: <<-EOF and <<\"Q\" both introduce + touch"
          ]
        },
        "honest_deviation": "One case I first wrote as a discriminating twin, 'echo \\\" && echo x > pkg/x.ts', did NOT turn red on revert: a '>' target is collected wherever it appears, so it blocked even while the passes disagreed. What the disagreement actually lost was the COMMAND-NAME writers (sed -i / rm / tee), because the collapsed segment's head word became 'echo'. I relabelled it in the matrix as a plain regression pin with that explanation rather than leaving it to read as a pin on this fix. That is why the reverse-verification shows 4 reds and not 5."
      },
      "precision_sweep": {
        "requirement": "no NEW false BLOCKS (the hook's stated precision-over-recall philosophy)",
        "result": "full pre-existing allowed-forms matrix green: documented fail-open shapes, non-ASCII-operator section, #10247 section, #10570 comment section, escape hatch, jq-less fallback, no-cwd cases. 0 failures at every step.",
        "new_allow_twins_added": [
          "the escape must not manufacture a target: echo \\\" ; echo hello / echo \\\" ; cat README.md / echo a\\ b",
          "same command aimed at a linked worktree stays allowed (CWD=$WT)",
          "a QUOTED '#' on an introducing line must not truncate the scan - truncating there would expose the heredoc body as commands (load-bearing precision case)",
          "ordering pin: unbalanced apostrophe inside a heredoc BODY must not desynchronise the comment rule for following lines"
        ]
      },
      "gates": {
        "derivation": "node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, run on the actual change set with NO paths passed; script self-derived 2 paths, three-dot vs merge base a2ec37707. Answer declared from objectstack-ai/objectstack at commit 1a619db2. No path-derived model-tier mandate.",
        "families_named": 4,
        "verdict_lines": {
          "pnpm check:doc-authoring": "gate-exit=0 | 'doc authoring guard: 389 files clean - no bare metadata literals.'",
          "pnpm --filter @objectstack/lint run check:doc-formula-expressions": "gate-exit=0 | 'check:doc-formula-expressions: 22 record-scoped formula example(s) across 416 files / 1447 TS blocks judged clean by @objectstack/formula.'",
          "pnpm check:pm-governed-merges": "gate-exit=0 | 'check-governed-merges --self-test: 119 assertions'",
          "pnpm check:skill-frame-sync": "gate-exit=0 | 'check-skill-frame-sync: 4 copies of the decision frame are structurally isomorphic across 3 files'",
          "node scripts/check-nul-bytes.mjs": "gate-exit=0 | 'check-nul-bytes: OK (scanned 6332 text file(s) -- 6332 tracked, 0 untracked-not-ignored; skipped 5 binary; no raw ASCII control bytes).'"
        },
        "exit_code_discipline": "every gate run as 'cmd > file 2>&1; e=$?' - redirect-then-capture, never a pipe, so no $? reads back tail's status. Verdict lines quoted above are the gates' OWN printed lines.",
        "one_transient_red": "check:doc-formula-expressions first returned gate-exit=1 with ERR_MODULE_NOT_FOUND on packages/lint/node_modules/@objectstack/formula/dist/index.mjs - the stale/absent-dist trap in a fresh worktree, unrelated to this diff (two .sh files). Green after: pnpm --workspace-concurrency=2 --filter '@objectstack/lint^...' build.",
        "control_hook": "guard-shared-stash.selftest.sh - shares the Bash matcher and payload plumbing, UNTOUCHED by this PR (empty diff vs a2ec377 and vs HEAD). Read 32 passed, 0 failed BOTH before and after."
      },
      "changeset": {
        "decision": "skip-changeset (.claude/**-only, publishes nothing)",
        "label_write": "additive POST to /issues/11278/labels, per repo convention",
        "read_back": "['size/m', 'skip-changeset'] - skip-changeset survived the size-labeler's write; not clobbered"
      },
      "files_changed": [
        ".claude/hooks/guard-main-checkout-bash.sh (+64/-2)",
        ".claude/hooks/guard-main-checkout-bash.selftest.sh (+50)"
      ],
      "file_surface_check": "exactly the hook + its selftest, verified with git diff --stat against the recorded BASE a2ec377. No breach. Exclusions honoured: #10570/PR #11129 comment-rule behaviour untouched; guard-shared-stash.sh untouched; objectui copy NOT touched (port follows after merge).",
      "governed_surface": "PR opened as DRAFT (draft=true confirmed on read-back), base main, head 1a619db2. Auto-merge NOT armed, never flipped ready, human merge only - stated in the PR body.",
      "design_choice_11133": "Card named two candidates. Took candidate 1 (teach the delimiter SCAN the comment rule; keeps existing pass order), not candidate 2 (one merged scanner). Reason: the card's own ordering hazard - an unbalanced apostrophe in a heredoc BODY desynchronising a quote-aware comment scanner - is structurally unreachable in candidate 1, because body lines are consumed by the `pending` branch and `continue` before the scan. Also keeps the landed #10570 comment rule as the single implementation instead of forking a second copy. Treated as implementation judgment, not needs_decision: hook-internal, no public contract / spec / API shape involved. Pinned with its own both-direction selftest case.",
      "summary": "All three family members implemented on one branch, one commit each, one Fixes line each, per the PM fold ruling. #11131: gave split_segments() the backslash branch tokenize() already had, so the two passes agree that an unquoted backslash escapes the next character; the measured probe flips ALLOWED->blocked and the #10247 mirror twins stay green. #11133: taught strip_heredocs()'s delimiter scan the comment rule via a narrow strip_line_comment() helper (the card's candidate 1), so a <<WORD merely named in prose no longer swallows every command after it; probe flips ALLOWED->blocked with real heredoc behaviour untouched. #11234: header count 'Three layers:' -> 'Four layers:', now matching the 4 enumerated items. Selftest 100 -> 121 passed, 0 failed; both directions pinned for every behaviour change; all 4 derived gate families plus check:nul-bytes green at head 1a619db2.",
      "tests": "guard-main-checkout-bash.selftest.sh at head 1a619db2 (git rev-parse --short HEAD, run AFTER the final commit on a clean tree): '121 passed, 0 failed', exit 0. Baseline at BASE a2ec377: '100 passed, 0 failed'. Staged: 100 -> 110 (#11131 cases) -> 121 (#11133 cases). Control, untouched: guard-shared-stash.selftest.sh '32 passed, 0 failed' both before and after. Reverse-verification per member, each from its own commit with disk-level proof of the mutation (grep counts) and a trap-restored tree: revert #11131 -> '106 passed, 4 failed'; revert #11133 -> '117 passed, 4 failed'; restore legs proven byte-identical by empty 'git diff --stat HEAD'. Gates (derived, not recalled) all gate-exit=0 with their own verdict lines quoted above; exit codes captured by redirect-then-capture, never through a pipe.",
      "deviations": [
        "Reverse-verification for #11131 shows 4 reds, not 5: one case I authored blocks either way (see reverse_verification.honest_deviation). Relabelled in the matrix rather than quietly dropped.",
        "Layer 3's header prose was extended by 4 lines to state the outside-quotes half of the backslash rule. This is inside the declared file surface and deliberately adds NO fifth enumerated item, so #11234's corrected count stays right.",
        "'pnpm install --workspace-concurrency=2' is rejected by pnpm v10.31.0 as an unknown option (install only; the flag is valid for the recursive build form used later). Re-ran without it. No repo change; noting it because the os-dev toolchain guidance spells it as a general prefix.",
        "Report delivered at draft-PR time per the standing contract; CI convergence is the PM's to read. No CI polling was done."
      ],
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  4. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    ACCEPT (skills seat, session session_01RMTpSRF5CjMmQBFfPtPCwJ, reviewer of record) — family PR #11278, all three members (#11131, #11133, #11234).

    Checklist conclusions, verified against GitHub: draft ✓ · base main ✓ · three Fixes lines, one per member ✓ · 3 commits, one per member ✓ · changed files = exactly the declared pair (hook + selftest) ✓ · skip-changeset read back surviving the size-labeler ✓. Per-member criteria all met with measured probe flips (ALLOWED→blocked) and the #10247 mirror twins staying green; selftest 100→121 with per-member reverse verification (4 reds each, mutation proven on disk, restore byte-identical); precision sweep reports zero new false blocks; control hook (guard-shared-stash) untouched and green both sides. The honest deviation (one authored twin relabelled as a regression pin because a > target blocks regardless of pass disagreement) is exactly the right handling — recorded, not dropped. The #11133 design choice (candidate 1, comment rule taught to the delimiter scan, keeping #10570's implementation single) is sound implementation judgment: the ordering hazard is structurally unreachable there, and it is pinned.

    Governed surface (.claude/**): PR stays draft, human merge only; review requested from os-zhuang (author is claude[bot]). CI was still converging at review time — the seat's PR subscription will surface any failure and the drive-to-green posture applies. Follow-up filed at this ACCEPT per the fold's exclusion list: the objectui verbatim-port card (blocked on this family landing). Deviation noted for the runbook backlog: the os-dev toolchain guidance spells --workspace-concurrency=2 as a general pnpm prefix but pnpm 10.31 rejects it for install — one-line fact for the next references touch, riding the existing DEFER list.


    Generated by Claude Code

  5. removed their assignment
    on Aug 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions