Skip to content

Commit 25ad712

Browse files
Bill LeoutsakosBill Leoutsakos
Bill Leoutsakos
authored and
Bill Leoutsakos
committed
fix(oci): stop requests after DNS deadlines
1 parent f5df063 commit 25ad712

2 files changed

Lines changed: 115 additions & 45 deletions

File tree

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

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => ({
1010
predicates: undefined as unknown,
1111
rows: [] as { encryptedServiceAccountKey: string | null }[],
1212
secureFetch: vi.fn(),
13+
validateUrl: vi.fn(),
1314
}))
1415

1516
vi.mock('@sim/db', () => ({
@@ -44,7 +45,8 @@ vi.mock('@/lib/core/security/encryption', () => ({ decryptSecret: mocks.decryptS
4445

4546
vi.mock('@/lib/core/security/input-validation.server', () => ({
4647
DEFAULT_MAX_RESPONSE_BYTES: 100 * 1024 * 1024,
47-
secureFetchWithValidation: mocks.secureFetch,
48+
secureFetchWithPinnedIP: mocks.secureFetch,
49+
validateUrlWithDNS: mocks.validateUrl,
4850
}))
4951

5052
vi.mock('@sim/utils/retry', () => ({
@@ -176,7 +178,7 @@ async function createPreparedClient(params: { region?: string } = {}): Promise<{
176178
}
177179

178180
function authorizationFromLastRequest(): string {
179-
const options = mocks.secureFetch.mock.calls.at(-1)?.[1] as { headers: Record<string, string> }
181+
const options = mocks.secureFetch.mock.calls.at(-1)?.[2] as { headers: Record<string, string> }
180182
return options.headers.authorization
181183
}
182184

@@ -187,6 +189,11 @@ describe('credential-bound OCI client', () => {
187189
mocks.decryptSecret.mockReset().mockResolvedValue({ decrypted: SECRET })
188190
mocks.backoff.mockReset().mockReturnValue(0)
189191
mocks.secureFetch.mockReset().mockResolvedValue(secureResponse({}))
192+
mocks.validateUrl.mockReset().mockResolvedValue({
193+
isValid: true,
194+
resolvedIP: '203.0.113.10',
195+
originalHostname: 'identity.us-ashburn-1.oci.oraclecloud.com',
196+
})
190197
})
191198

192199
afterEach(() => {
@@ -312,7 +319,7 @@ describe('credential-bound OCI client', () => {
312319
expect(authorizationFromLastRequest()).toBe(
313320
'Signature version="1",keyId="ocid1.tenancy.oc1..aaaaaaaafixedvector/ocid1.user.oc1..aaaaaaaafixedvector/25:53:22:62:aa:db:ff:ef:f5:77:08:d1:a2:ed:8b:e6",algorithm="rsa-sha256",headers="x-date (request-target) host content-type content-length x-content-sha256",signature="W2/OGoa2XuOin6+CQt32/+/lAXG5PWoamkAHr/k84oCYGUuub2mEYw1z9p4gc6/GPgeZ30wVp4DNVLzOjup3nJir1WsEsYzAk27XAIRVjxiQ7oBzCccnSnB88KLeNz1NDz7r4QPQGxZ50MBQEe0C+DEH2P+utpfFN73o7GCUhIN9hb27COg4l7ffdSLgjBWPN/B4AiZXpjz3I/GRHo29otGAhZ3MiX10gJTjy+qeAchAfmXmTx/nJqNhF0Aj255+B2lepCrHdkpcBpiTs5E+ppE6VvML0ByQ9ZLzBISB4MBljuFyey6tnTkueT73fqjQyM/OT+aO9HrAlemc3HSAXA=="'
314321
)
315-
expect(mocks.secureFetch.mock.calls[0][1].headers).toMatchObject({
322+
expect(mocks.secureFetch.mock.calls[0][2].headers).toMatchObject({
316323
'content-length': '0',
317324
'content-type': 'application/json',
318325
'x-content-sha256': '47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=',
@@ -338,13 +345,15 @@ describe('credential-bound OCI client', () => {
338345
maxResponseBytes: 1024,
339346
})
340347

341-
const [url, options] = mocks.secureFetch.mock.calls[0] as [
348+
const [url, resolvedIP, options] = mocks.secureFetch.mock.calls[0] as [
349+
string,
342350
string,
343351
{ body: Uint8Array; headers: Record<string, string> },
344352
]
345353
expect(url).toBe(
346354
'https://identity.us-ashburn-1.oci.oraclecloud.com/v1/%E2%98%83?z=last&a=&a=%20%21%27%28%29%2A'
347355
)
356+
expect(resolvedIP).toBe('203.0.113.10')
348357
expect([...options.body]).toEqual([...body])
349358
expect(options.body).not.toBe(body)
350359
expect(options.headers).toMatchObject({
@@ -380,7 +389,7 @@ describe('credential-bound OCI client', () => {
380389
timeoutMs: 10_000,
381390
maxResponseBytes: 1024,
382391
})
383-
const options = mocks.secureFetch.mock.calls.at(-1)?.[1]
392+
const options = mocks.secureFetch.mock.calls.at(-1)?.[2]
384393
expect(options.method).toBe(method)
385394
expect(options).not.toHaveProperty('body')
386395
expect(options.headers).not.toHaveProperty('content-length')
@@ -410,7 +419,7 @@ describe('credential-bound OCI client', () => {
410419
timeoutMs: 10_000,
411420
maxResponseBytes: 1024,
412421
})
413-
expect(mocks.secureFetch.mock.calls.at(-1)?.[1].headers['content-length']).toBe('0')
422+
expect(mocks.secureFetch.mock.calls.at(-1)?.[2].headers['content-length']).toBe('0')
414423
}
415424
)
416425

@@ -515,8 +524,8 @@ describe('credential-bound OCI client', () => {
515524
maxResponseBytes: 1024,
516525
})
517526

518-
const first = mocks.secureFetch.mock.calls[0][1]
519-
const second = mocks.secureFetch.mock.calls[1][1]
527+
const first = mocks.secureFetch.mock.calls[0][2]
528+
const second = mocks.secureFetch.mock.calls[1][2]
520529
expect([...first.body]).toEqual([...second.body])
521530
expect(first.headers['opc-retry-token']).toBe('operation-token')
522531
expect(second.headers['opc-retry-token']).toBe('operation-token')
@@ -809,13 +818,45 @@ describe('credential-bound OCI client', () => {
809818
await assertion
810819
})
811820

821+
it('does not start a pinned request when destination validation outlives the deadline', async () => {
822+
vi.useFakeTimers()
823+
let finishValidation:
824+
| ((value: { isValid: true; resolvedIP: string; originalHostname: string }) => void)
825+
| undefined
826+
mocks.validateUrl.mockImplementationOnce(
827+
() =>
828+
new Promise((resolve) => {
829+
finishValidation = resolve
830+
})
831+
)
832+
const { client, endpoint } = await createPreparedClient()
833+
const pending = client.request({
834+
endpoint,
835+
method: 'GET',
836+
encodedPath: '/v1/test',
837+
timeoutMs: 100,
838+
maxResponseBytes: 1024,
839+
})
840+
const assertion = expect(pending).rejects.toMatchObject({ code: 'deadline_exceeded' })
841+
await vi.advanceTimersByTimeAsync(101)
842+
await assertion
843+
finishValidation?.({
844+
isValid: true,
845+
resolvedIP: '203.0.113.10',
846+
originalHostname: 'identity.us-ashburn-1.oci.oraclecloud.com',
847+
})
848+
await Promise.resolve()
849+
expect(mocks.secureFetch).not.toHaveBeenCalled()
850+
})
851+
812852
it('propagates caller abort while setup destination validation is pending', async () => {
813853
const controller = new AbortController()
814-
mocks.secureFetch.mockImplementationOnce(() => new Promise(() => {}))
854+
mocks.validateUrl.mockImplementationOnce(() => new Promise(() => {}))
815855
const pending = verifyOciApiKeyCredentialForSetup(SECRET, controller.signal)
816-
await vi.waitFor(() => expect(mocks.secureFetch).toHaveBeenCalledOnce())
856+
await vi.waitFor(() => expect(mocks.validateUrl).toHaveBeenCalledOnce())
817857
controller.abort()
818858
await expect(pending).rejects.toMatchObject({ code: 'aborted' })
859+
expect(mocks.secureFetch).not.toHaveBeenCalled()
819860
})
820861

821862
it('does not start setup destination validation after an earlier caller abort', async () => {
@@ -824,16 +865,32 @@ describe('credential-bound OCI client', () => {
824865
await expect(
825866
verifyOciApiKeyCredentialForSetup(SECRET, controller.signal)
826867
).rejects.toMatchObject({ code: 'aborted' })
868+
expect(mocks.validateUrl).not.toHaveBeenCalled()
827869
expect(mocks.secureFetch).not.toHaveBeenCalled()
828870
})
829871

830872
it('applies the setup deadline while destination validation is pending', async () => {
831873
vi.useFakeTimers()
832-
mocks.secureFetch.mockImplementationOnce(() => new Promise(() => {}))
874+
let finishValidation:
875+
| ((value: { isValid: true; resolvedIP: string; originalHostname: string }) => void)
876+
| undefined
877+
mocks.validateUrl.mockImplementationOnce(
878+
() =>
879+
new Promise((resolve) => {
880+
finishValidation = resolve
881+
})
882+
)
833883
const pending = verifyOciApiKeyCredentialForSetup(SECRET)
834884
const assertion = expect(pending).rejects.toMatchObject({ code: 'deadline_exceeded' })
835885
await vi.advanceTimersByTimeAsync(10_001)
836886
await assertion
887+
finishValidation?.({
888+
isValid: true,
889+
resolvedIP: '203.0.113.10',
890+
originalHostname: 'objectstorage.us-ashburn-1.oraclecloud.com',
891+
})
892+
await Promise.resolve()
893+
expect(mocks.secureFetch).not.toHaveBeenCalled()
837894
})
838895

839896
it('applies the same deadline while reading the response body', async () => {

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

Lines changed: 47 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@ import { and, eq } from 'drizzle-orm'
1515
import { decryptSecret } from '@/lib/core/security/encryption'
1616
import {
1717
DEFAULT_MAX_RESPONSE_BYTES,
18+
type SecureFetchOptions,
1819
type SecureFetchResponse,
19-
secureFetchWithValidation,
20+
secureFetchWithPinnedIP,
21+
validateUrlWithDNS,
2022
} from '@/lib/core/security/input-validation.server'
2123
import {
2224
DEFAULT_MAX_ERROR_BODY_BYTES,
@@ -688,6 +690,23 @@ async function waitForRetry(delayMs: number, signal: AbortSignal): Promise<void>
688690
await raceWithAbort(() => sleep(delayMs), signal)
689691
}
690692

693+
async function secureOciFetch(
694+
url: string,
695+
options: SecureFetchOptions,
696+
paramName: string,
697+
signal: AbortSignal
698+
): Promise<SecureFetchResponse> {
699+
const validation = await raceWithAbort(
700+
() =>
701+
validateUrlWithDNS(url, paramName, options.profile, {
702+
logDetails: options.logUrlValidationDetails,
703+
}),
704+
signal
705+
)
706+
if (!validation.isValid) throw new Error(validation.error)
707+
return raceWithAbort(() => secureFetchWithPinnedIP(url, validation.resolvedIP, options), signal)
708+
}
709+
691710
/** Creates a lazily loaded OCI client bound to trusted workspace and service context. */
692711
export async function createOciClient(params: CreateOciClientParams): Promise<OciClient> {
693712
const service = getServiceConfigByServiceId(params.serviceId)
@@ -788,23 +807,20 @@ export async function createOciClient(params: CreateOciClientParams): Promise<Oc
788807

789808
let response: SecureFetchResponse
790809
try {
791-
response = await raceWithAbort(
792-
() =>
793-
secureFetchWithValidation(
794-
signed.url,
795-
{
796-
method: request.method,
797-
headers: { ...signed.headers },
798-
...(signed.body !== undefined ? { body: new Uint8Array(signed.body) } : {}),
799-
timeout: Math.max(1, Math.floor(remainingMs)),
800-
maxResponseBytes: request.maxResponseBytes,
801-
maxRedirects: 0,
802-
signal: deadline.signal,
803-
profile: 'configuredEndpoint',
804-
logUrlValidationDetails: false,
805-
},
806-
'OCI destination'
807-
),
810+
response = await secureOciFetch(
811+
signed.url,
812+
{
813+
method: request.method,
814+
headers: { ...signed.headers },
815+
...(signed.body !== undefined ? { body: new Uint8Array(signed.body) } : {}),
816+
timeout: Math.max(1, Math.floor(remainingMs)),
817+
maxResponseBytes: request.maxResponseBytes,
818+
maxRedirects: 0,
819+
signal: deadline.signal,
820+
profile: 'configuredEndpoint',
821+
logUrlValidationDetails: false,
822+
},
823+
'OCI destination',
808824
deadline.signal
809825
)
810826
} catch (error) {
@@ -914,22 +930,19 @@ export async function verifyOciApiKeyCredentialForSetup(
914930
headers: { accept: 'application/json' },
915931
signingDate: new Date(),
916932
})
917-
const response = await raceWithAbort(
918-
() =>
919-
secureFetchWithValidation(
920-
signed.url,
921-
{
922-
method: 'GET',
923-
headers: { ...signed.headers },
924-
timeout: SETUP_VERIFICATION_TIMEOUT_MS,
925-
maxResponseBytes: SETUP_VERIFICATION_RESPONSE_BYTES,
926-
maxRedirects: 0,
927-
signal: deadline.signal,
928-
profile: 'configuredEndpoint',
929-
logUrlValidationDetails: false,
930-
},
931-
'OCI credential verification destination'
932-
),
933+
const response = await secureOciFetch(
934+
signed.url,
935+
{
936+
method: 'GET',
937+
headers: { ...signed.headers },
938+
timeout: SETUP_VERIFICATION_TIMEOUT_MS,
939+
maxResponseBytes: SETUP_VERIFICATION_RESPONSE_BYTES,
940+
maxRedirects: 0,
941+
signal: deadline.signal,
942+
profile: 'configuredEndpoint',
943+
logUrlValidationDetails: false,
944+
},
945+
'OCI credential verification destination',
933946
deadline.signal
934947
)
935948
if (!response.ok) {

0 commit comments

Comments
 (0)