coder · BobbyHo · Aug 29, 2026
diff --git a/coderd/oauth2provider/authorize.go b/coderd/oauth2provider/authorize.go
@@ -29,8 +29,10 @@ import (
 )
 
 // Rejection reasons from scope negotiation, rendered into error_description.
-// Each is wrapped as `%q: %w` with the offending value: xerrors repeats the
-// sentinel's own text unless %w is the final verb.
+// Each names the offending value ahead of `%w`: xerrors repeats the sentinel's
+// own text unless %w is the final verb. Anything the token endpoint can render
+// is quoted with '%s' rather than %q, since RFC 6749 §5.2 excludes " and \
+// from that parameter.
 var (
 	// A requested name outside the external scope catalog: unrecognized, or
 	// recognized but internal-only.
@@ -108,7 +110,7 @@ func firstScopeNotCovered(ctx context.Context, logger slog.Logger, app database.
 				slog.F("app_id", app.ID.String()),
 				slog.F("ceiling", strings.Join(ceiling, " ")),
 				slog.F("scope", s))
-			return "", xerrors.Errorf("%q: %w", s, errCoverageUndecidable)
+			return "", xerrors.Errorf("'%s': %w", s, errCoverageUndecidable)
 		}
 		if !covered {
 			return s, nil

diff --git a/coderd/oauth2provider/authorize_internal_test.go b/coderd/oauth2provider/authorize_internal_test.go
@@ -251,11 +251,13 @@ func requirePersistableScope(t *testing.T, scope string) {
 // Rejection reasons for the package's black-box tests, which cannot reach the
 // sentinels.
 var (
-	ReasonUnknownScope     = errUnknownScope.Error()
-	ReasonNoGrantableScope = errNoGrantableScope.Error()
-	ReasonScopeNotAllowed  = errScopeNotAllowed.Error()
-	ReasonStaleScope       = errStaleScope.Error()
-	ReasonScopeNotGranted  = errScopeNotGranted.Error()
+	ReasonUnknownScope        = errUnknownScope.Error()
+	ReasonNoGrantableScope    = errNoGrantableScope.Error()
+	ReasonScopeNotAllowed     = errScopeNotAllowed.Error()
+	ReasonStaleScope          = errStaleScope.Error()
+	ReasonScopeNotGranted     = errScopeNotGranted.Error()
+	ReasonUnmintableScope     = errUnmintableScope.Error()
+	ReasonCoverageUndecidable = errCoverageUndecidable.Error()
 )
 
 func TestNoScopeAllowlist(t *testing.T) {

diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go
@@ -67,7 +67,7 @@ func scopeStillCoveredByAllowlist(ctx context.Context, logger slog.Logger, app d
 	allowlist := grantableScopes(app.Scope.String)
 	if len(allowlist) == 0 {
 		// Named verbatim for the same reason as in negotiateScope.
-		return xerrors.Errorf("%q: %w", app.Scope.String, errNoGrantableScope)
+		return xerrors.Errorf("'%s': %w", app.Scope.String, errNoGrantableScope)
 	}
 
 	// Canonicalized rather than assumed: the row may have been written by
@@ -77,7 +77,7 @@ func scopeStillCoveredByAllowlist(ctx context.Context, logger slog.Logger, app d
 		return err
 	}
 	if outside != "" {
-		return xerrors.Errorf("%q: %w", outside, errStaleScope)
+		return xerrors.Errorf("'%s': %w", outside, errStaleScope)
 	}
 	return nil
 }
@@ -629,6 +629,12 @@ func refreshTokenGrant(ctx context.Context, db database.Store, logger slog.Logge
 	// Grab the user roles so we can perform the refresh as the user.
 	//nolint:gocritic // OAuth2 system context, need to read the previous API key
 	prevKey, err := db.GetAPIKeyByID(dbauthz.AsSystemOAuth2(ctx), dbToken.APIKeyID)
+	// A revoked token: the key is gone and the refresh cannot mint from it. The
+	// other lookups in this function already answer errBadToken, and without
+	// this one a revocation reads as a server fault.
+	if errors.Is(err, sql.ErrNoRows) {
+		return codersdk.OAuth2TokenResponse{}, errBadToken
+	}
 	if err != nil {
 		return codersdk.OAuth2TokenResponse{}, err
 	}

diff --git a/coderd/oauth2provider/tokens_test.go b/coderd/oauth2provider/tokens_test.go
@@ -361,6 +361,196 @@ func requireExactlyOneMinted(ctx context.Context, t *testing.T, client *codersdk
 	require.Equal(t, 1, rejected)
 }
 
+// A revoked token must refresh as invalid_grant rather than as a server fault.
+// Both cases here pass before the GetAPIKeyByID mapping, because deleting
+// either row cascades the token row away and the prefix lookup answers first.
+// They stay because the property a client depends on is the response, not which
+// statement notices, and the cascades that make them pass are schema the
+// refresh does not control.
+func TestOAuth2RefreshRevokedToken(t *testing.T) {
+	t.Parallel()
+
+	db, pubsub := dbtestutil.NewDB(t)
+	client := coderdtest.New(t, &coderdtest.Options{
+		Database: db,
+		Pubsub:   pubsub,
+	})
+	owner := coderdtest.CreateFirstUser(t, client)
+
+	t.Run("KeyDeleted", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		app := seedAppWithSecret(t, db, sql.NullString{String: scopeInCatalog, Valid: true})
+		code, verifier := authorizeCode(ctx, t, client, app.ID.String(), "workspace:ssh")
+		token := exchangeCode(ctx, t, client, app, code, verifier)
+
+		keyID := tokenRow(ctx, t, db, token.RefreshToken).APIKeyID
+		require.NoError(t, client.DeleteAPIKey(ctx, owner.UserID.String(), keyID))
+
+		status, body := postTokenRequest(ctx, t, client, refreshForm(app, token.RefreshToken))
+		requireTokenGrantError(t, status, body)
+	})
+
+	t.Run("AppSecretDeleted", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		app := seedAppWithSecret(t, db, sql.NullString{String: scopeInCatalog, Valid: true})
+		code, verifier := authorizeCode(ctx, t, client, app.ID.String(), "workspace:ssh")
+		token := exchangeCode(ctx, t, client, app, code, verifier)
+
+		require.NoError(t, client.DeleteOAuth2ProviderAppSecret(ctx, app.ID, app.SecretID))
+
+		status, body := postTokenRequest(ctx, t, client, refreshForm(app, token.RefreshToken))
+		requireTokenGrantError(t, status, body)
+	})
+
+	// The third revocation path FR12 names. It never reaches the grant: the
+	// client_id no longer resolves, so authentication refuses first.
+	t.Run("AppDeleted", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		app := seedAppWithSecret(t, db, sql.NullString{String: scopeInCatalog, Valid: true})
+		code, verifier := authorizeCode(ctx, t, client, app.ID.String(), "workspace:ssh")
+		token := exchangeCode(ctx, t, client, app, code, verifier)
+
+		require.NoError(t, client.DeleteOAuth2ProviderApp(ctx, app.ID))
+
+		status, body := postTokenRequest(ctx, t, client, refreshForm(app, token.RefreshToken))
+		require.Equal(t, http.StatusUnauthorized, status, body)
+		require.Contains(t, body, string(codersdk.OAuth2ErrorCodeInvalidClient), body)
+	})
+}
+
+// The case the mapping actually moves: a token row whose api_key_id names no
+// key. The FK cascade makes that unreachable through any API, so the
+// constraints come off to seed it, and this test takes a database of its own
+// because disabling them applies to every table in it. Without the mapping the
+// refresh answers HTTP 500.
+func TestOAuth2RefreshKeyMissing(t *testing.T) {
+	t.Parallel()
+
+	db, pubsub := dbtestutil.NewDB(t)
+	client := coderdtest.New(t, &coderdtest.Options{
+		Database: db,
+		Pubsub:   pubsub,
+	})
+	owner := coderdtest.CreateFirstUser(t, client)
+	ctx := testutil.Context(t, testutil.WaitLong)
+
+	app := seedAppWithSecret(t, db, sql.NullString{String: scopeInCatalog, Valid: true})
+	refreshToken := seedRefreshToken(ctx, t, db, app, owner.UserID, "workspace:ssh")
+
+	dbtestutil.DisableForeignKeysAndTriggers(t, db)
+	require.NoError(t, db.DeleteAPIKeyByID(dbauthz.AsSystemRestricted(ctx),
+		tokenRow(ctx, t, db, refreshToken).APIKeyID))
+
+	status, body := postTokenRequest(ctx, t, client, refreshForm(app, refreshToken))
+	requireTokenGrantError(t, status, body)
+}
+
+// RFC 6749 §5.2 excludes " and \ from error_description, and %q emits both.
+// Every scope rejection names the offending value, so each is a place they can
+// return.
+func TestOAuth2TokenScopeErrorCharset(t *testing.T) {
+	t.Parallel()
+
+	db, pubsub := dbtestutil.NewDB(t)
+	client := coderdtest.New(t, &coderdtest.Options{
+		Database: db,
+		Pubsub:   pubsub,
+	})
+	owner := coderdtest.CreateFirstUser(t, client)
+
+	cases := []struct {
+		name   string
+		reason string
+		reject func(ctx context.Context, t *testing.T) (int, string)
+	}{
+		{
+			name:   "UnmintableScope",
+			reason: oauth2provider.ReasonUnmintableScope,
+			reject: func(ctx context.Context, t *testing.T) (int, string) {
+				app := seedAppWithSecret(t, db, sql.NullString{})
+				verifier, challenge := oauth2providertest.GeneratePKCE(t)
+				code := seedCode(ctx, t, db, app.ID, owner.UserID, challenge, scopeOutOfCatalog)
+				return postTokenRequest(ctx, t, client, tokenExchangeForm(app, code, verifier))
+			},
+		},
+		{
+			name:   "StaleScope",
+			reason: oauth2provider.ReasonStaleScope,
+			reject: func(ctx context.Context, t *testing.T) (int, string) {
+				app := seedAppWithSecret(t, db, sql.NullString{String: scopeInCatalog, Valid: true})
+				code, verifier := authorizeCode(ctx, t, client, app.ID.String(), "workspace:ssh")
+				setAppAllowlist(ctx, t, db, app, sql.NullString{String: scopeAlsoInCatalog, Valid: true})
+				return postTokenRequest(ctx, t, client, tokenExchangeForm(app, code, verifier))
+			},
+		},
+		{
+			name:   "NoGrantableScope",
+			reason: oauth2provider.ReasonNoGrantableScope,
+			reject: func(ctx context.Context, t *testing.T) (int, string) {
+				app := seedAppWithSecret(t, db, sql.NullString{String: scopeOutOfCatalog, Valid: true})
+				verifier, challenge := oauth2providertest.GeneratePKCE(t)
+				code := seedCode(ctx, t, db, app.ID, owner.UserID, challenge, "workspace:ssh")
+				return postTokenRequest(ctx, t, client, tokenExchangeForm(app, code, verifier))
+			},
+		},
+		{
+			name:   "CoverageUndecidable",
+			reason: oauth2provider.ReasonCoverageUndecidable,
+			reject: func(ctx context.Context, t *testing.T) (int, string) {
+				app := seedAppWithSecret(t, db, sql.NullString{String: scopeInCatalog, Valid: true})
+				verifier, challenge := oauth2providertest.GeneratePKCE(t)
+				// RBAC cannot expand the stored name, so the comparison against
+				// the allowlist fails rather than returning uncovered.
+				code := seedCode(ctx, t, db, app.ID, owner.UserID, challenge, scopeOutOfCatalog)
+				return postTokenRequest(ctx, t, client, tokenExchangeForm(app, code, verifier))
+			},
+		},
+		{
+			name:   "UnknownScope",
+			reason: oauth2provider.ReasonUnknownScope,
+			reject: func(ctx context.Context, t *testing.T) (int, string) {
+				app := seedAppWithSecret(t, db, sql.NullString{String: scopeInCatalog, Valid: true})
+				code, verifier := authorizeCode(ctx, t, client, app.ID.String(), "workspace:ssh")
+				token := exchangeCode(ctx, t, client, app, code, verifier)
+				form := refreshForm(app, token.RefreshToken)
+				form.Set("scope", scopeOutOfCatalog)
+				return postTokenRequest(ctx, t, client, form)
+			},
+		},
+		{
+			name:   "ScopeNotGranted",
+			reason: oauth2provider.ReasonScopeNotGranted,
+			reject: func(ctx context.Context, t *testing.T) (int, string) {
+				app := seedAppWithSecret(t, db, sql.NullString{String: scopeInCatalog, Valid: true})
+				code, verifier := authorizeCode(ctx, t, client, app.ID.String(), "workspace:ssh")
+				token := exchangeCode(ctx, t, client, app, code, verifier)
+				form := refreshForm(app, token.RefreshToken)
+				form.Set("scope", scopeAlsoInCatalog)
+				return postTokenRequest(ctx, t, client, form)
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			ctx := testutil.Context(t, testutil.WaitLong)
+
+			status, body := tc.reject(ctx, t)
+			description := requireTokenScopeError(t, status, body)
+			require.Contains(t, description, tc.reason)
+			require.NotContains(t, description, `"`)
+			require.NotContains(t, description, `\`)
+		})
+	}
+}
+
 // appWithSecret is seeded directly because the management API registers no
 // scope allowlist, and the allowlist is what these tests turn.
 type appWithSecret struct {
@@ -549,6 +739,14 @@ func requireTokenResponse(t *testing.T, status int, body string) codersdk.OAuth2
 	return token
 }
 
+// requireTokenGrantError asserts an RFC 6749 §5.2 invalid_grant response.
+func requireTokenGrantError(t *testing.T, status int, body string) {
+	t.Helper()
+
+	require.Equal(t, http.StatusBadRequest, status, body)
+	require.Contains(t, body, string(codersdk.OAuth2ErrorCodeInvalidGrant), body)
+}
+
 // requireTokenScopeError asserts an RFC 6749 §5.2 invalid_scope response and
 // returns its description.
 func requireTokenScopeError(t *testing.T, status int, body string) string {