Skip to content

fix(skills-hub): cover remaining SSRF fetch paths after #10029 - #22804

Closed
heathley wants to merge 1 commit into
NousResearch:mainfrom
heathley:security/skills-hub-ssrf-followup
Closed

heathley wants to merge 1 commit into
NousResearch:mainfrom
heathley:security/skills-hub-ssrf-followup

Conversation

@heathley

@heathley heathley commented May 9, 2026 •

Copy link
Copy Markdown

What does this PR do?

This PR covers the remaining Skills Hub SSRF fetch paths that are not addressed in #10029's current scope.

#10029 hardens the well-known and Skills.sh fetch paths, but the same SSRF surface is still reachable through:

  • UrlSource._fetch_text() — direct hermes skills install https://.../SKILL.md
  • ClawHubSource._fetch_text() — ClawHub version metadata rawUrl / downloadUrl / url file fetches
  • redirect-based fetches where a public URL can pass preflight validation and then redirect to a private/internal address

This PR keeps scope narrow and applies the same SSRF protection model to those remaining text-fetch paths.

Why this matters

Without this change, a malicious or compromised Skills Hub source can still trigger outbound requests to private/internal targets through paths not covered by #10029.

Examples:

  • hermes skills install http://127.0.0.1:<port>/x/SKILL.md
  • ClawHub metadata returning rawUrl: http://169.254.169.254/...
  • a safe public URL redirecting to a loopback or metadata endpoint after preflight checks

Changes made

  • Added a shared guarded fetch helper in tools/skills_hub.py
  • Applied SSRF checks to:
    • WellKnownSkillSource._parse_index()
    • WellKnownSkillSource._fetch_text()
    • UrlSource._fetch_text()
    • ClawHubSource._fetch_text()
  • Disabled automatic redirect following for these paths and re-validated each redirect target manually before continuing
  • Added regression coverage for:
    • blocking direct private/loopback URLs in UrlSource
    • blocking redirect-to-private SSRF in UrlSource
    • blocking private rawUrl targets in ClawHub fallback metadata fetches

Scope

This PR does not introduce a new security model. It keeps the scope narrow and only extends the existing Skills Hub SSRF hardening to the remaining uncovered text-fetch surfaces.

Tests

pytest tests/tools/test_skills_hub.py -q
pytest tests/tools/test_skills_hub_clawhub.py -q

Both pass locally.

Related

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround tool/skills Skills system (list, view, manage) labels May 9, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Merged via salvage PR #22843. salvage cherry-picked your commit; authorship preserved. PR #10029 closed in favor of this since this is a strict superset (covers redirect-bypass too). Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 High — major feature broken, no workaround tool/skills Skills system (list, view, manage) type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants