Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pradhankukiran wants to merge 5 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4886930
fix(@angular/build): prevent deleting parent directories of project root
pradhankukiran Mar 27, 2026
6c745cc
fixup! fix(@angular/build): prevent deleting parent directories of pr…
pradhankukiran Mar 27, 2026
a2b1fab
fixup! fix(@angular/build): prevent deleting parent directories of pr…
alan-agius4 Mar 27, 2026
9d3f5d4
fix(@angular/build): handle Windows absolute paths in output dir check
pradhankukiran Mar 27, 2026
3a6d97a
fixup! fix(@angular/build): prevent deleting parent directories of pr…
pradhankukiran Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix(@angular/build): prevent deleting parent directories of project root 
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 #6485
  • Loading branch information
@pradhankukiran @alan-agius4
pradhankukiran authored and alan-agius4 committed Mar 27, 2026
commit 4886930b4f7002caef69d252406a30e13d14f0c9
9 changes: 6 additions & 3 deletions packages/angular/build/src/utils/delete-output-dir.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { readdir, rm } from 'node:fs/promises';
import { join, resolve } from 'node:path';
import { join, relative, resolve } from 'node:path';

/**
* Delete an output directory, but error out if it's the root of the project.
Expand All @@ -18,8 +18,11 @@ export async function deleteOutputDir(
emptyOnlyDirectories?: string[],
): Promise<void> {
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 •
edited
Loading

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

@pradhankukiran pradhankukiran Mar 27, 2026

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

@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.

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.

if (!relativePath || !relativePath.startsWith('..')) {
throw new Error(
`Output path "${resolvedOutputPath}" MUST not be the project root directory or a parent of it.`,
);
}

const directoriesToEmpty = emptyOnlyDirectories
Expand Down
64 changes: 64 additions & 0 deletions packages/angular/build/src/utils/delete-output-dir_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.dev/license
*/

import { mkdir, writeFile } from 'node:fs/promises';
import { join } from 'node:path';
import { deleteOutputDir } from './delete-output-dir';

describe('deleteOutputDir', () => {
let root: string;

beforeEach(async () => {
// Use a unique temp directory for each test
const { mkdtemp } = await import('node:fs/promises');
const { tmpdir } = await import('node:os');
root = await mkdtemp(join(tmpdir(), 'ng-test-'));
});

it('should throw when output path is the project root', async () => {
await expectAsync(deleteOutputDir(root, '.')).toBeRejectedWithError(
/MUST not be the project root directory or a parent of it/,
);
});

it('should throw when output path is a parent of the project root', async () => {
await expectAsync(deleteOutputDir(root, '..')).toBeRejectedWithError(
/MUST not be the project root directory or a parent of it/,
);
});

it('should throw when output path is a grandparent of the project root', async () => {
await expectAsync(deleteOutputDir(root, '../..')).toBeRejectedWithError(
/MUST not be the project root directory or a parent of it/,
);
});

it('should not throw when output path is a child of the project root', async () => {
const outputDir = join(root, 'dist');
await mkdir(outputDir, { recursive: true });
await writeFile(join(outputDir, 'old-file.txt'), 'content');

await expectAsync(deleteOutputDir(root, 'dist')).toBeResolved();
});

it('should delete contents of a valid output directory', async () => {
const outputDir = join(root, 'dist');
await mkdir(outputDir, { recursive: true });
await writeFile(join(outputDir, 'old-file.txt'), 'content');

await deleteOutputDir(root, 'dist');

Comment thread
alan-agius4 marked this conversation as resolved.
const { readdir } = await import('node:fs/promises');
const entries = await readdir(outputDir);
expect(entries.length).toBe(0);
});

it('should not throw when output directory does not exist', async () => {
await expectAsync(deleteOutputDir(root, 'nonexistent')).toBeResolved();
});
});