Skip to content

Commit

Permalink
fix(@angular/build): update critical CSS inlining to support autoCsp
Browse files Browse the repository at this point in the history
This update improves the handling of inlined critical CSS to align with `autoCsp`, ensuring compliance with Content Security Policy (CSP) directives. Previously, inlined styles could trigger CSP violations in certain configurations. With this fix, critical CSS is inlined in a way that maintains security while supporting `autoCsp`.

Closes angular#29603
  • Loading branch information
alan-agius4 committed Feb 14, 2025
1 parent 65c99b8 commit 0fde9a5
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,6 @@ export async function executeBuild(
);
}

// Override auto-CSP settings if we are serving through Vite middleware.
if (context.builder.builderName === 'dev-server' && options.security) {
options.security.autoCsp = false;
}

// Perform i18n translation inlining if enabled
if (i18nOptions.shouldInline) {
const result = await inlineI18n(metafile, options, executionResult, initialFiles);
Expand Down
11 changes: 10 additions & 1 deletion packages/angular/build/src/builders/application/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,16 @@ export async function normalizeOptions(
}
}

// Disable auto-CSP settings if we are serving through Vite middleware.
const autoCsp = context.builder.builderName !== 'dev-server' && options.security?.autoCsp;
const security = {
autoCsp: autoCsp
? {
unsafeEval: autoCsp === true ? false : !!autoCsp.unsafeEval,
}
: undefined,
};

// Initial options to keep
const {
allowedCommonJsDependencies,
Expand Down Expand Up @@ -415,7 +425,6 @@ export async function normalizeOptions(
partialSSRBuild = false,
externalRuntimeStyles,
instrumentForCoverage,
security,
} = options;

// Return all the normalized options
Expand Down
11 changes: 1 addition & 10 deletions packages/angular/build/src/tools/esbuild/index-html-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,6 @@ export async function generateIndexHtml(
throw new Error(`Output file does not exist: ${relativefilePath}`);
};

// Read the Auto CSP options.
const autoCsp = buildOptions.security?.autoCsp;
const autoCspOptions =
autoCsp === true
? { unsafeEval: false }
: autoCsp
? { unsafeEval: !!autoCsp.unsafeEval }
: undefined;

// Create an index HTML generator that reads from the in-memory output files
const indexHtmlGenerator = new IndexHtmlGenerator({
indexPath: indexHtmlOptions.input,
Expand All @@ -103,7 +94,7 @@ export async function generateIndexHtml(
buildOptions.prerenderOptions ||
buildOptions.appShellOptions
),
autoCsp: autoCspOptions,
autoCsp: buildOptions.security.autoCsp,
});

indexHtmlGenerator.readAsset = readAsset;
Expand Down
2 changes: 1 addition & 1 deletion packages/angular/build/src/utils/index-file/auto-csp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export async function autoCsp(html: string, unsafeEval = false): Promise<string>
scriptContent = [];
}

rewriter.on('startTag', (tag, html) => {
rewriter.on('startTag', (tag) => {
if (tag.tagName === 'script') {
openedScriptTag = tag;
const src = getScriptAttributeValue(tag, 'src');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export class IndexHtmlGenerator {

// CSR plugins
if (options?.optimization?.styles?.inlineCritical) {
this.csrPlugins.push(inlineCriticalCssPlugin(this));
this.csrPlugins.push(inlineCriticalCssPlugin(this, !!options.autoCsp));
}

this.csrPlugins.push(addNoncePlugin());
Expand Down Expand Up @@ -197,11 +197,15 @@ function inlineFontsPlugin({ options }: IndexHtmlGenerator): IndexHtmlGeneratorP
return async (html) => inlineFontsProcessor.process(html);
}

function inlineCriticalCssPlugin(generator: IndexHtmlGenerator): IndexHtmlGeneratorPlugin {
function inlineCriticalCssPlugin(
generator: IndexHtmlGenerator,
autoCsp: boolean,
): IndexHtmlGeneratorPlugin {
const inlineCriticalCssProcessor = new InlineCriticalCssProcessor({
minify: generator.options.optimization?.styles.minify,
deployUrl: generator.options.deployUrl,
readAsset: (filePath) => generator.readAsset(filePath),
autoCsp,
});

return async (html, options) =>
Expand Down
22 changes: 14 additions & 8 deletions packages/angular/build/src/utils/index-file/inline-critical-css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export interface InlineCriticalCssProcessorOptions {
minify?: boolean;
deployUrl?: string;
readAsset?: (path: string) => Promise<string>;
autoCsp?: boolean;
}

/** Partial representation of an `HTMLElement`. */
Expand Down Expand Up @@ -163,7 +164,7 @@ class BeastiesExtended extends BeastiesBase {
const returnValue = await super.embedLinkedStylesheet(link, document);
const cspNonce = this.findCspNonce(document);

if (cspNonce) {
if (cspNonce || this.optionsExtended.autoCsp) {
const beastiesMedia = link.getAttribute('onload')?.match(MEDIA_SET_HANDLER_PATTERN);

if (beastiesMedia) {
Expand All @@ -180,11 +181,13 @@ class BeastiesExtended extends BeastiesBase {
// a way of doing that at the moment so we fall back to doing it any time a `link` tag is
// inserted. We mitigate it by only iterating the direct children of the `<head>` which
// should be pretty shallow.
document.head.children.forEach((child) => {
if (child.tagName === 'style' && !child.hasAttribute('nonce')) {
child.setAttribute('nonce', cspNonce);
}
});
if (cspNonce) {
document.head.children.forEach((child) => {
if (child.tagName === 'style' && !child.hasAttribute('nonce')) {
child.setAttribute('nonce', cspNonce);
}
});
}
}

return returnValue;
Expand Down Expand Up @@ -215,16 +218,19 @@ class BeastiesExtended extends BeastiesBase {
*/
private conditionallyInsertCspLoadingScript(
document: PartialDocument,
nonce: string,
nonce: string | null,
link: PartialHTMLElement,
): void {
if (this.addedCspScriptsDocuments.has(document)) {
return;
}

const script = document.createElement('script');
script.setAttribute('nonce', nonce);
script.textContent = LINK_LOAD_SCRIPT_CONTENT;
if (nonce) {
script.setAttribute('nonce', nonce);
}

// Prepend the script to the head since it needs to
// run as early as possible, before the `link` tags.
document.head.insertBefore(script, link);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,26 @@ describe('InlineCriticalCssProcessor', () => {
expect(content).toContain('<style>body{margin:0}html{color:white}</style>');
});

it(`should process the inline 'onload' handlers if a 'autoCsp' is true`, async () => {
const inlineCssProcessor = new InlineCriticalCssProcessor({
readAsset,
autoCsp: true,
});

const { content } = await inlineCssProcessor.process(getContent(''), {
outputPath: '/dist/',
});

expect(content).toContain(
'<link href="styles.css" rel="stylesheet" media="print" ngCspMedia="all">',
);
expect(tags.stripIndents`${content}`).toContain(tags.stripIndents`
<style>
body { margin: 0; }
html { color: white; }
</style>`);
});

it('should process the inline `onload` handlers if a CSP nonce is specified', async () => {
const inlineCssProcessor = new InlineCriticalCssProcessor({
readAsset,
Expand Down
10 changes: 8 additions & 2 deletions tests/legacy-cli/e2e/tests/build/auto-csp.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import assert from 'node:assert';
import { getGlobalVariable } from '../../utils/env';
import { expectFileToMatch, writeMultipleFiles } from '../../utils/fs';
import { expectFileToMatch, writeFile, writeMultipleFiles } from '../../utils/fs';
import { findFreePort } from '../../utils/network';
import { execAndWaitForOutputToMatch, ng } from '../../utils/process';
import { updateJsonFile } from '../../utils/project';
Expand All @@ -13,6 +13,9 @@ export default async function () {
'This test should not be called in the Webpack suite.',
);

// Add global css to trigger critical css inlining
await writeFile('src/styles.css', `body { color: green }`);

// Turn on auto-CSP
await updateJsonFile('angular.json', (json) => {
const build = json['projects']['test-project']['architect']['build'];
Expand Down Expand Up @@ -54,7 +57,7 @@ export default async function () {
</head>
<body>
<app-root></app-root>
<script>
const inlineScriptBodyCreated = 1338;
console.warn("Inline Script Body: " + inlineScriptHeadCreated);
Expand Down Expand Up @@ -130,6 +133,9 @@ export default async function () {
// Make sure the output files have auto-CSP as a result of `ng build`
await expectFileToMatch('dist/test-project/browser/index.html', CSP_META_TAG);

// Make sure if contains the critical CSS inlining CSP code.
await expectFileToMatch('dist/test-project/browser/index.html', 'ngCspMedia');

// Make sure that our e2e protractor tests run to confirm that our angular project runs.
const port = await spawnServer();
await ng('e2e', `--base-url=http://localhost:${port}`, '--dev-server-target=');
Expand Down

0 comments on commit 0fde9a5

Please sign in to comment.