Skip to content

Add rbw edit --field to update custom fields - #370

Closed
m11y wants to merge 14 commits into
doy:mainfrom
m11y:feat/edit-custom-field
Closed

m11y wants to merge 14 commits into
doy:mainfrom
m11y:feat/edit-custom-field

Conversation

@m11y

@m11y m11y commented Sep 2, 2026 •

Copy link
Copy Markdown

Summary

  • rbw edit --field=<name> updates one existing custom field on login, secure note, card, and identity entries.
  • The vault is synced before the name/URI selector is resolved, then again after a successful PUT so the agent rebuilds its master-password-reprompt set. Agent sync writes the vault before replacing that in-memory set (same lock). Any local failure after the server write (token save, cache refresh, reload) is exit 2 (rbw sync, do not retry the write); a write that never landed stays exit 1.
  • Piped stdin is the new value (trailing newlines stripped) and is not written to disk on failure (the error says to re-run the producing command). An interactive editor uses per-edit random help markers; leftover help body is refused; failed editor edits stash plaintext under the rbw runtime unsaved-edits/ directory (0700 / files 0600 O_EXCL) for rbw add and rbw edit. rbw purge removes that directory.
  • Empty values are refused. SSH key entries and linked fields are refused. Boolean fields must be true/false.
  • Encrypt with the entry item key when present and send key on PUT (rbw edit corrupts entries that have an individual encryption key #364 / edit: encrypt with item key when entry has one #368). Cipher PUT also sends reprompt, favorite, archivedDate, and lastKnownRevisionDate.
  • HTTP 400 bodies containing "out of date" (and 409) map to CipherRevisionConflict. Field edits rebase unless the target field's plaintext changed (official clients re-encrypt every custom field on save, so ciphertext inequality is not a same-field edit).
  • rbw get --list-custom-fields lists editable custom field names without decrypting hidden values.
  • Library: db::Entry gained favorite / archived_date / revision_date (JSON-compatible via serde(default); Rust struct-literal break). actions::edit / Client::edit are removed — they omitted item key and reset favorite/archive. Use edit_with_meta. rbw::edit::edit stays as the source-agnostic wrapper around edit_from.

Depends on: #368
Related: #364

Test plan

  • cargo test --lib --bin rbw
  • cargo clippy --bin rbw --lib -- -D warnings
  • Pipe a new value into rbw edit --field=<existing> <entry> and confirm rbw get --field=<existing> returns it
  • Failed pipe edit does not create a file under unsaved-edits/ and mentions re-running the producer
  • Failed editor edit creates a 0600 file; rbw purge removes the directory
  • Concurrent change to another field (including via the official client, which re-encrypts all fields) rebases; concurrent change to the same field's plaintext conflicts
  • Keyed item still decrypts after --field edit
  • rbw edit --field on an SSH key / linked field errors

m11y added 2 commits September 2, 2026 16:34
Password/notes editing is unchanged. --field updates one existing custom
field in place: piped stdin is stored as the value, otherwise the editor
opens. This is the missing write path for Secure Note items that keep
secrets in hidden custom fields.
Encrypt custom-field (and password/notes) values with the entry's
individual item key when present, and send that key on the cipher PUT.
Without this, keyed items become mixed-key and fail with invalid mac
(doy#364; same plumbing as doy#368).

Refuse SSH key entries (Client::edit is unreachable for them) and linked
fields. Require boolean fields to be true/false. Interactive edits strip
only the appended help suffix so user lines starting with # are kept.
@m11y

m11y commented Sep 2, 2026

Copy link
Copy Markdown
Author

Addressed the review:

Return pipe vs editor from rbw::edit::edit_from so field edits do not
re-check is_terminal. List custom fields separately for fish completion
of edit --field, and complete the rest of the edit flags. Document
refused empty values and that trailing newlines are stripped. Wrap the
README usage paragraph to 80 columns and match the changelog heading
style.
@m11y

m11y commented Sep 2, 2026

Copy link
Copy Markdown
Author

Follow-up for the remaining review notes (71ee8b4; item key / SSH / # lines were already in dc2facd):

  1. SSH key: --field still returns before the password/notes type match, but edit_custom_field refuses SshKey before Client::edit.
  2. Item key: encrypt takes entry_key and the cipher PUT sends key (dc2facd). Same plumbing as edit: encrypt with item key when entry has one #368.
  3. # lines: editor output only strips the appended help suffix.
  4. Pipe vs editor: rbw::edit::edit_from returns Source::{Pipe,Editor} from a single stdin_is_pipe() check; field edits no longer re-test the tty.
  5. Fish: edit --field completes via rbw get --list-custom-fields (custom names only). Edit also completes -i / -h / --folder.
  6. Docs: README usage wrapped to 80 columns; changelog uses ## Added and cites (Add rbw edit --field to update custom fields #370); help text says trailing newlines are stripped and empty values are refused.
  7. Empty values: still refused on purpose (no --allow-empty); documented in --field help and the editor stub.

Bump PROTOCOL_REVISION independently of Cargo.toml so a leftover 1.15.0
agent is restarted instead of ignoring Encrypt.entry_key (serde drops
unknown fields). Cipher PUT now sends reprompt, favorite, and
archivedDate from the last sync, matching official ToCipherDetails.

Help text is removed by content (CRLF, text after the stub, leftover
stub). Boolean edits re-open the editor; list-custom-fields decrypts
names only and skips linked fields. Card/identity --field edits are
intentional (the API has PUT bodies; SSH does not).
@m11y

m11y commented Sep 2, 2026

Copy link
Copy Markdown
Author

4b8b031 addresses the remaining findings. Rationale lives next to the code (PROTOCOL_REVISION, PUT metadata, help-block removal, names-only listing).

Protocol: independent PROTOCOL_REVISION (not a Cargo.toml bump). serde still ignores unknown fields; the version check is what fail-closes leftover agents.

PUT: reprompt / favorite / archivedDate are now loaded from sync and sent on every cipher PUT, because official ToCipherDetails overwrites those with request defaults.

Did not split the PR: --field without the item-key encrypt path is data-corrupting, and #368 does not bump protocol revision. Happy to split after this is green if upstream prefers two merges.

Bitwarden has no field PATCH, so edits send a full cipher. Sync first
and send lastKnownRevisionDate so a stale cache cannot overwrite another
client's password/notes or unfavorite on the first post-upgrade edit.

Replace prose-suffix help stripping with start/end markers so user
newlines are not rewritten. Cap boolean editor retries. Do not list
custom fields on SSH key entries.
@m11y

m11y commented Sep 2, 2026

Copy link
Copy Markdown
Author

10c5f5b: Bitwarden has no field PATCH, so the real contract is "full PUT of a fresh snapshot". Edits now sync before the editor, then PUT with lastKnownRevisionDate from that snapshot (409 → CipherRevisionConflict). That also closes the post-upgrade favorite/archive default hole without treating serde(default) as server truth.

Help stub is now a start/end marker block sliced out of the original buffer (no newline rewrite). Boolean editor retries are capped. SSH key entries are not listed as editable.

Still not splitting the PR: --field without the item-key + PUT-metadata work is unsafe. Happy to split after this is green if upstream wants two merges.

m11y added 9 commits September 2, 2026 20:05
Find the name/URI after a fresh sync so a renamed item is not edited by
stale ID. Help fences use a per-edit nonce so a user value that already
contains a marker is not truncated. Official Bitwarden and Vaultwarden
signal revision conflicts as HTTP 400 "out of date"; map that (and 409)
to CipherRevisionConflict. Field edits retry once on conflict by
re-applying only the field. PUT patches the local snapshot instead of
a second full sync.
The agent rebuilds its master-password-reprompt set only on sync, so a
disk-only patch let `rbw get` skip the prompt for a newly written hidden
value. Sync after PUT.

Field conflict retries now compare the target field ciphertext and
re-validate booleans; a concurrent change to the same field stays a
conflict. Password/notes conflicts stash the editor buffer at 0600.
`--field` decrypts names plus the target value only. Help-body residue
is derived from the same strings field_help emits.
Same-field conflicts and other failed PUTs keep the typed value in the
rbw runtime unsaved-edits dir (0600) and tell the user to delete it.
A successful PUT followed by a failed cache refresh is a warning, not
an edit failure.

Restore the 1.15 `edit` signatures as deprecated wrappers and put the
new metadata on `edit_with_meta`, so lib callers still compile.
Agent sync now writes the vault before replacing the in-memory
master-password-reprompt set, under the same lock. A refresh failure
after a successful PUT is an error.

Piped stdin is not written to unsaved-edits/; only interactive editor
content is. That directory is created 0700 with the existing helper,
files are O_EXCL 0600, and rbw purge removes it.

Drop the 1.15 edit wrappers: they omitted item key and reset
favorite/archive. Callers use edit_with_meta. Entry's new fields stay
and are documented as a source break.
Official Bitwarden clients encrypt CipherView from scratch on every
save, so a notes-only edit rotates every custom field's ciphertext.
Treat the target field as unchanged when its decrypted value still
matches; skip the decrypt when ciphertext is identical.
Scripts treating any non-zero as "the vault was not updated" would
retry or roll back a PUT that already landed. Exit 2, and say not to
retry the write. Do not stash or tell a pipe caller to regenerate.

Replace the post-editor IIFEs with named helpers. Document that add
and edit share unsaved-edits/, and that edit() remains the
source-agnostic wrapper around edit_from.
PUT/add already succeeded before save_db, sync, or load_db. Wrapping
only the sync left token persistence and cache reload as exit 1, so
scripts would retry and duplicate items. Rename add_after_editor's
ciphertext parameters so they are not mixed with plaintext uris/folder.
The helper now takes that optional refreshed access token and does
token save, sync, and db reload itself. Call sites must have the
add/edit_with_meta result in hand; a free-floating closure can no
longer wrap an arbitrary pre-write failure as exit 2.
@m11y

m11y commented Sep 3, 2026

Copy link
Copy Markdown
Author

Split into #371 for cipher write safety and retry semantics, and #372 for the edit --field feature.

@m11y m11y closed this Sep 3, 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