Audit and harden project code

.gitignore

@@ -1,5 +1,7 @@
 /target
 /dist
+__pycache__/
+*.pyc
 
 .claude
 .tmp*
@@ -8,4 +10,4 @@ _*.md
 # Packaging artifacts
 *.deb
 *.rpm
-gatel_*_*/
+gatel_*_*/

_improve.md

@@ -0,0 +1,64 @@
+# Gatel Code Improvement Report
+
+审查基线：`main` 最新提交，新分支 `chris/project-improve-audit`。审查范围覆盖 Rust workspace、核心代理/认证/配置解析代码、示例配置、打包脚本和文档。基础验证结果：`cargo check --workspace` 通过，`cargo clippy --workspace --all-targets -- -D warnings` 通过，`cargo test --workspace` 通过。
+
+## 已完成
+
+- [x] 自动生成 KDL 配置时缺少字符串转义。
+  - 位置：`crates/core/src/config/parse.rs::auto_config_from_env`、`crates/gatel/src/main.rs` 的 `serve` 子命令。
+  - 风险：环境变量或 CLI path 中的 `"`、换行等字符会破坏生成的 KDL，严重时可插入额外配置指令。
+  - 改进：新增 `kdl_string` 统一生成 KDL 字符串字面量，并用于环境变量配置和 `serve` 命令合成配置。
+
+- [x] Basic auth / forward proxy auth 的 Base64 解码过于宽松。
+  - 位置：`crates/core/src/encoding.rs`、`crates/core/src/proxy/forward_proxy.rs`。
+  - 风险：旧实现遇到 `=` 后直接停止，`SGVsbG8=bad` 这类带尾随垃圾的数据仍可能被接受；forward proxy 还维护了一份重复解码器。
+  - 改进：严格拒绝 padding 后追加数据、非法 padding 长度和非 Base64 字符；forward proxy 复用核心解码器，避免两套认证解析行为漂移。
+
+- [x] `encoding` 模块使用模块级 `#![allow(dead_code)]` 掩盖未使用代码。
+  - 位置：`crates/core/src/encoding.rs`。
+  - 风险：模块级豁免会让后续真正的死代码继续累积。
+  - 改进：删除未接入的 `percent_encode` 和 `html_escape`，移除模块级 `dead_code` 豁免。
+
+## 第二轮已完成
+
+- [x] `gatel reload --address` 手动指定 admin 地址时不会自动带 token。
+  - 位置：`crates/gatel/src/main.rs::Commands::Reload`。
+  - 影响：使用 `--address` 时即使配置文件中有 `admin-auth-token`/`admin-write-token`，请求也不会带认证头；这可能导致安全配置下 reload 命令意外失败。
+  - 改进：新增 `reload_target`，保留 `--address` 覆盖地址，同时仍从配置读取 `admin-auth-token` / `admin-write-token`。
+
+- [x] `reload_config` 在 Windows 构建路径下被 `#[allow(dead_code)]` 保留。
+  - 位置：`crates/gatel/src/main.rs::reload_config`。
+  - 影响：该函数仅 Unix SIGHUP 路径使用，Windows 上属于条件编译导致的死代码。
+  - 改进：用 `#[cfg(unix)]` 约束热重载函数，用 `cfg_attr` 只在非 Unix 下允许 signal handler 参数未使用。
+
+- [x] FastCGI 记录头读取后没有校验版本号和 request id。
+  - 位置：`crates/core/src/proxy/fastcgi.rs::read_record_header` / response loop。
+  - 影响：当前会忽略 `version`、`request_id` 字段，异常或串扰的 FastCGI 响应不会被明确拒绝。
+  - 改进：新增记录头解析校验，拒绝非 FastCGI v1 或非当前 request id 的响应，并补充单元测试。
+
+- [x] FastCGI `FCGI_ABORT_REQUEST` 常量未接入。
+  - 位置：`crates/core/src/proxy/fastcgi.rs`。
+  - 影响：常量目前只靠局部 `#[allow(dead_code)]` 保留，实际取消请求时不会向 FastCGI 后端发送 abort。
+  - 改进：删除未接入常量和对应 `dead_code` 豁免，避免暗示已支持取消传播。
+
+- [x] 管理 API 支持 loopback 无 token 访问，文档已提示但默认安全边界依赖部署者。
+  - 位置：`crates/core/src/admin/mod.rs`、`crates/core/src/server/mod.rs`。
+  - 影响：loopback 监听在容器、端口转发或 sidecar 环境中可能不等同于“只有本机可信用户可访问”。
+  - 改进：保留兼容行为，但在 loopback 且未配置 bearer token 时增加启动警告，提示生产和转发/容器化场景配置 token。
+
+- [x] `rustfmt.toml` 包含多项 nightly-only 配置。
+  - 位置：`rustfmt.toml`。
+  - 影响：stable rustfmt 每次都会输出 warnings；但这些配置是项目选择 nightly rustfmt 时才生效的格式策略。
+  - 改进：保留原配置，不作为代码改动处理；验证时如需无 warning 应使用 `cargo +nightly fmt`。
+
+- [x] `benchmarks/http-servers/__pycache__` 被纳入版本控制。
+  - 位置：`benchmarks/http-servers/__pycache__/run.cpython-314.pyc`。
+  - 影响：生成物进入仓库，增加无意义 diff，也可能造成跨 Python 版本噪声。
+  - 改进：删除已跟踪 pyc，并在 `.gitignore` 增加 `__pycache__/` / `*.pyc`。
+
+## 本轮验证
+
+- [x] `cargo check --workspace`
+- [x] `cargo test --workspace`
+- [x] `cargo fmt --all -- --check`
+- [x] `cargo clippy --workspace --all-targets -- -D warnings`

crates/core/src/config/mod.rs

@@ -1,5 +1,5 @@
 mod parse;
 mod types;
 
-pub use parse::{ConfigError, auto_config_from_env, parse_config, parse_config_file};
+pub use parse::{ConfigError, auto_config_from_env, kdl_string, parse_config, parse_config_file};
 pub use types::*;

crates/core/src/config/parse.rs

@@ -1,4 +1,5 @@
 use std::collections::{HashMap, HashSet};
+use std::fmt::Write as _;
 use std::net::SocketAddr;
 use std::path::{Path, PathBuf};
 use std::time::Duration;
@@ -324,48 +325,69 @@ pub fn auto_config_from_env() -> Option<String> {
     // Global block.
     out.push_str("global {\n");
     if let Some(addr) = &http_addr {
-        out.push_str(&format!("    http \"{addr}\"\n"));
+        out.push_str(&format!("    http {}\n", kdl_string(addr)));
     }
     if let Some(addr) = &https_addr {
-        out.push_str(&format!("    https \"{addr}\"\n"));
+        out.push_str(&format!("    https {}\n", kdl_string(addr)));
     }
     if let Some(addr) = &admin_addr {
-        out.push_str(&format!("    admin \"{addr}\"\n"));
+        out.push_str(&format!("    admin {}\n", kdl_string(addr)));
     }
     if let Some(token) = &admin_auth_token {
-        out.push_str(&format!("    admin-auth-token \"{token}\"\n"));
+        out.push_str(&format!("    admin-auth-token {}\n", kdl_string(token)));
     }
     if let Some(token) = &admin_read_token {
-        out.push_str(&format!("    admin-read-token \"{token}\"\n"));
+        out.push_str(&format!("    admin-read-token {}\n", kdl_string(token)));
     }
     if let Some(token) = &admin_write_token {
-        out.push_str(&format!("    admin-write-token \"{token}\"\n"));
+        out.push_str(&format!("    admin-write-token {}\n", kdl_string(token)));
     }
     out.push_str("}\n");
 
     // TLS / ACME block (only when an email is provided).
     if let Some(email) = &acme_email {
         out.push_str("tls {\n");
         out.push_str("    acme {\n");
-        out.push_str(&format!("        email \"{email}\"\n"));
-        out.push_str(&format!("        ca \"{acme_ca}\"\n"));
+        out.push_str(&format!("        email {}\n", kdl_string(email)));
+        out.push_str(&format!("        ca {}\n", kdl_string(&acme_ca)));
         out.push_str("    }\n");
         out.push_str("}\n");
     }
 
     // Site / proxy block (only when an upstream is provided).
     if let Some(upstream_addr) = &upstream {
         let site_host = host.as_deref().unwrap_or("*");
-        out.push_str(&format!("site \"{site_host}\" {{\n"));
+        out.push_str(&format!("site {} {{\n", kdl_string(site_host)));
         out.push_str("    route \"/*\" {\n");
-        out.push_str(&format!("        proxy \"{upstream_addr}\"\n"));
+        out.push_str(&format!("        proxy {}\n", kdl_string(upstream_addr)));
         out.push_str("    }\n");
         out.push_str("}\n");
     }
 
     Some(out)
 }
 
+/// Quote a string for use as a KDL string literal.
+pub fn kdl_string(value: &str) -> String {
+    let mut out = String::with_capacity(value.len() + 2);
+    out.push('"');
+    for ch in value.chars() {
+        match ch {
+            '"' => out.push_str("\\\""),
+            '\\' => out.push_str("\\\\"),
+            '\n' => out.push_str("\\n"),
+            '\r' => out.push_str("\\r"),
+            '\t' => out.push_str("\\t"),
+            ch if ch.is_control() => {
+                write!(out, "\\u{{{:x}}}", ch as u32).expect("writing to a string cannot fail")
+            }
+            ch => out.push(ch),
+        }
+    }
+    out.push('"');
+    out
+}
+
 // ---------------------------------------------------------------------------
 // Global
 // ---------------------------------------------------------------------------
@@ -2372,6 +2394,25 @@ site "api.example.com" {
         );
     }
 
+    #[test]
+    fn kdl_string_escapes_quotes_and_control_chars() {
+        let input = format!(
+            "site {} {{ route \"/*\" {{ proxy {} }} }}",
+            kdl_string("app\"demo\n\u{1b}.example.com"),
+            kdl_string("http://127.0.0.1:3000/a\"b")
+        );
+        assert!(input.contains("\\u{1b}"));
+        let config = parse_config(&input).unwrap();
+
+        assert_eq!(config.sites[0].host, "app\"demo\n\u{1b}.example.com");
+        match &config.sites[0].routes[0].handler {
+            HandlerConfig::Proxy(proxy) => {
+                assert_eq!(proxy.upstreams[0].addr, "http://127.0.0.1:3000/a\"b");
+            }
+            _ => panic!("expected proxy handler"),
+        }
+    }
+
     #[test]
     fn test_parse_fastcgi_config() {
         let input = r#"

crates/core/src/encoding.rs

@@ -1,5 +1,4 @@
 //! Encoding and decoding utilities.
-#![allow(dead_code)]
 
 const BASE64_TABLE: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
 
@@ -23,54 +22,37 @@ pub fn percent_decode(input: &str) -> String {
     String::from_utf8(result).unwrap_or_else(|_| input.to_string())
 }
 
-/// Percent-encode a string for use in a URI component.
-pub fn percent_encode(input: &str) -> String {
-    let mut result = String::with_capacity(input.len());
-    for byte in input.bytes() {
-        match byte {
-            b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => {
-                result.push(byte as char);
-            }
-            _ => result.push_str(&format!("%{byte:02X}")),
-        }
-    }
-    result
-}
-
-/// Escape special HTML characters to prevent XSS.
-pub fn html_escape(input: &str) -> String {
-    input
-        .replace('&', "&")
-        .replace('<', "&lt;")
-        .replace('>', "&gt;")
-        .replace('"', "&quot;")
-        .replace('\'', "&#39;")
-}
-
 /// Decode a standard Base64 string.
 pub fn base64_decode(input: &str) -> Option<Vec<u8>> {
     let input = input.trim();
     if input.is_empty() {
         return Some(Vec::new());
     }
 
+    let mut seen_padding = false;
+    let mut padding = 0usize;
+    let mut data_len = 0usize;
     let mut output = Vec::with_capacity(input.len() * 3 / 4);
     let mut buffer = 0u32;
     let mut bits = 0u32;
 
     for &byte in input.as_bytes() {
         if byte == b'=' {
-            break;
+            seen_padding = true;
+            padding += 1;
+            if padding > 2 {
+                return None;
+            }
+            continue;
+        }
+        if seen_padding {
+            return None;
         }
         let value = match BASE64_TABLE.iter().position(|&candidate| candidate == byte) {
             Some(value) => value as u32,
-            None => {
-                if byte == b'\n' || byte == b'\r' || byte == b' ' {
-                    continue;
-                }
-                return None;
-            }
+            None => return None,
         };
+        data_len += 1;
         buffer = (buffer << 6) | value;
         bits += 6;
         if bits >= 8 {
@@ -80,6 +62,11 @@ pub fn base64_decode(input: &str) -> Option<Vec<u8>> {
         }
     }
 
+    let total_len = data_len + padding;
+    if total_len % 4 == 1 || (padding > 0 && !total_len.is_multiple_of(4)) {
+        return None;
+    }
+
     Some(output)
 }
 
@@ -99,19 +86,21 @@ mod tests {
     #[test]
     fn percent_round_trip() {
         assert_eq!(percent_decode("%2F"), "/");
-        assert_eq!(percent_encode("hello world"), "hello%20world");
     }
 
     #[test]
-    fn html_escaping() {
-        assert_eq!(
-            html_escape("<script>alert('xss')</script>"),
-            "&lt;script&gt;alert(&#39;xss&#39;)&lt;/script&gt;"
-        );
+    fn base64_decoding() {
+        assert_eq!(base64_decode("SGVsbG8="), Some(b"Hello".to_vec()));
     }
 
     #[test]
-    fn base64_decoding() {
-        assert_eq!(base64_decode("SGVsbG8="), Some(b"Hello".to_vec()));
+    fn base64_rejects_trailing_data_after_padding() {
+        assert_eq!(base64_decode("SGVsbG8=bad"), None);
+    }
+
+    #[test]
+    fn base64_rejects_invalid_padding_length() {
+        assert_eq!(base64_decode("A="), None);
+        assert_eq!(base64_decode("A==="), None);
     }
 }

crates/core/src/proxy/fastcgi.rs

@@ -19,8 +19,6 @@ use crate::{Body, ProxyError, full_body, goals};
 
 const FCGI_VERSION_1: u8 = 1;
 const FCGI_BEGIN_REQUEST: u8 = 1;
-#[allow(dead_code)]
-const FCGI_ABORT_REQUEST: u8 = 2;
 const FCGI_END_REQUEST: u8 = 3;
 const FCGI_PARAMS: u8 = 4;
 const FCGI_STDIN: u8 = 5;
@@ -390,11 +388,7 @@ async fn send_stream_records(
 // ---------------------------------------------------------------------------
 
 struct RecordHeader {
-    #[allow(dead_code)]
-    version: u8,
     record_type: u8,
-    #[allow(dead_code)]
-    request_id: u16,
     content_length: u16,
     padding_length: u8,
 }
@@ -406,10 +400,26 @@ async fn read_record_header(stream: &mut TcpStream) -> Result<RecordHeader, Prox
         .await
         .map_err(|e| ProxyError::Internal(format!("failed to read FastCGI record header: {e}")))?;
 
+    parse_record_header(buf)
+}
+
+fn parse_record_header(buf: [u8; FCGI_HEADER_LEN]) -> Result<RecordHeader, ProxyError> {
+    let version = buf[0];
+    let request_id = u16::from_be_bytes([buf[2], buf[3]]);
+
+    if version != FCGI_VERSION_1 {
+        return Err(ProxyError::Internal(format!(
+            "invalid FastCGI record version: {version}"
+        )));
+    }
+    if request_id != FCGI_REQUEST_ID {
+        return Err(ProxyError::Internal(format!(
+            "unexpected FastCGI request id: {request_id}"
+        )));
+    }
+
     Ok(RecordHeader {
-        version: buf[0],
         record_type: buf[1],
-        request_id: u16::from_be_bytes([buf[2], buf[3]]),
         content_length: u16::from_be_bytes([buf[4], buf[5]]),
         padding_length: buf[6],
     })
@@ -590,6 +600,24 @@ mod tests {
         assert_eq!(record.len(), FCGI_HEADER_LEN + 8); // 8 content, 0 padding
     }
 
+    #[test]
+    fn parse_record_header_rejects_wrong_version() {
+        let mut buf = [0u8; FCGI_HEADER_LEN];
+        buf[0] = 2;
+        buf[2..4].copy_from_slice(&FCGI_REQUEST_ID.to_be_bytes());
+
+        assert!(parse_record_header(buf).is_err());
+    }
+
+    #[test]
+    fn parse_record_header_rejects_wrong_request_id() {
+        let mut buf = [0u8; FCGI_HEADER_LEN];
+        buf[0] = FCGI_VERSION_1;
+        buf[2..4].copy_from_slice(&2u16.to_be_bytes());
+
+        assert!(parse_record_header(buf).is_err());
+    }
+
     #[test]
     fn test_encode_length_short() {
         let mut buf = BytesMut::new();