Skip to content

fix(oauth): require management auth on the OAuth login and connection routes - #15044

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
HouMinXi:fix/oauth-routes-manage-scope
Sep 29, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
HouMinXi:fix/oauth-routes-manage-scope

Conversation

@HouMinXi

@HouMinXi HouMinXi commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

/api/oauth/ is on the public route list so the authz pipeline leaves the
decision to each handler. Several handlers checked isAuthenticated(), and for
a public path that helper does not treat the request as a management call, so
any valid client API key passes, with no scope check and even when the key is
sent in the query string. A key issued only for inference could then start
device and browser logins, import tokens, paste credentials and poll flows to
completion, which creates provider connections or overwrites an existing one
by connectionId or matching email. That lets the key holder put an account
they control into the provider pool, or replace the operator's credentials.

The import routes in the same directory (codex, cursor, kiro, trae,
cliproxy) already call requireManagementAuth(). Use it in the remaining
handlers too, so they accept the same credentials as the rest of the
connection API: a dashboard session, the local CLI token, a scoped access
token, or an API key with the manage scope. An invalid key still gets 401 as
before; a valid key without the manage scope now gets 403.

requireManagementAuth also stopped short on a public path before a password
exists: isAuthRequired() answers "no auth needed" for any public path in that
window, so a remote caller passed the check on an unconfigured instance. It
now judges a public path as a management path, which leaves the window open
to the local operator only. The modals that call these routes read the error
through a shared helper, since the 401 and 403 bodies are objects. The static
guard test now covers every route in the directory and rejects isAuthenticated().

Related Issues

  • None. This fixes a defect found by review, not a filed issue.

Validation

  • Change type: other
  • Focused tests: tests/integration/security-hardening.test.ts, tests/unit/oauth-routes-manage-scope.test.ts
  • npm run lint
  • Reconciled with the current active release base
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • tests/integration/security-hardening.test.ts
  • tests/unit/oauth-routes-manage-scope.test.ts

Coverage Notes

  • The change is covered by the test files listed above. No coverage drop is expected; the new tests exercise the paths this PR adds.

Reviewer Notes

  • The OAuth login and connection routes now require a management key. Inference-only keys that previously called them will get 401.

… routes

/api/oauth/ is on the public route list so the authz pipeline leaves the
decision to each handler. Several handlers checked isAuthenticated(), and for
a public path that helper does not treat the request as a management call, so
any valid client API key passes, with no scope check and even when the key is
sent in the query string. A key issued only for inference could then start
device and browser logins, import tokens, paste credentials and poll flows to
completion, which creates provider connections or overwrites an existing one
by connectionId or matching email. That lets the key holder put an account
they control into the provider pool, or replace the operator's credentials.

The import routes in the same directory (codex, cursor, kiro, trae,
cliproxy) already call requireManagementAuth(). Use it in the remaining
handlers too, so they accept the same credentials as the rest of the
connection API: a dashboard session, the local CLI token, a scoped access
token, or an API key with the manage scope. An invalid key still gets 401 as
before; a valid key without the manage scope now gets 403.

requireManagementAuth also stopped short on a public path before a password
exists: isAuthRequired() answers "no auth needed" for any public path in that
window, so a remote caller passed the check on an unconfigured instance. It
now judges a public path as a management path, which leaves the window open
to the local operator only. The modals that call these routes read the error
through a shared helper, since the 401 and 403 bodies are objects. The static
guard test now covers every route in the directory and rejects isAuthenticated().

Signed-off-by: Minxi Hou <houminxi@gmail.com>
@diegosouzapw
diegosouzapw merged commit aa75ac1 into diegosouzapw:release/v3.8.51 Sep 29, 2026
9 of 16 checks passed
diegosouzapw added a commit that referenced this pull request Sep 29, 2026
… 4) (#15109)

Release-captain base-red fix (v3.8.51 release PR #11442, unit shards 3-4): one production defect (proxyLogger pulled into every settings→proxies import and queried the DB at import time; helper extracted to src/lib/proxyLogHost.ts) and seven contract propagations from #14732, #15044, #15067, #13548, #14117×#14844, #12810. 74/74 across the seven files, 176/176 proxy-log neighbours, typecheck clean.
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