From fe34d4ac932a7c8d66e6ea331177b8cad7f0fb82 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Sun, 13 Sep 2026 20:14:32 +0200 Subject: [PATCH] fix(news): enforce shared comment publication and moderation rules --- docs/operations/security-review-2026-09-13.md | 2 +- docs/testing/database-integration.md | 4 +- integration/database.test.ts | 128 +++++++++ src/actions/article-comments.ts | 114 +++----- src/app/api/articles/[slug]/comment/route.ts | 88 +++--- .../article-comment-submission.test.ts | 257 ++++++++++++++++++ .../services/article-comment-submission.ts | 87 ++++++ 7 files changed, 554 insertions(+), 126 deletions(-) create mode 100644 src/lib/services/article-comment-submission.test.ts create mode 100644 src/lib/services/article-comment-submission.ts diff --git a/docs/operations/security-review-2026-09-13.md b/docs/operations/security-review-2026-09-13.md index 1de7d2c4..cde4a68d 100644 --- a/docs/operations/security-review-2026-09-13.md +++ b/docs/operations/security-review-2026-09-13.md @@ -28,4 +28,4 @@ The additional `withNitroStaff` code and its tests mentioned in the supplied por ## Follow-up identified during verification -The article-comment REST endpoint needs a separate review of publication visibility and moderation parity with the website form. This was discovered while enumerating bearer consumers; it is not silently treated as covered by the supplied review. +The separate comment review is now implemented: both the website form and REST API use one submission service, require a published article whose publication time is due, apply the same moderation, and share a five-attempt/30-second per-user quota. The publication check locks the current article in the insertion transaction. Regression tests cover both entrypoints; real MariaDB/Redis coverage includes publication eligibility, word filtering and alternating submissions. Moderation retains its existing fail-open behavior on service outages. Form input beyond 255 characters is now rejected instead of truncated, and temporary API storage failures return 503. Real integration execution remains a required CI check. diff --git a/docs/testing/database-integration.md b/docs/testing/database-integration.md index 3c5f9925..52aea00e 100644 --- a/docs/testing/database-integration.md +++ b/docs/testing/database-integration.md @@ -2,7 +2,7 @@ Run `pnpm test:integration` on a Docker-capable host. Missing Docker or failed container setup fails the suite. CI runs this check before deployment. -Fourteen tests exercise the production database commands, news actions, public article query and delivery worker against MariaDB 11.4.5 and Redis 7.4.2: +Sixteen database tests exercise the production database commands, news actions, public article query and delivery worker against MariaDB 11.4.5 and Redis 7.4.2: - Migration CLI replay/status, committed catalog bulk edits and undo history, complete rollback after an audit insert fails, and competing catalog previews. - Real Redis expiry metadata and cache-key isolation, concurrent request idempotency, operation/outbox rollback, and exclusive delivery claims. @@ -20,4 +20,4 @@ The fixture models the emulator's catalog/audit baseline plus the legacy article These are application-service integration tests, not browser or HTTP end-to-end tests: they do not start a Next.js server, render the news page, verify login/ACL behavior, or send external webhooks. The cache outage test closes and restores the actual application Redis connection; it does not stop the Redis server or model a multi-host network partition. Passing TypeScript or unit tests without Docker is not a passing result for this suite. -Public comment pagination and reaction aggregation are covered by unit tests using the production Drizzle query builder with a substituted database transport, plus server-rendered page tests with substituted service results. This integration fixture does not create the users, article-comments or article-reactions tables and does not execute those two queries against MariaDB. The article-cache upgrade test verifies that legacy cached absence is ignored under the versioned key; it is a unit test, separate from the real Redis publication and delivery checks above. +Public comment pagination and reaction aggregation are covered by unit tests using the production Drizzle query builder with a substituted database transport, plus server-rendered page tests with substituted service results. This integration fixture does not create users or article-reactions tables and does not execute the public pagination and aggregation queries against MariaDB; its article-comments table is used for submission tests. Comment submission now additionally uses real MariaDB and Redis to verify publication eligibility, filtering and the shared quota across the actual form/API handlers, with authentication replaced at the boundary. The article-cache upgrade test verifies that legacy cached absence is ignored under the versioned key; it is a unit test, separate from the real Redis publication and delivery checks above. diff --git a/integration/database.test.ts b/integration/database.test.ts index 5b7b881a..ed440d8f 100644 --- a/integration/database.test.ts +++ b/integration/database.test.ts @@ -42,6 +42,8 @@ vi.mock("next/navigation", () => ({ }, })); vi.mock("@/lib/services/webhook", () => ({ notify: boundaries.notify })); +vi.mock("@/lib/auth", () => ({ auth: async () => ({ user: { id: "7" } }) })); +vi.mock("@/lib/api-auth", () => ({ bearerUserId: async () => 7 })); const exec = promisify(execFile); const NEWS_REVISION_KEY = "cms:news:revision"; @@ -63,6 +65,10 @@ let operations: typeof import("@/features/operations/server"); let cache: typeof import("@/lib/cache"); let articles: typeof import("@/actions/admin-articles"); let publicNews: typeof import("@/lib/services/news-detail"); +let commentSubmission: typeof import("@/lib/services/article-comment-submission"); +let commentModeration: typeof import("@/lib/services/moderation"); +let commentAction: typeof import("@/actions/article-comments"); +let commentApi: typeof import("@/app/api/articles/[slug]/comment/route"); let scheduler: typeof import("@/lib/services/news-scheduler"); let worker: typeof import("@/features/operations/worker"); let migrationRoot: string | undefined; @@ -143,6 +149,7 @@ beforeAll(async () => { process.env.DATABASE_URL = databaseUrl; process.env.REDIS_URL = `redis://:${redisPassword}@${redisContainer.getHost()}:${redisContainer.getMappedPort(6379)}/0`; delete process.env.SKIP_ENV_VALIDATION; + delete process.env.OPENAI_API_KEY; Object.assign(process.env, { NODE_ENV: "test" }); process.env.HOTEL_NAME = "Integration"; @@ -158,6 +165,12 @@ beforeAll(async () => { await connection.query( "CREATE TABLE website_articles (id BIGINT UNSIGNED AUTO_INCREMENT PRIMARY KEY, slug VARCHAR(255) NOT NULL UNIQUE, title VARCHAR(255) NOT NULL, short_story VARCHAR(255) NOT NULL, full_story LONGTEXT NOT NULL, user_id INT NULL, image VARCHAR(255) NOT NULL, created_at TIMESTAMP NULL DEFAULT NULL, updated_at TIMESTAMP NULL DEFAULT NULL) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4", ); + await connection.query( + "CREATE TABLE website_article_comments (id BIGINT UNSIGNED AUTO_INCREMENT PRIMARY KEY, article_id BIGINT UNSIGNED NOT NULL, user_id INT NOT NULL, comment VARCHAR(255) NOT NULL, created_at TIMESTAMP NULL, updated_at TIMESTAMP NULL) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4", + ); + await connection.query( + "CREATE TABLE website_wordfilter (id BIGINT UNSIGNED AUTO_INCREMENT PRIMARY KEY, word VARCHAR(255) NOT NULL UNIQUE, created_at TIMESTAMP NULL, updated_at TIMESTAMP NULL) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4", + ); // The emulator owns core tables. Exercise the real CMS migration CLI over this baseline. migrationRoot = await mkdtemp(join(resolve("integration"), ".migration-")); await mkdir(join(migrationRoot, "scripts")); @@ -188,6 +201,10 @@ beforeAll(async () => { operations = await import("@/features/operations/server"); articles = await import("@/actions/admin-articles"); publicNews = await import("@/lib/services/news-detail"); + commentSubmission = await import("@/lib/services/article-comment-submission"); + commentModeration = await import("@/lib/services/moderation"); + commentAction = await import("@/actions/article-comments"); + commentApi = await import("@/app/api/articles/[slug]/comment/route"); scheduler = await import("@/lib/services/news-scheduler"); worker = await import("@/features/operations/worker"); }); @@ -226,6 +243,14 @@ beforeEach(async () => { ); await connection.query("DELETE FROM website_article_revisions"); await connection.query("DELETE FROM website_article_drafts"); + await connection.query("DELETE FROM website_article_comments"); + await connection.query("DELETE FROM website_wordfilter"); + commentModeration.reloadWordFilter(); + await appRedis?.del( + "ratelimit:comment:7", + "ratelimit:comment:8", + "ratelimit:comment:9", + ); await connection.query("DELETE FROM website_articles"); await connection.query("DELETE FROM cms_outbox"); await connection.query("DELETE FROM cms_operations"); @@ -862,3 +887,106 @@ describe("real news publication, scheduling and cache delivery", () => { } }); }); + +async function seedCommentArticles() { + if (!connection) throw Error("Integration database is not connected"); + await connection.query( + "INSERT INTO website_articles (id,slug,title,short_story,full_story,image,status,publish_at) VALUES (101,'comment-public','Public','','','','published',NULL),(102,'comment-draft','Draft','','','','draft',NULL),(103,'comment-future','Future','','','','published',DATE_ADD(UTC_TIMESTAMP(), INTERVAL 1 DAY)),(104,'comment-due','Due','','','','published',DATE_SUB(UTC_TIMESTAMP(), INTERVAL 1 DAY))", + ); +} + +describe("real comment publication, moderation and shared quota", () => { + it("checks both target forms with MariaDB and rejects blocked text through the real word filter", async () => { + await seedCommentArticles(); + for (const userId of [8, 9]) { + for (const [id, slug, eligible] of [ + [101, "comment-public", true], + [102, "comment-draft", false], + [103, "comment-future", false], + [104, "comment-due", true], + ] as const) { + const result = await commentSubmission.submitArticleComment({ + userId, + target: userId === 8 ? { id: String(id) } : { slug }, + comment: "Database verified", + }); + expect(result).toEqual( + eligible ? { ok: true, slug } : { ok: false, reason: "not_found" }, + ); + } + } + expect( + await rows( + "SELECT article_id,user_id,comment FROM website_article_comments ORDER BY id", + ), + ).toEqual([ + { article_id: "101", user_id: 8, comment: "Database verified" }, + { article_id: "104", user_id: 8, comment: "Database verified" }, + { article_id: "101", user_id: 9, comment: "Database verified" }, + { article_id: "104", user_id: 9, comment: "Database verified" }, + ]); + await connection?.query( + "INSERT INTO website_wordfilter (word) VALUES ('blocked-content')", + ); + commentModeration.reloadWordFilter(); + expect( + await commentSubmission.submitArticleComment({ + userId: 8, + target: { slug: "comment-public" }, + comment: "BLOCKED-CONTENT", + }), + ).toEqual({ ok: false, reason: "moderated" }); + expect(await rows("SELECT id FROM website_article_comments")).toHaveLength( + 4, + ); + }); + + it("shares the Redis bucket when alternating the real form and API handlers", async () => { + if (!appRedis) throw Error("Redis must be enabled in integration tests"); + await seedCommentArticles(); + const form = new FormData(); + form.set("articleId", "101"); + form.set("slug", "comment-public"); + form.set("comment", "Shared quota"); + form.set("userId", "999"); + const api = () => + commentApi.POST( + new Request("https://hotel.test/api/articles/comment-public/comment", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ comment: "Shared quota", userId: 999 }), + }), + { params: Promise.resolve({ slug: "comment-public" }) }, + ); + for (let attempt = 0; attempt < 5; attempt++) { + if (attempt % 2 === 0) { + await expect(commentAction.postComment(form)).rejects.toThrow( + "/news/comment-public?comment=posted", + ); + } else { + const response = await api(); + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ ok: true }); + } + } + const limited = await api(); + expect(limited.status).toBe(429); + expect(Number(limited.headers.get("Retry-After"))).toBeGreaterThan(0); + await expect(commentAction.postComment(form)).rejects.toThrow( + "/news/comment-public?error=ratelimit", + ); + expect(await appRedis.get("ratelimit:comment:7")).toBe("7"); + expect(await appRedis.pttl("ratelimit:comment:7")).toBeGreaterThan(0); + expect(await appRedis.pttl("ratelimit:comment:7")).toBeLessThanOrEqual( + 30_000, + ); + expect( + await rows("SELECT user_id,comment FROM website_article_comments"), + ).toEqual( + Array.from({ length: 5 }, () => ({ + user_id: 7, + comment: "Shared quota", + })), + ); + }); +}); diff --git a/src/actions/article-comments.ts b/src/actions/article-comments.ts index 50046204..94eda359 100644 --- a/src/actions/article-comments.ts +++ b/src/actions/article-comments.ts @@ -1,15 +1,11 @@ "use server"; -import { eq } from "drizzle-orm"; import { revalidatePath } from "next/cache"; import { redirect } from "next/navigation"; import { auth } from "@/lib/auth"; -import { db, WebsiteArticleComments, WebsiteArticles } from "@/lib/db"; -import { clientIp, rateLimit } from "@/lib/rate-limit"; -import { isAllowed } from "@/lib/services/moderation"; - -// website_article_comments.comment is VARCHAR(255); keep the write within bounds. -const COMMENT_MAX = 255; +import { sessionUserId } from "@/lib/auth/session-user"; +import { logger } from "@/lib/logger"; +import { submitArticleComment } from "@/lib/services/article-comment-submission"; type CommentOutcome = | "posted" @@ -31,86 +27,54 @@ function isNextRedirect(e: unknown): boolean { !!e && typeof e === "object" && "digest" in e && - typeof (e as { digest?: unknown }).digest === "string" && - (e as { digest: string }).digest.startsWith("NEXT_REDIRECT") + typeof e.digest === "string" && + e.digest.startsWith("NEXT_REDIRECT") ); } -/** - * Post a comment on a news article as the SIGNED-IN user. The author id is read - * from the session (re-fetched via auth()), never from the submitted FormData, - * so a crafted form cannot post as another account. The articleId comes from the - * form and is validated as a BigInt (website_articles.id is UNSIGNED BIGINT). - * - * Errors redirect back with a machine-readable ?error= code; success redirects - * with ?comment=posted. - */ +/** The signed-in session determines ownership; form-supplied author IDs are ignored. */ export async function postComment(formData: FormData): Promise { - const slugHint = String(formData.get("slug") ?? "") + let slug = String(formData.get("slug") ?? "") .normalize("NFC") .trim(); - let outcome: CommentOutcome = "error"; - let slug = slugHint; - try { const session = await auth(); - if (!session?.user?.id) { - redirect("/login"); - } - - const userId = Number(session.user.id); - if (!Number.isFinite(userId) || userId <= 0) { - redirect("/login"); - } - - await clientIp(); - if (!(await rateLimit(`comment:${userId}`, 5, 30_000)).ok) { - outcome = "ratelimit"; - } else { - const comment = String(formData.get("comment") ?? "") - .normalize("NFC") - .trim() - .slice(0, COMMENT_MAX); - if (!comment) { - outcome = "empty"; - } else if (!(await isAllowed(comment)).ok) { - outcome = "moderated"; - } else { - const articleIdRaw = String(formData.get("articleId") ?? "") + const userId = sessionUserId(session?.user?.id); + if (!userId) redirect("/login"); + const result = await submitArticleComment({ + userId, + target: { + id: String(formData.get("articleId") ?? "") .normalize("NFC") - .trim(); - if (!/^\d+$/.test(articleIdRaw)) { - outcome = "invalid"; - } else { - const articleId = BigInt(articleIdRaw); - const [article] = await db - .select({ slug: WebsiteArticles.slug }) - .from(WebsiteArticles) - .where(eq(WebsiteArticles.id, articleId)) - .limit(1); - if (!article) { - outcome = "not_found"; - } else { - slug = article.slug; - const now = new Date(); - await db.insert(WebsiteArticleComments).values({ - articleId, - userId, - comment, - createdAt: now, - updatedAt: now, - }); - outcome = "posted"; - } - } - } + .trim(), + }, + comment: formData.get("comment"), + }); + if (result.ok) { + slug = result.slug; + outcome = "posted"; + } else { + outcome = result.reason === "too_long" ? "invalid" : result.reason; } - } catch (e) { - if (isNextRedirect(e)) throw e; - outcome = "error"; + } catch (error) { + if (isNextRedirect(error)) throw error; + // Driver errors can include SQL and comment text: retain only safe context. + logger.error("Article comment submission failed", { + module: "article-comments", + channel: "site", + }); } - if (slug) revalidatePath(`/news/${slug}`); + if (outcome === "posted") { + try { + revalidatePath(`/news/${slug}`); + } catch { + logger.error("Article comment refresh failed", { + module: "article-comments", + channel: "site", + }); + } + } commentRedirect(slug, outcome); } diff --git a/src/app/api/articles/[slug]/comment/route.ts b/src/app/api/articles/[slug]/comment/route.ts index 01b33385..e963dc66 100644 --- a/src/app/api/articles/[slug]/comment/route.ts +++ b/src/app/api/articles/[slug]/comment/route.ts @@ -1,60 +1,52 @@ -// Public REST API — post a comment on an article as the Bearer-authed user. -// -// POST /api/articles/:slug/comment — looks up the website_article by slug for -// its id, then inserts a website_article_comments row owned by the user behind -// the Authorization: Bearer token. Comment is required, non-empty, max 255 -// chars (matches the VARCHAR(255) column). Fails soft — never returns a 500 for -// DB issues, just a generic error envelope. - -import { eq } from "drizzle-orm"; -import { apiError, apiJson } from "@/lib/api"; +// Public REST API: comments belong to the authenticated Bearer token's user. +import { apiError, apiJson, apiUnavailable } from "@/lib/api"; import { bearerUserId } from "@/lib/api-auth"; -import { db, WebsiteArticleComments, WebsiteArticles } from "@/lib/db"; -import { rateLimit } from "@/lib/rate-limit"; +import { logger } from "@/lib/logger"; +import { submitArticleComment } from "@/lib/services/article-comment-submission"; export async function POST( req: Request, { params }: { params: Promise<{ slug: string }> }, ) { - const uid = await bearerUserId(req, ["articles:write"]); - if (!uid) return apiError("Unauthorized", 401); - - if (!(await rateLimit(`article-comment:${uid}`, 10, 60_000)).ok) { - return apiError("Too many comments. Please wait a minute.", 429); - } - - const { slug } = await params; - - const body = (await req.json().catch(() => ({}))) as { comment?: unknown }; - const comment = typeof body.comment === "string" ? body.comment.trim() : ""; - if (!comment) { - return apiError("Comment is required", 422); - } - if (comment.length > 255) { - return apiError("Comment may not be longer than 255 characters", 422); - } - try { - const [article] = await db - .select({ id: WebsiteArticles.id }) - .from(WebsiteArticles) - .where(eq(WebsiteArticles.slug, slug)) - .limit(1); - if (!article) { - return apiError("Article not found", 404); - } - - const now = new Date(); - await db.insert(WebsiteArticleComments).values({ - articleId: article.id, + const uid = await bearerUserId(req, ["articles:write"]); + if (!uid) return apiError("Unauthorized", 401); + const { slug } = await params; + const body: unknown = await req.json().catch(() => null); + const result = await submitArticleComment({ userId: uid, - comment, - createdAt: now, - updatedAt: now, + target: { slug }, + comment: + body && typeof body === "object" && "comment" in body + ? body.comment + : undefined, }); - - return apiJson({ ok: true }); + if (result.ok) return apiJson({ ok: true }); + switch (result.reason) { + case "ratelimit": { + const response = apiError( + "Too many comments. Please wait a minute.", + 429, + ); + response.headers.set("Retry-After", String(result.retryAfter ?? 30)); + return response; + } + case "empty": + return apiError("Comment is required", 422); + case "too_long": + return apiError("Comment may not be longer than 255 characters", 422); + case "moderated": + return apiError("Comment was blocked by moderation", 422); + case "invalid": + case "not_found": + return apiError("Article not found", 404); + } } catch { - return apiError("Could not post comment", 400); + // Driver errors may contain SQL and user content; log a safe diagnostic. + logger.error("Article comment submission failed", { + module: "article-comments", + channel: "api", + }); + return apiUnavailable("Could not post comment"); } } diff --git a/src/lib/services/article-comment-submission.test.ts b/src/lib/services/article-comment-submission.test.ts new file mode 100644 index 00000000..5e4f8160 --- /dev/null +++ b/src/lib/services/article-comment-submission.test.ts @@ -0,0 +1,257 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const state = vi.hoisted(() => ({ + userId: 100, + sessionId: "100" as string | null, + article: { + id: "12", + slug: "public-news", + status: "published", + publishAt: null as Date | null, + }, + words: [] as string[], + queries: [] as string[], + writes: [] as unknown[][], + transactions: 0, + lockedWrites: [] as boolean[], + inTransaction: false, + failRead: false, + failWrite: false, + withdrawDuringModeration: false, + log: vi.fn(), + revalidate: vi.fn(), +})); + +vi.mock("@/lib/db", async () => { + const schema = await import("@/db/schema"); + const { drizzle } = await import("drizzle-orm/mysql-proxy"); + const db = drizzle(async (query, parameters) => { + state.queries.push(query); + if (query.includes(" from `website_wordfilter`")) { + if (state.withdrawDuringModeration) state.article.status = "draft"; + return { rows: state.words.map((word) => [word]) }; + } + if ( + query.startsWith("select ") && + query.includes(" from `website_articles`") + ) { + if (state.failRead) throw new Error("private SQL and comment payload"); + if ( + query.includes("`status` = ?") && + state.article.status !== "published" + ) + return { rows: [] }; + if ( + query.includes("<= NOW()") && + state.article.publishAt && + state.article.publishAt > new Date() + ) + return { rows: [] }; + if ( + String(parameters[0]) !== state.article.id && + parameters[0] !== state.article.slug + ) + return { rows: [] }; + const columns = query + .slice(7, query.indexOf(" from ")) + .split(", ") + .map((column) => column.replaceAll("`", "")); + return { + rows: [columns.map((column) => state.article[column as "id" | "slug"])], + }; + } + if (query.startsWith("insert into `website_article_comments`")) { + if (state.failWrite) throw new Error("private SQL and comment payload"); + state.writes.push(parameters); + state.lockedWrites.push( + state.inTransaction && + state.queries.some((sql) => sql.endsWith("for update")), + ); + return { rows: [{ insertId: 1, affectedRows: 1 }] }; + } + return { rows: [] }; + }); + Object.defineProperty(db, "transaction", { + value: async (callback: (tx: typeof db) => Promise) => { + state.transactions += 1; + state.inTransaction = true; + try { + return await callback(db); + } finally { + state.inTransaction = false; + } + }, + }); + return { ...schema, db }; +}); +vi.mock("@/lib/auth", () => ({ + auth: async () => ({ user: { id: state.sessionId } }), +})); +vi.mock("@/lib/api-auth", () => ({ + bearerUserId: async () => state.userId || null, +})); +vi.mock("@/env", () => ({ env: {} })); +vi.mock("@/lib/redis", () => ({ redis: null })); +vi.mock("@/lib/logger", () => ({ logger: { error: state.log } })); +vi.mock("next/cache", () => ({ revalidatePath: state.revalidate })); +vi.mock("next/navigation", () => ({ + redirect: (url: string) => { + throw Object.assign(new Error(url), { + digest: `NEXT_REDIRECT;replace;${url};307;`, + }); + }, +})); + +import { postComment } from "@/actions/article-comments"; +import { POST } from "@/app/api/articles/[slug]/comment/route"; +import { reloadWordFilter } from "@/lib/services/moderation"; + +function api(comment: unknown = "Hello", slug = "public-news") { + return POST( + new Request("https://hotel.test/api/articles/public-news/comment", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ comment, userId: 999 }), + }), + { params: Promise.resolve({ slug }) }, + ); +} +async function site(comment = "Hello", articleId = "12") { + const form = new FormData(); + form.set("comment", comment); + form.set("articleId", articleId); + form.set("slug", "public-news"); + form.set("userId", "999"); + try { + await postComment(form); + } catch (error) { + if (error instanceof Error && "digest" in error) return error.message; + throw error; + } + throw new Error("Expected action redirect"); +} + +beforeEach(() => { + state.userId += 1; + state.sessionId = String(state.userId); + state.article = { + id: "12", + slug: "public-news", + status: "published", + publishAt: null, + }; + state.words = []; + state.queries = []; + state.writes = []; + state.lockedWrites = []; + state.transactions = 0; + state.inTransaction = false; + state.failRead = false; + state.failWrite = false; + state.withdrawDuringModeration = false; + state.log.mockReset(); + state.revalidate.mockReset(); + reloadWordFilter(); +}); + +describe("article comment entrypoints share publication and abuse policy", () => { + it.each(["draft", "scheduled"])( + "blocks %s articles through both channels", + async (status) => { + state.article.status = status; + expect(await site()).toBe("/news/public-news?error=not_found"); + expect((await api()).status).toBe(404); + expect(state.writes).toHaveLength(0); + }, + ); + it("blocks published articles whose scheduled date is still in the future", async () => { + state.article.publishAt = new Date(Date.now() + 60_000); + expect(await site()).toBe("/news/public-news?error=not_found"); + expect((await api()).status).toBe(404); + expect(state.writes).toHaveLength(0); + }); + it("keeps due articles writable and locks eligibility until each insert", async () => { + state.article.publishAt = new Date(Date.now() - 60_000); + expect(await site()).toBe("/news/public-news?comment=posted"); + const response = await api(); + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ ok: true }); + expect(state.transactions).toBe(2); + expect(state.lockedWrites).toEqual([true, true]); + expect(state.writes).toHaveLength(2); + for (const parameters of state.writes) { + expect(parameters).toContain(state.userId); + expect(parameters).not.toContain(999); + } + }); + it("applies the real configured word filter to both channels", async () => { + state.words = ["forbidden"]; + expect(await site("FORBIDDEN content")).toBe( + "/news/public-news?error=moderated", + ); + expect((await api("FORBIDDEN content")).status).toBe(422); + expect(state.writes).toHaveLength(0); + }); + it("rechecks publication after moderation completes", async () => { + state.withdrawDuringModeration = true; + expect((await api()).status).toBe(404); + expect(state.writes).toHaveLength(0); + }); + it.each(["site", "api"])( + "shares five attempts across channels starting with %s", + async (first) => { + for (let i = 0; i < 5; i++) { + if (first === "site") expect(await site()).toContain("comment=posted"); + else expect((await api()).status).toBe(200); + } + if (first === "site") expect((await api()).status).toBe(429); + else expect(await site()).toBe("/news/public-news?error=ratelimit"); + expect(state.writes).toHaveLength(5); + }, + ); + it("normalizes the same text before moderation and persistence", async () => { + await site(" cafe\u0301 "); + await api(" cafe\u0301 "); + expect(state.writes).toHaveLength(2); + for (const parameters of state.writes) expect(parameters).toContain("café"); + }); + it.each(["failRead", "failWrite"] as const)( + "returns safe recoverable errors for %s", + async (failure) => { + state[failure] = true; + expect(await site()).toBe("/news/public-news?error=error"); + const response = await api(); + expect(response.status).toBe(503); + expect(await response.json()).toEqual({ + error: "Could not post comment", + }); + expect(state.log).toHaveBeenCalledTimes(2); + expect(JSON.stringify(state.log.mock.calls)).not.toContain("private SQL"); + expect(state.writes).toHaveLength(0); + }, + ); + it("rejects missing articles without inserting", async () => { + expect(await site("Hello", "42")).toContain("error=not_found"); + expect((await api("Hello", "missing")).status).toBe(404); + expect(state.writes).toHaveLength(0); + }); + it("keeps sessions mandatory for forms and bearer authentication for API", async () => { + state.userId = 0; + state.sessionId = null; + expect(await site()).toBe("/login"); + expect((await api()).status).toBe(401); + expect(state.writes).toHaveLength(0); + }); + it("rejects oversized input before insertion without silently truncating", async () => { + expect(await site("a".repeat(256))).toContain("error=invalid"); + expect((await api("a".repeat(256))).status).toBe(422); + expect(state.writes).toHaveLength(0); + }); + it("does not turn a committed comment into a retry when revalidation fails", async () => { + state.revalidate.mockImplementation(() => { + throw new Error("cache unavailable"); + }); + expect(await site()).toContain("comment=posted"); + expect(state.writes).toHaveLength(1); + }); +}); diff --git a/src/lib/services/article-comment-submission.ts b/src/lib/services/article-comment-submission.ts new file mode 100644 index 00000000..a67244fc --- /dev/null +++ b/src/lib/services/article-comment-submission.ts @@ -0,0 +1,87 @@ +import "server-only"; +import { and, eq, or, sql } from "drizzle-orm"; +import { db, WebsiteArticleComments, WebsiteArticles } from "@/lib/db"; +import { rateLimit } from "@/lib/rate-limit"; +import { isAllowed } from "@/lib/services/moderation"; + +export type ArticleCommentTarget = { id: string } | { slug: string }; +export type ArticleCommentResult = + | { ok: true; slug: string } + | { + ok: false; + reason: + | "empty" + | "too_long" + | "invalid" + | "moderated" + | "ratelimit" + | "not_found"; + retryAfter?: number; + }; + +/** Authenticated callers share the same quota and uncached publication check. */ +export async function submitArticleComment({ + userId, + target, + comment: rawComment, +}: { + userId: number; + target: ArticleCommentTarget; + comment: unknown; +}): Promise { + if (!Number.isSafeInteger(userId) || userId <= 0) + return { ok: false, reason: "invalid" }; + // Preserve the site's five-attempt burst; API and form consume one bucket. + const limit = await rateLimit(`comment:${userId}`, 5, 30_000); + if (!limit.ok) + return { ok: false, reason: "ratelimit", retryAfter: limit.retryAfter }; + + const comment = + typeof rawComment === "string" ? rawComment.normalize("NFC").trim() : ""; + if (!comment) return { ok: false, reason: "empty" }; + if (comment.length > 255) return { ok: false, reason: "too_long" }; + + let articleId = 0n; + if ("id" in target) { + if (!/^\d{1,20}$/.test(target.id)) return { ok: false, reason: "invalid" }; + articleId = BigInt(target.id); + if (articleId <= 0n || articleId > 18446744073709551615n) + return { ok: false, reason: "invalid" }; + } else if (!target.slug || target.slug.length > 255) { + return { ok: false, reason: "invalid" }; + } + if (!(await isAllowed(comment)).ok) return { ok: false, reason: "moderated" }; + + // Moderate before locking: the optional remote check can take several seconds. + // A locking read sees current publication state and serializes withdrawal/deletion + // with this insert, rather than trusting a prior page or cached article lookup. + return db.transaction(async (tx): Promise => { + const [article] = await tx + .select({ id: WebsiteArticles.id, slug: WebsiteArticles.slug }) + .from(WebsiteArticles) + .where( + and( + "slug" in target + ? eq(WebsiteArticles.slug, target.slug) + : eq(WebsiteArticles.id, articleId), + eq(WebsiteArticles.status, "published"), + or( + sql`${WebsiteArticles.publishAt} IS NULL`, + sql`${WebsiteArticles.publishAt} <= NOW()`, + ), + ), + ) + .limit(1) + .for("update"); + if (!article) return { ok: false, reason: "not_found" }; + const now = new Date(); + await tx.insert(WebsiteArticleComments).values({ + articleId: article.id, + userId, + comment, + createdAt: now, + updatedAt: now, + }); + return { ok: true, slug: article.slug }; + }); +}