From 1ad15d6e3af44e1d2c2dbc4200f6a486d5154cba Mon Sep 17 00:00:00 2001 From: Evandro Fonseca Junior Date: Sat, 10 Oct 2026 04:25:50 -0300 Subject: [PATCH] fix(security): keep OAuth tokens out of renderer IPC and compare API keys in constant time The cloud ORPC routes (addGoogleAccount, listCloudAccounts, refreshAccountQuota, syncLocalAccount) returned the full account including access and refresh tokens, but the renderer never reads them. Blank the two secrets in those responses; token metadata (expiry, project id) is kept so the schema holds. Also compare the presented gateway API key with timingSafeEqual. --- src/ipc/cloud/router.ts | 9 ++-- src/server/guards/admin.guard.ts | 9 +++- src/server/guards/api-key-auth.util.ts | 15 ++++++ src/server/modules/proxy/proxy.guard.ts | 3 +- .../unit/cloud-account-redaction.test.ts | 52 +++++++++++++++++++ .../unit/server/api-key-auth.util.test.ts | 16 ++++++ src/utils/cloud-account-redaction.ts | 20 +++++++ 7 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 src/tests/unit/cloud-account-redaction.test.ts create mode 100644 src/utils/cloud-account-redaction.ts diff --git a/src/ipc/cloud/router.ts b/src/ipc/cloud/router.ts index 345d290..924cff9 100644 --- a/src/ipc/cloud/router.ts +++ b/src/ipc/cloud/router.ts @@ -28,6 +28,7 @@ import { CloudAccountRepo } from '../database/cloudHandler'; import { CloudAccountSchema } from '../../types/cloudAccount'; import { DeviceProfileSchema, DeviceProfilesSnapshotSchema } from '../../types/account'; import { logger } from '../../utils/logger'; +import { redactCloudAccountTokens } from '../../utils/cloud-account-redaction'; import { getSwitchMetricsSnapshot } from '../switchMetrics'; import { getSwitchGuardSnapshot } from '../switchGuard'; import { getDeviceHardeningSnapshot } from '../device/handler'; @@ -107,11 +108,11 @@ export const cloudRouter = os.router({ .input(z.object({ authCode: z.string(), oauthClientKey: z.string().optional() })) .output(CloudAccountSchema) .handler(async ({ input }) => { - return addGoogleAccount(input.authCode, input.oauthClientKey); + return redactCloudAccountTokens(await addGoogleAccount(input.authCode, input.oauthClientKey)); }), listCloudAccounts: os.output(z.array(CloudAccountSchema)).handler(async () => { - return listCloudAccounts(); + return (await listCloudAccounts()).map(redactCloudAccountTokens); }), deleteCloudAccount: os @@ -125,7 +126,7 @@ export const cloudRouter = os.router({ .input(z.object({ accountId: z.string() })) .output(CloudAccountSchema) .handler(async ({ input }) => { - return refreshAccountQuota(input.accountId); + return redactCloudAccountTokens(await refreshAccountQuota(input.accountId)); }), switchCloudAccount: os @@ -206,7 +207,7 @@ export const cloudRouter = os.router({ try { const result = await CloudAccountRepo.syncFromIDE(); - return result; + return result ? redactCloudAccountTokens(result) : result; } catch (error: unknown) { logger.error( '[ORPC] syncLocalAccount error:', diff --git a/src/server/guards/admin.guard.ts b/src/server/guards/admin.guard.ts index 40e95c1..4527227 100644 --- a/src/server/guards/admin.guard.ts +++ b/src/server/guards/admin.guard.ts @@ -1,6 +1,11 @@ import { CanActivate, ExecutionContext, Injectable, UnauthorizedException } from '@nestjs/common'; import { getServerConfig } from '../server-config'; -import { extractApiKeyToken, hasConfiguredApiKey, RequestHeaders } from './api-key-auth.util'; +import { + apiKeyMatches, + extractApiKeyToken, + hasConfiguredApiKey, + RequestHeaders, +} from './api-key-auth.util'; @Injectable() export class AdminGuard implements CanActivate { @@ -16,7 +21,7 @@ export class AdminGuard implements CanActivate { const headers = request.headers as RequestHeaders; const clientToken = extractApiKeyToken(headers); - if (clientToken && clientToken === apiKey) { + if (apiKeyMatches(clientToken, apiKey)) { return true; } diff --git a/src/server/guards/api-key-auth.util.ts b/src/server/guards/api-key-auth.util.ts index 6cba6ce..afcb1be 100644 --- a/src/server/guards/api-key-auth.util.ts +++ b/src/server/guards/api-key-auth.util.ts @@ -1,3 +1,4 @@ +import { createHash, timingSafeEqual } from 'crypto'; import { isEmpty, isString } from 'lodash-es'; export type RequestHeaderValue = string | string[] | undefined; @@ -8,6 +9,20 @@ export function hasConfiguredApiKey(apiKey: string | undefined): apiKey is strin return isString(apiKey) && !isEmpty(apiKey.trim()); } +/** + * Compares the presented key with the configured one without leaking, through response time, + * how many leading characters matched. Both values are hashed so the buffers always have the + * same length, which timingSafeEqual requires. + */ +export function apiKeyMatches(clientToken: string | null, apiKey: string): boolean { + if (clientToken === null) { + return false; + } + const presented = createHash('sha256').update(clientToken).digest(); + const expected = createHash('sha256').update(apiKey).digest(); + return timingSafeEqual(presented, expected); +} + export function extractApiKeyToken(headers: RequestHeaders): string | null { const authorizationHeader = readHeaderValue(headers['authorization']); if (authorizationHeader) { diff --git a/src/server/modules/proxy/proxy.guard.ts b/src/server/modules/proxy/proxy.guard.ts index f5d761c..4a22717 100644 --- a/src/server/modules/proxy/proxy.guard.ts +++ b/src/server/modules/proxy/proxy.guard.ts @@ -7,6 +7,7 @@ import { } from '@nestjs/common'; import { getServerConfig } from '../../server-config'; import { + apiKeyMatches, extractApiKeyToken, hasConfiguredApiKey, RequestHeaders, @@ -32,7 +33,7 @@ export class ProxyGuard implements CanActivate { return true; } - if (clientToken === apiKey) { + if (apiKeyMatches(clientToken, apiKey)) { return true; } this.logger.warn(`Rejected unauthorized request from ${request.ip}`); diff --git a/src/tests/unit/cloud-account-redaction.test.ts b/src/tests/unit/cloud-account-redaction.test.ts new file mode 100644 index 0000000..f738eae --- /dev/null +++ b/src/tests/unit/cloud-account-redaction.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from 'vitest'; +import { redactCloudAccountTokens } from '../../utils/cloud-account-redaction'; +import type { CloudAccount } from '../../types/cloudAccount'; + +function makeAccount(): CloudAccount { + return { + id: 'acc-1', + provider: 'google', + email: 'user@example.com', + name: 'User', + token: { + access_token: 'ya29.secret-access', + refresh_token: '1//secret-refresh', + expires_in: 3600, + expiry_timestamp: 1_900_000_000, + token_type: 'Bearer', + project_id: 'project-1', + is_gcp_tos: true, + }, + created_at: 1, + last_used: 2, + }; +} + +describe('redactCloudAccountTokens', () => { + it('blanks the OAuth credentials and keeps the non-secret token metadata', () => { + const redacted = redactCloudAccountTokens(makeAccount()); + expect(redacted.token.access_token).toBe(''); + expect(redacted.token.refresh_token).toBe(''); + expect(redacted.token).toMatchObject({ + expires_in: 3600, + expiry_timestamp: 1_900_000_000, + token_type: 'Bearer', + project_id: 'project-1', + is_gcp_tos: true, + }); + expect(redacted.email).toBe('user@example.com'); + }); + + it('does not mutate the original account', () => { + const account = makeAccount(); + redactCloudAccountTokens(account); + expect(account.token.refresh_token).toBe('1//secret-refresh'); + expect(account.token.access_token).toBe('ya29.secret-access'); + }); + + it('never leaves a credential in the serialized payload', () => { + const payload = JSON.stringify(redactCloudAccountTokens(makeAccount())); + expect(payload).not.toContain('secret-access'); + expect(payload).not.toContain('secret-refresh'); + }); +}); diff --git a/src/tests/unit/server/api-key-auth.util.test.ts b/src/tests/unit/server/api-key-auth.util.test.ts index cfbc528..853f6c1 100644 --- a/src/tests/unit/server/api-key-auth.util.test.ts +++ b/src/tests/unit/server/api-key-auth.util.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest'; import { + apiKeyMatches, hasConfiguredApiKey, extractApiKeyToken, type RequestHeaders, @@ -81,4 +82,19 @@ describe('api-key-auth.util', () => { expect(extractApiKeyToken(headers)).toBeNull(); }); }); + + describe('apiKeyMatches', () => { + it('accepts only the exact configured key', () => { + expect(apiKeyMatches('secret-key', 'secret-key')).toBe(true); + expect(apiKeyMatches('secret-key ', 'secret-key')).toBe(false); + expect(apiKeyMatches('Secret-Key', 'secret-key')).toBe(false); + }); + + it('rejects missing, empty and different-length tokens without throwing', () => { + expect(apiKeyMatches(null, 'secret-key')).toBe(false); + expect(apiKeyMatches('', 'secret-key')).toBe(false); + expect(apiKeyMatches('secret', 'secret-key')).toBe(false); + expect(apiKeyMatches('secret-key-and-more', 'secret-key')).toBe(false); + }); + }); }); diff --git a/src/utils/cloud-account-redaction.ts b/src/utils/cloud-account-redaction.ts new file mode 100644 index 0000000..fe0b279 --- /dev/null +++ b/src/utils/cloud-account-redaction.ts @@ -0,0 +1,20 @@ +import type { CloudAccount } from '../types/cloudAccount'; + +/** + * Returns a copy of the account that is safe to hand to the renderer. + * + * The renderer never needs the OAuth credentials: they are only used by the main process + * (quota refresh, IDE switch, proxy). Keeping them out of IPC payloads means an XSS or a + * compromised renderer cannot read refresh tokens. The non-secret token metadata + * (expiry, project id, tier flags) is preserved so the output schema stays valid. + */ +export function redactCloudAccountTokens(account: T): T { + return { + ...account, + token: { + ...account.token, + access_token: '', + refresh_token: '', + }, + }; +}