Skip to content

fix(api): answer 400 for a JSON body that is not an object on the logs, settings and providers routes - #16671

Merged
diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.52from
pacocartones:fix/mgmt-null-json-body-400
Oct 15, 2026
Merged

diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.52from
pacocartones:fix/mgmt-null-json-body-400

Conversation

@pacocartones

Copy link
Copy Markdown
Contributor

Summary

  • A null JSON body made four management handlers throw Cannot read properties of null (reading ...) and the request ended as a 500: POST /api/logs/detail (body.enabled), PATCH and PUT /api/settings (body.expectedRevision, read by parseExpectedRevision; PUT calls PATCH) and DELETE /api/providers (body.ids). POST /api/logs/detail also had no try/catch around req.json(), so malformed JSON was an unhandled rejection there, and it answered a string, a number or true body with a 200 that switched detailed logs off (settings and providers already answered 400 for those three).
  • Each handler now answers 400 Request body must be a JSON object for a body that is not an object (and Invalid JSON body for malformed JSON on POST /api/logs/detail, the message the other routes already use). PATCH and PUT /api/settings use the code INVALID_BODY and emit the same failure audit as their existing INVALID_JSON path.
  • Impact: a malformed request now gets an error response to that same request instead of a thrown TypeError, and a non-object body can no longer change the logs setting. This is input validation, not a crash fix: other clients and the process are unaffected either way.

Related Issues

Target Branch

  • Base is the active release/v3.8.x branch (the highest open release/v*), not main

Proof Of Fix (required)

  • TDD (preferred) — a test that fails before the fix and passes after it.
    • Test file(s): tests/unit/mgmt-null-json-body-400.test.ts
    • Failing output before the fix: handler threw (=> 500): Cannot read properties of null (reading 'enabled') (and 'expectedRevision' for settings PATCH and PUT, 'ids' for providers DELETE), and expected 400, got 200 for a string, a number or true body on /api/logs/detail; 8 of 18 tests fail on the base. The other 10 (string, number and true on settings and providers, and an object body on logs) pass on both and pin the behaviour that was already right.

Tests Added Or Updated

  • tests/unit/mgmt-null-json-body-400.test.ts: null, a string, a number and true to each of the four handlers and malformed JSON to /api/logs/detail assert a 400; an object body to /api/logs/detail still answers 200. It reuses the temp data directory harness of api-malformed-json-400.test.ts.

Coverage Notes

  • The existing api-malformed-json-400.test.ts keeps passing (23 passed together with the new file).

Scope

  • Only the files the failing test proves need changing are touched (the three route files, the test and the changelog fragment)
  • No test was weakened or deleted to make the suite pass

Validation

  • Change type: other (management API validation)
  • Focused tests: the new test and api-malformed-json-400 pass; check:changelog-integrity passes; prettier --check is clean on the changed files and eslint reports no new problem.

Reviewer Notes

  • I sent the same bodies to the other JSON routes under src/app/api that are safe to call in a test (embeddings, images, audio, moderations, batches, responses input tokens, quota, resilience, the other settings routes) and found no other handler that throws on them; they answer 400 through their zod schemas.
  • POST /v1/files answers 500 Upload failed for a non-multipart body; I left it out because it is a wrong content type, not a malformed-JSON case, and it deserves its own decision.

pacocartones and others added 2 commits October 15, 2026 09:51
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants