Skip to content

feat(browser): import cookies into the agent's browser from the dashboard - #23332

Open
bolichen97 wants to merge 1 commit into
mainfrom
supersede/12377-browser-cookie-import
Open

bolichen97 wants to merge 1 commit into
mainfrom
supersede/12377-browser-cookie-import

Conversation

@bolichen97

@bolichen97 bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Problem / Motivation

Goal: one click in the dashboard, no config editing, no restart, values never shown: the agent's browser sessions on a remote gateway start signed in from cookies the owner imports.

On a remote gateway (the common cloud desktop over SSH setup) the agent's browser has no way to become a logged in browser. attach --extension needs the CLI and the user's browser on the same machine, and the dashboard's own address bar is owner only by design. The only path today is a hand written PLAYWRIGHT_MCP_CONFIG pointing at a storageState the user assembled themselves, which is undocumented for most users and silently ignored when the file is wrong. Related asks: #6546, #12319, #11382.

The Codex desktop app solves the same gap with an "Import cookies" button. This PR adds the equivalent to the Browser panel.

Supersedes #12377 (fork PR by @alejacre; reopened from a same-repo branch so the sandbox and security path checks can run).

Why it matters

Any user whose gateway runs somewhere other than their laptop cannot have the agent read a page behind their SSO today without a manual config edit. Every "open this internal page for me" turn stops at a login form, and the failure looks like an expired session rather than a missing capability.

Not a goal

  • Reading the user's local browser profile, or a browser extension: the attach path already covers the local case.
  • Changing any default: nothing is removed or replaced, and a gateway that never imports cookies behaves exactly as today.

What changed (motivation → approach → change)

The Browser panel gets Import cookies and Clear cookies in its overflow menu, plus a read only chip that shows how many cookies are stored, for which sites and when the first one expires. The dialog opens by saying what the import does: the agent's browser is signed in to the sites in the export until those cookies expire or the owner chooses Clear cookies, and whenever a chat asks the agent to browse one of them it can open, read and act on the pages as the owner, at the request of anyone who can chat with that agent; a second line says, in plain words, where the values go: saved only on the computer running Kiro Crew, for the agent's browser, and never shown in the dashboard; the export formats follow as a short How to export list. Once an export is pasted or chosen, the dialog names the sites and the cookie count Import would use, before Import is pressed; it reads only domains and expiries, and the server still parses the export again and decides. A refused import, a file that cannot be read, a status read that keeps failing, an import that saved where no agent browser can be reached (the dialog stays open and says no agent browser will be signed in), and an import or clear that saved but left an open agent browser behind each show an error notice; once a set is saved and the dialog stays open, its footer is a single Done; the last two say what is left and when it ends (that browser gets the cookies when it next starts; a cleared one stays signed in until it closes). A clear's outcome is kept by the panel's one cookie hook, tagged with its chat, so it survives the menu closing and never shows in another chat. The status refetches when the window regains focus, so a second window sees an import or clear made in another. A host that cannot reach the agent's browser at all (no Unix sockets, such as Windows) refuses the import and stores nothing, and its message names the computer running Kiro Crew rather than a host.

Part What it does
Parse and store (browser_cli/cookies.py) Accepts a Playwright storageState, a Cookie-Editor or "Get cookies.txt" JSON export, or Netscape cookies.txt (a cookie with an empty value included); normalises to Playwright's cookie shape, drops expired cookies, caps input at 2 MiB and 5000 cookies, and writes a storage-state.json (0600) inside a 0700 browser-cookie-import directory under the data home.
Keep the stored file out of the sandbox The agent's own browser can still list the cookies it was given (cookie-list), as the spec says; what the mask removes is a durable cookie file on disk inside the sandbox. The directory is masked in every sandbox mode (sandbox_leaves.py), made before every spawn so the mask always mounts (sandbox_precreate.py), and fenced at the tool gate (security/paths_lists.py). No config the agent or its daemon reads names it.
Reach the agent's browser The gateway finds every agent daemon's control socket under the default root and under an operator socket root, resolved the way the agent launch resolves it, treats each named browser of a session as its own target (only real socket files reached through real directories, never a symlink; a socket whose daemon is gone is skipped, not reported), and speaks the daemon's own newline delimited JSON protocol: cookie-set per cookie on import, cookie-clear on clear. No process is spawned, so a value is never on an argv; a cookie the browser refuses is counted and the rest still go in; every failure is a fixed, value free reason, and one 30 s deadline bounds the whole import or clear fan-out. Where the launch would never put agent sockets under those roots (an operator-fixed session name, a CLI without the lifecycle hooks), the import reply says the cookies cannot be delivered instead of promising new sessions; that check is the launch path's own validation (session name, lifecycle hooks, absolute overrides, socket path length), run without creating anything. A gateway thread (SessionWatcher) reaches sessions that start later; it starts in a background task after the port binds, in the dashboard and in the headless server alike (server_runtime/browser_lifecycle.py, server_runtime/api_entrypoint.py) or on the first import, and shares one lock with import and clear.
Routes (messaging_api/browser_routes.py, server_runtime/mcp_routes.py, handlers/messaging.py, dashboard/server.py) GET, POST and DELETE /api/browser/cookies: owner only, refused in a restricted session, SEL audited with counts and at most 20 domains (control characters dropped), import and clear serialised by one lock; POST answers 501 where no browser could receive the cookies.
Frontend WebPreviewCookies.tsx (the dialog on the shared Modal, so the page's keyboard shortcuts never see a key typed in it), lib/cookieExportPreview.ts (the pre-import preview, same three shapes, expired cookies dropped, null for anything it cannot read), useBrowserCookies.ts, the client endpoints in api/client/browserAndComputerUse.ts, the keys in every website/src/i18n catalog, and a dev only capture harness (website/capture/cookie-import.tsx) for the screenshots below.

Delivery has no audience scope: every agent browser the gateway reaches gets the cookies, including browsers that serve non-owner members and restricted sessions. A maintainer accepted that exposure; it is residual (iii) in the spec and a new entry in docs/decisions/ (2026-10-23-imported-browser-cookies-have-no-audience-scope.md).

Docs: docs/system-specs/modules/browser.md, the agent facing browser-control.md in the package docs, docs/decisions/, and the feature map.

Backwards compatibility

Removes nothing: the diff only adds a stored file, three routes and panel controls; desired_config() is unchanged, and browser_socket_env in browser_cli/launch.py returns the same additions and logs the same warnings, its path checks now shared with the import's read-only reach check, and no existing route, key or file changes shape.

Tests

  • test/test_browser_cookies.py: the three parsers and their normalisation (an empty cookies.txt value kept), limits, the 0600 write, the value free summary, socket discovery under the default and an operator root, every named browser of one session reached on import and on clear, the cookie-set request shape, injection over a faked socket (one request per cookie, a refused cookie not stopping the rest, a reply nested past the parser limit read as a malformed reply, fixed value free reasons, a trickled reply ending at the deadline, no process spawned), one round trip over a real socketpair, clear over the same sockets, the watcher, and the three handlers (owner 403, restricted 403, the 501 that stores nothing, both locks held). Injection tests skip where the platform has no AF_UNIX.
  • test/test_dashboard_server_coverage.py: the watcher start runs in a background task the startup hook does not wait on, cleanup stops it, and both the dashboard and the headless entrypoint register it.
  • test/test_handlers_messaging_composition_contract.py, test/test_json_object_body_guard.py, test/test_browser_cli_journeys.py, test/test_sandbox_governance_mask.py, test/test_dashboard_server_composition_contract_reach.py, test/test_sandbox_protected_name_holds.py, test/test_security_gate_liveness.py: the repository guards name the new routes and their order, the body read, the masked directory, its one new protected name per tier and one more line in the security package.
  • website/src/test/useBrowserCookies.test.tsx, website/src/test/WebPreviewPanel.cookies.test.tsx, website/src/test/ApiClient.refactor.surface.test.ts: a status read answered after an import or a clear never overwrites it (the test waits for that read to settle and drains the remaining timers, so no fixed delay decides the verdict), status, import, the opening sentence and How to export list, keystrokes kept from the page shortcuts, a failed file read and a failed import through the error notice, a Netscape export sent unchanged so a last cookie with an empty value keeps its trailing tab, a partial live outcome kept on screen through the error notice for import and for clear, with the hand-off on clear, a window focus refetching the status, the opening sentence naming what the agent can do as the owner, a failing status read, another slot refetching, clear, chip, 403 hides the control, and the three client methods in the pinned key order.
  • test/test_browser_cookies.py also pins: a symlink or plain file is never a session socket, and a symlinked session, s or cli directory is never followed, a stale socket is neither a failure nor still signed in, one overall deadline for import and for clear, the capped audit, and no delivery promise when the session name is operator-fixed or the CLI lacks the lifecycle hooks.
  • website/src/test/cookieExportPreview.test.ts and the preview case in WebPreviewPanel.cookies.test.tsx: each export shape, expired cookies dropped, null for blank, malformed or over-limit input, no value ever returned, and the dialog naming the sites before Import with no request sent.
  • Determinism: the changed backend tests ran 20 times in a row and once in a shuffled order, and the two frontend files ran 10 times with a shuffled order, with no failure. The cookie test files on both surfaces then ran in three more shuffled orders with no failure. The new preview tests ran three times in a shuffled order with no failure. useBrowserCookies.test.tsx ran five more times in a shuffled order with no failure; with the cancel of the in-flight read removed from the hook, both late-read tests fail, and they pass with it.

Manual verification

  • Real namespace sandbox, on a host where userns_available() is true: for each of the standard, cc and strict tiers, a child launched through sandbox.namespace_argv saw an empty browser-cookie-import directory, got ENOENT reading storage-state.json, and its planted file never reached the gateway's copy; a control write one directory over landed.
  • Against a real playwright-cli daemon on Linux: started a session, sent cookies through the socket path, and cookie-list showed them, including a name and value starting with - and a fractional expiry.
  • Each screenshot below was taken by a script that also checked the state on the page (the label, the notice text, the three list rows).

Screenshots / video

Rendered from the capture harness (website/capture/cookie-import.html), which mounts the real CookieDialog, CookieChip, CookieMenuItem and CookieClearMenuItem with the real English strings.

Import dialog: the opening sentence, then How to export:

Import cookies dialog

Import dialog with an export pasted, naming the sites and the cookie count before Import:

Import dialog naming the sites before Import, saved with one open browser left, footer now Done

More actions menu with Import cookies and Clear cookies, then the armed Clear after one click:

More actions menu, Clear idle

More actions menu, Clear armed

Error states: the import refused on a host that cannot reach the browser, a file that cannot be read, a clear that failed, and a status read that failed. The two partial outcomes (saved but one open browser did not take the cookies; cleared but one open browser stays signed in) name what is left and when it ends, shown above and here:

Import failed

File read failed

Clear failed

Cleared, one open browser stays signed in until it closes

Status read failed

Status chip in the toolbar, and the compact chip with the new sessions hint in the Live view header:

Imported cookies status chip

Compact chip and hint

Related Issues

Related asks: #6546, #12319, #11382. No tracking issue yet.

Supersedes #12377.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

@bolichen97
bolichen97 requested review from a team and KiroCrewCaptain as code owners October 23, 2026 04:47
@bolichen97
bolichen97 requested a review from CrysisDeu October 23, 2026 04:47
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Sensitive Change Review

Sensitive files changed:

  • src/kiro_crew/sandbox_leaves.py

  • src/kiro_crew/sandbox_precreate.py

  • src/kiro_crew/security/paths_lists.py

  • @bolichen97 posted a complete reasoning template for 7299b13.

@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Oct 23, 2026
@bolichen97

Copy link
Copy Markdown
Contributor Author

Sensitive change review

  • Head commit: 90bf0f0
  • What rule changed: one new data home directory, browser-cookie-import, joins the agent hidden leaves in sandbox_leaves.py, the pre-created leaves in sandbox_precreate.py, and the crew secret leaves fenced by the tool gate in security/paths_lists.py. No existing rule is removed or loosened.
  • Before -> after (what was allowed/blocked, now): before, the directory did not exist and nothing named it. After, every sandbox tier bind masks it with an empty stand in, the gateway creates it before each spawn so the mask always has a target, and file tools are refused on it. Every other path keeps its current verdict.
  • Worst case if wrong (breaks commands / leaks secret / locks user out): if the mask failed, an agent could read the imported session cookies of the owner, which is a secret leak. If the pre-create failed, a spawn could be refused; the probe below shows spawns succeed on every tier. Nothing here can lock the user out, because the gateway itself is not sandboxed.
  • Evidence checked (which sandbox result proves it): a real namespace sandbox probe on a host where userns_available() is true. For each of standard, cc and strict, a child launched through sandbox.namespace_argv saw an empty browser-cookie-import listing, got ENOENT reading storage-state.json, the secret value never appeared in its output, and its planted file never reached the gateway copy, while a control write one directory over landed and the child ran in its own mount namespace with the sandbox marker set. Run after an import and again before any import, 6 of 6 cases passed.
  • No regression:
    • Tools still work (which tools/commands were run, result): the same real sandbox host ran the real sandbox suites of the namespace sandbox CI job (test_pinned_order_real_sandbox.py, test_snapshot_read_only_real_view.py, test_md_notebook_real_sandbox.py, test_script_hooks.py) together with the cookie and mask suites, 883 passed, 0 failed.
    • Backward compatible (old config / old data / old callers still work, how checked): the change only adds a name, so no existing path changes verdict; test_sandbox_governance_mask.py, test_sandbox_protected_name_holds.py and test_security_gate_liveness.py pass with exactly one new protected name per tier and no other diff in their expected sets.
    • Existing tests (which suites passed on this head): the scoped backend run for this diff passed 18891 tests; its 18 failures are all in test_acp_spawn_offload.py and test_app_agent_launch_copy.py and fail identically on a clean main checkout on the same host, so they come from that host, not this diff. The frontend scoped tests, mypy, flake8, isort, docs lint and the i18n checks pass on this head, and CI runs the full suites.
  • How to undo: revert the single commit of this PR; the three list entries go with it, and an existing browser-cookie-import directory on disk is inert because nothing else reads it.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5) — ✅ PASS

