Repository navigation
fix(scripts): stop the fixture-git-isolation gate crediting heredoc bodies - #3154
Merged
Merged
Conversation
…odies The gate scanned every physical line without tracking heredoc extent, so a credit-granting token inside a heredoc body was read as executed code. A heredoc body is data — those lines never run in the enclosing suite. Reproduced against main before changing anything. Three suites doing identical unisolated fixture work; only the one with no heredoc was caught: control.test.sh no token VIOLATION heredoc-unset.test.sh `unset GIT_DIR …` in a body isolated heredoc-scope.test.sh `fixture-isolation-scope:` in a body declared heredoc-source.test.sh `source <isolating harness>` in a body isolated All three credit paths leaked, not the two the issue named: SOURCE leaks too, and because CLEARS registers a file's basename as an isolating harness, a heredoc-only clear also let the enclosing file excuse every other file that sources it. Track extent instead of `<<` presence. Every delimiter a code line opens is queued (bash allows several per line); `<<-` tab-stripping, quoted and backslash-escaped delimiters are honoured; `<<<` here-strings are not heredocs; and a terminator must match exactly, because matching loosely would end a body early and restore credit inside it. The asymmetry is deliberate and pinned by its own test: CREDIT (CLEARS, SCOPE, SOURCE) is skipped inside a body since all three EXCUSE a suite, while FIXTURE detection keeps reading bodies since conscripting a suite is the safe direction the gate already commits to. The delimiter scan steps over quoted spans. Without that, the `<<` in `grep -q x <<<"a <<b"` opened a body on a delimiter that never reappeared and swallowed every real clear to end of file — caught by a probe, not by review. Scope and blast radius: the defect is latent. `--list` over the live corpus is byte-identical before and after (86 isolated, 0 violating), so no suite is mis-credited today and no suite loses credit. This is guardrail hardening, not an incident fix. Verification: - 7 new self-test cases, both directions, asserting exact exit codes rather than merely non-zero (#3109 notes a prior PR shipped a test that passed identically with its guard deleted). - Mutation-checked: the 4 defect-pinning cases fail against unpatched main with rc=0 (the gate reported OK), so they detect this defect specifically. - Full existing self-test, all 11 affected shell suites, and the 3 affected Python suites pass. ShellCheck and the portability gate are clean. - Runtime 1.10s -> 1.17s on the tracked corpus; the scan is gated on a cheap `<<` test and adds no process spawns. Closes #3109 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018hdhx7fLgEfda8fHJKw1Sc
Contributor
|
No description provided. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3109
Summary
scripts/check-fixture-git-isolation.shscanned every physical line without tracking heredoc extent, so a credit-granting token sitting inside a heredoc body was read as executed code. A heredoc body is data — those lines never run in the enclosing suite — so the gate could hand out isolation credit that was not in effect.#3109 was filed as recovered, unverified work, explicitly asking that the defect be demonstrated before anything was adopted. It was demonstrated first, and the recovered diff was not ported; this is a reimplementation against current
main.Fix
Heredoc extent is now tracked rather than inferred from
<<presence. Every delimiter a code line opens is queued (bash allows several per line);<<-tab-stripping and quoted or backslash-escaped delimiters are honoured;<<<here-strings are excluded; and a terminator must match the delimiter exactly, because matching loosely would end a body early and restore credit inside it.The asymmetry the issue called for is implemented and pinned by its own test:
CLEARS,SCOPE,SOURCE) is skipped inside a body. All three excuse a suite, and crediting a suite for data it merely printed is the under-selection this gate calls unsafe.unsetline, which is the safe direction the gate already commits to.Two findings beyond what the issue described:
SOURCEleaks too. The issue named two credit-granting intents; there are three. Sourcing an isolating harness from inside a body granted credit just as directly.CLEARSregisters a file's basename as an isolating harness, so a heredoc-only clear also let the enclosing file excuse every other file that sources it.The delimiter scan steps over quoted spans. Without that, the
<<ingrep -q x <<<"a <<b"opened a body on a delimiter that never reappeared and swallowed every real clear to end of file. That was caught by a probe, not by review.Verification
The defect was reproduced against unpatched
mainbefore any code changed. Four suites doing identical unisolated fixture work:maincontrol.test.shVIOLATION(correct)heredoc-unset.test.shunset GIT_DIR …in a bodyisolated(wrong)heredoc-scope.test.shfixture-isolation-scope:in a bodydeclared(wrong)heredoc-source.test.shsource <isolating harness>in a bodyisolated(wrong)Mutation-checked. #3109 warned that a prior PR shipped a test asserting only a non-zero rc that passed identically with its guard deleted. The 4 defect-pinning cases were therefore run against unpatched
main: all 4 fail there withrc=0— the gate reportedOK. They detect this defect specifically, not merely any failure. Every new case asserts an exact exit code.7 new self-test cases, both directions: 4 pinning the defect, 3 pinning that suppression did not break anything (extent ends at the terminator; a here-string does not open a body; fixture text in a body still conscripts).
Blast radius: the defect is latent.
--listover the tracked corpus is byte-identical before and after — 86 isolated, 0 violating. No suite is mis-credited today, and none loses credit. This is guardrail hardening, not an incident fix.Gates run locally, all green against current
main:scripts/check-fixture-git-isolation.test.sh—ALL PASS(existing cases plus the 7 new ones, and the live-corpus canary)scripts/affected-tests.sh --run— 11 affected shell suites passshellcheck --rcfile .shellcheckrc— cleanscripts/check-shell-portability.sh --paths— cleaneditorconfig-checker,typos— cleanscripts/check-changelog-parity.sh --check --check-order --check-bump— clean (no plugin touched, so no version bump applies)Runtime on the tracked corpus: 1.10s → 1.17s. The scan is gated on a cheap
<<test and adds no process spawns, which is what the file's stated performance discipline cares about.Related
git -Cdoes not survive an exported GIT_DIR #2840 — the fixture git-environment isolation work this gate enforces.fix/2840-fixture-git-isolation(commit68294cb7) still exists. scripts: the fixture-git-isolation gate may credit tokens inside heredoc bodies (recovered, unverified) #3109 step 4 calls for deleting it once the concept has landed; that deletion is left to a human and is not part of this PR.🤖 Generated with Claude Code
https://claude.ai/code/session_018hdhx7fLgEfda8fHJKw1Sc
Generated by Claude Code