Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 64 additions & 4 deletions Sources/CodexBarCore/BrowserCookieAccessGate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,7 @@ public enum BrowserCookieAccessGate {
guard browser.usesKeychainForCookieDecryption else { return true }
guard !KeychainAccessGate.isDisabled else { return false }
guard ProviderInteractionContext.current == .userInitiated else {
self.log.info(
"Skipping background Chromium cookie import to avoid a Keychain prompt",
metadata: ["browser": browser.displayName])
return false
return self.shouldAttemptInBackground(browser, now: now)
}
if self.deniedBrowsersForTesting?.contains(browser) == true {
return self.isExplicitRetryAllowed(for: browser)
Expand Down Expand Up @@ -284,6 +281,69 @@ public enum BrowserCookieAccessGate {
return false
}

/// Background (non-user-initiated) refreshes must never surface a Keychain prompt. Rather than
/// skipping unconditionally, reuse the strictly no-UI Safe Storage preflight: when the ACL already
/// grants access (an explicit `.allowed`), a scheduled refresh can read cookies without any prompt,
/// so background usage stays in sync instead of only updating when the menu is opened. Anything else
/// — interaction required, not found, or a query failure — keeps the no-surprise boundary and skips.
/// An active per-browser or Chromium-family denial cooldown is still honored.
private static func shouldAttemptInBackground(_ browser: Browser, now: Date) -> Bool {
if self.deniedBrowsersForTesting?.contains(browser) == true {
return false
}
if self.hasActiveDenialCooldown(for: browser, now: now) {
self.log.debug(
"Skipping background Chromium cookie import; denial cooldown active",
metadata: ["browser": browser.displayName])
return false
}
guard self.chromiumKeychainAccessIsAllowed(for: browser) else {
self.log.info(
"Skipping background Chromium cookie import to avoid a Keychain prompt",
metadata: ["browser": browser.displayName])
return false
}
self.log.debug(
"Background Chromium cookie import allowed by no-UI Keychain preflight",
metadata: ["browser": browser.displayName])
return true
}

/// Read-only check for an active per-browser or Chromium-family denial cooldown. Mirrors the
/// suppression window enforced on the user-initiated path without mutating persisted state, so a
/// scheduled refresh stays side-effect free.
private static func hasActiveDenialCooldown(for browser: Browser, now: Date) -> Bool {
self.lock.withLock { state in
self.loadIfNeeded(&state)
if let blockedUntil = state.deniedUntilByBrowser[browser.rawValue], blockedUntil > now {
return true
}
if let familyBlockedUntil = state.chromiumFamilyDeniedUntil, familyBlockedUntil > now {
return true
}
return false
}
}

/// Returns true only when the no-UI Safe Storage preflight explicitly reports `.allowed` for one of
/// the browser's labels before any label requires interaction. Symmetric with
/// `chromiumKeychainRequiresInteraction`; `.notFound`/`.failure` are skipped and never treated as a
/// grant, so only an already-authorized ACL enables a background read.
private static func chromiumKeychainAccessIsAllowed(for browser: Browser) -> Bool {
let labels = browser.safeStorageLabels.isEmpty ? self.safeStorageLabels : browser.safeStorageLabels
for label in labels {
switch KeychainAccessPreflight.checkGenericPassword(service: label.service, account: label.account) {
case .allowed:
return true
case .interactionRequired:
return false
case .notFound, .failure:
continue
}
}
return false
}

private static let safeStorageLabels: [(service: String, account: String)] = Browser.safeStorageLabels

private static func normalizedPath(_ url: URL) -> String {
Expand Down
109 changes: 109 additions & 0 deletions Tests/CodexBarTests/BrowserCookieAccessGateTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import Foundation
import Testing
@testable import CodexBarCore

#if os(macOS)
import SweetCookieKit

/// Covers the background (non-user-initiated) branch of `BrowserCookieAccessGate.shouldAttempt`.
/// Before the preflight was consulted, any scheduled refresh returned early, so an already-authorized
/// Safe Storage entry could only refresh when the menu was opened. These tests drive the gate through
/// the strictly no-UI preflight override, never touching the real Keychain.
///
/// Serialized because the gate persists denial cooldowns in process-global state that
/// `resetForTesting()` clears — parallel cases would clobber each other's setup.
@Suite(.serialized)
struct BrowserCookieAccessGateTests {
/// Evaluate `shouldAttempt` with the Keychain enabled, a stubbed no-UI preflight outcome, and an
/// explicit interaction context — the exact three seams the production gate reads.
private func evaluate(
_ browser: Browser,
preflight: KeychainAccessPreflight.Outcome,
interaction: ProviderInteraction,
now: Date = Date()) -> Bool
{
var result = false
KeychainAccessGate.withTaskOverrideForTesting(false) {
ProviderInteractionContext.$current.withValue(interaction) {
KeychainAccessPreflight.withCheckGenericPasswordOverrideForTesting { _, _ in preflight } operation: {
result = BrowserCookieAccessGate.shouldAttempt(browser, now: now)
}
}
}
return result
}

@Test
func `background refresh proceeds when the no-UI preflight already grants access`() {
BrowserCookieAccessGate.resetForTesting()
defer { BrowserCookieAccessGate.resetForTesting() }

#expect(self.evaluate(.chrome, preflight: .allowed, interaction: .background))
}

@Test
func `background refresh is skipped when the preflight requires interaction`() {
BrowserCookieAccessGate.resetForTesting()
defer { BrowserCookieAccessGate.resetForTesting() }

// An interaction-required ACL would surface a Keychain prompt, so a scheduled refresh must not
// attempt the read — preserving the no-surprise boundary.
#expect(self.evaluate(.chrome, preflight: .interactionRequired, interaction: .background) == false)
}

@Test
func `background refresh is skipped when the preflight cannot confirm access`() {
BrowserCookieAccessGate.resetForTesting()
defer { BrowserCookieAccessGate.resetForTesting() }

// Only an explicit `.allowed` enables a background read. A missing item (or a query failure)
// is never optimistically treated as a grant the way the user-initiated path allows.
#expect(self.evaluate(.chrome, preflight: .notFound, interaction: .background) == false)
#expect(self.evaluate(.chrome, preflight: .failure(-25293), interaction: .background) == false)
}

@Test
func `an active denial cooldown suppresses background refresh even when the ACL is allowed`() {
BrowserCookieAccessGate.resetForTesting()
defer { BrowserCookieAccessGate.resetForTesting() }

let start = Date(timeIntervalSince1970: 2000)
BrowserCookieAccessGate.recordDenied(for: .chrome, now: start)

// Within the six-hour cooldown the background gate stays suppressed regardless of a now-allowed
// preflight, matching the suppression the user-initiated path honors.
#expect(self.evaluate(
.chrome,
preflight: .allowed,
interaction: .background,
now: start.addingTimeInterval(60)) == false)

// Once the six-hour cooldown lapses, an allowed preflight lets the background refresh proceed
// again.
let sixHours: TimeInterval = 6 * 60 * 60
#expect(self.evaluate(
.chrome,
preflight: .allowed,
interaction: .background,
now: start.addingTimeInterval(sixHours + 60)))
}

@Test
func `user initiated refresh is unchanged and still proceeds on an allowed preflight`() {
BrowserCookieAccessGate.resetForTesting()
defer { BrowserCookieAccessGate.resetForTesting() }

#expect(self.evaluate(.chrome, preflight: .allowed, interaction: .userInitiated))
}

@Test
func `browsers that do not use the Keychain are unaffected by the background gate`() {
BrowserCookieAccessGate.resetForTesting()
defer { BrowserCookieAccessGate.resetForTesting() }

// Safari does not decrypt cookies via the Keychain, so the gate short-circuits to true before
// any preflight or interaction check — in the background just as in a user-initiated refresh.
#expect(self.evaluate(.safari, preflight: .interactionRequired, interaction: .background))
}
}
#endif
17 changes: 12 additions & 5 deletions Tests/CodexBarTests/BrowserDetectionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ struct BrowserDetectionTests {
}

@Test
func `background cookie import skips chromium before keychain preflight`() {
func `background chromium refresh proceeds when the no-UI preflight already grants access`() {
BrowserCookieAccessGate.resetForTesting()
defer { BrowserCookieAccessGate.resetForTesting() }

Expand All @@ -282,17 +282,21 @@ struct BrowserDetectionTests {
return .allowed
} operation: {
ProviderInteractionContext.$current.withValue(.background) {
#expect(BrowserCookieAccessGate.shouldAttempt(.chrome) == false)
// An already-authorized Safe Storage ACL lets a scheduled refresh read cookies
// without any prompt, so the gate now consults the strictly no-UI preflight instead
// of skipping outright.
#expect(BrowserCookieAccessGate.shouldAttempt(.chrome) == true)
#expect(BrowserCookieAccessGate.shouldAttempt(.safari) == true)
}
}
}

#expect(preflightCount == 0)
// Only the Keychain-backed browser reaches the preflight; Safari short-circuits before it.
#expect(preflightCount == 1)
}

@Test
func `background cookie import skips chromium without probing keychain interaction`() {
func `background chromium refresh skips when the no-UI preflight requires interaction`() {
BrowserCookieAccessGate.resetForTesting()
defer { BrowserCookieAccessGate.resetForTesting() }

Expand All @@ -304,13 +308,16 @@ struct BrowserDetectionTests {
return .interactionRequired
} operation: {
ProviderInteractionContext.$current.withValue(.background) {
// The no-UI preflight never prompts; when it reports interaction is required the
// background refresh still skips, preserving the no-surprise-prompt boundary.
#expect(BrowserCookieAccessGate.shouldAttempt(.chrome) == false)
#expect(BrowserCookieAccessGate.shouldAttempt(.safari) == true)
}
}
}

#expect(preflightCount == 0)
// The gate must probe the no-UI preflight to learn that interaction is required.
#expect(preflightCount == 1)
}

@Test
Expand Down