diff --git a/src/lib/logging/__tests__/redact.test.ts b/src/lib/logging/__tests__/redact.test.ts index 3c7e93829..e47b1a73a 100644 --- a/src/lib/logging/__tests__/redact.test.ts +++ b/src/lib/logging/__tests__/redact.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { redactSecrets, redactOptional } from "../redact"; +import { redactSecrets, redactOptional, PATH_SECRET_PATHS } from "../redact"; describe("redactSecrets", () => { it("redacts Bearer tokens", () => { @@ -118,6 +118,77 @@ describe("redactSecrets", () => { "Bearer [REDACTED] bot[REDACTED] ?code=[REDACTED]", ); }); + + // v1.4.25 W21 Fix-J: `WITHINGS_WEBHOOK_SECRET` travels as the trailing + // path segment on `/api/withings/webhook/`. The Wide Event + // builder feeds `http.path` and `http.route` through `redactSecrets`, + // so the path-segment rule must rewrite the secret to `[REDACTED]` + // before stdout / ring-buffer / Loki egress. + describe("path-segment secret redaction", () => { + it("registers the Withings webhook prefix in PATH_SECRET_PATHS", () => { + expect(PATH_SECRET_PATHS.map((entry) => entry.prefix)).toContain( + "/api/withings/webhook/", + ); + }); + + it("redacts the Withings webhook secret segment", () => { + expect(redactSecrets("/api/withings/webhook/test-secret")).toBe( + "/api/withings/webhook/[REDACTED]", + ); + expect( + redactSecrets( + "/api/withings/webhook/abc123_DEADBEEF-cafe.0123456789abcdef", + ), + ).toBe("/api/withings/webhook/[REDACTED]"); + }); + + it("preserves trailing path segments after the secret", () => { + expect(redactSecrets("/api/withings/webhook/test-secret/")).toBe( + "/api/withings/webhook/[REDACTED]/", + ); + expect(redactSecrets("/api/withings/webhook/test-secret/verify")).toBe( + "/api/withings/webhook/[REDACTED]/verify", + ); + }); + + it("preserves query strings after the secret segment", () => { + // Note: the existing query-string rule then redacts the + // `?secret=…` payload too; both layers compose. + expect( + redactSecrets("/api/withings/webhook/test-secret?retries=2"), + ).toBe("/api/withings/webhook/[REDACTED]?retries=2"); + }); + + it("redacts an absolute URL form (origin + path-segment secret)", () => { + expect( + redactSecrets( + "https://app.healthlog.dev/api/withings/webhook/super-secret", + ), + ).toBe("https://app.healthlog.dev/api/withings/webhook/[REDACTED]"); + }); + + it("leaves the legacy query-string webhook form alone (no path-segment secret)", () => { + // `/api/withings/webhook` (no trailing segment) keeps shape; the + // `?secret=` rule scrubs the query payload via the existing rule. + expect(redactSecrets("/api/withings/webhook?secret=abc123")).toBe( + "/api/withings/webhook?secret=[REDACTED]", + ); + expect(redactSecrets("/api/withings/webhook")).toBe( + "/api/withings/webhook", + ); + }); + + it("does not over-redact sibling routes that share the prefix root", () => { + // Sibling paths (`/api/withings/measure`, `/api/withings/auth`) + // must not match the webhook-specific prefix. + expect(redactSecrets("/api/withings/measure/123")).toBe( + "/api/withings/measure/123", + ); + expect(redactSecrets("/api/withings/auth/callback")).toBe( + "/api/withings/auth/callback", + ); + }); + }); }); describe("redactOptional", () => { diff --git a/src/lib/logging/redact.ts b/src/lib/logging/redact.ts index 2fe981be2..40c0ad54a 100644 --- a/src/lib/logging/redact.ts +++ b/src/lib/logging/redact.ts @@ -1,8 +1,8 @@ /** * Redact secrets from strings before they leave the process boundary - * (Loki, Glitchtip, console). Applied in `WideEventBuilder.setError()` - * and `reportToGlitchtip()` so every error reaching observability is - * scrubbed once, centrally. + * (Loki, Glitchtip, console). Applied in `WideEventBuilder.setError()`, + * `WideEventBuilder.setHttp()`, and `reportToGlitchtip()` so every + * value reaching observability is scrubbed once, centrally. * * Patterns: * - `Bearer ` — generic API tokens incl. our `hlk_` / @@ -18,13 +18,51 @@ * - `sk-…` and `sk-ant-…` — OpenAI / Anthropic API keys. We never * log them on purpose, but a misconfigured client error or a * dump of the request body could carry one. Scrub before egress. + * - path-segment secrets — routes like `/api/withings/webhook/` + * carry the shared secret as a positional path segment. The Wide + * Event `http.path` and `http.route` fields would otherwise leak + * the secret into stdout, the in-memory ring buffer, and Loki. + * See `PATH_SECRET_PATHS`. * * The substitution is intentionally generic ([REDACTED]) — we don't * want partial revelation of token entropy. */ +/** + * Registry of path prefixes where the trailing segment is a shared + * secret. Each entry's `prefix` must end with `/`; any non-empty + * segment after it is rewritten to `[REDACTED]`. Subsequent path + * segments are preserved (so `/foo//bar` becomes + * `/foo/[REDACTED]/bar`). + * + * Add a new entry whenever a route exposes a secret in its URL path + * (rather than headers, body, or query). Tested in `redact.test.ts`. + */ +export const PATH_SECRET_PATHS: ReadonlyArray<{ prefix: string }> = [ + // Withings webhook entrypoint: `WITHINGS_WEBHOOK_SECRET` travels as + // the trailing path segment (v1.4.25 W17a). Without this rule the + // secret lands in every Wide Event's `http.path` / `http.route`. + { prefix: "/api/withings/webhook/" }, +]; + +function redactPathSegments(input: string): string { + let out = input; + for (const { prefix } of PATH_SECRET_PATHS) { + // Match `` where `` is one or more + // non-`/` non-`?` chars, then preserve anything after (next path + // segment, query string, hash). Capture group 1 is the optional + // trailing context starting with `/`, `?`, or `#`. + const escaped = prefix.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const re = new RegExp(`${escaped}[^/?#\\s]+([/?#][^\\s]*)?`, "g"); + out = out.replace(re, (_match, tail: string | undefined) => { + return `${prefix}[REDACTED]${tail ?? ""}`; + }); + } + return out; +} + export function redactSecrets(input: string): string { return ( - input + redactPathSegments(input) .replace(/Bearer\s+\S+/gi, "Bearer [REDACTED]") .replace(/bot\d+:[A-Za-z0-9_-]+/g, "bot[REDACTED]") // `sk-` is a common English-prefix substring (task-force, risk-