From 3cb4d490ed67c9205ae6a08d628c5eb793a57649 Mon Sep 17 00:00:00 2001 From: worldmozara Date: Fri, 23 Oct 2026 10:43:05 +0800 Subject: [PATCH] core/tools: announce a remote shell instead of judging it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ssh prod` runs no command this scan can resolve, and what is typed or piped into the shell it opens never reaches any gate — `write_pty` and `run_argv(stdin=…)` are both unchecked input. The tool description said nothing about it, so the call looked like any other. The hop is now reported even when no command follows (`RemoteTarget.shell`), and the review announces it: `remote shell on 'prod' (ssh with no command; its input is not checked)`. It is a soft confirm like the other reviews (YOLO may skip it, an escalation behind a hop still may not) — the client itself stays outside the denylist, since there is no program to name. --- AGENTS.md | 2 +- src/plyngent/tools/command_scan.py | 14 ++++++++++---- src/plyngent/tools/danger.py | 11 ++++++++--- tests/test_tools/test_command_scan.py | 9 +++++++-- tests/test_tools/test_danger.py | 14 ++++++++++++++ 5 files changed, 40 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a81d389..d4bdf72 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -103,7 +103,7 @@ Module-level `@tool` handlers. Call `set_workspace_root()` before use. - **`process`**: `run_argv` (argv, no shell, timeout, optional stdin/env); `run_argv_batch` (serial steps, `pipe_out` on provider, `mix_stderr`, `stop_on_error`); PTY `open_pty` / `read_pty` / `write_pty` (literal) / `write_pty_keys` (escapes) / `ask_into_pty` (human→PTY only; `secret` no-echo; answer never in tool result) / `close_pty` (POSIX: `pty`+`fork`; Windows: ConPTY via `pywinpty` env marker dep). - PTY: backend in `pty_backend.py`; structured status (`alive`/`exit_code`/`data`); `read_pty(..., until=)` (**CSI sanitized** so host TTY is never reset); keys via `write_pty_keys` only (`\xHH`, `ctrl+x`, `key=esc`); secrets via `ask_into_pty` (not `write_pty` data); session limit/idle TTL/output budget; close terminate→kill. - CLI limit hooks: interactive confirm to raise tool-loop rounds, PTY session cap, or PTY output budget. The prompt accepts `y`/empty (once), `n`, or `yyy` = stop asking for the rest of the turn (`cli.limits` module flag, reset by `run_turn_with_retries`, `/btw`, session switches, and chat exit). `--auto-continue` / `[agent] auto_continue_limits` set the process-wide default (covers one-shot/PTY); the flag does not imply YOLO and vice versa. -- Destructive confirms: `classify_danger` + `ToolRegistry(on_confirm=…)`; CLI default deny; config `confirm_destructive` / `path_denylist`. Soft confirm also for interpreters — bare shells/REPLs, scripts, and `-c` one-liners alike (`-c` is code only for an interpreter, so `grep -c` / `od -c` stay unflagged; argv + code preview) —, detached runners (`nohup`), and wrapper options that carry a command line (`env -S "…"`, `su -c "…"`, whose text is printed under its own `env -S:` label), each resolved past env wrappers and ` run`; deny may include a user comment for the model. `ssh` / `mosh` are unwrapped like a wrapper (`command_scan`): the program reviewed is the one the client runs on the far end (`ssh prod rm -rf /` is refused as `rm`, `ssh prod bash -c …` prints the remote code, an escalation behind the hop keeps `yolo_skip=False`), and `-o BatchMode=yes` always asks because nothing on either end can prompt. A client with **no** command only opens a remote shell, which this scan does not look into — what is typed or piped in (`write_pty`, `run_argv(stdin=…)`) is neither checked nor shown; that gap is the reason a remote shell is a decision for the host, not a gate here. The `delete_path` confirm names what will happen — `move path 'x' to the system trash`, or `delete path 'x' permanently` when the call asks for it, the host cleared `[trash]`, or the path is scratch — and the pretty line prints `(to trash)` for a delete that landed there. Session YOLO: `/yolo on|off|once` and `--yes` (skip the soft confirms of `YOLO`-tagged tools — interpreter runs included — while untagged ones still prompt and a command that escalates privileges keeps its confirm through `DangerReason(yolo_skip=False)`; hard denylists unchanged; `once` expires after the next user turn). A round of tool calls asks **once**: `ToolRegistry.prepare_batch_confirm` pre-scans it (unknown tools, undecodable arguments, calls missing a required argument, and calls YOLO already covers are left out — nobody is asked about a call that cannot run or will not ask), two or more soft reasons collapse into one `ToolBatchConfirmHook` prompt whose `BatchConfirmDecision` `execute(..., decision=…)` applies to each call (CLI box lists every call: `y` = allow these, `m` = review them one at a time through the per-call prompt, `N` = deny with one comment shared by all), the answer covers that round only (the next one asks again), and a lone request keeps the plain one-call prompt. +- Destructive confirms: `classify_danger` + `ToolRegistry(on_confirm=…)`; CLI default deny; config `confirm_destructive` / `path_denylist`. Soft confirm also for interpreters — bare shells/REPLs, scripts, and `-c` one-liners alike (`-c` is code only for an interpreter, so `grep -c` / `od -c` stay unflagged; argv + code preview) —, detached runners (`nohup`), and wrapper options that carry a command line (`env -S "…"`, `su -c "…"`, whose text is printed under its own `env -S:` label), each resolved past env wrappers and ` run`; deny may include a user comment for the model. `ssh` / `mosh` are unwrapped like a wrapper (`command_scan`): the program reviewed is the one the client runs on the far end (`ssh prod rm -rf /` is refused as `rm`, `ssh prod bash -c …` prints the remote code, an escalation behind the hop keeps `yolo_skip=False`), and `-o BatchMode=yes` always asks because nothing on either end can prompt. A client with **no** command only opens a remote shell: nothing typed or piped into it (`write_pty`, `run_argv(stdin=…)`) is checked, so opening one is announced and confirmed instead — `remote shell on 'prod' (ssh with no command; its input is not checked)` — while the client itself stays outside the denylist (there is no program to name). The `delete_path` confirm names what will happen — `move path 'x' to the system trash`, or `delete path 'x' permanently` when the call asks for it, the host cleared `[trash]`, or the path is scratch — and the pretty line prints `(to trash)` for a delete that landed there. Session YOLO: `/yolo on|off|once` and `--yes` (skip the soft confirms of `YOLO`-tagged tools — interpreter runs included — while untagged ones still prompt and a command that escalates privileges keeps its confirm through `DangerReason(yolo_skip=False)`; hard denylists unchanged; `once` expires after the next user turn). A round of tool calls asks **once**: `ToolRegistry.prepare_batch_confirm` pre-scans it (unknown tools, undecodable arguments, calls missing a required argument, and calls YOLO already covers are left out — nobody is asked about a call that cannot run or will not ask), two or more soft reasons collapse into one `ToolBatchConfirmHook` prompt whose `BatchConfirmDecision` `execute(..., decision=…)` applies to each call (CLI box lists every call: `y` = allow these, `m` = review them one at a time through the per-call prompt, `N` = deny with one comment shared by all), the answer covers that round only (the next one asks again), and a lone request keeps the plain one-call prompt. - **`vcs`**: read-only VCS tools (`vcs_kind` / `vcs_status` / `vcs_diff` / `vcs_log` / `vcs_branch`) via `VcsBackend` protocol; **git** implemented; detectors are pluggable for other systems. - **`chat`**: human prompts as tools — `ask_user_line` / `ask_user_choice` / `ask_user_form` (shared `prompting` core); `wait` (line prompt with timeout; Enter disturbs). Container args are shape-checked (`tools/chat/shape.py`): a mis-typed string `options`/`fields` (or a non-string `question`) returns an explained error — `options="a, b"` used to iterate characters into a per-character menu. `wait`'s `duration` accepts numeric strings (`"5"` = 5s) and rejects booleans/text/containers. - **`todo`**: LIFO stack of **task groups** (not a queue of tasks) — `todo_push` creates one group of siblings; `todo_pop` removes the whole top group; `todo_update` by the number a listing shows (render order, TOP group first; the stored `id` stays internal); DFS breakdown push[T1,T2]→push[T1.1…]→pop→push[T2.1…]; stored on session row; open items = unfinished work (`[TODO OPEN WORK]`); all-terminal non-empty = hygiene (`[TODO HYGIENE]`); nag channel via `[agent] todo_nag_strategy` (`developer`/`user` = prose nag; `synthetic_tool` = forged `todo_list` call + **real** `stack.render()` result + ToolCall/Result events; `none`). diff --git a/src/plyngent/tools/command_scan.py b/src/plyngent/tools/command_scan.py index fcc7f5d..4003b12 100644 --- a/src/plyngent/tools/command_scan.py +++ b/src/plyngent/tools/command_scan.py @@ -275,6 +275,8 @@ class RemoteTarget: host: str #: ``-o BatchMode=yes``: the client never prompts, so the command runs. batch: bool = False + #: No command followed: the client opens a shell whose input is not scanned. + shell: bool = False @dataclass(frozen=True) @@ -346,7 +348,8 @@ def unwrap_command(argv: Sequence[str]) -> UnwrappedCommand: ``ssh`` / ``mosh`` are unwrapped the same way — the program under review is the one after the destination (``ssh prod rm -rf /`` runs ``rm``), and the hop is reported in :attr:`UnwrappedCommand.remote`. A client with no command - opens a remote shell, which this scan does not look into. + opens a remote shell (``remote.shell``); nothing behind it can be resolved + here. """ parts = list(argv) wrappers: list[str] = [] @@ -359,6 +362,8 @@ def unwrap_command(argv: Sequence[str]) -> UnwrappedCommand: start = _skip_remote(name, parts, index + 1) if start is not None: remote = start.target + if start.index is None: + break # a remote shell: the client is the whole command wrappers.append(name) index = start.index continue @@ -384,12 +389,13 @@ def unwrap_command(argv: Sequence[str]) -> UnwrappedCommand: class _RemoteStart: """Where a remote client's own command starts, and the hop it reaches.""" - index: int + #: Index of the remote command, or ``None`` when only a shell opens. + index: int | None target: RemoteTarget def _skip_remote(name: str, parts: list[str], start: int) -> _RemoteStart | None: - """Where the command *name* runs elsewhere starts; ``None`` for a bare shell. + """Where the command *name* runs elsewhere starts; ``None`` without a hop. The destination is the first token that is not one of the client's own options (``ssh -o BatchMode=yes -p 2222 [user@]host …``); the command follows @@ -415,7 +421,7 @@ def _skip_remote(name: str, parts: list[str], start: int) -> _RemoteStart | None if name == "mosh" and index < len(parts) and parts[index] == "--": index += 1 if index >= len(parts): - return None + return _RemoteStart(index=None, target=RemoteTarget(wrapper=name, host=host, batch=batch, shell=True)) return _RemoteStart(index=index, target=RemoteTarget(wrapper=name, host=host, batch=batch)) diff --git a/src/plyngent/tools/danger.py b/src/plyngent/tools/danger.py index d1cd3e4..d1b6056 100644 --- a/src/plyngent/tools/danger.py +++ b/src/plyngent/tools/danger.py @@ -63,15 +63,20 @@ def _command_reason(argv: Sequence[str], *, via: str) -> DangerReason | None: A command on another machine (``ssh``/``mosh``) is reviewed by the program it runs there — the client's destination and options are skipped. With ``-o BatchMode=yes`` the client cannot prompt at either end, so the command - is always confirmed, whatever program it is. + is always confirmed, whatever program it is. A client with no command opens a + remote shell, which is announced: nothing typed or piped into it can be + checked, so the human decides before it starts. """ scan = unwrap_command(argv) interpreter = is_interpreter(scan.base) findings = [f"{name} (detached run)" for name in scan.self_review] if interpreter: findings.append(f"interpreter {scan.base!r}") - if scan.remote is not None and scan.remote.batch: - findings.append(f"remote command on {scan.remote.host!r} (BatchMode=yes, no prompt on either end)") + remote = scan.remote + if remote is not None and remote.shell: + findings.append(f"remote shell on {remote.host!r} ({remote.wrapper} with no command; its input is not checked)") + elif remote is not None and remote.batch: + findings.append(f"remote command on {remote.host!r} (BatchMode=yes, no prompt on either end)") findings.extend(f"{span.label} runs a command line" for span in scan.command_strings) findings = list(dict.fromkeys(findings)) if not findings: diff --git a/tests/test_tools/test_command_scan.py b/tests/test_tools/test_command_scan.py index 2567e80..ea8b8e6 100644 --- a/tests/test_tools/test_command_scan.py +++ b/tests/test_tools/test_command_scan.py @@ -171,11 +171,16 @@ def test_unwrap_remote_batch_mode() -> None: def test_unwrap_remote_client_alone() -> None: """A client with no command opens a remote shell: nothing to unwrap.""" - for argv in (["ssh", "prod"], ["ssh", "-N", "prod"], ["ssh", "-G", "prod"], ["ssh"], ["mosh", "prod"]): + for argv in (["ssh", "prod"], ["ssh", "-N", "prod"], ["ssh", "-G", "prod"], ["mosh", "prod"]): scan = unwrap_command(argv) assert scan.wrappers == (), argv assert scan.base == argv[0], argv - assert scan.remote is None, argv + assert scan.remote is not None, argv + assert scan.remote.shell is True, argv + assert scan.remote.host == "prod", argv + # Without a destination there is no hop to announce. + for argv in (["ssh"], ["ssh", "-V"]): + assert unwrap_command(argv).remote is None, argv def test_unwrap_remote_command_still_reviews_what_runs() -> None: diff --git a/tests/test_tools/test_danger.py b/tests/test_tools/test_danger.py index 7c841cb..dfe6f47 100644 --- a/tests/test_tools/test_danger.py +++ b/tests/test_tools/test_danger.py @@ -174,6 +174,20 @@ def test_a_remote_command_is_judged_by_what_runs_there() -> None: assert pty is not None and "python3" in pty +def test_a_remote_shell_is_announced() -> None: + """``ssh host`` runs nothing this scan can see, so the human is told first.""" + reason = _reason("open_pty", {"command": ["ssh", "prod"]}) + assert reason is not None + assert "remote shell on 'prod'" in reason + assert "input is not checked" in reason + # The same for a one-shot client: stdin is another unchecked input channel. + assert "remote shell on 'prod'" in (_reason("run_argv", {"argv": ["ssh", "-p", "2222", "prod"]}) or "") + # A mosh shell counts too, and a hop with nothing after it is not silent. + assert "remote shell" in (_reason("open_pty", {"command": ["mosh", "prod"]}) or "") + # A remote *command* keeps its own review (no shell wording). + assert "remote shell" not in (_reason("run_argv", {"argv": ["ssh", "prod", "uptime"]}) or "") + + def test_batch_mode_is_always_reviewed() -> None: """``BatchMode=yes`` cannot prompt at either end, so the command is shown.""" reason = _reason("run_argv", {"argv": ["ssh", "-o", "BatchMode=yes", "prod", "deploy.sh"]})