From 7e3f6c0e6391fb80670f0205226d36dbf99b85eb Mon Sep 17 00:00:00 2001
From: Bobby Ho <bobby@coder.com>
Date: Mon, 10 Aug 2026 12:51:45 -0700
Subject: [PATCH 1/8] fix(coderd/oauth2provider): reject PKCE code_verifier
 below RFC 7636 length floor
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The token endpoint accepted any non-empty code_verifier, so a client
could authenticate with a one-character verifier. RFC 7636 §4.1 sets a
43 to 128 character floor over the unreserved character set. The
challenge travels in the authorization request URL and the code
travels in the redirect, both of which land in browser history,
referrer headers, and proxy logs, so an attacker holding those
brute-forces the verifier offline at whatever entropy the client
chose, with no server-side rate limit. A one-character verifier is a
one-character password, and the server should refuse it rather than
accept whatever the client picked.

ValidPKCEVerifier enforces the length and charset bounds before the
existing S256 comparison runs. The existing TestOAuth2InvalidPKCE test
already exercises a 14-character verifier end to end and continues to
pass, now rejected on length rather than on hash mismatch.
---
 coderd/oauth2provider/pkce.go      | 34 +++++++++++++
 coderd/oauth2provider/pkce_test.go | 78 ++++++++++++++++++++++++++++++
 coderd/oauth2provider/tokens.go    |  5 +-
 3 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/coderd/oauth2provider/pkce.go b/coderd/oauth2provider/pkce.go
index fd759dff889..dcaaba82a56 100644
--- a/coderd/oauth2provider/pkce.go
+++ b/coderd/oauth2provider/pkce.go
@@ -6,6 +6,40 @@ import (
 	"encoding/base64"
 )
 
+// PKCE code verifier bounds from RFC 7636 §4.1.
+const (
+	pkceVerifierMinLength = 43
+	pkceVerifierMaxLength = 128
+)
+
+// ValidPKCEVerifier reports whether a code_verifier meets RFC 7636 §4.1: 43 to
+// 128 characters of the unreserved set [A-Za-z0-9-._~].
+//
+// The length floor is the whole point. PKCE is the only client authentication
+// some clients have, and the challenge travels in the authorization request
+// URL while the code travels in the redirect, both of which land in browser
+// history, referrer headers, and proxy logs. An attacker holding those
+// brute-forces the verifier offline at whatever entropy the client chose,
+// where no server-side rate limit applies. A client that sends a
+// one-character verifier has set a one-character password, and the server
+// should refuse it rather than accept whatever the client picked.
+func ValidPKCEVerifier(verifier string) bool {
+	if len(verifier) < pkceVerifierMinLength || len(verifier) > pkceVerifierMaxLength {
+		return false
+	}
+	for _, r := range verifier {
+		switch {
+		case r >= 'A' && r <= 'Z',
+			r >= 'a' && r <= 'z',
+			r >= '0' && r <= '9',
+			r == '-', r == '.', r == '_', r == '~':
+		default:
+			return false
+		}
+	}
+	return true
+}
+
 // VerifyPKCE verifies that the code_verifier matches the code_challenge
 // using the S256 method as specified in RFC 7636.
 func VerifyPKCE(challenge, verifier string) bool {
diff --git a/coderd/oauth2provider/pkce_test.go b/coderd/oauth2provider/pkce_test.go
index da0ff3a9d24..d3893748614 100644
--- a/coderd/oauth2provider/pkce_test.go
+++ b/coderd/oauth2provider/pkce_test.go
@@ -3,6 +3,7 @@ package oauth2provider_test
 import (
 	"crypto/sha256"
 	"encoding/base64"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -124,3 +125,80 @@ func TestValidatePKCECodeChallengeMethod(t *testing.T) {
 		})
 	}
 }
+
+func TestValidPKCEVerifier(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		verifier    string
+		expectValid bool
+	}{
+		{
+			name:        "Empty",
+			verifier:    "",
+			expectValid: false,
+		},
+		{
+			name:        "OneCharacter",
+			verifier:    "a",
+			expectValid: false,
+		},
+		{
+			name: "OneBelowMinLength",
+			// 42 characters, one short of the RFC 7636 §4.1 floor.
+			verifier:    strings.Repeat("a", 42),
+			expectValid: false,
+		},
+		{
+			name: "AtMinLength",
+			// 43 characters, the RFC 7636 §4.1 floor.
+			verifier:    strings.Repeat("a", 43),
+			expectValid: true,
+		},
+		{
+			name: "AtMaxLength",
+			// 128 characters, the RFC 7636 §4.1 ceiling.
+			verifier:    strings.Repeat("a", 128),
+			expectValid: true,
+		},
+		{
+			name: "OneAboveMaxLength",
+			// 129 characters, one past the RFC 7636 §4.1 ceiling.
+			verifier:    strings.Repeat("a", 129),
+			expectValid: false,
+		},
+		{
+			name:        "AllowedCharacters",
+			verifier:    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~",
+			expectValid: true,
+		},
+		{
+			name:        "PlusIsRejected",
+			verifier:    strings.Repeat("a", 42) + "+",
+			expectValid: false,
+		},
+		{
+			name:        "SlashIsRejected",
+			verifier:    strings.Repeat("a", 42) + "/",
+			expectValid: false,
+		},
+		{
+			name:        "EqualsIsRejected",
+			verifier:    strings.Repeat("a", 42) + "=",
+			expectValid: false,
+		},
+		{
+			name:        "SpaceIsRejected",
+			verifier:    strings.Repeat("a", 42) + " ",
+			expectValid: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			require.Equal(t, tt.expectValid, oauth2provider.ValidPKCEVerifier(tt.verifier))
+		})
+	}
+}
diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go
index 3761d1010ca..0a5f9838f41 100644
--- a/coderd/oauth2provider/tokens.go
+++ b/coderd/oauth2provider/tokens.go
@@ -280,7 +280,10 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
 	// PKCE is mandatory for all authorization code flows
 	// (OAuth 2.1). Verify the code verifier against the stored
 	// challenge.
