Skip to content

fix(auto): use proper URL parsing in _is_local_url#742

Open
u7k4rs6 wants to merge 1 commit into
huggingface:mainfrom
u7k4rs6:fix/auto-is-local-url-parsing
Open

fix(auto): use proper URL parsing in _is_local_url#742
u7k4rs6 wants to merge 1 commit into
huggingface:mainfrom
u7k4rs6:fix/auto-is-local-url-parsing

Conversation

@u7k4rs6

@u7k4rs6 u7k4rs6 commented Jun 3, 2026

Copy link
Copy Markdown

Summary

Replace the naive substring matching in AutoEnv._is_local_url with urllib.parse.urlparse-based hostname extraction, fixing three classes of failure:

  • False positive - http://localhost.evil.com returned True because "localhost" appeared as a substring; the proxy-bypass logic then applied to a hostile host.
  • Missing 0.0.0.0 - was not checked at all.
  • Missing full loopback range - only 127.0.0.1 was matched; 127.x.x.x and the IPv6 loopback ::1 were not covered.

The new implementation:

  1. Parses the URL with urlparse to extract the hostname (handles bracketed IPv6 automatically).
  2. Exact-matches "localhost" and "0.0.0.0".
  3. Uses ipaddress.ip_address(hostname).is_loopback for all numeric addresses, covering 127.0.0.0/8 and ::1.
  4. Wraps everything in try/except so malformed URLs return False.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • New environment
  • Refactoring

Alignment Checklist

Before submitting, verify:

  • I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
  • I have checked .claude/docs/INVARIANTS.md and no invariants are violated
  • I have run /pre-submit-pr (or bash .claude/hooks/lint.sh and tests) and addressed all issues

RFC Status

  • Not required (bug fix, docs, minor refactoring)
  • RFC exists: #___
  • RFC needed (will create before merge)

Test Plan

New parametrized tests added to tests/envs/test_auto_env.py under TestIsLocalUrl:

URL Expected
http://localhost:8000 True
http://127.0.0.1:8000 True
http://127.5.5.5 True (rest of 127.0.0.0/8)
http://[::1]:8000 True (IPv6 loopback)
http://0.0.0.0:8000 True
http://localhost.evil.com False (headline bug)
http://my-127.0.0.1.io False
https://example.com False
not-a-url False
PYTHONPATH=src:envs uv run pytest tests/envs/test_auto_env.py::TestIsLocalUrl -v
# 15 passed

PYTHONPATH=src:envs uv run pytest tests/ -v --ignore=tests/envs
# 867 passed, 12 skipped

uv run ruff format src/ tests/ --check
# 157 files already formatted

Claude Code Review

N/A

Replace naive substring matching with urlparse-based hostname extraction
followed by exact string checks for "localhost"/"0.0.0.0" and
ipaddress.ip_address().is_loopback for numeric addresses.

Fixes three classes of failure:
- False positive: "http://localhost.evil.com" no longer returns True
- Missing 0.0.0.0
- Missing IPv6 loopback ::1 and the full 127.0.0.0/8 range
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.

1 participant