From 761ebdb857fd281cf1f6fcb4e6dd1ea57bb68e77 Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Fri, 2 Oct 2026 21:39:46 +0000 Subject: [PATCH 1/5] Run every command on the build VM over SSH (M1.3) execution.SSHHost.execute is now the only way the agent runs anything (L13). Commands travel as a script on SSH's stdin, so the secrets handed to a call are never on a command line, and every value handed out is redacted from the output. Timeouts kill the remote process group, and an unreachable host is reported as such, not as a failed command. Workspace and GitRepo run on the build VM. The clone uses the repository's SSH URL, so no credential is stored in it (#89); fetches and pushes get the push key over stdin, in a short-lived ssh-agent, and PRs are still opened from the agent's host with its API token. The bench runs on the build host too. Tests reach "the build VM" through a throwaway local sshd (nisse_dev .testing); CI installs openssh-server. The live push test now pushes with the sandbox's deploy key (NISSE_PUSH_KEY). Fixes #89. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE --- .env.example | 3 + .gitea/workflows/ci.yml | 11 ++ conftest.py | 18 ++ docs/Architecture.md | 4 +- nisse-dev.example.yaml | 9 +- nisse-dev/src/nisse_dev/agent.py | 61 +++++-- nisse-dev/src/nisse_dev/bench/runner.py | 13 +- nisse-dev/src/nisse_dev/bench/tasks.py | 25 +++ nisse-dev/src/nisse_dev/config.py | 47 ++++- nisse-dev/src/nisse_dev/execution.py | 207 ++++++++++++++++++++++ nisse-dev/src/nisse_dev/git.py | 141 +++++++++------ nisse-dev/src/nisse_dev/gitea.py | 4 + nisse-dev/src/nisse_dev/publish.py | 19 +- nisse-dev/src/nisse_dev/testing.py | 101 +++++++++++ nisse-dev/src/nisse_dev/tools.py | 179 +++++++++++-------- nisse-dev/tests/test_agent_build.py | 69 +++++--- nisse-dev/tests/test_bench.py | 27 ++- nisse-dev/tests/test_config.py | 23 +++ nisse-dev/tests/test_delegation.py | 7 +- nisse-dev/tests/test_execution.py | 77 ++++++++ nisse-dev/tests/test_executor.py | 5 +- nisse-dev/tests/test_git.py | 129 ++++++++++---- nisse-dev/tests/test_gitea.py | 8 +- nisse-dev/tests/test_publish.py | 21 ++- nisse-dev/tests/test_tools.py | 57 +++++- scripts/benchmark.py | 4 +- scripts/bringup.sh | 26 ++- tests/integration/nisse-test.yaml | 8 +- tests/integration/test_benchmark_live.py | 7 +- tests/integration/test_delegation_live.py | 27 ++- tests/integration/test_gitea_live.py | 36 ++-- tests/integration/test_milestone_live.py | 5 +- 32 files changed, 1098 insertions(+), 280 deletions(-) create mode 100644 conftest.py create mode 100644 nisse-dev/src/nisse_dev/execution.py create mode 100644 nisse-dev/src/nisse_dev/testing.py create mode 100644 nisse-dev/tests/test_execution.py diff --git a/.env.example b/.env.example index bda36a0..b5e1d85 100644 --- a/.env.example +++ b/.env.example @@ -11,6 +11,9 @@ NISSE_LLM_TOKEN=replace-me NISSE_MATRIX_TOKEN=replace-me # gitea.token: the Gitea service account's scoped token (B2) NISSE_GITEA_TOKEN=replace-me +# The sandbox repository's deploy key (write access), for the live push test: +# the private key itself, or here, since a .env line can't hold one, its path. +NISSE_PUSH_KEY=/root/nisse-e2e/ssh/push_sandbox # Where the config file is, if not ./nisse-dev.yaml. Where this file is, if # not ./.env, is NISSE_ENV, which has to be set in the real environment. diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 07030d1..a5c3c3b 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -24,6 +24,15 @@ jobs: - name: Sync workspace run: uv sync --all-packages + - name: Install sshd + # The tests reach "the build VM" over real SSH, through a throwaway + # sshd the test fixture starts (nisse_dev/testing.py). + run: | + SUDO=$(command -v sudo || true) + $SUDO apt-get update -qq + $SUDO apt-get install -y -qq openssh-server openssh-client > /dev/null + $SUDO mkdir -p /run/sshd + - name: Lint run: uv run ruff check . @@ -41,4 +50,6 @@ jobs: NISSE_LLM_TOKEN: ${{ secrets.NISSE_LLM_TOKEN }} NISSE_MATRIX_TOKEN: ${{ secrets.NISSE_MATRIX_TOKEN }} NISSE_GITEA_TOKEN: ${{ secrets.NISSE_GITEA_TOKEN }} + # The sandbox's deploy key (private half), which pushes go by (D4). + NISSE_PUSH_KEY: ${{ secrets.NISSE_PUSH_KEY }} run: uv run pytest -m integration -rs diff --git a/conftest.py b/conftest.py new file mode 100644 index 0000000..3514a20 --- /dev/null +++ b/conftest.py @@ -0,0 +1,18 @@ +"""Fixtures shared by every test tree.""" + +from collections.abc import Iterator +from pathlib import Path + +import pytest + +from nisse_dev.execution import SSHHost +from nisse_dev.testing import local_sshd + + +@pytest.fixture(scope="session") +def ssh_host(tmp_path_factory: pytest.TempPathFactory) -> Iterator[SSHHost]: + """A throwaway sshd on this machine, standing in for the build VM (L13). + It runs as the current user, so remote paths are this machine's paths and + tests can inspect what a command did.""" + with local_sshd(Path(tmp_path_factory.mktemp("sshd"))) as host: + yield host diff --git a/docs/Architecture.md b/docs/Architecture.md index 7e4628a..88c9594 100644 --- a/docs/Architecture.md +++ b/docs/Architecture.md @@ -543,8 +543,8 @@ event, work item, cause); every verdict, escalation and external effect is logge - **MCP tools are the one deliberate exception:** fixed code against an API, with the model choosing only arguments. They still pass through P3, P4 and L4, and their output is redacted (L12). -- **Tests use SSH too,** against a throwaway `sshd` container, so the tested path is - the production path. +- **Tests use SSH too,** against a throwaway `sshd`, so the tested path is the + production path. --- diff --git a/nisse-dev.example.yaml b/nisse-dev.example.yaml index 2fbd6bf..8690a50 100644 --- a/nisse-dev.example.yaml +++ b/nisse-dev.example.yaml @@ -9,9 +9,15 @@ # silently doing nothing. state_db: ~/nisse-demo-workspace/state.db -workspace: ~/nisse-demo-workspace/project # the clone +workspace: /root/work/demo # the clone, on the build host project: demo +build_host: # L13: every command runs here, over SSH + address: build-demo.example + key: ~/.ssh/nisse-dev # the agent's key for the build host + # user: root + # port: 22 + llm: # L9: LiteLLM is the only LLM destination base_url: http://litellm.example:4000 token: ${NISSE_LLM_TOKEN} @@ -41,6 +47,7 @@ gitea: # D4, D5 repo: owner/demo-repo operators: [your-gitea-login] base_branch: main + push_key: ~/.ssh/push-demo-repo # a deploy key with write access (D4) # What each knob does, and how to choose a value: docs/Tuning-Guide.md. # These are the defaults; the section can be left out entirely. The file is diff --git a/nisse-dev/src/nisse_dev/agent.py b/nisse-dev/src/nisse_dev/agent.py index a711a09..a97e81d 100644 --- a/nisse-dev/src/nisse_dev/agent.py +++ b/nisse-dev/src/nisse_dev/agent.py @@ -18,9 +18,10 @@ from nisse_dev.config import AgentConfig from nisse_dev.context import BudgetedClient from nisse_dev.control import Orchestrator from nisse_dev.delegation import Caps, Delegation +from nisse_dev.execution import SSHHost from nisse_dev.executor import LLMExecutor from nisse_dev.expert import ExpertDesk -from nisse_dev.git import GitRepo, authenticated_url +from nisse_dev.git import GitRepo from nisse_dev.gitea import Comment, GiteaClient from nisse_dev.llm import AnthropicPassthroughClient, LiteLLMClient from nisse_dev.matrix import MatrixTransport @@ -164,19 +165,42 @@ def make_llm_clients( return normalized, passthrough -def prepare_workspace(config: AgentConfig, account: str) -> GitRepo: - """Ensure the workspace is a clone of the project repo. - - Cloning here rather than on `use ` is the single-project - shortcut: resolving a project name to a repo is issue #27's work. - """ - repo = GitRepo(config.workspace) - remote = authenticated_url( - config.gitea_url, config.gitea_repo, account, config.gitea_token +def build_host(config: AgentConfig) -> SSHHost: + """The tool VM the workspace lives on (L13). Its known-hosts file and + connection socket live beside the state store.""" + return SSHHost( + config.build_host.address, + key=config.build_host.key, + user=config.build_host.user, + port=config.build_host.port, + state_dir=config.state_db.parent / "ssh", ) - if not (config.workspace / ".git").exists(): - config.workspace.parent.mkdir(parents=True, exist_ok=True) - GitRepo(config.workspace.parent).clone(remote, config.workspace) + + +def read_push_key(config: AgentConfig) -> str | None: + if config.push_key is None: + return None + try: + return config.push_key.read_text() + except OSError as exc: + raise RuntimeError( + f"cannot read the push key {config.push_key}: {exc}" + ) from exc + + +def prepare_workspace( + host: SSHHost, config: AgentConfig, account: str, remote: str, key: str | None +) -> GitRepo: + """Ensure the workspace on the build VM is a clone of the project repo. + + The clone uses the repository's SSH URL, so no credential is ever stored + in it (#89); the push key is handed to the clone and nothing else. + Cloning here rather than when a work item starts is the single-project + shortcut that M1.4 and D3's fresh workspace replace. + """ + repo = GitRepo(host, config.workspace) + if not repo.exists(): + repo.clone(remote, key=key) repo.configure_identity(account, f"{account}@users.noreply.local") return repo @@ -204,9 +228,13 @@ def build_agent( # anything depends on it. account = gitea.whoami() - git_repo = prepare_workspace(config, account) + host = build_host(config) + push_key = read_push_key(config) + remote = gitea.ssh_url(config.gitea_repo) + git_repo = prepare_workspace(host, config, account, remote, push_key) tuning = config.tuning workspace = Workspace( + host, config.workspace, timeout_s=tuning.tools.command_timeout_s, max_output_chars=tuning.tools.max_output_chars, @@ -249,9 +277,8 @@ def build_agent( gitea, config.gitea_repo, git_repo, - base_url=config.gitea_url, - account=account, - token=config.gitea_token, + remote_url=remote, + push_key=push_key, base_branch=config.base_branch, ) diff --git a/nisse-dev/src/nisse_dev/bench/runner.py b/nisse-dev/src/nisse_dev/bench/runner.py index 26ad4ed..358bb15 100644 --- a/nisse-dev/src/nisse_dev/bench/runner.py +++ b/nisse-dev/src/nisse_dev/bench/runner.py @@ -1,9 +1,11 @@ """Running one task once with one executor model, and recording it.""" import io +import shlex import subprocess import tempfile import time +import uuid from dataclasses import asdict, dataclass, field from datetime import UTC, datetime from pathlib import Path @@ -20,6 +22,7 @@ from nisse_dev.canonical import ( ) from nisse_dev.catalog import ModelFacts from nisse_dev.config import Tuning +from nisse_dev.execution import SSHHost from nisse_dev.executor import LLMExecutor from nisse_dev.llm import RoleMap from nisse_dev.oplog import OpLog @@ -150,6 +153,7 @@ def run_task( client: Any, tuning: Tuning, *, + host: SSHHost, note: str = "", commit: str = "", workdir: Path | None = None, @@ -161,12 +165,14 @@ def run_task( model under test, not a frontier model's success.""" with tempfile.TemporaryDirectory(dir=workdir) as tmp: root = Path(tmp) + # The workspace is on the build VM, like the agent's own (L13). workspace = Workspace( - root / "work", + host, + f"/tmp/nisse-bench/{uuid.uuid4().hex}", timeout_s=tuning.tools.command_timeout_s, max_output_chars=tuning.tools.max_output_chars, ) - task.seed_into(workspace.root) + task.seed_workspace(workspace) store = StateStore(root / "state.db", oplog=OpLog("nisse-bench", io.StringIO())) # One work item per run, as the agent's own tasks are (L1). store.open_work_item( @@ -198,7 +204,8 @@ def run_task( status, detail = "error", repr(exc) wall = time.monotonic() - started - success, check_output = task.run_check(workspace.root) + success, check_output = task.check_workspace(workspace) + host.execute(f"rm -rf -- {shlex.quote(str(workspace.root))}", timeout_s=60) failed, invented = _count(store.transcript("bench", "executor").read()) store.close() diff --git a/nisse-dev/src/nisse_dev/bench/tasks.py b/nisse-dev/src/nisse_dev/bench/tasks.py index e71a4fe..2985189 100644 --- a/nisse-dev/src/nisse_dev/bench/tasks.py +++ b/nisse-dev/src/nisse_dev/bench/tasks.py @@ -13,13 +13,18 @@ Each task is a directory under ``tasks/``: """ import hashlib +import shlex import shutil import subprocess from dataclasses import dataclass from pathlib import Path +from typing import TYPE_CHECKING import yaml +if TYPE_CHECKING: + from nisse_dev.tools import Workspace + TASKS_DIR = Path(__file__).parent / "tasks" CHECK_DIR = ".bench_check" @@ -75,6 +80,26 @@ class Task: output = (result.stdout + result.stderr)[-2000:] return result.returncode == 0, output + # -- on the build VM, where the agent's runs happen (L13) --------------- + + def seed_workspace(self, workspace: "Workspace") -> None: + workspace.upload_tree(self.path / "seed") + + def check_workspace( + self, workspace: "Workspace", timeout_s: int = 300 + ) -> tuple[bool, str]: + """run_check, on the build VM: copy the hidden files in fresh (never + trusting a directory the model made) and run the check there.""" + root = shlex.quote(str(workspace.root / CHECK_DIR)) + workspace.host.execute(f"rm -rf -- {root}", timeout_s=60) + workspace.upload_tree(self.path / "hidden", CHECK_DIR) + result = workspace.host.execute( + self.check, cwd=str(workspace.root), timeout_s=timeout_s + ) + if result.timed_out: + return False, f"check timed out after {timeout_s}s" + return result.exit_code == 0, (result.stdout + result.stderr)[-2000:] + def load_task(path: Path) -> Task: spec = yaml.safe_load((path / "task.yaml").read_text()) diff --git a/nisse-dev/src/nisse_dev/config.py b/nisse-dev/src/nisse_dev/config.py index ac208b6..bfeba29 100644 --- a/nisse-dev/src/nisse_dev/config.py +++ b/nisse-dev/src/nisse_dev/config.py @@ -47,8 +47,17 @@ REQUIRED_KEYS = ( "gitea.token", "gitea.repo", "gitea.operators", + "build_host.address", + "build_host.key", +) +OPTIONAL_KEYS = ( + "llm.max_output_tokens", + "llm.request_timeout_s", + "gitea.base_branch", + "gitea.push_key", + "build_host.user", + "build_host.port", ) -OPTIONAL_KEYS = ("llm.max_output_tokens", "llm.request_timeout_s", "gitea.base_branch") _REFERENCE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") @@ -205,6 +214,17 @@ def _read_file(path: Path, errors: list[str]) -> dict[str, Any]: return loaded +@dataclass(frozen=True) +class BuildHost: + """The tool VM the workspace lives on (L13, D3). ``key`` is the path to + the agent's SSH private key for it.""" + + address: str + key: Path + user: str = "root" + port: int = 22 + + @dataclass(frozen=True) class AgentConfig: state_db: Path @@ -219,7 +239,11 @@ class AgentConfig: gitea_token: str gitea_repo: str gitea_operators: frozenset[str] + build_host: BuildHost base_branch: str = "main" + # The key that may push to the project repository, handed to each push on + # the build VM and to nothing else (D4). + push_key: Path | None = None max_output_tokens: int = DEFAULT_MAX_OUTPUT_TOKENS request_timeout_s: float = DEFAULT_REQUEST_TIMEOUT_S tuning: Tuning = field(default_factory=Tuning) @@ -300,9 +324,17 @@ class AgentConfig: raise ValueError except (TypeError, ValueError): errors.append("llm.request_timeout_s: must be a positive number of seconds") + port = values.get("build_host.port", 22) + try: + port = int(port) + if not 0 < port < 65536: + raise ValueError + except (TypeError, ValueError): + errors.append("build_host.port: must be a TCP port number") if errors: raise ConfigError("cannot start: " + "; ".join(errors)) + push_key = values.get("gitea.push_key") config = cls( state_db=Path(values["state_db"]).expanduser(), workspace=Path(values["workspace"]).expanduser(), @@ -326,6 +358,13 @@ class AgentConfig: gitea_token=str(values["gitea.token"]), gitea_repo=str(values["gitea.repo"]), gitea_operators=_people(values["gitea.operators"]), + build_host=BuildHost( + address=str(values["build_host.address"]), + key=Path(values["build_host.key"]).expanduser(), + user=str(values.get("build_host.user") or "root"), + port=port, + ), + push_key=Path(push_key).expanduser() if push_key else None, base_branch=str(values.get("gitea.base_branch") or "main"), max_output_tokens=max_output, request_timeout_s=request_timeout, @@ -337,7 +376,9 @@ class AgentConfig: def _refuse_secrets_inside_the_clone(self, env: Mapping[str, str]) -> None: """The workspace is a git clone the agent commits with ``git add -A`` - and pushes. Anything inside it reaches the project repository, so a + and pushes. It lives on the build VM, so this only bites when the + build host is this machine (tests, a local bring-up), and there it + matters exactly as before. Anything inside it reaches the project repository, so a state store or an env file placed there would publish credentials as a side effect of the agent working correctly — silently, and once. The config file holds no secrets itself, but it names where they come @@ -367,7 +408,7 @@ class AgentConfig: """What the agent may safely announce about how it is configured.""" return { "project": self.project, - "workspace": str(self.workspace), + "workspace": f"{self.build_host.address}:{self.workspace}", "repo": self.gitea_repo, "planner": self.roles.planner, "executor": self.roles.executor, diff --git a/nisse-dev/src/nisse_dev/execution.py b/nisse-dev/src/nisse_dev/execution.py new file mode 100644 index 0000000..12892ec --- /dev/null +++ b/nisse-dev/src/nisse_dev/execution.py @@ -0,0 +1,207 @@ +"""Running commands on a tool VM over SSH (architecture L13). + +``SSHHost.execute`` is the only way the agent runs anything: there is no +local execution path, so the host the agent lives on never runs a command a +model chose. The harness binds the host; a model never names one. + +How a command travels. The remote side runs ``bash -s`` and reads a small +script from SSH's stdin: the secrets handed to this one call as ``export`` +lines, a ``cd``, then the command under ``timeout``. Nothing secret is ever on +a command line (``ps`` and ``/proc/*/cmdline`` would show it); the values exist +only in that shell and the command's environment. The command's own stdin is +``/dev/null``, or a here-document carrying ``stdin``, so it can never read the +rest of the script. + +Every value handed out is redacted from what comes back (L12): a test that +prints its environment must not put a key into the transcript. + +The connection is reused across calls (OpenSSH's ControlMaster), so a tool +call costs one round trip, not a handshake. +""" + +import os +import secrets as _random +import shlex +import subprocess +import tempfile +from collections.abc import Iterable, Mapping +from dataclasses import dataclass +from pathlib import Path + +# The remote command's exit code is passed back as is, except that 255 is +# what ssh itself uses for "could not connect"; a command exiting 255 is +# reported as 254 so the two can't be confused. +SSH_FAILED = 255 +_TIMED_OUT = 124 # coreutils `timeout` +_KILL_GRACE_S = 5 +# The local backstop, beyond the remote timeout, for a connection that hangs. +_LOCAL_SLACK_S = 30 + +REDACTED = "[redacted]" + + +class HostUnavailable(RuntimeError): + """The host could not be reached: not something the command did.""" + + +@dataclass(frozen=True) +class Result: + exit_code: int + stdout: str + stderr: str + timed_out: bool = False + + @property + def ok(self) -> bool: + return self.exit_code == 0 and not self.timed_out + + +def redact(text: str, values: Iterable[str]) -> str: + """Mask every non-trivial secret value in ``text``. Longest first, so a + value containing another is masked whole.""" + for value in sorted((v for v in values if len(v) >= 4), key=len, reverse=True): + text = text.replace(value, REDACTED) + return text + + +def _heredoc(body: str) -> tuple[str, str]: + """A here-document redirection whose delimiter can't occur in ``body``.""" + while True: + tag = f"NISSE_EOF_{_random.token_hex(8)}" + if tag not in body: + return f"<<'{tag}'", f"\n{body}\n{tag}" + + +def _script( + command: str, + cwd: str | None, + secrets: Mapping[str, str], + stdin: str | None, + timeout_s: int, +) -> str: + lines = [f"export {name}={shlex.quote(value)}" for name, value in secrets.items()] + if cwd is not None: + lines.append(f"cd -- {shlex.quote(cwd)} || exit 125") + redirect, body = ("< /dev/null", "") if stdin is None else _heredoc(stdin) + # `timeout` runs the command in its own process group and signals the + # whole group, so a backgrounded grandchild doesn't outlive it. + lines.append( + f"timeout --kill-after={_KILL_GRACE_S} {timeout_s} " + f"bash -c {shlex.quote(command)} {redirect}{body}" + ) + lines.append("rc=$?; [ $rc -eq 255 ] && rc=254; exit $rc") + return "\n".join(lines) + "\n" + + +class SSHHost: + """One tool VM, reached as ``user@address`` with a private key.""" + + def __init__( + self, + address: str, + *, + key: Path, + state_dir: Path, + user: str = "root", + port: int = 22, + ) -> None: + self.address = address + self.user = user + self.port = port + self._key = Path(key) + state_dir = Path(state_dir) + state_dir.mkdir(parents=True, exist_ok=True) + self._known_hosts = state_dir / "known_hosts" + # A Unix socket's path is limited to about 100 bytes, so the shared + # connection's socket lives in a short private directory rather than + # beside the state, named by %C, ssh's hash of the connection. + sockets = Path(tempfile.gettempdir()) / f"nisse-ssh-{os.getuid()}" + sockets.mkdir(mode=0o700, exist_ok=True) + self._control = sockets / "%C" + + def describe(self) -> str: + return f"{self.user}@{self.address}" + + def _ssh(self) -> list[str]: + return [ + "ssh", + "-i", + str(self._key), + "-p", + str(self.port), + "-o", + "BatchMode=yes", + "-o", + "IdentitiesOnly=yes", + "-o", + f"UserKnownHostsFile={self._known_hosts}", + "-o", + "StrictHostKeyChecking=accept-new", + "-o", + "ControlMaster=auto", + "-o", + f"ControlPath={self._control}", + "-o", + "ControlPersist=120", + "-o", + "ConnectTimeout=15", + "-o", + "ServerAliveInterval=15", + "-o", + "LogLevel=ERROR", + f"{self.user}@{self.address}", + "bash -s", + ] + + def execute( + self, + command: str, + *, + cwd: str | None = None, + secrets: Mapping[str, str] | None = None, + stdin: str | None = None, + timeout_s: int = 120, + ) -> Result: + """Run ``command`` with bash in ``cwd``. ``secrets`` become environment + variables of this one command and are redacted from its output. + Raises HostUnavailable when the host can't be reached.""" + handed_out = dict(secrets or {}) + script = _script(command, cwd, handed_out, stdin, timeout_s) + try: + completed = subprocess.run( + self._ssh(), + input=script.encode(), + capture_output=True, + timeout=timeout_s + _KILL_GRACE_S + _LOCAL_SLACK_S, + check=False, + ) + except subprocess.TimeoutExpired as exc: + raise HostUnavailable( + f"{self.describe()} stopped responding during a command" + ) from exc + except OSError as exc: + raise HostUnavailable(f"could not run ssh: {exc}") from exc + stdout = completed.stdout.decode(errors="replace") + stderr = completed.stderr.decode(errors="replace") + if completed.returncode == SSH_FAILED: + raise HostUnavailable( + f"could not reach {self.describe()}: " + + redact(stderr.strip() or "ssh failed", handed_out.values()) + ) + values = handed_out.values() + return Result( + exit_code=completed.returncode, + stdout=redact(stdout, values), + stderr=redact(stderr, values), + timed_out=completed.returncode in (_TIMED_OUT, 137), + ) + + def close(self) -> None: + """End the shared connection, if one is open.""" + args = self._ssh()[:-2] # without the destination and the command + subprocess.run( + [*args, "-O", "exit", f"{self.user}@{self.address}"], + capture_output=True, + timeout=10, + check=False, + ) diff --git a/nisse-dev/src/nisse_dev/git.py b/nisse-dev/src/nisse_dev/git.py index 55e3baf..89986cc 100644 --- a/nisse-dev/src/nisse_dev/git.py +++ b/nisse-dev/src/nisse_dev/git.py @@ -1,94 +1,135 @@ -"""Git operations for the agent's workspace (architecture D4). +"""Git operations in the work item's workspace, on its build VM (D4, L13). -Push authentication uses the Gitea service-account token embedded in the -remote URL. The token is never written to disk in the repo config and is -scrubbed from anything this module raises or returns, so a failed push cannot -leak the credential into a transcript, a log line, or a PR comment. +Every command runs on the build VM over SSH. Remotes are plain URLs with no +credential in them, so nothing secret is ever written to the clone's +configuration (#89). Operations that reach the remote (clone, fetch, push, +ls-remote) are handed the push key for that one operation: it is fed over +stdin to a short-lived ``ssh-agent`` on the VM and gone when the command +ends. The model's commands never receive it. + +Pull requests are opened from the agent's own host with its Gitea API token +(publish.py), which never reaches the VM. """ -import subprocess +import shlex from dataclasses import dataclass from pathlib import Path +from nisse_dev.execution import HostUnavailable, SSHHost + TIMEOUT_S = 120 +# ssh for git on the VM: no prompts, and trust Gitea's host key on first use. +_GIT_SSH = "ssh -o BatchMode=yes -o StrictHostKeyChecking=accept-new" + class GitError(RuntimeError): pass +def _with_key(command: str, known_hosts: str | None) -> str: + """Run ``command`` with the key on stdin loaded into a throwaway agent.""" + git_ssh = _GIT_SSH + if known_hosts is not None: + git_ssh += f" -o UserKnownHostsFile={shlex.quote(known_hosts)}" + return ( + 'eval "$(ssh-agent -s)" > /dev/null && ' + "ssh-add -q - 2> /dev/null && " + f"GIT_SSH_COMMAND={shlex.quote(git_ssh)} {command}; " + "rc=$?; ssh-agent -k > /dev/null 2>&1; exit $rc" + ) + + @dataclass(frozen=True) class GitRepo: + host: SSHHost root: Path + # Where git's ssh on the VM records the forge's host key; the default is + # the VM user's own known_hosts. + known_hosts: str | None = None - def _run(self, *args: str, secret: str | None = None) -> str: + def _git(self, *args: str, key: str | None = None, cwd: str | None = None) -> str: + command = "git " + " ".join(shlex.quote(a) for a in args) try: - completed = subprocess.run( - ["git", *args], - cwd=self.root, - capture_output=True, - text=True, - timeout=TIMEOUT_S, - check=False, + result = self.host.execute( + _with_key(command, self.known_hosts) if key else command, + cwd=cwd or str(self.root), + stdin=key, + timeout_s=TIMEOUT_S, ) - except (OSError, subprocess.TimeoutExpired) as exc: - raise GitError(f"git {args[0]} failed: {_scrub(str(exc), secret)}") from exc - if completed.returncode != 0: + except HostUnavailable as exc: + raise GitError(f"git {args[0]} failed: {exc}") from exc + if result.timed_out: + raise GitError(f"git {args[0]} timed out after {TIMEOUT_S}s") + if result.exit_code != 0: raise GitError( f"git {args[0]} failed: " - f"{_scrub(completed.stderr.strip() or completed.stdout.strip(), secret)}" + f"{result.stderr.strip() or result.stdout.strip()}" ) - return completed.stdout.strip() + return result.stdout.strip() # -- inspection ------------------------------------------------------- def current_branch(self) -> str: # --show-current rather than rev-parse: it answers correctly on a # freshly initialised repo, where HEAD points at an unborn branch. - return self._run("branch", "--show-current") + return self._git("branch", "--show-current") def head_sha(self) -> str: - return self._run("rev-parse", "HEAD") + return self._git("rev-parse", "HEAD") def has_changes(self) -> bool: - return bool(self._run("status", "--porcelain")) + return bool(self._git("status", "--porcelain")) def commits_since(self, ref: str) -> int: """How many commits HEAD has that ref lacks. The executor may commit its own work, so a clean tree does not mean nothing to publish.""" - return int(self._run("rev-list", "--count", f"{ref}..HEAD")) + return int(self._git("rev-list", "--count", f"{ref}..HEAD")) def remote_sha( - self, remote_url: str, branch: str, *, token: str | None = None + self, remote_url: str, branch: str, *, key: str | None = None ) -> str | None: """What the remote's branch points at, or None if it has no such branch. Pushes go by URL, so there is no tracking ref to read.""" - out = self._run("ls-remote", remote_url, f"refs/heads/{branch}", secret=token) + out = self._git("ls-remote", remote_url, f"refs/heads/{branch}", key=key) return out.split()[0] if out else None + def exists(self) -> bool: + """Whether the workspace is already a git repository.""" + try: + result = self.host.execute( + f"test -d {shlex.quote(str(self.root / '.git'))}", timeout_s=TIMEOUT_S + ) + except HostUnavailable as exc: + raise GitError(f"could not check the workspace: {exc}") from exc + return result.exit_code == 0 + # -- mutation --------------------------------------------------------- def init(self, *, default_branch: str = "main") -> None: - self.root.mkdir(parents=True, exist_ok=True) - self._run("init", "-q", "-b", default_branch) + self.host.execute(f"mkdir -p -- {shlex.quote(str(self.root))}") + self._git("init", "-q", "-b", default_branch) - def clone(self, remote_url: str, dest: Path, *, token: str | None = None) -> None: - self.root.mkdir(parents=True, exist_ok=True) - self._run("clone", "-q", remote_url, str(dest), secret=token) + def clone(self, remote_url: str, *, key: str | None = None) -> None: + """Clone into the workspace root. The remote URL carries no + credential, so none is stored in the clone's configuration.""" + parent = str(self.root.parent) + self.host.execute(f"mkdir -p -- {shlex.quote(parent)}") + self._git("clone", "-q", remote_url, str(self.root), key=key, cwd=parent) def configure_identity(self, name: str, email: str) -> None: - self._run("config", "user.name", name) - self._run("config", "user.email", email) + self._git("config", "user.name", name) + self._git("config", "user.email", email) def checkout_new_branch(self, name: str) -> None: - self._run("checkout", "-q", "-b", name) + self._git("checkout", "-q", "-b", name) def checkout_existing_branch(self, name: str) -> None: - self._run("checkout", "-q", name) + self._git("checkout", "-q", name) def branch_exists(self, name: str) -> bool: try: - self._run("rev-parse", "--verify", "--quiet", f"refs/heads/{name}") + self._git("rev-parse", "--verify", "--quiet", f"refs/heads/{name}") except GitError: return False return True @@ -98,14 +139,14 @@ class GitRepo: commit — an empty milestone is a fact to report, not an error.""" if not self.has_changes(): return None - self._run("add", "-A") - self._run("commit", "-q", "-m", message) + self._git("add", "-A") + self._git("commit", "-q", "-m", message) return self.head_sha() - def push(self, remote_url: str, branch: str, *, token: str | None = None) -> None: - # The URL is passed per-invocation rather than stored as a remote, so - # the token never lands in .git/config. - self._run("push", "-q", remote_url, f"HEAD:refs/heads/{branch}", secret=token) + def push(self, remote_url: str, branch: str, *, key: str | None = None) -> None: + # By URL, per invocation, rather than a configured remote with a + # credential in it. + self._git("push", "-q", remote_url, f"HEAD:refs/heads/{branch}", key=key) def discard_branch(self, branch: str, *, back_to: str) -> None: """Throw away a branch and everything on it. @@ -114,20 +155,10 @@ class GitRepo: atomicity that reconciliation gives an interrupted transition, and half-finished work must not survive to be built upon. """ - self._run("checkout", "-q", "--force", back_to) - self._run("branch", "-q", "-D", branch) + self._git("checkout", "-q", "--force", back_to) + self._git("branch", "-q", "-D", branch) def delete_remote_branch( - self, remote_url: str, branch: str, *, token: str | None = None + self, remote_url: str, branch: str, *, key: str | None = None ) -> None: - self._run("push", "-q", remote_url, f":refs/heads/{branch}", secret=token) - - -def authenticated_url(base_url: str, repo: str, user: str, token: str) -> str: - host = base_url.split("://", 1)[-1].rstrip("/") - scheme = base_url.split("://", 1)[0] if "://" in base_url else "https" - return f"{scheme}://{user}:{token}@{host}/{repo}.git" - - -def _scrub(text: str, secret: str | None) -> str: - return text.replace(secret, "***") if secret else text + self._git("push", "-q", remote_url, f":refs/heads/{branch}", key=key) diff --git a/nisse-dev/src/nisse_dev/gitea.py b/nisse-dev/src/nisse_dev/gitea.py index 386123e..25a9214 100644 --- a/nisse-dev/src/nisse_dev/gitea.py +++ b/nisse-dev/src/nisse_dev/gitea.py @@ -70,6 +70,10 @@ class GiteaClient: ) return response.json() if response.content else None + def ssh_url(self, repo: str) -> str: + """The repository's SSH clone URL, which carries no credential.""" + return str(self._request("GET", f"/repos/{repo}")["ssh_url"]) + def whoami(self) -> str: return str(self._request("GET", "/user")["login"]) diff --git a/nisse-dev/src/nisse_dev/publish.py b/nisse-dev/src/nisse_dev/publish.py index 189dd21..f925ffe 100644 --- a/nisse-dev/src/nisse_dev/publish.py +++ b/nisse-dev/src/nisse_dev/publish.py @@ -9,7 +9,7 @@ an idempotent transition reconciled against external truth. from dataclasses import dataclass -from nisse_dev.git import GitRepo, authenticated_url +from nisse_dev.git import GitRepo from nisse_dev.gitea import ( GiteaClient, GiteaError, @@ -36,9 +36,8 @@ class MilestonePublisher: repo: str, git_repo: GitRepo, *, - base_url: str, - account: str, - token: str, + remote_url: str, + push_key: str | None = None, base_branch: str = "main", ) -> None: self._store = store @@ -46,8 +45,10 @@ class MilestonePublisher: self._repo = repo self._git = git_repo self._base_branch = base_branch - self._remote = authenticated_url(base_url, repo, account, token) - self._token = token + # A URL with no credential in it; the push key is handed to each push + # on the build VM and nothing else (D4, L12). + self._remote = remote_url + self._key = push_key def reconcilers(self) -> dict[str, Reconciler]: return {OPEN_PR_INTENT: pull_request_reconciler(self._client, self._repo)} @@ -84,7 +85,7 @@ class MilestonePublisher: cause=f"publishing milestone {milestone}", work_item=task_id, # owned by the work item it publishes (L1) ) - self._git.push(self._remote, branch, token=self._token) + self._git.push(self._remote, branch, key=self._key) # Idempotent: if a PR for this branch is already open (a crash after # opening it, or a response lost on the way back), the push above # has updated it; adopt it rather than fail on a second. @@ -149,10 +150,10 @@ class MilestonePublisher: if sha is None: # The executor may have committed the fix itself. sha = self._git.head_sha() - pushed = self._git.remote_sha(self._remote, branch, token=self._token) + pushed = self._git.remote_sha(self._remote, branch, key=self._key) if sha == pushed: return None - self._git.push(self._remote, branch, token=self._token) + self._git.push(self._remote, branch, key=self._key) return sha diff --git a/nisse-dev/src/nisse_dev/testing.py b/nisse-dev/src/nisse_dev/testing.py new file mode 100644 index 0000000..95f70d5 --- /dev/null +++ b/nisse-dev/src/nisse_dev/testing.py @@ -0,0 +1,101 @@ +"""Test support: a throwaway sshd standing in for a tool VM (L13). + +Runs the system's sshd as the current user, on a free local port, with a +host key and client key made for the occasion. Tests then reach "the build +VM" over real SSH, so the code path under test is the production path. +""" + +import getpass +import os +import socket +import subprocess +import time +from collections.abc import Iterator +from contextlib import contextmanager +from pathlib import Path + +from nisse_dev.execution import SSHHost + +SSHD = "/usr/sbin/sshd" + + +def _free_port() -> int: + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + return int(sock.getsockname()[1]) + + +def _keygen(path: Path) -> None: + subprocess.run( + ["ssh-keygen", "-q", "-t", "ed25519", "-N", "", "-f", str(path)], check=True + ) + + +@contextmanager +def local_sshd(root: Path) -> Iterator[SSHHost]: + """An SSHHost for a private sshd on 127.0.0.1, torn down afterwards.""" + root.mkdir(parents=True, exist_ok=True) + host_key, client_key = root / "host_key", root / "client_key" + _keygen(host_key) + _keygen(client_key) + authorized = root / "authorized_keys" + authorized.write_text(client_key.with_suffix(".pub").read_text()) + authorized.chmod(0o600) + port = _free_port() + config = root / "sshd_config" + config.write_text( + "\n".join( + [ + f"Port {port}", + "ListenAddress 127.0.0.1", + f"HostKey {host_key}", + f"AuthorizedKeysFile {authorized}", + f"PidFile {root / 'sshd.pid'}", + "PasswordAuthentication no", + "KbdInteractiveAuthentication no", + "PubkeyAuthentication yes", + "PermitRootLogin prohibit-password", + "StrictModes no", + "UsePAM no", + "MaxSessions 50", + "MaxStartups 50", + # Stands in for a provisioned build VM: sessions see the test + # runner's PATH, so its Python and tools are "installed". + f"SetEnv PATH={os.environ.get('PATH', '/usr/bin:/bin')}", + "", + ] + ) + ) + process = subprocess.Popen( + [SSHD, "-D", "-e", "-f", str(config)], + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + ) + user = getpass.getuser() + host = SSHHost( + "127.0.0.1", key=client_key, state_dir=root / "client", user=user, port=port + ) + try: + deadline = time.monotonic() + 10 + while True: + if process.poll() is not None: + raise RuntimeError( + "sshd exited: " + (process.stderr.read() or b"").decode() + if process.stderr + else "sshd exited" + ) + try: + with socket.create_connection(("127.0.0.1", port), timeout=0.5): + break + except OSError: + if time.monotonic() > deadline: + raise RuntimeError("sshd did not start") from None + time.sleep(0.05) + yield host + finally: + host.close() + process.terminate() + try: + process.wait(timeout=5) + except subprocess.TimeoutExpired: + process.kill() diff --git a/nisse-dev/src/nisse_dev/tools.py b/nisse-dev/src/nisse_dev/tools.py index a1e40c4..1ae4686 100644 --- a/nisse-dev/src/nisse_dev/tools.py +++ b/nisse-dev/src/nisse_dev/tools.py @@ -1,26 +1,27 @@ -"""Executor tools (architecture L9: reduced schema, flat arguments). +"""Executor tools (architecture L9: reduced schema, flat arguments; L13). -Deliberately unsandboxed. The VM is the trust boundary (D7) and tool use is -auto-approved inside an approved unit of work (L3), so per-command -confinement would add failure modes without adding safety. The workspace is -where work happens — the default for relative paths and the command cwd — not -a wall. Protecting agent state from the agent is a mount-layout concern -(L8/D7), not this module's job. +Every tool runs on the work item's build VM over SSH (execution.py); the +agent's own host never runs a command a model chose. Within the VM nothing is +sandboxed: the VM is the trust boundary (D7), so per-command confinement would +add failure modes without adding safety. The workspace is where work happens, +the default for relative paths and the command's directory, not a wall. -What *is* enforced here is operational: commands cannot hang the loop, cannot -consume the context window with megabytes of build log, and do not inherit -the agent's own credentials. +What *is* enforced here is operational: commands cannot hang the loop and +cannot flood the context window with megabytes of build log. The agent's own +credentials are never on the build VM to inherit. """ -import os -import shutil -import signal -import subprocess +import base64 +import io +import shlex +import tarfile +from collections.abc import Mapping from dataclasses import dataclass from pathlib import Path from typing import Any from nisse_dev.context import head_and_tail +from nisse_dev.execution import HostUnavailable, Result, SSHHost # Tools that change or verify something. Reading a file or listing a # directory is a model looking around, not doing the work, so neither counts @@ -88,12 +89,6 @@ EXECUTOR_TOOLS: list[dict[str, Any]] = [ ] -def _child_env() -> dict[str, str]: - """The project's build and test code is arbitrary; it has no business - reading the agent's LiteLLM, Matrix, or Gitea credentials.""" - return {k: v for k, v in os.environ.items() if not k.startswith("NISSE_")} - - @dataclass(frozen=True) class ToolOutcome: result: str @@ -101,30 +96,53 @@ class ToolOutcome: class Workspace: + """The work item's workspace: a directory on its tool VM (L13, D3). + + Every operation runs there over SSH; nothing touches the agent's own + host. ``root`` is the path on that host. + """ + def __init__( self, - root: Path, + host: SSHHost, + root: Path | str, *, timeout_s: int = DEFAULT_TIMEOUT_S, max_output_chars: int = MAX_OUTPUT_CHARS, + secrets: Mapping[str, str] | None = None, ) -> None: + self.host = host self.root = Path(root) - self.root.mkdir(parents=True, exist_ok=True) self._timeout_s = timeout_s self._max_output_chars = max_output_chars + # The project secrets the model's commands may see (D7); redacted from + # whatever comes back. + self._secrets = dict(secrets or {}) + self._run(f"mkdir -p -- {shlex.quote(str(self.root))}") + + def _run( + self, command: str, *, stdin: str | None = None, cwd: str | None = None + ) -> Result: + return self.host.execute( + command, cwd=cwd, stdin=stdin, timeout_s=self._timeout_s + ) def _truncate(self, text: str) -> str: return head_and_tail(text, self._max_output_chars) - def _resolve(self, path: str) -> Path: + def _resolve(self, path: str) -> str: candidate = Path(path) - return candidate if candidate.is_absolute() else self.root / candidate + return str(candidate if candidate.is_absolute() else self.root / candidate) - def _inside(self, target: Path) -> bool: - try: - return target.resolve().is_relative_to(self.root.resolve()) - except OSError: + def _inside(self, target: str) -> bool: + result = self._run( + f"realpath -m -- {shlex.quote(target)} {shlex.quote(str(self.root))}" + ) + lines = result.stdout.splitlines() + if not result.ok or len(lines) != 2: return False + resolved, root = (Path(line) for line in lines) + return resolved == root or root in resolved.parents def dispatch(self, tool: str, arguments: dict[str, Any]) -> ToolOutcome: try: @@ -143,13 +161,24 @@ class Workspace: return ToolOutcome(f"no such tool: {tool}", success=False) except KeyError as exc: return ToolOutcome(f"missing argument {exc} for {tool}", success=False) + except HostUnavailable as exc: + # Not the work failing: the build VM is out of reach. + return ToolOutcome(f"the build VM is unreachable: {exc}", success=False) def read_file(self, path: str) -> ToolOutcome: target = self._resolve(path) + # base64 on the wire, so the bytes arrive intact and decoding is + # strict here, as it was for a local file. + result = self._run(f"base64 -w0 -- {shlex.quote(target)}") + if not result.ok: + return ToolOutcome( + f"could not read {path}: {result.stderr.strip()}", success=False + ) try: - return ToolOutcome(self._truncate(target.read_text()), success=True) - except (OSError, UnicodeDecodeError) as exc: + text = base64.b64decode(result.stdout).decode() + except (ValueError, UnicodeDecodeError) as exc: return ToolOutcome(f"could not read {path}: {exc}", success=False) + return ToolOutcome(self._truncate(text), success=True) def write_file(self, path: str, content: str) -> ToolOutcome: target = self._resolve(path) @@ -165,21 +194,25 @@ class Workspace: "relative paths.", success=False, ) - try: - target.parent.mkdir(parents=True, exist_ok=True) - target.write_text(content) - except OSError as exc: - return ToolOutcome(f"could not write {path}: {exc}", success=False) + quoted = shlex.quote(target) + result = self._run( + f'mkdir -p -- "$(dirname -- {quoted})" && base64 -d > {quoted}', + stdin=base64.b64encode(content.encode()).decode(), + ) + if not result.ok: + return ToolOutcome( + f"could not write {path}: {result.stderr.strip()}", success=False + ) return ToolOutcome(f"wrote {len(content)} characters to {path}", success=True) def list_dir(self, path: str) -> ToolOutcome: target = self._resolve(path) - try: - entries = sorted( - f"{p.name}/" if p.is_dir() else p.name for p in target.iterdir() + result = self._run(f"ls -1Ap -- {shlex.quote(target)}") + if not result.ok: + return ToolOutcome( + f"could not list {path}: {result.stderr.strip()}", success=False ) - except OSError as exc: - return ToolOutcome(f"could not list {path}: {exc}", success=False) + entries = sorted(line for line in result.stdout.splitlines() if line) return ToolOutcome( self._truncate("\n".join(entries) or "(empty)"), success=True ) @@ -187,56 +220,48 @@ class Workspace: def describe(self) -> str: """Told to the model, so it does not have to guess where it is.""" return ( - f"Your workspace is {self.root}. It is a git repository and the " - "only place your work is recorded; commands run there by default. " - "Use relative paths." + f"Your workspace is {self.root} on the build VM. It is a git " + "repository and the only place your work is recorded; commands run " + "there by default. Use relative paths." ) def run_command(self, command: str) -> ToolOutcome: try: - # Popen rather than run(): on timeout, run() kills only the direct - # child, and TimeoutExpired carries no pid, so a backgrounded - # grandchild (a dev server, say) would survive. Holding the pid - # lets us kill the whole session. - process = subprocess.Popen( + result = self.host.execute( command, - shell=True, - cwd=self.root, - env=_child_env(), - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - stdin=subprocess.DEVNULL, - text=True, - start_new_session=True, + cwd=str(self.root), + secrets=self._secrets, + timeout_s=self._timeout_s, ) - except OSError as exc: - return ToolOutcome(f"could not run command: {exc}", success=False) - try: - stdout, stderr = process.communicate(timeout=self._timeout_s) - except subprocess.TimeoutExpired: - self._kill_group(process) + except HostUnavailable as exc: + return ToolOutcome(f"the build VM is unreachable: {exc}", success=False) + if result.timed_out: return ToolOutcome( f"command timed out after {self._timeout_s}s: {command}", success=False, ) body = self._truncate( - f"exit code: {process.returncode}\nstdout:\n{stdout}\nstderr:\n{stderr}" + f"exit code: {result.exit_code}\nstdout:\n{result.stdout}" + f"\nstderr:\n{result.stderr}" ) - return ToolOutcome(body, success=process.returncode == 0) + return ToolOutcome(body, success=result.exit_code == 0) - @staticmethod - def _kill_group(process: "subprocess.Popen[str]") -> None: - try: - os.killpg(os.getpgid(process.pid), signal.SIGKILL) - except (ProcessLookupError, PermissionError): - process.kill() - # Drain the pipes of the now-dead group so no zombie is left behind. - try: - process.communicate(timeout=5) - except subprocess.TimeoutExpired: - pass + def upload_tree(self, local: Path, dest: str = ".") -> None: + """Copy a local directory's contents into the workspace (the bench + seeding a task); raises RuntimeError if it fails.""" + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as archive: + for entry in sorted(Path(local).iterdir()): + archive.add(entry, arcname=entry.name) + target = shlex.quote(self._resolve(dest)) + result = self._run( + f"mkdir -p -- {target} && base64 -d | tar -xzf - -C {target}", + stdin=base64.b64encode(buffer.getvalue()).decode(), + ) + if not result.ok: + raise RuntimeError(f"upload to {dest} failed: {result.stderr.strip()}") def reset(self) -> None: """Discard workspace contents; used by tests, not by the agent.""" - shutil.rmtree(self.root, ignore_errors=True) - self.root.mkdir(parents=True, exist_ok=True) + root = shlex.quote(str(self.root)) + self._run(f"rm -rf -- {root} && mkdir -p -- {root}") diff --git a/nisse-dev/tests/test_agent_build.py b/nisse-dev/tests/test_agent_build.py index c43ae32..b313a60 100644 --- a/nisse-dev/tests/test_agent_build.py +++ b/nisse-dev/tests/test_agent_build.py @@ -3,6 +3,7 @@ delegation harness and the comment poller — were fully built, fully tested, and reachable from nothing but their own tests. Unit tests that construct a component directly cannot catch that; only assembly can.""" +import subprocess from pathlib import Path from typing import Any @@ -13,7 +14,7 @@ from test_config import SECRETS, tree from nisse_dev.agent import build_agent, discover_models, prepare_workspace from nisse_dev.catalog import ModelCatalog, UnknownModel from nisse_dev.config import AgentConfig -from nisse_dev.git import GitRepo +from nisse_dev.execution import SSHHost from nisse_dev.oplog import OpLog # What the test roles resolve to, as LiteLLM would report them. @@ -36,47 +37,77 @@ TEST_CATALOG = ModelCatalog.from_payload( ) +# The build VM for every test here: the session's throwaway sshd. +HOST: SSHHost + + +@pytest.fixture(autouse=True) +def _build_host(ssh_host: SSHHost) -> None: + global HOST + HOST = ssh_host + + def config_for(tmp_path: Path, **changes: str) -> AgentConfig: return AgentConfig.from_tree( tree( state_db=str(tmp_path / "state.db"), workspace=str(tmp_path / "work"), + build_host__address=HOST.address, + build_host__user=HOST.user, + build_host__port=str(HOST.port), + build_host__key=str(HOST._key), **changes, ), SECRETS, ) +def git(*args: str) -> None: + subprocess.run(["git", *args], check=True, capture_output=True) + + @pytest.fixture def origin(tmp_path: Path) -> Path: """A bare repo standing in for the project on the forge.""" remote = tmp_path / "origin.git" - GitRepo(tmp_path)._run("init", "-q", "--bare", "-b", "main", str(remote)) - seed = GitRepo(tmp_path / "seed") - seed.init() - seed.configure_identity("seed", "seed@test") - (seed.root / "README.md").write_text("project\n") - seed.commit_all("seed") - seed.push(str(remote), "main") + git("init", "-q", "--bare", "-b", "main", str(remote)) + seed = tmp_path / "seed" + git("init", "-q", "-b", "main", str(seed)) + (seed / "README.md").write_text("project\n") + git("-C", str(seed), "add", "-A") + git( + "-C", + str(seed), + "-c", + "user.name=s", + "-c", + "user.email=s@t", + "commit", + "-qm", + "s", + ) + git("-C", str(seed), "push", "-q", str(remote), "HEAD:refs/heads/main") return remote -def test_workspace_is_cloned_when_absent(tmp_path: Path, origin: Path) -> None: - config = config_for(tmp_path, gitea__url=f"file://{origin}") - # authenticated_url would mangle a file:// remote, so clone directly. - GitRepo(tmp_path).clone(str(origin), config.workspace) - repo = prepare_workspace(config, "nisse-dev-testing") +def test_workspace_is_cloned_on_the_build_host_when_absent( + tmp_path: Path, origin: Path +) -> None: + config = config_for(tmp_path) + repo = prepare_workspace(HOST, config, "nisse-dev-testing", str(origin), None) assert (config.workspace / "README.md").exists() assert repo.current_branch() == "main" + # The clone's configuration holds the plain URL it was given (#89). + assert str(origin) in (config.workspace / ".git" / "config").read_text() def test_workspace_is_reused_when_already_cloned(tmp_path: Path, origin: Path) -> None: config = config_for(tmp_path) - GitRepo(tmp_path).clone(str(origin), config.workspace) + prepare_workspace(HOST, config, "nisse-dev-testing", str(origin), None) (config.workspace / "local-work.txt").write_text("in progress") - prepare_workspace(config, "nisse-dev-testing") + prepare_workspace(HOST, config, "nisse-dev-testing", str(origin), None) # Re-cloning would silently destroy uncommitted work. assert (config.workspace / "local-work.txt").exists() @@ -110,7 +141,6 @@ class FakeTransport: def build_offline(tmp_path: Path, origin: Path, monkeypatch: Any) -> Any: """Assemble the real graph with only the network boundaries faked.""" config = config_for(tmp_path) - GitRepo(tmp_path).clone(str(origin), config.workspace) monkeypatch.setattr( "nisse_dev.agent.GiteaClient.whoami", lambda self: "nisse-dev-testing" @@ -121,8 +151,7 @@ def build_offline(tmp_path: Path, origin: Path, monkeypatch: Any) -> Any: ) monkeypatch.setattr("nisse_dev.agent.MatrixTransport", lambda cfg: FakeTransport()) monkeypatch.setattr( - "nisse_dev.agent.authenticated_url", - lambda base, repo, user, token: str(origin), + "nisse_dev.agent.GiteaClient.ssh_url", lambda self, repo: str(origin) ) return build_agent(config) @@ -317,7 +346,6 @@ def test_tuning_reaches_every_component( tools=ToolsTuning(command_timeout_s=300, max_output_chars=2000), ), ) - GitRepo(tmp_path).clone(str(origin), config.workspace) monkeypatch.setattr( "nisse_dev.agent.GiteaClient.whoami", lambda self: "nisse-dev-testing" ) @@ -327,8 +355,7 @@ def test_tuning_reaches_every_component( ) monkeypatch.setattr("nisse_dev.agent.MatrixTransport", lambda cfg: FakeTransport()) monkeypatch.setattr( - "nisse_dev.agent.authenticated_url", - lambda base, repo, user, token: str(origin), + "nisse_dev.agent.GiteaClient.ssh_url", lambda self, repo: str(origin) ) agent = build_agent(config) executor = agent.orchestrator._executor diff --git a/nisse-dev/tests/test_bench.py b/nisse-dev/tests/test_bench.py index a7c98e2..5efbe53 100644 --- a/nisse-dev/tests/test_bench.py +++ b/nisse-dev/tests/test_bench.py @@ -20,6 +20,17 @@ from nisse_dev.bench.tasks import CHECK_DIR, load_tasks from nisse_dev.canonical import AssistantText, ToolCall from nisse_dev.catalog import ModelFacts from nisse_dev.config import ExecutorTuning, Tuning +from nisse_dev.execution import SSHHost + +# The build VM for every test here: the session's throwaway sshd. +HOST: SSHHost + + +@pytest.fixture(autouse=True) +def _build_host(ssh_host: SSHHost) -> None: + global HOST + HOST = ssh_host + TASKS = load_tasks() FACTS = ModelFacts( @@ -187,7 +198,9 @@ SOLVE_SLUGIFY = [ def test_a_solved_task_is_a_success(tmp_path: Path) -> None: [task] = load_tasks(["write-function"]) - record = run_task(task, FACTS, Scripted(list(SOLVE_SLUGIFY)), Tuning(), note="n") + record = run_task( + task, FACTS, Scripted(list(SOLVE_SLUGIFY)), Tuning(), host=HOST, note="n" + ) assert record.success assert record.outcome == "completed" @@ -212,7 +225,7 @@ def test_saying_finished_is_not_success(tmp_path: Path) -> None: ], [call("finish", "f", summary="done")], ] - record = run_task(task, FACTS, Scripted(wrong), Tuning()) + record = run_task(task, FACTS, Scripted(wrong), Tuning(), host=HOST) assert record.outcome == "completed" assert not record.success @@ -227,7 +240,7 @@ def test_misbehaviour_is_counted(tmp_path: Path) -> None: # A higher failure limit, so the invented tool and the failed command # don't escalate before the prose turn is reached. tuning = Tuning(executor=ExecutorTuning(failure_limit=5)) - record = run_task(task, FACTS, Scripted(turns), tuning) + record = run_task(task, FACTS, Scripted(turns), tuning, host=HOST) assert record.invented_tools == 1 assert record.failed_commands == 1 assert record.prose_turns >= 1 @@ -237,7 +250,7 @@ def test_misbehaviour_is_counted(tmp_path: Path) -> None: def test_a_fallback_answer_can_never_be_a_success(tmp_path: Path) -> None: [task] = load_tasks(["write-function"]) client = Scripted(list(SOLVE_SLUGIFY), answered="claude-sonnet-5") - record = run_task(task, FACTS, client, Tuning()) + record = run_task(task, FACTS, client, Tuning(), host=HOST) assert record.fallbacks == ["claude-sonnet-5"] * 3 assert not record.success # the check passed, but not for this model @@ -255,7 +268,9 @@ def test_wilson_intervals_behave() -> None: def record_json(**overrides: Any) -> dict[str, Any]: [task] = load_tasks(["write-function"]) - base = run_task(task, FACTS, Scripted(list(SOLVE_SLUGIFY)), Tuning()).to_json() + base = run_task( + task, FACTS, Scripted(list(SOLVE_SLUGIFY)), Tuning(), host=HOST + ).to_json() base.update(overrides) return base @@ -338,7 +353,7 @@ def test_a_run_records_its_rate_limit_waits(tmp_path: Path) -> None: [task] = load_tasks(["write-function"]) client = Scripted(list(SOLVE_SLUGIFY)) client.last_rate_limit_wait_s = 20.0 # every call waited once - record = run_task(task, FACTS, client, Tuning()) + record = run_task(task, FACTS, client, Tuning(), host=HOST) assert record.rate_limit_wait_s == 60.0 diff --git a/nisse-dev/tests/test_config.py b/nisse-dev/tests/test_config.py index f17198e..8c809c3 100644 --- a/nisse-dev/tests/test_config.py +++ b/nisse-dev/tests/test_config.py @@ -32,6 +32,7 @@ COMPLETE: dict[str, Any] = { "repo": "don/demo", "operators": ["don"], }, + "build_host": {"address": "build.test", "key": "~/.ssh/nisse-dev"}, } SECRETS = { "TEST_LLM_TOKEN": "sk-x", @@ -223,6 +224,9 @@ gitea: token: ${TEST_GITEA_TOKEN} repo: don/demo operators: [don] +build_host: + address: build.test + key: ~/.ssh/nisse-dev """ @@ -328,3 +332,22 @@ def test_context_tuning_is_read_and_checked() -> None: assert build().tuning.context.keep_recent_turns == 6 with pytest.raises(ConfigError, match="safety_margin: must be less than 1"): build(tuning={"context": {"safety_margin": 1}}) + + +def test_the_build_host_is_required_and_defaults_to_root_on_22() -> None: + config = build() + assert (config.build_host.address, config.build_host.user) == ("build.test", "root") + assert config.build_host.port == 22 + assert config.build_host.key.is_absolute() + with pytest.raises(ConfigError, match="build_host.address"): + build(build_host__address=None) + + +def test_a_bad_build_host_port_is_reported() -> None: + with pytest.raises(ConfigError, match="build_host.port"): + build(build_host__port="ssh") + + +def test_the_push_key_is_optional_and_a_path() -> None: + assert build().push_key is None + assert build(gitea__push_key="~/k").push_key == Path("~/k").expanduser() diff --git a/nisse-dev/tests/test_delegation.py b/nisse-dev/tests/test_delegation.py index 084aa52..acac56a 100644 --- a/nisse-dev/tests/test_delegation.py +++ b/nisse-dev/tests/test_delegation.py @@ -22,6 +22,7 @@ from nisse_dev.delegation import ( DelegationResult, delegation_reconciler, ) +from nisse_dev.execution import SSHHost from nisse_dev.git import GitRepo from nisse_dev.llm import BudgetExhausted, LLMError, UpstreamUnavailable from nisse_dev.oplog import OpLog @@ -73,8 +74,8 @@ def store(tmp_path: Path) -> StateStore: @pytest.fixture -def repo(tmp_path: Path) -> GitRepo: - r = GitRepo(tmp_path / "work") +def repo(ssh_host: SSHHost, tmp_path: Path) -> GitRepo: + r = GitRepo(ssh_host, tmp_path / "work") r.init() r.configure_identity("bot", "bot@example.test") (r.root / "seed.txt").write_text("seed") @@ -84,7 +85,7 @@ def repo(tmp_path: Path) -> GitRepo: @pytest.fixture def ws(repo: GitRepo) -> Workspace: - return Workspace(repo.root, timeout_s=10) + return Workspace(repo.host, repo.root, timeout_s=10) ITEM = "t" # the work item these delegations run in diff --git a/nisse-dev/tests/test_execution.py b/nisse-dev/tests/test_execution.py new file mode 100644 index 0000000..58dfbbd --- /dev/null +++ b/nisse-dev/tests/test_execution.py @@ -0,0 +1,77 @@ +"""Running commands on a tool VM over SSH (L13), against a throwaway sshd.""" + +import os +from pathlib import Path + +import pytest + +from nisse_dev.execution import REDACTED, HostUnavailable, SSHHost, redact + + +def test_exit_codes_and_streams_come_back(ssh_host: SSHHost) -> None: + result = ssh_host.execute("echo out; echo err >&2; exit 3") + assert (result.exit_code, result.stdout, result.stderr) == (3, "out\n", "err\n") + assert not result.ok + + +def test_cwd_and_stdin(ssh_host: SSHHost, tmp_path: Path) -> None: + result = ssh_host.execute("pwd; cat", cwd=str(tmp_path), stdin="line one\nline two") + assert result.stdout == f"{tmp_path}\nline one\nline two\n" + + +def test_stdin_that_looks_like_a_delimiter_survives(ssh_host: SSHHost) -> None: + payload = "NISSE_EOF\nEOF\n'quoted' $notexpanded `nor this`" + assert ssh_host.execute("cat", stdin=payload).stdout == payload + "\n" + + +def test_without_stdin_a_command_cannot_read_the_script(ssh_host: SSHHost) -> None: + # The script arrives on the same channel; the command must see /dev/null. + result = ssh_host.execute("cat; echo done") + assert result.stdout == "done\n" + + +def test_a_missing_cwd_fails_before_running(ssh_host: SSHHost, tmp_path: Path) -> None: + marker = tmp_path / "ran" + result = ssh_host.execute(f"touch {marker}", cwd=str(tmp_path / "absent")) + assert result.exit_code == 125 + assert not marker.exists() + + +def test_a_command_exiting_255_is_not_mistaken_for_ssh(ssh_host: SSHHost) -> None: + assert ssh_host.execute("exit 255").exit_code == 254 + + +def test_an_unreachable_host_raises(tmp_path: Path) -> None: + host = SSHHost("127.0.0.1", key=tmp_path / "k", state_dir=tmp_path / "s", port=1) + with pytest.raises(HostUnavailable): + host.execute("true") + + +def test_a_timeout_is_reported(ssh_host: SSHHost) -> None: + result = ssh_host.execute("sleep 30", timeout_s=1) + assert result.timed_out + + +def test_secrets_reach_the_command_and_never_its_output(ssh_host: SSHHost) -> None: + value = "s-" + os.urandom(12).hex() + result = ssh_host.execute( + 'echo "$API_KEY"; echo "$API_KEY" >&2', secrets={"API_KEY": value} + ) + assert result.ok + assert value not in result.stdout + result.stderr + assert result.stdout == f"{REDACTED}\n" + + +def test_secrets_are_never_on_a_command_line(ssh_host: SSHHost, tmp_path: Path) -> None: + # Made up here, so no command line in this test run can contain it except + # by leaking it. + value = "s-" + os.urandom(12).hex() + seen = tmp_path / "ps.txt" + ssh_host.execute(f"ps -eo args > {seen}", secrets={"API_KEY": value}) + assert "bash -s" in seen.read_text() # the listing did include our shell + assert value not in seen.read_text() + + +def test_redact_masks_longest_values_first() -> None: + assert redact("abc-abcdef", ["abcdef", "abc-abcdef"]) == REDACTED + assert redact("short ab stays", ["ab"]) == "short ab stays" # too short to mask diff --git a/nisse-dev/tests/test_executor.py b/nisse-dev/tests/test_executor.py index 72c22aa..fd96529 100644 --- a/nisse-dev/tests/test_executor.py +++ b/nisse-dev/tests/test_executor.py @@ -11,6 +11,7 @@ from nisse_dev.canonical import ( ToolCall, ToolResult, ) +from nisse_dev.execution import SSHHost from nisse_dev.executor import LLMExecutor from nisse_dev.llm import LLMError, RoleMap from nisse_dev.oplog import OpLog @@ -62,8 +63,8 @@ def store(tmp_path: Path) -> StateStore: @pytest.fixture -def ws(tmp_path: Path) -> Workspace: - return Workspace(tmp_path / "work", timeout_s=10) +def ws(ssh_host: SSHHost, tmp_path: Path) -> Workspace: + return Workspace(ssh_host, tmp_path / "work", timeout_s=10) ITEM = "t" # the work item these tests run in diff --git a/nisse-dev/tests/test_git.py b/nisse-dev/tests/test_git.py index 30f2d6f..c3f2c34 100644 --- a/nisse-dev/tests/test_git.py +++ b/nisse-dev/tests/test_git.py @@ -1,18 +1,28 @@ +"""Git on the build VM (D4, L13): here, a throwaway sshd on this machine.""" + +import shlex +import subprocess from pathlib import Path import pytest -from nisse_dev.git import GitError, GitRepo, _scrub, authenticated_url +from nisse_dev.execution import SSHHost +from nisse_dev.git import GitError, GitRepo @pytest.fixture -def repo(tmp_path: Path) -> GitRepo: - r = GitRepo(tmp_path / "work") +def repo(ssh_host: SSHHost, tmp_path: Path) -> GitRepo: + r = GitRepo(ssh_host, tmp_path / "work") r.init() r.configure_identity("nisse-dev-testing", "nisse@example.test") return r +def bare_remote(path: Path) -> str: + subprocess.run(["git", "init", "-q", "--bare", "-b", "main", str(path)], check=True) + return str(path) + + def test_init_creates_repo_on_default_branch(repo: GitRepo) -> None: assert repo.current_branch() == "main" @@ -40,31 +50,8 @@ def test_has_changes_tracks_the_worktree(repo: GitRepo) -> None: assert repo.has_changes() -def test_authenticated_url_shape() -> None: - url = authenticated_url("https://gitea.test/", "o/r", "bot", "secret") - assert url == "https://bot:secret@gitea.test/o/r.git" - - -def test_push_failure_does_not_leak_the_token(repo: GitRepo) -> None: - (repo.root / "a.txt").write_text("a") - repo.commit_all("first") - url = authenticated_url("https://127.0.0.1:1", "o/r", "bot", "sup3rsecret") - with pytest.raises(GitError) as excinfo: - repo.push(url, "main", token="sup3rsecret") - # git redacts credentials from its own messages, but that is git's - # behaviour, not ours; the scrub below is the guarantee we control. - assert "sup3rsecret" not in str(excinfo.value) - - -def test_scrub_removes_the_token_when_git_does_echo_it() -> None: - assert _scrub("failed to push to https://bot:s3cret@h/o/r.git", "s3cret") == ( - "failed to push to https://bot:***@h/o/r.git" - ) - assert _scrub("no secret here", None) == "no secret here" - - -def test_git_error_on_bad_command(tmp_path: Path) -> None: - repo = GitRepo(tmp_path / "not-a-repo") +def test_git_error_on_bad_command(ssh_host: SSHHost, tmp_path: Path) -> None: + repo = GitRepo(ssh_host, tmp_path / "not-a-repo") repo.root.mkdir() with pytest.raises(GitError): repo.head_sha() @@ -73,15 +60,87 @@ def test_git_error_on_bad_command(tmp_path: Path) -> None: def test_commits_since_and_remote_sha_against_a_local_remote( repo: GitRepo, tmp_path: Path ) -> None: - remote = GitRepo(tmp_path / "remote.git") - remote.root.mkdir() - remote._run("init", "-q", "--bare") + remote = bare_remote(tmp_path / "remote.git") (repo.root / "a.txt").write_text("a") base = repo.commit_all("base") - repo.push(str(remote.root), "main") - repo._run("update-ref", "refs/remotes/origin/main", "HEAD") - assert repo.remote_sha(str(remote.root), "main") == base - assert repo.remote_sha(str(remote.root), "absent") is None + repo.push(remote, "main") + repo._git("update-ref", "refs/remotes/origin/main", "HEAD") + assert repo.remote_sha(remote, "main") == base + assert repo.remote_sha(remote, "absent") is None (repo.root / "b.txt").write_text("b") repo.commit_all("ahead") assert repo.commits_since("origin/main") == 1 + + +# --- the push key (#89) ------------------------------------------------------ + + +@pytest.fixture +def ssh_remote(ssh_host: SSHHost, tmp_path: Path) -> tuple[str, str]: + """A bare repository reached by git over SSH, and the key that may push + to it: the forge, as the build VM sees it.""" + remote = bare_remote(tmp_path / "forge.git") + url = f"ssh://{ssh_host.user}@127.0.0.1:{ssh_host.port}{remote}" + return url, ssh_host._key.read_text() + + +def test_clone_and_push_with_the_key_store_no_credential( + ssh_host: SSHHost, tmp_path: Path, ssh_remote: tuple[str, str] +) -> None: + url, key = ssh_remote + seed = GitRepo(ssh_host, tmp_path / "seed", known_hosts=str(tmp_path / "kh")) + seed.init() + seed.configure_identity("t", "t@example.test") + (seed.root / "a.txt").write_text("a") + seed.commit_all("first") + seed.push(url, "main", key=key) + + clone = GitRepo(ssh_host, tmp_path / "clone", known_hosts=str(tmp_path / "kh")) + clone.clone(url, key=key) + assert (clone.root / "a.txt").read_text() == "a" + config = (clone.root / ".git" / "config").read_text() + assert "PRIVATE KEY" not in config + # The URL recorded is the credential-free one we gave. + assert url in config + + +def test_the_key_is_gone_when_the_command_ends( + ssh_host: SSHHost, tmp_path: Path, ssh_remote: tuple[str, str] +) -> None: + url, key = ssh_remote + repo = GitRepo(ssh_host, tmp_path / "w", known_hosts=str(tmp_path / "kh")) + repo.init() + repo.configure_identity("t", "t@example.test") + (repo.root / "a.txt").write_text("a") + repo.commit_all("first") + before = _agents() + repo.push(url, "main", key=key) + assert _agents() == before # the throwaway ssh-agent was killed + + +def test_a_failed_push_still_kills_its_agent( + ssh_host: SSHHost, tmp_path: Path, ssh_remote: tuple[str, str] +) -> None: + _, key = ssh_remote + repo = GitRepo(ssh_host, tmp_path / "w", known_hosts=str(tmp_path / "kh")) + repo.init() + before = _agents() + with pytest.raises(GitError): + repo.push( + f"ssh://nobody@127.0.0.1:{ssh_host.port}/nowhere.git", "main", key=key + ) + assert _agents() == before + + +def _agents() -> set[str]: + out = subprocess.run( + ["pgrep", "-x", "ssh-agent"], capture_output=True, text=True, check=False + ).stdout + return set(out.split()) + + +def test_a_remote_url_is_quoted_not_interpreted(repo: GitRepo, tmp_path: Path) -> None: + marker = tmp_path / "pwned" + with pytest.raises(GitError): + repo.push(f"x; touch {shlex.quote(str(marker))}", "main") + assert not marker.exists() diff --git a/nisse-dev/tests/test_gitea.py b/nisse-dev/tests/test_gitea.py index f30e251..2804e9d 100644 --- a/nisse-dev/tests/test_gitea.py +++ b/nisse-dev/tests/test_gitea.py @@ -1,5 +1,4 @@ import json -from pathlib import Path import httpx import pytest @@ -200,7 +199,6 @@ def test_intent_without_a_branch_is_unverifiable() -> None: def test_closing_comments_first_then_closes_issue_and_pr() -> None: from nisse_dev.approvals import PlanBoard - from nisse_dev.git import GitRepo from nisse_dev.publish import MilestonePublisher seen: list[tuple[str, str, dict]] = [] @@ -220,10 +218,8 @@ def test_closing_comments_first_then_closes_issue_and_pr() -> None: None, # type: ignore[arg-type] -- closing touches neither store nor git gitea, "o/r", - GitRepo(Path(".")), - base_url="https://gitea.test", - account="bot", - token="tok", + None, # type: ignore[arg-type] + remote_url="ssh://git@gitea.test/o/r.git", ) publisher.close_pull_request(20, "abandoned by don") diff --git a/nisse-dev/tests/test_publish.py b/nisse-dev/tests/test_publish.py index 2d2bf22..3572700 100644 --- a/nisse-dev/tests/test_publish.py +++ b/nisse-dev/tests/test_publish.py @@ -5,12 +5,22 @@ from typing import Any import pytest +from nisse_dev.execution import SSHHost from nisse_dev.git import GitRepo from nisse_dev.gitea import GiteaError, PullRequest from nisse_dev.oplog import OpLog from nisse_dev.publish import OPEN_PR_INTENT, MilestonePublisher from nisse_dev.state import StateStore +# The build VM for every test here: the session's throwaway sshd. +HOST: SSHHost + + +@pytest.fixture(autouse=True) +def _build_host(ssh_host: SSHHost) -> None: + global HOST + HOST = ssh_host + class FakeGitea: def __init__(self, *, fail: bool = False) -> None: @@ -46,7 +56,7 @@ class FakeGit: def __init__( self, root: Path, *, changes: bool = True, committed: bool = False ) -> None: - self.repo = GitRepo(root) + self.repo = GitRepo(HOST, root) self.repo.init() self.repo.configure_identity("bot", "bot@example.test") (root / "README.md").write_text("sandbox") @@ -67,7 +77,7 @@ class FakeGit: return self.repo.commits_since(ref) def remote_sha( - self, remote_url: str, branch: str, *, token: str | None = None + self, remote_url: str, branch: str, *, key: str | None = None ) -> str | None: return self.remote.get(branch) @@ -80,7 +90,7 @@ class FakeGit: def commit_all(self, message: str) -> str | None: return self.repo.commit_all(message) - def push(self, remote_url: str, branch: str, *, token: str | None = None) -> None: + def push(self, remote_url: str, branch: str, *, key: str | None = None) -> None: self.pushed.append(branch) self.remote[branch] = self.repo.head_sha() @@ -100,9 +110,8 @@ def build(store: StateStore, git: Any, client: Any) -> MilestonePublisher: client, "o/sandbox", git, - base_url="https://gitea.test", - account="bot", - token="tok", + remote_url="ssh://git@gitea.test/o/sandbox.git", + push_key="KEY", ) diff --git a/nisse-dev/tests/test_tools.py b/nisse-dev/tests/test_tools.py index ac8b53c..b5fef45 100644 --- a/nisse-dev/tests/test_tools.py +++ b/nisse-dev/tests/test_tools.py @@ -4,12 +4,13 @@ from pathlib import Path import pytest +from nisse_dev.execution import REDACTED, SSHHost from nisse_dev.tools import EXECUTOR_TOOLS, MAX_OUTPUT_CHARS, Workspace @pytest.fixture -def ws(tmp_path: Path) -> Workspace: - return Workspace(tmp_path / "work", timeout_s=10) +def ws(ssh_host: SSHHost, tmp_path: Path) -> Workspace: + return Workspace(ssh_host, tmp_path / "work", timeout_s=10) def test_manifest_is_small_and_flat() -> None: @@ -71,8 +72,8 @@ def test_command_runs_in_the_workspace(ws: Workspace) -> None: assert "marker.txt" in ws.run_command("ls").result -def test_command_timeout_is_reported(tmp_path: Path) -> None: - ws = Workspace(tmp_path / "work", timeout_s=1) +def test_command_timeout_is_reported(ssh_host: SSHHost, tmp_path: Path) -> None: + ws = Workspace(ssh_host, tmp_path / "work", timeout_s=1) outcome = ws.run_command("sleep 30") assert not outcome.success assert "timed out" in outcome.result @@ -90,8 +91,10 @@ def _is_running(pid: int) -> bool: return stat.rsplit(")", 1)[1].split()[0] != "Z" -def test_timeout_kills_the_whole_process_group(tmp_path: Path) -> None: - ws = Workspace(tmp_path / "work", timeout_s=1) +def test_timeout_kills_the_whole_process_group( + ssh_host: SSHHost, tmp_path: Path +) -> None: + ws = Workspace(ssh_host, tmp_path / "work", timeout_s=1) pidfile = ws.root / "child.pid" # A backgrounded grandchild must not outlive the timeout. ws.run_command(f"(sleep 30 & echo $! > {pidfile}); sleep 30") @@ -156,3 +159,45 @@ def test_escaping_via_parent_traversal_is_refused(ws: Workspace) -> None: def test_describe_names_the_root(ws: Workspace) -> None: assert str(ws.root) in ws.describe() + + +# --- on the build VM (L13) ----------------------------------------------------- + + +def test_project_secrets_reach_commands_and_are_redacted( + ssh_host: SSHHost, tmp_path: Path +) -> None: + value = "tok-" + os.urandom(8).hex() + ws = Workspace(ssh_host, tmp_path / "work", secrets={"TEST_TOKEN": value}) + outcome = ws.run_command('test -n "$TEST_TOKEN" && echo "token=$TEST_TOKEN"') + assert outcome.success # the command had it + assert value not in outcome.result + assert f"token={REDACTED}" in outcome.result + + +def test_an_unreachable_build_vm_is_a_failed_tool_call(tmp_path: Path) -> None: + ws = Workspace.__new__(Workspace) # no host to create the root on + ws.host = SSHHost( + "127.0.0.1", key=tmp_path / "no-key", state_dir=tmp_path / "s", port=1 + ) + ws.root = tmp_path / "work" + ws._timeout_s, ws._max_output_chars, ws._secrets = 5, MAX_OUTPUT_CHARS, {} + outcome = ws.dispatch("run_command", {"command": "true"}) + assert not outcome.success + assert "unreachable" in outcome.result + + +def test_upload_tree_copies_a_directory_in(ws: Workspace, tmp_path: Path) -> None: + src = tmp_path / "seed" + (src / "pkg").mkdir(parents=True) + (src / "pkg" / "m.py").write_text("x = 1\n") + ws.upload_tree(src) + assert (ws.root / "pkg" / "m.py").read_text() == "x = 1\n" + ws.upload_tree(src, "copy") + assert (ws.root / "copy" / "pkg" / "m.py").exists() + + +def test_non_text_files_read_as_a_failure(ws: Workspace) -> None: + (ws.root / "blob.bin").write_bytes(b"\xff\xfe\x00") + outcome = ws.read_file("blob.bin") + assert not outcome.success diff --git a/scripts/benchmark.py b/scripts/benchmark.py index c694a80..bac0802 100644 --- a/scripts/benchmark.py +++ b/scripts/benchmark.py @@ -14,7 +14,7 @@ import os import sys from pathlib import Path -from nisse_dev.agent import make_llm_clients +from nisse_dev.agent import build_host, make_llm_clients from nisse_dev.bench.results import append, load, render, results_file, summarise from nisse_dev.bench.runner import agent_commit, run_task from nisse_dev.bench.tasks import load_tasks @@ -46,6 +46,7 @@ def run(args: argparse.Namespace) -> int: catalog = ModelCatalog.discover(config.llm_base_url, config.llm_token) catalog.require_limits({m: m for m in models}) normalized, _passthrough = make_llm_clients(config, catalog) + host = build_host(config) # runs happen on the build VM, as the agent's do commit = agent_commit(REPO) for model in models: @@ -60,6 +61,7 @@ def run(args: argparse.Namespace) -> int: facts, normalized, config.tuning, + host=host, note=args.note, commit=commit, ) diff --git a/scripts/bringup.sh b/scripts/bringup.sh index d84333e..d18123d 100755 --- a/scripts/bringup.sh +++ b/scripts/bringup.sh @@ -17,7 +17,8 @@ # Layout, under $E2E (default /root/nisse-e2e): # nisse-dev.yaml, .env the run config and its secrets (made by hand once) # code/ the exported code under test, with its own .venv -# state.db, project/ the agent's state and workspace +# state.db the agent's state (its workspace is on the build VM) +# ssh/ the agent's key for the build VM, and push keys # run-info, collected/ what was run, and the output of `collect` # runs/