From 1d3233800a10aa271cea26fc6870e8b723f59526 Mon Sep 17 00:00:00 2001 From: josephkehan-prog Date: Sat, 18 Jul 2026 12:33:57 -0400 Subject: [PATCH 1/4] fix: consume and clean up copied Chrome profile in authenticated browser fetch (#161) --- src/fetch/auth.ts | 12 +- src/fetch/browser-pool.ts | 43 +++- src/fetch/profile-copy.ts | 49 +++++ src/fetch/router.ts | 49 +++-- tests/unit/fetch/browser-pool.profile.test.ts | 208 ++++++++++++++++++ tests/unit/fetch/profile-copy.test.ts | 61 +++++ tests/unit/fetch/router-auth-profile.test.ts | 159 +++++++++++++ 7 files changed, 555 insertions(+), 26 deletions(-) create mode 100644 src/fetch/profile-copy.ts create mode 100644 tests/unit/fetch/browser-pool.profile.test.ts create mode 100644 tests/unit/fetch/profile-copy.test.ts create mode 100644 tests/unit/fetch/router-auth-profile.test.ts diff --git a/src/fetch/auth.ts b/src/fetch/auth.ts index 9f8a2674..aaa6519b 100644 --- a/src/fetch/auth.ts +++ b/src/fetch/auth.ts @@ -1,9 +1,9 @@ -import { existsSync, cpSync, mkdtempSync } from 'node:fs'; +import { existsSync } from 'node:fs'; import { join } from 'node:path'; -import { tmpdir } from 'node:os'; import { getConfig } from '../config.js'; import { createLogger } from '../logger.js'; import { discoverSessions, isCDPReachable } from './cdp-client.js'; +import { copyProfileToTemp } from './profile-copy.js'; import type { CDPSession } from '../types.js'; export interface AuthOptions { @@ -30,10 +30,10 @@ export async function getAuthOptions(): Promise { profilePath: config.chromeProfilePath, }); } - const tempDir = mkdtempSync(join(tmpdir(), 'wigolo-chrome-')); - cpSync(config.chromeProfilePath, tempDir, { recursive: true }); - logger.debug('copied Chrome profile to temp directory', { from: config.chromeProfilePath, to: tempDir }); - return { userDataDir: tempDir }; + // Single-use temp copy: consumed by the browser tier's persistent-context + // launch and removed by the router (removeTempProfile) once the fetch + // settles — success, failure, or abort. + return { userDataDir: copyProfileToTemp(config.chromeProfilePath) }; } if (config.cdpUrl) { diff --git a/src/fetch/browser-pool.ts b/src/fetch/browser-pool.ts index f8305412..8f1b1d0d 100644 --- a/src/fetch/browser-pool.ts +++ b/src/fetch/browser-pool.ts @@ -70,6 +70,13 @@ export class ChallengeBlockedError extends Error { export interface BrowserFetchOptions { timeoutMs?: number; storageStatePath?: string; + /** + * Temp Chrome-profile copy (made by getAuthOptions via profile-copy.ts). + * Consumed as a DEDICATED `launchPersistentContext` so the profile's + * cookies/logins are presented to the site. The context is closed at + * end-of-fetch; the CALLER owns the copy and removes it afterwards. + * `cdpUrl` takes precedence when both are set. + */ userDataDir?: string; headers?: Record; screenshot?: boolean; @@ -409,8 +416,9 @@ export class MultiBrowserPool { let advertisedUa: string | null = null; // Stealth applies only to the launch path — the CDP path connects to an - // external browser that owns its own fingerprint. - const useStealth = options.stealth === true && !options.cdpUrl; + // external browser that owns its own fingerprint, and the persistent-profile + // path must present the profile's own fingerprint, not a hardened one. + const useStealth = options.stealth === true && !options.cdpUrl && !options.userDataDir; if (options.cdpUrl) { // CDP is always Chromium @@ -427,6 +435,37 @@ export class MultiBrowserPool { }); ctx = await this.acquireForType(resolvedType); } + } else if (options.userDataDir) { + // Authenticated profile fetch (WIGOLO_CHROME_PROFILE_PATH): launch a + // DEDICATED persistent context from the temp profile copy so the + // profile's cookies/logins are actually presented to the site. A + // Chrome-format profile is Chromium-only. launchPersistentContext owns + // its browser process (no separate Browser handle), so closing the + // context in the finally tears everything down; the CALLER owns the temp + // copy and removes it after this fetch settles (see profile-copy.ts). + // Bounded by the same dedicated-path semaphore as stealth so a burst of + // authenticated fetches cannot exceed the browser cap. + resolvedType = 'chromium'; + await this.acquireStealthSlot(); + stealthSlotHeld = true; + dedicated = true; + log.debug('fetching with browser (persistent profile context)', { url, userDataDir: options.userDataDir }); + try { + const cfgProfile = getConfig(); + const proxy = playwrightProxyOption(cfgProfile.proxyUrl, cfgProfile.useProxy); + ctx = await chromium.launchPersistentContext(options.userDataDir, { + headless: true, + acceptDownloads: true, + env: sanitizedChildEnv({ stripProxy: true }), + ...(proxy ? { proxy } : {}), + }); + } catch (err) { + // Free the slot before rethrowing so N launch failures cannot exhaust + // the semaphore (mirrors the stealth setup error path below). + this.releaseStealthSlot(); + stealthSlotHeld = false; + throw err; + } } else if (useStealth) { resolvedType = this.resolveType(options.browserType, url); // Bound concurrency BEFORE launching so a burst cannot exceed the cap. diff --git a/src/fetch/profile-copy.ts b/src/fetch/profile-copy.ts new file mode 100644 index 00000000..5202f704 --- /dev/null +++ b/src/fetch/profile-copy.ts @@ -0,0 +1,49 @@ +import { cpSync, mkdtempSync, rmSync } from 'node:fs'; +import { basename, join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { createLogger } from '../logger.js'; + +const logger = createLogger('fetch'); + +/** + * Prefix for temp Chrome-profile copies. Doubles as the deletion guard in + * `removeTempProfile` so cleanup can never touch a user-configured directory. + */ +export const TEMP_PROFILE_PREFIX = 'wigolo-chrome-'; + +/** + * Copy the user's Chrome profile into a fresh temp directory so the browser + * tier can open it without touching (or locking) the live profile. The copy is + * SINGLE-USE and caller-owned: whoever triggers the copy MUST remove it with + * `removeTempProfile` once the fetch settles (success, failure, or abort) — + * a surviving copy is a full-profile privacy leak in tmp. + */ +export function copyProfileToTemp(profilePath: string): string { + const tempDir = mkdtempSync(join(tmpdir(), TEMP_PROFILE_PREFIX)); + cpSync(profilePath, tempDir, { recursive: true }); + logger.debug('copied Chrome profile to temp directory', { from: profilePath, to: tempDir }); + return tempDir; +} + +/** + * Remove a temp Chrome-profile copy created by `copyProfileToTemp`. Guarded to + * wigolo-owned temp copies only (the `wigolo-chrome-` prefix) and best-effort: + * a cleanup failure is logged, never thrown, so it cannot mask the fetch's own + * outcome. No-op when no copy was made (`userDataDir` undefined). + */ +export function removeTempProfile(userDataDir: string | undefined): void { + if (!userDataDir) return; + if (!basename(userDataDir).startsWith(TEMP_PROFILE_PREFIX)) { + logger.warn('refusing to remove a directory that is not a wigolo temp profile copy', { userDataDir }); + return; + } + try { + rmSync(userDataDir, { recursive: true, force: true }); + logger.debug('removed temp Chrome profile copy', { userDataDir }); + } catch (err) { + logger.warn('failed to remove temp Chrome profile copy', { + userDataDir, + error: err instanceof Error ? err.message : String(err), + }); + } +} diff --git a/src/fetch/router.ts b/src/fetch/router.ts index 94270edc..612a06f5 100644 --- a/src/fetch/router.ts +++ b/src/fetch/router.ts @@ -1,7 +1,8 @@ import { getConfig, type Config } from '../config.js'; import { createLogger } from '../logger.js'; import { contentAppearsEmpty } from './content-check.js'; -import { getAuthOptions } from './auth.js'; +import { getAuthOptions, type AuthOptions } from './auth.js'; +import { removeTempProfile } from './profile-copy.js'; import { fetchWithPlaywright, shouldEscalate } from './playwright-tier.js'; import { describeFetchError } from './error-describe.js'; import { @@ -904,32 +905,44 @@ export class SmartRouter { // Actions always force Playwright --- actions need a live browser page if (actions && actions.length > 0) { if (!this.browserPool) throw new Error('SmartRouter: browserPool not configured'); - const authOptions = useAuth ? (await getAuthOptions() ?? {}) : {}; + const authOptions: AuthOptions = useAuth ? (await getAuthOptions() ?? {}) : {}; logger.debug('routing to playwright', { url, reason: 'actions present' }); - return this.browserFetch(url, { - headers, - screenshot, - actions, - ...authOptions, - signal, - stealth: stealthForBrowser(config, { antiBotEscalation: false }), - }); + try { + return await this.browserFetch(url, { + headers, + screenshot, + actions, + ...authOptions, + signal, + stealth: stealthForBrowser(config, { antiBotEscalation: false }), + }); + } finally { + // The temp Chrome-profile copy (if getAuthOptions made one) is + // single-use — remove it once the fetch settles (success, failure, or + // abort) so no full-profile copy survives in tmp. + removeTempProfile(authOptions.userDataDir); + } } // Always Playwright for auth or explicit override if (renderJs === 'always' || useAuth) { if (!this.browserPool) throw new Error('SmartRouter: browserPool not configured'); - const authOptions = useAuth ? (await getAuthOptions() ?? {}) : {}; + const authOptions: AuthOptions = useAuth ? (await getAuthOptions() ?? {}) : {}; logger.debug('routing to playwright', { url, reason: useAuth ? 'auth' : 'render_js=always' }); // Explicit browser request (auth / render_js:always) — not an anti-bot // escalation, so 'auto' leaves it unhardened; 'on' still hardens. - return this.browserFetch(url, { - headers, - screenshot, - ...authOptions, - signal, - stealth: stealthForBrowser(config, { antiBotEscalation: false }), - }); + try { + return await this.browserFetch(url, { + headers, + screenshot, + ...authOptions, + signal, + stealth: stealthForBrowser(config, { antiBotEscalation: false }), + }); + } finally { + // Single-use temp Chrome-profile copy — see the actions path above. + removeTempProfile(authOptions.userDataDir); + } } // HTTP only, no fallback diff --git a/tests/unit/fetch/browser-pool.profile.test.ts b/tests/unit/fetch/browser-pool.profile.test.ts new file mode 100644 index 00000000..257e1ef3 --- /dev/null +++ b/tests/unit/fetch/browser-pool.profile.test.ts @@ -0,0 +1,208 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { resetConfig } from '../../../src/config.js'; + +// Shared mock state for the persistent-profile (userDataDir) launch path. +interface ProfileState { + persistentContextsCreated: number; + persistentContextsClosed: number; + addInitScriptCalls: number; + cdpConnects: number; + // When set, page.goto rejects — simulates a fetch failure after launch. + gotoThrows: boolean; + // When set, launchPersistentContext rejects — simulates a launch failure. + persistentLaunchThrows: boolean; +} + +const state: ProfileState = { + persistentContextsCreated: 0, + persistentContextsClosed: 0, + addInitScriptCalls: 0, + cdpConnects: 0, + gotoThrows: false, + persistentLaunchThrows: false, +}; + +function makePage() { + return { + goto: vi.fn().mockImplementation(() => { + if (state.gotoThrows) return Promise.reject(new Error('goto boom')); + return Promise.resolve({ + status: () => 200, + url: () => 'https://example.com', + headers: () => ({ 'content-type': 'text/html' }), + }); + }), + waitForLoadState: vi.fn().mockResolvedValue(undefined), + waitForFunction: vi.fn().mockResolvedValue(undefined), + content: vi.fn().mockResolvedValue('ok'), + screenshot: vi.fn().mockResolvedValue(Buffer.from('x')), + setExtraHTTPHeaders: vi.fn().mockResolvedValue(undefined), + on: vi.fn(), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +function makePersistentContext() { + state.persistentContextsCreated++; + return { + addInitScript: vi.fn().mockImplementation(() => { + state.addInitScriptCalls++; + return Promise.resolve(undefined); + }), + close: vi.fn().mockImplementation(() => { + state.persistentContextsClosed++; + return Promise.resolve(undefined); + }), + newPage: vi.fn().mockResolvedValue(makePage()), + cookies: vi.fn().mockResolvedValue([]), + }; +} + +function makePooledContext() { + return { + close: vi.fn().mockResolvedValue(undefined), + newPage: vi.fn().mockResolvedValue(makePage()), + cookies: vi.fn().mockResolvedValue([]), + }; +} + +function makeBrowser() { + return { + newContext: vi.fn().mockImplementation(() => Promise.resolve(makePooledContext())), + contexts: vi.fn().mockReturnValue([makePooledContext()]), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +vi.mock('playwright', () => { + const launch = vi.fn().mockImplementation(() => Promise.resolve(makeBrowser())); + const launchPersistentContext = vi.fn().mockImplementation(() => { + if (state.persistentLaunchThrows) return Promise.reject(new Error('persistent launch boom')); + return Promise.resolve(makePersistentContext()); + }); + const connectOverCDP = vi.fn().mockImplementation(() => { + state.cdpConnects++; + return Promise.resolve(makeBrowser()); + }); + return { + chromium: { launch, launchPersistentContext, connectOverCDP }, + firefox: { launch }, + webkit: { launch }, + }; +}); + +import { chromium } from 'playwright'; +import { MultiBrowserPool } from '../../../src/fetch/browser-pool.js'; + +function resetState() { + state.persistentContextsCreated = 0; + state.persistentContextsClosed = 0; + state.addInitScriptCalls = 0; + state.cdpConnects = 0; + state.gotoThrows = false; + state.persistentLaunchThrows = false; +} + +describe('browser-pool persistent profile (userDataDir) path — issue #161', () => { + beforeEach(() => { + resetConfig(); + resetState(); + vi.mocked(chromium.launchPersistentContext).mockClear(); + vi.mocked(chromium.connectOverCDP).mockClear(); + }); + + it('launches a persistent context FROM the copied profile dir and closes it at end-of-fetch', async () => { + const pool = new MultiBrowserPool(); + const proto = Object.getPrototypeOf(pool) as { + releaseForType: (...args: unknown[]) => void; + }; + const releaseSpy = vi.spyOn(proto, 'releaseForType'); + + const result = await pool.fetchWithBrowser('https://intranet.example', { + userDataDir: '/tmp/wigolo-chrome-abc123', + }); + expect(result.method).toBe('browser'); + + // The copied profile dir was actually consumed (regression for the dead + // userDataDir option): passed as the persistent-context user data dir. + expect(chromium.launchPersistentContext).toHaveBeenCalledTimes(1); + expect(vi.mocked(chromium.launchPersistentContext).mock.calls[0][0]).toBe('/tmp/wigolo-chrome-abc123'); + expect(vi.mocked(chromium.launchPersistentContext).mock.calls[0][1]).toMatchObject({ headless: true }); + + // Dedicated lifecycle: closed at end-of-fetch, never handed to the pool. + expect(state.persistentContextsCreated).toBe(1); + expect(state.persistentContextsClosed).toBe(1); + expect(releaseSpy).not.toHaveBeenCalled(); + expect(pool.getStats()[0].pooledCount).toBe(0); + + releaseSpy.mockRestore(); + await pool.shutdown(); + }); + + it('closes the persistent context even when the fetch fails (goto rejects)', async () => { + state.gotoThrows = true; + const pool = new MultiBrowserPool(); + + await expect( + pool.fetchWithBrowser('https://intranet.example', { userDataDir: '/tmp/wigolo-chrome-err' }), + ).rejects.toThrow(/goto boom/); + + expect(state.persistentContextsCreated).toBe(1); + expect(state.persistentContextsClosed).toBe(1); + + await pool.shutdown(); + }); + + it('cdpUrl takes precedence over userDataDir (WIGOLO_CDP_URL path unchanged)', async () => { + const pool = new MultiBrowserPool(); + + const result = await pool.fetchWithBrowser('https://intranet.example', { + cdpUrl: 'http://localhost:9222', + userDataDir: '/tmp/wigolo-chrome-abc123', + }); + expect(result.method).toBe('browser'); + + expect(state.cdpConnects).toBe(1); + expect(chromium.launchPersistentContext).not.toHaveBeenCalled(); + + await pool.shutdown(); + }); + + it('userDataDir wins over stealth — the profile fingerprint is presented, not the hardened one', async () => { + const pool = new MultiBrowserPool(); + + const result = await pool.fetchWithBrowser('https://intranet.example', { + userDataDir: '/tmp/wigolo-chrome-abc123', + stealth: true, + }); + expect(result.method).toBe('browser'); + + expect(chromium.launchPersistentContext).toHaveBeenCalledTimes(1); + // The stealth init script must NOT be applied to the profile context. + expect(state.addInitScriptCalls).toBe(0); + expect(state.persistentContextsClosed).toBe(1); + + await pool.shutdown(); + }); + + it('a persistent-launch failure frees the dedicated slot (no semaphore leak)', async () => { + process.env.MAX_BROWSERS = '1'; + resetConfig(); + state.persistentLaunchThrows = true; + + const pool = new MultiBrowserPool(); + await expect( + pool.fetchWithBrowser('https://intranet.example', { userDataDir: '/tmp/wigolo-chrome-fail' }), + ).rejects.toThrow(/persistent launch boom/); + + // With limit=1, a leaked slot would hang the next dedicated fetch. + state.persistentLaunchThrows = false; + const result = await pool.fetchWithBrowser('https://intranet.example', { + userDataDir: '/tmp/wigolo-chrome-ok', + }); + expect(result.method).toBe('browser'); + + delete process.env.MAX_BROWSERS; + await pool.shutdown(); + }); +}); diff --git a/tests/unit/fetch/profile-copy.test.ts b/tests/unit/fetch/profile-copy.test.ts new file mode 100644 index 00000000..d3c78d6f --- /dev/null +++ b/tests/unit/fetch/profile-copy.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { copyProfileToTemp, removeTempProfile, TEMP_PROFILE_PREFIX } from '../../../src/fetch/profile-copy.js'; + +describe('profile-copy', () => { + let sourceDir: string; + const madeCopies: string[] = []; + + beforeEach(() => { + sourceDir = mkdtempSync(join(tmpdir(), 'wigolo-profile-src-')); + writeFileSync(join(sourceDir, 'Cookies'), 'cookie-bytes'); + mkdirSync(join(sourceDir, 'Default')); + writeFileSync(join(sourceDir, 'Default', 'Preferences'), '{}'); + }); + + afterEach(() => { + rmSync(sourceDir, { recursive: true, force: true }); + for (const dir of madeCopies.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('copyProfileToTemp copies the profile recursively into a prefixed temp dir', () => { + const copy = copyProfileToTemp(sourceDir); + madeCopies.push(copy); + + expect(copy).not.toBe(sourceDir); + expect(copy).toContain(TEMP_PROFILE_PREFIX); + expect(readFileSync(join(copy, 'Cookies'), 'utf8')).toBe('cookie-bytes'); + expect(existsSync(join(copy, 'Default', 'Preferences'))).toBe(true); + }); + + it('removeTempProfile deletes a wigolo temp copy', () => { + const copy = copyProfileToTemp(sourceDir); + madeCopies.push(copy); + expect(existsSync(copy)).toBe(true); + + removeTempProfile(copy); + expect(existsSync(copy)).toBe(false); + }); + + it('removeTempProfile is a no-op for undefined', () => { + expect(() => removeTempProfile(undefined)).not.toThrow(); + }); + + it('removeTempProfile refuses to delete a directory without the wigolo prefix', () => { + // A user-configured directory (e.g. the LIVE profile path) must never be + // deleted, even if it is mistakenly passed in. + removeTempProfile(sourceDir); + expect(existsSync(sourceDir)).toBe(true); + expect(existsSync(join(sourceDir, 'Cookies'))).toBe(true); + }); + + it('removeTempProfile tolerates an already-removed directory', () => { + const copy = copyProfileToTemp(sourceDir); + rmSync(copy, { recursive: true, force: true }); + expect(() => removeTempProfile(copy)).not.toThrow(); + }); +}); diff --git a/tests/unit/fetch/router-auth-profile.test.ts b/tests/unit/fetch/router-auth-profile.test.ts new file mode 100644 index 00000000..89893375 --- /dev/null +++ b/tests/unit/fetch/router-auth-profile.test.ts @@ -0,0 +1,159 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { join, basename } from 'node:path'; +import { tmpdir } from 'node:os'; +import { resetConfig } from '../../../src/config.js'; + +// Issue #161 regression: the WIGOLO_CHROME_PROFILE_PATH temp copy must be +// (a) handed to the browser tier as userDataDir and (b) removed once the fetch +// settles — success, failure, and challenge-block alike. auth.js is +// deliberately NOT mocked here so the REAL copy is made and cleaned up. + +// Browser-acquire mock — report the engine "ready" without a real install +// (same rationale as router.test.ts). +vi.mock('../../../src/fetch/browser-acquire.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + BrowserAcquirer: class { + ensureBrowser = vi.fn(async () => 'ready'); + }, + }; +}); + +import { SmartRouter } from '../../../src/fetch/router.js'; +import type { BrowserPoolInterface, BrowserFetchArgs } from '../../../src/fetch/router.js'; +import { ChallengeBlockedError } from '../../../src/fetch/browser-pool.js'; +import type { RawFetchResult } from '../../../src/types.js'; + +const FULL_HTML = `

${'real content long enough. '.repeat(20)}

`; + +function makeBrowserResult(url: string): RawFetchResult { + return { + url, + finalUrl: url, + html: FULL_HTML, + contentType: 'text/html', + statusCode: 200, + method: 'browser', + headers: {}, + }; +} + +describe('SmartRouter useAuth with WIGOLO_CHROME_PROFILE_PATH (issue #161)', () => { + const originalEnv = process.env; + let profileDir: string; + // Captured by the pool mock at fetch time. + let seenOptions: BrowserFetchArgs | undefined; + let dirExistedDuringFetch: boolean; + + function makeRouter(pool: BrowserPoolInterface): SmartRouter { + return new SmartRouter({ + httpClient: { fetch: vi.fn(async () => { throw new Error('http tier must not run'); }) }, + browserPool: pool, + pdfProbe: async () => false, + }); + } + + function capturingPool( + respond: (url: string) => Promise, + ): BrowserPoolInterface { + return { + fetchWithBrowser: vi.fn(async (url: string, options?: BrowserFetchArgs) => { + seenOptions = options; + dirExistedDuringFetch = options?.userDataDir ? existsSync(options.userDataDir) : false; + return respond(url); + }), + }; + } + + beforeEach(() => { + process.env = { ...originalEnv }; + delete process.env.WIGOLO_AUTH_STATE_PATH; + delete process.env.WIGOLO_CDP_URL; + profileDir = mkdtempSync(join(tmpdir(), 'wigolo-profile-src-')); + writeFileSync(join(profileDir, 'Cookies'), 'cookie-bytes'); + process.env.WIGOLO_CHROME_PROFILE_PATH = profileDir; + resetConfig(); + seenOptions = undefined; + dirExistedDuringFetch = false; + }); + + afterEach(() => { + rmSync(profileDir, { recursive: true, force: true }); + // Belt-and-braces: never leave a temp copy behind even on test failure. + if (seenOptions?.userDataDir) { + rmSync(seenOptions.userDataDir, { recursive: true, force: true }); + } + process.env = originalEnv; + resetConfig(); + vi.clearAllMocks(); + }); + + it('passes the temp profile copy to the browser tier and removes it after a successful fetch', async () => { + const router = makeRouter(capturingPool(async (url) => makeBrowserResult(url))); + + const result = await router.fetch('https://intranet.example/page', { useAuth: true }); + expect(result.method).toBe('browser'); + + // (a) the copy was consumed: passed to fetchWithBrowser and alive at fetch time. + expect(seenOptions?.userDataDir).toBeDefined(); + expect(seenOptions?.userDataDir).not.toBe(profileDir); + expect(basename(seenOptions!.userDataDir!)).toContain('wigolo-chrome-'); + expect(dirExistedDuringFetch).toBe(true); + + // (b) the copy is gone once the fetch settled; the source profile is untouched. + expect(existsSync(seenOptions!.userDataDir!)).toBe(false); + expect(existsSync(join(profileDir, 'Cookies'))).toBe(true); + }); + + it('removes the temp profile copy when the browser fetch fails', async () => { + const router = makeRouter(capturingPool(async () => { throw new Error('browser boom'); })); + + await expect( + router.fetch('https://intranet.example/page', { useAuth: true }), + ).rejects.toThrow(/browser boom/); + + expect(seenOptions?.userDataDir).toBeDefined(); + expect(dirExistedDuringFetch).toBe(true); + expect(existsSync(seenOptions!.userDataDir!)).toBe(false); + }); + + it('removes the temp profile copy when the fetch is aborted', async () => { + const router = makeRouter(capturingPool(async () => { + throw new DOMException('stage_timeout', 'AbortError'); + })); + + await expect( + router.fetch('https://intranet.example/page', { useAuth: true }), + ).rejects.toBeTruthy(); + + expect(seenOptions?.userDataDir).toBeDefined(); + expect(existsSync(seenOptions!.userDataDir!)).toBe(false); + }); + + it('removes the temp profile copy on a challenge-block (structured stage error path)', async () => { + const router = makeRouter(capturingPool(async () => { + throw new ChallengeBlockedError('https://intranet.example/page'); + })); + + const result = await router.fetch('https://intranet.example/page', { useAuth: true }); + expect((result as { error?: string }).error).toBe('blocked_by_challenge'); + + expect(seenOptions?.userDataDir).toBeDefined(); + expect(existsSync(seenOptions!.userDataDir!)).toBe(false); + }); + + it('removes the temp profile copy on the actions path too', async () => { + const router = makeRouter(capturingPool(async (url) => makeBrowserResult(url))); + + const result = await router.fetch('https://intranet.example/page', { + useAuth: true, + actions: [{ type: 'wait_for', selector: 'body' }], + }); + expect(result.method).toBe('browser'); + + expect(seenOptions?.userDataDir).toBeDefined(); + expect(existsSync(seenOptions!.userDataDir!)).toBe(false); + }); +}); From 3696ca43e8319d40ea14f192ff403d03f7f5dfc2 Mon Sep 17 00:00:00 2001 From: josephkehan-prog Date: Sun, 19 Jul 2026 16:02:13 -0400 Subject: [PATCH 2/4] fix(cache): parse zone-less UTC timestamps as UTC in expiry checks (#208) toIsoSeconds() persists "YYYY-MM-DD HH:MM:SS" (UTC, matching SQLite's datetime('now')), but isExpired/isCacheUsable/getCachedSearch parsed it back with new Date(), which treats the zone-less space-separated form as local time. Every TTL comparison shifted by the host's UTC offset: west-of-UTC hosts served expired rows as fresh, east-of-UTC expired early. Add parseUtcTimestamp() that re-attaches the UTC marker for the zone-less format and falls through unchanged otherwise; swap the three call sites. Regression tests flip TZ to UTC-8 at runtime and self-skip where the runtime ignores TZ changes. --- src/cache/store.ts | 20 ++++++++-- tests/unit/cache/store.test.ts | 69 ++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/src/cache/store.ts b/src/cache/store.ts index 241a80a6..cfb260dd 100644 --- a/src/cache/store.ts +++ b/src/cache/store.ts @@ -74,6 +74,20 @@ function toIsoSeconds(date: Date): string { return date.toISOString().replace('T', ' ').replace(/\.\d+Z$/, ''); } +const ZONELESS_UTC_TIMESTAMP = /^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/; + +// Timestamps are persisted by toIsoSeconds() (and SQLite's datetime('now')) +// as zone-less UTC: "YYYY-MM-DD HH:MM:SS". JavaScript's Date parser treats +// that space-separated form as LOCAL time, which shifts every expiry +// comparison by the host's UTC offset. Re-attach the UTC marker before +// parsing; leave any other format untouched. +function parseUtcTimestamp(value: string): number { + const normalized = ZONELESS_UTC_TIMESTAMP.test(value) + ? `${value.replace(' ', 'T')}Z` + : value; + return new Date(normalized).getTime(); +} + export function cacheContent(result: RawFetchResult, extraction: ExtractionResult): void { try { const db = getDatabase(); @@ -282,7 +296,7 @@ export function getMarkdownForNormalizedUrl(normalizedUrl: string): string | nul export function isExpired(cached: CachedContent): boolean { if (!cached.expiresAt) return false; - return new Date(cached.expiresAt).getTime() < Date.now(); + return parseUtcTimestamp(cached.expiresAt) < Date.now(); } export interface CacheLookupOptions { @@ -294,7 +308,7 @@ export function isCacheUsable( opts: CacheLookupOptions = {}, ): { usable: boolean; stale: boolean } { if (!cached.expiresAt) return { usable: true, stale: false }; - const expiresMs = new Date(cached.expiresAt).getTime(); + const expiresMs = parseUtcTimestamp(cached.expiresAt); const now = Date.now(); if (expiresMs >= now) return { usable: true, stale: false }; const staleMaxMs = (opts.staleMaxSeconds ?? 0) * 1000; @@ -433,7 +447,7 @@ export function getCachedSearchResults( if (!row) return null; if (row.expires_at) { - const expiresMs = new Date(row.expires_at).getTime(); + const expiresMs = parseUtcTimestamp(row.expires_at); const now = Date.now(); if (expiresMs < now) { const staleMaxMs = (opts.staleMaxSeconds ?? 0) * 1000; diff --git a/tests/unit/cache/store.test.ts b/tests/unit/cache/store.test.ts index 6beed513..64a6544b 100644 --- a/tests/unit/cache/store.test.ts +++ b/tests/unit/cache/store.test.ts @@ -5,6 +5,7 @@ import { cacheContent, getCachedContent, isExpired, + isCacheUsable, searchCache, cacheSearchResults, getCachedSearchResults, @@ -718,3 +719,71 @@ describe('getHashAndStatusForNormalizedUrl', () => { expect(combined.status).toBe(200); }); }); + +describe('timezone-independent expiry (issue #208)', () => { + // Stored timestamps are zone-less UTC ("YYYY-MM-DD HH:MM:SS"). The bug: + // new Date() parses that form as LOCAL time, shifting expiry by the host's + // UTC offset. These tests flip TZ west of UTC at runtime; on hosts where + // the runtime ignores a TZ change (e.g. Windows), they skip themselves. + const originalTz = process.env.TZ; + + function tzFlipTookEffect(): boolean { + process.env.TZ = 'Etc/GMT+8'; // POSIX sign convention: UTC-8 + return new Date().getTimezoneOffset() === 480; + } + + afterEach(() => { + if (originalTz === undefined) delete process.env.TZ; + else process.env.TZ = originalTz; + }); + + function zonelessUtc(msFromNow: number): string { + return new Date(Date.now() + msFromNow) + .toISOString() + .replace('T', ' ') + .replace(/\.\d+Z$/, ''); + } + + function makeCached(expiresAt: string): CachedContent { + return { + url: 'https://tz.test/', + normalizedUrl: 'https://tz.test', + title: 't', + markdown: 'm', + metadata: {}, + links: [], + images: [], + fetchMethod: 'http', + extractorUsed: 'defuddle', + contentHash: 'h', + fetchedAt: zonelessUtc(-3_600_000), + expiresAt, + } as unknown as CachedContent; + } + + it('isExpired stays true for a just-expired row under a UTC-8 clock', () => { + if (!tzFlipTookEffect()) return; + expect(isExpired(makeCached(zonelessUtc(-60_000)))).toBe(true); + }); + + it('isExpired stays false for a not-yet-expired row under a UTC-8 clock', () => { + if (!tzFlipTookEffect()) return; + expect(isExpired(makeCached(zonelessUtc(60_000)))).toBe(false); + }); + + it('isCacheUsable marks a row inside the stale window stale under a UTC-8 clock', () => { + if (!tzFlipTookEffect()) return; + const out = isCacheUsable(makeCached(zonelessUtc(-3_600_000)), { + staleMaxSeconds: 24 * 3600, + }); + expect(out).toEqual({ usable: true, stale: true }); + }); + + it('isCacheUsable rejects a row past the stale window under a UTC-8 clock', () => { + if (!tzFlipTookEffect()) return; + const out = isCacheUsable(makeCached(zonelessUtc(-25 * 3_600_000)), { + staleMaxSeconds: 24 * 3600, + }); + expect(out).toEqual({ usable: false, stale: false }); + }); +}); From bb4ed3a99e9948ede7eebe1aab1edbd6dd8909b0 Mon Sep 17 00:00:00 2001 From: boredinnyc Date: Thu, 23 Jul 2026 12:46:48 -0400 Subject: [PATCH 3/4] test(fetch): add page.evaluate mock to persistent-profile tests Main's shared settlePage (SPA settle overhaul) reads content metrics and the final DOM verdict via page.evaluate; the userDataDir page mock predates that path and crashed all four profile tests with 'page.evaluate is not a function'. --- tests/unit/fetch/browser-pool.profile.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/unit/fetch/browser-pool.profile.test.ts b/tests/unit/fetch/browser-pool.profile.test.ts index 257e1ef3..9eb941f3 100644 --- a/tests/unit/fetch/browser-pool.profile.test.ts +++ b/tests/unit/fetch/browser-pool.profile.test.ts @@ -34,6 +34,14 @@ function makePage() { }), waitForLoadState: vi.fn().mockResolvedValue(undefined), waitForFunction: vi.fn().mockResolvedValue(undefined), + // settlePage reads content metrics + the final DOM verdict via evaluate; + // a content-bearing verdict keeps the settle gate on its instant path. + evaluate: vi.fn().mockResolvedValue({ + hasContent: true, + hasSpaRoot: false, + hasNavChrome: false, + nearEmpty: false, + }), content: vi.fn().mockResolvedValue('ok'), screenshot: vi.fn().mockResolvedValue(Buffer.from('x')), setExtraHTTPHeaders: vi.fn().mockResolvedValue(undefined), From 0865723c2b6e06df8e9b29b76f2216e4b5b098fc Mon Sep 17 00:00:00 2001 From: boredinnyc Date: Sat, 25 Jul 2026 14:25:39 -0400 Subject: [PATCH 4/4] perf(fetch): copy and remove the temp Chrome profile without blocking cpSync/rmSync over a full Chrome profile can stall the event loop while the service is serving other fetches. Both helpers now use fs/promises and return promises; the two router cleanup sites and the auth path await them, and the unit tests follow. Prefix guard, best-effort cleanup, and logging are unchanged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PrjhynFVYgXjigBmJWXWZs --- src/fetch/auth.ts | 2 +- src/fetch/profile-copy.ts | 12 ++++++------ src/fetch/router.ts | 4 ++-- tests/unit/fetch/profile-copy.test.ts | 24 ++++++++++++------------ 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/fetch/auth.ts b/src/fetch/auth.ts index aaa6519b..4752160e 100644 --- a/src/fetch/auth.ts +++ b/src/fetch/auth.ts @@ -33,7 +33,7 @@ export async function getAuthOptions(): Promise { // Single-use temp copy: consumed by the browser tier's persistent-context // launch and removed by the router (removeTempProfile) once the fetch // settles — success, failure, or abort. - return { userDataDir: copyProfileToTemp(config.chromeProfilePath) }; + return { userDataDir: await copyProfileToTemp(config.chromeProfilePath) }; } if (config.cdpUrl) { diff --git a/src/fetch/profile-copy.ts b/src/fetch/profile-copy.ts index 5202f704..6183e9c1 100644 --- a/src/fetch/profile-copy.ts +++ b/src/fetch/profile-copy.ts @@ -1,4 +1,4 @@ -import { cpSync, mkdtempSync, rmSync } from 'node:fs'; +import { cp, mkdtemp, rm } from 'node:fs/promises'; import { basename, join } from 'node:path'; import { tmpdir } from 'node:os'; import { createLogger } from '../logger.js'; @@ -18,9 +18,9 @@ export const TEMP_PROFILE_PREFIX = 'wigolo-chrome-'; * `removeTempProfile` once the fetch settles (success, failure, or abort) — * a surviving copy is a full-profile privacy leak in tmp. */ -export function copyProfileToTemp(profilePath: string): string { - const tempDir = mkdtempSync(join(tmpdir(), TEMP_PROFILE_PREFIX)); - cpSync(profilePath, tempDir, { recursive: true }); +export async function copyProfileToTemp(profilePath: string): Promise { + const tempDir = await mkdtemp(join(tmpdir(), TEMP_PROFILE_PREFIX)); + await cp(profilePath, tempDir, { recursive: true }); logger.debug('copied Chrome profile to temp directory', { from: profilePath, to: tempDir }); return tempDir; } @@ -31,14 +31,14 @@ export function copyProfileToTemp(profilePath: string): string { * a cleanup failure is logged, never thrown, so it cannot mask the fetch's own * outcome. No-op when no copy was made (`userDataDir` undefined). */ -export function removeTempProfile(userDataDir: string | undefined): void { +export async function removeTempProfile(userDataDir: string | undefined): Promise { if (!userDataDir) return; if (!basename(userDataDir).startsWith(TEMP_PROFILE_PREFIX)) { logger.warn('refusing to remove a directory that is not a wigolo temp profile copy', { userDataDir }); return; } try { - rmSync(userDataDir, { recursive: true, force: true }); + await rm(userDataDir, { recursive: true, force: true }); logger.debug('removed temp Chrome profile copy', { userDataDir }); } catch (err) { logger.warn('failed to remove temp Chrome profile copy', { diff --git a/src/fetch/router.ts b/src/fetch/router.ts index d291975d..e2ec1faa 100644 --- a/src/fetch/router.ts +++ b/src/fetch/router.ts @@ -936,7 +936,7 @@ export class SmartRouter { // The temp Chrome-profile copy (if getAuthOptions made one) is // single-use — remove it once the fetch settles (success, failure, or // abort) so no full-profile copy survives in tmp. - removeTempProfile(authOptions.userDataDir); + await removeTempProfile(authOptions.userDataDir); } } @@ -957,7 +957,7 @@ export class SmartRouter { }); } finally { // Single-use temp Chrome-profile copy — see the actions path above. - removeTempProfile(authOptions.userDataDir); + await removeTempProfile(authOptions.userDataDir); } } diff --git a/tests/unit/fetch/profile-copy.test.ts b/tests/unit/fetch/profile-copy.test.ts index d3c78d6f..4aec30f9 100644 --- a/tests/unit/fetch/profile-copy.test.ts +++ b/tests/unit/fetch/profile-copy.test.ts @@ -22,8 +22,8 @@ describe('profile-copy', () => { } }); - it('copyProfileToTemp copies the profile recursively into a prefixed temp dir', () => { - const copy = copyProfileToTemp(sourceDir); + it('copyProfileToTemp copies the profile recursively into a prefixed temp dir', async () => { + const copy = await copyProfileToTemp(sourceDir); madeCopies.push(copy); expect(copy).not.toBe(sourceDir); @@ -32,30 +32,30 @@ describe('profile-copy', () => { expect(existsSync(join(copy, 'Default', 'Preferences'))).toBe(true); }); - it('removeTempProfile deletes a wigolo temp copy', () => { - const copy = copyProfileToTemp(sourceDir); + it('removeTempProfile deletes a wigolo temp copy', async () => { + const copy = await copyProfileToTemp(sourceDir); madeCopies.push(copy); expect(existsSync(copy)).toBe(true); - removeTempProfile(copy); + await removeTempProfile(copy); expect(existsSync(copy)).toBe(false); }); - it('removeTempProfile is a no-op for undefined', () => { - expect(() => removeTempProfile(undefined)).not.toThrow(); + it('removeTempProfile is a no-op for undefined', async () => { + await expect(removeTempProfile(undefined)).resolves.toBeUndefined(); }); - it('removeTempProfile refuses to delete a directory without the wigolo prefix', () => { + it('removeTempProfile refuses to delete a directory without the wigolo prefix', async () => { // A user-configured directory (e.g. the LIVE profile path) must never be // deleted, even if it is mistakenly passed in. - removeTempProfile(sourceDir); + await removeTempProfile(sourceDir); expect(existsSync(sourceDir)).toBe(true); expect(existsSync(join(sourceDir, 'Cookies'))).toBe(true); }); - it('removeTempProfile tolerates an already-removed directory', () => { - const copy = copyProfileToTemp(sourceDir); + it('removeTempProfile tolerates an already-removed directory', async () => { + const copy = await copyProfileToTemp(sourceDir); rmSync(copy, { recursive: true, force: true }); - expect(() => removeTempProfile(copy)).not.toThrow(); + await expect(removeTempProfile(copy)).resolves.toBeUndefined(); }); });