ci(coverage): enforce the critical core contract

.gitea/workflows/update-badges.yml

@@ -33,19 +33,28 @@ jobs:
       - name: Run coverage and extract percentage
         id: coverage
         run: |
-          python3 -m coverage run -m unittest discover -t . -s tests/unit > /dev/null 2>&1 || true
+          set -euo pipefail
-          PERCENT=$(python3 -m coverage report 2>/dev/null | grep '^TOTAL' | grep -oP '\d+(?=%)' | tail -1)
+          # Never publish a badge from a failed or partial test run.
+          python3 -m coverage run -m unittest discover -t . -s tests/unit
+          REPORT=$(python3 -m coverage report)
+          printf '%s\n' "$REPORT"
+          PERCENT=$(printf '%s\n' "$REPORT" | awk '$1 == "TOTAL" {gsub("%", "", $NF); print $NF}')
+          test -n "$PERCENT"
           echo "percent=$PERCENT" >> $GITHUB_OUTPUT
           echo "Coverage: $PERCENT%"
 
       - name: Extract core (critical-module) coverage percentage
         id: core_coverage
         run: |
+          set -euo pipefail
           # Reuses the .coverage data from the previous step. The core list is
-          # the single source of truth in scripts/critical-modules.txt; every
+          # validated single source of truth. Fail if a listed path disappeared
-          # core module is unit-tested, so the unit-only run is accurate for it.
+          # or if the measured core falls below ADR 0004's 90% minimum.
-          INCLUDE=$(grep -vE '^[[:space:]]*(#|$)' scripts/critical-modules.txt | paste -sd, -)
+          INCLUDE=$(python3 scripts/critical_modules.py)
-          PERCENT=$(python3 -m coverage report --include="$INCLUDE" 2>/dev/null | grep '^TOTAL' | grep -oP '\d+(?=%)' | tail -1)
+          REPORT=$(python3 -m coverage report --include="$INCLUDE" --fail-under=90)
+          printf '%s\n' "$REPORT"
+          PERCENT=$(printf '%s\n' "$REPORT" | awk '$1 == "TOTAL" {gsub("%", "", $NF); print $NF}')
+          test -n "$PERCENT"
           echo "percent=$PERCENT" >> $GITHUB_OUTPUT
           echo "Core coverage: $PERCENT%"
 

docs/decisions/0004-coverage-policy.md

@@ -34,12 +34,13 @@ a regression (Goodhart's law).
 Coverage is **risk-weighted**, measured over the **combined unit +
 integration** suites, with three rules:
 
-1. **Critical modules target ≥ 90%.** The security/logic core —
+1. **Critical modules must remain ≥ 90%.** The curated security/logic core
-   `egress_addon{,_core}.py`, `dlp_detectors.py`, `egress.py`,
+   covers the host and gateway egress policy, manifest trust boundary,
-   `manifest*.py`, `git_gate.py`, `git_http_backend.py`, `supervise.py`,
+   git-gate enforcement, supervise protocol/server, YAML parser, and bottle
-   `yaml_subset.py`, `bottle_state.py` — is Docker-independent and
+   state. The concrete module list lives in `scripts/critical-modules.txt`;
-   unit-testable, so it carries the high bar. We ratchet toward 90% as
+   `scripts/critical_modules.py` rejects stale or ambiguous entries before
-   these modules are touched; new gaps in them are not acceptable.
+   Coverage.py can silently ignore them. These modules are unit-testable, so
+   CI enforces the aggregate minimum independently of diff coverage.
 
 2. **Subprocess/backend orchestration is covered by the integration
    suite, not omitted.** `scripts/coverage.sh` runs unit + integration
@@ -82,6 +83,9 @@ omit list.
   (critical-module standard + diff coverage) are Docker-independent.
 - "We're at N%" is now a curated figure; outsiders should read the
   policy, not just the badge.
+- A rename or removal in the curated list fails CI. Updating the list is an
+  explicit review of where the security-critical behavior moved, not a way to
+  improve the percentage by omission.
 
 ## Links
 

scripts/coverage.sh

@@ -20,10 +20,10 @@ cd "$(dirname "$0")/.."
 
 PY="${PYTHON:-python3}"
 
-# Critical security/logic core held to the high bar by ADR 0004. The list
+# Critical security/logic core held to the high bar by ADR 0004. The helper
-# lives in one place (scripts/critical-modules.txt) so this report and the
+# fails before coverage when a curated path was renamed or removed; Coverage.py
-# README "core coverage" badge can't drift; comma-join it for --include.
+# itself would silently ignore that stale include and inflate the score.
-CRITICAL=$(grep -vE '^[[:space:]]*(#|$)' scripts/critical-modules.txt | paste -sd, -)
+CRITICAL=$("$PY" scripts/critical_modules.py)
 
 if [ "${1:-}" = "aggregate" ]; then
     # Aggregate mode: combine .coverage.* artifacts already in the workspace.
@@ -34,8 +34,8 @@ if [ "${1:-}" = "aggregate" ]; then
     "$PY" -m coverage report -m
 
     if [ "${2:-}" = "critical" ]; then
-        echo "== critical modules (ADR 0004 target: 90%) ==" >&2
+        echo "== critical modules (ADR 0004 minimum: 90%) ==" >&2
-        "$PY" -m coverage report --include="$CRITICAL"
+        "$PY" -m coverage report --include="$CRITICAL" --fail-under=90
     fi
     exit 0
 fi
@@ -55,6 +55,6 @@ echo "== combined report ==" >&2
 "$PY" -m coverage report -m
 
 if [ "${1:-}" = "critical" ]; then
-    echo "== critical modules (ADR 0004 target: 90%) ==" >&2
+    echo "== critical modules (ADR 0004 minimum: 90%) ==" >&2
-    "$PY" -m coverage report --include="$CRITICAL"
+    "$PY" -m coverage report --include="$CRITICAL" --fail-under=90
 fi

scripts/critical-modules.txt

@@ -7,19 +7,48 @@
 # number that silently stops measuring a module is worse than no badge.
 #
 # One module path per line, relative to the repo root. Blank lines and
-# `#` comments are ignored.
+# `#` comments are ignored. scripts/critical_modules.py rejects missing,
+# duplicate, non-Python, and out-of-repository entries before coverage runs.
+
+# Host-side egress planning and secret preparation.
+bot_bottle/egress/plan.py
+bot_bottle/egress/service.py
+
+# Gateway egress policy, matching, and DLP enforcement.
 bot_bottle/gateway/egress/addon.py
 bot_bottle/gateway/egress/addon_core.py
+bot_bottle/gateway/egress/context.py
+bot_bottle/gateway/egress/dlp.py
+bot_bottle/gateway/egress/dlp_config.py
 bot_bottle/gateway/egress/dlp_detectors.py
-bot_bottle/egress.py
+bot_bottle/gateway/egress/matching.py
-bot_bottle/manifest.py
+bot_bottle/gateway/egress/schema.py
-bot_bottle/manifest_egress.py
+bot_bottle/gateway/egress/types.py
-bot_bottle/manifest_agent.py
+
-bot_bottle/manifest_schema.py
+# Manifest trust boundary and schema.
-bot_bottle/git_gate.py
+bot_bottle/manifest/agent.py
+bot_bottle/manifest/bottle.py
+bot_bottle/manifest/egress.py
+bot_bottle/manifest/extends.py
+bot_bottle/manifest/git.py
+bot_bottle/manifest/index.py
+bot_bottle/manifest/loader.py
+bot_bottle/manifest/schema.py
+bot_bottle/manifest/util.py
+
+# Host-side and gateway-side git policy enforcement.
+bot_bottle/git_gate/host_key.py
+bot_bottle/git_gate/plan.py
+bot_bottle/git_gate/provision.py
+bot_bottle/git_gate/service.py
 bot_bottle/gateway/git_gate/render.py
-bot_bottle/git_gate_provision.py
 bot_bottle/gateway/git_gate/http_backend.py
-bot_bottle/supervise.py
+
+# Supervise proposal protocol and data plane.
+bot_bottle/supervisor/plan.py
+bot_bottle/supervisor/types.py
+bot_bottle/gateway/supervisor/server.py
+
+# Shared parsers and state validation.
 bot_bottle/yaml_subset.py
 bot_bottle/bottle_state.py

scripts/critical_modules.py

@@ -0,0 +1,101 @@
+#!/usr/bin/env python3
+"""Validate and render the critical-module coverage manifest.
+
+Coverage.py silently ignores an ``--include`` path that does not exist.  That
+is useful for broad globs, but dangerous for bot-bottle's curated security
+core: a rename could otherwise improve the reported percentage by removing a
+module from the measurement.  Keep the validation in one small stdlib helper
+and make every coverage consumer call it.
+"""
+
+from __future__ import annotations
+
+import argparse
+import sys
+from pathlib import Path
+
+
+REPO_ROOT = Path(__file__).resolve().parents[1]
+DEFAULT_MANIFEST = REPO_ROOT / "scripts" / "critical-modules.txt"
+
+
+class CriticalModulesError(ValueError):
+    """The critical-module manifest is empty, ambiguous, or stale."""
+
+
+def load_critical_modules(manifest: Path, *, root: Path) -> list[str]:
+    """Return validated module paths relative to *root*.
+
+    Entries must be unique, concrete Python files inside the repository.
+    Globs are deliberately rejected by the file check: each rename must update
+    this explicit security review surface.
+    """
+
+    root = root.resolve()
+    try:
+        lines = manifest.read_text(encoding="utf-8").splitlines()
+    except OSError as exc:
+        raise CriticalModulesError(
+            f"cannot read critical-module manifest {manifest}: {exc}"
+        ) from exc
+
+    modules: list[str] = []
+    seen: set[str] = set()
+    errors: list[str] = []
+    for line_number, raw in enumerate(lines, start=1):
+        entry = raw.strip()
+        if not entry or entry.startswith("#"):
+            continue
+        path = Path(entry)
+        prefix = f"{manifest}:{line_number}: {entry!r}"
+        if path.is_absolute():
+            errors.append(f"{prefix} must be relative to the repository root")
+            continue
+        try:
+            resolved = (root / path).resolve()
+            resolved.relative_to(root)
+        except ValueError:
+            errors.append(f"{prefix} escapes the repository root")
+            continue
+        if entry in seen:
+            errors.append(f"{prefix} is duplicated")
+            continue
+        seen.add(entry)
+        if path.suffix != ".py":
+            errors.append(f"{prefix} is not a Python module")
+            continue
+        if not resolved.is_file():
+            errors.append(f"{prefix} does not exist")
+            continue
+        modules.append(path.as_posix())
+
+    if not modules and not errors:
+        errors.append(f"{manifest}: contains no critical modules")
+    if errors:
+        raise CriticalModulesError("\n".join(errors))
+    return modules
+
+
+def main(argv: list[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(
+        description="validate and print the critical coverage include list"
+    )
+    parser.add_argument("--manifest", type=Path, default=DEFAULT_MANIFEST)
+    parser.add_argument("--root", type=Path, default=REPO_ROOT)
+    parser.add_argument(
+        "--check", action="store_true",
+        help="validate only; do not print the comma-separated include list",
+    )
+    args = parser.parse_args(argv)
+    try:
+        modules = load_critical_modules(args.manifest, root=args.root)
+    except CriticalModulesError as exc:
+        print(f"critical-modules: {exc}", file=sys.stderr)
+        return 1
+    if not args.check:
+        print(",".join(modules))
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

tests/unit/test_critical_modules.py

@@ -0,0 +1,54 @@
+"""Tests for the fail-closed critical coverage manifest."""
+
+from __future__ import annotations
+
+import tempfile
+import unittest
+from pathlib import Path
+
+from scripts.critical_modules import (
+    DEFAULT_MANIFEST,
+    REPO_ROOT,
+    CriticalModulesError,
+    load_critical_modules,
+)
+
+
+class TestCriticalModules(unittest.TestCase):
+    def test_repository_manifest_is_valid(self) -> None:
+        modules = load_critical_modules(DEFAULT_MANIFEST, root=REPO_ROOT)
+        self.assertGreater(len(modules), 20)
+        self.assertEqual(len(modules), len(set(modules)))
+
+    def test_missing_module_fails(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            manifest = root / "critical-modules.txt"
+            manifest.write_text("bot_bottle/renamed.py\n", encoding="utf-8")
+            with self.assertRaisesRegex(CriticalModulesError, "does not exist"):
+                load_critical_modules(manifest, root=root)
+
+    def test_duplicate_module_fails(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            module = root / "bot_bottle" / "core.py"
+            module.parent.mkdir()
+            module.write_text("", encoding="utf-8")
+            manifest = root / "critical-modules.txt"
+            manifest.write_text(
+                "bot_bottle/core.py\nbot_bottle/core.py\n", encoding="utf-8"
+            )
+            with self.assertRaisesRegex(CriticalModulesError, "duplicated"):
+                load_critical_modules(manifest, root=root)
+
+    def test_entry_cannot_escape_repository(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            manifest = root / "critical-modules.txt"
+            manifest.write_text("../outside.py\n", encoding="utf-8")
+            with self.assertRaisesRegex(CriticalModulesError, "escapes"):
+                load_critical_modules(manifest, root=root)
+
+
+if __name__ == "__main__":
+    unittest.main()