From c280bda78e2809d431a6286f3e9a5e9296f55bf0 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 7 Oct 2022 19:45:38 +0300 Subject: [PATCH 1/3] fix: Start SFTP sessions in user home (working directory) This is a simple fix for #3620 by changing the working directory of `coder agent` at runtime. Since the agent doesn't write files to it's working directory, I can't think of any downsides to this method. It does not, however, exclude that we rewrite the SFTP implementation to use the modernized `RequestServer` sometime in the future. Opted to place this in agent init vs `cli/agent` because of portability of fix, tests don't usually use `cli/agent`. Fixes #3620 --- agent/agent.go | 11 +++++++++++ agent/agent_test.go | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/agent/agent.go b/agent/agent.go index 6d0a9a952f44b..2e5acdb31ca68 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -378,6 +378,17 @@ func (a *agent) runStartupScript(ctx context.Context, script string) error { } func (a *agent) init(ctx context.Context) { + // Change current working directory to the users home + // directory so that SFTP connections land there. + // https://github.com/coder/coder/issues/3620 + u, err := user.Current() + if err != nil { + a.logger.Warn(ctx, "change working directory failed, unable to get current user", slog.Error(err)) + } else { + err = os.Chdir(u.HomeDir) + a.logger.Warn(ctx, "change working directory failed", slog.Error(err)) + } + a.logger.Info(ctx, "generating host key") // Clients' should ignore the host key when connecting. // The agent needs to authenticate with coderd to SSH, diff --git a/agent/agent_test.go b/agent/agent_test.go index 06a33598b755f..ac08a2fc45217 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -10,6 +10,7 @@ import ( "net/netip" "os" "os/exec" + "os/user" "path/filepath" "runtime" "strconv" @@ -212,12 +213,17 @@ func TestAgent(t *testing.T) { t.Run("SFTP", func(t *testing.T) { t.Parallel() + u, err := user.Current() + require.NoError(t, err, "get current user") conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) sshClient, err := conn.SSHClient() require.NoError(t, err) defer sshClient.Close() client, err := sftp.NewClient(sshClient) require.NoError(t, err) + wd, err := client.Getwd() + require.NoError(t, err, "get working directory") + require.Equal(t, u.HomeDir, wd, "working directory should be home user home") tempFile := filepath.Join(t.TempDir(), "sftp") file, err := client.Create(tempFile) require.NoError(t, err) From 35872b651ac16725bfe8f601d5dae71c92a04ba3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 7 Oct 2022 19:59:48 +0300 Subject: [PATCH 2/3] Fix if err --- agent/agent.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/agent/agent.go b/agent/agent.go index 2e5acdb31ca68..2c78ce3e145e3 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -386,7 +386,9 @@ func (a *agent) init(ctx context.Context) { a.logger.Warn(ctx, "change working directory failed, unable to get current user", slog.Error(err)) } else { err = os.Chdir(u.HomeDir) - a.logger.Warn(ctx, "change working directory failed", slog.Error(err)) + if err != nil { + a.logger.Warn(ctx, "change working directory failed", slog.Error(err)) + } } a.logger.Info(ctx, "generating host key") From 7a3e7280d1276328545591f6bdd0800cc72de065 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 7 Oct 2022 20:02:24 +0300 Subject: [PATCH 3/3] Fix test for windows --- agent/agent_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index ac08a2fc45217..ae3ebf52b34b6 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -215,6 +215,10 @@ func TestAgent(t *testing.T) { t.Parallel() u, err := user.Current() require.NoError(t, err, "get current user") + home := u.HomeDir + if runtime.GOOS == "windows" { + home = "/" + strings.ReplaceAll(home, "\\", "/") + } conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) sshClient, err := conn.SSHClient() require.NoError(t, err) @@ -223,7 +227,7 @@ func TestAgent(t *testing.T) { require.NoError(t, err) wd, err := client.Getwd() require.NoError(t, err, "get working directory") - require.Equal(t, u.HomeDir, wd, "working directory should be home user home") + require.Equal(t, home, wd, "working directory should be home user home") tempFile := filepath.Join(t.TempDir(), "sftp") file, err := client.Create(tempFile) require.NoError(t, err)