Skip to content

refactor: share loopback IP checks across URL and RPC policies - #945

Merged
LivXue merged 1 commit into
mainfrom
refactor/shared_loopback_check
Oct 14, 2026
Merged

LivXue merged 1 commit into
mainfrom
refactor/shared_loopback_check

Conversation

@LivXue

@LivXue LivXue commented Oct 14, 2026

Copy link
Copy Markdown
Member

Summary

RPC browser-origin checks and URL plaintext checks each implemented the same loopback IP classification. Both now use security.hosts.is_loopback_ip, which reuses the existing as_ip parser.

The callers retain their name policies: RPC accepts localhost alongside loopback IPs; URL plaintext checks also allow localhost.localdomain and trailing-dot aliases. Origin/Host matching and malformed-origin refusals remain in the RPC layer. Regression cases exercise both accepted and refused hosts through the real sign-in handler.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

Linux, Python 3.12.13, with existing dependencies matching the lockfile for the affected modules and check tools:

uv run --no-sync pytest tests/test_security_hosts.py tests/test_security_urls.py tests/test_rpc_transport.py tests/test_security_network.py tests/test_security_web_ssrf.py tests/test_browser_policy.py tests/test_market.py tests/test_rpc_skillhub.py tests/test_skill_hub_policy.py tests/test_skill_hub_client.py tests/test_cli_serve_commands.py -q

726 passed; one supervisor test timed out waiting for its child to write the state file. Its isolated rerun passed:

uv run --no-sync pytest tests/test_cli_serve_commands.py::TestTheSupervisorCleansUpOnTheSignalThatStopsIt::test_sigterm_removes_the_state_file -q -n 0

The expanded URL/RPC boundary cases passed before the production refactor (60 passed). A separate comparison of the original and shared predicates found no differences across 6,535 host strings and 39,214 origins, including legacy addresses, aliases, mapped IPv6 and malformed brackets.

These checks passed:

uv run --no-sync ruff check raven/security/hosts.py raven/security/urls.py raven/rpc/transports/ws.py tests/test_security_hosts.py tests/test_security_urls.py tests/test_rpc_transport.py
uv run --no-sync ruff format --check raven/security/hosts.py raven/security/urls.py raven/rpc/transports/ws.py tests/test_security_hosts.py tests/test_security_urls.py tests/test_rpc_transport.py
uv run --no-sync ty check raven/security raven/rpc/transports
uv run --no-sync lint-imports
uv run --no-sync pre-commit run --files raven/security/hosts.py raven/security/urls.py raven/rpc/transports/ws.py tests/test_security_hosts.py tests/test_security_urls.py tests/test_rpc_transport.py
  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

No user-facing documentation change is needed for this refactor. The full repository suite and native Windows/macOS runs were not performed. The supervisor timeout was not compared against an unmodified main checkout.

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

The change touches authentication and URL trust predicates. It retains each caller's local-name rules and the standard library's IP classification, including acceptance of IPv4-mapped loopback on Python 3.12.13. No DNS resolution, listeners, credentials or configuration are added. Reverting this refactor restores the previous separate checks without a data migration.

Related Issues

#939

Move canonical loopback IP classification into security.hosts and reuse it
from the browser origin and URL plaintext policies. Keep each caller's
local-name rules and the browser Origin/Host check in place.

Cover canonical addresses, legacy spellings, local aliases and forwarded
sign-ins so sharing the IP predicate preserves both policies.
@LivXue
LivXue requested review from 0xKT and gloryfromca October 14, 2026 07:59

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No blockers; this can merge as far as I am concerned.

Reviewed the full diff plus the surrounding URL/RPC policy code, callers, the originating port-forward history, backward compatibility, test changes, AGENTS.md/CLAUDE.md, CONTEXT-MAP.md and the security/layer-seat architecture rules. The shared predicate preserves the URL policy's rejection of legacy numeric spellings, keeps each caller's distinct local-name rules, and introduces only the documented surface-to-security dependency. I found no weakened tests.

Verification: uv run --no-sync pytest tests/test_security_hosts.py tests/test_security_urls.py tests/test_rpc_transport.py -x passed (168 tests); targeted Ruff check and format checks passed; git diff --check and the source-language check passed. The initial test run could not start because the declared dev plugin was absent, so I synced the repository's dev extra with uv and reran unchanged. CI's unit shards are still pending; all currently completed checks pass.

@ZuyiZhou ZuyiZhou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The shared is_loopback_ip is equivalent to both old predicates: the legacy-form guard in the URL check was redundant because as_ip already returns None for those spellings, and I found no origin differences on bracket edge cases for the RPC check. Each caller keeps its own name policy. The new tests pin the accepted and refused hosts through the real sign-in handler. CI green at 554705d.

@LivXue
LivXue merged commit fcb36ba into main Oct 14, 2026
24 checks passed
@LivXue
LivXue deleted the refactor/shared_loopback_check branch October 14, 2026 09:49
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.

3 participants