Skip to content

Commit b5cb204

Browse files
authored
Add a warning/confirmation when using BUILD-available secrets for local builds (#10337)
* Enable secret resolution during local App Hosting builds * fix build error * Handle env vars (especially secrets) with a Promise.all so it can be parallelized * Add a warning so that we do not use build-available secrets unless the user confirms * fix undefined boolean flag behavior * fix tests, address some review comments * fix test mocking
1 parent 4349832 commit b5cb204

4 files changed

Lines changed: 125 additions & 2 deletions

File tree

src/apphosting/localbuilds.spec.ts

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,10 @@ describe("localBuild", () => {
7676
MY_PLAIN_VAR: { value: "plain-value" },
7777
};
7878

79-
await localBuild("test-project", "./", "nextjs", envMap);
79+
await localBuild("test-project", "./", "nextjs", envMap, {
80+
nonInteractive: true,
81+
allowLocalBuildSecrets: true,
82+
});
8083

8184
expect(loadSecretStub).to.have.been.calledWith("test-project", "my-secret-id");
8285
// Confirm RUNTIME-only secret was ignored
@@ -117,4 +120,86 @@ describe("localBuild", () => {
117120
expect(process.env.MY_PLAIN_VAR).to.be.undefined;
118121
expect(process.env.ANOTHER_VAR).to.be.undefined;
119122
});
123+
124+
describe("localBuild secret confirmations", () => {
125+
let confirmStub: sinon.SinonStub;
126+
127+
beforeEach(() => {
128+
confirmStub = sinon.stub(require("../prompt"), "confirm");
129+
});
130+
131+
it("throws an error in non-interactive mode if build-available secrets are used without the bypass flag", async () => {
132+
const envMap: EnvMap = {
133+
MY_BUILD_SECRET: { secret: "my-secret-id", availability: ["BUILD"] },
134+
};
135+
136+
await expect(
137+
localBuild("test-project", "./", "nextjs", envMap, { nonInteractive: true }),
138+
).to.be.rejectedWith(
139+
"Using build-available secrets during a local build in non-interactive mode requires the --allow-local-build-secrets flag.",
140+
);
141+
});
142+
143+
it("allows build-available secrets in non-interactive mode if bypass flag is provided", async () => {
144+
const bundleConfig = {
145+
version: "v1" as const,
146+
runConfig: { runCommand: "npm run build:prod" },
147+
metadata: {
148+
adapterPackageName: "@apphosting/angular-adapter",
149+
adapterVersion: "14.1",
150+
framework: "nextjs",
151+
},
152+
outputFiles: { serverApp: { include: ["./next/standalone"] } },
153+
};
154+
sinon.stub(localBuildModule, "localBuild").resolves(bundleConfig);
155+
sinon.stub(secrets, "loadSecret").resolves("secret-value");
156+
157+
const envMap: EnvMap = {
158+
MY_BUILD_SECRET: { secret: "my-secret-id", availability: ["BUILD"] },
159+
};
160+
161+
await localBuild("test-project", "./", "nextjs", envMap, {
162+
nonInteractive: true,
163+
allowLocalBuildSecrets: true,
164+
});
165+
166+
expect(confirmStub).to.not.have.been.called;
167+
});
168+
169+
it("cancels the build if the user declines the secrets confirmation prompt", async () => {
170+
confirmStub.resolves(false);
171+
172+
const envMap: EnvMap = {
173+
MY_BUILD_SECRET: { secret: "my-secret-id", availability: ["BUILD"] },
174+
};
175+
176+
await expect(
177+
localBuild("test-project", "./", "nextjs", envMap, { nonInteractive: false }),
178+
).to.be.rejectedWith("Cancelled local build due to BUILD-available secrets.");
179+
expect(confirmStub).to.have.been.calledOnce;
180+
});
181+
182+
it("proceeds with the build if the user accepts the secrets confirmation prompt", async () => {
183+
confirmStub.resolves(true);
184+
const bundleConfig = {
185+
version: "v1" as const,
186+
runConfig: { runCommand: "npm run build:prod" },
187+
metadata: {
188+
adapterPackageName: "@apphosting/angular-adapter",
189+
adapterVersion: "14.1",
190+
framework: "nextjs",
191+
},
192+
outputFiles: { serverApp: { include: ["./next/standalone"] } },
193+
};
194+
sinon.stub(localBuildModule, "localBuild").resolves(bundleConfig);
195+
sinon.stub(secrets, "loadSecret").resolves("secret-value");
196+
197+
const envMap: EnvMap = {
198+
MY_BUILD_SECRET: { secret: "my-secret-id", availability: ["BUILD"] },
199+
};
200+
201+
await localBuild("test-project", "./", "nextjs", envMap, { nonInteractive: false });
202+
expect(confirmStub).to.have.been.calledOnce;
203+
});
204+
});
120205
});

