Skip to content

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

Closed
alejacre wants to merge 1 commit into
kirodotdev:mainfrom
alejacre:feat/browser-cookie-import
Closed

alejacre wants to merge 1 commit into
kirodotdev:mainfrom
alejacre:feat/browser-cookie-import

Conversation

@alejacre

@alejacre alejacre commented Sep 20, 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.

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 owner pastes or uploads a cookie export from the browser they are logged into, and the agent's browser sessions start signed in. A session that refuses the set, or a status read that keeps failing, is said in an error notice rather than hidden; every open slot shows the same stored set.

Part What it does
Parse and store (browser_cli/cookies.py) Accepts a Playwright storageState, a Cookie-Editor / "Get cookies.txt" JSON export, or Netscape cookies.txt; normalises to Playwright's cookie shape, drops expired cookies, caps input at 2 MiB / 5000 cookies, and writes browser-cookie-import/storage-state.json (0600, in a 0700 directory) under the data home.
Keep it from the agent The browser-cookie-import 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). A sandbox started before the first import, and the write's temp file, both stay behind the mask. No config the agent or its daemon reads names it.
Reach the agent's browser The gateway connects to each agent daemon's control socket and sends one cookie-set request per cookie in the daemon's own newline-delimited JSON protocol. No process is spawned, so a cookie value is never on any argv, and a failure reports a fixed reason, never reply text. Every exchange is bounded by one absolute deadline, re-applied before each read. Live sessions get them at import; a gateway thread (SessionWatcher) reaches sessions that start later. It starts only once a set is stored (server_runtime/browser_lifecycle.py and the import route), never where the platform has no AF_UNIX, and it shares one lock with import and clear, so a clear cannot be undone by a tick in flight.
Routes (messaging_api/browser_routes.py, server_runtime/mcp_routes.py) GET/POST/DELETE /api/browser/cookies: owner-only, refused in a restricted session, SEL-audited with counts and domains only, import and clear serialised by one lock.
Frontend WebPreviewCookies.tsx, useBrowserCookies.ts, the client endpoints in api/client/browserAndComputerUse.ts, 20 keys in every website/src/i18n catalog, and a dev-only capture harness (website/capture/cookie-import.tsx) for the screenshots below.

Docs: docs/system-specs/modules/browser.md, the agent-facing src/kiro_crew/docs/browser-control.md, and the feature map.

Backwards compatibility

Compatible: every change is additive; desired_config() is unchanged, and no existing route, key or file changes shape.

Removes nothing: the diff only adds a stored file, three routes and panel controls.

Tests

  • test/test_browser_cookies.py: the three parsers and their normalisation, limits, the 0600 write, the value-free summary, the cookie-set request shape, injection over a faked socket (one request per cookie, stop at first failure, fixed value-free reasons for refused / malformed / timed-out / unreachable daemons, a trickled reply ending at the deadline, and that no process is spawned), one round trip over a real socketpair, the watcher (no thread before a set exists or without AF_UNIX, ticks under the import/clear lock), the pre-created masked directory, and the three handlers (owner 403, restricted 403, both locks held). Injection tests skip where the platform has no AF_UNIX.
  • test/test_handlers_messaging_composition_contract.py, test/test_json_object_body_guard.py, test/spawn_audit_helpers.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 two value-free spawns (list, cookie-clear), 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: status, import, 400 inline error, a partial live failure kept on screen, a failing status read settling into a notice after bounded retries, another slot refetching after import and clear, clear, chip, 403 hides the control, the two-buttons-per-row guard, and the three client methods in the pinned key order.

Manual verification

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; a cookie with an invalid sameSite came back as "the browser refused the cookie". Not verified: Windows, where the injection reports "unsupported platform" because the daemon socket is a named pipe there.

Screenshots / video

Import dialog (file picker or paste):

Import cookies dialog

Status chip in the Live view header after an import (domains in the tooltip, two-step Clear):

Imported cookies status chip

Related Issues

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

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title
  • 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

@alejacre
alejacre requested review from a team and patrigao September 20, 2026 22:02
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 20, 2026
@github-actions

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5, fork) — ✅ PASS (a BLOCK with no Trigger and no Outcome does not block)

Design-level review of b0de98122110f07fda83f5a77fda6ce8a135b726 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

cannot evaluate: missing screenshot of the Import cookies dialog and the cookie status chip. The PR adds a new panel surface under website/src/, but the evidence list has zero images or recordings for this revision.

Blockers

No rendered evidence for the new Browser panel controls
Trigger: the diff adds website/src/components/WebPreviewCookies.tsx and changes WebPreviewPanel.tsx ("Import cookies and Clear cookies in its overflow menu, plus a read-only chip"). Both screenshot slots in the description read "[image removed]", and design-evidence.txt lists 0 downloaded attachments and 0 committed images.
Outcome: no image of the dialog, the chip or the two-step Clear → their shape and placement can't be judged → this new owner-facing surface would merge with no design review of how it looks.
Fix: attach the two captures from website/capture/cookie-import.tsx, or commit them under temp-screenshots/ at the head SHA.
Clears when: design-evidence.txt lists a downloaded or committed image of the Import cookies dialog and of the status chip for this revision.

Notes

  • The claim "The agent can never read the values" goes further than the design delivers. SessionWatcher sends cookie-set with every value to any pw/<8hex>/s/cli/*.sock it finds, and pw/ is agent-writable. An agent that binds a listener at that path receives the cookies in cleartext. Residual (ii) half-concedes this, so either reword the invariant to match or check socket ownership before injecting.
  • clear_live_sessions runs a full cookie-clear, which also wipes cookies the agent got by logging in itself. The docs should say so.

[DESIGN-REVIEWED] b0de981

Description read

[DESCRIPTION-READ] 3f8f75706626ba89431f929bf9a5b1a49dd28c862577ceca04a4e34f2344b9aa

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 Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review (fork) — 🔴 changes requested (blocking)

Reviewed b0de98122110f07fda83f5a77fda6ce8a135b726 via the fork AI-review pipeline; updated in place on each push.

Importing cookies writes the full cookie file to a temporary file in the data-home root first, and that temp name is not masked from agent sandboxes. The PR says the stored cookies are bind-masked out of every sandbox; this temp file is not.

BLOCKING — src/kiro_crew/browser_cli/cookies.py:397
atomic_write(path, payload, mode=0o600, restrict_to_owner=True)
Trigger: The owner sends POST /api/browser/cookies while an agent sandbox is running. The call path is _persist → save_storage_state → atomic_write, which runs tempfile.mkstemp(dir=str(path.parent)) (atomic_write.py:1272). That puts the temp file in the data-home root, which every sandbox can see. The mask on browser-storage-state.json covers only that exact name.
Outcome: While the write runs, the whole storageState (every cookie value) sits at a tmp*.tmp name that no mask or tool fence covers. A same-uid agent can read it or link(2) it, and a link keeps the bytes after the rename. If the gateway crashes mid-write, the unmasked copy stays on disk. This breaks the PR's claim that the agent can never read the values. hidden_leaf_staging.py exists to close exactly this gap, and the sibling browser_cli/token.py already uses it.
Anchor: residual/security
Fix: Publish with hidden_leaf_staging.publish_through_auth_store_staging(path, payload) instead of a plain atomic_write.

[BLOCK-MERGE] b0de981
[OPUS-REVIEWED] b0de981

@github-actions

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5, fork) — ✅ PASS

Premise-level review of b0de98122110f07fda83f5a77fda6ce8a135b726 via the fork AI-review pipeline — 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 and there is no product-shape change. Three subtractions are left: a second transport used only by Clear, two API fields nothing reads, and a new name for a cookie leaf that is already masked.

Notes

  • Item 8 — oversized: import talks to the daemon over its socket (_inject_into_session), but Clear spawns playwright-cli -s=<name> cookie-clear and falls back to list. _run_on_sessions, _live_sessions, _live_session_names and _session_env each have exactly 1 caller (clear_live_sessions), and they need 2 new BENIGN_PACKAGE_SPAWNS rows. Subtraction: send cookie-clear as one run request over the same socket exchange. That deletes the four functions and both spawn-audit rows.
  • Item 9 — zero consumers: "config_path": str(path) in the GET response is copied by useBrowserCookies.ts and shown by no component (0 render sites). filename in the POST body is read by no server code: the handler reads only body.get("content"). Subtraction: delete both fields from the route, the TS types and the client signature.
  • Item 4 — duplicate of playwright-storage-state.json (sandbox_leaves.py:245, paths_lists.py:268, sandbox_aliases.py:151): base already masks a leaf described as "the Playwright storage-state they become" in 3 lists. The new browser-storage-state.json is in only 2 of them. Subtraction: store under the existing leaf and drop the two new list entries. If a restored backup of that retired file must not be injected, keep the new name.

What this change ships

Inventory (10 items) — 7 justified

Intent: let the owner of a remote gateway sign the agent's browser in by importing cookies from their own logged-in browser — ADDITION.

  1. Browser panel menu gains Import cookies (paste or upload) — justified
  2. Clear cookies removes the stored set and clears open sessions — justified
  3. Chip shows cookie count, domains and earliest expiry, never values — justified
  4. Imported cookies stored owner-only under a new masked leaf name — duplicate of playwright-storage-state.json leaf
  5. Open agent browser sessions receive the cookies at import — justified
  6. Background gateway thread gives cookies to sessions started later (2 s poll) — justified
  7. Three owner-only cookie routes, refused in restricted sessions, SEL-audited — justified
  8. Clear reaches sessions by spawning the CLI, with a list fallback — oversized
  9. config_path response field and filename request field — zero consumers
  10. Agent-facing doc, spec section and feature-map row — justified

[FIRST-PRINCIPLES-REVIEWED] b0de981

Description read

[DESCRIPTION-READ] 23f9f12e4adf575ae9989d18ff9a40bbd29ad608d03ceeac9d3854e62b0ff553

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 Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review (fork) — 🔴 changes requested (blocking)

Reviewed b0de98122110f07fda83f5a77fda6ce8a135b726 via the fork AI-review pipeline; updated in place on each push.

3 of 4 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- src/kiro_crew/browser_cli/cookies.py:895 -- Serialize watcher injection with import and clear

reason = _inject_into_session(key[0], cookies)
Trigger: SessionWatcher._tick() loads cookies, then dashboard DELETE deletes the stored set and clears the live browser before the watcher finishes injecting its loaded copy.
Outcome: The watcher restores cookies after Clear reports success; the browser remains authenticated while the dashboard reports no imported cookies. The HTTP transaction lock does not cover the watcher.
Anchor: defect
Fix: Use one shared transaction lock around watcher read-and-inject and both HTTP transactions, acquiring it off the event loop.

BLOCKING -- src/kiro_crew/browser_cli/cookies.py:658 -- Enforce the absolute deadline during reply reads

chunk = sock.recv(65536)
Trigger: An agent-controlled socket under a generated session root sends reply bytes every ten seconds without a newline. POST reaches it through _live_sockets() and _inject_into_session(), which sets the timeout only before entering _read_reply().
Outcome: Every receive completes within its individual timeout, so the exchange continues beyond the fifteen-second deadline, holding the import transaction lock and preventing Clear until the reply-size ceiling is reached.
Anchor: defect
Fix: Pass the absolute deadline into _read_reply() and recompute the remaining timeout before every receive.

BLOCKING -- website/src/hooks/useBrowserCookies.ts:98 -- Refresh all session-scoped cookie-status caches

queryClient.setQueryData(queryKey, (prev) => ({
Trigger: The owner loads cookie status in slots A and B, imports cookies in A, then returns to B. Both slots use the shared query client, but the mutation updates only A’s key.
Outcome: With infinite stale time and no prefix invalidation, B retains its empty status and offers no Clear action despite active imported credentials. Clearing in A likewise leaves B displaying a removed set.
Anchor: defect
Fix: Invalidate the BROWSER_COOKIES_KEY prefix after both mutations, keeping each refetch scoped to its session’s authorization.

BLOCKING -- website/src/components/WebPreviewCookies.tsx:310 -- Surface cookie-status and live-injection failures

const reachedNone = result.hot_load.loaded.length === 0
Trigger: POST returns one successfully loaded session and another in hot_load.failed; submit() ignores the failures and closes the dialog. Separately, persistent cookie-status HTTP failures retry indefinitely, and the panel never renders cookies.error.
Outcome: Failed browser injection appears successful, and failed status reads provide no visible explanation or recovery action.
Anchor: errors-use-error-notice
Fix: Render both failures through ErrorNotice with the appropriate hand-off decision, and bound status retries so failures settle.

[BLOCK-MERGE] b0de981
[GPT-REVIEWED] b0de981

Adjudication (Opus 5.5) — is blocking on each finding proportionate?

F2 — MEDIUM, agent-triggerable. A peer on a pw/<8hex>/s/cli/*.sock socket (found at cookies.py:543–555, injected at cookies.py:753) can trickle bytes with no newline. The timeout is set only once per cookie (cookies.py:708). Each recv at cookies.py:658 then gets its own fresh timeout, and the only stop is the 1 MiB ceiling (cookies.py:656), which a slow enough trickle takes days to reach. Meanwhile import holds _get_cookie_lock() across to_thread(_persist) (browser_routes.py, patch line 1314), so Clear hangs until the gateway restarts. There is no recovery path short of that restart.
Real fix: pass deadline into _read_reply and re-run settimeout(deadline - monotonic()) before each recv. That is a few lines, adds no new state and no coordination, so the harm clearly justifies it.

[ADJUDICATION] b0de98122110f07fda83f5a77fda6ce8a135b726 total=1 uphold=1 downgrade=0
UPHOLD F2 src/kiro_crew/browser_cli/cookies.py:658 reason=harm-warrants-remedy
[GPT-ADJUDICATED] b0de98122110f07fda83f5a77fda6ce8a135b726

[ADJUDICATION-FENCED] b0de98122110f07fda83f5a77fda6ce8a135b726 fenced=3 flagged=0
UPHOLD-FENCED F1 src/kiro_crew/browser_cli/cookies.py:895 -- The watcher's `_tick` loads cookies and injects them (cookies.py:890–895) without the HTTP lock, and `clear_live_sessions` only resets the watcher's record (cookies.py:779), so a Clear that overlaps an injecting tick silently leaves the browser signed in; this is ordinary timing, not an extreme condition.
UPHOLD-FENCED F3 website/src/hooks/useBrowserCookies.ts:98 -- Two slots open on one query client is an ordinary dashboard state, and a stale empty status hides Clear while credentials are active, so the evidence for a FLAG is incomplete.
UPHOLD-FENCED F4 website/src/components/WebPreviewCookies.tsx:310 -- A partial hot-load failure needs only one unreachable session, which is common, and it then reads as success, so the condition is not extreme enough to FLAG.
[GPT-ADJUDICATED-FENCED] b0de98122110f07fda83f5a77fda6ce8a135b726

@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 Sep 20, 2026
@alejacre

Copy link
Copy Markdown
Contributor Author

Addressed the GPT 5.6 findings and the CI failures on 0f90b54 in d4b6cce4. Per finding:

cookies.py:54 — imported credentials exposed to the agent (BLOCKING). Fixed. browser-storage-state.json is hidden from the agent in every sandbox mode (sandbox._CREW_HIDDEN_LEAVES, next to the retired playwright-storage-state.json, plus the tool gate) and the agent's launch config is back to engine-only, so it never names the file. Because a daemon the agent starts runs inside its sandbox and could not open the masked file, the gateway now pre-warms each agent session's daemon itself (cookies.prewarm_session, called at both spawn sites right after browser_session_env) with a gateway-only config carrying contextOptions.storageState and the agent's socket/registry dirs; the agent's later commands connect to that daemon over the socket and never see the file. Pinned by test_storage_state_leaf_is_hidden_and_tool_gated, test_agent_config_never_names_the_state_even_when_present, and the prewarm argv/env tests with a fake CLI. Residual, documented: an agent whose first browser command lands before the pre-warmed daemon is up gets an in-sandbox, cookie-less daemon (the panel already tells the user cookies apply to new sessions).

client.ts — restricted sessions bypass cookie isolation (BLOCKING). Fixed. getBrowserCookies/importBrowserCookies/clearBrowserCookies send the active slot's X-Session-Key (same contract as openInBrowser), the hook scopes its query per slot, and GET /api/browser/cookies now applies _is_restricted_session after the owner gate, returning the same {"error", "code": "restricted_session"} 403 the mutations do. Tests pin the header on all three calls and the 403 on GET.

messaging.py:4010 — clearing leaves live sessions authenticated (BLOCKING). Fixed. DELETE runs cookie-clear on every live kc-* session after removing the file and rewriting the gateway config, and returns live: {cleared, failed} so a partial clear is visible. Covered with a fake CLI (TestClearLiveSessions) and in the handler test.

feature map (BLOCKING). The Browser-panel row now lists Import cookies / read-only chip / Clear, the More-actions navigation, components/WebPreviewCookies.tsx, hooks/useBrowserCookies.ts, browser_cli/cookies.py, and GET,POST,DELETE /api/browser/cookies. test_check_feature_map.py and scripts/check_feature_map.py pass against origin/main.

errors-use-error-notice (BLOCKING). A failed import (400/5xx) renders through ErrorNotice with the server's message; the inline muted hint remains only for client-side validation (empty paste, unreadable file) and has no role="alert". Hand-off decision recorded in the required comment: no hand-off, because the unsaved paste in the textarea is the draft and a hand-off would unmount the dialog. The previously unsurfaced Clear failure renders inside the menu as a passive ErrorNotice with a sibling ErrorNoticeMenuItem (describedBy = notice id).

max-two-buttons-per-row (BLOCKING). The header cookie button is gone and the chip is read-only (status + domain tooltip). Import and Clear are items in the existing "More actions" overflow; Clear is two-step (arm → "Click again to clear", menu stays open via preventDefault as in ExportSessionItem). The 5-control regression guard is unchanged and green; a new test counts exactly two action controls in the running header.

top-level-imports (finding). import subprocess hoisted in cookies.py; the deferred import in launch.py was a real cycle and is gone, since desired_config() no longer reads the state path.

CI on 0f90b54. Frontend: useBrowserCookies imported ApiError from api/client, which ChatPageMoreCoverage mocks — now imported from api/apiError; hi/bn/de/ja strings rewritten to the locale style guides. Backend: the cookie-import body cap is recorded in test_json_object_body_guard.py's register (browser_cli_cookies.MAX_IMPORT_BYTES, explicit). The two Windows failures (test_overload_integration_glue, test_runtime_cleanup_windows) do not touch this change; flagging them in case they recur.

Not verified live: the pre-warm path with the real CLI on a running gateway (launching a browser from the sandbox that wrote this was refused); it rests on the daemon reading contextOptions.storageState at context creation, which the earlier manual verification of the storageState path exercised.

@alejacre
alejacre force-pushed the feat/browser-cookie-import branch from d4b6cce to ff18b5b Compare September 20, 2026 23:36
@alejacre

Copy link
Copy Markdown
Contributor Author

Follow-up in ff18b5b9 (branch squashed to two commits for PR Hygiene; content of d4b6cce4 unchanged plus the items below):

cookies.py — deeply nested JSON crashes the handler (BLOCKING). json.loads now runs with a RecursionError guard: a pathological document raises CookieImportError ("nested too deeply") and the route answers 400. test_deeply_nested_json_is_a_user_error (100k levels).

cookies.py — non-finite expiry corrupts the persisted state (BLOCKING). _normalize_expires treats any non-finite value as a session cookie (-1), and the parser refuses the non-standard JSON constants NaN/Infinity/-Infinity via parse_constant, so nothing non-finite can reach the file. Tests: test_non_finite_expiry_string_is_session_cookie (inf/-inf/nan/Infinity/NaN), test_json_constants_are_rejected.

check-bundle-size.mjs — undeclared build-gate change (finding). Declared, not dropped: the all chunk holds the 13 eager i18n catalogs plus the entry, so no lazy import() boundary can move a translated string out of it (the file's own history says the same for every prior increment). This branch adds 20,992 B of catalog bytes; main alone sat 16 KB (0.13%) under the ceiling, so any feature shipping strings would have tripped it. Re-measured to the file's documented 5% convention with the attribution recorded beside the entry. The feature's components live in their own chunks (a lazy split of the dialog was tried and reverted because it did not touch all).

@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 Sep 20, 2026
@github-actions

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Security Scope Review (fork) — 🔴 BLOCK (blocking)

Which legitimate operations does b0de98122110f07fda83f5a77fda6ce8a135b726 newly refuse? Model proposes, scripts/deny_diff.py decides — updated in place on each push. A script-confirmed regression blocks PR readiness.

the per-platform reports could not be folded

Scope-Verdict: PASS

Reason: The only stricter rule is one new exact-name leaf, browser-storage-state.json. It was added to the file-tool fence (_CREW_SECRET_LEAVES) and to the OS sandbox mask (_CREW_HIDDEN_LEAVES). No file in the base tree has that name, so no existing reader or writer loses access. Neither guard is a shell-command check, so the classifier could not give either one a verdict. Both are named under UNADJUDICATED.

TIGHTENING: Exact-name secret leaf added to the file-tool read+write fence and to the OS sandbox mask. The ONE threat it refuses is an agent reading the owner's imported session cookies off disk, through its file tools or through open() in a shell it spawned. From security/paths_lists.py: + "browser-storage-state.json", under the comment # The imported-cookies storageState the dashboard owner uploads ... Live session cookies; read only by the gateway. From sandbox_leaves.py: + "browser-storage-state.json", inside _CREW_HIDDEN_LEAVES.

ADJUDICATED: 17 candidates, 0 confirmed newly-refused

PLATFORM COVERAGE: Candidates cover posix, windows and any. The posix rows use ~/.kiro/crew/ paths: a listing of the data home, a read of the neighbouring config.json, a look inside the pw/ lifecycle roots, a project-local state-save, a Playwright --storage-state fixture, and the maintainers' pytest run. Together they probe the leaf's parent directory and the names one token away from the leaf. The windows rows (%USERPROFILE%\.kiro\crew with dir and type, and a pytest path with \ separators) probe the same parent in the other host's spelling, because the fence is built from _CREW_HOME_PREFIXES projected under each home. The any rows are the agent's own playwright-cli -s=kc-… verbs (goto, cookie-list, cookie-clear, list), a panel- snapshot, and a grep for the leaf name as plain data. These are the daily operations in the touched area, and none of them carries a path. The diff's new socket injection assumes AF_UNIX, and on Windows it returns "unsupported platform" instead of refusing anything, so no shell spelling exists to probe there.

UNADJUDICATED:

  • File-tool fence (is_sensitive_path / is_sensitive_write_path). Call path: agent file read/edit tool → hooks.on_tool_call → security.paths.is_sensitive_path → _path_in_home_dirs(resolved, _SENSITIVE_HOME_DIRS), where _SENSITIVE_HOME_DIRS now includes <crew-prefix>/browser-storage-state.json. The new leaf's parent is the crew home root, which already has other root-level leaves in _KEYSTONE_ARTIFACT_PARENTS, so the derived *.tmp/*.lock fence covers no new directory. Inputs I would have submitted:

    • a file-tool read of ~/.kiro/crew/config.json and ~/.kiro/crew/sessions.db
    • a read of ~/.kiro/crew/playwright-cli-config.json
    • a read of a project-local playwright/.auth/storage-state.json
    • a read of %USERPROFILE%\.kiro\crew\config.json on Windows

    Each should stay allowed, because the leaf matches only its exact name.

  • OS sandbox mask (sandbox._CREW_HIDDEN_LEAVES → sandbox_credential_targets / bind-mask at agent spawn). Call path: agent spawn → sandbox preflight → bind-mask of <crew-home>/browser-storage-state.json. Inputs I would have submitted: an open() from a sandboxed shell of the visible browser paths (pw/, browser-state, playwright-snapshots, playwright-cli-config.json). These must stay readable for the agent's daemon to start, and the diff's own comment says they are kept off the list.

  • New owner-only routes (GET/POST/DELETE /api/browser/cookies): owner guard plus _is_restricted_session. These are new surfaces, not a stricter version of an existing route, so no existing caller can lose access.

[SCOPE-REVIEWED] b0de981

@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Sep 21, 2026
@alejacre
alejacre force-pushed the feat/browser-cookie-import branch from ff18b5b to ee62663 Compare September 21, 2026 08:31
@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 Sep 21, 2026
@alejacre

Copy link
Copy Markdown
Contributor Author

Round 3 in ee626633 (still two commits). The pre-warm design is gone; the finding was right.

cookies.py — gateway pre-warmed daemon bypasses the OS sandbox (BLOCKING). Accepted and removed entirely: prewarm_session, the gateway-side launch config and both spawn-site calls (acp/runtime.py, acp/client.py are back to main). The agent's daemon now always runs inside the agent's sandbox, started by the agent's own command as before this PR. Cookies reach it only over its control socket, as data: the gateway runs playwright-cli -s=<kc> cookie-set <name> <value> --domain … [--expires N] [--httpOnly] [--secure] [--sameSite …] per cookie (options before --, so a value starting with - is safe), so the daemon does addCookies and never opens a file. On import this runs against every live kc-* session; for sessions created afterwards a gateway-side SessionWatcher (daemon thread, 2 s tick, keyed on socket path+inode+ctime, reset on re-import and on clear) injects on first sight. Verified live against a real daemon started the agent's way: inject → cookie-list shows the cookies → cookie-clear empties them. Residuals, documented in browser.md: a brand-new session's first navigation may precede the watcher's next tick, and cookie values transit the gateway-spawned CLI client's argv for milliseconds (the daemon protocol has no stdin path for cookies; a file the daemon opens is exactly what the hidden leaf forbids). Not verified across a real bwrap mount namespace; the socket lives under the data home, which the existing cookie-clear path already relies on being visible from both sides.

acp/client.py — filesystem I/O on the event loop (BLOCKING). Moot: nothing cookie-related runs at agent spawn any more; both spawn sites are unchanged from main.

messaging.py — concurrent import and clear (BLOCKING). One module-level asyncio.Lock is held across the whole persist+inject transaction and the whole delete+cookie-clear+watcher-reset transaction. test_import_and_clear_are_serialised_by_one_lock fires a DELETE while a POST is inside its worker and asserts the ordering.

cookies.py — "secure": "false" coerced to true (finding). _normalize_flag accepts a real boolean, absent/null, or the strings true/false/1/0 case-insensitively; anything else is a CookieImportError. Parametrised tests.

Frontend Tests (2) — bn [changed-values]. All new bn strings rewritten in the তুমি register (0 violations). DELETE's live.failed is now surfaced in the Clear row ("Cleared on disk; N live session(s) still hold cookies") through the existing menu ErrorNotice path.

@dwu96 dwu96 added the needs-pr-triage PR scanner: awaiting automated triage label Sep 24, 2026
…oard

Add "Import cookies" to the Browser panel. The dashboard owner exports cookies
from the browser where they are logged in (a Playwright storageState, a
Cookie-Editor / "Get cookies.txt" JSON export, or Netscape cookies.txt) and
imports them. The gateway normalises them into an owner-only storageState
under the data home, a leaf masked from every agent sandbox and fenced at the
tool gate, and hands them to the agent's own browser daemons over each
daemon's control socket: one cookie-set request per cookie in the daemon's
newline-delimited JSON protocol, sent from the gateway process, so no cookie
value is ever on an argv. Live sessions get them at import; a gateway-side
watcher reaches sessions that start later. Clear removes the file and runs
cookie-clear on live sessions.

GET/POST/DELETE /api/browser/cookies are owner-only, refused in a restricted
session, SEL-audited with counts and domains only, and capped at 2 MiB and
5000 cookies. The summary the panel shows never carries a cookie value.
@bolichen97
bolichen97 force-pushed the feat/browser-cookie-import branch from b0de981 to c52591f Compare October 23, 2026 02:43
@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor
  • span=648b4f00762a Importing cookies writes the full cookie file to a temporary file in the data-home root first

fixed in c52591f: the state now lives at browser-cookie-import/storage-state.json, and the whole browser-cookie-import directory is the masked name.
atomic_write stages its temp in path.parent, which is now that masked directory, so the temp never sits in the visible data-home root.
The directory is in _CREW_PRECREATE_HIDDEN_DIR_LEAVES, so its mask mounts in every sandbox, including one spawned before the first import.
test_storage_state_dir_is_precreated_so_its_mask_always_mounts and test_every_masked_directory_is_precreated_or_made_at_boot pin it.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor
  • span=e795fd3062fc watcher lock and reply deadline

fixed in c52591f (Serialize watcher injection with import and clear): one module lock, cookies.transaction(), is held by every watcher tick across load and inject, by the import across save and inject, and by the clear across delete and cookie-clear; the handlers take it inside their worker thread, never on the event loop.
fixed in c52591f (Enforce the absolute deadline during reply reads): _read_reply now takes the exchange deadline and re-applies the remaining time before every recv, raising TimeoutError once it passes, so a trickling peer ends at the 15 s bound.
Tests: test_a_tick_reads_and_injects_under_the_transaction_lock, the lock assertions in test_clear_removes and test_import_injects_live_sessions_and_reports_the_note, and test_a_trickled_reply_cannot_outlive_the_exchange_deadline; each goes red with its fix reverted.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor
  • span=4de28b253547 Refresh all session-scoped cookie-status caches

fixed in c52591f: after an import or a clear, the hook writes its own slot's entry and invalidates every other entry under the browserCookies prefix, so another slot refetches under its own X-Session-Key instead of showing the old set.
test 'marks every OTHER slot stale after an import or clear' renders two slots on one query client and goes red without the invalidation.

@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor
  • span=305d946302da Surface cookie-status and live-injection failures

fixed in c52591f: an import whose hot_load.failed is not empty keeps the dialog open with an ErrorNotice naming how many live sessions refused it; the set stays saved, so there is no hand-off.
A status read is now retried twice and then settles as an error, which the Import menu item renders through ErrorNotice with a sibling hand-off item, the same pattern as a failed clear.
Tests: 'keeps the dialog open and says so when some live sessions refused the import', 'says a failed status read beside the Import item, with a hand-off', and 'settles a persistent status failure as an error after a bounded number of retries'.

@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 23, 2026
@bolichen97

Copy link
Copy Markdown
Contributor

This change continues in #23332, a branch of this repository, because the Sensitive Change Review gate cannot pass a fork PR that touches the sandbox and security path lists. The content is this PR's head rebased onto current main, plus two fixes for checks that were red here (a docs lint path and the regenerated pseudolocale). Your authorship is kept with a Co-authored-by trailer. This PR stays open for now; thank you for the work.

@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Oct 23, 2026
bolichen97 added a commit that referenced this pull request Oct 23, 2026
…oard

Add "Import cookies" to the Browser panel. The dashboard owner exports cookies
from the browser where they are logged in (a Playwright storageState, a
Cookie-Editor / "Get cookies.txt" JSON export, or Netscape cookies.txt) and
imports them. The gateway normalises them into an owner-only storageState
under the data home, a leaf masked from every agent sandbox and fenced at the
tool gate, and hands them to the agent's own browser daemons over each
daemon's control socket: one cookie-set request per cookie in the daemon's
newline-delimited JSON protocol, sent from the gateway process, so no cookie
value is ever on an argv. Live sessions get them at import; a gateway-side
watcher reaches sessions that start later. Clear removes the file and runs
cookie-clear on live sessions.

GET/POST/DELETE /api/browser/cookies are owner-only, refused in a restricted
session, SEL-audited with counts and domains only, and capped at 2 MiB and
5000 cookies. The summary the panel shows never carries a cookie value.

Supersedes #12377

Co-authored-by: Alejandro Diez Crespo <alejacre@amazon.com>
bolichen97 added a commit that referenced this pull request Oct 23, 2026
…oard

Add "Import cookies" to the Browser panel. The dashboard owner exports cookies
from the browser where they are logged in (a Playwright storageState, a
Cookie-Editor / "Get cookies.txt" JSON export, or Netscape cookies.txt) and
imports them. The gateway normalises them into an owner-only storageState
under the data home, a leaf masked from every agent sandbox and fenced at the
tool gate, and hands them to the agent's own browser daemons over each
daemon's control socket: one cookie-set request per cookie in the daemon's
newline-delimited JSON protocol, sent from the gateway process, so no cookie
value is ever on an argv. Live sessions get them at import; a gateway-side
watcher reaches sessions that start later. Clear removes the file and runs
cookie-clear on live sessions.

GET/POST/DELETE /api/browser/cookies are owner-only, refused in a restricted
session, SEL-audited with counts and domains only, and capped at 2 MiB and
5000 cookies. The summary the panel shows never carries a cookie value.

Supersedes #12377

Co-authored-by: Alejandro Diez Crespo <alejacre@amazon.com>
bolichen97 added a commit that referenced this pull request Oct 23, 2026
…oard

Add "Import cookies" to the Browser panel. The dashboard owner exports cookies
from the browser where they are logged in (a Playwright storageState, a
Cookie-Editor / "Get cookies.txt" JSON export, or Netscape cookies.txt) and
imports them. The gateway normalises them into an owner-only storageState
under the data home, a leaf masked from every agent sandbox and fenced at the
tool gate, and hands them to the agent's own browser daemons over each
daemon's control socket: one cookie-set request per cookie in the daemon's
newline-delimited JSON protocol, sent from the gateway process, so no cookie
value is ever on an argv. Live sessions get them at import; a gateway-side
watcher reaches sessions that start later. Clear removes the file and runs
cookie-clear on live sessions.

GET/POST/DELETE /api/browser/cookies are owner-only, refused in a restricted
session, SEL-audited with counts and domains only, and capped at 2 MiB and
5000 cookies. The summary the panel shows never carries a cookie value.

Supersedes #12377

Co-authored-by: Alejandro Diez Crespo <alejacre@amazon.com>
bolichen97 pushed a commit that referenced this pull request Oct 23, 2026
…oard

Add "Import cookies" to the Browser panel. The dashboard owner exports cookies
from the browser where they are logged in (a Playwright storageState, a
Cookie-Editor / "Get cookies.txt" JSON export, or Netscape cookies.txt) and
imports them. The gateway normalises them into an owner-only storageState
under the data home, a leaf masked from every agent sandbox and fenced at the
tool gate, and hands them to the agent's own browser daemons over each
daemon's control socket: one cookie-set request per cookie in the daemon's
newline-delimited JSON protocol, sent from the gateway process, so no cookie
value is ever on an argv. Live sessions get them at import; a gateway-side
watcher reaches sessions that start later. Clear removes the file and runs
cookie-clear on live sessions.

GET/POST/DELETE /api/browser/cookies are owner-only, refused in a restricted
session, SEL-audited with counts and domains only, and capped at 2 MiB and
5000 cookies. The summary the panel shows never carries a cookie value.

Supersedes #12377

Co-authored-by: Alejandro Diez Crespo <alejacre@amazon.com>
bolichen97 pushed a commit that referenced this pull request Oct 23, 2026
…oard

Add "Import cookies" to the Browser panel. The dashboard owner exports cookies
from the browser where they are logged in (a Playwright storageState, a
Cookie-Editor / "Get cookies.txt" JSON export, or Netscape cookies.txt) and
imports them. The gateway normalises them into an owner-only storageState
under the data home, a leaf masked from every agent sandbox and fenced at the
tool gate, and hands them to the agent's own browser daemons over each
daemon's control socket: one cookie-set request per cookie in the daemon's
newline-delimited JSON protocol, sent from the gateway process, so no cookie
value is ever on an argv. Live sessions get them at import; a gateway-side
watcher reaches sessions that start later. Clear removes the file and runs
cookie-clear on live sessions.

GET/POST/DELETE /api/browser/cookies are owner-only, refused in a restricted
session, SEL-audited with counts and domains only, and capped at 2 MiB and
5000 cookies. The summary the panel shows never carries a cookie value.

Supersedes #12377

Co-authored-by: Alejandro Diez Crespo <alejacre@amazon.com>
bolichen97 added a commit that referenced this pull request Oct 23, 2026
…oard

Add "Import cookies" to the Browser panel. The dashboard owner exports cookies
from the browser where they are logged in (a Playwright storageState, a
Cookie-Editor / "Get cookies.txt" JSON export, or Netscape cookies.txt) and
imports them. The gateway normalises them into an owner-only storageState
under the data home, a leaf masked from every agent sandbox and fenced at the
tool gate, and hands them to the agent's own browser daemons over each
daemon's control socket: one cookie-set request per cookie in the daemon's
newline-delimited JSON protocol, sent from the gateway process, so no cookie
value is ever on an argv. Live sessions get them at import; a gateway-side
watcher reaches sessions that start later. Clear removes the file and runs
cookie-clear on live sessions.

GET/POST/DELETE /api/browser/cookies are owner-only, refused in a restricted
session, SEL-audited with counts and domains only, and capped at 2 MiB and
5000 cookies. The summary the panel shows never carries a cookie value.

Supersedes #12377

Co-authored-by: Alejandro Diez Crespo <alejacre@amazon.com>
bolichen97 added a commit that referenced this pull request Oct 23, 2026
…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 added a commit that referenced this pull request Oct 24, 2026
…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 added a commit that referenced this pull request Oct 24, 2026
…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 added a commit that referenced this pull request Oct 24, 2026
…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 added a commit that referenced this pull request Oct 24, 2026
…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 added a commit that referenced this pull request Oct 24, 2026
…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 added a commit that referenced this pull request Oct 24, 2026
…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

Copy link
Copy Markdown
Contributor

[operator: bolichen97] Closing as superseded by #23332, which carries this change forward from a same-repo branch (a fork PR cannot pass the Sensitive Change Review). Please follow #23332; thank you for the contribution.

@bolichen97 bolichen97 closed this Oct 25, 2026
@github-actions github-actions Bot removed the readiness: action required A blocking check or review needs attention label Oct 25, 2026
@mbriones98 mbriones98 removed the drive-to-green PR claimed by drive-to-green pipeline label Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) merge conflict Branch has merge conflicts with its base — author must resolve before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants