diff --git a/frontend/src-tauri/src/extension_container.rs b/frontend/src-tauri/src/extension_container.rs index 247ea10..d45ab02 100644 --- a/frontend/src-tauri/src/extension_container.rs +++ b/frontend/src-tauri/src/extension_container.rs @@ -6,6 +6,8 @@ use windows_sys::Win32::Security::{ PSID, }; +static PACKAGE_ACL_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + pub struct Profile { name: Vec, sid: PSID, @@ -60,6 +62,18 @@ impl Profile { /// is used: every directory and file must be checked and granted separately. /// This adds an ACE; it does not sanitize pre-existing permissions. pub fn grant_package_read_execute(&self, object: &std::fs::File) -> Result<()> { + self.update_package_access(object, false) + } + /// Remove only this freshly-created instance's allowed ACEs, using the + /// original held object handle. Other principals keep their current ACLs. + pub fn revoke_package_access(&self, object: &std::fs::File) -> Result<()> { + self.update_package_access(object, true) + } + fn update_package_access(&self, object: &std::fs::File, revoke: bool) -> Result<()> { + // Serialize Host read/merge/write operations across concurrent instances. + let _lock = PACKAGE_ACL_LOCK + .lock() + .map_err(|_| HostError::new("EXTENSION_CONTAINER_ACL_FAILED"))?; use std::os::windows::{fs::MetadataExt, io::AsRawHandle}; use windows_sys::Win32::{ Foundation::LocalFree, @@ -82,12 +96,13 @@ impl Profile { let metadata = object .metadata() .map_err(|_| HostError::new("EXTENSION_CONTAINER_ACL_FAILED"))?; - if metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 - || !(metadata.is_file() || metadata.is_dir()) + if !revoke + && (metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 + || !(metadata.is_file() || metadata.is_dir())) { return Err(HostError::new("EXTENSION_CONTAINER_ACL_OBJECT_INVALID")); } - if metadata.is_file() { + if metadata.is_file() && !revoke { let mut info = BY_HANDLE_FILE_INFORMATION::default(); if unsafe { GetFileInformationByHandle(object.as_raw_handle(), &mut info) } == 0 || info.nNumberOfLinks != 1 @@ -117,7 +132,7 @@ impl Profile { } let entry = EXPLICIT_ACCESS_W { grfAccessPermissions: FILE_GENERIC_READ | FILE_GENERIC_EXECUTE, - grfAccessMode: GRANT_ACCESS, + grfAccessMode: if revoke { REVOKE_ACCESS } else { GRANT_ACCESS }, grfInheritance: 0, Trustee: TRUSTEE_W { TrusteeForm: TRUSTEE_IS_SID, @@ -217,6 +232,52 @@ impl Drop for Profile { } } +#[cfg(all(test, feature = "desktop"))] +pub(crate) fn test_acl_entries(object: &std::fs::File) -> Vec> { + use std::os::windows::io::AsRawHandle; + use windows_sys::Win32::{ + Foundation::LocalFree, + Security::{Authorization::*, *}, + }; + struct Allocation(*mut core::ffi::c_void); + impl Drop for Allocation { + fn drop(&mut self) { + unsafe { + LocalFree(self.0); + } + } + } + let mut acl = std::ptr::null_mut(); + let mut descriptor = std::ptr::null_mut(); + assert_eq!( + unsafe { + GetSecurityInfo( + object.as_raw_handle(), + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut acl, + std::ptr::null_mut(), + &mut descriptor, + ) + }, + 0 + ); + let _descriptor = Allocation(descriptor); + assert!(!acl.is_null()); + let mut entries = Vec::new(); + for index in 0..unsafe { (*acl).AceCount } { + let mut ace = std::ptr::null_mut(); + assert_ne!(unsafe { GetAce(acl, u32::from(index), &mut ace) }, 0); + let length = unsafe { (*(ace as *const ACE_HEADER)).AceSize }; + entries.push( + unsafe { std::slice::from_raw_parts(ace.cast::(), usize::from(length)) }.to_vec(), + ); + } + entries +} + #[cfg(test)] mod tests { use super::*; @@ -786,17 +847,39 @@ mod tests { // Actual native RPC: the child cannot name an identity or connect to // a shared endpoint; only its own stdio pipe reaches this broker. { - use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; - let sentinel = unsafe { - windows_sys::Win32::System::Threading::CreateEventW( - std::ptr::null(), - 1, - 0, - std::ptr::null(), - ) + use std::os::windows::io::AsRawHandle; + use windows_sys::Win32::Storage::FileSystem::{ + FileIdInfo, GetFileInformationByHandleEx, FILE_ID_INFO, }; - assert!(!sentinel.is_null()); - let sentinel = unsafe { OwnedHandle::from_raw_handle(sentinel) }; + let sentinel_dir = tempfile::tempdir().unwrap(); + let sentinel = std::fs::OpenOptions::new() + .read(true) + .write(true) + .create_new(true) + .open(sentinel_dir.path().join("host-only-sentinel")) + .unwrap(); + let mut identity: FILE_ID_INFO = unsafe { std::mem::zeroed() }; + assert_ne!( + unsafe { + GetFileInformationByHandleEx( + sentinel.as_raw_handle(), + FileIdInfo, + (&mut identity as *mut FILE_ID_INFO).cast(), + std::mem::size_of::() as u32, + ) + }, + 0 + ); + let sentinel_identity = format!( + "{}:{}", + identity.VolumeSerialNumber, + identity + .FileId + .Identifier + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::() + ); assert_ne!( unsafe { windows_sys::Win32::Foundation::SetHandleInformation( @@ -817,6 +900,7 @@ mod tests { rpc.arguments = vec![ "file_rpc".into(), (sentinel.as_raw_handle() as usize).to_string(), + sentinel_identity, ]; rpc.permissions.insert("notes.read".into()); rpc.expires_at_ms = 10_000; @@ -840,7 +924,10 @@ mod tests { let crate::extension_io::Event::Frame(request) = pump.receive(std::time::Duration::from_secs(5)).unwrap() else { - panic!("missing RPC request") + panic!( + "missing RPC request; child exit: {:?}", + running.wait(std::time::Duration::from_secs(1)) + ) }; let response = files.dispatch(&mut workspace, &request).unwrap(); pump.send(serde_json::to_vec(&response).unwrap()).unwrap(); diff --git a/frontend/src-tauri/src/extension_instance.rs b/frontend/src-tauri/src/extension_instance.rs index ecddea6..f75a5b1 100644 --- a/frontend/src-tauri/src/extension_instance.rs +++ b/frontend/src-tauri/src/extension_instance.rs @@ -355,6 +355,7 @@ impl Registry { matches!( code, "EXTENSION_CONTAINER_CLEANUP_FAILED" + | "EXTENSION_CONTAINER_ACL_REVOKE_FAILED" | "EXTENSION_RESOURCE_TERMINATE_FAILED" | "EXTENSION_INSTANCE_WORKER_FAILED" ) @@ -398,7 +399,18 @@ fn run_in_profile( profile: &Profile, ) -> Result<()> { let pinned = PinnedPackage::open(&spec.package, &spec.inventory, &spec.claims.tree_sha256)?; - pinned.grant_read_execute(profile)?; + let access = pinned.access(profile)?; + let result = run_with_access(spec, control, receiver, profile, &pinned); + access.finish()?; + result +} +fn run_with_access( + spec: LaunchSpec, + control: &Control, + receiver: Receiver, + profile: &Profile, + pinned: &PinnedPackage, +) -> Result<()> { let entry = pinned.bind_entry(&spec.claims.entry)?; let folder = profile.folder()?; let scratch = folder.join("Temp"); @@ -524,6 +536,10 @@ mod tests { .collect(); let dir = cap_std::fs::Dir::open_ambient_dir(&package, cap_std::ambient_authority()).unwrap(); + let acl_file = std::fs::File::open(&executable).unwrap(); + let acl_root = dir.try_clone().unwrap().into_std_file(); + let file_acl = crate::extension_container::test_acl_entries(&acl_file); + let root_acl = crate::extension_container::test_acl_entries(&acl_root); let inventory = || Inventory { files: files.clone(), expanded_size: bytes.len() as u64, @@ -752,6 +768,14 @@ mod tests { .unwrap(), 0 ); + assert_eq!( + crate::extension_container::test_acl_entries(&acl_file), + file_acl + ); + assert_eq!( + crate::extension_container::test_acl_entries(&acl_root), + root_acl + ); } fn channel() -> (Endpoint, Receiver) { let (commands, receiver) = mpsc::sync_channel(4); diff --git a/frontend/src-tauri/src/extension_pinned.rs b/frontend/src-tauri/src/extension_pinned.rs index c8cee63..ee0a2a9 100644 --- a/frontend/src-tauri/src/extension_pinned.rs +++ b/frontend/src-tauri/src/extension_pinned.rs @@ -15,6 +15,27 @@ pub struct PinnedPackage { files: BTreeMap, tree_sha256: String, } +/// Scoped ACL ownership, created before any mutation. Release only after all +/// instance processes/handles have closed; drop retries cleanup on error/unwind. +pub struct PackageAccess<'a> { + package: &'a PinnedPackage, + profile: &'a Profile, + active: bool, +} +impl PackageAccess<'_> { + pub fn finish(mut self) -> Result<()> { + self.package.revoke_access(self.profile)?; + self.active = false; + Ok(()) + } +} +impl Drop for PackageAccess<'_> { + fn drop(&mut self) { + if self.active { + let _ = self.package.revoke_access(self.profile); + } + } +} /// The package borrow and all ancestor handles must outlive the process using /// this path. Only files present in the verified package can produce this guard. pub struct BoundEntry<'a> { @@ -244,6 +265,49 @@ impl PinnedPackage { pub fn tree_sha256(&self) -> &str { &self.tree_sha256 } + pub fn access<'a>(&'a self, profile: &'a Profile) -> Result> { + self.access_with(profile, || self.grant_read_execute(profile)) + } + fn access_with<'a>( + &'a self, + profile: &'a Profile, + grant: impl FnOnce() -> Result<()>, + ) -> Result> { + let guard = PackageAccess { + package: self, + profile, + active: true, + }; + if let Err(error) = grant() { + guard.finish()?; + return Err(error); + } + Ok(guard) + } + fn revoke_access(&self, profile: &Profile) -> Result<()> { + let mut failed = false; + for dir in self.directories.values() { + if dir + .try_clone() + .map(|dir| dir.into_std_file()) + .map_err(HostError::from) + .and_then(|file| profile.revoke_package_access(&file)) + .is_err() + { + failed = true; + } + } + for file in self.files.values() { + if profile.revoke_package_access(file).is_err() { + failed = true; + } + } + if failed { + Err(HostError::new("EXTENSION_CONTAINER_ACL_REVOKE_FAILED")) + } else { + Ok(()) + } + } pub fn grant_read_execute(&self, profile: &Profile) -> Result<()> { for dir in self.directories.values() { profile.grant_package_read_execute(&dir.try_clone()?.into_std_file())?; @@ -347,4 +411,41 @@ mod tests { b"verified bytes" ); } + #[test] + fn scoped_access_restores_all_acl_entries_and_preserves_other_instances() { + let (_temp, root, inventory, hash) = fixture(); + let pinned = PinnedPackage::open(&root, &inventory, &hash).unwrap(); + let snapshot = || { + let mut result = Vec::new(); + for dir in pinned.directories.values() { + result.push(crate::extension_container::test_acl_entries( + &dir.try_clone().unwrap().into_std_file(), + )); + } + for file in pinned.files.values() { + result.push(crate::extension_container::test_acl_entries(file)); + } + result + }; + let before = snapshot(); + let first = Profile::create().unwrap(); + let second = Profile::create().unwrap(); + let other = pinned.access(&second).unwrap(); + let other_acl = snapshot(); + assert_ne!(other_acl, before); + let access = pinned.access(&first).unwrap(); + assert_ne!(snapshot(), other_acl); + access.finish().unwrap(); + assert_eq!(snapshot(), other_acl); + let failed = pinned.access_with(&first, || { + first.grant_package_read_execute(pinned.files.values().next().unwrap())?; + Err(HostError::new("INJECTED_PARTIAL_GRANT")) + }); + assert_eq!(failed.err().unwrap().code, "INJECTED_PARTIAL_GRANT"); + assert_eq!(snapshot(), other_acl); + drop(other); + assert_eq!(snapshot(), before); + first.remove().unwrap(); + second.remove().unwrap(); + } } diff --git a/frontend/src-tauri/tests/fixtures/sandbox_network_probe.rs b/frontend/src-tauri/tests/fixtures/sandbox_network_probe.rs index 3b8621a..523aa28 100644 --- a/frontend/src-tauri/tests/fixtures/sandbox_network_probe.rs +++ b/frontend/src-tauri/tests/fixtures/sandbox_network_probe.rs @@ -64,11 +64,25 @@ fn main() { } if args.get(1).is_some_and(|s| s == "file_rpc") { use std::io::{Read, Write}; + #[repr(C)] + struct FileIdentity { volume: u64, id: [u8; 16] } #[link(name = "kernel32")] - extern "system" { fn GetHandleInformation(handle: *mut std::ffi::c_void, flags: *mut u32) -> i32; } + extern "system" { + fn GetFileInformationByHandleEx(handle: *mut std::ffi::c_void, + class: i32, info: *mut std::ffi::c_void, size: u32) -> i32; + } let sentinel: usize = args[2].parse().unwrap(); - let mut flags = 0; - if unsafe { GetHandleInformation(sentinel as *mut _, &mut flags) } != 0 { std::process::exit(85); } + let mut identity = FileIdentity { volume: 0, id: [0; 16] }; + // Numeric handles may alias unrelated child objects. Compare the actual + // file identity without reading from a possibly aliased pipe handle. + if unsafe { GetFileInformationByHandleEx(sentinel as *mut _, 18, + (&mut identity as *mut FileIdentity).cast(), + std::mem::size_of::() as u32) } != 0 { + let hex: String = identity.id.iter().map(|b| format!("{b:02x}")).collect(); + if format!("{}:{}", identity.volume, hex) == args[3] { + std::process::exit(85); + } + } println!("{{\"method\":\"notes.read\",\"path\":\"fixture.md\"}}"); std::io::stdout().flush().unwrap(); let mut response = String::new();