test(sandbox): pin the two documented boundaries - #1261
Merged
Conversation
--sandbox had 9 tests and none for either boundary the docs draw. A child process cannot resolve an unmocked command: `sh -c 'command -v probe'` returns empty, so the command is unreachable rather than merely unreported. That is what makes `sh -c 'curl …'` useless as an escape off Windows. An absolute path does bypass the sandbox, which is documented, and nothing kept it a deliberate boundary. The mechanism is not what it looks like. Removing `export PATH` from sandbox.sh changes nothing -- PATH is exported already, so that mutation is a no-op. The property holds because the sandbox *replaces* PATH rather than prepending to it; mutating it to "$_BASHUNIT_SANDBOX_DIR:$PATH" fails the child-process test and nothing else. Both facts are in the comment, because a comment naming the wrong dependency is worse than none. Closes #1260
Chemaclass
added a commit
that referenced
this pull request
Aug 14, 2026
Narrowing PATH is not workable under Git Bash, so a command reached from a child process is not blocked there -- docs/test-doubles.md documents that exception and ADR-012 records the mechanism. The test added in #1261 asserted the non-Windows behaviour unconditionally and failed windows-acceptance.
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
1 participant
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.
🤔 Background
Related #1260
--sandboxhad 9 acceptance tests and none for either boundary the docs draw — both security-adjacent, both able to regress silently:sh -c 'command -v probe'returns empty under--sandbox: the narrowed PATH is inherited, so the command is unreachable rather than merely unreported. That is what makessh -c 'curl …'useless as an escape off Windows.💡 Changes
The mechanism is not what it looks like
Removing
export PATHfromsrc/runner/sandbox.shchanges nothing —PATHis exported already, so that mutation is a no-op and every test still passes. My first version of the comment claimed the property depended on it.It actually holds because the sandbox replaces PATH rather than prepending:
fails the child-process test and nothing else. Both facts are recorded in the comment — a comment naming the wrong dependency is worse than none, since the next person mutates what it names, sees green, and concludes the test is worthless.
Fixture safety checked: every test in the file filters, so the two added fixture entries are only reached via their own filters, and the fixture is not collected by the suite (does not match
*[tT]est.sh, andfixtures/is excluded by path).