fix(issuer): validate the credential id before it reaches the query

examples/issuer/src/lib/utils/parse-id-param.test.ts

@@ -0,0 +1,34 @@
+import { describe, expect, it } from "vitest"
+
+import { parseIdParam } from "./parse-id-param"
+
+describe("parseIdParam", () => {
+  it.each([
+    ["0", 0],
+    ["1", 1],
+    // A leading zero names the same row, and the parsed number is what the
+    // caller goes on to use, so the id it builds is the canonical one.
+    ["01", 1],
+    ["8192", 8192],
+  ])("reads '%s' as %i", (value, expected) => {
+    expect(parseIdParam(value)).toBe(expected)
+  })
+
+  it.each([
+    "abc",
+    "1abc",
+    // `parseInt` reads these as 1: hexadecimal, exponent and decimal notation
+    // all stop at the first character it cannot use.
+    "0x1",
+    "1e3",
+    "1.9",
+    "-1",
+    " 1",
+    "1 ",
+    "",
+    // Parses to `1e+21`, which is neither safe nor an integer.
+    "999999999999999999999",
+  ])("rejects '%s'", (value) => {
+    expect(parseIdParam(value)).toBeUndefined()
+  })
+})

examples/issuer/src/lib/utils/parse-id-param.ts

@@ -0,0 +1,27 @@
+import * as v from "valibot"
+
+const idParamSchema = v.pipe(
+  v.string(),
+  v.regex(/^\d+$/),
+  v.transform(Number),
+  // A long digit string parses to an unsafe integer or `Infinity`, which would
+  // reach the query.
+  v.safeInteger(),
+  v.minValue(0),
+)
+
+/**
+ * Parse a row id out of a URL parameter.
+ *
+ * `parseInt` maps a parameter with no leading digits to `NaN`, which reaches
+ * the query and fails it, and it stops at the first character it cannot read,
+ * so `/1abc`, `/0x1` and `/1e3` all select row 1.
+ *
+ * @param value - The URL parameter to parse
+ * @returns The id, or `undefined` if the parameter is not one
+ */
+export function parseIdParam(value: string): number | undefined {
+  const result = v.safeParse(idParamSchema, value)
+
+  return result.success ? result.output : undefined
+}

examples/issuer/src/routes/credentials.test.ts

@@ -335,6 +335,46 @@ describe("POST /credentials/controller", () => {
   })
 })
 
