From 2fb91b61cbae33375f619762dc1dfcddc210d2ec Mon Sep 17 00:00:00 2001
From: Dal Rupnik <legoless@gmail.com>
Date: Mon, 8 Jun 2026 09:29:12 +0200
Subject: [PATCH] fix(drop): propagate directory-read failures during drop copy
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`copyTree`/`copyInto` treated `contentsOfDirectory` errors as an empty
listing (`try? … ?? []`), so a permission/I/O failure inside a dropped
folder was silently ignored and the drop reported success — producing an
incomplete copy in the guest with no warning (data-lossy for users who
expect the whole folder to transfer).

Let the read error propagate instead. copyTree's do/catch already removes
the partial `dst` and rethrows, so an unreadable subtree now aborts the
drop and cleans up rather than half-completing.

Adds a regression test covering an unreadable nested directory.
---
 Sources/tart/DropProgressCopier.swift         | 15 +++++---
 Tests/TartTests/DropProgressCopierTests.swift | 34 +++++++++++++++++++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/Sources/tart/DropProgressCopier.swift b/Sources/tart/DropProgressCopier.swift
index 83b5478..0da8318 100644
--- a/Sources/tart/DropProgressCopier.swift
+++ b/Sources/tart/DropProgressCopier.swift
@@ -81,9 +81,13 @@ enum DropProgressCopier {
         try fm.createDirectory(at: dst, withIntermediateDirectories: true)
         // Deterministic depth-first walk so the destination tree mirrors the
         // source and directories are created before their contents.
-        let children = (try? fm.contentsOfDirectory(
+        // Surface (don't swallow) read failures: a permission/I/O error here
+        // means an incomplete copy, so let it propagate to the do/catch below,
+        // which removes the partial `dst` and rethrows rather than reporting a
+        // silent success.
+        let children = try fm.contentsOfDirectory(
           at: src, includingPropertiesForKeys: walkKeys, options: []
-        )) ?? []
+        )
         for child in children.sorted(by: { $0.path < $1.path }) {
           if token.isCancelled { throw DropCopyCancelled() }
           let childDst = dst.appendingPathComponent(child.lastPathComponent)
@@ -113,9 +117,12 @@ enum DropProgressCopier {
     let values = try? src.resourceValues(forKeys: walkKeySet)
     if values?.isDirectory == true {
       try fm.createDirectory(at: dst, withIntermediateDirectories: true)
-      let children = (try? fm.contentsOfDirectory(
+      // Propagate read failures so an unreadable nested directory aborts the
+      // drop (cleaned up by copyTree's catch) instead of silently producing a
+      // partial tree in the guest.
+      let children = try fm.contentsOfDirectory(
         at: src, includingPropertiesForKeys: walkKeys, options: []
-      )) ?? []
+      )
       for child in children.sorted(by: { $0.path < $1.path }) {
         if token.isCancelled { throw DropCopyCancelled() }
         try copyInto(child, dst.appendingPathComponent(child.lastPathComponent), &copied, token, report)
diff --git a/Tests/TartTests/DropProgressCopierTests.swift b/Tests/TartTests/DropProgressCopierTests.swift
index 8cf8e78..2b77aba 100644
--- a/Tests/TartTests/DropProgressCopierTests.swift
+++ b/Tests/TartTests/DropProgressCopierTests.swift
@@ -119,4 +119,38 @@ final class DropProgressCopierTests: XCTestCase {
     let src = try write("sized.bin", 4242)
     XCTAssertEqual(DropProgressCopier.totalSize(of: src), 4242)
   }
+
+  func testUnreadableNestedDirectoryThrowsAndRemovesPartial() throws {
+    // A nested directory whose contents can't be listed (permission denied)
+    // must abort the copy and clean up — not silently produce a partial tree
+    // in the guest and report success.
+    try XCTSkipIf(getuid() == 0, "root bypasses POSIX permissions; can't exercise the denied-read path")
+
+    let srcDir = tmp.appendingPathComponent("locked-bundle", isDirectory: true)
+    let sealed = srcDir.appendingPathComponent("sealed", isDirectory: true)
+    try FileManager.default.createDirectory(at: sealed, withIntermediateDirectories: true)
+    try Data(repeating: 0x41, count: 10).write(to: srcDir.appendingPathComponent("a.txt"))
+    try Data(repeating: 0x41, count: 20).write(to: sealed.appendingPathComponent("secret.txt"))
+
+    // Drop read/exec on the nested dir so contentsOfDirectory(at:) fails.
+    // Restore perms before the suite's tmp teardown so cleanup can recurse in
+    // (teardown blocks run LIFO, so this runs before the setUp cleanup).
+    try FileManager.default.setAttributes([.posixPermissions: 0], ofItemAtPath: sealed.path)
+    addTeardownBlock {
+      try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: sealed.path)
+    }
+
+    let dst = tmp.appendingPathComponent("locked-copy", isDirectory: true)
+    XCTAssertThrowsError(
+      try DropProgressCopier.copyTree(
+        from: srcDir, to: dst, totalBytes: 30, token: DropCancellationToken()
+      ) { _ in }
+    ) { error in
+      XCTAssertFalse(error is DropCopyCancelled, "should surface the read error, not a cancellation")
+    }
+    XCTAssertFalse(
+      FileManager.default.fileExists(atPath: dst.path),
+      "an unreadable nested directory must not leave a partial tree in the drop zone"
+    )
+  }
 }