Skip to content

Commit 5caf8fa

Browse files
committed
feat: implement zero-wildcard project-scoped NG_ALLOWED_HOSTS and strict location checks for Angular SSR
1 parent 8492301 commit 5caf8fa

2 files changed

Lines changed: 44 additions & 30 deletions

File tree

src/deploy/apphosting/prepare.spec.ts

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -822,20 +822,20 @@ describe("apphosting", () => {
822822
readFileSyncStub = sinon.stub(fs, "readFileSync");
823823
});
824824

825-
it("should do nothing for non-Angular applications", () => {
825+
it("should do nothing for non-Angular applications", async () => {
826826
const cfg: AppHostingSingle = { backendId: "foo", rootDir: "/", ignore: [] };
827827
const buildEnv: Record<string, EnvMap> = { foo: {} };
828828
const runtimeEnv: Record<string, EnvMap> = { foo: {} };
829829

830830
existsStub.returns(false);
831831

832-
injectAngularEnvVars(cfg, "/app-dir", buildEnv, runtimeEnv);
832+
await injectAngularEnvVars(cfg, "/app-dir", "my-project", "us-central1", buildEnv, runtimeEnv);
833833

834834
expect(buildEnv["foo"]).to.be.empty;
835835
expect(runtimeEnv["foo"]).to.be.empty;
836836
});
837837

838-
it("should inject defaults for Angular applications when headers are missing", () => {
838+
it("should inject defaults for Angular applications when headers are missing", async () => {
839839
const cfg: AppHostingSingle = { backendId: "foo", rootDir: "/", ignore: [] };
840840
const buildEnv: Record<string, EnvMap> = { foo: {} };
841841
const runtimeEnv: Record<string, EnvMap> = { foo: {} };
@@ -849,19 +849,19 @@ describe("apphosting", () => {
849849
}),
850850
);
851851

852-
injectAngularEnvVars(cfg, "/app-dir", buildEnv, runtimeEnv);
852+
await injectAngularEnvVars(cfg, "/app-dir", "my-project", "us-central1", buildEnv, runtimeEnv);
853853

854854
expect(runtimeEnv["foo"]["NG_TRUST_PROXY_HEADERS"]).to.deep.equal({
855855
value: "x-forwarded-host,x-forwarded-port,x-forwarded-proto,x-forwarded-for",
856856
availability: ["RUNTIME"],
857857
});
858858
expect(runtimeEnv["foo"]["NG_ALLOWED_HOSTS"]).to.deep.equal({
859-
value: "*.hosted.app,*.run.app,*.firestack.app,localhost",
859+
value: "foo-123456789.us-central1.run.app,foo--my-project.us-central1.hosted.app,foo--my-project.web.app,foo--my-project.firebaseapp.com",
860860
availability: ["RUNTIME"],
861861
});
862862
});
863863

864-
it("should accept valid NG_TRUST_PROXY_HEADERS overrides", () => {
864+
it("should accept valid NG_TRUST_PROXY_HEADERS overrides", async () => {
865865
const cfg: AppHostingSingle = { backendId: "foo", rootDir: "/", ignore: [] };
866866
const buildEnv: Record<string, EnvMap> = { foo: {} };
867867
const runtimeEnv: Record<string, EnvMap> = {
@@ -880,14 +880,14 @@ describe("apphosting", () => {
880880
}),
881881
);
882882

883-
injectAngularEnvVars(cfg, "/app-dir", buildEnv, runtimeEnv);
883+
await injectAngularEnvVars(cfg, "/app-dir", "my-project", "us-central1", buildEnv, runtimeEnv);
884884

885885
expect(runtimeEnv["foo"]["NG_TRUST_PROXY_HEADERS"]?.value).to.equal(
886886
"X-Forwarded-Host,X-Forwarded-Proto",
887887
);
888888
});
889889

890-
it("should throw an error for invalid NG_TRUST_PROXY_HEADERS values", () => {
890+
it("should throw an error for invalid NG_TRUST_PROXY_HEADERS values", async () => {
891891
const cfg: AppHostingSingle = { backendId: "foo", rootDir: "/", ignore: [] };
892892
const buildEnv: Record<string, EnvMap> = { foo: {} };
893893
const runtimeEnv: Record<string, EnvMap> = {
@@ -906,12 +906,12 @@ describe("apphosting", () => {
906906
}),
907907
);
908908

909-
expect(() => {
910-
injectAngularEnvVars(cfg, "/app-dir", buildEnv, runtimeEnv);
911-
}).to.throw(FirebaseError, "invalid value for NG_TRUST_PROXY_HEADERS");
909+
await expect(
910+
injectAngularEnvVars(cfg, "/app-dir", "my-project", "us-central1", buildEnv, runtimeEnv)
911+
).to.be.rejectedWith(FirebaseError, "invalid value for NG_TRUST_PROXY_HEADERS");
912912
});
913913

914-
it("should throw an error if NG_TRUST_PROXY_HEADERS explicitly omits RUNTIME availability", () => {
914+
it("should throw an error if NG_TRUST_PROXY_HEADERS explicitly omits RUNTIME availability", async () => {
915915
const cfg: AppHostingSingle = { backendId: "foo", rootDir: "/", ignore: [] };
916916
const buildEnv: Record<string, EnvMap> = { foo: {} };
917917
const runtimeEnv: Record<string, EnvMap> = {
@@ -930,12 +930,12 @@ describe("apphosting", () => {
930930
}),
931931
);
932932

933-
expect(() => {
934-
injectAngularEnvVars(cfg, "/app-dir", buildEnv, runtimeEnv);
935-
}).to.throw(FirebaseError, "must include RUNTIME in its availability");
933+
await expect(
934+
injectAngularEnvVars(cfg, "/app-dir", "my-project", "us-central1", buildEnv, runtimeEnv)
935+
).to.be.rejectedWith(FirebaseError, "must include RUNTIME in its availability");
936936
});
937937

938-
it("should throw an error if NG_ALLOWED_HOSTS explicitly omits RUNTIME availability", () => {
938+
it("should throw an error if NG_ALLOWED_HOSTS explicitly omits RUNTIME availability", async () => {
939939
const cfg: AppHostingSingle = { backendId: "foo", rootDir: "/", ignore: [] };
940940
const buildEnv: Record<string, EnvMap> = { foo: {} };
941941
const runtimeEnv: Record<string, EnvMap> = {
@@ -954,15 +954,15 @@ describe("apphosting", () => {
954954
}),
955955
);
956956

957-
expect(() => {
958-
injectAngularEnvVars(cfg, "/app-dir", buildEnv, runtimeEnv);
959-
}).to.throw(
957+
await expect(
958+
injectAngularEnvVars(cfg, "/app-dir", "my-project", "us-central1", buildEnv, runtimeEnv)
959+
).to.be.rejectedWith(
960960
FirebaseError,
961961
"NG_ALLOWED_HOSTS environment variable must be set with RUNTIME availability",
962962
);
963963
});
964964

965-
it("should throw an error if NG_ALLOWED_HOSTS omits availability entirely (replicating the Go preparer slices.Contains(nil) quirk)", () => {
965+
it("should throw an error if NG_ALLOWED_HOSTS omits availability entirely (replicating the Go preparer slices.Contains(nil) quirk)", async () => {
966966
const cfg: AppHostingSingle = { backendId: "foo", rootDir: "/", ignore: [] };
967967
const buildEnv: Record<string, EnvMap> = { foo: {} };
968968
const runtimeEnv: Record<string, EnvMap> = {
@@ -981,15 +981,15 @@ describe("apphosting", () => {
981981
}),
982982
);
983983

984-
expect(() => {
985-
injectAngularEnvVars(cfg, "/app-dir", buildEnv, runtimeEnv);
986-
}).to.throw(
984+
await expect(
985+
injectAngularEnvVars(cfg, "/app-dir", "my-project", "us-central1", buildEnv, runtimeEnv)
986+
).to.be.rejectedWith(
987987
FirebaseError,
988988
"NG_ALLOWED_HOSTS environment variable must be set with RUNTIME availability",
989989
);
990990
});
991991

992-
it("should pass if NG_ALLOWED_HOSTS has explicit RUNTIME availability", () => {
992+
it("should pass if NG_ALLOWED_HOSTS has explicit RUNTIME availability", async () => {
993993
const cfg: AppHostingSingle = { backendId: "foo", rootDir: "/", ignore: [] };
994994
const buildEnv: Record<string, EnvMap> = { foo: {} };
995995
const runtimeEnv: Record<string, EnvMap> = {
@@ -1008,7 +1008,7 @@ describe("apphosting", () => {
10081008
}),
10091009
);
10101010

1011-
injectAngularEnvVars(cfg, "/app-dir", buildEnv, runtimeEnv);
1011+
await injectAngularEnvVars(cfg, "/app-dir", "my-project", "us-central1", buildEnv, runtimeEnv);
10121012

10131013
expect(runtimeEnv["foo"]["NG_ALLOWED_HOSTS"]?.value).to.equal("my.domain.com");
10141014
});

src/deploy/apphosting/prepare.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,11 @@ export default async function (context: Context, options: Options): Promise<void
198198
await injectAutoInitEnvVars(cfg, backends, buildEnv, runtimeEnv);
199199

200200
const rootDir = path.resolve(options.projectRoot || process.cwd());
201-
injectAngularEnvVars(cfg, rootDir, buildEnv, runtimeEnv);
201+
const location = context.backendLocations[cfg.backendId];
202+
if (!location) {
203+
throw new FirebaseError(`Failed to find location for backend ${cfg.backendId}.`);
204+
}
205+
await injectAngularEnvVars(cfg, rootDir, projectId, location, buildEnv, runtimeEnv);
202206
// Generate a static 8-character hash of the Workspace Root directory path to guarantee
203207
// 100% sibling folder build isolation on the same machine, while preserving predictability.
204208
const pathHash = crypto.createHash("md5").update(rootDir).digest("hex").substring(0, 8);
@@ -465,12 +469,14 @@ function isAngularApplication(appDir: string): boolean {
465469
/**
466470
* Replicates the Go buildpack preparer's Angular environment variable injection and validation.
467471
*/
468-
export function injectAngularEnvVars(
472+
export async function injectAngularEnvVars(
469473
cfg: AppHostingSingle,
470474
projectRoot: string,
475+
projectId: string,
476+
location: string,
471477
buildEnv: Record<string, EnvMap>,
472478
runtimeEnv: Record<string, EnvMap>,
473-
): void {
479+
): Promise<void> {
474480
const appDir = path.join(projectRoot, cfg.rootDir || "");
475481
if (!isAngularApplication(appDir)) {
476482
return;
@@ -532,12 +538,20 @@ export function injectAngularEnvVars(
532538
);
533539
}
534540
} else {
541+
const projectNumber = await getProjectNumber({ project: projectId });
542+
const sharedDomain = "hosted.app";
543+
const allowedHostsValue = [
544+
`${backendId}-${projectNumber}.${location}.run.app`,
545+
`${backendId}--${projectId}.${location}.${sharedDomain}`,
546+
`${backendId}--${projectId}.web.app`,
547+
`${backendId}--${projectId}.firebaseapp.com`,
548+
].join(",");
535549
logLabeledBullet(
536550
"apphosting",
537-
`Angular SSR application detected. Injecting default NG_ALLOWED_HOSTS=*.hosted.app,*.run.app,*.firestack.app,localhost`,
551+
`Angular SSR application detected. Injecting default NG_ALLOWED_HOSTS=${allowedHostsValue}`,
538552
);
539553
runtimeEnv[backendId]["NG_ALLOWED_HOSTS"] = {
540-
value: "*.hosted.app,*.run.app,*.firestack.app,localhost",
554+
value: allowedHostsValue,
541555
availability: ["RUNTIME"],
542556
};
543557
}

0 commit comments

Comments
 (0)