feat(services/sftp): port from openssh to russh by chitralverma · Pull Request #7977 · apache/opendal

chitralverma · 2026-07-28T06:33:25Z

Which issue does this PR close?

Partially addresses #7391.

Rationale for this change

The SFTP service used the openssh crate, which drives the system ssh
binary over a Unix control socket. That made the service Unix-only and
required an ssh client on PATH. russh is a pure-Rust SSH client, so
neither constraint remains.

control_directory (#3963) only existed to place that control socket.
russh has no control socket, so the option is obsolete rather than
implemented.

sftp errors (#7391) now carry the SFTP status code, so NotFound,
PermissionDenied, and Unsupported are distinguishable instead of
collapsing into a generic failure. The 10s pool-acquisition timeout is
unchanged, so this only partially addresses that issue.

What changes are included in this PR?

Replace openssh + openssh-sftp-client with russh + russh-sftp.
Reimplement host key verification on russh's callback. strict, add,
and accept keep their OpenSSH StrictHostKeyChecking semantics.
Use posix-rename@openssh.com for rename, since SFTP v3 rename
fails when the destination exists.
Remove the unix-only services-sftp gates in the Java, Python, and
dotnet bindings.

Are there any user-facing changes?

SFTP now builds and runs on Windows, and no longer needs an ssh binary.

No API changes: the sftp:// scheme, SftpConfig, and Operator
behavior are unchanged. Password login (#2966) is still unsupported.

Two behavior differences worth noting:

Authentication falls back to the SSH agent and the default ~/.ssh
keys when key is unset; previously the system ssh client resolved
those implicitly.
~/.ssh/config and /etc/ssh/ssh_known_hosts are no longer consulted.
Only ~/.ssh/known_hosts is read. Setups relying on Host aliases,
ProxyJump, or a system-wide known_hosts file must configure the
builder explicitly.

Validation

Upstream behavior tests need 1Password-backed credentials, so this was
validated with a throwaway workflow in my fork that stands up its own
OpenSSH server on each runner
(workflow,
green run).
It is not part of this PR.

Job	Linux	macOS	Windows
build + clippy + unit + doc tests	pass	pass	pass
behavior suite (118 cases)	118/118	118/118	118/118

Also covered: the dockerised atmoz/sftp setup upstream CI uses, and the
strict / add / accept host key strategies, which upstream never
exercises beyond accept. Two consecutive green runs.

Three defects surfaced only on Windows and are fixed here:

MAX_AGENT_IDENTITIES was dead code off Unix and failed -D warnings.
OpenSSH on Windows omits . and .. from readdir, so the listed
directory itself was dropped and empty subdirectories disappeared from
recursive listings. The entry now comes from fstat on the open handle.
A trailing separator made Windows reject the path outright, turning
stat on a missing directory into Unexpected instead of NotFound.

AI Usage Statement

Written with OpenCode (Claude Opus 4.6). All changes were reviewed and
verified locally against the SFTP behavior tests.

The openssh crate drives the system ssh binary over a Unix control socket, so the service could not build on Windows and required an ssh client on PATH. russh is a pure-Rust SSH implementation. - Host key checks move to russh's callback; strict, add, and accept keep their OpenSSH StrictHostKeyChecking semantics. - rename uses posix-rename@openssh.com, since SFTP v3 rename fails when the destination already exists. - Reads, writes, and copies pipeline packets sized to the limits@openssh.com values; listing streams readdir batches. - Streams hold a detached session reference rather than a pool slot, and handshakes are throttled to stay under sshd MaxStartups. Public API is unchanged. Refs apache#7968

The unix-only gates existed because openssh could not build on Windows. Java and Python drop the gate; dotnet keeps it for services-monoiofs, which is unrelated. Refs apache#7968

Follow-up to review on apache#7977. - Release open/opendir handles on drop. The old client closed them in OwnedHandle::drop; without it a ranged read or a truncated listing leaked a server-side handle per call. - Resync copy on a short read instead of treating it as EOF, matching the reader. A mid-file short read silently truncated the target. - Try rename before removing the destination. The pre-delete destroyed the target when the source was missing or unreadable. - Clamp read and write payloads against packet_len, which the server applies to the serialized packet, not the payload. - Reject unbracketed IPv6 endpoints; "::1" parsed as host ":" port 1. - Warn when a host key cannot be recorded, since add then behaves as accept for every later connection. - Cap agent identity attempts to stay under MaxAuthTries.

MAX_AGENT_IDENTITIES is only referenced from the cfg(unix) agent path, so the constant was dead code on Windows and failed the -D warnings build.

OpenSSH on Windows omits "." and ".." from readdir, so keying the directory's own entry off a "." record dropped it entirely there, and empty subdirectories vanished from recursive listings. Take the metadata from fstat on the open handle instead and ignore both dot entries.

A trailing separator carries no meaning in SFTP. OpenSSH on Windows rejects such a path outright rather than reporting that it does not exist, so stat on a missing directory surfaced as Unexpected instead of NotFound.

Xuanwo · 2026-07-28T09:41:29Z

+/// in flight.
+///
+/// SFTP has no server-side copy, so the payload has to make the round trip.
+async fn stream_copy(


Don't need to simulate copy at backend side.

Xuanwo · 2026-07-28T09:43:55Z

+///
+/// Accepts both `[user@]host[:port]` and `ssh://[user@]host[:port]`, and
+/// defaults the port to 22.
+fn parse_endpoint(endpoint: &str) -> Result<(Option<String>, String, u16)> {


Can we use url crate for this?

Xuanwo · 2026-07-28T09:44:20Z

+    conn: SftpSessionRef,
+    handle: String,
    prefix: String,
+    buffer: VecDeque<(String, FileAttributes)>,


What's this for?

Xuanwo · 2026-07-28T09:45:47Z

+            return Ok(Buffer::new());
+        }
+
+        let results = futures::future::join_all(inflight).await;


Why we need to do this? Should this be covered by external concurrent logic?

erickguan · 2026-07-28T14:13:08Z

+}
+
+/// Validates the remote host key according to [`KnownHostsStrategy`].
+struct SshHandler {


Maybe SshEndpoint?

erickguan · 2026-07-28T14:15:27Z

+        let config = russh_sftp::client::Config {
+            // Every request is a single round trip, so allow slow servers more
+            // room than the crate default of 10 seconds.
+            request_timeout_secs: 60,


magical number :) Move to a constant.

erickguan · 2026-07-28T14:27:12Z

+            strategy: self.known_hosts_strategy,
+        };
+
+        let mut handle = client::connect(


We could parse lots of config ahead of time in core.

erickguan · 2026-07-28T14:30:06Z

        }
    }

+    /// Resolves an OpenDAL path against the configured root.


should use path.rs in raw lib.

erickguan · 2026-07-28T14:34:10Z

+    }
+}
+
+/// A detachable reference to a live SFTP session.


I understand your intention is to keep reference to hold Handle so that you can receive session. But it's a bit hard to see how russh process or manage client/channel/handle lifetime and event from the documentation.

The documentation also explains more but we are building connections in readers, writers and listers.

chitralverma · 2026-07-28T15:58:38Z

@Xuanwo @erickguan this is draft right now, I've got something to fix

chitralverma added 6 commits July 28, 2026 12:03

build(bindings): enable sftp on windows

34331a3

The unix-only gates existed because openssh could not build on Windows. Java and Python drop the gate; dotnet keeps it for services-monoiofs, which is unrelated. Refs apache#7968

fix(services/sftp): gate agent identity cap to unix

5f4c5cc

MAX_AGENT_IDENTITIES is only referenced from the cfg(unix) agent path, so the constant was dead code on Windows and failed the -D warnings build.

fix(services/sftp): drop trailing separators from remote paths

7901942

A trailing separator carries no meaning in SFTP. OpenSSH on Windows rejects such a path outright rather than reporting that it does not exist, so stat on a missing directory surfaced as Unexpected instead of NotFound.

Xuanwo reviewed Jul 28, 2026

View reviewed changes

erickguan reviewed Jul 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(services/sftp): port from openssh to russh - #7977

feat(services/sftp): port from openssh to russh#7977
chitralverma wants to merge 6 commits into
apache:mainfrom
chitralverma:feat/sftp-russh

chitralverma commented Jul 28, 2026 •

edited

Loading

Uh oh!

Xuanwo Jul 28, 2026

Uh oh!

Xuanwo Jul 28, 2026

Uh oh!

Xuanwo Jul 28, 2026

Uh oh!

Xuanwo Jul 28, 2026

Uh oh!

erickguan Jul 28, 2026

Uh oh!

erickguan Jul 28, 2026

Uh oh!

erickguan Jul 28, 2026

Uh oh!

erickguan Jul 28, 2026

Uh oh!

erickguan Jul 28, 2026

Uh oh!

chitralverma commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

chitralverma commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Validation

AI Usage Statement

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chitralverma commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chitralverma commented Jul 28, 2026 •

edited

Loading