-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Background
In PR #1 (#1), a project-level roles/aiplatform.user grant was added for the ambient-code/pull-reviews repository via Workload Identity Federation. A code review comment flagged that scoping the IAM binding at the project level is broader than necessary.
Issue
The google_project_iam_member resource for pull_reviews_vertex_ai_access grants roles/aiplatform.user at the project level. Any workflow in ambient-code/pull-reviews (any branch, any actor) can obtain this role, which may be broader than required.
Suggested Improvement
Consider one or both of the following:
- Scope the
roles/aiplatform.usergrant to the minimum necessary resource (e.g., a specific Vertex AI resource) rather than at the project level. - Add a stricter IAM condition on the binding using mapped WIF attributes (e.g.,
attribute.ref,attribute.job_workflow_ref) to restrict which workflows/refs can assume the role.
Security vs. Usability Trade-off
Note that tightening the attribute_condition (e.g., adding assertion.ref == 'refs/heads/main') would improve security but comes at a cost: it would block WIF authentication for workflows running on work-in-progress feature branches, making it harder to test GitHub Actions prior to merging to main. Any future hardening effort should weigh this trade-off and consider whether a separate, more permissive pool/provider can be used for development/staging, or whether branch-scoped testing is an acceptable restriction.
References
- PR
#1: feat: add pull-reviews to wif #1 - Review comment: feat: add pull-reviews to wif #1 (comment)
- Requested by:
@ktdreyer