diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1906bd2..3a0e4c9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,7 +71,10 @@ jobs: with: enable-cache: true - name: pytest - run: uv run --python ${{ matrix.python-version }} --extra dev pytest + # --extra pdf as well: the PDF renderer is optional at install time but + # not optional to test — it is the artefact an auditor reads, and it went + # unreachable from the CLI for six milestones without CI noticing. + run: uv run --python ${{ matrix.python-version }} --extra dev --extra pdf pytest determinism: name: determinism (byte-identical report) diff --git a/README.md b/README.md index 1c1f51f..6523560 100644 --- a/README.md +++ b/README.md @@ -157,9 +157,16 @@ markproof verify-report report.json --key public.pem MARKPROOF_SIGNING_KEY: ${{ secrets.MARKPROOF_SIGNING_KEY }} ``` -The output is signed JSON plus a job summary — no system dependencies, so this path -works on any runner. (A PDF renderer exists in the source but is not yet wired to the -CLI: [#21](https://github.com/Tippel-AI/markproof/issues/21).) +The default output is signed JSON plus a job summary — no system dependencies, so +that path works on any runner. A PDF for the auditor is opt-in through the config: + +```yaml +report: + formats: [json, summary, pdf] # needs: pipx install "markproof[pdf]" +``` + +`pdf` is pure Python. `pdf-html` renders through WeasyPrint and wants Pango and +cairo, which pip does not install — so it is never on the default path. ## Related projects diff --git a/examples/markproof.yaml b/examples/markproof.yaml index 5f43e71..3fc89c2 100644 --- a/examples/markproof.yaml +++ b/examples/markproof.yaml @@ -50,3 +50,8 @@ text_marking: rulepack: art50-eu-2026.07 report: sign_key: env:MARKPROOF_SIGNING_KEY # Ed25519 private key, PEM + # json + summary need nothing beyond the base install, which is why they are the + # default: the path a CI runner takes must not depend on a system library. + # Add `pdf` for the artefact you hand an auditor — extra [pdf], pure Python. + # `pdf-html` renders through WeasyPrint and also wants Pango and cairo. + formats: [json, summary] diff --git a/src/markproof/cli.py b/src/markproof/cli.py index 61247a2..c9f2bf8 100644 --- a/src/markproof/cli.py +++ b/src/markproof/cli.py @@ -16,6 +16,7 @@ from __future__ import annotations +import importlib import json import os import sys @@ -35,6 +36,7 @@ HttpChatProbeConfig, MarkproofConfig, MediaProbeConfig, + ReportConfig, UiProbeConfig, load_config, ) @@ -42,7 +44,7 @@ from markproof.probes.http_chat import HttpChatProbe from markproof.probes.media import MediaProbe from markproof.probes.ui import UiProbe -from markproof.report.model import ProbeRecord, build_report +from markproof.report.model import ProbeRecord, Report, build_report from markproof.report.sign import ( SigningError, generate_keypair, @@ -107,6 +109,47 @@ def main( """markproof — Article 50 conformance checks for live endpoints.""" +def _signing_key(report_config: ReportConfig) -> str: + """Where the signing key comes from, config first, environment second. + + ``report.sign_key`` was validated and then ignored, so an operator who wrote + ``sign_key: env:CI_SIGNING_KEY`` in their config got an unsigned report and no + hint why. The environment variable stays the fallback because that is what CI + documentation everywhere assumes. + """ + declared = (report_config.sign_key or "").strip() + if declared.startswith("env:"): + return os.environ.get(declared[4:], "").strip() + if declared: + return declared + return os.environ.get("MARKPROOF_SIGNING_KEY", "").strip() + + +def _render_pdf(report: Report, path: Path, *, engine: str) -> None: + """Write the PDF, or explain which extra is missing. + + Imported here rather than at module scope: both renderers are optional, and a + top-level import would make the default output path — the one that must work + on any runner — depend on a package the base install does not ship. + """ + module = "pdf_reportlab" if engine == "pdf" else "pdf_weasy" + extra = "pdf" if engine == "pdf" else "pdf-html" + try: + renderer = importlib.import_module(f"markproof.report.{module}") + renderer.render_pdf(report, path) + except Exception as exc: + # Both renderers already raise their own well-worded unavailability error — + # WeasyPrint's covers the case that actually bites people, where the wheel + # is installed but ctypes cannot find Pango or cairo. Their message is + # better than anything reconstructed here, so it is passed through and only + # framed. Broad on purpose: whatever goes wrong producing an optional + # artefact, the caller gets a sentence and exit 2, never a traceback. + raise ConfigError( + f"could not write the {engine} report: {exc}\n" + f' If the extra is missing: pip install "markproof[{extra}]"' + ) from exc + + def _readable(exc: Exception) -> str: """A sentence for a stranger, not a repr. @@ -221,6 +264,7 @@ def _write_report( timestamp: str | None, applicability: Applicability, probes: tuple[ProbeRecord, ...], + report_config: ReportConfig, ) -> None: """Write report.json and summary.md, signing when a key is configured. @@ -237,7 +281,7 @@ def _write_report( probes=probes, ) - key_source = os.environ.get("MARKPROOF_SIGNING_KEY", "").strip() + key_source = _signing_key(report_config) if key_source: try: report = sign_report(report, load_private_key(key_source)) @@ -251,15 +295,22 @@ def _write_report( report_path = report_dir / "report.json" summary_path = report_dir / "summary.md" - report_path.write_text( - json.dumps(report.model_dump(mode="json", exclude_none=True), indent=2, sort_keys=True) - + "\n", - encoding="utf-8", - ) - summary_path.write_text(render_summary(report), encoding="utf-8") - - console.print(f" report written to [cyan]{report_path}[/cyan]") - console.print(f" summary written to [cyan]{summary_path}[/cyan]") + formats = report_config.formats + if "json" in formats: + report_path.write_text( + json.dumps(report.model_dump(mode="json", exclude_none=True), indent=2, sort_keys=True) + + "\n", + encoding="utf-8", + ) + console.print(f" report written to [cyan]{report_path}[/cyan]") + if "summary" in formats: + summary_path.write_text(render_summary(report), encoding="utf-8") + console.print(f" summary written to [cyan]{summary_path}[/cyan]") + for fmt in ("pdf", "pdf-html"): + if fmt in formats: + pdf_path = report_dir / ("report.pdf" if fmt == "pdf" else "report-html.pdf") + _render_pdf(report, pdf_path, engine=fmt) + console.print(f" {fmt} written to [cyan]{pdf_path}[/cyan]") if not key_source: console.print( " [dim]unsigned — set MARKPROOF_SIGNING_KEY to produce verifiable evidence[/dim]" @@ -369,19 +420,34 @@ def run( json_out.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n", encoding="utf-8") console.print(f" findings written to [cyan]{json_out}[/cyan]") - if report_dir is not None: - _write_report( - report_dir, - config.target.name, - rulepack, - findings, - timestamp, - config.applicability, - tuple( - ProbeRecord(id=p.id, kind=p.probe_kind.value, url=p.url) - for p in config.target.probes - ), - ) + destination = report_dir if report_dir is not None else None + if destination is None and set(config.report.formats) - {"json", "summary"}: + # An operator who asked for a PDF in the config meant it; falling through + # to "no report at all" because they did not also pass --report-dir would + # silently discard the request. + destination = Path(config.report.output_dir) + if destination is not None: + # Guarded like the evaluation above, and for the same reason: producing an + # optional artefact can fail — a missing extra, an unwritable directory — + # and that is "the run could not finish", not "a rule failed". Exit 1 is + # reserved for the second. + try: + _write_report( + destination, + config.target.name, + rulepack, + findings, + timestamp, + config.applicability, + tuple( + ProbeRecord(id=p.id, kind=p.probe_kind.value, url=p.url) + for p in config.target.probes + ), + config.report, + ) + except (ConfigError, OSError) as exc: + err_console.print(f"[bold red]error:[/] {exc}") + raise typer.Exit(code=2) from exc raise typer.Exit(code=exit_code_for(findings)) diff --git a/src/markproof/config.py b/src/markproof/config.py index ff30357..dc4d924 100644 --- a/src/markproof/config.py +++ b/src/markproof/config.py @@ -314,8 +314,23 @@ class ReportConfig(BaseModel): model_config = ConfigDict(extra="forbid", frozen=True) sign_key: str | None = None - formats: tuple[Literal["json", "summary"], ...] = ("json", "summary") + """Where the Ed25519 signing key comes from: ``env:NAME`` for an environment + variable, or a path. ``MARKPROOF_SIGNING_KEY`` is used when this is unset, so + CI needs no config change.""" + + formats: tuple[Literal["json", "summary", "pdf", "pdf-html"], ...] = ("json", "summary") + """Which artefacts to write. + + ``json`` and ``summary`` need nothing beyond the base install and are the + default for that reason — the output path a CI runner takes must never depend + on a system library. ``pdf`` needs the ``[pdf]`` extra (pure Python) and + ``pdf-html`` the ``[pdf-html]`` one, which wants Pango and cairo and is not + pip-installable. Ask for one you have not installed and the run stops with the + install command rather than a traceback. + """ + output_dir: str = "markproof-report" + """Where those artefacts go, unless ``--report-dir`` overrides it.""" class TextMarkingConfig(BaseModel): diff --git a/tests/test_cli.py b/tests/test_cli.py index 20bf35f..2faad08 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -18,6 +18,7 @@ from __future__ import annotations +import importlib.util import json import stat from pathlib import Path @@ -31,6 +32,13 @@ RUNNER = CliRunner() +#: The PDF path needs an optional extra. CI installs it so the path is genuinely +#: covered; a contributor without it gets a skip rather than a puzzling failure. +needs_reportlab = pytest.mark.skipif( + importlib.util.find_spec("reportlab") is None, + reason="needs the [pdf] extra: pip install 'markproof[pdf]'", +) + _ENDPOINT = "https://api.example.invalid/v1/chat/completions" _DISCLOSED = "Hallo! Sie sprechen mit einer KI. Wie kann ich helfen?" _SILENT = "Hallo! Wie kann ich Ihnen helfen?" @@ -337,3 +345,98 @@ def test_the_timestamp_can_be_pinned(self, tmp_path: Path) -> None: ) report = json.loads((out / "report.json").read_text(encoding="utf-8")) assert report["run"]["timestamp"] == stamp + + +class TestPdfOutput: + """Issue #21: 354 lines of renderer that no invocation could reach. + + `ReportConfig.formats` was typed to accept only json and summary, the CLI + never imported either renderer, and `run --help` said nothing about PDF — while + the README advertised `pip install "markproof[pdf]"` as if installing the extra + got you one. The PDF is the artefact you hand an auditor, so having it exist + only in the source tree was the gap least likely to be noticed and most likely + to matter. + """ + + @needs_reportlab + @respx.mock + def test_a_pdf_is_written_when_the_config_asks_for_one(self, tmp_path: Path) -> None: + respx.post(_ENDPOINT).mock(return_value=_reply(_DISCLOSED)) + config = _config(tmp_path, extra="report:\n formats: [json, pdf]\n") + out = tmp_path / "out" + result = RUNNER.invoke(app, ["run", "-c", str(config), "--report-dir", str(out)]) + assert result.exit_code == 0, result.output + pdf = out / "report.pdf" + assert pdf.is_file() + assert pdf.read_bytes().startswith(b"%PDF-"), "not a PDF" + + @respx.mock + def test_formats_are_honoured_rather_than_validated_and_ignored(self, tmp_path: Path) -> None: + """Asking for json alone must not also produce a summary.""" + respx.post(_ENDPOINT).mock(return_value=_reply(_DISCLOSED)) + config = _config(tmp_path, extra="report:\n formats: [json]\n") + out = tmp_path / "out" + RUNNER.invoke(app, ["run", "-c", str(config), "--report-dir", str(out)]) + assert (out / "report.json").is_file() + assert not (out / "summary.md").exists() + + @needs_reportlab + @respx.mock + def test_a_pdf_request_alone_is_enough_to_write_a_report(self, tmp_path: Path) -> None: + """Without --report-dir, output_dir from the config is used. + + An operator who asked for a PDF meant it; producing nothing because they + did not also pass a flag would discard the request silently. + """ + respx.post(_ENDPOINT).mock(return_value=_reply(_DISCLOSED)) + config = _config( + tmp_path, extra=f"report:\n formats: [pdf]\n output_dir: {tmp_path / 'declared'}\n" + ) + RUNNER.invoke(app, ["run", "-c", str(config)]) + assert (tmp_path / "declared" / "report.pdf").is_file() + + @respx.mock + def test_a_missing_extra_is_a_sentence_with_the_install_command( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """pdf-html needs Pango and cairo, which pip cannot install for you.""" + from markproof.report import pdf_weasy + + def _unavailable(*_: object, **__: object) -> None: + raise pdf_weasy.WeasyPrintUnavailableError( + "WeasyPrint is not installed. pip install 'markproof[pdf-html]' — " + "it also needs Pango and cairo, which pip does not install." + ) + + monkeypatch.setattr(pdf_weasy, "render_pdf", _unavailable) + respx.post(_ENDPOINT).mock(return_value=_reply(_DISCLOSED)) + config = _config(tmp_path, extra="report:\n formats: [pdf-html]\n") + result = RUNNER.invoke( + app, ["run", "-c", str(config), "--report-dir", str(tmp_path / "out")] + ) + assert result.exit_code == 2, result.output + assert "Traceback" not in result.output + assert "pdf-html" in result.output + + +class TestSignKeyFromConfig: + @respx.mock + def test_the_config_can_name_the_environment_variable( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """`sign_key` was validated and then ignored, so a report went out unsigned.""" + respx.post(_ENDPOINT).mock(return_value=_reply(_DISCLOSED)) + keys = tmp_path / "keys" + keys.mkdir() + RUNNER.invoke(app, ["keygen", "--out-dir", str(keys)]) + monkeypatch.setenv( + "CI_SIGNING_KEY", (keys / "markproof-signing-key.pem").read_text(encoding="utf-8") + ) + monkeypatch.delenv("MARKPROOF_SIGNING_KEY", raising=False) + + config = _config(tmp_path, extra="report:\n sign_key: env:CI_SIGNING_KEY\n") + out = tmp_path / "out" + result = RUNNER.invoke(app, ["run", "-c", str(config), "--report-dir", str(out)]) + assert result.exit_code == 0, result.output + report = json.loads((out / "report.json").read_text(encoding="utf-8")) + assert "signature" in report, "the configured key was ignored"