Skip to content

Commit

Permalink
fix(@angular/build): update autoCsp to handle inlined critical CSS
Browse files Browse the repository at this point in the history
This update enhances the `autoCsp` functionality to properly handle inlined critical CSS, ensuring compliance with Content Security Policy (CSP) directives. Previously, inlined styles could cause CSP violations in certain configurations. With this fix, the mechanism correctly accounts for and integrates critical CSS while maintaining security.

Closes angular#29603
  • Loading branch information
alan-agius4 committed Feb 13, 2025
1 parent 57a08c9 commit e8b3b5b
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ 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;
options.security = undefined;
}

// Perform i18n translation inlining if enabled
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(
}
}

const autoCsp = options.security?.autoCsp;
let security;
if (autoCsp) {
security = {
autoCsp: {
unsafeEval: autoCsp === true ? false : !!autoCsp?.unsafeEval,
},
};
}

// 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
4 changes: 2 additions & 2 deletions packages/angular/build/src/utils/index-file/auto-csp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,11 @@ export async function autoCsp(html: string, unsafeEval = false): Promise<string>
function emitLoaderScript() {
const loaderScript = createLoaderScript(scriptContent, /* enableTrustedTypes = */ false);
hashes.push(hashTextContent(loaderScript));
rewriter.emitRaw(`<script>${loaderScript}</script>`);
rewriter.emitRaw(`<script type="text/javascript">${loaderScript}</script>`);
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
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 e8b3b5b

Please sign in to comment.