Skip to content

fix(security): route LobeHub skills fetches through guarded HTTP - #70356

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

fix(security): route LobeHub skills fetches through guarded HTTP#70356
zapabob wants to merge 2 commits into
NousResearch:mainfrom
zapabob:fix/lobehub-skills-http-ssrf

Conversation

@zapabob

@zapabob zapabob commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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 (#70334 ClawHub, #70336 skills.sh, #70343 GitHub hub) — LobeHub still used raw httpx.get.

Test plan

  • pytest tests/tools/test_lobehub_skills_http_ssrf.py -q

## 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`.
@alt-glitch alt-glitch added type/security Security vulnerability or hardening tool/skills Skills system (list, view, manage) P3 Low — cosmetic, nice to have needs-repro Bug needs reproduction steps sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 23, 2026

@teknium1 teknium1 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.

Thanks for covering the remaining LobeHub fetch paths. The current main implementation still uses raw httpx.get at tools/skills_hub.py:2829 and tools/skills_hub.py:2843, while the shared guarded path at tools/skills_hub.py:302 provides SSRF-safe connection handling and redirect-hop validation.

Problems

  • tests/tools/test_lobehub_skills_http_ssrf.py:27 only proves unsafe IDs return before the guard. It does not prove a valid per-agent fetch uses _guarded_http_get; that test would also pass if the valid path remained a raw httpx.get.

Suggested changes

  • Add a positive-path per-agent test with a valid ID, mocked guarded response, and assertions for the guarded URL and timeout=15.

This is an automated hermes-sweeper review.



def test_lobehub_fetch_agent_rejects_unsafe_id():
src = LobeHubSource()

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.

This covers rejection before any request, but not the successful agent-fetch route. Please add a valid-ID case that asserts _guarded_http_get receives https://chat-agents.lobehub.com/<id>.json with timeout=15; otherwise this test would still pass if that path used raw httpx.get.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 30, 2026
@egilewski

Copy link
Copy Markdown
Contributor

looks mergeable

The patch moves both LobeHub fetch paths onto the repository's SSRF- and policy-checked HTTP helper and rejects literal path, authority, and traversal delimiters before constructing agent URLs.

Review setup: I reviewed a run-owned local rebase or patch replay against current GitHub main because the submitted branch is stale or conflicted; this does not mean the submitted branch itself merges cleanly.

Security evidence:

  • trust boundary: CLI, dashboard, TUI, and cached or remote LobeHub data can supply the agent identifier. The identifier reaches LobeHubSource._fetch_agent, while the index and agent JSON are untrusted community content that later enters the existing bundle quarantine and scan path.
  • source/sink/invariant: _fetch_index uses the fixed LobeHub index origin and _fetch_agent only interpolates an identifier after literal delimiter/traversal checks. Both calls use _guarded_http_get, which performs URL and website-policy checks, connect-time SSRF validation, and per-hop redirect validation before JSON is consumed.
  • current-main reproduction: Current main performs both LobeHub requests without Skills Hub URL preflight or connect-time SSRF transport. The patch replaces both raw calls with the guarded helper.
  • PR-head or patch-replay validation: The patch replay preserves both guarded fetch paths; focused Skills Hub regression tests pass, and targeted checks cover safe and rejected identifiers plus a redirect to 127.0.0.1.
  • positive/negative cases: safe-agent_1 produces the expected fixed-origin URL. ../etc/passwd, http://evil.example/x, a/b, and ../../x are rejected before the HTTP helper is called. A redirect to http://127.0.0.1/secret is rejected before a second request.
  • residual bypass search: LobeHub fetch and router paths have no remaining alternate raw HTTP sink in the reviewed change. Percent-encoded separators remain path data on the fixed chat-agents origin, and non-printable input is rejected by the HTTP URL parser; no cross-origin or private-network bypass was demonstrated.
  • reviewer validation: Source review, focused Skills Hub tests, targeted probes, and whitespace checks all passed; no in-scope source-backed security finding requiring a change remains.

Not checked:

  • Full repository test suite
  • Live LobeHub network integration
  • CodeRabbit review
  • Ruff lint

Signed: GPT-5.6-luna-max in Codex

@zapabob

zapabob commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

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

Its production change and positive guarded per-agent regression are preserved in #70343, now rebased onto official main d3e124601f54ad36f0115dc499e7620aa951d625 at head c46337e817a20826f9c459eccc019380c0d4ff63. The consolidated focused suite has 48 passing tests and Ruff/diff checks pass.

@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

Labels

needs-repro Bug needs reproduction steps P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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.

4 participants