Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
31 changes: 31 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
8 changes: 6 additions & 2 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 } from "./redact.ts";

export enum LogLevel {
DEBUG = 0,
Expand Down Expand Up @@ -344,7 +345,10 @@ class ConsoleLogger implements Logger {
entry.conversation_id = entry.conversationId;
}

if (Object.keys(mergedContext).length > 0) entry.context = mergedContext;
// 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);
if (error) entry.error = error;

return JSON.stringify(entry);
Expand All @@ -361,7 +365,7 @@ 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), error, enableColor);

return `${timestamp} ${tag} ${glyph}${componentTag} ${message}${contextText}`;
}
Expand Down
155 changes: 155 additions & 0 deletions src/utils/logger/redact.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import "#veryfront/schemas/_test-setup.ts";
import { assertEquals } from "#veryfront/testing/assert.ts";
import { describe, it } from "#veryfront/testing/bdd.ts";
import { isSensitiveKey, REDACTED, redactSensitive } from "./redact.ts";

describe("logger/redact", () => {
describe("isSensitiveKey", () => {
it("matches credential-like keys across naming conventions", () => {
for (
const key of [
"password",
"passwd",
"pwd",
"passphrase",
"secret",
"clientSecret",
"token",
"access_token",
"refreshToken",
"jwt",
"jwtToken",
"apiKey",
"API-Key",
"x-api-key",
"accessKey",
"privateKey",
"credential",
"authorization",
"Authorization",
"Cookie",
"bearer",
"connectionString",
]
) {
assertEquals(isSensitiveKey(key), true, `expected ${key} to be sensitive`);
}
});

it("does not flag benign keys that merely look similar", () => {
// `author` must NOT match (the deny-list deliberately omits bare `auth`),
// and short tokens like `dsn`/`sas` are omitted to avoid masking e.g.
// `feedsNamespace`.
for (
const key of ["author", "count", "userId", "requestId", "url", "domain", "feedsNamespace"]
) {
assertEquals(isSensitiveKey(key), false, `expected ${key} to be non-sensitive`);
}
});

it("documents the accepted over-redaction of keys containing a pattern", () => {
// Over-redaction is the safe failure mode: a key like "tokenCount" is
// masked even though it is not itself a secret.
assertEquals(isSensitiveKey("tokenCount"), true);
});
});

describe("redactSensitive", () => {
it("masks top-level sensitive values and preserves the rest", () => {
const result = redactSensitive({
requestId: "req-1",
password: "hunter2",
authorization: "Bearer abc",
message: "ok",
});
assertEquals(result, {
requestId: "req-1",
password: REDACTED,
authorization: REDACTED,
message: "ok",
});
});

it("redacts nested objects and arrays of objects", () => {
const result = redactSensitive({
outer: {
apiKey: "k",
nested: { token: "t", keep: 1 },
},
list: [{ secret: "s", id: 2 }],
});
assertEquals(result, {
outer: {
apiKey: REDACTED,
nested: { token: REDACTED, keep: 1 },
},
list: [{ secret: REDACTED, id: 2 }],
});
});

it("traverses class instances so their secret fields cannot leak", () => {
class ApiConfig {
apiKey = "sk-secret";
name = "app";
}
const result = redactSensitive({ config: new ApiConfig() }) as Record<
string,
Record<string, unknown>
>;
assertEquals(result.config.apiKey, REDACTED);
assertEquals(result.config.name, "app");
});

it("does not mutate the input object", () => {
const input = { password: "hunter2", keep: "v" };
const result = redactSensitive(input);
assertEquals(input.password, "hunter2");
assertEquals((result as Record<string, unknown>).password, REDACTED);
});

it("leaves primitives and scalar-serializing objects untouched", () => {
const date = new Date(0);
const result = redactSensitive({ when: date, n: 5, flag: true, nil: null }) as Record<
string,
unknown
>;
// Date defines toJSON → serializes to a scalar → returned as-is.
assertEquals(result.when, date);
assertEquals(result.n, 5);
assertEquals(result.flag, true);
assertEquals(result.nil, null);
});

it("fails closed on cyclic references (no unredacted back-reference)", () => {
const cyclic: Record<string, unknown> = { token: "t", keep: 1 };
cyclic.self = cyclic;
const result = redactSensitive(cyclic) as Record<string, unknown>;
assertEquals(result.token, REDACTED);
assertEquals(result.keep, 1);
// The back-reference is masked rather than re-emitting the raw object.
assertEquals(result.self, REDACTED);
});

it("fails closed on a throwing getter", () => {
const obj: Record<string, unknown> = { password: "x" };
Object.defineProperty(obj, "boom", {
enumerable: true,
get() {
throw new Error("nope");
},
});
// The whole object is masked rather than crashing the log call.
assertEquals(redactSensitive({ wrap: obj }) as Record<string, unknown>, {
wrap: REDACTED,
});
});

it("fails closed past the max traversal depth", () => {
// Build a structure deeper than MAX_DEPTH (16) with a secret at the bottom.
let node: Record<string, unknown> = { token: "deep-secret" };
for (let i = 0; i < 20; i++) node = { child: node };
const serialized = JSON.stringify(redactSensitive(node));
assertEquals(serialized.includes("deep-secret"), false);
});
});
});
Loading