From e201517739b03c682f54d91ff0ae809b0c73e875 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Mon, 8 Jun 2026 07:11:13 -0700 Subject: [PATCH] test(browser): close mutation-testing coverage gaps in passkeys SDK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StrykerJS mutation testing — added by Andrew Bernat (@bernata) in PR #39 (https://github.com/codeheadsystems/pk-auth/pull/39) — reported a 65.1% mutation score for the browser SDK and pinpointed real test gaps that plain line coverage hid: code that ran but had no assertion pinning its behaviour, so a mutated version still passed. This adds the missing assertions. Why this matters: these guards, header rules, and error-mapping branches are the SDK's public contract with the server. A test that exercises them without asserting on them gives false confidence — exactly what mutation testing flags. Credit to Andrew for wiring up the tooling that surfaced these. Gaps closed (each new case kills a previously-surviving mutant): - results.ts: had ZERO coverage (0% score). New results.test.ts pins both branches of isCeremonySuccess / isCeremonyFailure and the success narrowing. - http.ts: assert the `accept` header is always sent; content-type/body are omitted on body-less requests; JSON `null`/primitive bodies leave PkAuthHttpError.data undefined (the && vs || object check); and an absent getToken yields the friendly validation error, not a raw TypeError. - refresh.ts: assert the negative branch of both type guards, the default /auth/refresh path, and that a non-JSON 401 body maps to "unknown" instead of throwing (the e.data?.detail optional-chaining branch). - admin.ts: cover the two previously-untested methods (completeEmailVerification, startPhoneVerification) and add a matrix that asserts every admin call carries the Bearer token, so the `authenticated` flag can no longer be mutated to false undetected. Note: a handful of remaining base64url survivors are equivalent mutants (out-of-bounds typed-array writes are ignored, atob is lenient, the padding regex anchor is moot for btoa output) and are intentionally left as-is. All 67 browser-SDK tests pass; tsc --noEmit clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- clients/passkeys-browser/test/admin.test.ts | 77 +++++++++++++++++++ clients/passkeys-browser/test/http.test.ts | 67 ++++++++++++++++ clients/passkeys-browser/test/refresh.test.ts | 41 ++++++++++ clients/passkeys-browser/test/results.test.ts | 72 +++++++++++++++++ 4 files changed, 257 insertions(+) create mode 100644 clients/passkeys-browser/test/results.test.ts diff --git a/clients/passkeys-browser/test/admin.test.ts b/clients/passkeys-browser/test/admin.test.ts index 03bef29..a670c18 100644 --- a/clients/passkeys-browser/test/admin.test.ts +++ b/clients/passkeys-browser/test/admin.test.ts @@ -165,4 +165,81 @@ describe("PkAuthAdminClient", () => { const r = await c.completePhoneVerification("+1555", "123456"); expect(r.verified).toBe(true); }); + + // Gaps surfaced by StrykerJS mutation testing (PR #39, @bernata): + // - completeEmailVerification / startPhoneVerification had no coverage at all + // (every mutant in them survived because no test executed them); + // - the per-method tests above assert path + verb but (except getAccount) + // never assert the Authorization header, so the `authenticated` argument + // (the literal `true` on each request() call) could be mutated to `false` + // and survive. Driving every method through one bearer-asserting matrix + // closes both holes. + it("completeEmailVerification POSTs the token", async () => { + const fetchImpl = stubFetch({ + "/auth/admin/email/complete-verification": (init) => { + expect(init.method).toBe("POST"); + expect(JSON.parse(String(init.body))).toEqual({ token: "verify-tok" }); + return { status: 200, body: JSON.stringify({ email: "a@b.c", verified: true }) }; + }, + }); + const c = new PkAuthAdminClient({ + apiBase: "https://x", + getToken: () => "t", + fetch: fetchImpl as unknown as typeof fetch, + }); + const r = await c.completeEmailVerification("verify-tok"); + expect(r.verified).toBe(true); + }); + + it("startPhoneVerification POSTs the phone", async () => { + const fetchImpl = stubFetch({ + "/auth/admin/phone/start-verification": (init) => { + expect(init.method).toBe("POST"); + expect(JSON.parse(String(init.body))).toEqual({ phone: "+1555" }); + return { status: 200, body: JSON.stringify({ phone: "+1555", dispatched: true }) }; + }, + }); + const c = new PkAuthAdminClient({ + apiBase: "https://x", + getToken: () => "t", + fetch: fetchImpl as unknown as typeof fetch, + }); + await c.startPhoneVerification("+1555"); + expect(fetchImpl).toHaveBeenCalledOnce(); + }); + + it.each([ + ["getAccount", "GET", "/auth/admin/account", (c: PkAuthAdminClient) => c.getAccount()], + ["listCredentials", "GET", "/auth/admin/credentials", (c: PkAuthAdminClient) => c.listCredentials()], + ["renameCredential", "PATCH", "/auth/admin/credentials/abc", (c: PkAuthAdminClient) => c.renameCredential("abc", "L")], + ["removeCredential", "DELETE", "/auth/admin/credentials/abc", (c: PkAuthAdminClient) => c.removeCredential("abc")], + ["regenerateBackupCodes", "POST", "/auth/admin/backup-codes/regenerate", (c: PkAuthAdminClient) => c.regenerateBackupCodes()], + ["remainingBackupCodes", "GET", "/auth/admin/backup-codes/count", (c: PkAuthAdminClient) => c.remainingBackupCodes()], + ["startEmailVerification", "POST", "/auth/admin/email/start-verification", (c: PkAuthAdminClient) => c.startEmailVerification("a@b.c")], + ["completeEmailVerification", "POST", "/auth/admin/email/complete-verification", (c: PkAuthAdminClient) => c.completeEmailVerification("tok")], + ["startPhoneVerification", "POST", "/auth/admin/phone/start-verification", (c: PkAuthAdminClient) => c.startPhoneVerification("+1")], + ["completePhoneVerification", "POST", "/auth/admin/phone/complete-verification", (c: PkAuthAdminClient) => c.completePhoneVerification("+1", "000")], + ] as const)( + "%s calls %s %s as an authenticated (bearer) request", + async (_name, method, path, call) => { + const seen: { method?: string; auth?: string; path?: string } = {}; + const recordingFetch = vi.fn(async (url: RequestInfo | URL, init?: RequestInit) => { + seen.path = String(url).replace(/^https:\/\/x/, ""); + seen.method = init?.method; + seen.auth = (init?.headers as Record | undefined)?.["authorization"]; + return new Response("{}", { status: 200 }); + }); + const c = new PkAuthAdminClient({ + apiBase: "https://x", + getToken: () => "bearer-tok", + fetch: recordingFetch as unknown as typeof fetch, + }); + await call(c); + expect(seen.path).toBe(path); + expect(seen.method).toBe(method); + // The whole point: every admin call must carry the bearer token. If the + // `authenticated` flag is flipped to false, this header disappears. + expect(seen.auth).toBe("Bearer bearer-tok"); + }, + ); }); diff --git a/clients/passkeys-browser/test/http.test.ts b/clients/passkeys-browser/test/http.test.ts index 3920dd5..6d89569 100644 --- a/clients/passkeys-browser/test/http.test.ts +++ b/clients/passkeys-browser/test/http.test.ts @@ -122,4 +122,71 @@ describe("request()", () => { const e = new PkAuthHttpError(400, JSON.stringify([1, 2, 3])); expect(e.data).toBeUndefined(); }); + + // Gaps surfaced by StrykerJS mutation testing (PR #39, @bernata): each case + // below kills a mutant that the existing line-covered tests left alive — the + // code ran, but nothing asserted the behaviour the mutation changed. + it("PkAuthHttpError leaves data undefined for the JSON literal null", () => { + // Kills the `parsed !== null && ...` -> `|| ...` mutant: with `||`, a body + // of "null" (typeof null === "object") would wrongly be treated as data. + const e = new PkAuthHttpError(400, "null"); + expect(e.data).toBeUndefined(); + expect(e.error).toBeUndefined(); + }); + + it("PkAuthHttpError leaves data undefined for a primitive JSON body", () => { + // Kills the "force the object-check true" mutant: a JSON number is valid + // JSON but not a record, so data must stay undefined. + expect(new PkAuthHttpError(400, "5").data).toBeUndefined(); + expect(new PkAuthHttpError(400, '"a string"').data).toBeUndefined(); + }); + + it("always sends an accept: application/json header", async () => { + // Kills the `{ accept: "application/json" }` -> `{}` / "" mutants at the + // header-initialiser: nothing previously asserted the accept header. + const fetchImpl = fakeFetch((_url, init) => { + expect((init.headers as Record)["accept"]).toBe("application/json"); + return { status: 200, body: "{}" }; + }); + await request( + { apiBase: "https://x", fetch: fetchImpl as unknown as typeof fetch }, + "GET", + "/p", + undefined, + false, + ); + expect(fetchImpl).toHaveBeenCalledOnce(); + }); + + it("omits content-type when there is no request body", async () => { + // Kills the `if (body !== undefined)` -> always-true mutant: a body-less + // GET must not advertise a JSON content-type or carry a body. + const fetchImpl = fakeFetch((_url, init) => { + expect((init.headers as Record)["content-type"]).toBeUndefined(); + expect(init.body).toBeUndefined(); + return { status: 200, body: "{}" }; + }); + await request( + { apiBase: "https://x", fetch: fetchImpl as unknown as typeof fetch }, + "GET", + "/p", + undefined, + false, + ); + }); + + it("throws the friendly error (not a TypeError) when getToken is absent", async () => { + // Kills the optional-chaining mutant `options.getToken?.()` -> `getToken()`: + // with no getToken at all, the optional call yields a clean validation + // error; the mutant would throw "getToken is not a function" instead. + await expect( + request( + { apiBase: "https://x" }, + "GET", + "/path", + undefined, + true, + ), + ).rejects.toThrow(/requires getToken/); + }); }); diff --git a/clients/passkeys-browser/test/refresh.test.ts b/clients/passkeys-browser/test/refresh.test.ts index f6538c9..e485292 100644 --- a/clients/passkeys-browser/test/refresh.test.ts +++ b/clients/passkeys-browser/test/refresh.test.ts @@ -81,4 +81,45 @@ describe("PkAuthRefreshClient", () => { const client = new PkAuthRefreshClient({ apiBase: "https://x", fetch: fetchImpl } as never); await expect(client.refresh("old.token")).rejects.toThrow(/HTTP 500/); }); + + // Gaps surfaced by StrykerJS mutation testing (PR #39, @bernata): the suite + // above only pinned the positive branch of each guard / path, so mutations of + // the negative branch survived. Closing them here. + it("type guards reject the opposite variant", () => { + // Kills the `r.kind === "success"` / `=== "failure"` -> always-true mutants: + // a guard that returns true for everything is broken but was untested. + const ok = { kind: "success", accessToken: "a", refreshToken: "r", expiresAt: "t" } as const; + const bad = { kind: "failure", reason: "expired" } as const; + expect(isRefreshSuccess(bad)).toBe(false); + expect(isRefreshFailure(ok)).toBe(false); + }); + + it("posts to the default /auth/refresh path when none is given", async () => { + // Kills the default-parameter mutant `path = "/auth/refresh"` -> `= ""`: + // nothing previously asserted the URL the default client actually calls. + let calledUrl = ""; + const fetchImpl = vi.fn(async (url: RequestInfo | URL) => { + calledUrl = String(url); + return new Response( + JSON.stringify({ refreshToken: "n", accessToken: "a", expiresAt: "t" }), + { status: 200 }, + ); + }); + const client = new PkAuthRefreshClient({ apiBase: "https://x", fetch: fetchImpl } as never); + await client.refresh("old.token"); + expect(calledUrl).toBe("https://x/auth/refresh"); + }); + + it("falls back to 'unknown' (without throwing) on a 401 with a non-JSON body", async () => { + // Kills the optional-chaining mutant `e.data?.detail` -> `e.data.detail`: + // a 401 whose body is not JSON leaves e.data undefined, so the mutant would + // throw a TypeError instead of returning the conservative failure. + const fetchImpl = stubFetch(() => ({ status: 401, body: "Unauthorized" })); + const client = new PkAuthRefreshClient({ apiBase: "https://x", fetch: fetchImpl } as never); + const result = await client.refresh("old.token"); + expect(isRefreshFailure(result)).toBe(true); + if (isRefreshFailure(result)) { + expect(result.reason).toBe("unknown"); + } + }); }); diff --git a/clients/passkeys-browser/test/results.test.ts b/clients/passkeys-browser/test/results.test.ts new file mode 100644 index 0000000..770de70 --- /dev/null +++ b/clients/passkeys-browser/test/results.test.ts @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT + +/** + * Tests for the CeremonyResult discriminated union and its type guards. + * + * Why this file exists: + * `src/results.ts` previously had *zero* test coverage. Mutation testing + * (StrykerJS) — introduced by Andrew Bernat (@bernata) in PR #39 + * (https://github.com/codeheadsystems/pk-auth/pull/39) — surfaced this as a + * 0% mutation-score file: every mutant of `isCeremonySuccess` / + * `isCeremonyFailure` survived because no test ever executed them. Line + * coverage hid the gap; mutation coverage did not. Thanks to Andrew for + * wiring up the tooling that made these blind spots visible. + * + * The guards are the public, documented way for callers to narrow a + * CeremonyResult, so both the positive and the negative branch are pinned + * here (a guard that always returns the same value is useless, and that is + * exactly the mutation Stryker injects). + */ + +import { describe, expect, it } from "vitest"; +import { + type CeremonyResult, + isCeremonyFailure, + isCeremonySuccess, +} from "../src/results"; + +const success: CeremonyResult = { outcome: "success", token: "jwt", credentialId: "cred-1" }; + +const failures: CeremonyResult[] = [ + { outcome: "challenge_invalid", detail: "stale" }, + { outcome: "origin_mismatch" }, + { outcome: "signature_invalid" }, + { outcome: "unknown_credential" }, + { outcome: "attestation_rejected" }, + { outcome: "duplicate_credential" }, + { outcome: "invalid_payload" }, + { outcome: "failed", detail: "boom" }, +]; + +describe("CeremonyResult guards", () => { + it("isCeremonySuccess is true only for the success variant", () => { + expect(isCeremonySuccess(success)).toBe(true); + for (const f of failures) { + expect(isCeremonySuccess(f)).toBe(false); + } + }); + + it("isCeremonyFailure is true for every non-success variant", () => { + expect(isCeremonyFailure(success)).toBe(false); + for (const f of failures) { + expect(isCeremonyFailure(f)).toBe(true); + } + }); + + it("the two guards are exact complements", () => { + for (const r of [success, ...failures]) { + expect(isCeremonySuccess(r)).toBe(!isCeremonyFailure(r)); + } + }); + + it("narrows to the success payload after isCeremonySuccess", () => { + const r: CeremonyResult = success; + if (isCeremonySuccess(r)) { + // Type narrowing must expose token; assert the runtime value too. + expect(r.token).toBe("jwt"); + expect(r.credentialId).toBe("cred-1"); + } else { + throw new Error("expected success to narrow via isCeremonySuccess"); + } + }); +});