Skip to content

fix(authz): let a tunnel-served dashboard verify and add provider API keys - #16023

Closed
bufftop25 wants to merge 3 commits into
cheaper-inference:release/v3.8.52from
bufftop25:fix/authz-remote-provider-key-validation
Closed

bufftop25 wants to merge 3 commits into
cheaper-inference:release/v3.8.52from
bufftop25:fix/authz-remote-provider-key-validation

Conversation

@bufftop25

Copy link
Copy Markdown
Contributor

Summary

Behind a tunnel, the dashboard cannot verify or add a provider API key: POST /api/providers/{validate,import,bulk} answer 403 LOCAL_ONLY: This endpoint requires localhost access, so "Verify", saving in the single tab (it validates first) and bulk add all fail for every remote admin.

#15242 added the three paths to LOCAL_ONLY_API_PATTERNS because the Devin CLI key fallback spawns a fixed binary. #15766 later gated that same spawn at its call site (allowLocalSpawn, from the peer locality) and wired it into POST /api/providers, /api/providers/{id}/test and /test-batch, but not into these three routes, because the path lock stood in for it. The path lock is now redundant for the spawn and breaks key management for remote dashboards.

This is not limited to custom providers: the lock is by path, so verifying or adding a key for any API-key provider (openai, anthropic, groq, openrouter, custom nodes) is refused behind a tunnel.

  • Remove /api/providers/(validate|import|bulk) from LOCAL_ONLY_API_PATTERNS and the matching x-loopback-only marks in docs/openapi.yaml.
  • Pass allowLocalSpawn: getRequestPeerLocality(request) !== "remote" from the three routes, the same convention as the existing callers. A remote caller still can never trigger the spawn.

Related Issues

⚠️ base-red inherited: #15306 (the failing gates there are suite timeouts, the MCP vitest, the package artifact and the CI-only check:ai-attribution step; none touch these files).

Validation

  • Change type: authz / provider routes
  • Focused tests and category gates from the golden path
  • npm run lint (eslint with the project suppressions: clean on the touched files)
  • Reconciled with the current active release base (release/v3.8.52 @ 4ea24a2); focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Also run locally: typecheck:core, check:file-size, check-openapi-security-tiers, check-openapi-routes, check-route-guard-membership (0 new gaps), check:ai-attribution on the commit message.

Tests Added Or Updated

  • tests/unit/authz/route-guard-providers-spawn-local-only.test.ts: the old assertion (the three paths ARE local-only) is replaced by "they are NOT path-locked", plus a new test that each route derives allowLocalSpawn from getRequestPeerLocality(request) !== "remote". With the source change reverted, both new tests fail; with it they pass.
  • tests/unit/devin-cli-fallback-spawn-local-only-15159.test.ts (existing, unchanged) keeps covering the spawn gate itself with a real marker-file spawn.
  • 360 existing authz / provider route / validation tests pass with the change.

Coverage Notes

  • No new production logic: the change removes a path pattern and passes an existing flag. The spawn gate (validateDevinCloudAgentProvider) already has behavioural coverage; the new tests pin that the routes feed it.

Reviewer Notes

  • Security trade-off, stated plainly: this relaxes the path-level lock that fix(authz): loopback-gate the two provider routes that spawn child processes (#15159 S-01) #15242 added for Hard Rules feat(npm): add npm package publishing with CLI entry point #15/fix(ci): explicit .npmrc auth for npm publish #17. The enforcement moves from the route path to the only spawn site (Devin CLI fallback), the same way the cursor-agent hop in /api/providers/{id}/models is already handled. A non-local caller still gets the HTTP verdict only and never reaches spawn. isLocalOnlyPath no longer says "local-only" for these paths, so anything relying on that classification for them should be reviewed. check-route-guard-membership reports 0 new gaps.
  • Verified with tunnel-style forwarding headers (X-Forwarded-For + CF-Connecting-IP) against the built image: before, validate and bulk answered 403 LOCAL_ONLY; after, they reach the auth check. With an access token the same calls reach the validator and return its verdict.
  • Open question for the maintainers: if you prefer to keep the path lock and instead exempt these routes for authenticated management callers, I can rework it that way.

… keys

cheaper-inference#15242 put POST /api/providers/{validate,import,bulk} on the loopback-only list because the Devin
CLI key fallback in webProvidersB.ts spawns a fixed binary. Those routes are also how the dashboard
verifies, saves and bulk-adds an API key, so behind a tunnel (Cloudflare, Tailscale Funnel...) every
admin got `403 LOCAL_ONLY: This endpoint requires localhost access` on "Verify", on saving a key in
the single tab (it validates first) and on bulk add. A new provider key could not be added at all
from a remote dashboard.

The spawn is the only side effect, and cheaper-inference#15766 already gates it at its call site: validateProviderApiKey
takes allowLocalSpawn and the Devin validator skips the CLI fallback when it is false. /api/providers,
/api/providers/{id}/test and /test-batch pass it from getRequestPeerLocality(request). These three
routes never did, because the path lock stood in for it.

- remove /api/providers/(validate|import|bulk) from LOCAL_ONLY_API_PATTERNS and the matching
  x-loopback-only marks in docs/openapi.yaml;
- pass allowLocalSpawn: getRequestPeerLocality(request) !== "remote" from the three routes, the same
  convention as the other callers, so a remote caller still can never spawn a process.

Reproduced with tunnel-style forwarding headers (X-Forwarded-For + CF-Connecting-IP): validate and
bulk answered 403 LOCAL_ONLY before, and reach the auth check after. The Devin spawn gate itself
stays covered by devin-cli-fallback-spawn-local-only-15159.test.ts.
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Hi @bufftop25 — closing this pull request to clear the open queue. The branch fix/authz-remote-provider-key-validation is kept, so the commits stay. Comment here if it should be reopened.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Hi @bufftop25 — reopening this pull request. The close was a mistake, and it is back in the merge queue. The branch fix/authz-remote-provider-key-validation was kept.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @bufftop25. This is covered on release/v3.8.52 by #16366 (merged 2026-10-16): the /(validate|import|bulk) local-only pattern is gone from routeGuard.ts, and validate, bulk and import each gate the Devin spawn with allowLocalSpawn: getRequestPeerLocality(request) !== "remote"; the tracking issue #16022 is closed. Closing as subsumed. (/merge-batch 2026-10-17)

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.

fix(auth): allow remote dashboards to validate and add provider API keys

2 participants