From 0a717090f75e5adfefe58ce5b00f9f00aa22a7df Mon Sep 17 00:00:00 2001 From: MeRezaRezaei Date: Tue, 1 Sep 2026 20:46:54 +0200 Subject: [PATCH 1/2] fix: apply configured auth storageState to the browser context The useAuth / WIGOLO_AUTH_STATE_PATH path returns a storageStatePath but fetchWithBrowser never loads it, so authenticated fetches run logged out. Restore the stored cookies (dropping expired) and origin localStorage onto the acquired context, mirroring the existing injectedCookies pattern, so a useAuth fetch runs as the stored account. Fixes #526 --- src/fetch/browser-pool.ts | 41 +++++ .../fetch/browser-pool.storage-state.test.ts | 159 ++++++++++++++++++ 2 files changed, 200 insertions(+) create mode 100644 tests/unit/fetch/browser-pool.storage-state.test.ts diff --git a/src/fetch/browser-pool.ts b/src/fetch/browser-pool.ts index 8531fd714..8a76e0cd3 100644 --- a/src/fetch/browser-pool.ts +++ b/src/fetch/browser-pool.ts @@ -797,6 +797,15 @@ export class MultiBrowserPool { .catch(() => {}); } + // Restore an authenticated session (cookies + origin localStorage) from a + // Playwright storage-state file so the fetch runs as the stored account + // rather than a logged-out visitor. Mirrors the injectedCookies pattern: + // applied to whichever context this fetch acquired, guarded for context + // stubs without the Playwright methods (unit-test mocks). + if (options.storageStatePath && typeof (ctx as { addCookies?: unknown }).addCookies === 'function') { + await this.applyStorageState(ctx, options.storageStatePath).catch(() => {}); + } + let page: import('playwright').Page; try { page = await ctx.newPage(); @@ -1309,6 +1318,38 @@ export class MultiBrowserPool { } } + /** + * Restore an authenticated session from a Playwright storage-state file onto + * the given context. Loads cookies via `addCookies` and replays any origin + * localStorage via an init script, so a `useAuth` fetch runs as the stored + * account. Best-effort: any failure logs at debug and is swallowed by the + * caller so a stale/malformed file degrades to a logged-out fetch rather than + * failing it. + */ + private async applyStorageState(ctx: BrowserContext, storageStatePath: string): Promise { + const raw = await readFile(storageStatePath, 'utf8'); + const state = JSON.parse(raw) as { cookies?: import('playwright').Cookie[]; origins?: Array<{ origin: string; localStorage: Array<{ name: string; value: string }> }> }; + const nowSec = Date.now() / 1000; + const live = (state.cookies ?? []).filter((c) => !c.expires || c.expires > nowSec); + + const addCookies = (ctx as { addCookies?: (c: import('playwright').Cookie[]) => Promise }).addCookies; + if (live.length > 0 && typeof addCookies === 'function') { + await addCookies.call(ctx, live); + } + + if (Array.isArray(state.origins)) { + const script = + 'const data = ' + + JSON.stringify(state.origins) + + ';' + + 'for (const o of data) { if (location.origin === o.origin) { for (const { name, value } of o.localStorage) { try { localStorage.setItem(name, value); } catch (e) {} } } }'; + const addInitScript = (ctx as { addInitScript?: (script: string) => Promise }).addInitScript; + if (typeof addInitScript === 'function') { + await addInitScript.call(ctx, script); + } + } + } + /** * Build the concrete injected rung callbacks and run the pure solve ladder in * the LIVE browser context (the in-band constraint — same session that owns diff --git a/tests/unit/fetch/browser-pool.storage-state.test.ts b/tests/unit/fetch/browser-pool.storage-state.test.ts new file mode 100644 index 000000000..20c9a9c9b --- /dev/null +++ b/tests/unit/fetch/browser-pool.storage-state.test.ts @@ -0,0 +1,159 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { resetConfig } from '../../../src/config.js'; + +// --- programmable page/context behaviour (mirrors browser-pool.clearance.test.ts) --- +const state: { + addCookiesCalls: Array>; + addInitScriptCalls: string[]; +} = { + addCookiesCalls: [], + addInitScriptCalls: [], +}; + +function makePage() { + return { + goto: vi.fn().mockResolvedValue({ status: () => 200, url: () => 'https://example.com/', headers: () => ({}) }), + waitForLoadState: vi.fn().mockResolvedValue(undefined), + waitForFunction: vi.fn().mockResolvedValue(undefined), + evaluate: vi.fn().mockResolvedValue({ textLen: 100, nodes: 2 }), + content: vi.fn().mockResolvedValue('ok'), + screenshot: vi.fn().mockResolvedValue(Buffer.from('x')), + setExtraHTTPHeaders: vi.fn().mockResolvedValue(undefined), + on: vi.fn(), + context: () => ({ cookies: () => Promise.resolve([]) }), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +function makeContext() { + return { + addInitScript: vi.fn().mockImplementation((s: string) => { + state.addInitScriptCalls.push(s); + return Promise.resolve(undefined); + }), + addCookies: vi.fn().mockImplementation((c: State['addCookiesCalls'][number]) => { + state.addCookiesCalls.push(c); + return Promise.resolve(undefined); + }), + close: vi.fn().mockResolvedValue(undefined), + newPage: vi.fn().mockResolvedValue(makePage()), + cookies: vi.fn().mockResolvedValue([]), + }; +} + +function makeBrowser() { + return { + newContext: vi.fn().mockImplementation(() => Promise.resolve(makeContext())), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +vi.mock('playwright', () => { + const launch = vi.fn().mockImplementation(() => Promise.resolve(makeBrowser())); + const stub = { launch }; + return { chromium: stub, firefox: stub, webkit: stub }; +}); + +vi.mock('node:dns', () => ({ + lookup: (_h: string, _o: unknown, cb: (e: null, a: Array<{ address: string; family: number }>) => void) => + cb(null, [{ address: '203.0.113.10', family: 4 }]), +})); + +import { MultiBrowserPool } from '../../../src/fetch/browser-pool.js'; + +const FUTURE = Math.floor(Date.now() / 1000) + 86400; +const PAST = Math.floor(Date.now() / 1000) - 86400; + +type State = typeof state; + +function reset() { + state.addCookiesCalls = []; + state.addInitScriptCalls = []; +} + +describe('browser-pool storage-state restore (useAuth)', () => { + beforeEach(() => { + resetConfig(); + reset(); + }); + afterEach(() => { + resetConfig(); + }); + + it('applies storageStatePath cookies (dropping expired ones) via context.addCookies', async () => { + const dir = mkdtempSync(join(tmpdir(), 'wigolo-ss-')); + const stateFile = join(dir, 'state.json'); + writeFileSync( + stateFile, + JSON.stringify({ + cookies: [ + { name: 'SID', value: 'live', domain: '.google.com', path: '/', expires: FUTURE, httpOnly: true, secure: true }, + { name: 'STALE', value: 'dead', domain: '.google.com', path: '/', expires: PAST, httpOnly: true, secure: true }, + ], + origins: [], + }), + ); + try { + const pool = new MultiBrowserPool(); + await pool.fetchWithBrowser('https://example.com/', { storageStatePath: stateFile }); + + expect(state.addCookiesCalls.length).toBe(1); + const applied = state.addCookiesCalls[0]; + const names = applied.map((c) => c.name); + expect(names).toContain('SID'); + expect(names).not.toContain('STALE'); + await pool.shutdown(); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('registers an addInitScript restoring origin localStorage from the storage state', async () => { + const dir = mkdtempSync(join(tmpdir(), 'wigolo-ss-')); + const stateFile = join(dir, 'state.json'); + writeFileSync( + stateFile, + JSON.stringify({ + cookies: [], + origins: [{ origin: 'https://example.com', localStorage: [{ name: 'theme', value: 'dark' }] }], + }), + ); + try { + const pool = new MultiBrowserPool(); + await pool.fetchWithBrowser('https://example.com/', { storageStatePath: stateFile }); + + expect(state.addInitScriptCalls.length).toBe(1); + expect(state.addInitScriptCalls[0]).toContain('theme'); + await pool.shutdown(); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('degrades gracefully when the storage state file is unreadable or malformed', async () => { + const dir = mkdtempSync(join(tmpdir(), 'wigolo-ss-')); + const stateFile = join(dir, 'bad.json'); + writeFileSync(stateFile, 'not-json{'); + try { + const pool = new MultiBrowserPool(); + const result = await pool.fetchWithBrowser('https://example.com/', { storageStatePath: stateFile }); + expect(result).toBeDefined(); + // Nothing was applied, but the fetch itself still succeeded. + expect(state.addCookiesCalls.length).toBe(0); + await pool.shutdown(); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('does nothing when storageStatePath is absent', async () => { + const pool = new MultiBrowserPool(); + await pool.fetchWithBrowser('https://example.com/'); + expect(state.addCookiesCalls.length).toBe(0); + expect(state.addInitScriptCalls.length).toBe(0); + await pool.shutdown(); + }); +}); \ No newline at end of file From a325766b614187346d557811d962f07bdbc17a2e Mon Sep 17 00:00:00 2001 From: MeRezaRezaei Date: Tue, 1 Sep 2026 22:54:04 +0200 Subject: [PATCH 2/2] fix: apply configured auth storageState via a dedicated context Refactor the useAuth fix so storageStatePath is passed through Playwright's native `storageState` option on a DEDICATED per-fetch context + throwaway browser, closed in finally and never returned to the shared pool. This prevents one caller's cookies/localStorage/IndexedDB from leaking to a later pooled fetch, and restores the complete state (including session cookies and IndexedDB) that a manual addCookies re-apply would drop. Adds a sequential reuse test proving the authenticated context is not reused by the next pooled fetch. Refs #526 --- src/fetch/browser-pool.ts | 68 ++++----- .../fetch/browser-pool.storage-state.test.ts | 129 ++++++++++-------- 2 files changed, 97 insertions(+), 100 deletions(-) diff --git a/src/fetch/browser-pool.ts b/src/fetch/browser-pool.ts index 8a76e0cd3..f9fd638f5 100644 --- a/src/fetch/browser-pool.ts +++ b/src/fetch/browser-pool.ts @@ -777,6 +777,33 @@ export class MultiBrowserPool { } throw err; } + } else if (options.storageStatePath) { + // An authenticated fetch requests a logged-in session. A seeded context + // must NEVER be returned to the shared pool: Playwright's pooled contexts + // are reused by later fetches, so restoring storage state onto one would + // leak this caller's cookies/localStorage to whichever fetch reuses that + // context next. Treat it like the dedicated stealth path — its own + // throwaway browser + a per-fetch context created WITH Playwright's + // `storageState` option (which restores cookies, origin localStorage and + // IndexedDB in one call), closed in the finally below and never released. + // The pooled path stays byte-identical when no storageStatePath is given. + resolvedType = this.resolveType(options.browserType, url); + dedicated = true; + log.debug('fetching with browser (authenticated context)', { url, type: resolvedType }); + try { + const cfg = getConfig(); + const proxy = playwrightProxyOption(cfg.proxyUrl, cfg.useProxy); + dedicatedBrowser = await getLauncher(resolvedType).launch({ + headless: true, + env: sanitizedChildEnv({ stripProxy: true }), + ...(proxy ? { proxy } : {}), + }); + ctx = await dedicatedBrowser.newContext({ storageState: options.storageStatePath, acceptDownloads: true }); + } catch (err) { + await dedicatedBrowser?.close().catch(() => {}); + dedicatedBrowser = null; + throw err; + } } else { resolvedType = this.resolveType(options.browserType, url); log.debug('fetching with browser', { url, type: resolvedType }); @@ -797,15 +824,6 @@ export class MultiBrowserPool { .catch(() => {}); } - // Restore an authenticated session (cookies + origin localStorage) from a - // Playwright storage-state file so the fetch runs as the stored account - // rather than a logged-out visitor. Mirrors the injectedCookies pattern: - // applied to whichever context this fetch acquired, guarded for context - // stubs without the Playwright methods (unit-test mocks). - if (options.storageStatePath && typeof (ctx as { addCookies?: unknown }).addCookies === 'function') { - await this.applyStorageState(ctx, options.storageStatePath).catch(() => {}); - } - let page: import('playwright').Page; try { page = await ctx.newPage(); @@ -1318,38 +1336,6 @@ export class MultiBrowserPool { } } - /** - * Restore an authenticated session from a Playwright storage-state file onto - * the given context. Loads cookies via `addCookies` and replays any origin - * localStorage via an init script, so a `useAuth` fetch runs as the stored - * account. Best-effort: any failure logs at debug and is swallowed by the - * caller so a stale/malformed file degrades to a logged-out fetch rather than - * failing it. - */ - private async applyStorageState(ctx: BrowserContext, storageStatePath: string): Promise { - const raw = await readFile(storageStatePath, 'utf8'); - const state = JSON.parse(raw) as { cookies?: import('playwright').Cookie[]; origins?: Array<{ origin: string; localStorage: Array<{ name: string; value: string }> }> }; - const nowSec = Date.now() / 1000; - const live = (state.cookies ?? []).filter((c) => !c.expires || c.expires > nowSec); - - const addCookies = (ctx as { addCookies?: (c: import('playwright').Cookie[]) => Promise }).addCookies; - if (live.length > 0 && typeof addCookies === 'function') { - await addCookies.call(ctx, live); - } - - if (Array.isArray(state.origins)) { - const script = - 'const data = ' + - JSON.stringify(state.origins) + - ';' + - 'for (const o of data) { if (location.origin === o.origin) { for (const { name, value } of o.localStorage) { try { localStorage.setItem(name, value); } catch (e) {} } } }'; - const addInitScript = (ctx as { addInitScript?: (script: string) => Promise }).addInitScript; - if (typeof addInitScript === 'function') { - await addInitScript.call(ctx, script); - } - } - } - /** * Build the concrete injected rung callbacks and run the pure solve ladder in * the LIVE browser context (the in-band constraint — same session that owns diff --git a/tests/unit/fetch/browser-pool.storage-state.test.ts b/tests/unit/fetch/browser-pool.storage-state.test.ts index 20c9a9c9b..69713216d 100644 --- a/tests/unit/fetch/browser-pool.storage-state.test.ts +++ b/tests/unit/fetch/browser-pool.storage-state.test.ts @@ -4,13 +4,17 @@ import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { resetConfig } from '../../../src/config.js'; -// --- programmable page/context behaviour (mirrors browser-pool.clearance.test.ts) --- +// --- programmable browser launcher capturing newContext options --- const state: { - addCookiesCalls: Array>; - addInitScriptCalls: string[]; + newContextOptions: Array>; + closedContexts: number; + closedBrowsers: number; + releasedToPool: number; } = { - addCookiesCalls: [], - addInitScriptCalls: [], + newContextOptions: [], + closedContexts: 0, + closedBrowsers: 0, + releasedToPool: 0, }; function makePage() { @@ -30,15 +34,12 @@ function makePage() { function makeContext() { return { - addInitScript: vi.fn().mockImplementation((s: string) => { - state.addInitScriptCalls.push(s); + addInitScript: vi.fn().mockResolvedValue(undefined), + addCookies: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockImplementation(() => { + state.closedContexts++; return Promise.resolve(undefined); }), - addCookies: vi.fn().mockImplementation((c: State['addCookiesCalls'][number]) => { - state.addCookiesCalls.push(c); - return Promise.resolve(undefined); - }), - close: vi.fn().mockResolvedValue(undefined), newPage: vi.fn().mockResolvedValue(makePage()), cookies: vi.fn().mockResolvedValue([]), }; @@ -46,11 +47,21 @@ function makeContext() { function makeBrowser() { return { - newContext: vi.fn().mockImplementation(() => Promise.resolve(makeContext())), - close: vi.fn().mockResolvedValue(undefined), + newContext: vi.fn().mockImplementation((opts?: Record) => { + state.newContextOptions.push(opts ?? {}); + const ctx = makeContext(); + return Promise.resolve(ctx); + }), + close: vi.fn().mockImplementation(() => { + state.closedBrowsers++; + return Promise.resolve(undefined); + }), }; } +// The pool's context-acquisition path calls pool.launch() -> browser.newContext() +// for the FIRST pooled fetch. For storage-state fetches we launch a DEDICATED +// throwaway browser directly (getLauncher().launch), never touching the pool. vi.mock('playwright', () => { const launch = vi.fn().mockImplementation(() => Promise.resolve(makeBrowser())); const stub = { launch }; @@ -64,14 +75,17 @@ vi.mock('node:dns', () => ({ import { MultiBrowserPool } from '../../../src/fetch/browser-pool.js'; -const FUTURE = Math.floor(Date.now() / 1000) + 86400; -const PAST = Math.floor(Date.now() / 1000) - 86400; - -type State = typeof state; - function reset() { - state.addCookiesCalls = []; - state.addInitScriptCalls = []; + state.newContextOptions = []; + state.closedContexts = 0; + state.closedBrowsers = 0; + state.releasedToPool = 0; +} + +function makeStateFile(dir: string, body: unknown): string { + const path = join(dir, 'state.json'); + writeFileSync(path, JSON.stringify(body)); + return path; } describe('browser-pool storage-state restore (useAuth)', () => { @@ -83,77 +97,74 @@ describe('browser-pool storage-state restore (useAuth)', () => { resetConfig(); }); - it('applies storageStatePath cookies (dropping expired ones) via context.addCookies', async () => { + it('creates a dedicated context seeded WITH the storageState option', async () => { const dir = mkdtempSync(join(tmpdir(), 'wigolo-ss-')); - const stateFile = join(dir, 'state.json'); - writeFileSync( - stateFile, - JSON.stringify({ - cookies: [ - { name: 'SID', value: 'live', domain: '.google.com', path: '/', expires: FUTURE, httpOnly: true, secure: true }, - { name: 'STALE', value: 'dead', domain: '.google.com', path: '/', expires: PAST, httpOnly: true, secure: true }, - ], - origins: [], - }), - ); + const stateFile = makeStateFile(dir, { + cookies: [{ name: 'SID', value: 'live', domain: '.google.com', path: '/', expires: 1893456000 }], + origins: [], + }); try { const pool = new MultiBrowserPool(); await pool.fetchWithBrowser('https://example.com/', { storageStatePath: stateFile }); - expect(state.addCookiesCalls.length).toBe(1); - const applied = state.addCookiesCalls[0]; - const names = applied.map((c) => c.name); - expect(names).toContain('SID'); - expect(names).not.toContain('STALE'); + // A dedicated context was created for the auth fetch, with the full + // storageState handed to Playwright (not manually re-applied). + const ctxOpts = state.newContextOptions; + expect(ctxOpts.length).toBeGreaterThan(0); + expect(ctxOpts[0].storageState).toBe(stateFile); await pool.shutdown(); } finally { rmSync(dir, { recursive: true, force: true }); } }); - it('registers an addInitScript restoring origin localStorage from the storage state', async () => { + it('closes the dedicated context + browser (never released to the pool)', async () => { const dir = mkdtempSync(join(tmpdir(), 'wigolo-ss-')); - const stateFile = join(dir, 'state.json'); - writeFileSync( - stateFile, - JSON.stringify({ - cookies: [], - origins: [{ origin: 'https://example.com', localStorage: [{ name: 'theme', value: 'dark' }] }], - }), - ); + const stateFile = makeStateFile(dir, { cookies: [], origins: [] }); try { const pool = new MultiBrowserPool(); await pool.fetchWithBrowser('https://example.com/', { storageStatePath: stateFile }); - expect(state.addInitScriptCalls.length).toBe(1); - expect(state.addInitScriptCalls[0]).toContain('theme'); + expect(state.closedContexts).toBeGreaterThan(0); + expect(state.closedBrowsers).toBeGreaterThan(0); await pool.shutdown(); } finally { rmSync(dir, { recursive: true, force: true }); } }); - it('degrades gracefully when the storage state file is unreadable or malformed', async () => { + it('does not leak account state to a later pooled fetch', async () => { const dir = mkdtempSync(join(tmpdir(), 'wigolo-ss-')); - const stateFile = join(dir, 'bad.json'); - writeFileSync(stateFile, 'not-json{'); + const stateFile = makeStateFile(dir, { + cookies: [{ name: 'SID', value: 'ACCOUNT_A', domain: '.google.com', path: '/', expires: 1893456000 }], + origins: [], + }); try { const pool = new MultiBrowserPool(); - const result = await pool.fetchWithBrowser('https://example.com/', { storageStatePath: stateFile }); - expect(result).toBeDefined(); - // Nothing was applied, but the fetch itself still succeeded. - expect(state.addCookiesCalls.length).toBe(0); + + // Authenticated fetch (dedicated context seeded with ACCOUNT_A state). + await pool.fetchWithBrowser('https://account-a.example/', { storageStatePath: stateFile }); + + // Anonymous fetch straight after — must get a POOLED context with NO + // storageState, so it cannot see ACCOUNT_A's cookies. + await pool.fetchWithBrowser('https://anonymous.example/'); + + const anonymousCtxOpts = state.newContextOptions.at(-1) ?? {}; + expect(anonymousCtxOpts.storageState).toBeUndefined(); await pool.shutdown(); } finally { rmSync(dir, { recursive: true, force: true }); } }); - it('does nothing when storageStatePath is absent', async () => { + it('left the pooled path byte-identical when no storageStatePath is given', async () => { const pool = new MultiBrowserPool(); await pool.fetchWithBrowser('https://example.com/'); - expect(state.addCookiesCalls.length).toBe(0); - expect(state.addInitScriptCalls.length).toBe(0); + + const ctxOpts = state.newContextOptions; + expect(ctxOpts.length).toBeGreaterThan(0); + // Pooled acquisition creates a context WITHOUT storageState. + expect(ctxOpts[0].storageState).toBeUndefined(); await pool.shutdown(); }); }); \ No newline at end of file