Skip to content

Commit eceae67

Browse files
UnbreakableMJclaude
andcommitted
feat(vault): M4 slice 4 — --json on lifecycle verbs + real vault sync
Close the two remaining M4 items. --json on unlock/lock/sync/stop-agent: all four stay silent on success in human mode (existing pipelines unchanged) and emit a small envelope under --json ({"unlocked":true}, {"locked":true}, {"stopped":true}, and for sync {"synced":true,"items":N,"last_sync":"…"}). cmd_simple is retired in favour of cmd_ack (Ok-only acks) and a dedicated cmd_sync. vault sync now actually re-pulls /sync over the unlock-time session and replaces the in-memory ciphers, folder map, and last_sync stamp (it was an M3 stub that only checked the unlocked flag). The agent answers a successful re-sync with a fresh Status snapshot so --json can report the new item count. Parity with unlock: only the in-memory vault is refreshed — the agent still doesn't write the on-disk vault-store cache. A sync long after unlock can 401 once the access token expires (no refresh-token flow in M4); it surfaces as IpcError::Network, same as add/edit/remove. The /sync → (ciphers, folders) decode is factored out of perform_unlock into a shared unlock::ciphers_and_folders, now the single spine of both unlock and resync, with two direct unit tests (typed-view decode + malformed-folder skip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 645b513 commit eceae67

5 files changed

Lines changed: 250 additions & 42 deletions

File tree

CHANGELOG.md

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,29 @@ range may break in any release.
1010

1111
### Added
1212

13+
- **M4 (slice 4) — `--json` on the lifecycle verbs + a real `vault sync`.**
14+
Closes the two remaining M4 items.
15+
- `unlock`, `lock`, `sync`, and `stop-agent` now take `--json`. They stay
16+
**silent on success in human mode** (unchanged — existing pipelines keep
17+
working); under `--json` each emits a small envelope: `{"unlocked":true}`,
18+
`{"locked":true}`, `{"stopped":true}`, and for sync
19+
`{"synced":true,"items":N,"last_sync":"…"}`.
20+
- `vault sync` now actually re-pulls `/sync` over the unlock-time session and
21+
replaces the in-memory ciphers, folder map, and `last_sync` stamp (it was an
22+
M3 stub that only checked the unlocked flag). The agent answers a successful
23+
re-sync with a fresh `Status` snapshot, so `--json` can report the new item
24+
count. Parity with `unlock`: only the in-memory vault is refreshed — the
25+
agent still doesn't write the on-disk `vault-store` cache. Known limitation:
26+
a `sync` long after `unlock` can `401` once the access token expires (no
27+
refresh-token flow yet in M4); it surfaces as `IpcError::Network`, same as
28+
`add`/`edit`/`remove`.
29+
- The `/sync``(ciphers, folders)` decode is factored out of
30+
`perform_unlock` into a shared `unlock::ciphers_and_folders`, now the single
31+
spine of both `unlock` and `resync`. Tests: two direct unit tests on that
32+
function (typed-view decode + malformed-folder skipping); the CLI's
33+
`cmd_simple` was retired in favour of `cmd_ack` (Ok-only acks) and a
34+
dedicated `cmd_sync`.
35+
1336
- **M4 (slice 3) — `vault add` + `vault edit`.** The two remaining write verbs,
1437
the inverse of the read path: caller-supplied plaintext fields are encrypted
1538
**inside the agent** (the user key never leaves it) and `POST`/`PUT` to the
@@ -34,8 +57,10 @@ range may break in any release.
3457
`apply_cipher_edits` cases proving a secondary URI survives an edit (agent);
3558
and `#[ignore]`d wiremock create/update + secure-note-marker tests (api).
3659

37-
(`add` + `edit` complete the write verbs; remaining M4: `--json` on
38-
`unlock`/`lock`/`sync`/`stop-agent`, and a real re-pull `vault sync`.)
60+
(`add` + `edit` complete the write verbs; `--json` on the lifecycle verbs and
61+
the real `vault sync` landed in slice 4 above. M4 feature work is complete —
62+
the remaining M4 gate is the end-to-end `add → list/get → edit → get → remove`
63+
run against a real Vaultwarden per `docs/m2-vaultwarden.md`.)
3964

4065
### Fixed
4166

crates/vault-agent/src/server.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,15 +102,18 @@ async fn dispatch(req: Request, state: &Arc<Mutex<AgentState>>) -> Response {
102102
Response::Ok
103103
}
104104
Request::Sync => {
105-
// M3 keeps Sync minimal: the agent already pulled /sync during
106-
// unlock. A standalone re-sync lands in M4 when the cache reload
107-
// path is split out of the unlock flow.
108-
let unlocked = state.lock().await.is_unlocked();
109-
if unlocked {
110-
Response::Ok
111-
} else {
112-
Response::Error(IpcError::Locked)
113-
}
105+
// Re-pull /sync over the unlock-time session and refresh the
106+
// in-memory cache. Hold the mutex across the network call (as with
107+
// Remove) — single-user / single-agent, so a coarse lock is fine.
108+
let mut s = state.lock().await;
109+
let res = s.resync().await;
110+
s.touch();
111+
let resp = match res {
112+
Ok(()) => Response::Status(s.status_snapshot()),
113+
Err(e) => Response::Error(e),
114+
};
115+
drop(s);
116+
resp
114117
}
115118
Request::List => {
116119
let mut s = state.lock().await;

crates/vault-agent/src/state.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,30 @@ impl AgentState {
319319
Ok(Saved { id, name })
320320
}
321321

322+
/// Re-pull `/sync` over the existing authenticated session and replace the
323+
/// in-memory ciphers, folder map, and `last_sync` stamp. Requires an
324+
/// unlocked agent.
325+
///
326+
/// Like `unlock`, this refreshes only the in-memory vault — the on-disk
327+
/// encrypted cache is not (yet) written by the agent. Known limitation: a
328+
/// `sync` long after `unlock` can fail with a `401` once the access token
329+
/// expires; there is no refresh-token flow in M4, so that surfaces as
330+
/// `IpcError::Network` (shared with `add`/`edit`/`remove`).
331+
pub async fn resync(&mut self) -> Result<(), IpcError> {
332+
let v = self.vault.as_mut().ok_or(IpcError::Locked)?;
333+
let sync = v
334+
.client
335+
.sync()
336+
.await
337+
.map_err(|e| IpcError::Network(e.to_string()))?;
338+
let (ciphers, folders) =
339+
crate::unlock::ciphers_and_folders(&sync, &v.user_enc, &v.user_mac);
340+
v.ciphers = ciphers;
341+
v.folders = folders;
342+
v.last_sync = crate::unlock::now_iso();
343+
Ok(())
344+
}
345+
322346
/// Decrypt the named field on the cipher matching `query` (case-insensitive).
323347
pub fn get_item(&self, query: &str, field: Field) -> Result<Item, IpcError> {
324348
let v = self.vault.as_ref().ok_or(IpcError::Locked)?;

crates/vault-agent/src/unlock.rs

Lines changed: 104 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::collections::HashMap;
77
use uuid::Uuid;
88
use zeroize::Zeroizing;
99

10-
use vault_api::{BaseUrls, BitwardenClient};
10+
use vault_api::{BaseUrls, BitwardenClient, SyncResponse};
1111
use vault_core::cipher::{Cipher, decrypt_user_key};
1212
use vault_core::kdf::{derive_master_key, stretch_master_key};
1313
use vault_ipc::proto::Error as IpcError;
@@ -49,16 +49,42 @@ pub async fn perform_unlock(server: &str, email: &str, password: &[u8]) -> Resul
4949
let user_mac = Zeroizing::new(user_mac_arr);
5050

5151
let sync = client.sync().await.map_err(api_err)?;
52+
let (ciphers, folders) = ciphers_and_folders(&sync, &user_enc, &user_mac);
5253

53-
// Server returns Profile / Folders / Ciphers / etc as serde_json::Value
54-
// at the vault-api layer — re-cast Ciphers and Folders into typed views.
54+
// `client` holds the access token internally (`login_password` stashed
55+
// it). Hand the client itself to the vault so subsequent Sync / Remove
56+
// / Edit / Add reuse the same authenticated session.
57+
Ok(Vault {
58+
server: server.to_owned(),
59+
email: email_lower,
60+
user_enc,
61+
user_mac,
62+
ciphers,
63+
folders,
64+
client,
65+
last_sync: now_iso(),
66+
})
67+
}
68+
69+
/// Re-cast a `/sync` payload into the typed views the agent caches: a list of
70+
/// [`Cipher`]s and an `id → decrypted-name` folder map. Shared by the initial
71+
/// [`perform_unlock`] and the standalone `resync` so both paths interpret the
72+
/// server response identically.
73+
///
74+
/// The server hands ciphers/folders to `vault-api` as `serde_json::Value`;
75+
/// here they are decoded into [`Cipher`] (dropping any that don't fit the
76+
/// schema) and folder names are decrypted eagerly — there are typically few.
77+
pub(crate) fn ciphers_and_folders(
78+
sync: &SyncResponse,
79+
user_enc: &[u8; 32],
80+
user_mac: &[u8; 32],
81+
) -> (Vec<Cipher>, HashMap<String, String>) {
5582
let ciphers: Vec<Cipher> = sync
5683
.ciphers
5784
.iter()
5885
.filter_map(|v| serde_json::from_value(v.clone()).ok())
5986
.collect();
6087

61-
// Decrypt folder names eagerly — there are typically few.
6288
let mut folders = HashMap::new();
6389
for f in &sync.folders {
6490
let Some(obj) = f.as_object() else { continue };
@@ -69,26 +95,13 @@ pub async fn perform_unlock(server: &str, email: &str, password: &[u8]) -> Resul
6995
continue;
7096
};
7197
if let Ok(enc) = vault_core::EncString::parse(name_enc)
72-
&& let Ok(pt) = enc.decrypt(&user_enc, &user_mac)
98+
&& let Ok(pt) = enc.decrypt(user_enc, user_mac)
7399
&& let Ok(name) = String::from_utf8(pt)
74100
{
75101
folders.insert(id.to_owned(), name);
76102
}
77103
}
78-
79-
// `client` holds the access token internally (`login_password` stashed
80-
// it). Hand the client itself to the vault so subsequent Sync / Remove
81-
// / Edit / Add reuse the same authenticated session.
82-
Ok(Vault {
83-
server: server.to_owned(),
84-
email: email_lower,
85-
user_enc,
86-
user_mac,
87-
ciphers,
88-
folders,
89-
client,
90-
last_sync: now_iso(),
91-
})
104+
(ciphers, folders)
92105
}
93106

94107
fn api_err(e: vault_api::Error) -> IpcError {
@@ -118,7 +131,7 @@ fn crypto_err(e: vault_core::Error) -> IpcError {
118131
}
119132

120133
#[allow(clippy::many_single_char_names)] // h/m/s/y/d are the conventional date-field names
121-
fn now_iso() -> Option<String> {
134+
pub(crate) fn now_iso() -> Option<String> {
122135
use std::time::SystemTime;
123136
let now = SystemTime::now();
124137
let dur = now.duration_since(SystemTime::UNIX_EPOCH).ok()?;
@@ -152,3 +165,74 @@ fn days_to_ymd(days_since_epoch: u64) -> (i32, u32, u32) {
152165
let y = y_minus_2000 + i64::from(m <= 2) + 2000;
153166
(i32::try_from(y).unwrap_or(0), m as u32, d as u32)
154167
}
168+
169+
#[cfg(test)]
170+
mod tests {
171+
use super::*;
172+
173+
fn enc_str(enc: &[u8; 32], mac: &[u8; 32], plain: &str) -> String {
174+
vault_core::EncString::encrypt(enc, mac, plain.as_bytes()).serialize()
175+
}
176+
177+
#[test]
178+
fn ciphers_and_folders_decodes_typed_views() {
179+
let enc = [1u8; 32];
180+
let mac = [2u8; 32];
181+
182+
// Two well-formed ciphers and one folder with an encrypted name.
183+
let sync = SyncResponse {
184+
profile: serde_json::Value::Null,
185+
folders: vec![serde_json::json!({
186+
"Id": "fid-1",
187+
"Name": enc_str(&enc, &mac, "Work"),
188+
})],
189+
collections: vec![],
190+
ciphers: vec![
191+
serde_json::json!({
192+
"Id": "c1",
193+
"Type": 1,
194+
"Name": enc_str(&enc, &mac, "github.com"),
195+
}),
196+
serde_json::json!({
197+
"Id": "c2",
198+
"Type": 2,
199+
"Name": enc_str(&enc, &mac, "note"),
200+
}),
201+
],
202+
domains: serde_json::Value::Null,
203+
sends: vec![],
204+
};
205+
206+
let (ciphers, folders) = ciphers_and_folders(&sync, &enc, &mac);
207+
208+
assert_eq!(ciphers.len(), 2);
209+
assert_eq!(ciphers[0].id, "c1");
210+
assert_eq!(
211+
ciphers[0].decrypt_name(&enc, &mac).unwrap().as_deref(),
212+
Some("github.com")
213+
);
214+
assert_eq!(folders.get("fid-1").map(String::as_str), Some("Work"));
215+
}
216+
217+
#[test]
218+
fn ciphers_and_folders_skips_malformed_folder_entries() {
219+
let enc = [3u8; 32];
220+
let mac = [4u8; 32];
221+
let sync = SyncResponse {
222+
profile: serde_json::Value::Null,
223+
// Missing Name, and an undecryptable Name — both dropped silently.
224+
folders: vec![
225+
serde_json::json!({ "Id": "fid-missing-name" }),
226+
serde_json::json!({ "Id": "fid-bad", "Name": "not-an-encstring" }),
227+
],
228+
collections: vec![],
229+
ciphers: vec![],
230+
domains: serde_json::Value::Null,
231+
sends: vec![],
232+
};
233+
234+
let (ciphers, folders) = ciphers_and_folders(&sync, &enc, &mac);
235+
assert!(ciphers.is_empty());
236+
assert!(folders.is_empty());
237+
}
238+
}

0 commit comments

Comments
 (0)