diff --git a/.env.example b/.env.example index b5e1d85..f5861a8 100644 --- a/.env.example +++ b/.env.example @@ -11,9 +11,8 @@ 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 +# The live push test reads the sandbox's deploy key from the path in +# tests/integration/nisse-test.yaml (gitea.push_key), not from here. # 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 a5c3c3b..c97c599 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -42,6 +42,15 @@ jobs: - name: Unit tests run: uv run pytest -m "not integration" + - name: Push key + # The sandbox's deploy key, where nisse-test.yaml expects it. + env: + NISSE_PUSH_KEY: ${{ secrets.NISSE_PUSH_KEY }} + run: | + mkdir -p /tmp/nisse-test + printf '%s\n' "$NISSE_PUSH_KEY" > /tmp/nisse-test/push_key + chmod 600 /tmp/nisse-test/push_key + - name: Integration tests # Everything but the secrets is in tests/integration/nisse-test.yaml, # which refers to these by name. A missing secret fails the live @@ -50,6 +59,4 @@ 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/docs/Architecture.md b/docs/Architecture.md index 88c9594..3047c80 100644 --- a/docs/Architecture.md +++ b/docs/Architecture.md @@ -674,9 +674,10 @@ Installing on any other host is dangerous. fresh and this one resumes from its branch. - **Publish only the work:** what the agent changed, counted from where the milestone started, never build artifacts; anything left out is reported. -- **Pushes run on the build VM** with a key the model's commands never receive - (L12); PRs are opened from the agent's container with its API token, which never - reaches a build VM. Gitea is the only copy that has to survive. +- **Pushes run on the build VM** with a push credential (a deploy key, or a + push-only account's token) the model's commands never receive (L12); PRs are + opened from the agent's container with its API token, which never reaches a + build VM. Gitea is the only copy that has to survive. - **Publishing is idempotent** (L8). A milestone with no changes is done if it wasn't expected to change code; otherwise it's raised with the operator. - **The agent never owns or administers a repository:** whoever does can turn @@ -727,7 +728,7 @@ execution: **Secrets are namespaced by project.** A build VM's commands receive only their project's, and each secret's configuration says whether the model's commands may -see it or only harness operations use it (the push key). Anything the model's +see it or only harness operations use it (the push credential). Anything the model's commands can see must be scoped for testing, never production. **Provisioning:** build VMs come from a template via Terraform; eventually at diff --git a/docs/Development-Plan.md b/docs/Development-Plan.md index e104578..9b1a5ac 100644 --- a/docs/Development-Plan.md +++ b/docs/Development-Plan.md @@ -354,8 +354,10 @@ if the implementation differs. - **D3 preflight:** a project's needs are read from its files (`Cargo.toml`, `pyproject.toml`, `package.json`) or a declaration in the repository. - **D4 publishing:** standard excludes are configured in each clone, and only what - the work produced is committed. A push hands the key over stdin to a - short-lived `ssh-agent` on the build VM, for that one push. + the work produced is committed. The push credential is handed to each push + only: a deploy key over stdin to a short-lived `ssh-agent` on the build VM, or + a push-only account's HTTPS token as that command's secrets, read by a + credential helper that names environment variables, never values. - **D7:** test services use ports 18000–18099, so opening access later is a firewall change. The inference host is behind LiteLLM, so it is not a second egress exception. diff --git a/nisse-dev/src/nisse_dev/agent.py b/nisse-dev/src/nisse_dev/agent.py index a97e81d..995cc87 100644 --- a/nisse-dev/src/nisse_dev/agent.py +++ b/nisse-dev/src/nisse_dev/agent.py @@ -21,7 +21,7 @@ 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 +from nisse_dev.git import GitRepo, HTTPSToken, PushCredential, SSHKey from nisse_dev.gitea import Comment, GiteaClient from nisse_dev.llm import AnthropicPassthroughClient, LiteLLMClient from nisse_dev.matrix import MatrixTransport @@ -177,30 +177,45 @@ def build_host(config: AgentConfig) -> SSHHost: ) -def read_push_key(config: AgentConfig) -> str | None: +def push_credential(config: AgentConfig) -> PushCredential | None: + """What pushes go by: a push-only account's token over HTTPS, or a + deploy key over SSH (D4).""" + if config.push_user and config.push_token: + return HTTPSToken(config.push_user, config.push_token) if config.push_key is None: return None try: - return config.push_key.read_text() + return SSHKey(config.push_key.read_text()) except OSError as exc: raise RuntimeError( f"cannot read the push key {config.push_key}: {exc}" ) from exc +def remote_url(gitea: GiteaClient, repo: str, credential: PushCredential | None) -> str: + """The credential-free URL the workspace clones from and pushes to.""" + if isinstance(credential, SSHKey): + return gitea.ssh_url(repo) + return gitea.clone_url(repo) + + def prepare_workspace( - host: SSHHost, config: AgentConfig, account: str, remote: str, key: str | None + host: SSHHost, + config: AgentConfig, + account: str, + remote: str, + credential: PushCredential | 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. + in it (#89); the push credential 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.clone(remote, credential=credential) repo.configure_identity(account, f"{account}@users.noreply.local") return repo @@ -229,9 +244,9 @@ def build_agent( account = gitea.whoami() 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) + credential = push_credential(config) + remote = remote_url(gitea, config.gitea_repo, credential) + git_repo = prepare_workspace(host, config, account, remote, credential) tuning = config.tuning workspace = Workspace( host, @@ -278,7 +293,7 @@ def build_agent( config.gitea_repo, git_repo, remote_url=remote, - push_key=push_key, + push_credential=credential, base_branch=config.base_branch, ) diff --git a/nisse-dev/src/nisse_dev/config.py b/nisse-dev/src/nisse_dev/config.py index bfeba29..e4d414c 100644 --- a/nisse-dev/src/nisse_dev/config.py +++ b/nisse-dev/src/nisse_dev/config.py @@ -55,6 +55,8 @@ OPTIONAL_KEYS = ( "llm.request_timeout_s", "gitea.base_branch", "gitea.push_key", + "gitea.push_user", + "gitea.push_token", "build_host.user", "build_host.port", ) @@ -241,9 +243,12 @@ class AgentConfig: 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). + # What may push to the project repository, handed to each push on the + # build VM and to nothing else (D4): a push-only account's HTTPS token + # (push_user, push_token), or the path to a deploy key (push_key). push_key: Path | None = None + push_user: str | None = None + push_token: str | None = None max_output_tokens: int = DEFAULT_MAX_OUTPUT_TOKENS request_timeout_s: float = DEFAULT_REQUEST_TIMEOUT_S tuning: Tuning = field(default_factory=Tuning) @@ -331,6 +336,12 @@ class AgentConfig: raise ValueError except (TypeError, ValueError): errors.append("build_host.port: must be a TCP port number") + push_user = values.get("gitea.push_user") + push_token = values.get("gitea.push_token") + if bool(push_user) != bool(push_token): + errors.append("gitea.push_user and gitea.push_token: set both or neither") + if push_token and values.get("gitea.push_key"): + errors.append("gitea: set a push token or a push key, not both") if errors: raise ConfigError("cannot start: " + "; ".join(errors)) @@ -365,6 +376,8 @@ class AgentConfig: port=port, ), push_key=Path(push_key).expanduser() if push_key else None, + push_user=str(push_user) if push_user else None, + push_token=str(push_token) if push_token else None, base_branch=str(values.get("gitea.base_branch") or "main"), max_output_tokens=max_output, request_timeout_s=request_timeout, diff --git a/nisse-dev/src/nisse_dev/git.py b/nisse-dev/src/nisse_dev/git.py index 89986cc..4df2292 100644 --- a/nisse-dev/src/nisse_dev/git.py +++ b/nisse-dev/src/nisse_dev/git.py @@ -3,9 +3,15 @@ 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. +ls-remote) are handed the push credential for that one operation, and the +model's commands never receive it: + +- an HTTPS token (the default: HTTPS reaches the forge from everywhere) goes + to git through a credential helper that names environment variables, never + values; the values are that one command's secrets, so they are never on a + command line and are redacted from its output; +- an SSH key is fed over stdin to a short-lived ``ssh-agent`` on the VM, gone + when the command ends. Pull requests are opened from the agent's own host with its Gitea API token (publish.py), which never reaches the VM. @@ -27,6 +33,31 @@ class GitError(RuntimeError): pass +@dataclass(frozen=True) +class HTTPSToken: + """A push-only account's token, for HTTPS remotes (D4).""" + + username: str + token: str + + +@dataclass(frozen=True) +class SSHKey: + """A deploy key's private half, for SSH remotes (D4).""" + + private_key: str + + +PushCredential = HTTPSToken | SSHKey + +# git asks the helper for credentials; it answers from the environment, so +# the command line names variables, never values. +_HELPER = ( + '!f() { test "$1" = get || return 0; ' + 'echo "username=$NISSE_PUSH_USER"; echo "password=$NISSE_PUSH_TOKEN"; }; f' +) + + 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 @@ -48,13 +79,33 @@ class GitRepo: # the VM user's own known_hosts. known_hosts: str | None = None - def _git(self, *args: str, key: str | None = None, cwd: str | None = None) -> str: - command = "git " + " ".join(shlex.quote(a) for a in args) + def _git( + self, + *args: str, + credential: PushCredential | None = None, + cwd: str | None = None, + ) -> str: + quoted = " ".join(shlex.quote(a) for a in args) + command, stdin, secrets = f"git {quoted}", None, {} + if isinstance(credential, SSHKey): + command = _with_key(command, self.known_hosts) + stdin = credential.private_key + elif isinstance(credential, HTTPSToken): + # The empty helper first clears any configured on the VM. + command = ( + "GIT_TERMINAL_PROMPT=0 git -c credential.helper= " + f"-c credential.helper={shlex.quote(_HELPER)} {quoted}" + ) + secrets = { + "NISSE_PUSH_USER": credential.username, + "NISSE_PUSH_TOKEN": credential.token, + } try: result = self.host.execute( - _with_key(command, self.known_hosts) if key else command, + command, cwd=cwd or str(self.root), - stdin=key, + stdin=stdin, + secrets=secrets, timeout_s=TIMEOUT_S, ) except HostUnavailable as exc: @@ -87,11 +138,13 @@ class GitRepo: return int(self._git("rev-list", "--count", f"{ref}..HEAD")) def remote_sha( - self, remote_url: str, branch: str, *, key: str | None = None + self, remote_url: str, branch: str, *, credential: PushCredential | 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._git("ls-remote", remote_url, f"refs/heads/{branch}", key=key) + out = self._git( + "ls-remote", remote_url, f"refs/heads/{branch}", credential=credential + ) return out.split()[0] if out else None def exists(self) -> bool: @@ -110,12 +163,16 @@ class GitRepo: self.host.execute(f"mkdir -p -- {shlex.quote(str(self.root))}") self._git("init", "-q", "-b", default_branch) - def clone(self, remote_url: str, *, key: str | None = None) -> None: + def clone( + self, remote_url: str, *, credential: PushCredential | 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) + self._git( + "clone", "-q", remote_url, str(self.root), credential=credential, cwd=parent + ) def configure_identity(self, name: str, email: str) -> None: self._git("config", "user.name", name) @@ -143,10 +200,14 @@ class GitRepo: self._git("commit", "-q", "-m", message) return self.head_sha() - def push(self, remote_url: str, branch: str, *, key: str | None = None) -> None: + def push( + self, remote_url: str, branch: str, *, credential: PushCredential | 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) + self._git( + "push", "-q", remote_url, f"HEAD:refs/heads/{branch}", credential=credential + ) def discard_branch(self, branch: str, *, back_to: str) -> None: """Throw away a branch and everything on it. @@ -159,6 +220,8 @@ class GitRepo: self._git("branch", "-q", "-D", branch) def delete_remote_branch( - self, remote_url: str, branch: str, *, key: str | None = None + self, remote_url: str, branch: str, *, credential: PushCredential | None = None ) -> None: - self._git("push", "-q", remote_url, f":refs/heads/{branch}", key=key) + self._git( + "push", "-q", remote_url, f":refs/heads/{branch}", credential=credential + ) diff --git a/nisse-dev/src/nisse_dev/gitea.py b/nisse-dev/src/nisse_dev/gitea.py index 25a9214..84dbdf7 100644 --- a/nisse-dev/src/nisse_dev/gitea.py +++ b/nisse-dev/src/nisse_dev/gitea.py @@ -74,6 +74,10 @@ class GiteaClient: """The repository's SSH clone URL, which carries no credential.""" return str(self._request("GET", f"/repos/{repo}")["ssh_url"]) + def clone_url(self, repo: str) -> str: + """The repository's HTTPS clone URL, which carries no credential.""" + return str(self._request("GET", f"/repos/{repo}")["clone_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 f925ffe..ed8a72d 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 +from nisse_dev.git import GitRepo, PushCredential from nisse_dev.gitea import ( GiteaClient, GiteaError, @@ -37,7 +37,7 @@ class MilestonePublisher: git_repo: GitRepo, *, remote_url: str, - push_key: str | None = None, + push_credential: PushCredential | None = None, base_branch: str = "main", ) -> None: self._store = store @@ -45,10 +45,10 @@ class MilestonePublisher: self._repo = repo self._git = git_repo self._base_branch = base_branch - # A URL with no credential in it; the push key is handed to each push - # on the build VM and nothing else (D4, L12). + # A URL with no credential in it; the push credential is handed to + # each push on the build VM and nothing else (D4, L12). self._remote = remote_url - self._key = push_key + self._credential = push_credential def reconcilers(self) -> dict[str, Reconciler]: return {OPEN_PR_INTENT: pull_request_reconciler(self._client, self._repo)} @@ -85,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, key=self._key) + self._git.push(self._remote, branch, credential=self._credential) # 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. @@ -150,10 +150,12 @@ 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, key=self._key) + pushed = self._git.remote_sha( + self._remote, branch, credential=self._credential + ) if sha == pushed: return None - self._git.push(self._remote, branch, key=self._key) + self._git.push(self._remote, branch, credential=self._credential) return sha diff --git a/nisse-dev/tests/test_agent_build.py b/nisse-dev/tests/test_agent_build.py index b313a60..aab4c43 100644 --- a/nisse-dev/tests/test_agent_build.py +++ b/nisse-dev/tests/test_agent_build.py @@ -151,7 +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.GiteaClient.ssh_url", lambda self, repo: str(origin) + "nisse_dev.agent.GiteaClient.clone_url", lambda self, repo: str(origin) ) return build_agent(config) @@ -355,7 +355,7 @@ def test_tuning_reaches_every_component( ) monkeypatch.setattr("nisse_dev.agent.MatrixTransport", lambda cfg: FakeTransport()) monkeypatch.setattr( - "nisse_dev.agent.GiteaClient.ssh_url", lambda self, repo: str(origin) + "nisse_dev.agent.GiteaClient.clone_url", lambda self, repo: str(origin) ) agent = build_agent(config) executor = agent.orchestrator._executor diff --git a/nisse-dev/tests/test_config.py b/nisse-dev/tests/test_config.py index 8c809c3..4b3943f 100644 --- a/nisse-dev/tests/test_config.py +++ b/nisse-dev/tests/test_config.py @@ -351,3 +351,12 @@ def test_a_bad_build_host_port_is_reported() -> None: 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() + + +def test_a_push_token_needs_its_user_and_excludes_a_key() -> None: + config = build(gitea__push_user="pusher", gitea__push_token="${TEST_GITEA_TOKEN}") + assert (config.push_user, config.push_token) == ("pusher", "gt") + with pytest.raises(ConfigError, match="set both or neither"): + build(gitea__push_user="pusher") + with pytest.raises(ConfigError, match="not both"): + build(gitea__push_user="p", gitea__push_token="t", gitea__push_key="~/k") diff --git a/nisse-dev/tests/test_git.py b/nisse-dev/tests/test_git.py index c3f2c34..a2b8e5e 100644 --- a/nisse-dev/tests/test_git.py +++ b/nisse-dev/tests/test_git.py @@ -7,7 +7,7 @@ from pathlib import Path import pytest from nisse_dev.execution import SSHHost -from nisse_dev.git import GitError, GitRepo +from nisse_dev.git import GitError, GitRepo, HTTPSToken, SSHKey @pytest.fixture @@ -76,16 +76,16 @@ def test_commits_since_and_remote_sha_against_a_local_remote( @pytest.fixture -def ssh_remote(ssh_host: SSHHost, tmp_path: Path) -> tuple[str, str]: +def ssh_remote(ssh_host: SSHHost, tmp_path: Path) -> tuple[str, SSHKey]: """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() + return url, SSHKey(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] + ssh_host: SSHHost, tmp_path: Path, ssh_remote: tuple[str, SSHKey] ) -> None: url, key = ssh_remote seed = GitRepo(ssh_host, tmp_path / "seed", known_hosts=str(tmp_path / "kh")) @@ -93,10 +93,10 @@ def test_clone_and_push_with_the_key_store_no_credential( seed.configure_identity("t", "t@example.test") (seed.root / "a.txt").write_text("a") seed.commit_all("first") - seed.push(url, "main", key=key) + seed.push(url, "main", credential=key) clone = GitRepo(ssh_host, tmp_path / "clone", known_hosts=str(tmp_path / "kh")) - clone.clone(url, key=key) + clone.clone(url, credential=key) assert (clone.root / "a.txt").read_text() == "a" config = (clone.root / ".git" / "config").read_text() assert "PRIVATE KEY" not in config @@ -105,7 +105,7 @@ def test_clone_and_push_with_the_key_store_no_credential( def test_the_key_is_gone_when_the_command_ends( - ssh_host: SSHHost, tmp_path: Path, ssh_remote: tuple[str, str] + ssh_host: SSHHost, tmp_path: Path, ssh_remote: tuple[str, SSHKey] ) -> None: url, key = ssh_remote repo = GitRepo(ssh_host, tmp_path / "w", known_hosts=str(tmp_path / "kh")) @@ -114,12 +114,12 @@ def test_the_key_is_gone_when_the_command_ends( (repo.root / "a.txt").write_text("a") repo.commit_all("first") before = _agents() - repo.push(url, "main", key=key) + repo.push(url, "main", credential=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] + ssh_host: SSHHost, tmp_path: Path, ssh_remote: tuple[str, SSHKey] ) -> None: _, key = ssh_remote repo = GitRepo(ssh_host, tmp_path / "w", known_hosts=str(tmp_path / "kh")) @@ -127,7 +127,9 @@ def test_a_failed_push_still_kills_its_agent( before = _agents() with pytest.raises(GitError): repo.push( - f"ssh://nobody@127.0.0.1:{ssh_host.port}/nowhere.git", "main", key=key + f"ssh://nobody@127.0.0.1:{ssh_host.port}/nowhere.git", + "main", + credential=key, ) assert _agents() == before @@ -144,3 +146,52 @@ def test_a_remote_url_is_quoted_not_interpreted(repo: GitRepo, tmp_path: Path) - with pytest.raises(GitError): repo.push(f"x; touch {shlex.quote(str(marker))}", "main") assert not marker.exists() + + +class RecordingHost: + """Captures what would be sent to the build VM.""" + + def __init__(self) -> None: + self.calls: list[dict] = [] + + def execute(self, command: str, **kwargs): + from nisse_dev.execution import Result + + self.calls.append({"command": command, **kwargs}) + return Result(0, "", "") + + +def test_an_https_token_travels_only_as_a_secret(tmp_path: Path) -> None: + host = RecordingHost() + repo = GitRepo(host, tmp_path) # type: ignore[arg-type] + token = "tok-" + "9f3a" * 6 + repo.push( + "https://gitea.test/o/r.git", "m1", credential=HTTPSToken("pusher", token) + ) + [call] = host.calls + assert token not in call["command"] # never on a command line + assert "pusher" not in call["command"] + assert call["secrets"] == {"NISSE_PUSH_USER": "pusher", "NISSE_PUSH_TOKEN": token} + # Git asks a helper that reads the environment, and any helper configured + # on the VM is cleared first. + assert "credential.helper= " in call["command"] + assert "$NISSE_PUSH_TOKEN" in call["command"] + assert "GIT_TERMINAL_PROMPT=0" in call["command"] + + +def test_an_https_token_answers_git_through_the_helper( + ssh_host: SSHHost, tmp_path: Path +) -> None: + # The helper as git runs it, on the build host: asked to "get", it + # answers from the environment; asked anything else, nothing. + from nisse_dev.git import _HELPER + + body = _HELPER.removeprefix("!") + asked = ssh_host.execute( + f"{body} get", + secrets={"NISSE_PUSH_USER": "pusher", "NISSE_PUSH_TOKEN": "tok-1234567"}, + ) + # Both travel as secrets, so both are masked in what comes back. + assert asked.stdout == "username=[redacted]\npassword=[redacted]\n" + stored = ssh_host.execute(f"{body} store", secrets={"NISSE_PUSH_TOKEN": "x"}) + assert stored.stdout == "" diff --git a/nisse-dev/tests/test_publish.py b/nisse-dev/tests/test_publish.py index 3572700..02ba4f5 100644 --- a/nisse-dev/tests/test_publish.py +++ b/nisse-dev/tests/test_publish.py @@ -6,7 +6,7 @@ from typing import Any import pytest from nisse_dev.execution import SSHHost -from nisse_dev.git import GitRepo +from nisse_dev.git import GitRepo, HTTPSToken from nisse_dev.gitea import GiteaError, PullRequest from nisse_dev.oplog import OpLog from nisse_dev.publish import OPEN_PR_INTENT, MilestonePublisher @@ -77,7 +77,7 @@ class FakeGit: return self.repo.commits_since(ref) def remote_sha( - self, remote_url: str, branch: str, *, key: str | None = None + self, remote_url: str, branch: str, *, credential: object = None ) -> str | None: return self.remote.get(branch) @@ -90,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, *, key: str | None = None) -> None: + def push(self, remote_url: str, branch: str, *, credential: object = None) -> None: self.pushed.append(branch) self.remote[branch] = self.repo.head_sha() @@ -111,7 +111,7 @@ def build(store: StateStore, git: Any, client: Any) -> MilestonePublisher: "o/sandbox", git, remote_url="ssh://git@gitea.test/o/sandbox.git", - push_key="KEY", + push_credential=HTTPSToken("pusher", "tok"), ) diff --git a/tests/integration/nisse-test.yaml b/tests/integration/nisse-test.yaml index b021725..7ad7fe3 100644 --- a/tests/integration/nisse-test.yaml +++ b/tests/integration/nisse-test.yaml @@ -42,3 +42,7 @@ gitea: token: ${NISSE_GITEA_TOKEN} repo: dongar/nisse-dev-testing-sandbox # the sandbox, never a real project operators: [dongar] + # What pushes go by (D4): the sandbox's deploy key, whose private half CI + # writes to this path from the NISSE_PUSH_KEY secret (locally: copy it + # there). An HTTPS push token works too: push_user plus push_token. + push_key: /tmp/nisse-test/push_key diff --git a/tests/integration/test_gitea_live.py b/tests/integration/test_gitea_live.py index d1049b8..19df19d 100644 --- a/tests/integration/test_gitea_live.py +++ b/tests/integration/test_gitea_live.py @@ -5,12 +5,12 @@ Cleans up after itself (closes the PR, deletes the branch) so repeated runs do not silt up the sandbox. """ -import os import uuid from pathlib import Path import pytest +from nisse_dev.agent import push_credential, remote_url from nisse_dev.config import AgentConfig from nisse_dev.execution import SSHHost from nisse_dev.git import GitError, GitRepo @@ -31,17 +31,6 @@ def intent_for(branch: str) -> Intent: ) -def push_key() -> str: - """The sandbox's deploy key with write access: what the agent pushes with - (D4). NISSE_PUSH_KEY holds the private key itself (CI) or, since a .env - line can't hold one, the path to it (local runs).""" - value = os.environ.get("NISSE_PUSH_KEY", "") - assert value.strip(), "NISSE_PUSH_KEY is not set: the sandbox's deploy key" - if "PRIVATE KEY" not in value and Path(value).expanduser().is_file(): - value = Path(value).expanduser().read_text() - return value if value.endswith("\n") else value + "\n" - - def test_push_open_pr_and_reconcile( tmp_path: Path, live_config: AgentConfig, gitea: GiteaClient, ssh_host: SSHHost ) -> None: @@ -60,9 +49,10 @@ def test_push_open_pr_and_reconcile( assert reconcile(intent_for(branch)) is False # Pushed the way the agent does: from the build VM (here the throwaway - # sshd), over SSH, with the deploy key and never the API token. - key = push_key() - url = client.ssh_url(repo_path) + # sshd), with the push credential the config names, never the API token. + key = push_credential(live_config) + assert key is not None, "nisse-test.yaml names no push credential (D4)" + url = remote_url(client, repo_path, key) repo = GitRepo(ssh_host, tmp_path / "work", known_hosts=str(tmp_path / "kh")) repo.init() repo.configure_identity(account, f"{account}@example.test") @@ -71,7 +61,7 @@ def test_push_open_pr_and_reconcile( pr = None try: - repo.push(url, branch, key=key) + repo.push(url, branch, credential=key) pr = client.open_pull_request( repo_path, head=branch, @@ -89,7 +79,7 @@ def test_push_open_pr_and_reconcile( assert reconcile(intent_for(branch)) is True # D4 is only real if the server enforces it. The agent's own token - # must not be able to merge the PR it just opened, and its push key + # must not be able to merge the PR it just opened, and its push credential # must not be able to push to the protected default branch at all. merged = client._http.post( f"/repos/{repo_path}/pulls/{pr.number}/merge", json={"Do": "merge"} @@ -99,9 +89,9 @@ def test_push_open_pr_and_reconcile( "the human gate" ) with pytest.raises(GitError): - repo.push(url, "main", key=key) + repo.push(url, "main", credential=key) finally: if pr is not None: client.close_pull_request(repo_path, pr.number) - repo.delete_remote_branch(url, branch, key=key) + repo.delete_remote_branch(url, branch, credential=key) client.close()