feat(byok): add organization-wide key inheritance - #6834
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview Resolution ( 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 Reviewed by Cursor Bugbot for commit 02fd983. Configure here. |
|
| 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
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- 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
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 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 SummaryThe PR adds encrypted organization-owned BYOK pools with workspace-first credential resolution and plan-aware organization inheritance.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope. No blocking failure remains.
|
| 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
Reviews (4): Last reviewed commit: "fix(byok): keep an abandoned entitlement..." | Re-trigger Greptile
d3c82b7 to
fbee11d
Compare
349fa0b to
9f8ff6e
Compare
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.
… 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.
…anization-byok-inheritance
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.
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.
…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.
…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.
…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.
|
@cursor review |
There was a problem hiding this comment.
✅ 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.


Summary
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
Testing
Reviewers should focus on the workspace-first resolver boundary, organization authorization and downgrade behavior, and the intentionally organization-wide credential blast radius.
Checklist
Screenshots/Videos
Manual UI validation completed locally for both BYOK scopes, inherited badges, downgrade controls, and inherited-status failure handling.