Merge pull request #895 from telemt/flow-api-patch-server-listeners

Allow PATCH/GET of server.listeners via config API
2026-09-07 02:56:27 +03:00 · 2026-08-22 10:10:34 +03:00
parent 5e70455ca2 32ff9e16c1
commit 96d467a400
3 changed files with 178 additions and 22 deletions
@@ -167,7 +167,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`. |
@@ -260,11 +261,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:
@@ -272,6 +274,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 |
 | --- | --- | --- | --- |
@@ -291,10 +298,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`

@@ -1404,14 +1411,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: <revision>` | no | Optimistic concurrency. `<revision>` 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.
@@ -1466,7 +1474,8 @@ Without a `reload` query parameter, the endpoint preserves the legacy behavior:
 | `202` | — | Patch applied and runtime reload accepted. |
 | `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`. |
@@ -1,13 +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, load_config_from_disk,
-    save_sections_to_disk,
+    EDITABLE_SECTIONS, EDITABLE_SERVER_FIELDS, compute_revision, current_revision,
+    is_editable_section, load_config_from_disk, save_sections_to_disk,
 };
 use super::model::ApiFailure;
 use crate::config::ProxyConfig;
@@ -94,7 +95,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
@@ -107,19 +108,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"));
@@ -182,19 +186,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<Toml> {
+    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.
@@ -336,8 +404,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",
@@ -347,17 +415,78 @@ 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]
@@ -118,7 +118,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
@@ -126,6 +125,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] = &[
@@ -136,6 +140,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(