Refuse a malformed catalogue entry on PUT /api/components/catalogue with 400

server/src/components/routes.ts

@@ -87,8 +87,29 @@ export function createComponentRoutes(
       return context.json({ error: "A list of components is required." }, 400);
     }
 
-    const valid = entries.flatMap((entry) => {
-      if (!entry || typeof entry !== "object") return [];
+    /*
+     * All or nothing, and a 400 names the entry. This used to drop malformed entries and
+     * answer 200 with whatever was left, so a deploy that typo'd `kind` as an object or
+     * sent a blank `description` got a success response while publishing nothing: `{added: []}`
+     * is also what "already in sync" looks like. The operator found out from a missing
+     * component, not from the API. A build announcing an empty catalogue sends `[]`, which
+     * still syncs to nothing and answers 200.
+     */
+    const valid: {
+      name: string;
+      title: string;
+      kind: string;
+      description: string;
+    }[] = [];
+    for (const [index, entry] of entries.entries()) {
+      if (!entry || typeof entry !== "object") {
+        return context.json(
+          {
+            error: `Component at index ${index} needs a name, a title, a kind and a description.`,
+          },
+          400,
+        );
+      }
       const { name, title, kind, description } = entry as Record<
         string,
         unknown
@@ -103,22 +124,25 @@ export function createComponentRoutes(
         typeof description !== "string" ||
         !description.trim()
       ) {
-        return [];
+        return context.json(
+          {
+            error: `Component at index ${index} needs a name, a title, a kind and a description.`,
+          },
+          400,
+        );
       }
       // Trimmed, because that is the string the guard above just approved. A component's `name` is
       // its identity -- `syncCatalogue` compares it against what is already published, `decide` and
       // `listForAgent` look it up by it, and a grant names it -- so publishing " weatherPanel "
       // adds a second component beside `weatherPanel` that nobody has granted and no Bot can be
       // held back from by the name people use.
-      return [
-        {
-          name: name.trim(),
-          title: title.trim(),
-          kind: kind.trim(),
-          description: description.trim(),
-        },
-      ];
-    });
+      valid.push({
+        name: name.trim(),
+        title: title.trim(),
+        kind: kind.trim(),
+        description: description.trim(),
+      });
+    }
 
     const { added } = await store.syncCatalogue(valid);
     // Only arrivals are recorded. Announcing happens on every page load, and a row per load would

server/tests/component-catalogue-entries.test.ts

