diff --git a/desktop/macos/Desktop/Sources/BrowserGoogleSession.swift b/desktop/macos/Desktop/Sources/BrowserGoogleSession.swift index 9f058597d1f..74608d45b28 100644 --- a/desktop/macos/Desktop/Sources/BrowserGoogleSession.swift +++ b/desktop/macos/Desktop/Sources/BrowserGoogleSession.swift @@ -1,9 +1,11 @@ import Darwin import Foundation +import Security struct BrowserGoogleSession: Equatable { let browserName: String let keychainService: String + let keychainAccount: String let cookiePath: String static let chromiumCookiePythonSupport = """ @@ -72,6 +74,15 @@ struct BrowserGoogleSession: Equatable { value = decrypted.decode('latin-1') except Exception: continue + elif enc[:1] == b'v' and enc[1:3].isdigit(): + # Versioned but not v10/v11 (e.g. v20 app-bound, or a newer macOS + # scheme whose key lives in iCloud Keychain). We can't decrypt it, so + # skip it — never fall through to the plaintext branch below, which + # would emit the raw ciphertext as a garbage "cookie" value. + # ponytail: deliberately no v20/app-bound decoder (YAGNI on macOS + # today). Ceiling: these browsers silently contribute no cookies; + # upgrade path is Google OAuth, not a per-version scraper. + continue elif enc: try: value = enc.decode('latin-1') @@ -114,7 +125,7 @@ struct BrowserGoogleSession: Equatable { static func all() -> [BrowserGoogleSession] { let homeDirectory = FileManager.default.homeDirectoryForCurrentUser return BrowserAutomationTargetResolver.knownTargets.flatMap { target in - guard let keychainService = keychainService(for: target) else { + guard let keychainIdentity = keychainIdentity(for: target) else { return [BrowserGoogleSession]() } let userDataPath = target.profileRoot(homeDirectory: homeDirectory).path @@ -128,7 +139,8 @@ struct BrowserGoogleSession: Equatable { let browserName = profileName == "Default" ? target.name : "\(target.name) (\(profileName))" return BrowserGoogleSession( browserName: browserName, - keychainService: keychainService, + keychainService: keychainIdentity.service, + keychainAccount: keychainIdentity.account, cookiePath: cookiePath ) } @@ -138,7 +150,12 @@ struct BrowserGoogleSession: Equatable { static func configsForPython(logPrefix: String) -> [[String: String]] { all().compactMap { session in guard FileManager.default.fileExists(atPath: session.cookiePath) else { return nil } - guard let password = BrowserKeychainCache.shared.password(for: session.keychainService) else { + guard + let password = BrowserKeychainCache.shared.password( + for: session.keychainService, + account: session.keychainAccount + ) + else { log("\(logPrefix): No keychain password for \(session.browserName)") return nil } @@ -159,7 +176,8 @@ struct BrowserGoogleSession: Equatable { .compactMap { entry -> (name: String, path: String)? in var isDirectory: ObjCBool = false let profilePath = "\(userDataPath)/\(entry)" - guard fm.fileExists(atPath: profilePath, isDirectory: &isDirectory), isDirectory.boolValue else { + guard fm.fileExists(atPath: profilePath, isDirectory: &isDirectory), isDirectory.boolValue + else { return nil } let networkCookies = "\(profilePath)/Network/Cookies" @@ -184,37 +202,48 @@ struct BrowserGoogleSession: Equatable { .filter { fm.fileExists(atPath: $0) } } - private static func keychainService(for target: BrowserAutomationTarget) -> String? { + static func keychainIdentity(for target: BrowserAutomationTarget) -> ( + service: String, account: String + )? { switch target.bundleIdentifier { case "company.thebrowser.Browser": - return "Arc Safe Storage" + return ("Arc Safe Storage", "Arc") case "com.google.Chrome", "com.google.Chrome.beta", "com.google.Chrome.canary", "com.openai.atlas": - return "Chrome Safe Storage" + return ("Chrome Safe Storage", "Chrome") case "com.brave.Browser", "com.brave.Browser.beta", "com.brave.Browser.nightly": - return "Brave Safe Storage" + return ("Brave Safe Storage", "Brave") case "com.microsoft.edgemac", "com.microsoft.edgemac.Beta", "com.microsoft.edgemac.Dev", "com.microsoft.edgemac.Canary": - return "Microsoft Edge Safe Storage" + return ("Microsoft Edge Safe Storage", "Microsoft Edge") case "com.operasoftware.Opera", "com.operasoftware.OperaGX": - return "Opera Safe Storage" + return ("Opera Safe Storage", "Opera") case "org.chromium.Chromium": - return "Chromium Safe Storage" + return ("Chromium Safe Storage", "Chromium") case "com.vivaldi.Vivaldi": - return "Vivaldi Safe Storage" + return ("Vivaldi Safe Storage", "Vivaldi") default: return nil } } } -/// Settled browser Safe Storage strategy for Chromium cookie scraping. +/// Browser Safe Storage strategy for Chromium cookie scraping. +/// +/// Primary path: read the browser-created generic-password item in-process via +/// `SecItemCopyMatching`. macOS attributes the keychain prompt to the *requesting +/// process*, so the in-process read shows " wants to access …" — the app +/// identity the user must recognize before granting — instead of "security wants to +/// access …" (which is what shelling out to `/usr/bin/security` produced). +/// +/// A first cross-app read requires user approval. For a stably signed app, choosing +/// "Always Allow" lets macOS add this app's code-signing identity and partition ID to +/// the item's ACL, so later reads can proceed without another prompt. We intentionally +/// do not retry through `/usr/bin/security`: that would attribute any second prompt to +/// the CLI and persist access for the wrong requester. /// -/// We use `/usr/bin/security find-generic-password -w` instead of -/// `SecItemCopyMatching` because the CLI path matches the browser-created -/// generic-password item and lets macOS remember the user's "Always Allow" -/// decision. The in-memory cache coalesces concurrent reads during this app run; -/// we do not duplicate browser Safe Storage secrets into app preferences. +/// The in-memory cache below coalesces concurrent reads within a single app run; we do +/// not duplicate browser Safe Storage secrets into app preferences. final class BrowserKeychainCache: @unchecked Sendable { static let shared = BrowserKeychainCache() @@ -231,33 +260,45 @@ final class BrowserKeychainCache: @unchecked Sendable { UserDefaults.standard.removeObject(forKey: "cachedBrowserKeychainPasswords") } - func password(for service: String) -> String? { - password(for: service) { - let process = Process() - process.executableURL = URL(fileURLWithPath: "/usr/bin/security") - process.arguments = ["find-generic-password", "-s", service, "-w"] - let pipe = Pipe() - let errPipe = Pipe() - process.standardOutput = pipe - process.standardError = errPipe - do { - try process.run() - process.waitUntilExit() - guard process.terminationStatus == 0 else { return nil } - let output = String(data: pipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8)? - .trimmingCharacters(in: .whitespacesAndNewlines) - return output?.isEmpty == false ? output : nil - } catch { - return nil - } + func password(for service: String, account: String) -> String? { + password(for: "\(service)\u{0}\(account)") { + Self.nativeSafeStoragePassword(for: service, account: account) + } + } + + static func safeStorageQuery(service: String, account: String) -> [String: Any] { + [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service, + kSecAttrAccount as String: account, + kSecReturnData as String: true, + kSecMatchLimit as String: kSecMatchLimitOne, + ] + } + + /// Reads the browser Safe Storage key in-process so the prompt and any durable + /// "Always Allow" grant belong to this app rather than `/usr/bin/security`. + private static func nativeSafeStoragePassword(for service: String, account: String) -> String? { + var item: CFTypeRef? + let status = SecItemCopyMatching( + safeStorageQuery(service: service, account: account) as CFDictionary, + &item + ) + guard status == errSecSuccess, + let data = item as? Data, + let password = String(data: data, encoding: .utf8), + !password.isEmpty + else { + return nil } + return password } - func password(for service: String, loader: () -> String?) -> String? { + func password(for cacheKey: String, loader: () -> String?) -> String? { loop: while true { lock.lock() - if let cached = cache[service] { + if let cached = cache[cacheKey] { lock.unlock() switch cached { case .found(let password): return password @@ -265,7 +306,7 @@ final class BrowserKeychainCache: @unchecked Sendable { } } - if let group = inFlight[service] { + if let group = inFlight[cacheKey] { lock.unlock() group.wait() continue loop @@ -273,18 +314,18 @@ final class BrowserKeychainCache: @unchecked Sendable { let group = DispatchGroup() group.enter() - inFlight[service] = group + inFlight[cacheKey] = group lock.unlock() let password = loader() lock.lock() if let password { - cache[service] = .found(password) + cache[cacheKey] = .found(password) } else { - cache[service] = .missing + cache[cacheKey] = .missing } - let completedGroup = inFlight.removeValue(forKey: service) + let completedGroup = inFlight.removeValue(forKey: cacheKey) lock.unlock() completedGroup?.leave() @@ -292,9 +333,9 @@ final class BrowserKeychainCache: @unchecked Sendable { } } - func invalidate(service: String) { + func invalidate(cacheKey: String) { lock.lock() - cache.removeValue(forKey: service) + cache.removeValue(forKey: cacheKey) lock.unlock() } } diff --git a/desktop/macos/Desktop/Tests/BrowserGoogleSessionTests.swift b/desktop/macos/Desktop/Tests/BrowserGoogleSessionTests.swift index 896773fc373..535ac546372 100644 --- a/desktop/macos/Desktop/Tests/BrowserGoogleSessionTests.swift +++ b/desktop/macos/Desktop/Tests/BrowserGoogleSessionTests.swift @@ -37,4 +37,80 @@ final class BrowserGoogleSessionTests: XCTestCase { profile2.appendingPathComponent("Cookies").path, ]) } + + func testSafeStorageIdentitiesMatchBrowserItems() throws { + let expected: [String: (service: String, account: String)] = [ + "com.openai.atlas": ("Chrome Safe Storage", "Chrome"), + "com.google.Chrome": ("Chrome Safe Storage", "Chrome"), + "com.google.Chrome.beta": ("Chrome Safe Storage", "Chrome"), + "com.google.Chrome.canary": ("Chrome Safe Storage", "Chrome"), + "com.brave.Browser": ("Brave Safe Storage", "Brave"), + "com.brave.Browser.beta": ("Brave Safe Storage", "Brave"), + "com.brave.Browser.nightly": ("Brave Safe Storage", "Brave"), + "com.microsoft.edgemac": ("Microsoft Edge Safe Storage", "Microsoft Edge"), + "com.microsoft.edgemac.Beta": ("Microsoft Edge Safe Storage", "Microsoft Edge"), + "com.microsoft.edgemac.Dev": ("Microsoft Edge Safe Storage", "Microsoft Edge"), + "com.microsoft.edgemac.Canary": ("Microsoft Edge Safe Storage", "Microsoft Edge"), + "company.thebrowser.Browser": ("Arc Safe Storage", "Arc"), + "com.operasoftware.Opera": ("Opera Safe Storage", "Opera"), + "com.operasoftware.OperaGX": ("Opera Safe Storage", "Opera"), + "org.chromium.Chromium": ("Chromium Safe Storage", "Chromium"), + "com.vivaldi.Vivaldi": ("Vivaldi Safe Storage", "Vivaldi"), + ] + + for target in BrowserAutomationTargetResolver.knownTargets { + let expectedIdentity = try XCTUnwrap(expected[target.bundleIdentifier]) + let actual = try XCTUnwrap(BrowserGoogleSession.keychainIdentity(for: target)) + XCTAssertEqual(actual.service, expectedIdentity.service, target.bundleIdentifier) + XCTAssertEqual(actual.account, expectedIdentity.account, target.bundleIdentifier) + } + } + + func testSafeStorageQueryMatchesTheFullGenericPasswordIdentity() { + let query = BrowserKeychainCache.safeStorageQuery( + service: "Chrome Safe Storage", + account: "Chrome" + ) + + XCTAssertEqual(query[kSecAttrService as String] as? String, "Chrome Safe Storage") + XCTAssertEqual(query[kSecAttrAccount as String] as? String, "Chrome") + XCTAssertEqual(query[kSecReturnData as String] as? Bool, true) + } + + func testUnknownVersionCookieBlobIsSkippedInsteadOfEmittedAsPlaintext() throws { + let databasePath = tempRoot.appendingPathComponent("Cookies").path + let script = + BrowserGoogleSession.chromiumCookiePythonSupport + """ + + db_path = sys.argv[1] + conn = sqlite3.connect(db_path) + conn.execute('CREATE TABLE meta (key TEXT, value TEXT)') + conn.execute('INSERT INTO meta VALUES ("version", "24")') + conn.execute('''CREATE TABLE cookies ( + host_key TEXT, name TEXT, encrypted_value BLOB, path TEXT, + is_secure INTEGER, expires_utc INTEGER + )''') + conn.execute( + 'INSERT INTO cookies VALUES (?, ?, ?, ?, ?, ?)', + ('.google.com', 'VERSIONED', sqlite3.Binary(b'v20ciphertext'), '/', 1, 0) + ) + conn.execute( + 'INSERT INTO cookies VALUES (?, ?, ?, ?, ?, ?)', + ('.google.com', 'PLAINTEXT', sqlite3.Binary(b'plain-cookie'), '/', 1, 0) + ) + conn.commit() + conn.close() + + cookies, error = decrypt_google_cookies(db_path, 'unused') + assert error is None, error + assert [cookie['name'] for cookie in cookies] == ['PLAINTEXT'], cookies + """ + + let result = try BrowserPythonRunner.run(script: script, arguments: [databasePath]) + XCTAssertEqual( + result.terminationStatus, + 0, + String(data: result.stderr, encoding: .utf8) ?? "Python cookie check failed" + ) + } } diff --git a/desktop/macos/changelog/unreleased/20260710-keychain-prompt-attribution.json b/desktop/macos/changelog/unreleased/20260710-keychain-prompt-attribution.json new file mode 100644 index 00000000000..de02d462b17 --- /dev/null +++ b/desktop/macos/changelog/unreleased/20260710-keychain-prompt-attribution.json @@ -0,0 +1,3 @@ +{ + "change": "Fixed the onboarding browser-import keychain prompt showing a confusing \"security wants to access\" dialog — it now clearly asks on behalf of Omi" +} diff --git a/docs/doc/developer/gmail-calendar-oauth-migration.md b/docs/doc/developer/gmail-calendar-oauth-migration.md new file mode 100644 index 00000000000..bad76d52365 --- /dev/null +++ b/docs/doc/developer/gmail-calendar-oauth-migration.md @@ -0,0 +1,138 @@ +# Gmail + Calendar: cookie-scraping → Google OAuth (migration plan) + +Status: **planned** · Scope: desktop onboarding "connect your context" import. + +Ponytail note: this plan is deliberately reuse-first. It builds almost nothing new — +it wires the desktop to OAuth infra the backend already ships. Sections marked +`ponytail:` are corners cut on purpose, with the ceiling named. + +## Why + +Onboarding imports Gmail + Calendar by scraping browser cookies: it reads the +Chromium "Safe Storage" key from the macOS Keychain and decrypts the cookie DB +(`BrowserGoogleSession` + `GmailReaderService` / `CalendarReaderService`). + +That path cannot be made silent or durable: + +- macOS **mandates** a Keychain prompt to read another app's item; no entitlement + removes the first prompt. +- It drifts every macOS version (Tahoe broke `/usr/bin/security` reads) and every + browser version (cookie formats v10→v20 app-bound, keys migrating to iCloud + Keychain). + +Google OAuth is the only path that is silent, automatic, and version-proof. + +## The seam (what changes vs. what does not) + +The fetch layer is cleanly separable. Only these two functions change: + +- `GmailReaderService.readRecentEmails() -> [GmailEmail]` +- `CalendarReaderService.readEvents() -> [CalendarEvent]` + +Everything downstream is source-agnostic and stays byte-for-byte: + +``` +readRecentEmails()/readEvents() ← REPLACE (cookies → OAuth) + ↓ [GmailEmail]/[CalendarEvent] +synthesizeFrom*() (LLM) ← unchanged + ↓ ImportEvidenceBatchItem +OnboardingImportEvidenceService.save() + ↓ POST /v3/memory-imports/batch ← unchanged +``` + +`ImportEvidenceBatchItem` is the contract; keep producing it and nothing else moves. + +## Reuse map — the backend already ships ~70% of this + +| Piece | Where | Reuse | +|---|---|---| +| Google OAuth flow (PKCE, callback) | `backend/routers/auth.py` (scopes: `openid email profile`) | extend | +| Google token refresh + retry | `backend/utils/retrieval/tools/google_utils.py` | copy as-is | +| OAuth callback + Firestore token storage | `backend/routers/integrations.py` → `users/{uid}/integrations/{app_key}` | clone | +| **Live Google Calendar OAuth integration** | `backend/routers/integrations.py` (google-calendar) | **wire desktop to it** | +| Full OAuth+sync connector template | `backend/utils/x_connector.py` | mirror for Gmail | +| Onboarding state machine | `backend/routers/calendar_onboarding.py` | copy → gmail | +| Desktop loopback OAuth callback | `desktop/macos/.../AuthService.swift` | reuse | +| Gmail API client + Gmail scope | — | **build (only real new work)** | + +## Architecture (decisions locked) + +- **Backend-mediated.** Desktop calls the backend; the backend holds the token and + calls Google. Keeps the client secret server-side, centralizes refresh via + `google_utils`, and makes mobile/web free later. +- **Separate Google connector, not incremental-auth-on-sign-in.** Users who signed + in with Apple have no Google token; a standalone connector works regardless of + sign-in method (and matches how Calendar already works). +- **Gmail scope: `gmail.readonly`** (matches today's subject+snippet fidelity). + ⚠️ This is a Google **restricted** scope → heightened verification + a likely + annual third-party security assessment (CASA). Budget it. Calendar's scope is + only "sensitive" and is already verified/live — which is why Calendar goes first. + +## Phases + +### Phase 0 — Calendar over OAuth (do first, small) +The backend Google Calendar integration already exists AND already reads events from +the Google API: `backend/utils/retrieval/tools/calendar_tools.py:290` calls +`googleapis.com/calendar/v3/.../events` with the stored OAuth token (refreshed via +`google_utils`). So Phase 0 is thin wiring, not new OAuth: +- Backend: expose a read endpoint (e.g. `GET /v1/calendar/events?days_back&days_forward`) + as a thin wrapper over the existing `calendar_tools` list-events call. Hermetic test + mocks the Google layer and asserts the response maps to the desktop `CalendarEvent`. +- Desktop: `CalendarReaderService.readEvents()` → call that endpoint (with the user's + Firebase session) instead of scraping cookies. +- Trigger the existing `/v1/integrations/google-calendar` OAuth-url flow from the + onboarding step when not yet connected; reuse `AuthService` loopback callback. +- Reuse `calendar_onboarding.py` status/skip/reset as-is. +- Result: Calendar import needs no Keychain prompt. Days, not weeks. + +### Phase 1 — Gmail backend (bigger) +- Add `gmail.readonly` to a Google connector (clone the X/Calendar callback + + `google_utils` refresh; store under `users/{uid}/integrations/gmail`). +- New Gmail API client + endpoint: list/get messages via + `gmail.googleapis.com/gmail/v1/users/me/messages`, refresh-on-401. +- New `gmail_onboarding.py` (copy `calendar_onboarding.py`). +- **Compliance gate:** restricted-scope verification / CASA. Start early; it's the + long pole, not the code. + +### Phase 2 — Desktop swap + flag +- Put both readers behind a `useOAuth` flag at the seam; on failure fall back to the + (now-fixed) cookie path during rollout. + +### Phase 3 — Mobile / web +- Free once backend-mediated. ponytail: not designed here until Phase 2 lands. + +## Explicitly NOT doing (ponytail cuts) + +- **No new token-encryption scheme.** Reuse the existing `integrations/{app_key}` + storage (plaintext at app layer, GCP-encrypted at rest), same as Calendar/X. + Ceiling: if integration-token sensitivity policy changes, migrate all connectors + together, not just Gmail. +- **No MCP-style multi-client OAuth tables** (`mcp_oauth.py`). YAGNI — one grant per + user per provider via `integrations/{app_key}`. Ceiling: only needed if third-party + clients must hold Gmail grants. +- **No incremental-auth-on-sign-in.** Standalone connector instead (works for Apple + sign-in users). +- **Do not rip out the cookie path.** It stays as the Phase 2 fallback and covers any + surface OAuth hasn't reached. The two committed cookie fixes (in-process Keychain + read + unknown-version skip) remain. + +## Open items + +- Confirm the exact current Google restricted-scope verification requirements before + committing Gmail work (policy shifts; verify against live Google docs). +- Map `calendar_tools` list-events output → desktop `CalendarEvent` (fields align; + confirm all-day + attendee shapes). + +## External blockers (cannot be done from the repo) + +These gate a *working, verified* migration and are outside code: +- **Google OAuth client credentials** (`GOOGLE_CLIENT_ID` / `GOOGLE_CLIENT_SECRET`) in a + runnable/deployed backend — needed to exercise any live OAuth flow. +- **A Google-connected test account** to verify the Calendar/Gmail read path end to end. +- **Google Cloud consent-screen scope config**: adding `gmail.readonly` (restricted) and + the Calendar scope to the app's OAuth consent screen. +- **Gmail restricted-scope verification / CASA** — a Google-side review measured in weeks. + +Implication: backend/desktop code can be written with hermetic (mocked-Google) tests, +but the live OAuth path cannot be exercised until the above are in place. Do not mark +the feature "done" on hermetic tests alone.