Skip to content

fix(@angular/build): serialize server manifest asset paths - #33901

Closed
Hexix23 wants to merge 1 commit into
angular:mainfrom
Hexix23:fix/angular-build-serialize-server-asset-paths
Closed

fix(@angular/build): serialize server manifest asset paths#33901
Hexix23 wants to merge 1 commit into
angular:mainfrom
Hexix23:fix/angular-build-serialize-server-asset-paths

Conversation

@Hexix23

@Hexix23 Hexix23 commented Aug 23, 2026

Copy link
Copy Markdown

PR Checklist

Please check to confirm your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Prerendered route parameters can become HTML output paths. The server manifest
generator then embeds each output path into executable JavaScript in two places
without serializing it as a JavaScript string.

The vulnerable code is in
generateAngularServerAppManifest():

serverAssets[file.path] =
  `{size: ${size}, hash: '${file.hash}', text: () => import('./${jsChunkFilePath}').then(m => m.default)}`;

// ...

Object.entries(serverAssets)
  .map(([key, value]) => `'${key}': ${value}`)

file.path is used as a single-quoted object key and its derived chunk path is
used as a single-quoted dynamic import specifier. A quote or line terminator in
a route-derived output path is therefore interpreted as JavaScript syntax
instead of remaining string data.

The source-to-sink path is:

getPrerenderParams() route value
  -> prerendered <route>/index.html output path
  -> additionalHtmlOutputFiles
  -> generateAngularServerAppManifest()
  -> unescaped object key and dynamic import specifier
  -> executable angular-app-manifest.mjs

Because main.server.mjs imports the generated manifest, injected source can
execute when the emitted Node server artifact is imported or started. The
build itself does not need to execute the inserted source.

Issue Number: N/A

What is the new behavior?

Both route-derived path uses are emitted with JSON.stringify(). This keeps
quotes, line terminators, backslashes, and other JavaScript-significant
characters inside string literals.

The regression test supplies a file-system-valid asset path containing an
apostrophe, verifies the exact serialized object key and import specifier, and
parses the complete generated manifest with esbuild.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

The affected path is part of the standard application builder's SSR/prerender
flow. The fix does not change safe asset paths, chunk naming, route semantics,
or the generated manifest API.

Validation:

pnpm bazel test //packages/angular/build:test --test_filter='generateAngularServerAppManifest'

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

Copy link
Copy Markdown

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 updates the Angular server app manifest generation to serialize asset paths and chunk paths using JSON.stringify before embedding them in the executable manifest, preventing syntax errors when paths contain special characters like quotes or newlines. A unit test has been added to verify this behavior. Feedback suggests that the sanitization logic for chunk filenames should be improved to replace all non-alphanumeric/safe characters, as filenames containing newlines or special characters can cause write failures on Windows and other filesystems.

Comment on lines +15 to +16
const assetPath = "products/quote's\nname/index.html";
const chunkPath = `assets-chunks/${assetPath.replace(/[./]/g, '_')}.mjs`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test uses an assetPath containing a single quote and a newline character, which results in a chunkPath (and thus a physical filename) containing these characters because the sanitization replace(/[./]/g, '_') only replaces dots and slashes.\n\nIn a real build, attempting to write a file with a newline or other invalid characters in its filename to the disk will fail on Windows and potentially other filesystems.\n\nConsider updating the sanitization logic in manifest.ts (line 166) to replace all non-alphanumeric/safe characters (e.g., using /[^a-zA-Z0-9_-]/g) to ensure that the generated chunk filenames are safe for all filesystems.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. The line terminator was only intended to exercise JavaScript serialization, but it is not a portable filename. The regression now uses a single quote, which is valid in filenames on the supported platforms and is sufficient to break the previous single-quoted JavaScript literal. I kept the existing chunk-name mapping unchanged so this fix remains limited to JavaScript serialization and does not introduce new filename collision behavior.

Serialize route-derived asset paths before embedding them in the executable server manifest.

Keep object keys and dynamic import specifiers as JavaScript string data.
@Hexix23
Hexix23 force-pushed the fix/angular-build-serialize-server-asset-paths branch from 59050d9 to 7ac0aa0 Compare August 23, 2026 22:30
@alan-agius4

Copy link
Copy Markdown
Collaborator

Thank you for opening this PR.

We do not consider this to be a security vulnerability. Prerender route parameters and build inputs originate from the application's source code and configuration, which are trusted and already execute within the build environment. There is no trust boundary crossed here that would allow an untrusted external actor to achieve arbitrary code execution. If a route path were to contain quotes or unescaped characters, it would result in a build-time/startup syntax error rather than a security compromise.

Since this is framed around an inapplicable threat model and does not address an active real-world bug or open issue, we are going to close this PR. If you encounter a practical, real-world bug with asset or route path serialization, please feel free to open an issue with a minimal reproduction.

Thanks again!

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