feat: remove v1 folder metadata, make v2 FilePointer canonical#150
Merged
Conversation
Eliminate v1/v2 dual-schema code to fix cross-device format oscillation bug where desktop writes v1 and web re-saves as hybrid v2 that the desktop parser rejects, causing empty FUSE directories. Phase 11.2 makes v2 (FilePointer) the only format, removes ~500 lines of dual-schema types/dispatch/conversion, and adds per-file IPNS publishing to desktop FUSE. 3 plans in 2 waves, verified by plan checker (2 blockers fixed in revision pass). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove FileEntry, FolderMetadataV2, FolderChildV2, AnyFolderMetadata types - Remove isV2Metadata type guard function - Rename FolderMetadataV2 -> FolderMetadata (version: 'v2' literal) - Rename FolderChildV2 -> FolderChild (FolderEntry | FilePointer) - validateFolderMetadata now rejects v1 data (only v2 accepted) - encryptFolderMetadata/decryptFolderMetadata accept/return FolderMetadata only Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace FolderChildV2 -> FolderChild across all 11 web app files - Replace FolderMetadataV2 -> FolderMetadata in folder.service.ts - Simplify isFilePointer guard (type discriminant only, no v1 check) - Remove v1 fallback branch from DetailsDialog render - Remove unnecessary type casts (metadata.children is already FolderChild[]) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nical - Delete FileEntry, AnyFolderMetadata, decrypt_any_folder_metadata, to_v1() - Rename FolderMetadataV2 -> FolderMetadata, FolderChildV2 -> FolderChild - New decrypt_folder_metadata rejects non-v2 version (strict validation) - Update tests: remove v1 tests, add v1 rejection test, use canonical types - Keep FileMetadata/default_encryption_mode unchanged (per-file IPNS schema) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace v1 FileEntry fixtures with v2 FilePointer fixtures - Change all version: 'v1' to version: 'v2' in test metadata - Add v1 rejection test (validateFolderMetadata throws CryptoError) - Add cid-without-fileMetaIpnsName rejection test - Remove isV2Metadata test block (function deleted) - Remove v1 backward compat test from file-ipns tests - All 225 tests pass Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 3/3 - Delete v1 types, rename v2 to canonical in crypto package - Update all web app imports and remove v1 guards - Update crypto package tests SUMMARY: .planning/phases/11.2-remove-v1-folder-metadata/11.2-01-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d_folder_metadata - Delete populate_folder (v1) and populate_folder_any dispatch - Rename populate_folder_v2 -> populate_folder (canonical) - build_folder_metadata emits FilePointer instead of FileEntry - decrypt_metadata_from_ipfs returns FolderMetadata directly (no AnyFolderMetadata) - PendingRefresh.metadata type changed to FolderMetadata - drain_refresh_completions stores metadata directly (no synthetic v1) - Pre-populate section uses populate_folder directly (no v1/v2 match) - init_vault and mkdir write version: "v2" - cache.rs test fixtures use version: "v2" - inode.rs test uses FilePointer instead of FileEntry Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - Delete v1 types, rename v2 to canonical in Rust crypto module - Rewrite FUSE layer and commands to v2-only with FilePointer SUMMARY: .planning/phases/11.2-remove-v1-folder-metadata/11.2-02-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ata publish in release() - create() derives file IPNS keypair via HKDF from privateKey + fileId - file_ipns_private_key field added to InodeKind::File for signing - release() encrypts FileMetadata with parent folder key and publishes to file's IPNS record - build_folder_metadata now skips files without valid file_meta_ipns_name (error, not placeholder) - get_folder_key() helper added to CipherBoxFS for accessing parent folder keys - publish_file_metadata() async helper handles encrypt + upload + IPNS sign + publish - mime_from_extension() detects MIME type from file extension for FileMetadata - All 118 tests pass Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oss-platform - Remove isV2 variable and v1 inline FileEntry branch from recovery.html - Add version check: reject non-v2 metadata with clear error message - Validate file children have fileMetaIpnsName, log error if missing - All 225 TypeScript crypto tests pass - Web app typecheck and build pass - All 118 Rust desktop tests pass - Zero v1 remnants in grep sweep across all source files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - Add per-file IPNS derivation in create() and FileMetadata publish in release() - Update recovery tool to v2-only and run full cross-platform verification SUMMARY: .planning/phases/11.2-remove-v1-folder-metadata/11.2-03-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughPhase 11.2 consolidates folder metadata from dual v1/v2 schemas to canonical v2 only across TypeScript and Rust codebases, renaming v2 types to remove suffixes, enforcing strict v2 validation that rejects v1, and integrating per-file IPNS publishing in desktop FUSE with derived keypair storage on file inodes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/desktop/src-tauri/src/fuse/mod.rs (1)
673-717:⚠️ Potential issue | 🟠 MajorFUSE thread blocked by synchronous FilePointer resolution — acknowledged TODO but high risk.
drain_refresh_completionsis called from FUSE callbacks on the single NFS thread. Theblock_with_timeoutloop (lines 695–699) performs synchronous network I/O for each unresolvedFilePointer, stalling the entire filesystem for up toN × 10s. The TODO on lines 674–677 acknowledges this needs refactoring to an async channel pattern.Given the coding guideline that "ALL FUSE callbacks run on a single thread" and "Any blocking call stalls the entire filesystem", this is a high-risk path. Even 2–3 unresolved pointers could stall Finder for 20–30 seconds on timeout. Consider prioritizing the async channel refactor before this ships to users.
As per coding guidelines:
apps/desktop/src-tauri/src/fuse/*.rs: ALL FUSE callbacks run on a single thread. Any blocking call stalls the entire filesystem. Background tasks communicate viampscchannels, drained inreaddir().🤖 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 673 - 717, This code blocks the single FUSE/NFS thread by calling block_with_timeout inside the drain loop; refactor drain_refresh_completions so it does not perform network I/O or blocking work: instead, for each unresolved (ino, ipns_name) found by get_unresolved_file_pointers and given the resolved folder key, send a lightweight job struct (ino, ipns_name, folder_key) into a new mpsc::Sender (e.g. file_pointer_tx) and return immediately; implement an async background worker that receives from file_pointer_rx, performs resolve_ipns and fetch_content, calls operations::decrypt_file_metadata_from_ipfs_public, and then posts the final result (or error) back to the main thread via an update channel which the FUSE thread drains to call self.inodes.resolve_file_pointer; remove block_with_timeout usage and any direct network/decrypt calls from drain_refresh_completions so the FUSE thread never blocks on network I/O.apps/desktop/src-tauri/src/fuse/inode.rs (1)
506-520:⚠️ Potential issue | 🟠 MajorScope
get_unresolved_file_pointersto parent inode to prevent cross-folder key mismatches.When
resolve_file_pointerfails (network error, decryption failure), the FilePointer remainsfile_meta_resolved: false. Subsequent calls toget_unresolved_file_pointers()from different folders return these persistent unresolved pointers alongside the current folder's FilePointers. Decryption then uses the wrong folder's key, causing AES-GCM authentication failures. No data corruption, but results in wasted round-trips, misleading warning logs, and O(total_unresolved) latency per refresh instead of O(folder_unresolved).Add a
parent_inoparameter toget_unresolved_file_pointersand return only direct children of that parent. Update all four call sites (operations.rs:217,mod.rs:678,mod.rs:867,mod.rs:932) to pass the relevant parent inode.Proposed fix
- pub fn get_unresolved_file_pointers(&self) -> Vec<(u64, String)> { - self.inodes.values().filter_map(|inode| { + pub fn get_unresolved_file_pointers(&self, parent_ino: u64) -> Vec<(u64, String)> { + let child_inos = self.inodes.get(&parent_ino) + .and_then(|p| p.children.as_ref()) + .cloned() + .unwrap_or_default(); + child_inos.iter().filter_map(|&ino| { + let inode = self.inodes.get(&ino)?; match &inode.kind { InodeKind::File { file_meta_ipns_name: Some(ipns_name), file_meta_resolved: false, .. } => Some((inode.ino, ipns_name.clone())), _ => None, } }).collect() }🤖 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 506 - 520, Change get_unresolved_file_pointers to accept a parent_ino parameter and only return File entries whose parent matches that parent_ino and file_meta_resolved is false (i.e., update the signature pub fn get_unresolved_file_pointers(&self, parent_ino: u64) -> Vec<(u64, String)> and filter with something like inode.parent == parent_ino and InodeKind::File { file_meta_ipns_name: Some(...), file_meta_resolved: false, ... }). Keep file_meta_resolved semantics and resolve_file_pointer behavior the same; then update every call site that currently calls get_unresolved_file_pointers() (there are four) to pass the appropriate parent inode from their local context so unresolved pointers are scoped per-folder. Ensure you reference the InodeKind::File match, inode.ino return value, and file_meta_ipns_name when making the filtered Vec.apps/desktop/src-tauri/src/fuse/operations.rs (1)
1556-1604:⚠️ Potential issue | 🟠 Major
upload_tx.sendfires before per-file IPNS publish — data race window.The background thread sends
UploadComplete(line 1567) before publishing the per-file IPNS record (line 1581). The main FUSE thread drains completions and may immediately build folder metadata with aFilePointerreferencing the file's IPNS name. If another device resolves that IPNS name beforepublish_file_metadatacompletes (or if it fails), the IPNS record won't exist yet, causing resolution errors on the other device.Consider either (a) publishing per-file IPNS before sending
UploadComplete, or (b) treating publish failure as a retriable error rather than a warning.Proposed reordering: publish file IPNS before notifying main thread
let result = rt.block_on(async { // 1. Upload encrypted file content to IPFS let file_cid = crate::api::ipfs::upload_content( &api, &ciphertext, ).await?; log::info!("File uploaded: ino {} -> CID {}", ino, file_cid); - // 2. Notify main thread of completed upload - let _ = upload_tx.send(crate::fuse::UploadComplete { - ino, - new_cid: file_cid.clone(), - parent_ino, - old_file_cid, - }); - - // 3. Publish per-file FileMetadata to file's own IPNS record + // 2. Publish per-file FileMetadata to file's own IPNS record if let (Some(ipns_key), Some(ipns_name), Some(folder_key)) = (&file_ipns_private_key, &file_meta_ipns_name, &folder_key_for_file_meta) { let mut file_meta_with_cid = file_meta; file_meta_with_cid.cid = file_cid.clone(); if let Err(e) = publish_file_metadata( &api, &file_meta_with_cid, folder_key, ipns_key, ipns_name, &coordinator, ).await { log::warn!("Per-file IPNS publish failed for ino {}: {}", ino, e); } } + // 3. Notify main thread of completed upload + let _ = upload_tx.send(crate::fuse::UploadComplete { + ino, + new_cid: file_cid.clone(), + parent_ino, + old_file_cid, + }); + Ok::<(), 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 1556 - 1604, The background thread sends UploadComplete via upload_tx.send before publish_file_metadata finishes, creating a race where the main thread can publish folder metadata referencing an IPNS name that isn't yet published; fix by moving the upload_tx.send call to after a successful publish_file_metadata (i.e., perform publish_file_metadata(&api, &file_meta_with_cid, folder_key, ipns_key, ipns_name, &coordinator).await and only on Ok send UploadComplete), and if publish_file_metadata fails implement a retry/backoff loop or return an Err so the background thread logs an error and does not notify success; update references around file_meta/file_meta_with_cid, publish_file_metadata, upload_tx.send, and UploadComplete accordingly.
🧹 Nitpick comments (8)
packages/crypto/src/__tests__/file-ipns.test.ts (1)
23-24: Nit: consolidate the two imports from the same module.♻️ Merge imports
-import type { FolderMetadata } from '../folder/types'; -import type { FolderEntry } from '../folder/types'; +import type { FolderMetadata, FolderEntry } from '../folder/types';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/crypto/src/__tests__/file-ipns.test.ts` around lines 23 - 24, Consolidate the two separate type imports in file-ipns.test.ts into a single import from the same module: replace the two lines importing FolderMetadata and FolderEntry from '../folder/types' with one consolidated import that includes both symbols (FolderMetadata, FolderEntry) to reduce redundancy and follow import style conventions.packages/crypto/src/folder/metadata.ts (1)
53-74: Consider validating folder-type children's required fields for parity with file validation.File children are validated for
fileMetaIpnsName, but folder children aren't validated for their required fields (ipnsName,folderKeyEncrypted,ipnsPrivateKeyEncrypted). A malformed folder entry would pass validation and only fail later at runtime when those fields are accessed.This is defense-in-depth on the decrypt path — the data originates from our own encrypt path, so risk is low, but the asymmetry could mask corruption or tampering.
♻️ Proposed validation for folder children
// v2 file children must have fileMetaIpnsName (FilePointer) if (entry.type === 'file') { const hasIpnsName = typeof entry.fileMetaIpnsName === 'string'; if (!hasIpnsName) { throw new CryptoError( 'Invalid metadata format: file entry must have fileMetaIpnsName', 'DECRYPTION_FAILED' ); } } + + // Folder children must have ipnsName, folderKeyEncrypted, ipnsPrivateKeyEncrypted + if (entry.type === 'folder') { + if ( + typeof entry.ipnsName !== 'string' || + typeof entry.folderKeyEncrypted !== 'string' || + typeof entry.ipnsPrivateKeyEncrypted !== 'string' + ) { + throw new CryptoError( + 'Invalid metadata format: folder entry missing required fields', + 'DECRYPTION_FAILED' + ); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/crypto/src/folder/metadata.ts` around lines 53 - 74, The validator currently checks v2 file children for fileMetaIpnsName but skips folder-specific required fields, so add checks in the same loop for when entry.type === 'folder' to ensure typeof entry.ipnsName === 'string', typeof entry.folderKeyEncrypted === 'string', and typeof entry.ipnsPrivateKeyEncrypted === 'string', and throw a CryptoError (same DECRYPTION_FAILED code) with a clear message (e.g., 'Invalid metadata format: folder entry missing ipnsName/folderKeyEncrypted/ipnsPrivateKeyEncrypted') if any are missing; locate this logic around the for (const child of obj.children) loop where CryptoError is used and mirror the file-child validation pattern for folder children.apps/web/public/recovery.html (1)
842-849: Consider moving the version check before the children check.The v2 rejection at line 842 runs after the early return for missing/empty
children(line 837). This means a v1 folder with no children silently returns instead of logging the version error. Moving the version gate above the children check gives consistent error reporting.Suggested reorder
const metadata = await decryptFolderMetadata(encryptedMeta, folderKey); + // Reject non-v2 metadata + if (metadata.version !== 'v2') { + log(` ERROR: Unsupported metadata version "${metadata.version}" for ${path || '/'} (only v2 supported). Skipping folder.`, 'error'); + errorCount++; + return; + } + if (!metadata.children || !Array.isArray(metadata.children)) { log(` No children in ${path || '/'}`, 'info'); return; } - // Reject non-v2 metadata - if (metadata.version !== 'v2') { - log(` ERROR: Unsupported metadata version "${metadata.version}" for ${path || '/'} (only v2 supported). Skipping folder.`, 'error'); - errorCount++; - return; - } - log(` Found ${metadata.children.length} items in ${path || '/'}`, 'success');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/public/recovery.html` around lines 842 - 849, Move the metadata version check so it runs before any checks on metadata.children: check metadata.version !== 'v2' and call log(..., 'error') and increment errorCount before you early-return for missing/empty children; update the block that currently inspects metadata.children and uses log(` Found ${metadata.children.length} items in ${path || '/'}`, 'success') to occur only after the version gate passes (use the same symbols metadata.version, metadata.children, log, errorCount, path to locate and reorder).apps/web/src/components/file-browser/FileListItem.tsx (1)
56-61: Nit: stale "(v2)" in JSDoc comment.Now that there's only one schema, the "(v2)" annotation on the
isFiletype guard doc comment is misleading. Consider updating to just "Type guard for file pointers."Suggested fix
/** - * Type guard for file pointers (v2). + * Type guard for file pointers. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/file-browser/FileListItem.tsx` around lines 56 - 61, Update the JSDoc above the isFile type guard to remove the stale "(v2)" annotation; change the comment text from "Type guard for file pointers (v2)." to "Type guard for file pointers." so the doc matches the current single-schema implementation and avoids confusion.apps/desktop/src-tauri/src/fuse/mod.rs (2)
689-716: Duplicate FilePointer resolution logic repeated 3 times.The same resolve-IPNS → fetch-content → decrypt-file-metadata → update-inode pattern appears in
drain_refresh_completions, and twice inmount_filesystem(root and subfolder). Consider extracting a helper likeresolve_file_pointers_for_folder(&mut self.inodes, parent_ino, folder_key, &api, &rt)to reduce duplication and ensure consistent error handling across all three call sites.Also applies to: 867-893, 932-957
🤖 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 689 - 716, Extract the repeated "resolve IPNS → fetch content → decrypt file metadata → update inode" logic into a single helper (e.g., resolve_file_pointers_for_folder) and call it from drain_refresh_completions and the two mount_filesystem sites to remove duplication; the helper should accept &mut self.inodes (or &mut InodeTable), parent ino, Option<FolderKey> folder_key, &api, &rt, iterate unresolved pairs, use block_with_timeout + crate::api::ipns::resolve_ipns and crate::api::ipfs::fetch_content, call operations::decrypt_file_metadata_from_ipfs_public, and invoke self.inodes.resolve_file_pointer with fm.cid, fm.file_key_encrypted, fm.file_iv, fm.size, fm.encryption_mode, preserving the existing warn logging for resolve/decrypt failures so behavior stays the same.
343-353: ReturnZeroizing<Vec<u8>>to ensure cryptographic key material is zeroized on drop.The method currently leaks key material into unprotected heap memory by returning plain
Vec<u8>. Theroot_folder_keyfield (line 299) is already wrapped inZeroizing<Vec<u8>>for this purpose, but.to_vec()returns unprotected bytes. Change the return type toOption<Zeroizing<Vec<u8>>>and wrap both return paths withZeroizing::new().Proposed fix
- pub fn get_folder_key(&self, folder_ino: u64) -> Option<Vec<u8>> { + pub fn get_folder_key(&self, folder_ino: u64) -> Option<Zeroizing<Vec<u8>>> { self.inodes.get(folder_ino).and_then(|inode| { match &inode.kind { - inode::InodeKind::Root { .. } => Some(self.root_folder_key.to_vec()), - inode::InodeKind::Folder { folder_key, .. } => Some(folder_key.to_vec()), + inode::InodeKind::Root { .. } => Some(Zeroizing::new(self.root_folder_key.to_vec())), + inode::InodeKind::Folder { folder_key, .. } => Some(Zeroizing::new(folder_key.to_vec())), _ => None, } }) }🤖 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 343 - 353, The get_folder_key method currently returns sensitive bytes as Vec<u8>; change its signature to return Option<Zeroizing<Vec<u8>>> and wrap both return values in Zeroizing::new(...) so key material is zeroized on drop: for Root return Zeroizing::new(self.root_folder_key.to_vec()) and for InodeKind::Folder return Zeroizing::new(folder_key.to_vec()); also add the Zeroizing import if missing and adjust any call sites to handle Option<Zeroizing<Vec<u8>>>.apps/desktop/src-tauri/src/fuse/operations.rs (2)
2369-2444: Significant code duplication across four decrypt helpers.
decrypt_metadata_from_ipfs(line 144),decrypt_metadata_from_ipfs_public(line 2372),decrypt_file_metadata_from_ipfs(line 285), anddecrypt_file_metadata_from_ipfs_public(line 2412) all repeat the same JSON parse → hex-decode IV → base64-decode ciphertext → construct sealed buffer logic. The two public wrappers also re-declareEncryptedFolderMetadatalocally.Extract the shared "JSON envelope → sealed bytes" conversion into one helper, then each function only needs to call the appropriate
decrypt_*on the result.Sketch: shared helper to eliminate duplication
+/// Parse the `{ "iv": hex, "data": base64 }` JSON envelope and reconstruct +/// the sealed AES-GCM payload (IV || ciphertext). +fn unseal_json_envelope(encrypted_bytes: &[u8]) -> Result<Vec<u8>, String> { + #[derive(serde::Deserialize)] + struct Envelope { iv: String, data: String } + + let env: Envelope = serde_json::from_slice(encrypted_bytes) + .map_err(|e| format!("Failed to parse encrypted JSON: {}", e))?; + let iv_bytes = hex::decode(&env.iv) + .map_err(|_| "Invalid IV hex".to_string())?; + 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(&env.data) + .map_err(|e| format!("Invalid base64: {}", e))?; + let mut sealed = Vec::with_capacity(12 + ciphertext.len()); + sealed.extend_from_slice(&iv_bytes); + sealed.extend_from_slice(&ciphertext); + Ok(sealed) +}Then each decrypt function reduces to ~3 lines.
🤖 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 2369 - 2444, Duplicate JSON-envelope parsing and sealed-bytes construction is repeated in decrypt_metadata_from_ipfs, decrypt_metadata_from_ipfs_public, decrypt_file_metadata_from_ipfs, and decrypt_file_metadata_from_ipfs_public; extract a single helper (e.g., parse_ipfs_encrypted_envelope or envelope_to_sealed_bytes) that takes encrypted_bytes: &[u8] and returns Result<Vec<u8>, String> by: 1) defining the EncryptedFolderMetadata struct once inside the helper, 2) serde_json::from_slice → hex::decode the iv and validate length == 12 → base64 decode the data, 3) build the sealed Vec by concatenating iv then ciphertext and return it or propagate formatted errors; then replace each decrypt_* function to call this helper and pass its Vec to crate::crypto::folder::decrypt_folder_metadata or decrypt_file_metadata (converting folder_key as before), removing the duplicated local struct and decode logic from those functions.
62-102: Consider using themime_guesscrate instead of a hand-rolled lookup.This static match covers ~35 extensions but will silently default to
application/octet-streamfor common types like.tiff,.woff2,.csv,.yaml,.rar,.7z, etc. Themime_guesscrate handles hundreds of extensions and stays current with IANA registrations.🤖 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 62 - 102, The hand-rolled mime_from_extension function should be replaced with mime_guess to cover many more extensions; add mime_guess to Cargo.toml, import it (e.g., use mime_guess::from_path) and change mime_from_extension to call from_path(filename).first_or_octet_stream() (or .first_raw() / .first() and fall back to application/octet-stream) and return the MIME as a String, removing the large static match; keep the function signature so callers are unchanged.
🤖 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/fuse/mod.rs`:
- Around line 481-501: The current loop silently skips files when
file_meta_ipns_name is missing (matching Some(name) else continue), which drops
entries from metadata; change this to return an explicit error instead of
continue so the caller can handle the inconsistency—replace the continue branch
with an Err(...) that includes child_ino, child.name and context (e.g., "missing
fileMetaIpnsName for FilePointer; create() should have derived IPNS") and
propagate that error from the enclosing function; ensure callers of this
function (the code that collects metadata_children and ultimately publishes) can
handle/propagate the Result rather than assuming success.
---
Outside diff comments:
In `@apps/desktop/src-tauri/src/fuse/inode.rs`:
- Around line 506-520: Change get_unresolved_file_pointers to accept a
parent_ino parameter and only return File entries whose parent matches that
parent_ino and file_meta_resolved is false (i.e., update the signature pub fn
get_unresolved_file_pointers(&self, parent_ino: u64) -> Vec<(u64, String)> and
filter with something like inode.parent == parent_ino and InodeKind::File {
file_meta_ipns_name: Some(...), file_meta_resolved: false, ... }). Keep
file_meta_resolved semantics and resolve_file_pointer behavior the same; then
update every call site that currently calls get_unresolved_file_pointers()
(there are four) to pass the appropriate parent inode from their local context
so unresolved pointers are scoped per-folder. Ensure you reference the
InodeKind::File match, inode.ino return value, and file_meta_ipns_name when
making the filtered Vec.
In `@apps/desktop/src-tauri/src/fuse/mod.rs`:
- Around line 673-717: This code blocks the single FUSE/NFS thread by calling
block_with_timeout inside the drain loop; refactor drain_refresh_completions so
it does not perform network I/O or blocking work: instead, for each unresolved
(ino, ipns_name) found by get_unresolved_file_pointers and given the resolved
folder key, send a lightweight job struct (ino, ipns_name, folder_key) into a
new mpsc::Sender (e.g. file_pointer_tx) and return immediately; implement an
async background worker that receives from file_pointer_rx, performs
resolve_ipns and fetch_content, calls
operations::decrypt_file_metadata_from_ipfs_public, and then posts the final
result (or error) back to the main thread via an update channel which the FUSE
thread drains to call self.inodes.resolve_file_pointer; remove
block_with_timeout usage and any direct network/decrypt calls from
drain_refresh_completions so the FUSE thread never blocks on network I/O.
In `@apps/desktop/src-tauri/src/fuse/operations.rs`:
- Around line 1556-1604: The background thread sends UploadComplete via
upload_tx.send before publish_file_metadata finishes, creating a race where the
main thread can publish folder metadata referencing an IPNS name that isn't yet
published; fix by moving the upload_tx.send call to after a successful
publish_file_metadata (i.e., perform publish_file_metadata(&api,
&file_meta_with_cid, folder_key, ipns_key, ipns_name, &coordinator).await and
only on Ok send UploadComplete), and if publish_file_metadata fails implement a
retry/backoff loop or return an Err so the background thread logs an error and
does not notify success; update references around file_meta/file_meta_with_cid,
publish_file_metadata, upload_tx.send, and UploadComplete accordingly.
---
Nitpick comments:
In `@apps/desktop/src-tauri/src/fuse/mod.rs`:
- Around line 689-716: Extract the repeated "resolve IPNS → fetch content →
decrypt file metadata → update inode" logic into a single helper (e.g.,
resolve_file_pointers_for_folder) and call it from drain_refresh_completions and
the two mount_filesystem sites to remove duplication; the helper should accept
&mut self.inodes (or &mut InodeTable), parent ino, Option<FolderKey> folder_key,
&api, &rt, iterate unresolved pairs, use block_with_timeout +
crate::api::ipns::resolve_ipns and crate::api::ipfs::fetch_content, call
operations::decrypt_file_metadata_from_ipfs_public, and invoke
self.inodes.resolve_file_pointer with fm.cid, fm.file_key_encrypted, fm.file_iv,
fm.size, fm.encryption_mode, preserving the existing warn logging for
resolve/decrypt failures so behavior stays the same.
- Around line 343-353: The get_folder_key method currently returns sensitive
bytes as Vec<u8>; change its signature to return Option<Zeroizing<Vec<u8>>> and
wrap both return values in Zeroizing::new(...) so key material is zeroized on
drop: for Root return Zeroizing::new(self.root_folder_key.to_vec()) and for
InodeKind::Folder return Zeroizing::new(folder_key.to_vec()); also add the
Zeroizing import if missing and adjust any call sites to handle
Option<Zeroizing<Vec<u8>>>.
In `@apps/desktop/src-tauri/src/fuse/operations.rs`:
- Around line 2369-2444: Duplicate JSON-envelope parsing and sealed-bytes
construction is repeated in decrypt_metadata_from_ipfs,
decrypt_metadata_from_ipfs_public, decrypt_file_metadata_from_ipfs, and
decrypt_file_metadata_from_ipfs_public; extract a single helper (e.g.,
parse_ipfs_encrypted_envelope or envelope_to_sealed_bytes) that takes
encrypted_bytes: &[u8] and returns Result<Vec<u8>, String> by: 1) defining the
EncryptedFolderMetadata struct once inside the helper, 2) serde_json::from_slice
→ hex::decode the iv and validate length == 12 → base64 decode the data, 3)
build the sealed Vec by concatenating iv then ciphertext and return it or
propagate formatted errors; then replace each decrypt_* function to call this
helper and pass its Vec to crate::crypto::folder::decrypt_folder_metadata or
decrypt_file_metadata (converting folder_key as before), removing the duplicated
local struct and decode logic from those functions.
- Around line 62-102: The hand-rolled mime_from_extension function should be
replaced with mime_guess to cover many more extensions; add mime_guess to
Cargo.toml, import it (e.g., use mime_guess::from_path) and change
mime_from_extension to call from_path(filename).first_or_octet_stream() (or
.first_raw() / .first() and fall back to application/octet-stream) and return
the MIME as a String, removing the large static match; keep the function
signature so callers are unchanged.
In `@apps/web/public/recovery.html`:
- Around line 842-849: Move the metadata version check so it runs before any
checks on metadata.children: check metadata.version !== 'v2' and call log(...,
'error') and increment errorCount before you early-return for missing/empty
children; update the block that currently inspects metadata.children and uses
log(` Found ${metadata.children.length} items in ${path || '/'}`, 'success') to
occur only after the version gate passes (use the same symbols metadata.version,
metadata.children, log, errorCount, path to locate and reorder).
In `@apps/web/src/components/file-browser/FileListItem.tsx`:
- Around line 56-61: Update the JSDoc above the isFile type guard to remove the
stale "(v2)" annotation; change the comment text from "Type guard for file
pointers (v2)." to "Type guard for file pointers." so the doc matches the
current single-schema implementation and avoids confusion.
In `@packages/crypto/src/__tests__/file-ipns.test.ts`:
- Around line 23-24: Consolidate the two separate type imports in
file-ipns.test.ts into a single import from the same module: replace the two
lines importing FolderMetadata and FolderEntry from '../folder/types' with one
consolidated import that includes both symbols (FolderMetadata, FolderEntry) to
reduce redundancy and follow import style conventions.
In `@packages/crypto/src/folder/metadata.ts`:
- Around line 53-74: The validator currently checks v2 file children for
fileMetaIpnsName but skips folder-specific required fields, so add checks in the
same loop for when entry.type === 'folder' to ensure typeof entry.ipnsName ===
'string', typeof entry.folderKeyEncrypted === 'string', and typeof
entry.ipnsPrivateKeyEncrypted === 'string', and throw a CryptoError (same
DECRYPTION_FAILED code) with a clear message (e.g., 'Invalid metadata format:
folder entry missing ipnsName/folderKeyEncrypted/ipnsPrivateKeyEncrypted') if
any are missing; locate this logic around the for (const child of obj.children)
loop where CryptoError is used and mirror the file-child validation pattern for
folder children.
Comment on lines
+481
to
501
| // FilePointer IPNS name is required for v2 metadata. | ||
| // create() derives the IPNS name via HKDF, so this should always be Some. | ||
| let ipns_name = match file_meta_ipns_name { | ||
| Some(name) if !name.is_empty() => name.clone(), | ||
| _ => { | ||
| log::error!( | ||
| "File '{}' (ino {}) has no fileMetaIpnsName -- this should not happen (create() derives IPNS keypair). Skipping file.", | ||
| child.name, child_ino | ||
| ); | ||
| continue; | ||
| } | ||
| }; | ||
|
|
||
| metadata_children.push(crate::crypto::folder::FolderChild::File( | ||
| crate::crypto::folder::FileEntry { | ||
| crate::crypto::folder::FilePointer { | ||
| id: uuid_from_ino(child_ino), | ||
| name: child.name.clone(), | ||
| cid: cid.clone(), | ||
| file_key_encrypted: encrypted_file_key.clone(), | ||
| file_iv: iv.clone(), | ||
| size: *size, | ||
| file_meta_ipns_name: ipns_name, | ||
| created_at: if created_ms > 0 { created_ms } else { now_ms }, | ||
| modified_at: if modified_ms > 0 { modified_ms } else { now_ms }, | ||
| encryption_mode: encryption_mode.clone(), | ||
| file_meta_ipns_name: file_meta_ipns_name.clone(), | ||
| }, |
There was a problem hiding this comment.
Silently skipping files with missing fileMetaIpnsName drops them from metadata — verify this is acceptable.
When a file inode lacks a fileMetaIpnsName (lines 483–492), the continue silently omits it from the folder metadata. If this ever triggers unexpectedly, the file vanishes from the folder listing on next publish with no recovery path other than the error log. The log::error! is good but the user has no visibility. Since the comment states this "should not happen" because create() derives the IPNS keypair, consider whether a hard error (returning Err(...)) would be safer — forcing the caller to handle the inconsistency rather than publishing incomplete metadata.
🤖 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 481 - 501, The current
loop silently skips files when file_meta_ipns_name is missing (matching
Some(name) else continue), which drops entries from metadata; change this to
return an explicit error instead of continue so the caller can handle the
inconsistency—replace the continue branch with an Err(...) that includes
child_ino, child.name and context (e.g., "missing fileMetaIpnsName for
FilePointer; create() should have derived IPNS") and propagate that error from
the enclosing function; ensure callers of this function (the code that collects
metadata_children and ultimately publishes) can handle/propagate the Result
rather than assuming success.
Owner
Author
There was a problem hiding this comment.
Good observation, but the skip-and-log approach is intentional here. In FUSE, a hard error would prevent the entire folder metadata publish from succeeding — blocking all other files in the folder because one entry is corrupt. The current behavior (skip + log::error!) is more resilient: partial success is better than total failure for a filesystem.
The missing-IPNS-name case should never happen in practice since create() always derives the keypair via HKDF and stores it on the inode. If it somehow does trigger, the error log gives us the diagnostic trail without cascading the failure.
There was a problem hiding this comment.
@FSM1, thank you for the clarification — that makes perfect sense. Partial success is indeed the right trade-off for filesystem resilience, especially when the corrupt entry case should never occur in normal operation. The log::error! provides the necessary diagnostic breadcrumbs without cascading the failure to other files in the folder.
I've noted this design decision for future reference.
✏️ Learnings added
Learnt from: FSM1
Repo: FSM1/cipher-box PR: 150
File: apps/desktop/src-tauri/src/fuse/mod.rs:481-501
Timestamp: 2026-02-19T03:15:45.866Z
Learning: In `apps/desktop/src-tauri/src/fuse/mod.rs` `build_folder_metadata()`: When serializing folder metadata, missing `file_meta_ipns_name` on a file inode triggers `log::error!` and `continue` (skip the file) rather than returning a hard error. This is intentional for filesystem resilience — partial success is preferred over blocking the entire folder metadata publish due to one corrupt entry.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: FSM1/cipher-box PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-27T04:37:16.091Z
Learning: Implement per-folder IPNS architecture where each folder has its own IPNS entry containing encrypted metadata with children array (files/folders with encrypted names, CIDs, encrypted keys, timestamps); always re-encrypt and republish the containing folder's IPNS entry when modifying files/folders.
Learnt from: CR
Repo: FSM1/cipher-box PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2026-02-19T00:17:48.341Z
Learning: Applies to apps/desktop/src-tauri/src/fuse/*.rs : Across all platforms: `InodeTable`, `MetadataCache`, `ContentCache` are platform-agnostic data structures. Use channel-based async prefetch pattern, debounced publish queue with per-folder coalescing, and `PublishCoordinator` for IPNS sequence number management
Learnt from: CR
Repo: FSM1/cipher-box PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2026-02-19T00:17:48.341Z
Learning: Applies to apps/desktop/src-tauri/src/fuse/mod.rs : File mutations trigger IPNS metadata publish via debounce queue with 1.5s debounce / 10s safety valve to coalesce rapid changes. Use `PublishCoordinator` for per-IPNS-name serialization and monotonic sequence numbers
Learnt from: FSM1
Repo: FSM1/cipher-box PR: 133
File: apps/web/src/components/file-browser/FileBrowser.tsx:285-285
Timestamp: 2026-02-17T01:48:33.094Z
Learning: Phase 12.6 v2 metadata migration is a clean-break requiring full DB wipe with no v1→v2 upgrade path; all existing vaults are destroyed and recreated, so v1 `FileEntry` data cannot exist at runtime post-migration, making `metadata.children as FolderChildV2[]` casts safe by design in apps/web components and services.
Learnt from: CR
Repo: FSM1/cipher-box PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2026-02-19T00:17:48.341Z
Learning: Applies to apps/desktop/src-tauri/src/fuse/mod.rs : Use FUSE-T's SMB backend on macOS, not NFS backend, due to macOS Sequoia 15.3+ kernel bug where NFS client never sends WRITE RPCs for newly created files
Learnt from: CR
Repo: FSM1/cipher-box PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2026-02-19T00:17:48.341Z
Learning: Applies to apps/desktop/src-tauri/src/fuse/mod.rs : On macOS: App cleans stale FUSE mount contents (including `.DS_Store`) before mount. If mount is stuck after crash, use `diskutil unmount force ~/CipherBox`
Learnt from: CR
Repo: FSM1/cipher-box PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-27T04:37:16.091Z
Learning: Applies to apps/web/**/*.{ts,tsx} : Every write operation (create/rename/move/delete file or folder) must: update in-memory folder metadata, re-encrypt the entire metadata JSON with the folder key, sign it with ECDSA private key, and call `POST /vault/publish-ipns` with the signed entry.
Learnt from: FSM1
Repo: FSM1/cipher-box PR: 114
File: apps/web/src/hooks/useFolder.ts:556-565
Timestamp: 2026-02-12T21:09:40.135Z
Learning: In the cipher-box IPFS/IPNS architecture: Orphaned IPNS records for deleted nested folders do not require explicit cleanup/unpublish operations. They naturally expire from the DHT after 48 hours when the TEE republisher stops processing them (since no encryptedIpnsPrivateKey is registered for orphaned folders).
Learnt from: FSM1
Repo: FSM1/cipher-box PR: 133
File: apps/web/src/services/folder.service.ts:0-0
Timestamp: 2026-02-17T01:48:50.861Z
Learning: Phase 12.6 implements a clean-break v2 migration for folder metadata in apps/web/src/services/folder.service.ts: all existing v1 data is wiped (DB wipe required), there is no v1→v2 upgrade path, and v1 FileEntry objects cannot exist at runtime by design. The cast `metadata.children as FolderChildV2[]` is safe because only v2 metadata can be loaded after migration.
Learnt from: CR
Repo: FSM1/cipher-box PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-27T04:37:16.091Z
Learning: MVP scope (v1.0) includes multi-method auth, E2E encrypted file upload/download, folder operations, React web UI, macOS FUSE desktop mount, and multi-device sync - defer billing, versioning, sharing, search, mobile, and Linux/Windows desktop to v1.1+.
5 tasks
This was referenced Feb 23, 2026
4 tasks
5 tasks
Merged
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
1 participant
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
FolderMetadata,FolderChild(no V2 suffix) across all platformscreate()derives file IPNS keypair via HKDF,release()publishes FileMetadata to file's own IPNS record after uploadvalidateFolderMetadataand Rustdecrypt_folder_metadataboth reject non-v2 metadata with explicit errorsBreaking Changes
This is a breaking schema change. After merge and deploy, the database must be wiped — existing v1 folder metadata will be rejected by all validators. No migration path (by design — clean break).
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor