fix(browser): persist managed Chrome cookies across restarts#98284
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 8:29 PM ET / 00:29 UTC. Summary PR surface: Source +80, Tests +106. Total +186 across 4 files. Reproducibility: yes. The linked issue includes a current-main live repro for a synthetic persistent cookie disappearing after managed browser restart, and current main source lacks the PR's Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land this bounded Chrome-owned persistence fix after maintainer acceptance of the mock-keychain security tradeoff, keeping cookie material inside Chrome's managed profile rather than adding an OpenClaw cookie sidecar. Do we have a high-confidence way to reproduce the issue? Yes. The linked issue includes a current-main live repro for a synthetic persistent cookie disappearing after managed browser restart, and current main source lacks the PR's Is this the best way to solve the issue? Yes, with a maintainer security decision. The PR follows the prior recommended shape by improving Chrome shutdown/profile handling and avoiding an OpenClaw plaintext cookie sidecar; the remaining choice is accepting mock keychain for new macOS headless profiles. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2c31af136429. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +80, Tests +106. Total +186 across 4 files. View PR surface stats
Security concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (8 earlier review cycles)
|
|
Updated the PR with the proof ClawSweeper asked for. Changes since the review:
After-fix live proof on
Validation:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Addressed the two ClawSweeper findings in Changes:
Validation:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
ee5511b to
1e9c646
Compare
4377e62 to
1670486
Compare
1670486 to
9a83733
Compare
|
Maintainer verification complete on the security-preserving redesign. What changed from the original cookie-sidecar proposal:
Security decision: I accept the deterministic mock-keychain tradeoff for this narrow profile class. The profile is new, isolated, OpenClaw-owned, and same-user private ( Automated proof:
Real Gateway/OpenAI/Browser proof on exact head:
Scope note: a session cookie without The shared sanitized-AWS pool was at its active-lease cap when this final head was ready, so I did not reuse another task's lease. The approved secretless exact-head hosted fork CI path above provided the full automated gate; macOS-specific keychain and Gateway behavior was proven live on the exact commit. Known proof gaps: none. |
|
Merged via squash.
|
…w#98284) * fix(browser): preserve managed Chrome cookies * fix(browser): guard graceful close process ownership --------- Co-authored-by: Peter Steinberger <[email protected]>
…w#98284) * fix(browser): preserve managed Chrome cookies * fix(browser): guard graceful close process ownership --------- Co-authored-by: Peter Steinberger <[email protected]>
Summary
openclawbrowser cookies across restarts by letting Chrome close gracefully through browser-level CDPBrowser.closebefore falling back to signals.--use-mock-keychainand mark the profile so future launches keep the same cookie encryption backend.What Problem This Solves
Managed browser sessions can hold login cookies in Chrome's live CDP cookie store while the profile's SQLite cookie DB remains empty. When OpenClaw stops and relaunches the managed browser without a graceful Chrome close, those cookies can disappear, so users lose authenticated browser state even when reusing the same managed profile.
Issue #96704 has a live repro against current upstream
origin/mainshowing a CDP-visible persistent cookie before restart,0rows in Chrome's cookie DB, and0cookies after relaunch with the same profile.Security / Product Shape
This final version does not store cookies in an OpenClaw plaintext sidecar. Cookie material stays in Chrome's profile storage.
For new local macOS headless managed profiles, the PR opts the isolated OpenClaw-owned profile into Chromium's mock keychain so headless Chrome can persist encrypted cookies without login-keychain prompts. That marker is stored in the managed profile's
Local State.For existing managed profiles created before this PR, the PR does not change the cookie encryption backend. The recovery path is: existing profiles keep their current backend and gain graceful
Browser.closeshutdown; sessions already lost by previous ungraceful restarts cannot be reconstructed and require signing in again.Evidence
96e8fa6c6629d9f0d7950fd295de0bf894dbecf6using isolated temp gateway/browser ports19984/19995, synthetic cookies only, temp state dirs under/private/tmp, and channel startup disabled.4377e6203386a3536e4125e8449543ef2d45d4ecrebased onto currentorigin/main, removed the release-ownedCHANGELOG.mdedit, and kept the browser files identical to the proven head (git diff --name-status 96e8fa6c6629..4377e620338 -- browser filesis empty).Fresh profile proof:
pnpm build.--headless; process args included--use-mock-keychain.profile.info_cache.Default.openclaw_mock_keychain=true.oc98284_smokethrough browser-level CDPStorage.setCookies;Storage.getCookiesreturnedmatchingCookies=1.openclaw browser stop; Chrome Cookies DB containedexample.test|oc98284_smoke|valueLen=0|encryptedLen=51|isPersistent=1.Storage.getCookiesreturnedtotalCookies=1,matchingCookies=1, value present.Upgraded current-main profile proof:
origin/mainat45f561ab6c2.--headless; process args did not include--use-mock-keychain.openclaw_mock_keychain=false.96e8fa6c6629.--use-mock-keychain, and the marker remainedfalse.oc98284_after_upgradethrough browser-level CDP;Storage.getCookiesreturnedmatchingCookies=1.openclaw browser stop; Chrome Cookies DB containedafter-upgrade.example.test|oc98284_after_upgrade|valueLen=0|encryptedLen=67|isPersistent=1.Storage.getCookiesreturnedtotalCookies=1,matchingCookies=1, value present.Validation on refreshed PR head
4377e6203386a3536e4125e8449543ef2d45d4ec:node scripts/run-vitest.mjs extensions/browser/src/browser/chrome.internal.test.ts extensions/browser/src/browser/chrome.test.ts:107/107passed.node_modules/.bin/oxfmt --check extensions/browser/src/browser/chrome.ts extensions/browser/src/browser/chrome.test.ts extensions/browser/src/browser/chrome.internal.test.ts extensions/browser/src/browser/chrome.profile-decoration.tsnode_modules/.bin/oxlint extensions/browser/src/browser/chrome.ts extensions/browser/src/browser/chrome.test.ts extensions/browser/src/browser/chrome.internal.test.ts extensions/browser/src/browser/chrome.profile-decoration.tsgit diff --cached --check && git diff --checkFixes #96704