From 35ba99768c0b2d3e3a2e83df9602e816a5b348fe Mon Sep 17 00:00:00 2001 From: autonomic-bot <64+autonomic-bot@noreply.git.autonomic.zone> Date: Mon, 5 Oct 2026 17:46:25 +0000 Subject: [PATCH] runner: never embed a fossil screenshot from a collided run dir (#45) --- runner/harness/results.py | 37 +++++++++++++++++ runner/harness/screenshot.py | 11 ++--- runner/run_recipe_ci.py | 4 ++ tests/unit/test_results.py | 47 ++++++++++++++++++++++ tests/unit/test_screenshot.py | 75 +++++++++++++++++++++++++++++++++++ 5 files changed, 169 insertions(+), 5 deletions(-) diff --git a/runner/harness/results.py b/runner/harness/results.py index 308f61d..b873fac 100644 --- a/runner/harness/results.py +++ b/runner/harness/results.py @@ -27,6 +27,7 @@ from __future__ import annotations import json import os +import shutil import xml.etree.ElementTree as ET from . import level as level_mod @@ -35,6 +36,17 @@ from . import level as level_mod # The dashboard serves these read-only at /runs//... (U0.4). Overridable for tests. RUNS_DIR_DEFAULT = "/var/lib/cc-ci-runs" +# The artifact files/dirs a run OWNS in its run dir (written fresh every run). +_OWNED_ARTIFACTS = ( + "results.json", + "summary.html", + "summary.png", + "badge.svg", + "lint.txt", + "screenshot.png", + "junit", +) + def runs_dir() -> str: return os.environ.get("CCCI_RUNS_DIR", RUNS_DIR_DEFAULT) @@ -49,6 +61,31 @@ def run_id() -> str: return os.environ.get("CCCI_APP_DOMAIN") or os.environ.get("CCCI_RUN_ID") or "manual" +def fresh_artifact_dir(path: str) -> str: + """Ensure the run artifact dir holds no stale artifacts from a PREVIOUS run with the same id. + + Drone build numbers restarted when its DB was re-created (2026-09-27) while /var/lib/cc-ci-runs + still holds old-era dirs numbered into the thousands — a new build reusing an old number would + otherwise mix old artifacts into the new run's card (observed 2026-10-05: !testme gitea build 35 + collided with a May-31 run 35 and embedded custom-html-tiny's fossil screenshot.png, because + screenshot.capture() skips its snap when out_path already exists). Removes ONLY the files/dirs + this run owns (see _OWNED_ARTIFACTS); anything else in the dir is left untouched. Best-effort + (R7): never raises into the run.""" + os.makedirs(path, exist_ok=True) + for name in _OWNED_ARTIFACTS: + p = os.path.join(path, name) + if os.path.isdir(p): + shutil.rmtree(p, ignore_errors=True) + else: + try: + os.remove(p) + except FileNotFoundError: + pass + except OSError: + pass # R7: a stuck file must never fail the run — the card is best-effort anyway + return path + + def junit_file(junit_dir: str, tier: str, source: str, path: str) -> str: """Deterministic per-(tier,source,file) JUnit XML path under junit_dir.""" base = os.path.splitext(os.path.basename(path))[0] diff --git a/runner/harness/screenshot.py b/runner/harness/screenshot.py index 15c1867..f8a78fb 100644 --- a/runner/harness/screenshot.py +++ b/runner/harness/screenshot.py @@ -142,12 +142,13 @@ def capture(domain: str, out_path: str, *, recipe_meta: dict | None = None) -> s page = context.new_page() if hook is not None: # Recipe-specific safe view (post-login etc.). The hook owns navigation + - # the no-secret-page guarantee; it should call page.screenshot itself, but if - # it doesn't, we still snap the resulting page below. SCREENSHOT(page, ctx) — - # the uniform ctx convention (rcust P3). + # the no-secret-page guarantee; we ALWAYS snap the page after it returns. + # Never trust a pre-existing out_path: a recycled run artifact dir (drone + # build numbers restarted 2026-09-27 while old-era dirs linger) must not get + # its fossil screenshot embedded (observed 2026-10-05, !testme gitea #10 + # build 35). SCREENSHOT(page, ctx) — the uniform ctx convention (rcust P3). hook(page, meta_mod.hook_ctx(domain, recipe_meta)) - if not os.path.exists(out_path): - _snap_with_blank_retry(page, out_path) + _snap_with_blank_retry(page, out_path) else: # Default: landing page. Accept any rendered status (200 or an auth redirect to a # login form) — both are credential-free and representative of "the app is up". diff --git a/runner/run_recipe_ci.py b/runner/run_recipe_ci.py index 19d8167..a2173e1 100644 --- a/runner/run_recipe_ci.py +++ b/runner/run_recipe_ci.py @@ -1184,6 +1184,10 @@ def main() -> int: # {tier,source,file,rc,junit} record; after the run we assemble results.json (per-stage/per-test + # level) into the artifact dir. Best-effort — never changes the verdict (R7). run_artifact_dir = os.path.join(results_mod.runs_dir(), results_mod.run_id()) + # Collision guard (2026-10-05): drone build numbers restarted when its DB was re-created while + # old-era run dirs (same numbers, other recipes' artifacts — e.g. a fossil screenshot.png) are + # still on disk. Wipe the files THIS run owns so nothing stale survives into the card. + results_mod.fresh_artifact_dir(run_artifact_dir) junit_dir = os.path.join(run_artifact_dir, "junit") records: list[dict] = [] diff --git a/tests/unit/test_results.py b/tests/unit/test_results.py index 7fb0ae1..66cd4ef 100644 --- a/tests/unit/test_results.py +++ b/tests/unit/test_results.py @@ -489,3 +489,50 @@ def test_write_results_roundtrip(tmp_path): assert path.endswith("/42/results.json") with open(path) as f: assert json.load(f)["level"] == 3 + + +def test_fresh_artifact_dir_removes_owned_stale_artifacts(tmp_path): + """Run-dir collision guard (2026-10-05): drone build numbers restarted while old-era run dirs + linger in /var/lib/cc-ci-runs — a new build reusing an old number must not mix the old run's + artifacts (fossil screenshot.png from ANOTHER recipe) into its card. Only OWNED artifacts go.""" + d = tmp_path / "35" + d.mkdir() + (d / "results.json").write_text('{"old": true}') + (d / "screenshot.png").write_bytes(b"stale-fossil") + (d / "summary.png").write_bytes(b"stale") + (d / "badge.svg").write_text("") + (d / "lint.txt").write_text("old") + (d / "summary.html").write_text("") + (d / "junit").mkdir() + (d / "junit" / "old.xml").write_text("") + (d / "untracked-thing.bin").write_bytes(b"keep me") # NOT owned — must survive + + R.fresh_artifact_dir(str(d)) + + for name in ( + "results.json", + "screenshot.png", + "summary.png", + "badge.svg", + "lint.txt", + "summary.html", + ): + assert not (d / name).exists(), f"stale owned artifact {name} survived" + assert not (d / "junit").exists(), "stale junit dir survived" + assert (d / "untracked-thing.bin").read_bytes() == b"keep me", "unowned file was deleted" + + +def test_fresh_artifact_dir_creates_missing_dir(tmp_path): + d = tmp_path / "36" + out = R.fresh_artifact_dir(str(d)) + assert out == str(d) and d.is_dir() + + +def test_fresh_artifact_dir_tolerant_of_odd_entries(tmp_path): + """R7: never raise into the run — a dir where an owned name is itself a dir must still work.""" + d = tmp_path / "37" + d.mkdir() + (d / "screenshot.png").mkdir() # perverse: a DIRECTORY named screenshot.png + (d / "junit").mkdir() + R.fresh_artifact_dir(str(d)) + assert not (d / "screenshot.png").exists() and not (d / "junit").exists() diff --git a/tests/unit/test_screenshot.py b/tests/unit/test_screenshot.py index 6c83d90..f9859f1 100644 --- a/tests/unit/test_screenshot.py +++ b/tests/unit/test_screenshot.py @@ -184,3 +184,78 @@ def test_screenshot_reachable_through_real_load_path(tmp_path): hook = S._load_screenshot_hook(meta) assert callable(hook), "SCREENSHOT hook did not survive the orchestrator load path (R2)" assert S._load_screenshot_hook(meta_mod.load("no-such", tests_dir=str(tmp_path))) is None + + +# ---- capture() fossil guard (2026-10-05) ----------------------------------------------------- +# Drone build numbers restarted when its DB was re-created while /var/lib/cc-ci-runs still holds +# old-era dirs numbered into the thousands; run 35 (!testme gitea #10) collided with a May-31 run +# 35 and embedded custom-html-tiny's fossil screenshot.png because the hook branch SKIPPED the +# snap when out_path already existed. capture() must always snap fresh. + + +class _FakeLaunchable: + def __init__(self, page): + self._page = page + self.chromium = self + + def launch(self, args=None): + return self + + def new_context(self, **kw): + return self + + def new_page(self): + return self._page + + def close(self): + pass + + +class _FakeSyncPlaywright: + """sync_playwright() replacement: context-manager factory handing out one fake browser.""" + + def __init__(self, page): + self._page = page + + def __call__(self): + return self + + def __enter__(self): + return _FakeLaunchable(self._page) + + def __exit__(self, *exc): + return False + + +def _install_fake_playwright(monkeypatch, page): + import types + + fake_api = types.ModuleType("playwright.sync_api") + fake_api.sync_playwright = _FakeSyncPlaywright(page) + fake = types.ModuleType("playwright") + fake.sync_api = fake_api + monkeypatch.setitem(sys.modules, "playwright", fake) + monkeypatch.setitem(sys.modules, "playwright.sync_api", fake_api) + + +def test_capture_overwrites_fossil_screenshot(tmp_path, monkeypatch): + """A pre-existing out_path in a recycled run artifact dir must never be embedded: the hook + branch snaps fresh regardless (the old exists-guard returned the fossil as 'captured').""" + out_path = str(tmp_path / "screenshot.png") + with open(out_path, "wb") as f: + f.write(b"STALE-FOSSIL-FROM-ANOTHER-RECIPE" * 400) + + fired = {} + page = _FakePage([35707]) # one real-sized frame, like a painted page + + def hook(p, ctx): + fired["yes"] = True # navigation only — hooks never save the PNG themselves + + _install_fake_playwright(monkeypatch, page) + got = S.capture("gitea.example.org", out_path, recipe_meta={"SCREENSHOT": hook}) + + assert got == out_path, "capture must report success after snapping fresh" + assert fired.get("yes"), "SCREENSHOT hook must run" + assert page.shots == 1, "capture must snap exactly once — the fossil is not a captured frame" + with open(out_path, "rb") as f: + assert f.read(4) == b"\x89PNG", "out_path still holds the fossil — snap did not overwrite"