[scanner] e2e coverage seal/report crash on any script URL with malformed percent-encoding (unguarded decodeURIComponent)
Maintainers usually reply within 1 day
A pull request for this has already been merged.
- #1159 by @mrbobbytables — merged
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 88/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- javascript, node.js
- Domain
- tooling
Research direction
Start in tests/tools/e2e-coverage-scripts.mjs: wrap the decodeURIComponent call in scriptPathname() in try/catch falling back to the raw pathname, and verify isEligibleScript() still passes a % -containing URL. Apply the same guard at the three unguarded decode sites in tests/tools/e2e-coverage-report.mjs (convertScript line 384, sourcePathFromReference line 117, decodeDataUrl line 156). Add a regression test proving a URL like http://localhost:3000/assets/js/100%.js is captured/reported without throwing; done when the reproduction command in the issue prints the pathname instead of URIError.
Written by the indexing model from the issue text.
Description
Finding
scriptPathname() in tests/tools/e2e-coverage-scripts.mjs (added in #1040, main @ 7ab301e) calls decodeURIComponent(parsed.pathname) with no guard. A script URL whose path contains a literal % not followed by two hex digits — perfectly valid in a URL and preserved as-is by the URL parser — throws URIError: URI malformed.
isEligibleScript() only try/catches new URL(), so such a URL passes eligibility and then crashes both unguarded call sites:
- Seal step —
captureRunScripts()doeswanted.set(scriptPathname(scriptCoverage.url), ...)(e2e-coverage-scripts.mjs:181).sealCoverageRunawaits this (e2e-coverage-run.mjs:124), and ci.yml:224 runse2e-coverage-run.mjs sealwithif: always(). The throw fails the seal step inside the End-to-end coverage job. - Reporter —
convertScript()callsscriptPathname(scriptCoverage.url)(e2e-coverage-report.mjs:384), aborting the whole report that enforces the--check-source 100 / --check-source-regions 80 / --require-source-filesmerge gate.
So one executed static asset with a % in its filename (e.g. assets/js/100%.js) breaks the e2e coverage gate for every subsequent run until the asset is renamed.
Steps to Reproduce / Evidence
$ node -e "import('./tests/tools/e2e-coverage-scripts.mjs').then(m => {
const url = 'http://localhost:3000/assets/js/100%.js';
console.log(m.isEligibleScript(url)); // true
m.scriptPathname(url); // throws
})"
true
URIError: URI malformed
A static file named 100%.js served by the dev server loads and executes normally; Chromium reports its coverage URL verbatim, and the seal step dies on it. The same unguarded decode pattern appears twice more in the reporter: sourcePathFromReference (e2e-coverage-report.mjs:117, on source-map sources entries) and decodeDataUrl (e2e-coverage-report.mjs:156, on non-base64 inline data: URLs) — both decode tool-controlled strings that are usually well-formed, but a malformed one aborts the report the same way.
Recommendation
Make the decode total: wrap decodeURIComponent in try/catch inside scriptPathname() (falling back to the raw pathname — containment is re-checked after resolve/realpath either way, so no security invariant depends on the decode succeeding), and apply the same guard at the two reporter sites. Add a regression test pinning that a %-containing script URL is captured/reported (or cleanly skipped) rather than crashing the seal.
Filed by scanner agent (ACMM L4 — issues-only mode)
🐝 Hive Agent: scanner | Instance: hosted-available-lke648397-260827-5n31 | SHA: unknown
— hive: agent=scanner backend=copilot model=kimi-k3 copilot=1.0.88
- Dominant language
- JavaScript
- Stars
- 0
- Forks
- 2
- Avg merge
- 21h 59m
- Merged PRs (30d)
- 431
Getting set up
Starts the project's dev container in your browser, under your own GitHub account.
- No Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from cncf/endusers
-
agent/scanner hive/hosted-available-lke648397-260827-5n31 quality testing
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
Maintainers usually reply within 1 day
-
enhancement security
Difficulty 5/5 Over a week Newbie friendliness 35/100
Maintainers usually reply within 1 day
-
agent/quality hive/hosted-available-lke648397-260827-5n31 quality testing
Difficulty 4/5 3-5 days Newbie friendliness 42/100
Maintainers usually reply within 1 day
-
quality testing
Difficulty 4/5 3-5 days Newbie friendliness 52/100
cncf/endusers#1187 · 1 comment ·
Maintainers usually reply within 1 day
-
agent/quality hive/hosted-available-lke648397-260827-5n31 hive/verified-open needs-human quality testing
Difficulty 4/5 3-5 days Newbie friendliness 35/100
cncf/endusers#1079 · 13 comments ·
Maintainers usually reply within 1 day
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
naver/egjs-flicking#971 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
Maintainers usually reply within 1 day