From 98cc9e05decd7c99be9e7b763a31bf4a26a2bc7d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 08:06:16 -0700 Subject: [PATCH 1/6] Add failing tests for error-sink redaction leaks and cause-chain loss Covers three redaction gaps in packages/error-sink/src/redact.ts (OAuth callback URL query-param tokens, raw secret strings in arrays under a non-secret key, and token=/key= style assignments in free text) plus the loss of an Error's .cause chain when reportError copies it. --- packages/error-sink/src/index.test.ts | 21 +++++++++++++++++ packages/error-sink/src/redact.test.ts | 32 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/packages/error-sink/src/index.test.ts b/packages/error-sink/src/index.test.ts index 6f7e4826d..8027b9d16 100644 --- a/packages/error-sink/src/index.test.ts +++ b/packages/error-sink/src/index.test.ts @@ -85,6 +85,27 @@ describe("reportError context capture", () => { const loggedError = records[0]?.message; expect(String(loggedError)).not.toContain("abc.def.ghi"); }); + + test("preserves the error's cause chain, redacted", () => { + const inner = new Error("rejected Bearer abc.def.ghi"); + const outer = new Error("wrapped failure", { cause: inner }); + reportError(outer, { operation: "op" }); + + const properties = records[0]?.properties as Record; + const loggedError = properties.error as Error; + expect(loggedError.cause).toBeInstanceOf(Error); + const cause = loggedError.cause as Error; + expect(cause.message).not.toContain("abc.def.ghi"); + expect(cause.message).toContain("[redacted]"); + }); + + test("caps a cyclic cause chain instead of recursing forever", () => { + const a = new Error("a"); + const b = new Error("b", { cause: a }); + a.cause = b; + + expect(() => reportError(b, { operation: "op" })).not.toThrow(); + }); }); describe("reportError never throws", () => { diff --git a/packages/error-sink/src/redact.test.ts b/packages/error-sink/src/redact.test.ts index 813dd6454..562afc935 100644 --- a/packages/error-sink/src/redact.test.ts +++ b/packages/error-sink/src/redact.test.ts @@ -25,6 +25,30 @@ describe("redactText", () => { "could not reach the hub", ); }); + + test("redacts sensitive query-param values in a URL while keeping it readable", () => { + expect( + redactText( + "callback failed: https://api.example.com/cb?access_token=SECRETVALUE123&code=abc&state=xyz", + ), + ).toBe( + "callback failed: https://api.example.com/cb?access_token=[redacted]&code=[redacted]&state=xyz", + ); + }); + + test("redacts token= and key= style assignments in free text", () => { + expect(redactText("failed request token=abc123xyz key=def456")).toBe( + "failed request token=[redacted] key=[redacted]", + ); + }); + + test("redacts a raw JWT with no keyword prefix", () => { + const jwt = + "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dGhpc2lzbm90YXJlYWxzaWc"; + expect(redactText(`session restore failed for ${jwt}`)).toBe( + "session restore failed for [redacted]", + ); + }); }); describe("redactExtra", () => { @@ -64,4 +88,12 @@ describe("redactExtra", () => { test("passes undefined through unchanged", () => { expect(redactExtra(undefined)).toBeUndefined(); }); + + test("redacts a raw secret string inside an array under a non-secret key", () => { + const jwt = + "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dGhpc2lzbm90YXJlYWxzaWc"; + expect(redactExtra({ sessions: [jwt, "plain-session-id"] })).toEqual({ + sessions: ["[redacted]", "plain-session-id"], + }); + }); }); From cecc17d58cec8c7422bca2f93da6cf8035a9ad01 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 08:06:23 -0700 Subject: [PATCH 2/6] error-sink: close three redaction leaks and preserve cause chains redactText missed OAuth callback URL query-param values entirely, raw secret strings with no keyword prefix inside arrays under a non-secret key, and token=/key= style assignments outside a Bearer/Authorization header. Added a name-based assignment pattern that redacts only the value (keeping URLs and messages structurally readable) and a JWT-shape pattern for prefix-less tokens; fixed the provider-key pattern to also match GitHub's underscore-separated tokens. reportError also silently dropped an Error's .cause chain when building its redacted copy. redactedCopyOf now recurses into .cause, redacting each link, capped at a fixed depth so a cyclic or unbounded chain can't blow up the logger. --- packages/error-sink/src/index.ts | 8 +++++++- packages/error-sink/src/redact.ts | 21 ++++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/packages/error-sink/src/index.ts b/packages/error-sink/src/index.ts index 7d731b678..54f20b106 100644 --- a/packages/error-sink/src/index.ts +++ b/packages/error-sink/src/index.ts @@ -29,6 +29,9 @@ import { redactExtra, redactText } from "./redact"; import { generateRefId } from "./ref-id"; const UNKNOWN_OPERATION = "unknown"; +// A cyclic or unbounded `.cause` chain must not make the logger recurse +// forever; this caps how many links get carried over. +const MAX_CAUSE_DEPTH = 5; const log = getLogger(["errors"]); @@ -36,9 +39,12 @@ function asError(error: unknown): Error { return error instanceof Error ? error : new Error(String(error)); } -function redactedCopyOf(error: Error): Error { +function redactedCopyOf(error: Error, depth = 0): Error { const redacted = new Error(redactText(error.message)); if (error.stack !== undefined) redacted.stack = redactText(error.stack); + if (depth < MAX_CAUSE_DEPTH && error.cause !== undefined) { + redacted.cause = redactedCopyOf(asError(error.cause), depth + 1); + } return redacted; } diff --git a/packages/error-sink/src/redact.ts b/packages/error-sink/src/redact.ts index 0669476fa..c5b9d9bf7 100644 --- a/packages/error-sink/src/redact.ts +++ b/packages/error-sink/src/redact.ts @@ -10,14 +10,33 @@ const SECRET_KEY_PATTERN = const SECRET_VALUE_PATTERNS: readonly RegExp[] = [ /bearer\s+\S+/gi, /authorization\s*:\s*\S+/gi, - /\b(sk|pk|rk|ghp|gho|ghu|ghs)-[a-z0-9]{8,}\b/gi, + // Provider key prefixes use either a hyphen (OpenAI's `sk-...`) or an + // underscore (GitHub's `ghp_...`) -- both must match. + /\b(sk|pk|rk|ghp|gho|ghu|ghs)[-_][a-z0-9]{8,}\b/gi, + // A raw JWT (header.payload.signature) carries no keyword prefix at all, + // but its base64url header always starts with the literal `eyJ` (base64 + // of `{"`), which is distinctive enough to key off heuristically. + /\beyJ[\w-]{10,}\.[\w-]{10,}\.[\w-]{10,}\b/g, ]; +// `token=`, `key=`, `access_token=`, ... assignments: covers both a raw +// OAuth callback URL's query string (`?access_token=...&code=...`) and the +// same shape typed into a free-text error message. Only the value is +// replaced so the param name -- and the rest of the URL/message -- stays +// readable for debugging. +const SENSITIVE_ASSIGNMENT_PATTERN = + /\b(access_token|refresh_token|id_token|api[-_]?key|apikey|secret|password|passwd|token|code|credential|key)(\s*=\s*)([^\s&#]+)/gi; + export function redactText(text: string): string { let redacted = text; for (const pattern of SECRET_VALUE_PATTERNS) { redacted = redacted.replace(pattern, "[redacted]"); } + redacted = redacted.replace( + SENSITIVE_ASSIGNMENT_PATTERN, + (_match, name: string, separator: string) => + `${name}${separator}[redacted]`, + ); return redacted; } From 358c2b29f5134a8c34d46251fff539631512442a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 08:06:29 -0700 Subject: [PATCH 3/6] Update docs: error-sink redaction coverage and grandfathered log.error States plainly what redaction is and isn't heuristically able to catch, and that new code should call reportError rather than a raw log.error call (the existing raw call sites predate this package and aren't a pattern to copy). --- packages/error-sink/README.md | 46 ++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/packages/error-sink/README.md b/packages/error-sink/README.md index 03718b473..50a704555 100644 --- a/packages/error-sink/README.md +++ b/packages/error-sink/README.md @@ -3,7 +3,9 @@ The structured convention every catch block reports a failure to instead of swallowing it (CL-6496 — the owner ruling this package exists to enforce): one function, `reportError(error, context)`, that never throws and returns -a `refId` a person can quote to support. +a `refId` a person can quote to support. It redacts (see below) and +preserves the error's `.cause` chain, depth-capped so a cyclic or +unbounded chain can't blow up the logger. ## Why this isn't a second logger @@ -23,6 +25,48 @@ structured-error shape (`operation`, optional `tenantId`/`roomId`/ registered once via `@intx/log`'s own `configureSync`/`setup` — no call site of `reportError` changes when that happens. +New code should always go through `reportError` rather than calling +`log.error` directly. The repo's existing raw `log.error` call sites predate +this package and are grandfathered, not a pattern to copy — they don't get +the redaction pass below, the `refId`, or the structured context shape. + +## What redaction does and doesn't catch + +`redactText`/`redactExtra` (`src/redact.ts`) are a heuristic pass, not a +general-purpose secret scanner — the bar is "never ships an obvious secret +in a common shape," not "catches every possible one." + +Caught: + +- A `Bearer ` fragment or an `Authorization: ` header fragment + anywhere in a string. +- Known provider key prefixes: `sk-`/`pk-`/`rk-`/`ghp-`/`gho-`/`ghu-`/`ghs-` + and their underscore variants (`ghp_...`, as GitHub actually issues them). +- A raw JWT (`eyJ...eyJ...`. shape) even with no keyword nearby — the + base64url header is distinctive enough to key off directly. +- `token=`, `key=`, `code=`, `secret=`, `password=`, `api_key=`, and similar + assignments, whether in a URL's query string (an OAuth callback landing in + an error message keeps its host/path/param names, only the sensitive + values become `[redacted]`) or in free-text messages. +- Any object key that itself looks credential-shaped (`token`, `secret`, + `password`, `apiKey`, `cookie`, ...) is redacted wholesale, including when + its value is an array or a nested object — `redactExtra` recurses through + both. + +Not caught, by design: + +- A raw secret string with no recognizable prefix, keyword, or shape sitting + under an unrelated key (e.g. a bare AWS access key in a field named + `values`). There is no reliable heuristic for this that doesn't also flag + ordinary IDs; put such values behind a credential-shaped key instead. +- Because the `token=`/`key=`/`code=`/... assignment match is name-based, + it can occasionally over-redact a non-secret field sharing one of these + names in free text (e.g. an HTTP `code=404` written inline). Prefer + structured `extra` fields over interpolating such values into a message + string if this matters for a given call site. +- Secrets embedded in non-string values (numbers, binary blobs) or inside a + JSON-serialized string that isn't itself parsed back into an object. + ## Retiring `@corbits/client-log` Out of scope for this unit, but the plan: migrate `apps/web`'s four From 3f6616b54aa4496404e21f24bd24d3f3ba8cb8ca Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 08:17:58 -0700 Subject: [PATCH 4/6] Add tests pinning code=/key= to query-string position only Peer review on PR #231 found the free-text code=/key= assignment match over-redacted everyday non-secret shapes (an HTTP code=404, logfmt's code=DB_TIMEOUT, a cache key=...). These tests pin the intended boundary: code=/key= must survive untouched in free text but still get redacted when they appear as a URL query param, and a token= value must not swallow unrelated trailing text. --- packages/error-sink/src/redact.test.ts | 38 ++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/error-sink/src/redact.test.ts b/packages/error-sink/src/redact.test.ts index 562afc935..e2d9dbe50 100644 --- a/packages/error-sink/src/redact.test.ts +++ b/packages/error-sink/src/redact.test.ts @@ -36,9 +36,9 @@ describe("redactText", () => { ); }); - test("redacts token= and key= style assignments in free text", () => { - expect(redactText("failed request token=abc123xyz key=def456")).toBe( - "failed request token=[redacted] key=[redacted]", + test("redacts token= style assignments in free text", () => { + expect(redactText("failed request token=abc123xyz retries=3")).toBe( + "failed request token=[redacted] retries=3", ); }); @@ -49,6 +49,38 @@ describe("redactText", () => { "session restore failed for [redacted]", ); }); + + test("redacts code=/key= only in query-string position, not free text", () => { + expect(redactText('code=404 message="Not Found"')).toBe( + 'code=404 message="Not Found"', + ); + expect( + redactText('level=error msg="db timeout" code=DB_TIMEOUT retries=3'), + ).toBe('level=error msg="db timeout" code=DB_TIMEOUT retries=3'); + expect(redactText("cache miss for key=user:1234:profile")).toBe( + "cache miss for key=user:1234:profile", + ); + expect(redactText("at /routes/key=handler.ts:12:5)")).toBe( + "at /routes/key=handler.ts:12:5)", + ); + }); + + test("redacts code=/key= when they appear as a URL query param", () => { + expect( + redactText( + "https://api.example.com/authorize?client_id=abc&code=SECRETCODE", + ), + ).toBe("https://api.example.com/authorize?client_id=abc&code=[redacted]"); + expect( + redactText("https://api.example.com/data?key=APIKEYVALUE&format=json"), + ).toBe("https://api.example.com/data?key=[redacted]&format=json"); + }); + + test("does not let the value group run past a stack-frame's trailing text", () => { + expect( + redactText("auth failed token=abc123).authenticate() at line 4"), + ).toBe("auth failed token=[redacted]).authenticate() at line 4"); + }); }); describe("redactExtra", () => { From 4045a3f815cdc49422a9e24553b09f8edad8ef53 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 08:18:05 -0700 Subject: [PATCH 5/6] error-sink: scope code=/key= redaction to query-string position code and key are too ambiguous with everyday non-secret shapes to redact wherever they appear as a bare assignment -- code=404, logfmt's code=DB_TIMEOUT, and a cache key=user:1234:profile were all getting destroyed. Split the assignment pattern in two: unambiguous names (token, secret, password, api_key, ...) still match anywhere; code and key only match right after a literal ? or & (the URL query-string case that motivated this in the first place). Also bounds the value capture to a token-shaped character class so it can't run past the intended value into unrelated trailing text. --- packages/error-sink/src/redact.ts | 37 +++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/error-sink/src/redact.ts b/packages/error-sink/src/redact.ts index c5b9d9bf7..fd76667f5 100644 --- a/packages/error-sink/src/redact.ts +++ b/packages/error-sink/src/redact.ts @@ -19,13 +19,31 @@ const SECRET_VALUE_PATTERNS: readonly RegExp[] = [ /\beyJ[\w-]{10,}\.[\w-]{10,}\.[\w-]{10,}\b/g, ]; -// `token=`, `key=`, `access_token=`, ... assignments: covers both a raw -// OAuth callback URL's query string (`?access_token=...&code=...`) and the -// same shape typed into a free-text error message. Only the value is -// replaced so the param name -- and the rest of the URL/message -- stays -// readable for debugging. -const SENSITIVE_ASSIGNMENT_PATTERN = - /\b(access_token|refresh_token|id_token|api[-_]?key|apikey|secret|password|passwd|token|code|credential|key)(\s*=\s*)([^\s&#]+)/gi; +// A value's character set once past `name=`: covers hex/base64/JWT-shaped +// tokens without running past the value into unrelated trailing text (a +// closing paren, a stack-frame's `:12:5)`, ...). +const ASSIGNMENT_VALUE = "[\\w.+/=%-]+"; + +// `token=`, `access_token=`, ... assignments: covers both a raw OAuth +// callback URL's query string (`?access_token=...`) and the same shape +// typed into a free-text error message. Only the value is replaced so the +// param name -- and the rest of the URL/message -- stays readable for +// debugging. These names are unambiguous: outside of a credential they +// don't ordinarily show up as a bare `name=value` assignment at all. +const SENSITIVE_ASSIGNMENT_PATTERN = new RegExp( + `\\b(access_token|refresh_token|id_token|api[-_]?key|apikey|secret|password|passwd|token|credential)(\\s*=\\s*)(${ASSIGNMENT_VALUE})`, + "gi", +); + +// `code` and `key` are NOT unambiguous -- `code=404`, logfmt's +// `code=DB_TIMEOUT`, and `key=user:1234:profile` are everyday non-secret +// shapes. The only place they reliably mean "secret" is a URL query string +// (an OAuth `code`, a `key=` API credential passed as a param), so these +// two are scoped to right after a literal `?` or `&`. +const QUERY_PARAM_SENSITIVE_ASSIGNMENT_PATTERN = new RegExp( + `([?&])(code|key)(\\s*=\\s*)(${ASSIGNMENT_VALUE})`, + "gi", +); export function redactText(text: string): string { let redacted = text; @@ -37,6 +55,11 @@ export function redactText(text: string): string { (_match, name: string, separator: string) => `${name}${separator}[redacted]`, ); + redacted = redacted.replace( + QUERY_PARAM_SENSITIVE_ASSIGNMENT_PATTERN, + (_match, prefix: string, name: string, separator: string) => + `${prefix}${name}${separator}[redacted]`, + ); return redacted; } From fdcfb114036b0371c910b39fbaef7b99127c2be1 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 08:18:12 -0700 Subject: [PATCH 6/6] Update docs: redaction's code=/key= scope is query-string only Peer review flagged the prior wording as broader than the actual (now-fixed) behavior. States explicitly that code=/key= redaction is scoped to a URL query string and is deliberately left alone everywhere else, since those two names collide too often with ordinary status codes and cache keys. --- packages/error-sink/README.md | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/error-sink/README.md b/packages/error-sink/README.md index 50a704555..e1d898b5f 100644 --- a/packages/error-sink/README.md +++ b/packages/error-sink/README.md @@ -44,10 +44,14 @@ Caught: and their underscore variants (`ghp_...`, as GitHub actually issues them). - A raw JWT (`eyJ...eyJ...`. shape) even with no keyword nearby — the base64url header is distinctive enough to key off directly. -- `token=`, `key=`, `code=`, `secret=`, `password=`, `api_key=`, and similar - assignments, whether in a URL's query string (an OAuth callback landing in - an error message keeps its host/path/param names, only the sensitive - values become `[redacted]`) or in free-text messages. +- `token=`, `secret=`, `password=`, `api_key=`, and similar assignments, + whether in a URL's query string or in a free-text message — only the + value becomes `[redacted]`, the param/field name stays. +- `code=` and `key=` specifically **only** when they appear as a URL query + param (right after a literal `?` or `&`, e.g. an OAuth callback's + `?code=...`). Elsewhere these two names are common non-secret shapes + (`code=404`, logfmt's `code=DB_TIMEOUT`, a cache `key=user:1234:profile`) + and are deliberately left untouched — see below. - Any object key that itself looks credential-shaped (`token`, `secret`, `password`, `apiKey`, `cookie`, ...) is redacted wholesale, including when its value is an array or a nested object — `redactExtra` recurses through @@ -59,11 +63,13 @@ Not caught, by design: under an unrelated key (e.g. a bare AWS access key in a field named `values`). There is no reliable heuristic for this that doesn't also flag ordinary IDs; put such values behind a credential-shaped key instead. -- Because the `token=`/`key=`/`code=`/... assignment match is name-based, - it can occasionally over-redact a non-secret field sharing one of these - names in free text (e.g. an HTTP `code=404` written inline). Prefer - structured `extra` fields over interpolating such values into a message - string if this matters for a given call site. +- `code=`/`key=` outside a URL query string. These two names are too + ambiguous with everyday non-secret shapes (an HTTP status `code=404`, + logfmt's `code=DB_TIMEOUT`, a cache `key=...`) to redact in free text + without regularly destroying debugging context — a log nobody can read is + a log nobody uses. If a call site genuinely has a bare secret under one of + these names outside a URL, put it in `extra` under a credential-shaped + key instead of interpolating it into the message string. - Secrets embedded in non-string values (numbers, binary blobs) or inside a JSON-serialized string that isn't itself parsed back into an object.