Skip to content

Commit 230ab8d

Browse files
fix(oci): bound and sanitize provider errors
1 parent a48813a commit 230ab8d

3 files changed

Lines changed: 195 additions & 23 deletions

File tree

apps/sim/lib/internal/oci/client.server.test.ts

Lines changed: 112 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ function secureResponse(params: {
2424
ok: boolean
2525
status: number
2626
body?: string
27+
responseBody?: ReadableStream<Uint8Array> | null
2728
opcRequestId?: string
2829
}) {
2930
return {
@@ -34,7 +35,7 @@ function secureResponse(params: {
3435
get: (name: string) =>
3536
name.toLowerCase() === 'opc-request-id' ? (params.opcRequestId ?? null) : null,
3637
},
37-
body: null,
38+
body: params.responseBody ?? null,
3839
text: vi.fn().mockResolvedValue(params.body ?? ''),
3940
json: vi.fn(),
4041
arrayBuffer: vi.fn(),
@@ -235,19 +236,17 @@ describe('OCI request client', () => {
235236
expect((failure as Error).message).not.toContain(echoedAuthorization)
236237
})
237238

238-
it('redacts encoded credentials and request URLs echoed by the provider', async () => {
239+
it('redacts encoded credentials instead of falling through to a status-only error', async () => {
239240
const encodedFingerprint = encodeURIComponent(credentials.fingerprint)
240-
const requestUrl = `${destination.origin}/n/`
241-
const encodedRequestUrl = encodeURIComponent(requestUrl)
242241
const escapedPassphrase = 'secret "pass"'
243-
const escapedPassphraseEcho = JSON.stringify(escapedPassphrase).slice(1, -1)
242+
const encodedPassphrase = encodeURIComponent(escapedPassphrase)
244243
secureFetchMock.mockResolvedValueOnce(
245244
secureResponse({
246245
ok: false,
247246
status: 401,
248247
body: JSON.stringify({
249248
code: 'NotAuthenticated',
250-
message: `provider echoed ${encodedFingerprint} ${encodedRequestUrl} ${escapedPassphraseEcho}`,
249+
message: `provider echoed ${encodedFingerprint} ${encodedPassphrase}`,
251250
}),
252251
})
253252
)
@@ -259,12 +258,63 @@ describe('OCI request client', () => {
259258
timeout: 10_000,
260259
maxResponseBytes: 65_536,
261260
}).catch((error: unknown) => error)
261+
expect((failure as Error).message).toContain('provider echoed')
262+
expect((failure as Error).message).toContain('[REDACTED]')
262263
expect((failure as Error).message).not.toContain(encodedFingerprint)
263-
expect((failure as Error).message).not.toContain(encodedRequestUrl)
264-
expect((failure as Error).message).not.toContain(escapedPassphraseEcho)
264+
expect((failure as Error).message).not.toContain(encodedPassphrase)
265265
expect((failure as Error).message).not.toContain(escapedPassphrase)
266266
})
267267

268+
it('redacts an encoded signed request URL instead of returning it', async () => {
269+
const encodedRequestUrl = encodeURIComponent(`${destination.origin}/n/`)
270+
secureFetchMock.mockResolvedValueOnce(
271+
secureResponse({
272+
ok: false,
273+
status: 401,
274+
body: JSON.stringify({
275+
code: 'NotAuthenticated',
276+
message: `provider echoed ${encodedRequestUrl}`,
277+
}),
278+
})
279+
)
280+
const failure = await sendOciRequest({
281+
destination,
282+
credentials,
283+
method: 'GET',
284+
encodedPath: '/n/',
285+
timeout: 10_000,
286+
maxResponseBytes: 65_536,
287+
}).catch((error: unknown) => error)
288+
expect((failure as Error).message).toContain('provider echoed')
289+
expect((failure as Error).message).toContain('[REDACTED]')
290+
expect((failure as Error).message).not.toContain(encodedRequestUrl)
291+
})
292+
293+
it('redacts caller-supplied service header values echoed by the provider', async () => {
294+
const serviceHeaderSecret = 'opaque-service-header-secret'
295+
secureFetchMock.mockResolvedValueOnce(
296+
secureResponse({
297+
ok: false,
298+
status: 401,
299+
body: JSON.stringify({
300+
code: 'NotAuthenticated',
301+
message: `provider echoed ${serviceHeaderSecret}`,
302+
}),
303+
})
304+
)
305+
const failure = await sendOciRequest({
306+
destination,
307+
credentials,
308+
method: 'GET',
309+
encodedPath: '/n/',
310+
timeout: 10_000,
311+
maxResponseBytes: 65_536,
312+
serviceHeaders: { 'opc-client-info': serviceHeaderSecret },
313+
}).catch((error: unknown) => error)
314+
expect((failure as Error).message).toContain('[REDACTED]')
315+
expect((failure as Error).message).not.toContain(serviceHeaderSecret)
316+
})
317+
268318
it('redacts an echoed finalized request body from provider diagnostics', async () => {
269319
const requestBody = 'opaque-request-body-secret'
270320
secureFetchMock.mockResolvedValueOnce(
@@ -376,6 +426,7 @@ describe('OCI request client', () => {
376426
'signing-string-value',
377427
'private-key-value',
378428
'api-key-value',
429+
'signature-value',
379430
]
380431
secureFetchMock.mockResolvedValueOnce(
381432
secureResponse({
@@ -391,6 +442,7 @@ describe('OCI request client', () => {
391442
signing_string: echoedSecrets[4],
392443
'private key': echoedSecrets[5],
393444
'api key': echoedSecrets[6],
445+
signature: echoedSecrets[7],
394446
}),
395447
}),
396448
})
@@ -407,6 +459,58 @@ describe('OCI request client', () => {
407459
for (const secret of echoedSecrets) expect((failure as Error).message).not.toContain(secret)
408460
})
409461

462+
it('fails closed for authorization signatures with flexible parameter spacing', async () => {
463+
const echoedSignature = 'unknown-provider-signature'
464+
secureFetchMock.mockResolvedValueOnce(
465+
secureResponse({
466+
ok: false,
467+
status: 401,
468+
body: JSON.stringify({
469+
code: 'NotAuthenticated',
470+
message: `provider echoed Signature version = "1", keyId = "unknown", signature = "${echoedSignature}"`,
471+
}),
472+
})
473+
)
474+
const failure = await sendOciRequest({
475+
destination,
476+
credentials,
477+
method: 'GET',
478+
encodedPath: '/n/',
479+
timeout: 10_000,
480+
maxResponseBytes: 65_536,
481+
}).catch((error: unknown) => error)
482+
expect((failure as Error).message).toBe('OCI request failed with status 401')
483+
expect((failure as Error).message).not.toContain(echoedSignature)
484+
})
485+
486+
it('bounds non-success response bodies independently of the caller response ceiling', async () => {
487+
const cancel = vi.fn()
488+
const response = secureResponse({
489+
ok: false,
490+
status: 502,
491+
opcRequestId: 'request-oversized',
492+
responseBody: new ReadableStream<Uint8Array>({
493+
start(controller) {
494+
controller.enqueue(new Uint8Array(65_537))
495+
},
496+
cancel,
497+
}),
498+
})
499+
secureFetchMock.mockResolvedValueOnce(response)
500+
const failure = await sendOciRequest({
501+
destination,
502+
credentials,
503+
method: 'GET',
504+
encodedPath: '/n/',
505+
timeout: 10_000,
506+
maxResponseBytes: 1024 * 1024,
507+
}).catch((error: unknown) => error)
508+
expect((failure as Error).message).toBe('OCI request failed with status 502')
509+
expect((failure as OciRequestError).opcRequestId).toBe('request-oversized')
510+
expect(cancel).toHaveBeenCalledOnce()
511+
expect(response.text).not.toHaveBeenCalled()
512+
})
513+
410514
it('fails closed for a percent-encoded sensitive JSON key', async () => {
411515
secureFetchMock.mockResolvedValueOnce(
412516
secureResponse({

apps/sim/lib/internal/oci/client.server.ts

Lines changed: 68 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import {
33
type SecureFetchResponse,
44
secureFetchWithValidation,
55
} from '@/lib/core/security/input-validation.server'
6+
import {
7+
DEFAULT_MAX_ERROR_BODY_BYTES,
8+
readResponseTextWithLimit,
9+
} from '@/lib/core/utils/stream-limits'
610
import type { ValidatedOciDestination } from '@/lib/internal/oci/endpoints'
711
import { OciRequestError, parseOciErrorBody } from '@/lib/internal/oci/errors'
812
import {
@@ -12,7 +16,7 @@ import {
1216
} from '@/lib/internal/oci/signing.server'
1317

1418
const MAX_OCI_TIMEOUT_MS = 5 * 60 * 1000
15-
const MAX_OCI_REDACTABLE_BODY_LENGTH = 65_536
19+
const MAX_OCI_REDACTABLE_REQUEST_MATERIAL_LENGTH = 65_536
1620

1721
export interface OciRequestResult {
1822
readonly response: SecureFetchResponse
@@ -68,7 +72,8 @@ function sensitiveRequestValues(
6872
credentials: OciSigningCredentials,
6973
authorization: string | undefined,
7074
requestUrl: string,
71-
requestBody: string | undefined
75+
requestBody: string | undefined,
76+
serviceHeaderValues: readonly string[]
7277
): string[] {
7378
return [
7479
credentials.tenancyId,
@@ -80,9 +85,51 @@ function sensitiveRequestValues(
8085
authorization ?? '',
8186
requestUrl,
8287
requestBody ?? '',
88+
...serviceHeaderValues,
8389
].filter(Boolean)
8490
}
8591

92+
function getSignedServiceHeaderValues(
93+
serviceHeaders: Readonly<Record<string, string>> | undefined,
94+
signedHeaders: Readonly<Record<string, string>>
95+
): string[] {
96+
return Object.keys(serviceHeaders ?? {}).flatMap((name) => {
97+
const value = signedHeaders[name.toLowerCase()]
98+
return value === undefined ? [] : [value]
99+
})
100+
}
101+
102+
function isRedactableRequestMaterial(values: readonly (string | undefined)[]): boolean {
103+
let totalLength = 0
104+
for (const value of values) {
105+
if (value === undefined) continue
106+
totalLength += value.length
107+
if (totalLength > MAX_OCI_REDACTABLE_REQUEST_MATERIAL_LENGTH) return false
108+
}
109+
return true
110+
}
111+
112+
async function readOciErrorBody(
113+
response: SecureFetchResponse,
114+
method: OciRequestMethod,
115+
maxResponseBytes: number,
116+
signal: AbortSignal | undefined
117+
): Promise<string | undefined> {
118+
try {
119+
return await readResponseTextWithLimit(response, {
120+
maxBytes: Math.min(DEFAULT_MAX_ERROR_BODY_BYTES, maxResponseBytes),
121+
label: 'OCI error response',
122+
signal,
123+
allowNoBodyFallback: true,
124+
requestMethod: method,
125+
})
126+
} catch (error) {
127+
if (signal?.aborted) throw error
128+
await response.body?.cancel().catch(() => {})
129+
return undefined
130+
}
131+
}
132+
86133
/** Sends one bounded, redirect-free OCI request to an already validated destination. */
87134
export async function sendOciRequest(params: {
88135
destination: ValidatedOciDestination
@@ -125,21 +172,34 @@ export async function sendOciRequest(params: {
125172
const opcRequestId = response.headers.get('opc-request-id') ?? undefined
126173
if (response.ok) return { response, opcRequestId }
127174

128-
const requestBodyIsRedactable =
129-
signed.body === undefined || signed.body.length <= MAX_OCI_REDACTABLE_BODY_LENGTH
175+
const serviceHeaderValues = getSignedServiceHeaderValues(params.serviceHeaders, signed.headers)
176+
const requestMaterialIsRedactable = isRedactableRequestMaterial([
177+
signed.body,
178+
...serviceHeaderValues,
179+
])
180+
if (!requestMaterialIsRedactable) {
181+
await response.body?.cancel().catch(() => {})
182+
throw new OciRequestError({ status: response.status })
183+
}
130184
const sensitiveValues = sensitiveRequestValues(
131185
params.credentials,
132186
signed.headers.authorization,
133187
signed.url,
134-
requestBodyIsRedactable ? signed.body : undefined
188+
signed.body,
189+
serviceHeaderValues
190+
)
191+
const body = await readOciErrorBody(
192+
response,
193+
signed.method,
194+
params.maxResponseBytes,
195+
params.signal
135196
)
136-
const body = await response.text()
137-
const error = requestBodyIsRedactable ? parseOciErrorBody(body, sensitiveValues) : {}
197+
const error = body === undefined ? {} : parseOciErrorBody(body, sensitiveValues)
138198
throw new OciRequestError({
139199
status: response.status,
140200
code: error.code,
141201
message: error.message,
142-
opcRequestId: requestBodyIsRedactable ? opcRequestId : undefined,
202+
opcRequestId,
143203
sensitiveValues,
144204
})
145205
}

apps/sim/lib/internal/oci/errors.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ import {
22
isSensitiveKey,
33
REDACTED_MARKER,
44
redactExactSensitiveValues,
5+
redactKnownSensitiveValues,
56
} from '@/lib/core/security/redaction'
67

78
const MAX_OCI_ERROR_FIELD_LENGTH = 1024
89
const MAX_OCI_ERROR_INPUT_LENGTH = 65_536
910
const MAX_NESTED_JSON_DEPTH = 3
10-
const OCI_SENSITIVE_JSON_FIELDS = new Set(['signingstring'])
11+
const OCI_SENSITIVE_JSON_FIELDS = new Set(['signature', 'signingstring'])
1112

1213
function normalizeJsonDiagnosticKey(key: string): string | undefined {
1314
let normalized = key
@@ -92,15 +93,22 @@ function sanitizeOciErrorField(
9293
): string | undefined {
9394
if (typeof value !== 'string') return undefined
9495
if (value.length > MAX_OCI_ERROR_INPUT_LENGTH) return undefined
95-
if (/%[0-9a-f]{2}/i.test(value)) return undefined
96-
if (/\\(?:u[0-9a-f]{4}|x[0-9a-f]{2})/i.test(value)) return undefined
97-
if (/\(request-target\)|x-content-sha256/i.test(value)) return undefined
98-
const decoded = decodeNestedJsonDiagnostic(value)
99-
if (decoded === undefined) return undefined
10096
const exactValues = sensitiveValues.flatMap((sensitiveValue) => {
10197
const jsonEncoded = JSON.stringify(sensitiveValue).slice(1, -1)
10298
return jsonEncoded === sensitiveValue ? [sensitiveValue] : [sensitiveValue, jsonEncoded]
10399
})
100+
let knownRedacted: string
101+
try {
102+
knownRedacted = redactKnownSensitiveValues(value, exactValues)
103+
} catch {
104+
return undefined
105+
}
106+
if (/%[0-9a-f]{2}/i.test(knownRedacted)) return undefined
107+
if (/\\(?:u[0-9a-f]{4}|x[0-9a-f]{2})/i.test(knownRedacted)) return undefined
108+
if (/\(request-target\)|x-content-sha256/i.test(knownRedacted)) return undefined
109+
if (/\bsignature\s*(?:version\s*)?=/i.test(knownRedacted)) return undefined
110+
const decoded = decodeNestedJsonDiagnostic(knownRedacted)
111+
if (decoded === undefined) return undefined
104112
let exactRedacted: string
105113
try {
106114
exactRedacted = redactExactSensitiveValues(decoded, exactValues)
@@ -110,7 +118,7 @@ function sanitizeOciErrorField(
110118
const sanitized = exactRedacted
111119
.replace(/-----BEGIN[\s\S]*/gi, '[redacted-key]')
112120
.replace(/https?:\/\/[^\s"']+/gi, '[redacted-url]')
113-
.replace(/Signature\s+version=\\*"1\\*",[^\r\n]*/gi, '[redacted-authorization]')
121+
.replace(/Signature\s+version\s*=\s*\\*"1\\*"\s*,[^\r\n]*/gi, '[redacted-authorization]')
114122
.replace(/[\u0000-\u001f\u007f]/g, ' ')
115123
.trim()
116124
return sanitized ? sanitized.slice(0, MAX_OCI_ERROR_FIELD_LENGTH) : undefined

0 commit comments

Comments
 (0)