execpolicy2 extension #6627
execpolicy2 extension #6627
Conversation
|
@codex review |
![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".
| /// Evaluate a command against a policy. | ||
| Check { | ||
| #[arg(short, long, value_name = "PATH")] | ||
| policy: PathBuf, | ||
| #[arg(short, long, value_name = "PATH", required = true)] | ||
| policies: Vec<PathBuf>, | ||
|
|
There was a problem hiding this comment.
Renaming the field from policy to policies in the Clap definition changed the generated long option to --policies, because #[arg(long)] derives the flag name from the field identifier. The README and existing CLI usage still rely on --policy, so the sample command (cargo run … -- check --policy …) now fails with “found argument '--policy' which wasn't expected”, and any automation using the old flag breaks. Please keep the long name stable by explicitly setting long = "policy" (or by keeping the field name singular) so the CLI remains backward compatible while still accepting multiple paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this crate hasn't been merged yet. we are free to make breaking changes
zhao-oai
force-pushed
the
dev/zhao/execpolicy2
branch
from
35f3561 to
ce220f0
Compare
zhao-oai
force-pushed
the
dev/zhao/execpolicy2-extension
branch
from
784ce0d to
bdb5210
Compare
codex-rs/execpolicy2/tests/basic.rs
Outdated
| prefix_rule( | ||
| pattern = ["git", "status"], | ||
| decision = "allow", | ||
| ) | ||
| prefix_rule( | ||
| pattern = ["git"], | ||
| decision = "prompt", | ||
| ) |
There was a problem hiding this comment.
What is the decision for git status: is it prompt because that's stricter than allow?
There was a problem hiding this comment.
yes it should be prompt!
There was a problem hiding this comment.
Does that make the git status rule useless? How would it ever be allowed?
zhao-oai
force-pushed
the
dev/zhao/execpolicy2
branch
from
ce220f0 to
ce50c94
Compare
zhao-oai
force-pushed
the
dev/zhao/execpolicy2-extension
branch
from
636cb31 to
340895f
Compare
zhao-oai
force-pushed
the
dev/zhao/execpolicy2
branch
from
ce50c94 to
94d2943
Compare
zhao-oai
force-pushed
the
dev/zhao/execpolicy2-extension
branch
from
340895f to
51249c8
Compare
zhao-oai
force-pushed
the
dev/zhao/execpolicy2-extension
branch
from
51249c8 to
c14008b
Compare
zhao-oai
force-pushed
the
dev/zhao/execpolicy2-extension
branch
from
c14008b to
43ca23f
Compare
codex-rs/execpolicy2/tests/basic.rs
Outdated
| prefix_rule( | ||
| pattern = ["git", "status"], | ||
| decision = "allow", | ||
| ) | ||
| prefix_rule( | ||
| pattern = ["git"], | ||
| decision = "prompt", | ||
| ) |
There was a problem hiding this comment.
Does that make the git status rule useless? How would it ever be allowed?
|
@bolinfest (not sure why it's not letting me reply) -- fixed the git status test case to be more realistic as well |
codex-rs/execpolicy2/tests/basic.rs
Outdated
| #[test] | ||
| fn strictest_decision_across_multiple_commands() { | ||
| let policy_src = r#" | ||
| prefix_rule( |
There was a problem hiding this comment.
As discussed, remove this and change ["git"] to "prompt".
Policy(useful if codex detects many.codexpolicyfiles)Policyto allow evaluation of multiple cmds at once (useful when we have chained commands)