feat: desktop FUSE journal durability and at-rest safety#533
Merged
Conversation
Entire-Checkpoint: 29e8f0ad16bd
Entire-Checkpoint: c5d1a25a475d
…ability-at-rest-safety # Conflicts: # .planning/STATE.md
…ogging Phase 52 Plan 01 (D-05, D-06): - Extend sanitize_error path scrub to /var, /tmp, /private, and Windows drive-letter X:\Users\ so host paths never leak into tray/notification copy - Replace three swallowed let _ = journal.remove sites with if let Err warn logging so a failed removal cannot silently cause a later double-publish - Add sanitize_error_extended_paths and remove_failure_is_logged tests Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Phase 52 Plan 02 (D-01, D-04): - Replace in-JSON ciphertext_b64 with a 0600 <id>.bin sidecar referenced by sidecar_path + sidecar_sha256, eliminating the multi-GB serde_json alloc - Rename plaintext filename/name to ECIES-encrypted filename_encrypted_hex / name_encrypted_hex with serde alias for one-time legacy replay compat - Add put_with_sidecar streaming writer and sidecar-aware remove - Add MAX_JOURNAL_PAYLOAD_BYTES, JOURNAL_GC_MAX_AGE_DAYS, JOURNAL_GC_MAX_SIZE_BYTES - sidecar_path + sidecar_sha256 default so pre-Phase-52 inline entries still load Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rable-ack Phase 52 Plan 03 (D-01, D-04): - Reject files over MAX_JOURNAL_PAYLOAD_BYTES in release() with EIO before any key generation, so the OOM/stall path holds no sensitive material - Build UploadFile/Mkdir journal entries with sidecar_path + sidecar_sha256 and ECIES-encrypted filename_encrypted_hex / name_encrypted_hex (failed name encrypt fails the write) - Move the heavy ciphertext-sidecar write + fsync off the FUSE callback thread onto an OS thread, callback blocks on a bounded recv_timeout before reply.ok() so the Phase-43 durable-ack contract (no false-ack) is preserved - Make NETWORK_TIMEOUT pub(crate) and add sha2 to the fuse crate Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oncurrent mount Phase 52 Plan 04 (D-03, D-04): - Replay reads ciphertext from the <id>.bin sidecar and verifies its SHA-256 before re-upload; a missing/empty path or hash mismatch returns Err and retains the entry instead of uploading absent or corrupt ciphertext - Decrypt filename_encrypted_hex / name_encrypted_hex transiently via decrypt_journal_name with passthrough-once legacy compat, never re-persisted - Bound every replay entry's network ops with tokio::time::timeout (mkdir 3x, upload 18x of NETWORK_TIMEOUT); a hung entry routes through record_failure - Run replay concurrently via rt.spawn on both the Unix and Windows mount paths, cloning key bytes before the Zeroizing move so the FS owns the sole copy Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Phase 52 Plan 05 (D-02): - Add WriteQueue::purge_vault to remove every .json + .bin for one vault (all statuses) via the sidecar-aware remove, closing the cross-vault retention leak in the shared journal dir - Add WriteQueue::gc_failed_entries: age-purge Failed entries, trim oldest-first to a size budget counting the .bin sidecar, and clean .bin orphans with no matching .json; best-effort, only Failed entries, never panics - Wire purge_vault into logout() before clear_keys (reads root_ipns_name which clear_keys zeroes); document the future switch_account/delete_account hook - Run gc_failed_entries at mount with JOURNAL_GC_MAX_AGE_DAYS / JOURNAL_GC_MAX_SIZE_BYTES, non-fatal so it never blocks the mount Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collapse the three identical warn-and-passthrough blocks in decrypt_journal_name into one closure, and merge the duplicated Unix/Windows path-scrub branches in regex_replace_paths into a single is_path_start check with one skip loop. Behavior-identical; cipherbox-fuse 64/64 and cipherbox-sdk 57/57 still green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CodeRabbit flagged that the winfsp mount path was missing the D-02 gc_failed_entries sweep present on the Unix path, leaking parked Failed entries and orphaned .bin sidecars across mounts on Windows. Add the same non-fatal mount-time GC before the concurrent replay spawn. Also correct the STATE.md Current focus to point at Phase 52. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 9 minutes and 30 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughPhase 52 hardens the FUSE write-journal and replay path against HARD-03 requirements. Ciphertext moves from inline JSON to 0600 ChangesPhase 52: Desktop FUSE Durability & At-Rest Safety
Phase 52 Planning Documentation
Sequence Diagram(s)sequenceDiagram
participant App as Desktop App
participant Auth as auth.rs logout
participant FuseMod as fuse/mod.rs mount
participant WQ as WriteQueue
participant RT as rt.spawn (replay task)
participant FUSEFS as CipherBoxFS
participant RO as read_ops handle_release
participant ST as std::thread (sidecar writer)
rect rgba(0, 100, 200, 0.5)
note over FuseMod,FUSEFS: Mount startup
FuseMod->>WQ: gc_failed_entries(age, size)
WQ-->>FuseMod: removed count
FuseMod->>RT: rt.spawn(replay_for_vault)
FuseMod->>FUSEFS: construct + mount
RT-->>RT: sidecar verify + ECIES decrypt + timeout(18× / 3×)
end
rect rgba(0, 150, 100, 0.5)
note over RO,ST: File release (durable-ack)
RO->>ST: std::thread::spawn(put_with_sidecar)
ST->>ST: write id.bin (0600) + fsync
ST->>ST: write id.json
ST-->>RO: mpsc send Ok/Err
alt within NETWORK_TIMEOUT * 18
RO->>FUSEFS: reply.ok()
else timeout
RO->>FUSEFS: reply.error(EIO)
end
end
rect rgba(200, 80, 0, 0.5)
note over Auth,WQ: Logout
Auth->>Auth: unmount + keychain delete
Auth->>WQ: purge_vault(ipns)
WQ-->>Auth: count / warn on err
Auth->>Auth: state.clear_keys()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
github-actions
Bot
added
release:cipherbox-fuse:feat
Contributor
Release Preview
|
Greptile SummaryThis PR (Phase 52) hardens the desktop FUSE write journal with five focused hardening sub-phases: sidecar-based ciphertext storage (eliminating multi-GB serde_json allocations), ECIES encryption of filenames/dir-names at rest, off-thread durable-ack writes with a bounded timeout, replay sidecar integrity verification + concurrent network timeouts, and a
Confidence Score: 5/5Safe to merge — no regressions found in the durable-ack contract, the sidecar write path, replay correctness, or the GC logic; all previously-reviewed issues are resolved. The sidecar write path, JSON-first remove ordering, purge_vault best-effort loop, gc_failed_entries 3-pass logic, migrate_legacy_inline in record_failure, and the concurrent replay spawn were all reviewed and found correct. The two observations are minor logging and coverage gaps that do not affect data safety or correctness. No files require special attention; the two suggestions are in crates/sdk/src/queue.rs (misleading log on purged entry) and crates/sdk/src/sync.rs (missing all-uppercase Windows path variant). Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant FUSE as FUSE callback thread
participant BG as OS background thread
participant DISK as Disk (journal dir)
participant RT as Tokio runtime
participant NET as Network (IPFS/IPNS)
Note over FUSE: release() called (file close)
FUSE->>FUSE: size cap check (2 GiB max)
FUSE->>FUSE: encrypt plaintext to ciphertext
FUSE->>FUSE: ecies wrap_key filename to filename_encrypted_hex
FUSE->>FUSE: build JournalEntry (sidecar_path + sidecar_sha256)
FUSE->>BG: spawn OS thread put_with_sidecar
BG->>DISK: write id.bin (0600, chunked, fsync)
BG->>DISK: write id.json (fsync + parent dir sync)
BG-->>FUSE: send Ok via channel
FUSE->>FUSE: recv_timeout 180s durable ack
FUSE-->>FUSE: reply.ok()
FUSE->>RT: spawn upload task
RT->>NET: upload ciphertext to CID
RT->>NET: publish IPNS records
RT->>DISK: journal.remove id
Note over RT: On next mount replay path
RT->>DISK: gc_failed_entries age size orphan purge
RT->>DISK: load_all_for_vault entries
RT->>DISK: read id.bin verify SHA-256
RT->>DISK: decrypt_journal_name filename_encrypted_hex
RT->>NET: re-upload ciphertext timeout 180s
RT->>DISK: journal.remove id on success
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant FUSE as FUSE callback thread
participant BG as OS background thread
participant DISK as Disk (journal dir)
participant RT as Tokio runtime
participant NET as Network (IPFS/IPNS)
Note over FUSE: release() called (file close)
FUSE->>FUSE: size cap check (2 GiB max)
FUSE->>FUSE: encrypt plaintext to ciphertext
FUSE->>FUSE: ecies wrap_key filename to filename_encrypted_hex
FUSE->>FUSE: build JournalEntry (sidecar_path + sidecar_sha256)
FUSE->>BG: spawn OS thread put_with_sidecar
BG->>DISK: write id.bin (0600, chunked, fsync)
BG->>DISK: write id.json (fsync + parent dir sync)
BG-->>FUSE: send Ok via channel
FUSE->>FUSE: recv_timeout 180s durable ack
FUSE-->>FUSE: reply.ok()
FUSE->>RT: spawn upload task
RT->>NET: upload ciphertext to CID
RT->>NET: publish IPNS records
RT->>DISK: journal.remove id
Note over RT: On next mount replay path
RT->>DISK: gc_failed_entries age size orphan purge
RT->>DISK: load_all_for_vault entries
RT->>DISK: read id.bin verify SHA-256
RT->>DISK: decrypt_journal_name filename_encrypted_hex
RT->>NET: re-upload ciphertext timeout 180s
RT->>DISK: journal.remove id on success
Reviews (6): Last reviewed commit: "Merge branch 'main' into feat/desktop-fu..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/fuse/src/journal_helpers.rs`:
- Around line 135-141: The file size validation check happens after read_all()
has already loaded the entire file into memory, which defeats the purpose of the
guard and can still cause memory allocation for oversized files. Retrieve the
file size from the handle's metadata before calling read_all(), check if the
file_size exceeds MAX_JOURNAL_PAYLOAD_BYTES, and only proceed to read_all() if
the file is within the size limit, otherwise return an error immediately.
In `@crates/fuse/src/lib.rs`:
- Around line 2241-2253: The error messages in this block include the raw
sidecar_path using {:?} formatting, which can leak sensitive system paths
through logs and error recording. Remove the sidecar_path parameter from both
error messages: the map_err closure when reading the ciphertext sidecar and the
error returned for the hash mismatch check. Retain the descriptive error
information (like the actual error details from reading and the hash values for
the mismatch case) but exclude the path itself to prevent exposing system
directories.
In `@crates/fuse/src/read_ops.rs`:
- Around line 838-847: The error handling in the recv_timeout match statement
(specifically the Timeout and Disconnected cases) returns errors immediately
without ensuring the detached writer thread has definitely finished. Since the
spawned thread can still be executing put_with_sidecar and complete a journal
commit after we return an error, this creates a race condition where a late
.json commit replays the write on the next mount despite reporting failure to
the OS. Before returning an error in these failure cases, either make the
visible journal commit cancellable or staged so it doesn't proceed if the close
fails, or add logic to wait for a definitive outcome from the writer thread that
confirms it has truly stopped before returning the error response.
- Around line 825-832: The code is unnecessarily cloning result.ciphertext which
can duplicate up to 2 GiB of data. Instead of cloning put_ciphertext for the
spawned thread, change the channel type from Result<(), String> to include the
ciphertext in the return value, then move result.ciphertext directly into the
spawned thread (not clone it). After put_journal.put_with_sidecar completes,
have the spawned thread send back both the durability result and the ciphertext
through the channel so it can be reused for the later upload thread without
duplication.
In `@crates/sdk/src/queue.rs`:
- Around line 403-409: The purge_vault method removes entries, but detached
upload workers that hold a journal_entry_snapshot can still call record_failure
later, which recreates the purged entry without its sidecar files. To fix this,
modify the record_failure method to check whether the entry file still exists
before attempting to put the entry back into the queue; if the file was already
removed by purge_vault, record_failure should become a no-op instead of
recreating the entry. This prevents stale workers from resurrecting purged
entries with incomplete data.
- Around line 49-67: Add a new compat field called `legacy_ciphertext_b64` to
capture the old inline ciphertext during deserialization of pre-Phase-52
entries. Mark this field with both `#[serde(default)]` to allow deserialization
of entries without it, and `#[serde(skip_serializing)]` to prevent it from being
written back to JSON on serialization. This ensures that when old entries with
inline `ciphertext_b64` are loaded, the ciphertext is preserved in memory for
one-time passthrough replay before the entry is discarded, rather than being
silently dropped during deserialization.
- Around line 273-276: The put_with_sidecar method writes the sidecar file to
self.sidecar_path_for(entry.id) but does not validate that entry.sidecar_path
matches the actual persisted path before committing the JournalEntry to JSON.
Before persisting the entry, validate that entry.sidecar_path is consistent with
the computed bin_path from self.sidecar_path_for(id), or update the entry to
reflect the actual sidecar path that was written. This ensures that during
replay, the persisted entry points to the correct sidecar file location that was
just fsynced.
- Around line 321-342: The remove method currently removes the binary sidecar
file (bin_path) before removing the JSON file (json_path). This ordering creates
a window where a crash could leave an orphaned .json entry with no payload,
causing issues on replay. Reverse the order by moving the
std::fs::remove_file(&json_path) call and its fsync logic to execute before the
sidecar removal, so the JSON file is removed and synced first. This ensures that
if a crash occurs, only an orphaned .bin file remains, which is safe since
garbage collection can clean it up without triggering replay.
- Around line 293-301: The code in the journal sidecar write section does not
clean up partial binary files when write_all or sync_all operations fail,
leaving orphaned `.bin` files until garbage collection runs. Wrap the entire
bin_file write loop and sync_all calls in error handling that deletes the
sidecar file when either operation fails. Ensure that when map_err is called on
write_all for the chunk writing loop or on sync_all, the code attempts to remove
the bin_file before returning the error, so partial sidecars are cleaned up
immediately rather than waiting for a later GC pass.
In `@crates/sdk/src/sync.rs`:
- Around line 344-394: The test function sanitize_error_extended_paths does not
cover case-insensitive Windows paths, specifically lowercase drive letters and
lowercase directory names. Add two additional assert_eq! test cases within the
sanitize_error_extended_paths function after the existing Windows drive-letter
path tests: one testing a lowercase drive letter followed by an uppercase path
component (e.g., c:\Users\alice\file), and another testing an uppercase drive
letter followed by a lowercase "users" directory (e.g., C:\users\bob\file). Both
should verify that the sanitize_error function correctly replaces the path with
[path] regardless of case.
- Around line 271-285: The Windows path matching logic for the is_path_start
condition is case-sensitive and only matches uppercase drive letters and paths
with uppercase backslash-Users-backslash. To fix this, change the check from
c.is_ascii_uppercase() to c.is_ascii_alphabetic() to accept both uppercase and
lowercase drive letters, and modify the path comparison logic to use
case-insensitive matching (convert the substring to lowercase before comparison
or use a method that performs case-insensitive comparison) so that variations
like c:\users\, C:\users\, and c:\Users\ are all properly detected and scrubbed
to [path].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 21cbc484-649e-410e-af75-27f4ba14bc76
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.planning/ROADMAP.md.planning/STATE.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-01-PLAN.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-01-SUMMARY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-02-PLAN.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-02-SUMMARY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-03-PLAN.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-03-SUMMARY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-04-PLAN.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-04-SUMMARY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-05-PLAN.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-05-SUMMARY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-CONTEXT.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-DISCUSSION-LOG.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-PATTERNS.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-RESEARCH.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-SECURITY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-VALIDATION.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-VERIFICATION.mdapps/desktop/src-tauri/src/commands/auth.rsapps/desktop/src-tauri/src/fuse/mod.rsapps/desktop/src-tauri/src/fuse/windows/mod.rscrates/fuse/Cargo.tomlcrates/fuse/src/journal_helpers.rscrates/fuse/src/lib.rscrates/fuse/src/read_ops.rscrates/fuse/src/write_ops.rscrates/sdk/Cargo.tomlcrates/sdk/src/lib.rscrates/sdk/src/queue.rscrates/sdk/src/sync.rsrelease-please-config.json
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #533 +/- ##
==========================================
+ Coverage 64.83% 67.99% +3.15%
==========================================
Files 143 159 +16
Lines 11147 18955 +7808
Branches 1258 1258
==========================================
+ Hits 7227 12888 +5661
- Misses 3675 5822 +2147
Partials 245 245
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- check payload size cap from temp-file metadata before read_all - avoid cloning ciphertext for sidecar writer; move and return via channel - scrub sidecar host paths from replay error strings - preserve legacy inline ciphertext for one-time passthrough replay - validate sidecar path before committing journal JSON - clean up partial sidecar on write/fsync failure - remove journal JSON before its sidecar to avoid dangling replay trigger - make purge_vault best-effort instead of fail-fast - no-op record_failure when the entry file was already purged - case-insensitive Windows path scrubbing plus tests Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/sdk/src/queue.rs (1)
611-635:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftClose the purge-vs-failure TOCTOU window.
The
exists()guard only handles entries purged before Line 617. Ifpurge_vaultunlinks the JSON after that check but before the laterput/update_status,record_failurecan still recreate a logged-out vault entry, often after its sidecar was deleted. Use a no-create rewrite/open, tombstone, or cancellation barrier so missing-after-purge cannot be recreated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/sdk/src/queue.rs` around lines 611 - 635, The exists() check in record_failure creates a race condition window where purge_vault can delete the JSON file after the check but before put() or update_status() are called, allowing the purged entry to be recreated without its sidecar. Modify the put() and update_status() methods to safely handle the case where the entry file is missing after the initial check, either by using atomic open flags that prevent file creation if it doesn't exist, or by adding defensive existence checks within put() and update_status() before performing their writes so that a post-purge deletion cannot cause resurrection of the entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/fuse/src/journal_helpers.rs`:
- Around line 683-696: Remove the local cap_guard closure that reimplements the
payload size check logic, as it doesn't actually test the real implementation.
Instead, create an oversized sparse or truncated temporary file handle and call
the actual build_upload_journal_entry function directly with this handle to
verify that the function properly rejects payloads exceeding the cap before
attempting to read them, which will catch regressions where the size check is
inadvertently removed or the read operation happens before the check.
In `@crates/fuse/src/lib.rs`:
- Around line 3498-3518: The test is using permission-based failure simulation
to trigger an error when calling journal.remove(id), but this approach is
unreliable and environment-dependent since root or capability-elevated Unix
runners can still unlink read-only directories. Instead of manipulating
directory permissions with set_mode(0o500), create an actual directory at the
path `<id>.json` before calling journal.remove(id); this will cause remove_file
to deterministically fail with a non-NotFound error (attempting to remove a file
when a directory exists at that path) without relying on permission checks.
Remove all the permission manipulation code including the std::fs::metadata
calls, perms.set_mode calls, and std::fs::set_permissions calls, and replace
them with a single call to std::fs::create_dir that creates a directory at the
target path.
- Around line 2149-2168: The decrypt_journal_name function currently treats all
decryption and UTF-8 conversion failures as legacy plaintext, allowing corrupted
Phase-52 encrypted data to be replayed as hex blobs. Change the return type from
String to Result<String, String>, and refactor the logic so that only hex decode
failures (which indicate pre-Phase-52 plaintext) use the legacy passthrough
fallback, while errors from cipherbox_crypto::ecies::unwrap_key and
String::from_utf8 conversion failures route through record_failure instead,
since these indicate corrupted Phase-52 data that should not be replayed.
- Around line 1545-1573: The sidecar_path field is being used directly from the
persisted journal entry in the replay pattern match, but this creates a security
issue where a tampered or stale entry could point to the wrong file. Instead of
trusting the persisted sidecar_path value, derive it programmatically using
journal.sidecar_path_for with the entry ID. For non-legacy entries in this match
arm, replace the direct usage of sidecar_path with a call to
journal.sidecar_path_for(&entry.id) before passing it to the replay_upload_entry
function call. If the entry is legacy, apply an alternative approach such as
canonicalizing and validating that the path is in the expected format
<journal_dir>/<id>.bin.
In `@crates/sdk/src/queue.rs`:
- Around line 282-346: The put_with_sidecar method writes ciphertext bytes to
disk and persists the caller-provided sidecar_sha256 without validating that the
hash matches the actual bytes being written. Compute the SHA-256 hash of the
ciphertext parameter after successfully writing and fsyncing it, then extract
the sidecar_sha256 from the UploadFile variant in entry.op and compare them
before calling self.put(entry). Return an error with a descriptive message if
the hashes do not match, ensuring the JSON entry is only written when the hash
verification succeeds.
- Around line 68-79: The `legacy_ciphertext_b64` field uses `skip_serializing`,
which prevents it from being written back to disk when an entry is re-persisted.
When a pre-Phase-52 entry with inline ciphertext undergoes a status or retry
rewrite, the field is lost in memory but never persisted to a sidecar, resulting
in a missing-payload entry. Before any status change, retry modification, or
re-put operation on an entry containing `legacy_ciphertext_b64` data, migrate
the legacy ciphertext bytes to the canonical `.bin` sidecar file format. This
ensures the payload is persisted in the sidecar before the in-memory field is
dropped during serialization, preventing data loss on transient failures.
- Around line 509-517: The malformed JSON error handling in the deserialization
section (where Err(e) is caught in the serde_json::from_slice call) currently
only logs and skips the malformed entry, but this leaves the matching sidecar
file intact because the JSON file still exists. To fix this, track which JSON
stems are successfully parsed (store valid entry identifiers), and ensure that
in pass 3 of the GC logic (around line 556-565), treat malformed JSON entries as
having no live owner so the matching sidecar file can be removed. Either
maintain a collection of parseable JSON stems that pass 3 can check, or actively
quarantine/remove malformed JSON and sidecar pairs together during the error
handling to allow GC to clean them up.
---
Outside diff comments:
In `@crates/sdk/src/queue.rs`:
- Around line 611-635: The exists() check in record_failure creates a race
condition window where purge_vault can delete the JSON file after the check but
before put() or update_status() are called, allowing the purged entry to be
recreated without its sidecar. Modify the put() and update_status() methods to
safely handle the case where the entry file is missing after the initial check,
either by using atomic open flags that prevent file creation if it doesn't
exist, or by adding defensive existence checks within put() and update_status()
before performing their writes so that a post-purge deletion cannot cause
resurrection of the entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 00713255-692d-4fc3-8e2b-bce0fe614365
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
.planning/ROADMAP.md.planning/STATE.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-01-PLAN.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-01-SUMMARY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-02-PLAN.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-02-SUMMARY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-03-PLAN.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-03-SUMMARY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-04-PLAN.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-04-SUMMARY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-05-PLAN.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-05-SUMMARY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-CONTEXT.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-DISCUSSION-LOG.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-PATTERNS.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-RESEARCH.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-SECURITY.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-VALIDATION.md.planning/phases/52-desktop-fuse-durability-at-rest-safety/52-VERIFICATION.md.planning/todos/completed/2026-06-18-fuse-journal-growth-and-replay-timeout.mdapps/desktop/src-tauri/src/commands/auth.rsapps/desktop/src-tauri/src/fuse/mod.rsapps/desktop/src-tauri/src/fuse/windows/mod.rscrates/fuse/Cargo.tomlcrates/fuse/src/journal_helpers.rscrates/fuse/src/lib.rscrates/fuse/src/read_ops.rscrates/fuse/src/write_ops.rscrates/sdk/Cargo.tomlcrates/sdk/src/lib.rscrates/sdk/src/queue.rscrates/sdk/src/sync.rsrelease-please-config.json
decrypt_journal_name distinguished a pre-Phase-52 plaintext name from a Phase-52 ECIES ciphertext using hex-validity alone. A legacy filename that is itself pure even-length hex — a hyphen-less UUID, a SHA-1/SHA-256 digest, a git object id — hex-decodes, fails ecies::unwrap_key, and returns Err, parking the entry as Failed until gc_failed_entries age-purges it (sidecar .bin and all), destroying the captured write. Gate the ECIES decrypt path on a length floor: a genuine ECIES name is always ephemeral_pubkey || nonce || tag || ciphertext, and in-place bit-rot does not shrink it, so any hex value shorter than ECIES_MIN_CIPHERTEXT_SIZE cannot be one and is passed through verbatim. Corruption of a real, long-enough ECIES name still returns Err and is retained for retry. Resolves Greptile P1 on PR #533. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Phase 52: Desktop FUSE Durability & At-Rest Safety
Hardens the desktop FUSE write journal against data loss, host-path leakage, unbounded growth, and at-rest plaintext exposure. Part of the v1.1 hardening block (Phases 50-55), closing tracked todo
#9.What changed
52-01 — at-rest safety basics (D-05, D-06)
sanitize_errorpath-scrub to/var,/tmp,/private, and Windows drive-letterX:\Users\so host paths never leak into tray/notification copy.let _ = journal.removesites withif let Err … warnlogging so a failed removal cannot silently cause a later double-publish.52-02 — ciphertext sidecar + encrypted-name journal shape (D-01, D-04)
ciphertext_b64blob with a0600<id>.binsidecar referenced bysidecar_path+sidecar_sha256, eliminating the multi-GBserde_jsonallocation on the FS thread.filename/nameto ECIES-encryptedfilename_encrypted_hex/name_encrypted_hex(with#[serde(alias)]+#[serde(default)]one-time legacy replay compat).52-03 — write-path size cap, filename encryption, off-thread durable-ack (D-01, D-04)
MAX_JOURNAL_PAYLOAD_BYTES(2 GiB) inrelease()with EIO before any key generation.recv_timeoutbeforereply.ok()so the Phase-43 durable-ack contract (no false-ack) is preserved.52-04 — replay sidecar read, name decryption, timeout, concurrent mount (D-03, D-04)
<id>.binsidecar and verifies its SHA-256 before re-upload; a missing/empty path or hash mismatch retains the entry instead of uploading absent/corrupt ciphertext.tokio::time::timeout(mkdir 3x, upload 18x ofNETWORK_TIMEOUT); a hung entry routes throughrecord_failure.rt.spawnon both the Unix and Windows mount paths so the mount returns immediately.52-05 — journal retention purge + Failed-entry GC (D-02)
WriteQueue::purge_vaultremoves every.json+.binfor one vault (all statuses), closing the cross-vault retention leak in the shared journal dir; wired intologout()beforeclear_keys().WriteQueue::gc_failed_entriesages out parkedFailedentries, trims oldest-first to a 500 MiB budget (counting the.bin), and cleans.binorphans; run at mount, best-effort, never blocks the mount.Verification
cargo test -p cipherbox-fuse— 64/64 greencargo test -p cipherbox-sdk— 57/57 green (54 baseline + 3 new GC/purge tests:purge_vault_removes_all,gc_purges_old_failed,gc_purges_to_size_budget)cargo check -p cipherbox-desktop --features fuse— cleanzeroize/signature/signlines removed; fail-closed signature handling and key zeroization untouched.Notes
--features winfspcheck fails only in upstreamwindows_corecrates on macOS (IMarshal/marshaler); validated locally under thefusefeature and relying on CI's Windows runner for winfsp.cipherbox-sdkqueue) and touches no TS publish/resolve, CAS sequencing, or key-lifecycle code.🤖 Generated with Claude Code
Summary by CodeRabbit