Skip to content

feat: add Workload Identity Federation auth for GCP Vertex AI - #448

Merged
waynesun09 merged 9 commits into
mainfrom
worktree-wif-migration
Apr 27, 2026
Merged

feat: add Workload Identity Federation auth for GCP Vertex AI#448
waynesun09 merged 9 commits into
mainfrom
worktree-wif-migration

Conversation

@waynesun09

@waynesun09 waynesun09 commented Apr 25, 2026

Copy link
Copy Markdown
Member

Summary

  • Add WIF as the recommended GCP auth method, replacing long-lived SA key JSON with short-lived OIDC token exchange
  • Support both WIF and SA key modes with automatic fallback in workflows — google-github-actions/auth@v3 uses WIF when secrets are set, falls back to SA key otherwise
  • Add credential prep step in workflows to pre-fetch OIDC tokens and rewrite credential configs for sandbox compatibility
  • Update installation guide with WIF setup instructions, security trade-offs for org-wide vs repo-scoped access, and SA key migration path

Fixes:
#245

Changes

Go code:

  • vertex.go — new AuthMode type (wif/sa_key), WIF secret constants, updated SecretNames()/Provision()/Variables()
  • admin.go--gcp-wif-provider and --gcp-wif-sa-email CLI flags with mutual exclusion validation
  • harness.go — optional OIDC token file in host_files
  • Unit tests for WIF mode in vertex_test.go and admin_test.go

Workflows (code/review/triage):

  • id-token: write permission for OIDC token requests
  • Dual-mode auth step (WIF preferred, SA key fallback)
  • Credential prep step — rewrites credential_source.url to file-based config for sandbox isolation

Harness configs:

  • Optional GCP_OIDC_TOKEN_FILE mapped into sandbox at /tmp/workspace/.gcp-oidc-token

Docs:

  • Installation guide: WIF setup (pool, provider, IAM binding), attribute-condition security guidance, migration from SA key
  • ADR-0014: new secrets FULLSEND_GCP_WIF_PROVIDER and FULLSEND_GCP_WIF_SA_EMAIL

Test plan

  • make go-test — vertex provider tests cover both WIF and SA key modes
  • make go-vet — no new issues
  • make lint — passes
  • Manual E2E: set WIF secrets in .fullsend repo, trigger agent workflow, verify auth step shows "Authenticated using Workload Identity Federation"
  • Manual E2E: remove WIF secrets, verify SA key fallback still works
  • Verify no credential values appear in workflow logs (secrets are masked)

Replace long-lived service account key JSON with short-lived OIDC
token exchange via GCP Workload Identity Federation. SA key mode is
retained as a fallback — the google-github-actions/auth action
auto-selects WIF when the provider secret is set.

Changes:
- vertex.go: add AuthMode type, WIF constants, Config fields, split
  Provision into WIF/SAKey paths
- admin.go: add --gcp-wif-provider and --gcp-wif-sa-email flags with
  mutual exclusivity validation against SA key flags
- workflows: add id-token:write permission, dual-mode auth step, and
  credential prep step that rewrites OIDC token paths for sandbox
- harness configs: add optional OIDC token file to host_files
- installation.md: WIF as recommended Option A, SA key as legacy
  Option B, migration guide
- ADR-0014: add WIF secrets to credential surface table

Signed-off-by: Wayne Sun <gsun@redhat.com>
Split the GCP auth step into two conditional steps (WIF vs SA key) to
satisfy google-github-actions/auth@v3 exactlyOneOf constraint. Rewrite
the sandbox credential prep step to pre-fetch OIDC tokens via
credential_source.url instead of the non-existent .file path.

Add Optional field to HostFile struct so the OIDC token host file can
be skipped in SA key mode without bash default syntax that Go's
os.ExpandEnv does not support. Default AuthMode to sa_key in New() to
prevent zero-value fallthrough.

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

Add explanatory comments to the sandbox credential preparation step in
review.yml and triage.yml for consistency with code.yml. Expand the WIF
attribute-condition documentation in the installation guide to explain
the security trade-offs between org-wide and repo-scoped access.

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

All 4 review agents flagged the repo-scoped WIF guidance as incomplete:
changing only the attribute-condition without updating the attribute-mapping
and IAM principalSet member would produce a broken or misleadingly-scoped
config. Add attribute.repository to the default mapping and document the
required IAM binding change for repo-scoped access.

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

github-actions Bot commented Apr 25, 2026

Copy link
Copy Markdown

Site preview

Preview: https://8caacd08-site.fullsend-ai.workers.dev

Commit: b4d48061c076f4ad80f1313c490ef75d2270e415

@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.

Review: #448

Head SHA: f45c047
Timestamp: 2026-04-25T00:00:00Z
Outcome: comment-only

Summary

This PR adds Workload Identity Federation as the recommended GCP auth method for Vertex AI, with a clean fallback to SA key mode. The implementation is well-structured: CLI flag validation enforces mutual exclusion, the vertex provider cleanly separates WIF and SA key provisioning, the Optional field on HostFile is a sensible extension, and the workflow credential-prep step handles sandbox isolation correctly by pre-fetching the OIDC token and rewriting the credential config to use a file source. Unit tests cover the new WIF provisioning paths and error cases. No critical or high findings — the items below are worth noting for future improvement.

Findings

Medium

  • [Correctness] internal/scaffold/fullsend-repo/.github/workflows/code.yml (and review.yml, triage.yml) — The credential prep step pre-fetches the GitHub OIDC token once and writes it to a static file mapped into the sandbox. When the GCP SDK's access token expires (~1 hour), it will re-read this file to obtain a fresh OIDC token for STS exchange, but the file contains the original (potentially expired) OIDC token. Agent runs exceeding the OIDC token lifetime will fail on access token refresh. Consider documenting this limitation or implementing a token-refresh mechanism (e.g., a host-side sidecar that periodically refreshes the OIDC token file).

Low

  • [Correctness] internal/inference/vertex/vertex.goNewAnalyzeOnly() creates a provider with an empty config, so SecretNames() always returns SA key secret names (FULLSEND_GCP_SA_KEY_JSON, FULLSEND_GCP_PROJECT_ID). For WIF installations, admin analyze will look for the wrong secrets and report them as missing. The auth mode would need to be persisted in config.yaml or detected from existing secrets to fix this.

  • [Style] internal/cli/admin.go — The explanatory comment // Zero credential bytes when done to limit exposure in memory. was removed from the credential-zeroing defer block. The defer itself is preserved, but the comment clarified why the zeroing is done, which is valuable for security-sensitive code.

Info

  • [Correctness] internal/harness/harness.go / internal/cli/run.go — The Optional field on HostFile correctly handles the case where the env var expands to empty, but does not handle the case where the env var is set to a path that doesn't exist on disk (the SCP would fail). In practice this doesn't arise because the workflow only sets GCP_OIDC_TOKEN_FILE after creating the file, but a more defensive implementation would also skip when the file is absent.

  • [Correctness] The three workflow files (code.yml, review.yml, triage.yml) contain identical 35-line credential prep blocks. This duplication is consistent with the existing scaffold pattern where each workflow is self-contained, but a shared composite action could reduce maintenance burden if more auth modes are added later.

Footer

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

@rh-hemartin

Copy link
Copy Markdown
Member

Solves #245 ?

@waynesun09

Copy link
Copy Markdown
Member Author

@rh-hemartin yes, updated issue description with it included

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

@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.

Review: #448

Head SHA: 593973f
Timestamp: 2026-04-27T00:00:00Z
Outcome: request-changes

Summary

This PR adds Workload Identity Federation as an alternative to SA key JSON for GCP auth, with dual-mode workflow steps and a sandbox credential prep mechanism. The Go code changes (CLI validation, vertex provider, harness Optional field) are well-structured and tested. However, the NewAnalyzeOnly() provider does not account for WIF mode, meaning health checks will report incorrect results for WIF-configured orgs. The pre-fetched OIDC token has a ~10 minute lifetime with no refresh mechanism in the sandbox, creating a silent auth failure for agent runs exceeding 1 hour. The Optional host file handling is incomplete — it only covers the env-var-unset case, not the file-missing case.

