Import Chrome sessions and history into cmux browser#1919
Import Chrome sessions and history into cmux browser#1919GaetanJuvin wants to merge 14 commits intomanaflow-ai:mainfrom
Conversation
…kies into WKWebView
…e conformance - Fix doc comment on chromeEpochOffset (seconds, not microseconds) - Add sqlite3_busy_timeout(1000) for WAL checkpoint lock contention - Remove SQLITE_OPEN_NOMUTEX flag - Make helper methods private static (readChromeKeychainPassword, deriveKey, decryptCookieValue, readCookies, cookieDBPath) - Fix sort closure edge case when both sides are "Default" - Add @unchecked Sendable to ChromeCookieImporter, Sendable to ImportResult
… browser settings
…cookies correctly Chrome stores host-only cookies without a leading dot (github.com) and domain cookies with one (.github.com). The old code unconditionally added a dot prefix, turning host-only cookies into domain cookies and breaking __Host- prefixed cookies which must not have a Domain attribute per the cookie spec. Also added diagnostic NSLog calls for debugging import issues.
Modern Chrome (v80+) prepends a 32-byte integrity hash to the plaintext before AES-128-CBC encryption. Our decryption was returning the hash + value as a single blob, which failed UTF-8 validation and caused all cookies to be silently skipped (0 cookies imported). Now strips the 32-byte prefix after decryption to extract the actual cookie value. Falls back to interpreting the full decrypted data for older Chrome formats where no prefix is present.
WKWebsiteDataStore.default().httpCookieStore was being accessed from the background importQueue, causing EXC_BREAKPOINT (SIGTRAP). WebKit APIs require main thread access. Move the cookie store access and setCookie loop inside DispatchQueue.main.async.
Adds ChromeCookieImporter.importIfNeeded() — a throttled import that runs at most every 5 minutes. Called from BrowserPanel.init so new browser tabs pick up any Chrome sessions that changed since the last import. Only runs if auto-import is enabled in settings.
Reads Chrome's History SQLite database (copied to temp file to avoid lock conflicts) and merges entries into BrowserHistoryStore. Imported URLs appear in the omnibar autocomplete. Skips URLs that already exist in cmux history. Limited to 5000 most recent entries.
…anel, use existing mergeImportedEntries, fix MainActor isolation
|
@GaetanJuvin is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds Chrome session import: a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as AppDelegate / UI
participant Importer as ChromeCookieImporter
participant Keychain as macOS Keychain
participant ChromeDB as Chrome Profile DBs
participant WK as WKWebsiteDataStore
participant History as BrowserHistoryStore
User->>App: Launch or click "Import Now"
App->>Importer: importIfNeeded() / importCookies(profile)
Importer->>Keychain: Request "Chrome Safe Storage" password
Keychain-->>Importer: Password or error
Importer->>ChromeDB: Open Cookies & History SQLite (copy if locked)
ChromeDB-->>Importer: Rows (cookies, visits)
Importer->>Importer: Decrypt cookie values (v10 → AES-128-CBC)
Importer->>WK: Set cookies (async)
Importer->>History: Merge history entries
Importer-->>App: ImportResult (cookieCount, optional error)
App->>User: Update UI/status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
Greptile SummaryThis PR adds first-party Chrome session bridging to cmux's embedded browser: a Key findings:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AppDelegate
participant BrowserPanel
participant ChromeCookieImporter
participant Keychain
participant ChromeSQLite
participant WKCookieStore
participant BrowserHistoryStore
User->>AppDelegate: App launch (auto-import enabled)
AppDelegate->>ChromeCookieImporter: importCookies()
ChromeCookieImporter->>Keychain: SecItemCopyMatching("Chrome Safe Storage")
Keychain-->>ChromeCookieImporter: password
ChromeCookieImporter->>ChromeCookieImporter: deriveKey (PBKDF2 / AES-128-CBC)
ChromeCookieImporter->>ChromeSQLite: sqlite3_open_v2(Cookies, READONLY)
ChromeSQLite-->>ChromeCookieImporter: [encrypted cookies]
ChromeCookieImporter->>ChromeCookieImporter: decryptCookieValue (strip v10 prefix + 32-byte hash)
ChromeCookieImporter->>ChromeSQLite: sqlite3_open_v2(History copy, READONLY)
ChromeSQLite-->>ChromeCookieImporter: [history entries]
ChromeCookieImporter-->>WKCookieStore: setCookie() ×N (main actor)
ChromeCookieImporter-->>BrowserHistoryStore: mergeImportedEntries()
User->>BrowserPanel: Open new browser tab
BrowserPanel->>ChromeCookieImporter: importIfNeeded() [throttled 5 min]
ChromeCookieImporter-->>BrowserPanel: no-op if < 5 min since last import
User->>User: Cmd+Shift+I (menu item)
User->>ChromeCookieImporter: importCookies() [manual]
ChromeCookieImporter-->>WKCookieStore: setCookie() ×N
|
| let isHostOnly = !hostKey.hasPrefix(".") | ||
| let domain = hostKey |
There was a problem hiding this comment.
isHostOnly is computed on this line but never referenced anywhere in the function body. The downstream logic uses only name.hasPrefix("__Host-") to decide whether to omit the Domain attribute. As a result:
- Non-
__Host-host-only cookies (wherehostKeyhas no leading dot, e.g."github.com") still receiveproperties[.domain] = domain, which sets aDomainattribute on what Chrome stored as a host-only cookie. Depending onNSHTTPCookie/WKWebView handling this may silently broaden the cookie's scope. - The
isHostOnlyvariable itself will generate a compiler warning about an unusedlet.
Consider using isHostOnly to gate the properties[.domain] assignment for cookies that are neither __Host- nor domain cookies:
if name.hasPrefix("__Host-") {
let scheme = isSecure ? "https" : "http"
properties[.originURL] = "\(scheme)://\(hostKey)"
} else if !isHostOnly {
properties[.domain] = domain // domain cookies (leading-dot hostKey)
} else {
properties[.domain] = domain // bare host — acceptable for NSHTTPCookie
}At minimum, remove the unused isHostOnly binding to eliminate the warning.
There was a problem hiding this comment.
3 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/Browser/ChromeCookieImporter.swift">
<violation number="1" location="Sources/Browser/ChromeCookieImporter.swift:84">
P2: Throttle logic is not in-flight safe; concurrent/rapid calls can queue multiple full imports before `lastImportTime` is updated.</violation>
<violation number="2" location="Sources/Browser/ChromeCookieImporter.swift:304">
P1: Chrome cookie import ignores plaintext `value` and can import empty cookie values when `encrypted_value` is absent.</violation>
<violation number="3" location="Sources/Browser/ChromeCookieImporter.swift:435">
P2: History import copies only the base SQLite file and ignores WAL sidecars, which can omit recent visits when Chrome is running in WAL mode.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
Resources/Localizable.xcstrings (1)
51188-51204: Subtitle text may be incomplete.The subtitle states "Import cookies from Chrome now" but the PR description indicates that both cookies AND browsing history are imported. Consider updating the copy to reflect the complete functionality.
Similarly, line 51143 mentions only "login cookies" without referencing history import.
📝 Suggested copy updates
For line 51194, consider:
-"value": "Import cookies from Chrome now." +"value": "Import cookies and browsing history from Chrome now."And for the Japanese translation:
-"value": "今すぐChromeからクッキーをインポートします。" +"value": "今すぐChromeからクッキーと閲覧履歴をインポートします。"For line 51143, consider:
-"value": "Copies your Chrome login cookies into cmux's browser so you're already signed in." +"value": "Copies your Chrome login cookies and browsing history into cmux's browser so you're already signed in with your session history preserved."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Localizable.xcstrings` around lines 51188 - 51204, Update the subtitle key "settings.browser.chromeCookie.importNow.subtitle" to mention both cookies and browsing history (e.g., "Import cookies and browsing history from Chrome now.") and update the Japanese localization accordingly; also find the other string that currently says only "login cookies" (search for the "login cookies" copy in Resources/Localizable.xcstrings) and change it to reference cookies and history (and update its Japanese translation) so the UI text matches the actual import behavior.Sources/Browser/ChromeCookieImporter.swift (1)
74-89: Minor: RedundantlastImportTimeupdate.
lastImportTimeis updated in two places: line 86 (only whencookieCount > 0) and line 534 (always on completion). This creates inconsistent behavior—successful imports with 0 cookies still update the timestamp via line 534 but skip line 86.Consider consolidating to a single update location for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Browser/ChromeCookieImporter.swift` around lines 74 - 89, Remove the duplicate shared.lastImportTime assignment in importIfNeeded and consolidate the timestamp update to the single, centralized completion location used after importCookies finishes; specifically, delete the shared.lastImportTime = now inside importIfNeeded's importCookies closure and rely on the existing centralized update (or if you prefer the other semantics, move the "only when cookieCount > 0" condition to that centralized completion so the timestamp is updated in exactly one place). Ensure the change references importIfNeeded, shared.lastImportTime, importCookies, and autoReimportInterval so the single-update behavior is consistent.Sources/Panels/BrowserPanel.swift (1)
2578-2581: Remove the XCTest verification request; gating is a valid optional optimization.The
importIfNeeded()call at line 2580 has no XCTest short-circuit, and there is no evidence it needs one—the method's own guards (auto-import toggle, Chrome installed check, throttle interval) are lightweight. However, gating the call to non-remote, default-profile panels is a valid optimization that avoids unnecessary UserDefaults access and isChromeInstalled checks on every remote/custom-profile panel creation.♻️ Suggested adjustment
- // Re-import Chrome cookies if enough time has passed since the last import. - ChromeCookieImporter.importIfNeeded() - let requestedProfileID = profileID ?? BrowserProfileStore.shared.effectiveLastUsedProfileID let resolvedProfileID = BrowserProfileStore.shared.profileDefinition(id: requestedProfileID) != nil ? requestedProfileID : BrowserProfileStore.shared.builtInDefaultProfileID + + // Re-import Chrome cookies only for local/default-profile panels. + if !isRemoteWorkspace, + resolvedProfileID == BrowserProfileStore.shared.builtInDefaultProfileID { + ChromeCookieImporter.importIfNeeded() + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanel.swift` around lines 2578 - 2581, Remove the XCTest short‑circuit and avoid calling ChromeCookieImporter.importIfNeeded() unconditionally; inside BrowserPanel (where ChromeCookieImporter.importIfNeeded() is invoked) wrap the call in a guard that only runs it for non‑remote panels using the default profile (e.g. check panel.isRemote == false and panel.profile.isDefault == true — adapt to the actual property names in the type), so you skip the import on remote/custom‑profile panels while leaving ChromeCookieImporter.importIfNeeded()'s own lightweight guards intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Browser/ChromeCookieImporter.swift`:
- Around line 9-11: ChromeCookieImporter currently exposes a non-thread-safe
stored property lastImportTime that is read from caller threads and written from
the main thread and the import completion, creating a data race; fix by making
lastImportTime access synchronized: either (a) add a private serial queue or
NSLock and replace direct accesses with thread-safe helpers like
getLastImportTime()/setLastImportTime(_:), or (b) move state into an actor or
ensure all reads/writes happen exclusively on importQueue (e.g., dispatch
reads/writes to importQueue). Update all places that touch lastImportTime
(including the import completion, main-thread writes, and caller reads) to use
the chosen synchronized accessor so the `@unchecked` Sendable no longer masks the
race.
- Around line 23-38: The errorDescription computed property returns hard-coded
English strings; update each returned message in the switch (in the
errorDescription implementation of the Chrome cookie importer error enum) to use
localized strings by replacing literal returns with String(localized: "…",
defaultValue: "…") (preserve interpolation for keychainError(status) and
databaseOpenFailed(reason) and include those values in the defaultValue), so
every case (.chromeNotInstalled, .keychainAccessDenied, .keychainError,
.databaseOpenFailed, .decryptionFailed, .noProfile) returns a localized String
using the String(localized:defaultValue:) API.
In `@Sources/cmuxApp.swift`:
- Around line 4340-4345: After repopulating chromeProfiles, validate
chromeCookieProfile against the refreshed list: if chromeCookieProfile is not
present in chromeProfiles.map(\.directory) then reset chromeCookieProfile to a
valid fallback (e.g. chromeProfiles.first?.directory) or nil when no profiles
exist; update the Picker binding and the Import Now action to be disabled when
chromeProfiles is empty or chromeCookieProfile is nil, and ensure any
auto-import logic checks the same validated value before submitting so stale or
deleted directories cannot be used (refer to chromeProfiles,
chromeCookieProfile, the Picker/ForEach tag and the Import Now action handler).
- Around line 4326-4327: The toggle label and subtitle using localization keys
"settings.browser.chromeCookie.autoImport" and
"settings.browser.chromeCookie.autoImport.subtitle" are misleading because the
behavior is not launch-only; it also controls throttled re-imports when new
browser tabs open. Update the localized strings to remove "on Launch" and
clearly describe continuous/import-on-new-tabs behavior (e.g., change title to
"Import Chrome Sessions" and subtitle to "Copies and periodically updates your
Chrome login cookies into cmux's browser, including when you open new tabs.")
and ensure any UI binding that references these keys continues to use the same
keys (or update the keys consistently wherever referenced).
- Around line 4357-4359: The settings UI is showing raw importer errors via
chromeCookieImportStatus = error.localizedDescription; change this to set
chromeCookieImportStatus to an app-localized, user-friendly message (use
String(localized:..., defaultValue:...)) and do not expose
result.error.localizedDescription to the user; instead log the raw error (e.g.,
via os_log or your logger) for diagnostics and keep chromeCookieImporting
updated as before. Ensure you reference chromeCookieImporting,
chromeCookieImportStatus and result.error when applying the change.
- Around line 4361-4365: The success message currently interpolates count into a
singular/localized string; change it to use pluralized localization with the
.one/.other keys and pass the numeric count to the localized formatter: update
the code that sets chromeCookieImportStatus (which reads result.cookieCount) to
call the pluralized localization API that accepts a count (so the localization
key "settings.browser.chromeCookie.importSuccess" uses .one/.other in the
stringsdict) instead of embedding "\(count)" in the default string.
---
Nitpick comments:
In `@Resources/Localizable.xcstrings`:
- Around line 51188-51204: Update the subtitle key
"settings.browser.chromeCookie.importNow.subtitle" to mention both cookies and
browsing history (e.g., "Import cookies and browsing history from Chrome now.")
and update the Japanese localization accordingly; also find the other string
that currently says only "login cookies" (search for the "login cookies" copy in
Resources/Localizable.xcstrings) and change it to reference cookies and history
(and update its Japanese translation) so the UI text matches the actual import
behavior.
In `@Sources/Browser/ChromeCookieImporter.swift`:
- Around line 74-89: Remove the duplicate shared.lastImportTime assignment in
importIfNeeded and consolidate the timestamp update to the single, centralized
completion location used after importCookies finishes; specifically, delete the
shared.lastImportTime = now inside importIfNeeded's importCookies closure and
rely on the existing centralized update (or if you prefer the other semantics,
move the "only when cookieCount > 0" condition to that centralized completion so
the timestamp is updated in exactly one place). Ensure the change references
importIfNeeded, shared.lastImportTime, importCookies, and autoReimportInterval
so the single-update behavior is consistent.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 2578-2581: Remove the XCTest short‑circuit and avoid calling
ChromeCookieImporter.importIfNeeded() unconditionally; inside BrowserPanel
(where ChromeCookieImporter.importIfNeeded() is invoked) wrap the call in a
guard that only runs it for non‑remote panels using the default profile (e.g.
check panel.isRemote == false and panel.profile.isDefault == true — adapt to the
actual property names in the type), so you skip the import on
remote/custom‑profile panels while leaving
ChromeCookieImporter.importIfNeeded()'s own lightweight guards intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 881e6524-68b8-40f2-9dcb-69b5d910bfe2
📒 Files selected for processing (6)
GhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/AppDelegate.swiftSources/Browser/ChromeCookieImporter.swiftSources/Panels/BrowserPanel.swiftSources/cmuxApp.swift
- Fix thread-safety race on lastImportTime with NSLock + importInFlight flag - Remove unused isHostOnly variable - Localize error descriptions with String(localized:defaultValue:) - Wrap NSLog calls in #if DEBUG to avoid leaking security-sensitive info - Add plaintext value column fallback when encrypted_value is absent - Copy WAL sidecar files (-wal, -shm) when importing Chrome history - Rename toggle to Auto-Import Chrome Sessions (covers launch + new tabs) - Validate stored profile against available profiles on settings appear - Show localized user-friendly error instead of raw error in settings UI - Pluralize success message (1 cookie vs N cookies)
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/cmuxApp.swift">
<violation number="1" location="Sources/cmuxApp.swift:4380">
P2: Settings `onAppear` now persistently rewrites the saved Chrome profile when discovery temporarily misses it, which can silently redirect future imports to the wrong profile.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| .onAppear { | ||
| chromeProfiles = ChromeCookieImporter.availableProfiles() | ||
| if !chromeProfiles.contains(where: { $0.directory == chromeCookieProfile }) { | ||
| chromeCookieProfile = chromeProfiles.first?.directory ?? ChromeCookieSettings.defaultProfile |
There was a problem hiding this comment.
P2: Settings onAppear now persistently rewrites the saved Chrome profile when discovery temporarily misses it, which can silently redirect future imports to the wrong profile.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/cmuxApp.swift, line 4380:
<comment>Settings `onAppear` now persistently rewrites the saved Chrome profile when discovery temporarily misses it, which can silently redirect future imports to the wrong profile.</comment>
<file context>
@@ -4356,23 +4356,29 @@ struct SettingsView: View {
.onAppear {
chromeProfiles = ChromeCookieImporter.availableProfiles()
+ if !chromeProfiles.contains(where: { $0.directory == chromeCookieProfile }) {
+ chromeCookieProfile = chromeProfiles.first?.directory ?? ChromeCookieSettings.defaultProfile
+ }
}
</file context>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Sources/cmuxApp.swift (1)
3816-3817:⚠️ Potential issue | 🟠 MajorNormalize the stored Chrome profile outside SettingsView.
The fallback here only runs when the settings section appears.
ChromeCookieImporter.importCookies(profile: nil)still readsChromeCookieSettings.profileKeystraight fromUserDefaults, so launch auto-import and the menu command can keep using a deleted/renamed profile until Settings is opened again.resetAllSettings()also writesdefaultProfileback without revalidating it, which can reintroduce the same stale value. This resolution needs to live in shared settings/importer code, not only in the view layer.Also applies to: 4377-4381, 5633-5634
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/cmuxApp.swift`:
- Around line 771-778: The manual menu/button actions call
ChromeCookieImporter.importCookies(...) which bypasses the importInFlight guard;
change those callers to the importer-level guarded entrypoint so manual triggers
participate in the same in-flight protection but still skip the 5-minute
throttle. Concretely, replace direct calls to
ChromeCookieImporter.importCookies(...) in Source cmuxApp.swift (and the other
noted block) with the shared guarded API on the importer (e.g.
ChromeCookieImporter.shared.importIfNeeded(skipThrottle: true) or a new
ChromeCookieImporter.importCookiesWithGuard(...)) so the call uses the
importInFlight lock while passing the flag to ignore the 5-minute throttle;
preserve the existing completion handling and error logging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ee8617c-c003-4fa6-9a2e-8f43535ae07b
📒 Files selected for processing (2)
Sources/Browser/ChromeCookieImporter.swiftSources/cmuxApp.swift
✅ Files skipped from review due to trivial changes (1)
- Sources/Browser/ChromeCookieImporter.swift
| Button(String(localized: "menu.view.importChromeSessions", defaultValue: "Import Chrome Sessions")) { | ||
| ChromeCookieImporter.importCookies { result in | ||
| if let error = result.error { | ||
| NSLog("[ChromeCookieImporter] manual import failed: \(error.localizedDescription)") | ||
| } | ||
| } | ||
| } | ||
| .keyboardShortcut("i", modifiers: [.command, .shift]) |
There was a problem hiding this comment.
Use the importer’s shared in-flight guard for both manual triggers.
These paths call ChromeCookieImporter.importCookies(...) directly, but in Sources/Browser/ChromeCookieImporter.swift that API bypasses the importInFlight lock used by importIfNeeded(). A launch/new-tab auto-import can therefore overlap with a menu/settings manual import, and the local chromeCookieImporting state only blocks repeated clicks in the current settings view. Please route manual imports through a shared importer-level entry point that still skips the 5-minute throttle but participates in the same in-flight protection.
Also applies to: 4353-4374
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/cmuxApp.swift` around lines 771 - 778, The manual menu/button actions
call ChromeCookieImporter.importCookies(...) which bypasses the importInFlight
guard; change those callers to the importer-level guarded entrypoint so manual
triggers participate in the same in-flight protection but still skip the
5-minute throttle. Concretely, replace direct calls to
ChromeCookieImporter.importCookies(...) in Source cmuxApp.swift (and the other
noted block) with the shared guarded API on the importer (e.g.
ChromeCookieImporter.shared.importIfNeeded(skipThrottle: true) or a new
ChromeCookieImporter.importCookiesWithGuard(...)) so the call uses the
importInFlight lock while passing the flag to ignore the 5-minute throttle;
preserve the existing completion handling and error logging.
Summary
ChromeCookieImporterthat reads Chrome's encrypted cookie database, decrypts values using the macOS Keychain, and injects them into WKWebView so users are already logged in to sites they use in Chrome.How it works
saltysalt, 1003 iterations)v10-prefixed cookie values, strips 32-byte integrity hash prefixWKWebsiteDataStore.default().httpCookieStoreBrowserHistoryStoreTest plan
Summary by CodeRabbit