Skip to content

Commit 91d34b3

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(oracle-fusion): harden response boundaries
1 parent 367f06f commit 91d34b3

2 files changed

Lines changed: 68 additions & 4 deletions

File tree

apps/sim/lib/internal/oracle-fusion-financials/client.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
redactApiKeys,
1313
redactExactSensitiveValues,
1414
} from '@/lib/core/security/redaction'
15-
import { consumeOrCancelBody } from '@/lib/core/utils/stream-limits'
15+
import { consumeOrCancelBody, isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
1616
import { normalizeOracleFusionApplicationOrigin } from '@/lib/credentials/client-credential-accounts/descriptors'
1717
import type { OracleFusionAuthInput } from '@/lib/internal/oracle-fusion-financials/schema'
1818

@@ -29,9 +29,11 @@ const TRANSIENT_TRANSPORT_CODES = new Set([
2929
'ETIMEDOUT',
3030
])
3131
const MAX_STRUCTURED_DIAGNOSTIC_DEPTH = 8
32+
const MAX_DIAGNOSTIC_KEY_ENCODING_DEPTH = 3
3233
const UNSAFE_DIAGNOSTIC = Symbol('unsafe-oracle-diagnostic')
34+
const ENCODED_DIAGNOSTIC_KEY_COMPONENT = /%[0-9A-F]{2}/i
3335
const DIAGNOSTIC_KEY_VALUE_PATTERN =
34-
/(?:"([^"\r\n]{1,128})"|'([^'\r\n]{1,128})'|([A-Za-z][A-Za-z0-9 _-]{0,127}))\s*(?::|=)/g
36+
/(?:"([^"\r\n]{1,128})"|'([^'\r\n]{1,128})'|([A-Za-z%][A-Za-z0-9% _-]{0,127}))\s*(?::|=)/g
3537
const LOSSLESS_DECIMAL_FIELDS = new Set([
3638
'InvoiceId',
3739
'InvoiceDistributionId',
@@ -92,7 +94,18 @@ function collectOracleErrorMessages(payload: unknown, depth = 0): string[] {
9294
}
9395

9496
function isSensitiveDiagnosticKey(key: string): boolean {
95-
return isSensitiveKey(key.trim().replaceAll(/\s+/g, '_'))
97+
let normalized = key.trim()
98+
for (let depth = 0; depth < MAX_DIAGNOSTIC_KEY_ENCODING_DEPTH; depth++) {
99+
if (!normalized.includes('%')) break
100+
if (!ENCODED_DIAGNOSTIC_KEY_COMPONENT.test(normalized)) return true
101+
try {
102+
normalized = decodeURIComponent(normalized)
103+
} catch {
104+
return true
105+
}
106+
}
107+
if (normalized.includes('%')) return true
108+
return isSensitiveKey(normalized.replaceAll(/[^A-Za-z0-9]+/g, '_'))
96109
}
97110

98111
function containsSensitiveDiagnosticKey(value: string): boolean {
@@ -270,6 +283,12 @@ export async function requestOracleFusionJson(
270283
response = await fetchAttempt(url, validation.resolvedIP, auth.accessToken, signal)
271284
} catch (error) {
272285
signal?.throwIfAborted()
286+
if (isPayloadSizeLimitError(error)) {
287+
throw new OracleFusionFinancialsProviderError(
288+
'Oracle Fusion Financials response could not be read',
289+
502
290+
)
291+
}
273292
if (isTransientTransportError(error) && attempt < MAX_RETRIES) {
274293
await waitForRetry(attempt, signal)
275294
continue

apps/sim/lib/internal/oracle-fusion-financials/oracle-fusion-financials.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ vi.mock('@sim/utils/retry', () => ({
2020
parseRetryAfter: vi.fn((value: string | null) => (value === '2' ? 2_000 : null)),
2121
}))
2222

23+
import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
2324
import {
2425
OracleFusionFinancialsProviderError,
2526
requestOracleFusionJson,
@@ -1064,9 +1065,39 @@ describe('Oracle Fusion Financials provider', () => {
10641065
})
10651066
})
10661067

1068+
it('maps a pinned-fetch response size rejection to a sanitized 502', async () => {
1069+
mockSecureFetch.mockRejectedValueOnce(
1070+
new PayloadSizeLimitError({
1071+
label: 'response body',
1072+
maxBytes: 5 * 1024 * 1024,
1073+
observedBytes: 5 * 1024 * 1024 + 1,
1074+
})
1075+
)
1076+
1077+
await expect(
1078+
requestOracleFusionJson(AUTH, { path: `${RESOURCE_PATH}/invoices` })
1079+
).rejects.toMatchObject({
1080+
name: 'OracleFusionFinancialsProviderError',
1081+
status: 502,
1082+
message: 'Oracle Fusion Financials response could not be read',
1083+
})
1084+
expect(mockSecureFetch).toHaveBeenCalledTimes(1)
1085+
expect(mockSleep).not.toHaveBeenCalled()
1086+
})
1087+
10671088
it('retries 429, 503, and 504 at most twice and honors Retry-After', async () => {
1089+
const retryBody = new ReadableStream<Uint8Array>({
1090+
start(controller) {
1091+
controller.enqueue(new TextEncoder().encode('retry response'))
1092+
controller.close()
1093+
},
1094+
})
1095+
const getReader = vi.spyOn(retryBody, 'getReader')
10681096
mockSecureFetch
1069-
.mockResolvedValueOnce(response(429, { title: 'slow down' }, { 'retry-after': '2' }))
1097+
.mockResolvedValueOnce({
1098+
...response(429, { title: 'slow down' }, { 'retry-after': '2' }),
1099+
body: retryBody,
1100+
})
10701101
.mockResolvedValueOnce(response(503, { title: 'unavailable' }))
10711102
.mockResolvedValueOnce(response(200, page([])))
10721103

@@ -1078,6 +1109,13 @@ describe('Oracle Fusion Financials provider', () => {
10781109
maxMs: 5_000,
10791110
})
10801111
expect(mockSleep).toHaveBeenCalledTimes(2)
1112+
expect(getReader).toHaveBeenCalledTimes(1)
1113+
expect(getReader.mock.invocationCallOrder[0]).toBeLessThan(
1114+
mockSleep.mock.invocationCallOrder[0]
1115+
)
1116+
const drainedReader = retryBody.getReader()
1117+
await expect(drainedReader.read()).resolves.toEqual({ done: true, value: undefined })
1118+
drainedReader.releaseLock()
10811119
})
10821120

10831121
it('retries classified transient transport failures within the existing bound', async () => {
@@ -1181,6 +1219,9 @@ describe('Oracle Fusion Financials provider', () => {
11811219
'api key': 'provider-spaced-api-key-canary',
11821220
' private key ': 'provider-spaced-private-key-canary',
11831221
'proxy authorization': 'provider-spaced-proxy-auth-canary',
1222+
'api%255Fkey': 'provider-encoded-api-key-canary',
1223+
'api%ZZkey': 'provider-malformed-key-canary',
1224+
'api%2525255Fkey': 'provider-overencoded-key-canary',
11841225
entries: [{ access_token: 'provider-nested-token-canary' }],
11851226
serialized: JSON.stringify({
11861227
private_key: 'provider-double-serialized-secret-canary',
@@ -1204,6 +1245,9 @@ describe('Oracle Fusion Financials provider', () => {
12041245
expect((error as Error).message).not.toContain('provider-spaced-api-key-canary')
12051246
expect((error as Error).message).not.toContain('provider-spaced-private-key-canary')
12061247
expect((error as Error).message).not.toContain('provider-spaced-proxy-auth-canary')
1248+
expect((error as Error).message).not.toContain('provider-encoded-api-key-canary')
1249+
expect((error as Error).message).not.toContain('provider-malformed-key-canary')
1250+
expect((error as Error).message).not.toContain('provider-overencoded-key-canary')
12071251
expect((error as Error).message).not.toContain('provider-nested-token-canary')
12081252
expect((error as Error).message).not.toContain('provider-double-serialized-secret-canary')
12091253
})
@@ -1228,6 +1272,7 @@ describe('Oracle Fusion Financials provider', () => {
12281272
response(400, {
12291273
detail: 'api key: "provider-fragment-secret-canary"',
12301274
message: 'client_secret: "provider-fragment-client-secret-canary"',
1275+
title: 'api%255Fkey: "provider-encoded-fragment-secret-canary"',
12311276
})
12321277
)
12331278

0 commit comments

Comments
 (0)