main: Honor the BROWSER environment variable on Unix systems

Prevously OpenTofu delegated browser launching entirely to the third-party
module github.com/cli/browser, which consists of a number of
platform-specific lists of executable commands to try to run to launch
a web browser.

On Unix systems there is also a de-facto convention of using an environment
variable called BROWSER to explicitly specify what to launch. That variable
can either point directly to a browser, or can point to a script which
implements some more complex policy for choosing a browser, such as
detecting whether the command is running in a GUI context and launching
either a GUI or textmode browser.

The BROWSER variable has been most commonly implemented with similar
treatment to earlier variables like EDITOR and PAGER where it's expected
to be set to just a single command to run, with the URL given as the first
and only argument. There was also an attempt to define a more complex
interpretation of this variable at http://www.catb.org/~esr/BROWSER/ , but
that extended treatment was only implemented in a small amount of software,
and those which implemented it did so slightly inconsistently due to the
specification being ambiguous.

OpenTofu's implementation therefore follows the common simpler convention,
but will silently ignore variable values it cannot use so that OpenTofu
won't fail when run in an environment that has that variable set in a way
that's intended for use by some other software. In that case OpenTofu
will continue to perform the default behavior as implemented in the
third-party library.

Because this convention is Unix-specific, OpenTofu will check for and use
this environment variable only on operating systems that the Go toolchain
considers to be "unix". This means that in particular on Windows systems
OpenTofu will continue to follow the Windows convention of specifying
the default browser via an entry in the Windows Registry.

As usual with this sort of system-integration mechanism it isn't really
viable to test this end-to-end in a portable way, but the main logic is
separated out into testable functions, and I manually tested this on my
own Linux system to verify that it works in a real OpenTofu executable.

Signed-off-by: Martin Atkins <mart@degeneration.co.uk>

cmd/tofu/commands.go

@@ -21,7 +21,6 @@ import (
 	"github.com/opentofu/opentofu/internal/command"
 	"github.com/opentofu/opentofu/internal/command/cliconfig"
 	"github.com/opentofu/opentofu/internal/command/views"
-	"github.com/opentofu/opentofu/internal/command/webbrowser"
 	"github.com/opentofu/opentofu/internal/getmodules"
 	"github.com/opentofu/opentofu/internal/getproviders"
 	pluginDiscovery "github.com/opentofu/opentofu/internal/plugin/discovery"
@@ -99,7 +98,7 @@ func initCommands(
 		Ui:               Ui,
 
 		Services:        services,
-		BrowserLauncher: webbrowser.NewNativeLauncher(),
+		BrowserLauncher: browserLauncher(),
 
 		RunningInAutomation: inAutomation,
 		CLIConfigDir:        configDir,

cmd/tofu/webbrowser.go

@@ -0,0 +1,19 @@
+// Copyright (c) The OpenTofu Authors
+// SPDX-License-Identifier: MPL-2.0
+// Copyright (c) 2023 HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package main
+
+import (
+	"github.com/opentofu/opentofu/internal/command/webbrowser"
+)
+
+// browserLauncher implements the policy for deciding how to launch a web
+// browser in the current execution environment.
+func browserLauncher() webbrowser.Launcher {
+	if envLauncher := browserLauncherFromEnv(); envLauncher != nil {
+		return envLauncher
+	}
+	return webbrowser.NewNativeLauncher()
+}

cmd/tofu/webbrowser_nonunix.go

@@ -0,0 +1,17 @@
+// Copyright (c) The OpenTofu Authors
+// SPDX-License-Identifier: MPL-2.0
+// Copyright (c) 2023 HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+//go:build !unix
+
+package main
+
+import (
+	"github.com/opentofu/opentofu/internal/command/webbrowser"
+)
+
+func browserLauncherFromEnv() webbrowser.Launcher {
+	// We know of no environment variable convention for the current platform.
+	return nil
+}

cmd/tofu/webbrowser_unix.go

@@ -0,0 +1,31 @@
+// Copyright (c) The OpenTofu Authors
+// SPDX-License-Identifier: MPL-2.0
+// Copyright (c) 2023 HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+//go:build unix
+
+package main
+
+import (
+	"os"
+
+	"github.com/opentofu/opentofu/internal/command/webbrowser"
+)
+
+func browserLauncherFromEnv() webbrowser.Launcher {
+	// On Unix systems we honor the de-facto standard BROWSER environment
+	// variable in its original, simpler form where it was required to refer
+	// only to a single command to run with the URL to open as the first
+	// and only argument.
+	//
+	// There's information on this convention in Debian's documentation,
+	// although this is not a Debian-specific mechanism:
+	//     https://wiki.debian.org/DefaultWebBrowser#BROWSER_environment_variable
+
+	execPath := webbrowser.ParseBrowserEnv(os.Getenv("BROWSER"))
+	if execPath != "" {
+		return webbrowser.NewExecLauncher(execPath)
+	}
+	return nil
+}

internal/command/webbrowser/exec.go

@@ -0,0 +1,84 @@
+// Copyright (c) The OpenTofu Authors
+// SPDX-License-Identifier: MPL-2.0
+// Copyright (c) 2023 HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package webbrowser
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+)
+
+// NewExecLauncher creates and returns a Launcher that just attempts to run
+// the executable at the given path, with the given URL as its first and
+// only argument.
+//
+// The given path must be ready to use, without reference to the PATH
+// environment variable. The caller can use [exec.LookPath] to prepare
+// a suitable path if searching PATH is appropriate.
+//
+// This is intended to allow overriding which browser to use using the
+// BROWSER environment variable on Unix-like systems, but the rules for
+// that are in "package main". [ParseBrowserEnv] implements parsing of the
+// value of that environment variable when the main package decides it's
+// appropriate to do so.
+func NewExecLauncher(execPath string) Launcher {
+	return execLauncher{
+		execPath: execPath,
+	}
+}
+
+type execLauncher struct {
+	execPath string
+}
+
+func (l execLauncher) OpenURL(url string) error {
+	cmd := &exec.Cmd{
+		Path: l.execPath,
+		Args: []string{l.execPath, url},
+		Env:  os.Environ(),
+	}
+	err := cmd.Run()
+	if err != nil {
+		return fmt.Errorf("%s: %w", l.execPath, err)
+	}
+	return nil
+}
+
+// ParseBrowserEnv takes the raw value of a BROWSER environment variable and
+// attempts to parse it as a reference to an executable, whose absolute
+// path is returned if successful. Returns an empty string if the value cannot
+// be interpreted as an executable to run.
+//
+// This implements the simple form of this environment variable commonly used
+// by software on Unix-like systems, where the value must be literally just
+// a command to run whose first and only argument would be the URL to open.
+//
+// It does NOT support the more complex interpretation of that environment
+// variable that was proposed at http://www.catb.org/~esr/BROWSER/ , because
+// that form has not been widely implemented and the implementations that
+// exist do not have consistent behavior due to the proposal being
+// ambiguous.
+//
+// Callers that use this should typically pass a successful result to
+// [NewExecLauncher] to use the resolved command as a browser launcher. The
+// caller is responsible for deciding the policy for whether to consider a
+// BROWSER environment variable and for accessing the environment table to
+// obtain its value.
+func ParseBrowserEnv(raw string) string {
+	if raw == "" {
+		return "" // empty is treated the same as unset
+	}
+
+	execPath, err := exec.LookPath(raw)
+	if err != nil {
+		// We silently ignore variable values we cannot use, because this
+		// environment variable is not OpenTofu-specific and so it may have
+		// been set for the benefit of software other than OpenTofu which
+		// interprets it differently.
+		return ""
+	}
+	return execPath
+}

internal/command/webbrowser/exec_test.go

@@ -0,0 +1,135 @@
+// Copyright (c) The OpenTofu Authors
+// SPDX-License-Identifier: MPL-2.0
+// Copyright (c) 2023 HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package webbrowser
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+)
+
+const fakeBrowserLaunchCmdOutputEnvName = "OPENTOFU_WEBBROWSER_EXEC_TEST_OUTPUT"
+
+// TestMain overrides the test entrypoint so that we can reuse the test
+// executable as a fake browser launcher command when testing
+// [NewExecLauncher].
+func TestMain(m *testing.M) {
+	if f := os.Getenv(fakeBrowserLaunchCmdOutputEnvName); f != "" {
+		err := fakeBrowserLauncherCommand(f)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "fake browser launcher failed: %s", err)
+			os.Exit(1)
+		}
+		os.Exit(0)
+	}
+	os.Exit(m.Run())
+}
+
+func fakeBrowserLauncherCommand(outputFilename string) error {
+	// The "exec" browser launcher must pass the URL to open in the
+	// first argument to the executable it launches.
+	url := os.Args[1]
+	return os.WriteFile(outputFilename, []byte(url), os.ModePerm)
+}
+
+func TestExecLauncher(t *testing.T) {
+	// For this test we re-use the text executable as a fake browser-launching
+	// program, through the special logic in [TestMain].
+	fakeExec := os.Args[0]
+
+	tempDir := t.TempDir()
+	outputFile := filepath.Join(tempDir, "browser-exec-launcher-test")
+	t.Setenv(fakeBrowserLaunchCmdOutputEnvName, outputFile)
+
+	launcher := NewExecLauncher(fakeExec)
+	err := launcher.OpenURL("http://example.com/")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	result, err := os.ReadFile(outputFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got, want := string(result), "http://example.com/"; got != want {
+		t.Errorf("wrong URL written to output file\ngot:  %s\nwant: %s", got, want)
+	}
+}
+
+func TestParseBrowserEnv_success(t *testing.T) {
+	// ParseBrowserEnv only actually needs to work on Unix-like systems, so
+	// the test scenario below is not written to be portable.
+	// There's no runtime equivalent of the "unix" build tag, so we just
+	// explicitly test the two main Unix OSes we support here.
+	if runtime.GOOS != "linux" && runtime.GOOS != "darwin" {
+		t.Skip("ParseBrowserEnv is only for unix systems")
+	}
+
+	tmpDir := t.TempDir()
+	fakeExec := filepath.Join(tmpDir, "fake-launch-browser")
+	err := os.WriteFile(fakeExec, []byte(`not a real program`), 0755)
+	if err != nil {
+		// NOTE: This test requires the temp directory to be somewhere that
+		// allows executables, so this won't work if the temp directory is
+		// on a "noexec" mount on a Unix-style system.
+		t.Fatal(err)
+	}
+	// Temporarily we'll reset the search path to just our temp directory
+	t.Setenv("PATH", tmpDir)
+	result := ParseBrowserEnv("fake-launch-browser")
+	if result == "" {
+		t.Fatal("failed to find fake executable")
+	}
+	if got, want := filepath.Base(result), "fake-launch-browser"; got != want {
+		t.Fatalf("returned path %q has wrong basename %q; want %q", result, got, want)
+	}
+}
+
+func TestParseBrowserEnv_empty(t *testing.T) {
+	result := ParseBrowserEnv("")
+	if result != "" {
+		t.Errorf("returned %q, but wanted empty string", result)
+	}
+}
+
+func TestParseBrowserEnv_esrComplexSpec(t *testing.T) {
+	// The following tests with a strings following the more complex
+	// interpretation of BROWSER from http://www.catb.org/~esr/BROWSER/ , which
+	// OpenTofu intentionally doesn't support and so should be treated as
+	// if the environment variable isn't set at all.
+	t.Run(`with %s`, func(t *testing.T) {
+		// The esr proposal calls for checking whether there's a %s sequence
+		// in the value and then, if so, substituting the URL there and then
+		// passing the entire result to a shell. This is the main thing that
+		// different implementations did inconsistently, because it's
+		// unspecified whether the %s should be placed in quotes in the
+		// environment variable, if those quotes should be inserted by the
+		// program acting on the variable, or if some other shell escaping
+		// strategy should be used instead. We just ignore this form entirely
+		// because it's apparently not commonly used and it's unclear how
+		// to implement it without causing security problems.
+		result := ParseBrowserEnv("example %s")
+		if result != "" {
+			t.Errorf("returned %q, but wanted empty string", result)
+		}
+	})
+	t.Run("multiple commands", func(t *testing.T) {
+		// The esr proposal calls for splitting the string on semicolon
+		// and trying one command at a time until one succeeds. That's
+		// ambiguous with there being a single command whose path contains
+		// a semicolon, so we just try to treat it as a single command and
+		// ignore the value if that doesn't work. In practice the need for
+		// multiple options to try tends to be met instead by setting BROWSER
+		// to refer to a wrapper script that deals with the selection policy,
+		// which is the pattern OpenTofu supports.
+		result := ParseBrowserEnv("example1;example2")
+		if result != "" {
+			t.Errorf("returned %q, but wanted empty string", result)
+		}
+	})
+}