Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/redact-sensitive-logger-output.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@trigger.dev/core": patch
---

Redact common credential and sensitive-data fields from structured logger output by default, including nested values and error metadata. Long strings and arrays are now truncated to keep log entries manageable.
24 changes: 20 additions & 4 deletions apps/webapp/app/services/logger.server.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { LogLevel } from "@trigger.dev/core/logger";
import { Logger } from "@trigger.dev/core/logger";
import { Logger, redact } from "@trigger.dev/core/logger";
import { patchConsoleToTelnet, startTelnetLogServer } from "@trigger.dev/core/v3/telnetLogServer";
import { sensitiveDataReplacer } from "./sensitiveDataReplacer";
import { AsyncLocalStorage } from "async_hooks";
Expand All @@ -12,24 +12,40 @@ export function trace<T>(fields: Record<string, unknown>, fn: () => T): T {
return currentFieldsStore.run(fields, fn);
}

// The keys below aren't already in the Logger's default deny-list. Passing them here means the
// extra data sent to Sentry gets the same redaction as the stdout line, instead of bypassing it.
const SENTRY_EXTRA_FILTERED_KEYS = ["examples", "connectionString"];

Logger.onError = (message, ...args) => {
const error = extractErrorFromArgs(args);
const extra = redact(flattenArgs(args), SENTRY_EXTRA_FILTERED_KEYS) as Record<string, unknown>;

if (error) {
captureException(error, {
captureException(redactError(error), {
extra: {
message,
...flattenArgs(args),
...extra,
},
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} else {
Comment on lines +21 to 30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Captured Sentry extra still carries the raw Error object

On the captureException path in apps/webapp/app/services/logger.server.ts:23-30, extra is built from redact(flattenArgs(args), ...), which still contains the original error key (an Error instance). redact leaves the Error mostly untouched (Error message/stack are non-enumerable, so Object.entries yields nothing and the instance is returned unchanged). While the captured exception itself is redacted via redactError, the error entry inside extra is the un-redacted Error. In practice JSON.stringify(Error) serializes to {}, so the message/stack usually don't leak through the extra field, but Sentry's own serialization of Error objects could differ. Worth confirming Sentry doesn't surface the raw message/stack via the extra payload.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

captureMessage(message, {
level: "error",
extra: flattenArgs(args),
extra,
});
}
};

function redactError(error: Error): Error {
const redactedError = new Error(redact(error.message) as string);
redactedError.name = error.name;

if (error.stack) {
redactedError.stack = redact(error.stack) as string;
}

return redactedError;
}

function extractErrorFromArgs(args: Array<Record<string, unknown> | undefined>) {
for (const arg of args) {
if (arg && "error" in arg && arg.error instanceof Error) {
Expand Down
69 changes: 69 additions & 0 deletions apps/webapp/test/logger.server.onError.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { beforeEach, describe, expect, it, vi } from "vitest";

const captureExceptionMock = vi.fn();
const captureMessageMock = vi.fn();

vi.mock("@sentry/remix", () => ({
captureException: captureExceptionMock,
captureMessage: captureMessageMock,
}));

describe("logger.server Logger.onError", () => {
beforeEach(() => {
captureExceptionMock.mockClear();
captureMessageMock.mockClear();
vi.spyOn(console, "error").mockImplementation(() => {});
});

it("redacts the extra payload sent to Sentry on the captureMessage path", async () => {
const { logger } = await import("~/services/logger.server");

logger.error("something failed", {
payload: { secret: "do-not-leak" },
apiKey: "tr_prod_should_not_leak",
});

expect(captureMessageMock).toHaveBeenCalledTimes(1);
const [, options] = captureMessageMock.mock.calls[0] as [string, { extra: unknown }];

expect(JSON.stringify(options.extra)).not.toContain("do-not-leak");
expect(JSON.stringify(options.extra)).not.toContain("tr_prod_should_not_leak");
});

it("redacts the exception and extra payload sent to Sentry", async () => {
const { logger } = await import("~/services/logger.server");
const error = new Error("tr_prod_should_not_leak");
error.stack = "Bearer secret-token";

logger.error("boom", {
error,
payload: { secret: "do-not-leak" },
});

expect(captureExceptionMock).toHaveBeenCalledTimes(1);
const [capturedError, options] = captureExceptionMock.mock.calls[0] as [
Error,
{ extra: unknown },
];

expect(capturedError).not.toBe(error);
expect(capturedError.message).not.toContain("tr_prod_should_not_leak");
expect(capturedError.stack).not.toContain("secret-token");
expect(JSON.stringify(options.extra)).not.toContain("do-not-leak");
});

it("still forwards non-sensitive extra fields", async () => {
const { logger } = await import("~/services/logger.server");

logger.error("something failed", { runId: "run_123", keep: "this stays" });

expect(captureMessageMock).toHaveBeenCalledTimes(1);
const [, options] = captureMessageMock.mock.calls[0] as [
string,
{ extra: Record<string, unknown> },
];

expect(options.extra.runId).toBe("run_123");
expect(options.extra.keep).toBe("this stays");
});
});
Loading
Loading