From 51855ddea649c49b3b8508a54fe675884f41126e Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN Date: Thu, 20 Aug 2026 12:52:58 -0400 Subject: [PATCH] Validate high-risk partition records before deriving secrets from them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first real finding from the new lint gate, and the one that justified turning it on. `decryptPartition` proves a partition file is AUTHENTIC — it decrypts under the partition key and Fernet carries an HMAC, so nobody without the factor wrote it. It proves nothing about a record's SHAPE: it checks only that each value is an object. Both hydration paths then asserted the result straight 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. Every entry 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 and validates nothing, so a shape divergence between the implementations, a renamed field or 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. `parsePartitionRecord` validates per RECORD rather than per file, 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, because the record IS the secret — asserted, along with the fact that a record of an unknown kind is still carried through untouched per spec §8.2, which validation must not quietly have turned into a refusal. Typing the API's hydration path properly removed the round trip it used to make — Entry to Record and back — so the fix subtracts assertions rather than trading one for another. Also clears the twelve dead imports the gate found. One of them was not dead: the `cause` binding in decryptPartition is discarded on purpose, because the reason a Fernet decrypt failed distinguishes "wrong key" from "corrupt blob" and surfacing that hands a caller an oracle for probing the factor. It is now `catch {}` with that written down, which says it better than an ignored name. Chained assertions: 44 -> 42. Tests: 689 passing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN --- js/packages/cli/src/api/routes.ts | 25 +++--- js/packages/cli/src/program.ts | 5 +- js/packages/cli/test/agentSecurity.test.ts | 2 - js/packages/cli/test/api.test.ts | 44 ++++++++++- js/packages/cli/test/highRisk.test.ts | 1 - js/packages/cli/test/nostr.test.ts | 2 +- js/packages/core/src/index.ts | 1 + .../core/src/vault/highRiskPartition.ts | 51 +++++++++++- .../core/test/highRiskPartition.test.ts | 77 +++++++++++++++++++ 9 files changed, 190 insertions(+), 18 deletions(-) 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"); + }); +});