fix(security): guard ClawHub downloads against redirect SSRF - #70334
fix(security): guard ClawHub downloads against redirect SSRF#70334zapabob wants to merge 3 commits into
Conversation
Salvage NousResearch#57571 size-cap work: route ZIP/API fetches through _guarded_http_get so CDN redirects cannot reach private targets. Co-authored-by: Cursor <cursoragent@cursor.com>
alt-glitch
added
type/security
Co-authored-by: Cursor <cursoragent@cursor.com>
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the ClawHub ZIP redirect path. The premise is confirmed on current main: tools/skills_hub.py:2666-2670 still uses httpx.get(..., follow_redirects=True), while the existing guard at tools/skills_hub.py:302-338 validates redirect targets and uses the connect-time-safe client at tools/skills_hub.py:294-299.
Problems
tests/tools/test_skills_hub_clawhub_ssrf.py:10mocks_guarded_http_get; it verifies delegation but not the security guarantee that a 302 to a private URL is rejected before a second connection.- Current main pruned 205 lines from
tests/tools/test_skills_hub_clawhub.pyin39975613b1, including regions this PR modifies, so those legacy-test hunks need reconciliation during salvage.
Suggested changes
- Add a focused redirect test that exercises
_guarded_http_getthrough_download_zipand asserts the private Location is never requested. - Preserve the production fix and migrate relevant coverage into the new focused test file rather than retaining obsolete legacy-test edits.
This is an automated hermes-sweeper review.
|
|
||
| def test_download_zip_uses_guarded_http_get(): | ||
| src = ClawHubSource() | ||
| with patch("tools.skills_hub._guarded_http_get", return_value=None) as mock_get: |
There was a problem hiding this comment.
Mocking _guarded_http_get only proves delegation. Add a regression that lets the guard handle a 302 Location to a private URL and verifies _ssrf_safe_http_get is never called for that private target.
teknium1
added
sweeper:risk-compatibility
|
This is a slice of #63920, not a leftover. #63920 already routes ClawHub zip / catalog / JSON helpers through "Not merged yet" is not the same as "ClawHub is unguarded." Please don't land this as the ClawHub fix over #63920. |
## Summary - Move LobeHub index/agent fetches onto `_guarded_http_get` (SSRF + redirect hop checks). - Sanitize `agent_id` to reject path traversal / scheme-bearing identifiers. - Add unit tests for sanitization and guarded-fetch wiring. ## Salvage / credit Skills Hub SSRF campaign siblings (NousResearch#70334 ClawHub, NousResearch#70336 skills.sh, NousResearch#70343 GitHub hub) — LobeHub still used raw `httpx.get`.
|
Closing this overlapping ClawHub slice in favor of consolidated survivor #70343. Its guarded ZIP download and private-redirect regression are preserved in #70343, now rebased onto official |
4 participants
Summary
httpx.get(..., follow_redirects=True), so a CDN 302 could reach private/link-local addresses._get_jsonthrough_guarded_http_get(SSRF + hop re-validation).Test plan
pytest tests/tools/test_skills_hub_clawhub_ssrf.py(2 passed)