diff --git a/pkg/agent/install.go b/pkg/agent/install.go index cdf91f89..55d64277 100644 --- a/pkg/agent/install.go +++ b/pkg/agent/install.go @@ -3,7 +3,9 @@ package agent import ( "fmt" "os" + "path" "runtime" + "strings" "github.com/google/uuid" @@ -76,6 +78,24 @@ func install(logger *logging.Logger, transport Transport, prompter string, cmdEx remoteFileName = "." + remoteFileName } fullRemotePath := remotePathFromHome(cmdExe, remoteFileName) + // On POSIX remotes, the agent binary is copied with scp and then executed + // over ssh using this same path. Historically that path is "~/"-prefixed and + // relies on "~" resolving identically for both steps. That assumption breaks + // when the remote SSH/SFTP working directory isn't the home directory (for + // example, Coder workspaces configured with an explicit directory, or + // devcontainers whose workspace folder differs from $HOME): scp resolves the + // path relative to the working directory while the ssh exec expands "~" to + // $HOME, so the freshly-copied binary can't be found. Resolve the absolute + // home directory once and use it for both the copy and the invocation so they + // agree regardless of the remote working directory. If resolution fails, fall + // back to the previous "~"-relative behavior. + if posix { + if home, homeErr := remoteHomeDirectory(transport); homeErr == nil { + fullRemotePath = path.Join(home, remoteFileName) + } else { + logger.Infof("unable to resolve remote home directory, using ~-relative agent path: %v", homeErr) + } + } if err = transport.Copy(agentExecutable, fullRemotePath); err != nil { return fmt.Errorf("unable to copy agent binary: %w", err) @@ -109,3 +129,21 @@ func install(logger *logging.Logger, transport Transport, prompter string, cmdEx // Success. return nil } + +// remoteHomeDirectory resolves the absolute path of the home directory on a +// POSIX remote by querying $HOME over the transport. It's used to construct an +// absolute agent installation path so that the scp copy and ssh execution steps +// agree even when the remote SSH/SFTP working directory isn't the home +// directory (e.g. Coder workspaces with a configured directory or devcontainer +// workspace folder). +func remoteHomeDirectory(transport Transport) (string, error) { + out, err := output(transport, `echo "$HOME"`) + if err != nil { + return "", fmt.Errorf("unable to query remote home directory: %w", err) + } + home := strings.TrimSpace(string(out)) + if !strings.HasPrefix(home, "/") { + return "", fmt.Errorf("invalid remote home directory: %q", home) + } + return home, nil +} diff --git a/pkg/agent/install_test.go b/pkg/agent/install_test.go index 55b6cbde..e164e5fd 100644 --- a/pkg/agent/install_test.go +++ b/pkg/agent/install_test.go @@ -1,4 +1,73 @@ package agent +import ( + "os" + "os/exec" + "testing" +) + // NOTE: Unfortunately the Install() method can't be tested directly, but it is // tested indirectly by integration tests. + +// echoTransport is a fake Transport whose Command returns a process that prints +// a fixed string to standard output, simulating a remote returning the value of +// $HOME. It's used to unit test remoteHomeDirectory without a real remote. +type echoTransport struct { + // stdout is the standard output that the created command will print. + stdout string + // failCommand, if true, causes the created command to exit non-zero with no + // standard output. + failCommand bool +} + +func (t *echoTransport) Copy(_, _ string) error { return nil } + +func (t *echoTransport) Command(_ string) (*exec.Cmd, error) { + if t.failCommand { + return exec.Command("false"), nil + } + return exec.Command("printf", "%s", t.stdout), nil +} + +func (t *echoTransport) ClassifyError(_ *os.ProcessState, _ string) (bool, bool, error) { + return false, false, nil +} + +// TestRemoteHomeDirectory validates parsing and validation of the remote home +// directory returned over a transport. +func TestRemoteHomeDirectory(t *testing.T) { + testCases := []struct { + name string + stdout string + failCommand bool + expected string + expectError bool + }{ + {name: "simple", stdout: "/home/ubuntu", expected: "/home/ubuntu"}, + {name: "trailing newline", stdout: "/home/ubuntu\n", expected: "/home/ubuntu"}, + {name: "surrounding whitespace", stdout: " /home/ubuntu \n", expected: "/home/ubuntu"}, + {name: "empty", stdout: "", expectError: true}, + {name: "unexpanded variable", stdout: "$HOME", expectError: true}, + {name: "relative", stdout: "home/ubuntu", expectError: true}, + {name: "command error", failCommand: true, expectError: true}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + transport := &echoTransport{stdout: testCase.stdout, failCommand: testCase.failCommand} + home, err := remoteHomeDirectory(transport) + if testCase.expectError { + if err == nil { + t.Fatalf("expected an error but got home %q", home) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if home != testCase.expected { + t.Fatalf("expected home %q but got %q", testCase.expected, home) + } + }) + } +}