From e6b626701bcf14aa3d86b363e41128679bd705ca Mon Sep 17 00:00:00 2001 From: webadderall <131426131+webadderall@users.noreply.github.com> Date: Wed, 23 Sep 2026 17:31:17 +1000 Subject: [PATCH] Enforce server-side feedback quotas and improve submission errors --- biome.json | 4 + services/supabase/README.md | 16 ++- services/supabase/config.toml | 5 + .../functions/submit-feedback/handler.ts | 117 ++++++++++++++++++ .../functions/submit-feedback/index.ts | 41 ++++++ .../202609230002_feedback_limits.sql | 44 +++++++ services/supabase/tests/feedback_limits.sql | 50 ++++++++ src/components/feedback/FeedbackDialog.tsx | 20 ++- src/lib/feedback/diagnostics.test.ts | 22 ++++ src/lib/feedback/diagnostics.ts | 24 ++-- src/lib/feedback/server.test.ts | 86 +++++++++++++ src/lib/feedback/submitFeedback.test.ts | 65 +++++----- src/lib/feedback/submitFeedback.ts | 68 +++++----- tests/ui/feedback.spec.ts | 8 +- 14 files changed, 484 insertions(+), 86 deletions(-) create mode 100644 services/supabase/config.toml create mode 100644 services/supabase/functions/submit-feedback/handler.ts create mode 100644 services/supabase/functions/submit-feedback/index.ts create mode 100644 services/supabase/migrations/202609230002_feedback_limits.sql create mode 100644 services/supabase/tests/feedback_limits.sql create mode 100644 src/lib/feedback/server.test.ts diff --git a/biome.json b/biome.json index 6c8695eb..5ae2a194 100644 --- a/biome.json +++ b/biome.json @@ -108,6 +108,10 @@ "javascript": { "formatter": { "quoteStyle": "double" } }, "css": { "parser": { "cssModules": true, "tailwindDirectives": true } }, "overrides": [ + { + "includes": ["services/supabase/functions/**/*.ts"], + "javascript": { "globals": ["Deno"] } + }, { "includes": ["*.ts", "*.tsx", "*.mts", "*.cts"], "linter": { diff --git a/services/supabase/README.md b/services/supabase/README.md index d5c3d6a9..a0d01030 100644 --- a/services/supabase/README.md +++ b/services/supabase/README.md @@ -1,9 +1,17 @@ # Desktop feedback -Apply `migrations/202609230001_feedback.sql` in the Supabase SQL editor for the project configured by `VITE_SUPABASE_URL` before releasing this feature. The desktop client uses its existing signed-in session; never put a service-role key in the app. +Before releasing feedback submission: -Feedback is stored in `public.feedback_reports`. View reports through the Supabase dashboard and retrieve attachments from the private `feedback-attachments` bucket using each report's attachment paths. There is no email notification or public issue creation. Owners can read their own reports; other users cannot. Staff use server-side service-role access. +1. Apply `migrations/202609230001_feedback.sql` and then `migrations/202609230002_feedback_limits.sql` in the configured Supabase project. If the first migration is already applied, apply only the second. +2. Deploy the `submit-feedback` Edge Function from the repository root with the Supabase CLI: `supabase functions deploy submit-feedback --workdir services --project-ref YOUR_PROJECT_REF`. The function explicitly validates the bearer token with `auth.getUser` before parsing or storing feedback. The function configuration disables only the gateway's JWT check, not the function's authentication. +3. Use the project's built-in `SUPABASE_URL` and `SUPABASE_SERVICE_ROLE_KEY` secrets for the function. Never put a service-role key in the desktop app. -On submission, the modal captures up to 100 recent renderer warnings/errors (2,000 characters each), browser/platform details, and a timestamp. Common secrets, URLs, emails, and local home paths are redacted. Arbitrary log objects are not serialized. Diagnostics are automatically included with each submission. Native-process logs are not collected. Attachments are chosen explicitly, up to five files and 10 MB total in the client, with a 10 MB per-object storage limit. +The desktop client calls the function with its existing signed-in session. Direct client report inserts and attachment uploads are disabled by the second migration. The function validates subject/message lengths, diagnostics bytes, and the actual uploaded files (five nonempty files, 10 MB total per submission), and caps the incoming request body. It reserves a per-account UTC daily quota atomically: 10 submission attempts, 25 files, and 50 MB. Failed attempts still consume quota, so repeated upload/delete/retry cycles do not reset it. No Storage schema triggers or modifications are required beyond the access policies. -A local demo account can preview the form but cannot submit without a real Supabase session. Failed submissions retain the draft in memory; closing and reopening the same modal keeps it, but restarting the app does not. Failed uploads are cleaned up on a best-effort basis; periodically remove unattached objects left by interrupted sessions. +Feedback is stored in `public.feedback_reports`. View reports through the Supabase dashboard and retrieve attachments from the private `feedback-attachments` bucket using each report's attachment paths. Owners can read their own reports; other users cannot. Staff use server-side service-role access. There is no email notification or public issue creation. + +On submission, the modal captures up to 100 recent renderer warnings/errors (2,000 characters each), browser/platform details, and a timestamp. Common secrets, URLs, emails, and local home paths are redacted. Arbitrary log objects are not serialized. The final JSON is bounded to 290,000 UTF-8 bytes by dropping oldest entries, below the database's 300,000-byte limit. Diagnostics are automatically included; native-process logs are not collected. Attachments are chosen explicitly. + +A local demo account can preview the form but cannot submit without a real Supabase session. Failed submissions retain the draft in memory; closing and reopening the same modal keeps it, but restarting the app does not. The function cleans up failed uploads on a best-effort basis. Periodically remove unattached objects left by interrupted sessions through the Storage API, and prune old `feedback_daily_usage` rows after their UTC day has ended. + +Unit tests exercise the server handler with an injected storage/auth backend. `tests/feedback_limits.sql` verifies the migration's quotas and permissions in a disposable database with the migrations applied; it rolls back its test data. Live deployment and end-to-end Supabase uploads must be checked separately before release. diff --git a/services/supabase/config.toml b/services/supabase/config.toml new file mode 100644 index 00000000..cd28e261 --- /dev/null +++ b/services/supabase/config.toml @@ -0,0 +1,5 @@ +project_id = "recordly-feedback" + +[functions.submit-feedback] +# The handler verifies the bearer token with auth.getUser before doing any work. +verify_jwt = false diff --git a/services/supabase/functions/submit-feedback/handler.ts b/services/supabase/functions/submit-feedback/handler.ts new file mode 100644 index 00000000..064a4200 --- /dev/null +++ b/services/supabase/functions/submit-feedback/handler.ts @@ -0,0 +1,117 @@ +const MAX_FILES = 5; +const MAX_BYTES = 10 * 1024 * 1024; +const MAX_REQUEST_BYTES = MAX_BYTES + 1024 * 1024; +const headers = { + "Access-Control-Allow-Origin": "*", + "Access-Control-Allow-Headers": "authorization, x-client-info, apikey, content-type", + "Access-Control-Allow-Methods": "POST, OPTIONS", + "Content-Type": "application/json", +}; +export interface FeedbackBackend { + authenticate(token: string): Promise; + reserve(userId: string, files: number, bytes: number): Promise; + upload(path: string, file: File): Promise; + insert(report: Record): Promise; + remove(paths: string[]): Promise; +} +const reply = (status: number, code?: string) => + new Response(JSON.stringify(code ? { code } : { success: true }), { status, headers }); + +// Bound the actual body, not just the caller's Content-Length header. +async function readForm(request: Request) { + const reader = request.body?.getReader(); + if (!reader) throw new Error("Missing body"); + const chunks: Uint8Array[] = []; + let bytes = 0; + try { + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + bytes += value.byteLength; + if (bytes > MAX_REQUEST_BYTES) throw new Error("Body too large"); + chunks.push(value); + } + } finally { + await reader.cancel(); + } + const body = new Uint8Array(bytes); + let offset = 0; + for (const chunk of chunks) { + body.set(chunk, offset); + offset += chunk.byteLength; + } + return new Response(body, { + headers: { "Content-Type": request.headers.get("Content-Type") ?? "" }, + }).formData(); +} + +export function feedbackHandler(backend: FeedbackBackend) { + return async (request: Request): Promise => { + if (request.method === "OPTIONS") return new Response(null, { status: 204, headers }); + if (request.method !== "POST") return reply(405, "METHOD_NOT_ALLOWED"); + const token = request.headers.get("Authorization")?.match(/^Bearer (.+)$/i)?.[1]; + if (!token) return reply(401, "SIGN_IN_REQUIRED"); + const uploaded: string[] = []; + try { + const userId = await backend.authenticate(token); + if (!userId) return reply(401, "SIGN_IN_REQUIRED"); + let form: FormData; + try { + form = await readForm(request); + } catch { + return reply(400, "INVALID_FEEDBACK"); + } + const title = form.get("title"); + const message = form.get("message"); + const subject = form.get("subject"); + const logs = form.get("logs"); + const files = form.getAll("files"); + if ( + typeof title !== "string" || + !title.trim() || + title.trim().length > 160 || + typeof message !== "string" || + !message.trim() || + message.trim().length > 10000 || + typeof subject !== "string" || + !["bug", "idea", "question", "other"].includes(subject) || + (logs !== null && + (typeof logs !== "string" || new TextEncoder().encode(logs).length > 300000)) + ) { + return reply(400, "INVALID_FEEDBACK"); + } + if ( + files.length > MAX_FILES || + files.some((file) => typeof file === "string" || file.size === 0) + ) { + return reply(400, "INVALID_ATTACHMENTS"); + } + const attachments = files as File[]; + const bytes = attachments.reduce((total, file) => total + file.size, 0); + if (bytes > MAX_BYTES) return reply(400, "INVALID_ATTACHMENTS"); + if (!(await backend.reserve(userId, attachments.length, bytes))) + return reply(429, "FEEDBACK_LIMIT"); + const id = crypto.randomUUID(); + const metadata = []; + for (const file of attachments) { + const path = `${userId}/${id}/${crypto.randomUUID()}`; + await backend.upload(path, file); + uploaded.push(path); + metadata.push({ path, name: file.name, size: file.size, type: file.type }); + } + await backend.insert({ + id, + user_id: userId, + title: title.trim(), + subject, + message: message.trim(), + logs, + attachments: metadata, + }); + return reply(200); + } catch { + if (uploaded.length) await backend.remove(uploaded).catch(() => undefined); + return reply(500, "SUBMISSION_FAILED"); + } + }; +} diff --git a/services/supabase/functions/submit-feedback/index.ts b/services/supabase/functions/submit-feedback/index.ts new file mode 100644 index 00000000..2ccafe4a --- /dev/null +++ b/services/supabase/functions/submit-feedback/index.ts @@ -0,0 +1,41 @@ +import { createClient } from "npm:@supabase/supabase-js@2.116.0"; +import { feedbackHandler } from "./handler.ts"; + +const client = createClient( + Deno.env.get("SUPABASE_URL")!, + Deno.env.get("SUPABASE_SERVICE_ROLE_KEY")!, + { + auth: { persistSession: false, autoRefreshToken: false }, + }, +); +Deno.serve( + feedbackHandler({ + async authenticate(token) { + const { data, error } = await client.auth.getUser(token); + return error ? null : (data.user?.id ?? null); + }, + async reserve(userId, files, bytes) { + const { data, error } = await client.rpc("reserve_feedback_quota", { + account_id: userId, + file_count: files, + byte_count: bytes, + }); + if (error) throw error; + return data === true; + }, + async upload(path, file) { + const { error } = await client.storage + .from("feedback-attachments") + .upload(path, file, { contentType: "application/octet-stream", upsert: false }); + if (error) throw error; + }, + async insert(report) { + const { error } = await client.from("feedback_reports").insert(report); + if (error) throw error; + }, + async remove(paths) { + const { error } = await client.storage.from("feedback-attachments").remove(paths); + if (error) throw error; + }, + }), +); diff --git a/services/supabase/migrations/202609230002_feedback_limits.sql b/services/supabase/migrations/202609230002_feedback_limits.sql new file mode 100644 index 00000000..f53eb033 --- /dev/null +++ b/services/supabase/migrations/202609230002_feedback_limits.sql @@ -0,0 +1,44 @@ +begin; +-- Submissions now go through the authenticated submit-feedback Edge Function. +-- No direct client uploads or report inserts may bypass server validation/quotas. +drop policy "Upload own feedback files" on storage.objects; +drop policy "Submit own feedback" on public.feedback_reports; +revoke insert on public.feedback_reports from anon, authenticated; +grant select, insert on public.feedback_reports to service_role; + +create table public.feedback_daily_usage ( + user_id uuid not null references auth.users(id) on delete cascade, + day date not null, + reports integer not null default 0, + files integer not null default 0, + bytes bigint not null default 0, + primary key (user_id, day) +); +alter table public.feedback_daily_usage enable row level security; +revoke all on public.feedback_daily_usage from public, anon, authenticated; + +-- The upsert takes a row lock, so simultaneous requests cannot overspend quota. +-- Failed attempts consume quota too; deletion/retry must not reset the limit. +create function public.reserve_feedback_quota(account_id uuid, file_count integer, byte_count bigint) +returns boolean language plpgsql security definer set search_path = '' as $$ +begin + if account_id is null or file_count is null or byte_count is null + or file_count < 0 or file_count > 5 or byte_count < 0 or byte_count > 10485760 then + return false; + end if; + insert into public.feedback_daily_usage as usage (user_id, day, reports, files, bytes) + values (account_id, (now() at time zone 'UTC')::date, 1, file_count, byte_count) + on conflict (user_id, day) do update + set reports = usage.reports + 1, + files = usage.files + excluded.files, + bytes = usage.bytes + excluded.bytes + where usage.reports < 10 + and usage.files + excluded.files <= 25 + and usage.bytes + excluded.bytes <= 52428800; + return found; +end; +$$; +revoke all on function public.reserve_feedback_quota(uuid, integer, bigint) from public, anon, authenticated; +grant execute on function public.reserve_feedback_quota(uuid, integer, bigint) to service_role; + +commit; diff --git a/services/supabase/tests/feedback_limits.sql b/services/supabase/tests/feedback_limits.sql new file mode 100644 index 00000000..5c6e8d0b --- /dev/null +++ b/services/supabase/tests/feedback_limits.sql @@ -0,0 +1,50 @@ +-- Run as migration owner in a disposable Supabase database after both migrations. +begin; +insert into auth.users (id) values + ('77000000-0000-0000-0000-000000000001'), + ('77000000-0000-0000-0000-000000000002'), + ('77000000-0000-0000-0000-000000000003'); +do $$ +declare + account uuid := '77000000-0000-0000-0000-000000000001'; +begin + if has_function_privilege('authenticated', 'public.reserve_feedback_quota(uuid,integer,bigint)', 'EXECUTE') + or has_function_privilege('anon', 'public.reserve_feedback_quota(uuid,integer,bigint)', 'EXECUTE') + or has_table_privilege('authenticated', 'public.feedback_daily_usage', 'UPDATE') + or has_table_privilege('authenticated', 'public.feedback_reports', 'INSERT') then + raise exception 'Client can bypass server quotas'; + end if; + if not has_function_privilege('service_role', 'public.reserve_feedback_quota(uuid,integer,bigint)', 'EXECUTE') then + raise exception 'Edge Function cannot reserve quota'; + end if; + if exists (select 1 from pg_policies where schemaname = 'storage' and policyname = 'Upload own feedback files') then + raise exception 'Direct attachment uploads remain enabled'; + end if; + if public.reserve_feedback_quota(account, 6, 1) + or public.reserve_feedback_quota(account, 1, 10485761) + or public.reserve_feedback_quota(account, -1, 0) then + raise exception 'Per-submission bounds bypassed'; + end if; + for i in 1..5 loop + if not public.reserve_feedback_quota(account, 5, 1) then raise exception 'Valid file quota rejected'; end if; + end loop; + if public.reserve_feedback_quota(account, 1, 1) then raise exception 'Daily file cap bypassed'; end if; + account := '77000000-0000-0000-0000-000000000002'; + for i in 1..5 loop + if not public.reserve_feedback_quota(account, 1, 10485760) then raise exception 'Valid byte quota rejected'; end if; + end loop; + if public.reserve_feedback_quota(account, 1, 1) then raise exception 'Daily byte cap bypassed'; end if; + account := '77000000-0000-0000-0000-000000000003'; + for i in 1..10 loop + if not public.reserve_feedback_quota(account, 0, 0) then raise exception 'Valid report quota rejected'; end if; + end loop; + if public.reserve_feedback_quota(account, 0, 0) then raise exception 'Daily report cap bypassed'; end if; + if (select reports from public.feedback_daily_usage where user_id = account) <> 10 then + raise exception 'Rejected reservation changed quota'; + end if; + -- A prior day's usage does not block today's quota. + update public.feedback_daily_usage set day = day - 1 where user_id = account; + if not public.reserve_feedback_quota(account, 1, 1) then raise exception 'Daily quota did not reset'; end if; +end; +$$; +rollback; diff --git a/src/components/feedback/FeedbackDialog.tsx b/src/components/feedback/FeedbackDialog.tsx index 075858d3..8016bf10 100644 --- a/src/components/feedback/FeedbackDialog.tsx +++ b/src/components/feedback/FeedbackDialog.tsx @@ -10,7 +10,11 @@ import { } from "@/components/ui/dialog"; import { ChatDots, File, X } from "@/components/ui/icons"; import { feedbackDiagnostics } from "@/lib/feedback/diagnostics"; -import { submitFeedback, validateAttachments } from "@/lib/feedback/submitFeedback"; +import { + feedbackErrorMessage, + submitFeedback, + validateAttachments, +} from "@/lib/feedback/submitFeedback"; export function FeedbackDialog({ className, @@ -103,8 +107,8 @@ export function FeedbackDialog({ setSubject(""); setMessage(""); setFiles([]); - } catch { - setError("Could not send feedback. Please try again."); + } catch (error) { + setError(feedbackErrorMessage(error)); } finally { sendingRef.current = false; setSending(false); @@ -116,7 +120,10 @@ export function FeedbackDialog({ isRequired isDisabled={sending} value={subject} - onChange={setSubject} + onChange={(value) => { + setSubject(value); + setError(""); + }} className="w-full" > @@ -127,7 +134,10 @@ export function FeedbackDialog({ isRequired isDisabled={sending} value={message} - onChange={setMessage} + onChange={(value) => { + setMessage(value); + setError(""); + }} className="w-full" > diff --git a/src/lib/feedback/diagnostics.test.ts b/src/lib/feedback/diagnostics.test.ts index e53510eb..6e5978f1 100644 --- a/src/lib/feedback/diagnostics.test.ts +++ b/src/lib/feedback/diagnostics.test.ts @@ -20,3 +20,25 @@ describe("feedback diagnostics", () => { expect(validateAttachments([file(100)])).toBeNull(); }); }); + +it("bounds final JSON bytes for Unicode, escapes, and oversized metadata", async () => { + const { vi } = await import("vitest"); + vi.stubGlobal("window", new EventTarget()); + vi.stubGlobal("navigator", { platform: "平台".repeat(5000), userAgent: "代理".repeat(5000) }); + const { installFeedbackDiagnostics, feedbackDiagnostics } = await import("./diagnostics"); + const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); + try { + installFeedbackDiagnostics(); + for (let i = 0; i < 100; i++) console.warn(`${i}: ${'中\\\"\u0001'.repeat(500)}`); + const payload = feedbackDiagnostics(); + expect(new TextEncoder().encode(payload).length).toBeLessThanOrEqual(290000); + const data = JSON.parse(payload); + expect(data.logs.length).toBeGreaterThan(0); + expect(data.logs.length).toBeLessThan(100); + expect(data.logs.at(-1)).toContain("99:"); + expect(data.capturedAt).toBeTruthy(); + } finally { + warn.mockRestore(); + vi.unstubAllGlobals(); + } +}); diff --git a/src/lib/feedback/diagnostics.ts b/src/lib/feedback/diagnostics.ts index 45b30d28..91b6cf90 100644 --- a/src/lib/feedback/diagnostics.ts +++ b/src/lib/feedback/diagnostics.ts @@ -40,15 +40,19 @@ export function installFeedbackDiagnostics() { window.addEventListener("error", (event) => record("error", [event.message])); window.addEventListener("unhandledrejection", (event) => record("rejection", [event.reason])); } +const MAX_DIAGNOSTIC_BYTES = 290_000; export function feedbackDiagnostics() { - return JSON.stringify( - { - capturedAt: new Date().toISOString(), - platform: navigator.platform, - userAgent: navigator.userAgent, - logs: [...entries], - }, - null, - 2, - ); + const metadata = { + capturedAt: new Date().toISOString(), + platform: navigator.platform.slice(0, 2000), + userAgent: navigator.userAgent.slice(0, 2000), + }; + const logs = [...entries]; + const encoder = new TextEncoder(); + let payload = JSON.stringify({ ...metadata, logs }); + while (encoder.encode(payload).byteLength > MAX_DIAGNOSTIC_BYTES && logs.length) { + logs.shift(); + payload = JSON.stringify({ ...metadata, logs }); + } + return payload; } diff --git a/src/lib/feedback/server.test.ts b/src/lib/feedback/server.test.ts new file mode 100644 index 00000000..f6bcb19a --- /dev/null +++ b/src/lib/feedback/server.test.ts @@ -0,0 +1,86 @@ +import { beforeEach, expect, it, vi } from "vitest"; +import { feedbackHandler } from "../../../services/supabase/functions/submit-feedback/handler"; +const backend = { + authenticate: vi.fn(), + reserve: vi.fn(), + upload: vi.fn(), + insert: vi.fn(), + remove: vi.fn(), +}; +const handle = feedbackHandler(backend); +function request(files = [new File(["notes"], "notes.txt")], logs = "logs") { + const body = new FormData(); + body.set("title", "Bug"); + body.set("subject", "bug"); + body.set("message", "Steps"); + body.set("logs", logs); + for (const file of files) body.append("files", file); + return new Request("https://example.test", { + method: "POST", + headers: { Authorization: "Bearer session" }, + body, + }); +} +beforeEach(() => { + vi.resetAllMocks(); + backend.authenticate.mockResolvedValue("user-id"); + backend.reserve.mockResolvedValue(true); + backend.upload.mockResolvedValue(undefined); + backend.insert.mockResolvedValue(undefined); + backend.remove.mockResolvedValue(undefined); +}); +it("authenticates and reserves quota before uploading, then saves server-owned paths", async () => { + expect((await handle(request())).status).toBe(200); + expect(backend.reserve).toHaveBeenCalledWith("user-id", 1, 5); + expect(backend.reserve.mock.invocationCallOrder[0]).toBeLessThan( + backend.upload.mock.invocationCallOrder[0], + ); + expect(backend.insert).toHaveBeenCalledWith( + expect.objectContaining({ + user_id: "user-id", + attachments: [expect.objectContaining({ path: expect.stringMatching(/^user-id\//) })], + }), + ); +}); +it("rejects unauthenticated requests and exhausted quotas before storage", async () => { + backend.authenticate.mockResolvedValueOnce(null); + expect((await handle(request())).status).toBe(401); + expect(backend.reserve).not.toHaveBeenCalled(); + backend.reserve.mockResolvedValue(false); + expect((await handle(request())).status).toBe(429); + expect(backend.upload).not.toHaveBeenCalled(); +}); +it("enforces attachment and diagnostics limits against direct requests", async () => { + for (const req of [ + request(Array.from({ length: 6 }, () => new File(["x"], "x"))), + request([new File([], "empty")]), + request([ + new File([new Uint8Array(6 * 1024 * 1024)], "a"), + new File([new Uint8Array(5 * 1024 * 1024)], "b"), + ]), + request([], "中".repeat(100001)), + ]) { + expect((await handle(req)).status).toBe(400); + } + expect(backend.reserve).not.toHaveBeenCalled(); + expect(backend.upload).not.toHaveBeenCalled(); +}); +it("cleans partial uploads and does not expose backend errors", async () => { + backend.insert.mockRejectedValue(new Error("private db details")); + const response = await handle(request()); + expect(response.status).toBe(500); + expect(await response.json()).toEqual({ code: "SUBMISSION_FAILED" }); + expect(backend.remove).toHaveBeenCalledWith([expect.stringMatching(/^user-id\//)]); +}); +it("caps streamed request bodies even without a content-length header", async () => { + const req = new Request("https://example.test", { + method: "POST", + headers: { + Authorization: "Bearer session", + "Content-Type": "multipart/form-data; boundary=x", + }, + body: new Uint8Array(12 * 1024 * 1024), + }); + expect((await handle(req)).status).toBe(400); + expect(backend.reserve).not.toHaveBeenCalled(); +}); diff --git a/src/lib/feedback/submitFeedback.test.ts b/src/lib/feedback/submitFeedback.test.ts index e53b4b39..512b69ac 100644 --- a/src/lib/feedback/submitFeedback.test.ts +++ b/src/lib/feedback/submitFeedback.test.ts @@ -1,18 +1,9 @@ import { beforeEach, expect, it, vi } from "vitest"; -const api = vi.hoisted(() => ({ - getUser: vi.fn(), - upload: vi.fn(), - remove: vi.fn(), - insert: vi.fn(), -})); +const api = vi.hoisted(() => ({ getUser: vi.fn(), invoke: vi.fn() })); vi.mock("@/lib/auth/recordlyAuth", () => ({ - recordlyAuth: { - auth: { getUser: api.getUser }, - storage: { from: () => ({ upload: api.upload, remove: api.remove }) }, - from: () => ({ insert: api.insert }), - }, + recordlyAuth: { auth: { getUser: api.getUser }, functions: { invoke: api.invoke } }, })); -import { submitFeedback } from "./submitFeedback"; +import { feedbackErrorMessage, submitFeedback } from "./submitFeedback"; const input = { title: " Bug ", subject: "bug", @@ -23,30 +14,36 @@ const input = { beforeEach(() => { vi.resetAllMocks(); api.getUser.mockResolvedValue({ data: { user: { id: "user-id" } }, error: null }); - api.upload.mockResolvedValue({ error: null }); - api.insert.mockResolvedValue({ error: null }); - api.remove.mockResolvedValue({ error: null }); + api.invoke.mockResolvedValue({ data: { success: true }, error: null }); }); -it("stores a report only after its private attachments upload", async () => { +it("submits attachments and diagnostics through the server endpoint", async () => { await submitFeedback(input); - expect(api.upload).toHaveBeenCalledOnce(); - expect(api.insert).toHaveBeenCalledWith( - expect.objectContaining({ - user_id: "user-id", - title: "Bug", - message: "Steps", - attachments: [expect.objectContaining({ name: "notes.txt" })], - }), - ); - expect(api.remove).not.toHaveBeenCalled(); + const [name, { body }] = api.invoke.mock.calls[0]; + expect(name).toBe("submit-feedback"); + expect(body.get("title")).toBe("Bug"); + expect(body.get("message")).toBe("Steps"); + expect(body.get("logs")).toBe("logs"); + expect(body.getAll("files")[0].name).toBe("notes.txt"); }); -it("cleans uploaded attachments up when the report is rejected", async () => { - api.insert.mockResolvedValue({ error: new Error("unavailable") }); - await expect(submitFeedback(input)).rejects.toThrow("unavailable"); - expect(api.remove).toHaveBeenCalledWith([expect.stringMatching(/^user-id\//)]); -}); -it("does not upload when the account session has expired", async () => { +it("does not submit when the session has expired and explains how to recover", async () => { api.getUser.mockResolvedValue({ data: { user: null }, error: null }); - await expect(submitFeedback(input)).rejects.toThrow("sign in"); - expect(api.upload).not.toHaveBeenCalled(); + const error = await submitFeedback(input).catch((error) => error); + expect(feedbackErrorMessage(error)).toContain("sign in again"); + expect(api.invoke).not.toHaveBeenCalled(); +}); +it("shows the server quota error without exposing internal errors", async () => { + api.invoke.mockResolvedValue({ + error: { + context: new Response(JSON.stringify({ code: "FEEDBACK_LIMIT" }), { status: 429 }), + }, + }); + const error = await submitFeedback(input).catch((error) => error); + expect(feedbackErrorMessage(error)).toContain("tomorrow"); + expect(feedbackErrorMessage(new Error("database private details"))).toBe( + "Could not send feedback. Please try again.", + ); +}); +it("rejects unconfirmed submissions", async () => { + api.invoke.mockResolvedValue({ data: {}, error: null }); + await expect(submitFeedback(input)).rejects.toThrow("not confirmed"); }); diff --git a/src/lib/feedback/submitFeedback.ts b/src/lib/feedback/submitFeedback.ts index 0030f2df..c9ef457d 100644 --- a/src/lib/feedback/submitFeedback.ts +++ b/src/lib/feedback/submitFeedback.ts @@ -1,5 +1,20 @@ import { recordlyAuth } from "@/lib/auth/recordlyAuth"; +export class FeedbackError extends Error {} + +export function feedbackErrorMessage(error: unknown) { + return error instanceof FeedbackError + ? error.message + : "Could not send feedback. Please try again."; +} + +const submissionErrors: Record = { + SIGN_IN_REQUIRED: "Please sign in again to send feedback.", + INVALID_FEEDBACK: "Check your subject and description, then try again.", + INVALID_ATTACHMENTS: "Attach up to 5 nonempty files totaling 10 MB or less.", + FEEDBACK_LIMIT: "You’ve reached today’s feedback limit. Please try again tomorrow.", +}; + export const MAX_FILES = 5; export const MAX_BYTES = 10 * 1024 * 1024; export function validateAttachments(files: File[]) { @@ -17,40 +32,29 @@ export async function submitFeedback(input: { logs: string | null; }) { const client = recordlyAuth; - if (!client) throw new Error("Feedback is unavailable until account services are configured."); + if (!client) + throw new FeedbackError("Feedback is unavailable until account services are configured."); const { data, error } = await client.auth.getUser(); - if (error || !data.user) throw new Error("Please sign in again to send feedback."); + if (error || !data.user) throw new FeedbackError("Please sign in again to send feedback."); const validation = validateAttachments(input.files); - if (validation) throw new Error(validation); - const id = crypto.randomUUID(); - const uploaded: string[] = []; - const attachments = []; - try { - for (const file of input.files) { - const path = `${data.user.id}/${id}/${crypto.randomUUID()}`; - const { error: uploadError } = await client.storage - .from("feedback-attachments") - .upload(path, file, { contentType: "application/octet-stream" }); - if (uploadError) throw uploadError; - uploaded.push(path); - attachments.push({ path, name: file.name, size: file.size, type: file.type }); + if (validation) throw new FeedbackError(validation); + const body = new FormData(); + body.set("title", input.title.trim()); + body.set("subject", input.subject); + body.set("message", input.message.trim()); + if (input.logs !== null) body.set("logs", input.logs); + for (const file of input.files) body.append("files", file); + const result = await client.functions.invoke("submit-feedback", { body }); + if (result.error) { + const response = result.error.context; + if (response instanceof Response) { + if (response.status === 401) throw new FeedbackError(submissionErrors.SIGN_IN_REQUIRED); + const payload = await response.json().catch(() => null); + if (payload?.code && Object.keys(submissionErrors).includes(payload.code)) { + throw new FeedbackError(submissionErrors[payload.code]); + } } - const { error: insertError } = await client.from("feedback_reports").insert({ - id, - user_id: data.user.id, - title: input.title.trim(), - subject: input.subject, - message: input.message.trim(), - logs: input.logs, - attachments, - }); - if (insertError) throw insertError; - } catch (error) { - if (uploaded.length) - await client.storage - .from("feedback-attachments") - .remove(uploaded) - .catch(() => undefined); - throw error; + throw result.error; } + if (result.data?.success !== true) throw new Error("Feedback submission was not confirmed"); } diff --git a/tests/ui/feedback.spec.ts b/tests/ui/feedback.spec.ts index 7c78c8c2..cf232129 100644 --- a/tests/ui/feedback.spec.ts +++ b/tests/ui/feedback.spec.ts @@ -27,8 +27,14 @@ test("feedback preserves a draft on failure, supports attachments, and includes await expect(modal.getByRole("checkbox")).toHaveCount(0); await expect(modal.getByLabel("Title", { exact: true })).toHaveCount(0); await modal.getByRole("button", { name: "Send feedback", exact: true }).click(); - await expect(modal.getByRole("alert")).toContainText("Could not send feedback"); + await expect(modal.getByRole("alert")).toContainText(/sign in again|unavailable/i); await expect(modal.getByLabel("Subject", { exact: true })).toHaveValue("Export stops early"); + await modal.getByLabel("Subject", { exact: true }).fill("Export stops early (updated)"); + await expect(modal.getByRole("alert")).toHaveCount(0); + await modal.getByRole("button", { name: "Send feedback", exact: true }).click(); + await expect(modal.getByRole("alert")).toBeVisible(); + await modal.getByLabel("Description", { exact: true }).fill("Updated reproduction steps."); + await expect(modal.getByRole("alert")).toHaveCount(0); await page.screenshot({ path: "test-results/feedback.png" }); await modal.getByRole("button", { name: "Close", exact: true }).click(); await page.getByRole("button", { name: "Home", exact: true }).click();