diff --git a/runner/harness/screenshot.py b/runner/harness/screenshot.py index f7936ef..15c1867 100644 --- a/runner/harness/screenshot.py +++ b/runner/harness/screenshot.py @@ -66,7 +66,9 @@ def settle(page, idle_timeout_ms: int = SETTLE_TIMEOUT_MS) -> None: def _snap_with_blank_retry(page, out_path: str) -> None: """Screenshot the page; if the PNG is blank/spinner-sized, retry ONCE after a longer settle. - The retry overwrites the tiny frame with a strictly-later one (same page, more paint time).""" + The retry is snapped to a temp path and kept only if it is >= the first frame's size — later + is usually more painted, but a page can also regress (redirect, error overlay) and a worse + frame must never overwrite a better one (adversary finding A1).""" page.screenshot(path=out_path, full_page=False) try: first = os.path.getsize(out_path) @@ -80,9 +82,19 @@ def _snap_with_blank_retry(page, out_path: str) -> None: flush=True, ) _settle(page, BLANK_RETRY_SETTLE_MS) - page.screenshot(path=out_path, full_page=False) - with contextlib.suppress(OSError): - print(f" screenshot: retry frame {os.path.getsize(out_path)} B", flush=True) + retry_path = out_path + ".retry" + try: + page.screenshot(path=retry_path, full_page=False) + retry = os.path.getsize(retry_path) + if retry >= first: + os.replace(retry_path, out_path) + print(f" screenshot: retry frame kept ({retry} B >= {first} B)", flush=True) + else: + os.remove(retry_path) + print(f" screenshot: retry frame discarded ({retry} B < {first} B)", flush=True) + finally: + with contextlib.suppress(OSError): + os.remove(retry_path) def screenshot_path(run_artifact_dir: str) -> str: diff --git a/tests/unit/test_screenshot.py b/tests/unit/test_screenshot.py index a25ca5a..cb5b238 100644 --- a/tests/unit/test_screenshot.py +++ b/tests/unit/test_screenshot.py @@ -94,6 +94,19 @@ def test_snap_retry_keeps_late_frame_even_if_still_blank(tmp_path): S._snap_with_blank_retry(page, out) assert page.shots == 2 assert os.path.getsize(out) == 4801 + assert not os.path.exists(out + ".retry"), "temp retry frame must be cleaned up" + + +def test_snap_retry_never_regresses_to_smaller_frame(tmp_path): + """Adversary finding A1: a partial-but-real first frame (just under the threshold) must + survive a retry that comes back WORSE (page regressed to blank during the extra settle) — + the larger frame wins.""" + out = str(tmp_path / "shot.png") + page = _FakePage([9999, 4801]) + S._snap_with_blank_retry(page, out) + assert page.shots == 2 + assert os.path.getsize(out) == 9999, "retry must never overwrite a larger frame (A1)" + assert not os.path.exists(out + ".retry"), "temp retry frame must be cleaned up" def test_blank_threshold_brackets_observed_sizes():