From e7cd77a7bd8e1f68bb335cc750346a10dc2321c4 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 13:41:43 +0100 Subject: [PATCH 01/13] fix(ssh): list all SSH URLs without selecting an app `ssh --all` lists the SSH URLs of every app, but it selected a container first, so it failed in non-interactive mode in a project with several apps ("Specifying the --app or --worker is required"). Add integration tests for `ssh --pipe`, `--all`, and app and worker selection, with helpers to serve the layout project over the mock SSH server. Co-Authored-By: Claude Opus 5.5 --- integration-tests/project_setup_test.go | 55 ++++++++++++++++++- integration-tests/ssh_test.go | 45 +++++++++++++++ .../Environment/EnvironmentSshCommand.php | 6 +- 3 files changed, 103 insertions(+), 3 deletions(-) diff --git a/integration-tests/project_setup_test.go b/integration-tests/project_setup_test.go index 41fb4df64..c8184a8c3 100644 --- a/integration-tests/project_setup_test.go +++ b/integration-tests/project_setup_test.go @@ -1,6 +1,7 @@ package tests import ( + "encoding/base64" "encoding/json" "net/http" "net/http/httptest" @@ -87,16 +88,66 @@ func setUpGitProject(t *testing.T, envs ...*mockapi.Environment) *gitProject { // Override any global hooks path. runGit(t, p.repo, "config", "core.hooksPath", filepath.Join(p.repo, "hooks")) + p.serveSSH(t, mockssh.ExecHandler(reposDir, nil)) + + return p +} + +// serveSSH starts a mock SSH server, to which the CLI connects for all SSH URLs. +func (p *testProject) serveSSH(t *testing.T, handler mockssh.CommandHandler) *mockssh.Server { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("the mock SSH server runs commands with bash") + } sshServer, err := mockssh.NewServer(t, p.authURL+"/ssh/authority") require.NoError(t, err) t.Cleanup(func() { _ = sshServer.Stop() }) - sshServer.CommandHandler = mockssh.ExecHandler(reposDir, nil) + sshServer.CommandHandler = handler p.f.extraEnv = append(p.f.extraEnv, EnvPrefix+"SSH_OPTIONS=HostName 127.0.0.1\nPort "+strconv.Itoa(sshServer.Port()), EnvPrefix+"SSH_HOST_KEYS="+sshServer.HostKeyConfig(), ) + return sshServer +} - return p +// remoteEnv returns the environment for commands run by the mock SSH server, with +// fake executables (shell scripts, by name) ahead of the system PATH, and relationships. +func remoteEnv(t *testing.T, scripts map[string]string, relationships map[string]any) []string { + t.Helper() + binDir := t.TempDir() + for name, script := range scripts { + require.NoError(t, os.WriteFile(filepath.Join(binDir, name), []byte("#!/bin/bash\n"+script), 0o755)) + } + env := append(os.Environ(), "PATH="+binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + if relationships != nil { + data, err := json.Marshal(relationships) + require.NoError(t, err) + env = append(env, "PLATFORM_RELATIONSHIPS="+base64.StdEncoding.EncodeToString(data)) + } + return env +} + +// sshEnv returns an active "main" environment with a deployment, and an SSH URL for each app and worker. +func sshEnv(d *mockapi.Deployment) *mockapi.Environment { + env := makeEnv(layoutProjectID, "main", "production", "active", nil) + if d.Services == nil { + d.Services = map[string]mockapi.App{} + } + if d.Workers == nil { + d.Workers = map[string]mockapi.Worker{} + } + if d.Routes == nil { + d.Routes = map[string]any{} + } + d.Links = mockapi.MakeHALLinks("self=/projects/" + layoutProjectID + "/environments/main/deployment/current") + env.SetCurrentDeployment(d) + for name := range d.WebApps { + env.Links["pf:ssh:"+name] = mockapi.HALLink{HREF: "ssh://" + layoutProjectID + "-main--" + name + "@ssh.cli-tests.example.com"} + } + for name := range d.Workers { + env.Links["pf:ssh:"+name] = mockapi.HALLink{HREF: "ssh://" + layoutProjectID + "-main--" + name + "@ssh.cli-tests.example.com"} + } + return env } // setHook writes a hook script in the served repository. diff --git a/integration-tests/ssh_test.go b/integration-tests/ssh_test.go index a9c56ac01..01c97f43a 100644 --- a/integration-tests/ssh_test.go +++ b/integration-tests/ssh_test.go @@ -1,13 +1,16 @@ package tests import ( + "fmt" "net/http/httptest" "os" "os/exec" "strconv" + "strings" "testing" "github.com/stretchr/testify/assert" + "golang.org/x/crypto/ssh" "github.com/upsun/cli/pkg/mockapi" "github.com/upsun/cli/pkg/mockssh" @@ -82,3 +85,45 @@ func TestSSH(t *testing.T) { assert.ErrorAs(t, err, &exitErr) assert.Equal(t, 2, exitErr.ExitCode()) } + +func TestSSHAppSelection(t *testing.T) { + t.Parallel() + p := setUpProject(t, sshEnv(&mockapi.Deployment{ + WebApps: map[string]mockapi.App{ + "app": {Name: "app", Type: "golang:1.23", Size: "M", Mounts: map[string]mockapi.Mount{}}, + "admin": {Name: "admin", Type: "php:8.4", Size: "M", Mounts: map[string]mockapi.Mount{}}, + }, + Workers: map[string]mockapi.Worker{ + "app--queue": {App: mockapi.App{Name: "app--queue", Type: "golang:1.23", Size: "S", Mounts: map[string]mockapi.Mount{}}}, + }, + })) + // The server prints the SSH user, which identifies the container. + p.serveSSH(t, func(conn ssh.ConnMetadata, command string, io mockssh.CommandIO) int { + _, _ = fmt.Fprintf(io.StdOut, "%s: %s\n", conn.User(), command) + return 0 + }) + f := p.f + sshUser := layoutProjectID + "-main--" + sshURL := func(name string) string { return sshUser + name + "@ssh.cli-tests.example.com" } + + assert.Equal(t, sshURL("admin"), f.Run("ssh", "-p", layoutProjectID, "-e", "main", "--app", "admin", "--pipe")) + assert.Equal(t, sshURL("app--queue"), + f.Run("ssh", "-p", layoutProjectID, "-e", "main", "--worker", "app--queue", "--pipe")) + assert.Equal(t, sshURL("app--queue"), + f.Run("ssh", "-p", layoutProjectID, "-e", "main", "--app", "app", "--worker", "queue", "--pipe")) + + all := strings.Split(strings.TrimSpace(f.Run("ssh", "-p", layoutProjectID, "-e", "main", "--all")), "\n") + assert.ElementsMatch(t, []string{sshURL("app"), sshURL("admin"), sshURL("app--queue")}, all) + + assert.Equal(t, sshUser+"admin: echo hi\n", f.Run("ssh", "-p", layoutProjectID, "-e", "main", "-A", "admin", "echo hi")) + assert.Equal(t, sshUser+"app--queue: ls -a\n", + f.Run("ssh", "-p", layoutProjectID, "-e", "main", "--worker", "app--queue", "--", "ls", "-a")) + + _, stdErr, err := f.RunCombinedOutput("ssh", "-p", layoutProjectID, "-e", "main", "--app", "missing", "--pipe") + assert.Error(t, err) + assert.Contains(t, stdErr, "missing") + + _, stdErr, err = f.RunCombinedOutput("ssh", "-p", layoutProjectID, "-e", "main", "--pipe") + assert.Error(t, err) + assert.Contains(t, stdErr, "Specifying the --app or --worker is required in non-interactive mode") +} diff --git a/legacy/src/Command/Environment/EnvironmentSshCommand.php b/legacy/src/Command/Environment/EnvironmentSshCommand.php index 10bea35fb..af0147545 100644 --- a/legacy/src/Command/Environment/EnvironmentSshCommand.php +++ b/legacy/src/Command/Environment/EnvironmentSshCommand.php @@ -56,7 +56,11 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { try { - $selection = $this->selector->getSelection($input, new SelectorConfig(chooseEnvFilter: SelectorConfig::filterEnvsMaybeActive())); + // The --all option lists every app, so it does not need a container. + $selection = $this->selector->getSelection($input, new SelectorConfig( + chooseEnvFilter: SelectorConfig::filterEnvsMaybeActive(), + selectRemoteContainer: !Option::bool($input, 'all'), + )); $environment = $selection->getEnvironment(); if (Option::bool($input, 'all')) { From 6f6626f850cfdac777fa5f5090bc20e9aecb5d1a Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 13:43:28 +0100 Subject: [PATCH 02/13] test: cover ssh-cert:load and ssh-cert:info Test certificate generation and reuse, --new, --new-key, --refresh-only, the SSH configuration files, and ssh-cert:info --no-refresh with missing, expired and mismatched certificates. The mock auth server's certificate key ID and lifetime are now configurable, as the CLI only reuses a certificate whose key ID is the user ID and which expires in more than 2 minutes. Co-Authored-By: Claude Opus 5.5 --- integration-tests/ssh_cert_test.go | 93 ++++++++++++++++++++++++++++++ pkg/mockapi/auth_server.go | 40 ++++++++++++- 2 files changed, 131 insertions(+), 2 deletions(-) diff --git a/integration-tests/ssh_cert_test.go b/integration-tests/ssh_cert_test.go index bcb6ad9ce..7f4a07b27 100644 --- a/integration-tests/ssh_cert_test.go +++ b/integration-tests/ssh_cert_test.go @@ -2,9 +2,15 @@ package tests import ( "net/http/httptest" + "os" + "path/filepath" + "strconv" + "strings" "testing" + "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/upsun/cli/pkg/mockapi" ) @@ -42,3 +48,90 @@ func TestSSHCerts(t *testing.T) { }) } } + +func TestSSHCertLoad(t *testing.T) { + t.Parallel() + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + myUserID := "my-user-id" + apiHandler := mockapi.NewHandler(t) + apiHandler.SetMyUser(&mockapi.User{ID: myUserID}) + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + _, stdErr, err := f.RunCombinedOutput("ssh-cert:info", "--no-refresh") + assertExitCode(t, 1, err) + assert.Contains(t, stdErr, "No valid SSH certificate found.") + + // The CLI treats a certificate as expired 2 minutes early. + authServer.SetSSHCertificate(myUserID, time.Minute) + _, stdErr, err = f.RunCombinedOutput("ssh-cert:load", "--refresh-only") + require.NoError(t, err) + assert.Contains(t, stdErr, "Generating SSH certificate...") + assert.NoFileExists(t, filepath.Join(f.home, ".ssh", "config")) + + _, stdErr, err = f.RunCombinedOutput("ssh-cert:info", "--no-refresh") + assertExitCode(t, 1, err) + assert.Contains(t, stdErr, "No valid SSH certificate found.") + + // A certificate with the wrong key ID is not valid. + authServer.SetSSHCertificate("other-user-id", time.Hour) + f.Run("ssh-cert:load", "--new") + _, _, err = f.RunCombinedOutput("ssh-cert:info", "--no-refresh") + assertExitCode(t, 1, err) + + // Without --no-refresh, an invalid certificate is regenerated. + authServer.SetSSHCertificate(myUserID, time.Hour) + before := time.Now() + output := f.Run("ssh-cert:info") + assert.Contains(t, output, "key_id: "+myUserID+"\n") + assert.Equal(t, 3, authServer.SSHCertificateCount()) + + validBefore, err := strconv.ParseInt(strings.TrimSpace(f.Run("ssh-cert:info", "-P", "valid_before", "--date-fmt", "U")), 10, 64) + require.NoError(t, err) + assert.WithinDuration(t, before.Add(time.Hour), time.Unix(validBefore, 0), time.Minute) + + certFile := strings.TrimSpace(f.Run("ssh-cert:info", "-P", "filename")) + keyFile := strings.TrimSpace(f.Run("ssh-cert:info", "-P", "key_filename")) + assert.Equal(t, keyFile+"-cert.pub", certFile) + assert.Equal(t, 3, authServer.SSHCertificateCount(), "a valid certificate is reused") + + // A valid certificate is reused, and the SSH configuration is written. + _, stdErr, err = f.RunCombinedOutput("ssh-cert:load") + require.NoError(t, err) + assert.Contains(t, stdErr, "A valid SSH certificate exists") + assert.Equal(t, 3, authServer.SSHCertificateCount()) + userConfig := readFile(t, filepath.Join(f.home, ".ssh", "config")) + assert.Contains(t, userConfig, "Host *.cli-tests.example.com\n") + assert.Contains(t, userConfig, "Include "+filepath.Join(f.home, ".platform-test-cli", "ssh", "*.config")) + assert.Contains(t, readFile(t, filepath.Join(f.home, ".platform-test-cli", "ssh", "session.config")), "Include ") + sessionConfig := readFile(t, filepath.Join(filepath.Dir(certFile), "config")) + assert.Contains(t, sessionConfig, "CertificateFile "+certFile) + + // --new refreshes the certificate, keeping the key. + publicKey := readFile(t, keyFile+".pub") + cert := readFile(t, certFile) + _, stdErr, err = f.RunCombinedOutput("ssh-cert:load", "--new") + require.NoError(t, err) + assert.Contains(t, stdErr, "Generating SSH certificate...") + assert.Equal(t, 4, authServer.SSHCertificateCount()) + assert.Equal(t, publicKey, readFile(t, keyFile+".pub")) + assert.NotEqual(t, cert, readFile(t, certFile)) + + // --new-key also generates a new key. + _, stdErr, err = f.RunCombinedOutput("ssh-cert:load", "--new-key") + require.NoError(t, err) + assert.Contains(t, stdErr, "The --new-key option is deprecated") + assert.Equal(t, 5, authServer.SSHCertificateCount()) + assert.NotEqual(t, publicKey, readFile(t, keyFile+".pub")) +} + +func readFile(t *testing.T, filename string) string { + t.Helper() + b, err := os.ReadFile(filename) + require.NoError(t, err) + return string(b) +} diff --git a/pkg/mockapi/auth_server.go b/pkg/mockapi/auth_server.go index c19f50298..123d04c61 100644 --- a/pkg/mockapi/auth_server.go +++ b/pkg/mockapi/auth_server.go @@ -43,6 +43,41 @@ type AuthServer struct { accessCount int reuseDetected bool revokedFamilies map[string]bool + + sshCertMu sync.Mutex + sshCertKeyID string + sshCertLifetime time.Duration + sshCertCount int +} + +// SetSSHCertificate sets the key ID and lifetime of issued SSH certificates (default: "test-key-id" and 1 minute). +// The legacy CLI only reuses a certificate if its key ID is the user ID, and it expires more than 2 minutes later. +func (s *AuthServer) SetSSHCertificate(keyID string, lifetime time.Duration) { + s.sshCertMu.Lock() + defer s.sshCertMu.Unlock() + s.sshCertKeyID = keyID + s.sshCertLifetime = lifetime +} + +// SSHCertificateCount returns the number of SSH certificates issued. +func (s *AuthServer) SSHCertificateCount() int { + s.sshCertMu.Lock() + defer s.sshCertMu.Unlock() + return s.sshCertCount +} + +func (s *AuthServer) nextSSHCertificate() (keyID string, lifetime time.Duration) { + s.sshCertMu.Lock() + defer s.sshCertMu.Unlock() + s.sshCertCount++ + keyID, lifetime = s.sshCertKeyID, s.sshCertLifetime + if keyID == "" { + keyID = "test-key-id" + } + if lifetime == 0 { + lifetime = time.Minute + } + return keyID, lifetime } // SetTokenLifetime sets the lifetime of issued access tokens (default: 1 hour). @@ -315,13 +350,14 @@ func NewAuthServer(t *testing.T) *AuthServer { extensions["permit-port-forwarding"] = "" extensions["permit-pty"] = "" extensions["permit-user-rc"] = "" + keyID, lifetime := srv.nextSSHCertificate() cert := &ssh.Certificate{ Key: key, Serial: 0, CertType: ssh.UserCert, - KeyId: "test-key-id", + KeyId: keyID, ValidAfter: uint64(time.Now().Add(-1 * time.Second).Unix()), - ValidBefore: uint64(time.Now().Add(time.Minute).Unix()), + ValidBefore: uint64(time.Now().Add(lifetime).Unix()), Permissions: ssh.Permissions{ Extensions: extensions, }, From b5bfd4a4effdfa0cfcf64719b9119e85d999d2a4 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 13:49:14 +0100 Subject: [PATCH 03/13] test: cover database, mount and service commands over SSH Run the commands against the mock SSH server, with fake remote clients (shell scripts on the remote PATH) which print their arguments: - db:dump: default and --file names, --gzip, --stdout, --directory, table and schema options, MariaDB and PostgreSQL, and remote failures - db:sql: queries, --raw, --schema, and piped input - db:size: PostgreSQL and MariaDB usage - mount:size: df and du output parsing - mount:upload and mount:download with rsync: --exclude and --delete - service:redis-cli and service:mongo:* commands Co-Authored-By: Claude Opus 5.5 --- integration-tests/db_ssh_test.go | 152 ++++++++++++++++++++++++ integration-tests/mount_ssh_test.go | 110 +++++++++++++++++ integration-tests/project_setup_test.go | 13 +- integration-tests/service_ssh_test.go | 80 +++++++++++++ integration-tests/ssh_cert_test.go | 3 +- integration-tests/ssh_test.go | 7 +- 6 files changed, 358 insertions(+), 7 deletions(-) create mode 100644 integration-tests/db_ssh_test.go create mode 100644 integration-tests/mount_ssh_test.go create mode 100644 integration-tests/service_ssh_test.go diff --git a/integration-tests/db_ssh_test.go b/integration-tests/db_ssh_test.go new file mode 100644 index 000000000..171d5a0f6 --- /dev/null +++ b/integration-tests/db_ssh_test.go @@ -0,0 +1,152 @@ +package tests + +import ( + "bytes" + "compress/gzip" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" + "github.com/upsun/cli/pkg/mockssh" +) + +// fakeDBClients are remote scripts which print their name and arguments, and any input. +var fakeDBClients = map[string]string{ + "mariadb": `echo "mariadb $*"; cat`, + "mariadb-dump": `echo "mariadb-dump $*"`, + "psql": `echo "psql $*"; cat`, + "pg_dump": `echo "pg_dump $*"`, +} + +func dbRelationships() map[string]any { + return map[string]any{ + "database": []any{map[string]any{ + "service": "db", "rel": "mysql", "scheme": "mysql", "type": "mariadb:11.4", + "host": "database.internal", "port": 3306, "username": "user", "password": "", "path": "main", + }}, + "pg": []any{map[string]any{ + "service": "pg", "rel": "postgresql", "scheme": "pgsql", "type": "postgresql:16", + "host": "pg.internal", "port": 5432, "username": "main", "password": "secret", "path": "main", + }}, + } +} + +// setUpDBProject serves an app with database relationships over SSH, with fake database clients. +func setUpDBProject(t *testing.T, clients map[string]string) *testProject { + t.Helper() + p := setUpProject(t, sshEnv(&mockapi.Deployment{ + WebApps: map[string]mockapi.App{ + "app": {Name: "app", Type: "golang:1.23", Size: "M", Mounts: map[string]mockapi.Mount{}}, + }, + Services: map[string]mockapi.App{ + "db": {Name: "db", Type: "mariadb:11.4", Size: "M", Disk: 1024}, + "pg": {Name: "pg", Type: "postgresql:16", Size: "M", Disk: 2048}, + }, + })) + p.serveSSH(t, mockssh.ExecHandler(t.TempDir(), remoteEnv(t, clients, dbRelationships()))) + return p +} + +func TestDbDump(t *testing.T) { + t.Parallel() + p := setUpDBProject(t, fakeDBClients) + f := p.f + args := []string{"-p", layoutProjectID, "-e", "main"} + mariadbArgs := "--user=user --password= --host=database.internal --port=3306 main" + + _, stdErr, err := f.RunCombinedOutput(append([]string{"db:dump", "-r", "database"}, args...)...) + require.NoError(t, err) + dumpFile := filepath.Join(f.dir, layoutProjectID+"--main-xyz--db--main--dump.sql") + assert.Contains(t, stdErr, "Creating SQL dump file: "+dumpFile) + assert.Equal(t, "mariadb-dump --single-transaction "+mariadbArgs+"\n", readFile(t, dumpFile)) + + // An existing file is overwritten in non-interactive mode. + _, stdErr, err = f.RunCombinedOutput(append([]string{"db:dump", "-r", "database"}, args...)...) + require.NoError(t, err) + assert.Contains(t, stdErr, "File exists: "+dumpFile+". Overwrite? [Y/n] y") + + _, stdErr, err = f.RunCombinedOutput(append([]string{"db:dump", "-r", "database", "--gzip", "-f", "dump.sql.gz", + "--table", "users", "--table", "posts", "--exclude-table", "cache"}, args...)...) + require.NoError(t, err) + assert.Contains(t, stdErr, "Creating gzipped SQL dump file: "+filepath.Join(f.dir, "dump.sql.gz")) + assert.Equal(t, "mariadb-dump --single-transaction "+mariadbArgs+ + " --ignore-table=main.cache --tables users posts\n", gunzipFile(t, filepath.Join(f.dir, "dump.sql.gz"))) + + tableDir := t.TempDir() + f.Run(append([]string{"db:dump", "-r", "database", "--table", "users", "--schema-only", "-d", tableDir}, args...)...) + assert.Equal(t, "mariadb-dump --single-transaction "+mariadbArgs+" --no-data --tables users\n", + readFile(t, filepath.Join(tableDir, layoutProjectID+"--main-xyz--db--main--users--schema--dump.sql"))) + + output := f.Run(append([]string{"db:dump", "-r", "pg", "--stdout", "--schema-only", + "--table", "users", "--exclude-table", "cache", "--charset", "UTF8"}, args...)...) + assert.Equal(t, "pg_dump --no-owner --if-exists --clean --blobs postgresql://main:secret@pg.internal:5432/main"+ + " --schema-only --table=users --exclude-table=cache --encoding=UTF8\n", output) + + // The remote exit code is returned. + failing := setUpDBProject(t, map[string]string{"mariadb-dump": "echo 'dump failed' >&2; exit 3"}) + _, stdErr, err = failing.f.RunCombinedOutput(append([]string{"db:dump", "-r", "database", "--stdout"}, args...)...) + assertExitCode(t, 3, err) + assert.Contains(t, stdErr, "dump failed") +} + +func TestDbSQL(t *testing.T) { + t.Parallel() + p := setUpDBProject(t, fakeDBClients) + f := p.f + args := []string{"-p", layoutProjectID, "-e", "main"} + + assert.Equal(t, "mariadb --no-auto-rehash --user=user --password= --host=database.internal --port=3306 main"+ + " --execute SELECT 1\n", f.Run(append([]string{"db:sql", "-r", "database", "SELECT 1"}, args...)...)) + + assert.Equal(t, "mariadb --no-auto-rehash --user=user --password= --host=database.internal --port=3306 other"+ + " --batch --raw --execute SELECT 2\n", + f.Run(append([]string{"db:sql", "-r", "database", "--schema", "other", "--raw", "SELECT 2"}, args...)...)) + + assert.Equal(t, "psql postgresql://main:secret@pg.internal:5432/main -t -c SELECT 3\n", + f.Run(append([]string{"sql", "-r", "pg", "--raw", "SELECT 3"}, args...)...)) + + // A query can be piped in. + f.stdin = strings.NewReader("SELECT 4;\n") + assert.Equal(t, "psql postgresql://main:secret@pg.internal:5432/main\nSELECT 4;\n", + f.Run(append([]string{"db:sql", "-r", "pg"}, args...)...)) +} + +func TestDbSize(t *testing.T) { + t.Parallel() + // Each query returns 50 MiB. + p := setUpDBProject(t, map[string]string{ + "mariadb": "cat > /dev/null; echo 52428800", + "psql": "cat > /dev/null; echo 52428800", + }) + f := p.f + args := []string{"-p", layoutProjectID, "-e", "main"} + + output, stdErr, err := f.RunCombinedOutput(append([]string{"db:size", "-r", "pg", "--format", "csv"}, args...)...) + require.NoError(t, err) + assert.Contains(t, stdErr, "Checking database service pg...") + assert.Equal(t, "Allocated disk,Estimated usage,% used\n2147483648,52428800,2\n", output) + + output = f.Run(append([]string{"db:size", "-r", "pg", "--no-header"}, args...)...) + assert.Contains(t, output, "| 2.0 GiB | 50.0 MiB | ~ 2% |") + + // The MySQL usage includes InnoDB and other tables separately. + output = f.Run(append([]string{"db:size", "-r", "database", "--format", "csv", "--no-header"}, args...)...) + assert.Equal(t, "1073741824,104857600,10\n", output) +} + +func gunzipFile(t *testing.T, filename string) string { + t.Helper() + b, err := os.ReadFile(filename) + require.NoError(t, err) + r, err := gzip.NewReader(bytes.NewReader(b)) + require.NoError(t, err) + out, err := io.ReadAll(r) + require.NoError(t, err) + return string(out) +} diff --git a/integration-tests/mount_ssh_test.go b/integration-tests/mount_ssh_test.go new file mode 100644 index 000000000..6b1676743 --- /dev/null +++ b/integration-tests/mount_ssh_test.go @@ -0,0 +1,110 @@ +package tests + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" + "github.com/upsun/cli/pkg/mockssh" +) + +func mountsDeployment() *mockapi.Deployment { + return &mockapi.Deployment{ + WebApps: map[string]mockapi.App{ + "app": {Name: "app", Type: "golang:1.23", Size: "M", Mounts: map[string]mockapi.Mount{ + "/var/log": {Source: "storage", SourcePath: "logs"}, + "web/uploads": {Source: "storage", SourcePath: "uploads"}, + "tmp": {Source: "tmp", SourcePath: "tmp"}, + }}, + }, + } +} + +func TestMountSize(t *testing.T) { + t.Parallel() + appDir := t.TempDir() + p := setUpProject(t, sshEnv(mountsDeployment())) + // The df output has two mounts on one disk and one on another. Each du output is the path's length in KiB. + env := remoteEnv(t, map[string]string{ + "df": `cat < Date: Thu, 15 Oct 2026 13:50:38 +0100 Subject: [PATCH 04/13] docs: mention SSH integration test helpers Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 74d3d2964..f6f4225b9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -155,7 +155,7 @@ make download-php Tests use github.com/stretchr/testify for assertions. Table-driven tests are preferred with a "cases" slice containing simple test case structs. -Integration tests (`integration-tests/`) call `t.Parallel()`, so they must not share state: no `t.Setenv` or unsynchronized package variables, and use `cmdFactory.useConfig` to change the CLI config file. Write output with `t.Log` (e.g. `mockapi.RequestLogger`), not to stdout or stderr: it is then attributed to its test, and shown only on failure or with `-v`. Tests of environment operations use `setUpProject`, and tests of Git commands (push, clone, fetch) use `setUpGitProject`, which also serves a bare repository over the mock SSH server. +Integration tests (`integration-tests/`) call `t.Parallel()`, so they must not share state: no `t.Setenv` or unsynchronized package variables, and use `cmdFactory.useConfig` to change the CLI config file. Write output with `t.Log` (e.g. `mockapi.RequestLogger`), not to stdout or stderr: it is then attributed to its test, and shown only on failure or with `-v`. Tests of environment operations use `setUpProject`, and tests of Git commands (push, clone, fetch) use `setUpGitProject`, which also serves a bare repository over the mock SSH server. Tests of other SSH commands use `serveSSH` with an app from `sshEnv`, and fake remote executables from `remoteEnv`. ### Configuration From 1c5f6764ffeaec6edc81704878c0b0f77c661a31 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 13:54:58 +0100 Subject: [PATCH 05/13] test(mockssh): serve concurrent connections, SFTP and port forwarding The mock SSH server handled one connection at a time, so a long-lived connection such as a tunnel blocked others. It now handles connections and channels concurrently, and Stop closes open connections and waits for their handlers. Add optional support for: - the SFTP subsystem (SFTPDir), with github.com/pkg/sftp, used by scp - local port forwarding ("direct-tcpip" channels), via DialTCP - remote forwarding of Unix sockets ("streamlocal-forward@openssh.com"), via ListenUnix, as used by environment:xdebug Add an integration test for environment:scp. Co-Authored-By: Claude Opus 5.5 --- go.mod | 2 + go.sum | 4 + integration-tests/environment_scp_test.go | 53 ++++++ pkg/mockssh/channels.go | 219 ++++++++++++++++++++++ pkg/mockssh/server.go | 155 ++++++--------- 5 files changed, 336 insertions(+), 97 deletions(-) create mode 100644 integration-tests/environment_scp_test.go create mode 100644 pkg/mockssh/channels.go diff --git a/go.mod b/go.mod index 6f4b528df..0eb8b999b 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/godbus/dbus/v5 v5.2.3 github.com/gofrs/flock v0.13.1 github.com/oklog/ulid/v2 v2.1.2 + github.com/pkg/sftp v1.13.11 github.com/platformsh/platformify v0.5.0 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 @@ -91,6 +92,7 @@ require ( github.com/kevinburke/ssh_config v1.6.0 // indirect github.com/klauspost/compress v1.18.7 // indirect github.com/klauspost/pgzip v1.2.6 // indirect + github.com/kr/fs v0.1.0 // indirect github.com/leodido/go-urn v1.5.0 // indirect github.com/lucasb-eyer/go-colorful v1.4.1 // indirect github.com/mailru/easyjson v0.9.2 // indirect diff --git a/go.sum b/go.sum index 698cd3da4..e16d23dbf 100644 --- a/go.sum +++ b/go.sum @@ -173,6 +173,8 @@ github.com/klauspost/compress v1.18.7/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl github.com/klauspost/cpuid v1.2.0/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek= github.com/klauspost/pgzip v1.2.6 h1:8RXeL5crjEUFnR2/Sn6GJNWtSQ3Dk8pq4CL3jvdDyjU= github.com/klauspost/pgzip v1.2.6/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs= +github.com/kr/fs v0.1.0 h1:Jskdu9ieNAYnjxsi0LbQp1ulIKZV1LAFgK1tWhpZgl8= +github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= @@ -226,6 +228,8 @@ github.com/pjbgf/sha1cd v0.7.0 h1:ZRNPKHj+gfkLBf0KJv/p1Hmz+7bqCT5o311YX0nq+DA= github.com/pjbgf/sha1cd v0.7.0/go.mod h1:pKR5Li+qTCo+ebqITZfwPlJGoCFCvSO00qxjbNtTUFQ= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pkg/sftp v1.13.11 h1:0N92SLTB8JqASJB14ZLHHzFnBV8mG9zw4K7jghEFWuE= +github.com/pkg/sftp v1.13.11/go.mod h1:uNkH9roSXglNJqM+glJJi+TQXQUm0fXFWqCFmT8hsN0= github.com/platformsh/platformify v0.5.0 h1:i9cRbM0K1f14Yr0dJ6dhYq9BcmZb8fsx/mpYSnw8ssA= github.com/platformsh/platformify v0.5.0/go.mod h1:fgmCcfQfHbhe1oXsIdIhpnniyZu8IdIMOlcBAa/ygic= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= diff --git a/integration-tests/environment_scp_test.go b/integration-tests/environment_scp_test.go new file mode 100644 index 000000000..874f47ff7 --- /dev/null +++ b/integration-tests/environment_scp_test.go @@ -0,0 +1,53 @@ +package tests + +import ( + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" + "github.com/upsun/cli/pkg/mockssh" +) + +func TestEnvironmentScp(t *testing.T) { + t.Parallel() + if _, err := exec.LookPath("scp"); err != nil { + t.Skip("scp is not installed") + } + remoteDir := t.TempDir() + writeFiles(t, remoteDir, map[string]string{"remote.txt": "remote", "logs/a.log": "a", "logs/b.log": "b"}) + + p := setUpProject(t, sshEnv(&mockapi.Deployment{ + WebApps: map[string]mockapi.App{ + "app": {Name: "app", Type: "golang:1.23", Size: "M", Mounts: map[string]mockapi.Mount{}}, + }, + })) + // Older scp clients run "scp -t" or "scp -f" instead of using SFTP. + sshServer := p.serveSSH(t, mockssh.ExecHandler(remoteDir, nil)) + sshServer.SFTPDir = remoteDir + f := p.f + f.dir = t.TempDir() + writeFiles(t, f.dir, map[string]string{"local.txt": "local"}) + args := []string{"scp", "-p", layoutProjectID, "-e", "main"} + + f.Run(append(args, "--", "local.txt", "remote:uploaded.txt")...) + assert.Equal(t, "local", readFile(t, filepath.Join(remoteDir, "uploaded.txt"))) + + f.Run(append(args, "--", "remote:remote.txt", "downloaded.txt")...) + assert.Equal(t, "remote", readFile(t, filepath.Join(f.dir, "downloaded.txt"))) + + f.Run(append(args, "-r", "--", "remote:logs", "logs")...) + assert.Equal(t, "a", readFile(t, filepath.Join(f.dir, "logs", "a.log"))) + assert.Equal(t, "b", readFile(t, filepath.Join(f.dir, "logs", "b.log"))) + + _, stdErr, err := f.RunCombinedOutput(append(args, "--", "remote:missing.txt", ".")...) + require.Error(t, err) + assert.Contains(t, stdErr, "missing.txt") + + _, stdErr, err = f.RunCombinedOutput(append(args, "--", "local.txt", "other.txt")...) + require.Error(t, err) + assert.Contains(t, stdErr, `At least one argument needs to contain the "remote:" prefix`) +} diff --git a/pkg/mockssh/channels.go b/pkg/mockssh/channels.go new file mode 100644 index 000000000..92e3ae8c3 --- /dev/null +++ b/pkg/mockssh/channels.go @@ -0,0 +1,219 @@ +package mockssh + +import ( + "errors" + "io" + "net" + "sync" + "time" + + "github.com/pkg/sftp" + "golang.org/x/crypto/ssh" +) + +func (s *Server) handleConn(conn net.Conn, config *ssh.ServerConfig) { + t := s.t + sshConn, chans, reqs, err := ssh.NewServerConn(conn, config) + if err != nil { + t.Logf("Handshake failed: %v", err) + return + } + defer sshConn.Close() + + t.Logf("Handling SSH connection from %s", sshConn.RemoteAddr()) + + // Listeners for remote forwarding, closed with the connection. + var ( + listenersMu sync.Mutex + listeners []net.Listener + ) + var wg sync.WaitGroup + defer func() { + listenersMu.Lock() + for _, l := range listeners { + _ = l.Close() + } + listenersMu.Unlock() + wg.Wait() + }() + + wg.Go(func() { + for req := range reqs { + if req.Type == "streamlocal-forward@openssh.com" { + if l := s.forwardUnix(sshConn, req, &wg); l != nil { + listenersMu.Lock() + listeners = append(listeners, l) + listenersMu.Unlock() + } + continue + } + if req.WantReply { + _ = req.Reply(false, nil) + } + } + }) + + for newChannel := range chans { + switch newChannel.ChannelType() { + case "session": + wg.Go(func() { s.handleSession(sshConn, newChannel) }) + case "direct-tcpip": + wg.Go(func() { s.handleDirectTCPIP(newChannel) }) + default: + _ = newChannel.Reject(ssh.UnknownChannelType, "unknown channel type") + } + } +} + +// handleSession handles a session channel, which runs a command or the SFTP subsystem. +func (s *Server) handleSession(conn ssh.ConnMetadata, newChannel ssh.NewChannel) { + t := s.t + channel, requests, err := newChannel.Accept() + if err != nil { + t.Errorf("Failed to accept channel: %v", err) + return + } + defer channel.Close() + + exitWithStatus := make(chan int, 1) + go func() { + for req := range requests { + var payload struct{ Value string } + switch { + case req.Type == "exec" && ssh.Unmarshal(req.Payload, &payload) == nil: + _ = req.Reply(true, nil) + t.Logf("Handling command: %s", payload.Value) + exitWithStatus <- s.CommandHandler(conn, payload.Value, CommandIO{ + StdIn: channel, + StdOut: channel, + StdErr: channel.Stderr(), + }) + return + case req.Type == "subsystem" && s.SFTPDir != "" && + ssh.Unmarshal(req.Payload, &payload) == nil && payload.Value == "sftp": + _ = req.Reply(true, nil) + t.Logf("Handling SFTP session in %s", s.SFTPDir) + exitWithStatus <- serveSFTP(channel, s.SFTPDir) + return + default: + if req.WantReply { + _ = req.Reply(false, nil) + } + } + } + }() + + select { + case status := <-exitWithStatus: + _, err = channel.SendRequest("exit-status", false, ssh.Marshal(struct{ Status uint32 }{uint32(status)})) + if err != nil { + t.Errorf("Failed to send exit status: %v", err) + } + case <-time.After(10 * time.Second): + t.Error("Timed out") + } +} + +func serveSFTP(channel ssh.Channel, dir string) int { + server, err := sftp.NewServer(channel, sftp.WithServerWorkingDirectory(dir)) + if err != nil { + return 1 + } + if err := server.Serve(); err != nil && !errors.Is(err, io.EOF) { + return 1 + } + return 0 +} + +// handleDirectTCPIP handles local port forwarding, if DialTCP is set. +func (s *Server) handleDirectTCPIP(newChannel ssh.NewChannel) { + t := s.t + if s.DialTCP == nil { + _ = newChannel.Reject(ssh.Prohibited, "port forwarding is disabled") + return + } + // See: https://datatracker.ietf.org/doc/html/rfc4254#section-7.2 + var payload struct { + Host string + Port uint32 + OriginHost string + OriginPort uint32 + } + if err := ssh.Unmarshal(newChannel.ExtraData(), &payload); err != nil { + _ = newChannel.Reject(ssh.ConnectionFailed, "invalid payload") + return + } + t.Logf("Forwarding a connection to %s:%d", payload.Host, payload.Port) + target, err := s.DialTCP(payload.Host, int(payload.Port)) + if err != nil { + _ = newChannel.Reject(ssh.ConnectionFailed, err.Error()) + return + } + channel, requests, err := newChannel.Accept() + if err != nil { + _ = target.Close() + return + } + go ssh.DiscardRequests(requests) + proxy(channel, target) +} + +// forwardUnix handles a request for remote forwarding of a Unix socket, if ListenUnix is set. +// It returns the listener, or nil if the request is rejected. +func (s *Server) forwardUnix(conn *ssh.ServerConn, req *ssh.Request, wg *sync.WaitGroup) net.Listener { + t := s.t + var payload struct{ Path string } + if s.ListenUnix == nil || ssh.Unmarshal(req.Payload, &payload) != nil { + _ = req.Reply(false, nil) + return nil + } + l, err := s.ListenUnix(payload.Path) + if err != nil { + t.Logf("Failed to listen at %s: %v", payload.Path, err) + _ = req.Reply(false, nil) + return nil + } + _ = req.Reply(true, nil) + t.Logf("Forwarding connections from the socket %s", payload.Path) + wg.Go(func() { + for { + c, err := l.Accept() + if err != nil { + return + } + wg.Go(func() { + // See: https://github.com/openssh/openssh-portable/blob/master/PROTOCOL + channel, requests, err := conn.OpenChannel("forwarded-streamlocal@openssh.com", + ssh.Marshal(struct{ Path, Reserved string }{Path: payload.Path})) + if err != nil { + _ = c.Close() + return + } + go ssh.DiscardRequests(requests) + proxy(channel, c) + }) + } + }) + return l +} + +// proxy copies data in both directions until either side is closed. +func proxy(channel ssh.Channel, conn net.Conn) { + done := make(chan struct{}, 2) + go func() { + _, _ = io.Copy(channel, conn) + _ = channel.CloseWrite() + done <- struct{}{} + }() + go func() { + _, _ = io.Copy(conn, channel) + if c, ok := conn.(interface{ CloseWrite() error }); ok { + _ = c.CloseWrite() + } + done <- struct{}{} + }() + <-done + <-done + _ = channel.Close() + _ = conn.Close() +} diff --git a/pkg/mockssh/server.go b/pkg/mockssh/server.go index d82d2732d..c86ba403a 100644 --- a/pkg/mockssh/server.go +++ b/pkg/mockssh/server.go @@ -15,7 +15,6 @@ import ( "os/exec" "sync" "testing" - "time" "golang.org/x/crypto/ssh" ) @@ -35,9 +34,25 @@ type Server struct { // be reused from custom handlers. CommandHandler CommandHandler + // SFTPDir enables the SFTP subsystem, with this working directory. + SFTPDir string + + // DialTCP enables local port forwarding ("direct-tcpip" channels, as with "ssh -L"), + // by connecting to the requested host and port. Otherwise, forwarding is rejected. + DialTCP func(host string, port int) (net.Conn, error) + + // ListenUnix enables remote forwarding of Unix sockets (as with "ssh -R /path:host:port"), + // by listening at the requested path. Otherwise, forwarding is rejected. + ListenUnix func(path string) (net.Listener, error) + // listener and port are set after Start. listener net.Listener port int + + // conns are the open connections, which Stop closes. + connsMu sync.Mutex + conns map[net.Conn]struct{} + wg sync.WaitGroup } type CommandIO struct { @@ -63,7 +78,7 @@ func NewServer(t *testing.T, authorityEndpoint string) (*Server, error) { return nil, fmt.Errorf("failed to fetch authority keys: %v", err) } - s := &Server{t: t, hostKey: hk} + s := &Server{t: t, hostKey: hk, conns: make(map[net.Conn]struct{})} s.CommandHandler = ExecHandler("", nil) s.CertChecker = s.defaultCertChecker() s.CertAuthorityKeys = keys @@ -106,48 +121,61 @@ func (s *Server) start() error { } t.Logf("Test SSH server listening at %s", listener.Addr()) - go func(l net.Listener) { + s.wg.Go(func() { for { - conn, err := l.Accept() + conn, err := listener.Accept() if err != nil { - if errors.Is(err, net.ErrClosed) { - break + if !errors.Is(err, net.ErrClosed) { + t.Errorf("Failed to accept connection: %v", err) } - t.Errorf("Failed to accept connection: %v", err) - continue + return } - - sshConn, chans, reqs, err := ssh.NewServerConn(conn, config) - if err != nil { - t.Logf("Handshake failed: %v", err) + if !s.trackConn(conn) { + _ = conn.Close() return } - - t.Logf("Handling SSH connection from %s", sshConn.RemoteAddr()) - - wg := sync.WaitGroup{} - wg.Add(1) - go func() { - ssh.DiscardRequests(reqs) - wg.Done() - }() - wg.Add(1) - go func() { - s.handleChannels(sshConn, chans) - wg.Done() - }() - wg.Wait() + s.wg.Go(func() { + defer s.untrackConn(conn) + s.handleConn(conn, config) + }) } - }(s.listener) + }) return nil } +// Stop stops the server, closes open connections, and waits for their handlers. func (s *Server) Stop() error { + var err error if s.listener != nil { - return s.listener.Close() + err = s.listener.Close() } - return nil + s.connsMu.Lock() + for c := range s.conns { + _ = c.Close() + } + s.conns = nil + s.connsMu.Unlock() + s.wg.Wait() + return err +} + +// trackConn records an open connection, returning false if the server has stopped. +func (s *Server) trackConn(c net.Conn) bool { + s.connsMu.Lock() + defer s.connsMu.Unlock() + if s.conns == nil { + return false + } + s.conns[c] = struct{}{} + return true +} + +func (s *Server) untrackConn(c net.Conn) { + _ = c.Close() + s.connsMu.Lock() + defer s.connsMu.Unlock() + delete(s.conns, c) } // ExecHandler returns a CommandHandler to execute a command in the given environment. @@ -216,73 +244,6 @@ func (s *Server) serverConfig() *ssh.ServerConfig { return conf } -func (s *Server) handleChannels(conn ssh.ConnMetadata, chans <-chan ssh.NewChannel) { - t := s.t - - for newChannel := range chans { - if newChannel.ChannelType() != "session" { - err := newChannel.Reject(ssh.UnknownChannelType, "unknown channel type") - if err != nil { - t.Errorf("Failed to reject channel: %v", err) - } - continue - } - - channel, requests, err := newChannel.Accept() - if err != nil { - t.Errorf("Failed to accept channel: %v", err) - return - } - - timer := time.NewTimer(time.Second * 10) - - var exitWithStatus = make(chan int, 1) - go func(in <-chan *ssh.Request) { - for req := range in { - if !req.WantReply { - continue - } - switch req.Type { - case "exec": - err := req.Reply(true, nil) - if err != nil { - t.Errorf("Failed to reply to command: %v", err) - } - // Strip the first four bytes of the payload, the uint32 representing the string length. - // See https://datatracker.ietf.org/doc/html/rfc4251#section-5 - cmd := req.Payload[4:] - t.Logf("Handling command: %s", cmd) - exitWithStatus <- s.CommandHandler(conn, string(cmd), CommandIO{ - StdIn: channel, - StdOut: channel, - StdErr: channel.Stderr(), - }) - return - default: - _ = req.Reply(false, nil) - } - } - }(requests) - - for { - select { - case s := <-exitWithStatus: - _, err = channel.SendRequest("exit-status", false, ssh.Marshal(struct{ Status uint32 }{uint32(s)})) - if err != nil { - t.Fatalf("Failed to send exit status: %v", err) - } - goto closeChannel - case <-timer.C: - t.Error("Timed out") - goto closeChannel - } - } - - closeChannel: - _ = channel.Close() - } -} - // generateHostKey returns a host key, generated once, as servers may run in parallel tests. var generateHostKey = sync.OnceValues(func() (ssh.Signer, error) { _, privateKey, err := ed25519.GenerateKey(rand.Reader) From a6803dbfd7c19386d6c1964f9a2b33ae83fc7780 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 14:02:50 +0100 Subject: [PATCH 06/13] fix(tunnel): report the port of an already open tunnel When a tunnel was already open, tunnel:open and tunnel:single reported the port that a new tunnel would have used, instead of the open tunnel's port. Add integration tests for tunnel:open, tunnel:info, tunnel:list, tunnel:close and tunnel:single, forwarding connections through the mock SSH server to local services. Co-Authored-By: Claude Opus 5.5 --- integration-tests/tunnel_test.go | 211 +++++++++++++++++++++ legacy/src/Service/TunnelManager.php | 7 +- legacy/tests/Service/TunnelManagerTest.php | 28 +++ 3 files changed, 241 insertions(+), 5 deletions(-) create mode 100644 integration-tests/tunnel_test.go diff --git a/integration-tests/tunnel_test.go b/integration-tests/tunnel_test.go new file mode 100644 index 000000000..766b2cbca --- /dev/null +++ b/integration-tests/tunnel_test.go @@ -0,0 +1,211 @@ +package tests + +import ( + "bytes" + "encoding/base64" + "encoding/json" + "fmt" + "io" + "net" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" + "github.com/upsun/cli/pkg/mockssh" +) + +// setUpTunnelProject serves an app over SSH, with relationships whose hosts the mock +// SSH server forwards to local services. Each service writes its name to a connection. +func setUpTunnelProject(t *testing.T) *testProject { + t.Helper() + p := setUpProject(t, sshEnv(&mockapi.Deployment{ + WebApps: map[string]mockapi.App{ + "app": {Name: "app", Type: "golang:1.23", Size: "M", Mounts: map[string]mockapi.Mount{}}, + }, + })) + services := map[string]string{} // host:port → local address + for _, name := range []string{"database.internal:3306", "redis.internal:6379"} { + services[name] = serveName(t, strings.Split(name, ".")[0]) + } + sshServer := p.serveSSH(t, mockssh.ExecHandler(t.TempDir(), remoteEnv(t, nil, mockRelationships()))) + sshServer.DialTCP = func(host string, port int) (net.Conn, error) { + addr, ok := services[net.JoinHostPort(host, strconv.Itoa(port))] + if !ok { + return nil, fmt.Errorf("unknown service: %s:%d", host, port) + } + return net.Dial("tcp", addr) + } + return p +} + +// serveName starts a TCP server that writes a name to each connection, returning its address. +func serveName(t *testing.T, name string) string { + t.Helper() + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = l.Close() }) + go func() { + for { + c, err := l.Accept() + if err != nil { + return + } + _, _ = io.WriteString(c, name) + _ = c.Close() + } + }() + return l.Addr().String() +} + +// readTunnel connects to a local port, retrying until the tunnel is ready, and returns what the service wrote. +func readTunnel(t *testing.T, port int) string { + t.Helper() + var out string + require.EventuallyWithT(t, func(c *assert.CollectT) { + conn, err := net.DialTimeout("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(port)), time.Second) + require.NoError(c, err) + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(5 * time.Second)) + b, err := io.ReadAll(conn) + require.NoError(c, err) + require.NotEmpty(c, b) + out = string(b) + }, 10*time.Second, 100*time.Millisecond) + return out +} + +// runWithFiles runs a command with its output written to files instead of pipes, as background +// processes that it starts inherit them, and waiting for a pipe would wait for those processes. +func runWithFiles(f *cmdFactory, args ...string) (stdOut, stdErr string, err error) { + f.t.Helper() + dir := f.t.TempDir() + stdOutFile, err := os.Create(filepath.Join(dir, "stdout")) + require.NoError(f.t, err) + defer stdOutFile.Close() + stdErrFile, err := os.Create(filepath.Join(dir, "stderr")) + require.NoError(f.t, err) + defer stdErrFile.Close() + cmd := f.buildCommand(args...) + cmd.Stdout = stdOutFile + cmd.Stderr = stdErrFile + f.t.Log("Running:", cmd) + err = cmd.Run() + stdOut, stdErr = readFile(f.t, stdOutFile.Name()), readFile(f.t, stdErrFile.Name()) + f.logStderr(stdErr) + return stdOut, stdErr, err +} + +// freePort returns a local port which is not in use. +func freePort(t *testing.T) int { + t.Helper() + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer l.Close() + return l.Addr().(*net.TCPAddr).Port +} + +func TestTunnelOpen(t *testing.T) { + t.Parallel() + p := setUpTunnelProject(t) + f := p.f + args := []string{"-p", layoutProjectID, "-e", "main"} + // The tunnels run in the background until they are closed, or the SSH server stops. + t.Cleanup(func() { _, _, _ = f.RunCombinedOutput("tunnel:close", "--all", "--yes") }) + + _, stdErr, err := runWithFiles(f, append([]string{"tunnel:open"}, args...)...) + require.NoError(t, err) + assert.Contains(t, stdErr, "SSH tunnel opened to database at: mysql://user:@127.0.0.1:") + assert.Contains(t, stdErr, "SSH tunnel opened to redis at: redis://:@127.0.0.1:") + + var relationships map[string][]map[string]any + encoded := strings.TrimSpace(f.Run(append([]string{"tunnel:info", "--encode"}, args...)...)) + decoded, err := base64.StdEncoding.DecodeString(encoded) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(decoded, &relationships)) + require.Len(t, relationships["database"], 1) + require.Len(t, relationships["redis"], 1) + assert.Equal(t, "127.0.0.1", relationships["database"][0]["host"]) + + dbPort := int(relationships["database"][0]["port"].(float64)) + redisPort := int(relationships["redis"][0]["port"].(float64)) + assert.Equal(t, "database", readTunnel(t, dbPort)) + assert.Equal(t, "redis", readTunnel(t, redisPort)) + + assertTrimmed(t, strconv.Itoa(dbPort), f.Run(append([]string{"tunnel:info", "-P", "database.0.port"}, args...)...)) + + list := f.Run(append([]string{"tunnel:list", "--format", "csv", "--columns", "port,relationship"}, args...)...) + assert.Equal(t, fmt.Sprintf("Port,Relationship\n%d,database\n%d,redis\n", dbPort, redisPort), list) + + // Opening tunnels again reuses them. + _, stdErr, err = runWithFiles(f, append([]string{"tunnel:open"}, args...)...) + require.NoError(t, err) + assert.Contains(t, stdErr, fmt.Sprintf("A tunnel is already opened to the relationship database, at: mysql://user:@127.0.0.1:%d", dbPort)) + + _, stdErr, err = f.RunCombinedOutput(append([]string{"tunnel:close", "--yes"}, args...)...) + require.NoError(t, err) + assert.Contains(t, stdErr, "Close tunnel to relationship database on "+layoutProjectID+"-main--app?") + assert.Contains(t, stdErr, "Close tunnel to relationship redis on "+layoutProjectID+"-main--app?") + + // The tunnel processes exit. + assert.Eventually(t, func() bool { + conn, err := net.Dial("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(dbPort))) + if err == nil { + _ = conn.Close() + } + return err != nil + }, 10*time.Second, 100*time.Millisecond) + + _, stdErr, err = f.RunCombinedOutput(append([]string{"tunnel:list"}, args...)...) + assertExitCode(t, 1, err) + assert.Contains(t, stdErr, "No tunnels found.") +} + +func TestTunnelSingle(t *testing.T) { + t.Parallel() + p := setUpTunnelProject(t) + f := p.f + port := freePort(t) + + cmd := f.buildCommand("tunnel:single", "-p", layoutProjectID, "-e", "main", "-r", "redis", + "--port", strconv.Itoa(port)) + var stdErr bytes.Buffer + cmd.Stderr = &stdErr + require.NoError(t, cmd.Start()) + var waitErr error + exited := make(chan struct{}) + go func() { + waitErr = cmd.Wait() + close(exited) + }() + t.Cleanup(func() { + select { + case <-exited: + case <-time.After(10 * time.Second): + _ = cmd.Process.Kill() + t.Error("tunnel:single did not exit") + } + f.logStderr(stdErr.String()) + }) + + assert.Equal(t, "redis", readTunnel(t, port)) + assertTrimmed(t, strconv.Itoa(port), f.Run("tunnel:info", "-p", layoutProjectID, "-e", "main", + "-P", "redis.0.port")) + + // Closing the tunnel ends the command. + f.Run("tunnel:close", "--all", "--yes") + select { + case <-exited: + // The command reports the stopped process, but it exits successfully. + assert.NoError(t, waitErr) + case <-time.After(10 * time.Second): + t.Fatal("tunnel:single did not exit after tunnel:close") + } + assert.Contains(t, stdErr.String(), fmt.Sprintf("SSH tunnel opened to redis at: redis://:@127.0.0.1:%d", port)) +} diff --git a/legacy/src/Service/TunnelManager.php b/legacy/src/Service/TunnelManager.php index 279a4e464..5e19d678e 100644 --- a/legacy/src/Service/TunnelManager.php +++ b/legacy/src/Service/TunnelManager.php @@ -142,8 +142,7 @@ private function saveTunnelInfo(): void * Checks whether a tunnel is already open. * * @return false|Tunnel - * If the tunnel is open, a new Tunnel object is returned with its PID - * set. + * The open tunnel, or false if it is not open. */ public function isOpen(Tunnel $tunnel): false|Tunnel { @@ -157,9 +156,7 @@ public function isOpen(Tunnel $tunnel): false|Tunnel $this->close($t); return false; } - $tunnel->pid = $t->pid; - - return $tunnel; + return $t; } } diff --git a/legacy/tests/Service/TunnelManagerTest.php b/legacy/tests/Service/TunnelManagerTest.php index c2a91783b..fbb751f01 100644 --- a/legacy/tests/Service/TunnelManagerTest.php +++ b/legacy/tests/Service/TunnelManagerTest.php @@ -302,6 +302,34 @@ public function testIsOpenLoadsStateWhenNotAlreadyLoaded(): void $this->assertSame(getmypid(), $result->pid); } + public function testIsOpenReturnsTheOpenTunnel(): void + { + // A new tunnel gets the next free port, but the open tunnel's port is the one in use. + $id = 'proj1--main--app--database--0'; + $metadata = [ + 'projectId' => 'proj1', + 'environmentId' => 'main', + 'appName' => 'app', + 'relationship' => 'database', + 'serviceKey' => 0, + 'service' => ['scheme' => 'mysql', 'host' => 'database.internal', 'port' => 3306], + ]; + $this->writeTunnelInfo($id, $metadata + [ + 'localPort' => 30000, + 'remoteHost' => 'database.internal', + 'remotePort' => 3306, + 'pid' => getmypid(), + ]); + + $manager = $this->createManager($this->tempDir); + + $result = $manager->isOpen(new Tunnel($id, 30001, 'database.internal', 3306, $metadata)); + + $this->assertInstanceOf(Tunnel::class, $result); + $this->assertSame(30000, $result->localPort); + $this->assertSame(getmypid(), $result->pid); + } + public function testCloseRemovesTunnelFromState(): void { // close() has to rewrite the state file itself. Leaving that to the From a938d97ef2cbcb4e19aa0fd107e41187fee9ce9b Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 14:07:41 +0100 Subject: [PATCH 07/13] test: cover the environment:xdebug tunnel Forward a connection from the remote Xdebug socket to a local listener through the mock SSH server, and check that the command exits when the SSH connection closes. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- integration-tests/environment_xdebug_test.go | 114 +++++++++++++++++++ integration-tests/tunnel_test.go | 24 +++- 3 files changed, 133 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f6f4225b9..9bca2399c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -155,7 +155,7 @@ make download-php Tests use github.com/stretchr/testify for assertions. Table-driven tests are preferred with a "cases" slice containing simple test case structs. -Integration tests (`integration-tests/`) call `t.Parallel()`, so they must not share state: no `t.Setenv` or unsynchronized package variables, and use `cmdFactory.useConfig` to change the CLI config file. Write output with `t.Log` (e.g. `mockapi.RequestLogger`), not to stdout or stderr: it is then attributed to its test, and shown only on failure or with `-v`. Tests of environment operations use `setUpProject`, and tests of Git commands (push, clone, fetch) use `setUpGitProject`, which also serves a bare repository over the mock SSH server. Tests of other SSH commands use `serveSSH` with an app from `sshEnv`, and fake remote executables from `remoteEnv`. +Integration tests (`integration-tests/`) call `t.Parallel()`, so they must not share state: no `t.Setenv` or unsynchronized package variables, and use `cmdFactory.useConfig` to change the CLI config file. Write output with `t.Log` (e.g. `mockapi.RequestLogger`), not to stdout or stderr: it is then attributed to its test, and shown only on failure or with `-v`. Tests of environment operations use `setUpProject`, and tests of Git commands (push, clone, fetch) use `setUpGitProject`, which also serves a bare repository over the mock SSH server. Tests of other SSH commands use `serveSSH` with an app from `sshEnv`, and fake remote executables from `remoteEnv`. The mock SSH server can also serve SFTP (`SFTPDir`) and forward connections (`DialTCP`, `ListenUnix`); tests of commands that start background processes (tunnels) must make sure they exit. ### Configuration diff --git a/integration-tests/environment_xdebug_test.go b/integration-tests/environment_xdebug_test.go index 2f8178c7d..2c21c6d64 100644 --- a/integration-tests/environment_xdebug_test.go +++ b/integration-tests/environment_xdebug_test.go @@ -1,10 +1,18 @@ package tests import ( + "bytes" + "io" + "net" "net/http/httptest" + "os" + "path/filepath" "strconv" + "strings" "sync" + "sync/atomic" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -121,3 +129,109 @@ func TestEnvironmentXdebug(t *testing.T) { mu.Unlock() } } + +func TestEnvironmentXdebugTunnel(t *testing.T) { + t.Parallel() + p := setUpProject(t, sshEnv(&mockapi.Deployment{ + WebApps: map[string]mockapi.App{ + "app": {Name: "app", Type: "php:8.4", Size: "M", Mounts: map[string]mockapi.Mount{}, + Runtime: map[string]any{"xdebug": map[string]any{"idekey": "secret_key"}}}, + }, + })) + sshServer := p.serveSSH(t, func(_ ssh.ConnMetadata, _ string, _ mockssh.CommandIO) int { return 0 }) + + // The server listens at a local socket instead of /run/xdebug-tunnel.sock. A short + // directory is used, as socket paths are limited to about 100 characters. + socketDir, err := os.MkdirTemp("", "xdebug") + require.NoError(t, err) + t.Cleanup(func() { _ = os.RemoveAll(socketDir) }) + socket := filepath.Join(socketDir, "xdebug.sock") + var requestedPath atomic.Value + sshServer.ListenUnix = func(path string) (net.Listener, error) { + requestedPath.Store(path) + return net.Listen("unix", socket) + } + + // The IDE listens locally. + ide, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = ide.Close() }) + port := tcpPort(t, ide) + received := make(chan string, 1) + go func() { + c, err := ide.Accept() + if err != nil { + return + } + defer c.Close() + b, _ := io.ReadAll(c) + received <- string(b) + }() + + cmd := p.f.buildCommand("xdebug", "-p", layoutProjectID, "-e", "main", "--port", strconv.Itoa(port)) + var stdErr syncBuffer + cmd.Stderr = &stdErr + require.NoError(t, cmd.Start()) + var waitErr error + exited := make(chan struct{}) + go func() { + waitErr = cmd.Wait() + close(exited) + }() + t.Cleanup(func() { + select { + case <-exited: + case <-time.After(10 * time.Second): + _ = cmd.Process.Kill() + t.Error("xdebug did not exit") + } + p.f.logStderr(stdErr.String()) + }) + + // A connection from the app (Xdebug) to the socket reaches the IDE. + var conn net.Conn + require.Eventually(t, func() bool { + conn, err = net.Dial("unix", socket) + return err == nil + }, 10*time.Second, 50*time.Millisecond) + _, err = io.WriteString(conn, "") + require.NoError(t, err) + require.NoError(t, conn.Close()) + select { + case got := <-received: + assert.Equal(t, "", got) + case <-time.After(10 * time.Second): + t.Fatal("the IDE did not receive a connection") + } + assert.Equal(t, "/run/xdebug-tunnel.sock", requestedPath.Load()) + require.Eventually(t, func() bool { + return strings.Contains(stdErr.String(), "Xdebug tunnel opened at: 127.0.0.1:"+strconv.Itoa(port)) + }, 10*time.Second, 50*time.Millisecond) + + // The tunnel closes with the SSH connection. + require.NoError(t, sshServer.Stop()) + select { + case <-exited: + case <-time.After(10 * time.Second): + t.Fatal("xdebug did not exit after the SSH connection closed") + } + assertExitCode(t, 255, waitErr) +} + +// syncBuffer is a buffer that is safe for concurrent use, e.g. to read a running command's output. +type syncBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *syncBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *syncBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} diff --git a/integration-tests/tunnel_test.go b/integration-tests/tunnel_test.go index 766b2cbca..0814f72df 100644 --- a/integration-tests/tunnel_test.go +++ b/integration-tests/tunnel_test.go @@ -108,7 +108,15 @@ func freePort(t *testing.T) int { l, err := net.Listen("tcp", "127.0.0.1:0") require.NoError(t, err) defer l.Close() - return l.Addr().(*net.TCPAddr).Port + return tcpPort(t, l) +} + +// tcpPort returns the port of a TCP listener. +func tcpPort(t *testing.T, l net.Listener) int { + t.Helper() + addr, ok := l.Addr().(*net.TCPAddr) + require.True(t, ok) + return addr.Port } func TestTunnelOpen(t *testing.T) { @@ -124,17 +132,20 @@ func TestTunnelOpen(t *testing.T) { assert.Contains(t, stdErr, "SSH tunnel opened to database at: mysql://user:@127.0.0.1:") assert.Contains(t, stdErr, "SSH tunnel opened to redis at: redis://:@127.0.0.1:") - var relationships map[string][]map[string]any + var relationships map[string][]struct { + Host string `json:"host"` + Port int `json:"port"` + } encoded := strings.TrimSpace(f.Run(append([]string{"tunnel:info", "--encode"}, args...)...)) decoded, err := base64.StdEncoding.DecodeString(encoded) require.NoError(t, err) require.NoError(t, json.Unmarshal(decoded, &relationships)) require.Len(t, relationships["database"], 1) require.Len(t, relationships["redis"], 1) - assert.Equal(t, "127.0.0.1", relationships["database"][0]["host"]) + assert.Equal(t, "127.0.0.1", relationships["database"][0].Host) - dbPort := int(relationships["database"][0]["port"].(float64)) - redisPort := int(relationships["redis"][0]["port"].(float64)) + dbPort := relationships["database"][0].Port + redisPort := relationships["redis"][0].Port assert.Equal(t, "database", readTunnel(t, dbPort)) assert.Equal(t, "redis", readTunnel(t, redisPort)) @@ -146,7 +157,8 @@ func TestTunnelOpen(t *testing.T) { // Opening tunnels again reuses them. _, stdErr, err = runWithFiles(f, append([]string{"tunnel:open"}, args...)...) require.NoError(t, err) - assert.Contains(t, stdErr, fmt.Sprintf("A tunnel is already opened to the relationship database, at: mysql://user:@127.0.0.1:%d", dbPort)) + assert.Contains(t, stdErr, fmt.Sprintf( + "A tunnel is already opened to the relationship database, at: mysql://user:@127.0.0.1:%d", dbPort)) _, stdErr, err = f.RunCombinedOutput(append([]string{"tunnel:close", "--yes"}, args...)...) require.NoError(t, err) From 96d13a79a9ed486addaa7b9e79c7fc3c783e8954 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 14:10:24 +0100 Subject: [PATCH 08/13] fix(mockssh): close forwarded connections with the SSH connection A forwarded connection whose peer stayed open kept its handler running after the SSH connection closed, so Stop could wait indefinitely. Also create the Xdebug test socket in /tmp, as the integration tests' temporary directory may exceed the Unix socket path length limit. Co-Authored-By: Claude Opus 5.5 --- integration-tests/environment_xdebug_test.go | 6 ++-- pkg/mockssh/channels.go | 33 ++++++++++++++------ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/integration-tests/environment_xdebug_test.go b/integration-tests/environment_xdebug_test.go index 2c21c6d64..25c9496b8 100644 --- a/integration-tests/environment_xdebug_test.go +++ b/integration-tests/environment_xdebug_test.go @@ -140,9 +140,9 @@ func TestEnvironmentXdebugTunnel(t *testing.T) { })) sshServer := p.serveSSH(t, func(_ ssh.ConnMetadata, _ string, _ mockssh.CommandIO) int { return 0 }) - // The server listens at a local socket instead of /run/xdebug-tunnel.sock. A short - // directory is used, as socket paths are limited to about 100 characters. - socketDir, err := os.MkdirTemp("", "xdebug") + // The server listens at a local socket instead of /run/xdebug-tunnel.sock. It is + // in /tmp, as socket paths are limited to about 100 characters. + socketDir, err := os.MkdirTemp("/tmp", "xdebug") require.NoError(t, err) t.Cleanup(func() { _ = os.RemoveAll(socketDir) }) socket := filepath.Join(socketDir, "xdebug.sock") diff --git a/pkg/mockssh/channels.go b/pkg/mockssh/channels.go index 92e3ae8c3..d38147df4 100644 --- a/pkg/mockssh/channels.go +++ b/pkg/mockssh/channels.go @@ -27,8 +27,11 @@ func (s *Server) handleConn(conn net.Conn, config *ssh.ServerConfig) { listenersMu sync.Mutex listeners []net.Listener ) + // connClosed stops forwarding, as forwarded connections may stay open after the SSH connection closes. + connClosed := make(chan struct{}) var wg sync.WaitGroup defer func() { + close(connClosed) listenersMu.Lock() for _, l := range listeners { _ = l.Close() @@ -40,7 +43,7 @@ func (s *Server) handleConn(conn net.Conn, config *ssh.ServerConfig) { wg.Go(func() { for req := range reqs { if req.Type == "streamlocal-forward@openssh.com" { - if l := s.forwardUnix(sshConn, req, &wg); l != nil { + if l := s.forwardUnix(sshConn, req, &wg, connClosed); l != nil { listenersMu.Lock() listeners = append(listeners, l) listenersMu.Unlock() @@ -58,7 +61,7 @@ func (s *Server) handleConn(conn net.Conn, config *ssh.ServerConfig) { case "session": wg.Go(func() { s.handleSession(sshConn, newChannel) }) case "direct-tcpip": - wg.Go(func() { s.handleDirectTCPIP(newChannel) }) + wg.Go(func() { s.handleDirectTCPIP(newChannel, connClosed) }) default: _ = newChannel.Reject(ssh.UnknownChannelType, "unknown channel type") } @@ -126,7 +129,7 @@ func serveSFTP(channel ssh.Channel, dir string) int { } // handleDirectTCPIP handles local port forwarding, if DialTCP is set. -func (s *Server) handleDirectTCPIP(newChannel ssh.NewChannel) { +func (s *Server) handleDirectTCPIP(newChannel ssh.NewChannel, connClosed <-chan struct{}) { t := s.t if s.DialTCP == nil { _ = newChannel.Reject(ssh.Prohibited, "port forwarding is disabled") @@ -155,12 +158,14 @@ func (s *Server) handleDirectTCPIP(newChannel ssh.NewChannel) { return } go ssh.DiscardRequests(requests) - proxy(channel, target) + proxy(channel, target, connClosed) } // forwardUnix handles a request for remote forwarding of a Unix socket, if ListenUnix is set. // It returns the listener, or nil if the request is rejected. -func (s *Server) forwardUnix(conn *ssh.ServerConn, req *ssh.Request, wg *sync.WaitGroup) net.Listener { +func (s *Server) forwardUnix( + conn *ssh.ServerConn, req *ssh.Request, wg *sync.WaitGroup, connClosed <-chan struct{}, +) net.Listener { t := s.t var payload struct{ Path string } if s.ListenUnix == nil || ssh.Unmarshal(req.Payload, &payload) != nil { @@ -190,15 +195,25 @@ func (s *Server) forwardUnix(conn *ssh.ServerConn, req *ssh.Request, wg *sync.Wa return } go ssh.DiscardRequests(requests) - proxy(channel, c) + proxy(channel, c, connClosed) }) } }) return l } -// proxy copies data in both directions until either side is closed. -func proxy(channel ssh.Channel, conn net.Conn) { +// proxy copies data in both directions until both sides are closed, or the SSH connection is closed. +func proxy(channel ssh.Channel, conn net.Conn, connClosed <-chan struct{}) { + finished := make(chan struct{}) + defer close(finished) + go func() { + select { + case <-connClosed: + case <-finished: + } + _ = channel.Close() + _ = conn.Close() + }() done := make(chan struct{}, 2) go func() { _, _ = io.Copy(channel, conn) @@ -214,6 +229,4 @@ func proxy(channel ssh.Channel, conn net.Conn) { }() <-done <-done - _ = channel.Close() - _ = conn.Close() } From 96db1f2c84955e7b379ff88117fcd47a58b9a051 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 14:12:43 +0100 Subject: [PATCH 09/13] fix(mockssh): close forwarding listeners with the SSH connection A listener registered while the connection was closing could stay open, so Stop waited indefinitely. Each listener now closes itself when the connection closes. Also use a synchronized buffer for the output of tunnel:single, which the test reads while the command runs. Co-Authored-By: Claude Opus 5.5 --- integration-tests/environment_xdebug_test.go | 19 ------------- integration-tests/tests.go | 18 ++++++++++++ integration-tests/tunnel_test.go | 3 +- pkg/mockssh/channels.go | 30 ++++++-------------- 4 files changed, 27 insertions(+), 43 deletions(-) diff --git a/integration-tests/environment_xdebug_test.go b/integration-tests/environment_xdebug_test.go index 25c9496b8..aa5cdfb25 100644 --- a/integration-tests/environment_xdebug_test.go +++ b/integration-tests/environment_xdebug_test.go @@ -1,7 +1,6 @@ package tests import ( - "bytes" "io" "net" "net/http/httptest" @@ -217,21 +216,3 @@ func TestEnvironmentXdebugTunnel(t *testing.T) { } assertExitCode(t, 255, waitErr) } - -// syncBuffer is a buffer that is safe for concurrent use, e.g. to read a running command's output. -type syncBuffer struct { - mu sync.Mutex - buf bytes.Buffer -} - -func (b *syncBuffer) Write(p []byte) (int, error) { - b.mu.Lock() - defer b.mu.Unlock() - return b.buf.Write(p) -} - -func (b *syncBuffer) String() string { - b.mu.Lock() - defer b.mu.Unlock() - return b.buf.String() -} diff --git a/integration-tests/tests.go b/integration-tests/tests.go index 127c72899..268184007 100644 --- a/integration-tests/tests.go +++ b/integration-tests/tests.go @@ -263,3 +263,21 @@ func waitForServer(t *testing.T, client *http.Client, url string) *http.Response }, 10*time.Second, 50*time.Millisecond, "server did not respond: %s", url) return resp } + +// syncBuffer is a buffer that is safe for concurrent use, e.g. to read a running command's output. +type syncBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *syncBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *syncBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} diff --git a/integration-tests/tunnel_test.go b/integration-tests/tunnel_test.go index 0814f72df..583c34d24 100644 --- a/integration-tests/tunnel_test.go +++ b/integration-tests/tunnel_test.go @@ -1,7 +1,6 @@ package tests import ( - "bytes" "encoding/base64" "encoding/json" "fmt" @@ -187,7 +186,7 @@ func TestTunnelSingle(t *testing.T) { cmd := f.buildCommand("tunnel:single", "-p", layoutProjectID, "-e", "main", "-r", "redis", "--port", strconv.Itoa(port)) - var stdErr bytes.Buffer + var stdErr syncBuffer cmd.Stderr = &stdErr require.NoError(t, cmd.Start()) var waitErr error diff --git a/pkg/mockssh/channels.go b/pkg/mockssh/channels.go index d38147df4..1aaed9e5f 100644 --- a/pkg/mockssh/channels.go +++ b/pkg/mockssh/channels.go @@ -22,32 +22,18 @@ func (s *Server) handleConn(conn net.Conn, config *ssh.ServerConfig) { t.Logf("Handling SSH connection from %s", sshConn.RemoteAddr()) - // Listeners for remote forwarding, closed with the connection. - var ( - listenersMu sync.Mutex - listeners []net.Listener - ) // connClosed stops forwarding, as forwarded connections may stay open after the SSH connection closes. connClosed := make(chan struct{}) var wg sync.WaitGroup defer func() { close(connClosed) - listenersMu.Lock() - for _, l := range listeners { - _ = l.Close() - } - listenersMu.Unlock() wg.Wait() }() wg.Go(func() { for req := range reqs { if req.Type == "streamlocal-forward@openssh.com" { - if l := s.forwardUnix(sshConn, req, &wg, connClosed); l != nil { - listenersMu.Lock() - listeners = append(listeners, l) - listenersMu.Unlock() - } + s.forwardUnix(sshConn, req, &wg, connClosed) continue } if req.WantReply { @@ -162,24 +148,25 @@ func (s *Server) handleDirectTCPIP(newChannel ssh.NewChannel, connClosed <-chan } // forwardUnix handles a request for remote forwarding of a Unix socket, if ListenUnix is set. -// It returns the listener, or nil if the request is rejected. -func (s *Server) forwardUnix( - conn *ssh.ServerConn, req *ssh.Request, wg *sync.WaitGroup, connClosed <-chan struct{}, -) net.Listener { +func (s *Server) forwardUnix(conn *ssh.ServerConn, req *ssh.Request, wg *sync.WaitGroup, connClosed <-chan struct{}) { t := s.t var payload struct{ Path string } if s.ListenUnix == nil || ssh.Unmarshal(req.Payload, &payload) != nil { _ = req.Reply(false, nil) - return nil + return } l, err := s.ListenUnix(payload.Path) if err != nil { t.Logf("Failed to listen at %s: %v", payload.Path, err) _ = req.Reply(false, nil) - return nil + return } _ = req.Reply(true, nil) t.Logf("Forwarding connections from the socket %s", payload.Path) + wg.Go(func() { + <-connClosed + _ = l.Close() + }) wg.Go(func() { for { c, err := l.Accept() @@ -199,7 +186,6 @@ func (s *Server) forwardUnix( }) } }) - return l } // proxy copies data in both directions until both sides are closed, or the SSH connection is closed. From c111a207270145ef4abf921b1c11eb7f479d320b Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 14:22:25 +0100 Subject: [PATCH 10/13] fix(ssh): report paused and inactive environments with --all Skipping the container selection for `ssh --all` also skipped the deployment lookup that reported paused and inactive environments, so the command printed the SSH URLs and exited successfully. Also assert the app-not-found message in TestSSHAppSelection. Co-Authored-By: Claude Opus 5.5 --- integration-tests/ssh_test.go | 23 ++++++++++++++++++- .../Environment/EnvironmentSshCommand.php | 4 ++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/integration-tests/ssh_test.go b/integration-tests/ssh_test.go index 62a705582..24c03b4e7 100644 --- a/integration-tests/ssh_test.go +++ b/integration-tests/ssh_test.go @@ -124,9 +124,30 @@ func TestSSHAppSelection(t *testing.T) { _, stdErr, err := f.RunCombinedOutput("ssh", "-p", layoutProjectID, "-e", "main", "--app", "missing", "--pipe") assert.Error(t, err) - assert.Contains(t, stdErr, "missing") + assert.Contains(t, stdErr, `App not found: missing`) _, stdErr, err = f.RunCombinedOutput("ssh", "-p", layoutProjectID, "-e", "main", "--pipe") assert.Error(t, err) assert.Contains(t, stdErr, "Specifying the --app or --worker is required in non-interactive mode") } + +func TestSSHInactiveEnvironment(t *testing.T) { + t.Parallel() + // Paused and inactive environments have no current deployment. + statuses := []string{"paused", "inactive"} + envs := make([]*mockapi.Environment, 0, len(statuses)) + for _, status := range statuses { + env := makeEnv(layoutProjectID, status, "development", status, "main") + env.Links["pf:ssh:app"] = mockapi.HALLink{HREF: "ssh://" + status + "--app@ssh.cli-tests.example.com"} + envs = append(envs, env) + } + f := setUpProject(t, envs...).f + + for _, status := range statuses { + for _, opt := range []string{"--pipe", "--all"} { + _, stdErr, err := f.RunCombinedOutput("ssh", "-p", layoutProjectID, "-e", status, opt) + assertExitCode(t, 1, err) + assert.Contains(t, stdErr, "is "+status+", so an SSH connection is not possible", opt) + } + } +} diff --git a/legacy/src/Command/Environment/EnvironmentSshCommand.php b/legacy/src/Command/Environment/EnvironmentSshCommand.php index af0147545..3d7fe3506 100644 --- a/legacy/src/Command/Environment/EnvironmentSshCommand.php +++ b/legacy/src/Command/Environment/EnvironmentSshCommand.php @@ -64,6 +64,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int $environment = $selection->getEnvironment(); if (Option::bool($input, 'all')) { + // Without a container selection, check the state here. + if (in_array($environment->status, ['inactive', 'paused'], true)) { + throw new EnvironmentStateException('The environment is ' . $environment->status, $environment); + } $output->writeln(array_values($environment->getSshUrls())); return 0; From 25e3024d396caf8a2c0b6f8f4376f04f1242a6c5 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 15 Oct 2026 14:25:54 +0100 Subject: [PATCH 11/13] test: write literal paths in the fake df output Co-Authored-By: Claude Opus 5.5 --- integration-tests/mount_ssh_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/integration-tests/mount_ssh_test.go b/integration-tests/mount_ssh_test.go index 6b1676743..12ac9a815 100644 --- a/integration-tests/mount_ssh_test.go +++ b/integration-tests/mount_ssh_test.go @@ -4,6 +4,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -31,13 +32,12 @@ func TestMountSize(t *testing.T) { p := setUpProject(t, sshEnv(mountsDeployment())) // The df output has two mounts on one disk and one on another. Each du output is the path's length in KiB. env := remoteEnv(t, map[string]string{ - "df": `cat <