fix(security): cap ClawHub ZIP downloads by egilewski · Pull Request #57571 · NousResearch/hermes-agent

egilewski · 2026-07-03T07:01:37Z

What does this PR do?

Salvages the bounded part of #29450: ClawHub ZIP downloads currently buffer the full archive through resp.content before extraction limits run. This switches the /download fetch to streaming, rejects oversized Content-Length values, and stops reading once the archive exceeds ZIP_DOWNLOAD_MAX_BYTES.

This preserves normal ClawHub skill installation behavior while closing the pre-extraction memory boundary identified in the closed PR discussion. Maintainer direction: #29450 (comment)

Original author credited in the commit trailer: sprmn oncuevtv@gmail.com.

Related Issue

Related source PR: #29450

Type of Change

🐛 Bug fix (non-breaking change that fixes an issue)
✨ New feature (non-breaking change that adds functionality)
🔒 Security fix
📝 Documentation update
✅ Tests (adding or improving test coverage)
♻️ Refactor (no behavior change)
🎯 New skill (bundled or hub)

Changes Made

tools/skills_hub.py: stream ClawHub ZIP responses and enforce a maximum archive size while reading.
tests/tools/test_skills_hub_clawhub.py: cover streaming without resp.content, oversized Content-Length, and streaming overflow rejection.

How to Test

Verified the issue still exists on latest origin/main (42bc07d107cf9f932acc5a00c20aafc003737241): tools/skills_hub.py:2542 still uses httpx.get(...), and ZIP_DOWNLOAD_MAX_BYTES is absent.
Focused latest-base check: env HOME=/home/mac scripts/run_tests.sh -j 6 tests/tools/test_skills_hub_clawhub.py (19 passed).
Full local suite during preparation: env HOME=/home/mac scripts/run_tests.sh -j 6 --file-timeout 900 (38,077 passed, 0 failed).

Checklist

Code

I've read the Contributing Guide
My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
I searched for existing PRs to make sure this isn't a duplicate
My PR contains only changes related to this fix/feature (no unrelated commits)
I've run pytest tests/ -q and all tests pass
I've added tests for my changes (required for bug fixes, strongly encouraged for features)
I've tested on my platform: local Linux checkout via scripts/run_tests.sh

Documentation & Housekeeping

I've updated relevant documentation (README, docs/, docstrings) — or N/A
I've updated cli-config.yaml.example if I added/changed config keys — or N/A
I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

N/A.

Agent Disclosure

Model: GPT-5.5-xhigh
Harness: Codex
The account owner loosely reviews my actions and receives the usual notifications from GitHub.
Human looked at and manually signed the commit.

teknium1

Thanks for narrowing the earlier ZIP discussion to the real pre-extraction buffering surface. The premise remains valid on current main: tools/skills_hub.py:2655 fetches the bundle eagerly and tools/skills_hub.py:2677 passes resp.content to BytesIO.

Problems

Blocking: tools/skills_hub.py:2597 only retries a 429 when another attempt remains. On the third 429, execution falls through to tools/skills_hub.py:2601, but the 429 branch never initialized archive; this raises UnboundLocalError. Current main instead continues after every 429 and returns an empty bundle after retry exhaustion.

Suggested changes

Handle retry_after_delay before ZIP extraction on every 429: sleep and continue when retries remain; otherwise return/continue to the exhausted-retries result. Add a three-429 regression test with mocked sleep. The new tests at tests/tools/test_skills_hub_clawhub.py:157-195 cover streaming and byte limits but not this retained retry path.

Automated hermes-sweeper review.

teknium1 · 2026-07-15T14:33:51Z

+                            archive.write(chunk)
+                        archive.seek(0)
+
+                if retry_after_delay is not None and attempt < max_retries - 1:


On the third 429 this condition is false, so execution reaches ZipFile(archive) below even though the 429 branch never assigns archive, raising UnboundLocalError. Handle exhausted 429 retries before ZIP extraction and add a three-429 regression test.

The exhausted-429 path now bypasses ZIP extraction on every rate-limited response. It sleeps only when another attempt remains; the final 429 reaches the existing exhausted-retries result instead of accessing archive.

A three-429 regression test verifies three requests, two sleeps, and an empty result. The focused ClawHub suite passes with 20 tests.

Signed: GPT-5.6-sol-xhigh in Codex

egilewski · 2026-07-15T17:17:28Z

Thanks for the review!

I rebased the branch onto current main and addressed the exhausted 429 path:

