* check for nil needle map before compaction sync
When CommitCompact runs concurrently, it sets v.nm = nil under
dataFileAccessLock. CompactByIndex does not hold that lock, so
v.nm.Sync() can hit a nil pointer. Add an early nil check to
return an error instead of crashing.
Fixes#8591
* guard copyDataBasedOnIndexFile size check against nil needle map
The post-compaction size validation at line 538 accesses
v.nm.ContentSize() and v.nm.DeletedSize(). If CommitCompact has
concurrently set v.nm to nil, this causes a SIGSEGV. Skip the
validation when v.nm is nil since the actual data copy uses local
needle maps (oldNm/newNm) and is unaffected.
Fixes#8591
* use atomic.Bool for compaction flags to prevent concurrent vacuum races
The isCompacting and isCommitCompacting flags were plain bools
read and written from multiple goroutines without synchronization.
This allowed concurrent vacuums on the same volume to pass the
guard checks and run simultaneously, leading to the nil pointer
crash. Using atomic.Bool with CompareAndSwap ensures only one
compaction or commit can run per volume at a time.
Fixes#8591
* use go-version-file in CI workflows instead of hardcoded versions
Use go-version-file: 'go.mod' so CI automatically picks up the Go
version from go.mod, avoiding future version drift. Reordered
checkout before setup-go in go.yml and e2e.yml so go.mod is
available. Removed the now-unused GO_VERSION env vars.
* capture v.nm locally in CompactByIndex to close TOCTOU race
A bare nil check on v.nm followed by v.nm.Sync() has a race window
where CommitCompact can set v.nm = nil between the two. Snapshot
the pointer into a local variable so the nil check and Sync operate
on the same reference.
* add dynamic timeouts to plugin worker vacuum gRPC calls
All vacuum gRPC calls used context.Background() with no deadline,
so the plugin scheduler's execution timeout could kill a job while
a large volume compact was still in progress. Use volume-size-scaled
timeouts matching the topology vacuum approach: 3 min/GB for compact,
1 min/GB for check, commit, and cleanup.
Fixes#8591
* Revert "add dynamic timeouts to plugin worker vacuum gRPC calls"
This reverts commit 80951934c37416bc4f6c1472a5d3f8d204a637d9.
* unify compaction lifecycle into single atomic flag
Replace separate isCompacting and isCommitCompacting flags with a
single isCompactionInProgress atomic.Bool. This ensures CompactBy*,
CommitCompact, Close, and Destroy are mutually exclusive — only one
can run at a time per volume.
Key changes:
- All entry points use CompareAndSwap(false, true) to claim exclusive
access. CompactByVolumeData and CompactByIndex now also guard v.nm
and v.DataBackend with local captures.
- Close() waits for the flag outside dataFileAccessLock to avoid
deadlocking with CommitCompact (which holds the flag while waiting
for the lock). It claims the flag before acquiring the lock so no
new compaction can start.
- Destroy() uses CAS instead of a racy Load check, preventing
concurrent compaction from racing with volume teardown.
- unmountVolumeByCollection no longer deletes from the map;
DeleteCollectionFromDiskLocation removes entries only after
successful Destroy, preventing orphaned volumes on failure.
Fixes#8591
* 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
* 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
* 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
* security: update golang.org/x/crypto to v0.45.0
Addresses security vulnerability in golang.org/x/crypto < 0.45.0
* 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.
* security: fix path traversal vulnerability in SFTP toAbsolutePath
The previous implementation had a critical security vulnerability:
- Path traversal via '../..' could escape the HomeDir chroot jail
- Absolute paths were not correctly prefixed with HomeDir
The fix:
1. Concatenate HomeDir with userPath directly, then clean
2. Add security check to ensure final path stays within HomeDir
3. If traversal detected, safely return HomeDir instead
Also adds path traversal prevention tests to verify the fix.
* fix: address PR review comments
1. Fix SkipCleanup check to use actual test config instead of default
- Added skipCleanup field to SftpTestFramework struct
- Store config.SkipCleanup during Setup()
- Use f.skipCleanup in Cleanup() instead of DefaultTestConfig()
2. Fix path prefix check false positive in mkdir
- Changed from strings.HasPrefix(absPath, fs.user.HomeDir)
- To: absPath == fs.user.HomeDir || strings.HasPrefix(absPath, fs.user.HomeDir+"/")
- Prevents matching partial directory names (e.g., /sftp/username when HomeDir is /sftp/user)
* fix: check write permission on parent dir for mkdir
Aligns makeDir's permission check with newFileWriter for consistency.
To create a directory, a user needs write permission on the parent
directory, not mkdir permission on the new directory path.
* fix: refine SFTP path traversal logic and tests
1. Refine toAbsolutePath:
- Use path.Join with strings.TrimPrefix for idiomatic path construction
- Return explicit error on path traversal attempt instead of clamping
- Updated all call sites to handle the error
2. Add Unit Tests:
- Added sftp_server_test.go to verify toAbsolutePath logic
- Covers normal paths, root path, and various traversal attempts
3. Update Integration Tests:
- Updated PathTraversalPrevention test to reflect that standard SFTP clients
sanitize paths before sending. The test now verifies successful containment
within the jail rather than blocking (since the server receives a clean path).
- The server-side blocking is verified by the new unit tests.
4. Makefile:
- Removed -v from default test target
* fix: address PR comments on tests and makefile
1. Enhanced Unit Tests:
- Added edge cases (empty path, multiple slashes, trailing slash) to sftp_server_test.go
2. Makefile Improvements:
- Added 'all' target as default entry point
3. Code Clarity:
- Added comment to mkdir permission check explaining defensive nature of HomeDir check
* fix: address PR review comments on permissions and tests
1. Security:
- Added write permission check on target directory in renameEntry
2. Logging:
- Changed dispatch log verbosity from V(0) to V(1)
3. Testing:
- Updated Makefile .PHONY targets
- Added unit test cases for empty/root HomeDir behavior in toAbsolutePath
* fix: set SFTP starting directory to virtual root
1. Critical Fix:
- Changed sftp.WithStartDirectory from fs.user.HomeDir to '/'
- Prevents double-prefixing when toAbsolutePath translates paths
- Users now correctly start at their virtual root which maps to HomeDir
2. Test Improvements:
- Use pointer for homeDir in tests for clearer nil vs empty distinction
* fix: clean HomeDir at config load time
Clean HomeDir path when loading users from JSON config.
This handles trailing slashes and other path anomalies at the source,
ensuring consistency throughout the codebase and avoiding repeated
cleaning on every toAbsolutePath call.
* test: strengthen assertions and add error checking in SFTP tests
1. Add error checking for cleanup operations in TestWalk
2. Strengthen cwd assertion to expect '/' explicitly in TestCurrentWorkingDirectory
3. Add error checking for cleanup in PathTraversalPrevention test
