♻️ Unify Windows security operations to handle-based API

Remove path field from SecurityDescriptor and eliminate all
path-based Win32 APIs (GetNamedSecurityInfoW/SetNamedSecurityInfoW).
All security operations now go through handle-based APIs, making
follow_links control purely a handle-opening concern.

- Fix UB in build_acl_buffer by setting Vec length after InitializeAcl
- Remove SecurityDescriptor::path field and try_from(path) constructor
- Extract apply_security_info helper to eliminate duplication
- Consolidate ACL constructors from 3 to 1 (try_from_handle)
- Unify get_facl/set_facl with follow_links parameter across platforms
- Wire set_facl nofollow into extraction path for symlink ACL restoration
- Remove inline #[cfg] from core.rs by using unified ACL API
- Add Debug derive to FileHandle
- Add broken symlink permission preservation test

Author: ChanTsune

cli/src/command/core.rs

@@ -979,7 +979,7 @@ pub(crate) fn apply_metadata(
 
         let handle = open_read_metadata(path, !meta.file_type().is_symlink())?;
         let info = file_information(handle.raw())?;
-        let sd = SecurityDescriptor::try_from_handle(handle.raw(), path)?;
+        let sd = SecurityDescriptor::try_from_handle(handle)?;
         let mode = mode_from_file_information(path, &info, meta.file_type().is_symlink());
         let user = sd.owner_sid()?;
         let group = sd.group_sid()?;
@@ -1011,14 +1011,8 @@ pub(crate) fn apply_metadata(
         if let AclStrategy::Always = keep_options.acl_strategy {
             use crate::chunk;
             use pna::RawChunk;
-            #[cfg(windows)]
-            let acl_result = if meta.file_type().is_symlink() {
-                utils::os::windows::acl::get_facl_nofollow(path)
-            } else {
-                utils::acl::get_facl(path)
-            };
-            #[cfg(not(windows))]
-            let acl_result = utils::acl::get_facl(path);
+            let follow_links = !meta.file_type().is_symlink();
+            let acl_result = utils::acl::get_facl(path, follow_links);
             match acl_result {
                 Ok(acl) => {
                     entry

cli/src/command/extract.rs

@@ -1408,7 +1408,8 @@ where
     }
     #[cfg(feature = "acl")]
     if !skip_xattr_acl {
-        restore_acls(path, item.acl()?, keep_options.acl_strategy)?;
+        let follow_links = item.header().data_kind() != DataKind::SymbolicLink;
+        restore_acls(path, item.acl()?, keep_options.acl_strategy, follow_links)?;
     }
     #[cfg(not(feature = "acl"))]
     if let AclStrategy::Always = keep_options.acl_strategy {
@@ -1474,7 +1475,12 @@ where
 /// On supported platforms, if the target filesystem does not support ACLs (e.g., FAT32),
 /// a warning is logged for that path and the operation continues.
 #[cfg(feature = "acl")]
-fn restore_acls(path: &Path, acls: Acls, acl_strategy: AclStrategy) -> io::Result<()> {
+fn restore_acls(
+    path: &Path,
+    acls: Acls,
+    acl_strategy: AclStrategy,
+    follow_links: bool,
+) -> io::Result<()> {
     #[cfg(any(
         target_os = "linux",
         target_os = "freebsd",
@@ -1495,6 +1501,7 @@ fn restore_acls(path: &Path, acls: Acls, acl_strategy: AclStrategy) -> io::Resul
                     platform,
                     entries: acl,
                 }),
+                follow_links,
             ) {
                 Ok(()) => {}
                 Err(e) if e.kind() == io::ErrorKind::Unsupported => {

cli/src/utils/os/unix/acl.rs

@@ -2,14 +2,26 @@ use crate::chunk::{Ace, AcePlatform, Acl, Flag, Identifier, OwnerType, Permissio
 use std::io;
 use std::path::Path;
 
-pub fn set_facl<P: AsRef<Path>>(path: P, acl: Acl) -> io::Result<()> {
+fn acl_option(follow_links: bool) -> Option<exacl::AclOption> {
+    (!follow_links).then_some(exacl::AclOption::SYMLINK_ACL)
+}
+
+pub fn set_facl<P: AsRef<Path>>(path: P, acl: Acl, follow_links: bool) -> io::Result<()> {
     let path = path.as_ref();
+    #[cfg(target_os = "linux")]
+    if !follow_links && path.symlink_metadata()?.file_type().is_symlink() {
+        return Ok(());
+    }
     let mut acl_entries: Vec<exacl::AclEntry> =
         acl.entries.into_iter().map(Into::into).collect::<Vec<_>>();
     #[cfg(target_os = "macos")]
     {
         use std::os::unix::fs::MetadataExt;
-        let meta = std::fs::metadata(path)?;
+        let meta = if follow_links {
+            std::fs::metadata(path)?
+        } else {
+            std::fs::symlink_metadata(path)?
+        };
 
         acl_entries = acl_entries
             .into_iter()
@@ -40,7 +52,7 @@ pub fn set_facl<P: AsRef<Path>>(path: P, acl: Acl) -> io::Result<()> {
             }
         }
         if !exist_user || !exist_group || !exist_other {
-            let facl = exacl::getfacl(path, None)?;
+            let facl = exacl::getfacl(path, acl_option(follow_links))?;
             if !exist_user {
                 acl_entries.push(
                     facl.iter()
@@ -82,11 +94,19 @@ pub fn set_facl<P: AsRef<Path>>(path: P, acl: Acl) -> io::Result<()> {
             }
         }
     }
-    exacl::setfacl(&[path], &acl_entries, None)
+    exacl::setfacl(&[path], &acl_entries, acl_option(follow_links))
 }
 
-pub fn get_facl<P: AsRef<Path>>(path: P) -> io::Result<Acl> {
-    let ace_list = exacl::getfacl(path.as_ref(), None)?;
+pub fn get_facl<P: AsRef<Path>>(path: P, follow_links: bool) -> io::Result<Acl> {
+    let path = path.as_ref();
+    #[cfg(target_os = "linux")]
+    if !follow_links && path.symlink_metadata()?.file_type().is_symlink() {
+        return Ok(Acl {
+            platform: AcePlatform::CURRENT,
+            entries: vec![],
+        });
+    }
+    let ace_list = exacl::getfacl(path, acl_option(follow_links))?;
     Ok(Acl {
         platform: AcePlatform::CURRENT,
         entries: ace_list.into_iter().map(Into::into).collect(),

cli/src/utils/os/windows/acl.rs

@@ -1,13 +1,12 @@
 use crate::{
     chunk::{self, AcePlatform, Identifier, OwnerType},
     utils::os::windows::{
-        fs::{open_read_metadata, open_write_dacl},
+        fs::{FileHandle, open_read_metadata, open_write_dacl},
         security::{SecurityDescriptor, Sid, SidType},
     },
 };
 use field_offset::offset_of;
 use std::{io, mem, path::Path, ptr::null_mut};
-use windows::Win32::Foundation::HANDLE;
 use windows::Win32::Security::{
     ACCESS_ALLOWED_ACE, ACCESS_DENIED_ACE, ACE_FLAGS, ACE_HEADER, ACL as Win32ACL, ACL_REVISION_DS,
     AddAccessAllowedAceEx, AddAccessDeniedAceEx, CONTAINER_INHERIT_ACE, GetAce, INHERIT_ONLY_ACE,
@@ -21,8 +20,13 @@ use windows::Win32::Storage::FileSystem::{
 };
 use windows::Win32::System::SystemServices::{ACCESS_ALLOWED_ACE_TYPE, ACCESS_DENIED_ACE_TYPE};
 
-pub fn set_facl<P: AsRef<Path>>(path: P, ace_list: chunk::Acl) -> io::Result<()> {
-    let acl = ACL::try_from(path.as_ref())?;
+pub fn set_facl<P: AsRef<Path>>(
+    path: P,
+    ace_list: chunk::Acl,
+    follow_links: bool,
+) -> io::Result<()> {
+    let handle = open_write_dacl(path.as_ref(), follow_links)?;
+    let acl = ACL::try_from_handle(handle)?;
     let group_sid = acl.security_descriptor.group_sid()?;
     let owner_sid = acl.security_descriptor.owner_sid()?;
     let acl_entries = ace_list
@@ -33,31 +37,9 @@ pub fn set_facl<P: AsRef<Path>>(path: P, ace_list: chunk::Acl) -> io::Result<()>
     acl.set_d_acl(&acl_entries)
 }
 
-pub fn get_facl<P: AsRef<Path>>(path: P) -> io::Result<chunk::Acl> {
-    let acl = ACL::try_from(path.as_ref())?;
-    let ace_list = acl.get_d_acl()?;
-    Ok(chunk::Acl {
-        platform: AcePlatform::Windows,
-        entries: ace_list.into_iter().map(Into::into).collect(),
-    })
-}
-
-pub fn set_facl_nofollow<P: AsRef<Path>>(path: P, ace_list: chunk::Acl) -> io::Result<()> {
-    let path = path.as_ref();
-    let handle = open_write_dacl(path, false)?;
-    let acl = ACL::try_from_handle(handle.raw(), path)?;
-    let group_sid = acl.security_descriptor.group_sid()?;
-    let owner_sid = acl.security_descriptor.owner_sid()?;
-    let acl_entries = ace_list
-        .entries
-        .into_iter()
-        .map(|it| it.into_acl_entry_with(&owner_sid, &group_sid))
-        .collect::<Vec<_>>();
-    acl.set_d_acl_by_handle(handle.raw(), &acl_entries)
-}
-
-pub fn get_facl_nofollow<P: AsRef<Path>>(path: P) -> io::Result<chunk::Acl> {
-    let acl = ACL::try_from_nofollow(path.as_ref())?;
+pub fn get_facl<P: AsRef<Path>>(path: P, follow_links: bool) -> io::Result<chunk::Acl> {
+    let handle = open_read_metadata(path.as_ref(), follow_links)?;
+    let acl = ACL::try_from_handle(handle)?;
     let ace_list = acl.get_d_acl()?;
     Ok(chunk::Acl {
         platform: AcePlatform::Windows,
@@ -74,22 +56,9 @@ pub struct ACL {
 }
 
 impl ACL {
-    pub fn try_from(path: &Path) -> io::Result<Self> {
+    pub fn try_from_handle(handle: FileHandle) -> io::Result<Self> {
         Ok(Self {
-            security_descriptor: SecurityDescriptor::try_from(path)?,
-        })
-    }
-
-    pub fn try_from_nofollow(path: &Path) -> io::Result<Self> {
-        let handle = open_read_metadata(path, false)?;
-        Ok(Self {
-            security_descriptor: SecurityDescriptor::try_from_handle(handle.raw(), path)?,
-        })
-    }
-
-    pub fn try_from_handle(handle: HANDLE, path: &Path) -> io::Result<Self> {
-        Ok(Self {
-            security_descriptor: SecurityDescriptor::try_from_handle(handle, path)?,
+            security_descriptor: SecurityDescriptor::try_from_handle(handle)?,
         })
     }
 
@@ -154,11 +123,6 @@ impl ACL {
         self.security_descriptor
             .apply(None, None, Some(buffer.as_ptr() as _))
     }
-
-    pub fn set_d_acl_by_handle(&self, handle: HANDLE, acl_entries: &[ACLEntry]) -> io::Result<()> {
-        let buffer = build_acl_buffer(acl_entries)?;
-        SecurityDescriptor::apply_by_handle(handle, None, None, Some(buffer.as_ptr() as _))
-    }
 }
 
 fn build_acl_buffer(acl_entries: &[ACLEntry]) -> io::Result<Vec<u8>> {
@@ -167,6 +131,7 @@ fn build_acl_buffer(acl_entries: &[ACLEntry]) -> io::Result<Vec<u8>> {
     let mut buffer = Vec::<u8>::with_capacity(acl_size);
     let ptr = buffer.as_mut_ptr();
     unsafe { InitializeAcl(ptr as _, acl_size as u32, ACL_REVISION_DS) }?;
+    unsafe { buffer.set_len(acl_size) };
     for ace in acl_entries {
         match ace.ace_type {
             AceType::AccessAllow => unsafe {
@@ -382,9 +347,10 @@ mod tests {
                         | chunk::Permission::EXECUTE,
                 }],
             }),
+            true,
         )
         .unwrap();
-        let acl = get_facl(file.path).unwrap();
+        let acl = get_facl(file.path, true).unwrap();
         assert_eq!(acl.entries.len(), 1);
 
         assert_eq!(
@@ -418,7 +384,7 @@ mod tests {
     fn get_acl() {
         let file = AutoRemoveFile::new("default.txt");
         file.write("default").unwrap();
-        let acl = get_facl(file.path).unwrap();
+        let acl = get_facl(file.path, true).unwrap();
         assert_ne!(acl.entries.len(), 0);
     }
 }

cli/src/utils/os/windows/fs.rs

@@ -1,6 +1,6 @@
 pub(crate) mod owner;
 
-use super::security::{SecurityDescriptor, Sid};
+use super::security::{Sid, apply_security_info};
 use crate::utils::str::encode_wide;
 use std::io;
 use std::mem::size_of;
@@ -23,6 +23,7 @@ const MODE_READ_BITS: u16 = 0o444;
 const MODE_WRITE_BITS: u16 = 0o222;
 const MODE_EXEC_BITS: u16 = 0o111;
 
+#[derive(Debug)]
 pub(crate) struct FileHandle(HANDLE);
 
 impl FileHandle {
@@ -132,7 +133,7 @@ pub(crate) fn lchown<U: Into<Sid>, G: Into<Sid>>(
     let owner_sid = owner.map(Into::into);
     let group_sid = group.map(Into::into);
     let handle = open_path(path, (READ_CONTROL | WRITE_OWNER).0, false)?;
-    SecurityDescriptor::apply_by_handle(
+    apply_security_info(
         handle.raw(),
         owner_sid.as_ref().map(Sid::as_psid),
         group_sid.as_ref().map(Sid::as_psid),
@@ -185,12 +186,14 @@ pub(crate) fn chmod(path: &Path, mode: u16) -> io::Result<()> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::utils::os::windows::security::SecurityDescriptor;
 
     #[test]
     fn file_chown() {
         let path = "chown.txt";
         std::fs::write(path, "chown").unwrap();
-        let sd = SecurityDescriptor::try_from(path.as_ref()).unwrap();
+        let handle = open_read_metadata(path.as_ref(), true).unwrap();
+        let sd = SecurityDescriptor::try_from_handle(handle).unwrap();
         lchown::<_, Sid>(path.as_ref(), Some(sd.owner_sid().unwrap()), None).unwrap();
         lchown::<Sid, _>(path.as_ref(), None, Some(sd.group_sid().unwrap())).unwrap();
         lchown(