feat: Windows cookie import for setup-browser-cookies#64
feat: Windows cookie import for setup-browser-cookies#64MichaelTheMay wants to merge 6 commits intogarrytan:mainfrom
Conversation
Bun's subprocess pipe handling and WebSocket client are broken on Windows, which prevents Playwright from launching Chromium. This commit adds a Node.js-based server path for Windows while keeping the original Bun path for macOS/Linux. Changes: - cli.ts: Windows detection, bun path resolution, 20s startup timeout, cross-platform /tmp handling, Windows-compatible process kill (taskkill) - win-server.ts: Node.js entry point with Bun API polyfills (serve, spawn, sleep, file, write) that imports the standard server.ts - cookie-import-browser.ts: Dynamic import for bun:sqlite to avoid crash on Node.js - meta-commands.ts, snapshot.ts, write-commands.ts: Replace hardcoded /tmp with os.tmpdir(), use path.sep for cross-platform path comparison - server.ts: Cross-platform import.meta.dir fallback - package.json: Remove Windows-incompatible glob in build script, add tsx dep Tested on Windows 11 with Node.js v22 + Bun v1.3.10: - browse goto, text, snapshot, screenshot, responsive, js, console, network, tabs, status all verified working Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Types (BrowserInfoBase, DomainEntry, ImportResult, PlaywrightCookie, RawCookie), CookieImportError, Chromium epoch utils, sameSite mapping, profile validation, and DB copy-when-locked helper — shared by macOS and Windows modules.
Windows module (cookie-import-browser-win.ts): Chrome/Edge/Brave detection, DPAPI master key via PowerShell, v10 AES-256-GCM cookie decryption, Network/Cookies path fallback for Chrome 96+. Platform dispatcher (cookie-import.ts): routes to macOS or Windows module based on process.platform. Refactored macOS module to import shared code from cookie-import-shared.ts. Added better-sqlite3 dependency for Node.js/tsx SQLite access on Windows. 27 new tests for Windows decryption pipeline.
- cookie-picker-routes.ts: import from platform dispatcher, await async calls - write-commands.ts: import from dispatcher, platform-detect open command - win-server.ts: remove bun:sqlite error handler (no longer needed) - cli.ts: fix IS_WINDOWS used before declaration (ReferenceError)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| // v10/v20 prefix → AES-256-GCM (Chrome 80+) | ||
| if (prefix === 'v10' || prefix === 'v20') { | ||
| // v20 uses App-Bound Encryption (Chrome/Edge 127+) — not decryptable by third-party tools | ||
| if (prefix === 'v20' && !keys.v20Key) { |
There was a problem hiding this comment.
[HIGH] Runtime crash risk: v20Key is always null but code assumes it could be non-null
Code checks if keys.v20Key exists before using it, but v20Key is hardcoded to null in getBrowserKeys, creating logical inconsistency and potential crashes.
Justification: Line 256: 'const v20Key: Buffer | null = null;' sets v20Key to null unconditionally. Lines 310-313: checks if prefix === 'v20' and !keys.v20Key, then throws error, but line 313 uses 'keys.v20Key!' with non-null assertion which would crash if reached.
Validation: Line 256 sets v20Key to null. Line 313 uses keys.v20Key! assuming it's non-null when prefix is 'v20', which will throw a runtime error. The check on lines 310-312 will always be true, throwing an error, but line 313 would execute first in ternary evaluation.
Suggestion: Remove the v20Key property from BrowserKeys interface or implement proper v20 key retrieval. Handle v20 prefix cookies differently.
| /** | ||
| * List unique cookie domains + counts from a browser's DB. No decryption. | ||
| */ | ||
| export function listDomains(browserName: string, profile = 'Default'): { domains: DomainEntry[]; browser: string } { |
There was a problem hiding this comment.
[MEDIUM] Platform detection bypass allows Node.js execution on macOS
The listDomains function has a platform guard but importCookies does not, allowing importCookies to be called on macOS Node.js, causing runtime errors.
Justification: The importCookies function lacks the platform check present in listDomains. When Database is null (Node.js on Windows or macOS), line 127 calls openDbWithCopy(dbPath, browser.name, Database) with a null Database parameter, causing a runtime exception.
Validation: Line 98: if (!Database) throw new CookieImportError(...) guards listDomains. Line 127: const db = openDbWithCopy(dbPath, browser.name, Database); in importCookies will execute with Database = null when dynamic import fails, leading to TypeError.
Suggestion: Add the same platform guard (if (!Database) throw ...) at the start of the importCookies function.
| ): T { | ||
| const tmpDir = os.tmpdir(); | ||
| const tmpPath = path.join(tmpDir, `browse-cookies-${browserName.toLowerCase()}-${crypto.randomUUID()}.db`); | ||
| try { |
There was a problem hiding this comment.
[MEDIUM] Synchronous file copy operations block event loop during cookie import
Cookie import uses synchronous file copy operations (fs.copyFileSync) when creating temporary copy of locked SQLite database, blocking event loop.
Justification: Lines 166-172 contain multiple fs.copyFileSync and fs.existsSync calls that execute synchronously, blocking event loop while copying potentially large database files (10-100MB).
Validation: Line 167: fs.copyFileSync(dbPath, tmpPath) — synchronous file copy. Lines 170-171: fs.copyFileSync(walPath, tmpPath + '-wal') and fs.copyFileSync(shmPath, tmpPath + '-shm') — additional synchronous copies.
Suggestion: Use asynchronous file operations (fs.promises.copyFile) with proper error handling.
2 participants
Summary
findInstalledBrowsers,listDomains,importCookiesto macOS or Windows module based onprocess.platform.cookie-import-shared.ts.better-sqlite3dependency — prebuilt native SQLite for Node.js/tsx on Windows.Network/Cookiesfallback — both macOS and Windows now check the new path first.cmd /c start(Windows),open(macOS),xdg-open(Linux).cli.tsIS_WINDOWS used before declaration (ReferenceError on Windows).win-server.tssimplified — removed bun:sqlite error handler.Pre-Landing Review
No issues found. (No SQL injection, no LLM outputs, no user-facing HTML. Parameterized queries, sanitized PowerShell inputs, path traversal prevention.)
Eval Results
No prompt-related files changed — evals skipped.
TODOS
Known Limitations
app_bound_encrypted_keyis DPAPI-encrypted with LocalMachine scope + browser-specific entropy, inaccessible to third-party tools. v10 cookies decrypt correctly.Test plan
cookie-import-browserdetects Chrome + Edge, picker UI opens, Edge DB reads + DPAPI decrypts🤖 Generated with Claude Code