From 6b0e4f120bf65f702eca096107c1a3d81f8fa2b8 Mon Sep 17 00:00:00 2001 From: Mansur Azatbek Date: Wed, 23 Sep 2026 16:53:08 +0500 Subject: [PATCH 1/2] fix(windows): isolate concurrent filesystem read grants Co-authored-by: codex --- .../cageforge-windows/src/filesystem/acl.rs | 10 +- .../tests/windows_backend.rs | 179 ++++++++++++++++++ 2 files changed, 184 insertions(+), 5 deletions(-) diff --git a/crates/cageforge-windows/src/filesystem/acl.rs b/crates/cageforge-windows/src/filesystem/acl.rs index fe18eed..a07ec0a 100644 --- a/crates/cageforge-windows/src/filesystem/acl.rs +++ b/crates/cageforge-windows/src/filesystem/acl.rs @@ -74,7 +74,6 @@ pub(crate) struct FilesystemAclEnforcement { } pub(crate) struct FilesystemAuthorities { - read_base_sid: String, profile_guard_sid: String, write_root_sids: BTreeMap, token_sids: Vec, @@ -475,7 +474,6 @@ impl FilesystemAuthorities { token_sids.sort_unstable(); token_sids.dedup(); Ok(Self { - read_base_sid, profile_guard_sid, write_root_sids, token_sids, @@ -560,7 +558,9 @@ impl<'plan> AclPlanBuilder<'plan> { FilesystemPlanAccess::ReadRoot => { let entries = vec![ AclEntry::allow(self.group_sid, READ_ALLOW_MASK), - AclEntry::allow(&self.authorities.read_base_sid, READ_ALLOW_MASK), + // The installation-wide read SID is also present in unrelated + // concurrent sandboxes. Bind policy reads to this profile. + AclEntry::allow(&self.authorities.profile_guard_sid, READ_ALLOW_MASK), ]; self.insert_foundation(path, entries, true, self.inherited_write_sids(path)); } @@ -572,7 +572,7 @@ impl<'plan> AclPlanBuilder<'plan> { })?; let entries = vec![ AclEntry::allow(self.group_sid, WRITE_ALLOW_MASK), - AclEntry::allow(&self.authorities.read_base_sid, READ_ALLOW_MASK), + AclEntry::allow(&self.authorities.profile_guard_sid, READ_ALLOW_MASK), AclEntry::allow(write_sid, WRITE_ALLOW_MASK), ]; self.insert_foundation( @@ -640,7 +640,7 @@ impl<'plan> AclPlanBuilder<'plan> { directory, vec![ AclEntry::allow(self.group_sid, WRITE_ALLOW_MASK), - AclEntry::allow(&self.authorities.read_base_sid, READ_ALLOW_MASK), + AclEntry::allow(&self.authorities.profile_guard_sid, READ_ALLOW_MASK), AclEntry::allow(write_sid, WRITE_ALLOW_MASK), ], true, diff --git a/crates/cageforge-windows/tests/windows_backend.rs b/crates/cageforge-windows/tests/windows_backend.rs index 4d8a995..49582fb 100644 --- a/crates/cageforge-windows/tests/windows_backend.rs +++ b/crates/cageforge-windows/tests/windows_backend.rs @@ -1275,6 +1275,51 @@ fn sandbox_process_fixture() { std::thread::sleep(Duration::from_secs(2)); fs::write(marker, b"escaped").expect("write descendant escape marker"); } + "cross-session-read-guard" => { + let secret = + PathBuf::from(std::env::var_os(SANDBOX_FIXTURE_DENIED_READ).expect("secret path")); + let trigger = + PathBuf::from(std::env::var_os(SANDBOX_FIXTURE_MARKER).expect("trigger path")); + let progress = + PathBuf::from(std::env::var_os(SANDBOX_FIXTURE_PROGRESS).expect("progress path")); + for phase in 1..=3 { + let deadline = Instant::now() + Duration::from_secs(60); + while fs::read_to_string(&trigger).ok().as_deref() + != Some(match phase { + 1 => "1", + 2 => "2", + _ => "3", + }) + { + assert!( + Instant::now() < deadline, + "read-guard trigger timed out at phase {phase}" + ); + thread::sleep(Duration::from_millis(10)); + } + let access = match fs::read(&secret) { + Ok(_) => "allowed", + Err(error) if error.kind() == io::ErrorKind::PermissionDenied => "denied", + Err(error) => panic!("unexpected read error at phase {phase}: {error}"), + }; + fs::write(&progress, format!("{phase}:{access}")) + .expect("write read-guard progress"); + } + } + "cross-session-read-grant" => { + let secret = + PathBuf::from(std::env::var_os(SANDBOX_FIXTURE_DENIED_READ).expect("secret path")); + let ready = PathBuf::from(std::env::var_os(SANDBOX_FIXTURE_READY).expect("ready path")); + let marker = + PathBuf::from(std::env::var_os(SANDBOX_FIXTURE_MARKER).expect("release path")); + assert_eq!(fs::read(&secret).expect("granted file read"), b"secret"); + fs::write(&ready, b"ready").expect("write grant-ready marker"); + let deadline = Instant::now() + Duration::from_secs(60); + while !marker.exists() { + assert!(Instant::now() < deadline, "read-grant release timed out"); + thread::sleep(Duration::from_millis(10)); + } + } "sleep" => std::thread::sleep(Duration::from_secs(30)), other => panic!("unknown Windows sandbox fixture mode: {other}"), } @@ -1514,6 +1559,140 @@ fn restricted_command_can_start_a_native_runtime_program() { drop(cleanup); } +#[test] +fn concurrent_read_grant_does_not_widen_an_existing_sandbox() { + let _setup_test_guard = setup_test_lock(); + let temporary = tempfile::tempdir().expect("temporary state"); + let config = WindowsSetupConfig::new() + .with_state_directory(temporary.path().join("state")) + .expect("absolute state directory") + .with_setup_helper_path(PathBuf::from(env!("CARGO_BIN_EXE_cageforge-windows-setup"))) + .expect("setup helper") + .with_command_runner_path(PathBuf::from(env!( + "CARGO_BIN_EXE_cageforge-windows-command-runner" + ))) + .expect("command runner"); + let setup = WindowsSetup::new(config); + let cleanup = SetupCleanup { + setup: &setup, + armed: true, + }; + setup.install().expect("install read-isolation setup"); + let backend = WindowsBackend::new( + WindowsBackendConfig::new() + .with_setup(setup.config().clone()) + .with_default_timeout(Duration::from_secs(90)) + .expect("bounded probe"), + ) + .expect("read-isolation backend"); + let workspace = tempfile::tempdir().expect("workspace"); + let resources = tempfile::tempdir().expect("external resources"); + let secret = resources.path().join("secret.txt"); + fs::write(&secret, b"secret").expect("secret fixture"); + let fixture = workspace.path().join("read-isolation-probe.exe"); + fs::copy(std::env::current_exe().expect("test executable"), &fixture) + .expect("copy read-isolation fixture"); + let trigger = workspace.path().join("guardian-trigger"); + let progress = workspace.path().join("guardian-progress"); + let ready = workspace.path().join("grant-ready"); + let release = workspace.path().join("grant-release"); + let common_rules = || { + vec![ + FilesystemRule::new(PathSelector::minimal(), AccessMode::Read), + FilesystemRule::new(PathSelector::workspace_root(), AccessMode::Write), + ] + }; + let guardian_env = EnvironmentSpec::inherit_core() + .with_var(SANDBOX_FIXTURE_MODE, "cross-session-read-guard") + .expect("guardian mode") + .with_var(SANDBOX_FIXTURE_DENIED_READ, secret.as_os_str()) + .expect("secret path") + .with_var(SANDBOX_FIXTURE_MARKER, trigger.as_os_str()) + .expect("guardian trigger") + .with_var(SANDBOX_FIXTURE_PROGRESS, progress.as_os_str()) + .expect("guardian progress"); + let (guardian_request, guardian_effective, guardian_context) = + request_with_filesystem_environment( + workspace.path(), + FilesystemPolicy::restricted(common_rules()), + NetworkPolicy::disabled(), + fixture_command(&fixture), + guardian_env, + ); + let prepared = backend + .prepare( + BackendRequest::new(&guardian_request, &guardian_effective), + &guardian_context, + ) + .expect("prepare guardian"); + let mut guardian = backend.spawn(prepared).expect("spawn guardian"); + let check_guardian = |phase: u8| { + fs::write(&trigger, phase.to_string()).expect("signal guardian"); + let expected = format!("{phase}:denied"); + let leaked = format!("{phase}:allowed"); + let deadline = Instant::now() + Duration::from_secs(30); + loop { + let observed = fs::read_to_string(&progress).unwrap_or_default(); + assert_ne!(observed, leaked, "concurrent grant widened the guardian"); + if observed == expected { + break; + } + assert!( + Instant::now() < deadline, + "guardian phase {phase} timed out: {observed}" + ); + thread::sleep(Duration::from_millis(10)); + } + }; + check_guardian(1); + + let mut grant_rules = common_rules(); + grant_rules.push(FilesystemRule::new( + PathSelector::absolute(&secret).expect("absolute secret"), + AccessMode::Read, + )); + let grant_env = EnvironmentSpec::inherit_core() + .with_var(SANDBOX_FIXTURE_MODE, "cross-session-read-grant") + .expect("grant mode") + .with_var(SANDBOX_FIXTURE_DENIED_READ, secret.as_os_str()) + .expect("secret path") + .with_var(SANDBOX_FIXTURE_READY, ready.as_os_str()) + .expect("ready path") + .with_var(SANDBOX_FIXTURE_MARKER, release.as_os_str()) + .expect("release path"); + let (grant_request, grant_effective, grant_context) = request_with_filesystem_environment( + workspace.path(), + FilesystemPolicy::restricted(grant_rules), + NetworkPolicy::disabled(), + fixture_command(&fixture), + grant_env, + ); + let prepared = backend + .prepare( + BackendRequest::new(&grant_request, &grant_effective), + &grant_context, + ) + .expect("prepare grant"); + let mut granted = backend.spawn(prepared).expect("spawn concurrent grant"); + let deadline = Instant::now() + Duration::from_secs(30); + while !ready.exists() { + assert!( + Instant::now() < deadline, + "concurrent grant never became ready" + ); + thread::sleep(Duration::from_millis(10)); + } + check_guardian(2); + fs::write(&release, b"release").expect("release grant"); + assert!(granted.wait().expect("wait grant").success()); + drop(granted); + check_guardian(3); + assert!(guardian.wait().expect("wait guardian").success()); + drop(guardian); + drop(backend); + drop(cleanup); +} + #[test] fn explicit_read_file_root_supports_launch_and_exact_acl_cleanup() { let _setup_test_guard = setup_test_lock(); From a2956671831dd2ecec5438959cf555f69ad6c82a Mon Sep 17 00:00:00 2001 From: Mansur Azatbek Date: Wed, 23 Sep 2026 16:58:22 +0500 Subject: [PATCH 2/2] fix(windows): separate read grants from deny authority Co-authored-by: codex --- .../cageforge-windows/src/capability/state.rs | 2 + .../cageforge-windows/src/filesystem/acl.rs | 37 +++++++++++++------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/crates/cageforge-windows/src/capability/state.rs b/crates/cageforge-windows/src/capability/state.rs index 3c5ed79..c482b51 100644 --- a/crates/cageforge-windows/src/capability/state.rs +++ b/crates/cageforge-windows/src/capability/state.rs @@ -130,6 +130,7 @@ pub(crate) struct PersistedDacl { pub(crate) enum CapabilityRole { ProfileGuard, WriteRoot, + ReadProfile, } #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] @@ -570,6 +571,7 @@ pub(crate) fn authority_key( let role = match role { CapabilityRole::ProfileGuard => 0, CapabilityRole::WriteRoot => 1, + CapabilityRole::ReadProfile => 2, }; (profile_sha256.to_string(), role, NativePathKey::new(path)) } diff --git a/crates/cageforge-windows/src/filesystem/acl.rs b/crates/cageforge-windows/src/filesystem/acl.rs index a07ec0a..2d15e03 100644 --- a/crates/cageforge-windows/src/filesystem/acl.rs +++ b/crates/cageforge-windows/src/filesystem/acl.rs @@ -75,6 +75,7 @@ pub(crate) struct FilesystemAclEnforcement { pub(crate) struct FilesystemAuthorities { profile_guard_sid: String, + read_profile_sid: String, write_root_sids: BTreeMap, token_sids: Vec, } @@ -443,10 +444,16 @@ impl FilesystemAuthorities { filesystem: &FilesystemPlan, state: &mut CapabilityStateSession<'_>, ) -> Result { - let mut declarations = vec![( - filesystem.profile_anchor().to_path_buf(), - CapabilityRole::ProfileGuard, - )]; + let mut declarations = vec![ + ( + filesystem.profile_anchor().to_path_buf(), + CapabilityRole::ProfileGuard, + ), + ( + filesystem.profile_anchor().to_path_buf(), + CapabilityRole::ReadProfile, + ), + ]; let mut write_roots = Vec::new(); for target in filesystem.targets() { if target.access() == FilesystemPlanAccess::WriteRoot { @@ -456,7 +463,7 @@ impl FilesystemAuthorities { } } let sids = state.ensure_authorities(filesystem.profile_sha256(), declarations)?; - let expected = write_roots.len() + 1; + let expected = write_roots.len() + 2; if sids.len() != expected { return Err(FilesystemAclError::AuthorityCount { expected, @@ -464,17 +471,23 @@ impl FilesystemAuthorities { }); } let profile_guard_sid = sids[0].clone(); + let read_profile_sid = sids[1].clone(); let mut write_root_sids = BTreeMap::new(); - for (path, sid) in write_roots.into_iter().zip(sids.into_iter().skip(1)) { + for (path, sid) in write_roots.into_iter().zip(sids.into_iter().skip(2)) { write_root_sids.insert(NativePathKey::new(&path), sid); } let read_base_sid = state.read_base_sid()?; - let mut token_sids = vec![read_base_sid.clone(), profile_guard_sid.clone()]; + let mut token_sids = vec![ + read_base_sid, + profile_guard_sid.clone(), + read_profile_sid.clone(), + ]; token_sids.extend(write_root_sids.values().cloned()); token_sids.sort_unstable(); token_sids.dedup(); Ok(Self { profile_guard_sid, + read_profile_sid, write_root_sids, token_sids, }) @@ -558,9 +571,9 @@ impl<'plan> AclPlanBuilder<'plan> { FilesystemPlanAccess::ReadRoot => { let entries = vec![ AclEntry::allow(self.group_sid, READ_ALLOW_MASK), - // The installation-wide read SID is also present in unrelated - // concurrent sandboxes. Bind policy reads to this profile. - AclEntry::allow(&self.authorities.profile_guard_sid, READ_ALLOW_MASK), + // The installation-wide read SID is shared by concurrent sandboxes. + // Use a profile-specific SID distinct from the deny guard. + AclEntry::allow(&self.authorities.read_profile_sid, READ_ALLOW_MASK), ]; self.insert_foundation(path, entries, true, self.inherited_write_sids(path)); } @@ -572,7 +585,7 @@ impl<'plan> AclPlanBuilder<'plan> { })?; let entries = vec![ AclEntry::allow(self.group_sid, WRITE_ALLOW_MASK), - AclEntry::allow(&self.authorities.profile_guard_sid, READ_ALLOW_MASK), + AclEntry::allow(&self.authorities.read_profile_sid, READ_ALLOW_MASK), AclEntry::allow(write_sid, WRITE_ALLOW_MASK), ]; self.insert_foundation( @@ -640,7 +653,7 @@ impl<'plan> AclPlanBuilder<'plan> { directory, vec![ AclEntry::allow(self.group_sid, WRITE_ALLOW_MASK), - AclEntry::allow(&self.authorities.profile_guard_sid, READ_ALLOW_MASK), + AclEntry::allow(&self.authorities.read_profile_sid, READ_ALLOW_MASK), AclEntry::allow(write_sid, WRITE_ALLOW_MASK), ], true,