From bca87b6df81ee113ca07bd6e8e02936cf35d7be3 Mon Sep 17 00:00:00 2001 From: Paulo Alvarado Date: Tue, 27 Oct 2026 19:11:34 +0100 Subject: [PATCH] Answer SFTP limits from the sandbox server asyncssh's SFTP server answers limits@openssh.com from its own constants, a 4 MiB write, and never asks the SFTPServer. A client that honours the answer sends a file under 4 MiB as one write packet. The sandbox's OpenSSH sftp-server ends the session on a packet of 256 KiB or more, so every upload of that size failed with a lost connection. The backend now asks the sandbox server its limits after the handshake, and the gateway answers a caller's request with them. A sandbox server without the extension leaves the client's 16 KiB default in place. The test sftp-server resolves at the Debian path too, so the SFTP tests run on Linux instead of skipping. --- docs/deploy.md | 4 +++- src/gateway/backend.py | 7 ++++--- src/gateway/server.py | 3 ++- src/gateway/sftp.py | 21 +++++++++++++++++++++ src/gateway/tests/localprocess.py | 19 ++++++++++++++----- src/gateway/tests/test_sftp.py | 22 ++++++++++++++++++++++ 6 files changed, 66 insertions(+), 10 deletions(-) diff --git a/docs/deploy.md b/docs/deploy.md index 9d2d776..6474e8c 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -297,7 +297,9 @@ legacy protocol) and remote port forwarding. SFTP runs the sandbox's own SFTP server over one persistent session for each SSH connection, thus repeated file operations on one connection start no new session. The session closes after a short idle period, so an inactive -sandbox still sleeps. +sandbox still sleeps. The gateway answers a client's `limits@openssh.com` +request with the sandbox server's own limits, so the client sizes its +packets to what that server accepts. Forward a local port to a service that listens on the sandbox loopback: diff --git a/src/gateway/backend.py b/src/gateway/backend.py index e432c4f..6f43899 100644 --- a/src/gateway/backend.py +++ b/src/gateway/backend.py @@ -113,9 +113,9 @@ async def _acquire(self) -> SFTPClientHandler: if self._idle_closer: self._idle_closer.cancel() self._idle_closer = None - if self._handler is None: + if not self._handler: await self._open() - assert self._handler is not None + assert self._handler self._in_flight += 1 return self._handler @@ -152,11 +152,12 @@ async def _open(self) -> None: ) try: await handler.start() + self._recv_task = asyncio.create_task(handler.recv_packets()) + await handler.request_limits() except Exception: await self._teardown() raise self._handler = handler - self._recv_task = asyncio.create_task(handler.recv_packets()) logger.info("gateway: sftp backend open host=%s", self._host_name) async def _close_after_idle(self) -> None: diff --git a/src/gateway/server.py b/src/gateway/server.py index 6c77af3..fdb929d 100644 --- a/src/gateway/server.py +++ b/src/gateway/server.py @@ -12,7 +12,7 @@ from core.database import async_session_factory from gateway.backend import SandboxSftpBackend from gateway.settings import GatewaySettings -from gateway.sftp import GatewaySFTPServer +from gateway.sftp import GatewaySFTPServer, relay_limits from hosts.models import Host, HostStatus from providers.base import SandboxProcess, TerminalSize from providers.exceptions import ProviderError @@ -203,6 +203,7 @@ def _open_sftp(channel: asyncssh.SSHServerChannel) -> GatewaySFTPServer: for extension in asyncssh.sftp.SFTPServerHandler._extensions if extension[0] not in _UNSUPPORTED_SFTP_EXTENSIONS ] + asyncssh.sftp.SFTPServerHandler._packet_handlers[b"limits@openssh.com"] = relay_limits server = cast(GatewayConnection, channel.get_connection().get_owner()) return GatewaySFTPServer(channel, server.get_sftp_backend()) diff --git a/src/gateway/sftp.py b/src/gateway/sftp.py index c33bb6e..5f30a3e 100644 --- a/src/gateway/sftp.py +++ b/src/gateway/sftp.py @@ -3,7 +3,10 @@ # each server handle straight to the client, so the handle-argument and # override checks do not apply here. # pyright: reportArgumentType=false, reportIncompatibleMethodOverride=false +from typing import cast + import asyncssh +from asyncssh.packet import SSHPacket from gateway.backend import SandboxSftpBackend @@ -72,6 +75,10 @@ async def realpath(self, path): names, _ = await handler.realpath(path) return names[0].filename + async def limits(self) -> asyncssh.SFTPLimits: + async with self._backend.session() as handler: + return handler.limits + def _unsupported(self, *args, **kwargs): raise asyncssh.SFTPOpUnsupported("not served by this gateway") @@ -85,3 +92,17 @@ def _unsupported(self, *args, **kwargs): link = _unsupported lock = _unsupported unlock = _unsupported + + +async def relay_limits( + handler: asyncssh.sftp.SFTPServerHandler, packet: SSHPacket +) -> asyncssh.SFTPLimits: + """Answer a caller's limits request with the sandbox server's limits. + + asyncssh's server answers it from its own constants, a 4 MiB write, and + never asks the ``SFTPServer``. The sandbox's OpenSSH server ends the + session on a packet of 256 KiB or more, so the caller must size its + packets by that server's answer. + """ + packet.check_end() + return await cast(GatewaySFTPServer, handler._server).limits() diff --git a/src/gateway/tests/localprocess.py b/src/gateway/tests/localprocess.py index 004e3ee..b708420 100644 --- a/src/gateway/tests/localprocess.py +++ b/src/gateway/tests/localprocess.py @@ -3,7 +3,16 @@ from providers.base import SandboxProcess, TerminalSize -SFTP_SERVER = "/usr/libexec/sftp-server" +# OpenSSH installs its SFTP server at the Debian path on Linux and under +# libexec on macOS. +SFTP_SERVER = next( + ( + path + for path in ("/usr/lib/openssh/sftp-server", "/usr/libexec/sftp-server") + if os.path.exists(path) + ), + "/usr/lib/openssh/sftp-server", +) SFTP_SERVER_COMMAND = f"exec {SFTP_SERVER}" @@ -34,19 +43,19 @@ async def open(cls, name, *, command, terminal): return cls(process) async def receive(self, max_bytes: int) -> bytes: - assert self._process.stdout is not None + assert self._process.stdout return await self._process.stdout.read(max_bytes) async def receive_stderr(self, max_bytes: int) -> bytes: - assert self._process.stderr is not None + assert self._process.stderr return await self._process.stderr.read(max_bytes) def send(self, data: bytes) -> None: - assert self._process.stdin is not None + assert self._process.stdin self._process.stdin.write(data) def send_eof(self) -> None: - assert self._process.stdin is not None + assert self._process.stdin self._process.stdin.write_eof() def resize(self, size: TerminalSize) -> None: diff --git a/src/gateway/tests/test_sftp.py b/src/gateway/tests/test_sftp.py index 19f5bd1..5eb2acc 100644 --- a/src/gateway/tests/test_sftp.py +++ b/src/gateway/tests/test_sftp.py @@ -169,3 +169,25 @@ async def test_a_caller_writes_dotfiles_by_home_and_a_command_reads_them(connect result = await connected.run("sh -c 'cat $HOME/.gitconfig'") assert "name = Sandbox" in result.stdout + + +async def test_the_advertised_limits_are_the_sandbox_servers(connected): + # asyncssh's own server advertises 4 MiB writes. The sandbox's OpenSSH + # server ends the session on a packet of 256 KiB or more, so the caller + # must hear the sandbox server's numbers, not asyncssh's. + async with connected.start_sftp_client() as sftp: + assert 0 < sftp.limits.max_write_len < 256 * 1024 + + +async def test_a_file_larger_than_the_sandbox_servers_packet_round_trips(connected, tmp_path): + # A put at the client's default block size trusts the advertised write + # length. With the sandbox server's limits, a 300 KiB file goes in + # packets the sandbox server accepts. + source = tmp_path / "source.bin" + source.write_bytes(os.urandom(300 * 1024)) + remote = tmp_path / "sb-sftp" / "upload.bin" + + async with connected.start_sftp_client() as sftp: + await sftp.put(str(source), str(remote)) + + assert remote.read_bytes() == source.read_bytes()