Skip to content

fix(osv-check): guard json.loads against non-JSON API responses - #29696

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

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

Conversation

@annguyenNous

Copy link
Copy Markdown
Contributor

Problem

_query_osv() in tools/osv_check.py calls json.loads(resp.read()) without a try/except guard. When the OSV API returns a non-JSON response (HTTP 502 HTML page, rate-limit response, or gateway error), this raises json.JSONDecodeError.

While the caller already catches Exception (fail-open pattern), handling this inside _query_osv() provides:

  1. A clearer, more specific error message in debug logs
  2. Returns the expected list type instead of raising
  3. Follows the principle of handling errors close to the source

Fix

Wrap json.loads(resp.read()) in try/except json.JSONDecodeError, log at debug level, and return an empty list.

Diff: +5 lines, -1 line (1 file)

Testing

  • Verified syntax: ast.parse() passes
  • The fix is purely defensive — no behavior change for valid JSON responses
  • Fail-open semantics preserved: empty list = no malware found = allow

The OSV API may return non-JSON responses (502 HTML, rate-limit pages)
which cause json.JSONDecodeError in _query_osv(). While the caller
already catches Exception (fail-open), handling this inside the
function provides a clearer error message and returns the expected
empty list type.

Fix: wrap json.loads(resp.read()) in try/except json.JSONDecodeError,
log at debug level, and return an empty list.
@alt-glitch alt-glitch added type/bug Something isn't working type/security Security vulnerability or hardening comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have and removed type/security Security vulnerability or hardening labels May 21, 2026
@alt-glitch

Copy link
Copy Markdown

Subset of #29019 which already guards this exact json.loads() call in tools/osv_check.py along with several other unguarded sites (weixin, CDP, api_server) and adds atomic writes.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants