Add a sandbox integrity runner with outcome and timing metrics (#51842)

## What changed

Export `run_integrity_checks` from `exec-server` to check whether the effective filesystem policy permits writing containment dependencies on Linux, macOS, and Windows. Run preparation and checks on a blocking worker, limit waiting to five seconds, and keep results observational without affecting execution.

Skip mismatched sandbox backends, full-disk-write policies, and `EscalatedSandboxWithRestrictions` overrides. Continue checking ordinary configured policies with root write access and deny restrictions.

Record check outcomes, preparation time, and total duration with bounded metric tags that omit filesystem paths.

## Testing

Add coverage for findings, clean results, missing dependencies, preparation errors, eligibility rules, and timeouts. Verify that timed-out workers emit no late results and that metrics contain no filesystem paths.

GitOrigin-RevId: a234f4a2f629f9bb090da622cea352c99456cb00
This commit is contained in:
iceweasel-oai
2026-10-07 21:52:20 +00:00
committed by copyberry
parent b5c37c35fd
commit c2eb1f42a0
8 changed files with 458 additions and 11 deletions
+6 -4
View File
@@ -42,10 +42,6 @@ mod rpc_server_requests;
mod rpc_timing;
mod runtime_options;
#[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))]
#[expect(
dead_code,
reason = "The integrity runner is added later in this stack."
)]
mod sandbox_integrity;
mod sandbox_selection;
mod sandboxed_file_open;
@@ -249,3 +245,9 @@ pub use server::RequestDispatchMode;
pub use server::run_main;
pub use server::run_main_with_telemetry;
pub use telemetry::ExecServerTelemetry;
#[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))]
pub use sandbox_integrity::run_integrity_checks;
#[cfg(not(any(target_os = "linux", target_os = "macos", target_os = "windows")))]
pub async fn run_integrity_checks(_request: &codex_sandboxing::SandboxExecRequest) {}
@@ -40,9 +40,9 @@ implementations. Review against this scope. Regressions in explicit policy
handling, executor context, or reporting remain actionable; this is not a blanket
exemption for false positives or false negatives.
## Open eligibility issue
## Eligibility
Approved escalations that retain only deny-read restrictions should be excluded.
The current full-disk-write guard does not exclude `:root = write` plus denials,
and `SandboxExecRequest` lacks the approval context to distinguish that escalation
from a configured weak policy. Do not infer approval from policy shape.
Skip `SandboxOverride::EscalatedSandboxWithRestrictions` before preparation or
telemetry: these approved escalations retain only deny-read restrictions.
An ordinary configured `:root = write` policy with denials is still checked.
Do not infer approval from policy shape.
@@ -1,10 +1,29 @@
//! Shared containment dependency inventory entries.
//! Shared inventory entries and filesystem facts, independent of any checker.
use codex_utils_absolute_path::AbsolutePathBuf;
use std::fs;
use std::io;
use std::path::PathBuf;
pub(super) type Dependency = (&'static str, io::Result<PathBuf>);
pub(super) struct DependencyTarget {
pub path: AbsolutePathBuf,
pub metadata: fs::Metadata,
pub resolved_path: Option<AbsolutePathBuf>,
}
impl DependencyTarget {
pub(super) fn inspect(path: io::Result<PathBuf>) -> io::Result<Self> {
let path = AbsolutePathBuf::from_absolute_path(path?)?;
Ok(Self {
metadata: fs::metadata(&path)?,
resolved_path: path.canonicalize().ok(),
path,
})
}
}
pub(super) fn codex_executable() -> Dependency {
("codex", std::env::current_exe())
}
@@ -3,6 +3,10 @@
#[cfg_attr(target_os = "windows", path = "mxc.rs")]
mod backend;
mod dependencies;
mod runner;
mod telemetry;
pub use runner::run_integrity_checks;
#[cfg(test)]
#[path = "backend_tests.rs"]
@@ -0,0 +1,86 @@
//! Prepare the policy and dependency facts, then run checks per dependency.
//! Results never affect execution. See AGENTS.md for scope and accepted approximations.
use super::backend;
use super::dependencies::DependencyTarget;
use super::telemetry::IntegrityMetrics;
use codex_otel::MetricsClient;
use codex_protocol::sandbox::SandboxOverride;
use codex_sandboxing::FileContentsChecker;
use codex_sandboxing::SandboxExecRequest;
use std::io;
use std::time::Duration;
use std::time::Instant;
const CHECK_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 5);
/// Observe executor-local containment dependencies without affecting the launch.
/// Used by both exec-server and the legacy local execution path.
pub async fn run_integrity_checks(request: &SandboxExecRequest) {
run_checks_with_metrics(request, codex_otel::global().as_ref()).await;
}
async fn run_checks_with_metrics(request: &SandboxExecRequest, metrics: Option<&MetricsClient>) {
if request.sandbox != backend::SANDBOX_TYPE
|| request.sandbox_override == SandboxOverride::EscalatedSandboxWithRestrictions
{
return;
}
let policy = request.permission_profile.file_system_sandbox_policy();
if policy.has_full_disk_write_access() {
return;
}
let telemetry = IntegrityMetrics::new(metrics, backend::NAME, &policy);
let _timer = telemetry.start_timer();
let request = request.clone();
let started_at = Instant::now();
let work = tokio::task::spawn_blocking(move || {
let mut dependencies = Vec::new();
let prepared_at = Instant::now();
let prepared = (|| {
let cwd = request.sandbox_policy_cwd.to_abs_path()?;
dependencies = backend::critical_dependencies(&request, &policy, &cwd)?;
let policy = backend::prepare_policy(&request, policy, &cwd)?;
// An empty glob expansion can remove the last write restriction.
if policy.has_full_disk_write_access() {
return Ok(None);
}
FileContentsChecker::new(&policy, &cwd).map(Some)
})();
let preparation_duration = prepared_at.elapsed();
let mut checks = Vec::new();
if matches!(&prepared, Ok(None)) {
return (preparation_duration, true, checks);
}
for (dependency, path) in dependencies {
let target = DependencyTarget::inspect(path);
let result = match (&prepared, &target) {
(Ok(Some(checker)), Ok(target)) => Some(checker.check(&target.path)),
_ => None,
};
checks.push((dependency, target, result));
}
(preparation_duration, prepared.is_ok(), checks)
});
// Timing out stops waiting; running blocking work is intentionally not cancelled.
let result = match tokio::time::timeout(CHECK_TIMEOUT, work).await {
Ok(result) => result.map_err(io::Error::other),
Err(error) => Err(io::Error::new(io::ErrorKind::TimedOut, error)),
};
match result {
Ok((duration, succeeded, checks)) => {
telemetry.record_preparation(duration, succeeded);
for (dependency, target, result) in checks {
telemetry.record_check("contents", dependency, &target, result);
}
}
Err(error) => {
telemetry.record_preparation(started_at.elapsed(), /*succeeded*/ false);
telemetry.record_check("contents", "preparation", &Err(error), /*result*/ None);
}
}
}
#[cfg(test)]
#[path = "runner_tests.rs"]
mod tests;
@@ -0,0 +1,205 @@
//! Check outcome accounting and timings without installing process-global metrics.
use super::super::backend_tests::deny_glob;
use super::super::backend_tests::sandbox_request;
use super::*;
use codex_otel::MetricsConfig;
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_sandboxing::SandboxType;
use codex_utils_absolute_path::AbsolutePathBuf;
use opentelemetry_sdk::metrics::InMemoryMetricExporter;
use opentelemetry_sdk::metrics::data::AggregatedMetrics;
use opentelemetry_sdk::metrics::data::MetricData;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
use std::fs;
#[test]
fn records_every_outcome_and_times_preparation_without_paths() -> anyhow::Result<()> {
let metrics = MetricsClient::new(
MetricsConfig::in_memory(
"test",
"sandbox-integrity",
env!("CARGO_PKG_VERSION"),
InMemoryMetricExporter::default(),
)
.with_runtime_reader(),
)?;
let temp = tempfile::tempdir()?;
let root = AbsolutePathBuf::from_absolute_path(temp.path())?.canonicalize()?;
let writable = root.join("writable");
let protected = root.join("protected");
fs::write(&writable, "fixture")?;
fs::write(&protected, "fixture")?;
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry::new(root.clone().into(), FileSystemAccessMode::Write),
FileSystemSandboxEntry::new(protected.clone().into(), FileSystemAccessMode::Read),
]);
let telemetry = IntegrityMetrics::new(Some(&metrics), "test", &policy);
let checker = FileContentsChecker::new(&policy, &root)?;
for (dependency, path) in [
("writable", Ok(writable.into_path_buf())),
("protected", Ok(protected.into_path_buf())),
("missing", Ok(root.join("missing").into_path_buf())),
("directory", Ok(root.to_path_buf())),
] {
let target = DependencyTarget::inspect(path);
let result = target
.as_ref()
.ok()
.map(|target| checker.check(&target.path));
telemetry.record_check("contents", dependency, &target, result);
}
telemetry.record_check(
"contents",
"preparation",
&DependencyTarget::inspect(Ok(root.join("writable").into_path_buf())),
/*result*/ None,
);
let runtime = tokio::runtime::Builder::new_current_thread()
.enable_time()
.max_blocking_threads(/*val*/ 1)
.build()?;
let broad_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
FileSystemAccessMode::Write,
),
FileSystemSandboxEntry::new(root.join("protected").into(), FileSystemAccessMode::Deny),
]);
let mut request = sandbox_request(&root, &broad_policy);
// The same policy is checked when configured, but skipped for an approved escalation.
runtime.block_on(run_checks_with_metrics(&request, Some(&metrics)));
request.sandbox_override = SandboxOverride::EscalatedSandboxWithRestrictions;
runtime.block_on(run_checks_with_metrics(&request, Some(&metrics)));
request.sandbox_override = SandboxOverride::NoOverride;
request.sandbox = SandboxType::None;
runtime.block_on(run_checks_with_metrics(&request, Some(&metrics)));
request.sandbox = backend::SANDBOX_TYPE;
request.permission_profile = PermissionProfile::Disabled;
runtime.block_on(run_checks_with_metrics(&request, Some(&metrics)));
let mut expected_preparations = 1;
if cfg!(any(target_os = "linux", target_os = "windows")) {
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
FileSystemAccessMode::Write,
),
deny_glob(format!("{}/absent/*.key", root.display())),
]);
request.permission_profile =
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted);
runtime.block_on(run_checks_with_metrics(&request, Some(&metrics)));
// Preparation still runs, but no dependency receives another check result.
expected_preparations += 1;
}
request = sandbox_request(&root, &policy);
runtime.block_on(async {
tokio::time::pause();
// Occupy the only worker; dropping release also unblocks it on assertion failure.
let (release, wait) = std::sync::mpsc::channel::<()>();
let (started, ready) = tokio::sync::oneshot::channel();
let blocker = tokio::task::spawn_blocking(move || {
let _ = started.send(());
let _ = wait.recv();
});
ready.await?;
let mut check = Box::pin(run_checks_with_metrics(&request, Some(&metrics)));
assert!(futures::poll!(&mut check).is_pending());
tokio::time::advance(CHECK_TIMEOUT + Duration::from_millis(/*millis*/ 1)).await;
assert!(futures::poll!(&mut check).is_ready());
drop(release);
blocker.await?;
Ok::<_, anyhow::Error>(())
})?;
// Wait for the abandoned worker before checking that it emitted no late results.
drop(runtime);
expected_preparations += 1;
let snapshot = metrics.snapshot()?;
let mut outcomes = BTreeMap::new();
let mut durations = Vec::new();
for metric in snapshot
.scope_metrics()
.flat_map(opentelemetry_sdk::metrics::data::ScopeMetrics::metrics)
{
match metric.data() {
AggregatedMetrics::U64(MetricData::Sum(sum)) => {
assert_eq!(metric.name(), "codex.sandbox_integrity.check");
for point in sum.data_points() {
let attributes: BTreeMap<_, _> = point
.attributes()
.map(|tag| (tag.key.as_str(), tag.value.as_str().into_owned()))
.collect();
assert_eq!(attributes["checker"], "contents");
if attributes["backend"] == "test"
|| attributes["error_kind"] == "timeout"
|| matches!(
attributes["dependency"].as_str(),
"codex" | "sandbox_launcher" | "preparation"
)
{
*outcomes
.entry(
["dependency", "outcome", "error_kind"]
.map(|key| attributes[key].clone()),
)
.or_insert(0) += point.value();
}
}
}
AggregatedMetrics::F64(MetricData::Histogram(histogram)) => {
assert_eq!(
histogram
.data_points()
.map(opentelemetry_sdk::metrics::data::HistogramDataPoint::count)
.sum::<u64>(),
expected_preparations
);
durations.push(metric.name().to_owned());
}
_ => panic!("unexpected metric type"),
}
}
let mut expected_outcomes = vec![
["codex", "finding", "none"],
["writable", "finding", "none"],
["protected", "clean", "none"],
["missing", "missing", "other"],
["directory", "error", "other"],
["preparation", "error", "preparation"],
["preparation", "error", "timeout"],
];
if matches!(
backend::SANDBOX_TYPE,
SandboxType::LinuxSeccomp | SandboxType::WindowsMxc
) {
expected_outcomes.push(["sandbox_launcher", "missing", "other"]);
}
assert_eq!(
outcomes,
expected_outcomes
.into_iter()
.map(|tags| (tags.map(str::to_owned), 1))
.collect::<BTreeMap<_, _>>()
);
durations.sort();
assert_eq!(
durations,
vec![
"codex.sandbox_integrity.duration_ms",
"codex.sandbox_integrity.preparation_ms"
]
);
assert!(!format!("{snapshot:?}").contains(root.as_path().to_str().expect("temporary path")));
Ok(())
}
@@ -0,0 +1,131 @@
//! Construct bounded tags from policies, dependency facts, and checker results, and emit metrics.
//! This module does not discover dependencies, inspect files, prepare policies, or run checks.
//! Without a metrics client, emission is a no-op; callers still run the checks.
use super::dependencies::DependencyTarget;
use codex_otel::MetricsClient;
use codex_otel::Timer;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_sandboxing::IntegrityFinding;
use std::fs;
use std::io;
use std::time::Duration;
pub(super) struct IntegrityMetrics<'a> {
client: Option<&'a MetricsClient>,
tags: [(&'static str, &'static str); 2],
}
impl<'a> IntegrityMetrics<'a> {
pub(super) fn new(
client: Option<&'a MetricsClient>,
backend: &'static str,
policy: &FileSystemSandboxPolicy,
) -> Self {
let has_deny_globs = policy
.entries
.iter()
.any(|entry| matches!(entry.path, FileSystemPath::GlobPattern { .. }));
Self {
client,
tags: [
("backend", backend),
("has_deny_globs", bool_tag(Some(has_deny_globs))),
],
}
}
pub(super) fn start_timer(&self) -> Option<Timer> {
self.client?
.start_timer("codex.sandbox_integrity.duration_ms", &self.tags)
.ok()
}
pub(super) fn record_preparation(&self, duration: Duration, succeeded: bool) {
let Some(client) = self.client else {
return;
};
let _ = client.record_duration(
"codex.sandbox_integrity.preparation_ms",
duration,
&[
self.tags[0],
self.tags[1],
("outcome", if succeeded { "ok" } else { "error" }),
],
);
}
/// A missing result means dependency or checker preparation failed.
pub(super) fn record_check(
&self,
checker: &'static str,
dependency: &'static str,
target: &io::Result<DependencyTarget>,
result: Option<io::Result<Option<IntegrityFinding>>>,
) {
let Some(client) = self.client else {
return;
};
let error_kind = match (target, &result) {
(Err(error), _) | (_, Some(Err(error))) => {
if error.kind() == io::ErrorKind::TimedOut {
"timeout"
} else {
"other"
}
}
(_, None) => "preparation",
(_, Some(Ok(_))) => "none",
};
let (outcome, multiple_roots) = match (target, result) {
(Err(error), _) if error.kind() == io::ErrorKind::NotFound => ("missing", None),
(_, Some(Ok(Some(finding)))) => ("finding", Some(finding.write_roots.len() > 1)),
(_, Some(Ok(None))) => ("clean", Some(false)),
_ => ("error", None),
};
let target = target.as_ref().ok();
let resolution_changed = target.and_then(|target| {
target
.resolved_path
.as_ref()
.map(|resolved| resolved != &target.path)
});
let hard_links = target.and_then(|target| multiple_hard_links(&target.metadata));
let _ = client.counter(
"codex.sandbox_integrity.check",
/*inc*/ 1,
&[
self.tags[0],
("checker", checker),
("dependency", dependency),
("outcome", outcome),
("error_kind", error_kind),
self.tags[1],
("target_resolution_changed", bool_tag(resolution_changed)),
("multiple_hard_links", bool_tag(hard_links)),
("multiple_write_roots", bool_tag(multiple_roots)),
],
);
}
}
#[cfg(unix)]
fn multiple_hard_links(metadata: &fs::Metadata) -> Option<bool> {
use std::os::unix::fs::MetadataExt;
Some(metadata.nlink() > 1)
}
#[cfg(not(unix))]
fn multiple_hard_links(_: &fs::Metadata) -> Option<bool> {
None
}
fn bool_tag(value: Option<bool>) -> &'static str {
match value {
Some(true) => "true",
Some(false) => "false",
None => "unknown",
}
}
+1 -1
View File
@@ -98,7 +98,7 @@ pub struct SandboxCommand {
/// Build this only at the execution boundary: in exec-server, or in its logical equivalent within
/// app-server. Orchestration and transport code should retain [`PathUri`] values and defer
/// conversion to native paths until this request is created.
#[derive(Debug)]
#[derive(Clone, Debug)]
pub struct SandboxExecRequest {
/// Controller-selected override, retained for observations without altering the sandbox command.
pub sandbox_override: SandboxOverride,