Skip to content

Commit f5df063

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(oci): enforce destination validation deadlines
1 parent d184331 commit f5df063

2 files changed

Lines changed: 96 additions & 55 deletions

File tree

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

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ import {
6767
type OciAuthenticatedResponse,
6868
type OciClient,
6969
type OciRequest,
70+
verifyOciApiKeyCredentialForSetup,
7071
} from '@/lib/internal/oci/client.server'
7172
import {
7273
createOciDiscoveredEndpointPolicy,
@@ -778,14 +779,7 @@ describe('credential-bound OCI client', () => {
778779

779780
it('propagates caller abort without leaking a transport failure', async () => {
780781
const controller = new AbortController()
781-
mocks.secureFetch.mockImplementationOnce(
782-
(_url: string, options: { signal: AbortSignal }) =>
783-
new Promise((_resolve, reject) => {
784-
options.signal.addEventListener('abort', () => reject(options.signal.reason), {
785-
once: true,
786-
})
787-
})
788-
)
782+
mocks.secureFetch.mockImplementationOnce(() => new Promise(() => {}))
789783
const { client, endpoint } = await createPreparedClient()
790784
const pending = client.request({
791785
endpoint,
@@ -801,14 +795,7 @@ describe('credential-bound OCI client', () => {
801795

802796
it('applies one deadline to in-flight transport work', async () => {
803797
vi.useFakeTimers()
804-
mocks.secureFetch.mockImplementationOnce(
805-
(_url: string, options: { signal: AbortSignal }) =>
806-
new Promise((_resolve, reject) => {
807-
options.signal.addEventListener('abort', () => reject(options.signal.reason), {
808-
once: true,
809-
})
810-
})
811-
)
798+
mocks.secureFetch.mockImplementationOnce(() => new Promise(() => {}))
812799
const { client, endpoint } = await createPreparedClient()
813800
const pending = client.request({
814801
endpoint,
@@ -822,6 +809,33 @@ describe('credential-bound OCI client', () => {
822809
await assertion
823810
})
824811

812+
it('propagates caller abort while setup destination validation is pending', async () => {
813+
const controller = new AbortController()
814+
mocks.secureFetch.mockImplementationOnce(() => new Promise(() => {}))
815+
const pending = verifyOciApiKeyCredentialForSetup(SECRET, controller.signal)
816+
await vi.waitFor(() => expect(mocks.secureFetch).toHaveBeenCalledOnce())
817+
controller.abort()
818+
await expect(pending).rejects.toMatchObject({ code: 'aborted' })
819+
})
820+
821+
it('does not start setup destination validation after an earlier caller abort', async () => {
822+
const controller = new AbortController()
823+
controller.abort()
824+
await expect(
825+
verifyOciApiKeyCredentialForSetup(SECRET, controller.signal)
826+
).rejects.toMatchObject({ code: 'aborted' })
827+
expect(mocks.secureFetch).not.toHaveBeenCalled()
828+
})
829+
830+
it('applies the setup deadline while destination validation is pending', async () => {
831+
vi.useFakeTimers()
832+
mocks.secureFetch.mockImplementationOnce(() => new Promise(() => {}))
833+
const pending = verifyOciApiKeyCredentialForSetup(SECRET)
834+
const assertion = expect(pending).rejects.toMatchObject({ code: 'deadline_exceeded' })
835+
await vi.advanceTimersByTimeAsync(10_001)
836+
await assertion
837+
})
838+
825839
it('applies the same deadline while reading the response body', async () => {
826840
vi.useFakeTimers()
827841
const cancel = vi.fn()

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

Lines changed: 66 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -654,19 +654,38 @@ function createDeadline(
654654
}
655655
}
656656

657-
async function waitForRetry(delayMs: number, signal: AbortSignal): Promise<void> {
658-
if (signal.aborted) throw toError(signal.reason)
659-
let rejectAbort: ((reason?: unknown) => void) | undefined
660-
const aborted = new Promise<never>((_, reject) => {
661-
rejectAbort = reject
657+
function raceWithAbort<T>(operation: () => Promise<T>, signal: AbortSignal): Promise<T> {
658+
if (signal.aborted) return Promise.reject(toError(signal.reason))
659+
return new Promise<T>((resolve, reject) => {
660+
const cleanup = () => signal.removeEventListener('abort', onAbort)
661+
const onAbort = () => {
662+
cleanup()
663+
reject(toError(signal.reason))
664+
}
665+
signal.addEventListener('abort', onAbort, { once: true })
666+
let pending: Promise<T>
667+
try {
668+
pending = operation()
669+
} catch (error) {
670+
cleanup()
671+
reject(error)
672+
return
673+
}
674+
pending.then(
675+
(value) => {
676+
cleanup()
677+
resolve(value)
678+
},
679+
(error: unknown) => {
680+
cleanup()
681+
reject(error)
682+
}
683+
)
662684
})
663-
const onAbort = () => rejectAbort?.(signal.reason)
664-
signal.addEventListener('abort', onAbort, { once: true })
665-
try {
666-
await Promise.race([sleep(delayMs), aborted])
667-
} finally {
668-
signal.removeEventListener('abort', onAbort)
669-
}
685+
}
686+
687+
async function waitForRetry(delayMs: number, signal: AbortSignal): Promise<void> {
688+
await raceWithAbort(() => sleep(delayMs), signal)
670689
}
671690

672691
/** Creates a lazily loaded OCI client bound to trusted workspace and service context. */
@@ -769,20 +788,24 @@ export async function createOciClient(params: CreateOciClientParams): Promise<Oc
769788

770789
let response: SecureFetchResponse
771790
try {
772-
response = await secureFetchWithValidation(
773-
signed.url,
774-
{
775-
method: request.method,
776-
headers: { ...signed.headers },
777-
...(signed.body !== undefined ? { body: new Uint8Array(signed.body) } : {}),
778-
timeout: Math.max(1, Math.floor(remainingMs)),
779-
maxResponseBytes: request.maxResponseBytes,
780-
maxRedirects: 0,
781-
signal: deadline.signal,
782-
profile: 'configuredEndpoint',
783-
logUrlValidationDetails: false,
784-
},
785-
'OCI destination'
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+
),
808+
deadline.signal
786809
)
787810
} catch (error) {
788811
if (deadline.signal.aborted) {
@@ -891,19 +914,23 @@ export async function verifyOciApiKeyCredentialForSetup(
891914
headers: { accept: 'application/json' },
892915
signingDate: new Date(),
893916
})
894-
const response = await secureFetchWithValidation(
895-
signed.url,
896-
{
897-
method: 'GET',
898-
headers: { ...signed.headers },
899-
timeout: SETUP_VERIFICATION_TIMEOUT_MS,
900-
maxResponseBytes: SETUP_VERIFICATION_RESPONSE_BYTES,
901-
maxRedirects: 0,
902-
signal: deadline.signal,
903-
profile: 'configuredEndpoint',
904-
logUrlValidationDetails: false,
905-
},
906-
'OCI credential verification destination'
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+
deadline.signal
907934
)
908935
if (!response.ok) {
909936
await readFailureCode(response, deadline.signal)

0 commit comments

Comments
 (0)