diff --git a/agent/agent.go b/agent/agent.go index 5deb9893f3..0a3977583f 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -14,7 +14,6 @@ import ( "net/http" "net/netip" "os" - "os/user" "path/filepath" "slices" "strconv" @@ -151,6 +150,9 @@ func New(options Options) Agent { if options.Filesystem == nil { options.Filesystem = afero.NewOsFs() } + if options.EnvInfo == nil { + options.EnvInfo = &usershell.SystemEnvInfo{} + } if options.TempDir == "" { options.TempDir = os.TempDir() } @@ -427,7 +429,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, pathStore, a.envInfo, a.updateCommandEnv, func() string { + a.processAPI = agentproc.NewAPI(a.logger.Named("processes"), a.execer, a.filesystem, pathStore, a.envInfo, a.updateCommandEnv, func() string { if m := a.manifest.Load(); m != nil { return m.Directory } @@ -1303,12 +1305,12 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context, // // An example is VS Code Remote, which must know the directory // before initializing a connection. - manifest.Directory, err = expandPathToAbs(manifest.Directory) + manifest.Directory, err = a.expandPathToAbs(manifest.Directory) if err != nil { return xerrors.Errorf("expand directory: %w", err) } // Normalize all devcontainer paths by making them absolute. - manifest.Devcontainers = agentcontainers.ExpandAllDevcontainerPaths(a.logger, expandPathToAbs, manifest.Devcontainers) + manifest.Devcontainers = agentcontainers.ExpandAllDevcontainerPaths(a.logger, a.expandPathToAbs, manifest.Devcontainers) subsys, err := agentsdk.ProtoFromSubsystems(a.subsystems) if err != nil { a.logger.Critical(ctx, "failed to convert subsystems", slog.Error(err)) @@ -1337,7 +1339,7 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context, // longer than file writes. Startup scripts still wait because they run // sequentially below. Env var injection is unaffected because it // happens lazily per-command in updateCommandEnv. - homeDir, err := os.UserHomeDir() + homeDir, err := a.envInfo.HomeDir() if err != nil { a.logger.Warn(ctx, "failed to resolve home directory for secret files", slog.Error(err)) } @@ -2347,31 +2349,16 @@ lifecycleWaitLoop: return nil } -// userHomeDir returns the home directory of the current user, giving -// priority to the $HOME environment variable. -func userHomeDir() (string, error) { - // First we check the environment. - homedir, err := os.UserHomeDir() - if err == nil { - return homedir, nil - } - - // As a fallback, we try the user information. - u, err := user.Current() - if err != nil { - return "", xerrors.Errorf("current user: %w", err) - } - return u.HomeDir, nil -} - // expandPathToAbs converts a path to an absolute path. It primarily resolves -// the home directory and any environment variables that may be set. -func expandPathToAbs(path string) (string, error) { +// the home directory and any environment variables that may be set. The home +// directory is resolved through the agent's EnvInfoer so the injected +// environment is honored. +func (a *agent) expandPathToAbs(path string) (string, error) { if path == "" { return "", nil } if path[0] == '~' { - home, err := userHomeDir() + home, err := a.envInfo.HomeDir() if err != nil { return "", err } @@ -2380,7 +2367,7 @@ func expandPathToAbs(path string) (string, error) { path = os.ExpandEnv(path) if !filepath.IsAbs(path) { - home, err := userHomeDir() + home, err := a.envInfo.HomeDir() if err != nil { return "", err } diff --git a/agent/agent_test.go b/agent/agent_test.go index a9b9431156..ac50b34aa7 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1473,10 +1473,12 @@ func TestAgent_SFTP(t *testing.T) { expectedDir = "/" + strings.ReplaceAll(customDir, "\\", "/") } - //nolint:dogsled - conn, agentClient, _, _, _ := setupAgent(t, agentsdk.Manifest{ + conn, agentClient, _, fs, _ := setupAgent(t, agentsdk.Manifest{ Directory: customDir, }, 0) + // The agent stats the working directory against its filesystem, so + // the directory must exist there for it to be honored. + require.NoError(t, fs.MkdirAll(customDir, 0o700)) sshClient, err := conn.SSHClient(ctx) require.NoError(t, err) defer sshClient.Close() @@ -1491,6 +1493,34 @@ func TestAgent_SFTP(t *testing.T) { _ = client.Close() assertConnectionReport(t, agentClient, proto.Connection_SSH, 0, "") }) + + t.Run("MissingWorkingDirectory", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + home, err := os.UserHomeDir() + require.NoError(t, err, "get home dir") + if runtime.GOOS == "windows" { + home = "/" + strings.ReplaceAll(home, "\\", "/") + } + + // A configured directory that does not exist on the agent's + // filesystem must fall back to the home directory. + missingDir := filepath.Join(t.TempDir(), "does-not-exist") + //nolint:dogsled + conn, _, _, _, _ := setupAgent(t, agentsdk.Manifest{ + Directory: missingDir, + }, 0) + sshClient, err := conn.SSHClient(ctx) + require.NoError(t, err) + defer sshClient.Close() + client, err := sftp.NewClient(sshClient) + require.NoError(t, err) + defer client.Close() + wd, err := client.Getwd() + require.NoError(t, err, "get working directory") + require.Equal(t, home, wd, "working directory should fall back to user home") + }) } func TestAgent_SCP(t *testing.T) { diff --git a/agent/agentproc/api.go b/agent/agentproc/api.go index 30c4a8c0da..4713485e1b 100644 --- a/agent/agentproc/api.go +++ b/agent/agentproc/api.go @@ -11,6 +11,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/google/uuid" + "github.com/spf13/afero" "cdr.dev/slog/v3" "github.com/coder/coder/v2/agent/agentchat" @@ -37,10 +38,10 @@ type API struct { } // NewAPI creates a new process API handler. -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 { +func NewAPI(logger slog.Logger, execer agentexec.Execer, fs afero.Fs, 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, envInfo, updateEnv, workingDir), + manager: newManager(logger, execer, fs, envInfo, updateEnv, workingDir), pathStore: pathStore, } } diff --git a/agent/agentproc/api_test.go b/agent/agentproc/api_test.go index ff90ff58b0..6a9cf8f130 100644 --- a/agent/agentproc/api_test.go +++ b/agent/agentproc/api_test.go @@ -138,7 +138,7 @@ 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, nil, nil, updateEnv, workingDir) + api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, nil, nil, updateEnv, workingDir) t.Cleanup(func() { _ = api.Close() }) @@ -153,7 +153,7 @@ func newTestAPIWithEnvInfo(t *testing.T, workingDir func() string, envInfo users logger := slogtest.Make(t, &slogtest.Options{ IgnoreErrors: true, }).Leveled(slog.LevelDebug) - api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, envInfo, nil, workingDir) + api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, nil, envInfo, nil, workingDir) t.Cleanup(func() { _ = api.Close() }) @@ -174,7 +174,7 @@ func TestAccessLogIncludesChatID(t *testing.T) { sink := testutil.NewFakeSink(t) logger := sink.Logger() - api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, nil, nil, nil) + api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, nil, nil, nil, nil) t.Cleanup(func() { _ = api.Close() }) @@ -1144,7 +1144,7 @@ func TestHandleStartProcess_ChatHeaders_EmptyWorkDir_StillNotifies(t *testing.T) defer unsub() logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) - api := agentproc.NewAPI(logger, agentexec.DefaultExecer, pathStore, nil, func(current []string) ([]string, error) { + api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, pathStore, nil, func(current []string) ([]string, error) { return current, nil }, nil) defer api.Close() diff --git a/agent/agentproc/process.go b/agent/agentproc/process.go index 8f0ca53322..c5c93a2a1a 100644 --- a/agent/agentproc/process.go +++ b/agent/agentproc/process.go @@ -10,6 +10,7 @@ import ( "time" "github.com/google/uuid" + "github.com/spf13/afero" "golang.org/x/xerrors" "cdr.dev/slog/v3" @@ -75,6 +76,7 @@ type manager struct { mu sync.Mutex logger slog.Logger execer agentexec.Execer + fs afero.Fs clock quartz.Clock procs map[string]*process closed bool @@ -84,13 +86,17 @@ type manager struct { } // newManager creates a new process manager. -func newManager(logger slog.Logger, execer agentexec.Execer, envInfo usershell.EnvInfoer, updateEnv func(current []string) (updated []string, err error), workingDir func() string) *manager { +func newManager(logger slog.Logger, execer agentexec.Execer, fs afero.Fs, envInfo usershell.EnvInfoer, updateEnv func(current []string) (updated []string, err error), workingDir func() string) *manager { + if fs == nil { + fs = afero.NewOsFs() + } if envInfo == nil { envInfo = &usershell.SystemEnvInfo{} } return &manager{ logger: logger, execer: execer, + fs: fs, clock: quartz.NewReal(), procs: make(map[string]*process), updateEnv: updateEnv, @@ -122,7 +128,7 @@ func (m *manager) start(req workspacesdk.StartProcessRequest, chatID string) (*p // the process is not tied to any HTTP request. ctx, cancel := context.WithCancel(context.Background()) cmd := m.execer.CommandContext(ctx, "sh", "-c", req.Command) - cmd.Dir = m.resolveWorkDir(req.WorkDir) + cmd.Dir = m.resolveWorkingDirectory(req.WorkDir) cmd.Stdin = nil cmd.SysProcAttr = procSysProcAttr() @@ -370,23 +376,21 @@ func (p *process) waitForOutput(ctx context.Context) error { return ctx.Err() } -// resolveWorkDir returns the directory a process should start in. -// Priority: explicit request dir > agent configured dir > $HOME. -// Falls through when a candidate is empty or does not exist on -// disk, matching the behavior of SSH sessions. -func (m *manager) resolveWorkDir(requested string) string { +// resolveWorkingDirectory returns the directory a process should start in. +// Priority: explicit request dir > agent configured dir > user home. +// The configured dir > home tail is shared with SSH sessions via +// usershell.ResolveWorkingDirectory so the two cannot drift. +func (m *manager) resolveWorkingDirectory(requested string) string { if requested != "" { return requested } + var configured string if m.workingDir != nil { - if dir := m.workingDir(); dir != "" { - if info, err := os.Stat(dir); err == nil && info.IsDir() { - return dir - } - } + configured = m.workingDir() } - if home, err := m.envInfo.HomeDir(); err == nil { - return home + dir, err := usershell.ResolveWorkingDirectory(m.fs, m.envInfo, configured) + if err != nil { + return "" } - return "" + return dir } diff --git a/agent/agentssh/agentssh.go b/agent/agentssh/agentssh.go index 1f7f714b56..eb2e9ebb6b 100644 --- a/agent/agentssh/agentssh.go +++ b/agent/agentssh/agentssh.go @@ -9,7 +9,6 @@ import ( "net" "os" "os/exec" - "os/user" "path/filepath" "runtime" "slices" @@ -185,13 +184,9 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom config.AnnouncementBanners = func() *[]codersdk.BannerConfig { return &[]codersdk.BannerConfig{} } } if config.WorkingDirectory == nil { - config.WorkingDirectory = func() string { - home, err := userHomeDir() - if err != nil { - return "" - } - return home - } + // Empty means unset, so resolveWorkingDirectory falls back to the + // EnvInfo home directory. + config.WorkingDirectory = func() string { return "" } } if config.EnvInfo == nil { config.EnvInfo = &usershell.SystemEnvInfo{} @@ -749,7 +744,7 @@ func (s *Server) startPTYSession(logger slog.Logger, session ptySession, magicTy } } - if !isQuietLogin(s.fs, session.RawCommand()) { + if !isQuietLogin(s.fs, s.config.EnvInfo, session.RawCommand()) { err := showMOTD(s.fs, session, s.config.MOTDFile()) if err != nil { logger.Error(ctx, "agent failed to show MOTD", slog.Error(err)) @@ -878,13 +873,14 @@ func (s *Server) sftpHandler(logger slog.Logger, session ssh.Session) error { // Change current working directory to the configured // directory (or home directory if not set) so that SFTP // connections land there. - dir := s.config.WorkingDirectory() - if dir == "" { - var err error - dir, err = userHomeDir() - if err != nil { - logger.Warn(ctx, "get sftp working directory failed, unable to get home dir", slog.Error(err)) - } + // + // The host EnvInfo is used here, not a container's. This is + // correct only while SFTP is blocked for container sessions + // (see the closeCause guard above). If container SFTP is added, + // the container EnvInfo must be resolved and passed here. + dir, err := s.resolveWorkingDirectory(s.config.EnvInfo) + if err != nil { + logger.Warn(ctx, "resolve sftp working directory failed", slog.Error(err)) } if dir != "" { opts = append(opts, sftp.WithServerWorkingDirectory(dir)) @@ -916,6 +912,12 @@ func (s *Server) sftpHandler(logger slog.Logger, session ssh.Session) error { return xerrors.Errorf("sftp server closed with error: %w", err) } +// resolveWorkingDirectory returns the working directory for a session, binding +// the server filesystem and configured directory to the shared resolver. +func (s *Server) resolveWorkingDirectory(ei usershell.EnvInfoer) (string, error) { + return usershell.ResolveWorkingDirectory(s.fs, ei, s.config.WorkingDirectory()) +} + func (s *Server) CommandEnv(ei usershell.EnvInfoer, addEnv []string) (shell, dir string, env []string, err error) { if ei == nil { ei = &usershell.SystemEnvInfo{} @@ -932,18 +934,9 @@ func (s *Server) CommandEnv(ei usershell.EnvInfoer, addEnv []string) (shell, dir return "", "", nil, xerrors.Errorf("get user shell: %w", err) } - dir = s.config.WorkingDirectory() - - // If the metadata directory doesn't exist, we run the command - // in the users home directory. - _, err = os.Stat(dir) - if dir == "" || err != nil { - // Default to user home if a directory is not set. - homedir, err := ei.HomeDir() - if err != nil { - return "", "", nil, xerrors.Errorf("get home dir: %w", err) - } - dir = homedir + dir, err = s.resolveWorkingDirectory(ei) + if err != nil { + return "", "", nil, xerrors.Errorf("resolve working dir: %w", err) } env = append(ei.Environ(), addEnv...) // Set login variables (see `man login`). @@ -1288,7 +1281,7 @@ func isLoginShell(rawCommand string) bool { // isQuietLogin checks if the SSH server should perform a quiet login or not. // // https://github.com/openssh/openssh-portable/blob/25bd659cc72268f2858c5415740c442ee950049f/session.c#L816 -func isQuietLogin(fs afero.Fs, rawCommand string) bool { +func isQuietLogin(fs afero.Fs, ei usershell.EnvInfoer, rawCommand string) bool { // We are always quiet unless this is a login shell. if !isLoginShell(rawCommand) { return true @@ -1296,7 +1289,7 @@ func isQuietLogin(fs afero.Fs, rawCommand string) bool { // Best effort, if we can't get the home directory, // we can't lookup .hushlogin. - homedir, err := userHomeDir() + homedir, err := ei.HomeDir() if err != nil { return false } @@ -1355,23 +1348,6 @@ func writeWithCarriageReturn(src io.Reader, dest io.Writer) error { return nil } -// userHomeDir returns the home directory of the current user, giving -// priority to the $HOME environment variable. -func userHomeDir() (string, error) { - // First we check the environment. - homedir, err := os.UserHomeDir() - if err == nil { - return homedir, nil - } - - // As a fallback, we try the user information. - u, err := user.Current() - if err != nil { - return "", xerrors.Errorf("current user: %w", err) - } - return u.HomeDir, nil -} - // UpdateHostSigner updates the host signer with a new key generated from the provided seed. // If an existing host key exists with the same algorithm, it is overwritten func (s *Server) UpdateHostSigner(seed int64) error { diff --git a/agent/usershell/usershell.go b/agent/usershell/usershell.go index 1819eb468a..7a386a6079 100644 --- a/agent/usershell/usershell.go +++ b/agent/usershell/usershell.go @@ -4,13 +4,15 @@ import ( "os" "os/user" + "github.com/spf13/afero" "golang.org/x/xerrors" ) -// HomeDir returns the home directory of the current user, giving -// priority to the $HOME environment variable. -// Deprecated: use EnvInfoer.HomeDir() instead. -func HomeDir() (string, error) { +// homeDir returns the home directory of the current user, giving +// priority to the $HOME environment variable. It backs +// SystemEnvInfo.HomeDir. Callers outside this package resolve the home +// directory through an EnvInfoer so the injected environment is honored. +func homeDir() (string, error) { // First we check the environment. homedir, err := os.UserHomeDir() if err == nil { @@ -25,6 +27,20 @@ func HomeDir() (string, error) { return u.HomeDir, nil } +// ResolveWorkingDirectory returns dir when it is non-empty and an existing +// directory on fs. Otherwise it falls back to the home directory +// reported by ei. SSH sessions and the process API share this so their +// working directory resolution cannot drift, and the home fallback goes +// through the injected EnvInfoer rather than the host directly. +func ResolveWorkingDirectory(fs afero.Fs, ei EnvInfoer, dir string) (string, error) { + if dir != "" { + if info, err := fs.Stat(dir); err == nil && info.IsDir() { + return dir, nil + } + } + return ei.HomeDir() +} + // EnvInfoer encapsulates external information about the environment. type EnvInfoer interface { // User returns the current user. @@ -64,11 +80,11 @@ func (SystemEnvInfo) Environ() []string { } func (SystemEnvInfo) HomeDir() (string, error) { - return HomeDir() + return homeDir() } func (SystemEnvInfo) Shell(username string) (string, error) { - return Get(username) + return get(username) } func (SystemEnvInfo) ModifyCommand(name string, args ...string) (string, []string) { diff --git a/agent/usershell/usershell_darwin.go b/agent/usershell/usershell_darwin.go index acc990db83..42500d7a72 100644 --- a/agent/usershell/usershell_darwin.go +++ b/agent/usershell/usershell_darwin.go @@ -9,9 +9,10 @@ import ( "golang.org/x/xerrors" ) -// Get returns the $SHELL environment variable. -// Deprecated: use SystemEnvInfo.UserShell instead. -func Get(username string) (string, error) { +// get resolves the user's shell via dscl, falling back to $SHELL. It +// backs SystemEnvInfo.Shell. Callers resolve the shell through an +// EnvInfoer. +func get(username string) (string, error) { // This command will output "UserShell: /bin/zsh" if successful, we // can ignore the error since we have fallback behavior. if !filepath.IsLocal(username) { diff --git a/agent/usershell/usershell_other.go b/agent/usershell/usershell_other.go index 6ee3ad2368..9093949655 100644 --- a/agent/usershell/usershell_other.go +++ b/agent/usershell/usershell_other.go @@ -10,9 +10,10 @@ import ( "golang.org/x/xerrors" ) -// Get returns the /etc/passwd entry for the username provided. -// Deprecated: use SystemEnvInfo.UserShell instead. -func Get(username string) (string, error) { +// get resolves the user's shell from /etc/passwd, falling back to +// $SHELL. It backs SystemEnvInfo.Shell. Callers resolve the shell +// through an EnvInfoer. +func get(username string) (string, error) { contents, err := os.ReadFile("/etc/passwd") if err != nil { return "", xerrors.Errorf("read /etc/passwd: %w", err) diff --git a/agent/usershell/usershell_test.go b/agent/usershell/usershell_test.go index 40873b5dee..5687b34e99 100644 --- a/agent/usershell/usershell_test.go +++ b/agent/usershell/usershell_test.go @@ -1,26 +1,32 @@ package usershell_test import ( + "os" "os/user" + "path/filepath" "runtime" "testing" + "github.com/spf13/afero" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" "github.com/coder/coder/v2/agent/usershell" ) //nolint:paralleltest,tparallel // This test sets an environment variable. -func TestGet(t *testing.T) { +func TestShell(t *testing.T) { if runtime.GOOS == "windows" { t.SkipNow() } + ei := usershell.SystemEnvInfo{} + t.Run("Fallback", func(t *testing.T) { t.Setenv("SHELL", "/bin/sh") t.Run("NonExistentUser", func(t *testing.T) { - shell, err := usershell.Get("notauser") + shell, err := ei.Shell("notauser") require.NoError(t, err) require.Equal(t, "/bin/sh", shell) }) @@ -31,14 +37,14 @@ func TestGet(t *testing.T) { t.Setenv("SHELL", "") t.Run("NotFound", func(t *testing.T) { - _, err := usershell.Get("notauser") + _, err := ei.Shell("notauser") require.Error(t, err) }) t.Run("User", func(t *testing.T) { u, err := user.Current() require.NoError(t, err) - shell, err := usershell.Get(u.Username) + shell, err := ei.Shell(u.Username) require.NoError(t, err) require.NotEmpty(t, shell) }) @@ -46,10 +52,102 @@ func TestGet(t *testing.T) { t.Run("Remove GOTRACEBACK=none", func(t *testing.T) { t.Setenv("GOTRACEBACK", "none") - ei := usershell.SystemEnvInfo{} env := ei.Environ() for _, e := range env { require.NotEqual(t, "GOTRACEBACK=none", e) } }) } + +// homeEnvInfo reports a fixed home directory and otherwise delegates to +// SystemEnvInfo, isolating ResolveWorkingDirectory tests from the host's real +// home directory. +type homeEnvInfo struct { + usershell.SystemEnvInfo + home string +} + +func (e homeEnvInfo) HomeDir() (string, error) { return e.home, nil } + +// errorEnvInfo reports an error from HomeDir to exercise the fallback +// error path. +type errorEnvInfo struct { + usershell.SystemEnvInfo + err error +} + +func (e errorEnvInfo) HomeDir() (string, error) { return "", e.err } + +func TestResolveWorkingDirectory(t *testing.T) { + t.Parallel() + + const home = "/home/coder" + ei := homeEnvInfo{home: home} + + t.Run("Exists", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + require.NoError(t, fs.MkdirAll("/work", 0o700)) + dir, err := usershell.ResolveWorkingDirectory(fs, ei, "/work") + require.NoError(t, err) + require.Equal(t, "/work", dir) + }) + + t.Run("Missing", func(t *testing.T) { + t.Parallel() + dir, err := usershell.ResolveWorkingDirectory(afero.NewMemMapFs(), ei, "/work") + require.NoError(t, err) + require.Equal(t, home, dir) + }) + + t.Run("Empty", func(t *testing.T) { + t.Parallel() + dir, err := usershell.ResolveWorkingDirectory(afero.NewMemMapFs(), ei, "") + require.NoError(t, err) + require.Equal(t, home, dir) + }) + + t.Run("NotADirectory", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + require.NoError(t, afero.WriteFile(fs, "/work", []byte("file"), 0o600)) + dir, err := usershell.ResolveWorkingDirectory(fs, ei, "/work") + require.NoError(t, err) + require.Equal(t, home, dir) + }) + + t.Run("HomeDirError", func(t *testing.T) { + t.Parallel() + ei := errorEnvInfo{err: xerrors.New("no home")} + _, err := usershell.ResolveWorkingDirectory(afero.NewMemMapFs(), ei, "") + require.ErrorContains(t, err, "no home") + }) + + t.Run("Symlink", func(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("symlink creation requires privileges on Windows") + } + // MemMapFs cannot model symlinks. Use the real filesystem to + // confirm Stat follows symlinks: a link to a directory is honored, + // a link to a non-directory falls back to home. + fs := afero.NewOsFs() + base := t.TempDir() + + realDir := filepath.Join(base, "real") + require.NoError(t, os.Mkdir(realDir, 0o700)) + linkToDir := filepath.Join(base, "link-dir") + require.NoError(t, os.Symlink(realDir, linkToDir)) + dir, err := usershell.ResolveWorkingDirectory(fs, ei, linkToDir) + require.NoError(t, err) + require.Equal(t, linkToDir, dir, "symlink to a directory should be honored") + + realFile := filepath.Join(base, "file") + require.NoError(t, os.WriteFile(realFile, []byte("x"), 0o600)) + linkToFile := filepath.Join(base, "link-file") + require.NoError(t, os.Symlink(realFile, linkToFile)) + dir, err = usershell.ResolveWorkingDirectory(fs, ei, linkToFile) + require.NoError(t, err) + require.Equal(t, home, dir, "symlink to a non-directory should fall back to home") + }) +} diff --git a/agent/usershell/usershell_windows.go b/agent/usershell/usershell_windows.go index 52823d900d..7ddf27ed2a 100644 --- a/agent/usershell/usershell_windows.go +++ b/agent/usershell/usershell_windows.go @@ -2,9 +2,10 @@ package usershell import "os/exec" -// Get returns the command prompt binary name. -// Deprecated: use SystemEnvInfo.UserShell instead. -func Get(username string) (string, error) { +// get resolves the Windows shell, preferring pwsh.exe, then +// powershell.exe, then cmd.exe. It backs SystemEnvInfo.Shell. Callers +// resolve the shell through an EnvInfoer. +func get(username string) (string, error) { _, err := exec.LookPath("pwsh.exe") if err == nil { return "pwsh.exe", nil