Skip to content

Commit bf20383

Browse files
committed
refactor(database-tools): wrap SQL families at registration and derive saved-connection schemas from the operations
- Register the PostgreSQL, MySQL, and SQL Server families through registerSavedDatabaseFamily, which applies withSavedDatabaseConnection and records each tool id's service. A registry test now fails when a tool that names a saved database connection runs outside a wrapped family, the case the saved-credential contract skips for operation-resolved providers. - Build the stored connection schemas from each operation's own connection schema (bounds tightened, strict), so the TLS values and defaults have one source. Stored blobs decode exactly as before. - Reuse the SQL Server default connection timeout instead of repeating it, and parse a submitted connection once. - Unit tests prove save-then-run round trips and drive refusals through the run wrapper; the workspace and revocation cases stay in the integration test, which now runs through the registered operation.
1 parent 351bccb commit bf20383

14 files changed

Lines changed: 277 additions & 222 deletions

File tree

‎apps/sim/lib/credentials/__integration__/database-connection.integration.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/**
22
* A saved PostgreSQL connection end to end against a real server: saving
33
* proves the login with SELECT 1 through the PostgreSQL tools' own guarded
4-
* connection path, and a run reads the stored connection inside the tool
5-
* operation, bound to the run's workspace, whatever connection fields the
4+
* connection path, and a run through the registered tool operation reads the
5+
* stored connection, bound to the run's workspace, whatever connection fields the
66
* call carries.
77
*/
88

@@ -19,7 +19,7 @@ vi.mock('@/lib/posthog/server', () => posthogServerMock)
1919

2020
import { env } from '@/lib/core/config/env'
2121
import { createCredentialRecord } from '@/lib/credentials/orchestration/credential-create'
22-
import { executePostgresqlTool } from '@/lib/internal/postgresql/execute-tool'
22+
import { getInternalToolOperationHandler } from '@/lib/internal/tool-operations/registry.server'
2323
import { POSTGRESQL_SERVICE_ACCOUNT_PROVIDER_ID } from '@/lib/oauth/types'
2424

2525
const databaseUrl = new URL(readTestDatabaseUrl())
@@ -50,8 +50,10 @@ function save(displayName: string, serviceAccountJson: string) {
5050
)
5151
}
5252

53-
function run(credentialId: string, runWorkspaceId: string) {
54-
return executePostgresqlTool({
53+
async function run(credentialId: string, runWorkspaceId: string) {
54+
const handler = await getInternalToolOperationHandler('postgresql_query')
55+
if (!handler) throw new Error('postgresql_query has no registered operation')
56+
const result = await handler({
5557
toolId: 'postgresql_query',
5658
input: {
5759
host: 'unreachable.invalid',
@@ -70,6 +72,8 @@ function run(credentialId: string, runWorkspaceId: string) {
7072
},
7173
requestId: generateId(),
7274
})
75+
if (!(result instanceof Response)) throw new Error('postgresql_query returned a file')
76+
return result
7377
}
7478

7579
describe('saved PostgreSQL connection', () => {

‎apps/sim/lib/credentials/database-connections/server.test.ts‎

Lines changed: 105 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ vi.mock('@/lib/internal/postgresql/operations', () => ({
1919
vi.mock('@/lib/internal/mysql/operations', () => ({ executeMysqlQuery: probes.mysql }))
2020
vi.mock('@/lib/internal/mssql/operations', () => ({ executeMssqlQuery: probes.mssql }))
2121

22+
import type { DatabaseConnectionProviderId } from '@/lib/credentials/database-connections/descriptors'
2223
import {
23-
getDatabaseConnection,
2424
verifyAndEncryptDatabaseConnection,
2525
withSavedDatabaseConnection,
2626
} from '@/lib/credentials/database-connections/server'
@@ -34,6 +34,7 @@ const connection = {
3434
ssl: 'required',
3535
}
3636
const stored = { type: 'postgresql_connection_v1', connection }
37+
const { ssl: _ssl, ...connectionWithoutTls } = connection
3738
const row = {
3839
workspaceId: 'workspace-1',
3940
organizationId: null,
@@ -43,27 +44,38 @@ const row = {
4344
encryptedServiceAccountKey: 'ciphertext',
4445
}
4546

47+
const savedConnectionContext = {
48+
workflowId: 'workflow-1',
49+
workspaceId: 'workspace-1',
50+
authorizedServiceAccountCredential: { credentialId: 'credential-1', service: 'postgresql' },
51+
}
52+
53+
const inlineInput = {
54+
host: 'attacker.example.com',
55+
port: 1,
56+
database: 'other',
57+
username: 'other',
58+
password: 'inline',
59+
ssl: 'disabled',
60+
query: 'SELECT 2',
61+
}
62+
4663
function call(overrides: Partial<InternalToolOperationCall> = {}): InternalToolOperationCall {
4764
return {
4865
toolId: 'postgresql_query',
49-
input: {
50-
host: 'attacker.example.com',
51-
port: 1,
52-
database: 'other',
53-
username: 'other',
54-
password: 'inline',
55-
ssl: 'disabled',
56-
query: 'SELECT 2',
57-
},
66+
input: inlineInput,
5867
headers: new Headers(),
5968
context: { workflowId: 'workflow-1', workspaceId: 'workspace-1' },
6069
requestId: 'request-1',
6170
...overrides,
6271
}
6372
}
6473

74+
const echo = vi.fn(async (request: InternalToolOperationCall) => Response.json(request.input))
75+
6576
describe('saved database connections', () => {
6677
beforeEach(() => {
78+
echo.mockClear()
6779
resetDbChainMock()
6880
for (const probe of Object.values(probes)) {
6981
probe.mockReset()
@@ -74,53 +86,75 @@ describe('saved database connections', () => {
7486
})
7587

7688
describe('saving', () => {
77-
it('proves the exact login with SELECT 1 before encrypting it', async () => {
78-
const result = await verifyAndEncryptDatabaseConnection(
79-
'postgresql-service-account',
80-
JSON.stringify(connection)
81-
)
82-
83-
expect(result).toEqual({
84-
encryptedServiceAccountKey: 'ciphertext',
85-
username: 'app_user',
86-
displayName: 'app_user@db.example.com/app',
87-
})
88-
expect(probes.postgresql.mock.calls[0]?.[0]).toEqual({
89-
...connection,
90-
query: 'SELECT 1 AS sim_connection_ok',
91-
})
92-
expect(probes.postgresql.mock.invocationCallOrder[0]).toBeLessThan(
93-
encryptionMockFns.mockEncryptSecret.mock.invocationCallOrder[0]
94-
)
95-
expect(JSON.parse(encryptionMockFns.mockEncryptSecret.mock.calls[0]?.[0])).toEqual(stored)
96-
})
89+
it.each([
90+
{
91+
providerId: 'postgresql-service-account',
92+
service: 'postgresql',
93+
probe: probes.postgresql,
94+
login: connectionWithoutTls,
95+
defaults: { ssl: 'preferred' },
96+
probeOnly: {},
97+
},
98+
{
99+
providerId: 'mysql-service-account',
100+
service: 'mysql',
101+
probe: probes.mysql,
102+
login: connectionWithoutTls,
103+
defaults: { ssl: 'preferred' },
104+
probeOnly: {},
105+
},
106+
{
107+
providerId: 'mssql-service-account',
108+
service: 'mssql',
109+
probe: probes.mssql,
110+
login: { ...connectionWithoutTls, port: 1433 },
111+
defaults: { encrypt: 'enabled', trustServerCertificate: 'disabled' },
112+
probeOnly: { connectionTimeout: 15_000 },
113+
},
114+
] as const)(
115+
'proves the exact $service login and a later run reads it back with its TLS defaults',
116+
async ({ providerId, service, probe, login, defaults, probeOnly }) => {
117+
encryptionMockFns.mockEncryptSecret.mockImplementation(async (plain: string) => ({
118+
encrypted: plain,
119+
iv: 'iv',
120+
}))
121+
encryptionMockFns.mockDecryptSecret.mockImplementation(async (encrypted: string) => ({
122+
decrypted: encrypted,
123+
}))
124+
125+
const saved = await verifyAndEncryptDatabaseConnection(
126+
providerId satisfies DatabaseConnectionProviderId,
127+
JSON.stringify(login)
128+
)
129+
expect(probe.mock.calls[0]?.[0]).toEqual({
130+
...login,
131+
...defaults,
132+
...probeOnly,
133+
query: 'SELECT 1 AS sim_connection_ok',
134+
})
135+
expect(saved).toMatchObject({
136+
username: 'app_user',
137+
displayName: `app_user@db.example.com/app`,
138+
})
97139

98-
it('stores the transport-security defaults explicitly so later runs never infer them', async () => {
99-
const { ssl: _ssl, ...withoutTls } = connection
100-
await verifyAndEncryptDatabaseConnection('mysql-service-account', JSON.stringify(withoutTls))
101-
await verifyAndEncryptDatabaseConnection(
102-
'mssql-service-account',
103-
JSON.stringify({ ...withoutTls, port: 1433 })
104-
)
140+
queueTableRows(credential, [
141+
{ ...row, providerId, encryptedServiceAccountKey: saved.encryptedServiceAccountKey },
142+
])
143+
const response = await withSavedDatabaseConnection(
144+
providerId,
145+
echo
146+
)(
147+
call({
148+
context: {
149+
...savedConnectionContext,
150+
authorizedServiceAccountCredential: { credentialId: 'credential-1', service },
151+
},
152+
})
153+
)
105154

106-
const [mysql, mssql] = encryptionMockFns.mockEncryptSecret.mock.calls.map(([blob]) =>
107-
JSON.parse(blob)
108-
)
109-
expect(mysql).toEqual({
110-
type: 'mysql_connection_v1',
111-
connection: { ...withoutTls, ssl: 'preferred' },
112-
})
113-
expect(mssql).toEqual({
114-
type: 'mssql_connection_v1',
115-
connection: {
116-
...withoutTls,
117-
port: 1433,
118-
encrypt: 'enabled',
119-
trustServerCertificate: 'disabled',
120-
},
121-
})
122-
expect(probes.mssql.mock.calls[0]?.[0]).toMatchObject({ connectionTimeout: 15_000 })
123-
})
155+
expect(await response.json()).toEqual({ ...inlineInput, ...login, ...defaults })
156+
}
157+
)
124158

125159
it.each([
126160
['malformed JSON', '{'],
@@ -160,52 +194,8 @@ describe('saved database connections', () => {
160194
})
161195
})
162196

163-
describe('loading', () => {
164-
it('returns the stored connection for its own provider and owner', async () => {
165-
queueTableRows(credential, [row])
166-
167-
await expect(
168-
getDatabaseConnection('postgresql-service-account', 'credential-1', {
169-
workspaceId: 'workspace-1',
170-
})
171-
).resolves.toEqual(connection)
172-
})
173-
174-
it.each([
175-
['another engine', { providerId: 'mysql-service-account' }],
176-
['another workspace', { workspaceId: 'workspace-2' }],
177-
['a revoked credential', { revokedAt: new Date() }],
178-
['an organization credential', { organizationId: 'organization-1' }],
179-
])('refuses %s before decrypting', async (_case, override) => {
180-
queueTableRows(credential, [{ ...row, ...override }])
181-
182-
await expect(
183-
getDatabaseConnection('postgresql-service-account', 'credential-1', {
184-
workspaceId: 'workspace-1',
185-
})
186-
).rejects.toThrow('PostgreSQL connection is unavailable')
187-
expect(encryptionMockFns.mockDecryptSecret).not.toHaveBeenCalled()
188-
})
189-
190-
it.each([
191-
['another engine’s blob', { ...stored, type: 'mysql_connection_v1' }],
192-
['an unknown stored field', { ...stored, connection: { ...connection, extra: 'x' } }],
193-
])('refuses %s', async (_case, blob) => {
194-
queueTableRows(credential, [row])
195-
encryptionMockFns.mockDecryptSecret.mockResolvedValue({ decrypted: JSON.stringify(blob) })
196-
197-
await expect(
198-
getDatabaseConnection('postgresql-service-account', 'credential-1', {
199-
workspaceId: 'workspace-1',
200-
})
201-
).rejects.toThrow('PostgreSQL connection is unavailable')
202-
})
203-
})
204-
205197
describe('running a tool', () => {
206-
const handler = vi.fn(async (request: InternalToolOperationCall) =>
207-
Response.json(request.input)
208-
)
198+
const handler = echo
209199

210200
it('hands an inline-connection call to the handler untouched', async () => {
211201
const request = call()
@@ -237,38 +227,30 @@ describe('saved database connections', () => {
237227
handler
238228
)(
239229
call({
240-
context: {
241-
workflowId: 'workflow-1',
242-
workspaceId: 'workspace-1',
243-
authorizedServiceAccountCredential: {
244-
credentialId: 'credential-1',
245-
service: 'postgresql',
246-
},
247-
},
230+
context: savedConnectionContext,
248231
})
249232
)
250233

251234
expect(await response.json()).toEqual({ ...connection, query: 'SELECT 2' })
252235
})
253236

254-
it('refuses with 403 and never runs when the saved connection is unavailable', async () => {
255-
queueTableRows(credential, [{ ...row, workspaceId: 'workspace-2' }])
237+
it.each([
238+
['a credential for another engine', { providerId: 'mysql-service-account' }, stored],
239+
['an organization credential', { organizationId: 'organization-1' }, stored],
240+
['another engine’s blob', {}, { ...stored, type: 'mysql_connection_v1' }],
241+
[
242+
'a blob with an unknown stored field',
243+
{},
244+
{ ...stored, connection: { ...connection, extra: 'x' } },
245+
],
246+
])('refuses %s with 403 and never runs', async (_case, rowOverride, blob) => {
247+
queueTableRows(credential, [{ ...row, ...rowOverride }])
248+
encryptionMockFns.mockDecryptSecret.mockResolvedValue({ decrypted: JSON.stringify(blob) })
256249

257250
const response = await withSavedDatabaseConnection(
258251
'postgresql-service-account',
259252
handler
260-
)(
261-
call({
262-
context: {
263-
workflowId: 'workflow-1',
264-
workspaceId: 'workspace-1',
265-
authorizedServiceAccountCredential: {
266-
credentialId: 'credential-1',
267-
service: 'postgresql',
268-
},
269-
},
270-
})
271-
)
253+
)(call({ context: savedConnectionContext }))
272254

273255
expect(response.status).toBe(403)
274256
expect(await response.json()).toEqual({ error: 'PostgreSQL connection is unavailable' })
@@ -290,14 +272,7 @@ describe('saved database connections', () => {
290272
)(
291273
call({
292274
signal: controller.signal,
293-
context: {
294-
workflowId: 'workflow-1',
295-
workspaceId: 'workspace-1',
296-
authorizedServiceAccountCredential: {
297-
credentialId: 'credential-1',
298-
service: 'postgresql',
299-
},
300-
},
275+
context: savedConnectionContext,
301276
})
302277
)
303278
).rejects.toThrow('run cancelled')

0 commit comments

Comments
 (0)