docs(security): note CodeQL custom-sanitizer limitation as dismissal precedent

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 documenting how to handle future
  occurrences (verify callchain → verify test coverage → dismiss with
  reference, do NOT duplicate the pattern inline).
- Extend CLAUDE.md Hard Rule #14 with the precedent so the next
  engineer doesn't try to "fix" the false positive by weakening the
  shared sanitizer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
diegosouzapw
2026-05-14 14:10:29 -03:00
parent bbf3ba0138
commit eff7de2284
4 changed files with 45 additions and 1 deletions

View File

@@ -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)

View File

@@ -113,6 +113,34 @@ Same zero-width-joiner trick as Claude Code, but with an expanded word list that
Strips Stainless SDK markers (`x-stainless-lang`, `x-stainless-package-version`, `x-stainless-os`, `x-stainless-arch`, `x-stainless-runtime`, `x-stainless-runtime-version`, `x-stainless-timeout`, `x-stainless-retry-count`, `x-stainless-helper-method`) before forwarding.
### ⚠️ Risk: `ANTIGRAVITY_CREDITS=always` (account-ban hot spot)
`ANTIGRAVITY_CREDITS=always` (consumed by `open-sse/executors/antigravity.ts`) routes **every** request through Antigravity AI Credit Overages (paid Google credits) instead of letting Google's free-tier quota gate things. This is documented as a feature, but it is **the single most common ToS-violation report we see** — multiple Google Ultra accounts have been banned with `403 / "service disabled for ToS violation" / insufficient_quota` after running for a few hours with `=always`.
The upstream enforcement is on **Google's side**, not anything OmniRoute can prevent. The env var name and the existing docs make it sound like a safe knob to flip; it isn't.
**Why this draws abuse detection more aggressively than free-tier-only usage:**
- Sustained automated spend on a single Google account flags differently than free-tier hits-quota-and-stops.
- Credit overages have no rate ceiling, so a misconfigured client can burn through several hundred USD in minutes and look like API-key resale or bot traffic.
- Multiple OmniRoute users hitting overage credits in parallel from the same external IP compounds the signal.
**Recommended posture:**
1. **Default to `ANTIGRAVITY_CREDITS=retry`** — overages are used only when free-tier returns 429, not on every request. This is the safer of the two non-zero modes.
2. **Spread load across providers via Auto-Combo** (`model: "auto"` or `kr/glm/etc`-combo) instead of saturating a single Antigravity account.
3. **Set per-connection RPM limits** in the Antigravity provider's edit page (Dashboard → Providers → Antigravity → connection → rate limit). 3060 RPM is a defensible upper bound for sustained use.
4. **Use distinct upstream IPs** per Antigravity account when possible (residential proxies aimed at the same account from many users compounds the abuse signal).
5. **If banned**: appeal via `support.google.com` → "Restore Workspace/Account access" with the exact `quota_exceeded` / `service disabled` response body Google sent. Restoration is not guaranteed.
This warning is also surfaced inline in the dashboard near the Antigravity provider edit screen when `ANTIGRAVITY_CREDITS` is set to `always` (or will be in v3.8.0; tracked separately).
Touch points:
- `open-sse/executors/antigravity.ts` — reads `process.env.ANTIGRAVITY_CREDITS`
- `src/lib/oauth/providers/antigravity.ts` — credential plumbing
- Original incident report: Discussion [#1183](https://github.com/diegosouzapw/OmniRoute/discussions/1183)
---
## CLI Fingerprint Registry — `open-sse/config/cliFingerprints.ts`