From 207495481902884ef06c2a19f622ee621d8200c8 Mon Sep 17 00:00:00 2001 From: Panopticon Agent Date: Thu, 29 Oct 2026 00:13:50 -0700 Subject: [PATCH] fix(spawner): resolve repository hooks from the configured directory Resolve stored hook names with the existing confined path helper before execution. This makes dashboard-selected hooks run independently of the runner working directory and rejects escaping paths before image or container work. Existing skip, environment, workspace, and failure behavior is preserved. Add file-scoped requirements and real-process regressions for nested names, changing config roots, cwd decoys, environment overrides, path and symlink escapes, optional hooks, and nonzero exit lifecycle details. Include independently reviewed verdicts for all twelve affected requirements. Validation: 10 regression cases fail before the fix; all 101 spawner/directory tests pass afterward. Ruff, format, mypy, spec lint, wheel build, clean-wheel smoke, and the changed-scope 2119 gate pass. Fresh independent spec, code, and test-honesty reviews approve the change. The unrelated full-gate verdict baseline remains separate. --- .2119/verdicts/REQ-026.1.4--d1cf450225ed.json | 8 + .2119/verdicts/REQ-043.3.2--76aa1e966e7c.json | 8 + ...epo-hook-resolution.1.1--5bea812f8a01.json | 8 + ...epo-hook-resolution.1.2--b7a0f16564e8.json | 8 + ...epo-hook-resolution.1.3--c1585e998b1e.json | 8 + ...epo-hook-resolution.1.4--1a5eb5620231.json | 8 + ...epo-hook-resolution.1.5--db24dbf80cc5.json | 8 + ...epo-hook-resolution.1.6--2bda6ca17047.json | 8 + ...epo-hook-resolution.2.1--4c27f86fb566.json | 8 + ...epo-hook-resolution.2.2--e122f08394e4.json | 8 + ...epo-hook-resolution.2.3--161db818e13d.json | 8 + ...epo-hook-resolution.2.4--5130a7db9e3c.json | 8 + specs/repo-hook-resolution.md | 35 ++++ src/panopticon/sessionservice/spawner.py | 3 +- tests/sessionservice/test_spawner.py | 194 +++++++++++++++++- 15 files changed, 322 insertions(+), 6 deletions(-) create mode 100644 .2119/verdicts/REQ-026.1.4--d1cf450225ed.json create mode 100644 .2119/verdicts/REQ-043.3.2--76aa1e966e7c.json create mode 100644 .2119/verdicts/repo-hook-resolution.1.1--5bea812f8a01.json create mode 100644 .2119/verdicts/repo-hook-resolution.1.2--b7a0f16564e8.json create mode 100644 .2119/verdicts/repo-hook-resolution.1.3--c1585e998b1e.json create mode 100644 .2119/verdicts/repo-hook-resolution.1.4--1a5eb5620231.json create mode 100644 .2119/verdicts/repo-hook-resolution.1.5--db24dbf80cc5.json create mode 100644 .2119/verdicts/repo-hook-resolution.1.6--2bda6ca17047.json create mode 100644 .2119/verdicts/repo-hook-resolution.2.1--4c27f86fb566.json create mode 100644 .2119/verdicts/repo-hook-resolution.2.2--e122f08394e4.json create mode 100644 .2119/verdicts/repo-hook-resolution.2.3--161db818e13d.json create mode 100644 .2119/verdicts/repo-hook-resolution.2.4--5130a7db9e3c.json create mode 100644 specs/repo-hook-resolution.md diff --git a/.2119/verdicts/REQ-026.1.4--d1cf450225ed.json b/.2119/verdicts/REQ-026.1.4--d1cf450225ed.json new file mode 100644 index 0000000..88b43a4 --- /dev/null +++ b/.2119/verdicts/REQ-026.1.4--d1cf450225ed.json @@ -0,0 +1,8 @@ +{ + "reviewId": "REQ-026.1.4--d1cf450225ed", + "requirementId": "REQ-026.1.4", + "hash": "d1cf450225ed", + "verdict": "pass", + "summary": "Independent review: The `spawnable_tasks` snapshot test includes a no-dependency task with an empty `depends_on_task_ids` (\"a\") *and* one with no key at all (\"e\"), asserting both are eligible, alongside rejected counterexamples for each conjunct: claimed (\"b\") and terminal (\"c\") are excluded. Loss of no-dep eligibility or of either gate flips the exact-list assertion. Narrow but sufficient for a \"retain existing behavior\" requirement.", + "timestamp": "2026-10-29T07:13:03.567Z" +} diff --git a/.2119/verdicts/REQ-043.3.2--76aa1e966e7c.json b/.2119/verdicts/REQ-043.3.2--76aa1e966e7c.json new file mode 100644 index 0000000..12b81f7 --- /dev/null +++ b/.2119/verdicts/REQ-043.3.2--76aa1e966e7c.json @@ -0,0 +1,8 @@ +{ + "reviewId": "REQ-043.3.2--76aa1e966e7c", + "requirementId": "REQ-043.3.2", + "hash": "76aa1e966e7c", + "verdict": "pass", + "summary": "Independent review: Two sites, both with `session=False` (pre-tmux, the requirement's hard case). `test_spawn_hook_failure_aborts_spawn` and `test_non_runtime_failures_latch_exact_reason_without_retry` (clone-prep and image-staging boundaries, injected as `OSError` — deliberately *not* `CalledProcessError`, so a classifier that treated any pre-session image-stage failure as recoverable would wrongly clear). Assertions: `phases[-1] == failed` with the exact diagnostic, `cleared == []`, `releases == []`, and `heal()` on the failed task is a no-op with no respawn — retention is verified end-to-end, not by keyword. Monkeypatching private `_prepare_task_dir` is mildly white-box but does not bypass the constrained behavior (the failure classifier in `_spawn`).", + "timestamp": "2026-10-29T07:13:04.768Z" +} diff --git a/.2119/verdicts/repo-hook-resolution.1.1--5bea812f8a01.json b/.2119/verdicts/repo-hook-resolution.1.1--5bea812f8a01.json new file mode 100644 index 0000000..a43280c --- /dev/null +++ b/.2119/verdicts/repo-hook-resolution.1.1--5bea812f8a01.json @@ -0,0 +1,8 @@ +{ + "reviewId": "repo-hook-resolution.1.1--5bea812f8a01", + "requirementId": "repo-hook-resolution.1.1", + "hash": "5bea812f8a01", + "verdict": "pass", + "summary": "Independent review: `test_spawn_executes_configured_hook_with_real_process` runs a **real** hook process, parametrized over `prep.sh` and `nested/prep.sh` (nested conjunct covered). It `chdir`s to an unrelated dir containing a same-named decoy hook; cwd-relative resolution would either run the decoy (`wrong-hook-ran` asserted absent in both workspace and caller) or fail the build-time inspection. \"Before image building\" is discriminated by wrapping `build_base_if_missing` to assert `hook-output` already exists — a hook run after building fails the test. Not tautological, not over-mocked (only external services are faked).", + "timestamp": "2026-10-29T07:12:51.640Z" +} diff --git a/.2119/verdicts/repo-hook-resolution.1.2--b7a0f16564e8.json b/.2119/verdicts/repo-hook-resolution.1.2--b7a0f16564e8.json new file mode 100644 index 0000000..6047306 --- /dev/null +++ b/.2119/verdicts/repo-hook-resolution.1.2--b7a0f16564e8.json @@ -0,0 +1,8 @@ +{ + "reviewId": "repo-hook-resolution.1.2--b7a0f16564e8", + "requirementId": "repo-hook-resolution.1.2", + "hash": "b7a0f16564e8", + "verdict": "pass", + "summary": "Independent review: The real hook writes `$PWD` as its first output line; expected value is the per-task workspace path. A violation (any other cwd) changes `$PWD` *and* strands the relatively-written `hook-output` file — double discrimination.", + "timestamp": "2026-10-29T07:12:52.820Z" +} diff --git a/.2119/verdicts/repo-hook-resolution.1.3--c1585e998b1e.json b/.2119/verdicts/repo-hook-resolution.1.3--c1585e998b1e.json new file mode 100644 index 0000000..256e27d --- /dev/null +++ b/.2119/verdicts/repo-hook-resolution.1.3--c1585e998b1e.json @@ -0,0 +1,8 @@ +{ + "reviewId": "repo-hook-resolution.1.3--c1585e998b1e", + "requirementId": "repo-hook-resolution.1.3", + "hash": "c1585e998b1e", + "verdict": "pass", + "summary": "Independent review: Strong counterexample construction: the three `PANOPTICON_*` vars are pre-set to `stale-value` in the runner env (so mere inheritance fails — they must be overridden per-task), `HOOK_TEST_INHERITED` proves the runner's environment is passed through, and the asserted workspace value is absolute. All three conjuncts discriminated.", + "timestamp": "2026-10-29T07:12:54.005Z" +} diff --git a/.2119/verdicts/repo-hook-resolution.1.4--1a5eb5620231.json b/.2119/verdicts/repo-hook-resolution.1.4--1a5eb5620231.json new file mode 100644 index 0000000..a208b28 --- /dev/null +++ b/.2119/verdicts/repo-hook-resolution.1.4--1a5eb5620231.json @@ -0,0 +1,8 @@ +{ + "reviewId": "repo-hook-resolution.1.4--1a5eb5620231", + "requirementId": "repo-hook-resolution.1.4", + "hash": "1a5eb5620231", + "verdict": "pass", + "summary": "Independent review: `test_spawn_rejects_escaping_hook_before_execution` covers `../relative`, absolute, **symlink-inside-pointing-outside**, and missing-escape. Asserts `ValueError`, `base_checks == 0`, `runner.spawned == []`, and that the outside script's side-effect file was never written — i.e., abort genuinely precedes execution and container start, verified by observable non-effects, not keyword matching.", + "timestamp": "2026-10-29T07:12:55.257Z" +} diff --git a/.2119/verdicts/repo-hook-resolution.1.5--db24dbf80cc5.json b/.2119/verdicts/repo-hook-resolution.1.5--db24dbf80cc5.json new file mode 100644 index 0000000..09a04ec --- /dev/null +++ b/.2119/verdicts/repo-hook-resolution.1.5--db24dbf80cc5.json @@ -0,0 +1,8 @@ +{ + "reviewId": "repo-hook-resolution.1.5--db24dbf80cc5", + "requirementId": "repo-hook-resolution.1.5", + "hash": "db24dbf80cc5", + "verdict": "pass", + "summary": "Independent review: One spawner instance, two spawns, `PANOPTICON_CONFIG` swapped between them; each config's hook writes a distinct `config-{index}` marker that is asserted. Import-time or construction-time caching of the hooks dir would execute config-1's hook on spawn 2 and fail. Genuinely discriminates per-spawn resolution.", + "timestamp": "2026-10-29T07:12:56.424Z" +} diff --git a/.2119/verdicts/repo-hook-resolution.1.6--2bda6ca17047.json b/.2119/verdicts/repo-hook-resolution.1.6--2bda6ca17047.json new file mode 100644 index 0000000..e35d9c3 --- /dev/null +++ b/.2119/verdicts/repo-hook-resolution.1.6--2bda6ca17047.json @@ -0,0 +1,8 @@ +{ + "reviewId": "repo-hook-resolution.1.6--2bda6ca17047", + "requirementId": "repo-hook-resolution.1.6", + "hash": "2bda6ca17047", + "verdict": "pass", + "summary": "Independent review: Same rejection test asserts `phases[-1] == (\"t1\", \"failed\", \"hook_file name '' escapes the hooks dir\")` — the rejected name verifiably appears in the detail — plus `client.cleared == []`, ruling out the lifecycle being cleared into the recoverable/`down` path. The exact-string match is wording-coupled but strictly stronger than the requirement, not weaker.", + "timestamp": "2026-10-29T07:12:57.631Z" +} diff --git a/.2119/verdicts/repo-hook-resolution.2.1--4c27f86fb566.json b/.2119/verdicts/repo-hook-resolution.2.1--4c27f86fb566.json new file mode 100644 index 0000000..f20df18 --- /dev/null +++ b/.2119/verdicts/repo-hook-resolution.2.1--4c27f86fb566.json @@ -0,0 +1,8 @@ +{ + "reviewId": "repo-hook-resolution.2.1--4c27f86fb566", + "requirementId": "repo-hook-resolution.2.1", + "hash": "4c27f86fb566", + "verdict": "pass", + "summary": "Independent review: `unset` and `empty` parametrizations assert the spawn fully continues (container id returned, `base_checks == 1`, final phase `awaiting`) with no hook side effects and the cwd decoy untouched. A violation in either direction (crash, or running something) fails.", + "timestamp": "2026-10-29T07:12:58.834Z" +} diff --git a/.2119/verdicts/repo-hook-resolution.2.2--e122f08394e4.json b/.2119/verdicts/repo-hook-resolution.2.2--e122f08394e4.json new file mode 100644 index 0000000..68509c5 --- /dev/null +++ b/.2119/verdicts/repo-hook-resolution.2.2--e122f08394e4.json @@ -0,0 +1,8 @@ +{ + "reviewId": "repo-hook-resolution.2.2--e122f08394e4", + "requirementId": "repo-hook-resolution.2.2", + "hash": "e122f08394e4", + "verdict": "pass", + "summary": "Independent review: `missing` and `non-executable` (real `0o644` file) cases assert continuation without execution (`hook-ran` absent). Critically, the **boundary against 1.4 is held honest**: `missing-escape` (`../missing.sh`) appears in the *rejection* test, so \"missing but inside → skip\" cannot be satisfied by blanket skip-on-missing.", + "timestamp": "2026-10-29T07:13:00.053Z" +} diff --git a/.2119/verdicts/repo-hook-resolution.2.3--161db818e13d.json b/.2119/verdicts/repo-hook-resolution.2.3--161db818e13d.json new file mode 100644 index 0000000..12042f5 --- /dev/null +++ b/.2119/verdicts/repo-hook-resolution.2.3--161db818e13d.json @@ -0,0 +1,8 @@ +{ + "reviewId": "repo-hook-resolution.2.3--161db818e13d", + "requirementId": "repo-hook-resolution.2.3", + "hash": "161db818e13d", + "verdict": "pass", + "summary": "Independent review: Real process exits 23 after writing a marker (proving it actually ran — no over-mocking); asserts the raise, `base_checks == 0`, and `runner.spawned == []` — abort precedes both image building and container start.", + "timestamp": "2026-10-29T07:13:01.229Z" +} diff --git a/.2119/verdicts/repo-hook-resolution.2.4--5130a7db9e3c.json b/.2119/verdicts/repo-hook-resolution.2.4--5130a7db9e3c.json new file mode 100644 index 0000000..4c0e627 --- /dev/null +++ b/.2119/verdicts/repo-hook-resolution.2.4--5130a7db9e3c.json @@ -0,0 +1,8 @@ +{ + "reviewId": "repo-hook-resolution.2.4--5130a7db9e3c", + "requirementId": "repo-hook-resolution.2.4", + "hash": "5130a7db9e3c", + "verdict": "pass", + "summary": "Independent review: Same test asserts `(\"t1\", \"failed\", \"repo hook '' exited 23\")` — the exit status 23 is in the detail — and `cleared == []`, discriminating against the hook failure being misrouted into the clear-lifecycle retry path.", + "timestamp": "2026-10-29T07:13:02.403Z" +} diff --git a/specs/repo-hook-resolution.md b/specs/repo-hook-resolution.md new file mode 100644 index 0000000..1594267 --- /dev/null +++ b/specs/repo-hook-resolution.md @@ -0,0 +1,35 @@ +# Repository hook resolution + +## Overview + +This spec covers the host-side repository hook on the Docker spawn path described in +`docs/hooks.md`. A stored `hook_file` is a name relative to the runner's configured hooks +directory, including names in nested directories. The hooks directory follows the existing +config-directory override rules. Containment follows the existing path resolver, including +symlink resolution. Shell workflows and dashboard input behavior are unchanged. + +## Requirements + +### 1: Hook selection and execution + +1. A Docker spawn with an executable stored hook name, including a nested relative name, MUST + execute the script from the runner's configured hooks directory before image building, + regardless of the runner's working directory. +2. The hook process MUST use the prepared per-task workspace as its working directory. +3. The hook process MUST receive the runner's environment with `PANOPTICON_TASK_ID`, + `PANOPTICON_REPO_NAME`, and `PANOPTICON_WORKSPACE` set to the spawning task's ID, repository + name, and absolute workspace path respectively. +4. A hook name that resolves outside the configured hooks directory MUST abort the spawn before + executing that script or starting a container. +5. Each spawn MUST select its hook from the hooks directory configured at the time of that spawn. +6. A rejected hook name that resolves outside the configured hooks directory MUST leave the + task's lifecycle as `failed` with the rejected hook name in the failure detail. + +### 2: Optional hooks and failures + +1. An unset or empty hook name MUST allow the spawn to continue without running a hook. +2. A hook name whose resolved target is within the configured hooks directory but missing or + non-executable MUST allow the spawn to continue without running a hook. +3. A hook process that exits nonzero MUST abort the spawn before image building or container start. +4. A hook process that exits nonzero MUST leave the task's lifecycle as `failed` with its exit + status in the failure detail. diff --git a/src/panopticon/sessionservice/spawner.py b/src/panopticon/sessionservice/spawner.py index 73df268..19cd139 100644 --- a/src/panopticon/sessionservice/spawner.py +++ b/src/panopticon/sessionservice/spawner.py @@ -22,6 +22,7 @@ import httpx from panopticon.client import JsonObj, TaskServiceClient +from panopticon.core.dirs import hook_file_path from panopticon.core.models import ContainerStatus, LifecyclePhase from panopticon.core.state import TERMINAL_LABELS from panopticon.harnesses import Harness, get_harness @@ -344,7 +345,7 @@ def _spawn_container(self, task: JsonObj, repo: JsonObj) -> str: workspace = self._prepare_task_dir( task, repo, clone=True ) # a container always mounts a checkout - if hook_file := repo.get("hook_file"): + if hook_file := hook_file_path(repo.get("hook_file")): self._run_hook(hook_file, task_id, repo["name"], workspace) _log.info( "task %s: building image (workflow=%s, repo=%s)", diff --git a/tests/sessionservice/test_spawner.py b/tests/sessionservice/test_spawner.py index 35192fe..3d77c2e 100644 --- a/tests/sessionservice/test_spawner.py +++ b/tests/sessionservice/test_spawner.py @@ -1619,13 +1619,16 @@ def list_tasks(self) -> list[JsonObj]: assert [t["id"] for t in spawnable_tasks(_Lister())()] == ["a", "e", "f", "m"] # type: ignore[arg-type] -def test_spawn_runs_repo_hook_with_correct_args() -> None: +def test_spawn_runs_repo_hook_with_correct_args( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("PANOPTICON_CONFIG", str(tmp_path)) calls: list[tuple[str, str, str, str]] = [] def _fake_hook(hook_file: str, task_id: str, repo_name: str, workspace: str) -> None: calls.append((hook_file, task_id, repo_name, workspace)) - repo = {**_REPO, "name": "acme/widgets", "hook_file": "/hooks/acme.sh"} + repo = {**_REPO, "name": "acme/widgets", "hook_file": "acme.sh"} client, runner = _FakeClient(repo=repo), _FakeRunner() cache = CloneCache("/cache", run=_no_op_run, exists=lambda _p: True, makedirs=lambda _p: None) # type: ignore[arg-type] spawner = Spawner( @@ -1643,10 +1646,189 @@ def _fake_hook(hook_file: str, task_id: str, repo_name: str, workspace: str) -> spawner.spawn_one( {"id": "t1", "repo_id": "r1", "workflow": "spike", "state": "PLANNING", "claimed_by": None} ) - assert calls == [("/hooks/acme.sh", "t1", "acme/widgets", "/tasks/t1")] + assert calls == [(str(tmp_path / "hooks/acme.sh"), "t1", "acme/widgets", "/tasks/t1")] assert runner.spawned # container still spawned after the hook +def _hook_spawner( + tmp_path: Path, repo: JsonObj, images: _FakeImageBuilder +) -> tuple[Spawner, _FakeClient, _FakeRunner]: + """Use real hook processes and workspace directories; fake only external services.""" + (tmp_path / "tasks/t1").mkdir(parents=True) + client, runner = _FakeClient(repo=repo), _FakeRunner(session=False) + cache = CloneCache(str(tmp_path / "cache"), run=_no_op_run) + spawner = Spawner( + client, # type: ignore[arg-type] + runner, # type: ignore[arg-type] + runner_id="host-1", + cache=cache, + tasks_root=str(tmp_path / "tasks"), + # Existing temporary workspaces have the expected source; no real git or network. + git=GitClones(run=lambda _args, **_kwargs: str(repo["git_url"])), + images=images, # type: ignore[arg-type] + credential_check=lambda _task, _repo: None, + ) + return spawner, client, runner + + +def _write_repo_hook(path: Path, body: str, *, executable: bool = True) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("#!/bin/sh\nset -eu\n" + body) + path.chmod(0o755 if executable else 0o644) + + +_HOOK_TASK: JsonObj = { + "id": "t1", + "repo_id": "r1", + "workflow": "spike", + "state": "PLANNING", + "claimed_by": None, +} + + +@pytest.mark.parametrize("hook_name", ["prep.sh", "nested/prep.sh"]) +def test_spawn_executes_configured_hook_with_real_process( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, hook_name: str +) -> None: + # 2119: repo-hook-resolution.1.1 + # 2119: repo-hook-resolution.1.2 + # 2119: repo-hook-resolution.1.3 + # 2119: repo-hook-resolution.1.5 + caller = tmp_path / "unrelated" + caller.mkdir() + monkeypatch.chdir(caller) + _write_repo_hook(caller / hook_name, "echo wrong-hook > wrong-hook-ran\n") + monkeypatch.setenv("HOOK_TEST_INHERITED", "inherited-value") + for name in ("PANOPTICON_TASK_ID", "PANOPTICON_REPO_NAME", "PANOPTICON_WORKSPACE"): + monkeypatch.setenv(name, "stale-value") + repo = {**_REPO, "name": "acme/widgets", "hook_file": hook_name} + images = _FakeImageBuilder() + spawner, client, runner = _hook_spawner(tmp_path, repo, images) + build_base = images.build_base_if_missing + + # Reuse one spawner after imports and a config change: resolution must happen per spawn. + for index in (1, 2): + task_id = f"t{index}" + workspace = tmp_path / "tasks" / task_id + workspace.mkdir(exist_ok=True) + config = tmp_path / f"config-{index}" + monkeypatch.setenv("PANOPTICON_CONFIG", str(config)) + _write_repo_hook( + config / "hooks" / hook_name, + 'printf \'%s\\n\' "$PWD" "$PANOPTICON_TASK_ID" "$PANOPTICON_REPO_NAME" ' + '"$PANOPTICON_WORKSPACE" "$HOOK_TEST_INHERITED" ' + f"'config-{index}' > hook-output\n", + ) + expected = [ + str(workspace), + task_id, + "acme/widgets", + str(workspace), + "inherited-value", + f"config-{index}", + ] + + def inspect_before_build( + *, + verbose: bool = False, + workspace: Path = workspace, + expected: list[str] = expected, + ) -> bool: + assert (workspace / "hook-output").read_text().splitlines() == expected + return build_base(verbose=verbose) + + monkeypatch.setattr(images, "build_base_if_missing", inspect_before_build) + assert spawner.spawn_one({**_HOOK_TASK, "id": task_id}) == f"panopticon-{task_id}" + assert runner.spawned[-1]["workspace"] == str(workspace) + assert client.phases[-1] == (task_id, "awaiting", None) + assert not (workspace / "wrong-hook-ran").exists() + assert images.base_checks == 2 + assert not (caller / "wrong-hook-ran").exists() + + +@pytest.mark.parametrize("case", ["unset", "empty", "missing", "non-executable"]) +def test_spawn_skips_unavailable_hooks_with_real_executor( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, case: str +) -> None: + # 2119: repo-hook-resolution.2.1 + # 2119: repo-hook-resolution.2.2 + config, caller = tmp_path / "config", tmp_path / "unrelated" + caller.mkdir() + monkeypatch.setenv("PANOPTICON_CONFIG", str(config)) + monkeypatch.chdir(caller) + _write_repo_hook(caller / "prep.sh", "echo wrong-hook > wrong-hook-ran\n") + repo = {**_REPO, "name": "acme/widgets"} + if case != "unset": + repo["hook_file"] = "" if case == "empty" else "prep.sh" + if case == "non-executable": + _write_repo_hook(config / "hooks/prep.sh", "echo ran > hook-ran\n", executable=False) + images = _FakeImageBuilder() + spawner, client, runner = _hook_spawner(tmp_path, repo, images) + assert spawner.spawn_one(_HOOK_TASK) == "panopticon-t1" + assert images.base_checks == 1 + assert len(runner.spawned) == 1 + assert client.phases[-1] == ("t1", "awaiting", None) + assert not (tmp_path / "tasks/t1/hook-ran").exists() + assert not (tmp_path / "tasks/t1/wrong-hook-ran").exists() + assert not (caller / "wrong-hook-ran").exists() + + +@pytest.mark.parametrize("case", ["relative", "absolute", "symlink", "missing-escape"]) +def test_spawn_rejects_escaping_hook_before_execution( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, case: str +) -> None: + # 2119: repo-hook-resolution.1.4 + # 2119: repo-hook-resolution.1.6 + config = tmp_path / "config" + hooks = config / "hooks" + hooks.mkdir(parents=True) + monkeypatch.setenv("PANOPTICON_CONFIG", str(config)) + outside = config / "outside.sh" + _write_repo_hook(outside, "echo ran > outside-ran\n") + names = { + "relative": "../outside.sh", + "absolute": str(outside), + "symlink": "linked.sh", + "missing-escape": "../missing.sh", + } + (hooks / "linked.sh").symlink_to(outside) + repo = {**_REPO, "name": "acme/widgets", "hook_file": names[case]} + images = _FakeImageBuilder() + spawner, client, runner = _hook_spawner(tmp_path, repo, images) + with pytest.raises(ValueError, match="escapes the hooks dir"): + spawner.spawn_one(_HOOK_TASK) + assert images.base_checks == 0 + assert runner.spawned == [] + assert not (tmp_path / "tasks/t1/outside-ran").exists() + assert client.phases[-1] == ( + "t1", + "failed", + f"hook_file name {names[case]!r} escapes the hooks dir", + ) + assert client.cleared == [] + + +def test_spawn_nonzero_hook_preserves_failure_with_real_process( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # 2119: repo-hook-resolution.2.3 + # 2119: repo-hook-resolution.2.4 + config = tmp_path / "config" + monkeypatch.setenv("PANOPTICON_CONFIG", str(config)) + hook = config / "hooks/fail.sh" + _write_repo_hook(hook, "echo started > hook-started\nexit 23\n") + repo = {**_REPO, "name": "acme/widgets", "hook_file": "fail.sh"} + images = _FakeImageBuilder() + spawner, client, runner = _hook_spawner(tmp_path, repo, images) + with pytest.raises(RuntimeError, match="exited 23"): + spawner.spawn_one(_HOOK_TASK) + assert (tmp_path / "tasks/t1/hook-started").read_text() == "started\n" + assert images.base_checks == 0 + assert runner.spawned == [] + assert client.phases[-1] == ("t1", "failed", f"repo hook {str(hook)!r} exited 23") + assert client.cleared == [] + + def _cleanup_spawner(runner: _FakeRunner, *, workspace_exists: bool) -> Spawner: """Helper: a spawner with injectable exists/rmtree for cleanup tests.""" client = _FakeClient(repo=_REPO) @@ -1807,12 +1989,14 @@ def release(self, task_id: str) -> JsonObj: assert removed == ["/tasks/t1"] # workspace cleaned -def test_spawn_hook_failure_aborts_spawn() -> None: +def test_spawn_hook_failure_aborts_spawn(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("PANOPTICON_CONFIG", str(tmp_path)) + # 2119: REQ-043.3.2 def _boom(hook_file: str, task_id: str, repo_name: str, workspace: str) -> None: raise RuntimeError("hook exited 1") - repo = {**_REPO, "name": "acme/widgets", "hook_file": "/hooks/acme.sh"} + repo = {**_REPO, "name": "acme/widgets", "hook_file": "acme.sh"} client, runner = _FakeClient(repo=repo), _FakeRunner(session=False) cache = CloneCache("/cache", run=_no_op_run, exists=lambda _p: True, makedirs=lambda _p: None) # type: ignore[arg-type] spawner = Spawner(