security(reviewer): fail closed on unknown manifest evidence

reviewer/noema_reviewer/manifest.py

@@ -9,12 +9,18 @@
 
 from __future__ import annotations
 
-from pydantic import BaseModel, Field
+from pydantic import BaseModel, ConfigDict, Field
 
 from .models import Severity
 
 
-class DependencyFinding(BaseModel):
+class _StrictManifestModel(BaseModel):
+    """Fail closed when untrusted manifest evidence contains unknown fields."""
+
+    model_config = ConfigDict(extra="forbid")
+
+
+class DependencyFinding(_StrictManifestModel):
     """A dependency vulnerability surfaced by OSV, Trivy, or dependency-review."""
 
     tool: str = Field(description="Scanner that reported the finding (osv, trivy, dependency-review).")
@@ -29,7 +35,7 @@ class DependencyFinding(BaseModel):
     )
 
 
-class SecurityFinding(BaseModel):
+class SecurityFinding(_StrictManifestModel):
     """A current-head code-scanning or SARIF finding."""
 
     tool: str = Field(description="Scanner that produced the finding.")
@@ -41,7 +47,7 @@ class SecurityFinding(BaseModel):
     url: str = Field(default="", description="GitHub alert URL, when present.")
 
 
-class ReviewComment(BaseModel):
+class ReviewComment(_StrictManifestModel):
     """A prior review comment preserved so the reviewer never loses context."""
 
     author: str = Field(description="Comment author login.")
@@ -55,21 +61,21 @@ class ReviewComment(BaseModel):
     )
 
 
-class CheckConclusion(BaseModel):
+class CheckConclusion(_StrictManifestModel):
     """A current GitHub check conclusion used in the verdict."""
 
     name: str = Field(description="Check or status context name.")
     conclusion: str = Field(description="Conclusion such as success, failure, or neutral.")
 
 
-class ChangedFile(BaseModel):
+class ChangedFile(_StrictManifestModel):
     """A changed file's path plus bounded current-head content for context."""
 
     path: str = Field(description="Repository-relative path.")
     content: str = Field(default="", description="Bounded current-head text content.")
 
 
-class ReviewManifest(BaseModel):
+class ReviewManifest(_StrictManifestModel):
     """Everything a review driver is allowed to see for one pull request."""
 
     repo: str = Field(description="owner/name of the target repository.")

reviewer/tests/test_manifest.py

@@ -2,7 +2,17 @@
 
 from __future__ import annotations
 
-from noema_reviewer.manifest import DependencyFinding, ReviewManifest
+import pytest
+from pydantic import BaseModel, ValidationError
+
+from noema_reviewer.manifest import (
+    ChangedFile,
+    CheckConclusion,
+    DependencyFinding,
+    ReviewComment,
+    ReviewManifest,
+    SecurityFinding,
+)
 from noema_reviewer.models import BLOCKING_SEVERITIES, Severity
 
 
@@ -17,15 +27,56 @@ def test_unresolved_blocking_findings_filtered_by_severity_and_state() -> None:
         [
             DependencyFinding(tool="osv", package_name="a", severity=Severity.HIGH),
             DependencyFinding(tool="osv", package_name="b", severity=Severity.LOW),
-            DependencyFinding(tool="trivy", package_name="c", severity=Severity.CRITICAL, resolved=True),
+            DependencyFinding(
+                tool="trivy",
+                package_name="c",
+                severity=Severity.CRITICAL,
+                resolved=True,
+            ),
             DependencyFinding(tool="trivy", package_name="d", severity=Severity.MEDIUM),
         ]
     )
-    names = {finding.package_name for finding in manifest.unresolved_dependency_findings(BLOCKING_SEVERITIES)}
+    names = {
+        finding.package_name
+        for finding in manifest.unresolved_dependency_findings(BLOCKING_SEVERITIES)
+    }
     assert names == {"a", "d"}
 
 
 def test_no_blocking_findings_returns_empty() -> None:
     """A manifest with only low findings returns nothing blocking."""
-    manifest = _manifest_with([DependencyFinding(tool="osv", package_name="x", severity=Severity.INFO)])
+    manifest = _manifest_with(
+        [DependencyFinding(tool="osv", package_name="x", severity=Severity.INFO)]
+    )
     assert manifest.unresolved_dependency_findings(BLOCKING_SEVERITIES) == []
+
+
+@pytest.mark.parametrize(
+    ("model", "payload"),
+    (
+        (
+            DependencyFinding,
+            {"tool": "osv", "package_name": "pkg", "severity": Severity.HIGH},
+        ),
+        (
+            SecurityFinding,
+            {
+                "tool": "semgrep",
+                "identifier": "rule-id",
+                "severity": Severity.MEDIUM,
+                "message": "finding",
+            },
+        ),
+        (ReviewComment, {"author": "reviewer", "body": "comment"}),
+        (CheckConclusion, {"name": "ci", "conclusion": "success"}),
+        (ChangedFile, {"path": "src/index.ts"}),
+        (ReviewManifest, {"repo": "o/r", "pr_number": 1}),
+    ),
+)
+def test_manifest_wire_models_reject_unknown_evidence_fields(
+    model: type[BaseModel],
+    payload: dict[str, object],
+) -> None:
+    """Untrusted manifest evidence fails closed instead of discarding fields."""
+    with pytest.raises(ValidationError):
+        model.model_validate({**payload, "unexpected_evidence": "must-not-disappear"})