diff --git a/src/apphosting/config.ts b/src/apphosting/config.ts index 6e73135e70c..5fa17e72dd2 100644 --- a/src/apphosting/config.ts +++ b/src/apphosting/config.ts @@ -14,8 +14,19 @@ import { logger } from "../logger"; import { updateOrCreateGitignore } from "../utils"; import { getOrPromptProject } from "../management/projects"; +// Common config across all environments export const APPHOSTING_BASE_YAML_FILE = "apphosting.yaml"; + +// Modern version of local configuration that is intended to be safe to commit. +// In order to ensure safety, values that are secret environment variables in +// apphosting.yaml cannot be made plaintext in apphosting.emulators.yaml +export const APPHOSTING_EMULATORS_YAML_FILE = "apphosting.emulator.yaml"; + +// Legacy/undocumented version of local configuration that is allowed to store +// values that are secrets in preceeding files as plaintext. It is not safe +// to commit to SCM export const APPHOSTING_LOCAL_YAML_FILE = "apphosting.local.yaml"; + export const APPHOSTING_YAML_FILE_REGEX = /^apphosting(\.[a-z0-9_]+)?\.yaml$/; export interface RunConfig { diff --git a/src/apphosting/yaml.ts b/src/apphosting/yaml.ts index fe2f6006d91..821f5e97f56 100644 --- a/src/apphosting/yaml.ts +++ b/src/apphosting/yaml.ts @@ -59,7 +59,7 @@ export class AppHostingYamlConfig { .map(([key]) => key); if (wereSecrets.some((key) => other.env[key]?.value)) { throw new FirebaseError( - `Cannot convert secret to plaintext in ${other.filename ? other.filename : "apphosting yaml"}`, + `Cannot convert secret to plaintext in ${other.filename ?? "apphosting yaml"}`, ); } } diff --git a/src/emulator/apphosting/config.spec.ts b/src/emulator/apphosting/config.spec.ts index 8cf420d0b9a..bc7caa0b17f 100644 --- a/src/emulator/apphosting/config.spec.ts +++ b/src/emulator/apphosting/config.spec.ts @@ -1,15 +1,11 @@ -import * as path from "path"; -import * as utils from "./developmentServer"; - import * as sinon from "sinon"; import { expect } from "chai"; import { getLocalAppHostingConfiguration } from "./config"; import * as configImport from "../../apphosting/config"; import { AppHostingYamlConfig } from "../../apphosting/yaml"; +import { FirebaseError } from "../../error"; describe("environments", () => { - let joinStub: sinon.SinonStub; - let loggerStub: sinon.SinonStub; let loadAppHostingYamlStub: sinon.SinonStub; let listAppHostingFilesInPathStub: sinon.SinonStub; const apphostingYamlEnvOne = { @@ -26,6 +22,12 @@ describe("environments", () => { randomSecretOne: { secret: "SECRET_ONE_FROM_CONFIG_TWO" }, randomSecretFour: { secret: "SECRET_FOUR_FROM_CONFIG_TWO" }, }; + const apphostingYamlSecretToPlaintext = { + randomSecretOne: { value: "RANDOM_SECRET_ONE_PLAINTEXT" }, + randomSecretTwo: { value: "RANDOM_SECRET_TWO_PLAINTEXT" }, + randomSecretThree: { value: "RANDOM_SECRET_THREE_PLAINTEXT" }, + randomSecretFour: { value: "RANDOM_SECRET_FOUR_PLAINTEXT" }, + }; // Configs used for stubs const apphostingYamlConfigOne = AppHostingYamlConfig.empty(); @@ -34,16 +36,15 @@ describe("environments", () => { const apphostingYamlConfigTwo = AppHostingYamlConfig.empty(); apphostingYamlConfigTwo.env = { ...apphostingYamlEnvTwo }; + const apphostingYamlConfigSecretsToPlaintext = AppHostingYamlConfig.empty(); + apphostingYamlConfigSecretsToPlaintext.env = { ...apphostingYamlSecretToPlaintext }; + beforeEach(() => { loadAppHostingYamlStub = sinon.stub(AppHostingYamlConfig, "loadFromFile"); - joinStub = sinon.stub(path, "join"); - loggerStub = sinon.stub(utils, "logger"); listAppHostingFilesInPathStub = sinon.stub(configImport, "listAppHostingFilesInPath"); }); afterEach(() => { - joinStub.restore(); - loggerStub.restore(); sinon.verifyAndRestore(); }); @@ -85,14 +86,58 @@ describe("environments", () => { .returns(apphostingYamlConfigOne); loadAppHostingYamlStub .withArgs("/parent/apphosting.local.yaml") + .returns(apphostingYamlConfigSecretsToPlaintext); + + const apphostingConfig = await getLocalAppHostingConfiguration("./"); + + expect(apphostingConfig.env).to.deep.equal({ + ...apphostingYamlEnvOne, + ...apphostingYamlSecretToPlaintext, + }); + }); + + it("should allow merging all three file types", async () => { + listAppHostingFilesInPathStub.returns([ + "/parent/cwd/apphosting.yaml", + "/parent/cwd/apphosting.emulator.yaml", + "/parent/apphosting.local.yaml", + ]); + + // Second config takes precedence + loadAppHostingYamlStub + .withArgs("/parent/cwd/apphosting.yaml") + .returns(apphostingYamlConfigOne); + loadAppHostingYamlStub + .withArgs("/parent/cwd/apphosting.emulator.yaml") .returns(apphostingYamlConfigTwo); + loadAppHostingYamlStub + .withArgs("/parent/apphosting.local.yaml") + .returns(apphostingYamlConfigSecretsToPlaintext); const apphostingConfig = await getLocalAppHostingConfiguration("./"); expect(apphostingConfig.env).to.deep.equal({ ...apphostingYamlEnvOne, ...apphostingYamlEnvTwo, + ...apphostingYamlSecretToPlaintext, }); }); + + it("Should not allow apphosting.emulator.yaml to convert secrets to plaintext", async () => { + listAppHostingFilesInPathStub.returns([ + "/parent/cwd/apphosting.yaml", + "/parent/cwd/apphosting.emulator.yaml", + ]); + + // Second config takes precedence + loadAppHostingYamlStub + .withArgs("/parent/cwd/apphosting.yaml") + .returns(apphostingYamlConfigOne); + loadAppHostingYamlStub + .withArgs("/parent/cwd/apphosting.emulator.yaml") + .returns(apphostingYamlConfigSecretsToPlaintext); + + await expect(getLocalAppHostingConfiguration("./")).to.be.rejectedWith(FirebaseError); + }); }); }); diff --git a/src/emulator/apphosting/config.ts b/src/emulator/apphosting/config.ts index 913943c0232..36d818fe5b9 100644 --- a/src/emulator/apphosting/config.ts +++ b/src/emulator/apphosting/config.ts @@ -1,41 +1,51 @@ import { basename } from "path"; import { APPHOSTING_BASE_YAML_FILE, + APPHOSTING_EMULATORS_YAML_FILE, APPHOSTING_LOCAL_YAML_FILE, listAppHostingFilesInPath, } from "../../apphosting/config"; import { AppHostingYamlConfig } from "../../apphosting/yaml"; /** - * Loads in apphosting.yaml & apphosting.local.yaml, giving - * apphosting.local.yaml precedence if present. + * Loads in apphosting.yaml, apphosting.emulator.yaml & apphosting.local.yaml as an + * overriding union. In order to keep apphosting.emulator.yaml safe to commit, + * users cannot change a secret environment variable to plaintext. + * apphosting.local.yaml can, however, for reverse compatibility, though its existence + * will be downplayed and tooling will not assist in creating or managing it. */ export async function getLocalAppHostingConfiguration( backendDir: string, ): Promise { const appHostingConfigPaths = listAppHostingFilesInPath(backendDir); // generate a map to make it easier to interface between file name and it's path - const fileNameToPathMap: Map = new Map(); - for (const path of appHostingConfigPaths) { - const fileName = basename(path); - fileNameToPathMap.set(fileName, path); - } + const fileNameToPathMap = Object.fromEntries( + appHostingConfigPaths.map((path) => [basename(path), path]), + ); + + const output = AppHostingYamlConfig.empty(); - const baseFilePath = fileNameToPathMap.get(APPHOSTING_BASE_YAML_FILE); - const localFilePath = fileNameToPathMap.get(APPHOSTING_LOCAL_YAML_FILE); + const baseFilePath = fileNameToPathMap[APPHOSTING_BASE_YAML_FILE]; + const emulatorsFilePath = fileNameToPathMap[APPHOSTING_EMULATORS_YAML_FILE]; + const localFilePath = fileNameToPathMap[APPHOSTING_LOCAL_YAML_FILE]; + + if (baseFilePath) { + // N.B. merging from empty helps tests stay hermetic. I previously ran into a test bug where + // using the returned value as the base caused the test stub to be modified and tests would succeed + // independently but would fail as part of a suite. + const baseFile = await AppHostingYamlConfig.loadFromFile(baseFilePath); + output.merge(baseFile, /* allowSecretsToBecomePlaintext= */ false); + } - // apphosting.local.yaml or apphosting.yaml are not required to run the emulator - if (!baseFilePath && !localFilePath) { - return AppHostingYamlConfig.empty(); + if (emulatorsFilePath) { + const emulatorsConfig = await AppHostingYamlConfig.loadFromFile(emulatorsFilePath); + output.merge(emulatorsConfig, /* allowSecretsToBecomePlaintext= */ false); } - // If one of them exists ... - if (!baseFilePath || !localFilePath) { - return await AppHostingYamlConfig.loadFromFile((baseFilePath || localFilePath)!); + if (localFilePath) { + const localYamlConfig = await AppHostingYamlConfig.loadFromFile(localFilePath); + output.merge(localYamlConfig, /* allowSecretsToBecomePlaintext= */ true); } - const localYamlConfig = await AppHostingYamlConfig.loadFromFile(localFilePath); - const baseConfig = await AppHostingYamlConfig.loadFromFile(baseFilePath); - baseConfig.merge(localYamlConfig); - return baseConfig; + return output; }