Repository navigation
fix(agent): exec refuses a typed ssh to a machine the registry knows - #552
Conversation
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the SSH detector must identify the executable and exact registered destination before refusing or naming a replacement.
I found three concrete failures in the new guard, detailed inline: ordinary path-qualified SSH invocations bypass it; same-host connections on different ports are resolved to the first row; and an unregistered address sharing a registered address's prefix is falsely refused.
Verification: uv run pytest tests/test_shell_machine_channel.py -q (32 passed), and uv run pytest tests/test_shell_comments.py tests/test_shell_approval.py tests/test_shell_background.py tests/test_rpc_spine.py tests/test_sandbox_unit.py -q (454 passed). I also reproduced all three cases directly against raw_ssh_target; the current tests do not cover them. GitHub's full check suite is green.
Coverage: I checked the repository rules and canonical Machine definition, the full diff, ExecTool callers and permission ordering, relevant registry/transport code and history, backward-compatibility claims, test changes for weakening, and the machine-identity architecture constraint.
84b22ed to
8ab4d06
Compare
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: compound-shell SSH and explicit -o Port destinations still need to be refused.
The three findings from the prior revision are fixed and their threads are resolved. This revision still has two concrete bypasses, detailed inline: valid compound shell lines can hide a registered SSH execution, and OpenSSH's explicit -o Port=... form is treated as port 22.
Verification: uv run pytest tests/test_shell_machine_channel.py -q passed 37 tests, and uv run pytest tests/test_shell_comments.py tests/test_shell_approval.py tests/test_shell_background.py tests/test_rpc_spine.py tests/test_sandbox_unit.py -q passed 454 tests. Direct reproductions confirmed both bypasses. GitHub's checks otherwise pass, but the coverage gate currently fails at 87.72% diff coverage (50/57 changed executable lines).
Coverage: I checked the repository rules and canonical Machine definition, the old-to-new delta and full diff, ExecTool callers and permission ordering, registry/transport behavior and relevant history, backward compatibility, test changes for weakening, and the machine-identity architecture constraints.
8ab4d06 to
244f610
Compare
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the open compound-command and explicit-port findings still need code changes.
This revision adds four coverage tests only; raven/agent/tools/machine_exec.py is unchanged from the prior revision. Re-running the exact reproductions still returns None for both reported bypasses, so those two threads remain open. I found no new defect in the test-only delta, and the added tests do not weaken existing assertions.
Verification: uv run pytest tests/test_shell_machine_channel.py -q passed 41 tests, and uv run pytest tests/test_shell_comments.py tests/test_shell_approval.py tests/test_shell_background.py tests/test_rpc_spine.py tests/test_sandbox_unit.py -q passed 454 tests. At review time all completed GitHub checks pass; unit shard 4/4 is still running and the coverage gate has not yet reported.
Coverage: I checked the repository rules and canonical Machine definition, the prior-to-current delta and full diff, callers and permission ordering, relevant history, backward compatibility, test-strength changes, and the machine-identity architecture constraints.
244f610 to
4696085
Compare
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Both open findings are fixed in this revision: compound commands now expose every SSH destination, including after unspaced operators, and -o Port=... participates in registry matching. I reproduced the original failing cases, replied to both threads, and resolved them. A sweep confirms every thread I opened is now resolved.
Verification: uv run pytest tests/test_shell_machine_channel.py -q passed 46 tests, and uv run pytest tests/test_shell_comments.py tests/test_shell_approval.py tests/test_shell_background.py tests/test_rpc_spine.py tests/test_sandbox_unit.py -q passed 454 tests. At review time every completed GitHub check passes; unit shard 4/4 is still running.
Coverage: I checked the repository rules and canonical Machine definition, the prior-to-current delta and full diff, callers and permission ordering, relevant history, backward compatibility, test-strength changes, and the machine-identity architecture constraints. I also applied the fourth-round convergence bar to parser edge cases and found none that justify another blocker.
|
Blocking: 1 finding needs correction before this revision can merge; see the inline notes. Scope: I reviewed the I reviewed the new detector and its single call site, re-ran every reproduction the earlier Confirmed findings:
Verification:
Checked and deliberately not reported: the detector is spelling-based, so |
4696085 to
c1f84cc
Compare
|
Revision c1f84cc answers both threads from the last round. Blocker (machine_exec.py, the -o branch): the option value is now split on an Nonblocking (the operator set): adding Tests: 12 new cases (two parametrised ExecTool tests for the spaced -o |
Two field runs on 2026-09-14 put a registered machine's address in the task statement, and the coding nodes typed "ssh -p <port> root@<ip> '... &'" from the local shell 58 times to start GPU work. The look-at-it channel is capped at 60 s and the on-call agent's job runner was not theirs to call, so the raw address was the path of least resistance. The cap, the process-group sweep that stops an orphan from holding a GPU, and the ledger all live on the other two paths, so none of that work was metered. The plain shell now refuses a command whose tokens run the ssh client to a destination the registry holds, naming both the machine and the two paths meant for it. The command is a shell line, not an argv. It is tokenised with punctuation_chars so an unspaced operator separates commands the way a shell reads them -- "true&&ssh" is two of them, and plain splitting hands back one word that is neither -- and every ssh in the line is read, because a compound line reaches each of its commands and stopping at the first let a registered destination through on the strength of an unregistered one beside it. A redirection is not the end of a command, and it is not a word either: "ssh 2>&1 -p 58717 root@host" is one ssh whose host follows the plumbing. Reading ">" as a terminator, or "&>" as the destination, lost the registered host after it; the operator and the file it names are stepped over instead, and the bare descriptor that punctuation_chars hands back in front of "2>&1" with it. The executable is recognised as "ssh", "/usr/bin/ssh" or "\ssh" alike: a leading backslash only suppresses alias lookup and an absolute path only skips PATH. The destination comes from ssh's own arguments rather than a search of the text, and the port from "-p" or from an "-o" port option in either spelling OpenSSH honours -- "-o Port=N" and "-o 'Port N'" print the same "port N" under ssh -G: the registry holds several machines at one address, so the address alone names the wrong one, and 203.0.113.70 is not 203.0.113.7. Untouched: an unregistered host still runs, scp and rsync still run (they move files and start nothing on the far side), and a registry that cannot be read or a line that cannot be tokenised recognises nothing, so ops being broken never takes the plain shell down with it. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The prior blocker and operator-set note both reproduce as fixed: spaced -o Port values now match the registered port, and redirections before the destination no longer hide it. I withdrew my confirming blocker on LivXue's thread; that thread remains for its owner to resolve.
Named nonblocking follow-up: repeated conflicting Port settings use the wrong precedence. OpenSSH reports port 58717 for ssh -G -o Port=58717 -o 'Port 22' example.com because the first obtained value wins, while _ssh_destinations and the new test choose 22. This requires contradictory duplicate settings and removing the later duplicate is an immediate working path, so it does not meet the late-round blocker bar.
Verification: uv run pytest tests/test_shell_machine_channel.py -q passed 58 tests, and uv run pytest tests/test_shell_comments.py tests/test_shell_approval.py tests/test_shell_background.py tests/test_rpc_spine.py tests/test_sandbox_unit.py -q passed 454 tests. GitHub had not reported checks for this force-pushed revision at review time.
Coverage: I checked the repository rules and canonical Machine definition, the prior-to-current delta and full diff, callers and permission ordering, relevant history, backward compatibility, test-strength changes, and the machine-identity architecture constraints. All threads I opened remain resolved.
c1f84cc to
ecc9cc1
Compare
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The rebase onto current main preserved the reviewed detector and its tests byte-for-byte; the PR diff is still the same four files, and the prior fixes for spaced Port options and shell redirections still reproduce.
Named nonblocking follow-ups: the repeated conflicting-Port precedence mismatch remains as documented in the prior review. Separately, the detector treats any token named ssh as an executable position: echo ssh -p 58717 root@203.0.113.7 is refused as reaching the registered machine even though the shell only prints those words. That can obstruct diagnostics or script generation. This change introduces the false positive and it is reachable in ordinary use, but an operator has working alternatives and it does not reopen the raw-SSH bypass, so it does not meet the round-seven blocker bar.
Verification: uv run pytest tests/test_shell_machine_channel.py -q passed 58 tests; uv run pytest tests/test_shell_comments.py tests/test_shell_approval.py tests/test_shell_background.py tests/test_rpc_spine.py tests/test_sandbox_unit.py -q passed 454 tests. At review time, 17 GitHub checks passed, the coverage gate was still pending, and docs deploy was skipped.
Coverage: I checked the repository rules and canonical Machine definition, the full diff and rebase delta, the detector's single caller and permission ordering, relevant history, backward compatibility, test-strength changes, and the machine-identity architecture constraints. All threads I opened remain resolved; the two open LivXue threads are not mine to resolve.
Reviewed 2026-09-21. ssh takes the first obtained value for every option, and -p and -o Port queue together: `ssh -G -p 2222 -o Port=58717 host` prints 2222, and the two reversed prints 58717 (OpenSSH 9.9p2). The detector overwrote instead, so `-p 58717 -p 22` read as port 22 -- a command that really reaches the registered machine on 58717 read as unregistered and ran on the plain shell path, past the cap, the process-group sweep and the ledger. The prior test pinned the wrong semantics under the name "later-option-wins"; it is corrected and joined by six cases covering both spellings in both orders, all failing on the previous head. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
|
Revision ba233ba: the named nonblocking precedence finding is fixed, and the You were right and my earlier test had it backwards. Measured on OpenSSH 9.9p2, The detector overwrote instead, so it was not only the conflicting-option case: The prior case in the redirection table asserted 22 under the id The second named follow-up -- Verification on this head: tests/test_shell_machine_channel.py 64 passed; all |
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
The repeated-Port precedence finding is fixed. I compared all six new -p/-o Port orderings with OpenSSH and the parser now keeps the same first obtained value; the corrected expectation is backed by broader coverage rather than a weakened test. The merge-up preserves the prior fixes and, against the refreshed current main, the PR remains the same four-file scope.
The previously named executable-position false-positive remains the only nonblocking follow-up. I found no new defect on this revision.
Verification: uv run pytest tests/test_shell_machine_channel.py -q passed 64 tests; uv run pytest tests/test_shell_comments.py tests/test_shell_approval.py tests/test_shell_background.py tests/test_rpc_spine.py tests/test_sandbox_unit.py -q passed 454 tests. GitHub CI was still in progress at review time, with 16 checks passed and three unit shards pending.
Coverage: I checked the repository rules and current canonical Machine definition, the full diff and prior-to-current delta, the detector's caller and permission ordering, relevant history, backward compatibility, test-strength changes, and the machine-identity architecture constraints. All threads I opened remain resolved; the two open LivXue threads are not mine to resolve.
|
Blocking: 2 findings need correction before this revision can merge; see the inline notes. Scope: I reviewed the Both findings I filed at Confirmed findings:
All three share one mechanism: an option shape the scan does not model yields not an unknown Verification:
Checked and deliberately not reported: the documented deviation for a destination that is a bare |
Two spellings ssh honours let a connection to a registered machine past the guard (review on #552, 2026-09-21): - a bundled short-flag group: `ssh -vp 58717 root@h` was skipped as a group taking no argument, so 58717 became the destination; - an option after the host: OpenSSH re-enters its option loop once it has the destination and reads on until the first non-option word or `--`, so `ssh root@h -p 58717 true` connects on 58717 while the scan, which stopped at the host, read port 22. Groups are now read letter by letter until one takes a value, and the scan continues past the host until the remote command starts. The value-taking set is derived from ssh's own getopt string (OpenSSH_9.9p2) rather than kept by hand; the hand-kept one had lost `B`, so `ssh -B lo ...` read the interface name as the destination. The expectations are measured with `ssh -G`, and a parametrised test re-measures them against the ssh on the machine running the suite. Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
…ssh_to_registered_machine # Conflicts: # CHANGELOG.md
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
I reproduced the three author-described parser corrections on this revision: bundled option groups now consume their values as getopt does, options after the destination affect the connection only until the remote command or --, and the value-taking option table includes -B and current -P. The new compatibility table agrees with the installed OpenSSH client, so the tests strengthen rather than mask the behavior. The merge-up leaves the pull request at the same four-file scope.
The previously named executable-position false positive remains the only nonblocking follow-up. I found no new defect on this revision.
Verification: uv run pytest tests/test_shell_machine_channel.py -q passed 94 tests; uv run pytest tests/test_shell_comments.py tests/test_shell_approval.py tests/test_shell_background.py tests/test_rpc_spine.py tests/test_sandbox_unit.py -q passed 475 tests. GitHub checks are green except unit shard 4/4, which is still running.
Coverage: repository rules and the canonical Machine definition, the full pull-request diff and revision delta, the detector's caller and ordering, relevant history, backward compatibility, test strength, and the stated architecture constraints. All threads I opened remain resolved; the remaining review threads were opened by another reviewer.
…onfig's own list Review findings on the registry-writer PR (2026-09-24). Blocker: a process that started with no machine kept an `exec` schema without `machine` after the first `ops_connection_add`, because the registry serves a tool from the copy it took at admission and `ExecTool` authored no `to_schema` -- so the reply, the typed-ssh refusal from #552 and the new Raven-Code guide all sent the model to `machine=<id>` while the schema it was shown had no such parameter. `ExecTool.to_schema` is authored now, which is how a tool declares a dynamic shape; the shape moves only when the registry goes from empty to not. The registry falls back to the owner's home only when there is none beside the instance's own config, in `raven.ops.connections.store_path` and in the on-call launcher's `connections_registry`. Home-first was right for a sub-agent, whose config sits in a state directory nothing writes, but it silently moved a `--config /x/config.json` host off `/x/connections.json` as soon as a registry appeared in the home -- which an agent's first add now writes with no owner action. A sub-agent still resolves the home: nothing is ever written beside its rendered config. Also: `ops_connection_add` joins the settings tool table's `run` group, and the guard that diffs that table against a loop's registry turns the flag on, so it can see opt-in tools; `CONTEXT.md`'s Machine entry and both on-call doc pages describe the lookup and the tool; the plugin's orphaned `_PROBE` is gone; and #552's ssh-version test asks the client whether `-P` takes a value instead of looking for the string "tag", which an older client prints as the hostname -- so it never skipped on OpenSSH 8.9. Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
… can add a machine (#690) ## Why In a research -> code -> oncall chain run on 2026-09-22, Raven-Code called `exec(machine="conn_cpu_32c")` and got "No connection is registered", on a home whose `connections.json` listed seven machines. It then read host/port/key out of the config and ran a raw `ssh root@<ip> -p <port> -i ~/.ssh/id_rsa`, putting the way in into the transcript. Two gaps: 1. **Read side.** `raven.ops.connections.store_path()` looked only beside the instance's own config. A sub-agent runs on a rendered config in its own state directory, so it looked in an empty directory. The host already hands every sub-agent `RAVEN_HOME`; the registry just was not looked for there. Only the on-call launcher worked around this (it sets `RAVEN_CONNECTIONS` itself). 2. **Write side.** `ops_connection_add` (#545) lived in the on-call plugin. In the usual chain the coding agent runs first, meets the empty registry first, and had no way to take the owner's answer. ## What changes - `store_path()`: env var -> `raven_home()/connections.json` if it exists -> beside the config if THAT exists -> the home (where a first add lands). An install that kept its own list beside its config keeps working. - New `raven/ops/connection_add.py`: `probe`, `ssh_defaults`, `parse_probe`, `write`, `write_ssh_alias`, and `ASK_OWNER` (the questions relayed to the owner). The tool calls these functions directly; `raven ops connection add` (the terminal command for a person) calls the same ones, so the logic exists once. - New `raven/agent/tools/connection_add.py`: `ops_connection_add` as a trunk tool, **opt-in** via `tools.connectionAdd` (on in raven-code and raven-oncall, off in the other three products, whose pinned tool faces are unchanged). - `raven.ops.transport.make_ssh_runner(identities_only=)` + `ISOLATE_IDENTITY`, moved from the plugin: a key the tool picked itself is probed with that key alone (see #545 review). - On-call plugin: `oncall_flow/connections.py` imports trunk's reader instead of keeping a hand-aligned copy (758 -> 212 lines); keeps only campaign lookups (`get`, `display_name`, `resolve_into`, `describe`). Its own tool file, factory and manifest row are removed. - Raven-Code guide (`TOOLS_CODE.md`) gains when to leave this computer: write here; verify where the software is; ask the owner for a machine only when installing the software here is wrong (GPU, memory) or heavy (compiled solver, toolchain, long download); never ssh from an address in the task statement. - `SHOWN` made public in `raven.ops.connections` (the plugin may not import an underscored trunk name; `_SHOWN` stays as an alias). - Docs: `docs-site/docs/oncall.md` / `oncall.zh.md` updated for the new lookup and the tool. ## Not in scope - The pre-dispatch registry gate removed in a44664d is NOT restored. Where a job runs is still decided inside the agent that runs it; this only makes that agent find the owner's registry and able to add to it. - #552 (exec refuses a typed ssh to a registered machine) is complementary: it only fires once the registry is visible, which this makes true for Raven-Code. ## Tests - `tests/test_ops_connection_add.py` (moved from the plugin suite, patches trunk modules), including the ssh `-G` measurement that only the candidate key is offered under isolation. - Five new cases in `tests/test_ops_connections.py` for the lookup order. - Plugin parity tests replaced by an identity pin (the plugin's names ARE trunk's objects). - Code launcher face ledger gains a machine lane; oncall face unchanged. - Full suite: see CI. @LivXue -- review requested: this touches `raven/ops` and the `RAVEN_CONNECTIONS` handoff described in a44664d. Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com> Generated with Claude Code (https://claude.com/claude-code) --------- Co-authored-by: xiaotian.luo <xiaotian.luo@thetahealth.ai> Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
…552) Two field runs on 2026-09-14 put a registered machine's address in the task statement, and the coding nodes typed "ssh -p <port> root@<ip> '... &'" from the local shell 58 times to start GPU work. The look-at-it channel is capped at 60 s and the on-call agent's job runner was not theirs to call, so the raw address was the path of least resistance. The cap, the process-group sweep that stops an orphan from holding a GPU, and the ledger all live on the other two paths, so none of that work was metered. The plain shell now refuses a command that names ssh and a host the registry knows, and the refusal names both the machine and the two paths that are meant for it. Everything else is untouched: an unknown host still works, scp and rsync still work (they move files and start nothing on the far side), and a registry that cannot be read recognises nothing, so ops being broken never takes the plain shell down with it. Type: fix Verification: - tests/test_shell_machine_channel.py: 32 passed, up from 28 on 0464aa1. The four new ones cover a typed ssh to a registered machine being refused with nothing run, an unknown host still running, a command that names the host without ssh still running, and a malformed registry refusing nothing. - Wider run over the shell surface (machine channel, approval, background, policy reasons, rpc): 316 passed, 0 failed. Risk: low, but it does close a path some prompt may rely on. The refusal is text, not an exception, and it names the replacement, so a model that reaches for the old path is told where to go. This does not add the missing channel itself: work between 60 s and a few minutes still has no first-class home outside the on-call agent, which is a separate design question. --------- Co-authored-by: xiaotian.luo <xiaotian.luo@thetahealth.ai> Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
… can add a machine (#690) ## Why In a research -> code -> oncall chain run on 2026-09-22, Raven-Code called `exec(machine="conn_cpu_32c")` and got "No connection is registered", on a home whose `connections.json` listed seven machines. It then read host/port/key out of the config and ran a raw `ssh root@<ip> -p <port> -i ~/.ssh/id_rsa`, putting the way in into the transcript. Two gaps: 1. **Read side.** `raven.ops.connections.store_path()` looked only beside the instance's own config. A sub-agent runs on a rendered config in its own state directory, so it looked in an empty directory. The host already hands every sub-agent `RAVEN_HOME`; the registry just was not looked for there. Only the on-call launcher worked around this (it sets `RAVEN_CONNECTIONS` itself). 2. **Write side.** `ops_connection_add` (#545) lived in the on-call plugin. In the usual chain the coding agent runs first, meets the empty registry first, and had no way to take the owner's answer. ## What changes - `store_path()`: env var -> `raven_home()/connections.json` if it exists -> beside the config if THAT exists -> the home (where a first add lands). An install that kept its own list beside its config keeps working. - New `raven/ops/connection_add.py`: `probe`, `ssh_defaults`, `parse_probe`, `write`, `write_ssh_alias`, and `ASK_OWNER` (the questions relayed to the owner). The tool calls these functions directly; `raven ops connection add` (the terminal command for a person) calls the same ones, so the logic exists once. - New `raven/agent/tools/connection_add.py`: `ops_connection_add` as a trunk tool, **opt-in** via `tools.connectionAdd` (on in raven-code and raven-oncall, off in the other three products, whose pinned tool faces are unchanged). - `raven.ops.transport.make_ssh_runner(identities_only=)` + `ISOLATE_IDENTITY`, moved from the plugin: a key the tool picked itself is probed with that key alone (see #545 review). - On-call plugin: `oncall_flow/connections.py` imports trunk's reader instead of keeping a hand-aligned copy (758 -> 212 lines); keeps only campaign lookups (`get`, `display_name`, `resolve_into`, `describe`). Its own tool file, factory and manifest row are removed. - Raven-Code guide (`TOOLS_CODE.md`) gains when to leave this computer: write here; verify where the software is; ask the owner for a machine only when installing the software here is wrong (GPU, memory) or heavy (compiled solver, toolchain, long download); never ssh from an address in the task statement. - `SHOWN` made public in `raven.ops.connections` (the plugin may not import an underscored trunk name; `_SHOWN` stays as an alias). - Docs: `docs-site/docs/oncall.md` / `oncall.zh.md` updated for the new lookup and the tool. ## Not in scope - The pre-dispatch registry gate removed in a44664d is NOT restored. Where a job runs is still decided inside the agent that runs it; this only makes that agent find the owner's registry and able to add to it. - #552 (exec refuses a typed ssh to a registered machine) is complementary: it only fires once the registry is visible, which this makes true for Raven-Code. ## Tests - `tests/test_ops_connection_add.py` (moved from the plugin suite, patches trunk modules), including the ssh `-G` measurement that only the candidate key is offered under isolation. - Five new cases in `tests/test_ops_connections.py` for the lookup order. - Plugin parity tests replaced by an identity pin (the plugin's names ARE trunk's objects). - Code launcher face ledger gains a machine lane; oncall face unchanged. - Full suite: see CI. @LivXue -- review requested: this touches `raven/ops` and the `RAVEN_CONNECTIONS` handoff described in a44664d. Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com> Generated with Claude Code (https://claude.com/claude-code) --------- Co-authored-by: xiaotian.luo <xiaotian.luo@thetahealth.ai> Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
…1023) ## Summary A machine registered by password under an ssh config alias, under a `HostKeyAlias`, or under a non-canonical address could never log in, and its registration blamed the password. Reported by 0xKT in the post-merge review of #1005 (point 1); point 2 is not in this PR. Cause: the askpass helper answers only `<user>@<host>'s password:` and `(<user>@<host>) Password:`, with the host as the registry row spells it. ssh names the host it resolved instead: the `HostName` the owner's ssh config gives an alias, the canonical form of a numeric address (`::1` for `0:0:0:0:0:0:0:1`), or the `HostKeyAlias` in place of either. Measured with OpenSSH 8.9p1 against a loopback test sshd: `alice@devbox` with `HostName 127.0.0.1` asks `alice@127.0.0.1's password: `, `alice@aliasbox` with `HostKeyAlias labbox` asks `alice@labbox's password: `, and `alice@0:0:0:0:0:0:0:1` asks `alice@::1's password: `. The helper refused each, ssh sent an empty password, and the probe appended "The password was refused, or this machine does not accept password logins". What changes: - `transport.ssh_env` now names, as the prompt the helper waits for, the host `ssh -G` resolves for the row: the `hostkeyalias` it prints, else its `hostname`. `ssh -G` is asked with the password connection's own options, before ssh starts and never from the helper (a helper that ran `ssh -G` itself once recursed into thousands of processes against a test ssh that called askpass), with askpass switched off for anything it starts, a 10 s timeout and tolerant decoding. When ssh cannot say (no ssh, a failure, a hang, no `hostname` line) the row's spelling stands, which is the behaviour before this change. - A password row whose host is the id of a `Host` alias Raven itself keeps in `~/.ssh/config` for a key machine is refused by name when that alias sends ssh to another address, or when its block is not the one Raven writes (a mark gone, a keyword Raven does not write such as `ProxyCommand` or `Include`, a missing or second `HostName`, a second name on the `Host` line), before ssh starts, and its password is not sent (review round, blocking thread). One whose `HostName` is its own id is followed. Without this, a key machine registered under the id `labbox`, which writes `Host labbox` / `HostName <its address>`, made the helper of a password machine registered at `labbox` wait for, and answer, the key machine's prompt; the model can register such a machine through `ops_connection_add`. The managed marks are read from the config ssh reads (the account's passwd home, which `$HOME` does not move) and from the one Raven writes (`~`); names compare ignoring case, the cautious direction, though OpenSSH is not case-blind here (measured 2026-10-20: `Host labbox` took `labbox` and not `LabBox`), and a block with one mark left still counts. `ALIAS_MARK` moves to `transport` so both modules share it. `machines.add` reports this refusal as `invalid`. The owner's own aliases resolve as before. - A declined prompt is no longer reported as a refused password anywhere. `connection_add.probe` says ssh asked something other than the account's password and that the password was not sent; `remote.explain` (node install, the tunnel) says the same instead of "check the password kept for it"; `machines.add` has a new reason, `prompt`, ahead of `login`, carried through `openrpc.json`, `models.py`, both generated clients, the register sheet and the en/zh catalog. A wrong password still reads as before. - When `ssh -G` fails, times out, cannot start or names no host, the fallback is no longer silent: `ssh_env` hands the reason to the helper (`RAVEN_MACHINE_PROMPT_UNRESOLVED`, empty when resolved), whose refusal then names it and the prompt it waited for, and the probe and `remote.explain` name the failed resolution instead of sending the owner to a key. - The resolution runs a subprocess (about 4 ms here), so every async caller that built a password runner on the event loop now builds it in a thread: exec's machine channel (`run_on_machine`), the openai sub-agent tunnel (`remote.tunnel`), and on-call's status, round-zero setup and backend in `ops_submit`, a declared action's runner, the campaigns listing, `ops_kill`, `ops_outputs`, the two case-dict tools, a reading's runner and the event watcher's backend. - `docs-site/docs/machines.md` (both languages), `CONTEXT.md` and the unreleased CHANGELOG entry say the prompt is the one for the address as ssh resolves it, so an alias from the owner's ssh config works, and that an alias Raven wrote for another machine is refused. Review round (gloryfromca, 0xKT): - Blocking thread (a managed alias redirects another machine's password): fixed as above, at the one place the helper's environment is built, so it also covers machines registered before the alias was written. Registration now refuses the pair up front too (second round, below). - 1 (on-call builds the runner on the loop): fixed at the ten on-call call sites above. Each has a test except the edit tool of the case-dict pair, which shares its helper with the case-changes tool's. - 2 (two of three classifiers still blame the password): fixed. On "the zh reader gets only the headline": the sheet shows ssh's English detail under the headline in both languages; what was wrong is that the zh (and en) headline told the reader to check the password. - Nit 1 (two ACP tests run the real `ssh -G`): fixed; they resolve through a stand-in, and the module now fails any test that runs the real `ssh -G` without `-F`. - Nit 2 (`ssh_defaults` is a second `ssh -G` reader that still decodes strictly): fixed in the second round, below. - Nit 3 (silent fallback): fixed as above. Second round (closing what the first left open): - One reader of `ssh -G`. `transport.resolve_with_ssh` is now the only one: the password machine's prompt and a key machine's registration defaults (`connection_add.ssh_defaults`: port, user, identity files in ssh's order) both resolve through it, each with its own command line. The key reader used to raise on a config byte that is not UTF-8 and on an ssh it could not start, and handed its `ssh -G` the caller's terminal and askpass; now it decodes tolerantly, reads such failures as "ssh cannot say" (empty defaults, as a missing ssh already was), and starts nothing that finds an askpass or a terminal. Where ssh answered before, a key machine's defaults are unchanged; the existing defaults tests pass as they were. - Registration refuses the colliding pair in either order, with what to do: a key machine whose id is a registered password machine's address (compared ignoring case, the cautious direction; OpenSSH itself took `Host labbox` for `labbox` only), and a password machine at a registered key machine's id (its alias may not be written yet) or at an alias Raven keeps with no machine behind it, a lone mark included (`connection_add.alias_collision`). `machines.add` (as `invalid`), the CLI and `ops_connection_add` ask before anything is kept, resolved or reached; `connection_add.write` asks again under the registry lock, which is what stops `raven ops connection add --skip-probe` (it wrote such a pair, since nothing connected). The refusal on connecting stays, for pairs registered before this check or written by hand. - Host rewriting. `ssh -G` makes none of the DNS lookups `CanonicalizeHostname` makes, while a connection that canonicalizes re-reads the config under the canonical name, where a `Host <id>` block Raven wrote can apply (OpenSSH 8.9 reports applying it under `-v`); with a `HostKeyAlias` the prompt would still match the helper. The password connection options now carry `CanonicalizeHostname=no`, which outranks the owner's config, so the connection reaches the host `ssh -G` named. The one later pass left is `Match final`: measured, it applies a Raven block re-read under the owner's `HostName`, but ssh keeps the first `HostName` it read, the owner's, and a test pins that. A machine that relied on canonicalization already failed at the prompt (the helper waited for the uncanonicalized name); it is registered at its full name. - Test isolation. Every password connection now asks `ssh -G` first, and seven tests that stubbed the connection still reached the real one through the resolution (two node-link tests, four registration cases, one aborted CLI add). The ACP module's guard is now a `no_real_ssh_g` fixture applied to the nine modules whose code reaches the reader, and those tests answer `-G` with a stand-in. A lock test pins that `transport.ssh_env` is the one place that builds the environment handing ssh a kept password, which every path (registration probes, CLI, `machines.add`, exec's machine channel, the ACP tunnel and node link, on-call runners and rsync) goes through. Windows test correction: the structural askpass-owner test now renders its discovered repository-relative paths with POSIX separators. Its scan, matching predicate, expected owner and assertion are unchanged, so native Windows no longer fails solely because pathlib displays backslashes. Third review round (0xKT; each point answered on the PR): - A key machine at its own name (`gpu1` at `gpu1`) beside a password machine at `gpu1` was refused at registration and on every connection, though ssh still logs in at `gpu1`: fixed. `transport.managed_alias_hostnames` reads where each managed block sends ssh, and `ssh_env` and `alias_collision` refuse only when a block in either home sends the name elsewhere or is not the one Raven writes; registration also asks the registry row the next add rewrites the block from, in both orders. The refusals that remain point at the IP address or full domain name. - The on-call event watcher built a campaign's backend on the loop: it now builds it with `asyncio.to_thread`, like the tools, and a new test counts the loop's heartbeats while a slow factory builds it. - The two resolution-failure tests capped every `ssh -G` stand-in at 0.3 s, which macOS's first exec of a freshly written script (320-530 ms) exceeded: only the stand-in that sleeps keeps that cap now, and a new case that starts 0.45 s late must still be named by its exit status. After main merged #1018 (0xKT, blocking; confirmed by gloryfromca): - `no_real_ssh_g` sat on `subprocess.run` and knew a stand-in only by the ssh first on `PATH`, so on the merged tree it stopped the two `ssh_defaults` tests #1018 rewrote before their Python stand-in, a swapped `subprocess.Popen`, took over. The guard is now a `subprocess.Popen` subclass that judges the argv it is finally handed: a wrapper's rewritten command line passes, and an `ssh -G` that names the computer's own ssh fails, named bare or by path, even with a stand-in first on `PATH`. A swapped `Popen` alone is not taken as a stand-in. Two more tests pin what the guard judges: an ssh started without -G runs, and the PATH it searches is the one handed to Popen. Found while re-checking every registration path against the narrower alias check: - `ops_connection_add` asked `alias_collision` before the row carried its address, so the model's path still refused key machine `gpu1` at `gpu1` beside a password machine at `gpu1`, naming "this machine's address ('')", while the CLI, `machines.add` and the registry write accepted the pair. The address now goes on an ssh row before the check; an ssh row with no address is still refused before anything is reached. Left as it was, and why: - The resolution reuses the connection's options (`-p`, the password-only options). No test notices if it read others, since none of them changes `HostName` or `HostKeyAlias`; the two mutants that swap them survive, and this is why. - An owner's own `Host` block with the same name as a managed id, placed earlier in the file, is refused too when Raven's block for that name sends ssh elsewhere: ssh would follow the owner's block, but a name Raven also routes to another address is ambiguous, and refusing sends no password anywhere. A key and a password registration of one physical machine under a shared name register as a pair when the key machine's alias names its own id as the address (key machine `gpu1` at `gpu1`, password machine `gpu1-shared` at `gpu1`); a pair whose alias would send ssh elsewhere is still refused, and the refusal says to register the password machine at its IP address or full domain name. - `test_the_table_agrees_with_the_ssh_on_this_computer` in `test_shell_machine_channel.py` (from #552, not touched here) asks the computer's own `ssh -G` how ssh parses its arguments, so it reads the runner's ssh config; it is not under the new guard, and the runs below deselect it. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification Current head, rebased onto main with #1003: `git range-diff` maps the 18 earlier commits as `=`, in order, and two commits sit on top: the guard's two tests and the registration fix above. - The registration fix: its tool test failed at the commit before it, refused with "this machine's address ('')", and passes after. Single-construct mutants: 5 of 5 killed (the old check order, the address set after the check, the address left off the row, the alias check removed, the empty-address refusal removed). `pytest -n 2 -p no:randomly` over `test_ops_connection_add.py`, `test_cli_ops_connection_commands.py`, `test_shell_machine_channel.py`, `test_agents_code_launcher.py` and `test_agents_oncall_launcher.py`: 359 passed, 1 skipped; with the tool file as it was before the fix, 358 passed, 1 skipped, and only the new test failed. - The guard: both `ssh_defaults` tests and the three guard tests (named bare, by path, a `Popen` wrapper that leaves ssh alone): 5 passed. Mutants of the two conditions the guard's two new tests pin (the `-G` check dropped, this process's `PATH` read instead of the one handed to `Popen`) each fail one of them. gloryfromca, on this base before the two commits on top: every module under the guard, 492 passed with 4 existing tcsh/csh skips. - Full suite, `pytest -q -n 4 -p no:randomly` on py3.12 with child processes importing the tree under test: 13 failed, 34431 passed, 182 skipped, 2 xfailed. Those 13 tests, rerun alone on main with #1003 and on this tree with the tool file as it was before the registration fix, fail the same way; none is in a file this PR changes. - Native Windows (Python 3.13.12) on the head before this rebase, whose 18 commits are patch-identical: 554 tests passed and 59 POSIX-only cases skipped, including both `ssh_defaults` stand-in cases, the guard tests, the registration collisions, the on-call thread boundaries and the RPC schema check; 5 probes with the real Windows OpenSSH against a scratch `-F` config only; 165 machine-page tests, the web and TUI type checks, both RPC generator checks and the TUI i18n check. - `ruff check` and `ruff format --check` over the changed files, the commit message lint, `scripts/check_source_language.py origin/main...HEAD`, `scripts/check_large_files.py origin/main...HEAD` and `git diff --check`: pass. Earlier rebase onto main with #992 (the branch had ten commits then): - `git range-diff` maps all ten commits as `=`, and the branch's own diff is identical to the one before the rebase apart from hunk offsets. The rebase had no conflict and changed no file outside this PR. The second-round, first-round and native Windows results further down were measured before this rebase and keep their original scope. - The `unit 1/4` failure on the previous head (`test_a_cancel_while_this_computer_goes_on_ends_the_command_and_keeps_no_record`, `assert remote_runs.pending() == [] and not remote_runs.was_left("t1")`) was a mark one test left for the next, not this PR's code. Both cases of `test_node_stderr_replay.py::test_a_left_cli_run_s_reason_on_stderr_reaches_its_pick_up`, which run in the same shard, end with dispatch `t1` marked as left and never read. A later test on the same worker that cancels `t1` while this computer goes on then reads that stale mark. Reproduced by running the two in that order on one worker: before the rebase the cancel test fails with the CI assertion verbatim. On plain main and on the rebased branch the same three tests pass, because #992's `_this_process_goes_on` fixture now clears the marks after every test. The cancel test passes on its own on all three trees. No test or product code was changed for it. - The strict idle-ceiling failure in `unit 4/4` on the head before last (setup idle of 3.45 s and 3.04 s in two unmarked tests whose own work takes about 0.4 s and 0.01 s) is read as a runner stall. No `slow` or `production_timing` marker was added, since it would record a reason that is not true. It did not recur in the 4/4 shard of the previous head. - On the rebased tree, serially (`-n 0`), with py3.12 and child processes that import the tree under test: `pytest -n 0 --idle-ceiling-strict` over the 17 test modules this PR touches plus `test_rpc_bootstrap.py`, `test_subagent_acp_remote.py`, `test_subagent_cli_remote.py`, `test_subagent_manager.py`, `test_resume_remote.py`, `test_remote_resume.py`, `test_node_stderr_replay.py`, `test_acp_client_resume.py` and `test_remote_runs.py`: 1113 passed, 7 skipped, nothing deselected, and no test started with a left mark from an earlier one. `tests/test_subagent_cli_remote.py` alone: 60 passed. `test_rpc_schema_match.py`, `test_rpc_registration.py`, `test_rpc_instances_declare_their_errors.py` and `test_channels_display_names.py`: 525 passed. - Gates on the rebased branch: `ruff check` and `ruff format --check` over the repository's lint targets, `ty check` over the source targets, `lint-imports` (10 kept, 0 broken), `scripts/check_commit_messages.py origin/main..HEAD`, `scripts/check_large_files.py origin/main..HEAD`, `scripts/check_source_language.py origin/main...HEAD`, `git diff --check`, and the generator checks of ui-web (`gen:check`) and ui-tui (`lint:rpc`, `lint:i18n`), since main and this branch both edit `openrpc.json` and the generated clients: pass. The ui-web vitest suite was not re-run locally; CI runs it on the new head. Second round: - New tests, each watched failing at the commit before it and passing after: 4 for the one reader (an undecodable byte raised `UnicodeDecodeError`, an ssh that cannot start raised `PermissionError`, the key machine's `ssh -G` read the terminal's input and inherited askpass, and the two machines used two readers), 2 for canonicalization (real OpenSSH reported `canonicalizehostname true` / `always` for the password options before), 8 for registration (the writer in both orders, a lone mark with no machine, the model's tool, `machines.add` and the CLI with `--skip-probe` in both orders; the CLI wrote the pair before). Lock tests that pass on both sides: owners' rows that do not collide still register; `Match final` keeps the owner's `HostName`; one function builds the askpass environment (a stray module naming its variable fails it). - Single-construct mutants for this round: 14 of 14 killed (the reader's tolerant decoding, askpass, stdin and start-failure handling, a separate defaults reader, canonicalization back on, the collision check removed from the writer, `machines.add`, the CLI and the tool in turn, either direction, the case fold, the registry lookup and the managed-alias lookup). - `pytest -n 0 tests/test_ops_transport.py tests/test_ops_connection_add.py tests/test_rpc_machines.py tests/test_cli_ops_connection_commands.py tests/test_acp_remote.py tests/test_node_client.py tests/test_shell_machine_channel.py tests/test_agents_oncall_flow_*.py tests/test_rpc_schema_match.py -k "not test_the_table_agrees_with_the_ssh_on_this_computer"`: 1846 passed, 9 skipped, 14 deselected. On the merged tree: 1823 passed, 9 skipped, 23 failed; the 23 are 22 `test_node_client` IDs plus `test_install_node_closes_the_live_node_it_asked`, and the same IDs fail on a plain `origin/main` tree run the same way (their node is started with `python -I` and imports another tree than the one under test), so none is introduced. - `ruff check` and `ruff format --check` over the changed Python files, `ty check` over the changed source files, `lint-imports` (10 kept, 0 broken), `scripts/check_source_language.py origin/main...HEAD`, `scripts/check_large_files.py origin/main...HEAD`, `scripts/check_commit_messages.py origin/main..HEAD`, `commitlint`, `git diff --check`: pass. Nothing in the web client, TUI or RPC contract changed this round. First round, still standing for the code it covered: - 16 tests for the managed alias and the declined-prompt wording, 1 for the register sheet, 8 for on-call, the two ACP tests under the guard; 18 of 19 mutants killed (the survivor was the ACP guard itself, with both tests on stand-ins). - `vitest run src/features/extAgents/ExtAgentsPage.test.tsx`: 160 passed (165 on the merged tree); the ui-web contract gates (24 passed), `tsc --noEmit`, `eslint` and the three generator checks: clean. - End-to-end runs against an asyncssh test sshd on loopback with the real OpenSSH client (alias, `HostKeyAlias`, `0:0:0:0:0:0:0:1` and keyboard-interactive alias rows register and log in; an sshd that asks only "Verification code:" gets an empty answer). They were not re-run for the second round, and the full suite was not run for either round. Those historical runs did not use native Windows; the native follow-up below is separate. Native Windows follow-up (Windows 11 build 26300, Python 3.13.12, UTF-8 mode): - Reproduced the reported structural test failure before editing: the discovered path was `raven\ops\transport.py`, while the expected repository spelling used `/`. The one-line `.as_posix()` correction passed the focused test with the strict idle ceiling enabled. - After rebasing and a frozen workspace/dev sync, ran: ```powershell uv run --frozen --no-sync pytest tests/test_ops_transport.py -n 0 -q --tb=short --idle-ceiling-strict --basetemp "$evidenceDir/rebased" --junitxml "$evidenceDir/rebased.xml" ``` Result: 10 passed, 45 skipped, exit 0. The skips are 43 POSIX askpass/helper cases and two POSIX file-mode cases. `$evidenceDir` is an external report directory. Imports were verified to resolve to the dedicated worktree; profile and OAuth paths were isolated. - An earlier native full-module run passed every assertion but failed the strict idle gate in `test_the_runner_sends_what_it_always_sent` (3.43 s idle). The final run above retained the strict gate and passed; no timing marker, ceiling or test assertion was weakened. - Full-PR `uv run --frozen --no-sync pre-commit run --from-ref origin/main --to-ref HEAD` passed using Git Bash. Its frontend hooks return without running their tools when the corresponding `node_modules` is absent; frontend tests/generators were not rerun locally for this test-only correction. - `uv run --frozen --no-sync ty check` over all changed production Python files passed. `uv run --frozen --no-sync lint-imports` kept all 10 contracts. The complete-range source-language, large-file and commit-message gates, lockfile-installed commitlint, and `git diff --check origin/main...HEAD` passed. On Windows the commit-message wrapper could not spawn `npx`, so its two checks were run directly: the installed `commitlint.cmd` and `scripts.commit_lint.check_commit_message`. - The previous remote head's Linux unit shard had 8666 passed, 23 skipped and 1 xfailed, but failed the strict idle ceiling for `test_the_window_starts_when_the_run_is_released` and `test_an_install_the_login_path_misses_is_not_reported_as_absent` (3.45 s and 3.04 s). Those tests and the ceiling were not changed by this correction. New-head CI is assessed separately. - No live SSH server, Linux suite, browser run, or full-repository native suite was run for this correction. Earlier broad results are retained above with their original scope. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk - A machine registered under an alias, a `HostKeyAlias` or a non-canonical address now logs in; nothing else changes which prompt the helper answers. The password still goes only to the host ssh connects to for the row, and never through a `Host` block Raven wrote that sends ssh to another address or is not the block Raven writes. - A password machine registered at a name that is also a key machine's id is now refused by name, with the password unsent, when that machine's alias sends ssh to another address; before this PR its login went to the key machine's address and failed there. Registering such a pair is refused in either order on every registration path, and a pair already registered is refused on connecting. A key machine whose alias names its own address (`gpu1` at `gpu1`) leaves a password machine at `gpu1` free to register and log in. - A key machine's registration defaults come from the same reader: unchanged where ssh answered, empty instead of an exception where it could not (an undecodable config byte, an ssh that cannot start). - Password connections no longer canonicalize the host; a password machine that relied on `CanonicalDomains` to complete a short name is registered at its full name. - Every password connection starts one `ssh -G` first (about 4 ms here; 10 s at most, after which the row's spelling stands), off the event loop everywhere. It reads the owner's ssh config, so a `Match exec` there runs once more. - `machines.add` can answer the new reason `prompt`; the web client and the TUI types are regenerated with it. The askpass helper's bytes change, and it is rewritten on the next password connection, as before. - Rollback: revert the commits; nothing is stored or migrated. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues Follows the post-merge review of #1005.
Two field runs on 2026-09-14 put a registered machine's address in the task
statement, and the coding nodes typed "ssh -p root@ '... &'" from
the local shell 58 times to start GPU work. The look-at-it channel is capped
at 60 s and the on-call agent's job runner was not theirs to call, so the raw
address was the path of least resistance. The cap, the process-group sweep
that stops an orphan from holding a GPU, and the ledger all live on the other
two paths, so none of that work was metered.
The plain shell now refuses a command that names ssh and a host the registry
knows, and the refusal names both the machine and the two paths that are
meant for it. Everything else is untouched: an unknown host still works, scp
and rsync still work (they move files and start nothing on the far side), and
a registry that cannot be read recognises nothing, so ops being broken never
takes the plain shell down with it.
Type: fix
Verification:
The four new ones cover a typed ssh to a registered machine being refused
with nothing run, an unknown host still running, a command that names the
host without ssh still running, and a malformed registry refusing nothing.
policy reasons, rpc): 316 passed, 0 failed.
Risk: low, but it does close a path some prompt may rely on. The refusal is
text, not an exception, and it names the replacement, so a model that reaches
for the old path is told where to go. This does not add the missing channel
itself: work between 60 s and a few minutes still has no first-class home
outside the on-call agent, which is a separate design question.