Skip to content

fix(desktop): reduce browser Keychain prompts during Google import#10124

Draft
skanderkaroui wants to merge 4 commits into
mainfrom
fix/browser-multi-profile-priority
Draft

fix(desktop): reduce browser Keychain prompts during Google import#10124
skanderkaroui wants to merge 4 commits into
mainfrom
fix/browser-multi-profile-priority

Conversation

@skanderkaroui

@skanderkaroui skanderkaroui commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Filter installed Chromium browsers and profiles using keychain-free Local State metadata, file activity, running-browser singleton links, and plaintext Google auth-cookie metadata.
  • Keep every recent Google-authenticated profile, ordered by running browser, preferred profile, and latest activity.
  • Fall back to only the newest authenticated stale profile when nothing was active within 30 days.

Root cause

configsForPython treated every Cookies file as sufficient evidence and read each browser's Safe Storage item before proving that the browser was recent or had a Google session. That surfaced unnecessary blocking macOS Keychain prompts for stale or unused browsers.

User impact

Onboarding now prompts only for plausible Gmail/Calendar sources. Multiple active profiles remain available in priority order, profiles in one browser reuse the same cached Safe Storage key, and imports stop on the first working session.

Validation

  • ./scripts/dev-feedback.py --once swift 'BrowserGoogleSessionTests'
  • ./scripts/swift-test-suites.sh (337 isolated Swift suites)
  • desktop/macos/scripts/swift-format-wrapper.sh lint Desktop/Sources/BrowserGoogleSession.swift Desktop/Tests/BrowserGoogleSessionTests.swift
  • python3 desktop/macos/scripts/check_desktop_test_quality.py
  • Mac mini M2: ./desktop/macos/test.sh passed all launcher checks and 376 Rust tests; 336/337 Swift suites passed headlessly, and the sole AppKit pasteboard test passed when rerun in the active GUI bootstrap it requires.
  • Mac mini M2: OMI_PR_BODY_FILE=/tmp/pr-body-browser-session.md make preflight passed all 18 selected checks.

Failure-Class: none

Review in cubic

Select one main profile per installed browser, reject stale profiles, and require plaintext Google account or auth-cookie evidence before reading Safe Storage.

Tests: ./scripts/dev-feedback.py --once swift 'BrowserGoogleSessionTests'; ./scripts/swift-test-suites.sh; OMI_LIVE_BROWSER_GATE=1 live production-gate probe with stubbed keychain loader; Swift format/lint and desktop test-quality checks.

Failure-Class: none
Keep every recent Google-authenticated profile, order running browsers and preferred profiles first, and fall back to only the newest stale session when no profile was active within 30 days.

Tests: ./scripts/dev-feedback.py --once swift 'BrowserGoogleSessionTests'; ./scripts/swift-test-suites.sh; Swift format lint and desktop test-quality checks.

Failure-Class: none
@Git-on-my-level

Copy link
Copy Markdown
Collaborator

Thanks for tightening this path — filtering browser/profile candidates before touching the Safe Storage key is the right direction and should reduce unnecessary Keychain prompts.

One thing I’d fix before marking this ready: the new cookie prefilter still uses substring matching for Google hosts (LIKE '%google.com%' / LIKE '%gmail.com%'). A profile with a lookalike domain such as evilgoogle.com and a cookie named SID would still be treated as Google-authenticated, causing an unnecessary Keychain prompt and cookie-decrypt attempt. Since this code is explicitly a privacy/security-sensitive gate before reading browser Safe Storage, please make the host predicate domain-bound instead (for example exact google.com/gmail.com or suffix .google.com/.gmail.com) and add a regression test for the lookalike-domain case.

I did not see evidence of a malicious/supply-chain attempt in the static diff. Because this is draft, touches browser cookies/Keychain behavior, and the Desktop Swift CI is still in progress, this should still get human maintainer review before merge.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@Git-on-my-level Git-on-my-level added security-review Touches auth, provider routing, secrets, or security-sensitive surfaces needs-maintainer-review Needs a human maintainer to sign off before merge labels Jul 20, 2026

@undivisible undivisible left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes

Direction is right: stop treating every Cookies file as a Keychain invitation. Local State + Google auth-cookie evidence before Safe Storage, running-browser first, 30-day recency with newest-stale fallback — that’s how you stop the Keychain paparazzi.

Please

  1. Mark Ready for review only after a live multi-browser pass (Chrome + one alt, multi-profile) confirms fewer prompts and first working session still imports.
  2. Rebase onto current main and clear the red/cancelled Desktop Swift Build & Tests run.
  3. Keep the draft until (1)+(2) — security-adjacent path, wants a clean gate.

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

Labels

needs-maintainer-review Needs a human maintainer to sign off before merge security-review Touches auth, provider routing, secrets, or security-sensitive surfaces

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants