mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Surface filesystem permission profiles in prompt context (#23924)
## Summary Some permission profiles can encode filesystem reads that should remain unavailable to the agent. Before this change, the model-visible context and automatic approval review prompt summarized the effective permissions as a legacy sandbox mode, which can omit permission-profile filesystem entries from escalation decisions. For example, a profile can grant workspace access while denying a private subtree across every workspace root: ```toml default_permissions = "restricted-workspace" [permissions.restricted-workspace.workspace_roots] "/Users/alice/project" = true "/Users/alice/other-project" = true [permissions.restricted-workspace.filesystem] ":minimal" = "read" [permissions.restricted-workspace.filesystem.":workspace_roots"] "." = "write" "private" = "deny" "private/**" = "deny" ``` The context window now describes the workspace roots and effective filesystem side of the `PermissionProfile` directly, with deny entries marked as non-escalatable: ```xml <environment_context> <cwd>/Users/alice/project</cwd> <shell>zsh</shell> <filesystem><workspace_roots><root>/Users/alice/project</root><root>/Users/alice/other-project</root></workspace_roots><permission_profile type="managed"><file_system type="restricted"><entry access="read"><special>:minimal</special></entry><entry access="write"><path>/Users/alice/project</path></entry><entry access="write"><path>/Users/alice/other-project</path></entry><entry access="deny" escalatable="false"><path>/Users/alice/project/private</path></entry><entry access="deny" escalatable="false"><path>/Users/alice/other-project/private</path></entry><entry access="deny" escalatable="false"><glob>/Users/alice/project/private/**</glob></entry><entry access="deny" escalatable="false"><glob>/Users/alice/other-project/private/**</glob></entry></file_system></permission_profile></filesystem> </environment_context> ``` Managed requirements can impose the same kind of deny-read restriction: ```toml [permissions.filesystem] deny_read = [ "/Users/alice/project/private", "/Users/alice/project/private/**", ] ``` The automatic approval review prompt also receives the parent turn's denied-read context, so review decisions can account for the active permission profile. ## What Changed - Render the effective filesystem profile in `<environment_context>`, including profile type, filesystem entries, workspace roots, and non-escalatable deny entries. - Persist effective `workspace_roots` in `TurnContextItem` so resumed/replayed context does not have to bind `:workspace_roots` through legacy `cwd` fallback. - Add explicit permission instructions that denied reads are policy restrictions, not escalation targets. - Pass the parent turn's denied-read context into automatic approval reviews. - Add targeted coverage for prompt rendering, workspace-root materialization, replay context, and review prompt context. - Keep the prompt-context test expectations platform-aware so the same filesystem rendering assertions pass on Unix and Windows paths. ## Testing - `just test -p codex-core context::environment_context::tests::serialize_environment_context_with_full_filesystem_profile` - `just test -p codex-core context::environment_context::tests::turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd` - `just test -p codex-core context::permissions_instructions::permissions_instructions_tests::builds_permissions_from_profile_with_denied_reads` - `just fix -p codex-core` I also attempted `just test -p codex-core`; the changed prompt-context tests passed, but the full local run did not complete cleanly in this sandboxed macOS environment due unrelated user-shell `CODEX_SANDBOX*` expectations and integration-test timeouts.
This commit is contained in:
@@ -46,10 +46,7 @@ fn text_user_input_parts(texts: Vec<String>) -> serde_json::Value {
|
||||
}
|
||||
|
||||
fn assert_default_env_context(text: &str, cwd: &str) {
|
||||
assert!(
|
||||
text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG),
|
||||
"expected environment context fragment: {text}"
|
||||
);
|
||||
assert_env_context_fragment(text);
|
||||
assert!(
|
||||
text.contains(&format!("<cwd>{cwd}</cwd>")),
|
||||
"expected cwd in environment context: {text}"
|
||||
@@ -58,6 +55,13 @@ fn assert_default_env_context(text: &str, cwd: &str) {
|
||||
text.contains(&format!("<shell>{}</shell>", default_user_shell().name())),
|
||||
"expected shell in environment context: {text}"
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_env_context_fragment(text: &str) {
|
||||
assert!(
|
||||
text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG),
|
||||
"expected environment context fragment: {text}"
|
||||
);
|
||||
assert!(
|
||||
text.contains("<current_date>") && text.contains("</current_date>"),
|
||||
"expected current_date in environment context: {text}"
|
||||
@@ -502,8 +506,24 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an
|
||||
expected_permissions_msg_2, expected_permissions_msg,
|
||||
"expected updated permissions message after override"
|
||||
);
|
||||
let expected_env_msg_2 = body2["input"][body1_input.len() + 1].clone();
|
||||
assert_eq!(expected_env_msg_2["role"].as_str(), Some("user"));
|
||||
let env_text = expected_env_msg_2["content"][0]["text"]
|
||||
.as_str()
|
||||
.expect("environment context text");
|
||||
assert_env_context_fragment(env_text);
|
||||
assert!(
|
||||
env_text.contains("<permission_profile type=\"managed\">")
|
||||
&& env_text.contains("<file_system type=\"restricted\">")
|
||||
&& env_text.contains(&format!(
|
||||
"<entry access=\"write\"><path>{}</path></entry>",
|
||||
writable.abs().display()
|
||||
)),
|
||||
"expected workspace-write filesystem profile in environment context: {env_text}"
|
||||
);
|
||||
let mut expected_body2 = body1_input.to_vec();
|
||||
expected_body2.push(expected_permissions_msg_2);
|
||||
expected_body2.push(expected_env_msg_2);
|
||||
expected_body2.push(expected_user_message_2);
|
||||
assert_eq!(body2["input"], serde_json::Value::Array(expected_body2));
|
||||
|
||||
@@ -1086,12 +1106,25 @@ async fn send_user_turn_with_changes_sends_environment_context() -> anyhow::Resu
|
||||
}),
|
||||
"expected model switch section after model override: {expected_settings_update_msg:?}"
|
||||
);
|
||||
let expected_env_update_msg = body2["input"][body1_input.len() + 1].clone();
|
||||
assert_eq!(expected_env_update_msg["role"].as_str(), Some("user"));
|
||||
let expected_env_update_text = expected_env_update_msg["content"][0]["text"]
|
||||
.as_str()
|
||||
.expect("environment context text");
|
||||
assert_env_context_fragment(expected_env_update_text);
|
||||
assert!(
|
||||
expected_env_update_text.contains(
|
||||
"<permission_profile type=\"disabled\"><file_system type=\"unrestricted\" /></permission_profile>",
|
||||
),
|
||||
"expected disabled filesystem profile in environment context: {expected_env_update_text}"
|
||||
);
|
||||
let expected_user_message_2 = text_user_input("hello 2".to_string());
|
||||
let expected_input_2 = serde_json::Value::Array(vec![
|
||||
expected_permissions_msg,
|
||||
expected_contextual_user_msg_1,
|
||||
expected_user_message_1,
|
||||
expected_settings_update_msg,
|
||||
expected_env_update_msg,
|
||||
expected_user_message_2,
|
||||
]);
|
||||
assert_eq!(body2["input"], expected_input_2);
|
||||
|
||||
@@ -28,6 +28,7 @@ fn resume_history(
|
||||
let turn_ctx = TurnContextItem {
|
||||
turn_id: Some(turn_id.clone()),
|
||||
cwd: config.cwd.to_path_buf(),
|
||||
workspace_roots: None,
|
||||
current_date: None,
|
||||
timezone: None,
|
||||
approval_policy: config.permissions.approval_policy.value(),
|
||||
|
||||
Reference in New Issue
Block a user