Skip to content

Commit c7b6b4c

Browse files
committed
fix(@angular/build): Fixing auto-csp edge cases where
- <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" ...>
1 parent aed726f commit c7b6b4c

File tree

2 files changed

+59
-14
lines changed

2 files changed

+59
-14
lines changed

packages/angular/build/src/utils/index-file/auto-csp.ts

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,19 +86,28 @@ export async function autoCsp(html: string, unsafeEval = false): Promise<string>
8686
let openedScriptTag: StartTag | undefined = undefined;
8787
let scriptContent: SrcScriptTag[] = [];
8888
const hashes: string[] = [];
89+
let inBody = false;
8990

9091
/**
9192
* Generates the dynamic loading script and puts it in the rewriter and adds the hash of the dynamic
9293
* loader script to the collection of hashes to add to the <meta> tag CSP.
9394
*/
9495
function emitLoaderScript() {
95-
const loaderScript = createLoaderScript(scriptContent);
96+
const loaderScript = createLoaderScript(
97+
scriptContent,
98+
/* enableTrustedTypes = */ false,
99+
inBody,
100+
);
96101
hashes.push(hashTextContent(loaderScript));
97102
rewriter.emitRaw(`<script>${loaderScript}</script>`);
98103
scriptContent = [];
99104
}
100105

101106
rewriter.on('startTag', (tag, html) => {
107+
if (tag.tagName === 'body') {
108+
inBody = true;
109+
}
110+
102111
if (tag.tagName === 'script') {
103112
openedScriptTag = tag;
104113
const src = getScriptAttributeValue(tag, 'src');
@@ -152,7 +161,7 @@ export async function autoCsp(html: string, unsafeEval = false): Promise<string>
152161
}
153162
}
154163

155-
if (tag.tagName === 'body' || tag.tagName === 'html') {
164+
if (tag.tagName === 'head' || tag.tagName === 'body' || tag.tagName === 'html') {
156165
// Write the loader script if a string of <script>s were the last opening tag of the document.
157166
if (scriptContent.length > 0) {
158167
emitLoaderScript();
@@ -258,7 +267,11 @@ function getStrictCsp(
258267
* Returns JS code for dynamically loading sourced (external) scripts.
259268
* @param srcList A list of paths for scripts that should be loaded.
260269
*/
261-
function createLoaderScript(srcList: SrcScriptTag[], enableTrustedTypes = false): string {
270+
function createLoaderScript(
271+
srcList: SrcScriptTag[],
272+
enableTrustedTypes = false,
273+
inBody = true,
274+
): string {
262275
if (!srcList.length) {
263276
throw new Error('Cannot create a loader script with no scripts to load.');
264277
}
@@ -267,7 +280,7 @@ function createLoaderScript(srcList: SrcScriptTag[], enableTrustedTypes = false)
267280
// URI encoding means value can't escape string, JS, or HTML context.
268281
const srcAttr = encodeURI(s.src).replaceAll("'", "\\'");
269282
// Can only be 'module' or a JS MIME type or an empty string.
270-
const typeAttr = s.type ? "'" + s.type + "'" : undefined;
283+
const typeAttr = s.type ? "'" + s.type + "'" : "''";
271284
const asyncAttr = s.async ? 'true' : 'false';
272285
const deferAttr = s.defer ? 'true' : 'false';
273286

@@ -288,7 +301,7 @@ function createLoaderScript(srcList: SrcScriptTag[], enableTrustedTypes = false)
288301
s.type = scriptUrl[1];
289302
s.async = !!scriptUrl[2];
290303
s.defer = !!scriptUrl[3];
291-
document.body.appendChild(s);
304+
document.${inBody ? 'body' : 'head'}.appendChild(s);
292305
});\n`
293306
: `
294307
var scripts = [${srcListFormatted}];
@@ -298,6 +311,6 @@ function createLoaderScript(srcList: SrcScriptTag[], enableTrustedTypes = false)
298311
s.type = scriptUrl[1];
299312
s.async = !!scriptUrl[2];
300313
s.defer = !!scriptUrl[3];
301-
document.body.appendChild(s);
314+
document.${inBody ? 'body' : 'head'}.appendChild(s);
302315
});\n`;
303316
}

packages/angular/build/src/utils/index-file/auto-csp_spec.ts

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ const getCsps = (html: string) => {
1818
const ONE_HASH_CSP =
1919
/script-src 'strict-dynamic' 'sha256-[^']+' https: 'unsafe-inline';object-src 'none';base-uri 'self';/;
2020

21+
const TWO_HASH_CSP =
22+
/script-src 'strict-dynamic' (?:'sha256-[^']+' ){2}https: 'unsafe-inline';object-src 'none';base-uri 'self';/;
23+
2124
const FOUR_HASH_CSP =
2225
/script-src 'strict-dynamic' (?:'sha256-[^']+' ){4}https: 'unsafe-inline';object-src 'none';base-uri 'self';/;
2326

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

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

8285
it('should rewrite a multiple source scripts with attributes', async () => {
8386
const result = await autoCsp(`
8487
<html>
8588
<head>
89+
<script src="./head.js"></script>
8690
</head>
8791
<body>
8892
<script src="./main1.js"></script>
@@ -96,13 +100,15 @@ describe('auto-csp', () => {
96100

97101
const csps = getCsps(result);
98102
expect(csps.length).toBe(1);
99-
expect(csps[0]).toMatch(ONE_HASH_CSP);
103+
expect(csps[0]).toMatch(TWO_HASH_CSP);
100104
expect(result).toContain(
101105
// eslint-disable-next-line max-len
102-
`var scripts = [['./main1.js', undefined, false, false],['./main2.js', undefined, true, false],['./main3.js', 'module', true, true]];`,
106+
`var scripts = [['./main1.js', '', false, false],['./main2.js', '', true, false],['./main3.js', 'module', true, true]];`,
103107
);
104-
// Only one loader script is created.
105-
expect(Array.from(result.matchAll(/<script>/g)).length).toEqual(1);
108+
// Head loader script is in the head.
109+
expect(result).toContain(`</script></head>`);
110+
// Only two loader scripts are created.
111+
expect(Array.from(result.matchAll(/<script>/g)).length).toEqual(2);
106112
});
107113

108114
it('should rewrite source scripts with weird URLs', async () => {
@@ -160,14 +166,40 @@ describe('auto-csp', () => {
160166
// Loader script for main.js and main2.js appear after 'foo' and before 'bar'.
161167
expect(result).toMatch(
162168
// eslint-disable-next-line max-len
163-
/console.log\('foo'\);<\/script>\s*<script>\s*var scripts = \[\['.\/main.js', undefined, false, false\],\['.\/main2.js', undefined, false, false\]\];[\s\S]*console.log\('bar'\);/,
169+
/console.log\('foo'\);<\/script>\s*<script>\s*var scripts = \[\['.\/main.js', '', false, false\],\['.\/main2.js', '', false, false\]\];[\s\S]*console.log\('bar'\);/,
164170
);
165171
// Loader script for main3.js and main4.js appear after 'bar'.
166172
expect(result).toMatch(
167173
// eslint-disable-next-line max-len
168-
/console.log\('bar'\);<\/script>\s*<script>\s*var scripts = \[\['.\/main3.js', undefined, false, false\],\['.\/main4.js', undefined, false, false\]\];/,
174+
/console.log\('bar'\);<\/script>\s*<script>\s*var scripts = \[\['.\/main3.js', '', false, false\],\['.\/main4.js', '', false, false\]\];/,
169175
);
170176
// Exactly 4 scripts should be left.
171177
expect(Array.from(result.matchAll(/<script>/g)).length).toEqual(4);
172178
});
179+
180+
it('should write a loader script that appends to head', async () => {
181+
const result = await autoCsp(`
182+
<html>
183+
<head>
184+
<script src="./head.js"></script>
185+
</head>
186+
<body>
187+
<div>Some text </div>
188+
</body>
189+
</html>
190+
`);
191+
192+
const csps = getCsps(result);
193+
expect(csps.length).toBe(1);
194+
expect(csps[0]).toMatch(ONE_HASH_CSP);
195+
196+
expect(result).toContain(
197+
// eslint-disable-next-line max-len
198+
`document.head.appendChild`,
199+
);
200+
// Head loader script is in the head.
201+
expect(result).toContain(`</script></head>`);
202+
// Only one loader script is created.
203+
expect(Array.from(result.matchAll(/<script>/g)).length).toEqual(1);
204+
});
173205
});

0 commit comments

Comments
 (0)