diff --git a/crates/buzz-agent/src/auth.rs b/crates/buzz-agent/src/auth.rs index f6825f822c..5a803a211e 100644 --- a/crates/buzz-agent/src/auth.rs +++ b/crates/buzz-agent/src/auth.rs @@ -2864,7 +2864,6 @@ mod tests { // A world-readable cache left by an older buzz-agent must be tightened // the moment we load it — on the plain cache-hit path, with no refresh // or save. Otherwise the pre-bug population stays exposed indefinitely. - // On Windows a plain `fs::write` already inherits the directory's ACL. let dir = tempfile::tempdir().unwrap(); let cfg = PkceOAuthConfig { discovery_url: "https://example.com/.well-known".into(), @@ -2895,6 +2894,8 @@ mod tests { fs::set_permissions(&cache_path, fs::Permissions::from_mode(0o644)).unwrap(); fs::metadata(&cache_path).unwrap().ino() }; + #[cfg(windows)] + buzz_private_file::grant_everyone_read(&cache_path); // Constructing the source loads the cache — repair happens here. let source = PkceOAuthTokenSource::new(cfg).unwrap(); diff --git a/crates/buzz-private-file/Cargo.toml b/crates/buzz-private-file/Cargo.toml index d5ffb36869..4f8e145c53 100644 --- a/crates/buzz-private-file/Cargo.toml +++ b/crates/buzz-private-file/Cargo.toml @@ -8,7 +8,7 @@ repository.workspace = true description = "Owner-only files on Windows, keeping Win32 security FFI out of buzz-agent" [features] -# `assert_owner_only`, for other crates' tests. +# `assert_owner_only` and `grant_everyone_read`, for other crates' tests. test-support = [] [target.'cfg(windows)'.dependencies] diff --git a/crates/buzz-private-file/src/lib.rs b/crates/buzz-private-file/src/lib.rs index f604735803..0db761eff2 100644 --- a/crates/buzz-private-file/src/lib.rs +++ b/crates/buzz-private-file/src/lib.rs @@ -49,11 +49,11 @@ const OWNER_ONLY: &str = "D:P(A;;FA;;;OW)"; /// removes. #[allow(unsafe_code)] pub fn create_new(path: &Path) -> io::Result { - let descriptor = owner_only()?; + let owner_only = descriptor(OWNER_ONLY)?; let path = verbatim(path)?; let attributes = SECURITY_ATTRIBUTES { nLength: size_of::() as u32, - lpSecurityDescriptor: descriptor.0, + lpSecurityDescriptor: owner_only.0, bInheritHandle: 0, }; // SAFETY: `path` is NUL-terminated and `attributes` points at a descriptor @@ -102,7 +102,7 @@ pub fn open_owned(path: &Path) -> io::Result { "file is not owned by the current user", )); } - restrict_to_owner(&file)?; + set_dacl(&file, OWNER_ONLY)?; Ok(file) } @@ -117,10 +117,10 @@ impl Drop for Local { } } -/// [`OWNER_ONLY`] as a security descriptor. +/// An SDDL string as a security descriptor. #[allow(unsafe_code)] -fn owner_only() -> io::Result { - let sddl: Vec = OWNER_ONLY.encode_utf16().chain(Some(0)).collect(); +fn descriptor(sddl: &str) -> io::Result { + let sddl: Vec = sddl.encode_utf16().chain(Some(0)).collect(); let mut descriptor = null_mut(); // SAFETY: `sddl` is NUL-terminated; on success Windows allocates `descriptor`. let ok = unsafe { @@ -137,10 +137,11 @@ fn owner_only() -> io::Result { Ok(Local(descriptor)) } -/// Replace the DACL of a handle opened with `WRITE_DAC` by [`OWNER_ONLY`]. +/// Replace the DACL of a handle opened with `WRITE_DAC` by the protected DACL +/// in `sddl`. #[allow(unsafe_code)] -fn restrict_to_owner(file: &File) -> io::Result<()> { - let descriptor = owner_only()?; +fn set_dacl(file: &File, sddl: &str) -> io::Result<()> { + let descriptor = descriptor(sddl)?; let (mut present, mut dacl, mut defaulted) = (0, null_mut(), 0); // SAFETY: `descriptor` is valid, and `dacl` points into it. if unsafe { GetSecurityDescriptorDacl(descriptor.0, &mut present, &mut dacl, &mut defaulted) } @@ -332,6 +333,22 @@ pub fn assert_owner_only(file: &File, why: &str) { assert_eq!(aces, "A;;FA;;;OW)", "{why}: DACL is not owner-only: {sddl}"); } +/// Test support: also let Everyone read `path`, and check that it took, so a +/// test can show that [`open_owned`] repairs a loose file. +#[cfg(any(test, feature = "test-support"))] +#[track_caller] +pub fn grant_everyone_read(path: &Path) { + use windows_sys::Win32::Storage::FileSystem::READ_CONTROL; + + let file = OpenOptions::new() + .access_mode(READ_CONTROL | WRITE_DAC) + .open(path) + .unwrap(); + set_dacl(&file, "D:P(A;;FA;;;OW)(A;;FR;;;WD)").unwrap(); + let sddl = dacl(&file); + assert!(sddl.contains(";;;WD)"), "Everyone cannot read: {sddl}"); +} + #[cfg(test)] mod tests { use super::*; @@ -369,10 +386,11 @@ mod tests { } #[test] - fn open_owned_tightens_inherited_permissions() { + fn open_owned_tightens_loose_permissions() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("token.json"); std::fs::write(&path, "legacy").unwrap(); + grant_everyone_read(&path); let file = open_owned(&path).unwrap(); assert_owner_only(&file, "tightened file"); assert_eq!(read(file), "legacy");