From 8e1a7098edb878325a38419046128d132d4887af Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 17:52:29 -0700 Subject: [PATCH] feat(review): capture full-page screenshots for before/after evidence Switches captureShot from a viewport-only render (fullPage: false) to a full-page render (fullPage: true), so before/after screenshots always show the same position on the page for any change, however far down it is -- a change midway down a long page previously fell outside the fixed 1440x900/390x844 viewport and was silently absent from both cells. Verified against a real headless-Chromium render (browserless, per #3608's self-host infra) that a 7300px test page renders as a full 1440x7300 PNG with a deliberately-placed marker visible at its correct position, not just the 900px viewport slice. Part of #3607. Closes #3667. --- src/review/visual/shot.ts | 7 ++++++- test/unit/visual-shot.test.ts | 8 ++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/review/visual/shot.ts b/src/review/visual/shot.ts index e944d29944..0dc538643c 100644 --- a/src/review/visual/shot.ts +++ b/src/review/visual/shot.ts @@ -161,7 +161,12 @@ export async function captureShot(env: Env, url: string, viewport: Viewport = VI console.log(JSON.stringify({ ev: "render_screenshot_auth_walled", url, final: page.url().slice(0, 200) })); return { png: null, authWalled: true }; } - const shot = (await page.screenshot({ type: "png", fullPage: false })) as Uint8Array; + // Full-page (not just the viewport): before/after must show the SAME position on the page for any + // change, however far down it is. A viewport-only shot only captures whatever happens to be in frame at + // load time, which for a change midway down a long page would silently miss it in both cells. Capturing + // the whole scrollable height means the changed region is always present in both images at the same + // relative offset, with no need to locate/scroll to it first. + const shot = (await page.screenshot({ type: "png", fullPage: true })) as Uint8Array; return { png: shot, authWalled: false }; } catch (error) { // Log before degrading to null — otherwise a networkidle0 timeout, a binding quota error, or a render diff --git a/test/unit/visual-shot.test.ts b/test/unit/visual-shot.test.ts index 684aead4b0..f6381f7742 100644 --- a/test/unit/visual-shot.test.ts +++ b/test/unit/visual-shot.test.ts @@ -112,6 +112,14 @@ describe("visual screenshot on-demand SSRF guard", () => { expect(mocks.screenshot).toHaveBeenCalled(); }); + it("captures the FULL page, not just the viewport — before/after must show the same page position for a change however far down it is", async () => { + mocks.finalUrl = "https://preview.pages.dev/page"; + + await handleShot(request("https://preview.pages.dev/page"), env()); + + expect(mocks.screenshot).toHaveBeenCalledWith({ type: "png", fullPage: true }); + }); + it("captureShot rejects an unsafe target before launching the browser (defense-in-depth)", async () => { const result = await captureShot(env(), "http://127.0.0.1/admin"); expect(result).toEqual({ png: null, authWalled: false });