src/apphosting/localbuilds.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { BuildConfig, Env } from "../gcp/apphosting";
22
import { localBuild as localAppHostingBuild } from "@apphosting/build";
33
import { EnvMap } from "./yaml";
44
import { loadSecret } from "./secrets";
5+
import { confirm } from "../prompt";
6+
import { FirebaseError } from "../error";
57

68
/**
79
* Triggers a local build of your App Hosting codebase.
@@ -23,11 +25,33 @@ export async function localBuild(
2325
projectRoot: string,
2426
framework: string,
2527
env: EnvMap = {},
28+
options?: { nonInteractive?: boolean; allowLocalBuildSecrets?: boolean },
2629
): Promise<{
2730
outputFiles: string[];
2831
annotations: Record<string, string>;
2932
buildConfig: BuildConfig;
3033
}> {
34+
const hasBuildAvailableSecrets = Object.values(env).some(
35+
(v) => v.secret && (!v.availability || v.availability.includes("BUILD")),
36+
);
37+
38+
if (hasBuildAvailableSecrets && !options?.allowLocalBuildSecrets) {
39+
if (options?.nonInteractive) {
40+
throw new FirebaseError(
41+
"Using build-available secrets during a local build in non-interactive mode requires the --allow-local-build-secrets flag.",
42+
);
43+
}
44+
if (
45+
!(await confirm({
46+
message:
47+
"Your build includes secrets that are available to the build environment. Using secrets in local builds may leave sensitive values in local artifacts/temporary files. Do you want to continue?",
48+
default: false,
49+
}))
50+
) {
51+
throw new FirebaseError("Cancelled local build due to BUILD-available secrets.");
52+
}
53+
}
54+
3155
// We need to inject the environment variables into the process.env
3256
// because the build adapter uses them to build the app.
3357
// We'll restore the original process.env after the build is done.

src/commands/deploy.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { pickHostingSiteName } from "../hosting/interactive";
1414
import { logBullet } from "../utils";
1515
import { createSite } from "../hosting/api";
1616
import { Options } from "../options";
17+
import * as experiments from "../experiments";
1718

1819
// in order of least time-consuming to most time-consuming
1920
export const VALID_DEPLOY_TARGETS = [
@@ -101,7 +102,16 @@ export const command = new Command("deploy")
101102
"--dry-run",
102103
"perform a dry run of your deployment. Validates your changes and builds your code without deploying any changes to your project. " +
103104
"In order to provide better validation, this may still enable APIs on the target project",
104-
)
105+
);
106+
107+
if (experiments.isEnabled("apphostinglocalbuilds")) {
108+
command.option(
109+
"--allow-local-build-secrets",
110+
"allow the use of build-available secrets in local builds for App Hosting without interactive confirmation",
111+
);
112+
}
113+
114+
command
105115
.before(requireConfig)
106116
.before((options: Options) => {
107117
options.filteredTargets = filterTargets(options, VALID_DEPLOY_TARGETS);

src/deploy/apphosting/prepare.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,10 @@ export default async function (context: Context, options: Options): Promise<void
193193
options.projectRoot || "./",
194194
"nextjs",
195195
buildEnv[cfg.backendId] || {},
196+
{
197+
nonInteractive: options.nonInteractive,
198+
allowLocalBuildSecrets: !!options.allowLocalBuildSecrets,
199+
},
196200
);
197201
if (outputFiles.length !== 1) {
198202
throw new FirebaseError(

0 commit comments

Comments
 (0)