From ba87908a7ae04a7d996e4eeb9589356a00dba7fb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 03:17:30 -0700 Subject: [PATCH] fix(startup): stop the profile load's keyring probe from gating the first window --- src/main/index.ts | 23 +- .../deferred-secret-hydration.test.ts | 229 ++++++++++++++++++ .../deferred-secret-hydration.ts | 89 +++++++ .../loading-store/loaded-state-parsing.ts | 71 ++---- .../protected-secret-decoding.ts | 93 +++++++ .../loading-store/store-runtime-state.ts | 10 + src/main/persistence/loading-store/store.ts | 26 ++ src/main/protected-secret-persistence.ts | 30 +++ .../defer-until-first-window-shown.test.ts | 113 +++++++++ .../window/defer-until-first-window-shown.ts | 41 ++++ 10 files changed, 669 insertions(+), 56 deletions(-) create mode 100644 src/main/persistence/loading-store/deferred-secret-hydration.test.ts create mode 100644 src/main/persistence/loading-store/deferred-secret-hydration.ts create mode 100644 src/main/persistence/loading-store/protected-secret-decoding.ts create mode 100644 src/main/window/defer-until-first-window-shown.test.ts create mode 100644 src/main/window/defer-until-first-window-shown.ts diff --git a/src/main/index.ts b/src/main/index.ts index 79f3f53f3a5..2d381070bcd 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -38,6 +38,7 @@ import { setSecretStore } from '../shared/secret-store' import { ElectronSecretStore } from './host/electron-secret-store' import { reportSecretProtectionGap } from './host/secret-protection-report' import { initSessionParseCachePersistence } from './ai-vault/session-parse-cache-persistence' +import { deferUntilFirstWindowShown } from './window/defer-until-first-window-shown' import { ensureActiveOrcaProfile, initOrcaProfilePaths } from './orca-profiles/profile-index-store' import { getOrcaCloudAuthConfig } from './orca-profiles/profile-cloud-auth-config' import { getProfileUserDataPath } from './orca-profiles/profile-storage-paths' @@ -2342,9 +2343,15 @@ void app.whenReady().then(async () => { // Why this early: the first window stamps the hosting id into its renderer's argv, so the durable // read has to have happened by then or the renderer and the browser-host lease disagree. initializeBrowserClientHostId(activeOrcaProfile.profileDirectory) + // Why deferred: the load decrypts every populated protected slot, and each decrypt probes the + // OS keyring — on Linux a blocking D-Bus round trip that a locked keyring never answers, run + // here with no window on screen to explain the wait (STA-5782). Serve is excluded: it opens no + // window, so deferring would move the block past the point clients can already pair. + const deferKeyringProbe = process.platform === 'linux' && !isServeMode store = new Store({ dataFile: activeOrcaProfile.dataFile, - storageAuthority: isServeMode ? 'runtime' : 'desktop' + storageAuthority: isServeMode ? 'runtime' : 'desktop', + deferKeyringProbe }) // Why here and not at install time: the report remembers what it last said, and that // state lives beside the profile data file, which does not exist until now. @@ -2445,6 +2452,20 @@ void app.whenReady().then(async () => { } catch { console.warn('[proxy] Failed to apply network proxy settings') } + if (deferKeyringProbe) { + const deferredSecretStore = store + deferUntilFirstWindowShown(() => { + const hydration = deferredSecretStore.hydrateDeferredProtectedSecrets() + // Why re-apply here and not via onSettingsChanged: the proxy is applied once at startup + // from settings, not from a listener, so a deferred load leaves it on the environment + // fallback for the rest of the session. + if ('httpProxyUrl' in hydration.settingsUpdates) { + void applyElectronProxySettings(deferredSecretStore.getSettings()).catch(() => { + console.warn('[proxy] Failed to apply network proxy settings after secret hydration') + }) + } + }) + } // Why: the preview session is protocol-scoped, so the handler must exist before any preview webview attaches. installDocPreviewProtocolHandler() registerDocPreviewGrantHandlers() diff --git a/src/main/persistence/loading-store/deferred-secret-hydration.test.ts b/src/main/persistence/loading-store/deferred-secret-hydration.test.ts new file mode 100644 index 00000000000..dfb4d96d48e --- /dev/null +++ b/src/main/persistence/loading-store/deferred-secret-hydration.test.ts @@ -0,0 +1,229 @@ +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { _resetSecretStoreForTests, setSecretStore } from '../../../shared/secret-store' +import type { GlobalSettings } from '../../../shared/global-settings-types' +import { dataFile, testState, writeDataFile } from '../../persistence-test-harness' +import { installFakeAppEnvironment } from '../../../../config/scripts/vitest-host-ports-setup' +import { initDataPath } from './user-data-path' +import { Store } from './store' + +vi.mock('electron', () => ({ app: { getPath: () => testState.dir } })) +vi.mock('../../telemetry/client', () => ({ track: vi.fn() })) +vi.mock('../../telemetry/cohort-classifier', () => ({ getCohortAtEmit: () => ({}) })) +vi.mock('../../ssh/ssh-config-parser', () => ({ + loadUserSshConfig: vi.fn(() => ({ hosts: [] })), + sshConfigHostsToTargets: vi.fn(() => []) +})) + +/** + * How long the fake keyring blocks the calling thread. Models the real hazard: on a locked + * Linux keyring, `safeStorage.isEncryptionAvailable()` accepts the call and does not return — + * a fast failure would not reproduce it. Bounded so a regression fails the suite instead of + * wedging the worker, since a synchronous spin cannot be interrupted by a test timeout. + */ +const KEYRING_BLOCK_MS = 2_000 +/** Generous enough to absorb loaded-CI jitter, far below one blocking probe. */ +const NON_BLOCKING_CEILING_MS = 1_000 + +const PROXY_URL = 'http://proxy.example:8080' +const KAGI_LINK = 'https://kagi.com/search?token=abcdef0123456789' +const OWNER_LEASE = '11111111-2222-4333-8444-555555555555' +const COOKIE = 'auth=opencode-session-value' + +function seal(plaintext: string): string { + return Buffer.from(`encrypted:${plaintext}`, 'utf-8').toString('base64') +} + +let probes = 0 + +function installBlockingKeyring(): void { + setSecretStore({ + isEncryptionAvailable: () => { + probes += 1 + // Why a real thread block and not a counter alone: this is what a locked keyring does to + // the main process, and it is the only thing that makes a pre-window probe fatal. + // Why only the first call: OSCrypt resolves the backend once, so the block lands on + // whichever call happens first — which is the whole question this file is about. + if (probes === 1) { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, KEYRING_BLOCK_MS) + } + return true + }, + encryptString: (plainText) => Buffer.from(`encrypted:${plainText}`, 'utf-8'), + decryptString: (cipher) => { + const decoded = cipher.toString('utf-8') + if (!decoded.startsWith('encrypted:')) { + throw new Error('invalid ciphertext') + } + return decoded.slice('encrypted:'.length) + }, + describeProtectionGap: () => null + }) +} + +function writeProfileWithEverySecretSlot(): void { + writeDataFile({ + settings: { httpProxyUrl: seal(PROXY_URL), opencodeSessionCookie: seal(COOKIE) }, + ui: { browserKagiSessionLink: seal(KAGI_LINK) }, + sshPtyConsumerRecoveries: [ + { + targetId: 'target-1', + clientInstanceId: 'client-1', + serverBuildId: 'build-1', + clientGeneration: 1, + ownerGeneration: 1, + ownerLease: seal(OWNER_LEASE) + } + ] + }) +} + +function createStore(options: { deferKeyringProbe?: boolean } = {}): Store { + installFakeAppEnvironment({ getPath: () => testState.dir }) + initDataPath() + return new Store({ dataFile: dataFile(), ...options }) +} + +describe('deferred protected-secret hydration', () => { + beforeEach(() => { + testState.dir = mkdtempSync(join(tmpdir(), 'orca-deferred-secret-')) + probes = 0 + installBlockingKeyring() + }) + + afterEach(() => { + _resetSecretStoreForTests() + rmSync(testState.dir, { recursive: true, force: true }) + }) + + it('never touches the keyring while loading a profile that has populated secret slots', () => { + // Why this is the regression (STA-5782): the Store constructor runs ~1000 lines before the + // first window exists, so one blocking probe here is a window that never appears. + writeProfileWithEverySecretSlot() + + const startedAt = performance.now() + const store = createStore({ deferKeyringProbe: true }) + const elapsedMs = performance.now() - startedAt + + expect(probes).toBe(0) + expect(elapsedMs).toBeLessThan(NON_BLOCKING_CEILING_MS) + // Withheld, not read: a deferred load exposes nothing, exactly as an unavailable keyring does. + expect(store.getSettings().httpProxyUrl).toBe('') + expect(store.getSettings().opencodeSessionCookie).toBe('') + store.freezeWrites() + }) + + it('restores every deferred slot once hydration runs', () => { + writeProfileWithEverySecretSlot() + const store = createStore({ deferKeyringProbe: true }) + + const result = store.hydrateDeferredProtectedSecrets() + + expect(result.hydrated).toBe(true) + expect(probes).toBeGreaterThan(0) + expect(store.getSettings().httpProxyUrl).toBe(PROXY_URL) + expect(store.getSettings().opencodeSessionCookie).toBe(COOKIE) + expect(store.getUI().browserKagiSessionLink).toBe(KAGI_LINK) + expect(store.getSshPtyConsumerRecovery('target-1')?.ownerLease).toBe(OWNER_LEASE) + store.freezeWrites() + }) + + it('tells settings and UI listeners the real values so startup work can re-run against them', () => { + writeProfileWithEverySecretSlot() + const store = createStore({ deferKeyringProbe: true }) + const seen: Partial[] = [] + const seenUi: (string | null | undefined)[] = [] + store.onSettingsChanged((updates) => void seen.push(updates)) + store.onUIChanged((ui) => void seenUi.push(ui.browserKagiSessionLink)) + + const result = store.hydrateDeferredProtectedSecrets() + + expect(result.settingsUpdates.httpProxyUrl).toBe(PROXY_URL) + expect(seen.map((updates) => updates.httpProxyUrl)).toEqual([PROXY_URL]) + expect(result.uiChanged).toBe(true) + expect(seenUi).toEqual([KAGI_LINK]) + store.freezeWrites() + }) + + it('keeps the stored ciphertext byte-for-byte when a save lands before hydration', () => { + // Why: a withheld read is not an empty value. Writing the profile back with the slots blank + // would destroy secrets the user still has, which no failed probe justifies. + writeProfileWithEverySecretSlot() + const before = JSON.parse(readFileSync(dataFile(), 'utf-8')) as { + settings: { httpProxyUrl: string } + } + const store = createStore({ deferKeyringProbe: true }) + + store.updateSettings({ terminalFontSize: 15 }) + store.flushOrThrow() + + const after = JSON.parse(readFileSync(dataFile(), 'utf-8')) as { + settings: { httpProxyUrl: string; opencodeSessionCookie: string } + ui: { browserKagiSessionLink: string } + sshPtyConsumerRecoveries: { ownerLease: string }[] + } + expect(after.settings.httpProxyUrl).toBe(before.settings.httpProxyUrl) + expect(after.settings.opencodeSessionCookie).toBe(seal(COOKIE)) + expect(after.ui.browserKagiSessionLink).toBe(seal(KAGI_LINK)) + expect(after.sshPtyConsumerRecoveries.at(0)?.ownerLease).toBe(seal(OWNER_LEASE)) + store.freezeWrites() + }) + + it('persists the clear when hydration finds an unusable secret, with no explicit flush', async () => { + // Why without a flush: hydration runs long after startup, so nothing else is guaranteed to + // save. Dropping the retained blob in memory only would resurrect the bad value next launch. + writeDataFile({ settings: { httpProxyUrl: seal('!!! not a url !!!') } }) + const store = createStore({ deferKeyringProbe: true }) + // Why flush first: load-time migrations schedule their own save, and a hydration that + // scheduled nothing would still reach disk on the back of it. + store.flushOrThrow() + expect( + (JSON.parse(readFileSync(dataFile(), 'utf-8')) as { settings: { httpProxyUrl: string } }) + .settings.httpProxyUrl + ).toBe(seal('!!! not a url !!!')) + + const result = store.hydrateDeferredProtectedSecrets() + expect(result.needsSave).toBe(true) + await vi.waitFor( + async () => { + await store.waitForPendingWrite() + const saved = JSON.parse(readFileSync(dataFile(), 'utf-8')) as { + settings: { httpProxyUrl: string } + } + expect(saved.settings.httpProxyUrl).toBe('') + }, + { timeout: 10_000, interval: 100 } + ) + store.freezeWrites() + }) + + it('probes inline when the host opens no window, so nothing pairs against a stalled main thread', () => { + // Why pinned: headless serve advertises readiness with no window to defer behind, and an + // earlier deferral moved this block to after clients could already connect (STA-5765). + writeProfileWithEverySecretSlot() + + const store = createStore() + + expect(probes).toBeGreaterThan(0) + expect(store.getSettings().httpProxyUrl).toBe(PROXY_URL) + expect(store.hydrateDeferredProtectedSecrets().hydrated).toBe(false) + store.freezeWrites() + }) + + it('reports nothing to hydrate when the load already probed', () => { + writeProfileWithEverySecretSlot() + const store = createStore() + + const result = store.hydrateDeferredProtectedSecrets() + + expect(result).toEqual({ + hydrated: false, + settingsUpdates: {}, + uiChanged: false, + needsSave: false + }) + store.freezeWrites() + }) +}) diff --git a/src/main/persistence/loading-store/deferred-secret-hydration.ts b/src/main/persistence/loading-store/deferred-secret-hydration.ts new file mode 100644 index 00000000000..51fed139eee --- /dev/null +++ b/src/main/persistence/loading-store/deferred-secret-hydration.ts @@ -0,0 +1,89 @@ +import type { GlobalSettings } from '../../../shared/global-settings-types' +import type { SshPtyConsumerRecovery } from '../../../shared/ssh-types' +import { PROTECTED_SECRET_SLOT } from '../../protected-secret-persistence' +import { + decodeBrowserKagiSessionLink, + decodeHttpProxyUrl, + decodeOpencodeSessionCookie, + decodeSshPtyOwnerLease +} from './protected-secret-decoding' +import type { StoreRuntimeState } from './store-runtime-state' + +type DeferredSecretHydrationRuntime = Pick + +export type DeferredSecretHydrationResult = { + /** False when the load already probed the keyring, so there is nothing left to finish. */ + hydrated: boolean + settingsUpdates: Partial + uiChanged: boolean + /** True when a slot was dropped as unusable, which only reaches disk on the next save. */ + needsSave: boolean +} + +const NOTHING_DEFERRED: DeferredSecretHydrationResult = { + hydrated: false, + settingsUpdates: {}, + uiChanged: false, + needsSave: false +} + +/** + * Finish the protected slots a deferred load left sealed, now that blocking the main thread + * no longer blocks a window from appearing. + * + * Why re-read from the retained ciphertext rather than from state: a deferred load exposes + * nothing, exactly as it does when the keyring is genuinely unavailable, so state holds the + * withheld value and the ciphertext lives only in the retention map. + */ +export function hydrateDeferredProtectedSecrets( + runtime: DeferredSecretHydrationRuntime +): DeferredSecretHydrationResult { + const secrets = runtime.protectedSecrets + if (!secrets.isKeyringProbeDeferred()) { + return NOTHING_DEFERRED + } + secrets.resumeKeyringProbe() + + const settingsUpdates: Partial = {} + let needsSave = false + let uiChanged = false + + const cookieBlob = secrets.retainedBlob(PROTECTED_SECRET_SLOT.opencodeSessionCookie) + if (cookieBlob) { + const cookie = decodeOpencodeSessionCookie(secrets, cookieBlob) + if (cookie !== runtime.state.settings.opencodeSessionCookie) { + runtime.state.settings.opencodeSessionCookie = cookie + settingsUpdates.opencodeSessionCookie = cookie + } + } + + const proxyBlob = secrets.retainedBlob(PROTECTED_SECRET_SLOT.httpProxyUrl) + if (proxyBlob) { + const proxy = decodeHttpProxyUrl(secrets, proxyBlob) + needsSave ||= proxy.cleared + if (proxy.value !== (runtime.state.settings.httpProxyUrl ?? '')) { + runtime.state.settings.httpProxyUrl = proxy.value + settingsUpdates.httpProxyUrl = proxy.value + } + } + + const kagiBlob = secrets.retainedBlob(PROTECTED_SECRET_SLOT.browserKagiSessionLink) + if (kagiBlob) { + const link = decodeBrowserKagiSessionLink(secrets, kagiBlob) + if (link !== (runtime.state.ui.browserKagiSessionLink ?? '')) { + runtime.state.ui.browserKagiSessionLink = link + uiChanged = true + } + } + + const recoveries = runtime.state.sshPtyConsumerRecoveries + if (recoveries && recoveries.length > 0) { + const decoded = recoveries + .map((record) => decodeSshPtyOwnerLease(secrets, record)) + .filter((record): record is SshPtyConsumerRecovery => record !== null) + needsSave ||= decoded.length !== recoveries.length + runtime.state.sshPtyConsumerRecoveries = decoded + } + + return { hydrated: true, settingsUpdates, uiChanged, needsSave } +} diff --git a/src/main/persistence/loading-store/loaded-state-parsing.ts b/src/main/persistence/loading-store/loaded-state-parsing.ts index 2f28c771508..f866e6797f7 100644 --- a/src/main/persistence/loading-store/loaded-state-parsing.ts +++ b/src/main/persistence/loading-store/loaded-state-parsing.ts @@ -1,7 +1,5 @@ import { readFileSync, existsSync } from 'node:fs' import { homedir } from 'node:os' -import { normalizeProxyUrl } from '../../../shared/network-proxy' -import { normalizeKagiSessionLink } from '../../../shared/browser-url' import type { PersistedState } from '../../../shared/persisted-state-types' import type { SshPtyConsumerRecovery } from '../../../shared/ssh-types' import { getDefaultPersistedState } from '../../../shared/constants' @@ -14,13 +12,11 @@ import { logStartupDiagnostic } from '../../startup/startup-diagnostics' import { - PROTECTED_SECRET_SLOT, - sshPtyOwnerLeaseSecretSlot -} from '../../protected-secret-persistence' -import { - isLegacyOpenCodeSessionCookie, - isLegacySshPtyOwnerLease -} from '../leasing-ssh-ptys/secret-validation' + decodeBrowserKagiSessionLink, + decodeHttpProxyUrl, + decodeOpencodeSessionCookie, + decodeSshPtyOwnerLease +} from './protected-secret-decoding' import { readGithubCacheSnapshot } from './user-data-path' import { gcStaleWorktreeMeta, @@ -94,43 +90,24 @@ export class LoadedStateParsingOperations { logPersistenceStartupMilestone('persistence-json-parse-done') // Why: secrets are stored encrypted via safeStorage; decrypt at the load boundary so the app sees plaintext. + const secrets = this.runtime.protectedSecrets if (parsed.settings?.opencodeSessionCookie) { - parsed.settings.opencodeSessionCookie = this.runtime.protectedSecrets.decrypt( - PROTECTED_SECRET_SLOT.opencodeSessionCookie, - parsed.settings.opencodeSessionCookie, - isLegacyOpenCodeSessionCookie + parsed.settings.opencodeSessionCookie = decodeOpencodeSessionCookie( + secrets, + parsed.settings.opencodeSessionCookie ) } if (parsed.settings?.httpProxyUrl) { - const decryptedProxy = this.runtime.protectedSecrets.decryptWithStatus( - PROTECTED_SECRET_SLOT.httpProxyUrl, - parsed.settings.httpProxyUrl, - (value) => normalizeProxyUrl(value).ok - ) - // Why (STA-3442): after a keychain reset decrypt returns raw ciphertext; a non-URL - // value must not masquerade as a configured proxy (silent DIRECT fallback) or - // re-persist as garbage. Plaintext URLs still pass, preserving the upgrade path. - if ( - decryptedProxy.status === 'unavailable' || - (decryptedProxy.status === 'failed' && !decryptedProxy.plaintext) - ) { - parsed.settings.httpProxyUrl = '' - } else if (normalizeProxyUrl(decryptedProxy.plaintext).ok) { - parsed.settings.httpProxyUrl = decryptedProxy.plaintext - } else { - console.warn( - '[persistence] httpProxyUrl could not be decrypted — clearing the stored proxy URL. Re-enter it in Settings > Advanced > Network.' - ) - parsed.settings.httpProxyUrl = '' - this.runtime.protectedSecrets.removeRetainedBlob(PROTECTED_SECRET_SLOT.httpProxyUrl) + const decodedProxy = decodeHttpProxyUrl(secrets, parsed.settings.httpProxyUrl) + parsed.settings.httpProxyUrl = decodedProxy.value + if (decodedProxy.cleared) { this.runtime.loadNeedsSave = true } } if (parsed.ui?.browserKagiSessionLink) { - parsed.ui.browserKagiSessionLink = this.runtime.protectedSecrets.decrypt( - PROTECTED_SECRET_SLOT.browserKagiSessionLink, - parsed.ui.browserKagiSessionLink, - (value) => normalizeKagiSessionLink(value) !== null + parsed.ui.browserKagiSessionLink = decodeBrowserKagiSessionLink( + secrets, + parsed.ui.browserKagiSessionLink ) } parsed.sshPtyConsumerRecoveries = ( @@ -140,23 +117,7 @@ export class LoadedStateParsingOperations { normalizeSshPtyConsumerRecovery(record, ENCRYPTED_SSH_PTY_OWNER_LEASE_MAX_LENGTH) ) .filter((record): record is SshPtyConsumerRecovery => record !== null) - .map((record) => { - const slot = sshPtyOwnerLeaseSecretSlot(record.targetId) - const decrypted = this.runtime.protectedSecrets.decryptWithStatus( - slot, - record.ownerLease, - isLegacySshPtyOwnerLease - ) - const normalized = - decrypted.status === 'unavailable' || - (decrypted.status === 'failed' && !decrypted.plaintext) - ? record - : normalizeSshPtyConsumerRecovery({ ...record, ownerLease: decrypted.plaintext }) - if (!normalized) { - this.runtime.protectedSecrets.removeRetainedBlob(slot) - } - return normalized - }) + .map((record) => decodeSshPtyOwnerLease(secrets, record)) .filter((record): record is SshPtyConsumerRecovery => record !== null) const terminalSettings = prepareLoadedTerminalSettings(parsed, () => { diff --git a/src/main/persistence/loading-store/protected-secret-decoding.ts b/src/main/persistence/loading-store/protected-secret-decoding.ts new file mode 100644 index 00000000000..87660d84f92 --- /dev/null +++ b/src/main/persistence/loading-store/protected-secret-decoding.ts @@ -0,0 +1,93 @@ +import { normalizeProxyUrl } from '../../../shared/network-proxy' +import { normalizeKagiSessionLink } from '../../../shared/browser-url' +import type { SshPtyConsumerRecovery } from '../../../shared/ssh-types' +import { + PROTECTED_SECRET_SLOT, + sshPtyOwnerLeaseSecretSlot, + type ProtectedSecretPersistence +} from '../../protected-secret-persistence' +import { + isLegacyOpenCodeSessionCookie, + isLegacySshPtyOwnerLease +} from '../leasing-ssh-ptys/secret-validation' +import { normalizeSshPtyConsumerRecovery } from '../leasing-ssh-ptys/ssh-normalization' + +/** + * The per-slot decode rules, shared by the load boundary and the deferred hydration that + * finishes the slots the load skipped. Why shared and not duplicated: the two paths must + * agree on what an undecryptable value means, and a second copy would drift silently. + */ +export type ProtectedSecretDecoder = Pick< + ProtectedSecretPersistence, + 'decrypt' | 'decryptWithStatus' | 'removeRetainedBlob' +> + +export function decodeOpencodeSessionCookie( + secrets: ProtectedSecretDecoder, + ciphertext: string +): string { + return secrets.decrypt( + PROTECTED_SECRET_SLOT.opencodeSessionCookie, + ciphertext, + isLegacyOpenCodeSessionCookie + ) +} + +export function decodeBrowserKagiSessionLink( + secrets: ProtectedSecretDecoder, + ciphertext: string +): string { + return secrets.decrypt( + PROTECTED_SECRET_SLOT.browserKagiSessionLink, + ciphertext, + (value) => normalizeKagiSessionLink(value) !== null + ) +} + +/** `cleared` means the retained ciphertext was dropped, so the caller must persist the clear. */ +export function decodeHttpProxyUrl( + secrets: ProtectedSecretDecoder, + ciphertext: string +): { value: string; cleared: boolean } { + const decrypted = secrets.decryptWithStatus( + PROTECTED_SECRET_SLOT.httpProxyUrl, + ciphertext, + (value) => normalizeProxyUrl(value).ok + ) + // Why (STA-3442): after a keychain reset decrypt returns raw ciphertext; a non-URL + // value must not masquerade as a configured proxy (silent DIRECT fallback) or + // re-persist as garbage. Plaintext URLs still pass, preserving the upgrade path. + if ( + decrypted.status === 'unavailable' || + (decrypted.status === 'failed' && !decrypted.plaintext) + ) { + return { value: '', cleared: false } + } + if (normalizeProxyUrl(decrypted.plaintext).ok) { + return { value: decrypted.plaintext, cleared: false } + } + console.warn( + '[persistence] httpProxyUrl could not be decrypted — clearing the stored proxy URL. Re-enter it in Settings > Advanced > Network.' + ) + secrets.removeRetainedBlob(PROTECTED_SECRET_SLOT.httpProxyUrl) + return { value: '', cleared: true } +} + +/** Null drops the record entirely; the caller must filter it out of the recovery list. */ +export function decodeSshPtyOwnerLease( + secrets: ProtectedSecretDecoder, + record: SshPtyConsumerRecovery +): SshPtyConsumerRecovery | null { + const slot = sshPtyOwnerLeaseSecretSlot(record.targetId) + const decrypted = secrets.decryptWithStatus(slot, record.ownerLease, isLegacySshPtyOwnerLease) + // Why the record unchanged on 'unavailable': the lease stays sealed, and replacing it with + // the empty plaintext would discard a live owner claim we can still re-read later. + const normalized = + decrypted.status === 'unavailable' || (decrypted.status === 'failed' && !decrypted.plaintext) + ? record + : normalizeSshPtyConsumerRecovery({ ...record, ownerLease: decrypted.plaintext }) + if (!normalized) { + secrets.removeRetainedBlob(slot) + } + return normalized +} diff --git a/src/main/persistence/loading-store/store-runtime-state.ts b/src/main/persistence/loading-store/store-runtime-state.ts index b14ea8ce3a4..c4915fecbc5 100644 --- a/src/main/persistence/loading-store/store-runtime-state.ts +++ b/src/main/persistence/loading-store/store-runtime-state.ts @@ -23,6 +23,13 @@ import type { export type StoreRuntimeOptions = { dataFile?: string storageAuthority?: AutomationStorageAuthority + /** + * Load without asking the OS keyring, leaving every protected slot sealed until + * `Store.hydrateDeferredProtectedSecrets()` runs. Only for hosts that open a window: the + * probe is a blocking D-Bus round trip on Linux, and the load runs before the first window + * exists (STA-5782). + */ + deferKeyringProbe?: boolean } /** Mutable coordination state shared only with this Store's private collaborators. */ @@ -72,6 +79,9 @@ export class StoreRuntimeState { constructor(options: StoreRuntimeOptions = {}) { this.dataFile = options.dataFile ?? getDataFile() this.storageAuthority = options.storageAuthority ?? 'desktop' + if (options.deferKeyringProbe) { + this.protectedSecrets.deferKeyringProbe() + } this.staleTempCleanup = removeStaleDurableWriteTempFiles(this.dataFile, { minimumAgeMs: STALE_DURABLE_WRITE_TEMP_AGE_MS }) diff --git a/src/main/persistence/loading-store/store.ts b/src/main/persistence/loading-store/store.ts index 888c0092ed2..3d235144ad9 100644 --- a/src/main/persistence/loading-store/store.ts +++ b/src/main/persistence/loading-store/store.ts @@ -8,6 +8,11 @@ import { ActiveViewPreference } from '../../active-view-preference' import { registerPersistedPaneKeyAlias } from '../restoring-sessions/pane-alias-normalization' import { normalizePersistedPaneIdentityState } from '../restoring-sessions/workspace-pane-normalization' import { StoreRuntimeState, type StoreRuntimeOptions } from './store-runtime-state' +import { + hydrateDeferredProtectedSecrets, + type DeferredSecretHydrationResult +} from './deferred-secret-hydration' +import { notifySettingsChanged, notifyUIChanged } from './profile-preferences' import { createStoreDomains, installStoreDomainContexts, @@ -87,6 +92,27 @@ export class Store { return dirname(this.runtime.dataFile) } + /** + * Decrypt the protected slots a `deferKeyringProbe` load left sealed, then tell listeners. + * + * Returns what changed so the caller can re-run the startup work that read the withheld + * values — the persisted proxy above all, which is applied from settings before any window + * exists and would otherwise stay unset for the whole session. + */ + hydrateDeferredProtectedSecrets(): DeferredSecretHydrationResult { + const result = hydrateDeferredProtectedSecrets(this.runtime) + if (Object.keys(result.settingsUpdates).length > 0) { + notifySettingsChanged(this, result.settingsUpdates) + } + if (result.uiChanged) { + notifyUIChanged(this) + } + if (result.needsSave) { + scheduleSave(this.domains.scheduling) + } + return result + } + freezeWrites(): void { this.runtime.writesFrozen = true if (this.runtime.writeTimer) { diff --git a/src/main/protected-secret-persistence.ts b/src/main/protected-secret-persistence.ts index 8445e67680f..9bcd38c9d64 100644 --- a/src/main/protected-secret-persistence.ts +++ b/src/main/protected-secret-persistence.ts @@ -33,6 +33,33 @@ type ProtectedSecretEncryption = { export class ProtectedSecretPersistence { private readonly retainedBlobs = new Map() private readonly sealedSlots = new Set() + private keyringProbeDeferred = false + + /** + * Answer "keyring unavailable" without asking the keyring, until {@link resumeKeyringProbe}. + * + * Why: on Linux `isEncryptionAvailable()` is a synchronous, blocking D-Bus round trip to + * `org.freedesktop.secrets`, and a keyring that is present but locked with no unlock prompter + * never replies. Run from the store load that precedes the first window, it holds the main + * thread with nothing on screen (STA-5782). Deferring reuses the existing unavailable path, + * which retains every ciphertext rather than clearing it. + */ + deferKeyringProbe(): void { + this.keyringProbeDeferred = true + } + + resumeKeyringProbe(): void { + this.keyringProbeDeferred = false + } + + isKeyringProbeDeferred(): boolean { + return this.keyringProbeDeferred + } + + /** The ciphertext held for a slot whose plaintext was withheld; '' when nothing is retained. */ + retainedBlob(slot: string): string { + return this.retainedBlobs.get(slot) ?? '' + } removeRetainedBlob(slot: string): void { this.retainedBlobs.delete(slot) @@ -129,6 +156,9 @@ export class ProtectedSecretPersistence { } private encryptionAvailable(): boolean { + if (this.keyringProbeDeferred) { + return false + } // Why getSecretStore() sits outside the try: an uninstalled store is a startup bug, // not a keyring failure. Swallowing it would degrade to an empty blob and report // 'unavailable' — the silent-wrong-state outcome the port throws to prevent. Only diff --git a/src/main/window/defer-until-first-window-shown.test.ts b/src/main/window/defer-until-first-window-shown.test.ts new file mode 100644 index 00000000000..1468618892d --- /dev/null +++ b/src/main/window/defer-until-first-window-shown.test.ts @@ -0,0 +1,113 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +type Listener = (...args: unknown[]) => void + +const appListeners = new Map() + +vi.mock('electron', () => ({ + app: { + once: (event: string, listener: Listener) => { + const existing = appListeners.get(event) ?? [] + existing.push(listener) + appListeners.set(event, existing) + } + } +})) + +const { deferUntilFirstWindowShown, FIRST_WINDOW_SHOWN_FALLBACK_MS } = + await import('./defer-until-first-window-shown') + +/** Stands in for the BrowserWindow the app creates first. */ +function createWindow(): { reveal: () => void } { + const revealListeners: Listener[] = [] + const window = { + once: (event: string, listener: Listener) => { + if (event === 'ready-to-show') { + revealListeners.push(listener) + } + } + } + appListeners + .get('browser-window-created') + ?.splice(0) + .forEach((listener) => listener({}, window)) + return { reveal: () => revealListeners.splice(0).forEach((listener) => listener()) } +} + +describe('deferUntilFirstWindowShown', () => { + let runs: number + + beforeEach(() => { + vi.useFakeTimers() + appListeners.clear() + runs = 0 + }) + + afterEach(() => { + vi.useRealTimers() + }) + + const arm = (): void => deferUntilFirstWindowShown(() => void (runs += 1)) + + /** Runs an already-queued setImmediate; the 1ms is slack, not a delay under test. */ + const drain = (): void => void vi.advanceTimersByTime(1) + + it('does not run while the first window is still being created', () => { + arm() + createWindow() + // Why drain: creation alone must arm nothing. Asserting before the queue drains would also + // pass if the task were merely queued off `browser-window-created`, which still lands before + // the window paints. + drain() + expect(runs).toBe(0) + }) + + it('runs once the window is ready to show', () => { + arm() + const window = createWindow() + window.reveal() + // Why: the reveal handler must return before a blocking task runs, or the paint it is + // waiting on is the thing being blocked. + expect(runs).toBe(0) + drain() + expect(runs).toBe(1) + }) + + it('still runs when ready-to-show never fires, so a failed reveal is not silent', () => { + arm() + createWindow() + // Why bracket the wait instead of flushing every timer: an unbounded flush passes for any + // fallback delay, including one long enough to never arrive in a real session. + vi.advanceTimersByTime(FIRST_WINDOW_SHOWN_FALLBACK_MS - 1_000) + drain() + expect(runs).toBe(0) + vi.advanceTimersByTime(1_100) + drain() + expect(runs).toBe(1) + }) + + it('runs once when the window reveals and the fallback would also have elapsed', () => { + arm() + const window = createWindow() + window.reveal() + drain() + expect(vi.getTimerCount()).toBe(0) + vi.advanceTimersByTime(FIRST_WINDOW_SHOWN_FALLBACK_MS * 2) + drain() + expect(runs).toBe(1) + }) + + it('does not run again when the window reveals after the fallback already ran', () => { + // Why this order and not the reverse: a GPU that presents late reveals the window after the + // fallback has run, and a second blocking task would freeze the main thread exactly as the + // user starts interacting. + arm() + const window = createWindow() + vi.advanceTimersByTime(FIRST_WINDOW_SHOWN_FALLBACK_MS + 100) + drain() + expect(runs).toBe(1) + window.reveal() + drain() + expect(runs).toBe(1) + }) +}) diff --git a/src/main/window/defer-until-first-window-shown.ts b/src/main/window/defer-until-first-window-shown.ts new file mode 100644 index 00000000000..a8d87045aed --- /dev/null +++ b/src/main/window/defer-until-first-window-shown.ts @@ -0,0 +1,41 @@ +import { app, type BrowserWindow } from 'electron' + +/** Matches the updater deferral in attach-main-window-services.ts. */ +export const FIRST_WINDOW_SHOWN_FALLBACK_MS = 15_000 + +/** + * Run `task` exactly once, after the first window has painted — never on the pre-window + * startup path, where blocking the main thread means nothing appears at all. + * + * Why a fallback timer as well as the window event: `ready-to-show` never fires when window + * creation itself fails (a GPU or driver fault), and a task armed only on the happy path + * would then never run. The timer is unref'd so it cannot hold the process alive. + * + * Not for headless hosts: with no window, the fallback is the only path, which moves the task + * to after readiness has already been advertised. + */ +export function deferUntilFirstWindowShown( + task: () => void, + options: { fallbackMs?: number } = {} +): void { + let done = false + let fallback: ReturnType | null = null + const runOnce = (): void => { + if (done) { + return + } + done = true + if (fallback) { + clearTimeout(fallback) + fallback = null + } + task() + } + fallback = setTimeout(runOnce, options.fallbackMs ?? FIRST_WINDOW_SHOWN_FALLBACK_MS) + fallback.unref?.() + app.once('browser-window-created', (_event: unknown, window: BrowserWindow) => { + // Why setImmediate: the reveal handler must return before a blocking task runs, or the + // paint it is waiting on is the thing being blocked. + window.once('ready-to-show', () => setImmediate(runOnce)) + }) +}