diff --git a/apps/desktop/electron/native-token-store.test.ts b/apps/desktop/electron/native-token-store.test.ts index bf22b4cbde0..12eff080f9f 100644 --- a/apps/desktop/electron/native-token-store.test.ts +++ b/apps/desktop/electron/native-token-store.test.ts @@ -357,3 +357,67 @@ test('an encrypt that returns null is refused rather than blanking the stored en // ...and the original token set still loads, refresh token intact. assert.deepEqual(loadNativeTokenSet(GATEWAY, createFakeDisk(before).io), TOKENS) }) + +// --- credential redaction in logs --- +// +// normalizeRemoteBaseUrl() strips query/fragment/trailing slashes but not +// userinfo, so a configured gateway URL can carry `user:password@` into this +// store. It must stay intact as the store KEY and never reach a log line. + +const CRED_GATEWAY = 'https://alice:supersecret@gw.example.com/hermes' + +test('a decryption failure logs the gateway host and path but not its credentials', () => { + const first = createFakeDisk() + + persistNativeTokenSet(CRED_GATEWAY, TOKENS, first.io) + + const before = first.fileText() + const locked = createFakeDisk(before, { decrypt: () => '' }) + + assert.equal(loadNativeTokenSet(CRED_GATEWAY, locked.io), null) + // Still identifies which gateway failed... + assert.match(locked.logs[0], /failed to decrypt stored tokens for https:\/\/gw\.example\.com\/hermes/) + assert.match(locked.logs[0], /keeping stored entry for retry/) + // ...without the userinfo. + assert.doesNotMatch(locked.logs[0], /alice/) + assert.doesNotMatch(locked.logs[0], /supersecret/) + // Redaction is log-only: the entry stays under the credential-bearing key. + assert.ok(JSON.parse(locked.fileText()!)[CRED_GATEWAY]) + assert.equal(locked.fileText(), before) +}) + +test('a parsing failure logs the gateway host and path but not its credentials', () => { + const disk = createFakeDisk(JSON.stringify({ [CRED_GATEWAY]: { encoding: 'safeStorage', value: 'bm90LWpzb24=' } })) + + assert.equal(loadNativeTokenSet(CRED_GATEWAY, disk.io), null) + assert.match(disk.logs[0], /failed to load stored tokens for https:\/\/gw\.example\.com\/hermes/) + assert.doesNotMatch(disk.logs[0], /alice/) + assert.doesNotMatch(disk.logs[0], /supersecret/) +}) + +test('the credential-bearing base URL stays the exact store key', () => { + const first = createFakeDisk() + + persistNativeTokenSet(CRED_GATEWAY, TOKENS, first.io) + + assert.deepEqual(Object.keys(JSON.parse(first.fileText()!)), [CRED_GATEWAY]) + // The original key still round-trips a full set after a restart. + assert.deepEqual(loadNativeTokenSet(CRED_GATEWAY, createFakeDisk(first.fileText()).io), TOKENS) + // The redacted form is a log string, never a lookup key. + assert.equal(loadNativeTokenSet('https://gw.example.com/hermes', createFakeDisk(first.fileText()).io), null) +}) + +test('an unparseable gateway URL logs a fixed placeholder rather than the raw value', () => { + // A space makes this unparseable by URL, so redaction cannot fall back to + // echoing the input — that would leak the very credentials it guards. + const invalid = 'ht tp://alice:supersecret@gw.example.com' + + const disk = createFakeDisk(JSON.stringify({ [invalid]: { encoding: 'safeStorage', value: 'AAAA' } }), { + decrypt: () => '' + }) + + assert.equal(loadNativeTokenSet(invalid, disk.io), null) + assert.match(disk.logs[0], //) + assert.doesNotMatch(disk.logs[0], /alice/) + assert.doesNotMatch(disk.logs[0], /supersecret/) +}) diff --git a/apps/desktop/electron/native-token-store.ts b/apps/desktop/electron/native-token-store.ts index b99c0aa9481..5506ae1c5fb 100644 --- a/apps/desktop/electron/native-token-store.ts +++ b/apps/desktop/electron/native-token-store.ts @@ -66,6 +66,31 @@ function readStore(io: NativeTokenStoreIo): Record { } } +/** + * A gateway URL safe to write into a log line. + * + * normalizeRemoteBaseUrl() strips query, fragment, and trailing slashes but + * NOT userinfo, so a configured gateway can legitimately carry + * `user:password@` all the way down to this store. Interpolating that into a + * failure log would spill the credentials into the desktop log file, so drop + * the userinfo and keep only what makes the line useful — scheme, host, port, + * path. A value URL cannot parse never falls back to the raw input: echoing it + * is the exact leak this guards against. + */ +function redactGatewayUrl(baseUrl: string): string { + try { + const parsed = new URL(baseUrl) + + parsed.username = '' + parsed.password = '' + + // `host` already carries a non-default port. + return `${parsed.protocol}//${parsed.host}${parsed.pathname}` + } catch { + return '' + } +} + /** * Write (or, with `tokens === null`, drop) one gateway's token set, merging * into whatever other gateways are already stored. @@ -108,6 +133,7 @@ export function persistNativeTokenSet(baseUrl: string, tokens: NativeTokenSet | * does not parse — never a partially-populated set. */ export function loadNativeTokenSet(baseUrl: string, io: NativeTokenStoreIo): NativeTokenSet | null { + // The UNREDACTED url is the store key — redaction is for logs only. const secret = readStore(io)[baseUrl] if (!secret) { @@ -120,7 +146,9 @@ export function loadNativeTokenSet(baseUrl: string, io: NativeTokenStoreIo): Nat if (!plaintext) { // A keychain that is merely locked/unavailable right now must not cost // the user their refresh token — leave the entry for the next attempt. - io.rememberLog?.(`[native-oauth] failed to decrypt stored tokens for ${baseUrl}; keeping stored entry for retry`) + io.rememberLog?.( + `[native-oauth] failed to decrypt stored tokens for ${redactGatewayUrl(baseUrl)}; keeping stored entry for retry` + ) return null } @@ -130,7 +158,7 @@ export function loadNativeTokenSet(baseUrl: string, io: NativeTokenStoreIo): Nat } catch (error) { const detail = error instanceof Error ? error.message : String(error) - io.rememberLog?.(`[native-oauth] failed to load stored tokens for ${baseUrl}: ${detail}`) + io.rememberLog?.(`[native-oauth] failed to load stored tokens for ${redactGatewayUrl(baseUrl)}: ${detail}`) return null }