agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Key request-permission grants by environment (#25850)

Browse Source 
## Stack

1. This PR (#25850) - Key request-permission grants by environment:
stores and applies sticky permission grants per environment id.
2. #25858 - Add `environmentId` to `request_permissions`: lets the model
target a selected environment and resolves relative permission paths
against it.
3. #25862 - Propagate permission approval environment id: carries the
selected environment id through approval events, app-server requests,
TUI prompts, and delegate forwarding.
4. #25867 - Add remote request permissions integration coverage:
verifies the selected remote environment across request, approval, grant
reuse, and exec.

#25858, #25862, and #25867 are stacked on this PR and should be reviewed
after it.

## Why

Multi-environment CCA turns can attach both local and remote executors,
but request-permission grants were still effectively cwd-only. Pending
permission requests tracked a cwd, while stored turn/session grants had
no environment identity, so sticky grants could be reused through the
wrong executor context.

This makes the first permission-grant step environment-aware without
changing the external `request_permissions` payload shape: omitted
environment targeting remains bound to the primary turn environment.

## What Changed

- Store turn- and session-scoped request-permission grants by
`environment_id`.
- Keep the selected `TurnEnvironmentSelection` with pending
`request_permissions` calls so approval responses normalize and record
grants against the same environment.
- Resolve relative `request_permissions` file paths against the primary
turn environment cwd instead of deprecated `turn.cwd`.
- Apply sticky grants in `shell`, `exec_command`, and `apply_patch` by
selected environment id while still using the actual tool cwd for
cwd-relative permission materialization.
- Update Guardian and request-permissions coverage for the
environment-keyed grant behavior.

## Testing

Not run locally. Added or updated focused coverage for:

- `request_permission_grants_are_environment_keyed`
-
`request_permissions_tool_resolves_relative_paths_against_primary_environment`
- related Guardian/request-permissions sticky grant tests
This commit is contained in:
jif
2026-06-02 20:16:57 +02:00
committed by GitHub
parent 9e3d5f29e2
commit 503ec190a8
10 changed files with 356 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/session/mod.rs
+60 -15
View File
@@ -2107,12 +2107,18 @@ impl Session {
args: RequestPermissionsArgs,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
self.request_permissions_for_cwd(
let Some(turn_environment) = turn_context.environments.primary() else {
return Some(RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
});
};
self.request_permissions_for_environment(
turn_context,
call_id,
args,
#[allow(deprecated)]
turn_context.cwd.clone(),
turn_environment.selection(),
cancellation_token,
)
.await
@@ -2122,12 +2128,12 @@ impl Session {
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub(crate) async fn request_permissions_for_cwd(
pub(crate) async fn request_permissions_for_environment(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cwd: AbsolutePathBuf,
environment: TurnEnvironmentSelection,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
match turn_context.as_ref().approval_policy.value() {
@@ -2221,10 +2227,11 @@ impl Session {
let response = Self::normalize_request_permissions_response(
requested_permissions,
response,
cwd.as_path(),
environment.cwd.as_path(),
);
self.record_granted_request_permissions_for_turn(
&response,
&environment.environment_id,
originating_turn_state.as_ref(),
)
.await;
@@ -2242,7 +2249,7 @@ impl Session {
PendingRequestPermissions {
tx_response,
requested_permissions: requested_permissions.clone(),
cwd: cwd.clone(),
environment: environment.clone(),
},
)
}
@@ -2259,7 +2266,7 @@ impl Session {
started_at_ms: now_unix_timestamp_ms(),
reason: args.reason,
permissions: requested_permissions,
cwd: Some(cwd),
cwd: Some(environment.cwd),
});
self.send_event(turn_context.as_ref(), event).await;
tokio::select! {
@@ -2276,6 +2283,33 @@ impl Session {
}
}
pub(crate) async fn request_permissions_for_cwd(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cwd: AbsolutePathBuf,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
let Some(primary_environment) = turn_context.environments.primary() else {
return Some(RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
});
};
let mut environment = primary_environment.selection();
environment.cwd = cwd;
self.request_permissions_for_environment(
turn_context,
call_id,
args,
environment,
cancellation_token,
)
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
@@ -2370,10 +2404,11 @@ impl Session {
let response = Self::normalize_request_permissions_response(
entry.requested_permissions,
response,
entry.cwd.as_path(),
entry.environment.cwd.as_path(),
);
self.record_granted_request_permissions_for_turn(
&response,
&entry.environment.environment_id,
originating_turn_state.as_ref(),
)
.await;
@@ -2417,6 +2452,7 @@ impl Session {
async fn record_granted_request_permissions_for_turn(
&self,
response: &RequestPermissionsResponse,
environment_id: &str,
originating_turn_state: Option<&Arc<Mutex<crate::state::TurnState>>>,
) {
if response.permissions.is_empty() {
@@ -2428,7 +2464,7 @@ impl Session {
let mut ts = turn_state.lock().await;
let permissions: AdditionalPermissionProfile =
response.permissions.clone().into();
ts.record_granted_permissions(permissions);
ts.record_granted_permissions(environment_id, permissions);
if response.strict_auto_review {
ts.enable_strict_auto_review();
}
@@ -2436,7 +2472,10 @@ impl Session {
}
PermissionGrantScope::Session => {
let mut state = self.state.lock().await;
state.record_granted_permissions(response.permissions.clone().into());
state.record_granted_permissions(
environment_id,
response.permissions.clone().into(),
);
}
}
}
@@ -2445,11 +2484,14 @@ impl Session {
clippy::await_holding_invalid_type,
reason = "active turn reads must stay consistent with the matching turn state"
)]
pub(crate) async fn granted_turn_permissions(&self) -> Option<AdditionalPermissionProfile> {
pub(crate) async fn granted_turn_permissions(
&self,
environment_id: &str,
) -> Option<AdditionalPermissionProfile> {
let active = self.active_turn.lock().await;
let active = active.as_ref()?;
let ts = active.turn_state.lock().await;
ts.granted_permissions()
ts.granted_permissions(environment_id)
}
#[expect(
@@ -2465,9 +2507,12 @@ impl Session {
ts.strict_auto_review_enabled()
}
pub(crate) async fn granted_session_permissions(&self) -> Option<AdditionalPermissionProfile> {
pub(crate) async fn granted_session_permissions(
&self,
environment_id: &str,
) -> Option<AdditionalPermissionProfile> {
let state = self.state.lock().await;
state.granted_permissions()
state.granted_permissions(environment_id)
}
#[expect(
codex-rs/core/src/session/tests.rs
+189 -6
View File
@@ -73,6 +73,7 @@ use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
use crate::tools::handlers::CreateGoalHandler;
use crate::tools::handlers::ExecCommandHandler;
use crate::tools::handlers::RequestPermissionsHandler;
use crate::tools::handlers::ShellCommandHandler;
use crate::tools::handlers::UpdateGoalHandler;
use crate::tools::registry::ToolExecutor;
@@ -5155,7 +5156,12 @@ async fn notify_request_permissions_response_ignores_unmatched_call_id() {
)
.await;
assert_eq!(session.granted_turn_permissions().await, None);
assert_eq!(
session
.granted_turn_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
None
);
}
#[tokio::test]
@@ -5182,16 +5188,84 @@ async fn record_granted_request_permissions_for_turn_uses_originating_turn() {
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
},
codex_exec_server::LOCAL_ENVIRONMENT_ID,
Some(&originating_turn_state),
)
.await;
assert_eq!(
originating_turn_state.lock().await.granted_permissions(),
originating_turn_state
.lock()
.await
.granted_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID),
Some(requested_permissions.into())
);
assert_eq!(current_turn_state.lock().await.granted_permissions(), None);
assert_eq!(session.granted_turn_permissions().await, None);
assert_eq!(
current_turn_state
.lock()
.await
.granted_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID),
None
);
assert_eq!(
session
.granted_turn_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
None
);
}
#[tokio::test]
async fn request_permission_grants_are_environment_keyed() {
let (session, _turn_context) = make_session_and_context().await;
let originating_active_turn = ActiveTurn::default();
let originating_turn_state = Arc::clone(&originating_active_turn.turn_state);
*session.active_turn.lock().await = Some(originating_active_turn);
let requested_permissions = RequestPermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
enabled: Some(true),
}),
..RequestPermissionProfile::default()
};
session
.record_granted_request_permissions_for_turn(
&codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: requested_permissions.clone(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
},
"remote",
Some(&originating_turn_state),
)
.await;
{
let turn_state = originating_turn_state.lock().await;
assert_eq!(
turn_state.granted_permissions("remote"),
Some(requested_permissions.clone().into())
);
assert_eq!(turn_state.granted_permissions("local"), None);
}
session
.record_granted_request_permissions_for_turn(
&codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: requested_permissions.clone(),
scope: PermissionGrantScope::Session,
strict_auto_review: false,
},
"remote",
/*originating_turn_state*/ None,
)
.await;
assert_eq!(
session.granted_session_permissions("remote").await,
Some(requested_permissions.into())
);
assert_eq!(session.granted_session_permissions("local").await, None);
}
#[tokio::test]
@@ -5214,6 +5288,7 @@ async fn enable_strict_auto_review_for_turn_uses_originating_turn() {
scope: PermissionGrantScope::Turn,
strict_auto_review: true,
},
codex_exec_server::LOCAL_ENVIRONMENT_ID,
Some(&originating_turn_state),
)
.await;
@@ -5333,6 +5408,107 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
assert_eq!(response, Some(expected_response));
}
#[tokio::test]
async fn request_permissions_tool_resolves_relative_paths_against_primary_environment() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
*session.active_turn.lock().await = Some(ActiveTurn::default());
let environment_cwd = {
#[allow(deprecated)]
let legacy_cwd = turn_context.cwd.clone();
legacy_cwd.join("request-permissions-environment")
};
std::fs::create_dir_all(environment_cwd.as_path()).expect("create environment cwd");
let turn_context_mut = Arc::get_mut(&mut turn_context).expect("single thread settings ref");
turn_context_mut
.approval_policy
.set(AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: true,
rules: true,
skill_approval: true,
request_permissions: true,
mcp_elicitations: true,
}))
.expect("test setup should allow updating approval policy");
turn_context_mut.environments.turn_environments[0].cwd = environment_cwd.clone();
let call_id = "call-1".to_string();
let handler = RequestPermissionsHandler;
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
let handle = tokio::spawn({
let session = Arc::clone(&session);
let turn_context = Arc::clone(&turn_context);
let tracker = Arc::clone(&tracker);
let call_id = call_id.clone();
async move {
handler
.handle(ToolInvocation {
session,
turn: turn_context,
cancellation_token: CancellationToken::new(),
tracker,
call_id,
tool_name: codex_tools::ToolName::plain("request_permissions"),
source: ToolCallSource::Direct,
payload: ToolPayload::Function {
arguments: json!({
"reason": "need write",
"permissions": {
"file_system": {
"entries": [{
"path": {
"type": "path",
"path": "relative.txt",
},
"access": "write",
}],
},
},
})
.to_string(),
},
})
.await
}
});
let request_event = tokio::time::timeout(StdDuration::from_secs(1), rx.recv())
.await
.expect("request_permissions event timed out")
.expect("request_permissions event missing");
let EventMsg::RequestPermissions(request) = request_event.msg else {
panic!("expected request_permissions event");
};
let expected_permissions = RequestPermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: environment_cwd.join("relative.txt"),
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
assert_eq!(request.permissions, expected_permissions);
session
.notify_request_permissions_response(
&request.call_id,
codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: request.permissions,
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
},
)
.await;
tokio::time::timeout(StdDuration::from_secs(1), handle)
.await
.expect("request_permissions handler timed out")
.expect("request_permissions handler join error")
.expect("request_permissions handler should succeed");
}
#[tokio::test]
async fn request_permissions_response_materializes_session_cwd_grants_before_recording() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
@@ -5425,7 +5601,9 @@ async fn request_permissions_response_materializes_session_cwd_grants_before_rec
assert_eq!(response, Some(expected_response));
assert_eq!(
session.granted_session_permissions().await,
session
.granted_session_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
Some(expected_permissions.into())
);
}
@@ -10450,7 +10628,12 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
);
pretty_assertions::assert_eq!(output, expected);
pretty_assertions::assert_eq!(session.granted_turn_permissions().await, None);
pretty_assertions::assert_eq!(
session
.granted_turn_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
None
);
// The rejection should not poison the non-escalated path for the same
// command. Force DangerFullAccess so this check stays focused on approval
codex-rs/core/src/session/tests/guardian_tests.rs
+19 -8
View File
@@ -148,7 +148,9 @@ async fn request_permissions_routes_to_guardian_when_reviewer_is_enabled() {
})
);
assert_eq!(
session.granted_turn_permissions().await,
session
.granted_turn_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
Some(requested_permissions.into())
);
@@ -246,7 +248,12 @@ async fn request_permissions_guardian_review_stops_when_cancelled() {
.expect("request_permissions should stop when cancelled")
.expect("request_permissions task should not panic");
assert_eq!(response, None);
assert_eq!(session.granted_turn_permissions().await, None);
assert_eq!(
session
.granted_turn_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
None
);
}
#[tokio::test]
@@ -380,6 +387,7 @@ async fn strict_auto_review_turn_grant_forces_guardian_for_shell_command_policy_
scope: PermissionGrantScope::Turn,
strict_auto_review: true,
},
codex_exec_server::LOCAL_ENVIRONMENT_ID,
Some(&originating_turn_state),
)
.await;
@@ -564,12 +572,15 @@ async fn shell_command_allows_sticky_turn_permissions_without_inline_request_per
let mut active_turn = session.active_turn.lock().await;
let active_turn = active_turn.as_mut().expect("active turn");
let mut turn_state = active_turn.turn_state.lock().await;
turn_state.record_granted_permissions(PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
..Default::default()
});
turn_state.record_granted_permissions(
codex_exec_server::LOCAL_ENVIRONMENT_ID,
PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
..Default::default()
},
);
}
let session = Arc::new(session);