Skip to content

feat(byok): add organization-wide key inheritance - #6834

Merged
icecrasher321 merged 21 commits into
stagingfrom
feat/organization-byok-inheritance
Aug 21, 2026
Merged

feat(byok): add organization-wide key inheritance#6834
icecrasher321 merged 21 commits into
stagingfrom
feat/organization-byok-inheritance

Conversation

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor

Summary

  • Add encrypted organization-owned BYOK key pools managed from the existing BYOK settings page.
  • Resolve provider credentials workspace-first, then inherit the current organization pool when the workspace has no local key for that provider.
  • Restrict organization key management to organization owners/admins, enforce Enterprise entitlement for writes and runtime use, and retain delete access after downgrade.
  • Show inherited provider status without exposing organization key metadata to workspace viewers.

Organization keys intentionally apply to executions in all current and future organization workspaces unless a workspace defines its own key for that provider. Existing provider fallback behavior remains unchanged when neither BYOK scope supplies a key.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

  • Focused BYOK tests: 4 files, 51 tests
  • Full @sim/app suite: 2,024 files and 27,612 tests
  • App, database, and testing package type checks
  • App lint and formatting
  • Strict API validation: 1123/1123 routes
  • React Query and package/client boundary audits
  • Migration safety and Drizzle schema-drift checks
  • git diff --check
  • Manual workspace/organization scope, inheritance, override, downgrade, permission-loss, and error-state validation

Reviewers should focus on the workspace-first resolver boundary, organization authorization and downgrade behavior, and the intentionally organization-wide credential blast radius.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

Manual UI validation completed locally for both BYOK scopes, inherited badges, downgrade controls, and inherited-status failure handling.

@vercel

vercel Bot commented Aug 19, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 21, 2026 12:38am

Request Review

@cursor

cursor Bot commented Aug 19, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Touches encrypted provider credentials, org-wide inheritance into every workspace (including Pi sandboxes), and billing-gated access. A resolver or authz bug could leak or misuse organization API keys.

Overview
Organization admins can now store encrypted BYOK keys once for the whole org. Runs still prefer a workspace key per provider, then inherit the live org pool, then Sim-hosted keys.

Resolution (getBYOKKey) treats a nonempty workspace pool as exclusive — it never falls through to org keys if decrypt fails. Org keys are entitlement-gated before decrypt/rotation, round-robin across member workspaces, and tagged with scope in logs. Pi Create/Update PR and Plan still inject the resolved key (including inherited org keys) into the sandbox; Pi search still requires an explicit block key.

Management is session-only for org admins/owners. Writes need an active org plan (Pro/Max for Teams or Enterprise); list/delete remain after lapse. Workspace viewers only see inherited provider IDs, not org key metadata. The BYOK settings page adds Workspace/Organization scope, inherited badges, and granular add/update/delete capabilities.

Also adds organization_byok_keys, ORGANIZATION_PLAN_REQUIRED, resolveOrganizationPlan / checkOrgPlan, and documents the new providers, scopes, and Pi sandbox warning.

Reviewed by Cursor Bugbot for commit 02fd983. Configure here.

@gitguardian

gitguardian Bot commented Aug 19, 2026

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
36334842 Triggered Generic High Entropy Secret 24859cb apps/sim/lib/api-key/byok.test.ts View secret
36334841 Triggered Generic High Entropy Secret 24859cb apps/sim/lib/api-key/byok.test.ts View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@greptile-apps

greptile-apps Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds encrypted organization-owned BYOK pools with workspace-first credential resolution and plan-aware organization inheritance.

  • Adds organization key management APIs, authorization, auditing, persistence, and settings UI.
  • Extends model, embedding, and knowledge execution paths to inherit organization keys when no workspace override exists.
  • Adds organization-plan entitlement handling, inherited-provider status, documentation, and focused tests.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/api-key/byok.ts Implements workspace-first credential selection and organization-level fallback while preserving existing provider fallback behavior.
