From a241e5d3ded4d546535b630cd586a822c26ae5e8 Mon Sep 17 00:00:00 2001 From: Jeff Repanich Date: Tue, 29 Sep 2026 14:27:57 -0400 Subject: [PATCH 1/3] docs: require a changelog section with every version bump --- AGENTS.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index e573669..360e2b8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,18 @@ it into magic. Run `npm run check` and `npm run build`. A changed stage must remain runnable, and journey assertions and generated contracts must agree with the example. +## Changelog + +Any change to the `version` field in `package.json`, whether a release, +prerelease, or patch bump, must include a matching `## ` section in +`CHANGELOG.md` in the same commit or pull request. Date the section and list +breaking changes (with migration notes), deprecations, additions, and fixes. +Move entries from `Unreleased` into the new version section rather than leaving +them there. If the repository has no `CHANGELOG.md` yet, create one (Keep a +Changelog style) at the next version bump. Do not publish or tag a version whose +changelog section is missing. This package is private and not published to npm, +but the rule still applies whenever its `version` changes. + ## Optimization Gate A benchmark number is only half of an optimization's success criterion. The From 537907e1eecb44ba8c9359d4649ebc6ea52fbc98 Mon Sep 17 00:00:00 2001 From: Jeff Repanich Date: Tue, 29 Sep 2026 15:52:40 -0400 Subject: [PATCH 2/3] test(api-ssr): guard defineQuery fetchers' (input, { signal }) contract (#28) Adds api-ssr/tests/queries.test.ts asserting every defineQuery fetcher uses the askr 0.4 (input, { signal }) signature, encodes ids, forwards the AbortSignal to fetch, and is cancelled when the signal aborts mid-flight. Closes #21 --- api-ssr/tests/queries.test.ts | 91 +++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 api-ssr/tests/queries.test.ts diff --git a/api-ssr/tests/queries.test.ts b/api-ssr/tests/queries.test.ts new file mode 100644 index 0000000..17f3428 --- /dev/null +++ b/api-ssr/tests/queries.test.ts @@ -0,0 +1,91 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + activityQuery, + dashboardQuery, + policyQuery, + sessionQuery, + userQuery, + usersQuery, +} from "../src/domains/queries.js"; + +type FetchCall = { url: string; init: RequestInit | undefined }; + +function stubFetch(body: unknown = {}): FetchCall[] { + const calls: FetchCall[] = []; + vi.stubGlobal("fetch", async (url: string, init?: RequestInit) => { + calls.push({ url, init }); + return new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, + }); + }); + return calls; +} + +describe("defineQuery fetchers", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + const collectionCases = [ + ["sessionQuery", sessionQuery, "/api/session"], + ["dashboardQuery", dashboardQuery, "/api/dashboard"], + ["activityQuery", activityQuery, "/api/activity"], + ["usersQuery", usersQuery, "/api/users"], + ] as const; + + it.each(collectionCases)( + "%s should forward the context signal to fetch", + async (_name, query, path) => { + const calls = stubFetch(); + const controller = new AbortController(); + + await query.fetch({}, { signal: controller.signal }); + + expect(calls).toHaveLength(1); + expect(calls[0]?.url).toBe(path); + expect(calls[0]?.init?.signal).toBe(controller.signal); + }, + ); + + const detailCases = [ + ["userQuery", userQuery, "/api/users/a%2Fb"], + ["policyQuery", policyQuery, "/api/policies/a%2Fb"], + ] as const; + + it.each(detailCases)( + "%s should read the id from input and forward the context signal", + async (_name, query, path) => { + const calls = stubFetch(); + const controller = new AbortController(); + + await query.fetch({ id: "a/b" }, { signal: controller.signal }); + + expect(calls).toHaveLength(1); + expect(calls[0]?.url).toBe(path); + expect(calls[0]?.init?.signal).toBe(controller.signal); + }, + ); + + it("should cancel an in-flight request when the context signal aborts", async () => { + let started!: () => void; + const inFlight = new Promise((resolve) => (started = resolve)); + vi.stubGlobal("fetch", (_url: string, init?: RequestInit) => { + started(); + // Never settles on its own: only an abort on the forwarded signal ends it. + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => reject(init.signal?.reason), { + once: true, + }); + }); + }); + const controller = new AbortController(); + const reason = new Error("navigated away"); + + const pending = userQuery.fetch({ id: "1" }, { signal: controller.signal }); + await inFlight; + controller.abort(reason); + + await expect(pending).rejects.toBe(reason); + }); +}); From 2b6ec256e1780a72ab5ae3c646b99ac8d03aff55 Mon Sep 17 00:00:00 2001 From: Jeff Repanich Date: Sat, 3 Oct 2026 08:35:22 -0400 Subject: [PATCH 3/3] fix(examples): validate login redirect targets (#31) --- api-ssr/src/features/session/login-page.tsx | 8 ++- api-ssr/src/features/session/redirect.ts | 40 ++++++++++++++ api-ssr/src/routes/api.ts | 5 +- api-ssr/tests/api-ssr.test.ts | 59 +++++++++++++++++++++ api-ssr/tests/redirect.test.ts | 35 ++++++++++++ 5 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 api-ssr/src/features/session/redirect.ts create mode 100644 api-ssr/tests/redirect.test.ts diff --git a/api-ssr/src/features/session/login-page.tsx b/api-ssr/src/features/session/login-page.tsx index 5902c2a..40d9920 100644 --- a/api-ssr/src/features/session/login-page.tsx +++ b/api-ssr/src/features/session/login-page.tsx @@ -2,6 +2,7 @@ import { createMutation } from "@askrjs/askr/data"; import type { AuthContext } from "@askrjs/auth"; import { Button, Card, CardContent, CardHeader, CardTitle, Stack } from "@askrjs/themes/components"; import { OperationsLayout } from "../../application/layout.js"; +import { safeNextPath } from "./redirect.js"; import { responseJson } from "../shared/transport.js"; export function LoginPage() { @@ -22,8 +23,11 @@ export function LoginPage() { const next = typeof window === "undefined" ? "/workspace" - : (new URLSearchParams(window.location.search).get("next") ?? "/workspace"); - window.location.assign(next.startsWith("/") ? next : "/workspace"); + : safeNextPath( + new URLSearchParams(window.location.search).get("next"), + window.location.origin, + ); + window.location.assign(next); }; return ( diff --git a/api-ssr/src/features/session/redirect.ts b/api-ssr/src/features/session/redirect.ts new file mode 100644 index 0000000..9f71bcd --- /dev/null +++ b/api-ssr/src/features/session/redirect.ts @@ -0,0 +1,40 @@ +const WORKSPACE_PATH = "/workspace"; + +export function safeNextPath(value: string | null | undefined, origin: string): string { + if (!value?.startsWith("/") || value.startsWith("//")) return WORKSPACE_PATH; + + const queryIndex = value.indexOf("?"); + const hashIndex = value.indexOf("#"); + const pathEnd = Math.min( + queryIndex < 0 ? value.length : queryIndex, + hashIndex < 0 ? value.length : hashIndex, + ); + const path = value.slice(0, pathEnd); + if (path.includes("\\")) return WORKSPACE_PATH; + + try { + const decoded = decodeURIComponent(value); + const decodedPath = decodeURIComponent(path); + if ( + decodedPath.includes("\\") || + [...decoded].some((character) => { + const code = character.charCodeAt(0); + return code <= 31 || code === 127; + }) || + decodedPath.startsWith("//") || + /^(?:\/)*(?:[a-z][a-z\d+.-]*:)/i.test(decodedPath) || + /(?:^|\/)\.\.(?:\/|$)/.test(decodedPath) + ) { + return WORKSPACE_PATH; + } + + const base = new URL(origin); + const target = new URL(value, base); + if (target.origin !== base.origin || target.pathname.startsWith("//")) { + return WORKSPACE_PATH; + } + return `${target.pathname}${target.search}${target.hash}`; + } catch { + return WORKSPACE_PATH; + } +} diff --git a/api-ssr/src/routes/api.ts b/api-ssr/src/routes/api.ts index b96e807..4007d3c 100644 --- a/api-ssr/src/routes/api.ts +++ b/api-ssr/src/routes/api.ts @@ -4,6 +4,7 @@ import { security } from "@askrjs/server/openapi"; import type { AskrAppApi } from "@askrjs/server/askr"; import type { AppDependencies } from "../boot/dependencies.js"; import { SESSION_COOKIE } from "../domains/sessions/repository.js"; +import { safeNextPath } from "../features/session/redirect.js"; import type { UserUpdate } from "../domains/users/repository.js"; function parseVersion(value: string | null): number | null { @@ -86,9 +87,7 @@ export function registerApiRoutes(api: AskrAppApi, schemas: Api documentation: { query: { next: {} } }, async handler(ctx, input, { sessions }) { const session = await sessions.create(); - const requested = input.query.next; - const next = - requested?.startsWith("/") && !requested.startsWith("//") ? requested : "/workspace"; + const next = safeNextPath(input.query.next, ctx.url.origin); const response = ctx.headers.get("accept")?.includes("text/html") ? ctx.redirect(next, 303) : ctx.created({ authenticated: true }); diff --git a/api-ssr/tests/api-ssr.test.ts b/api-ssr/tests/api-ssr.test.ts index c77ca59..a35fd0d 100644 --- a/api-ssr/tests/api-ssr.test.ts +++ b/api-ssr/tests/api-ssr.test.ts @@ -76,6 +76,65 @@ describe("API and SSR culmination", () => { expect(await after.json()).toMatchObject({ authenticated: false }); }); + it.each([ + ["protocol-relative URLs", "//attacker.example"], + ["backslash URL forms", "/\\\\attacker.example"], + ["dot-segment traversal", "/workspace/../login"], + ])("should fall back to the workspace for %s in native login redirects", async (_name, next) => { + const app = createApp(createDependencies()); + const response = await app.fetch( + new Request(`http://example.test/api/session?next=${encodeURIComponent(next)}`, { + method: "POST", + headers: { accept: "text/html" }, + }), + ); + + expect(response.status).toBe(303); + expect(response.headers.get("location")).toBe("/workspace"); + }); + + it("should preserve same-origin login redirect query and fragment values", async () => { + const app = createApp(createDependencies()); + const next = "/workspace/users/1?tab=recent#activity"; + const response = await app.fetch( + new Request(`http://example.test/api/session?next=${encodeURIComponent(next)}`, { + method: "POST", + headers: { accept: "text/html" }, + }), + ); + + expect(response.status).toBe(303); + expect(response.headers.get("location")).toBe(next); + }); + + it("should preserve traversal-like text inside native redirect query values", async () => { + const app = createApp(createDependencies()); + const next = "/workspace?path=/../reports#activity"; + const response = await app.fetch( + new Request(`http://example.test/api/session?next=${encodeURIComponent(next)}`, { + method: "POST", + headers: { accept: "text/html" }, + }), + ); + + expect(response.status).toBe(303); + expect(response.headers.get("location")).toBe(next); + }); + + it("should preserve encoded backslashes inside native redirect query values", async () => { + const app = createApp(createDependencies()); + const next = "/workspace?token=a%5Cb#activity"; + const response = await app.fetch( + new Request(`http://example.test/api/session?next=${encodeURIComponent(next)}`, { + method: "POST", + headers: { accept: "text/html" }, + }), + ); + + expect(response.status).toBe(303); + expect(response.headers.get("location")).toBe(next); + }); + it("should SSR prefetch dashboard data into hydration state without a loading branch", async () => { const app = createApp(createDependencies()); const cookie = await signIn(app); diff --git a/api-ssr/tests/redirect.test.ts b/api-ssr/tests/redirect.test.ts new file mode 100644 index 0000000..2467b3a --- /dev/null +++ b/api-ssr/tests/redirect.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import { safeNextPath } from "../src/features/session/redirect.js"; + +describe("safeNextPath", () => { + it("should preserve a same-origin path, query, and fragment", () => { + expect(safeNextPath("/workspace/users/1?tab=recent#activity", "https://example.test")).toBe( + "/workspace/users/1?tab=recent#activity", + ); + }); + + it("should preserve traversal-like text in query values", () => { + expect(safeNextPath("/workspace?path=/../reports#activity", "https://example.test")).toBe( + "/workspace?path=/../reports#activity", + ); + }); + + it("should preserve encoded backslashes in query values", () => { + expect(safeNextPath("/workspace?token=a%5Cb#activity", "https://example.test")).toBe( + "/workspace?token=a%5Cb#activity", + ); + }); + + it.each([ + "//attacker.example", + "///attacker.example", + "/\\\\attacker.example", + "https://attacker.example/path", + "/workspace/../login", + "/workspace/%2e%2e/login", + "/%5c%5cattacker.example", + "workspace", + ])("should fall back for unsafe or non-path values: %s", (next) => { + expect(safeNextPath(next, "https://example.test")).toBe("/workspace"); + }); +});