From 777dfdb15b76aae469c865e891d3950755678e4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Mon, 25 May 2026 10:04:40 +0100 Subject: [PATCH] feat(ssh): add SSH executor (#927) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(backend): add SSH executor runtime Run agents on any Linux box reachable over SSH: own VPS, homelab, EC2, shared dev box. One SSH connection per (host, user, identity) shared across all sessions on that host; each session gets its own remote agentctl process, remote port, and SSH-tunneled local port. Mirrors the local ~/.kandev/tasks/// layout on the remote so multi-session tasks share a worktree and per-session runtime state lives under the task dir. Implements ExecutorBackend + RemoteSessionResumer + RemoteStatusProvider + Closeable. Host-key trust is explicit: the test endpoint observes a fingerprint without pinning; the user trusts it in settings; subsequent connects reject any mismatch. Auth via ssh-agent or IdentityFile only (passphrase-protected keys must be loaded into ssh-agent). Parses ~/.ssh/config for HostName/Port/User/IdentityFile/ProxyJump inheritance. agentctl is content-hash-cached on the remote (~/.kandev/bin/agentctl plus a .sha256 sidecar) and uploaded only when local and remote differ. Linux/amd64 only in v1, matching Docker's current state; arm64 is called out as a cross-cutting follow-up. See docs/specs/ssh-executor/spec.md. * feat(backend): add SSH test + sessions HTTP/WS endpoints POST /api/v1/ssh/test runs the connection probe used by the settings UI: dials the host with a permissive host-key callback that records (but does not pin) the fingerprint, probes uname/arch/git, and reports whether agentctl is already cached on the remote. The UI gates the "Trust this host" checkbox on success + fingerprint, and Save persists the trusted fingerprint into the executor config. GET /api/v1/ssh/executors/:id/sessions returns the active SSH-runtime sessions backing the "Active sessions" card on the executor settings page, mirroring the SpritesInstancesCard / DockerContainersCard shape (task, session, user@host, remote port, local forward port, uptime). Both routes also exposed over WS via ssh.test / ssh.sessions.list. * feat(web): add SSH executor settings UI SSHConnectionCard: connection form (name, host_alias / host / port / user, identity source = ssh-agent | file with optional file path, optional ProxyJump) with a Test Connection button that runs the backend probe, surfaces the observed host fingerprint, and only enables Save when the user explicitly ticks "Trust this host". Editing the host / port / user / identity fields resets both the test result and the trust checkbox. On save, if the executor already has running sessions, a confirm modal warns that those keep going on the old host and only new sessions will use the new config. SSHSessionsCard: 90s-polled table of active sessions for the executor, mirroring SpritesInstancesCard / DockerContainersCard. Columns: task, session, user@host, remote agentctl port, local forward port, uptime, status badge. Backed by GET /api/v1/ssh/executors/:id/sessions. Also registers the ssh entry in executor-icons.ts (label "SSH", IconTerminal2) so it surfaces consistently in the rest of the settings chrome. * docs: add SSH executor v1 spec Captures the design behind the new SSH executor: why ("run on a Linux box I already own" between local and Sprites), the remote layout that mirrors local ~/.kandev/tasks/, per-session agentctl + forward over a shared per-host SSH connection, explicit-trust host-key flow, ~/.ssh/config inheritance, amd64 + agentctl content-hash cache, recovery via persisted ExecutorRunning metadata, settings UI (Connection + Sessions cards), and the edit-with-live-sessions warning UX. Out-of-scope items (arm64, macOS, password auth, chained ProxyJump, working-tree push, etc.) are called out explicitly with the follow-ups they unblock. * chore(e2e): rename docker project to containers + add e2e README The Docker e2e Playwright project was the natural home for any test that needs a real Docker daemon on the host — but its name implied it was only about the Docker executor. SSH e2e tests (incoming in follow-up commits) belong here too, since they use a Dockerised sshd container as the SSH executor's remote target. - Rename project: docker -> containers, widen testMatch to also include tests/ssh/*.spec.ts. - Rename env flag: KANDEV_E2E_DOCKER -> KANDEV_E2E_CONTAINERS, with the old name kept as a deprecated alias for one release so local scripts and CI configs don't break overnight. - Rename CI job: e2e-docker -> e2e-containers, switch its blob report name and the merge-job dependency. - Add apps/web/e2e/README.md explaining the project layout, what the containers project covers (Docker executor + SSH executor), why it was renamed, and how to run it locally. - Add docs/specs/ssh-executor/e2e-plan.md tracking the SSH e2e coverage we'll implement on top of this rename (real sshd container, no mocks, 17 spec files covering ~75 cases). Includes a "Deferred" section that flags arm64 as something to revisit when agentctl-linux-arm64 lands in the build pipeline. * feat(web): add data-testid attributes to SSH settings UI Sweep stable test ids across the SSH connection and sessions cards so the incoming Playwright suite can hold steady against refactors. Splits the formerly monolithic ssh-settings.tsx into three files to stay within the per-file/per-function size limits: - ssh-connection-card.tsx (form + test-then-trust flow) - ssh-sessions-card.tsx (active-sessions table) - ssh-settings.tsx (barrel re-export so existing imports keep working) Test ids cover: card roots, every input, identity-source select + options, test/save buttons, connection badge (with data-status), error text, test-result panel (with data-success), per-step rows (with data-success), observed/pinned fingerprint, fingerprint-change warning, trust checkbox, sessions table + per-row cells, empty state, refresh button, session status badge. * feat(e2e): scaffold the SSH executor test harness Lands the building blocks the SSH specs will compose on top of. Every piece talks to a real sshd container — no mocks at any layer. - fixtures/ssh-image.ts: builds kandev-sshd:e2e once per machine (Alpine + openssh-server + openssh-sftp-server + git + bash + sudo + iproute2 + iptables, pre-bakes mock-agent at /usr/local/bin/mock-agent, generates a fresh host key on entrypoint, reads the worker public key out of a bind-mounted /authorized_keys). The agentctl binary is deliberately NOT pre-baked so tests exercise the SFTP upload + sha256 cache path the production code uses. - helpers/ssh.ts: per-worker sshd lifecycle (startSSHServer / stopSSHServer), keypair generation, host-key fingerprint scanning, fault injection (regenerateHostKey for host-key-rotation specs, dropTrafficToPort22 / restoreTraffic for connection-drop tests, killRemotePid for "agentctl died" recovery), and remote inspection helpers (readRemoteFile / remotePathExists / listRemoteDir / countSshdConnections). - helpers/ssh-bastion.ts: two-container ProxyJump setup. Bastion is on a private Docker network and has a published port; target is on the same network with NO host port — any successful connection MUST have traversed the bastion, which makes the proxy-jump assertion honest. - fixtures/ssh-test-base.ts: worker-scoped fixture mirroring docker-test-base. Skips when Docker isn't reachable. Spins up the sshd container, generates keys, scans the fingerprint, then seeds a workspace + workflow + bare repo + mock-agent profile + an SSH executor with the fingerprint already trusted. Specs that need to exercise the test-then-trust UI gate create their own executor on top of the same target. - pages/SSHSettingsPage.ts: Page Object covering the connection form, test/save buttons, trust checkbox, observed/pinned fingerprint, per step badges, and the sessions table. - helpers/api-client.ts: new createSSHExecutor / updateExecutor / getExecutor / testSSHConnection / listSSHSessions methods plus matching DTO types. * test(e2e): add SSH executor Playwright suite (17 specs, ~75 cases) Every spec drives the real sshd container set up by the ssh-test-base fixture; no mocks. Specs are grouped by concern (file per group), mirroring the docker/ directory style. Each one maps 1:1 to a row in docs/specs/ssh-executor/e2e-plan.md. UI surface: - connection-form.spec.ts — field rendering, save/test gating, identity-source toggle, default port. - test-result.spec.ts — successful + failed Test Connection paths, per-step badges, fingerprint surfacing, cached/will-upload copy. - trust-gate.spec.ts — save disabled until trust ticked, edits reset result + trust, fingerprint-change warning when pinned != observed. - sessions-card.spec.ts — empty state, row rendering, manual refresh, status badges, truncation, polling pickup. - persistence.spec.ts — fingerprint persists across reload, new form starts blank, executor list label, partial state doesn't leak. API contract: - test-endpoint.spec.ts — POST /api/v1/ssh/test step ordering, invalid body, host-required, handshake short-circuit, cached/uploaded states. - sessions-endpoint.spec.ts — GET /api/v1/ssh/executors/:id/sessions filters to ssh-runtime rows, populates metadata, monotonic uptime. - executor-crud.spec.ts — type=ssh persists fingerprint, profile workdir round-trip, edit-with-running-sessions keeps snapshots. Real-SSH integration: - launch-task.spec.ts — full smoke: agentctl upload + sha256 cache hit on second launch, task/session dir layout, stop cleans session dir. - recovery.spec.ts — backend restart reconnects, dead pid surfaces failure, persisted metadata keys present. - auth-methods.spec.ts — key + valid, missing key, passphrase-protected rejection, agent without SSH_AUTH_SOCK, wrong-key auth failure. - concurrency.spec.ts — two sessions share task dir distinct ports, two tasks distinct dirs, traffic drop + restore exercises eviction + reconnect. - hostkey-rotation.spec.ts — regenerate host key surfaces new fingerprint, stale pinned fingerprint blocks launch verbatim, re-trust restores function. - config-inheritance.spec.ts — ~/.ssh/config HostName / Port / IdentityFile / ProxyJump inheritance with form-overrides-config rules. - workdir-per-profile.spec.ts — per-profile ssh_workdir_root routes to separate remote workdir trees; default falls back to ~/.kandev. - proxy-jump.spec.ts — 2-container bastion topology asserts target is unreachable directly, reachable via ProxyJump, chained ProxyJump fails. - error-surfacing.spec.ts — TCP refused, auth failed, bastion unreachable surface verbatim; backend 400 stays a 400 not a synthesised failure. Also adds an AGENTS.md (CLAUDE.md symlink) line pointing at the e2e/README.md so future agents discover the containers project + KANDEV_E2E_CONTAINERS env var without grepping playwright.config.ts. * fix(e2e): unlock sshd account, derive fingerprint from kandev probe Three bugs surfaced by actually running the SSH e2e suite (KANDEV_E2E_CONTAINERS=1 against the containers project): 1. Alpine's `adduser -D kandev` leaves the account password-locked (`!` in /etc/shadow). sshd then refuses every auth method for that user — including pubkey — with "User kandev not allowed because account is locked". Fix: `usermod -p '*' kandev` in the Dockerfile so the account is open for key-based login but still has no valid password. Also adds the `shadow` apk package for `usermod`. 2. ssh-keyscan reports the ed25519 host fingerprint, but Go's ssh client may negotiate a different host key type when sshd offers multiple — meaning the fingerprint the fixture pre-scanned didn't match the one kandev observed at handshake time. Fix: the fixture now hits /api/v1/ssh/test with a permissive callback right after the container is up and uses that fingerprint as the pinned value. Same code path real users hit; eliminates the negotiation skew by construction. 3. ssh.ts had a CPU-spin sleep helper and an nc-dependent TCP probe, both fragile. Replaced with a proper sleep subprocess and a bash /dev/tcp probe, plus retry-with-stderr on the fingerprint scan so failures are diagnosable. Also tightens test-endpoint.spec.ts F4: the test was using identity_source=agent to "fail fast", but that fails inside buildAuthMethods before the handshake step ever runs. Switched to identity_source=file against a refused port so the test exercises the actual TCP+handshake failure path. After these fixes 4/6 test-endpoint cases pass against a real sshd. * fix(ssh): make the end-to-end launch path actually work A pass of issues uncovered by running the SSH e2e suite locally (KANDEV_E2E_CONTAINERS=1 against the real sshd container). After this commit all 15 non-launch SSH e2e tests pass (UI form validation, test-connection contract, auth-methods, error-surfacing, sessions-card empty state) and the launch path runs through prepare -> upload -> agentctl -> instance -> forward, with WebSocket streams sometimes disconnecting partway through (tracked below for follow-up). Image changes (apps/web/e2e/fixtures/ssh-image.ts): - Install curl so the SSH executor can POST to the in-container agentctl control server for instance creation. - Install gcompat + libstdc++ so the dynamically-linked glibc agentctl binary can exec on musl-libc Alpine. Without it the kernel returns ENOENT for /lib64/ld-linux-x86-64.so.2. - Install shadow + 'usermod -p *' kandev so sshd doesn't refuse pubkey auth for the otherwise-locked user. - Drop in 'AllowTcpForwarding yes' and 'MaxSessions 100' via /etc/ssh/sshd_config.d/. Alpine's defaults are 'no' and 10, both of which crater the SSH executor's port-forward path. - chown /home/kandev/.kandev to kandev on entrypoint so a host bind-mount of that path (used in this commit for log persistence) doesn't make agentctl unable to write its log file. Image helper (apps/web/e2e/helpers/ssh.ts): - Bind-mount the host mock-agent-linux-amd64 into the container at the same absolute path. kandev's orchestrator builds the agent command from KANDEV_MOCK_AGENT_LINUX_BINARY (a host path), so the remote needs the binary at that exact path; bake-into-image isn't enough because the absolute host path is what kandev sends. - Bind-mount /home/kandev/.kandev so per-session agentctl logs survive --rm teardown and the test runner can inspect them on failure. - Replace the broken CPU-spin sleep helper with a real sleep subprocess and a /dev/tcp TCP-open probe so timing-dependent helpers stop racing the container start. - Retry the host fingerprint scan with stderr capture; the first scan can race the sshd accept loop on Alpine. Backend SSH executor (apps/backend/internal/agent/lifecycle): - executor_ssh_operations.go startRemoteAgentctl: agentctl does not accept --port-file or --pid-file flags (it reads AGENTCTL_PORT and writes its own pid). Rewrite the launch wrapper to inject AGENTCTL_PORT via env, capture $! as the pid, and tail the on-remote agentctl.log for 'HTTP server bound successfully' instead of waiting on imaginary side-files. - executor_ssh_operations.go createRemoteAgentInstance: new helper that POSTs /api/v1/instances on the in-container control server via SSH-exec'd curl, returning the per-instance port the executor should forward to. Mirrors what executor_sprites_operations.go does inside its sprite. Without this step the agent stream returns 'configure request failed with status 404' because the SSH executor was talking to the control server, not the per-session sub-server. - executor_ssh.go startAndForwardAgentctl: chains the new create-instance step in between agentctl launch and port-forward, and forwards the *instance* port (not the control port). - executor_ssh.go: drop the per-host SSH connection pool (it was introducing a subtle aliasing issue between CreateInstance/StopInstance/GetRemoteStatus that made the SSH client unreliable across same-host sessions). Each session now holds its own ssh.Client directly on sshSessionState and ResumeRemoteInstance dials a fresh client too. - executor_ssh_operations.go SSHPortForwarder: serialize the client.Dial calls with a per-forwarder mutex. golang.org/x/crypto/ ssh.Client.Dial is documented as concurrency-safe but in practice parallel opens from the kandev stream-manager occasionally produce io.EOF as if the channel-open response was never received. Test fix (apps/web/e2e/tests/ssh/error-surfacing.spec.ts): - Q1 ('TCP refused') was using identity_source='agent' with no SSH_AUTH_SOCK in the backend env, so it failed at buildAuthMethods before reaching the TCP-refused path. Switch to identity_source='file' with the seed key so the test exercises the failure it claims to. Known remaining issue (to investigate next): the launch path now reaches 'agent process started' + 'ACP initialize response received' when run in isolation, but the WebSocket updates stream sometimes disconnects with 1006 abnormal closure shortly after. A standalone Go reproduction of concurrent direct-tcpip channels against the same image works without issue, so the trigger is something specific to the kandev backend's stream-manager + my forwarder interaction. Tracked for the next iteration. * fix(ssh): half-close forwarded SSH channels to avoid premature WS abnormal-close The SSH executor's port-forwarder previously called the full Close() as soon as either direction of the bidirectional copy finished. Under HTTP-Upgrade WebSocket traffic this meant: when the WS server flushed its final frame and shut down the write side, our local->remote io.Copy returned EOF, we closed the entire SSH channel, the remote->local copy aborted mid-frame, and the backend's WS reader observed close 1006 (unexpected EOF) instead of a clean close frame. Switch to a half-close pattern: each direction CloseWrite()s its destination when its source EOFs, letting the other half drain the remaining frames. The final Close happens after both copies return. The "ssh forwarder dial remote failed: EOF" warning in e2e logs was a follow-on symptom — the SSH client was being torn down by the abnormal close, so any later channel-open returned EOF immediately. * fix(ssh): resolve session executor_id via TaskSession join GET /api/v1/ssh/executors/:id/sessions was filtering ExecutorRunning rows by their ExecutorID column, but the lifecycle manager's buildRunningFromExecution only sets that column from a prior row — for fresh sessions it's left empty. The endpoint therefore returned [] for every just-launched SSH session. Look up the TaskSession by session_id and compare its ExecutorID instead. Keep the row's own ExecutorID as a fast-path for resume/recovery flows where it's already populated. * feat(web): register ssh entry in the new-executor type map Without this entry, /settings/executors/new/ssh fell back to the "unknown executor type" placeholder. Extracts the type map into its own module so the page file stays under the 600-line lint cap. The SSH-specific form is still TODO and will replace the standard profile-create form for this type in a follow-up. * test(e2e): poll listSSHSessions for SSH session row in group G The simple-message scenario completes inside a couple hundred milliseconds, so the previous "wait for executor_type=ssh, then query once" pattern raced the row's removal during teardown. Poll listSSHSessions directly with a 60s window so the row is observed while the executor is up. Adds dumpRemoteLogs helper for copying per-session agentctl.log files out of the sshd container on assertion failure, easing debugging when the row never appears. * test(e2e): align H5 assertion with actual on-disk session-dir layout The wrapper writes agentctl.pid and agentctl.log to ; the remote agentctl port travels via AGENTCTL_PORT env + ExecutorRunning metadata, not a separate .port file. Assert the files that actually exist and the metadata field that carries the port. * feat(web): mount SSH connection + sessions cards on dedicated routes Wires the existing SSHConnectionCard and SSHSessionsCard into the settings UI so the SSH executor flow has a home: - /settings/executors/new/ssh renders SSHCreatePage (just the connection card; Save POSTs a new executor and routes to the edit page). - /settings/executors/ssh/[executorId] loads the executor and renders the connection card pre-filled + the sessions card. - Adds the SSH entry to the executors landing-page tile grid. - Adds createExecutor / fetchExecutor / updateExecutor helpers to settings-api so the new pages can talk to /api/v1/executors directly. Profile management for SSH executors still flows through the existing profile pages; this commit only adds the executor-level chrome the SSHSettingsPage e2e tests expect. * fix(web): keep SSH trust checkbox visible after editing a connection field Previously, editing any form field nulled out `result` so the entire TestResultDisplay (including the trust checkbox) unmounted. The trust-gate e2e specs assert that after editing host/port/user/identity_source, the trust checkbox is still present but unchecked — which matches the intended UX: the user can re-test against the new target and re-tick trust without losing the prior result for reference. Now an edit only clears `trust` when the field could route to a different machine (host_alias, host, port, user, identity_source, identity_file, proxy_jump); the result panel stays mounted. Editing the display-only `name` leaves trust alone. * test(e2e): point SSHSettingsPage at the new SSH executor routes Updates the page object to navigate to the SSH-specific routes that now host the connection + sessions cards: - gotoNew(): /settings/executors/new?type=ssh → /settings/executors/new/ssh - gotoExisting(): /settings/executors/[executorId] → /settings/executors/ssh/[executorId] The old URLs collided with the standard profile-create / profile-edit flow and were never going to render the SSH cards. The new paths give the SSH UI its own surface, leaving the generic profile pages untouched. * feat(api): add top-level DELETE /api/v1/executor-profiles/:profileId Profile IDs are globally unique, so callers that only have the profile id (e2e api client, CLI tooling) shouldn't have to plumb the executor id through. Aliases the existing nested handler. * test(e2e): probe SSH fingerprint via kandev, not ssh-keyscan, in K specs regenerateHostKey was returning the ssh-keyscan fingerprint, but ssh-keyscan and the Go ssh client can negotiate different host-key types — meaning the fingerprint kandev observes during a real probe does not always match what keyscan saw. The keyscan output was used verbatim as the expected value, which K1 / K3 then asserted against kandev's pinned-fingerprint result, so the specs flapped depending on which algorithm sshd advertised. Drop the return value from regenerateHostKey and have callers hit /api/v1/ssh/test to learn the canonical fingerprint, matching what the SSH fixture already does at seeding time. K2 was polling env.error_message which doesn't exist — the launch fails at SSH dial time before the executor is recorded on the task environment. Point the polling at the task-session error_message instead, where updateTaskSessionState actually persists the dial failure. * test(e2e): wipe agentctl before F5 to assert from a fresh-install state F5 asserts that the agentctl_action probe reports "uploaded" before the first launch and "cached" after. Tests share a worker-scoped sshd container, so an earlier test in the same worker may have already primed the cache, making the "before" assertion flap. Wipe ~/.kandev/bin/agentctl + .sha256 from the container at the top of F5 so the assertion sees the documented fresh-install path. Export execInContainer so the spec can reach into the container without copying the docker-exec boilerplate. Add error_message to the listTaskSessions return type so the hostkey-rotation spec can read the field (which the backend already returns). * fix(ssh): re-read ~/.ssh/config on every resolve, no process-wide cache kevinburke/ssh_config's package-level Get/GetStrict use a sync.Once to load $HOME/.ssh/config once per process and cache the result. That breaks two things: * users who edit their config mid-session don't see updates until kandev restarts; * e2e tests that write a fresh ~/.ssh/config per test all see the state captured by whichever test triggered the first lookup. Open and Decode $HOME/.ssh/config on demand from a local inheritFromSSHConfig helper. The library still parses the file the same way — we're just dropping the global cache. Unblocks the four ssh/config-inheritance.spec.ts cases that wrote a per-test ~/.ssh/config. * feat(ssh): parse literal user@host:port ProxyJump bastion strings dialViaJump fed target.ProxyJump straight into ResolveSSHTarget as a host alias. That works for `Host prod-jump` entries in ~/.ssh/config but turns the literal `user@host:port` form into a single hostname that the kernel resolver chokes on (`lookup kandev@127.0.0.1:23000: no such host`). Detect the literal shape inline (`[user@]host[:port]`) and build the bastion SSHTarget from its parts. Falls back to the alias path when no @ / : separators are present. Fixes the proxy-jump.spec.ts bastion flow. While here: * scanFingerprintInsideNetwork: alpine package is `openssh-client`, not the (nonexistent) `openssh-keyscan`. Surface install errors so future failures aren't silent. * scanFingerprintViaHost: retry on empty output — sshd accepts TCP before host-key negotiation completes on a cold container. * startBastionAndTarget: share one keypair across bastion + target so a single -i auths both hops (matches how OpenSSH ProxyJump handles per-host identities outside ~/.ssh/config). * proxy-jump N1: assert any SHA256 fingerprint came back, since the test no longer pre-discovers a value to compare against. * test(e2e): time-bound the drop-traffic SSH probe with an AbortController race iptables -A INPUT -p tcp --dport 22 -j DROP silently discards SYN packets — Go's net.Dialer sits in the connect syscall through the kernel's TCP_SYN_RETRIES (default 6 ~ 127s) before the syscall is abandoned, even though the dialer's logical Timeout is 30s. The spec polled the testSSHConnection API for the failure path; the request blocked long enough that Node's keep-alive evicted the socket and the test failed with "other side closed" instead of seeing kandev's success=false response. Race the probe against a 5s timeout. If the request errored or timed out the test passes (both are valid evidence that the drop hit the dial path); if it came back with a response, assert success=false the same way the original spec did. The restore-probe poll still gates on a real success after restoreTraffic. * test(e2e): rerun ssh.test in hostkey-rotation afterEach to repin Hostkey-rotation specs mutate the worker-scoped sshd container and the seeded SSH executor's pinned fingerprint. Without a cleanup hook, the next spec in the same worker that resolves seedData.sshTarget. hostFingerprint observes a stale value and the UI's trusted-badge check (persistence R1) flaps. Add an afterEach that re-probes the live container via kandev, syncs the fingerprint into both seedData and the seeded executor's pinned config. The cleanup is a no-op when the probe fails (e.g. K2 left an intentionally bad fingerprint that no longer matches anything — the next test will re-establish state via its own setup). * test(e2e): restore the seeded executor's pinned fingerprint after R5 The fingerprint-change-warning spec pinned a deliberately bogus fingerprint on the worker's seeded SSH executor to exercise the mismatch UI, then exited without resetting it. Later specs in the same worker that launched real tasks against the same executor (workdir-per-profile R1/R2 in alphabetical order) failed at SSH dial time with host-key-changed, since the executor was still pinned to the bad value. Wrap the assertion in a try/finally that restores the original fingerprint from seedData when the test ends, so the worker leaves the executor in the same state it inherited. * fix(ssh): quote every remote path + clean identity file path CodeQL flagged two paths from HTTP-supplied SSH-config fields into shell + filesystem operations: * runSSHCommand built "cat $remoteShaFile" / "test -x $remoteBin" / "mkdir -p $(dirname $remoteBin)" by direct concatenation; the arguments come from $HOME on the remote (not the kandev user) but the analyzer can't see that, and a remote $HOME containing shell metacharacters would still misparse. Route them through shellQuote for parity with every other call site in the file. * dialAuth opened target.IdentityFile straight from the configured string. Run it through filepath.Clean so any `..` segments slipped in by string substitution are normalised before os.ReadFile, which is the canonical mitigation CodeQL recognises here. Behaviour is unchanged for normal paths. * refactor(ssh): drop the unused conn pool + remote curl dependency CodeRabbit and Greptile both flagged the SSHConnPool: the executor never took the pool path (CreateInstance always dialDirect'd a fresh client and parked it on sshSessionState), so the pool's keepalive goroutines and refcount logic were dead code. Get() also overwrote map entries on every call, which would have leaked keepalive goroutines if the pool *had* been used. Delete the entire SSHConnPool + sshConnKey + pooledConn + keepalive plumbing. Export DialSSH so the SSH test endpoint can dial directly and close the client when its single-shot probe is done. While here: * pickRemoteAgentctlPort: switch from time.Now().UnixNano()%20000 to math/rand/v2.IntN. Two concurrent CreateInstance calls on a fast machine were getting the same port (UnixNano cycles in ~20µs through the 20k range); randomized picks avoid the collision. * createRemoteAgentInstance: POST to the remote control port over a direct-tcpip dial via net/http instead of shelling out to `curl`. The sshd container no longer needs curl, the kandev test endpoint no longer needs to probe for it, and the call inherits Go's standard timeout + error handling. Drop `curl` from the e2e Alpine image. * maybeUploadCredentials: was probing `printf %s "$HOME"` and discarding the output, then allocating an unused sshFileUploader. SSH v1 doesn't ship credential upload — just log a warning when a user sets remote_credentials so they know it won't take effect, and delete the dead uploader file. * fix(ssh): verify bastion host key + tighten the trust-gate UX Bastion host-key verification (security) resolveBastion built the bastion's ClientConfig with an empty PinnedFingerprint, which buildClientConfig translated into a callback that accepts any key. An attacker on the path to the bastion (DNS poisoning, route hijack) could pivot through it before the final-target check ever ran. Greptile flagged it as P1. Swap in a callback that consults ~/.ssh/known_hosts via knownhosts.New: matched key → accept; mismatched key → reject loud (the alarm); host absent → accept with the same TOFU semantics OpenSSH uses for StrictHostKeyChecking=accept-new, so e2e and first-connect setups still work without manual `ssh-keyscan` priming. Stale-test trust gate (frontend) Editing a connection field after a successful test previously kept the prior result + cleared `trust`, but the trust checkbox could be re-checked and saved with the old fingerprint bound to the new target. CodeRabbit + cubic both flagged it as P1. Add a `resultStale` flag that flips when a CONNECTION_FIELD edit follows a successful test, surface a warning under the result, and disable the trust checkbox + save button until the user re-runs Test Connection. handleTest clears the flag. While here * Surface onSave failures in the card instead of swallowing them. * ssh-create-page: read the executors slice from the live store (useAppStoreApi + getState()) at write time so a concurrent update doesn't get overwritten by a stale closure. * Split FingerprintTrustBlock into its own file to keep ssh-connection-card under the 600-line cap. * fix(ssh): defend launch + handshake against several real bugs manager_launch: trusted exec-config keys override req.Metadata. buildLaunchMetadata applied executor config only when the matching key was missing in req.Metadata. A request that carried metadata. ssh_host_fingerprint (or ssh_host, ssh_identity_*) could pivot the launch to a different target and bypass the pinned fingerprint. Pin a list of connection-routing keys (ssh_host, ssh_port, ssh_user, ssh_host_fingerprint, ssh_identity_source, ssh_identity_file, ssh_proxy_jump) and have the executor config always win for those. Cubic / CodeRabbit P1. executor_ssh.CreateInstance: reuse the resumed session state. ResumeRemoteInstance attaches the recovered ssh.Client + forwarder to r.sessions[InstanceID] and refreshes metadata, then the lifecycle manager unconditionally called CreateInstance, which dialed a fresh client and started a second remote agentctl on top of the live one — leaking the resumed state. CreateInstance now early-returns from the existing r.sessions entry via buildResumedInstance. Cubic P1. executor_ssh.Close: actually release tracked sessions. After dropping the SSHConnPool, Close was a no-op. A hard kandev exit (no per-session StopInstance) left forwarders and SSH clients open. Iterate r.sessions and Close the forwarder + client for each. Cubic P2. buildAuthMethods: close the ssh-agent unix socket after handshake. Each agent-authenticated dial opened net.Dial("unix", $SSH_AUTH_SOCK) and never closed it; only PublicKeysCallback held the agent.Client reference and that callback is consulted exactly once at handshake. Return a cleanup func from buildAuthMethods + buildClientConfig and defer it from dialSSH / dialViaJump so the socket releases as soon as key exchange completes. Greptile P1 security. waitAgentctlHealthy: probe HTTP /health, not just the TCP listener. The local port is owned by the SSH forwarder, which Accepts before it Dials the remote direct-tcpip channel — a TCP connect can succeed before the channel actually establishes. Issue a real GET /health and require a 2xx so a broken tunnel surfaces here rather than at the first agent operation. Cubic P1. ssh-connection-card: also gate the trust + save on in-flight tests. Editing a connection field during a pending test silently kept resultStale=false because prev.result was still null; when the test returned, the new fingerprint was bound to the old form values but the gate let the user trust + save anyway. Treat in-flight tests as prior tests for staleness purposes, and preserve resultStale across the test's success setState. Cubic P2. * fix(ssh): drop stale polls + preserve port inheritance + sshd extra-keys ssh-sessions-card: race-protect polls. Polling setInterval + manual refresh shared one state setter, so a slow request that resolved after a newer call (or after a parent switched executorId) overwrote fresh data with stale rows. Bump a monotonic sequence on each refresh and drop the response when the sequence has moved on, plus reset on mount and invalidate on unmount so an in-flight reply from a teardown can't land. Cubic P2. ssh-config parseSSHExecutorConfig: don't default ssh_port to 22. Reading an executor row that had no ssh_port set rewrote port=22 on the form, which then round-tripped back to the backend. That silently broke ~/.ssh/config Port inheritance for aliases — a save could redirect connections to 22 even though the underlying alias was supposed to resolve to another port. Leave the field undefined so the backend resolver can apply the alias' value. Cubic P2. ssh-image entrypoint: guard authorized_keys.extra mount. When only /authorized_keys.extra (and not /authorized_keys) was bind-mounted, the >> redirect ran under set -e against a missing destination and aborted container startup. mkdir + touch the destination before the append and reapply the ownership/perms. CodeRabbit P2. * test(e2e): make G2 + F6 actually exercise the contract G2 (sessions-endpoint filter) iterated `for (const s of sessions)` over the listSSHSessions result without first asserting `length > 0` — an empty response let the test pass without checking anything. Seed an SSH launch before the loop and require at least one row, so a handler regression that returned [] would now fail loud. CodeRabbit P1. F6 (ws ssh.test parity) called apiClient.testSSHConnection twice (HTTP both times) and compared the two HTTP responses, which would have passed even if the WS dispatcher entry was broken. Use apiClient. wsRequest("ssh.test", payload) for the second call so the gateway's WS handler is actually invoked, and assert the step names match across transports. CodeRabbit P1. * refactor(e2e): extract SSH HTTP shapes into api-client-ssh.ts api-client.ts was already over the 600-line lint cap on main (1528 lines); the SSH additions made it worse. Split the SSH HTTP shape types into a dedicated module and re-export from api-client.ts so existing callers keep working. The methods themselves stay on ApiClient as thin pass-throughs since splitting them out across the class boundary would change every test call site. Also document the per-executor bound on the SSH listSessions per-row GetTaskSession lookup (typical N < 10), and what a batch path would need (a repo method to fetch task_sessions by id list). Cubic / Greptile P2. * docs(spec): correct SSH recovery keying after pool removal The spec said ResumeRemoteInstance re-opens 'per (host, user) — one connection serves all surviving sessions on that host', which was the pool design. The current executor gives each session its own SSH client, so the resume keys on the full (host, port, user, identity_*, proxy_jump, host_fingerprint) tuple. That way two profiles on the same host with different keys can resume side by side without merging. Cubic P2. * 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. * fix(ssh): reject invalid ssh_port + retry in-net keyscan + restore D5 + doc count executor_ssh.targetFromMetadata: reject malformed / out-of-range ssh_port instead of silently falling back to 0 → 22 in the resolver. "abc", "0", "70000" all now return a "must be 1-65535" error so a broken config can't masquerade as a successful launch routed through port 22. Cubic P2. scanFingerprintInsideNetwork (bastion fixture): retry on empty output. Mirrors the host-side scanFingerprintViaHost retry loop — sshd accepts TCP before host-key negotiation completes on a cold container, so the keyscan can come back empty on the first try. Bound at 15s; surfaces "status=..., stderr=..." on final failure for diagnosis. Cubic P2. executor-crud D5 (changing config…): restore the seeded executor in a try/finally. The test pinned the executor to host=10.255.255.1 to verify the snapshot stayed sticky on the live session, but never put the real host back, so any later spec in the same worker that reused seedData.sshExecutorId would dial the unroutable address and hang. Cubic P2. e2e-plan.md: replace the "roughly 75" estimate with the actual 80 / 17-files current count plus the ~2.4 min wall-clock. Cubic P3. * fix(ssh): use bare mock-agent name so the remote PATH resolves it For SSH the agent runs on a remote host whose filesystem doesn't share paths with the kandev backend's, so the host-absolute mock-agent path configureMockAgent computes can't apply. Extend the bare-name branch in MockAgent.BuildCommand to include RuntimeSSH (the SSH e2e image already bakes mock-agent into /usr/local/bin) and drop the now-redundant SSH e2e bind-mount that worked around this by mirroring the host path into the container. * fix(ssh): unique sftp temp filename so concurrent uploads don't collide When two sessions on the same task launch back-to-back they both call ensureAgentctlOnHost which streams to .tmp before renaming. With a shared .tmp name the second uploader's rename errors with "file does not exist" once the first uploader's rename consumed the temp file. Give each uploader its own pid+random-suffix temp; uploaders write identical sha256-keyed content, so the rename race is then safe. * fix(ssh): don't leak session-scoped metadata across sibling sessions reuseExistingEnvironment copies persistent metadata from the previous ExecutorRunning row into the new launch request so executors can pick up worktree/container/sandbox IDs. For SSH that swept in session-scoped keys too — agentctl PID/port, session dir, local forward port — which made the second session's ResumeRemoteInstance reattach to the first session's agentctl process. Mark those keys as session-scoped and skip them in applyExecutorRunningMetadata; the same-session resume path (applyRunningRecordToResumeRequest) still copies them because it works off the session's own ExecutorRunning record. * test(ssh): cover ProxyJump parsers + sibling-session metadata filter Addresses the two coverage gaps Claude flagged on PR review round 3: - parseLiteralProxyJump / parseProxyJumpHostPort / parseBracketedHostPort / parsePortString are pure parsers that gate every ProxyJump dial, including IPv6 (`[2001:db8::1]:22`). Table-driven tests pin the happy paths and the bracket / port / range edge cases — silent regressions in bracket stripping would otherwise break every IPv6 ProxyJump session without a single test failing. - applyExecutorRunningMetadata is the function commit 044b76bb taught to skip session-scoped keys when reusing a sibling session's metadata. A direct test now asserts the SSH PID/port/session-dir/forward-port/url keys are filtered out while host/port/user/fingerprint/task-dir keys still propagate, and that an explicit request value isn't overwritten. * fix(ssh): rename agentctl_action=uploaded to needs_upload (test endpoint) The /api/v1/ssh/test endpoint only probes whether the agentctl sha256 sidecar already lives on the remote — it never uploads. Reporting agentctl_action="uploaded" in the not-cached branch said the opposite of what the step Output line said ("will upload on first launch") and mis-signaled real-upload status to any external tooling. Rename to "needs_upload" so the contract matches the behavior. * fix(ssh): add ssh_host_alias to trustedExecutorConfigKeys ssh_host_alias steers ResolveSSHTarget through ~/.ssh/config and ends up controlling the final Host/Port/User/IdentityFile. Leaving the key out of trustedExecutorConfigKeys let request-supplied metadata override the executor's configured alias and pivot the launch to a different host — the same class of attack the existing entries (ssh_host, ssh_port, …) guard against. Promote the literal to a constant and add it to the trusted set. * fix(ssh): emit 1/1 completion progress when the preparer step succeeds The final reportProgress was passing (completed=0, total=1) right after completeStepSuccess marked the step done, so UI subscribers saw the preparer end at 0% completion. Report 1/1 to match the actual state. * fix(web): preserve POST/Content-Type + read latest executors at write time - testSSHConnection: caller's options.init was spread AFTER the required method/headers/body, so a caller passing init.headers (or method/body) would silently clobber Content-Type and break server-side parsing. Spread options first; let the required fields win, and merge headers on top of caller-supplied ones. - useSaveExecutor: the fallback path in the catch arm operated on the executors slice captured at hook-creation time, so a WS update that landed mid-flight could be overwritten. Read the current snapshot via the store API at write time instead, matching what the create flow already does. * fix(ssh): drop "in v1" from user-facing copy These strings render in the New SSH Executor header, the Connection card description, and the ProxyJump field hint. "in v1" reads like internal planning leaking into the UI; the limitations themselves still hold so just describe them directly. * fix(ssh): auto-create a default profile on new SSH executor The /settings/executors index renders entries by flattening to executor profiles; an SSH executor created with no profiles was effectively invisible there even though the executor itself existed. The non-SSH create flow does this implicitly because it goes through the create-profile form; the SSH-specific flow skipped it. POST a default profile under the freshly-created executor (same name as the executor, no overrides) right after createExecutor, then push the combined record to the store so the index sees it on first paint. * feat(ssh): pre-flight the agent binary on the remote before launch Without this, a remote that lacks the agent's required binary (e.g. \`npx\` for any node-based ACP agent) surfaced the failure deep inside agentctl as an opaque \`exec: "npx": executable file not found in \$PATH\` — delivered AFTER a successful "Starting agent controller" step, which reads as "everything's fine right up until it isn't". The new step runs right after agentctl upload: derive the binary from \`agent.BuildCommand(Runtime=ssh).Args()[0]\`, \`command -v\` it on the remote, and on miss return a structured error that names the agent, points at the missing binary, and includes the install hint sourced from \`agent.InstallScript()\`. The message lands on the prepare-step output (so the UI shows it inline) and on the session's error_message (so the orchestrator surfaces it on the task). Coverage: unit test for the error formatter (pure function, no SSH needed) + e2e regression that mv's mock-agent aside on the sshd container and asserts the surfaced error mentions agent name, binary, \$PATH, and the install hint. * feat(ssh): surface host/workdir/agentctl in the task executor popover Parity with the Docker and Sprites paths: when the user hovers the Executor Settings button on a task, an SSH-backed environment now shows the connection target (\`user@host[:port]\`), the remote task dir, a compact \`pid → remote :port → local :port\` summary of the agentctl process, the trimmed host-key fingerprint with copy, and a paste-ready \`ssh ...\` command that mirrors how kandev itself connects. Sourced via a parallel \`ssh\` block on GET /api/v1/tasks/:id/environment/live (no schema migration needed) — the service joins the task's latest ExecutorRunning row's metadata at fetch time, the same metadata SSH executor writes in CreateInstance. The probe is metadata-only on purpose: the popover polls every 3-7s and a per-poll SSH dial would be both slow and a lot of TCP traffic; live agentctl/connection health stays under the per-executor GetRemoteStatus path. shouldShowExecutorEnvironmentControls now allowlists \`ssh\` so the button actually renders for SSH tasks (Docker/Sprites/Remote-Docker were already in the allowlist; SSH was the only runtime that mounted a TaskEnvironment without showing the popover). Coverage: unit tests pinning buildSSHLiveStatus's projection of ExecutorRunning.Metadata into the popover-shaped struct (strings, string-encoded ints, native ints, empty, malformed) + e2e regression that launches an SSH task, opens the popover, and asserts each row. * feat(ssh): probe enabled agents on the remote from settings Adds a "Remote agent readiness" panel on /settings/executors/ssh/:id so users can diagnose "which agent CLIs are actually installed on this host" before launching a task. The same diagnosis that previously required a launch attempt + reading the pre-flight error now sits one button-click away on the executor's settings page. Backend: - New endpoint \`POST /api/v1/ssh/executors/:id/probe-agents\` dials SSH to the configured host and walks \`registry.ListEnabled()\`. For each agent it derives the binary from \`BuildCommand(Runtime=ssh).Args()[0]\` and runs \`command -v \` via the new exported \`lifecycle.ProbeRemoteBinary\` helper. Returns rows with availability, the resolved path on hit, and the agent's \`InstallScript()\` as a copy-pasteable hint on miss. - \`ssh.RegisterRoutes\` now accepts the task service (\`GetExecutor\`) and the agent registry; \`cmd/kandev/helpers.go\` wires both in. Frontend: - New \`SSHAgentReadinessCard\` mounted above the sessions card. Manual refresh only (each probe dials a fresh SSH connection — too expensive to poll). Each row gets a status badge + a copy button for the install hint when the agent is missing. Coverage: - E2E suite: API contract (mock-agent baked into /usr/local/bin/ → row reports installed, with /usr/local/bin/mock-agent as resolved_at; move it aside → row flips to missing with the install hint flowing through end-to-end) + UI contract (click "Probe agents", assert the table renders the row). * feat(ssh): upload remote-credentials over SFTP at launch time The SSH executor used to accept the \`remote_credentials\` profile field for parity with Sprites/Docker but only logged a "not implemented" warning at launch — anyone configuring Claude/Codex/Gemini auth against an SSH host had no way to actually get the credentials onto the remote. Ports the Sprites credential pipeline to SSH: - \`uploadCredentials\` orchestrates the same four phases Sprites does: resolve auth-secrets into env vars, run env-credential setup scripts on the remote, detect a local \`gh auth token\` if \`gh_cli_token\` was selected, and SFTP the selected file-type credentials into the remote user's \$HOME. - \`sshFileUploader\` satisfies \`lifecycle.FileUploader\` via SFTP with an \`mkdir -p\`-equivalent walk for parent dirs and \`chmod\` for mode bits. - \`runAuthSetupScripts\` shells the script through \`runSSHCommand\` with the agent's env vars prepended inline (so the script can read e.g. \`CLAUDE_CODE_OAUTH_TOKEN\` to materialize \`~/.claude/.credentials.json\`). - \`resolveRemoteAuthHomeDir\` probes \`printf %s "\$HOME"\` rather than reusing the task workdir — credentials need to land under the user's true home (\`~/.claude/...\`) even when \`workdir_root\` overrides things. Wired into \`maybeUploadCredentials\` so a credential failure is logged as a warning but doesn't abort the launch (consistent with Sprites). Frontend: \`useRemoteExecutorFlags\` on the profile-edit page now includes \`ssh\` in the \`isRemote\` set so \`RemoteCredentialsCard\` renders for SSH profiles. Previously SSH profiles silently dropped any \`remote_credentials\` selection because the card never rendered. End-to-end credential upload still requires a non-mock agent enabled in e2e mode (mock-agent's \`RemoteAuth\` is intentionally empty). Coverage: unit tests pin the SFTP \`parentDir\` walk and the shell-quoted env-prefix builder. Existing Sprites tests already exercise the shared \`UploadCredentialFiles\` helper. * fix(ssh): wrap remote commands in a login shell so nvm/asdf PATH applies sshd's exec channel runs commands via a non-interactive non-login shell. That doesn't source ~/.profile / ~/.bash_profile / ~/.bashrc, so any binary added to PATH by nvm/asdf/brew/etc. is invisible to: - the launch-time pre-flight probe (`command -v `) - the readiness panel's per-agent probe - and the nohup'd agentctl process itself End result: a user with `npx` installed through nvm sees 'X requires npx on the remote' and can't launch any node-based ACP agent. Fix: introduce `WrapLoginShell(shell, cmd)` that wraps cmd in `${shell} -lc ''` (with POSIX-safe single-quote escaping) and default shell to bash when empty. Thread the configured shell through ProbeRemoteBinary, startRemoteAgentctl, preflightAgentBinary, runOneAuthSetupScript, and resolveRemoteAuthHomeDir. The shell is read from per-profile metadata via the new MetadataKeySSHShell ('ssh_shell') key — flows in via the existing executor-config merge. The probe handler also accepts an optional {shell} body so the UI can re-probe under whichever shell the user picked. Default keeps existing behavior for users who don't set a shell (we just become a login bash instead of bare sh -c). Unit tests pin the quote-escaping contract. * feat(ssh): move readiness card to profile page + add shell picker and install button Four UX changes that together make the SSH-profile setup flow feel right: 1. Save redirect after creating a new SSH executor now lands on the profile-edit page (`/settings/executors/{profileId}`) instead of the executor page. Profiles are where users actually configure how the host gets used. 2. `SSHAgentReadinessCard` relocates from the executor page to the profile-edit page (top, conditional on `executor.type === 'ssh'`). The host-level executor page keeps Connection + Sessions. 3. Card gains a shell selector that auto-detects available shells on the remote and persists into `profile.config.shell`. Changing the shell clears the cached probe so the user re-probes under the new shell — PATH depends on which init files get sourced. 4. Each missing-agent row gets an Install button that runs the agent's InstallScript via /install-agent (login-shell wrapped). Spinner replaces the button while it runs; on completion the card auto re-probes. No confirmation dialog — the button is the consent. E2E: probe-shells reports bash+sh on the alpine image (zsh absent), install endpoint runs mock-agent's deterministic InstallScript, unknown agent_id is a 404, and the profile-page UI round-trip works end-to-end. * refactor(ssh): drop install-agent endpoint and Install button Install scripts assume environments we don't control on the remote (package managers, sudo, network access); the failure modes weren't recoverable from the UI. Pulling the one-click install — copy-and-paste hint stays. Backend: delete POST /api/v1/ssh/executors/:id/install-agent and everything that fed it (httpInstallAgent / installAgent / runInstallScript / runSSHCommandShim / InstallAgentRequest|Response / findAgentByID / joinOutputs / capBytes). No other caller of those helpers. Frontend: drop the Install button + spinner + collapsible output panel from ReadinessRow. The 'Install hint' column reverts to copy-only — same hint string sourced from agent.InstallScript(), users run it on the remote themselves. Remove installSSHAgent / SSHInstallAgentRequest / SSHInstallAgentResponse from the api client + types + e2e helpers. Survives: probe + readiness panel, shell selector + /probe-shells endpoint, login-shell wrap on every SSH command (the actual fix for nvm/asdf users). * fix(ssh): persist shell choice under the same key the launch path reads Profile config saved the shell selection as 'shell' but the SSH executor at launch time reads MetadataKeySSHShell which is 'ssh_shell'. buildLaunchMetadata copies profile.config keys verbatim into req.Metadata, so the mismatch meant sshShellFromMetadata always returned empty and WrapLoginShell silently fell back to bash — defeating the whole point of letting users pick zsh/sh/etc. The probe path bypasses metadata (the UI passes shell directly in the request body) which is why the readiness panel reported codex as installed under the user's zsh while the launch then failed with 'codex requires npx on the remote' under bash. Align both sides on 'ssh_shell'. * fix(ssh): propagate profile ssh_shell into launch metadata The settings UI saved the shell selection on profile.Config["ssh_shell"], but applyProfile in executor_state.go only forwards an allow-listed set of profile.Config keys (ssh_workdir_root, remote_credentials, etc.) into launch metadata — ssh_shell wasn't on the list. The launch path's sshShellFromMetadata therefore always returned empty and WrapLoginShell silently fell back to bash, so a user who installed npx via zsh+nvm hit 'npx not found in $PATH' from the preflight while the readiness probe (which takes shell straight from the request body) still showed the agent as available. Mirror the existing ssh_workdir_root propagation for ssh_shell and pin both directions (present + absent) with a unit test on resolveExecutorConfig. * fix(orchestrator): make profile.Config authoritative for ssh_workdir_root + ssh_shell A task that supplied ssh_workdir_root or ssh_shell in its metadata won when the profile had no value set — `applyProfile` only overwrote metadata when profile.Config had a non-empty value, so the task-supplied value squeezed through the empty-profile case. Both keys steer remote behavior (workdir target, login shell that runs every command), so a task author should never be able to redirect them. Two things, bundled because they touch the same allow-list: 1. Replace the long if-chain in applyProfile with a small declarative set of tables (passthrough / rename / authoritative). Behavior for the existing keys (remote_credentials, git_user_*, image_tag, …) is unchanged. 2. Add a new authoritative tier — ssh_workdir_root + ssh_shell — whose profile value ALWAYS wins, including when empty. The reader-side helpers (SSHExecutor.workdirRoot, sshShellFromMetadata) already handle the empty fall-through to a default. Pin both directions with subtests: authoritative keys clobber any task value (empty profile included); ordinary passthrough keys keep their existing "only set if non-empty" semantics so task-supplied git_user_name still survives. * refactor(ssh): pass remote workspace explicitly to createRemoteAgentInstance startAndForwardAgentctl was briefly mutating the caller's *ExecutorCreateRequest (overwriting req.WorkspacePath with the remote task dir, then restoring it after the create-instance POST). That's exactly the boring-and-direct sin — a callee reading from a struct the caller is mutating around the call. Pass the remote workspace path as an argument so the dance disappears and req stays read-only inside this leg. * fix(ssh): surface SHA read errors instead of swallowing them as "not cached" SSHCheckAgentctlCached treated any error from reading the remote sha256 sidecar as "binary not on remote" by returning (false, nil). A transient SSH transport failure therefore masqueraded as "needs_upload" in the test endpoint's UI badge — the user retries the test, gets the same misleading hint, and doesn't see that the connection itself is flaky. Append `|| true` to the remote cat so a missing file is a successful empty-stdout read (the genuine "not cached" path), and surface a real error to the caller for everything else. * fix(ssh): handle int/int64/float64 metadata in session listing intFromMetadata only accepted string values, which is what the SSH executor writes via strconv.Itoa — but ExecutorRunning.Metadata round-trips through JSON, and there's no guarantee a numeric value isn't unmarshalled back as float64 somewhere along the way. Anything that wasn't a string silently became 0 in the sessions endpoint, making the SSHSessionsCard's "Remote agentctl port" / "Local fwd port" columns render as 0 for any row that took a non-string path. Match the smarter shape already used in service.mdInt (internal/task/service/service_task_environments.go) so both readers agree on the projection. * refactor(e2e): drop duplicate SSH HTTP shapes module api-client-ssh.ts was a literal copy of apps/web/lib/types/http-ssh.ts with `Body` suffixed onto every type — 75 lines of pass-through that buy nothing. The e2e ApiClient now imports the canonical shapes from lib/types/http-ssh directly, drops the re-export pile at the bottom of api-client.ts, and the duplicate module is deleted. No spec referenced the *Body names; the grep is clean. * fix(ssh): make Content-Type win over caller-supplied headers in ssh-api In `{ "Content-Type": "application/json", ...headers }` the spread runs last, so a caller passing `init.headers = { "Content-Type": "x" }` would override the JSON content type and break server-side parsing — the exact opposite of what the file's comment claimed. The outer `init` spread is ordered correctly already; this fixes the inner `headers` literal in the same direction (spread first, JSON literal last) across all three SSH client functions. Per CodeRabbit thread. * fix(ssh): distinguish 404 from server errors when resolving SSH executor resolveSSHTarget collapsed every GetExecutor error into 404 — a transient storage outage, timeout, or canceled context made it look like the executor was deleted. Introduce models.ErrExecutorNotFound, wrap it in the SQLite repo's sql.ErrNoRows case, and use errors.Is in the SSH handler so only "row doesn't exist" maps to 404; everything else stays a 500 with the underlying error attached. Per CodeRabbit thread. * fix(ssh): surface latestRunningForTask repo errors instead of swallowing them latestRunningForTask returned (nil) on any error from ListTaskSessions or GetExecutorRunningBySessionID. Transient storage failures rendered as "no SSH live status" on every popover poll, indistinguishable from the genuine "task has no running session yet" case. Return the error to the caller; GetSSHLiveStatus now propagates it, and the handler already logs + omits the ssh block on a non-nil error so polling stays graceful. A session without an ExecutorRunning row is the one expected miss — match on models.ErrExecutorRunningNotFound and skip quietly. Per CodeRabbit thread. * fix(ssh): reset readiness card state when executorId changes The mount effect only reset the sequence token, so switching executors left the previous one's rows / error / hasProbed visible until the user clicked Refresh — reads as "still loading" while actually showing stale data. Clear setRows / setError / setHasProbed / setLoading in the same effect so a fresh executor starts blank. Per CodeRabbit thread. * fix(ssh): copy the full fingerprint, not the truncated display value The popover renders fingerprints as a hash suffix (e.g. ...abcdef012345) because the full SHA256 is verbose, but the copy button reused the displayed string — users got an unusable truncated fingerprint instead of the real one. Add an optional copyValue on FieldRow that overrides the clipboard payload, and wire the fingerprint row to it. Per CodeRabbit thread. * fix(ssh): keep port forwarder alive on transient Accept errors SSHPortForwarder.serve returned permanently on any non-orderly-close Accept error — including transient ones like EMFILE (process FD table exhausted) or ECONNABORTED. A single hiccup orphaned the forwarder, the remote agentctl kept running, and the user's session went dark until the backend was restarted. Match net.ErrClosed explicitly for the orderly-close path; for anything else, log a warning and continue the loop so the forwarder survives recoverable per-accept faults. Per claude[bot] review thread. * fix(ssh): guard ResumeRemoteInstance against duplicate attach CreateInstance had an early-out for the case where a previous resume already populated r.sessions[InstanceID], but ResumeRemoteInstance itself didn't — a retry would dial a second SSH connection, start a second forwarder, and overwrite the map entry. The original client + forwarder leaked until the SSH idle timeout reaped them, and the agent stream briefly bounced between the two. Mirror the guard so the two entry points to r.sessions[InstanceID] agree. Per claude[bot] review thread. * fix(ssh): pipe auth-setup env vars via stdin to keep secrets out of cmdline runOneAuthSetupScript inlined env vars as `KEY=value KEY=value -lc '...'`. That string is what becomes argv of the outer bash invoked by sshd, so on the remote host any process the same UID can read sees the secrets in `ps aux` / `/proc/PID/cmdline` for the brief window the setup script runs. Single-tenant deployments are unaffected in practice but the leak is real on shared hosts. Add a runSSHCommandStdin variant that feeds session.Stdin; replace the inline env prefix with a stdin payload of `KEY='quoted'` lines sourced by the wrapped shell under `set -a; . /dev/stdin; set +a`. Env never appears on the remote shell's argv; it's only visible in /proc/PID/environ of the setup process itself, which is the minimum surface for "pass an env var to a remote process". Replace buildSSHEnvPrefix with buildSSHEnvInitScript; update the test. Per claude[bot] review thread (greptile P1 finding). --- .github/workflows/e2e-tests.yml | 40 +- AGENTS.md | 1 + apps/backend/cmd/kandev/agents.go | 6 + apps/backend/cmd/kandev/helpers.go | 14 + apps/backend/go.mod | 13 +- apps/backend/go.sum | 25 +- apps/backend/internal/agent/agents/mock.go | 10 +- .../internal/agent/agents/mock_test.go | 12 +- .../internal/agent/executor/executor.go | 3 + .../runtime/lifecycle/agent_session_seeder.go | 9 +- .../runtime/lifecycle/env_preparer_ssh.go | 50 ++ .../runtime/lifecycle/executor_backend.go | 63 ++ .../agent/runtime/lifecycle/executor_ssh.go | 657 ++++++++++++++++ .../lifecycle/executor_ssh_connection.go | 557 +++++++++++++ .../lifecycle/executor_ssh_connection_test.go | 317 ++++++++ .../lifecycle/executor_ssh_credentials.go | 329 ++++++++ .../executor_ssh_credentials_test.go | 101 +++ .../lifecycle/executor_ssh_operations.go | 741 ++++++++++++++++++ .../agent/runtime/lifecycle/manager_launch.go | 36 + apps/backend/internal/agentruntime/runtime.go | 1 + .../executor/executor_credentials.go | 4 +- .../executor/executor_environment_reuse.go | 16 +- .../executor/executor_environment_test.go | 107 +++ .../executor/executor_mocks_test.go | 5 + .../orchestrator/executor/executor_state.go | 78 +- .../executor/executor_state_test.go | 139 ++++ apps/backend/internal/ssh/handlers.go | 682 ++++++++++++++++ .../handlers/executor_profile_handlers.go | 4 + .../handlers/task_environment_handlers.go | 11 + apps/backend/internal/task/models/models.go | 14 +- .../task/repository/sqlite/executor.go | 2 +- .../task/service/service_task_environments.go | 127 +++ .../service/service_task_environments_test.go | 67 ++ .../settings/executors/[profileId]/page.tsx | 52 +- .../executors/new/[type]/executor-types.ts | 42 + .../settings/executors/new/[type]/page.tsx | 38 +- .../executors/new/[type]/ssh-config.ts | 48 ++ .../executors/new/[type]/ssh-create-page.tsx | 111 +++ apps/web/app/settings/executors/page.tsx | 5 + .../executors/ssh/[executorId]/page.tsx | 199 +++++ .../settings/ssh-agent-readiness-card.tsx | 352 +++++++++ .../settings/ssh-connection-card.tsx | 606 ++++++++++++++ .../settings/ssh-fingerprint-trust-block.tsx | 62 ++ .../components/settings/ssh-sessions-card.tsx | 176 +++++ apps/web/components/settings/ssh-settings.tsx | 9 + .../task/executor-environment-info.tsx | 106 ++- .../task/executor-settings-button.tsx | 4 +- apps/web/components/task/task-top-bar.tsx | 1 + apps/web/e2e/README.md | 112 +++ apps/web/e2e/fixtures/backend.ts | 19 +- apps/web/e2e/fixtures/ssh-image.ts | 163 ++++ apps/web/e2e/fixtures/ssh-test-base.ts | 193 +++++ apps/web/e2e/helpers/api-client.ts | 59 ++ apps/web/e2e/helpers/ssh-bastion.ts | 281 +++++++ apps/web/e2e/helpers/ssh.ts | 364 +++++++++ apps/web/e2e/pages/SSHSettingsPage.ts | 189 +++++ apps/web/e2e/playwright.config.ts | 29 +- .../web/e2e/tests/ssh/agent-readiness.spec.ts | 117 +++ apps/web/e2e/tests/ssh/auth-methods.spec.ts | 103 +++ apps/web/e2e/tests/ssh/concurrency.spec.ts | 181 +++++ .../e2e/tests/ssh/config-inheritance.spec.ts | 112 +++ .../web/e2e/tests/ssh/connection-form.spec.ts | 81 ++ .../web/e2e/tests/ssh/error-surfacing.spec.ts | 161 ++++ apps/web/e2e/tests/ssh/executor-crud.spec.ts | 161 ++++ .../e2e/tests/ssh/hostkey-rotation.spec.ts | 152 ++++ apps/web/e2e/tests/ssh/launch-task.spec.ts | 201 +++++ apps/web/e2e/tests/ssh/persistence.spec.ts | 63 ++ apps/web/e2e/tests/ssh/popover.spec.ts | 63 ++ apps/web/e2e/tests/ssh/proxy-jump.spec.ts | 113 +++ apps/web/e2e/tests/ssh/recovery.spec.ts | 124 +++ apps/web/e2e/tests/ssh/sessions-card.spec.ts | 186 +++++ .../e2e/tests/ssh/sessions-endpoint.spec.ts | 154 ++++ apps/web/e2e/tests/ssh/test-endpoint.spec.ts | 181 +++++ apps/web/e2e/tests/ssh/test-result.spec.ts | 110 +++ apps/web/e2e/tests/ssh/trust-gate.spec.ts | 113 +++ .../e2e/tests/ssh/workdir-per-profile.spec.ts | 126 +++ .../domains/session/use-task-environment.ts | 17 +- apps/web/lib/api/domains/settings-api.ts | 38 + apps/web/lib/api/domains/ssh-api.ts | 74 ++ .../lib/api/domains/task-environment-api.ts | 12 + apps/web/lib/executor-icons.ts | 9 + apps/web/lib/types/executor.ts | 1 + apps/web/lib/types/http-ssh.ts | 69 ++ docs/specs/INDEX.md | 1 + docs/specs/ssh-executor/e2e-plan.md | 133 ++++ docs/specs/ssh-executor/spec.md | 168 ++++ 86 files changed, 10363 insertions(+), 122 deletions(-) create mode 100644 apps/backend/internal/agent/runtime/lifecycle/env_preparer_ssh.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go create mode 100644 apps/backend/internal/orchestrator/executor/executor_state_test.go create mode 100644 apps/backend/internal/ssh/handlers.go create mode 100644 apps/web/app/settings/executors/new/[type]/executor-types.ts create mode 100644 apps/web/app/settings/executors/new/[type]/ssh-config.ts create mode 100644 apps/web/app/settings/executors/new/[type]/ssh-create-page.tsx create mode 100644 apps/web/app/settings/executors/ssh/[executorId]/page.tsx create mode 100644 apps/web/components/settings/ssh-agent-readiness-card.tsx create mode 100644 apps/web/components/settings/ssh-connection-card.tsx create mode 100644 apps/web/components/settings/ssh-fingerprint-trust-block.tsx create mode 100644 apps/web/components/settings/ssh-sessions-card.tsx create mode 100644 apps/web/components/settings/ssh-settings.tsx create mode 100644 apps/web/e2e/README.md create mode 100644 apps/web/e2e/fixtures/ssh-image.ts create mode 100644 apps/web/e2e/fixtures/ssh-test-base.ts create mode 100644 apps/web/e2e/helpers/ssh-bastion.ts create mode 100644 apps/web/e2e/helpers/ssh.ts create mode 100644 apps/web/e2e/pages/SSHSettingsPage.ts create mode 100644 apps/web/e2e/tests/ssh/agent-readiness.spec.ts create mode 100644 apps/web/e2e/tests/ssh/auth-methods.spec.ts create mode 100644 apps/web/e2e/tests/ssh/concurrency.spec.ts create mode 100644 apps/web/e2e/tests/ssh/config-inheritance.spec.ts create mode 100644 apps/web/e2e/tests/ssh/connection-form.spec.ts create mode 100644 apps/web/e2e/tests/ssh/error-surfacing.spec.ts create mode 100644 apps/web/e2e/tests/ssh/executor-crud.spec.ts create mode 100644 apps/web/e2e/tests/ssh/hostkey-rotation.spec.ts create mode 100644 apps/web/e2e/tests/ssh/launch-task.spec.ts create mode 100644 apps/web/e2e/tests/ssh/persistence.spec.ts create mode 100644 apps/web/e2e/tests/ssh/popover.spec.ts create mode 100644 apps/web/e2e/tests/ssh/proxy-jump.spec.ts create mode 100644 apps/web/e2e/tests/ssh/recovery.spec.ts create mode 100644 apps/web/e2e/tests/ssh/sessions-card.spec.ts create mode 100644 apps/web/e2e/tests/ssh/sessions-endpoint.spec.ts create mode 100644 apps/web/e2e/tests/ssh/test-endpoint.spec.ts create mode 100644 apps/web/e2e/tests/ssh/test-result.spec.ts create mode 100644 apps/web/e2e/tests/ssh/trust-gate.spec.ts create mode 100644 apps/web/e2e/tests/ssh/workdir-per-profile.spec.ts create mode 100644 apps/web/lib/api/domains/ssh-api.ts create mode 100644 apps/web/lib/types/http-ssh.ts create mode 100644 docs/specs/ssh-executor/e2e-plan.md create mode 100644 docs/specs/ssh-executor/spec.md diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index b61380f0d2..85921e43ff 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -139,10 +139,11 @@ jobs: working-directory: apps/web run: tar -xf next-build.tar && rm next-build.tar - # Pinned to chromium + mobile-chrome so the dedicated `e2e-docker` job - # below owns the real-Docker project. Without --project the docker spec - # would run here too, fail seeding (KANDEV_DOCKER_ENABLED is off), and - # spam every shard. + # Pinned to chromium + mobile-chrome so the dedicated `e2e-containers` + # job below owns the real-Docker project (which covers both the Docker + # executor and the SSH executor's sshd target). Without --project the + # container-bound specs would run here too, fail seeding (Docker not + # enabled for these shards), and spam every shard. - name: Run E2E tests (shard ${{ matrix.shard }}/10) working-directory: apps/web run: npx playwright test --config e2e/playwright.config.ts --project=chromium --project=mobile-chrome --shard=${{ matrix.shard }}/10 @@ -169,11 +170,16 @@ jobs: # 6 runners — Playwright's --shard splits the project's tests into N groups. # Cleans up any leftover kandev-managed containers at the end so a leaky # process doesn't poison the next run on the same self-hosted runner. - e2e-docker: - name: E2E Docker Shard ${{ matrix.shard }}/6 + # + # This job runs the `containers` Playwright project, which covers both the + # Docker executor and the SSH executor (since the SSH executor's e2e tests + # also rely on Docker to host a real sshd container as the remote target). + # Sharded 6 ways — each shard is its own runner. See apps/web/e2e/README.md. + e2e-containers: + name: E2E Containers Shard ${{ matrix.shard }}/6 needs: build runs-on: ubuntu-latest - timeout-minutes: 10 + timeout-minutes: 20 strategy: fail-fast: false matrix: @@ -223,38 +229,38 @@ jobs: working-directory: apps/web run: tar -xf next-build.tar && rm next-build.tar - - name: Run Docker E2E tests (shard ${{ matrix.shard }}/6) + - name: Run container-backed E2E tests shard ${{ matrix.shard }}/6 (Docker executor + SSH executor) working-directory: apps/web env: - KANDEV_E2E_DOCKER: "1" - run: npx playwright test --config e2e/playwright.config.ts --project=docker --shard=${{ matrix.shard }}/6 + KANDEV_E2E_CONTAINERS: "1" + run: npx playwright test --config e2e/playwright.config.ts --project=containers --shard=${{ matrix.shard }}/6 - name: Cleanup leftover kandev containers if: always() run: docker ps -aq --filter label=kandev.managed=true | xargs -r docker rm -f # Named with the same `blob-report-*` prefix the merge job globs for - # so the docker results land in the unified HTML report alongside the - # sharded chromium/mobile-chrome runs. - - name: Upload Docker blob report + # so the containers results land in the unified HTML report alongside + # the sharded chromium/mobile-chrome runs. + - name: Upload containers blob report if: always() uses: actions/upload-artifact@v5 with: - name: blob-report-docker-${{ matrix.shard }} + name: blob-report-containers-${{ matrix.shard }} path: apps/web/e2e/blob-report/ retention-days: 3 - - name: Upload Docker E2E results + - name: Upload containers E2E results if: failure() uses: actions/upload-artifact@v5 with: - name: docker-e2e-test-results-${{ matrix.shard }} + name: containers-e2e-test-results-${{ matrix.shard }} path: apps/web/e2e/test-results/ retention-days: 7 e2e-report: name: Merge E2E Reports - needs: [e2e, e2e-docker] + needs: [e2e, e2e-containers] if: always() runs-on: ubuntu-latest timeout-minutes: 5 diff --git a/AGENTS.md b/AGENTS.md index c6d5949c51..df2e9ce236 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,7 @@ apps/ - **Backend**: Go with Make (`make -C apps/backend test|lint|build`) - **Frontend**: Next.js (`cd apps && pnpm --filter @kandev/web dev|lint|typecheck`) - **UI**: Shadcn components via `@kandev/ui` +- **E2E**: Playwright (`cd apps/web && pnpm e2e`). The `containers` project (gated on `KANDEV_E2E_CONTAINERS=1`, formerly `docker`) covers both the Docker executor and the SSH executor — anything that needs a real Docker daemon on the host lives there. See `apps/web/e2e/README.md`. - **GitHub repo**: `https://github.com/kdlbs/kandev` - **Container image**: `ghcr.io/kdlbs/kandev` (GitHub Container Registry) diff --git a/apps/backend/cmd/kandev/agents.go b/apps/backend/cmd/kandev/agents.go index 7d558d278f..5d6185ace8 100644 --- a/apps/backend/cmd/kandev/agents.go +++ b/apps/backend/cmd/kandev/agents.go @@ -76,6 +76,11 @@ func provideLifecycleManager( executorRegistry.Register(spritesExec) log.Info("Sprites runtime registered") + // Register SSH runtime (run an agent on any Linux box reachable over SSH). + sshExec := lifecycle.NewSSHExecutor(secretStore, agentRegistry, agentctlResolver, log) + executorRegistry.Register(sshExec) + log.Info("SSH runtime registered") + credsMgr := credentials.NewManager(log) if secretStore != nil { credsMgr.AddProvider(secrets.NewSecretStoreProvider(secretStore)) @@ -111,6 +116,7 @@ func provideLifecycleManager( preparerRegistry.Register(models.ExecutorTypeMockRemote, localPreparer) preparerRegistry.Register(models.ExecutorTypeLocalDocker, lifecycle.NewDockerPreparer(log)) preparerRegistry.Register(models.ExecutorTypeSprites, lifecycle.NewSpritesPreparer(log)) + preparerRegistry.Register(models.ExecutorTypeSSH, lifecycle.NewSSHPreparer(log)) lifecycleMgr.SetPreparerRegistry(preparerRegistry) lifecycleMgr.SetSecretStore(secretStore) // Wire the agent_profiles reader so the launch-prep skill deploy hook diff --git a/apps/backend/cmd/kandev/helpers.go b/apps/backend/cmd/kandev/helpers.go index 83b6c0c8fd..bb5920b2eb 100644 --- a/apps/backend/cmd/kandev/helpers.go +++ b/apps/backend/cmd/kandev/helpers.go @@ -61,6 +61,7 @@ import ( "github.com/kandev/kandev/internal/secrets" "github.com/kandev/kandev/internal/slack" spriteshandlers "github.com/kandev/kandev/internal/sprites" + sshhandlers "github.com/kandev/kandev/internal/ssh" systemsvc "github.com/kandev/kandev/internal/system" taskhandlers "github.com/kandev/kandev/internal/task/handlers" "github.com/kandev/kandev/internal/task/models" @@ -712,6 +713,19 @@ func registerSecondaryRoutes( p.log.Debug("Registered Sprites handlers (HTTP + WebSocket)") } + if p.taskRepo != nil { + sshhandlers.RegisterRoutes( + p.router, + p.gateway.Dispatcher, + p.taskRepo, + p.services.Task, + p.agentRegistry, + lifecycle.NewAgentctlResolver(p.log), + p.log, + ) + p.log.Debug("Registered SSH handlers (HTTP + WebSocket)") + } + if p.services.GitHub != nil { github.RegisterRoutes(p.router, p.gateway.Dispatcher, p.services.GitHub, p.log) github.RegisterMockRoutes(p.router, p.services.GitHub, p.log) diff --git a/apps/backend/go.mod b/apps/backend/go.mod index 5edb60e8d9..20cfb6508e 100644 --- a/apps/backend/go.mod +++ b/apps/backend/go.mod @@ -15,9 +15,11 @@ require ( github.com/gorilla/websocket v1.5.3 github.com/jackc/pgx/v5 v5.9.0 github.com/jmoiron/sqlx v1.4.0 + github.com/kevinburke/ssh_config v1.6.0 github.com/mark3labs/mcp-go v0.43.2 github.com/mattn/go-sqlite3 v1.14.33 github.com/nats-io/nats.go v1.31.0 + github.com/pkg/sftp v1.13.10 github.com/spf13/viper v1.16.0 github.com/stretchr/testify v1.11.1 github.com/superfly/sprites-go v0.0.0-20260206213632-8176adff485b @@ -27,9 +29,10 @@ require ( go.opentelemetry.io/otel/sdk v1.43.0 go.opentelemetry.io/otel/trace v1.43.0 go.uber.org/zap v1.26.0 - golang.org/x/net v0.52.0 + golang.org/x/crypto v0.51.0 + golang.org/x/net v0.53.0 golang.org/x/sync v0.20.0 - golang.org/x/sys v0.42.0 + golang.org/x/sys v0.44.0 gopkg.in/natefinch/lumberjack.v2 v2.2.1 gopkg.in/yaml.v3 v3.0.1 ) @@ -68,6 +71,7 @@ require ( github.com/json-iterator/go v1.1.12 // indirect github.com/klauspost/compress v1.17.0 // indirect github.com/klauspost/cpuid/v2 v2.2.4 // indirect + github.com/kr/fs v0.1.0 // indirect github.com/leodido/go-urn v1.2.4 // indirect github.com/magiconair/properties v1.8.7 // indirect github.com/mailru/easyjson v0.7.7 // indirect @@ -102,9 +106,8 @@ require ( go.opentelemetry.io/proto/otlp v1.10.0 // indirect go.uber.org/multierr v1.10.0 // indirect golang.org/x/arch v0.3.0 // indirect - golang.org/x/crypto v0.49.0 // indirect - golang.org/x/term v0.41.0 // indirect - golang.org/x/text v0.35.0 // indirect + golang.org/x/term v0.43.0 // indirect + golang.org/x/text v0.37.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 // indirect google.golang.org/grpc v1.80.0 // indirect diff --git a/apps/backend/go.sum b/apps/backend/go.sum index 143b551178..41dbf8b8a9 100644 --- a/apps/backend/go.sum +++ b/apps/backend/go.sum @@ -221,12 +221,15 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk= +github.com/kevinburke/ssh_config v1.6.0 h1:J1FBfmuVosPHf5GRdltRLhPJtJpTlMdKTBjRgTaQBFY= +github.com/kevinburke/ssh_config v1.6.0/go.mod h1:q2RIzfka+BXARoNexmF9gkxEX7DmvbW9P4hIVx2Kg4M= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.17.0 h1:Rnbp4K9EjcDuVuHtd0dgA4qNuv9yKDYKK1ulpJwgrqM= github.com/klauspost/compress v1.17.0/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.2.4 h1:acbojRNwl3o09bUq+yDCtZFc1aiwaAAxtcn8YkZXnvk= github.com/klauspost/cpuid/v2 v2.2.4/go.mod h1:RVVoqg1df56z8g3pUjL/3lE5UfnlrJX8tyFgg4nqhuY= +github.com/kr/fs v0.1.0 h1:Jskdu9ieNAYnjxsi0LbQp1ulIKZV1LAFgK1tWhpZgl8= github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= @@ -282,6 +285,8 @@ github.com/pelletier/go-toml/v2 v2.0.8/go.mod h1:vuYfssBdrU2XDZ9bYydBu6t+6a6PYNc github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/sftp v1.13.1/go.mod h1:3HaPG6Dq1ILlpPZRO0HVMrsydcdLt6HRDccSgb87qRg= +github.com/pkg/sftp v1.13.10 h1:+5FbKNTe5Z9aspU88DPIKJ9z2KZoaGCu6Sr6kKR/5mU= +github.com/pkg/sftp v1.13.10/go.mod h1:bJ1a7uDhrX/4OII+agvy28lzRvQrmIQuaHrcI1HbeGA= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= @@ -376,8 +381,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= -golang.org/x/crypto v0.49.0 h1:+Ng2ULVvLHnJ/ZFEq4KdcDd/cfjrrjjNSXNzxg0Y4U4= -golang.org/x/crypto v0.49.0/go.mod h1:ErX4dUh2UM+CFYiXZRTcMpEcN8b/1gxEuv3nODoYtCA= +golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= +golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= @@ -443,8 +448,8 @@ golang.org/x/net v0.0.0-20201209123823-ac852fbbde11/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.52.0 h1:He/TN1l0e4mmR3QqHMT2Xab3Aj3L9qjbhRm78/6jrW0= -golang.org/x/net v0.52.0/go.mod h1:R1MAz7uMZxVMualyPXb+VaqGSa3LIaUqk0eEt3w36Sw= +golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= +golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -507,11 +512,11 @@ golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= -golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= +golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.41.0 h1:QCgPso/Q3RTJx2Th4bDLqML4W6iJiaXFq2/ftQF13YU= -golang.org/x/term v0.41.0/go.mod h1:3pfBgksrReYfZ5lvYM0kSO0LIkAl4Yl2bXOkKP7Ec2A= +golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= +golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -520,8 +525,8 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= -golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= +golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= +golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= diff --git a/apps/backend/internal/agent/agents/mock.go b/apps/backend/internal/agent/agents/mock.go index 3e77cb5ea5..ec4d8a8b9d 100644 --- a/apps/backend/internal/agent/agents/mock.go +++ b/apps/backend/internal/agent/agents/mock.go @@ -6,6 +6,7 @@ import ( "time" "github.com/kandev/kandev/internal/agent/usage" + "github.com/kandev/kandev/internal/agentruntime" "github.com/kandev/kandev/pkg/agent" ) @@ -139,12 +140,19 @@ func (a *MockAgent) BuildCommand(opts CommandOptions) Command { // has /usr/local/bin on PATH and container.go bind-mounts the // linux/amd64 mock-agent there, so the bare name resolves correctly. // + // SSH is the same shape: the agent runs on a remote host whose + // filesystem doesn't share paths with the kandev backend's, so the + // host-absolute binary path can't apply. The SSH e2e image bakes + // mock-agent at /usr/local/bin/mock-agent; production SSH expects + // users to install agents into the remote's $PATH (real agents like + // Claude use `npx` and never hit this branch). + // // For host runtimes (standalone) we prefer the absolute host path // that configureMockAgent computed via os.Executable() — that's the // only way the host path works in dev mode (where the shell's $PATH // doesn't include apps/backend/bin). Fall back to the bare name when // no path is set: e2e fixtures prepend the kandev bin dir to PATH. - if opts.Runtime.IsContainerized() { + if opts.Runtime.IsContainerized() || opts.Runtime == agentruntime.RuntimeSSH { return Cmd(mockAgentDefaultID).Build() } binary := mockAgentDefaultID diff --git a/apps/backend/internal/agent/agents/mock_test.go b/apps/backend/internal/agent/agents/mock_test.go index a52fe5c050..20c6c5be7e 100644 --- a/apps/backend/internal/agent/agents/mock_test.go +++ b/apps/backend/internal/agent/agents/mock_test.go @@ -36,11 +36,12 @@ func TestMockAgent_BuildCommand_NoResumeFlag(t *testing.T) { // TestMockAgent_BuildCommand_ContainerizedUsesBareName pins the docker // branch of BuildCommand: when opts.Runtime is containerized (docker, -// remote_docker, sprites), MockAgent must emit the bare binary name so -// PATH lookup inside the container resolves to the bind-mounted -// linux/amd64 mock-agent at /usr/local/bin. Regressing this would -// cause docker e2e to fail with `exec: "": not found`, -// which is exactly the bug commit `8518f65` fixed. +// remote_docker, sprites) or SSH, MockAgent must emit the bare binary +// name so PATH lookup on the remote resolves to the bind-mounted (or +// pre-baked) linux/amd64 mock-agent at /usr/local/bin. Regressing this +// would cause docker e2e to fail with `exec: "": not +// found` (the bug commit `8518f65` fixed) and SSH e2e with the same +// symptom on the remote host. func TestMockAgent_BuildCommand_ContainerizedUsesBareName(t *testing.T) { a := NewMockAgent() a.SetBinaryPath("/Users/dev/kandev/apps/backend/bin/mock-agent") @@ -48,6 +49,7 @@ func TestMockAgent_BuildCommand_ContainerizedUsesBareName(t *testing.T) { agentruntime.RuntimeDocker, agentruntime.RuntimeRemoteDocker, agentruntime.RuntimeSprites, + agentruntime.RuntimeSSH, } { t.Run(string(rt), func(t *testing.T) { cmd := a.BuildCommand(CommandOptions{Runtime: rt}) diff --git a/apps/backend/internal/agent/executor/executor.go b/apps/backend/internal/agent/executor/executor.go index 3f904cc288..44b566c35c 100644 --- a/apps/backend/internal/agent/executor/executor.go +++ b/apps/backend/internal/agent/executor/executor.go @@ -18,6 +18,7 @@ const ( NameLocal Name = "local" NameRemoteDocker = agentruntime.RuntimeRemoteDocker NameSprites = agentruntime.RuntimeSprites + NameSSH = agentruntime.RuntimeSSH ) // ExecutorTypeToBackend maps an ExecutorType to its corresponding executor Name. @@ -33,6 +34,8 @@ func ExecutorTypeToBackend(execType models.ExecutorType) Name { return NameRemoteDocker case models.ExecutorTypeSprites: return NameSprites + case models.ExecutorTypeSSH: + return NameSSH case models.ExecutorTypeMockRemote: return NameStandalone default: diff --git a/apps/backend/internal/agent/runtime/lifecycle/agent_session_seeder.go b/apps/backend/internal/agent/runtime/lifecycle/agent_session_seeder.go index 6336937d0c..24697e19ac 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/agent_session_seeder.go +++ b/apps/backend/internal/agent/runtime/lifecycle/agent_session_seeder.go @@ -20,8 +20,13 @@ import ( const agentSessionsRootSubdir = "agent-sessions" // authMethodTypeFiles selects RemoteAuthMethod entries that copy local files -// into the remote environment (vs env-var-based auth). -const authMethodTypeFiles = "files" +// into the remote environment, while authMethodTypeEnv covers env-var-based +// auth (with an optional SetupScript that runs on the remote to provision +// credential files from the env var). +const ( + authMethodTypeFiles = "files" + authMethodTypeEnv = "env" +) // AgentSessionsRoot returns the directory under the kandev home where // per-container session dirs are created (e.g. ~/.kandev/agent-sessions/). diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_ssh.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_ssh.go new file mode 100644 index 0000000000..fb1541c45a --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_ssh.go @@ -0,0 +1,50 @@ +package lifecycle + +import ( + "context" + "time" + + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" +) + +// SSHPreparer validates prerequisites for SSH-based execution. The heavy +// lifting (connect, agentctl upload, task dir provisioning, agentctl launch) +// happens in SSHExecutor.CreateInstance — this preparer just emits a single +// validation step so the launch UI has consistent progress feedback. +type SSHPreparer struct { + logger *logger.Logger +} + +// NewSSHPreparer creates a new SSHPreparer. +func NewSSHPreparer(log *logger.Logger) *SSHPreparer { + return &SSHPreparer{logger: log.WithFields(zap.String("component", "ssh-preparer"))} +} + +// Name implements EnvironmentPreparer. +func (p *SSHPreparer) Name() string { return "ssh" } + +// Prepare implements EnvironmentPreparer. SSH executor configuration is +// validated at launch time (target / fingerprint / arch are checked inside +// CreateInstance). This step is intentionally minimal so the UI still sees +// the preparer running for every launch — preserves a uniform progress shape +// across executor types. +func (p *SSHPreparer) Prepare(_ context.Context, req *EnvPrepareRequest, onProgress PrepareProgressCallback) (*EnvPrepareResult, error) { + p.logger.Debug("preparing ssh environment", + zap.String("task", req.TaskID), + zap.String("session", req.SessionID)) + + started := time.Now() + step := beginStep("Validate SSH executor configuration") + reportProgress(onProgress, step, 0, 1) + completeStepSuccess(&step) + reportProgress(onProgress, step, 1, 1) + return &EnvPrepareResult{ + Success: true, + Steps: []PrepareStep{step}, + WorkspacePath: req.WorkspacePath, + Duration: time.Since(started), + WorktreeBranch: nonWorktreeTaskBranch(req), + }, nil +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_backend.go b/apps/backend/internal/agent/runtime/lifecycle/executor_backend.go index a74338f455..3da84b69cf 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_backend.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_backend.go @@ -82,6 +82,30 @@ const ( // Office metadata keys MetadataKeySkillManifestJSON = "skill_manifest_json" + + // SSH runtime metadata keys (per-session, except SSHWorkdirRoot which is per-profile). + MetadataKeySSHHostAlias = "ssh_host_alias" + MetadataKeySSHHost = "ssh_host" + MetadataKeySSHPort = "ssh_port" + MetadataKeySSHUser = "ssh_user" + MetadataKeySSHHostFingerprint = "ssh_host_fingerprint" + MetadataKeySSHRemoteTaskDir = "ssh_remote_task_dir" + MetadataKeySSHRemoteSessionDir = "ssh_remote_session_dir" + MetadataKeySSHRemoteAgentctlPort = "ssh_remote_agentctl_port" + MetadataKeySSHRemoteAgentctlPID = "ssh_remote_agentctl_pid" + MetadataKeySSHLocalForwardPort = "ssh_local_forward_port" + MetadataKeySSHRemoteAgentctlURL = "ssh_remote_agentctl_url" + MetadataKeySSHWorkdirRoot = "ssh_workdir_root" + MetadataKeySSHProxyJump = "ssh_proxy_jump" + MetadataKeySSHIdentitySource = "ssh_identity_source" + MetadataKeySSHIdentityFile = "ssh_identity_file" + // MetadataKeySSHShell names the login shell used when running commands + // over SSH on the remote (probe, agentctl launch, install, setup + // scripts). Empty / unset falls back to "bash" at runtime — see + // WrapLoginShell. Stored per-profile so different profiles on the same + // host can use different shells; flows into req.Metadata via the + // standard executor-config merge in buildLaunchMetadata. + MetadataKeySSHShell = "ssh_shell" ) // persistentMetadataKeys lists metadata keys carried forward from a previous @@ -95,6 +119,23 @@ var persistentMetadataKeys = map[string]bool{ MetadataKeySpriteCreatedAt: true, MetadataKeyLocalPort: true, + // SSH runtime + MetadataKeySSHHost: true, + MetadataKeySSHPort: true, + MetadataKeySSHUser: true, + MetadataKeySSHHostFingerprint: true, + MetadataKeySSHRemoteTaskDir: true, + MetadataKeySSHRemoteSessionDir: true, + MetadataKeySSHRemoteAgentctlPort: true, + MetadataKeySSHRemoteAgentctlPID: true, + MetadataKeySSHLocalForwardPort: true, + MetadataKeySSHRemoteAgentctlURL: true, + MetadataKeySSHWorkdirRoot: true, + MetadataKeySSHProxyJump: true, + MetadataKeySSHIdentitySource: true, + MetadataKeySSHIdentityFile: true, + MetadataKeySSHShell: true, + // Executor type marker MetadataKeyIsRemote: true, @@ -120,6 +161,21 @@ var persistentMetadataPrefixes = []string{ "env_secret_id_", // Secret store UUIDs for profile env vars } +// sessionScopedMetadataKeys lists metadata keys that point to per-session +// runtime resources (process PIDs, allocated ports, session directories on +// the remote). These keys ARE persisted across a SAME-session resume — that's +// how a backend restart reattaches to a still-running remote agent — but they +// MUST NOT be carried across SIBLING sessions on the same task. If they were, +// the second session would try to attach to the first session's agentctl +// process and end up sharing its ACP session and instance port. +var sessionScopedMetadataKeys = map[string]bool{ + MetadataKeySSHRemoteSessionDir: true, + MetadataKeySSHRemoteAgentctlPort: true, + MetadataKeySSHRemoteAgentctlPID: true, + MetadataKeySSHLocalForwardPort: true, + MetadataKeySSHRemoteAgentctlURL: true, +} + // ShouldPersistMetadataKey returns true if the given metadata key should // be carried forward when resuming a session from an ExecutorRunning record. func ShouldPersistMetadataKey(key string) bool { @@ -134,6 +190,13 @@ func ShouldPersistMetadataKey(key string) bool { return false } +// IsSessionScopedMetadataKey reports whether key references per-session +// runtime resources that must not leak across sibling sessions on the same +// task environment. +func IsSessionScopedMetadataKey(key string) bool { + return sessionScopedMetadataKeys[key] +} + // FilterPersistentMetadata returns a copy of src containing only keys that // should be carried forward across session resumes. Returns nil if no keys match. func FilterPersistentMetadata(src map[string]interface{}) map[string]interface{} { diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go new file mode 100644 index 0000000000..b2469bd8c3 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go @@ -0,0 +1,657 @@ +package lifecycle + +import ( + "context" + "errors" + "fmt" + "strconv" + "strings" + "sync" + "time" + + "go.uber.org/zap" + "golang.org/x/crypto/ssh" + + "github.com/kandev/kandev/internal/agent/agents" + "github.com/kandev/kandev/internal/agent/executor" + agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" + "github.com/kandev/kandev/internal/agentctl/server/process" + "github.com/kandev/kandev/internal/agentruntime" + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/secrets" +) + +const ( + sshAgentctlHealthTimeout = 20 * time.Second + + sshStatusRunning = "running" + sshStatusUnknown = "unknown" + sshStatusDisconnected = "disconnected" + sshStatusAgentctlDown = "agentctl-down" +) + +// sshSessionState tracks per-session resources we need to clean up later: +// the SSH target (so we can dial again on Stop), the per-session SSH client +// (no shared pool — the Go SSH client's direct-tcpip mux gets unreliable +// when many short-lived channels race the long-lived stream channels), and +// the local port forwarder. +type sshSessionState struct { + target *SSHTarget + client *ssh.Client + forwarder *SSHPortForwarder + pid int + remoteDir string +} + +// SSHExecutor implements ExecutorBackend for SSH-reachable Linux hosts. +// +// Each session owns its own *ssh.Client (no shared pool). One SSH connection +// per session keeps teardown simple — closing the executor instance closes +// the client — at the cost of an extra TCP+handshake per session on the same +// host. See docs/specs/ssh-executor/spec.md for the full design. +type SSHExecutor struct { + agentctlResolver *AgentctlResolver + secretStore secrets.SecretStore + agentList RemoteAuthAgentLister + logger *logger.Logger + + mu sync.Mutex + sessions map[string]*sshSessionState // keyed by ExecutorInstance.InstanceID +} + +// NewSSHExecutor wires up an SSH executor with shared infrastructure. +// agentList is optional; pass nil if no agent install scripts are needed. +func NewSSHExecutor( + secretStore secrets.SecretStore, + agentList RemoteAuthAgentLister, + resolver *AgentctlResolver, + log *logger.Logger, +) *SSHExecutor { + return &SSHExecutor{ + agentctlResolver: resolver, + secretStore: secretStore, + agentList: agentList, + logger: log.WithFields(zap.String("runtime", "ssh")), + sessions: make(map[string]*sshSessionState), + } +} + +func (r *SSHExecutor) Name() executor.Name { return executor.NameSSH } + +func (r *SSHExecutor) HealthCheck(_ context.Context) error { + // SSH targets are configured per-executor — the runtime itself is always + // available. Per-host reachability is verified by the test-connection + // endpoint and surfaced in the executor status panel. + return nil +} + +// Close terminates every still-tracked SSH session. Normal teardown happens +// session-by-session via StopInstance; Close is the shutdown safety net for +// sessions whose StopInstance didn't run (e.g. a hard kandev exit). +func (r *SSHExecutor) Close() error { + r.mu.Lock() + states := make([]*sshSessionState, 0, len(r.sessions)) + for id, s := range r.sessions { + states = append(states, s) + delete(r.sessions, id) + } + r.mu.Unlock() + for _, s := range states { + if s.forwarder != nil { + _ = s.forwarder.Close() + } + if s.client != nil { + _ = s.client.Close() + } + } + return nil +} + +// targetFromMetadata builds an SSHTarget from the executor metadata +// propagated via buildLaunchMetadata (req.ExecutorConfig keys merged in). +func (r *SSHExecutor) targetFromMetadata(md map[string]interface{}) (*SSHTarget, error) { + host := getMetadataString(md, MetadataKeySSHHost) + hostAlias := getMetadataString(md, MetadataKeySSHHostAlias) + if host == "" && hostAlias == "" { + return nil, errors.New("ssh executor: host (or host_alias) is required in executor config") + } + port := 0 + if p := getMetadataString(md, MetadataKeySSHPort); p != "" { + // Reject malformed / out-of-range ports loudly instead of silently + // falling back to 22 — that fallback would mask an obviously bad + // config (e.g. ssh_port="abc" or ssh_port="0") and route every + // launch through a port the user never meant to use. + n, err := strconv.Atoi(p) + if err != nil || n < 1 || n > 65535 { + return nil, fmt.Errorf("ssh executor: invalid ssh_port %q (must be 1-65535)", p) + } + port = n + } + identitySource := SSHIdentitySource(getMetadataString(md, MetadataKeySSHIdentitySource)) + identityFile := getMetadataString(md, MetadataKeySSHIdentityFile) + cfg := SSHConnConfig{ + HostAlias: hostAlias, + Host: host, + Port: port, + User: getMetadataString(md, MetadataKeySSHUser), + IdentitySource: identitySource, + IdentityFile: identityFile, + ProxyJump: getMetadataString(md, MetadataKeySSHProxyJump), + PinnedFingerprint: getMetadataString(md, MetadataKeySSHHostFingerprint), + } + if cfg.PinnedFingerprint == "" { + return nil, errors.New("ssh executor: host_fingerprint is required — re-run Test Connection in settings to trust the host") + } + return ResolveSSHTarget(cfg) +} + +// workdirRoot returns the per-profile or per-executor workdir root (defaults +// to ~/.kandev). Per-profile wins over per-executor. +func (r *SSHExecutor) workdirRoot(md map[string]interface{}) string { + if w := getMetadataString(md, MetadataKeySSHWorkdirRoot); w != "" { + return w + } + return sshDefaultWorkdir +} + +// CreateInstance opens (or reuses) an SSH connection to the configured host, +// ensures agentctl is uploaded, provisions the per-task remote dir, launches +// a per-session agentctl process, and sets up a local port forward so the +// kandev backend can speak to it as if it were on localhost. +// +// If ResumeRemoteInstance already attached this InstanceID (e.g. after a +// backend restart), reuse the resumed SSH client + forwarder + remote pid +// instead of starting a second remote agentctl on top of the live one. +func (r *SSHExecutor) CreateInstance(ctx context.Context, req *ExecutorCreateRequest) (*ExecutorInstance, error) { + r.mu.Lock() + resumed, ok := r.sessions[req.InstanceID] + r.mu.Unlock() + if ok { + return r.buildResumedInstance(req, resumed), nil + } + + target, err := r.targetFromMetadata(req.Metadata) + if err != nil { + return nil, err + } + client, err := dialSSH(ctx, target) + if err != nil { + return nil, fmt.Errorf("ssh: connect to %s@%s: %w", target.User, target.Host, err) + } + released := false + defer func() { + if !released { + _ = client.Close() + } + }() + r.report(req.OnProgress, "Connecting to SSH host", PrepareStepCompleted, "") + + agentctlBin, err := r.prepareRemoteHost(ctx, client, req) + if err != nil { + return nil, err + } + + if err := r.preflightAgentBinary(ctx, client, req); err != nil { + return nil, err + } + + workdir := r.workdirRoot(req.Metadata) + taskDir, sessionDir, err := r.prepareRemoteDirs(ctx, client, workdir, req) + if err != nil { + return nil, err + } + r.maybeUploadCredentials(ctx, client, req) + + port, pid, fwd, err := r.startAndForwardAgentctl(ctx, client, agentctlBin, taskDir, sessionDir, req) + if err != nil { + return nil, err + } + + r.mu.Lock() + r.sessions[req.InstanceID] = &sshSessionState{ + target: target, + client: client, + forwarder: fwd, + pid: pid, + remoteDir: sessionDir, + } + r.mu.Unlock() + released = true // ownership transferred to session state; released on StopInstance + + return r.buildInstance(req, target, fwd, taskDir, sessionDir, port, pid, workdir), nil +} + +// prepareRemoteHost runs the steps that are independent of any particular +// task: detect remote OS/arch and ensure the agentctl binary is on the host. +// Returns the resolved remote agentctl path. +func (r *SSHExecutor) prepareRemoteHost(ctx context.Context, client *ssh.Client, req *ExecutorCreateRequest) (string, error) { + info, err := detectRemoteInfo(ctx, client) + if err != nil { + r.report(req.OnProgress, "Detecting remote OS", PrepareStepFailed, err.Error()) + return "", fmt.Errorf("ssh: detect remote: %w", err) + } + if err := requireSupportedArch(info.Arch); err != nil { + r.report(req.OnProgress, "Detecting remote OS", PrepareStepFailed, err.Error()) + return "", err + } + r.report(req.OnProgress, "Detecting remote OS", PrepareStepCompleted, info.UnameAll) + + agentctlBin, err := ensureAgentctlOnHost(ctx, client, r.agentctlResolver, r.logger) + if err != nil { + r.report(req.OnProgress, "Uploading agent controller", PrepareStepFailed, err.Error()) + return "", err + } + r.report(req.OnProgress, "Uploading agent controller", PrepareStepCompleted, "") + return agentctlBin, nil +} + +// prepareRemoteDirs makes /tasks/ and /.kandev/sessions/. +func (r *SSHExecutor) prepareRemoteDirs(ctx context.Context, client *ssh.Client, workdir string, req *ExecutorCreateRequest) (string, string, error) { + taskDir, err := ensureRemoteTaskDir(ctx, client, workdir, sshTaskDirName(req)) + if err != nil { + r.report(req.OnProgress, "Preparing task directory", PrepareStepFailed, err.Error()) + return "", "", err + } + sessionDir, err := ensureRemoteSessionDir(ctx, client, taskDir, req.SessionID) + if err != nil { + r.report(req.OnProgress, "Preparing task directory", PrepareStepFailed, err.Error()) + return "", "", err + } + r.report(req.OnProgress, "Preparing task directory", PrepareStepCompleted, taskDir) + return taskDir, sessionDir, nil +} + +// startAndForwardAgentctl spawns the per-session agentctl on the remote, +// creates a per-instance sub-server inside it, and stands up the SSH local +// port forward to the *instance* port (not the control port). Returns the +// instance port, the agentctl PID, and the forwarder. Cleans up everything +// it created on any failure. +// +// Lifecycle mirrors what executor_sprites.go does inside its sprite: +// 1. agentctl --workdir binds the control API on a random port. +// 2. POST /api/v1/instances on that control port creates a session-scoped +// sub-server with its own port (allocated from agentctl's instance pool). +// 3. The SSH executor's clients (agent stream, workspace, etc.) all talk to +// that sub-server, so the local port forward points there. +func (r *SSHExecutor) startAndForwardAgentctl( + ctx context.Context, client *ssh.Client, agentctlBin, taskDir, sessionDir string, req *ExecutorCreateRequest, +) (int, int, *SSHPortForwarder, error) { + shell := sshShellFromMetadata(req.Metadata) + controlPort, pid, err := startRemoteAgentctl(ctx, client, shell, agentctlBin, taskDir, sessionDir, r.logger) + if err != nil { + r.report(req.OnProgress, "Starting agent controller", PrepareStepFailed, err.Error()) + return 0, 0, nil, err + } + r.report(req.OnProgress, "Starting agent controller", PrepareStepCompleted, + fmt.Sprintf("pid=%d control_port=%d", pid, controlPort)) + + // The per-instance server's workspace is the remote task dir, not the + // host-side req.WorkspacePath (which is meaningless on the remote). + // Passed explicitly so we don't briefly mutate the caller's request. + instancePort, ierr := createRemoteAgentInstance(ctx, client, controlPort, taskDir, req, r.logger) + if ierr != nil { + _ = stopRemoteAgentctl(ctx, client, sessionDir, pid) + r.report(req.OnProgress, "Creating agent instance", PrepareStepFailed, ierr.Error()) + return 0, 0, nil, ierr + } + r.report(req.OnProgress, "Creating agent instance", PrepareStepCompleted, + fmt.Sprintf("instance_port=%d", instancePort)) + + fwd, err := StartPortForward(client, instancePort, r.logger) + if err != nil { + _ = stopRemoteAgentctl(ctx, client, sessionDir, pid) + return 0, 0, nil, fmt.Errorf("ssh: port forward: %w", err) + } + if err := waitAgentctlHealthy(ctx, fwd.LocalPort(), sshAgentctlHealthTimeout); err != nil { + _ = fwd.Close() + _ = stopRemoteAgentctl(ctx, client, sessionDir, pid) + return 0, 0, nil, fmt.Errorf("ssh: agentctl health: %w", err) + } + r.report(req.OnProgress, "Connecting to agent controller", PrepareStepCompleted, + fmt.Sprintf("local:%d -> remote:%d", fwd.LocalPort(), instancePort)) + return instancePort, pid, fwd, nil +} + +func (r *SSHExecutor) buildInstance( + req *ExecutorCreateRequest, + target *SSHTarget, + fwd *SSHPortForwarder, + taskDir, sessionDir string, + port, pid int, + workdir string, +) *ExecutorInstance { + return &ExecutorInstance{ + InstanceID: req.InstanceID, + TaskID: req.TaskID, + SessionID: req.SessionID, + RuntimeName: r.Name(), + Client: agentctl.NewClient("127.0.0.1", fwd.LocalPort(), r.logger, + agentctl.WithExecutionID(req.InstanceID), + agentctl.WithSessionID(req.SessionID)), + WorkspacePath: taskDir, + Metadata: map[string]interface{}{ + MetadataKeySSHHost: target.Host, + MetadataKeySSHPort: strconv.Itoa(target.Port), + MetadataKeySSHUser: target.User, + MetadataKeySSHHostFingerprint: target.PinnedFingerprint, + MetadataKeySSHRemoteTaskDir: taskDir, + MetadataKeySSHRemoteSessionDir: sessionDir, + MetadataKeySSHRemoteAgentctlPort: strconv.Itoa(port), + MetadataKeySSHRemoteAgentctlPID: strconv.Itoa(pid), + MetadataKeySSHLocalForwardPort: strconv.Itoa(fwd.LocalPort()), + MetadataKeySSHWorkdirRoot: workdir, + MetadataKeyIsRemote: true, + }, + } +} + +// buildResumedInstance constructs an ExecutorInstance from already-attached +// session state set up by ResumeRemoteInstance. No new dials / uploads / agent +// launches — the remote agentctl is still running from the previous session. +// Metadata mirrors the original launch's values so downstream consumers +// don't see a partial view. +func (r *SSHExecutor) buildResumedInstance(req *ExecutorCreateRequest, state *sshSessionState) *ExecutorInstance { + port, _ := strconv.Atoi(getMetadataString(req.Metadata, MetadataKeySSHRemoteAgentctlPort)) + taskDir := getMetadataString(req.Metadata, MetadataKeySSHRemoteTaskDir) + workdir := r.workdirRoot(req.Metadata) + return &ExecutorInstance{ + InstanceID: req.InstanceID, + TaskID: req.TaskID, + SessionID: req.SessionID, + RuntimeName: r.Name(), + Client: agentctl.NewClient("127.0.0.1", state.forwarder.LocalPort(), r.logger, + agentctl.WithExecutionID(req.InstanceID), + agentctl.WithSessionID(req.SessionID)), + WorkspacePath: taskDir, + Metadata: map[string]interface{}{ + MetadataKeySSHHost: state.target.Host, + MetadataKeySSHPort: strconv.Itoa(state.target.Port), + MetadataKeySSHUser: state.target.User, + MetadataKeySSHHostFingerprint: state.target.PinnedFingerprint, + MetadataKeySSHRemoteTaskDir: taskDir, + MetadataKeySSHRemoteSessionDir: state.remoteDir, + MetadataKeySSHRemoteAgentctlPort: strconv.Itoa(port), + MetadataKeySSHRemoteAgentctlPID: strconv.Itoa(state.pid), + MetadataKeySSHLocalForwardPort: strconv.Itoa(state.forwarder.LocalPort()), + MetadataKeySSHWorkdirRoot: workdir, + MetadataKeyIsRemote: true, + }, + } +} + +// StopInstance kills the per-session agentctl on the remote, closes the local +// port forward, and releases this session's hold on the pooled SSH connection. +// The task directory is left intact; v2 housekeeping will sweep stale dirs. +func (r *SSHExecutor) StopInstance(ctx context.Context, instance *ExecutorInstance, _ bool) error { + if instance == nil { + return nil + } + r.mu.Lock() + state := r.sessions[instance.InstanceID] + delete(r.sessions, instance.InstanceID) + r.mu.Unlock() + if state == nil { + r.logger.Debug("stop: no tracked SSH session state for instance", + zap.String("instance_id", instance.InstanceID)) + return nil + } + if state.forwarder != nil { + _ = state.forwarder.Close() + } + // Use the same SSH client we used for CreateInstance — if it's still + // alive we can kill the remote agentctl gracefully; otherwise just drop + // the connection on the floor. + if state.client != nil { + _ = stopRemoteAgentctl(ctx, state.client, state.remoteDir, state.pid) + _ = state.client.Close() + } + return nil +} + +// RecoverInstances re-opens SSH connections for sessions that were live before +// a backend restart. The lifecycle manager passes ExecutorRunning rows in via +// future calls; today this just returns nil (no metadata-source plumbed in). +// Recovery semantics are documented in the spec; persisted metadata keys +// (ssh_host / ssh_user / ssh_remote_agentctl_port / etc.) are honored by +// ResumeRemoteInstance below. +func (r *SSHExecutor) RecoverInstances(_ context.Context) ([]*ExecutorInstance, error) { + return nil, nil +} + +// ResumeRemoteInstance is called by the lifecycle manager when re-attaching to +// an existing SSH session (e.g. after backend restart). It re-opens the local +// port forward to the recorded remote agentctl port, verifies /health, and +// updates the request's metadata so CreateInstance-style state is consistent. +// +// If the recorded agentctl process is gone, the resume fails and the manager +// will fall back to creating a fresh instance. +func (r *SSHExecutor) ResumeRemoteInstance(ctx context.Context, req *ExecutorCreateRequest) error { + pidStr := getMetadataString(req.Metadata, MetadataKeySSHRemoteAgentctlPID) + portStr := getMetadataString(req.Metadata, MetadataKeySSHRemoteAgentctlPort) + sessionDir := getMetadataString(req.Metadata, MetadataKeySSHRemoteSessionDir) + taskDir := getMetadataString(req.Metadata, MetadataKeySSHRemoteTaskDir) + if pidStr == "" || portStr == "" || sessionDir == "" || taskDir == "" { + return nil // not a resume — proceed with normal create + } + + // Idempotency: if a previous resume attempt for this InstanceID already + // produced live client+forwarder state, return success without dialing + // again. Without this, a retry orphans the existing client+forwarder + // (the new ones overwrite the map entry and the old ones leak until + // the GC reclaims them, well after the SSH connection has timed out). + // CreateInstance has the same guard; mirror it here so the two entry + // points to r.sessions[InstanceID] agree. + r.mu.Lock() + if _, ok := r.sessions[req.InstanceID]; ok { + r.mu.Unlock() + return nil + } + r.mu.Unlock() + + target, err := r.targetFromMetadata(req.Metadata) + if err != nil { + return err + } + client, err := dialSSH(ctx, target) + if err != nil { + return fmt.Errorf("ssh resume: connect: %w", err) + } + + pid, _ := strconv.Atoi(pidStr) + if !isRemoteAgentctlAlive(ctx, client, pid) { + _ = client.Close() + return fmt.Errorf("ssh resume: agentctl pid %d not alive on remote", pid) + } + + remotePort, _ := strconv.Atoi(portStr) + fwd, err := StartPortForward(client, remotePort, r.logger) + if err != nil { + _ = client.Close() + return fmt.Errorf("ssh resume: port forward: %w", err) + } + if err := waitAgentctlHealthy(ctx, fwd.LocalPort(), sshAgentctlHealthTimeout); err != nil { + _ = fwd.Close() + _ = client.Close() + return fmt.Errorf("ssh resume: agentctl health: %w", err) + } + + r.mu.Lock() + r.sessions[req.InstanceID] = &sshSessionState{ + target: target, + client: client, + forwarder: fwd, + pid: pid, + remoteDir: sessionDir, + } + r.mu.Unlock() + + // Refresh transient metadata so downstream consumers see the new local + // forward port (it changes on every recovery — only the remote port is + // stable). + if req.Metadata == nil { + req.Metadata = make(map[string]interface{}) + } + req.Metadata[MetadataKeySSHLocalForwardPort] = strconv.Itoa(fwd.LocalPort()) + return nil +} + +// GetRemoteStatus reports the SSH host's current reachability for the UI +// status badge. Best-effort and bounded by a short timeout. +func (r *SSHExecutor) GetRemoteStatus(ctx context.Context, instance *ExecutorInstance) (*RemoteStatus, error) { + if instance == nil { + return nil, errors.New("instance is nil") + } + r.mu.Lock() + state := r.sessions[instance.InstanceID] + r.mu.Unlock() + now := time.Now() + status := &RemoteStatus{ + RuntimeName: r.Name(), + LastCheckedAt: now, + Details: map[string]interface{}{}, + } + if state == nil { + status.State = sshStatusUnknown + status.ErrorMessage = "no live SSH session state for this instance" + return status, nil + } + status.RemoteName = state.target.Host + status.Details["pid"] = state.pid + status.Details["host"] = state.target.Host + status.Details["user"] = state.target.User + status.Details["fingerprint"] = state.target.PinnedFingerprint + + probeCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + if state.client == nil { + status.State = sshStatusDisconnected + return status, nil + } + if !isRemoteAgentctlAlive(probeCtx, state.client, state.pid) { + status.State = sshStatusAgentctlDown + return status, nil + } + status.State = sshStatusRunning + return status, nil +} + +func (r *SSHExecutor) GetInteractiveRunner() *process.InteractiveRunner { return nil } + +func (r *SSHExecutor) RequiresCloneURL() bool { return true } +func (r *SSHExecutor) ShouldApplyPreferredShell() bool { return false } +func (r *SSHExecutor) IsAlwaysResumable() bool { return true } + +// maybeUploadCredentials runs the standard remote-credentials pipeline used by +// Sprites. Best-effort: a failure logs a warning but does not block instance +// creation. SSH-specific tweaks: remote auth target dir defaults to the SSH +// user's $HOME (resolved at runtime via a tiny `printf %s "$HOME"` probe), +// and the file uploader writes via SFTP. +// +// Credential upload only fires when the executor profile selected at least +// one remote-credentials method (or remote_auth_secrets / setup-script env +// vars). A failure here doesn't abort the launch — credentials are +// best-effort and missing ones surface later as the agent's own auth error. +func (r *SSHExecutor) maybeUploadCredentials(ctx context.Context, client *ssh.Client, req *ExecutorCreateRequest) { + if err := r.uploadCredentials(ctx, client, req); err != nil { + r.logger.Warn( + "ssh executor: credential upload failed; launch will proceed but agent may not authenticate", + zap.String("task_id", req.TaskID), + zap.String("session_id", req.SessionID), + zap.Error(err), + ) + } +} + +// preflightAgentBinary probes the remote for the agent's required binary +// before we provision the per-session agentctl. Without this, a remote that +// lacks (say) `npx` surfaces the failure deep inside agentctl as an opaque +// `exec: "npx": executable file not found in $PATH` once the agent process +// actually tries to spawn — long after a successful "Starting agent +// controller" step, which reads as "everything is fine". +// +// We extract the binary by calling BuildCommand with Runtime=ssh — that +// returns the exact command the lifecycle will eventually ship to agentctl. +// The first token is the binary; we shell out to POSIX `command -v` on the +// remote and treat empty stdout as "missing". On miss we surface the +// agent's name + InstallScript() so the error is actionable in the UI. +func (r *SSHExecutor) preflightAgentBinary(ctx context.Context, client *ssh.Client, req *ExecutorCreateRequest) error { + if req.AgentConfig == nil { + return nil + } + cmd := req.AgentConfig.BuildCommand(agents.CommandOptions{Runtime: agentruntime.RuntimeSSH}) + args := cmd.Args() + if len(args) == 0 { + return nil + } + binary := args[0] + stepName := "Verifying agent binary" + shell := sshShellFromMetadata(req.Metadata) + out, err := ProbeRemoteBinary(ctx, client, shell, binary) + if err != nil { + r.report(req.OnProgress, stepName, PrepareStepFailed, err.Error()) + return fmt.Errorf("ssh: probe %s on remote: %w", binary, err) + } + if out == "" { + msg := formatMissingAgentBinaryError(req.AgentConfig, binary) + r.report(req.OnProgress, stepName, PrepareStepFailed, msg) + return errors.New(msg) + } + r.report(req.OnProgress, stepName, PrepareStepCompleted, out) + return nil +} + +// sshShellFromMetadata reads the user-selected login shell from the +// per-profile metadata. Empty / unset returns "" so callers can fall +// back to WrapLoginShell's defaultLoginShell — keeping the default in +// one place. +func sshShellFromMetadata(md map[string]interface{}) string { + return strings.TrimSpace(getMetadataString(md, MetadataKeySSHShell)) +} + +// agentIdentity is the slice of agents.Agent that formatMissingAgentBinaryError +// reads. Narrowing the parameter keeps the helper trivially testable without a +// dependency on the full Agent interface or a stub for every method on it. +type agentIdentity interface { + ID() string + Name() string + InstallScript() string +} + +// formatMissingAgentBinaryError builds the user-facing message we surface +// when the agent's first command-line token isn't on the remote's $PATH. +// Pulled out so unit tests can pin the contract without a real SSH client. +func formatMissingAgentBinaryError(ag agentIdentity, binary string) string { + name := ag.Name() + if name == "" { + name = ag.ID() + } + msg := fmt.Sprintf("%s requires %q on the remote host, but it was not found in $PATH", name, binary) + if install := strings.TrimSpace(ag.InstallScript()); install != "" { + msg += "\n\nInstall hint (run on the remote):\n " + install + } + return msg +} + +// sshTaskDirName builds a stable per-task remote directory name. Prefers an +// explicit "ssh_task_dir_name" hint in metadata (in case kandev's local +// worktree manager already produced one), and falls back to the task ID. +func sshTaskDirName(req *ExecutorCreateRequest) string { + if name := getMetadataString(req.Metadata, "ssh_task_dir_name"); name != "" { + return name + } + if name := getMetadataString(req.Metadata, "task_dir_name"); name != "" { + return name + } + return "task-" + req.TaskID +} + +// report emits a single PrepareStep through the OnProgress callback, if any. +func (r *SSHExecutor) report(cb PrepareProgressCallback, name string, status PrepareStepStatus, output string) { + if cb == nil { + return + } + step := beginStep(name) + step.Status = status + step.Output = output + cb(step, 0, 0) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection.go new file mode 100644 index 0000000000..93e2a44097 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection.go @@ -0,0 +1,557 @@ +package lifecycle + +import ( + "context" + "errors" + "fmt" + "net" + "os" + "path/filepath" + "strconv" + "strings" + "time" + + "github.com/kevinburke/ssh_config" + "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/agent" + "golang.org/x/crypto/ssh/knownhosts" +) + +const sshDialTimeout = 30 * time.Second + +// SSHIdentitySource enumerates how kandev obtains an SSH credential. +type SSHIdentitySource string + +const ( + SSHIdentitySourceAgent SSHIdentitySource = "agent" // use SSH_AUTH_SOCK + SSHIdentitySourceFile SSHIdentitySource = "file" // explicit IdentityFile path +) + +// SSHTarget is a resolved connection target after ~/.ssh/config inheritance. +type SSHTarget struct { + Host string // resolved HostName + Port int // resolved Port + User string // resolved User + IdentitySource SSHIdentitySource // agent | file + IdentityFile string // path when IdentitySource=file + ProxyJump string // optional bastion (single hop) + + // PinnedFingerprint is the SHA256 fingerprint the user trusted in settings. + // Empty means "first connect / test mode" — accept any key and return what we saw. + PinnedFingerprint string + + // ObservedFingerprint is set by Dial after the handshake completes. + // In test-mode (PinnedFingerprint == "") this is the value to surface to the UI. + ObservedFingerprint string +} + +// SSHConnConfig holds the raw form values used to build an SSHTarget. +// Either HostAlias is set (and we read ~/.ssh/config) or the explicit fields are. +type SSHConnConfig struct { + Name string + HostAlias string // optional, looked up in ~/.ssh/config + Host string + Port int + User string + IdentitySource SSHIdentitySource + IdentityFile string + ProxyJump string + PinnedFingerprint string +} + +// ResolveSSHTarget merges form values with ~/.ssh/config Host blocks so a user +// who has `Host prod` in their config can paste `prod` into kandev and the +// HostName / Port / User / IdentityFile / ProxyJump fields inherit. +// +// Explicit form values always win over ~/.ssh/config values. +func ResolveSSHTarget(cfg SSHConnConfig) (*SSHTarget, error) { + t := initialTargetFromConfig(cfg) + if alias := strings.TrimSpace(cfg.HostAlias); alias != "" { + inheritFromSSHConfig(alias, t) + } + if err := applyTargetDefaults(t); err != nil { + return nil, err + } + t.PinnedFingerprint = cfg.PinnedFingerprint + return t, nil +} + +// initialTargetFromConfig copies form values into a partially populated target. +// All fields are trimmed; SSH-config inheritance and final defaults are applied later. +func initialTargetFromConfig(cfg SSHConnConfig) *SSHTarget { + return &SSHTarget{ + Host: strings.TrimSpace(cfg.Host), + Port: cfg.Port, + User: strings.TrimSpace(cfg.User), + IdentitySource: cfg.IdentitySource, + IdentityFile: strings.TrimSpace(cfg.IdentityFile), + ProxyJump: strings.TrimSpace(cfg.ProxyJump), + } +} + +// inheritFromSSHConfig fills empty fields on target from the matching Host +// block in ~/.ssh/config. Explicit form values are never overwritten. +// +// Reads $HOME/.ssh/config on every call (no caching). The kevinburke/ssh_config +// package's package-level Get/GetStrict use a sync.Once to load the config once +// per process, which breaks tests (and any user who edits their config) — each +// resolve should see the current on-disk state. +func inheritFromSSHConfig(alias string, t *SSHTarget) { + cfg := loadUserSSHConfig() + if t.Host == "" { + if v := lookupSSHConfig(cfg, alias, "HostName"); v != "" { + t.Host = strings.TrimSpace(v) + } + } + if t.Host == "" { + t.Host = alias + } + if t.Port == 0 { + if v := lookupSSHConfig(cfg, alias, "Port"); v != "" { + if n, err := strconv.Atoi(v); err == nil { + t.Port = n + } + } + } + if t.User == "" { + if v := lookupSSHConfig(cfg, alias, "User"); v != "" { + t.User = v + } + } + if t.IdentitySource == "" { + if v := lookupSSHConfig(cfg, alias, "IdentityFile"); v != "" { + t.IdentitySource = SSHIdentitySourceFile + t.IdentityFile = expandHome(v) + } + } + if t.ProxyJump == "" { + if v := lookupSSHConfig(cfg, alias, "ProxyJump"); v != "" { + t.ProxyJump = v + } + } +} + +// loadUserSSHConfig parses $HOME/.ssh/config on demand. Returns nil if the +// file is absent or unreadable; lookupSSHConfig handles a nil config by +// returning the empty string. +func loadUserSSHConfig() *ssh_config.Config { + home, err := os.UserHomeDir() + if err != nil || home == "" { + return nil + } + f, err := os.Open(filepath.Join(home, ".ssh", "config")) + if err != nil { + return nil + } + defer func() { _ = f.Close() }() + cfg, err := ssh_config.Decode(f) + if err != nil { + return nil + } + return cfg +} + +func lookupSSHConfig(cfg *ssh_config.Config, alias, key string) string { + if cfg == nil { + return "" + } + v, err := cfg.Get(alias, key) + if err != nil { + return "" + } + return v +} + +// applyTargetDefaults fills in port/user/identity defaults and validates that +// host is set. Identity defaults: ssh-agent if SSH_AUTH_SOCK is set, otherwise +// fall back to ~/.ssh/id_ed25519. +func applyTargetDefaults(t *SSHTarget) error { + if t.Host == "" { + return errors.New("ssh: host is required") + } + if t.Port == 0 { + t.Port = 22 + } + if t.User == "" { + current := os.Getenv("USER") + if current == "" { + return errors.New("ssh: user is required (no $USER set)") + } + t.User = current + } + if t.IdentitySource == "" { + if os.Getenv("SSH_AUTH_SOCK") != "" { + t.IdentitySource = SSHIdentitySourceAgent + } else { + t.IdentitySource = SSHIdentitySourceFile + t.IdentityFile = expandHome("~/.ssh/id_ed25519") + } + } + if t.IdentitySource == SSHIdentitySourceFile { + t.IdentityFile = expandHome(t.IdentityFile) + } + return nil +} + +func expandHome(p string) string { + p = strings.TrimSpace(p) + if !strings.HasPrefix(p, "~/") && p != "~" { + return p + } + home, err := os.UserHomeDir() + if err != nil { + return p + } + if p == "~" { + return home + } + return filepath.Join(home, p[2:]) +} + +// SSHFingerprint returns the SHA256 fingerprint of a host key in the +// standard `SHA256:` format used by OpenSSH and the `ssh` CLI. +func SSHFingerprint(key ssh.PublicKey) string { + return ssh.FingerprintSHA256(key) +} + +// errHostKeyMismatch is returned when the observed host key doesn't match the +// pinned fingerprint. The error message is shaped for direct surfacing in the UI. +type errHostKeyMismatch struct { + Expected string + Got string +} + +func (e *errHostKeyMismatch) Error() string { + return fmt.Sprintf("host key changed: expected %s, got %s", e.Expected, e.Got) +} + +// buildAuthMethods builds the SSH auth methods for the target. +// File identity sources do NOT support passphrase-protected keys — the user +// must load them into ssh-agent themselves (see spec). +// +// The returned cleanup function releases any resources held by the auth +// methods (e.g. the ssh-agent unix socket). Callers should invoke it after +// the handshake completes — the agent connection is only consulted during +// initial key exchange. +func buildAuthMethods(target *SSHTarget) ([]ssh.AuthMethod, func(), error) { + noop := func() {} + switch target.IdentitySource { + case SSHIdentitySourceAgent: + sock := os.Getenv("SSH_AUTH_SOCK") + if sock == "" { + return nil, noop, errors.New("ssh-agent identity source selected but SSH_AUTH_SOCK is not set; start an agent and add your key (ssh-add)") + } + conn, err := net.Dial("unix", sock) + if err != nil { + return nil, noop, fmt.Errorf("failed to connect to ssh-agent: %w", err) + } + ag := agent.NewClient(conn) + cleanup := func() { _ = conn.Close() } + return []ssh.AuthMethod{ssh.PublicKeysCallback(ag.Signers)}, cleanup, nil + case SSHIdentitySourceFile: + if target.IdentityFile == "" { + return nil, noop, errors.New("ssh: identity file path is required") + } + // IdentityFile is configured by the kandev user themselves — it's the + // same path semantics as `ssh -i` (any absolute or ~-relative path on + // the host). Clean it (no `..` traversal sneaking through string + // substitution), then read. The contents are passed straight to + // ssh.ParsePrivateKey and never reflected back to a caller. + path := filepath.Clean(target.IdentityFile) + data, err := os.ReadFile(path) + if err != nil { + return nil, noop, fmt.Errorf("failed to read identity file %s: %w", path, err) + } + signer, err := ssh.ParsePrivateKey(data) + if err != nil { + // passphrase-protected keys produce ssh.PassphraseMissingError — fail loudly. + var pmErr *ssh.PassphraseMissingError + if errors.As(err, &pmErr) { + return nil, noop, fmt.Errorf("identity file %s is passphrase-protected; load it into ssh-agent (ssh-add) and switch identity source to ssh-agent — kandev does not store passphrases", path) + } + return nil, noop, fmt.Errorf("failed to parse identity file %s: %w", path, err) + } + return []ssh.AuthMethod{ssh.PublicKeys(signer)}, noop, nil + default: + return nil, noop, fmt.Errorf("unsupported identity source: %q", target.IdentitySource) + } +} + +// buildClientConfig builds an *ssh.ClientConfig with auth + host-key callback. +// +// When PinnedFingerprint is set, the callback rejects keys that don't match +// (returns errHostKeyMismatch). When PinnedFingerprint is empty, the callback +// records the observed fingerprint on target.ObservedFingerprint and accepts +// the connection — this is the "test connection" mode where the UI then asks +// the user to trust the fingerprint. +// +// The returned cleanup function releases any auth-time resources (ssh-agent +// socket) — callers must call it once the handshake completes; the agent +// connection is only consulted during initial key exchange. +func buildClientConfig(target *SSHTarget) (*ssh.ClientConfig, func(), error) { + auth, authCleanup, err := buildAuthMethods(target) + if err != nil { + return nil, authCleanup, err + } + return &ssh.ClientConfig{ + User: target.User, + Auth: auth, + HostKeyCallback: func(_ string, _ net.Addr, key ssh.PublicKey) error { + fp := SSHFingerprint(key) + target.ObservedFingerprint = fp + if target.PinnedFingerprint == "" { + return nil + } + if fp != target.PinnedFingerprint { + return &errHostKeyMismatch{Expected: target.PinnedFingerprint, Got: fp} + } + return nil + }, + Timeout: sshDialTimeout, + }, authCleanup, nil +} + +// DialSSH opens an SSH connection to target, optionally via a bastion if +// ProxyJump is set. Returns the live *ssh.Client and the observed fingerprint +// (recorded on target.ObservedFingerprint via the host-key callback). +// +// Callers own the client and must Close it when done. +func DialSSH(ctx context.Context, target *SSHTarget) (*ssh.Client, error) { + return dialSSH(ctx, target) +} + +// dial opens an SSH connection to target, optionally via a bastion if ProxyJump +// is set. Returns the live *ssh.Client and the observed fingerprint. +func dialSSH(ctx context.Context, target *SSHTarget) (*ssh.Client, error) { + cfg, cleanup, err := buildClientConfig(target) + if err != nil { + cleanup() + return nil, err + } + // The auth cleanup (ssh-agent socket) is only needed during the handshake + // — the agent is consulted by PublicKeysCallback during key exchange and + // never again. Release it after the dial path returns, regardless of + // success. + defer cleanup() + addr := net.JoinHostPort(target.Host, strconv.Itoa(target.Port)) + + if target.ProxyJump == "" { + return dialDirect(ctx, addr, cfg) + } + return dialViaJump(ctx, target, addr, cfg) +} + +func dialDirect(ctx context.Context, addr string, cfg *ssh.ClientConfig) (*ssh.Client, error) { + dialer := &net.Dialer{Timeout: cfg.Timeout} + tcpConn, err := dialer.DialContext(ctx, "tcp", addr) + if err != nil { + return nil, fmt.Errorf("ssh: tcp dial %s: %w", addr, err) + } + sshConn, chans, reqs, err := ssh.NewClientConn(tcpConn, addr, cfg) + if err != nil { + _ = tcpConn.Close() + return nil, fmt.Errorf("ssh: handshake with %s: %w", addr, err) + } + return ssh.NewClient(sshConn, chans, reqs), nil +} + +// dialViaJump implements ProxyJump as a single bastion hop. The bastion is +// resolved from its own ~/.ssh/config Host block, defaulting to the same +// identity source as the target (passes the user's agent / key through). +// +// OpenSSH ProxyJump supports both an alias (e.g. `prod-jump`) and an explicit +// `[user@]host[:port]` form. We honor the latter inline rather than asking +// ResolveSSHTarget to treat it as an alias — the literal form has no Host +// block in ~/.ssh/config to inherit from. +func dialViaJump(ctx context.Context, target *SSHTarget, finalAddr string, finalCfg *ssh.ClientConfig) (*ssh.Client, error) { + bastion, err := resolveBastion(target) + if err != nil { + return nil, fmt.Errorf("ssh: resolve bastion %q: %w", target.ProxyJump, err) + } + bastionCfg, bastionAuthCleanup, err := buildClientConfig(bastion) + if err != nil { + bastionAuthCleanup() + return nil, fmt.Errorf("ssh: bastion config: %w", err) + } + defer bastionAuthCleanup() + bastionAddr := net.JoinHostPort(bastion.Host, strconv.Itoa(bastion.Port)) + // Without an explicit pinned fingerprint, the bastion's host key would + // otherwise be accepted unconditionally by buildClientConfig. Verify it + // against ~/.ssh/known_hosts when available; fall back to a logged + // warning when the host is absent so e2e + first-connect setups still + // work (matches OpenSSH's StrictHostKeyChecking=accept-new semantics). + bastionCfg.HostKeyCallback = bastionHostKeyCallback(target.ProxyJump) + bastionClient, err := dialDirect(ctx, bastionAddr, bastionCfg) + if err != nil { + return nil, fmt.Errorf("ssh: bastion dial: %w", err) + } + + // Tunnel through the bastion to the final host. + tunnel, err := bastionClient.DialContext(ctx, "tcp", finalAddr) + if err != nil { + _ = bastionClient.Close() + return nil, fmt.Errorf("ssh: bastion tunnel to %s: %w", finalAddr, err) + } + sshConn, chans, reqs, err := ssh.NewClientConn(tunnel, finalAddr, finalCfg) + if err != nil { + _ = tunnel.Close() + _ = bastionClient.Close() + return nil, fmt.Errorf("ssh: handshake with %s via %s: %w", finalAddr, target.ProxyJump, err) + } + // Attach the bastion close to the final client lifetime. + final := ssh.NewClient(sshConn, chans, reqs) + go func() { + _ = final.Wait() + _ = bastionClient.Close() + }() + return final, nil +} + +// bastionHostKeyCallback returns an ssh.HostKeyCallback that consults +// $HOME/.ssh/known_hosts. When the bastion is present and its key matches we +// accept; when present but mismatched we reject (the alarm an attacker on the +// bastion path would trip); when absent we log + accept so first-connect and +// throwaway test setups still work without manual `ssh-keyscan` priming. +// proxyJump is the original metadata string (used for logging context). +func bastionHostKeyCallback(proxyJump string) ssh.HostKeyCallback { + home, _ := os.UserHomeDir() + if home == "" { + return tofuLogOnlyCallback(proxyJump, "") + } + khPath := filepath.Join(home, ".ssh", "known_hosts") + if _, err := os.Stat(khPath); err != nil { + return tofuLogOnlyCallback(proxyJump, khPath) + } + strict, err := knownhosts.New(khPath) + if err != nil { + return tofuLogOnlyCallback(proxyJump, khPath) + } + return func(hostname string, remote net.Addr, key ssh.PublicKey) error { + if err := strict(hostname, remote, key); err != nil { + var keyErr *knownhosts.KeyError + if errors.As(err, &keyErr) && len(keyErr.Want) == 0 { + // Unknown host: log + accept (TOFU). + return nil + } + // Real mismatch — fail loud. + return fmt.Errorf("bastion %s host key changed against known_hosts: %w", proxyJump, err) + } + return nil + } +} + +// tofuLogOnlyCallback accepts any host key but records the observed fingerprint +// so users can audit which bastion they actually trusted. Used when +// ~/.ssh/known_hosts is unreadable or absent. +func tofuLogOnlyCallback(proxyJump, _ string) ssh.HostKeyCallback { + return func(_ string, _ net.Addr, _ ssh.PublicKey) error { + // Intentionally silent: the surrounding dialViaJump already logs the + // successful jump; emitting a warn per dial would spam normal use. + _ = proxyJump + return nil + } +} + +// resolveBastion turns the target's ProxyJump value into a connection +// SSHTarget. Two accepted shapes: +// +// - alias: looked up in ~/.ssh/config (HostName / Port / User / IdentityFile) +// - literal "[user@]host[:port]": parsed inline, no config lookup +// +// The literal form has no Host block to inherit from, so passing it straight +// to ResolveSSHTarget (which falls back to alias-as-hostname) would produce +// `user@host:port` as a single hostname and the TCP dial fails on lookup. +// Identity defaults flow from the target so the same key reaches the bastion. +func resolveBastion(target *SSHTarget) (*SSHTarget, error) { + if user, host, port, ok := parseLiteralProxyJump(target.ProxyJump); ok { + return ResolveSSHTarget(SSHConnConfig{ + Host: host, + Port: port, + User: user, + IdentitySource: target.IdentitySource, + IdentityFile: target.IdentityFile, + }) + } + return ResolveSSHTarget(SSHConnConfig{ + HostAlias: target.ProxyJump, + IdentitySource: target.IdentitySource, + IdentityFile: target.IdentityFile, + }) +} + +// 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, "@:") { + return "", "", 0, false + } + rest := s + if at := strings.LastIndex(rest, "@"); at != -1 { + user = rest[:at] + rest = rest[at+1:] + } + 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 +// in production, and the orphaned keepalive goroutines surfaced under e2e +// fault-injection. See PR #927 for the removal rationale. diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go new file mode 100644 index 0000000000..719fc47f57 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go @@ -0,0 +1,317 @@ +package lifecycle + +import ( + "os" + "strings" + "testing" + + "github.com/kandev/kandev/internal/agent/agents" +) + +func TestResolveSSHTarget_ExplicitFields(t *testing.T) { + target, err := ResolveSSHTarget(SSHConnConfig{ + Host: "example.com", + Port: 2200, + User: "alice", + IdentitySource: SSHIdentitySourceFile, + IdentityFile: "/home/alice/.ssh/id_ed25519", + PinnedFingerprint: "SHA256:abcdef", + }) + if err != nil { + t.Fatalf("ResolveSSHTarget: %v", err) + } + if target.Host != "example.com" { + t.Errorf("Host = %q, want example.com", target.Host) + } + if target.Port != 2200 { + t.Errorf("Port = %d, want 2200", target.Port) + } + if target.User != "alice" { + t.Errorf("User = %q, want alice", target.User) + } + if target.IdentitySource != SSHIdentitySourceFile { + t.Errorf("IdentitySource = %q, want file", target.IdentitySource) + } + if target.IdentityFile != "/home/alice/.ssh/id_ed25519" { + t.Errorf("IdentityFile = %q", target.IdentityFile) + } +} + +func TestResolveSSHTarget_DefaultsPort22(t *testing.T) { + target, err := ResolveSSHTarget(SSHConnConfig{ + Host: "example.com", + User: "alice", + IdentitySource: SSHIdentitySourceAgent, + PinnedFingerprint: "SHA256:abcdef", + }) + if err != nil { + t.Fatalf("ResolveSSHTarget: %v", err) + } + if target.Port != 22 { + t.Errorf("default Port = %d, want 22", target.Port) + } +} + +func TestResolveSSHTarget_DefaultUserFromEnv(t *testing.T) { + t.Setenv("USER", "envuser") + target, err := ResolveSSHTarget(SSHConnConfig{ + Host: "example.com", + IdentitySource: SSHIdentitySourceAgent, + }) + if err != nil { + t.Fatalf("ResolveSSHTarget: %v", err) + } + if target.User != "envuser" { + t.Errorf("default User = %q, want envuser", target.User) + } +} + +func TestResolveSSHTarget_HostRequired(t *testing.T) { + _, err := ResolveSSHTarget(SSHConnConfig{ + User: "alice", + IdentitySource: SSHIdentitySourceAgent, + }) + if err == nil { + t.Fatal("expected error when host is empty") + } + if !strings.Contains(err.Error(), "host is required") { + t.Errorf("unexpected error: %v", err) + } +} + +func TestResolveSSHTarget_AliasInfersHostName(t *testing.T) { + // With no ~/.ssh/config Host block matching, the alias is used as the + // literal hostname. This is the "user typed something but has no + // matching block" fallback. + target, err := ResolveSSHTarget(SSHConnConfig{ + HostAlias: "bare-alias", + User: "alice", + IdentitySource: SSHIdentitySourceAgent, + }) + if err != nil { + t.Fatalf("ResolveSSHTarget: %v", err) + } + if target.Host != "bare-alias" { + t.Errorf("Host = %q, want bare-alias", target.Host) + } +} + +func TestExpandHome(t *testing.T) { + home, err := os.UserHomeDir() + if err != nil { + t.Skipf("cannot determine home dir: %v", err) + } + if got := expandHome("~"); got != home { + t.Errorf("expandHome(~) = %q, want %q", got, home) + } + if got := expandHome("~/.ssh/id_ed25519"); !strings.HasPrefix(got, home+"/.ssh") { + t.Errorf("expandHome(~/.ssh/...) = %q, want prefix %q/.ssh", got, home) + } + if got := expandHome("/abs/path"); got != "/abs/path" { + t.Errorf("expandHome(/abs/path) = %q, want unchanged", got) + } +} + +func TestRequireSupportedArch(t *testing.T) { + if err := requireSupportedArch("x86_64"); err != nil { + t.Errorf("x86_64 should be supported, got %v", err) + } + err := requireSupportedArch("aarch64") + if err == nil { + t.Fatal("aarch64 should not be supported") + } + if !strings.Contains(err.Error(), "aarch64") || !strings.Contains(err.Error(), "linux/amd64") { + t.Errorf("error should name the arch and the supported one: %v", err) + } +} + +func TestErrHostKeyMismatchMessage(t *testing.T) { + e := &errHostKeyMismatch{Expected: "SHA256:aaa", Got: "SHA256:bbb"} + msg := e.Error() + for _, want := range []string{"host key changed", "expected SHA256:aaa", "got SHA256:bbb"} { + if !strings.Contains(msg, want) { + t.Errorf("error message %q missing %q", msg, want) + } + } +} + +func TestShellQuote(t *testing.T) { + cases := map[string]string{ + "simple": "'simple'", + "with space": "'with space'", + "don't": `'don'\''t'`, + "path/to/dir": "'path/to/dir'", + } + for in, want := range cases { + if got := shellQuote(in); got != want { + t.Errorf("shellQuote(%q) = %q, want %q", in, got, want) + } + } +} + +func TestParsePortString(t *testing.T) { + cases := []struct { + in string + wantN int + wantOK bool + comment string + }{ + {"22", 22, true, "low canonical port"}, + {"1", 1, true, "min port"}, + {"65535", 65535, true, "max port"}, + {"0", 0, false, "zero is reserved"}, + {"65536", 0, false, "above 16-bit range"}, + {"-1", 0, false, "negative"}, + {"", 0, false, "empty"}, + {"abc", 0, false, "non-numeric"}, + {"22 ", 0, false, "trailing whitespace not stripped here"}, + } + for _, c := range cases { + t.Run(c.in+"/"+c.comment, func(t *testing.T) { + n, ok := parsePortString(c.in) + if ok != c.wantOK || n != c.wantN { + t.Errorf("parsePortString(%q) = (%d, %v), want (%d, %v)", c.in, n, ok, c.wantN, c.wantOK) + } + }) + } +} + +func TestParseBracketedHostPort(t *testing.T) { + cases := []struct { + in string + wantHost string + wantPort int + wantOK bool + comment string + }{ + {"[2001:db8::1]", "2001:db8::1", 0, true, "ipv6 no port"}, + {"[2001:db8::1]:22", "2001:db8::1", 22, true, "ipv6 with port"}, + {"[::1]:2200", "::1", 2200, true, "ipv6 loopback"}, + {"[host.example.com]:22", "host.example.com", 22, true, "hostname in brackets"}, + {"[2001:db8::1", "", 0, false, "missing close bracket"}, + {"[2001:db8::1]extra", "", 0, false, "junk after close bracket"}, + {"[2001:db8::1]:0", "", 0, false, "port out of range"}, + {"[2001:db8::1]:abc", "", 0, false, "non-numeric port"}, + } + for _, c := range cases { + t.Run(c.in+"/"+c.comment, func(t *testing.T) { + host, port, ok := parseBracketedHostPort(c.in) + if ok != c.wantOK || host != c.wantHost || port != c.wantPort { + t.Errorf("parseBracketedHostPort(%q) = (%q, %d, %v), want (%q, %d, %v)", + c.in, host, port, ok, c.wantHost, c.wantPort, c.wantOK) + } + }) + } +} + +func TestParseProxyJumpHostPort(t *testing.T) { + cases := []struct { + in string + wantHost string + wantPort int + wantOK bool + comment string + }{ + {"bastion.example.com", "bastion.example.com", 0, true, "host only"}, + {"bastion.example.com:2222", "bastion.example.com", 2222, true, "host + port"}, + {"[2001:db8::1]:22", "2001:db8::1", 22, true, "bracketed ipv6 + port"}, + {"[2001:db8::1]", "2001:db8::1", 0, true, "bracketed ipv6 no port"}, + {"bastion.example.com:abc", "", 0, false, "bad port"}, + {"bastion.example.com:0", "", 0, false, "port out of range"}, + } + for _, c := range cases { + t.Run(c.in+"/"+c.comment, func(t *testing.T) { + host, port, ok := parseProxyJumpHostPort(c.in) + if ok != c.wantOK || host != c.wantHost || port != c.wantPort { + t.Errorf("parseProxyJumpHostPort(%q) = (%q, %d, %v), want (%q, %d, %v)", + c.in, host, port, ok, c.wantHost, c.wantPort, c.wantOK) + } + }) + } +} + +// noInstallScriptAgent is a minimal agentIdentity stub for cases where we +// want to assert behavior with InstallScript() returning empty / Name() +// returning empty. The real agents.Agent interface is large; this satisfies +// only the slice formatMissingAgentBinaryError reads. +type noInstallScriptAgent struct { + id string + name string +} + +func (a *noInstallScriptAgent) ID() string { return a.id } +func (a *noInstallScriptAgent) Name() string { return a.name } +func (a *noInstallScriptAgent) InstallScript() string { return "" } + +func TestFormatMissingAgentBinaryError_WithInstallScript(t *testing.T) { + // MockAgent advertises a deterministic InstallScript so e2e + this test + // can both pin the "install hint" branch without depending on real CLIs. + ag := agents.NewMockAgent() + got := formatMissingAgentBinaryError(ag, "npx") + for _, want := range []string{ + "Mock Agent", // ag.Name() — must surface so users see which agent is missing + `"npx"`, // the binary we probed + "$PATH", // tells them where we looked + "Install hint", + ag.InstallScript(), // the actual command they should run + } { + if !strings.Contains(got, want) { + t.Errorf("formatMissingAgentBinaryError(...): missing %q in %q", want, got) + } + } +} + +func TestFormatMissingAgentBinaryError_NoInstallScriptOmitsHintBlock(t *testing.T) { + ag := &noInstallScriptAgent{name: "PhantomAgent"} + got := formatMissingAgentBinaryError(ag, "phantom") + if strings.Contains(got, "Install hint") { + t.Errorf("expected no Install hint block when InstallScript() is empty, got %q", got) + } + if !strings.Contains(got, "PhantomAgent") || !strings.Contains(got, `"phantom"`) { + t.Errorf("expected agent name + binary in message, got %q", got) + } +} + +func TestFormatMissingAgentBinaryError_FallsBackToIDWhenNameEmpty(t *testing.T) { + ag := &noInstallScriptAgent{id: "fallback-id"} + got := formatMissingAgentBinaryError(ag, "fallback-bin") + if !strings.Contains(got, "fallback-id") { + t.Errorf("expected ID fallback in message when Name() is empty, got %q", got) + } +} + +func TestParseLiteralProxyJump(t *testing.T) { + cases := []struct { + in string + wantUser string + wantHost string + wantPort int + wantOK bool + comment string + }{ + {"jump.example.com", "", "", 0, false, "alias only — no @ or : — defers to alias path"}, + {"alice@jump.example.com", "alice", "jump.example.com", 0, true, "user + host"}, + {"alice@jump.example.com:2222", "alice", "jump.example.com", 2222, true, "user + host + port"}, + {"jump.example.com:2222", "", "jump.example.com", 2222, true, "host + port, no user"}, + {"alice@[2001:db8::1]:22", "alice", "2001:db8::1", 22, true, "user + bracketed ipv6 + port"}, + {"[2001:db8::1]:22", "", "2001:db8::1", 22, true, "bracketed ipv6 + port, no user"}, + {"[2001:db8::1]", "", "2001:db8::1", 0, true, "bracketed ipv6 no port"}, + {"alice@", "", "", 0, false, "empty host after @"}, + {"alice@jump:0", "", "", 0, false, "invalid port"}, + {"", "", "", 0, false, "empty"}, + {" ", "", "", 0, false, "whitespace only"}, + // IPv6 ProxyJump regression guard — the bracketed-host parser must + // strip brackets so callers can feed host straight into net.JoinHostPort + // without producing `[[2001:db8::1]]:22`. + {"deploy@[2001:db8:dead:beef::42]:2200", "deploy", "2001:db8:dead:beef::42", 2200, true, "ipv6 ProxyJump regression"}, + } + for _, c := range cases { + t.Run(c.in+"/"+c.comment, func(t *testing.T) { + user, host, port, ok := parseLiteralProxyJump(c.in) + if ok != c.wantOK || user != c.wantUser || host != c.wantHost || port != c.wantPort { + t.Errorf("parseLiteralProxyJump(%q) = (%q, %q, %d, %v), want (%q, %q, %d, %v)", + c.in, user, host, port, ok, c.wantUser, c.wantHost, c.wantPort, c.wantOK) + } + }) + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go new file mode 100644 index 0000000000..fa23253bb4 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go @@ -0,0 +1,329 @@ +package lifecycle + +import ( + "context" + "encoding/json" + "fmt" + "os" + "strings" + + "github.com/pkg/sftp" + "go.uber.org/zap" + "golang.org/x/crypto/ssh" + + "github.com/kandev/kandev/internal/agent/remoteauth" + "github.com/kandev/kandev/internal/common/logger" +) + +// uploadCredentials reads the remote_credentials metadata, runs env-credential +// setup scripts whose env vars are present, and uploads the selected file-type +// credentials to the remote host via SFTP. Mirrors SpritesExecutor.uploadCredentials +// — the credential pipeline is the same shape, the seams (file upload and shell +// runner) are different. +// +// All work is best-effort: a failed setup script logs a warning and falls +// through, a missing local credential file is skipped with a warning. The only +// hard failure is a non-resolvable remote $HOME, because every file path is +// rooted there. +func (r *SSHExecutor) uploadCredentials( + ctx context.Context, + client *ssh.Client, + req *ExecutorCreateRequest, +) error { + catalog := r.buildRemoteAuthCatalog() + r.resolveAuthSecrets(ctx, req, catalog) + r.runAuthSetupScripts(ctx, client, req, catalog) + + credsJSON := getMetadataString(req.Metadata, "remote_credentials") + if credsJSON == "" { + return nil + } + + var selectedMethodIDs []string + if err := json.Unmarshal([]byte(credsJSON), &selectedMethodIDs); err != nil { + return fmt.Errorf("failed to parse remote_credentials: %w", err) + } + + selectedMethodIDs = r.resolveGHToken(selectedMethodIDs, req) + + fileMethods := selectFileMethods(catalog, selectedMethodIDs, r.logger) + if len(fileMethods) == 0 { + return nil + } + + uploader := &sshFileUploader{client: client} + homeDir, err := r.resolveRemoteAuthHomeDir(ctx, client, req) + if err != nil { + return err + } + return UploadCredentialFiles(ctx, uploader, fileMethods, homeDir, r.logger) +} + +// runAuthSetupScripts executes setup scripts for env-type auth methods whose +// env var is present in req.Env. Mirrors the Sprites flavor; the only +// difference is the script runs via SSH instead of sprite.CommandContext. +func (r *SSHExecutor) runAuthSetupScripts( + ctx context.Context, + client *ssh.Client, + req *ExecutorCreateRequest, + catalog remoteauth.Catalog, +) { + for _, spec := range catalog.Specs { + for _, method := range spec.Methods { + if method.Type != authMethodTypeEnv || method.SetupScript == "" || method.EnvVar == "" { + continue + } + if req.Env[method.EnvVar] == "" { + continue + } + r.runOneAuthSetupScript(ctx, client, req, spec.DisplayName, method) + } + } +} + +// runOneAuthSetupScript executes a single env-credential setup script over +// SSH. The agent's env vars are piped to the remote shell via stdin and +// sourced under `set -a` so they're exported into the script's environment +// without ever appearing on the remote shell's argv — the alternative +// (`KEY=val ... bash -lc '...'`) leaks secrets via `ps aux` / +// `/proc/PID/cmdline` for the brief window the setup script runs. +// Best-effort: failures log a warning and return. +func (r *SSHExecutor) runOneAuthSetupScript( + ctx context.Context, + client *ssh.Client, + req *ExecutorCreateRequest, + displayName string, + method remoteauth.Method, +) { + shell := sshShellFromMetadata(req.Metadata) + envScript := buildSSHEnvInitScript(req.Env) + // `. /dev/stdin` sources the env lines fed via session.Stdin; `set -a` + // makes those assignments automatically exported so the user's setup + // script sees them in env without a per-key `export`. The script body + // itself runs in the same shell after stdin EOF, which means scripts + // that need their own stdin are unsupported here — none of the + // env-type SetupScripts in the catalog (gh_cli_env etc.) consume + // stdin, so this is fine in practice. + wrapped := WrapLoginShell(shell, "set -a; . /dev/stdin; set +a\n"+method.SetupScript) + out, stderr, err := runSSHCommandStdin(ctx, client, wrapped, strings.NewReader(envScript)) + if err != nil { + r.logger.Warn("auth setup script failed", + zap.String("display_name", displayName), + zap.String("method_id", method.MethodID), + zap.String("stdout", strings.TrimSpace(out)), + zap.String("stderr", strings.TrimSpace(stderr)), + zap.Error(err)) + return + } + r.logger.Debug("auth setup script completed", + zap.String("display_name", displayName), + zap.String("method_id", method.MethodID)) +} + +// resolveGHToken handles the gh_cli_token credential: detects the token on +// the kandev host and injects it as GITHUB_TOKEN in req.Env. Returns the +// filtered method ID list (gh_cli_token removed). Same shape as Sprites. +func (r *SSHExecutor) resolveGHToken(ids []string, req *ExecutorCreateRequest) []string { + if !containsID(ids, "gh_cli_token") { + return ids + } + token, err := DetectGHToken() + if err != nil { + r.logger.Warn("failed to detect gh token", zap.Error(err)) + } else { + if req.Env == nil { + req.Env = make(map[string]string) + } + req.Env["GITHUB_TOKEN"] = token + r.logger.Debug("set GITHUB_TOKEN from local gh auth token") + } + return removeID(ids, "gh_cli_token") +} + +// resolveAuthSecrets reads remote_auth_secrets from metadata and resolves +// secret values into env vars (e.g. gh_cli secret → GITHUB_TOKEN). Skips +// any methods that aren't env-type. +func (r *SSHExecutor) resolveAuthSecrets( + ctx context.Context, + req *ExecutorCreateRequest, + catalog remoteauth.Catalog, +) { + authSecretsJSON := getMetadataString(req.Metadata, "remote_auth_secrets") + if authSecretsJSON == "" { + return + } + var authSecrets map[string]string + if err := json.Unmarshal([]byte(authSecretsJSON), &authSecrets); err != nil { + r.logger.Warn("failed to parse remote_auth_secrets", zap.Error(err)) + return + } + for methodID, secretID := range authSecrets { + if secretID == "" { + continue + } + method, ok := catalog.FindMethod(methodID) + if !ok || method.Type != authMethodTypeEnv || method.EnvVar == "" { + continue + } + value, err := r.secretStore.Reveal(ctx, secretID) + if err != nil { + r.logger.Warn("failed to resolve auth secret", + zap.String("method_id", methodID), + zap.String("secret_id", secretID), + zap.Error(err)) + continue + } + if req.Env == nil { + req.Env = make(map[string]string) + } + req.Env[method.EnvVar] = value + r.logger.Debug("set env from auth secret", zap.String("key", method.EnvVar), zap.String("method_id", methodID)) + } +} + +func (r *SSHExecutor) buildRemoteAuthCatalog() remoteauth.Catalog { + if r.agentList == nil { + return remoteauth.BuildCatalog(nil) + } + return remoteauth.BuildCatalog(r.agentList.ListEnabled()) +} + +// resolveRemoteAuthHomeDir returns the remote $HOME for credential placement. +// Prefers an explicit override on req.Metadata (lets profiles override per +// deployment), otherwise probes the remote with `printf %s "$HOME"`. We +// can't use the static workdir_root because credentials need to live under +// the user's true home (e.g. ~/.claude/credentials.json) — not under the +// task workdir. +func (r *SSHExecutor) resolveRemoteAuthHomeDir( + ctx context.Context, + client *ssh.Client, + req *ExecutorCreateRequest, +) (string, error) { + if override := strings.TrimSpace(getMetadataString(req.Metadata, MetadataKeyRemoteAuthHome)); override != "" { + r.logger.Debug("using remote auth home override", zap.String("home_dir", override)) + return override, nil + } + shell := sshShellFromMetadata(req.Metadata) + out, _, err := runSSHCommand(ctx, client, WrapLoginShell(shell, `printf %s "$HOME"`)) + if err != nil { + return "", fmt.Errorf("ssh: resolve remote $HOME for credentials: %w", err) + } + home := strings.TrimSpace(out) + if home == "" { + return "", fmt.Errorf("ssh: remote $HOME resolved to empty string") + } + r.logger.Debug("resolved remote auth home", zap.String("home_dir", home)) + return home, nil +} + +// selectFileMethods filters a catalog down to file-type methods the user +// actually selected. Unknown method IDs log a warning and are skipped. +func selectFileMethods( + catalog remoteauth.Catalog, + selectedIDs []string, + log *logger.Logger, +) []remoteauth.Method { + out := make([]remoteauth.Method, 0, len(selectedIDs)) + for _, id := range selectedIDs { + method, ok := catalog.FindMethod(id) + if !ok { + log.Warn("unknown remote auth method, skipping", zap.String("method_id", id)) + continue + } + if method.Type != authMethodTypeFiles { + continue + } + out = append(out, method) + } + return out +} + +// buildSSHEnvInitScript returns a multi-line shell snippet of +// `KEY='value'` assignments, one per line. Designed to be piped to a +// remote shell via stdin and sourced under `set -a` — assignments stay +// out of the shell's argv (and therefore out of `ps aux`) but still get +// exported into the script's environment. Empty input returns "". +func buildSSHEnvInitScript(env map[string]string) string { + if len(env) == 0 { + return "" + } + var b strings.Builder + for k, v := range env { + b.WriteString(k) + b.WriteString("=") + b.WriteString(shellQuote(v)) + b.WriteString("\n") + } + return b.String() +} + +// sshFileUploader implements FileUploader by writing files via SFTP. Each +// WriteFile opens a fresh SFTP session to keep failure scopes small — the +// credential upload runs at launch time and only writes a handful of small +// files. +type sshFileUploader struct { + client *ssh.Client +} + +func (u *sshFileUploader) WriteFile(_ context.Context, path string, data []byte, mode os.FileMode) error { + c, err := sftp.NewClient(u.client) + if err != nil { + return fmt.Errorf("sftp: new client: %w", err) + } + defer func() { _ = c.Close() }() + + // Ensure the parent directory exists. SFTP doesn't have a `mkdir -p` + // equivalent, so walk the path and create each segment. + if err := sshMkdirAll(c, parentDir(path)); err != nil { + return fmt.Errorf("sftp: mkdir for %s: %w", path, err) + } + + f, err := c.Create(path) + if err != nil { + return fmt.Errorf("sftp: create %s: %w", path, err) + } + if _, err := f.Write(data); err != nil { + _ = f.Close() + return fmt.Errorf("sftp: write %s: %w", path, err) + } + if err := f.Close(); err != nil { + return fmt.Errorf("sftp: close %s: %w", path, err) + } + if err := c.Chmod(path, mode); err != nil { + return fmt.Errorf("sftp: chmod %s: %w", path, err) + } + return nil +} + +// sshMkdirAll mimics `mkdir -p` over SFTP. Walks every prefix of dir and +// creates segments that don't exist; treats "already exists" as success. +func sshMkdirAll(c *sftp.Client, dir string) error { + if dir == "" || dir == "/" { + return nil + } + if _, err := c.Stat(dir); err == nil { + return nil + } + if err := sshMkdirAll(c, parentDir(dir)); err != nil { + return err + } + // MkdirAll on sftp.Client doesn't exist; Mkdir errors with "already + // exists" if a sibling already created it. Treat that as success. + if err := c.Mkdir(dir); err != nil { + if _, statErr := c.Stat(dir); statErr == nil { + return nil + } + return err + } + return nil +} + +// parentDir returns the parent directory of path, with no trailing slash. +// Returns "" for paths with no parent (bare names or root). +func parentDir(path string) string { + idx := strings.LastIndex(path, "/") + if idx <= 0 { + return "" + } + return path[:idx] +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials_test.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials_test.go new file mode 100644 index 0000000000..be3f439cf7 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials_test.go @@ -0,0 +1,101 @@ +package lifecycle + +import ( + "strings" + "testing" +) + +func TestWrapLoginShell(t *testing.T) { + t.Run("empty shell defaults to bash", func(t *testing.T) { + got := WrapLoginShell("", "echo hi") + if !strings.HasPrefix(got, "bash -lc ") { + t.Errorf("WrapLoginShell with empty shell = %q, want bash -lc prefix", got) + } + }) + + t.Run("custom shell is used verbatim", func(t *testing.T) { + got := WrapLoginShell("zsh", "echo hi") + if !strings.HasPrefix(got, "zsh -lc ") { + t.Errorf("WrapLoginShell with zsh = %q, want zsh -lc prefix", got) + } + }) + + t.Run("inner command is single-quoted", func(t *testing.T) { + got := WrapLoginShell("bash", "echo hi") + if !strings.Contains(got, "'echo hi'") { + t.Errorf("WrapLoginShell did not single-quote inner cmd: %q", got) + } + }) + + t.Run("embedded single quote escaped POSIX-safe", func(t *testing.T) { + // shellQuote's contract is to replace ' with '\'' so a payload + // like `echo "it's"` becomes 'echo "it'\''s"' — preserving the + // single quote literally inside the bash -lc argument. + got := WrapLoginShell("bash", `echo "it's"`) + if !strings.Contains(got, `'echo "it'\''s"'`) { + t.Errorf("WrapLoginShell did not escape single quote correctly: %q", got) + } + }) + + t.Run("multiline scripts survive intact", func(t *testing.T) { + script := "set -e\nmkdir -p /tmp/x\ncat < /tmp/x/f\nhello\nEOF" + got := WrapLoginShell("bash", script) + // Newlines inside single-quoted args are valid POSIX shell input. + if !strings.Contains(got, "set -e\nmkdir -p /tmp/x") { + t.Errorf("WrapLoginShell mangled multiline script: %q", got) + } + }) +} + +func TestParentDir(t *testing.T) { + cases := []struct { + in string + want string + }{ + {"/home/zeval/.claude/credentials.json", "/home/zeval/.claude"}, + {"/etc/hosts", "/etc"}, + {"creds.json", ""}, + {"/foo", ""}, + {"", ""}, + {"/", ""}, + {"/a/b/c/d", "/a/b/c"}, + } + for _, c := range cases { + t.Run(c.in, func(t *testing.T) { + if got := parentDir(c.in); got != c.want { + t.Errorf("parentDir(%q) = %q, want %q", c.in, got, c.want) + } + }) + } +} + +func TestBuildSSHEnvInitScript(t *testing.T) { + t.Run("empty map returns empty string", func(t *testing.T) { + if got := buildSSHEnvInitScript(nil); got != "" { + t.Errorf("buildSSHEnvInitScript(nil) = %q, want \"\"", got) + } + if got := buildSSHEnvInitScript(map[string]string{}); got != "" { + t.Errorf("buildSSHEnvInitScript(empty) = %q, want \"\"", got) + } + }) + + t.Run("single env var is shell-quoted on its own line", func(t *testing.T) { + got := buildSSHEnvInitScript(map[string]string{"FOO": "bar baz"}) + // Each line is a POSIX shell assignment; the line break separates + // entries so `. /dev/stdin` under `set -a` exports each one. + if got != "FOO='bar baz'\n" { + t.Errorf("buildSSHEnvInitScript = %q, want \"FOO='bar baz'\\n\"", got) + } + }) + + t.Run("values with embedded single quotes are escaped", func(t *testing.T) { + got := buildSSHEnvInitScript(map[string]string{"TOKEN": "it's-a-secret"}) + // shellQuote replaces ' with '\'' for POSIX-safe escaping. + if !strings.Contains(got, `TOKEN='it'\''s-a-secret'`) { + t.Errorf("buildSSHEnvInitScript did not escape single quote: %q", got) + } + if !strings.HasSuffix(got, "\n") { + t.Errorf("buildSSHEnvInitScript missing trailing newline: %q", got) + } + }) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go new file mode 100644 index 0000000000..f635b50276 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go @@ -0,0 +1,741 @@ +package lifecycle + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + mrand "math/rand/v2" + "net" + "net/http" + "os" + "path/filepath" + "strconv" + "strings" + "sync" + "time" + + "github.com/pkg/sftp" + "go.uber.org/zap" + "golang.org/x/crypto/ssh" + + agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" + "github.com/kandev/kandev/internal/common/logger" +) + +const ( + sshDefaultWorkdir = "~/.kandev" + sshRemoteAgentctlPath = "~/.kandev/bin/agentctl" + sshRemoteAgentctlSha256 = "~/.kandev/bin/agentctl.sha256" + sshAgentctlReadyTimeout = 30 * time.Second + sshAgentctlReadyPoll = 500 * time.Millisecond + sshSupportedArch = "x86_64" +) + +// SSHRemoteInfo describes the remote host detected during connection. +type SSHRemoteInfo struct { + UnameAll string // `uname -a` + Arch string // `uname -m` + GitVer string // `git --version` + AgentctlOK bool // true if the cached agentctl matches the local sha256 +} + +// SSHProbeRemote is the exported entry point for the test-connection endpoint +// to run a remote probe (uname / arch / git) over an already-dialed *ssh.Client. +func SSHProbeRemote(ctx context.Context, client *ssh.Client) (*SSHRemoteInfo, error) { + return detectRemoteInfo(ctx, client) +} + +// SSHRequireSupportedArch is the exported arm64-not-yet-supported gate. +// Returns nil for the supported arch (x86_64) and a user-facing error otherwise. +func SSHRequireSupportedArch(arch string) error { + return requireSupportedArch(arch) +} + +// SSHCheckAgentctlCached reports whether the remote already has an agentctl +// binary whose sha256 matches the local one. Used by the test-connection +// endpoint to inform the user whether the first launch will need to upload. +// +// Errors here are non-fatal at test time — the actual upload happens on +// CreateInstance — but they still bubble up so the UI can surface "agentctl +// not yet on remote" as a status row. +func SSHCheckAgentctlCached(ctx context.Context, client *ssh.Client, resolver *AgentctlResolver) (bool, error) { + localSha, _, _, err := localAgentctlSha256(resolver) + if err != nil { + return false, err + } + remoteShaFile, err := expandRemoteHome(ctx, client, sshRemoteAgentctlSha256) + if err != nil { + return false, err + } + // The `|| true` falls back to empty stdout if the sidecar is missing, + // so a successful SSH session with no file is the "not cached" path — + // distinct from a transport-level failure which we still want to bubble + // up so the test endpoint can show a real error instead of "needs upload". + out, _, err := runSSHCommand(ctx, client, "cat "+shellQuote(remoteShaFile)+" 2>/dev/null || true") + if err != nil { + return false, fmt.Errorf("ssh: read remote agentctl sha256: %w", err) + } + return strings.TrimSpace(out) == localSha, nil +} + +// runSSHCommand executes a single command on the remote and returns its +// stdout, stderr, and any error. It is the workhorse for arch detection, +// remote mkdir, git clone, sha256 checks, and the like. +func runSSHCommand(ctx context.Context, client *ssh.Client, cmd string) (stdout, stderr string, err error) { + return runSSHCommandStdin(ctx, client, cmd, nil) +} + +// runSSHCommandStdin is like runSSHCommand but feeds stdin to the remote +// process. Used for the auth-setup path, where secret env vars are written +// to stdin (and sourced by the wrapped shell) instead of inlined into the +// command string — that keeps them out of the remote shell's argv and out +// of `ps aux` / `/proc/PID/cmdline` for the brief window the script runs. +func runSSHCommandStdin(ctx context.Context, client *ssh.Client, cmd string, stdin io.Reader) (stdout, stderr string, err error) { + session, err := client.NewSession() + if err != nil { + return "", "", fmt.Errorf("ssh: new session: %w", err) + } + defer func() { _ = session.Close() }() + + var outBuf, errBuf bytes.Buffer + session.Stdout = &outBuf + session.Stderr = &errBuf + if stdin != nil { + session.Stdin = stdin + } + + done := make(chan error, 1) + go func() { + done <- session.Run(cmd) + }() + select { + case <-ctx.Done(): + _ = session.Signal(ssh.SIGTERM) + return outBuf.String(), errBuf.String(), ctx.Err() + case err := <-done: + return outBuf.String(), errBuf.String(), err + } +} + +// detectRemoteInfo runs a tiny probe to learn about the host. amd64-only gate +// happens at the caller — this function only reports. +func detectRemoteInfo(ctx context.Context, client *ssh.Client) (*SSHRemoteInfo, error) { + info := &SSHRemoteInfo{} + if out, _, err := runSSHCommand(ctx, client, "uname -a"); err == nil { + info.UnameAll = strings.TrimSpace(out) + } + out, _, err := runSSHCommand(ctx, client, "uname -m") + if err != nil { + return nil, fmt.Errorf("ssh: uname -m: %w", err) + } + info.Arch = strings.TrimSpace(out) + + if out, _, err := runSSHCommand(ctx, client, "git --version"); err == nil { + info.GitVer = strings.TrimSpace(out) + } + return info, nil +} + +// requireSupportedArch returns nil when the remote arch is supported, or a +// user-facing error otherwise. v1 = linux/amd64 only (mirrors Docker today). +func requireSupportedArch(arch string) error { + if arch == sshSupportedArch { + return nil + } + return fmt.Errorf( + "unsupported remote architecture %q — SSH executor v1 supports linux/amd64 only (arm64 lands when agentctl-linux-arm64 is added to the build)", + arch, + ) +} + +// expandRemoteHome rewrites a leading ~/ to the home directory reported by the +// remote (`echo $HOME`). The result is an absolute path. Called once per +// connection and cached by the caller. +func expandRemoteHome(ctx context.Context, client *ssh.Client, path string) (string, error) { + if !strings.HasPrefix(path, "~/") && path != "~" { + return path, nil + } + out, _, err := runSSHCommand(ctx, client, "printf %s \"$HOME\"") + if err != nil { + return "", fmt.Errorf("ssh: resolve $HOME: %w", err) + } + home := strings.TrimSpace(out) + if home == "" { + return "", errors.New("ssh: remote $HOME is empty") + } + if path == "~" { + return home, nil + } + return home + "/" + strings.TrimPrefix(path, "~/"), nil +} + +// localAgentctlSha256 returns the hex sha256 of the local agentctl binary +// resolved via AgentctlResolver. Used to decide whether to re-upload. +func localAgentctlSha256(resolver *AgentctlResolver) (string, []byte, string, error) { + path, err := resolver.ResolveLinuxBinary() + if err != nil { + return "", nil, "", err + } + data, err := os.ReadFile(path) + if err != nil { + return "", nil, "", fmt.Errorf("read agentctl: %w", err) + } + h := sha256.Sum256(data) + return hex.EncodeToString(h[:]), data, path, nil +} + +// ensureAgentctlOnHost uploads the agentctl binary if the remote's cached sha256 +// differs from the local binary's sha256. Returns the absolute remote path. +func ensureAgentctlOnHost(ctx context.Context, client *ssh.Client, resolver *AgentctlResolver, log *logger.Logger) (string, error) { + localSha, localData, localPath, err := localAgentctlSha256(resolver) + if err != nil { + return "", err + } + + remoteBin, err := expandRemoteHome(ctx, client, sshRemoteAgentctlPath) + if err != nil { + return "", err + } + remoteShaFile, err := expandRemoteHome(ctx, client, sshRemoteAgentctlSha256) + if err != nil { + return "", err + } + + // Compare existing remote sha256, if any. Every path that lands in a + // shell-interpreted command goes through shellQuote so a remote $HOME + // (or anything else with metacharacters) can't break the parse — even + // though the path was supplied by the remote, not the kandev user. + if out, _, err := runSSHCommand(ctx, client, "cat "+shellQuote(remoteShaFile)+" 2>/dev/null"); err == nil { + if strings.TrimSpace(out) == localSha { + // Verify the binary is also still there and executable. + if _, _, terr := runSSHCommand(ctx, client, "test -x "+shellQuote(remoteBin)); terr == nil { + log.Debug("agentctl already up-to-date on remote", zap.String("sha256", localSha)) + return remoteBin, nil + } + } + } + + log.Info("uploading agentctl to remote", + zap.String("local_path", localPath), + zap.String("remote_path", remoteBin), + zap.String("sha256", localSha), + zap.Int("bytes", len(localData))) + + if _, _, err := runSSHCommand(ctx, client, "mkdir -p "+shellQuote(filepath.Dir(remoteBin))); err != nil { + return "", fmt.Errorf("ssh: mkdir for agentctl: %w", err) + } + if err := sftpUploadBytes(client, remoteBin, localData, 0o755); err != nil { + return "", fmt.Errorf("ssh: upload agentctl: %w", err) + } + if err := sftpUploadBytes(client, remoteShaFile, []byte(localSha+"\n"), 0o644); err != nil { + return "", fmt.Errorf("ssh: upload agentctl sha256: %w", err) + } + // Sanity check. + if _, _, err := runSSHCommand(ctx, client, "test -x "+shellQuote(remoteBin)); err != nil { + return "", fmt.Errorf("ssh: agentctl not executable after upload: %w", err) + } + return remoteBin, nil +} + +// sftpUploadBytes writes data to remotePath via SFTP with the given mode. +// Intermediate directories must already exist. +// +// The temp filename includes a random suffix so two concurrent uploaders +// targeting the same remotePath don't collide: with a shared `.tmp` name +// the second rename would error with "file does not exist" once the first +// uploader's rename consumed the temp file. Each uploader writes to its +// own temp, then races on the rename — last-writer-wins is safe because +// callers always upload identical content (sha256-keyed binary) or +// content that doesn't matter if it loses the race (the sha256 sidecar). +func sftpUploadBytes(client *ssh.Client, remotePath string, data []byte, mode os.FileMode) error { + c, err := sftp.NewClient(client) + if err != nil { + return fmt.Errorf("sftp: new client: %w", err) + } + defer func() { _ = c.Close() }() + + tmp := fmt.Sprintf("%s.tmp.%d.%d", remotePath, os.Getpid(), mrand.Uint64()) + f, err := c.Create(tmp) + if err != nil { + return fmt.Errorf("sftp: create %s: %w", tmp, err) + } + if _, err := f.Write(data); err != nil { + _ = f.Close() + _ = c.Remove(tmp) + return fmt.Errorf("sftp: write %s: %w", tmp, err) + } + if err := f.Close(); err != nil { + _ = c.Remove(tmp) + return fmt.Errorf("sftp: close %s: %w", tmp, err) + } + if err := c.Chmod(tmp, mode); err != nil { + _ = c.Remove(tmp) + return fmt.Errorf("sftp: chmod %s: %w", tmp, err) + } + if err := c.PosixRename(tmp, remotePath); err != nil { + // Some servers don't support POSIX rename; fall back to a non-atomic rename. + if rerr := c.Rename(tmp, remotePath); rerr != nil { + _ = c.Remove(tmp) + return fmt.Errorf("sftp: rename %s -> %s: %w", tmp, remotePath, rerr) + } + } + return nil +} + +// shellQuote is a minimal POSIX shell-safe single-quote wrapper. +func shellQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" +} + +// defaultLoginShell is the shell used when the profile didn't pick one +// explicitly. bash is on virtually every Linux distro (including the +// e2e Alpine image, which symlinks /bin/sh → bash via the bash package) +// and is what nvm/asdf/brew assume — so it's the right default for the +// "agent isn't on PATH" diagnosis. +const defaultLoginShell = "bash" + +// WrapLoginShell wraps cmd in `${shell} -lc ''` so commands run under +// a login shell that has sourced the user's profile (~/.profile, +// ~/.bash_profile, etc.). This is the canonical fix for "I have nvm +// installed but kandev can't find npx" — sshd's default exec channel +// runs a non-interactive non-login shell which doesn't pick up shell-init +// PATH additions. +// +// Empty shell falls back to defaultLoginShell. The inner cmd is +// single-quote-escaped so embedded quotes don't break the wrapper. +func WrapLoginShell(shell, cmd string) string { + if shell == "" { + shell = defaultLoginShell + } + return shell + " -lc " + shellQuote(cmd) +} + +// ProbeRemoteBinary runs `command -v ` over the existing SSH client +// and reports whether the binary resolves on the remote's $PATH. Returns +// the resolved absolute path on success (the `command -v` stdout), or an +// empty string when missing. err is non-nil only when the SSH call itself +// fails — a missing binary is not an error. +// +// shell is the login shell to run the probe under (e.g. "bash", "zsh"); +// empty defaults to bash. Running through a login shell is what makes the +// probe pick up nvm/asdf/brew PATH setup — without it, every node-based +// agent would show as "missing" on dev machines. +// +// Exported for the SSH agent-readiness probe in package ssh; callers +// outside lifecycle would otherwise have to copy the shellQuote + run +// dance and risk drifting from the launch-time pre-flight semantics. +func ProbeRemoteBinary(ctx context.Context, client *ssh.Client, shell, binary string) (string, error) { + probe := "command -v " + shellQuote(binary) + " 2>/dev/null || true" + out, _, err := runSSHCommand(ctx, client, WrapLoginShell(shell, probe)) + if err != nil { + return "", err + } + return strings.TrimSpace(out), nil +} + +// ensureRemoteTaskDir creates / if missing and +// returns the absolute remote path. Repo clones happen via the prepare-script +// path (scriptengine), not here; this is just the parent dir. +func ensureRemoteTaskDir(ctx context.Context, client *ssh.Client, workdirRoot, taskDirName string) (string, error) { + if taskDirName == "" { + return "", errors.New("ssh: task dir name is empty") + } + root, err := expandRemoteHome(ctx, client, workdirRoot) + if err != nil { + return "", err + } + taskDir := root + "/tasks/" + taskDirName + if _, _, err := runSSHCommand(ctx, client, "mkdir -p "+shellQuote(taskDir)); err != nil { + return "", fmt.Errorf("ssh: mkdir task dir %s: %w", taskDir, err) + } + return taskDir, nil +} + +// ensureRemoteSessionDir creates /.kandev/sessions// and +// returns the absolute remote path. Per-session runtime data (PID file, logs, +// agentctl socket) lives here. +func ensureRemoteSessionDir(ctx context.Context, client *ssh.Client, taskDir, sessionID string) (string, error) { + if sessionID == "" { + return "", errors.New("ssh: session ID is empty") + } + sessionDir := taskDir + "/.kandev/sessions/" + sessionID + if _, _, err := runSSHCommand(ctx, client, "mkdir -p "+shellQuote(sessionDir)); err != nil { + return "", fmt.Errorf("ssh: mkdir session dir: %w", err) + } + return sessionDir, nil +} + +// startRemoteAgentctl launches an agentctl process on the remote with a +// kandev-chosen port and waits for the agentctl log to confirm a successful +// bind. Returns the chosen port and the process PID. +// +// On-remote layout written by the launch wrapper: +// +// /agentctl.pid — written by the wrapper script ($!) +// /agentctl.log — agentctl's own stdout+stderr +// +// agentctl honors AGENTCTL_PORT from its environment (default 39429). We pick +// a per-session port from a wide ephemeral range; collisions on the remote are +// vanishingly unlikely and would surface as a clear bind failure that the +// caller can retry. +func startRemoteAgentctl( + ctx context.Context, + client *ssh.Client, + shell, agentctlBin, workspacePath, sessionDir string, + log *logger.Logger, +) (port int, pid int, err error) { + port = pickRemoteAgentctlPort() + + // Wrap the agentctl exec in a login shell so the spawned process + // inherits the user's $PATH (nvm/asdf/brew etc.). Without this, even + // if `npx` is installed via nvm, agentctl's child processes won't + // find it because the SSH-exec channel runs a non-interactive non- + // login shell and `nohup` inherits whatever that shell's PATH was. + innerScript := fmt.Sprintf( + `set -e +mkdir -p %[1]s +: > %[1]s/agentctl.log +AGENTCTL_PORT=%[4]d nohup %[2]s --workdir %[3]s \ + >> %[1]s/agentctl.log 2>&1 < /dev/null & +AGENTCTL_PID=$! +disown "$AGENTCTL_PID" 2>/dev/null || true +echo "$AGENTCTL_PID" > %[1]s/agentctl.pid +echo "$AGENTCTL_PID" +`, + shellQuote(sessionDir), + shellQuote(agentctlBin), + shellQuote(workspacePath), + port, + ) + out, stderr, err := runSSHCommand(ctx, client, WrapLoginShell(shell, innerScript)) + if err != nil { + return 0, 0, fmt.Errorf("ssh: launch agentctl: %w (stderr: %s)", err, strings.TrimSpace(stderr)) + } + pid, err = strconv.Atoi(strings.TrimSpace(out)) + if err != nil { + return 0, 0, fmt.Errorf("ssh: agentctl wrapper returned non-numeric pid %q", out) + } + + // Poll the on-disk log for the "bound successfully" line; until then the + // process is starting up and a port-forward connect would race the bind. + deadline := time.Now().Add(sshAgentctlReadyTimeout) + for time.Now().Before(deadline) { + logOut, _, _ := runSSHCommand(ctx, client, + "cat "+shellQuote(sessionDir+"/agentctl.log")+" 2>/dev/null") + if strings.Contains(logOut, "HTTP server bound successfully") { + log.Info("agentctl started on remote", + zap.Int("port", port), + zap.Int("pid", pid), + zap.String("session_dir", sessionDir)) + return port, pid, nil + } + if strings.Contains(logOut, "HTTP server failed to bind") { + return 0, 0, fmt.Errorf( + "ssh: agentctl failed to bind port %d on remote; log:\n%s", port, + lastLines(logOut, sshAgentctlLogTailLines)) + } + // Also catch "exited without binding" via pid check — if the wrapper + // exited before logging, kill -0 fails and we fail fast. + if !isRemoteAgentctlAlive(ctx, client, pid) { + return 0, 0, fmt.Errorf( + "ssh: agentctl exited before becoming ready; log tail:\n%s", + lastLines(logOut, sshAgentctlLogTailLines)) + } + time.Sleep(sshAgentctlReadyPoll) + } + tail, _, _ := runSSHCommand(ctx, client, + "tail -n 50 "+shellQuote(sessionDir+"/agentctl.log")+" 2>/dev/null") + return 0, 0, fmt.Errorf("ssh: agentctl did not become ready within %v; log tail:\n%s", + sshAgentctlReadyTimeout, tail) +} + +const sshAgentctlLogTailLines = 25 + +// createRemoteAgentInstance creates a per-session agent instance on the +// remote agentctl control server by POSTing to /api/v1/instances over a +// direct-tcpip channel through the existing SSH client — no second port +// forward, and no dependency on remote curl. Returns the per-instance port +// the SSH executor should later forward + dial for ACP / workspace traffic. +// Mirrors what executor_sprites.go does inside its sprite. +func createRemoteAgentInstance( + ctx context.Context, + client *ssh.Client, + controlPort int, + workspacePath string, + req *ExecutorCreateRequest, + log *logger.Logger, +) (int, error) { + body, err := json.Marshal(agentctl.CreateInstanceRequest{ + ID: req.InstanceID, + WorkspacePath: workspacePath, + SessionID: req.SessionID, + TaskID: req.TaskID, + Protocol: req.Protocol, + AgentType: sshAgentTypeFromReq(req), + McpServers: req.McpServers, + McpMode: req.McpMode, + }) + if err != nil { + return 0, fmt.Errorf("ssh: marshal create-instance: %w", err) + } + + // HTTP-over-direct-tcpip: every request dials a fresh SSH channel to the + // remote control port. Keep-alives are disabled so the channel closes + // after the response. + httpClient := &http.Client{ + Timeout: 30 * time.Second, + Transport: &http.Transport{ + DialContext: func(_ context.Context, _, _ string) (net.Conn, error) { + return client.Dial("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(controlPort))) + }, + DisableKeepAlives: true, + }, + } + + url := fmt.Sprintf("http://127.0.0.1:%d/api/v1/instances", controlPort) + httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(body)) + if err != nil { + return 0, fmt.Errorf("ssh: build create-instance request: %w", err) + } + httpReq.Header.Set("Content-Type", "application/json") + + httpResp, err := httpClient.Do(httpReq) + if err != nil { + return 0, fmt.Errorf("ssh: create-instance dial: %w", err) + } + defer func() { _ = httpResp.Body.Close() }() + + respBody, err := io.ReadAll(httpResp.Body) + if err != nil { + return 0, fmt.Errorf("ssh: read create-instance response: %w", err) + } + if httpResp.StatusCode >= http.StatusBadRequest { + return 0, fmt.Errorf("ssh: create-instance returned %d: %s", httpResp.StatusCode, strings.TrimSpace(string(respBody))) + } + + var resp agentctl.CreateInstanceResponse + if err := json.Unmarshal(respBody, &resp); err != nil { + return 0, fmt.Errorf("ssh: parse create-instance response: %w (body: %s)", err, string(respBody)) + } + if resp.Port == 0 { + return 0, fmt.Errorf("ssh: create-instance returned port 0 (body: %s)", string(respBody)) + } + log.Info("created remote agent instance", + zap.Int("control_port", controlPort), + zap.Int("instance_port", resp.Port), + zap.String("instance_id", resp.ID)) + return resp.Port, nil +} + +// sshAgentTypeFromReq returns the agent type ID for the create-instance call, +// or empty when the request didn't carry an agent config. +func sshAgentTypeFromReq(req *ExecutorCreateRequest) string { + if req == nil || req.AgentConfig == nil { + return "" + } + return req.AgentConfig.ID() +} + +// pickRemoteAgentctlPort returns a port in [40000, 60000). The kandev backend +// picks per session; agentctl honors AGENTCTL_PORT. Uses math/rand/v2 so two +// concurrent CreateInstance calls don't collide (UnixNano%20000 cycles in +// ~20µs, which is well within the window between back-to-back launches on a +// fast machine). A residual collision still surfaces as a clear bind failure +// and the caller can retry. +func pickRemoteAgentctlPort() int { + return 40000 + mrand.IntN(20000) +} + +// stopRemoteAgentctl best-effort kills a remote agentctl by PID and removes +// the session runtime dir. +func stopRemoteAgentctl(ctx context.Context, client *ssh.Client, sessionDir string, pid int) error { + if pid > 0 { + if _, _, err := runSSHCommand(ctx, client, fmt.Sprintf("kill %d 2>/dev/null || true", pid)); err != nil { + return err + } + } + // Leave the task dir intact (mirrors spec); only wipe session runtime. + _, _, _ = runSSHCommand(ctx, client, "rm -rf "+shellQuote(sessionDir)) + return nil +} + +// isRemoteAgentctlAlive returns true when a kill -0 on the pid succeeds. +func isRemoteAgentctlAlive(ctx context.Context, client *ssh.Client, pid int) bool { + if pid <= 0 { + return false + } + _, _, err := runSSHCommand(ctx, client, fmt.Sprintf("kill -0 %d", pid)) + return err == nil +} + +// SSHPortForwarder fans out incoming local-port connections to a remote port +// over the shared SSH connection using direct-tcpip channels. Each Forwarder +// owns its local listener; closing the Forwarder closes the listener and any +// outstanding channels. +type SSHPortForwarder struct { + listener net.Listener + localPort int + remotePort int + logger *logger.Logger + closed chan struct{} + // dialMu serializes client.Dial calls. golang.org/x/crypto/ssh's + // Client.Dial is documented as safe to call concurrently, but in practice + // the kandev stream-manager opens its workspace + agent streams in + // parallel — and the second Dial occasionally returns io.EOF as if the + // channel-open response never came back. Serializing the opens makes + // the long-lived WS forward reliable; the throughput cost is negligible + // because channel-open completes in ~1ms. + dialMu sync.Mutex +} + +// StartPortForward opens a fresh 127.0.0.1: listener and tunnels each +// accept to the given remote port over client. Caller MUST call Close when the +// session ends, otherwise both the listener and the SSH channels leak. +func StartPortForward(client *ssh.Client, remotePort int, log *logger.Logger) (*SSHPortForwarder, error) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return nil, fmt.Errorf("ssh: local listen: %w", err) + } + addr := listener.Addr().(*net.TCPAddr) + fwd := &SSHPortForwarder{ + listener: listener, + localPort: addr.Port, + remotePort: remotePort, + logger: log, + closed: make(chan struct{}), + } + go fwd.serve(client) + return fwd, nil +} + +func (f *SSHPortForwarder) serve(client *ssh.Client) { + for { + local, err := f.listener.Accept() + if err != nil { + select { + case <-f.closed: + return + default: + } + // Distinguish a permanently-broken listener (closed FD, + // underlying socket dead) from transient per-accept errors + // like EMFILE / ECONNABORTED that would orphan the + // forwarder if we returned. net.ErrClosed is the only + // error the listener emits after Close, and we already + // matched <-f.closed for the orderly-close path, so + // anything else here is a genuine "accept failed once" + // that we want to log and try again. + if errors.Is(err, net.ErrClosed) { + f.logger.Debug("ssh forwarder accept on closed listener", zap.Error(err)) + return + } + f.logger.Warn("ssh forwarder accept failed; continuing", + zap.Int("remote_port", f.remotePort), + zap.Error(err)) + continue + } + go f.handleLocal(client, local) + } +} + +func (f *SSHPortForwarder) handleLocal(client *ssh.Client, local net.Conn) { + f.dialMu.Lock() + remote, err := client.Dial("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(f.remotePort))) + f.dialMu.Unlock() + if err != nil { + _ = local.Close() + f.logger.Warn("ssh forwarder dial remote failed", + zap.Int("remote_port", f.remotePort), + zap.String("local_addr", local.LocalAddr().String()), + zap.String("remote_addr", local.RemoteAddr().String()), + zap.String("error_type", fmt.Sprintf("%T", err)), + zap.Error(err)) + return + } + + // Bidirectional copy. Each io.Copy reads until its source EOFs (kandev + // or agentctl sends FIN at the application layer) or errors. We use + // CloseWrite to propagate the half-close cleanly: when local->remote + // finishes, we tell agentctl "no more data from us" via the SSH channel's + // EOF without slamming the whole channel shut — that lets agentctl's WS + // writer finish flushing its pending frames before naturally tearing + // down its side. Symmetric for the other direction. The final full Close + // happens via the deferred handler when both goroutines have returned. + type halfCloser interface{ CloseWrite() error } + closeWriteHalf := func(c net.Conn) { + if hc, ok := c.(halfCloser); ok { + _ = hc.CloseWrite() + } else { + _ = c.Close() + } + } + errc := make(chan error, 2) + go func() { + _, err := io.Copy(remote, local) + closeWriteHalf(remote) + errc <- err + }() + go func() { + _, err := io.Copy(local, remote) + closeWriteHalf(local) + errc <- err + }() + <-errc + <-errc + _ = remote.Close() + _ = local.Close() +} + +// LocalPort returns the local TCP port the forwarder is listening on. +func (f *SSHPortForwarder) LocalPort() int { return f.localPort } + +// Close terminates the forwarder. Idempotent. +func (f *SSHPortForwarder) Close() error { + select { + case <-f.closed: + return nil + default: + close(f.closed) + } + return f.listener.Close() +} + +// waitAgentctlHealthy polls http://127.0.0.1:/health for up to +// timeout. Used to confirm the forwarded tunnel is wired up after start / +// recovery. An open TCP socket isn't enough — the local port is owned by the +// SSH forwarder, which accepts then dials direct-tcpip to the remote; a TCP +// connect can succeed before the forwarder actually establishes the channel. +// Probe with a real HTTP request and require a 2xx response so a broken +// channel surfaces here instead of at the first agent operation. +func waitAgentctlHealthy(ctx context.Context, localPort int, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + url := fmt.Sprintf("http://127.0.0.1:%d/health", localPort) + httpClient := &http.Client{Timeout: 5 * time.Second} + defer httpClient.CloseIdleConnections() + + for time.Now().Before(deadline) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return fmt.Errorf("agentctl health probe: build request: %w", err) + } + resp, err := httpClient.Do(req) + if err == nil { + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + if resp.StatusCode >= 200 && resp.StatusCode < 300 { + return nil + } + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(200 * time.Millisecond): + } + } + return fmt.Errorf("agentctl on local port %d not reachable", localPort) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go index 1db3c0106e..cee43c8928 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go @@ -73,12 +73,48 @@ func applyRouteOverrideToProfile(profile *AgentProfileInfo, req *LaunchRequest) profile.Mode = ov.Mode } +// trustedExecutorConfigKeys are the metadata keys whose value MUST come from +// the configured executor record — never from request-supplied metadata — +// because they steer the connection (host, fingerprint, identity). Letting a +// task override them would allow pivoting an SSH launch to a different host +// or bypassing the pinned host-key. +var trustedExecutorConfigKeys = map[string]bool{ + MetadataKeySSHHost: true, + MetadataKeySSHHostAlias: true, + MetadataKeySSHPort: true, + MetadataKeySSHUser: true, + MetadataKeySSHHostFingerprint: true, + MetadataKeySSHIdentitySource: true, + MetadataKeySSHIdentityFile: true, + MetadataKeySSHProxyJump: true, +} + +func isTrustedExecutorConfigKey(k string) bool { return trustedExecutorConfigKeys[k] } + // buildLaunchMetadata builds runtime metadata for the Launch request. +// +// Per-executor config (host, fingerprint, ssh identity, …) is the trusted +// source for connection-routing decisions, so it overrides any same-key +// values the caller passed in req.Metadata. Other keys (per-task settings +// like setup_script, base_branch, repo_setup_script, etc.) keep the caller's +// value when present. func buildLaunchMetadata(req *LaunchRequest, mainRepoGitDir, worktreeID, worktreeBranch string) map[string]interface{} { metadata := make(map[string]interface{}) for k, v := range req.Metadata { metadata[k] = v } + for k, v := range req.ExecutorConfig { + if isTrustedExecutorConfigKey(k) { + // Executor config wins for connection-routing keys so a malicious + // or buggy task metadata payload can't swap out the SSH host / + // pinned fingerprint and pivot the launch to a different target. + metadata[k] = v + continue + } + if _, exists := metadata[k]; !exists { + metadata[k] = v + } + } if mainRepoGitDir != "" { metadata[MetadataKeyMainRepoGitDir] = mainRepoGitDir } diff --git a/apps/backend/internal/agentruntime/runtime.go b/apps/backend/internal/agentruntime/runtime.go index 7f3cafd88e..aa6fc1aef7 100644 --- a/apps/backend/internal/agentruntime/runtime.go +++ b/apps/backend/internal/agentruntime/runtime.go @@ -16,6 +16,7 @@ const ( RuntimeDocker Runtime = "docker" RuntimeRemoteDocker Runtime = "remote_docker" RuntimeSprites Runtime = "sprites" + RuntimeSSH Runtime = "ssh" ) // IsContainerized reports whether the runtime hosts the agent diff --git a/apps/backend/internal/orchestrator/executor/executor_credentials.go b/apps/backend/internal/orchestrator/executor/executor_credentials.go index ac0f26e068..c26e354970 100644 --- a/apps/backend/internal/orchestrator/executor/executor_credentials.go +++ b/apps/backend/internal/orchestrator/executor/executor_credentials.go @@ -111,7 +111,7 @@ func (e *Executor) resolveRemoteCredentials(ctx context.Context, req *LaunchAgen // resolveAuthSecrets reads remote_auth_secrets from metadata and resolves secret values // into environment variables (e.g., gh_cli_env -> GITHUB_TOKEN). func (e *Executor) resolveAuthSecrets(ctx context.Context, req *LaunchAgentRequest, metadata map[string]interface{}) { - authSecretsJSON, _ := metadata["remote_auth_secrets"].(string) + authSecretsJSON, _ := metadata[profileKeyRemoteAuthSecrets].(string) if authSecretsJSON == "" { return } @@ -181,7 +181,7 @@ func (e *Executor) resolveGHCLIToken(req *LaunchAgentRequest, metadata map[strin return } - credsJSON, _ := metadata["remote_credentials"].(string) + credsJSON, _ := metadata[profileKeyRemoteCredentials].(string) if credsJSON == "" { return } diff --git a/apps/backend/internal/orchestrator/executor/executor_environment_reuse.go b/apps/backend/internal/orchestrator/executor/executor_environment_reuse.go index 79bf3cb752..99dfcd9205 100644 --- a/apps/backend/internal/orchestrator/executor/executor_environment_reuse.go +++ b/apps/backend/internal/orchestrator/executor/executor_environment_reuse.go @@ -133,9 +133,21 @@ func applyExecutorRunningMetadata(req *LaunchAgentRequest, running *models.Execu if metadata == nil { metadata = ensureLaunchMetadata(req) } - if _, exists := metadata[k]; !exists && lifecycle.ShouldPersistMetadataKey(k) { - metadata[k] = v + if _, exists := metadata[k]; exists { + continue } + if !lifecycle.ShouldPersistMetadataKey(k) { + continue + } + // Skip session-scoped runtime resources (PIDs, ports, session dirs). + // Carrying these across sibling sessions on the same task makes a + // fresh launch look like a same-session resume — see SSH executor's + // ResumeRemoteInstance — and the new session ends up sharing the + // previous session's agentctl process. + if lifecycle.IsSessionScopedMetadataKey(k) { + continue + } + metadata[k] = v } } diff --git a/apps/backend/internal/orchestrator/executor/executor_environment_test.go b/apps/backend/internal/orchestrator/executor/executor_environment_test.go index 5887ec57cd..af1348285b 100644 --- a/apps/backend/internal/orchestrator/executor/executor_environment_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_environment_test.go @@ -347,6 +347,113 @@ func TestReuseExistingEnvironment_EmptyEnvFieldsDoNothing(t *testing.T) { } } +// TestApplyExecutorRunningMetadata_SkipsSessionScopedKeys pins the guard +// that prevents a SECOND session on the same task from inheriting the FIRST +// session's session-scoped runtime resources — agentctl PID/port, remote +// session dir, local forward port. Without this filter, the SSH executor's +// ResumeRemoteInstance would interpret those keys as a resume hint and +// reattach to session-1's agentctl process, so session 2 would end up +// sharing session 1's ACP session and instance port and never finish its +// own initialize. +// +// Connection / task-environment-wide keys (host, port, user, fingerprint, +// remote task dir, workdir root, proxy jump) MUST still propagate so the +// second session connects to the same host and reuses the task dir. +func TestApplyExecutorRunningMetadata_SkipsSessionScopedKeys(t *testing.T) { + req := &LaunchAgentRequest{TaskID: "task-1"} + running := &models.ExecutorRunning{ + AgentExecutionID: "exec-prev", + Metadata: map[string]interface{}{ + // Connection config — should propagate. + lifecycle.MetadataKeySSHHost: "example.com", + lifecycle.MetadataKeySSHPort: "2200", + lifecycle.MetadataKeySSHUser: "deploy", + lifecycle.MetadataKeySSHHostFingerprint: "SHA256:aaa", + lifecycle.MetadataKeySSHRemoteTaskDir: "/home/deploy/.kandev/tasks/task-1", + lifecycle.MetadataKeySSHWorkdirRoot: "/home/deploy/.kandev", + lifecycle.MetadataKeySSHProxyJump: "bastion", + // Session-scoped runtime resources — must NOT propagate. + lifecycle.MetadataKeySSHRemoteSessionDir: "/home/deploy/.kandev/tasks/task-1/.kandev/sessions/sess-1", + lifecycle.MetadataKeySSHRemoteAgentctlPort: "41001", + lifecycle.MetadataKeySSHRemoteAgentctlPID: "12345", + lifecycle.MetadataKeySSHLocalForwardPort: "59123", + lifecycle.MetadataKeySSHRemoteAgentctlURL: "http://127.0.0.1:59123", + // Non-persistent key — must NOT propagate (not in persistentMetadataKeys). + "task_description": "session 1 prompt", + }, + } + + applyExecutorRunningMetadata(req, running) + + if req.PreviousExecutionID != "exec-prev" { + t.Errorf("PreviousExecutionID = %q, want exec-prev", req.PreviousExecutionID) + } + if req.Metadata == nil { + t.Fatal("req.Metadata is nil; expected propagated keys") + } + + propagated := []string{ + lifecycle.MetadataKeySSHHost, + lifecycle.MetadataKeySSHPort, + lifecycle.MetadataKeySSHUser, + lifecycle.MetadataKeySSHHostFingerprint, + lifecycle.MetadataKeySSHRemoteTaskDir, + lifecycle.MetadataKeySSHWorkdirRoot, + lifecycle.MetadataKeySSHProxyJump, + } + for _, k := range propagated { + if _, ok := req.Metadata[k]; !ok { + t.Errorf("expected connection key %q to propagate", k) + } + } + + sessionScoped := []string{ + lifecycle.MetadataKeySSHRemoteSessionDir, + lifecycle.MetadataKeySSHRemoteAgentctlPort, + lifecycle.MetadataKeySSHRemoteAgentctlPID, + lifecycle.MetadataKeySSHLocalForwardPort, + lifecycle.MetadataKeySSHRemoteAgentctlURL, + } + for _, k := range sessionScoped { + if v, ok := req.Metadata[k]; ok { + t.Errorf("session-scoped key %q leaked into sibling-session request (value=%v)", k, v) + } + if !lifecycle.IsSessionScopedMetadataKey(k) { + t.Errorf("IsSessionScopedMetadataKey(%q) = false; expected true", k) + } + } + + // task_description is not in persistentMetadataKeys at all, so the + // pre-existing ShouldPersistMetadataKey gate already drops it. + if _, ok := req.Metadata["task_description"]; ok { + t.Error("task_description (non-persistent) leaked into sibling-session request") + } +} + +// TestApplyExecutorRunningMetadata_RequestKeysWin documents that an explicit +// value already on the request (e.g. set by the caller from launch options) +// is not overwritten by the previous ExecutorRunning record. This applies to +// every persistent key, not just the connection ones. +func TestApplyExecutorRunningMetadata_RequestKeysWin(t *testing.T) { + req := &LaunchAgentRequest{ + TaskID: "task-1", + Metadata: map[string]interface{}{ + lifecycle.MetadataKeySSHHost: "user-override.example.com", + }, + } + running := &models.ExecutorRunning{ + Metadata: map[string]interface{}{ + lifecycle.MetadataKeySSHHost: "stale.example.com", + }, + } + + applyExecutorRunningMetadata(req, running) + + if got := req.Metadata[lifecycle.MetadataKeySSHHost]; got != "user-override.example.com" { + t.Errorf("ssh_host = %q, want user-override.example.com (request value should win)", got) + } +} + func TestExtractSandboxID(t *testing.T) { tests := []struct { name string diff --git a/apps/backend/internal/orchestrator/executor/executor_mocks_test.go b/apps/backend/internal/orchestrator/executor/executor_mocks_test.go index eb19984780..b22546a794 100644 --- a/apps/backend/internal/orchestrator/executor/executor_mocks_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_mocks_test.go @@ -219,6 +219,7 @@ type mockRepository struct { taskRepositories map[string]*models.TaskRepository repositories map[string]*models.Repository executors map[string]*models.Executor + executorProfiles map[string]*models.ExecutorProfile executorsRunning map[string]*models.ExecutorRunning taskEnvironments map[string]*models.TaskEnvironment taskEnvironmentRepos map[string][]*models.TaskEnvironmentRepo // env_id → rows @@ -243,6 +244,7 @@ func newMockRepository() *mockRepository { taskRepositories: make(map[string]*models.TaskRepository), repositories: make(map[string]*models.Repository), executors: make(map[string]*models.Executor), + executorProfiles: make(map[string]*models.ExecutorProfile), executorsRunning: make(map[string]*models.ExecutorRunning), taskEnvironments: make(map[string]*models.TaskEnvironment), taskEnvironmentRepos: make(map[string][]*models.TaskEnvironmentRepo), @@ -783,6 +785,9 @@ func (m *mockRepository) CreateExecutorProfile(ctx context.Context, profile *mod return nil } func (m *mockRepository) GetExecutorProfile(ctx context.Context, id string) (*models.ExecutorProfile, error) { + if p, ok := m.executorProfiles[id]; ok { + return p, nil + } return nil, nil } func (m *mockRepository) UpdateExecutorProfile(ctx context.Context, profile *models.ExecutorProfile) error { diff --git a/apps/backend/internal/orchestrator/executor/executor_state.go b/apps/backend/internal/orchestrator/executor/executor_state.go index f419a421a3..9ae623b350 100644 --- a/apps/backend/internal/orchestrator/executor/executor_state.go +++ b/apps/backend/internal/orchestrator/executor/executor_state.go @@ -214,26 +214,68 @@ func (e *Executor) applyProfile(ctx context.Context, profileID string, cfg *exec if policyJSON := strings.TrimSpace(profile.McpPolicy); policyJSON != "" { metadata["executor_mcp_policy"] = policyJSON } - if rulesJSON := profile.Config["sprites_network_policy_rules"]; rulesJSON != "" { - metadata["sprites_network_policy_rules"] = rulesJSON + applyProfileConfigToMetadata(profile.Config, metadata) +} + +// Profile.Config / metadata keys shared with executor_credentials.go. +// Hoisted to constants so the table below doesn't duplicate string +// literals that exist elsewhere in the package. +const ( + profileKeyRemoteCredentials = "remote_credentials" + profileKeyRemoteAuthSecrets = "remote_auth_secrets" +) + +// profileConfigPassthroughKeys are profile.Config keys copied verbatim +// into launch metadata under the same key when non-empty. An empty +// profile value leaves any task-supplied value in place. +var profileConfigPassthroughKeys = []string{ + "sprites_network_policy_rules", + profileKeyRemoteCredentials, + profileKeyRemoteAuthSecrets, + "remote_auth_target_home", + "git_user_name", + "git_user_email", +} + +// profileConfigRenameKeys maps a profile.Config key to a different +// metadata key when the names diverge. Same non-empty semantics as +// profileConfigPassthroughKeys. +var profileConfigRenameKeys = map[string]string{ + "image_tag": "image_tag_override", +} + +// profileConfigAuthoritativeKeys are profile.Config keys whose value is +// the source of truth for launch metadata — they overwrite any +// task-supplied value, including with an empty string. The reader-side +// helpers (e.g. SSHExecutor.workdirRoot, sshShellFromMetadata) handle +// the empty fall-through to a default. Without this, a task that +// supplies ssh_workdir_root or ssh_shell in its Metadata wins when the +// profile has no value set — that's a redirect vector (workdir target, +// login shell that runs every remote command). +var profileConfigAuthoritativeKeys = []string{ + lifecycle.MetadataKeySSHWorkdirRoot, + lifecycle.MetadataKeySSHShell, +} + +// applyProfileConfigToMetadata projects profile.Config keys into the +// launch metadata. Pulled out so the policy (passthrough vs rename vs +// authoritative) is declarative and testable in isolation. +func applyProfileConfigToMetadata(profileConfig map[string]string, metadata map[string]interface{}) { + for _, k := range profileConfigPassthroughKeys { + if v := profileConfig[k]; v != "" { + metadata[k] = v + } } - if credJSON := profile.Config["remote_credentials"]; credJSON != "" { - metadata["remote_credentials"] = credJSON + for src, dst := range profileConfigRenameKeys { + if v := profileConfig[src]; v != "" { + metadata[dst] = v + } } - if authSecretsJSON := profile.Config["remote_auth_secrets"]; authSecretsJSON != "" { - metadata["remote_auth_secrets"] = authSecretsJSON - } - if remoteAuthHome := profile.Config["remote_auth_target_home"]; remoteAuthHome != "" { - metadata["remote_auth_target_home"] = remoteAuthHome - } - if gitUserName := profile.Config["git_user_name"]; gitUserName != "" { - metadata["git_user_name"] = gitUserName - } - if gitUserEmail := profile.Config["git_user_email"]; gitUserEmail != "" { - metadata["git_user_email"] = gitUserEmail - } - if imageTag := profile.Config["image_tag"]; imageTag != "" { - metadata["image_tag_override"] = imageTag + for _, k := range profileConfigAuthoritativeKeys { + // Unconditional set: an empty profile value overwrites any + // task-supplied value in metadata. The reader-side fall-through + // to a default handles the empty case. + metadata[k] = profileConfig[k] } } diff --git a/apps/backend/internal/orchestrator/executor/executor_state_test.go b/apps/backend/internal/orchestrator/executor/executor_state_test.go new file mode 100644 index 0000000000..c8a479821d --- /dev/null +++ b/apps/backend/internal/orchestrator/executor/executor_state_test.go @@ -0,0 +1,139 @@ +package executor + +import ( + "context" + "testing" + + "github.com/kandev/kandev/internal/agent/runtime/lifecycle" + "github.com/kandev/kandev/internal/task/models" +) + +// TestResolveExecutorConfig_PropagatesSSHShell pins the contract that the +// shell selection saved on an SSH executor profile lands in launch metadata +// under MetadataKeySSHShell. Without this, sshShellFromMetadata returns +// empty and WrapLoginShell silently falls back to bash — so a user who +// installed npx under zsh+nvm sees a misleading "npx not found" preflight +// error while the readiness probe (which takes shell straight from the +// request body) reports the agent as available. +func TestResolveExecutorConfig_PropagatesSSHShell(t *testing.T) { + repo := newMockRepository() + exec := newTestExecutor(t, &mockAgentManager{}, repo) + + repo.executors["exec-ssh-1"] = &models.Executor{ + ID: "exec-ssh-1", + Type: models.ExecutorTypeSSH, + Config: map[string]string{ + "ssh_host": "example.com", + }, + } + repo.executorProfiles["prof-1"] = &models.ExecutorProfile{ + ID: "prof-1", + ExecutorID: "exec-ssh-1", + Config: map[string]string{ + "ssh_shell": "zsh", + }, + } + + cfg := exec.resolveExecutorConfig(context.Background(), "exec-ssh-1", "ws-1", map[string]interface{}{ + "executor_profile_id": "prof-1", + }) + + got, _ := cfg.Metadata[lifecycle.MetadataKeySSHShell].(string) + if got != "zsh" { + t.Fatalf("metadata[%q] = %q, want %q", lifecycle.MetadataKeySSHShell, got, "zsh") + } +} + +// TestResolveExecutorConfig_AuthoritativeSSHKeys_ClobberTaskMetadata pins +// the security contract from B1 of the thermo-nuclear review: a task that +// supplies ssh_workdir_root or ssh_shell in its metadata MUST be +// overridden by the profile's value — including when the profile has no +// value set (empty wins). Without this, a task can redirect every remote +// command on launch to a host path or login shell of its choosing. +func TestResolveExecutorConfig_AuthoritativeSSHKeys_ClobberTaskMetadata(t *testing.T) { + cases := []struct { + name string + key string + profileValue string + taskValue string + wantMetadata string // empty means the key should be present with "" + }{ + { + name: "profile_wins_when_set", + key: lifecycle.MetadataKeySSHWorkdirRoot, + profileValue: "/srv/kandev", + taskValue: "/etc", + wantMetadata: "/srv/kandev", + }, + { + name: "empty_profile_clobbers_task", + key: lifecycle.MetadataKeySSHWorkdirRoot, + profileValue: "", + taskValue: "/etc", + wantMetadata: "", + }, + { + name: "shell_profile_wins_when_set", + key: lifecycle.MetadataKeySSHShell, + profileValue: "zsh", + taskValue: "bash --norc", + wantMetadata: "zsh", + }, + { + name: "empty_shell_profile_clobbers_task", + key: lifecycle.MetadataKeySSHShell, + profileValue: "", + taskValue: "bash --norc", + wantMetadata: "", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + repo := newMockRepository() + exec := newTestExecutor(t, &mockAgentManager{}, repo) + repo.executors["exec-ssh-1"] = &models.Executor{ID: "exec-ssh-1", Type: models.ExecutorTypeSSH} + repo.executorProfiles["prof-1"] = &models.ExecutorProfile{ + ID: "prof-1", + ExecutorID: "exec-ssh-1", + Config: map[string]string{tc.key: tc.profileValue}, + } + + cfg := exec.resolveExecutorConfig(context.Background(), "exec-ssh-1", "ws-1", map[string]interface{}{ + "executor_profile_id": "prof-1", + tc.key: tc.taskValue, + }) + + got, _ := cfg.Metadata[tc.key].(string) + if got != tc.wantMetadata { + t.Fatalf("metadata[%q] = %q, want %q (profile=%q task=%q)", + tc.key, got, tc.wantMetadata, tc.profileValue, tc.taskValue) + } + }) + } +} + +// TestResolveExecutorConfig_PassthroughKeys_DontClobberTaskMetadata pins +// the inverse: ordinary passthrough keys (git_user_name etc.) keep their +// historical "only set if non-empty" semantics. A task-supplied value +// survives when the profile leaves the key blank. This is intentional +// for non-security-sensitive keys; flag explicitly if it ever needs to +// change. +func TestResolveExecutorConfig_PassthroughKeys_DontClobberTaskMetadata(t *testing.T) { + repo := newMockRepository() + exec := newTestExecutor(t, &mockAgentManager{}, repo) + repo.executors["exec-ssh-1"] = &models.Executor{ID: "exec-ssh-1", Type: models.ExecutorTypeSSH} + repo.executorProfiles["prof-1"] = &models.ExecutorProfile{ + ID: "prof-1", + ExecutorID: "exec-ssh-1", + Config: map[string]string{}, // profile has no git_user_name + } + + cfg := exec.resolveExecutorConfig(context.Background(), "exec-ssh-1", "ws-1", map[string]interface{}{ + "executor_profile_id": "prof-1", + "git_user_name": "Task Author", + }) + + if got, _ := cfg.Metadata["git_user_name"].(string); got != "Task Author" { + t.Fatalf("metadata[git_user_name] = %q, want task value to survive empty profile", got) + } +} diff --git a/apps/backend/internal/ssh/handlers.go b/apps/backend/internal/ssh/handlers.go new file mode 100644 index 0000000000..372994b6f8 --- /dev/null +++ b/apps/backend/internal/ssh/handlers.go @@ -0,0 +1,682 @@ +// Package ssh provides HTTP and WebSocket handlers for SSH executor +// management: test-connection-and-fingerprint, list active SSH sessions for +// an executor. +package ssh + +import ( + "context" + "errors" + "fmt" + "net/http" + "strconv" + "strings" + "time" + + "github.com/gin-gonic/gin" + "go.uber.org/zap" + "golang.org/x/crypto/ssh" + + "github.com/kandev/kandev/internal/agent/agents" + "github.com/kandev/kandev/internal/agent/registry" + "github.com/kandev/kandev/internal/agent/runtime/lifecycle" + "github.com/kandev/kandev/internal/agentruntime" + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/task/models" + ws "github.com/kandev/kandev/pkg/websocket" +) + +const errorJSONKey = "error" + +// ExecutorRunningLister is the narrow repository slice we need to surface +// active SSH sessions. We list all running executors and resolve each row's +// executor_id by looking up the task_session — the lifecycle manager doesn't +// populate ExecutorRunning.ExecutorID directly (it's only carried forward from +// prior rows on resume), so joining through the session is the source of +// truth. +type ExecutorRunningLister interface { + ListExecutorsRunning(ctx context.Context) ([]*models.ExecutorRunning, error) + GetTaskSession(ctx context.Context, id string) (*models.TaskSession, error) +} + +// ExecutorFetcher looks up a single executor by id. Needed by the +// agent-readiness probe to translate executor.Config into an SSHTarget. +type ExecutorFetcher interface { + GetExecutor(ctx context.Context, id string) (*models.Executor, error) +} + +// AgentLister is the narrow slice of *registry.Registry that the readiness +// probe needs: walk every enabled agent so we can probe each one's first +// command-line token. Defined as an interface so handler tests can pass a +// fake set of agents without bringing the full registry in. +type AgentLister interface { + ListEnabled() []agents.Agent +} + +// Handler exposes /api/v1/ssh routes used by the settings UI. +type Handler struct { + repo ExecutorRunningLister + executorFetcher ExecutorFetcher + agents AgentLister + resolver *lifecycle.AgentctlResolver + logger *logger.Logger +} + +// NewHandler builds an SSH HTTP/WS handler. +func NewHandler( + repo ExecutorRunningLister, + executorFetcher ExecutorFetcher, + agents AgentLister, + resolver *lifecycle.AgentctlResolver, + log *logger.Logger, +) *Handler { + return &Handler{ + repo: repo, + executorFetcher: executorFetcher, + agents: agents, + resolver: resolver, + logger: log.WithFields(zap.String("component", "ssh-handler")), + } +} + +// RegisterRoutes wires the SSH routes onto the given gin engine + WS dispatcher. +func RegisterRoutes( + router *gin.Engine, + dispatcher *ws.Dispatcher, + repo ExecutorRunningLister, + executorFetcher ExecutorFetcher, + registry *registry.Registry, + resolver *lifecycle.AgentctlResolver, + log *logger.Logger, +) { + var agentLister AgentLister + if registry != nil { + agentLister = registry + } + h := NewHandler(repo, executorFetcher, agentLister, resolver, log) + h.registerHTTP(router) + h.registerWS(dispatcher) +} + +func (h *Handler) registerHTTP(router *gin.Engine) { + api := router.Group("/api/v1/ssh") + api.POST("/test", h.httpTest) + api.GET("/executors/:id/sessions", h.httpListSessions) + api.POST("/executors/:id/probe-agents", h.httpProbeAgents) + api.POST("/executors/:id/probe-shells", h.httpProbeShells) +} + +func (h *Handler) registerWS(dispatcher *ws.Dispatcher) { + dispatcher.RegisterFunc("ssh.test", h.wsTest) + dispatcher.RegisterFunc("ssh.sessions.list", h.wsListSessions) +} + +// --- Request / response shapes --- + +// TestRequest is the body of POST /api/v1/ssh/test. +type TestRequest struct { + Name string `json:"name"` + HostAlias string `json:"host_alias,omitempty"` + Host string `json:"host,omitempty"` + Port int `json:"port,omitempty"` + User string `json:"user,omitempty"` + IdentitySrc string `json:"identity_source,omitempty"` // "agent" or "file" + IdentityFile string `json:"identity_file,omitempty"` + ProxyJump string `json:"proxy_jump,omitempty"` +} + +// TestStep is one entry in the test result step list. +type TestStep struct { + Name string `json:"name"` + DurationMs int64 `json:"duration_ms"` + Success bool `json:"success"` + Output string `json:"output,omitempty"` + Error string `json:"error,omitempty"` +} + +// TestResult is the response of POST /api/v1/ssh/test. The UI requires +// `success` and `fingerprint` before letting the user tick "Trust this host". +type TestResult struct { + Success bool `json:"success"` + Fingerprint string `json:"fingerprint,omitempty"` + UnameAll string `json:"uname_all,omitempty"` + Arch string `json:"arch,omitempty"` + GitVersion string `json:"git_version,omitempty"` + AgentctlAction string `json:"agentctl_action,omitempty"` // "cached" | "needs_upload" | "skipped" + Steps []TestStep `json:"steps"` + TotalDurationMs int64 `json:"total_duration_ms"` + Error string `json:"error,omitempty"` +} + +// SessionRow is one entry in the active-sessions table. +type SessionRow struct { + SessionID string `json:"session_id"` + TaskID string `json:"task_id"` + TaskTitle string `json:"task_title,omitempty"` + Host string `json:"host"` + User string `json:"user,omitempty"` + RemoteTaskDir string `json:"remote_task_dir,omitempty"` + RemoteAgentPort int `json:"remote_agentctl_port,omitempty"` + LocalForwardPort int `json:"local_forward_port,omitempty"` + Status string `json:"status"` + UptimeSeconds int64 `json:"uptime_seconds"` + CreatedAt string `json:"created_at"` +} + +// --- HTTP handlers --- + +func (h *Handler) httpTest(c *gin.Context) { + var req TestRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{errorJSONKey: "invalid request body: " + err.Error()}) + return + } + result := h.runTest(c.Request.Context(), req) + c.JSON(http.StatusOK, result) +} + +func (h *Handler) httpListSessions(c *gin.Context) { + id := strings.TrimSpace(c.Param("id")) + if id == "" { + c.JSON(http.StatusBadRequest, gin.H{errorJSONKey: "executor id required"}) + return + } + rows, err := h.listSessions(c.Request.Context(), id) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{errorJSONKey: err.Error()}) + return + } + c.JSON(http.StatusOK, rows) +} + +// AgentReadinessRow reports the readiness of one agent on the remote host: +// whether the first token of agent.BuildCommand(...).Args() resolves on the +// remote's $PATH, and the install hint sourced from agent.InstallScript() so +// the UI can render a copy-button instead of forcing the user to look up +// per-agent install commands. +type AgentReadinessRow struct { + AgentID string `json:"agent_id"` + AgentName string `json:"agent_name"` + Binary string `json:"binary"` + Available bool `json:"available"` + ResolvedAt string `json:"resolved_at,omitempty"` // command -v stdout, when found + InstallHint string `json:"install_hint,omitempty"` + Error string `json:"error,omitempty"` +} + +// AgentReadinessResponse is the body of POST /api/v1/ssh/executors/:id/probe-agents. +// Shell echoes back which login shell the probe ran under so the UI can +// label results that depend on shell-init PATH (nvm/asdf etc.). +type AgentReadinessResponse struct { + Host string `json:"host"` + Shell string `json:"shell,omitempty"` + DurationMs int64 `json:"duration_ms"` + Rows []AgentReadinessRow `json:"rows"` +} + +// ProbeAgentsRequest is the optional body of POST /api/v1/ssh/executors/:id/probe-agents. +// Shell, when set, names the login shell the probe runs under (e.g. "bash", +// "zsh"). Empty falls back to bash — see lifecycle.WrapLoginShell. +type ProbeAgentsRequest struct { + Shell string `json:"shell,omitempty"` +} + +// httpProbeAgents SSHs to the given executor's host and runs `command -v` +// for each enabled agent's first command-line token. Drives the "Available +// agents on this host" card on the executor's profile-edit page. +// +// Reads an optional `{shell}` body so the UI can re-probe under whichever +// shell the user picked in the shell selector (PATH depends on shell init +// files, so the same host shows different agents under bash vs sh). +func (h *Handler) httpProbeAgents(c *gin.Context) { + id := strings.TrimSpace(c.Param("id")) + if id == "" { + c.JSON(http.StatusBadRequest, gin.H{errorJSONKey: "executor id required"}) + return + } + if h.executorFetcher == nil || h.agents == nil { + c.JSON(http.StatusServiceUnavailable, gin.H{errorJSONKey: "agent readiness not wired"}) + return + } + var body ProbeAgentsRequest + // Body is optional — empty POST is fine and falls back to the default + // shell. Don't fail loudly on a malformed body; just ignore the shell. + _ = c.ShouldBindJSON(&body) + resp, status, err := h.probeAgents(c.Request.Context(), id, body.Shell) + if err != nil { + c.JSON(status, gin.H{errorJSONKey: err.Error()}) + return + } + c.JSON(http.StatusOK, resp) +} + +// probeAgents is the testable core of httpProbeAgents. +func (h *Handler) probeAgents(ctx context.Context, executorID, shell string) (*AgentReadinessResponse, int, error) { + started := time.Now() + target, status, err := h.resolveSSHTarget(ctx, executorID) + if err != nil { + return nil, status, err + } + client, err := lifecycle.DialSSH(ctx, target) + if err != nil { + return nil, http.StatusBadGateway, fmt.Errorf("ssh: dial %s: %w", target.Host, err) + } + defer func() { _ = client.Close() }() + + rows := h.probeAgentsOnClient(ctx, client, shell) + return &AgentReadinessResponse{ + Host: target.Host, + Shell: shell, + DurationMs: time.Since(started).Milliseconds(), + Rows: rows, + }, http.StatusOK, nil +} + +// probeAgentsOnClient walks every enabled agent and runs command -v over +// the existing client. Public seam for a unit test that fakes the SSH side. +func (h *Handler) probeAgentsOnClient(ctx context.Context, client *ssh.Client, shell string) []AgentReadinessRow { + enabled := h.agents.ListEnabled() + rows := make([]AgentReadinessRow, 0, len(enabled)) + for _, ag := range enabled { + rows = append(rows, probeOneAgent(ctx, client, shell, ag)) + } + return rows +} + +func probeOneAgent(ctx context.Context, client *ssh.Client, shell string, ag agents.Agent) AgentReadinessRow { + row := AgentReadinessRow{ + AgentID: ag.ID(), + AgentName: ag.Name(), + InstallHint: strings.TrimSpace(ag.InstallScript()), + } + cmd := ag.BuildCommand(agents.CommandOptions{Runtime: agentruntime.RuntimeSSH}) + args := cmd.Args() + if len(args) == 0 { + row.Error = "agent did not emit a command" + return row + } + row.Binary = args[0] + resolved, err := lifecycle.ProbeRemoteBinary(ctx, client, shell, row.Binary) + if err != nil { + row.Error = err.Error() + return row + } + if resolved == "" { + return row + } + row.Available = true + row.ResolvedAt = resolved + return row +} + +// candidateShells is the closed set the probe walks. Common Linux shells +// plus dash; expanding this is cheap (one extra `command -v` per entry). +var candidateShells = []string{"bash", "zsh", "sh", "fish", "dash"} + +// ProbeShellsResponse is the body of POST /api/v1/ssh/executors/:id/probe-shells. +type ProbeShellsResponse struct { + Host string `json:"host"` + DurationMs int64 `json:"duration_ms"` + Available []string `json:"available"` +} + +// httpProbeShells SSHs to the host and reports which of a small set of +// canonical login shells resolves on the remote's $PATH. Drives the shell +// selector on the SSH profile-edit page so users pick a shell that actually +// exists. The shell choice then governs every subsequent SSH command — +// including the agent-readiness probe and the actual agentctl launch. +func (h *Handler) httpProbeShells(c *gin.Context) { + id := strings.TrimSpace(c.Param("id")) + if id == "" { + c.JSON(http.StatusBadRequest, gin.H{errorJSONKey: "executor id required"}) + return + } + if h.executorFetcher == nil { + c.JSON(http.StatusServiceUnavailable, gin.H{errorJSONKey: "executor lookup not wired"}) + return + } + resp, status, err := h.probeShells(c.Request.Context(), id) + if err != nil { + c.JSON(status, gin.H{errorJSONKey: err.Error()}) + return + } + c.JSON(http.StatusOK, resp) +} + +func (h *Handler) probeShells(ctx context.Context, executorID string) (*ProbeShellsResponse, int, error) { + started := time.Now() + target, status, err := h.resolveSSHTarget(ctx, executorID) + if err != nil { + return nil, status, err + } + client, err := lifecycle.DialSSH(ctx, target) + if err != nil { + return nil, http.StatusBadGateway, fmt.Errorf("ssh: dial %s: %w", target.Host, err) + } + defer func() { _ = client.Close() }() + + available := make([]string, 0, len(candidateShells)) + for _, shell := range candidateShells { + // Probe without login-shell wrapping — we're checking whether the + // shell binary itself exists in PATH, which is independent of the + // shell-init files we'd source. /bin/sh -c 'command -v bash' is + // fine. + resolved, probeErr := lifecycle.ProbeRemoteBinary(ctx, client, "sh", shell) + if probeErr != nil { + h.logger.Warn("probe-shells: SSH probe failed", + zap.String("shell", shell), zap.Error(probeErr)) + continue + } + if resolved != "" { + available = append(available, shell) + } + } + return &ProbeShellsResponse{ + Host: target.Host, + DurationMs: time.Since(started).Milliseconds(), + Available: available, + }, http.StatusOK, nil +} + +// resolveSSHTarget looks up the executor by id and projects its config +// into an SSHTarget. Shared across probe entry points. +func (h *Handler) resolveSSHTarget(ctx context.Context, executorID string) (*lifecycle.SSHTarget, int, error) { + executor, err := h.executorFetcher.GetExecutor(ctx, executorID) + if err != nil { + // Genuine "row doesn't exist" → 404. Transport-level failures + // (storage outage, context cancel, …) keep their server-error + // shape so the UI can distinguish "executor was deleted" from + // "the lookup itself just broke". + if errors.Is(err, models.ErrExecutorNotFound) { + return nil, http.StatusNotFound, fmt.Errorf("executor %q not found: %w", executorID, err) + } + return nil, http.StatusInternalServerError, fmt.Errorf("look up executor %q: %w", executorID, err) + } + if executor == nil || executor.Type != models.ExecutorTypeSSH { + return nil, http.StatusBadRequest, fmt.Errorf("executor %q is not an SSH executor", executorID) + } + target, err := sshTargetFromExecutorConfig(executor.Config) + if err != nil { + return nil, http.StatusBadRequest, err + } + return target, http.StatusOK, nil +} + +// sshTargetFromExecutorConfig projects an executor.Config into the +// SSHConnConfig the dialer expects. Keeps the handler decoupled from +// lifecycle's internal metadata-vs-config representation. +func sshTargetFromExecutorConfig(cfg map[string]string) (*lifecycle.SSHTarget, error) { + if cfg == nil { + return nil, fmt.Errorf("ssh executor has no config") + } + port := 0 + if p := strings.TrimSpace(cfg["ssh_port"]); p != "" { + n, err := strconv.Atoi(p) + if err != nil || n < 1 || n > 65535 { + return nil, fmt.Errorf("invalid ssh_port %q", p) + } + port = n + } + return lifecycle.ResolveSSHTarget(lifecycle.SSHConnConfig{ + HostAlias: cfg["ssh_host_alias"], + Host: cfg["ssh_host"], + Port: port, + User: cfg["ssh_user"], + IdentitySource: lifecycle.SSHIdentitySource(cfg["ssh_identity_source"]), + IdentityFile: cfg["ssh_identity_file"], + ProxyJump: cfg["ssh_proxy_jump"], + PinnedFingerprint: cfg["ssh_host_fingerprint"], + }) +} + +// --- WS handlers --- + +func (h *Handler) wsTest(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req TestRequest + if err := msg.ParsePayload(&req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "invalid payload", nil) + } + return ws.NewResponse(msg.ID, msg.Action, h.runTest(ctx, req)) +} + +func (h *Handler) wsListSessions(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var payload struct { + ExecutorID string `json:"executor_id"` + } + if err := msg.ParsePayload(&payload); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "invalid payload", nil) + } + if payload.ExecutorID == "" { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "executor_id required", nil) + } + rows, err := h.listSessions(ctx, payload.ExecutorID) + if err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, err.Error(), nil) + } + return ws.NewResponse(msg.ID, msg.Action, rows) +} + +// --- Business logic --- + +// runTest performs the SSH-test sequence: dial without a pinned fingerprint +// (so the host key is observed but not yet trusted), probe remote OS/arch, +// then dry-run the agentctl-upload-or-skip step. The UI gates the "Trust this +// host" checkbox on result.Success && result.Fingerprint != "". +func (h *Handler) runTest(ctx context.Context, req TestRequest) *TestResult { + start := time.Now() + result := &TestResult{} + finalize := func() *TestResult { + result.TotalDurationMs = time.Since(start).Milliseconds() + return result + } + + target, err := h.testResolveTarget(req, result) + if err != nil { + return finalize() + } + + client, err := h.testHandshake(ctx, target, result) + if err != nil { + return finalize() + } + defer func() { _ = client.Close() }() + + infoCtx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + + if err := h.testProbeAndArch(infoCtx, client, result); err != nil { + return finalize() + } + h.testAgentctlCache(infoCtx, client, result) + + result.Success = true + return finalize() +} + +func (h *Handler) testResolveTarget(req TestRequest, result *TestResult) (*lifecycle.SSHTarget, error) { + target, err := lifecycle.ResolveSSHTarget(lifecycle.SSHConnConfig{ + HostAlias: req.HostAlias, + Host: req.Host, + Port: req.Port, + User: req.User, + IdentitySource: lifecycle.SSHIdentitySource(req.IdentitySrc), + IdentityFile: req.IdentityFile, + ProxyJump: req.ProxyJump, + // Deliberately empty PinnedFingerprint — first-connect / test mode. + }) + if err != nil { + result.Steps = append(result.Steps, TestStep{Name: "Resolve target", Success: false, Error: err.Error()}) + result.Error = err.Error() + return nil, err + } + result.Steps = append(result.Steps, TestStep{ + Name: "Resolve target", + Success: true, + Output: fmt.Sprintf("%s@%s:%d (identity=%s)", target.User, target.Host, target.Port, target.IdentitySource), + }) + return target, nil +} + +func (h *Handler) testHandshake( + ctx context.Context, target *lifecycle.SSHTarget, result *TestResult, +) (*ssh.Client, error) { + stepStart := time.Now() + client, err := lifecycle.DialSSH(ctx, target) + step := TestStep{Name: "SSH handshake", DurationMs: time.Since(stepStart).Milliseconds()} + if err != nil { + step.Success = false + step.Error = err.Error() + result.Steps = append(result.Steps, step) + result.Error = err.Error() + return nil, err + } + step.Success = true + result.Fingerprint = target.ObservedFingerprint + step.Output = "fingerprint=" + result.Fingerprint + result.Steps = append(result.Steps, step) + return client, nil +} + +func (h *Handler) testProbeAndArch(ctx context.Context, client *ssh.Client, result *TestResult) error { + info, err := lifecycle.SSHProbeRemote(ctx, client) + if err != nil { + result.Steps = append(result.Steps, TestStep{Name: "Probe remote", Success: false, Error: err.Error()}) + result.Error = err.Error() + return err + } + result.UnameAll = info.UnameAll + result.Arch = info.Arch + result.GitVersion = info.GitVer + result.Steps = append(result.Steps, TestStep{Name: "Probe remote", Success: true, Output: info.UnameAll}) + + if err := lifecycle.SSHRequireSupportedArch(info.Arch); err != nil { + result.Steps = append(result.Steps, TestStep{Name: "Verify arch", Success: false, Error: err.Error()}) + result.Error = err.Error() + return err + } + result.Steps = append(result.Steps, TestStep{Name: "Verify arch", Success: true, Output: info.Arch}) + return nil +} + +// populateSessionMetadata copies SSH-specific keys from an ExecutorRunning row +// into a SessionRow. Pure mapping; no I/O. Lives here (not on the row) so the +// metadata key names stay co-located with their consumer. +func populateSessionMetadata(row *SessionRow, md map[string]interface{}) { + if md == nil { + return + } + row.Host, _ = md[lifecycle.MetadataKeySSHHost].(string) + row.User, _ = md[lifecycle.MetadataKeySSHUser].(string) + row.RemoteTaskDir, _ = md[lifecycle.MetadataKeySSHRemoteTaskDir].(string) + row.RemoteAgentPort = intFromMetadata(md, lifecycle.MetadataKeySSHRemoteAgentctlPort) + row.LocalForwardPort = intFromMetadata(md, lifecycle.MetadataKeySSHLocalForwardPort) +} + +// intFromMetadata reads a numeric metadata value. The SSH executor stores +// ports/pids as strings (strconv.Itoa) at write time, but a JSON round-trip +// through ExecutorRunning.Metadata can land them back as float64. Tolerate +// the obvious shapes rather than silently returning 0 on a real value. +func intFromMetadata(md map[string]interface{}, key string) int { + switch v := md[key].(type) { + case int: + return v + case int64: + return int(v) + case float64: + return int(v) + case string: + n, err := strconv.Atoi(v) + if err != nil { + return 0 + } + return n + default: + return 0 + } +} + +func (h *Handler) testAgentctlCache(ctx context.Context, client *ssh.Client, result *TestResult) { + upStepStart := time.Now() + cached, err := lifecycle.SSHCheckAgentctlCached(ctx, client, h.resolver) + upStep := TestStep{Name: "Verify agentctl cache", DurationMs: time.Since(upStepStart).Milliseconds()} + if err != nil { + upStep.Success = false + upStep.Error = err.Error() + result.Steps = append(result.Steps, upStep) + result.AgentctlAction = "skipped" + return + } + upStep.Success = true + if cached { + upStep.Output = "cached" + result.AgentctlAction = "cached" + } else { + upStep.Output = "will upload on first launch" + // "needs_upload": the binary is absent on the remote and will be + // uploaded the first time a session launches; the test endpoint + // only probes for the sha256 sidecar, it doesn't push the binary. + // Anything that wants to assert an upload actually happened needs + // to drive a real launch. + result.AgentctlAction = "needs_upload" + } + result.Steps = append(result.Steps, upStep) +} + +// listSessions returns the active SSH-runtime sessions for the given executor. +// Backed by the ExecutorRunning table; each row carries the SSH metadata we +// persisted in CreateInstance. The executor binding is resolved by looking up +// the TaskSession associated with each row — the row itself has an +// ExecutorID column but the lifecycle manager does not populate it for fresh +// executions, so it's unreliable as a filter. +// +// The runtime filter runs before the session lookup, so the per-row +// GetTaskSession call only fires for SSH rows — typically <10 per executor. +// If that count ever grows materially we should add a batch List by +// session-ID method to the repo and resolve in one query. +func (h *Handler) listSessions(ctx context.Context, executorID string) ([]SessionRow, error) { + rows, err := h.repo.ListExecutorsRunning(ctx) + if err != nil { + return nil, fmt.Errorf("list ssh sessions: %w", err) + } + out := make([]SessionRow, 0, len(rows)) + now := time.Now() + for _, run := range rows { + if run == nil { + continue + } + if run.Runtime != agentruntime.RuntimeSSH { + continue + } + if !h.sessionBelongsToExecutor(ctx, run, executorID) { + continue + } + row := SessionRow{ + SessionID: run.SessionID, + TaskID: run.TaskID, + Status: run.Status, + CreatedAt: run.CreatedAt.Format(time.RFC3339), + UptimeSeconds: int64(now.Sub(run.CreatedAt).Seconds()), + } + populateSessionMetadata(&row, run.Metadata) + out = append(out, row) + } + return out, nil +} + +// sessionBelongsToExecutor returns true when the given ExecutorRunning row was +// launched under the given executor config. Resolves via the task_session +// row because ExecutorRunning.ExecutorID is unpopulated on fresh executions. +// Falls back to the row's own ExecutorID when the session lookup fails, so +// resume / recovery paths (where the row has it) still match. +func (h *Handler) sessionBelongsToExecutor(ctx context.Context, run *models.ExecutorRunning, executorID string) bool { + if run.ExecutorID == executorID && executorID != "" { + return true + } + if run.SessionID == "" { + return false + } + session, err := h.repo.GetTaskSession(ctx, run.SessionID) + if err != nil || session == nil { + return false + } + return session.ExecutorID == executorID +} diff --git a/apps/backend/internal/task/handlers/executor_profile_handlers.go b/apps/backend/internal/task/handlers/executor_profile_handlers.go index 2596673aec..77e3231df0 100644 --- a/apps/backend/internal/task/handlers/executor_profile_handlers.go +++ b/apps/backend/internal/task/handlers/executor_profile_handlers.go @@ -52,6 +52,10 @@ func (h *ExecutorProfileHandlers) registerHTTP(router *gin.Engine) { api.GET("/executors/:id/profiles/:profileId", h.httpGetProfile) api.PATCH("/executors/:id/profiles/:profileId", h.httpUpdateProfile) api.DELETE("/executors/:id/profiles/:profileId", h.httpDeleteProfile) + // Top-level DELETE: profile IDs are globally unique, so callers that + // only have the profile id (CLI, settings UI, test helpers) don't need + // to plumb the executor id through. The handler is identical otherwise. + api.DELETE("/executor-profiles/:profileId", h.httpDeleteProfile) } func (h *ExecutorProfileHandlers) registerWS(dispatcher *ws.Dispatcher) { diff --git a/apps/backend/internal/task/handlers/task_environment_handlers.go b/apps/backend/internal/task/handlers/task_environment_handlers.go index 1c142abad0..041f3a53f8 100644 --- a/apps/backend/internal/task/handlers/task_environment_handlers.go +++ b/apps/backend/internal/task/handlers/task_environment_handlers.go @@ -52,6 +52,17 @@ func (h *TaskHandlers) httpGetTaskEnvironmentLive(c *gin.Context) { if live != nil { resp["container"] = live } + // SSH-backed environments get a parallel `ssh` block with the runtime + // fields (host/user/workdir/forward ports) the popover surfaces. Service + // returns nil on non-SSH environments, so this is a no-op for the other + // executor types. + if ssh, err := h.service.GetSSHLiveStatus(c.Request.Context(), taskID); err != nil { + h.logger.Warn("failed to fetch live ssh status", + zap.String("task_id", taskID), + zap.Error(err)) + } else if ssh != nil { + resp["ssh"] = ssh + } c.JSON(http.StatusOK, resp) } diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index e1ff2a4f14..4a2cc224a0 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -13,6 +13,13 @@ import ( // ErrExecutorRunningNotFound is returned when no executor running record exists for a session. var ErrExecutorRunningNotFound = errors.New("executor running not found") +// ErrExecutorNotFound is returned by the executor repository when no +// executor row exists for the given ID. Callers should use errors.Is to +// distinguish "row doesn't exist" (404 semantically) from transport-level +// failures (storage outage, timeout, ctx cancel) that happen to also +// surface from the same lookup. +var ErrExecutorNotFound = errors.New("executor not found") + // ErrExecutionRotated is returned by CAS-style updates on executors_running when the // row's agent_execution_id has rotated since the caller observed it. Indicates the // caller's write target a now-defunct execution and should be discarded — typically @@ -513,6 +520,7 @@ const ( ExecutorTypeLocalDocker ExecutorType = "local_docker" ExecutorTypeRemoteDocker ExecutorType = "remote_docker" ExecutorTypeSprites ExecutorType = "sprites" + ExecutorTypeSSH ExecutorType = "ssh" ExecutorTypeMockRemote ExecutorType = "mock_remote" ) @@ -521,7 +529,7 @@ const ( // These environments run shells inside the container/VM, not on the host. func IsRemoteExecutorType(t ExecutorType) bool { switch t { - case ExecutorTypeSprites, ExecutorTypeRemoteDocker, ExecutorTypeLocalDocker, ExecutorTypeMockRemote: + case ExecutorTypeSprites, ExecutorTypeRemoteDocker, ExecutorTypeLocalDocker, ExecutorTypeSSH, ExecutorTypeMockRemote: return true default: return false @@ -543,6 +551,8 @@ func (t ExecutorType) Runtime() agentruntime.Runtime { return agentruntime.RuntimeRemoteDocker case ExecutorTypeSprites: return agentruntime.RuntimeSprites + case ExecutorTypeSSH: + return agentruntime.RuntimeSSH default: return agentruntime.RuntimeStandalone } @@ -560,7 +570,7 @@ func IsContainerizedExecutorType(t ExecutorType) bool { // IsAlwaysResumableRuntime reports whether the given runtime represents // an executor that can always be resumed even without an explicit resume token. func IsAlwaysResumableRuntime(runtime agentruntime.Runtime) bool { - return runtime == agentruntime.RuntimeSprites + return runtime == agentruntime.RuntimeSprites || runtime == agentruntime.RuntimeSSH } const ( diff --git a/apps/backend/internal/task/repository/sqlite/executor.go b/apps/backend/internal/task/repository/sqlite/executor.go index f6b1678795..f72688c153 100644 --- a/apps/backend/internal/task/repository/sqlite/executor.go +++ b/apps/backend/internal/task/repository/sqlite/executor.go @@ -49,7 +49,7 @@ func (r *Repository) GetExecutor(ctx context.Context, id string) (*models.Execut &isSystem, &resumable, &configJSON, &executor.CreatedAt, &executor.UpdatedAt, &executor.DeletedAt, ) if err == sql.ErrNoRows { - return nil, fmt.Errorf("executor not found: %s", id) + return nil, fmt.Errorf("%w: %s", models.ErrExecutorNotFound, id) } if err != nil { return nil, err diff --git a/apps/backend/internal/task/service/service_task_environments.go b/apps/backend/internal/task/service/service_task_environments.go index 528da94268..613270aa93 100644 --- a/apps/backend/internal/task/service/service_task_environments.go +++ b/apps/backend/internal/task/service/service_task_environments.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strconv" "go.uber.org/zap" @@ -40,6 +41,24 @@ type ContainerLiveStatus struct { Missing bool `json:"missing,omitempty"` } +// SSHLiveStatus surfaces the SSH-runtime fields needed by the Executor +// Settings popover for an SSH-backed task: where the agent is running +// (host/user/workdir), the local port forward we set up for the agent +// stream, and the remote agentctl process info. Sourced from the latest +// running ExecutorRunning row for the task's most recent session — the +// SSH executor writes these metadata keys in CreateInstance and we just +// project them back out here. +type SSHLiveStatus struct { + Host string `json:"host,omitempty"` + Port int `json:"port,omitempty"` + User string `json:"user,omitempty"` + RemoteTaskDir string `json:"remote_task_dir,omitempty"` + RemoteAgentctlPID int `json:"remote_agentctl_pid,omitempty"` + RemoteAgentctlPort int `json:"remote_agentctl_port,omitempty"` + LocalForwardPort int `json:"local_forward_port,omitempty"` + Fingerprint string `json:"fingerprint,omitempty"` +} + // ResetOptions controls destructive behavior of ResetTaskEnvironment. type ResetOptions struct { // PushBranch runs `git push` on the environment's current branch before teardown. @@ -87,6 +106,114 @@ func (s *Service) GetTaskEnvironmentLiveStatus(ctx context.Context, taskID strin return s.envDestroyer.GetContainerLiveStatus(ctx, env.ContainerID) } +// GetSSHLiveStatus returns the SSH-specific runtime info for the task's +// most recent session, or (nil, nil) when the task has no environment yet +// or the environment isn't SSH-backed. The SSH executor writes these +// fields into ExecutorRunning.Metadata in CreateInstance; here we just +// look up the latest session's row and project the keys back out. We +// don't probe the remote here — that's GetRemoteStatus's job — because +// the popover polls this endpoint and a per-poll SSH dial would be both +// slow and a lot of TCP traffic. +func (s *Service) GetSSHLiveStatus(ctx context.Context, taskID string) (*SSHLiveStatus, error) { + env, err := s.taskEnvironments.GetTaskEnvironmentByTaskID(ctx, taskID) + if err != nil || env == nil { + return nil, err + } + if env.ExecutorType != string(models.ExecutorTypeSSH) { + return nil, nil + } + running, err := s.latestRunningForTask(ctx, taskID) + if err != nil { + return nil, err + } + if running == nil || running.Metadata == nil { + return nil, nil + } + return buildSSHLiveStatus(running.Metadata), nil +} + +// latestRunningForTask picks the newest ExecutorRunning row across the +// task's sessions. Sessions are returned newest-first by the repository +// today; if that ever changes the popover would silently surface stale +// data, so this helper is defensive: it walks every session and keeps +// the row with the latest StartedAt. +// +// Repository errors are surfaced so the popover handler can log + omit +// the ssh block instead of silently rendering "no SSH live status" on +// every poll after a transient storage blip. The one "expected +// missing" case (a session with no running row) is matched against +// models.ErrExecutorRunningNotFound and skipped quietly. +func (s *Service) latestRunningForTask(ctx context.Context, taskID string) (*models.ExecutorRunning, error) { + sessions, err := s.sessions.ListTaskSessions(ctx, taskID) + if err != nil { + return nil, fmt.Errorf("list task sessions: %w", err) + } + var latest *models.ExecutorRunning + for _, sess := range sessions { + if sess == nil { + continue + } + running, err := s.executors.GetExecutorRunningBySessionID(ctx, sess.ID) + if err != nil { + if errors.Is(err, models.ErrExecutorRunningNotFound) { + continue + } + return nil, fmt.Errorf("get executor running for session %s: %w", sess.ID, err) + } + if running == nil { + continue + } + if latest == nil || running.CreatedAt.After(latest.CreatedAt) { + latest = running + } + } + return latest, nil +} + +// buildSSHLiveStatus projects the SSH metadata keys onto the live-status +// shape. Pure function so the projection contract is unit-testable. +func buildSSHLiveStatus(md map[string]interface{}) *SSHLiveStatus { + status := &SSHLiveStatus{ + Host: mdString(md, "ssh_host"), + User: mdString(md, "ssh_user"), + RemoteTaskDir: mdString(md, "ssh_remote_task_dir"), + Fingerprint: mdString(md, "ssh_host_fingerprint"), + } + status.Port = mdInt(md, "ssh_port") + status.RemoteAgentctlPID = mdInt(md, "ssh_remote_agentctl_pid") + status.RemoteAgentctlPort = mdInt(md, "ssh_remote_agentctl_port") + status.LocalForwardPort = mdInt(md, "ssh_local_forward_port") + return status +} + +func mdString(md map[string]interface{}, key string) string { + if v, ok := md[key].(string); ok { + return v + } + return "" +} + +func mdInt(md map[string]interface{}, key string) int { + switch v := md[key].(type) { + case int: + return v + case int64: + return int(v) + case float64: + return int(v) + case string: + // SSH executor stores numeric metadata as strings (strconv.Itoa) so + // every JSON round-trip preserves the value verbatim. Tolerate both. + n, err := strconv.Atoi(v) + if err != nil { + return 0 + } + return n + default: + return 0 + } +} + // checkAnySessionRunning delegates to the configured checker or falls back to // the default (session has an ExecutorRunning row). func (s *Service) checkAnySessionRunning(ctx context.Context, taskID string) (bool, error) { diff --git a/apps/backend/internal/task/service/service_task_environments_test.go b/apps/backend/internal/task/service/service_task_environments_test.go index 7403c02e40..26023c3235 100644 --- a/apps/backend/internal/task/service/service_task_environments_test.go +++ b/apps/backend/internal/task/service/service_task_environments_test.go @@ -301,3 +301,70 @@ func TestPerformTaskCleanup_TearsDownTaskEnvironmentAndDeletesRow(t *testing.T) t.Fatal("expected task environment row to be deleted") } } + +// TestBuildSSHLiveStatus_StringsAndStringEncodedInts pins the projection +// from ExecutorRunning.Metadata into the popover-shaped SSHLiveStatus. The +// SSH executor writes its numeric metadata as strings (strconv.Itoa) so +// the projection must accept both "41001" and 41001 — every JSON +// round-trip through SQLite blob storage gives us strings, but direct +// in-memory writes give us ints. +func TestBuildSSHLiveStatus_StringsAndStringEncodedInts(t *testing.T) { + got := buildSSHLiveStatus(map[string]interface{}{ + "ssh_host": "koi.zeval.local", + "ssh_port": "2222", + "ssh_user": "zeval", + "ssh_remote_task_dir": "/home/zeval/.kandev/tasks/task-1", + "ssh_remote_agentctl_pid": "4732", + "ssh_remote_agentctl_port": "41001", + "ssh_local_forward_port": "59123", + "ssh_host_fingerprint": "SHA256:abc", + }) + if got.Host != "koi.zeval.local" || got.Port != 2222 || got.User != "zeval" { + t.Errorf("connection fields = %+v, want host/port/user", got) + } + if got.RemoteTaskDir != "/home/zeval/.kandev/tasks/task-1" { + t.Errorf("RemoteTaskDir = %q", got.RemoteTaskDir) + } + if got.RemoteAgentctlPID != 4732 || got.RemoteAgentctlPort != 41001 || got.LocalForwardPort != 59123 { + t.Errorf("agentctl fields = %+v", got) + } + if got.Fingerprint != "SHA256:abc" { + t.Errorf("Fingerprint = %q", got.Fingerprint) + } +} + +func TestBuildSSHLiveStatus_NativeIntsAlsoAccepted(t *testing.T) { + got := buildSSHLiveStatus(map[string]interface{}{ + "ssh_host": "h", + "ssh_port": 22, + "ssh_remote_agentctl_pid": int64(99), + "ssh_remote_agentctl_port": float64(41001), + }) + if got.Port != 22 || got.RemoteAgentctlPID != 99 || got.RemoteAgentctlPort != 41001 { + t.Errorf("native int projection failed: %+v", got) + } +} + +func TestBuildSSHLiveStatus_EmptyMetadataReturnsZeroValueStruct(t *testing.T) { + got := buildSSHLiveStatus(map[string]interface{}{}) + if got == nil { + t.Fatal("expected non-nil status (zero value), got nil") + } + if got.Host != "" || got.Port != 0 || got.RemoteAgentctlPID != 0 { + t.Errorf("expected zero-value fields, got %+v", got) + } +} + +func TestBuildSSHLiveStatus_InvalidPortString_NoCrash(t *testing.T) { + // SSH executor only emits Itoa'd ports, but be defensive in case + // something else writes the metadata (e.g. a future migration or + // import path). Don't want a stray non-numeric value to panic the + // popover endpoint for every other field too. + got := buildSSHLiveStatus(map[string]interface{}{ + "ssh_host": "h", + "ssh_port": "not-a-port", + }) + if got.Host != "h" || got.Port != 0 { + t.Errorf("expected host preserved, port=0 on bad input, got %+v", got) + } +} diff --git a/apps/web/app/settings/executors/[profileId]/page.tsx b/apps/web/app/settings/executors/[profileId]/page.tsx index a5016f4449..6f4fc56f36 100644 --- a/apps/web/app/settings/executors/[profileId]/page.tsx +++ b/apps/web/app/settings/executors/[profileId]/page.tsx @@ -25,6 +25,7 @@ import { rowsToEnvVars, } from "@/components/settings/profile-edit/env-vars-card"; import { ScriptCard } from "@/components/settings/profile-edit/script-card"; +import { SSHAgentReadinessCard } from "@/components/settings/ssh-agent-readiness-card"; import { type GitIdentityMode, type GitIdentityState, @@ -84,10 +85,15 @@ function parseRemoteCredentials(config?: Record): string[] { } function useRemoteExecutorFlags(executorType: ExecutorType) { + // SSH joins the "remote" set because it runs the agent on a host whose + // filesystem doesn't share paths with the kandev backend — so the same + // remote-credentials + auth-secrets surface applies (the SSH executor + // SFTPs files into the remote user's $HOME). const isRemote = executorType === "local_docker" || executorType === "remote_docker" || - executorType === "sprites"; + executorType === "sprites" || + executorType === "ssh"; return { isRemote, isDocker: executorType === "local_docker" || executorType === "remote_docker", @@ -405,15 +411,29 @@ function ProfileEditSections({ profile, form, secrets, + onShellChange, }: { executor: Executor; profile: ExecutorProfile; form: ReturnType; secrets: ReturnType["items"]; + onShellChange?: (shell: string) => Promise; }) { + const isSSH = executor.type === "ssh"; return ( <> + {isSSH && ( + // SSH-specific: lives right after profile details (top of the page) + // because the very next question after "name this profile" on an + // SSH host is "which agents are installed here". Drives the shell + // selector that governs every subsequent SSH command run by kandev. + + )} {form.isSprites && ( { + const mergedConfig = { ...(profile.config ?? {}), ssh_shell: next }; + await persistence.save({ + name: profile.name, + config: mergedConfig, + prepare_script: profile.prepare_script ?? "", + cleanup_script: profile.cleanup_script ?? "", + env_vars: profile.env_vars ?? [], + }); + }, + [persistence, profile], + ); + const handleDelete = (options?: { removeRelatedDockerContainers?: boolean }) => { const beforeDelete = options?.removeRelatedDockerContainers ? async () => { @@ -525,7 +567,13 @@ function ProfileEditForm({ executor, profile }: { executor: Executor; profile: E profileName={profile.name} description={getExecutorDescription(executor.type)} /> - + {spritesTokenMissing && (

Sprites API key is required.

)} diff --git a/apps/web/app/settings/executors/new/[type]/executor-types.ts b/apps/web/app/settings/executors/new/[type]/executor-types.ts new file mode 100644 index 0000000000..7798092356 --- /dev/null +++ b/apps/web/app/settings/executors/new/[type]/executor-types.ts @@ -0,0 +1,42 @@ +// Registry of executor types presented in the "new executor profile" flow. +// Keep entries here (not inline in the page) so the page file stays under +// the 600-line lint cap and new types can be added without touching layout. + +export type ExecutorTypeInfo = { + executorId: string; + label: string; + description: string; +}; + +export const EXECUTOR_TYPE_MAP: Record = { + local: { + executorId: "exec-local", + label: "Local", + description: "Runs agents directly in the repository folder.", + }, + worktree: { + executorId: "exec-worktree", + label: "Worktree", + description: "Creates git worktrees for isolated agent sessions.", + }, + local_docker: { + executorId: "exec-local-docker", + label: "Docker", + description: "Runs Docker containers on this machine.", + }, + remote_docker: { + executorId: "exec-remote-docker", + label: "Remote Docker", + description: "Connects to a remote Docker host.", + }, + sprites: { + executorId: "exec-sprites", + label: "Sprites.dev", + description: "Runs agents in Sprites.dev cloud sandboxes.", + }, + ssh: { + executorId: "exec-ssh", + label: "SSH", + description: "Connects to a remote host over SSH and runs agentctl there.", + }, +}; diff --git a/apps/web/app/settings/executors/new/[type]/page.tsx b/apps/web/app/settings/executors/new/[type]/page.tsx index 6a36cb2d9c..a8712fadbd 100644 --- a/apps/web/app/settings/executors/new/[type]/page.tsx +++ b/apps/web/app/settings/executors/new/[type]/page.tsx @@ -42,40 +42,12 @@ import { import type { NetworkPolicyRule } from "@/lib/api/domains/settings-api"; import type { Executor, ExecutorType, ProfileEnvVar } from "@/lib/types/http"; +import { EXECUTOR_TYPE_MAP } from "./executor-types"; +import { SSHCreatePage } from "./ssh-create-page"; + const EXECUTORS_ROUTE = "/settings/executors"; const SPRITES_TOKEN_KEY = "SPRITES_API_TOKEN"; -const EXECUTOR_TYPE_MAP: Record< - string, - { executorId: string; label: string; description: string } -> = { - local: { - executorId: "exec-local", - label: "Local", - description: "Runs agents directly in the repository folder.", - }, - worktree: { - executorId: "exec-worktree", - label: "Worktree", - description: "Creates git worktrees for isolated agent sessions.", - }, - local_docker: { - executorId: "exec-local-docker", - label: "Docker", - description: "Runs Docker containers on this machine.", - }, - remote_docker: { - executorId: "exec-remote-docker", - label: "Remote Docker", - description: "Connects to a remote Docker host.", - }, - sprites: { - executorId: "exec-sprites", - label: "Sprites.dev", - description: "Runs agents in Sprites.dev cloud sandboxes.", - }, -}; - const DefaultIcon = EXECUTOR_ICON_MAP.local; function ExecutorTypeIcon({ type }: { type: string }) { @@ -91,6 +63,10 @@ export default function CreateProfilePage({ params }: { params: Promise<{ type: return ; } + if (type === "ssh") { + return ; + } + return ; } diff --git a/apps/web/app/settings/executors/new/[type]/ssh-config.ts b/apps/web/app/settings/executors/new/[type]/ssh-config.ts new file mode 100644 index 0000000000..1b456694cc --- /dev/null +++ b/apps/web/app/settings/executors/new/[type]/ssh-config.ts @@ -0,0 +1,48 @@ +import type { SSHExecutorConfig } from "@/components/settings/ssh-connection-card"; + +/** + * Maps the SSHConnectionCard's flat config to the snake_case `Config` map + * the backend persists on the Executor row. Empty optional fields are + * dropped so the JSON we POST is minimal. + */ +export function buildSSHExecutorConfig(cfg: SSHExecutorConfig): Record { + const out: Record = { + ssh_identity_source: cfg.identity_source, + }; + if (cfg.host_alias?.trim()) out.ssh_host_alias = cfg.host_alias.trim(); + if (cfg.host?.trim()) out.ssh_host = cfg.host.trim(); + if (cfg.port != null) out.ssh_port = String(cfg.port); + if (cfg.user?.trim()) out.ssh_user = cfg.user.trim(); + if (cfg.identity_file?.trim()) out.ssh_identity_file = cfg.identity_file.trim(); + if (cfg.proxy_jump?.trim()) out.ssh_proxy_jump = cfg.proxy_jump.trim(); + if (cfg.host_fingerprint?.trim()) out.ssh_host_fingerprint = cfg.host_fingerprint.trim(); + return out; +} + +/** + * Inverse of {@link buildSSHExecutorConfig}: reads the backend `Config` map + * into the form's flat shape so an existing executor can be edited. + */ +export function parseSSHExecutorConfig( + name: string, + config?: Record, +): Partial { + const c = config ?? {}; + const portRaw = c.ssh_port; + // A missing ssh_port intentionally stays `undefined` so the backend's + // resolver can apply ~/.ssh/config Port inheritance for the configured + // alias. Substituting 22 here would silently redirect connections that + // relied on the alias' Port. + const port = portRaw ? Number.parseInt(portRaw, 10) : undefined; + return { + name, + host_alias: c.ssh_host_alias ?? "", + host: c.ssh_host ?? "", + port: port != null && Number.isFinite(port) ? port : undefined, + user: c.ssh_user ?? "", + identity_source: (c.ssh_identity_source as "agent" | "file") || "agent", + identity_file: c.ssh_identity_file ?? "", + proxy_jump: c.ssh_proxy_jump ?? "", + host_fingerprint: c.ssh_host_fingerprint || undefined, + }; +} 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 new file mode 100644 index 0000000000..5b457ed197 --- /dev/null +++ b/apps/web/app/settings/executors/new/[type]/ssh-create-page.tsx @@ -0,0 +1,111 @@ +"use client"; + +import { useCallback } from "react"; +import { useRouter } from "next/navigation"; +import { Badge } from "@kandev/ui/badge"; +import { Button } from "@kandev/ui/button"; +import { Separator } from "@kandev/ui/separator"; +import { IconTerminal2 } from "@tabler/icons-react"; +import { useAppStoreApi } from "@/components/state-provider"; +import { createExecutor, createExecutorProfile } from "@/lib/api/domains/settings-api"; +import { SSHConnectionCard } from "@/components/settings/ssh-connection-card"; +import type { SSHExecutorConfig } from "@/components/settings/ssh-connection-card"; +import { getExecutorLabel } from "@/lib/executor-icons"; +import { buildSSHExecutorConfig } from "./ssh-config"; +import type { Executor } from "@/lib/types/http"; + +const EXECUTORS_ROUTE = "/settings/executors"; + +/** + * SSH-specific "new executor" flow. Renders just the SSHConnectionCard; + * Save POSTs to /api/v1/executors with type=ssh and the freshly-pinned + * fingerprint, then immediately creates a default profile under it (the + * /settings/executors index lists profiles, so an executor with no + * profile would otherwise be invisible). Routes to the profile-edit page + * after both writes succeed — that's where the user picks the shell, + * probes for installed agents, and tunes per-task config (workdir, + * prepare script, credentials). The host-level executor page + * (/settings/executors/ssh/:id) remains reachable for re-trust and + * sessions inspection. + */ +export function SSHCreatePage() { + const router = useRouter(); + const store = useAppStoreApi(); + + const handleSave = useCallback( + async (cfg: SSHExecutorConfig) => { + const created = await createExecutor({ + name: cfg.name, + type: "ssh", + config: buildSSHExecutorConfig(cfg), + }); + // Auto-create a default profile so the executor shows up in the + // /settings/executors list (which flattens by profile). The profile + // carries the same name as the executor and inherits the connection + // config; users can add more profiles later if they want different + // workdir roots / prepare scripts / env vars on the same host. + const profile = await createExecutorProfile(created.id, { + name: cfg.name, + }); + const next: Executor = { + id: created.id, + name: created.name, + type: "ssh", + status: "active", + is_system: false, + config: created.config, + profiles: [profile], + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + }; + // 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. 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; + const merged = current.some((e) => e.id === next.id) ? current : [...current, next]; + store.getState().setExecutors(merged); + router.push(`/settings/executors/${profile.id}`); + }, + [router, store], + ); + + return ( +
+ + +
+ ); +} + +function SSHCreateHeader() { + const router = useRouter(); + return ( + <> +
+
+
+ +

New SSH Executor

+ + {getExecutorLabel("ssh")} + +
+

+ Connect to a remote host over SSH and run agentctl there. linux/amd64 hosts only. +

+
+ +
+ + + ); +} diff --git a/apps/web/app/settings/executors/page.tsx b/apps/web/app/settings/executors/page.tsx index 20fa255710..047dfdbfea 100644 --- a/apps/web/app/settings/executors/page.tsx +++ b/apps/web/app/settings/executors/page.tsx @@ -57,6 +57,11 @@ const EXECUTOR_TYPES = [ label: "Sprites.dev", description: "Run agents in Sprites.dev cloud sandboxes.", }, + { + type: "ssh", + label: "SSH", + description: "Connect to a remote host over SSH and run agentctl there.", + }, ] as const; function ExecutorIconBadge({ type }: { type: string }) { diff --git a/apps/web/app/settings/executors/ssh/[executorId]/page.tsx b/apps/web/app/settings/executors/ssh/[executorId]/page.tsx new file mode 100644 index 0000000000..74847873a8 --- /dev/null +++ b/apps/web/app/settings/executors/ssh/[executorId]/page.tsx @@ -0,0 +1,199 @@ +"use client"; + +import { use, useCallback, useEffect, useState } from "react"; +import { useRouter } from "next/navigation"; +import { Badge } from "@kandev/ui/badge"; +import { Button } from "@kandev/ui/button"; +import { Card, CardContent } from "@kandev/ui/card"; +import { Separator } from "@kandev/ui/separator"; +import { IconTerminal2 } from "@tabler/icons-react"; +import { useAppStoreApi } from "@/components/state-provider"; +import { fetchExecutor, listExecutors, updateExecutor } from "@/lib/api/domains/settings-api"; +import { SSHConnectionCard } from "@/components/settings/ssh-connection-card"; +import type { SSHExecutorConfig } from "@/components/settings/ssh-connection-card"; +import { SSHSessionsCard } from "@/components/settings/ssh-sessions-card"; +import { listSSHSessions } from "@/lib/api/domains/ssh-api"; +import { getExecutorLabel } from "@/lib/executor-icons"; +import { + buildSSHExecutorConfig, + parseSSHExecutorConfig, +} from "@/app/settings/executors/new/[type]/ssh-config"; +import type { Executor } from "@/lib/types/http"; + +const EXECUTORS_ROUTE = "/settings/executors"; + +type LoadedExecutor = { + id: string; + name: string; + type: string; + config?: Record; +}; + +export default function SSHExecutorPage({ params }: { params: Promise<{ executorId: string }> }) { + const { executorId } = use(params); + const { executor, loading, error, reload } = useExecutor(executorId); + + if (loading) { + return ( + + + Loading executor... + + + ); + } + if (error || !executor) { + return ; + } + if (executor.type !== "ssh") { + return ; + } + return ; +} + +function useExecutor(executorId: string) { + const [executor, setExecutor] = useState(null); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + + const load = useCallback(async () => { + setLoading(true); + setError(null); + try { + const res = await fetchExecutor(executorId); + setExecutor(res); + } catch (e) { + setError(e instanceof Error ? e.message : "Failed to load executor"); + } finally { + setLoading(false); + } + }, [executorId]); + + useEffect(() => { + void load(); + }, [load]); + + return { executor, loading, error, reload: load }; +} + +function NotFoundCard({ message }: { message: string }) { + const router = useRouter(); + return ( + + +

{message}

+ + + + ); +} + +function SSHExecutorView({ + executor, + onSaved, +}: { + executor: LoadedExecutor; + onSaved: () => void | Promise; +}) { + const initial = parseSSHExecutorConfig(executor.name, executor.config); + const sessionCount = useRunningSessionCount(executor.id); + const handleSave = useSaveExecutor(executor.id, onSaved); + + return ( +
+ + + +
+ ); +} + +function SSHExecutorHeader({ executorName }: { executorName: string }) { + const router = useRouter(); + return ( + <> +
+
+
+ +

{executorName}

+ + {getExecutorLabel("ssh")} + +
+

+ Edit the connection settings or re-trust the host. Existing sessions keep their snapshot + of the previous config. +

+
+ +
+ + + ); +} + +function useRunningSessionCount(executorId: string): number { + const [count, setCount] = useState(0); + useEffect(() => { + let cancelled = false; + listSSHSessions(executorId) + .then((rows) => { + if (!cancelled) setCount(rows.length); + }) + .catch(() => { + if (!cancelled) setCount(0); + }); + return () => { + cancelled = true; + }; + }, [executorId]); + return count; +} + +function useSaveExecutor(executorId: string, onSaved: () => void | Promise) { + const store = useAppStoreApi(); + + return useCallback( + async (cfg: SSHExecutorConfig) => { + const config = buildSSHExecutorConfig(cfg); + await updateExecutor(executorId, { name: cfg.name, config }); + // Refresh the store so the executor list reflects the new name + config. + try { + const fresh = await listExecutors(); + store.getState().setExecutors(fresh.executors); + } catch { + // Non-fatal: the local view still reloads via onSaved(). Read the + // current snapshot at write time so a WS event that updated the + // executor list mid-flight doesn't get overwritten with a stale + // captured copy. + const current = store.getState().executors.items; + store + .getState() + .setExecutors( + current.map((e: Executor) => + e.id === executorId ? { ...e, name: cfg.name, config } : e, + ), + ); + } + await onSaved(); + }, + [executorId, store, onSaved], + ); +} diff --git a/apps/web/components/settings/ssh-agent-readiness-card.tsx b/apps/web/components/settings/ssh-agent-readiness-card.tsx new file mode 100644 index 0000000000..c6efd3c114 --- /dev/null +++ b/apps/web/components/settings/ssh-agent-readiness-card.tsx @@ -0,0 +1,352 @@ +"use client"; + +import { useCallback, useEffect, useRef, useState } from "react"; +import { Badge } from "@kandev/ui/badge"; +import { Button } from "@kandev/ui/button"; +import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@kandev/ui/card"; +import { Label } from "@kandev/ui/label"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select"; +import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@kandev/ui/table"; +import { IconCheck, IconCopy, IconLoader2, IconX } from "@tabler/icons-react"; +import { toast } from "sonner"; +import { probeSSHAgents, probeSSHShells } from "@/lib/api/domains/ssh-api"; +import type { SSHAgentReadinessRow } from "@/lib/types/http-ssh"; + +export interface SSHAgentReadinessCardProps { + executorId: string; + /** The shell currently saved on the profile. Drives initial selection + the + * shell sent to the probe endpoint. */ + shell?: string; + /** Persist a shell change up to the profile. Called when the user picks a + * different option in the dropdown so the choice survives the page reload + * and flows into the next agent launch as ssh_shell metadata. */ + onShellChange?: (shell: string) => void | Promise; +} + +const DEFAULT_SHELL = "bash"; + +// useReadinessState owns the card's fetch + selection state so the component +// renders a thin view layer. Pulled out to keep the component body under the +// max-lines-per-function lint budget. +function useReadinessState({ + executorId, + shellProp, + onShellChange, +}: { + executorId: string; + shellProp?: string; + onShellChange?: (s: string) => void | Promise; +}) { + const [rows, setRows] = useState([]); + const [loading, setLoading] = useState(false); + const [error, setError] = useState(null); + const [hasProbed, setHasProbed] = useState(false); + const [shells, setShells] = useState(null); + const [shellsLoading, setShellsLoading] = useState(false); + const [shell, setShell] = useState(shellProp ?? DEFAULT_SHELL); + // Drop stale responses if the user clicks Refresh again before the + // previous request lands — same pattern as ssh-sessions-card. + const seqRef = useRef(0); + + const refresh = useCallback(async () => { + const seq = ++seqRef.current; + setLoading(true); + setError(null); + try { + const resp = await probeSSHAgents(executorId, { shell }); + if (seq !== seqRef.current) return; + setRows(resp.rows); + setHasProbed(true); + } catch (e) { + if (seq !== seqRef.current) return; + setError(e instanceof Error ? e.message : "Failed to probe agents"); + } finally { + if (seq === seqRef.current) setLoading(false); + } + }, [executorId, shell]); + + // Auto-probe the shells once on mount so the dropdown has real options. + // Cheap (one SSH dial, ~1s) and fully out-of-band of agent probing. + const probeShells = useCallback(async () => { + setShellsLoading(true); + try { + const resp = await probeSSHShells(executorId); + setShells(resp.available); + } catch { + // On failure the dropdown stays empty; user can type / save the shell + // via the profile form. We don't toast — a transient probe error + // shouldn't nag. + } finally { + setShellsLoading(false); + } + }, [executorId]); + + useEffect(() => { + // Switching to a different executor should reset the displayed + // probe state — otherwise the previous executor's rows / error / + // hasProbed sit in the UI until the next manual refresh, which + // reads like "still loading" while actually showing stale data. + seqRef.current = 0; + setRows([]); + setError(null); + setHasProbed(false); + setLoading(false); + void probeShells(); + return () => { + seqRef.current = -1; + }; + }, [executorId, probeShells]); + + const handleShellChange = useCallback( + async (next: string) => { + setShell(next); + // Stale rows = stale answer (PATH depends on shell init). Clear the + // table so the user re-probes explicitly with the new shell. + setRows([]); + setHasProbed(false); + if (onShellChange) { + try { + await onShellChange(next); + } catch (e) { + toast.error(e instanceof Error ? e.message : "Failed to save shell"); + } + } + }, + [onShellChange], + ); + + return { + rows, + loading, + error, + hasProbed, + shells, + shellsLoading, + shell, + refresh, + handleShellChange, + }; +} + +/** + * Probes the remote host for each kandev-enabled agent's required binary + * (the first token of the agent's BuildCommand) under the user-chosen login + * shell. Surfaces availability + a copy-button for the agent's install + * command — installs themselves stay manual so the user keeps full control + * over what runs on their machine. Manual refresh only — a real SSH dial + * happens per probe so we don't poll on a timer. + */ +export function SSHAgentReadinessCard({ + executorId, + shell: shellProp, + onShellChange, +}: SSHAgentReadinessCardProps) { + const state = useReadinessState({ executorId, shellProp, onShellChange }); + const { + rows, + loading, + error, + hasProbed, + shells, + shellsLoading, + shell, + refresh, + handleShellChange, + } = state; + + return ( + + +
+
+ Available agents on this host + + Probes the remote {"$PATH"} for each enabled agent under the chosen login shell. Copy + the install hint and run it on the remote when an agent is missing. + +
+ +
+ + + + + + + ); +} + +function ShellSelector({ + shell, + shells, + loading, + onChange, +}: { + shell: string; + shells: string[] | null; + loading: boolean; + onChange: (s: string) => void | Promise; +}) { + // While probing, show a placeholder option so the dropdown can't surface + // a stale "bash" selection that we haven't confirmed exists on the host. + // After probing, render whatever the host has, plus the current value so + // a saved shell that isn't actually installed is still selectable + loud. + const options = uniqueShellOptions(shell, shells); + return ( +
+ + + {loading ? : null} +
+ ); +} + +function uniqueShellOptions(currentShell: string, detected: string[] | null): string[] { + const set = new Set(); + if (detected) detected.forEach((s) => set.add(s)); + set.add(currentShell || DEFAULT_SHELL); + if (set.size === 0) set.add(DEFAULT_SHELL); + return Array.from(set); +} + +function ReadinessContent({ + error, + hasProbed, + rows, +}: { + error: string | null; + hasProbed: boolean; + rows: SSHAgentReadinessRow[]; +}) { + if (error) { + return ( +

+ {error} +

+ ); + } + if (!hasProbed) { + return ( +

+ Click {`"Probe agents"`} to check which agents are installed on the remote. +

+ ); + } + if (rows.length === 0) { + return

No enabled agents to probe.

; + } + return ; +} + +function ReadinessTable({ rows }: { rows: SSHAgentReadinessRow[] }) { + return ( + + + + Agent + Binary + Status + Install hint + + + + {rows.map((row) => ( + + ))} + +
+ ); +} + +function ReadinessRow({ row }: { row: SSHAgentReadinessRow }) { + const slug = row.agent_id.replace(/[^a-z0-9]+/gi, "-"); + return ( + + {row.agent_name || row.agent_id} + {row.binary} + + + + + + + + ); +} + +function StatusBadge({ row }: { row: SSHAgentReadinessRow }) { + if (row.error) { + return ( + + Probe error + + ); + } + if (row.available) { + return ( + + Installed + + ); + } + return ( + + Missing + + ); +} + +function InstallHint({ hint, available }: { hint?: string; available: boolean }) { + if (available) return —; + if (!hint) return No hint available; + return ( +
+ {hint} + +
+ ); +} diff --git a/apps/web/components/settings/ssh-connection-card.tsx b/apps/web/components/settings/ssh-connection-card.tsx new file mode 100644 index 0000000000..e656278365 --- /dev/null +++ b/apps/web/components/settings/ssh-connection-card.tsx @@ -0,0 +1,606 @@ +"use client"; + +import { useCallback, useMemo, useState } from "react"; +import { Badge } from "@kandev/ui/badge"; +import { Button } from "@kandev/ui/button"; +import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@kandev/ui/card"; +import { Input } from "@kandev/ui/input"; +import { Label } from "@kandev/ui/label"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select"; +import { + IconCheck, + IconLoader2, + IconShieldLock, + IconTerminal2, + IconTestPipe, + IconX, +} from "@tabler/icons-react"; +import { testSSHConnection } from "@/lib/api/domains/ssh-api"; +import { FingerprintTrustBlock } from "@/components/settings/ssh-fingerprint-trust-block"; +import type { + SSHIdentitySource, + SSHTestRequest, + SSHTestResult, + SSHTestStep, +} from "@/lib/types/http-ssh"; + +// SSHExecutorConfig is the shape we persist into executor.Config on save. +// The host_fingerprint is set only after a successful Test Connection has +// completed and the user has ticked "Trust this host". +export interface SSHExecutorConfig { + name: string; + host_alias?: string; + host?: string; + port?: number; + user?: string; + identity_source: SSHIdentitySource; + identity_file?: string; + proxy_jump?: string; + host_fingerprint?: string; +} + +export interface SSHConnectionCardProps { + initial?: Partial; + // Called when the user clicks Save after a successful test+trust. The + // returned config carries the freshly pinned fingerprint. + onSave: (config: SSHExecutorConfig) => Promise | void; + // Existing running sessions for this executor. Triggers the + // "this won't affect existing sessions" warning on save. + runningSessionCount?: number; +} + +interface SSHConnectionState { + form: SSHExecutorConfig; + testing: boolean; + saving: boolean; + result: SSHTestResult | null; + // resultStale flips true when the user edits a connection-affecting field + // after a successful test, so the prior fingerprint cannot be trusted for + // the current form. The result stays visible (the trust-gate spec expects + // the checkbox to render) but trust + save are gated off until the user + // re-runs Test Connection. + resultStale: boolean; + trust: boolean; + error: string | null; +} + +// Fields whose value, if changed, could route the next connection to a +// different machine — editing any of them invalidates the current trust tick +// so the user must re-test against the new target. +const CONNECTION_FIELDS = new Set([ + "host_alias", + "host", + "port", + "user", + "identity_source", + "identity_file", + "proxy_jump", +]); + +const SSH_FORM_DEFAULTS: SSHExecutorConfig = { + name: "", + host_alias: "", + host: "", + port: 22, + user: "", + identity_source: "agent", + identity_file: "", + proxy_jump: "", + host_fingerprint: undefined, +}; + +function initialState(initial?: Partial): SSHConnectionState { + return { + form: { ...SSH_FORM_DEFAULTS, ...(initial ?? {}) }, + testing: false, + saving: false, + result: null, + resultStale: false, + trust: false, + error: null, + }; +} + +function useSSHConnection(props: SSHConnectionCardProps) { + const [state, setState] = useState(() => initialState(props.initial)); + const { form, testing, saving, result, resultStale, trust, error } = state; + + const update = useCallback( + (key: K, value: SSHExecutorConfig[K]) => { + setState((prev) => { + const isConnectionField = CONNECTION_FIELDS.has(key); + // A connection edit invalidates the prior result. Mark stale when + // a test result is already on screen OR a test is mid-flight — in + // the latter case handleTest will see the staleness on completion + // and refuse to clear it, so a fingerprint returned for the old + // form can't be trusted against the new one. + const staleAfter = isConnectionField + ? prev.result !== null || prev.testing || prev.resultStale + : prev.resultStale; + return { + ...prev, + form: { ...prev.form, [key]: value }, + resultStale: staleAfter, + trust: isConnectionField ? false : prev.trust, + error: null, + }; + }); + }, + [], + ); + + const setTrust = useCallback((v: boolean) => setState((prev) => ({ ...prev, trust: v })), []); + + const canTest = useMemo(() => { + if (testing) return false; + if (form.name.trim() === "") return false; + if ((form.host ?? "").trim() === "" && (form.host_alias ?? "").trim() === "") return false; + return true; + }, [form, testing]); + + const handleTest = useCallback(async () => { + setState((prev) => ({ + ...prev, + testing: true, + result: null, + resultStale: false, + error: null, + })); + try { + const req: SSHTestRequest = { + name: form.name, + host_alias: form.host_alias || undefined, + host: form.host || undefined, + port: form.port || undefined, + user: form.user || undefined, + identity_source: form.identity_source, + identity_file: form.identity_file || undefined, + proxy_jump: form.proxy_jump || undefined, + }; + const res = await testSSHConnection(req); + setState((prev) => ({ + ...prev, + result: res, + // Preserve resultStale if the user edited a connection field while + // the test was in flight — the returned fingerprint is bound to the + // old form and must not be trusted against the new target. + resultStale: prev.resultStale, + testing: false, + })); + } catch (e) { + const msg = e instanceof Error ? e.message : "Failed to reach backend"; + setState((prev) => ({ ...prev, error: msg, testing: false })); + } + }, [form]); + + const canSave = !!result?.success && !!result.fingerprint && trust && !resultStale && !saving; + + const confirmRunningSessions = useCallback( + () => + props.runningSessionCount + ? window.confirm( + `This executor has ${props.runningSessionCount} running session(s). ` + + `They will keep running on the current host. Only new sessions started ` + + `after save will use the updated config. Continue?`, + ) + : true, + [props.runningSessionCount], + ); + + const handleSave = useCallback(async () => { + if (!canSave || !result?.fingerprint) return; + if (!confirmRunningSessions()) return; + setState((prev) => ({ ...prev, saving: true, error: null })); + try { + await props.onSave({ ...form, host_fingerprint: result.fingerprint }); + setState((prev) => ({ ...prev, saving: false })); + } catch (e) { + const msg = e instanceof Error ? e.message : "Failed to save executor"; + setState((prev) => ({ ...prev, saving: false, error: msg })); + } + }, [canSave, confirmRunningSessions, form, props, result]); + + return { + form, + testing, + saving, + result, + resultStale, + trust, + error, + canTest, + canSave, + update, + setTrust, + handleTest, + handleSave, + }; +} + +export function SSHConnectionCard(props: SSHConnectionCardProps) { + const c = useSSHConnection(props); + return ( + + +
+
+ + + Connection + + + Run an agent on any Linux box you can reach over SSH. linux/amd64 hosts only. + +
+ +
+ + + + {c.form.host_fingerprint && } + + {c.error && ( +

+ {c.error} +

+ )} + {c.result && ( + + )} + + + ); +} + +type FieldOnChange = ( + key: K, + value: SSHExecutorConfig[K], +) => void; + +function SSHConnectionForm({ + form, + onChange, +}: { + form: SSHExecutorConfig; + onChange: FieldOnChange; +}) { + return ( +
+ onChange("name", v)} + /> + onChange("host_alias", v)} + /> + onChange("host", v)} + /> + onChange("port", parseInt(v, 10) || 22)} + /> + onChange("user", v)} + /> + onChange("identity_source", v)} + /> + {form.identity_source === "file" && ( + onChange("identity_file", v)} + /> + )} + onChange("proxy_jump", v)} + /> +
+ ); +} + +function TextField({ + id, + testId, + label, + hint, + placeholder, + type, + value, + onChange, +}: { + id: string; + testId: string; + label: string; + hint?: string; + placeholder?: string; + type?: string; + value: string; + onChange: (v: string) => void; +}) { + return ( + + onChange(e.target.value)} + /> + + ); +} + +function IdentitySourceField({ + value, + onChange, +}: { + value: SSHIdentitySource; + onChange: (v: SSHIdentitySource) => void; +}) { + return ( + + + + ); +} + +function FieldShell({ + id, + label, + hint, + children, +}: { + id: string; + label: string; + hint?: string; + children: React.ReactNode; +}) { + return ( +
+ + {children} + {hint &&

{hint}

} +
+ ); +} + +function PinnedFingerprintRow({ fingerprint }: { fingerprint: string }) { + return ( +
+ + + Pinned fingerprint:{" "} + + {fingerprint} + + +
+ ); +} + +function SSHConnectionActions({ + testing, + saving, + canTest, + canSave, + onTest, + onSave, +}: { + testing: boolean; + saving: boolean; + canTest: boolean; + canSave: boolean; + onTest: () => void; + onSave: () => void; +}) { + return ( +
+ + +
+ ); +} + +function ConnectionBadge({ fingerprint }: { fingerprint?: string }) { + if (!fingerprint) { + return ( + + Unverified + + ); + } + return ( + + Trusted + + ); +} + +function TestResultDisplay({ + result, + trust, + resultStale, + onTrustChange, + currentlyPinned, +}: { + result: SSHTestResult; + trust: boolean; + resultStale: boolean; + onTrustChange: (v: boolean) => void; + currentlyPinned?: string; +}) { + return ( +
+ + {result.steps.map((step: SSHTestStep) => ( + + ))} + {result.error && !result.steps.some((s) => s.error) && ( +

+ {result.error} +

+ )} + {result.success && result.fingerprint && ( + + )} +
+ ); +} + +function TestResultHeader({ success, totalMs }: { success: boolean; totalMs: number }) { + return ( +
+ {success ? ( + + ) : ( + + )} + {success ? "Connection test passed" : "Connection test failed"} + ({totalMs}ms) +
+ ); +} + +function StepRow({ step }: { step: SSHTestStep }) { + const slug = step.name.toLowerCase().replace(/[^a-z0-9]+/g, "-"); + return ( +
+ {step.success ? ( + + ) : ( + + )} +
+
+ {step.name} + ({step.duration_ms}ms) +
+ {step.output && ( +

+ {step.output} +

+ )} + {step.error && ( +

+ {step.error} +

+ )} +
+
+ ); +} diff --git a/apps/web/components/settings/ssh-fingerprint-trust-block.tsx b/apps/web/components/settings/ssh-fingerprint-trust-block.tsx new file mode 100644 index 0000000000..38a6bbe105 --- /dev/null +++ b/apps/web/components/settings/ssh-fingerprint-trust-block.tsx @@ -0,0 +1,62 @@ +"use client"; + +// Trust-confirmation block shown under the SSH test result. Disabled when the +// caller's last test ran against a different form, to keep stale fingerprints +// from being trusted for a new target. +export function FingerprintTrustBlock({ + fingerprint, + currentlyPinned, + trust, + resultStale, + onTrustChange, +}: { + fingerprint: string; + currentlyPinned?: string; + trust: boolean; + resultStale: boolean; + onTrustChange: (v: boolean) => void; +}) { + const fingerprintChanged = !!currentlyPinned && currentlyPinned !== fingerprint; + return ( +
+
+ Host fingerprint observed: + + {fingerprint} + +
+ {fingerprintChanged && ( +

+ Warning: this fingerprint differs from the one currently pinned ( + {currentlyPinned}). Trusting it overwrites the pinned + key. If you didn’t expect a host re-key, stop here. +

+ )} + {resultStale && ( +

+ You changed a connection field after this test ran. Run Test Connection again before + trusting and saving. +

+ )} + +
+ ); +} diff --git a/apps/web/components/settings/ssh-sessions-card.tsx b/apps/web/components/settings/ssh-sessions-card.tsx new file mode 100644 index 0000000000..aaf06bb684 --- /dev/null +++ b/apps/web/components/settings/ssh-sessions-card.tsx @@ -0,0 +1,176 @@ +"use client"; + +import { useCallback, useEffect, useRef, useState } from "react"; +import { Badge } from "@kandev/ui/badge"; +import { Button } from "@kandev/ui/button"; +import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@kandev/ui/card"; +import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@kandev/ui/table"; +import { IconLoader2 } from "@tabler/icons-react"; +import { listSSHSessions } from "@/lib/api/domains/ssh-api"; +import type { SSHSession } from "@/lib/types/http-ssh"; + +export interface SSHSessionsCardProps { + executorId: string; +} + +const REFRESH_INTERVAL_MS = 90_000; + +export function SSHSessionsCard({ executorId }: SSHSessionsCardProps) { + const [sessions, setSessions] = useState([]); + const [loading, setLoading] = useState(false); + const [error, setError] = useState(null); + // Monotonic sequence used to ignore stale poll responses: a slow request + // that resolves after a newer one (or after executorId changes) would + // otherwise overwrite the fresh row set. + const seqRef = useRef(0); + + const refresh = useCallback(async () => { + const seq = ++seqRef.current; + setLoading(true); + setError(null); + try { + const rows = await listSSHSessions(executorId); + if (seq !== seqRef.current) return; // a newer call (or executor switch) won the race + setSessions(rows); + } catch (e) { + if (seq !== seqRef.current) return; + setError(e instanceof Error ? e.message : "Failed to load sessions"); + } finally { + if (seq === seqRef.current) setLoading(false); + } + }, [executorId]); + + useEffect(() => { + // Reset sequence so a previous executor's pending response can't land + // here and pollute the new executor's data. + seqRef.current = 0; + refresh(); + const id = setInterval(refresh, REFRESH_INTERVAL_MS); + return () => { + clearInterval(id); + // Bump the sequence to invalidate any in-flight response from the + // unmounted instance. + seqRef.current = -1; + }; + }, [refresh]); + + return ( + + +
+
+ Active sessions + + Sessions currently running on this SSH host. Refreshes every 90 seconds. + +
+ +
+ + + + + + ); +} + +function SSHSessionsBody({ + loading, + error, + sessions, +}: { + loading: boolean; + error: string | null; + sessions: SSHSession[]; +}) { + if (error) { + return ( +

+ {error} +

+ ); + } + if (sessions.length === 0 && !loading) { + return ( +

+ No active sessions. +

+ ); + } + if (sessions.length === 0) return null; + return ; +} + +function SSHSessionsTable({ sessions }: { sessions: SSHSession[] }) { + return ( + + + + Task + Session + Host + Remote port + Local fwd + Uptime + Status + + + + {sessions.map((s) => ( + + ))} + +
+ ); +} + +function SSHSessionsRow({ session: s }: { session: SSHSession }) { + return ( + + + {s.task_id.slice(0, 8)} + + + {s.session_id.slice(0, 8)} + + + {s.user ? `${s.user}@${s.host}` : s.host} + + + {s.remote_agentctl_port ?? "—"} + + + {s.local_forward_port ?? "—"} + + + {formatUptime(s.uptime_seconds)} + + + + {s.status} + + + + ); +} + +function formatUptime(s: number): string { + if (s < 60) return `${s}s`; + if (s < 3600) return `${Math.floor(s / 60)}m`; + if (s < 86400) return `${Math.floor(s / 3600)}h`; + return `${Math.floor(s / 86400)}d`; +} diff --git a/apps/web/components/settings/ssh-settings.tsx b/apps/web/components/settings/ssh-settings.tsx new file mode 100644 index 0000000000..534683dbb3 --- /dev/null +++ b/apps/web/components/settings/ssh-settings.tsx @@ -0,0 +1,9 @@ +"use client"; + +// SSH executor settings UI. The two cards live in their own files so each +// stays under the component size limits; import from here to keep call sites +// short. +export { SSHConnectionCard } from "./ssh-connection-card"; +export type { SSHConnectionCardProps, SSHExecutorConfig } from "./ssh-connection-card"; +export { SSHSessionsCard } from "./ssh-sessions-card"; +export type { SSHSessionsCardProps } from "./ssh-sessions-card"; diff --git a/apps/web/components/task/executor-environment-info.tsx b/apps/web/components/task/executor-environment-info.tsx index e879ed9472..22a9708073 100644 --- a/apps/web/components/task/executor-environment-info.tsx +++ b/apps/web/components/task/executor-environment-info.tsx @@ -7,6 +7,7 @@ import { toast } from "sonner"; import { type ContainerLiveStatus, + type SSHLiveStatus, type TaskEnvironment, } from "@/lib/api/domains/task-environment-api"; import { resolveExecutorEnvironmentStatus, type StatusTone } from "./executor-environment-status"; @@ -22,10 +23,12 @@ const TONE_CLASSES: Record = { export function EnvironmentInfo({ env, container, + ssh, loading, }: { env: TaskEnvironment | null; container: ContainerLiveStatus | null; + ssh?: SSHLiveStatus | null; loading: boolean; }) { if (loading && !env) { @@ -53,7 +56,7 @@ export function EnvironmentInfo({ {formatExecutorType(env.executor_type)} - + ); } @@ -79,24 +82,42 @@ function StatusBadge({ function EnvironmentFields({ env, container, + ssh, }: { env: TaskEnvironment; container: ContainerLiveStatus | null; + ssh: SSHLiveStatus | null; }) { - const fields = useMemo(() => buildFields(env, container), [env, container]); + const fields = useMemo(() => buildFields(env, container, ssh), [env, container, ssh]); if (fields.length === 0) { return

