Repository navigation
feat(api-keys): configurable default rate limits via DEFAULT_RATE_LIMIT_PER_DAY - #2266
Merged
diegosouzapw merged 7 commits intoMay 14, 2026
Conversation
…tion Embed Gemini, Antigravity and Windsurf public OAuth/Firebase identifiers (extracted from upstream CLI binaries) through a XOR-masked byte sequence in open-sse/utils/publicCreds.ts instead of source literals, so pattern scanners (GitHub Secret Scanning, Semgrep) stop raising false positives on every release. decodePublicCred passes raw values through unchanged for users who already have plaintext in their .env (no migration needed). - New utils/publicCreds.ts with decode/encode + tests - Replace 6 hardcoded Google client_id/secret in oauth.ts + providerRegistry - Drop literals from .env.example (comment-only documentation) - Sanitize error messages inside buildErrorBody so every caller (incl. createErrorResult) is covered; cursor.ts now reuses the shared helper - Cover the new helpers with unit tests (publicCreds + error sanitization) Resolves the open code-scanning js/stack-trace-exposure findings and the secret-scanning Google API Key alert without breaking existing setups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…terns Fixes the 4 fixable alerts opened in the recent scan and adds enforceable guardrails so future development follows the same pattern. Code fixes: - src/mitm/cert/install.ts: pass certPath/certName/action via exec()'s env option instead of string-interpolating them into the bash script (CodeQL js/shell-command-injection-from-environment cheaper-inference#225) - scripts/docs/{gen-provider-reference,add-frontmatter,fix-internal-links}: escape backslash before other regex/markdown metacharacters (CodeQL js/incomplete-sanitization cheaper-inference#227, cheaper-inference#228, cheaper-inference#229) Documentation (mandatory patterns): - docs/security/PUBLIC_CREDS.md — embedding public upstream OAuth/Firebase identifiers via resolvePublicCred(); never as string literals - docs/security/ERROR_SANITIZATION.md — routing every error response through sanitizeErrorMessage()/buildErrorBody(); never raw err.stack/err.message - CLAUDE.md: 4 new Hard Rules (cheaper-inference#11-cheaper-inference#14) + Security section + scenario notes - AGENTS.md, CONTRIBUTING.md: cross-reference the two new docs - SECURITY.md: extended Hard Security Rules with the new mandatory patterns - docs/README.md: index entries pointing to the two new docs Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The PATH_REGEX introduced in 1a39c31 used a greedy character class ([\w\-./\\]+) that can backtrack quadratically on inputs like "///////..." — flagged by CodeQL js/polynomial-redos (cheaper-inference#232). Replace it with simple whitespace tokenization + a length-bounded prefix-check helper. Same observable behaviour (paths replaced with <path>), but linear time regardless of input shape. Adds a 4096-char input cap as defence in depth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…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 cheaper-inference#224 and cheaper-inference#231, May 2026): - Add a "Known CodeQL limitation" section in docs/security/ERROR_SANITIZATION.md - Extend CLAUDE.md Hard Rule cheaper-inference#14 with the precedent Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The fallback rate-limit table (1000/day, 5000/week, 20000/month) in `apiKeyPolicy.ts` was hard-coded and applied to any key whose `rate_limits` column was null. 1000/day is easily exhausted by multi-worker indexing pipelines and there was no per-deployment knob to relax it. Replace the constant array with a small builder reading `DEFAULT_RATE_LIMIT_PER_DAY`: - `0` (the default) returns an empty rule set, which `checkRateLimit()` already short-circuits as "allowed" (`rateLimiter.ts:93`). Keys without a configured limit are then effectively unlimited. - Any positive integer N enables N/day, 5N/week, 20N/month — same ratios as the original table, scaled by N. - Negative or non-numeric input is treated as unlimited rather than throwing at module load. `DEFAULT_RATE_LIMITS` is still evaluated once at import time so the runtime cost is unchanged; the builder is exported only so tests can exercise it deterministically without juggling `process.env` across test cases. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable fallback rate limit for API keys via the DEFAULT_RATE_LIMIT_PER_DAY environment variable, replacing the previous hardcoded defaults. The implementation includes documentation updates and new unit tests for the rate-limiting logic. Review feedback highlights a violation of the repository style guide regarding input validation, suggesting the use of Zod schemas instead of parseInt. Additionally, the reviewer recommends maintaining a 'secure by default' posture by keeping the fallback at 1000 requests per day when the environment variable is unset, rather than defaulting to unlimited access.
…ault
Addresses review feedback on the previous commit.
1) Switch the env parser from `parseInt` to a Zod schema
(`z.coerce.number().int().min(0)`). Repository Style Guide rule 8
requires env input through Zod, and `parseInt` silently accepts
garbage suffixes — `parseInt("1000 requests", 10)` returns `1000`,
turning a config typo into a partial value instead of flagging it.
2) Reverse the secure-by-default direction. The previous version
treated an unset env var as "unlimited fallback"; a deployment
upgrading from any earlier release would silently lose rate
limiting on all API keys with a null `rate_limits` row. The
builder now restores the legacy 1000/day, 5000/week, 20000/month
table whenever the env value is unset, empty, or malformed.
Going unlimited requires an explicit `DEFAULT_RATE_LIMIT_PER_DAY=0`.
3) Malformed input emits a `log.warn` once at module load and falls
back to the legacy default — never silently empty. Tests cover
four discrete malformed shapes (negative, non-numeric, suffix-
contaminated, decimal) plus the unset / empty / whitespace and the
explicit-0 cases.
`.env.example` and `docs/reference/ENVIRONMENT.md` are updated to
reflect the new default and the new opt-out semantics.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Collaborator
|
Thanks @gleber! 🎯 This solves a real pain point for multi-worker pipelines — making rate limits configurable while preserving the secure 1000/day default for existing deployments is exactly the right approach. The Zod validation (rejecting partial parses like '1000 requests') is a nice touch. Merging into release/v3.8.0. |
diegosouzapw
merged commit May 14, 2026
3ce114a
into
cheaper-inference:release/v3.8.0
1 of 2 checks passed
diegosouzapw
added a commit
that referenced
this pull request
May 14, 2026
gleber
added a commit
to gleber/OmniRoute
that referenced
this pull request
May 15, 2026
Self-contained Nix packaging for OmniRoute, sitting directly on top of `upstream/release/v3.8.0` — no source-level patches needed at this base because the four fixes that used to live in `nix/patches/` are now upstream: * PR cheaper-inference#2264 — Content-Length strip on non-stream forward * PR cheaper-inference#2265 — `managementPolicy` accepts manage- scope API keys * PR cheaper-inference#2266 — configurable default rate limits * the dynamic CLI-helper import fix (rewritten to static imports upstream in the v3.8.0 cycle) What the flake still does: * `packages.<system>.omniroute` — `buildNpmPackage` over Next.js 16 standalone output. Wires: - `libsecret` in `buildInputs` so keytar's native module finds `libsecret-1.pc` during node-gyp configure. - `npmInstallFlags = ["--include=dev"]` because `NODE_ENV=production` otherwise drops `@tailwindcss/postcss` and the rest of devDeps that the build actually needs. - `OMNIROUTE_USE_TURBOPACK=1` to match the devenv shell. - `postPatch` stubs three offline-build sticking points: * `next/font/google` (no network in sandbox) — replaced with a no-op `Inter()` factory; the `<link rel="preconnect">` in layout still loads webfonts at runtime. * `next-themes` import in `TierFlowDiagram.tsx` (upstream bug on release/v3.8.0: imported but not in `package.json`) — stubbed to always return dark mode. * `bin/cli/runtime/sqliteRuntime.mjs` (last two steps of its fallback chain reference `sql.js` and a dynamic `pkgRoot` path, breaking `outputFileTracing`) — replaced with a minimal stub that loads better-sqlite3 directly. - `autoPatchelfIgnoreMissingDeps` for the cross-platform binaries (musl, OpenBSD libc++) sharp, koffi, and ngrok ship alongside the glibc ones — platform-gated at load time, not loaded on glibc Linux. * `nixosModules.omniroute` (also `nixosModules.default`): - Options for ports/host/data dir, three secret-file options (jwt / api-key / optional initial password) plus an optional `storageEncryptionKeyFile` for AES-256-GCM key persistence — the failure mode that exists to prevent is losing the auto-generated key in `$DATA_DIR/server.env` and rendering every stored provider credential permanently unreadable. - Secrets are loaded via systemd `LoadCredential` (service-user readable only) and exported into the env by a small wrapper ExecStart, because the app reads `JWT_SECRET` / `API_KEY_SECRET` / `INITIAL_PASSWORD` / `STORAGE_ENCRYPTION_KEY` directly and does not honor the `_FILE` convention. - Standard hardening: NoNewPrivileges, ProtectSystem=strict, ProtectHome, RestrictAddressFamilies, etc. * `checks.<linux>.module` — `pkgs.testers.nixosTest` that boots the module, waits for the unit, asserts the server answers on the configured port, the data directory is owned by `omniroute:omniroute`, the configured secrets reached the app (so the "JWT_SECRET auto-generated" log line is absent), and `$DATA_DIR/server.env` is not written when a STORAGE_ENCRYPTION_KEY is supplied externally. Skipped on Darwin (`testers.nixosTest` needs a real QEMU VM). Verified locally on release/v3.8.0: `nix build .#default` succeeds and `nix build .#checks.x86_64-linux.module` passes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced May 16, 2026
Merged
HouMinXi
pushed a commit
to HouMinXi/OmniRoute
that referenced
this pull request
Aug 2, 2026
…IT_PER_DAY (cheaper-inference#2266) feat(api-keys): configurable default rate limits via DEFAULT_RATE_LIMIT_PER_DAY (cheaper-inference#2266 — thanks @gleber)
HouMinXi
pushed a commit
to HouMinXi/OmniRoute
that referenced
this pull request
Aug 2, 2026
Poid-ZA
pushed a commit
to Poid-ZA/OmniRoute
that referenced
this pull request
Aug 5, 2026
…IT_PER_DAY (cheaper-inference#2266) feat(api-keys): configurable default rate limits via DEFAULT_RATE_LIMIT_PER_DAY (cheaper-inference#2266 — thanks @gleber)
Poid-ZA
pushed a commit
to Poid-ZA/OmniRoute
that referenced
this pull request
Aug 5, 2026
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…IT_PER_DAY (cheaper-inference#2266) feat(api-keys): configurable default rate limits via DEFAULT_RATE_LIMIT_PER_DAY (cheaper-inference#2266 — thanks @gleber)
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The fallback rate-limit table (1000/day, 5000/week, 20000/month) in
src/shared/utils/apiKeyPolicy.tsis hard-coded and applied to any API key whoserate_limitscolumn is null. 1000/day is easily exhausted by multi-worker indexing pipelines and there is currently no per-deployment knob to relax it.This PR replaces the constant array with a small builder that reads
DEFAULT_RATE_LIMIT_PER_DAY:0(default) — empty rule set.checkRateLimit()already short-circuits empty input as allowed (rateLimiter.ts:93), so keys without an explicit configuration become effectively unlimited.N/day, 5N/week, 20N/month(same ratio as the original table, scaled).Note that the default behavior changes from "1000/day fallback" to "no fallback". Deployments that relied on the old defaults can restore them with
DEFAULT_RATE_LIMIT_PER_DAY=1000.Change
apiKeyPolicy.ts: extractbuildDefaultRateLimits(envValue?)(exported for testing) and call it once at module load to populate the existingDEFAULT_RATE_LIMITSconst.tests/unit/apikey-policy-default-rate-limits.test.ts: covers default-zero, positive N (deep-equal check on all three windows), and the negative / non-numeric fallback..env.example+docs/reference/ENVIRONMENT.md: document the new env var (required by the env/docs contract enforced inscripts/check/check-env-doc-sync.mjs).Test plan
node --import tsx/esm --test tests/unit/apikey-policy-default-rate-limits.test.ts— 3 passingcheck-env-doc-syncpasses (pre-commit hook)🤖 Generated with Claude Code