Skip to content

warn about missing SSRF protection config in angular v22#10523

Draft
leoortizz wants to merge 1 commit into
mainfrom
leoortizz_angular22warning
Draft

warn about missing SSRF protection config in angular v22#10523
leoortizz wants to merge 1 commit into
mainfrom
leoortizz_angular22warning

Conversation

@leoortizz
Copy link
Copy Markdown
Member

Angular 22 enabled strict SSRF protection on its SSR engine. Without explicit configuration, Firebase Hosting SSR functions will return errors due to rotating Cloud Run hostnames.

  • Add security pre-flight check for Angular 22 SSR builds
  • Detect missing security.allowedHosts in angular.json
  • Detect missing trustProxyHeaders in server entry point
  • Surface cohesive, indented warnings with links to official docs
  • Add unit tests

Description

Scenarios Tested

Sample Commands

Angular 22 enabled strict SSRF protection on its SSR engine. Without
explicit configuration, Firebase Hosting SSR functions will return
errors due to rotating Cloud Run hostnames.

- Add security pre-flight check for Angular 22 SSR builds
- Detect missing `security.allowedHosts` in angular.json
- Detect missing `trustProxyHeaders` in server entry point
- Surface cohesive, indented warnings with links to official docs
- Add unit tests
@wiz-9635d3485b
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 1 Medium
Software Management Finding Software Management Findings -
Total 1 Medium

View scan details in Wiz

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

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

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 implements a security pre-flight check for Angular 22 SSR projects to ensure they are correctly configured for Firebase Hosting. It adds logic to detect missing allowedHosts and trustProxyHeaders settings, which are necessary for SSR to work with Cloud Run's rotating hostnames. Feedback from the review focuses on improving the accuracy of configuration detection through more specific regex patterns, optimizing build performance by eliminating redundant context resolution, and increasing robustness by dynamically identifying the server entry point from project metadata instead of relying on hardcoded file paths.


const declared = opts.buildOptionsAllowedHosts ?? [];
const serverHasAllowedHosts =
!!opts.serverEntrySource && /\ballowedHosts\b/.test(opts.serverEntrySource);
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.

medium

The regex /\ballowedHosts\b/ is quite broad and may lead to false negatives (suppressing the warning) if the string appears in comments or unrelated variable names. Consider using a more specific pattern that targets property assignments or object keys to improve the accuracy of the detection.

Suggested change
!!opts.serverEntrySource && /\ballowedHosts\b/.test(opts.serverEntrySource);
!!opts.serverEntrySource && /\ballowedHosts\s*[:=]/.test(opts.serverEntrySource);

}

const trustProxyHeadersMissing =
!opts.serverEntrySource || !/\btrustProxyHeaders\b/.test(opts.serverEntrySource);
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.

medium

Similar to allowedHosts, the regex for trustProxyHeaders is very loose. Using a more specific pattern that looks for property assignment will reduce the chance of false negatives caused by comments or unrelated code.

Suggested change
!opts.serverEntrySource || !/\btrustProxyHeaders\b/.test(opts.serverEntrySource);
!opts.serverEntrySource || !/\btrustProxyHeaders\s*[:=]/.test(opts.serverEntrySource);

Comment on lines +755 to +756
const buildOptionsAllowedHosts = await io.readBuildOptionsAllowedHosts(dir, configuration);
const serverEntrySource = await io.readServerEntrySource(dir);
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.

medium

Calling io.readBuildOptionsAllowedHosts here triggers a call to getContext, which performs expensive operations like reading and parsing angular.json. Since getContext is already called during getBuildConfig in the main build flow, this results in redundant work. Consider refactoring to pass the already resolved context or necessary options to this function to improve build performance.

export async function readAngular22ServerEntrySource(dir: string): Promise<string | undefined> {
// Default Angular SSR layout has src/server.ts; fall back to .mjs/.js if the
// user converted to one of those.
for (const candidate of ["src/server.ts", "src/server.mjs", "src/server.js"]) {
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.

medium

Hardcoding the server entry path to src/server.* may result in the security check being skipped for projects with custom directory structures. Since the angular.json configuration is already being parsed in this flow, it would be more robust to retrieve the actual entry point from the build target options (e.g., options.ssr.entry for the application builder).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants