-
Notifications
You must be signed in to change notification settings - Fork 35
Closed
Labels
Description
Describe the bug
The SFTP backend suffers from a typed nil pointer issue that causes nil pointer dereference panics when SFTP client creation fails and the client is subsequently reused. When
GetClient()
fails, it can leave fs.sftpclient as a typed nil (*sftp.Client(nil)
) instead of nil. This typed nil passes Go's == nil
checks but causes panics when methods are called on it, specifically in github.com/pkg/sftp.(*Client).nextID()
.
To Reproduce
Steps to reproduce the behavior:
- Create an SFTP filesystem with invalid credentials (wrong password, nonexistent host, etc.)
- Attempt to create a client connection using
FileSystem.Client()
- this will fail and setfs.sftpclient
to a typed nil - Make a second call to
FileSystem.Client()
- this returns the cached typed nil client without error - Perform any SFTP operation that calls methods on the client (e.g., file operations, directory listing)
- See panic:
runtime error: invalid memory address or nil pointer dereference
ingithub.com/pkg/sftp.(*Client).nextID()
Minimal Reproduction
fs := NewFileSystem(WithOptions(Options{
Username: "baduser",
Password: "wrongpassword",
KnownHostsCallback: ssh.InsecureIgnoreHostKey(),
}))
auth, _ := authority.NewAuthority("sftp://baduser@nonexistent-host:22")
// First call fails properly
_, err1 := fs.Client(auth)
// err1 contains connection error
// Second call returns typed nil with no error
client, err2 := fs.Client(auth)
// err2 is nil, but client is *sftp.Client(nil)
// Any operation on client will panic
client.ReadDir("/") // PANIC: nil pointer dereference
Expected behavior
- Failed client creation should not leave typed nil pointers in the filesystem state
- Subsequent calls to
Client()
should either return a valid client or a proper error - No panics should occur during normal error handling scenarios
- The
Client()
method should detect both nil and typed nil pointers correctly
Additional context
- Root Cause: Go's interface nil semantics -
*sftp.Client(nil) != nil
when stored in an interface - Affected Operations: Any SFTP operation after a failed connection attempt
- Fix: Check for typed nils using
reflect.ValueOf(fs.sftpclient).IsNil()
in addition to== nil