github/codex
4
0 Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-03 16:23:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Enforce automatic review for managed models (#37511)

Browse Source 
## What changed

- Add the managed `auto_review.required_on_models` requirement, unioning model slugs across requirement layers and exposing the result through `configRequirements/read`.
- Force listed models to use `on-request` approvals with the `auto_review` reviewer. Downgrade Full Access to workspace-write when starting a protected session, and reject incompatible runtime overrides or use when automatic review is disabled.
- Preserve these protections across thread starts, resumes, forks, model changes, turn overrides, MCP approvals, and TUI session attachment.

## Testing

- Add coverage for layered model requirements and exact provider-alias matching.
- Add app-server tests for protected thread lifecycle operations, unsafe setting overrides, legacy thread upgrades, disabled automatic review, and requirements API output.

GitOrigin-RevId: c4b1d52c3b5b75e86880725412b8f0812e0dfcd9
This commit is contained in:
viyatb-oai authored and copyberry committed 2026-08-07 23:43:14 +00:00
1 parent 8073dbb20b
commit 208f05b233
37 files changed
+944 -36
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json
Generated
+24
View File
@@ -7385,6 +7385,20 @@
],
"type": "string"
},
"AutoReviewRequirements": {
"properties": {
"requiredOnModels": {
"items": {
"type": "string"
},
"type": [
"array",
"null"
]
}
},
"type": "object"
},
"BrowserUseRequirements": {
"properties": {
"disableAutoReview": {
@@ -8791,6 +8805,16 @@
"null"
]
},
"autoReview": {
"anyOf": [
{
"$ref": "#/definitions/v2/AutoReviewRequirements"
},
{
"type": "null"
}
]
},
"browserUse": {
"anyOf": [
{
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json
Generated
+24
View File
@@ -1244,6 +1244,20 @@
],
"type": "string"
},
"AutoReviewRequirements": {
"properties": {
"requiredOnModels": {
"items": {
"type": "string"
},
"type": [
"array",
"null"
]
}
},
"type": "object"
},
"BrowserUseRequirements": {
"properties": {
"disableAutoReview": {
@@ -4956,6 +4970,16 @@
"null"
]
},
"autoReview": {
"anyOf": [
{
"$ref": "#/definitions/AutoReviewRequirements"
},
{
"type": "null"
}
]
},
"browserUse": {
"anyOf": [
{
codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json
Generated
+24
View File
@@ -59,6 +59,20 @@
}
]
},
"AutoReviewRequirements": {
"properties": {
"requiredOnModels": {
"items": {
"type": "string"
},
"type": [
"array",
"null"
]
}
},
"type": "object"
},
"BrowserUseRequirements": {
"properties": {
"disableAutoReview": {
@@ -152,6 +166,16 @@
"null"
]
},
"autoReview": {
"anyOf": [
{
"$ref": "#/definitions/AutoReviewRequirements"
},
{
"type": "null"
}
]
},
"browserUse": {
"anyOf": [
{
codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst
Generated
BIN
View File
Binary file not shown.
codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst
Generated
BIN
View File
Binary file not shown.
codex-rs/app-server-protocol/schema/typescript/v2/AutoReviewRequirements.ts
Generated
+5
View File
@@ -0,0 +1,5 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
export type AutoReviewRequirements = { requiredOnModels: Array<string> | null, };
codex-rs/app-server-protocol/schema/typescript/v2/ConfigRequirements.ts
Generated
+2 -1
View File
@@ -4,6 +4,7 @@
import type { PathUri } from "../PathUri";
import type { WebSearchMode } from "../WebSearchMode";
import type { AskForApproval } from "./AskForApproval";
import type { AutoReviewRequirements } from "./AutoReviewRequirements";
import type { BrowserUseRequirements } from "./BrowserUseRequirements";
import type { ComputerUseRequirements } from "./ComputerUseRequirements";
import type { FeedbackRequirements } from "./FeedbackRequirements";
@@ -12,4 +13,4 @@ import type { ResidencyRequirement } from "./ResidencyRequirement";
import type { SandboxMode } from "./SandboxMode";
import type { WindowsSandboxSetupMode } from "./WindowsSandboxSetupMode";
export type ConfigRequirements = {allowedApprovalPolicies: Array<AskForApproval> | null, allowedSandboxModes: Array<SandboxMode> | null, allowedWindowsSandboxImplementations: Array<WindowsSandboxSetupMode> | null, allowedPermissionProfiles: { [key in string]?: boolean } | null, defaultPermissions: string | null, allowedWebSearchModes: Array<WebSearchMode> | null, allowManagedHooksOnly: boolean | null, allowAppshots: boolean | null, allowRemoteControl: boolean | null, computerUse: ComputerUseRequirements | null, browserUse: BrowserUseRequirements | null, featureRequirements: { [key in string]?: boolean } | null, enforceResidency: ResidencyRequirement | null, models: ModelsRequirements | null, sqliteHome: PathUri | null, logDir: PathUri | null, modelCatalogJson: PathUri | null, checkForUpdateOnStartup: boolean | null, allowLoginShell: boolean | null, feedback: FeedbackRequirements | null, windowsSandboxPrivateDesktop: boolean | null};
export type ConfigRequirements = {allowedApprovalPolicies: Array<AskForApproval> | null, allowedSandboxModes: Array<SandboxMode> | null, allowedWindowsSandboxImplementations: Array<WindowsSandboxSetupMode> | null, allowedPermissionProfiles: { [key in string]?: boolean } | null, defaultPermissions: string | null, allowedWebSearchModes: Array<WebSearchMode> | null, allowManagedHooksOnly: boolean | null, allowAppshots: boolean | null, allowRemoteControl: boolean | null, computerUse: ComputerUseRequirements | null, browserUse: BrowserUseRequirements | null, featureRequirements: { [key in string]?: boolean } | null, enforceResidency: ResidencyRequirement | null, autoReview: AutoReviewRequirements | null, models: ModelsRequirements | null, sqliteHome: PathUri | null, logDir: PathUri | null, modelCatalogJson: PathUri | null, checkForUpdateOnStartup: boolean | null, allowLoginShell: boolean | null, feedback: FeedbackRequirements | null, windowsSandboxPrivateDesktop: boolean | null};
codex-rs/app-server-protocol/schema/typescript/v2/index.ts
Generated
+1
View File
@@ -41,6 +41,7 @@ export type { AskForApproval } from "./AskForApproval";
export type { AttestationGenerateParams } from "./AttestationGenerateParams";
export type { AttestationGenerateResponse } from "./AttestationGenerateResponse";
export type { AutoReviewDecisionSource } from "./AutoReviewDecisionSource";
export type { AutoReviewRequirements } from "./AutoReviewRequirements";
export type { BrowserUseRequirements } from "./BrowserUseRequirements";
export type { ByteRange } from "./ByteRange";
export type { CancelLoginAccountParams } from "./CancelLoginAccountParams";
codex-rs/app-server-protocol/src/protocol/v2/config.rs
+8
View File
@@ -394,6 +394,7 @@ pub struct ConfigRequirements {
pub enforce_residency: Option<ResidencyRequirement>,
#[experimental("configRequirements/read.network")]
pub network: Option<NetworkRequirements>,
pub auto_review: Option<AutoReviewRequirements>,
pub models: Option<ModelsRequirements>,
#[schemars(with = "Option<String>")]
pub sqlite_home: Option<PathUri>,
@@ -407,6 +408,13 @@ pub struct ConfigRequirements {
pub windows_sandbox_private_desktop: Option<bool>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct AutoReviewRequirements {
pub required_on_models: Option<Vec<String>>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
codex-rs/app-server-protocol/src/protocol/v2/tests.rs
+1
View File
@@ -1987,6 +1987,7 @@ fn config_requirements_granular_allowed_approval_policy_is_marked_experimental()
hooks: None,
enforce_residency: None,
network: None,
auto_review: None,
models: None,
sqlite_home: None,
log_dir: None,
codex-rs/app-server/README.md
+10 -1
View File
@@ -278,7 +278,7 @@ Example with notification opt-out:
- `externalAgentConfig/import/readHistories` — read completed import histories and connector candidates detected from successfully imported session histories. Successful session entries include the original imported title when one was available. Connector candidates include a normalized display `name`, the number of imported sessions that used the connector, and the source metadata field used for detection.
- `config/value/write` — write a single config key/value to the user's config.toml on disk; dotted paths such as `desktop.someKey` use the same generic write surface. Writes that overlap a managed requirement are rejected with `configRequirementReadonly`.
- `config/batchWrite` — apply multiple config edits atomically to the user's config.toml on disk, with optional `reloadUserConfig: true` to hot-reload loaded threads, including multiple `desktop.*` edits. Session-static model, reasoning-effort, Plan-mode reasoning-effort, service-tier, and personality defaults do not reload existing threads.
- `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including exact managed values (`sqliteHome`, `logDir`, `modelCatalogJson`, `checkForUpdateOnStartup`, `allowLoginShell`, `feedback.enabled`, and `windowsSandboxPrivateDesktop`), allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`), the layered permission-profile allow map (`allowedPermissionProfiles`), the managed permission-profile default (`defaultPermissions`), lifecycle hook lockdown (`allowManagedHooksOnly`), remote-control policy (`allowRemoteControl`; `false` force-disables remote control while `true` or `null` preserves existing behavior), computer use policy (`computerUse`), Browser Use policy (`browserUse.disableAutoReview`), pinned feature values (`featureRequirements`, including the default-allowed `in_app_updates` policy that administrators can set to `false`), managed lifecycle hooks (`hooks`, including command handlers with optional `additionalContextLimit` and `mcp_tool` handlers with `server`, `tool`, `input`, `timeoutSec`, and `statusMessage`), `enforceResidency`, managed new-thread defaults (`models.newThread.model`, `models.newThread.modelReasoningEffort`, and `models.newThread.serviceTier`), and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly` and `dangerFullAccessDenylistOnly`.
- `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including exact managed values (`sqliteHome`, `logDir`, `modelCatalogJson`, `checkForUpdateOnStartup`, `allowLoginShell`, `feedback.enabled`, and `windowsSandboxPrivateDesktop`), allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`), the layered permission-profile allow map (`allowedPermissionProfiles`), the managed permission-profile default (`defaultPermissions`), lifecycle hook lockdown (`allowManagedHooksOnly`), remote-control policy (`allowRemoteControl`; `false` force-disables remote control while `true` or `null` preserves existing behavior), computer use policy (`computerUse`), Browser Use policy (`browserUse.disableAutoReview`), pinned feature values (`featureRequirements`, including the default-allowed `in_app_updates` policy that administrators can set to `false`), managed lifecycle hooks (`hooks`, including command handlers with optional `additionalContextLimit` and `mcp_tool` handlers with `server`, `tool`, `input`, `timeoutSec`, and `statusMessage`), `enforceResidency`, managed automatic review (`autoReview.requiredOnModels`), model defaults (`models.newThread.model`, `models.newThread.modelReasoningEffort`, and `models.newThread.serviceTier`), and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly` and `dangerFullAccessDenylistOnly`.
### Example: Start or resume a thread
@@ -866,6 +866,15 @@ You can optionally specify config overrides on the new turn. If specified, these
- `"user"` — default. Review approval requests directly in the client.
- `"auto_review"` — route approval requests to a carefully prompted subagent, which gathers relevant context and applies a risk-based decision framework before approving or denying the request. The legacy value `"guardian_subagent"` is still accepted for compatibility.
Managed `requirements.toml` can require automatic review for specific models:
```toml
[auto_review]
required_on_models = ["protected-model"]
```
Listed models always start with `approvalPolicy: "on-request"` and `approvalsReviewer: "auto_review"`, even when clients provide incompatible startup values. Full Access is automatically downgraded to workspace-write access. Incompatible runtime overrides or disabled Guardian automatic review are rejected.
```json
{ "method": "turn/start", "id": 30, "params": {
"threadId": "thr_123",
codex-rs/app-server/src/request_processors/config_processor.rs
+20 -5
View File
@@ -7,6 +7,7 @@ use crate::error_code::invalid_request;
use crate::outgoing_message::ConnectionRequestId;
use crate::outgoing_message::OutgoingMessageSender;
use codex_analytics::AnalyticsEventsClient;
use codex_app_server_protocol::AutoReviewRequirements;
use codex_app_server_protocol::BrowserUseRequirements;
use codex_app_server_protocol::ClientResponsePayload;
use codex_app_server_protocol::ComputerUseRequirements;
@@ -415,6 +416,11 @@ fn map_requirements_toml_to_api(requirements: ConfigRequirementsToml) -> ConfigR
.enforce_residency
.map(map_residency_requirement_to_api),
network: requirements.network.map(map_network_requirements_to_api),
auto_review: requirements
.auto_review
.map(|auto_review| AutoReviewRequirements {
required_on_models: auto_review.required_on_models,
}),
models: requirements.models.map(|models| ModelsRequirements {
new_thread: models.new_thread.map(|new_thread| NewThreadModelDefaults {
model: new_thread.model,
@@ -648,6 +654,7 @@ mod tests {
use super::map_requirements_toml_to_api;
use codex_app_server_protocol::FeedbackRequirements;
use codex_app_server_protocol::WindowsSandboxSetupMode;
use codex_config::AutoReviewRequirementsToml;
use codex_config::ComputerUseRequirementsToml;
use codex_config::ConfigRequirementsToml;
use codex_config::ModelsRequirementsToml;
@@ -717,8 +724,11 @@ mod tests {
}
#[test]
fn requirements_api_includes_new_thread_model_defaults() {
fn requirements_api_includes_model_auto_review_and_new_thread_defaults() {
let mapped = map_requirements_toml_to_api(ConfigRequirementsToml {
auto_review: Some(AutoReviewRequirementsToml {
required_on_models: Some(vec!["gpt-protected".to_string()]),
}),
models: Some(ModelsRequirementsToml {
new_thread: Some(NewThreadModelDefaultsToml {
model: Some("gpt-managed".to_string()),
@@ -729,10 +739,15 @@ mod tests {
..ConfigRequirementsToml::default()
});
let defaults = mapped
.models
.and_then(|models| models.new_thread)
.expect("new-thread defaults");
assert_eq!(
mapped
.auto_review
.expect("managed automatic-review requirements")
.required_on_models,
Some(vec!["gpt-protected".to_string()])
);
let models = mapped.models.expect("managed model requirements");
let defaults = models.new_thread.expect("new-thread defaults");
assert_eq!(defaults.model.as_deref(), Some("gpt-managed"));
assert_eq!(
defaults.model_reasoning_effort,
codex-rs/app-server/tests/suite/v2/config_rpc.rs
+18 -6
View File
@@ -179,11 +179,15 @@ in_app_updates = false
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn config_requirements_read_includes_new_thread_model_defaults() -> Result<()> {
async fn config_requirements_read_includes_model_auto_review_and_new_thread_defaults() -> Result<()>
{
let codex_home = TempDir::new()?;
std::fs::write(
codex_home.path().join("requirements.toml"),
r#"
[auto_review]
required_on_models = ["gpt-protected", "gpt-sensitive"]
[models.new_thread]
model = "gpt-managed"
model_reasoning_effort = "medium"
@@ -200,11 +204,19 @@ service_tier = "fast"
let response: ConfigRequirementsReadResponse =
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(request_id)).await??;
let defaults = response
.requirements
.and_then(|requirements| requirements.models)
.and_then(|models| models.new_thread)
.expect("managed new-thread defaults");
let requirements = response.requirements.expect("managed requirements");
assert_eq!(
requirements
.auto_review
.expect("managed automatic-review requirements")
.required_on_models,
Some(vec![
"gpt-protected".to_string(),
"gpt-sensitive".to_string()
])
);
let models = requirements.models.expect("managed model requirements");
let defaults = models.new_thread.expect("managed new-thread defaults");
assert_eq!(defaults.model.as_deref(), Some("gpt-managed"));
assert_eq!(
defaults.model_reasoning_effort,
codex-rs/app-server/tests/suite/v2/mod.rs
+1
View File
@@ -44,6 +44,7 @@ mod mcp_server_elicitation;
mod mcp_server_status;
mod mcp_tool;
mod memory_reset;
mod model_auto_review;
mod model_list;
mod model_provider_capabilities_read;
mod multi_agent_v2_developer_instructions;
codex-rs/app-server/tests/suite/v2/model_auto_review.rs
+313
View File
@@ -0,0 +1,313 @@
use anyhow::Result;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::create_mock_responses_server_repeating_assistant;
use codex_app_server_protocol::ApprovalsReviewer;
use codex_app_server_protocol::ApprovalsReviewer::AutoReview;
use codex_app_server_protocol::ApprovalsReviewer::User;
use codex_app_server_protocol::AskForApproval;
use codex_app_server_protocol::AskForApproval::Never;
use codex_app_server_protocol::AskForApproval::OnRequest;
use codex_app_server_protocol::JSONRPCError;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::SandboxMode;
use codex_app_server_protocol::SandboxPolicy;
use codex_app_server_protocol::ThreadForkParams as ForkParams;
use codex_app_server_protocol::ThreadForkResponse as ForkResponse;
use codex_app_server_protocol::ThreadResumeParams as ResumeParams;
use codex_app_server_protocol::ThreadResumeResponse as ResumeResponse;
use codex_app_server_protocol::ThreadSettingsUpdateParams as UpdateParams;
use codex_app_server_protocol::ThreadSettingsUpdateResponse as UpdateResponse;
use codex_app_server_protocol::ThreadSettingsUpdatedNotification as SettingsUpdated;
use codex_app_server_protocol::ThreadStartParams as StartParams;
use codex_app_server_protocol::TurnStartParams as TurnParams;
use codex_app_server_protocol::TurnStartResponse as TurnResponse;
use codex_app_server_protocol::UserInput;
use codex_features::Feature;
use pretty_assertions::assert_eq;
use std::time::Duration;
use tempfile::TempDir;
use tokio::time::timeout;
const TIMEOUT: Duration = Duration::from_secs(10);
const MODEL: &str = "protected-model";
const REQUIREMENTS: &str = "[auto_review]\nrequired_on_models = [\"protected-model\"]\n";
const UNSAFE: [(Option<AskForApproval>, Option<ApprovalsReviewer>); 2] =
[(Some(Never), None), (None, Some(User))];
macro_rules! params {
($ty:ident, $($field:ident $(= $value:expr)?),* $(,)?) => {
$ty { $($field $( : $value)?,)* ..Default::default() }
};
}
async fn app_server(
config: MockResponsesConfig,
requirements: &str,
) -> Result<(TempDir, TestAppServer)> {
let home = TempDir::new()?;
config.write(home.path())?;
std::fs::write(home.path().join("requirements.toml"), requirements)?;
let server = TestAppServer::builder()
.with_codex_home(home.path())
.build_initialized_with_timeout(TIMEOUT)
.await?;
Ok((home, server))
}
async fn managed_server() -> Result<(TempDir, TestAppServer)> {
app_server(
MockResponsesConfig::new("http://localhost/unused").with_approval_policy("on-request"),
REQUIREMENTS,
)
.await
}
async fn assert_error(server: &mut TestAppServer, request_id: i64, message: &str) -> Result<()> {
let error: JSONRPCError = timeout(
TIMEOUT,
server.read_stream_until_error_message(RequestId::Integer(request_id)),
)
.await??;
assert_eq!(error.error.code, -32600);
assert!(error.error.message.contains(message));
Ok(())
}
async fn assert_protected_update(server: &mut TestAppServer) -> Result<()> {
let updated: SettingsUpdated =
timeout(TIMEOUT, server.read_notification("thread/settings/updated")).await??;
let settings = updated.thread_settings;
assert_protected(
&settings.model,
settings.approval_policy,
settings.approvals_reviewer,
);
Ok(())
}
fn assert_protected(model: &str, policy: AskForApproval, reviewer: ApprovalsReviewer) {
assert_eq!((model, policy, reviewer), (MODEL, OnRequest, AutoReview));
}
#[tokio::test]
async fn thread_start_enforces_protected_model_auto_review() -> Result<()> {
let (_home, mut server) = managed_server().await?;
let started = server
.start_thread(params!(StartParams, model = Some(MODEL.to_string())))
.await?;
assert_protected(
&started.model,
started.approval_policy,
started.approvals_reviewer,
);
for (approval_policy, approvals_reviewer) in UNSAFE {
let started = server
.start_thread(params!(
StartParams,
model = Some(MODEL.to_string()),
approval_policy,
approvals_reviewer,
))
.await?;
assert_protected(
&started.model,
started.approval_policy,
started.approvals_reviewer,
);
}
let started = server
.start_thread(params!(
StartParams,
model = Some(MODEL.to_string()),
approval_policy = Some(Never),
approvals_reviewer = Some(User),
sandbox = Some(SandboxMode::DangerFullAccess),
))
.await?;
assert_protected(
&started.model,
started.approval_policy,
started.approvals_reviewer,
);
assert!(matches!(
started.sandbox,
SandboxPolicy::WorkspaceWrite { .. }
));
let (_home, mut disabled) = app_server(
MockResponsesConfig::new("http://localhost/unused")
.with_approval_policy("on-request")
.disable_feature(Feature::GuardianApproval),
REQUIREMENTS,
)
.await?;
let id = disabled
.send_thread_start_request_with_auto_env(params!(
StartParams,
model = Some(MODEL.to_string())
))
.await?;
assert_error(&mut disabled, id, "you need to use auto review").await
}
#[tokio::test]
async fn thread_and_turn_settings_enforce_protected_model_auto_review() -> Result<()> {
let (_home, mut server) = managed_server().await?;
let thread = server.start_thread(StartParams::default()).await?.thread;
let id = server
.send_thread_settings_update_request(params!(
UpdateParams,
thread_id = thread.id.clone(),
model = Some(MODEL.to_string())
))
.await?;
let _: UpdateResponse = timeout(TIMEOUT, server.read_response(id)).await??;
assert_protected_update(&mut server).await?;
for (approval_policy, approvals_reviewer) in UNSAFE {
let id = server
.send_thread_settings_update_request(params!(
UpdateParams,
thread_id = thread.id.clone(),
approval_policy,
approvals_reviewer,
))
.await?;
assert_error(&mut server, id, "you need to use auto review").await?;
}
let id = server
.send_thread_settings_update_request(params!(
UpdateParams,
thread_id = thread.id.clone(),
sandbox_policy = Some(SandboxPolicy::DangerFullAccess),
))
.await?;
assert_error(&mut server, id, "you need to use auto review").await?;
let id = server
.send_turn_start_request(params!(
TurnParams,
thread_id = thread.id,
approvals_reviewer = Some(User)
))
.await?;
assert_error(&mut server, id, "you need to use auto review").await?;
let turn_thread = server.start_thread(StartParams::default()).await?.thread;
let id = server
.send_turn_start_request(params!(
TurnParams,
thread_id = turn_thread.id.clone(),
model = Some(MODEL.to_string()),
approvals_reviewer = Some(User)
))
.await?;
assert_error(&mut server, id, "you need to use auto review").await?;
let id = server
.send_turn_start_request(params!(
TurnParams,
thread_id = turn_thread.id,
model = Some(MODEL.to_string())
))
.await?;
let _: TurnResponse = timeout(TIMEOUT, server.read_response(id)).await??;
assert_protected_update(&mut server).await
}
#[tokio::test]
async fn thread_resume_and_fork_upgrade_legacy_protected_model_settings() -> Result<()> {
let responses = create_mock_responses_server_repeating_assistant("Done").await;
let (home, mut legacy) = app_server(
MockResponsesConfig::new(&responses.uri()).with_model(MODEL),
"",
)
.await?;
let started = legacy.start_thread(StartParams::default()).await?;
assert_eq!(
(started.approval_policy, started.approvals_reviewer),
(Never, User),
);
let thread_id = started.thread.id;
legacy
.start_turn_and_wait_for_completion(params!(
TurnParams,
thread_id = thread_id.clone(),
input = vec![UserInput::Text {
text: "Save legacy settings".to_string(),
text_elements: Vec::new()
}],
))
.await?;
drop(legacy);
MockResponsesConfig::new(&responses.uri())
.with_model("ordinary-model")
.with_approval_policy("on-request")
.write(home.path())?;
std::fs::write(home.path().join("requirements.toml"), REQUIREMENTS)?;
let mut server = TestAppServer::builder()
.with_codex_home(home.path())
.build_initialized_with_timeout(TIMEOUT)
.await?;
let id = server
.send_thread_fork_request(params!(
ForkParams,
thread_id = thread_id.clone(),
model = Some(MODEL.to_string()),
approval_policy = Some(Never),
approvals_reviewer = Some(User),
))
.await?;
let fork: ForkResponse = timeout(TIMEOUT, server.read_response(id)).await??;
assert_protected(&fork.model, fork.approval_policy, fork.approvals_reviewer);
let id = server
.send_thread_fork_request(params!(
ForkParams,
thread_id = thread_id.clone(),
model = Some(MODEL.to_string())
))
.await?;
let fork: ForkResponse = timeout(TIMEOUT, server.read_response(id)).await??;
assert_protected(&fork.model, fork.approval_policy, fork.approvals_reviewer);
let id = server
.send_thread_resume_request(params!(
ResumeParams,
thread_id = thread_id.clone(),
approval_policy = Some(Never),
approvals_reviewer = Some(User),
))
.await?;
let resumed: ResumeResponse = timeout(TIMEOUT, server.read_response(id)).await??;
assert_protected(
&resumed.model,
resumed.approval_policy,
resumed.approvals_reviewer,
);
let id = server
.send_thread_resume_request(params!(ResumeParams, thread_id))
.await?;
let resumed: ResumeResponse = timeout(TIMEOUT, server.read_response(id)).await??;
assert_protected(
&resumed.model,
resumed.approval_policy,
resumed.approvals_reviewer,
);
Ok(())
}
#[tokio::test]
async fn thread_settings_update_enforces_global_reviewer_requirements() -> Result<()> {
let (_home, mut server) = app_server(
MockResponsesConfig::new("http://localhost/unused").with_approval_policy("on-request"),
"allowed_approvals_reviewers = [\"auto_review\"]\n",
)
.await?;
let thread = server.start_thread(StartParams::default()).await?;
assert_eq!(thread.approvals_reviewer, AutoReview);
let id = server
.send_thread_settings_update_request(params!(
UpdateParams,
thread_id = thread.thread.id,
approvals_reviewer = Some(User)
))
.await?;
assert_error(&mut server, id, "approvals_reviewer").await
}
codex-rs/config/src/config_requirements.rs
+134 -2
View File
@@ -12,6 +12,7 @@ use serde::de::Error as _;
use serde::de::value::Error as ValueDeserializerError;
use serde::de::value::StrDeserializer;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::fmt;
use std::path::PathBuf;
use wildmatch::WildMatchPattern;
@@ -159,6 +160,7 @@ pub struct ConfigRequirements {
pub feedback: Option<Sourced<FeedbackConfigToml>>,
pub approval_policy: ConstrainedWithSource<AskForApproval>,
pub approvals_reviewer: ConstrainedWithSource<ApprovalsReviewer>,
pub auto_review_required_models: Option<Sourced<BTreeSet<String>>>,
pub permission_profile: ConstrainedWithSource<PermissionProfile>,
pub windows_sandbox_mode: ConstrainedWithSource<Option<WindowsSandboxModeToml>>,
pub windows_sandbox_private_desktop: Option<Sourced<bool>>,
@@ -201,6 +203,7 @@ impl Default for ConfigRequirements {
Constrained::allow_any_from_default(),
/*source*/ None,
),
auto_review_required_models: None,
permission_profile: ConstrainedWithSource::new(
Constrained::allow_any(PermissionProfile::read_only()),
/*source*/ None,
@@ -236,6 +239,29 @@ impl Default for ConfigRequirements {
}
impl ConfigRequirements {
/// Returns whether a model slug or its supported provider alias requires auto-review.
pub fn auto_review_required_for_model(&self, model: &str) -> bool {
let Some(protected_models) = self.auto_review_required_models.as_ref() else {
return false;
};
let model = match model.split_once('/') {
Some((namespace, suffix))
if !namespace.is_empty()
&& !suffix.contains('/')
&& namespace.chars().all(|character| {
character.is_ascii_alphanumeric() || character == '_' || character == '-'
}) =>
{
suffix
}
Some(_) => return false,
None => model,
};
protected_models.value.contains(model)
}
pub fn managed_auth_policy(&self) -> ManagedAuthPolicy {
ManagedAuthPolicy {
allowed_login_methods: self
@@ -930,10 +956,16 @@ pub struct ConfigRequirementsToml {
#[serde(rename = "experimental_network")]
pub network: Option<NetworkRequirementsToml>,
pub permissions: Option<PermissionsRequirementsToml>,
pub auto_review: Option<AutoReviewRequirementsToml>,
pub models: Option<ModelsRequirementsToml>,
pub guardian_policy_config: Option<String>,
}
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
pub struct AutoReviewRequirementsToml {
pub required_on_models: Option<Vec<String>>,
}
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
pub struct ModelsRequirementsToml {
pub new_thread: Option<NewThreadModelDefaultsToml>,
@@ -1020,6 +1052,7 @@ pub struct ConfigRequirementsWithSources {
pub enforce_residency: Option<Sourced<ResidencyRequirement>>,
pub network: Option<Sourced<NetworkRequirementsToml>>,
pub permissions: Option<Sourced<PermissionsRequirementsToml>>,
pub auto_review: Option<Sourced<AutoReviewRequirementsToml>>,
pub models: Option<Sourced<ModelsRequirementsToml>>,
pub guardian_policy_config: Option<Sourced<String>>,
}
@@ -1074,6 +1107,7 @@ impl ConfigRequirementsWithSources {
enforce_residency: _,
network: _,
permissions: _,
auto_review: _,
models: _,
guardian_policy_config: _,
} = &other;
@@ -1125,6 +1159,32 @@ impl ConfigRequirementsWithSources {
}
);
if let Some(incoming_auto_review) = other.auto_review.take() {
if let Some(existing_auto_review) = self.auto_review.as_mut() {
let mut source_contributed = false;
if let Some(incoming_slugs) = incoming_auto_review.required_on_models {
let protected_slugs = existing_auto_review
.value
.required_on_models
.get_or_insert_default();
for slug in incoming_slugs {
if !protected_slugs.contains(&slug) {
protected_slugs.push(slug);
source_contributed = true;
}
}
}
if source_contributed && existing_auto_review.source != source {
existing_auto_review.source = RequirementSource::composite([
existing_auto_review.source.clone(),
source.clone(),
]);
}
} else {
self.auto_review = Some(Sourced::new(incoming_auto_review, source.clone()));
}
}
if let Some(incoming_apps) = other.apps.take() {
if let Some(existing_apps) = self.apps.as_mut() {
merge_app_requirements_descending(&mut existing_apps.value, incoming_apps);
@@ -1166,6 +1226,7 @@ impl ConfigRequirementsWithSources {
enforce_residency,
network,
permissions,
auto_review,
models,
guardian_policy_config,
} = self;
@@ -1201,6 +1262,7 @@ impl ConfigRequirementsWithSources {
enforce_residency: enforce_residency.map(|sourced| sourced.value),
network: network.map(|sourced| sourced.value),
permissions: permissions.map(|sourced| sourced.value),
auto_review: auto_review.map(|sourced| sourced.value),
models: models.map(|sourced| sourced.value),
guardian_policy_config: guardian_policy_config.map(|sourced| sourced.value),
}
@@ -1329,6 +1391,12 @@ impl ConfigRequirementsToml {
&& self.enforce_residency.is_none()
&& self.network.is_none()
&& self.permissions.is_none()
&& self.auto_review.as_ref().is_none_or(|auto_review| {
auto_review
.required_on_models
.as_ref()
.is_none_or(Vec::is_empty)
})
&& self
.models
.as_ref()
@@ -1451,8 +1519,8 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
fn try_from(toml: ConfigRequirementsWithSources) -> Result<Self, Self::Error> {
// Profile catalog selection remains on ConfigRequirementsToml for
// config loading and requirements API projection. Managed new-thread
// defaults also remain there because they are initialization values,
// not runtime constraints.
// defaults also remain there because they are initialization values;
// model-specific auto-review requirements are runtime constraints.
let ConfigRequirementsWithSources {
allowed_login_methods,
allowed_chatgpt_workspaces,
@@ -1484,10 +1552,38 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
enforce_residency,
network,
permissions,
auto_review,
models: _,
guardian_policy_config,
} = toml;
let auto_review_required_models = auto_review
.and_then(|auto_review| {
auto_review
.value
.required_on_models
.map(|slugs| Sourced::new(slugs, auto_review.source))
})
.filter(|models| !models.value.is_empty())
.map(|models| {
let Sourced { value, source } = models;
let mut protected_models = BTreeSet::new();
for slug in value {
if slug.trim().is_empty() || slug.trim() != slug || slug.contains('/') {
return Err(ConstraintError::InvalidValue {
field_name: "auto_review.required_on_models",
candidate: format!("{slug:?}"),
allowed: "non-empty model slugs without surrounding whitespace or provider namespaces"
.to_string(),
requirement_source: source,
});
}
protected_models.insert(slug);
}
Ok(Sourced::new(protected_models, source))
})
.transpose()?;
if let Some(requirements) = &mcp_servers {
validate_mcp_server_requirements(
&requirements.value,
@@ -1794,6 +1890,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
feedback,
approval_policy,
approvals_reviewer,
auto_review_required_models,
permission_profile,
windows_sandbox_mode,
windows_sandbox_private_desktop,
@@ -1971,6 +2068,7 @@ mod tests {
enforce_residency,
network,
permissions,
auto_review,
models,
guardian_policy_config,
} = toml;
@@ -2021,6 +2119,7 @@ mod tests {
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
network: network.map(|value| Sourced::new(value, RequirementSource::Unknown)),
permissions: permissions.map(|value| Sourced::new(value, RequirementSource::Unknown)),
auto_review: auto_review.map(|value| Sourced::new(value, RequirementSource::Unknown)),
models: models.map(|value| Sourced::new(value, RequirementSource::Unknown)),
guardian_policy_config: guardian_policy_config
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
@@ -2199,6 +2298,34 @@ mod tests {
Ok(())
}
#[test]
fn auto_review_required_for_model_matches_exact_provider_aliases() {
let requirements = ConfigRequirements {
auto_review_required_models: Some(Sourced::new(
BTreeSet::from(["protected-model".to_string()]),
RequirementSource::Unknown,
)),
..Default::default()
};
for (model, protected) in [
("protected-model", true),
("protected-model-preview", false),
("openai-codex/protected-model-preview", false),
("provider_1/protected-model", true),
("protected-modelish", false),
("/protected-model", false),
("bad.provider/protected-model", false),
("provider/nested/protected-model", false),
] {
assert_eq!(
requirements.auto_review_required_for_model(model),
protected,
"{model}"
);
}
}
#[test]
fn merge_unset_fields_copies_every_field_and_sets_sources() {
let mut target = ConfigRequirementsWithSources::default();
@@ -2221,6 +2348,9 @@ mod tests {
let computer_use = ComputerUseRequirementsToml {
allow_locked_computer_use: Some(false),
};
let auto_review = AutoReviewRequirementsToml {
required_on_models: Some(vec!["managed-model".to_string()]),
};
let models = ModelsRequirementsToml {
new_thread: Some(NewThreadModelDefaultsToml {
model: Some("managed-model".to_string()),
@@ -2280,6 +2410,7 @@ mod tests {
enforce_residency: Some(enforce_residency),
network: None,
permissions: None,
auto_review: Some(auto_review.clone()),
models: Some(models.clone()),
guardian_policy_config: Some(guardian_policy_config.clone()),
};
@@ -2349,6 +2480,7 @@ mod tests {
enforce_residency: Some(Sourced::new(enforce_residency, enforce_source)),
network: None,
permissions: None,
auto_review: Some(Sourced::new(auto_review, source.clone())),
models: Some(Sourced::new(models, source.clone())),
guardian_policy_config: Some(Sourced::new(guardian_policy_config, source)),
}
codex-rs/config/src/constraint.rs
+3
View File
@@ -16,6 +16,9 @@ pub enum ConstraintError {
requirement_source: RequirementSource,
},
#[error("To use model `{model}`, you need to use auto review.")]
AutoReviewRequired { model: String },
#[error("field `{field_name}` cannot be empty")]
EmptyField { field_name: String },
codex-rs/config/src/lib.rs
+1
View File
@@ -60,6 +60,7 @@ pub use config_requirements::AppRequirementToml;
pub use config_requirements::AppToolRequirementToml;
pub use config_requirements::AppToolsRequirementsToml;
pub use config_requirements::AppsRequirementsToml;
pub use config_requirements::AutoReviewRequirementsToml;
pub use config_requirements::BrowserUseRequirementsToml;
pub use config_requirements::ComputerUseRequirementsToml;
pub use config_requirements::ConfigRequirements;
codex-rs/config/src/requirements_layers/layer.rs
+3
View File
@@ -111,6 +111,7 @@ impl ComposableRequirementsLayer {
rules: requirements.rules,
hooks: requirements.hooks,
permissions: requirements.permissions,
auto_review: requirements.auto_review,
},
})
}
@@ -121,6 +122,7 @@ pub(super) struct DomainMergedRequirementsFields {
pub(super) rules: Option<RequirementsExecPolicyToml>,
pub(super) hooks: Option<ManagedHooksRequirementsToml>,
pub(super) permissions: Option<crate::config_requirements::PermissionsRequirementsToml>,
pub(super) auto_review: Option<crate::config_requirements::AutoReviewRequirementsToml>,
}
fn parse_layer_toml(
@@ -218,6 +220,7 @@ fn strip_special_fields(layer_toml: &mut TomlValue) {
remove_top_level_field(layer_toml, "rules");
remove_top_level_field(layer_toml, "hooks");
remove_nested_field_and_prune_empty(layer_toml, &["permissions", "filesystem", "deny_read"]);
remove_nested_field_and_prune_empty(layer_toml, &["auto_review", "required_on_models"]);
}
fn remove_top_level_field(value: &mut TomlValue, key: &str) -> Option<TomlValue> {
codex-rs/config/src/requirements_layers/mod.rs
+1
View File
@@ -1,5 +1,6 @@
mod hooks;
mod layer;
mod models;
mod permissions;
mod rules;
mod stack;
codex-rs/config/src/requirements_layers/models.rs
+59
View File
@@ -0,0 +1,59 @@
//! Model slugs requiring auto-review remain protected across all policy layers.
use crate::AutoReviewRequirementsToml;
use crate::RequirementSource;
use crate::Sourced;
use super::stack::merge_output_source;
#[derive(Default)]
pub(super) struct AutoReviewModelsMergeState {
slugs: Vec<String>,
source: Option<RequirementSource>,
}
impl AutoReviewModelsMergeState {
pub(super) fn merge(
&mut self,
incoming: Option<AutoReviewRequirementsToml>,
source: &RequirementSource,
) {
let Some(incoming_slugs) = incoming
.and_then(|auto_review| auto_review.required_on_models)
.filter(|slugs| !slugs.is_empty())
else {
return;
};
for slug in incoming_slugs {
if !self.slugs.contains(&slug) {
self.slugs.push(slug);
if let Some(existing_source) = self.source.as_mut() {
merge_output_source(existing_source, source);
} else {
self.source = Some(source.clone());
}
}
}
}
pub(super) fn apply_to(self, target: &mut Option<Sourced<AutoReviewRequirementsToml>>) {
if self.slugs.is_empty() {
return;
}
let source = self.source.unwrap_or(RequirementSource::Unknown);
let Some(existing) = target.as_mut() else {
*target = Some(Sourced::new(
AutoReviewRequirementsToml {
required_on_models: Some(self.slugs),
},
source,
));
return;
};
existing.value.required_on_models = Some(self.slugs);
merge_output_source(&mut existing.source, &source);
}
}
codex-rs/config/src/requirements_layers/stack.rs
+6
View File
@@ -12,6 +12,7 @@
//! active managed-dir conflicts.
//! - `permissions.filesystem.deny_read` is a high-priority-first union across
//! layers.
//! - `auto_review.required_on_models` is a high-priority-first union across layers.
use crate::ConfigRequirementsToml;
use crate::ConfigRequirementsWithSources;
@@ -27,6 +28,7 @@ use super::hooks::HookDirectoryField;
use super::hooks::HookMergeState;
use super::layer::ComposableRequirementsLayer;
use super::layer::RequirementsLayerEntry;
use super::models::AutoReviewModelsMergeState;
use super::permissions::DenyReadMergeState;
#[derive(Debug, Error, PartialEq, Eq)]
@@ -165,6 +167,7 @@ impl RequirementsLayerStack {
let mut hooks = HookMergeState::new(hook_directory_field);
let mut hooks_output = None;
let mut deny_read = DenyReadMergeState::default();
let mut auto_review_models = AutoReviewModelsMergeState::default();
// Regular TOML fields are folded low-to-high like config. These custom
// fields append or union values, so process them high-to-low to keep
// priority order visible in the output.
@@ -177,10 +180,12 @@ impl RequirementsLayerStack {
&layer.source,
)?;
deny_read.merge(domain_fields.permissions.clone(), &layer.source);
auto_review_models.merge(domain_fields.auto_review.clone(), &layer.source);
}
output.rules = rules;
output.hooks = hooks_output;
deny_read.apply_to(&mut output.permissions);
auto_review_models.apply_to(&mut output.auto_review);
let output_is_empty = output.clone().into_toml().is_empty();
Ok((!output_is_empty).then_some(output))
@@ -237,6 +242,7 @@ fn populate_merged_regular_fields_with_sources(
enforce_residency,
network,
permissions,
auto_review: _,
models,
guardian_policy_config,
} = requirements;
codex-rs/config/src/requirements_layers/stack_tests.rs
+51
View File
@@ -169,6 +169,57 @@ service_tier = "fast"
);
}
#[test]
fn auto_review_required_models_are_unioned_without_overwriting_new_thread_defaults() {
let low = layer(
"req_low",
"Low",
r#"[auto_review]
required_on_models = ["low-model", "shared-model"]
[models.new_thread]
model = "low-priority-model"
model_reasoning_effort = "low""#,
);
let high = layer(
"req_high",
"High",
r#"[auto_review]
required_on_models = ["high-model", "shared-model"]
[models.new_thread]
model = "high-priority-model""#,
);
let expected_source = RequirementSource::composite([high.source.clone(), low.source.clone()]);
let composed = compose_requirements_for_hostname(
vec![
low,
high,
layer(
"req_empty",
"Empty",
"[auto_review]\nrequired_on_models = []",
),
],
/*hostname*/ None,
)
.expect("compose requirements")
.expect("requirements present");
assert_eq!(
composed.clone().into_toml(),
expected_requirements(
r#"[auto_review]
required_on_models = ["high-model", "shared-model", "low-model"]
[models.new_thread]
model = "high-priority-model"
model_reasoning_effort = "low""#
)
);
assert_eq!(
composed.auto_review.map(|auto_review| auto_review.source),
Some(expected_source)
);
}
#[test]
fn relative_paths_resolve_against_their_own_layer_base() {
let low_dir = tempdir().expect("low-priority requirements directory");
codex-rs/core/src/config/config_tests.rs
+1
View File
@@ -9480,6 +9480,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset()
enforce_residency: None,
network: None,
permissions: None,
auto_review: None,
models: None,
guardian_policy_config: None,
};
codex-rs/core/src/config/mod.rs
+1
View File
@@ -3270,6 +3270,7 @@ impl Config {
feedback: _,
approval_policy: mut constrained_approval_policy,
approvals_reviewer: mut constrained_approvals_reviewer,
auto_review_required_models: _,
permission_profile: mut constrained_permission_profile,
windows_sandbox_mode: mut constrained_windows_sandbox_mode,
windows_sandbox_private_desktop: _,
codex-rs/core/src/connectors.rs
+9 -5
View File
@@ -506,12 +506,14 @@ pub fn with_app_plugin_sources(
pub(crate) fn mcp_approvals_reviewer(
config: &Config,
model: Option<&str>,
server_name: &str,
connector_id: Option<&str>,
) -> ApprovalsReviewer {
mcp_approvals_reviewer_from_layers(
&config.config_layer_stack,
config.approvals_reviewer,
model,
server_name,
connector_id,
)
@@ -520,9 +522,15 @@ pub(crate) fn mcp_approvals_reviewer(
pub(crate) fn mcp_approvals_reviewer_from_layers(
config_layer_stack: &codex_config::ConfigLayerStack,
default_reviewer: ApprovalsReviewer,
model: Option<&str>,
server_name: &str,
connector_id: Option<&str>,
) -> ApprovalsReviewer {
let requirements = config_layer_stack.requirements();
if model.is_some_and(|model| requirements.auto_review_required_for_model(model)) {
return ApprovalsReviewer::AutoReview;
}
let app_reviewer = if server_name == CODEX_APPS_MCP_SERVER_NAME {
apps_config_from_layer_stack(config_layer_stack).and_then(|apps_config| {
connector_id
@@ -539,11 +547,7 @@ pub(crate) fn mcp_approvals_reviewer_from_layers(
};
if let Some(reviewer) = app_reviewer
&& config_layer_stack
.requirements()
.approvals_reviewer
.can_set(&reviewer)
.is_ok()
&& requirements.approvals_reviewer.can_set(&reviewer).is_ok()
{
return reviewer;
}
codex-rs/core/src/connectors_tests.rs
+31 -5
View File
@@ -345,23 +345,39 @@ approvals_reviewer = "{app}"
.expect("config should build");
assert_eq!(
mcp_approvals_reviewer(&config, CODEX_APPS_MCP_SERVER_NAME, Some("calendar")),
mcp_approvals_reviewer(
&config,
config.model.as_deref(),
CODEX_APPS_MCP_SERVER_NAME,
Some("calendar")
),
expected_app
);
assert_eq!(
mcp_approvals_reviewer(&config, CODEX_APPS_MCP_SERVER_NAME, Some("drive")),
mcp_approvals_reviewer(
&config,
config.model.as_deref(),
CODEX_APPS_MCP_SERVER_NAME,
Some("drive")
),
expected_default
);
assert_eq!(
mcp_approvals_reviewer(
&config,
config.model.as_deref(),
CODEX_APPS_MCP_SERVER_NAME,
/*connector_id*/ None
),
expected_default
);
assert_eq!(
mcp_approvals_reviewer(&config, "custom_server", Some("calendar")),
mcp_approvals_reviewer(
&config,
config.model.as_deref(),
"custom_server",
Some("calendar")
),
expected_global
);
}
@@ -392,7 +408,12 @@ approvals_reviewer = "user"
.expect("config should build");
assert_eq!(
mcp_approvals_reviewer(&config, CODEX_APPS_MCP_SERVER_NAME, Some("calendar")),
mcp_approvals_reviewer(
&config,
config.model.as_deref(),
CODEX_APPS_MCP_SERVER_NAME,
Some("calendar")
),
ApprovalsReviewer::AutoReview
);
}
@@ -422,7 +443,12 @@ approvals_reviewer = "user"
.expect("config should build");
assert_eq!(
mcp_approvals_reviewer(&config, CODEX_APPS_MCP_SERVER_NAME, Some("calendar")),
mcp_approvals_reviewer(
&config,
config.model.as_deref(),
CODEX_APPS_MCP_SERVER_NAME,
Some("calendar")
),
ApprovalsReviewer::AutoReview
);
}
codex-rs/core/src/mcp_tool_call.rs
+2
View File
@@ -1293,6 +1293,7 @@ async fn maybe_request_mcp_tool_approval(
let approvals_reviewer = connectors::mcp_approvals_reviewer_from_layers(
&config.config_layer_stack,
config.approvals_reviewer,
Some(turn_context.model_info.slug.as_str()),
&invocation.server,
metadata.connector_id.as_deref(),
);
@@ -1455,6 +1456,7 @@ pub(crate) fn mcp_approvals_reviewer(
) -> ApprovalsReviewer {
connectors::mcp_approvals_reviewer(
turn_context.config.as_ref(),
Some(turn_context.model_info.slug.as_str()),
server_name,
metadata.and_then(|metadata| metadata.connector_id.as_deref()),
)
codex-rs/core/src/session/mcp.rs
+2
View File
@@ -752,6 +752,7 @@ async fn review_guardian_mcp_elicitation(
|| crate::connectors::mcp_approvals_reviewer_from_layers(
&mcp_config.config_layer_stack,
ApprovalsReviewer::AutoReview,
Some(turn_context.model_info.slug.as_str()),
request.server_name.as_str(),
connector_id,
) != ApprovalsReviewer::AutoReview
@@ -821,6 +822,7 @@ async fn review_guardian_mcp_elicitation(
let approvals_reviewer = crate::connectors::mcp_approvals_reviewer_from_layers(
&mcp_config.config_layer_stack,
mcp_config.approvals_reviewer,
Some(turn_context.model_info.slug.as_str()),
request.server_name.as_str(),
elicitation_connector_id(&request.elicitation),
);
codex-rs/core/src/session/mod.rs
+44
View File
@@ -622,6 +622,46 @@ impl Session {
config.http_client_factory(),
)
.await;
let trusted_guardian_reviewer =
crate::guardian::is_guardian_reviewer_source(&session_source)
&& !matches!(conversation_history, InitialHistory::Resumed(_));
if config
.config_layer_stack
.requirements()
.auto_review_required_for_model(&model)
&& !trusted_guardian_reviewer
{
let config = Arc::make_mut(&mut config);
if matches!(
config.legacy_sandbox_policy(),
SandboxPolicy::DangerFullAccess
) {
let permission_profile = PermissionProfile::workspace_write();
config
.permissions
.set_permission_profile(permission_profile.clone())
.map_err(|err| CodexErr::InvalidRequest(err.to_string()))?;
if let Some(network) = config.permissions.network.as_ref() {
config.permissions.network = Some(
network
.recompute_for_permission_profile(&permission_profile)
.map_err(|err| CodexErr::InvalidRequest(err.to_string()))?,
);
}
}
config
.permissions
.approval_policy
.set(AskForApproval::OnRequest)
.map_err(|err| CodexErr::InvalidRequest(err.to_string()))?;
config
.config_layer_stack
.requirements()
.approvals_reviewer
.can_set(&ApprovalsReviewer::AutoReview)
.map_err(|err| CodexErr::InvalidRequest(err.to_string()))?;
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
}
if allow_provider_model_fallback
&& let Some(requested_model) = config.model.as_ref()
&& model != *requested_model
@@ -709,6 +749,7 @@ impl Session {
metrics_service_name,
app_server_client_name: None,
app_server_client_version: None,
trusted_guardian_reviewer,
session_source,
history_mode,
forked_from_thread_id,
@@ -718,6 +759,9 @@ impl Session {
dynamic_tools,
user_shell_override,
};
session_configuration
.validate_auto_review_requirement()
.map_err(|err| CodexErr::InvalidRequest(err.to_string()))?;
// Generate a unique ID for the lifetime of this session.
let session_source_clone = session_configuration.session_source.clone();
codex-rs/core/src/session/session.rs
+64
View File
@@ -109,6 +109,8 @@ pub(crate) struct SessionConfiguration {
pub(super) metrics_service_name: Option<String>,
pub(super) app_server_client_name: Option<String>,
pub(super) app_server_client_version: Option<String>,
/// Guardian reviewer identity is trusted only when established during an in-memory spawn.
pub(super) trusted_guardian_reviewer: bool,
/// Source of the session (cli, vscode, exec, mcp, ...)
pub(super) session_source: SessionSource,
/// Persisted thread history contract selected when this thread was created.
@@ -251,6 +253,38 @@ impl SessionConfiguration {
}
}
pub(super) fn validate_auto_review_requirement(&self) -> ConstraintResult<()> {
if self.trusted_guardian_reviewer {
return Ok(());
}
let requirements = self
.original_config_do_not_use
.config_layer_stack
.requirements();
let model = self.collaboration_mode.model();
if !requirements.auto_review_required_for_model(model) {
return Ok(());
}
if self.approval_policy.value() == AskForApproval::OnRequest
&& self.approvals_reviewer == ApprovalsReviewer::AutoReview
&& !self
.file_system_sandbox_policy()
.has_full_disk_write_access()
&& self
.original_config_do_not_use
.features
.enabled(Feature::GuardianApproval)
{
return Ok(());
}
Err(ConstraintError::AutoReviewRequired {
model: model.to_string(),
})
}
pub(crate) fn apply(&self, updates: &SessionSettingsUpdate) -> ConstraintResult<Self> {
let mut next_configuration = self.clone();
let current_sandbox_policy = self.sandbox_policy();
@@ -303,8 +337,37 @@ impl SessionConfiguration {
next_configuration.approval_policy.set(approval_policy)?;
}
if let Some(approvals_reviewer) = updates.approvals_reviewer {
next_configuration
.original_config_do_not_use
.config_layer_stack
.requirements()
.approvals_reviewer
.can_set(&approvals_reviewer)?;
next_configuration.approvals_reviewer = approvals_reviewer;
}
if !next_configuration.trusted_guardian_reviewer
&& self.collaboration_mode.model() != next_configuration.collaboration_mode.model()
&& next_configuration
.original_config_do_not_use
.config_layer_stack
.requirements()
.auto_review_required_for_model(next_configuration.collaboration_mode.model())
{
if updates.approval_policy.is_none() {
next_configuration
.approval_policy
.set(AskForApproval::OnRequest)?;
}
if updates.approvals_reviewer.is_none() {
next_configuration
.original_config_do_not_use
.config_layer_stack
.requirements()
.approvals_reviewer
.can_set(&ApprovalsReviewer::AutoReview)?;
next_configuration.approvals_reviewer = ApprovalsReviewer::AutoReview;
}
}
if let Some(windows_sandbox_level) = updates.windows_sandbox_level {
next_configuration.windows_sandbox_level = windows_sandbox_level;
}
@@ -405,6 +468,7 @@ impl SessionConfiguration {
if let Some(app_server_client_version) = updates.app_server_client_version.clone() {
next_configuration.app_server_client_version = Some(app_server_client_version);
}
next_configuration.validate_auto_review_requirement()?;
Ok(next_configuration)
}
codex-rs/core/src/session/tests.rs
+8
View File
@@ -4107,6 +4107,7 @@ async fn set_rate_limits_retains_previous_credits() {
metrics_service_name: None,
app_server_client_name: None,
app_server_client_version: None,
trusted_guardian_reviewer: false,
session_source: SessionSource::Exec,
history_mode: Default::default(),
forked_from_thread_id: None,
@@ -4216,6 +4217,7 @@ async fn set_rate_limits_updates_plan_type_when_present() {
metrics_service_name: None,
app_server_client_name: None,
app_server_client_version: None,
trusted_guardian_reviewer: false,
session_source: SessionSource::Exec,
history_mode: Default::default(),
forked_from_thread_id: None,
@@ -4762,6 +4764,7 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati
metrics_service_name: None,
app_server_client_name: None,
app_server_client_version: None,
trusted_guardian_reviewer: false,
session_source: SessionSource::Exec,
history_mode: Default::default(),
forked_from_thread_id: None,
@@ -5557,6 +5560,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
metrics_service_name: None,
app_server_client_name: None,
app_server_client_version: None,
trusted_guardian_reviewer: false,
session_source: SessionSource::Exec,
history_mode: Default::default(),
forked_from_thread_id: None,
@@ -5698,6 +5702,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
metrics_service_name: None,
app_server_client_name: None,
app_server_client_version: None,
trusted_guardian_reviewer: false,
session_source: SessionSource::Exec,
history_mode: Default::default(),
forked_from_thread_id: None,
@@ -5972,6 +5977,7 @@ async fn make_session_with_config_and_rx(
metrics_service_name: None,
app_server_client_name: None,
app_server_client_version: None,
trusted_guardian_reviewer: false,
session_source: SessionSource::Exec,
history_mode: Default::default(),
forked_from_thread_id: None,
@@ -6086,6 +6092,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx(
metrics_service_name: None,
app_server_client_name: None,
app_server_client_version: None,
trusted_guardian_reviewer: false,
session_source: session_source.clone(),
history_mode: Default::default(),
forked_from_thread_id: None,
@@ -7918,6 +7925,7 @@ where
metrics_service_name: None,
app_server_client_name: None,
app_server_client_version: None,
trusted_guardian_reviewer: false,
session_source: SessionSource::Exec,
history_mode: Default::default(),
forked_from_thread_id: None,
codex-rs/core/tests/suite/mcp_turn_metadata.rs
+32 -7
View File
@@ -2,6 +2,7 @@
#![allow(clippy::unwrap_used)]
use anyhow::Result;
use codex_config::test_support::CloudConfigBundleFixture;
use codex_config::types::AppToolApproval;
use codex_core::config::Config;
use codex_features::Feature;
@@ -45,6 +46,7 @@ use core_test_support::wait_for_event_match;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::collections::HashMap;
use test_case::test_case;
fn set_calendar_approval_mode(config: &mut Config, approval_mode: AppToolApproval) {
let approval_mode = match approval_mode {
@@ -97,11 +99,12 @@ async fn submit_user_turn(
test: &TestCodex,
text: &str,
approval_policy: AskForApproval,
permission_profile: PermissionProfile,
collaboration_mode: Option<CollaborationMode>,
) -> Result<()> {
let session_model = test.session_configured.model.clone();
let (sandbox_policy, permission_profile) =
turn_permission_fields(PermissionProfile::Disabled, test.cwd.path());
turn_permission_fields(permission_profile, test.cwd.path());
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
@@ -205,6 +208,7 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> R
&test,
"Use [$calendar](app://calendar) to create a calendar event.",
AskForApproval::OnRequest,
PermissionProfile::Disabled,
/*collaboration_mode*/ None,
)
.await?;
@@ -261,8 +265,11 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> R
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apps_default_prompt_with_auto_review_routes_actual_mcp_approval_to_guardian() -> Result<()>
{
#[test_case(false; "unmanaged model")]
#[test_case(true; "protected model")]
async fn apps_default_prompt_with_auto_review_routes_actual_mcp_approval_to_guardian(
protected_model: bool,
) -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@@ -309,9 +316,13 @@ async fn apps_default_prompt_with_auto_review_routes_actual_mcp_approval_to_guar
.await;
let mut builder = search_capable_apps_builder(apps_server.chatgpt_base_url.clone())
.with_config(|config| {
// Use the opposite global reviewer so this route must come from apps._default.
config.approvals_reviewer = ApprovalsReviewer::User;
.with_config(move |config| {
let (reviewer, app_reviewer) = if protected_model {
(ApprovalsReviewer::AutoReview, ApprovalsReviewer::User)
} else {
(ApprovalsReviewer::User, ApprovalsReviewer::AutoReview)
};
config.approvals_reviewer = reviewer;
config
.features
.enable(Feature::ToolCallMcpElicitation)
@@ -319,15 +330,27 @@ async fn apps_default_prompt_with_auto_review_routes_actual_mcp_approval_to_guar
set_default_app_approval_mode_and_reviewer(
config,
AppToolApproval::Prompt,
ApprovalsReviewer::AutoReview,
app_reviewer,
);
});
if protected_model {
builder = builder.with_model("gpt-5.4").with_cloud_config_bundle(
CloudConfigBundleFixture::loader_with_enterprise_requirement(
"[auto_review]\nrequired_on_models = [\"gpt-5.4\"]\n",
),
);
}
let test = builder.build(&server).await?;
submit_user_turn(
&test,
"Use [$calendar](app://calendar) to create a calendar event.",
AskForApproval::OnRequest,
if protected_model {
PermissionProfile::workspace_write()
} else {
PermissionProfile::Disabled
},
/*collaboration_mode*/ None,
)
.await?;
@@ -428,6 +451,7 @@ async fn apps_default_writes_prompts_for_writes_but_not_reads() -> Result<()> {
&test,
"Use [$calendar](app://calendar) to list events, then create one.",
AskForApproval::OnRequest,
PermissionProfile::Disabled,
/*collaboration_mode*/ None,
)
.await?;
@@ -598,6 +622,7 @@ async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Resul
&test,
"Ask for confirmation, then create a calendar event.",
AskForApproval::Never,
PermissionProfile::Disabled,
Some(CollaborationMode {
mode: ModeKind::Plan,
settings: Settings {
codex-rs/tui/src/app/thread_routing.rs
+22
View File
@@ -1233,6 +1233,28 @@ impl App {
turns: Vec<Turn>,
presentation: ThreadAttachPresentation,
) -> Result<()> {
if let Err(err) = self
.config
.permissions
.approval_policy
.set(session.approval_policy.to_core())
{
tracing::warn!(%err, "failed to sync app approval policy from thread session");
}
if let Err(err) = self
.config
.permissions
.set_permission_profile_from_session_snapshot(
PermissionProfileSnapshot::from_session_snapshot(
session.permission_profile.clone(),
session.active_permission_profile.clone(),
),
)
{
tracing::warn!(%err, "failed to sync app permissions from thread session");
}
self.config.approvals_reviewer = session.approvals_reviewer;
let thread_id = session.thread_id;
self.primary_thread_id = Some(thread_id);
self.primary_session_configured = Some(session.clone());
codex-rs/tui/src/app_server_session.rs
+11
View File
@@ -2001,6 +2001,17 @@ fn display_permission_profile_from_thread_response(
thread_params_mode: ThreadParamsMode,
) -> PermissionProfile {
match thread_params_mode {
ThreadParamsMode::Embedded
if matches!(
config.permissions.effective_permission_profile(),
PermissionProfile::Disabled
) && !matches!(
sandbox,
codex_app_server_protocol::SandboxPolicy::DangerFullAccess
) =>
{
PermissionProfile::from_legacy_sandbox_policy_for_cwd(&sandbox.to_core(), cwd)
}
ThreadParamsMode::Embedded => config.permissions.effective_permission_profile(),
ThreadParamsMode::Remote => {
PermissionProfile::from_legacy_sandbox_policy_for_cwd(&sandbox.to_core(), cwd)
codex-rs/tui/src/chatwidget/permission_popups.rs
+7 -4
View File
@@ -104,12 +104,17 @@ impl ChatWidget {
name: APPROVE_FOR_ME_LABEL.to_string(),
description: Some(AUTO_REVIEW_DESCRIPTION.to_string()),
is_current: current_review_policy == ApprovalsReviewer::AutoReview
&& Self::preset_matches_current(
&& (Self::preset_matches_current(
current_approval,
&current_permission_profile,
self.config.cwd.as_path(),
&preset,
),
) || (current_approval == AskForApproval::OnRequest
&& self
.config
.config_layer_stack
.requirements()
.auto_review_required_for_model(self.current_model()))),
actions: self.permission_mode_actions(
&preset,
APPROVE_FOR_ME_LABEL.to_string(),
@@ -393,8 +398,6 @@ impl ChatWidget {
PermissionProfile::Managed { .. }
) && file_system_policy.can_write_path_with_cwd(cwd, cwd)
&& !file_system_policy.has_full_disk_write_access()
&& current_permission_profile.network_sandbox_policy()
== preset.permission_profile.network_sandbox_policy()
}
_ => current_permission_profile == &preset.permission_profile,
}
codex-rs/tui/src/debug_config.rs
+1
View File
@@ -975,6 +975,7 @@ interrupt_message = false
enforce_residency: Some(ResidencyRequirement::Us),
network: None,
permissions: None,
auto_review: None,
models: None,
};