Skip to content

feat: switch GCP auth to direct WIF - #856

Merged
waynesun09 merged 3 commits into
mainfrom
direct-wif
May 13, 2026
Merged

feat: switch GCP auth to direct WIF#856
waynesun09 merged 3 commits into
mainfrom
direct-wif

Conversation

@waynesun09

@waynesun09 waynesun09 commented May 12, 2026

Copy link
Copy Markdown
Member

Summary

  • Switch from SA-based WIF to direct WIF — the WIF pool principalSet now gets roles/aiplatform.user directly on the GCP project, eliminating the intermediate service account for Vertex AI access. Follows the google-github-actions/auth recommended pattern.
  • Tighten WIF scope to .fullsend repo only — attribute condition uses assertion.repository == 'org/.fullsend' instead of assertion.repository_owner == 'org', and IAM binding uses attribute.repository/{org}/.fullsend principalSet. Only the .fullsend config repo can authenticate to Vertex AI, not arbitrary repos in the org. parseConditionOrgs is backward-compatible with legacy repository_owner conditions.
  • Remove SA key auth path entirely — delete gcp.go (LiveGCPClient), AuthModeSAKey, SecretCredentials, CredentialJSON, GCPClient interface, --gcp-credentials-file / --gcp-service-account CLI flags, gcp_sa_key_json action input, and extractProjectID. WIF is now the only GCP auth mode.
  • Add SetProjectIAMBinding to GCFClient interface for project-level IAM bindings via Cloud Resource Manager v1 API, with retry-on-409 for concurrent updates.
  • Refactor IAM helpers — unified duplicated trySetIAMBinding / trySetIAMBindingWithMethod into a single method parameterized by get method and body.
  • Remove --gcp-wif-sa-email CLI flag, WIFServiceAccount config field, FULLSEND_GCP_WIF_SA_EMAIL secret, and service_account parameter from the setup-gcp composite action.
  • Rename SA from fullsend-dispatch to fullsend-mint (now purely the mint Cloud Function runtime identity).
  • Make service_account_impersonation_url conditional in sandbox credentials script — direct WIF credentials don't have this field, so omit it rather than writing a null value.
  • Pass GOOGLE_CLOUD_PROJECT into sandbox via gcp-vertex.env and project_id to the auth action across all stage workflows.
  • Update docs — installation guide (remove legacy SA key setup, update WIF steps for direct WIF with reference URLs), ADR-0014 (remove FULLSEND_GCP_SA_KEY_JSON from secret table, simplify WIF-only language).

IAM requirements for direct WIF

Direct WIF principals need one role on the GCP project:

  • roles/aiplatform.user — Vertex AI model access

Changed files (33)

Area Files
GCP client internal/dispatch/gcf/gcp.go, gcp_test.go
Provisioner internal/dispatch/gcf/provisioner.go, provisioner_test.go
Vertex provider internal/inference/vertex/vertex.go, vertex_test.go
Vertex removed internal/inference/vertex/gcp.go (deleted)
CLI internal/cli/admin.go, admin_test.go
E2E tests e2e/admin/admin_test.go
Scaffold action .github/actions/setup-gcp/action.yml
Scaffold workflows 6 workflow files (triage, code, review, fix, retro, prioritize)
Scaffold scripts scripts/prepare-sandbox-credentials.sh
Scaffold tests internal/scaffold/scaffold_test.go
Layers tests internal/layers/inference_test.go
Docs docs/guides/admin/installation.md, ADR-0014 SPEC.md

Test plan

  • go test ./... — all 18 packages pass
  • go vet ./... — clean
  • go build -tags e2e ./e2e/admin/ — compiles
  • Unit tests for SetProjectIAMBinding: success, already-bound (idempotent), 409 retry, error paths
  • Provisioner tests: error injection for project IAM failure, multi-org argument verification, principalSet format validation
  • WIF condition tests: single-org and multi-org conditions use assertion.repository with .fullsend suffix
  • Backward-compatible parsing of legacy repository_owner conditions (merge test)
  • Scaffold tests: Contains for WIF inputs/params, NotContains for removed SA key fields
  • Inference layer tests: fake provider uses FULLSEND_GCP_WIF_PROVIDER secret names
  • E2E test updated: E2E_HALFSEND_WIF_PROVIDER replaces E2E_HALFSEND_VERTEX_KEY, extractProjectID removed
  • Reviewed by 10-agent review squad — all findings addressed
  • E2E: deployed to nonflux org, confirmed direct WIF auth succeeds

