security: harden OAuth discovery and token endpoints#480
Conversation
There was a problem hiding this comment.
Pull request overview
Hardens OAuth discovery-derived endpoints to reduce protocol abuse and credential replay risk when interacting with potentially malicious OAuth/discovery servers.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Validates authorization endpoints before browser dispatch, allowing only HTTPS or loopback HTTP.
- Validates BC3 dynamic client registration endpoints with existing secure URL checks.
- Disables redirects for the auth HTTP client used by OAuth discovery/token refresh and adds coverage for unsafe authorization schemes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
internal/auth/auth.go |
Adds endpoint validation for dynamic registration and authorization URL construction. |
internal/auth/auth_test.go |
Adds regression coverage for accepted/rejected authorization endpoint schemes. |
internal/appctx/context.go |
Configures the auth HTTP client to avoid following redirects. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
jeremy
force-pushed
the
security/02-output-ansi-sanitize
branch
from
7b1d3ea to
61aea03
Compare
jeremy
force-pushed
the
security/03-oauth-hardening
branch
from
d858f7f to
9e7bb58
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e7bb58e48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jeremy
force-pushed
the
security/02-output-ansi-sanitize
branch
from
61aea03 to
e06c5c2
Compare
jeremy
force-pushed
the
security/03-oauth-hardening
branch
from
9e7bb58 to
3f9352d
Compare
jeremy
force-pushed
the
security/02-output-ansi-sanitize
branch
from
e06c5c2 to
9103d13
Compare
jeremy
force-pushed
the
security/03-oauth-hardening
branch
from
3f9352d to
396ecac
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 396ecac645
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jeremy
force-pushed
the
security/03-oauth-hardening
branch
from
396ecac to
6a3a964
Compare
jeremy
force-pushed
the
security/02-output-ansi-sanitize
branch
from
9103d13 to
dd63a13
Compare
jeremy
force-pushed
the
security/02-output-ansi-sanitize
branch
from
dd63a13 to
4566262
Compare
jeremy
force-pushed
the
security/03-oauth-hardening
branch
from
6a3a964 to
8add014
Compare
jeremy
force-pushed
the
security/02-output-ansi-sanitize
branch
from
4566262 to
c7b69c7
Compare
jeremy
force-pushed
the
security/03-oauth-hardening
branch
from
8add014 to
159b0ef
Compare
jeremy
force-pushed
the
security/03-oauth-hardening
branch
from
159b0ef to
472188d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 472188d466
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Scheme-validate the discovery authorization_endpoint (https, or http on loopback) before it's dispatched to the OS browser launcher, so a hostile discovery doc can't hand the OS a file:// (or other) URL. - Enforce RequireSecureURL on the DCR registration_endpoint. - Set CheckRedirect on the OAuth HTTP client so token exchange/refresh POST bodies (auth code, refresh_token) aren't replayed to a redirect target. Idempotent GET/HEAD (OAuth discovery) may still follow redirects, so we don't break discovery or force an unintended Launchpad fallback.
jeremy
force-pushed
the
security/03-oauth-hardening
branch
from
472188d to
e97f2a9
Compare
Harden OAuth discovery and token endpoints
(Threat: T1 malicious/compromised OAuth/discovery server.)
authorization_endpointscheme validation (auth.gobuildAuthURL): the endpoint comes from the server-controlled discovery doc and is later handed toOpenBrowser→xdg-open/open. Now required to behttps(orhttpon loopback); afile:///--leading value is rejected before dispatch.registration_endpointHTTPS (registerBC3Client): the DCR path now callsRequireSecureURL, matching the token-exchange and launchpad paths.appctx/context.go):CheckRedirect: ErrUseLastResponse. The exchange/refresh requests setGetBody, so Go would replayrefresh_token/auth code to a redirect target; RFC 6749 token endpoints don't legitimately 3xx-redirect POSTs. Mirrors the SDK API client's redirect guard.Tests
TestBuildAuthURL_RejectsUnsafeScheme(accepts https / http-loopback, rejectsfile://, foreignhttp://,javascript:,-flag).Part 3/6 of the stacked security series. Base:
security/02-output-ansi-sanitize.📚 Stack (merge bottom-up)
apito prevent token leak #478 — reject foreign-host URLs inapi(basemain)Each is independent except #482 depends on #481 (shared
root.go). #478 can land first/alone.