Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
83 changes: 51 additions & 32 deletions packages/angular/build/src/builders/application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { createJsonBuildManifest, emitFilesToDisk } from '../../tools/esbuild/ut
import { colors as ansiColors } from '../../utils/color';
import { deleteOutputDir } from '../../utils/delete-output-dir';
import { bazelEsbuildPluginPath, useJSONBuildLogs } from '../../utils/environment-options';
import { isSubDirectory } from '../../utils/path';
import { purgeStaleBuildCache } from '../../utils/purge-cache';
import { assertCompatibleAngularVersion } from '../../utils/version';
import { runEsBuildBuildAction } from './build-action';
Expand Down Expand Up @@ -197,45 +198,57 @@ export async function* buildApplication(

// Writes the output files to disk and ensures the containing directories are present
const directoryExists = new Set<string>();
await emitFilesToDisk(Object.entries(result.files), async ([filePath, file]) => {
if (
outputOptions.ignoreServer &&
(file.type === BuildOutputFileType.ServerApplication ||
file.type === BuildOutputFileType.ServerRoot)
) {
return;
}
try {
await emitFilesToDisk(Object.entries(result.files), async ([filePath, file]) => {
if (
outputOptions.ignoreServer &&
(file.type === BuildOutputFileType.ServerApplication ||
file.type === BuildOutputFileType.ServerRoot)
) {
return;
}

const fullFilePath = generateFullPath(filePath, file.type, outputOptions);
const fullFilePath = generateFullPath(filePath, file.type, outputOptions);

// Ensure output subdirectories exist
const fileBasePath = path.dirname(fullFilePath);
if (fileBasePath && !directoryExists.has(fileBasePath)) {
await fs.mkdir(fileBasePath, { recursive: true });
directoryExists.add(fileBasePath);
}
// Ensure output subdirectories exist
const fileBasePath = path.dirname(fullFilePath);
if (fileBasePath && !directoryExists.has(fileBasePath)) {
await fs.mkdir(fileBasePath, { recursive: true });
directoryExists.add(fileBasePath);
}

if (file.origin === 'memory') {
// Write file contents
await fs.writeFile(fullFilePath, file.contents);
} else {
// Copy file contents
await fs.cp(file.inputPath, fullFilePath, {
mode: fs.constants.COPYFILE_FICLONE,
preserveTimestamps: true,
});
}
});
if (file.origin === 'memory') {
// Write file contents
await fs.writeFile(fullFilePath, file.contents);
} else {
// Copy file contents
await fs.cp(file.inputPath, fullFilePath, {
mode: fs.constants.COPYFILE_FICLONE,
preserveTimestamps: true,
});
}
});
} catch (error) {
context.logger.error(error instanceof Error ? error.message : String(error));
yield { success: false };
continue;
}

// Delete any removed files if incremental
if (result.kind === ResultKind.Incremental && result.removed?.length) {
await Promise.all(
result.removed.map((file) => {
const fullFilePath = generateFullPath(file.path, file.type, outputOptions);
try {
await Promise.all(
result.removed.map((file) => {
const fullFilePath = generateFullPath(file.path, file.type, outputOptions);

return fs.rm(fullFilePath, { force: true, maxRetries: 3 });
}),
);
return fs.rm(fullFilePath, { force: true, maxRetries: 3 });
}),
);
} catch (error) {
context.logger.error(error instanceof Error ? error.message : String(error));
yield { success: false };
continue;
}
}

yield { success: true };
Expand Down Expand Up @@ -268,6 +281,12 @@ function generateFullPath(
// NOTE: 'base' is a fully resolved path at this point
const fullFilePath = path.join(outputOptions.base, typeDirectory, filePath);

if (!isSubDirectory(outputOptions.base, fullFilePath)) {
throw new Error(
`The output file path "${fullFilePath}" is outside of the configured output path "${outputOptions.base}".`,
);
}

return fullFilePath;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,28 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
}),
);
});

it('should error when browser directory escapes the output path base', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
polyfills: [],
outputPath: {
base: 'dist',
browser: '..',
},
ssr: false,
});

const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
expect(result?.success).toBeFalse();
expect(logs).toContain(
jasmine.objectContaining({
message: jasmine.stringMatching(
`The output file path .* is outside of the configured output path`,
),
}),
);
});
});
});
});
7 changes: 4 additions & 3 deletions packages/angular/build/src/utils/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.dev/license
*/

import { normalize, posix, resolve } from 'node:path';
import { isAbsolute, posix, relative, resolve } from 'node:path';
import { platform } from 'node:process';

const WINDOWS_PATH_SEPERATOR_REGEXP = /\\/g;
Expand Down Expand Up @@ -44,8 +44,9 @@ export function toPosixPath(path: string): string {
* @returns `true` if the child path is within the parent directory, `false` otherwise.
*/
export function isSubDirectory(parent: string, child: string): boolean {
const normalizedParent = normalize(parent);
const resolvedParent = resolve(parent);
const resolvedChild = resolve(parent, child);
const relativePath = toPosixPath(relative(resolvedParent, resolvedChild));

return resolvedChild.startsWith(normalizedParent);
return relativePath !== '..' && !relativePath.startsWith('../') && !isAbsolute(relativePath);
}
Comment thread
clydin marked this conversation as resolved.
41 changes: 41 additions & 0 deletions packages/angular/build/src/utils/path_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* @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 { isSubDirectory } from './path';

describe('isSubDirectory', () => {
it('should return true for a direct child', () => {
expect(isSubDirectory('/foo/bar', '/foo/bar/baz')).toBeTrue();
});

it('should return true for a nested child', () => {
expect(isSubDirectory('/foo/bar', '/foo/bar/baz/qux/file.txt')).toBeTrue();
});

it('should return true if paths are identical', () => {
expect(isSubDirectory('/foo/bar', '/foo/bar')).toBeTrue();
});

it('should return false for a sibling directory starting with same prefix', () => {
expect(isSubDirectory('/foo/bar', '/foo/bar-outside')).toBeFalse();
expect(isSubDirectory('/foo/bar', '/foo/bar-outside/file.txt')).toBeFalse();
});

it('should return false for directory outside parent via traversal', () => {
expect(isSubDirectory('/foo/bar', '/foo/bar/../outside')).toBeFalse();
});

it('should return false for parent directory', () => {
expect(isSubDirectory('/foo/bar', '/foo')).toBeFalse();
});

it('should return true for children containing directory names starting with double dots', () => {
expect(isSubDirectory('/foo/bar', '/foo/bar/..baz')).toBeTrue();
expect(isSubDirectory('/foo/bar', '/foo/bar/..baz/qux')).toBeTrue();
});
});
Loading