Skip to content

fix(skills-hub): cover remaining SSRF fetch paths (salvage #22804) - #22843

Merged
teknium1 merged 1 commit into
mainfrom
salvage/pr-22804
May 10, 2026
Merged

teknium1 merged 1 commit into
mainfrom
salvage/pr-22804

Conversation

@teknium1

@teknium1 teknium1 commented May 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Salvage of #22804 — tools/skills_hub.py now does manual redirect re-validation across all attacker-controlled fetch paths (UrlSource, WellKnownSkillSource, ClawHubSource rawUrl fallback), closing the redirect-bypass gap that #10029's preflight is_safe_url doesn't cover.

Threat model

Three fetch paths take URLs the attacker controls:

  • UrlSource._fetch_text() — user runs hermes skills install <attacker-URL>
  • WellKnownSkillSource._parse_index() / _fetch_text() — attacker-controlled .well-known/skills/ server lists arbitrary URLs
  • ClawHubSource._fetch_text() — rawUrl / downloadUrl from version metadata fallback path

#10029 (still open) preflights is_safe_url(url) but keeps follow_redirects=True. A public-looking URL that 302s to 127.0.0.1:6379 or 169.254.169.254/... (cloud metadata) bypasses the preflight entirely. That's the gap this PR closes for the skills hub.

Changes (contributor commit)

  • tools/skills_hub.py: new _guarded_http_get helper. Disables auto-redirects, re-runs is_safe_url + check_website_access per hop (5-hop cap), uses urljoin for relative Location headers. Wires it into the four attacker-controlled fetch sites listed above.
  • 3 regression tests covering direct private URL, redirect-to-private bypass, and ClawHub rawUrl private path.

Other fetch sites in the file (clawhub.ai download, skills.sh, api.github.com, lobehub, hermes docs) target hard-coded trusted hosts and aren't SSRF surface.

Validation

Closes #13583 (covers the remaining SSRF fetch paths). Supersedes #10029.

@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

🔎 Lint report: salvage/pr-22804 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 7953 on HEAD, 7953 on base (➖ 0)

🆕 New issues (3):

Rule Count
invalid-argument-type 3
First entries
run_agent.py:12944: [invalid-argument-type] invalid-argument-type: Argument to function `len` is incorrect: Expected `Sized`, found `(str & ~AlwaysFalsy) | (dict[Unknown | str, Unknown | str | dict[str, str]] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 3 union elements`
run_agent.py:6872: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `str | dict[Unknown | str, Unknown | str | dict[str, str]] | Any | ... omitted 3 union elements`
run_agent.py:12941: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `str | dict[Unknown | str, Unknown | str | dict[str, str]] | Any | ... omitted 3 union elements`

✅ Fixed issues (3):

Rule Count
invalid-argument-type 3
First entries
run_agent.py:12944: [invalid-argument-type] invalid-argument-type: Argument to function `len` is incorrect: Expected `Sized`, found `(str & ~AlwaysFalsy) | (dict[Unknown, Unknown] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 3 union elements`
run_agent.py:12941: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`
run_agent.py:6872: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`

Unchanged: 4198 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@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
teknium1 merged commit 0c5c4d1 into main May 10, 2026
16 of 18 checks passed
@teknium1
teknium1 deleted the salvage/pr-22804 branch May 10, 2026 00:52
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