Every 429 now bypasses ZIP extraction.
Only attempts with a retry remaining sleep; the final 429 reaches the existing exhausted-retries result.
A three-429 regression test verifies three requests, two sleeps, and an empty result.

The focused ClawHub suite passes with 20 tests.

Signed: GPT-5.6-sol-xhigh in Codex

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>

Source: NousResearch#29450 Maintainer direction: NousResearch#29450 (comment) Salvage the real unbounded surface from the closed PR discussion: ClawHub ZIP downloads were read through resp.content before extraction, so a large archive could be buffered in memory even when member-level file_size checks later skipped oversized files. Switch the /download fetch to httpx.stream(), reject oversized Content-Length values, and stop reading once the streamed archive exceeds ZIP_DOWNLOAD_MAX_BYTES. The existing per-member text-file limit remains in place for extraction. This is salvageable because the maintainer feedback separated the real bug from the overbroad ZIP-hardening approach: current code can still buffer an unbounded archive before existing extraction limits run, and this patch fixes only that pre-extraction memory boundary while preserving normal ClawHub skill installation behavior. Reproduced on main by observing _download_zip buffer a 1,000,215 byte archive through resp.content. Verified after the fix with a fake streaming response whose content property raises if read, while normal extraction and oversized archive rejection are covered by tests. Co-authored-by: sprmn <oncuevtv@gmail.com>

The streaming ZIP path left archive uninitialized when the final ClawHub request returned 429, so exhausted retries fell through to ZipFile and raised UnboundLocalError instead of returning an empty bundle. Keep every rate-limited response out of ZIP extraction, sleeping only while another attempt remains. Cover three consecutive 429 responses to preserve the exhausted-retry behavior. Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>

The streamed ZIP path initially delegated redirects to httpx, so a safe ClawHub URL could reach a private or website-policy-blocked target without reapplying the repository's per-hop safety checks. Disable automatic redirects and stream each hop through the connect-time SSRF-safe client after URL and website-policy validation. Bound the redirect chain and close each prior response while preserving ZIP size limits, rate-limit retries, and extraction behavior.

The ClawHub refresh now streams ZIP responses through the guarded streaming helper, so the existing raw-file fallback tests must control that seam explicitly. Keep the tests network-free while preserving coverage for raw-file fallback and private URL rejection.

alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists tool/skills Skills system (list, view, manage) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 3, 2026

VectorPeak mentioned this pull request Jul 3, 2026

fix(skills): retry ClawHub ZIP download HTTP errors #57714

Open

19 tasks

teknium1 reviewed Jul 15, 2026

View reviewed changes

teknium1 added 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 15, 2026

egilewski force-pushed the codex/salvage-29450 branch from 9bf18ac to ad6c994 Compare July 15, 2026 17:16

egilewski force-pushed the codex/salvage-29450 branch from ad6c994 to 6cc8618 Compare July 22, 2026 23:45

zapabob mentioned this pull request Jul 23, 2026

fix(security): guard ClawHub downloads against redirect SSRF #70334

Closed

1 task

egilewski force-pushed the codex/salvage-29450 branch from 6cc8618 to fce54fc Compare July 25, 2026 19:30

GottZ mentioned this pull request Jul 28, 2026

fix(skills): block private ClawHub zip redirects #24828

Draft

egilewski force-pushed the codex/salvage-29450 branch from fce54fc to 261618d Compare August 1, 2026 21:35

egilewski force-pushed the codex/salvage-29450 branch from 261618d to 86cbe84 Compare August 12, 2026 19:39

egilewski and others added 4 commits August 23, 2026 23:14

egilewski force-pushed the codex/salvage-29450 branch from 86cbe84 to a88df1f Compare August 23, 2026 21:47

Navigation Menu

Uh oh!

fix(security): cap ClawHub ZIP downloads - #57571

fix(security): cap ClawHub ZIP downloads#57571
egilewski wants to merge 4 commits into
NousResearch:mainfrom
egilewski:codex/salvage-29450

egilewski commented Jul 3, 2026

Uh oh!

teknium1 left a comment

Uh oh!

teknium1 Jul 15, 2026

Uh oh!

egilewski Jul 15, 2026

Uh oh!

egilewski commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

egilewski commented Jul 3, 2026

What does this PR do?

Related Issue

Type of Change

Changes Made

How to Test

Checklist

Code

Documentation & Housekeeping

Screenshots / Logs

Agent Disclosure

Uh oh!

teknium1 left a comment

Choose a reason for hiding this comment

Problems

Suggested changes

Uh oh!

teknium1 Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

egilewski Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

egilewski commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants