Skip to content
Merged
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
22 changes: 22 additions & 0 deletions src/errors/logging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,17 @@ describe("logging", () => {
assertStringIncludes(output, "Component render failed");
});

it("redacts credential-like context keys in the dev dump (#1989)", () => {
const error = RENDER_ERROR.create();

logError(error, { userId: "u-1", apiKey: "sk-secret" });

const output = consoleErrorOutput.join("\n");
assertStringIncludes(output, "[REDACTED]");
assertStringIncludes(output, "u-1");
assertEquals(output.includes("sk-secret"), false);
});

it("should use error.context when no context provided", () => {
const error = CONFIG_NOT_FOUND.create({
context: { originalContext: true },
Expand Down Expand Up @@ -122,6 +133,17 @@ describe("logging", () => {
assertEquals(parsed.context.componentPath, "/app/page.tsx");
});

it("redacts credential-like context keys in JSON output (#1989)", () => {
const error = RENDER_ERROR.create();

logError(error, { userId: "u-1", token: "sk-secret" });

const parsed = JSON.parse(consoleErrorOutput[0]);
assertEquals(parsed.context.token, "[REDACTED]");
assertEquals(parsed.context.userId, "u-1");
assertEquals(consoleErrorOutput[0].includes("sk-secret"), false);
});

it("should merge error context with extra context and prefer extra values", () => {
const error = CONFIG_NOT_FOUND.create({
context: {
Expand Down
10 changes: 7 additions & 3 deletions src/errors/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import { isProduction } from "#veryfront/platform/environment.ts";
import { serverLogger } from "#veryfront/utils/logger/logger.ts";
import { redactSensitive } from "#veryfront/utils/logger/redact.ts";
import { VeryfrontError } from "./types.ts";

export interface ErrorLogEntry {
Expand Down Expand Up @@ -54,6 +55,9 @@ export function logError(
context?: Record<string, unknown>,
): void {
const mergedContext = mergeContext(error.context, context);
// Redact once and reuse for both the production JSON entry and the dev-mode
// human-readable dump, so neither path can emit unredacted credentials.
const safeContext = redactSensitive(mergedContext);
const entry: ErrorLogEntry = {
level: "error",
slug: error.slug,
Expand All @@ -64,7 +68,7 @@ export function logError(
status: error.status,
docs: error.getDocsUrl(),
timestamp: new Date().toISOString(),
context: mergedContext,
context: safeContext,
};

if (isProduction()) {
Expand All @@ -81,8 +85,8 @@ export function logError(
serverLogger.error(` 💡 Suggestion: ${error.suggestion}`);
}
serverLogger.error(` 📚 Docs: ${entry.docs}`);
if (mergedContext) {
serverLogger.error(` Context: ${JSON.stringify(mergedContext, null, 2)}`);
if (safeContext) {
serverLogger.error(` Context: ${JSON.stringify(safeContext, null, 2)}`);
}
}
}
Expand Down
33 changes: 32 additions & 1 deletion src/observability/log-buffer.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import "#veryfront/schemas/_test-setup.ts";
import { assertEquals, assertExists } from "#veryfront/testing/assert.ts";
import { describe, it } from "#veryfront/testing/bdd.ts";
import { LogBuffer } from "./log-buffer.ts";
import { interceptConsole, LogBuffer } from "./log-buffer.ts";

describe("observability/log-buffer", () => {
describe("LogBuffer", () => {
Expand All @@ -18,6 +18,37 @@ describe("observability/log-buffer", () => {
assertEquals(a.id !== b.id, true);
});

it("redacts credential-like keys from entry data (#1989)", () => {
const buf = new LogBuffer();
const seen: Record<string, unknown>[] = [];
buf.subscribe((entry) => {
if (entry.data) seen.push(entry.data);
});

const entry = buf.info("request", "server", { userId: "u-1", apiKey: "sk-secret" });

assertEquals(entry.data?.apiKey, "[REDACTED]");
assertEquals(entry.data?.userId, "u-1");
// Subscribers (incl. the file writer) only ever see the redacted copy.
assertEquals(seen[0].apiKey, "[REDACTED]");
assertEquals(JSON.stringify(entry).includes("sk-secret"), false);
});

it("redacts object args captured via interceptConsole (#1989)", () => {
const buf = new LogBuffer();
const restore = interceptConsole(buf);
try {
console.error("auth attempt", { apiKey: "sk-secret", userId: "u-1" });
} finally {
restore();
}

const message = buf.tail(1)[0].message;
assertEquals(message.includes("sk-secret"), false);
assertEquals(message.includes("[REDACTED]"), true);
assertEquals(message.includes("u-1"), true);
});

it("should support all log levels", () => {
const buf = new LogBuffer();
buf.debug("d");
Expand Down
9 changes: 8 additions & 1 deletion src/observability/log-buffer.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { redactSensitive } from "#veryfront/utils/logger/redact.ts";

/** Public API contract for log level. */
export type LogLevel = "debug" | "info" | "warn" | "error";

Expand Down Expand Up @@ -40,6 +42,9 @@ export class LogBuffer {
append(entry: Omit<LogEntry, "id" | "timestamp">): LogEntry {
const fullEntry: LogEntry = {
...entry,
// Redact credential-like keys before the entry is buffered, surfaced to
// subscribers, or written to disk by the file subscriber (#1989).
data: entry.data ? redactSensitive(entry.data) : entry.data,
id: this.generateId(),
timestamp: Date.now(),
};
Expand Down Expand Up @@ -193,7 +198,9 @@ export function interceptConsole(buffer: LogBuffer, source = "console"): () => v
if (typeof a === "string") return a;

try {
return JSON.stringify(a);
// Redact object args before they are folded into the message string,
// where the per-entry data redaction can no longer reach them (#1989).
return JSON.stringify(redactSensitive(a));
} catch (_) {
/* expected: circular references or non-serializable values */
return String(a);
Expand Down
93 changes: 93 additions & 0 deletions src/utils/logger/logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,37 @@ describe("logger", () => {
}
});

it("redacts credential-like context keys before serialization (#1989)", () => {
const { getOutput, restore } = captureConsoleLog();

try {
withJsonLogFormat(() => {
serverLogger.info("Authenticating", {
userId: "u-1",
password: "hunter2",
authorization: "Bearer abc",
headers: { cookie: "session=xyz", accept: "json" },
});

const line = getOutput();
const entry = JSON.parse(line) as LogEntry;
const context = entry.context as Record<string, unknown>;
assertEquals(context.password, "[REDACTED]");
assertEquals(context.authorization, "[REDACTED]");
assertEquals((context.headers as Record<string, unknown>).cookie, "[REDACTED]");
// Non-sensitive fields survive.
assertEquals((context.headers as Record<string, unknown>).accept, "json");
// userId is a deliberate extracted field, not a secret.
assertEquals(entry.userId, "u-1");
// The raw secret must not appear anywhere in the serialized line.
assertEquals(line.includes("hunter2"), false);
assertEquals(line.includes("session=xyz"), false);
});
} finally {
restore();
}
});

it("should surface run user log routing fields as top-level JSON fields", () => {
const { getOutput, restore } = captureConsoleLog();

Expand Down Expand Up @@ -315,6 +346,45 @@ describe("logger", () => {
restore();
}
});

it("scrubs credentials embedded in error message/stack (#1989)", () => {
const { getOutput, restore } = captureConsoleLog();

try {
withJsonLogFormat(() => {
const err = new Error("db connect failed: postgres://admin:s3cret@db.host/app");
serverLogger.info("DB error", err);

const line = getOutput();
const entry = JSON.parse(line) as LogEntry;
assertEquals(line.includes("s3cret"), false);
assertEquals(entry.error?.message?.includes("[REDACTED]"), true);
});
} finally {
restore();
}
});

it("scrubs credentials from lifted request_url (#1989)", () => {
const { getOutput, restore } = captureConsoleLog();

try {
withJsonLogFormat(() => {
serverLogger.info("Incoming request", {
request_url: "https://api.example.com/cb?code=abc123&access_token=xyz&page=2",
});

const line = getOutput();
const entry = JSON.parse(line) as LogEntry;
assertEquals(line.includes("abc123"), false);
assertEquals(line.includes("xyz"), false);
assertEquals(entry.request_url?.includes("page=2"), true);
assertEquals(entry.request_url?.includes("[REDACTED]"), true);
});
} finally {
restore();
}
});
});

describe("text output format", () => {
Expand Down Expand Up @@ -343,6 +413,29 @@ describe("logger", () => {
__resetLoggerConfigForTests();
}
});

it("scrubs credentials from rendered error message (#1989)", () => {
Deno.env.set("LOG_FORMAT", "text");
Deno.env.set("NO_COLOR", "1");
__resetLoggerConfigForTests();

const { getOutput, restore } = captureConsoleLog();

try {
serverLogger.info("DB error", {
error: new Error("connect failed: mongodb://root:p4ss@cluster/db"),
});

const output = getOutput();
assertEquals(output.includes("p4ss"), false);
assertEquals(output.includes("[REDACTED]"), true);
} finally {
restore();
Deno.env.delete("LOG_FORMAT");
Deno.env.delete("NO_COLOR");
__resetLoggerConfigForTests();
}
});
});

describe("component() logger", () => {
Expand Down
29 changes: 24 additions & 5 deletions src/utils/logger/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
type SerializedError,
serializeError,
} from "./core.ts";
import { redactSensitive, sanitizeSerializedError, sanitizeUrlCredentials } from "./redact.ts";

export enum LogLevel {
DEBUG = 0,
Expand Down Expand Up @@ -313,8 +314,16 @@ class ConsoleLogger implements Logger {
extractToEntryField(entry, mergedContext, "trace_id", (v) => String(v));
extractToEntryField(entry, mergedContext, "span_id", (v) => String(v));
extractToEntryField(entry, mergedContext, "project_slug", (v) => String(v));
extractToEntryField(entry, mergedContext, "request_url", (v) => String(v));
extractToEntryField(entry, mergedContext, "domain", (v) => String(v));
// request_url / domain are URL-shaped and lifted out of mergedContext
// *before* redactSensitive runs, so they bypass the key-based redactor.
// Scrub embedded credentials (userinfo, ?access_token=, …) here (#1989).
extractToEntryField(
entry,
mergedContext,
"request_url",
(v) => sanitizeUrlCredentials(String(v)),
);
extractToEntryField(entry, mergedContext, "domain", (v) => sanitizeUrlCredentials(String(v)));
extractToEntryField(entry, mergedContext, "project_id", (v) => String(v));
extractToEntryField(entry, mergedContext, "release_id", (v) => String(v));
extractToEntryField(entry, mergedContext, "branch_id", (v) => String(v));
Expand Down Expand Up @@ -344,8 +353,14 @@ class ConsoleLogger implements Logger {
entry.conversation_id = entry.conversationId;
}

if (Object.keys(mergedContext).length > 0) entry.context = mergedContext;
if (error) entry.error = error;
// Redact credential-like keys from the free-form context bag before
// serialization (the deliberate top-level fields above are already
// extracted out of mergedContext, so they are unaffected).
if (Object.keys(mergedContext).length > 0) entry.context = redactSensitive(mergedContext);
// The serialized error (name/message/stack) bypasses the key-based
// redactor; scrub credentials embedded in its message/stack (DSNs, Mongo
// URIs, ?access_token= URLs, userinfo) before emission (#1989).
if (error) entry.error = sanitizeSerializedError(error);

return JSON.stringify(entry);
}
Expand All @@ -361,7 +376,11 @@ class ConsoleLogger implements Logger {
const componentTag = this.componentName
? ` ${colorize(`[${this.componentName}]`, ANSI.dim, enableColor)}`
: "";
const contextText = formatContextText(mergedContext, error, enableColor);
const contextText = formatContextText(
redactSensitive(mergedContext),
sanitizeSerializedError(error),
enableColor,
);

return `${timestamp} ${tag} ${glyph}${componentTag} ${message}${contextText}`;
}
Expand Down
Loading