From 4fdfc261ede73eadac7b20c4099415b0c56be4dd Mon Sep 17 00:00:00 2001 From: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz> Date: Wed, 30 Sep 2026 12:28:40 -0600 Subject: [PATCH] test(buzz-agent): check the Windows cache DACL in-process On the Windows runner every assert_owner_only call parsed empty output from a powershell Get-Acl child whose status and stderr it discarded, so the three save and load owner-only tests failed without saying why. Move the helper crate's in-process DACL reader out of its test module behind a test-support feature, and have buzz-agent's Windows arm call it. The tests no longer shell out, both crates share one SDDL check, and buzz-agent still has no unsafe code. Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz> --- crates/buzz-agent/Cargo.toml | 3 + crates/buzz-agent/src/auth.rs | 20 +---- crates/buzz-private-file/Cargo.toml | 4 + crates/buzz-private-file/src/lib.rs | 114 +++++++++++++++------------- 4 files changed, 68 insertions(+), 73 deletions(-) diff --git a/crates/buzz-agent/Cargo.toml b/crates/buzz-agent/Cargo.toml index a00d419eee..76487b38f9 100644 --- a/crates/buzz-agent/Cargo.toml +++ b/crates/buzz-agent/Cargo.toml @@ -77,6 +77,9 @@ nix = { version = "0.31", default-features = false, features = ["signal", "proce [target.'cfg(windows)'.dependencies] buzz-private-file = { path = "../buzz-private-file" } +[target.'cfg(windows)'.dev-dependencies] +buzz-private-file = { path = "../buzz-private-file", features = ["test-support"] } + [dev-dependencies] # Synthetic TLS OAuth/catalog servers exercise the strict HTTPS production path. rcgen = { version = "0.14", default-features = false, features = ["crypto", "ring"] } diff --git a/crates/buzz-agent/src/auth.rs b/crates/buzz-agent/src/auth.rs index aa69efd77b..f6825f822c 100644 --- a/crates/buzz-agent/src/auth.rs +++ b/crates/buzz-agent/src/auth.rs @@ -2731,25 +2731,7 @@ mod tests { assert_eq!(mode, 0o600, "{why}: got {mode:o}"); } #[cfg(windows)] - { - // SDDL, unlike `icacls` output, is not localized. - let out = std::process::Command::new("powershell") - .args([ - "-NoProfile", - "-Command", - "(Get-Acl -LiteralPath $env:ACL_PATH).Sddl", - ]) - .env("ACL_PATH", path) - .output() - .unwrap(); - let sddl = String::from_utf8_lossy(&out.stdout); - let dacl = sddl.trim().split_once("D:").map_or("", |(_, dacl)| dacl); - let (flags, aces) = dacl.split_once('(').unwrap_or_default(); - assert!( - flags.contains('P') && aces == "A;;FA;;;OW)", - "{why}: got {sddl}" - ); - } + buzz_private_file::assert_owner_only(&fs::File::open(path).unwrap(), why); } #[tokio::test] diff --git a/crates/buzz-private-file/Cargo.toml b/crates/buzz-private-file/Cargo.toml index 3d2ce5fcc4..3a5517e3a9 100644 --- a/crates/buzz-private-file/Cargo.toml +++ b/crates/buzz-private-file/Cargo.toml @@ -7,6 +7,10 @@ license.workspace = true 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. +test-support = [] + [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.61", features = [ "Win32_Foundation", diff --git a/crates/buzz-private-file/src/lib.rs b/crates/buzz-private-file/src/lib.rs index 016cb703f7..8bd5b23d16 100644 --- a/crates/buzz-private-file/src/lib.rs +++ b/crates/buzz-private-file/src/lib.rs @@ -237,62 +237,68 @@ fn verbatim(path: &Path) -> io::Result> { Ok(prefix.encode_utf16().chain(wide).chain(Some(0)).collect()) } +/// The file's DACL as SDDL, such as `D:P(A;;FA;;;OW)`. +#[cfg(any(test, feature = "test-support"))] +#[allow(unsafe_code)] +fn dacl(file: &File) -> String { + use windows_sys::Win32::Security::Authorization::ConvertSecurityDescriptorToStringSecurityDescriptorW; + + let mut descriptor = null_mut(); + // SAFETY: the handle is open; on success Windows allocates `descriptor`. + let status = unsafe { + GetSecurityInfo( + file.as_raw_handle(), + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + null_mut(), + null_mut(), + null_mut(), + null_mut(), + &mut descriptor, + ) + }; + win32(status).unwrap(); + let descriptor = Local(descriptor); + let (mut sddl, mut len) = (null_mut(), 0); + // SAFETY: `descriptor` is valid; on success Windows allocates `sddl` + // holding `len` UTF-16 units. + let ok = unsafe { + ConvertSecurityDescriptorToStringSecurityDescriptorW( + descriptor.0, + SDDL_REVISION_1, + DACL_SECURITY_INFORMATION, + &mut sddl, + &mut len, + ) + }; + assert_ne!(ok, 0, "{}", io::Error::last_os_error()); + let _sddl = Local(sddl.cast()); + // SAFETY: `sddl` holds `len` UTF-16 units. + let units = unsafe { std::slice::from_raw_parts(sddl, len as usize) }; + String::from_utf16_lossy(units) + .trim_end_matches('\0') + .to_owned() +} + +/// Test support: panic with `why` unless `file` has the owner-only DACL +/// [`create_new`] sets. Reads the DACL in-process, so tests need no shell. +#[cfg(any(test, feature = "test-support"))] +#[track_caller] +pub fn assert_owner_only(file: &File, why: &str) { + let sddl = dacl(file); + let (flags, aces) = sddl + .strip_prefix("D:") + .and_then(|rest| rest.split_once('(')) + .unwrap_or_else(|| panic!("{why}: unexpected DACL {sddl}")); + assert!(flags.contains('P'), "{why}: DACL is not protected: {sddl}"); + assert_eq!(aces, "A;;FA;;;OW)", "{why}: DACL is not owner-only: {sddl}"); +} + #[cfg(test)] mod tests { use super::*; use std::io::{Read, Write}; use std::process::Command; - use windows_sys::Win32::Security::Authorization::ConvertSecurityDescriptorToStringSecurityDescriptorW; - - /// The file's DACL as SDDL, such as `D:P(A;;FA;;;OW)`. - #[allow(unsafe_code)] - fn dacl(file: &File) -> String { - let mut descriptor = null_mut(); - // SAFETY: the handle is open; on success Windows allocates `descriptor`. - let status = unsafe { - GetSecurityInfo( - file.as_raw_handle(), - SE_FILE_OBJECT, - DACL_SECURITY_INFORMATION, - null_mut(), - null_mut(), - null_mut(), - null_mut(), - &mut descriptor, - ) - }; - win32(status).unwrap(); - let descriptor = Local(descriptor); - let (mut sddl, mut len) = (null_mut(), 0); - // SAFETY: `descriptor` is valid; on success Windows allocates `sddl` - // holding `len` UTF-16 units. - let ok = unsafe { - ConvertSecurityDescriptorToStringSecurityDescriptorW( - descriptor.0, - SDDL_REVISION_1, - DACL_SECURITY_INFORMATION, - &mut sddl, - &mut len, - ) - }; - assert_ne!(ok, 0, "{}", io::Error::last_os_error()); - let _sddl = Local(sddl.cast()); - // SAFETY: `sddl` holds `len` UTF-16 units. - let units = unsafe { std::slice::from_raw_parts(sddl, len as usize) }; - String::from_utf16_lossy(units) - .trim_end_matches('\0') - .to_owned() - } - - fn assert_owner_only(file: &File) { - let sddl = dacl(file); - let (flags, aces) = sddl - .strip_prefix("D:") - .and_then(|rest| rest.split_once('(')) - .unwrap_or_else(|| panic!("unexpected DACL {sddl}")); - assert!(flags.contains('P'), "DACL is not protected: {sddl}"); - assert_eq!(aces, "A;;FA;;;OW)", "DACL is not owner-only: {sddl}"); - } fn read(mut file: File) -> String { let mut body = String::new(); @@ -306,7 +312,7 @@ mod tests { let path = dir.path().join("token.json"); let mut file = create_new(&path).unwrap(); file.write_all(b"secret").unwrap(); - assert_owner_only(&file); + assert_owner_only(&file, "new file"); drop(file); let err = create_new(&path).unwrap_err(); @@ -321,7 +327,7 @@ mod tests { std::fs::create_dir_all(&parent).unwrap(); let path = parent.join("token.json"); create_new(&path).unwrap(); - assert_owner_only(&open_owned(&path).unwrap()); + assert_owner_only(&open_owned(&path).unwrap(), "long path"); } #[test] @@ -330,7 +336,7 @@ mod tests { let path = dir.path().join("token.json"); std::fs::write(&path, "legacy").unwrap(); let file = open_owned(&path).unwrap(); - assert_owner_only(&file); + assert_owner_only(&file, "tightened file"); assert_eq!(read(file), "legacy"); }