Fix params.defineList cors parsed as undefined at deploy

spec/v2/providers/https.spec.ts

@@ -68,6 +68,7 @@ describe("onRequest", () => {
   afterEach(() => {
     options.setGlobalOptions({});
     delete process.env.GCLOUD_PROJECT;
+    sinon.restore();
   });
 
   it("should return a minimal trigger/endpoint with appropriate values", () => {
@@ -290,8 +291,6 @@ describe("onRequest", () => {
       "Content-Length": "0",
       Vary: "Origin, Access-Control-Request-Headers",
     });
-
-    sinon.restore();
   });
 
   it("should NOT add CORS headers if debug feature is enabled and cors has value false", async () => {
@@ -314,8 +313,6 @@ describe("onRequest", () => {
     expect(resp.status).to.equal(200);
     expect(resp.body).to.be.equal("Good");
     expect(resp.headers).to.deep.equal({});
-
-    sinon.restore();
   });
 
   it("calls init function", async () => {
@@ -336,6 +333,84 @@ describe("onRequest", () => {
     await runHandler(func, req);
     expect(hello).to.equal("world");
   });
+
+  it("does not eagerly resolve defineList cors during onRequest definition", () => {
+    const allowedOrigins = defineList("ALLOWED_ORIGINS");
+    const runtimeValueStub = sinon.stub(allowedOrigins, "runtimeValue");
+    const valueSpy = sinon.spy(allowedOrigins, "value");
+
+    expect(() => {
+      https.onRequest({ cors: allowedOrigins }, (_req, res) => {
+        res.status(200).send("ok");
+      });
+    }).to.not.throw();
+
+    expect(valueSpy.called).to.be.false;
+    expect(runtimeValueStub.called).to.be.false;
+  });
+
+  it("treats a single-element expression list like a static single origin", async () => {
+    const allowedOrigins = defineList("ALLOWED_ORIGINS");
+    sinon.stub(allowedOrigins, "runtimeValue").returns(["https://app.example.com"]);
+
+    const func = https.onRequest({ cors: allowedOrigins }, (req, res) => {
+      res.status(200).send("ok");
+    });
+
+    const req = request({
+      headers: {
+        "Access-Control-Request-Method": "POST",
+        "Access-Control-Request-Headers": "origin",
+        origin: "https://evil.example.com",
+      },
+      method: "OPTIONS",
+    });
+
+    const resp = await runHandler(func, req);
+
+    expect(resp.status).to.equal(204);
+    expect(resp.headers["Access-Control-Allow-Origin"]).to.equal("https://app.example.com");
+  });
+
+  it("matches request origin for multi-element expression-backed cors", async () => {
+    const allowedOrigins = defineList("ALLOWED_ORIGINS");
+    const runtimeValueStub = sinon
+      .stub(allowedOrigins, "runtimeValue")
+      .returns(["https://app.example.com", "https://admin.example.com"]);
+
+    const func = https.onRequest({ cors: allowedOrigins }, (_req, res) => {
+      res.status(200).send("ok");
+    });
+
+    const allowedReq = request({
+      headers: {
+        "Access-Control-Request-Method": "POST",
+        "Access-Control-Request-Headers": "origin",
+        origin: "https://admin.example.com",
+      },
+      method: "OPTIONS",
+    });
+
+    const allowedResp = await runHandler(func, allowedReq);
+    expect(allowedResp.status).to.equal(204);
+    expect(allowedResp.headers["Access-Control-Allow-Origin"]).to.equal(
+      "https://admin.example.com"
+    );
+
+    const deniedReq = request({
+      headers: {
+        "Access-Control-Request-Method": "POST",
+        "Access-Control-Request-Headers": "origin",
+        origin: "https://evil.example.com",
+      },
+      method: "OPTIONS",
+    });
+
+    const deniedResp = await runHandler(func, deniedReq);
+    expect(deniedResp.status).to.equal(200);
+    expect(deniedResp.headers["Access-Control-Allow-Origin"]).to.be.undefined;
+    expect(runtimeValueStub.calledTwice).to.be.true;
+  });
 });
 
 describe("onCall", () => {
@@ -352,6 +427,7 @@ describe("onCall", () => {
     delete process.env.GCLOUD_PROJECT;
     delete process.env.ORIGINS;
     clearParams();
+    sinon.restore();
   });
 
   it("should return a minimal trigger/endpoint with appropriate values", () => {
@@ -569,8 +645,25 @@ describe("onCall", () => {
       "Content-Length": "0",
       Vary: "Origin, Access-Control-Request-Headers",
     });
+  });
 
-    sinon.restore();
+  it("should NOT add CORS headers if debug feature is enabled and cors has value false", async () => {
+    sinon.stub(debug, "isDebugFeatureEnabled").withArgs("enableCors").returns(true);
+
+    const func = https.onCall({ cors: false }, () => 42);
+    const req = request({
+      data: {},
+      headers: {
+        origin: "example.com",
+        "content-type": "application/json",
+      },
+      method: "POST",
+    });
+
+    const response = await runHandler(func, req);
+
+    expect(response.status).to.equal(200);
+    expect(response.headers).to.not.have.property("Access-Control-Allow-Origin");
   });
 
   it("adds CORS headers", async () => {
@@ -606,14 +699,10 @@ describe("onCall", () => {
   });
 
   describe("authPolicy", () => {
-    before(() => {
+    beforeEach(() => {
       sinon.stub(debug, "isDebugFeatureEnabled").withArgs("skipTokenVerification").returns(true);
     });
 
-    after(() => {
-      sinon.restore();
-    });
-
     it("should check isSignedIn", async () => {
       const func = https.onCall(
         {
@@ -673,6 +762,19 @@ describe("onCall", () => {
       expect(accessDenied.status).to.equal(403);
     });
   });
+
+  it("does not eagerly resolve defineList cors during onCall definition", () => {
+    const allowedOrigins = defineList("ALLOWED_ORIGINS");
+    const runtimeValueStub = sinon.stub(allowedOrigins, "runtimeValue");
+    const valueSpy = sinon.spy(allowedOrigins, "value");
+
+    expect(() => {
+      https.onCall({ cors: allowedOrigins }, () => 42);
+    }).to.not.throw();
+
+    expect(valueSpy.called).to.be.false;
+    expect(runtimeValueStub.called).to.be.false;
+  });
 });
 
 describe("onCallGenkit", () => {

src/params/types.ts

@@ -732,10 +732,10 @@ export class ListParam extends Param<string[]> {
   /** @internal */
   runtimeValue(): string[] {
     const val = JSON.parse(process.env[this.name]);
-    if (!Array.isArray(val) || !(val as string[]).every((v) => typeof v === "string")) {
+    if (!Array.isArray(val) || !val.every((v) => typeof v === "string")) {
       return [];
     }
-    return val as string[];
+    return val;
   }
 
   /** @hidden */

src/v2/providers/https.ts

@@ -286,6 +286,79 @@ export interface CallableFunction<T, Return, Stream = unknown> extends HttpsFunc
   ): { 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
+  ) => {
+    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
+  ) => {
+    if (
+      isDebugFeatureEnabled("enableCors") &&
+      (!options.respectCorsFalse || options.corsOpt !== false)
+    ) {
+      callback(null, true);
+      return;
+    }
+    let resolved: string | string[];
+    try {
+      resolved = corsExpression.runtimeValue();
+    } catch (err) {
+      callback(err instanceof Error ? err : new Error(String(err)), false);
+      return;
+    }
+    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);
+    }
+  };
+}
+
 /**
  * Handles HTTPS requests.
  * @param opts - Options to set on this function
@@ -324,18 +397,32 @@ export function onRequest(
   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> => {
@@ -434,23 +521,38 @@ export function onCall<T = any, Return = any | Promise<any>, Stream = unknown>(
     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, {
+        respectCorsFalse: true,
+        corsOpt: 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 = 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),
+      methods: "POST",
+    };
   }
 
   // fix the length of handler to make the call to handler consistent
@@ -468,7 +570,7 @@ export function onCall<T = any, Return = any | Promise<any>, Stream = unknown>(
 
   let func: any = onCallHandler(
     {
-      cors: { origin, methods: "POST" },
+      cors: corsOptions,
       enforceAppCheck,
       consumeAppCheckToken,
       heartbeatSeconds: opts.heartbeatSeconds,