apps/sim/lib/api-key/application/organization-byok-keys.ts Adds authorized organization key listing, mutation, deletion, auditing, and inherited-status operations.
apps/sim/lib/api-key/byok-entitlement.ts Adds fresh management-path and cached execution-path checks for organization-plan entitlement.
apps/sim/lib/billing/core/subscription.ts Adds organization-plan resolution for plan-gated BYOK management and runtime use.
packages/db/schema.ts Adds the organization BYOK key table definition corresponding to migration 0296.
packages/db/migrations/0296_organization_byok_keys.sql Creates encrypted organization key storage with organization and creator foreign keys plus provider lookup indexing.
apps/sim/app/workspace/[workspaceId]/settings/components/byok/byok.tsx Extends the BYOK settings surface with workspace and organization scopes and inherited-provider presentation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Run[Workspace execution] --> Local{Workspace key exists<br/>for provider?}
  Local -->|Yes| WorkspaceKey[Use workspace key pool]
  Local -->|No| Org{Workspace belongs to<br/>an entitled organization?}
  Org -->|Yes| OrganizationKey[Use organization key pool]
  Org -->|No| ExistingFallback[Use existing hosted or explicit-key fallback]
  WorkspaceKey --> Provider[Call provider]
  OrganizationKey --> Provider
  ExistingFallback --> Provider
Loading

Reviews (4): Last reviewed commit: "fix(byok): keep an abandoned entitlement..." | Re-trigger Greptile

Comment thread apps/sim/hooks/queries/byok-keys.ts
@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

The Org Admin can see this:

image

Everyone else can't even see this toggle.

For the admin, switched to "Organization" it looks like this:

image

For the rest, it looks like this:

image

Staging independently claimed 0293. Remove ours so the merge is clean;
it is regenerated at the next free index right after.
…anization-byok-inheritance

# Conflicts:
#	scripts/check-api-validation-contracts.ts
Staging claimed 0293-0295 during the merge; the regenerated SQL is
byte-identical to the dropped 0293.
…vider list

The BYOK section described workspace-scoped keys only. Add the organization
scope, its Enterprise requirement, the per-provider precedence rule, what an
entitlement lapse does, and the Pi sandbox exposure. Refresh the provider
table from the settings page, which had drifted from 14 to 34 entries.
Organization BYOK was gated on Enterprise, but an organization is the only
thing that can hold the keys, so every plan that can own an organization
should qualify — Pro for Teams, Max for Teams, and Enterprise.

Add checkOrgPlan/resolveOrganizationPlan beside the Enterprise pair rather
than widening checkEnterprisePlan, so the Enterprise-only gates (Access
Control, whitelabeling) are untouched, and restore
resolveOrganizationEnterprisePlan to module-private now that BYOK no longer
needs it.
getBYOKKey runs once per agent block and once per hosted-capable tool call,
so a loop over N items resolved N times — and each organization-inheriting
resolution paid three sequential billing queries on top of the two key reads.

Split the two reads by staleness tolerance. Key rows stay fresh, because
revocation must be immediate. The entitlement is a billing gate that tolerates
bounded staleness in the harmless direction (a lapsed organization keeps using
its own key for <=60s), so cache it per organization with an in-flight share so
concurrent blocks issue one query set. The management surfaces keep reading it
fresh, so an organization that just upgraded is never told otherwise.

Also run the block check and subscription read in parallel inside
resolveOrganizationPlan, and carry the resolved scope on BYOKKeyResult so a log
line can say whether a run used the workspace's key or an inherited one.
Comment thread apps/sim/lib/api-key/byok-entitlement.ts Outdated
… reads

Both ids were already in the BYOK contract enum and both are resolved at
execution time — getApiKeyWithBYOK reaches 'zai' (GLM models are in the hosted
catalog, so the BYOK branch runs), and 'cohere' backs both the Embeddings block
and Knowledge Base reranking — but neither appeared in the settings list, so
there was no way to store the key either path looks for.

Cohere had no icon; add one from the official multi-color mark so it stays
legible on a light and a dark page.

Cohere's embed-v4.0 is kbEligible:false, so the description says 'Embeddings
and Knowledge Base reranking' rather than claiming KB embeddings.
It sits beside 'Organization', so the scope reads from the pair; 'This'
only added width.
@icecrasher321

Copy link
Copy Markdown
Collaborator

@greptile

resolveOrganizationPlan maps a failed billing read to false, which is
indistinguishable from a real plan lapse. The entitlement cache stored that,
so one transient outage held the gate shut for the full TTL and every
inheriting run silently fell back to a metered hosted key — and the cache's
rejection path, which exists to prevent exactly this, was unreachable.

Give the resolver the onError option its neighbours already have and let the
cached read ask for 'throw', so a failure stays out of the cache and the next
resolution retries. Behavior for the call that saw the error is unchanged:
getBYOKKey still fails closed.

Reported by Cursor Bugbot.
Comment thread apps/sim/lib/billing/core/subscription.ts
The previous commit threaded onError through resolveOrganizationPlan's own
catch, but getOrganizationSubscriptionUsable soft-fails to null on its own, so
a failed subscription read still arrived as an ordinary 'no usable
subscription' and returned a successful false — which the entitlement cache
then stored for the full TTL. Thread the option into that call as well.

Test it at the billing layer rather than the cache layer: the entitlement test
mocks resolveOrganizationPlan wholesale, so it could never have caught this.
Verified the new test fails against the previous commit.

Reported by Cursor Bugbot.
@icecrasher321

Copy link
Copy Markdown
Collaborator

@greptile

…anization-byok-inheritance

# Conflicts:
#	scripts/check-api-validation-contracts.ts
…itlements

The hand-rolled version reinvented three things the codebase already has a
canonical answer for. lru-cache is a declared dependency of apps/sim and
lib/copilot/entitlements.ts already caches an entitlement with it — by storing
the in-flight Promise, which is what makes concurrent callers collapse onto one
resolution with no in-flight bookkeeping at all. TTL and the size bound come
from the library.

That removes the second Map, the manual eviction (and its interaction with an
in-flight entry), and the dead value-while-refreshing state: 23 executable
lines. The one thing the library does not cover is dropping a rejected promise
so a billing outage is not cached for the TTL, which is kept and pinned by a
test that fails without it.

TTL expiry is no longer re-tested — that is the library's behavior, not ours,
and lru-cache reads its clock at module load so faking timers never moved it.
Comment thread apps/sim/lib/api-key/byok-entitlement.ts Outdated
…light

lib/concurrency/singleflight.ts is the codebase's coalescing primitive and
oauth/credential-service.ts already pairs it with a read-through cache. Adopting
that shape fixes a case caching the promise directly did not: a *hung* billing
read wedged every caller for the full 60s TTL, where coalesceLocally evicts and
rejects at its settle deadline.

It also removes the hand-rolled rejection eviction — the cache is written only
on the success path, so an outage leaves no entry by construction.

The cache now holds booleans, which introduces the one trap worth a test: a
truthiness check would read a cached false as a miss and re-query billing on
every resolution for lapsed organizations. Pinned.
Comment thread apps/sim/lib/api-key/byok-entitlement.ts Outdated
…anization-byok-inheritance

# Conflicts:
#	scripts/check-api-validation-contracts.ts
coalesceLocally does not cancel a producer it timed out — its docstring says so
explicitly — so writing the cache from inside the producer let a late billing
result overwrite a fresher answer a retry had already cached, and hold it for a
full TTL.

Move the write onto the value the caller actually received. A caller that timed
out throws before reaching it, so an abandoned producer now resolves into
nothing. The test reproduces the overwrite and fails against the previous shape.

Reported by Cursor Bugbot.
@icecrasher321

Copy link
Copy Markdown
Collaborator

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 02fd983. Configure here.

@icecrasher321
icecrasher321 merged commit 42f6287 into staging Aug 21, 2026
30 checks passed
@icecrasher321
icecrasher321 deleted the feat/organization-byok-inheritance branch August 21, 2026 00:45
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.

2 participants