Skip to content

fix(@angular/build): prevent deleting parent directories of project root#32873

Open
pradhankukiran wants to merge 5 commits intoangular:mainfrom
pradhankukiran:fix-delete-output-dir-ancestor-check
Open

fix(@angular/build): prevent deleting parent directories of project root#32873
pradhankukiran wants to merge 5 commits intoangular:mainfrom
pradhankukiran:fix-delete-output-dir-ancestor-check

Conversation

@pradhankukiran
Copy link
Copy Markdown

@pradhankukiran pradhankukiran commented Mar 27, 2026

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?

The deleteOutputDir utility only checks if the output path is exactly equal to the project root. Setting the output path to a parent directory (e.g. ../ or ../../) bypasses this check, and ng build silently deletes all contents of that directory potentially wiping out source files or entire directory trees.

Closes: #6485

What is the new behavior?

Uses path.relative() to detect when the resolved output path is the project root OR any ancestor of it, and throws a descriptive error in both cases.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

The original issue reported a user losing 10 days of work after setting outDir: "../../" which caused ng build --prod to delete their entire project directory. This fix closes the gap that has been open since 2017.

@google-cla
Copy link
Copy Markdown

google-cla bot commented Mar 27, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

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 updates the deleteOutputDir utility to prevent the deletion of the project root or its parent directories by using path.relative for validation. It also introduces a new test suite to verify these safety constraints and general functionality. A suggestion was made to move a dynamic import to the top of the test file for better consistency.

@alan-agius4 alan-agius4 added the target: patch This PR is targeted for the next patch release label Mar 27, 2026
@alan-agius4 alan-agius4 added merge: squash commits When the PR is merged, a squash and merge should be performed action: merge The PR is ready for merge by the caretaker and removed merge: squash commits When the PR is merged, a squash and merge should be performed labels Mar 27, 2026
Previously, the output path validation only checked for an exact match
against the project root. This allowed ancestor directories (e.g. ../
or ../../) to be used as the output path, which would silently delete
all their contents including source files.

Use path.relative() to detect when the output path is the project root
or any ancestor of it, and throw a descriptive error in both cases.

Fixes angular#6485
@alan-agius4 alan-agius4 force-pushed the fix-delete-output-dir-ancestor-check branch from 38130b7 to 6c745cc Compare March 27, 2026 12:40
@alan-agius4 alan-agius4 removed the action: merge The PR is ready for merge by the caretaker label Mar 27, 2026
Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

Had a chat with @clydin about this and he correctly pointed out that this will break use-cases where the provided outputPath is absolute.

@pradhankukiran
Copy link
Copy Markdown
Author

Thanks for the review! I tested absolute output paths and they seem to work correctly /tmp/build and /home/other/build are allowed, only ancestors like /home/user (parent of /home/user/project) are blocked. Could you share a specific example of the absolute path case that would break? Happy to adjust.

@alan-agius4
Copy link
Copy Markdown
Collaborator

@pradhankukiran, based on the logic there is no special handling of absolute paths and thus it will fail. All absolute paths should be allowed.

Example:

deleteOutputDir('/my-project/app', '/tmp')

@pradhankukiran
Copy link
Copy Markdown
Author

I tested your exact example deleteOutputDir('/my-project/app', '/tmp') is allowed by the check. relative('/tmp', '/my-project/app') returns ../my-project/app which starts with .., so it passes. The check only blocks when the project root is inside the output path (the output path is an ancestor). Happy to add a test case for this if it helps.

@clydin
Copy link
Copy Markdown
Member

clydin commented Mar 27, 2026

On Windows, drive letters and UNC paths are also absolute and should be handled here as well.

@alan-agius4
Copy link
Copy Markdown
Collaborator

alan-agius4 commented Mar 27, 2026

@pradhankukiran, as @clydin mentioned absolute paths needs to be explicitly handled.

@pradhankukiran
Copy link
Copy Markdown
Author

Thanks for the feedback! I've added an isAbsolute() guard so that Windows drive letters and UNC paths are correctly allowed. Also added a test case for absolute paths outside the project. The nit is applied too

const resolvedOutputPath = resolve(root, outputPath);
if (resolvedOutputPath === root) {
throw new Error('Output path MUST not be project root directory!');
const relativePath = relative(resolvedOutputPath, root);
Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 Mar 27, 2026

Choose a reason for hiding this comment

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

NIT: I think this can be slightly simplified, made easier to follow and also make the errors more actionable.

const resolvedOutputPath = resolve(root, outputPath);
if (resolvedOutputPath === root) {
  throw new Error("Output path MUST not be workspace root directory.");
}

if (!isAbsolute(outputPath) && !resolvedOutputPath.startsWith(root)) {
  throw new Error(
    `Output path "${resolvedOutputPath}" must NOT be a parent of the workspace root directory via relative paths.`
  );
}

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 for the suggestion! I think there might be a couple of edge cases with this approach:

  1. Absolute ancestors are not caught — deleteOutputDir('/home/user/project', '/home') skips the second check because isAbsolute('/home') is true, so the ancestor /home is silently allowed.
  2. Relative sibling paths are incorrectly blocked — deleteOutputDir('/home/user/project', '../sibling/dist') resolves to /home/user/sibling/dist, which doesn't start with root, so it throws even though it's not an ancestor.

The relative() approach handles both of these because it directly answers "is root inside the output path?" regardless of how the path was specified.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The second one should be blocked as it’s outside of the workspace IMHO, and the first one should be allowed as it’s absolute.

@pradhankukiran
Copy link
Copy Markdown
Author

@alan-agius4 updated the approach per your suggestion, exact root match, and relative paths escaping the workspace. Absolute paths are now always allowed.
Also added root + sep instead of bare startsWith(root) to avoid false matches on paths like project-backup vs project.

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

Labels

area: @angular/build target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

prevent ng build from overwriting source files

3 participants