From 3e52965292f01d5247aff0655963f3dffe640cf3 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Wed, 2 Sep 2026 18:30:35 +0200 Subject: [PATCH] fix(housekeeping): protect poll destructive actions --- .../content/commands/content-commands.test.ts | 37 +++ .../content/commands/content-commands.ts | 15 +- .../mutation-runtime-database.test.ts | 292 +++++++++++++++++- .../services/mutation-runtime-database.ts | 171 ++++++++-- 4 files changed, 485 insertions(+), 30 deletions(-) diff --git a/src/features/housekeeping/domains/content/commands/content-commands.test.ts b/src/features/housekeeping/domains/content/commands/content-commands.test.ts index 4a0c7595..ef746dd9 100644 --- a/src/features/housekeeping/domains/content/commands/content-commands.test.ts +++ b/src/features/housekeeping/domains/content/commands/content-commands.test.ts @@ -30,11 +30,17 @@ const expected = [ PERMS.EVENTS_EDIT, ], ["content.engagement.poll.change", "poll.change", PERMS.POLLS_EDIT], + ["content.engagement.poll.delete", "poll.change", PERMS.POLLS_EDIT], [ "content.engagement.poll-question.change", "poll-question.change", PERMS.POLLS_EDIT, ], + [ + "content.engagement.poll-question.delete", + "poll-question.change", + PERMS.POLLS_EDIT, + ], ["content.media.photo.delete", "photo.delete", PERMS.PAGES_EDIT], ["content.media.asset.upload", "media.upload", PERMS.PAGES_EDIT], ["content.media.asset.delete", "media.delete", PERMS.PAGES_EDIT], @@ -194,6 +200,37 @@ describe("Content commands", () => { } }); + it("requires reasons for dedicated poll deletion commands only", () => { + const commands = createContentCommands({ execute: vi.fn() } as never); + const byId = new Map(commands.map((command) => [command.id, command])); + + for (const id of [ + "content.engagement.poll.delete", + "content.engagement.poll-question.delete", + ]) { + expect(byId.get(id)?.requiresReason, id).toBe(true); + expect( + byId.get(id)?.input.safeParse({ action: "delete", id: 7 }).success, + id, + ).toBe(true); + expect( + byId.get(id)?.input.safeParse({ action: "update", id: 7 }).success, + id, + ).toBe(false); + } + + for (const id of [ + "content.engagement.poll.change", + "content.engagement.poll-question.change", + ]) { + expect(byId.get(id)?.requiresReason, id).toBe(false); + expect( + byId.get(id)?.input.safeParse({ action: "delete", id: 7 }).success, + id, + ).toBe(false); + } + }); + it("executes the real mutation service with actor-bound authority", async () => { const execute = vi.fn(async () => ({ ok: true as const, diff --git a/src/features/housekeeping/domains/content/commands/content-commands.ts b/src/features/housekeeping/domains/content/commands/content-commands.ts index 2ad78599..ef28887f 100644 --- a/src/features/housekeeping/domains/content/commands/content-commands.ts +++ b/src/features/housekeeping/domains/content/commands/content-commands.ts @@ -38,11 +38,18 @@ const CONTENT_COMMAND_DEFINITIONS = [ PERMS.EVENTS_EDIT, ], ["content.engagement.poll.change", "poll.change", PERMS.POLLS_EDIT], + ["content.engagement.poll.delete", "poll.change", PERMS.POLLS_EDIT, true], [ "content.engagement.poll-question.change", "poll-question.change", PERMS.POLLS_EDIT, ], + [ + "content.engagement.poll-question.delete", + "poll-question.change", + PERMS.POLLS_EDIT, + true, + ], ["content.media.photo.delete", "photo.delete", PERMS.PAGES_EDIT], ["content.media.asset.upload", "media.upload", PERMS.PAGES_EDIT], ["content.media.asset.delete", "media.delete", PERMS.PAGES_EDIT], @@ -129,13 +136,17 @@ function inputForCommand( ): z.ZodType> { if ( id === "content.engagement.event.change" || - id === "content.engagement.event-type.change" + id === "content.engagement.event-type.change" || + id === "content.engagement.poll.change" || + id === "content.engagement.poll-question.change" ) { return eventChangeInput; } if ( id === "content.engagement.event.delete" || - id === "content.engagement.event-type.delete" + id === "content.engagement.event-type.delete" || + id === "content.engagement.poll.delete" || + id === "content.engagement.poll-question.delete" ) { return eventDeleteInput; } diff --git a/src/features/housekeeping/domains/content/services/mutation-runtime-database.test.ts b/src/features/housekeeping/domains/content/services/mutation-runtime-database.test.ts index 8b3bf943..60a1d538 100644 --- a/src/features/housekeeping/domains/content/services/mutation-runtime-database.test.ts +++ b/src/features/housekeeping/domains/content/services/mutation-runtime-database.test.ts @@ -17,13 +17,19 @@ const sqlMocks = vi.hoisted(() => ({ const database = vi.hoisted(() => { let selected: Record = { id: 7, title: "Existing" }; let selectedQueue: Record[] = []; + const insertedValues = vi.fn(async (_values: Record) => [ + { insertId: 8 }, + ]); + const insert = vi.fn((_table: unknown) => ({ values: insertedValues })); const set = vi.fn((values: Record) => ({ where: vi.fn(async () => undefined), values, })); const update = vi.fn(() => ({ set })); - const limit = vi.fn(async () => [selectedQueue.shift() ?? selected]); - const where = vi.fn(() => ({ limit })); + const nextSelected = () => [selectedQueue.shift() ?? selected]; + const limit = vi.fn(async () => nextSelected()); + const lock = vi.fn(async () => nextSelected()); + const where = vi.fn(() => ({ for: lock, limit })); const from = vi.fn(() => ({ where })); const select = vi.fn(() => ({ from })); const execute = vi.fn(async () => undefined); @@ -31,6 +37,9 @@ const database = vi.hoisted(() => { const remove = vi.fn((_table: unknown) => ({ where: deleteWhere })); return { execute, + insert, + insertedValues, + lock, set, update, select, @@ -52,7 +61,9 @@ const database = vi.hoisted(() => { }); vi.mock("drizzle-orm", () => ({ + count: vi.fn(() => "count"), eq: vi.fn(), + inArray: vi.fn(), sql: Object.assign(sqlMocks.tagged, { join: sqlMocks.join, raw: sqlMocks.raw, @@ -72,6 +83,7 @@ vi.mock("@/lib/db", () => { db: { delete: database.delete, execute: database.execute, + insert: database.insert, select: database.select, update: database.update, }, @@ -90,8 +102,9 @@ vi.mock("@/lib/db", () => { WebsiteEventType: namedTable("WebsiteEventType"), WebsiteEventWinner: namedTable("WebsiteEventWinner"), WebsiteHelpCenterCategories: table, - WebsitePoll: table, - WebsitePollQuestion: table, + WebsitePoll: namedTable("WebsitePoll"), + WebsitePollQuestion: namedTable("WebsitePollQuestion"), + WebsitePollVote: namedTable("WebsitePollVote"), WebsiteWriteableBoxes: table, }; }); @@ -136,6 +149,277 @@ describe("Content database mutation runtime partial updates", () => { ]); }); + it("deletes poll votes, questions, and the poll in child-first order", async () => { + database.selected({ + id: 7, + title: "Existing", + description: null, + status: "draft", + showResults: 1, + multipleChoice: 0, + startsAt: null, + endsAt: null, + }); + + await executeContentDatabaseMutation( + "poll.change", + { action: "delete", id: 7 }, + context, + undefined, + ); + + expect(database.removedTables()).toEqual([ + "WebsitePollVote", + "WebsitePollQuestion", + "WebsitePoll", + ]); + }); + + it("deletes question votes before the question and retains its audit snapshot", async () => { + database.selected({ + id: 7, + pollId: 3, + question: "Favourite colour?", + type: "single", + sortOrder: 0, + options: "Red\nBlue", + }); + + const snapshot = await executeContentDatabaseMutation( + "poll-question.change", + { action: "delete", id: 7 }, + context, + undefined, + ); + + expect(database.removedTables()).toEqual([ + "WebsitePollVote", + "WebsitePollQuestion", + ]); + expect(snapshot?.before).toMatchObject({ + id: 7, + pollId: 3, + question: "Favourite colour?", + }); + expect(snapshot?.after).toBeNull(); + }); + + it("rejects moving a question to another poll during update", async () => { + database.selected({ + id: 7, + pollId: 3, + question: "Favourite colour?", + type: "single", + sortOrder: 0, + options: "Red\nBlue", + }); + + await expect( + executeContentDatabaseMutation( + "poll-question.change", + { action: "update", id: 7, pollId: 9, question: "Moved" }, + context, + undefined, + ), + ).rejects.toMatchObject({ code: "VALIDATION" }); + expect(database.set).not.toHaveBeenCalled(); + }); + + it("persists empty canonical options for text questions", async () => { + database.queueSelected({ id: 3, status: "draft" }, { value: 0 }); + + await executeContentDatabaseMutation( + "poll-question.change", + { + action: "create", + pollId: 3, + question: "What should improve?", + type: "text", + sortOrder: 0, + options: " \n ", + }, + context, + undefined, + ); + + expect(database.insertedValues).toHaveBeenCalledWith( + expect.objectContaining({ options: "" }), + ); + expect(database.lock).toHaveBeenCalledWith("update"); + }); + + it("maps duplicate question options to an options field error", async () => { + database.queueSelected({ id: 3, status: "draft" }, { value: 0 }); + + await expect( + executeContentDatabaseMutation( + "poll-question.change", + { + action: "create", + pollId: 3, + question: "Favourite colour?", + type: "single", + sortOrder: 0, + options: "Red\nred", + }, + context, + undefined, + ), + ).rejects.toMatchObject({ + code: "VALIDATION", + fieldErrors: { options: ["errors.validation.invalid"] }, + }); + expect(database.insert).not.toHaveBeenCalled(); + }); + + it("validates a merged poll schedule and maps the error to endsAt", async () => { + database.selected({ + id: 7, + title: "Existing", + description: null, + status: "draft", + showResults: 1, + multipleChoice: 0, + startsAt: new Date("2026-09-03T10:00:00.000Z"), + endsAt: new Date("2026-09-04T10:00:00.000Z"), + }); + + await expect( + executeContentDatabaseMutation( + "poll.change", + { action: "update", id: 7, endsAt: "2026-09-02T10:00:00.000Z" }, + context, + undefined, + ), + ).rejects.toMatchObject({ + code: "VALIDATION", + fieldErrors: { endsAt: ["errors.validation.invalid"] }, + }); + expect(database.set).not.toHaveBeenCalled(); + }); + + it("updates only submitted poll columns after validating merged state", async () => { + database.selected({ + id: 7, + title: "Existing", + description: "Keep this", + status: "draft", + showResults: 1, + multipleChoice: 0, + startsAt: null, + endsAt: null, + }); + + await executeContentDatabaseMutation( + "poll.change", + { action: "update", id: 7, title: "Renamed" }, + context, + undefined, + ); + + const values = database.set.mock.calls[0]?.[0]; + expect(values).toMatchObject({ + title: "Renamed", + updatedAt: expect.any(Date), + }); + for (const omitted of [ + "description", + "status", + "showResults", + "multipleChoice", + "startsAt", + "endsAt", + ]) { + expect(values).not.toHaveProperty(omitted); + } + }); + + it("rejects question 101 without inserting it", async () => { + database.queueSelected({ id: 3, status: "draft" }, { value: 100 }); + + await expect( + executeContentDatabaseMutation( + "poll-question.change", + { + action: "create", + pollId: 3, + question: "Question 101", + type: "single", + options: "Yes\nNo", + }, + context, + undefined, + ), + ).rejects.toMatchObject({ code: "CONFLICT" }); + expect(database.insert).not.toHaveBeenCalled(); + }); + + it("rejects activating a poll with more than 50 questions", async () => { + database.queueSelected( + { + id: 7, + title: "Existing", + description: null, + status: "draft", + showResults: 1, + multipleChoice: 0, + startsAt: null, + endsAt: null, + }, + { value: 51 }, + ); + + await expect( + executeContentDatabaseMutation( + "poll.change", + { action: "update", id: 7, status: "active" }, + context, + undefined, + ), + ).rejects.toMatchObject({ code: "CONFLICT" }); + expect(database.set).not.toHaveBeenCalled(); + }); + + it("rejects question 51 for an active poll without inserting it", async () => { + database.queueSelected({ id: 3, status: "active" }, { value: 50 }); + + await expect( + executeContentDatabaseMutation( + "poll-question.change", + { + action: "create", + pollId: 3, + question: "Question 51", + type: "single", + options: "Yes\nNo", + }, + context, + undefined, + ), + ).rejects.toMatchObject({ code: "CONFLICT" }); + expect(database.insert).not.toHaveBeenCalled(); + }); + + it("canonicalizes submitted question options and preserves omitted columns", async () => { + database.selected({ + id: 7, + pollId: 3, + question: "Favourite colour?", + type: "single", + sortOrder: 0, + options: "Red\nBlue", + }); + + await executeContentDatabaseMutation( + "poll-question.change", + { action: "update", id: 7, options: " Green \nYellow " }, + context, + undefined, + ); + + expect(database.set).toHaveBeenCalledWith({ options: "Green\nYellow" }); + }); + it("blocks deletion of an event type referenced by an event", async () => { database.queueSelected({ id: 7, name: "Tournament" }, { id: 11 }); diff --git a/src/features/housekeeping/domains/content/services/mutation-runtime-database.ts b/src/features/housekeeping/domains/content/services/mutation-runtime-database.ts index 229f3ff3..b9dd75dd 100644 --- a/src/features/housekeeping/domains/content/services/mutation-runtime-database.ts +++ b/src/features/housekeeping/domains/content/services/mutation-runtime-database.ts @@ -1,6 +1,6 @@ import "server-only"; -import { eq, type SQL, sql } from "drizzle-orm"; +import { count, eq, inArray, type SQL, sql } from "drizzle-orm"; import type { ResultSetHeader } from "mysql2"; import { db, @@ -21,10 +21,12 @@ import { WebsiteHelpCenterCategories, WebsitePoll, WebsitePollQuestion, + WebsitePollVote, WebsiteWriteableBoxes, } from "@/lib/db"; import { slugify } from "@/lib/format"; import { canonicalize } from "@/lib/foundation/security"; +import { serializePollOptions } from "@/lib/polls/poll-semantics"; import { logStaffActivity } from "@/lib/services/staff-activity"; import { createEventSchema, @@ -35,6 +37,7 @@ import { } from "@/lib/validators/event"; import { createPollSchema, + pollQuestionPatchSchema, pollQuestionSchema, updatePollSchema, } from "@/lib/validators/poll"; @@ -58,10 +61,17 @@ function record(value: unknown): Record { return value as Record; } -function validation(): ContentMutationFailure { +function validation(error?: import("zod").ZodError): ContentMutationFailure { + const fieldErrors: Record = {}; + for (const issue of error?.issues ?? []) { + fieldErrors[String(issue.path[0] ?? "input")] = [ + "errors.validation.invalid", + ]; + } return new ContentMutationFailure( "VALIDATION", "errors.housekeeping.validation", + Object.keys(fieldErrors).length > 0 ? fieldErrors : undefined, ); } @@ -559,6 +569,35 @@ async function eventWinnerAdd( }; } +const POLL_EDITABLE_KEYS = [ + "title", + "description", + "status", + "showResults", + "multipleChoice", + "startsAt", + "endsAt", +] as const; + +const POLL_QUESTION_EDITABLE_KEYS = [ + "question", + "type", + "sortOrder", + "options", +] as const; + +async function pollQuestionCount( + connection: ContentDatabase, + pollId: number, +): Promise { + const [row] = await connection + .select({ value: count() }) + .from(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.pollId, pollId)) + .limit(1); + return Number(row?.value ?? 0); +} + async function pollChange( input: unknown, transaction: unknown, @@ -568,7 +607,7 @@ async function pollChange( const connection = database(transaction); if (action === "create") { const parsed = createPollSchema.safeParse(data); - if (!parsed.success) throw validation(); + if (!parsed.success) throw validation(parsed.error); const [result] = await connection .insert(WebsitePoll) .values({ ...parsed.data, updatedAt: new Date() }); @@ -584,31 +623,64 @@ async function pollChange( .select({ id: WebsitePoll.id, title: WebsitePoll.title, + description: WebsitePoll.description, status: WebsitePoll.status, + showResults: WebsitePoll.showResults, + multipleChoice: WebsitePoll.multipleChoice, + startsAt: WebsitePoll.startsAt, + endsAt: WebsitePoll.endsAt, }) .from(WebsitePoll) .where(eq(WebsitePoll.id, id)) .limit(1); if (!existing) throw notFound(); if (action === "delete") { + await connection + .delete(WebsitePollVote) + .where( + inArray( + WebsitePollVote.questionId, + connection + .select({ id: WebsitePollQuestion.id }) + .from(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.pollId, id)), + ), + ); + await connection + .delete(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.pollId, id)); await connection.delete(WebsitePoll).where(eq(WebsitePoll.id, id)); return { before: existing, after: null }; } if (action !== "update") throw validation(); - const parsed = updatePollSchema.safeParse(data); - if (!parsed.success) throw validation(); - const { - action: _action, - id: _id, - ...values - } = parsed.data as Record; + const patchInput: Record = {}; + for (const key of POLL_EDITABLE_KEYS) { + if (hasOwn(data, key)) patchInput[key] = data[key]; + } + requirePatch(patchInput); + const parsedPatch = updatePollSchema.safeParse(patchInput); + if (!parsedPatch.success) throw validation(parsedPatch.error); + const values: Partial = {}; + for (const key of POLL_EDITABLE_KEYS) { + if (hasOwn(patchInput, key)) { + (values as Record)[key] = parsedPatch.data[key]; + } + } + const merged = createPollSchema.safeParse({ ...existing, ...values }); + if (!merged.success) throw validation(merged.error); + if ( + merged.data.status === "active" && + (await pollQuestionCount(connection, id)) > 50 + ) { + throw conflict(); + } await connection .update(WebsitePoll) .set({ ...values, updatedAt: new Date() }) .where(eq(WebsitePoll.id, id)); return { before: existing, - after: { id, ...values }, + after: { id, ...merged.data }, output: { id: String(id) }, }; } @@ -621,11 +693,29 @@ async function pollQuestionChange( const action = text(data.action, 16, true); const connection = database(transaction); if (action === "create") { + const pollId = positiveInteger(data.pollId); + const [parent] = await connection + .select({ id: WebsitePoll.id, status: WebsitePoll.status }) + .from(WebsitePoll) + .where(eq(WebsitePoll.id, pollId)) + .for("update"); + if (!parent) throw notFound(); + const questionCount = await pollQuestionCount(connection, pollId); + if ( + questionCount >= 100 || + (parent.status === "active" && questionCount >= 50) + ) { + throw conflict(); + } const parsed = pollQuestionSchema.safeParse(data); - if (!parsed.success) throw validation(); + if (!parsed.success) throw validation(parsed.error); + const values = { + ...parsed.data, + options: serializePollOptions(parsed.data.type, parsed.data.options), + }; const [result] = await connection .insert(WebsitePollQuestion) - .values(parsed.data); + .values(values); const id = insertedId(result); return { before: null, @@ -634,27 +724,60 @@ async function pollQuestionChange( }; } const id = positiveInteger(data.id); + const [existing] = await connection + .select({ + id: WebsitePollQuestion.id, + pollId: WebsitePollQuestion.pollId, + question: WebsitePollQuestion.question, + type: WebsitePollQuestion.type, + sortOrder: WebsitePollQuestion.sortOrder, + options: WebsitePollQuestion.options, + }) + .from(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.id, id)) + .limit(1); + if (!existing) throw notFound(); if (action === "delete") { + await connection + .delete(WebsitePollVote) + .where(eq(WebsitePollVote.questionId, id)); await connection .delete(WebsitePollQuestion) .where(eq(WebsitePollQuestion.id, id)); - return { before: { id }, after: null }; + return { before: existing, after: null }; + } + if (action !== "update" || hasOwn(data, "pollId")) throw validation(); + const patchInput: Record = {}; + for (const key of POLL_QUESTION_EDITABLE_KEYS) { + if (hasOwn(data, key)) patchInput[key] = data[key]; + } + requirePatch(patchInput); + const parsedPatch = pollQuestionPatchSchema.safeParse(patchInput); + if (!parsedPatch.success) throw validation(parsedPatch.error); + const merged = pollQuestionSchema.safeParse({ + ...existing, + ...parsedPatch.data, + }); + if (!merged.success) throw validation(merged.error); + const values: Partial = {}; + for (const key of POLL_QUESTION_EDITABLE_KEYS) { + if (hasOwn(patchInput, key)) { + (values as Record)[key] = merged.data[key]; + } + } + if (hasOwn(patchInput, "options")) { + values.options = serializePollOptions( + merged.data.type, + merged.data.options, + ); } - if (action !== "update") throw validation(); - const parsed = pollQuestionSchema.partial().safeParse(data); - if (!parsed.success) throw validation(); - const { - action: _action, - id: _id, - ...values - } = parsed.data as Record; await connection .update(WebsitePollQuestion) .set(values) .where(eq(WebsitePollQuestion.id, id)); return { - before: { id }, - after: { id, ...values }, + before: existing, + after: { ...existing, ...values }, output: { id: String(id) }, }; }