From a3d77cd01511c3ce040f6d2b16f8d5b653309129 Mon Sep 17 00:00:00 2001 From: didericis Date: Tue, 12 May 2026 16:19:07 -0400 Subject: [PATCH] fix(ssh-gate): listen on the upstream port so URL-supplied ports work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: git fetch failed with "connect to host claude-bottle-ssh-gate-implementer port 30009: Connection refused". OpenSSH treats a URL-supplied port (the user's remote was ssh://git@gitea.dideric.is:30009/...) as overriding the ~/.ssh/config Port directive, so even though the config wrote Port 30000 the agent dialed :30009 — where nothing was listening because the gate had been assigned BASE_LISTEN_PORT + index. Fix: the gate's listen port now equals the upstream port. Same script, same socat, just port = entry.Port. Two entries on the same upstream port are rejected at prepare time (the gate is one container with a flat port space). Re-smoked: probe nc github.com via the gate at :22, banner came back as expected. PRD 0007 updated to record the design refinement. --- claude_bottle/ssh_gate.py | 56 +++++++++++++++++++++---------- docs/prds/0007-ssh-egress-gate.md | 10 ++++-- tests/unit/test_ssh_gate.py | 41 +++++++++++++++++----- 3 files changed, 78 insertions(+), 29 deletions(-) diff --git a/claude_bottle/ssh_gate.py b/claude_bottle/ssh_gate.py index a531d7f..9b73fee 100644 --- a/claude_bottle/ssh_gate.py +++ b/claude_bottle/ssh_gate.py @@ -17,13 +17,11 @@ from abc import ABC, abstractmethod from dataclasses import dataclass from pathlib import Path +from .log import die from .manifest import Bottle -# First listen port on the gate; entry i listens on BASE_LISTEN_PORT + i. -# Picked high enough to avoid colliding with anything an alpine image -# might pre-bind. The port space is per-gate (gate is per-agent) so -# collisions across bottles aren't possible. -BASE_LISTEN_PORT = 30000 +# Default port when an ssh entry has no `Port` field. Matches OpenSSH. +_DEFAULT_SSH_PORT = 22 @dataclass(frozen=True) @@ -32,7 +30,15 @@ class SSHGateUpstream: forward each connection to `upstream_host:upstream_port`. The `bottle_host_alias` is the `Host` value from the manifest entry, kept for diagnostics + so the ssh provisioner can correlate - upstreams with their alias.""" + upstreams with their alias. + + `listen_port` mirrors the upstream port. That choice lets git + URLs that bake the upstream port into the remote (e.g. + `ssh://git@host:30009/repo.git`) work without rewriting: OpenSSH + treats a URL-supplied port as overriding the config's `Port` + directive, so the gate must be reachable on the same port the URL + names. Two ssh entries that share an upstream port are a config + error and rejected at prepare time.""" listen_port: int upstream_host: str @@ -60,19 +66,33 @@ class SSHGatePlan: def ssh_gate_upstreams_for_bottle(bottle: Bottle) -> tuple[SSHGateUpstream, ...]: - """Deterministic assignment of listen ports to bottle.ssh entries: - BASE_LISTEN_PORT + index. Order matches `bottle.ssh` so a manifest - re-order yields a different port mapping (intentional — the - provisioner reads the same tuple).""" - return tuple( - SSHGateUpstream( - listen_port=BASE_LISTEN_PORT + i, - upstream_host=e.Hostname, - upstream_port=e.Port, - bottle_host_alias=e.Host, + """Build the gate's upstream table. Each ssh entry's listen port + equals its upstream port so URL-supplied ports (which override + `~/.ssh/config`'s `Port` directive) still reach the gate. + + Dies on two entries sharing an upstream port — the gate is a + single container with a flat port space, so each listener has to + be unique.""" + seen_ports: dict[int, str] = {} + upstreams: list[SSHGateUpstream] = [] + for e in bottle.ssh: + port = int(e.Port) if e.Port else _DEFAULT_SSH_PORT + if port in seen_ports: + die( + f"ssh entries '{seen_ports[port]}' and '{e.Host}' share upstream port " + f"{port}; the per-agent ssh gate can only forward one upstream " + f"per port. Change one of the upstream Ports in claude-bottle.json." + ) + seen_ports[port] = e.Host + upstreams.append( + SSHGateUpstream( + listen_port=port, + upstream_host=e.Hostname, + upstream_port=e.Port, + bottle_host_alias=e.Host, + ) ) - for i, e in enumerate(bottle.ssh) - ) + return tuple(upstreams) def ssh_gate_render_entrypoint(upstreams: tuple[SSHGateUpstream, ...]) -> str: diff --git a/docs/prds/0007-ssh-egress-gate.md b/docs/prds/0007-ssh-egress-gate.md index 78101ae..6ec1f66 100644 --- a/docs/prds/0007-ssh-egress-gate.md +++ b/docs/prds/0007-ssh-egress-gate.md @@ -108,8 +108,14 @@ script that backgrounds N socat invocations: socat TCP-LISTEN:,reuseaddr,fork TCP:: ``` -Listen ports are assigned deterministically per ssh entry (e.g. -`30000 + index`). One container, N listeners, N upstreams. +Listen ports mirror the upstream port (entry `Port`, default 22). +That choice is load-bearing: OpenSSH treats a URL-supplied port +(e.g. `ssh://git@host:30009/repo.git`) as overriding the config's +`Port` directive, so the gate has to be reachable on the same port +the URL names — otherwise git fetch hits "connection refused" on +the URL's port even though the config block points elsewhere. Two +ssh entries sharing an upstream port are a config error and +rejected at prepare time. One container, N listeners, N upstreams. ### Existing code touched diff --git a/tests/unit/test_ssh_gate.py b/tests/unit/test_ssh_gate.py index 4c8d89a..c972a86 100644 --- a/tests/unit/test_ssh_gate.py +++ b/tests/unit/test_ssh_gate.py @@ -6,8 +6,8 @@ import tempfile import unittest from pathlib import Path +from claude_bottle.manifest import Manifest from claude_bottle.ssh_gate import ( - BASE_LISTEN_PORT, SSHGate, SSHGatePlan, SSHGateUpstream, @@ -29,17 +29,20 @@ class _StubGate(SSHGate): class TestUpstreamAssignment(unittest.TestCase): - def test_indexed_listen_ports(self): + def test_listen_port_matches_upstream_port(self): + # Critical: URLs like ssh://git@host:30009/... override the + # config Port directive, so the gate must listen on the same + # port the URL names. bottle = fixture_with_ssh().bottles["dev"] upstreams = ssh_gate_upstreams_for_bottle(bottle) self.assertEqual(2, len(upstreams)) - self.assertEqual(BASE_LISTEN_PORT, upstreams[0].listen_port) - self.assertEqual(BASE_LISTEN_PORT + 1, upstreams[1].listen_port) + # Fixture: tailscale-gitea -> 100.78.141.42:30009, github -> github.com:22. + self.assertEqual(30009, upstreams[0].listen_port) + self.assertEqual(22, upstreams[1].listen_port) def test_upstream_fields_mirror_ssh_entry(self): bottle = fixture_with_ssh().bottles["dev"] first = ssh_gate_upstreams_for_bottle(bottle)[0] - # The fixture's first ssh entry: tailscale-gitea / 100.78.141.42:30009. self.assertEqual("tailscale-gitea", first.bottle_host_alias) self.assertEqual("100.78.141.42", first.upstream_host) self.assertEqual("30009", first.upstream_port) @@ -48,20 +51,40 @@ class TestUpstreamAssignment(unittest.TestCase): bottle = fixture_minimal().bottles["dev"] self.assertEqual((), ssh_gate_upstreams_for_bottle(bottle)) + def test_duplicate_upstream_port_is_rejected(self): + # Two entries on the same upstream port can't both have a + # listener — the gate is one container with a flat port + # space. Surface as a clear config error. + manifest = Manifest.from_json_obj({ + "bottles": { + "dev": { + "ssh": [ + {"Host": "a", "IdentityFile": "/dev/null", + "Hostname": "host-a.example", "User": "git", "Port": 22}, + {"Host": "b", "IdentityFile": "/dev/null", + "Hostname": "host-b.example", "User": "git", "Port": 22}, + ], + } + }, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, + }) + with self.assertRaises(SystemExit): + ssh_gate_upstreams_for_bottle(manifest.bottles["dev"]) + class TestEntrypointRender(unittest.TestCase): def test_one_socat_line_per_upstream(self): upstreams = ( - SSHGateUpstream(30000, "gitea.example", "22", "gitea"), - SSHGateUpstream(30001, "github.com", "22", "gh"), + SSHGateUpstream(30009, "gitea.example", "30009", "gitea"), + SSHGateUpstream(22, "github.com", "22", "gh"), ) script = ssh_gate_render_entrypoint(upstreams) self.assertIn("#!/bin/sh", script) self.assertIn( - "socat TCP-LISTEN:30000,reuseaddr,fork TCP:gitea.example:22 &", script + "socat TCP-LISTEN:30009,reuseaddr,fork TCP:gitea.example:30009 &", script ) self.assertIn( - "socat TCP-LISTEN:30001,reuseaddr,fork TCP:github.com:22 &", script + "socat TCP-LISTEN:22,reuseaddr,fork TCP:github.com:22 &", script ) # wait blocks the entrypoint so PID 1 stays alive while sockets # are open.