From a654fc852ab480320b175ed5ffeaaea568f31342 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 27 Aug 2026 23:08:24 -0700
Subject: [PATCH 01/15] Accept https redirect URIs for dynamically registered
 clients

Hosted MCP connectors register over DCR with https callback URLs, which
the loopback-only policy rejected. Registration now accepts redirect
URIs that are either http loopback (unchanged, with port-flexible
matching) or https on a non-loopback host (exact matching, no
fragments). Plain http on public hosts, custom schemes, and https
loopback stay rejected. Operator-provisioned clients are unaffected;
the trusted flag remains manual provisioning only.
---
 app/controllers/oauth/clients_controller.rb | 26 +++---
 app/models/oauth/client.rb                  |  8 +-
 test/integration/oauth_flow_test.rb         | 87 ++++++++++++++++++++-
 test/models/oauth/client_test.rb            | 27 ++++++-
 4 files changed, 130 insertions(+), 18 deletions(-)

diff --git a/app/controllers/oauth/clients_controller.rb b/app/controllers/oauth/clients_controller.rb
index d36296904a..c38f6ba3b3 100644
--- a/app/controllers/oauth/clients_controller.rb
+++ b/app/controllers/oauth/clients_controller.rb
@@ -6,7 +6,7 @@ class Oauth::ClientsController < Oauth::BaseController
 
   before_action :require_issuance_enabled
   before_action :validate_redirect_uris
-  before_action :validate_loopback_uris
+  before_action :validate_redirect_uri_origins
   before_action :validate_auth_method
 
   def create
@@ -46,9 +46,9 @@ def bounded_redirect_uris?(uris)
         uris.all? { |uri| !uri.is_a?(String) || uri.length <= Oauth::Client::MAXIMUM_REDIRECT_URI_LENGTH })
     end
 
-    def validate_loopback_uris
-      unless performed? || all_loopback_uris?(params[:redirect_uris])
-        oauth_error "invalid_redirect_uri", "Only loopback redirect URIs are allowed for dynamic registration"
+    def validate_redirect_uri_origins
+      unless performed? || all_registrable_uris?(params[:redirect_uris])
+        oauth_error "invalid_redirect_uri", "Only https or local loopback redirect URIs are allowed for dynamic registration"
       end
     end
 
@@ -58,16 +58,14 @@ def validate_auth_method
       end
     end
 
-    def all_loopback_uris?(uris)
+    def all_registrable_uris?(uris)
       uris.is_a?(Array) &&
-        uris.all? { |uri| uri.is_a?(String) && valid_loopback_uri?(uri) }
+        uris.all? { |uri| uri.is_a?(String) && registrable_uri?(uri) }
     end
 
-    def valid_loopback_uri?(uri)
+    def registrable_uri?(uri)
       parsed = URI.parse(uri)
-      parsed.scheme == "http" &&
-        Oauth::LOOPBACK_HOSTS.include?(parsed.host) &&
-        parsed.fragment.nil?
+      parsed.fragment.nil? && (loopback_uri?(parsed) || https_uri?(parsed))
     rescue URI::InvalidURIError
       false
     end
@@ -76,6 +74,14 @@ def registered_client_name
       (params[:client_name] if params[:client_name].is_a?(String)).presence || "MCP Client"
     end
 
+    def loopback_uri?(parsed)
+      parsed.scheme == "http" && Oauth::LOOPBACK_HOSTS.include?(parsed.host)
+    end
+
+    def https_uri?(parsed)
+      parsed.scheme == "https" && parsed.host.present? && !Oauth::LOOPBACK_HOSTS.include?(parsed.host)
+    end
+
     def validated_scopes
       requested = case params[:scope]
       when String then params[:scope].split
diff --git a/app/models/oauth/client.rb b/app/models/oauth/client.rb
index 2dd91a4b4b..c6425d3ac8 100644
--- a/app/models/oauth/client.rb
+++ b/app/models/oauth/client.rb
@@ -49,8 +49,8 @@ def validate_redirect_uri(uri)
         errors.add :redirect_uris, "must not contain fragments"
       end
 
-      if dynamically_registered? && !valid_loopback_uri?(parsed)
-        errors.add :redirect_uris, "must be a local loopback URI for dynamically registered clients"
+      if dynamically_registered? && !valid_loopback_uri?(parsed) && !valid_https_uri?(parsed)
+        errors.add :redirect_uris, "must be an https or local loopback URI for dynamically registered clients"
       end
     rescue URI::InvalidURIError
       errors.add :redirect_uris, "includes an invalid URI"
@@ -66,6 +66,10 @@ def valid_loopback_uri?(parsed)
       parsed.scheme == "http" && parsed.host.in?(Oauth::LOOPBACK_HOSTS)
     end
 
+    def valid_https_uri?(parsed)
+      parsed.scheme == "https" && parsed.host.present? && !parsed.host.in?(Oauth::LOOPBACK_HOSTS)
+    end
+
     # Only the port may vary, and only for an http loopback redirect (RFC 8252
     # §7.3): https or a native scheme on loopback stays exact. The host must match too:
     # 127.0.0.1, localhost and ::1 are not interchangeable, and localhost
diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb
index 3c84be5ce5..a2e975bc1c 100644
--- a/test/integration/oauth_flow_test.rb
+++ b/test/integration/oauth_flow_test.rb
@@ -836,12 +836,29 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_response :created
   end
 
-  test "DCR rejects non-loopback redirect" do
+  test "DCR creates client with https redirect" do
+    assert_difference "Oauth::Client.count", 1 do
+      untenanted do
+        post oauth_clients_path, params: {
+          client_name: "Hosted Connector",
+          redirect_uris: [ "https://connector.example.com/callback" ]
+        }, as: :json
+      end
+    end
+
+    assert_response :created
+    body = response.parsed_body
+
+    assert_not_nil body["client_id"]
+    assert_equal [ "https://connector.example.com/callback" ], body["redirect_uris"]
+  end
+
+  test "DCR rejects plain http non-loopback redirect" do
     assert_no_difference "Oauth::Client.count" do
       untenanted do
         post oauth_clients_path, params: {
           client_name: "Evil Client",
-          redirect_uris: [ "https://evil.com/steal" ]
+          redirect_uris: [ "http://evil.com/steal" ]
         }, as: :json
       end
     end
@@ -888,6 +905,72 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_equal "MCP Client", response.parsed_body["client_name"]
   end
 
