From bd707040d87cc1d95616ac7a683ca9b2d8e685d9 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Thu, 4 Jun 2026 13:36:33 -0400 Subject: [PATCH] fix(@angular/build): restrict application builder output paths to output directory Ensure all file writes and deletions produced by the application builder are strictly scoped to the configured `outputPath` base directory. This prevents accidental writes and overwrites of files outside of the output directory (e.g. if the `browser` directory option is configured with relative path segments like `..` that escape the base output folder). --- .../build/src/builders/application/index.ts | 83 ++++++++++++------- .../tests/options/output-path_spec.ts | 22 +++++ packages/angular/build/src/utils/path.ts | 7 +- packages/angular/build/src/utils/path_spec.ts | 41 +++++++++ 4 files changed, 118 insertions(+), 35 deletions(-) create mode 100644 packages/angular/build/src/utils/path_spec.ts diff --git a/packages/angular/build/src/builders/application/index.ts b/packages/angular/build/src/builders/application/index.ts index f127ef4bdc7f..b31fc0a8f81b 100644 --- a/packages/angular/build/src/builders/application/index.ts +++ b/packages/angular/build/src/builders/application/index.ts @@ -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'; @@ -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(); - 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 }; @@ -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; } diff --git a/packages/angular/build/src/builders/application/tests/options/output-path_spec.ts b/packages/angular/build/src/builders/application/tests/options/output-path_spec.ts index b6c72b9bee58..3fc98c83ed36 100644 --- a/packages/angular/build/src/builders/application/tests/options/output-path_spec.ts +++ b/packages/angular/build/src/builders/application/tests/options/output-path_spec.ts @@ -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`, + ), + }), + ); + }); }); }); }); diff --git a/packages/angular/build/src/utils/path.ts b/packages/angular/build/src/utils/path.ts index eafef4ee9f2b..e969d420e429 100644 --- a/packages/angular/build/src/utils/path.ts +++ b/packages/angular/build/src/utils/path.ts @@ -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; @@ -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); } diff --git a/packages/angular/build/src/utils/path_spec.ts b/packages/angular/build/src/utils/path_spec.ts new file mode 100644 index 000000000000..84c7d8ac31c4 --- /dev/null +++ b/packages/angular/build/src/utils/path_spec.ts @@ -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(); + }); +});