mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
app-server: accept permission profile overrides (#18279)
## Why `PermissionProfile` is becoming the canonical permissions shape shared by core and app-server. After app-server responses expose the active profile, clients need to be able to send that same shape back when starting, resuming, forking, or overriding a turn instead of translating through the legacy `sandbox`/`sandboxPolicy` shorthands. This still needs to preserve the existing requirements/platform enforcement model. A profile-shaped request can be downgraded or rejected by constraints, but the server should keep the user's elevated-access intent for project trust decisions. Turn-level profile overrides also need to retain existing read protections, including deny-read entries and bounded glob-scan metadata, so a permission override cannot accidentally drop configured protections such as `**/*.env = deny`. ## What changed - Adds optional `permissionProfile` request fields to `thread/start`, `thread/resume`, `thread/fork`, and `turn/start`. - Rejects ambiguous requests that specify both `permissionProfile` and the legacy `sandbox`/`sandboxPolicy` fields, including running-thread resume requests. - Converts profile-shaped overrides into core runtime filesystem/network permissions while continuing to derive the constrained legacy sandbox projection used by existing execution paths. - Preserves project-trust intent for profile overrides that are equivalent to workspace-write or full-access sandbox requests. - Preserves existing deny-read entries and `globScanMaxDepth` when applying turn-level `permissionProfile` overrides. - Updates app-server docs plus generated JSON/TypeScript schema fixtures and regression coverage. ## Verification - `cargo test -p codex-app-server-protocol schema_fixtures` - `cargo test -p codex-core session_configuration_apply_permission_profile_preserves_existing_deny_read_entries` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18279). * #18288 * #18287 * #18286 * #18285 * #18284 * #18283 * #18282 * #18281 * #18280 * __->__ #18279
This commit is contained in:
committed by
GitHub
Unverified
parent
ed4def8286
commit
18a26d7bbc
@@ -162,6 +162,7 @@ pub(super) async fn user_input_or_turn_inner(
|
||||
approval_policy: Some(approval_policy),
|
||||
approvals_reviewer,
|
||||
sandbox_policy: Some(sandbox_policy),
|
||||
permission_profile: None,
|
||||
windows_sandbox_level: None,
|
||||
collaboration_mode,
|
||||
reasoning_summary: summary,
|
||||
@@ -175,6 +176,56 @@ pub(super) async fn user_input_or_turn_inner(
|
||||
environments,
|
||||
)
|
||||
}
|
||||
Op::UserInputWithTurnContext {
|
||||
cwd,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox_policy,
|
||||
permission_profile,
|
||||
windows_sandbox_level,
|
||||
model,
|
||||
effort,
|
||||
summary,
|
||||
service_tier,
|
||||
final_output_json_schema,
|
||||
items,
|
||||
responsesapi_client_metadata,
|
||||
collaboration_mode,
|
||||
personality,
|
||||
environments,
|
||||
} => {
|
||||
let collaboration_mode = if let Some(collab_mode) = collaboration_mode {
|
||||
Some(collab_mode)
|
||||
} else {
|
||||
let state = sess.state.lock().await;
|
||||
Some(
|
||||
state
|
||||
.session_configuration
|
||||
.collaboration_mode
|
||||
.with_updates(model, effort, /*developer_instructions*/ None),
|
||||
)
|
||||
};
|
||||
(
|
||||
items,
|
||||
SessionSettingsUpdate {
|
||||
cwd,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox_policy,
|
||||
permission_profile,
|
||||
windows_sandbox_level,
|
||||
collaboration_mode,
|
||||
reasoning_summary: summary,
|
||||
service_tier,
|
||||
final_output_json_schema: Some(final_output_json_schema),
|
||||
personality,
|
||||
app_server_client_name: None,
|
||||
app_server_client_version: None,
|
||||
},
|
||||
responsesapi_client_metadata,
|
||||
environments,
|
||||
)
|
||||
}
|
||||
Op::UserInput {
|
||||
items,
|
||||
environments,
|
||||
@@ -1062,6 +1113,7 @@ pub(super) async fn submission_loop(
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox_policy,
|
||||
permission_profile,
|
||||
windows_sandbox_level,
|
||||
model,
|
||||
effort,
|
||||
@@ -1088,6 +1140,7 @@ pub(super) async fn submission_loop(
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox_policy,
|
||||
permission_profile,
|
||||
windows_sandbox_level,
|
||||
collaboration_mode: Some(collaboration_mode),
|
||||
reasoning_summary: summary,
|
||||
@@ -1099,7 +1152,9 @@ pub(super) async fn submission_loop(
|
||||
.await;
|
||||
false
|
||||
}
|
||||
Op::UserInput { .. } | Op::UserTurn { .. } => {
|
||||
Op::UserInput { .. }
|
||||
| Op::UserInputWithTurnContext { .. }
|
||||
| Op::UserTurn { .. } => {
|
||||
user_input_or_turn(&sess, sub.id.clone(), sub.op).await;
|
||||
false
|
||||
}
|
||||
|
||||
@@ -189,7 +189,7 @@ use self::review::spawn_review_thread;
|
||||
use self::session::AppServerClientMetadata;
|
||||
use self::session::Session;
|
||||
use self::session::SessionConfiguration;
|
||||
use self::session::SessionSettingsUpdate;
|
||||
pub(crate) use self::session::SessionSettingsUpdate;
|
||||
#[cfg(test)]
|
||||
use self::turn::AssistantMessageStreamParsers;
|
||||
#[cfg(test)]
|
||||
@@ -1308,6 +1308,14 @@ impl Session {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) async fn validate_settings(
|
||||
&self,
|
||||
updates: &SessionSettingsUpdate,
|
||||
) -> ConstraintResult<()> {
|
||||
let state = self.state.lock().await;
|
||||
state.session_configuration.apply(updates).map(|_| ())
|
||||
}
|
||||
|
||||
pub(crate) async fn set_session_startup_prewarm(
|
||||
&self,
|
||||
startup_prewarm: SessionStartupPrewarmHandle,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::config::ConstraintError;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
/// Context for an initialized model agent
|
||||
@@ -139,13 +140,6 @@ impl SessionConfiguration {
|
||||
if let Some(approvals_reviewer) = updates.approvals_reviewer {
|
||||
next_configuration.approvals_reviewer = approvals_reviewer;
|
||||
}
|
||||
let mut sandbox_policy_changed = false;
|
||||
if let Some(sandbox_policy) = updates.sandbox_policy.clone() {
|
||||
next_configuration.sandbox_policy.set(sandbox_policy)?;
|
||||
next_configuration.network_sandbox_policy =
|
||||
NetworkSandboxPolicy::from(next_configuration.sandbox_policy.get());
|
||||
sandbox_policy_changed = true;
|
||||
}
|
||||
if let Some(windows_sandbox_level) = updates.windows_sandbox_level {
|
||||
next_configuration.windows_sandbox_level = windows_sandbox_level;
|
||||
}
|
||||
@@ -166,13 +160,53 @@ impl SessionConfiguration {
|
||||
|
||||
let cwd_changed = absolute_cwd.as_path() != self.cwd.as_path();
|
||||
next_configuration.cwd = absolute_cwd;
|
||||
if sandbox_policy_changed {
|
||||
|
||||
if let Some(permission_profile) = updates.permission_profile.clone() {
|
||||
let sandbox_policy = permission_profile
|
||||
.to_legacy_sandbox_policy(&next_configuration.cwd)
|
||||
.map_err(|err| ConstraintError::InvalidValue {
|
||||
field_name: "permission_profile",
|
||||
candidate: format!("{permission_profile:?}"),
|
||||
allowed: format!(
|
||||
"permission profiles that can be represented by the active sandbox constraints: {err}"
|
||||
),
|
||||
requirement_source: codex_config::RequirementSource::Unknown,
|
||||
})?;
|
||||
next_configuration.sandbox_policy.set(sandbox_policy)?;
|
||||
let (mut file_system_sandbox_policy, network_sandbox_policy) =
|
||||
permission_profile.to_runtime_permissions();
|
||||
if file_system_sandbox_policy.glob_scan_max_depth.is_none() {
|
||||
file_system_sandbox_policy.glob_scan_max_depth =
|
||||
self.file_system_sandbox_policy.glob_scan_max_depth;
|
||||
}
|
||||
for deny_entry in self
|
||||
.file_system_sandbox_policy
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|entry| {
|
||||
entry.access == codex_protocol::permissions::FileSystemAccessMode::None
|
||||
})
|
||||
{
|
||||
if !file_system_sandbox_policy
|
||||
.entries
|
||||
.iter()
|
||||
.any(|entry| entry == deny_entry)
|
||||
{
|
||||
file_system_sandbox_policy.entries.push(deny_entry.clone());
|
||||
}
|
||||
}
|
||||
next_configuration.file_system_sandbox_policy = file_system_sandbox_policy;
|
||||
next_configuration.network_sandbox_policy = network_sandbox_policy;
|
||||
} else if let Some(sandbox_policy) = updates.sandbox_policy.clone() {
|
||||
next_configuration.sandbox_policy.set(sandbox_policy)?;
|
||||
next_configuration.file_system_sandbox_policy =
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy_preserving_deny_entries(
|
||||
next_configuration.sandbox_policy.get(),
|
||||
&next_configuration.cwd,
|
||||
&self.file_system_sandbox_policy,
|
||||
);
|
||||
next_configuration.network_sandbox_policy =
|
||||
NetworkSandboxPolicy::from(next_configuration.sandbox_policy.get());
|
||||
} else if cwd_changed && file_system_policy_matches_legacy {
|
||||
// Preserve richer split policies across cwd-only updates; only
|
||||
// rederive when the session is already using the legacy bridge.
|
||||
@@ -198,6 +232,7 @@ pub(crate) struct SessionSettingsUpdate {
|
||||
pub(crate) approval_policy: Option<AskForApproval>,
|
||||
pub(crate) approvals_reviewer: Option<ApprovalsReviewer>,
|
||||
pub(crate) sandbox_policy: Option<SandboxPolicy>,
|
||||
pub(crate) permission_profile: Option<PermissionProfile>,
|
||||
pub(crate) windows_sandbox_level: Option<WindowsSandboxLevel>,
|
||||
pub(crate) collaboration_mode: Option<CollaborationMode>,
|
||||
pub(crate) reasoning_summary: Option<ReasoningSummaryConfig>,
|
||||
|
||||
@@ -108,6 +108,7 @@ use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
use codex_protocol::protocol::W3cTraceContext;
|
||||
use core_test_support::PathBufExt;
|
||||
use core_test_support::PathExt;
|
||||
use core_test_support::context_snapshot;
|
||||
use core_test_support::context_snapshot::ContextSnapshotOptions;
|
||||
use core_test_support::context_snapshot::ContextSnapshotRenderMode;
|
||||
@@ -1545,6 +1546,7 @@ async fn fork_startup_context_then_first_turn_diff_snapshot() -> anyhow::Result<
|
||||
approval_policy: Some(AskForApproval::Never),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
windows_sandbox_level: None,
|
||||
model: None,
|
||||
effort: None,
|
||||
@@ -2714,6 +2716,53 @@ async fn session_configuration_apply_preserves_split_file_system_policy_on_cwd_o
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn session_configuration_apply_permission_profile_preserves_existing_deny_read_entries() {
|
||||
let mut session_configuration = make_session_configuration_for_tests().await;
|
||||
let cwd = tempfile::tempdir().expect("create temp dir");
|
||||
session_configuration.cwd = cwd.path().abs();
|
||||
|
||||
let workspace_policy = SandboxPolicy::new_workspace_write_policy();
|
||||
session_configuration.sandbox_policy =
|
||||
codex_config::Constrained::allow_any(workspace_policy.clone());
|
||||
let deny_entry = FileSystemSandboxEntry {
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: "**/*.env".to_string(),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
};
|
||||
let mut existing_file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy(
|
||||
&workspace_policy,
|
||||
session_configuration.cwd.as_path(),
|
||||
);
|
||||
existing_file_system_policy.glob_scan_max_depth = Some(2);
|
||||
existing_file_system_policy.entries.push(deny_entry.clone());
|
||||
session_configuration.file_system_sandbox_policy = existing_file_system_policy;
|
||||
|
||||
let requested_file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy(
|
||||
&workspace_policy,
|
||||
session_configuration.cwd.as_path(),
|
||||
);
|
||||
let permission_profile = codex_protocol::models::PermissionProfile::from_runtime_permissions(
|
||||
&requested_file_system_policy,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
);
|
||||
let updated = session_configuration
|
||||
.apply(&SessionSettingsUpdate {
|
||||
permission_profile: Some(permission_profile),
|
||||
..Default::default()
|
||||
})
|
||||
.expect("permission profile update should succeed");
|
||||
|
||||
let mut expected_file_system_policy = requested_file_system_policy;
|
||||
expected_file_system_policy.glob_scan_max_depth = Some(2);
|
||||
expected_file_system_policy.entries.push(deny_entry);
|
||||
assert_eq!(
|
||||
updated.file_system_sandbox_policy,
|
||||
expected_file_system_policy
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg_attr(windows, ignore)]
|
||||
#[tokio::test]
|
||||
async fn new_default_turn_uses_config_aware_skills_for_role_overrides() {
|
||||
@@ -3715,6 +3764,7 @@ fn op_kind_distinguishes_turn_ops() {
|
||||
approval_policy: None,
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
windows_sandbox_level: None,
|
||||
model: None,
|
||||
effort: None,
|
||||
@@ -3736,6 +3786,28 @@ fn op_kind_distinguishes_turn_ops() {
|
||||
.kind(),
|
||||
"user_input"
|
||||
);
|
||||
assert_eq!(
|
||||
Op::UserInputWithTurnContext {
|
||||
environments: None,
|
||||
items: vec![],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
cwd: None,
|
||||
approval_policy: None,
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: None,
|
||||
windows_sandbox_level: None,
|
||||
model: None,
|
||||
effort: None,
|
||||
summary: None,
|
||||
service_tier: None,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
}
|
||||
.kind(),
|
||||
"user_input_with_turn_context"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user