improve api application redirect uri validation

[geoffg-sentry]

Perform better RFC validation of redirect URIs for our api applications for potential path traversal and encoding bypasses.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

• ✅ Fixed: Double-encoded null bytes and backslashes bypass security checks
  • Redirect URI path validation now uses the same full iterative decoding as normalization before checking for null bytes and backslashes, so double-encoded payloads are rejected.
• ✅ Fixed: Normalization breaks token exchange redirect_uri binding check
  • Token exchange now normalizes the incoming redirect_uri before comparing it to the stored grant redirect_uri, preserving binding when authorization stored a canonicalized URI.

Or push these changes by commenting:

@cursor push f56e4ff50a

Preview (f56e4ff50a)

diff --git a/src/sentry/models/apiapplication.py b/src/sentry/models/apiapplication.py
--- a/src/sentry/models/apiapplication.py
+++ b/src/sentry/models/apiapplication.py
@@ -161,6 +161,14 @@
             return False
         return self.version >= min_version
 
+    def _fully_decode_path(self, path: str) -> str:
+        prev = None
+        decoded = unquote(path)
+        while decoded != prev:
+            prev = decoded
+            decoded = unquote(decoded)
+        return decoded
+
     def normalize_url(self, value):
         parts = urlparse(value)
         path = parts.path
@@ -168,11 +176,7 @@
         # Fully decode all layers of percent-encoding to a stable form.
         # This prevents multi-layer encoding (e.g. %252e%252e) from hiding
         # path traversal sequences that posixpath.normpath needs to resolve.
-        prev = None
-        decoded = unquote(path)
-        while decoded != prev:
-            prev = decoded
-            decoded = unquote(decoded)
+        decoded = self._fully_decode_path(path)
 
         has_trailing_slash = decoded.endswith("/")
         normalized_path = posixpath.normpath(decoded)
@@ -191,13 +195,15 @@
 
         raw_path = urlparse(value).path
 
+        decoded_raw_path = self._fully_decode_path(raw_path)
+
         # Reject null bytes — can cause string truncation in downstream servers.
-        if "\x00" in unquote(raw_path):
+        if "\x00" in decoded_raw_path:
             return False
 
         # Reject backslashes — some servers/proxies interpret \ as /, enabling
         # path traversal that posixpath.normpath wouldn't catch.
-        if "\\" in unquote(raw_path):
+        if "\\" in decoded_raw_path:
             return False
 
         value = self.normalize_url(value)

diff --git a/src/sentry/models/apitoken.py b/src/sentry/models/apitoken.py
--- a/src/sentry/models/apitoken.py
+++ b/src/sentry/models/apitoken.py
@@ -418,10 +418,15 @@
                 # Validate redirect_uri binding (RFC 6749 §4.1.3)
                 # Only validate if redirect_uri was provided in the token request (not None)
                 # This maintains backward compatibility with direct from_grant() calls
+                normalized_redirect_uri = (
+                    grant.application.normalize_url(redirect_uri)
+                    if redirect_uri is not None and grant.redirect_uri
+                    else redirect_uri
+                )
                 if (
                     redirect_uri is not None
                     and grant.redirect_uri
-                    and grant.redirect_uri != redirect_uri
+                    and grant.redirect_uri != normalized_redirect_uri
                 ):
                     # RFC 6749 §10.5: Authorization codes are single-use and must be invalidated
                     # on failed exchange attempts to prevent authorization code replay attacks

diff --git a/tests/sentry/models/test_apiapplication.py b/tests/sentry/models/test_apiapplication.py
--- a/tests/sentry/models/test_apiapplication.py
+++ b/tests/sentry/models/test_apiapplication.py
@@ -209,6 +209,7 @@
         assert not app.is_valid_redirect_uri("http://example.com/callback/%00evil")
         assert not app.is_valid_redirect_uri("http://example.com/callback/\x00evil")
         assert not app.is_valid_redirect_uri("http://example.com/%00/callback/")
+        assert not app.is_valid_redirect_uri("http://example.com/callback/%2500evil")
 
     def test_is_valid_redirect_uri_null_byte_strict(self) -> None:
         """Null bytes must be rejected in strict mode too."""
@@ -230,6 +231,7 @@
 
         assert not app.is_valid_redirect_uri("http://example.com/callback/..\\..\\secret")
         assert not app.is_valid_redirect_uri("http://example.com/callback/%5c..%5c../secret")
+        assert not app.is_valid_redirect_uri("http://example.com/callback/%255c..%255c../secret")
 
     def test_is_valid_redirect_uri_deep_encoding(self) -> None:
         """Deeply nested percent-encoding (3+ layers) must be resolved and rejected."""

diff --git a/tests/sentry/web/frontend/test_oauth_token.py b/tests/sentry/web/frontend/test_oauth_token.py
--- a/tests/sentry/web/frontend/test_oauth_token.py
+++ b/tests/sentry/web/frontend/test_oauth_token.py
@@ -358,6 +358,28 @@
         assert resp.status_code == 400
         assert json.loads(resp.content) == {"error": "invalid_grant"}
 
+    def test_redirect_uri_matches_after_normalization(self) -> None:
+        self.login_as(self.user)
+        grant = ApiGrant.objects.create(
+            user=self.user,
+            application=self.application,
+            redirect_uri="https://example.com/",
+        )
+
+        resp = self.client.post(
+            self.path,
+            {
+                "grant_type": "authorization_code",
+                "redirect_uri": "https://example.com",
+                "code": grant.code,
+                "client_id": self.application.client_id,
+                "client_secret": self.client_secret,
+            },
+        )
+
+        assert resp.status_code == 200
+        assert "access_token" in json.loads(resp.content)
+
     def test_no_open_id_token(self) -> None:
         """
         Checks that the OIDC token is not returned unless the right scope is approved.

_{This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}