Findings

Critical

(none)

High

(none)

Medium

  • [Correctness] internal/inference/vertex/vertex.go:64NewAnalyzeOnly() always returns SA key secret names. The analyze/health-check path (admin.go:608) calls this for any vertex-configured org, but WIF orgs don't have FULLSEND_GCP_SA_KEY_JSON. Health checks will produce false negatives for WIF-configured orgs.
    Remediation: Pass AuthMode into NewAnalyzeOnly (or detect mode from existing secrets/config in loadExistingInferenceProvider).

  • [Correctness] internal/scaffold/fullsend-repo/.github/workflows/code.yml (and review.yml, triage.yml) — The OIDC token is pre-fetched once and written to a static file. GitHub OIDC tokens expire after ~10 minutes. The GCP access token obtained via STS lasts ~1 hour, but when it expires, the GCP SDK will try to re-read the stale OIDC token file and fail. Agent runs exceeding 1 hour will silently lose GCP auth.
    Remediation: Document this limitation. Consider a background refresh mechanism or allowing sandbox OIDC endpoint access.

  • [Correctness] internal/cli/run.go:637 — The Optional field only skips the host file when the env var expands to empty. If the env var is set but the file doesn't exist (misconfiguration), SCP will fail with a hard error rather than being gracefully skipped.
    Remediation: Add an os.Stat check after expansion — if hf.Optional and the file doesn't exist, continue.

Low

  • [Intent alignment] Issue #245 requests Direct WIF (no service account). This PR implements SA-based WIF (with service_account impersonation). SA-based WIF is a security improvement but doesn't match the issue's stated goal of eliminating the SA layer entirely.
    Remediation: Update issue #245 to reflect the chosen approach and rationale.

  • [Correctness] docs/guides/admin/installation.md — Migration step 5 relies on manual deletion of FULLSEND_GCP_SA_KEY_JSON. The ADR spec says WIF and SA key secrets "must not coexist." A forgotten manual step leaves a long-lived credential in the repo.
    Remediation: Have the installer delete the opposing auth mode's secrets during provisioning.

Info

  • [Style] The "Prepare sandbox credentials" bash block (~30 lines) is duplicated identically across code.yml, review.yml, and triage.yml. Extract to a shared script or composite action.

Footer

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

- NewAnalyzeOnly now accepts AuthMode so health checks return correct
  secret names for WIF-mode orgs (finding 1)
- Document OIDC token lifetime limitation in all workflow credential
  prep steps (finding 2)
- Add os.Stat check for optional host files after env var expansion
  to prevent hard errors when file does not exist (finding 3)
- Auto-cleanup of opposing auth secrets deferred to #458 (finding 5)

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

@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.

Review: #448

Head SHA: 8a51ba2
Timestamp: 2026-04-27T00:00:00Z
Outcome: comment-only

Summary

This PR adds Workload Identity Federation as an alternative GCP auth mode alongside the existing SA key flow. The Go code changes are well-structured with proper mutual exclusion validation, clear separation of WIF/SA-key provisioning paths, and good test coverage. The workflow changes correctly implement dual-mode auth with OIDC token pre-fetching for sandbox compatibility. The main observations are operational: the OIDC bearer token extracted at runtime is not masked in workflow logs, and the install flow does not clean up stale SA key secrets when migrating to WIF mode (the ADR states the modes are mutually exclusive but the code relies on manual cleanup).

Findings

Medium

  • [content security] internal/scaffold/fullsend-repo/.github/workflows/code.yml (and review.yml, triage.yml) — The "Prepare sandbox credentials" step extracts OIDC_AUTH (a bearer token) from the credential config via jq and passes it to curl -H. Since this value is not a GitHub Actions secret (it is a runtime-extracted string), it will not be automatically masked. If ACTIONS_STEP_DEBUG is enabled or logs are forwarded, the bearer token could be exposed. Although the token is short-lived (~10 min), it is valid for GCP STS exchange during that window.
    Remediation: Add echo "::add-mask::$OIDC_AUTH" before the curl command to ensure the value is masked in logs.

  • [correctness] internal/layers/inference.go / internal/inference/vertex/vertex.go — When switching from SA key mode to WIF mode, the install flow creates the new WIF secrets but does not delete the old FULLSEND_GCP_SA_KEY_JSON secret. The ADR states "WIF secrets and the SA key secret must not coexist," but the code does not enforce this invariant. The migration docs tell users to delete it manually (step 5), but if they forget, a stale long-lived SA key remains in the repo — undermining the security benefit of WIF.
    Remediation: In InferenceLayer.Install(), after provisioning new secrets, delete any secrets from the "other" auth mode (e.g., if WIF mode, delete FULLSEND_GCP_SA_KEY_JSON; if SA key mode, delete WIF secrets). Alternatively, add a validation check in Analyze() that flags both modes' secrets coexisting as StatusDegraded.

