Conversation
![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.
ℹ️ 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".
| pub fn run(&self) -> Result<String> { | ||
| let policy = load_policies(&self.policies)?; | ||
| let evaluation = policy.check(&self.command); |
There was a problem hiding this comment.
The new execpolicycheck subcommand evaluates policies by calling policy.check(&self.command) on the raw argv, so it only considers the literal leading tokens. Runtime enforcement (core/src/exec_policy.rs lines 87‑94) first expands bash -lc/sh -lc and other shell commands via parse_shell_lc_plain_commands and then checks every resulting command, which is how approval decisions are actually made. With the current CLI implementation, commands wrapped in bash -lc "..." (or multiple commands separated by ;) will report noMatch in the checker even though the real runtime would inspect and potentially forbid or prompt for the inner command, making the validation tool misleading for the common shell-wrapped cases it’s meant to preview.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is for a cli tool -- we are assuming the user passes in unwrapped commands
bolinfest
left a comment
bolinfest
left a comment
There was a problem hiding this comment.
It would be good to have an integration test for the CLI if you don't already. Take a look at assert_cmd::Command::cargo_bin (or alternatively, escargot::CargoBuild) for how to run a bin target from an integration test.
Co-authored-by: Michael Bolin <mbolin@openai.com>
| use tempfile::TempDir; | ||
|
|
||
| #[test] | ||
| fn execpolicy_check_matches_expected_json() -> Result<(), Box<dyn std::error::Error>> { |
2 participants
adding execpolicycheck tool onto codex cli
this is useful for validating policies (can be multiple) against commands.
it will also surface errors in policy syntax:
this PR also changes output format when there's no match in the CLI. instead of returning the raw string
noMatch, we return{"noMatch":{}}this PR is a rewrite of: #6932 (due to the numerous merge conflicts present in the original PR)