Skip to content

Commit 5055d3d

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(oci): tighten request lifecycle
1 parent b22b106 commit 5055d3d

3 files changed

Lines changed: 109 additions & 23 deletions

File tree

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

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,15 @@ describe('credential-bound OCI client', () => {
201201
})
202202

203203
it('loads only an exact credential/workspace/type/provider row before decryption', async () => {
204-
await createPreparedClient()
204+
const { client, endpoint } = await createPreparedClient()
205+
206+
await client.request({
207+
endpoint,
208+
method: 'GET',
209+
encodedPath: '/v1/test',
210+
timeoutMs: 10_000,
211+
maxResponseBytes: 1024,
212+
})
205213

206214
expect(mocks.predicates).toEqual([
207215
{ field: 'credential.id', value: 'credential-authoritative' },
@@ -490,6 +498,28 @@ describe('credential-bound OCI client', () => {
490498
expect(mocks.secureFetch).not.toHaveBeenCalled()
491499
})
492500

501+
it.each(['DELETE', 'POST', 'PUT', 'PATCH'] as const)(
502+
'rejects caller-asserted safe retries for %s before signing or transport',
503+
async (method) => {
504+
const { client, endpoint } = await createPreparedClient()
505+
const bodyFields =
506+
method === 'DELETE' ? {} : { body: new Uint8Array(), contentType: 'text/plain' }
507+
508+
await expect(
509+
client.request({
510+
endpoint,
511+
method,
512+
encodedPath: '/v1/test',
513+
...bodyFields,
514+
retry: { kind: 'safe', maxAttempts: 2 },
515+
timeoutMs: 10_000,
516+
maxResponseBytes: 1024,
517+
} as unknown as OciRequest)
518+
).rejects.toMatchObject({ code: 'invalid_request' })
519+
expect(mocks.secureFetch).not.toHaveBeenCalled()
520+
}
521+
)
522+
493523
it('does not retry unless the operation opts in', async () => {
494524
mocks.secureFetch.mockResolvedValue(
495525
secureResponse({ status: 503, body: '{"message":"secret"}' })
@@ -508,12 +538,15 @@ describe('credential-bound OCI client', () => {
508538
})
509539

510540
it('re-signs every retry while preserving exact bytes and retry token', async () => {
541+
vi.useFakeTimers()
542+
vi.setSystemTime(new Date('2026-09-03T19:00:00.000Z'))
543+
mocks.backoff.mockReturnValue(1000)
511544
mocks.secureFetch
512545
.mockResolvedValueOnce(secureResponse({ status: 503, body: '{"code":"Busy"}' }))
513546
.mockResolvedValueOnce(secureResponse({ status: 200, body: 'ok' }))
514547
const { client, endpoint } = await createPreparedClient()
515548
const body = new Uint8Array([9, 8, 7])
516-
await client.request({
549+
const pending = client.request({
517550
endpoint,
518551
method: 'PUT',
519552
encodedPath: '/v1/test',
@@ -523,6 +556,8 @@ describe('credential-bound OCI client', () => {
523556
timeoutMs: 10_000,
524557
maxResponseBytes: 1024,
525558
})
559+
await vi.advanceTimersByTimeAsync(1000)
560+
await pending
526561

527562
const first = mocks.secureFetch.mock.calls[0][2]
528563
const second = mocks.secureFetch.mock.calls[1][2]
@@ -533,6 +568,31 @@ describe('credential-bound OCI client', () => {
533568
expect(first.headers.authorization).not.toBe(second.headers.authorization)
534569
})
535570

571+
it('never manufactures future signing dates under rapid request volume', async () => {
572+
vi.useFakeTimers()
573+
const now = new Date('2026-09-03T19:00:00.000Z')
574+
vi.setSystemTime(now)
575+
mocks.secureFetch.mockImplementation(async () => secureResponse({}))
576+
const { client, endpoint } = await createPreparedClient()
577+
578+
await Promise.all(
579+
Array.from({ length: 305 }, () =>
580+
client.request({
581+
endpoint,
582+
method: 'GET',
583+
encodedPath: '/v1/test',
584+
timeoutMs: 10_000,
585+
maxResponseBytes: 1024,
586+
})
587+
)
588+
)
589+
590+
const signingDates = mocks.secureFetch.mock.calls.map(
591+
(call) => (call[2] as { headers: Record<string, string> }).headers['x-date']
592+
)
593+
expect(new Set(signingDates)).toEqual(new Set([now.toUTCString()]))
594+
})
595+
536596
it('retries only the exact internal IncorrectState 409 classification', async () => {
537597
mocks.secureFetch
538598
.mockResolvedValueOnce(secureResponse({ status: 409, body: '{"code":"IncorrectState"}' }))

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

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -47,25 +47,50 @@ import { getServiceConfigByServiceId } from '@/lib/oauth/utils'
4747

4848
export type OciRequestMethod = 'GET' | 'HEAD' | 'DELETE' | 'POST' | 'PUT' | 'PATCH'
4949

50-
export type OciRetryPolicy =
51-
| { readonly kind: 'safe'; readonly maxAttempts: number }
52-
| { readonly kind: 'tokenized'; readonly maxAttempts: number; readonly retryToken: string }
50+
export interface OciSafeRetryPolicy {
51+
readonly kind: 'safe'
52+
readonly maxAttempts: number
53+
}
54+
55+
export interface OciTokenizedRetryPolicy {
56+
readonly kind: 'tokenized'
57+
readonly maxAttempts: number
58+
readonly retryToken: string
59+
}
5360

54-
export interface OciRequest {
61+
export type OciRetryPolicy = OciSafeRetryPolicy | OciTokenizedRetryPolicy
62+
63+
interface OciRequestBase {
5564
readonly endpoint: OciPreparedEndpoint
56-
readonly method: OciRequestMethod
5765
readonly encodedPath: string
5866
readonly queryPairs?: readonly (readonly [string, string])[]
5967
readonly headers?: Readonly<Record<string, string>>
60-
readonly body?: Uint8Array
61-
readonly contentType?: string
6268
readonly timeoutMs: number
6369
readonly maxResponseBytes: number
6470
readonly responseHeaders?: readonly string[]
65-
readonly retry?: OciRetryPolicy
6671
readonly signal?: AbortSignal
6772
}
6873

74+
export type OciRequest =
75+
| (OciRequestBase & {
76+
readonly method: 'GET' | 'HEAD'
77+
readonly body?: never
78+
readonly contentType?: never
79+
readonly retry?: OciRetryPolicy
80+
})
81+
| (OciRequestBase & {
82+
readonly method: 'DELETE'
83+
readonly body?: never
84+
readonly contentType?: never
85+
readonly retry?: OciTokenizedRetryPolicy
86+
})
87+
| (OciRequestBase & {
88+
readonly method: 'POST' | 'PUT' | 'PATCH'
89+
readonly body: Uint8Array
90+
readonly contentType: string
91+
readonly retry?: OciTokenizedRetryPolicy
92+
})
93+
6994
declare const authenticatedOciResponseBrand: unique symbol
7095

7196
export interface OciAuthenticatedResponse {
@@ -121,6 +146,7 @@ interface SignedOciRequest {
121146
}
122147

123148
const BODY_METHODS: ReadonlySet<OciRequestMethod> = new Set(['POST', 'PUT', 'PATCH'])
149+
const SAFE_RETRY_METHODS: ReadonlySet<OciRequestMethod> = new Set(['GET', 'HEAD'])
124150
const REQUEST_METHODS: ReadonlySet<string> = new Set([
125151
'GET',
126152
'HEAD',
@@ -470,6 +496,7 @@ function validateRequest(request: OciRequest): {
470496
typeof request.retry !== 'object' ||
471497
Array.isArray(request.retry) ||
472498
(request.retry.kind !== 'safe' && request.retry.kind !== 'tokenized') ||
499+
(request.retry.kind === 'safe' && !SAFE_RETRY_METHODS.has(request.method)) ||
473500
Object.keys(request.retry).some(
474501
(key) =>
475502
key !== 'kind' &&
@@ -715,17 +742,19 @@ export async function createOciClient(params: CreateOciClientParams): Promise<Oc
715742
}
716743

717744
let materialPromise: Promise<OciCredentialMaterial> | undefined
718-
let lastSigningTime = 0
745+
let credentialMaterial: OciCredentialMaterial | undefined
719746
const preparedEndpoints = new WeakSet<object>()
720747
const endpointPolicies = new WeakMap<object, OciEndpointPolicy>()
721748
const responseSnapshots = new WeakMap<object, BoundResponseSnapshot>()
722749

723-
const getMaterial = () => {
750+
const getMaterial = async () => {
724751
materialPromise ??= loadCredentialMaterial({
725752
credentialId: params.credentialId,
726753
workspaceId: params.workspaceId,
727754
})
728-
return materialPromise
755+
const material = await materialPromise
756+
credentialMaterial = material
757+
return material
729758
}
730759
const assertPolicyOwner = (policy: OciEndpointPolicy) => {
731760
if (policy.serviceId !== params.serviceId) throw new OciClientError('invalid_endpoint')
@@ -734,12 +763,6 @@ export async function createOciClient(params: CreateOciClientParams): Promise<Oc
734763
const material = await getMaterial()
735764
return resolveEffectiveOciRegion(material.region, params.region)
736765
}
737-
const nextSigningDate = () => {
738-
const now = Math.max(Date.now(), lastSigningTime + 1000)
739-
lastSigningTime = now
740-
return new Date(now)
741-
}
742-
743766
const client: OciClient = {
744767
async prepareStaticEndpoint(policy) {
745768
assertPolicyOwner(policy)
@@ -781,11 +804,12 @@ export async function createOciClient(params: CreateOciClientParams): Promise<Oc
781804
}
782805
const endpointPolicy = endpointPolicies.get(request.endpoint)
783806
if (!endpointPolicy) throw new OciClientError('invalid_endpoint')
807+
const material = credentialMaterial
808+
if (!material) throw new OciClientError('invalid_endpoint')
784809
const validated = validateRequest(request)
785810
const url = buildRequestUrl(request.endpoint, request.encodedPath, validated.queryPairs)
786811
const deadline = createDeadline(request.timeoutMs, request.signal)
787812
try {
788-
const material = await getMaterial()
789813
for (let attempt = 1; attempt <= validated.attempts; attempt += 1) {
790814
if (deadline.signal.aborted) {
791815
throw new OciClientError(deadline.expired() ? 'deadline_exceeded' : 'aborted')
@@ -802,7 +826,7 @@ export async function createOciClient(params: CreateOciClientParams): Promise<Oc
802826
},
803827
body: validated.body,
804828
contentType: request.contentType,
805-
signingDate: nextSigningDate(),
829+
signingDate: new Date(),
806830
})
807831

808832
let response: SecureFetchResponse

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,10 @@ export type OciEndpointPolicy = OciStaticEndpointPolicy | OciDiscoveredEndpointP
5757
/**
5858
* Realm and region snapshot copied from `oci-common@2.140.0` files
5959
* `lib/realm.js` and `lib/region.js`, and verified byte-for-byte against the
60-
* same registry files in `2.140.1`. Unknown runtime metadata is deliberately
61-
* excluded so credentials cannot weaken endpoint trust with local OCI config.
60+
* same registry files in `2.140.1`. When Oracle adds regions or realms, update
61+
* both maps from the official SDK in one reviewed change and keep the exhaustive
62+
* registry test passing. Unknown runtime metadata is deliberately excluded so
63+
* credentials cannot weaken endpoint trust with local OCI config.
6264
*/
6365
const REALM_DOMAINS = {
6466
oc1: 'oraclecloud.com',

0 commit comments

Comments
 (0)