Skip to content

Declarative security: grantNewRoles has no retry for the SA propagation race, and reports the failure as a permissions error #10903

Description

@IzaakGough

Environment

firebase-tools: 15.25.1 and 15.26.0
Platform: macOS

Summary

Deploying a requiresRole() codebase fails about 2 times in 5 during enrollment. Looks like one more surface of the SA propagation race from #10859 — this one in grantNewRoles, which #10871 didn't cover. Separately, the error message points at permissions rather than the actual cause, which made it slow to track down.

Steps to reproduce

  1. Deploy a codebase declaring roles via requiresRole() (ours declares 5).
  2. Delete every function in the codebase, leaving requiresRole() in the source.
  3. firebase deploy --only functions, and repeat.

Step 2 matters: existingManagedSA comes from a deployed endpoint's serviceAccount field (prepare.ts:95), so with none deployed each deploy creates a new SA and re-enters the race. Steady-state redeploys short-circuit at prepare.ts:162 and never hit it.

Actual behavior

Error: The declarative security roles for this codebase have changed, but you do not have access to see what has changed. Please ask an IAM administrator to perform the next deploy.

We held roles/owner, and the CLI's own testIamPermissions check at prepare.ts:186 had already passed in the same run for resourcemanager.projects.setIamPolicy and iam.serviceAccounts.create. The real error only shows up in Cloud Audit Logs, on SetIamPolicy:

code 3 (INVALID_ARGUMENT)
"Exception calling IAM: Service account firebase-fn-2717501595@<project>.iam.gserviceaccount.com does not exist."

--debug doesn't surface it either.


Cause 1 — grantNewRoles doesn't retry the create-then-grant race

fabricator.ts:113 creates the SA, fabricator.ts:135 calls addServiceAccountRoles(..., true) 1-2s later. Resource Manager hasn't observed the account yet and rejects the member. The true is skipAccountLookup (resourceManager.ts:67), which also skips the IAM read that might otherwise have forced propagation.

That call isn't routed through the executor — bare await in a try, no predicate, no backoff. #10871 added isServiceAccount404 and wired it into the function create/update sites (fabricator.ts:476, :608, :759, :837, :873), but grantNewRoles runs earlier and hits Resource Manager instead of Cloud Functions.

One gotcha for the fix: isServiceAccount404 wouldn't match here anyway. It returns early unless parseErrorCode(err) === 404 (executor.ts:36), and parseErrorCode reads err.status, which responseToError sets to the HTTP code — 400 for INVALID_ARGUMENT. The code: 3 above is the gRPC canonical code in the audit log, not what the error object carries. So matching on 3, or reusing isServiceAccount404 unchanged, won't fire.

Suggested fix: route addServiceAccountRoles through the executor with isTransientError plus a predicate for HTTP 400 whose message contains "does not exist" and an SA address (or widen isServiceAccount404 to 400). A fixed sleep won't help — the create→grant gap is 1-2s on successes and failures alike, so there's no threshold to wait past.

Cause 2 — the error message names a cause it hasn't established

That string is thrown unconditionally from four places, none of which inspect the error:

  • fabricator.ts:153 — catch-all around addServiceAccountRoles
  • fabricator.ts:165testIamPermissions failure (the one case where it's accurate)
  • fabricator.ts:209 — catch-all around removeServiceAccountRoles
  • prepare.ts:195 — catch-all around getServiceAccountRoles

The three catch-alls attach the real error as { original: e } and never print it. Worth fixing on its own, since it'll misreport any future failure on these paths the same way.

Suggested fix: only claim a permissions cause on 403, otherwise rethrow or use a neutral message. Even just appending original.message would have made this self-diagnosing.


Related

Retrying is only a partial workaround: generateManagedServiceAccountName (iam.ts:307-313) picks a random suffix, so each retry creates a fresh SA and starts the race over instead of converging. That's the naming question from #10860, which #10871 closed without changing naming or discovery — happy to file that separately if useful.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions