Repository navigation
Accept self-registered redirects only on loopback, cursor:// and claude://, and allowlisted https hosts - #3229
Conversation
…de://, and allowlisted https hosts Dynamic client registration is open and unauthenticated, so an https callback on any host let anyone register a client that sends codes to a host they choose. Self-registered clients now name an http loopback redirect, a cursor:// or claude:// app redirect, or https on a host in OAUTH_DCR_ALLOWED_HTTPS_HOSTS, as bc3's strict allowlist did before bc3#13778. Unset, no https host is allowed. SaaS lists the Claude, ChatGPT, Cursor and Grok callback hosts. The host check runs only on hosts a registration adds, so a client keeps working after its host leaves the list.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![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: 0367b0e1a7
ℹ️ 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".
A hostless cursor:/callback showed consent no destination, so app-scheme redirects now name a plain host, and consent shows it with its scheme. An app scheme naming a loopback host (cursor://localhost) was treated as loopback, which earned it pre-consent error redirects; loopback now means http.
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Tests on
The SaaS run on Codex: both findings on |
* origin/oauth-token-digests: Refuse an OAuth parameter that arrives as a list where one value is meant
* origin/oauth-token-digests: (48 commits) Bring main's tests up to this branch: Ruby 4, webhook attribution, token digests Pin the fizzy_mcp accessory to fizzy-mcp-server v0.1.0 by digest Scope webhook dispatch by account as well as board Test the reviewer audit against the structured log line production writes Revoke on the email alone, and record each use in the request's structured log Remove the reviewer's access tokens and passkeys on sign-out, so a code holder can't keep a way back in Wire the reviewer sign-in into production from Deploy/Fizzy Close the reviewer's live sockets on sign-out, and keep the identity id out of Sentry End the reviewer's sessions on rotation, so the old code's sessions don't outlive it Refuse a code that normalizes to nothing, not just a blank one Let the directory reviewer sign in with a fixed code, SaaS only and off unless configured Refuse docker-dev's shared default port in a git worktree Ruby 4.0.9 (#3234) Retry the offline clear only in an active worker Retry clearing old offline pages whenever the worker starts Clear offline pages rendered under an older rendering version Re-key every fragment and page ETag past crass 1.0.7 Bump crass to 1.0.8 in Gemfile.saas.lock, and test rich text style scrubbing Bump crass to 1.0.8 for GHSA-pq4g-8rxf-4g29 story accepted ...
* origin/oauth-token-digests: Filter the code parameter by its whole name, so code_challenge stays readable
* origin/oauth-token-digests: Redact retired refresh tokens from logged SQL
Consent names a self-registered app by where its access returns, so a callback on fizzy.do, basecamp.com, 37signals.com or hey.com would read as ours. The allowlist names only vendor hosts, so ours are already refused; this holds that with production's list.
… DCR allowlist B's account-picker test asserted its plain select; #3212 renders radio cards. CIMD's unknown-client tests read a JSON error; #3212 shows the error page. DCR's app-scheme test looked for the old consent line; #3212 says the app on your device. #3212's connected-apps test registered two https hosts that #3229 now vets.
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9c1872acc
ℹ️ 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".
Self-registered clients may now use only on-device callbacks, the cursor:// and claude:// schemes, or an https host the operator lists. An install that sets nothing refuses web-based clients' registrations after upgrading, so docker-deployment.md says how to list them.
…ping partway * origin/oauth-token-digests: Make this branch's migrations safe to run again after stopping partway Make this branch's migrations safe to run again after stopping partway Make the OAuth migrations safe to run again after stopping partway
* origin/oauth-token-digests: Answer 404 from a dark endpoint however many times it's asked
* tp/oauth-token-digests: Name v0.2.0 in the pin's comment Pin fizzy-mcp-server v0.2.0 Refuse a join code that was reset while it was being redeemed Filter presigned storage signatures from logs Keep the reviewer's memberships in reach, and print the member audit escaped Import an export that lacks a column this database has only just gained List the reviewer accounts' members when signing the reviewer out Bump thruster from 0.1.26 to 0.1.27 (#3199) Give the asset build a placeholder direct-upload credential Anchor the stubbed storage host pattern Sign direct uploads with a write-only storage credential Lock the reviewer's email address, so a code holder can't move its membership out of revocation's reach Use Ruby's GC defaults and log job memory (#3238)
* tp/oauth-cimd: (49 commits) Refuse an exchange whose subject token was rotated away before the lock Answer an exchange that isn't on the same way however many times it's asked Make this branch's migrations safe to run again after stopping partway Make this branch's migrations safe to run again after stopping partway Leave a client_id list to the shared single-valued check Hold the exchange to the token endpoint's single-valued check Refuse resource and account_id sent as lists as invalid_request Attribute what a delegated token sets up to the app the person connected Hold metadata documents to per-person budgets and the strict address policy Refuse a subject with too little life left to trade for a usable token Say what the exchange's media type gate is for, now the shared count reads every body Accept the exchange only form-urlencoded, where every repeated resource is visible Refuse an exchange that names several resources, counted in the raw body Accept the API's advertised resource identifier, and give each MCP resource a client of its own Resolve the exchange's scope again under the grant's lock Look again on the primary only for a token of exactly the minted shape Show a delegated token reaching its subject's account alone, and finding it at /my/identity Lock the exchange client while minting, judge a delegated token on the primary whole, and narrow it with its grant Judge the exchange's resource against the issuer the request came to Exchange an MCP grant for a short-lived API token (RFC 8693 delegation) ... # Conflicts: # app/models/oauth.rb # app/models/oauth/client.rb # config/environments/development.rb # config/environments/production.rb # config/environments/test.rb # saas/config/deploy.yml
* tp/oauth-cimd: Report a personal access token as pat whether or not it has the personal access client
jeremy
changed the title
* tp/oauth-cimd: Audit a personal access token from the personal access client as a pat Bring main's tests up to this branch: Ruby 4 and the access token finder
* tp/oauth-cimd: Pin the load balancers to kamal-proxy v0.10.2, and say why configure-lb-mcp.sh still checks Give the load balancer the real client IP
* tp/oauth-cimd: Answer a dark server's exchange with 404 first, and time exchanges with the token endpoint
|
🤖 @codex review |
There was a problem hiding this comment.
🔵 Needs a closer look
The security-sensitive OAuth registration policy needs final human review and remains explicitly held for Jeremy.
No open issues 🎉
What changed in this PR
Tightens OAuth dynamic client registration to accept only HTTP loopback callbacks, cursor:// and claude:// app callbacks, and explicitly allowlisted HTTPS hosts.
Changes:
- Centralizes redirect validation while preserving existing clients’ HTTPS hosts and operator-provisioned exemptions.
- Configures SaaS vendor hosts and documents strict-by-default self-hosted behavior.
- Updates consent destination display and adds model and integration coverage for registration and authorization.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
test/test_helpers/oauth_availability_test_helper.rb |
Adds temporary HTTPS allowlist configuration for tests. |
test/models/oauth/client_test.rb |
Tests host restrictions, app schemes, and existing-client compatibility. |
test/integration/oauth_dcr_redirect_allowlist_test.rb |
Covers registration restrictions and authorization flows. |
saas/config/deploy.yml |
Sets the SaaS HTTPS callback host allowlist. |
docs/docker-deployment.md |
Documents accepted callbacks and self-hosted configuration. |
config/environments/test.rb |
Sets the test callback host allowlist. |
config/environments/production.rb |
Loads allowed hosts from the environment. |
config/environments/development.rb |
Enables environment-based host configuration locally. |
app/views/oauth/authorizations/new.html.erb |
Distinguishes HTTP loopback and app-scheme destinations. |
app/models/oauth/client.rb |
Validates newly added HTTPS hosts and shared redirect rules. |
app/models/oauth.rb |
Defines shared redirect policy and allowlist helpers. |
app/controllers/oauth/clients_controller.rb |
Enforces registration policy and explains accepted redirects. |
Review effort: Balanced
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…-age * origin/oauth-cimd: Record a metadata document's registration as the client's own, find a revoking client by exact id, and read a quoted max-age Judge an exchange by the client row it locked, and look only for a delegated token's digest on the primary Deliver an app's push in an account only through a grant that reaches that account, and drop a duplicate render_error
|
🤖 @codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
* origin/oauth-cimd: Count a successful delegation exchange as a token grant, and run its sweep as a named cleanup step Refuse a whitespace resource indicator, and let an account-bound grant refresh only while its account is the person's
|
🤖 @codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
* origin/oauth-cimd: Answer a denial with access_denied whatever the stored row's scopes, and refund the fetch budget when admission is lost
|
🤖 @codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
The staff gate from basecamp#3246, reordered to land before the consent design (basecamp#3212). It carries basecamp#3246's net change against basecamp#3212 as one commit on basecamp#3229's head, with no dependency on basecamp#3212.
Held for Jeremy. Draft; don't merge. Stacked on #3192 (
oauth-token-digests).Dynamic client registration is open and unauthenticated. After #3080 it accepted an https callback on any host, so anyone could register a client that sends authorization codes to a host they chose; the consent screen was the only check. This tightens it to bc3's strict allowlist, the policy bc3 ran before bc3#13778 opened https (Jeremy's ruling for Fizzy).
The rule
A self-registered client may name only:
http://127.0.0.1,http://[::1],http://localhost, any port (RFC 8252 §7.3)LOOPBACK_ADDRESSEScursor://…,claude://…(RFC 8252 §7.1)DCR_ALLOWED_CUSTOM_SCHEMEShttps://on a host inOAUTH_DCR_ALLOWED_HTTPS_HOSTSDCR_ALLOWED_HTTPS_HOSTScursorandclaude. They must be hierarchical (cursor://host/path, never the opaquecursor:path, which can't carry?code=), with no userinfo or fragment, and must name a host or path. Dotted reverse-DNS schemes get no pass on shape, because some route to a remote handler (z39.50r,iris.beep), as bc3#13431 found. This is new for Fizzy: Accept https redirect URIs for dynamically registered clients #3080 rejected every custom scheme for DCR.invalid_redirect_uri. One unallowed URI refuses the whole registration, and the description lists what's accepted, hosts included.Oauth::Client#added_https_hosts_are_vetted, run on create or whenredirect_urischanges), never on hosts a client already holds. A client keeps saving and authorizing after its host leaves the list. Tests pin both.dynamically_registered: false) aren't held to the list, and neither are CIMD clients onoauth-cimd, which register withdynamically_registered: false. The pre-consent rule is also unchanged: errors for a self-registered client still render on Fizzy unless its redirect is loopback.Default host list (SaaS,
saas/config/deploy.yml)OAUTH_DCR_ALLOWED_HTTPS_HOSTS: "claude.ai claude.com chatgpt.com www.cursor.com grok.com"claude.aihttps://claude.ai/api/mcp/auth_callback)doc/oauth/production_light_up.md#L671claude.comhttps://claude.com/api/mcp/auth_callback)chatgpt.comhttps://chatgpt.com/aip/g/oauth/callback)www.cursor.comhttps://www.cursor.com/agents/mcp/oauth/callback)redirect_uris.rbbefore #13778,DCR_ALLOWED_HTTPS_HOSTSgrok.comhttps://grok.com/connectors-oauth-exchange-code/)bc3's list also held
muse.aiandagent.meta.ai, for Meta's Muse. They're left out because the ruling named Claude, ChatGPT, Cursor and Grok. Adding them is one line indeploy.yml.In bc3, Claude and ChatGPT reach the server through CIMD, not DCR. They're listed here because Fizzy's CIMD (
oauth-cimd) isn't merged, and DCR is their fallback until it is (the directory packet: "OAuth with CIMD; DCR as the fallback").OSS and self-hosted: strict by default
Unset,
OAUTH_DCR_ALLOWED_HTTPS_HOSTSallows no https host: only loopback and the two app schemes. A self-hoster who wants a hosted connector names its host. This mirrors bc3's positive model: an open-registration server shouldn't accept arbitrary remote sinks by default, and an install that never mentions DCR gets the safe posture.config.x.oauth.dcr_allowed_https_hostsis read inproduction.rbanddevelopment.rb, separated by spaces or commas.test.rbnamesconnector.example.comso existing tests keep their https client.Expected conflicts
oauth-cimd): both PRs touchapp/models/oauth/client.rb(validations, private helpers) andapp/controllers/oauth/clients_controller.rb. CIMD clients aren'tdynamically_registered, so the rules don't interact. Only the text conflicts.Oauth::Client. Expect small textual conflicts near the validations.connection_kindgroups by how a client registered. Acursor://orclaude://app redirect should probably group with "on your devices", like loopback. Worth a line when the two meet.Tests
test/integration/oauth_dcr_redirect_allowlist_test.rbcovers:cursor:) and its code arriving atcursor://…;test/models/oauth/client_test.rbcovers:z39.50r,iris.beep,javascript, opaquecursor:callback, userinfo and a fragment.Third-party data flow
A person who authorizes the Cursor or Claude desktop app via
cursor://orclaude://sends an authorization code for their own Fizzy data to that app, on their device. This is the flow the Trust & Compliance card Fizzy OAuth: people can connect third-party apps to their Fizzy data covers. No new subprocessor: Fizzy sends nothing to Cursor or Anthropic itself. This PR narrows the flow overall, from any https callback (#3080) to on-device callbacks, these two schemes, and operator-listed https hosts.