Skip to content

Commit

Permalink
fix(@angular/build): Fixing auto-csp edge cases where
Browse files Browse the repository at this point in the history 
- <script> is the last tag before </head> close
- .appendChild is called before </head> (because document.body is undefined then)
- <script> tags with a src attribute and no specified type attribute should not write <script type="undefined" ...>
  • Loading branch information
@aaronshim @dgp1130
aaronshim authored and dgp1130 committed Dec 11, 2024
1 parent 0d2c648 commit 210bf4e
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 13 deletions.
10 changes: 5 additions & 5 deletions packages/angular/build/src/utils/index-file/auto-csp.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export async function autoCsp(html: string, unsafeEval = false): Promise<string>
* loader script to the collection of hashes to add to the <meta> tag CSP.
*/
function emitLoaderScript() {
const loaderScript = createLoaderScript(scriptContent);
const loaderScript = createLoaderScript(scriptContent, /* enableTrustedTypes = */ false);
hashes.push(hashTextContent(loaderScript));
rewriter.emitRaw(`<script>${loaderScript}</script>`);
scriptContent = [];
Expand Down Expand Up @@ -152,7 +152,7 @@ export async function autoCsp(html: string, unsafeEval = false): Promise<string>
}
}

if (tag.tagName === 'body' || tag.tagName === 'html') {
if (tag.tagName === 'head' || tag.tagName === 'body' || tag.tagName === 'html') {
// Write the loader script if a string of <script>s were the last opening tag of the document.
if (scriptContent.length > 0) {
emitLoaderScript();
Expand Down Expand Up @@ -267,7 +267,7 @@ function createLoaderScript(srcList: SrcScriptTag[], enableTrustedTypes = false)
// URI encoding means value can't escape string, JS, or HTML context.
const srcAttr = encodeURI(s.src).replaceAll("'", "\\'");
// Can only be 'module' or a JS MIME type or an empty string.
const typeAttr = s.type ? "'" + s.type + "'" : undefined;
const typeAttr = s.type ? "'" + s.type + "'" : "''";
const asyncAttr = s.async ? 'true' : 'false';
const deferAttr = s.defer ? 'true' : 'false';

Expand All @@ -288,7 +288,7 @@ function createLoaderScript(srcList: SrcScriptTag[], enableTrustedTypes = false)
s.type = scriptUrl[1];
s.async = !!scriptUrl[2];
s.defer = !!scriptUrl[3];
document.body.appendChild(s);
document.lastElementChild.appendChild(s);
});\n`
: `
var scripts = [${srcListFormatted}];
Expand All @@ -298,6 +298,6 @@ function createLoaderScript(srcList: SrcScriptTag[], enableTrustedTypes = false)
s.type = scriptUrl[1];
s.async = !!scriptUrl[2];
s.defer = !!scriptUrl[3];
document.body.appendChild(s);
document.lastElementChild.appendChild(s);
});\n`;
}
48 changes: 40 additions & 8 deletions packages/angular/build/src/utils/index-file/auto-csp_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ const getCsps = (html: string) => {
const ONE_HASH_CSP =
/script-src 'strict-dynamic' 'sha256-[^']+' https: 'unsafe-inline';object-src 'none';base-uri 'self';/;

const TWO_HASH_CSP =
/script-src 'strict-dynamic' (?:'sha256-[^']+' ){2}https: 'unsafe-inline';object-src 'none';base-uri 'self';/;

const FOUR_HASH_CSP =
/script-src 'strict-dynamic' (?:'sha256-[^']+' ){4}https: 'unsafe-inline';object-src 'none';base-uri 'self';/;

Expand Down Expand Up @@ -55,7 +58,7 @@ describe('auto-csp', () => {
const csps = getCsps(result);
expect(csps.length).toBe(1);
expect(csps[0]).toMatch(ONE_HASH_CSP);
expect(result).toContain(`var scripts = [['./main.js', undefined, false, false]];`);
expect(result).toContain(`var scripts = [['./main.js', '', false, false]];`);
});

it('should rewrite a single source script in place', async () => {
Expand All @@ -75,14 +78,15 @@ describe('auto-csp', () => {
expect(csps[0]).toMatch(ONE_HASH_CSP);
// Our loader script appears after the HTML text content.
expect(result).toMatch(
/Some text<\/div>\s*<script>\s*var scripts = \[\['.\/main.js', undefined, false, false\]\];/,
/Some text<\/div>\s*<script>\s*var scripts = \[\['.\/main.js', '', false, false\]\];/,
);
});

it('should rewrite a multiple source scripts with attributes', async () => {
const result = await autoCsp(`
<html>
<head>
<script src="./head.js"></script>
</head>
<body>
<script src="./main1.js"></script>
Expand All @@ -96,13 +100,15 @@ describe('auto-csp', () => {

const csps = getCsps(result);
expect(csps.length).toBe(1);
expect(csps[0]).toMatch(ONE_HASH_CSP);
expect(csps[0]).toMatch(TWO_HASH_CSP);
expect(result).toContain(
// eslint-disable-next-line max-len
`var scripts = [['./main1.js', undefined, false, false],['./main2.js', undefined, true, false],['./main3.js', 'module', true, true]];`,
`var scripts = [['./main1.js', '', false, false],['./main2.js', '', true, false],['./main3.js', 'module', true, true]];`,
);
// Only one loader script is created.
expect(Array.from(result.matchAll(/<script>/g)).length).toEqual(1);
// Head loader script is in the head.
expect(result).toContain(`</script></head>`);
// Only two loader scripts are created.
expect(Array.from(result.matchAll(/<script>/g)).length).toEqual(2);
});

it('should rewrite source scripts with weird URLs', async () => {
Expand Down Expand Up @@ -160,14 +166,40 @@ describe('auto-csp', () => {
// Loader script for main.js and main2.js appear after 'foo' and before 'bar'.
expect(result).toMatch(
// eslint-disable-next-line max-len
/console.log\('foo'\);<\/script>\s*<script>\s*var scripts = \[\['.\/main.js', undefined, false, false\],\['.\/main2.js', undefined, false, false\]\];[\s\S]*console.log\('bar'\);/,
/console.log\('foo'\);<\/script>\s*<script>\s*var scripts = \[\['.\/main.js', '', false, false\],\['.\/main2.js', '', false, false\]\];[\s\S]*console.log\('bar'\);/,
);
// Loader script for main3.js and main4.js appear after 'bar'.
expect(result).toMatch(
// eslint-disable-next-line max-len
/console.log\('bar'\);<\/script>\s*<script>\s*var scripts = \[\['.\/main3.js', undefined, false, false\],\['.\/main4.js', undefined, false, false\]\];/,
/console.log\('bar'\);<\/script>\s*<script>\s*var scripts = \[\['.\/main3.js', '', false, false\],\['.\/main4.js', '', false, false\]\];/,
);
// Exactly 4 scripts should be left.
expect(Array.from(result.matchAll(/<script>/g)).length).toEqual(4);
});

it('should write a loader script that appends to head', async () => {
const result = await autoCsp(`
<html>
<head>
<script src="./head.js"></script>
</head>
<body>
<div>Some text </div>
</body>
</html>
`);

const csps = getCsps(result);
expect(csps.length).toBe(1);
expect(csps[0]).toMatch(ONE_HASH_CSP);

expect(result).toContain(
// eslint-disable-next-line max-len
`document.lastElementChild.appendChild`,
);
// Head loader script is in the head.
expect(result).toContain(`</script></head>`);
// Only one loader script is created.
expect(Array.from(result.matchAll(/<script>/g)).length).toEqual(1);
});
});

0 comments on commit 210bf4e

Please sign in to comment.