Repository navigation
feat(authz): managementPolicy accepts API keys with manage scope - #2265
Merged
diegosouzapw merged 7 commits intoMay 14, 2026
Merged
diegosouzapw merged 7 commits into
diegosouzapw merged 7 commits into
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>
…cope managementPolicy previously only allowed dashboard sessions and the internal model-sync probe — every other request had to hold a cookie session. Programmatic management (provisioning providers, setting rate limits) therefore required a browser login or per-route wiring of `requireManagementAuth`. All the pieces for API-key-based management already exist: `extractApiKey` / `isValidApiKey` (sse/services/auth), `getApiKeyMetadata` (lib/db/apiKeys), and `hasManageScope` (lib/api/requireManagementAuth). This change wires them into the policy itself so the rule is uniform across routes: after the dashboard-session check, if a request carries a valid API key whose stored scopes include `manage`, it is allowed as a `management_key` principal. Validation errors fall through to the existing reject path so a backend hiccup never turns into a 5xx. Tests cover three new branches against the existing management-policy.test.ts fixture: allow on manage-scope, reject on empty-scope, and reject on an invalid bearer. Existing assertions are untouched. The test file also opts out of the Redis-backed auth cache (`OMNIROUTE_DISABLE_REDIS_AUTH_CACHE=1`) so `isValidApiKey()` stays local — without it, the new tests stall on `ETIMEDOUT` against a Redis that doesn't exist in CI. 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 updates the management authorization policy to allow programmatic access via API keys with the 'manage' scope. It includes logic for API key extraction and validation, along with new unit tests covering various authorization scenarios. Feedback focuses on removing redundant fallback logic for the API key ID and improving error handling consistency by returning a 503 status instead of a 403 when backend validation fails.
…lure Addresses review feedback on the previous commit. 1) `meta.id ?? "api-key"` was unreachable. `getApiKeyMetadata` returns `null` whenever the row has no id (`apiKeys.ts:1019-1021`), so when `meta` is truthy `meta.id` is guaranteed non-empty. Drop the fallback; the static literal in the fallback branch could only ever mask a future regression in `getApiKeyMetadata`. 2) A thrown isValidApiKey / getApiKeyMetadata now returns `reject(503, "AUTH_BACKEND_UNAVAILABLE", "...")` instead of silently falling through to the 403/401 reject below. Masking a backend outage as an auth failure tells callers their credentials are wrong when the real problem is that the server cannot validate any credential right now. This matches the pattern already used by `requireManagementAuth.ts`, so management auth surfaces a consistent status code regardless of which entry point a route uses. The inline comment is rewritten to spell out the why-503 reasoning so the next reader does not have to cross-reference the helper. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Collaborator
|
Thanks @gleber! 🙌 This is a great addition — wiring the existing management auth pieces into the policy layer enables headless/programmatic management, which is critical for automation pipelines. The error handling (503 for backend failures instead of masking as 403) shows excellent security awareness. Merging into release/v3.8.0. |
diegosouzapw
merged commit May 14, 2026
24b2e77
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>
Merged
This was referenced May 17, 2026
HouMinXi
pushed a commit
to HouMinXi/OmniRoute
that referenced
this pull request
Aug 2, 2026
…heaper-inference#2265) feat(authz): managementPolicy accepts API keys with manage scope (cheaper-inference#2265 — thanks @gleber)
HouMinXi
pushed a commit
to HouMinXi/OmniRoute
that referenced
this pull request
Aug 2, 2026
HouMinXi
pushed a commit
to HouMinXi/OmniRoute
that referenced
this pull request
Aug 2, 2026
The lockfile is local Claude Code session state that was committed by accident in 860fc36 (cheaper-inference#2265). Untrack it and add a narrow .gitignore rule covering only runtime artifacts (lock, scheduled_tasks, sessions, state) so shared command definitions at .claude/commands/ stay tracked.
Poid-ZA
pushed a commit
to Poid-ZA/OmniRoute
that referenced
this pull request
Aug 5, 2026
…heaper-inference#2265) feat(authz): managementPolicy accepts API keys with manage scope (cheaper-inference#2265 — thanks @gleber)
Poid-ZA
pushed a commit
to Poid-ZA/OmniRoute
that referenced
this pull request
Aug 5, 2026
Poid-ZA
pushed a commit
to Poid-ZA/OmniRoute
that referenced
this pull request
Aug 5, 2026
The lockfile is local Claude Code session state that was committed by accident in 24b2e77 (cheaper-inference#2265). Untrack it and add a narrow .gitignore rule covering only runtime artifacts (lock, scheduled_tasks, sessions, state) so shared command definitions at .claude/commands/ stay tracked.
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…heaper-inference#2265) feat(authz): managementPolicy accepts API keys with manage scope (cheaper-inference#2265 — thanks @gleber)
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
The lockfile is local Claude Code session state that was committed by accident in 4a0a64d (cheaper-inference#2265). Untrack it and add a narrow .gitignore rule covering only runtime artifacts (lock, scheduled_tasks, sessions, state) so shared command definitions at .claude/commands/ stay tracked.
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
managementPolicy(src/server/authz/policies/management.ts) previously only allowed dashboard sessions and the internal model-sync probe. Every other request had to hold a cookie session — programmatic management (provisioning providers, setting rate limits) therefore required a browser login or per-route wiring ofrequireManagementAuth.All the pieces for API-key-based management already exist:
extractApiKey/isValidApiKey—src/sse/services/auth.tsgetApiKeyMetadata—src/lib/db/apiKeys.tshasManageScope—src/lib/api/requireManagementAuth.tsThis PR wires them into the policy itself. After the dashboard-session check, if a request carries a valid API key whose stored scopes include
manage, it is allowed as amanagement_keyprincipal. Validation errors fall through to the existing reject path so a backend hiccup never turns into a 5xx.Change
management.ts: import the three helpers and add the new branch.tests/unit/authz/management-policy.test.ts: three new test cases — allow on manage scope, reject on empty scope, reject on invalid bearer. Existing five test cases unchanged.OMNIROUTE_DISABLE_REDIS_AUTH_CACHE=1soisValidApiKey()does not stall onETIMEDOUTagainst an absent Redis in CI.Test plan
node --import tsx/esm --test tests/unit/authz/management-policy.test.ts— 8 passing (5 existing + 3 new)🤖 Generated with Claude Code