From e39e3727936b986a5ff91d58c2ef4f84c9ab74ad Mon Sep 17 00:00:00 2001 From: Aryan Falahatpisheh Date: Fri, 9 Oct 2026 13:38:07 -0700 Subject: [PATCH 1/6] Support build-time env vars and secrets for Cloud Run Reads the firebase.google.com/buildConfig.environmentVariablesAndSecrets annotation on the service. Plain values go to Cloud Build and local builds; secret references only work in local builds, which read them with the deployer's credentials. --- src/deploy/run/args.ts | 3 ++ src/deploy/run/buildEnv.spec.ts | 88 ++++++++++++++++++++++++++++++++ src/deploy/run/buildEnv.ts | 89 +++++++++++++++++++++++++++++++++ src/deploy/run/deploy.spec.ts | 21 +++++++- src/deploy/run/deploy.ts | 16 +++++- src/deploy/run/prepare.spec.ts | 28 +++++++++++ src/deploy/run/prepare.ts | 27 +++++++++- 7 files changed, 267 insertions(+), 5 deletions(-) create mode 100644 src/deploy/run/buildEnv.spec.ts create mode 100644 src/deploy/run/buildEnv.ts diff --git a/src/deploy/run/args.ts b/src/deploy/run/args.ts index 6848b6acc83..c98333634f8 100644 --- a/src/deploy/run/args.ts +++ b/src/deploy/run/args.ts @@ -1,5 +1,6 @@ import { RunSingle } from "../../firebaseConfig"; import * as runv2 from "../../gcp/runv2"; +import { BuildEnv } from "./buildEnv"; /** A service being deployed. prepare fills in the first fields, and deploy sets the rest. */ export interface ServiceDeploy { @@ -7,6 +8,8 @@ export interface ServiceDeploy { /** The service as it is in Cloud Run before this deploy, or undefined if it doesn't exist yet. */ existing?: runv2.Service; baseImage?: string; + /** Build-time environment. Prepare only allows secrets in it for local builds. */ + buildEnv?: BuildEnv; /** The local build's output. */ localBuild?: { scratchDir: string; diff --git a/src/deploy/run/buildEnv.spec.ts b/src/deploy/run/buildEnv.spec.ts new file mode 100644 index 00000000000..00c50090b8f --- /dev/null +++ b/src/deploy/run/buildEnv.spec.ts @@ -0,0 +1,88 @@ +import { expect } from "chai"; +import * as runv2 from "../../gcp/runv2"; +import { BUILD_ENV_ANNOTATION, getBuildEnv, secretNames, toLocalBuildEnv } from "./buildEnv"; + +describe("run buildEnv", () => { + const withAnnotation = (value: string): runv2.Service => + ({ + name: "projects/p/locations/us-central1/services/s", + annotations: { other: "x", [BUILD_ENV_ANNOTATION]: value }, + }) as unknown as runv2.Service; + + describe("getBuildEnv", () => { + it("returns {} without a service or annotation", () => { + expect(getBuildEnv(undefined)).to.deep.equal({}); + expect( + getBuildEnv({ annotations: { other: "x" } } as unknown as runv2.Service), + ).to.deep.equal({}); + }); + + it("parses a plain map like Cloud Run's buildConfig.environmentVariables", () => { + expect(getBuildEnv(withAnnotation('{"A":"1","B":""}'))).to.deep.equal({ A: "1", B: "" }); + }); + + it("parses secret references", () => { + const env = { + A: "1", + TOKEN: { secret: "npm-token", version: "3" }, + LATEST: { secret: "s" }, + OTHER: { secret: "projects/q/secrets/t", version: "latest" }, + }; + expect(getBuildEnv(withAnnotation(JSON.stringify(env)))).to.deep.equal(env); + }); + + it("rejects invalid JSON and non-objects", () => { + expect(() => getBuildEnv(withAnnotation("{"))).to.throw( + `Invalid ${BUILD_ENV_ANNOTATION} annotation on projects/p/locations/us-central1/services/s: it isn't valid JSON.`, + ); + for (const value of ["[]", "null", '"x"', "1"]) { + expect(() => getBuildEnv(withAnnotation(value))).to.throw("it must be a JSON object."); + } + }); + + it("rejects invalid values", () => { + const invalid = [ + 1, + null, + ["x"], + {}, + { secret: 1 }, + { secret: "a/b" }, + { secret: "a@1" }, + { secret: "projects/q/secrets/t/versions/1" }, + { secret: "s", version: 1 }, + { secret: "s", version: "one" }, + { secret: "s", extra: "x" }, + ]; + for (const value of invalid) { + expect(() => getBuildEnv(withAnnotation(JSON.stringify({ A: value })))).to.throw( + 'A must be a string or {"secret": "", "version": ""}.', + ); + } + }); + }); + + it("lists secret names", () => { + expect( + secretNames({ A: "1", B: { secret: "s" }, C: { secret: "t", version: "2" } }), + ).to.deep.equal(["B", "C"]); + }); + + it("converts to local build env, available only to the build", () => { + expect( + toLocalBuildEnv({ + A: "1", + B: { secret: "s" }, + C: { secret: "t", version: "2" }, + D: { secret: "projects/q/secrets/u" }, + E: { secret: "projects/q/secrets/v", version: "4" }, + }), + ).to.deep.equal({ + A: { value: "1", availability: ["BUILD"] }, + B: { secret: "s@latest", availability: ["BUILD"] }, + C: { secret: "t@2", availability: ["BUILD"] }, + D: { secret: "projects/q/secrets/u/versions/latest", availability: ["BUILD"] }, + E: { secret: "projects/q/secrets/v/versions/4", availability: ["BUILD"] }, + }); + }); +}); diff --git a/src/deploy/run/buildEnv.ts b/src/deploy/run/buildEnv.ts new file mode 100644 index 00000000000..5ba9d8cb263 --- /dev/null +++ b/src/deploy/run/buildEnv.ts @@ -0,0 +1,89 @@ +import { FirebaseError } from "../../error"; +import * as runv2 from "../../gcp/runv2"; +import { EnvMap } from "../../apphosting/yaml"; + +/** + * Prototype: build-time environment variables and secret references, stored on the Cloud Run + * service. It extends Cloud Run's own buildConfig.environmentVariables (a map of plain strings): + * the same map parses unchanged, and a value can also be a secret reference in the shape of + * Cloud Run's runtime secretKeyRef. It holds references only, never secret values. + * Example: {"API_URL": "https://example.com", "NPM_TOKEN": {"secret": "npm-token", "version": "3"}} + */ +export const BUILD_ENV_ANNOTATION = + "firebase.google.com/buildConfig.environmentVariablesAndSecrets"; + +/** A Secret Manager secret, as a name in the service's project or a full resource name. */ +export interface SecretRef { + secret: string; + /** A version number or "latest". Defaults to "latest". */ + version?: string; +} + +export type BuildEnv = Record; + +const SECRET_NAME = /^(?:projects\/[^/]+\/secrets\/)?[^/@]+$/; +const SECRET_VERSION = /^(?:latest|\d+)$/; + +/** Reads and validates the service's build environment. Returns {} if it has none. */ +export function getBuildEnv(service: runv2.Service | undefined): BuildEnv { + const raw = service?.annotations?.[BUILD_ENV_ANNOTATION]; + if (!service || !raw) { + return {}; + } + const invalid = (reason: string): FirebaseError => + new FirebaseError(`Invalid ${BUILD_ENV_ANNOTATION} annotation on ${service.name}: ${reason}`); + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + throw invalid("it isn't valid JSON."); + } + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + throw invalid("it must be a JSON object."); + } + for (const [name, value] of Object.entries(parsed)) { + if (typeof value === "string") { + continue; + } + const ref = value as Partial | null; + const valid = + typeof ref === "object" && + ref !== null && + !Array.isArray(ref) && + Object.keys(ref).every((k) => k === "secret" || k === "version") && + typeof ref.secret === "string" && + SECRET_NAME.test(ref.secret) && + (ref.version === undefined || + (typeof ref.version === "string" && SECRET_VERSION.test(ref.version))); + if (!valid) { + throw invalid( + `${name} must be a string or {"secret": "", "version": ""}.`, + ); + } + } + return parsed as BuildEnv; +} + +/** Names of the variables that are secret references. */ +export function secretNames(env: BuildEnv): string[] { + return Object.entries(env) + .filter(([, value]) => typeof value !== "string") + .map(([name]) => name); +} + +/** Converts the build environment to the form local builds take; they resolve the secrets. */ +export function toLocalBuildEnv(env: BuildEnv): EnvMap { + const result: EnvMap = {}; + for (const [name, value] of Object.entries(env)) { + if (typeof value === "string") { + result[name] = { value, availability: ["BUILD"] }; + } else { + const version = value.version || "latest"; + const secret = value.secret.startsWith("projects/") + ? `${value.secret}/versions/${version}` + : `${value.secret}@${version}`; + result[name] = { secret, availability: ["BUILD"] }; + } + } + return result; +} diff --git a/src/deploy/run/deploy.spec.ts b/src/deploy/run/deploy.spec.ts index d55ba00ea89..1dfa6e36cc1 100644 --- a/src/deploy/run/deploy.spec.ts +++ b/src/deploy/run/deploy.spec.ts @@ -84,6 +84,13 @@ describe("run deploy", () => { expect(buildpackBuild).not.to.have.property("enableAutomaticUpdates"); }); + it("passes the service's build env to Cloud Build", async () => { + await deployOne({ config: web, buildEnv: { API_URL: "https://api.example.com" } }); + + const { environmentVariables } = submitBuildStub.firstCall.args[2].buildpackBuild; + expect(environmentVariables).to.include({ API_URL: "https://api.example.com" }); + }); + it("rolls out a new revision of an existing service with all traffic, keeping its tags", async () => { const existing = { name: "projects/my-project/locations/us-central1/services/web", @@ -143,7 +150,7 @@ describe("run deploy", () => { { runtime: { value: "nodejs22" } }, "/project/apps/web", ); - expect(localBuildStub).to.have.been.calledWith( + expect(localBuildStub).to.have.been.calledWithMatch( "my-project", scratchDir, {}, @@ -184,6 +191,18 @@ describe("run deploy", () => { ]); }); + it("passes the build env, secrets included, to the local build", async () => { + const buildEnv = { API_URL: "https://api.example.com", TOKEN: { secret: "t", version: "2" } }; + + await deployOne({ config: localWeb, baseImage: NODEJS22, buildEnv }); + + expect(localBuildStub.firstCall.args[2]).to.deep.equal({ + API_URL: { value: "https://api.example.com", availability: ["BUILD"] }, + TOKEN: { secret: "t@2", availability: ["BUILD"] }, + }); + expect(localBuildStub.firstCall.args[3]).to.include({ allowLocalBuildSecrets: true }); + }); + it("cleans up and deploys nothing if the local build fails", async () => { localBuildStub.rejects(new Error("boom")); diff --git a/src/deploy/run/deploy.ts b/src/deploy/run/deploy.ts index 65f87d61a99..18da9d332b7 100644 --- a/src/deploy/run/deploy.ts +++ b/src/deploy/run/deploy.ts @@ -16,6 +16,7 @@ import { cloneDeep, logLabeledBullet } from "../../utils"; import { prepareLocalBuildScratchDirectory } from "../apphosting/prepare"; import { createLocalBuildTarArchive, createSourceDeployArchive } from "../apphosting/util"; import { Context, Payload, ServiceDeploy } from "./args"; +import { secretNames, toLocalBuildEnv } from "./buildEnv"; const DEPLOY_MESSAGE_ANNOTATION = "firebase.google.com/deploy-message"; /** Rolling out a revision can take longer than the operation poller's default timeout. */ @@ -69,12 +70,21 @@ async function buildLocally( path.join(options.config.projectDir, cfg.rootDir), ); logLabeledBullet("run", `Starting local build for service ${serviceId} in ${region}`); + const secrets = secretNames(svc.buildEnv || {}); + if (secrets.length) { + logLabeledBullet( + "run", + `Reading build secrets ${secrets.join(", ")} with your credentials. Their values may be ` + + `included in the build output.`, + ); + } await prepareLocalBuildScratchDirectory(options.config.projectDir, scratchDir, cfg); const { outputFiles, buildConfig } = await localBuild( context.projectId, scratchDir, - {}, - { rootDir: config.rootDir }, + toLocalBuildEnv(svc.buildEnv || {}), + // Unlike App Hosting, Cloud Run doesn't ask users to confirm build secrets in local builds. + { rootDir: config.rootDir, allowLocalBuildSecrets: true }, ); // Keep the env vars that the app needs at runtime, not just during the build. const env = (buildConfig.env || []) @@ -144,6 +154,8 @@ async function buildImage( // Only images built for a base image can have their base image updated automatically. ...(svc.baseImage && { baseImage: svc.baseImage, enableAutomaticUpdates: true }), environmentVariables: { + // Prepare rejects build secrets for builds on Cloud Build, so these are all plain values. + ...(svc.buildEnv as Record | undefined), // Use App Hosting's buildpacks, which need to know where to write the output bundle. X_GOOGLE_TARGET_PLATFORM: "fah", FIREBASE_OUTPUT_BUNDLE_DIR: "/workspace/.apphosting", diff --git a/src/deploy/run/prepare.spec.ts b/src/deploy/run/prepare.spec.ts index e2b853851d4..e577b1039df 100644 --- a/src/deploy/run/prepare.spec.ts +++ b/src/deploy/run/prepare.spec.ts @@ -3,6 +3,7 @@ import * as sinon from "sinon"; import * as runv2 from "../../gcp/runv2"; import { Options } from "../../options"; import { Context, Payload, ServiceDeploy } from "./args"; +import { BUILD_ENV_ANNOTATION } from "./buildEnv"; import { prepare } from "./prepare"; import * as prereqs from "./prereqs"; @@ -91,6 +92,33 @@ describe("run prepare", () => { }); }); + describe("build env", () => { + const withBuildEnv = (env: Record): runv2.Service => + ({ + ...liveWeb, + annotations: { [BUILD_ENV_ANNOTATION]: JSON.stringify(env) }, + }) as unknown as runv2.Service; + + it("reads the build env from the service's annotation", async () => { + getServiceStub.resolves(withBuildEnv({ API_URL: "https://api.example.com" })); + const [svc] = await prepareRun(web); + expect(svc.buildEnv).to.deep.equal({ API_URL: "https://api.example.com" }); + }); + + it("rejects build secrets on Cloud Build", async () => { + getServiceStub.resolves(withBuildEnv({ TOKEN: { secret: "t" } })); + await expect(prepareRun(web)).to.be.rejectedWith( + /Service web in us-central1 has build secrets \(TOKEN\).*"localBuild": true/, + ); + }); + + it("keeps build secrets for local builds", async () => { + getServiceStub.resolves(withBuildEnv({ TOKEN: { secret: "t", version: "2" } })); + const [svc] = await prepareRun(localWeb); + expect(svc.buildEnv).to.deep.equal({ TOKEN: { secret: "t", version: "2" } }); + }); + }); + it("only prepares the services that --only selects", async () => { const webEurope = { serviceId: "web", region: "europe-west1" }; const services = await prepareRun([web, webEurope], { only: "run:web:europe-west1" }); diff --git a/src/deploy/run/prepare.ts b/src/deploy/run/prepare.ts index 7f774dc33c0..293729dbab6 100644 --- a/src/deploy/run/prepare.ts +++ b/src/deploy/run/prepare.ts @@ -1,12 +1,14 @@ import { FirebaseError } from "../../error"; import { RunSingle } from "../../firebaseConfig"; import { Options } from "../../options"; +import { logLabeledBullet } from "../../utils"; import { Context, Payload, ServiceDeploy } from "./args"; +import { BUILD_ENV_ANNOTATION, getBuildEnv, secretNames } from "./buildEnv"; import { prereqs } from "./prereqs"; import { fullServiceName, getExistingService, getServiceConfigs } from "./util"; /** - * Reads each service's current state from Cloud Run and resolves its base image. + * Reads each service's current state from Cloud Run and resolves its base image and build env. */ export async function prepare(context: Context, options: Options, payload: Payload): Promise { const configs = getServiceConfigs(options); @@ -37,5 +39,26 @@ async function prepareService(context: Context, config: RunSingle): Promise Date: Fri, 9 Oct 2026 14:06:29 -0700 Subject: [PATCH 2/6] Make the build env tests easier to read Realistic names (API_URL, NPM_TOKEN, npm-token) and shorter fixtures. --- src/deploy/run/buildEnv.spec.ts | 85 +++++++++-------- src/deploy/run/deploy.spec.ts | 158 +++++++++++++++++--------------- src/deploy/run/prepare.spec.ts | 41 ++++----- src/init/features/run.spec.ts | 125 ++++++++++--------------- 4 files changed, 195 insertions(+), 214 deletions(-) diff --git a/src/deploy/run/buildEnv.spec.ts b/src/deploy/run/buildEnv.spec.ts index 00c50090b8f..e403ffde742 100644 --- a/src/deploy/run/buildEnv.spec.ts +++ b/src/deploy/run/buildEnv.spec.ts @@ -3,39 +3,39 @@ import * as runv2 from "../../gcp/runv2"; import { BUILD_ENV_ANNOTATION, getBuildEnv, secretNames, toLocalBuildEnv } from "./buildEnv"; describe("run buildEnv", () => { - const withAnnotation = (value: string): runv2.Service => - ({ - name: "projects/p/locations/us-central1/services/s", - annotations: { other: "x", [BUILD_ENV_ANNOTATION]: value }, - }) as unknown as runv2.Service; + const web = { name: "projects/my-project/locations/us-central1/services/web" } as runv2.Service; + /** The web service, with this build env annotation. */ + const withAnnotation = (value: string): runv2.Service => ({ + ...web, + annotations: { [BUILD_ENV_ANNOTATION]: value }, + }); describe("getBuildEnv", () => { it("returns {} without a service or annotation", () => { expect(getBuildEnv(undefined)).to.deep.equal({}); - expect( - getBuildEnv({ annotations: { other: "x" } } as unknown as runv2.Service), - ).to.deep.equal({}); + expect(getBuildEnv({ ...web, annotations: { team: "frontend" } })).to.deep.equal({}); }); it("parses a plain map like Cloud Run's buildConfig.environmentVariables", () => { - expect(getBuildEnv(withAnnotation('{"A":"1","B":""}'))).to.deep.equal({ A: "1", B: "" }); + const env = { API_URL: "https://api.example.com", BASE_PATH: "" }; + expect(getBuildEnv(withAnnotation(JSON.stringify(env)))).to.deep.equal(env); }); it("parses secret references", () => { const env = { - A: "1", - TOKEN: { secret: "npm-token", version: "3" }, - LATEST: { secret: "s" }, - OTHER: { secret: "projects/q/secrets/t", version: "latest" }, + API_URL: "https://api.example.com", + NPM_TOKEN: { secret: "npm-token" }, + SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token", version: "2" }, + GITHUB_TOKEN: { secret: "projects/shared-secrets/secrets/github-token", version: "latest" }, }; expect(getBuildEnv(withAnnotation(JSON.stringify(env)))).to.deep.equal(env); }); it("rejects invalid JSON and non-objects", () => { - expect(() => getBuildEnv(withAnnotation("{"))).to.throw( - `Invalid ${BUILD_ENV_ANNOTATION} annotation on projects/p/locations/us-central1/services/s: it isn't valid JSON.`, + expect(() => getBuildEnv(withAnnotation("API_URL=https://api.example.com"))).to.throw( + `Invalid ${BUILD_ENV_ANNOTATION} annotation on projects/my-project/locations/us-central1/services/web: it isn't valid JSON.`, ); - for (const value of ["[]", "null", '"x"', "1"]) { + for (const value of ["[]", "null", '"https://api.example.com"', "1"]) { expect(() => getBuildEnv(withAnnotation(value))).to.throw("it must be a JSON object."); } }); @@ -44,45 +44,54 @@ describe("run buildEnv", () => { const invalid = [ 1, null, - ["x"], + ["npm-token"], {}, { secret: 1 }, - { secret: "a/b" }, - { secret: "a@1" }, - { secret: "projects/q/secrets/t/versions/1" }, - { secret: "s", version: 1 }, - { secret: "s", version: "one" }, - { secret: "s", extra: "x" }, + { secret: "secrets/npm-token" }, + { secret: "npm-token@3" }, + { secret: "projects/shared-secrets/secrets/npm-token/versions/3" }, + { secret: "npm-token", version: 3 }, + { secret: "npm-token", version: "v3" }, + { secret: "npm-token", project: "shared-secrets" }, ]; for (const value of invalid) { - expect(() => getBuildEnv(withAnnotation(JSON.stringify({ A: value })))).to.throw( - 'A must be a string or {"secret": "", "version": ""}.', + expect(() => getBuildEnv(withAnnotation(JSON.stringify({ NPM_TOKEN: value })))).to.throw( + 'NPM_TOKEN must be a string or {"secret": "", "version": ""}.', ); } }); }); it("lists secret names", () => { - expect( - secretNames({ A: "1", B: { secret: "s" }, C: { secret: "t", version: "2" } }), - ).to.deep.equal(["B", "C"]); + const env = { + API_URL: "https://api.example.com", + NPM_TOKEN: { secret: "npm-token" }, + SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token", version: "2" }, + }; + expect(secretNames(env)).to.deep.equal(["NPM_TOKEN", "SENTRY_AUTH_TOKEN"]); }); it("converts to local build env, available only to the build", () => { expect( toLocalBuildEnv({ - A: "1", - B: { secret: "s" }, - C: { secret: "t", version: "2" }, - D: { secret: "projects/q/secrets/u" }, - E: { secret: "projects/q/secrets/v", version: "4" }, + API_URL: "https://api.example.com", + NPM_TOKEN: { secret: "npm-token" }, + SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token", version: "2" }, + GITHUB_TOKEN: { secret: "projects/shared-secrets/secrets/github-token" }, + CMS_TOKEN: { secret: "projects/shared-secrets/secrets/cms-token", version: "4" }, }), ).to.deep.equal({ - A: { value: "1", availability: ["BUILD"] }, - B: { secret: "s@latest", availability: ["BUILD"] }, - C: { secret: "t@2", availability: ["BUILD"] }, - D: { secret: "projects/q/secrets/u/versions/latest", availability: ["BUILD"] }, - E: { secret: "projects/q/secrets/v/versions/4", availability: ["BUILD"] }, + API_URL: { value: "https://api.example.com", availability: ["BUILD"] }, + NPM_TOKEN: { secret: "npm-token@latest", availability: ["BUILD"] }, + SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token@2", availability: ["BUILD"] }, + GITHUB_TOKEN: { + secret: "projects/shared-secrets/secrets/github-token/versions/latest", + availability: ["BUILD"], + }, + CMS_TOKEN: { + secret: "projects/shared-secrets/secrets/cms-token/versions/4", + availability: ["BUILD"], + }, }); }); }); diff --git a/src/deploy/run/deploy.spec.ts b/src/deploy/run/deploy.spec.ts index 1dfa6e36cc1..17ce8ce12b8 100644 --- a/src/deploy/run/deploy.spec.ts +++ b/src/deploy/run/deploy.spec.ts @@ -1,6 +1,5 @@ import { expect } from "chai"; import * as fs from "fs"; -import * as path from "path"; import * as sinon from "sinon"; import * as localbuilds from "../../apphosting/localbuilds"; import * as artifactregistry from "../../gcp/artifactregistry"; @@ -21,6 +20,8 @@ describe("run deploy", () => { const web = { serviceId: "web", region: "us-central1", rootDir: "apps/web" }; // Date.now() is stubbed to return 42, which tags the image. const image = "us-central1-docker.pkg.dev/my-project/cloud-run-source-deploy/web:42"; + const scratchDir = "/tmp/run-local-build-web-abc123"; + const builtApp = { bucket: "source-bucket", object: "web.tar.gz" }; let createArchiveStub: sinon.SinonStub; let submitBuildStub: sinon.SinonStub; let createServiceStub: sinon.SinonStub; @@ -33,13 +34,9 @@ describe("run deploy", () => { .stub(apphostingUtil, "createSourceDeployArchive") .resolves("/tmp/web.zip"); sinon.stub(fs, "createReadStream").returns("stream" as unknown as fs.ReadStream); - sinon.stub(gcs, "uploadObject").callsFake((src) => - Promise.resolve({ - bucket: "source-bucket", - object: path.basename(src.file), - generation: "1", - }), - ); + sinon + .stub(gcs, "uploadObject") + .resolves({ bucket: "source-bucket", object: "web.zip", generation: "1" }); sinon.stub(artifactregistry, "ensureDockerRepository").resolves(); submitBuildStub = sinon.stub(runv2, "submitBuild").resolves(); const deployed = { uri: "https://web.run.app" } as runv2.Service; @@ -124,28 +121,37 @@ describe("run deploy", () => { describe("local builds", () => { const localWeb = { ...web, localBuild: true }; + let rmSyncStub: sinon.SinonStub; let validateNodeStub: sinon.SinonStub; let localBuildStub: sinon.SinonStub; let tarArchiveStub: sinon.SinonStub; - let mkdtempSpy: sinon.SinonSpy; beforeEach(() => { - sinon.stub(apphostingPrepare, "prepareLocalBuildScratchDirectory").resolves(); + sinon.stub(fs, "mkdtempSync").returns(scratchDir); + rmSyncStub = sinon.stub(fs, "rmSync"); validateNodeStub = sinon.stub(localbuilds, "validateLocalBuildNodeVersion"); + sinon.stub(apphostingPrepare, "prepareLocalBuildScratchDirectory").resolves(); localBuildStub = sinon.stub(localbuilds, "localBuild").resolves({ outputFiles: [".next"], - buildConfig: { runCommand: "node server.js" }, + buildConfig: { runCommand: "npm run start" }, }); tarArchiveStub = sinon .stub(apphostingUtil, "createLocalBuildTarArchive") .resolves("/tmp/web.tar.gz"); - mkdtempSpy = sinon.spy(fs, "mkdtempSync"); + (gcs.uploadObject as sinon.SinonStub) + .withArgs(sinon.match({ file: "/tmp/web.tar.gz" })) + .resolves(builtApp); }); - it("builds on this machine and uploads the output, without Cloud Build", async () => { + /** Deploys web with "localBuild": true and returns the container that it rolled out. */ + async function deployLocalBuild(): Promise { await deployOne({ config: localWeb, baseImage: NODEJS22 }); + return createServiceStub.firstCall.args[3].template.containers[0]; + } + + it("builds on this machine, in a scratch folder that it deletes afterwards", async () => { + await deployLocalBuild(); - const scratchDir = mkdtempSpy.firstCall.returnValue as string; expect(validateNodeStub).to.have.been.calledWith( { runtime: { value: "nodejs22" } }, "/project/apps/web", @@ -154,71 +160,69 @@ describe("run deploy", () => { "my-project", scratchDir, {}, - { - rootDir: "apps/web", - }, + { rootDir: "apps/web" }, ); - expect(tarArchiveStub).to.have.been.calledWithMatch({ backendId: "web" }, scratchDir, [ - ".next", - ]); + expect(rmSyncStub).to.have.been.calledWith(scratchDir); + }); + + it("uploads the build's output and runs it as is, without Cloud Build", async () => { + const container = await deployLocalBuild(); + + expect(tarArchiveStub).to.have.been.calledWithMatch(sinon.match.any, scratchDir, [".next"]); expect(submitBuildStub).not.to.have.been.called; - expect(createServiceStub.firstCall.args[3].template.containers[0]).to.deep.equal({ - name: "web", + expect(container).to.deep.include({ image: "scratch", - sourceCode: { cloudStorageSource: { bucket: "source-bucket", object: "web.tar.gz" } }, - command: ["node", "server.js"], - baseImageUri: NODEJS22, + sourceCode: { cloudStorageSource: builtApp }, + command: ["npm", "run", "start"], }); - expect(fs.existsSync(scratchDir)).to.be.false; }); it("keeps the env vars the app needs at runtime", async () => { localBuildStub.resolves({ outputFiles: [".next"], buildConfig: { - runCommand: "node server.js", env: [ { variable: "NODE_ENV", value: "production", availability: ["RUNTIME"] }, - { variable: "BUILD_ONLY", value: "x", availability: ["BUILD"] }, + { variable: "NEXT_TELEMETRY_DISABLED", value: "1", availability: ["BUILD"] }, ], }, }); - await deployOne({ config: localWeb, baseImage: NODEJS22 }); + const container = await deployLocalBuild(); - expect(createServiceStub.firstCall.args[3].template.containers[0].env).to.deep.equal([ - { name: "NODE_ENV", value: "production" }, - ]); + expect(container.env).to.deep.equal([{ name: "NODE_ENV", value: "production" }]); }); it("passes the build env, secrets included, to the local build", async () => { - const buildEnv = { API_URL: "https://api.example.com", TOKEN: { secret: "t", version: "2" } }; + const buildEnv = { + API_URL: "https://api.example.com", + NPM_TOKEN: { secret: "npm-token", version: "2" }, + }; await deployOne({ config: localWeb, baseImage: NODEJS22, buildEnv }); - expect(localBuildStub.firstCall.args[2]).to.deep.equal({ - API_URL: { value: "https://api.example.com", availability: ["BUILD"] }, - TOKEN: { secret: "t@2", availability: ["BUILD"] }, - }); - expect(localBuildStub.firstCall.args[3]).to.include({ allowLocalBuildSecrets: true }); + expect(localBuildStub).to.have.been.calledWithMatch( + "my-project", + scratchDir, + { API_URL: { value: "https://api.example.com" }, NPM_TOKEN: { secret: "npm-token@2" } }, + { allowLocalBuildSecrets: true }, + ); }); it("cleans up and deploys nothing if the local build fails", async () => { - localBuildStub.rejects(new Error("boom")); + localBuildStub.rejects(new Error("next build failed")); - await expect(deployOne({ config: localWeb, baseImage: NODEJS22 })).to.be.rejectedWith("boom"); + await expect(deployLocalBuild()).to.be.rejectedWith("next build failed"); - expect(fs.existsSync(mkdtempSpy.firstCall.returnValue as string)).to.be.false; + expect(rmSyncStub).to.have.been.calledWith(scratchDir); expect(createServiceStub).not.to.have.been.called; }); it("reports the local build error, not the cleanup error, when both fail", async () => { - localBuildStub.rejects(new Error("boom")); - sinon.stub(fs, "rmSync").throws(new Error("EBUSY")); + localBuildStub.rejects(new Error("next build failed")); + rmSyncStub.throws(new Error("EBUSY: resource busy or locked")); - await expect(deployOne({ config: localWeb, baseImage: NODEJS22 })).to.be.rejectedWith("boom"); - - fs.rmdirSync(mkdtempSpy.firstCall.returnValue as string); + await expect(deployLocalBuild()).to.be.rejectedWith("next build failed"); }); }); @@ -239,7 +243,13 @@ describe("run deploy", () => { ], }, } as unknown as runv2.Service; - const builtApp = { bucket: "source-bucket", object: "web.tar.gz" }; + + /** The live web service, running a container with these fields. */ + function webRunning(container: Partial): runv2.Service { + return { + template: { containers: [{ name: "web", image: "old-image", ...container }] }, + } as runv2.Service; + } it("starts a new service with just the image", () => { expect(revisionTemplate({ config: web }, { image })).to.deep.equal({ @@ -273,10 +283,14 @@ describe("run deploy", () => { }); it("sets the deploy message, or clears the last one", () => { - const withMessage = revisionTemplate({ config: web, existing }, { image }, "Fix the login"); + const withMessage = revisionTemplate( + { config: web, existing }, + { image }, + "Fix the login page", + ); expect(withMessage.annotations).to.deep.equal({ team: "frontend", - [DEPLOY_MESSAGE]: "Fix the login", + [DEPLOY_MESSAGE]: "Fix the login page", }); const withoutMessage = revisionTemplate({ config: web, existing }, { image }); @@ -284,51 +298,43 @@ describe("run deploy", () => { }); it("runs a local build's output with the build's start command", () => { - const localBuild = { scratchDir: "/tmp/x", outputFiles: [], runCommand: " node server.js " }; + // The command is split on whitespace, so stray spaces don't become empty arguments. + const localBuild = { scratchDir, outputFiles: [".next"], runCommand: " npm run start " }; - const container = revisionTemplate({ config: web, existing, localBuild }, { builtApp }) - .containers?.[0]; + const template = revisionTemplate({ config: web, existing, localBuild }, { builtApp }); - expect(container).to.include({ image: "scratch" }); - expect(container?.sourceCode).to.deep.equal({ cloudStorageSource: builtApp }); - expect(container?.command).to.deep.equal(["node", "server.js"]); + expect(template.containers?.[0]).to.deep.include({ + image: "scratch", + sourceCode: { cloudStorageSource: builtApp }, + command: ["npm", "run", "start"], + }); }); it("adds the env vars a local build needs, unless the service already sets them", () => { const env = [ - { name: "MODE", value: "from-build" }, + { name: "MODE", value: "development" }, { name: "NODE_ENV", value: "production" }, ]; - const localBuild = { scratchDir: "/tmp/x", outputFiles: [], env }; + const localBuild = { scratchDir, outputFiles: [".next"], env }; - const container = revisionTemplate({ config: web, existing, localBuild }, { builtApp }) - .containers?.[0]; + const template = revisionTemplate({ config: web, existing, localBuild }, { builtApp }); - expect(container?.env).to.deep.equal([ + expect(template.containers?.[0].env).to.deep.equal([ { name: "MODE", value: "prod" }, { name: "NODE_ENV", value: "production" }, ]); }); it("switches a service from a local build back to an image", () => { - const fromLocalBuild = { - ...existing, - template: { - containers: [ - { - name: "web", - image: "scratch", - command: ["node", "server.js"], - sourceCode: { cloudStorageSource: builtApp }, - }, - ], - }, - } as unknown as runv2.Service; + const fromLocalBuild = webRunning({ + image: "scratch", + command: ["npm", "run", "start"], + sourceCode: { cloudStorageSource: builtApp }, + }); - const container = revisionTemplate({ config: web, existing: fromLocalBuild }, { image }) - .containers?.[0]; + const template = revisionTemplate({ config: web, existing: fromLocalBuild }, { image }); - expect(container).to.deep.equal({ name: "web", image }); + expect(template).to.deep.equal({ containers: [{ name: "web", image }] }); }); }); }); diff --git a/src/deploy/run/prepare.spec.ts b/src/deploy/run/prepare.spec.ts index e577b1039df..9933c4de7a4 100644 --- a/src/deploy/run/prepare.spec.ts +++ b/src/deploy/run/prepare.spec.ts @@ -3,7 +3,7 @@ import * as sinon from "sinon"; import * as runv2 from "../../gcp/runv2"; import { Options } from "../../options"; import { Context, Payload, ServiceDeploy } from "./args"; -import { BUILD_ENV_ANNOTATION } from "./buildEnv"; +import { BUILD_ENV_ANNOTATION, BuildEnv } from "./buildEnv"; import { prepare } from "./prepare"; import * as prereqs from "./prereqs"; @@ -15,6 +15,11 @@ describe("run prepare", () => { name: "projects/my-project/locations/us-central1/services/web", template: { containers: [{ name: "web", image: "old-image", baseImageUri: nodejs22 }] }, } as unknown as runv2.Service; + /** The live web service, with this build env annotation. */ + const withBuildEnv = (env: BuildEnv): runv2.Service => ({ + ...liveWeb, + annotations: { [BUILD_ENV_ANNOTATION]: JSON.stringify(env) }, + }); let prereqsStub: sinon.SinonStub; let getServiceStub: sinon.SinonStub; @@ -69,12 +74,14 @@ describe("run prepare", () => { describe("local builds", () => { it("require a base image", async () => { - getServiceStub.resolves({ + const webWithoutBaseImage = { ...liveWeb, template: { containers: [{ name: "web", image: "old-image" }] }, - }); + }; + getServiceStub.resolves(webWithoutBaseImage); + await expect(prepareRun(localWeb)).to.be.rejectedWith( - /Local builds require a base image.*firebase run:services:update web:us-central1 --base-image/, + 'Local builds require a base image. Set one for service web in us-central1 with "firebase run:services:update web:us-central1 --base-image ".', ); }); @@ -83,39 +90,31 @@ describe("run prepare", () => { 'Local builds require a base image. Set one for service web in us-central1 with "firebase init run".', ); }); - - it("are left to the deploy step", async () => { - getServiceStub.resolves(liveWeb); - const [svc] = await prepareRun(localWeb); - expect(svc.baseImage).to.equal(nodejs22); - expect(svc).not.to.have.property("localBuild"); - }); }); describe("build env", () => { - const withBuildEnv = (env: Record): runv2.Service => - ({ - ...liveWeb, - annotations: { [BUILD_ENV_ANNOTATION]: JSON.stringify(env) }, - }) as unknown as runv2.Service; - it("reads the build env from the service's annotation", async () => { getServiceStub.resolves(withBuildEnv({ API_URL: "https://api.example.com" })); + const [svc] = await prepareRun(web); + expect(svc.buildEnv).to.deep.equal({ API_URL: "https://api.example.com" }); }); it("rejects build secrets on Cloud Build", async () => { - getServiceStub.resolves(withBuildEnv({ TOKEN: { secret: "t" } })); + getServiceStub.resolves(withBuildEnv({ NPM_TOKEN: { secret: "npm-token" } })); + await expect(prepareRun(web)).to.be.rejectedWith( - /Service web in us-central1 has build secrets \(TOKEN\).*"localBuild": true/, + /Service web in us-central1 has build secrets \(NPM_TOKEN\).*"localBuild": true/, ); }); it("keeps build secrets for local builds", async () => { - getServiceStub.resolves(withBuildEnv({ TOKEN: { secret: "t", version: "2" } })); + getServiceStub.resolves(withBuildEnv({ NPM_TOKEN: { secret: "npm-token" } })); + const [svc] = await prepareRun(localWeb); - expect(svc.buildEnv).to.deep.equal({ TOKEN: { secret: "t", version: "2" } }); + + expect(svc.buildEnv).to.deep.equal({ NPM_TOKEN: { secret: "npm-token" } }); }); }); diff --git a/src/init/features/run.spec.ts b/src/init/features/run.spec.ts index 41d47ca7f46..098dc3b63a9 100644 --- a/src/init/features/run.spec.ts +++ b/src/init/features/run.spec.ts @@ -41,6 +41,13 @@ describe("init run", () => { afterEach(() => sinon.restore()); describe("askQuestions", () => { + /** Answers init's prompts, select or input, in the order it asks them. */ + function answerPrompts(...answers: unknown[]): void { + const nextAnswer = (): Promise => Promise.resolve(answers.shift()); + selectStub.callsFake(nextAnswer); + inputStub.callsFake(nextAnswer); + } + it("throws if no project is selected", async () => { await expect( askQuestions({ ...setup(), projectId: undefined }, config, options), @@ -144,55 +151,38 @@ describe("init run", () => { expect(s.featureInfo?.run?.serviceId).to.equal("my-service"); }); - it("enables localBuild and requires a base image when building locally", async () => { - selectStub - .onFirstCall() - .resolves("create") - .onSecondCall() - .resolves("us-central1") - .onThirdCall() - .resolves(true); - inputStub.onFirstCall().resolves("my-service").onSecondCall().resolves("nodejs22"); - inputStub.onThirdCall().resolves("/"); - const s = setup(); + it("builds a new service remotely by default", async () => { + answerPrompts("create", "us-central1", "web", /* localBuild= */ false, "", "/"); - await askQuestions(s, config, options); + await askQuestions(setup(), config, options); - expect(selectStub.thirdCall.args[0].default).to.be.false; - const baseImagePrompt = inputStub.secondCall.args[0]; - expect(baseImagePrompt.default).to.be.undefined; - expect(baseImagePrompt.validate("nodejs22")).to.be.true; - expect(baseImagePrompt.validate(" ")).to.include("Local builds require a base image"); - expect(s.featureInfo?.run).to.deep.equal({ - serviceId: "my-service", - region: "us-central1", - baseImage: "nodejs22", - rootDir: "/", - localBuild: true, - }); + const localBuildPrompt = selectStub.thirdCall.args[0]; + expect(localBuildPrompt.default).to.be.false; }); - it("defaults to building locally if the saved entry for the service does", async () => { - const existing = { - name: "projects/p/locations/europe-west1/services/web", - template: { containers: [{ name: "web", image: "i", baseImageUri: "nodejs22" }] }, - }; + it("defaults to building locally if firebase.json builds the service locally", async () => { + const existing = { name: "projects/p/locations/europe-west1/services/web", template: {} }; sinon.stub(runv2, "listServices").resolves([existing] as unknown as runv2.Service[]); - selectStub.onFirstCall().resolves("update").onSecondCall().resolves(existing); - selectStub.onThirdCall().callsFake((o) => Promise.resolve(o.default)); - inputStub.callsFake((o) => Promise.resolve(o.default)); - config.set("run", { - serviceId: "web", - region: "europe-west1", - rootDir: "/", - localBuild: true, - }); - const s = setup(); + config.set("run", { serviceId: "web", region: "europe-west1", localBuild: true }); + answerPrompts("update", existing, /* localBuild= */ true, "nodejs22", "/"); - await askQuestions(s, config, options); + await askQuestions(setup(), config, options); - expect(selectStub.thirdCall.args[0].default).to.be.true; - expect(s.featureInfo?.run?.localBuild).to.be.true; + const localBuildPrompt = selectStub.thirdCall.args[0]; + expect(localBuildPrompt.default).to.be.true; + }); + + it("saves the choice to build locally, which requires a base image", async () => { + answerPrompts("create", "us-central1", "web", /* localBuild= */ true, "nodejs22", "/"); + const initSetup = setup(); + + await askQuestions(initSetup, config, options); + + const baseImagePrompt = inputStub.secondCall.args[0]; + expect(baseImagePrompt.default).to.be.undefined; + expect(baseImagePrompt.validate(" ")).to.equal("Local builds require a base image."); + expect(baseImagePrompt.validate("nodejs22")).to.be.true; + expect(initSetup.featureInfo?.run).to.include({ localBuild: true }); }); }); @@ -284,45 +274,22 @@ describe("init run", () => { ]); }); - it("updates an existing service in place while preserving custom ignore and updating localBuild", () => { - config.set("run", { - serviceId: "web", - region: "us-central1", - rootDir: "/", - ignore: ["custom-ignore"], - }); + it("turns localBuild on for an existing service, keeping its custom ignore list", () => { + const web = { serviceId: "web", region: "us-central1", rootDir: "/" }; + config.set("run", { ...web, ignore: ["custom-ignore"] }); - upsertRunConfig( - { - serviceId: "web", - region: "us-central1", - rootDir: "apps/web", - localBuild: true, - }, - config, - ); - expect(config.src.run).to.deep.equal({ - serviceId: "web", - region: "us-central1", - rootDir: "apps/web", - localBuild: true, - ignore: ["custom-ignore"], - }); + upsertRunConfig({ ...web, localBuild: true }, config); - upsertRunConfig( - { - serviceId: "web", - region: "us-central1", - rootDir: "apps/web", - }, - config, - ); - expect(config.src.run).to.deep.equal({ - serviceId: "web", - region: "us-central1", - rootDir: "apps/web", - ignore: ["custom-ignore"], - }); + expect(config.src.run).to.deep.equal({ ...web, ignore: ["custom-ignore"], localBuild: true }); + }); + + it("turns localBuild off for an existing service, keeping its custom ignore list", () => { + const web = { serviceId: "web", region: "us-central1", rootDir: "/" }; + config.set("run", { ...web, ignore: ["custom-ignore"], localBuild: true }); + + upsertRunConfig(web, config); + + expect(config.src.run).to.deep.equal({ ...web, ignore: ["custom-ignore"] }); }); it("doesn't add an ignore list to an existing service", () => { From cea325c31bdd65acdfe5c7c9c257fbba1f916ad7 Mon Sep 17 00:00:00 2001 From: Aryan Falahatpisheh Date: Tue, 13 Oct 2026 14:16:49 -0400 Subject: [PATCH 3/6] Use App Hosting's secret format for build secrets A build secret is now written the way apphosting.yaml writes one ({"secret": "npm-token@3"}) and passed straight to loadSecret, so there are no extra regexes or format conversions. Cloud Build gets an explicit filter for plain values instead of a cast, and the comments and errors say why only local builds can use secrets. --- src/deploy/run/args.ts | 4 +- src/deploy/run/buildEnv.spec.ts | 44 +++++++++--------- src/deploy/run/buildEnv.ts | 80 ++++++++++++++++++--------------- src/deploy/run/deploy.spec.ts | 9 ++-- src/deploy/run/deploy.ts | 7 +-- src/deploy/run/prepare.ts | 10 +++-- 6 files changed, 83 insertions(+), 71 deletions(-) diff --git a/src/deploy/run/args.ts b/src/deploy/run/args.ts index 2304591cded..aff79ee8333 100644 --- a/src/deploy/run/args.ts +++ b/src/deploy/run/args.ts @@ -17,8 +17,8 @@ export interface ServiceDeploy { */ baseImage?: string; /** - * Set by prepare: the build-time environment. Prepare only allows secrets in it for local - * builds. + * Set by prepare: the build-time env vars from the service's annotation, each a plain value or + * a secret. Only local builds can use secrets: builds on Cloud Build only take plain values. */ buildEnv?: BuildEnv; /** Set by deploy: the service after the rollout. */ diff --git a/src/deploy/run/buildEnv.spec.ts b/src/deploy/run/buildEnv.spec.ts index e403ffde742..3f1974a4cfa 100644 --- a/src/deploy/run/buildEnv.spec.ts +++ b/src/deploy/run/buildEnv.spec.ts @@ -1,6 +1,12 @@ import { expect } from "chai"; import * as runv2 from "../../gcp/runv2"; -import { BUILD_ENV_ANNOTATION, getBuildEnv, secretNames, toLocalBuildEnv } from "./buildEnv"; +import { + BUILD_ENV_ANNOTATION, + getBuildEnv, + plainValues, + secretNames, + toLocalBuildEnv, +} from "./buildEnv"; describe("run buildEnv", () => { const web = { name: "projects/my-project/locations/us-central1/services/web" } as runv2.Service; @@ -21,12 +27,12 @@ describe("run buildEnv", () => { expect(getBuildEnv(withAnnotation(JSON.stringify(env)))).to.deep.equal(env); }); - it("parses secret references", () => { + it("parses secrets written the way apphosting.yaml writes them", () => { const env = { API_URL: "https://api.example.com", NPM_TOKEN: { secret: "npm-token" }, - SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token", version: "2" }, - GITHUB_TOKEN: { secret: "projects/shared-secrets/secrets/github-token", version: "latest" }, + SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token@2" }, + GITHUB_TOKEN: { secret: "projects/shared-secrets/secrets/github-token/versions/3" }, }; expect(getBuildEnv(withAnnotation(JSON.stringify(env)))).to.deep.equal(env); }); @@ -47,49 +53,39 @@ describe("run buildEnv", () => { ["npm-token"], {}, { secret: 1 }, - { secret: "secrets/npm-token" }, - { secret: "npm-token@3" }, - { secret: "projects/shared-secrets/secrets/npm-token/versions/3" }, - { secret: "npm-token", version: 3 }, - { secret: "npm-token", version: "v3" }, - { secret: "npm-token", project: "shared-secrets" }, + // The version goes in the secret string, e.g. "npm-token@3". + { secret: "npm-token", version: "3" }, ]; for (const value of invalid) { expect(() => getBuildEnv(withAnnotation(JSON.stringify({ NPM_TOKEN: value })))).to.throw( - 'NPM_TOKEN must be a string or {"secret": "", "version": ""}.', + 'NPM_TOKEN must be a string or {"secret": ""}.', ); } }); }); - it("lists secret names", () => { + it("splits secrets from plain values", () => { const env = { API_URL: "https://api.example.com", NPM_TOKEN: { secret: "npm-token" }, - SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token", version: "2" }, + SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token@2" }, }; expect(secretNames(env)).to.deep.equal(["NPM_TOKEN", "SENTRY_AUTH_TOKEN"]); + expect(plainValues(env)).to.deep.equal({ API_URL: "https://api.example.com" }); }); - it("converts to local build env, available only to the build", () => { + it("converts to local build env, available only to the build, with secrets unchanged", () => { expect( toLocalBuildEnv({ API_URL: "https://api.example.com", NPM_TOKEN: { secret: "npm-token" }, - SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token", version: "2" }, - GITHUB_TOKEN: { secret: "projects/shared-secrets/secrets/github-token" }, - CMS_TOKEN: { secret: "projects/shared-secrets/secrets/cms-token", version: "4" }, + GITHUB_TOKEN: { secret: "projects/shared-secrets/secrets/github-token/versions/3" }, }), ).to.deep.equal({ API_URL: { value: "https://api.example.com", availability: ["BUILD"] }, - NPM_TOKEN: { secret: "npm-token@latest", availability: ["BUILD"] }, - SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token@2", availability: ["BUILD"] }, + NPM_TOKEN: { secret: "npm-token", availability: ["BUILD"] }, GITHUB_TOKEN: { - secret: "projects/shared-secrets/secrets/github-token/versions/latest", - availability: ["BUILD"], - }, - CMS_TOKEN: { - secret: "projects/shared-secrets/secrets/cms-token/versions/4", + secret: "projects/shared-secrets/secrets/github-token/versions/3", availability: ["BUILD"], }, }); diff --git a/src/deploy/run/buildEnv.ts b/src/deploy/run/buildEnv.ts index 5ba9d8cb263..fe7866ab08b 100644 --- a/src/deploy/run/buildEnv.ts +++ b/src/deploy/run/buildEnv.ts @@ -3,27 +3,31 @@ import * as runv2 from "../../gcp/runv2"; import { EnvMap } from "../../apphosting/yaml"; /** - * Prototype: build-time environment variables and secret references, stored on the Cloud Run - * service. It extends Cloud Run's own buildConfig.environmentVariables (a map of plain strings): - * the same map parses unchanged, and a value can also be a secret reference in the shape of - * Cloud Run's runtime secretKeyRef. It holds references only, never secret values. - * Example: {"API_URL": "https://example.com", "NPM_TOKEN": {"secret": "npm-token", "version": "3"}} + * Prototype: build-time env vars and secrets, stored on the Cloud Run service as a JSON object. + * Each value is a plain string, or a Secret Manager secret written the way apphosting.yaml writes + * one: {"secret": ""}, {"secret": "@"}, or {"secret": ""}. + * For example: + * { + * "API_URL": "https://api.example.com", + * "NPM_TOKEN": {"secret": "npm-token"}, + * "SENTRY_AUTH_TOKEN": {"secret": "sentry-auth-token@2"}, + * "GITHUB_TOKEN": {"secret": "projects/shared-secrets/secrets/github-token/versions/3"} + * } + * The annotation only names secrets. Their values stay in Secret Manager. */ export const BUILD_ENV_ANNOTATION = "firebase.google.com/buildConfig.environmentVariablesAndSecrets"; -/** A Secret Manager secret, as a name in the service's project or a full resource name. */ +/** + * A Secret Manager secret, in App Hosting's format. A version is optional and defaults to the + * latest. Local builds read the secret with App Hosting's loadSecret, which checks the format. + */ export interface SecretRef { secret: string; - /** A version number or "latest". Defaults to "latest". */ - version?: string; } export type BuildEnv = Record; -const SECRET_NAME = /^(?:projects\/[^/]+\/secrets\/)?[^/@]+$/; -const SECRET_VERSION = /^(?:latest|\d+)$/; - /** Reads and validates the service's build environment. Returns {} if it has none. */ export function getBuildEnv(service: runv2.Service | undefined): BuildEnv { const raw = service?.annotations?.[BUILD_ENV_ANNOTATION]; @@ -42,47 +46,53 @@ export function getBuildEnv(service: runv2.Service | undefined): BuildEnv { throw invalid("it must be a JSON object."); } for (const [name, value] of Object.entries(parsed)) { - if (typeof value === "string") { - continue; - } - const ref = value as Partial | null; - const valid = - typeof ref === "object" && - ref !== null && - !Array.isArray(ref) && - Object.keys(ref).every((k) => k === "secret" || k === "version") && - typeof ref.secret === "string" && - SECRET_NAME.test(ref.secret) && - (ref.version === undefined || - (typeof ref.version === "string" && SECRET_VERSION.test(ref.version))); - if (!valid) { - throw invalid( - `${name} must be a string or {"secret": "", "version": ""}.`, - ); + if (typeof value !== "string" && !isSecretRef(value)) { + throw invalid(`${name} must be a string or {"secret": ""}.`); } } return parsed as BuildEnv; } -/** Names of the variables that are secret references. */ +/** Whether a value is {"secret": ""}, with no other fields. */ +function isSecretRef(value: unknown): value is SecretRef { + const ref = value as Partial | null; + return ( + typeof ref === "object" && + ref !== null && + Object.keys(ref).length === 1 && + typeof ref.secret === "string" + ); +} + +/** Names of the variables that are secrets. */ export function secretNames(env: BuildEnv): string[] { return Object.entries(env) .filter(([, value]) => typeof value !== "string") .map(([name]) => name); } -/** Converts the build environment to the form local builds take; they resolve the secrets. */ +/** The plain values, without the secrets. */ +export function plainValues(env: BuildEnv): Record { + const values: Record = {}; + for (const [name, value] of Object.entries(env)) { + if (typeof value === "string") { + values[name] = value; + } + } + return values; +} + +/** + * Converts the build environment to the form local builds take. Secrets pass through unchanged: + * the local build reads them with your credentials. + */ export function toLocalBuildEnv(env: BuildEnv): EnvMap { const result: EnvMap = {}; for (const [name, value] of Object.entries(env)) { if (typeof value === "string") { result[name] = { value, availability: ["BUILD"] }; } else { - const version = value.version || "latest"; - const secret = value.secret.startsWith("projects/") - ? `${value.secret}/versions/${version}` - : `${value.secret}@${version}`; - result[name] = { secret, availability: ["BUILD"] }; + result[name] = { secret: value.secret, availability: ["BUILD"] }; } } return result; diff --git a/src/deploy/run/deploy.spec.ts b/src/deploy/run/deploy.spec.ts index e5fc19e1572..fc57a27898b 100644 --- a/src/deploy/run/deploy.spec.ts +++ b/src/deploy/run/deploy.spec.ts @@ -81,11 +81,14 @@ describe("run deploy", () => { expect(buildpackBuild).not.to.have.property("enableAutomaticUpdates"); }); - it("passes the service's build env to Cloud Build", async () => { - await deployOne({ config: web, buildEnv: { API_URL: "https://api.example.com" } }); + it("passes the service's plain build env vars to Cloud Build, but never secrets", async () => { + const buildEnv = { API_URL: "https://api.example.com", NPM_TOKEN: { secret: "npm-token" } }; + + await deployOne({ config: web, buildEnv }); const { environmentVariables } = submitBuildStub.firstCall.args[2].buildpackBuild; expect(environmentVariables).to.include({ API_URL: "https://api.example.com" }); + expect(environmentVariables).not.to.have.property("NPM_TOKEN"); }); it("rolls out a new revision of an existing service with all traffic, keeping its tags", async () => { @@ -196,7 +199,7 @@ describe("run deploy", () => { it("passes the build env, secrets included, to the local build", async () => { const buildEnv = { API_URL: "https://api.example.com", - NPM_TOKEN: { secret: "npm-token", version: "2" }, + NPM_TOKEN: { secret: "npm-token@2" }, }; await deployOne({ config: localWeb, baseImage: NODEJS22, buildEnv }); diff --git a/src/deploy/run/deploy.ts b/src/deploy/run/deploy.ts index eeb696688f0..005a00419b9 100644 --- a/src/deploy/run/deploy.ts +++ b/src/deploy/run/deploy.ts @@ -16,7 +16,7 @@ import { cloneDeep, logLabeledBullet } from "../../utils"; import { prepareLocalBuildScratchDirectory } from "../apphosting/prepare"; import { createLocalBuildTarArchive, createSourceDeployArchive } from "../apphosting/util"; import { Context, Payload, ServiceDeploy } from "./args"; -import { secretNames, toLocalBuildEnv } from "./buildEnv"; +import { plainValues, secretNames, toLocalBuildEnv } from "./buildEnv"; import { mainContainer } from "./util"; const DEPLOY_MESSAGE_ANNOTATION = "firebase.google.com/deploy-message"; @@ -213,8 +213,9 @@ async function buildImage( logLabeledBullet("run", `Building service ${serviceId} in ${region}...`); const buildpackBuild: runv2.BuildpacksBuild = { environmentVariables: { - // Prepare rejects build secrets for builds on Cloud Build, so these are all plain values. - ...(svc.buildEnv as Record | undefined), + // Cloud Build only takes plain values, so leave out any secrets. Prepare already rejects + // secrets for builds on Cloud Build. + ...plainValues(svc.buildEnv || {}), // Use App Hosting's buildpacks, which need to know where to write the output bundle. X_GOOGLE_TARGET_PLATFORM: "fah", FIREBASE_OUTPUT_BUNDLE_DIR: "/workspace/.apphosting", diff --git a/src/deploy/run/prepare.ts b/src/deploy/run/prepare.ts index 6015924b6ca..f5ed11d8a10 100644 --- a/src/deploy/run/prepare.ts +++ b/src/deploy/run/prepare.ts @@ -43,12 +43,14 @@ async function prepareService(context: Context, config: RunSingle): Promise Date: Tue, 13 Oct 2026 14:20:55 -0400 Subject: [PATCH 4/6] Rename plainValues to nonSecretValues The name and doc comment now say that the result never includes secrets or their values. --- src/deploy/run/buildEnv.spec.ts | 4 ++-- src/deploy/run/buildEnv.ts | 7 +++++-- src/deploy/run/deploy.ts | 8 ++++---- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/deploy/run/buildEnv.spec.ts b/src/deploy/run/buildEnv.spec.ts index 3f1974a4cfa..710987bdfbc 100644 --- a/src/deploy/run/buildEnv.spec.ts +++ b/src/deploy/run/buildEnv.spec.ts @@ -3,7 +3,7 @@ import * as runv2 from "../../gcp/runv2"; import { BUILD_ENV_ANNOTATION, getBuildEnv, - plainValues, + nonSecretValues, secretNames, toLocalBuildEnv, } from "./buildEnv"; @@ -71,7 +71,7 @@ describe("run buildEnv", () => { SENTRY_AUTH_TOKEN: { secret: "sentry-auth-token@2" }, }; expect(secretNames(env)).to.deep.equal(["NPM_TOKEN", "SENTRY_AUTH_TOKEN"]); - expect(plainValues(env)).to.deep.equal({ API_URL: "https://api.example.com" }); + expect(nonSecretValues(env)).to.deep.equal({ API_URL: "https://api.example.com" }); }); it("converts to local build env, available only to the build, with secrets unchanged", () => { diff --git a/src/deploy/run/buildEnv.ts b/src/deploy/run/buildEnv.ts index fe7866ab08b..eb177127b5b 100644 --- a/src/deploy/run/buildEnv.ts +++ b/src/deploy/run/buildEnv.ts @@ -71,8 +71,11 @@ export function secretNames(env: BuildEnv): string[] { .map(([name]) => name); } -/** The plain values, without the secrets. */ -export function plainValues(env: BuildEnv): Record { +/** + * The env vars that aren't secrets, with their values. Secrets are left out entirely, so the result + * never names a secret or holds a secret's value. + */ +export function nonSecretValues(env: BuildEnv): Record { const values: Record = {}; for (const [name, value] of Object.entries(env)) { if (typeof value === "string") { diff --git a/src/deploy/run/deploy.ts b/src/deploy/run/deploy.ts index 005a00419b9..874cd63bed3 100644 --- a/src/deploy/run/deploy.ts +++ b/src/deploy/run/deploy.ts @@ -16,7 +16,7 @@ import { cloneDeep, logLabeledBullet } from "../../utils"; import { prepareLocalBuildScratchDirectory } from "../apphosting/prepare"; import { createLocalBuildTarArchive, createSourceDeployArchive } from "../apphosting/util"; import { Context, Payload, ServiceDeploy } from "./args"; -import { plainValues, secretNames, toLocalBuildEnv } from "./buildEnv"; +import { nonSecretValues, secretNames, toLocalBuildEnv } from "./buildEnv"; import { mainContainer } from "./util"; const DEPLOY_MESSAGE_ANNOTATION = "firebase.google.com/deploy-message"; @@ -213,9 +213,9 @@ async function buildImage( logLabeledBullet("run", `Building service ${serviceId} in ${region}...`); const buildpackBuild: runv2.BuildpacksBuild = { environmentVariables: { - // Cloud Build only takes plain values, so leave out any secrets. Prepare already rejects - // secrets for builds on Cloud Build. - ...plainValues(svc.buildEnv || {}), + // Cloud Build only takes plain values, so pass only the non-secret env vars. Prepare already + // rejects secrets for builds on Cloud Build. + ...nonSecretValues(svc.buildEnv || {}), // Use App Hosting's buildpacks, which need to know where to write the output bundle. X_GOOGLE_TARGET_PLATFORM: "fah", FIREBASE_OUTPUT_BUNDLE_DIR: "/workspace/.apphosting", From c84fca15ec6a0078171e4a220567c9cb84d3d94b Mon Sep 17 00:00:00 2001 From: Aryan Falahatpisheh Date: Tue, 13 Oct 2026 14:44:31 -0400 Subject: [PATCH 5/6] Accept numbers and booleans in the build env annotation, like apphosting.yaml --- src/deploy/run/buildEnv.spec.ts | 15 +++++++++++++-- src/deploy/run/buildEnv.ts | 24 ++++++++++++++++++------ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/deploy/run/buildEnv.spec.ts b/src/deploy/run/buildEnv.spec.ts index 710987bdfbc..8a8e580bce9 100644 --- a/src/deploy/run/buildEnv.spec.ts +++ b/src/deploy/run/buildEnv.spec.ts @@ -46,9 +46,20 @@ describe("run buildEnv", () => { } }); + it("converts numbers and booleans to strings, as apphosting.yaml does", () => { + const annotation = + '{"NEXT_TELEMETRY_DISABLED": 1, "CI": true, "DEBUG": false, "RATIO": 1.50}'; + expect(getBuildEnv(withAnnotation(annotation))).to.deep.equal({ + NEXT_TELEMETRY_DISABLED: "1", + CI: "true", + DEBUG: "false", + // JSON doesn't keep how a number was written. + RATIO: "1.5", + }); + }); + it("rejects invalid values", () => { const invalid = [ - 1, null, ["npm-token"], {}, @@ -58,7 +69,7 @@ describe("run buildEnv", () => { ]; for (const value of invalid) { expect(() => getBuildEnv(withAnnotation(JSON.stringify({ NPM_TOKEN: value })))).to.throw( - 'NPM_TOKEN must be a string or {"secret": ""}.', + 'NPM_TOKEN must be a string, number, boolean, or {"secret": ""}.', ); } }); diff --git a/src/deploy/run/buildEnv.ts b/src/deploy/run/buildEnv.ts index eb177127b5b..ecc7785eb19 100644 --- a/src/deploy/run/buildEnv.ts +++ b/src/deploy/run/buildEnv.ts @@ -4,11 +4,13 @@ import { EnvMap } from "../../apphosting/yaml"; /** * Prototype: build-time env vars and secrets, stored on the Cloud Run service as a JSON object. - * Each value is a plain string, or a Secret Manager secret written the way apphosting.yaml writes - * one: {"secret": ""}, {"secret": "@"}, or {"secret": ""}. + * Each value is a string, a number or boolean (which becomes a string, as in apphosting.yaml), or a + * Secret Manager secret written the way apphosting.yaml writes one: {"secret": ""}, + * {"secret": "@"}, or {"secret": ""}. * For example: * { * "API_URL": "https://api.example.com", + * "NEXT_TELEMETRY_DISABLED": 1, * "NPM_TOKEN": {"secret": "npm-token"}, * "SENTRY_AUTH_TOKEN": {"secret": "sentry-auth-token@2"}, * "GITHUB_TOKEN": {"secret": "projects/shared-secrets/secrets/github-token/versions/3"} @@ -28,7 +30,10 @@ export interface SecretRef { export type BuildEnv = Record; -/** Reads and validates the service's build environment. Returns {} if it has none. */ +/** + * Reads and validates the service's build environment. Returns {} if it has none. Numbers and + * booleans come back as strings, since env vars are strings. + */ export function getBuildEnv(service: runv2.Service | undefined): BuildEnv { const raw = service?.annotations?.[BUILD_ENV_ANNOTATION]; if (!service || !raw) { @@ -45,12 +50,19 @@ export function getBuildEnv(service: runv2.Service | undefined): BuildEnv { if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { throw invalid("it must be a JSON object."); } + const env: BuildEnv = {}; for (const [name, value] of Object.entries(parsed)) { - if (typeof value !== "string" && !isSecretRef(value)) { - throw invalid(`${name} must be a string or {"secret": ""}.`); + if (typeof value === "string" || isSecretRef(value)) { + env[name] = value; + } else if (typeof value === "number" || typeof value === "boolean") { + // apphosting.yaml takes unquoted numbers and booleans too. JSON doesn't keep how a number was + // written, so 1.50 becomes "1.5". Quote a value to keep it exactly as written. + env[name] = String(value); + } else { + throw invalid(`${name} must be a string, number, boolean, or {"secret": ""}.`); } } - return parsed as BuildEnv; + return env; } /** Whether a value is {"secret": ""}, with no other fields. */ From b4b4cb3340c6cb268c06d1ba9497dbf0d5f584a1 Mon Sep 17 00:00:00 2001 From: Aryan Falahatpisheh Date: Tue, 13 Oct 2026 14:47:20 -0400 Subject: [PATCH 6/6] Check for secrets with isSecretRef instead of typeof string --- src/deploy/run/buildEnv.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/deploy/run/buildEnv.ts b/src/deploy/run/buildEnv.ts index ecc7785eb19..6c1eab6a979 100644 --- a/src/deploy/run/buildEnv.ts +++ b/src/deploy/run/buildEnv.ts @@ -79,7 +79,7 @@ function isSecretRef(value: unknown): value is SecretRef { /** Names of the variables that are secrets. */ export function secretNames(env: BuildEnv): string[] { return Object.entries(env) - .filter(([, value]) => typeof value !== "string") + .filter(([, value]) => isSecretRef(value)) .map(([name]) => name); } @@ -90,7 +90,7 @@ export function secretNames(env: BuildEnv): string[] { export function nonSecretValues(env: BuildEnv): Record { const values: Record = {}; for (const [name, value] of Object.entries(env)) { - if (typeof value === "string") { + if (!isSecretRef(value)) { values[name] = value; } } @@ -104,10 +104,10 @@ export function nonSecretValues(env: BuildEnv): Record { export function toLocalBuildEnv(env: BuildEnv): EnvMap { const result: EnvMap = {}; for (const [name, value] of Object.entries(env)) { - if (typeof value === "string") { - result[name] = { value, availability: ["BUILD"] }; - } else { + if (isSecretRef(value)) { result[name] = { secret: value.secret, availability: ["BUILD"] }; + } else { + result[name] = { value, availability: ["BUILD"] }; } } return result;