diff --git a/src/components/features/service-accounts/ApiKeyList.stories.tsx b/src/components/features/service-accounts/ApiKeyList.stories.tsx index 51d26037..542d0c02 100644 --- a/src/components/features/service-accounts/ApiKeyList.stories.tsx +++ b/src/components/features/service-accounts/ApiKeyList.stories.tsx @@ -4,11 +4,11 @@ import type { ServiceAccountKey } from "@/types"; /** * A service account's API keys, as its page lists them. Each row gives the - * key's label and its last four characters (`sck_…Xy9Q`) — enough to match a - * key in someone's environment to its record — and, to the right, how it has - * been used and when it ends. Hover those two lines for the exact dates and - * who issued the key. A live key's "⋯" menu changes its expiry or revokes it; - * a revoked key has nothing left to do. + * key's label and its last six characters (`sck_…Xy9QeT`), which are its + * checksum — enough to match a key in someone's environment to its record — + * and, to the right, how it has been used and when it ends. Hover those two + * lines for the exact dates and who issued the key. A live key's "⋯" menu + * changes its expiry or revokes it; a revoked key has nothing left to do. * * The actions are mocked in `.storybook/preview.tsx`. Dates here are set * relative to today, so the wording reads the same whenever the story is @@ -31,7 +31,7 @@ const key = (overrides: Partial): ServiceAccountKey => ({ key_id: "6f1c2a3b-4d5e-4f60-8a9b-0c1d2e3f4a5b", account_id: "miskatonic--nightly-sync", label: "HPC cron job", - hint: "Xy9Q", + hint: "Xy9QeT", created_at: at(-200), created_by: "acoltrane", expires_at: at(160), @@ -42,11 +42,11 @@ const key = (overrides: Partial): ServiceAccountKey => ({ export const Default: Story = { args: { keys: [ - key({ key_id: "k1", label: "HPC cron job", hint: "Xy9Q", last_used_at: at(-3) }), - key({ key_id: "k2", label: "Instrument uploader", hint: "m2Rd", expires_at: null, last_used_at: at(0) }), - key({ key_id: "k3", label: "Laptop, for testing", hint: "Q8_z", created_at: at(-1), expires_at: at(30) }), - key({ key_id: "k4", label: "Last year's sync", hint: "t0pA", created_at: at(-400), expires_at: at(-35), last_used_at: at(-40) }), - key({ key_id: "k5", label: "Old laptop", hint: "a_7k", created_at: at(-300), expires_at: null, revoked_at: at(-270) }), + key({ key_id: "k1", label: "HPC cron job", hint: "Xy9QeT", last_used_at: at(-3) }), + key({ key_id: "k2", label: "Instrument uploader", hint: "m2RdK7", expires_at: null, last_used_at: at(0) }), + key({ key_id: "k3", label: "Laptop, for testing", hint: "Q8vz0a", created_at: at(-1), expires_at: at(30) }), + key({ key_id: "k4", label: "Last year's sync", hint: "t0pAw3", created_at: at(-400), expires_at: at(-35), last_used_at: at(-40) }), + key({ key_id: "k5", label: "Old laptop", hint: "a07kT2", created_at: at(-300), expires_at: null, revoked_at: at(-270) }), ], }, }; @@ -57,8 +57,8 @@ export const Single: Story = { }; /** - * A key issued before keys kept their last four characters: listed without a - * hint, rather than with an empty one. + * A key issued before keys kept a hint of their last characters: listed + * without one, rather than with an empty one. */ export const WithoutHint: Story = { args: { keys: [key({ hint: undefined, last_used_at: at(-12) })] }, diff --git a/src/components/features/service-accounts/IssueApiKeyDialog.stories.tsx b/src/components/features/service-accounts/IssueApiKeyDialog.stories.tsx index 00eefcb6..2480808a 100644 --- a/src/components/features/service-accounts/IssueApiKeyDialog.stories.tsx +++ b/src/components/features/service-accounts/IssueApiKeyDialog.stories.tsx @@ -7,7 +7,7 @@ import { IssueApiKeyDialog } from "./IssueApiKeyDialog"; * any AWS SDK or the AWS CLI at it. * * `issueApiKey` is mocked in `.storybook/preview.tsx` and resolves with a key - * of the real shape, `sck_` and 43 characters of nothing secret, so + * of the real shape, `sck_` and 36 characters of nothing secret, so * **submitting the form shows the show-once view**. Open the dialog, give it * a label, and issue. */ diff --git a/src/components/features/service-accounts/IssueApiKeyDialog.tsx b/src/components/features/service-accounts/IssueApiKeyDialog.tsx index 14b99140..04678087 100644 --- a/src/components/features/service-accounts/IssueApiKeyDialog.tsx +++ b/src/components/features/service-accounts/IssueApiKeyDialog.tsx @@ -70,7 +70,7 @@ export function IssueApiKeyDialog({ Listed as {maskedApiKey(state.issued.record)} from now - on: its last four characters, to match against the key you hold. + on: its last six characters, to match against the key you hold. {environment && ( diff --git a/src/components/features/service-accounts/ServiceAccountDetail.stories.tsx b/src/components/features/service-accounts/ServiceAccountDetail.stories.tsx index 08cfb761..0d0c64f5 100644 --- a/src/components/features/service-accounts/ServiceAccountDetail.stories.tsx +++ b/src/components/features/service-accounts/ServiceAccountDetail.stories.tsx @@ -85,7 +85,7 @@ const summary: ServiceAccountSummary = { key_id: "k1", account_id: "nightly-sync", label: "HPC cron job", - hint: "Xy9Q", + hint: "Xy9QeT", created_at: "2026-03-12T00:00:00Z", created_by: "acoltrane", expires_at: "2027-03-12T00:00:00Z", @@ -95,7 +95,7 @@ const summary: ServiceAccountSummary = { key_id: "k2", account_id: "nightly-sync", label: "Old laptop", - hint: "a_7k", + hint: "a07kT2", created_at: "2025-03-12T00:00:00Z", created_by: "acoltrane", expires_at: null, diff --git a/src/components/features/service-accounts/ServiceAccountList.stories.tsx b/src/components/features/service-accounts/ServiceAccountList.stories.tsx index 4a0b6490..58dc7fb7 100644 --- a/src/components/features/service-accounts/ServiceAccountList.stories.tsx +++ b/src/components/features/service-accounts/ServiceAccountList.stories.tsx @@ -66,7 +66,7 @@ const summary = ( key_id: `k1-${account_id}`, account_id, label: "HPC cron job", - hint: "Xy9Q", + hint: "Xy9QeT", created_at: "2026-03-12T00:00:00Z", created_by: "acoltrane", expires_at: "2027-03-12T00:00:00Z", @@ -76,7 +76,7 @@ const summary = ( key_id: `k2-${account_id}`, account_id, label: "Old laptop", - hint: "a_7k", + hint: "a07kT2", created_at: "2025-03-12T00:00:00Z", created_by: "acoltrane", expires_at: null, diff --git a/src/lib/actions/__mocks__/service-account-keys.ts b/src/lib/actions/__mocks__/service-account-keys.ts index 61f375ba..6e2234fb 100644 --- a/src/lib/actions/__mocks__/service-account-keys.ts +++ b/src/lib/actions/__mocks__/service-account-keys.ts @@ -1,26 +1,28 @@ import { fn } from "storybook/test"; import type * as Real from "../service-account-keys"; -import type { ApiKeyActionState } from "@/types"; +import { apiKeyChecksum, type ApiKeyActionState } from "@/types"; /** * Storybook stand-in for the API-key server actions, redirected to by * `sb.mock()` in `.storybook/preview.tsx`. `issueApiKey` resolves with a key * of the real shape, so the show-once view is reachable by submitting the - * dialog. + * dialog. The key is assembled at run time so that secret scanners don't flag + * this file. */ const idle = (): ApiKeyActionState => ({ message: "", success: false }); +const FIXTURE_BODY = "storyFixtureNotARealKey1234567"; export const issueApiKey: typeof Real.issueApiKey = fn( async (_prev, formData): Promise => ({ message: "", success: true, issued: { - key: "sck_storyFixtureNotARealKey0123456789abcdefghij", + key: `sck_${FIXTURE_BODY}${apiKeyChecksum(FIXTURE_BODY)}`, record: { key_id: "6f1c2a3b-4d5e-4f60-8a9b-0c1d2e3f4a5b", account_id: "nightly-sync", label: String(formData.get("label") || "CI"), - hint: "ghij", + hint: apiKeyChecksum(FIXTURE_BODY), created_at: "2026-03-12T00:00:00Z", created_by: "acoltrane", expires_at: null, diff --git a/src/lib/actions/service-account-keys.test.ts b/src/lib/actions/service-account-keys.test.ts index 52071dfb..6d0147bc 100644 --- a/src/lib/actions/service-account-keys.test.ts +++ b/src/lib/actions/service-account-keys.test.ts @@ -3,7 +3,7 @@ import { issueApiKey, revokeApiKey, setApiKeyExpiry } from "./service-account-ke import { serviceAccountKeysTable } from "../clients"; import { getPageSession } from "../api/utils"; import { managedServiceAccount } from "@/lib/accounts/service-accounts"; -import { API_KEY_PATTERN, AccountType, type Account, type ApiKeyActionState, type UserSession } from "@/types"; +import { isApiKey, AccountType, type Account, type ApiKeyActionState, type UserSession } from "@/types"; jest.mock("../clients", () => ({ serviceAccountKeysTable: { create: jest.fn(), listByAccount: jest.fn(), set: jest.fn() }, @@ -44,12 +44,12 @@ describe("issueApiKey", () => { const result = await issueApiKey(IDLE, form({ account_id: "acme--nightly-sync", label: "HPC", expires_in_days: "90" })); expect(result.success).toBe(true); const key = result.issued!.key; - expect(key).toMatch(API_KEY_PATTERN); + expect(isApiKey(key)).toBe(true); const stored = keys.create.mock.calls[0][0]; expect(stored).toMatchObject({ key_hash: sha256(key), account_id: "acme--nightly-sync", label: "HPC", created_by: "alice" }); - // The last four characters, and only those, so the key can be recognised later. - expect(stored.hint).toBe(key.slice(-4)); - expect(result.issued!.record.hint).toBe(key.slice(-4)); + // The checksum, the last six characters and only those, so the key can be recognised later. + expect(stored.hint).toBe(key.slice(-6)); + expect(result.issued!.record.hint).toBe(key.slice(-6)); expect(stored.expires_at).not.toBeNull(); expect(JSON.stringify(stored)).not.toContain(key); // The record handed back is the public one: no hash, and the same handle. diff --git a/src/lib/actions/service-account-keys.ts b/src/lib/actions/service-account-keys.ts index 28498cef..46818ae0 100644 --- a/src/lib/actions/service-account-keys.ts +++ b/src/lib/actions/service-account-keys.ts @@ -1,10 +1,12 @@ "use server"; import { revalidatePath } from "next/cache"; -import { createHash, randomBytes, randomUUID } from "crypto"; +import { createHash, randomInt, randomUUID } from "crypto"; import { LOGGER } from "@/lib/logging"; import { + API_KEY_ALPHABET, API_KEY_PREFIX, + apiKeyChecksum, publicKey, ServiceAccountKeyRecordSchema, type ApiKeyActionState, @@ -51,15 +53,16 @@ export async function issueApiKey( const expires_at = expiryFrom(formData); if (expires_at === undefined) return outcome("Expiry must be between 1 and 3650 days", false); - // 32 random bytes in base64url: fixed length, no bias, all entropy. - const key = API_KEY_PREFIX + randomBytes(32).toString("base64url"); + // 30 characters drawn uniformly from base62 (178 bits), then their checksum. + const body = Array.from({ length: 30 }, () => API_KEY_ALPHABET[randomInt(62)]).join(""); + const key = API_KEY_PREFIX + body + apiKeyChecksum(body); const now = new Date().toISOString(); const parsed = ServiceAccountKeyRecordSchema.safeParse({ key_hash: hashApiKey(key), key_id: randomUUID(), account_id: account.account_id, label: String(formData.get("label") ?? "").trim(), - hint: key.slice(-4), + hint: key.slice(-6), created_at: now, created_by: session.account.account_id, expires_at, diff --git a/src/types/service-account-key.test.ts b/src/types/service-account-key.test.ts new file mode 100644 index 00000000..64496371 --- /dev/null +++ b/src/types/service-account-key.test.ts @@ -0,0 +1,56 @@ +import { apiKeyChecksum, isApiKey, ServiceAccountKeySchema } from "./service-account-key"; + +// Checksums computed independently with Python's zlib.crc32. Keys are +// assembled at run time so that secret scanners don't flag this file. +const key = (body: string, checksum: string) => `sck_${body}${checksum}`; +const GOOD = key("a".repeat(30), "1yLcDB"); + +describe("apiKeyChecksum", () => { + it("is zlib's CRC-32 of the body in six base62 digits", () => { + expect(apiKeyChecksum("a".repeat(30))).toBe("1yLcDB"); + // A CRC above 2^31, which a signed 32-bit shift would get wrong. + expect(apiKeyChecksum("0123456789ABCDEFGHIJabcdefghij")).toBe("4Us3aw"); + // A CRC below 62^5, whose checksum keeps its leading zero. + expect(apiKeyChecksum("0".repeat(29) + "1")).toBe("010Ohw"); + }); +}); + +describe("isApiKey", () => { + it("accepts a key whose checksum holds", () => { + expect(isApiKey(GOOD)).toBe(true); + }); + + it.each([ + ["a key cut short", GOOD.slice(0, -1)], + ["a key with a character too many", `${GOOD}a`], + ["a mistyped character", GOOD.replace("sck_a", "sck_b")], + ["a mistyped checksum", `${GOOD.slice(0, -1)}C`], + ["the wrong case", GOOD.replace("sck_", "SCK_")], + ["a character outside base62", key(`${"a".repeat(29)}-`, "1yLcDB")], + ["the checksum-less 47-character format", `sck_${"a".repeat(43)}`], + ["surrounding whitespace, which callers trim", ` ${GOOD}\n`], + ])("refuses %s", (_, token) => { + expect(isApiKey(token)).toBe(false); + }); +}); + +describe("hint", () => { + const record = (hint: string) => + ServiceAccountKeySchema.safeParse({ + key_id: "6f1c2a3b-4d5e-4f60-8a9b-0c1d2e3f4a5b", + account_id: "acme--nightly-sync", + label: "CI", + hint, + created_at: "2026-09-29T00:00:00Z", + created_by: "alice", + expires_at: null, + }).success; + + it("is the checksum, or the four characters recorded before keys had one", () => { + expect(record("1yLcDB")).toBe(true); + expect(record("Xy9Q")).toBe(true); + expect(record("a_7k")).toBe(true); + expect(record("1yLcD")).toBe(false); + expect(record("1yLc-B")).toBe(false); + }); +}); diff --git a/src/types/service-account-key.ts b/src/types/service-account-key.ts index f801ae19..89a31908 100644 --- a/src/types/service-account-key.ts +++ b/src/types/service-account-key.ts @@ -14,13 +14,17 @@ export const ServiceAccountKeySchema = z account_id: z.string(), label: z.string().min(1).max(64), /** - * The key's last four characters, to tell which key is which: 24 of its - * 256 random bits, leaving far too many to guess. It confirms a key in - * hand against this record, and is never used to look one up — across - * the platform, keys will share it. Absent on keys issued before it - * was recorded. + * The key's last six characters — its checksum — to tell which key is + * which. A checksum of the key's 178 random bits narrows them by 32, + * leaving far too many to guess. It confirms a key in hand against this + * record, and is never used to look one up, since keys may share it. + * Four characters on keys issued before keys carried a checksum; absent + * on keys issued before it was recorded. */ - hint: z.string().regex(/^[A-Za-z0-9_-]{4}$/).optional(), + hint: z + .string() + .regex(/^([0-9A-Za-z]{6}|[A-Za-z0-9_-]{4})$/) + .optional(), created_at: z.string().datetime(), created_by: z.string(), /** Null for a key that lasts until revoked. */ @@ -47,13 +51,38 @@ export type ServiceAccountKeyRecord = z.infer key; /** - * Every key is `sck_` + 32 random bytes in base64url: a fixed 47 characters, - * all entropy after the prefix. The pattern is what secret scanners register. + * Every key is `sck_`, 30 random base62 characters, and six more that are + * their checksum: a fixed 40 characters, GitHub's own token layout (ADR-013). + * The pattern is what secret scanners register; the checksum lets anything + * holding a key refuse one that was cut short or mistyped without a lookup. */ export const API_KEY_PREFIX = "sck_"; -export const API_KEY_PATTERN = /^sck_[A-Za-z0-9_-]{43}$/; +export const API_KEY_PATTERN = /^sck_[0-9A-Za-z]{36}$/; +export const API_KEY_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; -/** How a key is shown once it is no longer in hand: `sck_…Xy9Q`, or null without a hint. */ +/** + * The six characters that end a key: the CRC-32 of its 30 random characters + * (IEEE, as zlib computes it), in base62, most significant digit first. It is + * computed here rather than with `zlib` because this module is also bundled + * for the browser. + */ +export function apiKeyChecksum(body: string): string { + let crc = ~0; + for (let i = 0; i < body.length; i++) { + crc ^= body.charCodeAt(i); + for (let bit = 0; bit < 8; bit++) crc = crc & 1 ? (crc >>> 1) ^ 0xedb88320 : crc >>> 1; + } + let n = ~crc >>> 0; + let digits = ""; + for (let i = 0; i < 6; i++, n = Math.floor(n / 62)) digits = API_KEY_ALPHABET[n % 62] + digits; + return digits; +} + +/** Whether a string is exactly a key: the pattern, and a checksum that holds. */ +export const isApiKey = (token: string) => + API_KEY_PATTERN.test(token) && token.slice(-6) === apiKeyChecksum(token.slice(4, 34)); + +/** How a key is shown once it is no longer in hand: `sck_…Xy9QeT`, or null without a hint. */ export const maskedApiKey = (key: Pick) => key.hint ? `${API_KEY_PREFIX}…${key.hint}` : null;