fix(core/issues-notes): validate GitHub label color, use html template for heading#5207
Merged
marcoscaceres merged 9 commits intomainfrom Apr 27, 2026
Merged
fix(core/issues-notes): validate GitHub label color, use html template for heading#5207marcoscaceres merged 9 commits intomainfrom
marcoscaceres merged 9 commits intomainfrom
Conversation
…centHTML
Validate bgColor against /^[0-9a-f]{6}$/i before use in style attribute;
invalid/empty values fall back to #f6f8fa (GitHub gray) to prevent CSS
injection. Replace insertAdjacentHTML with html tagged template + prepend
for DOM-safe heading insertion. Add tests for both fixes.
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens the Issues/Notes GitHub label rendering against CSS injection and shifts issue-summary heading insertion to use the existing DOM-safe html templating helper.
Changes:
- Validate GitHub label
colorvalues against a 6-digit hex regex and fall back to#f6f8fawhen invalid. - Replace
insertAdjacentHTML()usage for the issue-summary heading withprepend(html\...`)`. - Add/extend tests covering label color sanitization and heading insertion behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/spec/core/issues-notes-spec.js | Adds assertions/tests for sanitized label colors and for issue-summary heading insertion. |
| src/core/issues-notes.js | Sanitizes GitHub label background colors before interpolating into a style attribute; uses html template for inserting the issue-summary heading. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The issue-summary heading is injected as h1 by issues-notes.js but core/structure.js renames it to h2 based on section nesting depth, and core/id-headers.js then wraps it in div.header-wrapper. The test was asserting tagName "H1" directly, which would always fail after structure processing. Update to query the final rendered structure consistent with surrounding localization tests.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Add explicit size check on getElementsByClassName result before positional destructuring to catch fixture changes early.
sidvishnoi
changed the title
sidvishnoi
reviewed
Apr 25, 2026
Remove the standalone "sanitizes GitHub label colors" test — its
assertions duplicate those already in "shows labels for github issues".
Remove the "generates issue summary heading as a DOM element" test —
as Sid correctly noted, instanceof HTMLHeadingElement passes regardless
of whether the element was created via insertAdjacentHTML or DOM APIs,
so the test could not distinguish between them.
Remove the weak not.toContain("javascript") assertion on blankLabel
(empty color can never produce "javascript" in the style attribute).
sidvishnoi
approved these changes
Apr 26, 2026
marcoscaceres
added a commit
that referenced
this pull request
Apr 28, 2026
…e for heading (#5207) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
marcoscaceres
added a commit
that referenced
this pull request
Apr 28, 2026
…e for heading (#5207) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bgColorfrom GitHub labels API is now validated against/^[0-9a-f]{6}$/ibefore being interpolated into astyleattribute. Invalid values fall back to#f6f8fa(GitHub's default gray). This prevents a malformed or adversarially crafted label color from injecting arbitrary CSS properties.insertAdjacentHTML("afterbegin", `<h1>...\`)inmakeIssueSectionSummarywithprepend(html\`<h1>...\`)— thehtmltagged template literal is already used throughout the file and creates DOM nodes safely rather than parsing HTML strings.Test plan
pnpm start --browser ChromeHeadless --grep="Core - Issues and Notes"pnpm start --browser FirefoxHeadless --grep="Core - Issues and Notes"