whitelist command prefix integration in core and tui #7033

zhao-oai · 2025-11-20T21:47:32Z

this PR enables TUI to approve commands and add their prefixes to an allowlist:

note: we only show the option to whitelist the command when

command is not multi-part (e.g git add -A && git commit -m 'hello world')
command is not already matched by an existing rule

zhao-oai · 2025-11-21T18:58:13Z

@codex review

chatgpt-codex-connector · 2025-11-21T19:02:27Z

Codex Review: Didn't find any major issues. 👍

ℹ️ 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 · 2025-11-21T20:28:18Z

codex-rs/core/src/exec_policy.rs

+    commands: &[Vec<String>],
+    features: &Features,
+) -> Option<Vec<String>> {
+    if features.enabled(Feature::ExecPolicy) && commands.len() == 1 {


I don't love the solution I came up with here w.r.t only persisting allow prefix if ExecPolicy is enabled. I think a better solution would be to change the way we store Policy in session state. We should store either an enum indicating whether ExecPolicy is enabled or store Policy as an optional. However, I think this is fit for a followup PR

Yes, this relates to my long comment above on NeedsApproval.

zhao-oai · 2025-11-21T22:27:16Z

@codex review

chatgpt-codex-connector · 2025-11-21T22:30:54Z

Codex Review: Didn't find any major issues. Nice work!

ℹ️ 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".

codex-rs/core/tests/suite/approvals.rs

codex-rs/core/src/tools/orchestrator.rs

codex-rs/core/src/tools/sandboxing.rs

bolinfest · 2025-12-02T07:46:21Z

codex-rs/core/src/codex.rs

    features: Features,
    /// Execpolicy policy, applied only when enabled by feature flag.
-    exec_policy: Arc<ExecPolicy>,
+    exec_policy: Arc<RwLock<ExecPolicy>>,


Hmm, this feels slightly undesirable since SessionConfiguration is already generally wrapped in a mutex, but maybe this is OK, I'm not sure yet...

codex-rs/core/src/codex.rs

codex-rs/core/src/exec_policy.rs

bolinfest · 2025-12-02T07:58:37Z

codex-rs/protocol/src/approvals.rs

 }

 #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
 pub struct ExecApprovalRequestEvent {


We will need to update the v2 app server protocol as well, I expect. /cc @owenlin0

@zhao-oai yes that'd be great if we can keep app-server at feature-parity (but can be done as a followup PR). We'd probably want to add a field here: https://github.com/openai/codex/blob/main/codex-rs/app-server-protocol/src/protocol/v2.rs#L1428 and wire it up

gotcha. let's do this as a followup

bolinfest · 2025-12-02T22:17:05Z

codex-rs/core/src/unified_exec/session_manager.rs

+        let approval_requirement = {
+            let exec_policy = context.session.current_exec_policy().await;
+            create_approval_requirement_for_command(
+                &exec_policy,


What if instead of adding current_exec_policy(), we just pass &context.exec_policy to create_approval_requirement_for_command()?

Is the issue that context.session.state is held by a tokio::sync::mutex and so we need to clone the values while holding the lock so we can pass them into create_approval_requirement_for_command() because it is not async?

If so, should we make create_approval_requirement_for_command() async?

@zhao-oai How's this: #7501

OK, merged. Rebase and rejoice!

I think this might help with #7033 because `create_approval_requirement_for_command()` will soon need access to `Session.state`, which is a `tokio::sync::Mutex` that needs to be accessed via `async`.

codex-rs/core/src/codex.rs

codex-rs/core/src/tools/sandboxing.rs

bolinfest · 2025-12-03T19:25:27Z

codex-rs/core/src/codex.rs

    features: Features,
    /// Execpolicy policy, applied only when enabled by feature flag.
-    exec_policy: Arc<ExecPolicy>,
+    exec_policy: Arc<RwLock<ExecPolicy>>,


codex-rs/core/src/exec_policy.rs

bolinfest · 2025-12-03T20:23:12Z

codex-rs/core/src/exec_policy.rs

+    commands: &[Vec<String>],
+    features: &Features,
+) -> Option<Vec<String>> {
+    if features.enabled(Feature::ExecPolicy) && commands.len() == 1 {


Yes, this relates to my long comment above on NeedsApproval.

codex-rs/core/src/exec_policy.rs

codex-rs/core/tests/suite/approvals.rs

bolinfest

Make sure to rebase on #7540 when cleaning up merge conflicts!

Add explicit prefix-approval decision and wire it through execpolicy/UI snapshots update doc mutating in memory policy instead of reloading using RW locks clippy refactor: adding allow_prefix into ApprovedAllowPrefix fmt do not send allow_prefix if execpolicy is disabled moving args around cleanup exec_policy getters undo diff fixing rw lock bug causing tui to hang updating phrasing integration test . fix compile fix flaky test fix compile error running test with single thread fixup allow_prefix_if_applicable fix formatting fix approvals test only cloning when needed docs add docstring fix rebase bug fixing rebase issues Revert "fixing rebase issues" This reverts commit 79ce7e1. fix rebase errors

zhao-oai changed the base branch from main to pr7032 November 20, 2025 21:48

zhao-oai force-pushed the pr7032 branch 3 times, most recently from 683bc5f to 1273f94 Compare November 20, 2025 22:16

zhao-oai force-pushed the pr7033 branch 4 times, most recently from 9cb7ed0 to 80c63d3 Compare November 21, 2025 17:15

zhao-oai commented Nov 21, 2025

View reviewed changes

zhao-oai changed the title ~~Add approval allow-prefix flow in core and tui~~ allow-prefix integration in core and tui Nov 21, 2025

zhao-oai changed the title ~~allow-prefix integration in core and tui~~ whitelist command prefix integration in core and tui Nov 21, 2025

zhao-oai requested a review from bolinfest November 21, 2025 22:18

zhao-oai force-pushed the pr7032 branch from c5f0f62 to 71b53b2 Compare December 2, 2025 01:54

zhao-oai force-pushed the pr7033 branch from d48bd60 to 2310ebd Compare December 2, 2025 04:43

bolinfest reviewed Dec 2, 2025

View reviewed changes

codex-rs/core/tests/suite/approvals.rs Show resolved Hide resolved

bolinfest reviewed Dec 2, 2025

View reviewed changes

zhao-oai force-pushed the pr7033 branch from d651168 to 82778f6 Compare December 2, 2025 18:40

Base automatically changed from pr7032 to main December 2, 2025 20:05

zhao-oai force-pushed the pr7033 branch from 8168ded to dcf5e1e Compare December 2, 2025 20:06

bolinfest reviewed Dec 2, 2025

View reviewed changes

bolinfest mentioned this pull request Dec 2, 2025

chore: make create_approval_requirement_for_command an async fn #7501

Merged

zhao-oai force-pushed the pr7033 branch from 36e51e4 to 338ec43 Compare December 2, 2025 22:42

zhao-oai force-pushed the pr7033 branch 3 times, most recently from d23f506 to 312ceb4 Compare December 3, 2025 02:58

zhao-oai force-pushed the pr7033 branch from 312ceb4 to b6dc1be Compare December 3, 2025 03:01

zhao-oai requested a review from bolinfest December 3, 2025 17:42

bolinfest reviewed Dec 3, 2025

View reviewed changes

zhao-oai force-pushed the pr7033 branch 5 times, most recently from 86b1d98 to 31c5e11 Compare December 3, 2025 23:14

bolinfest approved these changes Dec 4, 2025

View reviewed changes

zhao-oai added 3 commits December 3, 2025 20:04

add back line

be1a90b

.

bf0ba87

zhao-oai force-pushed the pr7033 branch from 31c5e11 to bf0ba87 Compare December 4, 2025 04:50

zhao-oai merged commit e925a38 into main Dec 4, 2025
71 of 73 checks passed

zhao-oai deleted the pr7033 branch December 4, 2025 07:17

github-actions bot locked and limited conversation to collaborators Dec 4, 2025

whitelist command prefix integration in core and tui #7033

whitelist command prefix integration in core and tui #7033

Conversation

zhao-oai commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhao-oai commented Nov 21, 2025

Uh oh!

chatgpt-codex-connector bot commented Nov 21, 2025

Uh oh!

zhao-oai Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhao-oai commented Nov 21, 2025

Uh oh!

chatgpt-codex-connector bot commented Nov 21, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

owenlin0 Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bolinfest left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zhao-oai commented Nov 20, 2025 •

edited

Loading

zhao-oai Nov 21, 2025 •

edited

Loading

owenlin0 Dec 2, 2025 •

edited

Loading