Skip to content

fix(@angular/build): update critical CSS inlining to support autoCsp #29638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 14, 2025
Merged
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 @@ -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
10 changes: 9 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,15 @@ export async function normalizeOptions(
}
}

const autoCsp = 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 +424,6 @@ export async function normalizeOptions(
partialSSRBuild = false,
externalRuntimeStyles,
instrumentForCoverage,
security,
} = options;

// Return all the normalized options
Expand Down
5 changes: 5 additions & 0 deletions packages/angular/build/src/builders/dev-server/vite-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ export async function* serveWithVite(
browserOptions.ssr ||= true;
}

// Disable auto CSP.
browserOptions.security = {
autoCsp: false,
};

// Set all packages as external to support Vite's prebundle caching
browserOptions.externalPackages = serverOptions.prebundle;

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
Loading