From adf7e6b91c42e67857a7386b086039ac6ae75c2a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 10:18:07 +0000 Subject: [PATCH 1/2] test(web): cover same-origin checks on kit and collection writes The session path of kit and collection writes already refuses a foreign Origin through resolveLimitedScopedActor. Pin it per route group: a foreign Origin or same-site fetch is 403 cross_origin and writes nothing, while same-origin, header-less and API-key writes pass. Collections also cover PATCH and DELETE. Kit origin cases move to their own file to keep kit-revisions under 200 lines. Closes #113 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YPFjLezDdYaFNQwfYGjQvq --- .../collections/collections-origin.test.ts | 47 +++++++++++++++++++ .../tests/personal-kits/kit-origin.test.ts | 47 +++++++++++++++++++ .../tests/personal-kits/kit-revisions.test.ts | 13 ----- 3 files changed, 94 insertions(+), 13 deletions(-) create mode 100644 apps/web/tests/collections/collections-origin.test.ts create mode 100644 apps/web/tests/personal-kits/kit-origin.test.ts diff --git a/apps/web/tests/collections/collections-origin.test.ts b/apps/web/tests/collections/collections-origin.test.ts new file mode 100644 index 0000000..7f1d1be --- /dev/null +++ b/apps/web/tests/collections/collections-origin.test.ts @@ -0,0 +1,47 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { call, create, h, ownerKey } from "./collections-harness"; + +/** + * A collection write in a web session (no `Authorization`) rides the ambient + * cookie, so it must come from this origin; an API key is never ambient and is + * not checked. + */ +const session = vi.hoisted(() => ({ user: null as { id: string } | null })); +vi.mock("~/lib/auth/session-helpers", () => ({ + getSession: async () => (session.user === null ? null : { user: session.user }), +})); + +beforeEach(() => { + session.user = null; +}); + +const count = () => h.d1.raw.prepare("SELECT COUNT(*) AS n FROM collections").get() as { n: number }; + +describe("collections: same-origin web sessions", () => { + it("refuses a cross-origin web-session write on every write method, writing nothing", async () => { + session.user = { id: "user-a" }; + for (const headers of [{ Origin: "https://evil.example" }, { Origin: "https://sub.skillx.test" }, { "Sec-Fetch-Site": "same-site" }]) { + const refused = await create({}, { key: null, headers }); + expect(refused.status).toBe(403); + expect(refused.body.error.code).toBe("cross_origin"); + } + expect(count().n).toBe(0); + + const { id } = (await create({}, { key: ownerKey })).body.collection; + const foreign = { key: null, headers: { Origin: "https://evil.example" } }; + expect((await call(`/api/v1/collections/${id}`, { name: "x" }, { ...foreign, method: "PATCH" })).body.error.code).toBe("cross_origin"); + expect((await call(`/api/v1/collections/${id}`, {}, { ...foreign, method: "DELETE" })).body.error.code).toBe("cross_origin"); + expect((await call(`/api/v1/collections/${id}`, undefined, { key: ownerKey })).body.collection.name).toBe("Starter"); + }); + + it("accepts a same-origin or header-less web-session write", async () => { + session.user = { id: "user-a" }; + expect((await create({}, { key: null, headers: { Origin: "https://skillx.test" } })).status).toBe(201); + expect((await create({ slug: "second" }, { key: null, headers: { "Sec-Fetch-Site": "same-origin" } })).status).toBe(201); + expect((await create({ slug: "third" }, { key: null })).status).toBe(201); + }); + + it("does not check the origin of an API-key write", async () => { + expect((await create({}, { headers: { Origin: "https://evil.example" } })).status).toBe(201); + }); +}); diff --git a/apps/web/tests/personal-kits/kit-origin.test.ts b/apps/web/tests/personal-kits/kit-origin.test.ts new file mode 100644 index 0000000..d3da970 --- /dev/null +++ b/apps/web/tests/personal-kits/kit-origin.test.ts @@ -0,0 +1,47 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { BODY, createKitHarness, skillText } from "./kit-harness"; +import type { KitHarness } from "./kit-harness"; + +/** + * A kit write in a web session (no `Authorization`) rides the ambient cookie, + * so it must come from this origin; an API key is never ambient and is not checked. + */ +const session = vi.hoisted(() => ({ user: null as { id: string } | null })); +vi.mock("~/lib/auth/session-helpers", () => ({ + getSession: async () => (session.user === null ? null : { user: session.user }), +})); + +let k: KitHarness; +const MINE = { "SKILL.md": skillText(BODY.replace("two\n", "two (mine)\n")), "notes.md": "my notes\n" }; + +beforeEach(async () => { + session.user = null; + k = await createKitHarness(); + await k.publish("@alpha/demo", "1.0.0", { "SKILL.md": skillText(BODY) }); +}); + +describe("personal kits: same-origin web sessions", () => { + it("refuses a cross-origin web-session write, then accepts the same write from this origin", async () => { + session.user = { id: "user-a" }; + const write = async (headers: Record) => + k.call("revisions", { expectedGeneration: 0, idempotencyKey: k.idem(), document: k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]) }, { key: null, headers }); + for (const headers of [{ Origin: "https://evil.example" }, { "Sec-Fetch-Site": "same-site" }]) { + const refused = await write(headers); + expect(refused.status).toBe(403); + expect(refused.body.error.code).toBe("cross_origin"); + } + // Nothing was written: generation 0 is still the current one. + expect((await write({ Origin: "https://skillx.test" })).status).toBe(201); + }); + + it("accepts a header-less web-session write", async () => { + session.user = { id: "user-a" }; + const body = { expectedGeneration: 0, idempotencyKey: k.idem(), document: k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]) }; + expect((await k.call("revisions", body, { key: null })).status).toBe(201); + }); + + it("does not check the origin of an API-key write", async () => { + const body = { expectedGeneration: 0, idempotencyKey: k.idem(), document: k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]) }; + expect((await k.call("revisions", body, { headers: { Origin: "https://evil.example" } })).status).toBe(201); + }); +}); diff --git a/apps/web/tests/personal-kits/kit-revisions.test.ts b/apps/web/tests/personal-kits/kit-revisions.test.ts index b7add9d..c11c7f1 100644 --- a/apps/web/tests/personal-kits/kit-revisions.test.ts +++ b/apps/web/tests/personal-kits/kit-revisions.test.ts @@ -75,19 +75,6 @@ describe("personal kit revisions", () => { expect(lock.body.documents).toHaveLength(1); }); - it("refuses a cross-origin web-session write, then accepts the same write from this origin", async () => { - session.user = { id: "user-a" }; - const write = async (headers: Record) => - k.call("revisions", { expectedGeneration: 0, idempotencyKey: k.idem(), document: k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]) }, { key: null, headers }); - for (const headers of [{ Origin: "https://evil.example" }, { "Sec-Fetch-Site": "same-site" }]) { - const refused = await write(headers); - expect(refused.status).toBe(403); - expect(refused.body.error.code).toBe("cross_origin"); - } - // Nothing was written: generation 0 is still the current one. - expect((await write({ Origin: "https://skillx.test" })).status).toBe(201); - }); - it("refuses a client compositionDigest that the pinned bytes do not give, writing nothing", async () => { const bad = { ...(await k.entry("@alpha/demo", "1.0.0", MINE)), compositionDigest: `sha256:${"1".repeat(64)}` }; const res = await k.call("revisions", { expectedGeneration: 0, idempotencyKey: k.idem(), document: k.doc([bad]) }); From fe35e8ed446e1b7dfec6cd0ed21dcb8ad76f9d40 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 10:22:18 +0000 Subject: [PATCH 2/2] test(web): pin the origin check on every kit and collection write sub-route Assert status and error code for every collection and kit write route, so a write later classified as a read cannot drop the check silently. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YPFjLezDdYaFNQwfYGjQvq --- .../collections/collections-origin.test.ts | 17 ++++++++++++++--- apps/web/tests/personal-kits/kit-origin.test.ts | 9 +++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/apps/web/tests/collections/collections-origin.test.ts b/apps/web/tests/collections/collections-origin.test.ts index 7f1d1be..1ba1368 100644 --- a/apps/web/tests/collections/collections-origin.test.ts +++ b/apps/web/tests/collections/collections-origin.test.ts @@ -29,9 +29,20 @@ describe("collections: same-origin web sessions", () => { const { id } = (await create({}, { key: ownerKey })).body.collection; const foreign = { key: null, headers: { Origin: "https://evil.example" } }; - expect((await call(`/api/v1/collections/${id}`, { name: "x" }, { ...foreign, method: "PATCH" })).body.error.code).toBe("cross_origin"); - expect((await call(`/api/v1/collections/${id}`, {}, { ...foreign, method: "DELETE" })).body.error.code).toBe("cross_origin"); - expect((await call(`/api/v1/collections/${id}`, undefined, { key: ownerKey })).body.collection.name).toBe("Starter"); + for (const [path, body, method] of [ + [`/api/v1/collections/${id}`, { name: "x" }, "PATCH"], + [`/api/v1/collections/${id}`, {}, "DELETE"], + [`/api/v1/collections/${id}/revisions`, { expectedRevision: 1, members: [] }, "POST"], + [`/api/v1/collections/${id}/members`, { expectedRevision: 1, remove: ["@alpha/demo"] }, "POST"], + [`/api/v1/collections/${id}/clone`, { slug: "copy" }, "POST"], + ] as const) { + const refused = await call(path, body, { ...foreign, method }); + expect(refused.status).toBe(403); + expect(refused.body.error.code).toBe("cross_origin"); + } + const after = await call(`/api/v1/collections/${id}`, undefined, { key: ownerKey }); + expect(after.body.collection).toMatchObject({ name: "Starter", latestRevisionNumber: 1 }); + expect(count().n).toBe(1); }); it("accepts a same-origin or header-less web-session write", async () => { diff --git a/apps/web/tests/personal-kits/kit-origin.test.ts b/apps/web/tests/personal-kits/kit-origin.test.ts index d3da970..ab3ce4f 100644 --- a/apps/web/tests/personal-kits/kit-origin.test.ts +++ b/apps/web/tests/personal-kits/kit-origin.test.ts @@ -34,6 +34,15 @@ describe("personal kits: same-origin web sessions", () => { expect((await write({ Origin: "https://skillx.test" })).status).toBe(201); }); + it("refuses a cross-origin web-session write on every write sub-route", async () => { + session.user = { id: "user-a" }; + for (const sub of ["proposals", "proposals/validate", "preview-update", "activate", "rollback", "rebase", "conflicts/c-1/resolve", "revisions/rev-1/reject", "revisions/rev-1/review", "revisions/rev-1/quarantine"]) { + const refused = await k.call(sub, {}, { key: null, headers: { Origin: "https://evil.example" } }); + expect(refused.status, sub).toBe(403); + expect(refused.body.error.code, sub).toBe("cross_origin"); + } + }); + it("accepts a header-less web-session write", async () => { session.user = { id: "user-a" }; const body = { expectedGeneration: 0, idempotencyKey: k.idem(), document: k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]) };