Skip to content

fix(ng-dev): prevent arbitrary file write via symlink#3734

Open
josephperrott wants to merge 2 commits into
angular:mainfrom
josephperrott:security-fix
Open

fix(ng-dev): prevent arbitrary file write via symlink#3734
josephperrott wants to merge 2 commits into
angular:mainfrom
josephperrott:security-fix

Conversation

@josephperrott
Copy link
Copy Markdown
Member

This PR addresses a vulnerability where arbitrary file writes/information leaks could occur via symlink dereferencing when copying user-controlled files into the optimization build output.

Changes:

  • Removed the -L (dereference) flag from cp -Rf in optimize.sh to prevent copying symlink targets.
  • Added a pre-build genrule in index.bzl that acts as a strict security gatekeeper. It validates the original workspace srcs and securely aborts the build if any of the underlying source files is a symlink, fulfilling strict architectural security requirements while maintaining compatibility with Bazel's internal sandbox symlinking.

@josephperrott josephperrott added the action: merge The PR is ready for merge by the caretaker label Jun 4, 2026
@josephperrott josephperrott requested a review from alan-agius4 June 4, 2026 15:55
Copy link
Copy Markdown

@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 introduces a security check to prevent symbolic links in user input by adding a genrule in index.bzl and removing the -L flag from the copy command in optimize.sh. A critical security issue was identified in the genrule where relative symlinks are resolved incorrectly relative to the execroot instead of the symlink's directory, potentially bypassing the check. A code suggestion has been provided to resolve this path resolution issue.

Comment thread bazel/rules/rules_angular/src/optimization/index.bzl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant