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
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,27 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
harness.expectFile('dist/browser/subdirectory/test.svg').content.toBe('<svg></svg>');
});

it('fails if asset input option is outside workspace root (relative)', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
assets: [{ glob: '**/*', input: '../outside', output: '.' }],
});

const { error } = await harness.executeOnce({ outputLogsOnException: false });

expect(error?.message).toContain('asset path must be within the workspace root');
});

it('fails if asset input option is outside workspace root (absolute)', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
assets: [{ glob: '**/*', input: '/tmp/outside-workspace', output: '.' }],
});

const { error } = await harness.executeOnce({ outputLogsOnException: false });

expect(error?.message).toContain('asset path must be within the workspace root');
});
it('fails if output option is not within project output path', async () => {
await harness.writeFile('test.svg', '<svg></svg>');

Expand Down
5 changes: 5 additions & 0 deletions packages/angular/build/src/utils/normalize-asset-patterns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import assert from 'node:assert';
import { statSync } from 'node:fs';
import * as path from 'node:path';
import { AssetPattern, AssetPatternClass } from '../builders/application/schema';
import { isSubDirectory } from './path';

export class MissingAssetSourceRootException extends Error {
constructor(path: string) {
Expand Down Expand Up @@ -68,6 +69,10 @@ export function normalizeAssetPatterns(

assetPattern = { glob, input, output };
} else {
if (!isSubDirectory(workspaceRoot, assetPattern.input)) {
throw new Error(`The ${assetPattern.input} asset path must be within the workspace root.`);
}

assetPattern.output = path.join('.', assetPattern.output ?? '');
}

Expand Down
16 changes: 15 additions & 1 deletion 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 { posix } from 'node:path';
import { normalize, posix, resolve } from 'node:path';
import { platform } from 'node:process';

const WINDOWS_PATH_SEPERATOR_REGEXP = /\\/g;
Expand Down Expand Up @@ -35,3 +35,17 @@ const WINDOWS_PATH_SEPERATOR_REGEXP = /\\/g;
export function toPosixPath(path: string): string {
return platform === 'win32' ? path.replace(WINDOWS_PATH_SEPERATOR_REGEXP, posix.sep) : path;
}

/**
* Determines if a path is a subdirectory or file within a parent directory.
*
* @param parent - The parent directory path.
* @param child - The child path to check.
* @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 resolvedChild = resolve(parent, child);

return resolvedChild.startsWith(normalizedParent);
}
6 changes: 6 additions & 0 deletions packages/angular/build/src/utils/resolve-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import path from 'node:path';
import { glob } from 'tinyglobby';
import { isSubDirectory } from './path';

export async function resolveAssets(
entries: {
Expand All @@ -25,7 +26,12 @@ export async function resolveAssets(
const outputFiles: { source: string; destination: string }[] = [];

for (const entry of entries) {
if (!isSubDirectory(root, entry.input)) {
throw new Error(`The ${entry.input} asset path must be within the workspace root.`);
}

const cwd = path.resolve(root, entry.input);

const files = await glob(entry.glob, {
cwd,
dot: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,28 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
harness.expectFile('dist/subdirectory/test.svg').content.toBe('<svg></svg>');
});

it('fails if asset input option is outside workspace root (relative)', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
assets: [{ glob: '**/*', input: '../outside', output: '.' }],
});

const { result } = await harness.executeOnce();

expect(result?.error).toMatch('asset path must be within the workspace root');
});

it('fails if asset input option is outside workspace root (absolute)', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
assets: [{ glob: '**/*', input: '/tmp/outside-workspace', output: '.' }],
});

const { result } = await harness.executeOnce();

expect(result?.error).toMatch('asset path must be within the workspace root');
});

it('fails if output option is not within project output path', async () => {
await harness.writeFile('test.svg', '<svg></svg>');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function normalizeAssetPatterns(
}

let glob: string, input: string;
let isDirectory = false;
let isDirectory: boolean;

try {
isDirectory = statSync(resolvedAssetPath).isDirectory();
Expand All @@ -68,6 +68,11 @@ export function normalizeAssetPatterns(

assetPattern = { glob, input, output };
} else {
const resolvedInput = path.resolve(workspaceRoot, assetPattern.input);
if (!resolvedInput.startsWith(workspaceRoot)) {
throw new Error(`The ${assetPattern.input} asset path must be within the workspace root.`);
}

assetPattern.output = path.join('.', assetPattern.output ?? '');
}

Expand Down
73 changes: 22 additions & 51 deletions tests/legacy-cli/e2e/tests/commands/serve/assets.ts
Original file line number Diff line number Diff line change
@@ -1,59 +1,30 @@
import assert from 'node:assert';
import { randomUUID } from 'node:crypto';
import { mkdir, rm, writeFile } from 'node:fs/promises';
import { ngServe, updateJsonFile } from '../../../utils/project';
import { ngServe } from '../../../utils/project';
import { getGlobalVariable } from '../../../utils/env';

export default async function () {
const outsideDirectoryName = `../outside-${randomUUID()}`;
const port = await ngServe();
let response = await fetch(`http://localhost:${port}/favicon.ico`);
assert.strictEqual(response.status, 200, 'favicon.ico response should be ok');

await updateJsonFile('angular.json', (json) => {
// Ensure assets located outside the workspace root work with the dev server
json.projects['test-project'].architect.build.options.assets.push({
'input': outsideDirectoryName,
'glob': '**/*',
'output': './outside',
});
// A non-existent HTML file request with accept header should fallback to the index HTML
response = await fetch(`http://localhost:${port}/does-not-exist.html`, {
headers: { accept: 'text/html' },
});

await mkdir(outsideDirectoryName);
try {
await writeFile(`${outsideDirectoryName}/some-asset.xyz`, 'XYZ');

const port = await ngServe();

let response = await fetch(`http://localhost:${port}/favicon.ico`);
assert.strictEqual(response.status, 200, 'favicon.ico response should be ok');

response = await fetch(`http://localhost:${port}/outside/some-asset.xyz`);
assert.strictEqual(response.status, 200, 'outside/some-asset.xyz response should be ok');
assert.strictEqual(await response.text(), 'XYZ', 'outside/some-asset.xyz content is wrong');

// A non-existent HTML file request with accept header should fallback to the index HTML
response = await fetch(`http://localhost:${port}/does-not-exist.html`, {
headers: { accept: 'text/html' },
});
assert.strictEqual(
response.status,
200,
'non-existent file response should fallback and be ok',
);
assert.match(
await response.text(),
/<app-root/,
'non-existent file response should fallback and contain html',
);

// Vite will incorrectly fallback in all non-existent cases so skip last test case
// TODO: Remove conditional when Vite handles this case
if (getGlobalVariable('argv')['esbuild']) {
return;
}

// A non-existent file without an html accept header should not be found.
response = await fetch(`http://localhost:${port}/does-not-exist.png`);
assert.strictEqual(response.status, 404, 'non-existent file response should be not found');
} finally {
await rm(outsideDirectoryName, { force: true, recursive: true });
assert.strictEqual(response.status, 200, 'non-existent file response should fallback and be ok');
assert.match(
await response.text(),
/<app-root/,
'non-existent file response should fallback and contain html',
);

// Vite will incorrectly fallback in all non-existent cases so skip last test case
// TODO: Remove conditional when Vite handles this case
if (getGlobalVariable('argv')['esbuild']) {
return;
}

// A non-existent file without an html accept header should not be found.
response = await fetch(`http://localhost:${port}/does-not-exist.png`);
assert.strictEqual(response.status, 404, 'non-existent file response should be not found');
}
Loading