Skip to content

feat(access-control): add per-model denylist to permission groups#4794

Merged
waleedlatif1 merged 3 commits into
stagingfrom
waleedlatif1/model-allowlist-access-groups
May 29, 2026
Merged

feat(access-control): add per-model denylist to permission groups#4794
waleedlatif1 merged 3 commits into
stagingfrom
waleedlatif1/model-allowlist-access-groups

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Add a per-workspace model denylist to access 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 deniedModels.
  • Chose a denylist (not an allowlist) so dynamically-discovered models from vLLM, Ollama, and LiteLLM stay usable by default as their catalogs change — admins block specific models per workspace.
  • Enforced everywhere the provider allowlist is: server-side in validateModelProvider and assertPermissionsAllowed (executor handlers, /api/providers, guardrails, tools) returning 403, and client-side in the model dropdown via usePermissionConfig().isModelAllowed so blocked models disappear from the picker.
  • Access-control UI: the Model Providers tab now has expandable provider rows with per-model checkboxes, a per-provider search + Allow All / Block All, and an "N blocked" badge. Models load lazily on expand (live fetch for vLLM/Ollama/LiteLLM/OpenRouter/Fireworks, static for the rest).
  • Additive schema/contract change (deniedModels: string[], defaults to []), so existing groups are unaffected.

Type of Change

  • New feature

Testing

  • permission-check unit tests extended (denylist hit, case-insensitivity, denylist enforced with no provider allowlist, negative case) — 28 pass; tools/index 79 pass.
  • tsc --noEmit clean repo-wide; Biome clean; check:api-validation passed.
  • Not yet click-tested in a running app.

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)

@cursor
Copy link
Copy Markdown

cursor Bot commented May 29, 2026

PR Summary

Medium Risk
Changes workspace model access control on execution paths (/api/providers, guardrails, executor gates); behavior is additive with defaults and unit tests, but misconfiguration could block expected models.

Overview
Adds a per-model denylist (deniedModels) on permission groups, applied after the existing provider allowlist so a model must pass both checks.

Enforcement introduces ModelNotAllowedError and case-insensitive deny matching in validateModelProvider and assertPermissionsAllowed (including when no provider allowlist is set). /api/providers and guardrails hallucination validation now treat blocked models like blocked providers (403 or validation failure with the error message).

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 usePermissionConfig().isModelAllowed.

Schema/API defaults keep deniedModels as [], so existing groups are unchanged.

Reviewed by Cursor Bugbot for commit 60802ea. Configure here.

Comment thread apps/sim/lib/api/contracts/permission-groups.ts Outdated
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Greptile Summary

This 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 deniedModels.

  • Core enforcement: validateModelProvider and assertPermissionsAllowed in permission-check.ts now check deniedModels after the provider allowlist, throwing a new ModelNotAllowedError (HTTP 403) when a model is on the denylist. The check applies even when no provider allowlist is configured.
  • Client-side filtering: usePermissionConfig exposes a new isModelAllowed helper; the model combobox applies it before isProviderAllowed so blocked models disappear from the picker.
  • Admin UI: The Model Providers tab gains expandable provider rows with per-model checkboxes, per-provider search, and Allow All / Block All; dynamic providers (Ollama, vLLM, etc.) fetch their catalog lazily on expand.
  • Schema: deniedModels: string[] defaults to [] on both the Zod schema and the parsed config, so existing groups are unaffected.

Confidence Score: 5/5

Safe 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

Filename Overview
apps/sim/ee/access-control/utils/permission-check.ts New ModelNotAllowedError class and isModelDenied helper added; validateModelProvider and assertPermissionsAllowed both check the denylist after the provider allowlist with correct short-circuit ordering.
apps/sim/ee/access-control/utils/permission-check.test.ts Four new test cases cover denylist hit, case-insensitivity, denylist with no provider allowlist, and negative (allowed) case; assertPermissionsAllowed also gains a denylist test.
apps/sim/ee/access-control/components/access-control.tsx Adds ProviderRow, ModelCheckboxGrid, DynamicProviderModels, StaticProviderModels components; expand state in ProviderRow does not reset when isProviderAllowed toggles to false, causing the panel to re-open unexpectedly on re-enable.
apps/sim/hooks/use-permission-config.ts New isModelAllowed memoized helper checks config.deniedModels with case-insensitive comparison; correctly exposed in both return paths.
apps/sim/lib/permission-groups/types.ts Additive schema/contract change: deniedModels: string[] added to the Zod schema (optional), interface (required), default config, and parsePermissionGroupConfig; backward-compatible.
apps/sim/lib/api/contracts/permission-groups.ts Adds deniedModels: z.array(z.string()).default([]) to the full config schema; default keeps existing groups unaffected.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/combobox.tsx Both static and fetched option filters now call isModelAllowed before isProviderAllowed; isModelAllowed added to the memoization dependency arrays.
apps/sim/app/api/providers/route.ts Catch block updated to also handle ModelNotAllowedError alongside ProviderNotAllowedError; returns 403 correctly.
apps/sim/app/api/guardrails/validate/route.ts Catch block updated symmetrically with the providers route to handle ModelNotAllowedError.

Sequence Diagram

sequenceDiagram
    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
Loading

Reviews (3): Last reviewed commit: "chore(access-control): reuse canonical D..." | Re-trigger Greptile

Comment thread apps/sim/ee/access-control/components/access-control.tsx Outdated
@vercel
Copy link
Copy Markdown

vercel Bot commented May 29, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 29, 2026 7:57pm

Request Review

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile-apps re: the hydratedOption note on combobox.tsx (L201-209) — leaving this as-is intentionally. The hydrated option is the currently-selected value; filtering it out would blank the picker for a model that's still stored on the block, which is more confusing than showing it. As you note, the same behavior already applies to the isProviderAllowed filter, so this keeps the two consistent, and server-side enforcement (validateModelProvider / assertPermissionsAllowed) still returns 403 if a denied model is actually used. No security bypass — just a deliberate UX trade-off.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

That makes sense — showing the stored value rather than blanking the picker is the right call, and the consistency with isProviderAllowed keeps the behavior predictable. Since the 403 from validateModelProvider/assertPermissionsAllowed is the actual enforcement gate, the UI here is purely informational anyway. Thanks for the clarification.

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/ee/access-control/components/access-control.tsx Outdated
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

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 60802ea. Configure here.

@waleedlatif1 waleedlatif1 merged commit c95aa87 into staging May 29, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/model-allowlist-access-groups branch May 29, 2026 20:08
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