fix(gcp): restrict setup wizard compute permissions by cristim · Pull Request #2122 · LeanerCloud/cloud-commitments-cli

cristim · 2026-09-29T23:09:07Z

configure-gcp currently grants full Compute Admin access before minting a service-account key. Replace that grant with Compute Viewer and a project custom role containing only compute.commitments.create.

The wizard validates existing custom roles before binding them and refuses to widen conditional-only membership. It preserves existing grants, including broad grants from older installations, which need separate operator review. Setup documentation distinguishes operator permissions from runtime permissions and notes separate Recommender access requirements.

Validation uses the real wizard coordinator and Google SDK against local HTTP fixtures. The regression fails on the original Compute Admin policy request. Cases cover custom-role reuse and rejection, preserved conditional and broad grants, create conflicts, and partial policy-write failure before key minting. No live IAM mutation or purchase was performed; fixture evidence does not establish live Google IAM propagation.

Verified commit: 1272e721b8d5185634840c9aa9fc7698dd7cb124.

Full short/race suite: passed, 468.281 seconds.
Final focused regression: passed, 27.920 seconds.
Build, Go vet, pinned golangci-lint 2.10.1 and normal commit hooks: passed.
Independent adversarial review by gpt-6-astra under the user-approved session substitution: approved this exact SHA, no actionable findings. Independent focused race run passed in 26.000 seconds; unchanged fixture against original production failed on the intended Compute Admin assertion in 3.180 seconds. CodeRabbit was waived for this independent review path.

Closes #1946

Summary by CodeRabbit

New Features
- GCP setup now grants the permissions needed for commitment purchasing without granting the broader Compute Admin role.
- Setup creates the required project role when it is missing and verifies that an existing role has only the required permission.
Bug Fixes
- Setup now stops when the existing role is incompatible or access is granted only through conditional IAM bindings, rather than broadening access.
Documentation
- Updated the cloud setup guide with the required permissions and behavior when rerunning setup.

Replace the setup wizard's Compute Admin grant with Compute Viewer and an exact project custom role for compute.commitments.create. Preserve conditional access and fail rather than silently widening existing grants. Cover the real SDK policy requests and wizard failure exits with local HTTP fixtures. Existing broad grants require separate operator review.

coderabbitai · 2026-09-29T23:09:22Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: 48e46237-239a-4ae5-b22e-7274df78e7ea

📥 Commits

Reviewing files that changed from the base of the PR and between 6c44a78 and 1272e72.

📒 Files selected for processing (4)

cmd/configure_gcp.go
cmd/configure_gcp_iam_test.go
cmd/configure_gcp_test.go
docs/cli/cloud-setup.md

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

Walkthrough

GCP setup replaces the project-level roles/compute.admin grant with roles/compute.viewer and a validated cudlyCommitmentPurchaser custom role. IAM binding updates now reject conditional-only access instead of adding unconditional access.

Changes

GCP IAM setup

Layer / File(s)	Summary
Conditional IAM membership handling `cmd/configure_gcp.go`, `cmd/configure_gcp_test.go`	The binding helper returns an error when a member has only conditional access. Existing unconditional membership remains unchanged. Tests assert the helper’s error and change result.
Purchaser role provisioning and setup `cmd/configure_gcp.go`, `cmd/configure_gcp_iam_test.go`, `docs/cli/cloud-setup.md`	The setup step creates the custom role only when lookup returns 404, validates its name, status, and sole permission, then grants it with `roles/compute.viewer`. Tests cover role, policy, and coordinator scenarios. The guide documents required permissions and setup behavior.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: High

Sequence Diagram(s)

sequenceDiagram
  actor Operator
  participant ConfigureGCP as gcpStepGrantRole
  participant PurchaserRole as ensureGCPPurchaserRole
  participant IAM as Google Cloud IAM API
  Operator->>ConfigureGCP: Select Run
  ConfigureGCP->>PurchaserRole: Validate or create project role
  PurchaserRole->>IAM: Get cudlyCommitmentPurchaser
  alt Role lookup returns 404
    PurchaserRole->>IAM: Create role with compute.commitments.create
  end
  PurchaserRole-->>ConfigureGCP: Return validated role
  ConfigureGCP->>IAM: Read and update compute.viewer policy binding
  ConfigureGCP->>IAM: Read and update purchaser role policy binding

Merge Risk: ⚪ Minimal · up to 1272e

The narrowed permissions are ready to merge after normal checks. Provisioning failures stop before key creation, and rerunning setup repairs partial grants.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: restricting the GCP setup wizard's compute permissions.
Linked Issues check	✅ Passed	Issue [`#1946`] requires narrowing the project IAM grant used by the GCP setup wizard. The PR replaces roles/compute.admin with roles/compute.viewer and the cudlyCommitmentPurchaser custom role. The cus…
Out of Scope Changes check	✅ Passed	The changed source implements the narrowed IAM grant and safe binding behavior required by [`#1946`]. The added tests exercise the changed wizard coordinator and IAM paths. The documentation describes t…

Full details: Docstring Coverage

Explanation

Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

_{Comment @coderabbitai help to get the list of available commands.}

cristim added triaged Item has been triaged priority/p1 Next up; this sprint severity/high Significant harm urgency/this-sprint Within the current sprint impact/many Affects most users effort/s Hours type/security Security finding labels Sep 29, 2026

cristim merged commit 76d1d4e into main Sep 29, 2026
12 checks passed

This was referenced Sep 29, 2026

ops(gcp): review legacy Compute Admin grants from setup wizard #2123

Open

fix(gcp): complete the Recommender permission setup contract #2124

Open

sec(cli): the GCP setup wizard grants roles/compute.admin at project scope #1946

Closed

Navigation Menu

fix(gcp): restrict setup wizard compute permissions - #2122

fix(gcp): restrict setup wizard compute permissions#2122
cristim merged 1 commit into
mainfrom
fix/gcp-least-privilege-1946

cristim commented Sep 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 29, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

cristim commented Sep 29, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cristim commented Sep 29, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 29, 2026 •

edited

Loading