+  test "DCR rejects https loopback redirect" do
+    assert_no_difference "Oauth::Client.count" do
+      untenanted do
+        post oauth_clients_path, params: {
+          client_name: "HTTPS Loopback",
+          redirect_uris: [ "https://127.0.0.1:8888/callback" ]
+        }, as: :json
+      end
+    end
+
+    assert_response :bad_request
+    assert_equal "invalid_redirect_uri", response.parsed_body["error"]
+  end
+
+  test "DCR rejects https redirect with fragment" do
+    assert_no_difference "Oauth::Client.count" do
+      untenanted do
+        post oauth_clients_path, params: {
+          client_name: "Fragment Client",
+          redirect_uris: [ "https://connector.example.com/callback#section" ]
+        }, as: :json
+      end
+    end
+
+    assert_response :bad_request
+    assert_equal "invalid_redirect_uri", response.parsed_body["error"]
+  end
+
+  test "https redirect requires exact match in authorization" do
+    sign_in_as :david
+
+    untenanted do
+      post oauth_clients_path, params: {
+        client_name: "Hosted Connector",
+        redirect_uris: [ "https://connector.example.com/callback" ]
+      }, as: :json
+    end
+    client_id = response.parsed_body["client_id"]
+
+    untenanted do
+      get new_oauth_authorization_path, params: {
+        client_id: client_id,
+        redirect_uri: "https://connector.example.com/callback",
+        response_type: "code",
+        code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
+        code_challenge_method: "S256",
+        scope: "read",
+        state: "xyz123"
+      }
+    end
+    assert_response :success
+
+    untenanted do
+      get new_oauth_authorization_path, params: {
+        client_id: client_id,
+        redirect_uri: "https://connector.example.com:8443/callback",
+        response_type: "code",
+        code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
+        code_challenge_method: "S256",
+        scope: "read",
+        state: "xyz123"
+      }
+    end
+    assert_response :bad_request
+  end
+
   test "DCR requires redirect_uris" do
     untenanted do
       post oauth_clients_path, params: { client_name: "No Redirect" }, as: :json
diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index e4a0d14207..5521b49405 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -26,14 +26,23 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert_includes client.errors[:redirect_uris], "can't be blank"
   end
 
-  test "dynamically registered clients must use http loopback URIs" do
+  test "dynamically registered clients can use https URIs" do
+    client = Oauth::Client.new(
+      name: "Hosted Connector",
+      redirect_uris: %w[ https://connector.example.com/callback ],
+      dynamically_registered: true
+    )
+    assert client.valid?
+  end
+
+  test "dynamically registered clients reject plain http for non-loopback hosts" do
     client = Oauth::Client.new(
       name: "External",
-      redirect_uris: %w[ https://evil.com/callback ],
+      redirect_uris: %w[ http://example.com/callback ],
       dynamically_registered: true
     )
     assert_not client.valid?
-    assert_includes client.errors[:redirect_uris], "must be a local loopback URI for dynamically registered clients"
+    assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients"
   end
 
   test "dynamically registered clients reject https loopback" do
@@ -43,7 +52,17 @@ class Oauth::ClientTest < ActiveSupport::TestCase
       dynamically_registered: true
     )
     assert_not client.valid?
-    assert_includes client.errors[:redirect_uris], "must be a local loopback URI for dynamically registered clients"
+    assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients"
+  end
+
+  test "dynamically registered clients reject custom schemes" do
+    client = Oauth::Client.new(
+      name: "Native App",
+      redirect_uris: %w[ myapp://callback ],
+      dynamically_registered: true
+    )
+    assert_not client.valid?
+    assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients"
   end
 
   test "redirect URIs must not contain fragments" do

From a1d9f1b56d4993bc34c986b9aa9c0ecfcf3b500a Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 27 Aug 2026 23:22:36 -0700
Subject: [PATCH 02/15] Match loopback redirects per URI and reject empty
 fragments
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Port-flexible loopback matching was gated on every registered URI being
loopback, so a client registering both a loopback and an https callback
lost the flexibility for its loopback URI. Match each presented URI on
its own merits instead, which leaves loopback? without callers — gone.

Model validation also accepted an empty fragment (https://x/cb#) that
the DCR endpoint rejects: URI#fragment returns "" there, which
present? misses. Reject any non-nil fragment so both layers agree.
---
 app/models/oauth/client.rb       |  8 ++------
 test/models/oauth/client_test.rb | 27 +++++++++++++++++----------
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/app/models/oauth/client.rb b/app/models/oauth/client.rb
index c6425d3ac8..931aeff97c 100644
--- a/app/models/oauth/client.rb
+++ b/app/models/oauth/client.rb
@@ -20,12 +20,8 @@ class Oauth::Client < ApplicationRecord
   scope :dynamically_registered, -> { where dynamically_registered: true }
 
 
-  def loopback?
-    redirect_uris.all? { |uri| loopback_uri?(uri) }
-  end
-
   def allows_redirect?(uri)
-    redirect_uris.include?(uri) || (loopback? && loopback_uri?(uri) && matching_loopback?(uri))
+    redirect_uris.include?(uri) || (loopback_uri?(uri) && matching_loopback?(uri))
   end
 
   def allows_scope?(requested_scope)
@@ -45,7 +41,7 @@ def redirect_uris_are_valid
     def validate_redirect_uri(uri)
       parsed = URI.parse(uri)
 
-      if parsed.fragment.present?
+      unless parsed.fragment.nil?
         errors.add :redirect_uris, "must not contain fragments"
       end
 
diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index 5521b49405..a6db930311 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -75,6 +75,16 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert_includes client.errors[:redirect_uris], "must not contain fragments"
   end
 
+  test "redirect URIs must not contain empty fragments" do
+    client = Oauth::Client.new(
+      name: "Empty Fragment",
+      redirect_uris: %w[ https://connector.example.com/callback# ],
+      dynamically_registered: true
+    )
+    assert_not client.valid?
+    assert_includes client.errors[:redirect_uris], "must not contain fragments"
+  end
+
   test "dynamically registered clients can use 127.0.0.1" do
     client = Oauth::Client.new(
       name: "Loopback",
@@ -102,16 +112,6 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert client.valid?
   end
 
-  test "loopback? returns true for loopback-only clients" do
-    client = Oauth::Client.new(redirect_uris: %w[ http://127.0.0.1:8888/cb http://localhost:9999/cb ])
-    assert client.loopback?
-  end
-
-  test "loopback? returns false for non-loopback clients" do
-    client = Oauth::Client.new(redirect_uris: %w[ https://example.com/cb ])
-    assert_not client.loopback?
-  end
-
   test "allows_redirect? matches exact URI" do
     client = Oauth::Client.new(redirect_uris: %w[ http://127.0.0.1:8888/callback ])
     assert client.allows_redirect?("http://127.0.0.1:8888/callback")
@@ -157,6 +157,13 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert_not client.allows_redirect?("http://127.0.0.1:9999/other")
   end
 
+  test "allows_redirect? keeps loopback flexibility for mixed registrations" do
+    client = Oauth::Client.new(redirect_uris: %w[ http://127.0.0.1:8888/callback https://connector.example.com/callback ])
+    assert client.allows_redirect?("http://127.0.0.1:9999/callback")
+    assert client.allows_redirect?("https://connector.example.com/callback")
+    assert_not client.allows_redirect?("https://connector.example.com:8443/callback")
+  end
+
   test "allows_scope? checks client scopes" do
     client = Oauth::Client.new(scopes: %w[ read write ])
     assert client.allows_scope?("read")

From e3051785254d9afb0040291a8fa65e0fe1aa504b Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 27 Aug 2026 23:25:25 -0700
Subject: [PATCH 03/15] Downcase hosts before loopback checks and name the
 redirect host on consent

Hostnames are case-insensitive but the loopback checks weren't, so
https://LOCALHOST/callback slipped past the https-loopback rejection.
All loopback classification now funnels through Oauth.loopback_host?,
which downcases first.

The consent screen also labeled every dynamically registered client a
local tool. Hosted clients now show the redirect host instead, so the
person authorizing can see where access is headed.
---
 app/controllers/oauth/clients_controller.rb |  4 ++--
 app/models/oauth.rb                         |  4 ++++
 app/models/oauth/client.rb                  |  8 ++++----
 app/views/oauth/authorizations/new.html.erb |  7 ++++++-
 test/integration/oauth_flow_test.rb         | 15 +++++++++++++++
 test/models/oauth/client_test.rb            | 10 ++++++++++
 6 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/app/controllers/oauth/clients_controller.rb b/app/controllers/oauth/clients_controller.rb
index c38f6ba3b3..934b21f724 100644
--- a/app/controllers/oauth/clients_controller.rb
+++ b/app/controllers/oauth/clients_controller.rb
@@ -75,11 +75,11 @@ def registered_client_name
     end
 
     def loopback_uri?(parsed)
-      parsed.scheme == "http" && Oauth::LOOPBACK_HOSTS.include?(parsed.host)
+      parsed.scheme == "http" && Oauth.loopback_host?(parsed.host)
     end
 
     def https_uri?(parsed)
-      parsed.scheme == "https" && parsed.host.present? && !Oauth::LOOPBACK_HOSTS.include?(parsed.host)
+      parsed.scheme == "https" && parsed.host.present? && !Oauth.loopback_host?(parsed.host)
     end
 
     def validated_scopes
diff --git a/app/models/oauth.rb b/app/models/oauth.rb
index f824442f28..8a1010e63a 100644
--- a/app/models/oauth.rb
+++ b/app/models/oauth.rb
@@ -7,6 +7,10 @@ def self.canonical_scope(scope)
     scope.to_s.split.include?("write") ? "read write" : "read"
   end
 
+  def self.loopback_host?(host)
+    LOOPBACK_HOSTS.include?(host.to_s.downcase)
+  end
+
   def self.table_name_prefix
     "oauth_"
   end
diff --git a/app/models/oauth/client.rb b/app/models/oauth/client.rb
index 931aeff97c..b39b77cbce 100644
--- a/app/models/oauth/client.rb
+++ b/app/models/oauth/client.rb
@@ -53,17 +53,17 @@ def validate_redirect_uri(uri)
     end
 
     def loopback_uri?(uri)
-      Oauth::LOOPBACK_HOSTS.include?(URI.parse(uri).host)
+      Oauth.loopback_host?(URI.parse(uri).host)
     rescue URI::InvalidURIError
       false
     end
 
     def valid_loopback_uri?(parsed)
-      parsed.scheme == "http" && parsed.host.in?(Oauth::LOOPBACK_HOSTS)
+      parsed.scheme == "http" && Oauth.loopback_host?(parsed.host)
     end
 
     def valid_https_uri?(parsed)
-      parsed.scheme == "https" && parsed.host.present? && !parsed.host.in?(Oauth::LOOPBACK_HOSTS)
+      parsed.scheme == "https" && parsed.host.present? && !Oauth.loopback_host?(parsed.host)
     end
 
     # Only the port may vary, and only for an http loopback redirect (RFC 8252
@@ -77,7 +77,7 @@ def matching_loopback?(uri)
         redirect = URI.parse(redirect_uri)
 
         redirect.scheme == "http" && parsed.scheme == "http" &&
-          redirect.host.in?(Oauth::LOOPBACK_HOSTS) &&
+          Oauth.loopback_host?(redirect.host) &&
           redirect.host.casecmp?(parsed.host.to_s) &&
           redirect.path == parsed.path &&
           redirect.query == parsed.query
diff --git a/app/views/oauth/authorizations/new.html.erb b/app/views/oauth/authorizations/new.html.erb
index a37a0d24e5..8ce29b9c2b 100644
--- a/app/views/oauth/authorizations/new.html.erb
+++ b/app/views/oauth/authorizations/new.html.erb
@@ -17,7 +17,12 @@
       <strong>Application</strong>
       <p class="margin-none"><%= @client.name %></p>
       <% if @client.dynamically_registered? %>
-        <p class="txt-small txt-muted margin-none">Registered by a local tool. Only authorize if you trust it.</p>
+        <% redirect_host = URI.parse(@redirect_uri).host %>
+        <% if Oauth.loopback_host?(redirect_host) %>
+          <p class="txt-small txt-muted margin-none">Registered by a local tool. Only authorize if you trust it.</p>
+        <% else %>
+          <p class="txt-small txt-muted margin-none">Self-registered app that will receive access at <strong><%= redirect_host %></strong>. Only authorize if you trust it.</p>
+        <% end %>
       <% end %>
     </div>
 
diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb
index a2e975bc1c..bfc5d11740 100644
--- a/test/integration/oauth_flow_test.rb
+++ b/test/integration/oauth_flow_test.rb
@@ -919,6 +919,20 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_equal "invalid_redirect_uri", response.parsed_body["error"]
   end
 
+  test "DCR rejects https loopback redirect regardless of host case" do
+    assert_no_difference "Oauth::Client.count" do
+      untenanted do
+        post oauth_clients_path, params: {
+          client_name: "Cased Loopback",
+          redirect_uris: [ "https://LOCALHOST:8888/callback" ]
+        }, as: :json
+      end
+    end
+
+    assert_response :bad_request
+    assert_equal "invalid_redirect_uri", response.parsed_body["error"]
+  end
+
   test "DCR rejects https redirect with fragment" do
     assert_no_difference "Oauth::Client.count" do
       untenanted do
@@ -956,6 +970,7 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
       }
     end
     assert_response :success
+    assert_match "connector.example.com", response.body
 
     untenanted do
       get new_oauth_authorization_path, params: {
diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index a6db930311..862b18d7fd 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -45,6 +45,16 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients"
   end
 
+  test "dynamically registered clients reject https loopback regardless of host case" do
+    client = Oauth::Client.new(
+      name: "Cased Loopback",
+      redirect_uris: %w[ https://LOCALHOST:8888/callback ],
+      dynamically_registered: true
+    )
+    assert_not client.valid?
+    assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients"
+  end
+
   test "dynamically registered clients reject https loopback" do
     client = Oauth::Client.new(
       name: "HTTPS Loopback",

From 8ad173dc4886735c64aa46595f94181c57e35db1 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 27 Aug 2026 23:47:20 -0700
Subject: [PATCH 04/15] Decode percent-encoded hosts before loopback
 classification
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

https://%6cocalhost slipped past the enumerated loopback list: Ruby
preserves the encoding in URI#host while user agents decode it, so the
two layers disagreed about where the redirect lands. Decode before
comparing.

Deeper canonicalization (punycode, alternate IP spellings like 127.1)
stays out of scope: a miss classifies the host as non-loopback, which
is the stricter branch — exact matching only — and the redirect still
targets the user's own machine, granting the registrant nothing.
---
 app/models/oauth.rb                 |  2 +-
 test/integration/oauth_flow_test.rb | 14 ++++++++++++++
 test/models/oauth/client_test.rb    | 10 ++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/app/models/oauth.rb b/app/models/oauth.rb
index 8a1010e63a..363fb2690c 100644
--- a/app/models/oauth.rb
+++ b/app/models/oauth.rb
@@ -8,7 +8,7 @@ def self.canonical_scope(scope)
   end
 
   def self.loopback_host?(host)
-    LOOPBACK_HOSTS.include?(host.to_s.downcase)
+    LOOPBACK_HOSTS.include?(URI.decode_www_form_component(host.to_s).downcase)
   end
 
   def self.table_name_prefix
diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb
index bfc5d11740..0acae423d2 100644
--- a/test/integration/oauth_flow_test.rb
+++ b/test/integration/oauth_flow_test.rb
@@ -933,6 +933,20 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_equal "invalid_redirect_uri", response.parsed_body["error"]
   end
 
+  test "DCR rejects percent-encoded https loopback redirect" do
+    assert_no_difference "Oauth::Client.count" do
+      untenanted do
+        post oauth_clients_path, params: {
+          client_name: "Encoded Loopback",
+          redirect_uris: [ "https://%6cocalhost:8888/callback" ]
+        }, as: :json
+      end
+    end
+
+    assert_response :bad_request
+    assert_equal "invalid_redirect_uri", response.parsed_body["error"]
+  end
+
   test "DCR rejects https redirect with fragment" do
     assert_no_difference "Oauth::Client.count" do
       untenanted do
diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index 862b18d7fd..c9dc76d8ec 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -55,6 +55,16 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients"
   end
 
+  test "dynamically registered clients reject percent-encoded https loopback" do
+    client = Oauth::Client.new(
+      name: "Encoded Loopback",
+      redirect_uris: %w[ https://%6cocalhost:8888/callback ],
+      dynamically_registered: true
+    )
+    assert_not client.valid?
+    assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients"
+  end
+
   test "dynamically registered clients reject https loopback" do
     client = Oauth::Client.new(
       name: "HTTPS Loopback",

From 773e70ccc1c8a140af2067e90d5098d1117a3787 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <clawdito3@basecamp.com>
Date: Mon, 31 Aug 2026 12:52:02 -0700
Subject: [PATCH 05/15] Reject redirect hosts whose percent-encoding is invalid
 UTF-8
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

An unauthenticated DCR request supplying a host like https://%FF/callback
made Oauth.loopback_host? decode to an invalidly-encoded string and raise
ArgumentError on downcase, which no caller rescued — returning a 500 instead
of an invalid_redirect_uri error. Convert the decode failure into
URI::InvalidURIError at the single shared chokepoint so every caller (all of
which already rescue it) rejects the host cleanly.
---
 app/models/oauth.rb                 |  5 +++++
 test/integration/oauth_flow_test.rb | 14 ++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/app/models/oauth.rb b/app/models/oauth.rb
index 363fb2690c..cf199af7dc 100644
--- a/app/models/oauth.rb
+++ b/app/models/oauth.rb
@@ -9,6 +9,11 @@ def self.canonical_scope(scope)
 
   def self.loopback_host?(host)
     LOOPBACK_HOSTS.include?(URI.decode_www_form_component(host.to_s).downcase)
+  rescue ArgumentError
+    # A percent-encoding that decodes to invalid UTF-8 is not a usable host.
+    # Surface it as an invalid URI so every caller — all of which already
+    # rescue URI::InvalidURIError — rejects it instead of raising a 500.
+    raise URI::InvalidURIError, "invalid percent-encoding in host"
   end
 
   def self.table_name_prefix
diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb
index 0acae423d2..0dc7194f00 100644
--- a/test/integration/oauth_flow_test.rb
+++ b/test/integration/oauth_flow_test.rb
@@ -947,6 +947,20 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_equal "invalid_redirect_uri", response.parsed_body["error"]
   end
 
+  test "DCR rejects a redirect whose host decodes to invalid UTF-8" do
+    assert_no_difference "Oauth::Client.count" do
+      untenanted do
+        post oauth_clients_path, params: {
+          client_name: "Broken Host",
+          redirect_uris: [ "https://%FF/callback" ]
+        }, as: :json
+      end
+    end
+
+    assert_response :bad_request
+    assert_equal "invalid_redirect_uri", response.parsed_body["error"]
+  end
+
   test "DCR rejects https redirect with fragment" do
     assert_no_difference "Oauth::Client.count" do
       untenanted do

From 6c22487903fdd69e37a87b6c43c45c15ce903973 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 5 Oct 2026 20:06:12 -0700
Subject: [PATCH 06/15] Never vary a loopback port onto a redirect with a
 fragment
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Registration rejects fragments, but the loopback port exception matched only
scheme, host and path, so it admitted an unregistered redirect carrying one
(RFC 6749 §3.1.2).
---
 app/models/oauth/client.rb       | 10 +++++++++-
 test/models/oauth/client_test.rb |  6 ++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/app/models/oauth/client.rb b/app/models/oauth/client.rb
index b39b77cbce..2ca8a282be 100644
--- a/app/models/oauth/client.rb
+++ b/app/models/oauth/client.rb
@@ -24,6 +24,13 @@ def allows_redirect?(uri)
     redirect_uris.include?(uri) || (loopback_uri?(uri) && matching_loopback?(uri))
   end
 
+  # Whether we may send the browser to this redirect before the user has seen
+  # the consent screen. A self-registered https host is one nobody vetted, so
+  # bouncing to it unprompted would make us an open redirector (RFC 9700 §4.11.2).
+  def vetted_redirect?(uri)
+    !dynamically_registered? || loopback_uri?(uri)
+  end
+
   def allows_scope?(requested_scope)
     requested = requested_scope.to_s.split
     requested.present? && requested.all? { |s| scopes.include?(s) }
@@ -80,7 +87,8 @@ def matching_loopback?(uri)
           Oauth.loopback_host?(redirect.host) &&
           redirect.host.casecmp?(parsed.host.to_s) &&
           redirect.path == parsed.path &&
-          redirect.query == parsed.query
+          redirect.query == parsed.query &&
+          parsed.fragment.nil?
       end
     rescue URI::InvalidURIError
       false
diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index c9dc76d8ec..04cc2388db 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -177,6 +177,12 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert_not client.allows_redirect?("http://127.0.0.1:9999/other")
   end
 
+  test "allows_redirect? never varies a loopback port onto a fragment" do
+    client = oauth_clients(:mcp_client)
+    assert_not client.allows_redirect?("http://127.0.0.1:9999/callback#fragment")
+    assert_not client.allows_redirect?("http://127.0.0.1:9999/callback#")
+  end
+
   test "allows_redirect? keeps loopback flexibility for mixed registrations" do
     client = Oauth::Client.new(redirect_uris: %w[ http://127.0.0.1:8888/callback https://connector.example.com/callback ])
     assert client.allows_redirect?("http://127.0.0.1:9999/callback")

From cb61db32691bb137245edfce62e12d0b6bc4ce78 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 5 Oct 2026 20:06:16 -0700
Subject: [PATCH 07/15] Render pre-consent errors here for self-registered
 https clients
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Validation errors are found before the user has seen the consent screen.
Redirecting them to a self-registered https host would let anyone register
a URL and use us to bounce a signed-in user to it unprompted (RFC 9700
§4.11.2). Those errors now render locally. Loopback and operator-provisioned
redirects still get the error redirect, and Deny still redirects because
the consent screen has named the host by then.
---
 .../oauth/authorizations_controller.rb        |  5 +++-
 test/integration/oauth_flow_test.rb           | 30 +++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb
index 2345ddbed8..e13fdedf98 100644
--- a/app/controllers/oauth/authorizations_controller.rb
+++ b/app/controllers/oauth/authorizations_controller.rb
@@ -115,8 +115,11 @@ def valid_state?
       params[:state].nil? || (params[:state].is_a?(String) && params[:state].length <= STATE_MAXIMUM_LENGTH)
     end
 
+    # Errors are found before the user sees the consent screen, so they only
+    # redirect to a vetted target; Deny redirects anywhere allowed, since the
+    # consent screen has named the host by then.
     def redirect_with_error(error, description)
-      if params[:redirect_uri].present? && @client&.allows_redirect?(params[:redirect_uri])
+      if params[:redirect_uri].present? && @client&.allows_redirect?(params[:redirect_uri]) && @client.vetted_redirect?(params[:redirect_uri])
         redirect_to error_redirect_uri(error, description), allow_other_host: true
       else
         @error = error
diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb
index 0dc7194f00..aecd2ce9de 100644
--- a/test/integration/oauth_flow_test.rb
+++ b/test/integration/oauth_flow_test.rb
@@ -98,6 +98,36 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_match "code_challenge", redirect_params["error_description"].first
   end
 
+  test "authorization errors for a self-registered https client render here instead of redirecting" do
+    sign_in_as :david
+    client = Oauth::Client.create!(name: "Hosted", redirect_uris: %w[ https://connector.example.com/callback ], dynamically_registered: true)
+
+    get_consent_screen client: client, code_challenge: nil
+
+    assert_response :bad_request
+    assert_select "code", text: "invalid_request"
+  end
+
+  test "authorization errors for a self-registered loopback client still redirect" do
+    sign_in_as :david
+
+    get_consent_screen code_challenge: nil
+
+    assert_response :redirect
+    assert_equal "invalid_request", Rack::Utils.parse_query(URI.parse(response.location).query)["error"]
+  end
+
+  test "denying a self-registered https client still redirects, after the consent screen named its host" do
+    sign_in_as :david
+    client = Oauth::Client.create!(name: "Hosted", redirect_uris: %w[ https://connector.example.com/callback ], dynamically_registered: true)
+
+    post_consent client: client, error: "access_denied"
+
+    assert_response :redirect
+    assert_match %r{\Ahttps://connector\.example\.com/callback\?}, response.location
+    assert_equal "access_denied", Rack::Utils.parse_query(URI.parse(response.location).query)["error"]
+  end
+
   test "authorization consent issues code" do
     sign_in_as :david
     client = oauth_clients(:mcp_client)

From 5465e39e7b78a24c4fe30b5e2d731571ab857883 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 5 Oct 2026 21:06:45 -0700
Subject: [PATCH 08/15] Pin every pre-consent error against redirecting to a
 self-registered https host
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Each validation that redirects an error (scope, state, response_type,
code_challenge_method) now has a case showing it renders here for an
unvetted https client, and an operator-provisioned https client still
gets the redirect. Guards RFC 9700 §4.11.2 against a new validation
that bypasses redirect_with_error's vetting.
---
 test/integration/oauth_flow_test.rb | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb
index aecd2ce9de..784db9a696 100644
--- a/test/integration/oauth_flow_test.rb
+++ b/test/integration/oauth_flow_test.rb
@@ -108,6 +108,34 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_select "code", text: "invalid_request"
   end
 
+  test "no pre-consent error redirects to a self-registered https host" do
+    sign_in_as :david
+    client = Oauth::Client.create!(name: "Hosted", redirect_uris: %w[ https://connector.example.com/callback ], dynamically_registered: true)
+
+    {
+      { scope: "admin" } => "invalid_scope",
+      { state: nil } => "invalid_request",
+      { response_type: "token" } => "unsupported_response_type",
+      { code_challenge_method: "plain" } => "invalid_request"
+    }.each do |overrides, error|
+      get_consent_screen client: client, **overrides
+
+      assert_response :bad_request, "#{overrides} should not redirect"
+      assert_nil response.location
+      assert_select "code", text: error
+    end
+  end
+
+  test "pre-consent errors still redirect to an operator-provisioned https client" do
+    sign_in_as :david
+
+    get_consent_screen client: oauth_clients(:trusted_client), scope: "admin"
+
+    assert_response :redirect
+    assert_match %r{\Ahttps://app\.example\.com/oauth/callback\?}, response.location
+    assert_equal "invalid_scope", Rack::Utils.parse_query(URI.parse(response.location).query)["error"]
+  end
+
   test "authorization errors for a self-registered loopback client still redirect" do
     sign_in_as :david
 

From a540baa5a73f69ce6904c0798360c72118957328 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 5 Oct 2026 21:14:24 -0700
Subject: [PATCH 09/15] Pin that a mixed registration never varies an https
 loopback port (Codex)

---
 test/models/oauth/client_test.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index 04cc2388db..b270a6628b 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -188,6 +188,10 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert client.allows_redirect?("http://127.0.0.1:9999/callback")
     assert client.allows_redirect?("https://connector.example.com/callback")
     assert_not client.allows_redirect?("https://connector.example.com:8443/callback")
+
+    https_loopback = Oauth::Client.new(redirect_uris: %w[ https://127.0.0.1:8443/callback https://connector.example.com/callback ])
+    assert https_loopback.allows_redirect?("https://127.0.0.1:8443/callback")
+    assert_not https_loopback.allows_redirect?("https://127.0.0.1:9443/callback")
   end
 
   test "allows_scope? checks client scopes" do

From eb172323eb1be4bab53b67d8e780eafc568b2a65 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Tue, 6 Oct 2026 03:26:15 -0700
Subject: [PATCH 10/15] Register only redirects whose authority is plain, and
 show their port on consent

Codex's last four findings on this PR came from the same root: a self-registered
redirect's authority could say one thing and mean another. A percent-encoded
host showed raw on consent although the browser decodes it. Userinfo was ignored
when a loopback port varied. An out-of-range port registered fine but stranded
the user after consent. A nondefault port was left out of the consent warning.

Rather than canonicalize the authority everywhere it is shown or compared,
registration now accepts only a plain one: no userinfo, a host of letters,
digits, hyphens and dots (or a bracketed IPv6 literal), and a port from 1 to
65535 (Oauth.plain_authority?). The model and the DCR endpoint share that rule.

Loopback port variation also requires matching userinfo. The consent warning
names host:port whenever the port isn't the scheme's default.
---
 app/controllers/oauth/clients_controller.rb |  4 ++--
 app/models/oauth.rb                         | 13 ++++++++++++
 app/models/oauth/client.rb                  |  8 +++++---
 app/views/oauth/authorizations/new.html.erb |  6 +++---
 test/integration/oauth_flow_test.rb         | 22 +++++++++++++++++++++
 test/models/oauth/client_test.rb            | 20 +++++++++++++++++++
 6 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/app/controllers/oauth/clients_controller.rb b/app/controllers/oauth/clients_controller.rb
index 934b21f724..33fdbba6ca 100644
--- a/app/controllers/oauth/clients_controller.rb
+++ b/app/controllers/oauth/clients_controller.rb
@@ -75,11 +75,11 @@ def registered_client_name
     end
 
     def loopback_uri?(parsed)
-      parsed.scheme == "http" && Oauth.loopback_host?(parsed.host)
+      parsed.scheme == "http" && Oauth.plain_authority?(parsed) && Oauth.loopback_host?(parsed.host)
     end
 
     def https_uri?(parsed)
-      parsed.scheme == "https" && parsed.host.present? && !Oauth.loopback_host?(parsed.host)
+      parsed.scheme == "https" && Oauth.plain_authority?(parsed) && !Oauth.loopback_host?(parsed.host)
     end
 
     def validated_scopes
diff --git a/app/models/oauth.rb b/app/models/oauth.rb
index cf199af7dc..60a2e81bd3 100644
--- a/app/models/oauth.rb
+++ b/app/models/oauth.rb
@@ -16,6 +16,19 @@ def self.loopback_host?(host)
     raise URI::InvalidURIError, "invalid percent-encoding in host"
   end
 
+  # A host made of letters, digits, hyphens and dots, or a bracketed IPv6
+  # literal. Percent-encoding, which browsers decode, isn't plain.
+  PLAIN_HOST = /\A(?:\[[0-9a-f:.]+\]|[a-z0-9](?:[a-z0-9-]*[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)*)\z/i
+
+  # A self-registered redirect must name its destination plainly, so that
+  # consent shows the host a browser will actually reach and loopback matching
+  # compares like with like. That means no userinfo, a plain host, and a port
+  # a browser accepts. Rejecting anything else at registration, rather than
+  # canonicalizing it wherever it's shown or compared, keeps one rule in one place.
+  def self.plain_authority?(uri)
+    uri.userinfo.nil? && uri.host.to_s.match?(PLAIN_HOST) && uri.port.to_i.between?(1, 65535)
+  end
+
   def self.table_name_prefix
     "oauth_"
   end
diff --git a/app/models/oauth/client.rb b/app/models/oauth/client.rb
index 2ca8a282be..0f676b998d 100644
--- a/app/models/oauth/client.rb
+++ b/app/models/oauth/client.rb
@@ -66,15 +66,16 @@ def loopback_uri?(uri)
     end
 
     def valid_loopback_uri?(parsed)
-      parsed.scheme == "http" && Oauth.loopback_host?(parsed.host)
+      parsed.scheme == "http" && Oauth.plain_authority?(parsed) && Oauth.loopback_host?(parsed.host)
     end
 
     def valid_https_uri?(parsed)
-      parsed.scheme == "https" && parsed.host.present? && !Oauth.loopback_host?(parsed.host)
+      parsed.scheme == "https" && Oauth.plain_authority?(parsed) && !Oauth.loopback_host?(parsed.host)
     end
 
     # Only the port may vary, and only for an http loopback redirect (RFC 8252
-    # §7.3): https or a native scheme on loopback stays exact. The host must match too:
+    # §7.3): https or a native scheme on loopback stays exact. The host and any
+    # userinfo must match too:
     # 127.0.0.1, localhost and ::1 are not interchangeable, and localhost
     # may not even resolve to loopback (RFC 8252 §8.3).
     def matching_loopback?(uri)
@@ -86,6 +87,7 @@ def matching_loopback?(uri)
         redirect.scheme == "http" && parsed.scheme == "http" &&
           Oauth.loopback_host?(redirect.host) &&
           redirect.host.casecmp?(parsed.host.to_s) &&
+          redirect.userinfo == parsed.userinfo &&
           redirect.path == parsed.path &&
           redirect.query == parsed.query &&
           parsed.fragment.nil?
diff --git a/app/views/oauth/authorizations/new.html.erb b/app/views/oauth/authorizations/new.html.erb
index 8ce29b9c2b..40bd97e190 100644
--- a/app/views/oauth/authorizations/new.html.erb
+++ b/app/views/oauth/authorizations/new.html.erb
@@ -17,11 +17,11 @@
       <strong>Application</strong>
       <p class="margin-none"><%= @client.name %></p>
       <% if @client.dynamically_registered? %>
-        <% redirect_host = URI.parse(@redirect_uri).host %>
-        <% if Oauth.loopback_host?(redirect_host) %>
+        <% redirect = URI.parse(@redirect_uri) %>
+        <% if Oauth.loopback_host?(redirect.host) %>
           <p class="txt-small txt-muted margin-none">Registered by a local tool. Only authorize if you trust it.</p>
         <% else %>
-          <p class="txt-small txt-muted margin-none">Self-registered app that will receive access at <strong><%= redirect_host %></strong>. Only authorize if you trust it.</p>
+          <p class="txt-small txt-muted margin-none">Self-registered app that will receive access at <strong><%= redirect.port == redirect.default_port ? redirect.host : "#{redirect.host}:#{redirect.port}" %></strong>. Only authorize if you trust it.</p>
         <% end %>
       <% end %>
     </div>
diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb
index 784db9a696..baecb3d9c3 100644
--- a/test/integration/oauth_flow_test.rb
+++ b/test/integration/oauth_flow_test.rb
@@ -145,6 +145,16 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_equal "invalid_request", Rack::Utils.parse_query(URI.parse(response.location).query)["error"]
   end
 
+  test "consent names a self-registered redirect's port when it isn't the default" do
+    sign_in_as :david
+    client = Oauth::Client.create!(name: "Hosted", redirect_uris: %w[ https://connector.example.com:8443/callback ], dynamically_registered: true)
+
+    get_consent_screen client: client
+
+    assert_response :success
+    assert_select "strong", text: "connector.example.com:8443"
+  end
+
   test "denying a self-registered https client still redirects, after the consent screen named its host" do
     sign_in_as :david
     client = Oauth::Client.create!(name: "Hosted", redirect_uris: %w[ https://connector.example.com/callback ], dynamically_registered: true)
@@ -1005,6 +1015,18 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
     assert_equal "invalid_redirect_uri", response.parsed_body["error"]
   end
 
+  test "DCR rejects a redirect whose authority isn't plain" do
+    %w[ https://%65vil.example/callback https://user:secret@connector.example.com/callback
+        https://connector.example.com:65536/callback http://user@127.0.0.1:8888/callback ].each do |uri|
+      assert_no_difference "Oauth::Client.count" do
+        untenanted { post oauth_clients_path, params: { client_name: "Unplain", redirect_uris: [ uri ] }, as: :json }
+      end
+
+      assert_response :bad_request, uri
+      assert_equal "invalid_redirect_uri", response.parsed_body["error"], uri
+    end
+  end
+
   test "DCR rejects a redirect whose host decodes to invalid UTF-8" do
     assert_no_difference "Oauth::Client.count" do
       untenanted do
diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index b270a6628b..2f2635cebb 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -65,6 +65,20 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients"
   end
 
+  test "dynamically registered clients name their redirect authority plainly" do
+    %w[ https://%65vil.example/callback https://user:secret@connector.example.com/callback http://user@127.0.0.1:8888/callback
+        https://connector.example.com:65536/callback https://connector.example.com:0/callback ].each do |uri|
+      client = Oauth::Client.new(name: "Unplain", redirect_uris: [ uri ], dynamically_registered: true)
+
+      assert_not client.valid?, uri
+      assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients", uri
+    end
+
+    %w[ https://connector.example.com:8443/callback https://Connector.Example.com/callback http://[::1]:8888/callback http://localhost/callback ].each do |uri|
+      assert Oauth::Client.new(name: "Plain", redirect_uris: [ uri ], dynamically_registered: true).valid?, uri
+    end
+  end
+
   test "dynamically registered clients reject https loopback" do
     client = Oauth::Client.new(
       name: "HTTPS Loopback",
@@ -172,6 +186,12 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert_not client.allows_redirect?("https://127.0.0.1:9443/callback")
   end
 
+  test "allows_redirect? never varies a loopback port onto different userinfo" do
+    client = Oauth::Client.new(redirect_uris: %w[ http://127.0.0.1:8888/callback https://connector.example.com/callback ])
+    assert_not client.allows_redirect?("http://alice:secret@127.0.0.1:9999/callback")
+    assert_not client.allows_redirect?("http://alice@127.0.0.1:8888/callback")
+  end
+
   test "allows_redirect? requires matching path for loopback flexibility" do
     client = Oauth::Client.new(redirect_uris: %w[ http://127.0.0.1:8888/callback ])
     assert_not client.allows_redirect?("http://127.0.0.1:9999/other")

From 7881af5ffb32511ab72d000854f0626678142d74 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Tue, 6 Oct 2026 03:30:23 -0700
Subject: [PATCH 11/15] Pin that a percent-encoded IPv6 loopback host doesn't
 register

http://%5B%3A%3A1%5D:8888/callback decodes to the [::1] loopback, but browsers
reject it as a URL. The plain-authority rule already refuses it, and this test
keeps it refused.
---
 test/integration/oauth_flow_test.rb | 2 +-
 test/models/oauth/client_test.rb    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb
index baecb3d9c3..8f9c245df5 100644
--- a/test/integration/oauth_flow_test.rb
+++ b/test/integration/oauth_flow_test.rb
@@ -1017,7 +1017,7 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
 
   test "DCR rejects a redirect whose authority isn't plain" do
     %w[ https://%65vil.example/callback https://user:secret@connector.example.com/callback
-        https://connector.example.com:65536/callback http://user@127.0.0.1:8888/callback ].each do |uri|
+        https://connector.example.com:65536/callback http://user@127.0.0.1:8888/callback http://%5B%3A%3A1%5D:8888/callback ].each do |uri|
       assert_no_difference "Oauth::Client.count" do
         untenanted { post oauth_clients_path, params: { client_name: "Unplain", redirect_uris: [ uri ] }, as: :json }
       end
diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index 2f2635cebb..ecdbc2b127 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -67,7 +67,7 @@ class Oauth::ClientTest < ActiveSupport::TestCase
 
   test "dynamically registered clients name their redirect authority plainly" do
     %w[ https://%65vil.example/callback https://user:secret@connector.example.com/callback http://user@127.0.0.1:8888/callback
-        https://connector.example.com:65536/callback https://connector.example.com:0/callback ].each do |uri|
+        https://connector.example.com:65536/callback https://connector.example.com:0/callback http://%5B%3A%3A1%5D:8888/callback ].each do |uri|
       client = Oauth::Client.new(name: "Unplain", redirect_uris: [ uri ], dynamically_registered: true)
 
       assert_not client.valid?, uri

From a96b78e1c3824fe495448077b0dad5e40b6d8909 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Tue, 6 Oct 2026 03:46:59 -0700
Subject: [PATCH 12/15] Vary a loopback port only to a presented URI with a
 plain authority

A mixed client's loopback redirect let an authorization request vary the port to
65536 or 0. Ruby parses those ports, but browsers reject the URL, so the user
was stranded after consent. The presented URI must now meet the same
plain-authority rule as a registered one, port range included.
---
 app/models/oauth/client.rb       | 2 +-
 test/models/oauth/client_test.rb | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/app/models/oauth/client.rb b/app/models/oauth/client.rb
index 0f676b998d..d21a9272d4 100644
--- a/app/models/oauth/client.rb
+++ b/app/models/oauth/client.rb
@@ -81,7 +81,7 @@ def valid_https_uri?(parsed)
     def matching_loopback?(uri)
       parsed = URI.parse(uri)
 
-      redirect_uris.any? do |redirect_uri|
+      Oauth.plain_authority?(parsed) && redirect_uris.any? do |redirect_uri|
         redirect = URI.parse(redirect_uri)
 
         redirect.scheme == "http" && parsed.scheme == "http" &&
diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index ecdbc2b127..1e26940735 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -192,6 +192,13 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     assert_not client.allows_redirect?("http://alice@127.0.0.1:8888/callback")
   end
 
+  test "allows_redirect? never varies a loopback port out of the usable range" do
+    client = Oauth::Client.new(redirect_uris: %w[ http://127.0.0.1:8888/callback https://connector.example.com/callback ])
+    assert client.allows_redirect?("http://127.0.0.1:65535/callback")
+    assert_not client.allows_redirect?("http://127.0.0.1:65536/callback")
+    assert_not client.allows_redirect?("http://127.0.0.1:0/callback")
+  end
+
   test "allows_redirect? requires matching path for loopback flexibility" do
     client = Oauth::Client.new(redirect_uris: %w[ http://127.0.0.1:8888/callback ])
     assert_not client.allows_redirect?("http://127.0.0.1:9999/other")

From 43ab3b31f5da48b0b88511159d1147915d8edf1f Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Tue, 6 Oct 2026 04:14:34 -0700
Subject: [PATCH 13/15] Take a numeric last label as IPv4, and require a valid
 dotted quad

Browsers parse a host whose last label is all digits as IPv4, so a redirect
like https://999.999.999.999/callback registered fine but browsers reject it.
Within the plain-authority rule, such a host must now be four octets, each
0-255. DNS names, including ones with numeric inner labels, are unchanged.
---
 app/models/oauth.rb              | 12 +++++++++++-
 test/models/oauth/client_test.rb |  6 ++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/app/models/oauth.rb b/app/models/oauth.rb
index 60a2e81bd3..6f3132a7f0 100644
--- a/app/models/oauth.rb
+++ b/app/models/oauth.rb
@@ -26,7 +26,17 @@ def self.loopback_host?(host)
   # a browser accepts. Rejecting anything else at registration, rather than
   # canonicalizing it wherever it's shown or compared, keeps one rule in one place.
   def self.plain_authority?(uri)
-    uri.userinfo.nil? && uri.host.to_s.match?(PLAIN_HOST) && uri.port.to_i.between?(1, 65535)
+    uri.userinfo.nil? && plain_host?(uri.host.to_s) && uri.port.to_i.between?(1, 65535)
+  end
+
+  # Browsers parse a host whose last label is all digits as IPv4 (WHATWG URL),
+  # so such a host is plain only as a dotted quad of octets.
+  def self.plain_host?(host)
+    if host.match?(/(?:\A|\.)\d+\z/)
+      host.match?(/\A(?:\d{1,3}\.){3}\d{1,3}\z/) && host.split(".").all? { |octet| octet.to_i <= 255 }
+    else
+      host.match?(PLAIN_HOST)
+    end
   end
 
   def self.table_name_prefix
diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index 1e26940735..de37953dd8 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -67,14 +67,16 @@ class Oauth::ClientTest < ActiveSupport::TestCase
 
   test "dynamically registered clients name their redirect authority plainly" do
     %w[ https://%65vil.example/callback https://user:secret@connector.example.com/callback http://user@127.0.0.1:8888/callback
-        https://connector.example.com:65536/callback https://connector.example.com:0/callback http://%5B%3A%3A1%5D:8888/callback ].each do |uri|
+        https://connector.example.com:65536/callback https://connector.example.com:0/callback http://%5B%3A%3A1%5D:8888/callback
+        https://999.999.999.999/callback https://10.1/callback https://example.123/callback ].each do |uri|
       client = Oauth::Client.new(name: "Unplain", redirect_uris: [ uri ], dynamically_registered: true)
 
       assert_not client.valid?, uri
       assert_includes client.errors[:redirect_uris], "must be an https or local loopback URI for dynamically registered clients", uri
     end
 
-    %w[ https://connector.example.com:8443/callback https://Connector.Example.com/callback http://[::1]:8888/callback http://localhost/callback ].each do |uri|
+    %w[ https://connector.example.com:8443/callback https://Connector.Example.com/callback http://[::1]:8888/callback http://localhost/callback
+        https://203.0.113.7/callback https://3com.example/callback ].each do |uri|
       assert Oauth::Client.new(name: "Plain", redirect_uris: [ uri ], dynamically_registered: true).valid?, uri
     end
   end

From 9804c27eb2897b9236482e66d9dbe7484f18abc1 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Tue, 6 Oct 2026 04:26:36 -0700
Subject: [PATCH 14/15] Accept a numeric host only as canonical decimal octets

Browsers read a leading zero as octal, so https://0177.0.0.1/callback
registered as an outside host but navigates to 127.0.0.1. That slipped past
the https-loopback refusal and the consent warning. 08.0.0.1 registered but
browsers reject it. A host whose last label is a number, decimal or 0x hex
as in WHATWG's ends-in-a-number check, must now be four decimal octets from
0 to 255 with no leading zeroes.
---
 app/models/oauth.rb              | 13 +++++++++----
 test/models/oauth/client_test.rb |  5 +++--
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/app/models/oauth.rb b/app/models/oauth.rb
index 6f3132a7f0..882a7f875e 100644
--- a/app/models/oauth.rb
+++ b/app/models/oauth.rb
@@ -29,11 +29,16 @@ def self.plain_authority?(uri)
     uri.userinfo.nil? && plain_host?(uri.host.to_s) && uri.port.to_i.between?(1, 65535)
   end
 
-  # Browsers parse a host whose last label is all digits as IPv4 (WHATWG URL),
-  # so such a host is plain only as a dotted quad of octets.
+  # Browsers parse a host whose last label is a number, decimal or 0x hex, as
+  # IPv4 (WHATWG URL's ends-in-a-number check), and read a leading zero as
+  # octal. So such a host is plain only as a dotted quad of canonical decimal
+  # octets: 0-255, with no leading zeroes.
+  DECIMAL_OCTET = /(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)/
+  DOTTED_QUAD = /\A#{DECIMAL_OCTET}(?:\.#{DECIMAL_OCTET}){3}\z/
+
   def self.plain_host?(host)
-    if host.match?(/(?:\A|\.)\d+\z/)
-      host.match?(/\A(?:\d{1,3}\.){3}\d{1,3}\z/) && host.split(".").all? { |octet| octet.to_i <= 255 }
+    if host.match?(/(?:\A|\.)(?:\d+|0x\h*)\z/i)
+      host.match?(DOTTED_QUAD)
     else
       host.match?(PLAIN_HOST)
     end
diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb
index de37953dd8..1a6d4f1fbd 100644
--- a/test/models/oauth/client_test.rb
+++ b/test/models/oauth/client_test.rb
@@ -68,7 +68,8 @@ class Oauth::ClientTest < ActiveSupport::TestCase
   test "dynamically registered clients name their redirect authority plainly" do
     %w[ https://%65vil.example/callback https://user:secret@connector.example.com/callback http://user@127.0.0.1:8888/callback
         https://connector.example.com:65536/callback https://connector.example.com:0/callback http://%5B%3A%3A1%5D:8888/callback
-        https://999.999.999.999/callback https://10.1/callback https://example.123/callback ].each do |uri|
+        https://999.999.999.999/callback https://10.1/callback https://example.123/callback https://0177.0.0.1/callback https://08.0.0.1/callback
+        https://0x7f.0.0.1/callback https://example.0x1/callback ].each do |uri|
       client = Oauth::Client.new(name: "Unplain", redirect_uris: [ uri ], dynamically_registered: true)
 
       assert_not client.valid?, uri
@@ -76,7 +77,7 @@ class Oauth::ClientTest < ActiveSupport::TestCase
     end
 
     %w[ https://connector.example.com:8443/callback https://Connector.Example.com/callback http://[::1]:8888/callback http://localhost/callback
-        https://203.0.113.7/callback https://3com.example/callback ].each do |uri|
+        https://203.0.113.7/callback https://3com.example/callback https://10.0.0.1/callback ].each do |uri|
       assert Oauth::Client.new(name: "Plain", redirect_uris: [ uri ], dynamically_registered: true).valid?, uri
     end
   end

From 5d74b5309ec2fe6646709d8faa38fa3a31b28525 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Fri, 16 Oct 2026 23:56:50 -0700
Subject: [PATCH 15/15] Pin a too-long state, not a missing one, as a
 pre-consent error

#2296 now accepts a request without state, so the error left to pin
for a self-registered https host is a state past 2048 characters.
---
 test/integration/oauth_flow_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb
index 8f9c245df5..53a76489ce 100644
--- a/test/integration/oauth_flow_test.rb
+++ b/test/integration/oauth_flow_test.rb
@@ -114,7 +114,7 @@ class OauthFlowTest < ActionDispatch::IntegrationTest
 
     {
       { scope: "admin" } => "invalid_scope",
-      { state: nil } => "invalid_request",
+      { state: "s" * 2049 } => "invalid_request",
       { response_type: "token" } => "unsupported_response_type",
       { code_challenge_method: "plain" } => "invalid_request"
     }.each do |overrides, error|