Repository navigation
test: pin the decodeEntities bound, bare named reference and remoteTarget anchors in uri-safety - #1792
test: pin the decodeEntities bound, bare named reference and remoteTarget anchors in uri-safety#1792hivecommons-hive[bot] wants to merge 1 commit into
Conversation
…et anchors
scripts/lib/uri-safety.mjs is at 100% line and region coverage, so what was
missing was assertions, not execution. StrykerJS scored it 90.91% with 10 of
110 mutants surviving; five of those were behaviour no test pinned:
- lines 59/65, `code <= 0x10ffff` weakened to `<`. The suite asserted only the
rejected side of the bound, never that the highest valid code point decodes.
- line 70, `/&([a-z]+);?/gi` narrowed so the semicolon is mandatory. Every
named-reference case wrote the semicolon, so nothing noticed `&colon`
ceasing to decode -- the form a value uses to hide a scheme from a gate that
only decodes the semicolon spelling.
- lines 148 and 166, the `^` anchors deleted from the two authority patterns in
remoteTarget. Without them a relative reference that merely contains `http:`
or `https://` further along its path is reported as a fetch to another host,
which the SVG and MDX gates turn into a build failure against a local asset.
Every remote case in the suite is absolute, so nothing reached past them.
Four cases close those five mutants; the file now scores 95.45%.
The remaining five are left alone deliberately. Line 86's `+` is equivalent
under a global replace with the empty string, and line 111's anchor is reached
only once the scheme has been read as `data` at index 0. The unanchored line
163 mutant has no distinguishing input at all, and the two surviving scheme
mutants (`(https?:)?` and `/^https?:\/\//`) are distinguished only by host-less
values: `remoteTarget('http://')` returns `'http://'` today, a target with no
host, while the comment above line 148 says such a value has nothing to report.
Killing them means pinning a behaviour the source says should not happen, so no
test here does.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
Important Held for human review by the hive's ACMM level gate. This PR was opened by the "quality" agent while Hive policy required a human checkpoint for that agent. Agent PRs are held at ACMM L3–L5; an Hive will keep the |
✅ Deploy Preview for endusers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verified against the tree at cce669b and this looks correct to me. The bound at scripts/lib/uri-safety.mjs:59/:65 is code <= 0x10ffff, the named-reference regex at :70 is /&([a-z]+);?/gi, and both authority patterns at :148/:166 carry the ^ anchors the new cases pin — each test asserts real current behaviour, and TZ=UTC node --test tests/uri-safety.test.mjs passes 15/15 locally. The diff implements the four checkboxes of #1791 exactly, so Closes #1791 is the right link.
One thing a maintainer should know before merging: this PR conflicts with #1790. The Scope section here says no open PR touches tests/uri-safety.test.mjs, but #1790 (copilot/mutants-group-d) edits the same file — it rewrites the same header comment (lines 8–16) and appends tests for the same three behaviours (max code point /, bare &colon, and the mid-path remoteTarget anchors, e.g. remoteTarget('/img/http:a.png') === null). I read both diffs in full: they are semantically overlapping but not textually identical, so whichever merges second will hit a git conflict and need a rebase that drops or reconciles the duplicated cases. #1790 is a fork PR the hive cannot push to, so if #1790 lands first, this PR is largely redundant; if this one lands first, #1790's author needs to rebase its uri-safety.test.mjs hunk.
No defects found. I have not approved or closed anything — merge ordering is for a maintainer to settle.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
|
|
Superseded by #1790 (Group D of #1788), which extended |
Test Improvement
Tests only — no production change. Touches one file,
tests/uri-safety.test.mjs, adding four cases and importingremoteTargetalongside the exports the file already pulls from
scripts/lib/uri-safety.mjs.scripts/lib/uri-safety.mjsis the URI classifier both content gates share, andit is already at 100% lines / 100% regions — the gap is assertion strength, not
execution. StrykerJS scored it 90.91% with 10 of 110 mutants surviving.
Five were behaviour nothing in the suite pinned:
code <= 0x10ffffweakened to<in both the hex andthe decimal decoder. The suite asserted only the rejected side of the bound
(
�stays as written) and never that the highest valid code pointdecodes. New case:
andboth resolve toString.fromCodePoint(0x10ffff)./&([a-z]+);?/ginarrowed so the semicolon is mandatory. Everynamed-reference case in the suite wrote the semicolon, so nothing failed when
&colonstopped decoding — and that is the form a value uses to hide a schemefrom a gate decoding only the semicolon spelling. New case asserts
&colonand
&solresolve bare, and thatnormalizeUri('javascript&colon alert(1)')collapses tojavascript:alert(1).^anchors deleted from the two authoritypatterns in
remoteTarget. Verified against the mutated patterns directly:remoteTarget('/img/http:logo.png')isnullwith the line-148 anchor and'http://http:logo.png'without it;remoteTarget('/img/https://evil.example/a.png')is
nullwith the line-166 anchor and the whole relative path without it.Both mutants turn a local reference into a remote finding, which the SVG and
MDX gates surface as a build failure the author cannot act on. Every remote
case in the suite today is absolute, so nothing reached past the anchors.
The file now scores 95.45% with 5 survivors, and the file header comment is
updated to say what
remoteTargetis and is not asserted on here.Why the other five survivors are left alone
[\u0000-\u0020\u007f]+without the+: equivalent under a globalreplace with the empty string.
/^data:([^,;]*)/unanchored: reached only after the scheme hasbeen read as
dataat index 0, so the leftmost match is the same one./^(https?:)?[/\\]{2,}/unanchored: a match past index 0 cannotmake the rewritten value start with
//, so the return value neverchanges. Exhaustive search over
http:,https:,/,\,a,?,#,.,:to length 4 finds no distinguishing input.(https?:)?→(https:)?and line 166/^https?:\/\//→/^https:\/\//: the same search distinguishes these only on host-lessvalues,
http://andhttp:///.remoteTarget('http://')returns'http://'today — a target with no host — while the comment above line 148states such a value has nothing to report. Killing these two means pinning a
behaviour the source says should not happen, so this PR does not. Resolving
that inconsistency is a production change in
remoteTargetand is recordedin [quality] pin the decodeEntities code-point boundary, the optional-semicolon named reference, and the two remoteTarget anchors in uri-safety.mjs #1791 rather than worked around here.
Verification
Run locally at
761fa39(node v26.12.0):TZ=UTC npx stryker run --mutate scripts/lib/uri-safety.mjs— 90.91% / 10survivors before, 95.45% / 5 survivors after
TZ=UTC node --test tests/uri-safety.test.mjs— 15/15 passnpx prettier --checkandnpx eslint --max-warnings 0clean on the changedfile
Related Issue
Closes #1791
Scope
Touches only
tests/uri-safety.test.mjs, which no open held PR modifies:#1782 is
tests/mdx-escape.test.mjs(scripts/lib/mdx-escape.mjs), #1784 istests/profile-image.test.mjs(scripts/lib/profile-image.mjs), and #1786 istests/validate-utils.test.mjs(scripts/lib/validate-utils.mjs). None ofthem touch
scripts/lib/uri-safety.mjsor its test.Human review required.
— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88