diff --git a/docs/Architecture/API/API.md b/docs/Architecture/API/API.md index 6b3a349..24b4da8 100644 --- a/docs/Architecture/API/API.md +++ b/docs/Architecture/API/API.md @@ -163,7 +163,8 @@ Notes: | --- | --- | --- | | `400` | `bad_request` | Invalid JSON, validation failures, malformed request body. | | `400` | `access_not_editable` | `PATCH /v1/config` body contains an `access` key (managed via users API). | -| `400` | `section_not_editable` | `PATCH /v1/config` body contains `server`, `network`, or an unknown top-level key. | +| `400` | `section_not_editable` | `PATCH /v1/config` body contains `network` or an unknown top-level key. | +| `400` | `field_not_editable` | `PATCH /v1/config` body contains a forbidden nested field under a partially editable section (e.g. `server.api`, `server.port`). | | `401` | `unauthorized` | Missing/invalid `Authorization` when `auth_header` is configured. | | `403` | `forbidden` | Source IP is not allowed by whitelist. | | `403` | `read_only` | Mutating endpoint called while `read_only=true`. | @@ -254,11 +255,12 @@ bob = ["198.51.100.42/32"] ### `PatchConfigRequest` -A sparse JSON object containing only the top-level config sections to modify. Each key must be one of the editable sections (`general`, `timeouts`, `censorship`, `upstreams`, `show_link`, `dc_overrides`). Tables within a section are deep-merged field-by-field into the existing config; arrays and scalar values replace the existing value wholesale. Untouched sections and file comments are preserved. +A sparse JSON object containing only the top-level config sections to modify. Each key must be one of the editable sections (`general`, `timeouts`, `censorship`, `upstreams`, `dc_overrides`) or the partially editable `server` object (only `listeners` is allowed under `server`; see below). Tables within a section are deep-merged field-by-field into the existing config; arrays and scalar values replace the existing value wholesale. Untouched sections and file comments are preserved. **Rejected keys:** - `access` → `400 access_not_editable` (users/secrets are managed via `POST/PATCH /v1/users`). -- `server`, `network`, or any unknown top-level key → `400 section_not_editable`. +- `network`, or any unknown top-level key → `400 section_not_editable`. +- `server` with any key other than `listeners` (e.g. `port`, `api`, `admin_api`) → `400 field_not_editable`. - An object with no editable keys → `400 bad_request` (empty patch). Example — patch only the SNI domain: @@ -266,6 +268,11 @@ Example — patch only the SNI domain: {"censorship": {"tls_domain": "front.example.com"}} ``` +Example — replace `[[server.listeners]]` (other `[server]` fields including `[server.api]` are preserved): +```json +{"server": {"listeners": [{"ip": "0.0.0.0", "port": 443, "client_mss": "92"}]}} +``` + ### `RotateSecretRequest` | Field | Type | Required | Description | | --- | --- | --- | --- | @@ -285,10 +292,10 @@ Returned by `GET /v1/config` as the envelope `data`. The fields are exactly the | `timeouts` | `object?` | `[timeouts]` section, if present. | | `censorship` | `object?` | `[censorship]` section, if present. | | `upstreams` | `object?` | `[upstreams]` section, if present. | -| `show_link` | `object?` | `[show_link]` section, if present. | | `dc_overrides` | `object?` | `[dc_overrides]` section, if present. | +| `server` | `object?` | Partial `[server]` view when editable nested fields are present. Currently only `listeners` may appear; `api`/`admin_api`, `port`, unix sockets, and other bind-identity fields are never returned. | -Sections absent from the config file are absent from the response (not `null`). Only the editable sections above are returned; `access` (users/secrets), `server` (carries the API `auth_header` and per-node identity), and `network` (per-node addresses) are always excluded. +Sections absent from the config file are absent from the response (not `null`). Only the editable sections above are returned; `access` (users/secrets) and `network` (per-node addresses) are always excluded. Under `server`, only the nested field-level allowlist (`listeners`) is exposed. ### `PatchConfigResponse` @@ -1360,14 +1367,15 @@ Applies a sparse patch to the editable config sections. The merged config is ful | `Content-Type: application/json` | recommended | Not enforced, but body must be valid JSON. | | `If-Match: ` | no | Optimistic concurrency. `` is the `revision` value from `GET /v1/config` or `config_hash` from `GET /v1/system/info`. If supplied and it does not match the current on-disk revision, returns `409 revision_conflict`. If omitted, the patch applies unconditionally. | -**Editable sections:** `general`, `timeouts`, `censorship`, `upstreams`, `show_link`, `dc_overrides`. +**Editable sections:** `general`, `timeouts`, `censorship`, `upstreams`, `dc_overrides`, plus partially editable `server` (only nested `listeners`). **Rejected keys and their error codes:** | Key | HTTP | `error.code` | | --- | --- | --- | | `access` | `400` | `access_not_editable` | -| `server`, `network`, or any unknown key | `400` | `section_not_editable` | +| `network`, or any unknown top-level key | `400` | `section_not_editable` | +| `server` with keys other than `listeners` | `400` | `field_not_editable` | | Object with no editable key | `400` | `bad_request` | **Merge semantics:** tables are deep-merged field-by-field; arrays and scalar values replace the existing value wholesale. File comments and untouched sections are preserved. @@ -1396,7 +1404,8 @@ Applies a sparse patch to the editable config sections. The merged config is ful | `200` | — | Patch applied successfully. | | `400` | `bad_request` | Invalid JSON, empty patch, or config validation/deserialization failure. | | `400` | `access_not_editable` | Patch contains an `access` key. | -| `400` | `section_not_editable` | Patch contains `server`, `network`, or an unknown top-level key. | +| `400` | `section_not_editable` | Patch contains `network` or an unknown top-level key. | +| `400` | `field_not_editable` | Patch contains a forbidden nested `server.*` field (anything other than `listeners`). | | `401` | `unauthorized` | Missing or invalid `Authorization` header. | | `403` | `read_only` | API is in read-only mode. | | `405` | `method_not_allowed` | Method other than `GET` or `PATCH` used on `/v1/config`. | diff --git a/src/api/config_edit.rs b/src/api/config_edit.rs index 8f25cfc..043ce88 100644 --- a/src/api/config_edit.rs +++ b/src/api/config_edit.rs @@ -1,12 +1,14 @@ //! Config-editing API: read managed sections and apply sparse field patches. //! `access.*` is intentionally not editable here (owned by the users API). +//! `[server]` is only partially editable — see [`EDITABLE_SERVER_FIELDS`]. use serde_json::Value as Json; use toml::Value as Toml; use super::ApiShared; use super::config_store::{ - EDITABLE_SECTIONS, compute_revision, current_revision, save_sections_to_disk, + EDITABLE_SECTIONS, EDITABLE_SERVER_FIELDS, compute_revision, current_revision, + is_editable_section, save_sections_to_disk, }; use super::model::ApiFailure; use crate::config::ProxyConfig; @@ -55,7 +57,7 @@ pub(super) async fn apply_patch_to_path( )); } - // 2. convert + reject access / unknown sections + // 2. convert + reject access / unknown sections / forbidden server fields let patch_toml = json_to_toml(patch_json) .map_err(|e| ApiFailure::bad_request(format!("invalid patch: {}", e)))?; let patch_table = patch_toml @@ -68,19 +70,22 @@ pub(super) async fn apply_patch_to_path( "access.* is managed via the users API, not editable here", )); } - for key in patch_table.keys() { - if !EDITABLE_SECTIONS.contains(&key.as_str()) { + for (key, value) in patch_table { + if !is_editable_section(key.as_str()) { return Err(ApiFailure::new( hyper::StatusCode::BAD_REQUEST, "section_not_editable", format!("section not editable: {}", key), )); } + if key == "server" { + validate_server_patch(value)?; + } } let touched: Vec<&str> = patch_table .keys() .map(|k| k.as_str()) - .filter(|k| EDITABLE_SECTIONS.contains(k)) + .filter(|k| is_editable_section(k)) .collect(); if touched.is_empty() { return Err(ApiFailure::bad_request("empty patch: no editable sections")); @@ -138,19 +143,83 @@ pub(super) async fn read_managed_config(config_path: &Path) -> Result<(Toml, Str .cloned() .unwrap_or_else(toml::value::Table::new); // Whitelist: return ONLY the editable sections. A blacklist (just removing - // `access`) would leak `server` (carries the API `auth_header` + per-node - // identity) and `network` (per-node addresses). Mirror the PATCH contract. + // `access`) would leak `server.api` (auth_header) and `network` (per-node + // addresses). Mirror the PATCH contract, including the nested server + // field-level allowlist. let mut table = toml::value::Table::new(); for section in EDITABLE_SECTIONS { if let Some(value) = parsed_table.get(*section) { table.insert((*section).to_string(), value.clone()); } } + if let Some(server) = parsed_table.get("server") { + if let Some(filtered) = filter_server_for_read(server) { + table.insert("server".to_string(), filtered); + } + } let revision = compute_revision(&original); Ok((Toml::Table(table), revision)) } +/// Keep only [`EDITABLE_SERVER_FIELDS`] from a `[server]` table for GET. +fn filter_server_for_read(server: &Toml) -> Option { + let Some(src) = server.as_table() else { + return None; + }; + let mut out = toml::value::Table::new(); + for field in EDITABLE_SERVER_FIELDS { + if let Some(value) = src.get(*field) { + // Skip empty listeners arrays so absent-vs-empty stays consistent + // with other optional sections. + if *field == "listeners" { + if let Some(arr) = value.as_array() { + if arr.is_empty() { + continue; + } + } + } + out.insert((*field).to_string(), value.clone()); + } + } + if out.is_empty() { + None + } else { + Some(Toml::Table(out)) + } +} + +/// Reject any `[server]` patch keys outside [`EDITABLE_SERVER_FIELDS`]. +fn validate_server_patch(server: &Toml) -> Result<(), ApiFailure> { + let Some(table) = server.as_table() else { + return Err(ApiFailure::new( + hyper::StatusCode::BAD_REQUEST, + "section_not_editable", + "server patch must be a JSON object", + )); + }; + if table.is_empty() { + return Err(ApiFailure::bad_request( + "empty server patch: provide at least one editable field \ + (currently: listeners)", + )); + } + for key in table.keys() { + if !EDITABLE_SERVER_FIELDS.contains(&key.as_str()) { + return Err(ApiFailure::new( + hyper::StatusCode::BAD_REQUEST, + "field_not_editable", + format!( + "server.{} is not editable via the config API; allowed server fields: {}", + key, + EDITABLE_SERVER_FIELDS.join(", ") + ), + )); + } + } + Ok(()) +} + /// Convert a serde_json value to a toml value. `null` is dropped from objects /// (a patch never sets a key to TOML-null). Numbers become integers when exact, /// otherwise floats. @@ -289,8 +358,8 @@ mod tests { #[tokio::test] async fn read_managed_config_returns_only_editable_sections() { - // server carries the API auth_header + per-node identity; network carries - // per-node addresses. Neither must be exposed by GET /v1/config. + // Full server (api/port) and network must not leak. Listeners-only server + // is returned via the nested allowlist (covered in a dedicated test). let (path, _d) = temp_config(concat!( "[censorship]\ntls_domain = \"a\"\n", "[server]\nport = 443\n[server.api]\nauth_header = \"SECRET\"\n", @@ -300,17 +369,74 @@ mod tests { let (value, _rev) = read_managed_config(&path).await.unwrap(); let table = value.as_table().unwrap(); assert!(table.contains_key("censorship")); - assert!(!table.contains_key("server")); // no API auth_header / identity leak + assert!(!table.contains_key("server")); // no listeners → omit whole server assert!(!table.contains_key("network")); // no per-node identity leak assert!(!table.contains_key("access")); // no users/secrets } #[tokio::test] - async fn patch_rejects_server_section() { + async fn read_managed_config_returns_server_listeners_only() { + let (path, _d) = temp_config(concat!( + "[censorship]\ntls_domain = \"a\"\n", + "[server]\nport = 443\n", + "[server.api]\nauth_header = \"SECRET\"\n", + "[[server.listeners]]\nip = \"0.0.0.0\"\nport = 443\n", + )); + let (value, _rev) = read_managed_config(&path).await.unwrap(); + let table = value.as_table().unwrap(); + let server = table.get("server").expect("server.listeners present").as_table().unwrap(); + assert!(server.contains_key("listeners")); + assert!(!server.contains_key("api")); + assert!(!server.contains_key("port")); + let listeners = server["listeners"].as_array().unwrap(); + assert_eq!(listeners.len(), 1); + assert_eq!(listeners[0]["port"].as_integer(), Some(443)); + } + + #[tokio::test] + async fn patch_rejects_forbidden_server_fields() { let (path, _d) = temp_config("[censorship]\ntls_domain = \"a\"\n"); let patch: Json = serde_json::json!({"server": {"port": 1}}); let err = apply_patch_to_path(&path, &patch, None).await.unwrap_err(); - assert_eq!(err.code, "section_not_editable"); + assert_eq!(err.code, "field_not_editable"); + } + + #[tokio::test] + async fn patch_rejects_server_api_field() { + let (path, _d) = temp_config("[censorship]\ntls_domain = \"a\"\n"); + let patch: Json = serde_json::json!({"server": {"api": {"enabled": false}}}); + let err = apply_patch_to_path(&path, &patch, None).await.unwrap_err(); + assert_eq!(err.code, "field_not_editable"); + } + + #[tokio::test] + async fn patch_server_listeners_preserves_api() { + let (path, _d) = temp_config(concat!( + "[censorship]\ntls_domain = \"a\"\n", + "[server]\nport = 443\n", + "[server.api]\nenabled = true\nauth_header = \"SECRET\"\n", + "[[server.listeners]]\nip = \"0.0.0.0\"\nport = 443\n", + )); + let patch: Json = serde_json::json!({ + "server": { + "listeners": [ + {"ip": "0.0.0.0", "port": 8443, "client_mss": "92"} + ] + } + }); + let resp = apply_patch_to_path(&path, &patch, None).await.unwrap(); + assert!(resp.changed.iter().any(|c| c == "server")); + let written = tokio::fs::read_to_string(&path).await.unwrap(); + let parsed: toml::Value = toml::from_str(&written).unwrap(); + assert_eq!( + parsed["server"]["api"]["auth_header"].as_str(), + Some("SECRET"), + "{written}" + ); + let listeners = parsed["server"]["listeners"].as_array().unwrap(); + assert_eq!(listeners.len(), 1, "{written}"); + assert_eq!(listeners[0]["port"].as_integer(), Some(8443), "{written}"); + assert_eq!(listeners[0]["client_mss"].as_str(), Some("92"), "{written}"); } #[tokio::test] diff --git a/src/api/config_store.rs b/src/api/config_store.rs index cc5dc25..288a45f 100644 --- a/src/api/config_store.rs +++ b/src/api/config_store.rs @@ -103,7 +103,6 @@ pub(super) async fn save_config_to_disk( /// identity invariant at the Telemt layer too): /// /// - `access` : owned by the users API. -/// - `server` : carries per-node identity (`port`, `api`/`api_bind`, listeners). /// - `network` : carries per-node identity (`ipv4`/`ipv6`). /// - `show_link` : legacy top-level scalar/array (not a `[table]`), superseded /// by the editable `general.links.show` sub-table. The @@ -111,6 +110,11 @@ pub(super) async fn save_config_to_disk( /// `[[array-of-tables]]` blocks; a bare top-level key cannot be /// located or replaced safely, so it is edited via `general`. /// +/// `server` is partially editable: only the nested fields listed in +/// [`EDITABLE_SERVER_FIELDS`] (currently `listeners`) may appear in GET/PATCH. +/// Secrets and bind identity (`api`/`admin_api`, `port`, unix sockets, …) stay +/// blocked. See also the field-level allowlist note below for `network.*`. +/// /// A future field-level allowlist can re-admit specific safe fields /// (e.g. `network.dns_overrides`) without opening the whole section. pub(super) const EDITABLE_SECTIONS: &[&str] = &[ @@ -121,6 +125,20 @@ pub(super) const EDITABLE_SECTIONS: &[&str] = &[ "dc_overrides", ]; +/// Nested fields under `[server]` that may be read/patched via the config API. +/// +/// Arrays (e.g. `listeners`) replace wholesale on PATCH, matching the existing +/// merge semantics for non-table values. +pub(super) const EDITABLE_SERVER_FIELDS: &[&str] = &["listeners"]; + +/// Whether `key` is an allowed top-level PATCH/GET section name. +/// +/// Fully editable sections from [`EDITABLE_SECTIONS`], plus `server` which is +/// further restricted by [`EDITABLE_SERVER_FIELDS`]. +pub(super) fn is_editable_section(key: &str) -> bool { + EDITABLE_SECTIONS.contains(&key) || key == "server" +} + /// Re-render the given top-level tables from `cfg` and upsert each into the /// on-disk file, preserving every untouched section (and its comments). pub(super) async fn save_sections_to_disk(