Repository navigation
fix(bash-policy): screen shell nesting past the recursion limit instead of dropping it - #63
Merged
Merged
Conversation
…ad of dropping it Closes the gap recorded in #61 and tracked upstream as ApodexHarness#632. `_parse_commands` stopped recursing at `_MAX_NEST` (4) and stopped silently, so every level below was assessed by nobody. Measured in `off` mode — the default, and what most deployments run: $($($($($($(sudo id)))))) -> allow (6 levels) $($($($($($(ssh h id)))))) -> allow $($($($($($(pkill -f x)))))) -> allow against Layer 1.5 rules that are supposed to bind in every mode. `enforce` denied them, but only because the masked sentinel left in executable position is off the allowlist — incidental, not the rule that should have applied. At the limit the remaining text is now screened by its WORDS in one linear pass (`_expansion_words`): expansion punctuation is blanked and each word becomes a candidate executable, so `sudo` falls out of any depth. Two wrong turns on the way, both worth recording since each looked right: - Removing the cap and recursing on gave `RecursionError` at depth 2,000 — a crash instead of a verdict, which is worse than the bypass. - Peeling the chain one layer at a time cost a scan per layer: 143ms at depth 500 against main's 19ms, 11.6s at 5,000 — and at any step bound it lost the payload again, restoring the very bypass. The word screen is 19.7ms at 500 and 770ms at 20,000. The screen is deliberately conservative past the limit: it cannot tell a command name from a word that looks like one, so `$(...$(echo ssh)...)` nested 8 deep is refused on the strength of `ssh` appearing in it. That applies only past a depth no real command reaches, and it can only add refusals. Differential check over the nesting corpus: 9 verdicts change, all of them `off`-mode allow -> deny with the correct group; no benign form changes at any depth, and no `enforce` verdict moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes the deep-nesting gap recorded in #61 and tracked upstream as ApodexAI/ApodexHarness#632.
_parse_commandspreviously stopped inspecting code after four nesting levels. The fallback now flattens expansion punctuation, splits shell command separators, and checks decoded quoted payloads without recursive parsing. Commands such as deeply nesteddate;sudo id,date|ssh h id, andbash -c 'sudo id'are refused in every policy mode with the correct group. Interactive group hits still require human confirmation.Complete argument lists and execution order are retained for hard denials, including protected-path deletion,
cd /etc; rm -rf ., control-flow prefixes, andfind -exec. Both decoding passes and scanned input have fixed budgets. Malformed residual code or exhausted budgets produce a hard denial, including in interactive mode. Beyond the nesting limit this deliberately treats code-looking data conservatively; ordinary benign nesting remains allowed inoffmode.Verification:
dateremained allowed.main(a351f33), full pytest with-rs --basetemp=/tmp/fa63-merged-suite: 4,791 passed, 7 skipped, no failures or errors. Six skips are path checks for/root,/boot, and/etc/cron.d, which are absent on this macOS host; one is real bubblewrap isolation, which requires Linux and remains for CI to verify.git diff --check: passed.Adversarial commands are only handed to
assess_bash_command; none is executed.Merged latest
mainwithout rewriting history. The only merge conflict was inCHANGELOG.md; both the deep-shell policy entry and main's sandbox-verification entry were retained.