feat(access-control): page-based permission groups, tool-level deny-list, settings row-action consistency#5216
Conversation
…ist, settings row-action consistency - Replace the cramped configure modal with a full-surface tabbed Access Control page (General/Model Providers/Blocks/Platform) with a sticky save bar - Add deniedTools denylist to permission groups: deny individual tools within an allowed integration; enforced at the universal executeTool chokepoint via ToolNotAllowedError, and hidden from the operation dropdown for governed users - Add per-section Select/Deselect All on the Blocks tab and expandable per-tool deny rows (mirrors Providers->Models) - Standardize every settings list row on the canonical "..." DropdownMenu (custom-tools, mcp, workflow-mcp-servers, api-keys, secrets, credential-sets) and align badges (ChipTag), avatars (MemberAvatar), inputs (ChipInput), and the mothership env picker (ChipSelect)
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Permission groups gain a Settings UI standardizes list row actions on a Reviewed by Cursor Bugbot for commit 887e009. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR introduces three coordinated changes: (1) a full-surface tabbed settings page for access-control group configuration replaces the old cramped modal, (2) a
Confidence Score: 5/5Safe to merge. The new per-tool denylist is enforced server-side at the universal Core enforcement logic is thoroughly tested with 5 targeted cases including the exempt-block edge case. Type definitions, schema validation, config parsing, and the runtime check are all updated in sync. No DB migration is required. The UI changes are cosmetic (settings row consistency) and the new GroupDetail component correctly mirrors established patterns for the per-tool deny rows. No files require special attention. The most complex new file, Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[executeTool called] --> B{scope.userId && scope.workspaceId?}
B -- No --> E[Execute tool directly]
B -- Yes --> C[assertPermissionsAllowed\ntoolId: normalizedToolId\ntoolKind: mcp/custom/skill/undefined]
C --> D{blockTypeExempt &&\n!model && !toolKind\n&& !toolId?}
D -- Yes --> E
D -- No --> F[Load getUserPermissionConfig]
F --> G{blockType passed\n& not exempt\n& config?}
G -- Yes --> H{allowedIntegrations\nexcludes blockType?}
H -- Yes --> I[throw IntegrationNotAllowedError]
H -- No --> J{toolId in\ndeniedTools?}
G -- No --> J
J -- Yes --> K[throw ToolNotAllowedError]
J -- No --> L{toolKind checks\nmcp/custom/skill}
L --> M[Pass / Execute tool]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[executeTool called] --> B{scope.userId && scope.workspaceId?}
B -- No --> E[Execute tool directly]
B -- Yes --> C[assertPermissionsAllowed\ntoolId: normalizedToolId\ntoolKind: mcp/custom/skill/undefined]
C --> D{blockTypeExempt &&\n!model && !toolKind\n&& !toolId?}
D -- Yes --> E
D -- No --> F[Load getUserPermissionConfig]
F --> G{blockType passed\n& not exempt\n& config?}
G -- Yes --> H{allowedIntegrations\nexcludes blockType?}
H -- Yes --> I[throw IntegrationNotAllowedError]
H -- No --> J{toolId in\ndeniedTools?}
G -- No --> J
J -- Yes --> K[throw ToolNotAllowedError]
J -- No --> L{toolKind checks\nmcp/custom/skill}
L --> M[Pass / Execute tool]
Reviews (6): Last reviewed commit: "fix(access-control): don't seed a denied..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ges save fails The unsaved-changes dialog's Save action navigated back unconditionally after handleSaveConfig, but that helper swallows mutation errors — so a failed save still exited the view and silently dropped the edits. handleSaveConfig now returns success, and the dialog only closes + navigates back when the save actually succeeded.
deniedTools only matters while a block is allowed, but toggleIntegration/setBlocksAllowed left a disabled block's denied tools in the config. Disabling then re-enabling an integration would silently re-apply the old per-tool denials. Both handlers now prune deniedTools to the set of allowed blocks, keeping the invariant that deniedTools only holds tools of currently-allowed integrations.
|
@greptile |
|
@cursor review |
…en pruning A tool id can appear in more than one block's tools.access. The single tool->block map meant pruneDeniedTools (and the per-block denied count) attributed a shared tool to only one block, so disabling that block could drop a denial while the tool was still exposed by another allowed block. Tools now map to all exposing block types; a denial is pruned only when no allowed block exposes the tool, and the per-block count is derived from each block's own tool list.
|
@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 ae1abfa. Configure here.
… filter The Platform tab's bulk Select/Deselect All toggled every feature regardless of the active search, unlike the Blocks tab which scopes its per-section toggle to the filtered view. Both the all-visible check and the bulk update now operate on filteredPlatformFeatures for consistent behavior while searching.
|
Addressed the Platform-tab note from the Greptile summary in a0fae05: the Platform Select/Deselect All now scopes to the active search filter ( |
|
@greptile |
|
@cursor review |
… search filter Like the Platform fix, the Providers tab's bulk action toggled every provider via allProviderIds regardless of the active search. Added setProvidersAllowed (mirroring setBlocksAllowed) so the bulk toggle and its label operate on filteredProviders, keeping all three tabs (Blocks/Platform/Providers) consistent while searching.
|
@greptile |
|
@cursor review |
The operation dropdown hides denied tools from the picker, but defaultOptionValue returned the block's defaultValue without checking deniedOperationIds, so a new block could start on an operation the user isn't allowed to run. It now falls back to the first allowed option when the configured default is denied. Existing stored operation values are intentionally left untouched (auto-rewriting a user's saved block would be destructive; the server remains the authoritative gate).
|
@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 887e009. Configure here.
Convert declaration-level rationale comments to TSDoc (/** */) and trim redundant/verbose inline comments added during review, per the project's TSDoc convention.
1 participant
Summary
deniedToolsdenylist to permission groups: admins can deny individual tools within an allowed integration (e.g. allow Slack, deny "Create Canvas"). Enforced server-side at the universalexecuteToolchokepoint viaToolNotAllowedError, and hidden from the operation dropdown for governed users. JSONB config field — no DB migration, existing groups unaffected...DropdownMenu(custom-tools, mcp, workflow-mcp-servers, api-keys, secrets, credential-sets) and align badges (ChipTag), member avatars (MemberAvatar), inputs (ChipInput), and the Mothership env picker (ChipSelect)Type of Change
Testing
tscclean, biome clean across all touched filesdeniedToolsenforcement casesbun run check:api-validationpassesChecklist