Skip to content

fix(rwa): validate --rpc-url scheme in rwa_lookup.py (#968) - #979

Open
Tiktokaiagent wants to merge 2 commits into
use-agent-os:mainfrom
Tiktokaiagent:fix/968-rwa-lookup-rpc-url
Open

fix(rwa): validate --rpc-url scheme in rwa_lookup.py (#968)#979
Tiktokaiagent wants to merge 2 commits into
use-agent-os:mainfrom
Tiktokaiagent:fix/968-rwa-lookup-rpc-url

Conversation

@Tiktokaiagent

Copy link
Copy Markdown
Contributor

Summary

The lookup script passed --rpc-url straight into urllib.request.urlopen without scheme validation, allowing file:// and other non-http(s) schemes to read arbitrary local files when invoked from an agent context.

Fix

  • Added _validate_rpc_url() using urlsplit that rejects any scheme outside {http, https}
  • Validates at the call site: both _rpc_batch() (line 222) and main() before any network use
  • Corrected all Bandit # noqa: S310 comments to document the validated endpoint

Closes #968

Testing

39 deterministic regression tests:

Tests What they prove
15x test_url_valve Parametrized: 6 valid + 9 invalid schemes
6x TestRpcBatch _rpc_batch validates at call site
8x TestMainCli main() validation via CLI exit code 2
5x TestEdgeCases Paths, ports, query params, auth, case-insensitive
3x TestS310Suppression Every # noqa: S310 has documented endpoint

Quality gate

  • ruff check src tests — passed
  • uv run pytest tests/test_skills/test_rwa_lookup_rpc_url.py -v --tb=short — 39 passed
  • uv build --wheel — passed

@Tiktokaiagent

Copy link
Copy Markdown
Contributor Author

@andreapn gentle bump — still mergeable, all CI green, ready for review whenever you have time. Thanks!

@andreapn andreapn 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 rwa_lookup.py change is the right one: urlsplit-based scheme + host validation enforced at both _rpc_batch() and main(), and the three # noqa: S310 comments corrected. Verified locally — reverting only rwa_lookup.py fails 30 of the 39 new tests, so they are genuinely coupled to the fix.

One blocker, in CHANGELOG.md:

  1. The entry is malformed. Both lines lost their code spans, so the sentence has no subject:

    -  now validates the --rpc-url scheme via _validate_rpc_url
      and rejects any URL outside {http, https}, preventing local file reads (#968).
    
  2. It does not land under [Unreleased]. The branch was cut before the 2026.9.4 release. A trial merge into current main places these lines at CHANGELOG.md:204 — inside the already-shipped ## [2026.9.3] section.

Please rebase onto main and either rewrite the entry under [Unreleased]### Security with the subject restored, or drop CHANGELOG.md from the PR (several recent bundled-skill fixes shipped without one). The source and test changes need no further work.

@Tiktokaiagent
Tiktokaiagent force-pushed the fix/968-rwa-lookup-rpc-url branch from a21db43 to 76412e5 Compare September 5, 2026 05:20
@Tiktokaiagent

Copy link
Copy Markdown
Contributor Author

Fixed as requested:

  1. ✅ Rebased onto latest main (was cut before the 2026.9.4 release)
  2. ✅ Moved CHANGELOG entry to [Unreleased] → ### Security
  3. ✅ Restored the subject line with proper **robinhood-rwa-addresses** now validates --rpc-urlscheme via_validate_rpc_url``
  4. ✅ No stale code from the merge

Please take another look when you have time.

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.

[Bug]: [Bug]: robinhood-rwa-addresses: unvalidated --rpc-url scheme allows local file read via urlopen

2 participants