From cd0f6aba8b4378592e722eb2e7a2d3e90d818585 Mon Sep 17 00:00:00 2001 From: chrislu Date: Tue, 2 Dec 2025 23:24:38 -0800 Subject: [PATCH 1/5] fix: SFTP HomeDir path translation for user operations When users have a non-root HomeDir (e.g., '/sftp/user'), their SFTP operations should be relative to that directory. Previously, when a user uploaded to '/' via SFTP, the path was not translated to their home directory, causing 'permission denied for / for permission write'. This fix adds a toAbsolutePath() method that implements chroot-like behavior where the user's HomeDir becomes their root. All file and directory operations now translate paths through this method. Example: User with HomeDir='/sftp/user' uploading to '/' now correctly maps to '/sftp/user'. Fixes: https://github.com/seaweedfs/seaweedfs/issues/7470 --- weed/sftpd/sftp_file_writer.go | 5 +-- weed/sftpd/sftp_filer.go | 62 +++++++++++++++++++--------------- weed/sftpd/sftp_server.go | 27 +++++++++++++++ 3 files changed, 64 insertions(+), 30 deletions(-) diff --git a/weed/sftpd/sftp_file_writer.go b/weed/sftpd/sftp_file_writer.go index 0a662d021..fed60eec0 100644 --- a/weed/sftpd/sftp_file_writer.go +++ b/weed/sftpd/sftp_file_writer.go @@ -72,6 +72,7 @@ func (l listerat) ListAt(ls []os.FileInfo, offset int64) (int, error) { type SeaweedSftpFileWriter struct { fs SftpServer req *sftp.Request + absPath string // Absolute path after HomeDir translation mu sync.Mutex tmpFile *os.File permissions os.FileMode @@ -105,6 +106,6 @@ func (w *SeaweedSftpFileWriter) Close() error { return err } - // Stream the file instead of loading it - return w.fs.putFile(w.req.Filepath, w.tmpFile, w.fs.user) + // Stream the file to the absolute path (after HomeDir translation) + return w.fs.putFile(w.absPath, w.tmpFile, w.fs.user) } diff --git a/weed/sftpd/sftp_filer.go b/weed/sftpd/sftp_filer.go index 9baaf41d7..f1514af23 100644 --- a/weed/sftpd/sftp_filer.go +++ b/weed/sftpd/sftp_filer.go @@ -100,18 +100,20 @@ func (fs *SftpServer) withTimeoutContext(fn func(ctx context.Context) error) err // ==================== Command Dispatcher ==================== func (fs *SftpServer) dispatchCmd(r *sftp.Request) error { - glog.V(0).Infof("Dispatch: %s %s", r.Method, r.Filepath) + absPath := fs.toAbsolutePath(r.Filepath) + glog.V(0).Infof("Dispatch: %s %s (absolute: %s)", r.Method, r.Filepath, absPath) switch r.Method { case "Remove": - return fs.removeEntry(r) + return fs.removeEntry(absPath) case "Rename": - return fs.renameEntry(r) + absTarget := fs.toAbsolutePath(r.Target) + return fs.renameEntry(absPath, absTarget) case "Mkdir": - return fs.makeDir(r) + return fs.makeDir(absPath) case "Rmdir": - return fs.removeDir(r) + return fs.removeDir(absPath) case "Setstat": - return fs.setFileStat(r) + return fs.setFileStatWithRequest(absPath, r) default: return fmt.Errorf("unsupported: %s", r.Method) } @@ -120,10 +122,11 @@ func (fs *SftpServer) dispatchCmd(r *sftp.Request) error { // ==================== File Operations ==================== func (fs *SftpServer) readFile(r *sftp.Request) (io.ReaderAt, error) { - if err := fs.checkFilePermission(r.Filepath, "read"); err != nil { + absPath := fs.toAbsolutePath(r.Filepath) + if err := fs.checkFilePermission(absPath, "read"); err != nil { return nil, err } - entry, err := fs.getEntry(r.Filepath) + entry, err := fs.getEntry(absPath) if err != nil { return nil, err } @@ -131,7 +134,8 @@ func (fs *SftpServer) readFile(r *sftp.Request) (io.ReaderAt, error) { } func (fs *SftpServer) newFileWriter(r *sftp.Request) (io.WriterAt, error) { - dir, _ := util.FullPath(r.Filepath).DirAndName() + absPath := fs.toAbsolutePath(r.Filepath) + dir, _ := util.FullPath(absPath).DirAndName() if err := fs.checkFilePermission(dir, "write"); err != nil { glog.Errorf("Permission denied for %s", dir) return nil, err @@ -145,6 +149,7 @@ func (fs *SftpServer) newFileWriter(r *sftp.Request) (io.WriterAt, error) { return &SeaweedSftpFileWriter{ fs: *fs, req: r, + absPath: absPath, tmpFile: tmpFile, permissions: 0644, uid: fs.user.Uid, @@ -153,16 +158,16 @@ func (fs *SftpServer) newFileWriter(r *sftp.Request) (io.WriterAt, error) { }, nil } -func (fs *SftpServer) removeEntry(r *sftp.Request) error { - return fs.deleteEntry(r.Filepath, false) +func (fs *SftpServer) removeEntry(absPath string) error { + return fs.deleteEntry(absPath, false) } -func (fs *SftpServer) renameEntry(r *sftp.Request) error { - if err := fs.checkFilePermission(r.Filepath, "rename"); err != nil { +func (fs *SftpServer) renameEntry(absPath, absTarget string) error { + if err := fs.checkFilePermission(absPath, "rename"); err != nil { return err } - oldDir, oldName := util.FullPath(r.Filepath).DirAndName() - newDir, newName := util.FullPath(r.Target).DirAndName() + oldDir, oldName := util.FullPath(absPath).DirAndName() + newDir, newName := util.FullPath(absTarget).DirAndName() return fs.callWithClient(false, func(ctx context.Context, client filer_pb.SeaweedFilerClient) error { _, err := client.AtomicRenameEntry(ctx, &filer_pb.AtomicRenameEntryRequest{ OldDirectory: oldDir, OldName: oldName, @@ -172,15 +177,15 @@ func (fs *SftpServer) renameEntry(r *sftp.Request) error { }) } -func (fs *SftpServer) setFileStat(r *sftp.Request) error { - if err := fs.checkFilePermission(r.Filepath, "write"); err != nil { +func (fs *SftpServer) setFileStatWithRequest(absPath string, r *sftp.Request) error { + if err := fs.checkFilePermission(absPath, "write"); err != nil { return err } - entry, err := fs.getEntry(r.Filepath) + entry, err := fs.getEntry(absPath) if err != nil { return err } - dir, _ := util.FullPath(r.Filepath).DirAndName() + dir, _ := util.FullPath(absPath).DirAndName() // apply attrs if r.AttrFlags().Permissions { entry.Attributes.FileMode = uint32(r.Attributes().FileMode()) @@ -201,18 +206,19 @@ func (fs *SftpServer) setFileStat(r *sftp.Request) error { // ==================== Directory Operations ==================== func (fs *SftpServer) listDir(r *sftp.Request) (sftp.ListerAt, error) { - if err := fs.checkFilePermission(r.Filepath, "list"); err != nil { + absPath := fs.toAbsolutePath(r.Filepath) + if err := fs.checkFilePermission(absPath, "list"); err != nil { return nil, err } if r.Method == "Stat" || r.Method == "Lstat" { - entry, err := fs.getEntry(r.Filepath) + entry, err := fs.getEntry(absPath) if err != nil { return nil, err } fi := &EnhancedFileInfo{FileInfo: FileInfoFromEntry(entry), uid: entry.Attributes.Uid, gid: entry.Attributes.Gid} return listerat([]os.FileInfo{fi}), nil } - return fs.listAllPages(r.Filepath) + return fs.listAllPages(absPath) } func (fs *SftpServer) listAllPages(dirPath string) (sftp.ListerAt, error) { @@ -259,18 +265,18 @@ func (fs *SftpServer) fetchDirectoryPage(dirPath, start string) ([]os.FileInfo, } // makeDir creates a new directory with proper permissions. -func (fs *SftpServer) makeDir(r *sftp.Request) error { +func (fs *SftpServer) makeDir(absPath string) error { if fs.user == nil { return fmt.Errorf("cannot create directory: no user info") } - dir, name := util.FullPath(r.Filepath).DirAndName() - if err := fs.checkFilePermission(r.Filepath, "mkdir"); err != nil { + dir, name := util.FullPath(absPath).DirAndName() + if err := fs.checkFilePermission(absPath, "mkdir"); err != nil { return err } // default mode and ownership err := filer_pb.Mkdir(context.Background(), fs, string(dir), name, func(entry *filer_pb.Entry) { mode := uint32(0755 | os.ModeDir) - if strings.HasPrefix(r.Filepath, fs.user.HomeDir) { + if strings.HasPrefix(absPath, fs.user.HomeDir) { mode = uint32(0700 | os.ModeDir) } entry.Attributes.FileMode = mode @@ -288,8 +294,8 @@ func (fs *SftpServer) makeDir(r *sftp.Request) error { } // removeDir deletes a directory. -func (fs *SftpServer) removeDir(r *sftp.Request) error { - return fs.deleteEntry(r.Filepath, false) +func (fs *SftpServer) removeDir(absPath string) error { + return fs.deleteEntry(absPath, false) } func (fs *SftpServer) putFile(filepath string, reader io.Reader, user *user.User) error { diff --git a/weed/sftpd/sftp_server.go b/weed/sftpd/sftp_server.go index f158aeb64..0b1827ccb 100644 --- a/weed/sftpd/sftp_server.go +++ b/weed/sftpd/sftp_server.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "path" "time" "github.com/pkg/sftp" @@ -37,6 +38,32 @@ func NewSftpServer(filerAddr pb.ServerAddress, grpcDialOption grpc.DialOption, d } } +// toAbsolutePath translates a user-relative path to an absolute filer path. +// When a user has HomeDir="/sftp/user", their view of "/" maps to "/sftp/user". +// This implements chroot-like behavior where the user's home directory +// becomes their root. +func (fs *SftpServer) toAbsolutePath(userPath string) string { + // If user has root as home directory, no translation needed + if fs.user.HomeDir == "" || fs.user.HomeDir == "/" { + return userPath + } + + // Clean the path to normalize it + cleanPath := path.Clean(userPath) + if cleanPath == "." { + cleanPath = "/" + } + + // Join the home directory with the user path + // path.Join handles the case where cleanPath starts with "/" + // by treating it as relative to the home directory + if cleanPath == "/" { + return fs.user.HomeDir + } + + return path.Join(fs.user.HomeDir, cleanPath) +} + // Fileread is invoked for “get” requests. func (fs *SftpServer) Fileread(req *sftp.Request) (io.ReaderAt, error) { return fs.readFile(req) From 4f922bb69e12b0fedbefcb793510321f6a2b2bff Mon Sep 17 00:00:00 2001 From: chrislu Date: Tue, 2 Dec 2025 23:30:53 -0800 Subject: [PATCH 2/5] test: add SFTP integration tests Add comprehensive integration tests for the SFTP server including: - HomeDir path translation tests (verifies fix for issue #7470) - Basic file upload/download operations - Directory operations (mkdir, rmdir, list) - Large file handling (1MB test) - File rename operations - Stat/Lstat operations - Path edge cases (trailing slashes, .., unicode filenames) - Admin root access verification The test framework starts a complete SeaweedFS cluster with: - Master server - Volume server - Filer server - SFTP server with test user credentials Test users are configured in testdata/userstore.json: - admin: HomeDir=/ with full access - testuser: HomeDir=/sftp/testuser with access to home - readonly: HomeDir=/public with read-only access --- test/sftp/Makefile | 39 ++ test/sftp/README.md | 91 +++++ test/sftp/basic_test.go | 606 ++++++++++++++++++++++++++++++ test/sftp/framework.go | 364 ++++++++++++++++++ test/sftp/go.mod | 17 + test/sftp/go.sum | 54 +++ test/sftp/testdata/userstore.json | 36 ++ 7 files changed, 1207 insertions(+) create mode 100644 test/sftp/Makefile create mode 100644 test/sftp/README.md create mode 100644 test/sftp/basic_test.go create mode 100644 test/sftp/framework.go create mode 100644 test/sftp/go.mod create mode 100644 test/sftp/go.sum create mode 100644 test/sftp/testdata/userstore.json diff --git a/test/sftp/Makefile b/test/sftp/Makefile new file mode 100644 index 000000000..f5592b122 --- /dev/null +++ b/test/sftp/Makefile @@ -0,0 +1,39 @@ +.PHONY: build test test-verbose test-short clean deps + +# Build the weed binary first +build: + cd ../../weed && go build -o weed . + +# Install test dependencies +deps: + go mod download + +# Run all tests +test: build deps + go test -v -timeout 5m ./... + +# Run tests with verbose output +test-verbose: build deps + go test -v -timeout 5m ./... + +# Run quick tests only (skip integration tests) +test-short: deps + go test -short -v ./... + +# Run specific test +test-homedir: build deps + go test -v -timeout 5m -run TestHomeDirPathTranslation ./... + +# Run tests with debug output from SeaweedFS +test-debug: build deps + go test -v -timeout 5m ./... 2>&1 | tee test.log + +# Clean up test artifacts +clean: + rm -f test.log + go clean -testcache + +# Update go.sum +tidy: + go mod tidy + diff --git a/test/sftp/README.md b/test/sftp/README.md new file mode 100644 index 000000000..e2908f166 --- /dev/null +++ b/test/sftp/README.md @@ -0,0 +1,91 @@ +# SeaweedFS SFTP Integration Tests + +This directory contains integration tests for the SeaweedFS SFTP server. + +## Prerequisites + +1. Build the SeaweedFS binary: + ```bash + cd ../../weed + go build -o weed . + ``` + +2. Ensure `ssh-keygen` is available (for generating test SSH host keys) + +## Running Tests + +### Run all tests +```bash +make test +``` + +### Run tests with verbose output +```bash +make test-verbose +``` + +### Run a specific test +```bash +go test -v -run TestHomeDirPathTranslation +``` + +### Skip long-running tests +```bash +go test -short ./... +``` + +## Test Structure + +- `framework.go` - Test framework that starts SeaweedFS cluster with SFTP +- `basic_test.go` - Basic SFTP operation tests including: + - HomeDir path translation (fixes issue #7470) + - File upload/download + - Directory operations + - Large file handling + - Edge cases + +## Test Configuration + +Tests use `testdata/userstore.json` which defines test users: + +| Username | Password | HomeDir | Permissions | +|----------|----------|---------|-------------| +| admin | adminpassword | / | Full access | +| testuser | testuserpassword | /sftp/testuser | Full access to home | +| readonly | readonlypassword | /public | Read-only | + +## Key Tests + +### TestHomeDirPathTranslation + +Tests the fix for [issue #7470](https://github.com/seaweedfs/seaweedfs/issues/7470) where +users with a non-root HomeDir (e.g., `/sftp/testuser`) could not upload files to `/` +because the path wasn't being translated to their home directory. + +The test verifies: +- Uploading to `/` correctly maps to the user's HomeDir +- Creating directories at `/` works +- Listing `/` shows the user's home directory contents +- All path operations respect the HomeDir translation + +## Debugging + +To debug test failures: + +1. Enable verbose output: + ```bash + go test -v -run TestName + ``` + +2. Keep test artifacts (don't cleanup): + ```go + config := DefaultTestConfig() + config.SkipCleanup = true + ``` + +3. Enable debug logging: + ```go + config := DefaultTestConfig() + config.EnableDebug = true + ``` + diff --git a/test/sftp/basic_test.go b/test/sftp/basic_test.go new file mode 100644 index 000000000..e3191ca10 --- /dev/null +++ b/test/sftp/basic_test.go @@ -0,0 +1,606 @@ +package sftp + +import ( + "bytes" + "io" + "path" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestHomeDirPathTranslation tests that SFTP operations correctly translate +// paths relative to the user's HomeDir. +// This is the fix for https://github.com/seaweedfs/seaweedfs/issues/7470 +func TestHomeDirPathTranslation(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + config := DefaultTestConfig() + config.EnableDebug = testing.Verbose() + + fw := NewSftpTestFramework(t, config) + err := fw.Setup(config) + require.NoError(t, err, "failed to setup test framework") + defer fw.Cleanup() + + // Test with user "testuser" who has HomeDir="/sftp/testuser" + // When they upload to "/", it should actually go to "/sftp/testuser" + sftpClient, sshConn, err := fw.ConnectSFTP("testuser", "testuserpassword") + require.NoError(t, err, "failed to connect as testuser") + defer sshConn.Close() + defer sftpClient.Close() + + // Test 1: Upload file to "/" (should map to /sftp/testuser/) + t.Run("UploadToRoot", func(t *testing.T) { + testContent := []byte("Hello from SFTP test!") + filename := "test_upload.txt" + + // Create file at "/" from user's perspective + file, err := sftpClient.Create("/" + filename) + require.NoError(t, err, "should be able to create file at /") + + _, err = file.Write(testContent) + require.NoError(t, err, "should be able to write to file") + err = file.Close() + require.NoError(t, err, "should be able to close file") + + // Verify file exists and has correct content + readFile, err := sftpClient.Open("/" + filename) + require.NoError(t, err, "should be able to open file") + defer readFile.Close() + + content, err := io.ReadAll(readFile) + require.NoError(t, err, "should be able to read file") + require.Equal(t, testContent, content, "file content should match") + + // Clean up + err = sftpClient.Remove("/" + filename) + require.NoError(t, err, "should be able to remove file") + }) + + // Test 2: Create directory at "/" (should map to /sftp/testuser/) + t.Run("CreateDirAtRoot", func(t *testing.T) { + dirname := "test_dir" + + err := sftpClient.Mkdir("/" + dirname) + require.NoError(t, err, "should be able to create directory at /") + + // Verify directory exists + info, err := sftpClient.Stat("/" + dirname) + require.NoError(t, err, "should be able to stat directory") + require.True(t, info.IsDir(), "should be a directory") + + // Clean up + err = sftpClient.RemoveDirectory("/" + dirname) + require.NoError(t, err, "should be able to remove directory") + }) + + // Test 3: List directory at "/" (should list /sftp/testuser/) + t.Run("ListRoot", func(t *testing.T) { + // Create a test file first + testContent := []byte("list test content") + filename := "list_test.txt" + + file, err := sftpClient.Create("/" + filename) + require.NoError(t, err) + _, err = file.Write(testContent) + require.NoError(t, err) + file.Close() + + // List root directory + files, err := sftpClient.ReadDir("/") + require.NoError(t, err, "should be able to list root directory") + + // Should find our test file + found := false + for _, f := range files { + if f.Name() == filename { + found = true + break + } + } + require.True(t, found, "should find test file in listing") + + // Clean up + err = sftpClient.Remove("/" + filename) + require.NoError(t, err) + }) + + // Test 4: Nested directory operations + t.Run("NestedOperations", func(t *testing.T) { + // Create nested directory structure + err := sftpClient.MkdirAll("/nested/dir/structure") + require.NoError(t, err, "should be able to create nested directories") + + // Create file in nested directory + testContent := []byte("nested file content") + file, err := sftpClient.Create("/nested/dir/structure/file.txt") + require.NoError(t, err) + _, err = file.Write(testContent) + require.NoError(t, err) + file.Close() + + // Verify file exists + readFile, err := sftpClient.Open("/nested/dir/structure/file.txt") + require.NoError(t, err) + content, err := io.ReadAll(readFile) + require.NoError(t, err) + readFile.Close() + require.Equal(t, testContent, content) + + // Clean up + err = sftpClient.Remove("/nested/dir/structure/file.txt") + require.NoError(t, err) + err = sftpClient.RemoveDirectory("/nested/dir/structure") + require.NoError(t, err) + err = sftpClient.RemoveDirectory("/nested/dir") + require.NoError(t, err) + err = sftpClient.RemoveDirectory("/nested") + require.NoError(t, err) + }) + + // Test 5: Rename operation + t.Run("RenameFile", func(t *testing.T) { + testContent := []byte("rename test content") + + file, err := sftpClient.Create("/original.txt") + require.NoError(t, err) + _, err = file.Write(testContent) + require.NoError(t, err) + file.Close() + + // Rename file + err = sftpClient.Rename("/original.txt", "/renamed.txt") + require.NoError(t, err, "should be able to rename file") + + // Verify old file doesn't exist + _, err = sftpClient.Stat("/original.txt") + require.Error(t, err, "original file should not exist") + + // Verify new file exists with correct content + readFile, err := sftpClient.Open("/renamed.txt") + require.NoError(t, err, "renamed file should exist") + content, err := io.ReadAll(readFile) + require.NoError(t, err) + readFile.Close() + require.Equal(t, testContent, content) + + // Clean up + err = sftpClient.Remove("/renamed.txt") + require.NoError(t, err) + }) +} + +// TestAdminRootAccess tests that admin user with HomeDir="/" can access everything +func TestAdminRootAccess(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + config := DefaultTestConfig() + config.EnableDebug = testing.Verbose() + + fw := NewSftpTestFramework(t, config) + err := fw.Setup(config) + require.NoError(t, err, "failed to setup test framework") + defer fw.Cleanup() + + // Connect as admin with HomeDir="/" + sftpClient, sshConn, err := fw.ConnectSFTP("admin", "adminpassword") + require.NoError(t, err, "failed to connect as admin") + defer sshConn.Close() + defer sftpClient.Close() + + // Admin should be able to create directories anywhere + t.Run("CreateAnyDirectory", func(t *testing.T) { + // Create the user's home directory structure + err := sftpClient.MkdirAll("/sftp/testuser") + require.NoError(t, err, "admin should be able to create any directory") + + // Create file in that directory + testContent := []byte("admin created this") + file, err := sftpClient.Create("/sftp/testuser/admin_file.txt") + require.NoError(t, err) + _, err = file.Write(testContent) + require.NoError(t, err) + file.Close() + + // Verify file exists + info, err := sftpClient.Stat("/sftp/testuser/admin_file.txt") + require.NoError(t, err) + require.False(t, info.IsDir()) + + // Clean up + err = sftpClient.Remove("/sftp/testuser/admin_file.txt") + require.NoError(t, err) + }) +} + +// TestLargeFileUpload tests uploading larger files through SFTP +func TestLargeFileUpload(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + config := DefaultTestConfig() + config.EnableDebug = testing.Verbose() + + fw := NewSftpTestFramework(t, config) + err := fw.Setup(config) + require.NoError(t, err, "failed to setup test framework") + defer fw.Cleanup() + + sftpClient, sshConn, err := fw.ConnectSFTP("testuser", "testuserpassword") + require.NoError(t, err, "failed to connect as testuser") + defer sshConn.Close() + defer sftpClient.Close() + + // Create a 1MB file + t.Run("Upload1MB", func(t *testing.T) { + size := 1024 * 1024 // 1MB + testData := bytes.Repeat([]byte("A"), size) + + file, err := sftpClient.Create("/large_file.bin") + require.NoError(t, err) + n, err := file.Write(testData) + require.NoError(t, err) + require.Equal(t, size, n) + file.Close() + + // Verify file size + info, err := sftpClient.Stat("/large_file.bin") + require.NoError(t, err) + require.Equal(t, int64(size), info.Size()) + + // Verify content + readFile, err := sftpClient.Open("/large_file.bin") + require.NoError(t, err) + content, err := io.ReadAll(readFile) + require.NoError(t, err) + readFile.Close() + require.Equal(t, testData, content) + + // Clean up + err = sftpClient.Remove("/large_file.bin") + require.NoError(t, err) + }) +} + +// TestStatOperations tests Stat and Lstat operations +func TestStatOperations(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + config := DefaultTestConfig() + config.EnableDebug = testing.Verbose() + + fw := NewSftpTestFramework(t, config) + err := fw.Setup(config) + require.NoError(t, err, "failed to setup test framework") + defer fw.Cleanup() + + sftpClient, sshConn, err := fw.ConnectSFTP("testuser", "testuserpassword") + require.NoError(t, err, "failed to connect as testuser") + defer sshConn.Close() + defer sftpClient.Close() + + // Create a test file + testContent := []byte("stat test content") + file, err := sftpClient.Create("/stat_test.txt") + require.NoError(t, err) + _, err = file.Write(testContent) + require.NoError(t, err) + file.Close() + + t.Run("StatFile", func(t *testing.T) { + info, err := sftpClient.Stat("/stat_test.txt") + require.NoError(t, err) + require.Equal(t, "stat_test.txt", info.Name()) + require.Equal(t, int64(len(testContent)), info.Size()) + require.False(t, info.IsDir()) + }) + + t.Run("StatDirectory", func(t *testing.T) { + err := sftpClient.Mkdir("/stat_dir") + require.NoError(t, err) + + info, err := sftpClient.Stat("/stat_dir") + require.NoError(t, err) + require.Equal(t, "stat_dir", info.Name()) + require.True(t, info.IsDir()) + + // Clean up + err = sftpClient.RemoveDirectory("/stat_dir") + require.NoError(t, err) + }) + + t.Run("StatRoot", func(t *testing.T) { + // Should be able to stat "/" which maps to user's home directory + info, err := sftpClient.Stat("/") + require.NoError(t, err, "should be able to stat root (home) directory") + require.True(t, info.IsDir(), "root should be a directory") + }) + + // Clean up + err = sftpClient.Remove("/stat_test.txt") + require.NoError(t, err) +} + +// TestWalk tests walking directory trees +func TestWalk(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + config := DefaultTestConfig() + config.EnableDebug = testing.Verbose() + + fw := NewSftpTestFramework(t, config) + err := fw.Setup(config) + require.NoError(t, err, "failed to setup test framework") + defer fw.Cleanup() + + sftpClient, sshConn, err := fw.ConnectSFTP("testuser", "testuserpassword") + require.NoError(t, err, "failed to connect as testuser") + defer sshConn.Close() + defer sftpClient.Close() + + // Create directory structure + err = sftpClient.MkdirAll("/walk/a/b") + require.NoError(t, err) + err = sftpClient.MkdirAll("/walk/c") + require.NoError(t, err) + + // Create files + for _, p := range []string{"/walk/file1.txt", "/walk/a/file2.txt", "/walk/a/b/file3.txt", "/walk/c/file4.txt"} { + file, err := sftpClient.Create(p) + require.NoError(t, err) + file.Write([]byte("test")) + file.Close() + } + + t.Run("WalkEntireTree", func(t *testing.T) { + var paths []string + walker := sftpClient.Walk("/walk") + for walker.Step() { + if walker.Err() != nil { + continue + } + paths = append(paths, walker.Path()) + } + + // Should find all directories and files + require.Contains(t, paths, "/walk") + require.Contains(t, paths, "/walk/a") + require.Contains(t, paths, "/walk/a/b") + require.Contains(t, paths, "/walk/c") + }) + + // Clean up + for _, p := range []string{"/walk/file1.txt", "/walk/a/file2.txt", "/walk/a/b/file3.txt", "/walk/c/file4.txt"} { + sftpClient.Remove(p) + } + for _, p := range []string{"/walk/a/b", "/walk/a", "/walk/c", "/walk"} { + sftpClient.RemoveDirectory(p) + } +} + +// TestCurrentWorkingDirectory tests that Getwd and Chdir work correctly +func TestCurrentWorkingDirectory(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + config := DefaultTestConfig() + config.EnableDebug = testing.Verbose() + + fw := NewSftpTestFramework(t, config) + err := fw.Setup(config) + require.NoError(t, err, "failed to setup test framework") + defer fw.Cleanup() + + sftpClient, sshConn, err := fw.ConnectSFTP("testuser", "testuserpassword") + require.NoError(t, err, "failed to connect as testuser") + defer sshConn.Close() + defer sftpClient.Close() + + // Create test directory + err = sftpClient.Mkdir("/cwd_test") + require.NoError(t, err) + + t.Run("GetCurrentDir", func(t *testing.T) { + cwd, err := sftpClient.Getwd() + require.NoError(t, err) + // The initial working directory should be the user's home directory + // which from the user's perspective is "/" + require.NotEmpty(t, cwd) + }) + + t.Run("ChangeAndCreate", func(t *testing.T) { + // Create file in subdirectory using relative path after chdir + // Note: pkg/sftp doesn't support Chdir, so we test using absolute paths + file, err := sftpClient.Create("/cwd_test/relative_file.txt") + require.NoError(t, err) + file.Write([]byte("test")) + file.Close() + + // Verify using absolute path + _, err = sftpClient.Stat("/cwd_test/relative_file.txt") + require.NoError(t, err) + + // Clean up + sftpClient.Remove("/cwd_test/relative_file.txt") + }) + + // Clean up + err = sftpClient.RemoveDirectory("/cwd_test") + require.NoError(t, err) +} + +// TestPathEdgeCases tests various edge cases in path handling +func TestPathEdgeCases(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + config := DefaultTestConfig() + config.EnableDebug = testing.Verbose() + + fw := NewSftpTestFramework(t, config) + err := fw.Setup(config) + require.NoError(t, err, "failed to setup test framework") + defer fw.Cleanup() + + sftpClient, sshConn, err := fw.ConnectSFTP("testuser", "testuserpassword") + require.NoError(t, err, "failed to connect as testuser") + defer sshConn.Close() + defer sftpClient.Close() + + t.Run("PathWithDotDot", func(t *testing.T) { + // Create directory structure + err := sftpClient.MkdirAll("/edge/subdir") + require.NoError(t, err) + + // Create file using path with .. + file, err := sftpClient.Create("/edge/subdir/../file.txt") + require.NoError(t, err) + file.Write([]byte("test")) + file.Close() + + // Verify file was created in /edge + _, err = sftpClient.Stat("/edge/file.txt") + require.NoError(t, err, "file should be created in parent directory") + + // Clean up + sftpClient.Remove("/edge/file.txt") + sftpClient.RemoveDirectory("/edge/subdir") + sftpClient.RemoveDirectory("/edge") + }) + + t.Run("PathWithTrailingSlash", func(t *testing.T) { + err := sftpClient.Mkdir("/trailing") + require.NoError(t, err) + + // Stat with trailing slash + info, err := sftpClient.Stat("/trailing/") + require.NoError(t, err) + require.True(t, info.IsDir()) + + // Clean up + sftpClient.RemoveDirectory("/trailing") + }) + + t.Run("CreateFileAtRootPath", func(t *testing.T) { + // This is the exact scenario from issue #7470 + // User with HomeDir="/sftp/testuser" uploads to "/" + file, err := sftpClient.Create("/issue7470.txt") + require.NoError(t, err, "should be able to create file at / (issue #7470)") + file.Write([]byte("This tests the fix for issue #7470")) + file.Close() + + // Verify + _, err = sftpClient.Stat("/issue7470.txt") + require.NoError(t, err) + + // Clean up + sftpClient.Remove("/issue7470.txt") + }) +} + +// TestFileContent tests reading and writing file content correctly +func TestFileContent(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + config := DefaultTestConfig() + config.EnableDebug = testing.Verbose() + + fw := NewSftpTestFramework(t, config) + err := fw.Setup(config) + require.NoError(t, err, "failed to setup test framework") + defer fw.Cleanup() + + sftpClient, sshConn, err := fw.ConnectSFTP("testuser", "testuserpassword") + require.NoError(t, err, "failed to connect as testuser") + defer sshConn.Close() + defer sftpClient.Close() + + t.Run("BinaryContent", func(t *testing.T) { + // Create binary data with all byte values + data := make([]byte, 256) + for i := 0; i < 256; i++ { + data[i] = byte(i) + } + + file, err := sftpClient.Create("/binary.bin") + require.NoError(t, err) + n, err := file.Write(data) + require.NoError(t, err) + require.Equal(t, 256, n) + file.Close() + + // Read back + readFile, err := sftpClient.Open("/binary.bin") + require.NoError(t, err) + content, err := io.ReadAll(readFile) + require.NoError(t, err) + readFile.Close() + + require.Equal(t, data, content, "binary content should match") + + // Clean up + sftpClient.Remove("/binary.bin") + }) + + t.Run("EmptyFile", func(t *testing.T) { + file, err := sftpClient.Create("/empty.txt") + require.NoError(t, err) + file.Close() + + info, err := sftpClient.Stat("/empty.txt") + require.NoError(t, err) + require.Equal(t, int64(0), info.Size()) + + // Clean up + sftpClient.Remove("/empty.txt") + }) + + t.Run("UnicodeFilename", func(t *testing.T) { + filename := "/文件名.txt" + content := []byte("Unicode content: 你好世界") + + file, err := sftpClient.Create(filename) + require.NoError(t, err) + file.Write(content) + file.Close() + + // Read back + readFile, err := sftpClient.Open(filename) + require.NoError(t, err) + readContent, err := io.ReadAll(readFile) + require.NoError(t, err) + readFile.Close() + + require.Equal(t, content, readContent) + + // Verify in listing + files, err := sftpClient.ReadDir("/") + require.NoError(t, err) + found := false + for _, f := range files { + if f.Name() == path.Base(filename) { + found = true + break + } + } + require.True(t, found, "should find unicode filename in listing") + + // Clean up + sftpClient.Remove(filename) + }) +} + diff --git a/test/sftp/framework.go b/test/sftp/framework.go new file mode 100644 index 000000000..351424dda --- /dev/null +++ b/test/sftp/framework.go @@ -0,0 +1,364 @@ +package sftp + +import ( + "fmt" + "net" + "os" + "os/exec" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/pkg/sftp" + "github.com/stretchr/testify/require" + "golang.org/x/crypto/ssh" +) + +// SftpTestFramework provides utilities for SFTP integration testing +type SftpTestFramework struct { + t *testing.T + tempDir string + dataDir string + masterProcess *os.Process + volumeProcess *os.Process + filerProcess *os.Process + sftpProcess *os.Process + masterAddr string + volumeAddr string + filerAddr string + sftpAddr string + weedBinary string + userStoreFile string + hostKeyFile string + isSetup bool +} + +// TestConfig holds configuration for SFTP tests +type TestConfig struct { + NumVolumes int + EnableDebug bool + SkipCleanup bool // for debugging failed tests + UserStoreFile string +} + +// DefaultTestConfig returns a default configuration for SFTP tests +func DefaultTestConfig() *TestConfig { + return &TestConfig{ + NumVolumes: 3, + EnableDebug: false, + SkipCleanup: false, + UserStoreFile: "", + } +} + +// NewSftpTestFramework creates a new SFTP testing framework +func NewSftpTestFramework(t *testing.T, config *TestConfig) *SftpTestFramework { + if config == nil { + config = DefaultTestConfig() + } + + tempDir, err := os.MkdirTemp("", "seaweedfs_sftp_test_") + require.NoError(t, err) + + // Generate SSH host key for SFTP server + hostKeyFile := filepath.Join(tempDir, "ssh_host_key") + cmd := exec.Command("ssh-keygen", "-t", "ed25519", "-f", hostKeyFile, "-N", "") + err = cmd.Run() + require.NoError(t, err, "failed to generate SSH host key") + + // Use provided userstore or copy the test one + userStoreFile := config.UserStoreFile + if userStoreFile == "" { + // Copy test userstore to temp dir + userStoreFile = filepath.Join(tempDir, "userstore.json") + testDataPath := findTestDataPath() + input, err := os.ReadFile(filepath.Join(testDataPath, "userstore.json")) + require.NoError(t, err, "failed to read test userstore.json") + err = os.WriteFile(userStoreFile, input, 0644) + require.NoError(t, err, "failed to write userstore.json") + } + + return &SftpTestFramework{ + t: t, + tempDir: tempDir, + dataDir: filepath.Join(tempDir, "data"), + masterAddr: "127.0.0.1:19333", + volumeAddr: "127.0.0.1:18080", + filerAddr: "127.0.0.1:18888", + sftpAddr: "127.0.0.1:12022", + weedBinary: findWeedBinary(), + userStoreFile: userStoreFile, + hostKeyFile: hostKeyFile, + isSetup: false, + } +} + +// Setup starts SeaweedFS cluster with SFTP server +func (f *SftpTestFramework) Setup(config *TestConfig) error { + if f.isSetup { + return fmt.Errorf("framework already setup") + } + + // Create data directory + if err := os.MkdirAll(f.dataDir, 0755); err != nil { + return fmt.Errorf("failed to create data directory: %v", err) + } + + // Start master + if err := f.startMaster(config); err != nil { + return fmt.Errorf("failed to start master: %v", err) + } + + // Wait for master to be ready + if err := f.waitForService(f.masterAddr, 30*time.Second); err != nil { + return fmt.Errorf("master not ready: %v", err) + } + + // Start volume server + if err := f.startVolumeServer(config); err != nil { + return fmt.Errorf("failed to start volume server: %v", err) + } + + // Wait for volume server to be ready + if err := f.waitForService(f.volumeAddr, 30*time.Second); err != nil { + return fmt.Errorf("volume server not ready: %v", err) + } + + // Start filer + if err := f.startFiler(config); err != nil { + return fmt.Errorf("failed to start filer: %v", err) + } + + // Wait for filer to be ready + if err := f.waitForService(f.filerAddr, 30*time.Second); err != nil { + return fmt.Errorf("filer not ready: %v", err) + } + + // Start SFTP server + if err := f.startSftpServer(config); err != nil { + return fmt.Errorf("failed to start SFTP server: %v", err) + } + + // Wait for SFTP server to be ready + if err := f.waitForService(f.sftpAddr, 30*time.Second); err != nil { + return fmt.Errorf("SFTP server not ready: %v", err) + } + + f.isSetup = true + return nil +} + +// Cleanup stops all processes and removes temporary files +func (f *SftpTestFramework) Cleanup() { + // Stop processes in reverse order + processes := []*os.Process{f.sftpProcess, f.filerProcess, f.volumeProcess, f.masterProcess} + for _, proc := range processes { + if proc != nil { + proc.Signal(syscall.SIGTERM) + proc.Wait() + } + } + + // Remove temp directory + if !DefaultTestConfig().SkipCleanup { + os.RemoveAll(f.tempDir) + } +} + +// GetSftpAddr returns the SFTP server address +func (f *SftpTestFramework) GetSftpAddr() string { + return f.sftpAddr +} + +// GetFilerAddr returns the filer address +func (f *SftpTestFramework) GetFilerAddr() string { + return f.filerAddr +} + +// ConnectSFTP creates an SFTP client connection with the given credentials +func (f *SftpTestFramework) ConnectSFTP(username, password string) (*sftp.Client, *ssh.Client, error) { + config := &ssh.ClientConfig{ + User: username, + Auth: []ssh.AuthMethod{ + ssh.Password(password), + }, + HostKeyCallback: ssh.InsecureIgnoreHostKey(), + Timeout: 5 * time.Second, + } + + sshConn, err := ssh.Dial("tcp", f.sftpAddr, config) + if err != nil { + return nil, nil, fmt.Errorf("failed to connect SSH: %v", err) + } + + sftpClient, err := sftp.NewClient(sshConn) + if err != nil { + sshConn.Close() + return nil, nil, fmt.Errorf("failed to create SFTP client: %v", err) + } + + return sftpClient, sshConn, nil +} + +// startMaster starts the SeaweedFS master server +func (f *SftpTestFramework) startMaster(config *TestConfig) error { + args := []string{ + "master", + "-ip=127.0.0.1", + "-port=19333", + "-mdir=" + filepath.Join(f.dataDir, "master"), + "-raftBootstrap", + } + if config.EnableDebug { + args = append(args, "-v=4") + } + + cmd := exec.Command(f.weedBinary, args...) + cmd.Dir = f.tempDir + if config.EnableDebug { + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + } + if err := cmd.Start(); err != nil { + return err + } + f.masterProcess = cmd.Process + return nil +} + +// startVolumeServer starts SeaweedFS volume server +func (f *SftpTestFramework) startVolumeServer(config *TestConfig) error { + args := []string{ + "volume", + "-mserver=" + f.masterAddr, + "-ip=127.0.0.1", + "-port=18080", + "-dir=" + filepath.Join(f.dataDir, "volume"), + fmt.Sprintf("-max=%d", config.NumVolumes), + } + if config.EnableDebug { + args = append(args, "-v=4") + } + + cmd := exec.Command(f.weedBinary, args...) + cmd.Dir = f.tempDir + if config.EnableDebug { + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + } + if err := cmd.Start(); err != nil { + return err + } + f.volumeProcess = cmd.Process + return nil +} + +// startFiler starts the SeaweedFS filer server +func (f *SftpTestFramework) startFiler(config *TestConfig) error { + args := []string{ + "filer", + "-master=" + f.masterAddr, + "-ip=127.0.0.1", + "-port=18888", + } + if config.EnableDebug { + args = append(args, "-v=4") + } + + cmd := exec.Command(f.weedBinary, args...) + cmd.Dir = f.tempDir + if config.EnableDebug { + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + } + if err := cmd.Start(); err != nil { + return err + } + f.filerProcess = cmd.Process + return nil +} + +// startSftpServer starts the SeaweedFS SFTP server +func (f *SftpTestFramework) startSftpServer(config *TestConfig) error { + args := []string{ + "sftp", + "-filer=" + f.filerAddr, + "-ip.bind=127.0.0.1", + "-port=12022", + "-sshPrivateKey=" + f.hostKeyFile, + "-userStoreFile=" + f.userStoreFile, + } + if config.EnableDebug { + args = append(args, "-v=4") + } + + cmd := exec.Command(f.weedBinary, args...) + cmd.Dir = f.tempDir + if config.EnableDebug { + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + } + if err := cmd.Start(); err != nil { + return err + } + f.sftpProcess = cmd.Process + return nil +} + +// waitForService waits for a service to be available +func (f *SftpTestFramework) waitForService(addr string, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + conn, err := net.DialTimeout("tcp", addr, 1*time.Second) + if err == nil { + conn.Close() + return nil + } + time.Sleep(100 * time.Millisecond) + } + return fmt.Errorf("service at %s not ready within timeout", addr) +} + +// findWeedBinary locates the weed binary +func findWeedBinary() string { + // Try different possible locations + candidates := []string{ + "../../weed/weed", + "../weed/weed", + "./weed", + "weed", // in PATH + } + + for _, candidate := range candidates { + if _, err := exec.LookPath(candidate); err == nil { + return candidate + } + if _, err := os.Stat(candidate); err == nil { + abs, _ := filepath.Abs(candidate) + return abs + } + } + + // Default fallback + return "weed" +} + +// findTestDataPath locates the testdata directory +func findTestDataPath() string { + candidates := []string{ + "./testdata", + "../sftp/testdata", + "test/sftp/testdata", + } + + for _, candidate := range candidates { + if _, err := os.Stat(candidate); err == nil { + abs, _ := filepath.Abs(candidate) + return abs + } + } + + return "./testdata" +} + diff --git a/test/sftp/go.mod b/test/sftp/go.mod new file mode 100644 index 000000000..9cfef5247 --- /dev/null +++ b/test/sftp/go.mod @@ -0,0 +1,17 @@ +module seaweedfs-sftp-tests + +go 1.21 + +require ( + github.com/pkg/sftp v1.13.6 + github.com/stretchr/testify v1.8.4 + golang.org/x/crypto v0.17.0 +) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/kr/fs v0.1.0 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + golang.org/x/sys v0.15.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/test/sftp/go.sum b/test/sftp/go.sum new file mode 100644 index 000000000..2b3b30535 --- /dev/null +++ b/test/sftp/go.sum @@ -0,0 +1,54 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/kr/fs v0.1.0 h1:Jskdu9ieNAYnjxsi0LbQp1ulIKZV1LAFgK1tWhpZgl8= +github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= +github.com/pkg/sftp v1.13.6 h1:JFZT4XbOU7l77xGSpOdW+pwIMqP044IyjXX6FGyEKFo= +github.com/pkg/sftp v1.13.6/go.mod h1:tz1ryNURKu77RL+GuCzmoJYxQczL3wLNNpPWagdg4Qk= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= +golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= +golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= +golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= +golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= +golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= +golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.15.0 h1:y/Oo/a/q3IXu26lQgl04j/gjuBDOBlx7X6Om1j2CPW4= +golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= +golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/test/sftp/testdata/userstore.json b/test/sftp/testdata/userstore.json new file mode 100644 index 000000000..540a9486d --- /dev/null +++ b/test/sftp/testdata/userstore.json @@ -0,0 +1,36 @@ +[ + { + "Username": "admin", + "Password": "adminpassword", + "PublicKeys": [], + "HomeDir": "/", + "Permissions": { + "/": ["*"] + }, + "Uid": 0, + "Gid": 0 + }, + { + "Username": "testuser", + "Password": "testuserpassword", + "PublicKeys": [], + "HomeDir": "/sftp/testuser", + "Permissions": { + "/sftp/testuser": ["*"] + }, + "Uid": 1001, + "Gid": 1001 + }, + { + "Username": "readonly", + "Password": "readonlypassword", + "PublicKeys": [], + "HomeDir": "/public", + "Permissions": { + "/public": ["read", "list"] + }, + "Uid": 1002, + "Gid": 1002 + } +] + From cafbbd3b84c00153722be698ed2aa58c55e24b58 Mon Sep 17 00:00:00 2001 From: chrislu Date: Tue, 2 Dec 2025 23:43:54 -0800 Subject: [PATCH 3/5] fix: correct SFTP HomeDir path translation and add CI Fix path.Join issue where paths starting with '/' weren't joined correctly. path.Join('/sftp/user', '/file') returns '/file' instead of '/sftp/user/file'. Now we strip the leading '/' before joining. Test improvements: - Update go.mod to Go 1.24 - Fix weed binary discovery to prefer local build over PATH - Add stabilization delay after service startup - All 8 SFTP integration tests pass locally Add GitHub Actions workflow for SFTP tests: - Runs on push/PR affecting sftpd code or tests - Tests HomeDir path translation, file ops, directory ops - Covers issue #7470 fix verification --- .github/workflows/sftp-tests.yml | 92 ++++++++++++++++++++++++++++++++ test/sftp/framework.go | 89 ++++++++++++++++++++---------- test/sftp/go.mod | 10 ++-- test/sftp/go.sum | 34 +++++++----- weed/sftpd/sftp_server.go | 14 +++-- 5 files changed, 189 insertions(+), 50 deletions(-) create mode 100644 .github/workflows/sftp-tests.yml diff --git a/.github/workflows/sftp-tests.yml b/.github/workflows/sftp-tests.yml new file mode 100644 index 000000000..d2ec47eb4 --- /dev/null +++ b/.github/workflows/sftp-tests.yml @@ -0,0 +1,92 @@ +name: "SFTP Integration Tests" + +on: + push: + branches: [ master, main ] + paths: + - 'weed/sftpd/**' + - 'weed/command/sftp.go' + - 'test/sftp/**' + - '.github/workflows/sftp-tests.yml' + pull_request: + branches: [ master, main ] + paths: + - 'weed/sftpd/**' + - 'weed/command/sftp.go' + - 'test/sftp/**' + - '.github/workflows/sftp-tests.yml' + +concurrency: + group: ${{ github.head_ref }}/sftp-tests + cancel-in-progress: true + +permissions: + contents: read + +env: + GO_VERSION: '1.24' + TEST_TIMEOUT: '15m' + +jobs: + sftp-integration: + name: SFTP Integration Testing + runs-on: ubuntu-22.04 + timeout-minutes: 20 + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Go ${{ env.GO_VERSION }} + uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + + - name: Install dependencies + run: | + sudo apt-get update + sudo apt-get install -y openssh-client + + - name: Build SeaweedFS + run: | + cd weed + go build -o weed . + chmod +x weed + ./weed version + + - name: Run SFTP Integration Tests + run: | + cd test/sftp + + echo "🧪 Running SFTP integration tests..." + echo "============================================" + + # Install test dependencies + go mod download + + # Run all SFTP tests + go test -v -timeout=${{ env.TEST_TIMEOUT }} ./... + + echo "============================================" + echo "✅ SFTP integration tests completed" + + - name: Test Summary + if: always() + run: | + echo "## 🔐 SFTP Integration Test Summary" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "### Test Coverage" >> $GITHUB_STEP_SUMMARY + echo "- ✅ **HomeDir Path Translation**: User home directory mapping (fixes #7470)" >> $GITHUB_STEP_SUMMARY + echo "- ✅ **File Operations**: Upload, download, delete" >> $GITHUB_STEP_SUMMARY + echo "- ✅ **Directory Operations**: Create, list, remove" >> $GITHUB_STEP_SUMMARY + echo "- ✅ **Large File Handling**: 1MB+ file support" >> $GITHUB_STEP_SUMMARY + echo "- ✅ **Path Edge Cases**: Unicode, trailing slashes, .. paths" >> $GITHUB_STEP_SUMMARY + echo "- ✅ **Admin Access**: Root user verification" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "### Test Configuration" >> $GITHUB_STEP_SUMMARY + echo "| User | HomeDir | Permissions |" >> $GITHUB_STEP_SUMMARY + echo "|------|---------|-------------|" >> $GITHUB_STEP_SUMMARY + echo "| admin | / | Full access |" >> $GITHUB_STEP_SUMMARY + echo "| testuser | /sftp/testuser | Home directory only |" >> $GITHUB_STEP_SUMMARY + echo "| readonly | /public | Read-only |" >> $GITHUB_STEP_SUMMARY + diff --git a/test/sftp/framework.go b/test/sftp/framework.go index 351424dda..26c9f2abc 100644 --- a/test/sftp/framework.go +++ b/test/sftp/framework.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "syscall" "testing" "time" @@ -100,9 +101,16 @@ func (f *SftpTestFramework) Setup(config *TestConfig) error { return fmt.Errorf("framework already setup") } - // Create data directory - if err := os.MkdirAll(f.dataDir, 0755); err != nil { - return fmt.Errorf("failed to create data directory: %v", err) + // Create all data directories + dirs := []string{ + f.dataDir, + filepath.Join(f.dataDir, "master"), + filepath.Join(f.dataDir, "volume"), + } + for _, dir := range dirs { + if err := os.MkdirAll(dir, 0755); err != nil { + return fmt.Errorf("failed to create directory %s: %v", dir, err) + } } // Start master @@ -145,6 +153,9 @@ func (f *SftpTestFramework) Setup(config *TestConfig) error { return fmt.Errorf("SFTP server not ready: %v", err) } + // Additional wait for all services to stabilize (gRPC endpoints) + time.Sleep(500 * time.Millisecond) + f.isSetup = true return nil } @@ -209,9 +220,7 @@ func (f *SftpTestFramework) startMaster(config *TestConfig) error { "-port=19333", "-mdir=" + filepath.Join(f.dataDir, "master"), "-raftBootstrap", - } - if config.EnableDebug { - args = append(args, "-v=4") + "-peers=none", } cmd := exec.Command(f.weedBinary, args...) @@ -237,9 +246,6 @@ func (f *SftpTestFramework) startVolumeServer(config *TestConfig) error { "-dir=" + filepath.Join(f.dataDir, "volume"), fmt.Sprintf("-max=%d", config.NumVolumes), } - if config.EnableDebug { - args = append(args, "-v=4") - } cmd := exec.Command(f.weedBinary, args...) cmd.Dir = f.tempDir @@ -262,9 +268,6 @@ func (f *SftpTestFramework) startFiler(config *TestConfig) error { "-ip=127.0.0.1", "-port=18888", } - if config.EnableDebug { - args = append(args, "-v=4") - } cmd := exec.Command(f.weedBinary, args...) cmd.Dir = f.tempDir @@ -289,9 +292,6 @@ func (f *SftpTestFramework) startSftpServer(config *TestConfig) error { "-sshPrivateKey=" + f.hostKeyFile, "-userStoreFile=" + f.userStoreFile, } - if config.EnableDebug { - args = append(args, "-v=4") - } cmd := exec.Command(f.weedBinary, args...) cmd.Dir = f.tempDir @@ -321,41 +321,72 @@ func (f *SftpTestFramework) waitForService(addr string, timeout time.Duration) e } // findWeedBinary locates the weed binary +// Prefers local build over system-installed weed to ensure we test the latest code func findWeedBinary() string { - // Try different possible locations - candidates := []string{ - "../../weed/weed", - "../weed/weed", - "./weed", - "weed", // in PATH + // Get the directory where this source file is located + // This ensures we find the locally built weed binary first + _, thisFile, _, ok := runtime.Caller(0) + if ok { + thisDir := filepath.Dir(thisFile) + // From test/sftp/, the weed binary should be at ../../weed/weed + candidates := []string{ + filepath.Join(thisDir, "../../weed/weed"), + filepath.Join(thisDir, "../weed/weed"), + } + for _, candidate := range candidates { + if _, err := os.Stat(candidate); err == nil { + abs, _ := filepath.Abs(candidate) + return abs + } + } } + // Try relative paths from current working directory + cwd, _ := os.Getwd() + candidates := []string{ + filepath.Join(cwd, "../../weed/weed"), + filepath.Join(cwd, "../weed/weed"), + filepath.Join(cwd, "./weed"), + } for _, candidate := range candidates { - if _, err := exec.LookPath(candidate); err == nil { - return candidate - } if _, err := os.Stat(candidate); err == nil { abs, _ := filepath.Abs(candidate) return abs } } + // Fallback to PATH only if local build not found + if path, err := exec.LookPath("weed"); err == nil { + return path + } + // Default fallback return "weed" } // findTestDataPath locates the testdata directory func findTestDataPath() string { + // Get the directory where this source file is located + _, thisFile, _, ok := runtime.Caller(0) + if ok { + thisDir := filepath.Dir(thisFile) + testDataPath := filepath.Join(thisDir, "testdata") + if _, err := os.Stat(testDataPath); err == nil { + return testDataPath + } + } + + // Try relative paths from current working directory + cwd, _ := os.Getwd() candidates := []string{ - "./testdata", - "../sftp/testdata", - "test/sftp/testdata", + filepath.Join(cwd, "testdata"), + filepath.Join(cwd, "../sftp/testdata"), + filepath.Join(cwd, "test/sftp/testdata"), } for _, candidate := range candidates { if _, err := os.Stat(candidate); err == nil { - abs, _ := filepath.Abs(candidate) - return abs + return candidate } } diff --git a/test/sftp/go.mod b/test/sftp/go.mod index 9cfef5247..29178d4d3 100644 --- a/test/sftp/go.mod +++ b/test/sftp/go.mod @@ -1,17 +1,17 @@ module seaweedfs-sftp-tests -go 1.21 +go 1.24 require ( - github.com/pkg/sftp v1.13.6 - github.com/stretchr/testify v1.8.4 - golang.org/x/crypto v0.17.0 + github.com/pkg/sftp v1.13.7 + github.com/stretchr/testify v1.10.0 + golang.org/x/crypto v0.31.0 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/kr/fs v0.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/sys v0.15.0 // indirect + golang.org/x/sys v0.28.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/test/sftp/go.sum b/test/sftp/go.sum index 2b3b30535..79a0aa1a1 100644 --- a/test/sftp/go.sum +++ b/test/sftp/go.sum @@ -3,49 +3,59 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/kr/fs v0.1.0 h1:Jskdu9ieNAYnjxsi0LbQp1ulIKZV1LAFgK1tWhpZgl8= github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= -github.com/pkg/sftp v1.13.6 h1:JFZT4XbOU7l77xGSpOdW+pwIMqP044IyjXX6FGyEKFo= -github.com/pkg/sftp v1.13.6/go.mod h1:tz1ryNURKu77RL+GuCzmoJYxQczL3wLNNpPWagdg4Qk= +github.com/pkg/sftp v1.13.7 h1:uv+I3nNJvlKZIQGSr8JVQLNHFU9YhhNpvC14Y6KgmSM= +github.com/pkg/sftp v1.13.7/go.mod h1:KMKI0t3T6hfA+lTR/ssZdunHo+uwq7ghoN09/FSu3DY= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= -github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= +github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= -golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= +golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U= +golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= -golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= +golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= +golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA= +golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.15.0 h1:y/Oo/a/q3IXu26lQgl04j/gjuBDOBlx7X6Om1j2CPW4= +golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= +golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= +golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q= +golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= +golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= +golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/weed/sftpd/sftp_server.go b/weed/sftpd/sftp_server.go index 0b1827ccb..66afc2852 100644 --- a/weed/sftpd/sftp_server.go +++ b/weed/sftpd/sftp_server.go @@ -54,14 +54,20 @@ func (fs *SftpServer) toAbsolutePath(userPath string) string { cleanPath = "/" } - // Join the home directory with the user path - // path.Join handles the case where cleanPath starts with "/" - // by treating it as relative to the home directory + // If the path is exactly "/", return the home directory if cleanPath == "/" { return fs.user.HomeDir } - return path.Join(fs.user.HomeDir, cleanPath) + // Strip leading "/" from the user path and join with home directory + // path.Join("/sftp/user", "/file") would return "/file" (wrong) + // path.Join("/sftp/user", "file") returns "/sftp/user/file" (correct) + relativePath := cleanPath + if len(relativePath) > 0 && relativePath[0] == '/' { + relativePath = relativePath[1:] + } + + return path.Join(fs.user.HomeDir, relativePath) } // Fileread is invoked for “get” requests. From 307075e57739d27e492e1fcc0f6e8529bf4860f1 Mon Sep 17 00:00:00 2001 From: chrislu Date: Tue, 2 Dec 2025 23:45:02 -0800 Subject: [PATCH 4/5] security: update golang.org/x/crypto to v0.45.0 Addresses security vulnerability in golang.org/x/crypto < 0.45.0 --- test/sftp/go.mod | 6 +++--- test/sftp/go.sum | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/test/sftp/go.mod b/test/sftp/go.mod index 29178d4d3..34d9053a8 100644 --- a/test/sftp/go.mod +++ b/test/sftp/go.mod @@ -1,17 +1,17 @@ module seaweedfs-sftp-tests -go 1.24 +go 1.24.0 require ( github.com/pkg/sftp v1.13.7 github.com/stretchr/testify v1.10.0 - golang.org/x/crypto v0.31.0 + golang.org/x/crypto v0.45.0 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/kr/fs v0.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/sys v0.28.0 // indirect + golang.org/x/sys v0.38.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/test/sftp/go.sum b/test/sftp/go.sum index 79a0aa1a1..112e6f88a 100644 --- a/test/sftp/go.sum +++ b/test/sftp/go.sum @@ -17,8 +17,8 @@ github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5t golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= -golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U= -golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk= +golang.org/x/crypto v0.45.0 h1:jMBrvKuj23MTlT0bQEOBcAE0mjg8mK9RXFhRH6nyF3Q= +golang.org/x/crypto v0.45.0/go.mod h1:XTGrrkGJve7CYK7J8PEww4aY7gM3qMCElcJQ8n8JdX4= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= @@ -37,15 +37,15 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA= -golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= +golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= -golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q= -golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM= +golang.org/x/term v0.37.0 h1:8EGAD0qCmHYZg6J17DvsMy9/wJ7/D/4pV/wfnld5lTU= +golang.org/x/term v0.37.0/go.mod h1:5pB4lxRNYYVZuTLmy8oR2BH8dflOR+IbTYFD8fi3254= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= From 5ed6db9dfe347ecb21422c93ad72204aa82b53e9 Mon Sep 17 00:00:00 2001 From: chrislu Date: Tue, 2 Dec 2025 23:46:28 -0800 Subject: [PATCH 5/5] security: use proper SSH host key verification in tests Replace ssh.InsecureIgnoreHostKey() with ssh.FixedHostKey() that verifies the server's host key matches the known test key we generated. This addresses CodeQL warning go/insecure-hostkeycallback. Also updates go.mod to specify go 1.24.0 explicitly. --- test/sftp/framework.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/test/sftp/framework.go b/test/sftp/framework.go index 26c9f2abc..6d0a39880 100644 --- a/test/sftp/framework.go +++ b/test/sftp/framework.go @@ -189,12 +189,18 @@ func (f *SftpTestFramework) GetFilerAddr() string { // ConnectSFTP creates an SFTP client connection with the given credentials func (f *SftpTestFramework) ConnectSFTP(username, password string) (*sftp.Client, *ssh.Client, error) { + // Load the known host public key for verification + hostKeyCallback, err := f.getHostKeyCallback() + if err != nil { + return nil, nil, fmt.Errorf("failed to get host key callback: %v", err) + } + config := &ssh.ClientConfig{ User: username, Auth: []ssh.AuthMethod{ ssh.Password(password), }, - HostKeyCallback: ssh.InsecureIgnoreHostKey(), + HostKeyCallback: hostKeyCallback, Timeout: 5 * time.Second, } @@ -212,6 +218,26 @@ func (f *SftpTestFramework) ConnectSFTP(username, password string) (*sftp.Client return sftpClient, sshConn, nil } +// getHostKeyCallback returns a callback that verifies the server's host key +// matches the known test server key we generated +func (f *SftpTestFramework) getHostKeyCallback() (ssh.HostKeyCallback, error) { + // Read the public key file generated alongside the private key + pubKeyFile := f.hostKeyFile + ".pub" + pubKeyBytes, err := os.ReadFile(pubKeyFile) + if err != nil { + return nil, fmt.Errorf("failed to read host public key: %v", err) + } + + // Parse the public key + pubKey, _, _, _, err := ssh.ParseAuthorizedKey(pubKeyBytes) + if err != nil { + return nil, fmt.Errorf("failed to parse host public key: %v", err) + } + + // Return a callback that verifies the server key matches our known key + return ssh.FixedHostKey(pubKey), nil +} + // startMaster starts the SeaweedFS master server func (f *SftpTestFramework) startMaster(config *TestConfig) error { args := []string{