Prevent forbidden tags in head html template by jurgenwerk · Pull Request #3937 · cardstack/boxel

jurgenwerk · 2026-02-02T10:42:20Z

Do not allow forbidden tags in head templates, only allow <titlte>, <link>, <meta>, do not allow any other ones - strip them out when rendering
Show a warning if user saves a head template with forbidden elements (screenshot below)
Provide a lint rule so that the correctness checks catches the issue in case AI generates code that does not conform to the mentioned rule. The correctness check will automatically fix it

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c59b1c914f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

packages/host/app/utils/sanitize-head-html.ts

Copilot

Pull request overview

This PR introduces a security enhancement by implementing a whitelist-based sanitization mechanism for head HTML insertion. The change prevents potential XSS and code injection attacks by filtering HTML content before inserting it into the document head.

Changes:

Added sanitizeHeadHTML utility function that whitelists only meta and title tags with specific attributes
Modified host-mode-service.ts to use the new sanitization function before inserting HTML into the document head
Added comprehensive unit tests to verify the sanitization behavior

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
packages/host/app/utils/sanitize-head-html.ts	New utility implementing whitelist-based HTML sanitization for head content
packages/host/app/services/host-mode-service.ts	Updated to use sanitization function before inserting head HTML
packages/host/tests/unit/utils/sanitize-head-html-test.ts	Comprehensive test suite for the sanitization function

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/host/tests/unit/utils/sanitize-head-html-test.ts

packages/host/app/utils/sanitize-head-html.ts

packages/runtime-common/helpers/sanitize-head-html.ts

packages/host/app/utils/sanitize-head-html.ts

packages/runtime-common/helpers/sanitize-head-html.ts

github-actions · 2026-02-02T10:50:14Z

Preview deployments

github-actions · 2026-02-02T11:09:27Z

Host Test Results

1 files ±0 1 suites ±0 1h 39m 24s ⏱️ +33s
1 849 tests +2 1 836 ✅ +2 13 💤 ±0 0 ❌ ±0
1 864 runs +2 1 851 ✅ +2 13 💤 ±0 0 ❌ ±0

Results for commit 1c29dc7. ± Comparison against base commit 0d85522.

♻️ This comment has been updated with latest results.

lukemelia

Will users be able to understand this sanitization? Should there be some indicator in code mode that some of your head format output is not allowed? /cc @backspace

backspace · 2026-02-02T16:17:03Z

I know this issue wasn’t that explicit but I think this needs to happen in a different place; maybe where you’ve inserted it is correct, but it should also be present at least in server.ts where the index.html gets content injected into head.

I’m not sure how much of an affordance we want but I’d be surprised to have script tags be stripped out if I included one in my head template, which is a common pattern for adding analytics or the like. Should there be a warning to the user if they add forbidden tags or attributes in a head template?

jurgenwerk · 2026-02-03T10:13:55Z

I think user could add <script> tags in other templates too, for example isolated, so the problem is not strictly tied to the head template. Now that I'm thinking about this - the user could inject malicious code using other mechanisms, such as fetching it using javascript, so if we prevented script tags there are still other ways of injecting external code.

Perhaps we should only validate head template to the point where it can only include the elements from the html head spec, and show a warning/error in code mode if user tries to use any other elements in the head template?

backspace · 2026-02-03T20:02:06Z

Perhaps we should only validate head template to the point where it can only include the elements from the html head spec, and show a warning/error in code mode if user tries to use any other elements in the head template?

okay, but I think we do need to consider the broader problem of XSS anywhere. Maybe it just fits in the “abuse” issue.

jurgenwerk · 2026-02-09T15:20:43Z

notes from office hours:

we don't want to allow <script>, , <style>, others are ok (https://www.w3schools.com/tags/tag_head.asp)
if user does add forbidden elements, show a warning in head format preview
add a lint rule (correctness check will be able to fix it then)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9e03326b76

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-02-16T09:40:26Z

packages/runtime-common/helpers/sanitize-head-html.ts

+  for (let node of Array.from(template.content.childNodes)) {
+    if (node.nodeType === ELEMENT_NODE) {
+      let tagName = (node as Element).tagName.toLowerCase();
+      if (!ALLOWED_HEAD_TAGS.has(tagName) && !disallowed.includes(tagName)) {
+        disallowed.push(tagName);


Surface stripped <link> entries in head warnings

findDisallowedHeadTags only checks whether a top-level tag name is allowlisted, but the sanitizer also drops <link> elements when their rel or href fails safety checks (sanitizeLinkElement). That means inputs like <link rel="stylesheet" ...> or <link rel="icon" href="javascript:..."> are removed at render time with no warning in the head preview, which makes valid-looking head templates fail silently for authors.

Useful? React with 👍 / 👎.

jurgenwerk requested a review from Copilot February 2, 2026 10:42

Copilot started reviewing on behalf of jurgenwerk February 2, 2026 10:42 View session

chatgpt-codex-connector bot reviewed Feb 2, 2026

View reviewed changes

packages/host/app/utils/sanitize-head-html.ts Outdated Show resolved Hide resolved

Copilot AI reviewed Feb 2, 2026

View reviewed changes

jurgenwerk force-pushed the cs-9876-consider-xss-vulnerability-of-head-insertion-implementation branch 2 times, most recently from add26bc to b32c147 Compare February 2, 2026 12:01

jurgenwerk requested a review from a team February 2, 2026 14:35

lukemelia reviewed Feb 2, 2026

View reviewed changes

Sanitize <head> content

87397e8

jurgenwerk force-pushed the cs-9876-consider-xss-vulnerability-of-head-insertion-implementation branch from b32c147 to 87397e8 Compare February 13, 2026 11:13

jurgenwerk marked this pull request as draft February 13, 2026 11:38

jurgenwerk added 4 commits February 13, 2026 13:10

Cleanup

b6e0b7c

Add missing test entry

7d4b076

Add a lint rule guarding against forbidden elements in head tag

c1f8d8e

Lint fix

a9e1e9f

jurgenwerk marked this pull request as ready for review February 16, 2026 09:29

jurgenwerk requested a review from a team February 16, 2026 09:30

chatgpt-codex-connector bot reviewed Feb 16, 2026

View reviewed changes

jurgenwerk changed the title ~~Prevent potential malicious code injection by whitelisting head html insertion~~ Prevent forbidden tags in head html template Feb 16, 2026

Consolidate tests

1c29dc7

jurgenwerk force-pushed the cs-9876-consider-xss-vulnerability-of-head-insertion-implementation branch from 9e03326 to 1c29dc7 Compare February 16, 2026 10:26

lukemelia approved these changes Feb 16, 2026

View reviewed changes

jurgenwerk merged commit cc414d3 into main Feb 17, 2026
106 of 107 checks passed

Comments

Conversation

jurgenwerk commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 2, 2026

Preview deployments

Uh oh!

github-actions bot commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Host Test Results

Uh oh!

lukemelia left a comment

Choose a reason for hiding this comment

Uh oh!

backspace commented Feb 2, 2026

Uh oh!

jurgenwerk commented Feb 3, 2026

Uh oh!

backspace commented Feb 3, 2026

Uh oh!

jurgenwerk commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jurgenwerk commented Feb 2, 2026 •

edited

Loading

github-actions bot commented Feb 2, 2026 •

edited

Loading

jurgenwerk commented Feb 9, 2026 •

edited

Loading