fix(ssh): IPv6 ProxyJump + port range + dedup + docker timeouts

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.
This commit is contained in:
José Almeida
2026-05-17 21:44:28 +01:00
parent 274e039a0f
commit 99edda84b8
3 changed files with 68 additions and 18 deletions
@@ -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
@@ -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],
+7 -1
View File
@@ -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 });