feat(metadata): implement Windows ACLs via GetSecurityInfo/SetSecurityInfo (#1866)#3533
Merged
Conversation
…yInfo (#1866) Adds a real Windows ACL apply path that uses Win32 GetNamedSecurityInfoW/SetNamedSecurityInfoW so --acls/-A preserves NTFS discretionary ACLs in cross-platform transfers. Replaces the prior no-op stub which only emitted a warning on Windows. The new module reads/writes DACL_SECURITY_INFORMATION only (SACL is deliberately left as follow-up to avoid SE_SECURITY_NAME privilege requirements). SID/uid mapping follows upstream's lossy cross-platform convention: senders encode the resolved account name and use the SID's lower sub-authority (RID) as the synthetic uid/gid; receivers look up the SID by account name and drop ACEs that cannot be mapped, mirroring acls.c:902-928. Wires into the existing acl module dispatch via lib.rs cfg gating so non-Windows builds remain unchanged.
Adds the windows crate to the metadata Cargo.lock entry to match the target-gated dependency added in the parent commit. Without this, cargo --locked (used by MSRV and fmt+clippy CI checks) fails with "the lock file needs to be updated but --locked was passed".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1866.
Summary
crates/metadata/src/acl_windows.rsthat uses Win32GetNamedSecurityInfoW/SetNamedSecurityInfoWto preserve NTFS DACLs when--acls/-Ais in effect, replacing the prioracl_noopstub on Windows.DACL_SECURITY_INFORMATIONonly; SACL preservation, inheritance flag round-tripping, and protected DACL bits are intentionally deferred (they requireSE_SECURITY_NAMEprivilege and broader policy decisions).acls.c:902-928): senders resolve the account name withLookupAccountSidWand use the SID's lower sub-authority (RID) as the synthetic uid/gid; receivers look up the SID by account name withLookupAccountNameWand drop ACEs that cannot be mapped, emitting a one-shot warning so operators can audit lossy applications.aclmodule dispatch (get_rsync_acl,sync_acls,apply_acls_from_cache) vialib.rscfggating:cfg(all(feature = "acl", windows))selects the new module, theacl_noopfallback is no longer chosen on Windows, and thewindowscrate dependency is target-gated so non-Windows builds are unchanged.Audit findings
mapping_win.rs(a 49-line user/group name stub) and CLI/protocol wiring. It did not implementGetSecurityInfo/SetSecurityInfo. On Windows,acl_noopwas active and merely emitted a warning that ACLs were not preserved.cfg(windows). CI exercises the cross-platform compile gate via the existing Windows stable matrix.Test plan
cargo fmt --all -- --checkcfg(windows)and the gatedread_dacl_on_temp_file_returns_dacl/sync_acls_round_trips_on_ntfsintegration testsOut of scope / follow-ups
SE_SECURITY_NAMEprivilege handling).LookupAccountSidWcalls on large transfers.