Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2d559b84c
ℹ️ 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".
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn exec_approval_requirement_honors_permissions_scoped_rule() { |
There was a problem hiding this comment.
Add integration coverage for scoped exec rules
This change threads active_permission_profile into the agent's exec-approval decision path, but the added coverage is unit-level only; agent-logic changes in this repo require an integration test under core/suite, which would exercise the actual shell/unified tool flow with a named permission profile and catch propagation regressions that these direct ExecPolicyManager calls can miss. Please add an end-to-end test covering matching and non-matching permissions-scoped rules.
AGENTS.md reference: AGENTS.md:L106-L110
Useful? React with 👍 / 👎.
63bfa4a to
aabfed7
Compare
|
Could approval-generated amendments retain the active permissions profile from server-owned pending approval state? The initial shell evaluation has that profile, but That means approving a prefix under profile A can make it bypass approval under profile B, which appears to leave the PR's original persisted-approval issue open for this path. I reproduced the durable part on this head: after the approval writer appends and reloads the rule, matching returns allow under both profile A and profile B. The existing manually authored scoped-rule test still passes. Could the pending approval capture the original command and profile at proposal time, consume that state on the matching approval id, and use the same scoped rule for deduplication, disk, and live policy? An end-to-end test that approves under A, restarts with the same home under B, and still observes a prompt would cover the missing seam. |
|
Closing this pull request because it has had no updates for more than 14 days. If you plan to continue working on it, feel free to reopen or open a new PR. |
Why
Exec policy prefix rules were not aware of the active permissions profile. That made persisted command approval rules global even though the same command can have different risk depending on whether Codex is running under a managed profile, a sandbox profile, or another named profile.
This PR adds a concise
permissions = "..."rule option so a prefix rule only applies when the matching permissions profile is active. It also lets inline[rules]inconfig.tomlparticipate in the same policy flow asrules/*.rulesandrequirements.toml.What Changed
permissionsto Starlarkprefix_rule(...)and TOMLprefix_rules, with matching gated on the active permissions profile.[rules]fromconfig.tomllayers and overlay them with existingrules/*.rulesfiles in layer precedence order.requirements.tomlrules withpermissionswhile preserving the existing restriction that requirements cannot usedecision = "allow".config.schema.jsonand add coverage for scoped matching, config TOML rules, child-session policy reuse, and the end-to-end shell approval flow for matching and non-matching scoped rules.Starlark Examples
An existing unscoped rule continues to apply regardless of the active permissions profile:
A permissions-scoped rule only applies when the active permissions profile id matches the
permissionsvalue. The value is a profile id such as one defined by[permissions.ci_tools]; built-in ids keep their leading colon, for example:workspace.Scoped prompt or forbidden rules use the same field:
Verification
just test -p codex-execpolicyjust test -p codex-configjust test -p codex-core child_does_not_use_parent_exec_policy_when_config_toml_rules_differjust test -p codex-core loads_config_toml_rulesjust test -p codex-core ignores_config_toml_rules_when_config_stack_disables_exec_policy_rulesjust test -p codex-core exec_approval_requirement_honors_permissions_scoped_rulejust test -p codex-core permissions_scoped_prefix_rule_uses_active_profile_in_shell_flowDocumentation
The Codex config docs on developers.openai.com/codex should document the new
permissionsfield for Starlark/TOML prefix rules and the new[rules]surface inconfig.toml.