Validate forge URLs before opening a browser

internal/cli/browse.go

@@ -2,10 +2,12 @@ package cli
 
 import (
 	"fmt"
+	"net/url"
 	"os"
 	"os/exec"
 	"runtime"
 	"strconv"
+	"strings"
 
 	"github.com/git-pkgs/forge/internal/resolve"
 	"github.com/spf13/cobra"
@@ -67,7 +69,7 @@ var browseCmd = &cobra.Command{
 			return nil
 		}
 
-		return openBrowser(url)
+		return openBrowser(url, domain)
 	},
 }
 
@@ -94,11 +96,31 @@ func init() {
 	browseCmd.Flags().BoolVar(&flagBrowsePulls, "pulls", false, "Open pull requests page")
 }
 
-func openBrowser(url string) error {
-	argv := browserCmd(runtime.GOOS, url)
+func openBrowser(rawURL, domain string) error {
+	if err := validateBrowserURL(rawURL, domain); err != nil {
+		return err
+	}
+	argv := browserCmd(runtime.GOOS, rawURL)
 	return exec.Command(argv[0], argv[1:]...).Start()
 }
 
+func validateBrowserURL(rawURL, domain string) error {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return fmt.Errorf("invalid browser URL: %w", err)
+	}
+	if (u.Scheme != "http" && u.Scheme != "https") || u.Hostname() == "" || u.Opaque != "" {
+		return fmt.Errorf("invalid browser URL: must be an absolute HTTP or HTTPS URL with a host")
+	}
+	if u.User != nil {
+		return fmt.Errorf("invalid browser URL: user information is not allowed")
+	}
+	if !strings.EqualFold(u.Host, domain) {
+		return fmt.Errorf("invalid browser URL: host %q does not match forge host %q", u.Host, domain)
+	}
+	return nil
+}
+
 // browserCmd returns the argv to open url in a browser. The BROWSER
 // environment variable takes precedence over the platform default.
 //

internal/cli/browse_test.go

@@ -1,10 +1,128 @@
 package cli
 
 import (
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
 	"slices"
+	"strings"
 	"testing"
+
+	"github.com/git-pkgs/forge/gitea"
+	"github.com/git-pkgs/forge/internal/config"
+	"github.com/git-pkgs/forge/internal/resolve"
 )
 
+func TestValidateBrowserURL(t *testing.T) {
+	tests := []struct {
+		name, url, domain string
+		valid             bool
+	}{
+		{"https", "https://github.com/owner/repo", "github.com", true},
+		{"http", "http://forge.local:3000/owner/repo", "forge.local:3000", true},
+		{"case insensitive", "HTTPS://FORGE.EXAMPLE/owner/repo", "forge.example", true},
+		{"ipv6", "http://[::1]:3000/owner/repo", "[::1]:3000", true},
+		{"query and fragment", "https://forge.example/owner/repo?q=one&sort=asc#L1", "forge.example", true},
+		{"escaped path", "https://forge.example/owner/repo/blob/feature%2Fone/file%20name", "forge.example", true},
+		{"empty", "", "forge.example", false},
+		{"local path", "/tmp/repo.html", "forge.example", false},
+		{"option", "-aCalculator", "forge.example", false},
+		{"relative", "owner/repo", "forge.example", false},
+		{"scheme relative", "//forge.example/owner/repo", "forge.example", false},
+		{"file", "file:///tmp/repo.html", "forge.example", false},
+		{"custom scheme", "forge-probe://forge.example/action", "forge.example", false},
+		{"javascript", "javascript:alert(1)", "forge.example", false},
+		{"opaque", "https:forge.example/owner/repo", "forge.example", false},
+		{"missing host", "https:///owner/repo", "forge.example", false},
+		{"empty hostname", "https://:443/owner/repo", ":443", false},
+		{"other host", "https://other.example/owner/repo", "forge.example", false},
+		{"host suffix", "https://forge.example.other.example/owner/repo", "forge.example", false},
+		{"other port", "http://forge.local:3001/owner/repo", "forge.local:3000", false},
+		{"userinfo", "https://user:password@forge.example/owner/repo", "forge.example", false},
+		{"empty userinfo", "https://@forge.example/owner/repo", "forge.example", false},
+		{"host in userinfo", "https://forge.example@other.example/owner/repo", "forge.example", false},
+		{"backslash", `https://other.example\@forge.example/owner/repo`, "forge.example", false},
+		{"control character", "https://forge.example/owner/repo\n", "forge.example", false},
+		{"invalid escape", "https://forge.example/%zz", "forge.example", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateBrowserURL(tt.url, tt.domain)
+			if (err == nil) != tt.valid {
+				t.Fatalf("validateBrowserURL(%q, %q) = %v, valid = %v", tt.url, tt.domain, err, tt.valid)
+			}
+		})
+	}
+}
+
+func TestBrowserCommandsRejectUnsafeURLs(t *testing.T) {
+	commands := [][]string{
+		{"browse"},
+		{"browse", "7"},
+		{"browse", "README.md"},
+		{"browse", "--settings"},
+		{"browse", "--wiki"},
+		{"browse", "--actions"},
+		{"browse", "--releases"},
+		{"browse", "--issues"},
+		{"browse", "--pulls"},
+		{"issue", "view", "7", "--web"},
+		{"issue", "list", "--web"},
+		{"pr", "view", "7", "--web"},
+		{"pr", "list", "--web"},
+		{"label", "list", "--web"},
+	}
+	for _, args := range commands {
+		for _, target := range []string{"file:///tmp/repo.html", "https://other.example/owner/repo"} {
+			t.Run(strings.Join(args, " ")+"/"+target, func(t *testing.T) {
+				resetCmd(rootCmd)
+				t.Cleanup(func() {
+					resetCmd(rootCmd)
+					rootCmd.SetArgs(nil)
+				})
+				t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+				t.Setenv("BROWSER", filepath.Join(t.TempDir(), "missing-browser"))
+				config.ResetCache()
+				t.Cleanup(config.ResetCache)
+
+				server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.Header().Set("Content-Type", "application/json")
+					switch r.URL.Path {
+					case "/api/v1/repos/owner/repo":
+						repoURL := target
+						if slices.Contains(args, "7") {
+							repoURL = "https://forge.example/owner/repo"
+						}
+						_ = json.NewEncoder(w).Encode(map[string]any{
+							"name": "repo", "full_name": "owner/repo", "owner": map[string]string{"login": "owner"},
+							"html_url": repoURL, "default_branch": "main",
+						})
+					case "/api/v1/repos/owner/repo/topics":
+						_, _ = io.WriteString(w, `{"topics":[]}`)
+					case "/api/v1/repos/owner/repo/issues/7", "/api/v1/repos/owner/repo/pulls/7":
+						_ = json.NewEncoder(w).Encode(map[string]any{
+							"number": 7, "title": "Example", "state": "closed", "html_url": target,
+						})
+					default:
+						t.Errorf("unexpected API request: %s", r.URL.Path)
+						http.NotFound(w, r)
+					}
+				}))
+				defer server.Close()
+				resolve.SetTestForge(gitea.New(server.URL, "", server.Client()), "owner", "repo", "forge.example")
+				t.Cleanup(resolve.ResetTestForge)
+				rootCmd.SetArgs(args)
+				err := rootCmd.Execute()
+				if err == nil || !strings.Contains(err.Error(), "invalid browser URL:") {
+					t.Fatalf("expected URL rejection before launching browser, got %v", err)
+				}
+			})
+		}
+	}
+}
+
 func TestBrowserCmd(t *testing.T) {
 	const url = "https://github.com/owner/repo"
 

internal/cli/issue.go

@@ -53,7 +53,7 @@ func issueViewCmd() *cobra.Command {
 				return fmt.Errorf("invalid issue number: %s", args[0])
 			}
 
-			forge, owner, repoName, _, err := resolve.Repo(flagRepo, flagForgeType)
+			forge, owner, repoName, domain, err := resolve.Repo(flagRepo, flagForgeType)
 			if err != nil {
 				return err
 			}
@@ -64,7 +64,7 @@ func issueViewCmd() *cobra.Command {
 			}
 
 			if flagWeb {
-				return openBrowser(issue.HTMLURL)
+				return openBrowser(issue.HTMLURL, domain)
 			}
 
 			p := printer()
@@ -139,7 +139,7 @@ func issueListCmd() *cobra.Command {
 		Aliases: []string{"ls"},
 		Short:   "List issues",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			forge, owner, repoName, _, err := resolve.Repo(flagRepo, flagForgeType)
+			forge, owner, repoName, domain, err := resolve.Repo(flagRepo, flagForgeType)
 			if err != nil {
 				return err
 			}
@@ -149,7 +149,7 @@ func issueListCmd() *cobra.Command {
 				if err != nil {
 					return fmt.Errorf("getting repository: %w", err)
 				}
-				return openBrowser(forge.Issues().ListURL(repo.HTMLURL))
+				return openBrowser(forge.Issues().ListURL(repo.HTMLURL), domain)
 			}
 
 			opts := forges.ListIssueOpts{

internal/cli/label.go

@@ -37,7 +37,7 @@ func labelListCmd() *cobra.Command {
 		Aliases: []string{"ls"},
 		Short:   "List labels",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			f, owner, repoName, _, err := resolve.Repo(flagRepo, flagForgeType)
+			f, owner, repoName, domain, err := resolve.Repo(flagRepo, flagForgeType)
 			if err != nil {
 				return err
 			}
@@ -47,7 +47,7 @@ func labelListCmd() *cobra.Command {
 				if err != nil {
 					return fmt.Errorf("getting repository: %w", err)
 				}
-				return openBrowser(f.Labels().ListURL(repo.HTMLURL))
+				return openBrowser(f.Labels().ListURL(repo.HTMLURL), domain)
 			}
 
 			opts := forges.ListLabelOpts{

internal/cli/pr.go

@@ -71,7 +71,7 @@ branch matches the current git branch.`,
 				return fmt.Errorf("--json is not supported; use --output json instead (field selection is not supported)\n\nTry: %s", strings.Join(hints, "\n     "))
 			}
 
-			forge, owner, repoName, _, err := resolve.Repo(flagRepo, flagForgeType)
+			forge, owner, repoName, domain, err := resolve.Repo(flagRepo, flagForgeType)
 			if err != nil {
 				return err
 			}
@@ -102,7 +102,7 @@ branch matches the current git branch.`,
 			}
 
 			if flagWeb {
-				return openBrowser(pr.HTMLURL)
+				return openBrowser(pr.HTMLURL, domain)
 			}
 
 			p := printer()
@@ -194,7 +194,7 @@ func prListCmd() *cobra.Command {
 		Aliases: []string{"ls"},
 		Short:   "List pull requests",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			forge, owner, repoName, _, err := resolve.Repo(flagRepo, flagForgeType)
+			forge, owner, repoName, domain, err := resolve.Repo(flagRepo, flagForgeType)
 			if err != nil {
 				return err
 			}
@@ -204,7 +204,7 @@ func prListCmd() *cobra.Command {
 				if err != nil {
 					return fmt.Errorf("getting repository: %w", err)
 				}
-				return openBrowser(forge.PullRequests().ListURL(repo.HTMLURL))
+				return openBrowser(forge.PullRequests().ListURL(repo.HTMLURL), domain)
 			}
 
 			opts := forges.ListPROpts{