iframe-proxy

cristim · 2026-10-16T02:21:09Z

Closes #2007

Summary

At Step 3 of configure-gcp, gcpStepCreateServiceAccount returned the default email cudly-service-account@<project>.iam.gserviceaccount.com on Skip and on unknown input. The grant step (Step 4) and the key step (Step 5) then acted on a principal that was never created or verified, and the wizard failed later with an IAM error that did not mention the skipped step.

On Skip or unknown input, the wizard now prompts for the existing service account email. It reads the answer with the existing readRequiredInputLine helper.
Validation fails loud on bad input. It requires exactly one @, a non-empty local part, and a non-empty label before .gserviceaccount.com, compared case-insensitively. It rejects whitespace and control characters. The email is returned lowercased, because service account IDs are lowercase.
The run branch behaves as before. It now returns the email it created directly, since the fabricated default is gone.
docs/cli/cloud-setup.md now notes that Skip at this step prompts for an existing email.
The tests that drive the full wizard (configure_gcp_iam_test.go, configure_gcp_signal_test.go) feed the extra email line after the Step 3 s.

Evidence (local, macOS, `GOTOOLCHAIN=go1.26.9`; stdin-driven unit tests, no GCP calls)

Fail before the fix: TestGCPStepCreateServiceAccount_SkipUsesOperatorEmail fails on origin/main code. It expected existing-sa@my-proj.iam.gserviceaccount.com and got cudly-service-account@my-proj.iam.gserviceaccount.com. The unknown-option test and the invalid-email test also fail without the fix.
Mutation probes: each of these makes a test fail.
- The skip branch returns the default email.
- The unknown-input branch returns the default email.
- The suffix check is weakened.
- The label == "" check is removed.
- The unicode.IsControl check is removed.
Checks:
- make build: ok.
- go vet ./...: clean.
- go test -race ./cmd/...: pass.
- golangci-lint v2.10.1 (the CI pin): 0 issues.
- Pre-commit hooks: pass.
Review: an independent non-author reviewer (sonnet, high effort) did two passes. Pass 1 had five low findings: reuse an existing helper, tighten the validation, prompt text, missing test cases, and docs. All are fixed. Pass 2 was clean with two optional lows, which are also applied. The final check found no actionable findings at 9c7a96a.
Not covered: the real wizard was not run against a live GCP project. That run would need an operator-authorized project.

🤖 Generated with Claude Code

… creation gcpStepCreateServiceAccount returned the fabricated default email cudly-service-account@<project>.iam.gserviceaccount.com on skip and on unknown input, so the grant and key steps acted on a principal that was never created or verified and failed later with an opaque IAM error. On skip or unknown input it now asks for the existing service account email, validates it (non-empty, no whitespace, <name>@...gserviceaccount.com) and fails loud otherwise. Wizard-driving tests gain the extra email line. Closes #2007 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

coderabbitai · 2026-10-16T02:21:22Z

The 46-dash run matched the AWS-secret pattern in the git-secrets allowlist self-test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cristim · 2026-10-16T08:04:19Z

Merge-gate verdict: CLEAN at `b5af035`

Independent adversarial review (non-author agent, Sonnet high, cold brief, full diff origin/main...b5af0358): no blocking findings.

Traced end to end (code reading): the Skip/unknown-option branch now reads an existing SA email, trims and lowercases it, and validates it (one @, non-empty local part, .gserviceaccount.com suffix with a non-empty label, no whitespace or control characters). The value flows to gcpStepGrantRole (serviceAccount:<email>) and to createGCPServiceAccountKey (projects/-/serviceAccounts/<email>) as SDK values, with no shell execution. The create-SA branch is unchanged, and --skip-setup bypasses this function. At EOF the reader returns an error, so there is no loop and no panic.

Non-blocking notes:

Invalid or empty input after Skip aborts the wizard instead of re-prompting. This matches readRequiredInputLine for the project ID.
Any *.gserviceaccount.com is accepted, including developer./appspot. default SAs, which is intended. A malformed label such as x..gserviceaccount.com passes local validation; the IAM API rejects it later.

Local verification on macOS at b5af035 (fixture/mock-based, stdin readers, no cloud credentials):

go build -o cudly ./cmd: OK. go vet ./... (go1.26.9 per go.work): clean.
AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./... -count=1: ok cmd 86.4s (the module has a single package).
Fail-before: with cmd/configure_gcp.go from origin/main and this PR's tests, TestGCPStepCreateServiceAccount* fails (expected existing-sa@my-proj..., got cudly-service-account@my-proj...). It passes at head.
Mutation probes, each reverted at once: removing strings.ToLower fails ..._SkipAcceptsUppercaseEmail, and disabling the suffix check fails TestGCPStepCreateServiceAccount*.
Real-account coverage gap: the GCP setup wizard was not driven against a real project.

CI is green on b5af035, mergeability is CLEAN, and labels are mirrored from #2007.

CR waived: quota, adversarial review + local verification + green CI. A retrospective CR review is tracked as a follow-up.

🤖 Generated with Claude Code

* docs(changelog): record the #2206 and #2211 fixes Both PRs merged with user-visible behaviour changes but without a CHANGELOG entry: configure-gcp now asks for an existing service account when the create step is skipped, and count changes re-derive the projection columns and the Redshift node count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(changelog): note ProjectedCoverage needs known existing coverage Addresses the CodeRabbit finding on #2220 about the #2206 entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

cristim added priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-quarter Within the quarter impact/few Limited audience effort/xs Trivial / one-liner type/bug Defect triaged Item has been triaged labels Oct 16, 2026

test(cli): shorten GCP test section header below the secret-scan width

b5af035

The 46-dash run matched the AWS-secret pattern in the git-secrets allowlist self-test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cristim merged commit 028551d into main Oct 16, 2026
12 checks passed

cristim deleted the fix/gcp-skip-sa-email branch October 16, 2026 08:08

cristim mentioned this pull request Oct 16, 2026

docs(changelog): record the #2206 and #2211 fixes #2220

Merged

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

fix(cli): prompt for an existing service account when GCP setup skips creation - #2211

fix(cli): prompt for an existing service account when GCP setup skips creation#2211
cristim merged 2 commits into
mainfrom
fix/gcp-skip-sa-email

cristim commented Oct 16, 2026

Uh oh!

coderabbitai Bot commented Oct 16, 2026 •

edited

Loading

Review limit reached

Uh oh!

cristim commented Oct 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

cristim commented Oct 16, 2026

Summary

Evidence (local, macOS, GOTOOLCHAIN=go1.26.9; stdin-driven unit tests, no GCP calls)

Uh oh!

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

Uh oh!

Review limit reached

Uh oh!

cristim commented Oct 16, 2026

Merge-gate verdict: CLEAN at b5af035

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Evidence (local, macOS, `GOTOOLCHAIN=go1.26.9`; stdin-driven unit tests, no GCP calls)

coderabbitai Bot commented Oct 16, 2026 •

edited

Loading

Merge-gate verdict: CLEAN at `b5af035`