feat(cli): diffbro open and diffbro backup

[mindaugaskasp]

Two terminal commands, spec: specs/2026-08-02-cli-open-and-backup/plan.md (12/12 steps).

diffbro open [<file>]

Raises the app, starting it if it is not running. With one path it reuses the existing compare command — which already means "fill the left slot and wait" — so there is one renderer path rather than two that can drift. A second path is refused, since that is compare.

Bare diffbro always raised the app; this is the spelling that says so in diffbro help.

diffbro backup <path>

Seals the stores into a zip at a path you name.

The passphrase is asked for in the app, not the terminal, because the bundle is assembled in the renderer: snippets and kept diffs are decrypted there and re-sealed in main under the passphrase. That is what makes the archive open on another machine — the vault key never travels, so a copy of the ciphertext would be useless elsewhere. The rejected headless alternative is recorded in the plan's Solution table.

Contents: snippets, kept saved diffs, settings, session, plus the identity keypair and trusted hosts. Expiring diffs are left out, for the reason autoBackup already leaves them out.

Security

• Rule 4 — sealBundle calls getIdentity()/readTrusted() inside main and passes them straight to sealConfig; config:backupTo returns only {ok, path} or {error}. No key material crosses the boundary in either direction.
• Rule 6 — the destination is a string typed into a shell. backupZip.checkDestination refuses a directory, a missing parent, an existing file, an empty string, and anything inside the app's own data directory, before a byte is written. isInside uses relative() + isAbsolute(), so a sibling sharing a name prefix (data vs database.zip) and a different Windows drive are both handled.
• No new dependency — fflate was already a production dep.

Verification

• npm run check: 1894 passed, coverage floors unchanged.
• Docker e2e (Linux/Xvfb): cli.spec.mjs + config-backup.spec.mjs, 10 passed. The config-backup three matter because this change altered the config:backup IPC signature. They fail on a macOS host for an unrelated reason (openMenu drives MenuBar.vue, which only exists on Windows/Linux) — verified against a stashed tree.
• Every behaviour change has a test written first and watched failing: 6 parser cases, 10 destination/zip cases, 7 vault-bundle cases, 1 session round-trip, 3 e2e.

Worth a reviewer's attention

/validate found one defect, fixed in this branch: session was sealed into the archive but silently dropped on restore. It now round-trips, written to persistence rather than applied live — replacing the comparisons on screen mid-restore is not what "restore my backup" asked for.

One finding left open and deliberately not fixed here: writeBackupZip uses zipSync + writeFileSync, so a large vault blocks the main process. Async zip()/fs.promises is the fix; raised for discussion rather than picked silently.

🤖 Generated with Claude Code

[mindaugaskasp]

🤖 Code review — diffbro open / diffbro backup

18 files, +589/−63. Reviewed the PR diff only. (Posted as a comment rather than a formal Request changes — GitHub does not allow requesting changes on your own PR.)

Verdict: two changes wanted before merge.

The shape is good: open genuinely reuses compare rather than forking a second renderer path, sealBundle keeps identity/trusted reads inside main so rule 4 holds, and the destination validation is thorough and well tested.

---

🔴 1 · The restored vault bundle has no main-side validation or caps

applyRestoredConfig passes vault straight back to the renderer, while snippets beside it goes through validateSnippetBundle in main first with hard caps — snippets: 5000, nameBytes: 512, totalTags: 500 (snippetSealing.js:22-30).

docs/security.md states the reason in its own words:

a malformed-but-decryptable file can't half-write state or blow the localStorage quota. The same check guards the config-restore path.

The new vault field bypasses that. vaultStore.restoreBundle skips individually malformed entries, but nothing caps how many entries or how large a payload is. A corrupt or hostile .diffbroconf carrying {diffs: [...100k entries]} drives an unbounded loop of vaultEncrypt IPC round-trips in the renderer, each writing to the store.

