From ba67dab03226c159ace1b52acb64c5429ddb9029 Mon Sep 17 00:00:00 2001 From: Ronaldo Martins Date: Mon, 10 Aug 2026 17:47:16 -0300 Subject: [PATCH 1/2] fix(api): sanitize error details in HTTP responses (ENG-1668) - Add sanitizeErrorForResponse helper (core/errors.ts): first line only, 200-char cap, redacts paths/connection strings/stacks/secrets/hosts - Replace all 52 'details: String(error)' leaks in router.ts - Sanitize task.lastError before posting Linear failure comments - 12 unit tests covering redaction patterns --- packages/api/src/core/errors.test.ts | 99 ++++++++++++++++++++++++ packages/api/src/core/errors.ts | 53 +++++++++++++ packages/api/src/router.ts | 109 ++++++++++++++------------- 3 files changed, 207 insertions(+), 54 deletions(-) create mode 100644 packages/api/src/core/errors.test.ts create mode 100644 packages/api/src/core/errors.ts diff --git a/packages/api/src/core/errors.test.ts b/packages/api/src/core/errors.test.ts new file mode 100644 index 0000000..6a077df --- /dev/null +++ b/packages/api/src/core/errors.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from "bun:test"; +import { sanitizeErrorForResponse } from "./errors"; + +describe("sanitizeErrorForResponse (ENG-1668)", () => { + it("returns first line of a safe Error message", () => { + expect(sanitizeErrorForResponse(new Error("Task not found"))).toBe( + "Task not found", + ); + }); + + it("accepts plain strings", () => { + expect(sanitizeErrorForResponse("Invalid payload")).toBe("Invalid payload"); + }); + + it("returns generic message for non-Error/non-string values", () => { + expect(sanitizeErrorForResponse({ foo: 1 })).toBe("Internal error"); + expect(sanitizeErrorForResponse(undefined)).toBe("Internal error"); + expect(sanitizeErrorForResponse(null)).toBe("Internal error"); + expect(sanitizeErrorForResponse(42)).toBe("Internal error"); + }); + + it("returns generic message for empty messages", () => { + expect(sanitizeErrorForResponse(new Error(""))).toBe("Internal error"); + expect(sanitizeErrorForResponse(" ")).toBe("Internal error"); + }); + + it("drops everything after the first line (stack-ish bodies)", () => { + expect( + sanitizeErrorForResponse(new Error("Boom\n at handler (/app/x.ts:1:1)")), + ).toBe("Boom"); + }); + + it("redacts filesystem paths", () => { + expect( + sanitizeErrorForResponse( + new Error("ENOENT: /Users/ronaldo/secret/file.json missing"), + ), + ).toBe("Internal error"); + expect( + sanitizeErrorForResponse(new Error("Cannot read C:\\Windows\\env")), + ).toBe("Internal error"); + }); + + it("redacts connection strings", () => { + expect( + sanitizeErrorForResponse( + new Error("connect failed postgres://user:pass@db:5432/app"), + ), + ).toBe("Internal error"); + expect( + sanitizeErrorForResponse(new Error("redis://cache failed")), + ).toBe("Internal error"); + }); + + it("redacts credentials in URLs", () => { + expect( + sanitizeErrorForResponse(new Error("fetch https://a:b@example.com")), + ).toBe("Internal error"); + }); + + it("redacts stack trace fragments on the first line", () => { + expect( + sanitizeErrorForResponse( + new Error("failed at run (/app/src/router.ts:12:5)"), + ), + ).toBe("Internal error"); + expect(sanitizeErrorForResponse(new Error("router.ts:44:10 threw"))).toBe( + "Internal error", + ); + }); + + it("redacts secrets and tokens", () => { + expect( + sanitizeErrorForResponse(new Error("bad api_key: abc123")), + ).toBe("Internal error"); + expect( + sanitizeErrorForResponse(new Error("token sk_live1234567890 rejected")), + ).toBe("Internal error"); + expect( + sanitizeErrorForResponse(new Error("Authorization: Bearer x")), + ).toBe("Internal error"); + }); + + it("redacts env and internal hosts", () => { + expect( + sanitizeErrorForResponse(new Error("process.env.SECRET is undefined")), + ).toBe("Internal error"); + expect( + sanitizeErrorForResponse(new Error("ECONNREFUSED 127.0.0.1:5432")), + ).toBe("Internal error"); + }); + + it("truncates long messages to 200 chars", () => { + const long = "x".repeat(300); + const result = sanitizeErrorForResponse(new Error(long)); + expect(result.length).toBe(201); // 200 + ellipsis + expect(result.endsWith("…")).toBe(true); + }); +}); diff --git a/packages/api/src/core/errors.ts b/packages/api/src/core/errors.ts new file mode 100644 index 0000000..fddae57 --- /dev/null +++ b/packages/api/src/core/errors.ts @@ -0,0 +1,53 @@ +// ============================================================================ +// Error sanitization for HTTP responses (ENG-1668) +// Prevents leaking internal details (paths, connection strings, stack traces, +// credentials) to API clients. Full errors must still be logged server-side +// via console.error at the call site. +// ============================================================================ + +const SENSITIVE_PATTERNS: RegExp[] = [ + // Absolute filesystem paths (macOS/Linux/Windows) + /(?:\/(?:Users|home|var|etc|tmp|opt|private|srv)\/|[A-Za-z]:\\)/, + // Connection strings / URLs with credentials + /(?:postgres(?:ql)?|mysql|mongodb(?:\+srv)?|redis|amqp):\/\//i, + /\/\/[^\s/]+:[^\s@]+@/, + // Stack trace lines + /\bat\s+.+\(.+:\d+:\d+\)/, + /\.(?:ts|js|tsx|jsx|mjs|cjs):\d+:\d+/, + // Secrets / tokens / keys + /(?:api[_-]?key|secret|token|password|passwd|authorization|bearer)\s*[:=]/i, + /\b(?:sk|pk|ghp|gho|ghs|xox[abps])_[A-Za-z0-9]{8,}/, + // Env var dumps + /\bprocess\.env\b/, + // Internal hosts + /\b(?:localhost|127\.0\.0\.1|0\.0\.0\.0|::1)\b/, +]; + +const MAX_DETAIL_LENGTH = 200; + +/** + * Produces a safe, generic error string for inclusion in HTTP response bodies. + * - Uses only the first line of the error message + * - Truncates to 200 chars + * - Replaces the whole message with "Internal error" if it matches any + * sensitive pattern (paths, connection strings, stacks, credentials) + */ +export function sanitizeErrorForResponse(error: unknown): string { + let message: string; + if (error instanceof Error) { + message = error.message; + } else if (typeof error === "string") { + message = error; + } else { + return "Internal error"; + } + + const firstLine = (message.split("\n")[0] ?? "").trim(); + if (!firstLine) return "Internal error"; + if (SENSITIVE_PATTERNS.some((p) => p.test(firstLine))) { + return "Internal error"; + } + return firstLine.length > MAX_DETAIL_LENGTH + ? `${firstLine.slice(0, MAX_DETAIL_LENGTH)}…` + : firstLine; +} diff --git a/packages/api/src/router.ts b/packages/api/src/router.ts index 57060e7..3585b01 100644 --- a/packages/api/src/router.ts +++ b/packages/api/src/router.ts @@ -7,6 +7,7 @@ import { JobStatus, } from "./core/types"; import { Orchestrator } from "./core/orchestrator"; +import { sanitizeErrorForResponse } from "./core/errors"; import { TaskRunner } from "./core/task-runner"; import { db } from "./integrations/db"; import { dbJobs } from "./integrations/db-jobs"; @@ -122,7 +123,7 @@ function startBackgroundTaskRunner(task: Task): void { } else if (processedTask.status === "FAILED") { await linear.addComment( processedTask.linearIssueId, - `❌ **AutoDev failed to complete this task**\n\nReason: ${processedTask.lastError}\n\nThis issue may require manual implementation.`, + `❌ **AutoDev failed to complete this task**\n\nReason: ${sanitizeErrorForResponse(processedTask.lastError ?? "")}\n\nThis issue may require manual implementation.`, ); } } @@ -510,7 +511,7 @@ async function handleCheckRunEvent( } else if (processedTask.status === "FAILED") { await linear.addComment( task.linearIssueId, - `❌ **AutoDev failed to complete this task**\n\nReason: ${processedTask.lastError}\n\nThis issue may require manual implementation.`, + `❌ **AutoDev failed to complete this task**\n\nReason: ${sanitizeErrorForResponse(processedTask.lastError ?? "")}\n\nThis issue may require manual implementation.`, ); } } @@ -618,7 +619,7 @@ async function handlePullRequestReviewEvent( } catch (error) { console.error(`[Webhook] Error reprocessing task ${task.id}:`, error); return Response.json( - { error: "Failed to reprocess task", details: String(error) }, + { error: "Failed to reprocess task", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1279,7 +1280,7 @@ route("POST", "/api/tasks/cleanup", async (req) => { } catch (error) { console.error("[Cleanup] Error:", error); return Response.json( - { error: "Failed to cleanup stale tasks", details: String(error) }, + { error: "Failed to cleanup stale tasks", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1349,7 +1350,7 @@ route("DELETE", "/api/tasks/failed", async (req) => { } catch (error) { console.error("[Delete Failed Tasks] Error:", error); return Response.json( - { error: "Failed to delete tasks", details: String(error) }, + { error: "Failed to delete tasks", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1423,7 +1424,7 @@ route("GET", "/api/tasks/cleanup/stats", async (req) => { } catch (error) { console.error("[Cleanup Stats] Error:", error); return Response.json( - { error: "Failed to get cleanup stats", details: String(error) }, + { error: "Failed to get cleanup stats", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1467,7 +1468,7 @@ route("GET", "/api/costs", async (req) => { } catch (error) { console.error("[Costs] Error:", error); return Response.json( - { error: "Failed to get cost summary", details: String(error) }, + { error: "Failed to get cost summary", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1491,7 +1492,7 @@ route("GET", "/api/costs/by-model", async (req) => { } catch (error) { console.error("[Costs] Error:", error); return Response.json( - { error: "Failed to get cost by model", details: String(error) }, + { error: "Failed to get cost by model", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1515,7 +1516,7 @@ route("GET", "/api/costs/by-agent", async (req) => { } catch (error) { console.error("[Costs] Error:", error); return Response.json( - { error: "Failed to get cost by agent", details: String(error) }, + { error: "Failed to get cost by agent", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1539,7 +1540,7 @@ route("GET", "/api/costs/daily", async (req) => { } catch (error) { console.error("[Costs] Error:", error); return Response.json( - { error: "Failed to get daily costs", details: String(error) }, + { error: "Failed to get daily costs", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1562,7 +1563,7 @@ route("GET", "/api/costs/task/:id", async (req) => { } catch (error) { console.error("[Costs] Error:", error); return Response.json( - { error: "Failed to get task cost", details: String(error) }, + { error: "Failed to get task cost", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1578,7 +1579,7 @@ route("GET", "/api/costs/alerts", async () => { } catch (error) { console.error("[Costs] Error:", error); return Response.json( - { error: "Failed to check budget alerts", details: String(error) }, + { error: "Failed to check budget alerts", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1594,7 +1595,7 @@ route("GET", "/api/costs/optimizations", async () => { } catch (error) { console.error("[Costs] Error:", error); return Response.json( - { error: "Failed to get optimizations", details: String(error) }, + { error: "Failed to get optimizations", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1629,7 +1630,7 @@ route("GET", "/api/costs/export", async (req) => { } catch (error) { console.error("[Costs] Error:", error); return Response.json( - { error: "Failed to export costs", details: String(error) }, + { error: "Failed to export costs", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1892,7 +1893,7 @@ route("PUT", "/api/config/models", async (req) => { } catch (error) { console.error("[ModelConfig] Error updating config:", error); return Response.json( - { error: "Failed to update model config", details: String(error) }, + { error: "Failed to update model config", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -1913,7 +1914,7 @@ route("POST", "/api/config/models/reset", async () => { } catch (error) { console.error("[ModelConfig] Error resetting config:", error); return Response.json( - { error: "Failed to reset model config", details: String(error) }, + { error: "Failed to reset model config", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -2709,7 +2710,7 @@ route("POST", "/api/rag/index", async (req) => { github = new GitHubClient(); } catch (error) { return Response.json( - { error: "GitHub client not configured", details: String(error) }, + { error: "GitHub client not configured", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -3398,7 +3399,7 @@ route("POST", "/api/tasks/:id/reject", async (req) => { } catch (error) { console.error(`[API] Error reprocessing task ${task.id}:`, error); return Response.json( - { error: "Failed to reprocess task", details: String(error) }, + { error: "Failed to reprocess task", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -4485,7 +4486,7 @@ route("POST", "/api/linear/sync", async (req) => { } catch (error) { console.error("[API] Error syncing to Linear:", error); return Response.json( - { error: "Failed to sync issues", details: String(error) }, + { error: "Failed to sync issues", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -5982,7 +5983,7 @@ route("POST", "/api/repositories", async (req) => { } catch (error) { console.error("[API] Failed to create repository:", error); return Response.json( - { error: "Failed to link repository", details: String(error) }, + { error: "Failed to link repository", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6042,7 +6043,7 @@ route("DELETE", "/api/repositories/:id", async (req) => { } catch (error) { console.error("[API] Failed to delete repository:", error); return Response.json( - { error: "Failed to unlink repository", details: String(error) }, + { error: "Failed to unlink repository", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6067,7 +6068,7 @@ route("POST", "/api/repositories/sync", async () => { } catch (error) { console.error("[API] Failed to sync repositories:", error); return Response.json( - { error: "Failed to sync repositories", details: String(error) }, + { error: "Failed to sync repositories", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6141,7 +6142,7 @@ route("POST", "/api/issues", async (req) => { } catch (error) { console.error("[API] Failed to create issue:", error); return Response.json( - { error: "Failed to create issue", details: String(error) }, + { error: "Failed to create issue", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6189,7 +6190,7 @@ route("GET", "/api/issues/:owner/:repo", async (req) => { } catch (error) { console.error("[API] Failed to list issues:", error); return Response.json( - { error: "Failed to list issues", details: String(error) }, + { error: "Failed to list issues", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6278,7 +6279,7 @@ route("POST", "/api/tasks/import", async (req) => { } catch (error) { console.error("[API] Failed to import issues:", error); return Response.json( - { error: "Failed to import issues", details: String(error) }, + { error: "Failed to import issues", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6309,7 +6310,7 @@ route("GET", "/api/plans", async (req) => { } catch (error) { console.error("[API] Failed to list plans:", error); return Response.json( - { error: "Failed to list plans", details: String(error) }, + { error: "Failed to list plans", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6352,7 +6353,7 @@ route("POST", "/api/plans", async (req) => { } catch (error) { console.error("[API] Failed to create plan:", error); return Response.json( - { error: "Failed to create plan", details: String(error) }, + { error: "Failed to create plan", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6398,7 +6399,7 @@ route("GET", "/api/plans/:id", async (req) => { } catch (error) { console.error("[API] Failed to get plan:", error); return Response.json( - { error: "Failed to get plan", details: String(error) }, + { error: "Failed to get plan", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6437,7 +6438,7 @@ route("PUT", "/api/plans/:id", async (req) => { } catch (error) { console.error("[API] Failed to update plan:", error); return Response.json( - { error: "Failed to update plan", details: String(error) }, + { error: "Failed to update plan", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6467,7 +6468,7 @@ route("DELETE", "/api/plans/:id", async (req) => { } catch (error) { console.error("[API] Failed to delete plan:", error); return Response.json( - { error: "Failed to delete plan", details: String(error) }, + { error: "Failed to delete plan", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6499,7 +6500,7 @@ route("GET", "/api/plans/:id/cards", async (req) => { } catch (error) { console.error("[API] Failed to get plan cards:", error); return Response.json( - { error: "Failed to get plan cards", details: String(error) }, + { error: "Failed to get plan cards", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6547,7 +6548,7 @@ route("POST", "/api/plans/:id/cards", async (req) => { } catch (error) { console.error("[API] Failed to create card:", error); return Response.json( - { error: "Failed to create card", details: String(error) }, + { error: "Failed to create card", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6588,7 +6589,7 @@ route("POST", "/api/plans/:id/cards/reorder", async (req) => { } catch (error) { console.error("[API] Failed to reorder cards:", error); return Response.json( - { error: "Failed to reorder cards", details: String(error) }, + { error: "Failed to reorder cards", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6615,7 +6616,7 @@ route("GET", "/api/cards/:id", async (req) => { } catch (error) { console.error("[API] Failed to get card:", error); return Response.json( - { error: "Failed to get card", details: String(error) }, + { error: "Failed to get card", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6666,7 +6667,7 @@ route("PUT", "/api/cards/:id", async (req) => { } catch (error) { console.error("[API] Failed to update card:", error); return Response.json( - { error: "Failed to update card", details: String(error) }, + { error: "Failed to update card", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6696,7 +6697,7 @@ route("DELETE", "/api/cards/:id", async (req) => { } catch (error) { console.error("[API] Failed to delete card:", error); return Response.json( - { error: "Failed to delete card", details: String(error) }, + { error: "Failed to delete card", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6820,7 +6821,7 @@ route("POST", "/api/plans/:id/create-issues", async (req) => { } catch (error) { console.error("[API] Failed to create issues from plan:", error); return Response.json( - { error: "Failed to create issues from plan", details: String(error) }, + { error: "Failed to create issues from plan", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -6973,7 +6974,7 @@ route("POST", "/api/tasks/:id/chat", async (req) => { } catch (error) { console.error("[Chat] Error processing message:", error); return Response.json( - { error: "Failed to process chat message", details: String(error) }, + { error: "Failed to process chat message", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7005,7 +7006,7 @@ route("GET", "/api/tasks/:id/conversations", async (req) => { } catch (error) { console.error("[Chat] Error listing conversations:", error); return Response.json( - { error: "Failed to list conversations", details: String(error) }, + { error: "Failed to list conversations", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7045,7 +7046,7 @@ route("GET", "/api/conversations/:id/messages", async (req) => { } catch (error) { console.error("[Chat] Error getting messages:", error); return Response.json( - { error: "Failed to get messages", details: String(error) }, + { error: "Failed to get messages", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7077,7 +7078,7 @@ route("GET", "/api/tasks/:id/external-sessions", async (req) => { } catch (error) { console.error("[Chat] Error listing external sessions:", error); return Response.json( - { error: "Failed to list external sessions", details: String(error) }, + { error: "Failed to list external sessions", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7133,7 +7134,7 @@ route("POST", "/api/tasks/:id/external-sessions", async (req) => { } catch (error) { console.error("[Chat] Error creating external session:", error); return Response.json( - { error: "Failed to create external session", details: String(error) }, + { error: "Failed to create external session", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7172,7 +7173,7 @@ route("PATCH", "/api/conversations/:id", async (req) => { } catch (error) { console.error("[Chat] Error updating conversation:", error); return Response.json( - { error: "Failed to update conversation", details: String(error) }, + { error: "Failed to update conversation", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7248,7 +7249,7 @@ route("POST", "/api/tasks/:id/run-visual-tests", async (req) => { ); } return Response.json( - { error: "Failed to run visual tests", details: String(error) }, + { error: "Failed to run visual tests", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7268,7 +7269,7 @@ route("GET", "/api/tasks/:id/visual-tests", async (req) => { } catch (error) { console.error("[API] Failed to get visual test runs:", error); return Response.json( - { error: "Failed to get visual test runs", details: String(error) }, + { error: "Failed to get visual test runs", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7294,7 +7295,7 @@ route("GET", "/api/visual-tests/:runId", async (req) => { } catch (error) { console.error("[API] Failed to get visual test run:", error); return Response.json( - { error: "Failed to get visual test run", details: String(error) }, + { error: "Failed to get visual test run", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7343,7 +7344,7 @@ route("POST", "/api/plan-conversations", async (req) => { } catch (error) { console.error("[PlanConversation] Error creating conversation:", error); return Response.json( - { error: "Failed to create conversation", details: String(error) }, + { error: "Failed to create conversation", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7371,7 +7372,7 @@ route("GET", "/api/plan-conversations", async (req) => { } catch (error) { console.error("[PlanConversation] Error listing conversations:", error); return Response.json( - { error: "Failed to list conversations", details: String(error) }, + { error: "Failed to list conversations", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7408,7 +7409,7 @@ route("GET", "/api/plan-conversations/:id", async (req) => { } catch (error) { console.error("[PlanConversation] Error getting conversation:", error); return Response.json( - { error: "Failed to get conversation", details: String(error) }, + { error: "Failed to get conversation", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7536,7 +7537,7 @@ route("POST", "/api/plan-conversations/:id/messages", async (req) => { } catch (error) { console.error("[PlanConversation] Error sending message:", error); return Response.json( - { error: "Failed to send message", details: String(error) }, + { error: "Failed to send message", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7582,7 +7583,7 @@ route("PATCH", "/api/plan-conversations/:id", async (req) => { } catch (error) { console.error("[PlanConversation] Error updating conversation:", error); return Response.json( - { error: "Failed to update conversation", details: String(error) }, + { error: "Failed to update conversation", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7624,7 +7625,7 @@ route("PATCH", "/api/plan-draft-cards/:id", async (req) => { } catch (error) { console.error("[PlanConversation] Error updating card:", error); return Response.json( - { error: "Failed to update card", details: String(error) }, + { error: "Failed to update card", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7647,7 +7648,7 @@ route("DELETE", "/api/plan-draft-cards/:id", async (req) => { } catch (error) { console.error("[PlanConversation] Error deleting card:", error); return Response.json( - { error: "Failed to delete card", details: String(error) }, + { error: "Failed to delete card", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } @@ -7731,7 +7732,7 @@ route("POST", "/api/plan-conversations/:id/convert", async (req) => { } catch (error) { console.error("[PlanConversation] Error converting to plan:", error); return Response.json( - { error: "Failed to convert to plan", details: String(error) }, + { error: "Failed to convert to plan", details: sanitizeErrorForResponse(error) }, { status: 500 }, ); } From 0e5b56bba9f54a00979197171564656fd7708306 Mon Sep 17 00:00:00 2001 From: Ronaldo Martins Date: Mon, 10 Aug 2026 21:37:50 -0300 Subject: [PATCH 2/2] fix(api): redact any absolute path and looser credential phrasing (ENG-1668) Path detection previously relied on an enumerated allowlist of roots (/Users, /home, /var, ...) and missed container WORKDIRs like /app, letting messages such as ENOENT: /app/packages/api/.env leak verbatim. Replace the allowlist with a general absolute-path matcher (POSIX with >=2 segments, or Windows drive paths) so any filesystem path is caught regardless of root. Also widen the credential-phrase pattern to allow words between the sensitive noun and the colon/equals (e.g. "API key provided:"), and the vendor-token pattern to accept hyphenated prefixes (sk-proj-...), closing the two leaks reproduced in review. Adds regression tests for /app paths, an arbitrary Windows path, and both reviewer-reported bypasses. --- packages/api/src/core/errors.test.ts | 45 ++++++++++++++++++++++++++++ packages/api/src/core/errors.ts | 16 ++++++---- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/packages/api/src/core/errors.test.ts b/packages/api/src/core/errors.test.ts index 6a077df..da8f2f8 100644 --- a/packages/api/src/core/errors.test.ts +++ b/packages/api/src/core/errors.test.ts @@ -41,6 +41,31 @@ describe("sanitizeErrorForResponse (ENG-1668)", () => { ).toBe("Internal error"); }); + it("redacts container WORKDIR paths (e.g. /app) not on an enumerated allowlist", () => { + expect( + sanitizeErrorForResponse( + new Error("ENOENT: /app/packages/api/.env not found"), + ), + ).toBe("Internal error"); + expect( + sanitizeErrorForResponse( + new Error( + "Cannot find module '/app/node_modules/some-pkg/index.js'", + ), + ), + ).toBe("Internal error"); + }); + + it("redacts arbitrary Windows drive paths", () => { + expect( + sanitizeErrorForResponse( + new Error( + "EBUSY: resource busy or locked, open 'D:\\builds\\app\\secrets.json'", + ), + ), + ).toBe("Internal error"); + }); + it("redacts connection strings", () => { expect( sanitizeErrorForResponse( @@ -81,6 +106,26 @@ describe("sanitizeErrorForResponse (ENG-1668)", () => { ).toBe("Internal error"); }); + it("redacts credential-looking phrases with words between the noun and colon", () => { + // Reviewer repro (PR #428): "API key provided:" — the colon is not + // immediately after "key", so a naive `key\s*[:=]` pattern misses it. + expect( + sanitizeErrorForResponse( + new Error( + "Incorrect API key provided: sk-proj-abcdefghijklmnopqrstuvwxyz", + ), + ), + ).toBe("Internal error"); + }); + + it("redacts hyphenated vendor-prefixed tokens (e.g. sk-proj-...)", () => { + expect( + sanitizeErrorForResponse( + new Error("upstream rejected sk-proj-abcdefghijklmnop"), + ), + ).toBe("Internal error"); + }); + it("redacts env and internal hosts", () => { expect( sanitizeErrorForResponse(new Error("process.env.SECRET is undefined")), diff --git a/packages/api/src/core/errors.ts b/packages/api/src/core/errors.ts index fddae57..52e2eaf 100644 --- a/packages/api/src/core/errors.ts +++ b/packages/api/src/core/errors.ts @@ -6,17 +6,23 @@ // ============================================================================ const SENSITIVE_PATTERNS: RegExp[] = [ - // Absolute filesystem paths (macOS/Linux/Windows) - /(?:\/(?:Users|home|var|etc|tmp|opt|private|srv)\/|[A-Za-z]:\\)/, + // Absolute filesystem paths (POSIX with >=2 segments, or Windows drive paths). + // Matches any `/seg1/seg2...` (e.g. /app, /srv, /workspace, /Users, ...) + // rather than an enumerated allowlist of roots, so container WORKDIRs like + // /app are covered without needing to keep the list in sync. + /(?:\/[^\s/\\:*?"<>|]+\/[^\s/\\:*?"<>|]+|[A-Za-z]:\\)/, // Connection strings / URLs with credentials /(?:postgres(?:ql)?|mysql|mongodb(?:\+srv)?|redis|amqp):\/\//i, /\/\/[^\s/]+:[^\s@]+@/, // Stack trace lines /\bat\s+.+\(.+:\d+:\d+\)/, /\.(?:ts|js|tsx|jsx|mjs|cjs):\d+:\d+/, - // Secrets / tokens / keys - /(?:api[_-]?key|secret|token|password|passwd|authorization|bearer)\s*[:=]/i, - /\b(?:sk|pk|ghp|gho|ghs|xox[abps])_[A-Za-z0-9]{8,}/, + // Secrets / tokens / keys. Matches "api key: ...", "API key provided: ...", + // "secret =", etc. — any sensitive-noun phrase followed eventually by a + // colon/equals, not just an immediate `key:`. + /(?:api[_-]?\s*key|secret|token|password|passwd|authorization|bearer)\b[^:=\n]{0,20}[:=]/i, + // Vendor-prefixed credential-looking tokens (sk_, sk-, sk-proj-, pk_, ghp_, xoxb-, ...) + /\b(?:sk|pk|ghp|gho|ghs|xox[abps])[_-][A-Za-z0-9-]{6,}/, // Env var dumps /\bprocess\.env\b/, // Internal hosts