Enhance IP checks and hostname parsing

[HackingRepo]

No description provided.

[qodo-code-review]

Review Summary by Qodo

Enhance IP checks and hostname parsing for security

✨ Enhancement 🐞 Bug fix

Walkthroughs

Description

• Enhanced IPv6 internal IP detection with IPv4-mapped and IPv4-compatible address handling
• Added checks for additional IPv6 CIDR ranges (64:ff9b:1::/48, 5f00::/8, 3fff::/20, fec0::/10)
• Fixed hostname parsing in URL validation by removing IPv6 bracket notation
• Imported node:net module for improved IP validation capabilities

Diagram

flowchart LR
  A["URL Input"] --> B["Normalize & Parse"]
  B --> C["Extract Hostname"]
  C --> D["Remove IPv6 Brackets"]
  D --> E["Validate IP Type"]
  E --> F["IPv4 Check"]
  E --> G["IPv6 Check"]
  G --> H["Check IPv4-mapped/Compatible"]
  H --> I["Check Extra CIDR Ranges"]
  I --> J["Security Decision"]
  F --> J

Loading

File Changes

1. src/helpers.ts Enhancement, bug fix +29/-7

Enhanced IP validation and hostname bracket removal

• Added node:net import for enhanced IP validation
• Refactored is_ip_internal() function to handle IPv4-mapped and IPv4-compatible IPv6 addresses by
 recursively checking their IPv4 equivalents
• Added checks for four additional IPv6 CIDR ranges that should be blocked (64:ff9b:1::/48,
 5f00::/8, 3fff::/20, fec0::/10)
• Fixed hostname parsing in both is_url_safe() and is_url_safe_debug() functions to strip IPv6
 bracket notation using regex replacement

src/helpers.ts

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0)

1. Redirect IPv6 normalization gap 🐞 Bug ⛨ Security

Description

is_url_safe() now strips IPv6 literal brackets before internal/DNS checks, but is_redirect_safe()
still passes current.hostname directly into is_hostname_resolve_to_internal_ip(), so redirect-hop
safety can differ from the main URL safety logic for bracketed IPv6 literals.

Code

src/helpers.ts[516]

+    const hostname = parsed.hostname.replace(/^\[|\]$/g, "");

Evidence

is_url_safe() explicitly strips leading/trailing brackets from URL.hostname before calling
is_hostname_resolve_to_internal_ip(), implying downstream checks expect a bare host.
is_redirect_safe() does not apply this normalization and instead passes current.hostname as-is into
is_hostname_resolve_to_internal_ip(), whose direct-IP path only runs when ipaddr.isValid(host) is
true; this can cause redirect-hop checks to miss IP-literal classification when the hostname is not
in the normalized form expected by ipaddr/is_ipv6.

src/helpers.ts[451-462]
src/helpers.ts[504-535]
src/helpers.ts[310-317]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`is_url_safe()` normalizes IPv6-literal hostnames by stripping `[` and `]` before calling `is_hostname_resolve_to_internal_ip()`, but `is_redirect_safe()` does not. This can make redirect-hop validation inconsistent with the primary URL validation for IPv6 literals.

### Issue Context
- `is_url_safe()` now does `parsed.hostname.replace(/^\[|\]$/g, "")`.
- `is_redirect_safe()` currently passes `current.hostname` directly into `is_hostname_resolve_to_internal_ip()`.
- `is_hostname_resolve_to_internal_ip()` only performs direct-IP classification when `ipaddr.isValid(host)` is true.

### Fix Focus Areas
- src/helpers.ts[451-477]
- src/helpers.ts[310-317]
- src/helpers.ts[504-532]

### Suggested fix
Prefer a single canonical normalization point:
1) Add bracket-stripping inside `is_hostname_resolve_to_internal_ip()` (e.g., `const host = hostname.trim().replace(/^\[|\]$/g, "");`) so *all* callers are protected.
2) Optionally also normalize `current.hostname` in `is_redirect_safe()` for clarity/defense-in-depth.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unused net import 🐞 Bug ⚙ Maintainability

Description

src/helpers.ts imports node:net but never uses it, leaving dead code and an unnecessary dependency
in the emitted output.

Code

src/helpers.ts[5]

+import * as net from "node:net"; // Added for net.isIPv6 check

Evidence

The module imports node:net, but there are no references to net anywhere else in the file (only
the import line matches).

src/helpers.ts[3-18]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`import * as net from "node:net"` is unused in `src/helpers.ts`.

### Issue Context
This adds dead code and can increase bundling/runtime surface without benefit.

### Fix Focus Areas
- src/helpers.ts[3-6]

### Suggested fix
- Remove the `node:net` import if it’s not needed.
- If it *is* intended, replace the current IPv6 detection/normalization logic to actually use `net.isIPv6(...)` and ensure the import is referenced.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. CIDR reparsed per call 🐞 Bug ➹ Performance

Description

is_ip_internal() reparses constant IPv6 CIDR ranges on every invocation, adding avoidable overhead
in DNS classification paths that may call is_ip_internal repeatedly.

Code

src/helpers.ts[R250-260]

+        // Additional manual blocks for IPv6
+        const extraBadRanges = [
+            ipaddr.parseCIDR("64:ff9b:1::/48"),
+            ipaddr.parseCIDR("5f00::/8"),
+            ipaddr.parseCIDR("3fff::/20"),
+            ipaddr.parseCIDR("fec0::/10"),
+        ];
+
+        for (const [range, bits] of extraBadRanges) {
+            // @ts-ignore
+            if (parsed.match(range, bits)) return true;

Evidence

The new extraBadRanges array is constructed inside is_ip_internal(), so ipaddr.parseCIDR() runs
every time is_ip_internal() is called. is_ip_internal() is used inside IP classification helpers and
hostname resolution logic, which can invoke it multiple times per URL/hostname validation.

src/helpers.ts[225-262]
src/helpers.ts[289-301]
src/helpers.ts[320-345]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`is_ip_internal()` reparses the same IPv6 CIDRs on every call via `ipaddr.parseCIDR(...)`.

### Issue Context
These CIDRs are constants and can be parsed once at module load time.

### Fix Focus Areas
- src/helpers.ts[225-262]

### Suggested fix
- Move `extraBadRanges` to a module-scope `const EXTRA_BAD_IPV6_RANGES = [...]`.
- Reuse it inside `is_ip_internal()` to avoid repeated parsing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: build (24.x)

Failed stage: Run npm run build --if-present [❌]

Failed test name: ""

Failure summary: The action failed during the npm run build --if-present step when tsc (TypeScript compiler) exited with code 2 due to TypeScript configuration errors in tsconfig.json: - tsconfig.json(5,25): TS5107 — moduleResolution=node10 is deprecated and treated as an error; TypeScript suggests adding compilerOption: "ignoreDeprecations": "6.0" or updating the setting. - tsconfig.json(8,5): TS5011 — the common source directory is ./src, but rootDir is not explicitly set; rootDir must be set to ./src (or another appropriate path) to control the output layout.

Relevant error logs: 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 137: shell: /usr/bin/bash -e {0} 138: ##[endgroup] 139: added 108 packages, and audited 109 packages in 3s 140: 41 packages are looking for funding 141: run `npm fund` for details 142: 1 moderate severity vulnerability 143: To address all issues, run: 144: npm audit fix 145: Run `npm audit` for details. 146: ##[group]Run npm run build --if-present 147: �[36;1mnpm run build --if-present�[0m 148: shell: /usr/bin/bash -e {0} 149: ##[endgroup] 150: > [email protected] build 151: > tsc 152: ##[error]tsconfig.json(5,25): error TS5107: Option 'moduleResolution=node10' is deprecated and will stop functioning in TypeScript 7.0. Specify compilerOption '"ignoreDeprecations": "6.0"' to silence this error. 153: Visit https://aka.ms/ts6 for migration information. 154: ##[error]tsconfig.json(8,5): error TS5011: The common source directory of 'tsconfig.json' is './src'. The 'rootDir' setting must be explicitly set to this or another path to adjust your output's file layout. 155: Visit https://aka.ms/ts6 for migration information. 156: ##[error]Process completed with exit code 2. 157: Post job cleanup.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[qodo-code-review]

1. Redirect ipv6 normalization gap 🐞 Bug ⛨ Security

is_url_safe() now strips IPv6 literal brackets before internal/DNS checks, but is_redirect_safe()
still passes current.hostname directly into is_hostname_resolve_to_internal_ip(), so redirect-hop
safety can differ from the main URL safety logic for bracketed IPv6 literals.

Agent Prompt

### Issue description
`is_url_safe()` normalizes IPv6-literal hostnames by stripping `[` and `]` before calling `is_hostname_resolve_to_internal_ip()`, but `is_redirect_safe()` does not. This can make redirect-hop validation inconsistent with the primary URL validation for IPv6 literals.

### Issue Context
- `is_url_safe()` now does `parsed.hostname.replace(/^\[|\]$/g, "")`.
- `is_redirect_safe()` currently passes `current.hostname` directly into `is_hostname_resolve_to_internal_ip()`.
- `is_hostname_resolve_to_internal_ip()` only performs direct-IP classification when `ipaddr.isValid(host)` is true.

### Fix Focus Areas
- src/helpers.ts[451-477]
- src/helpers.ts[310-317]
- src/helpers.ts[504-532]

### Suggested fix
Prefer a single canonical normalization point:
1) Add bracket-stripping inside `is_hostname_resolve_to_internal_ip()` (e.g., `const host = hostname.trim().replace(/^\[|\]$/g, "");`) so *all* callers are protected.
2) Optionally also normalize `current.hostname` in `is_redirect_safe()` for clarity/defense-in-depth.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools