* sftp: check delete permission on an existing rename target
renameEntry authorized only the source path and the target directory,
so renaming over a file with a more restrictive ACL destroyed it even
though Remove on the same file was denied.
* sftp: deny a rename that would strand a carved-out ACL
A rename moves data without moving its path-based ACL: renaming an
ancestor directory left a more specific read-only entry behind, so the
relocated file fell under a permissive ancestor ACL and became
writable. Deny the rename when a configured ACL under the source would
grant less write/delete access than the relocated path would get.
* sftp: cover inherited and destination-side ACLs on rename
The rename ACL check compared only configured paths under the source,
so it missed restrictions inherited from ancestors above the source and
deeper grants configured under the destination. It now compares the
moved root plus every configured path under either tree mapped to its
source counterpart, and skips paths whose entries do not actually move.
The target delete check no longer depends on an existence lookup, so a
protected target created during a rename cannot be overwritten; a no-op
rename skips it. Root bypasses the comparison like every other check.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* sftp: fail closed on rename ACL lookups and restore new-name renames
- checkRenameKeepsAcl returns lookup errors instead of treating them as a missing entry, so a transient filer failure cannot skip the ACL comparison; it also compares read and list gains, not only write and delete, and evaluates each pair with the moved entry's own IsDirectory.
- The target delete check now distinguishes an existing target (still requires delete) from a fresh name (no delete needed) while still enforcing a configured ACL on the target name itself, so a concurrent create cannot slip a protected target under the rename.
* filer: add fail_if_target_exists option to AtomicRenameEntry
Callers that authorize a rename while the target is absent cannot see a
target created in the gap before the move runs. The flag makes the filer
reject the rename when the target exists, checked under the rename path
locks that CreateEntry on the same filer also takes.
* sftp: guard rename against concurrent targets and file-child ACL gains
A rename onto a name that inherits its directory's ACL still needs no
delete access, but now asks the filer to fail if a target appears before
the move, so a concurrently created file cannot be overwritten without
delete permission. Moved directories are also compared in the file ACL
shape, where list no longer counts as read, so their file children cannot
gain read access through the relocation.
---------
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* sftp: keep a file's existing bytes when it is opened for write without truncate
* sftp: preserve sparse gaps and refuse unreadable entries when copying existing content
The copy used a single chunk-stream read, which ends at the first sparse
gap: the tail past a hole was dropped, and a remote-only entry with no
local chunks copied zero bytes, so closing the handle uploaded an empty
file. Read through PrepareStreamContent, which fills unstored regions
with zeros, and refuse entries that carry no local bytes at all.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* filer: report a failed destination write from a completed chunk fetch
retriedStreamFetchChunkData broke out of the retry loop on a clean HTTP
end before checking writeErr, so a destination write that failed mid-
stream returned a short write and nil error. Check writeErr first.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* sftp: cover remote and all-holes sizes when copying existing content
Take the declared size from RemoteEntry when it exceeds FileSize, pad
inline content to the declared size, and treat a chunkless local file as
all holes rather than unreadable. A remote-only entry still refuses the
copy, now checked with a zero FileSize as well.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* sftp: size the existing-content copy the way filer.FileSize does
A locally shortened file still names its older, larger remote object;
taking RemoteSize unconditionally padded the copy back out to it.
filer.FileSize only honors the remote size when the remote is newer,
and also covers the chunk-extent size. A remote-only entry still
refuses the copy whenever it points at remote bytes.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* sftp: fail a directory listing on a stream error, and page past hidden entries
* sftp: test a fully hidden listing page
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
newFileWriter authorized uploads against the directory, so a file with
a more specific read-only ACL or mode could still be overwritten. The
same parent-directory fallback used for nonexistent paths now applies
through CheckFilePermission on the target itself.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
sftp: url-encode the upload path so filenames can't inject filer query commands
The SFTP put handler concatenated the user-controlled filename straight into
the filer upload URL, so a name containing "?" was parsed as a query string.
Build the URL via url.URL{Path: ...} so "?" becomes %3F and stays a literal
path character.
Introduce security.BearerPrefix ("Bearer ", RFC 6750) and use it
everywhere an "Authorization: Bearer <token>" header is constructed,
replacing the scattered "BEARER "/"Bearer " string literals. SeaweedFS
matches the scheme case-insensitively when parsing (security.GetJwt), so
behavior is unchanged; this removes the magic string and settles the
casing on the standard form. The parser's upper-case comparison stays as
is on purpose.
A Canceled/DeadlineExceeded from the caller's per-request context was
treated like a dead channel: it closed the shared cached ClientConn and
cancelled every other in-flight RPC on it with "the client connection is
closing". Under a burst of concurrent chunk assigns (e.g. a large S3
multipart upload) one slow assign hitting its 10s attempt timeout could
poison the connection for all the rest, cascading into a flood of 500s.
Thread the caller's context into shouldInvalidateConnection and only
invalidate on Canceled/DeadlineExceeded while that context is still live,
which isolates the genuine stale-channel signal (a peer restart behind a
k8s Service VIP). To carry the context, add a ctx parameter to the
existing WithGrpcClient, WithMasterClient, and WithMasterServerClient; the
master assign and volume-lookup paths pass their per-attempt context and
every other caller passes context.Background().
* sftpd: use global TLS-aware HTTP client for filer uploads (#8794)
putFile() hardcoded http:// and used http.DefaultClient, which broke
file uploads when the filer has HTTPS/TLS enabled. Switch to the global
HTTP client which reads [https.client] from security.toml and
automatically normalizes the URL scheme.
* sftpd: propagate NormalizeUrl error instead of swallowing it
* Fix SFTP file upload failures with JWT filer tokens (issue #8425)
When JWT authentication is enabled for filer operations via jwt.filer_signing.*
configuration, SFTP server file upload requests were rejected because they lacked
JWT authorization headers.
Changes:
- Added JWT signing key and expiration fields to SftpServer struct
- Modified putFile() to generate and include JWT tokens in upload requests
- Enhanced SFTPServiceOptions with JWT configuration fields
- Updated SFTP command startup to load and pass JWT config to service
This allows SFTP uploads to authenticate with JWT-enabled filers, consistent
with how other SeaweedFS components (S3 API, file browser) handle filer auth.
Fixes#8425
* Apply suggestion from @gemini-code-assist[bot]
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* 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
