Skip to content

Fix params.defineList cors parsed as undefined at deploy #1813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HassanBahati wants to merge 4 commits into
base: master
Choose a base branch
from
+118 −30
Open

Fix params.defineList cors parsed as undefined at deploy #1813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
09a3491
fix: params defineList cors request time
HassanBahati Feb 4, 2026
f29f704
fix(https): defer cors Expression resolution to request time
HassanBahati Feb 4, 2026
dee756c
fix(https): defer cors Expression resolution to request time
HassanBahati Feb 4, 2026
f8c5e69
fix(https): defer cors Expression resolution to request time
HassanBahati Feb 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/params/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,11 +634,19 @@

/** @internal */
runtimeValue(): string[] {
const val = JSON.parse(process.env[this.name]);
if (!Array.isArray(val) || !(val as string[]).every((v) => typeof v === "string")) {
return [];
const raw = process.env[this.name];
if (!raw) {
throw new Error(
`Parameter "${this.name}" is not set. Set it in .env or .env.local, or ensure the Functions runtime has provided it.`
);
}
const val = JSON.parse(raw);
if (!Array.isArray(val) || !val.every((v) => typeof v === "string")) {
throw new Error(
`Parameter "${this.name}" is not a valid JSON array of strings. Value is: ${raw}`
);
}
return val as string[];

Check failure on line 649 in src/params/types.ts

View workflow job for this annotation

GitHub Actions / lint (22.x) 

This assertion is unnecessary since it does not change the type of the expression
}

/** @hidden */
Expand Down
134 changes: 107 additions & 27 deletions src/v2/providers/https.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,64 @@
): { stream: AsyncIterable<Stream>; output: Return };
}

/**
* Builds a CORS origin callback for a static value (boolean, string, RegExp, or array).
* Used by onRequest and onCall for non-Expression cors; function form avoids CodeQL permissive CORS alert.
*/
function buildStaticCorsOriginCallback(
origin: string | boolean | RegExp | Array<string | RegExp>
): NonNullable<cors.CorsOptions["origin"]> {
return (reqOrigin: string | undefined, cb: (err: Error | null, allow?: boolean | string) => void) => {

Check failure on line 296 in src/v2/providers/https.ts

View workflow job for this annotation

GitHub Actions / lint (22.x) 

Replace `reqOrigin:·string·|·undefined,·cb:·(err:·Error·|·null,·allow?:·boolean·|·string)·=>·void` with `⏎····reqOrigin:·string·|·undefined,⏎····cb:·(err:·Error·|·null,·allow?:·boolean·|·string)·=>·void⏎··`
if (typeof origin === "boolean" || typeof origin === "string") {
return cb(null, origin);
}
if (reqOrigin === undefined) {
return cb(null, true);
}
if (origin instanceof RegExp) {
return cb(null, origin.test(reqOrigin) ? reqOrigin : false);
}
if (
Array.isArray(origin) &&
origin.some((o) => (typeof o === "string" ? o === reqOrigin : o.test(reqOrigin)))
) {
return cb(null, reqOrigin);
}
return cb(null, false);
};
}

/**
* Builds a CORS origin callback that resolves an Expression (e.g. defineList) at request time.
* Used by onRequest and onCall so params are not read during deployment.
*/
function buildCorsOriginFromExpression(
corsExpression: Expression<string | string[]>,
options: { respectCorsFalse?: boolean; corsOpt?: unknown }
): NonNullable<cors.CorsOptions["origin"]> {
return (reqOrigin: string | undefined, callback: (err: Error | null, allow?: boolean | string) => void) => {

Check failure on line 324 in src/v2/providers/https.ts

View workflow job for this annotation

GitHub Actions / lint (22.x) 

Replace `reqOrigin:·string·|·undefined,·callback:·(err:·Error·|·null,·allow?:·boolean·|·string)·=>·void` with `⏎····reqOrigin:·string·|·undefined,⏎····callback:·(err:·Error·|·null,·allow?:·boolean·|·string)·=>·void⏎··`
if (isDebugFeatureEnabled("enableCors") && (!options.respectCorsFalse || options.corsOpt !== false)) {

Check failure on line 325 in src/v2/providers/https.ts

View workflow job for this annotation

GitHub Actions / lint (22.x) 

Replace `isDebugFeatureEnabled("enableCors")·&&·(!options.respectCorsFalse·||·options.corsOpt·!==·false)` with `⏎······isDebugFeatureEnabled("enableCors")·&&⏎······(!options.respectCorsFalse·||·options.corsOpt·!==·false)⏎····`
callback(null, true);
return;
}
const resolved = corsExpression.runtimeValue();
if (Array.isArray(resolved)) {
if (resolved.length === 1) {
callback(null, resolved[0]);
return;
}
if (reqOrigin === undefined) {
callback(null, true);
return;
}
const allowed = resolved.indexOf(reqOrigin) !== -1;
callback(null, allowed ? reqOrigin : false);
} else {
callback(null, resolved as string);

Check failure on line 342 in src/v2/providers/https.ts

View workflow job for this annotation

GitHub Actions / lint (22.x) 

This assertion is unnecessary since it does not change the type of the expression
}
};
}