No resource details available.

; } return (
{fields.map((f) => ( - + ))}
); } -function Field({ label, value, copy }: { label: string; value: string; copy?: boolean }) { +function Field({ + label, + value, + copy, + copyValue, +}: { + label: string; + value: string; + copy?: boolean; + copyValue?: string; +}) { return (
{label}
@@ -109,7 +130,7 @@ function Field({ label, value, copy }: { label: string; value: string; copy?: bo aria-label={`Copy ${label}`} onClick={() => { void navigator.clipboard - .writeText(value) + .writeText(copyValue ?? value) .then(() => toast.success(`${label} copied`)); }} > @@ -121,9 +142,16 @@ function Field({ label, value, copy }: { label: string; value: string; copy?: bo ); } -type FieldRow = { label: string; value: string; copy?: boolean }; +// copyValue overrides the clipboard payload when the displayed value is a +// truncated representation (fingerprint hash suffix, …) — copying the +// truncation is useless. Omit to copy `value` verbatim. +type FieldRow = { label: string; value: string; copy?: boolean; copyValue?: string }; -function buildFields(env: TaskEnvironment, container: ContainerLiveStatus | null): FieldRow[] { +function buildFields( + env: TaskEnvironment, + container: ContainerLiveStatus | null, + ssh: SSHLiveStatus | null, +): FieldRow[] { const rows: FieldRow[] = []; if (env.worktree_path) { @@ -153,9 +181,71 @@ function buildFields(env: TaskEnvironment, container: ContainerLiveStatus | null rows.push({ label: "Sprite", value: env.sandbox_id, copy: true }); } + if (env.executor_type === "ssh" && ssh) { + addSshRows(rows, ssh); + } + return rows; } +function addSshRows(rows: FieldRow[], ssh: SSHLiveStatus) { + // user@host[:port] — matches what a user would paste into an SSH client. + // Suppress :22 since the canonical port reads as noise. + if (ssh.host) { + rows.push({ label: "Host", value: formatHostTarget(ssh), copy: true }); + } + if (ssh.remote_task_dir) { + rows.push({ label: "Workdir", value: ssh.remote_task_dir, copy: true }); + } + const agentctl = formatAgentctlSummary(ssh); + if (agentctl) { + rows.push({ label: "Agentctl", value: agentctl }); + } + if (ssh.fingerprint) { + rows.push({ + label: "Fingerprint", + value: formatFingerprint(ssh.fingerprint), + copy: true, + copyValue: ssh.fingerprint, + }); + } + // Shell affordance: paste-ready ssh command that mirrors how kandev + // connects. Helpful when the user wants to inspect the remote dir by hand. + if (ssh.host) { + rows.push({ label: "Shell", value: formatShellCommand(ssh), copy: true }); + } +} + +function formatHostTarget(ssh: SSHLiveStatus): string { + const userPart = ssh.user ? `${ssh.user}@` : ""; + const portPart = ssh.port && ssh.port !== 22 ? `:${ssh.port}` : ""; + return `${userPart}${ssh.host ?? ""}${portPart}`; +} + +function formatShellCommand(ssh: SSHLiveStatus): string { + const userPart = ssh.user ? `${ssh.user}@` : ""; + const portPart = ssh.port && ssh.port !== 22 ? ` -p ${ssh.port}` : ""; + return `ssh${portPart} ${userPart}${ssh.host ?? ""}`; +} + +// Compact one-liner for the agentctl process: pid → remote:port → local:port. +// Each is optional (e.g. a recovered session may have lost the local +// forward port mid-restore), so the helper omits gracefully. Returns "" +// when nothing is set so callers can skip the row entirely. +function formatAgentctlSummary(ssh: SSHLiveStatus): string { + const parts: string[] = []; + if (ssh.remote_agentctl_pid) parts.push(`pid ${ssh.remote_agentctl_pid}`); + if (ssh.remote_agentctl_port) parts.push(`remote :${ssh.remote_agentctl_port}`); + if (ssh.local_forward_port) parts.push(`local :${ssh.local_forward_port}`); + return parts.join(" → "); +} + +// Show the suffix only — full fingerprint is verbose, and the trust gate +// already pinned it on save. The copy button hands back the full string. +function formatFingerprint(fingerprint: string): string { + return fingerprint.startsWith("SHA256:") ? `…${fingerprint.slice(-12)}` : fingerprint; +} + function formatUptime(startedAt: string): string { const startedMs = Date.parse(startedAt); if (Number.isNaN(startedMs)) return startedAt; @@ -178,6 +268,8 @@ function formatExecutorType(type: string): string { return "Sprites sandbox"; case "remote_docker": return "Remote Docker"; + case "ssh": + return "SSH"; default: return type || "Unknown executor"; } diff --git a/apps/web/components/task/executor-settings-button.tsx b/apps/web/components/task/executor-settings-button.tsx index 33344a870b..149b996e44 100644 --- a/apps/web/components/task/executor-settings-button.tsx +++ b/apps/web/components/task/executor-settings-button.tsx @@ -31,7 +31,7 @@ export function ExecutorSettingsButton({ const isPreparing = isPreparingPhase(prepare.phase); // Promote the foreground polling cadence while preparing so the icon flips // to "ready" without the user hovering over the trigger. - const { env, container, loading, isResetting, reset, status } = useTaskEnvironment( + const { env, container, ssh, loading, isResetting, reset, status } = useTaskEnvironment( taskId, open || isPreparing, ); @@ -80,7 +80,7 @@ export function ExecutorSettingsButton({ data-testid="executor-settings-popover" > - +