diff --git a/agent-team/ci/README.md b/agent-team/ci/README.md index 7652ff8..cf1e316 100644 --- a/agent-team/ci/README.md +++ b/agent-team/ci/README.md @@ -127,15 +127,15 @@ does not redefine them): ## Tests The workflow's embedded gate logic (diff integrity, the trust-control denylist -with path-canonicalization + rename detection, declared-scope enforcement, and -the pure-code pass/fail gate) is **stdlib-only, type-hinted, and ruff-clean**. -It was verified against good and adversarial diffs (workflow edits, IAM edits, -renames into denied paths, path traversal, out-of-scope and unscoped diffs, hash -mismatch, and every gate branch). Because this leaf owns only the two files in -this directory, the executable pytest suite for the importable foundation -modules lives in `../tests/` (run `python3 -m pytest agent-team/tests/ -q` from -the repo root); the inline CI gate logic is validated as part of the workflow's -own steps at deploy time and was proven correct during authoring. +with path-canonicalization + rename/copy/delete detection, the symlink-escape +reject, and declared-scope enforcement) is **stdlib-only, type-hinted, and +ruff-clean**, and is covered by a committed, runnable suite: +`../tests/test_ci_gate_workflow.py` extracts the inline guard script from this +YAML and executes it against good and adversarial diffs — clean in-scope, +hash mismatch, workflow delete, copy-into-denied, symlink addition, non-UTF-8, +out-of-scope, unscoped, and escaping-scope. Run it with the rest of the suite: +`python3 -m pytest agent-team/tests/ -q` from the repo root. (The claim that the +gate is "verified" is therefore backed by that test, not by authoring alone.) ## Deploy gating (do NOT skip) diff --git a/agent-team/ci/agent-team-apply-verify.yml b/agent-team/ci/agent-team-apply-verify.yml index 49626e8..623e6e3 100644 --- a/agent-team/ci/agent-team-apply-verify.yml +++ b/agent-team/ci/agent-team-apply-verify.yml @@ -214,13 +214,63 @@ jobs: touched.add(canonical(m.group(1).strip())) return touched + def find_symlink_additions(diff_text: str) -> list[tuple[str, str]]: + """Return ``[(path, target)]`` for every symlink the diff creates. + + A symlink shows as git file mode ``120000``; its link target is the + single added content line. Textual path canonicalization (``canonical``) + cannot see a symlink that redirects a later in-diff write into a denied + location (e.g. ``sub/link -> ../.github/workflows`` then a write to + ``sub/link/evil.yml``). A candidate auto-build diff has no legitimate + reason to introduce a symlink, so guard treats ANY symlink addition as + a hard reject (boundary 2), closing the symlink-escape vector. + """ + additions: list[tuple[str, str]] = [] + cur_path: str | None = None + pending = False + for line in diff_text.splitlines(): + g = re.match(r"^diff --git (\S+) (\S+)$", line) + if g: + cur_path, pending = g.group(2), False + continue + p = re.match(r"^\+\+\+ (.+)$", line) + if p and p.group(1).strip() != "/dev/null": + cur_path = p.group(1).split("\t", 1)[0].strip() + continue + if re.match(r"^(?:new file mode|new mode) 120000\s*$", line): + pending = True + continue + if pending and line.startswith("+") and not line.startswith("+++"): + try: + path_c = canonical(cur_path) if cur_path else "" + except ValueError: + path_c = (cur_path or "").lstrip("ab/") + additions.append((path_c, line[1:].strip())) + pending = False + return additions + def denied(path: str) -> bool: """True if ``path`` matches any trust-control denylist glob.""" return any(fnmatch.fnmatch(path, g) for g in DENY_GLOBS) + def safe_scope(scope: list[str]) -> list[str]: + """Canonicalize declared-scope globs, dropping any that escape root. + + An absolute or parent-escaping scope entry is discarded rather than + trusted, so a malformed scope can only SHRINK what is allowed, never + widen it past the repo root (mirrors the box-side normalizer). + """ + safe: list[str] = [] + for g in scope: + try: + safe.append(canonical(g)) + except ValueError: + continue + return safe + def in_scope(path: str, scope: list[str]) -> bool: """True if ``path`` is covered by the declared-scope globs.""" - return any(fnmatch.fnmatch(path, canonical(g)) for g in scope) + return any(fnmatch.fnmatch(path, g) for g in scope) def main() -> int: diff_path = os.environ["DIFF_PATH"] @@ -236,7 +286,16 @@ jobs: print(f"::error::diff hash mismatch: expected={expected} actual={actual}") return 2 - touched = parse_touched_paths(raw.decode("utf-8", errors="replace")) + # Fail CLOSED on a non-UTF-8 diff rather than silently replacing bytes + # (errors='replace' could let a homoglyph/encoding trick evade the path + # match). A legitimate diff over source is valid UTF-8. + try: + text = raw.decode("utf-8") + except UnicodeDecodeError as exc: + print(f"::error::diff is not valid UTF-8 ({exc}); refusing to parse") + return 8 + + touched = parse_touched_paths(text) if not touched: print("::error::no paths parsed from diff; refusing empty/garbled diff") return 3 @@ -249,10 +308,24 @@ jobs: print("::error::diff touches the trust-control surface; escalate to human + GPT cross-review") return 4 + # Boundary 2a': symlink escape. A symlink can redirect a later in-diff + # write into a denied path that textual matching cannot see, so any + # symlink addition is rejected outright. + symlinks = find_symlink_additions(text) + if symlinks: + for path, target in symlinks: + print(f"::error::diff introduces a symlink ({path} -> {target}); symlinks can redirect writes into denied paths and are not allowed in an auto-built diff") + print("::error::symlink in candidate diff; escalate to human + GPT cross-review") + return 7 + # Boundary 2b: declared-scope enforcement. if not scope: print("::error::no declared scope provided; refusing unscoped diff") return 5 + scope = safe_scope(scope) + if not scope: + print("::error::declared scope has no valid (non-escaping) entries; refusing diff") + return 5 out_of_scope = sorted(p for p in touched if not in_scope(p, scope)) if out_of_scope: for p in out_of_scope: @@ -290,8 +363,11 @@ jobs: with: # Block, not audit: this is where untrusted patch code executes. A # narrow allowlist for dependency resolution only; everything else is - # denied so a prompt-injected patch cannot phone home. Tighten/extend - # per the target repo's package registries at deploy time. + # denied so a prompt-injected patch cannot phone home. + # DEPLOY: this allowlist is GitHub + PyPI only. Before enabling this + # workflow for a repo, replace/extend it with EXACTLY that repo's + # package registries (npm, crates, Go proxy, ...) and nothing more — + # an over-broad allowlist weakens the egress boundary. egress-policy: block allowed-endpoints: > github.com:443 diff --git a/agent-team/tests/test_ci_gate_workflow.py b/agent-team/tests/test_ci_gate_workflow.py new file mode 100644 index 0000000..232ab56 --- /dev/null +++ b/agent-team/tests/test_ci_gate_workflow.py @@ -0,0 +1,141 @@ +"""Tests for the embedded guard logic in ``ci/agent-team-apply-verify.yml``. + +The §3.3.2 trust-boundary guard (diff-integrity hash, the trust-control-surface +denylist, the symlink-escape reject, declared-scope enforcement) lives as an +inline Python heredoc inside the CI workflow, so it cannot be imported directly. +These tests extract that script from the YAML and execute it as the workflow +does — via env vars and a diff file — asserting the exit code for good and +adversarial diffs. This backs the workflow's correctness claim with a real, +runnable suite instead of an "verified during authoring" assertion, and guards +the symlink / non-UTF-8 / header-only-section fixes against regression. + +It does NOT enable, provision, or run the workflow itself; it only exercises the +pure-code gate the workflow embeds. +""" + +from __future__ import annotations + +import hashlib +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +_WORKFLOW = Path(__file__).resolve().parents[1] / "ci" / "agent-team-apply-verify.yml" + + +def _extract_guard_script() -> str: + """Pull the first ``python3 - <<'PY' ... PY`` heredoc (the guard) from the YAML. + + The body is indented to sit under the YAML ``run:`` block; we strip the + common 10-space lead so it is valid module source. + """ + lines = _WORKFLOW.read_text(encoding="utf-8").splitlines() + start = end = None + for i, line in enumerate(lines): + if start is None and line.strip() == "python3 - <<'PY'": + start = i + 1 + elif start is not None and line.strip() == "PY": + end = i + break + assert start is not None and end is not None, "guard heredoc not found" + body = lines[start:end] + return "\n".join(ln[10:] if ln.startswith(" " * 10) else ln for ln in body) + + +@pytest.fixture(scope="module") +def guard_script(tmp_path_factory: pytest.TempPathFactory) -> Path: + path = tmp_path_factory.mktemp("guard") / "guard.py" + path.write_text(_extract_guard_script(), encoding="utf-8") + return path + + +def _run_guard( + guard_script: Path, + tmp_path: Path, + diff: str | bytes, + scope: str, + *, + bad_hash: bool = False, +) -> int: + raw = diff.encode("utf-8") if isinstance(diff, str) else diff + diff_path = tmp_path / "candidate.diff" + diff_path.write_bytes(raw) + expected = "deadbeef" if bad_hash else hashlib.sha256(raw).hexdigest() + env = dict( + os.environ, + DIFF_PATH=str(diff_path), + EXPECTED_DIFF_HASH=expected, + DECLARED_SCOPE=scope, + ) + result = subprocess.run( + [sys.executable, str(guard_script)], env=env, capture_output=True, text=True + ) + return result.returncode + + +CLEAN = ( + "diff --git a/src/app.py b/src/app.py\n" + "--- a/src/app.py\n+++ b/src/app.py\n@@ -1 +1 @@\n-x\n+y\n" +) +SYMLINK = ( + "diff --git a/src/link b/src/link\nnew file mode 120000\n" + "--- /dev/null\n+++ b/src/link\n@@ -0,0 +1 @@\n+../.github/workflows\n" +) +WORKFLOW_DELETE = ( + "diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml\n" + "deleted file mode 100644\n--- a/.github/workflows/ci.yml\n+++ /dev/null\n" + "@@ -1 +0,0 @@\n-on: push\n" +) +COPY_TO_DENIED = ( + "diff --git a/src/x.py b/.github/workflows/evil.yml\nsimilarity index 100%\n" + "copy from src/x.py\ncopy to .github/workflows/evil.yml\n" +) +NON_UTF8 = ( + b"diff --git a/src/app.py b/src/app.py\n--- a/src/app.py\n" + b"+++ b/src/app.py\n@@ -1 +1 @@\n-x\n+\xff\xfe\n" +) + + +def test_clean_in_scope_diff_passes(guard_script: Path, tmp_path: Path) -> None: + assert _run_guard(guard_script, tmp_path, CLEAN, "src/**") == 0 + + +def test_hash_mismatch_fails(guard_script: Path, tmp_path: Path) -> None: + assert _run_guard(guard_script, tmp_path, CLEAN, "src/**", bad_hash=True) == 2 + + +def test_workflow_delete_is_rejected(guard_script: Path, tmp_path: Path) -> None: + # Header-only section (delete) caught via diff --git, not a +++ body line. + assert ( + _run_guard(guard_script, tmp_path, WORKFLOW_DELETE, "src/**\n.github/**") == 4 + ) + + +def test_copy_into_denied_path_is_rejected(guard_script: Path, tmp_path: Path) -> None: + assert _run_guard(guard_script, tmp_path, COPY_TO_DENIED, "src/**\n.github/**") == 4 + + +def test_symlink_addition_is_rejected(guard_script: Path, tmp_path: Path) -> None: + # The symlink-escape vector: rejected outright (exit 7). + assert _run_guard(guard_script, tmp_path, SYMLINK, "src/**") == 7 + + +def test_non_utf8_diff_fails_closed(guard_script: Path, tmp_path: Path) -> None: + assert _run_guard(guard_script, tmp_path, NON_UTF8, "src/**") == 8 + + +def test_out_of_scope_path_is_rejected(guard_script: Path, tmp_path: Path) -> None: + assert _run_guard(guard_script, tmp_path, CLEAN, "other/**") == 6 + + +def test_unscoped_diff_is_rejected(guard_script: Path, tmp_path: Path) -> None: + assert _run_guard(guard_script, tmp_path, CLEAN, "") == 5 + + +def test_escaping_scope_entries_are_dropped(guard_script: Path, tmp_path: Path) -> None: + # A parent-escaping scope entry must not widen coverage; it is dropped, so a + # diff under it is treated as unscoped. + assert _run_guard(guard_script, tmp_path, CLEAN, "../../etc") == 5