From 974c7079f130916976af954a2e7fba2b31342d43 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 14 Oct 2022 13:46:14 +0300 Subject: [PATCH 1/2] 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 --- 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 6d0a9a952f44b..73f06d8a2dca0 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -447,7 +447,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 06a33598b755f..ae3ebf52b34b6 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 9834e27e5f39c..54a607661a284 100644 --- a/go.mod +++ b/go.mod @@ -51,6 +51,12 @@ replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220926024748-50f0 // 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.20221014103342-f1dc1fef3d9e + 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 13fdc5724f6b6..5107710042116 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.20221014103342-f1dc1fef3d9e h1:+xFsQ1Ugjladoxp2Pju5m9FSCp799Opvr/ncVhDk7EI= +github.com/mafredri/sftp v1.13.6-0.20221014103342-f1dc1fef3d9e/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= From 04b1492607916b34199e27cc82ffc785dd11222d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 14 Oct 2022 15:57:05 +0300 Subject: [PATCH 2/2] Update fork --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 54a607661a284..09ce8e6ba1041 100644 --- a/go.mod +++ b/go.mod @@ -55,7 +55,7 @@ replace github.com/gliderlabs/ssh => github.com/coder/ssh v0.0.0-20220811105153- // 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.20221014103342-f1dc1fef3d9e +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 diff --git a/go.sum b/go.sum index 5107710042116..e0b8c0404cb6d 100644 --- a/go.sum +++ b/go.sum @@ -1225,8 +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.20221014103342-f1dc1fef3d9e h1:+xFsQ1Ugjladoxp2Pju5m9FSCp799Opvr/ncVhDk7EI= -github.com/mafredri/sftp v1.13.6-0.20221014103342-f1dc1fef3d9e/go.mod h1:wHDZ0IZX6JcBYRK1TH9bcVq8G7TLpVHYIGJRFnmPfxg= +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=