fix: prune secret versions labelled firebase-managed=functions - #11067
Open
IzaakGough wants to merge 7 commits into
Open
IzaakGough wants to merge 7 commits into
IzaakGough wants to merge 7 commits into
Conversation
pruneSecrets queried labels.firebase-managed=true, but secrets created since 13.6.1 are labelled firebase-managed=functions, so the query returned nothing and both functions:secrets:set and functions:secrets:prune reported success over an empty list. List without a label filter and select with isFunctionsManaged, which already accepts both values and excludes App Hosting secrets.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the secret pruning logic to support both firebase-managed=true and firebase-managed=functions labels by fetching all secrets and filtering them client-side. The reviewer suggests optimizing this by performing parallel, server-side filtered API requests for both labels to avoid performance issues in large GCP projects, and updating the unit tests accordingly.
Pruning stale versions lists and destroys secret versions, which the command did not declare. The calls were unreachable while pruneSecrets returned nothing, so a caller on a custom role would have failed with a raw 403 partway through instead of the upfront permission check.
IzaakGough
marked this pull request as ready for review
September 10, 2026 11:31
…ailures Disabled versions are a user's recoverable rollback, so pruneSecrets now queries state: ENABLED. destroySecretVersions runs every destroy and reports which ones failed instead of aborting on the first rejection.
Both commands abort if any region was unreachable, since a missing function would otherwise read as an unused version. prune counts every deployed function as a consumer and refuses to run non-interactively without --force. set only destroys the versions the updated functions were bound to, leaving the rest to prune. Both report failures per version.
cabljac
self-requested a review
September 24, 2026 16:07
This branch has not been deployed
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
2 participants
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.
Fixes #11066.
isFunctionsManaged.pruneSecretsfiltered onlabels.firebase-managed=true, butlabels()has writtenfunctionssince 13.6.1, so the query matched nothing and neither command destroyed anything. Listing without a label filter and selecting client-side keeps one definition of "managed" and still matches older secrets.functions:secrets:destroy.secrets:setonly destroys the versions the updated functions were bound to. Anything else unused is left tosecrets:prune, which lists what it will remove before asking.secrets:prunerequires--forcewhen non-interactive rather than destroying on the prompt default, and counts every deployed function as a consumer, not only Firebase-deployed ones.Live tested against a real project: prunes the orphaned
firebase-managed=functionsversionmainmisses, still matches the oldertruelabel, skips disabled versions, refuses--non-interactivewithout--force, and destroys only the version a redeployed function moved off. The unreachable-region guard, the every-function-counts change and partial destroy failures are unit tested only.