Skip to content

Commit 6daba17

Browse files
Bill LeoutsakosBill Leoutsakos
Bill Leoutsakos
authored and
Bill Leoutsakos
committed
Align Oracle integration with repository audit contracts
1 parent 02fdecb commit 6daba17

18 files changed

Lines changed: 66 additions & 90 deletions

‎apps/sim/lib/internal/oracledb/client.test.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ class FakeChild extends EventEmitter {
7272

7373
describe('Oracle worker client', () => {
7474
beforeEach(() => {
75-
vi.clearAllMocks()
7675
proxyMocks.close.mockResolvedValue(undefined)
7776
proxyMocks.createOracleConnectProxy.mockResolvedValue({
7877
host: '127.0.0.1',

‎apps/sim/lib/internal/oracledb/client.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ function workerSemaphore(): OracleWorkerSemaphore {
4747
return carrier[ORACLE_WORKER_SEMAPHORE]
4848
}
4949

50-
export class OracleWorkerError extends Error {}
50+
class OracleWorkerError extends Error {}
5151

5252
function cleanupWorkerWaiter(waiter: WorkerSlotWaiter): void {
53-
waiter.signal?.removeEventListener('abort', waiter.onAbort!)
53+
if (waiter.onAbort) waiter.signal?.removeEventListener('abort', waiter.onAbort)
5454
if (waiter.timeout) clearTimeout(waiter.timeout)
5555
const waiters = workerSemaphore().waiters
5656
const index = waiters.indexOf(waiter)
@@ -60,7 +60,8 @@ function cleanupWorkerWaiter(waiter: WorkerSlotWaiter): void {
6060
function releaseWorkerSlot(): void {
6161
const semaphore = workerSemaphore()
6262
while (semaphore.waiters.length > 0) {
63-
const waiter = semaphore.waiters.shift()!
63+
const waiter = semaphore.waiters.shift()
64+
if (!waiter) break
6465
cleanupWorkerWaiter(waiter)
6566
if (waiter.signal?.aborted) {
6667
waiter.reject(waiter.signal.reason)

‎apps/sim/lib/internal/oracledb/connect-proxy.test.ts‎

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@
44

55
import { once } from 'node:events'
66
import net from 'node:net'
7+
import {
8+
inputValidationMock,
9+
inputValidationMockFns,
10+
} from '@sim/testing/mocks/input-validation.mock'
711
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
812

9-
const validationMocks = vi.hoisted(() => ({
10-
validateDatabaseHost: vi.fn(),
11-
}))
12-
13-
vi.mock('@/lib/core/security/input-validation.server', () => validationMocks)
13+
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
1414

1515
import {
1616
createOracleConnectProxy,
@@ -55,10 +55,9 @@ async function listen(server: net.Server): Promise<number> {
5555
}
5656

5757
describe('Oracle loopback CONNECT proxy', () => {
58-
beforeEach(() => vi.clearAllMocks())
58+
beforeEach(() => inputValidationMockFns.mockValidateDatabaseHost.mockReset())
5959

6060
afterEach(async () => {
61-
vi.restoreAllMocks()
6261
await Promise.all(
6362
Array.from(servers, (server) =>
6463
server.listening
@@ -72,7 +71,7 @@ describe('Oracle loopback CONNECT proxy', () => {
7271
it('validates the requested target and dials only the pinned IP', async () => {
7372
const upstream = net.createServer((socket) => socket.pipe(socket))
7473
const upstreamPort = await listen(upstream)
75-
validationMocks.validateDatabaseHost.mockResolvedValue({
74+
inputValidationMockFns.mockValidateDatabaseHost.mockResolvedValue({
7675
isValid: true,
7776
resolvedIP: '127.0.0.1',
7877
originalHostname: 'redirect.example.com',
@@ -86,7 +85,7 @@ describe('Oracle loopback CONNECT proxy', () => {
8685
client.write('oracle-net-payload')
8786
const [echo] = (await once(client, 'data')) as [Buffer]
8887
expect(echo.toString()).toBe('oracle-net-payload')
89-
expect(validationMocks.validateDatabaseHost).toHaveBeenCalledWith(
88+
expect(inputValidationMockFns.mockValidateDatabaseHost).toHaveBeenCalledWith(
9089
'redirect.example.com',
9190
'Oracle Database host',
9291
{ logDetails: false }
@@ -96,7 +95,7 @@ describe('Oracle loopback CONNECT proxy', () => {
9695
})
9796

9897
it('blocks a private or otherwise denied redirect before dialing it', async () => {
99-
validationMocks.validateDatabaseHost.mockResolvedValue({
98+
inputValidationMockFns.mockValidateDatabaseHost.mockResolvedValue({
10099
isValid: false,
101100
error: 'Oracle Database host resolves to a blocked address',
102101
})
@@ -115,7 +114,7 @@ describe('Oracle loopback CONNECT proxy', () => {
115114
it('revalidates every redirect tunnel instead of trusting the first destination', async () => {
116115
const upstream = net.createServer((socket) => socket.pipe(socket))
117116
const upstreamPort = await listen(upstream)
118-
validationMocks.validateDatabaseHost
117+
inputValidationMockFns.mockValidateDatabaseHost
119118
.mockResolvedValueOnce({
120119
isValid: true,
121120
resolvedIP: '127.0.0.1',
@@ -140,10 +139,9 @@ describe('Oracle loopback CONNECT proxy', () => {
140139
redirect.write(`CONNECT redirect.example.com:${upstreamPort} HTTP/1.1\r\n\r\n`)
141140
const [redirectResponse] = (await once(redirect, 'data')) as [Buffer]
142141
expect(redirectResponse.toString()).toContain('403 CONNECT Target Blocked')
143-
expect(validationMocks.validateDatabaseHost.mock.calls.map(([host]) => host)).toEqual([
144-
'initial.example.com',
145-
'redirect.example.com',
146-
])
142+
expect(
143+
inputValidationMockFns.mockValidateDatabaseHost.mock.calls.map(([host]) => host)
144+
).toEqual(['initial.example.com', 'redirect.example.com'])
147145
redirect.destroy()
148146
await proxy.close()
149147
})
@@ -162,7 +160,7 @@ describe('Oracle loopback CONNECT proxy', () => {
162160
it('rejects oversized or ambiguous CONNECT authorities before validation', () => {
163161
expect(oracleConnectProxyInternals.parseConnectTarget(`${'a'.repeat(1021)}:1521`)).toBeNull()
164162
expect(oracleConnectProxyInternals.parseConnectTarget('db.example.com /:1521')).toBeNull()
165-
expect(validationMocks.validateDatabaseHost).not.toHaveBeenCalled()
163+
expect(inputValidationMockFns.mockValidateDatabaseHost).not.toHaveBeenCalled()
166164
})
167165

168166
it('bounds the complete HTTP CONNECT header', async () => {
@@ -173,12 +171,12 @@ describe('Oracle loopback CONNECT proxy', () => {
173171
)
174172

175173
expect(response).toContain('431 Request Header Fields Too Large')
176-
expect(validationMocks.validateDatabaseHost).not.toHaveBeenCalled()
174+
expect(inputValidationMockFns.mockValidateDatabaseHost).not.toHaveBeenCalled()
177175
await proxy.close()
178176
})
179177

180178
it('allows at most eight redirect tunnels for one operation', async () => {
181-
validationMocks.validateDatabaseHost.mockResolvedValue({
179+
inputValidationMockFns.mockValidateDatabaseHost.mockResolvedValue({
182180
isValid: false,
183181
error: 'blocked for test',
184182
})
@@ -193,7 +191,7 @@ describe('Oracle loopback CONNECT proxy', () => {
193191
sendRawConnect(proxy.port, 'CONNECT redirect-8.example.com:1521 HTTP/1.1\r\n\r\n')
194192
).resolves.toContain('429 Too Many CONNECT Requests')
195193

196-
expect(validationMocks.validateDatabaseHost).toHaveBeenCalledTimes(8)
194+
expect(inputValidationMockFns.mockValidateDatabaseHost).toHaveBeenCalledTimes(8)
197195
expect(proxy.getFailureReason()).toContain('redirect tunnel limit')
198196
await proxy.close()
199197
})
@@ -205,7 +203,7 @@ describe('Oracle loopback CONNECT proxy', () => {
205203
socket.once('close', () => upstreamSockets.delete(socket))
206204
})
207205
const upstreamPort = await listen(upstream)
208-
validationMocks.validateDatabaseHost.mockResolvedValue({
206+
inputValidationMockFns.mockValidateDatabaseHost.mockResolvedValue({
209207
isValid: true,
210208
resolvedIP: '127.0.0.1',
211209
originalHostname: 'db.example.com',
@@ -226,15 +224,15 @@ describe('Oracle loopback CONNECT proxy', () => {
226224
`CONNECT db.example.com:${upstreamPort} HTTP/1.1\r\n\r\n`
227225
)
228226
expect(overflowResponse).not.toContain('200 Connection Established')
229-
expect(validationMocks.validateDatabaseHost).toHaveBeenCalledTimes(2)
227+
expect(inputValidationMockFns.mockValidateDatabaseHost).toHaveBeenCalledTimes(2)
230228

231229
for (const client of clients) client.destroy()
232230
for (const socket of upstreamSockets) socket.destroy()
233231
await proxy.close()
234232
})
235233

236234
it('times out a stalled upstream dial and cleans up both sockets', async () => {
237-
validationMocks.validateDatabaseHost.mockResolvedValue({
235+
inputValidationMockFns.mockValidateDatabaseHost.mockResolvedValue({
238236
isValid: true,
239237
resolvedIP: '203.0.113.10',
240238
originalHostname: 'db.example.com',
@@ -272,7 +270,7 @@ describe('Oracle loopback CONNECT proxy', () => {
272270
})
273271
const upstream = net.createServer((socket) => resolveUpstream?.(socket))
274272
const upstreamPort = await listen(upstream)
275-
validationMocks.validateDatabaseHost.mockResolvedValue({
273+
inputValidationMockFns.mockValidateDatabaseHost.mockResolvedValue({
276274
isValid: true,
277275
resolvedIP: '127.0.0.1',
278276
originalHostname: 'db.example.com',

‎apps/sim/lib/internal/oracledb/execute-tool.test.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { createExecutionContext } from '@sim/testing'
5-
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { describe, expect, it, vi } from 'vitest'
66

77
const operationMocks = vi.hoisted(() => {
88
class OracleOperationInputError extends Error {}
@@ -105,8 +105,6 @@ const EXECUTION_MOCKS = [
105105
]
106106

107107
describe('executeOracledbTool', () => {
108-
beforeEach(() => vi.clearAllMocks())
109-
110108
it('validates and executes Query with cancellation', async () => {
111109
const controller = new AbortController()
112110
operationMocks.executeOracleQuery.mockResolvedValue({

‎apps/sim/lib/internal/oracledb/execute-tool.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
} from '@/lib/internal/oracledb/operations'
1212
import {
1313
oracleDeleteInputSchema,
14-
oracleExecuteInputSchema,
1514
oracleInsertInputSchema,
1615
oracleIntrospectInputSchema,
1716
oracleQueryInputSchema,
@@ -68,7 +67,7 @@ export const executeOracledbTool: InternalToolOperationHandler = async ({
6867
)
6968
case 'oracledb_execute':
7069
return executeOperation(
71-
oracleExecuteInputSchema,
70+
oracleQueryInputSchema,
7271
input,
7372
executeOracleStatement,
7473
'Oracle Database execute failed',

‎apps/sim/lib/internal/oracledb/introspection.test.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
import { describe, expect, it, vi } from 'vitest'
55

66
const clientMocks = vi.hoisted(() => ({
77
executeOracleStatements: vi.fn(),
@@ -26,8 +26,6 @@ const CONNECTION = {
2626
} as const
2727

2828
describe('Oracle Database introspection', () => {
29-
beforeEach(() => vi.clearAllMocks())
30-
3129
it('formats dimensions for common Oracle column types', () => {
3230
expect(
3331
oracleIntrospectionInternals.formatOracleType({

‎apps/sim/lib/internal/oracledb/operations.test.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
import { describe, expect, it, vi } from 'vitest'
55

66
const clientMocks = vi.hoisted(() => ({
77
executeOracleStatements: vi.fn(),
@@ -33,8 +33,6 @@ const CONNECTION = {
3333
} as const
3434

3535
describe('Oracle Database operations', () => {
36-
beforeEach(() => vi.clearAllMocks())
37-
3836
it('runs Query in one read-only worker request and forwards cancellation', async () => {
3937
const controller = new AbortController()
4038
clientMocks.executeOracleStatements.mockResolvedValue([

‎apps/sim/lib/internal/oracledb/operations.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,3 @@ export async function executeOracleIntrospection(
175175
schemas: result.schemas,
176176
}
177177
}
178-
179-
export const oracleOperationInternals = {
180-
connectionFromInput,
181-
}

‎apps/sim/lib/internal/oracledb/oracle-worker.cjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ let requestConnection
1919
class RowLimitError extends Error {}
2020

2121
function isRecord(value) {
22+
// utils-lint-allow: This standalone Node worker ships without the @sim/utils package.
2223
return typeof value === 'object' && value !== null && !Array.isArray(value)
2324
}
2425

‎apps/sim/lib/internal/oracledb/public-contract.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,11 @@ const operationFields = {
4646
} as const
4747

4848
describe('Oracle Database public integration contract', () => {
49-
const buildParams = OracleDatabaseBlock.tools.config.params!
50-
const selectTool = OracleDatabaseBlock.tools.config.tool!
49+
const buildParams = OracleDatabaseBlock.tools.config.params
50+
const selectTool = OracleDatabaseBlock.tools.config.tool
51+
if (typeof buildParams !== 'function' || typeof selectTool !== 'function') {
52+
throw new Error('Oracle Database block must declare tool and parameter builders')
53+
}
5154

5255
const connection = {
5356
host: 'db.example.com',

0 commit comments

Comments
 (0)