Low

  • [platform security] internal/scaffold/fullsend-repo/.github/workflows/code.yml (and review.yml, triage.yml) — The OIDC token is written to $RUNNER_TEMP/gcp-oidc-token at the default umask. While $RUNNER_TEMP is runner-scoped, restricting permissions would follow defense-in-depth.
    Remediation: Add umask 077 before or chmod 600 "$OIDC_DEST" after the curl command.

  • [correctness] internal/cli/admin.go:625 — In runAnalyze, the error from client.RepoSecretExists() for WIF detection is silently discarded (wifExists, _ := ...). If the API call fails (e.g., permission issue), the analysis silently defaults to SA key mode, potentially producing a misleading analyze report.
    Remediation: Log the error or add a comment explaining why it is intentionally ignored.

  • [correctness] internal/inference/vertex/vertex.go:provisionWIF() — No format validation on WIFProvider. The expected format is projects/*/locations/*/workloadIdentityPools/*/providers/*. A typo in the CLI flag would be stored as a secret and only fail at workflow runtime (auth action), making debugging harder.
    Remediation: Add a basic regex check for the expected resource name format in provisionWIF() or in the CLI flag validation.

Info

  • [intent alignment] Issue #245 specifically requests "Direct Workload Identity Federation" (no intermediate service account), but this PR implements SA-based WIF (with service_account impersonation). This is a reasonable pragmatic choice given the limitations noted in the issue (direct WIF cannot generate OAuth 2.0 access tokens), but the issue should be updated to reflect this decision.

  • [correctness] The OIDC token expires after ~10 minutes while the GCP access token lasts 1 hour. Agent runs exceeding 1 hour will fail on access token refresh. The workflow comments document this limitation, which is appropriate for now.

Footer

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

- Add ::add-mask:: for OIDC_AUTH to prevent bearer token leaking in
  workflow logs when ACTIONS_STEP_DEBUG is enabled
- chmod 600 the OIDC token file for defense-in-depth

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

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review summary

Good PR — the WIF integration is well-structured, the CLI validation is thorough, and test coverage for the new vertex auth modes is solid. One actionable finding:

Request changes (1):

  • The ~40-line "Prepare sandbox credentials" bash script is duplicated verbatim across all 3 workflow files. Extract to a shared script (e.g. scripts/prepare-sandbox-credentials.sh).

Verified non-issues:

  • secrets.* in step if: expressions — valid syntax (GitHub Actions auto-wraps in ${{ }}).
  • Stale SA key cleanup during migration — already deferred to #458.
  • OIDC token file permissions — already fixed in 77e8ea5 with chmod 600 and ::add-mask::.

Comment thread internal/scaffold/fullsend-repo/.github/workflows/code.yml
Move the ~40-line WIF credential preparation bash block from
code.yml, review.yml, and triage.yml into scripts/prepare-sandbox-credentials.sh
so bug fixes only need to be applied once.

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

@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.

Review: #448

Head SHA: 2ebe0b4
Timestamp: 2026-04-27T00:00:00Z
Outcome: comment-only

Summary

This PR adds SA-based Workload Identity Federation as an alternative to long-lived service account keys for GCP Vertex AI auth. The implementation is well-structured with proper mutual exclusion validation, dual-mode workflow steps, a sandbox credential preparation script, and good test coverage. No critical or high findings — the issues below are worth addressing but do not block merge.

Findings

Medium

  • [Intent alignment] PR description / Issue #245 — The linked issue (#245) explicitly requests Direct Workload Identity Federation (no intermediate service account — IAM bindings on the pool principal directly). This PR implements SA-based WIF (service account impersonation via workloadIdentityUser role). SA-based WIF is still a significant security improvement over long-lived keys, but "Fixes: #245" may be premature since the issue's core request (eliminate the SA entirely) is not addressed. The issue's "Limitations to verify" section hints at why direct WIF may not work (no OAuth 2.0 access tokens), but that determination should be documented.
    Remediation: Either update the issue to reflect that SA-based WIF is the chosen approach (with rationale for why direct WIF was ruled out), or change "Fixes: #245" to "Partially addresses #245" and file a follow-up for direct WIF evaluation.

  • [Correctness] internal/cli/admin.go:624-627 — In runAnalyze, the error from RepoSecretExists is silently discarded: if wifExists, _ := client.RepoSecretExists(...). If the API call fails (e.g., insufficient permissions), analyze silently defaults to SA key mode and reports incorrect secret expectations.
    Remediation: Log the error or return it so operators can diagnose analyze failures.

  • [Correctness] internal/layers/inference.go / internal/inference/vertex/vertex.go — When migrating from SA key to WIF, the installer creates the new WIF secrets but does not delete the old FULLSEND_GCP_SA_KEY_JSON secret. The ADR update (SPEC.md) states the two auth modes "must not coexist," but the InferenceLayer.Install() method only creates secrets from Provision() — it never removes secrets from the previous mode. The migration guide (step 5) tells admins to manually remove the old secret, but this is error-prone and contradicts the "must not coexist" language.
    Remediation: Consider having the installer delete FULLSEND_GCP_SA_KEY_JSON when WIF mode is active (and vice versa), or soften the ADR language to "should not" with a note that manual cleanup is required during migration.

Low

  • [Content security] scripts/prepare-sandbox-credentials.sh:29 — The script masks the Authorization header (::add-mask::$OIDC_AUTH) but does not mask the OIDC token body written to $OIDC_DEST. If a subsequent workflow step logs the file contents, the raw OIDC token would appear unmasked. The token is short-lived (~10 min) which limits impact.
    Remediation: Add ::add-mask::$(cat "$OIDC_DEST") after the curl step, or mask the token value before writing it.

Info

  • [Style/conventions] docs/guides/admin/installation.md — The 1-hour access token lifetime limitation (documented in the script comments) is not mentioned in the installation guide. Admins should be aware that WIF-based agent runs are effectively capped at ~1 hour before needing a token refresh that will fail in the sandbox.
    Remediation: Add a note to the WIF section of the installation guide about the 1-hour runtime ceiling.

Footer

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

The RepoSecretExists call for WIF mode detection was silently
discarding errors, causing analyze to default to SA key mode
without any indication when the API call fails.

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

@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.

Review: #448

Head SHA: b4d4806
Timestamp: 2026-04-27T00:00:00Z
Outcome: comment-only

Summary

This PR adds Workload Identity Federation (WIF) as an alternative auth mode for GCP Vertex AI, with clean dual-mode support across CLI flags, provisioning logic, scaffold workflows, and documentation. The implementation is solid — flag validation, mutual exclusion, test coverage, and the sandbox credential-prep script are all well-constructed. No critical or high findings. A few observations worth noting: the install flow doesn't clean up stale secrets when switching auth modes (conflicting with the ADR-0014 mutual-exclusion constraint), the implementation uses SA-based WIF rather than the Direct WIF requested in issue #245, and the OIDC token content should be masked in workflow logs.

Findings

Medium

  • [Correctness] internal/cli/admin.goNo secret cleanup on auth mode migration. When an admin re-runs install with --gcp-wif-provider (migrating from SA key to WIF), the new WIF secrets are created but the old FULLSEND_GCP_SA_KEY_JSON secret is not deleted. ADR-0014 states the two auth modes are mutually exclusive and "must not coexist." The install guide (step 5 of migration) tells admins to manually delete the old secret, but the tooling should enforce this. Similarly, migrating from WIF back to SA key would leave orphaned WIF secrets.
    Remediation: Add a cleanup step in the secrets layer (or the install command) that deletes the opposing mode's secrets when provisioning. For WIF mode: delete FULLSEND_GCP_SA_KEY_JSON. For SA key mode: delete FULLSEND_GCP_WIF_PROVIDER and FULLSEND_GCP_WIF_SA_EMAIL.

  • [Intent alignment] Issue #245 requests Direct Workload Identity Federation (no intermediate service account — the WIF pool principal gets IAM bindings directly). This PR implements SA-based WIF (service account impersonation via service_account_impersonation_url). This is a valid incremental step but doesn't fully address the issue's ask. The issue specifically calls out eliminating "an entire layer of credential indirection" and not needing a SA email at all.
    Remediation: If this is intentional (incremental delivery), note it in the PR description or issue. If Direct WIF is still the goal, file a follow-up issue.

Low

  • [Content security] scripts/prepare-sandbox-credentials.sh:31 — The script masks the Authorization header (::add-mask::$OIDC_AUTH) but does not mask the OIDC token content written to $OIDC_DEST. If GitHub Actions debug logging is enabled (ACTIONS_STEP_DEBUG=true), the token content could appear in logs. The token is short-lived (~10 min) but still a credential.
    Remediation: Add echo "::add-mask::$(cat $OIDC_DEST)" after writing the token file, or pipe curl output through a masking step.

Info

  • [Correctness] Workflow files (code.yml, review.yml, triage.yml) — Both auth steps (WIF and SA key) use if: secrets.FULLSEND_GCP_WIF_PROVIDER != '' / == '' to select the mode. If both secret sets are present (violating ADR-0014), both steps could execute, with the SA key step overwriting GOOGLE_APPLICATION_CREDENTIALS set by the WIF step. The ADR constraint prevents this in practice, but a defensive guard (e.g., failing the workflow if both are set) would make the invariant explicit.

  • [Style] internal/inference/vertex/vertex.go — The Variables() method returns the region variable regardless of auth mode, which is correct. Minor: consider adding a doc comment noting WIF mode still requires the region variable.

Footer

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

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Strategic review

Good idea, right scope, addresses a real issue. Migrating from long-lived SA key JSON to WIF is a clear security win, and the SA-impersonation variant is the correct choice given that Vertex AI requires OAuth 2.0 access tokens (which direct WIF cannot produce).

All code-level findings from my previous review have been addressed:

  • ✅ Credential prep script extracted to scripts/prepare-sandbox-credentials.sh
  • ✅ OIDC bearer token masked with ::add-mask::
  • ✅ Token file permissions restricted with chmod 600
  • RepoSecretExists error surfaced as a warning

One note (non-blocking): The PR says "Fixes: #245" but #245 specifically requests Direct WIF (no service account). This PR implements SA-based WIF — the right call, but the rationale (Vertex AI requires OAuth 2.0 tokens, which direct WIF can't produce) should be documented on #245 so the decision is discoverable.

@ralphbean

Copy link
Copy Markdown
Member

Note (non-blocking): This PR says "Fixes: #245" but #245 specifically requests Direct Workload Identity Federation (no intermediate service account). This PR implements SA-based WIF instead — which is the correct choice because direct WIF cannot generate OAuth 2.0 access tokens, and Vertex AI requires them.

Consider updating #245 with this rationale so the decision is discoverable, and changing the link to "Partially addresses #245" (or closing #245 with a comment explaining why direct WIF was ruled out).

@waynesun09
waynesun09 added this pull request to the merge queue Apr 27, 2026
Merged via the queue into main with commit 7415553 Apr 27, 2026
6 checks passed
@waynesun09
waynesun09 deleted the worktree-wif-migration branch April 27, 2026 18:25
ascerra added a commit to ascerra/fullsend that referenced this pull request Apr 28, 2026
- Use PRE_AGENT_HEAD instead of HEAD~1 for multi-commit safety
- Add dual WIF/SA-key auth to fix.yml aligned with fullsend-ai#448
- Trap exit code 2 in post-fix.sh so labels/summary still run
- Filter review body fetch to review bot identity only
- Add fork PR check + bot exclusion to dispatch-fix-human
- Fail-safe iteration counter defaults to cap on API failure
- Fix hardcoded >= 23 in workflows_test.go (use managedFiles)
- Add $id to fix-result.schema.json for consistency
- Document exit code 2 in process-fix-result.py docstring
- Add test for return 2 path in process-fix-result-test.py
- Fix leading spaces in ::error:: annotation in post-fix.sh

Made-with: Cursor
ascerra added a commit to ascerra/fullsend that referenced this pull request Apr 28, 2026
- Use PRE_AGENT_HEAD instead of HEAD~1 for multi-commit safety
- Add dual WIF/SA-key auth to fix.yml aligned with fullsend-ai#448
- Trap exit code 2 in post-fix.sh so labels/summary still run
- Filter review body fetch to review bot identity only (exact org match)
- Add fork PR check + bot exclusion to dispatch-fix-human
- Fail-safe iteration counter defaults to cap on API failure
- Fix hardcoded >= 23 in workflows_test.go (use managedFiles)
- Add $id to fix-result.schema.json for consistency
- Document exit code 2 in process-fix-result.py docstring
- Add test for return 2 path in process-fix-result-test.py
- Fix leading spaces in ::error:: annotation in post-fix.sh
- Add FULLSEND_OUTPUT_FILE test cases to validate-output-schema-test.sh
- Move imports to module level in process-fix-result-test.py

Made-with: Cursor
ascerra added a commit to ascerra/fullsend that referenced this pull request Apr 28, 2026
- Use PRE_AGENT_HEAD instead of HEAD~1 for multi-commit safety
- Add dual WIF/SA-key auth to fix.yml aligned with fullsend-ai#448
- Trap exit code 2 in post-fix.sh so labels/summary still run
- Filter review body fetch to review bot identity only (exact org match)
- Add fork PR check + bot exclusion to dispatch-fix-human
- Fail-safe iteration counter defaults to cap on API failure
- Fix hardcoded >= 23 in workflows_test.go (use managedFiles)
- Add $id to fix-result.schema.json for consistency
- Document exit code 2 in process-fix-result.py docstring
- Add test for return 2 path in process-fix-result-test.py
- Fix leading spaces in ::error:: annotation in post-fix.sh
- Add FULLSEND_OUTPUT_FILE test cases to validate-output-schema-test.sh
- Move imports to module level in process-fix-result-test.py

Made-with: Cursor
ascerra added a commit to ascerra/fullsend that referenced this pull request Apr 28, 2026
- Use PRE_AGENT_HEAD instead of HEAD~1 for multi-commit safety
- Add dual WIF/SA-key auth to fix.yml aligned with fullsend-ai#448
- Trap exit code 2 in post-fix.sh so labels/summary still run
- Filter review body fetch to review bot identity only (exact org match)
- Add fork PR check + bot exclusion to dispatch-fix-human
- Fail-safe iteration counter defaults to cap on API failure
- Fix hardcoded >= 23 in workflows_test.go (use managedFiles)
- Add $id to fix-result.schema.json for consistency
- Document exit code 2 in process-fix-result.py docstring
- Add test for return 2 path in process-fix-result-test.py
- Fix leading spaces in ::error:: annotation in post-fix.sh
- Add FULLSEND_OUTPUT_FILE test cases to validate-output-schema-test.sh
- Move imports to module level in process-fix-result-test.py

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants