Skip to content

Conversation

@7amed3li
Copy link

@7amed3li 7amed3li commented Jan 7, 2026

Description

This PR implements the detection of local IP addresses (both IPv4 and IPv6) and localhost hostnames within URLs, as requested in issue #449. This is handled by a new warning type shady-url with Information severity to distinguish it from external shady-link warnings.

Changes

  • Core Logic: Updated ShadyURL class to identify private IP ranges and localhost.
  • IPv6 Fix: Fixed a bug where IPv6 hostnames extracted from URL objects (e.g., [::1]) retained their square brackets, causing ipaddr.js validation to fail.
  • Warning System: Added shady-url to warnings.ts with its localized key and Information severity.
  • Probe Integration: Modified isLiteral probe to differentiate between shady-link and shady-url based on the detection result.
  • Documentation: Added a new documentation file docs/shady-url.md explaining the risks (SSRF, Data Exfiltration, Environment Spillage).
  • Changeset: Included a changeset for a minor version bump.

Verification Results

I have added and updated unit tests in ShadyURL.spec.ts and isLiteral.spec.ts. All 78 tests passed successfully on Windows using tsx.

npx tsx --test workspaces/js-x-ray/test/ShadyURL.spec.ts workspaces/js-x-ray/test/probes/isLiteral.spec.ts  
Total tests: 78
Passing: 78
Failing: 0

@changeset-bot
Copy link

changeset-bot bot commented Jan 7, 2026

🦋 Changeset detected

Latest commit: 16cdffa

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

This PR includes changesets to release 1 package
Name Type
@nodesecure/js-x-ray Minor

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

@7amed3li
Copy link
Author

7amed3li commented Jan 7, 2026

Hi maintainers 👋
This PR is ready from my side.
Please let me know if you’d like me to make any changes or adjustments.
Thanks for your time!

@fraxken
Copy link
Member

fraxken commented Jan 7, 2026

In the issue by throwing a shady-url warning I wanted shady-link (not a new warning that achieve the same purpose ^^).

@7amed3li
Copy link
Author

7amed3li commented Jan 8, 2026

Updated based on feedback

Comment on lines 175 to 179
it("should return false for .link TLD", () => {
assert.equal(ShadyURL.isSafe("https://malicious.link", {
collectableSetRegistry
}), false);
});
Copy link
Member

@fraxken fraxken Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why some of the tests are been deleted (please review the diff)

collectableSetRegistry.add("ip", { value: hostname, file, location: sourceArrayLocation });
if (this.#isPrivateIPAddress(hostname)) {
return true;
// التحقق المبكر من localhost
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??

@7amed3li 7amed3li force-pushed the feat/shady-url-local-detection branch from 6ffe7a8 to 16cdffa Compare January 10, 2026 07:56
@7amed3li
Copy link
Author

Hi @fraxken 👋

Thank you very much for the review and for taking the time to guide me through the changes — I really appreciate it.

This is my first pull request to this project (and one of my first OSS contributions), so I apologize for the earlier mistakes and any confusion caused along the way. Your feedback helped me understand the project conventions much better.

Thanks again for your patience and clear explanations — they were extremely helpful 🙏
Please let me know if there’s anything else you’d like me to adjust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants