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/observability.test.ts b/apps/cloud/src/observability/observability.test.ts index 581377d12f..878bdb8a80 100644 --- a/apps/cloud/src/observability/observability.test.ts +++ b/apps/cloud/src/observability/observability.test.ts @@ -409,3 +409,104 @@ describe("Durable Object platform reset noise", () => { expect(options.beforeSend(event)).toBeNull(); }); }); + +describe("Sentry privacy boundary", () => { + it("rejects arbitrary values in classification tags and caller fingerprints", () => { + const secret = "SYNTHETIC_PRIVATE_MARKER"; + const sent = beforeSendCloudEvent({ + type: undefined, + fingerprint: [secret], + tags: { + operation: secret, + reason: secret, + status: secret, + otel_trace_id: secret, + otel_span_id: secret, + "mcp.do.cause_owner": secret, + code: secret, + "executor.ui.surface": secret, + "executor.ui.action": secret, + }, + exception: { values: [{ type: secret, value: secret }] }, + }); + expect(sent).not.toBeNull(); + expect(JSON.stringify(sent)).not.toContain(secret); + expect(sent?.exception?.values?.[0]?.type).toBe("Error"); + }); + + it("retains known failure classifications and disables Sentry log payloads", () => { + const options = cloudSentryOptions({ SENTRY_DSN: "https://public@example.invalid/1" } as Env); + const sent = options.beforeSend({ + type: undefined, + tags: { operation: "getOrganization", reason: "connect_timeout", status: 503 }, + }); + expect(sent?.tags).toEqual({ + operation: "getOrganization", + reason: "connect_timeout", + status: "503", + }); + expect(options.enableLogs).toBe(false); + expect(options.sendDefaultPii).toBe(false); + }); + + it("retains storage classifications without SQL or raw causes", () => { + const sent = beforeSendCloudEvent({ + type: undefined, + tags: { operation: "connection.create", code: "22021" }, + exception: { values: [{ type: "StorageError", value: "private SQL and bound values" }] }, + extra: { cause: "private SQL and bound values" }, + }); + expect(sent?.tags).toEqual({ operation: "connection.create", code: "22021" }); + expect(sent?.exception?.values?.[0]).toMatchObject({ + type: "StorageError", + value: "connection.create failed (22021)", + }); + expect(JSON.stringify(sent)).not.toContain("private SQL"); + }); + + it("strips secrets from auto-captured errors while retaining diagnostic locations", () => { + const secret = "SYNTHETIC_PRIVATE_MARKER"; + const sent = cloudSentryOptions({ + SENTRY_DSN: "https://public@example.invalid/1", + } as Env).beforeSend({ + type: undefined, + event_id: "safe-event-id", + message: secret, + user: { email: secret }, + request: { + url: `https://example.test/?token=${secret}`, + headers: { authorization: secret }, + data: secret, + }, + extra: { cause: secret }, + breadcrumbs: [{ message: secret }], + tags: { token: secret, otel_trace_id: traceId }, + exception: { + values: [ + { + type: "TypeError", + value: secret, + stacktrace: { + frames: [ + { + filename: `/assets/example.js?token=${secret}`, + function: "handleRequest", + lineno: 42, + vars: { secret }, + }, + ], + }, + }, + ], + }, + }); + expect(JSON.stringify(sent)).not.toContain(secret); + expect(sent?.event_id).toBe("safe-event-id"); + expect(sent?.tags?.otel_trace_id).toBe(traceId); + expect(sent?.exception?.values?.[0]?.stacktrace?.frames?.[0]).toMatchObject({ + filename: "/assets/example.js", + function: "handleRequest", + lineno: 42, + }); + }); +}); diff --git a/apps/cloud/src/observability/redact-span-urls.test.ts b/apps/cloud/src/observability/redact-span-urls.test.ts index 288c0f65ce..942c1b2a41 100644 --- a/apps/cloud/src/observability/redact-span-urls.test.ts +++ b/apps/cloud/src/observability/redact-span-urls.test.ts @@ -47,6 +47,17 @@ const exportSpanWith = ( }; describe("UrlRedactingSpanProcessor", () => { + it("omits non-URL exception payloads recorded as span attributes", () => { + const secret = "SYNTHETIC_PRIVATE_MARKER"; + const exported = exportSpanWith({ + "exception.message": secret, + "exception.stacktrace": secret, + "http.request.method": "POST", + }); + expect(exported?.attributes["http.request.method"]).toBe("POST"); + expect(JSON.stringify(exported?.attributes)).not.toContain(secret); + }); + it("scrubs the span before the exporter sees it", () => { const exported = exportSpanWith({ "url.full": callbackUrl, @@ -216,13 +227,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]"); }); }); @@ -284,3 +295,22 @@ describe("credential canary — no export channel carries the secret", () => { }, ); }); + +describe("non-URL secrets in exceptions", () => { + it("does not export a provider response or SQL values as error text", () => { + const secret = "synthetic-plain-secret"; + const exported = exportSpanWith({ "http.response.status_code": "500" }, (span) => { + span.recordException({ + name: "ProviderError", + message: `Failed query values: ${secret}`, + stack: `at provider: ${secret}`, + }); + span.setStatus({ code: SpanStatusCode.ERROR, message: secret }); + }); + expect(JSON.stringify({ events: exported?.events, status: exported?.status })).not.toContain( + secret, + ); + expect(exported?.status.code).toBe(SpanStatusCode.ERROR); + expect(exported?.events[0]?.attributes?.["exception.type"]).toBe("ProviderError"); + }); +}); 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/auth-evidence.test.ts b/e2e/cloud/auth-evidence.test.ts new file mode 100644 index 0000000000..58d8876afd --- /dev/null +++ b/e2e/cloud/auth-evidence.test.ts @@ -0,0 +1,185 @@ +import { randomBytes } from "node:crypto"; +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect } from "@effect/vitest"; +import { Effect, Schema } from "effect"; + +import { RUNS_DIR, scenario } from "../src/scenario"; +import { RunDir, Target, Telemetry } from "../src/services"; + +const decodeString = Schema.decodeUnknownSync(Schema.String); + +const decodeKey = Schema.decodeUnknownSync( + Schema.Struct({ id: Schema.String, value: Schema.String }), +); + +scenario( + "Authentication · valid credentials work in headers but not query parameters", + {}, + Effect.gen(function* () { + const target = yield* Target; + const runDir = yield* RunDir; + const identity = yield* target.newIdentity(); + const keyResponse = yield* Effect.promise(() => + fetch(new URL("/api/account/api-keys", target.baseUrl), { + method: "POST", + headers: { + ...identity.headers, + origin: target.baseUrl, + "content-type": "application/json", + }, + body: JSON.stringify({ name: "authentication-evidence" }), + }), + ); + expect(keyResponse.status).toBe(200); + const key = decodeKey(yield* Effect.promise(() => keyResponse.json())); + yield* Effect.promise(async () => { + const cases: Array<{ surface: string; carrier: string; status: number }> = []; + for (const surface of ["api", "mcp"]) { + const send = async (query: string | null, header: boolean) => { + const url = new URL(surface === "api" ? "/api/policies" : "/mcp", target.baseUrl); + if (query) url.searchParams.set(query, key.value); + const response = await fetch(url, { + method: surface === "api" ? "GET" : "POST", + headers: { + accept: "application/json, text/event-stream", + "content-type": "application/json", + ...(header ? { authorization: `Bearer ${key.value}` } : {}), + }, + ...(surface === "mcp" + ? { + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-03-26", + capabilities: {}, + clientInfo: { name: "authentication-evidence", version: "1" }, + }, + }), + } + : {}), + }); + await response.text(); + cases.push({ + surface, + carrier: query ?? "Authorization header", + status: response.status, + }); + return response.status; + }; + expect(await send(null, true), `${surface} accepts the valid header credential`).toBe(200); + for (const query of [ + "api_key", + "apikey", + "key", + "token", + "access_token", + "authorization", + ]) { + expect(await send(query, false), `${surface} rejects query-only ${query}`).toBe( + surface === "api" ? 403 : 401, + ); + } + } + await writeFile( + join(runDir, "authentication-carriers.json"), + JSON.stringify({ cases }, null, 2), + ); + }).pipe( + Effect.ensuring( + Effect.promise(async () => { + const response = await fetch(new URL(`/api/account/api-keys/${key.id}`, target.baseUrl), { + method: "DELETE", + headers: { ...identity.headers, origin: target.baseUrl }, + }); + expect(response.status, "the disposable key is revoked").toBe(200); + }), + ), + ); + }), +); + +scenario( + "Authentication · successful login exports diagnostics without its credentials", + {}, + Effect.gen(function* () { + const target = yield* Target; + const telemetry = yield* Telemetry; + const runDir = yield* RunDir; + const identity = yield* target.newIdentity(); + const bootLog = join(RUNS_DIR, "cloud", "server-logs", "boot.log"); + const initialLogLength = (yield* Effect.promise(() => readFile(bootLog, "utf8"))).length; + const traceId = randomBytes(16).toString("hex"); + const headers = { traceparent: `00-${traceId}-${randomBytes(8).toString("hex")}-01` }; + const credentials = yield* Effect.promise(async () => { + const login = await fetch(new URL("/api/auth/login", target.baseUrl), { redirect: "manual" }); + expect(login.status).toBe(302); + expect(login.headers.get("referrer-policy")).toBe("no-referrer"); + const authorize = new URL(decodeString(login.headers.get("location"))); + const state = decodeString(authorize.searchParams.get("state")); + const stateCookie = login.headers + .getSetCookie() + .find((cookie) => cookie.startsWith("wos-login-state=")); + expect(stateCookie !== undefined).toBe(true); + authorize.searchParams.set("login_hint", identity.label); + const consent = await fetch(authorize, { redirect: "manual" }); + expect(consent.status).toBe(302); + const callback = new URL(decodeString(consent.headers.get("location"))); + const code = decodeString(callback.searchParams.get("code")); + const signedIn = await fetch(callback, { + redirect: "manual", + headers: { + ...headers, + cookie: decodeString(stateCookie).split(";")[0] ?? "", + }, + }); + expect(signedIn.status).toBe(302); + expect(signedIn.headers.get("referrer-policy")).toBe("no-referrer"); + const session = signedIn.headers + .getSetCookie() + .find((cookie) => cookie.startsWith("wos-session=")); + const sessionPair = decodeString(session).split(";")[0] ?? ""; + const verified = await fetch(new URL("/api/auth/me", target.baseUrl), { + headers: { cookie: sessionPair }, + }); + expect(verified.status).toBe(200); + return [state, code, sessionPair.slice("wos-session=".length)]; + }); + yield* telemetry.expectSpan({ traceId }); + const spans = yield* telemetry.searchSpans({ traceId }); + const exported = JSON.stringify(spans); + const logs = (yield* Effect.promise(() => readFile(bootLog, "utf8"))).slice(initialLogLength); + const matches = credentials.map((credential) => ({ + trace: exported.includes(credential), + serverLog: logs.includes(credential), + })); + // Assert booleans so even a failure cannot print a credential. + expect(matches.every((match) => !match.trace && !match.serverLog)).toBe(true); + yield* Effect.promise(() => + writeFile( + join(runDir, "login-diagnostics.json"), + JSON.stringify( + { + environment: "isolated cloud Worker with WorkOS emulator", + loginStatus: 302, + authenticatedSessionStatus: 200, + traceId, + exportedSpanCount: spans.length, + checkedCredentials: ["OAuth state", "authorization code", "sealed session cookie"], + credentialMatches: matches, + sample: spans.map(({ span }) => ({ + operation: span.operationName, + status: span.status, + attributeNames: Object.keys(span.tags), + })), + }, + null, + 2, + ), + ), + ); + }), +); 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/e2e/scenarios/artifact-preview-xss.test.ts b/e2e/scenarios/artifact-preview-xss.test.ts new file mode 100644 index 0000000000..66eb1fa5f5 --- /dev/null +++ b/e2e/scenarios/artifact-preview-xss.test.ts @@ -0,0 +1,76 @@ +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { AccountHttpApi } from "@executor-js/api"; +import { composePluginApi } from "@executor-js/api/server"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; +import { visit } from "../src/surfaces/browser"; + +const coreApi = composePluginApi([] as const); + +scenario( + "Artifacts · uploaded previews stay inert after storage and reload", + { timeout: 120_000 }, + Effect.gen(function* () { + const target = yield* Target; + const api = yield* Api; + const browser = yield* Browser; + const identity = yield* target.newIdentity(); + const client = yield* api.client(coreApi, identity); + const account = yield* api.client(AccountHttpApi, identity); + const me = yield* account.account.me(); + const title = "Preview security check"; + const marker = "Safe preview content"; + const artifact = yield* client.artifacts.save({ + payload: { + title, + code: "function App() { return
Preview security check
; }", + }, + }); + + yield* Effect.gen(function* () { + const uploaded = yield* client.artifacts.setPreview({ + params: { artifactId: artifact.id }, + payload: { + preview: + `
${marker}` + + "" + + '' + + "" + + "Unsafe link
", + }, + }); + expect(uploaded.stored).toBe(true); + const saved = yield* client.artifacts.get({ params: { artifactId: artifact.id } }); + expect(saved.preview?.markup).toBe(`
${marker}Unsafe link
`); + + yield* browser.session(identity, async ({ page, step }) => { + const galleryPath = me.organization?.slug + ? `/${me.organization.slug}/artifacts` + : "/artifacts"; + const card = page.locator('[data-slot="artifact-card"]').filter({ hasText: title }); + const preview = card.locator('[data-slot="artifact-preview"]'); + const checkPreview = async () => { + await preview.getByText(`${marker}Unsafe link`, { exact: true }).waitFor(); + expect( + await preview.locator("script, img, iframe, a, [onclick], [onerror]").count(), + ).toBe(0); + expect(await page.evaluate(() => document.body.dataset.previewXss)).toBeUndefined(); + }; + await step("Open the saved preview with injected markup removed", async () => { + await visit(page, `${target.baseUrl}${galleryPath}`); + await checkPreview(); + }); + await step("Reload and verify the stored preview remains inert", async () => { + await page.reload(); + await checkPreview(); + }); + }); + }).pipe( + Effect.ensuring( + client.artifacts.remove({ params: { artifactId: artifact.id } }).pipe(Effect.ignore), + ), + ); + }), +); 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-large-response.test.ts b/packages/plugins/graphql/src/sdk/introspect-large-response.test.ts new file mode 100644 index 0000000000..e42610d514 --- /dev/null +++ b/packages/plugins/graphql/src/sdk/introspect-large-response.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Layer } from "effect"; +import { HttpClient, HttpClientResponse } from "effect/unstable/http"; + +import { introspect } from "./introspect"; + +describe("GraphQL large introspection compatibility", () => { + it.effect("accepts a valid response larger than 32 MiB", () => + Effect.gen(function* () { + const description = "x".repeat(33 * 1024 * 1024); + const schema = { + queryType: { name: "Query" }, + mutationType: null, + types: [ + { + kind: "OBJECT", + name: "Query", + description, + fields: [], + inputFields: null, + enumValues: null, + }, + ], + }; + const client = HttpClient.make((request) => + Effect.succeed( + HttpClientResponse.fromWeb(request, Response.json({ data: { __schema: schema } })), + ), + ); + const result = yield* introspect("https://example.test/graphql").pipe( + Effect.provide(Layer.succeed(HttpClient.HttpClient)(client)), + ); + expect(result.__schema.queryType).toEqual({ name: "Query" }); + expect(result.__schema.types[0]?.description?.length).toBe(description.length); + }), + ); +}); 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({