From ed724c5728cca57bd16dc59a3aeb3c8236579402 Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Tue, 28 Jul 2026 16:52:57 +0100 Subject: [PATCH] test(functions): add failing tests for declarative security managed SA bugs Reproduces two bugs in the requireRoles managed service account flow: - createV2Function does not retry the 404 GCF returns when a freshly created service account has not propagated through IAM, and grantNewRoles never polls for SA visibility before function creation - discoverSecurityDetails only rediscovers the managed SA via deployed endpoints, so a failed first deploy orphans the SA (and its project-level role grants) and every retry creates a new one Tests intentionally fail until the bugs are fixed. --- src/deploy/functions/prepare.spec.ts | 43 +++++++++++++++++- .../functions/release/fabricator.spec.ts | 45 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/deploy/functions/prepare.spec.ts b/src/deploy/functions/prepare.spec.ts index 06b845d87a1..bd10275042c 100644 --- a/src/deploy/functions/prepare.spec.ts +++ b/src/deploy/functions/prepare.spec.ts @@ -1200,12 +1200,15 @@ describe("prepare", () => { describe("discoverSecurityDetails", () => { let testIamPermissionsStub: sinon.SinonStub; + let generateManagedSANameStub: sinon.SinonStub; beforeEach(() => { testIamPermissionsStub = sinon .stub(iam, "testIamPermissions") .resolves({ passed: true } as any); - sinon.stub(iam, "generateManagedServiceAccountName").resolves("firebase-fn-123"); + generateManagedSANameStub = sinon + .stub(iam, "generateManagedServiceAccountName") + .resolves("firebase-fn-123"); sinon.stub(resourcemanager, "getServiceAccountRoles").resolves([]); }); @@ -1229,6 +1232,44 @@ describe("prepare", () => { expect(e.labels?.["firebase-declarative-security-etag"]).to.equal(result.newEtag); }); + it("reuses an existing managed service account in the project when no deployed functions reference it (BUG: currently generates a new one)", async () => { + // Scenario: a previous deploy created the managed SA and granted it roles, but + // function creation failed, so `have` is empty. The project still contains + // the managed SA, discoverable via IAM lookup. + const preexistingSA = "firebase-fn-1234567890@project.iam.gserviceaccount.com"; + const getServiceAccountStub = sinon.stub(iam, "getServiceAccount").resolves({ + name: `projects/project/serviceAccounts/${preexistingSA}`, + projectId: "project", + uniqueId: "12345", + email: preexistingSA, + displayName: "Firebase Functions managed service account", + etag: "etag", + description: "", + oauth2ClientId: "", + disabled: false, + }); + + const e: backend.Endpoint = { + ...ENDPOINT, + }; + const want = backend.of(e); + want.requiredRoles = ["roles/viewer"]; + const have = backend.empty(); + + const result = await prepare.discoverSecurityDetails("default", want, have, "project"); + + // Desired behavior: reuse the SA that already exists in the project instead + // of generating a brand new random name (which orphans the previous SA and + // its project-level role grants on every failed deploy). + expect( + getServiceAccountStub.called || !generateManagedSANameStub.called, + "expected discoverSecurityDetails to look up existing managed service accounts " + + "in the project instead of unconditionally generating a new random SA name", + ).to.be.true; + expect(result.managedSA).to.equal(preexistingSA); + expect(e.serviceAccount).to.equal(preexistingSA); + }); + it("should reset endpoints to default service account when unenrolling (opting out)", async () => { const e: backend.Endpoint = { ...ENDPOINT, diff --git a/src/deploy/functions/release/fabricator.spec.ts b/src/deploy/functions/release/fabricator.spec.ts index bcb17f49f66..5adfbf5f210 100644 --- a/src/deploy/functions/release/fabricator.spec.ts +++ b/src/deploy/functions/release/fabricator.spec.ts @@ -670,6 +670,31 @@ describe("Fabricator", () => { expect(gcfv2.deleteFunction).to.have.been.called; }); + it("retries function creation when a freshly created service account has not yet propagated (BUG: currently fails immediately)", async () => { + // GCF returns HTTP 404 when the deploy references a service account that + // was created moments ago and has not propagated through IAM yet. + const saNotFoundErr = new Error( + "Service account projects/-/serviceAccounts/firebase-fn-123@test-project.iam.gserviceaccount.com was not found. Please verify that the caller has iam.serviceAccounts.actAs permission on the service account.", + ); + (saNotFoundErr as any).status = 404; + gcfv2.createFunction.onFirstCall().rejects(saNotFoundErr); + gcfv2.createFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.setInvokerCreate.resolves(); + + // Use the same executor type as a real deploy (release/index.ts) so that + // retry-code based mitigations are exercised too, with fast backoff. + const queueFab = new fabricator.Fabricator({ + ...ctorArgs, + functionExecutor: new executor.QueueExecutor({ retries: 3, backoff: 1, maxBackoff: 10 }), + }); + + const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" }); + await queueFab.createV2Function(ep, new scraper.SourceTokenScraper()); + + expect(gcfv2.createFunction).to.have.been.calledTwice; + }); + it("throws on set invoker failure", async () => { gcfv2.createFunction.resolves({ name: "op", done: false }); poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); @@ -2071,6 +2096,26 @@ describe("Fabricator", () => { ); }); + it("waits for a newly created service account to be visible before returning from grantNewRoles (BUG: no propagation check)", async () => { + // IAM service account creation is eventually consistent. grantNewRoles + // should poll getServiceAccount until the new SA is visible before the + // deploy proceeds to create functions that actAs it. + const getServiceAccountStub = sinon.stub(iam, "getServiceAccount").resolves({ + name: "projects/test-project/serviceAccounts/firebase-fn-123@my-proj.iam.gserviceaccount.com", + email: "firebase-fn-123@my-proj.iam.gserviceaccount.com", + } as any); + const plan: planner.CodebasePlan = { + regionalChangesets: {}, + serviceAccountToCreate: "firebase-fn-123@my-proj.iam.gserviceaccount.com", + managedServiceAccount: "firebase-fn-123@my-proj.iam.gserviceaccount.com", + }; + + await fab.grantNewRoles(plan, "default"); + + expect(createServiceAccountStub).to.have.been.called; + expect(getServiceAccountStub).to.have.been.called; + }); + it("should remove roles or delete SA in removeOldRoles", async () => { const plan: planner.CodebasePlan = { regionalChangesets: {},