Skip to content

Commit c000b7b

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(oracle-epm): address follow-up review findings
1 parent 7ba3213 commit c000b7b

10 files changed

Lines changed: 204 additions & 23 deletions

File tree

apps/sim/lib/credentials/client-credential-accounts/minters/oracle-epm.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ describe('mintOracleEpmServiceAccountToken', () => {
3333
{ clientId: 'user:name', clientSecret: 'password' },
3434
{ clientId: 'user\nname', clientSecret: 'password' },
3535
{ clientId: 'user', clientSecret: 'pass\nword' },
36+
{ clientId: 'user\uD800', clientSecret: 'password' },
37+
{ clientId: 'user', clientSecret: 'password\uDC00' },
3638
{ clientId: '', clientSecret: 'password' },
3739
])('rejects unsafe Basic credential text', async (credentials) => {
3840
await expect(
@@ -43,6 +45,15 @@ describe('mintOracleEpmServiceAccountToken', () => {
4345
).rejects.toBeInstanceOf(TokenServiceAccountValidationError)
4446
})
4547

48+
it('preserves valid surrogate pairs in Basic credential values', async () => {
49+
const result = await mintOracleEpmServiceAccountToken({
50+
orgId: 'https://epm.example.com',
51+
clientId: 'integration-😀',
52+
clientSecret: 'password-🔒',
53+
})
54+
expect(Buffer.from(result.accessToken, 'base64').toString()).toBe('integration-😀:password-🔒')
55+
})
56+
4657
it('does not reflect secrets in validation errors', async () => {
4758
const secret = 'password-with-newline\n'
4859
const error = await mintOracleEpmServiceAccountToken({

apps/sim/lib/credentials/client-credential-accounts/minters/oracle-epm.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const SYNTHETIC_TOKEN_TTL_SECONDS = 600
1313
const MAX_USERNAME_BYTES = 255
1414
const MAX_AUTH_VALUE_BYTES = 1_024
1515
const FORBIDDEN_CREDENTIAL_TEXT = /[\u0000-\u001f\u007f]/
16+
const MALFORMED_UTF16 = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/
1617

1718
function invalidCredentials(reason: string): TokenServiceAccountValidationError {
1819
return new TokenServiceAccountValidationError('invalid_credentials', 400, {
@@ -49,13 +50,15 @@ export async function mintOracleEpmServiceAccountToken(
4950
!username ||
5051
username.includes(':') ||
5152
FORBIDDEN_CREDENTIAL_TEXT.test(username) ||
53+
MALFORMED_UTF16.test(username) ||
5254
Buffer.byteLength(username, 'utf8') > MAX_USERNAME_BYTES
5355
) {
5456
throw invalidCredentials('integration username is invalid')
5557
}
5658
if (
5759
!password ||
5860
FORBIDDEN_CREDENTIAL_TEXT.test(password) ||
61+
MALFORMED_UTF16.test(password) ||
5962
Buffer.byteLength(password, 'utf8') > MAX_AUTH_VALUE_BYTES
6063
) {
6164
throw invalidCredentials('password is invalid')

apps/sim/lib/internal/oracle-epm/client.server.test.ts

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/** @vitest-environment node */
22
import { beforeEach, describe, expect, it, vi } from 'vitest'
3+
import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
34

45
const { mockSecureFetch, mockValidateUrl } = vi.hoisted(() => ({
56
mockSecureFetch: vi.fn(),
@@ -93,7 +94,7 @@ describe('Oracle EPM guarded client', () => {
9394
{ logDetails: false }
9495
)
9596
expect(mockSecureFetch).toHaveBeenCalledWith(
96-
expect.any(String),
97+
'https://epm.example.com/gateway/acme/SyntheticAlpha/rest/v3/jobs/job%20with%20spaces?limit=25',
9798
'203.0.113.10',
9899
expect.objectContaining({
99100
method: 'GET',
@@ -146,6 +147,50 @@ describe('Oracle EPM guarded client', () => {
146147
expect(mockValidateUrl).not.toHaveBeenCalled()
147148
})
148149

150+
it('preserves valid surrogate pairs in encoded path and query parameters', async () => {
151+
const endpoint = routes.defineEndpoint({
152+
method: 'GET',
153+
version: 'v3',
154+
path: [oracleEpmLiteral('files'), oracleEpmPathParameter('fileId', { maxBytes: 32 })],
155+
query: { label: oracleEpmQuery.string({ maxBytes: 32 }) },
156+
body: 'none',
157+
response: 'json',
158+
timeoutMs: 2_000,
159+
maxResponseBytes: 1_024,
160+
})
161+
const client = createOracleEpmClient({
162+
instanceUrl: 'https://epm.example.com',
163+
accessToken: Buffer.from('u:p').toString('base64'),
164+
})
165+
await client.request(endpoint, {
166+
pathParams: { fileId: 'report-😀' },
167+
query: { label: 'locked-🔒' },
168+
})
169+
expect(mockSecureFetch.mock.calls[0][0]).toBe(
170+
'https://epm.example.com/SyntheticAlpha/rest/v3/files/report-%F0%9F%98%80?label=locked-%F0%9F%94%92'
171+
)
172+
})
173+
174+
it('preserves streamed response size failures as payload-too-large errors', async () => {
175+
mockSecureFetch.mockResolvedValue({
176+
...secureResponse({}),
177+
json: vi.fn().mockRejectedValue(
178+
new PayloadSizeLimitError({
179+
label: 'secure fetch response',
180+
maxBytes: 4_096,
181+
observedBytes: 4_097,
182+
})
183+
),
184+
})
185+
const client = createOracleEpmClient({
186+
instanceUrl: 'https://epm.example.com',
187+
accessToken: Buffer.from('u:p').toString('base64'),
188+
})
189+
await expect(client.request(getJob, { pathParams: { jobId: '42' } })).rejects.toMatchObject({
190+
category: 'payload_too_large',
191+
})
192+
})
193+
149194
it('suppresses arbitrary provider bodies in failed requests', async () => {
150195
mockSecureFetch.mockResolvedValue(
151196
secureResponse({
@@ -310,6 +355,8 @@ describe('Oracle EPM guarded client', () => {
310355
'https://epm.example.com/gateway/SyntheticAlpha/rest/v3/files/abc?token=x&token=y',
311356
'https://epm.example.com/gateway/SyntheticAlpha/rest/v3/files/abc?unknown=x',
312357
'https://epm.example.com/gateway/SyntheticAlpha/rest/v3/files/abc?token=x#fragment',
358+
'https://epm.example.com/gateway/SyntheticAlpha/rest/v3/files/ab\nc?token=x',
359+
'https://epm.example.com/gateway/SyntheticAlpha/rest/v3/files/\uD800?token=x',
313360
])('rejects unsafe returned link %j', (href) => {
314361
const policy = routes.defineReturnedLinkPolicy({
315362
relation: 'download',

apps/sim/lib/internal/oracle-epm/client.server.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ import type {
3636
} from '@/lib/internal/oracle-epm/types'
3737

3838
const SAFE_TOKEN = /^[A-Za-z0-9+/]+={0,2}$/
39-
const LONE_SURROGATE = /[\uD800-\uDFFF]/
39+
const MALFORMED_UTF16 = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/
40+
const FORBIDDEN_LINK_TEXT = /[\u0000-\u001f\u007f]/
4041
const validatedLinks = new WeakMap<
4142
object,
4243
{ owner: object; url: string; policy: OracleEpmReturnedLinkPolicyDefinition }
@@ -64,7 +65,7 @@ function validatePathValue(
6465
value === '.' ||
6566
value === '..' ||
6667
/[/\\\u0000-\u001f\u007f]/.test(value) ||
67-
LONE_SURROGATE.test(value) ||
68+
MALFORMED_UTF16.test(value) ||
6869
Buffer.byteLength(value, 'utf8') > declaration.maxBytes ||
6970
(declaration.pattern && !declaration.pattern.test(value))
7071
) {
@@ -93,7 +94,7 @@ function serializeQueryValue(value: unknown, declaration: OracleEpmQueryParamete
9394
if (declaration.kind === 'string') {
9495
if (
9596
typeof value !== 'string' ||
96-
LONE_SURROGATE.test(value) ||
97+
MALFORMED_UTF16.test(value) ||
9798
Buffer.byteLength(value, 'utf8') > declaration.maxBytes ||
9899
(declaration.pattern && !declaration.pattern.test(value))
99100
)
@@ -152,7 +153,7 @@ function buildHeaders(
152153
}
153154
if (
154155
/\r|\n|\u0000/.test(value) ||
155-
LONE_SURROGATE.test(value) ||
156+
MALFORMED_UTF16.test(value) ||
156157
Buffer.byteLength(value, 'utf8') > declaration.maxBytes ||
157158
(declaration.pattern && !declaration.pattern.test(value))
158159
)
@@ -305,7 +306,8 @@ async function projectResponse(
305306
try {
306307
const data = await response.json()
307308
return Object.freeze({ status: response.status, data, correlationId })
308-
} catch {
309+
} catch (error) {
310+
if (isPayloadSizeLimitError(error)) throw oracleEpmLocalError('payload_too_large')
309311
throw oracleEpmLocalError('invalid_response')
310312
}
311313
}
@@ -464,7 +466,9 @@ export function createOracleEpmClient(input: {
464466
link.rel !== policy.relation ||
465467
(link.method !== undefined && link.method !== policy.method) ||
466468
typeof link.href !== 'string' ||
467-
link.href.length > 8_192
469+
link.href.length > 8_192 ||
470+
FORBIDDEN_LINK_TEXT.test(link.href) ||
471+
MALFORMED_UTF16.test(link.href)
468472
)
469473
throw oracleEpmLocalError('invalid_input')
470474
let url: URL

apps/sim/lib/internal/oracle-epm/destination.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ describe('Oracle EPM destination', () => {
3838
'https://epm.example.com/a%2Fb',
3939
'https:////epm.example.com/gateway',
4040
'https://epm.example.com/a\\b',
41+
'https://epm.example.com/gateway/\uD800',
42+
'https://epm.example.com/gateway/\uDC00',
4143
])('rejects unsafe destination %j', (value) => {
4244
expect(() => defineOracleEpmDestination(value)).toThrow()
4345
})

apps/sim/lib/internal/oracle-epm/destination.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const MAX_DESTINATION_LENGTH = 2_048
44
const MAX_PATH_SEGMENTS = 32
55
const MAX_PATH_SEGMENT_BYTES = 255
66
const FORBIDDEN_TEXT = /[\u0000-\u001f\u007f\\]/
7+
const MALFORMED_UTF16 = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/
78
const destinations = new WeakMap<object, { origin: string; baseSegments: readonly string[] }>()
89

910
function decodeSegment(segment: string): string {
@@ -33,6 +34,7 @@ function validateAndDecodeSegment(encoded: string): string {
3334
safetyValue === '..' ||
3435
safetyValue.includes('/') ||
3536
FORBIDDEN_TEXT.test(safetyValue) ||
37+
MALFORMED_UTF16.test(decoded) ||
3638
Buffer.byteLength(decoded, 'utf8') > MAX_PATH_SEGMENT_BYTES
3739
) {
3840
throw new Error('Oracle EPM environment URL base path is invalid')
@@ -43,7 +45,12 @@ function validateAndDecodeSegment(encoded: string): string {
4345
/** Validates and freezes the credential-bound Oracle EPM environment URL. */
4446
export function defineOracleEpmDestination(rawUrl: string): OracleEpmDestination {
4547
const value = rawUrl.trim()
46-
if (!value || value.length > MAX_DESTINATION_LENGTH || FORBIDDEN_TEXT.test(value)) {
48+
if (
49+
!value ||
50+
value.length > MAX_DESTINATION_LENGTH ||
51+
FORBIDDEN_TEXT.test(value) ||
52+
MALFORMED_UTF16.test(value)
53+
) {
4754
throw new Error('Oracle EPM environment URL is invalid')
4855
}
4956

apps/sim/lib/internal/oracle-epm/files.server.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,74 @@ describe('Oracle EPM file primitives', () => {
187187
expect(destroy).toHaveBeenCalled()
188188
})
189189

190+
it('destroys a source canceled while its storage stream is opening', async () => {
191+
let finishOpen: ((stream: Readable) => void) | undefined
192+
mocks.downloadFileStream.mockReturnValue(
193+
new Promise((resolve) => {
194+
finishOpen = resolve
195+
})
196+
)
197+
const controller = new AbortController()
198+
const source = await openOracleEpmSourceFile({
199+
file: {
200+
id: 'f',
201+
name: 'x',
202+
url: '',
203+
size: 0,
204+
type: '',
205+
key: 'workspace/key',
206+
context: 'workspace',
207+
},
208+
userId: 'user-1',
209+
maxBytes: 3,
210+
signal: controller.signal,
211+
})
212+
const pending = (async () => {
213+
for await (const _chunk of source.chunks) {
214+
// Consume the guarded stream.
215+
}
216+
})()
217+
await vi.waitFor(() => expect(mocks.downloadFileStream).toHaveBeenCalled())
218+
controller.abort(new DOMException('user', 'AbortError'))
219+
const stream = Readable.from([])
220+
const destroy = vi.spyOn(stream, 'destroy')
221+
finishOpen?.(stream)
222+
223+
await expect(pending).rejects.toMatchObject({ name: 'AbortError' })
224+
expect(destroy).toHaveBeenCalled()
225+
})
226+
227+
it('rejects a source canceled as an empty storage stream reaches EOF', async () => {
228+
const controller = new AbortController()
229+
const stream = new Readable({
230+
read() {
231+
this.push(null)
232+
controller.abort(new DOMException('user', 'AbortError'))
233+
},
234+
})
235+
mocks.downloadFileStream.mockResolvedValue(stream)
236+
const source = await openOracleEpmSourceFile({
237+
file: {
238+
id: 'f',
239+
name: 'x',
240+
url: '',
241+
size: 0,
242+
type: '',
243+
key: 'workspace/key',
244+
context: 'workspace',
245+
},
246+
userId: 'user-1',
247+
maxBytes: 3,
248+
signal: controller.signal,
249+
})
250+
251+
await expect(async () => {
252+
for await (const _chunk of source.chunks) {
253+
// Consume the guarded stream.
254+
}
255+
}).rejects.toMatchObject({ name: 'AbortError' })
256+
})
257+
190258
it('streams a bounded provider response into execution storage and returns UserFile', async () => {
191259
const body = new ReadableStream<Uint8Array>({
192260
start(controller) {

apps/sim/lib/internal/oracle-epm/files.server.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ export async function openOracleEpmSourceFile(input: {
7979
const abort = () => stream.destroy(signal?.reason)
8080
signal?.addEventListener('abort', abort, { once: true })
8181
try {
82+
signal?.throwIfAborted()
8283
for await (const chunk of stream) {
8384
signal?.throwIfAborted()
8485
const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk as Uint8Array)
@@ -92,6 +93,7 @@ export async function openOracleEpmSourceFile(input: {
9293
}
9394
yield buffer
9495
}
96+
signal?.throwIfAborted()
9597
} finally {
9698
signal?.removeEventListener('abort', abort)
9799
stream.destroy()

apps/sim/lib/oauth/token-resolution.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,52 @@ describe('resolveCredentialAccessToken', () => {
492492
})
493493
})
494494

495+
it('preserves an existing service-account tool without OAuth service metadata', async () => {
496+
mockResolveOAuthAccountId.mockResolvedValue({
497+
accountId: 'credential-1',
498+
credentialId: 'credential-1',
499+
credentialType: 'service_account',
500+
providerId: 'claude-platform-service-account',
501+
usedCredentialTable: true,
502+
})
503+
mockGetToolMetadata.mockReturnValue({ oauth: undefined })
504+
mockAuthorizeCredentialUseForAuth.mockResolvedValue({
505+
ok: true,
506+
requesterUserId: 'user-1',
507+
credentialOwnerUserId: 'owner-1',
508+
workspaceId: 'ws-1',
509+
resolvedCredentialId: 'credential-1',
510+
})
511+
mockResolveServiceAccountToken.mockResolvedValue({ accessToken: 'workspace-api-key' })
512+
513+
await expect(
514+
resolveCredentialAccessToken({
515+
requestId: 'req-1',
516+
credentialId: 'credential-1',
517+
toolId: 'managed_agent_run_session',
518+
authenticate,
519+
})
520+
).resolves.toEqual({
521+
ok: true,
522+
token: {
523+
accessToken: 'workspace-api-key',
524+
credentialType: 'service_account',
525+
apiDomain: undefined,
526+
authStyle: undefined,
527+
cloudId: undefined,
528+
domain: undefined,
529+
instanceUrl: undefined,
530+
},
531+
})
532+
expect(mockGetServiceConfigByProviderId).not.toHaveBeenCalled()
533+
expect(mockResolveServiceAccountToken).toHaveBeenCalledWith(
534+
'credential-1',
535+
'claude-platform-service-account',
536+
[],
537+
undefined
538+
)
539+
})
540+
495541
it('rejects a mismatched OAuth account after loading its authoritative provider', async () => {
496542
mockResolveOAuthAccountId.mockResolvedValue({
497543
accountId: 'account-1',

0 commit comments

Comments
 (0)