Public: anyone can read this; the crew can change it.
A mistyped studio URL gets the forge's page, not a JSON blob #137
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/117-a-mistyped-studio-url-gets-the-forge-s-p → main 1 commit fork
3 files
changed +105
−2
src/http/respond.ts +42
| @@ -50,9 +50,51 @@ export const forbidden = () => | ||
| 50 | 50 | apiError("forbidden", 403, "your credentials lack the scope for this action"); |
| 51 | 51 | // A path matching no route at all — distinct from a missing resource, so a |
| 52 | 52 | // client can tell a typo'd URL from a resource that genuinely isn't there. |
| 53 | // The message survives into the HTML 404 too (`src/web/chrome.ts`), as a | |
| 54 | // code comment there rather than a header a person would never read. | |
| 53 | 55 | export const noRoute = (method: string, path: string) => |
| 54 | 56 | apiError("no_route", 404, `no ${method} route for ${path}`); |
| 55 | 57 | |
| 58 | /** | |
| 59 | * One media range beats another only when it names it (or a wildcard that | |
| 60 | * reaches it) at a strictly higher `q`; a tie — including the everything | |
| 61 | * range curl sends by default — favours the type this forge answered with | |
| 62 | * before this distinction existed, so a script that never asked for HTML | |
| 63 | * keeps getting JSON. | |
| 64 | */ | |
| 65 | function acceptQuality(accept: string, type: string): number { | |
| 66 | const [group, kind] = type.split("/"); | |
| 67 | let best = -1; | |
| 68 | for (const range of accept.split(",")) { | |
| 69 | const [rawType, ...params] = range.split(";"); | |
| 70 | const candidate = (rawType ?? "").trim().toLowerCase(); | |
| 71 | const [rg, rk] = candidate.split("/"); | |
| 72 | if (rg !== "*" && rg !== group) continue; | |
| 73 | if (rg !== "*" && rk !== "*" && rk !== kind) continue; | |
| 74 | let q = 1; | |
| 75 | for (const param of params) { | |
| 76 | const [key, value] = param.split("=").map((s) => s.trim()); | |
| 77 | if (key === "q" && value !== undefined) q = Number(value); | |
| 78 | } | |
| 79 | if (Number.isFinite(q) && q > best) best = q; | |
| 80 | } | |
| 81 | return best; | |
| 82 | } | |
| 83 | ||
| 84 | /** | |
| 85 | * Whether a request, judged by `Accept` alone, would rather have the forge's | |
| 86 | * own page than a JSON error — a browser addressing a typo'd or moved URL, | |
| 87 | * not a script or `curl` that never said otherwise. | |
| 88 | */ | |
| 89 | export function prefersHtml(request: Request, pathname: string): boolean { | |
| 90 | if (pathname.startsWith("/api/")) return false; | |
| 91 | const accept = request.headers.get("accept"); | |
| 92 | if (!accept) return false; | |
| 93 | return ( | |
| 94 | acceptQuality(accept, "text/html") > acceptQuality(accept, "application/json") | |
| 95 | ); | |
| 96 | } | |
| 97 | ||
| 56 | 98 | /** |
| 57 | 99 | * Map an action's error to a response: the three access errors have fixed |
| 58 | 100 | * shapes; anything else is a validation code the caller chose a status for. |
src/http/server.ts +27 −2
| @@ -218,7 +218,7 @@ import { | ||
| 218 | 218 | type Ctx, |
| 219 | 219 | type Handler, |
| 220 | 220 | } from "./context.ts"; |
| 221 | import { apiError, json, noRoute, notFound } from "./respond.ts"; | |
| 221 | import { apiError, json, noRoute, notFound, prefersHtml } from "./respond.ts"; | |
| 222 | 222 | import { RateLimiter, type Verdict } from "../core/ratelimit.ts"; |
| 223 | 223 | import { |
| 224 | 224 | MAX_KEY_LENGTH, |
| @@ -1096,7 +1096,32 @@ export async function serve(app: App): Promise<Servers> { | ||
| 1096 | 1096 | const url = publicUrl(request, app.config.url); |
| 1097 | 1097 | if (url.pathname === "/healthz") return healthResponse(); |
| 1098 | 1098 | const matched = router.match(request.method, url.pathname); |
| 1099 | if (!matched) return noRoute(request.method, url.pathname); | |
| 1099 | if (!matched) { | |
| 1100 | // A path under no route at all is what a mistyped or stale studio | |
| 1101 | // URL lands on. A browser gets the forge's own page for it, same as | |
| 1102 | // any other 404; an API client — anything outside `/api/` that never | |
| 1103 | // said it wanted HTML — keeps the `no_route` body, distinct from | |
| 1104 | // `not_found` so it can tell a typo from a resource that is really | |
| 1105 | // gone. The page is rendered from a viewer-less `Ctx`: resolving a | |
| 1106 | // session or token here would spend a database read on a path that, | |
| 1107 | // by construction, cannot belong to a signed-in-only route, at the | |
| 1108 | // cost of the topbar reading "Sign in" even for someone who is. | |
| 1109 | if (prefersHtml(request, url.pathname)) { | |
| 1110 | const themeCookie = cookieValue(request, "forge_theme") ?? ""; | |
| 1111 | const modeCookieValue = cookieValue(request, "forge_mode") ?? ""; | |
| 1112 | const anonCtx: Ctx = { | |
| 1113 | app, | |
| 1114 | request, | |
| 1115 | url, | |
| 1116 | params: {}, | |
| 1117 | auth: { user: undefined, scope: undefined, via: "none" }, | |
| 1118 | theme: isThemeName(themeCookie) ? themeCookie : undefined, | |
| 1119 | mode: isMode(modeCookieValue) ? modeCookieValue : DEFAULT_MODE, | |
| 1120 | }; | |
| 1121 | return notFoundPage(anonCtx); | |
| 1122 | } | |
| 1123 | return noRoute(request.method, url.pathname); | |
| 1124 | } | |
| 1100 | 1125 | const auth = await resolveAuth(app, request); |
| 1101 | 1126 | |
| 1102 | 1127 | // The two things standing between one runaway agent and everybody |
test/web.test.ts +36
| @@ -302,6 +302,42 @@ describe("web pages end to end", () => { | ||
| 302 | 302 | expect(text).toContain("404"); |
| 303 | 303 | } |
| 304 | 304 | }); |
| 305 | ||
| 306 | test("a path under no route at all gets the forge's page for a browser, and no_route for anyone else", async () => { | |
| 307 | const browser = await fetch(`${base}/ada/engine/tells`, { | |
| 308 | headers: { | |
| 309 | accept: | |
| 310 | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", | |
| 311 | }, | |
| 312 | }); | |
| 313 | expect(browser.status).toBe(404); | |
| 314 | expect(browser.headers.get("content-type")).toContain("text/html"); | |
| 315 | expect(browser.headers.get("content-security-policy")).toContain( | |
| 316 | "default-src 'none'", | |
| 317 | ); | |
| 318 | expect(browser.headers.get("x-content-type-options")).toBe("nosniff"); | |
| 319 | expect(browser.headers.get("referrer-policy")).toBeTruthy(); | |
| 320 | const html = await browser.text(); | |
| 321 | expect(html).toContain("404"); | |
| 322 | expect(html).toContain("Go home"); | |
| 323 | ||
| 324 | // No Accept header at all — a plain script or `curl` — keeps the JSON | |
| 325 | // contract, same as before this page existed. | |
| 326 | const plain = await fetch(`${base}/ada/engine/tells`); | |
| 327 | expect(plain.status).toBe(404); | |
| 328 | expect(plain.headers.get("content-type")).toContain("application/json"); | |
| 329 | const body = (await plain.json()) as { error: string; message: string }; | |
| 330 | expect(body.error).toBe("no_route"); | |
| 331 | expect(body.message).toContain("/ada/engine/tells"); | |
| 332 | ||
| 333 | // An API path is unchanged even for a browser's own Accept header. | |
| 334 | const api = await fetch(`${base}/api/v1/nonsense`, { | |
| 335 | headers: { accept: "text/html,*/*;q=0.8" }, | |
| 336 | }); | |
| 337 | expect(api.headers.get("content-type")).toContain("application/json"); | |
| 338 | const apiBody = (await api.json()) as { error: string }; | |
| 339 | expect(apiBody.error).toBe("no_route"); | |
| 340 | }); | |
| 305 | 341 | }); |
| 306 | 342 | |
| 307 | 343 | describe("themes", () => { |
new above
offline