fix: validate external url scheme and host (#29569)

* coder/terraform-provider-coder#483og
  corresponds to coder/terraform-provider-coder#542

Coder apps marked `external: true` are handed to the browser as-is and
navigated to directly, so their URL must carry a scheme and a host.
Previously the provisioner didn't validate this, so a template could
define an external app with a relative or scheme-less URL (e.g.
`coder.com/docs`) and the browser would either refuse the link or
resolve it against the dashboard's own origin instead of failing at
plan/apply time.

Add `ValidateExternalURL` (`provisioner/urls.go`) and call it from
`ConvertState` for every `external` `coder_app`. Terraform reports
values it can't resolve until apply as an empty string, so validation is
skipped when the URL is still empty at plan time and only runs once a
value is actually known.


https://github.com/coder/terraform-provider-coder/pull/542#discussion_r3936424268

provisioner/terraform/resources.go

@@ -522,6 +522,13 @@ func ConvertState(ctx context.Context, modules []*tfjson.StateModule, rawGraph s
 		}
 		appSlugs[attrs.Slug] = struct{}{}
 
+		// The browser navigates to external app URLs directly, so they must be valid urls
+		if attrs.External && attrs.URL != "" {
+			if err := provisioner.ValidateExternalURL(attrs.URL); err != nil {
+				return nil, xerrors.Errorf("invalid external url %q for app %q: %w", attrs.URL, attrs.Slug, err)
+			}
+		}
+
 		var healthcheck *proto.Healthcheck
 		if len(attrs.Healthcheck) != 0 {
 			healthcheck = &proto.Healthcheck{

provisioner/terraform/resources_test.go

@@ -1299,6 +1299,63 @@ func TestAppSlugValidation(t *testing.T) {
 	}
 }
 
+//nolint:tparallel
+func TestAppExternalURLInvalid(t *testing.T) {
+	t.Parallel()
+	ctx, logger := ctxAndLogger(t)
+
+	// nolint:dogsled
+	_, filename, _, _ := runtime.Caller(0)
+
+	// Load the multiple-apps state file and edit it.
+	dir := filepath.Join(filepath.Dir(filename), "testdata", "resources", "multiple-apps")
+	tfPlanRaw, err := os.ReadFile(filepath.Join(dir, "multiple-apps.tfplan.json"))
+	require.NoError(t, err)
+	var tfPlan tfjson.Plan
+	err = json.Unmarshal(tfPlanRaw, &tfPlan)
+	require.NoError(t, err)
+	tfPlanGraph, err := os.ReadFile(filepath.Join(dir, "multiple-apps.tfplan.dot"))
+	require.NoError(t, err)
+
+	cases := []struct {
+		name        string
+		external    bool
+		url         any
+		errContains string
+	}{
+		{name: "MissingScheme", external: true, url: "coder.com/docs", errContains: "must include a scheme"},
+		{name: "SchemeOnly", external: true, url: "https://", errContains: `"https" URLs must include a host`},
+		{name: "PortOnly", external: true, url: "https://:8080", errContains: `"https" URLs must include a host`},
+		{name: "AbsoluteURL", external: true, url: "https://coder.com/docs", errContains: ""},
+		{name: "CustomScheme", external: true, url: "zed://ssh/coder.dev", errContains: ""},
+		// Terraform reports URLs it cannot resolve until apply as empty, and
+		// non-external apps are proxied rather than opened by the browser.
+		{name: "UnresolvedURL", external: true, url: nil, errContains: ""},
+		{name: "NotExternal", external: false, url: "coder.com/docs", errContains: ""},
+	}
+
+	//nolint:paralleltest
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			// Change the first app to match the current case.
+			for _, resource := range tfPlan.PlannedValues.RootModule.Resources {
+				if resource.Type == "coder_app" {
+					resource.AttributeValues["external"] = c.external
+					resource.AttributeValues["url"] = c.url
+					break
+				}
+			}
+
+			_, err := terraform.ConvertState(ctx, []*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph), logger)
+			if c.errContains != "" {
+				require.ErrorContains(t, err, c.errContains)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
 func TestAppSlugDuplicate(t *testing.T) {
 	t.Parallel()
 	ctx, logger := ctxAndLogger(t)

provisioner/urls.go

@@ -0,0 +1,29 @@
+package provisioner
+
+import (
+	"net/url"
+
+	"golang.org/x/xerrors"
+)
+
+// ValidateExternalURL validates that value is a URL has a parseable scheme and host defined
+func ValidateExternalURL(value string) error {
+	u, err := url.Parse(value)
+	if err != nil {
+		return xerrors.Errorf("parse URL: %w", err)
+	}
+
+	if u.String() == "" {
+		return xerrors.New(`must include a scheme and host`)
+	}
+
+	if u.Scheme == "" {
+		return xerrors.New(`must include a scheme, for example "https://"`)
+	}
+
+	if u.Host == "" || u.Hostname() == "" {
+		return xerrors.Errorf("%q URLs must include a host", u.Scheme)
+	}
+
+	return nil
+}

provisioner/urls_test.go

@@ -0,0 +1,58 @@
+package provisioner_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/coder/coder/v2/provisioner"
+)
+
+func TestValidateExternalURL(t *testing.T) {
+	t.Parallel()
+
+	validURLs := []string{
+		"https://coder.com",
+		"https://coder.com/docs/code-server",
+		"http://localhost:3000",
+		"http://127.0.0.1:8080/path?query=1#frag",
+		"zed://ssh/coder.dev",
+		"vscode://coder.coder-remote/open?owner=me",
+		"jetbrains-gateway://connect#type=coder",
+		"coder://dev.coder.com/v0/open/ws/dev/agent/main/rdp",
+	}
+
+	invalidURLs := []string{
+		"",
+		"coder.com",
+		"coder.com/docs",
+		"/relative/path",
+		"//coder.com",
+		"https://",
+		"https://:8080",
+		"mailto:support@coder.com",
+		"file:///home/coder",
+		"https://coder.com/\x7f",
+	}
+
+	for _, value := range validURLs {
+		t.Run("Valid/"+value, func(t *testing.T) {
+			t.Parallel()
+			require.NoError(t, provisioner.ValidateExternalURL(value))
+		})
+	}
+
+	for _, value := range invalidURLs {
+		t.Run("Invalid/"+value, func(t *testing.T) {
+			t.Parallel()
+			require.Error(t, provisioner.ValidateExternalURL(value))
+		})
+	}
+}
+
+func TestValidateExternalURLEmpty(t *testing.T) {
+	t.Parallel()
+
+	err := provisioner.ValidateExternalURL("")
+	require.EqualError(t, err, "must include a scheme and host")
+}