Skip to content

fix: harden attribute escaping#15740

Merged
matthewp merged 1 commit intomainfrom
sec-attr-amp
Mar 4, 2026
Merged

fix: harden attribute escaping#15740
matthewp merged 1 commit intomainfrom
sec-attr-amp

Conversation

@matthewp
Copy link
Contributor

@matthewp matthewp commented Mar 3, 2026

Changes

  • Removes a special case in addAttribute() that skipped escaping for URL values containing &
  • All dynamic attribute values now go through consistent escaping via toAttributeString()
  • Removes the now-unused isHttpUrl helper

Testing

  • packages/astro/test/units/app/url-attribute-xss.test.js (new)
  • Updated assertion in packages/astro/test/astro-attrs.test.js

Docs

N/A, bug fix

@changeset-bot
Copy link

changeset-bot bot commented Mar 3, 2026

🦋 Changeset detected

Latest commit: 3ec0f35

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Mar 3, 2026
@matthewp matthewp changed the title fix: harden attribute escaping by removing URL escape bypass fix: harden attribute escaping Mar 3, 2026
@matthewp matthewp marked this pull request as ready for review March 3, 2026 19:39
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 3, 2026

Merging this PR will not alter performance

✅ 18 untouched benchmarks


Comparing sec-attr-amp (3ec0f35) with main (e6e146c)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (29682f3) during the generation of this report, so e6e146c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@matthewp matthewp merged commit c5016fc into main Mar 4, 2026
41 of 48 checks passed
@matthewp matthewp deleted the sec-attr-amp branch March 4, 2026 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants