diff --git a/CHANGELOG.md b/CHANGELOG.md index ee8cd0d43..903cec3f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -218,6 +218,17 @@ All notable changes to Raven are documented here. Measured with four sessions in one process: 18 of 183 approvals lost, each inside another session's `ssh`. The command's stdin now reads EOF at once, as the background executor's already did. + +- `exec` on this computer refuses a typed `ssh` to a machine the connection + registry knows, and names the two paths that exist for it: `machine=` + for a look (capped at 60 s, nothing left running) and the on-call agent's + `ops_submit` for anything longer. Two field runs on 2026-09-14 had put the + machine's address in the task statement, and the coding nodes started GPU + work over raw ssh from the local shell 58 times, past the cap, the sweep + and the ledger. `scp` and `rsync` to the machine are untouched; a registry + that cannot be read refuses nothing. Options are read the way ssh reads + them -- a bundled group like `-vp 58717`, and options written after the + host -- so a spelling ssh honours does not read as port 22. - The web file viewer opens a sub-agent's report again. `/file` anchored the state-directory fence on the session's working directory whenever the page named a session, so the fence exempted `~/.raven/tmp/` and refused diff --git a/raven/agent/tools/machine_exec.py b/raven/agent/tools/machine_exec.py index 03157c151..a6a935567 100644 --- a/raven/agent/tools/machine_exec.py +++ b/raven/agent/tools/machine_exec.py @@ -47,6 +47,7 @@ import asyncio import re import shlex +from pathlib import PurePosixPath from typing import Any # Long enough to tail a large log or hash a directory; far short of any solver @@ -96,6 +97,222 @@ def machines_registered() -> bool: return False +# What punctuation_chars hands back as its own token, split by what the token +# does to the command. A separator ends it, so the words after one belong to the +# next command and not to this ssh. +_COMMAND_SEPARATORS = frozenset({";", "&", "&&", "|", "||", "|&", "(", ")"}) + +# A redirection does not end the command: `ssh 2>/dev/null -p 58717 root@host` +# is a single ssh. Reading one as a terminator -- or, worse, reading `&>` as a +# word and taking it for the destination -- lost the registered host that came +# after it (reviewed 2026-09-20). The operator and the file it names are +# stepped over instead, and this ssh's own words keep being read. +_REDIRECTIONS = frozenset({"<", ">", ">>", "<<", "<<<", "<&", ">&", "&>", "&>>", ">|", "<>"}) + +# An -o option's name and its value are separated by an equals sign or by +# whitespace; OpenSSH honours both spellings. +_OPTION_SPLIT = re.compile(r"\s*=\s*|\s+") + +# ssh(1)'s own getopt string, copied from the OpenSSH_9.9p2 binary rather than +# listed from memory: a letter followed by ':' takes a value. The hand-kept set +# this replaces had lost `B` (bind interface), so `ssh -B lo -p 58717 host` +# read `lo` as the destination (reviewed 2026-09-21). OpenSSH_8.9 differs in +# one letter -- `P` takes no value there -- and the current release is read. +_SSH_OPTSTRING = "1246ab:c:e:fgi:kl:m:no:p:qstvxAB:CD:E:F:GI:J:KL:MNO:P:Q:R:S:TVw:W:XYy" +_SSH_VALUE_FLAGS = frozenset(ch for i, ch in enumerate(_SSH_OPTSTRING) if _SSH_OPTSTRING[i + 1 : i + 2] == ":") + + +def _read_option_group(word: str, tokens: list[str], index: int) -> tuple[int, str | None]: + """Read one ``-xyz`` option group the way getopt does. ``(index, port)``. + + Letters are read in turn until one takes a value; the rest of the group is + that value, or the next word when nothing is left (``-p58717``, ``-p + 58717``, ``-vp 58717``, ``-vvvp58717`` all name port 58717). Reading only a + two-character ``-p`` skipped ``-vp`` as a group with no argument and took + its port for the destination (reviewed 2026-09-21). ``port`` is the value + the group gives the port -- from ``-p`` or from an ``-o`` port option -- or + None; ``index`` is past whatever the group consumed. + """ + letters = word[1:] + for offset, letter in enumerate(letters): + if letter not in _SSH_VALUE_FLAGS: + continue + value = letters[offset + 1 :] + if not value and index < len(tokens): + value = tokens[index] + index += 1 + if letter == "p": + return index, value + if letter == "o": + # `ssh -G` prints `port 58717` for -o Port=58717 and for + # -o "Port 58717" alike. Splitting only on the equals sign dropped + # the spaced spelling's value and left the port at 22, so a machine + # registered on another port went unrecognised (reviewed 2026-09-20). + pair = _OPTION_SPLIT.split(value.strip(), maxsplit=1) + return index, (pair[1].strip() if len(pair) == 2 and pair[0].lower() == "port" else None) + return index, None + return index, None + + +def _ssh_destinations(command: str) -> list[tuple[str, int]]: + """Every host and port an ``ssh`` word in a shell command would connect to. + + Empty when no token runs the ssh client, or when the line cannot be + tokenised at all. Every ssh in the line is collected, not the first: a + compound line reaches each of its commands, so stopping at one destination + lets ``ssh ; ssh `` through on the strength of + the half that was allowed. + + Tokenised with ``punctuation_chars`` so that unspaced operators separate + words the way a shell reads them -- ``true&&ssh`` is two commands, and + plain splitting hands back one token that is neither. + + The executable may be written ``ssh``, ``/usr/bin/ssh`` or ``\\ssh`` (a + backslash suppresses alias lookup and still runs the client), and all three + reach the far side. + + The port is read from ``-p`` and from an ``-o`` port option in either of the + spellings OpenSSH honours -- ``-o Port=58717`` and ``-o "Port 58717"`` -- + because the registry holds several machines at one address on different + ports: the address alone picks whichever row is listed first and names the + wrong machine. Repeats keep the first value, as ssh does. + + Options are read the way ssh reads them: a ``-xyz`` group letter by letter + (``-vp 58717``), and on past the destination until the first word that is + not an option (``ssh root@h -p 58717 true`` is port 58717, ``ssh root@h + true -p 58717`` is 22) or a ``--``. + """ + try: + lexer = shlex.shlex(command, posix=True, punctuation_chars=True) + lexer.whitespace_split = True + tokens = list(lexer) + except ValueError: + # An unbalanced quote is not a shell line this can read; the shell will + # reject it too, so nothing reaches a machine either way. + return [] + + found: list[tuple[str, int]] = [] + index = 0 + while index < len(tokens): + if PurePosixPath(tokens[index].lstrip("\\")).name != "ssh": + index += 1 + continue + index += 1 + port = 22 + port_set = False + destination: str | None = None + options_open = True + options_ended = False + while index < len(tokens): + word = tokens[index] + if word in _COMMAND_SEPARATORS or PurePosixPath(word.lstrip("\\")).name == "ssh": + # The command ended, or the next one began; either way this + # ssh's arguments are over and the token is left for the outer + # loop to read. + break + following = tokens[index + 1] if index + 1 < len(tokens) else None + if word in _REDIRECTIONS: + # The operator and the file it names; the file is skipped only + # when there is one, so a redirection left dangling before a + # separator does not swallow the separator. + index += 1 + if following is not None and following not in _COMMAND_SEPARATORS and following not in _REDIRECTIONS: + index += 1 + continue + if word.isdigit() and following in _REDIRECTIONS: + # `2>&1` arrives as the three tokens 2, >& and 1, so a bare file + # descriptor can stand in front of the operator. Without this + # the digit was taken for the destination and the real host, + # further along the line, was never read. The tokens do not say + # whether a space separated the digit from the operator, so a + # destination that is itself a bare integer is read as a + # descriptor here -- a shape no registry row has, since an + # address carries dots or letters. + index += 1 + continue + index += 1 + if not options_open: + # The remote command; nothing in it is this ssh's to read. + continue + if word == "--" and not options_ended: + # getopt's end of options. Before the destination it makes the + # next word the host whatever it looks like; after it, the rest + # is the remote command: `ssh -G root@h -- -p 58717` prints 22. + options_ended = True + if destination is not None: + options_open = False + continue + if word.startswith("-") and len(word) > 1 and not options_ended: + index, value = _read_option_group(word, tokens, index) + # First obtained value wins, which is ssh's own rule for every + # option: `ssh -G -p 2222 -o Port=58717 host` prints 2222, and + # reversing the two prints 58717. Overwriting instead read + # `-p 58717 -p 22` as port 22 and let a command that really + # reaches the registered machine past the guard (reviewed + # 2026-09-21). + if value is not None and value.isdigit() and not port_set: + port = int(value) + port_set = True + continue + if destination is None: + destination = word.rsplit("@", 1)[-1].strip("[]").lower() + # OpenSSH re-enters its option loop once it has the host, so + # `ssh root@h -p 58717 true` connects on 58717; stopping at the + # destination read it as 22 and let the line past the guard + # (reviewed 2026-09-21). A `--` already seen means no re-entry. + options_open = not options_ended + continue + # The first non-option word after the host starts the remote + # command, and ssh stops reading options there: `ssh -G root@h + # true -p 58717` prints 22. + options_open = False + if destination: + found.append((destination, port)) + return found + + +def raw_ssh_target(command: str) -> dict[str, Any] | None: + """The registered machine a plain-shell command reaches over its own ssh. + + ``None`` when the command runs no ssh client, or names a destination the + registry does not know, or the registry cannot be read: the plain shell + keeps working for everything that is not the bypass this looks for. + + Host and port are both matched, and the host as a whole word rather than a + substring -- ``203.0.113.70`` is not ``203.0.113.7``, and a machine the + registry does not hold must still be reachable from here. + + The bypass is measured, not hypothetical. Two field runs on 2026-09-14 + put the 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 here is capped at 60 s and on-call's job runner + was not theirs to call, so the address was the path of least resistance, + and the ledger never saw the runs. Only ssh is matched -- ``scp`` and + ``rsync`` move files and start nothing on the far side. + """ + destinations = _ssh_destinations(command) + if not destinations: + return None + try: + from raven.ops.connections import load + + rows = load() + except Exception: # noqa: BLE001 -- a malformed registry must leave the plain shell working + return None + for host, port in destinations: + for row in rows: + row_host = str(row.get("host") or "").strip().lower() + if not row_host or row_host != host: + continue + try: + row_port = int(row.get("port") or 22) + except (TypeError, ValueError): + row_port = 22 + if row_port == port: + return row + return None + + def _runner_for_connection(conn_id: str, *, cap_seconds: float | None = None): """A command runner for a machine named by id, plus its row. diff --git a/raven/agent/tools/shell.py b/raven/agent/tools/shell.py index 89ccf5d7c..095b2b4d1 100644 --- a/raven/agent/tools/shell.py +++ b/raven/agent/tools/shell.py @@ -223,6 +223,24 @@ async def execute( from raven.agent.tools.machine_exec import run_on_machine return await run_on_machine(command, connection=machine, cwd=working_dir) + # Same import discipline as above: the registry is read only to + # recognise a registered address, and a registry that cannot be read + # recognises nothing. + from raven.agent.tools.machine_exec import raw_ssh_target + + if (row := raw_ssh_target(command)) is not None: + # Typed ssh to a registered machine is the registry bypassed: the + # cap, the process-group sweep and the ledger all live on the + # other two paths. Refused by name so the model learns the path, + # not just that this one closed. + conn_id = str(row.get("id") or "").strip() + return ( + f"Error: this command reaches {row.get('display_name') or conn_id} over raw ssh from " + f"this computer, and that machine is registered as {conn_id!r}. Look at it with " + f"exec(machine={conn_id!r}) (capped at 60s, nothing left running), and hand anything " + "longer, or anything that must keep running, to the on-call agent's ops_submit " + "(budget, dedup, ledger). scp and rsync to it are still fine here. Nothing was run." + ) cwd = self._cwd_for(working_dir) diff --git a/tests/test_shell_machine_channel.py b/tests/test_shell_machine_channel.py index 7b0714e2d..868d52a92 100644 --- a/tests/test_shell_machine_channel.py +++ b/tests/test_shell_machine_channel.py @@ -355,3 +355,415 @@ def fake_run(argv, **kwargs): assert calls == [["C:/Windows/System32/taskkill.exe", "/T", "/F", "/PID", "4242"]] assert killed == ["kill"], "and the shell itself is still killed when the tree kill left it" + + +# ---- the plain shell refuses typed ssh to a machine the registry knows ---- + + +@pytest.mark.asyncio +async def test_typed_ssh_to_a_registered_machine_is_refused(registry, tmp_path): + tool = ExecTool(working_dir=str(tmp_path)) + marker = tmp_path / "ran" + + out = await tool.execute(command=f"ssh -p 58717 root@203.0.113.7 'nohup python train.py &'; touch {marker}") + + assert "Error:" in out + assert "conn_gpu" in out and "ops_submit" in out + assert not marker.exists() + + +@pytest.mark.asyncio +async def test_ssh_to_an_unregistered_host_still_runs(registry, tmp_path): + tool = ExecTool(working_dir=str(tmp_path)) + + out = await tool.execute(command="echo ssh 198.51.100.9 would be typed here") + + assert "would be typed here" in out + + +@pytest.mark.asyncio +async def test_naming_the_host_without_ssh_is_not_a_bypass(registry, tmp_path): + tool = ExecTool(working_dir=str(tmp_path)) + + out = await tool.execute(command="echo the box is 203.0.113.7") + + assert "203.0.113.7" in out and "Error:" not in out + + +@pytest.mark.asyncio +async def test_a_broken_registry_refuses_nothing_in_the_plain_shell(monkeypatch, tmp_path): + def boom(): + raise ValueError("mangled json") + + monkeypatch.setattr("raven.ops.connections.load", boom) + tool = ExecTool(working_dir=str(tmp_path)) + + out = await tool.execute(command="echo ssh 203.0.113.7 with a broken registry") + + assert "with a broken registry" in out + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "written", + ["ssh", "/usr/bin/ssh", "\\ssh"], + ids=["bare", "path-qualified", "backslash-escaped"], +) +async def test_the_ssh_client_is_recognised_however_it_is_written(registry, tmp_path, written): + """All three spellings run the client, so all three reach the far side. + + A leading backslash suppresses alias lookup and an absolute path skips + PATH; neither changes what executes. Matching only the bare word left both + on the plain path, past the cap, the process-group sweep and the ledger. + """ + tool = ExecTool(working_dir=str(tmp_path)) + marker = tmp_path / f"ran-{written.strip('\\/').replace('usr', '')}" + + out = await tool.execute(command=f"{written} -o ConnectTimeout=1 -p 58717 root@203.0.113.7 true; touch {marker}") + + assert "Error:" in out and "conn_gpu" in out + assert not marker.exists() + + +@pytest.mark.asyncio +async def test_a_different_port_at_the_same_address_is_a_different_machine(monkeypatch, tmp_path): + """The registry holds several machines at one address on different ports. + + Matching the address alone returns whichever row is listed first, so the + refusal names a machine the command was not reaching and the recovery it + proposes would run the work somewhere else. + """ + other = dict(ROW) | {"id": "conn_gpu_b", "display_name": "GPU box B", "port": 64101} + monkeypatch.setattr("raven.ops.connections.load", lambda: [dict(ROW), other]) + tool = ExecTool(working_dir=str(tmp_path)) + + out = await tool.execute(command="ssh -o ConnectTimeout=1 -p 64101 root@203.0.113.7 true") + + assert "conn_gpu_b" in out, "the refusal names the machine on the port that was typed" + assert "conn_gpu'" not in out + + +@pytest.mark.asyncio +async def test_a_longer_address_that_starts_with_a_registered_one_still_runs(registry, tmp_path): + """203.0.113.70 is not 203.0.113.7. + + A substring test made every address with a registered one as its prefix + unreachable from here, which breaks the guarantee that ssh to a machine + the registry does not hold keeps working. + """ + tool = ExecTool(working_dir=str(tmp_path)) + + out = await tool.execute(command="echo ssh root@203.0.113.70 would be typed here") + + assert "would be typed here" in out and "Error:" not in out + + +@pytest.mark.asyncio +async def test_a_line_that_cannot_be_tokenised_is_not_judged(registry, tmp_path): + """An unbalanced quote is not a shell line this can read. + + The shell rejects it too, so nothing reaches a machine either way, and + guessing at a half-parsed line would refuse commands that never ran. + """ + tool = ExecTool(working_dir=str(tmp_path)) + + out = await tool.execute(command="ssh root@203.0.113.7 'unterminated") + + assert "conn_gpu" not in out, "no refusal is issued for a line nobody can parse" + + +@pytest.mark.asyncio +async def test_a_flag_that_takes_no_value_does_not_hide_the_destination(registry, tmp_path): + """-v takes no argument, so the destination is the next word, not the one after it.""" + tool = ExecTool(working_dir=str(tmp_path)) + marker = tmp_path / "ran" + + out = await tool.execute(command=f"ssh -v -o ConnectTimeout=1 -p 58717 root@203.0.113.7 true; touch {marker}") + + assert "Error:" in out and "conn_gpu" in out + assert not marker.exists() + + +@pytest.mark.asyncio +async def test_ssh_with_no_destination_at_all_is_left_alone(registry, tmp_path): + """Flags and nothing else reaches no machine; ssh prints its usage and stops.""" + tool = ExecTool(working_dir=str(tmp_path)) + + out = await tool.execute(command="ssh -V") + + assert "conn_gpu" not in out + + +@pytest.mark.asyncio +async def test_a_row_whose_port_is_not_a_number_is_read_as_the_default(monkeypatch, tmp_path): + """A hand-edited registry must not make the guard give up on the row. + + 22 is what ssh itself would use, so a row with no usable port is compared + against the port a command that names none would reach. + """ + monkeypatch.setattr("raven.ops.connections.load", lambda: [dict(ROW) | {"port": "not-a-number"}]) + tool = ExecTool(working_dir=str(tmp_path)) + + out = await tool.execute(command="ssh -o ConnectTimeout=1 root@203.0.113.7 true") + + assert "Error:" in out and "conn_gpu" in out + + +@pytest.mark.asyncio +async def test_an_unspaced_operator_still_separates_the_commands(registry, tmp_path): + """A shell reads "true&&ssh" as two commands; splitting on whitespace reads one word. + + That word is neither "true" nor "ssh", so the guard saw no ssh at all and + the registered machine was reached from the plain shell. + """ + tool = ExecTool(working_dir=str(tmp_path)) + marker = tmp_path / "ran" + + out = await tool.execute(command=f"true&&ssh -o ConnectTimeout=1 -p 58717 root@203.0.113.7 true; touch {marker}") + + assert "Error:" in out and "conn_gpu" in out + assert not marker.exists() + + +@pytest.mark.asyncio +async def test_every_ssh_in_the_line_is_read_not_just_the_first(registry, tmp_path): + """A compound line reaches each of its commands, so each destination counts. + + Stopping at the first let a line through on the strength of the half that + was allowed, and the registered machine in the second half still ran. + """ + tool = ExecTool(working_dir=str(tmp_path)) + marker = tmp_path / "ran" + + out = await tool.execute( + command=( + "ssh -o ConnectTimeout=1 root@198.51.100.9 true; " + f"ssh -o ConnectTimeout=1 -p 58717 root@203.0.113.7 true; touch {marker}" + ) + ) + + assert "Error:" in out and "conn_gpu" in out + assert not marker.exists() + + +@pytest.mark.asyncio +async def test_the_port_given_as_an_option_counts_like_the_flag(registry, tmp_path): + """OpenSSH honours "-o Port=N" exactly as it honours "-p N" (checked with ssh -G). + + Consuming -o as a value-taking flag and dropping its value left the guard + on port 22, so a machine registered elsewhere read as unregistered. + """ + tool = ExecTool(working_dir=str(tmp_path)) + marker = tmp_path / "ran" + + out = await tool.execute(command=f"ssh -o ConnectTimeout=1 -o Port=58717 root@203.0.113.7 true; touch {marker}") + + assert "Error:" in out and "conn_gpu" in out + assert not marker.exists() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "option", + ["-o 'Port 58717'", '-o "Port 58717"', "-o 'Port = 58717'"], + ids=["single-quoted", "double-quoted", "spaced-equals"], +) +async def test_the_port_option_is_read_when_a_space_separates_it(registry, tmp_path, option): + """OpenSSH takes the option written either way, so the guard has to read both. + + `ssh -G -o "Port 58717" root@203.0.113.7` prints `port 58717`, the same as + the equals spelling. Splitting the value on the equals sign alone discarded + the spaced one, left the guard on port 22, and the line ran on the plain + shell path -- past the cap, the process-group sweep and the ledger. + """ + tool = ExecTool(working_dir=str(tmp_path)) + marker = tmp_path / "ran" + + out = await tool.execute(command=f"ssh -o ConnectTimeout=1 {option} root@203.0.113.7 true; touch {marker}") + + assert "Error:" in out and "conn_gpu" in out + assert not marker.exists() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "redirection", + ["2>&1", "&>/dev/null", "2>/dev/null"], + ids=["descriptor-dup", "both-streams", "descriptor-to-file"], +) +async def test_a_redirection_before_the_destination_does_not_hide_it(registry, tmp_path, redirection): + """A redirection is plumbing; the ssh's own words continue after it. + + `2>&1` arrives as the three tokens 2, >& and 1, so the bare descriptor was + taken for the destination, and `&>` -- matching no operator the reader knew + -- became one itself. Either way the registered host further along the line + was never read and the command ran on the plain shell path. + """ + tool = ExecTool(working_dir=str(tmp_path)) + marker = tmp_path / "ran" + + out = await tool.execute( + command=f"ssh {redirection} -o ConnectTimeout=1 -p 58717 root@203.0.113.7 true; touch {marker}" + ) + + assert "Error:" in out and "conn_gpu" in out + assert not marker.exists() + + +@pytest.mark.parametrize( + ("command", "expected"), + [ + ("ssh -p 58717 root@h true 2>&1 | tee log", [("h", 58717)]), + ("ssh -p 58717 root@h >", [("h", 58717)]), + ("ssh root@h >; ssh -p 9 root@k", [("h", 22), ("k", 9)]), + ("ssh root@h <&1", []), + ], + ids=["after-host", "dangling", "dangling-then-separator", "heredoc", "first-port-wins", "no-host"], +) +def test_redirections_are_stepped_over_wherever_they_fall(command, expected): + """A redirection is neither a destination nor the end of the command. + + Before the destination it is skipped so the host after it is read; after + the destination it changes nothing; dangling before a separator it does not + swallow the separator, so the next ssh in the line is still found. + """ + assert machine_exec._ssh_destinations(command) == expected + + +@pytest.mark.parametrize( + ("command", "expected"), + [ + ("ssh -o Port=58717 -o 'Port 22' root@h", 58717), + ("ssh -o 'Port 22' -o Port=58717 root@h", 22), + ("ssh -p 2222 -o Port=58717 root@h", 2222), + ("ssh -o Port=58717 -p 2222 root@h", 58717), + ("ssh -p 58717 -p 22 root@h", 58717), + ("ssh -p 22 -p 58717 root@h", 22), + ], + ids=["option-then-option", "reversed", "flag-then-option", "option-then-flag", "flag-twice", "flag-twice-reversed"], +) +def test_a_repeated_port_keeps_the_first_value_as_ssh_does(command, expected): + """ssh takes the first obtained value for every option, `-p` and `-o Port` + queueing together: `ssh -G -p 2222 -o Port=58717 host` prints 2222 and the + two reversed prints 58717 (measured with OpenSSH 9.9p2). + + Overwriting instead read `-p 58717 -p 22` as port 22, so a command that + really reaches the registered machine on 58717 read as unregistered and + ran on the plain shell path -- the bypass this guard exists to close. + """ + assert machine_exec._ssh_destinations(command) == [("h", expected)] + + +@pytest.mark.asyncio +async def test_an_option_that_is_not_the_port_leaves_the_port_alone(registry, tmp_path): + """-o carries many settings; only Port changes where the command lands.""" + tool = ExecTool(working_dir=str(tmp_path)) + + out = await tool.execute(command="echo ssh -o StrictHostKeyChecking=no root@203.0.113.7 true") + + assert "conn_gpu" not in out, "port 22 is not the registered 58717" + + +@pytest.mark.asyncio +async def test_a_value_taking_flag_does_not_stand_in_for_the_destination(registry, tmp_path): + """-l takes the login name, so the bare host after it is still the destination. + + Also the form with no "user@": the destination is the whole word, not + whatever follows an at sign that is not there. + """ + tool = ExecTool(working_dir=str(tmp_path)) + marker = tmp_path / "ran" + + out = await tool.execute(command=f"ssh -l root -o ConnectTimeout=1 -p 58717 203.0.113.7 true; touch {marker}") + + assert "Error:" in out and "conn_gpu" in out + assert not marker.exists() + + +# Each spelling beside what OpenSSH itself resolves it to. The expectations were +# measured with `ssh -G` (OpenSSH_9.9p2 here, and OpenSSH_8.9p1 in review for +# the ones that apply to both), and the test below re-measures them wherever an +# ssh client is installed, so a wrong expectation cannot hide in this table. +GETOPT_SPELLINGS = [ + ("ssh -vp 58717 root@h true", ("h", 58717)), + ("ssh -4p 58717 h", ("h", 58717)), + ("ssh -vvvp58717 h", ("h", 58717)), + ("ssh -vo Port=58717 h", ("h", 58717)), + ("ssh -o Port=58717 -vp 22 h", ("h", 58717)), + ("ssh root@h -p 58717 true", ("h", 58717)), + ("ssh root@h -o Port=58717 true", ("h", 58717)), + ("ssh root@h -vp 58717 ls -la", ("h", 58717)), + ("ssh -p 58717 root@h -p 22", ("h", 58717)), + ("ssh root@h true -p 58717", ("h", 22)), + ("ssh root@h -- -p 58717", ("h", 22)), + ("ssh -- root@h -p 58717", ("h", 22)), + ("ssh -B lo -p 58717 root@h", ("h", 58717)), + ("ssh -P tag -p 58717 root@h", ("h", 58717)), +] +GETOPT_IDS = [ + "bundled", + "bundled-after-a-number-flag", + "bundled-attached", + "bundled-option", + "bundled-after-a-port", + "port-after-host", + "option-after-host", + "bundled-after-host", + "first-port-wins-across-host", + "after-the-remote-command", + "after-double-dash", + "double-dash-before-host", + "bind-interface-value", + "tag-value", +] + + +@pytest.mark.parametrize(("command", "expected"), GETOPT_SPELLINGS, ids=GETOPT_IDS) +def test_options_are_read_the_way_getopt_and_ssh_read_them(command, expected): + """Two readings the reviewer showed let a real connection past the guard + (2026-09-21): a bundled group (`-vp 58717`) was skipped as if it took no + argument, so the port became the destination; and the scan stopped at the + host, while OpenSSH goes back to reading options after it until the first + word that is not one. `-B` takes a value too; the set that said which flags + do was kept by hand and had lost it, so it is read from ssh's own getopt + string now.""" + assert machine_exec._ssh_destinations(command) == [expected] + + +@pytest.mark.parametrize(("command", "expected"), GETOPT_SPELLINGS, ids=GETOPT_IDS) +def test_the_table_agrees_with_the_ssh_on_this_computer(command, expected): + import shutil + import subprocess + + if not shutil.which("ssh"): + pytest.skip("no ssh on this computer") + words = command.split()[1:] + out = subprocess.run(["ssh", "-G", *words], capture_output=True, text=True, check=False, timeout=10).stdout + seen = dict(line.split(" ", 1) for line in out.splitlines() if line.startswith(("hostname ", "port "))) + if "-P" in words and "tag" not in out: + pytest.skip("this ssh predates `-P tag` (OpenSSH < 9.2), where P takes no value") + assert (seen.get("hostname"), int(seen.get("port", 0))) == expected + + +def test_the_value_taking_flags_are_ssh_s_own(): + """The set is derived from the getopt string, not listed beside it: the + hand-kept list this replaces had lost `B`.""" + assert machine_exec._SSH_VALUE_FLAGS == frozenset("bceilmopBDEFIJLOPQRSWw") + + +@pytest.mark.asyncio +async def test_a_port_written_after_the_host_is_still_refused(registry, tmp_path): + """Through ExecTool, the shape the reviewer ran: the whole line used to run + on the plain shell path and reached 203.0.113.7:58717.""" + tool = ExecTool(working_dir=str(tmp_path)) + marker = tmp_path / "ran" + + for command in ( + f"ssh -o ConnectTimeout=1 root@203.0.113.7 -p 58717 true; touch {marker}", + f"ssh -o ConnectTimeout=1 -vp 58717 root@203.0.113.7 true; touch {marker}", + ): + out = await tool.execute(command=command) + assert "Error:" in out and "conn_gpu" in out, command + assert not marker.exists()