Skip to content

fix(auth): stop sessions carrying an organization the user left - #770

Merged
izadoesdev merged 6 commits into
mainfrom
staging
Sep 8, 2026
Merged

fix(auth): stop sessions carrying an organization the user left#770
izadoesdev merged 6 commits into
mainfrom
staging

Conversation

@izadoesdev

@izadoesdev izadoesdev commented Sep 8, 2026

Copy link
Copy Markdown
Member

Found by a referential-integrity sweep of production, not by a report.

The bug

session.create.before returned early whenever activeOrganizationId was set, without checking the user was still a member of it. So a user removed from an organization keeps that pointer on every subsequent session, and the orphan-recovery path added in a2b16e519 never runs for them.

It is also security-adjacent: Better Auth falls back to activeOrganizationId whenever hasPermission is called without an explicit organizationId. A session advertising an organization the user has no membership in is exactly the condition our permission rules warn about.

Production evidence

The sweep found 7 users with no organization membership at all (all real accounts, all email-verified) and 21 organizations with zero members, which between them own 7 websites still collecting data nobody can reach.

One user signed up on Dec 17 and was still signing in on 2026-08-28. Their sessions on Mar 26, Jul 27, Jul 28, Aug 10 and Aug 13 all point at an organization they are not a member of. Recovery was skipped every time.

What I ruled out first

  • Not a half-completed provision. The orphaned users have no organization named for them at all, so the 7 users and the 21 memberless orgs are unrelated findings.
  • Not a missing feature. Self-healing has existed since May in the same hook. The early return was the only thing that fit the timeline.

The fix

Verify membership before trusting the pointer, clear it when the membership is gone, and thread the cleared value through every remaining branch including the catch — otherwise a throw after detection hands back a session still carrying it.

33 packages typecheck, lint clean.

Not included

The 7 orphaned websites in memberless organizations are a data-recovery decision (reassign or archive), not a code fix.

Also carries #765, #766 and #768, already reviewed.


Summary by cubic

Fixes a session bug where a user who left an organization kept that organization on future sessions, and includes previously reviewed business-context improvements.

  • Auth now verifies membership before trusting activeOrganizationId, clears it when membership is gone, and carries the cleared value through every branch including the catch.
  • Team-defined priorities, success definitions, and exclusions now persist through regeneration and are delivered to chat agents with mixed provenance.
  • Investigations and resumed replies freeze the supplied business-context snapshot into the durable outcome so later profile edits don't change what was used; the insight page now shows which context was available without claiming per-claim attribution.
  • Chat preflight website-access checks now use the same organization-scoped, case-insensitive domain matching as MCP tools.
  • Dashboard business-context editor recovers unfinished briefs and team inputs across navigation and reload.
  • Business-context save, restore, and cancel are exposed over RPC with new audit actions.
  • Users with a stale organization pointer will have it cleared on their next sign-in.
  • The 7 orphaned websites in memberless organizations are a data-recovery decision, not a code fix.

Written for commit 4422cd6. Summary will update on new commits.

Review in cubic

…orities (#766)

* feat(dashboard): recover business context edits and preserve team priorities

* fix(dashboard): protect newer context drafts from late saves
* feat(ai): deliver canonical organization context to chat agents

* feat(ai): deliver structured team context and mixed provenance

* fix(ai): attribute team-defined event and success meanings once

* fix(ai): preserve business assertions when references exceed budget

* revert(ai): drop ineffective event-attribution prompt rule

The three-case present-context follow-up stayed at 16/18 on the small manual rubric, still missed the targeted emitter attribution, and increased words by 7.3%. Remove the extra rule and its tests rather than retaining prompt growth without a demonstrated benefit.

Preserve the original attributed-assertions guidance and reference-budget fallback. The small fixture context again matches the original paired run byte for byte. This reverses the rule from 7aa476d after rebasing onto merged parent #766.
session.create.before returned early whenever activeOrganizationId was
set, without checking the user was still a member of it. A user removed
from an organization keeps that pointer on every subsequent session, so
the orphan recovery added in a2b16e5 never runs for them, and the
session advertises an organization they have no membership in. That
matters because Better Auth falls back to activeOrganizationId whenever
hasPermission is called without an explicit organizationId.

Production has seven users with no membership at all. One signed up in
December and was still signing in on 2026-08-28, with sessions on Mar 26,
Jul 27, Jul 28, Aug 10 and Aug 13 all pointing at an organization they
are not a member of. Recovery had been skipped every time.

The guard now verifies membership before trusting the pointer, clears it
when the membership is gone, and threads the cleared value through the
remaining paths including the catch, so no branch can hand back a session
still carrying it.
@vercel

vercel Bot commented Sep 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
dashboard (staging) Ready Ready Preview Sep 8, 2026 10:19pm UTC
databuddy-status Ready Ready Preview Sep 8, 2026 10:19pm UTC
documentation (staging) Ready Ready Preview Sep 8, 2026 10:19pm UTC

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9905ae57-1130-43f3-adaa-cce77231cd10

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@unkey-deploy

unkey-deploy Bot commented Sep 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Unkey Deploy

Name Status Preview Inspect Updated (UTC)
links (preview) Ready Visit Preview Inspect Sep 8, 2026 10:18pm

@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR repairs stale active-organization pointers during session creation and also carries substantial business-context work from earlier changes:

  • validates an active organization before trusting it and attempts orphan-account recovery;
  • adds canonical organization business context to dashboard and shared agent prompts with organization and website authorization boundaries;
  • adds team context, persistent drafts, generation cancellation, version history and restoration;
  • hardens generation deadlines, cancellation checks, auditing, provenance handling and related tests.
  • Two correctness gaps remain in the session error fallback and generation billing idempotency, alongside two explicit repository-rule violations.

Confidence Score: 3/5

The PR is not yet safe to merge because session creation can still preserve a stale organization on membership-query failure, and a stalled generation can bill the same work twice.

The session fallback does not clear the organization until after a successful membership lookup, leaving the intended integrity repair incomplete during database failures. Separately, random billing keys do not survive BullMQ crash recovery, allowing repeated charges for one generation. The two repository-rule violations must also be resolved before merging.

Files Needing Attention: packages/auth/src/auth.ts; apps/insights/src/organization-business-context.ts; apps/api/src/routes/agent-business-context.test.ts; apps/dashboard/app/(main)/organizations/components/business-context-editor.tsx

Important Files Changed

Filename Overview
packages/auth/src/auth.ts Adds membership validation and orphan recovery for session organization pointers, but the first-query exception path can still return the original stale pointer.
apps/insights/src/organization-business-context.ts Adds deadline, cancellation and billing accounting hardening, but regenerates billing idempotency keys when stalled work is processed again.
packages/services/src/organization-business-context.ts Adds transactional history, restore, cancellation, structured team context and conservative provenance handling.
packages/ai/src/lib/organization-business-context.ts Introduces a bounded, authorization-gated canonical profile loader and prompt formatter with escaped untrusted content.
apps/dashboard/app/(main)/organizations/components/business-context-editor.tsx Adds draft recovery, team fields, generation cancellation and history review, with a repository accessibility-rule violation on heading focus.
apps/api/src/routes/agent-business-context.test.ts Adds end-to-end model-boundary coverage but uses prohibited unknown types in several new mocks.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Create session] --> B{Active organization set?}
  B -- No --> E[Find another membership or provision default organization]
  B -- Yes --> C[Query active membership]
  C -- Found --> D[Keep active organization]
  C -- Missing --> F[Clear stale pointer]
  F --> E
  C -- Query throws --> G[Current code returns original session data]
  G --> H[Stale organization may remain]

  I[Business-context generation job] --> J[Run model phase]
  J --> K[Bill using generation + phase + random UUID]
  K --> L{Completion persisted?}
  L -- Yes --> M[Publish ready draft]
  L -- Worker crashes --> N[Stalled job is processed again]
  N --> J
  N --> O[New UUID defeats charge deduplication]
Loading

Reviews (1): Last reviewed commit: "fix(auth): stop sessions carrying an org..." | Re-trigger Greptile

Comment thread packages/auth/src/auth.ts
}

return { data: sessionData };
return { data: base };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stale organization survives errors

If the new membership lookup throws, base still contains the original activeOrganizationId, so the catch returns the stale pointer this change is intended to remove. A transient database failure during session creation can therefore persist another session with an organization the user may have left. Initialize the fallback with a cleared organization before the lookup, and restore the original value only after membership is confirmed.

let billingFailure: Error | undefined;
const model = getAILogger().wrap(createModelFromId(MODEL));
const options = (phase: string) => {
const key = `org-business-context:${input.generationId}:${phase}:${randomUUID()}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Retries can double bill

The billing idempotency key contains a fresh randomUUID(), so processing the same stalled generation again produces a different key. If a worker charges successfully and crashes before saving completion, the still-running job can be processed again and charged under the new key. Use a stable key derived from the generation and phase so the billing provider can deduplicate crash recovery.

profile: null as OrganizationBusinessProfile | null,
read: vi.fn(),
prompts: [] as Parameters<MockLanguageModelV3["doStream"]>[0][],
contexts: [] as Record<string, unknown>[],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Prohibited unknown types added

The new test uses Record<string, unknown> instead of an explicit context type, contrary to the repository directive not to use any, unknown, or never. The same pattern appears in the changed createConfig parameter at line 112 and model wrapper at line 148. Define explicit model-context and input types before merging.

Context Used: Basic guidelines for the project so vibe coders do... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

{review === "generation"
<Dialog.Title
render={(props) => (
<h2 {...props} ref={reviewTitleRef} tabIndex={-1}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Heading violates focus rule

This assigns tabIndex to a non-interactive <h2>, violating the repository directive not to put tabIndex on non-interactive elements. Use the dialog primitive's supported focus target instead. This repository requirement must be satisfied before merging.

Context Used: Ultracite Rules - AI-Ready Formatter and Linter (source)

@blacksmith-sh

This comment has been minimized.

)

* feat(insights): retain supplied business context on investigations

* fix(dashboard): clarify business context disclosure

* fix(dashboard): avoid claiming unavailable business context
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