fix(scripts): split every shell separator before the exec-bit guard extracts

[devantler]

Why

The exec-bit guard checks that every script CI runs directly is committed as executable. Without that bit, the CI step fails before it checks anything. The guard could be fooled by how a command line is written: with no space around a separator, or with the direct call placed before a separator, a non-executable script went unchecked and the guard still reported success. In one case it did the opposite and flagged a script nothing runs directly.

What

The guard now splits command lines at every shell separator in one pass before judging them, leaving quoted text and redirections alone. Each command is therefore judged on its own however it is spelled. The repository's real scripts produce exactly the same result as before, so this closes the gaps without new false alarms.

Fixes #3693
Part of #3692

🤖 Generated with Claude Code

https://claude.ai/code/session_011Aut24ni8XbKPYyY5mYrbb

---

Generated by Claude Code

[devantler]

🤖 Generated by the Agentic Engineer (interactive Claude Code session)

Evidence for head fcbf8de616bb097235d14b7411910de516af9c8c

RED: the new fixtures against main's guard (e6fd759f)

Running the extended scripts/tests/test-guard-tracked-exec-bit.sh against the unchanged guard gives 7 failures:

fixture	main	want
echo ready| scripts/x.sh (glued pipe)	rc=0	1
echo ready&& scripts/x.sh (glued &&)	rc=0	1
echo ready;scripts/x.sh (glued both sides)	rc=0	1
sleep 1 & scripts/x.sh (background &)	rc=0	1
./scripts/x.sh; bash ./scripts/other.sh (direct call before ;)	rc=0	1
./scripts/x.sh | bash ./scripts/other.sh (direct call before |)	rc=0	1
echo "done;" scripts/x.sh (quoted ;)	rc=1, a false positive	0

The first six are fail-opens. The first three are the #3693 table, and the next three are the same class found while fixing it. The last is a live false positive: the classifier's quote-unaware ; split manufactured a command position inside a quoted string.

GREEN: this head

• The full suite passes: 41 assertions, all exec-bit guard assertions passed. All 29 pre-existing assertions pass unmodified, and 12 were added.
• The interpreter controls stay accepted (ready| bash …, ready;bash …), so the fix is not "demand the bit after every separator". The same holds for the redirection control (cmd 2>&1 scripts/x.sh) and for a real && following a quoted ;.
• Quote-awareness ablation: with the pass's quote tracking disabled, exactly the two quoted-separator fixtures fail (echo "done;" … and echo 'a |' …). Those fixtures therefore discriminate and do not pass vacuously.

