Skip to content

inject default Angular SSR environment variables for local builds#10706

Open
falahat wants to merge 9 commits into
mainfrom
local_builds_angular_ssr
Open

inject default Angular SSR environment variables for local builds#10706
falahat wants to merge 9 commits into
mainfrom
local_builds_angular_ssr

Conversation

@falahat

@falahat falahat commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Description

This adds NG_ALLOWED_HOSTS and NG_ALLOWED_PROXY_HEADERS similar to Firebase App Hosting Source Deploys. This is required for Angular SSR to work with Firebase Deployments.

Scenarios Tested

I tested Angular v19 through v22 with local vs source deploys and confirmed that the results were correct and the same (not falling back to CSR for local builds.)

Sample Commands

@wiz-9635d3485b

wiz-9635d3485b Bot commented Jun 24, 2026

Copy link
Copy Markdown

Wiz Scan Summary

Scanner Findings
Vulnerability Finding Vulnerabilities -
Data Finding Sensitive Data -
Secret Finding Secrets -
IaC Misconfiguration IaC Misconfigurations -
SAST Finding SAST Findings 4 Medium
Software Management Finding Software Management Findings -
Total 4 Medium

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

@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 replicates the Go buildpack preparer's Angular environment variable injection and validation for NG_TRUST_PROXY_HEADERS and NG_ALLOWED_HOSTS. The feedback recommends adding a defensive check to ensure the parsed package.json is a non-null object before accessing its properties, and extending the NG_TRUST_PROXY_HEADERS validation to check both runtimeEnv and buildEnv to prevent users from bypassing validation.

Comment thread src/deploy/apphosting/prepare.ts Outdated
Comment thread src/deploy/apphosting/prepare.ts Outdated
Comment thread src/deploy/apphosting/prepare.spec.ts Outdated
availability: ["RUNTIME"],
});
expect(runtimeEnv["foo"]["NG_ALLOWED_HOSTS"]).to.deep.equal({
value: "*.hosted.app,*.run.app,*.firestack.app,localhost",

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.

this is too permissive, I need to scope it to the user's project/backend, something like

[BACKEND_ID]--[PROJECT_ID].[REGION].[SHARED_DOMAIN],
[BACKEND_ID]--[PROJECT_ID].web.app,
[BACKEND_ID]--[PROJECT_ID].firebaseapp.com

@falahat falahat force-pushed the local_builds_angular_ssr branch from de626c2 to 5caf8fa Compare June 24, 2026 14:35
@sjjj986 sjjj986 self-assigned this Jun 24, 2026
@falahat falahat marked this pull request as ready for review June 24, 2026 18:48
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.

3 participants