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

@@ -1,6 +1,10 @@
 {
-  "_comment": "Justified security suppressions. Every inline `# nosec S<n>` / `# nosec B<n>` marker in src/gaia MUST have a matching entry here with a real justification. Do not blanket-suppress to quiet the scanner.",
+  "_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"},
     {
       "path": "src/gaia/agents/tools/shell_tools.py",
       "rule": "B602",

tests/unit/test_check_security_gates.py

@@ -1,19 +1,29 @@
 # Copyright(C) 2025-2026 Advanced Micro Devices, Inc. All rights reserved.
 # SPDX-License-Identifier: MIT
 
-"""Unit tests for the bandit HIGH security gate (util/check_security_gates.py)."""
+"""Unit tests for util/check_security_gates.py.
+
+Covers both gates the module ships:
+
+1. The suppression review gate — 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.
+2. The bandit HIGH gate — finding keys, the baseline diff, JSON parsing, and the
+   invariant that the shipped baseline stays empty.
+"""
 
 import json
 import sys
 from pathlib import Path
 
 import pytest
 
-# util/ is not a package; add it to the path so we can import the module.
-_UTIL_DIR = Path(__file__).resolve().parents[2] / "util"
-sys.path.insert(0, str(_UTIL_DIR))
+REPO_ROOT = Path(__file__).resolve().parents[2]
+sys.path.insert(0, str(REPO_ROOT / "util"))
 
 import check_security_gates  # noqa: E402
+import check_security_gates as gates  # noqa: E402
 from check_security_gates import (  # noqa: E402
     bandit_finding_key,
     load_baseline,
@@ -23,6 +33,12 @@
 )
 
 
+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")
+
+
 def _finding(path, test_id, severity="HIGH", line=10):
     return {
         "filename": path,
@@ -33,6 +49,130 @@ def _finding(path, test_id, severity="HIGH", line=10):
     }
 
 
+# ===========================================================================
+# Suppression review gate
+# ===========================================================================
+
+
+# ---------------------------------------------------------------------------
+# 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
+
+
+# ===========================================================================
+# Bandit HIGH gate
+# ===========================================================================
+
+
 class TestBanditFindingKey:
     def test_key_is_path_and_test_id(self):
         f = _finding("src/gaia/util.py", "B602", line=42)