From 43590276a8860d00c360ed0c0d29ae68c2d8a62a Mon Sep 17 00:00:00 2001 From: Mustafa Senoglu Date: Tue, 1 Sep 2026 13:24:18 +0300 Subject: [PATCH] fix(provider): validate coder_app URL scheme for external apps When external=true, the URL field now requires a scheme (e.g. https://, vscode://, jetbrains-gateway://). Bare strings like "my-repo" or relative paths like "/some/path" are rejected because they cause JavaScript TypeError in the Coder frontend. Uses CustomizeDiff to cross-reference external and url fields. Internal URLs (external=false) still accept relative paths since they are resolved server-side. Fixes #483 --- provider/app.go | 27 ++++++++ provider/app_test.go | 100 ++++++++++++++++++++++++++++ provider/helpers/validation.go | 30 +++++++++ provider/helpers/validation_test.go | 85 +++++++++++++++++++++++ 4 files changed, 242 insertions(+) diff --git a/provider/app.go b/provider/app.go index fb567573..9ff28e7a 100644 --- a/provider/app.go +++ b/provider/app.go @@ -2,6 +2,8 @@ package provider import ( "context" + "fmt" + "net/url" "regexp" "github.com/google/uuid" @@ -35,6 +37,31 @@ func appResource() *schema.Resource { SchemaVersion: 1, Description: "Use this resource to define shortcuts to access applications in a workspace.", + CustomizeDiff: func(ctx context.Context, diff *schema.ResourceDiff, i any) error { + external, ok := diff.GetOkExists("external") + if !ok || !external.(bool) { + return nil + } + + urlVal, ok := diff.GetOkExists("url") + if !ok { + return nil + } + + u, err := url.Parse(urlVal.(string)) + if err != nil { + return fmt.Errorf("invalid URL %q: %w", urlVal.(string), err) + } + + if u.Scheme == "" { + return fmt.Errorf( + "\"url\" must have a URL scheme (e.g. https://, vscode://, jetbrains-gateway://) when \"external\" is true, got %q", + urlVal.(string), + ) + } + + return nil + }, CreateContext: func(c context.Context, resourceData *schema.ResourceData, i any) diag.Diagnostics { resourceData.SetId(uuid.NewString()) diff --git a/provider/app_test.go b/provider/app_test.go index 17b3dce4..4e545c37 100644 --- a/provider/app_test.go +++ b/provider/app_test.go @@ -576,6 +576,106 @@ func TestApp(t *testing.T) { } }) + t.Run("ExternalURLValidation", func(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + url string + external bool + expectError *regexp.Regexp + }{ + { + name: "ExternalWithScheme", + url: "https://example.com", + external: true, + }, + { + name: "ExternalWithVSCodeScheme", + url: "vscode://remote-ssh", + external: true, + }, + { + name: "ExternalWithJetBrainsScheme", + url: "jetbrains-gateway://connection", + external: true, + }, + { + name: "ExternalBareString", + url: "my-repo", + external: true, + expectError: regexp.MustCompile(`"url" must have a URL scheme`), + }, + { + name: "ExternalRelativePath", + url: "/some/path", + external: true, + expectError: regexp.MustCompile(`"url" must have a URL scheme`), + }, + { + name: "ExternalLocalhostNoScheme", + url: "localhost:8080", + external: true, // Go's url.Parse treats "localhost" as the scheme, so this passes + }, + { + name: "InternalBareString", + url: "my-repo", + external: false, + }, + { + name: "InternalRelativePath", + url: "/some/path", + external: false, + }, + { + name: "InternalLocalhost", + url: "http://localhost:8080", + external: false, + }, + { + name: "InternalLocalhostNoScheme", + url: "localhost:8080", + external: false, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + externalLine := "external = false" + if tc.external { + externalLine = "external = true" + } + + config := fmt.Sprintf(` + provider "coder" {} + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + } + resource "coder_app" "test" { + agent_id = coder_agent.dev.id + slug = "test" + display_name = "Testing" + url = %q + %s + } + `, tc.url, externalLine) + + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: config, + ExpectError: tc.expectError, + }}, + }) + }) + } + }) + t.Run("ConflictsWith", func(t *testing.T) { t.Parallel() diff --git a/provider/helpers/validation.go b/provider/helpers/validation.go index e95310a4..53f81f73 100644 --- a/provider/helpers/validation.go +++ b/provider/helpers/validation.go @@ -21,6 +21,36 @@ func ValidateURL(value any, label string) ([]string, []error) { return nil, nil } +// ValidateExternalURL validates that a URL intended for external use contains +// a scheme (e.g. http://, https://, vscode://, jetbrains-gateway://). +// Go's url.Parse is permissive and accepts bare strings like "my-repo" as +// relative URLs, but JavaScript's new URL() requires a scheme and will crash +// if one is not present. +func ValidateExternalURL(value any, label string) ([]string, []error) { + val, ok := value.(string) + if !ok { + return nil, []error{fmt.Errorf("expected %q to be a string", label)} + } + + if val == "" { + return nil, nil + } + + parsed, err := url.Parse(val) + if err != nil { + return nil, []error{err} + } + + if parsed.Scheme == "" { + return nil, []error{fmt.Errorf( + "%q must have a URL scheme (e.g. https://, vscode://, jetbrains-gateway://), got %q", + label, val, + )} + } + + return nil, nil +} + // WarnDirNotHome returns a warning if dir is set to a value other // than $HOME, because this breaks Coder Desktop file sync. The dir // attribute is deprecated and will be removed in a future release. diff --git a/provider/helpers/validation_test.go b/provider/helpers/validation_test.go index f27c8960..cf2316a2 100644 --- a/provider/helpers/validation_test.go +++ b/provider/helpers/validation_test.go @@ -150,6 +150,91 @@ func TestValidateURL(t *testing.T) { } } +func TestValidateExternalURL(t *testing.T) { + tests := []struct { + name string + value any + label string + expectError bool + errorContains string + }{ + // Valid cases + { + name: "empty string", + value: "", + label: "url", + expectError: false, + }, + { + name: "valid https URL", + value: "https://example.com", + label: "url", + expectError: false, + }, + { + name: "valid http URL", + value: "http://localhost:8080", + label: "url", + expectError: false, + }, + { + name: "vscode scheme", + value: "vscode://remote-ssh", + label: "url", + expectError: false, + }, + { + name: "jetbrains scheme", + value: "jetbrains-gateway://connection", + label: "url", + expectError: false, + }, + // Invalid cases + { + name: "bare string no scheme", + value: "my-repo", + label: "url", + expectError: true, + errorContains: "must have a URL scheme", + }, + { + name: "relative path no scheme", + value: "/some/path", + label: "url", + expectError: true, + errorContains: "must have a URL scheme", + }, + { + name: "localhost without scheme", + value: "localhost:8080", + label: "url", + expectError: false, // Go's url.Parse treats "localhost" as the scheme + }, + { + name: "non-string type", + value: 123, + label: "url", + expectError: true, + errorContains: "expected \"url\" to be a string", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + warnings, errors := ValidateExternalURL(tt.value, tt.label) + + if tt.expectError { + require.Len(t, errors, 1, "expected an error but got none") + require.Contains(t, errors[0].Error(), tt.errorContains) + } else { + require.Empty(t, errors, "expected no errors but got: %v", errors) + } + + require.Nil(t, warnings, "expected warnings to be nil but got: %v", warnings) + }) + } +} + func TestWarnDirNotHome(t *testing.T) { tests := []struct { name string