From ff7a12d5f8a46f3d80911ff1a36f3ab5b16ca031 Mon Sep 17 00:00:00 2001 From: Mirotin Artem Date: Tue, 9 Jun 2026 12:13:32 +0300 Subject: [PATCH] fix(api): GET /v1/config returns only editable sections; tolerate commented TOML headers; doc fixes --- docs/Architecture/API/API.md | 5 ++--- src/api/config_edit.rs | 32 +++++++++++++++++++++++++++++--- src/api/config_store.rs | 22 ++++++++++++++++++---- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/docs/Architecture/API/API.md b/docs/Architecture/API/API.md index 3fa4cca..2c2de51 100644 --- a/docs/Architecture/API/API.md +++ b/docs/Architecture/API/API.md @@ -277,11 +277,10 @@ An empty request body is accepted and generates a new secret automatically. ### `ConfigData` -Returned by `GET /v1/config`. The top-level fields mirror the editable TOML sections; `access.*` is intentionally omitted. +Returned by `GET /v1/config` as the envelope `data`. The fields are exactly the editable TOML sections. The current revision is returned in the envelope `revision` field (same value as `config_hash` in `SystemInfoData`), **not** inside `data`. | Field | Type | Description | | --- | --- | --- | -| `revision` | `string` | SHA-256 hex of the current on-disk config content (same value as `config_hash` in `SystemInfoData` and the envelope `revision`). | | `general` | `object?` | `[general]` section, if present in config. | | `timeouts` | `object?` | `[timeouts]` section, if present. | | `censorship` | `object?` | `[censorship]` section, if present. | @@ -289,7 +288,7 @@ Returned by `GET /v1/config`. The top-level fields mirror the editable TOML sect | `show_link` | `object?` | `[show_link]` section, if present. | | `dc_overrides` | `object?` | `[dc_overrides]` section, if present. | -Sections absent from the config file are absent from the response (not `null`). The `access` section is always stripped — users and secrets are never exposed here. +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. ### `PatchConfigResponse` diff --git a/src/api/config_edit.rs b/src/api/config_edit.rs index 5fe1f57..1de100b 100644 --- a/src/api/config_edit.rs +++ b/src/api/config_edit.rs @@ -125,7 +125,7 @@ pub(super) async fn apply_patch_to_path( }) } -/// Return the editable config sections (no `access.*`) + current revision. +/// Return only the editable config sections + current revision. pub(super) async fn read_managed_config(config_path: &Path) -> Result<(Toml, String), ApiFailure> { let original = tokio::fs::read_to_string(config_path) .await @@ -133,11 +133,19 @@ pub(super) async fn read_managed_config(config_path: &Path) -> Result<(Toml, Str let parsed: Toml = toml::from_str(&original) .map_err(|e| ApiFailure::internal(format!("failed to parse config: {}", e)))?; - let mut table = parsed + let parsed_table = parsed .as_table() .cloned() .unwrap_or_else(toml::value::Table::new); - table.remove("access"); // never expose users/secrets via this endpoint + // 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. + 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()); + } + } let revision = compute_revision(&original); Ok((Toml::Table(table), revision)) @@ -279,6 +287,24 @@ mod tests { assert_eq!(revision, current_revision(&path).await.unwrap()); } + #[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. + let (path, _d) = temp_config(concat!( + "[censorship]\ntls_domain = \"a\"\n", + "[server]\nport = 443\n[server.api]\nauth_header = \"SECRET\"\n", + "[network]\nipv4 = \"1.2.3.4\"\n", + "[access.users]\nbob = \"deadbeef\"\n", + )); + 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("network")); // no per-node identity leak + assert!(!table.contains_key("access")); // no users/secrets + } + #[tokio::test] async fn patch_rejects_server_section() { let (path, _d) = temp_config("[censorship]\ntls_domain = \"a\"\n"); diff --git a/src/api/config_store.rs b/src/api/config_store.rs index 233fb17..e0ca68d 100644 --- a/src/api/config_store.rs +++ b/src/api/config_store.rs @@ -354,14 +354,16 @@ fn find_toml_table_bounds(source: &str, table_name: &str) -> Option<(usize, usiz let mut start = None; for line in source.split_inclusive('\n') { - let trimmed = line.trim(); + // Drop any inline comment so a hand-edited header like + // `[censorship] # note` still matches. Section names never contain `#`. + let header = line.trim().split('#').next().unwrap_or("").trim(); if let Some(start_offset) = start { - let is_same_array = trimmed == array; - let is_new_header = trimmed.starts_with('['); + let is_same_array = header == array; + let is_new_header = header.starts_with('['); if is_new_header && !is_same_array { return Some((start_offset, offset)); } - } else if trimmed == single || trimmed == array { + } else if header == single || header == array { start = Some(offset); } offset = offset.saturating_add(line.len()); @@ -453,6 +455,18 @@ mod tests { assert!(slice.contains("kind = \"b\"")); // spans through the last upstream block } + #[test] + fn find_bounds_matches_header_with_inline_comment() { + let src = "[censorship] # notes\ntls_domain = \"a\"\n\n[server]\nport = 1\n"; + let bounds = find_toml_table_bounds(src, "censorship"); + assert!(bounds.is_some(), "commented header must still match"); + let (start, end) = bounds.unwrap(); + let slice = &src[start..end]; + assert!(slice.starts_with("[censorship] # notes")); + assert!(slice.contains("tls_domain")); + assert!(!slice.contains("[server]")); // terminates at the next header + } + #[test] fn render_user_rate_limits_section() { let mut cfg = ProxyConfig::default();