Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
11 changes: 8 additions & 3 deletions packages/angular/build/src/utils/delete-output-dir.ts
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 { isAbsolute, 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,13 @@ 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

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.

// When relative() returns an absolute path, the paths are on different drives/roots
// (e.g. Windows drive letters or UNC paths), so it cannot be an ancestor.
if (!relativePath || (!isAbsolute(relativePath) && !relativePath.startsWith('..'))) {
throw new Error(
`Output path "${resolvedOutputPath}" MUST not be the project root directory or its parent.`,
);
}

const directoriesToEmpty = emptyOnlyDirectories
Expand Down
68 changes: 68 additions & 0 deletions packages/angular/build/src/utils/delete-output-dir_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* @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, mkdtemp, readdir, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { deleteOutputDir } from './delete-output-dir';

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

beforeEach(async () => {
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 its parent/,
);
});

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 its parent/,
);
});

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 its parent/,
);
});

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');

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();
});

it('should not throw when output path is an absolute path outside the project', async () => {
const externalDir = await mkdtemp(join(tmpdir(), 'ng-test-external-'));
await writeFile(join(externalDir, 'old-file.txt'), 'content');

await expectAsync(deleteOutputDir(root, externalDir)).toBeResolved();
});
});
Loading