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: '', + }, + }; +}