Vote for your favorite SkillMD. The submission with the most likes wins the $1,000 Audience Choice Award for the NandaHack x HCLTech hackathon. Voting is open through September 25.Vote now →
mergedhumanPR #277Other

fix(coordinator): refuse JSON values the run cannot export

Follow-up to #264–#272, from the same correctness audit of main at df0b5f1.

Author

JamesCarnley avatar

@JamesCarnley

github profile →
Status
Merged
Merged on
Sep 14
Branch
fix/coordinator-reject-non-finite-json

Description

The pitch.

Follow-up to #264–#272, from the same correctness audit of `main` at `df0b5f1`.
It merges cleanly with the other open PRs that touch the coordinator, runner or
README.

### Problem

A Track participant could send a JSON body with any of these values:

- NaN, Infinity or -Infinity;
- a number that overflows to infinity, such as `1e999`;
- a string or key containing an unpaired surrogate.

Python's `json` module accepts all of these, so the coordinator accepted and
stored the body. The `/events` export then failed with HTTP 500, because
Starlette refuses to render such values. The runner crashed with a traceback,
and no bundle was written. A malformed participant therefore looked like a Town
crash, and all evidence of the run was lost.

There was also a latent second problem. Even if the export had succeeded,
pydantic would have recorded NaN as `null`, so replay would differ and `verify`
would reject the bundle.

### Change

- **One strict parser.** Every coordinator route parses its JSON body through
  it. Number literals are checked as they are parsed. Strings and keys are
  checked for UTF-8 encodability after parsing.
- **Refusal.** Such a body is refused with HTTP 422:
  `{"error": "invalid_json_value", "problem": "non_finite_number" | "unpaired_surrogate", "reason": …}`.
  A refused number's literal is named; a refused string is never echoed.
- **Evidence.** A joined participant's refused send or ack is recorded as an
  intent plus an `invalid_json_value_rejected` event. That is the same pattern
  as `grant_permission_denied`, and the body itself is never stored. Nothing is
  written for admin routes, joins, unknown sessions or finished runs.
- **Contract text.** The README's "raw HTTP contract" now says what is refused:
  - NaN and Infinity, which JSON (RFC 8259) does not define;
  - numbers too large for a double, such as `1e999`, which are valid JSON but
    cannot be stored as finite numbers;
  - unpaired surrogates.
- **Report line.** The report shows a "Refused as invalid JSON values" count, so
  a subject whose send or ack was refused can see why its run is inconclusive.
- **Bounded literal.** A refused number's literal is shortened to about 32
  characters wherever it is recorded.

**Worth knowing.**

- The body is parsed before the handler's permission check. So a grant session
  without `send` permission that posts NaN is recorded as an invalid JSON value,
  not as a permission denial. Both attribute the refusal to the subject.
- Duplicate keys are treated differently for the two kinds of value, and both
  outcomes are safe:
  - `{"a": NaN, "a": 1}` is refused, because numbers are checked while parsing.
  - `{"a": "\ud800", "a": "x"}` is accepted, because strings are checked on the
    parsed value.
- Only refused sends and acks are recorded as events. Other request-schema
  failures are still not recorded, as on `main`.

### Compatibility

- Valid JSON parses and is stored exactly as before.
- The evaluator and its version, profiles and bundle format are unchanged.
- Town's own participants are unaffected. The scripted ones send only integers
  and strings, and the model-driven ones go through httpx, which refuses to
  encode these values in the first place.

### Verification

- **Coordinator tests.** New `tests/test_invalid_json_values.py` covers
  non-finite numbers, overflowing numbers and surrogates in note, body, key,
  nested and raw-byte positions. It fails on `main` and passes with this
  change.
- **Real run.** `nandatown test-agent --role seller --cmd …` with a fixture
  seller that sends a surrogate key and a NaN value:
  - On `main`: a traceback, and no bundle.
  - With this change: an honest INCOMPLETE bundle that `nandatown verify`
    accepts, with both refusals recorded as events.
- **Regression checks.** Full suite: 866 passed, twice. All eight Track profiles
  with stock agents and the `examples/byoa_seller.py` run still pass and verify.
- **Valid JSON is unchanged.** Across 66 probe cases, sta

…

Try it

Open PR on GitHubView diff

Checkout locally

git fetch origin pull/277/head:pr-277
git checkout pr-277