fix(transport): restore private download staging

Cargo.lock

@@ -6302,6 +6302,7 @@ dependencies = [
  "tracing",
  "unicode-normalization",
  "uuid",
+ "windows-sys 0.61.2",
  "zeroize",
  "zmodem2",
 ]

crates/nyaterm-transport/Cargo.toml

@@ -33,3 +33,12 @@ unicode-normalization.workspace = true
 uuid.workspace = true
 zeroize.workspace = true
 zmodem2.workspace = true
+
+[target.'cfg(windows)'.dependencies]
+windows-sys = { version = "0.61", features = [
+    "Win32_Foundation",
+    "Win32_Security",
+    "Win32_Security_Authorization",
+    "Win32_Storage_FileSystem",
+    "Win32_System_Threading",
+] }

crates/nyaterm-transport/src/download_path.rs

@@ -1,9 +1,12 @@
 //! Download target name validation and local path boundary checks.
 
 use std::fs::{self, OpenOptions};
-use std::io::{self, Write};
+use std::io::{self, Read, Write};
 use std::path::{Component, Path, PathBuf};
 
+#[cfg(windows)]
+mod windows;
+
 /// Unlike Path::exists, treat dangling links as occupied and propagate probe errors.
 pub(crate) fn target_exists(target: &Path) -> io::Result<bool> {
     match fs::symlink_metadata(target) {
@@ -130,6 +133,59 @@ pub(crate) struct DownloadTemporary {
     replace_existing: bool,
 }
 
+struct NewTargetGuard {
+    path: PathBuf,
+    file: Option<fs::File>,
+    committed: bool,
+}
+
+impl NewTargetGuard {
+    fn create(target: &Path) -> io::Result<Self> {
+        let file = OpenOptions::new()
+            .write(true)
+            .create_new(true)
+            .open(target)?;
+        Ok(Self {
+            path: target.to_path_buf(),
+            file: Some(file),
+            committed: false,
+        })
+    }
+
+    fn file_mut(&mut self) -> &mut fs::File {
+        self.file
+            .as_mut()
+            .expect("new download target file must remain open until commit")
+    }
+
+    fn commit(mut self) -> io::Result<()> {
+        self.file_mut().sync_all()?;
+        self.file.take();
+        self.committed = true;
+        Ok(())
+    }
+}
+
+impl Drop for NewTargetGuard {
+    fn drop(&mut self) {
+        if self.committed {
+            return;
+        }
+        self.file.take();
+        let _ = fs::remove_file(&self.path);
+    }
+}
+
+fn populate_new_target<R: Read>(source: &mut R, target: &Path) -> anyhow::Result<()> {
+    use anyhow::Context as _;
+    let mut destination = NewTargetGuard::create(target)?;
+    io::copy(source, destination.file_mut())
+        .context("download commit failed while populating new target")?;
+    destination
+        .commit()
+        .context("failed to sync committed download")
+}
+
 impl DownloadTemporary {
     /// Copy the resume prefix during preparation, before entering the download loop.
     /// Any preparation failure leaves the original file intact.
@@ -166,11 +222,19 @@ impl DownloadTemporary {
             path,
             replace_existing,
         };
-        let file = OpenOptions::new()
-            .read(true)
-            .write(true)
-            .create_new(true)
-            .open(&temporary.path)?;
+        #[cfg(not(windows))]
+        let file = {
+            let mut options = OpenOptions::new();
+            options.read(true).write(true).create_new(true);
+            #[cfg(unix)]
+            {
+                use std::os::unix::fs::OpenOptionsExt as _;
+                options.mode(0o600);
+            }
+            options.open(&temporary.path)?
+        };
+        #[cfg(windows)]
+        let file = windows::create_private_file(&temporary.path)?;
         Ok((temporary, file))
     }
 
@@ -204,16 +268,7 @@ impl DownloadTemporary {
                 .context("failed to sync committed download")?;
         } else {
             let mut source = fs::File::open(&self.path)?;
-            let mut destination = OpenOptions::new()
-                .write(true)
-                .create_new(true)
-                .open(target)?;
-            use anyhow::Context as _;
-            io::copy(&mut source, &mut destination)
-                .context("download commit failed; target may contain partial data")?;
-            destination
-                .sync_all()
-                .context("failed to sync committed download")?;
+            populate_new_target(&mut source, target)?;
         }
         Ok(())
     }
@@ -409,6 +464,37 @@ mod tests {
         Ok(())
     }
 
+    struct FailingReader {
+        first: Option<&'static [u8]>,
+    }
+
+    impl std::io::Read for FailingReader {
+        fn read(&mut self, buffer: &mut [u8]) -> std::io::Result<usize> {
+            if let Some(bytes) = self.first.take() {
+                let length = bytes.len().min(buffer.len());
+                buffer[..length].copy_from_slice(&bytes[..length]);
+                return Ok(length);
+            }
+            Err(std::io::Error::other("injected read failure"))
+        }
+    }
+
+    #[test]
+    fn failed_new_target_commit_removes_partial_final_file() -> anyhow::Result<()> {
+        let root =
+            std::env::temp_dir().join(format!("nyaterm-commit-cleanup-{}", nyaterm_core::uuid()));
+        std::fs::create_dir(&root)?;
+        let target = root.join("download");
+        let mut source = FailingReader {
+            first: Some(b"partial"),
+        };
+        assert!(super::populate_new_target(&mut source, &target).is_err());
+        assert!(!target.exists());
+        assert_eq!(std::fs::read_dir(&root)?.count(), 0);
+        std::fs::remove_dir_all(root)?;
+        Ok(())
+    }
+
     #[tokio::test]
     async fn failed_resume_preparation_preserves_source_and_cleans_temporary() -> anyhow::Result<()>
     {
@@ -429,7 +515,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn temporary_prefix_is_dropped_without_committing() -> anyhow::Result<()> {
+    async fn temporary_prefix_is_private_and_dropped_without_committing() -> anyhow::Result<()> {
         let root =
             std::env::temp_dir().join(format!("nyaterm-download-prefix-{}", nyaterm_core::uuid()));
         std::fs::create_dir(&root)?;
@@ -438,6 +524,11 @@ mod tests {
         let (temporary, file) = super::DownloadTemporary::prepare_async(&root, &target, 6).await?;
         let path = temporary.path.clone();
         assert_eq!(path.parent(), Some(root.as_path()));
+        #[cfg(unix)]
+        {
+            use std::os::unix::fs::PermissionsExt as _;
+            assert_eq!(std::fs::metadata(&path)?.permissions().mode() & 0o077, 0);
+        }
         assert_eq!(std::fs::read(&path)?, b"prefix");
         drop(file);
         drop(temporary);

crates/nyaterm-transport/src/download_path/windows.rs

@@ -0,0 +1,234 @@
+//! Restrict temporary file DACLs at creation to avoid inheriting broader directory read access.
+
+use std::ffi::c_void;
+use std::fs::File;
+use std::io;
+use std::mem::size_of;
+use std::os::windows::ffi::OsStrExt as _;
+use std::os::windows::io::{AsRawHandle as _, FromRawHandle as _, OwnedHandle};
+use std::path::Path;
+use std::ptr::null_mut;
+
+use windows_sys::Win32::Foundation::{
+    ERROR_INSUFFICIENT_BUFFER, GENERIC_READ, GENERIC_WRITE, INVALID_HANDLE_VALUE, LocalFree,
+};
+use windows_sys::Win32::Security::Authorization::{
+    ConvertSidToStringSidW, ConvertStringSecurityDescriptorToSecurityDescriptorW, SDDL_REVISION_1,
+};
+use windows_sys::Win32::Security::{
+    GetTokenInformation, SECURITY_ATTRIBUTES, TOKEN_QUERY, TOKEN_USER, TokenUser,
+};
+use windows_sys::Win32::Storage::FileSystem::{
+    CREATE_NEW, CreateFileW, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_DELETE, FILE_SHARE_READ,
+    FILE_SHARE_WRITE,
+};
+use windows_sys::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken};
+
+struct LocalAllocation(*mut c_void);
+
+impl Drop for LocalAllocation {
+    fn drop(&mut self) {
+        // SAFETY: The pointer comes from a Windows conversion function requiring LocalFree.
+        unsafe {
+            LocalFree(self.0);
+        }
+    }
+}
+
+fn current_user_sid() -> io::Result<String> {
+    let mut raw = null_mut();
+    // SAFETY: The process pseudo-handle and output pointer are valid;
+    // OwnedHandle takes ownership of the token on success.
+    if unsafe { OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut raw) } == 0 {
+        return Err(io::Error::last_os_error());
+    }
+    let token = unsafe { OwnedHandle::from_raw_handle(raw) };
+    let mut length = 0;
+    unsafe {
+        GetTokenInformation(token.as_raw_handle(), TokenUser, null_mut(), 0, &mut length);
+    }
+    let error = io::Error::last_os_error();
+    if error.raw_os_error() != Some(ERROR_INSUFFICIENT_BUFFER as i32) {
+        return Err(error);
+    }
+    if (length as usize) < size_of::<TOKEN_USER>() {
+        return Err(io::ErrorKind::InvalidData.into());
+    }
+    // TOKEN_USER contains pointers; use usize for alignment and query the buffer size from Windows.
+    let mut buffer = vec![0usize; (length as usize).div_ceil(size_of::<usize>())];
+    if unsafe {
+        GetTokenInformation(
+            token.as_raw_handle(),
+            TokenUser,
+            buffer.as_mut_ptr().cast(),
+            length,
+            &mut length,
+        )
+    } == 0
+    {
+        return Err(io::Error::last_os_error());
+    }
+    let user = unsafe { &*buffer.as_ptr().cast::<TOKEN_USER>() };
+    let mut sid = null_mut();
+    if unsafe { ConvertSidToStringSidW(user.User.Sid, &mut sid) } == 0 {
+        return Err(io::Error::last_os_error());
+    }
+    let _allocation = LocalAllocation(sid.cast());
+    let mut length = 0;
+    // SAFETY: Successful conversion returns NUL-terminated UTF-16;
+    // the allocation remains alive until the copy completes.
+    unsafe {
+        while *sid.add(length) != 0 {
+            length += 1;
+        }
+        Ok(String::from_utf16_lossy(std::slice::from_raw_parts(
+            sid, length,
+        )))
+    }
+}
+
+pub(super) fn create_private_file(path: &Path) -> io::Result<File> {
+    // Specify the user SID explicitly: an elevated process's default owner may be Administrators.
+    // P disables parent ACL inheritance; grant access only to the current user and SYSTEM.
+    let user = current_user_sid()?;
+    let sddl: Vec<u16> = format!("O:{user}D:P(A;;FA;;;{user})(A;;FA;;;SY)")
+        .encode_utf16()
+        .chain(Some(0))
+        .collect();
+    let mut descriptor = null_mut();
+    if unsafe {
+        ConvertStringSecurityDescriptorToSecurityDescriptorW(
+            sddl.as_ptr(),
+            SDDL_REVISION_1,
+            &mut descriptor,
+            null_mut(),
+        )
+    } == 0
+    {
+        return Err(io::Error::last_os_error());
+    }
+    let _descriptor = LocalAllocation(descriptor);
+    let attributes = SECURITY_ATTRIBUTES {
+        nLength: size_of::<SECURITY_ATTRIBUTES>() as u32,
+        lpSecurityDescriptor: descriptor,
+        bInheritHandle: 0,
+    };
+    // Support absolute paths like std file operations; canonicalizing only the parent
+    // does not require the new file to exist.
+    let parent = path
+        .parent()
+        .unwrap_or_else(|| Path::new("."))
+        .canonicalize()?;
+    let name = path
+        .file_name()
+        .ok_or_else(|| io::Error::from(io::ErrorKind::InvalidInput))?;
+    let mut wide: Vec<u16> = parent.join(name).as_os_str().encode_wide().collect();
+    if wide.contains(&0) {
+        return Err(io::ErrorKind::InvalidInput.into());
+    }
+    wide.push(0);
+    // SAFETY: The path and security descriptor remain valid during the call;
+    // CREATE_NEW prevents overwriting an existing file.
+    let handle = unsafe {
+        CreateFileW(
+            wide.as_ptr(),
+            GENERIC_READ | GENERIC_WRITE,
+            FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+            &attributes,
+            CREATE_NEW,
+            FILE_ATTRIBUTE_NORMAL,
+            null_mut(),
+        )
+    };
+    if handle == INVALID_HANDLE_VALUE {
+        return Err(io::Error::last_os_error());
+    }
+    // SAFETY: Creation succeeded; File takes sole ownership of the handle.
+    // The security descriptor can be freed immediately.
+    Ok(unsafe { File::from_raw_handle(handle) })
+}
+
+#[cfg(test)]
+mod tests {
+    use std::io::Write as _;
+    use std::os::windows::io::AsRawHandle as _;
+    use std::ptr::null_mut;
+
+    use windows_sys::Win32::Security::Authorization::{
+        ConvertSecurityDescriptorToStringSecurityDescriptorW, GetSecurityInfo, SDDL_REVISION_1,
+        SE_FILE_OBJECT,
+    };
+    use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;
+
+    fn dacl_sddl(file: &std::fs::File) -> anyhow::Result<String> {
+        let mut descriptor = null_mut();
+        let status = unsafe {
+            GetSecurityInfo(
+                file.as_raw_handle(),
+                SE_FILE_OBJECT,
+                DACL_SECURITY_INFORMATION,
+                null_mut(),
+                null_mut(),
+                null_mut(),
+                null_mut(),
+                &mut descriptor,
+            )
+        };
+        anyhow::ensure!(status == 0, "GetSecurityInfo failed: {status}");
+        let _descriptor = super::LocalAllocation(descriptor);
+        let mut text = null_mut();
+        let mut length = 0;
+        let converted = unsafe {
+            ConvertSecurityDescriptorToStringSecurityDescriptorW(
+                descriptor,
+                SDDL_REVISION_1,
+                DACL_SECURITY_INFORMATION,
+                &mut text,
+                &mut length,
+            )
+        };
+        anyhow::ensure!(converted != 0, "{}", std::io::Error::last_os_error());
+        let _text = super::LocalAllocation(text.cast());
+        Ok(unsafe {
+            String::from_utf16_lossy(std::slice::from_raw_parts(text, length as usize - 1))
+        })
+    }
+
+    #[test]
+    fn private_file_has_protected_acl_and_exclusive_creation() -> anyhow::Result<()> {
+        let root =
+            std::env::temp_dir().join(format!("nyaterm-private-acl-{}", nyaterm_core::uuid()));
+        std::fs::create_dir(&root)?;
+        let path = root.join("download");
+        let mut file = super::create_private_file(&path)?;
+        file.write_all(b"private")?;
+        let sddl = dacl_sddl(&file)?;
+        assert!(sddl.starts_with("D:P"), "{sddl}");
+        assert_eq!(sddl.matches("(A;;FA;;;").count(), 2, "{sddl}");
+        assert!(sddl.contains(";;;SY)"), "{sddl}");
+        assert!(
+            !sddl.contains(";;;WD)") && !sddl.contains(";;;BU)"),
+            "{sddl}"
+        );
+        drop(file);
+        assert!(super::create_private_file(&path).is_err());
+        assert_eq!(std::fs::read(&path)?, b"private");
+        std::fs::remove_dir_all(root)?;
+        Ok(())
+    }
+    #[test]
+    fn committed_download_uses_normal_destination_acl() -> anyhow::Result<()> {
+        let root = std::env::temp_dir().join(format!("nyaterm-final-acl-{}", nyaterm_core::uuid()));
+        std::fs::create_dir(&root)?;
+        let reference = root.join("reference");
+        std::fs::write(&reference, b"normal")?;
+        let target = root.join("download");
+        crate::download_path::staged_write_file(&root, &target, b"downloaded")?;
+        let reference_sddl = dacl_sddl(&std::fs::File::open(&reference)?)?;
+        let target_sddl = dacl_sddl(&std::fs::File::open(&target)?)?;
+        assert_eq!(target_sddl, reference_sddl);
+        assert_eq!(std::fs::read(&target)?, b"downloaded");
+        std::fs::remove_dir_all(root)?;
+        Ok(())
+    }
+}