Design-level review of 7299b13c6dd7b370217797dd40cc87988d0e5b0e — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

No design problem blocks this. The cookie file never enters the sandbox, cookies are delivered over the control socket with no argv exposure, the take-away is a new leaf with no prior readers, and rendered evidence is attached.

Notes

  • SessionWatcher polls, so a new session can browse before its cookies arrive ("at most one interval late"). Pushing over the socket when the launch path creates the session would remove that window and the always-on thread. This could be a follow-up; this PR doesn't need it.

[DESIGN-REVIEWED] 7299b13

Description read

[DESCRIPTION-READ] 02cf2644dc91f4fba45dda8fdae40aa3d9c0a7d7dc58a14b8d8bdd0c2e4fc1f5

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5) — ✅ PASS

Premise-level review of 7299b13c6dd7b370217797dd40cc87988d0e5b0e — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

Nothing blocks: every item has a named harm (remote gateways cannot sign the agent's browser in) and nothing like it exists today. One note: import and clear are each serialised by two locks.

Notes

  • Item 6 — oversized: _get_cookie_lock() (asyncio, state messaging._cookie_lock) wraps _persist and _clear. Each of those already holds browser_cli_cookies.transaction() across its whole save+inject or delete+clear. "The lock spans persist AND inject so a concurrent DELETE cannot slip in" is already true without it. Counted: 2 call sites per lock, the same 2 transactions. Subtraction: delete _get_cookie_lock, the _cookie_lock facade global and its composition-contract rows, and keep only the threading transaction().

What this change ships

Inventory (8 items) — 7 justified

Intent: the owner of a remote gateway gets the agent's browser signed in to their sites without editing config. ADDITION.

  1. Browser panel overflow menu gains Import cookies and Clear cookies, plus a chip showing stored count, sites and expiry — justified
  2. Import dialog previews sites and cookie count before Import — justified
  3. Three export formats parsed and stored owner-only under the data home — justified
  4. The stored cookie file is masked from every sandbox and fenced at the tool gate — justified
  5. Cookies are pushed into open agent browsers over their control sockets, and a background watcher reaches browsers that start later — justified
  6. Import and clear are serialised by an asyncio lock on top of the module's thread lock — oversized: the second lock does the same job
  7. New owner-only GET/POST/DELETE /api/browser/cookies, SEL-audited — justified
  8. launch.py path validation moved into _lifecycle_paths so the import's reach check can run it read-only — justified

[FIRST-PRINCIPLES-REVIEWED] 7299b13

Description read

[DESCRIPTION-READ] 9545a1aa676f6f7a60ca8444a1a7480040c9680929276f9d8ce63cde42e993b8

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5) — 🔴 BLOCK

UX-level review of 7299b13c6dd7b370217797dd40cc87988d0e5b0e — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

UX-Verdict: BLOCK

When an import or clear only partly lands, the result looks exactly like a failure. Users can't tell whether their cookies were imported or cleared.

Blockers

"Imported." and "Cleared." render as errors
Trigger: the user imports or clears cookies while an agent browser is open. That is the normal case, because the dialog is reached from the Browser panel.
Outcome: cookies_import_live_failed ("Imported. 1 open agent browser(s) did not take the cookies yet…") and cookies_clear_live_failed ("Cleared. … stay signed in until they close…") both go through ErrorNotice. That gives them the same red box and warning triangle as "Import failed" and "Couldn't clear the cookies". The blind reader said: "it says 'Imported' but looks like an error. I can't tell whether this worked or failed" (shot-02). Of the clear: "no idea whether this counts as success or failure… it makes me nervous" (shot-08). After a clear, they would not know whether the agent is still signed in as them.
Severity: high. It happens often (any open browser), the user can't tell what state they're in, and it happens on every partial delivery.
Fix: show these outcomes as a neutral or success notice, using a check mark and "Imported" or "Cleared". Reserve the warning style for the leftover open browser, and say what happens next to it.

Notes

  • The Import button has its own blocker: the blind reader said they would "probably" not click it after reading the intro. They would be blocked even before that, at "How to export": "I don't recognise most of these words… I would get stuck here". Split cookies_dialog_intro into one bolded consequence line ("Anyone who can chat with this agent can act as you on these sites"), and link the export steps instead of listing tool names.
  • In shot-08 the long cookies_clear_live_failed text stretches the overflow menu across the screen. Give the menu a max width and wrap the notice.
  • Strings like "(1 cookies)" and "browser(s)" read as unfinished. Use the i18n plural forms for cookies_count and the two *_live_failed keys.

[UX-REVIEWED] 7299b13

Description read

[DESCRIPTION-READ] 94098de1f18239b2d00ea525a45ab66cd613fbea3ed322fa2506d208ad62d99c

That sha256 names everything this verdict read: the PR description, plus the 14 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review — ✅ no blocking findings

Reviewed 7299b13c6dd7b370217797dd40cc87988d0e5b0e — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 7299b13

Verdict parsed from the review's SHA-scoped output markers for commit 7299b13c6dd7b370217797dd40cc87988d0e5b0e.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 7299b13c6dd7b370217797dd40cc87988d0e5b0e false-positive: <reason, 20 to 500 characters>

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review — ✅ no blocking findings

GPT 6.1 completed its review of 7299b13c6dd7b370217797dd40cc87988d0e5b0e and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 7299b13

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 7299b13c6dd7b370217797dd40cc87988d0e5b0e false-positive: <reason, 20 to 500 characters>

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Security Scope Review -- ✅ PASS

Which legitimate operations does 7299b13c6dd7b370217797dd40cc87988d0e5b0e newly refuse? Updated in place on each push. A script-confirmed regression blocks; the model proposes candidates and never decides a refusal.

Verdict basis: adjudicated, zero confirmed regressions.

Security scope review -- adjudicated candidates

  • macos-latest (platform posix): 12 classified, 0 newly refused, 11 still allowed.
  • ubuntu-latest (platform posix): 12 classified, 0 newly refused, 11 still allowed.
  • windows-latest (platform windows): 10 classified, 0 newly refused, 9 still allowed.

No candidate flipped from allowed to refused.

Scope-Verdict: PASS

Reason: The change adds one name, browser-cookie-import, to the secret-leaf list, the sandbox mask list and the pre-create list. None of those three are read by the shell deny checks, so the 16 shell candidates can't show a refusal. The path gate for the agent's file tools and the OS sandbox mask are the guards that actually changed, and nothing could classify them; they are named under UNADJUDICATED. This PR adds the directory and the code that reads it (only the gateway, outside the sandbox). The agent's browser config never names the directory, so nothing the agent ran before this change read it.

TIGHTENING: the secret-leaf fence and sandbox mask now cover the new imported-cookie directory -- it refuses an agent's file tool or sandboxed shell reading the live session cookies the owner imported. The hunks: + "browser-cookie-import", # imported cookies (browser_cli/cookies.py) in _CREW_SECRET_LEAVES (security/paths_lists.py), + "browser-cookie-import", in _CREW_HIDDEN_LEAVES (sandbox_leaves.py), and + "browser-cookie-import", in _CREW_PRECREATE_HIDDEN_DIR_LEAVES (sandbox_precreate.py).

ADJUDICATED: 16 candidates, 0 confirmed newly-refused

PLATFORM COVERAGE: The diff only adds a bare leaf name. It adds no new separator, executable name or environment-variable assumption. The leaf is still projected under the existing ~/.kiro/crew, legacy ~/.kirocrew and KIROCREW_HOME anchors.

  • posix: data-home listing, config.json read, the live browser-state and playwright-snapshots neighbours, a project-relative state-save, and the pytest run.
  • windows: the same listing and config.json read written as %USERPROFILE%\.kiro\crew with \ separators, the playwright-cli.cmd shim, and the .venv\Scripts\python.exe test run.
  • any: the playwright-cli session verbs (open, snapshot, tab-list, -s=… screenshot), kirocrew restart, and git grep on the leaf name.

I wrote no candidate for a relocated KIROCREW_HOME data home. The leaf goes through the existing re-anchor code unchanged, and shell text doesn't settle path anchoring anyway.

UNADJUDICATED:

  • Guard 1: the file-tool path gate. hooks.on_tool_call → security.paths.is_sensitive_path / sensitive_write_path_refusal → _path_in_home_dirs(resolved, _SENSITIVE_HOME_DIRS). That list now includes <crew-prefix>/browser-cookie-import for each crew prefix, matched as the exact name or as startswith(target + os.sep).
    • Same guard, publish-artifact fence: _is_keystone_publish_artifact → _KEYSTONE_ARTIFACT_PARENTS. This parent set does not change, because the crew root is already a parent through browser-cookies.txt.
    • Inputs I would have submitted, all of which should stay allowed:
      • a file-tool Read of ~/.kiro/crew/config.json
      • a Read of ~/.kiro/crew/browser-state/Default/Preferences
      • a Read of ~/.kiro/crew/playwright-cli-config.json
      • a Read or Write of a sibling that only shares the prefix, ~/.kiro/crew/browser-cookie-import-notes.md. The + os.sep boundary should keep this one out.
      • a Write of ./login-state.json in the project
      • the same reads under C:\Users\<u>\.kiro\crew\… and under a relocated KIROCREW_HOME
  • Guard 2: the OS sandbox mask and pre-create.
    • Agent sandbox: _CREW_HIDDEN_LEAVES / _CREW_PRECREATE_HIDDEN_DIR_LEAVES → the spawn's bind-mask, which pre-creates an empty directory at every spawn.
    • Adapter sandbox: security.paths.sandbox_credential_targets → its deny list, which is built from _SENSITIVE_HOME_DIRS and so now masks the directory there too.
    • Inputs I would have submitted:
      • a sandboxed ls ~/.kiro/crew, which should still list everything and show the new directory as empty
      • a sandboxed playwright-cli open, which keeps working because the gateway delivers the cookies over the control socket rather than through the file
      • an adapter (Claude Code or kiro-cli) start-up that reads its own credential store
    • No legitimate reader inside a sandbox exists in the tree, so I can argue no narrowing.
  • Guard 3: the dashboard cookie routes. The new api_browser_cookies_get, api_browser_cookies_import and api_browser_cookies_clear sit behind _deny_non_owner_browser_request and _restricted_cookie_denial. These are new routes, not a tightened existing guard, so no existing legitimate request loses access.

[SCOPE-REVIEWED] 7299b13

@bolichen97
bolichen97 enabled auto-merge (squash) October 23, 2026 05:05
@bolichen97

Copy link
Copy Markdown
Contributor Author

Answers to the three subtractions the First Principles lane suggested on the original PR (#12377), checked against this head 90bf0f0:

  1. Clear reaches sessions by spawning the CLI, with a list fallback. Accepted, to be fixed on the next push of this PR. The lane raises it again on this head (item 3): injection only finds sessions through _live_sockets(), so the list fallback can only "clear" sessions that never received cookies. The fix sends cookie-clear over the same socket exchange _inject_into_session uses and deletes _run_on_sessions, _live_session_names, _session_env and the two spawn audit rows.
  2. config_path in the GET response and filename in the POST body have no consumer. Accepted, to be fixed in the same push. No component renders config_path and no server code reads filename, so both go from the route, the TypeScript types and the client signature.
  3. Store under the existing playwright-storage-state.json leaf instead of a new name. Rebutted. The new name is a directory, not a file: the atomic write's temp file is created inside it, so the temp file is masked too, which a single file leaf at the data home root cannot give. Reusing the retired name would also make a restored backup of that retired file start being injected into the agent's sessions, which is the exception the lane itself named. On this head the lane marks the masked directory as justified (item 6).

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running labels Oct 23, 2026
@bolichen97
bolichen97 force-pushed the supersede/12377-browser-cookie-import branch from 90bf0f0 to e308a96 Compare October 23, 2026 06:58
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 23, 2026
@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  • span=cbb056b59a08 Cookie discovery ignores configured socket roots

fixed in e308a96: _lifecycle_roots() scans the default root and the gateway's own PWTEST_SOCKETS_DIR when it is absolute and foreign, resolved with the same _operator_base the agent launch uses, so import, watcher and clear reach sessions under an operator root.
test: test_an_operator_socket_root_is_scanned_as_the_launch_resolves_it fails on the previous head; test_an_inherited_session_root_is_not_an_operator_root pins the inherited case.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  • span=cbb056b59a08 A deeply nested daemon reply aborts the import

fixed in e308a96: the reply parse catches RecursionError with ValueError and returns the fixed reason "malformed reply", so the import never raises out of the live injection.
test: test_a_reply_nested_past_the_parser_limit_is_a_malformed_reply raises RecursionError on the previous head.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  • span=cbb056b59a08 Import reports success on a host without AF_UNIX while nothing delivers the cookies

fixed in e308a96: POST /api/browser/cookies answers 501 unsupported_platform before anything is stored when delivery_supported() is false, and the dialog shows the reason through ErrorNotice; the live helpers no longer say the cookies apply to new sessions there.
test: test_import_on_a_host_without_af_unix_fails_and_stores_nothing and test_no_af_unix_means_no_delivery_and_a_fixed_reason fail on the previous head.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  • span=55c40d1451bd File-read failures bypass the required error surface

fixed in e308a96: FileReader.onerror now sets the dialog's failure slot, which renders through ErrorNotice (role alert) with no hand-off, so the paste stays; the inline hint is left for "paste something first" only.
test: renders a file that cannot be read through ErrorNotice, not as a hint fails on the previous head.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  • span=55c40d1451bd Global keyboard shortcuts unmount the hand-built import dialog and lose the pasted export

fixed in e308a96: the dialog now renders on the shared Modal, whose panel stops every keystroke but Escape before the page's document level shortcuts; backdrop and Escape do not dismiss while the paste holds text, and nothing dismisses while the import is in flight.
test: keeps every keystroke in the dialog away from the page shortcuts fails on the previous head.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  • span=648b4f00762a One rejected cookie stops every cookie after it from loading

fixed in e308a96: a refused cookie-set is counted and the exchange continues; only a transport or protocol failure stops it. The reason reads "the browser refused N of M cookies", still value free.
test: test_one_refused_cookie_does_not_keep_the_rest_out fails on the previous head.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  • span=648b4f00762a A cookies.txt cookie with an empty value is silently dropped

fixed in e308a96: blank and comment lines are judged on the stripped text, but the columns are split from the line with only leading spaces removed, so the trailing tab of an empty value survives.
test: test_netscape_cookie_with_an_empty_value_is_kept fails on the previous head.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  • span=0fb1bfbeb7ff The cookie watcher start was added to the boot path before the socket binds

fixed in e308a96: the watcher start moved into the startup hook's existing background task, ahead of the reclaim, so the hook awaits nothing before the bind.
This was also the shard 1 red on Linux and Windows: test_startup_reclaim_parks_in_the_state_background_tasks blocks its to_thread stub, so the awaited watcher start deadlocked the hook until the 120 s timeout. It passes on this head.
test: test_the_cookie_watcher_start_waits_on_nothing_before_the_bind fails on the previous head.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  • span=e77700beed5b Primary "Import" button: the reader would not dare to press it

fixed in e308a96: the dialog now opens with one plain sentence: "The agent's browser will be signed in to the sites in your export until those cookies expire or you choose Clear cookies in the More actions menu." The formats moved into a three line How to export list, and the values note is its own short line.
Evidence gaps: the description now shows the More actions menu with Import cookies and Clear cookies, the armed "Click again to clear" state, the compact chip with the new sessions hint, and every error state.
Notes taken: the chip reads "first cookie expires in 19h", Choose a file has a file icon, and the opening sentence says where Clear lives.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Oct 24, 2026
@bolichen97
bolichen97 force-pushed the supersede/12377-browser-cookie-import branch 2 times, most recently from 8a88bd4 to f7e6a9f Compare October 24, 2026 05:35
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 24, 2026
@bolichen97

Copy link
Copy Markdown
Contributor Author
  • The "saved but not delivered" error in the cookie import dialog has no askAgent hand-off

fixed in f7e6a9f: the liveNotice ErrorNotice (saved, but undeliverable or refused by an open browser) now passes askAgent with onHandoff={close}, and the wrong No hand-off comment is replaced: the set is already saved and editing the paste clears the notice, so nothing unsaved can be lost.
The failed-import notice (paste not saved) keeps no hand-off.
Test: WebPreviewPanel.cookies offers the hand-off on a saved-but-not-delivered import, never on a failed one.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running labels Oct 24, 2026
@bolichen97
bolichen97 force-pushed the supersede/12377-browser-cookie-import branch from f7e6a9f to 99ba25a Compare October 24, 2026 06:44
@bolichen97

Copy link
Copy Markdown
Contributor Author
  • Import completion discards edits made while the request is pending span=305d946302da

fixed in 99ba25a: while cookies.importing is true the paste textarea, the Choose a file button and the file input are disabled, so the draft cannot change between submit and the reply; the dialog is already not dismissable then.
So a close() on success only clears the export that was sent, and Done after a partial result describes that same export.
Test: WebPreviewPanel.cookies disables draft controls during a pending import and re-enables them after incomplete delivery.

@bolichen97

Copy link
Copy Markdown
Contributor Author
  • Results that worked are shown as errors span=91d2f4ae19ab

needs-a-decision: the GPT and Opus lanes both hold that these notices must render through ErrorNotice, because the values come from browser operations that failed (hot_load.failed, live.failed) and the blocking website rule errors-use-error-notice says "decide by where the value comes from". An earlier head used a neutral StatusNotice and was flagged for it; the Opus lane also asked for the Ask-the-agent hand-off on the import notice, now added.
What changed for the reader: each notice states the outcome first ("Imported." / "Cleared.") and what happens next; the dialog footer becomes a single Done, so nothing invites a second import.
Question to a maintainer: keep ErrorNotice for partial cookie outcomes (the repo rule), or record a decision that excepts them so they render as a status? The code follows the rule until then.

@bolichen97

Copy link
Copy Markdown
Contributor Author
  • The primary Import button is one a first-time user would not click span=5d9e64a38e78

needs-a-decision: the dialog says plainly who can use the imported sign-in ("anyone you let chat with this agent, such as Slack or Discord members you allowed"). That sentence is the exposure a maintainer accepted (docs/decisions/2026-10-23-imported-browser-cookies-have-no-audience-scope.md); a reader who hesitates after reading it is the informed choice the decision asks for, not a defect to word away.
Not done: hiding or softening that line, or a live member count (no single read of every provider's allow-list exists, and a partial count would understate who can ask).
Question to a maintainer: is the plain statement the intended first-run experience, or should the dialog change (for example a separate line or a confirm step) as a product decision?

@bolichen97

Copy link
Copy Markdown
Contributor Author

Sensitive change review

  • Head commit: 99ba25a
  • What rule changed: one new data home directory, browser-cookie-import, joins the agent hidden leaves in sandbox_leaves.py, the pre-created leaves in sandbox_precreate.py, and the crew secret leaves fenced by the tool gate in security/paths_lists.py. No existing rule is removed or loosened. The PR's entries in the three sensitive files are the same as on the previous head f7e6a9f; this head rebases onto current main (fada42f) and sets the package line budget in test/test_security_gate_liveness.py to main's 32,526 plus the same one added line (32,527). Outside the sensitive files, socket discovery now refuses symlinked sockets and symlinked directories on the way to them, so an import or clear cannot be steered into a browser outside the agent's own tree. browser_cli/launch.py shares its lifecycle path checks with the import's read-only reach check; browser_socket_env returns the same additions and logs the same warnings.
  • Before -> after (what was allowed or blocked, now): before, the directory did not exist and nothing named it. After, every sandbox tier bind masks it with an empty stand in, the gateway creates it before each spawn so the mask always has a target, and file tools are refused on it. Every other path keeps its current verdict.
  • Worst case if wrong (breaks commands, leaks a secret, locks the user out): if the mask failed, an agent could read the imported session cookies of the owner, which is a secret leak. If the pre-create failed, a spawn could be refused. Nothing here can lock the user out, because the gateway itself is not sandboxed. Separately, a maintainer accepted that delivered cookies reach every agent browser, including non-owner members' and restricted sessions (feat(browser): import cookies into the agent's browser from the dashboard #23332 (comment)); that is a delivery choice, not a change to these three lists.
  • Evidence checked (which sandbox result proves it): the real namespace sandbox probe recorded for 90bf0f0 (standard, cc and strict, 6 of 6 cases) covers these same three rules, which this head does not change. It was not re run on this head; the namespace sandbox CI job is the live proof for this head.
  • No regression:
    • Tools still work (which tools or commands were run, result): on this head test/test_security_gate_liveness.py, test/test_sandbox_protected_name_holds.py, test/test_sandbox_governance_mask.py and test/test_browser_cookies.py pass (1080 tests with the route-table, launch and body-cap guards); the cookie panel and hook suites and every i18n test pass in the frontend.
    • Backward compatible (old config, old data, old callers still work, how checked): the change only adds a name, so no existing path changes verdict.
    • Existing tests (which suites passed on this head): black, isort, flake8, mypy, tsc, eslint on the changed files, i18n check, docs lint, the decision ledger and feature map checks pass locally. The scoped local runs showed failures in test_acp_spawn_offload.py, test_app_agent_launch_copy.py and a few frontend page suites (ArtifactDetailPage, SchedulePage secrets, ChatInput approval, CrewChatWindow, DiffBlock); each fails the same way on an unmodified main worktree on this host. CI runs the full suites.
  • How to undo: revert the single commit of this PR; the three list entries go with it, and an existing browser-cookie-import directory on disk is inert because nothing else reads it.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: checking Automated validation is still running labels Oct 24, 2026
@bolichen97

Copy link
Copy Markdown
Contributor Author
  • A partial success looks the same as a failure span=5450d9847dad

needs-a-decision: the GPT and Opus lanes both hold that these notices must render through ErrorNotice, because the values come from browser operations that failed (hot_load.failed, live.failed) and the blocking website rule errors-use-error-notice says "decide by where the value comes from". An earlier head used a neutral StatusNotice and was flagged for it; the Opus lane also asked for the Ask-the-agent hand-off on the import notice, now added.
What changed for the reader: each notice states the outcome first ("Imported." / "Cleared.") and what happens next; the dialog footer becomes a single Done, so nothing invites a second import.
Question to a maintainer: keep ErrorNotice for partial cookie outcomes (the repo rule), or record a decision that excepts them so they render as a status? The code follows the rule until then.

@bolichen97

Copy link
Copy Markdown
Contributor Author
  • Import is the main action, and the first-time reader would not press it span=a94dfdaf0fff

needs-a-decision: the opening sentence states the accepted exposure plainly (docs/decisions/2026-10-23-imported-browser-cookies-have-no-audience-scope.md), and each round's reader balks at that sentence or at the export tool list in turn. Making the first-run flow feel safe to press while still telling the owner who can use the sign-in is a product wording call.
Question to a maintainer: keep the plain statement and the three-line How to export list, or redesign the first-run copy (for example a step-by-step export guide or a confirm step)?

…oard

Adds Import cookies and Clear cookies to the Browser panel. The gateway
parses a Playwright storageState, a browser extension JSON export or a
Netscape cookies.txt, stores it in a directory masked from the agent in
every sandbox mode, and hands the cookies to the agent's own browser over
its control socket, so a remote gateway's browser can start signed in.

The dialog says in plain words where the values are saved (only on the
computer running the product), and the two late status read tests wait
on the read completing instead of a fixed delay.

Supersedes #12377.
@bolichen97
bolichen97 force-pushed the supersede/12377-browser-cookie-import branch from 99ba25a to 7299b13 Compare October 24, 2026 09:36
@bolichen97

Copy link
Copy Markdown
Contributor Author
  • Closing the menu discards pending clear failures span=305d946302da
    self-added: no
    mechanism: clear outcome held by the panel's one useBrowserCookies instance, tagged with its sessionKey

fixed in 7299b13. Span retrospective (3rd recurrence on WebPreviewCookies.tsx): one cause behind r7, r9 and r10 is request outcomes kept in component state that mounts and unmounts with a menu or dialog. The clear outcome now lives in the hook the panel owns, so it survives the dropdown unmounting.
The menu item derives its notice from the hook (clearError / lastClear); the Clear row stays mounted and disabled while a notice shows with present false; ErrorNotice and the sibling hand-off are unchanged.
The outcome is tagged with the session key of the DELETE that produced it and shown only in that chat; it is dropped when a new clear starts, on a successful import, or when a refetch finds cookies present again; an import during a pending DELETE no longer resets the mutation, so a second clear cannot start.
Tests: WebPreviewPanel.cookies keeps a rejected pending clear notice after Escape and reopening the menu, keeps a partial pending clear notice after Escape and reopening the menu, does not show a %s clear notice in another slot without remounting, keeps the Clear row disabled after importing while DELETE is still pending; useBrowserCookies keeps a partial clear in lastClear after the caller unmounts, drops a partial clear outcome after an external import refetch. Each failed on the prior head.

@bolichen97

Copy link
Copy Markdown
Contributor Author

Sensitive change review

  • Head commit: 7299b13
  • What rule changed: one new data home directory, browser-cookie-import, joins the agent hidden leaves in sandbox_leaves.py, the pre-created leaves in sandbox_precreate.py, and the crew secret leaves fenced by the tool gate in security/paths_lists.py. No existing rule is removed or loosened. The PR's entries in the three sensitive files are the same as on the previous head 99ba25a; this head rebases onto current main (1241954) and sets the package line budget in test/test_security_gate_liveness.py to main's 32,526 plus the same one added line (32,527). Outside the sensitive files, socket discovery now refuses symlinked sockets and symlinked directories on the way to them, so an import or clear cannot be steered into a browser outside the agent's own tree. browser_cli/launch.py shares its lifecycle path checks with the import's read-only reach check; browser_socket_env returns the same additions and logs the same warnings.
  • Before -> after (what was allowed or blocked, now): before, the directory did not exist and nothing named it. After, every sandbox tier bind masks it with an empty stand in, the gateway creates it before each spawn so the mask always has a target, and file tools are refused on it. Every other path keeps its current verdict.
  • Worst case if wrong (breaks commands, leaks a secret, locks the user out): if the mask failed, an agent could read the imported session cookies of the owner, which is a secret leak. If the pre-create failed, a spawn could be refused. Nothing here can lock the user out, because the gateway itself is not sandboxed. Separately, a maintainer accepted that delivered cookies reach every agent browser, including non-owner members' and restricted sessions (feat(browser): import cookies into the agent's browser from the dashboard #23332 (comment)); that is a delivery choice, not a change to these three lists.
  • Evidence checked (which sandbox result proves it): the real namespace sandbox probe recorded for 90bf0f0 (standard, cc and strict, 6 of 6 cases) covers these same three rules, which this head does not change. It was not re run on this head; the namespace sandbox CI job is the live proof for this head.
  • No regression:
    • Tools still work (which tools or commands were run, result): on this head test/test_security_gate_liveness.py, test/test_sandbox_protected_name_holds.py, test/test_sandbox_governance_mask.py and test/test_browser_cookies.py pass (1080 tests with the route-table, launch and body-cap guards); the cookie panel and hook suites and every i18n test pass in the frontend.
    • Backward compatible (old config, old data, old callers still work, how checked): the change only adds a name, so no existing path changes verdict.
    • Existing tests (which suites passed on this head): black, isort, flake8, mypy, tsc, eslint on the changed files, i18n check, docs lint, the decision ledger and feature map checks pass locally. The scoped local runs showed failures in test_acp_spawn_offload.py, test_app_agent_launch_copy.py and a few frontend page suites (ArtifactDetailPage, SchedulePage secrets, ChatInput approval, CrewChatWindow, DiffBlock); each fails the same way on an unmodified main worktree on this host. CI runs the full suites.
  • How to undo: revert the single commit of this PR; the three list entries go with it, and an existing browser-cookie-import directory on disk is inert because nothing else reads it.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Oct 24, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant