feat(api): refuse an unknown key in a request body instead of dropping it (BACKLOG #1109)

[wshallwshall]

What this is, and why the item stays open

The API limb of BACKLOG #1109 (ASVS 2.2.1). The item's verdict is research and its closing act
is scorecard-rescore: the ASVS Tracker re-scores the cell in the vault, then mails the Lander the
item numbers for the banner flip. Two acts, two seats, neither of them a builder's. Shipped work
plus an open item is the complete outcome here. This PR does not claim to move the cell, and the
BACKLOG note says so in those words.

The scorecard is not in this repository and nothing here touches it.

The defect

messagefoundry/api/models.py and messagefoundry/api/auth_models.py carried zero
model_config across 125 model classes, so every API request body ran Pydantic's default
extra=ignore. An unknown or misspelled key was accepted, dropped, and the route answered success.

Measured at HEAD a2eef0f3 before anything changed, with a positive control in the same run: five
extra="forbid" in messagefoundry/config/models.py. Both counts on record had drifted. The
item says 84 models; the 2026-08-20 research says 116 across two files; the files have since grown
to 93 + 32. The substance held on both readings, the numbers did not, so the note records the new
ones.

The sharpest instance is not "a key is dropped" in the abstract. PUT /users/{id}/channel-scope
takes ChannelScope, whose channels is optional and whose None means all channels. A body of
{"chanels": ["IB_ACME_ADT"]} asked for one connection, granted every one, and answered 200. An RBAC
over-grant by typo.

The change

A new messagefoundry/api/request_model.py holds one base class, RequestModel, carrying
extra="forbid". The 32 models FastAPI parses out of a request body now subclass it. The 93
response-only models stay tolerant.

The split is directional and that is the whole design. messagefoundry/apiclient/client.py
validates engine responses into these same classes and the web console ships as a separately
versioned wheel, so a blanket extra="forbid" would make an older client raise on a newer engine
that merely grew a field. That is the "not a one-line fix" correction the 2026-08-20 research left on
the item, and it is carried in a comment beside the base class exactly as the research asked, so it
is not later tidied away.

Which models are request models is read from FastAPI's own route.body_field at runtime, not
re-derived from source with a regex. A regex would be a second, silently different definition of
"request model" -- the failure CLAUDE.md section 11 records for the backlog glyph parser.

tests/test_api_request_models_forbid_extra.py (7 tests) pins the split in both directions off
that same runtime read. The second direction is the control, not decoration: a blanket flip across
all 125 models passes the "every request model is strict" assertion and breaks every client, so the
"response-only models stay tolerant" assertion is the only thing that tells the two apart. Every
refusal assertion is paired with the same body minus the unknown key going through.

Callers: none needed changing, and that is a measurement

• Web console builds these models by explicit keyword against declared fields
  (routes/search.py:191,240, routes/core.py:682, routes/monitoring_writes.py:131,164,
  routes/uploaded_logs.py:312,387, routes/connection_writes.py:119, routes/config.py:58) and
  never splats a form dict into one.
• apiclient/ sends field-exact JSON bodies. reauth() sends purpose as well as password
  and both are declared (auth_models.py:121-130), which is the one that looked like a break.
• Harness reaches the engine only through apiclient and imports response models alone
  (ConnectionRow, DeadLetterRow, MessageDetail, MessageSummary).
• IDE extension sends three bodies -- /auth/login, /ai/chat, /config/reload -- and each
  names declared fields only.
• Approvals replay reads its stored params as a bare Mapping (api/app.py:530-560) and never
  re-validates them into a request model.

Checks

Run in this worktree on a python -m venv 3.14.6 environment, pip install -e ".[dev,harness,fhir,dicom,x12,xml,webauthn]" -e packaging/messagefoundry-webconsole --constraint constraints.lock:

Check	Command	Result
lint	ruff check .	All checks passed!
format	ruff format --check .	1243 files already formatted
types	mypy messagefoundry messagefoundry_webconsole	Success: no issues found in 301 source files
new test	pytest tests/test_api_request_models_forbid_extra.py -q	7 passed in 5.92s
engine + console	pytest tests/ packaging/messagefoundry-webconsole/tests/ -q -n 4 -m "not tooling"	12097 passed, 836 skipped, 43 warnings in 1553.19s (0:25:53)
backlog gates	pytest over the tooling tests that read docs/BACKLOG.md	229 passed in 164.41s

Not run here, and they must be read on the runner: every hosted-only leg -- windows-service-smoke
(NSSM), the SQL Server and PostgreSQL store legs, the tooling job's full partition, and the
Windows/Ubuntu matrix legs. A Builder's process exits before any of them reports.

One known-spurious result to read carefully rather than blame on this diff: on a first pass with
-x and a second pytest session competing for the same CPU,
tests/test_connscale_cpu_probe.py::test_sampler_measures_a_descendant_that_actually_burns_cpu failed
with ProbeDegraded.WALK_TIMEOUT. Re-run alone on an idle machine it is 22 passed, 2 skipped in 57.80s. It samples a two-second CPU burn, so it is load-sensitive by construction.

Left open on purpose

1. The method question is not decided, and it was not mine to decide. Which clause binds a
   level-1 requirement inside an L3 assessment -- "all input", or the L1 focus on "input used to make
   specific business or security decisions" -- is a method ruling, and per the item it decides
   whether the cell is reachable at all. Stated as the open blocker it is.
2. The HL7 limb was not touched. validation.strict still ships False at
   config/models.py:667, parsing/peek.py is unchanged, and the 2026-08-20 finding that this
   cell's ceiling is partial stands. The item calls that the canonical genuine product cost and
   it is right: a deploying site would start NAKing the clinical messages the engine exists to
   tolerate.
3. The five dual-use shapes are a judgment call I made and am flagging. AdGroupMap,
   AdGroupMapEntry, AdGroupScopeEntry, AdGroupScopeMap and ChannelScope are parsed from a
   request body and returned in a response, so one model_config cannot be right for both
   directions. I made them strict, because ChannelScope is the concrete RBAC over-grant above and
   excusing it would leave the worst instance unfixed. The cost is that adding a field to one of
   those five is a client-visible wire change needing the client bump in the same release; that is
   written in the base class and pinned by test_dual_use_models_are_exactly_the_recorded_set, so
   growing the set is a decision rather than a drift. The alternative I did not take was splitting
   each into a request-only sibling: five more classes and a drift risk between the pairs, for a
   forward-compatibility guarantee on five tiny RBAC shapes. Overturn it if you disagree -- the whole
   decision is one base-class reference per model.
4. The "logical limits" half of the pinned verb is only partly addressed. extra="forbid" is the
   "compare to an expected structure" half. Field-level bounds are already dense in these models
   (max_length, ge/le, Literal) but were not audited field by field in this change, and the
   research lists that under separate unallocated subjects.

Claim taken with scripts/coord/claim.ps1 -Take 1109.

🤖 Generated with Claude Code

[wshallwshall]

Disclosure: the reviewed label on this pull request was applied by the Manager seat that commissioned the diff.

It is not an independent read of the scope. COMMON.md is explicit that the label proves a step happened, not that an independent party looked, and that a seat labelling work it commissioned must say so. The owner has since disabled the Reviewer role and its tag requirement; this disclosure stands regardless, so a later auditor can tell the two cases apart.

What I actually checked, so the record is specific rather than a blanket claim:

• Verified the headline finding independently against origin/main. ChannelScope.channels is list[str] | None defaulting to None, its own docstring states None = all channels, and it inherits plain BaseModel. So a typo'd key on PUT /users/{id}/channel-scope would be dropped and the scope would widen to every connection, answering 200. Conditional per section 0: zero deployments, so this is what a first deployment would meet.
• Confirmed the ledger row is the last commit and alone.

I did NOT re-run the 12,097-test suite, re-derive the 125/32/93 model split, or verify the caller survey myself. Those are the Builder's measurements.

This is a privilege over-grant, not a validation nicety. Whoever lands this should weigh it on that basis rather than as an ASVS documentation item.