fix(ssh-gate): listen on the upstream port so URL-supplied ports work
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.
This commit is contained in:
+38
-18
@@ -17,13 +17,11 @@ from abc import ABC, abstractmethod
|
|||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
from .log import die
|
||||||
from .manifest import Bottle
|
from .manifest import Bottle
|
||||||
|
|
||||||
# First listen port on the gate; entry i listens on BASE_LISTEN_PORT + i.
|
# Default port when an ssh entry has no `Port` field. Matches OpenSSH.
|
||||||
# Picked high enough to avoid colliding with anything an alpine image
|
_DEFAULT_SSH_PORT = 22
|
||||||
# might pre-bind. The port space is per-gate (gate is per-agent) so
|
|
||||||
# collisions across bottles aren't possible.
|
|
||||||
BASE_LISTEN_PORT = 30000
|
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
@@ -32,7 +30,15 @@ class SSHGateUpstream:
|
|||||||
forward each connection to `upstream_host:upstream_port`. The
|
forward each connection to `upstream_host:upstream_port`. The
|
||||||
`bottle_host_alias` is the `Host` value from the manifest entry,
|
`bottle_host_alias` is the `Host` value from the manifest entry,
|
||||||
kept for diagnostics + so the ssh provisioner can correlate
|
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
|
listen_port: int
|
||||||
upstream_host: str
|
upstream_host: str
|
||||||
@@ -60,19 +66,33 @@ class SSHGatePlan:
|
|||||||
|
|
||||||
|
|
||||||
def ssh_gate_upstreams_for_bottle(bottle: Bottle) -> tuple[SSHGateUpstream, ...]:
|
def ssh_gate_upstreams_for_bottle(bottle: Bottle) -> tuple[SSHGateUpstream, ...]:
|
||||||
"""Deterministic assignment of listen ports to bottle.ssh entries:
|
"""Build the gate's upstream table. Each ssh entry's listen port
|
||||||
BASE_LISTEN_PORT + index. Order matches `bottle.ssh` so a manifest
|
equals its upstream port so URL-supplied ports (which override
|
||||||
re-order yields a different port mapping (intentional — the
|
`~/.ssh/config`'s `Port` directive) still reach the gate.
|
||||||
provisioner reads the same tuple)."""
|
|
||||||
return tuple(
|
Dies on two entries sharing an upstream port — the gate is a
|
||||||
SSHGateUpstream(
|
single container with a flat port space, so each listener has to
|
||||||
listen_port=BASE_LISTEN_PORT + i,
|
be unique."""
|
||||||
upstream_host=e.Hostname,
|
seen_ports: dict[int, str] = {}
|
||||||
upstream_port=e.Port,
|
upstreams: list[SSHGateUpstream] = []
|
||||||
bottle_host_alias=e.Host,
|
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:
|
def ssh_gate_render_entrypoint(upstreams: tuple[SSHGateUpstream, ...]) -> str:
|
||||||
|
|||||||
@@ -108,8 +108,14 @@ script that backgrounds N socat invocations:
|
|||||||
socat TCP-LISTEN:<port_i>,reuseaddr,fork TCP:<Hostname_i>:<Port_i>
|
socat TCP-LISTEN:<port_i>,reuseaddr,fork TCP:<Hostname_i>:<Port_i>
|
||||||
```
|
```
|
||||||
|
|
||||||
Listen ports are assigned deterministically per ssh entry (e.g.
|
Listen ports mirror the upstream port (entry `Port`, default 22).
|
||||||
`30000 + index`). One container, N listeners, N upstreams.
|
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
|
### Existing code touched
|
||||||
|
|
||||||
|
|||||||
@@ -6,8 +6,8 @@ import tempfile
|
|||||||
import unittest
|
import unittest
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
from claude_bottle.manifest import Manifest
|
||||||
from claude_bottle.ssh_gate import (
|
from claude_bottle.ssh_gate import (
|
||||||
BASE_LISTEN_PORT,
|
|
||||||
SSHGate,
|
SSHGate,
|
||||||
SSHGatePlan,
|
SSHGatePlan,
|
||||||
SSHGateUpstream,
|
SSHGateUpstream,
|
||||||
@@ -29,17 +29,20 @@ class _StubGate(SSHGate):
|
|||||||
|
|
||||||
|
|
||||||
class TestUpstreamAssignment(unittest.TestCase):
|
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"]
|
bottle = fixture_with_ssh().bottles["dev"]
|
||||||
upstreams = ssh_gate_upstreams_for_bottle(bottle)
|
upstreams = ssh_gate_upstreams_for_bottle(bottle)
|
||||||
self.assertEqual(2, len(upstreams))
|
self.assertEqual(2, len(upstreams))
|
||||||
self.assertEqual(BASE_LISTEN_PORT, upstreams[0].listen_port)
|
# Fixture: tailscale-gitea -> 100.78.141.42:30009, github -> github.com:22.
|
||||||
self.assertEqual(BASE_LISTEN_PORT + 1, upstreams[1].listen_port)
|
self.assertEqual(30009, upstreams[0].listen_port)
|
||||||
|
self.assertEqual(22, upstreams[1].listen_port)
|
||||||
|
|
||||||
def test_upstream_fields_mirror_ssh_entry(self):
|
def test_upstream_fields_mirror_ssh_entry(self):
|
||||||
bottle = fixture_with_ssh().bottles["dev"]
|
bottle = fixture_with_ssh().bottles["dev"]
|
||||||
first = ssh_gate_upstreams_for_bottle(bottle)[0]
|
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("tailscale-gitea", first.bottle_host_alias)
|
||||||
self.assertEqual("100.78.141.42", first.upstream_host)
|
self.assertEqual("100.78.141.42", first.upstream_host)
|
||||||
self.assertEqual("30009", first.upstream_port)
|
self.assertEqual("30009", first.upstream_port)
|
||||||
@@ -48,20 +51,40 @@ class TestUpstreamAssignment(unittest.TestCase):
|
|||||||
bottle = fixture_minimal().bottles["dev"]
|
bottle = fixture_minimal().bottles["dev"]
|
||||||
self.assertEqual((), ssh_gate_upstreams_for_bottle(bottle))
|
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):
|
class TestEntrypointRender(unittest.TestCase):
|
||||||
def test_one_socat_line_per_upstream(self):
|
def test_one_socat_line_per_upstream(self):
|
||||||
upstreams = (
|
upstreams = (
|
||||||
SSHGateUpstream(30000, "gitea.example", "22", "gitea"),
|
SSHGateUpstream(30009, "gitea.example", "30009", "gitea"),
|
||||||
SSHGateUpstream(30001, "github.com", "22", "gh"),
|
SSHGateUpstream(22, "github.com", "22", "gh"),
|
||||||
)
|
)
|
||||||
script = ssh_gate_render_entrypoint(upstreams)
|
script = ssh_gate_render_entrypoint(upstreams)
|
||||||
self.assertIn("#!/bin/sh", script)
|
self.assertIn("#!/bin/sh", script)
|
||||||
self.assertIn(
|
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(
|
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
|
# wait blocks the entrypoint so PID 1 stays alive while sockets
|
||||||
# are open.
|
# are open.
|
||||||
|
|||||||
Reference in New Issue
Block a user