Skip to content

Run File Search maintenance per workspace under RLS (#1418) - #1557

Merged
helpcodeai merged 7 commits into
mainfrom
fix/file-search-rls-maintenance
Nov 6, 2026
Merged

helpcodeai merged 7 commits into
mainfrom
fix/file-search-rls-maintenance

Conversation

@helpcodeai

Copy link
Copy Markdown
Contributor

Summary

The file_* tables use FORCE ROW LEVEL SECURITY with an org_isolation policy on app.current_org. Every query that ran outside FileSearchRepository.withTenant saw no rows when the database user is neither superuser nor BYPASSRLS (managed PostgreSQL: RDS, Azure, Supabase…). On such installs syncs never started, leases were never released, scans never ran, cleanups and share links did not work, and Enterprise folder rules were invisible, so deny rules were ignored (fail open).

This PR runs every one of those queries one workspace at a time inside withTenant. The org_isolation policies are unchanged; there is no maintenance switch or other bypass. The only new policy is a SELECT-only share_by_token on file_shares, which shows a row only to whoever already holds its secret token.

Changes

  • Migration 20261106120000_file_share_token_policy: share_by_token on file_shares, FOR SELECT, USING (token_hash = NULLIF(current_setting('app.share_token_hash', true), '')). An unset setting (NULL) or an emptied one ('') never matches. Permissive policies are ORed per command, so it adds nothing to INSERT/UPDATE/DELETE.
  • FileSearchRepository: mode() (amcp_files / subject to RLS / bypassing RLS), withShareToken(hash, fn) (transaction-local app.share_token_hash, app.current_org cleared, amcp_files when granted), tenantsWithFileSearch() (connectors has no RLS), visibility() for the startup check. The amcp_files role check no longer errors out when the role does not exist.
  • Scheduler (ee): each tick surveys every workspace in its own withTenant (most overdue source, live lease, oldest queued scan, stale scans), sorts candidates by next_sync_at across workspaces, then claims each in its own workspace transaction. The claim takes pg_try_advisory_xact_lock on the workspace, re-checks every lease condition and uses FOR UPDATE SKIP LOCKED. Leases this worker holds are kept in memory, and renew/release/stop run in the lease's workspace with organization_id in the WHERE clause. Scans are claimed per workspace, oldest first. The sweep (drafts, scan samples, 30-day licence pause, File Share) runs per workspace. The workspace list is cached for 30 s, and kick() refreshes it. TICK_MS and LEASE_MIN are unchanged.
  • files-worker (ee): closeInterruptedRuns runs per workspace. At start the worker logs its mode and warns once when File Search folders exist but no file_sources row is visible.
  • Folder rules: scopeFor reads file_access_rules inside withTenant. Any failure throws, and execute returns folder_rules_unavailable without querying the index (fail closed).
  • Share links: byToken reads through withShareToken, and the download count is updated inside withTenant(row.organizationId). The workspace name comes from a separate query on organizations. The signed download URL is now <id>.<orgId>.<exp>.<sig> with the workspace inside the HMAC (download-v2: domain prefix). The signature is verified before the row is read in that workspace. Old <id>.<exp>.<sig> URLs are refused; they lived 60 seconds (DOWNLOAD_URL_TTL_S). FileShareService.sweep runs per workspace.
  • Docs: docs/connectors/file-search.md, new "Database user" section (works without superuser or BYPASSRLS, what amcp_files is, grants on managed databases, startup log modes). It replaces the outdated note that said superuser or BYPASSRLS was required.

Cloud and self-hosted

  • Cloud (superuser, member of amcp_files): same behaviour. Maintenance now also runs under the policies, so a scheduler bug cannot touch another workspace. Each 5 s tick costs one small transaction per workspace with a File Search connector (3 workspaces today), plus one per claim.
  • Self-hosted, default Docker (database owner, superuser): same as Cloud.
  • Self-hosted on a managed database (no superuser/BYPASSRLS, with or without amcp_files): syncs, scans, lease release on deploy, interrupted-run cleanup, sweeps, share links and folder rules now work. Folder deny rules are honoured.

Type

  • Bug fix
  • Documentation

Testing

  • Added/updated unit tests
  • Tested against a real Postgres (integration spec, runs in CI)

New src/ee/file-search/file-search-rls.integration.spec.ts, gated on AUDIT_TEST_DATABASE_URL (already set in the backend CI job; FILES_RLS_TEST_DATABASE_URL overrides it). As the superuser it checks that share_by_token is SELECT-only, then runs the same checks in three variants: the superuser; a fresh LOGIN NOSUPERUSER NOBYPASSRLS role that owns nothing, has table/sequence grants and is a member of amcp_files; and the same role without amcp_files. The checks cover claim order and one claim per workspace, a second worker finding everything busy, renew in the wrong workspace touching 0 rows, release and stop(), scan claim and stale-fail, closeInterruptedRuns, the draft and share sweeps, share by token, signed download (a tampered org is refused), folder deny rule, isolation (A cannot read or update B, a wrong hash shows nothing, the right hash shows only that row and cannot UPDATE/DELETE), and unset, emptied or session-reset settings revealing nothing, including a row whose token_hash is ''. Negative check: with scopeFor reverted to the bare connection, the deny-rule test fails in both non-superuser variants and passes as superuser.

Commands (in packages/backend, after prisma generate):

Command Result
docker run -d --rm --name fsrls-pg -e POSTGRES_USER=amcp -e POSTGRES_PASSWORD=testpassword -e POSTGRES_DB=anythingmcp_test -p 55432:5432 postgres:17-alpine then DATABASE_URL=postgresql://amcp:testpassword@localhost:55432/anythingmcp_test npx prisma migrate deploy (from main incl. 20261106100000_tool_role_rate_limits) all migrations applied
AUDIT_TEST_DATABASE_URL=postgresql://amcp:testpassword@localhost:55432/anythingmcp_test npx jest src/ee/file-search/file-search-rls.integration.spec.ts --maxWorkers=4 18 passed
AUDIT_TEST_DATABASE_URL=… npx jest src/file-search src/ee/file-search --maxWorkers=4 17 suites passed, 1 skipped (driver live tests); 121 passed, 5 skipped
npx jest src/file-search src/ee/file-search --maxWorkers=4 (no DB) 16 passed, 2 skipped; 103 passed, 23 skipped
npx jest src/ee/file-search/sync/scheduler.spec.ts src/ee/file-search/files-worker.service.spec.ts src/file-search/file-share.service.spec.ts src/file-search/file-search-scope.spec.ts src/file-search/file-search.engine.spec.ts --maxWorkers=4 all passed (scheduler 10, worker 4, share 8, scope 8)
npx eslint <the 11 changed .ts files> (no --fix) no findings
npx tsc --noEmit -p tsconfig.json no errors
npx jest --maxWorkers=4 (full backend, no DB) 1034 suites passed, 10 skipped, 1 failed: src/auth/jwt-or-management-key.guard.spec.ts needs a live DB at DATABASE_URL (none locally; CI has one). Baseline, not touched by this PR

Checklist

  • My code follows the existing code style
  • I have tested my changes locally
  • I have updated documentation (if applicable)
  • All existing tests pass (npm test): everything except the DB-backed guard spec above, which needs CI's database
  • This PR has a descriptive title

Notes

  • Shares uploaded with share_upload have no connector_id. If a workspace later deletes every FILE_SEARCH connector, it leaves the sweep's workspace list, and its expired upload files stay in storage until a File Search connector exists again. Shares from a connector are cascade-deleted with it, which is existing behaviour.
  • Folder deny rules on an affected managed-database install were ignored until this fix (Cloud not affected).

Fixes #1418

@helpcodeai
helpcodeai merged commit 34480e6 into main Nov 6, 2026
16 checks passed
@helpcodeai
helpcodeai deleted the fix/file-search-rls-maintenance branch November 6, 2026 11:47
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File Search worker finds no work when the database user has neither superuser nor BYPASSRLS

1 participant