diff --git a/apps/cloud/src/auth/errors.ts b/apps/cloud/src/auth/errors.ts index 6c3c8e3ac2..5720941d11 100644 --- a/apps/cloud/src/auth/errors.ts +++ b/apps/cloud/src/auth/errors.ts @@ -228,7 +228,7 @@ export const withServiceLogging = ( effect: Effect.Effect, ): Effect.Effect => effect.pipe( - Effect.tapCause((cause) => Effect.logError(`${name} failed`, cause)), + Effect.tapCause(() => Effect.logError(`${name} failed`)), Effect.mapError(publicError), Effect.withSpan(name), ) as Effect.Effect; diff --git a/apps/cloud/src/auth/handlers.ts b/apps/cloud/src/auth/handlers.ts index b378771af7..f5e42b16ea 100644 --- a/apps/cloud/src/auth/handlers.ts +++ b/apps/cloud/src/auth/handlers.ts @@ -507,7 +507,7 @@ export const CloudSessionAuthHandlers = HttpApiBuilder.group( Effect.gen(function* () { yield* Effect.logWarning( "createOrganization: could not provision the Autumn customer", - { organizationId: org.id, error }, + { organizationId: org.id }, ); yield* captureCauseEffect(error); }), @@ -629,10 +629,10 @@ export const CloudSessionAuthHandlers = HttpApiBuilder.group( { organizationId }, ), ), - Effect.tapError((error) => + Effect.tapError(() => Effect.logError( "deleteOrganization: org marked deleted but the Autumn customer could not be deleted; retry the deletion", - { organizationId, error }, + { organizationId }, ), ), Effect.mapError(() => new OrganizationDeletionIncomplete({ step: "billing" })), @@ -672,10 +672,10 @@ export const CloudSessionAuthHandlers = HttpApiBuilder.group( s.deleteOrganizationCascade(organizationId, deletedAt), ) .pipe( - Effect.tapError((error) => + Effect.tapError(() => Effect.logError( "deleteOrganization: org marked deleted, removed from WorkOS and Autumn, but local purge failed, tenant data and secrets orphaned; retry the deletion", - { organizationId, error }, + { organizationId }, ), ), ); diff --git a/apps/cloud/src/auth/workos-events-runner.ts b/apps/cloud/src/auth/workos-events-runner.ts index e5979e48b4..9990681e97 100644 --- a/apps/cloud/src/auth/workos-events-runner.ts +++ b/apps/cloud/src/auth/workos-events-runner.ts @@ -103,7 +103,7 @@ export const runWorkOsEventsSync = (): Promise => Effect.scoped, Effect.catchCause((cause) => Effect.gen(function* () { - yield* Effect.logError("workos_events: sync run failed", cause); + yield* Effect.logError("workos_events: sync run failed"); yield* captureCauseEffect(cause); }), ), diff --git a/apps/cloud/src/edge/referrer-policy.ts b/apps/cloud/src/edge/referrer-policy.ts new file mode 100644 index 0000000000..583019f39f --- /dev/null +++ b/apps/cloud/src/edge/referrer-policy.ts @@ -0,0 +1,9 @@ +/** Prevent document and redirect URLs (including OAuth codes) from becoming + * Referer headers. Preserve the response stream, cookies and status. WebSocket + * upgrades cannot navigate a browser and must retain their platform handle. */ +export const withPrivateReferrerPolicy = (response: Response): Response => { + if (response.status === 101) return response; + const result = new Response(response.body, response); + result.headers.set("Referrer-Policy", "no-referrer"); + return result; +}; diff --git a/apps/cloud/src/engine/execution-gate.ts b/apps/cloud/src/engine/execution-gate.ts index 60c102663f..21d6cfd1a3 100644 --- a/apps/cloud/src/engine/execution-gate.ts +++ b/apps/cloud/src/engine/execution-gate.ts @@ -175,7 +175,7 @@ export const makeExecutionLimitGate = (checkBalance: ExecutionBalanceCheck) => { Effect.catch((error: unknown) => Effect.gen(function* () { yield* Effect.sync(() => { - console.warn("[billing] execution balance check failed open:", error); + console.warn("[billing] execution balance check failed open"); }); yield* captureCauseEffect(error); return { blocked: false } as const satisfies GateDecision; diff --git a/apps/cloud/src/engine/execution-rate-limit.ts b/apps/cloud/src/engine/execution-rate-limit.ts index 04c35f5774..3eb432e618 100644 --- a/apps/cloud/src/engine/execution-rate-limit.ts +++ b/apps/cloud/src/engine/execution-rate-limit.ts @@ -132,7 +132,7 @@ const failOpen = ( "rate_limit.check.error_tag": outcome.errorTag, }); yield* Effect.sync(() => { - console.warn("[rate-limit] execution rate limit check failed open:", error); + console.warn("[rate-limit] execution rate limit check failed open"); }); if (!outcome.timedOut) yield* captureCauseEffect(error); return { blocked: false } as const satisfies GateDecision; @@ -303,7 +303,6 @@ export const makeExecutionRateLimiter = ( `[rate-limit] exemption lookup failed for ${organizationId}; treating as ${ cached ? "last known" : "not exempt" }:`, - error, ); }); yield* captureCauseEffect(error); diff --git a/apps/cloud/src/extensions/billing/member-seats.ts b/apps/cloud/src/extensions/billing/member-seats.ts index 7ed5f2ab19..5ca8c16c23 100644 --- a/apps/cloud/src/extensions/billing/member-seats.ts +++ b/apps/cloud/src/extensions/billing/member-seats.ts @@ -64,10 +64,9 @@ export const forkReportMemberSeats = ( waitUntil(Effect.runPromise(autumn.setMemberSeats(organizationId, seats))); }); }).pipe( - Effect.catch((error) => + Effect.catch(() => Effect.logWarning("reportMemberSeats: seat recount failed", { organizationId, - error, }), ), Effect.withSpan("billing.reportMemberSeats"), diff --git a/apps/cloud/src/extensions/billing/route.ts b/apps/cloud/src/extensions/billing/route.ts index e353c6c2be..d6205dc0e1 100644 --- a/apps/cloud/src/extensions/billing/route.ts +++ b/apps/cloud/src/extensions/billing/route.ts @@ -1,5 +1,5 @@ import { env } from "cloudflare:workers"; -import { Cause, Effect } from "effect"; +import { Effect } from "effect"; import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstable/http"; import { autumnHandler } from "autumn-js/backend"; @@ -127,7 +127,7 @@ const handler = Effect.gen(function* () { ); if (statusCode >= 400) { - console.error("[autumn] upstream error:", statusCode, response); + console.error("[autumn] upstream error", { status: statusCode }); return yield* new HttpResponseError({ status: statusCode, code: "billing_request_failed", @@ -139,7 +139,7 @@ const handler = Effect.gen(function* () { }).pipe( Effect.catchCause((err) => { if (isServerError(err)) { - console.error("[autumn] request failed:", Cause.pretty(err)); + console.error("[autumn] request failed", { status: 500 }); } return toErrorServerResponseEffect(err); }), diff --git a/apps/cloud/src/extensions/billing/service.ts b/apps/cloud/src/extensions/billing/service.ts index af04a5b829..75de6e1fb6 100644 --- a/apps/cloud/src/extensions/billing/service.ts +++ b/apps/cloud/src/extensions/billing/service.ts @@ -182,7 +182,7 @@ const make = Effect.sync(() => { // Silent billing data loss is worth paging on: autumn.trackExecution // is fire-and-forget so the caller doesn't handle it themselves. yield* Effect.sync(() => { - console.error("[billing] track failed:", error); + console.error("[billing] track failed"); }); yield* captureCauseEffect(error); yield* Effect.annotateCurrentSpan({ "autumn.track.failed": true }); @@ -217,7 +217,7 @@ const make = Effect.sync(() => { // Silent seat drift means wrong invoices, so failures page just // like a lost execution track. yield* Effect.sync(() => { - console.error("[billing] seat sync failed:", error); + console.error("[billing] seat sync failed"); }); yield* captureCauseEffect(error); yield* Effect.annotateCurrentSpan({ "autumn.members.failed": true }); diff --git a/apps/cloud/src/observability/error-logging.ts b/apps/cloud/src/observability/error-logging.ts index 008bbaf653..2583534472 100644 --- a/apps/cloud/src/observability/error-logging.ts +++ b/apps/cloud/src/observability/error-logging.ts @@ -1,17 +1,6 @@ import { Cause, Effect, Option, Predicate, Result } from "effect"; import { HttpRouter, HttpServerRequest } from "effect/unstable/http"; -const MAX_LOGGED_CAUSE_CHARS = 4_000; - -const truncate = (value: string): string => - value.length <= MAX_LOGGED_CAUSE_CHARS - ? value - : `${value.slice(0, MAX_LOGGED_CAUSE_CHARS)}\n...[truncated ${ - value.length - MAX_LOGGED_CAUSE_CHARS - } chars]`; - -const loggedCause = (cause: Cause.Cause): string => truncate(Cause.pretty(cause)); - const objectValue = (value: unknown, key: string): unknown => Predicate.hasProperty(value, key) ? value[key] : undefined; @@ -65,7 +54,8 @@ export const logApiErrorCause = ( path: requestPath(request), status: httpStatus(error), errorTag: errorTag(error), - cause: loggedCause(cause), + // Error causes can contain provider responses, request bodies, and secrets. + // Keep only the classification; never serialize the exception or its stack. }); }; diff --git a/apps/cloud/src/observability/index.ts b/apps/cloud/src/observability/index.ts index 88b88419e5..526b9bd905 100644 --- a/apps/cloud/src/observability/index.ts +++ b/apps/cloud/src/observability/index.ts @@ -7,7 +7,7 @@ // `withObservability` (in @executor-js/api) wraps every handler effect; when // it sees an unmapped cause it asks `ErrorCapture.captureException` for a // trace id and fails with `InternalError({ traceId })`. The client gets -// the opaque id, we get the full cause + stack in Sentry. +// the opaque id; Sentry receives a minimized diagnostic event. // --------------------------------------------------------------------------- import * as Sentry from "@sentry/cloudflare"; @@ -15,16 +15,12 @@ import type { ErrorEvent, Scope } from "@sentry/cloudflare"; import { Cause, Effect, Layer, Predicate } from "effect"; import type * as Tracer from "effect/Tracer"; +import { minimizeDiagnosticTags, minimizeSentryEvent } from "./sentry-privacy"; + import { ErrorCapture } from "@executor-js/api"; import { classifyDurableObjectError } from "@executor-js/cloudflare/mcp/durable-object-errors"; import { withStableGroupingFingerprint } from "@executor-js/sdk/sentry-grouping"; -// Drizzle/postgres-js include the failing SQL (params + bound values) in -// their error message. For OpenAPI source inserts that's 1MB+ of spec -// text which blows past terminal scrollback and hides the actual pg -// error. Sentry still receives the full, untruncated cause via -// `setExtra`; only the dev-console mirror is capped. -const MAX_CONSOLE_CAUSE_CHARS = 4_000; const OTEL_TRACE_ID_PATTERN = /^[0-9a-f]{32}$/; const OTEL_SPAN_ID_PATTERN = /^[0-9a-f]{16}$/; @@ -52,11 +48,6 @@ export type OtelCorrelationContext = { readonly spanId: string; }; -const truncate = (s: string): string => - s.length <= MAX_CONSOLE_CAUSE_CHARS - ? s - : `${s.slice(0, MAX_CONSOLE_CAUSE_CHARS)}\n…[truncated ${s.length - MAX_CONSOLE_CAUSE_CHARS} chars]`; - const validOtelContext = (context: OtelCorrelationContext): boolean => OTEL_TRACE_ID_PATTERN.test(context.traceId) && OTEL_SPAN_ID_PATTERN.test(context.spanId); @@ -212,7 +203,7 @@ export const beforeSendCloudEvent = ( options?: { readonly logPayload?: boolean }, ): ErrorEvent | null => { const reported = beforeSendWithOtelCorrelation(event, options); - return reported === null ? null : withStableGroupingFingerprint(reported); + return reported === null ? null : withStableGroupingFingerprint(minimizeSentryEvent(reported)); }; /** @@ -230,8 +221,8 @@ export const beforeSendCloudEvent = ( export const cloudSentryOptions = (env: Env) => ({ dsn: env.SENTRY_DSN, tracesSampleRate: 0, - enableLogs: true, - sendDefaultPii: true, + enableLogs: false, + sendDefaultPii: false, skipOpenTelemetrySetup: true, beforeSend: (event: ErrorEvent) => beforeSendCloudEvent(event, { @@ -274,11 +265,12 @@ export const sentryPayloadForCause = ( // operation and no reason in it at all. Tags survive, group, and are // searchable. Values are failure modes and operation names; never a query, a // value, or anything customer-derived. -const CLASSIFICATION_TAG_FIELDS = ["operation", "reason", "status"] as const; +const CLASSIFICATION_TAG_FIELDS = ["operation", "reason", "status", "code"] as const; /** The errors those fields are read from. An allowlist, because the fields are * only known to be safe on the errors this app defines. */ const CLASSIFIED_ERROR_TAGS = [ + "StorageError", "UserStoreError", "WorkOSError", "McpSessionMetaUnavailableError", @@ -323,7 +315,7 @@ const classificationTagsOf = (input: unknown): Readonly> } } } - return tags; + return minimizeDiagnosticTags(tags); }; export const captureCause = ( @@ -365,8 +357,15 @@ export const ErrorCaptureLive: Layer.Layer = Layer.succeed( ErrorCapture.of({ captureException: (cause) => Effect.gen(function* () { - console.error("[api] unhandled cause:", truncate(Cause.pretty(cause))); - return (yield* captureCauseEffect(cause)) ?? ""; + const eventId = (yield* captureCauseEffect(cause)) ?? ""; + console.error( + JSON.stringify({ + event: "api_unhandled_cause", + sentry_event_id: eventId, + tags: classificationTagsOf(cause), + }), + ); + return eventId; }), }), ); diff --git a/apps/cloud/src/observability/redact-span-urls.test.ts b/apps/cloud/src/observability/redact-span-urls.test.ts index 288c0f65ce..064f7a1e8b 100644 --- a/apps/cloud/src/observability/redact-span-urls.test.ts +++ b/apps/cloud/src/observability/redact-span-urls.test.ts @@ -216,13 +216,13 @@ describe("UrlRedactingSpanProcessor", () => { span.setStatus({ code: SpanStatusCode.ERROR, message }); }); - // Non-vacuous: the exception event exists and kept its scrubbed URL. + // The exception event and classification survive without raw error text. const events = JSON.stringify(exported?.events); expect(events).toContain("exception"); - expect(events).toContain("https://api.test/graphql"); + expect(events).toContain("[REDACTED]"); expect(events).not.toContain("synthetic-userinfo-secret"); expect(events).not.toContain("synthetic-key-secret"); - expect(exported?.status.message).toBe("Transport: fetch failed (GET https://api.test/graphql)"); + expect(exported?.status.message).toBe("[REDACTED]"); }); }); diff --git a/apps/cloud/src/observability/redact-span-urls.ts b/apps/cloud/src/observability/redact-span-urls.ts index ced0513214..ff1e126bc1 100644 --- a/apps/cloud/src/observability/redact-span-urls.ts +++ b/apps/cloud/src/observability/redact-span-urls.ts @@ -90,6 +90,9 @@ export class UrlRedactingSpanProcessor implements SpanProcessor { // Work on a copy so the redaction decision is made from the current values // and applied through the caller's writer (span API vs direct mutation). const draft: Record = { ...span.attributes }; + for (const key of ["exception.message", "exception.stacktrace"]) { + if (key in draft) draft[key] = "[REDACTED]"; + } const stripped = new Set(redactSpanUrlAttributes(draft)); for (const [name, value] of Object.entries(draft)) { if (typeof value === "string" && value !== span.attributes[name]) write(name, value); @@ -100,6 +103,9 @@ export class UrlRedactingSpanProcessor implements SpanProcessor { // own attributes get, since a link carries an arbitrary attribute bag. for (const link of span.links) { if (link.attributes === undefined) continue; + for (const key of ["exception.message", "exception.stacktrace"]) { + if (key in link.attributes) link.attributes[key] = "[REDACTED]"; + } for (const key of redactSpanUrlAttributes(link.attributes)) stripped.add(key); } @@ -110,6 +116,10 @@ export class UrlRedactingSpanProcessor implements SpanProcessor { if (name !== event.name) event.name = name; if (event.attributes === undefined) continue; for (const [key, value] of Object.entries(event.attributes)) { + if (key === "exception.message" || key === "exception.stacktrace") { + event.attributes[key] = "[REDACTED]"; + continue; + } // Event attributes permit string[] exactly as span attributes do, so // array elements get the same free-text scrub, in place. if (Array.isArray(value)) { @@ -124,8 +134,7 @@ export class UrlRedactingSpanProcessor implements SpanProcessor { const message = span.status.message; if (typeof message === "string") { - const redacted = redactUrlsInText(message); - if (redacted !== message) span.status.message = redacted; + span.status.message = "[REDACTED]"; } } } diff --git a/apps/cloud/src/observability/sentry-privacy.ts b/apps/cloud/src/observability/sentry-privacy.ts new file mode 100644 index 0000000000..a720356e3d --- /dev/null +++ b/apps/cloud/src/observability/sentry-privacy.ts @@ -0,0 +1,122 @@ +import type { ErrorEvent } from "@sentry/cloudflare"; +import { Match, Option } from "effect"; + +const ERROR_TYPES = new Set([ + "Error", + "TypeError", + "RangeError", + "ReferenceError", + "SyntaxError", + "URIError", + "AggregateError", + "UserStoreError", + "WorkOSError", + "McpSessionMetaUnavailableError", + "GateCheckTimeoutError", + "AutumnError", + "StorageError", + "ResponseError", + "RequestError", + "FrontendHandledError", +]); + +const OPERATIONS = new Set([ + "ensureAccount", + "getAccount", + "upsertOrganization", + "getOrganization", + "getOrganizationBySlug", + "markOrganizationDeleted", + "deleteOrganizationCascade", +]); +// Only built-in model/method vocabulary can be reported; plugin-supplied names +// and arbitrary operation strings never become diagnostic labels. +const STORAGE_OPERATION = + /^(?:connection|integration|tool|policy|credential|plugin_storage|execution|oauth_client)\.(?:create|update|delete|findFirst|findMany|count|upsert)$/; +const REASONS = new Set(["connect_timeout", "connection_closed", "query", "unknown", "upstream"]); + +// A field name alone does not make its contents safe. Accept only the values +// generated by these diagnostic contracts, including exact correlation widths. +const safeTag = (key: string, value: unknown): boolean => { + if (typeof value !== "string" && typeof value !== "number") return false; + const text = String(value); + return Match.value(key).pipe( + Match.when("otel_trace_id", () => /^[0-9a-f]{32}$/.test(text)), + Match.when("otel_span_id", () => /^[0-9a-f]{16}$/.test(text)), + Match.when("operation", () => OPERATIONS.has(text) || STORAGE_OPERATION.test(text)), + Match.when("code", () => /^[0-9A-Z]{5}$/.test(text)), + Match.when("executor.ui.surface", () => text === "api_client"), + Match.when("executor.ui.action", () => text === "decode_or_transport"), + Match.when("executor.ui.severity", () => text === "error" || text === "warning"), + Match.when("reason", () => REASONS.has(text)), + Match.when("status", () => /^[1-5][0-9]{2}$/.test(text)), + Match.when("mcp.do.cause_owner", () => text === "durable_object"), + Match.option, + Option.getOrElse(() => false), + ); +}; + +/** Project known diagnostic values before they reach logs or a reporter. */ +export const minimizeDiagnosticTags = ( + tags: Readonly>, +): Record => + Object.fromEntries( + Object.entries(tags) + .filter(([key, value]) => safeTag(key, value)) + .map(([key, value]) => [key, String(value)]), + ); + +const identifier = (value: string | undefined): string | undefined => + value !== undefined && /^[\w.$<>:/@ -]{1,160}$/.test(value) ? value : undefined; + +const sourceFile = (value: string | undefined): string | undefined => { + if (!value) return undefined; + const path = URL.canParse(value) ? new URL(value).pathname : value.split(/[?#]/)[0]; + return path && /^\/?(?:assets\/)?[\w./-]+\.(?:js|mjs|ts|tsx)$/.test(path) ? path : undefined; +}; + +/** Keep error classification, source positions and correlation; omit all raw payloads. */ +export const minimizeSentryEvent = (event: ErrorEvent): ErrorEvent => { + const tags = minimizeDiagnosticTags(event.tags ?? {}); + return { + type: undefined, + event_id: event.event_id, + timestamp: event.timestamp, + platform: event.platform, + level: event.level, + release: event.release, + environment: event.environment, + tags, + exception: { + values: event.exception?.values?.map((exception) => ({ + type: + exception.type !== undefined && ERROR_TYPES.has(exception.type) + ? exception.type + : "Error", + value: + exception.type === "StorageError" && tags.operation + ? `${tags.operation} failed${tags.code ? ` (${tags.code})` : ""}` + : tags["executor.ui.surface"] === "api_client" + ? "API request failed (decode_or_transport)" + : "Details omitted to protect request data", + mechanism: exception.mechanism + ? { + type: identifier(exception.mechanism.type) ?? "generic", + handled: exception.mechanism.handled, + synthetic: exception.mechanism.synthetic, + } + : undefined, + stacktrace: { + frames: exception.stacktrace?.frames?.map((frame) => ({ + filename: sourceFile(frame.filename), + function: identifier(frame.function), + module: identifier(frame.module), + lineno: frame.lineno, + colno: frame.colno, + in_app: frame.in_app, + })), + }, + })), + }, + }; +}; diff --git a/apps/cloud/src/routes/__root.tsx b/apps/cloud/src/routes/__root.tsx index 1c0f8b8a10..40d88322f1 100644 --- a/apps/cloud/src/routes/__root.tsx +++ b/apps/cloud/src/routes/__root.tsx @@ -27,6 +27,7 @@ import { AuthProvider, useAuth } from "../web/auth"; import { loginPath } from "../auth/return-to"; import { ONBOARDING_PATHS, PUBLIC_PATHS } from "../auth/route-paths"; import { SupportOptions } from "../web/components/support-options"; +import { minimizeSentryEvent } from "../observability/sentry-privacy"; import { Shell } from "../web/shell"; import appCss from "@executor-js/react/globals.css?url"; @@ -35,6 +36,9 @@ if (typeof window !== "undefined" && import.meta.env.VITE_PUBLIC_SENTRY_DSN) { dsn: import.meta.env.VITE_PUBLIC_SENTRY_DSN, tunnel: "/api/sentry-tunnel", tracesSampleRate: 0, + sendDefaultPii: false, + enableLogs: false, + beforeSend: minimizeSentryEvent, replaysSessionSampleRate: 0.1, replaysOnErrorSampleRate: 1.0, }); diff --git a/apps/cloud/src/server.ts b/apps/cloud/src/server.ts index 71905e352f..2dde0e8403 100644 --- a/apps/cloud/src/server.ts +++ b/apps/cloud/src/server.ts @@ -13,6 +13,7 @@ import handler from "@tanstack/react-start/server-entry"; import { isAppOwnedPath, servedByAppPlane } from "./app-paths"; import { marketingProxyRequest } from "./edge/marketing"; import { passthroughResponse } from "./edge/passthrough"; +import { withPrivateReferrerPolicy } from "./edge/referrer-policy"; import { runWorkOsEventsSync } from "./auth/workos-events-runner"; import { makeCloudMcpAgentHandler } from "./mcp/agent-handler"; import { classifyMcpPath, prepareMcpOrgScope } from "./mcp/mount"; @@ -303,7 +304,7 @@ const prewarmAppPlane = (ctx: ExecutionContext): void => { ); }; -const cloudflareHandler: ExportedHandler = { +const cloudflareHandler = { fetch: async (request, env, ctx) => { isolateRequestSeq += 1; @@ -502,6 +503,10 @@ const cloudflareHandler: ExportedHandler = { await runWorkOsEventsSync(); ctx.waitUntil(flushTracerProvider()); }, -}; +} satisfies ExportedHandler; -export default Sentry.withSentry(cloudSentryOptions, cloudflareHandler); +export default Sentry.withSentry(cloudSentryOptions, { + ...cloudflareHandler, + fetch: async (request, env, ctx) => + withPrivateReferrerPolicy(await cloudflareHandler.fetch(request, env, ctx)), +}); diff --git a/apps/cloud/wrangler.jsonc b/apps/cloud/wrangler.jsonc index 4ee8b7a2b4..ac07846ebc 100644 --- a/apps/cloud/wrangler.jsonc +++ b/apps/cloud/wrangler.jsonc @@ -27,6 +27,7 @@ }, "observability": { "enabled": true, + "redact_query_string": true, }, "ratelimits": [ { diff --git a/e2e/cloud/frontend-error-reporting.test.ts b/e2e/cloud/frontend-error-reporting.test.ts index 5c9b731281..9df2e68b09 100644 --- a/e2e/cloud/frontend-error-reporting.test.ts +++ b/e2e/cloud/frontend-error-reporting.test.ts @@ -1,16 +1,6 @@ -// Cloud (browser): a failed API request is reported as a titled error. -// -// The console reports handled UI failures to the crash reporter, and every -// producer of one starts from an Effect `Cause` — a plain object with no name, -// message or stack. Handed that directly, the reporter has nothing to title -// the report with, so it files a message-less one and groups it on the -// reporting frame: unrelated frontend failures all land in a single nameless -// bucket that says only which function did the reporting, never what broke. -// -// The report is the product surface here, so this scenario reads it the way -// the outside world does. The browser SDK is configured to POST its envelopes -// same-origin (`tunnel`), so the suite intercepts that request and asserts on -// the payload the page actually tried to send. +// Inspect the browser's actual Sentry envelope for a failed API request. +// Reporting must preserve classification and source positions while omitting +// the response body, request credentials and raw error message. import { expect } from "@effect/vitest"; import { Effect } from "effect"; @@ -25,6 +15,9 @@ type ReportedException = { // that was not a real error and had to invent a stack for it — the stack of // whatever frame did the reporting. readonly mechanism?: { readonly synthetic?: boolean }; + readonly stacktrace?: { + readonly frames?: ReadonlyArray<{ readonly filename?: string; readonly lineno?: number }>; + }; }; type ReportedEvent = { @@ -51,7 +44,7 @@ const errorEventsIn = (body: string): ReadonlyArray => }); scenario( - "Frontend errors · a failed API request is reported with a real message", + "Frontend errors · a failed API request is reported without request data", { timeout: 120_000 }, Effect.gen(function* () { const browser = yield* Browser; @@ -89,7 +82,7 @@ scenario( await route.fulfill({ status: 500, contentType: "text/plain", - body: "upstream exploded", + body: "SYNTHETIC_PRIVATE_RESPONSE_MARKER", }); }); await revisit(page); @@ -101,30 +94,40 @@ scenario( // notices failures, so wait for one rather than sleeping. await expect .poll(() => reportedFailures().map((failure) => failure.value ?? ""), { - message: "the reported failure says which request failed, and how", + message: "the failed API request produces a classified report", timeout: 20_000, }) - .toContainEqual(expect.stringMatching(/500 .*\/api\/integrations/)); + .toContain("API request failed (decode_or_transport)"); + const serialized = JSON.stringify(reports); + expect(serialized).not.toContain("SYNTHETIC_PRIVATE_RESPONSE_MARKER"); + expect(serialized).not.toContain("500 GET"); + for (const credential of Object.values(identity.headers ?? {})) { + expect(serialized, "request credentials stay out of the report").not.toContain(credential); + } + const apiReports = reports.filter( + (event) => event.tags?.["executor.ui.surface"] === "api_client", + ); + expect(apiReports.length).toBeGreaterThan(0); + for (const report of apiReports) { + expect(report.tags).toMatchObject({ + "executor.ui.surface": "api_client", + "executor.ui.action": "decode_or_transport", + "executor.ui.severity": "error", + }); + } + expect( + reportedFailures().some((failure) => + failure.stacktrace?.frames?.some( + (frame) => frame.filename !== undefined && frame.lineno !== undefined, + ), + ), + "reported failures retain actionable source positions", + ).toBe(true); for (const failure of reportedFailures()) { - // A report with no message is the bug: it cannot be titled, so it - // groups on the reporting frame and swallows every other failure. - expect(failure.value ?? "", "every report carries a message").not.toBe(""); - expect(failure.type ?? "", "every report carries an error name").not.toBe(""); - // What a reporter falls back to when it is handed something that is - // not an error at all — the shape every message-less report had. - expect(failure.value ?? "", "no report is a bag of keys").not.toMatch( - /captured as exception with keys/, - ); - // The other half of the bug, and the half a readable message can hide: - // handed a non-error, the reporter still has no stack of its own to - // group on and invents one from the reporting frame, so unrelated - // failures keep merging into a single bucket. Only a real error clears - // this flag. - expect( - failure.mechanism?.synthetic ?? false, - "the report carries the failure's own stack, not the reporter's frame", - ).toBe(false); + expect(failure.value).toBe("API request failed (decode_or_transport)"); + expect(failure.type).toBeTruthy(); + expect(failure.mechanism?.synthetic, "the failure carries its own stack").not.toBe(true); } await page.unroute("**/api/integrations"); diff --git a/e2e/cloud/storage-error-report-shape.test.ts b/e2e/cloud/storage-error-report-shape.test.ts index ab8a848f4c..25efa44292 100644 --- a/e2e/cloud/storage-error-report-shape.test.ts +++ b/e2e/cloud/storage-error-report-shape.test.ts @@ -1,36 +1,12 @@ -// Cloud-only: what an operator SEES when a write is rejected by the database. -// -// The product guarantee: a storage failure is reported under a stable headline -// built from the operation and the database's error code — never the statement -// text, never the values that were bound into it. Two consequences, both of -// them things production got wrong: -// -// - The values bound into a rejected statement are customer data (the -// organization id, the connection name, whatever the user typed into the -// description). They must not appear in the report's headline. -// - The headline is the grouping key of the error reporter, so one defect that -// hits several tables — or the same table through several WHERE shapes — -// must arrive as ONE report, not one per statement. -// -// The failure is induced through the public typed API only: PostgreSQL cannot -// store a NUL byte in a text column, so a connection whose description carries -// one is rejected by the driver with SQLSTATE 22021 while the statement and its -// bound parameters are already assembled. That is the same class of failure the -// production reports came from, reachable without touching the database. -// -// Two surfaces are asserted, both public: -// 1. What the CALLER gets — an opaque `InternalError` carrying only a trace -// id, with no driver text anywhere in the payload. -// 2. What the OPERATOR gets — the server's own error log, where the trace id -// the caller received joins to the report the server filed. Its headline — -// the captured exception's type and message — is what the error reporter -// files the report under, and groups by. +// Exercise a real rejected database write through the typed API. The caller +// receives an opaque correlation ID; the operator gets the same ID plus the +// operation and SQLSTATE, without SQL, bound values or raw driver causes. import { randomBytes } from "node:crypto"; import { readFileSync } from "node:fs"; import { resolve } from "node:path"; import { expect } from "@effect/vitest"; -import { Cause, Effect, Exit, Schedule } from "effect"; +import { Cause, Effect, Exit, Schedule, Schema } from "effect"; import type { HttpApiClient } from "effect/unstable/httpapi"; import { composePluginApi } from "@executor-js/api/server"; import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; @@ -159,62 +135,39 @@ const readServerLog = (): string => { return texts.join("\n"); }; -const REPORT_PREFIX = "[api] unhandled cause: "; - -/** A stack frame in the logged cause — where the report's headline stops. */ -const STACK_FRAME = /^\s+at /; - -interface FiledReport { - /** Type + message: what the reporter names and groups the report by. */ - readonly headline: string; - /** The whole record, headline and chained cause — what a diagnosis reads. */ - readonly full: string; -} +const decodeReport = Schema.decodeUnknownOption( + Schema.Struct({ + event: Schema.Literal("api_unhandled_cause"), + sentry_event_id: Schema.String, + tags: Schema.Record(Schema.String, Schema.String), + }), +); -/** - * The report the server filed for one request. - * - * The headline is the captured cause's type and message up to the first stack - * frame — exactly what `Cause.prettyErrors` hands the reporter as the - * exception. The message is multi-line whenever the driver's text is - * (`Failed query: …\nparams: …`), so the whole headline has to be read, not - * just its first line. - * - * Found by walking back from the correlation record carrying the caller's trace - * id, so it is THIS request's report and not a neighbour's. - */ -const reportFor = (traceId: string): Effect.Effect => +/** Find the structured operator report by the ID returned to the caller. */ +const reportFor = (traceId: string) => Effect.sync(() => { - const lines = readServerLog().split("\n"); - const correlated = lines.findLastIndex( - (line) => - line.includes('"event":"sentry_before_send_otel_correlation"') && - line.includes(`"sentry_event_id":"${traceId}"`), - ); - if (correlated === -1) return undefined; - const reported = lines - .slice(0, correlated) - .findLastIndex((line) => line.startsWith(REPORT_PREFIX)); - if (reported === -1) return undefined; - const block = [ - lines[reported]!.slice(REPORT_PREFIX.length), - ...lines.slice(reported + 1, correlated), - ]; - const end = block.slice(1).findIndex((line) => STACK_FRAME.test(line)); - return { - headline: block - .slice(0, end === -1 ? 1 : end + 1) - .join("\n") - .trimEnd(), - full: block.join("\n"), - }; + for (const line of readServerLog().split("\n")) { + if (!line.startsWith("{")) continue; + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: mixed stdout includes non-JSON records + try { + const report = decodeReport(JSON.parse(line)); + if (report._tag === "Some" && report.value.sentry_event_id === traceId) { + return { + headline: JSON.stringify(report.value.tags), + full: line, + tags: report.value.tags, + }; + } + } catch { + /* Other stdout records are not diagnostic envelopes. */ + } + } + return undefined; }).pipe( Effect.filterOrFail( - (report): report is FiledReport => report !== undefined, + (report) => report !== undefined, () => `no error report joined to trace id ${traceId} in the server log`, ), - // The log is a file the dev stack appends to; the write lands moments after - // the response. Poll rather than sleep (~20s ceiling). Effect.retry(Schedule.both(Schedule.spaced("500 millis"), Schedule.recurs(40))), ); @@ -261,12 +214,16 @@ scenario( expect(headline, "the report still names the failing operation").toContain("connection.create"); expect(headline, "the report still names the database's error code").toContain("22021"); - // Shaping the headline must not mean throwing the diagnosis away: the - // driver's own text is still filed with the report, one level down, where - // it informs a fix instead of naming the report. - expect(report.full, "the driver's statement is still filed under the report").toContain( + expect(report.tags).toMatchObject({ operation: "connection.create", code: "22021" }); + for (const forbidden of [ "Failed query", - ); + "insert into", + "params:", + first.name, + first.description, + ]) { + expect(report.full, "the complete report omits SQL and caller data").not.toContain(forbidden); + } // The fan-out: the two writes bound different names, descriptions and // secrets, so their statements differ in every parameter. One defect, one diff --git a/packages/core/sdk/src/connections.test.ts b/packages/core/sdk/src/connections.test.ts index 1fd653c093..28db1f9618 100644 --- a/packages/core/sdk/src/connections.test.ts +++ b/packages/core/sdk/src/connections.test.ts @@ -2630,10 +2630,11 @@ describe("tool catalog sync safety", () => { ); expect(failureWarning).toBeDefined(); expect(failureWarning).toContain("broken"); - // Both halves: the failure and the cause that names what to fix. A bare - // structural render of the error drops the cause entirely. - expect(failureWarning).toContain("upstream listing refused"); - expect(failureWarning).toContain("connect ECONNREFUSED"); + // Keep the failure class and affected connection, without raw provider + // text or nested causes that can carry credentials or SQL values. + expect(failureWarning).toContain("StorageError"); + expect(failureWarning).not.toContain("upstream listing refused"); + expect(failureWarning).not.toContain("connect ECONNREFUSED"); // The healthy peer is not swept into the failure. expect(failureWarning).not.toContain("healthy"); }), diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index fdbc9b7671..0f338405cd 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -911,23 +911,6 @@ const storageFailureFromUnknown = (message: string, cause: unknown): StorageFail const pluginStorageFailure = (pluginId: string, hook: string, cause: unknown): StorageFailure => storageFailureFromUnknown(`${hook} failed for plugin ${pluginId}`, cause); -// oxlint-disable executor/no-instanceof-error, executor/no-unknown-error-message -- boundary: render an arbitrary failure into one readable log field -/** One-line rendering of a failed rebuild, for the operator-facing warning. - * A `StorageError` carries the actionable detail in its `cause` (the plugin's - * own failure) while its own message only names the hook, and structural - * stringification drops a `cause` that is an `Error` — so unwrap one level and - * keep both halves. */ -const describeSyncFailure = (error: unknown): string => { - const base = - error instanceof Error && error.message.length > 0 - ? error.message - : Inspectable.toStringUnknown(error, 0); - const cause = (error as { readonly cause?: unknown } | null | undefined)?.cause; - if (cause instanceof Error && cause.message.length > 0) return `${base}: ${cause.message}`; - return base; -}; -// oxlint-enable executor/no-instanceof-error, executor/no-unknown-error-message - const createDefaultMemoryDb = (tables: FumaTables): ExecutorDb => { const version = "1.0.0"; const latestSchema = fumaSchema>({ @@ -4228,11 +4211,11 @@ export const createExecutor = Effect.logWarning("executor stale tool sync scan failed", { - error: describeSyncFailure(error), + errorTag: Predicate.isTagged(error, "StorageError") ? "StorageError" : "Unknown", }), ), ), diff --git a/packages/core/sdk/src/fuma-runtime.ts b/packages/core/sdk/src/fuma-runtime.ts index 7d2a476405..bb7b7a1e57 100644 --- a/packages/core/sdk/src/fuma-runtime.ts +++ b/packages/core/sdk/src/fuma-runtime.ts @@ -4,6 +4,9 @@ import type { AnySchema, AnyTable, Schema as FumaSchema } from "@executor-js/fum export class StorageError extends Data.TaggedError("StorageError")<{ readonly message: string; + /** Structured diagnostic inputs; reporting boundaries must allowlist their values. */ + readonly operation?: string; + readonly code?: string; readonly cause: unknown; }> {} @@ -210,6 +213,8 @@ export const fumaFailureFromCause = (label: string, cause: unknown): StorageFail } return new StorageError({ message: stableMessage(label, causeCode(cause)), + operation: label, + code: causeCode(cause), cause, }); }; diff --git a/packages/core/sdk/src/oauth-service.ts b/packages/core/sdk/src/oauth-service.ts index 7d8d7d46ea..770437d908 100644 --- a/packages/core/sdk/src/oauth-service.ts +++ b/packages/core/sdk/src/oauth-service.ts @@ -14,7 +14,7 @@ // redeems the session, exchanges the code, and mints the connection. // --------------------------------------------------------------------------- -import { Duration, Effect, Exit, Layer, Match, Option, Predicate, Schema } from "effect"; +import { Cause, Duration, Effect, Exit, Layer, Match, Option, Predicate, Schema } from "effect"; import { FetchHttpClient, type HttpClient } from "effect/unstable/http"; import { connectionIdentifier } from "./connection-name-identifier"; @@ -1100,7 +1100,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { { owner: input.owner, client: String(input.slug), - cause, + causeKind: Cause.isCause(cause) ? "Cause" : "Error", }, ).pipe(Effect.as(false)), ), @@ -2084,7 +2084,9 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { ) .pipe( Effect.catch((failure) => - Effect.logWarning("executor oauth expired-session sweep failed", { cause: failure }), + Effect.logWarning("executor oauth expired-session sweep failed", { + failureType: typeof failure, + }), ), ); diff --git a/packages/core/sdk/src/subject-registry.ts b/packages/core/sdk/src/subject-registry.ts index ca02cef65a..c103c13c64 100644 --- a/packages/core/sdk/src/subject-registry.ts +++ b/packages/core/sdk/src/subject-registry.ts @@ -186,7 +186,7 @@ export const touchSubject = (db: FumaDb, input: TouchSubjectInput): Effect. Effect.logWarning("executor subject touch failed", { tenant: input.tenant, externalId: input.externalId, - cause, + failureType: typeof cause, }), ), Effect.withSpan("executor.subject.touch"), diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts index 7613459221..805dd14d16 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts @@ -1077,7 +1077,7 @@ export abstract class McpAgentSessionDOBase< try: () => candidate.dispose("cap"), catch: (cause: unknown) => cause, }).pipe( - Effect.catch((cause: unknown) => + Effect.catch(() => Effect.sync(() => { console.warn( JSON.stringify({ @@ -1085,7 +1085,7 @@ export abstract class McpAgentSessionDOBase< sessionId: candidate.sessionId, }), ); - console.error("[mcp-session] cap eviction request failed:", cause); + console.error("[mcp-session] cap eviction request failed"); }), ), ), @@ -1143,7 +1143,6 @@ export abstract class McpAgentSessionDOBase< sessionId: self.sessionIdForTelemetry(), resetKind: input.failure.kind, disposition: input.failure.disposition, - cause: Cause.pretty(input.cause), }), ); yield* Effect.annotateCurrentSpan({ @@ -1196,16 +1195,12 @@ export abstract class McpAgentSessionDOBase< }): Effect.Effect { const self = this; return Effect.gen(function* () { - const first = Cause.prettyErrors(input.cause)[0]; console.error( JSON.stringify({ event: "mcp_execution_owner_directory_error", operation: input.operation, executionId: input.executionId, sessionId: self.sessionIdForTelemetry(), - exceptionType: first?.name ?? "Error", - exceptionMessage: first?.message ?? "unknown", - cause: Cause.pretty(input.cause), }), ); yield* Effect.annotateCurrentSpan({ @@ -1222,16 +1217,12 @@ export abstract class McpAgentSessionDOBase< }): Effect.Effect { const self = this; return Effect.gen(function* () { - const first = Cause.prettyErrors(input.cause)[0]; console.error( JSON.stringify({ event: "mcp_model_resume_forward_error", executionId: input.executionId, sessionId: self.sessionIdForTelemetry(), ownerSessionId: input.owner.sessionId, - exceptionType: first?.name ?? "Error", - exceptionMessage: first?.message ?? "unknown", - cause: Cause.pretty(input.cause), }), ); yield* Effect.annotateCurrentSpan({ @@ -1535,7 +1526,7 @@ export abstract class McpAgentSessionDOBase< if (failure) { yield* self.recordDurableObjectReset({ operation: "init", failure, cause }); } else { - console.error("[mcp-session] init failed:", Cause.pretty(cause)); + console.error("[mcp-session] init failed"); yield* self.captureCauseEffect(cause); } yield* self.recordCauseOnSpan(cause); @@ -2070,10 +2061,7 @@ export abstract class McpAgentSessionDOBase< Effect.tapCause((cause) => Effect.gen(function* () { yield* Effect.sync(() => { - console.error( - "[mcp-session] pending approval lease start failed:", - Cause.pretty(cause), - ); + console.error("[mcp-session] pending approval lease start failed"); }); yield* self.captureCauseEffect(cause); }), @@ -2097,10 +2085,7 @@ export abstract class McpAgentSessionDOBase< Effect.tapCause((cause) => Effect.gen(function* () { yield* Effect.sync(() => { - console.error( - "[mcp-session] pending approval lease expiration failed:", - Cause.pretty(cause), - ); + console.error("[mcp-session] pending approval lease expiration failed"); }); yield* self.captureCauseEffect(cause); }), diff --git a/packages/hosts/mcp/src/tool-server.ts b/packages/hosts/mcp/src/tool-server.ts index f7c1a349f7..91562be746 100644 --- a/packages/hosts/mcp/src/tool-server.ts +++ b/packages/hosts/mcp/src/tool-server.ts @@ -395,11 +395,6 @@ const readDebugDefault = (): boolean => { return value === "1" || value === "true"; }; -const capabilitySnapshot = (server: McpServer) => ({ - clientCapabilities: server.server.getClientCapabilities() ?? null, - elicitationSupport: getElicitationSupport(server), -}); - class McpNativeElicitationTransportError extends Data.TaggedError( "McpNativeElicitationTransportError", )<{ @@ -835,10 +830,9 @@ const toMcpFailureResult = (cause: Cause.Cause): McpToolResult => { Predicate.isTagged("McpNativeElicitationTransportError")(defect.success); // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: best-effort defect logging must tolerate non-serializable causes try { - console.error( - `[executor:mcp] execute defect correlation_id=${correlationId}`, - Cause.pretty(cause), - ); + console.error(`[executor:mcp] execute defect correlation_id=${correlationId}`, { + nativeElicitationFailed, + }); } catch { /* ignore logger failures */ } @@ -2383,9 +2377,14 @@ export const createExecutorMcpServer = ( smoke(input.code), ).pipe( Effect.catchCause((cause) => - Effect.as(Effect.logWarning("create-artifact smoke render was unavailable", cause), { - status: "ok", - } satisfies ArtifactSmokeRenderResult), + Effect.as( + Effect.logWarning("create-artifact smoke render was unavailable", { + causeKind: Cause.isCause(cause) ? "Cause" : "Error", + }), + { + status: "ok", + } satisfies ArtifactSmokeRenderResult, + ), ), ); const renderRejection = smokeRenderRejection(smokeResult); @@ -2896,7 +2895,7 @@ export const createExecutorMcpServer = ( console.error( "[executor] MCP session mode", JSON.stringify({ - ...capabilitySnapshot(server), + elicitationSupport: getElicitationSupport(server), elicitationMode: elicitationMode.mode, resumeEnabled: elicitationMode.mode !== "native", }), diff --git a/packages/plugins/graphql/src/sdk/introspect-credential-logging.test.ts b/packages/plugins/graphql/src/sdk/introspect-credential-logging.test.ts index 24950c34c8..0727baff0d 100644 --- a/packages/plugins/graphql/src/sdk/introspect-credential-logging.test.ts +++ b/packages/plugins/graphql/src/sdk/introspect-credential-logging.test.ts @@ -31,7 +31,8 @@ const ENDPOINT = "https://graph.example.test/graphql"; * live `message` getter, which is the exact path the leak took. */ const capturingLogger = (sink: Array) => Logger.make((options) => { - sink.push(String(options.message)); + // Preserve structured log fields so the secret check covers them too. + sink.push(JSON.stringify(options.message)); sink.push(Cause.pretty(options.cause)); }); diff --git a/packages/plugins/graphql/src/sdk/introspect.ts b/packages/plugins/graphql/src/sdk/introspect.ts index d484dcdfb7..6949fc621d 100644 --- a/packages/plugins/graphql/src/sdk/introspect.ts +++ b/packages/plugins/graphql/src/sdk/introspect.ts @@ -308,7 +308,9 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* ( } const response = yield* client.execute(request).pipe( - Effect.tapCause((cause) => Effect.logError("graphql introspection request failed", cause)), + Effect.tapCause(() => + Effect.logError("graphql introspection request failed", { host: requestUrl.hostname }), + ), Effect.mapError( () => new GraphqlIntrospectionError({ @@ -339,7 +341,7 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* ( } const raw = yield* response.json.pipe( - Effect.tapCause((cause) => Effect.logError("graphql introspection JSON parse failed", cause)), + Effect.tapCause(() => Effect.logError("graphql introspection JSON parse failed")), Effect.mapError( () => new GraphqlIntrospectionError({