Conservation (#3692 / #3693 acceptance)

The directly-invoked set is identical before and after. Both guards were run over the same tree with the resolved set dumped, from scratch copies that differed only by that one dump line:

• main tree: old guard vs new guard → identical (36 tracked + 2 untracked fixture paths), diff empty
• this branch's tree: new guard → the same set, exec-bit guard OK; 36 directly-invoked script(s) are tracked 100755
• Non-vacuity control: the same comparison over a one-line tree containing echo ready| scripts/x.sh → the sets differ (> scripts/x.sh). The comparison can see a change when one exists.

Portability

The pass is plain POSIX awk. Over the full scanned corpus (56,241 lines, 76,020 after splitting), its output is byte-identical under mawk (Ubuntu's default awk) and goawk (a strict POSIX implementation). The whole suite also passes with goawk substituted as awk. I could not run BSD awk here.

Validation

shellcheck over the guard and its test ✅ · bash scripts/guard-tracked-exec-bit.sh . ✅ · the test suite ✅. These are exactly the commands CI's exec-bit step runs.

No runtime surface beyond CI: this guard only runs in CI, and the fixtures above are its real invocation path.

Scope note: this closes #3693 and meets #3692's fixture, quoted-separator, redirection and conservation criteria. It does not add #3692's inversion, where a prefix the classifier cannot classify raises a named error. That stays open on #3692.

---

Generated by Claude Code

[devantler]

🤖 Generated by the Agentic Engineer

@coderabbitai review

---

Generated by Claude Code

[coderabbitai]

@devantler I’ll review the current changes in #4056.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Warning

Review limit reached

Next included review available in 55 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Advanced

Run ID: ab863f89-b3fc-4df9-bbc8-16f9ddf72261

📥 Commits

Reviewing files that changed from the base of the PR and between e6fd759 and 0d19406.

📒 Files selected for processing (2)

• scripts/guard-tracked-exec-bit.sh
• scripts/tests/test-guard-tracked-exec-bit.sh

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devantler]

🤖 Generated by the Agentic Engineer (interactive Claude Code session)

Evidence for head 0d194063e02f0fe778fc5f3940d2c76dca267737 (the independent reviewer's fix on top of fcbf8de6)

What changed and why

Review of fcbf8de6 found a new fail-open. The quote-aware split resets its quote state at each line, so the closing quote of a multi-line string (echo "multi … line"; ./scripts/x.sh) opened a quote that ran to the end of that line. Every separator after it stayed unsplit, and the direct invocation behind it was never judged. On main, the whitespace &&/|| split and the classifier's ;/| reset caught this shape. Now a quote counts only when its partner is on the same line; an unpaired quote is plain text and splitting continues past it.

RED / GREEN (four new UNPAIRED fixtures, each a run block whose second line closes the string)

closing line	main (e6fd759f)	fcbf8de6	this head
line"; ./scripts/fixture-target.sh	rc=1	rc=0 (fail-open)	rc=1
line" && scripts/fixture-target.sh	rc=1	rc=0	rc=1
line"| ./scripts/fixture-target.sh	rc=1	rc=0	rc=1
line"; bash ./scripts/fixture-target.sh (interpreter control)	rc=0	rc=0	rc=0

The full suite at this head passes: 45 assertions. The earlier 41 pass unmodified. It passes under both mawk (the system awk) and goawk. shellcheck is clean on the guard and its test.

Conservation

I dumped the resolved directly-invoked set from three scratch copies of the guard: main's, fcbf8de6's and this head's. The copies differ only by that dump line. I ran each over both the main tree and this branch's tree. All six sets are identical: 38 lines, which are the 36 tracked scripts plus the 2 untracked fixture paths. So the fix adds no new false positive on the real scripts. Non-vacuity: over the line"; ./scripts/fixture-target.sh fixture, the fcbf8de6 and this-head sets differ (> scripts/fixture-target.sh).

Exposure measured over the scanned corpus: 2,275 lines end with an unpaired quote (mostly prose apostrophes). Today none of them has a separator followed by a script path, so the gap was latent, not live.

CI

At this head, the 🔍 Detect Changes job runs this guard over the tree and runs its test suite, with no path filter. It succeeded, and all 28 checks are success or skipped.

---

Generated by Claude Code

[devantler]

🤖 Generated by the Agentic Engineer

Self-review (fallback — CodeRabbit, Codex and Cursor Bugbot unavailable)

Reviewed commit: 0d194063e02f0fe778fc5f3940d2c76dca267737

• CodeRabbit: requested at this head 2026-09-23T05:13:4xZ (comment 5789467650); replied "Review limit reached — next included review available in 55 minutes" at 2026-09-23T05:13:57Z. No review was delivered at this head.
• Codex: usage limit reached (measured 2026-09-22T20:53:06Z on monorepo#3507; maintainer-only, not re-requested)
• Cursor Bugbot: usage limit reached (measured 2026-09-22T20:53:35Z on monorepo#3507; maintainer-only, not re-requested)

What I checked (independent reviewer; I also authored the second commit, which fixes the P1 below, and reviewed the whole diff again at this head):

• P1 found at fcbf8de6 and fixed here: quote state reset per line, so an unpaired quote (the closing line of a multi-line string) hid every separator after it. line"; ./scripts/x.sh, line" && scripts/x.sh and line"| ./scripts/x.sh all passed the guard over a 100644 script, while main rejected them. That was a new fail-open. Now a quote counts only when its partner is on the same line. Four fixtures pin it: three fail at fcbf8de6 and pass here, and one interpreter control is accepted by all three guards.
• Correctness of the split: &&, ||, ;, |, |&, ( and a lone & split. The redirection forms 2>&1, >&2, <&3, &> and >| do not. A backslash escape outside quotes, and inside double quotes, is honoured identically by the walk and by the new closes() look-ahead. Single-quoted text honours no escapes, matching bash.
• The classifier no longer resets on separator tokens. That is only sound because every unquoted separator is now split upstream, and after this fix that includes those behind an unpaired quote. A separator surviving into a prefix is genuinely quoted text, which the QUOTED-SEP fixtures pin.
• Conservation: the resolved directly-invoked set is identical (38 lines) for the main, fcbf8de6 and this-head guards, over both the main tree and this branch's tree. A non-vacuity control shows that the comparison detects a difference.
• False-positive surface: glued |, ( and & in unquoted prose can now create a line-leading bare-path position that main did not. None exists in the corpus today, and this is the loud direction, which is what #3692 asks for.
• Portability: the suite passes under mawk and goawk, and shellcheck is clean. The guard runs unconditionally in the 🔍 Detect Changes job, which is green at this head along with every other check (28: 17 success, 11 skipped).
• Security: no workflow, trigger or permission change. The only new code is an awk function over repository text.
• Pre-existing, out of scope (not introduced here, still true on main): a direct invocation whose argument is another script path (./scripts/a.sh ./scripts/b.sh) is swallowed by the greedy match and never judged. Reported separately.

Verdict: no P0/P1 findings

---

Generated by Claude Code