From 99edda84b8235bbfba64dd447927bf042cfaa408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= Date: Sun, 17 May 2026 21:44:28 +0100 Subject: [PATCH] fix(ssh): IPv6 ProxyJump + port range + dedup + docker timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parseLiteralProxyJump: handle bracketed IPv6 + reject invalid ports. The literal parser used LastIndex(":") to split host from port, which for [2001:db8::1]:22 left host as "[2001:db8::1]" and produced an invalid double-bracket address through net.JoinHostPort. Recognise the bracketed form explicitly via a parseBracketedHostPort helper and strip the brackets. Validate port is in [1, 65535] instead of accepting any integer (Atoi happily parses negative or out-of-range values). Split the host/port branch into its own function so the nesting stays under the golangci nestif limit. ssh-create-page: dedupe on append. setExecutors([...current, next]) would double-list the same executor if a WS event (or another refresh) inserted it while createExecutor was in flight. Skip the append when current already has the id. ssh-image fixture: time-bound docker probe + build. hasSSHContainerSupport ran `docker info` with no timeout — an unreachable / hung daemon would stall the Playwright worker indefinitely. Cap at 10s for the probe. buildE2ESSHImage runs `docker build` with no timeout; cap at 15 min so a stuck registry pull or BuildKit hang fails fast. --- .../lifecycle/executor_ssh_connection.go | 71 +++++++++++++++---- .../executors/new/[type]/ssh-create-page.tsx | 7 +- apps/web/e2e/fixtures/ssh-image.ts | 8 ++- 3 files changed, 68 insertions(+), 18 deletions(-) diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection.go index c6549d82f4..93e2a44097 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection.go @@ -479,10 +479,10 @@ func resolveBastion(target *SSHTarget) (*SSHTarget, error) { }) } -// parseLiteralProxyJump recognizes a `[user@]host[:port]` ProxyJump literal. -// Returns ok=false when the input doesn't contain an `@` or `:` (treat as -// alias instead). Anything that doesn't parse cleanly also returns false so -// the alias path can take over. +// parseLiteralProxyJump recognizes a `[user@]host[:port]` ProxyJump literal, +// including IPv6 hosts in `[2001:db8::1]:22` form. Returns ok=false when the +// input doesn't contain an `@` or `:` (treat as alias instead). Anything that +// doesn't parse cleanly also returns false so the alias path can take over. func parseLiteralProxyJump(s string) (user, host string, port int, ok bool) { s = strings.TrimSpace(s) if s == "" || !strings.ContainsAny(s, "@:") { @@ -493,22 +493,63 @@ func parseLiteralProxyJump(s string) (user, host string, port int, ok bool) { user = rest[:at] rest = rest[at+1:] } - if colon := strings.LastIndex(rest, ":"); colon != -1 { - portStr := rest[colon+1:] - n, err := strconv.Atoi(portStr) - if err != nil { - return "", "", 0, false - } - port = n - rest = rest[:colon] - } - host = rest - if host == "" { + host, port, ok = parseProxyJumpHostPort(rest) + if !ok || host == "" { return "", "", 0, false } return user, host, port, true } +// parseProxyJumpHostPort splits the `host[:port]` segment of a ProxyJump +// literal, with IPv6 bracketing recognised. ok=false on any malformed input +// (mismatched brackets, junk after `]`, non-numeric or out-of-range port). +func parseProxyJumpHostPort(s string) (host string, port int, ok bool) { + if strings.HasPrefix(s, "[") { + return parseBracketedHostPort(s) + } + if colon := strings.LastIndex(s, ":"); colon != -1 { + p, pok := parsePortString(s[colon+1:]) + if !pok { + return "", 0, false + } + return s[:colon], p, true + } + return s, 0, true +} + +// parseBracketedHostPort handles the `[host][:port]?` IPv6 form. Strips the +// brackets so callers can pass host straight to net.JoinHostPort without +// producing `[[…]]:port`. +func parseBracketedHostPort(s string) (host string, port int, ok bool) { + end := strings.Index(s, "]") + if end == -1 { + return "", 0, false + } + host = s[1:end] + tail := s[end+1:] + if tail == "" { + return host, 0, true + } + if !strings.HasPrefix(tail, ":") { + return "", 0, false + } + p, pok := parsePortString(tail[1:]) + if !pok { + return "", 0, false + } + return host, p, true +} + +// parsePortString parses a base-10 port and rejects values outside the +// inclusive [1, 65535] TCP port range. +func parsePortString(s string) (int, bool) { + n, err := strconv.Atoi(s) + if err != nil || n < 1 || n > 65535 { + return 0, false + } + return n, true +} + // SSH connections are owned per-session (no shared pool): a session's client // lives on sshSessionState and is Close()d by StopInstance. The earlier // pool/refcount/keepalive plumbing was dead code — the executor never used it diff --git a/apps/web/app/settings/executors/new/[type]/ssh-create-page.tsx b/apps/web/app/settings/executors/new/[type]/ssh-create-page.tsx index b32da451c7..ec63d5cc1f 100644 --- a/apps/web/app/settings/executors/new/[type]/ssh-create-page.tsx +++ b/apps/web/app/settings/executors/new/[type]/ssh-create-page.tsx @@ -48,9 +48,12 @@ export function SSHCreatePage() { }; // Read the latest store snapshot at write time so a concurrent // updater (another fetch, a WS event, etc.) that landed while - // createExecutor was in-flight doesn't get overwritten. + // createExecutor was in-flight doesn't get overwritten. Dedupe + // on id so a WS event that already inserted this executor doesn't + // double-list it after our append. const current = store.getState().executors.items; - store.getState().setExecutors([...current, next]); + const merged = current.some((e) => e.id === next.id) ? current : [...current, next]; + store.getState().setExecutors(merged); router.push(`/settings/executors/ssh/${created.id}`); }, [router, store], diff --git a/apps/web/e2e/fixtures/ssh-image.ts b/apps/web/e2e/fixtures/ssh-image.ts index 4b5caa2142..c67ed70be2 100644 --- a/apps/web/e2e/fixtures/ssh-image.ts +++ b/apps/web/e2e/fixtures/ssh-image.ts @@ -123,7 +123,9 @@ ENTRYPOINT ["/entrypoint.sh"] * implying anything about the Docker executor. */ export function hasSSHContainerSupport(): boolean { - return spawnSync("docker", ["info"], { stdio: "ignore" }).status === 0; + // 10s ceiling so a hung / unreachable Docker daemon doesn't stall the + // Playwright worker indefinitely — we just want a yes/no probe. + return spawnSync("docker", ["info"], { stdio: "ignore", timeout: 10_000 }).status === 0; } /** @@ -150,6 +152,10 @@ export function buildE2ESSHImage(): void { fs.copyFileSync(mockAgentPath, path.join(ctxDir, "mock-agent-linux-amd64")); execFileSync("docker", ["build", "-t", SSH_E2E_IMAGE_TAG, ctxDir], { stdio: process.env.E2E_DEBUG ? "inherit" : "ignore", + // 15-minute ceiling — even cold-cache builds (apk add openssh-server + // + a few hundred KB of mock-agent + sshd config) finish well under + // a minute. A stuck registry pull or BuildKit hang should fail fast. + timeout: 15 * 60 * 1000, }); } finally { fs.rmSync(ctxDir, { recursive: true, force: true });