Skip to content

fix(authz): loopback-gate the two provider routes that spawn child processes (#15159 S-01) - #15242

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.52from
jonlwheat2-gif:fix/backend-route-guard-spawn-local-only-15159
Oct 2, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.52from
jonlwheat2-gif:fix/backend-route-guard-spawn-local-only-15159

Conversation

@jonlwheat2-gif

Copy link
Copy Markdown
Contributor

Summary

Fixes S-01 from the v3.8.52 whole-repository audit (#15159) — the next item in the owner's merge order.

Two /api/providers/ surfaces spawn child processes with no loopback gate, breaking Hard Rules #15 / #17: the spawn runs before any auth check, so a leaked JWT via a tunnel could reach it.

Hop 1 — fixed paths, spawn is unconditional

src/lib/providers/validation/webProvidersB.ts:535 — spawn(bin, ["acp", "--agent-type", "summarizer"], …), reached from POST /api/providers/{validate,import,bulk}. The binary comes from a fixed candidate list and argv is fixed, so this is not caller-controlled RCE — but the loopback gate is still mandatory.

Classified via a new LOCAL_ONLY_API_PATTERNS entry scoped to exactly those three paths, so ordinary provider CRUD stays remote-reachable.

Hop 2 — deliberately NOT path-classified

src/app/api/providers/[id]/models transitively spawns via fetchCursorAgentModels() → runCursorAgent() → spawn() (src/lib/providerModels/cursorAgent.ts:17) — but only on the provider === "cursor" branch, and {id} is a connection id, never a provider name.

A [^/]+/models pattern therefore cannot separate that one spawn from the ~50 other providers' pure-HTTP discovery living in the same route; it would lock remote model discovery for every non-Cursor provider — precisely the over-broadening the existing /api/providers/[^/]+/login precedent exists to prevent.

Enforced instead at the call site: the route requires x-omniroute-peer-locality === "loopback" (stamped by the authz pipeline from the real TCP peer, never the spoofable Host header) before calling fetchCursorAgentModels(), failing closed to the cached/local catalog otherwise — the same pattern as handleCursorAgentImageGeneration in cursorAgentImage.ts.

Gate bookkeeping

The membership gate matches spawn(/exec( only inside route.ts, so it cannot follow this transitive, provider-conditional chain (that is the G-09 blind spot, tracked separately). The route is frozen in KNOWN_UNCLASSIFIED_SOURCE_SPAWN with a justification pointing at the guard — the remedy the audit itself prescribes for G-09 ("freeze the known call chains").

The old KNOWN_UNCLASSIFIED_SOURCE_SPAWN must be empty assertion is replaced by two equivalent guards rather than simply deleted: the two already-resolved routes may never be re-frozen, and every entry must carry a substantive justification.

docs/openapi.yaml: x-loopback-only: true added to the three newly gated paths.

Verification

  • TDD: wrote the failing tests first — the three paths were reachable — watched them fail, then applied the fix.
  • 462/462 pass across tests/unit/authz/, tests/unit/route-guard-*.test.ts, security-route-guard-tiers, check-route-guard-membership.
  • npm run check:route-guard-membership → OK, 0 new gaps.
  • npm run typecheck:core → clean.
  • ESLint (with the project's suppressions file, as CI runs it) → clean.
  • npm run check:docs-all → all gates pass.
  • New guard test also asserts the locality check appears before the spawn call and fails closed, so the guard cannot be silently removed or reordered.

Pre-existing issues observed (not caused by this change, not fixed here)

  • check-openapi-security-tiers.mjs already fails on CRLF checkouts: its //.*$ line-comment strip cannot match past \r, so any pattern with a trailing comment is silently dropped from the parsed set (this drops the existing volcengine-plan/connect pattern → 6 spurious mismatches). Reproduced against the unmodified base file. The new pattern puts its justification on its own line so it parses under both LF and CRLF. Worth folding into the "lying gates" batch.
  • tests/unit/cursor-agent-models.test.ts — 2 sigkill tests fail on Windows with spawn … ENOENT (fake shebang binary is not directly executable on Windows). No cursor files are touched by this diff.

Notes

  • One item per PR; cut from jonlwheat2-gif/OmniRoute:release/v3.8.52 (base dbe703a000).
  • Single commit, no AI attribution.

…ocesses (cheaper-inference#15159 S-01)

Hard Rules #15 + #17 were broken on two /api/providers/ surfaces: the spawn
runs before any auth check, so a leaked JWT via tunnel could reach it.

Hop 1 - fixed paths, spawn is unconditional (webProvidersB.ts:535, fixed
binary + fixed argv). Classified LOCAL_ONLY via a new pattern in
LOCAL_ONLY_API_PATTERNS, scoped to the three exact paths so the rest of
/api/providers/ CRUD stays remote-reachable.

Hop 2 - /api/providers/{id}/models reaches cursorAgent.ts:17, but only on
the provider === "cursor" branch, and {id} is a CONNECTION id rather than a
provider name. No path pattern can separate it from the ~50 other providers'
pure-HTTP discovery in the same route, so a [^/]+/models pattern would lock
remote model discovery for every non-Cursor provider - the exact
over-broadening the /login precedent exists to prevent. The spawn is instead
gated at its call site on x-omniroute-peer-locality === "loopback" (stamped
by the authz pipeline from the real TCP peer, never the spoofable Host
header), failing closed to the cached/local catalog - the same pattern as
handleCursorAgentImageGeneration.

The membership gate cannot follow this transitive, provider-conditional chain
(that is the G-09 blind spot, tracked separately), so the route is frozen in
KNOWN_UNCLASSIFIED_SOURCE_SPAWN with a justification pointing at the guard,
as the audit prescribes for known call chains. The freeze-empty assertion is
replaced by two equivalent guards: the resolved routes may never be
re-frozen, and every entry must carry a substantive justification.

TDD: wrote the failing membership tests first and watched all three fail
(validate/import/bulk were reachable), then applied the fix.

- tests/unit/authz/route-guard-providers-spawn-local-only.test.ts (new)
- tests/unit/check-route-guard-membership.test.ts (freeze invariants)
- docs/openapi.yaml: x-loopback-only on the three newly gated paths
…local catalog

The non-loopback guard in /api/providers/{id}/models fell through to
fetchCursorAgentModels() (a child-process spawn) whenever
buildDiscoveryFallbackResponse() returned null. Return an explicit 403 instead and cover
the empty cache + empty catalog path with a behavioral route test.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit 358d200 into cheaper-inference:release/v3.8.52 Oct 2, 2026
8 of 9 checks passed
woodsonl added a commit to woodsonl/OmniRoute that referenced this pull request Oct 11, 2026
…oute tier

A reverse-proxied dashboard (docker + nginx) got 403 LOCAL_ONLY on
/api/providers/validate for every provider key — rendered by the Add API
Key modal as a bare 'Invalid' badge with no reason, because the modal only
rendered string-shaped error bodies. cheaper-inference#15242 added the blanket pattern for
the devin CLI-spawn fallback; cheaper-inference#15766 already established the precise fix
(peer-locality gate at the spawn call site) but only wired the
test/create/test-batch routes. Drop the blunt pattern, thread allowLocalSpawn
into validate/bulk/import (cheaper-inference#15766 shape), and surface {code,message} error
envelopes in the modals via validationErrorMessage().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants