diff --git a/src/db/projections.ts b/src/db/projections.ts index dcb249f..c75b30d 100644 --- a/src/db/projections.ts +++ b/src/db/projections.ts @@ -83,7 +83,6 @@ export const printerProjection = projection(printers, [ "cameraStream", "vendorState", "vendorErrorCode", - "metadata", "lastSeenAt", "createdAt", "updatedAt", diff --git a/src/db/validators.ts b/src/db/validators.ts index 5e9daa9..21a50b7 100644 --- a/src/db/validators.ts +++ b/src/db/validators.ts @@ -28,7 +28,14 @@ export const PrinterInsertSchema = createInsertSchema(printers, { * The columns a client may set on a printer, derived from the insert schema * above so a renamed column still fails to compile. * - * Everything left out is owned by the gateway, not by the API: `status`, the + * `metadata` is deliberately not here, and nor is it in the projection. It is + * a free-form jsonb column that nothing reads, so leaving it open meant a + * well-meaning client could store a printer's access code in it and the API + * would serve it back - the same credential that was removed from the schema + * on purpose. The column stays, so reopening it is a one-line change once + * something actually needs it, with a shape to match. + * + * Everything else left out is owned by the gateway, not by the API: `status`, the * temperatures, layer counts, progress, `vendorState`, `lastSeenAt` and the * rest are written only by `persistTelemetry` in routes/ws/edge.ts. A client * that could POST `status: "idle"` could make the scheduler send work to a @@ -47,7 +54,6 @@ export const PRINTER_WRITABLE = [ "lanAddress", "serialNumber", "gatewayPrinterId", - "metadata", ] as const; export const PrinterCreateSchema = t.Pick(PrinterInsertSchema, PRINTER_WRITABLE); diff --git a/tests/authz.test.ts b/tests/authz.test.ts index ca64b0d..ec0c008 100644 --- a/tests/authz.test.ts +++ b/tests/authz.test.ts @@ -6,7 +6,7 @@ import { startDevAuthIssuer } from "../scripts/dev-auth"; import { Value } from "@sinclair/typebox/value"; import { printers } from "../src/db/schema"; import { farmProjection, printerProjection } from "../src/db/projections"; -import { PrinterCreateSchema } from "../src/db/validators"; +import { PRINTER_WRITABLE, PrinterCreateSchema } from "../src/db/validators"; const USER = "11111111-1111-1111-1111-111111111111"; @@ -297,14 +297,24 @@ describe("credentials the backend must not hold", () => { expect(Object.keys(farmProjection.columns)).not.toContain("edgeProxyToken"); }); - test("metadata is the remaining way a secret could reach the cloud", () => { - // Not a bug to fix here, but a hole to know about: `metadata` is a - // free-form jsonb column that is both writable and in the public - // projection, so a well-meaning client could stash an access code in it - // and the API would store it and serve it back. Removing the dedicated - // column narrowed the door; it did not close it. If this assertion ever - // fails because `metadata` left the projection, that is an improvement - - // delete the test. - expect(Object.keys(printerProjection.columns)).toContain("metadata"); + test("printer metadata is neither writable nor served, so it cannot carry a secret", () => { + // `metadata` was the way around the access-code decision: a free-form + // jsonb column that was both writable and in the projection, so a client + // could stash a credential in it and read it back. It is now closed at + // both ends. Pinned as three assertions because either end reopening it + // alone (a writable field nobody can read, or a readable one nobody can + // write) is still half of the hole. + expect(Object.keys(printerProjection.columns)).not.toContain("metadata"); + + const cleaned = Value.Clean(PrinterCreateSchema, { + farmId: "3f2b1c5e-0000-4000-8000-000000000000", + name: "P1P", + vendor: "bambu" as const, + metadata: { accessCode: "SECRET123" }, + }) as Record; + expect(cleaned).not.toHaveProperty("metadata"); + expect(cleaned.name).toBe("P1P"); + + expect(PRINTER_WRITABLE as readonly string[]).not.toContain("metadata"); }); });