Import packages as encrypted folders

[tobihagemann]

Copying a package into a vault has never worked. Files hands packages to the File Provider extension as directories, and createPlaceholderItemForFile rejects every typeDirectory with folderUploadNotSupported, so an .rtfd or iWork bundle fails with "The file doesn't exist." Copying a folder that contains one is worse: the plain files land, the package is skipped, and the single error names only the enclosing folder.

This imports packages recursively as ordinary encrypted folders. Inside the vault a package looks like a folder named Design Notes.rtfd, which is what it is on disk. Copying it back out reconstitutes a working package, since a directory with a registered package extension is the package. Verified on device: copied in, browsed, copied back out, opened in Quick Look.

Packages are not presented as documents inside the vault, so they can't be opened in place. Desktop does present them, since Finder derives package-ness from the extension and a mounted vault is an ordinary volume to it. Matching that here needs a package-level aggregate: one root item, one real on-disk package directory, one package operation. Shipping only the presentation half gives you something that looks like a document and opens empty, which is worse than just showing a folder.

Flow

sequenceDiagram
  participant Files as Files.app
  participant A as FileProviderAdapter
  participant S as WorkflowScheduler
  participant C as Cloud
  Files->>A: importDocument(packageURL)
  A->>A: walk tree, copy bytes, create rows
  A-->>Files: completionHandler(root placeholder)
  A->>S: schedule createFolder for root
  S->>C: createFolder()
  C-->>S: ok
  A->>S: schedule children inside parent's continuation
  S->>C: createFolder() / uploadFile()

Loading

The ordering matters. The completion handler fires with the root placeholder before any network work, and each node's task, upload record and workflow are built inside its parent's scheduled continuation. A folder that fails never lets its descendants reach the cloud, and sibling subtrees stay independent.

Four decisions worth flagging:

• Interior collisions fail the import instead of renaming. Renaming a component inside a package changes user data a manifest or internal reference may depend on. The root still renames like any other item.
• A symlink anywhere in the package fails the import rather than being skipped. A relative link copied into an isolated item directory has no valid target, and a directory link can escape the package or cycle.
• Rollback happens only before the completion handler fires. Cannot Copy .rtfd Package Files into Vault #430 reports moving packages in, and Files deletes the source once the handler returns, so any later failure marks the folder and keeps the local bytes.
• Uploading folders lose renaming and reparenting, since the collision handler can overwrite a user rename during the remote-create window. Adding sub-items stays allowed, which is how Files copies a folder tree in.

Closes #141, closes #430.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 474fa2c1-fc2d-4faf-bf38-eeb9a1c44e9b

📥 Commits

Reviewing files that changed from the base of the PR and between 037b06c and 4250e92.

📒 Files selected for processing (1)

• CryptomatorFileProvider/PermissionProvider.swift

🚧 Files skipped from review as they are similar to previous changes (1)

• CryptomatorFileProvider/PermissionProvider.swift

---

Walkthrough

The change adds package-directory imports as encrypted folder trees. It detects package types, traverses contents, caches files, rejects symbolic links, rolls back partial imports, and schedules staged uploads. Collision handling distinguishes root renaming from failures inside imported packages. Uploading folders no longer allow renaming or reparenting during upload. Tests cover package import, collisions, rollback, mocks, and permissions.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• cryptomator/ios#459: Modifies the same cloud-task and collision-handling components to control collision retries.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary change: importing packages as encrypted folders.
Description check	✅ Passed	The description directly explains package import behavior, failure handling, and the user-facing fixes for issues [#141] and [#430].
Linked Issues check	✅ Passed	The implementation recursively imports package directories as encrypted folder trees and addresses the reported Files.app failures [#141] [#430].
Out of Scope Changes check	✅ Passed	The changes support package import, collision handling, rollback, permissions, and related test coverage without identified unrelated code.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/import-packages-as-folders

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

CryptomatorFileProvider/FileProviderAdapter.swift (1)

226-235: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Log local import failures without cleartext names.

reportLocalImportFailure is called from both single-file and package import, and logs identifier via fileURL.lastPathComponent. Package import elsewhere logs item ids for failures. Log the file extension or item id here instead of the cleartext name.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CryptomatorFileProvider/FileProviderAdapter.swift` around lines 226 - 235,
Update reportLocalImportFailure to stop logging the cleartext importing name in
both filename-collision and local-import failure messages. Log a non-sensitive
identifier instead, such as the file extension or relevant item id, while
preserving the existing error handling and completion behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CryptomatorFileProvider/FileProviderAdapter.swift`:
- Around line 544-553: Update markFolderAsUploadError to mark the failed folder
and all descendant metadata rows as .uploadError, not only the supplied
itemMetadata. Reuse the existing metadata-manager query/update mechanisms to
traverse the subtree, persist each descendant’s status, and signal updates for
every affected FileProviderItem so descendants become deletable and no longer
remain .isUploading.

In `@CryptomatorFileProvider/PermissionProvider.swift`:
- Line 31: Correct the Note documentation in PermissionProvider by replacing “an
running upload” with grammatically correct wording that clearly describes an
active folder upload or folder creation in the cloud, while preserving the
existing restrictions on renaming and reparenting.

In
`@CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterTestCase.swift`:
- Around line 92-121: The shared test doubles are not thread-safe under parallel
package execution. In
CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterTestCase.swift:92-121,
serialize virtualCloudFileSystem access in uploadFile and createFolder, and
protect CloudFileGraphHandler.createItem and getItem with a lock; in
CryptomatorFileProviderTests/Middleware/TaskExecutor/CloudTaskExecutorTestCase.swift:63-127,
use the same locking approach to guard MetadataManagerMock state:
cachedMetadata, updatedMetadata, getCloudPathForReceivedInvocations, and
persistedSnapshots.

---

Nitpick comments:
In `@CryptomatorFileProvider/FileProviderAdapter.swift`:
- Around line 226-235: Update reportLocalImportFailure to stop logging the
cleartext importing name in both filename-collision and local-import failure
messages. Log a non-sensitive identifier instead, such as the file extension or
relevant item id, while preserving the existing error handling and completion
behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a3e62fc9-3ccf-4873-a43c-f6f2c6854fe6

📥 Commits

Reviewing files that changed from the base of the PR and between 2dc295a and 037b06c.

📒 Files selected for processing (12)

• CryptomatorFileProvider/CloudTask/CloudTask.swift
• CryptomatorFileProvider/CloudTask/FolderCreationTask.swift
• CryptomatorFileProvider/FileProviderAdapter.swift
• CryptomatorFileProvider/FileProviderAdapterError.swift
• CryptomatorFileProvider/Middleware/OnlineItemNameCollisionHandler.swift
• CryptomatorFileProvider/PermissionProvider.swift
• CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterImportDirectoryTests.swift
• CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterImportDocumentTests.swift
• CryptomatorFileProviderTests/FileProviderAdapter/FileProviderAdapterTestCase.swift
• CryptomatorFileProviderTests/Middleware/OnlineItemNameCollisionHandlerTests.swift
• CryptomatorFileProviderTests/Middleware/TaskExecutor/CloudTaskExecutorTestCase.swift
• CryptomatorFileProviderTests/PermissionProviderImplTests.swift