fix: surface denied filesystem reads in prompts

Expose denied-read entries from the active permission profile to model-visible context and automatic approval review prompts so escalation decisions can account for policy-restricted paths.
This commit is contained in:
Michael Bolin
2026-05-21 13:22:52 -07:00
parent 24faf49b2a
commit 9b3a7c9570
8 changed files with 336 additions and 26 deletions

View File

@@ -1,9 +1,11 @@
use crate::session::turn_context::TurnContext;
use crate::session::turn_context::TurnEnvironment;
use crate::shell::Shell;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::TurnContextItem;
use codex_protocol::protocol::TurnContextNetworkItem;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::path::Path;
use super::ContextualUserFragment;
@@ -13,6 +15,7 @@ pub(crate) struct EnvironmentContext {
pub(crate) current_date: Option<String>,
pub(crate) timezone: Option<String>,
pub(crate) network: Option<NetworkContext>,
pub(crate) filesystem: Option<FileSystemContext>,
pub(crate) subagents: Option<String>,
}
@@ -83,6 +86,55 @@ impl EnvironmentContextEnvironments {
}
}
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub(crate) struct FileSystemContext {
unreadable_roots: Vec<String>,
unreadable_globs: Vec<String>,
}
impl FileSystemContext {
fn new(unreadable_roots: Vec<String>, unreadable_globs: Vec<String>) -> Option<Self> {
if unreadable_roots.is_empty() && unreadable_globs.is_empty() {
None
} else {
Some(Self {
unreadable_roots,
unreadable_globs,
})
}
}
fn from_permission_profile(permission_profile: &PermissionProfile, cwd: &Path) -> Option<Self> {
let file_system_sandbox_policy = permission_profile.file_system_sandbox_policy();
Self::new(
file_system_sandbox_policy
.get_unreadable_roots_with_cwd(cwd)
.into_iter()
.map(|root| root.to_string_lossy().into_owned())
.collect(),
file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd),
)
}
fn render(&self) -> String {
let mut rendered = "<filesystem><deny_read escalatable=\"false\">".to_string();
Self::push_rendered_element(&mut rendered, "paths", &self.unreadable_roots);
Self::push_rendered_element(&mut rendered, "globs", &self.unreadable_globs);
rendered.push_str("</deny_read></filesystem>");
rendered
}
fn push_rendered_element(rendered_filesystem: &mut String, name: &str, values: &[String]) {
if values.is_empty() {
return;
}
rendered_filesystem.push_str(&format!("<{name}>"));
rendered_filesystem.push_str(&values.join(","));
rendered_filesystem.push_str(&format!("</{name}>"));
}
}
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub(crate) struct NetworkContext {
allowed_domains: Vec<String>,
@@ -129,6 +181,7 @@ impl EnvironmentContext {
current_date,
timezone,
network,
filesystem: None,
subagents,
}
}
@@ -138,6 +191,7 @@ impl EnvironmentContext {
current_date: Option<String>,
timezone: Option<String>,
network: Option<NetworkContext>,
filesystem: Option<FileSystemContext>,
subagents: Option<String>,
) -> Self {
Self {
@@ -145,6 +199,7 @@ impl EnvironmentContext {
current_date,
timezone,
network,
filesystem,
subagents,
}
}
@@ -157,6 +212,7 @@ impl EnvironmentContext {
&& self.current_date == other.current_date
&& self.timezone == other.timezone
&& self.network == other.network
&& self.filesystem == other.filesystem
&& self.subagents == other.subagents
}
@@ -165,6 +221,7 @@ impl EnvironmentContext {
after: &EnvironmentContext,
) -> Self {
let before_network = Self::network_from_turn_context_item(before);
let before_filesystem = Self::filesystem_from_turn_context_item(before);
let environments = match &after.environments {
EnvironmentContextEnvironments::Single(environment) => {
if before.cwd.as_path() != environment.cwd.as_path() {
@@ -186,17 +243,25 @@ impl EnvironmentContext {
} else {
before_network
};
let filesystem = if before_filesystem != after.filesystem {
after.filesystem.clone()
} else {
before_filesystem
};
EnvironmentContext::new_with_environments(
environments,
after.current_date.clone(),
after.timezone.clone(),
network,
filesystem,
/*subagents*/ None,
)
}
pub(crate) fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self {
Self::new(
#[allow(deprecated)]
let cwd = &turn_context.cwd;
let mut context = Self::new(
EnvironmentContextEnvironment::from_turn_environments(
&turn_context.environments.turn_environments,
shell,
@@ -205,7 +270,10 @@ impl EnvironmentContext {
turn_context.timezone.clone(),
Self::network_from_turn_context(turn_context),
/*subagents*/ None,
)
);
context.filesystem =
FileSystemContext::from_permission_profile(&turn_context.permission_profile, cwd);
context
}
pub(crate) fn from_turn_context_item(
@@ -216,11 +284,14 @@ impl EnvironmentContext {
Ok(cwd) => cwd,
Err(_) => AbsolutePathBuf::resolve_path_against_base(&turn_context_item.cwd, "/"),
};
Self::new(
vec![EnvironmentContextEnvironment::legacy(cwd, shell)],
Self::new_with_environments(
EnvironmentContextEnvironments::from_vec(vec![EnvironmentContextEnvironment::legacy(
cwd, shell,
)]),
turn_context_item.current_date.clone(),
turn_context_item.timezone.clone(),
Self::network_from_turn_context_item(turn_context_item),
Self::filesystem_from_turn_context_item(turn_context_item),
/*subagents*/ None,
)
}
@@ -266,6 +337,15 @@ impl EnvironmentContext {
denied_domains.clone(),
))
}
fn filesystem_from_turn_context_item(
turn_context_item: &TurnContextItem,
) -> Option<FileSystemContext> {
FileSystemContext::from_permission_profile(
&turn_context_item.permission_profile(),
&turn_context_item.cwd,
)
}
}
impl ContextualUserFragment for EnvironmentContext {
@@ -324,6 +404,9 @@ impl ContextualUserFragment for EnvironmentContext {
// lines.push(" <network enabled=\"false\" />".to_string());
}
}
if let Some(filesystem) = &self.filesystem {
lines.push(format!(" {}", filesystem.render()));
}
if let Some(subagents) = &self.subagents {
lines.push(" <subagents>".to_string());
lines.extend(subagents.lines().map(|line| format!(" {line}")));

View File

@@ -79,6 +79,36 @@ fn serialize_environment_context_with_network() {
assert_eq!(context.render(), expected);
}
#[test]
fn serialize_environment_context_with_filesystem_denied_reads() {
let mut context = EnvironmentContext::new(
vec![EnvironmentContextEnvironment {
id: "local".to_string(),
cwd: test_path_buf("/repo").abs(),
shell: fake_shell_name(),
}],
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
/*subagents*/ None,
);
context.filesystem = FileSystemContext::new(
vec!["/repo/private".to_string()],
vec!["/repo/private/**".to_string()],
);
let expected = format!(
r#"<environment_context>
<cwd>{}</cwd>
<shell>bash</shell>
<filesystem><deny_read escalatable="false"><paths>/repo/private</paths><globs>/repo/private/**</globs></deny_read></filesystem>
</environment_context>"#,
test_path_buf("/repo").display()
);
assert_eq!(context.render(), expected);
}
#[test]
fn serialize_read_only_environment_context() {
let context = EnvironmentContext::new(

View File

@@ -4,6 +4,7 @@ use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::config_types::SandboxMode;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::format_allow_prefixes;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::GranularApprovalConfig;
@@ -68,9 +69,11 @@ impl PermissionsInstructions {
exec_permission_approvals_enabled: bool,
request_permissions_tool_enabled: bool,
) -> Self {
let (sandbox_mode, writable_roots) = sandbox_prompt_from_profile(permission_profile, cwd);
let file_system_sandbox_policy = permission_profile.file_system_sandbox_policy();
let (sandbox_mode, writable_roots) =
sandbox_prompt_from_policy(&file_system_sandbox_policy, cwd);
Self::from_permissions_with_network(
Self::from_permissions_with_network_and_denied_reads(
sandbox_mode,
network_access_from_policy(permission_profile.network_sandbox_policy()),
PermissionsPromptConfig {
@@ -81,14 +84,32 @@ impl PermissionsInstructions {
request_permissions_tool_enabled,
},
writable_roots,
denied_reads_text(&file_system_sandbox_policy, cwd),
)
}
#[cfg(test)]
fn from_permissions_with_network(
sandbox_mode: SandboxMode,
network_access: NetworkAccess,
config: PermissionsPromptConfig<'_>,
writable_roots: Option<Vec<WritableRoot>>,
) -> Self {
Self::from_permissions_with_network_and_denied_reads(
sandbox_mode,
network_access,
config,
writable_roots,
/*denied_reads*/ None,
)
}
fn from_permissions_with_network_and_denied_reads(
sandbox_mode: SandboxMode,
network_access: NetworkAccess,
config: PermissionsPromptConfig<'_>,
writable_roots: Option<Vec<WritableRoot>>,
denied_reads: Option<String>,
) -> Self {
let mut text = String::new();
append_section(&mut text, &sandbox_text(sandbox_mode, network_access));
@@ -105,6 +126,9 @@ impl PermissionsInstructions {
if let Some(writable_roots) = writable_roots_text(writable_roots) {
append_section(&mut text, &writable_roots);
}
if let Some(denied_reads) = denied_reads {
append_section(&mut text, &denied_reads);
}
if !text.ends_with('\n') {
text.push('\n');
}
@@ -112,27 +136,19 @@ impl PermissionsInstructions {
}
}
fn sandbox_prompt_from_profile(
permission_profile: &PermissionProfile,
fn sandbox_prompt_from_policy(
file_system_policy: &FileSystemSandboxPolicy,
cwd: &Path,
) -> (SandboxMode, Option<Vec<WritableRoot>>) {
match permission_profile {
PermissionProfile::Disabled | PermissionProfile::External { .. } => {
(SandboxMode::DangerFullAccess, None)
}
PermissionProfile::Managed { .. } => {
let file_system_policy = permission_profile.file_system_sandbox_policy();
if file_system_policy.has_full_disk_write_access() {
return (SandboxMode::DangerFullAccess, None);
}
if file_system_policy.has_full_disk_write_access() {
return (SandboxMode::DangerFullAccess, None);
}
let writable_roots = file_system_policy.get_writable_roots_with_cwd(cwd);
if writable_roots.is_empty() {
(SandboxMode::ReadOnly, None)
} else {
(SandboxMode::WorkspaceWrite, Some(writable_roots))
}
}
let writable_roots = file_system_policy.get_writable_roots_with_cwd(cwd);
if writable_roots.is_empty() {
(SandboxMode::ReadOnly, None)
} else {
(SandboxMode::WorkspaceWrite, Some(writable_roots))
}
}
@@ -254,6 +270,28 @@ fn writable_roots_text(writable_roots: Option<Vec<WritableRoot>>) -> Option<Stri
})
}
fn denied_reads_text(file_system_policy: &FileSystemSandboxPolicy, cwd: &Path) -> Option<String> {
let mut entries = file_system_policy
.get_unreadable_roots_with_cwd(cwd)
.into_iter()
.map(|root| format!("- path `{}`", root.to_string_lossy()))
.collect::<Vec<_>>();
entries.extend(
file_system_policy
.get_unreadable_globs_with_cwd(cwd)
.into_iter()
.map(|glob| format!("- glob `{glob}`")),
);
if entries.is_empty() {
return None;
}
Some(format!(
"## Denied filesystem reads\nThe active permission profile denies reading these paths/globs. Do not request escalation or additional permissions to read them; these denials are policy restrictions.\n{}",
entries.join("\n")
))
}
fn approved_command_prefixes_text(exec_policy: &Policy) -> Option<String> {
format_allow_prefixes(exec_policy.get_allowed_prefixes())
.filter(|prefixes| !prefixes.is_empty())

View File

@@ -83,6 +83,51 @@ fn builds_permissions_from_profile() {
assert!(text.contains(writable_root.to_string_lossy().as_ref()));
}
#[test]
fn builds_permissions_from_profile_with_denied_reads() {
let cwd = PathBuf::from("/tmp");
let denied_root =
AbsolutePathBuf::from_absolute_path(cwd.join("blocked")).expect("absolute path");
let permission_profile = PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: denied_root.clone(),
},
access: FileSystemAccessMode::Deny,
},
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: "/tmp/blocked/**".to_string(),
},
access: FileSystemAccessMode::Deny,
},
]),
NetworkSandboxPolicy::Restricted,
);
let instructions = PermissionsInstructions::from_permission_profile(
&permission_profile,
AskForApproval::OnRequest,
ApprovalsReviewer::AutoReview,
&Policy::empty(),
&cwd,
/*exec_permission_approvals_enabled*/ false,
/*request_permissions_tool_enabled*/ false,
);
let text = instructions.body();
assert!(text.contains("## Denied filesystem reads"));
assert!(text.contains("Do not request escalation or additional permissions"));
assert!(text.contains(denied_root.to_string_lossy().as_ref()));
assert!(text.contains("glob `/tmp/blocked/**`"));
}
#[test]
fn includes_request_rule_instructions_for_on_request() {
let mut exec_policy = Policy::empty();

View File

@@ -147,6 +147,8 @@ use prompt::GuardianTranscriptEntryKind;
#[cfg(test)]
use prompt::build_guardian_prompt_items;
#[cfg(test)]
use prompt::build_guardian_prompt_items_with_parent_turn;
#[cfg(test)]
use prompt::collect_guardian_transcript_entries;
#[cfg(test)]
use prompt::guardian_output_schema;

View File

@@ -10,6 +10,7 @@ use serde_json::Value;
use crate::compact::content_items_to_text;
use crate::event_mapping::is_contextual_user_message_content;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use codex_utils_output_truncation::approx_bytes_for_tokens;
use codex_utils_output_truncation::approx_token_count;
use codex_utils_output_truncation::approx_tokens_from_byte_count;
@@ -86,11 +87,29 @@ pub(crate) enum GuardianPromptMode {
/// Split the variable request into separate user content items so the
/// Responses request snapshot shows clear boundaries while preserving exact
/// prompt text through trailing newlines.
#[cfg(test)]
pub(crate) async fn build_guardian_prompt_items(
session: &Session,
retry_reason: Option<String>,
request: GuardianApprovalRequest,
mode: GuardianPromptMode,
) -> serde_json::Result<GuardianPromptItems> {
build_guardian_prompt_items_with_parent_turn(
session,
/*parent_turn*/ None,
retry_reason,
request,
mode,
)
.await
}
pub(crate) async fn build_guardian_prompt_items_with_parent_turn(
session: &Session,
parent_turn: Option<&TurnContext>,
retry_reason: Option<String>,
request: GuardianApprovalRequest,
mode: GuardianPromptMode,
) -> serde_json::Result<GuardianPromptItems> {
let history = session.clone_history().await;
let transcript_entries = collect_guardian_transcript_entries(history.raw_items());
@@ -172,6 +191,11 @@ pub(crate) async fn build_guardian_prompt_items(
if let Some(note) = omission_note {
push_text(format!("\n{note}\n"));
}
if let Some(denied_reads_context) = parent_turn.and_then(parent_turn_denied_reads_context) {
push_text("\n>>> PARENT TURN PERMISSION CONTEXT START\n".to_string());
push_text(denied_reads_context);
push_text(">>> PARENT TURN PERMISSION CONTEXT END\n".to_string());
}
match &request {
GuardianApprovalRequest::NetworkAccess { trigger, .. } => {
push_text(">>> APPROVAL REQUEST START\n".to_string());
@@ -216,6 +240,31 @@ pub(crate) async fn build_guardian_prompt_items(
})
}
fn parent_turn_denied_reads_context(turn: &TurnContext) -> Option<String> {
#[allow(deprecated)]
let cwd = &turn.cwd;
let file_system_policy = turn.permission_profile.file_system_sandbox_policy();
let mut entries = file_system_policy
.get_unreadable_roots_with_cwd(cwd)
.into_iter()
.map(|root| format!("- path `{}`", root.to_string_lossy()))
.collect::<Vec<_>>();
entries.extend(
file_system_policy
.get_unreadable_globs_with_cwd(cwd)
.into_iter()
.map(|glob| format!("- glob `{glob}`")),
);
if entries.is_empty() {
return None;
}
Some(format!(
"The parent turn's active permission profile denies reading these paths/globs. These are policy restrictions; do not approve escalation whose purpose is to read them.\n{}\n",
entries.join("\n")
))
}
enum GuardianPromptShape {
Full,
Delta { already_seen_entry_count: usize },

View File

@@ -49,7 +49,7 @@ use super::GUARDIAN_REVIEWER_NAME;
use super::GuardianApprovalRequest;
use super::prompt::GuardianPromptMode;
use super::prompt::GuardianTranscriptCursor;
use super::prompt::build_guardian_prompt_items;
use super::prompt::build_guardian_prompt_items_with_parent_turn;
use super::prompt::guardian_policy_prompt;
use super::prompt::guardian_policy_prompt_with_config;
@@ -670,8 +670,9 @@ async fn run_review_on_session(
)
.await;
build_guardian_prompt_items(
build_guardian_prompt_items_with_parent_turn(
params.parent_session.as_ref(),
Some(params.parent_turn.as_ref()),
params.retry_reason.clone(),
params.request.clone(),
prompt_mode,

View File

@@ -33,6 +33,11 @@ use codex_protocol::models::ContentItem;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::ReasoningEffort;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
@@ -357,6 +362,63 @@ async fn build_guardian_prompt_full_mode_preserves_initial_review_format() -> an
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Result<()> {
let (mut session, mut turn) = crate::session::tests::make_session_and_context().await;
session.conversation_id = fixed_guardian_parent_session_id();
let denied_root = test_path_buf("/repo/private").abs();
turn.permission_profile = PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: denied_root.clone(),
},
access: FileSystemAccessMode::Deny,
},
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: "/repo/private/**".to_string(),
},
access: FileSystemAccessMode::Deny,
},
]),
NetworkSandboxPolicy::Restricted,
);
let session = Arc::new(session);
let turn = Arc::new(turn);
seed_guardian_parent_history(&session, &turn).await;
let prompt = build_guardian_prompt_items_with_parent_turn(
session.as_ref(),
Some(turn.as_ref()),
Some("Sandbox denied reading /repo/private/secret.txt.".to_string()),
GuardianApprovalRequest::Shell {
id: "shell-1".to_string(),
command: vec!["cat".to_string(), "/repo/private/secret.txt".to_string()],
cwd: test_path_buf("/repo").abs(),
sandbox_permissions: crate::sandboxing::SandboxPermissions::RequireEscalated,
additional_permissions: None,
justification: Some("Need to inspect the secret file.".to_string()),
},
GuardianPromptMode::Full,
)
.await?;
let text = guardian_prompt_text(&prompt.items);
assert!(text.contains("PARENT TURN PERMISSION CONTEXT START"));
assert!(text.contains("do not approve escalation whose purpose is to read them"));
assert!(text.contains(denied_root.to_string_lossy().as_ref()));
assert!(text.contains("glob `/repo/private/**`"));
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn build_guardian_prompt_delta_mode_preserves_original_numbering() -> anyhow::Result<()> {
let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await;