Repository navigation
Description
The gap
The workflow-artifact privacy check, policyengine_uk_data/tests/test_workflow_artifacts.py, allows six controlled report paths when their reviewed producer text appears before the upload. These are fit_summary.json, distribution_report.json and aggregate_result.json, each in the test and base-build jobs (lines 92, 100, 110, 134, 143 and 153; checked at 347–349).
The check proves the producer's text is present. It doesn't prove the uploaded file is what the producer wrote. So a later step that copies a raw calibration log over an allowed name still passes:
- run: cp constituency_calibration_log.csv fit_summary.json # inserted before the uploadproblems("pull_request.yaml", workflow) returns [] for all six names after this mutation. actions/upload-artifact uploads the bytes as they are. The downstream comparison would then fail to parse the file, but only after the artifact is uploaded.
Scope
This is not a current leak. The workflow at release head 21d5dbf9 (#608) has no such overwrite, and #603 documented this limit at test_workflow_artifacts.py:31–38: the patterns show text presence, not execution, order or content. The independent review of #608 found the gap and rated it P3. This issue asks for the stronger guarantee.
Options
- Content check before upload. A step validates each allowed file against its schema (for example
policyengine-uk-data-fit-summary/3) and fails on anything else, including CSV bytes. - Controlled staging. Producers write into a fresh directory that only they write to. The upload takes from there, and the guard rejects any other write to that directory.
- Make the guard reject writes to an allowed path by any step other than its producer.
Option 1 is the smallest change and also catches accidental mistakes.
Reproducer
This uses a synthetic CSV only: no microdata, no build, no network.
from pathlib import Path
import importlib.util, yaml
path = Path("policyengine_uk_data/tests/test_workflow_artifacts.py")
spec = importlib.util.spec_from_file_location("guard", path)
guard = importlib.util.module_from_spec(spec); spec.loader.exec_module(guard)
wf = yaml.safe_load(Path(".github/workflows/pull_request.yaml").read_text())
steps = wf["jobs"]["test"]["steps"]
i = next(i for i, s in enumerate(steps)
if s.get("uses", "").startswith("actions/upload-artifact@")
and s["with"]["path"] == "fit_summary.json")
steps.insert(i, {"run": "cp constituency_calibration_log.csv fit_summary.json"})
assert guard.problems("pull_request.yaml", yaml.safe_load(yaml.safe_dump(wf))) == []A test for the fix: this mutation should produce a problem for each of the six names.
Activity