diff --git a/server/src/components/sandboxed-routes.ts b/server/src/components/sandboxed-routes.ts index 659f87ece..c32ebd750 100644 --- a/server/src/components/sandboxed-routes.ts +++ b/server/src/components/sandboxed-routes.ts @@ -53,14 +53,14 @@ export function createSandboxedRoutes( if (forbidden) return forbidden; const body = (await context.req.json().catch(() => null)) as { - slug?: string; - title?: string; - description?: string; - html?: string; - css?: string; - jsFunctions?: string; - argumentSchema?: Record; - sampleArguments?: Record; + slug?: unknown; + title?: unknown; + description?: unknown; + html?: unknown; + css?: unknown; + jsFunctions?: unknown; + argumentSchema?: unknown; + sampleArguments?: unknown; } | null; if ( @@ -72,16 +72,70 @@ export function createSandboxedRoutes( return context.json({ error: "A name and a title are required." }, 400); } + /* + * Every other field is optional, but none of them is untyped. This used to pass + * `body.description ?? ""` straight into `store.save`, so `{"description": 123}` or + * `{"argumentSchema": "not-an-object"}` travelled into a text/jsonb column and came + * back as an unhandled 500 from the database. Absent still means the default; a + * present value must be its type, else 400 naming the field. + */ + for (const field of [ + "description", + "html", + "css", + "jsFunctions", + ] as const) { + const value = body[field]; + if (value !== undefined && typeof value !== "string") { + return context.json( + { error: `The component ${field} must be text.` }, + 400, + ); + } + } + for (const field of ["argumentSchema", "sampleArguments"] as const) { + const value = body[field]; + if ( + value !== undefined && + (!value || + typeof value !== "object" || + Array.isArray(value) || + Object.getPrototypeOf(value) !== Object.prototype) + ) { + return context.json( + { error: `The component ${field} must be an object.` }, + 400, + ); + } + } + + const slug = body.slug as string; + const title = body.title as string; + const description = + body.description === undefined ? "" : (body.description as string); + const html = body.html === undefined ? "" : (body.html as string); + const css = body.css === undefined ? "" : (body.css as string); + const jsFunctions = + body.jsFunctions === undefined ? "" : (body.jsFunctions as string); + const argumentSchema = + body.argumentSchema === undefined + ? {} + : (body.argumentSchema as Record); + const sampleArguments = + body.sampleArguments === undefined + ? {} + : (body.sampleArguments as Record); + try { const component = await store.save({ - slug: body.slug.trim(), - title: body.title.trim(), - description: body.description ?? "", - html: body.html ?? "", - css: body.css ?? "", - jsFunctions: body.jsFunctions ?? "", - argumentSchema: body.argumentSchema ?? {}, - sampleArguments: body.sampleArguments ?? {}, + slug: slug.trim(), + title: title.trim(), + description, + html, + css, + jsFunctions, + argumentSchema, + sampleArguments, by: actorEmail(context), }); return context.json({ component }); diff --git a/server/tests/sandboxed-save-fields.test.ts b/server/tests/sandboxed-save-fields.test.ts new file mode 100644 index 000000000..0e70abdbd --- /dev/null +++ b/server/tests/sandboxed-save-fields.test.ts @@ -0,0 +1,183 @@ +import { describe, expect, test } from "bun:test"; +import type { MiddlewareHandler } from "hono"; +import { Hono } from "hono"; +import type { AppVariables } from "../src/auth/guards"; +import { createSandboxedRoutes } from "../src/components/sandboxed-routes"; + +/** + * The playground save writes text and jsonb columns, so its types are the database's. + * + * Only `slug` and `title` were checked. `description`, `html`, `css` and `jsFunctions` + * flowed in as `?? ""` and `argumentSchema`/`sampleArguments` as `?? {}`, so a number, + * an array or a JSON string travelled into a text/jsonb column and came back as an + * unhandled 500. Absent still means the default; a present value must be its type. + */ + +const ADMIN = { + id: "u1", + email: "admin@openbot.test", + role: "admin", +} as const; + +type Saved = { + slug: string; + title: string; + description: unknown; + html: unknown; + css: unknown; + jsFunctions: unknown; + argumentSchema: unknown; + sampleArguments: unknown; +}; + +function harness() { + const saved: Saved[] = []; + const store = { + list: async () => [], + published: async () => [], + save: async (input: Saved) => { + saved.push(input); + return { name: input.slug, title: input.title }; + }, + } as never; + + const requireUser: MiddlewareHandler<{ Variables: AppVariables }> = async ( + context, + next, + ) => { + context.set("actor", { ...ADMIN }); + await next(); + }; + + const hono = new Hono().route( + "/api/sandboxed", + createSandboxedRoutes(store, requireUser), + ); + return { saved, hono }; +} + +const post = (body: unknown) => ({ + method: "POST", + headers: { "content-type": "application/json" }, + body: typeof body === "string" ? body : JSON.stringify(body), +}); + +const BASE = { slug: "weather_card", title: "Weather card" }; + +describe("saving a sandboxed component's optional fields", () => { + test("stores defaults when only the required fields arrive", async () => { + const { saved, hono } = harness(); + const response = await hono.request("http://t/api/sandboxed", post(BASE)); + + expect(response.status).toBe(200); + expect(saved).toHaveLength(1); + expect(saved[0]).toMatchObject({ + slug: "weather_card", + title: "Weather card", + description: "", + html: "", + css: "", + jsFunctions: "", + argumentSchema: {}, + sampleArguments: {}, + }); + }); + + test("stores well-typed optional fields as given", async () => { + const { saved, hono } = harness(); + const response = await hono.request( + "http://t/api/sandboxed", + post({ + ...BASE, + description: "Shows the sky.", + html: "
", + css: ".a{}", + jsFunctions: "export const x = 1;", + argumentSchema: { type: "object" }, + sampleArguments: { city: "Berlin" }, + }), + ); + + expect(response.status).toBe(200); + expect(saved).toHaveLength(1); + expect(saved[0]).toMatchObject({ + description: "Shows the sky.", + argumentSchema: { type: "object" }, + sampleArguments: { city: "Berlin" }, + }); + }); + + test.each([ + ["description", 123], + ["description", null], + ["description", {}], + ["description", ["text"]], + ["html", 42], + ["html", false], + ["css", 0], + ["css", null], + ["jsFunctions", 7], + ["jsFunctions", {}], + ])( + "refuses a non-string %s with 400 and saves nothing", + async (field, value) => { + const { saved, hono } = harness(); + const response = await hono.request( + "http://t/api/sandboxed", + post({ ...BASE, [field]: value }), + ); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ + error: `The component ${field} must be text.`, + }); + expect(saved).toHaveLength(0); + }, + ); + + test.each([ + ["argumentSchema", "not-an-object"], + ["argumentSchema", 42], + ["argumentSchema", null], + ["argumentSchema", []], + ["argumentSchema", [{ type: "object" }]], + ["sampleArguments", "city=Berlin"], + ["sampleArguments", 7], + ["sampleArguments", null], + ["sampleArguments", []], + ])( + "refuses a non-object %s with 400 and saves nothing", + async (field, value) => { + const { saved, hono } = harness(); + const response = await hono.request( + "http://t/api/sandboxed", + post({ ...BASE, [field]: value }), + ); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ + error: `The component ${field} must be an object.`, + }); + expect(saved).toHaveLength(0); + }, + ); + + test("a full body with one bad field still saves nothing", async () => { + const { saved, hono } = harness(); + const response = await hono.request( + "http://t/api/sandboxed", + post({ + ...BASE, + description: "Shows the sky.", + html: "
", + css: ".a{}", + jsFunctions: "export const x = 1;", + argumentSchema: { type: "object" }, + sampleArguments: "city=Berlin", + }), + ); + + expect(response.status).toBe(400); + expect(saved).toHaveLength(0); + }); +});