+describe("GET /credentials/controller/:id", () => {
+  let issuer: DidWithSigner
+
+  beforeAll(async () => {
+    issuer = await createDidWebWithSigner("https://issuer.example.com")
+
+    process.env.ISSUER_PRIVATE_KEY = bytesToHexString(issuer.keypair.privateKey)
+    process.env.BASE_URL = "https://issuer.example.com"
+  })
+
+  afterEach(() => {
+    vi.clearAllMocks()
+  })
+
+  function get(id: string) {
+    const resolver = new DidResolver()
+    resolver.addToCache(issuer.did, issuer.didDocument)
+    vi.mocked(getDidResolver).mockReturnValue(resolver)
+
+    return app.request(`/credentials/controller/${id}`)
+  }
+
+  it("looks up the credential the id names", async () => {
+    const res = await get("1")
+
+    expect(res.status).toBe(200)
+    expect(vi.mocked(getCredential)).toHaveBeenCalledWith(expect.anything(), 1)
+  })
+
+  it.each(["abc", "1abc", "0x1", "1e3", "1.9", "999999999999999999999"])(
+    "responds 404 to '%s' without querying for it",
+    async (id) => {
+      const res = await get(id)
+
+      expect(res.status).toBe(404)
+      expect(vi.mocked(getCredential)).not.toHaveBeenCalled()
+    },
+  )
+})
+
 describe("DELETE /credentials/controller", () => {
   let controller: DidWithSigner
   let signedPayload: string

examples/issuer/src/routes/credentials.ts

@@ -25,6 +25,7 @@ import {
 } from "@/db/queries/credentials"
 import { buildSignedCredential } from "@/lib/credentials/build-signed-credential"
 import type { CredentialResponse } from "@/lib/types"
+import { parseIdParam } from "@/lib/utils/parse-id-param"
 import { database } from "@/middleware/database"
 import { didResolver } from "@/middleware/did-resolver"
 import { issuer as issuerMiddleware } from "@/middleware/issuer"
@@ -120,13 +121,18 @@ app.post(
  * }
  */
 app.get("/:id", async (c): Promise<ApiResponse<CredentialResponse>> => {
-  const { id } = c.req.param()
   const db = c.get("db")
   const issuer = c.get("issuer")
   const resolver = c.get("resolver")
   const { BASE_URL } = env(c)
 
-  const credential = await getCredential(db, parseInt(id))
+  const id = parseIdParam(c.req.param("id"))
+
+  if (id === undefined) {
+    return notFound("Credential not found")
+  }
+
+  const credential = await getCredential(db, id)
 
   if (!credential) {
     return notFound("Credential not found")

examples/issuer/src/routes/receipts.test.ts

@@ -424,3 +424,38 @@ describe("DELETE /credentials/receipts", () => {
     })
   })
 })
+
+describe("GET /credentials/receipts/:id", () => {
+  beforeEach(async () => {
+    const resolver = new DidResolver()
+    vi.mocked(getDidResolver).mockReturnValue(resolver)
+
+    const issuer = await createDidWebWithSigner("https://issuer.example.com", {
+      resolver,
+    })
+
+    process.env.ISSUER_PRIVATE_KEY = bytesToHexString(issuer.keypair.privateKey)
+    process.env.BASE_URL = "https://issuer.example.com"
+  })
+
+  afterEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it("looks up the credential the id names", async () => {
+    const res = await app.request("/credentials/receipts/1")
+
+    expect(res.status).toBe(200)
+    expect(vi.mocked(getCredential)).toHaveBeenCalledWith(expect.anything(), 1)
+  })
+
+  it.each(["abc", "1abc", "0x1", "1e3", "1.9", "999999999999999999999"])(
+    "responds 404 to '%s' without querying for it",
+    async (id) => {
+      const res = await app.request(`/credentials/receipts/${id}`)
+
+      expect(res.status).toBe(404)
+      expect(vi.mocked(getCredential)).not.toHaveBeenCalled()
+    },
+  )
+})

examples/issuer/src/routes/receipts.ts

@@ -27,6 +27,7 @@ import {
 } from "@/db/queries/credentials"
 import { buildSignedCredential } from "@/lib/credentials/build-signed-credential"
 import type { CredentialResponse } from "@/lib/types"
+import { parseIdParam } from "@/lib/utils/parse-id-param"
 import { database } from "@/middleware/database"
 import { didResolver } from "@/middleware/did-resolver"
 import { issuer as issuerMiddleware } from "@/middleware/issuer"
@@ -156,12 +157,18 @@ export default app
  * }
  */
 app.get("/:id", async (c): Promise<ApiResponse<CredentialResponse>> => {
-  const { id } = c.req.param()
   const db = c.get("db")
   const issuer = c.get("issuer")
   const resolver = c.get("resolver")
   const { BASE_URL } = env(c)
-  const credential = await getCredential(db, parseInt(id))
+
+  const id = parseIdParam(c.req.param("id"))
+
+  if (id === undefined) {
+    return notFound("Credential not found")
+  }
+
+  const credential = await getCredential(db, id)
 
   if (!credential) {
     return notFound("Credential not found")

examples/issuer/src/routes/status.ts

@@ -13,6 +13,7 @@ import * as v from "valibot"
 
 import { getStatusList } from "@/db/queries/status-lists"
 import { compressBitString } from "@/lib/utils/compress-bit-string"
+import { parseIdParam } from "@/lib/utils/parse-id-param"
 import { database } from "@/middleware/database"
 import { didResolver } from "@/middleware/did-resolver"
 import { issuer as issuerMiddleware } from "@/middleware/issuer"
@@ -51,26 +52,13 @@ app.get(
     // Parse the id before it reaches either the query or the credential id, so
     // `/status/01` and `/status/1abc` cannot sign caller-supplied text into the
     // credential id while selecting the same row.
-    const listId = v.safeParse(
-      v.pipe(
-        v.string(),
-        v.regex(/^\d+$/),
-        v.transform(Number),
-        // A long digit string parses to an unsafe integer or `Infinity`, which
-        // would reach the query.
-        v.safeInteger(),
-        // Status list ids are zero-based: `getStatusListPosition` puts the
-        // first 8192 credentials on list 0.
-        v.minValue(0),
-      ),
-      c.req.param("listId"),
-    )
+    const listId = parseIdParam(c.req.param("listId"))
 
-    if (!listId.success) {
+    if (listId === undefined) {
       return notFound("Status list not found")
     }
 
-    const statusList = await getStatusList(db, listId.output)
+    const statusList = await getStatusList(db, listId)
 
     if (!statusList) {
       return notFound("Status list not found")
@@ -79,7 +67,7 @@ app.get(
     const encodedList = compressBitString(statusList.data)
 
     const credential = createStatusListCredential({
-      url: `${BASE_URL}/status/${listId.output}`,
+      url: `${BASE_URL}/status/${listId}`,
       encodedList,
       issuer: issuer.did,
     })