From dfcd830cc144ea2662b9c9f1f1d7be284a817f7e Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 10 Aug 2026 19:34:09 +0300 Subject: [PATCH 1/2] docs(planning): file the two defects deferred from the access-log fix Both were found while fixing the Litestar access-log body leak (#162) and kept out of it to leave a security fix unencumbered. - log_stream binds sys.stdout at import, so structlog output ignores a stdout the process rebinds before bootstrap, while the root-logger handler follows it. - Litestar.from_config() skips the request_max_body_size default that Litestar.__init__ applies, so every body-reading handler returns 500 unless the caller sets the field themselves. --- ...6-08-10.02-log-stream-bind-at-bootstrap.md | 74 ++++++++++ ...08-10.03-litestar-request-max-body-size.md | 126 ++++++++++++++++++ 2 files changed, 200 insertions(+) create mode 100644 planning/changes/2026-08-10.02-log-stream-bind-at-bootstrap.md create mode 100644 planning/changes/2026-08-10.03-litestar-request-max-body-size.md diff --git a/planning/changes/2026-08-10.02-log-stream-bind-at-bootstrap.md b/planning/changes/2026-08-10.02-log-stream-bind-at-bootstrap.md new file mode 100644 index 0000000..87aa0af --- /dev/null +++ b/planning/changes/2026-08-10.02-log-stream-bind-at-bootstrap.md @@ -0,0 +1,74 @@ +--- +summary: Resolve `_MemoryLoggerFactoryConfig.log_stream` at bootstrap instead of at import, so structlog output follows a stdout the process rebinds after importing `lite_bootstrap` (the root-logger handler already does). +--- + +# Change: Bind the memory logger's stream at bootstrap, not at import + +**Lane:** lightweight — ≲30 LOC net, ≤2 files, no new file, no public-API +change, a single straightforward test. + +## Goal + +`_MemoryLoggerFactoryConfig.log_stream` (`lite_bootstrap/instruments/logging_factory.py`) +defaults to a bare `sys.stdout`, evaluated once when the module is imported. +Every `MemoryLoggerFactory` handler therefore writes to whatever `sys.stdout` +was at import time, even when the process rebinds `sys.stdout` before +bootstrapping. Resolve it at bootstrap instead. + +## Approach + +```python +log_stream: typing.Any = dataclasses.field(default_factory=lambda: sys.stdout) +``` + +The config is constructed in `LoggingInstrument.memory_logger_factory`, so a +`default_factory` moves the lookup to bootstrap time — the same moment +`_configure_foreign_loggers` already binds its root-logger +`logging.StreamHandler(sys.stdout)`. Today the two disagree: the root handler +follows a rebound stdout and the structlog path does not. + +Observed with a plain `FreeBootstrapper` (all bootstrappers share +`LoggingInstrument`, so this is not Litestar-specific): + +```python +with contextlib.redirect_stdout(buffer): + FreeBootstrapper(bootstrap_config=FreeConfig(service_name="svc", logging_buffer_capacity=0)).bootstrap() + structlog.get_logger("demo").info("hello after redirect") +# buffer is empty; the line went to the real stdout instead +``` + +Two consequences worth naming. In production, anything that wraps or replaces +`sys.stdout` after import — `contextlib.redirect_stdout`, a supervisor that +re-points the stream, a test harness — is silently bypassed by structlog output +while stdlib output follows along. In this repo's own test suite it is why +neither `capsys` nor `capfd` can observe structlog lines (pytest installs its +capture before collection imports the module), which forced +`tests/test_litestar_bootstrap.py` to record through a handler attached to the +`litestar` logger. That workaround stays either way; it is independent of the +capture mechanism, which is the point of it. + +Behavior is unchanged for the ordinary case, where nothing rebinds `sys.stdout` +between import and bootstrap. + +## Files + +- `lite_bootstrap/instruments/logging_factory.py` — `log_stream` gains a `default_factory`. +- `tests/instruments/test_logging_instrument.py` — test added. + +## Verification + +- [ ] Failing test first: bootstrap a `FreeBootstrapper` inside + `contextlib.redirect_stdout(io.StringIO())`, log one line, assert it lands + in the buffer. Command: `just test -k "log_stream"`. Expected failure: the + buffer is empty because the line went to the import-time stdout. +- [ ] Apply the change. +- [ ] Test passes — `just test -k "log_stream"`. +- [ ] `just test` — full suite green, coverage still 100%. +- [ ] `just lint` — clean. + +## Notes + +Found while fixing the Litestar access-log body leak +(`planning/changes/2026-08-10.01-litestar-middleware-logging.md`), which is +where the test-capture consequence is documented. Deliberately left out of that +change to keep a security fix unencumbered. diff --git a/planning/changes/2026-08-10.03-litestar-request-max-body-size.md b/planning/changes/2026-08-10.03-litestar-request-max-body-size.md new file mode 100644 index 0000000..cfd1c51 --- /dev/null +++ b/planning/changes/2026-08-10.03-litestar-request-max-body-size.md @@ -0,0 +1,126 @@ +--- +summary: Fill `request_max_body_size` with Litestar's own 10 MB default when the bootstrapped `AppConfig` leaves it `Empty`, so body-reading handlers stop returning 500 under `Litestar.from_config()`, and pin the constant against Litestar's signature so an upstream change fails CI. +--- + +# Design: Apply Litestar's request_max_body_size default when the AppConfig leaves it unset + +## Summary + +`LitestarBootstrapper` builds its application with `Litestar.from_config()`, +which — unlike `Litestar(...)` — does not apply the 10 MB +`request_max_body_size` default. An `AppConfig` that leaves the field at +`Empty` therefore yields an application where every handler that reads a +request body returns 500. Fill the field in the bootstrapper when, and only +when, it is `Empty`. + +## Motivation + +Reproduced on litestar 2.24.0: + +```python +app = litestar.Litestar(route_handlers=[echo]) # request_max_body_size == 10_000_000 +app = litestar.Litestar.from_config(AppConfig(...)) # request_max_body_size is Empty +``` + +With the second form, a `POST` to a handler taking `data: dict` returns 500: + +``` +ImproperlyConfiguredException: 500: 'request_max_body_size' set to 'Empty' on all layers. +To omit a limit, set 'request_max_body_size=None' +``` + +`LitestarConfig.application_config` defaults to a bare `AppConfig()`, and a +caller who supplies their own `AppConfig` hits the same default, so **the +failure is the norm rather than the edge case**: any lite-bootstrap Litestar +service whose handlers accept a body 500s unless the caller happens to know to +set `request_max_body_size` themselves. It is not per-route recoverable either +— the exception is raised while resolving the layered value, so the only fixes +are on the handler, a router, or the app. + +Found while fixing the access-log body leak +(`planning/changes/2026-08-10.01-litestar-middleware-logging.md`), whose tests +work around it with `request_max_body_size=1000` on their handlers. That +workaround is what should disappear. + +## Design + +`LitestarBootstrapper._apply_config` already owns exactly this job — it is the +one place that mutates the caller's `AppConfig` before +`Litestar.from_config()` runs, setting `debug` and appending the teardown hook. +Add the fill there: + +```python +# litestar_bootstrapper.py, module level +# Litestar.from_config() skips the default that Litestar.__init__ applies, leaving the +# field Empty and 500-ing every body-reading handler. Pinned by a guard test. +_LITESTAR_DEFAULT_REQUEST_MAX_BODY_SIZE: typing.Final = 10_000_000 +``` + +```python + def _apply_config(self, application_config: "AppConfig") -> None: + application_config.debug = self.bootstrap_config.service_debug + if application_config.request_max_body_size is Empty: + application_config.request_max_body_size = _LITESTAR_DEFAULT_REQUEST_MAX_BODY_SIZE + application_config.on_shutdown.append(self.teardown) +``` + +`Empty` is an enum member (`litestar.types.Empty`, `_EmptyEnum.EMPTY`), not a +class, so the check is an identity comparison — `isinstance` would raise. +`Empty` joins the existing `if import_checker.is_litestar_installed:` import +block. + +The guard is the `is Empty` test: a caller's own value, including an explicit +`None` (Litestar's "no limit"), is left alone. Only the unset case is filled. + +**Pinning the constant.** Litestar exposes no public constant for the default; +the value lives only in `Litestar.__init__`'s signature, so hardcoding it can +drift silently on a Litestar bump. A guard test reads the signature default and +asserts it equals our constant, turning a drift into a CI failure rather than a +behavior change nobody notices. Runtime introspection was rejected: it makes +every bootstrap depend on a parameter name Litestar does not publish as API, +and it fails opaquely if that name changes. + +**Upstream.** `Litestar.from_config()` diverging from `Litestar(...)` on a +constructor default looks like a Litestar defect, not an intended contract. +File an issue against litestar-org/litestar; if it is fixed upstream, the +`is Empty` branch simply stops firing and the guard test keeps the constant +honest until the fill can be dropped. + +## Non-goals + +- Exposing `request_max_body_size` as a `LitestarConfig` field. Callers who + want a non-default limit set it on their own `AppConfig`, which is where + every other Litestar app-level knob already lives. +- Auditing the other `AppConfig` fields where `from_config()` may diverge from + `Litestar.__init__`. If more turn up, they get their own change. + +## Testing + +`just test -k "request_max_body_size"`, in `tests/test_litestar_bootstrap.py`: + +- a bootstrapped app with a body-reading handler and no explicit + `request_max_body_size`: `POST` succeeds (today: 500). +- a caller-supplied value survives: `AppConfig(request_max_body_size=42)` + bootstraps to `42`. +- an explicit `None` (Litestar's no-limit form) survives as `None`. +- guard: `inspect.signature(litestar.Litestar.__init__).parameters["request_max_body_size"].default` + equals `_LITESTAR_DEFAULT_REQUEST_MAX_BODY_SIZE`. + +Once green, drop the `request_max_body_size=1000` workaround from the +access-logging tests added in `2026-08-10.01`, so the suite stops carrying a +note about a defect that no longer exists. + +Then `just lint-ci` and the full `just test`. + +## Risk + +**The constant drifts from Litestar's default.** Low likelihood, low impact +(the number is a size limit), and the guard test converts it into a failed CI +run on the bump that changes it. + +**A caller relying on the 500.** Implausible — it is an +`ImproperlyConfiguredException`, not a documented limit. + +**Promotion:** `architecture/bootstrappers.md` records that `_apply_config` now +also fills Litestar's unset body-size default, alongside `debug` and the +teardown hook. From 1ba620a07ad4d7d5e5a4c9598f90f9ff6b8eabdb Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 10 Aug 2026 19:43:07 +0300 Subject: [PATCH 2/2] docs(planning): point the body-size change at the upstream issue The from_config/__init__ default divergence is already tracked as litestar-org/litestar#4296; link it and our reproduction instead of asking a future implementer to file a duplicate. --- ...026-08-10.03-litestar-request-max-body-size.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/planning/changes/2026-08-10.03-litestar-request-max-body-size.md b/planning/changes/2026-08-10.03-litestar-request-max-body-size.md index cfd1c51..c1e9813 100644 --- a/planning/changes/2026-08-10.03-litestar-request-max-body-size.md +++ b/planning/changes/2026-08-10.03-litestar-request-max-body-size.md @@ -81,10 +81,17 @@ every bootstrap depend on a parameter name Litestar does not publish as API, and it fails opaquely if that name changes. **Upstream.** `Litestar.from_config()` diverging from `Litestar(...)` on a -constructor default looks like a Litestar defect, not an intended contract. -File an issue against litestar-org/litestar; if it is fixed upstream, the -`is Empty` branch simply stops firing and the guard test keeps the constant -honest until the fill can be dropped. +constructor default is already reported as +[litestar-org/litestar#4296](https://github.com/litestar-org/litestar/issues/4296), +which lists five such mismatches; our reproduction and the reason this one is a +hard failure rather than a cosmetic difference are in +[a comment there](https://github.com/litestar-org/litestar/issues/4296#issuecomment-5243196116). +`from_config` passes every `AppConfig` field explicitly +(`cls(**dict(extract_dataclass_items(config)))`), so an `__init__` default can +never apply — and on 2.24.0 `request_max_body_size` is the only field where +`AppConfig()` is `Empty` while `__init__` has a real default. If upstream fixes +it, the `is Empty` branch simply stops firing and the guard test keeps the +constant honest until the fill can be dropped. ## Non-goals