diff --git a/agent/agent.go b/agent/agent.go index 61b116de6b..5deb9893f3 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -52,6 +52,7 @@ import ( "github.com/coder/coder/v2/agent/proto" "github.com/coder/coder/v2/agent/proto/resourcesmonitor" "github.com/coder/coder/v2/agent/reconnectingpty" + "github.com/coder/coder/v2/agent/usershell" "github.com/coder/coder/v2/agent/x/agentdesktop" "github.com/coder/coder/v2/agent/x/agentmcp" "github.com/coder/coder/v2/buildinfo" @@ -89,7 +90,10 @@ type Options struct { Client Client ReconnectingPTYTimeout time.Duration EnvironmentVariables map[string]string - Logger slog.Logger + // EnvInfo overrides the session command environment source. Only + // tests set this. Nil defaults to usershell.SystemEnvInfo. + EnvInfo usershell.EnvInfoer + Logger slog.Logger // IgnorePorts tells the api handler which ports to ignore when // listing all listening ports. This is helpful to hide ports that // are used by the agent, that the user does not care about. @@ -226,6 +230,7 @@ func New(options Options) Agent { statsReportInterval: options.StatsReportInterval, announcementBannersRefreshInterval: options.ServiceBannerRefreshInterval, sshMaxTimeout: options.SSHMaxTimeout, + envInfo: options.EnvInfo, subsystems: options.Subsystems, logSender: agentsdk.NewLogSender(options.Logger), blockFileTransfer: options.BlockFileTransfer, @@ -303,6 +308,7 @@ type agent struct { announcementBannersRefreshInterval time.Duration sshServer *agentssh.Server sshMaxTimeout time.Duration + envInfo usershell.EnvInfoer blockFileTransfer bool blockReversePortForwarding bool blockLocalPortForwarding bool @@ -365,6 +371,7 @@ func (a *agent) init() { AnnouncementBanners: func() *[]codersdk.BannerConfig { return a.announcementBanners.Load() }, UpdateEnv: a.updateCommandEnv, WorkingDirectory: func() string { return a.manifest.Load().Directory }, + EnvInfo: a.envInfo, BlockFileTransfer: a.blockFileTransfer, BlockReversePortForwarding: a.blockReversePortForwarding, BlockLocalPortForwarding: a.blockLocalPortForwarding, @@ -420,7 +427,7 @@ func (a *agent) init() { pathStore := agentgit.NewPathStore() a.filesAPI = agentfiles.NewAPI(a.logger.Named("files"), a.filesystem, pathStore) - a.processAPI = agentproc.NewAPI(a.logger.Named("processes"), a.execer, a.updateCommandEnv, pathStore, func() string { + a.processAPI = agentproc.NewAPI(a.logger.Named("processes"), a.execer, pathStore, a.envInfo, a.updateCommandEnv, func() string { if m := a.manifest.Load(); m != nil { return m.Directory } diff --git a/agent/agent_test.go b/agent/agent_test.go index 8c615b4d44..a9b9431156 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1741,6 +1741,43 @@ func TestAgent_SSHConnectionLoginVars(t *testing.T) { } } +// TestAgent_SSHEnvInfoShell verifies that an agent.Options.EnvInfo whose +// Shell() reports a custom shell is piped through to the SSH session, so the +// session command runs under that shell instead of the host default. +func TestAgent_SSHEnvInfoShell(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("the fake shell is a POSIX script") + } + + // A fake shell that ignores its arguments and prints a sentinel. The + // sentinel only appears in the session output if the injected Shell() was + // honored. Otherwise the command's own output ("should-not-run") appears. + const marker = "injected-shell-was-used" + shellPath := filepath.Join(t.TempDir(), "fakeshell") + //nolint:gosec // Executable test shell with test-controlled content. + err := os.WriteFile(shellPath, []byte("#!/bin/sh\necho "+marker+"\n"), 0o700) + require.NoError(t, err) + + session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil, func(_ *agenttest.Client, o *agent.Options) { + o.EnvInfo = shellOverrideEnvInfo{shell: shellPath} + }) + + output, err := session.Output("echo should-not-run") + require.NoError(t, err) + require.Contains(t, string(output), marker) + require.NotContains(t, string(output), "should-not-run") +} + +// shellOverrideEnvInfo is a usershell.EnvInfoer that delegates to the system +// implementation but reports a custom shell. +type shellOverrideEnvInfo struct { + usershell.SystemEnvInfo + shell string +} + +func (e shellOverrideEnvInfo) Shell(string) (string, error) { return e.shell, nil } + func TestAgent_Metadata(t *testing.T) { t.Parallel() diff --git a/agent/agentproc/api.go b/agent/agentproc/api.go index 8b8e1ce2ec..30c4a8c0da 100644 --- a/agent/agentproc/api.go +++ b/agent/agentproc/api.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/v2/agent/agentchat" "github.com/coder/coder/v2/agent/agentexec" "github.com/coder/coder/v2/agent/agentgit" + "github.com/coder/coder/v2/agent/usershell" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/workspacesdk" @@ -36,10 +37,10 @@ type API struct { } // NewAPI creates a new process API handler. -func NewAPI(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error), pathStore *agentgit.PathStore, workingDir func() string) *API { +func NewAPI(logger slog.Logger, execer agentexec.Execer, pathStore *agentgit.PathStore, envInfo usershell.EnvInfoer, updateEnv func(current []string) (updated []string, err error), workingDir func() string) *API { return &API{ logger: logger, - manager: newManager(logger, execer, updateEnv, workingDir), + manager: newManager(logger, execer, envInfo, updateEnv, workingDir), pathStore: pathStore, } } diff --git a/agent/agentproc/api_test.go b/agent/agentproc/api_test.go index 704d968899..ff90ff58b0 100644 --- a/agent/agentproc/api_test.go +++ b/agent/agentproc/api_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" "runtime" "strings" "sync" @@ -24,6 +25,7 @@ import ( "github.com/coder/coder/v2/agent/agentexec" "github.com/coder/coder/v2/agent/agentgit" "github.com/coder/coder/v2/agent/agentproc" + "github.com/coder/coder/v2/agent/usershell" "github.com/coder/coder/v2/coderd/httpmw/loggermw" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/codersdk" @@ -136,19 +138,43 @@ func newTestAPIWithOptions(t *testing.T, updateEnv func([]string) ([]string, err logger := slogtest.Make(t, &slogtest.Options{ IgnoreErrors: true, }).Leveled(slog.LevelDebug) - api := agentproc.NewAPI(logger, agentexec.DefaultExecer, updateEnv, nil, workingDir) + api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, nil, updateEnv, workingDir) t.Cleanup(func() { _ = api.Close() }) return agentchat.Middleware(api.Routes()) } +// newTestAPIWithEnvInfo creates a new API with an injected EnvInfoer +// and an optional workingDir hook. +func newTestAPIWithEnvInfo(t *testing.T, workingDir func() string, envInfo usershell.EnvInfoer) http.Handler { + t.Helper() + + logger := slogtest.Make(t, &slogtest.Options{ + IgnoreErrors: true, + }).Leveled(slog.LevelDebug) + api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, envInfo, nil, workingDir) + t.Cleanup(func() { + _ = api.Close() + }) + return agentchat.Middleware(api.Routes()) +} + +// homeOverrideEnvInfo is a usershell.EnvInfoer that delegates to the +// system implementation but reports a custom home directory. +type homeOverrideEnvInfo struct { + usershell.SystemEnvInfo + home string +} + +func (e homeOverrideEnvInfo) HomeDir() (string, error) { return e.home, nil } + func TestAccessLogIncludesChatID(t *testing.T) { t.Parallel() sink := testutil.NewFakeSink(t) logger := sink.Logger() - api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, nil, nil) + api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, nil, nil, nil) t.Cleanup(func() { _ = api.Close() }) @@ -403,6 +429,40 @@ func TestStartProcess(t *testing.T) { require.Equal(t, homeDir, proc.WorkDir) }) + t.Run("DefaultWorkDirUsesInjectedEnvInfoHome", func(t *testing.T) { + t.Parallel() + + // With no explicit or configured directory available, + // the home fallback must come from the injected EnvInfo + // rather than the real user home. + homeDir := t.TempDir() + handler := newTestAPIWithEnvInfo(t, func() string { + return filepath.Join(t.TempDir(), "nonexistent") + }, homeOverrideEnvInfo{home: homeDir}) + + id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{ + Command: "echo ok", + }) + + resp := waitForExit(t, handler, id) + require.NotNil(t, resp.ExitCode) + require.Equal(t, 0, *resp.ExitCode) + + w := getList(t, handler) + require.Equal(t, http.StatusOK, w.Code) + var listResp workspacesdk.ListProcessesResponse + require.NoError(t, json.NewDecoder(w.Body).Decode(&listResp)) + var proc *workspacesdk.ProcessInfo + for i := range listResp.Processes { + if listResp.Processes[i].ID == id { + proc = &listResp.Processes[i] + break + } + } + require.NotNil(t, proc, "process not found in list") + require.Equal(t, homeDir, proc.WorkDir) + }) + t.Run("CustomEnv", func(t *testing.T) { t.Parallel() @@ -1084,9 +1144,9 @@ func TestHandleStartProcess_ChatHeaders_EmptyWorkDir_StillNotifies(t *testing.T) defer unsub() logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) - api := agentproc.NewAPI(logger, agentexec.DefaultExecer, func(current []string) ([]string, error) { + api := agentproc.NewAPI(logger, agentexec.DefaultExecer, pathStore, nil, func(current []string) ([]string, error) { return current, nil - }, pathStore, nil) + }, nil) defer api.Close() routes := agentchat.Middleware(api.Routes()) diff --git a/agent/agentproc/process.go b/agent/agentproc/process.go index d4cecdff9b..8f0ca53322 100644 --- a/agent/agentproc/process.go +++ b/agent/agentproc/process.go @@ -14,6 +14,7 @@ import ( "cdr.dev/slog/v3" "github.com/coder/coder/v2/agent/agentexec" + "github.com/coder/coder/v2/agent/usershell" "github.com/coder/coder/v2/codersdk/workspacesdk" "github.com/coder/quartz" ) @@ -79,10 +80,14 @@ type manager struct { closed bool updateEnv func(current []string) (updated []string, err error) workingDir func() string + envInfo usershell.EnvInfoer } // newManager creates a new process manager. -func newManager(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error), workingDir func() string) *manager { +func newManager(logger slog.Logger, execer agentexec.Execer, envInfo usershell.EnvInfoer, updateEnv func(current []string) (updated []string, err error), workingDir func() string) *manager { + if envInfo == nil { + envInfo = &usershell.SystemEnvInfo{} + } return &manager{ logger: logger, execer: execer, @@ -90,6 +95,7 @@ func newManager(logger slog.Logger, execer agentexec.Execer, updateEnv func(curr procs: make(map[string]*process), updateEnv: updateEnv, workingDir: workingDir, + envInfo: envInfo, } } @@ -379,7 +385,7 @@ func (m *manager) resolveWorkDir(requested string) string { } } } - if home, err := os.UserHomeDir(); err == nil { + if home, err := m.envInfo.HomeDir(); err == nil { return home } return "" diff --git a/agent/agentssh/agentssh.go b/agent/agentssh/agentssh.go index 9a7e2ead31..1f7f714b56 100644 --- a/agent/agentssh/agentssh.go +++ b/agent/agentssh/agentssh.go @@ -107,6 +107,10 @@ type Config struct { // where users will land when they connect via SSH. Default is the home // directory of the user. WorkingDirectory func() string + // EnvInfo sources the session command environment. Default is + // usershell.SystemEnvInfo. A container override still applies per + // session when ExperimentalContainers is enabled. + EnvInfo usershell.EnvInfoer // X11DisplayOffset is the offset to add to the X11 display number. // Default is 10. X11DisplayOffset *int @@ -189,6 +193,9 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom return home } } + if config.EnvInfo == nil { + config.EnvInfo = &usershell.SystemEnvInfo{} + } if config.ReportConnection == nil { config.ReportConnection = func(uuid.UUID, MagicSessionType, string) func(int, string) { return func(int, string) {} } } @@ -619,7 +626,7 @@ func (s *Server) sessionStart(logger slog.Logger, session ssh.Session, env []str ptyLabel = "yes" } - var ei usershell.EnvInfoer + ei := s.config.EnvInfo var err error if s.config.ExperimentalContainers && container != "" { ei, err = agentcontainers.EnvInfo(ctx, s.Execer, container, containerUser)