Skip to content

fix(security): guard skills.sh sitemap and detail HTTP fetches - #70336

Closed
zapabob wants to merge 2 commits into
NousResearch:mainfrom
zapabob:fix/skills-sh-http-ssrf
Closed

fix(security): guard skills.sh sitemap and detail HTTP fetches#70336
zapabob wants to merge 2 commits into
NousResearch:mainfrom
zapabob:fix/skills-sh-http-ssrf

Conversation

@zapabob

@zapabob zapabob commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • skills.sh sitemap index locs were fetched with raw httpx.get(..., follow_redirects=True) without host allowlisting, so a malicious sitemap-skills loc (or redirect hop) could target private/link-local addresses.
  • Route search, sitemap, featured, and detail fetches through _guarded_http_get (SSRF + hop re-validation).
  • Only accept sitemap locs whose host is skills.sh / www.skills.sh.
  • Preserve gzip Accept-Encoding for sitemap payloads (brotli decode workaround).

Test plan

  • pytest tests/tools/test_skills_sh_ssrf.py (2 passed)

@zapabob @cursoragent
fix(security): guard skills.sh sitemap and detail HTTP fetches
9e2563b 
Route skills.sh search/sitemap/featured/detail through _guarded_http_get
and only follow sitemap locs that remain on skills.sh hosts.

Co-authored-by: Cursor <cursoragent@cursor.com>
@zapabob zapabob mentioned this pull request Jul 23, 2026
Open
1 task
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P3 Low — cosmetic, nice to have tool/skills Skills system (list, view, manage) needs-repro Bug needs reproduction steps labels Jul 23, 2026
@alt-glitch

alt-glitch commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #63920, which already routes the Skills Hub fetch family through the guarded redirect path more broadly. This narrower skills.sh-focused hardening may be useful to consolidate with that work.

@zapabob @cursoragent
fix(tests): align skills.sh hub mocks with SSRF-safe GET
0db26a2 
Co-authored-by: Cursor <cursoragent@cursor.com>
@zapabob zapabob mentioned this pull request Jul 23, 2026
Closed
1 task
@teknium1

teknium1 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Thanks for narrowing the skills.sh SSRF hardening.

Problems

  • The new coverage in tests/tools/test_skills_sh_ssrf.py:5-18 only tests the host predicate. It does not execute SkillsShSource._sitemap_catalog against a hostile <loc> or a redirect chain. Current main’s exploitable flow is the arbitrary sitemap fetch at tools/skills_hub.py:1749-1773 after the redirect-following index fetch at tools/skills_hub.py:1741-1746.

Suggested changes

  • Add mocked catalog-path regressions proving a foreign/private sitemap <loc> is never fetched, a redirect to a blocked address is not followed, and the sitemap request keeps Accept-Encoding: gzip.

The production direction fits the existing _guarded_http_get mechanism at tools/skills_hub.py:302-338. This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@Adolanium Adolanium mentioned this pull request Aug 15, 2026
Closed
1 task
@Adolanium

Adolanium commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

This is a slice of #63920, not a leftover.

#63920 already routes skills.sh search / sitemap / detail / featured through _guarded_http_get (and the other hub GETs). Open since Jul 13. This PR is the skills.sh-only cut from Jul 23, with #70330 / #70334 / #70343 doing the same for browse.sh, ClawHub, and GitHub.

"Not merged yet" is not the same as "skills.sh is unguarded." Please don't land this as the skills.sh fix over #63920.

@Adolanium Adolanium mentioned this pull request Aug 15, 2026
Closed
1 task
@alt-glitch alt-glitch removed sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 15, 2026
zapabob added a commit to zapabob/hermes-agent-windows that referenced this pull request Aug 20, 2026
@zapabob @cursoragent
fix(security): route skills.sh hub fetches through guarded GET
f0a1400 
Match the NousResearch#70336 mock pattern: SkillsSh search/sitemap/featured/detail
now call _guarded_http_get so TestSkillsShSource patches of
_ssrf_safe_http_get intercept instead of leaking live catalog data.
Keep root contents discovery on httpx.get with a dual mock where needed.

Co-authored-by: Cursor <cursoragent@cursor.com>
zapabob added a commit to zapabob/hermes-agent-windows that referenced this pull request Aug 20, 2026
@zapabob
fix(security): route LobeHub skills fetches through guarded HTTP
43a863b 
## 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`.
@zapabob

zapabob commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Closing this overlapping skills.sh slice in favor of consolidated survivor #70343.

The skills.sh search, sitemap, featured, and detail fetches now use the existing guarded transport in #70343, alongside the related GitHub, ClawHub, and LobeHub routes. The survivor is rebased onto official main d3e124601f54ad36f0115dc499e7620aa951d625 at head c46337e817a20826f9c459eccc019380c0d4ff63; its focused suite has 48 passing tests.

@zapabob zapabob closed this Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

needs-repro Bug needs reproduction steps P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform tool/skills Skills system (list, view, manage) type/security Security vulnerability or hardening

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zapabob @alt-glitch @teknium1 @Adolanium