feat(browser): expose cookies command for HttpOnly-aware reads#1839
Open
Benjamin-eecs wants to merge 2 commits into
Open
feat(browser): expose cookies command for HttpOnly-aware reads#1839Benjamin-eecs wants to merge 2 commits into
Benjamin-eecs wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new opencli browser cookies subcommand to fetch scoped cookies from the active browser session, along with Vitest coverage.
Changes:
- Introduce
browser cookiesCLI command with--domain/--urlscoping and structured JSON output. - Add unit tests validating domain scoping, url scoping, empty results, and missing-scope error behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/cli.ts | Adds the browser cookies command implementation and JSON output envelope. |
| src/cli.test.ts | Adds tests for the new command, including scoping and error cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1285
to
+1290
| console.log(JSON.stringify({ | ||
| session: getPageSession(page), | ||
| captured_at: new Date().toISOString(), | ||
| count: cookies.length, | ||
| cookies, | ||
| }, null, 2)); |
Comment on lines
+2197
to
+2198
| describe('browser cookies command', () => { | ||
| const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); |
Comment on lines
+2267
to
+2270
| expect(browserState.page!.getCookies).not.toHaveBeenCalled(); | ||
| expect(process.exitCode).toBeDefined(); | ||
| const out = lastJsonLog(); | ||
| expect(out.error.code).toBe('missing_scope'); |
Wire the existing IPage.getCookies() path to a user-facing CLI verb so agents can read HttpOnly cookies that document.cookie cannot reach. Refs jackwener#1472
Only forward the truthy scope option so getCookies receives the same shape internal callers already use.
Benjamin-eecs
force-pushed
the
fix/browser-cookies-command
branch
from
4cf5777 to
e997ecd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
IPage.getCookies()already exists end-to-end on main: the daemon action'cookies'is registered insrc/browser/daemon-client.ts:24, the extension'shandleCookiesinextension/src/background.ts:1637callschrome.cookies.getAll(details)and returns the cookies including theirhttpOnly: c.httpOnlyflag, and theBrowserCookietype insrc/types.ts:14exposeshttpOnly?: boolean. Internal callers (src/cli.ts:1329forbrowser analyze,src/pipeline/steps/download.ts:47,62for the yt-dlp cookie export path) already rely on this. The capability just has no user-facing CLI verb, so agents driving the browser cannot read HttpOnly cookies thatdocument.cookie/browser evalcannot reach.chrome.cookies.getAllis the documented way to surface HttpOnly cookies from MV3 and is the same path #1472 asked for under "httpOnly cookies viaNetwork.getAllCookies".Add
opencli browser cookies --domain <d> | --url <u>as a thin wrapper overpage.getCookies({ domain, url }). The command mirrors the read-only JSON envelope shape already used bybrowser console(session/captured_at/count/cookies). Scope is required: if neither--domainnor--urlis provided, the command emits amissing_scopeerror envelope and exits withUSAGE_ERROR. This matches the extension's own existing guard atextension/src/background.ts:1638("Cookie scope required: provide domain or url to avoid dumping all cookies") and keeps the verb from being a blanket cookie dump.Related issue: refs #1472. This covers the second of the three use cases in that issue (HttpOnly cookies via the existing extension
chrome.cookiespath). The first (persistent request interception) and third (top-level await inbrowser eval) are out of scope for this PR.Type of Change
Checklist
Screenshots / Output
opencli browser <session> cookies --domain example.comreturns the existing extension envelope unchanged, withhttpOnlypreserved per cookie. Sample shape:{ "session": "test", "captured_at": "2026-06-03T07:06:35.000Z", "count": 2, "cookies": [ { "name": "session", "value": "abc", "domain": "example.com", "path": "/", "secure": true, "httpOnly": true }, { "name": "theme", "value": "dark", "domain": "example.com", "path": "/", "secure": false, "httpOnly": false } ] }opencli browser <session> cookies(no scope) emits amissing_scopeerror envelope and sets a non-zero exit code, matching the existing extension-side guard and preventing accidental whole-profile dumps.Two regression tests under
describe('browser cookies command', ...)insrc/cli.test.tscover both paths against a mockedIPage.getCookies. Fullsrc/cli.test.tscontinues to pass: 164/164. Full unit sweep: 1159 passed / 1 skipped.npx tsc --noEmitclean.npm run buildclean.cli-manifest.jsonuntouched.