fix(wfctl): bootstrap skips regeneration for write-only secret providers#417
Merged
intel352 merged 2 commits intoApr 20, 2026
Merged
Conversation
bootstrapSecrets previously called provider.Get() to check whether a secret already existed, then treated ErrUnsupported as "proceed to regenerate". GitHub Actions secrets are write-only (Get always returns ErrUnsupported), so every wfctl infra bootstrap run regenerated all secrets. For provider_credential generators that reach out to cloud APIs (e.g. digitalocean.spaces), this created a new upstream credential on every run and left the previous ones orphaned — one repo reported three duplicate DO Spaces access keys from three bootstrap invocations. Fall back to provider.List() when Get returns ErrUnsupported, and probe for <key>_access_key for provider_credential entries since that is the sub-key actually stored. If List is also unsupported, preserve the existing behaviour of regenerating (best-effort fallback). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes wfctl infra bootstrap repeatedly regenerating secrets when the configured secrets provider is write-only (e.g., GitHub Actions), preventing orphaned upstream credentials (notably for provider_credential sources like DigitalOcean Spaces).
Changes:
- Update
bootstrapSecretsto treatErrUnsupportedfromGetas “check viaList” (cached per bootstrap run) instead of “regenerate”. - Adjust existence probing for
provider_credentialgenerators to check the stored sub-key name (<key>_access_key). - Add unit tests covering write-only provider behavior and
Listfallback/unsupported cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cmd/wfctl/infra_bootstrap.go | Adds Get→List fallback with caching and probe-key logic for provider_credential secrets to avoid unnecessary regeneration. |
| cmd/wfctl/infra_bootstrap_secrets_test.go | Introduces tests for write-only provider behavior, including skip/regenerate paths and probe-key behavior. |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Three issues from review:
1. Partial provider_credential state — checking only <key>_access_key
skipped regeneration even when <key>_secret_key was missing (e.g.
manually deleted). Replace single-probe with a known sub-keys table
(providerCredentialSubKeys) and require ALL expected keys to be
present before skipping.
2. O(N*M) linear scan — lookupViaList now builds a map[string]struct{}
once from the List() result, so each subsequent probe is O(1).
3. Test coverage — add a package-level generateSecret hook so tests
can stub provider_credential generation without contacting DO.
New tests cover: both sub-keys present skips, partial state
regenerates, bare key without sub-keys still regenerates. The
existing test that conflated random_hex with provider_credential
semantics is replaced with dedicated cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
intel352
deleted the
fix/bootstrap-secret-existence-write-only-providers
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
`wfctl infra bootstrap` regenerated every secret on every run when the secrets provider was write-only (GitHub Actions). `GitHubSecretsProvider.Get` returns `ErrUnsupported`, and the bootstrap loop treated that as "unknown, regenerate" — which for `provider_credential` entries (e.g. `digitalocean.spaces`) created a new upstream credential every run. A consumer reported three orphaned `bmw-deploy-key` Spaces keys from three bootstrap invocations.
Fix
Test plan
🤖 Generated with Claude Code