feat(knowledge): administrator access mode with mirrored source permissions - #7477
Draft
waleedlatif1 wants to merge 16 commits into
Draft
feat(knowledge): administrator access mode with mirrored source permissions#7477waleedlatif1 wants to merge 16 commits into
waleedlatif1 wants to merge 16 commits into
Conversation
…-aware resolver A connector's access token was resolved three different ways — once in the sync engine and twice in the connector application layer — each calling `refreshAccessTokenIfNeeded` with no scopes. Google's service-account resolver throws `Scopes are required for service account credentials` without them, so a service-account credential could not authenticate a connector at all: creation failed config validation, and a connector that got past it failed at its first mint. The string wrapper also discarded the `cloudId` an Atlassian service account resolves with, which is the only way such a credential can name its site — its API token cannot call `accessible-resources` to discover one. `resolveConnectorAccessToken` now owns that resolution for all three call sites, turning a connector's declared `ConnectorAuthConfig` plus its credential or key into a token bundle. `serviceAccountScopes` lets a connector declare the scopes its provider accepts in a two-legged grant where those differ from the ones its consent screen asks for, defaulting to `requiredScopes` where they coincide. A resolved cloud id seeds the same `syncContext` slot the connector would memoise it into, so no connector needs a service-account branch of its own. `getMissingRequiredScopes` now reports nothing missing for a service account: it names its scopes per request and has no granted-scope list, so measuring it against the required set flagged every scope missing and offered a reconnect that would have granted nothing. With both fixed, the add-connector modal stops filtering service accounts out of its credential list.
The mirrored-ACL model needs one place that decides who a Drive file is readable by, and it is the piece with the least margin for error: a wrong arm here publishes a document rather than failing a request. `driveFileAcl` maps a file's `permissions[]` to the token vocabulary the document ACL already speaks — `u:` for a named person, `g:` for a group, `pub` for a genuinely public file — with two rules worth stating outright. An open share grants nothing unless an admin has opted in for that connector. A file shared to a whole domain, or to anyone who finds it, is usually shared that way by accident, and its contents are exactly what nobody meant to publish; an admin who knows their domain's sharing hygiene can turn it on. A link-only share never grants, opt-in or not. `allowFileDiscovery: false` is Drive's own "reachable by link, findable by nobody", and Drive excludes those files from its search for the same reason we exclude them from ours. Onyx's file path misses this — it makes any `anyone` grant public without reading the flag, though its folder path does read it — so a link pasted once would make a document searchable by the whole workspace. Group membership and shared-drive membership are deliberately not resolved here. Both are directory state; expanding them per file would re-read the directory once per document, so the group is recorded as a token and the directory sync resolves it. A file whose every grant is unrepresentable resolves to `link` rather than an empty ACL, keeping "hidden on purpose" distinguishable from "hidden because we failed".
Admin mode mirrors a source's own permissions onto each document, and the whole point is that permissions change far more often than content: somebody joins a group, a folder is reshared. So the ACL write cannot be a field on the content write. A document the sync classifies as unchanged never reaches the document update path at all, and that path sets `processingStatus: 'pending'` — the sole trigger of re-embedding — so routing an ACL change through it would re-embed a corpus every time a group membership changed. `persistDocumentAcls` assigns `acl` and nothing else, leaving `contentHash`, `processingStatus`, `chunkCount` and the embedding rows untouched. Documents are grouped by identical ACL before writing, since files under one folder overwhelmingly share theirs, so a crawl of thousands resolves to a handful of statements; `IS DISTINCT FROM` means a re-run that changes nothing writes nothing, which is what lets permissions sync on a faster clock than content. Both properties are covered by tests that fail if the assignment widens. `SyncDocumentAccess` gains an `admin` arm alongside `members`. Both derive their ACL from something the content sync does not know, so both are born hidden and made visible by a separate pass — a document indexed before its permissions are known is invisible, never workspace-wide. `validateAcl` enforces a 5,000-token ceiling and the token shapes the database constrains. A document whose ACL we cannot store is stored as readable by nobody rather than skipped, because leaving the previous ACL in place would keep serving it under permissions we just failed to verify. Onyx declares the same ceiling and, by its own comment, never enforces it; catching it here also names the offending document instead of failing whichever batch it shared a statement with.
…s permissions Completes the admin-mode path end to end for Drive: one crawl under a service account, each document stored with the ACL the source itself reports. A service account owns nothing in a Workspace domain, so a crawl under one sees an empty corpus until it impersonates somebody. The subject lives on the connector rather than the credential — one `google-service-account` credential matches every Google service, so a subject stored on it would silently apply to a workflow reading that person's mail as well as to this crawl reading their Drive. `serviceAccountSubjectFieldId` names the config field holding it, and the service-account scope set is read-only and narrower than the interactive one: a crawl that reads every file in a domain should never hold write access. That administrator's email domain is also the tenant segment of every group token the crawl writes. It has to be settled before the first token is stored, because deriving it differently later would orphan every ACL already written — which is why Drive could not be switched on until the subject existed. The listing now requests `permissions` and `permissionIds` together, because Drive sometimes reports more ids than it expands. A file whose two counts disagree is left readable by nobody rather than mirrored under the subset that arrived: that sounds like the safe direction but is not, since the grants that went missing are exactly the ones nobody verified. The ACL pass runs over the whole listing rather than the documents whose content changed. A membership or sharing change moves no content, so restricting it to changed documents would let a revoked grant stay readable until somebody happened to edit the file. It runs after the content pass, so a document this run inserted — born hidden — is present to be made readable, and before reconciliation, so a revoked grant lands even on a run that removes nothing. The access-mode vocabulary moves into one leaf module. Three separate queries — the scheduler's due sweep, the queue's dispatch claim, and the engine's own lock — each hard-coded `accessMode = 'workspace'`, so widening the engine without widening all three would have left admin-mode connectors dispatchable but never dispatched, or worse, dispatched and then refused. They now read one constant.
`user.email` is unique byte-for-byte only, so `Alice@corp.com` and `alice@corp.com` can both exist as separate accounts. Every identity binding in the product compares the case-folded address, so those two rows are one identity to it: a credential-group enrollment for either matches both, and each account receives the `s:` subject token of a managed credential belonging to the other. That is live on the members-mode path today, and it is the same fold the `u:` document token depends on. `user_email_lower_unique` is the constraint that makes the state unreachable — the standard Postgres form of a case-insensitive unique email, on `lower(btrim(email))`. Access resolution probes that exact expression, so a predicate written any other way would silently become a sequential scan of `user` on every read; verified as an index scan against a real database. The migration pre-checks for duplicates inside the runner's batch transaction, so a database that already holds one fails the deploy with a sentence naming the problem and rolls back having changed nothing. Without that check the concurrent build would fail on the first duplicate and leave an INVALID index that `IF NOT EXISTS` skips on every later run — the constraint would appear to exist while enforcing nothing, which is the one outcome worth engineering against. Both paths were verified against a real database in rolled-back transactions: the index builds on current data, and a deliberately inserted case-variant duplicate raises and rolls back. Access resolution keeps its own ambiguity check rather than trusting the constraint to still be there. An index can be dropped during an incident, and a restore can bring back a database built before it existed; neither should silently hand one person another's documents. An ambiguous address binds to nothing, so both accounts keep the tokens every workspace member holds and lose only what their identity would have granted. The enrollment join also stops reading `normalized_email`. That column is declared unique but written by nothing, so the `COALESCE` over it always fell through to the folded address — and would have silently started matching a broader set of people the day anything backfilled it.
…e right people A mirrored ACL names a group; nothing until now said who was in one, so a document Drive shared with a group was readable by nobody. This closes the loop: the directory is enumerated into `knowledge_external_group` and its membership, and access resolution turns a reader's address into the `g:` tokens they hold. Groups are scoped by workspace, provider and tenant rather than by connector — two connectors over one Google Workspace domain grant the same groups, and resolving the directory once per connector would multiply Admin SDK traffic by the number of knowledge bases. Membership is keyed by case-folded email rather than Sim user id, because a directory reports addresses and most members of a granted group have no Sim account; storing the address means someone who signs up later inherits their grants on first read, with no backfill. Nested groups are flattened. Onyx reads one level and stops, so a person who belongs only through a subgroup silently gets nothing even though the source grants them access. The walk carries a visited set and a depth bound, because directories nest arbitrarily and will happily report a cycle. The unit of work is a group, not the directory. A group that enumerates completely is replaced in one transaction; one that does not is left exactly as it was, with a failure recorded and its `lastSyncedAt` untouched. That is the deliberate departure from Onyx, whose sync marks every row stale, upserts what the source returned and sweeps the rest — clean until the directory half-fails, at which point it revokes real members whose rows simply were not returned. Here an outage costs freshness and nothing else. That patience needs a bound, or a sync that stopped running would keep granting forever from membership nobody has checked. A group unconfirmed for longer than `EXTERNAL_GROUP_STALE_AFTER_MS` stops granting: an outage is survivable, an abandoned sync is not. Verified against a real database — the read plans as index scans on both sides, and a directory pushed past the window drops from 500 matching groups to none. The refresh runs inside the admin-mode crawl, before ACLs are written, so a crawl can never publish grants against membership this workspace has never read. It is rate-limited on its own clock so a frequently-syncing connector does not re-read the directory every run, and a failure is logged rather than thrown — last-known-good membership is still serving reads, and failing the content sync over it would strand the documents as well as the groups.
The mirroring path was complete but unreachable: `accessMode` accepted only `workspace` and `members`, so nothing could ask for the mode the last four commits built. This makes it selectable, end to end. The mode-switch matrix stays linear rather than growing to three-by-three. Both credential-backed modes change the same way — swap the credential, keep the documents — so they share one fast path, and `members` remains the special case it always was. What entering a mode does to existing ACLs moves into one table: `workspace` publishes to the workspace, `members` and `admin` both hide, because in both the ACL belongs to a pass that has not run yet. Hiding on entry is what makes an interrupted switch safe — documents are hidden early, never shown early — and the exit branch now reads that table instead of hard-coding `WORKSPACE_ACL`. Administrator mode takes the same role as members mode. Both decide whose data the workspace indexes, and both refuse rather than warn when the connector cannot deliver: a source that reports no per-document permissions has no administrator mode, and one that has not been told which administrator to crawl as would index every document readable by nobody — indistinguishable from a broken sync. Failing when the mode is chosen says what is missing while the person choosing it can still supply it. `currentAccess` in the edit modal stops folding unknown modes into Workspace. That catch-all would have told an admin their documents were visible to the whole workspace when they were not, and silently rewritten the mode on the next save. The engine-ownership rule is now covered by a test that walks every mode: the content and member engines hold mutually exclusive leases, so a mode claimed by both — or by neither — is a connector that either never runs or runs twice.
…ntial Groups One availability check governed everything permission-aware, and half of it was about Credential Groups. Administrator mode mirrors a source's own ACLs and touches no Credential Group, so an operator turning that feature off would have silently revoked every document an administrator crawl had mirrored — from a feature it does not use. The check now answers two questions from one billing read: whether source- mirrored access is available, and whether member-scoped access is. Both are enterprise features on Sim Cloud and both sit behind the same kill switch, so turning permission-aware knowledge off still hides every permission-scoped document on the next read; only the Credential Groups clause is now scoped to the mode that needs it. They are returned as a pair so a caller cannot check one and act on the other. Access resolution mints each token family under its own answer, and choosing administrator mode is refused when the workspace is not entitled to it — before a crawl indexes a corpus whose ACLs nobody would be able to match.
`display_name`, `consecutive_failures` and `last_error` were written on every sync and read by nothing. The staleness ratchet is `last_synced_at` alone — a failed enumeration writes nothing at all, and the timestamp not advancing *is* the record — so the failure counter measured something no decision consulted, and the error string duplicated a log line. The display name was speculation about a UI that does not exist. Removing them takes `recordGroupFailure` with them: the failure path now writes to the database not at all, which is both simpler and a stronger statement of the invariant it was there to protect.
…tions Confluence joins Drive as a source an administrator crawl can mirror, and it is the case that shaped the contract. A page's restrictions come back only when that page is asked for, so they cannot ride along with the listing the way Drive's permissions do. `getDocumentAcls` resolves them for the whole listing at once, after it — round trips bounded by the corpus rather than by the page size, with the space's principals, each page's restriction, and every address resolved once per run and reused. Only an unrestricted page pays for its ancestry, which is the expensive lookup. A restriction replaces the space's permissions rather than narrowing them. Real Confluence access is the intersection, so this over-grants in exactly one case: somebody named on a page restriction who cannot view the space at all. That is a misconfiguration in the source, it errs toward a page they were deliberately named on, and it is what Onyx does. Representing the true intersection would mean expanding both principal sets to member addresses, which our group tables could do — but it emits one token per member, and a five-thousand-person space would carry five-thousand-token ACLs on every restricted page. `null` and `[]` are different answers throughout: no restriction means inherit from the nearest restricted ancestor, then the space; a restriction naming nobody means readable by nobody. Confluence itself only ever produces the first, but collapsing them would publish every deliberately locked page. One departure from the plan, which said to identify groups by name as Onyx does. Onyx uses names because its membership sync is keyed by name; ours is keyed by whatever the permissions API returns, and that is the id. Using it costs no lookup per group and survives a rename, which a name-keyed ACL would not. Directory enumeration moves behind one connector hook. The tenant is baked into every stored group token, and only the connector knows what a tenant is for its source — a Workspace domain for Drive, a site's cloud id for Confluence. The engine previously derived it from the impersonation subject, which Confluence does not have: its service account authenticates with an API token and impersonates nobody, so directory refresh would have silently skipped. The limit worth knowing: Confluence Cloud withholds an address whose owner's profile hides it, and a person we cannot name cannot be granted access. Those grants are dropped and counted rather than guessed at, and a group with a withheld member is reported incomplete so it never replaces a stored membership with a subset.
Group membership decides who can read an already-indexed document, so it has to move independently of the corpus: someone leaving a group should lose access in minutes, not on whatever schedule their documents happen to be re-crawled on. Until now the only thing that refreshed a directory was the admin crawl itself, which made the five-minute interval a ceiling rather than a cadence — on a connector syncing daily, a revoked membership stood for a day, bounded only by the staleness ratchet. A scheduler now offers every admin-mode connector each tick and lets `syncExternalDirectoryGroups` decide whether its directory is actually due. The crawl keeps its own refresh, which is a floor rather than a duplicate: it is what guarantees a crawl never publishes grants against membership nobody has read. Connectors sharing a directory cost one refresh between them — the first brings it up to date and the rest skip on the interval gate. Two ticks overlapping on one directory would both enumerate and write the same rows, which is wasteful and never wrong, so it takes no lease to prevent; every write on this path is idempotent, and a lease would be new state to keep correct for no behavioural gain. Failure is contained per connector. One workspace whose credential lapsed must not stop the tick refreshing every other workspace's directory, and a test covers exactly that. The refresh moves out of the sync engine so both callers share it, and the Confluence connector drops a concurrency helper it should never have had — `mapWithConcurrency` already existed in `lib/core/utils`.
Two real bugs, then the duplication and drift a full re-read of the branch turned up. Administrator mode was unreachable for Confluence. Entering the mode required an impersonation subject on every connector, but a Confluence service account holds an API token that already speaks for the site and impersonates nobody, so the check refused every attempt. A subject is now required only of a connector whose auth declares a subject field, and a test pins the token-backed case. An incremental listing could not carry a revoked grant. A permission change moves no content — re-sharing a file does not touch its modified time in Drive, restricting a page does not touch its version in Confluence — so an incremental run listed only edited documents and the ACL pass refreshed only those. A grant revoked on an unchanged document stood until the next full sync happened to run, which is the over-grant direction. Administrator mode now always lists the whole corpus; content is still hydrated by hash, so unchanged documents are never re-fetched or re-embedded, and the cost is metadata pages only. Configuration validation minted without impersonation. The shared token resolver took the source config as optional, and the validate path did not pass it, so a Drive service account checked its configuration against an empty domain. The config is required now, and every caller — sync, validation, mode switch, creation, and the directory scheduler — passes it. The Workspace domain was derived in two places with a comment saying they must agree. It is one function now, beside the Google directory adapter, which moves from the knowledge library to the Drive connector where Confluence's equivalent already lives; provider-specific API clients belong with their connector, and the orchestration that calls them stays provider-agnostic. The Confluence connector memoised its cloud id in four separate copies, which became one. A group identifier is canonicalised once, in `canonicalGroupId`, by the crawl that writes a token and the directory sync that stores the membership it resolves against. Both already lower-cased by different routes; now they cannot drift. Confluence identifies groups by id, so the docs claiming "never an opaque id" were wrong and are corrected in the token vocabulary and the schema. Removed what nothing read: the impersonation subject the token resolver returned, a `force` flag no caller passed, two vestigial type aliases, and a nesting-depth constant that had one consumer and now lives with it. The directory recency check is one aggregate query rather than two probes; the Confluence ACL resolver enriches principals once rather than once per page; the mode picker validates its value instead of casting it; and the admin-mode hint no longer describes a field only Drive has. The create path had two copies of the admin-role gate, one per permission-scoped mode, and has one.
…witch 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.
…he number came from The comment justified 5,000 tokens by appeal to a reference implementation. The real reason is that the ceiling is a bug detector: with group tokens a legitimate document names tens of principals, so thousands means a connector expanded a group to its members — the exact failure group tokens prevent, and one that bloats the GIN index for every other document in the workspace.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…two migrations into one The directory-sync scheduler is now in the Helm cron map beside member-sync, and in the self-hosting background-jobs table. The Helm templates iterate the map, so the values entry is the whole change. The unique address index and the directory-group tables ship as one migration. The tables and the duplicate pre-check run inside the runner's batch transaction; the embedded COMMIT then lets the index on the hot `user` table build concurrently. A failure after that COMMIT replays the whole file against tables that are already committed, so every earlier statement is idempotent. Replaying it on a real database exposed one defect on the way: drizzle's derived foreign-key name for the membership table is 71 characters, Postgres silently truncates identifiers at 63, and the replay guard looked the full name up in `pg_constraint` and never found it. Both foreign keys now carry explicit short names in the schema. The pre-COMMIT section replays cleanly twice in a rolled-back transaction with both guards resolving.
…f the dead column An address was folded five different ways across the codebase: three SQL spellings — `lower(x)`, `lower(trim(x))`, `lower(btrim(x))` — plus reads of `user.normalized_email`, plus inline `trim().toLowerCase()`. Only one of the SQL forms matches the expression the new index is built on, so the others were sequential scans of `user` wearing the costume of an indexed lookup. `normalized_email` turned out to be populated after all — for a fifth of accounts, by a signup plugin removed in June, using Gmail dot-and-tag stripping. That is the right function for deduplicating signups and the wrong one for identity: it merges addresses a mail provider may route to different people, and it stops at the day the plugin left. Every read of the column is gone. The column itself stays for one release, because Better Auth selects every schema column and dropping it while the previous release still serves would break sign-in; the drop is the follow-up, and the repo's drop audit will name the argless reads that must be fixed first. `foldedEmail` now lives in the schema beside the index that indexes it, so the predicate and the index are one expression by construction. Its TypeScript twin is `normalizeEmail` from `@sim/utils/string`, which the new access code now uses instead of inlining the fold. The index is no longer unique. Production holds thirteen addresses that collide once folded — real, verified, active accounts — so a unique build would fail the deploy by design. Access resolution refuses to bind an ambiguous address, which keeps either account from reading the other's documents until the pairs are merged and the index can be promoted. The directory-sync cron joins docker/crontab; the parity audit caught that Helm alone was not enough.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an administrator access mode for knowledge connectors. An admin connects a source once with a service account; the crawl indexes every document with the permissions the source itself reports; every search is filtered by who is asking. Nobody has to connect their own account.
Sim already had two modes — workspace (everyone sees everything) and members (each person connects their own account). This is the third, and the one enterprise buyers expect. Ships for Google Drive and Confluence.
How it works
Sources
Google Drive — reads each file's permissions with the listing, falling back to
permissions.listfor shared-drive files. Whole-domain and public shares stay out of search until an admin opts in per connector; link-only shares never grant.Confluence — space permissions plus page restrictions, resolved through the page's ancestors. A restriction replaces the space's grant rather than narrowing it.
Also in this PR
user.emailgains a case-insensitive unique index. Two accounts differing only in case were one identity to every email-based binding, each inheriting the other's grants. The migration pre-checks for duplicates and fails loudly rather than leaving an invalid index behind.directory-sync, refreshes mirrored directories independently of content syncs. It needs a cron entry in infra before it does anything.Verification
type-check,lint:check, all audits,check:migrations, and the full test suite pass. Every behavioural fix has a test confirmed to fail with the fix reverted. Index usage, the unique-index build and its duplicate rollback, and the group staleness cutoff were checked against a real database in rolled-back transactions.Not yet done
directory-sync(infra).