@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@github-actions

github-actions Bot commented May 12, 2026

Copy link
Copy Markdown

Site preview

Preview: https://a08a3763-site.fullsend-ai.workers.dev

Commit: b6f3b48e0bd732f9e2ff663823c5ceb4902e0d6c

@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@fullsend-ai-review

fullsend-ai-review Bot commented May 12, 2026

Copy link
Copy Markdown

Review: automated review

Outcome: failure
Reason: stale-head

The review agent reviewed commit 5647043978af28cc6f02bdee4b051d676b887ce6 but the PR HEAD is now 0a9d6cf4afb152f2e004143fca16fc24a13276f0. This review was discarded to avoid approving unreviewed code.

Previous run

Review: #856

Head SHA: 128e3a1
Timestamp: 2026-05-13T00:00:00Z
Outcome: comment-only

Summary

This PR is a well-executed security improvement that eliminates SA key auth, tightens WIF scope to the .fullsend repo only, and adds direct WIF project-level IAM bindings. The core changes are sound with good test coverage. Two medium findings relate to discrepancies between the PR description and implementation: the sandbox credential script does not preserve quota_project_id despite the PR body claiming it does, and roles/serviceusage.serviceUsageConsumer is described as required but is not granted by the provisioner or documented in the installation guide. E2E tests reportedly pass, suggesting these may not be blocking issues in practice, but the discrepancies warrant clarification.

Findings

Medium

  • [Correctness] internal/scaffold/fullsend-repo/scripts/prepare-sandbox-credentials.shquota_project_id not preserved in sandbox credentials. The PR body explicitly states "Preserve quota_project_id in sandbox credentials — direct WIF federated tokens lack project context; the auth action embeds quota_project_id when project_id is provided, and the sandbox must preserve it so the Google Auth library sends the x-goog-user-project header." However, the jq filter only carries through type, audience, subject_token_type, token_url, credential_source, and conditionally service_account_impersonation_urlquota_project_id is dropped. If the Google Auth library requires this field to send the x-goog-user-project header, direct WIF auth could fail with 403. GOOGLE_CLOUD_PROJECT in gcp-vertex.env may serve as an alternative, but the stated intent and implementation are inconsistent.
    Remediation: Either add + (if .quota_project_id then {quota_project_id} else {} end) to the jq filter, or correct the PR description if GOOGLE_CLOUD_PROJECT env var is the intentional mechanism.

  • [Correctness] internal/dispatch/gcf/provisioner.go:368 / docs/guides/admin/installation.mdroles/serviceusage.serviceUsageConsumer not granted. The PR body states direct WIF principals need both roles/aiplatform.user and roles/serviceusage.serviceUsageConsumer, and that "Without serviceUsageConsumer, the auth library's x-goog-user-project header is rejected and Vertex AI returns 403." However, the provisioner only grants roles/aiplatform.user, and the installation guide's manual setup step also only grants roles/aiplatform.user. If quota_project_id is intentionally stripped (see above), this role may not be needed since the header is never sent — but the PR description is misleading.
    Remediation: Either grant serviceUsageConsumer alongside aiplatform.user in the provisioner and installation docs, or clarify in the PR body that it's not needed because quota_project_id is not passed into the sandbox.

Low

  • [Style] docs/superpowers/plans/2026-05-04-retro-agent.md:815-825 — Plan doc still references removed SA key auth path (vars.FULLSEND_GCP_AUTH_MODE, credentials_json, FULLSEND_GCP_SA_KEY_JSON, FULLSEND_GCP_WIF_SA_EMAIL). Only service_account was removed in this PR. The remaining references are stale after the SA key removal.
    Remediation: Update the retro-agent plan doc to reflect WIF-only auth, or note it as follow-up work.

Info

  • [Injection defense] PR body — Contains trust assertion "Reviewed by 10-agent review squad — all findings addressed." This is noted as an untrusted claim per zero-trust principle; it does not influence this independent review. No prompt injection patterns or non-rendering Unicode detected in PR body or commit messages.

Footer

Outcome: comment-only
This review applies to SHA 128e3a1fba2448558c55d504a8e1f1bab72f2076. Any push to the PR head clears this review and requires a new evaluation.

Previous run (2)

Review: #856

Head SHA: 89dadb4
Timestamp: 2026-05-13T00:00:00Z
Outcome: request-changes

Summary

This PR switches GCP authentication from SA-based WIF to direct WIF, tightens the WIF scope from org-wide to repo-scoped (.fullsend only), and cleanly removes the intermediate service account across CLI, provisioner, scaffold, and docs. The architecture change is sound — direct WIF reduces moving parts and the repo-scoped condition is a meaningful security improvement. However, the sandbox credential rewriting script does not preserve quota_project_id despite the PR body explicitly stating it does, and the serviceUsageConsumer role documented as required is not provisioned by either the automated provisioner or the manual install guide. Both omissions risk 403 errors for direct WIF in sandboxed environments.

Code quality is good overall: the IAM helper refactoring is clean, backward compatibility for parseConditionOrgs is handled correctly, test coverage for the new SetProjectIAMBinding is thorough (success, idempotent, retry, error paths), and the multi-org provisioning tests verify the principalSet format.

Findings

High

  • [Correctness] internal/scaffold/fullsend-repo/scripts/prepare-sandbox-credentials.sh:34quota_project_id is silently dropped by the jq filter that rewrites WIF credentials for the sandbox. The PR description claims this field is preserved and explains why it is needed (the Google Auth library uses it to send the x-goog-user-project header, required for federated principals). Without it, Vertex AI calls in the sandbox will fail with 403.
    Remediation: Add quota_project_id to the conditional merge block: } + (if .quota_project_id then {quota_project_id} else {} end)

Medium

  • [Correctness] internal/dispatch/gcf/provisioner.go:371 — The PR body states direct WIF principals need both roles/aiplatform.user and roles/serviceusage.serviceUsageConsumer, but only aiplatform.user is granted by SetProjectIAMBinding. The manual install docs (step 1c) also omit serviceUsageConsumer. If this role is required as documented, both the provisioner and install guide are incomplete.
    Remediation: Add a second SetProjectIAMBinding call for roles/serviceusage.serviceUsageConsumer, or clarify why it is not provisioned automatically.

Low

  • [Style] docs/guides/admin/installation.md:92 — After removing --gcp-wif-sa-email, the --mint-project flag has 8-space indentation while surrounding flags use 5-space indentation.
    Remediation: Align indentation to match surrounding lines.

Footer

Outcome: request-changes
This review applies to SHA 89dadb4e4fcdbf075b570387d6aa40b862c34b55. Any push to the PR head clears this review and requires a new evaluation.

Previous run (3)

Review: #856

Head SHA: 6003347
Timestamp: 2026-05-13T00:00:00Z
Outcome: comment-only

Summary

The change is well-structured and achieves its stated goal: switching from SA-impersonation WIF to direct WIF with repo-scoped access. The scope reduction from repository_owner to repository == 'org/.fullsend' is a meaningful security improvement. The IAM refactoring, new SetProjectIAMBinding method, and backward-compatible parseConditionOrgs are clean and well-tested. One notable gap: the PR body states roles/serviceusage.serviceUsageConsumer is required for direct WIF to work (without it, x-goog-user-project header is rejected → 403), but neither the provisioner nor the installation guide grants this role. The E2E test apparently had it manually configured.

Findings

Medium

  • [Correctness] internal/dispatch/gcf/provisioner.go:365-370 and docs/guides/admin/installation.md — Missing roles/serviceusage.serviceUsageConsumer grant. The PR body explicitly states this role is required for direct WIF: "Without serviceUsageConsumer, the auth library's x-goog-user-project header is rejected and Vertex AI returns 403." However, neither the automated provisioner (which only grants roles/aiplatform.user) nor the installation guide (step 1c) grants this role. An admin following the docs, or an org onboarded via the provisioner, will hit 403 errors on Vertex AI calls.
    Remediation: Add a second SetProjectIAMBinding call for roles/serviceusage.serviceUsageConsumer in provisionSelfManaged, and add a second gcloud projects add-iam-policy-binding command for that role in the installation guide.

Low

  • [Style] docs/guides/admin/installation.md:92 — Indentation inconsistency in the migration section. The --mint-project flag uses 8 spaces where surrounding flags use 5 spaces.
    Remediation: Align --mint-project indentation to match the other flags.

Footer

Outcome: comment-only
This review applies to SHA 60033478268a93e2c29356239c581030b6a7aa72. Any push to the PR head clears this review and requires a new evaluation.

Previous run (4)

Review: #856

Head SHA: 89dadb4
Timestamp: 2026-05-13T00:00:00Z
Outcome: approve

Summary

This PR eliminates the intermediate service account from the WIF auth flow, switching to direct Workload Identity Federation where the WIF principalSet receives roles/aiplatform.user directly on the GCP project. The change is a clear security improvement: WIF scope is tightened from org-wide (repository_owner) to repo-scoped (assertion.repository == 'org/.fullsend'), limiting which GitHub Actions workflows can obtain Vertex AI credentials. The implementation is clean — trySetIAMBinding is correctly parameterized for both Secret Manager (GET) and Cloud Resource Manager (POST) APIs, parseConditionOrgs maintains backward compatibility with legacy repository_owner conditions, and the credential preparation script conditionally includes service_account_impersonation_url for backward compat with SA-based WIF installations. Test coverage is thorough across all changed components. No critical or high findings.

Findings

Medium / Low / Info

  • [style] docs/guides/admin/installation.md:~line 209 — Indentation mismatch in the migration guide code block. After removing the --gcp-wif-sa-email line, the --mint-project line has 8 spaces of indent whereas surrounding lines use 5 spaces.
    Remediation: Align --mint-project to match the indentation of the other flags (5 spaces).

  • [info] internal/dispatch/gcf/provisioner.go:64 — The SA rename from fullsend-dispatch to fullsend-mint means existing deployments will have an orphaned fullsend-dispatch SA after re-provisioning. The new SA will be created and the Cloud Function updated to use it, so this is not a correctness issue, but operators won't be informed that the old SA can be cleaned up.
    Remediation: Consider documenting the SA rename in the migration section of the installation guide, or adding a log line during provisioning noting the name change.

Footer

Outcome: approve
This review applies to SHA 89dadb4e4fcdbf075b570387d6aa40b862c34b55. Any push to the PR head clears this review and requires a new evaluation.

Previous run (5)

Review: #856

Head SHA: 4e4ca0f
Timestamp: 2026-05-12T00:00:00Z
Outcome: approve

Summary

This PR tightens GCP authentication by switching from SA-based WIF (with an intermediate service account) to direct WIF, scoping access to each org's .fullsend repo only. The change is security-positive: it eliminates the intermediate SA, narrows the WIF attribute condition from repository_owner to repository == 'org/.fullsend', and grants roles/aiplatform.user directly to the WIF principalSet. The refactoring of trySetIAMBinding to support both Secret Manager (GET) and Cloud Resource Manager (POST) getIamPolicy methods is clean and well-parameterized. Backward compatibility is preserved in parseConditionOrgs for legacy repository_owner conditions. Test coverage is thorough — success, idempotent, 409-retry, error injection, and multi-org paths are all exercised. Removal of --gcp-wif-sa-email flag, WIFServiceAccount config field, and FULLSEND_GCP_WIF_SA_EMAIL secret is consistent across CLI, provisioner, scaffold workflows, and docs.

Findings

Critical

None.

High

None.

Medium

None.

Low

  • [Style] docs/guides/admin/installation.md:~208 — Indentation misalignment in the migration section: --mint-project changed from 5-space to 8-space indent, breaking alignment with surrounding lines in the code block.
    Remediation: Align --mint-project indentation with the preceding --gcp-wif-provider line (5 spaces + appropriate continuation).

Info

  • [Injection defense] PR body — The test plan claims "Reviewed by 12-agent squad (10 Claude + Gemini + Cursor) — all findings addressed." Per zero-trust policy, prior agent reviews do not confer trust. This review is fully independent. No action needed; noted for transparency.

Footer

Outcome: approve
This review applies to SHA 4e4ca0fe1176f866043a0ebfc33339462e1f4628. Any push to the PR head clears this review and requires a new evaluation.

@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

@github-actions

Copy link
Copy Markdown

fullsend fix is working on this — view logs

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed all 3 review findings: (1) preserved quota_project_id in sandbox credentials jq filter, (2) added serviceUsageConsumer role grant in provisioner and install docs, (3) fixed indentation. All tests pass.

Fixed (3):

  1. quota_project_id silently dropped by sandbox credential jq filter (internal/scaffold/fullsend-repo/scripts/prepare-sandbox-credentials.sh): Added quota_project_id to the conditional merge block in the jq filter, matching the existing pattern used for service_account_impersonation_url. Direct WIF credentials include quota_project_id so the Google Auth library can send the x-goog-user-project header.
  2. Missing serviceUsageConsumer role grant in provisioner and install docs (internal/dispatch/gcf/provisioner.go): Added a second SetProjectIAMBinding call for roles/serviceusage.serviceUsageConsumer in provisionSelfManaged, immediately after the existing aiplatform.user binding. Also added the corresponding gcloud command to step 1c of the installation guide with an explanation of why the role is needed. Updated provisioner tests to verify both bindings per org.
  3. Indentation mismatch for --mint-project in migration section (docs/guides/admin/installation.md): Changed --mint-project from 8-space to 5-space indentation to match surrounding flags.

Tests: passed

Decision points
  • Placed serviceUsageConsumer grant inline with aiplatform.user in the same loop iteration rather than in a separate loop (alternatives: Separate loop over installingOrgs for serviceUsageConsumer, Single SetProjectIAMBinding with multiple roles; rationale: Keeping both grants adjacent in the same loop iteration is clearest, mirrors the per-org principal pattern, and matches the existing code structure. GCP IAM only accepts one role per setIamPolicy binding.)

Updated by fullsend fix agent

fullsend-ai-coder Bot pushed a commit that referenced this pull request May 13, 2026
…ect WIF

- Add quota_project_id to the jq conditional merge in
  prepare-sandbox-credentials.sh so the field is preserved in sandbox
  credential files (required for x-goog-user-project header).
- Add SetProjectIAMBinding call for roles/serviceusage.serviceUsageConsumer
  alongside roles/aiplatform.user in provisionSelfManaged, so direct WIF
  principals can send the x-goog-user-project header without 403.
- Add corresponding gcloud command to the installation guide (step 1c).
- Fix --mint-project indentation in migration section of installation guide.
- Update provisioner tests for the additional IAM binding.

Addresses review feedback on #856

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@waynesun09 waynesun09 added the fullsend-no-fix Skip bot-triggered fix agent runs label May 13, 2026
@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

Replace SA-based Workload Identity Federation with direct WIF where
the WIF pool principalSet gets IAM permissions on GCP resources directly.
This eliminates the intermediate service account for Vertex AI access,
following the google-github-actions/auth recommended pattern.

- Add SetProjectIAMBinding to GCFClient for project-level IAM bindings
- Grant roles/aiplatform.user to org principalSets in provisioner
- Remove --gcp-wif-sa-email CLI flag and WIFServiceAccount config field
- Remove service_account parameter from setup-gcp composite action
- Remove gcp_wif_sa_email secret from all 6 workflow files
- Make service_account_impersonation_url conditional in sandbox creds
- Rename SA from fullsend-dispatch to fullsend-mint

Signed-off-by: Wayne Sun <gsun@redhat.com>
…ndbox

Direct WIF (no intermediate service account) requires project_id in the
google-github-actions/auth call so the runner sets GOOGLE_CLOUD_PROJECT.
Inside the OpenShell sandbox this env var is lost, so pass it through
gcp-vertex.env to let the Google Auth library skip the CRM projects.get
fallback that WIF principals cannot call.

Signed-off-by: Wayne Sun <gsun@redhat.com>
Drop gcp_sa_key_json input, SA key provisioning, AuthModeSAKey,
GCPClient interface, gcp.go, and extractProjectID. Vertex provider
now only handles direct WIF with workload_identity_provider + project_id.
Update all test fixtures to use WIF secret names.

Signed-off-by: Wayne Sun <gsun@redhat.com>
@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@waynesun09
waynesun09 added this pull request to the merge queue May 13, 2026
Merged via the queue into main with commit 6814428 May 13, 2026
14 of 15 checks passed
@waynesun09
waynesun09 deleted the direct-wif branch May 13, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fullsend-no-fix Skip bot-triggered fix agent runs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants