feat(lint): require security suppressions to be reviewed in an allowlist

.github/workflows/lint.yml

@@ -14,23 +14,27 @@ on:
     branches: [ main ]
     paths:
       - 'src/**'
+      - 'hub/**'
       - 'tests/**'
       - 'setup.py'
       - 'pyproject.toml'
       - 'util/lint.py'
       - 'util/check_*.py'
+      - '.security-suppressions.json'
       - '.github/dependabot.yml'
       - '.github/workflows/lint.yml'
   pull_request:
     branches: [ main ]
     types: [opened, synchronize, reopened, ready_for_review]
     paths:
       - 'src/**'
+      - 'hub/**'
       - 'tests/**'
       - 'setup.py'
       - 'pyproject.toml'
       - 'util/lint.py'
       - 'util/check_*.py'
+      - '.security-suppressions.json'
       - '.github/dependabot.yml'
       - '.github/workflows/lint.yml'
   merge_group:

.security-suppressions.json

@@ -0,0 +1,9 @@
+{
+  "_comment": "Approved security suppressions. Every '# noqa: S<n>' (flake8-bandit) and '# nosec' (bandit) comment in src/ or hub/ MUST be listed here with a justification, or CI fails (util/check_security_gates.py, run by util/lint.py). Adding an entry requires PR review — this is the gate that would have caught the GAIA hub tar-slip (CWE-22), where a '# noqa: S202 - hub artifacts are trusted' silenced an unvalidated tarfile.extractall. Keyed by (path, rule), not line number. See SECURITY.md.",
+  "suppressions": [
+    {"path": "src/gaia/eval/scorecard_gate.py", "rule": "S603", "justification": "git is a fixed, trusted executable; args are a constructed list, never shell-interpreted"},
+    {"path": "src/gaia/hub/installer.py", "rule": "S603", "justification": "'uv pip install' args are a constructed list (no user string), never shell-interpreted"},
+    {"path": "src/gaia/hub/native_launcher.py", "rule": "S603", "justification": "native binary path is an installed hub artifact (operator-controlled); args are constructed, not shell"},
+    {"path": "src/gaia/mcp/mcp_bridge.py", "rule": "B104", "justification": "binds 0.0.0.0 only on explicit caller opt-in (documented flag), never by default"}
+  ]
+}

tests/unit/test_check_security_gates.py

@@ -0,0 +1,139 @@
+# Copyright(C) 2025-2026 Advanced Micro Devices, Inc. All rights reserved.
+# SPDX-License-Identifier: MIT
+
+"""Unit tests for util/check_security_gates.py (the suppression review gate).
+
+Covers the scanner (which comment forms count as a security suppression), the
+allowlist loader (justification required), the violation diff, and a backtest
+proving the gate flags the exact suppression that hid the hub tar-slip.
+"""
+
+import sys
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).resolve().parents[2]
+sys.path.insert(0, str(REPO_ROOT / "util"))
+
+import check_security_gates as gates  # noqa: E402
+
+
+def _write(tmp_path: Path, rel: str, body: str) -> None:
+    p = tmp_path / rel
+    p.parent.mkdir(parents=True, exist_ok=True)
+    p.write_text(body, encoding="utf-8")
+
+
+# ---------------------------------------------------------------------------
+# find_suppressions
+# ---------------------------------------------------------------------------
+
+
+def test_detects_noqa_bandit_code(tmp_path):
+    _write(tmp_path, "src/a.py", "x = 1  # noqa: S202 - trust me\n")
+    found = gates.find_suppressions(tmp_path)
+    assert found == [{"path": "src/a.py", "rule": "S202", "line": 1}]
+
+
+def test_detects_nosec_with_and_without_code(tmp_path):
+    _write(
+        tmp_path,
+        "src/b.py",
+        "a()  # nosec B104 - opt-in\nb()  # nosec\n",
+    )
+    found = {(f["path"], f["rule"]) for f in gates.find_suppressions(tmp_path)}
+    assert ("src/b.py", "B104") in found
+    assert ("src/b.py", "nosec") in found
+
+
+def test_ignores_non_security_noqa_codes(tmp_path):
+    # SLF001 (flake8-self) and E402 are not bandit security codes — must be ignored.
+    _write(
+        tmp_path, "src/c.py", "z = obj._x  # noqa: SLF001\nimport os  # noqa: E402\n"
+    )
+    assert gates.find_suppressions(tmp_path) == []
+
+
+def test_scans_hub_not_just_src(tmp_path):
+    _write(tmp_path, "hub/agents/x/y.py", "run()  # nosec\n")
+    found = gates.find_suppressions(tmp_path)
+    assert found and found[0]["path"] == "hub/agents/x/y.py"
+
+
+def test_excludes_vendored_dirs(tmp_path):
+    _write(tmp_path, "src/node_modules/dep.py", "x = 1  # noqa: S202\n")
+    assert gates.find_suppressions(tmp_path) == []
+
+
+# ---------------------------------------------------------------------------
+# violations
+# ---------------------------------------------------------------------------
+
+
+def test_allowlisted_suppression_is_not_a_violation():
+    supp = [{"path": "src/a.py", "rule": "S603", "line": 5}]
+    allow = {("src/a.py", "S603")}
+    assert gates.suppression_violations(supp, allow) == []
+
+
+def test_unlisted_suppression_is_a_violation():
+    supp = [{"path": "src/a.py", "rule": "S202", "line": 5}]
+    assert gates.suppression_violations(supp, set()) == supp
+
+
+# ---------------------------------------------------------------------------
+# load_allowlist
+# ---------------------------------------------------------------------------
+
+
+def test_allowlist_requires_justification(tmp_path, monkeypatch):
+    f = tmp_path / ".security-suppressions.json"
+    f.write_text(
+        '{"suppressions": [{"path": "src/a.py", "rule": "S603"}]}', encoding="utf-8"
+    )
+    monkeypatch.setattr(gates, "SUPPRESSIONS_FILE", f)
+    with pytest.raises(ValueError, match="justification"):
+        gates.load_allowlist()
+
+
+def test_allowlist_loads_valid_entries(tmp_path, monkeypatch):
+    f = tmp_path / ".security-suppressions.json"
+    f.write_text(
+        '{"suppressions": [{"path": "src/a.py", "rule": "S603", '
+        '"justification": "constructed args"}]}',
+        encoding="utf-8",
+    )
+    monkeypatch.setattr(gates, "SUPPRESSIONS_FILE", f)
+    assert gates.load_allowlist() == {("src/a.py", "S603")}
+
+
+# ---------------------------------------------------------------------------
+# Backtest: the gate would have caught the hub tar-slip (CWE-22)
+# ---------------------------------------------------------------------------
+
+
+def test_backtest_flags_the_tarslip_suppression(tmp_path):
+    """The tar-slip shipped as `tf.extractall(...)  # noqa: S202 - hub artifacts
+    are trusted`. With that line present and NOT in the allowlist, the gate must
+    flag it — i.e. this gate would have forced review before it merged."""
+    _write(
+        tmp_path,
+        "src/gaia/hub/installer.py",
+        "with tarfile.open(p) as tf:\n"
+        "    tf.extractall(d)  # noqa: S202 - hub artifacts are trusted\n",
+    )
+    found = gates.find_suppressions(tmp_path)
+    violations = gates.suppression_violations(found, allowlist=set())
+    assert any(v["rule"] == "S202" for v in violations)
+
+
+# ---------------------------------------------------------------------------
+# The real repo passes its own gate
+# ---------------------------------------------------------------------------
+
+
+def test_repo_suppressions_all_reviewed():
+    """Every real suppression in the tree is in the committed allowlist."""
+    ok, _msgs = gates.check_suppressions()
+    assert ok is True

util/check_security_gates.py

@@ -0,0 +1,176 @@
+# Copyright(C) 2025-2026 Advanced Micro Devices, Inc. All rights reserved.
+# SPDX-License-Identifier: MIT
+
+"""Security suppression review gate.
+
+Every ``# noqa: S<n>`` (flake8-bandit) and ``# nosec`` (bandit) comment under
+``src/`` and ``hub/`` must be listed in ``.security-suppressions.json`` with a
+justification. A suppression not listed there fails, forcing a human to review and
+justify it in the PR. This is the gate that would have caught the GAIA hub
+tar-slip (CWE-22): a ``# noqa: S202 - hub artifacts are trusted`` silenced an
+unvalidated ``tarfile.extractall`` from both bandit and the weekly audit.
+
+Entries key on ``(path, rule)`` — never a line number — so the allowlist survives
+edits instead of re-firing on every line shift.
+
+Runs as part of ``python util/lint.py --all`` (or ``--security``). Run directly
+with ``python util/check_security_gates.py``.
+
+NOTE: a companion "new bandit HIGH" gate (grandfathering today's findings via a
+baseline, then failing on genuinely new HIGH findings) is a planned follow-up,
+enabled once the pre-existing HIGH findings are fixed.
+"""
+
+from __future__ import annotations
+
+import json
+import re
+import sys
+from pathlib import Path
+from typing import Any, Dict, List, Tuple
+
+# Anchor to the repo root so the script works regardless of CWD — matches
+# util/check_dependabot.py and util/check_doc_versions.py.
+REPO_ROOT = Path(__file__).resolve().parents[1]
+SUPPRESSIONS_FILE = REPO_ROOT / ".security-suppressions.json"
+
+# Directories scanned for suppressions. Security suppressions can hide a finding
+# anywhere shippable code lives — core (src/) and the hub agents (hub/).
+SCAN_DIRS = ("src", "hub")
+EXCLUDE_PARTS = {
+    "node_modules",
+    ".venv",
+    "venv",
+    "__pycache__",
+    ".mypy_cache",
+    "build",
+    "dist",
+    "site-packages",
+    ".backup",
+}
+
+# A flake8-bandit security code is 'S' + exactly three digits (S101..S704).
+# This deliberately excludes non-security 'S'-prefixed codes like SLF001
+# (flake8-self) or SIM/… which are style, not security.
+_BANDIT_NOQA_CODE = re.compile(r"S\d{3}")
+_NOQA = re.compile(r"#\s*noqa:\s*([A-Za-z0-9,\s]+)")
+_ALL_CODES = re.compile(r"[A-Za-z]+\d+")
+_NOSEC = re.compile(r"#\s*nosec\b(.*)")
+_NOSEC_BCODE = re.compile(r"B\d{3}")
+
+
+def _iter_python_files(root: Path):
+    for scan in SCAN_DIRS:
+        base = root / scan
+        if not base.exists():
+            continue
+        for path in base.rglob("*.py"):
+            if EXCLUDE_PARTS & set(path.parts):
+                continue
+            yield path
+
+
+def find_suppressions(root: Path | None = None) -> List[Dict[str, Any]]:
+    """Return every security suppression found under the scanned dirs.
+
+    Each item is ``{"path": <repo-relative posix>, "rule": <code>, "line": <n>}``.
+    ``rule`` is a flake8-bandit ``S<n>`` code, a bandit ``B<n>`` code, or the
+    literal ``"nosec"`` for a bare ``# nosec`` with no explicit code.
+    """
+    root = root or REPO_ROOT
+    found: List[Dict[str, Any]] = []
+    for path in _iter_python_files(root):
+        rel = path.relative_to(root).as_posix()
+        try:
+            lines = path.read_text(encoding="utf-8").splitlines()
+        except (OSError, UnicodeDecodeError):
+            continue
+        for lineno, line in enumerate(lines, start=1):
+            noqa = _NOQA.search(line)
+            if noqa:
+                for code in _ALL_CODES.findall(noqa.group(1)):
+                    if _BANDIT_NOQA_CODE.fullmatch(code):
+                        found.append({"path": rel, "rule": code, "line": lineno})
+            nosec = _NOSEC.search(line)
+            if nosec:
+                bcodes = _NOSEC_BCODE.findall(nosec.group(1))
+                if bcodes:
+                    for b in bcodes:
+                        found.append({"path": rel, "rule": b, "line": lineno})
+                else:
+                    found.append({"path": rel, "rule": "nosec", "line": lineno})
+    return found
+
+
+def load_allowlist() -> set[Tuple[str, str]]:
+    """Load the approved ``(path, rule)`` suppression pairs."""
+    if not SUPPRESSIONS_FILE.exists():
+        raise FileNotFoundError(
+            f"{SUPPRESSIONS_FILE} is missing. It is the allowlist of approved "
+            f"security suppressions; the security gate cannot run without it."
+        )
+    try:
+        data = json.loads(SUPPRESSIONS_FILE.read_text(encoding="utf-8"))
+    except json.JSONDecodeError as exc:
+        raise ValueError(f"{SUPPRESSIONS_FILE} is not valid JSON: {exc}") from exc
+    allow: set[Tuple[str, str]] = set()
+    for entry in data.get("suppressions", []):
+        path = entry.get("path")
+        rule = entry.get("rule")
+        if not path or not rule:
+            raise ValueError(
+                f"{SUPPRESSIONS_FILE}: every suppression needs a 'path' and 'rule' "
+                f"(offending entry: {entry!r})."
+            )
+        if not entry.get("justification"):
+            raise ValueError(
+                f"{SUPPRESSIONS_FILE}: suppression {path}:{rule} has no "
+                f"'justification' — every approved suppression must say why."
+            )
+        allow.add((path, rule))
+    return allow
+
+
+def suppression_violations(
+    suppressions: List[Dict[str, Any]], allowlist: set[Tuple[str, str]]
+) -> List[Dict[str, Any]]:
+    """Return suppressions whose ``(path, rule)`` is not in the allowlist."""
+    return [s for s in suppressions if (s["path"], s["rule"]) not in allowlist]
+
+
+def check_suppressions() -> Tuple[bool, List[str]]:
+    """Run the suppression gate. Returns ``(ok, messages)``."""
+    suppressions = find_suppressions()
+    allowlist = load_allowlist()
+    violations = suppression_violations(suppressions, allowlist)
+    msgs: List[str] = []
+    if not violations:
+        msgs.append(
+            f"[OK] {len(suppressions)} security suppression(s) - all reviewed in "
+            f"{SUPPRESSIONS_FILE.name}."
+        )
+        return True, msgs
+    msgs.append(
+        f"[FAIL] {len(violations)} security suppression(s) are NOT reviewed in "
+        f"{SUPPRESSIONS_FILE.name}:"
+    )
+    for v in violations:
+        msgs.append(f"    {v['path']}:{v['line']}  {v['rule']}")
+    msgs.append(
+        "  Every '# noqa: S<n>' / '# nosec' must be justified in "
+        f"{SUPPRESSIONS_FILE.name} so a human reviews it (this is the gate that "
+        "would have caught the hub tar-slip). Add an entry with a 'justification', "
+        "or remove the suppression and fix the finding."
+    )
+    return False, msgs
+
+
+def main() -> int:
+    ok, msgs = check_suppressions()
+    for m in msgs:
+        print(m)
+    return 0 if ok else 1
+
+
+if __name__ == "__main__":
+    sys.exit(main())