alkdev/reverse-proxy
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add mtime TOCTOU check and wildcard flag to ConfigReloadHandle (ADR-029/030)

Browse Source 
Extract shared read_and_validate_config() with before/after mtime check
to detect mid-write config file changes. Add ReloadError enum with
FileChangedDuringRead variant. Return HTTP 409 Conflict on mtime change
from admin reload endpoint. Store cli_allow_wildcard_bind in
ConfigReloadHandle and use it in reload() validation instead of hardcoded
false. Update all ConfigReloadHandle::new() call sites.
This commit is contained in:
glm-5.1
2026-06-15 06:13:48 +00:00
parent 3ea3f56de7
commit c6dda716f4
7 changed files with 335 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
src/admin/handler.rs
View File

@@ -40,10 +40,26 @@ pub struct RotateKeyResponse {
}
pub async fn reload_handler(State(state): State<Arc<AdminState>>) -> impl IntoResponse {
let config_content = match tokio::fs::read_to_string(&state.config_path).await {
Ok(content) => content,
let result = crate::config::read_and_validate_config(
&state.config_path,
state.reload_handle.cli_allow_wildcard_bind(),
)
.await;
let (new_static, new_dynamic) = match result {
Ok(configs) => configs,
Err(crate::config::ReloadError::FileChangedDuringRead) => {
tracing::warn!("admin reload: config file changed during read");
return (
StatusCode::CONFLICT,
Json(ReloadResponse {
status: "error",
message: Some("config file changed during read, please retry".to_string()),
}),
);
}
Err(e) => {
tracing::error!("admin reload: failed to read config file: {}", e);
tracing::error!("admin reload: config read/validate failed: {}", e);
return (
StatusCode::INTERNAL_SERVER_ERROR,
Json(ReloadResponse {
@@ -54,22 +70,6 @@ pub async fn reload_handler(State(state): State<Arc<AdminState>>) -> impl IntoRe
}
};
let full_config = match crate::config::FullConfig::parse(&config_content) {
Ok(c) => c,
Err(e) => {
tracing::error!("admin reload: failed to parse config file: {}", e);
return (
StatusCode::INTERNAL_SERVER_ERROR,
Json(ReloadResponse {
status: "error",
message: Some("reload failed".to_string()),
}),
);
}
};
let (new_static, new_dynamic) = full_config.into_static_and_dynamic();
match state.reload_handle.reload(new_static, new_dynamic).await {
Ok(changed_fields) => {
if !changed_fields.is_empty() {

24
src/config/dynamic_config.rs
View File

@@ -112,14 +112,20 @@ pub struct ConfigReloadHandle {
config: Arc<ArcSwap<DynamicConfig>>,
static_config: ArcSwap<StaticConfig>,
reload_mutex: Mutex<()>,
cli_allow_wildcard_bind: bool,
}
impl ConfigReloadHandle {
pub fn new(config: Arc<ArcSwap<DynamicConfig>>, static_config: StaticConfig) -> Self {
pub fn new(
config: Arc<ArcSwap<DynamicConfig>>,
static_config: StaticConfig,
cli_allow_wildcard_bind: bool,
) -> Self {
Self {
config,
static_config: ArcSwap::from_pointee(static_config),
reload_mutex: Mutex::new(()),
cli_allow_wildcard_bind,
}
}
@@ -131,6 +137,10 @@ impl ConfigReloadHandle {
self.static_config.load_full()
}
pub fn cli_allow_wildcard_bind(&self) -> bool {
self.cli_allow_wildcard_bind
}
pub async fn reload(
&self,
new_static: StaticConfig,
@@ -138,7 +148,7 @@ impl ConfigReloadHandle {
) -> anyhow::Result<Vec<String>> {
let _guard = self.reload_mutex.lock().await;
validate(&new_static, &new_dynamic, false).map_err(|errors| {
validate(&new_static, &new_dynamic, self.cli_allow_wildcard_bind).map_err(|errors| {
anyhow::anyhow!(
"{}",
errors
@@ -193,7 +203,7 @@ mod tests {
let initial = test_fixtures::test_dynamic_config();
let config_arc = Arc::new(ArcSwap::from_pointee(initial.clone()));
let static_config = test_fixtures::test_static_config();
let handle = ConfigReloadHandle::new(config_arc.clone(), static_config);
let handle = ConfigReloadHandle::new(config_arc.clone(), static_config, false);
let loaded = handle.load();
assert_eq!(loaded.sites.len(), 1);
@@ -231,7 +241,7 @@ mod tests {
let initial = test_fixtures::test_dynamic_config();
let config_arc = Arc::new(ArcSwap::from_pointee(initial.clone()));
let static_config = test_fixtures::test_static_config();
let handle = ConfigReloadHandle::new(config_arc.clone(), static_config);
let handle = ConfigReloadHandle::new(config_arc.clone(), static_config, false);
let mut invalid_dynamic = initial.clone();
invalid_dynamic.rate_limit.requests_per_second = 0;
@@ -250,7 +260,7 @@ mod tests {
let initial = test_fixtures::test_dynamic_config();
let config_arc = Arc::new(ArcSwap::from_pointee(initial.clone()));
let static_config = test_fixtures::test_static_config();
let handle = Arc::new(ConfigReloadHandle::new(config_arc.clone(), static_config));
let handle = Arc::new(ConfigReloadHandle::new(config_arc.clone(), static_config, false));
let mut handles = Vec::new();
for i in 1..=5u32 {
@@ -299,7 +309,7 @@ mod tests {
let initial = test_fixtures::test_dynamic_config();
let config_arc = Arc::new(ArcSwap::from_pointee(initial.clone()));
let original_static = test_fixtures::test_static_config();
let handle = ConfigReloadHandle::new(config_arc.clone(), original_static.clone());
let handle = ConfigReloadHandle::new(config_arc.clone(), original_static.clone(), false);
let mut changed_static = original_static.clone();
changed_static.health_check_port = 8080;
@@ -322,7 +332,7 @@ mod tests {
let initial = test_fixtures::test_dynamic_config();
let config_arc = Arc::new(ArcSwap::from_pointee(initial.clone()));
let original_static = test_fixtures::test_static_config();
let handle = ConfigReloadHandle::new(config_arc.clone(), original_static.clone());
let handle = ConfigReloadHandle::new(config_arc.clone(), original_static.clone(), false);
let mut changed_static = original_static.clone();
changed_static.health_check_port = 8080;

282
src/config/mod.rs
View File

@@ -11,6 +11,31 @@ pub use static_config::{ListenerConfig, LoggingConfig, StaticConfig, TlsConfig};
pub use validation::{validate, ValidationError};
use serde::Deserialize;
use thiserror::Error;
#[derive(Debug, Error)]
pub enum ReloadError {
#[error("IO error reading config file: {0}")]
Io(#[from] std::io::Error),
#[error("Failed to parse config file: {0}")]
Parse(#[from] toml::de::Error),
#[error("Config validation failed: {0}")]
Validation(String),
#[error("config file changed during read, please retry")]
FileChangedDuringRead,
}
impl From<Vec<ValidationError>> for ReloadError {
fn from(errors: Vec<ValidationError>) -> Self {
ReloadError::Validation(
errors
.iter()
.map(|e| e.to_string())
.collect::<Vec<_>>()
.join("; "),
)
}
}
#[derive(Debug, Deserialize)]
pub struct FullConfig {
@@ -56,3 +81,260 @@ impl FullConfig {
(static_config, dynamic_config)
}
}
pub async fn read_and_validate_config(
config_path: &str,
cli_allow_wildcard_bind: bool,
) -> Result<(StaticConfig, DynamicConfig), ReloadError> {
let metadata_before = tokio::fs::metadata(config_path).await?;
let config_content = tokio::fs::read_to_string(config_path).await?;
let metadata_after = tokio::fs::metadata(config_path).await?;
if metadata_before.modified().ok() != metadata_after.modified().ok() {
tracing::warn!(
event = "CONFIG_RELOAD",
status = "rejected",
reason = "file_mtime_changed",
path = config_path
);
return Err(ReloadError::FileChangedDuringRead);
}
let full_config: FullConfig = toml::from_str(&config_content)?;
let (new_static, new_dynamic) = full_config.into_static_and_dynamic();
validate(&new_static, &new_dynamic, cli_allow_wildcard_bind)?;
Ok((new_static, new_dynamic))
}
#[cfg(test)]
mod tests {
use super::*;
use std::io::Write;
fn valid_config_toml() -> &'static str {
r#"
health_check_port = 9900
admin_key_path = "/tmp/test-admin-key"
[logging]
level = "info"
format = "text"
[rate_limit]
requests_per_second = 20
burst = 40
[body]
limit_bytes = 104857600
[[listeners]]
bind_addr = "127.0.0.1"
http_port = 80
https_port = 443
[listeners.tls]
mode = "acme"
acme_domains = ["test.local"]
acme_cache_dir = "/tmp/acme-cache"
acme_directory = "staging"
acme_contact = "mailto:admin@test.local"
[[listeners.sites]]
host = "test.local"
upstream = "127.0.0.1:8080"
"#
}
#[tokio::test]
async fn read_and_validate_config_valid_file() {
let dir = tempfile::tempdir().unwrap();
let config_path = dir.path().join("config.toml");
std::fs::write(&config_path, valid_config_toml()).unwrap();
let result = read_and_validate_config(config_path.to_str().unwrap(), false).await;
assert!(result.is_ok());
let (static_config, dynamic_config) = result.unwrap();
assert_eq!(dynamic_config.rate_limit.requests_per_second, 20);
assert_eq!(static_config.health_check_port, 9900);
}
#[tokio::test]
async fn read_and_validate_config_missing_file() {
let result = read_and_validate_config("/nonexistent/config.toml", false).await;
assert!(result.is_err());
match result.unwrap_err() {
ReloadError::Io(_) => {}
e => panic!("expected Io error, got: {:?}", e),
}
}
#[tokio::test]
async fn read_and_validate_config_invalid_toml() {
let dir = tempfile::tempdir().unwrap();
let config_path = dir.path().join("config.toml");
std::fs::write(&config_path, "invalid toml {{{").unwrap();
let result = read_and_validate_config(config_path.to_str().unwrap(), false).await;
assert!(result.is_err());
match result.unwrap_err() {
ReloadError::Parse(_) => {}
e => panic!("expected Parse error, got: {:?}", e),
}
}
#[tokio::test]
async fn read_and_validate_config_validation_fails() {
let dir = tempfile::tempdir().unwrap();
let config_path = dir.path().join("config.toml");
let config_content = r#"
health_check_port = 9900
[logging]
level = "info"
format = "text"
[rate_limit]
requests_per_second = 0
burst = 20
[body]
limit_bytes = 104857600
[[listeners]]
bind_addr = "127.0.0.1"
http_port = 80
https_port = 443
[listeners.tls]
mode = "acme"
acme_domains = ["test.local"]
acme_cache_dir = "/tmp/acme-cache"
acme_contact = "mailto:admin@test.local"
"#;
std::fs::write(&config_path, config_content).unwrap();
let result = read_and_validate_config(config_path.to_str().unwrap(), false).await;
assert!(result.is_err());
match result.unwrap_err() {
ReloadError::Validation(msg) => {
assert!(msg.contains("requests_per_second"));
}
e => panic!("expected Validation error, got: {:?}", e),
}
}
#[tokio::test]
async fn read_and_validate_config_wildcard_bind_allowed_with_flag() {
let dir = tempfile::tempdir().unwrap();
let config_path = dir.path().join("config.toml");
let config_content = r#"
health_check_port = 9900
[logging]
level = "info"
format = "text"
[rate_limit]
requests_per_second = 10
burst = 20
[body]
limit_bytes = 104857600
[[listeners]]
bind_addr = "0.0.0.0"
http_port = 80
https_port = 443
[listeners.tls]
mode = "acme"
acme_domains = ["test.local"]
acme_cache_dir = "/tmp/acme-cache"
acme_contact = "mailto:admin@test.local"
[[listeners.sites]]
host = "test.local"
upstream = "127.0.0.1:8080"
"#;
std::fs::write(&config_path, config_content).unwrap();
let result = read_and_validate_config(config_path.to_str().unwrap(), true).await;
assert!(result.is_ok());
}
#[tokio::test]
async fn read_and_validate_config_wildcard_bind_rejected_without_flag() {
let dir = tempfile::tempdir().unwrap();
let config_path = dir.path().join("config.toml");
let config_content = r#"
health_check_port = 9900
[logging]
level = "info"
format = "text"
[rate_limit]
requests_per_second = 10
burst = 20
[body]
limit_bytes = 104857600
[[listeners]]
bind_addr = "0.0.0.0"
http_port = 80
https_port = 443
[listeners.tls]
mode = "acme"
acme_domains = ["test.local"]
acme_cache_dir = "/tmp/acme-cache"
acme_contact = "mailto:admin@test.local"
[[listeners.sites]]
host = "test.local"
upstream = "127.0.0.1:8080"
"#;
std::fs::write(&config_path, config_content).unwrap();
let result = read_and_validate_config(config_path.to_str().unwrap(), false).await;
assert!(result.is_err());
match result.unwrap_err() {
ReloadError::Validation(msg) => {
assert!(msg.contains("0.0.0.0"));
}
e => panic!("expected Validation error, got: {:?}", e),
}
}
#[tokio::test]
async fn read_and_validate_config_mtime_change_detected() {
let dir = tempfile::tempdir().unwrap();
let config_path = dir.path().join("config.toml");
std::fs::write(&config_path, valid_config_toml()).unwrap();
let path = config_path.clone();
let handle = tokio::spawn(async move {
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
let mut file = std::fs::OpenOptions::new()
.write(true)
.truncate(true)
.open(&path)
.unwrap();
file.write_all(valid_config_toml().as_bytes()).unwrap();
file.sync_all().unwrap();
});
let result = read_and_validate_config(config_path.to_str().unwrap(), false).await;
handle.await.unwrap();
match result {
Err(ReloadError::FileChangedDuringRead) => {}
Ok(_) => {}
Err(e) => panic!("expected FileChangedDuringRead or Ok, got: {:?}", e),
}
}
}

4
src/health.rs
View File

@@ -84,7 +84,7 @@ mod tests {
test_fixtures::test_dynamic_config(),
));
let static_config = test_fixtures::test_static_config();
let reload_handle = Arc::new(ConfigReloadHandle::new(config_arc, static_config));
let reload_handle = Arc::new(ConfigReloadHandle::new(config_arc, static_config, false));
let admin_state = Arc::new(AdminState {
reload_handle,
@@ -169,7 +169,7 @@ upstream = "127.0.0.1:8080"
test_fixtures::test_dynamic_config(),
));
let static_config = test_fixtures::test_static_config();
let reload_handle = Arc::new(ConfigReloadHandle::new(config_arc, static_config));
let reload_handle = Arc::new(ConfigReloadHandle::new(config_arc, static_config, false));
let admin_state = Arc::new(AdminState {
reload_handle,

1
src/main.rs
View File

@@ -85,6 +85,7 @@ async fn run_server(loaded_config: cli::LoadedConfig, config_path: &str) -> Resu
let reload_handle = Arc::new(ConfigReloadHandle::new(
config_arc.clone(),
loaded_config.static_config.clone(),
loaded_config.allow_wildcard_bind,
));
reverse_proxy::shutdown::register_signal_handlers(

23
src/shutdown.rs
View File

@@ -85,24 +85,20 @@ pub async fn handle_sighup_reload(
reload_handle: &Arc<crate::config::ConfigReloadHandle>,
config_path: &str,
) {
let config_content = match tokio::fs::read_to_string(config_path).await {
Ok(content) => content,
let result = crate::config::read_and_validate_config(
config_path,
reload_handle.cli_allow_wildcard_bind(),
)
.await;
let (new_static, new_dynamic) = match result {
Ok(configs) => configs,
Err(e) => {
tracing::error!(event = "CONFIG_RELOAD", status = "error", error = %e);
return;
}
};
let full_config = match crate::config::FullConfig::parse(&config_content) {
Ok(c) => c,
Err(e) => {
tracing::error!(event = "CONFIG_RELOAD", status = "error", error = %e);
return;
}
};
let (new_static, new_dynamic) = full_config.into_static_and_dynamic();
match reload_handle.reload(new_static, new_dynamic).await {
Ok(changed_fields) => {
if !changed_fields.is_empty() {
@@ -170,6 +166,7 @@ mod tests {
let reload_handle = Arc::new(crate::config::ConfigReloadHandle::new(
config_arc.clone(),
static_config,
false,
));
let dir = tempfile::tempdir().unwrap();
@@ -227,6 +224,7 @@ upstream = "127.0.0.1:8080"
let reload_handle = Arc::new(crate::config::ConfigReloadHandle::new(
config_arc.clone(),
static_config,
false,
));
let dir = tempfile::tempdir().unwrap();
@@ -253,6 +251,7 @@ upstream = "127.0.0.1:8080"
let reload_handle = Arc::new(crate::config::ConfigReloadHandle::new(
config_arc.clone(),
static_config,
false,
));
handle_sighup_reload(&reload_handle, "/nonexistent/config.toml").await;

4
tests/integration_test.rs
View File

@@ -865,6 +865,7 @@ async fn test_sighup_config_reload_valid_config() {
let reload_handle = Arc::new(reverse_proxy::config::ConfigReloadHandle::new(
config_arc.clone(),
static_config,
false,
));
let dir = tempfile::tempdir().unwrap();
@@ -892,8 +893,8 @@ https_port = 443
mode = "acme"
acme_domains = ["test.local"]
acme_cache_dir = "/tmp/acme-cache"
acme_contact = "mailto:admin@test.local"
acme_directory = "staging"
acme_contact = "mailto:admin@test.local"
[[listeners.sites]]
host = "test.local"
@@ -921,6 +922,7 @@ async fn test_sighup_config_reload_invalid_config_keeps_old() {
let reload_handle = Arc::new(reverse_proxy::config::ConfigReloadHandle::new(
config_arc.clone(),
static_config,
false,
));
let dir = tempfile::tempdir().unwrap();