From 2d43430405991ccac65275b01811f7e052450aa8 Mon Sep 17 00:00:00 2001 From: lilleman Date: Mon, 15 Jun 2026 10:47:47 +0200 Subject: [PATCH] =?UTF-8?q?Consolidate=20related=20unit=20tests=20(todo=20?= =?UTF-8?q?=C2=A70):=2059=20=E2=86=92=2042=20cases,=20assertions=20preserv?= =?UTF-8?q?ed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/app.test.ts | 31 +++++++++++----------------- src/config.test.ts | 5 +---- src/context.test.ts | 15 +++++--------- src/cookie.test.ts | 43 ++++++++++---------------------------- src/jwt.test.ts | 50 +++++++++++++++++++-------------------------- todo.md | 2 +- 6 files changed, 51 insertions(+), 95 deletions(-) diff --git a/src/app.test.ts b/src/app.test.ts index 6633607..7e0f01a 100644 --- a/src/app.test.ts +++ b/src/app.test.ts @@ -28,10 +28,15 @@ test("serves the home page as HTML", async () => { assert.match(await res.text(), /Plainpages/); }); -test("serves static CSS", async () => { - const res = await fetch(base + "/public/css/style.css"); - assert.equal(res.status, 200); - assert.match(res.headers.get("content-type") ?? "", /text\/css/); +test("serves a static file: GET sends body + content-type, HEAD sends headers only", async () => { + const get = await fetch(base + "/public/css/style.css"); + assert.equal(get.status, 200); + assert.match(get.headers.get("content-type") ?? "", /text\/css/); + + const head = await fetch(base + "/public/css/style.css", { method: "HEAD" }); + assert.equal(head.status, 200); + assert.ok(Number(head.headers.get("content-length")) > 0); + assert.equal((await head.text()).length, 0); }); // Production caches compiled templates; rendering must stay correct across repeated requests. @@ -81,21 +86,9 @@ test("renders the 403 error page as HTML", async () => { assert.match(html, /style\.css/); }); -test("blocks encoded path traversal out of /public/ with 403", async () => { - const res = await fetch(base + "/public/..%2f..%2fapp.ts"); - assert.equal(res.status, 403); -}); - -test("rejects a control char (NUL) in a static path with 403", async () => { - const res = await fetch(base + "/public/%00"); - assert.equal(res.status, 403); -}); - -test("HEAD on a static file sends headers but no body", async () => { - const res = await fetch(base + "/public/css/style.css", { method: "HEAD" }); - assert.equal(res.status, 200); - assert.ok(Number(res.headers.get("content-length")) > 0); - assert.equal((await res.text()).length, 0); +test("rejects unsafe static request paths (encoded traversal, NUL) with 403", async () => { + assert.equal((await fetch(base + "/public/..%2f..%2fapp.ts")).status, 403); + assert.equal((await fetch(base + "/public/%00")).status, 403); }); test("resolveStaticPath blocks traversal and control chars, allows nested files", () => { diff --git a/src/config.test.ts b/src/config.test.ts index 477ecb5..9b4c726 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -32,12 +32,9 @@ test("rejects a malformed Ory URL", () => { assert.throws(() => loadConfig({ KETO_READ_URL: "not a url" }), /KETO_READ_URL/); }); -test("production requires every secret to be set", () => { +test("production rejects a missing or dev-throwaway secret", () => { assert.throws(() => loadConfig({ NODE_ENV: "production" }), /COOKIE_SECRET/); assert.throws(() => loadConfig({ COOKIE_SECRET: "real", NODE_ENV: "production" }), /CSRF_SECRET/); -}); - -test("production rejects a dev throwaway secret", () => { assert.throws( () => loadConfig({ COOKIE_SECRET: "dev-insecure-cookie-secret", CSRF_SECRET: "real", NODE_ENV: "production" }), /COOKIE_SECRET/, diff --git a/src/context.test.ts b/src/context.test.ts index 405a71b..899ea00 100644 --- a/src/context.test.ts +++ b/src/context.test.ts @@ -12,23 +12,18 @@ function reqRes(url?: string): { req: IncomingMessage; res: ServerResponse } { return { req, res: new ServerResponse(req) }; } -test("buildContext parses the URL and defaults to an anonymous user", () => { - const { req, res } = reqRes("/users?q=ann"); +test("buildContext parses the URL, exposes query, and defaults to an anonymous user", () => { + const { req, res } = reqRes("/users?q=ann&page=2"); const ctx = buildContext(req, res); assert.equal(ctx.req, req); assert.equal(ctx.res, res); assert.equal(ctx.url.pathname, "/users"); - assert.equal(ctx.user, null); - assert.deepEqual(ctx.roles, []); - assert.deepEqual(ctx.params, {}); -}); - -test("buildContext exposes query as the URL's search params", () => { - const { req, res } = reqRes("/users?q=ann&page=2"); - const ctx = buildContext(req, res); assert.equal(ctx.query, ctx.url.searchParams); // same instance, not a copy assert.equal(ctx.query.get("q"), "ann"); assert.equal(ctx.query.get("page"), "2"); + assert.equal(ctx.user, null); + assert.deepEqual(ctx.roles, []); + assert.deepEqual(ctx.params, {}); }); test("buildContext threads path params supplied by the router", () => { diff --git a/src/cookie.test.ts b/src/cookie.test.ts index 5b612c3..e78389f 100644 --- a/src/cookie.test.ts +++ b/src/cookie.test.ts @@ -10,13 +10,11 @@ test("parseCookies returns an empty object for an absent or empty header", () => assert.deepEqual(flat(""), {}); }); -test("parseCookies splits pairs and trims surrounding whitespace", () => { +test("parseCookies splits pairs, trims, keeps `=` in values, and skips nameless/`=`-less pairs", () => { assert.deepEqual(flat("a=1; b=2"), { a: "1", b: "2" }); assert.deepEqual(flat(" a = 1 ;b= 2"), { a: "1", b: "2" }); -}); - -test("parseCookies keeps `=` inside the value (base64/JWT-like tokens)", () => { - assert.deepEqual(flat("t=ey.Jh=="), { t: "ey.Jh==" }); + assert.deepEqual(flat("t=ey.Jh=="), { t: "ey.Jh==" }); // `=` inside a base64/JWT value is kept + assert.deepEqual(flat("novalue; =orphan; a=1"), { a: "1" }); }); test("parseCookies decodes percent-encoded values, raw on malformed", () => { @@ -28,10 +26,6 @@ test("parseCookies strips one layer of surrounding double-quotes", () => { assert.equal(parseCookies('a="quoted"').a, "quoted"); }); -test("parseCookies skips pairs without a name or `=`", () => { - assert.deepEqual(flat("novalue; =orphan; a=1"), { a: "1" }); -}); - test("parseCookies keeps the first occurrence of a duplicate name", () => { assert.equal(parseCookies("a=first; a=second").a, "first"); }); @@ -44,12 +38,9 @@ test("parseCookies is not vulnerable to prototype pollution", () => { assert.equal(Object.getPrototypeOf({}), Object.prototype); // global prototype untouched }); -test("serializeCookie emits a bare name=value, encoding the value", () => { +test("serializeCookie emits name=value, encoding specials but leaving JWT chars (-_.) readable", () => { assert.equal(serializeCookie("session", "abc"), "session=abc"); assert.equal(serializeCookie("session", "a b&c"), "session=a%20b%26c"); -}); - -test("serializeCookie leaves JWT characters (-_.) readable", () => { assert.equal(serializeCookie("session", "ab-_.cd"), "session=ab-_.cd"); }); @@ -58,35 +49,28 @@ test("serializeCookie appends the secure-by-default attribute flags", () => { assert.equal(out, "session=x; Path=/; HttpOnly; SameSite=Lax; Secure"); }); -test("serializeCookie writes Max-Age and rejects a non-integer", () => { +test("serializeCookie writes Max-Age (incl. non-positive, expire-now) and rejects a non-integer", () => { assert.match(serializeCookie("a", "1", { maxAge: 600 }), /; Max-Age=600(;|$)/); assert.match(serializeCookie("a", "1", { maxAge: 0 }), /; Max-Age=0(;|$)/); + assert.match(serializeCookie("a", "1", { maxAge: -1 }), /; Max-Age=-1(;|$)/); assert.throws(() => serializeCookie("a", "1", { maxAge: 1.5 }), /integer/); }); -test("serializeCookie writes Expires from a Date and rejects an invalid one", () => { +test("serializeCookie writes Expires from a Date and rejects invalid or out-of-range ones", () => { assert.match(serializeCookie("a", "1", { expires: new Date(0) }), /; Expires=Thu, 01 Jan 1970 00:00:00 GMT/); assert.throws(() => serializeCookie("a", "1", { expires: new Date("nope") }), /Expires/); -}); - -test("serializeCookie rejects an Expires year outside the 4-digit RFC range", () => { // toUTCString() of a year > 9999 yields a 6-digit year browsers may reject — fail loud instead. assert.throws(() => serializeCookie("a", "1", { expires: new Date(8640000000000000) }), /Expires/); }); -test("serializeCookie writes Domain and Path", () => { +test("serializeCookie writes Domain/Path and rejects empty or injecting values", () => { const out = serializeCookie("a", "1", { domain: "example.com", path: "/admin" }); assert.match(out, /; Domain=example\.com/); assert.match(out, /; Path=\/admin/); -}); - -test("serializeCookie rejects an empty Domain or Path (misconfigured deploy)", () => { - assert.throws(() => serializeCookie("a", "1", { domain: "" }), /domain/); + assert.throws(() => serializeCookie("a", "1", { domain: "" }), /domain/); // misconfigured deploy assert.throws(() => serializeCookie("a", "1", { path: "" }), /path/); -}); - -test("serializeCookie allows a non-positive Max-Age (expire immediately, by design)", () => { - assert.match(serializeCookie("a", "1", { maxAge: -1 }), /; Max-Age=-1(;|$)/); + assert.throws(() => serializeCookie("a", "1", { path: "/x; Domain=evil.com" }), /path/); // attribute injection + assert.throws(() => serializeCookie("a", "1", { domain: "evil\r\nSet-Cookie: x=y" }), /domain/); // header split }); test("serializeCookie rejects SameSite=None without Secure (browsers would drop it)", () => { @@ -99,11 +83,6 @@ test("serializeCookie rejects an invalid cookie name", () => { assert.throws(() => serializeCookie("a;b", "1"), /name/); }); -test("serializeCookie rejects attribute values that could inject extra attributes", () => { - assert.throws(() => serializeCookie("a", "1", { path: "/x; Domain=evil.com" }), /path/); - assert.throws(() => serializeCookie("a", "1", { domain: "evil\r\nSet-Cookie: x=y" }), /domain/); -}); - test("serializeCookie and parseCookies round-trip an arbitrary value", () => { const value = "header.payload.sig with spaces & symbols="; const setCookie = serializeCookie("session", value, { httpOnly: true }); diff --git a/src/jwt.test.ts b/src/jwt.test.ts index 3a56cb3..5e231fc 100644 --- a/src/jwt.test.ts +++ b/src/jwt.test.ts @@ -33,32 +33,28 @@ test("verifies an ES256 token (raw r‖s signature)", () => { assert.deepEqual(verifyJws(token, ecJwk).payload, { sub: "u" }); }); -test("rejects a tampered payload", () => { +// All three reach and fail the signature check itself, not an earlier structural guard. +test("rejects a signature that fails verification (tampered payload, wrong key, empty)", () => { const token = makeJws("RS256", rsa.privateKey, { roles: ["user"], sub: "u" }); - const [header, , signature] = token.split("."); + const [header, payload, signature] = token.split("."); + const forged = `${header}.${b64url(JSON.stringify({ roles: ["admin"], sub: "u" }))}.${signature}`; assert.throws(() => verifyJws(forged, rsaJwk), /invalid signature/); -}); -test("rejects a signature from a different key", () => { - const token = makeJws("RS256", rsa.privateKey, { sub: "u" }); - const other = generateKeyPairSync("rsa", { modulusLength: 2048 }); - assert.throws(() => verifyJws(token, other.publicKey.export({ format: "jwk" }) as JsonWebKey), /invalid signature/); -}); + const otherJwk = generateKeyPairSync("rsa", { modulusLength: 2048 }).publicKey.export({ format: "jwk" }) as JsonWebKey; + assert.throws(() => verifyJws(token, otherJwk), /invalid signature/); -test("rejects an empty signature segment", () => { - const [header, payload] = makeJws("RS256", rsa.privateKey, { sub: "u" }).split("."); assert.throws(() => verifyJws(`${header}.${payload}.`, rsaJwk), /invalid signature/); }); -test("rejects alg:none", () => { - const token = `${b64url(JSON.stringify({ alg: "none", typ: "JWT" }))}.${b64url(JSON.stringify({ sub: "u" }))}.`; - assert.throws(() => verifyJws(token, rsaJwk), /unsupported alg/); -}); +// The algParams allowlist is the alg-confusion defense: anything outside RS*/ES* is refused +// (`HS*` symmetric and `none` would otherwise let an attacker forge tokens). +test("rejects an alg outside the allowlist (none, HS256)", () => { + const none = `${b64url(JSON.stringify({ alg: "none", typ: "JWT" }))}.${b64url(JSON.stringify({ sub: "u" }))}.`; + assert.throws(() => verifyJws(none, rsaJwk), /unsupported alg/); -test("rejects symmetric alg HS256", () => { - const token = `${b64url(JSON.stringify({ alg: "HS256" }))}.${b64url(JSON.stringify({ sub: "u" }))}.${b64url("x")}`; - assert.throws(() => verifyJws(token, rsaJwk), /unsupported alg/); + const hs256 = `${b64url(JSON.stringify({ alg: "HS256" }))}.${b64url(JSON.stringify({ sub: "u" }))}.${b64url("x")}`; + assert.throws(() => verifyJws(hs256, rsaJwk), /unsupported alg/); }); test("rejects when key type does not match the alg family", () => { @@ -76,22 +72,18 @@ test("rejects a symmetric JWK (kty:oct) for an asymmetric alg — second defense assert.throws(() => verifyJws(token, { k: b64url("secret"), kty: "oct" }), /invalid JWK/); }); -test("rejects a token without three segments", () => { +// decodeJws structural guards, all rejected before any crypto runs. +test("rejects malformed tokens before crypto (segment count, payload type, base64url, kid)", () => { + const p = b64url(JSON.stringify({ sub: "u" })); assert.throws(() => verifyJws("only.two", rsaJwk), /expected 3 segments/); -}); -test("rejects a non-object (array) payload", () => { - const token = `${b64url(JSON.stringify({ alg: "RS256" }))}.${b64url(JSON.stringify([1, 2, 3]))}.${b64url("x")}`; - assert.throws(() => verifyJws(token, rsaJwk), /payload not an object/); -}); + const arrayPayload = `${b64url(JSON.stringify({ alg: "RS256" }))}.${b64url(JSON.stringify([1, 2, 3]))}.${b64url("x")}`; + assert.throws(() => verifyJws(arrayPayload, rsaJwk), /payload not an object/); -test("rejects a non-canonical base64url segment before any crypto", () => { - assert.throws(() => verifyJws(`ab*c.${b64url(JSON.stringify({ sub: "u" }))}.${b64url("x")}`, rsaJwk), /base64url/); -}); + assert.throws(() => verifyJws(`ab*c.${p}.${b64url("x")}`, rsaJwk), /base64url/); -test("rejects a non-string kid in the header", () => { - const token = `${b64url(JSON.stringify({ alg: "RS256", kid: 123 }))}.${b64url(JSON.stringify({ sub: "u" }))}.${b64url("x")}`; - assert.throws(() => verifyJws(token, rsaJwk), /kid/); + const badKid = `${b64url(JSON.stringify({ alg: "RS256", kid: 123 }))}.${p}.${b64url("x")}`; + assert.throws(() => verifyJws(badKid, rsaJwk), /kid/); }); test("decodeJws exposes header and payload without verifying", () => { diff --git a/todo.md b/todo.md index afbdc53..9507891 100644 --- a/todo.md +++ b/todo.md @@ -19,7 +19,7 @@ everything via Docker. - [x] Config/env loader: Ory endpoints, cookie/CSRF secret, JWKS location, ports. → `src/config.ts` (`loadConfig`); validated at boot, dev defaults for clean-clone, prod requires real secrets; wired into `server.ts`. - [x] Run the architecture _and_ the stability reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Both: no bugs/security issues. Addressed: wired `buildContext` into `app.ts`; graceful SIGTERM/SIGINT shutdown; EJS template caching in prod. Deferred `core/`/`shell/` split (premature for an 8-file scaffold; revisit at §2/§4). - [x] Go over all comments in the code and the README and try to make it shorter and more information dense. Remove not strictly needed stuff. → Tightened comments across `src/*.ts`, Dockerfile, and trimmed verbose/duplicated prose in README; tests + typecheck green. -- [ ] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us. +- [x] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us. → Merged related cases across jwt/cookie/app/context/config tests (59 → 42), every assertion preserved; typecheck + tests green. ### 0.1 Extra input from human - [ ] Remove all usage of NODE_ENV - add a new core principle to the project that the app should at all times be unaware of what environment it is running in. Configuration should be explicit, like "disable email" or "cache templates".