feat: exec policy integration in shell mcp #7609
Merged
Conversation
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
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
4 times, most recently
from
fb0cb00 to
e9a6dc4
Compare
bolinfest
reviewed
Dec 4, 2025
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
2 times, most recently
from
e6b51aa to
a6eddba
Compare
bolinfest
reviewed
Dec 4, 2025
bolinfest
added a commit
that referenced
this pull request
Dec 4, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`.
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
a6eddba to
185e685
Compare
zhao-oai
pushed a commit
that referenced
this pull request
Dec 4, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`.
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
185e685 to
2d1a155
Compare
zhao-oai
pushed a commit
that referenced
this pull request
Dec 4, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`.
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
2d1a155 to
d0519b6
Compare
zhao-oai
pushed a commit
that referenced
this pull request
Dec 4, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`. .
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
d0519b6 to
1b2b3ca
Compare
zhao-oai
pushed a commit
that referenced
this pull request
Dec 4, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`. .
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
1b2b3ca to
212857d
Compare
zhao-oai
pushed a commit
that referenced
this pull request
Dec 4, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`. .
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
212857d to
627e70d
Compare
Contributor
Author
|
@codex review |
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
Comment on lines
+199
to
+207
| Ok(match evaluation.decision { | ||
| Decision::Forbidden => ExecPolicyOutcome::Forbidden, | ||
| Decision::Prompt => ExecPolicyOutcome::Prompt { | ||
| run_with_escalated_permissions: decision_driven_by_policy, | ||
| }, |
Contributor
There was a problem hiding this comment.
When execpolicy yields Decision::Prompt, run_with_escalated_permissions is set whenever any non-heuristic rule matched, so determine_action will run the command via EscalateAction::Escalate after the user approves. That means an execpolicy rule that merely requests approval will cause the command to bypass the sandbox entirely once accepted, whereas the core execpolicy flow only bypasses the sandbox for explicit Allow rules (see core/src/exec_policy.rs where bypass_sandbox is tied to Decision::Allow). This over-escalates any prompted command defined in policy, weakening sandbox protections without the policy opting into it.
Useful? React with 👍 / 👎.
bolinfest
reviewed
Dec 5, 2025
bolinfest
reviewed
Dec 5, 2025
bolinfest
reviewed
Dec 5, 2025
bolinfest
reviewed
Dec 5, 2025
zhao-oai
pushed a commit
that referenced
this pull request
Dec 5, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`. .
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
627e70d to
fd8d68a
Compare
zhao-oai
pushed a commit
that referenced
this pull request
Dec 5, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`. .
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
fd8d68a to
aa58369
Compare
zhao-oai
pushed a commit
that referenced
this pull request
Dec 5, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`. .
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
aa58369 to
d65dd48
Compare
zhao-oai
pushed a commit
that referenced
this pull request
Dec 5, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`. .
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
d65dd48 to
be05876
Compare
zhao-oai
pushed a commit
that referenced
this pull request
Dec 5, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`. .
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
be05876 to
1ef31fc
Compare
zhao-oai
pushed a commit
that referenced
this pull request
Dec 5, 2025
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`. .
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
1ef31fc to
4c23d0b
Compare
…7615) The caller should decide whether wrapping the policy in `Arc<RwLock>` is necessary. This should make #7609 a bit smoother. - `exec_policy_for()` -> `load_exec_policy_for_features()` - introduce `load_exec_policy()` that does not take `Features` as an arg - both return `Result<Policy, ExecPolicyError>` instead of Result<Arc<RwLock<Policy>>, ExecPolicyError>` This simplifies the tests as they have no need for `Arc<RwLock>`. .
zhao-oai
force-pushed
the
dev/zhao/execpolicy-mcp
branch
from
4c23d0b to
f1ddea3
Compare
bolinfest
reviewed
Dec 5, 2025
Comment on lines
+189
to
+195
| let evaluation = policy.check(&command, &|cmd| { | ||
| if command_might_be_dangerous(cmd) { | ||
| Decision::Prompt | ||
| } else { | ||
| Decision::Allow | ||
| } | ||
| }); |
Collaborator
There was a problem hiding this comment.
This HeuristicsFallback abstraction feels weird to me. I'm going to revisit in a follow-up.
bolinfest
approved these changes
Dec 5, 2025
Collaborator
bolinfest
left a comment
bolinfest
left a comment
There was a problem hiding this comment.
Let's get this in and I can change, as necessary.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
adding execpolicy support into the
posixmcp