From e932dfbfbb9b0aff7c278ec1ad1140118b7a37d3 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:09:42 -0400 Subject: [PATCH] fix(relay): never send a raw error message to the desktop as a control error code (#30873) A host control request that failed for an unexpected reason sent the thrown error's message to the desktop as the error code. During a staging database outage the desktop received Postgres's own text, and node-pg messages can carry the database connection URL. The relay now sends store codes and this codebase's snake_case invariant slugs unchanged, and maps everything else to database_transient or control_operation_failed. The original is logged through the existing redaction (now shared with the background-operation logger), so only the slug shape and SQLSTATE leave the process. --- .../relay/src/host-session-registry.test.ts | 74 ++++++++++++++++++- cloud/apps/relay/src/host-session-registry.ts | 16 ++-- .../relay/src/relay-background-operation.ts | 18 +---- cloud/apps/relay/src/relay-error-redaction.ts | 14 ++++ cloud/apps/relay/src/relay.blackbox.test.ts | 4 +- 5 files changed, 104 insertions(+), 22 deletions(-) create mode 100644 cloud/apps/relay/src/relay-error-redaction.ts diff --git a/cloud/apps/relay/src/host-session-registry.test.ts b/cloud/apps/relay/src/host-session-registry.test.ts index 4a994c4b091..0db469ad457 100644 --- a/cloud/apps/relay/src/host-session-registry.test.ts +++ b/cloud/apps/relay/src/host-session-registry.test.ts @@ -11,7 +11,7 @@ import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } fr import type WebSocket from 'ws' import type { RelayAssignmentStore } from './assignment-store.js' import type { RelayConfig } from './config.js' -import type { RelayCredentialStore } from './credential-store.js' +import { RelayStoreError, type RelayCredentialStore } from './credential-store.js' import { CONTROL_RENEWAL_BATCH_INTERVAL_MS } from './control-renewal-batch.js' import { CONTROL_RENEWAL_STATEMENT_OUTCOMES, @@ -2199,3 +2199,75 @@ describe('controls closed after host proof but before wiring', () => { expect(refusedLines()).toEqual([]) }) }) + +describe('control error codes', () => { + beforeEach(() => vi.useFakeTimers()) + afterEach(() => { + vi.clearAllTimers() + vi.useRealTimers() + }) + + async function revokeFailure(error: Error): Promise<{ sent: string[]; warned: string[] }> { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const activateControl = vi + .fn() + .mockResolvedValue('control:production-gce-c3:1') + const { registry, activate } = createRegistry(activateControl, { + revoke: vi.fn().mockRejectedValue(error) + }) + const socket = new FakeSocket() + await activate(socket as unknown as WebSocket, identity, null, 1, false, 1) + socket.emit( + 'message', + Buffer.from(JSON.stringify({ type: 'device-revoke', reqId: 'revoke-1', relayDeviceId: 'device-1' })), + false + ) + await vi.advanceTimersByTimeAsync(0) + const sent = socket.send.mock.calls.map((call) => String(call[0])) + const warned = warn.mock.calls.map((call) => String(call[0])) + warn.mockRestore() + registry.drain(0) + vi.advanceTimersByTime(0) + return { sent, warned } + } + + function controlErrors(sent: string[]): unknown[] { + return sent + .map((line) => JSON.parse(line) as Record) + .filter((message) => message.type === 'control-error') + } + + it('never sends a database error message, which can carry the connection URL', async () => { + // Assembled at runtime so secret scanners do not flag a fixture URL. + const url = ['postgres', '://relay:', 'fake-pw', '@db.internal:5432/relay'].join('') + const error = Object.assign(new Error(`connect to ${url} failed: Connection terminated`), { + code: '08006' + }) + const { sent, warned } = await revokeFailure(error) + + expect(controlErrors(sent)).toEqual([ + { type: 'control-error', reqId: 'revoke-1', code: 'database_transient' } + ]) + expect(sent.join('\n')).not.toContain('fake-pw') + expect(warned).toContain('[orca-relay] control device-revoke failed: Error: redacted code=08006') + expect(warned.join('\n')).not.toContain('fake-pw') + }) + + it('maps any other unexpected message to a fixed code', async () => { + const { sent } = await revokeFailure(new Error('Unexpected token < in JSON')) + + expect(controlErrors(sent)).toEqual([ + { type: 'control-error', reqId: 'revoke-1', code: 'control_operation_failed' } + ]) + }) + + it.each([ + ['a store code', new RelayStoreError('reject-revoked'), 'reject-revoked'], + ['an invariant slug', new Error('authorization_expired'), 'authorization_expired'] + ])('keeps %s on the wire', async (_kind, error, code) => { + const { sent, warned } = await revokeFailure(error) + + expect(controlErrors(sent)).toEqual([{ type: 'control-error', reqId: 'revoke-1', code }]) + expect(warned.join('\n')).not.toContain('control device-revoke failed') + }) +}) diff --git a/cloud/apps/relay/src/host-session-registry.ts b/cloud/apps/relay/src/host-session-registry.ts index 3ea5c39fb73..1ccbabb98e7 100644 --- a/cloud/apps/relay/src/host-session-registry.ts +++ b/cloud/apps/relay/src/host-session-registry.ts @@ -48,6 +48,7 @@ import { PostProofCloseLog, type PostProofCloseCause } from './post-proof-close-log.js' +import { describeRelayFailure, relayErrorSlug } from './relay-error-redaction.js' import { relayHostLogDigest } from './relay-host-log-digest.js' import type { RelayTokenClaims } from './relay-token-verifier.js' import { @@ -2528,11 +2529,7 @@ export class HostSessionRegistry { ? String(candidate.reqId) : undefined })() - this.sendControlError( - session, - reqId, - error instanceof Error ? error.message : 'control_operation_failed' - ) + this.sendControlError(session, reqId, controlErrorCode(type, error)) } } @@ -2640,6 +2637,15 @@ export class HostSessionRegistry { export type RelayIdentityKey = { userId: string; relayHostId: string } +// Intentional throws are store codes or slugs; anything else could carry the database URL. +function controlErrorCode(type: string, error: unknown): string { + if (error instanceof RelayStoreError) return error.code + const slug = relayErrorSlug(error) + if (slug) return slug + console.warn(`[orca-relay] control ${type} failed: ${describeRelayFailure(error)}`) + return isRelayDatabaseTransientError(error) ? 'database_transient' : 'control_operation_failed' +} + function databaseCause(error: unknown): PhoneDatabaseCause { return isRelayDatabaseTransientError(error) ? 'database_transient' : 'error' } diff --git a/cloud/apps/relay/src/relay-background-operation.ts b/cloud/apps/relay/src/relay-background-operation.ts index be3943b7b1a..adf704106cd 100644 --- a/cloud/apps/relay/src/relay-background-operation.ts +++ b/cloud/apps/relay/src/relay-background-operation.ts @@ -1,19 +1,9 @@ +import { describeRelayFailure } from './relay-error-redaction.js' + type Warn = (message: string) => void // A swallowed error hides which sweep step is failing (a silently dead -// cleanup chain stalled production rehoming for hours), but raw messages can -// embed secrets such as connection strings. Only two provably inert shapes -// are logged: this codebase's snake_case invariant slugs, and five-character -// SQLSTATE codes; everything else stays redacted. -function describeFailure(error: unknown): string { - const name = error instanceof Error ? error.name : typeof error - const message = error instanceof Error ? error.message : '' - const slug = /^[a-z0-9_]{1,64}$/.test(message) ? message : 'redacted' - const code = (error as { code?: unknown } | null)?.code - const sqlState = typeof code === 'string' && /^[0-9A-Z]{5}$/.test(code) ? ` code=${code}` : '' - return `${name}: ${slug}${sqlState}` -} - +// cleanup chain stalled production rehoming for hours). export async function runRelayBackgroundOperation( operation: () => Promise, failureMessage: string, @@ -23,6 +13,6 @@ export async function runRelayBackgroundOperation( await operation() } catch (error) { // Dependency outages must fail readiness without crashing liveness. - warn(`${failureMessage}: ${describeFailure(error)}`) + warn(`${failureMessage}: ${describeRelayFailure(error)}`) } } diff --git a/cloud/apps/relay/src/relay-error-redaction.ts b/cloud/apps/relay/src/relay-error-redaction.ts new file mode 100644 index 00000000000..f67642a0400 --- /dev/null +++ b/cloud/apps/relay/src/relay-error-redaction.ts @@ -0,0 +1,14 @@ +// Raw error messages can embed secrets such as connection strings. Only two +// provably inert shapes leave the process: this codebase's snake_case +// invariant slugs, and five-character SQLSTATE codes; everything else is redacted. +export function relayErrorSlug(error: unknown): string | null { + const message = error instanceof Error ? error.message : '' + return /^[a-z0-9_]{1,64}$/.test(message) ? message : null +} + +export function describeRelayFailure(error: unknown): string { + const name = error instanceof Error ? error.name : typeof error + const code = typeof error === 'object' && error !== null && 'code' in error ? error.code : null + const sqlState = typeof code === 'string' && /^[0-9A-Z]{5}$/.test(code) ? ` code=${code}` : '' + return `${name}: ${relayErrorSlug(error) ?? 'redacted'}${sqlState}` +} diff --git a/cloud/apps/relay/src/relay.blackbox.test.ts b/cloud/apps/relay/src/relay.blackbox.test.ts index 84f8a87f80b..35785ac6528 100644 --- a/cloud/apps/relay/src/relay.blackbox.test.ts +++ b/cloud/apps/relay/src/relay.blackbox.test.ts @@ -1542,7 +1542,7 @@ describe('served relay URL', () => { expect(await failed).toMatchObject({ type: 'control-error', reqId: 'fault-install', - code: 'injected SQL failure' + code: 'control_operation_failed' }) const status = nextMessage(host.socket) host.socket.send( @@ -1642,7 +1642,7 @@ describe('served relay URL', () => { expect(await directFailure).toMatchObject({ type: 'control-error', reqId: 'fallback-install', - code: 'injected SQL failure' + code: 'control_operation_failed' }) const status = nextMessage(host.socket) host.socket.send(