Skip to content

Commit d184331

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(oci): close transport review gaps
1 parent a393f2d commit d184331

4 files changed

Lines changed: 67 additions & 8 deletions

File tree

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,22 @@ describe('credential-bound OCI client', () => {
587587
expect(mocks.secureFetch).toHaveBeenCalledOnce()
588588
})
589589

590+
it('retries the bounded transport timeout', async () => {
591+
mocks.secureFetch
592+
.mockRejectedValueOnce(new Error('Request timed out after 9000ms'))
593+
.mockResolvedValueOnce(secureResponse({ status: 200 }))
594+
const { client, endpoint } = await createPreparedClient()
595+
await client.request({
596+
endpoint,
597+
method: 'GET',
598+
encodedPath: '/v1/test',
599+
retry: { kind: 'safe', maxAttempts: 2 },
600+
timeoutMs: 10_000,
601+
maxResponseBytes: 1024,
602+
})
603+
expect(mocks.secureFetch).toHaveBeenCalledTimes(2)
604+
})
605+
590606
it('discards provider messages and exposes only safe status and request IDs', async () => {
591607
const opaqueProviderSecret = 'opaque-diagnostic-secret-7f3a'
592608
mocks.secureFetch.mockResolvedValueOnce(
@@ -729,6 +745,37 @@ describe('credential-bound OCI client', () => {
729745
).rejects.toMatchObject({ code: 'invalid_endpoint' })
730746
})
731747

748+
it('prepares discovery from the retained safe Location header', async () => {
749+
const policy = createOciDiscoveredEndpointPolicy({
750+
serviceId: OCI_SERVICE_ID,
751+
serviceName: 'database',
752+
hostnameTemplate: 'regional',
753+
responsePolicy: STATIC_POLICY,
754+
source: { kind: 'header', name: 'location' },
755+
})
756+
const { client, endpoint } = await createPreparedClient()
757+
mocks.secureFetch.mockResolvedValueOnce(
758+
secureResponse({
759+
headers: {
760+
location: 'https://resource.database.us-ashburn-1.oraclecloud.com',
761+
'x-provider-secret': 'hidden',
762+
},
763+
})
764+
)
765+
const response = await client.request({
766+
endpoint,
767+
method: 'GET',
768+
encodedPath: '/v1/test',
769+
responseHeaders: ['location'],
770+
timeoutMs: 10_000,
771+
maxResponseBytes: 1024,
772+
})
773+
expect((await client.prepareDiscoveredEndpoint(policy, response)).origin).toBe(
774+
'https://resource.database.us-ashburn-1.oraclecloud.com'
775+
)
776+
expect(response.headers).not.toHaveProperty('x-provider-secret')
777+
})
778+
732779
it('propagates caller abort without leaking a transport failure', async () => {
733780
const controller = new AbortController()
734781
mocks.secureFetch.mockImplementationOnce(

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,9 +591,12 @@ async function readFailureCode(
591591
}
592592

593593
function isRetryableTransportFailure(error: unknown): boolean {
594-
if (!error || typeof error !== 'object') return false
594+
if (!(error instanceof Error)) return false
595595
const code = (error as { code?: unknown }).code
596-
return typeof code === 'string' && RETRYABLE_TRANSPORT_CODES.has(code)
596+
return (
597+
(typeof code === 'string' && RETRYABLE_TRANSPORT_CODES.has(code)) ||
598+
/^Request timed out after \d+ms$/.test(error.message)
599+
)
597600
}
598601

599602
function extractDiscoveredOrigin(

‎apps/sim/lib/internal/oci/endpoints.test.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,14 @@ describe('OCI endpoint policies', () => {
113113
serviceName: 'database',
114114
hostnameTemplate: 'regional',
115115
responsePolicy: staticPolicy,
116-
source: { kind: 'header', name: 'Endpoint' },
116+
source: { kind: 'header', name: 'location' },
117117
allowRegionalHost: true,
118118
})
119119
expect(
120120
resolveDiscoveredOciEndpoint(policy, region, 'https://database.us-ashburn-1.oraclecloud.com')
121121
.origin
122122
).toBe('https://database.us-ashburn-1.oraclecloud.com')
123-
expect(policy.source).toEqual({ kind: 'header', name: 'endpoint' })
123+
expect(policy.source).toEqual({ kind: 'header', name: 'location' })
124124
expect(Object.isFrozen(policy.source)).toBe(true)
125125
})
126126

@@ -158,5 +158,14 @@ describe('OCI endpoint policies', () => {
158158
source: { kind: 'json', path: ['endpoint'] },
159159
})
160160
).toThrow('same owning service')
161+
expect(() =>
162+
createOciDiscoveredEndpointPolicy({
163+
serviceId: OCI_SERVICE_ID,
164+
serviceName: 'database',
165+
hostnameTemplate: 'regional',
166+
responsePolicy: staticPolicy,
167+
source: { kind: 'header', name: 'x-custom-endpoint' } as never,
168+
})
169+
).toThrow('safe Location')
161170
})
162171
})

‎apps/sim/lib/internal/oci/endpoints.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export interface OciStaticEndpointPolicy {
3838
}
3939

4040
export type OciDiscoverySource =
41-
| { readonly kind: 'header'; readonly name: string }
41+
| { readonly kind: 'header'; readonly name: 'location' }
4242
| { readonly kind: 'json'; readonly path: readonly string[] }
4343

4444
export interface OciDiscoveredEndpointPolicy {
@@ -214,8 +214,8 @@ function assertHostnameTemplate(value: OciHostnameTemplate): void {
214214

215215
function assertDiscoverySource(source: OciDiscoverySource): void {
216216
if (source.kind === 'header') {
217-
if (!/^[!#$%&'*+.^_`|~0-9A-Za-z-]+$/.test(source.name)) {
218-
throw new Error('OCI discovery header name is invalid')
217+
if (source.name !== 'location') {
218+
throw new Error('OCI discovery header must be the safe Location response header')
219219
}
220220
return
221221
}
@@ -266,7 +266,7 @@ export function createOciDiscoveredEndpointPolicy(params: {
266266
const source =
267267
params.source.kind === 'json'
268268
? Object.freeze({ ...params.source, path: Object.freeze([...params.source.path]) })
269-
: Object.freeze({ ...params.source, name: params.source.name.toLowerCase() })
269+
: Object.freeze({ ...params.source })
270270
return Object.freeze({
271271
kind: 'authenticated-discovery',
272272
serviceId: params.serviceId,

0 commit comments

Comments
 (0)