Skip to content

Fix cipher write safety and retry semantics - #371

Open
m11y wants to merge 1 commit into
doy:mainfrom
m11y:fix/cipher-write-safety
Open

m11y wants to merge 1 commit into
doy:mainfrom
m11y:fix/cipher-write-safety

Conversation

@m11y

@m11y m11y commented Sep 3, 2026 •

Copy link
Copy Markdown

Summary

  • Encrypt edited values with an entry's individual item key and send the key plus reprompt, favorite, archivedDate, and lastKnownRevisionDate on full cipher PUTs.
  • Sync before selector resolution and after successful writes; update the agent's reprompt set only after the vault is persisted, and report post-write local failures with exit 2 so callers do not retry a write that landed.
  • Preserve failed interactive editor buffers with restrictive permissions while keeping piped secrets off disk. This removes the unsafe actions::edit / Client::edit library entry points in favor of edit_with_meta and adds fields to db::Entry, which is a documented Rust source break.

Relationship to #368

This overlaps #368 by @Mic92, which was opened first and fixes the same item-key corruption more narrowly. The item-key approach here is the same one; this PR additionally preserves reprompt / favorite / archivedDate, sends lastKnownRevisionDate so a stale full PUT cannot clobber a concurrent edit, fixes the agent's reprompt-set ordering, and reworks post-write exit semantics. Those additions are why it renames edit to edit_with_meta instead of adding a key parameter, so the two branches conflict textually.

Either order works. If #368 merges first I will rebase onto it and drop the duplicated part; if this one is preferred, #368 can be closed.

Test plan

  • cargo test --all-features
  • cargo clippy --bin rbw --lib -- -D warnings
  • cargo fmt --check
  • Edit a keyed item against a real vault and verify it remains decryptable
  • Trigger a stale cipher write and verify it does not overwrite a concurrent edit
  • Verify post-write sync failure exits 2 without suggesting a retry

CI's lint job is red here for reasons that predate this branch: stable clippy 1.98 fails --all-targets --all-features -- -Dwarnings on main itself, at eight sites in code this PR does not touch. #373 fixes those. That job then fails again at cargo deny check on RustSec advisories in Cargo.lock (bytes, quick-xml, rand, rustls-webpki, yanked spin), which is also independent of this branch and needs a dependency refresh. cargo clippy --bin rbw --lib, cargo fmt --check, and cargo test --all-features are clean on this branch.

Related: #364

@m11y
m11y force-pushed the fix/cipher-write-safety branch from bed1c18 to 341bf9f Compare September 3, 2026 06:48
@m11y

m11y commented Sep 4, 2026

Copy link
Copy Markdown
Author

Hey @doy, hope you're doing well!

I submitted this PR a while ago and just wanted to gently check in to see if you've had a chance to take a look. No rush at all - I know everyone has a lot on their plate!

There are also 3 other open PRs that might be worth reviewing when you have time:

Happy to make any changes or clarifications if needed. Thanks so much for your time and for maintaining this project!

@m11y m11y mentioned this pull request Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant