Skip to content

Commit 4c785f0

Browse files
waleedlatif1waleedlatif1
committed
fix(knowledge): read a user's visible documents from the ACL index and cover the probe in PostgreSQL
The visibility probe moves to its own module so it can run against a real database. Planned inline, LIMIT 1 made a sequential scan of the document table look cheaper than doc_acl_gin_idx, because PostgreSQL cannot estimate array overlap. A materialized CTE now reads the user's grants from the index first, bounding the probe by that user's grants. The email goes through userToken, so a mixed-case or padded directory address matches the normalized ACL token. The new PostgreSQL integration test covers fresh, stale and missing permission evidence, another user's grant, another connector, excluded, archived and deleted documents, and email normalization, and runs in the Search progress PostgreSQL CI step.
1 parent f4ec7f3 commit 4c785f0

4 files changed

Lines changed: 126 additions & 23 deletions

File tree

‎.github/workflows/test-build.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ jobs:
271271
lib/knowledge/__integration__/connector-deferral.integration.ts
272272
lib/knowledge/__integration__/stored-document-recovery.integration.ts
273273
lib/knowledge/__integration__/connector-partition-work.integration.ts
274+
lib/knowledge/__integration__/user-document-visibility.integration.ts
274275
lib/knowledge/__integration__/listing-continuation.integration.ts
275276
lib/knowledge/__integration__/member-scope-renewal.integration.ts
276277
lib/knowledge/__integration__/slack-empty-threads.integration.ts

‎apps/sim/lib/knowledge/__integration__/user-document-visibility.integration.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/** Real PostgreSQL coverage for the probe that keeps a user's visible documents from being skipped. */
2+
import { db } from '@sim/db'
3+
import { document, knowledgeConnector, organization, user, workspace } from '@sim/db/schema'
4+
import { generateId } from '@sim/utils/id'
5+
import { eq, inArray } from 'drizzle-orm'
6+
import { afterAll, afterEach, beforeEach, describe, expect, it } from 'vitest'
7+
import { seedKnowledgeAclFixture } from '@/lib/knowledge/__integration__/seed-source-access-fixture'
8+
import { hasVisibleUserDocuments } from '@/lib/knowledge/connectors/user-document-visibility'
9+
10+
const DAY_MS = 24 * 60 * 60 * 1000
11+
12+
describe('Visible user documents in PostgreSQL', () => {
13+
let owner: Awaited<ReturnType<typeof seedKnowledgeAclFixture>>
14+
let otherConnectorId: string
15+
const email = () => `${owner.aliceId}@fixture.test`
16+
17+
beforeEach(async () => {
18+
owner = await seedKnowledgeAclFixture(undefined, { connectorType: 'google_drive' })
19+
otherConnectorId = generateId()
20+
await db.insert(knowledgeConnector).values({
21+
id: otherConnectorId,
22+
knowledgeBaseId: owner.knowledgeBaseId,
23+
connectorType: 'google_drive',
24+
sourceConfig: {},
25+
accessMode: 'admin',
26+
status: 'active',
27+
credentialId: owner.credentialId,
28+
})
29+
})
30+
afterEach(async () => {
31+
await db.delete(workspace).where(eq(workspace.id, owner.workspaceId))
32+
await db.delete(organization).where(eq(organization.id, owner.organizationId))
33+
await db.delete(user).where(inArray(user.id, [owner.aliceId, owner.bobId]))
34+
})
35+
afterAll(() => db.$client.end())
36+
37+
const insert = (overrides: Partial<typeof document.$inferInsert> = {}) =>
38+
db.insert(document).values({
39+
id: generateId(),
40+
knowledgeBaseId: owner.knowledgeBaseId,
41+
connectorId: owner.connectorId,
42+
externalId: generateId(),
43+
filename: 'Event.txt',
44+
mimeType: 'text/plain',
45+
fileUrl: '',
46+
fileSize: 0,
47+
acl: [`u:${email()}`],
48+
aclVerifiedAt: new Date(),
49+
...overrides,
50+
})
51+
52+
it('finds a live, included document granted to the user with fresh permission evidence', async () => {
53+
await insert()
54+
expect(await hasVisibleUserDocuments(owner.connectorId, email())).toBe(true)
55+
})
56+
57+
it('matches a mixed-case, padded directory email to the normalized ACL token', async () => {
58+
await insert()
59+
expect(await hasVisibleUserDocuments(owner.connectorId, ` ${email().toUpperCase()} `)).toBe(
60+
true
61+
)
62+
})
63+
64+
it.each([
65+
{
66+
label: 'permission evidence older than the freshness limit',
67+
overrides: () => ({ aclVerifiedAt: new Date(Date.now() - DAY_MS - 60_000) }),
68+
},
69+
{ label: 'no permission evidence', overrides: () => ({ aclVerifiedAt: null }) },
70+
{
71+
label: 'a grant to a different user',
72+
overrides: () => ({ acl: [`u:${owner.bobId}@fixture.test`] }),
73+
},
74+
{
75+
label: 'a document in another connector',
76+
overrides: () => ({ connectorId: otherConnectorId }),
77+
},
78+
{ label: 'a user-excluded document', overrides: () => ({ userExcluded: true }) },
79+
{ label: 'an archived document', overrides: () => ({ archivedAt: new Date() }) },
80+
{ label: 'a deleted document', overrides: () => ({ deletedAt: new Date() }) },
81+
])('ignores $label', async ({ overrides }) => {
82+
await insert(overrides())
83+
expect(await hasVisibleUserDocuments(owner.connectorId, email())).toBe(false)
84+
})
85+
86+
it('refuses an address that cannot be a user token', async () => {
87+
await insert()
88+
expect(await hasVisibleUserDocuments(owner.connectorId, ' ')).toBe(false)
89+
})
90+
})

‎apps/sim/lib/knowledge/connectors/sync-content-pass.ts‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { document, knowledgeConnector } from '@sim/db/schema'
33
import { and, asc, eq, inArray, isNotNull, isNull, lt, type SQL, sql } from 'drizzle-orm'
44
import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution'
55
import type { DbOrTx } from '@/lib/db/types'
6-
import { SOURCE_ACL_MAX_AGE_MS } from '@/lib/knowledge/access/freshness'
76
import {
87
type ConnectorAccessMode,
98
effectiveConnectorSyncIntervalMinutes,
@@ -46,6 +45,7 @@ import {
4645
resolveReconciliationDeleteCap,
4746
storedHashIsCurrent,
4847
} from '@/lib/knowledge/connectors/sync-primitives'
48+
import { hasVisibleUserDocuments } from '@/lib/knowledge/connectors/user-document-visibility'
4949
import { hardDeleteDocuments } from '@/lib/knowledge/documents/service'
5050
import { SIM_SEARCH_SYNC_INTERVAL_MINUTES } from '@/lib/sim-search/constants'
5151
import { googleCompanyUserContextSchema } from '@/connectors/google-workspace/company-work'
@@ -349,28 +349,6 @@ export async function runConnectorContentPass(input: ContentPassInput) {
349349
}
350350
}
351351

352-
/**
353-
* Whether readers can still see any of this connector's documents granted to one user. The user
354-
* token is read through `doc_acl_gin_idx` alone behind an `OFFSET 0` fence, so the probe is bounded
355-
* by that user's grants instead of walking the connector's documents.
356-
*/
357-
async function hasVisibleUserDocuments(connectorId: string, email: string): Promise<boolean> {
358-
const rows = await db.execute(sql`
359-
SELECT 1 FROM (
360-
SELECT ${document.connectorId}, ${document.userExcluded}, ${document.archivedAt}, ${document.aclVerifiedAt}
361-
FROM ${document}
362-
WHERE ${document.deletedAt} IS NULL AND ${document.acl} && ARRAY[${`u:${email}`}]::text[]
363-
OFFSET 0
364-
) AS ${document}
365-
WHERE ${document.connectorId} = ${connectorId}
366-
AND ${document.userExcluded} = false
367-
AND ${document.archivedAt} IS NULL
368-
AND ${document.aclVerifiedAt} > statement_timestamp() - (${SOURCE_ACL_MAX_AGE_MS} * interval '1 millisecond')
369-
LIMIT 1
370-
`)
371-
return rows.length > 0
372-
}
373-
374352
/** Reconciles absence only after EOF, with bounded queries and the existing deletion guards. */
375353
async function reconcileCompletedListing(
376354
input: ContentPassInput,

‎apps/sim/lib/knowledge/connectors/user-document-visibility.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { db } from '@sim/db'
2+
import { document } from '@sim/db/schema'
3+
import { sql } from 'drizzle-orm'
4+
import { SOURCE_ACL_MAX_AGE_MS } from '@/lib/knowledge/access/freshness'
5+
import { userToken } from '@/lib/knowledge/access/tokens'
6+
7+
/**
8+
* Whether readers can still see any of this connector's documents granted to one user: a live,
9+
* included document carrying their token with permission evidence inside the freshness limit.
10+
* The user's grants are materialized from `doc_acl_gin_idx` first, so the probe is bounded by
11+
* that user's grants instead of the connector's size. Planned inline, `LIMIT 1` makes a
12+
* sequential scan look cheaper than the index, because PostgreSQL cannot estimate array overlap.
13+
*/
14+
export async function hasVisibleUserDocuments(
15+
connectorId: string,
16+
email: string
17+
): Promise<boolean> {
18+
const token = userToken(email)
19+
if (!token) return false
20+
const rows = await db.execute(sql`
21+
WITH granted AS MATERIALIZED (
22+
SELECT ${document.connectorId}, ${document.userExcluded}, ${document.archivedAt}, ${document.aclVerifiedAt}
23+
FROM ${document}
24+
WHERE ${document.deletedAt} IS NULL AND ${document.acl} && ARRAY[${token}]::text[]
25+
)
26+
SELECT 1 FROM granted AS ${document}
27+
WHERE ${document.connectorId} = ${connectorId}
28+
AND ${document.userExcluded} = false
29+
AND ${document.archivedAt} IS NULL
30+
AND ${document.aclVerifiedAt} > statement_timestamp() - (${SOURCE_ACL_MAX_AGE_MS} * interval '1 millisecond')
31+
LIMIT 1
32+
`)
33+
return rows.length > 0
34+
}

0 commit comments

Comments
 (0)