diff --git a/src/review/ledger-anchor-rekor.ts b/src/review/ledger-anchor-rekor.ts index afe3e863d4..b5a5b448fa 100644 --- a/src/review/ledger-anchor-rekor.ts +++ b/src/review/ledger-anchor-rekor.ts @@ -44,9 +44,11 @@ export type HashedRekordRequestV002 = { export type RekorTransparencyLogEntryResponse = { logIndex: number; logId: { keyId: string }; - /** Rekor's own entry identifier, the `uuid` a verifier passes to `rekor-cli verify`. Present as the - * response object's own key in the v2 API (one entry per request), not a fixed field name. */ - uuid: string; + /** The signed checkpoint from the entry's inclusion proof -- the v2 locator. Rekor v2 has no `uuid`: an + * entry is identified by its log index plus the checkpoint the inclusion proof was issued against, and + * that pair is what a verifier needs to re-derive inclusion against the tile-backed log. Null when the + * log omits it (the entry is still recorded; only the offline re-check is unavailable). */ + checkpoint: string | null; }; /** @@ -83,23 +85,48 @@ function base64Encode(bytes: Uint8Array): string { * only, for a single-entry submission) value. Returns `null` for any response shape that doesn't match, * rather than throwing, so a Rekor API change degrades to a recorded failure instead of an unhandled crash. */ +/** Rekor v2 serializes `log_index` as a proto3 int64, which JSON-encodes as a STRING ("0"), while a + * hand-written mock or a future revision may send a number. Both are accepted; anything non-finite is not, + * so a garbled value fails the parse rather than becoming NaN in a published backendRef. */ +function parseLogIndex(value: unknown): number | null { + if (typeof value === "number") return Number.isFinite(value) ? value : null; + if (typeof value !== "string" || value.trim() === "") return null; + const parsed = Number(value); + return Number.isFinite(parsed) ? parsed : null; +} + +/** The checkpoint lives at `inclusionProof.checkpoint`, which the protobuf-JSON encodes as + * `{ envelope: "..." }`. Accepted as a bare string too, since that is the shape a plain reading of the + * field name suggests and costs nothing to tolerate. */ +function parseCheckpoint(inclusionProof: unknown): string | null { + if (typeof inclusionProof !== "object" || inclusionProof === null) return null; + const checkpoint = (inclusionProof as Record)["checkpoint"]; + if (typeof checkpoint === "string") return checkpoint; + if (typeof checkpoint === "object" && checkpoint !== null) { + const envelope = (checkpoint as Record)["envelope"]; + if (typeof envelope === "string") return envelope; + } + return null; +} + +/** + * Parse Rekor v2's `TransparencyLogEntry`. + * + * This previously read the v1 shape: a wrapper object keyed by entry uuid, with a numeric `logIndex` and a + * `uuid` field. Rekor v2 returns the entry DIRECTLY, encodes `logIndex` as a string (proto3 int64), and has + * no `uuid` at all -- so every field the old parser required was absent or the wrong type, and this backend + * could never record a successful anchor even when the submission itself was accepted (#9851). The request + * side was already v2 (`hashedRekordRequestV002`, `POST /api/v2/log/entries`); only the response side lagged. + */ export function parseRekorResponse(raw: unknown): RekorTransparencyLogEntryResponse | null { if (typeof raw !== "object" || raw === null) return null; - const entries = Object.values(raw as Record); - const entry = entries[0]; - if (typeof entry !== "object" || entry === null) return null; - const candidate = entry as Record; + const candidate = raw as Record; + const logIndex = parseLogIndex(candidate["logIndex"]); const logId = candidate["logId"]; - if ( - typeof candidate["logIndex"] !== "number" || - typeof candidate["uuid"] !== "string" || - typeof logId !== "object" || - logId === null || - typeof (logId as Record)["keyId"] !== "string" - ) { - return null; - } - return { logIndex: candidate["logIndex"], uuid: candidate["uuid"], logId: { keyId: (logId as Record)["keyId"] as string } }; + if (logIndex === null || typeof logId !== "object" || logId === null) return null; + const keyId = (logId as Record)["keyId"]; + if (typeof keyId !== "string" || keyId === "") return null; + return { logIndex, logId: { keyId }, checkpoint: parseCheckpoint(candidate["inclusionProof"]) }; } /** @@ -156,7 +183,7 @@ export async function submitToRekor( keyId: signed.keyId, backend: "rekor", status: "ok", - backendRef: { shardBaseUrl, logIndex: parsed.logIndex, logIdKeyId: parsed.logId.keyId, uuid: parsed.uuid }, + backendRef: { shardBaseUrl, logIndex: parsed.logIndex, logIdKeyId: parsed.logId.keyId, checkpoint: parsed.checkpoint }, // Deferred past this PR: storing the full TransparencyLogEntry (inclusion proof + signed checkpoint) in // R2 for fully offline verification. Online verification (rekor-cli against shardBaseUrl + uuid) works // fully without it today; the offline path is a documented enhancement, not a gap in this backend. diff --git a/test/unit/ledger-anchor-rekor.test.ts b/test/unit/ledger-anchor-rekor.test.ts index c41fc1cd3f..08fada1cd9 100644 --- a/test/unit/ledger-anchor-rekor.test.ts +++ b/test/unit/ledger-anchor-rekor.test.ts @@ -24,7 +24,18 @@ async function realSignedAnchor(): Promise<{ signed: SignedLedgerAnchor; publicK return { signed, publicKeySpki }; } -const REKOR_RESPONSE = { "24296fb24b8ad77a": { logIndex: 42, uuid: "24296fb24b8ad77a", logId: { keyId: "c2iga0d1" } } }; +// The real Rekor v2 TransparencyLogEntry shape: the entry DIRECTLY (no uuid-keyed wrapper), `logIndex` as a +// proto3-int64 STRING, no `uuid` field, and the checkpoint nested under inclusionProof. The old fixture here +// was the v1 shape, which is why the parser's v1 assumptions survived review -- the test agreed with the bug. +const REKOR_RESPONSE = { + logIndex: "42", + logId: { keyId: "c2iga0d1" }, + kindVersion: { kind: "hashedrekord", version: "0.0.2" }, + integratedTime: "0", + inclusionPromise: null, + inclusionProof: { logIndex: "42", rootHash: "cm9vdA==", treeSize: "43", hashes: [], checkpoint: { envelope: "log2025-1.rekor.sigstore.dev\n43\ncm9vdA==\n" } }, + canonicalizedBody: "Ym9keQ==", +}; describe("buildHashedRekordRequest (#9272)", () => { it("builds the exact hashedRekordRequestV002 shape Rekor v2 expects", async () => { @@ -46,19 +57,24 @@ describe("buildHashedRekordRequest (#9272)", () => { }); describe("parseRekorResponse", () => { - it("parses a real-shaped Rekor v2 response, reading the entry under its dynamic uuid key", () => { - expect(parseRekorResponse(REKOR_RESPONSE)).toEqual({ logIndex: 42, uuid: "24296fb24b8ad77a", logId: { keyId: "c2iga0d1" } }); + // #9851: this block previously asserted the v1 shape -- its own title said "reading the entry under its + // dynamic uuid key", which was the mistaken premise, not a description of Rekor v2. Corrected rather than + // deleted, so the change of contract is visible in history. + it("parses a real-shaped Rekor v2 TransparencyLogEntry", () => { + expect(parseRekorResponse(REKOR_RESPONSE)).toEqual({ + logIndex: 42, + logId: { keyId: "c2iga0d1" }, + checkpoint: "log2025-1.rekor.sigstore.dev\n43\ncm9vdA==\n", + }); }); it("returns null (never throws) for any response shape it does not recognize", () => { expect(parseRekorResponse(null)).toBeNull(); expect(parseRekorResponse("a string")).toBeNull(); expect(parseRekorResponse({})).toBeNull(); - expect(parseRekorResponse({ x: {} })).toBeNull(); - expect(parseRekorResponse({ x: { logIndex: "not-a-number", uuid: "u", logId: { keyId: "k" } } })).toBeNull(); - expect(parseRekorResponse({ x: { logIndex: 1, uuid: "u", logId: null } })).toBeNull(); - expect(parseRekorResponse({ x: { logIndex: 1, uuid: "u" } })).toBeNull(); - expect(parseRekorResponse({ x: { logIndex: 1, uuid: "u", logId: { keyId: 7 } } })).toBeNull(); + // The old v1 wrapper is itself unrecognized now -- a log still speaking v1 must fail loudly rather than + // half-parse into a backendRef that points nowhere. + expect(parseRekorResponse({ "24296fb24b8ad77a": { logIndex: 42, uuid: "24296fb24b8ad77a", logId: { keyId: "c2iga0d1" } } })).toBeNull(); }); }); @@ -76,7 +92,7 @@ describe("submitToRekor (#9272)", () => { seq: 1, backend: "rekor", status: "ok", - backendRef: { shardBaseUrl: "https://log2026-1.rekor.sigstore.dev", logIndex: 42, logIdKeyId: "c2iga0d1", uuid: "24296fb24b8ad77a" }, + backendRef: { shardBaseUrl: "https://log2026-1.rekor.sigstore.dev", logIndex: 42, logIdKeyId: "c2iga0d1", checkpoint: expect.stringContaining("log2025-1.rekor.sigstore.dev") }, }); expect(fetchMock).toHaveBeenCalledWith( "https://log2026-1.rekor.sigstore.dev/api/v2/log/entries", @@ -163,3 +179,61 @@ describe("submitToRekor (#9272)", () => { expect(anchors[0]!.error).toContain("/api/v2/log/entries"); }); }); + +// #9851: the request side was v2 from the start (hashedRekordRequestV002, POST /api/v2/log/entries) but the +// response parser expected v1 -- a uuid-keyed wrapper, a numeric logIndex, and a uuid field. Rekor v2 sends +// none of those, so this backend could never record a successful anchor even when the log accepted the +// submission. Found on a live instance: the failure moved from "fetch failed" to "did not match the expected +// TransparencyLogEntry shape" once the shard URL was corrected. +describe("parseRekorResponse against the real v2 shape (#9851)", () => { + it("REGRESSION: parses the entry DIRECTLY, not from a uuid-keyed wrapper", () => { + const parsed = parseRekorResponse(REKOR_RESPONSE); + expect(parsed).toMatchObject({ logIndex: 42, logId: { keyId: "c2iga0d1" } }); + }); + + it("REGRESSION: accepts logIndex as a proto3-int64 STRING, which is what v2 actually sends", () => { + expect(parseRekorResponse({ ...REKOR_RESPONSE, logIndex: "907" })?.logIndex).toBe(907); + }); + + it("still accepts a numeric logIndex, so a future revision or a hand-built mock is not rejected", () => { + expect(parseRekorResponse({ ...REKOR_RESPONSE, logIndex: 907 })?.logIndex).toBe(907); + }); + + it("rejects a logIndex that is not a number at all, rather than publishing NaN in a backendRef", () => { + for (const bad of ["", " ", "not-a-number", null, undefined, {}, Number.NaN, Number.POSITIVE_INFINITY]) { + expect(parseRekorResponse({ ...REKOR_RESPONSE, logIndex: bad })).toBeNull(); + } + }); + + it("rejects a missing or malformed logId, which is what identifies the log that signed the entry", () => { + for (const bad of [undefined, null, "c2iga0d1", {}, { keyId: "" }, { keyId: 7 }]) { + expect(parseRekorResponse({ ...REKOR_RESPONSE, logId: bad })).toBeNull(); + } + }); + + it("extracts the checkpoint from inclusionProof -- the v2 locator that replaced uuid", () => { + expect(parseRekorResponse(REKOR_RESPONSE)?.checkpoint).toContain("log2025-1.rekor.sigstore.dev"); + }); + + it("accepts a bare-string checkpoint as well as the { envelope } encoding", () => { + const bare = { ...REKOR_RESPONSE, inclusionProof: { checkpoint: "a-raw-checkpoint" } }; + expect(parseRekorResponse(bare)?.checkpoint).toBe("a-raw-checkpoint"); + }); + + it("records the entry with a NULL checkpoint rather than failing when the proof omits one", () => { + // The entry is still in the log; only the offline re-check is unavailable. Dropping the whole anchor + // because one optional field is absent would lose a real, successful submission. + for (const proof of [undefined, null, {}, { checkpoint: 42 }, { checkpoint: {} }]) { + const parsed = parseRekorResponse({ ...REKOR_RESPONSE, inclusionProof: proof }); + expect(parsed).toMatchObject({ logIndex: 42 }); + expect(parsed?.checkpoint).toBeNull(); + } + }); + + it("rejects a non-object body outright", () => { + for (const bad of [null, undefined, "string", 42, []]) { + // An array has no logIndex, so it fails the same way an unexpected object would. + expect(parseRekorResponse(bad)).toBeNull(); + } + }); +}); diff --git a/test/unit/ledger-anchor-scheduler.test.ts b/test/unit/ledger-anchor-scheduler.test.ts index aead87c67f..fc31725fcc 100644 --- a/test/unit/ledger-anchor-scheduler.test.ts +++ b/test/unit/ledger-anchor-scheduler.test.ts @@ -243,7 +243,11 @@ describe("runScheduledLedgerAnchor (#9274)", () => { it("uses the REAL submitToRekor by default when no submitRekor is injected", async () => { const { env } = await keyedEnv(); await seedOneDecision(env); - const fetchSpy = vi.fn().mockResolvedValue(new Response(JSON.stringify({ x: { logIndex: 1, uuid: "u", logId: { keyId: "k" } } }), { status: 201 })); + // #9851: the real Rekor v2 TransparencyLogEntry -- the entry directly, logIndex as a proto3-int64 string. + // This mock was previously the v1 uuid-keyed wrapper, which the parser no longer accepts. + const fetchSpy = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ logIndex: "1", logId: { keyId: "k" }, inclusionProof: { checkpoint: { envelope: "cp" } } }), { status: 201 }), + ); vi.stubGlobal("fetch", fetchSpy); try { await runScheduledLedgerAnchor(env, { isHourly: true }); // no deps at all -- exercises the real default