diff --git a/spec/v2/providers/https.spec.ts b/spec/v2/providers/https.spec.ts index a62dfea5a..d683f4b64 100644 --- a/spec/v2/providers/https.spec.ts +++ b/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", () => { diff --git a/src/params/types.ts b/src/params/types.ts index 946cedf51..62fbae872 100644 --- a/src/params/types.ts +++ b/src/params/types.ts @@ -732,10 +732,10 @@ export class ListParam extends Param { /** @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 */ diff --git a/src/v2/providers/https.ts b/src/v2/providers/https.ts index cfb3cfee3..dc659bb6d 100644 --- a/src/v2/providers/https.ts +++ b/src/v2/providers/https.ts @@ -286,6 +286,79 @@ export interface CallableFunction extends HttpsFunc ): { stream: AsyncIterable; 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 +): NonNullable { + 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, + options: { respectCorsFalse?: boolean; corsOpt?: unknown } +): NonNullable { + 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 => { @@ -434,23 +521,38 @@ export function onCall, Stream = unknown>( opts = optsOrHandler as CallableOptions; } - let cors: string | boolean | RegExp | Array | 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 | undefined; + if ("cors" in opts) { + cors = opts.cors as string | boolean | RegExp | Array; } 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, Stream = unknown>( let func: any = onCallHandler( { - cors: { origin, methods: "POST" }, + cors: corsOptions, enforceAppCheck, consumeAppCheckToken, heartbeatSeconds: opts.heartbeatSeconds,