fix: relax OAuth2 app form validation for names and callback URLs (#29333)

## Problem The OAuth2 application settings form rejects application names with spaces and callbacks using native application schemes. A VS Code extension client registered through DCR exposed this: `VS Code Coder Extension` and `vscode://coder.coder-remote/oauth/callback` were already stored, but the admin form prevented editing them. ## Fix - Allow application names with spaces and punctuation, limited to 64 UTF-8 bytes on both client and server. - Allow native callback schemes while rejecting dangerous schemes, unsupported URNs, opaque callbacks such as `localhost:3000`, and empty targets such as `vscode:`. HTTP(S) callbacks still require a host. - Reuse the public-client registration policy when admins update public clients, so `mailto`, `tel`, `sms`, external HTTP, and fragments remain rejected. - Trim surrounding whitespace from names and callbacks when submitting, without stripping spaces while typing. - Update the admin API request validators too, so accepted form values can be saved. The server remains authoritative for URL parsing. Public-client edits reuse the existing DCR redirect validator using the stored client type; confidential admin-created applications retain their existing HTTP policy. Native host/path callbacks and the supported OOB URN remain accepted. ## Validation - Vitest component tests assert create/edit submission payloads, whitespace normalization, Unicode byte limits, and rejection of unsafe or malformed callbacks. - Storybook stories cover accepted native-client values and invalid callback states. - Go tests cover name boundaries and accepted/rejected callback values through both admin create and update requests. - `make pre-commit` and targeted tests run before pushing; CI checked on each revision. <details> <summary>Implementation decisions</summary> - Replace the shared slug validator only for OAuth2 application names; leave user, workspace, and template names unchanged. - Share the backend's 64-byte name limit with the frontend through the generated `OAuth2AppNameMaxBytes` constant. - Reuse `codersdk.ValidateRedirectURIScheme` for common admin callback validation. Public-client updates additionally call `codersdk.ValidateRedirectURIs`; confidential admin HTTP behavior is preserved. - Normalize name and callback whitespace in the submitted payload rather than on each keystroke. - Keep behavior assertions in Vitest and use Storybook for visual states. </details> Closes ENG-3342 > 🤖 This PR was created with the help of Coder Agents, and needs a human review. 🧑💻
2026-10-16 14:48:56 +08:00 · 2026-09-16 21:06:31 +05:00
parent c61ca62d64
commit f79a36469e
12 changed files with 631 additions and 51 deletions
@@ -9,6 +9,7 @@ import (
 	"flag"
 	"fmt"
 	"net/http"
+	"net/url"
 	"reflect"
 	"strings"
 	"time"
@@ -49,13 +50,56 @@ func init() {
 		valid := codersdk.NameValid(str)
 		return valid == nil
 	}
-	for _, tag := range []string{"username", "organization_name", "template_name", "workspace_name", "oauth2_app_name"} {
+	for _, tag := range []string{"username", "organization_name", "template_name", "workspace_name"} {
 		err := Validate.RegisterValidation(tag, nameValidator)
 		if err != nil {
 			panic(err)
 		}
 	}

+	oauth2AppNameValidator := func(fl validator.FieldLevel) bool {
+		str, ok := fl.Field().Interface().(string)
+		if !ok {
+			return false
+		}
+		return codersdk.OAuth2AppNameValid(str) == nil
+	}
+	err := Validate.RegisterValidation("oauth2_app_name", oauth2AppNameValidator)
+	if err != nil {
+		panic(err)
+	}
+
+	// oauth2_callback_url validates the common callback target shape for OAuth2
+	// app administration. Public clients receive the additional DCR redirect URI
+	// policy in the handler after their stored client type is available.
+	oauth2CallbackURLValidator := func(fl validator.FieldLevel) bool {
+		str, ok := fl.Field().Interface().(string)
+		if !ok {
+			return false
+		}
+		u, err := url.Parse(str)
+		if err != nil {
+			return false
+		}
+		if err := codersdk.ValidateRedirectURIScheme(u); err != nil {
+			return false
+		}
+		if u.Scheme == "urn" {
+			return true
+		}
+		if (u.Scheme == "http" || u.Scheme == "https") && u.Host == "" {
+			return false
+		}
+		if u.Opaque != "" || (u.Host == "" && u.Path == "") {
+			return false
+		}
+		return true
+	}
+	err = Validate.RegisterValidation("oauth2_callback_url", oauth2CallbackURLValidator)
+	if err != nil {
+		panic(err)
+	}
+
 	displayNameValidator := func(fl validator.FieldLevel) bool {
 		f := fl.Field().Interface()
 		str, ok := f.(string)
@@ -81,7 +125,7 @@ func init() {
 		valid := codersdk.TemplateVersionNameValid(str)
 		return valid == nil
 	}
-	err := Validate.RegisterValidation("template_version_name", templateVersionNameValidator)
+	err = Validate.RegisterValidation("template_version_name", templateVersionNameValidator)
 	if err != nil {
 		panic(err)
 	}
@@ -143,6 +143,18 @@ func UpdateApp(db database.Store, accessURL *url.URL, auditor *audit.Auditor, lo
 		if !httpapi.Read(ctx, rw, r, &req) {
 			return
 		}
+		if app.IsPublic() {
+			if err := codersdk.ValidateRedirectURIs([]string{req.CallbackURL}, codersdk.OAuth2ClientTypePublic); err != nil {
+				httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
+					Message: "Validation failed.",
+					Validations: []codersdk.ValidationError{{
+						Field:  "callback_url",
+						Detail: err.Error(),
+					}},
+				})
+				return
+			}
+		}
 		app, err := db.UpdateOAuth2ProviderAppByID(ctx, database.UpdateOAuth2ProviderAppByIDParams{
 			ID:                      app.ID,
 			UpdatedAt:               dbtime.Now(),
@@ -2,7 +2,11 @@ package oauth2provider_test

 import (
 	"context"
+	"errors"
 	"fmt"
+	"net/http"
+	"slices"
+	"strings"
 	"testing"
 	"time"

@@ -36,16 +40,16 @@ func TestOAuth2ProviderAppValidation(t *testing.T) {
 				},
 			},
 			{
-				name: "NameSpaces",
+				name: "NameTooLong",
 				req: codersdk.PostOAuth2ProviderAppRequest{
-					Name:        "foo bar",
+					Name:        strings.Repeat("a", 65),
 					CallbackURL: "http://localhost:3000",
 				},
 			},
 			{
-				name: "NameTooLong",
+				name: "NameLeadingSpace",
 				req: codersdk.PostOAuth2ProviderAppRequest{
-					Name:        "too loooooooooooooooooooooooooong",
+					Name:        " foo",
 					CallbackURL: "http://localhost:3000",
 				},
 			},
@@ -55,13 +59,6 @@ func TestOAuth2ProviderAppValidation(t *testing.T) {
 					Name: "foo",
 				},
 			},
-			{
-				name: "URLLocalhostNoScheme",
-				req: codersdk.PostOAuth2ProviderAppRequest{
-					Name:        "foo",
-					CallbackURL: "localhost:3000",
-				},
-			},
 			{
 				name: "URLNoScheme",
 				req: codersdk.PostOAuth2ProviderAppRequest{
@@ -125,6 +122,148 @@ func TestOAuth2ProviderAppValidation(t *testing.T) {
 		}
 	})

+	t.Run("AcceptsDCRValues", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		_ = coderdtest.CreateFirstUser(t, client)
+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		// Values registered through Dynamic Client Registration (RFC 7591),
+		// such as a name with spaces and a custom native-app callback scheme,
+		// must be creatable and editable via the admin OAuth2 app settings.
+		app, err := client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
+			Name:        "VS Code Coder Extension",
+			CallbackURL: "vscode://coder.coder-remote/oauth/callback",
+		})
+		require.NoError(t, err)
+		require.Equal(t, "VS Code Coder Extension", app.Name)
+		require.Equal(t, "vscode://coder.coder-remote/oauth/callback", app.CallbackURL)
+
+		updated, err := client.PutOAuth2ProviderApp(ctx, app.ID, codersdk.PutOAuth2ProviderAppRequest{
+			Name:        "Cursor (MCP)",
+			CallbackURL: "cursor://anysphere.cursor-mcp/oauth/callback",
+		})
+		require.NoError(t, err)
+		require.Equal(t, "Cursor (MCP)", updated.Name)
+		require.Equal(t, "cursor://anysphere.cursor-mcp/oauth/callback", updated.CallbackURL)
+	})
+
+	t.Run("CallbackURLSchemes", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		_ = coderdtest.CreateFirstUser(t, client)
+
+		tests := []struct {
+			name        string
+			callbackURL string
+			valid       bool
+		}{
+			{name: "OpaqueLocalhost", callbackURL: "localhost:3000"},
+			{name: "SchemeOnly", callbackURL: "vscode:"},
+			{name: "ShortSchemeOnly", callbackURL: "a:"},
+			{name: "EmptyNativeTarget", callbackURL: "vscode://"},
+			{name: "NativeScheme", callbackURL: "vscode://coder.coder-remote/oauth/callback", valid: true},
+			{name: "PathOnlyNativeScheme", callbackURL: "com.example.app:/oauth2redirect", valid: true},
+			{name: "UppercaseOOBURN", callbackURL: "URN:ietf:wg:oauth:2.0:oob", valid: true},
+			{name: "MalformedHTTP", callbackURL: "http:foo"},
+			{name: "HostlessHTTP", callbackURL: "http:/callback"},
+			{name: "HostlessHTTPS", callbackURL: "https:///callback"},
+			{name: "DangerousSchemeMixedCase", callbackURL: "JaVaScRiPt:alert(1)"},
+			{name: "DangerousDataSchemeMixedCase", callbackURL: "DaTa:text/plain,invalid"},
+			{name: "DangerousFileSchemeMixedCase", callbackURL: "FiLe:///tmp/invalid"},
+			{name: "DangerousFTPSchemeMixedCase", callbackURL: "FtP://example.com/invalid"},
+			{name: "UnsupportedURN", callbackURL: "urn:example:invalid"},
+			{name: "UnsupportedURNMixedCase", callbackURL: "URN:example:invalid"},
+		}
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				t.Parallel()
+
+				ctx := testutil.Context(t, testutil.WaitLong)
+				app, err := client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
+					Name:        testutil.GetRandomName(t),
+					CallbackURL: "https://example.com/callback",
+				})
+				require.NoError(t, err)
+
+				_, postErr := client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
+					Name:        testutil.GetRandomName(t),
+					CallbackURL: test.callbackURL,
+				})
+				if test.valid {
+					require.NoError(t, postErr)
+				} else {
+					requireCallbackURLValidationError(t, postErr)
+				}
+
+				_, putErr := client.PutOAuth2ProviderApp(ctx, app.ID, codersdk.PutOAuth2ProviderAppRequest{
+					Name:        testutil.GetRandomName(t),
+					CallbackURL: test.callbackURL,
+				})
+				if test.valid {
+					require.NoError(t, putErr)
+				} else {
+					requireCallbackURLValidationError(t, putErr)
+				}
+			})
+		}
+	})
+
+	t.Run("PublicDCRCallbackURLPolicy", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		_ = coderdtest.CreateFirstUser(t, client)
+		oauth2providertest.EnableDCR(t, client)
+
+		tests := []struct {
+			name        string
+			callbackURL string
+			valid       bool
+		}{
+			{name: "Mailto", callbackURL: "mailto:user@example.com"},
+			{name: "MailtoHierarchical", callbackURL: "mailto://user@example.com"},
+			{name: "TelHierarchical", callbackURL: "tel:/15551234567"},
+			{name: "SMSHierarchical", callbackURL: "sms:/15551234567"},
+			{name: "Tel", callbackURL: "tel:+15551234567"},
+			{name: "SMS", callbackURL: "sms:+15551234567"},
+			{name: "ExternalHTTP", callbackURL: "http://example.com/callback"},
+			{name: "Fragment", callbackURL: "https://example.com/callback#fragment"},
+			{name: "LoopbackHTTP", callbackURL: "http://127.0.0.1:8080/callback", valid: true},
+			{name: "Native", callbackURL: "com.example.app:/oauth2redirect", valid: true},
+			{name: "HTTPS", callbackURL: "https://example.com/updated-callback", valid: true},
+		}
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				t.Parallel()
+
+				ctx := testutil.Context(t, testutil.WaitLong)
+				registered, err := client.PostOAuth2ClientRegistration(ctx, codersdk.OAuth2ClientRegistrationRequest{
+					ClientName:              testutil.GetRandomName(t),
+					RedirectURIs:            []string{"https://example.com/callback"},
+					TokenEndpointAuthMethod: codersdk.OAuth2TokenEndpointAuthMethodNone,
+				})
+				require.NoError(t, err)
+				appID, err := uuid.Parse(registered.ClientID)
+				require.NoError(t, err)
+
+				updated, err := client.PutOAuth2ProviderApp(ctx, appID, codersdk.PutOAuth2ProviderAppRequest{
+					Name:        testutil.GetRandomName(t),
+					CallbackURL: test.callbackURL,
+				})
+				if !test.valid {
+					requireCallbackURLValidationError(t, err)
+					return
+				}
+				require.NoError(t, err)
+				require.Equal(t, test.callbackURL, updated.CallbackURL)
+				require.Equal(t, codersdk.OAuth2ClientTypePublic, updated.ClientType)
+			})
+		}
+	})
+
 	t.Run("DuplicateNames", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
@@ -422,7 +561,17 @@ func TestOAuth2ProviderAppOperations(t *testing.T) {
 	})
 }

-// Helper functions
+func requireCallbackURLValidationError(t *testing.T, err error) {
+	t.Helper()
+
+	require.Error(t, err)
+	var apiErr *codersdk.Error
+	require.True(t, errors.As(err, &apiErr))
+	require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
+	require.True(t, slices.ContainsFunc(apiErr.Validations, func(validation codersdk.ValidationError) bool {
+		return validation.Field == "callback_url"
+	}), "expected callback_url validation error, got: %+v", apiErr.Validations)
+}

 type provisionedApps struct {
 	Default   codersdk.OAuth2ProviderApp
@@ -59,6 +59,28 @@ func NameValid(str string) error {
 	return nil
 }

+// OAuth2AppNameMaxBytes is the maximum UTF-8 byte length of an OAuth2
+// application name.
+const OAuth2AppNameMaxBytes = 64
+
+// OAuth2AppNameValid returns whether the input string is a valid OAuth2
+// application name: a non-empty string of at most OAuth2AppNameMaxBytes UTF-8
+// bytes with no leading or trailing whitespace. Unlike NameValid, it allows
+// spaces and other characters that appear in dynamically registered client
+// names such as "VS Code Coder Extension".
+func OAuth2AppNameValid(str string) error {
+	if len(str) < 1 {
+		return xerrors.New("must be >= 1 character")
+	}
+	if len(str) > OAuth2AppNameMaxBytes {
+		return xerrors.Errorf("must be <= %d bytes", OAuth2AppNameMaxBytes)
+	}
+	if strings.TrimSpace(str) != str {
+		return xerrors.New("must not have leading or trailing whitespace")
+	}
+	return nil
+}
+
 // TemplateVersionNameValid returns whether the input string is a valid template version name.
 func TemplateVersionNameValid(str string) error {
 	if len(str) > 64 {
@@ -1,6 +1,7 @@
 package codersdk_test

 import (
+	"fmt"
 	"strings"
 	"testing"

@@ -287,3 +288,41 @@ func TestGroupNameValid(t *testing.T) {
 		})
 	}
 }
+
+func TestOAuth2AppNameValid(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		Name  string
+		Valid bool
+	}{
+		{"", false},
+		{"a", true},
+		{"my-app", true},
+		// DCR-registered clients use spaces and other characters that the
+		// slug-style name validator rejects.
+		{"VS Code Coder Extension", true},
+		{"Cursor (MCP)", true},
+		{"new", true},
+		{"create", true},
+		{" leading", false},
+		{"trailing ", false},
+		{" ", false},
+		{strings.Repeat("a", codersdk.OAuth2AppNameMaxBytes), true},
+		{strings.Repeat("a", codersdk.OAuth2AppNameMaxBytes+1), false},
+		{strings.Repeat("é", codersdk.OAuth2AppNameMaxBytes/2), true},
+		{strings.Repeat("é", codersdk.OAuth2AppNameMaxBytes/2+1), false},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.Name, func(t *testing.T) {
+			t.Parallel()
+			err := codersdk.OAuth2AppNameValid(testCase.Name)
+			assert.Equal(t, testCase.Valid, err == nil,
+				"Test case %q failed: expected valid=%t but got error: %v",
+				testCase.Name, testCase.Valid, err)
+			if len(testCase.Name) > codersdk.OAuth2AppNameMaxBytes {
+				assert.EqualError(t, err, fmt.Sprintf("must be <= %d bytes", codersdk.OAuth2AppNameMaxBytes))
+			}
+		})
+	}
+}
@@ -80,7 +80,7 @@ func (c *Client) OAuth2ProviderApp(ctx context.Context, id uuid.UUID) (OAuth2Pro

 type PostOAuth2ProviderAppRequest struct {
 	Name        string `json:"name" validate:"required,oauth2_app_name"`
-	CallbackURL string `json:"callback_url" validate:"required,http_url"`
+	CallbackURL string `json:"callback_url" validate:"required,oauth2_callback_url"`
 	Icon        string `json:"icon" validate:"omitempty"`
 }

@@ -101,7 +101,7 @@ func (c *Client) PostOAuth2ProviderApp(ctx context.Context, app PostOAuth2Provid

 type PutOAuth2ProviderAppRequest struct {
 	Name        string `json:"name" validate:"required,oauth2_app_name"`
-	CallbackURL string `json:"callback_url" validate:"required,http_url"`
+	CallbackURL string `json:"callback_url" validate:"required,oauth2_callback_url"`
 	Icon        string `json:"icon" validate:"omitempty"`
 }

@@ -18,7 +18,7 @@ func (req *OAuth2ClientRegistrationRequest) Validate() error {

 	// The client type is derived once, by DetermineClientType, so which RFC 8252
 	// rules apply here cannot drift from what gets stored in client_type.
-	if err := validateRedirectURIs(req.RedirectURIs, req.DetermineClientType()); err != nil {
+	if err := ValidateRedirectURIs(req.RedirectURIs, req.DetermineClientType()); err != nil {
 		return xerrors.Errorf("invalid redirect_uris: %w", err)
 	}

@@ -140,11 +140,11 @@ func validateScheme(u *url.URL) error {
 	return nil
 }

-// validateRedirectURIs validates redirect URIs according to RFC 7591, 8252.
+// ValidateRedirectURIs validates redirect URIs according to RFC 7591, 8252.
 // clientType selects which rules apply and is derived by DetermineClientType,
 // the single owner of that mapping, so this cannot disagree with the type the
 // app is stored as.
-func validateRedirectURIs(uris []string, clientType OAuth2ClientType) error {
+func ValidateRedirectURIs(uris []string, clientType OAuth2ClientType) error {
 	if len(uris) == 0 {
 		return xerrors.New("at least one redirect URI is required")
 	}
@@ -6484,6 +6484,13 @@ export interface OAuth2AppEndpoints {
 	readonly device_authorization: string;
 }

+// From codersdk/name.go
+/**
+ * OAuth2AppNameMaxBytes is the maximum UTF-8 byte length of an OAuth2
+ * application name.
+ */
+export const OAuth2AppNameMaxBytes = 64;
+
 // From codersdk/oauth2.go
 /**
 * OAuth2AuthorizationServerMetadata represents RFC 8414 OAuth 2.0 Authorization Server Metadata.
@@ -80,20 +80,29 @@ export const WithValidationError: Story = {
 	},
 };

-export const InvalidName: Story = {
+export const InvalidCallbackURL: Story = {
 	play: async ({ canvasElement }) => {
 		const canvas = within(canvasElement);
-		const nameInput = await canvas.findByLabelText(/^name/i);
-		await userEvent.type(nameInput, "Foo@Application");
+		await userEvent.type(await canvas.findByLabelText(/^name/i), "test-app");
+		const callbackInput = canvas.getByLabelText(/callback url/i);
+		// oxlint-disable-next-line eslint/no-script-url -- Deliberately invalid input exercises callback URL rejection.
+		await userEvent.type(callbackInput, "javascript:alert(1)");
+		await userEvent.tab();
+	},
+};
+
+export const DynamicallyRegisteredValues: Story = {
+	play: async ({ canvasElement }) => {
+		const canvas = within(canvasElement);
+		await userEvent.type(
+			await canvas.findByLabelText(/^name/i),
+			"VS Code Coder Extension",
+		);
+		await userEvent.type(
+			canvas.getByLabelText(/callback url/i),
+			"vscode://coder.coder-remote/oauth/callback",
+		);
 		await userEvent.tab();
-		await expect(
-			await canvas.findByText(
-				/special characters \(e\.g\.: !, @, #\) are not supported/i,
-			),
-		).toBeVisible();
-		await expect(
-			canvas.getByRole("button", { name: /create application/i }),
-		).toBeDisabled();
 	},
 };

@@ -155,6 +155,31 @@ export const DeleteDialogOpen: Story = {
 	},
 };

+export const DynamicallyRegisteredValues: Story = {
+	parameters: {
+		queries: [
+			{
+				key: oauth2ProviderAppKey(appId),
+				data: {
+					...mockApp,
+					name: "VS Code Coder Extension",
+					callback_url: "vscode://coder.coder-remote/oauth/callback",
+				},
+			},
+			{
+				key: oauth2ProviderAppSecretsKey(appId),
+				data: MockOAuth2ProviderAppSecrets,
+			},
+		],
+	},
+	play: async ({ canvasElement }) => {
+		const canvas = within(canvasElement);
+		const nameField = await canvas.findByLabelText(/^name/i);
+		await userEvent.clear(nameField);
+		await userEvent.type(nameField, "Cursor (MCP)");
+	},
+};
+
 export const PublicClient: Story = {
 	parameters: {
 		queries: [
@@ -0,0 +1,225 @@
+import { act, screen, waitFor } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import {
+	MockOAuth2ProviderAppPublic,
+	MockOAuth2ProviderApps,
+} from "#/testHelpers/entities";
+import { render } from "#/testHelpers/renderHelpers";
+import { OAuth2AppForm } from "./OAuth2AppForm";
+
+describe("OAuth2AppForm", () => {
+	it.each([
+		"VS Code Coder Extension",
+		" VS Code Coder Extension",
+		"VS Code Coder Extension ",
+		" VS Code Coder Extension ",
+	])("submits a trimmed name for %j", async (name) => {
+		const user = userEvent.setup();
+		const onSubmit = vi.fn();
+
+		render(
+			<OAuth2AppForm onSubmit={onSubmit} isUpdating={false} disabled={false} />,
+		);
+
+		await user.type(screen.getByLabelText(/^name/i), name);
+		await user.type(
+			screen.getByLabelText(/callback url/i),
+			"vscode://coder.coder-remote/oauth/callback",
+		);
+		await user.click(
+			screen.getByRole("button", { name: /create application/i }),
+		);
+
+		await waitFor(() => {
+			expect(onSubmit).toHaveBeenCalledWith({
+				name: "VS Code Coder Extension",
+				callback_url: "vscode://coder.coder-remote/oauth/callback",
+				icon: "",
+			});
+		});
+	});
+
+	it("submits edited dynamically registered client values", async () => {
+		const user = userEvent.setup();
+		const onSubmit = vi.fn();
+		const app = {
+			...MockOAuth2ProviderApps[0],
+			name: "VS Code Coder Extension",
+			callback_url: "vscode://coder.coder-remote/oauth/callback",
+		};
+
+		render(
+			<OAuth2AppForm
+				app={app}
+				onSubmit={onSubmit}
+				isUpdating={false}
+				disabled={false}
+			/>,
+		);
+
+		await user.clear(screen.getByLabelText(/^name/i));
+		await user.type(screen.getByLabelText(/^name/i), " Cursor MCP Extension ");
+		await user.click(
+			screen.getByRole("button", { name: /update application/i }),
+		);
+
+		await waitFor(() => {
+			expect(onSubmit).toHaveBeenCalledWith({
+				name: "Cursor MCP Extension",
+				callback_url: "vscode://coder.coder-remote/oauth/callback",
+				icon: app.icon,
+			});
+		});
+	});
+
+	it.each([
+		// oxlint-disable-next-line eslint/no-script-url -- Deliberately invalid input exercises callback URL rejection.
+		"javascript:alert(1)",
+		// oxlint-disable-next-line eslint/no-script-url -- Scheme rejection is case insensitive.
+		"JaVaScRiPt:alert(1)",
+		"data:text/plain,hello",
+		"file:///tmp/callback",
+		"ftp://example.com/callback",
+		"urn:example:callback",
+		"http:foo",
+		"https:/example.com",
+		"http:///example.com",
+		"localhost:3000",
+		"vscode:",
+		"a:",
+		"vscode://",
+		"mailto:a@b",
+		"tel:+1234",
+		"sms:+1234",
+	])("does not submit invalid callback %j", async (callback) => {
+		const user = userEvent.setup();
+		const onSubmit = vi.fn();
+
+		render(
+			<OAuth2AppForm onSubmit={onSubmit} isUpdating={false} disabled={false} />,
+		);
+
+		await user.type(screen.getByLabelText(/^name/i), "test-app");
+		const callbackURL = screen.getByLabelText(/callback url/i);
+		await user.type(callbackURL, callback);
+		await user.click(
+			screen.getByRole("button", { name: /create application/i }),
+		);
+
+		await act(async () => {});
+		expect(onSubmit).not.toHaveBeenCalled();
+	});
+
+	it.each([
+		" vscode://coder.coder-remote/oauth/callback",
+		"vscode://coder.coder-remote/oauth/callback ",
+		" vscode://coder.coder-remote/oauth/callback ",
+		"URN:ietf:wg:oauth:2.0:oob",
+		"com.example.app:/oauth2redirect",
+		"cursor://anysphere.cursor-mcp/oauth/callback",
+	])("submits trimmed callback %j", async (callback) => {
+		const user = userEvent.setup();
+		const onSubmit = vi.fn();
+		render(
+			<OAuth2AppForm onSubmit={onSubmit} isUpdating={false} disabled={false} />,
+		);
+		await user.type(screen.getByLabelText(/^name/i), "OAuth App");
+		await user.type(screen.getByLabelText(/callback url/i), callback);
+		await user.click(
+			screen.getByRole("button", { name: /create application/i }),
+		);
+		await waitFor(() =>
+			expect(onSubmit).toHaveBeenCalledWith({
+				name: "OAuth App",
+				callback_url: callback.trim(),
+				icon: "",
+			}),
+		);
+	});
+
+	it.each([
+		{ name: "é".repeat(32), valid: true },
+		{ name: "a" + "é".repeat(32), valid: false },
+		{ name: "é".repeat(33), valid: false },
+		{ name: "界".repeat(21), valid: true },
+		{ name: "界".repeat(22), valid: false },
+	])("enforces the UTF-8 byte limit for $name", async ({ name, valid }) => {
+		const user = userEvent.setup();
+		const onSubmit = vi.fn();
+		render(
+			<OAuth2AppForm onSubmit={onSubmit} isUpdating={false} disabled={false} />,
+		);
+		await user.type(screen.getByLabelText(/^name/i), name);
+		await user.type(
+			screen.getByLabelText(/callback url/i),
+			"https://example.com/callback",
+		);
+		await user.click(
+			screen.getByRole("button", { name: /create application/i }),
+		);
+		await act(async () => {});
+		if (valid) {
+			await waitFor(() =>
+				expect(onSubmit).toHaveBeenCalledWith({
+					name,
+					callback_url: "https://example.com/callback",
+					icon: "",
+				}),
+			);
+		} else {
+			expect(onSubmit).not.toHaveBeenCalled();
+		}
+	});
+
+	it.each([
+		{ callback: "mailto:a@b", valid: false },
+		{ callback: "mailto://a@b", valid: false },
+		{ callback: "tel:+1234", valid: false },
+		{ callback: "tel:/1234", valid: false },
+		{ callback: "sms:+1234", valid: false },
+		{ callback: "sms:/1234", valid: false },
+		{ callback: "http://example.com/callback", valid: false },
+		{ callback: "https://example.com/callback#fragment", valid: false },
+		{ callback: "http://localhost:3000/callback", valid: true },
+		{ callback: "http://127.0.0.1:3000/callback", valid: true },
+		{ callback: "http://[::1]:3000/callback", valid: true },
+		{ callback: "https://example.com/callback", valid: true },
+		{ callback: "vscode://coder.coder-remote/oauth/callback", valid: true },
+		{ callback: "com.example.app:/oauth2redirect", valid: true },
+	])(
+		"validates public client callback $callback",
+		async ({ callback, valid }) => {
+			const user = userEvent.setup();
+			const onSubmit = vi.fn();
+			render(
+				<OAuth2AppForm
+					app={MockOAuth2ProviderAppPublic}
+					onSubmit={onSubmit}
+					isUpdating={false}
+					disabled={false}
+				/>,
+			);
+			await user.clear(screen.getByLabelText(/callback url/i));
+			await user.type(
+				screen.getByLabelText(/callback url/i),
+				callback.replaceAll("[", "[["),
+			);
+			await user.type(screen.getByLabelText(/^name/i), " updated");
+			await user.click(
+				screen.getByRole("button", { name: /update application/i }),
+			);
+			await act(async () => {});
+			if (valid) {
+				await waitFor(() =>
+					expect(onSubmit).toHaveBeenCalledWith({
+						name: `${MockOAuth2ProviderAppPublic.name} updated`,
+						callback_url: callback,
+						icon: MockOAuth2ProviderAppPublic.icon,
+					}),
+				);
+			} else {
+				expect(onSubmit).not.toHaveBeenCalled();
+			}
+		},
+	);
+});
@@ -4,6 +4,7 @@ import { type FC, useEffect, useRef } from "react";
 import { Link } from "react-router";
 import * as Yup from "yup";
 import type * as TypesGen from "#/api/typesGenerated";
+import { OAuth2AppNameMaxBytes } from "#/api/typesGenerated";
 import { ErrorAlert } from "#/components/Alert/ErrorAlert";
 import { Button } from "#/components/Button/Button";
 import { ConfirmDialog } from "#/components/Dialog/ConfirmDialog/ConfirmDialog";
@@ -13,12 +14,7 @@ import { IconField } from "#/components/IconField/IconField";
 import { Label } from "#/components/Label/Label";
 import { Spinner } from "#/components/Spinner/Spinner";
 import { useUnsavedChangesPrompt } from "#/hooks/useUnsavedChangesPrompt";
-import {
-	getFormHelpers,
-	iconValidator,
-	nameValidator,
-	onChangeTrimmed,
-} from "#/utils/formUtils";
+import { getFormHelpers, iconValidator } from "#/utils/formUtils";

 type OAuth2AppFormValues = {
 	name: string;
@@ -38,28 +34,77 @@ type OAuth2AppFormProps = {

 const BACK_HREF = "/deployment/oauth2-provider/apps";

-const isHttpUrl = (value: string | undefined): boolean => {
+// Mirror codersdk.ValidateRedirectURIScheme and httpapi's oauth2_callback_url.
+// The server remains authoritative for URL syntax differences between parsers.
+// oxlint-disable-next-line eslint/no-script-url -- This blocklist rejects the scheme; it is never used as a navigation target.
+const DANGEROUS_CALLBACK_SCHEMES = ["javascript:", "data:", "file:", "ftp:"];
+
+const isValidCallbackURL = (
+	value: string | undefined,
+	isPublicClient: boolean,
+): boolean => {
 	if (!value) {
 		return false;
 	}
 	try {
 		const url = new URL(value);
-		return url.protocol === "http:" || url.protocol === "https:";
+		if (url.protocol === "urn:") {
+			return url.href === "urn:ietf:wg:oauth:2.0:oob";
+		}
+		if (DANGEROUS_CALLBACK_SCHEMES.includes(url.protocol)) {
+			return false;
+		}
+		const target = value.slice(value.indexOf(":") + 1);
+		if (!target.startsWith("/") || (!url.host && !url.pathname)) {
+			return false;
+		}
+		if (isPublicClient) {
+			if (
+				value.includes("#") ||
+				["mailto:", "tel:", "sms:"].includes(url.protocol)
+			) {
+				return false;
+			}
+			if (
+				url.protocol === "http:" &&
+				!["localhost", "127.0.0.1", "[::1]"].includes(url.hostname)
+			) {
+				return false;
+			}
+		}
+		if (
+			(url.protocol === "http:" || url.protocol === "https:") &&
+			!/^https?:\/\/[^/\\\s]/i.test(value)
+		) {
+			return false;
+		}
+		return true;
 	} catch {
 		return false;
 	}
 };

-const validationSchema = Yup.object({
-	name: nameValidator("Name"),
-	callback_url: Yup.string()
-		.trim()
-		.required("Please enter a callback URL.")
-		.test("http-url", "Callback URL must be a valid URL.", (value) =>
-			isHttpUrl(value),
-		),
-	icon: iconValidator,
-});
+const validationSchema = (isPublicClient: boolean) =>
+	Yup.object({
+		name: Yup.string()
+			.trim()
+			.required("Please enter a name.")
+			.test(
+				"name-byte-length",
+				`Name cannot be longer than ${OAuth2AppNameMaxBytes} UTF-8 bytes.`,
+				(value) =>
+					new TextEncoder().encode(value).length <= OAuth2AppNameMaxBytes,
+			),
+		callback_url: Yup.string()
+			.trim()
+			.required("Please enter a callback URL.")
+			.test(
+				"valid-callback-url",
+				"Please enter a valid callback URL.",
+				(value) => isValidCallbackURL(value, isPublicClient),
+			),
+		icon: iconValidator,
+	});

 export const OAuth2AppForm: FC<OAuth2AppFormProps> = ({
 	app,
@@ -77,11 +122,15 @@ export const OAuth2AppForm: FC<OAuth2AppFormProps> = ({
 			callback_url: app?.callback_url ?? defaultValues?.callback_url ?? "",
 			icon: app?.icon ?? defaultValues?.icon ?? "",
 		},
-		validationSchema,
+		validationSchema: validationSchema(app?.client_type === "public"),
 		validateOnMount: true,
 		onSubmit: async (values) => {
 			didSubmit.current = true;
-			await onSubmit(values);
+			await onSubmit({
+				...values,
+				name: values.name.trim(),
+				callback_url: values.callback_url.trim(),
+			});
 		},
 	});
 	const getFieldHelpers = getFormHelpers(form, error);
@@ -124,7 +173,6 @@ export const OAuth2AppForm: FC<OAuth2AppFormProps> = ({
 					label="Name"
 					description="The name of your Coder app."
 					disabled={formDisabled}
-					onChange={onChangeTrimmed(form)}
 					autoFocus
 					required
 				/>