feat: Phase 11 — Windows Desktop (WinFsp virtual filesystem)#189
Merged
Conversation
Phase 11: Windows Desktop - Implementation decisions documented - Phase boundary established - Scope split: Windows-only (Linux deferred to Phase 11.3) - WinFsp chosen over ProjFS for filesystem integration - Full macOS feature parity on day one Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 11: Windows Desktop - WinFsp Rust crate API and FileSystemContext trait mapped to fuser callbacks - Platform abstraction architecture documented (shared core + cfg modules) - NSIS installer hooks for WinFsp driver bundling - Windows-specific pitfalls catalogued (mount point, interior mutability, special files) - CI/CD with GitHub Actions windows-latest runner - Shared vs platform-specific code analysis from existing macOS codebase Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 11: Windows Desktop - 3 plans in 3 waves - Plan 01: Platform abstraction layer (FileAttrs, cross-platform types, Cargo.toml winfsp dep) - Plan 02: WinFsp FileSystemContext implementation (all callbacks, mount/unmount) - Plan 03: Platform branching + Tauri NSIS packaging + CI Windows build - Ready for execution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address 2 blockers, 3 warnings, 2 info items from plan verification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cture - Add FileAttrs struct replacing fuser::FileAttr in InodeData (always available) - Add to_fuse_attr() conversion method (cfg-gated to fuse feature) - Add winfsp feature flag with winfsp + widestring dependencies - Move libc to cfg(unix) target-only dependency - Add WinFsp delayload linking in build.rs for Windows - Add windows-native feature to keyring crate - Gate InodeTable methods with any(fuse, winfsp) for cross-platform use - Remove uid/gid from core data structures (injected at operations layer) - Update all tests to use FileAttrs instead of fuser::FileAttr Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add AccessMode enum replacing POSIX libc O_RDONLY/O_WRONLY/O_RDWR flags - Update file_handle to use AccessMode, remove libc dependency - Gate shared CipherBoxFS types under cfg(any(fuse, winfsp)) - Add to_fuse_attr() conversion at operations boundary with uid/gid injection - Replace ttl_for(FileType) with ttl_for_is_dir(bool) for platform independence - Make decrypt helpers accessible to both fuse and winfsp features - Gate mount_filesystem to fuse-only, unmount_filesystem to macOS-only Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - Task 1: Platform-agnostic inode types and WinFsp build infrastructure - Task 2: Platform-agnostic file_handle and operations layer updates SUMMARY: .planning/phases/11-windows-desktop/11-01-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Implement all 15 FileSystemContext callbacks for WinFsp - WinFspContext with Arc<Mutex<CipherBoxFS>> interior mutability - resolve_path() translates Windows backslash paths to inode lookups - is_windows_special() filters desktop.ini, Thumbs.db, $RECYCLE.BIN, etc. - Content prefetch via channel-based async pattern (same as macOS) - FILETIME conversion helpers for Windows timestamp format - Version creation with 15-minute cooldown and MAX_VERSIONS_PER_FILE=10 - Self-contained metadata decrypt (no dependency on fuse::operations module) - normalize_name() made pub(crate) for cross-module access Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Create fuse/windows/mod.rs with WinFsp mount_filesystem/unmount_filesystem - WinFsp host creation with VolumeParams (CipherBox label, 1s cache, case-insensitive) - Pre-populate root folder and subfolders from IPNS before mount - Eager FilePointer resolution for all children - Stale mount point cleanup (WinFsp creates reparse point) - Stop signal via OnceLock<AtomicBool> for clean shutdown - 2-second early failure detection pattern (same as macOS) - Add #[cfg(feature = "winfsp")] pub mod windows to fuse/mod.rs - Re-export mount/unmount for platform dispatch (same function names) - Make operations::implementation pub(crate) for mount module access Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - Task 1: WinFsp FileSystemContext implementation - Task 2: Windows mount/unmount and module dispatch SUMMARY: .planning/phases/11-windows-desktop/11-02-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add WinFsp runtime detection via Windows Registry check at startup - Show notification if WinFsp is missing (app can still launch without mount) - Platform-branch tray "open" handler: explorer.exe on Windows, open on macOS - Use .ico tray icon on Windows, .png template on macOS - Update mount/unmount cfg gates from fuse-only to any(fuse, winfsp) - Add winreg dependency for Windows Registry access - Add autostart plugin comment explaining MacosLauncher param on Windows - Verify --dev-key headless mode is cross-platform (debug_assertions only) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Configure tauri.conf.json with NSIS installer hooks and WinFsp MSI resource - Add windows/installer-hooks.nsh with silent WinFsp MSI install via ExecWait - NSIS PREINSTALL checks registry before installing; POSTUNINSTALL cleans mount - Add cargo-check-windows CI job for fast Rust compilation check on windows-latest - Add build-desktop-windows CI job for full Tauri NSIS build with WinFsp - CI downloads WinFsp MSI from GitHub releases (not committed to repo) - Add resources/.gitkeep for WinFsp MSI bundling directory - Add Windows bundle settings (SHA-256 digest, timestamp URL) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - Platform branching in main.rs, tray, commands, and WinFsp runtime detection - Tauri NSIS packaging with WinFsp bundling and CI pipeline Phase 11 (Windows Desktop) is now COMPLETE (3/3 plans). SUMMARY: .planning/phases/11-windows-desktop/11-03-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
drain_refresh_completions() in fuse/mod.rs referenced operations::decrypt_* functions gated to cfg(fuse) only. On Windows (cfg(winfsp)), the operations module doesn't compile, causing unresolved references. Extracts the two decrypt functions into fuse/decrypt.rs gated with cfg(any(fuse, winfsp)) so drain_refresh_completions works on both platforms. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 11 verified: 4/4 must-haves passed. PLAT-02 requirement marked Complete. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughAdds Windows desktop support (WinFsp) and cross-platform refactors: introduces FileAttrs and AccessMode, implements WinFsp mount/unmount and registry checks, NSIS installer hooks, broadens cfgs for fuse/winfsp, updates CI (Windows/macOS desktop jobs), and adds planning, UAT, and verification documentation marking Phase 11 complete. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as main.rs<br/>(startup)
participant WinFsp as WinFsp<br/>Registry
participant Mount as mount_filesystem<br/>(Windows)
participant IPFS as IPFS/IPNS
participant FS as CipherBoxFS<br/>(in-memory)
participant Host as WinFsp<br/>Host
User->>App: Launch CipherBox
App->>WinFsp: check_winfsp_installed()
WinFsp-->>App: InstallDir exists?
alt WinFsp Missing
App->>User: Show warning notification
App->>App: Log error, continue
else WinFsp Found
App->>Mount: mount_filesystem(keys, IPNS)
end
Mount->>Mount: Clean stale mount points
Mount->>IPFS: Fetch root folder from IPNS
IPFS-->>Mount: Encrypted metadata
Mount->>Mount: Decrypt metadata (FileAttrs)
Mount->>FS: Populate root inode
Mount->>IPFS: Fetch subfolders/FilePointers
IPFS-->>Mount: Encrypted file metadata
Mount->>Mount: Decrypt & resolve
Mount->>FS: Pre-populate inodes
Mount->>Host: Create WinFsp context<br/>with CipherBoxFS
Mount->>Host: Start filesystem host<br/>(dedicated thread)
Host->>Host: Mount volume at C:\Users\...\CipherBox
Host-->>Mount: Mount complete
Mount-->>App: Return success
App->>User: Desktop ready
sequenceDiagram
participant Client as User<br/>Process
participant Handler as OpenFileHandle<br/>(AccessMode)
participant Inode as InodeData<br/>(FileAttrs)
participant FUSE as FUSE<br/>Reply
Client->>Handler: open(ReadOnly)
Handler->>Handler: new_read(ino) →<br/>AccessMode::ReadOnly
Handler-->>Client: File opened
Client->>Inode: getattr()
Inode->>Inode: attr: FileAttrs<br/>(platform-agnostic)
Inode->>FUSE: to_fuse_attr(uid, gid)<br/>→ fuser::FileAttr
FUSE-->>Client: Attributes (macOS)
Note over Inode: Windows: FileAttrs<br/>used directly by WinFsp
Client->>Handler: close()
Handler->>Handler: access_mode checked<br/>for flush/upload
Handler-->>Client: File closed
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Replace dtolnay/rust-toolchain and swatinem/rust-cache (not in repo's allowed actions list) with rustup (pre-installed), actions/cache (GitHub-owned), and tauri-apps/tauri-action (allowed). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move nsis config from bundle.nsis to bundle.windows.nsis (Tauri v2 schema requires nsis inside the windows object) - Chain build-desktop-windows after cargo-check-windows to avoid redundant compilation on check failure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tauri-build validates the resources glob pattern during cargo check. The cargo-check-windows job doesn't need the actual MSI, but the glob must match at least one file to pass validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix 21 compilation errors from first CI run against winfsp 0.12.4: - Move OpenFileInfo import to winfsp::filesystem - Use winfsp::FspError (error module is private) - Fix get_volume_info to fill &mut VolumeInfo (not return) - Fix get_security_by_name signature (FnOnce, FileSecurity, c_void) - Remove generic type params from open/create/rename/set_delete - Add file_info: &mut FileInfo param to write/get_file_info - Fix cleanup context to &Self::FileContext (not &mut) - Rewrite read_directory for buffer-based API (DirMarker, &mut [u8]) - Use DirInfo::new()/set_file_info()/set_name() (fields are private) - Use FileSystemParams::default_params() (no ::new()) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix 43 compilation errors from CI: - Replace FspError named variants with NTSTATUS codes via helper fns - Fix OpenFileInfo: use as_mut() for FileInfo access - Fix DirInfo: use file_info_mut()/set_name()/write() from WideNameInfo - Fix DirMarker: use is_none()/is_current()/is_parent()/inner_as_cstr() - Fix flush signature: context is Option<&Self::FileContext> - Fix create params: FILE_ACCESS_RIGHTS, FILE_FLAGS_AND_ATTRIBUTES, &[c_void] - Fix open params: FILE_ACCESS_RIGHTS - Fix set_basic_info: FILE_FLAGS_AND_ATTRIBUTES - Fix FileSecurity construction with proper attributes type - Add windows crate dependency for Win32 type re-exports Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use plain u32 for granted_access/file_attributes (not Windows wrapper types) - Remove windows crate dependency (not needed) - Fix DirInfo: use set_name_cstr() and append_to_buffer() - Fix FileSystemHost::new() to take VolumeParams directly - Fix double mutable borrow in read() via owned clone - Fix FileSecurity.attributes to plain u32 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove .0 field access on u32 granted_access (was FILE_ACCESS_RIGHTS) - Fix append_to_buffer: takes (&mut [u8], &mut u32) -> bool - Use let _ for set_name_cstr Result - Pass full buffer with cursor to append_to_buffer Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DirInfo requires explicit BUFFER_SIZE const generic (255 chars default). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Clone inode fields before accessing content_cache and prefetching sets to avoid simultaneous immutable/mutable borrows of CipherBoxFS. Also prefix unused function parameters with underscore. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The tauri-action was passing `--features winfsp` which adds winfsp on top of the default `fuse` feature. On Windows, the vendored fuser crate fails because pkg-config and libfuse aren't available. Fixed by adding `--no-default-features` to disable the fuse feature. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tauri CLI doesn't accept --no-default-features directly. Pass it through to cargo using the -- separator so that Cargo receives --no-default-features --features winfsp. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WinFsp 32-bit installer writes registry key under WOW6432Node which 64-bit Rust binary couldn't find. Now checks both native and WOW6432Node paths. Added image-ico Tauri feature for Windows tray icon support. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lder 26 UAT tests completed (25 pass, 1 partial). Documents two bugs found and fixed: WinFsp mount lifecycle (non-blocking dispatcher) and file flush timing (cleanup vs close IRP deferral). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds cargo-check-macos and build-desktop-macos jobs mirroring the existing Windows jobs. Installs FUSE-T via Homebrew and uses the default fuse feature. Catches macOS compilation regressions on PRs instead of waiting until staging deploys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
> [!CAUTION]
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)apps/desktop/src-tauri/src/fuse/mod.rs (1)
712-752:⚠️ Potential issue | 🟠 MajorAvoid network I/O in
drain_refresh_completions.
This path performs IPNS resolve + IPFS fetch viablock_with_timeoutwhile running insidedrain_refresh_completions, which is called fromlookup()/readdir(). That can stall the single FUSE thread and violates the “no network I/O in callbacks” rule. Move FilePointer resolution to background tasks (channel-based, drained like refresh/content) and apply results only when received.Based on learnings: "apps/desktop/src-tauri/src/fuse/*.rs : FUSE callbacks must never perform network I/O except in
release()which spawns a background task."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/mod.rs` around lines 712 - 752, The drain_refresh_completions path is performing network I/O (block_with_timeout calling crate::api::ipns::resolve_ipns and crate::api::ipfs::fetch_content, then decrypt::decrypt_file_metadata_from_ipfs_public) inside a FUSE callback which can block the single FUSE thread; move FilePointer resolution out of drain_refresh_completions by creating a background worker: add a file_pointer_tx/file_pointer_rx channel pair, change the loop that uses self.inodes.get_unresolved_file_pointers() to enqueue unresolved entries (ino, ipns_name, folder_key) to file_pointer_tx instead of resolving inline, spawn an async task (or dedicated runtime task via self.rt.clone()) to receive from file_pointer_rx, perform resolve_ipns/fetch_content/decrypt there with timeouts, and then send results back (success or error) to the main thread (or a results channel) so drain_refresh_completions can apply the final self.inodes.resolve_file_pointer(...) only when the resolved result arrives; ensure any logs/errors are produced from the background task and no network calls remain in drain_refresh_completions/lookup/readdir.apps/desktop/src-tauri/src/fuse/operations.rs (1)
1842-2034:⚠️ Potential issue | 🔴 CriticalFix mkdir closure return type annotation after FileAttrs migration.
The closure at line 1842 declares
Result<FileAttr, String>butFileAttris not imported (onlyFileAttrsis imported at line 21). The closure constructs and returnsFileAttrs(line 1867, 2028), and the calling code invokesattr.to_fuse_attr()which is a method onFileAttrs. This type mismatch will cause a compilation error.Change the return type annotation from
Result<FileAttr, String>toResult<FileAttrs, String>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/operations.rs` around lines 1842 - 2034, The closure currently annotated as Result<FileAttr, String> should be Result<FileAttrs, String> to match the actual return value and the constructed InodeData/attr used later (see the closure that builds FileAttrs, the variable attr, and the call attr.to_fuse_attr()); update the closure's return type annotation from FileAttr to FileAttrs so the types align and compilation succeeds.
🧹 Nitpick comments (8)
apps/desktop/src-tauri/tauri.conf.json (1)
31-38: NSIS packaging configuration looks correct.The
digestAlgorithm: "sha256", NSISinstallMode: "both", and installer hooks reference are properly configured for the Windows installer. One minor note: thetimestampUrluses HTTP — while this is common for Authenticode timestamp servers, HTTPS (https://timestamp.comodoca.com) would be slightly more secure if the server supports it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/tauri.conf.json` around lines 31 - 38, Update the timestampUrl value in the tauri configuration so it uses HTTPS instead of HTTP to improve security; locate the "timestampUrl" field in the "windows" configuration block (the same block containing "digestAlgorithm" and "nsis" with "installMode" and "installerHooks") and change its value from "http://timestamp.comodoca.com" to "https://timestamp.comodoca.com" (verify the timestamp server supports HTTPS after the change).apps/desktop/src-tauri/Cargo.toml (1)
47-48: Optionalwinfspdependency is not platform-gated.The
winfspandwidestringoptional dependencies are in the general[dependencies]section rather than under[target.'cfg(windows)'.dependencies]. If someone accidentally enables thewinfspfeature on a non-Windows platform, compilation will fail in thewinfspcrate itself (unclear error). This is low-risk since the feature is explicitly controlled, but moving them to the Windows target section withoptional = truewould be more robust.That said, Cargo's interaction between
[features]and target-specific optional dependencies can be tricky, so the current approach may be intentional for simplicity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/Cargo.toml` around lines 47 - 48, The winfsp and widestring optional dependencies are declared in the general [dependencies] section which allows enabling the winfsp feature on non-Windows platforms and can cause non-obvious build failures; move the winfsp = { version = "0.12", optional = true } and widestring = { version = "1", optional = true } entries into the target.'cfg(windows)'.dependencies table so they are only resolved on Windows, and then ensure any feature in [features] that enables "winfsp" only references the (now target-scoped) dependency name so the feature still toggles the crate on Windows but cannot introduce the dependency on other platforms..github/workflows/ci.yml (2)
218-221: WinFsp MSI download URL is not integrity-checked.The MSI is downloaded from GitHub releases without verifying a checksum. While the source is the official WinFsp repository, adding a SHA256 check would protect against supply chain attacks or CDN compromises.
🛡️ Suggested checksum verification
run: | $url = "https://github.com/winfsp/winfsp/releases/download/v2.1/winfsp-2.1.25156.msi" $out = "winfsp.msi" Invoke-WebRequest -Uri $url -OutFile $out + $expected = "<SHA256_HASH_HERE>" + $actual = (Get-FileHash $out -Algorithm SHA256).Hash + if ($actual -ne $expected) { throw "WinFsp MSI checksum mismatch: expected $expected, got $actual" } Start-Process msiexec.exe -ArgumentList "/i","$out","/qn","INSTALLLEVEL=1000" -Wait -NoNewWindow🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 218 - 221, The MSI download step using $url/$out with Invoke-WebRequest and Start-Process msiexec.exe should verify the file integrity: obtain or embed the expected SHA256 for the WinFsp MSI, compute the downloaded file's hash with Get-FileHash -Algorithm SHA256 for $out, compare it to the expected value, and if it doesn't match log an error and fail the job (remove the bad file and exit non-zero) before calling Start-Process msiexec.exe; modify the block around $url, $out, Invoke-WebRequest and Start-Process to include this hash check and fail-fast on mismatch.
242-278: Missing cargo cache inbuild-desktop-windowsjob.The
cargo-check-windowsjob (line 230-237) has a cargo cache step, butbuild-desktop-windowsdoes not. Since these run on separate runners, the cache from the check job isn't available. Adding a similar cache step here would significantly speed up builds by reusing downloaded crate dependencies.♻️ Suggested cache step
- name: Build crypto package run: pnpm --filter `@cipherbox/crypto` build + - uses: actions/cache@v4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + apps/desktop/src-tauri/target + key: windows-cargo-build-${{ hashFiles('apps/desktop/src-tauri/Cargo.lock') }} + restore-keys: | + windows-cargo-build- + windows-cargo- + - uses: tauri-apps/tauri-action@v0apps/desktop/src-tauri/src/fuse/decrypt.rs (2)
1-44: Code is duplicated infuse/windows/mod.rs— reuse these shared helpers instead.
decrypt_metadata_from_ipfs(lines 25–64) anddecrypt_file_metadata_from_ipfs(lines 67–102) infuse/windows/mod.rsare near-identical copies of these two public functions. Since this shareddecryptmodule was introduced specifically to be platform-agnostic, the Windows mount module should call these public helpers rather than maintaining private duplicates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/decrypt.rs` around lines 1 - 44, Windows mount has near-duplicate implementations of the metadata decryption logic; replace those duplicates by calling the shared helpers in the platform-agnostic decrypt module instead. In fuse/windows/mod.rs remove the bodies of decrypt_metadata_from_ipfs and decrypt_file_metadata_from_ipfs and invoke the public functions decrypt_metadata_from_ipfs_public and decrypt_file_metadata_from_ipfs_public (or similarly named helpers) from crate::fuse::decrypt, converting/propagating errors as needed; ensure you import the decrypt module and match expected argument types (pass encrypted bytes and folder_key slices, convert keys to &[u8] if necessary) and preserve the same return signature by mapping error messages to the existing error type.
5-44: Both functions share ~30 lines of identical JSON→IV→base64→sealed logic.Consider extracting the common "parse encrypted JSON envelope and reconstruct sealed bytes" step into a private helper, leaving each public function to only handle the key type conversion and delegation to the appropriate
decrypt_*function.♻️ Sketch of a shared inner helper
/// Parse IPFS encrypted JSON and reconstruct sealed bytes (IV || ciphertext). fn parse_ipfs_sealed_envelope(encrypted_bytes: &[u8], label: &str) -> Result<Vec<u8>, String> { #[derive(serde::Deserialize)] struct EncryptedEnvelope { iv: String, data: String } let encrypted: EncryptedEnvelope = serde_json::from_slice(encrypted_bytes) .map_err(|e| format!("Failed to parse encrypted {} JSON: {}", label, e))?; let iv_bytes = hex::decode(&encrypted.iv) .map_err(|_| format!("Invalid {} IV hex", label))?; if iv_bytes.len() != 12 { return Err(format!("Invalid IV length: {} (expected 12)", iv_bytes.len())); } use base64::Engine; let ciphertext = base64::engine::general_purpose::STANDARD .decode(&encrypted.data) .map_err(|e| format!("Invalid {} base64: {}", label, e))?; let mut sealed = Vec::with_capacity(12 + ciphertext.len()); sealed.extend_from_slice(&iv_bytes); sealed.extend_from_slice(&ciphertext); Ok(sealed) }Also applies to: 47-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/decrypt.rs` around lines 5 - 44, The JSON→IV→base64→sealed reconstruction is duplicated; extract it into a private helper fn parse_ipfs_sealed_envelope(encrypted_bytes: &[u8], label: &str) -> Result<Vec<u8>, String> that deserializes the envelope, hex-decodes and validates the IV (12 bytes), base64-decodes the data, and returns the sealed Vec (IV || ciphertext) with contextual error messages using the label. Replace the duplicated blocks in decrypt_metadata_from_ipfs_public (and the other function around lines 47-82) to call parse_ipfs_sealed_envelope, then perform the existing key conversions (e.g., folder_key -> [u8;32]) and delegate to crate::crypto::folder::decrypt_folder_metadata (or the corresponding decrypt_* function) while preserving the current error mapping.apps/desktop/src-tauri/src/tray/mod.rs (1)
144-161: "Open" handler has no fallback for Linux/other platforms.When neither
target_os = "macos"nortarget_os = "windows"matches, the code passes themount_point.exists()check but silently does nothing. Once Phase 11.3 (Linux) lands, this will need anxdg-openbranch. Consider adding a compile-time warning or a#[cfg(not(any(...)))]block withlog::warn!so the gap is visible.💡 Suggested fallback for future Linux support
#[cfg(target_os = "windows")] { if let Err(e) = std::process::Command::new("explorer.exe") .arg(mount_point.to_str().unwrap_or_default()) .spawn() { log::error!("Failed to open CipherBox in Explorer: {}", e); } } + #[cfg(not(any(target_os = "macos", target_os = "windows")))] + { + if let Err(e) = std::process::Command::new("xdg-open") + .arg(mount_point.to_str().unwrap_or_default()) + .spawn() + { + log::error!("Failed to open CipherBox in file manager: {}", e); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/tray/mod.rs` around lines 144 - 161, The open-handler block in tray/mod.rs only handles macOS and Windows so on other platforms (e.g., Linux) it silently does nothing; add a fallback cfg branch for non-mac/win (#[cfg(not(any(target_os = "macos", target_os = "windows")))]) that attempts to spawn "xdg-open" with the same mount_point handling (use mount_point.to_str().unwrap_or_default()) and logs errors similarly, and also include a log::warn! in that fallback to make the gap visible at runtime/compile-time; alternatively add a #[cfg(not(any(...)))] branch that emits a compile-time warning log to surface missing platform support.apps/desktop/src-tauri/src/fuse/windows/mod.rs (1)
160-399: Pre-population logic is deeply nested (~8+ levels) and hard to follow.The root folder pre-population, FilePointer resolution, and subfolder pre-population blocks span ~240 lines with very deep nesting. Consider extracting helpers like
pre_populate_folder(...)andresolve_file_pointers(...)to improve readability and reduce indentation depth. This would also make the subfolder FilePointer resolution (which repeats the root pattern) easier to maintain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/windows/mod.rs` around lines 160 - 399, The pre-population block is too deeply nested—extract the repeated logic into helpers: implement a pre_populate_folder(root_ipns_name, root_folder_key, inode_ino, &state, &private_key, &public_key, &mut inodes, &mut metadata_cache) that performs resolve_ipns, fetch_content, decrypt_metadata_from_ipfs, calls inodes.populate_folder and returns the decrypted metadata and cid (or error); and implement resolve_file_pointers(unresolved: &[(u64,String)], folder_key: &[u8], &state, &mut inodes) that resolves IPNS, fetches content, calls decrypt_file_metadata_from_ipfs and then inodes.resolve_file_pointer for each entry (use get_unresolved_file_pointers to obtain unresolved lists). Replace the inline root and subfolder loops with calls to these helpers (root uses inode::ROOT_INO and root_folder_key, subfolders iterate subfolder_infos and call the same helpers with their ino/ipns/key), preserving existing log messages and warning handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src-tauri/src/commands.rs`:
- Line 209: The current log call uses a hardcoded Unix-style path in
log::info!("Filesystem mounted at ~/CipherBox");—update this to log the actual
mount path returned or computed at runtime: locate where the filesystem is
mounted (the mount function or variable that performs the mount in commands.rs)
and change the log::info call to include that path string (or derive it using
the same logic used to create the mount, e.g., the returned mount_path or
equivalent variable) so Windows shows the correct path instead of "~/CipherBox".
In `@apps/desktop/src-tauri/src/fuse/inode.rs`:
- Around line 23-40: normalize_name currently returns the raw name for the
winfsp-only build, which breaks case-insensitive lookups; update the winfsp
branch of normalize_name to perform case-folding (e.g., call to_lowercase() or
an appropriate Unicode case-fold) so keys used by name_to_ino match WinFsp's
case-insensitive lookup behavior while leaving the fuse (NFC) branch unchanged;
the original casing should continue to be preserved in InodeData.name for
display.
In `@apps/desktop/src-tauri/src/fuse/windows/mod.rs`:
- Around line 130-137: The code discards errors from std::fs::remove_dir_all and
unconditionally logs "Cleaned stale mount point" which hides failures; modify
the block that uses crate::fuse::mount_point() and remove_dir_all(&mount_path)
to check the Result returned by remove_dir_all, log success with log::info only
on Ok and log::error (including the error returned) on Err so failures are
visible and handled appropriately (you may also return or propagate the error if
that would prevent an unusable mount).
- Around line 311-366: get_unresolved_file_pointers() is returning global
unresolved pointers so root-level pointers are retried for each subfolder with
the subfolder key (sk); restrict the set to only pointers that are children of
the current subfolder. Fix by scoping the query: either call a new API like
inodes.get_unresolved_file_pointers_for_parent(sub_ino) or filter the Result
from inodes.get_unresolved_file_pointers() to keep only entries whose inode is a
descendant/child of the current subfolder inode before iterating (the place
where sub_unresolved is defined and used, and where sk / sub_key,
decrypt_file_metadata_from_ipfs, and inodes.resolve_file_pointer are
referenced).
- Around line 104-106: WINFSP_STOP is currently a OnceLock which causes the stop
signal to remain the old Arc<AtomicBool> across remounts; change the static to a
mutable container like Mutex<Option<Arc<std::sync::atomic::AtomicBool>>>
(initialized via std::sync::LazyLock or once_cell::sync::Lazy or
parking_lot::Mutex) and update mount_filesystem and unmount_filesystem to
write/read this slot: in mount_filesystem create a new Arc<AtomicBool>, lock the
mutex and replace the Option with Some(new_signal) (then clone for the thread as
stop_clone), and in unmount_filesystem lock the mutex and take() the Option (if
Some) and set its AtomicBool to true so the correct, current stop signal is
signaled on unmount.
In `@apps/desktop/src-tauri/windows/installer-hooks.nsh`:
- Around line 10-22: The WinFsp MSI install logic currently in the
NSIS_HOOK_PREINSTALL macro must be moved to NSIS_HOOK_POSTINSTALL because
$INSTDIR is not populated yet (the MSI at
$INSTDIR\resources\winfsp-2.1.25156.msi won’t exist during
NSIS_HOOK_PREINSTALL); relocate the entire block that reads the registry,
ExecWait '"msiexec" /i "$INSTDIR\resources\winfsp-2.1.25156.msi" /qn ...' and
related MessageBox/DetailPrint into the NSIS_HOOK_POSTINSTALL macro. Also
address elevation: either enforce system-wide installs by changing installMode
to require admin (so msiexec /i ... /qn can run), or add privilege elevation
logic around the msiexec call in NSIS_HOOK_POSTINSTALL to handle per-user
installs failing without admin rights.
---
Outside diff comments:
In `@apps/desktop/src-tauri/src/fuse/mod.rs`:
- Around line 712-752: The drain_refresh_completions path is performing network
I/O (block_with_timeout calling crate::api::ipns::resolve_ipns and
crate::api::ipfs::fetch_content, then
decrypt::decrypt_file_metadata_from_ipfs_public) inside a FUSE callback which
can block the single FUSE thread; move FilePointer resolution out of
drain_refresh_completions by creating a background worker: add a
file_pointer_tx/file_pointer_rx channel pair, change the loop that uses
self.inodes.get_unresolved_file_pointers() to enqueue unresolved entries (ino,
ipns_name, folder_key) to file_pointer_tx instead of resolving inline, spawn an
async task (or dedicated runtime task via self.rt.clone()) to receive from
file_pointer_rx, perform resolve_ipns/fetch_content/decrypt there with timeouts,
and then send results back (success or error) to the main thread (or a results
channel) so drain_refresh_completions can apply the final
self.inodes.resolve_file_pointer(...) only when the resolved result arrives;
ensure any logs/errors are produced from the background task and no network
calls remain in drain_refresh_completions/lookup/readdir.
In `@apps/desktop/src-tauri/src/fuse/operations.rs`:
- Around line 1842-2034: The closure currently annotated as Result<FileAttr,
String> should be Result<FileAttrs, String> to match the actual return value and
the constructed InodeData/attr used later (see the closure that builds
FileAttrs, the variable attr, and the call attr.to_fuse_attr()); update the
closure's return type annotation from FileAttr to FileAttrs so the types align
and compilation succeeds.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 218-221: The MSI download step using $url/$out with
Invoke-WebRequest and Start-Process msiexec.exe should verify the file
integrity: obtain or embed the expected SHA256 for the WinFsp MSI, compute the
downloaded file's hash with Get-FileHash -Algorithm SHA256 for $out, compare it
to the expected value, and if it doesn't match log an error and fail the job
(remove the bad file and exit non-zero) before calling Start-Process
msiexec.exe; modify the block around $url, $out, Invoke-WebRequest and
Start-Process to include this hash check and fail-fast on mismatch.
In `@apps/desktop/src-tauri/Cargo.toml`:
- Around line 47-48: The winfsp and widestring optional dependencies are
declared in the general [dependencies] section which allows enabling the winfsp
feature on non-Windows platforms and can cause non-obvious build failures; move
the winfsp = { version = "0.12", optional = true } and widestring = { version =
"1", optional = true } entries into the target.'cfg(windows)'.dependencies table
so they are only resolved on Windows, and then ensure any feature in [features]
that enables "winfsp" only references the (now target-scoped) dependency name so
the feature still toggles the crate on Windows but cannot introduce the
dependency on other platforms.
In `@apps/desktop/src-tauri/src/fuse/decrypt.rs`:
- Around line 1-44: Windows mount has near-duplicate implementations of the
metadata decryption logic; replace those duplicates by calling the shared
helpers in the platform-agnostic decrypt module instead. In fuse/windows/mod.rs
remove the bodies of decrypt_metadata_from_ipfs and
decrypt_file_metadata_from_ipfs and invoke the public functions
decrypt_metadata_from_ipfs_public and decrypt_file_metadata_from_ipfs_public (or
similarly named helpers) from crate::fuse::decrypt, converting/propagating
errors as needed; ensure you import the decrypt module and match expected
argument types (pass encrypted bytes and folder_key slices, convert keys to
&[u8] if necessary) and preserve the same return signature by mapping error
messages to the existing error type.
- Around line 5-44: The JSON→IV→base64→sealed reconstruction is duplicated;
extract it into a private helper fn parse_ipfs_sealed_envelope(encrypted_bytes:
&[u8], label: &str) -> Result<Vec<u8>, String> that deserializes the envelope,
hex-decodes and validates the IV (12 bytes), base64-decodes the data, and
returns the sealed Vec (IV || ciphertext) with contextual error messages using
the label. Replace the duplicated blocks in decrypt_metadata_from_ipfs_public
(and the other function around lines 47-82) to call parse_ipfs_sealed_envelope,
then perform the existing key conversions (e.g., folder_key -> [u8;32]) and
delegate to crate::crypto::folder::decrypt_folder_metadata (or the corresponding
decrypt_* function) while preserving the current error mapping.
In `@apps/desktop/src-tauri/src/fuse/windows/mod.rs`:
- Around line 160-399: The pre-population block is too deeply nested—extract the
repeated logic into helpers: implement a pre_populate_folder(root_ipns_name,
root_folder_key, inode_ino, &state, &private_key, &public_key, &mut inodes, &mut
metadata_cache) that performs resolve_ipns, fetch_content,
decrypt_metadata_from_ipfs, calls inodes.populate_folder and returns the
decrypted metadata and cid (or error); and implement
resolve_file_pointers(unresolved: &[(u64,String)], folder_key: &[u8], &state,
&mut inodes) that resolves IPNS, fetches content, calls
decrypt_file_metadata_from_ipfs and then inodes.resolve_file_pointer for each
entry (use get_unresolved_file_pointers to obtain unresolved lists). Replace the
inline root and subfolder loops with calls to these helpers (root uses
inode::ROOT_INO and root_folder_key, subfolders iterate subfolder_infos and call
the same helpers with their ino/ipns/key), preserving existing log messages and
warning handling.
In `@apps/desktop/src-tauri/src/tray/mod.rs`:
- Around line 144-161: The open-handler block in tray/mod.rs only handles macOS
and Windows so on other platforms (e.g., Linux) it silently does nothing; add a
fallback cfg branch for non-mac/win (#[cfg(not(any(target_os = "macos",
target_os = "windows")))]) that attempts to spawn "xdg-open" with the same
mount_point handling (use mount_point.to_str().unwrap_or_default()) and logs
errors similarly, and also include a log::warn! in that fallback to make the gap
visible at runtime/compile-time; alternatively add a #[cfg(not(any(...)))]
branch that emits a compile-time warning log to surface missing platform
support.
In `@apps/desktop/src-tauri/tauri.conf.json`:
- Around line 31-38: Update the timestampUrl value in the tauri configuration so
it uses HTTPS instead of HTTP to improve security; locate the "timestampUrl"
field in the "windows" configuration block (the same block containing
"digestAlgorithm" and "nsis" with "installMode" and "installerHooks") and change
its value from "http://timestamp.comodoca.com" to
"https://timestamp.comodoca.com" (verify the timestamp server supports HTTPS
after the change).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
.github/workflows/ci.yml.gitignore.planning/REQUIREMENTS.md.planning/ROADMAP.md.planning/STATE.md.planning/phases/11-windows-desktop/11-01-PLAN.md.planning/phases/11-windows-desktop/11-01-SUMMARY.md.planning/phases/11-windows-desktop/11-02-PLAN.md.planning/phases/11-windows-desktop/11-02-SUMMARY.md.planning/phases/11-windows-desktop/11-03-PLAN.md.planning/phases/11-windows-desktop/11-03-SUMMARY.md.planning/phases/11-windows-desktop/11-CONTEXT.md.planning/phases/11-windows-desktop/11-RESEARCH.md.planning/phases/11-windows-desktop/11-UAT.md.planning/phases/11-windows-desktop/11-VERIFICATION.mdapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/build.rsapps/desktop/src-tauri/resources/.gitkeepapps/desktop/src-tauri/resources/winfsp-placeholder.msiapps/desktop/src-tauri/src/commands.rsapps/desktop/src-tauri/src/fuse/decrypt.rsapps/desktop/src-tauri/src/fuse/file_handle.rsapps/desktop/src-tauri/src/fuse/inode.rsapps/desktop/src-tauri/src/fuse/mod.rsapps/desktop/src-tauri/src/fuse/operations.rsapps/desktop/src-tauri/src/fuse/windows/mod.rsapps/desktop/src-tauri/src/fuse/windows/operations.rsapps/desktop/src-tauri/src/main.rsapps/desktop/src-tauri/src/tray/mod.rsapps/desktop/src-tauri/tauri.conf.jsonapps/desktop/src-tauri/windows/installer-hooks.nsh
Cargo discovers .cargo/config.toml from the working directory, not from --manifest-path. The PKG_CONFIG_PATH for FUSE-T is set in apps/desktop/src-tauri/.cargo/config.toml, so cargo check must run from that directory to find it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
window.open() is unreliable on Windows WebView2 — the NewWindowRequested event / on_new_window handler may silently fail (tauri-apps/tauri#14263). Replace with an open_oauth_popup Tauri command that creates the popup window directly from Rust, bypassing the WebView2 event chain. Also fix two pre-existing macOS FUSE compilation errors: - FileAttr typo (should be FileAttrs) in mkdir closure - Borrow-after-move of attr in create() and mkdir() callbacks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src-tauri/src/fuse/operations.rs (1)
58-77:⚠️ Potential issue | 🟡 Minor
.Trash-*wildcard pattern not filtered.The coding guideline specifies filtering
.Trash-*(e.g.,.Trash-501,.Trash-1000), but only the exact name.Trashesis matched at line 63. Consider adding astarts_with(".Trash-")check.Proposed fix
fn is_platform_special(name: &str) -> bool { name.starts_with("._") || name == ".DS_Store" || name == ".Trashes" + || name.starts_with(".Trash-") || name == ".fseventsd"Based on learnings: "Platform special files on macOS must be filtered:
.Trash-*,.directory."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/operations.rs` around lines 58 - 77, The is_platform_special function currently only checks for the exact ".Trashes" name and misses per-user trash directories like ".Trash-501"; update the is_platform_special function to also reject names that start with ".Trash-" (e.g., use a starts_with(".Trash-") check) alongside the existing ".Trashes" check so filenames like ".Trash-<id>" are filtered out; modify the boolean expression in is_platform_special to include this starts_with check and keep the existing checks (e.g., ".directory") intact.
♻️ Duplicate comments (1)
apps/desktop/src-tauri/src/commands.rs (1)
213-213: Hardcoded Unix-style path~/CipherBoxin log message is still inaccurate on Windows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/commands.rs` at line 213, The log::info! call that prints "Filesystem mounted at ~/CipherBox" is using a hardcoded Unix-style path; replace that literal with the actual resolved mount path (e.g., a mount_path or filesystem_path variable) or construct the path via the platform-aware API (dirs::home_dir or std::env/tauri path helpers) and log mount_path.display() so Windows paths are accurate; update the log::info! invocation in commands.rs to reference the real path variable rather than the "~/CipherBox" string.
🧹 Nitpick comments (5)
.github/workflows/ci.yml (2)
208-241: WinFsp version is hardcoded in two separate jobs.
winfsp-2.1.25156.msiappears both incargo-check-windows(line 218) andbuild-desktop-windows(line 278). A version bump requires two edits that can easily drift.Extract to a top-level
envblock:♻️ Proposed refactor
+env: + WINFSP_VERSION: "2.1.25156" + WINFSP_MSI: "winfsp-2.1.25156.msi" + jobs: ... - $url = "https://github.com/winfsp/winfsp/releases/download/v2.1/winfsp-2.1.25156.msi" - $out = "winfsp.msi" + $url = "https://github.com/winfsp/winfsp/releases/download/v2.1/${{ env.WINFSP_MSI }}" + $out = "winfsp.msi" ... - $out = "$resourceDir/winfsp-2.1.25156.msi" + $out = "$resourceDir/${{ env.WINFSP_MSI }}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 208 - 241, Extract the hardcoded WinFsp version string (winfsp-2.1.25156.msi) into a top-level env variable (e.g., WINFSP_VERSION or WINFSP_MSI_URL) in the workflow and update both jobs cargo-check-windows and build-desktop-windows to reference that env var in their PowerShell steps (the Invoke-WebRequest URL and any created placeholder filename) instead of the literal string so a single version bump updates both places.
300-304: Built NSIS installer is discarded — noupload-artifactstep.The PR test plan lists "build-desktop-windows producing NSIS" as a verification goal, but without an
actions/upload-artifactstep the artifact is thrown away when the runner is recycled. Adding an upload step lets reviewers download and smoke-test the installer and confirms the bundler actually produced an output.🔧 Suggested addition
- uses: tauri-apps/tauri-action@v0 with: projectPath: apps/desktop tauriScript: pnpm tauri args: -- --no-default-features --features winfsp + + - name: Upload Windows installer + uses: actions/upload-artifact@v4 + with: + name: windows-installer + path: apps/desktop/src-tauri/target/release/bundle/nsis/*.exe + if-no-files-found: error🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 300 - 304, The workflow currently runs the tauri build step (uses: tauri-apps/tauri-action@v0 with keys projectPath, tauriScript, args) but never preserves the NSIS installer; add an actions/upload-artifact@v3 step immediately after that tauri-action step that uploads the NSIS output from the Tauri bundler, give the step a clear name like upload-nsis-installer and an artifact name reviewers can download, and configure its path to the directory/file produced by the tauri build so the installer is retained for review.apps/desktop/src-tauri/src/fuse/operations.rs (2)
161-194: ~70 lines of duplicated decrypt logic between internal and public functions.
decrypt_metadata_from_ipfs(line 161) anddecrypt_metadata_from_ipfs_public(line 2446) are nearly identical, as aredecrypt_file_metadata_from_ipfs(line 303) anddecrypt_file_metadata_from_ipfs_public(line 2486). Since both are compiled whenfeature = "fuse"is active, the internal versions could simply delegate to the public wrappers.Proposed refactor (folder metadata example)
fn decrypt_metadata_from_ipfs( encrypted_bytes: &[u8], folder_key: &[u8], ) -> Result<crate::crypto::folder::FolderMetadata, String> { - let encrypted: EncryptedFolderMetadata = serde_json::from_slice(encrypted_bytes) - .map_err(|e| format!("Failed to parse encrypted metadata JSON: {}", e))?; - // ... ~30 lines of duplicated logic ... - crate::crypto::folder::decrypt_folder_metadata(&sealed, &folder_key_arr) - .map_err(|e| format!("Metadata decryption failed: {}", e)) + crate::fuse::operations::decrypt_metadata_from_ipfs_public(encrypted_bytes, folder_key) }Also applies to: 2445-2482
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/operations.rs` around lines 161 - 194, There is duplicated decrypt logic between internal and public functions; replace the bodies of decrypt_metadata_from_ipfs and decrypt_file_metadata_from_ipfs so they delegate to the existing public wrappers decrypt_metadata_from_ipfs_public and decrypt_file_metadata_from_ipfs_public (and decrypt_file_metadata_from_ipfs_public) instead of duplicating parsing/decoding/decryption logic; call the public functions with the same inputs (convert types as needed), propagate/map their Result<String>/<FolderMetadata>/<FileMetadata> error into the same error strings currently expected, and remove the duplicated code blocks so only the public implementations contain the parsing, hex/base64 decoding and AES-256-GCM assembly logic. Ensure module visibility/imports allow the internal functions to call the public wrappers (use full path or add use statements) and preserve the original error formatting.
60-77: Consider including$RECYCLE.BINand ADS patterns for cross-platform coverage.While this module is
#[cfg(feature = "fuse")](macOS), theis_platform_specialfunction already includes Windows-specific names (Thumbs.db,desktop.ini). For consistency and future cross-platform use, consider adding$RECYCLE.BINand Zone.Identifier ADS filtering if these names could surface through the shared inode layer. Based on learnings: "Filter platform special files:desktop.ini,Thumbs.db,$RECYCLE.BIN, Zone.Identifier ADS."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/operations.rs` around lines 60 - 77, Update the is_platform_special function to also filter Windows recycle and ADS artifacts: add a literal check for "$RECYCLE.BIN" and an ADS check that treats names ending with ":Zone.Identifier" (or containing "Zone.Identifier") as special; modify the boolean chain in is_platform_special to include name == "$RECYCLE.BIN" and a test like name.ends_with(":Zone.Identifier") || name.contains("Zone.Identifier") so those artifacts are ignored similarly to "Thumbs.db" and "desktop.ini".apps/desktop/src-tauri/src/commands.rs (1)
11-12: Misleading comment onPOPUP_COUNTER—clarify that each module maintains its own counter.Both
commands.rsandtray/mod.rsdefine their own separatePOPUP_COUNTERstatics. The comment "shared with tray handler" is misleading because they do not share the counter value; each module independently uses its own counter to generate popup window labels. The actual coordination happens through the"oauth-popup-"label prefix convention, not through counter sharing.🧹 Suggested comment clarification
-/// Counter for unique OAuth popup window labels (shared with tray handler). +/// Counter for unique OAuth popup window labels. static POPUP_COUNTER: AtomicU32 = AtomicU32::new(0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/commands.rs` around lines 11 - 12, The comment on the POPUP_COUNTER static is misleading; update the comment above the POPUP_COUNTER AtomicU32 declaration to clarify that this counter is local to this module (each module defines its own AtomicU32) and that coordination between modules happens via the "oauth-popup-" label prefix convention rather than a shared counter; reference POPUP_COUNTER and the "oauth-popup-" prefix in the new comment so future readers understand the per-module counter semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 264-266: The CI step named "Cargo check with fuse feature"
currently runs just `cargo check`; update that step so it explicitly enables the
fuse feature and disables defaults to match the Windows job by changing the
command to `cargo check --no-default-features --features fuse`. Locate the step
by its name ("Cargo check with fuse feature") in the workflow and replace the
`run: cargo check` line with the new command.
- Around line 306-334: The build job build-desktop-macos is missing the Cargo
cache step; insert an actions/cache@v4 step immediately after the "Install
FUSE-T" step that mirrors the cache configuration used in cargo-check-macos
(cache key, restore-keys, and paths for Cargo such as ~/.cargo/registry,
~/.cargo/git and target directories) so Rust dependencies are cached for the
macOS desktop build; ensure the step uses the same cache key pattern and paths
as the cargo-check-macos job to keep behavior consistent.
- Around line 217-221: The workflow downloads winfsp MSI with Invoke-WebRequest
into $out and installs it via Start-Process without verifying integrity; add a
SHA-256 checksum verification step after the download using Get-FileHash on $out
and compare it to a known expected hash constant (store expected hash in a
variable like $expectedHash) and fail the job (throw/exit) if mismatched before
calling Start-Process; ensure the comparison uses the Hash property from
Get-FileHash and provides a clear error message including the expected vs actual
hash.
- Around line 268-304: The build-desktop-windows job is missing a Cargo cache
restore so it recompiles from scratch; add an actions/cache@v4 step immediately
after the "Install WinFsp" step to restore the same cache key/restore-keys used
by cargo-check-windows (so the runner reuses the previously cached Rust
artifacts), and include at minimum the Cargo cache paths (~/.cargo/registry,
~/.cargo/git) plus the workspace/target used by the desktop build (e.g.,
apps/desktop/target or the same target path saved by cargo-check-windows) before
running tauri-apps/tauri-action@v0; ensure the cache key/paths match the cache
configuration in the cargo-check-windows job so the cache actually restores.
In `@apps/desktop/src-tauri/src/commands.rs`:
- Around line 541-562: open_oauth_popup currently accepts any URL string and
creates a popup, enabling phishing; add a scheme+host allowlist check before
building the window: after parsing the URL in open_oauth_popup, verify
parsed_url.scheme() and parsed_url.host_str() against a small constant/CONFIG
allowlist (e.g., "https" + permitted hosts like "accounts.google.com" or your
auth domain), return Err with a clear message if not allowed, and only then
proceed to call tauri::WebviewWindowBuilder (keep POPUP_COUNTER and label logic
unchanged). Ensure the check is strict (require https and exact host match) and
centralize the allowlist so it can be updated easily.
In `@apps/desktop/src/auth.ts`:
- Around line 725-727: The call to invoke('open_oauth_popup', { url: authUrl })
currently only logs errors, so a rejection leaves the surrounding poll/timeout
logic running and the user waits until the 2-minute timeout; change the catch
handler to propagate the error to the outer flow: when invoke rejects,
immediately clear the polling interval and timeout, and reject (or throw) from
the enclosing promise/state so the caller gets immediate failure feedback;
update the catch associated with invoke('open_oauth_popup', ...) to perform
cleanup (stop the poll and clear the two-minute timeout) and propagate the error
instead of only calling console.error.
---
Outside diff comments:
In `@apps/desktop/src-tauri/src/fuse/operations.rs`:
- Around line 58-77: The is_platform_special function currently only checks for
the exact ".Trashes" name and misses per-user trash directories like
".Trash-501"; update the is_platform_special function to also reject names that
start with ".Trash-" (e.g., use a starts_with(".Trash-") check) alongside the
existing ".Trashes" check so filenames like ".Trash-<id>" are filtered out;
modify the boolean expression in is_platform_special to include this starts_with
check and keep the existing checks (e.g., ".directory") intact.
---
Duplicate comments:
In `@apps/desktop/src-tauri/src/commands.rs`:
- Line 213: The log::info! call that prints "Filesystem mounted at ~/CipherBox"
is using a hardcoded Unix-style path; replace that literal with the actual
resolved mount path (e.g., a mount_path or filesystem_path variable) or
construct the path via the platform-aware API (dirs::home_dir or std::env/tauri
path helpers) and log mount_path.display() so Windows paths are accurate; update
the log::info! invocation in commands.rs to reference the real path variable
rather than the "~/CipherBox" string.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 208-241: Extract the hardcoded WinFsp version string
(winfsp-2.1.25156.msi) into a top-level env variable (e.g., WINFSP_VERSION or
WINFSP_MSI_URL) in the workflow and update both jobs cargo-check-windows and
build-desktop-windows to reference that env var in their PowerShell steps (the
Invoke-WebRequest URL and any created placeholder filename) instead of the
literal string so a single version bump updates both places.
- Around line 300-304: The workflow currently runs the tauri build step (uses:
tauri-apps/tauri-action@v0 with keys projectPath, tauriScript, args) but never
preserves the NSIS installer; add an actions/upload-artifact@v3 step immediately
after that tauri-action step that uploads the NSIS output from the Tauri
bundler, give the step a clear name like upload-nsis-installer and an artifact
name reviewers can download, and configure its path to the directory/file
produced by the tauri build so the installer is retained for review.
In `@apps/desktop/src-tauri/src/commands.rs`:
- Around line 11-12: The comment on the POPUP_COUNTER static is misleading;
update the comment above the POPUP_COUNTER AtomicU32 declaration to clarify that
this counter is local to this module (each module defines its own AtomicU32) and
that coordination between modules happens via the "oauth-popup-" label prefix
convention rather than a shared counter; reference POPUP_COUNTER and the
"oauth-popup-" prefix in the new comment so future readers understand the
per-module counter semantics.
In `@apps/desktop/src-tauri/src/fuse/operations.rs`:
- Around line 161-194: There is duplicated decrypt logic between internal and
public functions; replace the bodies of decrypt_metadata_from_ipfs and
decrypt_file_metadata_from_ipfs so they delegate to the existing public wrappers
decrypt_metadata_from_ipfs_public and decrypt_file_metadata_from_ipfs_public
(and decrypt_file_metadata_from_ipfs_public) instead of duplicating
parsing/decoding/decryption logic; call the public functions with the same
inputs (convert types as needed), propagate/map their
Result<String>/<FolderMetadata>/<FileMetadata> error into the same error strings
currently expected, and remove the duplicated code blocks so only the public
implementations contain the parsing, hex/base64 decoding and AES-256-GCM
assembly logic. Ensure module visibility/imports allow the internal functions to
call the public wrappers (use full path or add use statements) and preserve the
original error formatting.
- Around line 60-77: Update the is_platform_special function to also filter
Windows recycle and ADS artifacts: add a literal check for "$RECYCLE.BIN" and an
ADS check that treats names ending with ":Zone.Identifier" (or containing
"Zone.Identifier") as special; modify the boolean chain in is_platform_special
to include name == "$RECYCLE.BIN" and a test like
name.ends_with(":Zone.Identifier") || name.contains("Zone.Identifier") so those
artifacts are ignored similarly to "Thumbs.db" and "desktop.ini".
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/ci.ymlapps/desktop/src-tauri/src/commands.rsapps/desktop/src-tauri/src/fuse/operations.rsapps/desktop/src-tauri/src/main.rsapps/desktop/src/auth.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src-tauri/src/main.rs
- Replace OnceLock with Mutex<Option<...>> for WinFsp stop signal so remount after unmount works correctly - Move WinFsp MSI install from NSIS PREINSTALL to POSTINSTALL since $INSTDIR is not populated during pre-install - Add case-insensitive name folding (to_lowercase) in normalize_name for WinFsp volumes - Add HTTPS scheme + host allowlist to open_oauth_popup (security) - Propagate popup IPC failure immediately instead of waiting for 2-minute timeout - Add Cargo cache to build-desktop-windows and build-desktop-macos CI jobs - Log actual mount path instead of hardcoded ~/CipherBox - Handle stale mount cleanup errors with proper match instead of silent discard - Scope unresolved FilePointer resolution to parent folder to avoid wrong-key decryption attempts - Add SHA-256 hash verification for WinFsp MSI downloads in CI - Make macOS cargo check explicitly pass --features fuse Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/auth.ts (1)
483-486:⚠️ Potential issue | 🟡 Minor
platform: 'macos'is hardcoded in device factor metadata — will produce incorrect records on Windows.Both
inputRecoveryPhrase(Line 485) andpollApprovalStatus(Line 587) create a new device factor withplatform: 'macos'unconditionally. With Phase 11 adding Windows support, a Windows user recovering via mnemonic or device approval will have their device registered with the wrong platform label in Web3Auth metadata.🐛 Proposed fix: derive platform dynamically
Add a small helper (similar to
getDeviceName):+function getPlatform(): string { + const ua = navigator.userAgent; + if (/Macintosh|Mac OS X/i.test(ua)) return 'macos'; + if (/Windows/i.test(ua)) return 'windows'; + if (/Linux/i.test(ua)) return 'linux'; + return 'unknown'; +}Then replace both hardcoded strings:
- platform: 'macos', + platform: getPlatform(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/auth.ts` around lines 483 - 486, The device factor metadata currently hardcodes platform: 'macos' causing incorrect platform labels; add a small helper (e.g., getDevicePlatform or getPlatform) that maps Node/Electron's process.platform values to the expected strings ('macos' for 'darwin', 'windows' for 'win32', 'linux' for 'linux', fallback 'unknown'), place it alongside getDeviceName, and replace the hardcoded "macos" in both inputRecoveryPhrase and pollApprovalStatus with a call to this helper so device registration uses the correct platform value.apps/desktop/src-tauri/src/fuse/inode.rs (1)
901-923:⚠️ Potential issue | 🟡 MinorThe test assertions are correct, but the reasoning in the review comment is inaccurate.
Lines 916–923 assertions pass, but not because
derive_file_ipns_keypairvalidates a zero-byte secp256k1 private key. Instead:
derive_file_ipns_keypair(&[0u8;32], "file-1")fails immediately withHkdfError::InvalidFileIdbecause the file ID"file-1"is only 6 characters butMIN_FILE_ID_LENGTHis 10.- The error triggers
file_ipns_key = Nonein the match, sofile_ipns_private_keyisNone✓- Since
file_ipns_keyisNone,cached_encrypted_hexis alsoNone✓The HKDF function itself (
derive_ipns_keypair) does not validate secp256k1 constraints on the input private key—it accepts all 32-byte inputs (including all-zeros) and produces a valid Ed25519 keypair. The public_key[0u8; 33]would have also failedwrap_key(which expects 65 bytes), but that code path is never reached due to the earlier file ID validation.The test is functionally correct but relies on incidental input validation, not on secp256k1 private-key validation. Consider using a valid file ID (≥10 chars) to test the intended behavior more explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/inode.rs` around lines 901 - 923, The test is passing due to incidental validation of file ID length, not because derive_ipns_keypair rejects an all-zero secp256k1 key; update the test to exercise the intended HKDF/secp256k1 paths by using a valid file id length (>= MIN_FILE_ID_LENGTH) and a correctly-sized compressed/uncompressed public key so path through derive_file_ipns_keypair -> derive_ipns_keypair -> wrap_key is reached; specifically change the file id string used when calling derive_file_ipns_keypair to be >=10 chars and supply a 65-byte uncompressed public_key (or generate a real secp256k1 keypair) instead of the 33-byte zero array so the assertions on InodeKind::File (file_meta_ipns_name, file_ipns_private_key, file_ipns_key_encrypted_hex) validate the actual derivation behavior.
♻️ Duplicate comments (2)
apps/desktop/src/auth.ts (1)
725-729: Past concern resolved — IPC failure now correctly propagates to the outer promise.The previous version silently swallowed
invokerejections. The updated catch block now callscleanup()to cancel the poll and timeout, then immediatelyrejects the outer promise with a descriptive error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/auth.ts` around lines 725 - 729, The invoke('open_oauth_popup', { url: authUrl }) call must not swallow IPC errors—ensure its .catch handler logs the error, calls cleanup() to cancel any polling/timeout, and then rejects the outer promise with a descriptive Error that includes the original err (use reject(new Error(`Failed to open OAuth popup: ${err}`)) or similar) so failures propagate; verify this behavior around the invoke, cleanup, and reject symbols.apps/desktop/src-tauri/windows/installer-hooks.nsh (1)
19-23:⚠️ Potential issue | 🟠 Major
msiexec /i ... /qnrequires elevation; per-user CipherBox installs will silently fail to install WinFsp.WinFsp is a kernel-mode filesystem driver and requires system-wide (admin) installation. With
installMode: "both", a per-user CipherBox install runs without UAC elevation, andmsiexec /i ... /qnwill fail (typically exit code1925or1603). The MessageBox is shown, but CipherBox won't function for those users.Fixes: restrict
installModeto system-wide only (so the installer always elevates), or request elevation specifically for themsiexeccall via NSIS'sUACplugin orShellExecWait "runas".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/windows/installer-hooks.nsh` around lines 19 - 23, The installer currently runs msiexec via ExecWait ('ExecWait "\"msiexec\" /i \"$INSTDIR\\resources\\winfsp-2.1.25156.msi\" /qn INSTALLLEVEL=1000"') which requires elevation and will silently fail for per-user installs; update the installer so WinFsp is installed with admin rights: either change the installer configuration to force system-wide installs (set installMode to system-wide only) so the whole NSIS installer runs elevated, or replace the ExecWait call with an elevated execution (use NSIS UAC plugin or ShellExecWait "runas" to invoke msiexec as admin), and keep the existing fallback MessageBox for non-elevated failures (the winfsp_installed branch can remain unchanged). Ensure the change references the same ExecWait/msiexec invocation so WinFsp installation is attempted with elevation.
🧹 Nitpick comments (5)
apps/desktop/src-tauri/src/fuse/inode.rs (4)
319-329: Orphaned grandchild inodes after folder removal in!merge_onlypath.When a subfolder is removed (lines 325-327), only the direct entry is cleaned from
self.inodesandname_to_ino. Its descendants remain inself.inodesas unreachable orphans until unmount. While bounded per session (the table rebuilds on remount), in long-lived mounts with frequent refreshes this can inflate memory usage.Consider calling
self.remove(old_ino)for consistency and, for folder children, recursively collecting descendant inodes to remove via a depth-first walk before the loop terminates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/inode.rs` around lines 319 - 329, The cleanup loop for removed children currently removes only the direct inode entries from self.inodes and name_to_ino, leaving descendant inodes orphaned; modify the loop in the merge-only branch to call self.remove(old_ino) instead of directly removing entries, and for directory children (when old_child.kind indicates a folder) perform a depth-first traversal starting from old_ino to collect all descendant inodes and remove them via self.remove to ensure name_to_ino and any other indexes are cleaned consistently (use old_child_inos, self.inodes, normalize_name, and the existing remove method to locate and clear descendants).
233-235:SeqCstordering is stronger than necessary for single-threaded FUSE.
fetch_addatomicity alone guarantees uniqueness — no cross-thread memory ordering is required here. Since all FUSE callbacks run on a single thread (per architecture constraint),Relaxedis sufficient.♻️ Proposed change
- self.next_ino.fetch_add(1, Ordering::SeqCst) + self.next_ino.fetch_add(1, Ordering::Relaxed)Based on learnings: "ALL FUSE callbacks must run on a single thread. Any blocking call stalls the entire filesystem."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/inode.rs` around lines 233 - 235, The allocate_ino method uses a stronger memory ordering than needed; change the atomic fetch_add on next_ino in fn allocate_ino to use Ordering::Relaxed instead of Ordering::SeqCst so uniqueness is preserved without unnecessary cross-thread ordering—update the fetch_add call in the allocate_ino function to pass Ordering::Relaxed.
578-585: O(n²) linear scan inmerge_onlypath — use aHashSetinstead.
child_inos.contains(&old_ino)is O(n) inside an O(n) loop overold_child_inos, producing O(n²) complexity. For large directories this degrades readdir/refresh performance.♻️ Proposed fix
+ let child_inos_set: std::collections::HashSet<u64> = child_inos.iter().copied().collect(); if merge_only { for &old_ino in &old_child_inos { - if !child_inos.contains(&old_ino) { + if !child_inos_set.contains(&old_ino) { child_inos.push(old_ino); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/inode.rs` around lines 578 - 585, The merge_only loop currently does an O(n²) check by calling child_inos.contains(&old_ino) inside a loop over old_child_inos; replace this with an O(1) membership check using a HashSet: construct a HashSet from child_inos (or build one and keep it in sync), then iterate old_child_inos and for each old_ino insert into the set and push to child_inos only if insertion succeeds (i.e., it was not already present). Update the merge_only block that references child_inos and old_child_inos and add the necessary use std::collections::HashSet import.
625-651: Extract preserved fields frominode.kindin a single match before reassignment.The three independent
match &inode.kind { ... }arms on lines 632–644 each borrow and pattern-matchinode.kindseparately. Extracting all three fields in one match before constructing the new variant is cleaner and makes the preservation intent explicit.♻️ Proposed refactor
if let Some(inode) = self.inodes.get_mut(&ino) { + let (preserved_ipns_name, preserved_ipns_key, preserved_ipns_key_hex) = + match &inode.kind { + InodeKind::File { + file_meta_ipns_name, + file_ipns_private_key, + file_ipns_key_encrypted_hex, + .. + } => ( + file_meta_ipns_name.clone(), + file_ipns_private_key.clone(), + file_ipns_key_encrypted_hex.clone(), + ), + _ => (None, None, None), + }; inode.kind = InodeKind::File { cid, encrypted_file_key, iv, size, encryption_mode, - file_meta_ipns_name: match &inode.kind { - InodeKind::File { file_meta_ipns_name, .. } => file_meta_ipns_name.clone(), - _ => None, - }, + file_meta_ipns_name: preserved_ipns_name, file_meta_resolved: true, - file_ipns_private_key: match &inode.kind { - InodeKind::File { file_ipns_private_key, .. } => file_ipns_private_key.clone(), - _ => None, - }, - file_ipns_key_encrypted_hex: match &inode.kind { - InodeKind::File { file_ipns_key_encrypted_hex, .. } => file_ipns_key_encrypted_hex.clone(), - _ => None, - }, + file_ipns_private_key: preserved_ipns_key, + file_ipns_key_encrypted_hex: preserved_ipns_key_hex, versions, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/inode.rs` around lines 625 - 651, The code currently pattern-matches inode.kind three times to preserve file_meta_ipns_name, file_ipns_private_key, and file_ipns_key_encrypted_hex; change this to a single match on &inode.kind (e.g., let (preserved_meta_name, preserved_ipns_priv, preserved_ipns_encrypted) = match &inode.kind { InodeKind::File { file_meta_ipns_name, file_ipns_private_key, file_ipns_key_encrypted_hex, .. } => (file_meta_ipns_name.clone(), file_ipns_private_key.clone(), file_ipns_key_encrypted_hex.clone()), _ => (None, None, None), }; then use these three variables when constructing the new InodeKind::File so you only pattern-match once and preserve the fields, keeping the existing updates to inode.attr.size and inode.attr.blocks.apps/desktop/src-tauri/src/fuse/windows/mod.rs (1)
25-102:decrypt_metadata_from_ipfsanddecrypt_file_metadata_from_ipfsduplicate logic fromoperations.rs; use the sharedfuse/decrypt.rsmodule instead.Both functions (lines 25–64 and 67–102) have identical counterparts in
apps/desktop/src-tauri/src/fuse/windows/operations.rs. The PR objectives note that a sharedfuse/decrypt.rsmodule was extracted precisely for this cross-platform decryption logic. The comment "Self-contained implementation for Windows mount pre-population" indicates intentionality, but this is now a three-way duplication (mod.rs,operations.rs,decrypt.rs).Additionally, the inner struct
EncryptedFolderMetadatais defined identically in both functions:- fn decrypt_metadata_from_ipfs(...) { - #[derive(serde::Deserialize)] - struct EncryptedFolderMetadata { iv: String, data: String } - ... - } - - fn decrypt_file_metadata_from_ipfs(...) { - #[derive(serde::Deserialize)] - struct EncryptedFolderMetadata { iv: String, data: String } - ... - }Extract the shared struct and delegate to the functions in
crate::fuse::decrypt(or re-export them fromoperations) rather than maintaining a local copy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src-tauri/src/fuse/windows/mod.rs` around lines 25 - 102, Both decrypt_metadata_from_ipfs and decrypt_file_metadata_from_ipfs duplicate logic already implemented in the shared fuse/decrypt module; remove the local implementations and delegate to the shared functions (e.g., crate::fuse::decrypt::decrypt_metadata and crate::fuse::decrypt::decrypt_file_metadata or the appropriate re-exports used by operations.rs). Also remove the duplicate EncryptedFolderMetadata struct, or import it from the shared module, and adapt the signatures to accept the same input types as the shared helpers (convert hex/base64/sealed creation to use the shared functions if they handle it). Update calls in mod.rs to call the shared functions (preserving error mapping) and delete the duplicated code blocks in decrypt_metadata_from_ipfs and decrypt_file_metadata_from_ipfs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 361-364: The macOS build job (`build-desktop-macos`) in the CI
workflow is missing explicit Cargo feature flags; update the
tauri-apps/tauri-action invocation for the macOS job (the block using
projectPath: apps/desktop and tauriScript: pnpm tauri) to pass args: --
--no-default-features --features fuse so it matches `cargo-check-macos` and
`build-desktop-windows` and avoids relying on implicit default features.
In `@apps/desktop/src-tauri/src/fuse/windows/mod.rs`:
- Around line 549-551: In unmount_filesystem replace the silent discard of
std::fs::remove_dir_all(&mount_path) with explicit handling that logs both
success and failure similar to mount_filesystem: call
std::fs::remove_dir_all(&mount_path), match on the Result and use the same
logger (or process_logger/trace logger used in mount_filesystem) to emit an
info/debug on Ok and an error with the Err on Err so cleanup failures are
visible; reference the unmount_filesystem function and mirror the
mount_filesystem logging pattern.
In `@apps/desktop/src-tauri/windows/installer-hooks.nsh`:
- Around line 22-24: The installer currently only treats msiexec exit code "0"
as success and falls through after the failure MessageBox into the success
label; update the check around StrCmp and $0 to treat both "0" and "3010" as
success (i.e., success if $0 == "0" or $0 == "3010"), and ensure the failure
path does not fall through to winfsp_installed by adding an explicit jump (Goto)
after the MessageBox to a failure/exit label; reference StrCmp, $0, MessageBox,
winfsp_installed and add a new failure label or Goto to prevent printing the
success DetailPrint when installation actually failed.
- Around line 16-17: The installer only checks the 64-bit registry path using
ReadRegStr and misses 32-bit installs; update the check so that after ReadRegStr
$0 HKLM "SOFTWARE\WinFsp" and before StrCmp $0 "" 0 winfsp_installed you attempt
the 32-bit hive (either by calling ReadRegStr $0 HKLM
"SOFTWARE\WOW6432Node\WinFsp" if $0 is empty, or by using SetRegView 32 then
re-reading) so $0 will contain a WinFsp InstallDir from either hive and the
existing StrCmp to winfsp_installed can work as intended.
---
Outside diff comments:
In `@apps/desktop/src-tauri/src/fuse/inode.rs`:
- Around line 901-923: The test is passing due to incidental validation of file
ID length, not because derive_ipns_keypair rejects an all-zero secp256k1 key;
update the test to exercise the intended HKDF/secp256k1 paths by using a valid
file id length (>= MIN_FILE_ID_LENGTH) and a correctly-sized
compressed/uncompressed public key so path through derive_file_ipns_keypair ->
derive_ipns_keypair -> wrap_key is reached; specifically change the file id
string used when calling derive_file_ipns_keypair to be >=10 chars and supply a
65-byte uncompressed public_key (or generate a real secp256k1 keypair) instead
of the 33-byte zero array so the assertions on InodeKind::File
(file_meta_ipns_name, file_ipns_private_key, file_ipns_key_encrypted_hex)
validate the actual derivation behavior.
In `@apps/desktop/src/auth.ts`:
- Around line 483-486: The device factor metadata currently hardcodes platform:
'macos' causing incorrect platform labels; add a small helper (e.g.,
getDevicePlatform or getPlatform) that maps Node/Electron's process.platform
values to the expected strings ('macos' for 'darwin', 'windows' for 'win32',
'linux' for 'linux', fallback 'unknown'), place it alongside getDeviceName, and
replace the hardcoded "macos" in both inputRecoveryPhrase and pollApprovalStatus
with a call to this helper so device registration uses the correct platform
value.
---
Duplicate comments:
In `@apps/desktop/src-tauri/windows/installer-hooks.nsh`:
- Around line 19-23: The installer currently runs msiexec via ExecWait
('ExecWait "\"msiexec\" /i \"$INSTDIR\\resources\\winfsp-2.1.25156.msi\" /qn
INSTALLLEVEL=1000"') which requires elevation and will silently fail for
per-user installs; update the installer so WinFsp is installed with admin
rights: either change the installer configuration to force system-wide installs
(set installMode to system-wide only) so the whole NSIS installer runs elevated,
or replace the ExecWait call with an elevated execution (use NSIS UAC plugin or
ShellExecWait "runas" to invoke msiexec as admin), and keep the existing
fallback MessageBox for non-elevated failures (the winfsp_installed branch can
remain unchanged). Ensure the change references the same ExecWait/msiexec
invocation so WinFsp installation is attempted with elevation.
In `@apps/desktop/src/auth.ts`:
- Around line 725-729: The invoke('open_oauth_popup', { url: authUrl }) call
must not swallow IPC errors—ensure its .catch handler logs the error, calls
cleanup() to cancel any polling/timeout, and then rejects the outer promise with
a descriptive Error that includes the original err (use reject(new Error(`Failed
to open OAuth popup: ${err}`)) or similar) so failures propagate; verify this
behavior around the invoke, cleanup, and reject symbols.
---
Nitpick comments:
In `@apps/desktop/src-tauri/src/fuse/inode.rs`:
- Around line 319-329: The cleanup loop for removed children currently removes
only the direct inode entries from self.inodes and name_to_ino, leaving
descendant inodes orphaned; modify the loop in the merge-only branch to call
self.remove(old_ino) instead of directly removing entries, and for directory
children (when old_child.kind indicates a folder) perform a depth-first
traversal starting from old_ino to collect all descendant inodes and remove them
via self.remove to ensure name_to_ino and any other indexes are cleaned
consistently (use old_child_inos, self.inodes, normalize_name, and the existing
remove method to locate and clear descendants).
- Around line 233-235: The allocate_ino method uses a stronger memory ordering
than needed; change the atomic fetch_add on next_ino in fn allocate_ino to use
Ordering::Relaxed instead of Ordering::SeqCst so uniqueness is preserved without
unnecessary cross-thread ordering—update the fetch_add call in the allocate_ino
function to pass Ordering::Relaxed.
- Around line 578-585: The merge_only loop currently does an O(n²) check by
calling child_inos.contains(&old_ino) inside a loop over old_child_inos; replace
this with an O(1) membership check using a HashSet: construct a HashSet from
child_inos (or build one and keep it in sync), then iterate old_child_inos and
for each old_ino insert into the set and push to child_inos only if insertion
succeeds (i.e., it was not already present). Update the merge_only block that
references child_inos and old_child_inos and add the necessary use
std::collections::HashSet import.
- Around line 625-651: The code currently pattern-matches inode.kind three times
to preserve file_meta_ipns_name, file_ipns_private_key, and
file_ipns_key_encrypted_hex; change this to a single match on &inode.kind (e.g.,
let (preserved_meta_name, preserved_ipns_priv, preserved_ipns_encrypted) = match
&inode.kind { InodeKind::File { file_meta_ipns_name, file_ipns_private_key,
file_ipns_key_encrypted_hex, .. } => (file_meta_ipns_name.clone(),
file_ipns_private_key.clone(), file_ipns_key_encrypted_hex.clone()), _ => (None,
None, None), }; then use these three variables when constructing the new
InodeKind::File so you only pattern-match once and preserve the fields, keeping
the existing updates to inode.attr.size and inode.attr.blocks.
In `@apps/desktop/src-tauri/src/fuse/windows/mod.rs`:
- Around line 25-102: Both decrypt_metadata_from_ipfs and
decrypt_file_metadata_from_ipfs duplicate logic already implemented in the
shared fuse/decrypt module; remove the local implementations and delegate to the
shared functions (e.g., crate::fuse::decrypt::decrypt_metadata and
crate::fuse::decrypt::decrypt_file_metadata or the appropriate re-exports used
by operations.rs). Also remove the duplicate EncryptedFolderMetadata struct, or
import it from the shared module, and adapt the signatures to accept the same
input types as the shared helpers (convert hex/base64/sealed creation to use the
shared functions if they handle it). Update calls in mod.rs to call the shared
functions (preserving error mapping) and delete the duplicated code blocks in
decrypt_metadata_from_ipfs and decrypt_file_metadata_from_ipfs.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/ci.ymlapps/desktop/src-tauri/src/commands.rsapps/desktop/src-tauri/src/fuse/inode.rsapps/desktop/src-tauri/src/fuse/windows/mod.rsapps/desktop/src-tauri/windows/installer-hooks.nshapps/desktop/src/auth.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src-tauri/src/commands.rs
- Check both 64-bit and WOW6432Node registry hives for existing WinFsp - Handle msiexec exit code 3010 (reboot required) as success - Prevent failure MessageBox from falling through to success DetailPrint - Log errors from remove_dir_all in unmount_filesystem (consistency) - Pass explicit --features fuse to macOS tauri-action build Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 task
FSM1
added a commit
that referenced
this pull request
Feb 24, 2026
chore: add missed Phase 11 Windows Desktop entry to v0.16.0 changelog Release Please missed the Phase 11 commit (feat: Phase 11 — Windows Desktop) because the em dash in the subject confused the commit parser. The parser fix landed in #192 but too late for #189. Entire-Checkpoint: 14f35626e440 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Feb 28, 2026
This was referenced Mar 24, 2026
4 tasks
This was referenced Jun 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FileAttrsstruct replacingfuser::FileAttr,AccessModeenum replacing POSIX libc flags, shared modules gated withcfg(any(fuse, winfsp))OnceLock<AtomicBool>stop signalHKLM\SOFTWARE\WinFsp) + DLL verification at startup, non-blocking notification if missingcargo-check-windows+build-desktop-windowsonwindows-latestfuse/decrypt.rsfor platform-agnostic metadata decryptionTest plan
cargo-check-windowsCI job passes (first real compilation of WinFsp code)build-desktop-windowsCI job produces NSIS installer artifactcargo check --features fusestill passes (no regressions)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores