fix(secrets): stop telling Linux users to install a keyring they already run (#24013)

This commit is contained in:
Neil
2026-09-29 23:01:06 -07:00
committed by GitHub
parent dcaef9dee5
commit b29947d585
2 changed files with 95 additions and 12 deletions
@@ -18,6 +18,24 @@ describe('ElectronSecretStore', () => {
safeStorageMock.getSelectedStorageBackend.mockReturnValue('gnome_libsecret')
})
function withDesktop<T>(desktop: string | undefined, run: () => T): T {
const original = process.env.XDG_CURRENT_DESKTOP
if (desktop === undefined) {
delete process.env.XDG_CURRENT_DESKTOP
} else {
process.env.XDG_CURRENT_DESKTOP = desktop
}
try {
return run()
} finally {
if (original === undefined) {
delete process.env.XDG_CURRENT_DESKTOP
} else {
process.env.XDG_CURRENT_DESKTOP = original
}
}
}
function withPlatform<T>(platform: NodeJS.Platform, run: () => T): T {
const original = process.platform
Object.defineProperty(process, 'platform', { configurable: true, value: platform })
@@ -100,6 +118,52 @@ describe('ElectronSecretStore', () => {
expect(new ElectronSecretStore().describeProtectionGap()).toMatch(/keyring is unavailable/)
})
})
// Why these three: on Hyprland/sway/river/niri the desktop is unrecognised, so the
// backend is basic_text AND sealing is unavailable — and the old text told those users
// to install a keyring that gnome-keyring was already serving the whole time.
it('does not blame a missing keyring when the desktop was simply not recognised', () => {
safeStorageMock.isEncryptionAvailable.mockReturnValue(false)
safeStorageMock.getSelectedStorageBackend.mockReturnValue('basic_text')
withPlatform('linux', () => {
const gap = new ElectronSecretStore().describeProtectionGap()
expect(gap).toMatch(/could not tell which keyring service/)
expect(gap).toMatch(/already running/)
expect(gap).not.toMatch(/Install and unlock/)
})
})
it('names the desktop in the unrecognised-desktop gap, so support can act on the log', () => {
safeStorageMock.isEncryptionAvailable.mockReturnValue(false)
safeStorageMock.getSelectedStorageBackend.mockReturnValue('basic_text')
withDesktop('Hyprland', () => {
withPlatform('linux', () => {
expect(new ElectronSecretStore().describeProtectionGap()).toContain(
'XDG_CURRENT_DESKTOP=Hyprland'
)
})
})
})
it('omits the parenthetical when no desktop is set rather than printing an empty one', () => {
safeStorageMock.isEncryptionAvailable.mockReturnValue(false)
safeStorageMock.getSelectedStorageBackend.mockReturnValue('basic_text')
withDesktop(undefined, () => {
withPlatform('linux', () => {
const gap = new ElectronSecretStore().describeProtectionGap()
expect(gap).not.toMatch(/XDG_CURRENT_DESKTOP/)
expect(gap).toContain('this desktop uses, so secrets')
})
})
})
it('still blames the keyring when a real backend was selected but cannot seal', () => {
safeStorageMock.isEncryptionAvailable.mockReturnValue(false)
safeStorageMock.getSelectedStorageBackend.mockReturnValue('gnome_libsecret')
withPlatform('linux', () => {
expect(new ElectronSecretStore().describeProtectionGap()).toMatch(/Install and unlock/)
})
})
})
// Why this shape: the whole safety argument for the SecretStore refactor is that the
+31 -12
View File
@@ -19,24 +19,41 @@ export class ElectronSecretStore implements SecretStore {
}
describeProtectionGap(): string | null {
// Why availability first: it is the call that actually probes the keyring, so every
// backend read below is free and cannot change which call blocks.
if (!safeStorage.isEncryptionAvailable()) {
// Why platform-specific: the fix differs, and "encryption unavailable" alone
// sends users looking in the wrong place.
return process.platform === 'linux'
? 'The OS keyring is unavailable, so secrets are stored unencrypted. Install and unlock gnome-keyring or kwallet to seal them.'
: 'The OS keychain is unavailable, so secrets are stored unencrypted.'
if (process.platform !== 'linux') {
return 'The OS keychain is unavailable, so secrets are stored unencrypted.'
}
return readLinuxBackend() === 'basic_text'
? // Chromium picks the backend from XDG_CURRENT_DESKTOP and recognises none of the
// tiling compositors, so it never asks the secret service that is usually running
// the whole time. Telling these users to install a keyring sends them after one
// they already have.
`Orca could not tell which keyring service this desktop uses${describeDesktop()}, so secrets are stored unencrypted — even if a secret service is already running. Start Orca with --password-store=gnome-libsecret (or --password-store=kwallet6 on KDE) to name one.`
: 'The OS keyring is unavailable, so secrets are stored unencrypted. Install and unlock gnome-keyring or kwallet to seal them.'
}
// Why this is not folded into isEncryptionAvailable(): on Linux with no keyring,
// Electron falls back to `basic_text`, which "encrypts" with a hardcoded password.
// It round-trips, so sealing and unsealing genuinely work and must keep working —
// reporting it unavailable would strand every credential already stored this way.
// But it protects nothing, and reporting it as sealed is the actual lie.
// Why this is not folded into isEncryptionAvailable(): `basic_text` "encrypts" with a
// hardcoded password, and Electron makes that key available only after an explicit
// `setUsePlainTextEncryption(true)` (or `--password-store=basic`) — which this app never
// asks for, so the branch is currently unreachable on Linux and stays for the day it is
// not. Where it does apply, sealing round-trips and must keep working: reporting it
// unavailable would strand every credential already stored that way. But it protects
// nothing, and reporting it as sealed is the actual lie.
return describeLinuxBackendGap()
}
}
/** The active desktop, as a parenthetical for support triage, or '' when unset. */
function describeDesktop(): string {
const desktop = process.env.XDG_CURRENT_DESKTOP?.trim()
return desktop ? ` (XDG_CURRENT_DESKTOP=${desktop})` : ''
}
// Electron omits getSelectedStorageBackend at runtime outside Linux despite its type declaration.
function describeLinuxBackendGap(): string | null {
function readLinuxBackend(): string | null {
if (process.platform !== 'linux') {
return null
}
@@ -44,13 +61,15 @@ function describeLinuxBackendGap(): string | null {
if (typeof probe !== 'function') {
return null
}
let backend: string
try {
backend = probe.call(safeStorage)
return probe.call(safeStorage)
} catch {
return null
}
return backend === 'basic_text'
}
function describeLinuxBackendGap(): string | null {
return readLinuxBackend() === 'basic_text'
? 'Secrets are obfuscated with a built-in key, not protected by the OS keyring. Install and unlock gnome-keyring or kwallet, then restart Orca, to seal them properly.'
: null
}