mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-11-13 00:01:52 +00:00
security: durable vault writes, path validation, and safe restore semantics
Continues the security-review fixes. Durability and concurrency: - Vault, config, fingerprint-registry and parent-seed writes are now atomic: temp file in the same directory, fsync, rename, then an fsync of the directory. Previously the target was truncated and rewritten in place, so a crash could destroy the vault. - The lock now spans the whole read-modify-write cycle, not just the write. Serializing writes alone still lost changes -- two commands each read the same index, mutated their copy, and wrote in turn. Every mutating command goes through mutateVault(), which holds the lock from open to save. A test runs five concurrent writers and asserts all five entries survive. - 0600 is now enforced on existing files too: writeFile's mode argument only applies on creation, so a loosened vault stayed loosened. - The index is schema-validated before it can reach disk. Path handling: - Profile fingerprints are validated (16 uppercase hex) before being used in any path, and removeProfile additionally checks the resolved directory still sits directly under the app directory. A tampered registry could otherwise have steered a recursive delete elsewhere. - Malformed entries in fingerprints.json are dropped on read. Restore semantics: - `nostr restore` merges remote state into the local vault instead of replacing it. Replacing silently discarded entries created since the last sync; that now requires --replace --yes, and any overwritten vault is copied aside first. - Deltas are bound to the manifest they extend (filtered on the e tag, and re-checked locally), so a relay cannot replay a validly-signed delta from another snapshot lineage into a restore. - Manifest selection breaks created_at ties by relay order, so a same-second republish cannot leave a restore on a stale snapshot. - mergeIndexPayloads gained an index0 option. The default still refuses to merge populated Python atlas state rather than drop it; restore uses preserve-current, keeping the local block verbatim, because Python recomputes canonical views on load and the default would block every restore into a Python-created profile. 193 core + 69 CLI tests green; 36/36 cross-implementation checks green. Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
1 parent
9926b48e16
commit
55879eaa1b
9 files changed
+519
-115
No files matched your search
@@ -0,0 +1,129 @@
|
||||
/**
|
||||
* Durability and input-validation guarantees for vault writes.
|
||||
*
|
||||
* From a security review: a bad numeric option once persisted NaN as JSON
|
||||
* null and made every later open fail; writes truncated the target in place
|
||||
* with no lock, so a crash or a concurrent command could lose the vault.
|
||||
*/
|
||||
|
||||
import { beforeAll, describe, expect, it } from "vitest";
|
||||
import { mkdtemp, readFile, writeFile, stat, chmod } from "node:fs/promises";
|
||||
import { existsSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import process from "node:process";
|
||||
import { mnemonics, entriesIndex } from "@seedpass/test-vectors";
|
||||
import { deriveIndexKeyBytes, encryptV3, utf8 } from "@seedpass/core";
|
||||
import { buildProgram, AppDir, type ProgramIo } from "../src/index.js";
|
||||
|
||||
const MNEMONIC = mnemonics["abandon12"]!;
|
||||
let vaultPath: string;
|
||||
let dir: string;
|
||||
|
||||
async function run(...argv: string[]): Promise<{ stdout: string; error?: unknown }> {
|
||||
const out: string[] = [];
|
||||
const io: ProgramIo = { out: (l: string) => out.push(l), err: () => {} };
|
||||
let error: unknown;
|
||||
const saved = process.env["SEEDPASS_MNEMONIC"];
|
||||
process.env["SEEDPASS_MNEMONIC"] = MNEMONIC;
|
||||
try {
|
||||
await buildProgram(io).parseAsync(["node", "seedpass-js", ...argv]);
|
||||
} catch (e) {
|
||||
error = e;
|
||||
} finally {
|
||||
if (saved === undefined) delete process.env["SEEDPASS_MNEMONIC"];
|
||||
else process.env["SEEDPASS_MNEMONIC"] = saved;
|
||||
}
|
||||
return { stdout: out.join("\n"), error };
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
dir = await mkdtemp(join(tmpdir(), "seedpass-durability-"));
|
||||
vaultPath = join(dir, "vault.enc");
|
||||
await writeFile(
|
||||
vaultPath,
|
||||
await encryptV3(deriveIndexKeyBytes(MNEMONIC), utf8(JSON.stringify(entriesIndex.entries))),
|
||||
);
|
||||
});
|
||||
|
||||
describe("numeric input validation", () => {
|
||||
it("rejects a non-numeric length instead of persisting NaN", async () => {
|
||||
const before = await readFile(vaultPath);
|
||||
const r = await run("--vault", vaultPath, "entry", "add", "password", "bad", "--length", "not-a-number");
|
||||
expect(String((r.error as Error).message)).toContain("whole number");
|
||||
// The vault is untouched and still opens
|
||||
expect(await readFile(vaultPath)).toEqual(before);
|
||||
expect((await run("--vault", vaultPath, "entry", "list")).error).toBeUndefined();
|
||||
});
|
||||
|
||||
it("rejects out-of-range and fractional values", async () => {
|
||||
for (const bad of ["0", "5", "999", "16.5", "1e3", "", " "]) {
|
||||
const r = await run("--vault", vaultPath, "entry", "add", "password", "bad", "--length", bad);
|
||||
expect(r.error, `--length ${JSON.stringify(bad)} should be refused`).toBeTruthy();
|
||||
}
|
||||
const totp = await run(
|
||||
"--vault", vaultPath, "entry", "add", "totp", "bad", "--period", "abc",
|
||||
);
|
||||
expect(String((totp.error as Error).message)).toContain("whole number");
|
||||
});
|
||||
|
||||
it("still accepts valid values", async () => {
|
||||
const r = await run("--vault", vaultPath, "entry", "add", "password", "good", "--length", "24");
|
||||
expect(r.error).toBeUndefined();
|
||||
expect(JSON.parse(r.stdout).length).toBe(24);
|
||||
});
|
||||
});
|
||||
|
||||
describe("atomic, permission-correct writes", () => {
|
||||
it("leaves no temp files and keeps the vault at 0600", async () => {
|
||||
await chmod(vaultPath, 0o644); // simulate a loosened file
|
||||
await run("--vault", vaultPath, "entry", "add", "key-value", "perm", "k", "v");
|
||||
const info = await stat(vaultPath);
|
||||
// writeFile's mode only applies on creation, so this must be repaired
|
||||
expect(info.mode & 0o077).toBe(0);
|
||||
const leftovers = (await import("node:fs/promises")).readdir(dir);
|
||||
expect((await leftovers).filter((f) => f.includes(".tmp"))).toEqual([]);
|
||||
expect(existsSync(`${vaultPath}.lock`)).toBe(false);
|
||||
});
|
||||
|
||||
it("survives concurrent writers without losing entries", async () => {
|
||||
const labels = ["c1", "c2", "c3", "c4", "c5"];
|
||||
await Promise.all(
|
||||
labels.map((label) =>
|
||||
run("--vault", vaultPath, "entry", "add", "key-value", label, "k", "v"),
|
||||
),
|
||||
);
|
||||
const rows = JSON.parse((await run("--vault", vaultPath, "entry", "list")).stdout) as {
|
||||
label: string;
|
||||
}[];
|
||||
const seen = rows.map((r) => r.label);
|
||||
// Without locking, concurrent read-modify-write cycles silently drop
|
||||
// entries; every one of these must be present.
|
||||
for (const label of labels) expect(seen).toContain(label);
|
||||
});
|
||||
});
|
||||
|
||||
describe("profile path validation", () => {
|
||||
it("refuses a fingerprint that is not 16 hex characters", async () => {
|
||||
const app = new AppDir(dir);
|
||||
expect(() => app.profileDir("../../etc")).toThrow(/invalid profile fingerprint/);
|
||||
await expect(app.removeProfile("../../etc")).rejects.toThrow(/invalid profile fingerprint/);
|
||||
await expect(app.switchProfile("nope")).rejects.toThrow(/invalid profile fingerprint/);
|
||||
});
|
||||
|
||||
it("ignores malformed entries in the fingerprint registry", async () => {
|
||||
const appRoot = await mkdtemp(join(tmpdir(), "seedpass-registry-"));
|
||||
await writeFile(
|
||||
join(appRoot, "fingerprints.json"),
|
||||
JSON.stringify({
|
||||
fingerprints: ["../../evil", "0123456789ABCDEF"],
|
||||
last_used: "../../evil",
|
||||
names: {},
|
||||
}),
|
||||
);
|
||||
const app = new AppDir(appRoot);
|
||||
const data = await app.readFingerprints();
|
||||
expect(data.fingerprints).toEqual(["0123456789ABCDEF"]);
|
||||
expect(data.last_used).toBeNull();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user