feat(access-control): add per-model denylist to permission groups#4794
Conversation
PR SummaryMedium Risk Overview Enforcement introduces UI updates the Access Control Model Providers tab with expandable rows, per-model checkboxes (static or lazy-fetched catalogs), search, Allow/Block all, and blocked-count badges. Workflow model pickers filter via Schema/API defaults keep Reviewed by Cursor Bugbot for commit 60802ea. Configure here. |
Greptile SummaryThis PR adds a per-workspace model denylist to permission groups, layered on top of the existing provider allowlist. A model is usable only when its provider is allowed and the model is not in
Confidence Score: 5/5Safe to merge. Server-side enforcement is correct and complete; schema change is fully backward-compatible. The denylist logic is consistently applied in all three enforcement points (validateModelProvider, assertPermissionsAllowed, and the client-side hook), tests cover the key edge cases (case-insensitivity, denylist with no provider allowlist, negative case), and the additive schema change defaults to [] so existing groups are unaffected. The two UI issues flagged are cosmetic and do not affect access control correctness. apps/sim/ee/access-control/components/access-control.tsx — expand-state reset and Allow All no-op guard. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Client (combobox)
participant Hook as usePermissionConfig
participant Route as API Route (providers / guardrails)
participant Check as permission-check.ts
participant DB as DB (permissionGroup)
Client->>Hook: isModelAllowed(modelId)
Hook-->>Client: false if modelId in deniedModels
Client->>Hook: isProviderAllowed(providerId)
Hook-->>Client: false if provider not in allowedModelProviders
Client->>Route: "POST /api/providers {model}"
Route->>Check: "assertPermissionsAllowed({userId, workspaceId, model})"
Check->>DB: getUserPermissionConfig(userId, workspaceId)
DB-->>Check: PermissionGroupConfig
alt "allowedModelProviders != null"
Check->>Check: getProviderFromModel(model)
Check-->>Route: throw ProviderNotAllowedError (403)
end
alt model in deniedModels
Check-->>Route: throw ModelNotAllowedError (403)
end
Route-->>Client: 403 or continue execution
Reviews (3): Last reviewed commit: "chore(access-control): reuse canonical D..." | Re-trigger Greptile |
…ocked badge on disabled rows, trim comments
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@greptile-apps re: the |
|
That makes sense — showing the stored value rather than blanking the picker is the right call, and the consistency with |
|
@greptile |
|
@cursor review |
|
@greptile |
|
@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 60802ea. Configure here.
Summary
deniedModels.validateModelProviderandassertPermissionsAllowed(executor handlers,/api/providers, guardrails, tools) returning 403, and client-side in the model dropdown viausePermissionConfig().isModelAllowedso blocked models disappear from the picker.deniedModels: string[], defaults to[]), so existing groups are unaffected.Type of Change
Testing
permission-checkunit tests extended (denylist hit, case-insensitivity, denylist enforced with no provider allowlist, negative case) — 28 pass;tools/index79 pass.tsc --noEmitclean repo-wide; Biome clean;check:api-validationpassed.Checklist