mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-11-11 15:21:44 +00:00
Fix: migrating a seed entry produced an index that would not parse
Found while writing a test for a mutation survivor on the API's partition gate. A high-risk STUB — what the index keeps after `agent high-risk migrate` moves an entry's substance into the encrypted partition file — was being validated against the full schema of the kind it stands in for. It is a different shape on purpose: kind, index, label, archived and the partition pointers, so an index read without the factor discloses that a high-risk entry exists and nothing about it. `seedEntrySchema` requires `word_count`, which a stub does not carry. So: - `agent high-risk migrate` on any vault holding a seed entry built an index saveVault refused to write — correctly, that check exists so a malformed index never reaches disk — but only AFTER writePartition had copied the secret into the partition file. The migration failed while leaving a copy of the secrets behind. - Worse, Python builds the byte-identical stub (high_risk_partition_store.py, migrate_high_risk_entries) and validates nothing on save, so it writes one happily. A vault Python had migrated could not be OPENED by this implementation at all. That is a cross-implementation data-compatibility break, not a TypeScript-only bug. Fixed by modelling the stub as its own shape in the entry union rather than by adding `word_count` to the stub, which would have made the two implementations write different bytes and diverged sync's canonical hashing. It is not a validation bypass: a stub holds no secret material, the `partition: "high_risk"` literal is the discriminator, and an entry claiming a known kind without it still faces that kind's own schema — asserted. Only `ssh` was ever exercised, which is exactly why this survived: ssh's schema has no required field a stub omits, and pgp's extras carry defaults. The new tests cover all five high-risk kinds, so a future kind with a required field breaks a test rather than a vault. 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
4184de9b9e
commit
5315d8fae9
3 files changed
+195
-3
No files matched your search
@@ -25,9 +25,16 @@ import {
|
||||
addPasswordEntry,
|
||||
addKeyValueEntry,
|
||||
addTotpDeterministic,
|
||||
addSshKeyEntry,
|
||||
partitionStub,
|
||||
type VaultIndex,
|
||||
} from "@seedpass/core";
|
||||
import { setFactor, tagForFactor } from "../src/highRisk.js";
|
||||
import {
|
||||
setFactor,
|
||||
tagForFactor,
|
||||
writePartition,
|
||||
factorConfigured,
|
||||
} from "../src/highRisk.js";
|
||||
import {
|
||||
UNPORTED_PREFIXES,
|
||||
ApiServer,
|
||||
@@ -1036,6 +1043,86 @@ describe("high-risk partition over the API", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses a partition entry once the session has EXPIRED, not just when locked", async () => {
|
||||
// The dangerous half of this gate. With no tag at all, dropping the check
|
||||
// fails anyway — readPartition cannot decrypt with null. But an EXPIRED
|
||||
// session still holds its tag, so without the check readPartition
|
||||
// succeeds with the stale key and hands back the secret. That is the
|
||||
// whole second factor lapsing silently, and nothing covered it: no test
|
||||
// read a partition entry through the API at all.
|
||||
// Self-sufficient rather than relying on an earlier test having set it.
|
||||
if (!factorConfigured(appDir)) await setFactor(appDir, "api-second-factor");
|
||||
const tag = await tagForFactor(appDir, "api-second-factor");
|
||||
const dir = app.profileDir(FINGERPRINT);
|
||||
|
||||
// A stub in the index, and the real record in the partition file.
|
||||
const vaultIndex = { schema_version: 4, entries: {} } as VaultIndex;
|
||||
addPasswordEntry(vaultIndex, "bank.example", 16, { username: "alice", tags: ["money"] });
|
||||
addKeyValueEntry(vaultIndex, "api-token", "TOKEN", "kv-secret-value");
|
||||
addTotpDeterministic(vaultIndex, "email-2fa", MNEMONIC);
|
||||
addTotpDeterministic(vaultIndex, "blank-secret-2fa", MNEMONIC);
|
||||
for (const entry of Object.values(vaultIndex.entries) as Record<string, unknown>[]) {
|
||||
if (entry["label"] === "blank-secret-2fa") entry["secret"] = "";
|
||||
}
|
||||
// An ssh entry, because key_value is not a high-risk kind — only ssh,
|
||||
// pgp, seed, nostr and managed_account are partitioned. And the real stub
|
||||
// builder rather than a hand-made shape, which would prove nothing about
|
||||
// what production writes.
|
||||
const stubId = addSshKeyEntry(vaultIndex, "sealed", {});
|
||||
const entries = vaultIndex.entries as unknown as Record<string, Record<string, unknown>>;
|
||||
entries[stubId] = partitionStub(stubId, entries[stubId]!, "ssh", Date.now() / 1000);
|
||||
await writeFile(
|
||||
join(dir, INDEX_FILENAME),
|
||||
await encryptV3(deriveIndexKeyBytes(MNEMONIC), utf8(JSON.stringify(vaultIndex))),
|
||||
);
|
||||
await writePartition(dir, tag, {
|
||||
[stubId]: {
|
||||
type: "ssh",
|
||||
kind: "ssh",
|
||||
label: "sealed",
|
||||
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();
|
||||
const get = () =>
|
||||
fetch(`http://${bound.host}:${bound.port}/api/v1/entry/${stubId}/secret`, {
|
||||
headers: {
|
||||
authorization: `Bearer ${TOKEN}`,
|
||||
"x-seedpass-password": PASSWORD,
|
||||
},
|
||||
});
|
||||
try {
|
||||
// Live session: the secret comes back, so the test cannot pass by the
|
||||
// route being broken.
|
||||
ctx2.highRiskTag = tag;
|
||||
ctx2.highRiskExpiresAt = Math.floor(Date.now() / 1000) + 600;
|
||||
const live = await get();
|
||||
expect(live.status).toBe(200);
|
||||
// High-risk kinds derive their secret rather than storing it, so the
|
||||
// partition supplies the metadata the derivation needs. Capture what a
|
||||
// live session returns and require the expired one not to leak it.
|
||||
const secret = (await live.json()).value as string;
|
||||
expect(typeof secret).toBe("string");
|
||||
expect(secret.length).toBeGreaterThan(0);
|
||||
|
||||
// Same tag, expiry in the past. Must be 423, and must not carry the
|
||||
// secret in any form.
|
||||
ctx2.highRiskTag = tag;
|
||||
ctx2.highRiskExpiresAt = Math.floor(Date.now() / 1000) - 1;
|
||||
const expired = await get();
|
||||
expect(expired.status).toBe(423);
|
||||
expect(await expired.text()).not.toContain(secret);
|
||||
// And the stale tag is dropped rather than left to be retried against.
|
||||
expect(ctx2.highRiskTag).toBeNull();
|
||||
} finally {
|
||||
await srv.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("locks on request", async () => {
|
||||
await call("POST", "/api/v1/high-risk/unlock", {
|
||||
password: PASSWORD,
|
||||
|
||||
@@ -211,9 +211,50 @@ export type UnknownEntry = z.infer<typeof unknownEntrySchema>;
|
||||
* the vault). Code branching on `kind` must treat an unrecognized value as
|
||||
* opaque — materializeSecret's throwing default is the model.
|
||||
*/
|
||||
/**
|
||||
* A high-risk partition STUB: the placeholder left in the index when an
|
||||
* entry's substance moves into the encrypted partition file.
|
||||
*
|
||||
* A stub is a different shape from the entry it stands for, and validating it
|
||||
* against that entry's full schema is simply wrong. `seed` is where the
|
||||
* mistake bit: `seedEntrySchema` requires `word_count`, and a stub carries
|
||||
* only kind, index, label, archived and the partition pointers — by design,
|
||||
* so an index read without the factor discloses that a high-risk entry exists
|
||||
* and nothing about it. So `agent high-risk migrate` on any vault holding a
|
||||
* seed entry produced an index that would not parse, and saveVault refused to
|
||||
* write it — after writePartition had already copied the secret into the
|
||||
* partition file.
|
||||
*
|
||||
* Python builds the byte-identical stub (high_risk_partition_store.py,
|
||||
* migrate_high_risk_entries) but validates nothing on save, so it writes one
|
||||
* happily. A vault Python had migrated could therefore not be OPENED by this
|
||||
* implementation at all. Fixing it here rather than by adding `word_count` to
|
||||
* the stub keeps the two byte-identical, which is what sync's canonical
|
||||
* hashing needs.
|
||||
*
|
||||
* This is not a validation bypass: a stub holds no secret material, so there
|
||||
* is nothing for a malformed one to smuggle. It must still be a stub — the
|
||||
* `partition` literal is the discriminator, and anything claiming a known
|
||||
* kind without it still faces that kind's own schema.
|
||||
*/
|
||||
export const partitionStubSchema = z
|
||||
.object({
|
||||
kind: z.string(),
|
||||
type: z.string().optional(),
|
||||
partition: z.literal("high_risk"),
|
||||
partition_ref: z.union([z.string(), z.number()]),
|
||||
index: z.number().int().nonnegative(),
|
||||
label: z.string(),
|
||||
archived: z.boolean().default(false),
|
||||
modified_ts: z.number().optional(),
|
||||
})
|
||||
.loose();
|
||||
|
||||
export const entrySchema = z.preprocess(
|
||||
withKind,
|
||||
z.union([entryUnionSchema, unknownEntrySchema]),
|
||||
// The stub comes first: it is the narrower shape, and a stub of a known
|
||||
// kind must not be measured against that kind's full schema.
|
||||
z.union([partitionStubSchema, entryUnionSchema, unknownEntrySchema]),
|
||||
) as unknown as z.ZodType<Entry>;
|
||||
|
||||
export type Entry = z.infer<typeof entryUnionSchema>;
|
||||
|
||||
@@ -20,8 +20,18 @@ import {
|
||||
partitionStub,
|
||||
isPartitionStub,
|
||||
HighRiskError,
|
||||
HIGH_RISK_KINDS,
|
||||
parseVaultIndex,
|
||||
addSshKeyEntry,
|
||||
addNostrKeyEntry,
|
||||
addSeedEntry,
|
||||
addPgpKeyEntry,
|
||||
addManagedAccountEntry,
|
||||
type VaultIndex,
|
||||
} from "@seedpass/core";
|
||||
import { highRiskFixture as fx } from "@seedpass/test-vectors";
|
||||
import { highRiskFixture as fx, mnemonics } from "@seedpass/test-vectors";
|
||||
|
||||
const MNEMONIC = mnemonics["abandon12"]!;
|
||||
|
||||
describe("interop with Python", () => {
|
||||
it("derives the same partition key tag", () => {
|
||||
@@ -132,3 +142,57 @@ describe("index stubs", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("a stub still parses as part of the index", () => {
|
||||
/**
|
||||
* The stub is what the index KEEPS after migration, so if it does not parse
|
||||
* the vault does not open. This was broken for `seed`: seedEntrySchema
|
||||
* requires `word_count` and the stub carries only kind, index, label,
|
||||
* archived and the partition pointers — deliberately, so a locked index
|
||||
* discloses that a high-risk entry exists and nothing about it.
|
||||
*
|
||||
* The consequence was not subtle. `agent high-risk migrate` on any vault
|
||||
* holding a seed entry built an index saveVault refused to write — after
|
||||
* writePartition had already copied the secret across. And Python builds
|
||||
* the byte-identical stub while validating nothing on save, so a vault
|
||||
* Python had migrated could not be opened by this implementation at all.
|
||||
*
|
||||
* Every high-risk kind is covered rather than the one that happened to be
|
||||
* in the fixture: `ssh` passed throughout, which is exactly why nothing
|
||||
* caught this. A future kind with a required field would break it again.
|
||||
*/
|
||||
const build: Record<string, (index: VaultIndex) => string> = {
|
||||
ssh: (i) => addSshKeyEntry(i, "a-key", {}),
|
||||
nostr: (i) => addNostrKeyEntry(i, "a-key", {}),
|
||||
seed: (i) => addSeedEntry(i, "a-seed", {}),
|
||||
pgp: (i) => addPgpKeyEntry(i, "a-key", {}),
|
||||
managed_account: (i) => addManagedAccountEntry(i, "an-account", MNEMONIC, {}),
|
||||
};
|
||||
|
||||
for (const kind of HIGH_RISK_KINDS) {
|
||||
it(`round-trips a ${kind} stub through parseVaultIndex`, () => {
|
||||
const index = { schema_version: 4, entries: {} } as VaultIndex;
|
||||
const id = build[kind]!(index);
|
||||
const entries = index.entries as unknown as Record<string, Record<string, unknown>>;
|
||||
entries[id] = partitionStub(id, entries[id]!, kind, 1_700_000_000);
|
||||
|
||||
// Through JSON, the way saveVault validates it.
|
||||
const parsed = parseVaultIndex(JSON.parse(JSON.stringify(index)));
|
||||
const stub = (parsed.entries as unknown as Record<string, Record<string, unknown>>)[id]!;
|
||||
expect(isPartitionStub(stub)).toBe(true);
|
||||
expect(stub["label"]).toBe(entries[id]!["label"]);
|
||||
// The substance is gone — that is the point of the stub.
|
||||
expect(JSON.stringify(stub)).not.toContain("secret");
|
||||
});
|
||||
}
|
||||
|
||||
it("does not let a NON-stub entry skip its own schema", () => {
|
||||
// The stub shape is tried first, so it must be pinned to the partition
|
||||
// marker. A malformed seed entry without it still has to fail.
|
||||
const index = { schema_version: 4, entries: {} } as VaultIndex;
|
||||
const id = addSeedEntry(index, "a-seed", {});
|
||||
const entries = index.entries as unknown as Record<string, Record<string, unknown>>;
|
||||
delete entries[id]!["word_count"];
|
||||
expect(() => parseVaultIndex(JSON.parse(JSON.stringify(index)))).toThrow();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user