Skip to content

Commit d7a71f3

Browse files
placerdaCopilot
andcommitted
fix: reject malformed diagnostics and contradictory quality receipts
Add regression-first Q6 coverage with real aggregate CLI negative cases. Validate diagnostic fields before baselining; preserve protected evaluation and zero active exception approvals. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 932ae75 commit d7a71f3

5 files changed

Lines changed: 149 additions & 7 deletions

File tree

.github/scripts/check-quality.py

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -355,11 +355,13 @@ def lint(root: Path, config: Path, sources: dict, entries: list[dict],
355355
"--output-format", "json", "--no-cache",
356356
*[str(root / path) for path in sources]], config.parent)
357357
try:
358-
diagnostics = json.loads(result.stdout)
358+
diagnostics = json.loads(result.stdout, object_pairs_hook=q.unique_keys)
359359
except json.JSONDecodeError as exc:
360360
raise q.QualityError("Ruff did not return JSON") from exc
361361
if not isinstance(diagnostics, list) or (result.returncode == 1 and not diagnostics):
362362
raise q.QualityError("Incomplete Ruff execution")
363+
if result.returncode == 0 and diagnostics:
364+
raise q.QualityError("Ruff exit status contradicts its diagnostics")
363365
approved_sites = []
364366
path_ids = {path: key for key, path in modules.items()}
365367
for site in q.handler_sites(sources):
@@ -373,9 +375,17 @@ def lint(root: Path, config: Path, sources: dict, entries: list[dict],
373375
findings = []
374376
used_ids = set()
375377
for item in diagnostics:
376-
if not all(key in item for key in ("code", "filename", "location", "message")):
378+
if (not isinstance(item, dict)
379+
or not all(isinstance(item.get(key), str) and item[key]
380+
for key in ("code", "filename", "message"))
381+
or not isinstance(item.get("location"), dict)
382+
or not all(type(item["location"].get(key)) is int and item["location"][key] >= 1
383+
for key in ("row", "column"))):
377384
raise q.QualityError("Unsupported Ruff diagnostic")
378-
file = Path(item["filename"]).resolve().relative_to(root).as_posix()
385+
path = Path(item["filename"]).resolve()
386+
if not path.is_relative_to(root):
387+
raise q.QualityError("Ruff emitted an out-of-repository source error")
388+
file = path.relative_to(root).as_posix()
379389
if item["code"] == "BLE001":
380390
approved = [record_id for site, record_id in approved_sites
381391
if site["file"] == file
@@ -402,15 +412,22 @@ def typing(root: Path, config: Path, sources: dict, modules: dict, covered: set,
402412
if not line.strip():
403413
continue
404414
try:
405-
item = json.loads(line)
415+
item = json.loads(line, object_pairs_hook=q.unique_keys)
406416
except json.JSONDecodeError as exc:
407417
raise q.QualityError("Mypy returned an unrecognized diagnostic") from exc
408-
if not all(key in item for key in ("file", "line", "column", "message", "code", "severity")):
418+
if (not isinstance(item, dict)
419+
or not all(isinstance(item.get(key), str) and item[key]
420+
for key in ("file", "message", "severity"))
421+
or type(item.get("line")) is not int or item["line"] < 1
422+
or type(item.get("column")) is not int or item["column"] < 0
423+
or "code" not in item):
409424
raise q.QualityError("Incomplete mypy diagnostic")
410425
if item["severity"] not in {"error", "note"}:
411426
raise q.QualityError("Unsupported mypy severity")
412427
if item["severity"] != "error":
413428
continue
429+
if not isinstance(item["code"], str) or not item["code"]:
430+
raise q.QualityError("Mypy error lacks a diagnostic code")
414431
path = Path(item["file"])
415432
path = (config.parent / path).resolve() if not path.is_absolute() else path.resolve()
416433
if not path.is_relative_to(root):

.github/scripts/quality_policy.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,9 +758,11 @@ def aggregate(jobs: dict, reports: dict, base_sha: str, head_sha: str) -> list[d
758758
continue
759759
report = reports.get(name, {})
760760
if not all((
761-
report.get("schema_version") == 1, report.get("check_name") == name,
761+
type(report.get("schema_version")) is int and report["schema_version"] == 1,
762+
report.get("check_name") == name,
762763
report.get("status") == "passed", report.get("base_sha") == base_sha,
763764
report.get("head_sha") == head_sha,
765+
report.get("findings") == [],
764766
isinstance(report.get("policy_sha"), str) and len(report["policy_sha"]) == 64,
765767
)):
766768
result.append(finding("invalid-report", reason=name))

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@
2323

2424
### Fixed
2525

26+
- **Malformed quality evidence cannot become a passing receipt.** Validate
27+
Ruff/mypy diagnostic types, positions, duplicate keys and exit consistency;
28+
reject aggregate reports whose passed status contradicts their findings.
29+
Real aggregate CLI fixtures cover missing/skipped jobs and stale or mismatched
30+
artifacts without treating incomplete execution as successful validation.
31+
2632
- **Direct uploads require authoritative Search confirmation.** Exercise the
2733
actual `/ingest-documents` route with the pinned SDK response model, reject
2834
missing, duplicate and unrelated results, and retain the per-record response

docs/python-quality.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,14 @@ declared area are retained. Record parsing rejects missing/unknown fields,
188188
invalid nested types and unsupported lifecycle values before running checks.
189189
Boolean or floating-point schema versions are not version 1. The exact
190190
requirements manifest must agree with the policy toolchain.
191+
Tool diagnostics are validated before they can participate in the baseline:
192+
Ruff and mypy require typed, nonempty diagnostic fields and valid source
193+
positions, and duplicate JSON keys or contradictory exit status are execution
194+
errors. The aggregate also rejects a report claiming `passed` while carrying
195+
findings. Existing-runner fixtures execute the aggregate CLI with missing or
196+
skipped jobs, missing reports, wrong base/head/policy/source/run/attempt,
197+
skipped or stale test evidence, and duplicate evidence keys, alongside the
198+
clean positive control.
191199

192200
## Broad handlers and remaining acceptance
193201

tests/test_quality_policy.py

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,8 @@ def timeout(*args, **kwargs):
482482

483483
def report(name):
484484
return {"schema_version": 1, "check_name": name, "status": "passed",
485-
"base_sha": "b" * 40, "head_sha": "c" * 40, "policy_sha": "d" * 64}
485+
"base_sha": "b" * 40, "head_sha": "c" * 40, "policy_sha": "d" * 64,
486+
"findings": []}
486487

487488

488489
def test_aggregate_requires_fixed_actual_results_and_fresh_reports():
@@ -499,6 +500,114 @@ def test_aggregate_requires_fixed_actual_results_and_fresh_reports():
499500
assert quality.aggregate({}, reports, "b" * 40, "c" * 40)
500501

501502

503+
@pytest.fixture
504+
def checker():
505+
spec = importlib.util.spec_from_file_location("check_quality_under_test", SCRIPT.with_name("check-quality.py"))
506+
assert spec and spec.loader
507+
module = importlib.util.module_from_spec(spec)
508+
spec.loader.exec_module(module)
509+
return module
510+
511+
512+
@pytest.mark.parametrize(("field", "value"), [
513+
("line", True), ("line", "1"), ("line", 0), ("column", None),
514+
("column", False), ("file", ""), ("message", []), ("code", None),
515+
("severity", "warning"),
516+
])
517+
def test_malformed_mypy_diagnostics_are_execution_errors(checker, monkeypatch, tmp_path, field, value):
518+
diagnostic = {
519+
"file": str(tmp_path / "a.py"), "line": 1, "column": 0,
520+
"message": "bad return type", "code": "return-value", "severity": "error",
521+
}
522+
diagnostic[field] = value
523+
monkeypatch.setattr(quality, "run_tool", lambda *a, **k: subprocess.CompletedProcess(
524+
[], 1, json.dumps(diagnostic), "",
525+
))
526+
with pytest.raises(quality.QualityError):
527+
checker.typing(tmp_path, tmp_path / "pyproject.toml", {"a.py": "value = 1"},
528+
{"a": "a.py"}, {"a"}, {"entries": []})
529+
530+
531+
@pytest.mark.parametrize(("returncode", "diagnostics"), [(1, ""), (0, "error")])
532+
def test_mypy_status_must_match_diagnostics(checker, monkeypatch, tmp_path, returncode, diagnostics):
533+
stdout = "" if not diagnostics else json.dumps({
534+
"file": str(tmp_path / "a.py"), "line": 1, "column": 0,
535+
"message": "bad return type", "code": "return-value", "severity": "error",
536+
})
537+
monkeypatch.setattr(quality, "run_tool", lambda *a, **k: subprocess.CompletedProcess(
538+
[], returncode, stdout, "",
539+
))
540+
with pytest.raises(quality.QualityError):
541+
checker.typing(tmp_path, tmp_path / "pyproject.toml", {"a.py": "value = 1"},
542+
{"a": "a.py"}, {"a"}, {"entries": []})
543+
544+
545+
@pytest.mark.parametrize(("field", "value"), [
546+
("code", None), ("filename", ""), ("message", []),
547+
("location", {"row": True, "column": 1}), ("location", {"row": 0, "column": 1}),
548+
])
549+
def test_malformed_ruff_diagnostics_are_execution_errors(checker, monkeypatch, tmp_path, field, value):
550+
diagnostic = {
551+
"filename": str(tmp_path / "a.py"), "location": {"row": 1, "column": 1},
552+
"message": "undefined name", "code": "F821",
553+
}
554+
diagnostic[field] = value
555+
monkeypatch.setattr(quality, "run_tool", lambda *a, **k: subprocess.CompletedProcess(
556+
[], 1, json.dumps([diagnostic]), "",
557+
))
558+
with pytest.raises(quality.QualityError):
559+
checker.lint(tmp_path, tmp_path / "pyproject.toml", {"a.py": "missing"}, [], {"a": "a.py"}, "initial")
560+
561+
562+
@pytest.mark.parametrize("mutation", [
563+
"none", "missing-job", "skipped-job", "error-job", "missing-report",
564+
"wrong-head", "wrong-base", "wrong-policy", "wrong-source", "wrong-run", "wrong-attempt",
565+
"status-contradiction", "skipped-test", "stale-test", "wrong-test-attempt", "duplicate-test-key",
566+
])
567+
def test_aggregate_cli_requires_actual_consistent_current_receipts(tmp_path, mutation):
568+
jobs = {name: {"result": "success"} for name in quality.REQUIRED_CHECKS}
569+
for name in quality.REQUIRED_CHECKS[:-1]:
570+
data = dict(report(name), repository="Azure/gpt-rag-ingestion", source_sha="a" * 64,
571+
run_id="123", run_attempt="1")
572+
if name == "lint":
573+
changes = {
574+
"wrong-head": ("head_sha", "e" * 40), "wrong-base": ("base_sha", "e" * 40),
575+
"wrong-policy": ("policy_sha", "e" * 64), "wrong-source": ("source_sha", "e" * 64),
576+
"wrong-run": ("run_id", "124"), "wrong-attempt": ("run_attempt", "2"),
577+
"status-contradiction": ("findings", [{"rule": "execution-error", "reason": "crash"}]),
578+
}
579+
if mutation in changes:
580+
key, value = changes[mutation]
581+
data[key] = value
582+
if mutation == "missing-report":
583+
continue
584+
data["artifact_integrity"] = quality.digest(data)
585+
(tmp_path / f"{name}.json").write_text(json.dumps(data), encoding="utf-8")
586+
if mutation == "missing-job":
587+
del jobs["unit-tests"]
588+
elif mutation in {"skipped-job", "error-job"}:
589+
jobs["unit-tests"]["result"] = mutation.removesuffix("-job")
590+
evidence = {
591+
"schema_version": 1, "base_sha": "b" * 40,
592+
"head_sha": "e" * 40 if mutation == "stale-test" else "c" * 40,
593+
"run_id": "123", "run_attempt": "2" if mutation == "wrong-test-attempt" else "1",
594+
"junit_sha": "f" * 64,
595+
"tests": {"tests/test_failure.py::test_error": "skipped" if mutation == "skipped-test" else "passed"},
596+
}
597+
evidence["artifact_integrity"] = quality.digest(evidence)
598+
encoded = json.dumps(evidence)
599+
if mutation == "duplicate-test-key":
600+
encoded = encoded.replace('"tests": {', '"tests": {}, "tests": {')
601+
(tmp_path / "test-evidence.json").write_text(encoded, encoding="utf-8")
602+
result = subprocess.run(
603+
[sys.executable, "-I", str(SCRIPT.with_name("quality-gate.py")),
604+
"--reports", str(tmp_path), "--base-sha", "b" * 40, "--head-sha", "c" * 40],
605+
env=dict(os.environ, NEEDS_JSON=json.dumps(jobs), GITHUB_RUN_ID="123", GITHUB_RUN_ATTEMPT="1"),
606+
capture_output=True, text=True, timeout=30,
607+
)
608+
assert (result.returncode == 0) == (mutation == "none"), result.stderr
609+
610+
502611
@pytest.fixture
503612
def policy_repository(tmp_path):
504613
"""A committed minimum policy and a separate trusted evaluator copy."""

0 commit comments

Comments
 (0)