feat(#6226): gate WIF secrets in per-repo shim by credential mode - #6241
feat(#6226): gate WIF secrets in per-repo shim by credential mode#6241fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
|
🤖 Finished Review · ✅ Success · Started 9:52 PM UTC · Completed 10:11 PM UTC Commit: |
Site previewPreview: https://ce4501f2-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewFindingsMedium
Low
Next steps:
Previous runReviewFindingsHigh
Medium
Low
Next steps:
Previous run (2)ReviewFindingsHigh
Medium
Low
Labels: PR modifies scaffold/install code, dispatch workflow, and documentation Next steps:
Previous run (3)ReviewFindingsHigh
Medium
Low
Next steps:
|
e3ac5c2 to
e188658
Compare
|
🤖 Finished Review · ✅ Success · Started 10:54 PM UTC · Completed 11:14 PM UTC Commit: |
e188658 to
b300bb1
Compare
|
🤖 Finished Review · ✅ Success · Started 12:42 AM UTC · Completed 1:22 AM UTC Commit: |
Signed-off-by: Greg Allen <gallen@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
b300bb1 to
8ac981b
Compare
|
🤖 Finished Review · ✅ Success · Started 1:39 AM UTC · Completed 2:00 AM UTC Commit: |
There was a problem hiding this comment.
See the review comment for full details.
Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:
.github/workflows/reusable-dispatch.yml(file-level): Line 626 · [medium] API contract violation
The setup-gcp composite action is called unconditionally in every stage job, and its action.yml declares gcp_wif_provider as required: true. For OIDC-mode repos, stripWIFSecrets removes FULLSEND_GCP_WIF_PROVIDER from the generated shim, so the secret reaches reusable-dispatch.yml as an empty string. google-github-actions/auth will fail with an empty workload_identity_provider, aborting the stage job. This is a known-deferred item (issue #6226 item 2) and is fail-closed, but the PR claims Closes #6226 while this item remains unaddressed.
Suggested fix: Either add an if condition to each setup-gcp step (e.g., if: secrets.FULLSEND_GCP_WIF_PROVIDER != ''), or change Closes #6226 to a partial reference and file a follow-up for the setup-gcp conditional.
.github/workflows/reusable-dispatch.yml:66: [medium] protected-path
This PR modifies a file under the .github/ protected path. The change is authorized by issue #6226 item 3 (change FULLSEND_GCP_PROJECT_ID to required: false). Human approval is required for all protected-path changes regardless of context.
.github/workflows/reusable-dispatch.yml:66: [low] secret-requirement-relaxation
FULLSEND_GCP_PROJECT_ID changed from required: true to required: false. Combined with FULLSEND_GCP_WIF_PROVIDER already being required: false, the reusable workflow now accepts calls with no GCP credentials. This is fail-closed in practice (setup-gcp still rejects empty values), but a YAML comment explaining why these secrets are optional would aid future maintainers.
internal/scaffold/installfiles.go:49: [low] API-shape-consistency
CollectPerRepoInstallFiles takes 4 positional parameters (bool + 3 strings), while its sibling CollectInstallFiles uses a CollectInstallFilesOptions struct. The new credentialMode parameter deepens this divergence.
internal/repos/remote_scaffold.go:34: [low] growing-positional-parameter-list
FetchRemoteScaffold now accepts 7 parameters including 4 consecutive strings, which is error-prone at call sites.
internal/scaffold/render.go:120: [low] magic-string-vs-constant
stripWIFSecrets compares against the string literal oidc rather than a named constant. The CredModeOIDC constant in the repos package cannot be imported due to circular dependency; consider a package-local constant.
Summary
FULLSEND_GCP_WIF_PROVIDER,FULLSEND_GCP_PROJECT_ID) in the per-repo shim workflow template by credential mode — OIDC repos no longer get confusing WIF secret references in their generated.github/workflows/fullsend.yamlCredentialModeto scaffoldRenderOptionsand thread it throughCollectPerRepoInstallFiles,BuildScaffoldFiles, andFetchRemoteScaffoldFULLSEND_GCP_PROJECT_IDoptional (required: false) inreusable-dispatch.ymlso OIDC-mode shims don't fail validationwif/oidc/token) in the mint administration guideChanges
internal/scaffold/render.go: AddCredentialModefield toRenderOptions; addstripWIFSecrets()that removes WIF secret lines when credential mode isoidcinternal/scaffold/installfiles.go: AddcredentialModeparameter toCollectPerRepoInstallFilesinternal/repos/install.go: Resolve credential mode viaresolveCredentialMode()inBuildScaffoldFilesbefore passing to scaffold layerinternal/repos/remote_scaffold.go: ThreadcredentialModethroughFetchRemoteScaffoldandfetchRemoteGitHubScaffoldinternal/repos/batch_install.go: Pass resolved credential mode toFetchRemoteScaffoldinternal/cli/github.go: Pass empty credential mode (backward compat for org-mode setup).github/workflows/reusable-dispatch.yml: ChangeFULLSEND_GCP_PROJECT_IDtorequired: falsedocs/guides/infrastructure/mint-administration.md: Add "Credential modes" sectionTesting
TestRenderPerRepoShimWIFMode— verifies WIF secrets present when credential mode iswifTestRenderPerRepoShimOIDCMode— verifies WIF secrets stripped and OTEL secrets preserved when credential mode isoidcTestRenderPerRepoShimDefaultCredMode— verifies empty credential mode defaults to including WIF secretsTestCollectPerRepoInstallFiles_OIDCModeandTestCollectPerRepoInstallFiles_WIFMode— end-to-end tests through install file collectionTestFetchRemoteScaffold_GitHubOIDCMode— remote scaffold path strips WIF secrets in OIDC modeOut of scope
Upgrade SHA-pinning behavior change moved to a separate issue.
Closes #6226