Merge 9f47eab75f into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-05-11 22:39:06 -07:00
committed by GitHub
3 changed files with 93 additions and 28 deletions

View File

@@ -97,9 +97,9 @@ pub struct ExecParams {
/// Resolved filesystem overrides for the Windows sandbox backends.
///
/// The unelevated restricted-token backend only consumes extra deny-write
/// carveouts on top of the legacy `WorkspaceWrite` allow set. The elevated
/// backend can also consume explicit read and write roots during setup/refresh.
/// The unelevated restricted-token backend consumes explicit write roots and
/// extra deny-write carveouts on top of the legacy `WorkspaceWrite` shape. The
/// elevated backend can also consume explicit read roots during setup/refresh.
/// Read-root overrides are layered on top of the baseline helper roots that the
/// elevated setup path needs to launch the sandboxed command. Split policies
/// that opt into platform defaults carry that explicitly with the override.
@@ -617,11 +617,11 @@ async fn exec_windows_sandbox(
.collect::<Vec<_>>()
})
.unwrap_or_default();
let elevated_read_roots_override = windows_sandbox_filesystem_overrides
let read_roots_override = windows_sandbox_filesystem_overrides
.and_then(|overrides| overrides.read_roots_override.clone());
let elevated_read_roots_include_platform_defaults = windows_sandbox_filesystem_overrides
let read_roots_include_platform_defaults = windows_sandbox_filesystem_overrides
.is_some_and(|overrides| overrides.read_roots_include_platform_defaults);
let elevated_write_roots_override = windows_sandbox_filesystem_overrides
let write_roots_override = windows_sandbox_filesystem_overrides
.and_then(|overrides| overrides.write_roots_override.clone());
let elevated_deny_write_paths = windows_sandbox_filesystem_overrides
.map(|overrides| {
@@ -645,10 +645,9 @@ async fn exec_windows_sandbox(
timeout_ms,
use_private_desktop: windows_sandbox_private_desktop,
proxy_enforced,
read_roots_override: elevated_read_roots_override.as_deref(),
read_roots_include_platform_defaults:
elevated_read_roots_include_platform_defaults,
write_roots_override: elevated_write_roots_override.as_deref(),
read_roots_override: read_roots_override.as_deref(),
read_roots_include_platform_defaults,
write_roots_override: write_roots_override.as_deref(),
deny_write_paths_override: &elevated_deny_write_paths,
},
)
@@ -661,6 +660,7 @@ async fn exec_windows_sandbox(
&cwd,
env,
timeout_ms,
write_roots_override.as_deref(),
&additional_deny_write_paths,
windows_sandbox_private_desktop,
)
@@ -1085,12 +1085,18 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides(
.map(|root| normalize_windows_override_path(root.root.as_path()))
.collect::<std::result::Result<_, _>>()?;
if legacy_root_paths != split_root_paths {
let write_roots_override = if legacy_root_paths == split_root_paths {
None
} else if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. })
&& !split_root_paths.iter().any(|path| path.parent().is_none())
{
Some(split_root_paths.iter().cloned().collect::<Vec<_>>())
} else {
return Err(
"windows unelevated restricted-token sandbox cannot enforce split writable root sets directly; refusing to run unsandboxed"
.to_string(),
);
}
};
for writable_root in &split_writable_roots {
for read_only_subpath in &writable_root.read_only_subpaths {
@@ -1112,22 +1118,18 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides(
let mut additional_deny_write_paths = BTreeSet::new();
for split_root in &split_writable_roots {
let split_root_path = normalize_windows_override_path(split_root.root.as_path())?;
let Some(legacy_root) = legacy_writable_roots.iter().find(|candidate| {
let legacy_root = legacy_writable_roots.iter().find(|candidate| {
normalize_windows_override_path(candidate.root.as_path())
.is_ok_and(|candidate_path| candidate_path == split_root_path)
}) else {
return Err(
"windows unelevated restricted-token sandbox cannot enforce split writable root sets directly; refusing to run unsandboxed"
.to_string(),
);
};
});
for read_only_subpath in &split_root.read_only_subpaths {
if !legacy_root
.read_only_subpaths
.iter()
.any(|candidate| candidate == read_only_subpath)
{
if !legacy_root.is_some_and(|legacy_root| {
legacy_root
.read_only_subpaths
.iter()
.any(|candidate| candidate == read_only_subpath)
}) {
additional_deny_write_paths.insert(normalize_windows_override_path(
read_only_subpath.as_path(),
)?);
@@ -1135,14 +1137,14 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides(
}
}
if additional_deny_write_paths.is_empty() {
if write_roots_override.is_none() && additional_deny_write_paths.is_empty() {
return Ok(None);
}
Ok(Some(WindowsSandboxFilesystemOverrides {
read_roots_override: None,
read_roots_include_platform_defaults: false,
write_roots_override: None,
write_roots_override,
additional_deny_write_paths: additional_deny_write_paths
.into_iter()
.map(|path| AbsolutePathBuf::from_absolute_path(path).map_err(|err| err.to_string()))

View File

@@ -664,6 +664,56 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() {
);
}
#[test]
fn windows_restricted_token_supports_extra_split_writable_root_override() {
let temp_dir = tempfile::TempDir::new().expect("tempdir");
let cwd = dunce::canonicalize(temp_dir.path())
.expect("canonicalize temp dir")
.abs();
let extra_root = cwd.join("extra-root");
std::fs::create_dir_all(extra_root.as_path()).expect("create extra root");
let policy = SandboxPolicy::WorkspaceWrite {
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};
let file_system_policy = FileSystemSandboxPolicy::restricted(vec![
codex_protocol::permissions::FileSystemSandboxEntry {
path: codex_protocol::permissions::FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::Root,
},
access: codex_protocol::permissions::FileSystemAccessMode::Read,
},
codex_protocol::permissions::FileSystemSandboxEntry {
path: codex_protocol::permissions::FileSystemPath::Path { path: cwd.clone() },
access: codex_protocol::permissions::FileSystemAccessMode::Write,
},
codex_protocol::permissions::FileSystemSandboxEntry {
path: codex_protocol::permissions::FileSystemPath::Path {
path: extra_root.clone(),
},
access: codex_protocol::permissions::FileSystemAccessMode::Write,
},
]);
assert_eq!(
resolve_windows_restricted_token_filesystem_overrides(
SandboxType::WindowsRestrictedToken,
&policy,
&file_system_policy,
NetworkSandboxPolicy::Restricted,
&cwd,
WindowsSandboxLevel::RestrictedToken,
),
Ok(Some(WindowsSandboxFilesystemOverrides {
read_roots_override: None,
read_roots_include_platform_defaults: false,
write_roots_override: Some(vec![cwd.to_path_buf(), extra_root.to_path_buf()]),
additional_deny_write_paths: vec![],
}))
);
}
#[test]
fn windows_elevated_supports_split_restricted_read_roots() {
let temp_dir = tempfile::TempDir::new().expect("tempdir");

View File

@@ -254,6 +254,7 @@ mod windows_impl {
use super::acl::revoke_ace;
use super::allow::AllowDenyPaths;
use super::allow::compute_allow_paths;
use super::allow::protected_child_deny_paths_for_roots;
use super::cap::load_or_create_cap_sids;
use super::cap::workspace_cap_sid_for_cwd;
use super::logging::log_failure;
@@ -339,6 +340,7 @@ mod windows_impl {
cwd,
env_map,
timeout_ms,
None,
&[],
use_private_desktop,
)
@@ -353,6 +355,7 @@ mod windows_impl {
cwd: &Path,
mut env_map: HashMap<String, String>,
timeout_ms: Option<u64>,
write_roots_override: Option<&[PathBuf]>,
additional_deny_write_paths: &[PathBuf],
use_private_desktop: bool,
) -> Result<CaptureResult> {
@@ -421,8 +424,18 @@ mod windows_impl {
}
let persist_aces = is_workspace_write;
let AllowDenyPaths { allow, mut deny } =
compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
let AllowDenyPaths { allow, mut deny } = if let Some(write_roots) = write_roots_override {
AllowDenyPaths {
allow: write_roots
.iter()
.filter(|path| path.exists())
.cloned()
.collect(),
deny: protected_child_deny_paths_for_roots(write_roots, sandbox_policy_cwd),
}
} else {
compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map)
};
for path in additional_deny_write_paths {
if path.exists() {
deny.insert(path.clone());