feat(credential-groups): add resource access policies - #6892
feat(credential-groups): add resource access policies#6892TheodoreSpeaks wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview Adds a generic Internal executor delegation now carries current workflow authority (draft vs active Settings gain an Access tab to add/remove those grants with revision-based updates. Reviewed by Cursor Bugbot for commit ede2aba. Bugbot is set up for automated code reviews on this repo. Configure here. |
9995853 to
0c9fb23
Compare
Greptile SummaryAdds versioned allow-only access policies for Credential Groups and propagates deployed-workflow authority through nested execution.
Confidence Score: 5/5The PR appears safe to merge because no eligible blocking failure remains in the follow-up review. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/resource-policies/authorization.ts | Evaluates allow-only policy subjects against canonical workspace and delegated workflow execution context. |
| apps/sim/lib/resource-policies/repository.ts | Persists one policy per resource using atomic optimistic-revision inserts and updates. |
| apps/sim/lib/credential-groups/application/manage-access.ts | Adds authorized read and replacement-update operations for Credential Group access grants. |
| apps/sim/lib/auth/internal-delegation.ts | Binds delegated current-workflow authority to canonical workspace and deployment state. |
| apps/sim/executor/handlers/workflow/workflow-handler.ts | Propagates draft or deployed child-workflow authority through nested execution. |
| apps/sim/ee/credential-groups/components/credential-group-access.tsx | Adds the administrative UI for listing, adding, and removing explicit access grants. |
| packages/db/migrations/0298_sparkling_hemingway.sql | Introduces resource-policy persistence and uniqueness constraints aligned with the schema. |
Sequence Diagram
sequenceDiagram
participant Admin as Workspace Admin
participant API as Credential Group Access API
participant Policy as Resource Policy Store
participant Exec as Workflow Executor
participant Auth as Delegation Binding
participant Cred as Credential Resolver
Admin->>API: PUT explicit grants + expected revision
API->>Policy: Validate subjects and replace policy
Policy-->>API: New revision and grants
Exec->>Auth: Delegated principal + current workflow authority
Auth->>Auth: Bind workspace and deployment version
Exec->>Cred: Request managed credential
Cred->>Policy: Evaluate actor default and explicit grants
Policy-->>Cred: Allow or deny credential use
Reviews (3): Last reviewed commit: "test(resource-policies): cover actor gra..." | Re-trigger Greptile
| ? workflows.isPending | ||
| : subjectType === 'access_control_group' | ||
| ? permissionConfig.isPending || permissionGroups.isPending | ||
| : false |
There was a problem hiding this comment.
ACG dropdown stays loading without org
Medium Severity
targetOptionsPending treats permissionGroups.isPending as loading, but usePermissionGroups stays disabled when organizationId is missing. In TanStack Query v5 a disabled query with no data remains isPending forever, so Access Control Group mode keeps a disabled “Loading access control groups...” field after config has loaded for a workspace with no organization instead of showing an empty state.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit eebc37f. Configure here.
aaea6ae to
52e3168
Compare
95eafd9 to
9d5f513
Compare
c2d891e to
64f547f
Compare
| ALTER TABLE "resource_policy" ADD CONSTRAINT "resource_policy_created_by_user_id_fk" FOREIGN KEY ("created_by") REFERENCES "public"."user"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "resource_policy" ADD CONSTRAINT "resource_policy_updated_by_user_id_fk" FOREIGN KEY ("updated_by") REFERENCES "public"."user"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint | ||
| CREATE UNIQUE INDEX "resource_policy_resource_unique" ON "resource_policy" USING btree ("resource_type","resource_id");--> statement-breakpoint | ||
| CREATE INDEX "resource_policy_workspace_id_idx" ON "resource_policy" USING btree ("workspace_id"); |
There was a problem hiding this comment.
Orphan policies after group delete
Low Severity
Deleting a Credential Group removes the group row but leaves any resource_policy for that group. There is no foreign key from resource_policy.resource_id to credential_group, and deleteCredentialGroupSettings never deletes the policy, so access documents become permanent orphans after delete.
Reviewed by Cursor Bugbot for commit 64f547f. Configure here.
| throw new OrchestrationError('conflict', error.message) | ||
| } | ||
| throw error | ||
| } |
There was a problem hiding this comment.
Duplicate grants become server errors
Low Severity
updateCredentialGroupAccessBodySchema accepts duplicate subjects or grant ids, and only parseResourcePolicyDocument rejects them later. That failure is a raw Zod/Error rather than an OrchestrationError, so the access route maps it to a generic 500 instead of a validation response.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 64f547f. Configure here.
64f547f to
eccb03a
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
3 issues from previous reviews remain unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit eccb03a. Configure here.
eccb03a to
ede2aba
Compare
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ede2aba. Configure here.
| queryClient.invalidateQueries({ | ||
| queryKey: credentialGroupKeys.access(variables.workspaceId, variables.groupId), | ||
| }), | ||
| }) |
There was a problem hiding this comment.
Stale revision on rapid access edits
Medium Severity
useUpdateCredentialGroupAccess only invalidates the access query after settle and never writes the mutation response into the cache. saveGrants keeps sending expectedRevision from the last fetched policy, so a second remove or add that starts before refetch finishes uses a stale revision and hits a conflict even though the first save succeeded.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ede2aba. Configure here.
| ) | ||
| .max(100), | ||
| }) | ||
| .strict() |
There was a problem hiding this comment.
Duplicate grants return server error
Low Severity
updateCredentialGroupAccessBodySchema accepts duplicate subjects, while parseResourcePolicyDocument later rejects them. That failure is a plain Zod error rather than an orchestration validation error, so the access route surfaces it as an unhandled 500 instead of a client-visible 400.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ede2aba. Configure here.


Summary
Stacked on #6891.
Type of Change
Testing
bun run lintbun run check:auditsbun run check:migrations origin/stagingChecklist
Screenshots/Videos
Not included.