feat: improve remote port errors

cmd/topo/deploy.go

@@ -10,6 +10,7 @@ import (
 	"github.com/arm/topo/internal/deploy"
 	"github.com/arm/topo/internal/deploy/operation"
 	checks "github.com/arm/topo/internal/deploy/project_checks"
+	"github.com/arm/topo/internal/env"
 	goperation "github.com/arm/topo/internal/operation"
 	"github.com/arm/topo/internal/output/logger"
 	"github.com/arm/topo/internal/ssh"
@@ -18,11 +19,12 @@ import (
 )
 
 var (
-	noRegistry        bool
-	registryPort      string
-	skipProjectChecks bool
-	forceRecreate     bool
-	noRecreate        bool
+	noRegistry          bool
+	registryPort        string
+	skipRemotePortCheck bool
+	skipProjectChecks   bool
+	forceRecreate       bool
+	noRecreate          bool
 )
 
 var deployOpts deploy.DeployOptions
@@ -78,8 +80,9 @@ The compose file (compose.yaml) must be in the current working directory, as thi
 		goos := runtime.GOOS
 		if deploy.SupportsRegistry(noRegistry, deployOpts.TargetHost) {
 			deployOpts.Registry = &deploy.RegistryConfig{
-				Port:              resolvedPort,
-				UseControlSockets: deploy.SupportsSSHControlSockets(goos),
+				Port:                resolvedPort,
+				SkipRemotePortCheck: resolveSkipRemotePortCheck(cmd),
+				UseControlSockets:   deploy.SupportsSSHControlSockets(goos),
 			}
 		}
 		switch {
@@ -127,7 +130,10 @@ func validatePort(port string) error {
 	return nil
 }
 
-const portEnvVar = "TOPO_PORT"
+const (
+	portEnvVar                = "TOPO_PORT"
+	skipRemotePortCheckEnvVar = "TOPO_SKIP_REMOTE_PORT_CHECK"
+)
 
 func resolvePort(cmd *cobra.Command, flagValue string) (string, error) {
 	if cmd.Flags().Changed("registry-port") {
@@ -139,10 +145,20 @@ func resolvePort(cmd *cobra.Command, flagValue string) (string, error) {
 	return flagValue, nil
 }
 
+func resolveSkipRemotePortCheck(cmd *cobra.Command) bool {
+	flagValue, _ := cmd.Flags().GetBool("skip-remote-port-check")
+	if cmd.Flags().Changed("skip-remote-port-check") {
+		return flagValue
+	}
+
+	return env.IsVarTruthy(skipRemotePortCheckEnvVar)
+}
+
 func init() {
 	addTargetFlag(deployCmd)
 	deployCmd.Flags().StringVarP(&registryPort, "registry-port", "p", operation.DefaultRegistryPort, fmt.Sprintf("registry and SSH tunnel port (can also be set via %s env var)", portEnvVar))
 	deployCmd.Flags().BoolVar(&noRegistry, "no-registry", false, "disable private registry flow; use direct save/load transfer")
+	deployCmd.Flags().BoolVar(&skipRemotePortCheck, "skip-remote-port-check", false, fmt.Sprintf("skip checking whether the SSH tunnel port is exposed on the remote network (can also be set via %s env var)", skipRemotePortCheckEnvVar))
 	deployCmd.Flags().BoolVar(&forceRecreate, "force-recreate", false, "force recreation of containers even if their configuration and image haven't changed")
 	deployCmd.Flags().BoolVar(&noRecreate, "no-recreate", false, "prevent recreation of containers even if their configuration and image have changed")
 	deployCmd.Flags().BoolVar(&skipProjectChecks, "skip-project-checks", false, "skip project compatibility checks for the target platform")

internal/deploy/deploy.go

@@ -9,8 +9,9 @@ import (
 )
 
 type RegistryConfig struct {
-	Port              string
-	UseControlSockets bool
+	Port                string
+	SkipRemotePortCheck bool
+	UseControlSockets   bool
 }
 
 type DeployOptions struct {
@@ -46,7 +47,9 @@ func NewDeployment(composeFile string, opts DeployOptions) (goperation.Sequence,
 			cleanup = stop
 			ops = append(ops, operation.NewRunRegistry(opts.Registry.Port)...)
 			ops = append(ops, start)
-			ops = append(ops, securityCheck)
+			if !opts.Registry.SkipRemotePortCheck {
+				ops = append(ops, securityCheck)
+			}
 			ops = append(ops, operation.NewRegistryTransfer(composeFile, sourceHost, targetHost, opts.Registry.Port))
 			ops = append(ops, stop)
 		} else {

internal/deploy/deploy_test.go

@@ -148,6 +148,21 @@ func TestNewDeployment(t *testing.T) {
 		)
 		assert.Equal(t, want, got)
 	})
+
+	t.Run("excludes the remote port check when skipped", func(t *testing.T) {
+		remoteDest := ssh.NewDestination("user@remote")
+		opts := deploy.DeployOptions{
+			TargetHost: remoteDest,
+			Registry: &deploy.RegistryConfig{
+				SkipRemotePortCheck: true,
+			},
+		}
+
+		got, _ := deploy.NewDeployment(composeFile, opts)
+
+		_, remotePortCheck, _ := ssh.NewSSHTunnel(remoteDest, opts.Registry.Port, opts.Registry.UseControlSockets)
+		assert.NotContains(t, got, remotePortCheck)
+	})
 }
 
 func TestDeployment(t *testing.T) {

internal/ssh/config.go

@@ -22,6 +22,19 @@ func NewConfig(dest Destination) Config {
 	return NewConfigFromBytes(output)
 }
 
+func resolveHostName(dest Destination) (string, error) {
+	output, err := readConfig(dest)
+	if err != nil {
+		return "", fmt.Errorf("could not resolve SSH configuration for %q: %w", dest.String(), err)
+	}
+
+	hostName := NewConfigFromBytes(output).HostName
+	if hostName == "" {
+		return "", fmt.Errorf("could not resolve SSH hostname for %q: SSH configuration did not provide a hostname", dest.String())
+	}
+	return hostName, nil
+}
+
 func NewConfigFromBytes(data []byte) Config {
 	var config Config
 	scanner := bufio.NewScanner(bytes.NewReader(data))

internal/ssh/ssh_tunnel.go

@@ -2,12 +2,15 @@ package ssh
 
 import (
 	"crypto/sha256"
+	"errors"
 	"fmt"
 	"io"
+	"net"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
+	"strings"
 
 	"github.com/arm/topo/internal/command"
 	"github.com/arm/topo/internal/operation"
@@ -92,7 +95,7 @@ type CheckRemoteForwardNotExposed struct {
 
 // Checks whether the RemoteForward port exposes the registry to the target's
 // network, rather than being limited to target loopback. This can happen when
-// sshd permits non-loopback remote forwards, such as GatewayPorts.
+// the SSH server permits non-loopback remote forwards.
 func NewCheckRemoteForwardNotExposed(targetDest Destination, port string) *CheckRemoteForwardNotExposed {
 	return &CheckRemoteForwardNotExposed{TargetDest: targetDest, Port: port}
 }
@@ -106,27 +109,53 @@ func (ct *CheckRemoteForwardNotExposed) Run(w io.Writer) error {
 		return nil
 	}
 
-	host := NewConfig(ct.TargetDest).HostName
-	if RemotePortRefusedConnection(host, ct.Port) {
-		_, _ = fmt.Fprintf(w, "Port %s is bound to remote loopback only\n", ct.Port)
-		return nil
+	host, err := resolveHostName(ct.TargetDest)
+	if err != nil {
+		return remotePortCheckErrorWithSuggestion(err, ct.Port)
+	}
+	listening, err := isRemotePortListening(host, ct.Port)
+	if err != nil {
+		return remotePortCheckErrorWithSuggestion(err, ct.Port)
 	}
-	return fmt.Errorf("remote sshd might be exposing the forwarded port %s on its network (likely GatewayPorts=yes); the local registry may be reachable without SSH auth", ct.Port)
+	if listening {
+		return fmt.Errorf("the remote SSH server is exposing forwarded registry port %s beyond remote loopback; configure the SSH server to bind remote forwards to loopback only, or use `--skip-remote-port-check` if you understand that the registry may be reachable without SSH authentication", ct.Port)
+	}
+	_, _ = fmt.Fprintf(w, "Port %s is bound to remote loopback only\n", ct.Port)
+	return nil
 }
 
-func RemotePortRefusedConnection(host, port string) bool {
-	cmd := exec.Command("curl", fmt.Sprintf("%s:%s", host, port), "--max-time", "5")
-	cmd.Stdout = io.Discard
+func isRemotePortListening(host, port string) (bool, error) {
+	address := net.JoinHostPort(host, port)
+	var remoteIP strings.Builder
+	// Release binaries use Go's resolver because CGO is disabled. Use curl so
+	// host resolution matches OpenSSH for mDNS, NSS, and split-DNS configurations.
+	// remote_ip detects a connection even when HTTP fails, noproxy ensures the
+	// connection is direct, and silent suppresses curl's progress and errors.
+	cmd := exec.Command("curl", "http://"+address, "--max-time", "5", "--noproxy", "*", "--output", os.DevNull, "--silent", "--write-out", "%{remote_ip}")
+	cmd.Stdout = &remoteIP
 	cmd.Stderr = io.Discard
-	_ = cmd.Run()
+	err := cmd.Run()
+	if err == nil || remoteIP.Len() > 0 {
+		return true, nil
+	}
 
-	// Only curl exit 7 ("Failed to connect to host") proves no TCP listener
-	// answered. Anything else — connection succeeded, DNS failure, timeout,
-	// curl missing — leaves us unable to certify the port is not not listening.
-	if cmd.ProcessState != nil && cmd.ProcessState.ExitCode() == 7 {
-		return true
+	exitError, ok := errors.AsType[*exec.ExitError](err)
+	if ok {
+		switch exitError.ExitCode() {
+		case 7:
+			return false, nil
+		case 28:
+			return false, fmt.Errorf("timed out while checking whether remote port %s is exposed: %w", address, err)
+		case 6:
+			return false, fmt.Errorf("could not resolve remote host %q while checking tunnel exposure: %w", host, err)
+		}
 	}
-	return false
+
+	return false, fmt.Errorf("could not verify whether remote port %s is exposed: %w", address, err)
+}
+
+func remotePortCheckErrorWithSuggestion(err error, port string) error {
+	return fmt.Errorf("cannot conclusively rule out network access to registry port %s because the exposure check did not complete: %w; retry after resolving the connectivity issue, or use `--skip-remote-port-check` if you understand the security risk", port, err)
 }
 
 type SSHTunnelStop struct {

internal/ssh/ssh_tunnel_test.go

@@ -2,6 +2,7 @@ package ssh_test
 
 import (
 	"fmt"
+	"io"
 	"net"
 	"os"
 	"strings"
@@ -106,36 +107,69 @@ func TestCheckRemoteForwardNotExposed(t *testing.T) {
 			assert.Equal(t, "Check tunnel port is not exposed on remote network", got)
 		})
 	})
-}
 
-func TestRemotePortRefusedConnection(t *testing.T) {
-	t.Run("it returns true when nothing answers on the remote port", func(t *testing.T) {
-		port := reserveFreePort(t)
+	t.Run("Run", func(t *testing.T) {
+		t.Run("it skips the check for localhost", func(t *testing.T) {
+			check := ssh.NewCheckRemoteForwardNotExposed(ssh.PlainLocalhost, "invalid")
+			var output strings.Builder
 
-		assert.True(t, ssh.RemotePortRefusedConnection("127.0.0.1", port))
-	})
+			err := check.Run(&output)
+
+			assert.NoError(t, err)
+			assert.Empty(t, output.String())
+		})
 
-	t.Run("it returns false when a TCP listener is bound on the remote port", func(t *testing.T) {
-		listener, err := net.Listen("tcp", "127.0.0.1:0")
-		require.NoError(t, err)
-		t.Cleanup(func() { _ = listener.Close() })
-		_, port, err := net.SplitHostPort(listener.Addr().String())
-		require.NoError(t, err)
+		t.Run("it fails when the SSH hostname cannot be resolved", func(t *testing.T) {
+			check := ssh.NewCheckRemoteForwardNotExposed(ssh.Destination{}, "12345")
 
-		assert.False(t, ssh.RemotePortRefusedConnection("127.0.0.1", port))
-	})
+			err := check.Run(io.Discard)
+
+			assert.ErrorContains(t, err, "cannot conclusively rule out network access to registry port 12345")
+			assert.ErrorContains(t, err, `could not resolve SSH configuration for "ssh://"`)
+			assert.ErrorContains(t, err, "use `--skip-remote-port-check` if you understand the security risk")
+		})
+
+		t.Run("it succeeds when the remote port refuses the connection", func(t *testing.T) {
+			port := reserveFreePort(t, "0.0.0.0")
+			check := ssh.NewCheckRemoteForwardNotExposed(ssh.NewDestination("0.0.0.0"), port)
+			var output strings.Builder
+
+			err := check.Run(&output)
 
-	t.Run("it returns false when curl exits with any other non-7 error", func(t *testing.T) {
-		// `.invalid` is reserved by RFC 6761 to never resolve, so curl
-		// exits 6 ("Couldn't resolve host") rather than 7. We can't
-		// certify the tunnel is safe, so we must fail.
-		assert.False(t, ssh.RemotePortRefusedConnection("nonexistent.invalid", "12345"))
+			assert.NoError(t, err)
+			assert.Equal(t, fmt.Sprintf("Port %s is bound to remote loopback only\n", port), output.String())
+		})
+
+		t.Run("it fails when the remote port accepts a connection", func(t *testing.T) {
+			listener, err := net.Listen("tcp", "127.0.0.1:0")
+			require.NoError(t, err)
+			t.Cleanup(func() { _ = listener.Close() })
+			connectionClosed := make(chan error, 1)
+			go func() {
+				connection, acceptErr := listener.Accept()
+				if acceptErr != nil {
+					connectionClosed <- acceptErr
+					return
+				}
+				connectionClosed <- connection.Close()
+			}()
+			_, port, err := net.SplitHostPort(listener.Addr().String())
+			require.NoError(t, err)
+			check := ssh.NewCheckRemoteForwardNotExposed(ssh.NewDestination("localhost."), port)
+
+			err = check.Run(io.Discard)
+			listenerCloseErr := listener.Close()
+
+			assert.EqualError(t, err, fmt.Sprintf("the remote SSH server is exposing forwarded registry port %s beyond remote loopback; configure the SSH server to bind remote forwards to loopback only, or use `--skip-remote-port-check` if you understand that the registry may be reachable without SSH authentication", port))
+			assert.NoError(t, listenerCloseErr)
+			assert.NoError(t, <-connectionClosed)
+		})
 	})
 }
 
-func reserveFreePort(t *testing.T) string {
+func reserveFreePort(t *testing.T, host string) string {
 	t.Helper()
-	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	listener, err := net.Listen("tcp", net.JoinHostPort(host, "0"))
 	require.NoError(t, err)
 	_, port, err := net.SplitHostPort(listener.Addr().String())
 	require.NoError(t, err)