Forward the local registry port over the OS ssh client by assirims · Pull Request #1887 · basecamp/kamal

assirims · 2026-06-21T23:19:55Z

Problem

With the local registry (registry: { server: localhost:5555 }), kamal deploy hangs in Build#pull once an image layer exceeds ~27 MiB. Kamal::Cli::Build::PortForwarding carries the reverse tunnel with net-ssh (ssh.forward.remote + a single ssh.loop(0.1) in a background thread). During docker pull the bulk data flows host → deploy-machine; that forwarded channel's pump starves while the main thread drives the pull over its own net-ssh/SSHKit connection, the channel window is exhausted, and the transfer wedges. I could reproduce it deterministically — the tunnel froze at the same byte offset (~27.3 MiB) every time, with TCP retransmits. A plain OS ssh -R reverse tunnel carries the identical pull with no problem, which points at the net-ssh transport rather than the network. (Likely the same root cause as the Errno::ERANGE: send(2) reported in #1690.)

Fix

Establish the reverse forward with the OS ssh client instead of net-ssh's in-process forwarding. The local-registry design is unchanged — same -R 127.0.0.1:port:localhost:port reverse tunnel, registry stays on localhost — only the transport differs. Readiness is detected via a token printed by the remote command, which (with ExitOnForwardFailure=yes) only runs once the forward is established, preserving the existing "block until the tunnel is up, 30s timeout" contract. The existing ssh_options (user, port, keys, keys_only, config, forward_agent, keepalive, proxy) are mapped to the equivalent ssh flags.

Testing

Added a unit test for the command construction (test/cli/build/port_forwarding_test.rb), including a guard that -N is never passed (it would suppress the readiness command and hang the deploy).
The existing build_test.rb port-forwarding / pull tests (which mock PortForwarding) still pass.
Verified end-to-end against a real host: a 40 MB layer that reliably deadlocked under net-ssh now pulls cleanly through the OS-ssh tunnel.
rubocop clean.

Known limitation

ssh_options[:key_data] (inline key material) can't be passed to the OS ssh client as a flag, so that path now relies on an agent / on-disk key. key_data is already deprecated for removal in Kamal 3. Happy to handle it (e.g. via a temporary identity file) — or to rework this toward a net-ssh-level fix instead, if you'd prefer to keep the transport in-process.

net-ssh's in-process reverse forwarding deadlocks when a large amount of data flows back through the tunnel: pulling an image whose changed layer exceeds ~27 MiB from the local registry hangs `kamal deploy` in `Build#pull`. The single `ssh.loop(0.1)` pump starves while the main thread drives the pull, the forwarded channel's window is exhausted, and the transfer wedges (stalls deterministically at the same byte offset, with TCP retransmits). A plain OS `ssh -R` carries the identical pull fine. Carry the reverse tunnel over the OS `ssh` client instead of net-ssh's in-process forwarding. The local-registry design is unchanged; only the transport differs. The existing ssh_options are mapped to the equivalent ssh flags. Fixes basecamp#1886. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

This PR addresses local-registry deploy hangs by switching reverse port forwarding from net-ssh in-process forwarding to an OS ssh -R tunnel, using a readiness handshake to preserve the “block until tunnel is up” behavior.

Changes:

Replace net-ssh reverse forwarding with an OS ssh subprocess and a READY token handshake.
Add lifecycle management for the spawned ssh processes (startup, readiness wait, teardown).
Add unit tests validating ssh command construction and option mapping.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
lib/kamal/cli/build/port_forwarding.rb	Implements OS-`ssh` reverse tunnel startup/readiness/teardown and maps Kamal SSH options to `ssh` flags.
test/cli/build/port_forwarding_test.rb	Adds unit coverage for the constructed `ssh` command and option mappings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

assirims · 2026-07-18T10:36:48Z

+    def forward_ports
+      hosts.each do |host|
+        @children << start_forward(host)
+        wait_until_ready(@children.last)
+      end


Good catch — fixed in 9ae8f7a. All tunnels are now spawned before any readiness wait, so they establish in parallel (as the net-ssh version did) and READY_TIMEOUT is enforced as a single deadline shared across hosts, restoring the 30s-total contract.

assirims · 2026-07-18T10:36:49Z

+    def config_option
+      ssh_options[:config] ? [ "-F", ssh_options[:config].to_s ] : []
+    end


Fixed in 9ae8f7a. nil/true now emit no flag so the OS client reads its default configs, false maps to -F /dev/null, and a path or one-element array maps to -F. For multiple config files I went with a clear error rather than emitting multiple -F flags: OpenSSH honors only the last -F, so passing them all through would silently drop files. Faithful multi-file support would need a generated config using Include — happy to add that if it's wanted.

assirims · 2026-07-18T10:36:57Z

+    def key_options
+      options = Array(ssh_options[:keys]).flat_map { |key| [ "-i", key.to_s ] }
+      options += [ "-o", "IdentitiesOnly=yes" ] if ssh_options[:keys_only]
+      options += [ "-o", "ForwardAgent=yes" ] if ssh_options[:forward_agent]
+      options
+    end


Agreed — fixed in 9ae8f7a: key_data now raises a clear error up front instead of being silently ignored. I went with fail-fast rather than a temporary identity file since that would write private key material to disk, and key_data is already deprecated for removal in Kamal 3. Can switch to the tempfile approach if you'd rather keep that path working.

assirims · 2026-07-18T10:36:58Z

+  test "maps a proxy command to ProxyCommand" do
+    command = ssh_command(proxy: Net::SSH::Proxy::Command.new("connect -S relay %h %p"))
+
+    assert_includes_sequence command, [ "-o", "ProxyCommand=connect -S relay %h %p" ]
+  end
+
+  private


Added in 9ae8f7a: coverage for config as unset/true/false/path/one-element array/multiple files, the key_data error, and the shared readiness deadline.

- Map every documented ssh config type to the right flag: nil/true lets ssh read its default config files, false ignores them via -F /dev/null, and a single path (or one-element array) maps to -F. Multiple config files raise instead of silently dropping all but the last -F. - Fail fast with a clear error when key_data is configured, instead of silently ignoring the inline keys and failing auth later. - Spawn all tunnels before waiting so they establish in parallel and READY_TIMEOUT stays a single 30s deadline shared across hosts, as with the previous net-ssh implementation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

assirims · 2026-07-18T14:29:26Z

+        "-o", "ExitOnForwardFailure=yes",
+        "-o", "BatchMode=yes",
+        "-o", "ServerAliveInterval=#{ssh_options[:keepalive_interval] || 15}",
+        "-o", "ServerAliveCountMax=4",


Fair on the host key half — fe79052 adds -o StrictHostKeyChecking=accept-new, which restores net-ssh's default policy exactly (accept_new_or_local_tunnel: unknown keys are accepted silently, changed keys still fail), so a first-contact host works without prompting. I've kept BatchMode=yes though: a background tunnel subprocess can't reliably service password/passphrase prompts — in CI there's no TTY and the deploy would just hang until the readiness timeout, and in a terminal the prompt would race the 30s deadline. Failing fast with ssh's error message seems strictly better there.

assirims · 2026-07-18T14:29:27Z

+    rescue Timeout::Error
+      raise "Timed out waiting for port forwarding to be established"


Fixed in fe79052 — the timeout error now names the first host that failed to become ready.

StrictHostKeyChecking=accept-new restores net-ssh's default host key policy (accept unknown keys silently, still reject changed ones), so a first-contact host doesn't fail under BatchMode. BatchMode stays: a background tunnel can't service password or passphrase prompts, so failing fast beats hanging until the readiness deadline. Also include the host in the readiness timeout error to make multi-host failures diagnosable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

+        "-o", "ServerAliveInterval=#{ssh_options[:keepalive_interval] || 15}",
+        "-o", "ServerAliveCountMax=4",


+    def terminate(wait_thread)
+      Process.kill "TERM", wait_thread.pid
+      return if wait_thread.join(TEARDOWN_GRACE)


+    assert_includes_sequence command, [ "-o", "ServerAliveInterval=45" ]
+  end
+


Omit the ServerAlive options when ssh_options[:keepalive] is explicitly false, matching net-ssh, and skip TERM/KILL once the ssh process has been reaped so a recycled PID can't be signaled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

+      Timeout.timeout(remaining) do
+        while (line = child[:output].gets)
+          return if line.strip == READY_TOKEN
        end
+
+        raise "Failed to establish port forward on #{child[:host]} (ssh exited #{child[:wait_thread].value.exitstatus})"
+      end


Copilot AI review requested due to automatic review settings June 21, 2026 23:19

Copilot started reviewing on behalf of assirims June 21, 2026 23:20 View session

Copilot AI reviewed Jun 21, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 18, 2026 10:36

Copilot started reviewing on behalf of assirims July 18, 2026 10:37 View session

Copilot AI reviewed Jul 18, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 18, 2026 14:29

Copilot started reviewing on behalf of assirims July 18, 2026 14:29 View session

Copilot AI reviewed Jul 18, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 18, 2026 17:01

Copilot started reviewing on behalf of assirims July 18, 2026 17:01 View session

Copilot AI reviewed Jul 18, 2026

View reviewed changes

Navigation Menu

Uh oh!

Forward the local registry port over the OS ssh client - #1887

Forward the local registry port over the OS ssh client#1887
assirims wants to merge 4 commits into
basecamp:mainfrom
assirims:fix-local-registry-port-forward-deadlock

assirims commented Jun 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

assirims Jul 18, 2026

Uh oh!

assirims Jul 18, 2026

Uh oh!

assirims Jul 18, 2026

Uh oh!

assirims Jul 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

assirims Jul 18, 2026

Uh oh!

assirims Jul 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		rescue Timeout::Error
		raise "Timed out waiting for port forwarding to be established"

		"-o", "ServerAliveInterval=#{ssh_options[:keepalive_interval] \|\| 15}",
		"-o", "ServerAliveCountMax=4",

		assert_includes_sequence command, [ "-o", "ServerAliveInterval=45" ]
		end

Uh oh!

Conversation

assirims commented Jun 21, 2026

Problem

Fix

Testing

Known limitation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

assirims Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

assirims Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

assirims Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

assirims Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

assirims Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

assirims Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants