fix(approval): recognize wrapper-prefixed kill spellings in self-host guard - #165
Merged
Merged
Conversation
… guard Review follow-up on upstream PR NousResearch#43157: `command kill <pid>` and `builtin kill <pid>` execute kill but bypassed _KILL_CMD_RE, which only matched kill as the first word of a command segment. Anchor the kill guard at command position via _CMDPOS and add `command [-p]` / builtin to the shared wrapper inventory, so wrapper chains (sudo, env, exec, nohup, setsid, time, command, builtin) are consumed before the anchor. `command -v/-V` stays unmatched — it resolves a name without executing. tests/tools/test_hardline_blocklist.py: 5 new cases covering the reported bypass probes end-to-end, wrapper chains, foreign-PID allow, and command -v precision. 115 passed; approval cluster 370 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🔎 Lint report:
|
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.
Why
Upstream review on NousResearch#43157 (the mirror of #128) found a bypass:
command kill <pid>andbuiltin kill <pid>are plain shell spellings that still execute kill, but_KILL_CMD_REonly matchedkillas the first word of a command segment, so both returned approved.What changed
tools/approval.py:_KILL_CMD_REnow anchorskillat command position via the shared_CMDPOSfragment, and_CMDPOS's wrapper inventory gainscommand [-p]andbuiltin. Wrapper chains (sudo,env,exec,nohup,setsid,time,command,builtin) are consumed before the anchor, so all those spellings hit the guard.command -v/-Vstays unmatched — it resolves a name without executing it. The_CMDPOSextension also hardens the shutdown/reboot hardline patterns for free (command shutdownwas the same class of bypass).tests/tools/test_hardline_blocklist.py: 5 new cases — the reported probes end-to-end throughcheck_all_command_guards, wrapper chains, foreign-PID allow via wrappers, andcommand -vprecision.Verification
tests/tools/test_hardline_blocklist.py— 115 passed (110 pre-existing + 5 new) on this branch and on the upstream mirror branch. Full approval cluster (6 files) — 370 passed. The reviewer's exact probe (command kill <own-pid>/builtin kill <own-pid>via_check_self_host_kill+check_all_command_guards) now blocks both forms.Notes
Fork-side mirror of the review fix pushed to upstream PR NousResearch#43157 (branch
mirror/self-host-kill-guard, commit 2f05e2b9c cherry-picked here). Follow-up to #128.