Prevent potential malicious code injection by whitelisting head html insertion

[jurgenwerk]

No description provided.

[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".

[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.

[github-actions]

Preview deployments

• Host staging preview
• Host production preview

[github-actions]

Host Test Results

    1 files  ±0      1 suites  ±0   1h 43m 13s ⏱️ -27s
1 930 tests +5  1 913 ✅ +5  17 💤 ±0  0 ❌ ±0 
1 945 runs  +5  1 928 ✅ +5  17 💤 ±0  0 ❌ ±0 

Results for commit b32c147. ± Comparison against base commit 397e601.

♻️ 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]

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]

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]

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.