Skip to content

.env-based secret param binding for Functions#10714

Open
Berlioz wants to merge 1 commit into
mainfrom
vsfan_env_params_use
Open

.env-based secret param binding for Functions#10714
Berlioz wants to merge 1 commit into
mainfrom
vsfan_env_params_use

Conversation

@Berlioz

@Berlioz Berlioz commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Pulls &-defined secrets from the Functions .env files and propagates them into the Functions Build's SecretEnvVars (and from there, into the resolved Backend, and from there, into the JSON uploaded to Cloud Functions/Cloud Run).

Experimental-flagged.

Params support not included in this PR, to reduce the size of the review. This is obviously going to be a terrible user experience before params are supported (hence the flag) but this should be enough to test the actual binding behavior.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces experimental support (functionsEnvParams) for storing references to Secrets in .env files for Cloud Functions deployments. It partitions secret references (prefixed with &) from standard environment variables, parses and validates them, and applies overrides to the build's secret environment variables. The review feedback identifies several critical issues in the validation and parsing logic: 1) validateKey will fail on keys starting with & unless the prefix is stripped first; 2) the regex anchors \A and \z are unsupported in JavaScript and will cause version validation to fail; 3) SECRET_REF_RE contains a redundant double ^ anchor and is too restrictive to support common secret names containing uppercase letters or underscores; and 4) overriding an existing secret with an empty reference will incorrectly set the secret name to an empty string instead of defaulting to the key name.

Comment thread src/functions/env.ts
Comment on lines +248 to 251
const validationErrors: (KeyValidationError | SecretValidationError)[] = [];
for (const key of Object.keys(envs)) {
try {
validateKey(key);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

When parseStrict is called, it iterates over all keys in envs and calls validateKey(key). If a key starts with & (e.g., &API_KEY), validateKey will throw a KeyValidationError because & is not a valid character according to the regex /^[A-Z_][A-Z0-9_]*$/. This prevents any successful deployment when secrets are defined in the .env file. We should strip the leading & before validating the key.

Suggested change
const validationErrors: (KeyValidationError | SecretValidationError)[] = [];
for (const key of Object.keys(envs)) {
try {
validateKey(key);
const validationErrors: (KeyValidationError | SecretValidationError)[] = [];
for (const key of Object.keys(envs)) {
try {
validateKey(key.startsWith("&") ? key.slice(1) : key);

Comment thread src/functions/env.ts
if (!version) {
return;
}
if (version !== "latest" && !/\A\d+\z/.test(version)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The regular expression \A\d+\z uses \A and \z anchors, which are not supported in JavaScript/TypeScript RegExp. In JS, \A is treated as a literal A and \z as a literal z, meaning this check will always fail for valid integer versions (e.g., 4). Use standard JS anchors ^ and $ instead.

Suggested change
if (version !== "latest" && !/\A\d+\z/.test(version)) {
if (version !== "latest" && !/^\d+$/.test(version)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they what??????????

Comment thread src/functions/env.ts
Comment on lines +65 to +70
const SECRET_REF_RE = new RegExp(
"^" + // begin line
"^[a-z]([-a-z0-9]*[a-z0-9])?" + // cloud resource ID
"(?:@(.+))?" + // optional reference to a specific version
"$", // end line
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The SECRET_REF_RE regular expression has a redundant double ^ anchor due to string concatenation ("^" + "^[a-z]..."). Additionally, it is too restrictive as it does not allow uppercase letters or underscores, which are extremely common in secret names (e.g., STRIPE_SECRET). We should simplify the regex and allow alphanumeric characters, underscores, and hyphens.

Suggested change
const SECRET_REF_RE = new RegExp(
"^" + // begin line
"^[a-z]([-a-z0-9]*[a-z0-9])?" + // cloud resource ID
"(?:@(.+))?" + // optional reference to a specific version
"$", // end line
);
const SECRET_REF_RE = new RegExp(
"^" + // begin line
"[a-zA-Z0-9_-]+" + // cloud resource ID
"(?:@(.+))?" + // optional reference to a specific version
"$", // end line
);

endpoint.secretEnvironmentVariables?.forEach((envVar) => {
if (envVar.key === key) {
needEnvInsert = false;
envVar.secret = resourceId;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

When overriding an existing secret environment variable, if secretRef is empty (e.g., &FOO=), resourceId will be "". Setting envVar.secret = resourceId; directly will overwrite the secret name with an empty string, which is invalid. It should default to the key name, matching the behavior used when inserting a new secret environment variable.

Suggested change
envVar.secret = resourceId;
envVar.secret = resourceId === "" ? key : resourceId;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants