CL-6496: Fix three redaction leaks and cause-chain data loss in error-sink

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,54 @@ 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 <token>` fragment or an `Authorization: <value>` 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=`, `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
+  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.
+- `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.
+
 ## Retiring `@corbits/client-log`
 
 Out of scope for this unit, but the plan: migrate `apps/web`'s four

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<string, unknown>;
+    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", () => {

packages/error-sink/src/index.ts

@@ -29,16 +29,22 @@ 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"]);
 
 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;
 }
 

packages/error-sink/src/redact.test.ts

@@ -25,6 +25,62 @@ 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= style assignments in free text", () => {
+    expect(redactText("failed request token=abc123xyz retries=3")).toBe(
+      "failed request token=[redacted] retries=3",
+    );
+  });
+
+  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]",
+    );
+  });
+
+  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", () => {
@@ -64,4 +120,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"],
+    });
+  });
 });

packages/error-sink/src/redact.ts

@@ -10,14 +10,56 @@ 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,
 ];
 
+// 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;
   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]`,
+  );
+  redacted = redacted.replace(
+    QUERY_PARAM_SENSITIVE_ASSIGNMENT_PATTERN,
+    (_match, prefix: string, name: string, separator: string) =>
+      `${prefix}${name}${separator}[redacted]`,
+  );
   return redacted;
 }