From 0f596026225599e61035d310f59133e6be764c01 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN Date: Wed, 19 Aug 2026 17:30:42 -0400 Subject: [PATCH] Close every routes.ts mutation gap: seven unasserted API behaviours routes.ts now reaches zero mutation survivors (30 mutants). The suite covered what the API refuses very thoroughly and what it REPORTS almost not at all, which is where all seven gaps were. The one that could destroy data: DELETE /api/v1/fingerprint/:fingerprint guards against deleting the profile the server is currently serving. Removing that guard entirely left the suite green -- the fixture only ever had one profile, so the guard was never exercisable. The test now seeds a second profile and asserts the pairing: the served one is refused, another is deleted. A refuse-everything implementation cannot pass it. The rest report vault state that nothing checked: - GET /api/v1/fingerprint's `current` flag. Inverted, every profile EXCEPT the served one is flagged, which a profile switcher shows straight to the user. Asserted as a pairing, since "at least one is current" survives the inversion. - GET /api/v1/vault/status's `locked`. Inverted, it reports locked while unlocked; the test also confirms the flag tracks something real by checking a locked vault refuses secrets. - GET /api/v1/high-risk/status, which had no test at all: the exact-expiry boundary, the tag-AND-unexpired conjunction (a stale expiry with no tag read as open), and the expires_at null decode. Built on its own context so the clock can be frozen. - POST /api/v1/entry with an empty totp `secret` must DERIVE one, not import the empty string as a key. An entry that looks like a second factor and is keyed on nothing. - GET /api/v1/totp/export must export a stored secret as stored and derive past a blank one. Inverted, every exported secret is still well-formed base32 -- so shape assertions pass while the codes are for another account. The fixture gains an entry with a present-but-empty secret, the shape a legacy or Python-written index can hold and the API cannot create. - POST /api/v1/high-risk/unlock answers 409 not_configured rather than falling through to 401 factor_invalid, which tells someone who never set a factor that the one they just chose is wrong. Two fixture counts were hardcoded literals; they now derive from the fixture, so adding an entry cannot silently weaken "no secrets in the listing" into "no secrets in an empty listing". Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN --- js/packages/cli/test/api.test.ts | 231 ++++++++++++++++++++++++++++++- 1 file changed, 227 insertions(+), 4 deletions(-) diff --git a/js/packages/cli/test/api.test.ts b/js/packages/cli/test/api.test.ts index 7b803752..c15cb94e 100644 --- a/js/packages/cli/test/api.test.ts +++ b/js/packages/cli/test/api.test.ts @@ -42,6 +42,11 @@ const MNEMONIC = mnemonics["abandon12"]!; const FINGERPRINT = generateFingerprint(MNEMONIC); const PASSWORD = "api-test-password"; const TOKEN = "test-token-not-random-on-purpose"; +// The fixture's entries, so tests can assert against what was actually seeded +// instead of a hand-counted literal that has to be chased every time the +// fixture grows. +const FIXTURE_TOTP_LABELS = ["blank-secret-2fa", "email-2fa"]; +const FIXTURE_LABELS = ["api-token", "bank.example", ...FIXTURE_TOTP_LABELS].sort(); let appDir: string; let app: AppDir; @@ -105,6 +110,14 @@ beforeAll(async () => { addPasswordEntry(index, "bank.example", 16, { username: "alice", tags: ["money"] }); addKeyValueEntry(index, "api-token", "TOKEN", "kv-secret-value"); addTotpDeterministic(index, "email-2fa", MNEMONIC); + // A deterministic entry that also carries a BLANK `secret` field. Python + // and older indexes can produce this shape, and the export has to read it + // as "no stored secret, derive one" rather than exporting the empty string + // as if it were the key. Built here because the API refuses to create it. + addTotpDeterministic(index, "blank-secret-2fa", MNEMONIC); + for (const entry of Object.values(index.entries) as Record[]) { + if (entry["label"] === "blank-secret-2fa") entry["secret"] = ""; + } await writeFile( join(dir, INDEX_FILENAME), await encryptV3(deriveIndexKeyBytes(MNEMONIC), utf8(JSON.stringify(index))), @@ -155,7 +168,13 @@ describe("what the API refuses", () => { it("keeps secret values out of listings", async () => { const res = await call("GET", "/api/v1/entry"); expect(res.status).toBe(200); - expect(res.json).toHaveLength(3); + // Every fixture entry is listed — asserted against the fixture rather + // than a literal, so adding one does not silently weaken the check below + // into "no secrets in an empty list". + expect(res.json.length).toBeGreaterThan(0); + expect(res.json.map((r: { label: string }) => r.label).sort()).toEqual( + FIXTURE_LABELS, + ); // Reference-first, like every other surface. expect(res.text).not.toContain("kv-secret-value"); for (const row of res.json) expect(row.ref).toMatch(/^sp:\/\/entry\//); @@ -391,6 +410,134 @@ describe("entries", () => { }); }); +describe("profiles and vault state", () => { + // Everything here was reachable and unasserted: mutation testing disabled + // the delete guard, inverted the "current" flag and inverted the locked + // flag, and the suite stayed green through all three. + const OTHER = generateFingerprint(mnemonics["legal12"]!); + + it("marks exactly the served profile as current", async () => { + await app.mutateFingerprints((data) => { + if (!data.fingerprints.includes(OTHER)) data.fingerprints.push(OTHER); + data.names[OTHER] = "other"; + }); + const res = await call("GET", "/api/v1/fingerprint"); + expect(res.status).toBe(200); + const current = res.json.filter((f: { current: boolean }) => f.current); + // Assert the PAIRING, not that some entry is current: inverted, every + // profile except the served one is flagged, and "at least one is current" + // still holds. This is what a profile switcher shows the user. + expect(current).toHaveLength(1); + expect(current[0].fingerprint).toBe(FINGERPRINT); + expect( + res.json.find((f: { fingerprint: string }) => f.fingerprint === OTHER).current, + ).toBe(false); + }); + + it("refuses to delete the profile it is serving, but deletes another", async () => { + // Destructive and unrecoverable: without the guard the server removes the + // vault it is holding open, and keeps answering requests about it. Paired + // with a successful delete so the test cannot pass by refusing everything. + const refused = await call("DELETE", `/api/v1/fingerprint/${FINGERPRINT}`, { + password: PASSWORD, + }); + expect(refused.status).toBe(400); + expect(existsSync(app.profileDir(FINGERPRINT))).toBe(true); + + await mkdir(app.profileDir(OTHER), { recursive: true }); + const deleted = await call("DELETE", `/api/v1/fingerprint/${OTHER}`, { + password: PASSWORD, + }); + expect(deleted.status).toBe(200); + expect(existsSync(app.profileDir(OTHER))).toBe(false); + }); + + it("reports lock state that follows the actual seed", async () => { + expect((await call("GET", "/api/v1/vault/status")).json.locked).toBe(false); + + expect((await call("POST", "/api/v1/vault/lock")).status).toBe(200); + expect((await call("GET", "/api/v1/vault/status")).json.locked).toBe(true); + // And the flag is not decorative — a locked vault refuses secrets. + expect((await call("GET", "/api/v1/entry/1/secret", { password: PASSWORD })).status) + .not.toBe(200); + + const unlocked = await call("POST", "/api/v1/vault/unlock", { password: PASSWORD }); + expect(unlocked.status).toBe(200); + expect((await call("GET", "/api/v1/vault/status")).json.locked).toBe(false); + }); +}); + +describe("the high-risk status the API reports", () => { + // Nothing asserted this route, so mutation testing walked straight through + // it: `>` to `>=`, `&&` to `||`, and the expires_at null check inverted all + // survived. It is the only thing telling a caller whether the second factor + // is currently open, so every one of those is a lie about the vault's + // state. Built on its own context because it needs a clock it controls. + const AT_SECONDS = 1_800_000_000; + + async function frozen(): Promise<{ + ctx2: ReturnType; + status: () => Promise; + close: () => Promise; + setClock: (seconds: number) => void; + }> { + let ms = AT_SECONDS * 1000; + const srv = new ApiServer({ host: "127.0.0.1", port: 0, token: TOKEN }); + const ctx2 = buildContext({ + app, + fingerprint: FINGERPRINT, + mnemonic: MNEMONIC, + now: () => ms, + }); + registerRoutes(srv, ctx2); + const bound = await srv.listen(); + const url = `http://${bound.host}:${bound.port}/api/v1/high-risk/status`; + return { + ctx2, + setClock: (seconds: number) => { + ms = seconds * 1000; + }, + status: async () => + (await fetch(url, { headers: { authorization: `Bearer ${TOKEN}` } })).json(), + close: () => srv.close(), + }; + } + + it("reports unlocked only while a tag is held AND unexpired", async () => { + const { ctx2, status, setClock, close } = await frozen(); + try { + // No tag at all. + expect((await status()).unlocked).toBe(false); + expect((await status()).expires_at).toBeNull(); + + ctx2.highRiskTag = "a".repeat(64); + ctx2.highRiskExpiresAt = AT_SECONDS + 60; + expect(await status()).toMatchObject({ + unlocked: true, + expires_at: AT_SECONDS + 60, + }); + + // One second short of expiry: still open. + setClock(AT_SECONDS + 59); + expect((await status()).unlocked).toBe(true); + + // EXACTLY at expiry: closed. `>=` would report one more second of a + // second factor that has lapsed. + setClock(AT_SECONDS + 60); + expect((await status()).unlocked).toBe(false); + + // A future expiry with no tag is not an unlock — that is the `&&`. + // Without it, an expiry left behind by a previous session reads as open. + setClock(AT_SECONDS); + ctx2.highRiskTag = null; + ctx2.highRiskExpiresAt = AT_SECONDS + 600; + expect(await status()).toMatchObject({ unlocked: false, expires_at: null }); + } finally { + await close(); + } + }); +}); + describe("totp, config, relays, stats", () => { it("requires the master password for live TOTP codes", async () => { // A live code authenticates, so a leaked bearer token must not produce @@ -404,9 +551,72 @@ describe("totp, config, relays, stats", () => { it("returns live TOTP codes with the time remaining", async () => { const res = await call("GET", "/api/v1/totp", { password: PASSWORD }); expect(res.status).toBe(200); - expect(res.json.codes).toHaveLength(1); - expect(res.json.codes[0].code).toMatch(/^\d{6}$/); - expect(res.json.codes[0].seconds_remaining).toBeGreaterThan(0); + // One per TOTP entry in the fixture, and every one a real code. + expect(res.json.codes).toHaveLength(FIXTURE_TOTP_LABELS.length); + expect(res.json.codes.map((c: { label: string }) => c.label).sort()).toEqual( + FIXTURE_TOTP_LABELS, + ); + for (const c of res.json.codes) { + expect(c.code).toMatch(/^\d{6}$/); + expect(c.seconds_remaining).toBeGreaterThan(0); + } + }); + + it("derives past a blank stored secret rather than exporting the blank", async () => { + // `typeof secret === "string" && secret` — the second half is the part + // that matters. Loosened to `||`, an entry whose `secret` is present but + // empty exports as an empty secret: an authenticator entry that silently + // produces nothing, from an export that looked like it worked. + const exported = await call("GET", "/api/v1/totp/export", { password: PASSWORD }); + const entries = exported.json.entries as { label: string; secret: string }[]; + const blank = entries.find((e) => e.label === "blank-secret-2fa"); + expect(blank).toBeDefined(); + expect(blank!.secret).toMatch(/^[A-Z2-7]{16,}$/); + }); + + it("exports an imported secret as imported, not re-derived from the seed", async () => { + // The export picks per entry: stored secret if it has one, derived + // otherwise. Invert that choice and every exported secret is still + // well-formed base32 — so shape assertions pass while the codes the user + // loads into their authenticator are for a different account entirely. + const IMPORTED = "JBSWY3DPEHPK3PXP"; + const created = await call("POST", "/api/v1/entry", { + password: PASSWORD, + body: { kind: "totp", label: "imported-2fa", secret: IMPORTED }, + }); + expect(created.status).toBe(201); + + const exported = await call("GET", "/api/v1/totp/export", { password: PASSWORD }); + const entries = exported.json.entries as { label: string; secret: string }[]; + const mine = entries.find((e) => e.label === "imported-2fa"); + expect(mine?.secret).toBe(IMPORTED); + // And the derived entries in the same export are NOT the imported one: + // the branch has to be taken per entry, not once for the whole export. + for (const other of entries.filter((e) => e.label !== "imported-2fa")) { + expect(other.secret).not.toBe(IMPORTED); + } + }); + + it("derives a TOTP secret when none is supplied, rather than importing nothing", async () => { + // `secret.length > 0` decides between importing the caller's secret and + // deriving one from the seed. Relaxed to `>= 0`, an omitted-but-present + // empty secret would be IMPORTED -- an entry that looks like a second + // factor and is keyed on nothing. Nothing tested the empty case, so the + // relaxation went unnoticed. + const created = await call("POST", "/api/v1/entry", { + password: PASSWORD, + body: { kind: "totp", label: "derived-2fa", secret: "" }, + }); + expect(created.status).toBe(201); + + const exported = await call("GET", "/api/v1/totp/export", { password: PASSWORD }); + const entry = exported.json.entries.find( + (e: { label: string }) => e.label === "derived-2fa", + ); + expect(entry).toBeDefined(); + // A derived secret is real base32 of the usual length; an imported empty + // one could only be empty. + expect(entry.secret).toMatch(/^[A-Z2-7]{16,}$/); }); it("gates the full 2FA export behind the master password", async () => { @@ -606,6 +816,19 @@ describe("high-risk partition over the API", () => { expect(res.status).toBe(200); expect(res.json.configured).toBe(false); expect(res.json.unlocked).toBe(false); + + // And unlocking says so, rather than blaming the factor. Without this + // check the request falls through to tagForFactor, which fails and + // reports "high_risk_factor_invalid" — telling someone who has never set + // a factor that the factor they just chose is wrong, permanently. Both + // refuse, so only the distinction is testable, and the distinction is + // the whole value. + const unconfigured = await call("POST", "/api/v1/high-risk/unlock", { + password: PASSWORD, + headers: { "x-seedpass-high-risk-factor": "anything-at-all" }, + }); + expect(unconfigured.status).toBe(409); + expect(unconfigured.json.detail).toContain("not_configured"); }); it("needs the master password AND the factor to unlock", async () => {