Skip to content

Commit a48813a

Browse files
fix(oci): harden signed request boundaries
1 parent 47f8461 commit a48813a

3 files changed

Lines changed: 122 additions & 13 deletions

File tree

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

Lines changed: 108 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,7 @@ describe('OCI request client', () => {
211211
})
212212

213213
it('redacts authorization material embedded in a serialized JSON message', async () => {
214-
const echoedAuthorization =
215-
'Signature version="1",keyId="tenant/user/fingerprint",headers="(request-target) host x-date",signature="provider-echo"'
214+
const echoedAuthorization = 'opaque-authorization-value'
216215
secureFetchMock.mockResolvedValueOnce(
217216
secureResponse({
218217
ok: false,
@@ -232,10 +231,8 @@ describe('OCI request client', () => {
232231
maxResponseBytes: 65_536,
233232
}).catch((error: unknown) => error)
234233
expect(failure).toBeInstanceOf(OciRequestError)
235-
expect((failure as Error).message).toBe('OCI request failed with status 401')
236-
expect((failure as Error).message).not.toContain('provider-echo')
237-
expect((failure as Error).message).not.toContain('(request-target)')
238-
expect((failure as Error).message).not.toContain('tenant/user/fingerprint')
234+
expect((failure as Error).message).toContain('[REDACTED]')
235+
expect((failure as Error).message).not.toContain(echoedAuthorization)
239236
})
240237

241238
it('redacts encoded credentials and request URLs echoed by the provider', async () => {
@@ -268,6 +265,57 @@ describe('OCI request client', () => {
268265
expect((failure as Error).message).not.toContain(escapedPassphrase)
269266
})
270267

268+
it('redacts an echoed finalized request body from provider diagnostics', async () => {
269+
const requestBody = 'opaque-request-body-secret'
270+
secureFetchMock.mockResolvedValueOnce(
271+
secureResponse({
272+
ok: false,
273+
status: 400,
274+
body: JSON.stringify({
275+
code: 'InvalidParameter',
276+
message: `provider echoed ${requestBody}`,
277+
}),
278+
})
279+
)
280+
const failure = await sendOciRequest({
281+
destination,
282+
credentials,
283+
method: 'POST',
284+
encodedPath: '/n/',
285+
timeout: 10_000,
286+
maxResponseBytes: 65_536,
287+
body: requestBody,
288+
}).catch((error: unknown) => error)
289+
expect((failure as Error).message).toContain('[REDACTED]')
290+
expect((failure as Error).message).not.toContain(requestBody)
291+
})
292+
293+
it('fails closed instead of redacting an unbounded request body', async () => {
294+
const requestBody = 's'.repeat(65_537)
295+
secureFetchMock.mockResolvedValueOnce(
296+
secureResponse({
297+
ok: false,
298+
status: 400,
299+
opcRequestId: 'request-body-echo',
300+
body: JSON.stringify({
301+
code: 'InvalidParameter',
302+
message: `provider echoed ${requestBody.slice(0, 1024)}`,
303+
}),
304+
})
305+
)
306+
const failure = await sendOciRequest({
307+
destination,
308+
credentials,
309+
method: 'POST',
310+
encodedPath: '/n/',
311+
timeout: 10_000,
312+
maxResponseBytes: 65_536,
313+
body: requestBody,
314+
}).catch((error: unknown) => error)
315+
expect((failure as Error).message).toBe('OCI request failed with status 400')
316+
expect((failure as OciRequestError).opcRequestId).toBeUndefined()
317+
})
318+
271319
it('redacts a maximum-size passphrase before bounding an encoded diagnostic', async () => {
272320
const longPassphrase = ' '.repeat(4096)
273321
const encodedPassphrase = new URLSearchParams({ value: longPassphrase })
@@ -319,13 +367,13 @@ describe('OCI request client', () => {
319367
expect((failure as Error).message).toBe('OCI request failed with status 401')
320368
})
321369

322-
it('redacts generic and percent-encoded sensitive JSON fields', async () => {
370+
it('redacts generic, spaced, and OCI-specific sensitive JSON fields', async () => {
323371
const echoedSecrets = [
324372
'access-value',
325373
'token-value',
326374
'secret-value',
327375
'password-value',
328-
'(request-target) host x-date',
376+
'signing-string-value',
329377
'private-key-value',
330378
'api-key-value',
331379
]
@@ -339,7 +387,7 @@ describe('OCI request client', () => {
339387
access_token: echoedSecrets[0],
340388
token: echoedSecrets[1],
341389
secret: echoedSecrets[2],
342-
'pass%70hrase': echoedSecrets[3],
390+
passphrase: echoedSecrets[3],
343391
signing_string: echoedSecrets[4],
344392
'private key': echoedSecrets[5],
345393
'api key': echoedSecrets[6],
@@ -355,9 +403,32 @@ describe('OCI request client', () => {
355403
timeout: 10_000,
356404
maxResponseBytes: 65_536,
357405
}).catch((error: unknown) => error)
406+
expect((failure as Error).message).toContain('[REDACTED]')
358407
for (const secret of echoedSecrets) expect((failure as Error).message).not.toContain(secret)
359408
})
360409

410+
it('fails closed for a percent-encoded sensitive JSON key', async () => {
411+
secureFetchMock.mockResolvedValueOnce(
412+
secureResponse({
413+
ok: false,
414+
status: 401,
415+
body: JSON.stringify({
416+
code: 'NotAuthenticated',
417+
message: JSON.stringify({ 'pass%70hrase': 'provider-echo' }),
418+
}),
419+
})
420+
)
421+
const failure = await sendOciRequest({
422+
destination,
423+
credentials,
424+
method: 'GET',
425+
encodedPath: '/n/',
426+
timeout: 10_000,
427+
maxResponseBytes: 65_536,
428+
}).catch((error: unknown) => error)
429+
expect((failure as Error).message).toBe('OCI request failed with status 401')
430+
})
431+
361432
it('fails closed when structured JSON follows a plain-text prefix', async () => {
362433
const message = `provider failed: ${JSON.stringify({ authorization: 'provider-echo' })}`
363434
secureFetchMock.mockResolvedValueOnce(
@@ -402,6 +473,29 @@ describe('OCI request client', () => {
402473
expect((failure as Error).message).toBe('OCI request failed with status 401')
403474
})
404475

476+
it.each([
477+
'provider echoed \\u0028request-target\\u0029 host x-date',
478+
'provider echoed \\u0068ttps\\u003a\\u002f\\u002fexample.com',
479+
'provider echoed \\x28request-target\\x29',
480+
])('fails closed for Unicode-escaped diagnostics', async (message) => {
481+
secureFetchMock.mockResolvedValueOnce(
482+
secureResponse({
483+
ok: false,
484+
status: 401,
485+
body: JSON.stringify({ code: 'NotAuthenticated', message }),
486+
})
487+
)
488+
const failure = await sendOciRequest({
489+
destination,
490+
credentials,
491+
method: 'GET',
492+
encodedPath: '/n/',
493+
timeout: 10_000,
494+
maxResponseBytes: 65_536,
495+
}).catch((error: unknown) => error)
496+
expect((failure as Error).message).toBe('OCI request failed with status 401')
497+
})
498+
405499
it.each([
406500
'{"authorization":"Signature version=\\"1\\",signature=\\"echoed\\"',
407501
JSON.stringify({ level1: { level2: { level3: { authorization: 'echoed' } } } }),
@@ -432,6 +526,11 @@ describe('OCI request client', () => {
432526
'/path#fragment',
433527
'/path\\replacement',
434528
'/path%ZZ',
529+
'/n/../tenant',
530+
'/n/./tenant',
531+
'/n/%2e/tenant',
532+
'/n/%2E%2E/tenant',
533+
'/n/.%2e/tenant',
435534
])('rejects unsafe encoded paths: %s', (encodedPath) => {
436535
expect(() => buildOciRequestUrl(destination, encodedPath)).toThrow(
437536
'single encoded absolute path'

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
} from '@/lib/internal/oci/signing.server'
1313

1414
const MAX_OCI_TIMEOUT_MS = 5 * 60 * 1000
15+
const MAX_OCI_REDACTABLE_BODY_LENGTH = 65_536
1516

1617
export interface OciRequestResult {
1718
readonly response: SecureFetchResponse
@@ -43,6 +44,9 @@ export function buildOciRequestUrl(
4344
) {
4445
throw new Error('OCI request path must be a single encoded absolute path')
4546
}
47+
if (new URL(`${destination.origin}${encodedPath}`).pathname !== encodedPath) {
48+
throw new Error('OCI request path must be a single encoded absolute path')
49+
}
4650
const query = serializeOciQueryPairs(queryPairs)
4751
return `${destination.origin}${encodedPath}${query ? `?${query}` : ''}`
4852
}
@@ -63,7 +67,8 @@ function validateRequestLimits(timeout: number, maxResponseBytes: number): void
6367
function sensitiveRequestValues(
6468
credentials: OciSigningCredentials,
6569
authorization: string | undefined,
66-
requestUrl: string
70+
requestUrl: string,
71+
requestBody: string | undefined
6772
): string[] {
6873
return [
6974
credentials.tenancyId,
@@ -74,6 +79,7 @@ function sensitiveRequestValues(
7479
credentials.passphrase ?? '',
7580
authorization ?? '',
7681
requestUrl,
82+
requestBody ?? '',
7783
].filter(Boolean)
7884
}
7985

@@ -119,18 +125,21 @@ export async function sendOciRequest(params: {
119125
const opcRequestId = response.headers.get('opc-request-id') ?? undefined
120126
if (response.ok) return { response, opcRequestId }
121127

128+
const requestBodyIsRedactable =
129+
signed.body === undefined || signed.body.length <= MAX_OCI_REDACTABLE_BODY_LENGTH
122130
const sensitiveValues = sensitiveRequestValues(
123131
params.credentials,
124132
signed.headers.authorization,
125-
signed.url
133+
signed.url,
134+
requestBodyIsRedactable ? signed.body : undefined
126135
)
127136
const body = await response.text()
128-
const error = parseOciErrorBody(body, sensitiveValues)
137+
const error = requestBodyIsRedactable ? parseOciErrorBody(body, sensitiveValues) : {}
129138
throw new OciRequestError({
130139
status: response.status,
131140
code: error.code,
132141
message: error.message,
133-
opcRequestId,
142+
opcRequestId: requestBodyIsRedactable ? opcRequestId : undefined,
134143
sensitiveValues,
135144
})
136145
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ function sanitizeOciErrorField(
9393
if (typeof value !== 'string') return undefined
9494
if (value.length > MAX_OCI_ERROR_INPUT_LENGTH) return undefined
9595
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
9697
if (/\(request-target\)|x-content-sha256/i.test(value)) return undefined
9798
const decoded = decodeNestedJsonDiagnostic(value)
9899
if (decoded === undefined) return undefined

0 commit comments

Comments
 (0)