Skip to content

Commit 8af86c8

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(oci): preserve endpoint and failure invariants
1 parent 4716ac5 commit 8af86c8

8 files changed

Lines changed: 96 additions & 33 deletions

File tree

apps/sim/app/api/v2/credentials/[credentialId]/route.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ describe('PATCH /api/v2/credentials/[credentialId]', () => {
127127
expect(body).not.toContain('MUST_NOT_LEAK_CIPHERTEXT')
128128
})
129129

130-
it('forwards a complete OCI rotation tuple and preserves explicit passphrase clearing', async () => {
130+
it('forwards a complete OCI rotation tuple with an omitted replacement passphrase', async () => {
131131
const request = patchRequest({
132132
tenancyOcid: 'ocid1.tenancy.oc1..tenant',
133133
userOcid: 'ocid1.user.oc1..replacement',

apps/sim/lib/credentials/oci-api-key-service-account.server.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,14 @@ describe('OCI API-key credential setup', () => {
181181
expect(dependencies.encryptSecret).not.toHaveBeenCalled()
182182
})
183183

184+
it('preserves an encryption failure after successful provider verification', async () => {
185+
const encryptionFailure = new Error('internal encryption failure')
186+
dependencies.encryptSecret.mockRejectedValueOnce(encryptionFailure)
187+
188+
await expect(verifyAndEncryptOciApiKeyCredential(fields())).rejects.toBe(encryptionFailure)
189+
expect(dependencies.verifySetup).toHaveBeenCalledOnce()
190+
})
191+
184192
it('forwards cancellation and never encrypts an aborted verification', async () => {
185193
const controller = new AbortController()
186194
const reason = new DOMException('canceled', 'AbortError')

apps/sim/lib/credentials/service-account-secret.test.ts

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -217,28 +217,44 @@ describe('verifyAndBuildServiceAccountSecret', () => {
217217
expect(mockVerifyAndEncryptOci).not.toHaveBeenCalled()
218218
})
219219

220-
it('classifies OCI verification outages without exposing provider details', async () => {
221-
const { OciCredentialVerificationError } = await import(
222-
'@/lib/credentials/oci-api-key-service-account.server'
223-
)
224-
mockVerifyAndEncryptOci.mockRejectedValue(
225-
new OciCredentialVerificationError('service_unavailable')
226-
)
220+
it.each(['service_unavailable', 'invalid_response'] as const)(
221+
'classifies OCI %s failures as provider outages without exposing provider details',
222+
async (code) => {
223+
const { OciCredentialVerificationError } = await import(
224+
'@/lib/credentials/oci-api-key-service-account.server'
225+
)
226+
mockVerifyAndEncryptOci.mockRejectedValue(new OciCredentialVerificationError(code))
227227

228-
const failure = await verifyAndBuildServiceAccountSecret('oci-api-key-service-account', {
229-
tenancyOcid: 'ocid1.tenancy.oc1..tenant',
230-
userOcid: 'ocid1.user.oc1..principal',
231-
fingerprint: '00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff',
232-
privateKey: 'provider-secret-key',
233-
region: 'us-ashburn-1',
234-
}).catch((error: unknown) => error)
228+
const failure = await verifyAndBuildServiceAccountSecret('oci-api-key-service-account', {
229+
tenancyOcid: 'ocid1.tenancy.oc1..tenant',
230+
userOcid: 'ocid1.user.oc1..principal',
231+
fingerprint: '00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff',
232+
privateKey: 'provider-secret-key',
233+
region: 'us-ashburn-1',
234+
}).catch((error: unknown) => error)
235235

236-
expect(failure).toBeInstanceOf(ServiceAccountSecretError)
237-
expect(failure).toMatchObject({
238-
message: 'OCI is temporarily unavailable for credential verification',
239-
providerErrorCode: 'provider_unavailable',
240-
})
241-
expect(JSON.stringify(failure)).not.toContain('provider-secret-key')
236+
expect(failure).toBeInstanceOf(ServiceAccountSecretError)
237+
expect(failure).toMatchObject({
238+
message: 'OCI is temporarily unavailable for credential verification',
239+
providerErrorCode: 'provider_unavailable',
240+
})
241+
expect(JSON.stringify(failure)).not.toContain('provider-secret-key')
242+
}
243+
)
244+
245+
it('does not misclassify an internal OCI credential failure as rejected credentials', async () => {
246+
const internalFailure = new Error('internal encryption failure')
247+
mockVerifyAndEncryptOci.mockRejectedValue(internalFailure)
248+
249+
await expect(
250+
verifyAndBuildServiceAccountSecret('oci-api-key-service-account', {
251+
tenancyOcid: 'ocid1.tenancy.oc1..tenant',
252+
userOcid: 'ocid1.user.oc1..principal',
253+
fingerprint: '00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff',
254+
privateKey: 'provider-secret-key',
255+
region: 'us-ashburn-1',
256+
})
257+
).rejects.toBe(internalFailure)
242258
})
243259

244260
it('rejects an unknown non-empty providerId instead of persisting it as Google', async () => {

apps/sim/lib/credentials/service-account-secret.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -259,14 +259,16 @@ async function buildOciApiKeyServiceAccountSecret(
259259
}
260260
} catch (error) {
261261
if (error instanceof OciCredentialVerificationError) {
262+
const providerUnavailable =
263+
error.code === 'service_unavailable' || error.code === 'invalid_response'
262264
throw new ServiceAccountSecretError(
263-
error.code === 'service_unavailable'
265+
providerUnavailable
264266
? 'OCI is temporarily unavailable for credential verification'
265267
: 'OCI rejected the API-key credential',
266-
error.code === 'service_unavailable' ? 'provider_unavailable' : 'invalid_credentials'
268+
providerUnavailable ? 'provider_unavailable' : 'invalid_credentials'
267269
)
268270
}
269-
throw new ServiceAccountSecretError('OCI API-key credential is invalid')
271+
throw error
270272
}
271273
}
272274

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -583,10 +583,14 @@ describe('credential-bound OCI client', () => {
583583
})
584584

585585
it('discards provider messages and exposes only safe status and request IDs', async () => {
586+
const opaqueProviderSecret = 'opaque-diagnostic-secret-7f3a'
586587
mocks.secureFetch.mockResolvedValueOnce(
587588
secureResponse({
588589
status: 401,
589-
body: JSON.stringify({ message: PRIVATE_KEY, nested: { authorization: 'secret' } }),
590+
body: JSON.stringify({
591+
message: opaqueProviderSecret,
592+
nested: { authorization: 'another-opaque-secret' },
593+
}),
590594
headers: { 'opc-request-id': 'request-401' },
591595
})
592596
)
@@ -607,7 +611,8 @@ describe('credential-bound OCI client', () => {
607611
status: 401,
608612
opcRequestId: 'request-401',
609613
})
610-
expect(JSON.stringify(failure)).not.toContain('BEGIN PRIVATE KEY')
614+
expect(JSON.stringify(failure)).not.toContain(opaqueProviderSecret)
615+
expect(JSON.stringify(failure)).not.toContain('another-opaque-secret')
611616
expect(JSON.stringify(failure)).not.toContain('authorization')
612617
})
613618

@@ -785,6 +790,29 @@ describe('credential-bound OCI client', () => {
785790
expect(cancel).toHaveBeenCalled()
786791
})
787792

793+
it('propagates caller abort while reading a failed response body', async () => {
794+
const controller = new AbortController()
795+
const cancel = vi.fn()
796+
mocks.secureFetch.mockResolvedValueOnce({
797+
...secureResponse({ status: 409 }),
798+
headers: new Headers(),
799+
body: new ReadableStream<Uint8Array>({ cancel }),
800+
})
801+
const { client, endpoint } = await createPreparedClient()
802+
const pending = client.request({
803+
endpoint,
804+
method: 'GET',
805+
encodedPath: '/v1/test',
806+
signal: controller.signal,
807+
timeoutMs: 10_000,
808+
maxResponseBytes: 1024,
809+
})
810+
await vi.waitFor(() => expect(mocks.secureFetch).toHaveBeenCalledOnce())
811+
controller.abort()
812+
await expect(pending).rejects.toMatchObject({ code: 'aborted' })
813+
expect(cancel).toHaveBeenCalled()
814+
})
815+
788816
it('propagates caller abort during retry backoff', async () => {
789817
vi.useFakeTimers()
790818
mocks.backoff.mockReturnValue(1000)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,8 +583,9 @@ async function readFailureCode(
583583
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return undefined
584584
const code = (parsed as Record<string, unknown>).code
585585
return typeof code === 'string' && code.length <= 128 ? code : undefined
586-
} catch {
586+
} catch (error) {
587587
await response.body?.cancel().catch(() => {})
588+
if (signal.aborted) throw toError(signal.reason ?? error)
588589
return undefined
589590
}
590591
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,21 @@ describe('OCI endpoint policies', () => {
5656

5757
it('freezes declarative policies and derives exact static origins', () => {
5858
expect(Object.isFrozen(staticPolicy)).toBe(true)
59-
expect(resolveStaticOciEndpoint(staticPolicy, region)).toMatchObject({
59+
const endpoint = resolveStaticOciEndpoint(staticPolicy, region)
60+
expect(endpoint).toMatchObject({
6061
origin: 'https://identity.us-ashburn-1.oraclecloud.com',
6162
hostname: 'identity.us-ashburn-1.oraclecloud.com',
6263
serviceId: OCI_SERVICE_ID,
6364
serviceName: 'identity',
6465
provenance: 'static',
6566
})
67+
expect(Object.isFrozen(endpoint)).toBe(true)
68+
expect(Object.isFrozen(endpoint.region)).toBe(true)
69+
expect(Object.isFrozen(endpoint.region.realm)).toBe(true)
70+
expect(Reflect.set(endpoint, 'origin', 'https://attacker.example')).toBe(false)
71+
expect(Reflect.set(endpoint.region, 'id', 'attacker-region-1')).toBe(false)
72+
expect(endpoint.origin).toBe('https://identity.us-ashburn-1.oraclecloud.com')
73+
expect(endpoint.region.id).toBe('us-ashburn-1')
6674
})
6775

6876
it('accepts discovered resource hosts only beneath the declared service, region, and realm', () => {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,10 @@ export function getOciRegion(regionId: string): OciRegion {
182182
? REGION_REALMS[normalized as keyof typeof REGION_REALMS]
183183
: undefined
184184
if (!realmId) throw new Error('OCI region is not recognized')
185-
return {
185+
return Object.freeze({
186186
id: normalized,
187-
realm: { id: realmId, domain: REALM_DOMAINS[realmId] },
188-
}
187+
realm: Object.freeze({ id: realmId, domain: REALM_DOMAINS[realmId] }),
188+
})
189189
}
190190

191191
export function resolveEffectiveOciRegion(defaultRegion: string, override?: string): OciRegion {
@@ -312,14 +312,14 @@ function validateOciOrigin(params: {
312312
if (!hostnameMatches) {
313313
throw new Error('OCI destination hostname is not owned by the requested service')
314314
}
315-
return {
315+
return Object.freeze({
316316
origin: url.origin,
317317
hostname: url.hostname,
318318
serviceId: params.policy.serviceId,
319319
serviceName: params.policy.serviceName,
320320
region: knownRegion,
321321
provenance: params.provenance,
322-
} as OciPreparedEndpoint
322+
}) as OciPreparedEndpoint
323323
}
324324

325325
/** Resolves a static policy exclusively from its service and validated region. */

0 commit comments

Comments
 (0)