From e9904f1394b2ed83d1e900212eb95b8b1fea5ece Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Thu, 14 May 2026 14:13:36 -0300 Subject: [PATCH] docs(security): note CodeQL custom-sanitizer limitation as dismissal precedent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL js/stack-trace-exposure does not recognize sanitizers reached through a custom helper indirection — flagging callsites like open-sse/utils/error.ts::errorResponse and open-sse/executors/cursor.ts::buildErrorResponse even though both route through sanitizeErrorMessage(). Record the dismissal precedent (alerts #224 and #231, May 2026): - Add a "Known CodeQL limitation" section in docs/security/ERROR_SANITIZATION.md - Extend CLAUDE.md Hard Rule #14 with the precedent Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 2 +- docs/security/ERROR_SANITIZATION.md | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index e05986c19c..46b087aa5d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -412,4 +412,4 @@ git push -u origin feat/your-feature 11. Never embed public upstream OAuth client_id/secret or Firebase Web keys as string literals — always go through `resolvePublicCred()` (`open-sse/utils/publicCreds.ts`). See `docs/security/PUBLIC_CREDS.md`. 12. Never return raw `err.stack` / `err.message` in HTTP / SSE / executor responses — always route through `buildErrorBody()` or `sanitizeErrorMessage()` (`open-sse/utils/error.ts`). See `docs/security/ERROR_SANITIZATION.md`. 13. Never string-interpolate external paths or runtime values into shell scripts passed to `exec()`/`spawn()` — pass via the `env` option instead. Reference: `src/mitm/cert/install.ts::updateNssDatabases`. -14. Never dismiss a CodeQL / Secret-Scanning alert without (a) first checking the pattern docs above to see if the helper applies, and (b) recording the technical justification in the dismissal comment. +14. Never dismiss a CodeQL / Secret-Scanning alert without (a) first checking the pattern docs above to see if the helper applies, and (b) recording the technical justification in the dismissal comment. Precedent: `js/stack-trace-exposure` raised on callsites that already route through `sanitizeErrorMessage()` is a known CodeQL limitation (custom sanitizers not recognized) — dismiss as `false positive` referencing `docs/security/ERROR_SANITIZATION.md`. diff --git a/docs/security/ERROR_SANITIZATION.md b/docs/security/ERROR_SANITIZATION.md index a356fb8d37..461db1a8d8 100644 --- a/docs/security/ERROR_SANITIZATION.md +++ b/docs/security/ERROR_SANITIZATION.md @@ -132,6 +132,21 @@ When adding a new route or executor, copy the assertion pattern from this file. - The `pino` redaction config (`src/lib/log/redaction.ts` — if present) handles structured log redaction separately. This doc covers only the response-message surface. - Upstream-header denylist (`src/shared/constants/upstreamHeaders.ts`) covers header leakage — keep both files aligned when adding a new exfiltration concern. +## Known CodeQL limitation: custom sanitizers not recognized + +The CodeQL query [`js/stack-trace-exposure`](https://codeql.github.com/codeql-query-help/javascript/js-stack-trace-exposure/) uses a fixed allowlist of sanitizer patterns (e.g. inline `.split("\n")[0]`, `String#replace` with specific regex shapes, access to `.message` on `Error`). It does **not** recognize indirection through a custom helper like our `sanitizeErrorMessage()`. + +This means callsites that demonstrably sanitize via this module — for example `open-sse/utils/error.ts::errorResponse` and `open-sse/executors/cursor.ts::buildErrorResponse` — may continue to raise the alert even though the code is functionally safe. Precedent dismissals: `#224`, `#231` (May 2026), both marked `false positive` with technical justification. + +**How to handle a new occurrence:** + +1. Confirm the callsite actually routes the message through `sanitizeErrorMessage` / `buildErrorBody` / one of the wrappers documented above (read the call chain end-to-end — don't trust a comment). +2. Confirm `tests/unit/error-message-sanitization.test.ts` exercises the path (or add coverage). +3. Dismiss the alert via `gh api ... -X PATCH state=dismissed -f 'dismissed_reason=false positive'` referencing this doc. +4. Do **not** "fix" by inlining `.split("\n")[0]` everywhere — the helper is the single source of truth; duplicating the pattern weakens the sanitizer (loses path scrubbing, length cap, type coercion) for the appearance of placating the scanner. + +Adopting opt-in features like CodeQL's [`@codeql/javascript-models` custom sanitizer config](https://codeql.github.com/docs/codeql-language-guides/customizing-library-models-for-javascript/) is the long-term fix; it lives outside this doc. + ## References - [CWE-209: Information Exposure Through an Error Message](https://cwe.mitre.org/data/definitions/209.html)