fix: answer review on ledger integrity and JWKS rotation

Bind every ledger column into the chain hash. The preimage covered only
payloadJson and prevHash, so entryType, jws, createdAt and the identifiers
were rewritable with every hash still verifying — an attestation.late entry
could be relabelled issued, or a signature swapped between entries.

Refuse to nest a storage transaction. SQLite has no nested transactions, so
a second BEGIN IMMEDIATE threw and the surrounding catch rolled back the
outer transaction. One helper now tracks the open transaction per connection
and fails explicitly instead.

Re-resolve discovery on the forced JWKS refresh. A provider that rotates its
signing key and its jwks_uri together published the new key only at the new
URI, so the forced refresh refetched the retired one and every sponsor proof
failed until the discovery cache expired. The path stays behind the
per-issuer cooldown, so an unknown kid still cannot amplify.

Import the signing key once per finalize request instead of once per commit.

Verification: build, typecheck, test (480 pass, 0 fail) and the SDK contract
check all exited 0. The two new OIDC tests were confirmed red before the fix.

Author: khaliqgant

packages/server/src/__tests__/attestations.test.ts

@@ -25,6 +25,51 @@ type FinalizeResponse = {
   attestations: Array<{ sha: string; jws: string }>;
 };
 
+type LedgerRow = {
+  seq: number;
+  org_id: string;
+  org_seq: number;
+  entry_type: string;
+  jti: string | null;
+  commit_sha: string | null;
+  repo: string | null;
+  agent_id: string | null;
+  sponsor_id: string | null;
+  payload_json: string;
+  jws: string;
+  prev_hash: string;
+  entry_hash: string;
+  created_at: string;
+};
+
+/**
+ * Recompute an entry's chain hash the way an external auditor would: from the
+ * stored columns alone, spelling out the canonical preimage rather than calling
+ * the production helper. If the preimage ever changes shape, this fails.
+ */
+function recomputeEntryHash(row: LedgerRow): string {
+  const preimage: Record<string, string | number | null> = {
+    agentId: row.agent_id,
+    commitSha: row.commit_sha,
+    createdAt: row.created_at,
+    entryType: row.entry_type,
+    jti: row.jti,
+    jws: row.jws,
+    orgId: row.org_id,
+    orgSeq: row.org_seq,
+    payloadJson: row.payload_json,
+    prevHash: row.prev_hash,
+    repo: row.repo,
+    sponsorId: row.sponsor_id,
+  };
+  const canonical = `{${
+    Object.keys(preimage).sort()
+      .map((key) => `${JSON.stringify(key)}:${JSON.stringify(preimage[key])}`)
+      .join(",")
+  }}`;
+  return crypto.createHash("sha256").update(canonical, "utf8").digest("hex");
+}
+
 function createIdentity(): StoredIdentity {
   const base = generateTestIdentity({
     id: "agent_attestation",
@@ -277,9 +322,11 @@ test("ledger is immutable and retention only deletes audit_logs", async (t) => {
     jws: "test-jws-retention",
     createdAt: "2000-01-01T00:00:00.000Z",
   });
-  const row = await app.storage.DB.prepare(
-    "SELECT seq, payload_json, prev_hash, entry_hash FROM attestation_ledger LIMIT 1",
-  ).first<{ seq: number; payload_json: string; prev_hash: string; entry_hash: string }>();
+  const row = await app.storage.DB.prepare(`
+    SELECT seq, org_id, org_seq, entry_type, jti, commit_sha, repo, agent_id, sponsor_id,
+           payload_json, jws, prev_hash, entry_hash, created_at
+    FROM attestation_ledger LIMIT 1
+  `).first<LedgerRow>();
   assert.ok(row);
   await assert.rejects(
     app.storage.DB.prepare("UPDATE attestation_ledger SET payload_json = ? WHERE seq = ?")
@@ -289,16 +336,29 @@ test("ledger is immutable and retention only deletes audit_logs", async (t) => {
     app.storage.DB.prepare("DELETE FROM attestation_ledger WHERE seq = ?")
       .bind(row.seq).run(),
   );
-  const recomputed = crypto.createHash("sha256")
-    .update(row.payload_json, "utf8")
-    .update(row.prev_hash, "utf8")
-    .digest("hex");
-  assert.equal(recomputed, row.entry_hash);
-  const handTampered = crypto.createHash("sha256")
-    .update('{"tampered":true}', "utf8")
-    .update(row.prev_hash, "utf8")
-    .digest("hex");
-  assert.notEqual(handTampered, row.entry_hash, "chain recomputation must detect a hand-tampered payload");
+  assert.equal(recomputeEntryHash(row), row.entry_hash);
+  // Every field a verifier reads must be inside the preimage, so tampering with
+  // any one of them has to break the chain — not just the payload.
+  assert.notEqual(
+    recomputeEntryHash({ ...row, payload_json: '{"tampered":true}' }),
+    row.entry_hash,
+    "chain recomputation must detect a hand-tampered payload",
+  );
+  assert.notEqual(
+    recomputeEntryHash({ ...row, entry_type: "attestation.issued" }),
+    row.entry_hash,
+    "chain recomputation must detect a relabelled entry_type",
+  );
+  assert.notEqual(
+    recomputeEntryHash({ ...row, jws: "swapped-signature" }),
+    row.entry_hash,
+    "chain recomputation must detect a swapped jws",
+  );
+  assert.notEqual(
+    recomputeEntryHash({ ...row, created_at: "1999-01-01T00:00:00.000Z" }),
+    row.entry_hash,
+    "chain recomputation must detect a rewritten created_at",
+  );
 
   await app.storage.DB.prepare(`
     INSERT INTO audit_logs (id, action, org_id, result, timestamp, created_at)

packages/server/src/__tests__/sponsor-oidc-binding.test.ts

@@ -16,6 +16,7 @@ import {
 
 const OIDC_KEY_PAIR = crypto.generateKeyPairSync("rsa", { modulusLength: 2048 });
 const OIDC_KID = "fixture-rs256-key";
+const ROTATED_OIDC_KID = "fixture-rs256-key-rotated";
 const OIDC_PUBLIC_JWK = {
   ...(OIDC_KEY_PAIR.publicKey.export({ format: "jwk" }) as JsonWebKey),
   alg: "RS256",
@@ -80,6 +81,57 @@ async function startOidcFixture(
   };
 }
 
+/**
+ * An IdP that rotates its signing key and its `jwks_uri` in the same step.
+ *
+ * This is the case a cached discovery document hides: the new key is only
+ * published at the new URI, so a client that refreshes the JWKS without
+ * re-resolving discovery refetches the retired URI and never finds it.
+ */
+async function startRotatingOidcFixture(
+  t: test.TestContext,
+  cacheControl = "public, max-age=0",
+): Promise<{ issuer: string; rotate(): void; requestCount(path: string): number }> {
+  let issuer = "";
+  let jwksPath = "/jwks-initial";
+  let servedKid = OIDC_KID;
+  const requestCounts = new Map<string, number>();
+  const server = createServer((request, response) => {
+    const path = request.url ?? "";
+    requestCounts.set(path, (requestCounts.get(path) ?? 0) + 1);
+    response.setHeader("content-type", "application/json");
+    response.setHeader("cache-control", cacheControl);
+    if (path === "/.well-known/openid-configuration") {
+      response.end(JSON.stringify({ issuer, jwks_uri: `${issuer}${jwksPath}` }));
+      return;
+    }
+    if (path === jwksPath) {
+      response.end(JSON.stringify({ keys: [{ ...OIDC_PUBLIC_JWK, kid: servedKid }] }));
+      return;
+    }
+    response.statusCode = 404;
+    response.end(JSON.stringify({ error: "not_found" }));
+  });
+
+  await new Promise<void>((resolve, reject) => {
+    server.once("error", reject);
+    server.listen(0, "127.0.0.1", resolve);
+  });
+  const address = server.address() as AddressInfo;
+  issuer = `http://127.0.0.1:${address.port}`;
+  t.after(() => new Promise<void>((resolve, reject) => {
+    server.close((error) => error ? reject(error) : resolve());
+  }));
+  return {
+    issuer,
+    rotate: () => {
+      jwksPath = "/jwks-rotated";
+      servedKid = ROTATED_OIDC_KID;
+    },
+    requestCount: (path: string) => requestCounts.get(path) ?? 0,
+  };
+}
+
 function adminAuthorization(org: string): HeadersInit {
   return {
     Authorization: `Bearer ${generateTestToken({
@@ -485,12 +537,57 @@ test("unknown OIDC kids cannot bypass JWKS cache or forced-refresh cooldown", as
   );
 
   await assertJsonResponse<SponsorProof>(await proofRequest(OIDC_KID), 201);
-  await assertJsonResponse<{ code: string }>(await proofRequest("attacker-kid-1"), 403);
-  await assertJsonResponse<{ code: string }>(await proofRequest("attacker-kid-2"), 403);
-  assert.equal(requestCount("/.well-known/openid-configuration"), 1);
+  for (let attempt = 1; attempt <= 5; attempt++) {
+    await assertJsonResponse<{ code: string }>(await proofRequest(`attacker-kid-${attempt}`), 403);
+  }
+  // The cooldown admits one forced refresh per issuer however many unknown
+  // kids arrive, and max-age=0 cannot drive a fetch per request because the
+  // parsed cache lifetime has a floor. That single refresh re-resolves the
+  // discovery document as well as the JWKS, so the ceiling is one extra fetch
+  // of each per window rather than one of each per request.
+  assert.equal(requestCount("/.well-known/openid-configuration"), 2);
   assert.equal(requestCount("/jwks"), 2);
 });
 
+test("a provider that rotates its signing key and jwks_uri together stays reachable", async (t) => {
+  const { issuer, rotate } = await startRotatingOidcFixture(t);
+  const org = "org_oidc_jwks_uri_rotation";
+  const app = createTestApp({
+    RELAYAUTH_SPONSOR_FEDERATIONS: JSON.stringify({
+      [org]: { sponsorBinding: "oidc", issuer, clientId: "chief-fixture" },
+    }),
+  });
+  const apiKey = await createWorkspaceApiKey(app, org);
+  const now = Math.floor(Date.now() / 1000);
+  const proofRequest = (kid: string) => app.request(
+    createTestRequest(
+      "POST",
+      "/v1/sponsors/proof",
+      {
+        idToken: signIdToken({
+          iss: issuer,
+          sub: "alice",
+          aud: "chief-fixture",
+          iat: now,
+          exp: now + 300,
+        }, kid),
+        intent: "approval",
+      },
+      { "x-api-key": apiKey },
+    ),
+    undefined,
+    app.bindings,
+  );
+
+  await assertJsonResponse<SponsorProof>(await proofRequest(OIDC_KID), 201);
+  rotate();
+  // The unknown kid drives the one permitted forced refresh. That refresh has
+  // to re-resolve discovery too: resolving only the JWKS would refetch the
+  // retired jwks_uri, so the rotated key would stay unreachable and every
+  // sponsor proof for this org would fail until the discovery cache expired.
+  await assertJsonResponse<SponsorProof>(await proofRequest(ROTATED_OIDC_KID), 201);
+});
+
 test("OIDC subject mapping is collision-free for raw and encoded-looking values", async (t) => {
   const { issuer } = await startOidcFixture(t);
   const org = "org_oidc_subject_encoding";

packages/server/src/lib/sponsor-binding.ts

@@ -413,9 +413,12 @@ export class SponsorOidcService {
       return validateJwks(config.jwks);
     }
 
-    // Key rotation refreshes the JWKS itself; the separately cached discovery
-    // document must not be refetched for every attacker-controlled unknown kid.
-    const jwksUri = config.jwksUri ?? await this.#resolveJwksUri(config, false);
+    // A provider can rotate its signing keys and its jwks_uri together, so the
+    // forced refresh must be able to re-resolve discovery too — otherwise the
+    // rotated key is fetched from the stale URI and never found. This path is
+    // reachable only behind #canForceRefresh, which caps an attacker-controlled
+    // unknown kid at one discovery + one JWKS fetch per issuer per cooldown.
+    const jwksUri = config.jwksUri ?? await this.#resolveJwksUri(config, forceRefresh);
     assertSecureProviderUrl(jwksUri, "jwksUri");
     const cached = this.#jwksCache.get(jwksUri);
     if (!forceRefresh && cached && cached.expiresAt > Date.now()) {

packages/server/src/routes/attestations.ts

@@ -5,7 +5,7 @@ import { matchScope } from "@relayauth/sdk";
 import type { AppEnv } from "../env.js";
 import { authenticateAndAuthorizeFromContext } from "../lib/auth.js";
 import { rsaPublicJwkFromPem } from "../lib/jwk.js";
-import { keyIdFromPublicJwk, signCanonicalRs256 } from "../lib/sign-rs256.js";
+import { importRsaPrivateKey, keyIdFromPublicJwk, signCanonicalRs256 } from "../lib/sign-rs256.js";
 import type {
   AppendAttestationLedgerEntryInput,
   AttestationGrant,
@@ -30,6 +30,8 @@ type FinalizeCommit = {
   sha: string;
 };
 
+type LedgerSigner = (payload: Record<string, unknown>) => Promise<string>;
+
 const MAX_GRANT_TTL_SECONDS = 24 * 60 * 60;
 const DEFAULT_GRANT_TTL_SECONDS = 60 * 60;
 const MAX_FINALIZE_COMMITS = 100;
@@ -122,7 +124,8 @@ attestations.post("/grants", async (c) => {
     ts: grant.createdAt,
     ...(grant.taskRef ? { taskRef: grant.taskRef } : {}),
   };
-  const jws = await signLedgerPayload(c, payload);
+  const signLedgerPayload = await createLedgerSigner(c);
+  const jws = await signLedgerPayload(payload);
   const ledgerEntry: AppendAttestationLedgerEntryInput = {
     orgId: grant.orgId,
     entryType: late ? "attestation.late" : "attestation.granted",
@@ -184,6 +187,7 @@ attestations.post("/finalize", async (c) => {
   }
 
   const ts = new Date().toISOString();
+  const signLedgerPayload = await createLedgerSigner(c);
   const ledgerEntries: AppendAttestationLedgerEntryInput[] = [];
   const responseAttestations: Array<{ sha: string; jws: string }> = [];
   for (const commit of commits) {
@@ -196,7 +200,7 @@ attestations.post("/finalize", async (c) => {
       sponsorId: grant.sponsorId,
       ts,
     };
-    const jws = await signLedgerPayload(c, payload);
+    const jws = await signLedgerPayload(payload);
     ledgerEntries.push({
       orgId: grant.orgId,
       entryType: grant.late ? "attestation.late" : "attestation.issued",
@@ -228,16 +232,24 @@ attestations.post("/finalize", async (c) => {
 
 export default attestations;
 
-async function signLedgerPayload(c: Context<AppEnv>, payload: Record<string, unknown>): Promise<string> {
-  const privateKey = c.env.RELAYAUTH_SIGNING_KEY_PEM?.trim();
-  if (!privateKey) {
+/**
+ * Import the signing key and derive its `kid` once per request.
+ *
+ * Finalize signs up to MAX_FINALIZE_COMMITS payloads. Resolving the key inside
+ * that loop would repeat the PKCS#8 import and the JWK thumbprint digest for
+ * every commit, on the request thread, for no change in output.
+ */
+async function createLedgerSigner(c: Context<AppEnv>): Promise<LedgerSigner> {
+  const privateKeyPem = c.env.RELAYAUTH_SIGNING_KEY_PEM?.trim();
+  if (!privateKeyPem) {
     throw new Error("RELAYAUTH_SIGNING_KEY_PEM must be set");
   }
   const publicKey = c.env.RELAYAUTH_SIGNING_KEY_PEM_PUBLIC?.trim();
   const kid = publicKey
     ? await keyIdFromPublicJwk(await rsaPublicJwkFromPem(publicKey, ""))
     : "rs256-key";
-  return signCanonicalRs256(payload, privateKey, kid);
+  const privateKey = await importRsaPrivateKey(privateKeyPem);
+  return (payload) => signCanonicalRs256(payload, privateKey, kid);
 }
 
 async function resolveWorkspaceToken(