fix(knowledge): read shared-drive permissions, and finish a pending switch before mirroring

Two mechanics found by tracing the admin-mode path end to end rather than
reading it.

Every file on a shared drive was invisible. Drive does not populate a file's
`permissions` when it lives on a shared drive — the docs say so outright, and
the field must come from `permissions.list` instead. The listing left those
files without an ACL, and the pass treated "no ACL" as "readable by nobody". A
whole shared drive indexed to nothing is the failure the plan's own note about
Onyx's `permissionIds` comment was warning about, and it was never built.

The contract now says what it should have: `getDocumentAcls` is called with
exactly the ids the listing could not answer for, and the engine merges the two
sources with the listing's answer winning where it exists. Drive implements it
by paging `permissions.list` per file under bounded concurrency; a file whose
inline entries were incomplete goes the same way instead of being hidden.
Confluence carries nothing inline, so it is unchanged. The merge is a pure
function with its own tests, and the shared-drive tests fail if the hook is
removed.

A switch into administrator mode whose hide outgrew its request budget was
never finished. The completion write cleared `accessRewritePending`, but the
only thing the content engine did with the flag was restore workspace ACLs,
which admin mode's SQL guard correctly ignored — so documents still carrying
`{ws}` from before the switch kept it until the pass overwrote them, and any the
listing missed kept it indefinitely. The pending hide now runs under the lease
before the pass writes real ACLs, mirroring how the member engine finishes its
own; the flag is then cleared on the strength of that.

The shared-drive group token is gone. Nothing resolved it — the directory sync
enumerates groups, not drives — and Drive already reports a drive's members as
ordinary inherited permissions on each file, so the token was both redundant
and a grant nobody could hold.

Author: waleedlatif1

apps/sim/connectors/google-drive/google-drive.test.ts

@@ -649,7 +649,6 @@ describe('mirroring Drive permissions onto listed documents', () => {
     const url = String(mockFetch.mock.calls[0][0])
     expect(decodeURIComponent(url)).toContain('permissions(id,type,emailAddress,domain,role,')
     expect(decodeURIComponent(url)).toContain('permissionIds')
-    expect(decodeURIComponent(url)).toContain('driveId')
   })
 
   it('tags each document with who may read it', async () => {
@@ -735,15 +734,63 @@ describe('mirroring Drive permissions onto listed documents', () => {
     expect(doc.acl).toEqual(['link'])
   })
 
-  it('records the shared drive a file lives on', async () => {
-    const doc = await listWith(
-      driveFile({
-        driveId: 'shared-drive-1',
-        permissions: [{ id: 'p1', type: 'user', emailAddress: 'alice@corp.com' }],
-      }),
-      ADMIN
+  /**
+   * Drive does not populate `permissions` for a file on a shared drive; the
+   * only source is `permissions.list`. A listing that left the ACL unset must
+   * therefore be answered by the fallback, not treated as readable by nobody.
+   */
+  it('resolves a file the listing could not describe through permissions.list', async () => {
+    const doc = await listWith(driveFile({}), ADMIN)
+    expect(doc.acl).toBeUndefined()
+
+    mockFetch.mockResolvedValueOnce(
+      jsonResponse({
+        permissions: [
+          { id: 'p1', type: 'user', emailAddress: 'alice@corp.com' },
+          { id: 'p2', type: 'group', emailAddress: 'eng@corp.com' },
+        ],
+      })
     )
 
-    expect(doc.acl).toEqual(['g:google-drive:corp.com:shared-drive-1', 'u:alice@corp.com'])
+    await expect(
+      googleDriveConnector.getDocumentAcls?.('token', ADMIN, [FILE_ID], {})
+    ).resolves.toEqual({
+      [FILE_ID]: ['g:google-drive:corp.com:eng@corp.com', 'u:alice@corp.com'],
+    })
+    const url = String(mockFetch.mock.calls[1][0])
+    expect(url).toContain(`/files/${FILE_ID}/permissions`)
+    expect(url).toContain('supportsAllDrives=true')
+  })
+
+  it('follows the permission list across pages', async () => {
+    mockFetch
+      .mockResolvedValueOnce(
+        jsonResponse({
+          permissions: [{ id: 'p1', type: 'user', emailAddress: 'alice@corp.com' }],
+          nextPageToken: 'p2',
+        })
+      )
+      .mockResolvedValueOnce(
+        jsonResponse({ permissions: [{ id: 'p2', type: 'user', emailAddress: 'bob@corp.com' }] })
+      )
+
+    await expect(
+      googleDriveConnector.getDocumentAcls?.('token', ADMIN, [FILE_ID], {})
+    ).resolves.toEqual({ [FILE_ID]: ['u:alice@corp.com', 'u:bob@corp.com'] })
+  })
+
+  it('omits a file whose permissions could not be read, so it stays hidden', async () => {
+    mockFetch.mockResolvedValueOnce(jsonResponse({ error: 'nope' }, 403))
+
+    await expect(
+      googleDriveConnector.getDocumentAcls?.('token', ADMIN, [FILE_ID], {})
+    ).resolves.toEqual({})
+  })
+
+  it('answers nothing for a crawl that mirrors no permissions', async () => {
+    await expect(
+      googleDriveConnector.getDocumentAcls?.('token', {}, [FILE_ID], {})
+    ).resolves.toEqual({})
+    expect(mockFetch).not.toHaveBeenCalled()
   })
 })

apps/sim/connectors/google-drive/google-drive.ts

@@ -1,6 +1,7 @@
 import { createLogger } from '@sim/logger'
 import { getErrorMessage, toError } from '@sim/utils/errors'
 import { isPlainRecord } from '@sim/utils/object'
+import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
 import {
   type DrivePermission,
   driveFileAcl,
@@ -225,7 +226,6 @@ interface DriveFile {
    * its real grants.
    */
   permissionIds?: string[]
-  driveId?: string
 }
 
 interface DriveChange {
@@ -485,14 +485,15 @@ function driveAclContext(sourceConfig: Record<string, unknown>): DriveAclContext
 }
 
 /**
- * The file's mirrored ACL, or undefined when this crawl cannot speak for it.
+ * The file's mirrored ACL from its listing, or undefined when the listing
+ * cannot speak for it and {@link resolveDriveAcls} must.
  *
- * Drive sometimes returns fewer expanded permissions than it reports ids for,
- * which is why both are requested. Mirroring the subset that did arrive would
- * store the file under narrower grants than it really has — which sounds like
- * the safe direction but is not, because the grants that went missing are
- * exactly the ones nobody verified. Undefined leaves the file readable by
- * nobody until a crawl sees the whole set.
+ * Drive does not populate `permissions` for a file on a shared drive at all,
+ * and sometimes returns fewer expanded entries than it reports ids for — which
+ * is why both are requested. Mirroring the subset that did arrive would store
+ * the file under narrower grants than it really has, which sounds like the
+ * safe direction but is not, because the grants that went missing are exactly
+ * the ones nobody verified. Undefined sends the file to `permissions.list`.
  */
 function fileAcl(file: DriveFile, context: DriveAclContext | null): string[] | undefined {
   if (!context || !file.permissions) return undefined
@@ -502,10 +503,94 @@ function fileAcl(file: DriveFile, context: DriveAclContext | null): string[] | u
     providerId: context.providerId,
     tenantId: context.tenantId,
     policy: context.policy,
-    driveId: file.driveId,
   })
 }
 
+/** Files whose permissions are fetched at once. Bounded to keep a crawl responsive. */
+const PERMISSION_FETCH_CONCURRENCY = 8
+
+/** Guards against a file that keeps paginating; far above any real permission list. */
+const MAX_PERMISSION_PAGES = 50
+
+/**
+ * A file's full permission list, from the one endpoint that serves it for every
+ * file — including those on a shared drive, whose listing carries none.
+ *
+ * Throws rather than returning a partial list: a file mirrored under the
+ * permissions that happened to arrive is a file whose missing grants nobody
+ * verified.
+ */
+async function listFilePermissions(
+  accessToken: string,
+  fileId: string
+): Promise<DrivePermission[]> {
+  const permissions: DrivePermission[] = []
+  let pageToken: string | undefined
+  for (let page = 0; page < MAX_PERMISSION_PAGES; page += 1) {
+    const query = new URLSearchParams({
+      fields: 'nextPageToken,permissions(id,type,emailAddress,domain,role,allowFileDiscovery)',
+      pageSize: '100',
+      supportsAllDrives: 'true',
+    })
+    if (pageToken) query.set('pageToken', pageToken)
+
+    const response = await fetchGoogleDriveWithRetry(
+      `https://www.googleapis.com/drive/v3/files/${encodeURIComponent(fileId)}/permissions?${query.toString()}`,
+      {
+        method: 'GET',
+        headers: { Authorization: `Bearer ${accessToken}`, Accept: 'application/json' },
+      }
+    )
+    if (!response.ok) {
+      throw new Error(`Google Drive permissions request failed: ${response.status}`)
+    }
+    const body = (await response.json()) as {
+      permissions?: DrivePermission[]
+      nextPageToken?: string
+    }
+    permissions.push(...(body.permissions ?? []))
+    pageToken = body.nextPageToken
+    if (!pageToken) return permissions
+  }
+  throw new Error(`Google Drive permissions exceeded ${MAX_PERMISSION_PAGES} pages`)
+}
+
+/**
+ * The ACLs of files whose listing could not describe them — every file on a
+ * shared drive, and any whose inline permissions were incomplete.
+ *
+ * A file whose permissions cannot be read is omitted, which leaves it readable
+ * by nobody until a run can read them: the failure is logged per file and the
+ * rest of the batch still resolves.
+ */
+async function resolveDriveAcls(
+  accessToken: string,
+  sourceConfig: Record<string, unknown>,
+  externalIds: string[]
+): Promise<Record<string, string[]>> {
+  const context = driveAclContext(sourceConfig)
+  if (!context) return {}
+
+  const acls: Record<string, string[]> = {}
+  await mapWithConcurrency(externalIds, PERMISSION_FETCH_CONCURRENCY, async (fileId) => {
+    try {
+      const permissions = await listFilePermissions(accessToken, fileId)
+      acls[fileId] = driveFileAcl({
+        permissions,
+        providerId: context.providerId,
+        tenantId: context.tenantId,
+        policy: context.policy,
+      })
+    } catch (error) {
+      logger.warn("Could not read a file's permissions; it stays readable by nobody", {
+        fileId,
+        ...googleDriveErrorLogFields(error),
+      })
+    }
+  })
+  return acls
+}
+
 function fileToStub(file: DriveFile, acl?: string[]): ExternalDocument {
   /**
    * Sheets moved from a first-sheet-only CSV export to the complete XLSX source.
@@ -563,7 +648,7 @@ export const googleDriveConnector: ConnectorConfig = {
       pageSize: String(effectivePageSize),
       orderBy: 'modifiedTime desc',
       fields:
-        'kind,nextPageToken,incompleteSearch,files(id,name,mimeType,modifiedTime,createdTime,webViewLink,owners,size,starred,parents,driveId,permissionIds,permissions(id,type,emailAddress,domain,role,allowFileDiscovery,permittedBy))',
+        'kind,nextPageToken,incompleteSearch,files(id,name,mimeType,modifiedTime,createdTime,webViewLink,owners,size,starred,parents,permissionIds,permissions(id,type,emailAddress,domain,role,allowFileDiscovery,permittedBy))',
       supportsAllDrives: 'true',
       includeItemsFromAllDrives: 'true',
     })
@@ -646,6 +731,9 @@ export const googleDriveConnector: ConnectorConfig = {
   openDirectory: async (accessToken, sourceConfig) =>
     openGoogleDirectory(accessToken, sourceConfig.adminEmail),
 
+  getDocumentAcls: (accessToken, sourceConfig, externalIds) =>
+    resolveDriveAcls(accessToken, sourceConfig, externalIds),
+
   getDocument: async (
     accessToken: string,
     sourceConfig: Record<string, unknown>,

apps/sim/connectors/types.ts

@@ -448,14 +448,15 @@ export interface ConnectorConfig extends ConnectorMeta {
   ) => Promise<ConnectorDirectory | null>
 
   /**
-   * The ACLs of a batch of already-listed documents, for a source whose
-   * permissions do not come back with its listing.
+   * The ACLs of listed documents the listing itself could not answer for.
    *
-   * Drive reports each file's permissions in the same page that lists it, so it
-   * fills {@link ExternalDocument.acl} directly and needs none of this.
-   * Confluence reports a page's restrictions only when asked for that page, so
-   * it resolves them here — batched, and after the listing, so the round trips
-   * are bounded by the corpus rather than by the page size.
+   * Called with exactly the external ids whose {@link ExternalDocument.acl}
+   * the listing left unset, after the listing and once for all of them, so the
+   * round trips are bounded by what the listing could not carry rather than by
+   * the page size. Drive fills the ACL inline for most files and lands here
+   * only for the ones its listing cannot describe — a shared drive's files,
+   * whose permissions Drive serves solely through `permissions.list`.
+   * Confluence carries none inline, so every page lands here.
    *
    * Returns tokens per external id. An id the connector omits is readable by
    * nobody: a connector declaring {@link ConnectorMeta.mirrorsSourceAcls}

apps/sim/lib/knowledge/access/drive-permissions.test.ts

@@ -15,8 +15,8 @@ const PROVIDER = 'google-drive'
 const TENANT = 'C01abcdef'
 const OPEN: OpenSharingPolicy = { domain: true, anyone: true }
 
-function acl(permissions: DrivePermission[], policy = CLOSED_OPEN_SHARING, driveId?: string) {
-  return driveFileAcl({ permissions, providerId: PROVIDER, tenantId: TENANT, policy, driveId })
+function acl(permissions: DrivePermission[], policy = CLOSED_OPEN_SHARING) {
+  return driveFileAcl({ permissions, providerId: PROVIDER, tenantId: TENANT, policy })
 }
 
 describe('driveFileAcl', () => {
@@ -46,12 +46,6 @@ describe('driveFileAcl', () => {
     ).toEqual([`g:${PROVIDER}:${TENANT}:eng@corp.com`])
   })
 
-  it('records the shared drive as a group rather than expanding its membership', () => {
-    expect(
-      acl([{ type: 'user', emailAddress: 'alice@corp.com' }], CLOSED_OPEN_SHARING, 'drive-1')
-    ).toEqual([`g:${PROVIDER}:${TENANT}:drive-1`, 'u:alice@corp.com'])
-  })
-
   describe('open sharing is closed by default', () => {
     it('drops a whole-domain share', () => {
       expect(acl([{ type: 'domain', domain: 'corp.com' }])).toEqual(['link'])
@@ -138,8 +132,7 @@ describe('driveFileAcl', () => {
         { type: 'domain', domain: 'corp.com' },
         { type: 'anyone' },
       ],
-      OPEN,
-      'drive-1'
+      OPEN
     )
     for (const token of tokens) expect(token).toMatch(ACCESS_TOKEN_PATTERN)
   })

apps/sim/lib/knowledge/access/drive-permissions.ts

@@ -60,21 +60,16 @@ export interface DriveAclInput {
   /** The Google Workspace customer the crawl runs against. */
   tenantId: string | null
   policy: OpenSharingPolicy
-  /**
-   * The shared drive the file lives on, if any. Recorded as a group rather than
-   * expanded here: a shared drive's membership is directory state, and
-   * resolving it during the crawl would re-read it once per file.
-   */
-  driveId?: string | null
 }
 
 /**
  * The ACL of one Drive file, from the permissions the listing returned.
  *
- * Inheritance is deliberately not resolved here. A grant that descends from a
- * folder still arrives in `permissions[]` with `permittedBy` naming its source,
- * so it maps like any other; what is *not* resolved is group membership, which
- * belongs to the directory sync. That keeps the crawl one pass over files.
+ * Inheritance needs no resolving here. A grant that descends from a folder or
+ * from shared-drive membership arrives in `permissions[]` as an ordinary
+ * principal, so it maps like any other; what is *not* resolved is group
+ * membership, which belongs to the directory sync. That keeps the crawl one
+ * pass over files.
  *
  * A file whose every grant is unrepresentable — an `anyone` share that is
  * link-only, a principal with no email — resolves to a single `link` token
@@ -131,11 +126,6 @@ export function driveFileAcl(input: DriveAclInput): string[] {
     }
   }
 
-  if (input.driveId) {
-    const token = groupToken({ providerId, tenantId, groupId: input.driveId })
-    if (token) tokens.add(token)
-  }
-
   if (tokens.size === 0) return [LINK_ACCESS_TOKEN]
   return sortAccessTokens(tokens)
 }