From 23c6e0e04b247e81f6080c7de2dc74187edba2f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Fri, 17 Apr 2026 09:40:01 +0200 Subject: [PATCH] security: sanitise Prisma error leaks in AI-tool helpers (#53) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five helper error mappers (timeline / project-creation / resource-creation / vacation-creation / task-action-execution) fell through to `return { error: error.message }` for BAD_REQUEST and CONFLICT cases. When the TRPCError wrapped a Prisma error, the message contained column names, relation paths, and the offending unique-constraint value — all of which would reach the LLM in chat context and, via audit_log.changes JSONB, the DB. Add `sanitizeAssistantErrorMessage()` that regex-detects Prisma and raw Postgres signatures (P2002/P2003/P2025, not-null, FK, check-constraint, duplicate-key) and replaces them with a generic "Invalid input". Also caps messages at 500 chars to defend against stack-trace-like payloads. Wire the helper into all five call-sites; the developer-constructed `AssistantVisibleError` branch in `normalizeAssistantExecutionError` is left untouched since those strings are hand-written. Coverage: 11 new tests in assistant-tools-error-sanitiser.test.ts; existing vacation / task-action / resource-creation / project-creation error tests (12 tests, 5 files) all remain green. Co-Authored-By: Claude Opus 4.7 --- .../assistant-tools-error-sanitiser.test.ts | 72 +++++++++++++++++++ .../api/src/router/assistant-tools/helpers.ts | 47 ++++++++++-- 2 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 packages/api/src/__tests__/assistant-tools-error-sanitiser.test.ts diff --git a/packages/api/src/__tests__/assistant-tools-error-sanitiser.test.ts b/packages/api/src/__tests__/assistant-tools-error-sanitiser.test.ts new file mode 100644 index 0000000..9ded1e7 --- /dev/null +++ b/packages/api/src/__tests__/assistant-tools-error-sanitiser.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; +import { sanitizeAssistantErrorMessage } from "../router/assistant-tools/helpers.js"; + +/** + * Ticket #53 — AI-tool helpers previously returned `error.message` verbatim + * for BAD_REQUEST / CONFLICT cases. When the underlying cause was a Prisma + * error (P2002 unique, P2003 FK, P2025 missing), the text included column + * names, relation paths, and the offending value — all of which ended up + * in LLM chat context and, via audit_log.changes, in the DB. + * + * `sanitizeAssistantErrorMessage` replaces those patterns with a generic + * "Invalid input" while letting hand-crafted router messages through. + */ +describe("sanitizeAssistantErrorMessage (#53)", () => { + it("replaces P2002 unique-constraint leak with generic text", () => { + const leak = + "Invalid `prisma.user.create()` invocation in\n/app/src/router/users.ts:142:5\n\nUnique constraint failed on the fields: (`email`)"; + expect(sanitizeAssistantErrorMessage(leak)).toBe("Invalid input"); + }); + + it("replaces P2003 FK-violation leak", () => { + const leak = "Foreign key constraint failed on the field: `clientId`"; + expect(sanitizeAssistantErrorMessage(leak)).toBe("Invalid input"); + }); + + it("replaces P2025 missing-record leak", () => { + const leak = + "An operation failed because it depends on one or more records that were required but not found."; + expect(sanitizeAssistantErrorMessage(leak)).toBe("Invalid input"); + }); + + it("replaces raw Postgres unique-violation leak", () => { + const leak = + 'duplicate key value violates unique constraint "User_email_key"\nDETAIL: Key (email)=(alice@example.com) already exists.'; + expect(sanitizeAssistantErrorMessage(leak)).toBe("Invalid input"); + }); + + it("replaces raw Postgres not-null leak", () => { + const leak = + 'null value in column "projectId" of relation "Allocation" violates not-null constraint'; + expect(sanitizeAssistantErrorMessage(leak)).toBe("Invalid input"); + }); + + it("replaces raw Postgres check-constraint leak", () => { + const leak = 'new row for relation "Project" violates check constraint "Project_status_check"'; + expect(sanitizeAssistantErrorMessage(leak)).toBe("Invalid input"); + }); + + it("caps excessively long messages (stack-trace dump defence)", () => { + const giant = "A".repeat(600); + expect(sanitizeAssistantErrorMessage(giant)).toBe("Invalid input"); + }); + + it("handles empty message defensively", () => { + expect(sanitizeAssistantErrorMessage("")).toBe("Invalid input"); + }); + + it("lets short hand-crafted router messages through unchanged", () => { + const safe = "The project must have a client assigned."; + expect(sanitizeAssistantErrorMessage(safe)).toBe(safe); + }); + + it("lets business-rule validation text through", () => { + const safe = "Vacation cannot be approved in its current status."; + expect(sanitizeAssistantErrorMessage(safe)).toBe(safe); + }); + + it("lets shortCode conflict messages through (quoted value is user-provided)", () => { + const safe = 'A project with short code "ACME01" already exists.'; + expect(sanitizeAssistantErrorMessage(safe)).toBe(safe); + }); +}); diff --git a/packages/api/src/router/assistant-tools/helpers.ts b/packages/api/src/router/assistant-tools/helpers.ts index 59d573b..8d20f7e 100644 --- a/packages/api/src/router/assistant-tools/helpers.ts +++ b/packages/api/src/router/assistant-tools/helpers.ts @@ -19,6 +19,43 @@ export class AssistantVisibleError extends Error { } } +// Signatures of raw Prisma / database errors that must never reach the LLM. +// We'd rather surface a generic "Invalid input" than leak column names, FK +// relation paths, or the offending value from a unique-constraint failure +// (which can include user PII on a second write attempt). +const PRISMA_LEAK_SIGNATURES = [ + /Invalid\s+`prisma\./i, + /Unique constraint failed on the fields?:/i, + /Foreign key constraint failed on the field/i, + /An operation failed because it depends on one or more records/i, + /The column\s+`[^`]+`\s+does not exist/i, + /relation\s+"[^"]+"\s+does not exist/i, + /duplicate key value violates unique constraint/i, + /null value in column\s+"/i, + /violates (?:check|not-null|foreign key) constraint/i, +]; + +const SAFE_ERROR_FALLBACK = "Invalid input"; +const MAX_ASSISTANT_ERROR_LENGTH = 500; + +/** + * Sanitises a TRPCError / downstream error message before it's handed back + * to the LLM. Hand-written BAD_REQUEST / CONFLICT messages in routers are + * user-safe, but a subset of error paths pass raw Prisma text straight + * through — that would leak schema details (column names, relation paths, + * offending values) into chat context and, transitively, into audit JSONB. + * + * Strategy: regex-detect Prisma-flavoured signatures and replace with a + * generic fallback. Also hard-cap length as a belt-and-suspenders defence + * against stack-trace-like payloads. + */ +export function sanitizeAssistantErrorMessage(message: string): string { + if (!message) return SAFE_ERROR_FALLBACK; + if (message.length > MAX_ASSISTANT_ERROR_LENGTH) return SAFE_ERROR_FALLBACK; + if (PRISMA_LEAK_SIGNATURES.some((re) => re.test(message))) return SAFE_ERROR_FALLBACK; + return message; +} + export function assertPermission(ctx: ToolContext, perm: PermissionKey): void { if (!ctx.permissions.has(perm)) { throw new AssistantVisibleError( @@ -293,7 +330,7 @@ export function toAssistantTimelineMutationError( } if (error.code === "BAD_REQUEST" || error.code === "CONFLICT") { - return { error: error.message }; + return { error: sanitizeAssistantErrorMessage(error.message) }; } } @@ -369,7 +406,7 @@ export function toAssistantProjectCreationError( } if (error.code === "BAD_REQUEST" || error.code === "UNPROCESSABLE_CONTENT") { - return { error: error.message }; + return { error: sanitizeAssistantErrorMessage(error.message) }; } } @@ -612,7 +649,7 @@ export function toAssistantResourceCreationError(error: unknown): AssistantToolE } if (error.code === "BAD_REQUEST" || error.code === "UNPROCESSABLE_CONTENT") { - return { error: error.message }; + return { error: sanitizeAssistantErrorMessage(error.message) }; } if (error.code === "NOT_FOUND") { @@ -770,7 +807,7 @@ export function toAssistantVacationCreationError(error: unknown): AssistantToolE } if (error.code === "BAD_REQUEST") { - return { error: error.message }; + return { error: sanitizeAssistantErrorMessage(error.message) }; } } @@ -1219,7 +1256,7 @@ export function toAssistantTaskActionError(error: unknown): AssistantToolErrorRe if (error.message === "Assignment is already CONFIRMED") { return { error: "Assignment is already confirmed." }; } - return { error: error.message }; + return { error: sanitizeAssistantErrorMessage(error.message) }; } if (error instanceof TRPCError && error.code === "FORBIDDEN") {