mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-10-29 08:52:04 +00:00
fix(cli): real rolling backups, and imports bound to their profile (L-4, L-8)
Two findings from the 2026-08-18 audit, entangled in the same files.
L-4 — the additional backup location was inert. The TUI accepted a path and
answered "Additional backups will be written to X"; nothing ever wrote one.
That is worse than an absent setting: it is false assurance in the feature a
user leans on precisely when everything else has gone wrong. The cause was
larger than the setting — Python's BackupManager is not ported at all, so
`backup_interval` was dead for the same reason.
New backups.ts ports the write side: after every committed mutation, the
encrypted index is copied to <profile>/backups/entries_db_backup_<ts>.json.enc
at 0600, and mirrored to `additional_backup_path` as <fingerprint>_<name>.
Filenames match Python's byte for byte so the two implementations share one
backup directory. Hooked after the save in both mutation funnels, matching
Python's create_backup()-after-_save_index() order, so a snapshot is the
post-mutation state.
Two deliberate deviations, both documented in the source:
- Throttling reads the newest snapshot on disk rather than Python's
in-memory _last_backup_time. Every CLI command is its own process, so an
in-memory counter would make backup_interval inert for CLI use — the same
bug in a new place.
- A failed mirror is reported rather than swallowed. Best-effort must not
fail a committed vault write, but silence would restore the false
assurance this change exists to remove.
Quick Unlock stays in the menu for item-order parity but now says it is not
implemented instead of storing a preference nothing reads. In Python the flag
never changed unlocking either — it writes an audit entry and raises a
security finding.
L-8 — plaintext backup import was not bound to a profile. A seed-only backup
is implicitly seed-bound: the wrong seed cannot decrypt it. A plaintext one
imports into any profile and is then re-derived from the TARGET seed, so every
password, SSH key, PGP key and seed silently comes out different from the one
the backup was taken to preserve. Nothing errors anywhere along the way.
Both the CLI and the TUI now compare the wrapper's fingerprint to the target
profile and require an explicit override (--allow-fingerprint-mismatch, or a
confirmation in the TUI). `vault import --inspect` reports the backup's
fingerprint and encryption mode so the mismatch is visible before committing.
The CLI also stops deciding encryption mode by regex over the whole file: new
parseBackupWrapper reads the envelope, which is the structural question being
asked.
Tests: backups.test.ts (9 cases: filename parity, 0600, verbatim ciphertext,
mirror naming, mirror-failure reporting, interval throttling, CLI end-to-end)
plus foreign-import cases in cli.test.ts and tuiMenus.test.ts using a genuine
second profile rather than a doctored file. All mutation-verified: removing
each hook or guard turns the matching tests red.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
This commit is contained in:
1 parent
063fae191f
commit
4b01ac62e0
8 files changed
+591
-16
No files matched your search
@@ -23,6 +23,7 @@ import {
|
||||
} from "./vaultFile.js";
|
||||
import {
|
||||
importBackup,
|
||||
parseBackupWrapper,
|
||||
exportBackup,
|
||||
generateFingerprint,
|
||||
assertValidMnemonic,
|
||||
@@ -159,6 +160,7 @@ import {
|
||||
} from "./configFile.js";
|
||||
import { AgentClient, AgentDaemon, agentSocketPath, DEFAULT_TTL_SECONDS } from "./agent.js";
|
||||
import { AuditLog } from "./audit.js";
|
||||
import { createIndexBackup } from "./backups.js";
|
||||
import { runTui } from "./tui/app.js";
|
||||
|
||||
export interface ProgramIo {
|
||||
@@ -283,6 +285,17 @@ async function mutateVault<T>(
|
||||
const vault = await openVault(path, mnemonic);
|
||||
const result = await fn(vault);
|
||||
await saveVaultHoldingLock(vault);
|
||||
// Rolling snapshot, after the write and still under the lock, matching
|
||||
// Python's create_backup() call order. Best-effort by design: a failed
|
||||
// backup must not make a committed vault write look like it failed.
|
||||
try {
|
||||
const config = await loadConfig(dirname(path), mnemonic);
|
||||
await createIndexBackup({ indexPath: path, config });
|
||||
} catch {
|
||||
// Deliberately swallowed -- see BackupResult.mirrorError for the one
|
||||
// failure that is worth surfacing; the rest are not worth failing a
|
||||
// successful mutation over.
|
||||
}
|
||||
return result;
|
||||
});
|
||||
}
|
||||
@@ -1592,13 +1605,25 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
|
||||
.description("restore a portable backup into the active vault")
|
||||
.option("--inspect", "verify and summarize without writing")
|
||||
.option("--yes", "confirm replacing a vault that already has entries")
|
||||
.action(async (srcFile: string, o: { inspect?: boolean; yes?: boolean }) => {
|
||||
.option(
|
||||
"--allow-fingerprint-mismatch",
|
||||
"import a backup belonging to a DIFFERENT seed (every derived secret " +
|
||||
"will change)",
|
||||
)
|
||||
.action(
|
||||
async (
|
||||
srcFile: string,
|
||||
o: { inspect?: boolean; yes?: boolean; allowFingerprintMismatch?: boolean },
|
||||
) => {
|
||||
const opts = program.opts() as GlobalOpts;
|
||||
const app = new AppDir(resolveAppDir(opts.appDir));
|
||||
const raw = await readFile(srcFile);
|
||||
// Encrypted backups need the seed; plaintext ones do not, so only
|
||||
// resolve when required (and use the agent, not just the env).
|
||||
const isEncrypted = !/"encryption_mode"\s*:\s*"none"/.test(raw.toString("utf8"));
|
||||
// Read the envelope rather than pattern-matching the file text: the
|
||||
// payload is attacker-influenced in the sense that it holds arbitrary
|
||||
// user data, and deciding whether a seed is needed by regex over the
|
||||
// whole file is a fragile way to ask a structural question.
|
||||
const wrapper = parseBackupWrapper(new Uint8Array(raw));
|
||||
const isEncrypted = wrapper.encryption_mode !== "none";
|
||||
const mnemonic = isEncrypted ? await resolveMnemonic(app, opts) : undefined;
|
||||
const parsed = await importBackup(new Uint8Array(raw), {
|
||||
...(mnemonic !== undefined && { mnemonic }),
|
||||
@@ -1612,6 +1637,8 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
|
||||
inspected: srcFile,
|
||||
schema_version: index.schema_version,
|
||||
entry_count: entryCount,
|
||||
encryption_mode: wrapper.encryption_mode,
|
||||
fingerprint: wrapper.fingerprint,
|
||||
written: false,
|
||||
}),
|
||||
);
|
||||
@@ -1619,6 +1646,26 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
|
||||
}
|
||||
|
||||
const targetMnemonic = mnemonic ?? (await resolveMnemonic(app, opts));
|
||||
|
||||
// A seed-only backup is bound to its seed implicitly — the wrong seed
|
||||
// cannot decrypt it. A plaintext backup has no such binding: it will
|
||||
// import cleanly into any profile, and then every derived entry
|
||||
// (passwords, SSH, PGP, seeds) silently produces a DIFFERENT secret
|
||||
// than the original, because the secret comes from the target profile's
|
||||
// seed and not from the backup. Nothing fails; the user just gets the
|
||||
// wrong passwords. The wrapper records which seed it came from, so
|
||||
// check it rather than trusting the operator to have noticed.
|
||||
const targetFingerprint = generateFingerprint(targetMnemonic);
|
||||
if (wrapper.fingerprint !== targetFingerprint && !o.allowFingerprintMismatch) {
|
||||
throw new Error(
|
||||
`backup belongs to profile ${wrapper.fingerprint}, but the target ` +
|
||||
`is ${targetFingerprint}. Importing it would re-derive every ` +
|
||||
`secret from the target's seed, silently producing different ` +
|
||||
`passwords than the ones in the backup. Import into the matching ` +
|
||||
`profile, or pass --allow-fingerprint-mismatch if that is really ` +
|
||||
`what you want.`,
|
||||
);
|
||||
}
|
||||
// Resolve the profile only when the target actually depends on it: an
|
||||
// explicit --vault path needs no profile, and demanding one anyway made
|
||||
// `vault import --vault x` fail on machines with no ~/.seedpass — an
|
||||
@@ -1644,7 +1691,8 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
|
||||
entry_count: entryCount,
|
||||
}),
|
||||
);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
return program;
|
||||
}
|
||||
Reference in new issue
Block a user