fix(ingest): enforce the per-system boundary on crawler ingest (SEC-2026-09 C-01, H-04, M-05, M-07, L-16)

[WimvandenHeijkant]

Summary

Restores the per-system boundary on crawler ingest and hardens the crawler data plane, following the SEC-2026-09 white-box review. Four commits, one per concern:

1. C-01 + H-04: systems are never reconciled, the reconcile delete always needs a bound, and a key restricted to specific systems stays inside them.
2. M-07: caps on open ingest sessions, session ownership, and an idle-in-transaction timeout.
3. M-05: the five data-plane endpoints are gated and scoped.
4. L-16: bounds on extendedAttributes, and a cap on the attribute keys that column discovery returns.

I re-checked every finding against main @ 086a1a322. All five still reproduced before this change. C-01 was reproduced by running the real ingest() against a migrated schema: a one-record full sync to Systems deleted the other two systems.

Findings addressed

ID	Severity	What changed
C-01	Critical	POST /ingest/systems runs a full batch as a delta, logs a warning, and deletes nothing. Both Omada registration calls now send delta, and the Omada crawler skips the empty batch. scopedDelete refuses any reconcile that has no system, scope or ownership predicate: it logs the refusal and returns deleted: 0. This applies to non-empty batches too.
H-04	High	New module ingest/systemBoundary.js. For a key with systemIds set, the API returns 403 when: • a record's systemId (or a context's scopeSystemId) is outside the key's systems; • the batch would update an existing row another system owns; • the batch is a full sync with no scope on a table that has no systemId column. Ownership is checked with one indexed jsonb_populate_recordset lookup. For tables without a systemId it is derived: a synced context's scopeSystemId, a context member's context, the principal of an identity member or activity row, and an identity's linked principals. deletedIds, the full-sync reconcile and the session reconcile only reach owned rows. For every caller, server-managed columns (deletedAt, riskScore, riskTier, linkConfidence, analystOverride, analystVerified, analystNotes, createdAt, updatedAt, …) are dropped from the writable column set. A field with one of those names lands in extendedAttributes like any other unknown field. For /ingest/systems, a restricted key can register new systems but cannot update one outside its systemIds.
M-05	Medium	• classify-business-role-assignments requires refreshViews. It is limited to an accessible body.systemId, or to the key's own systems, and an unrestricted key keeps the tenant-wide pass. • Matview refreshes from classify and refresh-views go through a serialised runner. Only one refresh runs at a time, and callers that arrive mid-refresh share one follow-up. That follow-up starts at least 5 s after the run it waited for (MATRIX_REFRESH_MIN_INTERVAL_MS). A call that finds nothing in flight runs immediately. • matrix-default-filter requires admin. • sync-log stamps crawlerId and an accessible systemId, added by migration 064. It also caps text fields and rejects dates that don't parse. • principals-presence is limited to the restricted key's systems. • crawlers/job-progress is worker-only.
M-07	Medium	• Open sessions are capped at 3 per crawler and 7 in total, configurable with INGEST_MAX_SESSIONS_PER_CRAWLER and INGEST_MAX_SESSIONS_GLOBAL. A start past either cap gets 429. • The slot is reserved before the first await, so concurrent starts can't slip past the cap. • A session that fails to open now releases its connection. It used to leak it. • A session is only visible to the crawler that opened it. • Every pooled connection gets idle_in_transaction_session_timeout of 10 min (PG_IDLE_IN_TRANSACTION_TIMEOUT_MS). A session sets its own 30-min bound with SET LOCAL.
L-16	Low	• A record's packed extendedAttributes is limited to 500 keys and 512 KB; anything larger gets 400. • discoverExtendedAttrValues returns only the 300 most frequent safe keys, and unsafe names are filtered in SQL before the cap.

Not addressed / deferred

• statement_timeout: deliberately not set. Matview refreshes, the full-sync reconcile on large tenants and migration index builds (one took about 11 min) legitimately run for many minutes. A statement cap tight enough to stop abuse would break them. The M-07 exposure is covered by the session caps and the idle-in-transaction timeout instead.

• systemId = EXCLUDED.systemId on the upsert (report fix 3): not done, by design. Shipped crawlers legitimately write one row under several systems through the built-in worker:

  • Azure RM principal stubs are existing Entra principal rows.
  • Built-in Entra directory-role ids are the same in every tenant.
  • CSV deterministic ids are shared across SystemName systems.

  A blanket predicate would silently stop those writes. Ownership is enforced by access for restricted keys instead.

• Record systemId is checked, not overwritten from the envelope. The demo dataset loader sends per-record system ids that match its envelope, so checking by access changes the least.

• Reconcile of tables without a systemId column by unrestricted keys is unchanged. Entra, Omada, CSV and the demo loader full-sync Identities, IdentityMembers and ContextMembers without a scope and rely on that reconcile. Restricting it would leave departed users' identities in place forever.

• Out of my lane, noticed: setup/docker/Build-FGContexts.ps1 posts to /api/ingest/refresh-contexts, which has no route. The 404 is swallowed. This is not a security issue.

Compatibility

• Built-in worker and every shipped crawler: no behaviour change beyond the Omada full→delta switch.

  • The worker has systemIds = null, so it skips every ownership check.
  • The worker holds ingest, refreshViews and admin, so classify, matrix-default-filter and job-progress keep working.
  • No shipped producer sends any server-managed column.

• An older worker image (or an external script) that still sends syncMode: 'full' to /ingest/systems keeps working. The batch is run as a delta rather than rejected.

• External connectors with a key restricted to specific systems may now get 403 where they previously succeeded. That happens when they:

  • write rows another system owns;
  • full-sync identities, identity-members or principal-activity (these have no scope map), or contexts or context-members without a scope;
  • update contexts they created without setting scopeSystemId;
  • update identities that have no linked principals of their own.

  All of these are the boundary working as intended. Unrestricted keys are unaffected.

• External keys that only have ingest can no longer call classify-business-role-assignments (needs refreshViews), matrix-default-filter (needs admin) or crawlers/job-progress (worker only). The only shipped callers are the CSV crawler, the demo loader and the dispatcher, all on the worker key. A standalone Ingest-DemoDataset.ps1 run with a non-worker key will fail to seed the default matrix filter.

• Session caps: shipped chunking is sequential (one session at a time). SCIM user buckets and midPoint per-resource streams can overlap, and they run on the worker key, which is only bound by the global cap of 7. Beyond 7 concurrent streams, the pool of 10 was already exhausted before this change.

• Migration 064 adds two nullable columns to GraphSyncLog (ADD COLUMN IF NOT EXISTS) and needs no backfill. Its hash is added to .ci/migration-hashes.json.

• Mutation-scope guard: the four new modules are recorded in stryker.features.config.json under mutationExclusions with a "not yet measured" reason. Stryker's vitest runner doesn't activate mutants on Windows: the existing featureFlags.js scope scored 18.5% locally against 100% in CI, so no honest score could be produced. They should be measured on Linux and moved into a scope.

Tests

• API unit (Vitest): 3135 passed, 238 files. npm run lint: 0 errors, and the 5 existing warnings are unchanged. npm run test:coverage is above the aggregate floors, and every touched file is above its per-file floor. New or extended suites:

  • systemBoundary.test.js
  • sessions.limits.test.js
  • dataPlane.test.js
  • serializedRunner.test.js
  • connectionDefaults.test.js
  • engine.test.js, engine.ingest.test.js
  • ingest.coverage.test.js, ingest.helpers.test.js
  • crawlerPresence.test.js, columnCache.test.js, normalization.test.js
  • crawlers.selfservice.authz.test.js

• Discriminating check: with the source changes stashed, all 19 new route-level tests failed. The new session-cap concurrency test lets all 5 starts through without the synchronous slot reservation.

• Contract (contract-tests/ingestSystemBoundary.integration.test.js): real app, real crawler keys, two seeded systems. Covers:

  • a one-record full sync to /ingest/systems leaves both systems in place;
  • a restricted key is refused when it forges a system, overwrites by id, updates a foreign Systems row, or full-syncs identities without a scope;
  • deletedIds and the full-sync reconcile only reach the key's own system.

  Docker isn't available here, so I ran the same file through vitest against the real app on a migrated in-process Postgres (PGlite, desktop mode). It passed 7/7, and all 7 fail with the source changes stashed. I also checked the SQL against PGlite for the ownership predicates on every table, scoped deletedIds, the restricted reconcile, the presence filter, the key-cap query, the sync-log insert and SET LOCAL.

• Pester: OmadaCrawlerPhases.Tests.ps1 and OmadaCrawlerFunctions.Tests.ps1 pass 122/122. The two new Omada tests fail on the old code, which sent full twice. PSScriptAnalyzer found no new findings on the touched files, and Register-OmadaSystems measures cc 13 / cog 14.

• Complexity (the ratchet's ESLint rules, run locally): every touched JS unit is under cc 20 and cognitive 15. jscpd found no clones involving the changed files.

Also in this PR

• tools/crawlers/demo/ConfigWizard.e2e.mjs: the demo re-import afterAll hook polls for up to 2 minutes, but it ran under Playwright's default 30 s hook timeout. It now sets its own timeout. This was a latent flake that an earlier revision of this PR made reproducible.

Manual follow-ups

• None outside the repo.
• Operators who give external connectors keys restricted to specific systems should confirm those connectors set scopeSystemId on their contexts, send a scope on full syncs of contexts, and use delta for identities.

🤖 Generated with Claude Code