fix(ssh): stop readiness on host-key rejection

[shunkakinoki]

SSH readiness currently treats a rejected host key as an ordinary startup delay, discards the diagnostic, and repeats until the readiness deadline. Stop on the first host-key verification failure with a sanitized explanation and guidance to verify the lease identity and host trust.

The change covers direct, proxy, and WSL readiness paths. The detector retains only fixed-size streaming match state, not diagnostic text, so split writes and output beyond 64 KiB cannot erase a rejection. The returned readiness explanation remains sanitized, while the original exit cause is retained. Authentication startup failures, connection failures, toolchain readiness, cancellation, and workspace-owner failures retain their existing behavior. No host keys are removed and verification is never weakened.

This complements the provider trust-isolation fix in #1785 and the incident in #1748. It does not replace that fix: new leases still need the correct trust scope. This PR changes only failure handling and diagnosis, with no provider configuration or credential changes.

Validation used the repository's Go 1.26.5 toolchain:

• Native OpenSSH regression against an isolated loopback server: the previous source waited until cancellation on both direct and proxy paths; the new source returns immediately and leaves the old pin byte-identical. No provider resources or account SSH keys were used.
• go test -race ./internal/cli -run 'Test(WaitForSSHReady|SSHReadiness|ResolveSSHPortNoInput|ProxySSHReadiness|WSL2Readiness)' -count=1 -timeout=2m
• go test -race ./internal/providers/asciibox -count=1 -timeout=2m
• go vet ./internal/cli ./internal/providers/asciibox
• go build -trimpath -o <isolated-output> ./cmd/crabbox
• git diff --check

The full repository suite and a new paid-provider lifecycle were not run. The local executable was built for verification only; this does not update fleet installations.

Maintainer follow-up and current proof

The original fix is preserved, with two gaps repaired: host-key rejection during WSL's separate SFTP connection now stops before port fallback, and streaming detection survives arbitrary write boundaries and oversized stderr without changing the existing bounded-buffer truncation contract. Three overflow cases and two WSL SFTP cases failed on the submitted head before these additions; the split-write and no-phrase controls passed.

On integrated head 1621a5d1525b0f00f1c5e631afe6d61162ab8509, the related CLI race suite passed in 44.542 seconds with no skips, including both real OpenSSH loopback cases and byte-identical known-hosts checks:

GOTOOLCHAIN=go1.26.5 go test -race -count=1 -timeout=5m \
  -run 'Test.*(SSH.*Readiness|SSHReady|WSL2SSHReady|WSLSFTP|WSLStage|WaitForSSHReadyRejectsChangedHostKey)' ./internal/cli

The proxy test exercises the proxy routing branch against the owned local server, not a hosted provider gateway. WSL coverage uses the existing synthetic SSH recorder and in-process SFTP fixtures; native Windows is not claimed. No raw stderr/key text is retained by the detector, no account SSH keys or provider resources were used, and no trust checks were relaxed.

Managed Codex review is P0 scoped-clean. The SSH documentation and Unreleased entry are maintainer-added landing work; contributor credit remains with @shunkakinoki. Published release-note sections are unchanged.

Landing verification

All applicable exact-head workflows passed: CI, Release Check, Connector E2E Smokes, and Docs UI Proof. The full repository suite was not run locally; this CI result is separate from the focused local proof above. CodeQL default setup does not analyze fork pull requests, so no CodeQL result is claimed here.

The optional bot review produced no current-head verdict: its input-safety classification stopped on pre-existing test content. That input remains blocked and no scanner policy or fixture qualification was changed. The independent managed scan/review and maintainer inspection above are separate evidence, not a qualification of the bot's input. Landing uses the ordinary maintainer merge path with GitHub protections intact, not bot automerge or an administrative override.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review blocked

Automated review did not run, so no review verdict was produced.

Reason: The input-safety check rejected material in this revision. No detected value, path, or scanner output is reproduced here.

ClawSweeper will not retry this unchanged revision.

Next step: If this is a genuine credential, remove and rotate it. If it is an intentional test fixture, a maintainer must review and qualify it.

View the workflow run.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 5, 2026, 5:50 AM ET / 09:50 UTC.

ClawSweeper review

What this changes

The CLI captures bounded SSH diagnostics and stops readiness waits on detected host-key rejection, with regression tests and troubleshooting guidance.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked before merge - 5 items remain

This remains useful work: main and v0.49.1 still discard the diagnostic, and the linked provider fix addresses a different problem. Two introduced regressions prevent approval.

Priority: P2
Reviewed head: c0a0f2a143bfdea0ac7ab2407f809080e579f673

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The focused implementation and native fixture are useful, but two behavioral regressions and the missing execution record limit readiness.
Proof confidence	🦐 gold shrimp (3/6)	Needs stronger real behavior proof before merge: The captured body reports production readiness exercised through native OpenSSH against a loopback server, changing timeout behavior to immediate rejection while preserving the pin. That is relevant real-transport coverage, but no captured run output accompanies the report, and the fixture does not cover fallback selection. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	2 actionable review findings remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: The captured body reports production readiness exercised through native OpenSSH against a loopback server, changing timeout behavior to immediate rejection while preserving the pin. That is relevant real-transport coverage, but no captured run output accompanies the report, and the fixture does not cover fallback selection. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	10 items	Verified scope: The checkout matches the pinned PR head, and the complete introduced diff changes five files without provider, dependency, workflow, or stored-format changes. Still needed on main: Main uses quiet probes, recognizes workspace-owner setup failures, and otherwise continues its readiness loop after host-key rejection. Latest release still affected: The supplied latest release, v0.49.1, contains the same quiet-probe readiness behavior.
Findings	2 actionable findings	[P1] Preserve later verified ports after one candidate rejects its key [P2] Classify host-key failures from client diagnostics only
Security	None	None.

How this fits together

Crabbox checks a runner’s SSH connection and toolchain before syncing files or executing commands. Provider-supplied targets select direct, proxy, or WSL readiness probes.

flowchart TD
  A[Runner connection details] --> C[SSH readiness probes]
  B[Existing host trust] --> C
  C --> D{Probe result}
  D -->|Ready| E[Sync or execute]
  D -->|Host-key rejection| F[Stop with diagnostic]
  D -->|Startup pending| G[Bounded wait]
  G --> C

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The captured body reports production readiness exercised through native OpenSSH against a loopback server, changing timeout behavior to immediate rejection while preserving the pin. That is relevant real-transport coverage, but no captured run output accompanies the report, and the fixture does not cover fallback selection. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Preserve later verified ports after one candidate rejects its key (P1) - With configured fallback ports, a stale pin on the first reachable port now returns from the entire readiness operation before trying a later port with a valid pin. Main tries those candidates in order, as documented in docs/features/network.md; the same regression occurs in the proxy and WSL loops. Keep the rejected endpoint unusable, but preserve selection of another configured endpoint that independently verifies, and add a multi-port regression.
• Classify host-key failures from client diagnostics only (P2) - This buffer contains remote stderr as well as local OpenSSH diagnostics, and SSH also returns 255 when the remote command exits 255. A supported external-provider readyCheck that invokes another SSH connection can therefore emit this message over an already trusted connection and incorrectly terminate readiness with advice to repair the runner’s host trust. Preserve diagnostic provenance and add a case where remote stderr contains the marker with exit 255 while local host verification succeeds.
• Resolve merge risk (P2) - Existing multi-port configurations can lose a working, trusted fallback after upgrade when an earlier port has a stale pin; the supplied native coverage uses only single-port targets.
• Complete next step (P2) - Fix both readiness regressions and attach the native OpenSSH run transcript; redact keys, IPs, private endpoints, and other private details. Terminal output is sufficient. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review.

Findings

• [P1] Preserve later verified ports after one candidate rejects its key — internal/cli/ssh.go:357-358
• [P2] Classify host-key failures from client diagnostics only — internal/cli/ssh_readiness.go:22-23

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test growth	production +37 net lines; tests +93; docs +6	Production growth is justified by bounded diagnostic capture and readiness classification, with most additions devoted to regression coverage.

Merge-risk options

Maintainer options:

1. Preserve verified fallback selection (recommended)
   Keep rejected endpoints unusable while allowing remaining configured candidates to verify, and cover the existing multi-port upgrade behavior.
2. Explicitly approve stopping all candidates
   Accept the fallback compatibility break only with documented maintainer approval and evidence showing its effect on existing configurations.

Technical review

Best possible solution:

Stop genuine client host-key failures promptly while preserving verified fallback selection and retryable remote checks, with compatibility coverage for existing multi-port configurations.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: a rejected primary exits before a valid fallback, and a trusted remote readiness command returning 255 with the matching stderr text is misclassified. These scenarios were not executed during this read-only review.

Is this the best way to solve the issue?

No, not as written: the narrow repair should distinguish local SSH rejection from remote command failure and preserve the established candidate-selection contract.

Full review comments:

• [P1] Preserve later verified ports after one candidate rejects its key — internal/cli/ssh.go:357-358
  With configured fallback ports, a stale pin on the first reachable port now returns from the entire readiness operation before trying a later port with a valid pin. Main tries those candidates in order, as documented in docs/features/network.md; the same regression occurs in the proxy and WSL loops. Keep the rejected endpoint unusable, but preserve selection of another configured endpoint that independently verifies, and add a multi-port regression.
  Confidence: 0.96
• [P2] Classify host-key failures from client diagnostics only — internal/cli/ssh_readiness.go:22-23
  This buffer contains remote stderr as well as local OpenSSH diagnostics, and SSH also returns 255 when the remote command exits 255. A supported external-provider readyCheck that invokes another SSH connection can therefore emit this message over an already trusted connection and incorrectly terminate readiness with advice to repair the runner’s host trust. Preserve diagnostic provenance and add a case where remote stderr contains the marker with exit 255 while local host verification succeeds.
  Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against add89881ae9b.

Labels

Label changes:

• add P2: This fixes misleading readiness waits after SSH trust rejection, with a bounded CLI impact.
• add merge-risk: 🚨 compatibility: The new terminal return can prevent existing configurations from reaching a later SSH port that still passes host verification.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The captured body reports production readiness exercised through native OpenSSH against a loopback server, changing timeout behavior to immediate rejection while preserving the pin. That is relevant real-transport coverage, but no captured run output accompanies the report, and the fixture does not cover fallback selection. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

• P2: This fixes misleading readiness waits after SSH trust rejection, with a bounded CLI impact.
• merge-risk: 🚨 compatibility: The new terminal return can prevent existing configurations from reaching a later SSH port that still passes host verification.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The captured body reports production readiness exercised through native OpenSSH against a loopback server, changing timeout behavior to immediate rejection while preserving the pin. That is relevant real-transport coverage, but no captured run output accompanies the report, and the fixture does not cover fallback selection. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Verified scope: The checkout matches the pinned PR head, and the complete introduced diff changes five files without provider, dependency, workflow, or stored-format changes. (c0a0f2a143bf)
• Still needed on main: Main uses quiet probes, recognizes workspace-owner setup failures, and otherwise continues its readiness loop after host-key rejection. (internal/cli/ssh.go:349, add89881ae9b)
• Latest release still affected: The supplied latest release, v0.49.1, contains the same quiet-probe readiness behavior. (internal/cli/ssh.go:349, 775ebb1160d6)
• Established fallback contract: The documented automatic-selection contract tries configured candidates in order until one connects. The introduced returns inside the direct, proxy, and WSL candidate loops stop selection before later candidates can verify. (docs/features/network.md:185, c0a0f2a143bf)
• Client and remote diagnostics are conflated: The classifier matches status 255 and text from the combined stderr destination. External providers support configurable readyCheck commands, and the existing local-diagnostics helper forwards client logs into that same destination after execution; remote command output can therefore satisfy the new classifier. (internal/cli/ssh_readiness.go:22, c0a0f2a143bf)
• Configurable remote readiness entrypoint: The external-provider protocol copies the supplied readyCheck into the SSH target, establishing a supported path for remote readiness commands whose stderr and exit status are not local SSH diagnostics. (internal/providers/external/protocol.go:182, c0a0f2a143bf)

Likely related people:

• Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Preserve verified fallback candidates and cover direct, proxy, and WSL candidate selection.
• Separate client diagnostics from remote stderr and cover a remote exit-255 false positive.
• Attach redacted native OpenSSH before/after output bound to the revised source.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.