From 024fa0084dae744c87f2788d57d2bec4b6678777 Mon Sep 17 00:00:00 2001
From: DocFast CEO <openclawd@users.noreply.git.cloonar.com>
Date: Mon, 2 Mar 2026 17:05:45 +0100
Subject: [PATCH] fix: clean up request interceptor in recyclePage to prevent
 pool contamination
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When renderUrlPdf() sets up request interception for SSRF DNS pinning,
the interceptor and event listener were never cleaned up in recyclePage().
This could cause subsequent HTML-to-PDF conversions on the same pooled
page to have external resources blocked by the stale interceptor.

- Export recyclePage for testability
- Add removeAllListeners('request') + setRequestInterception(false)
- Add browser-recycle.test.ts with TDD (red→green verified)

Tests: 443 passing (was 442)
---
 src/__tests__/browser-recycle.test.ts | 58 +++++++++++++++++++++++++++
 src/services/browser.ts               |  5 ++-
 2 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 src/__tests__/browser-recycle.test.ts

diff --git a/src/__tests__/browser-recycle.test.ts b/src/__tests__/browser-recycle.test.ts
new file mode 100644
index 0000000..cbb1da6
--- /dev/null
+++ b/src/__tests__/browser-recycle.test.ts
@@ -0,0 +1,58 @@
+import { describe, it, expect, vi } from "vitest";
+
+// Don't use the global mock — we test the real recyclePage
+vi.unmock("../services/browser.js");
+
+// Mock puppeteer so initBrowser doesn't launch real browsers
+vi.mock("puppeteer", () => ({
+  default: {
+    launch: vi.fn(),
+  },
+}));
+
+describe("recyclePage", () => {
+  it("cleans up request interception listeners before navigating to about:blank", async () => {
+    // Dynamic import to get the real (unmocked) module
+    const { recyclePage } = await import("../services/browser.js");
+
+    const callOrder: string[] = [];
+
+    const mockPage = {
+      createCDPSession: vi.fn().mockResolvedValue({
+        send: vi.fn().mockResolvedValue(undefined),
+        detach: vi.fn().mockResolvedValue(undefined),
+      }),
+      removeAllListeners: vi.fn().mockImplementation((event: string) => {
+        callOrder.push(`removeAllListeners:${event}`);
+        return mockPage;
+      }),
+      setRequestInterception: vi.fn().mockImplementation((val: boolean) => {
+        callOrder.push(`setRequestInterception:${val}`);
+        return Promise.resolve();
+      }),
+      cookies: vi.fn().mockResolvedValue([]),
+      deleteCookie: vi.fn(),
+      goto: vi.fn().mockImplementation((url: string) => {
+        callOrder.push(`goto:${url}`);
+        return Promise.resolve();
+      }),
+    };
+
+    await recyclePage(mockPage as any);
+
+    // Verify request interception cleanup happens
+    expect(mockPage.removeAllListeners).toHaveBeenCalledWith("request");
+    expect(mockPage.setRequestInterception).toHaveBeenCalledWith(false);
+
+    // Verify cleanup happens BEFORE navigation to about:blank
+    const removeIdx = callOrder.indexOf("removeAllListeners:request");
+    const interceptIdx = callOrder.indexOf("setRequestInterception:false");
+    const gotoIdx = callOrder.indexOf("goto:about:blank");
+
+    expect(removeIdx).toBeGreaterThanOrEqual(0);
+    expect(interceptIdx).toBeGreaterThanOrEqual(0);
+    expect(gotoIdx).toBeGreaterThanOrEqual(0);
+    expect(removeIdx).toBeLessThan(gotoIdx);
+    expect(interceptIdx).toBeLessThan(gotoIdx);
+  });
+});
diff --git a/src/services/browser.ts b/src/services/browser.ts
index 3b79ff1..becf34c 100644
--- a/src/services/browser.ts
+++ b/src/services/browser.ts
@@ -40,11 +40,14 @@ export function getPoolStats() {
   };
 }
 
-async function recyclePage(page: Page): Promise<void> {
+export async function recyclePage(page: Page): Promise<void> {
   try {
     const client = await page.createCDPSession();
     await client.send("Network.clearBrowserCache").catch(() => {});
     await client.detach().catch(() => {});
+    // Clean up request interception (set by renderUrlPdf for SSRF protection)
+    page.removeAllListeners("request");
+    await page.setRequestInterception(false).catch(() => {});
     const cookies = await page.cookies();
     if (cookies.length > 0) {
       await page.deleteCookie(...cookies);