Skip to content

fix(auth): contain automatic Slack identity linking by email - #7498

Draft
bokelley wants to merge 6 commits into
mainfrom
conductor/slack-automatic-email-link-containment
Draft

bokelley wants to merge 6 commits into
mainfrom
conductor/slack-automatic-email-link-containment

Conversation

@bokelley

@bokelley bokelley commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

An automatic match against denormalized email can bind a Slack principal to an existing WorkOS credential, lending that credential's platform-admin or committee-leader authority to the Slack principal. This change refuses mapping_source: 'email_auto' at SlackDatabase.mapUser before credential lifecycle entry, authorization epoch changes, mapping SQL, or name backfill, including when a caller supplies a transaction client.

Containment only: #6827 remains open. Explicit token-correlated user_claimed and global-admin manual_admin linking retain their existing behavior. Historical mappings and historical side effects remain effective; this is not complete Slack reversal or integrity remediation.

Exact publication boundary

Item Pinned value
Base branch fix/6827-identity-containment / #7459
Base and sole parent 9a7cd5362787a0badbfccd0f1cb1dd002654c22f
Implementation head ee7076b5bb50e23a4553045a144d28b2f7925cde
Implementation tree 0ae2b26d8dbbd36e0b2021e575b70aa00cd6f723
PR delta 1 commit, 10 files, +577 / -40

This is a sibling containment slice on exact #7459, not a stack on #7464 or #7460. Only the implementation head is published. The synthetic local #7450 composition commit and tree are excluded from this branch and PR.

No migration, historical cleanup, or protocol changeset. Draft review only; auto-merge must remain off. No ready-for-review transition, approval, merge, deployment, or settings change is authorized by this publication.

GitHub CI coverage and landing requirements

This PR is not landing-ready. GitHub currently has only stacked-PR metadata and auxiliary checks on exact head ee7076b5bb50e23a4553045a144d28b2f7925cde; full implementation CI is not scheduled and cannot schedule through the current pull_request workflow filters while the base is fix/6827-identity-containment / #7459.

Build Check and Dependency Review accept PR bases main, develop, 3.1.x, and 3.0.x; CodeQL accepts main, 3.1.x, and 3.0.x; Semgrep, Check Migration Numbers, and Migration Smoke Test accept main. None accepts this stack base. Passing metadata checks therefore do not establish a full GitHub CI pass. The current checks are IPR Agreement, IPR Policy / Signature, and GitGuardian (passed), with AI PR Review and Mintlify (skipped).

The coordinator reports that root independently reviewed exact #7498, found no blocker, and passed 26/26 focused tests. That read-only review and the local validation below do not replace the missing GitHub CI or constitute approval to land.

After #7459 lands, this slice must be restacked onto main, receive full GitHub CI on the exact resulting candidate head, receive a fresh security review of that same head, and obtain human approval before landing. This requirement does not authorize restacking or changing this PR's head, base, Draft state, reviewers, or auto-merge now. Containment only; #6827 remains open.

Caller coverage and stable outcomes

The seam throws SlackEmailAutoLinkContainedError with stable code slack_email_auto_linking_disabled, distinct from infrastructure failure. All three production email-auto callers recognize it before link-derived preference transfer, chapter or organization writes, downstream WorkOS membership calls, success caches, or link counters.

Entry point Handling
Slack event dispatch / team_join tryAutoMapByEmail returns contained; a matched attempt stops before profile upsert, preference transfer, welcome/prospect work, chapter sync, or success invalidation, including when no Slack discovery row exists yet.
Web OAuth /auth/callback email fallback tryAutoLinkWebsiteUserToSlack returns linked: false with the stable reason; existing link-success branches are not entered.
WorkOS user.created Same contained result after independent signed-provider user ingestion.
Active WorkOS organization_membership.created Same contained result after independent signed-provider membership ingestion.
Scheduled slack-auto-link job Bulk pass reports contained separately from errors; zero linked/chapter/org/hint counters; no mapped-success set insertion.
Admin POST /api/admin/slack/auto-link-suggested HTTP 200 with containment counts and zero link success; bulk profile refresh and pending-organization backfill have been removed from this pass.
Admin POST /api/admin/slack/sync Inventory verified this is standalone profile ingestion; it remains available with auto_mapped: 0 and no identity mapping calls.

user_change, Slack channel/message activity, and WorkOS update webhooks do not add another email-auto mapping implementation. Contacts' separate email_auto source is unrelated to Slack identity linking. Generic explicit link/unlink behavior is not expanded.

Validation on the exact implementation tree

Counts below overlap where focused files are included in broader suites; they are not additive totals.

Validation Result
Normal pre-commit root unit suite 71 files, 1,063 passed
Normal pre-commit full server unit suite 616 files, 9,418 passed, 31 skipped (9,449 total; 1,072.60 seconds)
Focused containment unit file 12 passed
Real OAuth callback / registered scheduler wiring file 14 passed
Docker PostgreSQL 16 focused integration suites 11 files, 92 passed, including the 12-test PostgreSQL containment file
Migration filename/version guard 3 passed; no migration diff
Source typecheck and fixed-trace test-aware typecheck Passed
Image-quality, dynamic-import, format-identity, callapi-state-change, and diff guards Passed
Fresh independent security review PASS, no findings or unresolved blockers; final head, tree, sole parent, and clean worktree verified

The implementation commit passed the normal pre-commit hook without bypasses. At publication, the advisory pre-push storyboard matrix was selected by the hook's fallback comparison to origin/main, although this PR changes no training-agent or compliance files. All seven tenant runs failed capability resolution before producing test totals: the local compliance cache is 3.2.0-rc.2, while the test seller advertises through 3.2-rc.1. Publication retried with the documented ADCP_SKIP_STORYBOARD_MATRIX=1 override, which also skips the subsequent local 3.0 storyboard compatibility run. Full GitHub CI remains required for landing but cannot schedule against the current stack base, as explained above. Version synchronization, changeset scope, schema-link checks, and 55/55 documentation navigation tests passed on the retry. No implementation changes or hook edits were made.

Focused commands:

npx vitest run --config server/vitest.config.ts tests/unit/slack-email-auto-containment.test.ts
npx vitest run --config server/vitest.config.ts tests/unit/native-auth-http-wiring.test.ts
# DATABASE_URL points only to the isolated Docker PostgreSQL fixture.
npx vitest run --config server/vitest.config.ts \
  tests/integration/slack-email-auto-containment.test.ts \
  tests/integration/slack-db-name-backfill.test.ts \
  tests/integration/workos-user-deletion-race.test.ts \
  tests/integration/workos-user-deleted-primary.test.ts \
  tests/integration/mcp-oauth-deletion-race.test.ts \
  tests/integration/authorization-deletion-cross-replica.test.ts \
  tests/integration/authorization-epoch.test.ts \
  tests/integration/identity-layer.test.ts \
  tests/integration/admin-link-unlink-credential.test.ts \
  tests/integration/admin-promote-credential.test.ts \
  tests/integration/workos-sync-email-domain.test.ts
npm run typecheck
npm run test:migrations
git diff --check 9a7cd5362787a0badbfccd0f1cb1dd002654c22f ee7076b5bb50e23a4553045a144d28b2f7925cde
git diff --exit-code 9a7cd5362787a0badbfccd0f1cb1dd002654c22f ee7076b5bb50e23a4553045a144d28b2f7925cde -- server/src/db/migrations

PostgreSQL tests compare complete rows serialized by PostgreSQL, including timestamps and metadata: Slack mappings, users, identity/credential edges, authorization epochs, organizations/memberships/domains, working groups/memberships/leaders/topics, administrator audit rows, preferences, aliases, relationships, and #7459's user CASCADE/SET NULL snapshot inventory. Repeated active-admin and ordinary committee-leader attempts remain inert and do not acquire Slack authority. Historical mappings of every source remain unchanged; explicit manual linking is a positive authority control.

Two #7459 deletion-race fixture writers now use explicit user_claimed / manual_admin sources so they still reach and test the lifecycle fence. New email-auto tests separately prove rejection happens earlier. The deletion/race/recovery assertions remain in place.

Deterministic composition plan

Exact #7450: 5595e6ce6eda4d8faf54b92c7b93c6b1c63b1d1e

A separate local composition was tested; it is not included in this PR. It proved real resolveSlackUserAAOAdminAccess returns forbidden/unmapped and canScheduleMeetings remains false after containment, with positive explicit-link controls for both an administrator and an ordinary committee leader. The independent reviewer checked the semantic reconciliation; #7450's administrator implementation remained byte-identical to its published version.

To reproduce after separate composition authorization:

  1. Start a detached scratch worktree at the pinned implementation head and merge the exact fix(addie): authorize admin tools by authenticated credential #7450 hash without committing. Preserve the implementation branch.
  2. Resolve the four baseline conflicts in server/src/addie/admin-status-lookup.ts, server/src/addie/mcp/admin-tools.ts, server/tests/unit/aao-admin-status.test.ts, and server/tests/unit/addie-admin-global-auth.test.ts. Retain fix(addie): authorize admin tools by authenticated credential #7450's uncached, typed credential-based authority lookups and its ID-only group lookup; retain fix(auth): contain destructive identity consolidation #7459's credential lifecycle fixture and repeated positive-lookup/freshness controls. Do not restore positive or negative administrator caching.
  3. Adapt tests that reference removed admin-cache getters to repeated real authority lookups. Preserve every substantive deletion/race/recovery assertion. In the containment integration file, assert the typed forbidden result and scheduling denial; use explicit administrator and ordinary-leader links as positive controls.
  4. Rerun the containment, name-backfill, deletion/lifecycle/epoch, administrator, meeting, content, and voice authorization suites and source typecheck. Record the resulting tree and both exact parents separately.

Local composition evidence: 20 selected unit/integration files, 265 final tests passed, with changed files rechecked after adaptation. The initial run had 263/264 passed and one legacy deleted-primary test failed because #7450 removed the cache getter. After the test compatibility adjustment, its entire 23-test file passed. The enhanced composition containment file then passed 13 tests, including the added ordinary-leader positive control. Source typecheck and diff guards passed. These are composition results, not additional tests or commits in this PR, and do not claim a full audit of all #7450 surfaces.

Exact #7476: 71e684617cadd0d52683f11e55b51002fe2233a2

Start from this slice's server/src/slack/sync.ts and add exactly:

  • import { withAuthorizationEpochBump } from '../db/authorization-epoch-db.js';
  • Wrap the retained local organization_memberships INSERT in checkAndAssignOrganizationByDomain with withAuthorizationEpochBump([workosUserId], client => client.query(...)).
  • Wrap the corresponding retained INSERT in autoAddVerifiedDomainUsersAsMembers with withAuthorizationEpochBump([user.workos_user_id], client => client.query(...)).

Preserve the email containment import/catches, mapping-result fences, and both #7459 early domain-assignment deferrals. Taking #7476's whole file would remove those deferrals and the website lifecycle-result fence. The import overlap was reconciled in a local copy; the resulting textual delta contains only the import and two epoch wrappers. This is a deterministic textual reconciliation, not a runtime #7476 composition claim.

Broader #7476 integration has baseline conflicts outside sync.ts. Preserve #7459's credential deletion protections while adding #7476's withAuthorizationEpochBump, getExactCredentialAuthorizationEpoch, and dispatch-time credential/epoch/administrator/organization checks. Rerun credential-grant-epoch-routes, authorization-epoch, addie/slack-mutation-authority, addie/slack-mutation-production-wiring, addie/mutation-authority, and the containment/deletion suites. An incorrectly established email binding would pass subsequent epoch checks, which is why refusal must remain before establishment.

Limitations

  • Historical email-auto mappings, grants, preferences, and name changes remain effective. No migration, reversal, historical cleanup, or complete Slack integrity claim.
  • Standalone profile sync and ordinary profile events still ingest Slack metadata. Signed WorkOS ingestion and authenticated login updates remain independent. Row no-op assertions isolate the email-link branch and its downstream effects; they do not claim entire provider events or logins are mutation-free.
  • Slack/WorkOS providers are mocked. OAuth tests execute the real HTTP callback with mocked database/provider dependencies and include caught missing-mock methods in independent login/relationship work; they are not full live-provider login tests. Actual row and administrator/leader authority assertions use PostgreSQL. No live-provider or production verification was performed.
  • Generic explicit linking/unlinking integrity and the rest of Identity linking: isolate organization authorization to the authenticated credential #6827 remain outside this containment slice.

Open workspace in Conductor

Draft disposition — 2026-10-09

This draft is retained for the stated workstream. Continue on this existing branch; no parallel PR is needed. This disposition does not waive CI, review, migration, or release gates.

@bokelley
bokelley requested review from a team and bmilekic September 14, 2026 07:17
@bokelley
bokelley force-pushed the fix/6827-identity-containment branch 3 times, most recently from 4120a39 to 6e35597 Compare September 14, 2026 20:40
@bokelley
bokelley force-pushed the fix/6827-identity-containment branch 2 times, most recently from c5873d6 to 9fa7707 Compare September 17, 2026 12:47
Base automatically changed from fix/6827-identity-containment to main September 17, 2026 12:59

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant