Skip to content

Commit

Permalink
Fix bug where secret values provided in env files were parsed as list. (
Browse files Browse the repository at this point in the history
#7422)

* Fix bug where secret values provided in env files were parsed as list.

* Add changelog.

* Fix linter errors.
  • Loading branch information
taeold authored Jul 9, 2024
1 parent 461c926 commit 587e593
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
- Pass down `functions.ignore` values in `firebase.json` to functions emulator so that supposedly ignored directories/files will not trigger reload. (#7414)
- Fixes bug where secret values provided in env files were parsed as list (#7422)
70 changes: 69 additions & 1 deletion src/deploy/functions/build.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from "chai";
import * as build from "./build";
import { ParamValue } from "./params";
import { ParamValue, Param } from "./params";
import { FirebaseError } from "../../error";

describe("toBackend", () => {
Expand Down Expand Up @@ -186,3 +186,71 @@ describe("toBackend", () => {
}).to.throw(FirebaseError, /Value "INVALID" is an invalid egress setting./);
});
});

describe("envWithType", () => {
it("converts raw environment variables to params with correct type", () => {
const params: Param[] = [
{
name: "A_STR",
type: "string",
},
{
name: "AN_INT",
type: "int",
},
{
name: "A_BOOL",
type: "boolean",
},
];
const rawEnvs: Record<string, string> = {
A_STR: "foo",
AN_INT: "1",
A_BOOL: "true",
NOT_PARAM: "not-a-param",
};
const out = build.envWithTypes(params, rawEnvs);

expect(out).to.include.keys(["A_STR", "AN_INT", "A_BOOL"]);

expect(out.A_STR.legalString).to.be.true;
expect(out.A_STR.legalBoolean).to.be.false;
expect(out.A_STR.legalNumber).to.be.false;
expect(out.A_STR.legalList).to.be.false;
expect(out.A_STR.asString()).to.equal("foo");

expect(out.AN_INT.legalString).to.be.false;
expect(out.AN_INT.legalBoolean).to.be.false;
expect(out.AN_INT.legalNumber).to.be.true;
expect(out.AN_INT.legalList).to.be.false;
expect(out.AN_INT.asNumber()).to.equal(1);

expect(out.A_BOOL.legalString).to.be.false;
expect(out.A_BOOL.legalBoolean).to.be.true;
expect(out.A_BOOL.legalNumber).to.be.false;
expect(out.A_BOOL.legalList).to.be.false;
expect(out.A_BOOL.asBoolean()).to.be.true;
});

it("converts raw environment variable for secret param with correct type", () => {
const params: Param[] = [
{
name: "WHOOPS_SECRET",
type: "secret",
},
];
const rawEnvs: Record<string, string> = {
A_STR: "foo",
WHOOPS_SECRET: "super-secret",
};
const out = build.envWithTypes(params, rawEnvs);

expect(out).to.include.keys(["WHOOPS_SECRET"]);

expect(out.WHOOPS_SECRET.legalString).to.be.true;
expect(out.WHOOPS_SECRET.legalBoolean).to.be.false;
expect(out.WHOOPS_SECRET.legalNumber).to.be.false;
expect(out.WHOOPS_SECRET.legalList).to.be.false;
expect(out.WHOOPS_SECRET.asString()).to.equal("super-secret");
});
});
15 changes: 13 additions & 2 deletions src/deploy/functions/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export interface HttpsTrigger {
// Trigger definitions for RPCs servers using the HTTP protocol defined at
// https://firebase.google.com/docs/functions/callable-reference
// eslint-disable-next-line
interface CallableTrigger { }
interface CallableTrigger {}

// Trigger definitions for endpoints that should be called as a delegate for other operations.
// For example, before user login.
Expand Down Expand Up @@ -301,7 +301,8 @@ export async function resolveBackend(
return { backend: toBackend(build, paramValues), envs: paramValues };
}

function envWithTypes(
// Exported for testing
export function envWithTypes(
definedParams: params.Param[],
rawEnvs: Record<string, string>,
): Record<string, params.ParamValue> {
Expand Down Expand Up @@ -344,6 +345,16 @@ function envWithTypes(
number: false,
list: true,
};
} else if (param.type === "secret") {
// NOTE(danielylee): Secret values are not supposed to be
// provided in the env files. However, users may do it anyway.
// Secret values will be provided as strings in those cases.
providedType = {
string: true,
boolean: false,
number: false,
list: false,
};
}
}
}
Expand Down

0 comments on commit 587e593

Please sign in to comment.