Skip to content

Commit 2d01c68

Browse files
committed
permissions: make SessionConfigured profile-only
1 parent bf530bf commit 2d01c68

17 files changed

Lines changed: 192 additions & 161 deletions

File tree

codex-rs/app-server/src/codex_message_processor.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4235,9 +4235,9 @@ impl CodexMessageProcessor {
42354235
thread_status,
42364236
/*has_live_in_progress_turn*/ false,
42374237
);
4238-
let permission_profile = thread_response_permission_profile(
4239-
codex_thread.config_snapshot().await.permission_profile,
4240-
);
4238+
let config_snapshot = codex_thread.config_snapshot().await;
4239+
let permission_profile =
4240+
thread_response_permission_profile(config_snapshot.permission_profile.clone());
42414241

42424242
let response = ThreadResumeResponse {
42434243
thread,
@@ -4248,7 +4248,7 @@ impl CodexMessageProcessor {
42484248
instruction_sources,
42494249
approval_policy: session_configured.approval_policy.into(),
42504250
approvals_reviewer: session_configured.approvals_reviewer.into(),
4251-
sandbox: session_configured.sandbox_policy.into(),
4251+
sandbox: config_snapshot.sandbox_policy.into(),
42524252
permission_profile,
42534253
reasoning_effort: session_configured.reasoning_effort,
42544254
};
@@ -4831,9 +4831,9 @@ impl CodexMessageProcessor {
48314831
.await,
48324832
/*has_in_progress_turn*/ false,
48334833
);
4834-
let permission_profile = thread_response_permission_profile(
4835-
forked_thread.config_snapshot().await.permission_profile,
4836-
);
4834+
let config_snapshot = forked_thread.config_snapshot().await;
4835+
let permission_profile =
4836+
thread_response_permission_profile(config_snapshot.permission_profile.clone());
48374837

48384838
let response = ThreadForkResponse {
48394839
thread: thread.clone(),
@@ -4844,7 +4844,7 @@ impl CodexMessageProcessor {
48444844
instruction_sources,
48454845
approval_policy: session_configured.approval_policy.into(),
48464846
approvals_reviewer: session_configured.approvals_reviewer.into(),
4847-
sandbox: session_configured.sandbox_policy.into(),
4847+
sandbox: config_snapshot.sandbox_policy.into(),
48484848
permission_profile,
48494849
reasoning_effort: session_configured.reasoning_effort,
48504850
};

codex-rs/core/src/session/session.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -867,7 +867,6 @@ impl Session {
867867
// Dispatch the SessionConfiguredEvent first and then report any errors.
868868
// If resuming, include converted initial messages in the payload so UIs can render them immediately.
869869
let initial_messages = initial_history.get_event_msgs();
870-
let session_sandbox_policy = session_configuration.sandbox_policy();
871870
let events = std::iter::once(Event {
872871
id: INITIAL_SUBMIT_ID.to_owned(),
873872
msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
@@ -879,8 +878,7 @@ impl Session {
879878
service_tier: session_configuration.service_tier,
880879
approval_policy: session_configuration.approval_policy.value(),
881880
approvals_reviewer: session_configuration.approvals_reviewer,
882-
sandbox_policy: session_sandbox_policy.clone(),
883-
permission_profile: Some(session_configuration.permission_profile()),
881+
permission_profile: session_configuration.permission_profile(),
884882
cwd: session_configuration.cwd.clone(),
885883
reasoning_effort: session_configuration.collaboration_mode.reasoning_effort(),
886884
history_log_id,

codex-rs/core/src/session/tests.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1527,17 +1527,12 @@ async fn session_configured_reports_permission_profile_for_external_sandbox() ->
15271527

15281528
let test = builder.build(&server).await?;
15291529

1530-
assert_eq!(
1531-
test.session_configured.sandbox_policy,
1532-
expected_sandbox_policy
1533-
);
15341530
let expected_permission_profile =
15351531
codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy(
15361532
&expected_sandbox_policy,
15371533
);
15381534
assert_eq!(
1539-
test.session_configured.permission_profile,
1540-
Some(expected_permission_profile),
1535+
test.session_configured.permission_profile, expected_permission_profile,
15411536
"ExternalSandbox is represented explicitly instead of as a lossy root-write profile"
15421537
);
15431538
Ok(())

codex-rs/exec/src/lib.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,8 +1046,9 @@ fn session_configured_from_thread_response(
10461046
service_tier,
10471047
approval_policy,
10481048
approvals_reviewer,
1049-
sandbox_policy,
1050-
permission_profile,
1049+
permission_profile: permission_profile.unwrap_or_else(|| {
1050+
PermissionProfile::from_legacy_sandbox_policy_for_cwd(&sandbox_policy, cwd.as_path())
1051+
}),
10511052
cwd,
10521053
reasoning_effort,
10531054
history_log_id: 0,

codex-rs/exec/tests/event_processor_with_json_output.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ use codex_app_server_protocol::TurnStartedNotification;
2828
use codex_app_server_protocol::TurnStatus;
2929
use codex_app_server_protocol::WebSearchAction as ApiWebSearchAction;
3030
use codex_protocol::ThreadId;
31+
use codex_protocol::models::PermissionProfile;
3132
use codex_protocol::models::WebSearchAction;
3233
use codex_protocol::protocol::AskForApproval;
33-
use codex_protocol::protocol::SandboxPolicy;
3434
use codex_protocol::protocol::SessionConfiguredEvent;
3535
use codex_utils_absolute_path::test_support::PathBufExt;
3636
use codex_utils_absolute_path::test_support::test_path_buf;
@@ -114,8 +114,7 @@ fn session_configured_produces_thread_started_event() {
114114
service_tier: None,
115115
approval_policy: AskForApproval::Never,
116116
approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer::User,
117-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
118-
permission_profile: None,
117+
permission_profile: PermissionProfile::read_only(),
119118
cwd: test_path_buf("/tmp/project").abs(),
120119
reasoning_effort: None,
121120
history_log_id: 0,

codex-rs/mcp-server/src/outgoing_message.rs

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -231,10 +231,10 @@ mod tests {
231231

232232
use anyhow::Result;
233233
use codex_protocol::ThreadId;
234+
use codex_protocol::models::PermissionProfile;
234235
use codex_protocol::openai_models::ReasoningEffort;
235236
use codex_protocol::protocol::AskForApproval;
236237
use codex_protocol::protocol::EventMsg;
237-
use codex_protocol::protocol::SandboxPolicy;
238238
use codex_protocol::protocol::SessionConfiguredEvent;
239239
use codex_utils_absolute_path::test_support::PathBufExt;
240240
use codex_utils_absolute_path::test_support::test_path_buf;
@@ -304,8 +304,7 @@ mod tests {
304304
service_tier: None,
305305
approval_policy: AskForApproval::Never,
306306
approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer::User,
307-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
308-
permission_profile: None,
307+
permission_profile: PermissionProfile::read_only(),
309308
cwd: test_path_buf("/home/user/project").abs(),
310309
reasoning_effort: Some(ReasoningEffort::default()),
311310
history_log_id: 1,
@@ -349,8 +348,7 @@ mod tests {
349348
service_tier: None,
350349
approval_policy: AskForApproval::Never,
351350
approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer::User,
352-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
353-
permission_profile: None,
351+
permission_profile: PermissionProfile::read_only(),
354352
cwd: test_path_buf("/home/user/project").abs(),
355353
reasoning_effort: Some(ReasoningEffort::default()),
356354
history_log_id: 1,
@@ -389,9 +387,7 @@ mod tests {
389387
"model_provider_id": "test-provider",
390388
"approval_policy": "never",
391389
"approvals_reviewer": "user",
392-
"sandbox_policy": {
393-
"type": "read-only"
394-
},
390+
"permission_profile": session_configured_event.permission_profile,
395391
"cwd": test_path_buf("/home/user/project"),
396392
"reasoning_effort": session_configured_event.reasoning_effort,
397393
"history_log_id": session_configured_event.history_log_id,
@@ -419,8 +415,7 @@ mod tests {
419415
service_tier: None,
420416
approval_policy: AskForApproval::Never,
421417
approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer::User,
422-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
423-
permission_profile: None,
418+
permission_profile: PermissionProfile::read_only(),
424419
cwd: test_path_buf("/home/user/project").abs(),
425420
reasoning_effort: Some(ReasoningEffort::default()),
426421
history_log_id: 1,
@@ -460,9 +455,7 @@ mod tests {
460455
"model_provider_id": "test-provider",
461456
"approval_policy": "never",
462457
"approvals_reviewer": "user",
463-
"sandbox_policy": {
464-
"type": "read-only"
465-
},
458+
"permission_profile": session_configured_event.permission_profile,
466459
"cwd": test_path_buf("/home/user/project"),
467460
"reasoning_effort": session_configured_event.reasoning_effort,
468461
"history_log_id": session_configured_event.history_log_id,

codex-rs/protocol/src/protocol.rs

Lines changed: 88 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3541,7 +3541,7 @@ pub struct SessionNetworkProxyRuntime {
35413541
pub socks_addr: String,
35423542
}
35433543

3544-
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
3544+
#[derive(Debug, Clone, Serialize, JsonSchema, TS)]
35453545
pub struct SessionConfiguredEvent {
35463546
pub session_id: ThreadId,
35473547
#[serde(skip_serializing_if = "Option::is_none")]
@@ -3569,16 +3569,8 @@ pub struct SessionConfiguredEvent {
35693569
#[serde(default)]
35703570
pub approvals_reviewer: ApprovalsReviewer,
35713571

3572-
/// Legacy sandbox projection for commands executed in the system.
3573-
///
3574-
/// Consumers should prefer `permission_profile` when it is present. This
3575-
/// field remains available as a compatibility fallback for older emitters
3576-
/// and sessions that only expose legacy sandbox state.
3577-
pub sandbox_policy: SandboxPolicy,
3578-
35793572
/// Canonical effective permissions for commands executed in the session.
3580-
#[serde(default, skip_serializing_if = "Option::is_none")]
3581-
pub permission_profile: Option<PermissionProfile>,
3573+
pub permission_profile: PermissionProfile,
35823574

35833575
/// Working directory that should be treated as the *root* of the
35843576
/// session.
@@ -3609,6 +3601,67 @@ pub struct SessionConfiguredEvent {
36093601
pub rollout_path: Option<PathBuf>,
36103602
}
36113603

3604+
impl<'de> Deserialize<'de> for SessionConfiguredEvent {
3605+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
3606+
where
3607+
D: serde::Deserializer<'de>,
3608+
{
3609+
#[derive(Deserialize)]
3610+
struct Wire {
3611+
session_id: ThreadId,
3612+
forked_from_id: Option<ThreadId>,
3613+
#[serde(default)]
3614+
thread_name: Option<String>,
3615+
model: String,
3616+
model_provider_id: String,
3617+
service_tier: Option<ServiceTier>,
3618+
approval_policy: AskForApproval,
3619+
#[serde(default)]
3620+
approvals_reviewer: ApprovalsReviewer,
3621+
sandbox_policy: Option<SandboxPolicy>,
3622+
permission_profile: Option<PermissionProfile>,
3623+
cwd: AbsolutePathBuf,
3624+
reasoning_effort: Option<ReasoningEffortConfig>,
3625+
history_log_id: u64,
3626+
history_entry_count: usize,
3627+
initial_messages: Option<Vec<EventMsg>>,
3628+
network_proxy: Option<SessionNetworkProxyRuntime>,
3629+
rollout_path: Option<PathBuf>,
3630+
}
3631+
3632+
let wire = Wire::deserialize(deserializer)?;
3633+
let permission_profile = match (wire.permission_profile, wire.sandbox_policy) {
3634+
(Some(permission_profile), _) => permission_profile,
3635+
(None, Some(sandbox_policy)) => PermissionProfile::from_legacy_sandbox_policy_for_cwd(
3636+
&sandbox_policy,
3637+
wire.cwd.as_path(),
3638+
),
3639+
(None, None) => {
3640+
return Err(serde::de::Error::missing_field("permission_profile"));
3641+
}
3642+
};
3643+
3644+
Ok(Self {
3645+
session_id: wire.session_id,
3646+
forked_from_id: wire.forked_from_id,
3647+
thread_name: wire.thread_name,
3648+
model: wire.model,
3649+
model_provider_id: wire.model_provider_id,
3650+
service_tier: wire.service_tier,
3651+
approval_policy: wire.approval_policy,
3652+
approvals_reviewer: wire.approvals_reviewer,
3653+
permission_profile,
3654+
cwd: wire.cwd,
3655+
reasoning_effort: wire.reasoning_effort,
3656+
history_log_id: wire.history_log_id,
3657+
history_entry_count: wire.history_entry_count,
3658+
initial_messages: wire.initial_messages,
3659+
network_proxy: wire.network_proxy,
3660+
rollout_path: wire.rollout_path,
3661+
})
3662+
}
3663+
}
3664+
36123665
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
36133666
pub struct ThreadNameUpdatedEvent {
36143667
pub thread_id: ThreadId,
@@ -5088,6 +5141,7 @@ mod tests {
50885141
fn serialize_event() -> Result<()> {
50895142
let conversation_id = ThreadId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8")?;
50905143
let rollout_file = NamedTempFile::new()?;
5144+
let permission_profile = PermissionProfile::read_only();
50915145
let event = Event {
50925146
id: "1234".to_string(),
50935147
msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
@@ -5099,8 +5153,7 @@ mod tests {
50995153
service_tier: None,
51005154
approval_policy: AskForApproval::Never,
51015155
approvals_reviewer: ApprovalsReviewer::User,
5102-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
5103-
permission_profile: None,
5156+
permission_profile: permission_profile.clone(),
51045157
cwd: test_path_buf("/home/user/project").abs(),
51055158
reasoning_effort: Some(ReasoningEffortConfig::default()),
51065159
history_log_id: 0,
@@ -5120,9 +5173,7 @@ mod tests {
51205173
"model_provider_id": "openai",
51215174
"approval_policy": "never",
51225175
"approvals_reviewer": "user",
5123-
"sandbox_policy": {
5124-
"type": "read-only"
5125-
},
5176+
"permission_profile": permission_profile,
51265177
"cwd": test_path_buf("/home/user/project"),
51275178
"reasoning_effort": "medium",
51285179
"history_log_id": 0,
@@ -5134,6 +5185,28 @@ mod tests {
51345185
Ok(())
51355186
}
51365187

5188+
#[test]
5189+
fn deserialize_legacy_session_configured_event_uses_sandbox_policy() -> Result<()> {
5190+
let cwd = test_path_buf("/home/user/project");
5191+
let value = json!({
5192+
"session_id": "67e55044-10b1-426f-9247-bb680e5fe0c8",
5193+
"model": "codex-mini-latest",
5194+
"model_provider_id": "openai",
5195+
"approval_policy": "never",
5196+
"approvals_reviewer": "user",
5197+
"sandbox_policy": {
5198+
"type": "read-only"
5199+
},
5200+
"cwd": cwd,
5201+
"history_log_id": 0,
5202+
"history_entry_count": 0,
5203+
});
5204+
5205+
let event: SessionConfiguredEvent = serde_json::from_value(value)?;
5206+
assert_eq!(event.permission_profile, PermissionProfile::read_only());
5207+
Ok(())
5208+
}
5209+
51375210
#[test]
51385211
fn vec_u8_as_base64_serialization_and_deserialization() -> Result<()> {
51395212
let event = ExecCommandOutputDeltaEvent {

codex-rs/tui/src/app/config_persistence.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,7 @@ mod tests {
556556
use crate::app::test_support::app_enabled_in_effective_config;
557557
use crate::app::test_support::make_test_app;
558558
use crate::test_support::PathBufExt;
559+
use codex_protocol::models::PermissionProfile;
559560
use codex_protocol::protocol::Event;
560561
use codex_protocol::protocol::EventMsg;
561562
use codex_protocol::protocol::SessionConfiguredEvent;
@@ -653,8 +654,7 @@ mod tests {
653654
service_tier: None,
654655
approval_policy: AskForApproval::Never,
655656
approvals_reviewer: ApprovalsReviewer::User,
656-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
657-
permission_profile: None,
657+
permission_profile: PermissionProfile::read_only(),
658658
cwd: next_cwd.clone().abs(),
659659
reasoning_effort: None,
660660
history_log_id: 0,

0 commit comments

Comments
 (0)