-	if req.CodeVerifier == "" {
+	// Reject a verifier outside RFC 7636 §4.1's bounds before comparing it. A
+	// short verifier hashes to a valid-looking challenge, so the comparison
+	// below cannot tell a well-formed secret from a one-character one.
+	if !ValidPKCEVerifier(req.CodeVerifier) {
 		return codersdk.OAuth2TokenResponse{}, errInvalidPKCE
 	}
 	if !dbCode.CodeChallenge.Valid || dbCode.CodeChallenge.String == "" {

From a125238d71ce4c6b146383d29be0352b1e78b1a6 Mon Sep 17 00:00:00 2001
From: Bobby Ho <bobby@coder.com>
Date: Mon, 10 Aug 2026 14:25:38 -0700
Subject: [PATCH 2/8] fix(scripts/oauth2): generate PKCE verifiers at the RFC
 7636 floor length

tr -d "=+/" deleted every '+' and '/' character that happened to appear
in the base64 output instead of translating them to the URL-safe
alphabet, so cut -c -43 truncated a string that was often already
short. Roughly 70% of runs produced a verifier below the 43-character
floor coderd/oauth2provider now enforces (#28003), so the manual and
scripted OAuth2 flows these scripts drive failed token exchange
intermittently.

Use tr '+/' '-_' | tr -d '=' instead: translating first and then
stripping the single padding character is deterministic, since 32
random bytes always base64-encode to a fixed length. This always
yields exactly 43 characters, so the cut is no longer needed.
---
 scripts/oauth2/generate-pkce.sh    | 2 +-
 scripts/oauth2/test-manual-flow.sh | 2 +-
 scripts/oauth2/test-mcp-oauth2.sh  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/oauth2/generate-pkce.sh b/scripts/oauth2/generate-pkce.sh
index cb94120d569..a0a96cb450a 100755
--- a/scripts/oauth2/generate-pkce.sh
+++ b/scripts/oauth2/generate-pkce.sh
@@ -4,7 +4,7 @@
 # Usage: ./generate-pkce.sh
 
 # Generate code verifier (43-128 characters, URL-safe)
-CODE_VERIFIER=$(openssl rand -base64 32 | tr -d "=+/" | cut -c -43)
+CODE_VERIFIER=$(openssl rand -base64 32 | tr '+/' '-_' | tr -d '=')
 
 # Generate code challenge (S256 method)
 CODE_CHALLENGE=$(echo -n "$CODE_VERIFIER" | openssl dgst -sha256 -binary | base64 | tr -d "=" | tr '+/' '-_')
diff --git a/scripts/oauth2/test-manual-flow.sh b/scripts/oauth2/test-manual-flow.sh
index 734c3a9c5e0..73dd9de09d6 100755
--- a/scripts/oauth2/test-manual-flow.sh
+++ b/scripts/oauth2/test-manual-flow.sh
@@ -39,7 +39,7 @@ if ! command -v go &>/dev/null; then
 fi
 
 # Generate PKCE parameters
-CODE_VERIFIER=$(openssl rand -base64 32 | tr -d "=+/" | cut -c -43)
+CODE_VERIFIER=$(openssl rand -base64 32 | tr '+/' '-_' | tr -d '=')
 export CODE_VERIFIER
 CODE_CHALLENGE=$(echo -n "$CODE_VERIFIER" | openssl dgst -sha256 -binary | base64 | tr -d "=" | tr '+/' '-_')
 export CODE_CHALLENGE
diff --git a/scripts/oauth2/test-mcp-oauth2.sh b/scripts/oauth2/test-mcp-oauth2.sh
index 9139010f6a3..746faac69b9 100755
--- a/scripts/oauth2/test-mcp-oauth2.sh
+++ b/scripts/oauth2/test-mcp-oauth2.sh
@@ -66,7 +66,7 @@ echo -e "${GREEN}✓ Created client secret${NC}\n"
 
 # Test 2: PKCE Flow
 echo -e "${YELLOW}Test 2: PKCE Flow${NC}"
-CODE_VERIFIER=$(openssl rand -base64 32 | tr -d "=+/" | cut -c -43)
+CODE_VERIFIER=$(openssl rand -base64 32 | tr '+/' '-_' | tr -d '=')
 CODE_CHALLENGE=$(echo -n "$CODE_VERIFIER" | openssl dgst -sha256 -binary | base64 | tr -d "=" | tr '+/' '-_')
 STATE=$(openssl rand -hex 16)
 
@@ -132,7 +132,7 @@ fi
 echo -e "${YELLOW}Test 4: Resource Parameter Support${NC}"
 RESOURCE="https://api.example.com"
 STATE=$(openssl rand -hex 16)
-RESOURCE_CODE_VERIFIER=$(openssl rand -base64 32 | tr -d "=+/" | cut -c -43)
+RESOURCE_CODE_VERIFIER=$(openssl rand -base64 32 | tr '+/' '-_' | tr -d '=')
 RESOURCE_CODE_CHALLENGE=$(echo -n "$RESOURCE_CODE_VERIFIER" | openssl dgst -sha256 -binary | base64 | tr -d "=" | tr '+/' '-_')
 RESOURCE_AUTH_URL="$BASE_URL/oauth2/authorize?client_id=$CLIENT_ID&response_type=code&redirect_uri=http://localhost:9876/callback&state=$STATE&resource=$RESOURCE&code_challenge=$RESOURCE_CODE_CHALLENGE&code_challenge_method=S256"
 

From eea4094037bab1328c0eeabc812fae39e524751b Mon Sep 17 00:00:00 2001
From: Bobby Ho <bobby@coder.com>
Date: Mon, 10 Aug 2026 14:49:10 -0700
Subject: [PATCH 3/8] fix(coderd/oauth2provider): validate code_challenge
 format at authorize
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

extractAuthorizeParams only checked code_challenge for non-emptiness, so
a malformed value (wrong length, disallowed characters, an arbitrarily
large blob) was persisted verbatim and only surfaced as a failure at
token exchange, with an error that misleadingly names code_verifier
instead of the parameter that was actually invalid.

RFC 7636 gives code_verifier and code_challenge the same ABNF, so reuse
the existing bounds check rather than adding a second one: rename
ValidPKCEVerifier to ValidPKCEFormat and validate code_challenge against
it in extractAuthorizeParams, rejecting a malformed value with
invalid_request at the authorization request per RFC 7636 §4.4.1.

TestExtractAuthorizeParams_Scopes used a 14-character placeholder
code_challenge that the new check now correctly rejects; lengthened it
to a valid value since that test only exercises scope parsing.
---
 coderd/oauth2provider/authorize.go            | 24 ++++--
 .../oauth2providertest/oauth2_test.go         | 33 +++++++++
 coderd/oauth2provider/pkce.go                 | 18 +++--
 coderd/oauth2provider/pkce_test.go            |  4 +-
 coderd/oauth2provider/tokens.go               |  2 +-
 coderd/oauth2provider/tokens_internal_test.go | 73 ++++++++++++++++++-
 6 files changed, 138 insertions(+), 16 deletions(-)

diff --git a/coderd/oauth2provider/authorize.go b/coderd/oauth2provider/authorize.go
index 1480259c1fa..8b0e9cceecb 100644
--- a/coderd/oauth2provider/authorize.go
+++ b/coderd/oauth2provider/authorize.go
@@ -54,12 +54,24 @@ func extractAuthorizeParams(r *http.Request, callbackURL *url.URL) (authorizePar
 		codeChallengeMethod: p.String(vals, "", "code_challenge_method"),
 	}
 
-	// PKCE is required for authorization code flow requests.
-	if params.responseType == codersdk.OAuth2ProviderResponseTypeCode && params.codeChallenge == "" {
-		p.Errors = append(p.Errors, codersdk.ValidationError{
-			Field:  "code_challenge",
-			Detail: `Query param "code_challenge" is required and cannot be empty`,
-		})
+	// PKCE is required for authorization code flow requests. Reject a
+	// malformed code_challenge here (RFC 7636 §4.4.1) rather than storing it
+	// verbatim and failing later at token exchange, where the error would
+	// point at the code_verifier instead of the parameter that was actually
+	// invalid.
+	if params.responseType == codersdk.OAuth2ProviderResponseTypeCode {
+		switch {
+		case params.codeChallenge == "":
+			p.Errors = append(p.Errors, codersdk.ValidationError{
+				Field:  "code_challenge",
+				Detail: `Query param "code_challenge" is required and cannot be empty`,
+			})
+		case !ValidPKCEFormat(params.codeChallenge):
+			p.Errors = append(p.Errors, codersdk.ValidationError{
+				Field:  "code_challenge",
+				Detail: "must be 43 to 128 characters from the unreserved character set [A-Za-z0-9-._~]",
+			})
+		}
 	}
 
 	// Validate resource indicator syntax (RFC 8707): must be absolute URI without fragment
diff --git a/coderd/oauth2provider/oauth2providertest/oauth2_test.go b/coderd/oauth2provider/oauth2providertest/oauth2_test.go
index 22d8ac05341..c5411473d31 100644
--- a/coderd/oauth2provider/oauth2providertest/oauth2_test.go
+++ b/coderd/oauth2provider/oauth2providertest/oauth2_test.go
@@ -189,6 +189,39 @@ func TestOAuth2WithoutPKCEIsRejected(t *testing.T) {
 	)
 }
 
+// TestOAuth2MalformedCodeChallengeIsRejected verifies that a code_challenge
+// below the RFC 7636 §4.1 length floor is rejected at the authorization
+// request, rather than being stored and only failing once a client attempts
+// to exchange the resulting code.
+func TestOAuth2MalformedCodeChallengeIsRejected(t *testing.T) {
+	t.Parallel()
+
+	client := coderdtest.New(t, &coderdtest.Options{
+		IncludeProvisionerDaemon: false,
+	})
+	_ = coderdtest.CreateFirstUser(t, client)
+
+	app, _ := oauth2providertest.CreateTestOAuth2App(t, client)
+	t.Cleanup(func() {
+		oauth2providertest.CleanupOAuth2App(t, client, app.ID)
+	})
+
+	state := oauth2providertest.GenerateState(t)
+
+	authParams := oauth2providertest.AuthorizeParams{
+		ClientID:            app.ID.String(),
+		ResponseType:        "code",
+		RedirectURI:         oauth2providertest.TestRedirectURI,
+		State:               state,
+		CodeChallenge:       "too-short",
+		CodeChallengeMethod: "S256",
+	}
+
+	oauth2providertest.AuthorizeOAuth2AppExpectingError(
+		t, client, client.URL.String(), authParams, http.StatusBadRequest,
+	)
+}
+
 func TestOAuth2TokenExchangeClientSecretBasic(t *testing.T) {
 	t.Parallel()
 
diff --git a/coderd/oauth2provider/pkce.go b/coderd/oauth2provider/pkce.go
index dcaaba82a56..c52e58f31f7 100644
--- a/coderd/oauth2provider/pkce.go
+++ b/coderd/oauth2provider/pkce.go
@@ -12,8 +12,11 @@ const (
 	pkceVerifierMaxLength = 128
 )
 
-// ValidPKCEVerifier reports whether a code_verifier meets RFC 7636 §4.1: 43 to
-// 128 characters of the unreserved set [A-Za-z0-9-._~].
+// ValidPKCEFormat reports whether s meets RFC 7636 §4.1: 43 to 128 characters
+// of the unreserved set [A-Za-z0-9-._~]. RFC 7636 gives code_verifier and
+// code_challenge the same ABNF, so this check applies to both: a code_verifier
+// directly, and a code_challenge because the S256 method that produces it
+// (base64url(SHA256(verifier))) always yields a string within these bounds.
 //
 // The length floor is the whole point. PKCE is the only client authentication
 // some clients have, and the challenge travels in the authorization request
@@ -22,12 +25,15 @@ const (
 // brute-forces the verifier offline at whatever entropy the client chose,
 // where no server-side rate limit applies. A client that sends a
 // one-character verifier has set a one-character password, and the server
-// should refuse it rather than accept whatever the client picked.
-func ValidPKCEVerifier(verifier string) bool {
-	if len(verifier) < pkceVerifierMinLength || len(verifier) > pkceVerifierMaxLength {
+// should refuse it rather than accept whatever the client picked. The same
+// bound on code_challenge keeps a malformed value from being persisted
+// verbatim and failing late, at token exchange, instead of at the
+// authorization request where RFC 7636 §4.4.1 expects it to be rejected.
+func ValidPKCEFormat(s string) bool {
+	if len(s) < pkceVerifierMinLength || len(s) > pkceVerifierMaxLength {
 		return false
 	}
-	for _, r := range verifier {
+	for _, r := range s {
 		switch {
 		case r >= 'A' && r <= 'Z',
 			r >= 'a' && r <= 'z',
diff --git a/coderd/oauth2provider/pkce_test.go b/coderd/oauth2provider/pkce_test.go
index d3893748614..07ec53d8ead 100644
--- a/coderd/oauth2provider/pkce_test.go
+++ b/coderd/oauth2provider/pkce_test.go
@@ -126,7 +126,7 @@ func TestValidatePKCECodeChallengeMethod(t *testing.T) {
 	}
 }
 
-func TestValidPKCEVerifier(t *testing.T) {
+func TestValidPKCEFormat(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
@@ -198,7 +198,7 @@ func TestValidPKCEVerifier(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			require.Equal(t, tt.expectValid, oauth2provider.ValidPKCEVerifier(tt.verifier))
+			require.Equal(t, tt.expectValid, oauth2provider.ValidPKCEFormat(tt.verifier))
 		})
 	}
 }
diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go
index 0a5f9838f41..b201156f885 100644
--- a/coderd/oauth2provider/tokens.go
+++ b/coderd/oauth2provider/tokens.go
@@ -283,7 +283,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
 	// Reject a verifier outside RFC 7636 §4.1's bounds before comparing it. A
 	// short verifier hashes to a valid-looking challenge, so the comparison
 	// below cannot tell a well-formed secret from a one-character one.
-	if !ValidPKCEVerifier(req.CodeVerifier) {
+	if !ValidPKCEFormat(req.CodeVerifier) {
 		return codersdk.OAuth2TokenResponse{}, errInvalidPKCE
 	}
 	if !dbCode.CodeChallenge.Valid || dbCode.CodeChallenge.String == "" {
diff --git a/coderd/oauth2provider/tokens_internal_test.go b/coderd/oauth2provider/tokens_internal_test.go
index 7f25b68827c..59ec2192cc9 100644
--- a/coderd/oauth2provider/tokens_internal_test.go
+++ b/coderd/oauth2provider/tokens_internal_test.go
@@ -318,7 +318,10 @@ func TestExtractAuthorizeParams_Scopes(t *testing.T) {
 			query.Set("response_type", "code")
 			query.Set("client_id", "test-client")
 			query.Set("redirect_uri", "http://localhost:3000/callback")
-			query.Set("code_challenge", "test-challenge")
+			// This test only exercises scope parsing, but code_challenge is
+			// still required for response_type=code and must satisfy the
+			// RFC 7636 §4.1 length floor, so use a valid-length value.
+			query.Set("code_challenge", strings.Repeat("a", 43))
 			if tc.scopeParam != "" {
 				query.Set("scope", tc.scopeParam)
 			}
@@ -342,6 +345,74 @@ func TestExtractAuthorizeParams_Scopes(t *testing.T) {
 	}
 }
 
+// TestExtractAuthorizeParams_CodeChallengeFormat ensures a code_challenge is
+// rejected at the authorization request (RFC 7636 §4.4.1) when it does not
+// meet the same length and character bounds as a code_verifier, rather than
+// being stored and failing later at token exchange.
+func TestExtractAuthorizeParams_CodeChallengeFormat(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name          string
+		codeChallenge string
+		expectValid   bool
+	}{
+		{
+			name:          "ValidLength",
+			codeChallenge: strings.Repeat("a", 43),
+			expectValid:   true,
+		},
+		{
+			name:          "TooShort",
+			codeChallenge: strings.Repeat("a", 42),
+			expectValid:   false,
+		},
+		{
+			name:          "TooLong",
+			codeChallenge: strings.Repeat("a", 129),
+			expectValid:   false,
+		},
+		{
+			name:          "DisallowedCharacter",
+			codeChallenge: strings.Repeat("a", 42) + "+",
+			expectValid:   false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			callbackURL, err := url.Parse("http://localhost:3000/callback")
+			require.NoError(t, err)
+
+			query := url.Values{}
+			query.Set("response_type", "code")
+			query.Set("client_id", "test-client")
+			query.Set("redirect_uri", "http://localhost:3000/callback")
+			query.Set("code_challenge", tc.codeChallenge)
+
+			reqURL, err := url.Parse("http://localhost:8080/oauth2/authorize?" + query.Encode())
+			require.NoError(t, err)
+
+			req := &http.Request{
+				Method: http.MethodGet,
+				URL:    reqURL,
+			}
+
+			_, validationErrs, err := extractAuthorizeParams(req, callbackURL)
+			if tc.expectValid {
+				require.NoError(t, err)
+				require.Empty(t, validationErrs)
+			} else {
+				require.Error(t, err)
+				require.Len(t, validationErrs, 1)
+				require.Equal(t, "code_challenge", validationErrs[0].Field)
+			}
+		})
+	}
+}
+
 // TestExtractAuthorizeParams_TokenResponseTypeDoesNotRequirePKCE ensures
 // response_type=token is parsed without requiring PKCE fields so callers can
 // return unsupported_response_type instead of invalid_request.

From e7d78d5d446e95a86a91dc895104bf9de6eff37b Mon Sep 17 00:00:00 2001
From: Bobby Ho <bobby@coder.com>
Date: Mon, 10 Aug 2026 16:50:43 -0700
Subject: [PATCH 4/8] fix(coderd/oauth2provider): return invalid_request for
 malformed code_verifier
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A malformed code_verifier (wrong length or disallowed characters) and a
well-formed verifier that simply fails the PKCE hash comparison both
returned the same error: invalid_grant, "The PKCE code verifier is
invalid." A client that sent a too-short verifier had no way to tell
that apart from a genuine hash mismatch, would re-check its SHA-256
computation, find nothing wrong, and retry the same bad verifier
indefinitely since invalid_grant conventionally signals "retry."

RFC 6749 §5.2 assigns a malformed parameter to invalid_request; RFC 7636
§4.6 reserves invalid_grant for the comparison failure specifically. Move
the code_verifier format check out of authorizationCodeGrant and into
extractTokenRequest, which already owns syntax validation for this grant
type, so the two failure modes return distinct, spec-accurate errors.

Several existing tests sent an empty or placeholder code_verifier
incidental to what they were actually testing (client_secret
requirements, scope parsing, malformed-code handling); updated them to
use a valid-length value so they still reach the behavior under test.
---
 coderd/oauth2_test.go                         |  5 +++
 coderd/oauth2provider/tokens.go               | 38 ++++++++++++++-----
 coderd/oauth2provider/tokens_internal_test.go | 15 ++++++--
 3 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/coderd/oauth2_test.go b/coderd/oauth2_test.go
index 3a8d5917fda..d2a78bc225c 100644
--- a/coderd/oauth2_test.go
+++ b/coderd/oauth2_test.go
@@ -498,6 +498,11 @@ func TestOAuth2ProviderTokenExchange(t *testing.T) {
 			var verifier string
 			if test.defaultCode != nil {
 				code = *test.defaultCode
+				// These subtests exercise malformed/expired code_
+				// handling; the code lookup fails before code_verifier is
+				// ever compared, but it still has to satisfy RFC 7636
+				// §4.1's format floor to reach that point.
+				verifier = strings.Repeat("a", 43)
 			} else {
 				var err error
 				code, verifier, err = authorizationFlow(ctx, userClient, valid)
diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go
index b201156f885..0a31ea19276 100644
--- a/coderd/oauth2provider/tokens.go
+++ b/coderd/oauth2provider/tokens.go
@@ -97,6 +97,17 @@ func extractTokenRequest(r *http.Request, callbackURL *url.URL) (codersdk.OAuth2
 				Detail: "Parameter \"client_secret\" is required and cannot be empty",
 			})
 		}
+		// A code_verifier outside RFC 7636 §4.1's bounds is a syntax error
+		// (RFC 6749 §5.2), distinct from a well-formed verifier that fails the
+		// PKCE hash comparison in authorizationCodeGrant, which RFC 7636 §4.6
+		// maps to invalid_grant instead. Checking it here, alongside the other
+		// syntax validation, keeps the two failure modes distinguishable.
+		if !ValidPKCEFormat(req.CodeVerifier) {
+			p.Errors = append(p.Errors, codersdk.ValidationError{
+				Field:  "code_verifier",
+				Detail: "must be 43 to 128 characters from the unreserved character set [A-Za-z0-9-._~] (RFC 7636 §4.1)",
+			})
+		}
 	}
 
 	// Validate redirect URI - errors are added to p.Errors.
@@ -158,6 +169,18 @@ func Tokens(db database.Store, lifetimes codersdk.SessionLifetime) http.HandlerF
 					return
 				}
 			}
+
+			// A malformed code_verifier gets its own message so a client that
+			// sent a well-formed but wrong verifier (rejected later, as
+			// invalid_grant, by the PKCE hash comparison) can tell the two
+			// failures apart instead of retrying the same bad verifier forever.
+			if slices.ContainsFunc(validationErrs, func(validationError codersdk.ValidationError) bool {
+				return validationError.Field == "code_verifier"
+			}) {
+				httpapi.WriteOAuth2Error(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, "The code_verifier parameter must be 43 to 128 characters from the unreserved character set [A-Za-z0-9-._~] (RFC 7636 §4.1)")
+				return
+			}
+
 			// Generic invalid request for other validation errors
 			httpapi.WriteOAuth2Error(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, "The request is missing required parameters or is otherwise malformed")
 			return
@@ -277,15 +300,12 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
 		}
 	}
 
-	// PKCE is mandatory for all authorization code flows
-	// (OAuth 2.1). Verify the code verifier against the stored
-	// challenge.
-	// Reject a verifier outside RFC 7636 §4.1's bounds before comparing it. A
-	// short verifier hashes to a valid-looking challenge, so the comparison
-	// below cannot tell a well-formed secret from a one-character one.
-	if !ValidPKCEFormat(req.CodeVerifier) {
-		return codersdk.OAuth2TokenResponse{}, errInvalidPKCE
-	}
+	// PKCE is mandatory for all authorization code flows (OAuth 2.1). Verify
+	// the code verifier against the stored challenge. extractTokenRequest
+	// already rejected a malformed verifier as invalid_request, so
+	// req.CodeVerifier is guaranteed to meet RFC 7636 §4.1's bounds here; a
+	// mismatch below is a wrong-but-well-formed verifier, RFC 7636 §4.6's
+	// invalid_grant case.
 	if !dbCode.CodeChallenge.Valid || dbCode.CodeChallenge.String == "" {
 		// Code was issued without a challenge — should not happen
 		// with authorize endpoint enforcement, but defend in depth.
diff --git a/coderd/oauth2provider/tokens_internal_test.go b/coderd/oauth2provider/tokens_internal_test.go
index 59ec2192cc9..fc2148353cf 100644
--- a/coderd/oauth2provider/tokens_internal_test.go
+++ b/coderd/oauth2provider/tokens_internal_test.go
@@ -110,6 +110,10 @@ func TestExtractTokenParams_Scopes(t *testing.T) {
 			form.Set("client_id", "test-client")
 			form.Set("client_secret", "test-secret")
 			form.Set("code", "test-code")
+			// This test only exercises scope parsing, but code_verifier is
+			// validated unconditionally for this grant type, so use a value
+			// that satisfies the RFC 7636 §4.1 length floor.
+			form.Set("code_verifier", strings.Repeat("a", 43))
 			if tc.scopeParam != "" {
 				form.Set("scope", tc.scopeParam)
 			}
@@ -147,22 +151,22 @@ func TestExtractTokenParams_ScopesURLEncoded(t *testing.T) {
 	}{
 		{
 			name:           "PlusEncodedSpaces",
-			rawQuery:       "grant_type=authorization_code&client_id=test&client_secret=secret&code=code&scope=scope1+scope2+scope3",
+			rawQuery:       "grant_type=authorization_code&client_id=test&client_secret=secret&code=code&code_verifier=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa&scope=scope1+scope2+scope3",
 			expectedScopes: []string{"scope1", "scope2", "scope3"},
 		},
 		{
 			name:           "PercentEncodedSpaces",
-			rawQuery:       "grant_type=authorization_code&client_id=test&client_secret=secret&code=code&scope=scope1%20scope2%20scope3",
+			rawQuery:       "grant_type=authorization_code&client_id=test&client_secret=secret&code=code&code_verifier=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa&scope=scope1%20scope2%20scope3",
 			expectedScopes: []string{"scope1", "scope2", "scope3"},
 		},
 		{
 			name:           "MixedEncoding",
-			rawQuery:       "grant_type=authorization_code&client_id=test&client_secret=secret&code=code&scope=scope1+scope2%20scope3",
+			rawQuery:       "grant_type=authorization_code&client_id=test&client_secret=secret&code=code&code_verifier=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa&scope=scope1+scope2%20scope3",
 			expectedScopes: []string{"scope1", "scope2", "scope3"},
 		},
 		{
 			name:           "ColonEncodedInScope",
-			rawQuery:       "grant_type=authorization_code&client_id=test&client_secret=secret&code=code&scope=coder%3Aworkspace.create+coder%3Aworkspace.operate",
+			rawQuery:       "grant_type=authorization_code&client_id=test&client_secret=secret&code=code&code_verifier=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa&scope=coder%3Aworkspace.create+coder%3Aworkspace.operate",
 			expectedScopes: []string{"coder:workspace.create", "coder:workspace.operate"},
 		},
 	}
@@ -216,6 +220,7 @@ func TestExtractTokenParams_ScopesEdgeCases(t *testing.T) {
 				form.Set("client_id", "test-client")
 				form.Set("client_secret", "test-secret")
 				form.Set("code", "test-code")
+				form.Set("code_verifier", strings.Repeat("a", 43))
 				return form
 			},
 			expectedScopes: []string{},
@@ -229,6 +234,7 @@ func TestExtractTokenParams_ScopesEdgeCases(t *testing.T) {
 				form.Set("client_id", "test-client")
 				form.Set("client_secret", "test-secret")
 				form.Set("code", "test-code")
+				form.Set("code_verifier", strings.Repeat("a", 43))
 				form.Set("scope", "   ")
 				return form
 			},
@@ -244,6 +250,7 @@ func TestExtractTokenParams_ScopesEdgeCases(t *testing.T) {
 				form.Set("client_id", "test-client")
 				form.Set("client_secret", "test-secret")
 				form.Set("code", "test-code")
+				form.Set("code_verifier", strings.Repeat("a", 43))
 				form.Set("scope", longScope)
 				return form
 			},

From fb7e90b1b9f103bd134382497e0d538a382c804a Mon Sep 17 00:00:00 2001
From: Bobby Ho <bobby@coder.com>
Date: Mon, 10 Aug 2026 17:31:15 -0700
Subject: [PATCH 5/8] fix(coderd/oauth2provider/oauth2providertest): restore
 e2e coverage of PKCE hash mismatch

InvalidCodeVerifier ("wrong-verifier", 14 chars) was rejected on length
before VerifyPKCE ever ran, so no test exercised the token endpoint's
hash-comparison branch end to end; TestVerifyPKCE unit-tests the
function, but nothing proved the endpoint still calls it.

Lengthen InvalidCodeVerifier to a well-formed but wrong 43-character
value so it again reaches the hash comparison. Add MalformedCodeVerifier
and a new test asserting the length-rejection path returns
invalid_request, now that the previous commit gives it a distinct error
from the hash-mismatch invalid_grant case.
---
 .../oauth2providertest/fixtures.go            | 12 ++++-
 .../oauth2providertest/oauth2_test.go         | 46 +++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/coderd/oauth2provider/oauth2providertest/fixtures.go b/coderd/oauth2provider/oauth2providertest/fixtures.go
index 8dbccb511a3..df9de772beb 100644
--- a/coderd/oauth2provider/oauth2providertest/fixtures.go
+++ b/coderd/oauth2provider/oauth2providertest/fixtures.go
@@ -13,8 +13,16 @@ const (
 	// TestResourceURI is used for testing resource parameter
 	TestResourceURI = "https://api.example.com"
 
-	// Invalid PKCE verifier for negative testing
-	InvalidCodeVerifier = "wrong-verifier"
+	// InvalidCodeVerifier is well-formed (43 characters, RFC 7636 §4.1's
+	// unreserved set) but does not hash to any issued challenge, so it
+	// exercises the PKCE comparison failure (invalid_grant) rather than the
+	// length/charset check (invalid_request).
+	InvalidCodeVerifier = "wrong-verifier-that-is-well-formed-43-chars"
+
+	// MalformedCodeVerifier is below RFC 7636 §4.1's 43-character floor, so
+	// it exercises the length/charset check (invalid_request) instead of the
+	// PKCE comparison.
+	MalformedCodeVerifier = "too-short"
 )
 
 // OAuth2ErrorTypes contains standard OAuth2 error codes
diff --git a/coderd/oauth2provider/oauth2providertest/oauth2_test.go b/coderd/oauth2provider/oauth2providertest/oauth2_test.go
index c5411473d31..86741ae06d7 100644
--- a/coderd/oauth2provider/oauth2providertest/oauth2_test.go
+++ b/coderd/oauth2provider/oauth2providertest/oauth2_test.go
@@ -158,6 +158,52 @@ func TestOAuth2InvalidPKCE(t *testing.T) {
 	)
 }
 
+// TestOAuth2MalformedCodeVerifierIsRejected verifies that a code_verifier
+// below the RFC 7636 §4.1 length floor is rejected as invalid_request,
+// distinct from a well-formed verifier that fails the PKCE hash comparison
+// (invalid_grant, covered by TestOAuth2InvalidPKCE).
+func TestOAuth2MalformedCodeVerifierIsRejected(t *testing.T) {
+	t.Parallel()
+
+	client := coderdtest.New(t, &coderdtest.Options{
+		IncludeProvisionerDaemon: false,
+	})
+	_ = coderdtest.CreateFirstUser(t, client)
+
+	app, clientSecret := oauth2providertest.CreateTestOAuth2App(t, client)
+	t.Cleanup(func() {
+		oauth2providertest.CleanupOAuth2App(t, client, app.ID)
+	})
+
+	_, codeChallenge := oauth2providertest.GeneratePKCE(t)
+	state := oauth2providertest.GenerateState(t)
+
+	authParams := oauth2providertest.AuthorizeParams{
+		ClientID:            app.ID.String(),
+		ResponseType:        "code",
+		RedirectURI:         oauth2providertest.TestRedirectURI,
+		State:               state,
+		CodeChallenge:       codeChallenge,
+		CodeChallengeMethod: "S256",
+	}
+
+	code := oauth2providertest.AuthorizeOAuth2App(t, client, client.URL.String(), authParams)
+	require.NotEmpty(t, code, "should receive authorization code")
+
+	tokenParams := oauth2providertest.TokenExchangeParams{
+		GrantType:    "authorization_code",
+		Code:         code,
+		ClientID:     app.ID.String(),
+		ClientSecret: clientSecret,
+		CodeVerifier: oauth2providertest.MalformedCodeVerifier,
+		RedirectURI:  oauth2providertest.TestRedirectURI,
+	}
+
+	oauth2providertest.PerformTokenExchangeExpectingError(
+		t, client.URL.String(), tokenParams, oauth2providertest.OAuth2ErrorTypes.InvalidRequest,
+	)
+}
+
 // TestOAuth2WithoutPKCEIsRejected verifies that authorization requests without
 // a code_challenge are rejected now that PKCE is mandatory.
 func TestOAuth2WithoutPKCEIsRejected(t *testing.T) {

From 4fe0b1bfc4a5d4a050311bf1dded16347e3c5164 Mon Sep 17 00:00:00 2001
From: Bobby Ho <bobby@coder.com>
Date: Mon, 10 Aug 2026 20:29:09 -0700
Subject: [PATCH 6/8] fix(coderd/oauth2provider): revoke authorization code on
 PKCE failure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The code was deleted only inside the success-path transaction, so every
PKCE rejection (errInvalidPKCE) left it live in the database. RFC 6749
§10.5 requires authorization codes to be single-use; without that, an
attacker holding a leaked code (the exact threat PKCE defends against,
since codes and challenges land in browser history, referrer headers, and
proxy logs) could retry the token endpoint with different code_verifier
guesses for the entire 10-minute code lifetime, unthrottled. The
43-character length floor bounds guess format, not entropy.

Add revokeOAuth2CodeOnPKCEFailure, called from both PKCE rejection paths
in authorizationCodeGrant. It deletes the code using the same system
authz context already used for reads in this function; a deletion
failure is noted on the request's log line rather than changing the
response, since surfacing it as a different error would let a caller
distinguish delete success from failure, itself a new oracle.

Added TestOAuth2PKCEFailureConsumesCode to verify the code is
unredeemable, even with the correct verifier, once a PKCE mismatch has
occurred.
---
 .../oauth2providertest/oauth2_test.go         | 61 +++++++++++++++++++
 coderd/oauth2provider/tokens.go               | 25 ++++++++
 2 files changed, 86 insertions(+)

diff --git a/coderd/oauth2provider/oauth2providertest/oauth2_test.go b/coderd/oauth2provider/oauth2providertest/oauth2_test.go
index 86741ae06d7..b7d5649406d 100644
--- a/coderd/oauth2provider/oauth2providertest/oauth2_test.go
+++ b/coderd/oauth2provider/oauth2providertest/oauth2_test.go
@@ -158,6 +158,67 @@ func TestOAuth2InvalidPKCE(t *testing.T) {
 	)
 }
 
+// TestOAuth2PKCEFailureConsumesCode verifies that a code_verifier that fails
+// the PKCE hash comparison consumes the authorization code (RFC 6749 §10.5:
+// codes are single-use). Without this, a leaked code could be replayed with
+// unlimited further code_verifier guesses for the rest of its lifetime.
+func TestOAuth2PKCEFailureConsumesCode(t *testing.T) {
+	t.Parallel()
+
+	client := coderdtest.New(t, &coderdtest.Options{
+		IncludeProvisionerDaemon: false,
+	})
+	_ = coderdtest.CreateFirstUser(t, client)
+
+	app, clientSecret := oauth2providertest.CreateTestOAuth2App(t, client)
+	t.Cleanup(func() {
+		oauth2providertest.CleanupOAuth2App(t, client, app.ID)
+	})
+
+	codeVerifier, codeChallenge := oauth2providertest.GeneratePKCE(t)
+	state := oauth2providertest.GenerateState(t)
+
+	authParams := oauth2providertest.AuthorizeParams{
+		ClientID:            app.ID.String(),
+		ResponseType:        "code",
+		RedirectURI:         oauth2providertest.TestRedirectURI,
+		State:               state,
+		CodeChallenge:       codeChallenge,
+		CodeChallengeMethod: "S256",
+	}
+
+	code := oauth2providertest.AuthorizeOAuth2App(t, client, client.URL.String(), authParams)
+	require.NotEmpty(t, code, "should receive authorization code")
+
+	// Attempt the exchange with a well-formed but wrong verifier. This fails
+	// the PKCE hash comparison (invalid_grant) and must consume the code.
+	failedParams := oauth2providertest.TokenExchangeParams{
+		GrantType:    "authorization_code",
+		Code:         code,
+		ClientID:     app.ID.String(),
+		ClientSecret: clientSecret,
+		CodeVerifier: oauth2providertest.InvalidCodeVerifier,
+		RedirectURI:  oauth2providertest.TestRedirectURI,
+	}
+	oauth2providertest.PerformTokenExchangeExpectingError(
+		t, client.URL.String(), failedParams, oauth2providertest.OAuth2ErrorTypes.InvalidGrant,
+	)
+
+	// The correct verifier can no longer redeem the code: the failed PKCE
+	// comparison above already consumed it.
+	retryParams := oauth2providertest.TokenExchangeParams{
+		GrantType:    "authorization_code",
+		Code:         code,
+		ClientID:     app.ID.String(),
+		ClientSecret: clientSecret,
+		CodeVerifier: codeVerifier,
+		RedirectURI:  oauth2providertest.TestRedirectURI,
+	}
+	oauth2providertest.PerformTokenExchangeExpectingError(
+		t, client.URL.String(), retryParams, oauth2providertest.OAuth2ErrorTypes.InvalidGrant,
+	)
+}
+
 // TestOAuth2MalformedCodeVerifierIsRejected verifies that a code_verifier
 // below the RFC 7636 §4.1 length floor is rejected as invalid_request,
 // distinct from a well-formed verifier that fails the PKCE hash comparison
diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go
index 0a31ea19276..6c359128dba 100644
--- a/coderd/oauth2provider/tokens.go
+++ b/coderd/oauth2provider/tokens.go
@@ -13,12 +13,14 @@ import (
 	"github.com/google/uuid"
 	"golang.org/x/xerrors"
 
+	"cdr.dev/slog/v3"
 	"github.com/coder/coder/v2/coderd/apikey"
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/dbauthz"
 	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/coderd/httpapi"
 	"github.com/coder/coder/v2/coderd/httpmw"
+	"github.com/coder/coder/v2/coderd/httpmw/loggermw"
 	"github.com/coder/coder/v2/coderd/rbac"
 	"github.com/coder/coder/v2/codersdk"
 )
@@ -234,6 +236,22 @@ func Tokens(db database.Store, lifetimes codersdk.SessionLifetime) http.HandlerF
 	}
 }
 
+// revokeOAuth2CodeOnPKCEFailure deletes a code that failed PKCE verification
+// so it cannot be replayed with further code_verifier guesses (RFC 6749
+// §10.5). Deletion failure does not change the response returned to the
+// caller: surfacing it as a different error would let a caller distinguish
+// "delete succeeded" from "delete failed," defeating the point of revoking
+// the code in the first place. It is instead noted on the request's log line
+// so operators can see it happened.
+func revokeOAuth2CodeOnPKCEFailure(ctx context.Context, db database.Store, codeID uuid.UUID) {
+	//nolint:gocritic // OAuth2 system context, no authenticated user during token exchange
+	if err := db.DeleteOAuth2ProviderAppCodeByID(dbauthz.AsSystemOAuth2(ctx), codeID); err != nil {
+		if rlogger := loggermw.RequestLoggerFromContext(ctx); rlogger != nil {
+			rlogger.WithFields(slog.F("oauth2_pkce_failure_code_revoke_error", err.Error()))
+		}
+	}
+}
+
 func authorizationCodeGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, lifetimes codersdk.SessionLifetime, req codersdk.OAuth2TokenRequest) (codersdk.OAuth2TokenResponse, error) {
 	// Validate the client secret.
 	secret, err := ParseFormattedSecret(req.ClientSecret)
@@ -306,12 +324,19 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
 	// req.CodeVerifier is guaranteed to meet RFC 7636 §4.1's bounds here; a
 	// mismatch below is a wrong-but-well-formed verifier, RFC 7636 §4.6's
 	// invalid_grant case.
+	//
+	// RFC 6749 §10.5 requires codes to be single-use. A code that survives a
+	// failed PKCE check would otherwise let a leaked code (the exact threat
+	// PKCE defends against) be replayed with different code_verifier guesses
+	// for the rest of its lifetime, unthrottled.
 	if !dbCode.CodeChallenge.Valid || dbCode.CodeChallenge.String == "" {
 		// Code was issued without a challenge — should not happen
 		// with authorize endpoint enforcement, but defend in depth.
+		revokeOAuth2CodeOnPKCEFailure(ctx, db, dbCode.ID)
 		return codersdk.OAuth2TokenResponse{}, errInvalidPKCE
 	}
 	if !VerifyPKCE(dbCode.CodeChallenge.String, req.CodeVerifier) {
+		revokeOAuth2CodeOnPKCEFailure(ctx, db, dbCode.ID)
 		return codersdk.OAuth2TokenResponse{}, errInvalidPKCE
 	}
 

From 663865aa51f588de0cbada35da9f0091fbdc337e Mon Sep 17 00:00:00 2001
From: Bobby Ho <bobby@coder.com>
Date: Tue, 11 Aug 2026 08:16:53 -0700
Subject: [PATCH 7/8] fix: resolve remaining coder-agents-review findings on
 PKCE hardening

Tighten the ValidPKCEFormat doc comment and correct a false claim
(CRF-8, CRF-10). The rationale restated the same threat model across
three separate rhetorical framings, and claimed PKCE is the only
client authentication some clients have, which is false today since
authorizationCodeGrant validates a client secret before PKCE ever
runs; that claim only becomes true once #27873 adds public clients.
Trim the paragraph to a single concrete why and note the caveat.

Delete four boundary-case comments in pkce_test.go (CRF-9). Each one
restated the case name and the strings.Repeat literal beside it; the
RFC provenance already lives on ValidPKCEFormat's doc comment and the
pkceVerifierMinLength/pkceVerifierMaxLength constants, so the comments
carried no information and would drift if either constant changed.

Replace an em-dash with a comma in a comment inside the block this
PR's PKCE-failure handling touches (CRF-2), per the repo's no-emdash
rule. It survived lint because the check scans only changed lines by
default, and this comment was pre-existing context rather than a line
this PR added.

Fix the PKCE example in docs/admin/integrations/oauth2-provider.md
(CRF-7). tr -d "=+/" deleted reserved base64 characters instead of
translating them to the URL-safe alphabet, so the example computed a
code_challenge that failed to verify roughly 74% of the time. Also
strip the newline openssl base64 inserts at its default 64-column
wrap, which the 96-byte verifier example crosses; the prior
cut -c1-128 never merged the wrapped lines back together either.
---
 coderd/oauth2provider/pkce.go              | 13 ++++++-------
 coderd/oauth2provider/pkce_test.go         | 12 ++++--------
 coderd/oauth2provider/tokens.go            |  2 +-
 docs/admin/integrations/oauth2-provider.md |  4 ++--
 4 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/coderd/oauth2provider/pkce.go b/coderd/oauth2provider/pkce.go
index c52e58f31f7..c43c83c3384 100644
--- a/coderd/oauth2provider/pkce.go
+++ b/coderd/oauth2provider/pkce.go
@@ -18,14 +18,13 @@ const (
 // directly, and a code_challenge because the S256 method that produces it
 // (base64url(SHA256(verifier))) always yields a string within these bounds.
 //
-// The length floor is the whole point. PKCE is the only client authentication
-// some clients have, and the challenge travels in the authorization request
-// URL while the code travels in the redirect, both of which land in browser
-// history, referrer headers, and proxy logs. An attacker holding those
+// The length floor matters because the challenge and code both travel
+// through the authorization URL and redirect, landing in browser history,
+// referrer headers, and proxy logs. An attacker who recovers either one
 // brute-forces the verifier offline at whatever entropy the client chose,
-// where no server-side rate limit applies. A client that sends a
-// one-character verifier has set a one-character password, and the server
-// should refuse it rather than accept whatever the client picked. The same
+// with no server-side rate limit to slow them down. A client secret also
+// authenticates the token request today, but public clients (#27873) will
+// rely on this bound alone, so it must hold on its own merit. The same
 // bound on code_challenge keeps a malformed value from being persisted
 // verbatim and failing late, at token exchange, instead of at the
 // authorization request where RFC 7636 §4.4.1 expects it to be rejected.
diff --git a/coderd/oauth2provider/pkce_test.go b/coderd/oauth2provider/pkce_test.go
index 07ec53d8ead..62d92a91fbb 100644
--- a/coderd/oauth2provider/pkce_test.go
+++ b/coderd/oauth2provider/pkce_test.go
@@ -145,26 +145,22 @@ func TestValidPKCEFormat(t *testing.T) {
 			expectValid: false,
 		},
 		{
-			name: "OneBelowMinLength",
-			// 42 characters, one short of the RFC 7636 §4.1 floor.
+			name:        "OneBelowMinLength",
 			verifier:    strings.Repeat("a", 42),
 			expectValid: false,
 		},
 		{
-			name: "AtMinLength",
-			// 43 characters, the RFC 7636 §4.1 floor.
+			name:        "AtMinLength",
 			verifier:    strings.Repeat("a", 43),
 			expectValid: true,
 		},
 		{
-			name: "AtMaxLength",
-			// 128 characters, the RFC 7636 §4.1 ceiling.
+			name:        "AtMaxLength",
 			verifier:    strings.Repeat("a", 128),
 			expectValid: true,
 		},
 		{
-			name: "OneAboveMaxLength",
-			// 129 characters, one past the RFC 7636 §4.1 ceiling.
+			name:        "OneAboveMaxLength",
 			verifier:    strings.Repeat("a", 129),
 			expectValid: false,
 		},
diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go
index 6c359128dba..caa5fb6b77f 100644
--- a/coderd/oauth2provider/tokens.go
+++ b/coderd/oauth2provider/tokens.go
@@ -330,7 +330,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
 	// PKCE defends against) be replayed with different code_verifier guesses
 	// for the rest of its lifetime, unthrottled.
 	if !dbCode.CodeChallenge.Valid || dbCode.CodeChallenge.String == "" {
-		// Code was issued without a challenge — should not happen
+		// Code was issued without a challenge, which should not happen
 		// with authorize endpoint enforcement, but defend in depth.
 		revokeOAuth2CodeOnPKCEFailure(ctx, db, dbCode.ID)
 		return codersdk.OAuth2TokenResponse{}, errInvalidPKCE
diff --git a/docs/admin/integrations/oauth2-provider.md b/docs/admin/integrations/oauth2-provider.md
index 51c588e513c..181cada6641 100644
--- a/docs/admin/integrations/oauth2-provider.md
+++ b/docs/admin/integrations/oauth2-provider.md
@@ -185,8 +185,8 @@ confidential clients must include PKCE parameters:
 1. Generate a code verifier and challenge:
 
    ```sh
-   CODE_VERIFIER=$(openssl rand -base64 96 | tr -d "=+/" | cut -c1-128)
-   CODE_CHALLENGE=$(echo -n $CODE_VERIFIER | openssl dgst -sha256 -binary | base64 | tr -d "=+/" | cut -c1-43)
+   CODE_VERIFIER=$(openssl rand -base64 96 | tr -d '\n' | tr '+/' '-_' | tr -d '=')
+   CODE_CHALLENGE=$(echo -n $CODE_VERIFIER | openssl dgst -sha256 -binary | base64 | tr -d "=" | tr '+/' '-_')
    ```
 
 2. Include PKCE parameters in the authorization request:

From 912ce41b4bd02f62d5ae7035d0f24731524e2a02 Mon Sep 17 00:00:00 2001
From: Bobby Ho <bobby@coder.com>
Date: Tue, 11 Aug 2026 08:56:49 -0700
Subject: [PATCH 8/8] fix(docs/admin): document PKCE length and charset
 requirement
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The PKCE Flow section showed how to generate a code_verifier and
code_challenge but never stated the bound now enforced server-side:
43 to 128 characters from the unreserved set [A-Za-z0-9-._~] (RFC
7636 §4.1). A value outside these bounds returns invalid_request,
at the token endpoint for code_verifier and at the authorization
endpoint for code_challenge.
---
 docs/admin/integrations/oauth2-provider.md | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/docs/admin/integrations/oauth2-provider.md b/docs/admin/integrations/oauth2-provider.md
index 181cada6641..1cbb17a3e1d 100644
--- a/docs/admin/integrations/oauth2-provider.md
+++ b/docs/admin/integrations/oauth2-provider.md
@@ -182,6 +182,13 @@ PKCE is **required** for all OAuth2 authorization code flows. Coder enforces
 PKCE in compliance with the OAuth 2.1 specification. Both public and
 confidential clients must include PKCE parameters:
 
+> [!NOTE]
+> `code_verifier` and `code_challenge` must each be 43-128 characters from
+> the unreserved character set `[A-Za-z0-9-._~]` (RFC 7636 §4.1). A value
+> outside these bounds is rejected with an `invalid_request` error, at the
+> token endpoint for `code_verifier` and at the authorization endpoint for
+> `code_challenge`.
+
 1. Generate a code verifier and challenge:
 
    ```sh