Conversation
…-brief.sh Bash 3.2 (macOS system bash) breaks parsing of a heredoc nested inside $(...) the moment its body contains an unescaped apostrophe, and the failure surfaces as a syntax error for the rest of the script - this is what #945 reintroduced with new wording, and how it passed CI on a newer bash while breaking every macOS crewmate. Replace every VAR=$(cat <<EOF ... EOF) in fm-brief.sh with read -r -d '' VAR <<EOF ... EOF || true, which builds the same multi-line variable without a command substitution to break quote tracking. Add a structural test that greps for the vulnerable idiom directly, since a prior wording- only regression test failed to catch this exact recurrence.
Author
|
Closed at the repository owner’s request. |
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.
Intent
Repair the tracked bin/fm-brief.sh syntax regression on firstmate main: bash -n bin/fm-brief.sh failed with 'unexpected EOF while looking for matching )' at line 314, and bin/fm-brief.sh crashed identically for any caller. Root cause: bash 3.2 (macOS system bash) has a parser bug where a heredoc nested inside a$(...) command substitution breaks quote tracking for the rest of the script the moment the heredoc body contains a single unescaped apostrophe. Commit ec09871 (#945) reintroduced this bug class by adding new wording ('firstmate's authority check') to one of three DOD=$ (cat <<EOF ... EOF) heredocs inside the ship-mode case statement; it passed CI because CI's bash is newer and does not have this parser flaw, so only macOS crewmates hit it (tracked as GitHub issue #1000). I bisected the exact regressing line via binary search on the commit's diff hunks, confirmed the mechanism with isolated bash 3.2 reproductions (both case and if/else nesting, both quoted and unquoted heredoc delimiters), then fixed it structurally rather than just rewording: converted all four VAR=$(cat <<EOF ... EOF) assignments in fm-brief.sh (the three ship-mode DOD heredocs plus one HERDR_SECTION heredoc that used the same fragile idiom but had not yet tripped it) to read -r -d '' VAR <<EOF ... EOF || true, which builds the same multi-line variable content without nesting a heredoc inside a command substitution, eliminating the bug class rather than dodging one instance of it. Verified byte-for-byte content fidelity (multi-line text, blank lines, apostrophes) between the old and new construction pattern before applying it. Added a structural regression test (test_no_command_substitution_heredocs) that greps the script for the vulnerable =$(cat << idiom directly, since the existing regression test only pinned one exact previously-bad wording string and demonstrably failed to catch this new recurrence with different wording. Verified the new test actually fails against the original broken commit and passes against the fix. All work stayed inside bin/fm-brief.sh and its colocated test file; no generated files, changelogs, or unrelated files were touched.
What Changed
read -r -d ''assignments while preserving generated brief content.=$(cat <<...)pattern on all Bash versions and consolidate the related test rationale.Risk Assessment
✅ Low: Captain, the change is narrowly scoped, preserves generated content, removes all four vulnerable constructions, and adds a structural regression guard.
Testing
Reproduced the original line-314 failure on the base script with macOS Bash 3.2, confirmed the target parses, passed the focused behavior suite, and manually generated and inspected all three ship-mode briefs; an initial evidence-wrapper argument-splitting mistake was corrected and rerun successfully.
Evidence: Bash 3.2 parser counterfactual
Base script fails on Apple Bash 3.2 at line 314 with unmatched). Target script exits 0.Evidence: End-user brief generation transcript
Shows successful CLI scaffolding and rendered content for no-mistakes, direct-PR, and local-only modes under Apple Bash 3.2.Evidence: Structural regression proof
Base contains four vulnerable assignments; target contains none and shows four structural replacements.Evidence: Generated no-mistakes brief
Evidence: Generated direct-PR brief
Evidence: Generated local-only brief
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
Inspectedgit diff b05eb244ce10f83192a95457b5e2b15c49b4e763 d7d21724075c3c79b6790272c69c754edb651573 -- bin/fm-brief.sh tests/fm-brief.test.shRan/bin/bash tests/fm-brief.test.shunder Apple Bash 3.2.57Ran/bin/bash -n <base-commit fm-brief.sh>and/bin/bash -n bin/fm-brief.shas a before-and-after counterfactualRanFM_HOME=<evidence-home> /bin/bash bin/fm-brief.sh brief-default no-registry-projRanFM_HOME=<evidence-home> /bin/bash bin/fm-brief.sh brief-direct direct-projRanFM_HOME=<evidence-home> /bin/bash bin/fm-brief.sh brief-local local-projChecked generated briefs for one Definition of done section, zero leakedEOFmarkers, expected mode-specific text, blank lines, andfirstmate's authority checkComparedgrep -n '=\$(cat <<'results between the base and target scriptsVerifiedgit status --shortremained clean✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.