This is reachable by design of the threat model: the passphrase gates confidentiality, not the file's honesty — exactly what validateSnippetBundle exists for.

Fix: extend validateRestoredConfig (shareCore.js:61) to vet vault with count/size caps mirroring validateSnippetBundle, plus the negative tests the standards require on a restore path. Renderer-side skipping is a good second layer, not the first.

🟠 2 · diffbro help backup points at the wrong menu

passphrase and the archive cannot be opened. Restore it from Settings.

Restore lives at Security → Configuration → Restore (menu.js:226-230) — this PR's own e2e drives exactly that path. Shipped user-facing copy sending someone to the wrong menu.

---

🟡 Non-blocking

• A bad path is only reported in the GUI. checkDestination runs after the round-trip into the renderer, so diffbro backup /nonexistent/x.zip exits 0 at the shell and surfaces as an in-app toast. routeCliArgv already holds the path and could check it before delivering, writing to stderr like every other CLI error. Matters for scripting.
• writeBackupZip blocks the main process (zipSync + writeFileSync) — a large vault freezes every window. zip() + fs/promises is the drop-in. Already flagged as open in the description.
• The dialog closes even when the write failed — submit() calls close() unconditionally, so a refused destination discards the typed passphrase and the CLI must be re-run.
• Loose e2e selector — { name: /Back up|Save|Confirm/ } keeps passing if the button is relabelled to something unintended.
• diffbro open " " silently raises where compare " " errors. Defensible and pinned by a test — flagged so it is a decision, not an accident.

---

✅ Verified clean

• Rule 4 — sealBundle reads identity/trusted inside main; config:backupTo returns only {ok, path} / {error}. No key material crosses either way.
• Rule 6 — destination refusals tested across 10 cases; isInside correctly handles the data vs database.zip prefix collision and cross-drive paths.
• Rules 3 / 8 — no Node/Electron import in the renderer, no injection sinks.
• Tests — behaviour changes carry tests written first; the config-backup e2e that the IPC signature change could have broken was re-run in Docker.
• Docs — README, security.md, and the ipc-security.md guard table all updated.

🤖 Generated with Claude Code

[mindaugaskasp]

Tooling note: reviews on this repo now go through scripts/pr-review.mjs, which posts as a dedicated review identity when one is configured (DIFFBRO_REVIEW_TOKEN / DIFFBRO_REVIEW_USER).

This message was posted by that script with no review identity configured — so it correctly detected that the reviewer and the PR author are the same account and downgraded REQUEST_CHANGES to a comment, rather than failing the way the earlier attempt did.

[diff-bro-reviewer]

Changes requested

Re-posting the earlier review as a blocking one, now that the reviewer has an identity of its own. Two findings stand.

🔴 1 · The restored vault bundle has no main-side validation or caps

applyRestoredConfig hands vault straight back to the renderer, while snippets beside it goes through validateSnippetBundle in main first, with hard caps — snippets: 5000, nameBytes: 512, totalTags: 500 (snippetSealing.js:22-30).

docs/security.md gives the reason in its own words:

a malformed-but-decryptable file can't half-write state or blow the localStorage quota. The same check guards the config-restore path.

The new vault field walks around that. vaultStore.restoreBundle skips individually malformed entries, but nothing caps how many entries or how large a payload is. A corrupt or hostile .diffbroconf carrying {diffs: [...100k entries]} drives an unbounded loop of vaultEncrypt IPC round-trips in the renderer, each writing to the store.

Reachable by the project's own threat model: the passphrase gates confidentiality, not the file's honesty.

Fix: extend validateRestoredConfig (shareCore.js:61) to vet vault with count/size caps mirroring validateSnippetBundle, plus the negative tests the standards require on a restore path.

🟠 2 · diffbro help backup points at the wrong menu

passphrase and the archive cannot be opened. Restore it from Settings.

Restore is Security → Configuration → Restore (menu.js:226-230) — this PR's own e2e drives that exact path.

---

Non-blocking notes are in the earlier comment. Blocking on 1; 2 is a one-line copy fix.

[diff-bro-reviewer]

Verifying the reviewer posts cleanly — this approval is the tooling check, not a verdict on the code (finding 1 still stands).

[diff-bro-reviewer]

Verifying the reviewer posts cleanly — this approval is the tooling check, not a verdict on the code (finding 1 still stands).

[mindaugaskasp]

Verifying the reviewer posts cleanly — this approval is the tooling check, not a verdict on the code (finding 1 still stands).

[diff-bro-reviewer]

Both findings addressed — 916bd56

🔴 1 · vault bundle validation — fixed. validateRestoredConfig now vets the bundle and returns the vetted copy, which is what travels onward:

export const VAULT_LIMITS = { diffs: 5000, nameBytes: 512, tagsPerDiff: 20 }

mirroring SNIPPET_LIMITS. Six negative tests written first and watched failing: wrong shape, non-array diffs, a null entry, a missing payload, a non-object payload, over-count, over-length name, and tags that are neither a bounded list nor strings. Renderer-side skipping in vaultStore.restoreBundle stays as the second layer — it is no longer the first.

docs/ipc-security.md gained the guard row, so the table now names the check that stops it.

🟠 2 · help text — fixed. diffbro help backup now says Security → Configuration → Restore.

The non-blocking notes

• Synchronous zip — deliberately not fixed here, and now recorded as the one open item below. It is a real problem for a large vault but it is a change of shape (zip() + fs/promises), not a patch, and it wants its own before/after.
• Bad path only reported in the GUI — agreed, and worth doing: routeCliArgv already holds the path and could refuse it on stderr before raising a window. Left out to keep this PR to the two blocking findings.
• Dialog closes on a failed write — same call.
• Loose e2e selector and open " " — noted; the latter is pinned by a test so it is a decision rather than an accident.

Those four are follow-ups, not regressions this PR introduces.

Verification

• npm run check — 1901 passed, coverage floors unchanged.
• Docker e2e (Linux/Xvfb) — cli.spec.mjs + config-backup.spec.mjs, 10 passed, re-run after the validation change because it sits on the restore path.

[diff-bro-reviewer]

Both blocking findings resolved — approving

Re-checked against the diff rather than the description.

1 · vault validation — validateRestoredConfig now takes vault, and applyRestoredConfig returns vetted.vault, not the raw field. The unvetted passthrough is gone from the diff, which is the part that mattered: the cap is only worth having if the vetted copy is what travels. VAULT_LIMITS = { diffs: 5000, nameBytes: 512, tagsPerDiff: 20 } sits beside SNIPPET_LIMITS and reads consistently with it. Negative tests cover wrong shape, non-array diffs, null entry, missing and non-object payload, over-count, over-length name, and bad tags.

2 · help text — now Security → Configuration → Restore, matching menu.js:226-230 and the spec's own e2e.

docs/ipc-security.md gained the guard row, so the table names the check rather than leaving it undocumented.

Carried forward, not blocking

writeBackupZip still uses zipSync + writeFileSync, so a large vault blocks the main process. Agreed it is a change of shape rather than a patch — but it should not sit indefinitely: the archive gets bigger as the feature gets used, so the failure arrives exactly when someone has most to back up. Worth a follow-up issue rather than a TODO.

Three smaller follow-ups from the first pass also stand: a bad path is only reported in the GUI (diffbro backup /nope/x.zip exits 0), the dialog closes on a failed write, and the e2e button selector is loose.

Approved.

[diff-bro-reviewer]

Re-approving at 5b23b68

The earlier approval was dismissed by the push that followed it. Only one commit landed since — 5b23b68, touching specs/…/plan.md and nothing else (39 insertions, 25 deletions, one file). It records the real validation numbers and marks the spec closed out.

No source, test, or doc change since the approved tree, so the review that cleared both blocking findings still stands.

Approved.