From 69d28191e8db288bb05ee7382bf64912a0eaefd9 Mon Sep 17 00:00:00 2001 From: blaipr Date: Wed, 26 Aug 2026 13:49:17 +0200 Subject: [PATCH] feat: explicit request body limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The limit was always enforced — axum ships a 2 MB default — but silently: nothing declared it, and an oversized push got a bare 413. It is now the server.max_body_bytes config.yaml key (same 2 MiB default, restart-only, reported by a reload like the other router-layer settings), and exceeding it answers 413 with the standard error body naming the key and the limit. --- CHANGELOG.md | 8 ++++++ docs/api.md | 2 ++ docs/config-api.md | 6 ++++- docs/configuration.md | 5 ++++ src/adapters/in/http/routes.rs | 27 +++++++++++++++++++- src/config.rs | 21 ++++++++++++++++ src/lib.rs | 10 ++++++++ tests/sync_test.rs | 45 ++++++++++++++++++++++++++++++++++ 8 files changed, 122 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5295d2..afb17f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,14 @@ project adheres to [Semantic Versioning](https://semver.org/). land; the shutdown grace is read when the drain starts, so the last reloaded value governs it. +- **`server.max_body_bytes` makes the request body limit explicit.** The limit + was always enforced — axum ships a 2 MB default — but silently: nothing + declared it, and an oversized push (a whole-directory config `PUT` is one + body) got a bare 413 with no explanation. The limit is now a named + `config.yaml` setting with the same 2 MiB default, and exceeding it answers + `413` with the standard `{"error": ...}` body naming the key and the + configured limit. + ## [0.22.0] - 2026-08-22 ### Added diff --git a/docs/api.md b/docs/api.md index 3498b0c..332da00 100644 --- a/docs/api.md +++ b/docs/api.md @@ -80,6 +80,8 @@ status code, read the message. That includes `401`: the API-key middleware rejects a request before any handler runs, but it renders the same shape — `missing API key` names the header to pass, `invalid API key` means one was passed and matched nothing. +It also includes `413`: a request body larger than `server.max_body_bytes` +(default 2 MiB) is refused with a body naming that key and the limit. --- diff --git a/docs/config-api.md b/docs/config-api.md index 50fb0c5..19f10ee 100644 --- a/docs/config-api.md +++ b/docs/config-api.md @@ -134,6 +134,10 @@ Every problem at once, the same list `--check-config` prints — because it is the same code, run against a staged copy of the directory the push would produce. +A whole-directory push is one request body, so it lives under +`server.max_body_bytes` (default 2 MiB) like every other route; a directory +that outgrows it answers `413` naming the key to raise. + --- ## What a reload can and cannot apply @@ -159,7 +163,7 @@ is already running. **Needs a restart** — reported, never silently ignored: `server.host`, `server.port`, `server.cors_allowed_origins`, -`server.metrics_require_auth`, +`server.metrics_require_auth`, `server.max_body_bytes`, `cache.persistence`, `projects.dir`, `config_api.enabled`. A write that touches one of them still lands on disk; the response names it: diff --git a/docs/configuration.md b/docs/configuration.md index d32622c..a868fb9 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -58,6 +58,11 @@ server: # before writing the final cache snapshot anyway. Size it to fit inside the # pod's terminationGracePeriodSeconds with room for the snapshot write. shutdown_grace_seconds: 20 + # Optional, default 2097152 (2 MiB — what was always enforced, implicitly). + # Largest request body accepted on any route; an oversized one answers 413 + # naming this key. Size it for the biggest configuration push (a whole + # directory is one body) or host-vars write you expect. + max_body_bytes: 2097152 # Optional. Absent = purely in-memory cache (restarts start empty). # With the block, the cache is snapshotted to `path` every `interval_seconds` diff --git a/src/adapters/in/http/routes.rs b/src/adapters/in/http/routes.rs index dc18e0b..71fbfdf 100644 --- a/src/adapters/in/http/routes.rs +++ b/src/adapters/in/http/routes.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use axum::http::HeaderValue; use axum::{ Router, middleware, - response::Redirect, + response::{IntoResponse, Redirect}, routing::{delete, get, post, put}, }; use tower_http::compression::CompressionLayer; @@ -28,6 +28,7 @@ pub fn create_router( api_keys: Arc, cors_allowed_origins: Vec, metrics_require_auth: bool, + max_body_bytes: usize, ) -> Router<()> { let mut api_routes = Router::new() .route("/api/v1/sources", get(http::sources::list_cached_sources)) @@ -120,6 +121,30 @@ pub fn create_router( ) .with_state(state); + // The body limit was always enforced — axum ships a 2 MB default — but + // silently: nothing declared it, and the extractor's rejection is plain + // text, unlike every other failure (see error.rs). The layer makes the + // limit the configured value; the response mapper below gives its 413 the + // standard {"error": ...} body naming the setting to raise. Rewriting is + // safe unconditionally because no handler answers 413 itself. + let router = router + .layer(axum::extract::DefaultBodyLimit::max(max_body_bytes)) + .layer(middleware::map_response( + move |response: axum::response::Response| async move { + if response.status() == axum::http::StatusCode::PAYLOAD_TOO_LARGE { + return http::error::ApiError::new( + axum::http::StatusCode::PAYLOAD_TOO_LARGE, + format!( + "request body exceeds server.max_body_bytes ({} bytes)", + max_body_bytes + ), + ) + .into_response(); + } + response + }, + )); + // No configured origins = no CORS layer: the browser same-origin policy // applies and server-to-server consumers are unaffected. This replaces // the old always-on allow-anything layer. diff --git a/src/config.rs b/src/config.rs index 5395c66..893e1a1 100644 --- a/src/config.rs +++ b/src/config.rs @@ -83,6 +83,15 @@ pub struct ServerConfig { // behavior — the snapshot is best-effort — rather than blocking exit. #[serde(default = "default_shutdown_grace_seconds")] pub shutdown_grace_seconds: u64, + + // Largest request body accepted, in bytes, on every route. This was + // always enforced — axum ships a 2 MB default — but silently: nothing + // declared it and an oversized push got a bare 413. Now the limit is a + // named setting, and the 413 carries the standard error body naming it. + // Size it for the biggest config file a pipeline pushes (a whole-directory + // PUT is one body) or the largest host-vars document a consumer writes. + #[serde(default = "default_max_body_bytes")] + pub max_body_bytes: usize, } fn default_refresh_timeout_seconds() -> u64 { @@ -97,6 +106,12 @@ fn default_shutdown_grace_seconds() -> u64 { 20 } +fn default_max_body_bytes() -> usize { + // axum's own default, kept so making the limit explicit changes nothing + // for existing deployments. + 2 * 1024 * 1024 +} + // Cache behavior — config.yaml, `cache:` section (optional) #[derive(Deserialize, Default)] #[serde(deny_unknown_fields)] @@ -231,6 +246,7 @@ pub struct RestartOnlySettings { pub port: u16, pub cors_allowed_origins: Vec, pub metrics_require_auth: bool, + pub max_body_bytes: usize, pub persistence_path: Option, pub persistence_interval_seconds: Option, pub projects_dir: String, @@ -244,6 +260,7 @@ impl RestartOnlySettings { port: cfg.server.port, cors_allowed_origins: cfg.server.cors_allowed_origins.clone(), metrics_require_auth: cfg.server.metrics_require_auth, + max_body_bytes: cfg.server.max_body_bytes, persistence_path: cfg.cache.persistence.as_ref().map(|p| p.path.clone()), persistence_interval_seconds: cfg .cache @@ -274,6 +291,10 @@ impl RestartOnlySettings { self.metrics_require_auth != other.metrics_require_auth, "server.metrics_require_auth", ); + check( + self.max_body_bytes != other.max_body_bytes, + "server.max_body_bytes", + ); check( self.persistence_path != other.persistence_path || self.persistence_interval_seconds != other.persistence_interval_seconds, diff --git a/src/lib.rs b/src/lib.rs index 90b748b..93fcfce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,6 +60,7 @@ pub struct AppBuilder { refresh_timeout_seconds: u64, refresh_max_concurrent: usize, shutdown_grace_seconds: u64, + max_body_bytes: usize, } impl AppBuilder { @@ -88,6 +89,8 @@ impl AppBuilder { refresh_timeout_seconds: 15, refresh_max_concurrent: 8, shutdown_grace_seconds: 20, + // axum's own default, mirrored like the three above + max_body_bytes: 2 * 1024 * 1024, } } @@ -161,6 +164,7 @@ impl AppBuilder { self.refresh_timeout_seconds = cfg.server.refresh_timeout_seconds; self.refresh_max_concurrent = cfg.server.refresh_max_concurrent; self.shutdown_grace_seconds = cfg.server.shutdown_grace_seconds; + self.max_body_bytes = cfg.server.max_body_bytes; self } @@ -227,6 +231,11 @@ impl AppBuilder { self } + pub fn max_body_bytes(mut self, bytes: usize) -> Self { + self.max_body_bytes = bytes; + self + } + pub fn build(self) -> Router<()> { let (router, _state) = self.build_with_state(); router @@ -292,6 +301,7 @@ impl AppBuilder { )), self.cors_allowed_origins, self.metrics_require_auth, + self.max_body_bytes, ); (router, state) } diff --git a/tests/sync_test.rs b/tests/sync_test.rs index 303e5cd..2107f37 100644 --- a/tests/sync_test.rs +++ b/tests/sync_test.rs @@ -530,6 +530,51 @@ async fn put_host_adds_to_cache() { assert_eq!(dataset["hostvars"].as_object().unwrap().len(), 7); // 6 + 1 } +// ========================================================================= +// Test: a body over server.max_body_bytes answers 413 with the standard shape +// ========================================================================= +#[tokio::test] +async fn an_oversized_body_answers_413_with_a_json_error() { + let mut sources = HashMap::new(); + sources.insert("src-test".to_string(), test_source("default")); + let app = unified_api::AppBuilder::new() + .sources(sources) + .max_body_bytes(64) + .build(); + + let (_, _) = request(app.clone(), "POST", "/api/v1/sources/src-test/sync").await; + + // Comfortably over the 64-byte limit. + let (status, body) = request_with_json( + app.clone(), + "PUT", + "/api/v1/sources/src-test/hosts/togusa.section9.net", + serde_json::json!({"notes": "x".repeat(200)}), + ) + .await; + + assert_eq!(status, StatusCode::PAYLOAD_TOO_LARGE); + // The refusal names the setting to raise, in the standard error shape — + // not axum's plain-text rejection. + let parsed: serde_json::Value = serde_json::from_str(&body).expect("a JSON error body"); + let message = parsed["error"].as_str().expect("an error field"); + assert!( + message.contains("server.max_body_bytes") && message.contains("64"), + "message: {}", + message + ); + + // An in-limit write on the same app still lands. + let (status, _) = request_with_json( + app.clone(), + "PUT", + "/api/v1/sources/src-test/hosts/togusa.section9.net", + serde_json::json!({"role": "detective"}), + ) + .await; + assert_eq!(status, StatusCode::OK); +} + // ========================================================================= // Test: DELETE host — immediate removal // =========================================================================