fix(approval-policy): allow user added through groups#6929
Conversation
|
💬 Discussion in Slack: #pr-review-infisical-6929-fix-approval-policy-allow-user-added-through-groups Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9c273c0f6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const groupApprovers = approvers | ||
| ?.filter((approver) => approver.type === ApproverType.Group) | ||
| .map((approver) => approver.id); | ||
| .map((approver) => approver.id) | ||
| .filter(Boolean) as string[]; |
There was a problem hiding this comment.
Reject blank group approver IDs instead of dropping them
When a secret-approval request contains a group approver with id: "" (the v1/v2 route schemas use plain z.string() for group IDs), this filter removes the only approver before membership verification and insertion. On create this can persist a policy with approvals: 1 and no approver rows; on update the same pattern can delete existing approvers and reinsert none, because insertMany([]) is a no-op. Please validate/reject blank group IDs rather than filtering them out.
Useful? React with 👍 / 👎.
Context
A user that is added into a project by a group cannot be added as a single approver on secrets and access policies. This was happening because the check only validated users, and not users by group memberships.
There was also another bug that was fixed on this PR. group membership were not being checked when creating an approval policy, which allowed the addition of groups as approvers that did not belong to a project.
Screenshots
Steps to verify the change
Bug 1: User added into a project through group
Bug 2: add a group that does not belong to a project as approver
Type
Checklist
type(scope): short description(scope is optional, e.g.,fix: prevent crash on syncorfix(api): handle null response).