Skip to content

Commit cef636b

Browse files
committed
fix(knowledge): carry detach reservations through payer moves and refuse credentials to removed connectors
1 parent 2ba26c6 commit cef636b

4 files changed

Lines changed: 60 additions & 3 deletions

File tree

apps/sim/lib/billing/storage/payer-transfer.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ vi.mock('@sim/db/schema', () => ({
3131
id: 'knowledgeBase.id',
3232
workspaceId: 'knowledgeBase.workspaceId',
3333
},
34+
knowledgeConnector: {
35+
__table: 'knowledgeConnector',
36+
detachedAt: 'knowledgeConnector.detachedAt',
37+
detachReservedBytes: 'knowledgeConnector.detachReservedBytes',
38+
knowledgeBaseId: 'knowledgeConnector.knowledgeBaseId',
39+
},
3440
organization: {
3541
__table: 'organization',
3642
id: 'organization.id',
@@ -535,7 +541,10 @@ describe('changeWorkspaceStoragePayerInTx', () => {
535541
expect(query.values).not.toContain('workspaceFiles.deletedAt')
536542
expect(query.values).toContain('document.connectorId')
537543
expect(query.values).toContain('document.deletedAt')
538-
expect(query.values.filter((value) => value === 'workspace-1')).toHaveLength(3)
544+
/** A detaching connector's reservation is already charged, so a payer move carries it. */
545+
expect(query.values).toContain('knowledgeConnector.detachReservedBytes')
546+
expect(query.values).toContain('knowledgeConnector.detachedAt')
547+
expect(query.values.filter((value) => value === 'workspace-1')).toHaveLength(4)
539548
})
540549

541550
it('fails closed when a billable file is missing canonical size metadata', async () => {

apps/sim/lib/billing/storage/payer-transfer.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
document,
33
knowledgeBase,
4+
knowledgeConnector,
45
organization,
56
userStats,
67
workspace,
@@ -77,7 +78,8 @@ function parseExactBytes(value: number | string, label: string): number {
7778
* Computes one workspace's live billable bytes with two index-bounded scalar
7879
* aggregates. Archived workspace files and documents remain billable while
7980
* their objects are retained; mothership files, connector documents, and
80-
* deleted documents are excluded.
81+
* deleted documents are excluded. A detaching connector's reservation counts:
82+
* it was charged when the connector was removed with its documents kept.
8183
*/
8284
async function getExactWorkspaceStorageBytes(tx: DbOrTx, workspaceId: string): Promise<number> {
8385
const [row] = await tx.execute<ExactWorkspaceStorageRow>(sql`
@@ -103,6 +105,13 @@ async function getExactWorkspaceStorageBytes(tx: DbOrTx, workspaceId: string): P
103105
WHERE ${knowledgeBase.workspaceId} = ${workspaceId}
104106
AND ${document.connectorId} IS NULL
105107
AND ${document.deletedAt} IS NULL
108+
), 0)::bigint + COALESCE((
109+
SELECT SUM(${knowledgeConnector.detachReservedBytes})
110+
FROM ${knowledgeConnector}
111+
INNER JOIN ${knowledgeBase}
112+
ON ${knowledgeBase.id} = ${knowledgeConnector.knowledgeBaseId}
113+
WHERE ${knowledgeBase.workspaceId} = ${workspaceId}
114+
AND ${knowledgeConnector.detachedAt} IS NOT NULL
106115
), 0)::bigint AS document_bytes
107116
`)
108117

@@ -209,6 +218,20 @@ async function getExactWorkspaceStorageBytesBatch(
209218
AND ${document.connectorId} IS NULL
210219
AND ${document.deletedAt} IS NULL
211220
GROUP BY ${knowledgeBase.workspaceId}
221+
222+
UNION ALL
223+
224+
SELECT
225+
${knowledgeBase.workspaceId} AS workspace_id,
226+
0::bigint AS workspace_file_bytes,
227+
SUM(${knowledgeConnector.detachReservedBytes}) AS document_bytes,
228+
0::bigint AS workspace_file_missing_size_count
229+
FROM ${knowledgeConnector}
230+
INNER JOIN ${knowledgeBase}
231+
ON ${knowledgeBase.id} = ${knowledgeConnector.knowledgeBaseId}
232+
WHERE ${inArray(knowledgeBase.workspaceId, workspaceIds)}
233+
AND ${knowledgeConnector.detachedAt} IS NOT NULL
234+
GROUP BY ${knowledgeBase.workspaceId}
212235
) storage_by_workspace
213236
GROUP BY storage_by_workspace.workspace_id
214237
ORDER BY storage_by_workspace.workspace_id

apps/sim/lib/knowledge/connectors/member-access.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { credentialGroup, knowledgeBase, knowledgeConnector } from '@sim/db/schema'
5-
import { dbChainMockFns, hasMockCondition, resetDbChainMock } from '@sim/testing'
5+
import { dbChainMockFns, hasMockCondition, queueTableRows, resetDbChainMock } from '@sim/testing'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77

88
const mocks = vi.hoisted(() => ({
@@ -307,6 +307,7 @@ describe('knowledge connector member access', () => {
307307
})
308308

309309
it('resolves the token when the policy names the connector under the credential option and audits it', async () => {
310+
queueTableRows(knowledgeConnector, [{ id: 'connector-1' }])
310311
mocks.requireResourcePolicy.mockResolvedValue(
311312
storedPolicy(2, [
312313
{ credentialGroupOptionId: 'option-drive', connectorIds: ['connector-1'] },
@@ -338,7 +339,21 @@ describe('knowledge connector member access', () => {
338339
)
339340
})
340341

342+
it('refuses a removed connector even while its policy grant remains', async () => {
343+
mocks.requireResourcePolicy.mockResolvedValue(
344+
storedPolicy(2, [
345+
{ credentialGroupOptionId: 'option-drive', connectorIds: ['connector-1'] },
346+
])
347+
)
348+
349+
await expect(mintKnowledgeConnectorMemberToken(mintInput)).rejects.toThrow(
350+
'Knowledge connector has been removed'
351+
)
352+
expect(mocks.resolveManagedOAuthToken).not.toHaveBeenCalled()
353+
})
354+
341355
it('reports provider rejection only under the current connector grant', async () => {
356+
queueTableRows(knowledgeConnector, [{ id: 'connector-1' }])
342357
mocks.requireResourcePolicy.mockResolvedValue(
343358
storedPolicy(2, [
344359
{ credentialGroupOptionId: 'option-drive', connectorIds: ['connector-1'] },
@@ -451,6 +466,7 @@ describe('knowledge connector member access', () => {
451466

452467
describe('list', () => {
453468
it('pages the option credentials only for a granted connector', async () => {
469+
queueTableRows(knowledgeConnector, [{ id: 'connector-1' }])
454470
mocks.requireResourcePolicy.mockResolvedValue(
455471
storedPolicy(2, [
456472
{ credentialGroupOptionId: 'option-drive', connectorIds: ['connector-1'] },

apps/sim/lib/knowledge/connectors/member-access.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,15 @@ export async function assertKnowledgeConnectorCredentialAccess(
302302
}
303303
return
304304
}
305+
/** The policy grant outlives a removal until its revocation lands; the connector row does not. */
306+
const [liveConnector] = await db
307+
.select({ id: knowledgeConnector.id })
308+
.from(knowledgeConnector)
309+
.where(and(eq(knowledgeConnector.id, binding.connectorId), connectorIsLive()))
310+
.limit(1)
311+
if (!liveConnector) {
312+
throw new KnowledgeConnectorMemberAccessDeniedError('Knowledge connector has been removed')
313+
}
305314
const policy = await requireResourcePolicy(
306315
policyTarget({ ...binding, workspaceId: scope.workspaceId })
307316
).catch((error: unknown) => {

0 commit comments

Comments
 (0)