Sandbox::https://github.com/GSA-TTS/agentic-coding-quickstart/pull/653

mogul · 2026-10-29T00:59:00Z

🤖: AI agent (First Mate / OpenCode). AI-assisted; human review required.

Context

macOS ships Bash 3.2.57. The only 3.2 gate is CI (9 to 16 minutes per round trip), and it recently caught several 3.2-only failures that newer bash accepts (empty array under set -u; a heredoc in a process substitution with an apostrophe/backtick/paren in its body). This adds a repo-owned local command so developers, including agents in Linux sandboxes, get that feedback first. The CI workflow is untouched and stays authoritative.

Plan / what changed

scripts/test-acq-bash32 [bats args]: resolves a Bash 3.2.57 (ACQ_BASH32, cache install, macOS /bin/bash, /opt/bash32), fails closed unless BASH_VERSION is exactly 3.2.57, exposes it as bash on a private PATH dir (so bats and nested #!/usr/bin/env bash use it), then runs scripts/test-acq-bats with the arguments unchanged. No args = full suite.
--syntax [FILE...]: bash-3.2 -n over acq, acq.backends/*.sh, install.sh, bash/sh scripts in scripts/, test/bats/helper.bash, plus the heredoc scan. --lint, --which.
scripts/scan-heredoc-subst.py: best-effort heuristic for heredocs in $( )/<( ) whose body has odd quotes, unpaired backtick or unbalanced parens. bash-3.2 -n is the authority. Open PR for the Windows fixes proposes a similar scanner (test/bats/heredoc-scan.py) that had not merged; the hazard rules are the same and the ADR says to fold them into one file.
--install: source build with the same fingerprint-anchored OpenPGP verification as CI (a bats test asserts the fingerprint matches the workflow).
--install-oci: digest-pinned Docker Hub bash:3.2.57 fetch (no Docker, no remote script) for sandboxes with no compiler or GNU mirror.
docs/howto/bash32-testing.md, ADR-0038 (options comparison), CONTRIBUTING.md note, pre-commit file filter, test/bats/152-bash32-runner.bats (28 tests).

Provisioning comparison (tried in an offline-restricted aarch64 Linux sandbox: no compiler, no sudo, GNU mirrors and cache.nixos.org blocked; GitHub/PyPI/npm/Debian/Docker Hub reachable; Nix 2.24 present; mise absent): Nix/devenv could not fetch or build (no cache, no compiler); mise not installed and has no bash 3.2 backend; override works anywhere; source build is the strongest route where mirrors and a compiler exist; OCI fetch worked. Implemented: override + source build + OCI fallback. Nix/devenv/mise not implemented (the override accepts their output).

Verification

Real GNU Bash 3.2.57(1)-release (OCI-fetched, musl) used for: --syntax (32 files parse), the whole 152-bash32-runner.bats (28/28, no skips), the runner script itself parses/runs under 3.2, and a full-suite run under 3.2: 1078 ok, 2 failures, both environmental to this sandbox (117 msys needs a writable /tmp/opencode, fails under bash 5 too; 103 kit-fetch-lifecycle re-execs "$BASH", which is the raw binary under the OCI launcher; documented limitation of that route).
Under host bash 5.3: 152-bash32-runner.bats 28/28 (tests needing a real 3.2 skip with a reason).
shellcheck 0.10.0 via scripts/run-shellcheck: clean. markdownlint: clean. scripts/fix-exec-bits --check: OK. Scanner reports nothing on the repo; bash-3.2 -n agrees.
Timing (2-core sandbox, OCI interpreter): --syntax 0.8 s; one small bats file about 3 s; full suite about 15 min (883 s), so use focused files while iterating.
Adversarial review sub-agent, 2 rounds. Round 1 blocker (--install EXIT trap leak on success) and should-fixes (signal traps, relative ACQ_BASH32, scanner false negatives, docs) fixed. Round 2: no blockers; scanner quoting/false-positive/unterminated issues and a false-green cleanup test fixed.
Not verified: --install against real GNU mirrors with a compiler (mirrors blocked here; exercised only with stubbed curl/gpg/make, plus fail-closed on bad signature); anything on macOS; CI results (pending).

Limitations

Validates the bash interpreter only, not BSD/macOS utilities (sed, find, stat, xargs). The OCI interpreter is musl, content-pinned not signer-authenticated.

Rollback

Revert the commit; no runtime or workflow code is touched, and acq does not depend on any of this.

Security impact

Dev tooling only. New network use is opt-in (--install, --install-oci) and documented as needing user approval in agent sessions. OCI path verifies the manifest and every layer sha256 and extracts three fixed paths (no archive-controlled destinations). No secrets handled.

Add scripts/test-acq-bash32, which runs the vendored bats suite (or focused files) under a verified GNU Bash 3.2.57 and fails closed on any other version. It resolves the interpreter from ACQ_BASH32, a cache install, macOS /bin/bash or the CI path, and exposes it as `bash` on a private PATH so nested `#!/usr/bin/env bash` scripts also use it. Also add: - --syntax: `bash-3.2 -n` over the host-side shell files plus a heredoc scan - scripts/scan-heredoc-subst.py: heuristic for heredocs in $( ) / <( ) whose bodies bash 3.2 mis-parses - --install: source build with the same GNU OpenPGP signature check as CI - --install-oci: digest-pinned Docker Hub bash:3.2.57 fetch for sandboxes with no compiler or GNU mirror access - docs/howto/bash32-testing.md, ADR-0038 and a bats suite (152) The CI workflow is unchanged and remains authoritative. Refs: #632 Co-authored-by: OpenCode Agent <user@gsa.gov>

basiliskus

Verdict: request changes — 91046fb0308208fa397c2e9cc7f06a559aa76262

Major — scripts/fetch-bash32-oci.py:102-111: Please shell-quote or reject unsafe absolute prefixes before generating the OCI launcher. A prefix containing ' produces invalid shell, and crafted path text can become shell syntax when the launcher is immediately executed.
Major — .github/workflows/bash32-compat.yml:133-139: Please include the new runner and helpers in the Bash 3.2 relevance paths. Otherwise a future runner-only change skips the only real-3.2 CI lane while host tests may skip genuine-parser coverage.
Minor — scripts/scan-heredoc-subst.py:72-73: Please avoid indexing the comment-stripped string with offsets from the original line; a <<TAG example in a trailing comment currently raises IndexError and prints a traceback.

Assisted-by: OpenCode [gpt-5.6-sol-latest-guardrails-defaultv2] skill:change-review

…workflow paths - fetch-bash32-oci.py: generate the launcher with shlex.quote for every path, reject non-absolute, control-character, non-UTF-8 and root prefixes before any network or disk work, and unlink existing files instead of writing through them. - scan-heredoc-subst.py: blank trailing comments with length-preserving spaces so opener offsets match the masked line (fixes IndexError); report an unexpected exception as a single line with exit 2 and tolerate a closed stdout pipe. - tests: hostile-prefix launcher execution, fail-closed prefixes, comment/string heredoc cases, repo-wide scanner fuzz, exception-guard, and a workflow relevance path test that skips with a reason until bash32-compat.yml lists the new paths. Co-authored-by: OpenCode Agent <user@gsa.gov>

mogul · 2026-10-30T01:21:39Z

🤖: AI agent (First Mate) — addressed in ff4574d (all CI checks green: bash 3.2 build, pre-commit, markdown, link check, PR title).

Launcher injection (major) — fixed. scripts/fetch-bash32-oci.py now builds the launcher in launcher_text() with shlex.quote on the loader, library and binary paths. check_prefix() runs before any network or disk work and rejects relative prefixes, control characters (newline, tab, NUL, DEL), non-UTF-8 text and /. Existing files are unlinked before being rewritten, so a pre-planted symlink is not written through. Tests execute a real launcher for 14 hostile prefixes (quote, space, $(...), backtick, semicolon, newline, dash, &&, glob and others) against a fake loader that records its argv. They assert the argv is literal and no canary command ran. Control-character and root prefixes fail closed. Mutation checks: reverting to '%s' quoting fails the hostile-prefix test, and disabling the control-character check fails the fail-closed test.
Workflow relevance paths (major) — deferred to the workflow branch. The token used here cannot push .github/workflows/*, so I did not edit bash32-compat.yml. The human-pushed ci/bash32-matrix branch will carry it. The relevance list in the changes job needs these entries added: scripts/test-acq-bash32, scripts/fetch-bash32-oci.py and scripts/scan-heredoc-subst.py. The test now parses that list. It skips, with the missing paths named in the reason, so CI stays green. I confirmed locally that it passes once the three paths are added (workflow change reverted afterwards). I also suggest adding docs/howto/bash32-testing.md for any docs coverage you want, but the test doesn't require it.
Scanner IndexError (minor) — fixed. Trailing comments are now blanked with same-length spaces, so offsets match the original line. New tests cover <<EOF, $(, <( and >( in comments and << in strings, plus a case where a real finding is still reported. A seeded fuzz runs the scanner over acq, acq.backends/*.sh, install.sh, scripts/*.sh and random truncated lines. main() turns any unexpected exception into one line on stdout with exit 2, and a closed stdout pipe no longer produces a traceback. Reverting the comment-blanking fails the two comment tests, and removing the exception guard fails the exception test.

A pre-merge adversarial review (launcher injection, extraction, scanner) found no blockers. It did produce minor hardening, which is in this commit: the UTF-8 and root-prefix checks, the broken-pipe handling, and a stronger assertion in the fail-closed prefix test.

Not addressed: very long pathological lines (tens of thousands of repeated "a\ or $(() can take tens of seconds in the scanner's regexes. It only scans the repo's own files, so I left it.

basiliskus

Verdict: request changes — ff4574d4641baf9493bfbf71f5d738449c400d80

Prior launcher-quoting and scanner-crash findings are resolved. The CI relevance finding remains open:

Major — .github/workflows/bash32-compat.yml:133-139 and test/bats/152-bash32-runner.bats:482: Add scripts/test-acq-bash32, scripts/fetch-bash32-oci.py, and scripts/scan-heredoc-subst.py to the relevance paths, and make missing coverage fail rather than skip. Otherwise later tooling-only changes skip the real Bash 3.2 lane. Merge the companion correction first if necessary; deferral does not resolve this prerequisite.

Assisted-by: OpenCode [gpt-6.1-sol-latest-guardrails-defaultv2] skill:change-review

mogul requested a review from a team as a code owner October 29, 2026 00:59

This was referenced Oct 29, 2026

test(bash32): unify the two heredoc-in-substitution scanners after #644 and #653 merge #665

Open

docs: describe the sharded bash 3.2 job and timing summary in CONTRIBUTING and the bash 3.2 how-to #666

Open

basiliskus requested changes Oct 29, 2026

View reviewed changes

basiliskus requested changes Oct 30, 2026

View reviewed changes

Navigation Menu

Repository navigation

feat(test): add local bash 3.2 test runner - #653

feat(test): add local bash 3.2 test runner#653
mogul wants to merge 2 commits into
mainfrom
feat/632-local-bash32-runner

mogul commented Oct 29, 2026

Uh oh!

basiliskus left a comment

Uh oh!

mogul commented Oct 30, 2026

Uh oh!

basiliskus left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mogul commented Oct 29, 2026

Context

Plan / what changed

Verification

Limitations

Rollback

Security impact

Uh oh!

basiliskus left a comment

Choose a reason for hiding this comment

Uh oh!

mogul commented Oct 30, 2026

Uh oh!

basiliskus left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants