Add reusable interception guards - #2167
Conversation
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36407cf3a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review This PR introduces new interception guards with complex shell parsing logic. Multiple unresolved P1/P2 review comments identify security bypass vulnerabilities in the command parsing (process substitution, bash -c --, code_interpreter payloads). The security-relevant nature of these guards combined with identified bypass issues warrants human review. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74f1e493a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 093b0cb96a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7aeccfdfd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
xeophon
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd22cb5334
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aab863436d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7576740771
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c968feef7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ec20775 to
04864d3
Compare
658f576 to
cec154b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cec154b784
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| continue | ||
| segments = [[]] | ||
| for token in tokens: | ||
| if token and set(token) <= set("{};&|()"): |
There was a problem hiding this comment.
When a restricted executable is launched through Bash process substitution, such as cat <(rm -rf /tmp/x), the punctuation splitter retains <( in the outer segment and never emits rm as a separate invocation. As a result, intercept_shell_commands("rm") allows the deletion to execute; the same bypass affects intercept_code_search() for inputs such as cat <(rg secret .). Parse the command body of <(...) and >(...) as an independent pending invocation.
Useful? React with 👍 / 👎.
| has_command = any( | ||
| key in source | ||
| for source in sources | ||
| for key in ("command", "commands", "cmd") |
There was a problem hiding this comment.
When a code_interpreter_call contains shell syntax in its code field, such as {"code":"!rm -rf /tmp/x"}, alias normalization sends it through the Bash guard, but command extraction only recognizes command, commands, and cmd, so intercept_shell_commands("rm") receives an empty string; intercept_code_search() similarly misses !rg secret .. Fresh evidence beyond the earlier intercept_tool_calls case is that its new raw_fallback=True does not apply to these two commands_only=True paths, so include the code payload or stop classifying code-interpreter calls as shell-shaped.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54a609e04c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif index + 1 < len(segment): | ||
| pending.append(segment[index + 1]) |
There was a problem hiding this comment.
-- before the nested shell command
When a guarded command is launched as bash -c -- 'rm -rf /tmp/x' (or bash -lc -- 'rg secret .'), Bash treats the argument after -- as the -c command, but this branch queues -- itself and never inspects the actual payload. bash --help documents the invocation form as -c command, and Bash 5.2 confirms that the examples execute, so both shell-command and code-search guards are bypassed. Fresh evidence beyond the earlier nested-shell finding is this still-unhandled end-of-options form; skip the optional -- before queuing the command string.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 54a609e. Configure here.
| elif kind != "function_call" and kind.endswith(("_call", "_tool_result")): | ||
| name = item.get("name") or kind.removesuffix("_tool_result").removesuffix( | ||
| "_call" | ||
| ) |
There was a problem hiding this comment.
Custom tool calls counted twice
Low Severity
_tool_calls skips function_call items in provider_state because they already live in tool_calls, but still ingests custom_tool_call. Dialects put both kinds into tool_calls and keep the raw items in provider_state, so custom calls are duplicated. That can inflate call lists and make containing checks match on the extra copy.
Reviewed by Cursor Bugbot for commit 54a609e. Configure here.
| "stderr": "", | ||
| "outcome": {"type": "exit", "exit_code": 0}, | ||
| } | ||
| ] |
There was a problem hiding this comment.
Blocked shell reports success
Medium Severity
Rewriting a shell_call_output always sets exit_code to 0, so a blocked command looks successful. Guards that rewrite bash tool results (rather than terminate) can leave the model believing the command worked despite the block text in stdout.
Reviewed by Cursor Bugbot for commit 54a609e. Configure here.
|
Superseded by #2178, which replaces this guard layer with the reworked interception surface (typed rules, prebuilt guards, backend simplifications). |
1 participant
Overview
This adds reusable guards for common tool checks and a minimal trace-based interface for custom guards.
A custom
@vf.intercepthandler receives onevf.Trace. During interception,trace.messagescontains the current exchange andtrace.last_messageis the candidate crossing the model boundary. Returningtrace.replace(content)creates an inert replacement of the same message kind.What this PR adds
trace.messages,trace.last_message, andtrace.replace(content)for custom handlers.Stack
This is PR 4 of 5. Each PR is based on the one before it and is intended to merge in order.
@vf.interceptbefore model requests and before responses reach the harness, and make rewrites canonical.vf.Trace.@vf.interceptin a runnable taskset.