/**
* Handles HTTPS requests.
* @param opts - Options to set on this function
Expand Down Expand Up @@ -324,18 +382,32 @@
handler = withErrorHandler(handler);

if (isDebugFeatureEnabled("enableCors") || "cors" in opts) {
let origin = opts.cors instanceof Expression ? opts.cors.value() : opts.cors;
if (isDebugFeatureEnabled("enableCors")) {
// Respect `cors: false` to turn off cors even if debug feature is enabled.
origin = opts.cors === false ? false : true;
}
// Arrays cause the access-control-allow-origin header to be dynamic based
// on the origin header of the request. If there is only one element in the
// array, this is unnecessary.
if (Array.isArray(origin) && origin.length === 1) {
origin = origin[0];
let corsOptions: cors.CorsOptions;
if (opts.cors instanceof Expression) {
// Defer resolution to request time so params are not read during deployment.
corsOptions = {
origin: buildCorsOriginFromExpression(opts.cors, {
respectCorsFalse: true,
corsOpt: opts.cors,
}),
};
} else {
let origin = opts.cors;
if (isDebugFeatureEnabled("enableCors")) {
// Respect `cors: false` to turn off cors even if debug feature is enabled.
origin = opts.cors === false ? false : true;
}
// Arrays cause the access-control-allow-origin header to be dynamic based
// on the origin header of the request. If there is only one element in the
// array, this is unnecessary.
if (Array.isArray(origin) && origin.length === 1) {
origin = origin[0];
}
corsOptions = {
origin: buildStaticCorsOriginCallback(origin),
};
}
const middleware = cors({ origin });
const middleware = cors(corsOptions);

const userProvidedHandler = handler;
handler = (req: Request, res: express.Response): void | Promise<void> => {
Expand Down Expand Up @@ -434,30 +506,38 @@
opts = optsOrHandler as CallableOptions;
}

let cors: string | boolean | RegExp | Array<string | RegExp> | undefined;
if ("cors" in opts) {
if (opts.cors instanceof Expression) {
cors = opts.cors.value();
let corsOptions: cors.CorsOptions;
if ("cors" in opts && opts.cors instanceof Expression) {
// Defer resolution to request time so params are not read during deployment.
corsOptions = {
origin: buildCorsOriginFromExpression(opts.cors, {}),
methods: "POST",
};
} else {
let cors: string | boolean | RegExp | Array<string | RegExp> | undefined;
if ("cors" in opts) {
cors = opts.cors as string | boolean | RegExp | Array<string | RegExp>;
} else {
cors = opts.cors;
cors = true;
}
} else {
cors = true;
}

let origin = isDebugFeatureEnabled("enableCors") ? true : cors;
// Arrays cause the access-control-allow-origin header to be dynamic based
// on the origin header of the request. If there is only one element in the
// array, this is unnecessary.
if (Array.isArray(origin) && origin.length === 1) {
origin = origin[0];
let origin = isDebugFeatureEnabled("enableCors") ? true : cors;
Comment on lines +517 to +523
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 4, 2026

Choose a reason for hiding this comment

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

medium

There's an inconsistency in how the enableCors debug flag interacts with cors: false between onCall and onRequest. In onRequest, cors: false is respected even when the debug flag is on, correctly disabling CORS. However, in onCall, the current logic allows the debug flag to override cors: false, which enables CORS contrary to the explicit configuration. To ensure consistent behavior across both function types, I suggest refactoring this logic to align with the implementation in onRequest.

    let origin = "cors" in opts ? opts.cors : true;
    if (isDebugFeatureEnabled("enableCors")) {
      // Respect `cors: false` to turn off cors even if debug feature is enabled.
      origin = opts.cors === false ? false : true;
    }

// Arrays cause the access-control-allow-origin header to be dynamic based
// on the origin header of the request. If there is only one element in the
// array, this is unnecessary.
if (Array.isArray(origin) && origin.length === 1) {
origin = origin[0];
}
corsOptions = {
origin: buildStaticCorsOriginCallback(origin),
methods: "POST",
};
}

// fix the length of handler to make the call to handler consistent
const fixedLen = (req: CallableRequest<T>, resp?: CallableResponse<Stream>) => handler(req, resp);
let func: any = onCallHandler(
{
cors: { origin, methods: "POST" },
cors: corsOptions,
enforceAppCheck: opts.enforceAppCheck ?? options.getGlobalOptions().enforceAppCheck,
consumeAppCheckToken: opts.consumeAppCheckToken,
heartbeatSeconds: opts.heartbeatSeconds,
Expand Down
Loading