diff --git a/CHANGELOG.md b/CHANGELOG.md index 729f5b58..bad9447e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,54 @@ # Changelog +## Unreleased + +## Added + +* `rbw edit --field=` updates an existing custom field on login, + secure note, card, and identity entries. Piped stdin is stored as the + new value; an interactive terminal opens the editor. SSH key entries + and linked fields are refused; boolean fields must be `true` or + `false`. Empty values are refused. The editor help block is removed by + content, not only as a suffix (#370). +* `rbw get --list-custom-fields` lists editable custom field names + without decrypting hidden values (#370). + +## Changed + +* `db::Entry` now includes `favorite`, `archived_date`, and + `revision_date` so a field-only PUT can round-trip those values. + This is a Rust source break for struct literals and exhaustive + matches; JSON cache load stays compatible via `serde(default)`. +* `actions::edit` and `Client::edit` are removed. They omitted item + `key` and wrote `favorite=false` / `archivedDate=null`, which could + mix-key-corrupt keyed items and unfavorite/unarchive others. Use + `edit_with_meta`. + +## Fixed + +* `rbw edit` encrypts with the entry's individual item key when present + and preserves `key` on the cipher PUT, so keyed items are not corrupted + (#364, #368). +* Cipher PUT now sends `reprompt`, `favorite`, `archivedDate`, and + `lastKnownRevisionDate`. `rbw edit` syncs before resolving the + name/URI selector, and again after a successful PUT so the agent + rebuilds its master-password-reprompt set. The agent writes the + vault to disk before replacing that in-memory set; a refresh + failure after a successful PUT is an error (`rbw sync`), not a + silent success. After the server write, any local follow-up failure + (token save, cache refresh, reload) makes `rbw add`, `rbw edit`, and + `rbw generate` exit 2 (not 1) and tells you not to retry the write. + Stale-cipher + responses (HTTP 400 "out of date" or 409) map to a retryable + conflict error. Field conflict retries compare the target field's + plaintext: official clients re-encrypt every custom field on save, + so a notes-only edit changes ciphertext without changing the value. +* A rejected interactive editor (`rbw add`, `rbw edit`, `rbw edit + --field`) keeps the plaintext in `$RBW runtime/unsaved-edits/` + (directory mode 0700, files 0600, `O_EXCL`) until you delete it. + Piped stdin is not written to disk. `rbw purge` removes that + directory. + ## [1.15.0] - 2025-12-31 ## Added diff --git a/README.md b/README.md index eb9074b8..3f686be8 100644 --- a/README.md +++ b/README.md @@ -123,12 +123,26 @@ out by running `rbw purge`, and you can explicitly lock the database by running `rbw help` can be used to get more information about the available functionality. -Run `rbw get ` to get your passwords. If you also want to get the username -or the note associated, you can use the flag `--full`. You can also use the flag -`--field={field}` to get whatever default or custom field you want. The `--raw` -flag will show the output as JSON. In addition to matching against the name, -you can pass a UUID as the name to search for the entry with that id, or a -URL to search for an entry with a matching website entry. +Run `rbw get ` to get your passwords. If you also want to get the +username or the note associated, you can use the flag `--full`. You can +also use the flag `--field={field}` to get whatever default or custom +field you want. The `--raw` flag will show the output as JSON. Run +`rbw edit --field={field} ` to change an existing custom field on +login, secure note, card, and identity entries. `rbw edit` syncs the +vault before opening the editor, so it needs network access. Piped +stdin is stored as the new value (trailing newlines are stripped), or an +editor is opened when stdin is a terminal. Empty values are refused. +SSH key entries and linked fields cannot be edited this way. +If an interactive editor save fails before the server write, the typed +buffer is kept under the rbw runtime `unsaved-edits/` directory; delete +it after recovering, or run `rbw purge`. Piped stdin is never written +there. If the server write succeeded but the local cache could not be +refreshed, the command exits 2 — run `rbw sync` and do not retry the +write. +`rbw get --list-custom-fields` lists the editable custom field names for +an entry. In addition to matching against the name, you can pass a UUID +as the name to search for the entry with that id, or a URL to search for +an entry with a matching website entry. *Note to users of the official Bitwarden server (at bitwarden.com)*: The official server has a tendency to detect command line traffic as bot traffic diff --git a/src/actions.rs b/src/actions.rs index 79d304d4..8446a25a 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -193,7 +193,7 @@ fn add_once( Ok(()) } -pub fn edit( +pub fn edit_with_meta( access_token: &str, refresh_token: &str, id: &str, @@ -204,6 +204,7 @@ pub fn edit( notes: Option<&str>, folder_uuid: Option<&str>, history: &[crate::db::HistoryEntry], + meta: &crate::db::CipherWriteMeta<'_>, ) -> Result<(Option, ())> { with_exchange_refresh_token(access_token, refresh_token, |access_token| { edit_once( @@ -216,6 +217,7 @@ pub fn edit( notes, folder_uuid, history, + meta, ) }) } @@ -230,9 +232,10 @@ fn edit_once( notes: Option<&str>, folder_uuid: Option<&str>, history: &[crate::db::HistoryEntry], + meta: &crate::db::CipherWriteMeta<'_>, ) -> Result<()> { let (client, _) = api_client()?; - client.edit( + client.edit_with_meta( access_token, id, org_id, @@ -242,6 +245,7 @@ fn edit_once( notes, folder_uuid, history, + meta, )?; Ok(()) } diff --git a/src/api.rs b/src/api.rs index a817fb26..e019562b 100644 --- a/src/api.rs +++ b/src/api.rs @@ -428,6 +428,12 @@ struct SyncResCipher { key: Option, #[serde(rename = "Reprompt", alias = "reprompt")] reprompt: CipherRepromptType, + #[serde(default, rename = "Favorite", alias = "favorite")] + favorite: bool, + #[serde(default, rename = "ArchivedDate", alias = "archivedDate")] + archived_date: Option, + #[serde(default, rename = "RevisionDate", alias = "revisionDate")] + revision_date: Option, } impl SyncResCipher { @@ -551,6 +557,9 @@ impl SyncResCipher { history, key: self.key.clone(), master_password_reprompt: self.reprompt, + favorite: self.favorite, + archived_date: self.archived_date.clone(), + revision_date: self.revision_date.clone(), }) } } @@ -780,6 +789,21 @@ struct CiphersPutReq { secure_note: Option, #[serde(rename = "passwordHistory")] password_history: Vec, + key: Option, + // Non-optional on the official server; omitting them resets the + // live cipher (see CipherRequestModel.ToCipherDetails). + reprompt: CipherRepromptType, + favorite: bool, + #[serde(rename = "archivedDate")] + archived_date: Option, + // Official server only conflict-checks when this is present. An + // omitted value allows a stale full PUT to overwrite a password or + // notes another client just saved. + #[serde( + rename = "lastKnownRevisionDate", + skip_serializing_if = "Option::is_none" + )] + last_known_revision_date: Option, } #[derive(serde::Serialize, Debug)] @@ -1317,7 +1341,7 @@ impl Client { } } - pub fn edit( + pub fn edit_with_meta( &self, access_token: &str, id: &str, @@ -1328,6 +1352,7 @@ impl Client { notes: Option<&str>, folder_uuid: Option<&str>, history: &[crate::db::HistoryEntry], + meta: &crate::db::CipherWriteMeta<'_>, ) -> Result<()> { let mut req = CiphersPutReq { ty: match data { @@ -1361,6 +1386,15 @@ impl Client { password: entry.password.clone(), }) .collect(), + key: meta.key.map(std::string::ToString::to_string), + reprompt: meta.reprompt, + favorite: meta.favorite, + archived_date: meta + .archived_date + .map(std::string::ToString::to_string), + last_known_revision_date: meta + .last_known_revision_date + .map(std::string::ToString::to_string), }; match data { crate::db::EntryData::Login { @@ -1456,15 +1490,12 @@ impl Client { .json(&req) .send() .map_err(|source| Error::Reqwest { source })?; - match res.status() { - reqwest::StatusCode::OK => Ok(()), - reqwest::StatusCode::UNAUTHORIZED => { - Err(Error::RequestUnauthorized) - } - _ => Err(Error::RequestFailed { - status: res.status().as_u16(), - }), + let status = res.status(); + if status == reqwest::StatusCode::OK { + return Ok(()); } + let body = res.text().unwrap_or_default(); + Err(cipher_write_error(status, &body)) } pub fn remove(&self, access_token: &str, id: &str) -> Result<()> { @@ -1766,3 +1797,59 @@ fn classify_login_error(error_res: &ConnectErrorRes, code: u16) -> Error { log::warn!("unexpected error received during login: {error_res:?}"); Error::RequestFailed { status: code } } + +fn cipher_write_error(status: reqwest::StatusCode, body: &str) -> Error { + // Official Bitwarden throws BadRequestException ("out of date") → 400. + // Vaultwarden uses err!("... out of date.") → also 400. Some forks + // might still use 409. Matching the body is what surfaces + // CipherRevisionConflict; status 409 is kept as a belt. + if status == reqwest::StatusCode::CONFLICT + || is_stale_cipher_message(body) + { + Error::CipherRevisionConflict + } else if status == reqwest::StatusCode::UNAUTHORIZED { + Error::RequestUnauthorized + } else { + Error::RequestFailed { + status: status.as_u16(), + } + } +} + +fn is_stale_cipher_message(body: &str) -> bool { + body.to_ascii_lowercase().contains("out of date") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn revision_conflict_from_official_and_vaultwarden_bodies() { + assert!(matches!( + cipher_write_error( + reqwest::StatusCode::BAD_REQUEST, + "The item cannot be saved because it is out of date. Please reload and try again.", + ), + Error::CipherRevisionConflict + )); + assert!(matches!( + cipher_write_error( + reqwest::StatusCode::BAD_REQUEST, + r#"{"message":"The client copy of this cipher is out of date."}"#, + ), + Error::CipherRevisionConflict + )); + assert!(matches!( + cipher_write_error(reqwest::StatusCode::CONFLICT, ""), + Error::CipherRevisionConflict + )); + assert!(matches!( + cipher_write_error( + reqwest::StatusCode::BAD_REQUEST, + "Name is required", + ), + Error::RequestFailed { status: 400 } + )); + } +} diff --git a/src/bin/rbw-agent/actions.rs b/src/bin/rbw-agent/actions.rs index 9ddd2ad9..724c01fc 100644 --- a/src/bin/rbw-agent/actions.rs +++ b/src/bin/rbw-agent/actions.rs @@ -527,7 +527,6 @@ pub async fn sync( ) = rbw::actions::sync(&access_token, &refresh_token) .await .context("failed to sync database from server")?; - state.lock().await.set_master_password_reprompt(&entries); if let Some(access_token) = access_token { db.access_token = Some(access_token); } @@ -535,7 +534,19 @@ pub async fn sync( db.protected_private_key = Some(protected_private_key); db.protected_org_keys = protected_org_keys; db.entries = entries; - save_db(&db).await?; + // Disk first, then the in-memory ciphertext set, under the same + // lock. The reverse order (or updating memory before a failed + // save) leaves the set matching the server while the on-disk + // cache still has the previous vault: decrypting an old hidden + // value would miss the set and skip master-password reprompt. + // Holding the lock across the save closes the window where a + // concurrent decrypt would see new disk bytes against the old + // set (the original skip-prompt bug this sync exists to fix). + { + let mut state = state.lock().await; + save_db(&db).await?; + state.set_master_password_reprompt(&db.entries); + } if let Err(e) = subscribe_to_notifications(state.clone()).await { eprintln!("failed to subscribe to notifications: {e}"); @@ -692,6 +703,7 @@ pub async fn encrypt( sock: &mut crate::sock::Sock, state: std::sync::Arc>, plaintext: &str, + entry_key: Option<&str>, org_id: Option<&str>, ) -> anyhow::Result<()> { let state = state.lock().await; @@ -700,8 +712,20 @@ pub async fn encrypt( "failed to find encryption keys in in-memory state" )); }; + let entry_keys = if let Some(entry_key) = entry_key { + let key_cipherstring = + rbw::cipherstring::CipherString::new(entry_key) + .context("failed to parse individual item encryption key")?; + Some(rbw::locked::Keys::new( + key_cipherstring.decrypt_locked_symmetric(keys).context( + "failed to decrypt individual item encryption key", + )?, + )) + } else { + None + }; let cipherstring = rbw::cipherstring::CipherString::encrypt_symmetric( - keys, + entry_keys.as_ref().unwrap_or(keys), plaintext.as_bytes(), ) .context("failed to encrypt plaintext secret")?; diff --git a/src/bin/rbw-agent/agent.rs b/src/bin/rbw-agent/agent.rs index 1691ed51..aac7de5f 100644 --- a/src/bin/rbw-agent/agent.rs +++ b/src/bin/rbw-agent/agent.rs @@ -162,11 +162,16 @@ async fn handle_request( .await?; true } - rbw::protocol::Action::Encrypt { plaintext, org_id } => { + rbw::protocol::Action::Encrypt { + plaintext, + entry_key, + org_id, + } => { crate::actions::encrypt( sock, state.clone(), plaintext, + entry_key.as_deref(), org_id.as_deref(), ) .await?; diff --git a/src/bin/rbw/actions.rs b/src/bin/rbw/actions.rs index a0a34e8c..1edd3964 100644 --- a/src/bin/rbw/actions.rs +++ b/src/bin/rbw/actions.rs @@ -107,6 +107,7 @@ pub fn decrypt( pub fn encrypt( plaintext: &str, + entry_key: Option<&str>, org_id: Option<&str>, ) -> anyhow::Result { let mut sock = connect()?; @@ -114,6 +115,7 @@ pub fn encrypt( get_environment(), rbw::protocol::Action::Encrypt { plaintext: plaintext.to_string(), + entry_key: entry_key.map(std::string::ToString::to_string), org_id: org_id.map(std::string::ToString::to_string), }, ))?; diff --git a/src/bin/rbw/commands.rs b/src/bin/rbw/commands.rs index bddf0efe..8bce3daf 100644 --- a/src/bin/rbw/commands.rs +++ b/src/bin/rbw/commands.rs @@ -1227,6 +1227,37 @@ const HELP_NOTES: &str = r" # Lines with leading # will be ignored. "; +fn unique_help_markers(current: &str) -> (String, String) { + use rand::distr::SampleString as _; + loop { + let nonce = + rand::distr::Alphanumeric.sample_string(&mut rand::rng(), 16); + let start = format!("# --- rbw field help start {nonce} ---"); + let end = format!("# --- rbw field help end {nonce} ---"); + if !current.contains(&start) && !current.contains(&end) { + return (start, end); + } + } +} + +const HELP_BODY: &str = + "# The content of this file will be stored as the custom field value.\n\ + # Empty values are refused."; +const HELP_BODY_BOOLEAN: &str = + "# Enter true or false (nothing else).\n# Empty values are refused."; + +fn help_body(boolean: bool) -> &'static str { + if boolean { + HELP_BODY_BOOLEAN + } else { + HELP_BODY + } +} + +fn field_help(boolean: bool, start: &str, end: &str) -> String { + format!("\n{start}\n{}\n{end}\n", help_body(boolean)) +} + pub fn config_show() -> anyhow::Result<()> { let config = rbw::config::Config::load()?; serde_json::to_writer_pretty(std::io::stdout(), &config) @@ -1394,6 +1425,7 @@ pub fn get( clipboard: bool, ignore_case: bool, list_fields: bool, + list_custom_fields: bool, ) -> anyhow::Result<()> { unlock()?; @@ -1405,6 +1437,15 @@ pub fn get( needle ); + if list_custom_fields { + // Names only: decrypting hidden *values* would master-password + // reprompt, which is hostile for fish `edit --field` completion. + let entry = find_db_entry(&db, needle, user, folder, ignore_case) + .with_context(|| format!("couldn't find entry for '{desc}'"))?; + print_editable_custom_field_names(&entry)?; + return Ok(()); + } + let (_, decrypted) = find_entry(&db, needle, user, folder, ignore_case) .with_context(|| format!("couldn't find entry for '{desc}'"))?; @@ -1566,29 +1607,51 @@ pub fn add( let mut db = load_db()?; // unwrap is safe here because the call to unlock above is guaranteed to // populate these or error - let mut access_token = db.access_token.as_ref().unwrap().clone(); - let refresh_token = db.refresh_token.as_ref().unwrap(); - let name = crate::actions::encrypt(name, None)?; + let encrypted_name = crate::actions::encrypt(name, None, None)?; - let username = username - .map(|username| crate::actions::encrypt(username, None)) + let encrypted_username = username + .map(|username| crate::actions::encrypt(username, None, None)) .transpose()?; - let contents = rbw::edit::edit("", HELP_PW)?; + let (contents, source) = rbw::edit::edit_from("", HELP_PW)?; + after_editor( + source, + &contents, + add_after_editor( + &mut db, + &encrypted_name, + encrypted_username, + uris, + folder, + &contents, + ), + ) +} - let (password, notes) = parse_editor(&contents); +fn add_after_editor( + db: &mut rbw::db::Db, + encrypted_name: &str, + encrypted_username: Option, + uris: &[(String, Option)], + folder: Option<&str>, + contents: &str, +) -> anyhow::Result<()> { + let mut access_token = db.access_token.as_ref().unwrap().clone(); + let refresh_token = db.refresh_token.as_ref().unwrap().clone(); + + let (password, notes) = parse_editor(contents); let password = password - .map(|password| crate::actions::encrypt(&password, None)) + .map(|password| crate::actions::encrypt(&password, None, None)) .transpose()?; let notes = notes - .map(|notes| crate::actions::encrypt(¬es, None)) + .map(|notes| crate::actions::encrypt(¬es, None, None)) .transpose()?; let uris: Vec<_> = uris .iter() .map(|uri| { Ok(rbw::db::Uri { - uri: crate::actions::encrypt(&uri.0, None)?, + uri: crate::actions::encrypt(&uri.0, None, None)?, match_type: uri.1, }) }) @@ -1597,11 +1660,11 @@ pub fn add( let mut folder_id = None; if let Some(folder_name) = folder { let (new_access_token, folders) = - rbw::actions::list_folders(&access_token, refresh_token)?; + rbw::actions::list_folders(&access_token, &refresh_token)?; if let Some(new_access_token) = new_access_token { access_token.clone_from(&new_access_token); db.access_token = Some(new_access_token); - save_db(&db)?; + save_db(db)?; } let folders: Vec<(String, String)> = folders @@ -1620,38 +1683,32 @@ pub fn add( if folder_id.is_none() { let (new_access_token, id) = rbw::actions::create_folder( &access_token, - refresh_token, - &crate::actions::encrypt(folder_name, None)?, + &refresh_token, + &crate::actions::encrypt(folder_name, None, None)?, )?; if let Some(new_access_token) = new_access_token { access_token.clone_from(&new_access_token); db.access_token = Some(new_access_token); - save_db(&db)?; + save_db(db)?; } folder_id = Some(id); } } - if let (Some(access_token), ()) = rbw::actions::add( + let (new_token, ()) = rbw::actions::add( &access_token, - refresh_token, - &name, + &refresh_token, + encrypted_name, &rbw::db::EntryData::Login { - username, + username: encrypted_username, password, uris, totp: None, }, notes.as_deref(), folder_id.as_deref(), - )? { - db.access_token = Some(access_token); - save_db(&db)?; - } - - crate::actions::sync()?; - - Ok(()) + )?; + after_server_write(db, new_token) } pub fn generate( @@ -1674,16 +1731,16 @@ pub fn generate( let mut access_token = db.access_token.as_ref().unwrap().clone(); let refresh_token = db.refresh_token.as_ref().unwrap(); - let name = crate::actions::encrypt(name, None)?; + let name = crate::actions::encrypt(name, None, None)?; let username = username - .map(|username| crate::actions::encrypt(username, None)) + .map(|username| crate::actions::encrypt(username, None, None)) .transpose()?; - let password = crate::actions::encrypt(&password, None)?; + let password = crate::actions::encrypt(&password, None, None)?; let uris: Vec<_> = uris .iter() .map(|uri| { Ok(rbw::db::Uri { - uri: crate::actions::encrypt(&uri.0, None)?, + uri: crate::actions::encrypt(&uri.0, None, None)?, match_type: uri.1, }) }) @@ -1716,7 +1773,7 @@ pub fn generate( let (new_access_token, id) = rbw::actions::create_folder( &access_token, refresh_token, - &crate::actions::encrypt(folder_name, None)?, + &crate::actions::encrypt(folder_name, None, None)?, )?; if let Some(new_access_token) = new_access_token { access_token.clone_from(&new_access_token); @@ -1727,7 +1784,7 @@ pub fn generate( } } - if let (Some(access_token), ()) = rbw::actions::add( + let (new_token, ()) = rbw::actions::add( &access_token, refresh_token, &name, @@ -1739,12 +1796,8 @@ pub fn generate( }, None, folder_id.as_deref(), - )? { - db.access_token = Some(access_token); - save_db(&db)?; - } - - crate::actions::sync()?; + )?; + after_server_write(&mut db, new_token)?; } Ok(()) @@ -1755,24 +1808,35 @@ pub fn edit( username: Option<&str>, folder: Option<&str>, ignore_case: bool, + field: Option<&str>, ) -> anyhow::Result<()> { unlock()?; - let mut db = load_db()?; - let access_token = db.access_token.as_ref().unwrap(); - let refresh_token = db.refresh_token.as_ref().unwrap(); - let desc = format!( "{}{}", username.map_or_else(String::new, |s| format!("{s}@")), name ); - let (entry, decrypted) = - find_entry(&db, name, username, folder, ignore_case) - .with_context(|| format!("couldn't find entry for '{desc}'"))?; + // Resolve the selector against a fresh vault. Finding by name/URI in + // the old cache and then refreshing *that ID* would edit the wrong + // cipher if the name had been moved to another item. + crate::actions::sync()?; + let mut db = load_db()?; + let entry = find_db_entry(&db, name, username, folder, ignore_case) + .with_context(|| format!("couldn't find entry for '{desc}'"))?; + + if let Some(field_name) = field { + // Decrypt only the target field. Full decrypt_cipher would pinentry + // once per hidden value, and encrypt_field_on_entry used to do it + // a second time. + edit_custom_field(&mut db, &entry, field_name)?; + return Ok(()); + } + + let decrypted = decrypt_cipher(&entry)?; - let (data, fields, notes, history) = match &decrypted.data { + match &decrypted.data { DecryptedData::Login { password, .. } => { let mut contents = format!("{}\n", password.as_deref().unwrap_or("")); @@ -1780,101 +1844,605 @@ pub fn edit( write!(contents, "\n{notes}\n").unwrap(); } - let contents = rbw::edit::edit(&contents, HELP_PW)?; - - let (password, notes) = parse_editor(&contents); - let password = password - .map(|password| { - crate::actions::encrypt( - &password, - entry.org_id.as_deref(), - ) - }) - .transpose()?; - let notes = notes - .map(|notes| { - crate::actions::encrypt(¬es, entry.org_id.as_deref()) - }) - .transpose()?; - let mut history = entry.history.clone(); - let rbw::db::EntryData::Login { - username: entry_username, - password: entry_password, - uris: entry_uris, - totp: entry_totp, - } = &entry.data - else { - unreachable!(); - }; - - if let Some(prev_password) = entry_password.clone() { - let new_history_entry = rbw::db::HistoryEntry { - last_used_date: format!( - "{}", - humantime::format_rfc3339( - std::time::SystemTime::now() - ) - ), - password: prev_password, - }; - history.insert(0, new_history_entry); - } - - let data = rbw::db::EntryData::Login { - username: entry_username.clone(), - password, - uris: entry_uris.clone(), - totp: entry_totp.clone(), - }; - (data, entry.fields, notes, history) + let (contents, source) = + rbw::edit::edit_from(&contents, HELP_PW)?; + after_editor( + source, + &contents, + put_login_edit(&mut db, &entry, &contents), + ) } DecryptedData::SecureNote => { - let data = rbw::db::EntryData::SecureNote {}; - let editor_content = decrypted.notes.map_or_else( || "\n".to_string(), |notes| format!("{notes}\n"), ); - let contents = rbw::edit::edit(&editor_content, HELP_NOTES)?; + let (contents, source) = + rbw::edit::edit_from(&editor_content, HELP_NOTES)?; + after_editor( + source, + &contents, + put_note_edit(&mut db, &entry, &contents), + ) + } + _ => Err(anyhow::anyhow!( + "modifications are only supported for login and note entries" + )), + } +} - // prepend blank line to be parsed as pw by `parse_editor` - let (_, notes) = parse_editor(&format!("\n{contents}\n")); +fn is_revision_conflict(err: &anyhow::Error) -> bool { + matches!( + err.downcast_ref::(), + Some(rbw::error::Error::CipherRevisionConflict) + ) +} - let notes = notes - .map(|notes| { - crate::actions::encrypt(¬es, entry.org_id.as_deref()) - }) - .transpose()?; +fn put_login_edit( + db: &mut rbw::db::Db, + entry: &rbw::db::Entry, + contents: &str, +) -> anyhow::Result<()> { + let (password, notes) = parse_editor(contents); + let password = password + .map(|password| { + crate::actions::encrypt( + &password, + entry.key.as_deref(), + entry.org_id.as_deref(), + ) + }) + .transpose()?; + let notes = notes + .map(|notes| { + crate::actions::encrypt( + ¬es, + entry.key.as_deref(), + entry.org_id.as_deref(), + ) + }) + .transpose()?; + let mut history = entry.history.clone(); + let rbw::db::EntryData::Login { + username: entry_username, + password: entry_password, + uris: entry_uris, + totp: entry_totp, + } = &entry.data + else { + unreachable!(); + }; - (data, entry.fields, notes, entry.history) - } - _ => { - return Err(anyhow::anyhow!( - "modifications are only supported for login and note entries" - )); - } + if let Some(prev_password) = entry_password.clone() { + let new_history_entry = rbw::db::HistoryEntry { + last_used_date: format!( + "{}", + humantime::format_rfc3339(std::time::SystemTime::now()) + ), + password: prev_password, + }; + history.insert(0, new_history_entry); + } + + let data = rbw::db::EntryData::Login { + username: entry_username.clone(), + password, + uris: entry_uris.clone(), + totp: entry_totp.clone(), }; + put_cipher(db, entry, &data, &entry.fields, notes.as_deref(), &history) +} - if let (Some(access_token), ()) = rbw::actions::edit( +fn put_note_edit( + db: &mut rbw::db::Db, + entry: &rbw::db::Entry, + contents: &str, +) -> anyhow::Result<()> { + // prepend blank line to be parsed as pw by `parse_editor` + let (_, notes) = parse_editor(&format!("\n{contents}\n")); + let notes = notes + .map(|notes| { + crate::actions::encrypt( + ¬es, + entry.key.as_deref(), + entry.org_id.as_deref(), + ) + }) + .transpose()?; + put_cipher( + db, + entry, + &rbw::db::EntryData::SecureNote {}, + &entry.fields, + notes.as_deref(), + &entry.history, + ) +} + +fn persist_unsaved_after_edit(source: rbw::edit::Source) -> bool { + // Piped stdin never touched disk; writing unsaved-edits/ would + // change that. Interactive editor buffers live in a tempfile that + // disappears when this process returns, so those are worth keeping. + source == rbw::edit::Source::Editor +} + +fn persist_unsaved_edit(buf: &str) -> anyhow::Result { + persist_unsaved_edit_in(&rbw::dirs::unsaved_edit_dir(), buf) +} + +fn persist_unsaved_edit_in( + dir: &std::path::Path, + buf: &str, +) -> anyhow::Result { + use std::io::Write as _; + rbw::dirs::create_dir_all_with_permissions(dir, 0o700) + .with_context(|| format!("failed to create {}", dir.display()))?; + let path = dir.join(format!( + "unsaved-{}.txt", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_nanos()) + )); + let mut opts = std::fs::OpenOptions::new(); + opts.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + opts.mode(0o600); + } + let mut file = opts + .open(&path) + .with_context(|| format!("failed to create {}", path.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + file.set_permissions(std::fs::Permissions::from_mode(0o600)) + .context("failed to chmod unsaved-edit file")?; + } + file.write_all(buf.as_bytes()) + .context("failed to write unsaved-edit file")?; + Ok(path) +} + +fn err_with_unsaved(err: anyhow::Error, plaintext: &str) -> anyhow::Error { + match persist_unsaved_edit(plaintext) { + Ok(path) => err.context(format!( + "plaintext you just typed is in {} — delete that file after recovering", + path.display() + )), + Err(stash_err) => err.context(stash_err), + } +} + +fn after_editor( + source: rbw::edit::Source, + plaintext: &str, + result: anyhow::Result, +) -> anyhow::Result { + result.map_err(|err| { + // Server write already landed. Do not stash, and do not tell + // a pipe caller to regenerate the value. + if is_local_cache_refresh_failed(&err) { + err + } else if persist_unsaved_after_edit(source) { + err_with_unsaved(err, plaintext) + } else { + err.context( + "the piped value was not written to disk; \ + re-run the producing command", + ) + } + }) +} + +#[derive(Debug)] +struct LocalCacheRefreshFailed; + +impl std::fmt::Display for LocalCacheRefreshFailed { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str( + "saved on the server, but local cache refresh failed; \ + run `rbw sync` and do not retry the write", + ) + } +} + +impl std::error::Error for LocalCacheRefreshFailed {} + +/// Persist local state after `add` / `edit_with_meta` already returned +/// Ok. `new_token` is that Ok value's optional refreshed access token, +/// so the call site has to have completed the server write first. +/// Token save, vault sync, and db reload are exit 2 on failure. +fn after_server_write( + db: &mut rbw::db::Db, + new_token: Option, +) -> anyhow::Result<()> { + if let Some(new_token) = new_token { + db.access_token = Some(new_token); + save_db(db).context(LocalCacheRefreshFailed)?; + } + crate::actions::sync().context(LocalCacheRefreshFailed)?; + *db = load_db().context(LocalCacheRefreshFailed)?; + Ok(()) +} + +fn is_local_cache_refresh_failed(err: &anyhow::Error) -> bool { + // anyhow::Error::is sees a `.context(LocalCacheRefreshFailed)` + // wrapper; `chain()` downcast does not, because the concrete + // type is anyhow's private context object. + err.is::() +} + +pub fn exit_status(err: &anyhow::Error) -> i32 { + if is_local_cache_refresh_failed(err) { + 2 + } else { + 1 + } +} + +fn put_cipher( + db: &mut rbw::db::Db, + entry: &rbw::db::Entry, + data: &rbw::db::EntryData, + fields: &[rbw::db::Field], + notes: Option<&str>, + history: &[rbw::db::HistoryEntry], +) -> anyhow::Result<()> { + let meta = entry.cipher_write_meta(); + let access_token = db.access_token.as_ref().unwrap(); + let refresh_token = db.refresh_token.as_ref().unwrap(); + let (new_token, ()) = rbw::actions::edit_with_meta( access_token, refresh_token, &entry.id, entry.org_id.as_deref(), &entry.name, - &data, - &fields, - notes.as_deref(), + data, + fields, + notes, entry.folder_id.as_deref(), - &history, - )? { - db.access_token = Some(access_token); - save_db(&db)?; + history, + &meta, + )?; + // Agent rebuilds the master-password-reprompt ciphertext set only + // during sync, and only after the new vault is on disk. Any local + // failure after the PUT (token save, sync, reload) is exit 2: the + // server write landed; callers must not retry it. + after_server_write(db, new_token) +} + +fn find_custom_field_index( + fields: &[DecryptedField], + name: &str, +) -> anyhow::Result { + let exact: Vec = fields + .iter() + .enumerate() + .filter_map(|(i, field)| { + field.name.as_deref().filter(|n| *n == name).map(|_| i) + }) + .collect(); + match exact.len() { + 1 => return Ok(exact[0]), + n if n > 1 => { + anyhow::bail!("multiple custom fields named '{name}'") + } + _ => {} } - crate::actions::sync()?; + let lower = name.to_lowercase(); + let case_insensitive: Vec = fields + .iter() + .enumerate() + .filter_map(|(i, field)| { + field + .name + .as_deref() + .filter(|n| n.to_lowercase() == lower) + .map(|_| i) + }) + .collect(); + match case_insensitive.len() { + 1 => Ok(case_insensitive[0]), + 0 => anyhow::bail!("no custom field named '{name}'"), + _ => anyhow::bail!("multiple custom fields matching '{name}'"), + } +} + +fn strip_trailing_newlines(s: &str) -> String { + s.trim_end_matches(['\n', '\r']).to_string() +} + +fn marker_span_start(raw: &str, marker_at: usize) -> usize { + if marker_at >= 2 + && raw.as_bytes()[marker_at - 2] == b'\r' + && raw.as_bytes()[marker_at - 1] == b'\n' + { + marker_at - 2 + } else if marker_at >= 1 && raw.as_bytes()[marker_at - 1] == b'\n' { + marker_at - 1 + } else { + marker_at + } +} + +fn strip_help_block( + raw: &str, + start: &str, + end: &str, +) -> anyhow::Result { + // Per-edit random markers: a user value may already contain a + // previous help fence. find() would then delete from that fence to + // the appended END and silently truncate. The nonce is chosen not + // to appear in the current value. + let start_at = raw.find(start); + let end_at = raw.find(end); + let combined = match (start_at, end_at) { + (Some(s), Some(e)) if e > s => { + let from = marker_span_start(raw, s); + let to = e + end.len(); + format!("{}{}", &raw[..from], &raw[to..]) + } + (None, None) => raw.to_string(), + _ => anyhow::bail!( + "unbalanced help markers; leave the rbw help block intact \ + or remove it entirely" + ), + }; + if combined.contains(start) + || combined.contains(end) + || contains_help_body(&combined) + { + anyhow::bail!( + "editor output still contains the help text; \ + remove it or leave the help block intact" + ); + } + Ok(strip_trailing_newlines(&combined)) +} + +fn contains_help_body(text: &str) -> bool { + help_body(false) + .lines() + .chain(help_body(true).lines()) + .filter(|line| !line.is_empty()) + .any(|help_line| { + text.lines() + .any(|line| line.trim_end_matches('\r') == help_line) + }) +} + +fn read_custom_field_value( + current: &str, + ty: Option, +) -> anyhow::Result<(String, rbw::edit::Source)> { + let (start, end) = unique_help_markers(current); + let help = + field_help(ty == Some(rbw::api::FieldType::Boolean), &start, &end); + let mut current = current.to_string(); + let mut attempts = 0_u8; + loop { + let (raw, source) = rbw::edit::edit_from(¤t, &help)?; + let value = match source { + rbw::edit::Source::Pipe => strip_trailing_newlines(&raw), + rbw::edit::Source::Editor => { + match strip_help_block(&raw, &start, &end) { + Ok(value) => value, + Err(e) => return Err(err_with_unsaved(e, &raw)), + } + } + }; + if value.is_empty() { + anyhow::bail!("refusing to set an empty custom field value"); + } + match validate_boolean_field(ty, &value) { + Ok(()) => return Ok((value, source)), + Err(e) if source == rbw::edit::Source::Editor => { + // Non-interactive $EDITOR that echoes the file (true, + // /bin/true) would loop forever if the stored value is + // already invalid. + attempts += 1; + if value == current || attempts >= 3 { + return Err(err_with_unsaved(e, &value)); + } + eprintln!("{e}; edit again"); + current = value; + } + Err(e) => return Err(e), + } + } +} + +fn validate_boolean_field( + ty: Option, + value: &str, +) -> anyhow::Result<()> { + if ty == Some(rbw::api::FieldType::Boolean) + && value != "true" + && value != "false" + { + anyhow::bail!("boolean custom fields must be true or false"); + } Ok(()) } +fn edit_custom_field( + db: &mut rbw::db::Db, + entry: &rbw::db::Entry, + field_name: &str, +) -> anyhow::Result<()> { + // Client::edit_with_meta has no SSH key PUT payload (unreachable!). + // Login, Secure Note, Card, and Identity all have typed PUT bodies. + if matches!(entry.data, rbw::db::EntryData::SshKey { .. }) { + anyhow::bail!( + "custom field edits are not supported for SSH key entries" + ); + } + + let (idx, ty) = named_custom_field(entry, field_name)?; + if ty == Some(rbw::api::FieldType::Linked) { + anyhow::bail!("linked custom fields cannot be edited as values"); + } + + let current = decrypt_custom_field_value(entry, idx)?; + let (new_value, source) = read_custom_field_value(¤t, ty)?; + let original_ct = entry.fields[idx].value.clone(); + after_editor( + source, + &new_value, + put_field_plaintext( + db, + entry, + field_name, + &new_value, + original_ct.as_deref(), + ¤t, + ), + ) +} + +fn named_custom_field( + entry: &rbw::db::Entry, + field_name: &str, +) -> anyhow::Result<(usize, Option)> { + let mut decrypted_names = Vec::with_capacity(entry.fields.len()); + for field in &entry.fields { + let name = field + .name + .as_ref() + .map(|name| { + crate::actions::decrypt( + name, + entry.key.as_deref(), + entry.org_id.as_deref(), + ) + }) + .transpose()?; + decrypted_names.push(DecryptedField { + name, + value: None, + ty: field.ty, + }); + } + let idx = find_custom_field_index(&decrypted_names, field_name)?; + Ok((idx, entry.fields[idx].ty)) +} + +fn decrypt_custom_field_value( + entry: &rbw::db::Entry, + idx: usize, +) -> anyhow::Result { + entry.fields[idx] + .value + .as_ref() + .map(|value| { + crate::actions::decrypt( + value, + entry.key.as_deref(), + entry.org_id.as_deref(), + ) + }) + .transpose() + .map(std::option::Option::unwrap_or_default) +} + +/// True when a concurrent save changed this field's plaintext. +/// +/// Ciphertext inequality is not enough. Official Bitwarden clients +/// encrypt `CipherView` from scratch on every save +/// (`FieldView::encrypt_composite` always encrypts name+value). AES-CBC +/// uses a random IV, so a notes-only edit rotates this field's +/// ciphertext while leaving the value alone. rbw itself leaves +/// untouched fields' ciphertext unchanged, so equal ciphertext is a +/// decrypt-free fast path. +fn target_field_plaintext_changed( + original_ciphertext: Option<&str>, + original_plaintext: &str, + server_ciphertext: Option<&str>, + server_plaintext: &str, +) -> bool { + original_ciphertext != server_ciphertext + && original_plaintext != server_plaintext +} + +fn put_field_plaintext( + db: &mut rbw::db::Db, + entry: &rbw::db::Entry, + field_name: &str, + plaintext: &str, + original_ciphertext: Option<&str>, + original_plaintext: &str, +) -> anyhow::Result<()> { + let (data, fields, notes, history) = + encrypt_named_field(entry, field_name, plaintext)?; + match put_cipher(db, entry, &data, &fields, notes.as_deref(), &history) { + Ok(()) => Ok(()), + Err(err) if is_revision_conflict(&err) => { + // PUT did not land. A failed rebase sync is still exit 1. + crate::actions::sync()?; + *db = load_db()?; + let entry = db + .entries + .iter() + .find(|item| item.id == entry.id) + .cloned() + .ok_or_else(|| { + anyhow::anyhow!("entry disappeared after conflict sync") + })?; + let (idx, ty) = named_custom_field(&entry, field_name)?; + let server_ct = entry.fields[idx].value.as_deref(); + if original_ciphertext != server_ct { + let server_pt = decrypt_custom_field_value(&entry, idx)?; + if target_field_plaintext_changed( + original_ciphertext, + original_plaintext, + server_ct, + &server_pt, + ) { + return Err(err.context( + "the same custom field was changed on the server", + )); + } + } + validate_boolean_field(ty, plaintext)?; + let (data, fields, notes, history) = + encrypt_named_field(&entry, field_name, plaintext)?; + put_cipher(db, &entry, &data, &fields, notes.as_deref(), &history) + } + Err(err) => Err(err), + } +} + +fn encrypt_named_field( + entry: &rbw::db::Entry, + field_name: &str, + plaintext: &str, +) -> anyhow::Result<( + rbw::db::EntryData, + Vec, + Option, + Vec, +)> { + let (idx, ty) = named_custom_field(entry, field_name)?; + if ty == Some(rbw::api::FieldType::Linked) { + anyhow::bail!("linked custom fields cannot be edited as values"); + } + let encrypted = crate::actions::encrypt( + plaintext, + entry.key.as_deref(), + entry.org_id.as_deref(), + )?; + let mut fields = entry.fields.clone(); + fields[idx].value = Some(encrypted); + Ok(( + entry.data.clone(), + fields, + entry.notes.clone(), + entry.history.clone(), + )) +} + pub fn remove( name: Needle, username: Option<&str>, @@ -1944,10 +2512,21 @@ pub fn purge() -> anyhow::Result<()> { stop_agent()?; remove_db()?; + remove_unsaved_edits()?; Ok(()) } +fn remove_unsaved_edits() -> anyhow::Result<()> { + let dir = rbw::dirs::unsaved_edit_dir(); + match std::fs::remove_dir_all(&dir) { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(e) + .with_context(|| format!("failed to remove {}", dir.display())), + } +} + pub fn stop_agent() -> anyhow::Result<()> { crate::actions::quit()?; @@ -2012,15 +2591,27 @@ fn version_or_quit() -> anyhow::Result { fn find_entry( db: &rbw::db::Db, - mut needle: Needle, + needle: Needle, username: Option<&str>, folder: Option<&str>, ignore_case: bool, ) -> anyhow::Result<(rbw::db::Entry, DecryptedCipher)> { + let entry = find_db_entry(db, needle, username, folder, ignore_case)?; + let decrypted_entry = decrypt_cipher(&entry)?; + Ok((entry, decrypted_entry)) +} + +fn find_db_entry( + db: &rbw::db::Db, + mut needle: Needle, + username: Option<&str>, + folder: Option<&str>, + ignore_case: bool, +) -> anyhow::Result { if let Needle::Uuid(uuid, s) = needle { for cipher in &db.entries { if uuid::Uuid::parse_str(&cipher.id) == Ok(uuid) { - return Ok((cipher.clone(), decrypt_cipher(cipher)?)); + return Ok(cipher.clone()); } } needle = Needle::Name(s); @@ -2036,8 +2627,30 @@ fn find_entry( .collect::>()?; let (entry, _) = find_entry_raw(&ciphers, &needle, username, folder, ignore_case)?; - let decrypted_entry = decrypt_cipher(&entry)?; - Ok((entry, decrypted_entry)) + Ok(entry) +} + +fn print_editable_custom_field_names( + entry: &rbw::db::Entry, +) -> anyhow::Result<()> { + if matches!(entry.data, rbw::db::EntryData::SshKey { .. }) { + anyhow::bail!("SSH key entries have no editable custom fields"); + } + for field in &entry.fields { + if field.ty == Some(rbw::api::FieldType::Linked) { + continue; + } + let Some(name) = &field.name else { + continue; + }; + let name = crate::actions::decrypt( + name, + entry.key.as_deref(), + entry.org_id.as_deref(), + )?; + println!("{name}"); + } + Ok(()) } fn find_entry_raw( @@ -4157,6 +4770,9 @@ mod test { history: vec![], key: None, master_password_reprompt: rbw::api::CipherRepromptType::None, + favorite: false, + archived_date: None, + revision_date: None, }, DecryptedSearchCipher { id: id.to_string(), @@ -4175,4 +4791,230 @@ mod test { }, ) } + + fn custom_field(name: &str) -> DecryptedField { + DecryptedField { + name: Some(name.to_string()), + value: Some("v".to_string()), + ty: Some(rbw::api::FieldType::Hidden), + } + } + + #[test] + fn test_find_custom_field_index() { + let fields = vec![ + custom_field("GITHUB_CONTENT_TOKEN"), + custom_field("GITHUB_ISSUE_TOKEN"), + ]; + assert_eq!( + find_custom_field_index(&fields, "GITHUB_CONTENT_TOKEN").unwrap(), + 0 + ); + assert_eq!( + find_custom_field_index(&fields, "github_issue_token").unwrap(), + 1 + ); + assert!(find_custom_field_index(&fields, "missing").is_err()); + } + + #[test] + fn test_find_custom_field_index_duplicates() { + let fields = vec![custom_field("FOO"), custom_field("FOO")]; + assert!(find_custom_field_index(&fields, "FOO").is_err()); + let fields = vec![custom_field("FOO"), custom_field("foo")]; + assert_eq!(find_custom_field_index(&fields, "FOO").unwrap(), 0); + assert_eq!(find_custom_field_index(&fields, "foo").unwrap(), 1); + assert!(find_custom_field_index(&fields, "FoO").is_err()); + } + + #[test] + fn test_strip_trailing_newlines() { + assert_eq!(strip_trailing_newlines("abc\n"), "abc"); + assert_eq!(strip_trailing_newlines("abc\r\n"), "abc"); + assert_eq!(strip_trailing_newlines("abc"), "abc"); + assert_eq!(strip_trailing_newlines("a\nbc\n"), "a\nbc"); + } + + #[test] + fn test_strip_help_block() { + let start = "# --- rbw field help start TESTNONCE ---"; + let end = "# --- rbw field help end TESTNONCE ---"; + let help = field_help(false, start, end); + let contents = format!("value\n#keep{help}"); + assert_eq!( + strip_help_block(&contents, start, end).unwrap(), + "value\n#keep" + ); + + let crlf = format!("value\r\n#keep{}", help.replace('\n', "\r\n")); + assert_eq!( + strip_help_block(&crlf, start, end).unwrap(), + "value\r\n#keep" + ); + + let after = format!("value{help}# mine\n"); + assert_eq!( + strip_help_block(&after, start, end).unwrap(), + "value\n# mine" + ); + + assert_eq!(strip_help_block(&help, start, end).unwrap(), ""); + assert_eq!( + strip_help_block("just-piped\n", start, end).unwrap(), + "just-piped" + ); + + let crlf_value = format!("a\r\nb{help}"); + assert_eq!( + strip_help_block(&crlf_value, start, end).unwrap(), + "a\r\nb" + ); + + let stale = "# --- rbw field help start OLDNONCE ---"; + let with_stale = format!("keep {stale} keep{help}"); + assert_eq!( + strip_help_block(&with_stale, start, end).unwrap(), + format!("keep {stale} keep") + ); + + assert!( + strip_help_block(&format!("value\n{end}\n"), start, end).is_err() + ); + assert!(strip_help_block(&format!("value\n{start}\n"), start, end) + .is_err()); + let end_before_start = format!("value\n{end}\n{start}\n"); + assert!(strip_help_block(&end_before_start, start, end).is_err()); + let leftover_body = format!("value\n{HELP_BODY}\n"); + assert!(strip_help_block(&leftover_body, start, end).is_err()); + } + + #[test] + fn test_unique_help_markers_avoid_existing_fence() { + let stale_start = "# --- rbw field help start OLDNONCE ---"; + let stale_end = "# --- rbw field help end OLDNONCE ---"; + let current = format!("keep {stale_start} {stale_end} keep"); + let (start, end) = unique_help_markers(¤t); + assert_ne!(start, stale_start); + assert_ne!(end, stale_end); + assert!(!current.contains(&start)); + assert!(!current.contains(&end)); + } + + #[test] + fn test_validate_boolean_field() { + assert!(validate_boolean_field( + Some(rbw::api::FieldType::Hidden), + "any" + ) + .is_ok()); + assert!(validate_boolean_field( + Some(rbw::api::FieldType::Boolean), + "true" + ) + .is_ok()); + assert!(validate_boolean_field( + Some(rbw::api::FieldType::Boolean), + "false" + ) + .is_ok()); + assert!(validate_boolean_field( + Some(rbw::api::FieldType::Boolean), + "yes" + ) + .is_err()); + // Linked is rejected before read/validate, not here. + assert!(validate_boolean_field( + Some(rbw::api::FieldType::Linked), + "anything" + ) + .is_ok()); + } + + #[test] + fn test_target_field_plaintext_changed() { + let old_ct = Some("2.aaa"); + let new_ct = Some("2.bbb"); + assert!(!target_field_plaintext_changed( + old_ct, "token", old_ct, "token" + )); + // Official client re-encrypted the field in place. + assert!(!target_field_plaintext_changed( + old_ct, "token", new_ct, "token" + )); + assert!(target_field_plaintext_changed( + old_ct, "token", new_ct, "other" + )); + assert!(!target_field_plaintext_changed(None, "", None, "")); + assert!(target_field_plaintext_changed(None, "", new_ct, "other")); + } + + #[test] + fn test_persist_unsaved_after_edit_skips_pipe() { + assert!(!persist_unsaved_after_edit(rbw::edit::Source::Pipe)); + assert!(persist_unsaved_after_edit(rbw::edit::Source::Editor)); + } + + #[cfg(unix)] + #[test] + fn test_persist_unsaved_edit_permissions() { + use std::os::unix::fs::PermissionsExt as _; + let dir = tempfile::tempdir().unwrap(); + let path = + persist_unsaved_edit_in(dir.path(), "secret-value").unwrap(); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "secret-value"); + assert_eq!( + std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + assert_eq!( + std::fs::metadata(dir.path()).unwrap().permissions().mode() + & 0o777, + 0o700 + ); + } + + #[test] + fn test_after_editor_pipe_does_not_mention_stash() { + let err = after_editor::<()>( + rbw::edit::Source::Pipe, + "piped-secret", + Err(anyhow::anyhow!("put failed")), + ) + .unwrap_err(); + let msg = format!("{err:#}"); + assert!(msg.contains("put failed")); + assert!(msg.contains("the piped value was not written to disk")); + assert!(!msg.contains("plaintext you just typed")); + } + + #[test] + fn test_after_editor_does_not_stash_after_server_save() { + let err = anyhow::anyhow!("network").context(LocalCacheRefreshFailed); + let err = after_editor::<()>( + rbw::edit::Source::Editor, + "typed-secret", + Err(err), + ) + .unwrap_err(); + let msg = format!("{err:#}"); + assert!(msg.contains("do not retry the write")); + assert!(!msg.contains("plaintext you just typed")); + assert_eq!(exit_status(&err), 2); + assert_eq!(exit_status(&anyhow::anyhow!("put failed")), 1); + } + + #[test] + fn test_after_editor_pipe_does_not_rerun_after_server_save() { + let err = anyhow::anyhow!("network").context(LocalCacheRefreshFailed); + let err = after_editor::<()>( + rbw::edit::Source::Pipe, + "piped-secret", + Err(err), + ) + .unwrap_err(); + let msg = format!("{err:#}"); + assert!(msg.contains("do not retry the write")); + assert!(!msg.contains("the piped value was not written to disk")); + assert_eq!(exit_status(&err), 2); + } } diff --git a/src/bin/rbw/completion/rbw.fish b/src/bin/rbw/completion/rbw.fish index 3cbb1ec9..a6a5444a 100644 --- a/src/bin/rbw/completion/rbw.fish +++ b/src/bin/rbw/completion/rbw.fish @@ -2,7 +2,7 @@ function __fish_rbw_get_completion_name set -l cmd (commandline -xpc) set -e cmd[1] # rbw - argparse -i folder= f/field= full raw clipboard i/ignorecase h/help l/list-fields -- $cmd + argparse -i folder= f/field= full raw clipboard i/ignorecase h/help l/list-fields list-custom-fields -- $cmd set -e argv[1] # get set -l candidates (command rbw list --fields name,folder,user) @@ -57,7 +57,7 @@ function __fish_rbw_get_completion_fields set -e cmd[-1] # -f/--field end - argparse -i folder= f/field= full raw clipboard i/ignorecase h/help l/list-fields -- $cmd + argparse -i folder= f/field= full raw clipboard i/ignorecase h/help l/list-fields list-custom-fields -- $cmd set -e argv[1] # get if test (count $argv) -gt 0 @@ -65,12 +65,44 @@ function __fish_rbw_get_completion_fields end end +function __fish_rbw_edit_completion_fields + set -l cmd (commandline -xpc) + set -e cmd[1] # rbw + if test -z "$(commandline -xpt)" + set -e cmd[-1] # -f/--field + end + + argparse -i folder= f/field= i/ignorecase h/help -- $cmd + set -e argv[1] # edit + + if test (count $argv) -eq 0 + return + end + set -l get_args + if set -q _flag_folder + set -a get_args --folder $_flag_folder + end + if set -q _flag_ignorecase + set -a get_args --ignorecase + end + set -a get_args $argv[1] + if test (count $argv) -gt 1 + set -a get_args $argv[2] + end + command rbw get $get_args --list-custom-fields 2>/dev/null +end + complete -f -c rbw -n '__fish_seen_subcommand_from get edit' -a '(__fish_rbw_get_completion_name)' +complete -f -c rbw -n '__fish_seen_subcommand_from edit' -s f -l field -r -d 'Custom field to edit' -a '(__fish_rbw_edit_completion_fields)' +complete -f -c rbw -n '__fish_seen_subcommand_from edit' -l folder -r -d 'Folder name to search in' -a '(command rbw list --fields folder)' +complete -f -c rbw -n '__fish_seen_subcommand_from edit' -s i -l ignorecase -d 'Ignore case' +complete -f -c rbw -n '__fish_seen_subcommand_from edit' -s h -l help -d 'Print help' # Complete options for `rbw get` complete -f -c rbw -n '__fish_seen_subcommand_from get' -s i -l ignorecase -d 'Ignore case' complete -f -c rbw -n '__fish_seen_subcommand_from get' -s f -l field -r -d 'Field to get' -a '(__fish_rbw_get_completion_fields)' complete -f -c rbw -n '__fish_seen_subcommand_from get' -s l -l list-fields -r -d 'List fields in this entry' +complete -f -c rbw -n '__fish_seen_subcommand_from get' -l list-custom-fields -d 'List custom fields in this entry' complete -f -c rbw -n '__fish_seen_subcommand_from get' -l folder -r -d 'Folder name to search in' -a '(command rbw list --fields folder)' complete -f -c rbw -n '__fish_seen_subcommand_from get' -l full -d 'Display the notes in addition to the password' complete -f -c rbw -n '__fish_seen_subcommand_from get' -l raw -d 'Display output as JSON' diff --git a/src/bin/rbw/main.rs b/src/bin/rbw/main.rs index ff2ec740..c6109f35 100644 --- a/src/bin/rbw/main.rs +++ b/src/bin/rbw/main.rs @@ -1,6 +1,5 @@ use std::io::Write as _; -use anyhow::Context as _; use clap::{CommandFactory as _, Parser as _}; mod actions; @@ -84,6 +83,12 @@ enum Opt { clipboard: bool, #[structopt(short, long, help = "List fields in this entry")] list_fields: bool, + #[arg( + long, + help = "List custom fields in this entry (not linked fields)", + conflicts_with = "list_fields" + )] + list_custom_fields: bool, }, #[command(about = "Search for entries")] @@ -204,11 +209,24 @@ enum Opt { The editor to use is determined by the value of the \ $VISUAL or $EDITOR environment variables. The first line \ will be saved as the password and the remainder will be saved \ - as a note." + as a note.\n\n\ + Pass --field to change a custom field instead of the \ + password and notes. Piped stdin is stored as the new value \ + (trailing newlines are stripped); an interactive terminal \ + opens the editor. The named custom field must already exist. \ + Empty values are refused. Login, secure note, card, and \ + identity entries are supported; SSH key entries and linked \ + fields cannot be edited this way." )] Edit { #[command(flatten)] find_args: FindArgs, + #[arg( + short, + long, + help = "Custom field to edit instead of password/notes" + )] + field: Option, }, #[command(about = "Remove a given entry", visible_alias = "rm")] @@ -226,7 +244,8 @@ enum Opt { #[command(about = "Lock the password database")] Lock, - #[command(about = "Remove the local copy of the password database")] + #[command(about = "Remove the local copy of the password database, \ + including unsaved editor buffers")] Purge, #[command(name = "stop-agent", about = "Terminate the background agent")] @@ -346,6 +365,7 @@ fn main() { #[cfg(feature = "clipboard")] clipboard, list_fields, + list_custom_fields, } => commands::get( find_args.needle.clone(), find_args.user.as_deref(), @@ -359,6 +379,7 @@ fn main() { false, find_args.ignorecase, list_fields, + list_custom_fields, ), Opt::Search { term, @@ -430,11 +451,12 @@ fn main() { ty, ) } - Opt::Edit { find_args } => commands::edit( + Opt::Edit { find_args, field } => commands::edit( find_args.needle, find_args.user.as_deref(), find_args.folder.as_deref(), find_args.ignorecase, + field.as_deref(), ), Opt::Remove { find_args } => commands::remove( find_args.needle, @@ -515,11 +537,11 @@ fn main() { } Ok(()) } - } - .with_context(|| format!("rbw {subcommand_name}")); + }; if let Err(e) = res { - eprintln!("{e:#}"); - std::process::exit(1); + let code = commands::exit_status(&e); + eprintln!("{:#}", e.context(format!("rbw {subcommand_name}"))); + std::process::exit(code); } } diff --git a/src/db.rs b/src/db.rs index fec0af7c..7bcf728f 100644 --- a/src/db.rs +++ b/src/db.rs @@ -19,12 +19,53 @@ pub struct Entry { pub history: Vec, pub key: Option, pub master_password_reprompt: crate::api::CipherRepromptType, + // Official Bitwarden PUT maps omitted bool/enum/date fields to + // defaults (favorite=false, reprompt=None, archivedDate=null) via + // CipherRequestModel.ToCipherDetails(existingCipher). Persist what + // sync gave us so a field-only edit does not clobber them. + // + // `serde(default)` is only for reading a 1.15.0 on-disk cache. + // Those defaults are not server truth — `edit` syncs before + // resolving the selector so the first PUT after upgrade does not + // unfavorite/unarchive. + // + // Adding these fields is a Rust source break for downstream struct + // literals. JSON load stays compatible. Cipher PUTs go through + // `edit_with_meta`; the 1.15 `edit` signatures are gone because + // they omitted `key` and reset favorite/archive. + #[serde(default)] + pub favorite: bool, + #[serde(default)] + pub archived_date: Option, + #[serde(default)] + pub revision_date: Option, +} + +/// PUT metadata copied from a post-sync `Entry`. Prefer this over +/// scattering more positional arguments; tests can lock the mapping. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CipherWriteMeta<'a> { + pub key: Option<&'a str>, + pub reprompt: crate::api::CipherRepromptType, + pub favorite: bool, + pub archived_date: Option<&'a str>, + pub last_known_revision_date: Option<&'a str>, } impl Entry { pub fn master_password_reprompt(&self) -> bool { self.master_password_reprompt != crate::api::CipherRepromptType::None } + + pub fn cipher_write_meta(&self) -> CipherWriteMeta<'_> { + CipherWriteMeta { + key: self.key.as_deref(), + reprompt: self.master_password_reprompt, + favorite: self.favorite, + archived_date: self.archived_date.as_deref(), + last_known_revision_date: self.revision_date.as_deref(), + } + } } #[derive(serde::Serialize, Debug, Clone, Eq, PartialEq)] @@ -314,3 +355,55 @@ impl Db { || self.protected_key.is_none() } } + +#[cfg(test)] +mod tests { + use super::*; + + fn sample_entry() -> Entry { + Entry { + id: "id".into(), + org_id: None, + folder: None, + folder_id: None, + name: "enc".into(), + data: EntryData::SecureNote, + fields: vec![], + notes: None, + history: vec![], + key: Some("item-key".into()), + master_password_reprompt: + crate::api::CipherRepromptType::Password, + favorite: true, + archived_date: Some("2026-01-01T00:00:00Z".into()), + revision_date: Some("2026-02-01T00:00:00Z".into()), + } + } + + #[test] + fn cipher_write_meta_uses_entry_values_not_defaults() { + let entry = sample_entry(); + let meta = entry.cipher_write_meta(); + assert_eq!(meta.key, Some("item-key")); + assert_eq!(meta.reprompt, crate::api::CipherRepromptType::Password); + assert!(meta.favorite); + assert_eq!(meta.archived_date, Some("2026-01-01T00:00:00Z")); + assert_eq!( + meta.last_known_revision_date, + Some("2026-02-01T00:00:00Z") + ); + } + + #[test] + fn old_cache_json_defaults_are_not_server_truth() { + let mut value = serde_json::to_value(sample_entry()).unwrap(); + let obj = value.as_object_mut().unwrap(); + obj.remove("favorite"); + obj.remove("archived_date"); + obj.remove("revision_date"); + let entry: Entry = serde_json::from_value(value).unwrap(); + assert!(!entry.favorite); + assert!(entry.archived_date.is_none()); + assert!(entry.revision_date.is_none()); + } +} diff --git a/src/dirs.rs b/src/dirs.rs index 079fc880..43241655 100644 --- a/src/dirs.rs +++ b/src/dirs.rs @@ -10,7 +10,7 @@ pub fn make_all() -> Result<()> { Ok(()) } -fn create_dir_all_with_permissions( +pub fn create_dir_all_with_permissions( path: &std::path::Path, mode: u32, ) -> Result<()> { @@ -71,6 +71,12 @@ pub fn ssh_agent_socket_file() -> std::path::PathBuf { runtime_dir().join("ssh-agent-socket") } +/// Directory for editor buffers kept after a failed cipher PUT. +/// Under the rbw runtime dir (not a shared world-readable $TMPDIR). +pub fn unsaved_edit_dir() -> std::path::PathBuf { + runtime_dir().join("unsaved-edits") +} + fn config_dir() -> std::path::PathBuf { let project_dirs = directories::ProjectDirs::from("", "", &profile()).unwrap(); diff --git a/src/edit.rs b/src/edit.rs index 7295a93a..58a11ae0 100644 --- a/src/edit.rs +++ b/src/edit.rs @@ -4,12 +4,31 @@ use std::io::{Read as _, Write as _}; use is_terminal::IsTerminal as _; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum Source { + Pipe, + Editor, +} + +pub fn stdin_is_pipe() -> bool { + !std::io::stdin().is_terminal() +} + +/// Read an edited buffer from a pipe or `$VISUAL`/`$EDITOR`. +/// +/// Drops whether the value came from a pipe. Prefer [`edit_from`] when +/// the caller must treat those sources differently (for example to +/// avoid writing piped secrets to disk). pub fn edit(contents: &str, help: &str) -> Result { - if !std::io::stdin().is_terminal() { + Ok(edit_from(contents, help)?.0) +} + +pub fn edit_from(contents: &str, help: &str) -> Result<(String, Source)> { + if stdin_is_pipe() { // directly read from piped content return match std::io::read_to_string(std::io::stdin()) { Err(e) => Err(Error::FailedToReadFromStdin { err: e }), - Ok(res) => Ok(res), + Ok(res) => Ok((res, Source::Pipe)), }; } @@ -87,7 +106,7 @@ pub fn edit(contents: &str, help: &str) -> Result { fh.read_to_string(&mut contents).unwrap(); drop(fh); - Ok(contents) + Ok((contents, Source::Editor)) } fn contains_shell_metacharacters(cmd: &std::ffi::OsStr) -> bool { diff --git a/src/error.rs b/src/error.rs index b7789a92..204efa57 100644 --- a/src/error.rs +++ b/src/error.rs @@ -74,6 +74,12 @@ pub enum Error { #[error("invalid cipherstring: {reason}")] InvalidCipherString { reason: String }, + #[error( + "cipher was changed on the server since this snapshot; \ + run rbw sync and retry" + )] + CipherRevisionConflict, + #[error( "invalid value for ${var}: {}", .editor.to_string_lossy() diff --git a/src/protocol.rs b/src/protocol.rs index ec0c06eb..40896465 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -1,5 +1,18 @@ use std::os::unix::ffi::{OsStrExt as _, OsStringExt as _}; +/// Increment when the client/agent JSON protocol changes, even if the +/// crate version does not. +/// +/// serde ignores unknown `Action` fields. A leftover 1.15.0 agent would +/// therefore accept `Encrypt` without `entry_key`, encrypt with the +/// user/org key, and the new CLI would PUT that ciphertext while still +/// sending the item `key` — mixed-key corruption. `ensure_agent()` +/// compares [`VERSION`] and restarts the agent on mismatch, so this +/// revision is the fail-closed gate. Do not rely on bumping Cargo.toml +/// alone: the crate-version term below collides for some triples +/// (1.15.1 and 1.16.0 both add to `17_000_000`). +pub const PROTOCOL_REVISION: u32 = 1; + pub const VERSION: u32 = { const fn unwrap(res: &Result) -> u32 { match res { @@ -15,6 +28,7 @@ pub const VERSION: u32 = { unwrap(&u32::from_str_radix(major, 10)) * 1_000_000 + unwrap(&u32::from_str_radix(minor, 10)) * 1_000_000 + unwrap(&u32::from_str_radix(patch, 10)) * 1_000_000 + + PROTOCOL_REVISION }; #[derive(serde::Serialize, serde::Deserialize, Debug)] @@ -181,6 +195,9 @@ pub enum Action { }, Encrypt { plaintext: String, + // Bump PROTOCOL_REVISION when this shape changes. Unknown fields + // are ignored, so an old agent would encrypt without this key. + entry_key: Option, org_id: Option, }, ClipboardStore { @@ -199,3 +216,52 @@ pub enum Response { Encrypt { cipherstring: String }, Version { version: u32 }, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn version_includes_protocol_revision() { + assert_eq!(VERSION % 1_000_000, PROTOCOL_REVISION); + } + + #[test] + fn encrypt_action_accepts_legacy_json_without_entry_key() { + let action: Action = serde_json::from_str( + r#"{"type":"Encrypt","plaintext":"p","org_id":null}"#, + ) + .unwrap(); + match action { + Action::Encrypt { + plaintext, + entry_key, + org_id, + } => { + assert_eq!(plaintext, "p"); + assert!(entry_key.is_none()); + assert!(org_id.is_none()); + } + _ => panic!("expected Encrypt"), + } + } + + #[test] + fn encrypt_action_roundtrips_entry_key() { + let action = Action::Encrypt { + plaintext: "p".into(), + entry_key: Some("k".into()), + org_id: None, + }; + let json = serde_json::to_string(&action).unwrap(); + assert!(json.contains("entry_key")); + let back: Action = serde_json::from_str(&json).unwrap(); + match back { + Action::Encrypt { + entry_key: Some(key), + .. + } => assert_eq!(key, "k"), + other => panic!("expected Encrypt with key, got {other:?}"), + } + } +}