Fix/download path validation (#16601)

* fix: validate download destinations and peer filename rules

Reject unsafe or linked download destinations and validate empty-directory
paths before creating them. Bind overwrite approval to accepted transfer
entries and propagate receiver errors to the UI.

Validate remote listings using the peer's filename rules so Unix names
remain browsable on Windows. Reject locally unsafe basenames before
starting downloads from either the Flutter or legacy UI.

Finish the previous file before validating the next digest so rejecting
the next destination preserves the file already received.

Require confirmation before restarting interrupted downloads with stale
staging files. Preserve matching resumes and cover receiver recovery.

Retain explicit overwrite rejection for peers older than 1.1.10 that
cannot negotiate confirmation.

Signed-off-by: fufesou <linlong1266@gmail.com>

* comments

Signed-off-by: fufesou <linlong1266@gmail.com>

* fix: use async I/O for download preparation and resume

Use Tokio filesystem APIs for download setup and resume metadata checks. Run the Windows link-count query on the blocking pool and flush digest writes before returning.

Signed-off-by: fufesou <linlong1266@gmail.com>

---------

Signed-off-by: fufesou <linlong1266@gmail.com>

libs/base/src/fs.rs

@@ -27,6 +27,12 @@ use hbb_common::{
 
 static NEXT_JOB_ID: AtomicI32 = AtomicI32::new(1);
 
+pub mod download;
+
+#[cfg(test)]
+#[path = "fs/download_tests.rs"]
+mod download_tests;
+
 pub fn get_next_job_id() -> i32 {
     NEXT_JOB_ID.fetch_add(1, Ordering::SeqCst)
 }
@@ -417,6 +423,8 @@ pub struct TransferJob {
     default_overwrite_strategy: Option<bool>,
     #[serde(skip_serializing)]
     digest: FileDigest,
+    #[serde(skip)]
+    download: Option<download::DownloadState>,
 }
 
 #[derive(Debug, Default, Serialize, Deserialize, Clone)]
@@ -587,6 +595,7 @@ impl TransferJob {
             files: Vec::new(),
             total_size: 0,
             enable_overwrite_detection,
+            download: (is_remote && r#type == JobType::Generic).then(Default::default),
             ..Default::default()
         }
     }
@@ -648,6 +657,9 @@ impl TransferJob {
 
     #[inline]
     pub fn set_files(&mut self, files: Vec<FileEntry>) -> ResultType<()> {
+        if self.is_download() && !self.files.is_empty() {
+            bail!("cannot replace an accepted download file list");
+        }
         validate_transfer_file_names(&files)?;
         if let DataSource::FilePath(base) = &self.data_source {
             for file in &files {
@@ -692,7 +704,12 @@ impl TransferJob {
 
     fn resolve_entry_path(&self, base: &PathBuf, name: &str) -> Option<PathBuf> {
         if self.r#type == JobType::Generic {
-            match join_validated_path(base, name) {
+            let result = if self.is_download() {
+                download::validated_target(base, name)
+            } else {
+                join_validated_path(base, name)
+            };
+            match result {
                 Ok(path) => Some(path),
                 Err(err) => {
                     log::error!("Invalid file name in transfer job {}: {}", self.id, err);
@@ -705,6 +722,12 @@ impl TransferJob {
     }
 
     pub fn modify_time(&self) {
+        if self.is_download() {
+            if let Err(err) = self.finish_download() {
+                log::error!("Failed to finish download {}: {}", self.id, err);
+            }
+            return;
+        }
         if self.r#type == JobType::Printer {
             return;
         }
@@ -729,6 +752,9 @@ impl TransferJob {
     }
 
     pub fn remove_download_file(&self) {
+        if self.is_download() && self.data_stream.is_none() {
+            return;
+        }
         if self.r#type == JobType::Printer {
             return;
         }
@@ -761,6 +787,9 @@ impl TransferJob {
     }
 
     pub async fn write(&mut self, block: FileTransferBlock) -> ResultType<()> {
+        if self.is_download() {
+            return self.write_download(block).await;
+        }
         if block.id != self.id {
             bail!("Wrong id");
         }
@@ -1160,6 +1189,13 @@ impl TransferJob {
     }
 
     pub async fn confirm(&mut self, r: &FileTransferSendConfirmRequest) -> bool {
+        if self.is_download() {
+            self.confirm_download(r);
+            // Open receiver resume streams in write() so validation errors reach the caller.
+            if matches!(r.union, Some(file_transfer_send_confirm_request::Union::OffsetBlk(offset)) if offset > 0) {
+                return true;
+            }
+        }
         if self.file_num() != r.file_num {
             // This branch will always be hit if:
             // 1. `confirm()` is called in `ui_cm_interface.rs`

libs/base/src/fs/download.rs

@@ -0,0 +1,331 @@
+use super::{
+    get_string, is_write_need_confirmation, join_validated_path, validate_file_name_no_traversal,
+    validate_no_symlink_components, DataSource, DataStream, DigestCheckResult, TransferJob,
+};
+use crate::message_proto::{
+    file_transfer_send_confirm_request, FileEntry, FileTransferBlock, FileTransferDigest,
+    FileTransferSendConfirmRequest, FileType,
+};
+use hbb_common::{anyhow::anyhow, bail, compress::decompress, ResultType};
+use std::{
+    borrow::Cow,
+    io::ErrorKind,
+    path::{Path, PathBuf},
+};
+use tokio::{
+    fs::{self, OpenOptions},
+    io::{AsyncSeekExt, AsyncWriteExt},
+};
+
+const SINGLE_LINK_COUNT: u64 = 1;
+
+#[derive(Default, Debug)]
+pub(super) struct DownloadState {
+    pub(super) confirmation: Option<(i32, u32)>,
+}
+
+pub fn validate_listing(entries: &[FileEntry], peer_windows: bool) -> ResultType<()> {
+    for entry in entries {
+        let name = &entry.name;
+        if entry.entry_type.enum_value() == Ok(FileType::DirDrive)
+            && matches!(name.as_bytes(), [drive, b':'] if drive.is_ascii_alphabetic())
+        {
+            continue;
+        }
+        if name.is_empty()
+            || name == "."
+            || name == ".."
+            || name.contains('\0')
+            || name.contains('/')
+            || (peer_windows
+                && (name.contains('\\')
+                    || matches!(name.as_bytes(), [drive, b':', ..] if drive.is_ascii_alphabetic())))
+        {
+            bail!("invalid file name in remote directory listing: {name}");
+        }
+    }
+    Ok(())
+}
+
+pub fn validate_download_name(path: &str, peer_windows: bool) -> ResultType<()> {
+    let separator = |c| c == '/' || (peer_windows && c == '\\');
+    let path = path.trim_end_matches(separator);
+    validate_component(path.rsplit(separator).next().unwrap_or(""))
+}
+
+fn validate_component(name: &str) -> ResultType<()> {
+    validate_file_name_no_traversal(name)?;
+    if name.is_empty() || name == "." || name.contains('/') {
+        bail!("invalid download file name: {name}");
+    }
+    #[cfg(windows)]
+    if name.contains(['\\', ':', '<', '>', '"', '|', '?', '*'])
+        || name.ends_with('.')
+        || name.ends_with(' ')
+    {
+        bail!("invalid Windows download file name: {name}");
+    }
+    Ok(())
+}
+
+pub(super) fn validated_target(base: &PathBuf, name: &str) -> ResultType<PathBuf> {
+    let parent = base
+        .parent()
+        .ok_or_else(|| anyhow!("download destination has no parent"))?;
+    let leaf = base
+        .file_name()
+        .and_then(|name| name.to_str())
+        .ok_or_else(|| anyhow!("invalid download destination"))?;
+    validate_component(leaf)?;
+    #[cfg(windows)]
+    for component in name.split(['/', '\\']).filter(|part| !part.is_empty()) {
+        validate_component(component)?;
+    }
+    validate_no_symlink_components(&parent.to_path_buf(), leaf)?;
+    let target = join_validated_path(base, name)?;
+    let parent = target
+        .parent()
+        .ok_or_else(|| anyhow!("download file has no parent"))?;
+    let leaf = target
+        .file_name()
+        .and_then(|name| name.to_str())
+        .ok_or_else(|| anyhow!("invalid download file name"))?;
+    for suffix in ["download", "digest"] {
+        validate_no_symlink_components(&parent.to_path_buf(), &format!("{leaf}.{suffix}"))?;
+    }
+    Ok(target)
+}
+
+fn remove_if_exists(path: &Path) -> ResultType<()> {
+    match std::fs::remove_file(path) {
+        Ok(()) => Ok(()),
+        Err(err) if err.kind() == ErrorKind::NotFound => Ok(()),
+        Err(err) => Err(err.into()),
+    }
+}
+
+async fn remove_if_exists_async(path: &Path) -> ResultType<()> {
+    match fs::remove_file(path).await {
+        Ok(()) => Ok(()),
+        Err(err) if err.kind() == ErrorKind::NotFound => Ok(()),
+        Err(err) => Err(err.into()),
+    }
+}
+
+async fn validate_resume_file(file: &tokio::fs::File) -> ResultType<()> {
+    let metadata = file.metadata().await?;
+    if !metadata.is_file() {
+        bail!("download resume staging path is not a regular file");
+    }
+    #[cfg(unix)]
+    let links = std::os::unix::fs::MetadataExt::nlink(&metadata);
+    #[cfg(windows)]
+    let links = {
+        use std::{mem::MaybeUninit, os::windows::io::AsRawHandle};
+        use winapi::{
+            shared::minwindef::FALSE,
+            um::fileapi::{GetFileInformationByHandle, BY_HANDLE_FILE_INFORMATION},
+        };
+        let file = file.try_clone().await?;
+        tokio::task::spawn_blocking(move || -> ResultType<u64> {
+            let mut information = MaybeUninit::<BY_HANDLE_FILE_INFORMATION>::uninit();
+            if unsafe {
+                GetFileInformationByHandle(file.as_raw_handle() as _, information.as_mut_ptr())
+            } == FALSE
+            {
+                return Err(std::io::Error::last_os_error().into());
+            }
+            Ok(u64::from(
+                unsafe { information.assume_init() }.nNumberOfLinks,
+            ))
+        })
+        .await??
+    };
+    if links != SINGLE_LINK_COUNT {
+        bail!("hard-linked download resume staging file is not allowed");
+    }
+    Ok(())
+}
+
+impl TransferJob {
+    pub(super) fn is_download(&self) -> bool {
+        self.download.is_some()
+    }
+
+    pub fn download_target(&self, file_num: i32) -> ResultType<PathBuf> {
+        let entry = self
+            .files
+            .get(file_num as usize)
+            .ok_or_else(|| anyhow!("Wrong file number"))?;
+        let DataSource::FilePath(base) = &self.data_source else {
+            bail!("download destination is not a file path");
+        };
+        validated_target(base, &entry.name)
+    }
+
+    pub fn download_digest(
+        &mut self,
+        can_resume: bool,
+        digest: &FileTransferDigest,
+    ) -> ResultType<DigestCheckResult> {
+        if self.file_num != digest.file_num && self.data_stream.is_some() {
+            let result = self.finish_download();
+            self.data_stream.take();
+            result?;
+        }
+        let target = self.download_target(digest.file_num)?;
+        let resume = can_resume && self.is_resume && !target.try_exists()?;
+        let result = is_write_need_confirmation(resume, &get_string(&target), digest)?;
+        if matches!(result, DigestCheckResult::NoSuchFile)
+            && (Path::new(&format!("{}.download", target.display())).try_exists()?
+                || Path::new(&format!("{}.digest", target.display())).try_exists()?)
+        {
+            return Ok(DigestCheckResult::NeedConfirm(FileTransferDigest {
+                id: digest.id,
+                file_num: digest.file_num,
+                ..Default::default()
+            }));
+        }
+        Ok(result)
+    }
+
+    pub(super) fn confirm_download(&mut self, request: &FileTransferSendConfirmRequest) {
+        if request.id != self.id {
+            return;
+        }
+        let Some(download) = self.download.as_mut() else {
+            return;
+        };
+        download.confirmation = self.files.get(request.file_num as usize).and_then(|_| {
+            let offset = match request.union {
+                Some(file_transfer_send_confirm_request::Union::OffsetBlk(offset)) => offset,
+                Some(file_transfer_send_confirm_request::Union::Skip(false)) => 0,
+                _ => return None,
+            };
+            Some((request.file_num, offset))
+        });
+    }
+
+    fn download_confirmation_offset(&self, file_num: i32) -> Option<u32> {
+        let (confirmed, offset) = self.download.as_ref()?.confirmation.as_ref()?;
+        (*confirmed == file_num).then_some(*offset)
+    }
+
+    async fn start_download_file(&mut self, file_num: i32) -> ResultType<()> {
+        let result = self.finish_download();
+        self.data_stream.take();
+        result?;
+        let target = self.download_target(file_num)?;
+        let staging = PathBuf::from(format!("{}.download", target.display()));
+        let digest = PathBuf::from(format!("{}.digest", target.display()));
+        let confirmation = self.download_confirmation_offset(file_num);
+        if (fs::try_exists(&target).await?
+            || fs::try_exists(&staging).await?
+            || fs::try_exists(&digest).await?)
+            && confirmation.is_none()
+        {
+            bail!("download overwrite requires confirmation");
+        }
+        if let Some(offset) = confirmation.filter(|offset| *offset > 0) {
+            return self.resume_download(file_num, offset).await;
+        }
+        if let Some(parent) = target.parent() {
+            fs::create_dir_all(parent).await?;
+        }
+        remove_if_exists_async(&staging).await?;
+        let file = OpenOptions::new()
+            .write(true)
+            .create_new(true)
+            .open(&staging)
+            .await?;
+        self.file_num = file_num;
+        self.data_stream = Some(DataStream::FileStream(file));
+        remove_if_exists_async(&digest).await?;
+        let mut metadata = OpenOptions::new()
+            .write(true)
+            .create_new(true)
+            .open(&digest)
+            .await?;
+        metadata
+            .write_all(serde_json::to_string(&self.digest)?.as_bytes())
+            .await?;
+        metadata.flush().await?;
+        Ok(())
+    }
+
+    async fn resume_download(&mut self, file_num: i32, offset: u32) -> ResultType<()> {
+        let target = self.download_target(file_num)?;
+        if fs::try_exists(&target).await? {
+            bail!("download overwrite requires confirmation");
+        }
+        let digest = fs::metadata(format!("{}.digest", target.display())).await?;
+        if !digest.is_file() {
+            bail!("download resume metadata is not a regular file");
+        }
+        let mut file = OpenOptions::new()
+            .write(true)
+            .open(format!("{}.download", target.display()))
+            .await?;
+        validate_resume_file(&file).await?;
+        if file.metadata().await?.len() != u64::from(offset) {
+            bail!("download resume offset does not match staging file size");
+        }
+        file.seek(std::io::SeekFrom::Start(u64::from(offset)))
+            .await?;
+        self.file_num = file_num;
+        self.data_stream = Some(DataStream::FileStream(file));
+        self.transferred += u64::from(offset);
+        self.finished_size += u64::from(offset);
+        Ok(())
+    }
+
+    pub(super) async fn write_download(&mut self, block: FileTransferBlock) -> ResultType<()> {
+        if block.id != self.id {
+            bail!("Wrong id");
+        }
+        if block.file_num as usize >= self.files.len() {
+            bail!("Wrong file number");
+        }
+        if block.file_num != self.file_num || self.data_stream.is_none() {
+            self.start_download_file(block.file_num).await?;
+        }
+        let data = if block.compressed {
+            Cow::Owned(decompress(&block.data))
+        } else {
+            Cow::Borrowed(block.data.as_ref())
+        };
+        let Some(DataStream::FileStream(stream)) = self.data_stream.as_mut() else {
+            bail!("download file stream is missing");
+        };
+        stream.write_all(&data).await?;
+        stream.flush().await?;
+        self.finished_size += data.len() as u64;
+        self.transferred += block.data.len() as u64;
+        Ok(())
+    }
+
+    pub fn finish_download(&self) -> ResultType<()> {
+        if !self.is_download() {
+            self.modify_time();
+            return Ok(());
+        }
+        if self.data_stream.is_none() {
+            return Ok(());
+        }
+        let target = self.download_target(self.file_num)?;
+        if target.try_exists()? && self.download_confirmation_offset(self.file_num) != Some(0) {
+            bail!("download overwrite requires confirmation");
+        }
+        std::fs::rename(format!("{}.download", target.display()), &target)?;
+        remove_if_exists(&PathBuf::from(format!("{}.digest", target.display())))?;
+        let entry = self
+            .files
+            .get(self.file_num as usize)
+            .ok_or_else(|| anyhow!("Wrong file number"))?;
+        filetime::set_file_mtime(
+            &target,
+            filetime::FileTime::from_unix_time(entry.modified_time as _, 0),
+        )?;
+        Ok(())
+    }
+}

libs/base/src/fs/download_tests.rs

@@ -0,0 +1,324 @@
+use super::*;
+
+struct DownloadDir(PathBuf);
+
+impl DownloadDir {
+    fn new() -> Self {
+        static NEXT_DIRECTORY: AtomicI32 = AtomicI32::new(1);
+        let sequence = NEXT_DIRECTORY.fetch_add(1, Ordering::Relaxed);
+        let nonce = SystemTime::now()
+            .duration_since(UNIX_EPOCH)
+            .unwrap()
+            .as_nanos();
+        let path = std::env::temp_dir().join(format!(
+            "rustdesk_download_{}_{nonce}_{sequence}",
+            std::process::id()
+        ));
+        std::fs::create_dir(&path).unwrap();
+        Self(path)
+    }
+
+    fn job(&self) -> TransferJob {
+        self.job_with_files(vec![FileEntry::default()])
+    }
+
+    fn job_with_files(&self, files: Vec<FileEntry>) -> TransferJob {
+        TransferJob::new_write(
+            1,
+            JobType::Generic,
+            "/remote/file.txt".into(),
+            DataSource::FilePath(self.0.join("file.txt")),
+            0,
+            false,
+            true,
+            true,
+        )
+        .with_files(files)
+        .unwrap()
+    }
+}
+
+impl Drop for DownloadDir {
+    fn drop(&mut self) {
+        std::fs::remove_dir_all(&self.0).unwrap();
+    }
+}
+
+fn block() -> FileTransferBlock {
+    FileTransferBlock {
+        id: 1,
+        file_num: 0,
+        data: b"received".to_vec().into(),
+        ..Default::default()
+    }
+}
+
+fn approval(file_num: i32, offset: u32) -> FileTransferSendConfirmRequest {
+    FileTransferSendConfirmRequest {
+        id: 1,
+        file_num,
+        union: Some(file_transfer_send_confirm_request::Union::OffsetBlk(offset)),
+        ..Default::default()
+    }
+}
+
+#[test]
+fn download_listing_rejects_unsafe_names_and_keeps_drive_rows() {
+    for name in ["../outside", "/outside", ".", "..", "", "dir/file"] {
+        let entry = FileEntry {
+            name: name.into(),
+            ..Default::default()
+        };
+        assert!(
+            download::validate_listing(&[entry], false).is_err(),
+            "{}",
+            name
+        );
+    }
+    let entries = [
+        FileEntry {
+            name: "report.txt".into(),
+            ..Default::default()
+        },
+        FileEntry {
+            name: "report:notes".into(),
+            ..Default::default()
+        },
+        FileEntry {
+            name: "C:".into(),
+            entry_type: FileType::DirDrive.into(),
+            ..Default::default()
+        },
+    ];
+    download::validate_listing(&entries, true).unwrap();
+}
+
+#[tokio::test]
+async fn download_requires_confirmation_to_overwrite() {
+    let dir = DownloadDir::new();
+    let target = dir.0.join("file.txt");
+    std::fs::write(&target, b"original").unwrap();
+    let mut job = dir.job();
+    assert!(job.write(block()).await.is_err());
+    assert_eq!(std::fs::read(&target).unwrap(), b"original");
+    assert!(!dir.0.join("file.txt.download").exists());
+
+    job.confirm(&approval(0, 0)).await;
+    job.write(block()).await.unwrap();
+    assert!(job.set_files(vec![FileEntry::default()]).is_err());
+    job.finish_download().unwrap();
+    assert_eq!(std::fs::read(&target).unwrap(), b"received");
+}
+
+#[tokio::test]
+async fn download_requires_separate_confirmation_for_each_existing_file() {
+    let dir = DownloadDir::new();
+    let mut job = dir.job_with_files(vec![
+        FileEntry {
+            name: "first.txt".into(),
+            ..Default::default()
+        },
+        FileEntry {
+            name: "second.txt".into(),
+            ..Default::default()
+        },
+    ]);
+    job.write(block()).await.unwrap();
+    let second = dir.0.join("file.txt/second.txt");
+    std::fs::write(&second, b"original").unwrap();
+    assert!(job
+        .write(FileTransferBlock {
+            file_num: 1,
+            ..block()
+        })
+        .await
+        .is_err());
+    assert_eq!(std::fs::read(&second).unwrap(), b"original");
+    assert_eq!(
+        std::fs::read(dir.0.join("file.txt/first.txt")).unwrap(),
+        b"received"
+    );
+    job.confirm(&approval(1, 0)).await;
+    job.write(FileTransferBlock {
+        file_num: 1,
+        ..block()
+    })
+    .await
+    .unwrap();
+    job.finish_download().unwrap();
+    assert_eq!(std::fs::read(&second).unwrap(), b"received");
+}
+
+#[tokio::test]
+async fn download_preserves_received_files_when_publication_fails() {
+    for next_is_digest in [false, true] {
+        let dir = DownloadDir::new();
+        let files = ["first.txt", "second.txt"]
+            .iter()
+            .map(|&name| FileEntry {
+                name: name.into(),
+                ..Default::default()
+            })
+            .collect();
+        let mut job = dir.job_with_files(files);
+        job.write(block()).await.unwrap();
+        let target = dir.0.join("file.txt/first.txt");
+        std::fs::write(&target, b"existing").unwrap();
+        if next_is_digest {
+            assert!(job
+                .download_digest(
+                    true,
+                    &FileTransferDigest {
+                        id: 1,
+                        file_num: 1,
+                        ..Default::default()
+                    },
+                )
+                .is_err());
+        } else {
+            assert!(job
+                .write(FileTransferBlock {
+                    file_num: 1,
+                    ..block()
+                })
+                .await
+                .is_err());
+        }
+        job.remove_download_file();
+        assert_eq!(std::fs::read(target).unwrap(), b"existing");
+        assert_eq!(
+            std::fs::read(dir.0.join("file.txt/first.txt.download")).unwrap(),
+            b"received"
+        );
+        assert!(dir.0.join("file.txt/first.txt.digest").exists());
+    }
+}
+
+#[tokio::test]
+async fn download_recovers_interrupted_transfer() {
+    for (resuming, changed) in [(false, false), (true, true), (true, false)] {
+        let dir = DownloadDir::new();
+        let mut digest = FileTransferDigest {
+            id: 1,
+            file_size: block().data.len() as _,
+            last_modified: 1,
+            ..Default::default()
+        };
+        let mut interrupted = dir.job();
+        interrupted.set_digest(digest.file_size, digest.last_modified);
+        let partial = FileTransferBlock {
+            data: b"rec".to_vec().into(),
+            ..block()
+        };
+        interrupted.write(partial).await.unwrap();
+        drop(interrupted);
+
+        let mut job = dir.job();
+        job.is_resume = resuming;
+        digest.last_modified += u64::from(changed);
+        job.set_digest(digest.file_size, digest.last_modified);
+        let confirmation = match job.download_digest(true, &digest).unwrap() {
+            DigestCheckResult::NeedConfirm(confirmation) => confirmation,
+            _ => panic!("interrupted download must require confirmation"),
+        };
+        let offset = if resuming && !changed {
+            b"rec".len()
+        } else {
+            0
+        };
+        assert_eq!(confirmation.is_identical, offset > 0);
+        assert_eq!(confirmation.transferred_size, offset as u64);
+        assert!(job.write(block()).await.is_err());
+        assert_eq!(
+            std::fs::read(dir.0.join("file.txt.download")).unwrap(),
+            b"rec"
+        );
+        job.confirm(&approval(0, offset as _)).await;
+        job.write(FileTransferBlock {
+            data: block().data.slice(offset..),
+            ..block()
+        })
+        .await
+        .unwrap();
+        job.finish_download().unwrap();
+        assert_eq!(std::fs::read(dir.0.join("file.txt")).unwrap(), b"received");
+    }
+}
+
+#[tokio::test]
+async fn download_does_not_publish_or_remove_unreceived_staging_files() {
+    let dir = DownloadDir::new();
+    let target = dir.0.join("file.txt");
+    let staging = dir.0.join("file.txt.download");
+    std::fs::write(&target, b"original").unwrap();
+    std::fs::write(&staging, b"another transfer").unwrap();
+    let job = dir.job();
+    job.modify_time();
+    job.remove_download_file();
+    assert_eq!(std::fs::read(&target).unwrap(), b"original");
+    assert_eq!(std::fs::read(&staging).unwrap(), b"another transfer");
+
+    let metadata_only = DownloadDir::new();
+    let metadata = metadata_only.0.join("file.txt.digest");
+    std::fs::write(&metadata, b"unrelated file").unwrap();
+    assert!(metadata_only.job().write(block()).await.is_err());
+    assert_eq!(std::fs::read(metadata).unwrap(), b"unrelated file");
+}
+
+#[test]
+#[cfg(windows)]
+fn download_rejects_windows_streams_and_ambiguous_components() {
+    let dir = DownloadDir::new();
+    for name in ["nested/file:stream", "nested/.. /outside", "nested/file."] {
+        assert!(
+            download::validated_target(&dir.0.join("docs"), name).is_err(),
+            "{}",
+            name
+        );
+    }
+}
+
+#[tokio::test]
+#[cfg_attr(windows, ignore = "requires symlink privilege")]
+async fn download_refuses_symlink_destination_directory() {
+    let dir = DownloadDir::new();
+    let outside = dir.0.join("outside");
+    let link = dir.0.join("file.txt");
+    std::fs::create_dir_all(&outside).unwrap();
+    #[cfg(unix)]
+    std::os::unix::fs::symlink(&outside, &link).unwrap();
+    #[cfg(windows)]
+    std::os::windows::fs::symlink_dir(&outside, &link).unwrap();
+    let mut job = dir.job_with_files(vec![FileEntry {
+        name: "child.txt".into(),
+        ..Default::default()
+    }]);
+    assert!(job.write(block()).await.is_err());
+    job.modify_time();
+    assert!(!outside.join("child.txt").exists());
+    assert!(!outside.join("child.txt.download").exists());
+}
+
+#[tokio::test]
+#[cfg_attr(windows, ignore = "requires symlink privilege")]
+async fn download_refuses_linked_target_and_staging_files() {
+    for suffix in ["", ".download", ".digest"] {
+        let dir = DownloadDir::new();
+        let outside = dir.0.join("outside.txt");
+        let link = dir.0.join(format!("file.txt{suffix}"));
+        std::fs::write(&outside, b"original").unwrap();
+        #[cfg(unix)]
+        std::os::unix::fs::symlink(&outside, &link).unwrap();
+        #[cfg(windows)]
+        std::os::windows::fs::symlink_file(&outside, &link).unwrap();
+        let mut job = dir.job();
+        assert!(job.write(block()).await.is_err());
+        job.modify_time();
+        job.remove_download_file();
+        assert_eq!(std::fs::read(&outside).unwrap(), b"original");
+        assert!(std::fs::symlink_metadata(link)
+            .unwrap()
+            .file_type()
+            .is_symlink());
+    }
+}