improvement(byok): make available for all plans #2782
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile OverviewGreptile SummaryThis PR successfully democratizes the BYOK (Bring Your Own Key) feature by removing enterprise plan restrictions, making it available to all workspace admins on hosted deployments. The implementation is generally solid with clean removal of authorization gates and proper permission checks. What ChangedAccess Control Evolution:
Execution & Billing Impact:
Documentation Updates:
Issues Found
Security & Access Control AssessmentThe new permission model is appropriate and secure:
The removal of the admin BYOK bulk deletion endpoint is safe as it was only used for enterprise plan churn cleanup, which is no longer relevant when BYOK is available to all plans. Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User as Workspace Admin
participant UI as Settings UI
participant API as BYOK API
participant DB as Database
participant Exec as Workflow Execution
participant Provider as AI Provider
Note over User,Provider: BYOK Setup Flow (Now Available to All Plans)
User->>UI: Navigate to Settings → BYOK
UI->>API: GET /api/workspaces/{id}/byok-keys
API->>DB: Check workspace admin permission
DB-->>API: Permission verified
API->>DB: Query existing BYOK keys
DB-->>API: Return encrypted keys
API->>API: Decrypt and mask keys
API-->>UI: Return masked keys list
UI-->>User: Display provider cards with Add/Update/Delete
User->>UI: Click "Add Key" for provider
UI-->>User: Show API key input modal
User->>UI: Enter API key and save
UI->>API: POST /api/workspaces/{id}/byok-keys<br/>{providerId, apiKey}
API->>DB: Verify workspace admin permission
DB-->>API: Permission verified
API->>API: Encrypt API key
API->>DB: Upsert BYOK key for provider
DB-->>API: Success
API-->>UI: Return success with masked key
UI-->>User: Show success message
Note over User,Provider: Workflow Execution with BYOK
User->>Exec: Trigger workflow with AI block
Exec->>Exec: Identify provider and model
Exec->>DB: Query BYOK key for workspace
alt BYOK key exists
DB-->>Exec: Return encrypted key
Exec->>Exec: Decrypt key
Exec->>Provider: Call API with user's key
Provider-->>Exec: Return response
Exec->>Exec: Calculate cost (no multiplier)
Note over Exec: isBYOK=true, skip billing
else No BYOK key
Exec->>Exec: Use platform hosted key
Exec->>Provider: Call API with platform key
Provider-->>Exec: Return response
Exec->>Exec: Calculate cost with 1.4x multiplier
Exec->>DB: Log usage and bill user
end
Exec-->>User: Return workflow results
|
Additional Comments (3)
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! Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/hooks/queries/byok-keys.ts
Line: 20:20
Comment:
The `byokEnabled` field in the interface is no longer functionally used since the enterprise gate was removed. The API still returns `byokEnabled: true` (line 100 in route.ts), but the frontend no longer checks this value. Consider removing this field from the interface to keep the codebase clean, or if it's needed for backward compatibility, add a comment explaining why it's kept.
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.
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! Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/app/api/workspaces/[id]/byok-keys/route.ts
Line: 100:100
Comment:
The `byokEnabled: true` field is being returned but is no longer used by the frontend since the enterprise check was removed. The BYOK component no longer reads or checks this value. Consider removing this field from the response to keep the API clean, or update it to reflect the new behavior (e.g., always available for workspace admins).
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/docs/content/docs/en/enterprise/index.mdx
Line: 93:93
Comment:
This documentation states "BYOK is only available on hosted Sim Studio" but the PR makes BYOK available to all plans on hosted deployments, not just Enterprise. This note is accurate for self-hosted but may be confusing since the section is about Enterprise features. Consider clarifying that BYOK is now available to all plans on hosted deployments, or moving this note to a more appropriate location.
How can I resolve this? If you propose a fix, please make it concise. |
Greptile OverviewGreptile SummaryThis PR democratizes the Bring Your Own Key (BYOK) feature by making it available to all users instead of restricting it to Enterprise plan customers. The changes remove enterprise-only feature gating from the API routes, core execution logic, UI components, and documentation. Key ChangesAccess Control Shift:
Code Changes:
Documentation Updates:
Architectural ImpactThe permission model now relies solely on workspace-level permissions via During workflow execution, Minor Issue IdentifiedThe API response still includes a Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant UI as BYOK Settings UI
participant API as /api/workspaces/[id]/byok-keys
participant Auth as getUserEntityPermissions
participant Core as getApiKeyWithBYOK
participant DB as Database
User->>UI: Navigate to Settings → BYOK
Note over UI: No enterprise gate check
UI->>API: GET /api/workspaces/[id]/byok-keys
API->>Auth: Check user permissions
Auth->>DB: Query permissions table
DB-->>Auth: Return permission level
Auth-->>API: Returns 'admin', 'write', 'read', or null
alt User has permission
API->>DB: Fetch BYOK keys for workspace
DB-->>API: Return encrypted keys
API-->>UI: Return keys + byokEnabled: true
UI->>User: Display BYOK providers
else No permission
API-->>UI: 401 Unauthorized
end
User->>UI: Add/Update API Key
UI->>API: POST /api/workspaces/[id]/byok-keys
API->>Auth: Check workspace admin permission
alt User is workspace admin
API->>DB: Encrypt and store key
DB-->>API: Success
API-->>UI: Return success
UI->>User: Key saved
else Not workspace admin
API-->>UI: 403 Forbidden
end
Note over Core: During workflow execution
Core->>DB: Check for BYOK key (no enterprise check)
alt BYOK key exists
DB-->>Core: Return encrypted key
Core->>Core: Decrypt key
Core-->>Core: Use BYOK key
else No BYOK key
Core->>Core: Fall back to hosted key with multiplier
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {PROVIDERS.find((p) => p.id === deleteConfirmProvider)?.name} | ||
| </span>{' '} | ||
| API key? This workspace will revert to using platform keys with the 2x multiplier. | ||
| API key? This workspace will revert to using platform hosted keys. |
There was a problem hiding this comment.
The text "platform hosted keys" is unclear. Consider being more explicit about what happens when BYOK is removed, especially since the cost implications have changed with this PR.
Previously, it mentioned "2x multiplier" (which was actually 1.4x based on the docs), but now BYOK is available to everyone. The message should clarify what "platform hosted keys" means in terms of cost.
Suggested improvement:
| API key? This workspace will revert to using platform hosted keys. | |
| API key? This workspace will revert to using Sim Studio's hosted keys with the 1.4x pricing multiplier. |
This matches the documentation at apps/docs/content/docs/en/execution/costs.mdx line 51 and 84 which explicitly states the 1.4x multiplier.
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-modal/components/byok/byok.tsx
Line: 317:317
Comment:
The text "platform hosted keys" is unclear. Consider being more explicit about what happens when BYOK is removed, especially since the cost implications have changed with this PR.
Previously, it mentioned "2x multiplier" (which was actually 1.4x based on the docs), but now BYOK is available to everyone. The message should clarify what "platform hosted keys" means in terms of cost.
Suggested improvement:
```suggestion
API key? This workspace will revert to using Sim Studio's hosted keys with the 1.4x pricing multiplier.
```
This matches the documentation at `apps/docs/content/docs/en/execution/costs.mdx` line 51 and 84 which explicitly states the 1.4x multiplier.
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.| <Callout type="info"> | ||
| Users not assigned to any permission group have full access. Permission restrictions are enforced at both UI and execution time. | ||
| </Callout> | ||
|
|
There was a problem hiding this comment.
The BYOK section was removed from the English enterprise docs, but the same section still exists in other language versions (German, Spanish, French, Japanese, and Chinese). These need to be updated for consistency.
The BYOK section should be removed from:
apps/docs/content/docs/de/enterprise/index.mdxapps/docs/content/docs/es/enterprise/index.mdxapps/docs/content/docs/fr/enterprise/index.mdxapps/docs/content/docs/ja/enterprise/index.mdxapps/docs/content/docs/zh/enterprise/index.mdx
And potentially added to the corresponding costs.mdx files in those languages (though I note those files may not exist yet based on the execution/costs.mdx files I saw).
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/docs/content/docs/en/enterprise/index.mdx
Line: 31:31
Comment:
The BYOK section was removed from the English enterprise docs, but the same section still exists in other language versions (German, Spanish, French, Japanese, and Chinese). These need to be updated for consistency.
The BYOK section should be removed from:
- `apps/docs/content/docs/de/enterprise/index.mdx`
- `apps/docs/content/docs/es/enterprise/index.mdx`
- `apps/docs/content/docs/fr/enterprise/index.mdx`
- `apps/docs/content/docs/ja/enterprise/index.mdx`
- `apps/docs/content/docs/zh/enterprise/index.mdx`
And potentially added to the corresponding `costs.mdx` files in those languages (though I note those files may not exist yet based on the execution/costs.mdx files I saw).
How can I resolve this? If you propose a fix, please make it concise.
Summary
BYOK available for everyone.
Type of Change
Testing
Tested manually
Checklist