FormRequest: body-only input on write methods, with opt-in query merge

docs/adr/053-validated-reads-query-and-body.md

@@ -4,6 +4,8 @@ Date: 2026-10-31
 
 Extends the FastAPI integration of [ADR 048](048-async-first-validation.md).
 
+Status: amended by [ADR 056](056-validated-write-methods-read-the-body-only.md). Write methods with a body now read the body only, unless the model sets `merge_query = True`.
+
 ## Problem
 
 `Validated[Model]` declared its input as a required `Body()`, so a `FormRequest` could only validate a JSON body. A `GET` endpoint with filters (`/lexemes?window=today&page=2`) could not use the same class: FastAPI answered `422 body: Field required` before the dependency ran, even when every query parameter was valid. Applications fell back to binding the model with `Annotated[Model, Query()]`, which only runs pydantic, so `authorize`, `prepare_for_validation`, the fluent rules and `after` were silently skipped.
@@ -34,6 +36,8 @@ Option 3.
 
 ### Query input on write methods
 
+Amended by [ADR 056](056-validated-write-methods-read-the-body-only.md): this merge now needs `merge_query = True`.
+
 Write methods with a JSON body still merge the query, as Laravel's `$request->all()` does. As a result, a declared field can be set from the URL: `PATCH /profile?role=admin` with `{"name": "n"}` validates with `role == "admin"`, and `role` is in `model_fields_set`, so `update(data.model_dump(exclude_unset=True))` writes it. This gives the client no new power. The same client can put `role` in the body, the model's declared fields remain the only assignable fields, and the cross-site path is closed because the merge needs a JSON body. URLs end up in access logs and proxies, so a sensitive field (a role or a password) must not be accepted from the query. Exclude the field from the model, or reject it in `prepare_for_validation` or `after`. Keeping the merge lets one model take its filters from the URL and its payload from the body.
 
 ### OpenAPI

docs/adr/056-validated-write-methods-read-the-body-only.md

@@ -0,0 +1,49 @@
+# 056. `Validated` write methods read the body only, with an opt-in query merge
+
+Date: 2026-10-31
+
+Amends [ADR 053](053-validated-reads-query-and-body.md).
+
+## Problem
+
+Since ADR 053, `Validated[Model]` merges the query string into the input of every request that has a JSON body, on every method. Two consequences on write methods:
+
+- **Secrets in URLs.** `POST /login?password=secret` with `{"email": "a@b.c"}` validates. The client learns that the URL is an accepted channel, and the password ends up in access logs, proxy logs and browser history.
+- **Mass assignment from the URL.** `PATCH /profile?role=admin` with `{"name": "n"}` validates with `role == "admin"` and `role` in `model_fields_set`, so `update(data.model_dump(exclude_unset=True))` writes it. ADR 053 asked developers to keep sensitive fields off such models, which is easy to forget. The PR #336 review flagged it.
+
+Write payloads belong in the body. The query string is the natural input of safe methods, where ADR 053 keeps working well.
+
+## Alternatives
+
+1. **Keep the merge, document the risk (ADR 053).** Safe only when every model is written with the risk in mind.
+2. **Drop query input on write methods with no way back.** Breaks applications that rely on Laravel's `$request->all()` parity, for example a `POST /search?page=2` with filters in the body.
+3. **Body only on write methods by default, with a class-level opt-in.** A `FormRequest` that wants the merge says so once, next to its fields. Per-request control is already possible with `prepare_for_validation`, so no extra hook is needed.
+4. **A per-request hook (`merges_query(request) -> bool`).** More API for a rare need that `prepare_for_validation` covers.
+
+## Decision
+
+Option 3.
+
+- **Safe methods** (`GET`, `HEAD`, `OPTIONS`) keep ADR 053's behaviour: the query input alone without a body, `{**query, **body}` with an object body.
+- **Body-less `DELETE`** keeps reading the query input. A `DELETE` with a body is a write and reads the body only.
+- **Write methods with a body** (`POST`, `PUT`, `PATCH`, a `DELETE` with a body, and any other method) validate the body alone. Query keys are ignored, so they never reach `model_fields_set`.
+- **Opt-in:** `merge_query: ClassVar[bool] = False` on `FormRequest`. With `merge_query = True`, write methods validate `{**query_input, **body}` (the body wins on a clash), as ADR 053 did. The name says what happens (the query is merged under the body) and follows the existing `stop_on_first_failure` class setting. `query_on_write` and `accepts_query` were rejected: the first hides that the body wins, the second suggests the query is accepted alone.
+- **Per request:** `prepare_for_validation` receives the `Validator` with the request, so a model can merge the query only when it wants: `validator.data = {**cls.query_input(validator.request.query_params), **validator.data}`.
+- **Embedded models.** With several body parameters, a model whose embed key is absent or `null` in a present body validates an empty object on write methods, so its `required` rules report under its prefix (`search.term`). With `merge_query = True`, or on a safe method, it reads the query input as before.
+- Unchanged: a body-less write method still reports `"": ["Field required"]` (per embed key when embedded) before `authorize` runs, so the CSRF protection of ADR 053 stands. A non-object body (a list, a string, a JSON `null`) still fails with `model_type` under `""`. Aliases, repeated keys for sequences, the error shapes and the OpenAPI query parameters of query methods are unchanged.
+
+### Implementation
+
+- `form_request.py` adds `SAFE_METHODS = {"GET", "HEAD", "OPTIONS"}`. `QUERY_METHODS` (safe methods plus `DELETE`) still decides which body-less requests read the query.
+- `FormRequest.request_input(params, body=ABSENT, *, method)` takes the HTTP method as a required keyword. Without a body it returns `query_input`. A non-object body passes through. An object body is merged with the query input when `merge_query` is set or the method is safe, and returned alone otherwise. The keyword is required so a direct caller cannot fall back to the merge by accident.
+- The `Validated` dependency passes `request.method` and turns a missing or `null` embed key in a present body into `{}`.
+- `document_query_input` is unchanged. Write methods never listed query parameters. With `merge_query = True` the merged fields are still documented only by the body schema.
+
+### Behaviour changes
+
+- `POST`/`PUT`/`PATCH` (and a `DELETE` with a body) no longer read the query string unless the model sets `merge_query = True`. A field sent only in the URL is now missing: a required field reports its `required` message, an optional field keeps its default.
+- On those methods, an embedded model whose key is absent from the body no longer reads the query; it reports its `required` rules.
+
+## Validation
+
+`tests/validation/test_fastapi_validation.py` covers: `POST`, `PUT` and `PATCH` with a JSON body and query parameters ignoring the query by default; the same routes with a `merge_query` model merging the query with the body winning; `model_fields_set` without query keys on a `PATCH`; a `DELETE` with a body reading the body only; `GET` still reading the query and merging it with a body; the per-request merge in `prepare_for_validation`; an embedded model with an absent key on a `POST` reporting its `required` rules by default and reading the query when it opts in. The body-less CSRF tests, the non-object body tests and the OpenAPI tests of ADR 053 still pass. The full framework suite passes with coverage above the threshold, and `ruff` and `basedpyright` are clean.

docs/adr/AGENTS.md

@@ -46,3 +46,4 @@ Read this index first, then open the records relevant to the change. Before impl
 | [051](051-nullable-column-casts.md) | 2026-10-31 | Nullable annotations infer the cast of their inner type | `Caster.normalize_type` only knew bare types, so `int \| None`, `Optional[int]` and `Union[int, None]` fell through to the `"str"` cast and nullable columns (e.g. a `HasMany` foreign key) were read back as strings, breaking eager-load key matching. `Caster.build` (replacing `build_casts`) removes `None` from a top-level union, also around or inside `Annotated`, maps the inner type to its usual cast and records the field as nullable. For nullable fields only, `Caster.get` turns `None` into `None` (or the `Field` default) and `Caster.set` writes `None`; bare annotations keep their behaviour. Unions of several non-`None` types get a pass-through `BaseCast`. Relationship key matching stays type-strict. |
 | [052](052-sqlite-savepoint-isolation.md) | 2026-10-31 | SQLite savepoints start a real transaction | pysqlite/aiosqlite defer `BEGIN` until the first DML, so a savepoint opened first in an outer transaction became a top-level transaction and `RELEASE` committed it, leaking rows past `DatabaseTransaction`/`RefreshDatabase` rollbacks. `SQliteConnection` listens to the engine `savepoint` event and runs `BEGIN` when the driver connection is not in a transaction. On the shared `StaticPool` connection, a lost `BEGIN` race (`OperationalError` while a transaction is already open) is ignored, so concurrent nested transactions still succeed as on `main`. SQLAlchemy's `isolation_level=None` + `BEGIN` on every `begin` recipe was rejected because concurrent transactions on `SQLiteConfig`'s shared `StaticPool` connection then fail with `cannot start a transaction within a transaction`. |
 | [053](053-validated-reads-query-and-body.md) | 2026-10-31 | `Validated` reads the query string and the body | `Validated[Model]` declared a required `Body()`, so a `GET` with filters got `422 body: Field required` and apps bound models with `Query()`, skipping `authorize`, rules and `after`. The body parameter now defaults to `None`. Without a body, `GET`/`HEAD`/`OPTIONS`/`DELETE` validate `query_input`, while `POST`/`PUT`/`PATCH` still report `"": Field required` (per embed key when embedded), so a body-less cross-site form `POST` cannot pass on the query string (CSRF). `DELETE` always needs a CORS preflight. With a body, `request_input` merges `{**query, **body}` for an object; any other body, a JSON `null` included, passes through and fails with `model_type`. `query_input` keeps the keys pydantic accepts (`validation_alias`, `AliasChoices`, single-key `AliasPath`, the name with `validate_by_name`/`populate_by_name`). `collections.abc` `Sequence`/`Set` fields (not `str`/`bytes`) get every repeated value. Declared fields can be set from the URL on write methods with a JSON body, as with Laravel's `$request->all()`: keep sensitive fields off such models. `document_query_input` (installed by `FastAPIProvider`) replaces `requestBody` with `in: query` parameters on query-method routes. Chosen over a separate `ValidatedQuery` so one `FormRequest` serves `GET` and `POST`. |
+| [056](056-validated-write-methods-read-the-body-only.md) | 2026-10-31 | `Validated` write methods read the body only, with an opt-in query merge | Amends ADR 053, whose `{**query, **body}` merge on every method let `POST /login?password=…` validate (secrets in logs) and `PATCH /profile?role=admin` set `role` in `model_fields_set` (mass assignment). `request_input(params, body, *, method)` now merges the query only on `GET`/`HEAD`/`OPTIONS` (`SAFE_METHODS`) or when the model sets `merge_query: ClassVar[bool] = True` (named like `stop_on_first_failure`; the body wins). `POST`/`PUT`/`PATCH` and a `DELETE` with a body validate the body alone; a body-less `DELETE` still reads the query. An embedded model whose key is absent or `null` in a write body validates `{}` (its `required` rules under its prefix) unless it opts in. Per-request merging uses `prepare_for_validation` with `cls.query_input(validator.request.query_params)`, so no new hook. Body-less writes still report `"": Field required` (CSRF), non-object bodies still fail with `model_type`, OpenAPI is unchanged. |

fastapi_startkit/src/fastapi_startkit/validation/form_request.py

@@ -38,7 +38,8 @@
 RECORDED = "fastapi_startkit.validation.recorded"
 PENDING_ERRORS = "fastapi_startkit.validation.errors"
 VALIDATED_KEY = "__validated_key__"
-QUERY_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "DELETE"})
+SAFE_METHODS = frozenset({"GET", "HEAD", "OPTIONS"})
+QUERY_METHODS = SAFE_METHODS | {"DELETE"}
 TEXT_TYPES = (str, bytes, bytearray)
 ABSENT: Any = object()
 
@@ -92,6 +93,7 @@ def rule_fields(cls) -> Iterator[tuple[str, Field]]:
                     yield name, metadata
 
     stop_on_first_failure: ClassVar[bool] = False
+    merge_query: ClassVar[bool] = False
 
     @classmethod
     def authorize(cls, validator: Validator) -> bool | Awaitable[bool]:
@@ -162,10 +164,10 @@ def query_input(cls, params: QueryParams) -> dict[str, Any]:
         }
 
     @classmethod
-    def request_input(cls, params: QueryParams, body: Any = ABSENT) -> Any:
+    def request_input(cls, params: QueryParams, body: Any = ABSENT, *, method: str) -> Any:
         if body is ABSENT:
             return cls.query_input(params)
-        if isinstance(body, Mapping):
+        if isinstance(body, Mapping) and (cls.merge_query or method in SAFE_METHODS):
             return {**cls.query_input(params), **body}
         return body
 
@@ -312,9 +314,10 @@ async def dependency(**parameters: Any) -> Any:
         if not raw and request.method not in QUERY_METHODS:
             errors.add(key if embedded else "", "Field required")
         else:
-            body = parameters[key] if raw and not (embedded and parameters[key] is None) else ABSENT
+            body = ABSENT if not raw else {} if embedded and parameters[key] is None else parameters[key]
+            data = model.request_input(request.query_params, body, method=request.method)
             try:
-                instance = await model.validate_input(model.request_input(request.query_params, body), context, request)
+                instance = await model.validate_input(data, context, request)
             except ValidationException as error:
                 errors.merge(error.bag, prefix=key if embedded else None)
         if errors.any() and (route is None or route.last is dependency):