@@ -0,0 +1,135 @@
+import { describe, expect, test } from "bun:test";
+import type { MiddlewareHandler } from "hono";
+import { Hono } from "hono";
+import type { AppVariables } from "../src/auth/guards";
+import { createComponentRoutes } from "../src/components/routes";
+import type { CatalogueEntry, ComponentStore } from "../src/components/store";
+
+/**
+ * A build's announcement is a claim about what exists, so a malformed entry is a 400.
+ *
+ * The route used to drop entries that failed the shape check and answer 200 with the rest,
+ * so `{"components": [{"name": 123}, "oops", null]}` returned `{added: []}` — the same body
+ * as "already in sync". A deploy that typo'd a field published nothing and was told success.
+ * An empty list still means "nothing to announce" and answers 200; anything present must
+ * be complete, and the error names its index.
+ */
+
+const asSignedIn: MiddlewareHandler<{ Variables: AppVariables }> = async (
+  context,
+  next,
+) => {
+  context.set("actor", { id: "u1", email: "someone@openbot.test" });
+  return next();
+};
+
+function harness() {
+  const published: CatalogueEntry[] = [];
+  const store = {
+    syncCatalogue: async (entries: CatalogueEntry[]) => {
+      published.push(...entries);
+      return { added: entries.map((entry) => entry.name) };
+    },
+  } as unknown as ComponentStore;
+
+  const app = new Hono().route(
+    "/components",
+    createComponentRoutes(store, asSignedIn, undefined, async () => true),
+  );
+
+  return {
+    published,
+    announce: (components: unknown) =>
+      app.request("http://openbot.local/components/catalogue", {
+        method: "PUT",
+        headers: { "content-type": "application/json" },
+        body: JSON.stringify({ components }),
+      }),
+  };
+}
+
+function entry(over: Record<string, unknown> = {}) {
+  return {
+    name: "weatherPanel",
+    title: "Weather",
+    kind: "panel",
+    description: "The forecast where the reader is.",
+    ...over,
+  };
+}
+
+describe("announcing a catalogue with malformed entries", () => {
+  test("an empty list still syncs to nothing with 200", async () => {
+    const { published, announce } = harness();
+    const response = await announce([]);
+    expect(response.status).toBe(200);
+    await expect(response.json()).resolves.toEqual({ added: [] });
+    expect(published).toEqual([]);
+  });
+
+  test("a two-entry list with one malformed entry syncs neither", async () => {
+    const { published, announce } = harness();
+    const response = await announce([entry(), entry({ kind: {} })]);
+    expect(response.status).toBe(400);
+    await expect(response.json()).resolves.toEqual({
+      error:
+        "Component at index 1 needs a name, a title, a kind and a description.",
+    });
+    expect(published).toEqual([]);
+  });
+
+  test.each([
+    ["a number", 123],
+    ["a string", "oops"],
+    ["null", null],
+    ["an array", []],
+  ])("refuses a non-object entry %s at its index", async (_name, bad) => {
+    const { published, announce } = harness();
+    const response = await announce([entry(), bad]);
+    expect(response.status).toBe(400);
+    await expect(response.json()).resolves.toEqual({
+      error:
+        "Component at index 1 needs a name, a title, a kind and a description.",
+    });
+    expect(published).toEqual([]);
+  });
+
+  test.each([
+    ["a numeric name", { name: 123 }],
+    ["a null title", { title: null }],
+    ["a blank kind", { kind: "   " }],
+    ["an empty description", { description: "" }],
+    ["a missing description", { description: undefined }],
+    ["an object kind", { kind: {} }],
+  ])("refuses an entry with %s at index 0", async (_name, over) => {
+    const { published, announce } = harness();
+    const clean = entry();
+    const body = { ...clean };
+    for (const [key, value] of Object.entries(over)) {
+      if (value === undefined) delete body[key as keyof typeof body];
+      else (body as Record<string, unknown>)[key] = value;
+    }
+    const response = await announce([body]);
+    expect(response.status).toBe(400);
+    await expect(response.json()).resolves.toEqual({
+      error:
+        "Component at index 0 needs a name, a title, a kind and a description.",
+    });
+    expect(published).toEqual([]);
+  });
+
+  test("three valid entries still publish together", async () => {
+    const { published, announce } = harness();
+    const response = await announce([
+      entry(),
+      entry({ name: "newsPanel", title: "News" }),
+      entry({ name: "clockPanel", title: "Clock" }),
+    ]);
+    expect(response.status).toBe(200);
+    expect(published.map((row) => row.name)).toEqual([
+      "weatherPanel",
+      "newsPanel",
+      "clockPanel",
+    ]);
+  });
+});

server/tests/component-catalogue.test.ts

@@ -95,9 +95,14 @@ describe("a build announcing what it can draw", () => {
     expect(published[0]?.kind).toBe("panel");
   });
 
-  test("still refuses an entry that is only whitespace", async () => {
+  test("refuses an entry that is only whitespace instead of dropping it", async () => {
     const { published, announce } = harness();
-    await announce([entry({ name: "   " })]);
+    const response = await announce([entry({ name: "   " })]);
+    expect(response.status).toBe(400);
+    await expect(response.json()).resolves.toEqual({
+      error:
+        "Component at index 0 needs a name, a title, a kind and a description.",
+    });
     expect(published).toEqual([]);
   });