Skip to content

fix(relay,osv): guard json.loads against JSONDecodeError - #54225

Open
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/guard-json-loads-relay-osv
Open

AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/guard-json-loads-relay-osv

Conversation

@AlexFucuson9

Copy link
Copy Markdown
Contributor

Problem

Two locations call json.loads() on external input without catching JSONDecodeError:

gateway/relay/descriptor.py:74CapabilityDescriptor.from_json()

raw = json.loads(data)           # crashes on malformed JSON
filtered = {k: v for k, v in raw.items() if k in known}  # crashes if raw is not a dict

A malformed handshake message (e.g. JSON array, truncated JSON, or non-JSON payload) causes either JSONDecodeError or AttributeError on .items().

tools/osv_check.py:165_query_osv_batch()

with urllib.request.urlopen(req, timeout=_TIMEOUT) as resp:
    result = json.loads(resp.read())  # crashes on non-JSON response

The OSV API can return HTML error pages (rate limit, 502 gateway error), causing the entire vulnerability check to crash instead of gracefully returning no results.

Fix

  • descriptor.py: Wrap json.loads() in try/except (json.JSONDecodeError, TypeError), raise ValueError with clear message. Add isinstance(raw, dict) guard before .items().
  • osv_check.py: Wrap json.loads() in try/except (json.JSONDecodeError, UnicodeDecodeError), log warning, return empty list.

Impact

  • Severity: P1 — crash on malformed external input
  • Scope: 2 files, 12 lines added
  • Risk: Minimal — only adds error handling

gateway/relay/descriptor.py: CapabilityDescriptor.from_json() calls
json.loads() without catching JSONDecodeError, and raw.items() crashes
with AttributeError if the JSON is not a dict (e.g. a list or string).
Now raises ValueError with clear message on invalid input.

tools/osv_check.py: _query_osv_batch() calls json.loads() on HTTP
response without catching JSONDecodeError. If the OSV API returns a
non-JSON response (rate limit HTML, 502 gateway error), the entire
vulnerability check crashes. Now logs warning and returns empty list.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets labels Jun 28, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defensive json.loads guards in relay descriptor and OSV check (12 additions, 2 files). Proper error handling for malformed JSON and type validation. Clean and minimal.

Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing two real external-input crash paths. The current-main failures reproduce at gateway/relay/descriptor.py:74-76 and tools/osv_check.py:165-167.

Problems

  • The OSV patch still permits a crash for valid but non-object JSON: after decoding [], the unchanged result.get("vulns", []) raises AttributeError at tools/osv_check.py:168. Please validate the decoded response is a dict before using it.
  • Please add regressions for malformed/non-object descriptor JSON and malformed/non-object OSV response bodies. Current tests in tests/gateway/relay/test_descriptor.py and tests/tools/test_osv_check.py cover valid payloads but not these failure paths.

Suggested changes

  • Fail open with a warning when the OSV response is not a JSON object (and validate the vulns shape before filtering).
  • Add targeted tests for HTML/malformed bytes and [], then run the two affected test files through scripts/run_tests.sh.

Automated hermes-sweeper review.

Comment thread tools/osv_check.py
raw = resp.read()
try:
result = json.loads(raw)
except (json.JSONDecodeError, UnicodeDecodeError):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A valid JSON but non-object response such as [] reaches this line and raises AttributeError, so the OSV check is still not fail-open for all malformed/unexpected response shapes. Please return [] when result is not a dict (and consider validating vulns is a list).

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026

This branch has not been deployed

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

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants