From 7731d3ad4d4af4d87ef1ad779bc3b46cb237ff5d Mon Sep 17 00:00:00 2001 From: zaebee Date: Thu, 22 Oct 2026 09:11:45 +0000 Subject: [PATCH 1/5] fix(guardian): review the PR as data, never run its code beside the keys `/guardian review` checked the PR out at the root and ran its lockfile, its scripts and (with GUARDIAN_EVIDENCE=1) its checkers while the provider keys were in the environment. The write-access check limits who triggers a review, not whose code runs, and the repository is public. - The job now checks out the default branch for all tooling and the PR into pr/, which is only diffed, read and parsed. - guardian_review.py gains --repo-root for the reviewed checkout; the review fingerprint is still computed from this repository. It refuses GUARDIAN_EVIDENCE=1 with a foreign checkout, and the workflow no longer passes the flag. - tests/unit/test_guardian_workflow.py pins the shape: PR in its own directory without credentials, default branch at the root, only git runs in pr/, and the one step holding secrets reviews pr/ without evidence. The same flag is what reviewing another repository (owner-api) needs. Closes #730 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/guardian.yml | 29 +++++--- scripts/guardian_review.py | 39 ++++++++++- src/cgis/guardian/evidence.py | 14 ++-- tests/unit/test_guardian_review_script.py | 83 ++++++++++++++++++++++- tests/unit/test_guardian_workflow.py | 74 ++++++++++++++++++++ 5 files changed, 222 insertions(+), 17 deletions(-) create mode 100644 tests/unit/test_guardian_workflow.py diff --git a/.github/workflows/guardian.yml b/.github/workflows/guardian.yml index a3c09250..d1f6f6aa 100644 --- a/.github/workflows/guardian.yml +++ b/.github/workflows/guardian.yml @@ -53,14 +53,28 @@ jobs: BASE_BRANCH=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json baseRefName --jq '.baseRefName') echo "BASE_BRANCH=$BASE_BRANCH" >> $GITHUB_ENV - - name: Checkout PR branch + # The PR is data, never code (#730). Everything that runs — uv sync, the + # guardian scripts, cgis itself — comes from the default branch checked + # out here; the PR lands in pr/ and is only diffed, read and parsed. + # This step and the next keep the job's provider keys out of reach of + # whoever wrote the PR. + - name: Checkout trusted tooling (default branch) + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + + - name: Checkout PR branch as data uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: # PR_NUMBER validated as digits-only in previous step ref: refs/pull/${{ env.PR_NUMBER }}/head + path: pr fetch-depth: 0 + persist-credentials: false - name: Fetch base branch for diff + working-directory: pr run: git fetch origin "$BASE_BRANCH" - name: Install uv @@ -70,7 +84,7 @@ jobs: run: uv sync --frozen --extra guardian - name: Build code graph - run: uv run --frozen cgis ingest ./src --output graph.db + run: uv run --frozen cgis ingest pr/src --output graph.db - name: Restore metrics from data branch run: | @@ -94,12 +108,10 @@ jobs: GUARDIAN_FEATURES: ${{ vars.GUARDIAN_FEATURES }} GUARDIAN_SKEPTIC: ${{ vars.GUARDIAN_SKEPTIC }} GUARDIAN_SKEPTIC_MODEL: ${{ vars.GUARDIAN_SKEPTIC_MODEL }} - # Static-evidence pass for the skeptic (#401). Wired but not set: - # the repository variable is deliberately absent, so enabling it is a - # decision made in repo settings rather than a code change nobody - # reviews. An unset variable expands to "", which the flag reads as - # off — only the literal "1" enables it. - GUARDIAN_EVIDENCE: ${{ vars.GUARDIAN_EVIDENCE }} + # GUARDIAN_EVIDENCE (#401) is deliberately not passed: the evidence + # pass runs `uv run mypy/ruff` inside the reviewed checkout, which is + # the PR, while this step holds the provider keys (#730). + # guardian_review.py refuses that combination with --repo-root. run: | uv run --frozen python scripts/guardian_review.py \ --output guardian_report.md \ @@ -108,6 +120,7 @@ jobs: --metrics guardian_metrics.jsonl \ --record-finder guardian_finder.json \ --inline \ + --repo-root pr \ --base-branch "$BASE_BRANCH" - name: Upload metrics artifact diff --git a/scripts/guardian_review.py b/scripts/guardian_review.py index 3fa01737..07ee96c7 100644 --- a/scripts/guardian_review.py +++ b/scripts/guardian_review.py @@ -3,12 +3,14 @@ import argparse import asyncio import os +from collections.abc import Mapping from pathlib import Path import structlog import cgis from cgis.guardian.collector import ContextCollector, parse_features +from cgis.guardian.evidence import EVIDENCE_FLAG from cgis.guardian.metrics import reject_metrics_path from cgis.guardian.review_fingerprint import ( compute_fingerprint, @@ -31,6 +33,28 @@ def _append_github_output(path: str, posted_inline: bool) -> None: fh.write(f"posted_inline={'true' if posted_inline else 'false'}\n") +def reviewed_root(requested: Path | None, project_root: Path, env: Mapping[str, str]) -> Path: + """The checkout to review: `--repo-root`, or this repository when it is not given. + + A foreign checkout is data only (#730): the evidence pass would run + `uv run mypy/ruff` inside it, which executes code and config the PR's author + controls while the job holds provider keys, so that combination is refused. + """ + if requested is None: + return project_root + root = requested.resolve() + if not root.is_dir(): + _msg = f"--repo-root is not a directory: {requested}" + raise ValueError(_msg) + if root != project_root.resolve() and (env.get(EVIDENCE_FLAG) or "").strip() == "1": + _msg = ( + f"{EVIDENCE_FLAG}=1 runs the reviewed checkout's own toolchain, and {requested} " + f"is not this repository; unset it for a --repo-root review (#730)." + ) + raise ValueError(_msg) + return root + + async def main() -> None: """Run Guardian review and write the result to stdout or a file.""" parser = argparse.ArgumentParser(description="Run CGIS Guardian AI code review.") @@ -69,6 +93,15 @@ async def main() -> None: default=None, help="Write the finder pass (findings + diff) to this .json for offline re-scoring.", ) + parser.add_argument( + "--repo-root", + type=Path, + default=None, + help=( + "The checkout to review (default: this repository). guardian.yml passes the PR " + "checkout here, so the PR is read and parsed but none of its code runs (#730)." + ), + ) parser.add_argument( "--inline", action="store_true", @@ -85,6 +118,9 @@ async def main() -> None: _msg = f"Refusing metrics path '{args.metrics}': {refusal}." raise ValueError(_msg) + project_root = Path(__file__).parent.parent.absolute() + repo_root = reviewed_root(args.repo_root, project_root, os.environ) + provider, model = build_provider(os.environ) # Was an isinstance sniff that returned "gemini" for an Ollama provider, # which then picked the wrong default skeptic. The provider knows its own @@ -93,14 +129,13 @@ async def main() -> None: skeptic = build_skeptic_provider(os.environ, primary=primary) features = parse_features(os.environ.get("GUARDIAN_FEATURES", "")) inline_repo = os.environ.get("GITHUB_REPOSITORY") if args.inline else None - project_root = Path(__file__).parent.parent.absolute() # Computed here rather than inside runner.py: nothing in the review closure # may import the fingerprint module, or the hasher enters its own hashed set # (#375 §4.2). project_root is this repository, not the reviewed checkout. active = resolve_active_providers(provider.name, skeptic[0].name if skeptic else None) review_fingerprint = compute_fingerprint(disk_reader(project_root), active) collector = ContextCollector( - project_root=project_root, db_path=args.db, base_branch=args.base_branch, features=features + project_root=repo_root, db_path=args.db, base_branch=args.base_branch, features=features ) log.info("Running guardian review...", model=model) report, posted_inline = await run_guardian( diff --git a/src/cgis/guardian/evidence.py b/src/cgis/guardian/evidence.py index a1770ed6..8d6873f0 100644 --- a/src/cgis/guardian/evidence.py +++ b/src/cgis/guardian/evidence.py @@ -10,12 +10,14 @@ This module runs that command. It does not lower the bar; it makes the bar reachable. -Scope, stated rather than implied: the verbs are Python-only. `guardian.yml` and -`guardian_bench` review this repository, where `uv` and the checkers are -installed; `guardian_martian` reviews five foreign repositories whose toolchains -are not ours, and there this returns None and the skeptic behaves exactly as it -does today. `collect_evidence` is the seam a TypeScript implementation would -extend, not replace. +Scope, stated rather than implied: the verbs are Python-only. `guardian_bench` +reviews this repository, where `uv` and the checkers are installed; +`guardian_martian` reviews five foreign repositories whose toolchains are not +ours, and there this returns None and the skeptic behaves exactly as it does +today. `guardian.yml` never runs it: there the reviewed checkout is the PR, and +running its checkers would run its author's code beside the provider keys +(#730), so `guardian_review.py --repo-root` refuses the flag. `collect_evidence` +is the seam a TypeScript implementation would extend, not replace. Every failure is absence, never a verdict. A missing toolchain, a crashing checker, a timeout and a path outside the repository all produce None. Reporting diff --git a/tests/unit/test_guardian_review_script.py b/tests/unit/test_guardian_review_script.py index 32162175..dc41b0bc 100644 --- a/tests/unit/test_guardian_review_script.py +++ b/tests/unit/test_guardian_review_script.py @@ -15,7 +15,7 @@ import guardian_review as gr -_NO_KEYS = ("GEMINI_API_KEY", "MISTRAL_API_KEY", "GUARDIAN_PROVIDER") +_NO_KEYS = ("GEMINI_API_KEY", "MISTRAL_API_KEY", "GUARDIAN_PROVIDER", "GUARDIAN_EVIDENCE") def _argv(monkeypatch: pytest.MonkeyPatch, *args: str) -> None: @@ -54,3 +54,84 @@ async def test_a_usable_metrics_path_gets_past_the_check( _argv(monkeypatch, "--metrics", str(tmp_path / "guardian_metrics.jsonl")) with pytest.raises(RuntimeError, match="API_KEY"): await gr.main() + + +@pytest.mark.asyncio +async def test_a_repo_root_that_is_not_a_directory_is_refused_before_building_a_provider( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """`--repo-root` names the reviewed checkout (#730); a typo must not review nothing.""" + _argv(monkeypatch, "--repo-root", str(tmp_path / "missing")) + with pytest.raises(ValueError, match="--repo-root"): + await gr.main() + + +@pytest.mark.asyncio +async def test_evidence_on_a_foreign_checkout_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The evidence pass runs `uv run mypy/ruff` inside the reviewed checkout. + + With `--repo-root` that checkout is the PR — code its author controls — and + the job holds provider keys (#730). Refused loudly, not skipped quietly: a + silent skip would leave the repository variable claiming evidence is on. + """ + _argv(monkeypatch, "--repo-root", str(tmp_path)) + monkeypatch.setenv("GUARDIAN_EVIDENCE", "1") + with pytest.raises(ValueError, match="GUARDIAN_EVIDENCE"): + await gr.main() + + +@pytest.mark.asyncio +async def test_a_foreign_checkout_without_evidence_gets_past_the_checks( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The normal #730 case: a separate PR checkout, evidence off, reaches the provider.""" + _argv(monkeypatch, "--repo-root", str(tmp_path)) + with pytest.raises(RuntimeError, match="API_KEY"): + await gr.main() + + +class _CollectedError(Exception): + """Raised by the stand-in collector to stop the review once its root is known.""" + + +@pytest.fixture +def roots(tmp_path: Path) -> tuple[Path, Path]: + """(the reviewed checkout as resolved, this repository) — resolved outside the event loop.""" + return tmp_path.resolve(), Path(gr.__file__).parent.parent.absolute() + + +@pytest.mark.asyncio +async def test_the_review_reads_the_repo_root_and_fingerprints_this_repository( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, roots: tuple[Path, Path] +) -> None: + """The flag must reach the collector. + + Validating it and then reviewing `.` would pass every test above. + """ + seen: dict[str, Path] = {} + + class _Provider: + name = "stub" + + def _collector(**kwargs: object) -> None: + seen["root"] = Path(str(kwargs["project_root"])) + raise _CollectedError + + def _fingerprint(reader: object, _active: object) -> str: + seen["fingerprint_reader"] = Path(str(reader)) + return "fp" + + _argv(monkeypatch, "--repo-root", str(tmp_path)) + monkeypatch.setenv("GUARDIAN_SKEPTIC", "off") + monkeypatch.setattr(gr, "build_provider", lambda _env: (_Provider(), "stub-model")) + monkeypatch.setattr(gr, "build_skeptic_provider", lambda _env, **_kw: None) + monkeypatch.setattr(gr, "disk_reader", lambda root: root) + monkeypatch.setattr(gr, "compute_fingerprint", _fingerprint) + monkeypatch.setattr(gr, "ContextCollector", _collector) + expected_root, this_repository = roots + with pytest.raises(_CollectedError): + await gr.main() + assert seen["root"] == expected_root + assert seen["fingerprint_reader"] == this_repository diff --git a/tests/unit/test_guardian_workflow.py b/tests/unit/test_guardian_workflow.py new file mode 100644 index 00000000..34aa45c7 --- /dev/null +++ b/tests/unit/test_guardian_workflow.py @@ -0,0 +1,74 @@ +"""The guardian review job treats the PR as data, never as code (#730). + +`/guardian review` runs with provider keys in its environment, and the PR it +reviews may come from anyone. The job used to check the PR out at the root and +run its lockfile, its scripts and its checkers there. These tests pin the shape +that closes that: trusted tooling at the root, the PR in its own directory, and +no step that holds a secret running anything from inside it. +""" + +from pathlib import Path +from typing import Any + +import pytest +import yaml + +WORKFLOW = Path(__file__).parent.parent.parent / ".github" / "workflows" / "guardian.yml" +PR_DIR = "pr" + + +@pytest.fixture(scope="module") +def steps() -> list[dict[str, Any]]: + """The steps of the `review` job.""" + jobs = yaml.safe_load(WORKFLOW.read_text(encoding="utf-8"))["jobs"] + result: list[dict[str, Any]] = jobs["review"]["steps"] + return result + + +def _checkouts(steps: list[dict[str, Any]]) -> list[dict[str, Any]]: + return [s for s in steps if str(s.get("uses", "")).startswith("actions/checkout@")] + + +def _holds_secret(step: dict[str, Any]) -> bool: + return any("secrets." in str(v) for v in (step.get("env") or {}).values()) + + +def test_the_pr_is_checked_out_into_its_own_directory(steps: list[dict[str, Any]]) -> None: + """A PR checkout at the root would put its scripts and lockfile where the job runs them.""" + pr = [s for s in _checkouts(steps) if "refs/pull/" in str(s["with"].get("ref", ""))] + assert len(pr) == 1 + assert pr[0]["with"].get("path") == PR_DIR + assert pr[0]["with"].get("persist-credentials") is False + + +def test_the_root_checkout_is_the_default_branch(steps: list[dict[str, Any]]) -> None: + """The tooling that runs must come from reviewed, merged code.""" + root = [s for s in _checkouts(steps) if not s.get("with", {}).get("path")] + assert len(root) == 1 + assert root[0]["with"]["ref"] == "${{ github.event.repository.default_branch }}" + + +def test_no_step_runs_inside_the_pr_checkout_except_git(steps: list[dict[str, Any]]) -> None: + """Only git may run in pr/: uv, python or a checker there executes the PR's own config.""" + for step in steps: + if step.get("working-directory") == PR_DIR: + assert str(step["run"]).strip().startswith("git fetch "), step["name"] + assert not _holds_secret(step), step["name"] + + +def test_the_review_reads_the_pr_checkout_and_runs_without_evidence( + steps: list[dict[str, Any]], +) -> None: + """The step with the keys reviews pr/ via --repo-root and never enables the evidence pass.""" + review = [s for s in steps if _holds_secret(s)] + assert [s["name"] for s in review] == ["Run Guardian Review"] + assert f"--repo-root {PR_DIR}" in review[0]["run"] + assert "GUARDIAN_EVIDENCE" not in review[0]["env"] + assert "working-directory" not in review[0] + + +def test_the_graph_is_built_from_the_pr_sources(steps: list[dict[str, Any]]) -> None: + """The graph must describe the code under review, which now lives in pr/.""" + ingest = [s for s in steps if "cgis ingest" in str(s.get("run", ""))] + assert len(ingest) == 1 + assert f" {PR_DIR}/src " in ingest[0]["run"] From f5c1cdc276ebfb5fba01bf4f32f4167c6d695f02 Mon Sep 17 00:00:00 2001 From: zaebee Date: Thu, 22 Oct 2026 09:20:24 +0000 Subject: [PATCH 2/5] fix(guardian): keep checkout reads inside it, and pin the workflow tighter Review of #731 found that reading the PR as data was not yet enough: the collector followed symlinks. A PR replacing CONTRIBUTING.md (read on every review) with a link to /proc/self/environ would put the provider keys in the prompt, and a prompt injection could carry them into the public comment. - collector.contained(): read_file, collect_full_files and the drift patterns.yaml read only paths that resolve inside the checkout; links that stay inside still work, as with `cgis ingest`. - test_guardian_workflow.py now fails on each regression the review tried: `git fetch && uv sync` in pr/, `uv sync --directory pr`, `python pr/scripts/...`, a dropped working-directory, job-level GUARDIAN_EVIDENCE, and the PR back at the root. Gemini: safe `.get("with")`. - guardian_review.reviewed_root resolves this repository once (Gemini). - Comments that said production runs GUARDIAN_EVIDENCE=1 or that CI ingests ./src now say what is true since #730; the metrics restore notes that it fetches anonymously. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/guardian.yml | 3 + scripts/guardian_bench.py | 8 ++- scripts/guardian_review.py | 5 +- scripts/skeptic_arms.py | 13 ++-- src/cgis/guardian/collector.py | 36 ++++++++-- tests/unit/test_guardian_collector.py | 48 +++++++++++++ tests/unit/test_guardian_review_script.py | 5 +- tests/unit/test_guardian_workflow.py | 84 +++++++++++++++++------ 8 files changed, 160 insertions(+), 42 deletions(-) diff --git a/.github/workflows/guardian.yml b/.github/workflows/guardian.yml index d1f6f6aa..051f7dba 100644 --- a/.github/workflows/guardian.yml +++ b/.github/workflows/guardian.yml @@ -86,6 +86,9 @@ jobs: - name: Build code graph run: uv run --frozen cgis ingest pr/src --output graph.db + # Fetches anonymously: the root checkout keeps no credentials (#730). + # Fine while this repository is public; on a private one this restore + # fails into `|| true` and save-metrics would overwrite the history. - name: Restore metrics from data branch run: | git fetch origin data/guardian-metrics 2>/dev/null && \ diff --git a/scripts/guardian_bench.py b/scripts/guardian_bench.py index 36c3d928..12f17ceb 100644 --- a/scripts/guardian_bench.py +++ b/scripts/guardian_bench.py @@ -146,9 +146,11 @@ async def _run_one( # hardcode None on the reasoning that a recording is not a checkout — # true, and beside the point: the commit is named by the fixture, so a # worktree gets one, which is exactly what `guardian_replay_skeptic` - # does. Production runs with GUARDIAN_EVIDENCE=1, so judging without it - # measured a configuration nobody runs and quietly attributed the - # difference to whatever else the arm changed (#246). + # does. Production ran with GUARDIAN_EVIDENCE=1 at the time, so judging + # without it measured a configuration nobody ran and quietly attributed + # the difference to whatever else the arm changed (#246). Since #730 + # guardian.yml runs no evidence pass (the reviewed checkout is the PR); + # the bench keeps collecting it so its numbers stay comparable. evidence = await asyncio.to_thread( _evidence_at_head, truth.head, recording.diff, _REPO_ROOT ) diff --git a/scripts/guardian_review.py b/scripts/guardian_review.py index 07ee96c7..450d9f06 100644 --- a/scripts/guardian_review.py +++ b/scripts/guardian_review.py @@ -40,13 +40,14 @@ def reviewed_root(requested: Path | None, project_root: Path, env: Mapping[str, `uv run mypy/ruff` inside it, which executes code and config the PR's author controls while the job holds provider keys, so that combination is refused. """ + this_repository = project_root.resolve() if requested is None: - return project_root + return this_repository root = requested.resolve() if not root.is_dir(): _msg = f"--repo-root is not a directory: {requested}" raise ValueError(_msg) - if root != project_root.resolve() and (env.get(EVIDENCE_FLAG) or "").strip() == "1": + if root != this_repository and (env.get(EVIDENCE_FLAG) or "").strip() == "1": _msg = ( f"{EVIDENCE_FLAG}=1 runs the reviewed checkout's own toolchain, and {requested} " f"is not this repository; unset it for a --repo-root review (#730)." diff --git a/scripts/skeptic_arms.py b/scripts/skeptic_arms.py index 315ded12..0e0d7b7a 100644 --- a/scripts/skeptic_arms.py +++ b/scripts/skeptic_arms.py @@ -23,12 +23,13 @@ The original single-orientation design cannot tell those apart. **Evidence is collected, not skipped.** `guardian_bench`'s replay path hardcodes -`evidence=None`, which would measure a configuration production does not run -(`GUARDIAN_EVIDENCE=1`). Here it is collected once per PR in a worktree at the -fixture head — the same mechanism `guardian_replay_skeptic` uses. Four of the six -fixtures touch Python and yield evidence, covering 96 of 135 findings; the other -two touch none, so their findings are judged with `evidence=None` because that is -also what production would do for them. +`evidence=None`, which would have measured a configuration production did not +run (`GUARDIAN_EVIDENCE=1`; production has run without it since #730). Here it +is collected once per PR in a worktree at the fixture head — the same mechanism +`guardian_replay_skeptic` uses. Four of the six fixtures touch Python and yield +evidence, covering 96 of 135 findings; the other two touch none, so their +findings are judged with `evidence=None` because that is also what production +would do for them. No finder call is made. The cost is one skeptic call per finding per arm. """ diff --git a/src/cgis/guardian/collector.py b/src/cgis/guardian/collector.py index 6445eca0..7b83af20 100644 --- a/src/cgis/guardian/collector.py +++ b/src/cgis/guardian/collector.py @@ -67,6 +67,24 @@ def features_setting(env: Mapping[str, str]) -> tuple[frozenset[str], FeaturesSo return parse_features(raw), "env" +def contained(root: Path, relative_path: str) -> Path | None: + """`root / relative_path` when it resolves inside `root`, else None. + + The reviewed checkout is a PR, and a PR is data its author controls (#730). + A symlink or a `..` is data that reads somewhere else: a CONTRIBUTING.md + linked to /proc/self/environ would put the job's provider keys in the + prompt, and from there in a public review comment. `cgis ingest` refuses + such links the same way. A link that stays inside the checkout is allowed. + """ + path = root / relative_path + try: + if path.resolve().is_relative_to(root.resolve()): + return path + except (OSError, RuntimeError): # a loop of links + return None + return None + + class SourceCollector: """What the PR changed and what its files say: git and the working tree.""" @@ -140,8 +158,8 @@ def read_file(self, relative_path: str) -> str: so a path that resolves to a directory degrades to "" instead of raising IsADirectoryError on read. """ - file_path = self.project_root / relative_path - if not file_path.is_file(): + file_path = contained(self.project_root, relative_path) + if file_path is None or not file_path.is_file(): return "" return file_path.read_text(encoding="utf-8") @@ -156,7 +174,10 @@ def collect_full_files(self, files: list[str] | None = None) -> str: sized: list[tuple[int, str, str]] = [] omitted: list[str] = [] for rel_path in changed: - path = self.project_root / rel_path + path = contained(self.project_root, rel_path) + if path is None: + omitted.append(f"file omitted: links outside the checkout ({rel_path})") + continue if not path.exists(): # deleted in this PR — nothing to show at HEAD continue text = path.read_text(encoding="utf-8") @@ -187,8 +208,9 @@ def __init__(self, project_root: Path, db_path: Path | None, source_root: str = """Set the graph DB, the ingest root its FQNs are relative to, and the project root. source_root must match the ingest root used to build the graph DB (CI - runs `cgis ingest ./src`): changed-file paths are stripped of it before - lookup. project_root locates docs/ontology/patterns.yaml for drift. + runs `cgis ingest pr/src` with project_root `pr`): changed-file paths + are stripped of it before lookup. project_root locates + docs/ontology/patterns.yaml for drift. """ self.project_root = project_root self.db_path = db_path @@ -243,8 +265,8 @@ def collect_drift(self) -> str: """ if not self.has_db(): return "" - patterns = self.project_root / "docs" / "ontology" / "patterns.yaml" - if not patterns.exists(): + patterns = contained(self.project_root, "docs/ontology/patterns.yaml") + if patterns is None or not patterns.exists(): return "" try: scorer = DriftScorer(str(patterns)) diff --git a/tests/unit/test_guardian_collector.py b/tests/unit/test_guardian_collector.py index 794bd97f..64c0acdd 100644 --- a/tests/unit/test_guardian_collector.py +++ b/tests/unit/test_guardian_collector.py @@ -527,3 +527,51 @@ def test_chunked_path_keeps_ts_files(tmp_path: Path) -> None: assert "app.handler" in context["graph_context"] assert "```ts\n" in context["full_files"] assert "README.md" not in context["full_files"] + + +# A PR is data (#730), and a symlink is data that reads somewhere else: one +# pointing at /proc/self/environ would put the job's provider keys in the prompt, +# and from there in a public review comment. Reads stay inside the checkout. + + +def _outside_secret(tmp_path: Path) -> tuple[Path, Path]: + """(the reviewed checkout, a file outside it holding a fake secret).""" + root = tmp_path / "pr" + root.mkdir() + secret = tmp_path / "environ" + secret.write_text("FAKE_API_KEY=sk-LEAKME\n") + return root, secret + + +def test_read_file_refuses_a_symlink_out_of_the_checkout(tmp_path: Path) -> None: + """CONTRIBUTING.md is read on every review, so a link there needs no feature flag.""" + root, secret = _outside_secret(tmp_path) + (root / "CONTRIBUTING.md").symlink_to(secret) + collector = ContextCollector(project_root=root) + assert collector.source.read_file("CONTRIBUTING.md") == "" + + +def test_read_file_refuses_a_parent_path(tmp_path: Path) -> None: + """The relative path is the caller's, but `..` must not climb out either.""" + root, _secret = _outside_secret(tmp_path) + collector = ContextCollector(project_root=root) + assert collector.source.read_file("../environ") == "" + + +def test_read_file_follows_a_link_that_stays_inside(tmp_path: Path) -> None: + """A repository may link one of its own files to another; that is not an escape.""" + root, _secret = _outside_secret(tmp_path) + (root / "docs.md").write_text("standards\n") + (root / "CONTRIBUTING.md").symlink_to(root / "docs.md") + collector = ContextCollector(project_root=root) + assert collector.source.read_file("CONTRIBUTING.md") == "standards\n" + + +def test_collect_full_files_refuses_a_symlink_out_of_the_checkout(tmp_path: Path) -> None: + """A changed source file that links out is left out, not read.""" + root, secret = _outside_secret(tmp_path) + (root / "leak.py").symlink_to(secret) + collector = ContextCollector(project_root=root, features=frozenset({"full_files"})) + with patch.object(collector.source, "get_changed_source_files", return_value=["leak.py"]): + result = collector.source.collect_full_files() + assert "sk-LEAKME" not in result diff --git a/tests/unit/test_guardian_review_script.py b/tests/unit/test_guardian_review_script.py index dc41b0bc..6c39dff2 100644 --- a/tests/unit/test_guardian_review_script.py +++ b/tests/unit/test_guardian_review_script.py @@ -73,8 +73,9 @@ async def test_evidence_on_a_foreign_checkout_is_refused( """The evidence pass runs `uv run mypy/ruff` inside the reviewed checkout. With `--repo-root` that checkout is the PR — code its author controls — and - the job holds provider keys (#730). Refused loudly, not skipped quietly: a - silent skip would leave the repository variable claiming evidence is on. + the job holds provider keys (#730). Refused loudly, not skipped quietly: + whoever passes the flag with a foreign checkout learns it cannot work. + guardian.yml itself does not pass it, and says why. """ _argv(monkeypatch, "--repo-root", str(tmp_path)) monkeypatch.setenv("GUARDIAN_EVIDENCE", "1") diff --git a/tests/unit/test_guardian_workflow.py b/tests/unit/test_guardian_workflow.py index 34aa45c7..9703816d 100644 --- a/tests/unit/test_guardian_workflow.py +++ b/tests/unit/test_guardian_workflow.py @@ -4,9 +4,10 @@ reviews may come from anyone. The job used to check the PR out at the root and run its lockfile, its scripts and its checkers there. These tests pin the shape that closes that: trusted tooling at the root, the PR in its own directory, and -no step that holds a secret running anything from inside it. +nothing running from inside it but one fixed `git fetch`. """ +import re from pathlib import Path from typing import Any @@ -15,13 +16,34 @@ WORKFLOW = Path(__file__).parent.parent.parent / ".github" / "workflows" / "guardian.yml" PR_DIR = "pr" +#: The only command allowed to run inside the PR checkout. +PR_FETCH = 'git fetch origin "$BASE_BRANCH"' +#: The only two ways a command may name the PR checkout: parse it, review it. +ALLOWED_MENTIONS = (f"cgis ingest {PR_DIR}/src ", f"--repo-root {PR_DIR} ") +#: Ways a command could reach into the PR checkout, after the allowed mentions are removed. +FORBIDDEN_MENTIONS = ( + re.compile(rf"(? list[dict[str, Any]]: +def workflow() -> dict[Any, Any]: + """The parsed workflow file.""" + result: dict[Any, Any] = yaml.safe_load(WORKFLOW.read_text(encoding="utf-8")) + return result + + +@pytest.fixture(scope="module") +def steps(workflow: dict[Any, Any]) -> list[dict[str, Any]]: """The steps of the `review` job.""" - jobs = yaml.safe_load(WORKFLOW.read_text(encoding="utf-8"))["jobs"] - result: list[dict[str, Any]] = jobs["review"]["steps"] + result: list[dict[str, Any]] = workflow["jobs"]["review"]["steps"] + return result + + +def _with(step: dict[str, Any]) -> dict[str, Any]: + result: dict[str, Any] = step.get("with") or {} return result @@ -35,40 +57,58 @@ def _holds_secret(step: dict[str, Any]) -> bool: def test_the_pr_is_checked_out_into_its_own_directory(steps: list[dict[str, Any]]) -> None: """A PR checkout at the root would put its scripts and lockfile where the job runs them.""" - pr = [s for s in _checkouts(steps) if "refs/pull/" in str(s["with"].get("ref", ""))] + pr = [s for s in _checkouts(steps) if "refs/pull/" in str(_with(s).get("ref", ""))] assert len(pr) == 1 - assert pr[0]["with"].get("path") == PR_DIR - assert pr[0]["with"].get("persist-credentials") is False + assert _with(pr[0]).get("path") == PR_DIR + assert _with(pr[0]).get("persist-credentials") is False def test_the_root_checkout_is_the_default_branch(steps: list[dict[str, Any]]) -> None: """The tooling that runs must come from reviewed, merged code.""" - root = [s for s in _checkouts(steps) if not s.get("with", {}).get("path")] + root = [s for s in _checkouts(steps) if not _with(s).get("path")] assert len(root) == 1 - assert root[0]["with"]["ref"] == "${{ github.event.repository.default_branch }}" + assert _with(root[0]).get("ref") == "${{ github.event.repository.default_branch }}" -def test_no_step_runs_inside_the_pr_checkout_except_git(steps: list[dict[str, Any]]) -> None: - """Only git may run in pr/: uv, python or a checker there executes the PR's own config.""" +def test_only_the_base_fetch_runs_inside_the_pr_checkout(steps: list[dict[str, Any]]) -> None: + """Exactly one fixed git command runs in pr/; `git fetch … && uv sync` must not pass.""" + inside = [s for s in steps if s.get("working-directory") == PR_DIR] + assert [str(s["run"]).strip() for s in inside] == [PR_FETCH] + assert not any(_holds_secret(s) for s in inside) + + +def test_no_command_reaches_into_the_pr_checkout(steps: list[dict[str, Any]]) -> None: + """`uv sync --directory pr` or `python pr/scripts/…` at the root: the same leak, respelt.""" for step in steps: - if step.get("working-directory") == PR_DIR: - assert str(step["run"]).strip().startswith("git fetch "), step["name"] - assert not _holds_secret(step), step["name"] + run = str(step.get("run", "")) + " " + for allowed in ALLOWED_MENTIONS: + run = run.replace(allowed, " ") + for pattern in FORBIDDEN_MENTIONS: + assert not pattern.search(run), (step.get("name"), pattern.pattern) -def test_the_review_reads_the_pr_checkout_and_runs_without_evidence( - steps: list[dict[str, Any]], -) -> None: - """The step with the keys reviews pr/ via --repo-root and never enables the evidence pass.""" +def test_the_review_runs_this_repositorys_script_on_the_pr(steps: list[dict[str, Any]]) -> None: + """The step with the keys runs scripts/ from the root and reviews pr/ through --repo-root.""" review = [s for s in steps if _holds_secret(s)] assert [s["name"] for s in review] == ["Run Guardian Review"] - assert f"--repo-root {PR_DIR}" in review[0]["run"] - assert "GUARDIAN_EVIDENCE" not in review[0]["env"] + run = str(review[0]["run"]) + " " + assert "uv run --frozen python scripts/guardian_review.py" in run + assert f"--repo-root {PR_DIR} " in run assert "working-directory" not in review[0] def test_the_graph_is_built_from_the_pr_sources(steps: list[dict[str, Any]]) -> None: - """The graph must describe the code under review, which now lives in pr/.""" + """The graph must describe the code under review, parsed by the root's cgis.""" ingest = [s for s in steps if "cgis ingest" in str(s.get("run", ""))] assert len(ingest) == 1 - assert f" {PR_DIR}/src " in ingest[0]["run"] + assert str(ingest[0]["run"]).startswith(f"uv run --frozen cgis ingest {PR_DIR}/src ") + assert "working-directory" not in ingest[0] + + +def test_the_evidence_flag_reaches_no_level_of_the_review_job( + workflow: dict[Any, Any], steps: list[dict[str, Any]] +) -> None: + """Evidence runs the PR's own checkers; not at step, job or workflow level.""" + levels = [workflow.get("env") or {}, workflow["jobs"]["review"].get("env") or {}] + levels += [s.get("env") or {} for s in steps] + assert not any("GUARDIAN_EVIDENCE" in env for env in levels) From b3c6136a440f7cd7aeaca518768d54a8d6e022e5 Mon Sep 17 00:00:00 2001 From: zaebee Date: Thu, 22 Oct 2026 09:22:51 +0000 Subject: [PATCH 3/5] test(guardian): cover the patterns.yaml containment, and note the anonymous fetch From guardian's own review of this PR (Haiku, first production run): - the drift path's refusal of a symlinked patterns.yaml had no test; it now has one, seen failing with the guard reverted; - the base-branch fetch in pr/ is anonymous, which a private-repository copy of the job must replace with a token; said next to the step. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/guardian.yml | 3 +++ tests/unit/test_guardian_collector.py | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/.github/workflows/guardian.yml b/.github/workflows/guardian.yml index 051f7dba..8a8e4e82 100644 --- a/.github/workflows/guardian.yml +++ b/.github/workflows/guardian.yml @@ -73,6 +73,9 @@ jobs: fetch-depth: 0 persist-credentials: false + # Anonymous, like the checkout above it: fine for this public repository. + # A copy of this job for a private one must pass a token here, or the + # fetch fails the job. - name: Fetch base branch for diff working-directory: pr run: git fetch origin "$BASE_BRANCH" diff --git a/tests/unit/test_guardian_collector.py b/tests/unit/test_guardian_collector.py index 64c0acdd..00ee0984 100644 --- a/tests/unit/test_guardian_collector.py +++ b/tests/unit/test_guardian_collector.py @@ -575,3 +575,14 @@ def test_collect_full_files_refuses_a_symlink_out_of_the_checkout(tmp_path: Path with patch.object(collector.source, "get_changed_source_files", return_value=["leak.py"]): result = collector.source.collect_full_files() assert "sk-LEAKME" not in result + + +def test_collect_drift_refuses_a_symlinked_patterns_file(tmp_db: Path, tmp_path: Path) -> None: + """patterns.yaml is read from the reviewed checkout too; a link out is never opened.""" + root, secret = _outside_secret(tmp_path) + (root / "docs" / "ontology").mkdir(parents=True) + (root / "docs" / "ontology" / "patterns.yaml").symlink_to(secret) + collector = ContextCollector(project_root=root, db_path=tmp_db, features=frozenset({"drift"})) + with patch("cgis.guardian.collector.DriftScorer") as scorer: + assert collector.graph.collect_drift() == "" + scorer.assert_not_called() From 2bd8724e6500890c67a9e3e3d6c5523cb213fe01 Mon Sep 17 00:00:00 2001 From: zaebee Date: Thu, 22 Oct 2026 09:30:13 +0000 Subject: [PATCH 4/5] fix(guardian): close the re-review's gaps in the workflow pin and the collector Re-review of #731: - working-directory is compared as a path, so `./pr` or `pr/` counts as the PR checkout; no step may use a local action (`uses: ./...`) or point an action's working-directory at pr/; `--project ./pr` and `pushd pr` are caught too. The allowed mentions are anchored instead of space-suffixed, so `--repo-root=pr` reads as allowed. Each respelling was seen failing. - collect_full_files skips a changed path that is not a file, and both readers replace undecodable bytes: a PR could otherwise end a review with IsADirectoryError or UnicodeDecodeError. - read_file's docstring says it also refuses paths outside the checkout. Co-Authored-By: Claude Opus 5.5 --- scripts/skeptic_arms.py | 3 +-- src/cgis/guardian/collector.py | 11 +++++---- tests/unit/test_guardian_collector.py | 18 ++++++++++++++ tests/unit/test_guardian_workflow.py | 35 +++++++++++++++++++++------ 4 files changed, 52 insertions(+), 15 deletions(-) diff --git a/scripts/skeptic_arms.py b/scripts/skeptic_arms.py index 0e0d7b7a..4dbfd2c2 100644 --- a/scripts/skeptic_arms.py +++ b/scripts/skeptic_arms.py @@ -28,8 +28,7 @@ is collected once per PR in a worktree at the fixture head — the same mechanism `guardian_replay_skeptic` uses. Four of the six fixtures touch Python and yield evidence, covering 96 of 135 findings; the other two touch none, so their -findings are judged with `evidence=None` because that is also what production -would do for them. +findings are judged with `evidence=None`, as production judged such PRs then. No finder call is made. The cost is one skeptic call per finding per arm. """ diff --git a/src/cgis/guardian/collector.py b/src/cgis/guardian/collector.py index 7b83af20..a5e6d816 100644 --- a/src/cgis/guardian/collector.py +++ b/src/cgis/guardian/collector.py @@ -149,19 +149,20 @@ def get_changed_source_files(self) -> list[str]: return [p for p in result.stdout.splitlines() if is_supported(p)] def read_file(self, relative_path: str) -> str: - """Reads a file from the project root; returns "" when it does not exist. + """Reads a file from the project root; "" when it is missing or resolves outside it. An empty string (not an error marker) lets the prompt builder omit the corresponding section entirely — a repo without CONTRIBUTING.md or docs/ontology/ should review cleanly, not get "Error: File ..." injected as if it were the standards/ontology text. `is_file()` (not `exists()`) so a path that resolves to a directory degrades to "" instead of raising - IsADirectoryError on read. + IsADirectoryError on read. Undecodable bytes are replaced: the file is the + PR's, and one bad byte must not end the review. """ file_path = contained(self.project_root, relative_path) if file_path is None or not file_path.is_file(): return "" - return file_path.read_text(encoding="utf-8") + return file_path.read_text(encoding="utf-8", errors="replace") def collect_full_files(self, files: list[str] | None = None) -> str: """Full HEAD text of given (default: changed) source files, smallest-first. @@ -178,9 +179,9 @@ def collect_full_files(self, files: list[str] | None = None) -> str: if path is None: omitted.append(f"file omitted: links outside the checkout ({rel_path})") continue - if not path.exists(): # deleted in this PR — nothing to show at HEAD + if not path.is_file(): # deleted in this PR, or not a file — nothing to show continue - text = path.read_text(encoding="utf-8") + text = path.read_text(encoding="utf-8", errors="replace") if len(text.splitlines()) > _MAX_FILE_LINES: omitted.append(f"file omitted: too large ({rel_path})") continue diff --git a/tests/unit/test_guardian_collector.py b/tests/unit/test_guardian_collector.py index 00ee0984..bae2123f 100644 --- a/tests/unit/test_guardian_collector.py +++ b/tests/unit/test_guardian_collector.py @@ -586,3 +586,21 @@ def test_collect_drift_refuses_a_symlinked_patterns_file(tmp_db: Path, tmp_path: with patch("cgis.guardian.collector.DriftScorer") as scorer: assert collector.graph.collect_drift() == "" scorer.assert_not_called() + + +def test_collect_full_files_skips_a_changed_path_that_is_a_directory(tmp_path: Path) -> None: + """A PR can make a changed `.py` path a directory (or a link to one); the review goes on.""" + (tmp_path / "pkg.py").mkdir() + collector = ContextCollector(project_root=tmp_path, features=frozenset({"full_files"})) + with patch.object(collector.source, "get_changed_source_files", return_value=["pkg.py"]): + assert collector.source.collect_full_files() == "" + + +def test_reads_survive_bytes_that_are_not_utf8(tmp_path: Path) -> None: + """Undecodable bytes in a PR file are replaced, not raised: one file must not end a review.""" + (tmp_path / "CONTRIBUTING.md").write_bytes(b"rules \xff\n") + (tmp_path / "bad.py").write_bytes(b"x = '\xff'\n") + collector = ContextCollector(project_root=tmp_path, features=frozenset({"full_files"})) + assert collector.source.read_file("CONTRIBUTING.md").startswith("rules ") + with patch.object(collector.source, "get_changed_source_files", return_value=["bad.py"]): + assert "#### `bad.py`" in collector.source.collect_full_files() diff --git a/tests/unit/test_guardian_workflow.py b/tests/unit/test_guardian_workflow.py index 9703816d..468a56c1 100644 --- a/tests/unit/test_guardian_workflow.py +++ b/tests/unit/test_guardian_workflow.py @@ -8,7 +8,7 @@ """ import re -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Any import pytest @@ -19,15 +19,26 @@ #: The only command allowed to run inside the PR checkout. PR_FETCH = 'git fetch origin "$BASE_BRANCH"' #: The only two ways a command may name the PR checkout: parse it, review it. -ALLOWED_MENTIONS = (f"cgis ingest {PR_DIR}/src ", f"--repo-root {PR_DIR} ") +ALLOWED_MENTIONS = ( + re.compile(rf"cgis ingest {PR_DIR}/src(?=\s)"), + re.compile(rf"--repo-root[ =]{PR_DIR}(?=\s)"), +) #: Ways a command could reach into the PR checkout, after the allowed mentions are removed. +#: Out of scope, as contrived rather than natural respellings: indirection through a +#: shell variable, `git -C`, UV_PROJECT in env, and writes to $GITHUB_ENV. FORBIDDEN_MENTIONS = ( re.compile(rf"(? bool: + """Whether a working directory is the PR checkout or below it, however it is spelt.""" + parts = PurePosixPath(str(directory or ".")).parts + return parts[:1] == (PR_DIR,) + + @pytest.fixture(scope="module") def workflow() -> dict[Any, Any]: """The parsed workflow file.""" @@ -72,17 +83,25 @@ def test_the_root_checkout_is_the_default_branch(steps: list[dict[str, Any]]) -> def test_only_the_base_fetch_runs_inside_the_pr_checkout(steps: list[dict[str, Any]]) -> None: """Exactly one fixed git command runs in pr/; `git fetch … && uv sync` must not pass.""" - inside = [s for s in steps if s.get("working-directory") == PR_DIR] - assert [str(s["run"]).strip() for s in inside] == [PR_FETCH] + inside = [s for s in steps if _in_pr(s.get("working-directory"))] + assert [str(s.get("run", "")).strip() for s in inside] == [PR_FETCH] assert not any(_holds_secret(s) for s in inside) +def test_no_action_runs_from_or_inside_the_pr_checkout(steps: list[dict[str, Any]]) -> None: + """A local action (`uses: ./…`) is checkout code; an action aimed at pr/ reads its config.""" + for step in steps: + assert not str(step.get("uses", "")).startswith("./"), step.get("name") + if "uses" in step: + assert not _in_pr(_with(step).get("working-directory")), step.get("name") + + def test_no_command_reaches_into_the_pr_checkout(steps: list[dict[str, Any]]) -> None: """`uv sync --directory pr` or `python pr/scripts/…` at the root: the same leak, respelt.""" for step in steps: run = str(step.get("run", "")) + " " for allowed in ALLOWED_MENTIONS: - run = run.replace(allowed, " ") + run = allowed.sub(" ", run) for pattern in FORBIDDEN_MENTIONS: assert not pattern.search(run), (step.get("name"), pattern.pattern) @@ -93,7 +112,7 @@ def test_the_review_runs_this_repositorys_script_on_the_pr(steps: list[dict[str, assert [s["name"] for s in review] == ["Run Guardian Review"] run = str(review[0]["run"]) + " " assert "uv run --frozen python scripts/guardian_review.py" in run - assert f"--repo-root {PR_DIR} " in run + assert ALLOWED_MENTIONS[1].search(run) assert "working-directory" not in review[0] From 0a2bddf9a34d738d39ad92e1015cd1dcd3147fbc Mon Sep 17 00:00:00 2001 From: zaebee Date: Thu, 22 Oct 2026 09:39:16 +0000 Subject: [PATCH 5/5] fix(guardian): contained() returns the path it checked, and refuses a null byte From Gemini's second review of #731. The caller now reads the resolved path that passed the check rather than a spelling that resolves again, and a path with a null byte (ValueError from resolve) reads as absent instead of crashing the review. Co-Authored-By: Claude Opus 5.5 --- src/cgis/guardian/collector.py | 10 +++++----- tests/unit/test_guardian_collector.py | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/cgis/guardian/collector.py b/src/cgis/guardian/collector.py index a5e6d816..305ce0cc 100644 --- a/src/cgis/guardian/collector.py +++ b/src/cgis/guardian/collector.py @@ -68,7 +68,7 @@ def features_setting(env: Mapping[str, str]) -> tuple[frozenset[str], FeaturesSo def contained(root: Path, relative_path: str) -> Path | None: - """`root / relative_path` when it resolves inside `root`, else None. + """`root / relative_path`, resolved, when it lies inside `root`; else None. The reviewed checkout is a PR, and a PR is data its author controls (#730). A symlink or a `..` is data that reads somewhere else: a CONTRIBUTING.md @@ -76,11 +76,11 @@ def contained(root: Path, relative_path: str) -> Path | None: prompt, and from there in a public review comment. `cgis ingest` refuses such links the same way. A link that stays inside the checkout is allowed. """ - path = root / relative_path try: - if path.resolve().is_relative_to(root.resolve()): - return path - except (OSError, RuntimeError): # a loop of links + resolved = (root / relative_path).resolve() + if resolved.is_relative_to(root.resolve()): + return resolved + except (OSError, RuntimeError, ValueError): # a loop of links, or a null byte return None return None diff --git a/tests/unit/test_guardian_collector.py b/tests/unit/test_guardian_collector.py index bae2123f..cc158927 100644 --- a/tests/unit/test_guardian_collector.py +++ b/tests/unit/test_guardian_collector.py @@ -8,7 +8,7 @@ from cgis.core.models import Edge, EdgeType, Node, NodeNamespace, NodeType from cgis.guardian.chunker import Chunk -from cgis.guardian.collector import ContextCollector, parse_features +from cgis.guardian.collector import ContextCollector, contained, parse_features from cgis.languages import extractors from cgis.pipeline import IngestionPipeline from cgis.storage.sqlite_store import SQLiteStore @@ -604,3 +604,16 @@ def test_reads_survive_bytes_that_are_not_utf8(tmp_path: Path) -> None: assert collector.source.read_file("CONTRIBUTING.md").startswith("rules ") with patch.object(collector.source, "get_changed_source_files", return_value=["bad.py"]): assert "#### `bad.py`" in collector.source.collect_full_files() + + +def test_read_file_refuses_a_path_with_a_null_byte(tmp_path: Path) -> None: + """A path the OS cannot even name reads as absent, not as a crash.""" + collector = ContextCollector(project_root=tmp_path) + assert collector.source.read_file("CONTRIBUTING\0.md") == "" + + +def test_contained_returns_the_resolved_path(tmp_path: Path) -> None: + """What is read is the path that was checked, not a spelling that resolves again later.""" + (tmp_path / "real.md").write_text("x\n") + (tmp_path / "alias.md").symlink_to(tmp_path / "real.md") + assert contained(tmp_path, "alias.md") == (tmp_path / "real.md").resolve()