From fbe37109a4c052b36ab1961c9f6f7eb9c0098ba4 Mon Sep 17 00:00:00 2001 From: Ole-Morten Duesund Date: Mon, 25 May 2026 13:54:07 +0200 Subject: [PATCH] fix(feedback): stop exposing done_by user id in API responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: GET /api/feedback (moderator-readable) returned the user id of the admin who marked an entry done. Moderators don't need to triangulate "which admin closed which ticket" — done_at alone is sufficient signal that the entry has been triaged. Keeping the user id in the response made it possible to cross-reference admins with the user list via a second authenticated call. Fix: the `feedback.done_by` column stays in the schema (server-side audit trail is preserved) but the column is no longer SELECTed by the list or update endpoints, and is no longer in the FeedbackEntry wire type. Moderators see only the `done_at` timestamp. Surfaced by /audit security (data exposure lens). --- server/feedback.ts | 8 +++++--- shared/types.ts | 6 ++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/server/feedback.ts b/server/feedback.ts index c00393d..4ba706c 100644 --- a/server/feedback.ts +++ b/server/feedback.ts @@ -40,7 +40,7 @@ feedbackRoutes.post('/', requireAuth, async (c) => { const entry: FeedbackEntry = { id, kind: body.kind, body: trimmed, created_at: Date.now(), - done_at: null, done_by: null, + done_at: null, }; return c.json(entry, 201); }); @@ -49,9 +49,11 @@ feedbackRoutes.get('/', requireAuth, (c) => { const userId = c.get('userId'); if (!isModerator(userId)) return c.json({ error: 'forbidden' }, 403); + // done_by is intentionally NOT selected — it's stored for server-side audit + // but never returned via the API. See FeedbackEntry doc in shared/types.ts. const rows = getDb() .prepare(` - SELECT f.id, f.kind, f.body, f.created_at, f.done_at, f.done_by, + SELECT f.id, f.kind, f.body, f.created_at, f.done_at, f.user_id, u.email AS user_email, u.display_name AS user_display FROM feedback f JOIN users u ON u.id = f.user_id @@ -85,7 +87,7 @@ feedbackRoutes.patch('/:id', requireAuth, async (c) => { } const row = db.prepare(` - SELECT f.id, f.kind, f.body, f.created_at, f.done_at, f.done_by, + SELECT f.id, f.kind, f.body, f.created_at, f.done_at, f.user_id, u.email AS user_email, u.display_name AS user_display FROM feedback f JOIN users u ON u.id = f.user_id diff --git a/shared/types.ts b/shared/types.ts index d1ff2be..bec887e 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -152,8 +152,10 @@ export interface FeedbackEntry { created_at: number; /** Set when an admin has marked the entry as done. */ done_at: number | null; - /** User id of the admin who marked it done; null when done_at is null. */ - done_by: string | null; + // The admin who marked the entry done is recorded in the `feedback.done_by` + // column for audit purposes, but deliberately NOT exposed via the API — + // moderators don't need to triangulate which admin closed which ticket + // against the user list. The `done_at` timestamp is sufficient signal. // Moderator-only fields; included when the caller is a moderator viewing // the list. (The submit endpoint doesn't return these — a submitter doesn't // need to see them.)