feat(agent-team): P3-flip Phase 1 — gate-weakening detector (§4.5)
A diff that ADDS a lint/type/coverage/security suppression (noqa, type: ignore, pragma: no cover, nosec, nosemgrep), a test skip/xfail, or a hook bypass (--no-verify) could make CI pass falsely. The pure-code gate now flags these via gate_weakening_violations() and BLOCKs in evaluate_ci_gate as a top-priority trust violation (step 1b, alongside the denylist) — regardless of the authenticated CI conclusion. A build cannot pass itself by disabling its own checks; flagged diffs escalate to a human. Only ADDED lines are inspected (removing a suppression is fine). 1015 tests pass, ruff clean.
This commit is contained in:
parent
e77ec6514f
commit
07de7a4b24
2 changed files with 142 additions and 0 deletions
|
|
@ -71,6 +71,7 @@ __all__ = [
|
|||
"denylist_violations",
|
||||
"diff_touched_paths",
|
||||
"evaluate_ci_gate",
|
||||
"gate_weakening_violations",
|
||||
"verify_diff_hash",
|
||||
]
|
||||
|
||||
|
|
@ -368,6 +369,63 @@ def _within_scope(path: str, scope: Sequence[str]) -> bool:
|
|||
return False
|
||||
|
||||
|
||||
# §4.5 gate-weakening: substrings whose appearance on an ADDED diff line means the
|
||||
# change is suppressing a quality/security gate (a self-passing move). Matched
|
||||
# case-insensitively against added-line content.
|
||||
_GATE_WEAKENING_MARKERS: tuple[tuple[str, str], ...] = (
|
||||
("noqa", "ruff/flake8 lint suppression (noqa)"),
|
||||
("type: ignore", "type-check suppression (type: ignore)"),
|
||||
("type:ignore", "type-check suppression (type:ignore)"),
|
||||
("pragma: no cover", "coverage suppression (pragma: no cover)"),
|
||||
("pragma: no-cover", "coverage suppression (pragma: no-cover)"),
|
||||
("nosec", "bandit security suppression (nosec)"),
|
||||
("nosemgrep", "semgrep suppression (nosemgrep)"),
|
||||
("--no-verify", "git/pre-commit hook bypass (--no-verify)"),
|
||||
)
|
||||
|
||||
# Test skip/xfail decorators and calls (carry args, so regex not substring).
|
||||
_TEST_SKIP_RE: re.Pattern[str] = re.compile(
|
||||
r"@(?:pytest\.mark\.(?:skip|xfail)|unittest\.skip\w*)\b"
|
||||
r"|\bpytest\.(?:skip|xfail)\s*\("
|
||||
r"|\.skipTest\s*\("
|
||||
)
|
||||
|
||||
|
||||
def _added_diff_lines(unified_diff: str) -> list[str]:
|
||||
"""Return the content of lines ADDED by a unified diff (excluding ``+++`` headers)."""
|
||||
if not isinstance(unified_diff, str):
|
||||
raise CiGateError(
|
||||
f"unified_diff must be str, got {type(unified_diff).__name__}"
|
||||
)
|
||||
return [
|
||||
line[1:]
|
||||
for line in unified_diff.splitlines()
|
||||
if line.startswith("+") and not line.startswith("+++")
|
||||
]
|
||||
|
||||
|
||||
def gate_weakening_violations(unified_diff: str) -> list[str]:
|
||||
"""Return reasons the diff WEAKENS a quality/security gate (empty if none).
|
||||
|
||||
A diff that ADDS a lint/type/coverage/security suppression, a test
|
||||
skip/xfail, or a hook bypass could make CI pass falsely — so the pure-code
|
||||
gate flags it even when the authenticated CI conclusion is ``success``
|
||||
(§4.5). Such diffs are escalated to a human, never auto-built. Only ADDED
|
||||
lines are inspected (removing a suppression is fine); matching is
|
||||
case-insensitive on the marker text.
|
||||
"""
|
||||
violations: list[str] = []
|
||||
for content in _added_diff_lines(unified_diff):
|
||||
low = content.lower()
|
||||
snippet = content.strip()[:120]
|
||||
for marker, desc in _GATE_WEAKENING_MARKERS:
|
||||
if marker in low:
|
||||
violations.append(f"gate-weakening: added {desc}: {snippet!r}")
|
||||
if _TEST_SKIP_RE.search(content):
|
||||
violations.append(f"gate-weakening: added a test skip/xfail: {snippet!r}")
|
||||
return violations
|
||||
|
||||
|
||||
def verify_diff_hash(
|
||||
candidate_diff: str,
|
||||
*,
|
||||
|
|
@ -464,6 +522,21 @@ def evaluate_ci_gate(
|
|||
ci_conclusion=None,
|
||||
)
|
||||
|
||||
# (1b) Gate-weakening (§4.5): a diff that suppresses a lint/type/coverage/
|
||||
# security check or skips tests could make CI pass falsely -> BLOCK regardless
|
||||
# of the authenticated conclusion. A build cannot pass itself by disabling the
|
||||
# checks; these escalate to a human.
|
||||
weakening = gate_weakening_violations(candidate_diff)
|
||||
if weakening:
|
||||
reasons.extend(weakening)
|
||||
return GateResult(
|
||||
decision=GateDecision.BLOCK,
|
||||
reasons=reasons,
|
||||
run_id=expected_run_id,
|
||||
diff_hash=ledger_hash,
|
||||
ci_conclusion=None,
|
||||
)
|
||||
|
||||
# (2) Diff-hash integrity: bind the decision to the exact bytes the ledger
|
||||
# recorded (and, if present, what CI verified before applying).
|
||||
ci_verified_hash = (
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ from agent_team.ci_gate import (
|
|||
denylist_violations,
|
||||
diff_touched_paths,
|
||||
evaluate_ci_gate,
|
||||
gate_weakening_violations,
|
||||
verify_diff_hash,
|
||||
)
|
||||
from agent_team.state_store import compute_content_hash
|
||||
|
|
@ -411,3 +412,71 @@ def test_gate_result_carries_provenance() -> None:
|
|||
assert result.run_id == "run-7"
|
||||
assert result.diff_hash == h
|
||||
assert result.reasons # always records why
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# §4.5 gate-weakening detector
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
|
||||
def _diff_adding(added_line: str, path: str = "src/mod.py") -> str:
|
||||
"""A unified diff that ADDS one line (plus an unchanged context line)."""
|
||||
return (
|
||||
f"diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n"
|
||||
f"@@ -1 +1,2 @@\n unchanged\n+{added_line}\n"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"added_line",
|
||||
[
|
||||
"x = 1 # noqa",
|
||||
"y = 2 # noqa: E501",
|
||||
"z = bad() # type: ignore",
|
||||
"z = bad() # type:ignore[arg-type]",
|
||||
"def f(): # pragma: no cover",
|
||||
"def f(): # pragma: no-cover",
|
||||
"subprocess.run(cmd) # nosec",
|
||||
"eval(x) # nosemgrep",
|
||||
" git commit --no-verify",
|
||||
"@pytest.mark.skip(reason='flaky')",
|
||||
"@pytest.mark.xfail",
|
||||
" pytest.skip('todo')",
|
||||
" self.skipTest('later')",
|
||||
],
|
||||
)
|
||||
def test_gate_weakening_added_line_flagged(added_line: str) -> None:
|
||||
violations = gate_weakening_violations(_diff_adding(added_line))
|
||||
assert violations, f"expected {added_line!r} to be flagged as gate-weakening"
|
||||
|
||||
|
||||
def test_gate_weakening_clean_diff_has_no_violations() -> None:
|
||||
assert gate_weakening_violations(_diff_for("src/foo.py")) == []
|
||||
|
||||
|
||||
def test_gate_weakening_only_inspects_added_lines() -> None:
|
||||
# A diff that REMOVES a suppression (line starts with '-') must not be flagged.
|
||||
removing = (
|
||||
"diff --git a/m.py b/m.py\n--- a/m.py\n+++ b/m.py\n"
|
||||
"@@ -1,2 +1 @@\n-x = 1 # noqa\n unchanged\n"
|
||||
)
|
||||
assert gate_weakening_violations(removing) == []
|
||||
|
||||
|
||||
def test_gate_weakening_rejects_non_str() -> None:
|
||||
with pytest.raises(CiGateError):
|
||||
gate_weakening_violations(None) # type: ignore[arg-type]
|
||||
|
||||
|
||||
def test_gate_blocks_weakening_diff_even_if_ci_success() -> None:
|
||||
# A diff that adds a coverage suppression BLOCKs even with a green CI run.
|
||||
diff = _diff_adding("def f(): # pragma: no cover")
|
||||
result = evaluate_ci_gate(
|
||||
candidate_diff=diff,
|
||||
ledger_hash=_ledger_hash(diff),
|
||||
ci_result=_good_ci("run-w", diff, "success"),
|
||||
expected_run_id="run-w",
|
||||
)
|
||||
assert result.decision is GateDecision.BLOCK
|
||||
assert result.blocked is True
|
||||
assert any("gate-weakening" in r for r in result.reasons)
|
||||
|
|
|
|||
Reference in a new issue