Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions backend/cli/src/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { describeRoute, generateSpecs, validator, resolver, openAPIRouteHandler
import { Hono } from "hono"
import { cors } from "hono/cors"
import { streamSSE } from "hono/streaming"
import { serveWebAsset } from "../web/serve"
import { serveWebAsset, wantsJson } from "../web/serve"
import { isAllowedHost, isAllowedOrigin, isCrossOrigin } from "./host-guard"
import { timingSafeEqual } from "../util/timing-safe"
import { FolderResolveRoutes } from "./routes/folder-resolve"
Expand Down Expand Up @@ -80,7 +80,7 @@ export namespace Server {
return _server?.requestIP(req)?.address
}

const app = new Hono()
const app = new Hono({ strict: false })
export const App: () => Hono = lazy(
() =>
// TODO: Break server.ts into smaller route files to fix type inference
Expand Down Expand Up @@ -640,6 +640,11 @@ export namespace Server {
// try to JSON.parse `<!doctype`) and never proxy upstream.
if (c.req.path.startsWith("/api/")) return c.notFound()

if (wantsJson(c.req.header("accept") ?? null, c.req.header("content-type") ?? null)) {
log.warn("unmatched API-shaped request", { method: c.req.method, path: c.req.path })
return c.json({ error: "not_found", path: c.req.path }, 404)
}

const local = await serveWebAsset(c)
if (local) {
local.headers.set("Content-Security-Policy", csp)
Expand Down
12 changes: 12 additions & 0 deletions backend/cli/src/web/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ function contentType(reqPath: string): string {
return MIME[reqPath.slice(dot).toLowerCase()] ?? "application/octet-stream"
}

/**
* True when a request is an API/data call (expects JSON), not a browser
* navigation (expects an HTML page). The catch-all uses this to choose a JSON
* 404 over the SPA index.html fallback — SPA-falling-back a data call makes the
* client JSON.parse `<!doctype html>` and crash (#180/#197/#189).
*/
export function wantsJson(accept: string | null, contentTypeHeader: string | null): boolean {
if (contentTypeHeader?.includes("application/json")) return true
const a = accept ?? ""
return a.includes("application/json") && !a.includes("text/html")
}

export async function serveWebAsset(c: Context): Promise<Response | undefined> {
if (!WEB_INDEX) return undefined

Expand Down
78 changes: 78 additions & 0 deletions backend/cli/test/server/spa-fallback.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { describe, expect, test } from "bun:test"
import path from "path"
import { Instance } from "../../src/project/instance"
import { Server } from "../../src/server/server"
import { Log } from "../../src/util/log"

const projectRoot = path.join(__dirname, "../..")
Log.init({ print: false })

describe("spa fallback", () => {
test("unmatched API-shaped request under /settings gets a JSON 404, not SPA HTML", async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
const fetch = Server.internalFetch()
const response = await fetch("http://openscience.internal/settings/nonexistent", {
headers: { "content-type": "application/json" },
})

expect(response.status).toBe(404)
expect(response.headers.get("content-type")).toContain("application/json")

const text = await response.text()
expect(text.startsWith("<")).toBe(false)
expect(JSON.parse(text)).toEqual({ error: "not_found", path: "/settings/nonexistent" })
},
})
})

test("browser navigation to an unmatched route still gets the SPA index.html", async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
const fetch = Server.internalFetch()
const response = await fetch("http://openscience.internal/definitely/not/a/route", {
headers: { accept: "text/html" },
})

expect(response.status).toBe(200)
expect(response.headers.get("content-type")).toContain("text/html")
expect(response.headers.get("content-security-policy")).toContain("default-src 'self'")

const text = await response.text()
expect(text.toLowerCase().startsWith("<!doctype")).toBe(true)
},
})
})

test("unmatched /api/* request still 404s (regression)", async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
const fetch = Server.internalFetch()
const response = await fetch("http://openscience.internal/api/also-nonexistent")

expect(response.status).toBe(404)
},
})
})

test("GET /settings/local/ (trailing slash) resolves to the real route, not the catch-all", async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
const fetch = Server.internalFetch()
const response = await fetch("http://openscience.internal/settings/local/", {
headers: { "content-type": "application/json" },
})

expect(response.status).toBe(200)
expect(response.headers.get("content-type")).toContain("application/json")

const body = await response.json()
expect(Array.isArray(body.presets)).toBe(true)
},
})
})
})
28 changes: 28 additions & 0 deletions backend/cli/test/web/serve.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { describe, expect, test } from "bun:test"
import { wantsJson } from "../../src/web/serve"

describe("wantsJson", () => {
test("content-type application/json → true", () => {
expect(wantsJson(null, "application/json")).toBe(true)
})

test("accept includes application/json, no text/html → true", () => {
expect(wantsJson("application/json, text/plain", null)).toBe(true)
})

test("browser navigation accept header → false", () => {
expect(wantsJson("text/html,application/xhtml+xml,*/*", null)).toBe(false)
})

test("text/html present alongside application/json → treated as navigation, false", () => {
expect(wantsJson("text/html,application/json", null)).toBe(false)
})

test("wildcard accept → false", () => {
expect(wantsJson("*/*", null)).toBe(false)
})

test("both null → false", () => {
expect(wantsJson(null, null)).toBe(false)
})
})
10 changes: 5 additions & 5 deletions frontend/workspace/src/components/settings/LocalModels.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const LocalModels: Component = () => {
call<{ detected: Detected[] }>("/detect").then((r) => r.detected),
)
const [configured, { refetch: refetchConfigured }] = createResource(() =>
call<{ providers: Configured[] }>("/").then((r) => r.providers),
call<{ providers: Configured[] }>("").then((r) => r.providers),
)
const [status, { refetch: refetchStatus }] = createResource(() =>
call<{ runtimes: Runtime[] }>("/status").then((r) => r.runtimes),
Expand All @@ -80,7 +80,7 @@ const LocalModels: Component = () => {
const addRuntime = (d: Detected) =>
guard(
() =>
call("/", {
call("", {
method: "POST",
body: JSON.stringify({ url: d.baseURL, id: d.id, name: `${d.name} (local)`, models: d.models }),
}),
Expand All @@ -106,7 +106,7 @@ const LocalModels: Component = () => {
showToast({ title: `${rt.name} isn't installed`, description: `Install it, then start it here.` })
window.open(r.install ?? rt.install, "_blank", "noopener")
} else if (r.running && r.models?.length) {
await call("/", {
await call("", {
method: "POST",
body: JSON.stringify({ url: rt.baseURL, id: rt.id, name: `${rt.name} (local)`, models: r.models }),
})
Expand All @@ -126,7 +126,7 @@ const LocalModels: Component = () => {
const addRunning = (rt: Runtime) =>
guard(
() =>
call("/", {
call("", {
method: "POST",
body: JSON.stringify({ url: rt.baseURL, id: rt.id, name: `${rt.name} (local)`, models: rt.models }),
}),
Expand Down Expand Up @@ -172,7 +172,7 @@ const LocalModels: Component = () => {
guard(async () => {
const models = [...selected()]
if (!models.length) throw new Error("Select at least one model.")
await call("/", {
await call("", {
method: "POST",
body: JSON.stringify({ url: url().trim(), key: key().trim() || undefined, models }),
})
Expand Down
40 changes: 40 additions & 0 deletions frontend/workspace/src/components/settings/api.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { describe, expect, test } from "bun:test"
import { settingsApi } from "./api"

const base = "http://x"
const path = "/settings/local"

describe("settingsApi", () => {
test("throws a descriptive error when a 200 response is not JSON", async () => {
const fetchFn = (async () =>
new Response("<!doctype html><html></html>", {
status: 200,
headers: { "content-type": "text/html; charset=utf-8" },
})) as unknown as typeof fetch

const error = await settingsApi<never>(base, fetchFn, path).catch((e: Error) => e)

expect(error).toBeInstanceOf(Error)
expect(error.message).toContain("/settings/local")
expect(error.message).toContain("Expected JSON")
expect(error.message).not.toContain("Unexpected token")
})

test("resolves with parsed JSON on a normal 200 response", async () => {
const fetchFn = (async () => Response.json({ ok: true }, { status: 200 })) as unknown as typeof fetch

expect(await settingsApi<{ ok: boolean }>(base, fetchFn, path)).toEqual({ ok: true })
})

test("resolves with undefined on 204", async () => {
const fetchFn = (async () => new Response(null, { status: 204 })) as unknown as typeof fetch

expect(await settingsApi(base, fetchFn, path)).toBeUndefined()
})

test("preserves the existing non-ok error path", async () => {
const fetchFn = (async () => new Response("boom", { status: 500 })) as unknown as typeof fetch

await expect(settingsApi(base, fetchFn, path)).rejects.toThrow(/boom|500/)
})
})
4 changes: 4 additions & 0 deletions frontend/workspace/src/components/settings/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,9 @@ export async function settingsApi<T>(
throw new Error(text || `${res.status} ${res.statusText}`)
}
if (res.status === 204) return undefined as T
const contentType = res.headers.get("content-type") ?? ""
if (!contentType.includes("application/json")) {
throw new Error(`Expected JSON from ${path}, but got ${res.status} (${contentType || "no content-type"})`)
}
return (await res.json()) as T
}
Loading