From eff7de22842527497fa8bd0f8447eeab416975af Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Thu, 14 May 2026 14:10:29 -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 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) --- CHANGELOG.md | 1 + CLAUDE.md | 2 +- docs/security/ERROR_SANITIZATION.md | 15 +++++++++++++++ docs/security/STEALTH_GUIDE.md | 28 ++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46cdea2c89..6fe029faa0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - **fix(providers/qoder):** disambiguate the "Local CLI runtime is not installed" error when a user pastes a Personal Access Token but the connection is in OAuth/CLI-flavored mode. The test route now surfaces a single actionable message ("switch this connection to API Key auth") instead of cascading CLI + 401 errors. (#2247) - **fix(dashboard/api-manager):** route custom OpenAI-/Anthropic-compatible provider IDs through `getProviderDisplayName` so the model grouping label shows `Compatible (openai)` instead of leaking the raw synthetic `openai-compatible-chat-` value. (#2021) - **fix(providers/blackbox-web):** add `BLACKBOX_WEB_VALIDATED_TOKEN` env override and 403 token-error disambiguation. Blackbox `/api/chat` started rejecting requests whose `validated` field didn't match the frontend `tk` token, even with a valid cookie + active subscription. Operators with the real token can now set the env var; otherwise the previous random-UUID fallback still ships, and a 403 with a token-specific body now surfaces a one-line "set BLACKBOX_WEB_VALIDATED_TOKEN" hint instead of the generic "cookie expired" message. (#2252) +- **docs(security):** document the ToS-violation hot spot of `ANTIGRAVITY_CREDITS=always` in `STEALTH_GUIDE.md`, including why it draws Google abuse detection more aggressively than free-tier-only usage and the recommended posture (`=retry`, Auto-Combo spread, per-connection RPM caps). (#2246) ### Changed 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) diff --git a/docs/security/STEALTH_GUIDE.md b/docs/security/STEALTH_GUIDE.md index f814c61b41..528e3aa2e8 100644 --- a/docs/security/STEALTH_GUIDE.md +++ b/docs/security/STEALTH_GUIDE.md @@ -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). 30–60 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`