Skip to content

fix: test failures due to esm.sh rate limits - #5511

Draft
Eugene (OEvgeny) wants to merge 9 commits into
mainfrom
fix/tests
Draft

fix: test failures due to esm.sh rate limits#5511
Eugene (OEvgeny) wants to merge 9 commits into
mainfrom
fix/tests

Conversation

@OEvgeny

@OEvgeny Eugene (OEvgeny) commented Jul 8, 2025

Copy link
Copy Markdown
Collaborator

Fixes #

Changelog Entry

Description

Design

Specific Changes

-

  • I have added tests and executed them locally
  • I have updated CHANGELOG.md
  • I have updated documentation

Review Checklist

This section is for contributors to review your work.

  • Accessibility reviewed (tab order, content readability, alt text, color contrast)
  • Browser and platform compatibilities reviewed
  • CSS styles reviewed (minimal rules, no z-index)
  • Documents reviewed (docs, samples, live demo)
  • Internationalization reviewed (strings, unit formatting)
  • package.json and package-lock.json reviewed
  • Security reviewed (no data URIs, check for nonce leak)
  • Tests reviewed (coverage, legitimacy)

github-advanced-security[bot]
github-advanced-security AI found potential problems Jul 8, 2025
View reviewed changes
Comment thread packages/test/web-server/src/handleEsm.ts Fixed
Comment thread packages/test/web-server/src/handleEsm.ts Fixed
Comment thread packages/test/web-server/src/handleEsm.ts Fixed
Eugene (OEvgeny) and others added 3 commits July 8, 2025 23:56
@OEvgeny
Update test definitions
9198719
@OEvgeny @github-advanced-security
Potential fix for code scanning alert no. 34: Information exposure th…
c39183d 
…rough a stack trace

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@OEvgeny
Fix nits
62a410a
github-advanced-security[bot]
github-advanced-security AI found potential problems Jul 9, 2025
View reviewed changes
Comment thread packages/test/web-server/src/handleESM.ts
const targetPath = req.url.replace(/^\/esm/u, '');
const upstreamUrl = `http://esm:8080${targetPath}`;

const upstreamRes = await fetch(upstreamUrl);

Check failure

Code scanning / CodeQL

Server-side request forgery

The [URL](1) of this request depends on a [user-provided value](2).

Copilot Autofix

AI about 1 year ago

To fix the SSRF vulnerability, we need to restrict the user input (targetPath) to ensure it cannot be used to construct malicious URLs. Specifically:

  1. Validate targetPath against an allow-list of known safe paths or subpaths.
  2. Ensure that the constructed upstreamUrl always points to a trusted domain and does not allow path traversal or injection of unintended query parameters.

The best approach is to introduce an allow-list of valid paths that the user can request. If the targetPath does not match any entry in the allow-list, the request should be rejected with a 400 Bad Request response.

Suggested changeset 1
packages/test/web-server/src/handleESM.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/test/web-server/src/handleESM.ts b/packages/test/web-server/src/handleESM.ts
--- a/packages/test/web-server/src/handleESM.ts
+++ b/packages/test/web-server/src/handleESM.ts
@@ -44,3 +44,4 @@
     const targetPath = req.url.replace(/^\/esm/u, '');
-    if (!targetPath.startsWith('/')) {
+    const allowedPaths = ['/module1', '/module2', '/module3']; // Example allow-list
+    if (!allowedPaths.includes(targetPath)) {
       res.writeHead(400);
EOF
@@ -44,3 +44,4 @@
const targetPath = req.url.replace(/^\/esm/u, '');
if (!targetPath.startsWith('/')) {
const allowedPaths = ['/module1', '/module2', '/module3']; // Example allow-list
if (!allowedPaths.includes(targetPath)) {
res.writeHead(400);
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@a-b-r-o-w-n Andy Brown (a-b-r-o-w-n) Awaiting requested review from a-b-r-o-w-n a-b-r-o-w-n will be requested when the pull request is marked ready for review a-b-r-o-w-n is a code owner
@compulim William Wong (compulim) Awaiting requested review from compulim compulim will be requested when the pull request is marked ready for review compulim is a code owner
@cwhitten Chris Whitten (cwhitten) Awaiting requested review from cwhitten cwhitten will be requested when the pull request is marked ready for review cwhitten is a code owner
@srinaath Srinaath Ravichandran (srinaath) Awaiting requested review from srinaath srinaath will be requested when the pull request is marked ready for review srinaath is a code owner
@tdurnford TJ Durnford (tdurnford) Awaiting requested review from tdurnford tdurnford will be requested when the pull request is marked ready for review tdurnford is a code owner
Copilot code review Copilot
Copilot will automatically review once the pull request is marked ready for review

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@OEvgeny @github-advanced-security