mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
Derive sandbox telemetry labels directly from permission profiles (#60500)
## What changed Compute the `sandbox_policy` label for `codex.conversation_starts` from the effective `PermissionProfile` without constructing a legacy `SandboxPolicy`. Preserve the historical labels and native-path compatibility fallbacks, including `workspace-write` for writable profiles that cannot be represented by a legacy policy. Share the full-disk write compatibility check with session startup and the write-intent check with legacy policy conversion. ## Testing Extend telemetry tests to assert sandbox labels in both logs and traces across disabled, external, read-only, workspace-write, and full-disk profiles, including unrenderable writable URIs and read restrictions. Add protocol assertions for legacy write-intent detection. GitOrigin-RevId: 00f11d763c366453e7bb0899546a4ebbdb93c4d5
This commit is contained in:
@@ -842,12 +842,9 @@ impl Session {
|
||||
let permission_profile = config.permissions.effective_permission_profile();
|
||||
if permission_profile.enforcement() != SandboxEnforcement::External
|
||||
&& permission_profile.network_sandbox_policy().is_enabled()
|
||||
&& {
|
||||
let file_system = permission_profile.file_system_sandbox_policy();
|
||||
file_system.has_full_disk_write_access()
|
||||
// Preserve the legacy fallback for writable URIs that cannot be projected locally.
|
||||
&& file_system.entries.iter().all(|entry| entry.legacy_writable_path().is_ok())
|
||||
}
|
||||
&& permission_profile
|
||||
.file_system_sandbox_policy()
|
||||
.has_legacy_full_disk_write_access()
|
||||
{
|
||||
let config = Arc::make_mut(&mut config);
|
||||
let permission_profile = PermissionProfile::workspace_write();
|
||||
|
||||
@@ -1552,9 +1552,8 @@ impl Session {
|
||||
config.model_context_window,
|
||||
config.model_auto_compact_token_limit,
|
||||
config.permissions.approval_policy.value(),
|
||||
config
|
||||
.permissions
|
||||
.legacy_sandbox_policy(session_configuration.cwd().as_path()),
|
||||
&config.permissions.effective_permission_profile(),
|
||||
session_configuration.cwd().as_path(),
|
||||
mcp_server_names.iter().map(String::as_str).collect(),
|
||||
&crate::config::legacy_multi_agent_disabled_sources(&config.config_layer_stack),
|
||||
);
|
||||
|
||||
@@ -48,11 +48,11 @@ use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::ToolResultLogConfig;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::CodexErrKind;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::TokenUsage;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
@@ -62,6 +62,7 @@ use opentelemetry_sdk::metrics::data::ResourceMetrics;
|
||||
use reqwest::Error;
|
||||
use reqwest::Response;
|
||||
use std::future::Future;
|
||||
use std::path::Path;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
use tokio::time::error::Elapsed;
|
||||
@@ -693,10 +694,12 @@ impl SessionTelemetry {
|
||||
context_window: Option<i64>,
|
||||
auto_compact_token_limit: Option<i64>,
|
||||
approval_policy: AskForApproval,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
permission_profile: &PermissionProfile,
|
||||
cwd: &Path,
|
||||
mcp_servers: Vec<&str>,
|
||||
legacy_multi_agent_disabled_sources: &[&str],
|
||||
) {
|
||||
let sandbox_policy = permission_profile.legacy_sandbox_policy_label(cwd);
|
||||
log_and_trace_event!(
|
||||
self,
|
||||
common: {
|
||||
|
||||
@@ -34,15 +34,22 @@ use codex_protocol::ToolName;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::ToolResultLogConfig;
|
||||
use codex_protocol::models::ImageReference;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::GuardianAssessmentEvent;
|
||||
use codex_protocol::protocol::GuardianAssessmentOutcome;
|
||||
use codex_protocol::protocol::GuardianAssessmentStatus;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::TokenUsage;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use eventsource_stream::Event as StreamEvent;
|
||||
|
||||
fn log_attributes(record: &SdkLogRecord) -> BTreeMap<String, String> {
|
||||
@@ -874,17 +881,91 @@ fn otel_export_routing_policy_routes_api_request_auth_observability() {
|
||||
.with_auth_env(auth_env_metadata());
|
||||
let root_span = tracing::info_span!("root");
|
||||
let _root_guard = root_span.enter();
|
||||
manager.conversation_starts(
|
||||
"openai",
|
||||
/*reasoning_effort*/ None,
|
||||
ReasoningSummary::Auto,
|
||||
/*context_window*/ None,
|
||||
/*auto_compact_token_limit*/ None,
|
||||
AskForApproval::Never,
|
||||
SandboxPolicy::DangerFullAccess,
|
||||
Vec::new(),
|
||||
/*legacy_multi_agent_disabled_sources*/ &[],
|
||||
let cwd = tempfile::tempdir().expect("telemetry cwd");
|
||||
let root_write = FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
FileSystemAccessMode::Write,
|
||||
);
|
||||
for permission_profile in [
|
||||
PermissionProfile::Disabled,
|
||||
PermissionProfile::read_only(),
|
||||
PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::unrestricted(),
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
),
|
||||
PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Tmpdir,
|
||||
},
|
||||
FileSystemAccessMode::Write,
|
||||
)]),
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
),
|
||||
PermissionProfile::workspace_write(),
|
||||
// Effective workspace roots are explicit paths without a full-root grant.
|
||||
PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new(
|
||||
AbsolutePathBuf::from_absolute_path(cwd.path())
|
||||
.expect("absolute writable workspace")
|
||||
.into(),
|
||||
FileSystemAccessMode::Write,
|
||||
)]),
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
),
|
||||
PermissionProfile::External {
|
||||
network: NetworkSandboxPolicy::Enabled,
|
||||
},
|
||||
PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::restricted(vec![root_write.clone()]),
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
),
|
||||
PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::restricted(vec![root_write.clone()]),
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
),
|
||||
PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::restricted(vec![
|
||||
root_write.clone(),
|
||||
FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Path {
|
||||
path: "file:///%00/bad/path/YQ"
|
||||
.parse()
|
||||
.expect("opaque writable URI"),
|
||||
},
|
||||
FileSystemAccessMode::Write,
|
||||
),
|
||||
]),
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
),
|
||||
PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::restricted(vec![
|
||||
root_write,
|
||||
FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Tmpdir,
|
||||
},
|
||||
FileSystemAccessMode::Read,
|
||||
),
|
||||
]),
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
),
|
||||
] {
|
||||
manager.conversation_starts(
|
||||
"openai",
|
||||
/*reasoning_effort*/ None,
|
||||
ReasoningSummary::Auto,
|
||||
/*context_window*/ None,
|
||||
/*auto_compact_token_limit*/ None,
|
||||
AskForApproval::Never,
|
||||
&permission_profile,
|
||||
cwd.path(),
|
||||
Vec::new(),
|
||||
/*legacy_multi_agent_disabled_sources*/ &[],
|
||||
);
|
||||
}
|
||||
let agent_identity_telemetry = AgentIdentityTelemetry {
|
||||
agent_id: "agent-runtime-otel".to_string(),
|
||||
task_id: "task-run-otel".to_string(),
|
||||
@@ -913,6 +994,31 @@ fn otel_export_routing_policy_routes_api_request_auth_observability() {
|
||||
|
||||
let logs = log_exporter.get_emitted_logs().expect("log export");
|
||||
let conversation_log = find_log_by_event_name(&logs, "codex.conversation_starts");
|
||||
let expected_policy_labels = vec![
|
||||
"danger-full-access",
|
||||
"read-only",
|
||||
"external-sandbox",
|
||||
"workspace-write",
|
||||
"workspace-write",
|
||||
"workspace-write",
|
||||
"external-sandbox",
|
||||
"danger-full-access",
|
||||
"external-sandbox",
|
||||
"workspace-write",
|
||||
"workspace-write",
|
||||
];
|
||||
assert_eq!(
|
||||
logs.iter()
|
||||
.map(|log| log_attributes(&log.record))
|
||||
.filter(|attributes| {
|
||||
attributes
|
||||
.get("event.name")
|
||||
.is_some_and(|name| name == "codex.conversation_starts")
|
||||
})
|
||||
.map(|attributes| attributes["sandbox_policy"].clone())
|
||||
.collect::<Vec<_>>(),
|
||||
expected_policy_labels,
|
||||
);
|
||||
let conversation_log_attrs = log_attributes(&conversation_log.record);
|
||||
assert_eq!(
|
||||
conversation_log_attrs
|
||||
@@ -991,6 +1097,21 @@ fn otel_export_routing_policy_routes_api_request_auth_observability() {
|
||||
let conversation_trace_event =
|
||||
find_span_event_by_name_attr(&spans[0].events.events, "codex.conversation_starts");
|
||||
let conversation_trace_attrs = span_event_attributes(conversation_trace_event);
|
||||
assert_eq!(
|
||||
spans[0]
|
||||
.events
|
||||
.events
|
||||
.iter()
|
||||
.map(span_event_attributes)
|
||||
.filter(|attributes| {
|
||||
attributes
|
||||
.get("event.name")
|
||||
.is_some_and(|name| name == "codex.conversation_starts")
|
||||
})
|
||||
.map(|attributes| attributes["sandbox_policy"].clone())
|
||||
.collect::<Vec<_>>(),
|
||||
expected_policy_labels,
|
||||
);
|
||||
assert_eq!(
|
||||
conversation_trace_attrs
|
||||
.get("auth.env_provider_key_present")
|
||||
|
||||
@@ -689,6 +689,30 @@ impl PermissionProfile {
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the historical `sandbox_policy` diagnostic label without constructing
|
||||
/// a legacy policy. Retains native-path compatibility fallbacks rather than
|
||||
/// evaluating executor authorization.
|
||||
pub fn legacy_sandbox_policy_label(&self, cwd: &Path) -> &'static str {
|
||||
match self {
|
||||
Self::Disabled => "danger-full-access",
|
||||
Self::External { .. } => "external-sandbox",
|
||||
Self::Managed { network, .. } => {
|
||||
let file_system = self.file_system_sandbox_policy();
|
||||
if file_system.has_legacy_full_disk_write_access() {
|
||||
if network.is_enabled() {
|
||||
"danger-full-access"
|
||||
} else {
|
||||
"external-sandbox"
|
||||
}
|
||||
} else if file_system.has_legacy_write_intent(cwd) {
|
||||
"workspace-write"
|
||||
} else {
|
||||
"read-only"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn to_legacy_sandbox_policy(&self, cwd: &Path) -> io::Result<SandboxPolicy> {
|
||||
match self {
|
||||
Self::Managed {
|
||||
|
||||
@@ -1494,6 +1494,42 @@ impl FileSystemSandboxPolicy {
|
||||
self.get_writable_roots_with_cwd_impl(cwd, FileSystemPolicyProjection::LinuxMounts)
|
||||
}
|
||||
|
||||
/// Reports full-disk writes when every explicit writable path can be projected locally.
|
||||
/// Preserves the legacy fallback used by startup checks and diagnostic labels,
|
||||
/// using this host's path convention.
|
||||
pub fn has_legacy_full_disk_write_access(&self) -> bool {
|
||||
self.has_full_disk_write_access()
|
||||
&& self
|
||||
.entries
|
||||
.iter()
|
||||
.all(|entry| entry.legacy_writable_path().is_ok())
|
||||
}
|
||||
|
||||
/// Reports entry write intent recognized by the legacy adapter and its diagnostic labels.
|
||||
/// This does not inspect path existence or apply read/deny precedence: missing and
|
||||
/// unrenderable writable paths still count, while unknown special paths and globs do not.
|
||||
pub fn has_legacy_write_intent(&self, cwd: &Path) -> bool {
|
||||
self.entries.iter().any(|entry| {
|
||||
entry.access.can_write()
|
||||
&& match &entry.path {
|
||||
FileSystemPath::Path { .. } => true,
|
||||
FileSystemPath::GlobPattern { .. } => false,
|
||||
FileSystemPath::Special { value } => match value {
|
||||
FileSystemSpecialPath::Root
|
||||
| FileSystemSpecialPath::Tmpdir
|
||||
| FileSystemSpecialPath::ProjectRoots { subpath: None } => true,
|
||||
FileSystemSpecialPath::ProjectRoots { subpath: Some(_) } => {
|
||||
AbsolutePathBuf::from_absolute_path(cwd).is_ok()
|
||||
}
|
||||
FileSystemSpecialPath::SlashTmp => cfg!(unix),
|
||||
FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => {
|
||||
false
|
||||
}
|
||||
},
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/// Reports configured writable roots for diagnostics without inspecting the filesystem.
|
||||
///
|
||||
/// Unlike runtime root resolution, this includes configured roots even if they
|
||||
@@ -1888,7 +1924,6 @@ impl FileSystemSandboxPolicy {
|
||||
let mut writable_roots = Vec::new();
|
||||
let mut tmpdir_writable = false;
|
||||
let mut slash_tmp_writable = false;
|
||||
let mut unbridgeable_root_write = false;
|
||||
|
||||
for entry in &self.entries {
|
||||
match &entry.path {
|
||||
@@ -1903,14 +1938,7 @@ impl FileSystemSandboxPolicy {
|
||||
}
|
||||
}
|
||||
FileSystemPath::Special { value } => match value {
|
||||
FileSystemSpecialPath::Root => match entry.access {
|
||||
FileSystemAccessMode::Deny => {}
|
||||
FileSystemAccessMode::Read => {}
|
||||
FileSystemAccessMode::Write => {
|
||||
unbridgeable_root_write = true;
|
||||
}
|
||||
},
|
||||
FileSystemSpecialPath::Minimal => {}
|
||||
FileSystemSpecialPath::Root | FileSystemSpecialPath::Minimal => {}
|
||||
FileSystemSpecialPath::ProjectRoots { subpath } => {
|
||||
if subpath.is_none() && entry.access.can_write() {
|
||||
workspace_root_writable = true;
|
||||
@@ -1956,11 +1984,7 @@ impl FileSystemSandboxPolicy {
|
||||
exclude_tmpdir_env_var: !tmpdir_writable,
|
||||
exclude_slash_tmp: !slash_tmp_writable,
|
||||
}
|
||||
} else if unbridgeable_root_write
|
||||
|| !writable_roots.is_empty()
|
||||
|| tmpdir_writable
|
||||
|| (cfg!(unix) && slash_tmp_writable)
|
||||
{
|
||||
} else if self.has_legacy_write_intent(cwd) {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"permissions profile requests filesystem writes outside the workspace root, which is not supported until the runtime enforces FileSystemSandboxPolicy directly",
|
||||
|
||||
@@ -113,6 +113,14 @@ fn writable_root_presence_matches_materialized_roots() {
|
||||
)]),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
policies
|
||||
.iter()
|
||||
.map(|policy| policy.has_legacy_write_intent(cwd.path()))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![false, false, false, true, true, true, false],
|
||||
);
|
||||
|
||||
for policy in policies {
|
||||
assert_eq!(
|
||||
policy.has_configured_writable_roots_with_cwd(cwd.path()),
|
||||
|
||||
Reference in New Issue
Block a user