From 173b7a2c8383a28a23a40e9fe6da5be640f325cd Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 21 Oct 2022 17:54:06 +0300 Subject: [PATCH] fix: Start SFTP sessions in user home (working directory) (#4549) * fix: Start SFTP sessions in user home (working directory) This commit switches to our fork of `pkg/sftp` which includes a Server option for changing the current working directory. Attempt to upstream: https://github.com/pkg/sftp/pull/528 Supercedes and closes #4420 Fixes #3620 * Update fork --- agent/agent.go | 13 ++++++++++++- agent/agent_test.go | 10 ++++++++++ go.mod | 6 ++++++ go.sum | 6 ++---- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index ffaf2ed454..6b49f82ef7 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -448,7 +448,18 @@ func (a *agent) init(ctx context.Context) { "sftp": func(session ssh.Session) { session.DisablePTYEmulation() - server, err := sftp.NewServer(session) + var opts []sftp.ServerOption + // 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, "get sftp working directory failed, unable to get current user", slog.Error(err)) + } else { + opts = append(opts, sftp.WithServerWorkingDirectory(u.HomeDir)) + } + + server, err := sftp.NewServer(session, opts...) if err != nil { a.logger.Debug(session.Context(), "initialize sftp server", slog.Error(err)) return diff --git a/agent/agent_test.go b/agent/agent_test.go index e10eee7f11..ac7afd5de6 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,21 @@ 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") + 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) 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, home, wd, "working directory should be home user home") tempFile := filepath.Join(t.TempDir(), "sftp") file, err := client.Create(tempFile) require.NoError(t, err) diff --git a/go.mod b/go.mod index 758f1d6a4f..caf155fffb 100644 --- a/go.mod +++ b/go.mod @@ -51,6 +51,12 @@ replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20221015033036-5861 // makes importing it directly a bit messy. replace github.com/gliderlabs/ssh => github.com/coder/ssh v0.0.0-20220811105153-fcea99919338 +// The sftp server implementation used by us does not support changing +// the working directory, this fork adds support for it. +// +// Attempt to upstream: https://github.com/pkg/sftp/pull/528 +replace github.com/pkg/sftp => github.com/mafredri/sftp v1.13.6-0.20221014125459-6a7168cf46fd + require ( cdr.dev/slog v1.4.2-0.20220525200111-18dce5c2cd5f cloud.google.com/go/compute v1.10.0 diff --git a/go.sum b/go.sum index 099a4d96cb..4ffdf1a75d 100644 --- a/go.sum +++ b/go.sum @@ -1225,6 +1225,8 @@ github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69 github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I= github.com/lyft/protoc-gen-star v0.5.3/go.mod h1:V0xaHgaf5oCCqmcxYcWiDfTiKsZsRc87/1qhoTACD8w= +github.com/mafredri/sftp v1.13.6-0.20221014125459-6a7168cf46fd h1:X7ZK1YGbCoPkllDq/lG5PLV4k3LVddypzoH5hVgzmiw= +github.com/mafredri/sftp v1.13.6-0.20221014125459-6a7168cf46fd/go.mod h1:wHDZ0IZX6JcBYRK1TH9bcVq8G7TLpVHYIGJRFnmPfxg= github.com/mafredri/udp v0.1.2-0.20220805105907-b2872e92e98d h1:E+lU8/1jcUd3guqaRvjAGCcwoPe7jcYrNv9K0OzEwdQ= github.com/mafredri/udp v0.1.2-0.20220805105907-b2872e92e98d/go.mod h1:GUd681aT3Tj7pdNkUtqBz5pp/GLMGIaMI9Obq6+ob48= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= @@ -1505,10 +1507,6 @@ github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/profile v1.6.0/go.mod h1:qBsxPvzyUincmltOk6iyRVxHYg4adc0OFOv72ZdLa18= -github.com/pkg/sftp v1.10.1/go.mod h1:lYOWFsE0bwd1+KfKJaKeuokY15vzFx25BLbzYYoAxZI= -github.com/pkg/sftp v1.13.1/go.mod h1:3HaPG6Dq1ILlpPZRO0HVMrsydcdLt6HRDccSgb87qRg= -github.com/pkg/sftp v1.13.5 h1:a3RLUqkyjYRtBTZJZ1VRrKbN3zhuPLlUc3sphVz81go= -github.com/pkg/sftp v1.13.5/go.mod h1:wHDZ0IZX6JcBYRK1TH9bcVq8G7TLpVHYIGJRFnmPfxg= github.com/pmezard/go-difflib v0.0.0-20151028094244-d8ed2627bdf0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=