Harden CI guard: symlink-escape reject, strict decode, scope canon; back with tests

Resolves the GPT-4.1 cross-review FIX items on the §3.3.2 CI apply/verify guard:
- Symlink-escape (Medium-High): reject any candidate diff that introduces a
  symlink (git mode 120000). A symlink can redirect a later in-diff write into a
  denied path that textual canonicalization cannot see; auto-built diffs have no
  legitimate symlinks, so this fails closed (exit 7).
- Diff-parse robustness (Medium): decode the diff as strict UTF-8 and fail closed
  (exit 8) instead of errors='replace', closing homoglyph/encoding evasion.
- Declared-scope canonicalization (Medium): drop parent-escaping scope globs so a
  malformed scope can only shrink coverage, never widen it past repo root.
- Egress allowlist (Low-Med): explicit DEPLOY marker to parameterize the
  build-test registries per target repo before enabling.

Backs the gate's correctness claim with a committed, runnable suite
(tests/test_ci_gate_workflow.py) that extracts the inline guard from the YAML and
exercises good + adversarial diffs (clean, hash mismatch, workflow delete,
copy-into-denied, symlink, non-UTF-8, out-of-scope, unscoped, escaping scope).
Corrects the README "Tests" section that claimed coverage that did not exist.
Full suite: 557 passed, 1 skipped; ruff clean.

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)
 

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 "<unknown>"
+                      except ValueError:
+                          path_c = (cur_path or "<unknown>").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

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