diff --git a/CHANGELOG.md b/CHANGELOG.md index 12980eb5..a27085b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ All notable changes to vouch are documented here. Format follows ## [Unreleased] ### Fixed +- **sandbox docker argv on Windows** (#582): omit `--user uid:gid` when + `os.getuid` / `os.getgid` are unavailable so sandboxed dual-solve no + longer raises `AttributeError` while building the docker command. +- **`kb.session_transcript` handler test needs a KB** : `test_handler_returns_degraded_when_absent` now chdirs into a temp KB and points Claude/Codex search roots at empty dirs, so the handler can return the degraded payload instead of raising `KBNotFoundError`. +) - **`kb.search` excludes retracted claims and archived pages** (#581): `search_kb` now drops `ARCHIVED` / `SUPERSEDED` / `REDACTED` claims and `ARCHIVED` pages the same way `kb.context` already does, so lifecycle diff --git a/src/vouch/sandbox.py b/src/vouch/sandbox.py index eb4f6883..66ea29bc 100644 --- a/src/vouch/sandbox.py +++ b/src/vouch/sandbox.py @@ -109,7 +109,15 @@ def _docker_argv(self, agent_argv: list[str], cwd: str | None) -> list[str]: args = [ "docker", "run", "--rm", "-i", "--entrypoint", "", - "--user", f"{os.getuid()}:{os.getgid()}", + ] + # os.getuid/getgid are POSIX-only. On Windows (Docker Desktop) omit + # --user so argv construction does not AttributeError before docker + # runs (#582). + getuid = getattr(os, "getuid", None) + getgid = getattr(os, "getgid", None) + if callable(getuid) and callable(getgid): + args += ["--user", f"{getuid()}:{getgid()}"] + args += [ "-w", str(workdir), "-e", f"HOME={self.container_home}", "-v", f"{self.sandbox_home}:{self.container_home}", diff --git a/tests/test_sandbox.py b/tests/test_sandbox.py index 37e93134..4ac16b9c 100644 --- a/tests/test_sandbox.py +++ b/tests/test_sandbox.py @@ -54,8 +54,14 @@ def test_docker_agent_runner_wraps_agent_with_worktree_and_home_mounts( argv = fr.calls[0] assert argv[:3] == ["docker", "run", "--rm"] assert "--entrypoint" in argv and "" in argv - assert "--user" in argv - assert f"{os.getuid()}:{os.getgid()}" in argv + # match sandbox._docker_argv: both must be callable, not merely present. + uid_ok = callable(getattr(os, "getuid", None)) + gid_ok = callable(getattr(os, "getgid", None)) + if uid_ok and gid_ok: + assert "--user" in argv + assert f"{os.getuid()}:{os.getgid()}" in argv + else: + assert "--user" not in argv assert "-w" in argv and str(worktree.resolve()) in argv assert "-e" in argv assert f"HOME={sandbox.CONTAINER_HOME}" in argv @@ -72,6 +78,51 @@ def test_docker_agent_runner_wraps_agent_with_worktree_and_home_mounts( runner.close() +@pytest.mark.parametrize( + "break_uid,break_gid", + [ + pytest.param(True, False, id="getuid-missing"), + pytest.param(False, True, id="getgid-missing"), + pytest.param(True, True, id="both-missing"), + pytest.param("noncallable", False, id="getuid-noncallable"), + pytest.param(False, "noncallable", id="getgid-noncallable"), + ], +) +def test_docker_argv_omits_user_when_uid_gid_unavailable( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + break_uid: bool | str, + break_gid: bool | str, +) -> None: + """Regression for #582: omit --user unless both getuid and getgid are callable.""" + repo = tmp_path / "repo" + repo.mkdir() + + def _break(attr: str, how: bool | str) -> None: + if how is False: + return + if how == "noncallable": + monkeypatch.setattr(sandbox.os, attr, object(), raising=False) + else: + monkeypatch.delattr(sandbox.os, attr, raising=False) + + _break("getuid", break_uid) + _break("getgid", break_gid) + + fr = FakeRunner() + runner = sandbox.DockerAgentRunner( + repo_root=repo, runner=fr, image="agent-img", host_home=tmp_path, + ) + try: + runner.run(["claude", "-p", "hi"], cwd=str(repo)) + argv = fr.calls[0] + assert argv[:3] == ["docker", "run", "--rm"] + assert "--user" not in argv + assert "agent-img" in argv + finally: + runner.close() + + def test_require_docker_sandbox_reports_missing_image(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(sandbox.shutil, "which", lambda name: "/usr/bin/docker") fr = FakeRunner(ap.RunResult(1, "", "no such image")) diff --git a/tests/test_session_transcript.py b/tests/test_session_transcript.py index 19c0a6d9..5af1a244 100644 --- a/tests/test_session_transcript.py +++ b/tests/test_session_transcript.py @@ -196,9 +196,17 @@ def test_handler_bad_agent_is_invalid_request() -> None: assert resp["error"]["code"] == "invalid_request" -def test_handler_returns_degraded_when_absent() -> None: +def test_handler_returns_degraded_when_absent( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: from vouch.jsonl_server import handle_request + # handle_request discovers the KB via cwd; point both agent search roots + # at empty dirs so the locator misses and returns the degraded payload. + monkeypatch.chdir(store.root) + monkeypatch.setenv("VOUCH_CLAUDE_PROJECTS_DIR", str(store.kb_dir / "no-claude")) + monkeypatch.setenv("CODEX_HOME", str(store.kb_dir / "no-codex")) + resp = handle_request({ "id": "3", "method": "kb.session_transcript", "params": {"session_id": "11111111-1111-1111-1111-111111111111"},