Skip to content

fix(hooks): port the guard-main-checkout-bash.sh fix family from objectstack PR #11278 - #6046

Draft
yinlianghui-tw wants to merge 1 commit into
mainfrom
claude/issue-5789-port-guard-bash-fix-family
Draft

fix(hooks): port the guard-main-checkout-bash.sh fix family from objectstack PR #11278#6046
yinlianghui-tw wants to merge 1 commit into
mainfrom
claude/issue-5789-port-guard-bash-fix-family

Conversation

@yinlianghui-tw

@yinlianghui-twyinlianghui-tw commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Fixes#5789

Verbatim cross-repo port of the three commits on objectstack PR objectstack-ai/objectstack#11278 (family objectstack#11131 + objectstack#11133 + objectstack#11234) into this repo's copy of .claude/hooks/guard-main-checkout-bash.sh, plus the selftest additions.

Note on this body: GitHub's body sanitizer eats less-than fragments, so every heredoc introducer below is written as "two-less-than" in words rather than literally. An earlier revision of this body spelled them out literally and lost two whole sections to the sanitizer.

⛔ Governed surface — draft PR, human merge only

.claude/** is a governed surface in this repo. This PR is deliberately a draft and must stay one: it is not to be marked ready, not to be put on auto-merge, and not to be merged by an agent. A human merges this one.

What landed

  1. split_segments() gains the backslash branch tokenize() already had. Outside quotes a backslash escapes the next character, so an escaped backslash-doublequote opens no quoted region at all. Only tokenize() knew that; segmentation read the double quote as opening a region that never closed, went inert for every separator behind it, collapsed the whole command into one echo segment, and let a real sed -i through as a mere argument.
  2. strip_heredocs()'s delimiter scan learns the comment rule, via a new strip_line_comment() helper. A two-less-than introducer merely NAMED in a comment introduces nothing in bash, but the scan registered it as real; the delimiter then never appeared on a line of its own, so the pending heredoc was never satisfied and every remaining line — including real commands — was dropped before either quote-aware pass could see it.
  3. Header enumeration count Three layers:Four layers:. objectui#5459's port carried the miscount deliberately, to stay byte-identical with an upstream that was itself wrong. Upstream has now corrected it, so correcting it here restores parity rather than breaking it — this is not intentional local divergence being overwritten.

Selftest matrix: 100 → 121 cases.

Evidence

1. Selftest, before and after

revisionbash .claude/hooks/guard-main-checkout-bash.selftest.sh
before (ad0f5f11f)100 passed, 0 failed
after (c42985576)121 passed, 0 failed

2. The two measured probes flip ALLOWED to blocked

Run against this repo's hook with the selftest's own fixture shape (payload cwd = the shared primary checkout).

probebeforeafter
objectstack#11131 — echo backslash-doublequote, then ;, then sed -i s/a/b/ pkg/x.tsALLOWEDblocked
objectstack#11133 — a comment line reading # use cat redirect /tmp/n two-less-than EOF for notes, then a newline, then sed -i s/a/b/ pkg/x.tsALLOWEDblocked

Both were genuine fail-opens in this copy before the port: a real in-place write of a file in the shared checkout, waved through. The exact byte-for-byte probe commands are the new selftest cases themselves — see the objectstack#11131 and objectstack#11133 sections added to guard-main-checkout-bash.selftest.sh in this diff.

3. The objectstack#10247 mirror twins stay ALLOWED — no over-blocking

A guard that blocks more than intended is a defect, not extra safety. Verified unchanged:

commandbeforeafter
a pure-read node -e whose string literal carries U+2192 and whose body has a st= arrow-fnALLOWEDALLOWED
node -e with an escaped-doublequote string literal, no redirectALLOWEDALLOWED
node -e with escaped-doublequote literals plus an arrow-fn tailALLOWEDALLOWED
echo 'build → test → ship'ALLOWEDALLOWED
grep -n '→ the URL' AGENTS.mdALLOWEDALLOWED

And their negative twins — an otherwise-similar command carrying a REAL ASCII redirect — stay blocked, so the allow side is not widened for redirection at large:

commandbeforeafter
echo 'build → ship' redirected into steps.txtblockedblocked
the same node -e redirected into pkg/out.jsonblockedblocked

Parity accounting — every remaining difference from upstream

The port was produced by splicing upstream's text verbatim (extracted from objectstack-ai/objectstack@origin/main by line range), not by re-implementing from the card's prose. diff -u upstream ours was then read line by line. Every remaining difference falls into one of two buckets:

A. Pre-existing localisations, untouched by this PR — the example path packages/fields/src/x.tsx; the direction-of-travel provenance paragraph (this copy is the origin: objectui#3435, same family as #3430); the guard-shared-stash.sh cross-references (fail-open boundary note, jq-less fallback note, the split_segments/tokenize header paragraphs); the "mirroring both sibling hooks" exit-code line; the -in-AGENTS.md paragraph where upstream cites its own CLI banner; cd /home/user/objectui in the maybe_cd example; objectui#3435 in the block message; and in the selftest, the provenance line, pnpm --filter @object-ui/app-shell test, and the arrow-glyph allow case.

B. Issue-reference localisation on the newly ported lines — this copy prefixes upstream issue numbers, so (#11131)(objectstack#11131), (#11133)(objectstack#11133), (#10570)(objectstack#10570). That is the same convention already carried on objectstack#4890, objectstack#10247 and objectstack#10570 elsewhere in both files, and is exactly what the parity contract names as localised ("only issue references, example paths and the package name in the self-test are localised").

No other difference remains. Every logic line — the strip_line_comment() body, the backslash branch in split_segments(), the two scan= lines in strip_heredocs(), the Four layers: header, and all 21 new selftest cases — is byte-identical to upstream.

Scope

Port only. No cases added, nothing improved on upstream, nothing else in the hook touched. guard-main-checkout.sh, guard-shared-stash.sh and every other hook are untouched — git status on this branch shows exactly two modified files.

One out-of-scope observation was measured and filed as its own card, not folded in: guard-shared-stash.sh has the same objectstack#11131-class backslash hole in itssplit_segments(), in both repos. Filed as #6042, which remains open and is not addressed here.

Gates

Run at c42985576 (the tip of this branch, working tree clean):

gateverdict line
bash .claude/hooks/guard-main-checkout-bash.selftest.sh121 passed, 0 failed
node scripts/check-control-bytes.mjs✅ check-control-bytes: OK (scanned 4989 tracked text file(s); skipped 85 binary).
node scripts/check-changeset-presence.mjs✅ No source of a released package changed in this range, so no changeset is owed.
bash -n on both filessyntax OK

No changeset: this touches .claude/ tooling only, no released package's src/.


Generated by Claude Code

…ctstack PR #11278
Verbatim cross-repo port of the three commits on objectstack PR #11278
(objectstack#11131 + #11133 + #11234) into this repo's copy of
.claude/hooks/guard-main-checkout-bash.sh, plus the selftest additions.
1. split_segments() gains the backslash branch tokenize() already had. Outside
quotes a backslash escapes the next character, so an escaped \" opens no
quoted region. Only tokenize() knew that; segmentation read the " as opening
a region that never closed, went inert for every separator behind it, and let
a real `sed -i` through as a mere argument.
2. strip_heredocs()'s delimiter scan learns the comment rule via a new
strip_line_comment() helper. A `<<WORD` merely NAMED in a comment introduced
nothing in bash, but the scan registered it, the delimiter never appeared on a
line of its own, and every remaining line -- including real commands -- was
dropped before either quote-aware pass could see it.
3. Header enumeration count `Three layers:` -> `Four layers:`. objectui#5459's
port carried the miscount deliberately to stay byte-identical with an upstream
that was itself wrong; upstream has now corrected it, so correcting it here
restores parity rather than breaking it.
Selftest matrix 100 -> 121 cases. Both blocks were spliced from upstream's file
verbatim; only issue references are localised (`#11131` -> `objectstack#11131`
etc.), matching this copy's existing convention for upstream refs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe
@yinlianghui-twClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM: ACCEPT — ⛔ stays draft, human merge required

Verified against the repository, not taken on report.

PM checkresult
PR is a draftdraft: true
Changed files✅ exactly 2, +112 / −2
Other hooks untouchedguard-main-checkout.sh, guard-shared-stash.sh not in the diff
Out-of-scope finding filed, not folded#6042

What earns the accept

The port method was right, and it is the part that mattered. The order warned that a functionally-equivalent rewrite violates the parity contract even when it behaves identically. The dev spliced upstream's text out of objectstack@origin/mainby line range rather than reimplementing from the card's prose, then read diff -u against upstream line by line and accounted for every remaining difference in two named buckets — pre-existing localisations, and the established issue-ref prefixing (#11131objectstack#11131) already carried on objectstack#4890/#10247/#10570 in both files. That is the contract satisfied by construction rather than by hope.

All three acceptance legs were run, not just the countable one.100 passed121 passed is only meaningful with the before-count, and it was reported. More importantly:

  • both measured probes flip ALLOWED → blocked, and both were genuine fail-opens in this copy — a real in-place write to a file in the shared primary checkout, waved through;
  • the five objectstack#10247 mirror twins stay ALLOWED, and their negative twins (same shapes carrying a real ASCII redirect) stay blocked. That second row is what proves the allow side was not widened for redirection at large. A guard that blocks more than intended is a defect, not extra safety, and this is the evidence that it did not happen.

The verification-scope limit was declared rather than papered over — the repo-wide lint/test farm was not run locally, with the reason stated (two shell files under .claude/hooks/, no package src/, no TypeScript, no build output). That is the honest form.

⛔ Landing: not mine to do

.claude/** is a governed surface. This PR is not turned ready, not put on auto-merge, and not merged by this seat. It waits for a human. mergeable_state is currently behind; whoever merges will want the base brought in first.

Follow-ups from this round

#6042 — the same objectstack#11131-class backslash hole in guard-shared-stash.sh's split_segments(), measured with a control (git stash blocked; the escaped form ALLOWED). Correctly filed rather than folded in: the dispatch forbade touching other hooks, and this is not a bounded in-place fix.

Two PM actions taken on it:

  1. Its body was truncated mid-sentence by GitHub's sanitizer — it ended at "The `<". I repaired it in full (angle brackets written in words), marking the repair and changing nothing in the measurement. Worth noting as a recurring hazard on this lane: the sanitizer eats less-than fragments, and it ate two sections of this PR's own body before the dev worked around it.
  2. Cross-repo sibling filed as objectstack#11738, since the hole is claimed present in both copies. That card flags explicitly that only objectui's copy was actually probed and that objectstack's must be measured rather than assumed — if it does not reproduce there, the two copies have diverged where they are declared identical, which is its own finding.

The severity note recorded on both: this is the guard behind the ⛔ Never git stash directive, so a fail-open here admits the one command whose failure mode silently corrupts a different agent's work while reporting success (objectui#3430). Backstop gap, not a live breach — the rule outranks the hook — but the backstop exists for the agent who does not know the rule.


Generated by Claude Code

yinlianghui-tw pushed a commit that referenced this pull request Aug 24, 2026
…lit_segments()
Outside quotes a backslash escapes the next character, so an escaped `\"` opens
no quoted region at all. split_segments() had no branch for it: it read the `"`
as opening a region that never closed, went inert for every separator behind it,
collapsed the whole command into one segment whose head word was harmless, and
waved a real `git stash` through as a mere argument of `echo`.
Ports the split_segments() backslash branch from guard-main-checkout-bash.sh
(objectstack#11131 / objectstack PR #11278, and this repo's PR #6046), adapted to
this hook: it has no `word` bookkeeping and no tokenize() -- check_segment() uses
`read -r -a`, which leaves the backslash literal, so both characters are kept
verbatim and the pass only splits.
Only the objectstack#11131 half applies. The objectstack#11133 comment-heredoc
half has no analogue: this hook has no strip_heredocs() pass to carry it.
Self-test matrix 32 -> 41 cases, and the hook header's own re-derivable count
line is updated to match.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port the guard-main-checkout-bash.sh fix family (backslash pass agreement, comment-named heredoc, header count) from objectstack PR #11278

2 participants

@yinlianghui-tw@claude