Skip to content

Commit 23dc42c

Browse files
committed
permissions: make SessionConfigured profile-only
1 parent 6b5373d commit 23dc42c

15 files changed

Lines changed: 163 additions & 145 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
@@ -4674,9 +4674,9 @@ impl CodexMessageProcessor {
46744674
thread_status,
46754675
/*has_live_in_progress_turn*/ false,
46764676
);
4677-
let permission_profile = thread_response_permission_profile(
4678-
codex_thread.config_snapshot().await.permission_profile,
4679-
);
4677+
let config_snapshot = codex_thread.config_snapshot().await;
4678+
let permission_profile =
4679+
thread_response_permission_profile(config_snapshot.permission_profile.clone());
46804680

46814681
let response = ThreadResumeResponse {
46824682
thread,
@@ -4687,7 +4687,7 @@ impl CodexMessageProcessor {
46874687
instruction_sources,
46884688
approval_policy: session_configured.approval_policy.into(),
46894689
approvals_reviewer: session_configured.approvals_reviewer.into(),
4690-
sandbox: session_configured.sandbox_policy.into(),
4690+
sandbox: config_snapshot.sandbox_policy.into(),
46914691
permission_profile,
46924692
reasoning_effort: session_configured.reasoning_effort,
46934693
};
@@ -5405,9 +5405,9 @@ impl CodexMessageProcessor {
54055405
.await,
54065406
/*has_in_progress_turn*/ false,
54075407
);
5408-
let permission_profile = thread_response_permission_profile(
5409-
forked_thread.config_snapshot().await.permission_profile,
5410-
);
5408+
let config_snapshot = forked_thread.config_snapshot().await;
5409+
let permission_profile =
5410+
thread_response_permission_profile(config_snapshot.permission_profile.clone());
54115411

54125412
let response = ThreadForkResponse {
54135413
thread: thread.clone(),
@@ -5418,7 +5418,7 @@ impl CodexMessageProcessor {
54185418
instruction_sources,
54195419
approval_policy: session_configured.approval_policy.into(),
54205420
approvals_reviewer: session_configured.approvals_reviewer.into(),
5421-
sandbox: session_configured.sandbox_policy.into(),
5421+
sandbox: config_snapshot.sandbox_policy.into(),
54225422
permission_profile,
54235423
reasoning_effort: session_configured.reasoning_effort,
54245424
};

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
@@ -1043,8 +1043,9 @@ fn session_configured_from_thread_response(
10431043
service_tier,
10441044
approval_policy,
10451045
approvals_reviewer,
1046-
sandbox_policy,
1047-
permission_profile,
1046+
permission_profile: permission_profile.unwrap_or_else(|| {
1047+
PermissionProfile::from_legacy_sandbox_policy_for_cwd(&sandbox_policy, cwd.as_path())
1048+
}),
10481049
cwd,
10491050
reasoning_effort,
10501051
history_log_id: 0,

codex-rs/protocol/src/protocol.rs

Lines changed: 88 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3538,7 +3538,7 @@ pub struct SessionNetworkProxyRuntime {
35383538
pub socks_addr: String,
35393539
}
35403540

3541-
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
3541+
#[derive(Debug, Clone, Serialize, JsonSchema, TS)]
35423542
pub struct SessionConfiguredEvent {
35433543
pub session_id: ThreadId,
35443544
#[serde(skip_serializing_if = "Option::is_none")]
@@ -3566,16 +3566,8 @@ pub struct SessionConfiguredEvent {
35663566
#[serde(default)]
35673567
pub approvals_reviewer: ApprovalsReviewer,
35683568

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

35803572
/// Working directory that should be treated as the *root* of the
35813573
/// session.
@@ -3606,6 +3598,67 @@ pub struct SessionConfiguredEvent {
36063598
pub rollout_path: Option<PathBuf>,
36073599
}
36083600

3601+
impl<'de> Deserialize<'de> for SessionConfiguredEvent {
3602+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
3603+
where
3604+
D: serde::Deserializer<'de>,
3605+
{
3606+
#[derive(Deserialize)]
3607+
struct Wire {
3608+
session_id: ThreadId,
3609+
forked_from_id: Option<ThreadId>,
3610+
#[serde(default)]
3611+
thread_name: Option<String>,
3612+
model: String,
3613+
model_provider_id: String,
3614+
service_tier: Option<ServiceTier>,
3615+
approval_policy: AskForApproval,
3616+
#[serde(default)]
3617+
approvals_reviewer: ApprovalsReviewer,
3618+
sandbox_policy: Option<SandboxPolicy>,
3619+
permission_profile: Option<PermissionProfile>,
3620+
cwd: AbsolutePathBuf,
3621+
reasoning_effort: Option<ReasoningEffortConfig>,
3622+
history_log_id: u64,
3623+
history_entry_count: usize,
3624+
initial_messages: Option<Vec<EventMsg>>,
3625+
network_proxy: Option<SessionNetworkProxyRuntime>,
3626+
rollout_path: Option<PathBuf>,
3627+
}
3628+
3629+
let wire = Wire::deserialize(deserializer)?;
3630+
let permission_profile = match (wire.permission_profile, wire.sandbox_policy) {
3631+
(Some(permission_profile), _) => permission_profile,
3632+
(None, Some(sandbox_policy)) => PermissionProfile::from_legacy_sandbox_policy_for_cwd(
3633+
&sandbox_policy,
3634+
wire.cwd.as_path(),
3635+
),
3636+
(None, None) => {
3637+
return Err(serde::de::Error::missing_field("permission_profile"));
3638+
}
3639+
};
3640+
3641+
Ok(Self {
3642+
session_id: wire.session_id,
3643+
forked_from_id: wire.forked_from_id,
3644+
thread_name: wire.thread_name,
3645+
model: wire.model,
3646+
model_provider_id: wire.model_provider_id,
3647+
service_tier: wire.service_tier,
3648+
approval_policy: wire.approval_policy,
3649+
approvals_reviewer: wire.approvals_reviewer,
3650+
permission_profile,
3651+
cwd: wire.cwd,
3652+
reasoning_effort: wire.reasoning_effort,
3653+
history_log_id: wire.history_log_id,
3654+
history_entry_count: wire.history_entry_count,
3655+
initial_messages: wire.initial_messages,
3656+
network_proxy: wire.network_proxy,
3657+
rollout_path: wire.rollout_path,
3658+
})
3659+
}
3660+
}
3661+
36093662
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
36103663
pub struct ThreadNameUpdatedEvent {
36113664
pub thread_id: ThreadId,
@@ -5085,6 +5138,7 @@ mod tests {
50855138
fn serialize_event() -> Result<()> {
50865139
let conversation_id = ThreadId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8")?;
50875140
let rollout_file = NamedTempFile::new()?;
5141+
let permission_profile = PermissionProfile::read_only();
50885142
let event = Event {
50895143
id: "1234".to_string(),
50905144
msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
@@ -5096,8 +5150,7 @@ mod tests {
50965150
service_tier: None,
50975151
approval_policy: AskForApproval::Never,
50985152
approvals_reviewer: ApprovalsReviewer::User,
5099-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
5100-
permission_profile: None,
5153+
permission_profile: permission_profile.clone(),
51015154
cwd: test_path_buf("/home/user/project").abs(),
51025155
reasoning_effort: Some(ReasoningEffortConfig::default()),
51035156
history_log_id: 0,
@@ -5117,9 +5170,7 @@ mod tests {
51175170
"model_provider_id": "openai",
51185171
"approval_policy": "never",
51195172
"approvals_reviewer": "user",
5120-
"sandbox_policy": {
5121-
"type": "read-only"
5122-
},
5173+
"permission_profile": permission_profile,
51235174
"cwd": test_path_buf("/home/user/project"),
51245175
"reasoning_effort": "medium",
51255176
"history_log_id": 0,
@@ -5131,6 +5182,28 @@ mod tests {
51315182
Ok(())
51325183
}
51335184

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

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3495,8 +3495,7 @@ async fn render_clear_ui_header_after_long_transcript_for_snapshot() -> String {
34953495
service_tier: None,
34963496
approval_policy: AskForApproval::Never,
34973497
approvals_reviewer: ApprovalsReviewer::User,
3498-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
3499-
permission_profile: None,
3498+
permission_profile: PermissionProfile::read_only(),
35003499
cwd: test_path_buf("/tmp/project").abs(),
35013500
reasoning_effort: Some(ReasoningEffortConfig::High),
35023501
history_log_id: 0,
@@ -4246,8 +4245,7 @@ async fn backtrack_selection_with_duplicate_history_targets_unique_turn() {
42464245
service_tier: None,
42474246
approval_policy: AskForApproval::Never,
42484247
approvals_reviewer: ApprovalsReviewer::User,
4249-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
4250-
permission_profile: None,
4248+
permission_profile: PermissionProfile::read_only(),
42514249
cwd: test_path_buf("/home/user/project").abs(),
42524250
reasoning_effort: None,
42534251
history_log_id: 0,
@@ -4310,8 +4308,7 @@ async fn backtrack_selection_with_duplicate_history_targets_unique_turn() {
43104308
service_tier: None,
43114309
approval_policy: AskForApproval::Never,
43124310
approvals_reviewer: ApprovalsReviewer::User,
4313-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
4314-
permission_profile: None,
4311+
permission_profile: PermissionProfile::read_only(),
43154312
cwd: test_path_buf("/home/user/project").abs(),
43164313
reasoning_effort: None,
43174314
history_log_id: 0,
@@ -4404,8 +4401,7 @@ async fn backtrack_resubmit_preserves_data_image_urls_in_user_turn() {
44044401
service_tier: None,
44054402
approval_policy: AskForApproval::Never,
44064403
approvals_reviewer: ApprovalsReviewer::User,
4407-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
4408-
permission_profile: None,
4404+
permission_profile: PermissionProfile::read_only(),
44094405
cwd: test_path_buf("/home/user/project").abs(),
44104406
reasoning_effort: None,
44114407
history_log_id: 0,
@@ -4788,8 +4784,7 @@ async fn new_session_requests_shutdown_for_previous_conversation() {
47884784
service_tier: None,
47894785
approval_policy: AskForApproval::Never,
47904786
approvals_reviewer: ApprovalsReviewer::User,
4791-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
4792-
permission_profile: None,
4787+
permission_profile: PermissionProfile::read_only(),
47934788
cwd: test_path_buf("/home/user/project").abs(),
47944789
reasoning_effort: None,
47954790
history_log_id: 0,
@@ -4901,8 +4896,7 @@ async fn clear_only_ui_reset_preserves_chat_session_state() {
49014896
service_tier: None,
49024897
approval_policy: AskForApproval::Never,
49034898
approvals_reviewer: ApprovalsReviewer::User,
4904-
sandbox_policy: SandboxPolicy::new_read_only_policy(),
4905-
permission_profile: None,
4899+
permission_profile: PermissionProfile::read_only(),
49064900
cwd: test_path_buf("/tmp/project").abs(),
49074901
reasoning_effort: None,
49084902
history_log_id: 0,

codex-rs/tui/src/chatwidget.rs

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,8 @@ use codex_protocol::items::AgentMessageContent;
141141
use codex_protocol::items::AgentMessageItem;
142142
use codex_protocol::items::UserMessageItem;
143143
use codex_protocol::models::MessagePhase;
144-
use codex_protocol::models::PermissionProfile;
145-
use codex_protocol::models::SandboxEnforcement;
146144
use codex_protocol::models::local_image_label_text;
147145
use codex_protocol::parse_command::ParsedCommand;
148-
use codex_protocol::permissions::FileSystemSandboxPolicy;
149-
use codex_protocol::permissions::NetworkSandboxPolicy;
150146
use codex_protocol::plan_tool::PlanItemArg as UpdatePlanItemArg;
151147
use codex_protocol::plan_tool::StepStatus as UpdatePlanItemStatus;
152148
#[cfg(test)]
@@ -1617,8 +1613,7 @@ fn thread_session_state_to_legacy_event(
16171613
service_tier: session.service_tier,
16181614
approval_policy: session.approval_policy,
16191615
approvals_reviewer: session.approvals_reviewer,
1620-
sandbox_policy: session.sandbox_policy,
1621-
permission_profile: Some(session.permission_profile),
1616+
permission_profile: session.permission_profile,
16221617
cwd: session.cwd,
16231618
reasoning_effort: session.reasoning_effort,
16241619
history_log_id: session.history_log_id,
@@ -2376,32 +2371,14 @@ impl ChatWidget {
23762371
self.config.permissions.approval_policy =
23772372
Constrained::allow_only(event.approval_policy);
23782373
}
2379-
let permission_sync = match event.permission_profile.clone() {
2380-
Some(permission_profile) => self
2381-
.config
2382-
.permissions
2383-
.set_permission_profile(permission_profile),
2384-
None => self
2385-
.config
2386-
.permissions
2387-
.set_legacy_sandbox_policy(event.sandbox_policy.clone(), event.cwd.as_path()),
2388-
};
2374+
let permission_sync = self
2375+
.config
2376+
.permissions
2377+
.set_permission_profile(event.permission_profile.clone());
23892378
if let Err(err) = permission_sync {
23902379
tracing::warn!(%err, "failed to sync permissions from SessionConfigured");
2391-
let permission_profile = event.permission_profile.clone().unwrap_or_else(|| {
2392-
let file_system_sandbox_policy =
2393-
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(
2394-
&event.sandbox_policy,
2395-
event.cwd.as_path(),
2396-
);
2397-
PermissionProfile::from_runtime_permissions_with_enforcement(
2398-
SandboxEnforcement::from_legacy_sandbox_policy(&event.sandbox_policy),
2399-
&file_system_sandbox_policy,
2400-
NetworkSandboxPolicy::from(&event.sandbox_policy),
2401-
)
2402-
});
24032380
self.config.permissions.permission_profile =
2404-
Constrained::allow_only(permission_profile);
2381+
Constrained::allow_only(event.permission_profile.clone());
24052382
}
24062383
self.config.approvals_reviewer = event.approvals_reviewer;
24072384
self.status_line_project_root_name_cache = None;

0 commit comments

Comments
 (0)