Skip to content

feat(workhost): reach a dev host over whichever path is up, and say so - #1287

Open
ZacxDev wants to merge 6 commits into
mainfrom
feat/workhost
Open

feat(workhost): reach a dev host over whichever path is up, and say so#1287
ZacxDev wants to merge 6 commits into
mainfrom
feat/workhost

Conversation

@ZacxDev

@ZacxDev ZacxDev commented Sep 4, 2026

Copy link
Copy Markdown
Member

workhost reaches a dev host over whichever network path is actually working, and reports the state of all of them on every invocation.

$ workhost path
workhost: workbench
  lan        192.168.50.250   untrusted-key   host key for alias 'workbench' not in known_hosts
  nebula     10.42.0.30       untrusted-key   host key for alias 'workbench' not in known_hosts
  tailscale  -                not-configured  no tailscale binary on PATH
  -> run `workhost ssh --accept-key` once to trust it,
     or: ssh -o HostKeyAlias=workbench 192.168.50.250

Design decisions

Four states per path, never two. ok / unreachable / untrusted-key / not-configured. Collapsing them is the defect the tool exists to avoid — "nothing happened" is the observable the most causes share, so it identifies none of them. Tailscale is implemented now and reports not-configured; it lights up with no code change once a tailnet exists, because its address is resolved at runtime from tailscale status --json rather than hardcoded. A tailnet that exists but does not carry the host is also not-configured, with a different detail — a different problem with a different fix. A missing ssh client is not-configured too, for the same reason: blaming the network for a missing package is the same fold one layer down.

untrusted-key, and why the tool could not bootstrap itself. -o HostKeyAlias=<hostname> makes ssh look up the ALIAS in known_hosts and ignore the address entries, and the probe runs BatchMode=yes while StrictHostKeyChecking defaults to ask. On a client that has never trusted that alias — the laptop, i.e. the only machine where this tool does anything, since on workbench it takes the local branch — all three paths reported unreachable on a perfectly healthy network and every verb refused with exit 3. select_path gates every verb (and --path) on state == ok, so no invocation ever reached the interactive ssh that would have prompted.

Reproduced twice before fixing: a fresh client with an empty known_hosts running the exact probe argv got Host key verification failed. and wrote zero bytes to known_hosts; this dev host got Permission denied (publickey,…) instead — and only because workbench ssh-ed25519 … sits at known_hosts line 369 of 370, appended during this PR's own development.

So it is its own state, classified from the probe's stderr (ssh exits 255 for auth, DNS, timeout and host key alike, so the status cannot tell them apart), with REMOTE HOST IDENTIFICATION HAS CHANGED sharing the state but carrying its own detail and its own advice — ssh-keygen -R, never --accept-key, because ssh does not offer to accept a key that changed.

TOFU was rejected; --accept-key is the escape hatch instead. The probe gains no StrictHostKeyChecking override: a probe that silently accepts a new host key trusts the machine on the operator's behalf, every run, with no record that a decision was made. --accept-key widens the set of acceptable states by exactly one — untrusted-key becomes selectable, unreachable never does — and touches nothing else. The selected path is then connected over by the ordinary action command, which carries no BatchMode, so real ssh prints the fingerprint and asks a human once. The probe argv is byte-identical with and without the flag, and a test pins that.

🔴 And the escape hatch has to be reachable by the route the tool prints. It was not. The advice said workhost ssh --accept-key, and that exact command did not work: flags after an action verb belong to the verb, so --accept-key landed in ssh's pass-through args, was never parsed, and the run exited 3 saying "re-run with --accept-key" — the flag just passed. Measured on a fresh client against a real sshd:

workhost --accept-key ssh   -> reached real ssh, host-key prompt path. WORKS.
workhost ssh --accept-key   -> rc 3, "re-run with --accept-key".      LOOPS.

The same "cannot bootstrap itself" defect one layer up, landing exactly when the operator is most stuck and least able to guess the other word order. --accept-key is now hoisted back to workhost from any verb's arguments (HOISTED_FLAGS, a one-entry enumerated exception — workhost run --json still sends --json to the remote command), and because hoisting removes the token from the remote argv, main says so on stderr rather than dropping it silently. The escape for a remote command that genuinely wants the literal flag is the ordinary one for run: quote it.

A string that names a command is a claim about the parser. Every such string was re-checked, enumerated by walking the script's own sys.std*.write / ValueError / parser.error call sites rather than from memory: select_path's error named a flag (it is the message seen while looping) and now names the whole command; the no-spec forward error's example is asserted to be a spec validate_forward_spec itself accepts; the changed-key advice's ssh-keygen -R <name> must name the HostKeyAlias, not an address, or it deletes the wrong known_hosts line; the or: ssh … fallback is now derived from ssh_options() instead of spelled beside it; and scripts/README.md named the same broken form and was false for exactly as long.

The forward error's workhost ssh -N -L <spec> suggestion was suspected of being false too — ssh's synopsis puts options before the destination, and workhost appends verb args after it. Measured instead of assumed, on OpenSSH_10.2p1, with four controls: -L bogusspec before and after the destination both give Bad local forwarding specification; putting a command first (… host echo -L bogusspec) suppresses it; and with no -L it never appears. ssh permutes up to the first non-option token, so the suggestion is correct and was kept — with the measurement and its version scope in a comment.

The probe is a real auth handshake, not a TCP connect. Measured on this fleet: both addresses complete the TCP and SSH transport layers and then fail auth with Permission denied, exiting 255. A port probe calls that reachable and the subsequent connect fails in the operator's face. The probe is ssh -o BatchMode=yes … true.

-o HostKeyAlias=<hostname> on every ssh/scp/rsync/forward/tunnel invocation. One machine reached by three addresses otherwise accrues three known_hosts entries, and a later address reuse produces a host-key-mismatch wall — known_hosts on the dev host already had two raw-address entries for this single machine before the tool existed. rsync gets the options threaded through -e, since it opens its own ssh and would otherwise silently lose the alias. The cost of the alias is exactly the bootstrap problem above, which is now named rather than suffered.

Always probe all three, in parallel, bounded by --timeout. Serial probing would put three timeouts in front of every command. Failures go to stderr so piped stdout carries only the remote command's output; for path/check the report is the product, so it goes to stdout.

A forward that reports success must actually forward. ExitOnForwardFailure defaults to no (measured from man ssh_config on this host), so ssh stayed alive after a failed local bind: workhost forward 8080:… against an already-bound 8080 blocked forever with no verdict while the operator believed the tunnel was up and their connections reached an unrelated process. spec = list(args) was never validated either, so a bare workhost forward built a fully authenticated session with zero -L flags and idled until killed. Both tunnels now carry -o ExitOnForwardFailure=yes, the spec is validated as [bind:]port:host:hostport (with the error naming workhost ssh -N -L … for ssh's rarer spellings), and the forward test pins the whole argv rather than the presence of two tokens.

Exit-code contract, honestly scoped. 3 = no path reachable — a conventional sentinel, not a reserved one. workhost run 'exit 3' over a working path also exits 3, and the usage/unknown-host code 2 likewise collides with a remote exit 2. Nothing in 0–255 is safe from a remote command that picks it, short of abandoning passthrough. So the exit code is a hint and --json is the machine-readable channel: selected is null exactly when no path was usable. Both collisions have their own tests, so the claim cannot get wider than the code again. (An earlier revision of this PR and of the source docstring said 3 was "distinct from any remote exit code"; that was wrong and is retracted.)

Local exec detection uses an interface address, not hostname. hostname is nixos on more than one machine in this fleet — the same measured fact scripts/lib/host_identity.py exists to encode — so it would make the laptop believe it is workbench and silently run commands on the wrong machine. The nebula mesh address is the discriminator, unique across the mesh by construction.

🔴 The LAN address is accepted only when the host table names no nebula address for that host. An unconditional LAN fallback re-created the rejected hazard one layer down: a LAN address is unique within one subnet, so a laptop on a network that hands it 192.168.50.250 satisfied is_local_host(workbench) and ran workhost run 'kubectl delete ns prod' on itself, exit 0, printing nothing in the quiet path that said workbench was never touched. Named cost of the tightening: a host that has a nebula address but whose nebula interface is down no longer recognises itself and SSHes to itself over the LAN — a slow success, the same safe direction local_ipv4_addresses() already degrades in.

kubectl uses an SSH tunnel, not a server rewrite. Measured from the live k3s serving certificate: its SANs contain 127.0.0.1 and 192.168.50.250 but not the nebula address 10.42.0.30. A --server=https://<address>:6443 rewrite is TLS-valid over LAN and TLS-invalid over nebula — it would work right up until the LAN path is the one that is down. Tunnelling to the host's own loopback keeps the cert valid over every path, and uses the local kubectl rather than depending on the remote non-login shell having one (on NixOS it frequently does not). The irony that follows: the newly-depended-on local binary was the one unhandled case — missing kubectl died with a bare traceback and exit 1, because main's FileNotFoundError -> 127 guard sits on the other branch. It now exits 127 with a message. free_local_port() is inherently racy, so the tunnel additionally re-checks that ssh is still alive after the readiness probe connects; without that plus ExitOnForwardFailure, a connect to a port thief would satisfy the probe and kubectl --server would be aimed at an unrelated local service.

Host table, not a code fork. HOSTS has one real entry. laptop/production are deliberately absent rather than guessed. homelab is Talos and has no SSH.

Failing loudly beats crashing. tailscale status --json printing valid-but-non-object JSON (null, []) raised AttributeError and exit 1 from every verb, path included — whose whole job is to report a broken path rather than crash on one. WORKHOST_TIMEOUT=10s raised ValueError while building the parser, so every invocation crashed including --help, with nothing on screen naming the variable; it now warns once and falls back, and --timeout's help names the variable the way --host names $WORKHOST_HOST. --dry-run uses shlex.join, so an argument containing a space prints as the argv that actually runs, and the kubectl dry run says on stderr that its -L 0:… port is a placeholder chosen at run time.

Test matrix

scripts/tests/test_workhost.py158 tests, hermetic: no real network, no real ssh. Every binary (ssh, scp, rsync, tmux, kubectl, tailscale, ip) is a testlib.mockbin stub and PATH is set to that dir alone — a prepend would let a real tailscale installed later silently change what the not-configured tests assert. The ip stub is equally load-bearing in the other direction: this suite is developed on workbench, so without it every "remote" test would take the local branch and pass for the wrong reason. The ssh stub strips ssh's options the way ssh does and then executes the remaining args, so exit-code passthrough is real rather than a canned number.

🔴 The stub used to exit 255 for every failure, so the suite ASSERTED the ssh-255 conflation — nothing in it could tell an auth failure from a host-key refusal, which is precisely the bug F1 turned out to be. It now produces all three: Permission denied for auth, Host key verification failed for a key never seen, and the REMOTE HOST IDENTIFICATION HAS CHANGED block for a key that changed. It also models the mechanism --accept-key depends on rather than a canned success: under BatchMode=yes it refuses the unknown key as ssh does, and without BatchMode it proceeds, as ssh does once a human answers. So test_accept_key_lets_an_untrusted_path_be_used is green only if the flag really causes an interactive attempt.

Covered: path selection across the probe matrix incl. all-fail and precedence; the four-state distinction in both directions (absent vs present-but-down vs tailnet-without-this-host vs untrusted key vs auth failure); the --accept-key opt-in and its two limits (cannot reach the probe, cannot revive an unreachable path); the advice block and its changed-key variant; exit-code passthrough plus both collisions; HostKeyAlias on every path × every ssh-based verb; local vs remote exec, including the wrong-machine hazard pinned as a hazard; --path forcing; --json shape field by field; concurrency; --timeout and every unusable $WORKHOST_TIMEOUT; forward-spec validation; argument forwarding incl. spaces and quotes; stream discipline; delivery.

One test is driven through the module rather than the CLI: test_the_lan_address_is_accepted_when_the_host_has_no_nebula_address. HOSTS deliberately holds exactly one real host and that host has a nebula address, so the conditional branch is unreachable from argv — and inventing a second table entry to test with is the thing test_only_real_hosts_are_in_the_table forbids.

Neither concurrency nor the timeout is asserted on wall-clock time. Concurrency is proven by interval overlap — the stubs timestamp entry and exit, and the test asserts the latest start precedes the earliest end. Serial execution cannot produce overlapping intervals at any load. The --timeout test is likewise structural: the stub sleeps 10s against a 1s deadline, and the timed out detail is reachable only via subprocess.TimeoutExpired, so a dropped timeout changes the reported state rather than the duration.

Red-then-green

Two rounds, each measured with the new test file against the previous scripts/workhost, in a .git-less cp -a copy under PYTHONDONTWRITEBYTECODE=1.

round base result at base at HEAD
the five audit findings 041cd4db 37 failed, 102 passed 139 passed
the unreachable escape hatch 65f68594 9 failed, 149 passed 158 passed

Round 2's nine are the fix item by item: all three untrusted-key rows of the printed-command ledger, the advice/parser pairing test, the follow-the-advice end-to-end test, the README claim, both after-the-verb --accept-key positions, and the hoist notice.

🔴 Red for the right reason, checked. split_argv's return is indexed, not unpacked, in the test helper — against the pre-fix 3-tuple an unpacking ValueError would have turned the pairing test red for a harness reason and proved nothing. The observed failure is the claim: ('workhost ssh --accept-key', 'ssh', ['--accept-key'])accept_key false, flag stranded in the verb's args.

Named honestly as invariant guards, NOT regression coverage — these pass against the base too, because they pin behaviour the fix had to preserve, or a claim rather than a behaviour change. Each is still killed by a mutant below, which is what makes it non-vacuous: test_an_auth_failure_is_unreachable_not_untrusted_key, test_the_probe_never_enables_tofu, test_no_advice_is_printed_when_a_path_is_actually_usable, test_forward_accepts_an_explicit_bind_address, test_a_valid_workhost_timeout_is_still_honoured, test_exit_3_is_distinguishable_from_a_remote_failure, both exit-code collision tests, test_only_declared_flags_are_hoisted, test_nothing_is_said_when_no_flag_was_hoisted, and test_accept_key_is_honoured_in_either_position[before-verb].

Mutation battery — every test watched to fail

scripts/tests/mutants-workhost.sh: mutates a copy in mktemp -d, diffs each mutation before running it, scores on pytest's FAILED …::<test> content rather than an exit code, enforces a MIN_TESTS floor, runs under PYTHONDONTWRITEBYTECODE=1, and verifies the copy is restored byte-identical.

59/59 rows ok (was 24), each killed by the test it names, no survivors. Positive control killed; SURVIVES control survived. Groups added across both rounds:

group mutants
the fourth state untrusted-key-collapsed, hostkey-missing-unclassified, auth-called-a-key-problem, changed-key-loses-its-detail, probe-gains-tofu
cause + way out advice-block-silenced, advice-fires-when-fine, changed-key-offered-tofu
the escape hatch and its limits accept-key-inert, accept-key-too-wide
a forward that forwards forward-bind-failure-ok, kubectl-bind-failure-ok, forward-spec-unvalidated, forward-shape-unchecked, forward-rejects-bind-addr
wrong machine lan-fallback-unconditional, lan-fallback-removed
crashing is not reporting local-kubectl-crash, tailscale-non-object, missing-ssh-blamed-on-net, env-timeout-unguarded, env-timeout-always-default, timeout-help-hides-the-var
--dry-run prints what runs dry-run-space-joined, kubectl-hides-placeholder
a string that names a command accept-key-not-hoisted, hoist-too-wide, hoist-leaves-the-flag, hoist-is-silent, hoist-notice-always-fires, advice-names-a-non-command, manual-line-loses-alias, ssh-keygen-names-address, forward-example-invalid, select-message-drops-cmd

🔴 The battery earned its keep again, on a guard added in this very PR. advice-names-a-non-command — which rewrites the advice to workhost ssh --accept-keys — came back WRONG-KILLER: it died to the two pairing tests but not to the ledger test whose whole job is to check every printed command. Measured directly:

workhost ssh --accept-key    globals=['--accept-key']  verb_args=[]                 accept_key=True
workhost ssh --accept-keys   globals=[]                verb_args=['--accept-keys']  accept_key=False

--accept-keys is not a workhost option, so split_argv hands it to the verb and the global half parses perfectly cleanly. The ledger asserted only "it parses" — so it was green on the exact shape of the defect it exists to catch. It now declares what each printed command must parse TO: workhost_flags rows assert the named options are set and that no --prefixed token was left in the verb's arguments; verb_flags rows assert the verb args exactly, for the commands whose flags really are meant for the verb's own tool. Renamed to test_every_workhost_command_the_tool_prints_parses_as_intended so the name is no wider than the body. A guard that reads as coverage while providing none is worse than none, because it stops anyone looking.

Two stale claims in the battery's own header are corrected, since the file's stated purpose is that its claims be re-derivable: the test count (said 89, was 96, is now 158) and the --deselect of test_workhost_is_tracked_by_git, justified as "cannot pass against a .git-less copy" — which stopped being true at 041cd4d, when that test grew a second arm. The deselect is removed rather than left carrying a dead reason.

Gate

nix develop <worktree> --command bash scripts/gate.sh --tier pytest --set hermetic

GATE: RESULT=FAIL exit=1TOTAL collected=21234 passed=21230 skipped=3 failed=1. The wrapper reported exit code 0 while the gate itself had failed, so the verdict was read from the log CONTENT rather than the status. scripts/tests itself: PASS, collected=12231 passed=12231 (floor 10269) — which is exactly the 158-test workhost file, confirming that target ran the final content.

The one failure is scripts/claude-hooks/tests/test_clawgate_task_interview_guard.py::test_a_body_file_written_by_a_heredoc_on_the_same_line_is_read, and it is not this PR's. Attributed by running that file at three points:

tree result
origin/main (f75c92d4) passes
041cd4db — this PR's head before any of this work fails
this PR's head fails

So it is red on the branch and green on main: the branch is 30 commits behind, and main carries 8c27c5cf (#1303) — "a stale file at the --body-file path shadowed the heredoc about to overwrite it — the verdict was a property of the HOST" — which the branch does not. It is a host-state-dependent guard already fixed upstream; merging main will clear it, and that merge is deliberately not part of these commits.

⚠️ An earlier revision of this PR body called this failure "pre-existing red, verified failing on an untouched origin/main checkout". That is false — main fixed it. Corrected rather than carried forward.

Both tiers, run explicitly (this PR already produced one CI-only failure from the .git asymmetry): dev shell with .git present — 158 passed; cp -a copy with .git removed — 158 passed.

test_no_public_ips.py / test_no_client_hostnames.py / test_no_real_launchers.py re-run after staging, since the first two enumerate git ls-files and cannot see an unstaged file — 110 passed. test_no_real_launchers.py went red once during the first round: a systemctl example in a docstring made workhost a new reacher of an acknowledged-but-unstubbed binary. It is a text scan and the example was illustrative rather than a guarantee, so the example was changed instead of the ledger.

Not verified

  • 🔴 The fix to the escape hatch has NOT been run against a real ssh. What was verified live, on a fresh client with empty known_hosts against the real workbench sshd: that 041cd4db reports unreachable — Host key verification failed. while 65f68594 reports untrusted-key with the advice block, and that workhost --accept-key ssh (flag before the verb) reaches real ssh and enters the host-key prompt path. The thing this round changed — --accept-key written after the verb — is covered hermetically only.
  • The REMOTE HOST IDENTIFICATION HAS CHANGED path has never been seen live, only reproduced from ssh's own message text.
  • The workhost ssh -N -L <spec> escape hatch was measured against OpenSSH_10.2p1 only. An ssh old enough not to permute options after the destination would make that suggestion wrong; that version boundary was not bisected.
  • No real tailnet exists, so the tailscale path is implemented and hermetically tested against a faked binary only.
  • The kubectl tunnel is tested against a stub ssh that really binds the port, and the cert-SAN reasoning is measured — but it has not been run against the live apiserver from a remote client.
  • ExitOnForwardFailure is set from a documented default read out of man ssh_config on this host; the behaviour change (ssh exiting on a failed bind) has not been observed against real ssh, only asserted on the argv. The free_local_port() thief race was likewise not induced.
  • The tailscale JSON parsing was checked against the authoritative struct in ipn/ipnstate/ipnstate.go rather than against a running daemon.
  • These tests were developed on a shared, loaded host.

🤖 Generated with Claude Code

ZacxDev and others added 3 commits September 4, 2026 00:07
Adds `scripts/workhost`, a CLI that probes every network path to a dev host
in parallel on each invocation, reports the state of all of them, then
connects over the best one.

Three states per path, never two: ok / unreachable / not-configured.
Collapsing the last two is the defect the tool exists to avoid. Tailscale is
implemented now and reports not-configured; its address is resolved at
runtime from `tailscale status --json`, so it lights up with no code change
once a tailnet exists.

Measured facts the design rests on, rather than assumed:

* Both workbench addresses complete TCP and the SSH transport layer and then
  fail auth with `Permission denied` (rc 255). A port probe would call that
  reachable, so the probe runs a real BatchMode auth handshake.
* known_hosts on this host already carried two raw-address entries for the
  single machine workbench. Hence `-o HostKeyAlias=<hostname>` on every
  ssh/scp/rsync/forward/tunnel call, threaded through `-e` for rsync.
* The k3s serving cert's SANs include 127.0.0.1 and the LAN address but NOT
  the nebula address, so a `--server=https://<addr>:6443` rewrite is
  TLS-invalid over nebula. kubectl therefore tunnels to the host's own
  loopback, which keeps verification on over every path.
* `hostname` is `nixos` on more than one machine in this fleet, so local-exec
  detection keys on the nebula interface address instead.

A live smoke test caught the tool returning exit 3 on workbench itself: it
demanded a reachable network path before considering local exec. You do not
need a path to the machine you are sitting on; there is a regression test.

Delivery: on PATH via mkOutOfStoreSymlink in nix/home.nix, matching the other
CLIs, with a test asserting it is executable, wired up, and git-tracked.

Tests: 92 hermetic tests (no real network or ssh; every binary is a mockbin
stub and PATH is set to the stub dir alone). Mutation battery
scripts/tests/mutants-workhost.sh scores 21/21, each mutant killed by the
test it names, with a positive control and a SURVIVES control. It earned its
keep: `report-pollutes-stdout` survived the first run because the
stdout-cleanliness test passed vacuously (no report is emitted in the quiet
path), so two tests were added that actually emit one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RMYjPVhL74So5kqc6XTzCW
Claude-Session-Id: 8ad54f57-207e-4fe2-9cec-b9d2fcf014f3
…variant

The full gate caught what the targeted runs could not: test_no_real_launchers
pins every test file that REPLACES PATH rather than prepending, and
test_workhost.py is a new such site, so the suite went red until it was
justified.

Registered with an enumeration justification rather than prose, and backed by
a live assertion (test_the_stub_path_contains_only_declared_stubs) so it
cannot rot: the stub directory's contents must be a subset of
SANDBOX_BINARIES, no launcher-shaped entry may be a symlink, and each must
carry this suite's own WH_LOGDIR marker. This is the first pinned clobber
whose directory deliberately CONTAINS launcher names — ssh, scp, rsync, tmux
and kubectl are all present as stubs, because answering those calls without a
network is the entire point of the suite.

Replacing PATH is required in both directions, not stylistic: the
`not-configured` assertions need `tailscale` genuinely absent and prepending
cannot make a binary unfindable, while the real `ip` on workbench reports the
nebula address, so a prepended PATH would leak the host's true identity into
the tests that must believe they are remote and pass them for the wrong
reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RMYjPVhL74So5kqc6XTzCW
Claude-Session-Id: 8ad54f57-207e-4fe2-9cec-b9d2fcf014f3
…in the other

CI (tekton/devrc-pytests) failed on test_workhost_is_tracked_by_git while the
same test passed locally. Mechanism established, not guessed: the suite runs in
TWO TIERS and they are blind in opposite directions. The dev shell has the
repo's `.git`; the nix sandbox does not — its source is copied in — so
`git -C REPO ls-files --error-unmatch` exits 128 `not a git repository` there.
Reproduced by running the test in a `cp -a` copy with `.git` removed: rc 128,
identical to CI.

The test asserted only the git form, so it could not survive the tier that has
no git — and a test that passes only in the environment its author happened to
run is the failure this file exists to catch, not one it should suffer.

Fixed with two real arms and no skipif (matching test_analyze_service_index_
commit.py's "🔴 NOT a skipif" precedent), each asserting the strongest claim its
tier can make:

  .git present -> ask git whether the path is tracked.
  .git absent  -> we are in the sandbox, whose source is the flake's
                  TRACKED-files-only copy, so the file being present AT ALL is
                  the tracked-ness proof for that tier.

Neither arm is vacuous. Controlled in both tiers and both directions:
  git tree, tracked              -> PASS
  no-git copy, file present      -> PASS
  no-git copy, file deleted      -> FAIL
  git tree, path untracked       -> FAIL
Full suite: 96 passed in the dev-shell tier, 96 passed in a sandbox-shaped tree.

The first version of this fix referenced a constant `SCRIPT` that does not exist
in this file (it is `WORKHOST`). The no-git arm therefore errored rather than
asserting, and the control caught it — without running the sandbox-shaped tier
it would have shipped a second red CI reading as a fixed one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RMYjPVhL74So5kqc6XTzCW
Claude-Session-Id: 8ad54f57-207e-4fe2-9cec-b9d2fcf014f3
@ZacxDev ZacxDev closed this Sep 4, 2026
@ZacxDev ZacxDev reopened this Sep 4, 2026
ZacxDev and others added 3 commits September 4, 2026 22:35
…ork was down

Five audit findings, all reproduced before being fixed.

F1 — `-o HostKeyAlias=workbench` makes ssh look up the ALIAS in known_hosts and
ignore the address entries, while the probe runs `BatchMode=yes` against a
`StrictHostKeyChecking` that defaults to `ask`. On a client that has never
trusted that alias — the laptop, i.e. the only machine where this tool does
anything — every path reported `unreachable` on a healthy network and every verb
refused with exit 3. Reproduced twice: a fresh client with an empty known_hosts
running the exact probe argv got `Host key verification failed.` and wrote zero
bytes to known_hosts; this dev host got `Permission denied` instead only because
`workbench ssh-ed25519 …` sits at known_hosts line 369, appended during this
PR's development.

Fixed by naming it: a fourth state `untrusted-key`, classified from the probe's
stderr (both `Host key verification failed` and, with its own detail text,
`REMOTE HOST IDENTIFICATION HAS CHANGED`), reported with the real cause and the
way out, and `--accept-key` as a one-shot bootstrap. TOFU stays rejected: the
probe gains no `StrictHostKeyChecking` override, and `--accept-key` changes only
which already-probed path `select_path` may return, so the probe argv is
byte-identical with and without it (pinned by a test). The selected path is then
connected over by the ordinary action command, which carries no BatchMode, so
real ssh asks a human once.

The suite previously ASSERTED the conflation: the ssh stub exited 255 for auth
and host-key failures alike, so nothing could tell them apart. The stub now
produces all three flavours and they are pinned apart in both directions.

F2 — `forward` could report success while forwarding nothing.
`ExitOnForwardFailure` defaults to `no`, so ssh survived a failed local bind and
sat there; `spec = list(args)` was never validated, so a bare `workhost forward`
built an authenticated session with zero `-L` and idled; and the kubectl
tunnel's `free_local_port()` race let the readiness probe succeed against a
thief. Adds `-o ExitOnForwardFailure=yes` to both tunnels, validates the spec,
re-checks the tunnel is alive after the probe connects, and replaces the
two-token forward assertion with the exact argv.

F3 — the exit-code claim was wider than the code. `3` is conventional, not
reserved: `workhost run 'exit 3'` also exits 3. Claim narrowed rather than
faked, `--json` named as the machine-readable channel, and the test that said
"distinguishable from a remote failure" while parametrising `[1]` now covers
1/2/42 plus both collisions explicitly.

F4 — `workhost kubectl` with no local kubectl died with a bare traceback and
exit 1: `main`'s FileNotFoundError->127 guard sits on the other branch. Now 127
with a message that keeps the irony visible — this verb tunnels precisely so it
can use the LOCAL kubectl.

F5 — `is_local_host`'s LAN fallback could exec on the wrong machine. A LAN
address is unique within one subnet, so a laptop handed 192.168.50.250 ran
commands locally, exit 0, silently. The LAN address is now accepted only when
the host table names no nebula address; the cost (a host with nebula down SSHes
to itself) is stated rather than hidden.

Also: a non-object `tailscale status --json` no longer crashes every verb;
`WORKHOST_TIMEOUT=10s` warns and falls back instead of raising at parser-build
time, and `--timeout`'s help now names the variable; "no ssh binary on PATH" is
`not-configured` like its tailscale mirror rather than `unreachable`;
`--dry-run` uses shlex.join so the printed line is the argv that runs, and the
kubectl dry run admits its `-L 0:` port is a placeholder.

Tests: 96 -> 139, all hermetic. 37 of the new/changed assertions were watched
RED against 041cd4d's `scripts/workhost` and are green at HEAD. Mutation
battery 24 -> 49 rows, all killed by the test each names, with the positive and
SURVIVES controls intact. Both tiers green (dev shell with .git: 139 passed;
`cp -a` copy with .git removed: 139 passed). The battery's two stale claims are
corrected: the test count, and the deselect whose reason stopped being true at
041cd4d — the deselect is gone rather than left carrying a dead justification.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RMYjPVhL74So5kqc6XTzCW
Claude-Session-Id: 8ad54f57-207e-4fe2-9cec-b9d2fcf014f3
… printed

The untrusted-key report told a stuck operator to run
`workhost ssh --accept-key`. That exact command did not work. Flags after an
action verb belong to the verb, so `--accept-key` landed in ssh's pass-through
args, was never parsed, and the run exited 3 saying "re-run with --accept-key"
— the flag just passed. Measured on a fresh client against a real sshd:

    workhost --accept-key ssh   -> reached real ssh, host-key prompt. WORKS.
    workhost ssh --accept-key   -> rc 3, "re-run with --accept-key". LOOPS.

The same "cannot bootstrap itself" defect one layer up, landing exactly when
the operator is most stuck and least able to guess the other word order.

Both halves fixed.

1. `--accept-key` is hoisted back to workhost from any verb's arguments, so
   both orders work. The positional rule is otherwise untouched: `HOISTED_FLAGS`
   is an enumerated, one-entry exception, and `workhost run --json` still sends
   `--json` to the remote command. The cost is named rather than hidden —
   hoisting removes the token from the remote argv, so `hoist_bootstrap_flags`
   returns what it took and `main` says so on stderr. Silently dropping it was
   the one option that was not acceptable. The escape for a remote command that
   genuinely wants the literal flag is the ordinary one for `run`: quote it.

2. The advice and the parser are now pinned TO EACH OTHER. The printed command
   is extracted from real output and fed back through `split_argv` plus the
   parser, asserting `accept_key=True` and that the flag is not also left in the
   verb args; a second test takes the printed command and RUNS it, asserting it
   reaches an interactive (no-BatchMode) connection. A test that grepped the
   advice for the substring `--accept-key` would have been green against the
   broken behaviour the whole time, which is why that is not what these do.

Re-checked the same class across every emitted string, enumerated by walking
the script's own `sys.std*.write` / `ValueError` / `parser.error` call sites
rather than from memory. Four more claims, all now pinned by a ledger of
scenarios that runs each printed command through the parser:

  * `select_path`'s error named a FLAG, not a command — it is the message the
    operator saw while looping. It now names the whole command, and gets its own
    ledger row (`--json` suppresses the text advice, so that row cannot be
    satisfied on its behalf by the report block).
  * the no-spec `forward` error names an example spec; a test asserts
    `validate_forward_spec` itself accepts it.
  * the changed-key advice's `ssh-keygen -R <name>` must name the HostKeyAlias,
    not an address, or it deletes the wrong known_hosts line. Pinned against
    `ssh_options()`.
  * the `or: ssh …` fallback is now DERIVED from `ssh_options()` instead of
    spelled beside it, so it cannot drift into advice that omits the alias.
  * `scripts/README.md` names `workhost ssh --accept-key` too, and was false for
    exactly as long. Now covered by the same parser pin.

The `forward` error's `workhost ssh -N -L <spec>` suggestion was SUSPECTED of
being false — ssh's synopsis puts options before the destination and workhost
appends verb args after it. Measured instead of assumed, on OpenSSH_10.2p1,
with four controls: `-L bogusspec` before and after the destination BOTH give
"Bad local forwarding specification"; putting a command first suppresses it; and
with no `-L` it never appears. ssh permutes up to the first non-option token, so
the suggestion is correct. Kept, with the measurement and its version scope in a
comment.

Tests 139 -> 158. Six are red against 65f6859's script and green here, each on
the claim rather than on a harness detail (`split_argv`'s return is indexed, not
unpacked, so the pre-fix 3-tuple does not turn the pairing test red for the
wrong reason). Mutation battery 49 -> 59 rows.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RMYjPVhL74So5kqc6XTzCW
Claude-Session-Id: 8ad54f57-207e-4fe2-9cec-b9d2fcf014f3
…g it was written for

Found by the mutation battery, not by inspection. The row that mutates the
advice string to a command the parser cannot honour —
`workhost ssh --accept-keys` — came back WRONG-KILLER: it died to the two
pairing tests but NOT to the ledger test whose whole job is to check every
printed command.

Measured directly:

    workhost ssh --accept-key    globals=['--accept-key']  verb_args=[]                  accept_key=True
    workhost ssh --accept-keys   globals=[]                verb_args=['--accept-keys']   accept_key=False

`--accept-keys` is not a workhost option, so `split_argv` hands it to the VERB
and the global half parses perfectly cleanly. The ledger asserted only "it
parses", so it was green — on the exact shape of the defect it exists to catch.
A guard that reads as coverage while providing none is worse than no guard,
because it stops anyone looking.

The ledger now declares what each printed command must parse **TO**, not that
it parses: `workhost_flags` rows assert the named options are set AND that no
`-`-prefixed token was left in the verb's arguments; `verb_flags` rows assert
the verb args exactly, for the commands whose flags really are meant for the
verb's own tool (`workhost ssh -N -L <spec>`). Renamed to
`test_every_workhost_command_the_tool_prints_parses_as_intended` so the name is
no wider than the body.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RMYjPVhL74So5kqc6XTzCW
Claude-Session-Id: 8ad54f57-207e-4fe2-9cec-b9d2fcf014f3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant