diff --git a/js/packages/cli/src/api/routes.ts b/js/packages/cli/src/api/routes.ts index fd6a73db..cec88e84 100644 --- a/js/packages/cli/src/api/routes.ts +++ b/js/packages/cli/src/api/routes.ts @@ -10,7 +10,7 @@ import { readFile, mkdir, rm } from "node:fs/promises"; import { existsSync } from "node:fs"; -import { basename, dirname, join, resolve as resolvePath } from "node:path"; +import { join, resolve as resolvePath } from "node:path"; import { addDocumentEntry, addKeyValueEntry, @@ -39,6 +39,7 @@ import { removeLink, restoreEntry, isPartitionStub, + parsePartitionRecord, emitEntryEvents, splitSecret, recoverSecret, @@ -48,7 +49,6 @@ import { semanticStatus, isStaleSemanticIndex, type SemanticRecord, - totpCodeAt, Bip85, type Entry, type PasswordPolicy, @@ -65,7 +65,7 @@ import { ConfigValueError, } from "../configFile.js"; import { openVault, saveVault, saveVaultHoldingLock, withVaultLock, atomicWrite } from "../vaultFile.js"; -import { entryMetadata, refFor, resolveEntry } from "../refs.js"; +import { entryMetadata, refFor } from "../refs.js"; import { materializeSecret } from "../secrets.js"; import { createIndexBackup } from "../backups.js"; import { factorConfigured, tagForFactor, readPartition, partitionPath } from "../highRisk.js"; @@ -81,7 +81,7 @@ import { listRecoveryDrills, verifyRecoveryDrills, } from "../recoveryDrills.js"; -import { HttpError, type ApiRequest, type ApiResponse, type ApiServer } from "./server.js"; +import { HttpError, type ApiRequest, type ApiServer } from "./server.js"; /** * Route prefixes that exist in the Python API but are not implemented here. @@ -136,8 +136,8 @@ export interface ApiContext { async function hydratePartitionedFor( ctx: ApiContext, id: string, - entry: Record, -): Promise> { + entry: Entry, +): Promise { if (!isPartitionStub(entry)) return entry; const live = ctx.highRiskTag !== null && ctx.highRiskExpiresAt > ctx.now() / 1000; if (!live) { @@ -156,7 +156,14 @@ async function hydratePartitionedFor( `entry ${id} points at a high-risk record that is not in the partition file`, ); } - return full; + // Validated, not asserted: this record is about to become a secret. A + // HighRiskError here is a 409 like the missing-record case above -- the + // request is fine, the stored data is not what it claims to be. + try { + return parsePartitionRecord(id, full); + } catch (e) { + throw new HttpError(409, (e as Error).message); + } } function requireUnlocked(ctx: ApiContext): string { @@ -320,10 +327,10 @@ export function registerRoutes(server: ApiServer, ctx: ApiContext): void { const id = entryIdOf(req); const stub = vault.index.entries[id]; if (!stub) throw new HttpError(404, "Not found"); - const entry = await hydratePartitionedFor(ctx, id, stub as unknown as Record); + const entry = await hydratePartitionedFor(ctx, id, stub); const timestamp = optInt(req.query.get("at") ?? undefined, "at"); const config = await loadConfig(profileDir(ctx), mnemonic); - const secret = materializeSecret(vault.index, id, entry as Entry, mnemonic, { + const secret = materializeSecret(vault.index, id, entry, mnemonic, { ...(timestamp !== undefined && { timestamp }), basePolicy: passwordPolicyFromConfig(config), }); diff --git a/js/packages/cli/src/program.ts b/js/packages/cli/src/program.ts index b80da904..436bbe06 100644 --- a/js/packages/cli/src/program.ts +++ b/js/packages/cli/src/program.ts @@ -30,6 +30,7 @@ import { splitSecret, recoverSecret, isPartitionStub, + parsePartitionRecord, emitEntryEvents, partitionStub, type Entry, @@ -170,7 +171,6 @@ function resolveHome(p: string): string { import { AppDir, resolveAppDir, INDEX_FILENAME, BACKUP_EXTENSION, defaultBackupFilename, DEFAULT_PBKDF2_ITERATIONS } from "./appDir.js"; import { loadConfig, - saveConfig, mutateConfig, passwordPolicyFromConfig, } from "./configFile.js"; @@ -497,7 +497,8 @@ async function openReadAccess(opts: GlobalOpts): Promise { `partition file; the partition may be from a different factor.`, ); } - return full as unknown as Entry; + // Validated, not asserted: this record is about to become a secret. + return parsePartitionRecord(id, full); }; const localSecret = async (id: string, timestamp?: number) => { diff --git a/js/packages/cli/test/agentSecurity.test.ts b/js/packages/cli/test/agentSecurity.test.ts index e538bbc8..2cbb8f31 100644 --- a/js/packages/cli/test/agentSecurity.test.ts +++ b/js/packages/cli/test/agentSecurity.test.ts @@ -9,7 +9,6 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { createConnection } from "node:net"; import { mkdtemp, readFile, writeFile, stat, mkdir } from "node:fs/promises"; -import { join as joinPath } from "node:path"; import { tmpdir } from "node:os"; import { join } from "node:path"; import process from "node:process"; @@ -20,7 +19,6 @@ import { buildProgram, AgentDaemon, AgentClient, - agentSocketPath, INDEX_FILENAME, type ProgramIo, } from "../src/index.js"; diff --git a/js/packages/cli/test/api.test.ts b/js/packages/cli/test/api.test.ts index 825067ff..bde4eec0 100644 --- a/js/packages/cli/test/api.test.ts +++ b/js/packages/cli/test/api.test.ts @@ -15,7 +15,6 @@ import { existsSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; -import process from "node:process"; import { mnemonics } from "@seedpass/test-vectors"; import { generateFingerprint, @@ -1369,6 +1368,49 @@ describe("high-risk partition over the API", () => { } }); + it("refuses a partition record whose shape it cannot trust", async () => { + // The record decrypts — it is authentic, written under the right factor. + // It is just not a valid entry. Deriving from it would hand back a + // confidently wrong secret, so the route refuses instead. + if (!factorConfigured(appDir)) await setFactor(appDir, "api-second-factor"); + const tag = await tagForFactor(appDir, "api-second-factor"); + const dir = app.profileDir(FINGERPRINT); + + const vaultIndex = { schema_version: 4, entries: {} } as VaultIndex; + const stubId = addSshKeyEntry(vaultIndex, "sealed-bad", {}); + const entries = vaultIndex.entries as unknown as Record>; + entries[stubId] = partitionStub(stubId, entries[stubId]!, "ssh", Date.now() / 1000); + await writeFile( + join(dir, INDEX_FILENAME), + await encryptV3(deriveIndexKeyBytes(MNEMONIC), utf8(JSON.stringify(vaultIndex))), + ); + // A seed record with no word_count — the exact shape a divergence between + // the two implementations would produce. + await writePartition(dir, tag, { + [stubId]: { type: "seed", kind: "seed", label: "sealed-bad", index: 0 }, + }); + + const srv = new ApiServer({ host: "127.0.0.1", port: 0, token: TOKEN, rateLimit: 100_000 }); + const ctx2 = buildContext({ app, fingerprint: FINGERPRINT, mnemonic: MNEMONIC }); + registerRoutes(srv, ctx2); + const bound = await srv.listen(); + try { + ctx2.highRiskTag = tag; + ctx2.highRiskExpiresAt = Math.floor(Date.now() / 1000) + 600; + const res = await fetch( + `http://${bound.host}:${bound.port}/api/v1/entry/${stubId}/secret`, + { headers: { authorization: `Bearer ${TOKEN}`, "x-seedpass-password": PASSWORD } }, + ); + expect(res.status).toBe(409); + const body = await res.text(); + expect(body).toContain("does not match any known entry shape"); + // And the refusal does not echo the record back over the wire. + expect(body).not.toContain("sealed-bad"); + } finally { + await srv.close(); + } + }); + it("locks on request", async () => { await call("POST", "/api/v1/high-risk/unlock", { password: PASSWORD, diff --git a/js/packages/cli/test/highRisk.test.ts b/js/packages/cli/test/highRisk.test.ts index f27a773c..35f47300 100644 --- a/js/packages/cli/test/highRisk.test.ts +++ b/js/packages/cli/test/highRisk.test.ts @@ -22,7 +22,6 @@ import { utf8, addSshKeyEntry, addPasswordEntry, - partitionKeyTag, type VaultIndex, } from "@seedpass/core"; import { diff --git a/js/packages/cli/test/nostr.test.ts b/js/packages/cli/test/nostr.test.ts index 467ec06b..8ec2fc8c 100644 --- a/js/packages/cli/test/nostr.test.ts +++ b/js/packages/cli/test/nostr.test.ts @@ -4,7 +4,7 @@ */ import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdtemp, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import process from "node:process"; diff --git a/js/packages/core/src/index.ts b/js/packages/core/src/index.ts index cf1a6840..7e5909ac 100644 --- a/js/packages/core/src/index.ts +++ b/js/packages/core/src/index.ts @@ -100,6 +100,7 @@ export { generatePartitionKey, partitionStub, isPartitionStub, + parsePartitionRecord, type PartitionEnvelope, } from "./vault/highRiskPartition.js"; export { diff --git a/js/packages/core/src/vault/highRiskPartition.ts b/js/packages/core/src/vault/highRiskPartition.ts index 8c844e01..136f64ea 100644 --- a/js/packages/core/src/vault/highRiskPartition.ts +++ b/js/packages/core/src/vault/highRiskPartition.ts @@ -18,6 +18,7 @@ import { pbkdf2 } from "@noble/hashes/pbkdf2.js"; import { base64, base64url } from "@scure/base"; import { fernetDecrypt, fernetEncrypt } from "./fernet.js"; import { bytesToHex, utf8 } from "../util/bytes.js"; +import { entrySchema, type Entry } from "../schema/entries.js"; export const PARTITION_FILENAME = "seedpass_high_risk_entries.json.enc"; export const PARTITION_SCHEMA_VERSION = 1; @@ -72,8 +73,12 @@ export async function decryptPartition( let plaintext: Uint8Array; try { plaintext = await fernetDecrypt(partitionFileKey(tag), blob); - } catch (cause) { - // The same reason string Python raises, so callers can branch on it. + } catch { + // The cause is dropped deliberately, not carelessly: the reason a Fernet + // decrypt failed distinguishes "wrong key" from "corrupt blob", and + // surfacing that difference hands a caller an oracle for probing the + // factor. One reason string, the same one Python raises so callers can + // branch on it, and nothing about which way it failed. throw new HighRiskError("invalid_partition_key_tag", "invalid_partition_key_tag"); } let data: unknown; @@ -223,6 +228,48 @@ export function partitionStub( }; } +/** + * Validate one decrypted partition record before anything derives from it. + * + * WHY THIS EXISTS + * + * `decryptPartition` proves the file is AUTHENTIC -- it decrypts under the + * partition key, and Fernet carries an HMAC, so nobody without the factor + * wrote it. It says nothing about the record's SHAPE: it checks only that + * each value is an object. Callers then asserted the result into `Entry` and + * handed it to materializeSecret. + * + * That was the one place in the vault where a record reached secret + * derivation without passing the schema. Everything arriving through the + * index goes through parseVaultIndex; the partition path did not, and the + * partition is where the ssh keys, pgp keys and seeds live. Python writes + * these files too and validates nothing, so a shape divergence between the + * implementations -- a renamed field, a kind added on one side -- would not + * have raised anything here. It would have derived from whatever fields it + * found and returned a confidently wrong secret, which is worse than an + * error by a wide margin. + * + * Validated per RECORD rather than for the whole file on read, so one odd + * record fails one entry instead of locking the user out of every secret in + * the partition. + * + * The error names the entry and nothing else. The record is the secret. + */ +export function parsePartitionRecord( + id: string, + raw: Record, +): Entry { + const result = entrySchema.safeParse(raw); + if (!result.success) { + throw new HighRiskError( + "invalid_partition_record", + `the high-risk record for entry ${id} does not match any known entry ` + + `shape, so deriving from it could return the wrong secret`, + ); + } + return result.data; +} + /** Is this index entry a stub standing in for a partitioned record? */ export function isPartitionStub(entry: Record): boolean { return String(entry["partition"] ?? "") === "high_risk"; diff --git a/js/packages/core/test/highRiskPartition.test.ts b/js/packages/core/test/highRiskPartition.test.ts index c2221e98..fa507ce3 100644 --- a/js/packages/core/test/highRiskPartition.test.ts +++ b/js/packages/core/test/highRiskPartition.test.ts @@ -19,6 +19,7 @@ import { partitionKeyTag, partitionStub, isPartitionStub, + parsePartitionRecord, HighRiskError, HIGH_RISK_KINDS, parseVaultIndex, @@ -196,3 +197,79 @@ describe("a stub still parses as part of the index", () => { expect(() => parseVaultIndex(JSON.parse(JSON.stringify(index)))).toThrow(); }); }); + +describe("a partition record is validated before it becomes a secret", () => { + /** + * decryptPartition proves the file is AUTHENTIC — it decrypts under the + * partition key and Fernet carries an HMAC. It says nothing about a + * record's SHAPE, and callers used to assert the result straight into + * `Entry` and hand it to materializeSecret. That was the one path in the + * vault reaching secret derivation without passing the schema, and it is + * where the ssh keys, pgp keys and seeds live. + */ + it("returns a well-formed record as a typed entry", () => { + const record = { + type: "ssh", + kind: "ssh", + label: "prod-server", + index: 0, + archived: false, + notes: "", + }; + const parsed = parsePartitionRecord("7", record); + expect(parsed.kind).toBe("ssh"); + expect(parsed.label).toBe("prod-server"); + }); + + it("refuses a record that does not match its own kind", () => { + // A seed entry without word_count. Python writes these files too and + // validates nothing, so a shape divergence between the implementations + // arrives here — and deriving from it would return a confidently wrong + // secret, which is worse than an error. + expect(() => + parsePartitionRecord("7", { + type: "seed", + kind: "seed", + label: "cold-wallet", + index: 0, + }), + ).toThrow(HighRiskError); + try { + parsePartitionRecord("7", { type: "seed", kind: "seed", label: "x", index: 0 }); + } catch (e) { + expect((e as HighRiskError).reason).toBe("invalid_partition_record"); + } + }); + + it("never puts the record in the error message", () => { + // The record IS the secret. An error that quotes it writes it to whatever + // is reading stderr or an HTTP response body. + try { + parsePartitionRecord("7", { + type: "seed", + kind: "seed", + label: "cold-wallet", + index: 0, + secret_material: "abandon abandon abandon", + }); + expect.unreachable("should have refused"); + } catch (e) { + const text = String((e as Error).message) + String((e as Error).stack ?? ""); + expect(text).not.toContain("abandon"); + expect(text).not.toContain("cold-wallet"); + expect(text).toContain("entry 7"); + } + }); + + it("still carries a record of an unknown kind through", () => { + // Spec §8.2: a record written by a newer build or another application is + // carried untouched, never dropped. Validation must not have quietly + // turned that into a refusal. + const parsed = parsePartitionRecord("9", { + kind: "some_future_kind", + label: "from a newer build", + whatever: 1, + }); + expect(parsed.kind).toBe("some_future_kind"); + }); +});