Skip to content

Commit 1988008

Browse files
author
Paul C
committed
v0.4.11: surface include failures + clippy-clean with CI gate
WolfProxy silently swallowed include problems: a glob that matched no files (e.g. Debian's extensionless sites-enabled entries vs `*.conf`) or an explicit `include` of a missing file just produced an empty config, and `wolfproxy --test` printed "configuration OK" regardless. Real nginx errors loudly here, so the operator had no signal why their sites weren't served (wabil report). - NginxConfig gains include_warnings: Vec<String>, populated for a zero-match glob, a missing explicit include, or an invalid glob. merge_configs propagates them up nested include chains. - `wolfproxy --test` now prints each include warning (still exit 0: a missing include never takes the proxy down, and failing would make WolfStack's reload-gate refuse to reload a node with a pre-existing broken include). The server-block count remains the primary signal. - The live loader logs the same warnings via warn! on startup/reload. - Server-context include failures (snippets) now warn instead of being dropped silently. Also clears all clippy warnings at root (manual_strip -> strip_prefix/ strip_suffix, map_entry, while_let -> for, let_and_return, unnecessary_map_or; one documented allow for the 8-arg request handler) and adds a CI lint gate (cargo clippy --all-targets -- -D warnings + tests) that blocks the release build.
1 parent 2a2a6b1 commit 1988008

6 files changed

Lines changed: 177 additions & 40 deletions

File tree

.github/workflows/release.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,24 @@ on:
2020
workflow_dispatch:
2121

2222
jobs:
23+
# Hard gate: clippy with warnings-as-errors + the test suite must pass
24+
# before any release binary is built. Keeps the tree clippy-clean (the
25+
# wolfstack repo enforces the same bar). Fix at root — no bare #[allow].
26+
lint:
27+
runs-on: ubuntu-latest
28+
steps:
29+
- uses: actions/checkout@v5
30+
- name: Install Rust
31+
uses: dtolnay/rust-toolchain@stable
32+
with:
33+
components: clippy
34+
- name: Clippy (warnings = errors)
35+
run: cargo clippy --all-targets -- -D warnings
36+
- name: Tests
37+
run: cargo test --all-targets
38+
2339
build:
40+
needs: lint
2441
strategy:
2542
matrix:
2643
include:

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "wolfproxy"
3-
version = "0.4.10"
3+
version = "0.4.11"
44
edition = "2021"
55
description = "A Rust-based nginx proxy replacement that reads nginx configuration files"
66
authors = ["Wolf Software Systems Ltd"]

src/firewall.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,16 +150,13 @@ pub struct Firewall {
150150
#[allow(dead_code)]
151151
impl Firewall {
152152
pub fn new(config: FirewallConfig) -> Self {
153-
let fw = Self {
153+
Self {
154154
config,
155155
blocked: DashMap::new(),
156156
trackers: DashMap::new(),
157157
total_blocks: AtomicU64::new(0),
158158
total_denied: AtomicU64::new(0),
159-
};
160-
161-
// Spawn cleanup task
162-
fw
159+
}
163160
}
164161

165162
/// Check if an IP is blocked. Returns true if blocked.

src/main.rs

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,16 @@ async fn run_config_test(config_path: &str) -> i32 {
306306
nginx_config.upstreams.len(),
307307
config.nginx.config_dir,
308308
);
309+
// Report include problems so the operator sees that a directive pulled in
310+
// nothing — previously `--test` said "OK" no matter what the includes did,
311+
// which is exactly what left wabil with no signal (2026-06-13). These are
312+
// warnings, not hard failures: a missing include never takes the proxy
313+
// down (it just isn't loaded), and failing here would make WolfStack's
314+
// reload-gate refuse to reload a node that had a pre-existing broken
315+
// include. The server-block count above is the real signal.
316+
for w in &nginx_config.include_warnings {
317+
println!("wolfproxy: include warning — {w}");
318+
}
309319
0
310320
}
311321

@@ -461,10 +471,17 @@ password = "admin"
461471
// Load nginx configuration
462472
let nginx_config = nginx::load_nginx_config(Path::new(&config.nginx.config_dir));
463473

464-
info!("Loaded {} server blocks and {} upstreams",
465-
nginx_config.servers.len(),
474+
info!("Loaded {} server blocks and {} upstreams",
475+
nginx_config.servers.len(),
466476
nginx_config.upstreams.len());
467-
477+
478+
// Surface include problems loudly — a glob that matched nothing or a
479+
// missing include file otherwise produces a silently-empty config and an
480+
// operator with no idea why their site isn't served (wabil, 2026-06-13).
481+
for w in &nginx_config.include_warnings {
482+
warn!("nginx config: {}", w);
483+
}
484+
468485
// Build virtual hosts map
469486
let mut vhosts: HashMap<String, VirtualHost> = HashMap::new();
470487
let mut default_vhosts: HashMap<u16, VirtualHost> = HashMap::new();
@@ -1142,6 +1159,11 @@ fn is_ws_upgrade(headers: &HeaderMap) -> bool {
11421159
.unwrap_or(false)
11431160
}
11441161

1162+
// The 8 parameters are the full per-request proxy context (connection, parsed
1163+
// route, server-level headers, TLS flag). Bundling them into a struct purely
1164+
// to satisfy the lint would add indirection to the hot request path without
1165+
// making the call site clearer, so this one is allowed deliberately.
1166+
#[allow(clippy::too_many_arguments)]
11451167
async fn handle_proxy(
11461168
state: &Arc<AppState>,
11471169
mut req: Request,
@@ -1187,7 +1209,9 @@ async fn handle_proxy(
11871209
let path_suffix = req.uri().path_and_query().map(|pq| pq.to_string()).unwrap_or_default();
11881210

11891211
let target: ProxyTarget = if proxy_pass.starts_with("http://") || proxy_pass.starts_with("https://") {
1190-
let after_scheme = if proxy_pass.starts_with("https://") { &proxy_pass[8..] } else { &proxy_pass[7..] };
1212+
let after_scheme = proxy_pass.strip_prefix("https://")
1213+
.or_else(|| proxy_pass.strip_prefix("http://"))
1214+
.unwrap_or(proxy_pass);
11911215
let potential_upstream = after_scheme.split('/').next().unwrap_or(after_scheme);
11921216
let potential_upstream = potential_upstream.split(':').next().unwrap_or(potential_upstream);
11931217

@@ -1448,10 +1472,8 @@ async fn handle_proxy(
14481472
}
14491473
for ph in server_headers {
14501474
let key = ph.name.to_lowercase();
1451-
if !custom_headers.contains_key(&key) {
1452-
let value = expand_proxy_header_value(&ph.value, &headers, client_addr, is_https);
1453-
custom_headers.insert(key, value);
1454-
}
1475+
custom_headers.entry(key).or_insert_with(||
1476+
expand_proxy_header_value(&ph.value, &headers, client_addr, is_https));
14551477
}
14561478
if !custom_headers.contains_key("host") {
14571479
if let Some(host) = uri.host() {

src/monitoring.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,7 @@ pub struct MonitoringState {
113113
fn check_auth(headers: &HeaderMap, username: &str, password: &str) -> bool {
114114
if let Some(auth_header) = headers.get(header::AUTHORIZATION) {
115115
if let Ok(auth_str) = auth_header.to_str() {
116-
if auth_str.starts_with("Basic ") {
117-
let encoded = &auth_str[6..];
116+
if let Some(encoded) = auth_str.strip_prefix("Basic ") {
118117
if let Ok(decoded) = BASE64.decode(encoded) {
119118
if let Ok(credentials) = String::from_utf8(decoded) {
120119
let expected = format!("{}:{}", username, password);

src/nginx.rs

Lines changed: 126 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,15 @@ pub struct NginxConfig {
236236
pub upstreams: HashMap<String, Upstream>,
237237
pub servers: Vec<ServerBlock>,
238238
pub includes: Vec<PathBuf>,
239+
/// Human-readable problems found while resolving `include` directives:
240+
/// a glob that matched no files, an explicit include whose file is
241+
/// missing, or an invalid glob pattern. These are NOT part of the proxy's
242+
/// routing state — they exist purely so `wolfproxy --test` and the startup
243+
/// log can tell the operator their include pulled in nothing, instead of
244+
/// failing silently (the wabil report, 2026-06-13). Real nginx errors
245+
/// loudly here; WolfProxy used to swallow it.
246+
#[serde(default)]
247+
pub include_warnings: Vec<String>,
239248
}
240249

241250
/// Parse an nginx configuration file
@@ -275,15 +284,43 @@ pub fn parse_nginx_content(content: &str, base_dir: Option<&Path>) -> NginxConfi
275284

276285
// Handle glob patterns
277286
if include.contains('*') {
278-
if let Ok(paths) = glob::glob(&include_path.to_string_lossy()) {
279-
for entry in paths.flatten() {
280-
let sub_config = parse_nginx_config(&entry);
281-
merge_configs(&mut config, sub_config);
287+
match glob::glob(&include_path.to_string_lossy()) {
288+
Ok(paths) => {
289+
let mut matched = 0usize;
290+
for entry in paths.flatten() {
291+
let sub_config = parse_nginx_config(&entry);
292+
merge_configs(&mut config, sub_config);
293+
matched += 1;
294+
}
295+
// A glob matching nothing is legal in nginx (not an
296+
// error) but is almost always a mistake — e.g.
297+
// `sites-enabled/*.conf` against Debian's extensionless
298+
// symlinks. Surface it so it isn't silent.
299+
if matched == 0 {
300+
config.include_warnings.push(format!(
301+
"include '{}' matched no files — check the path and that the \
302+
filenames match the pattern (Debian sites-enabled entries often \
303+
have no .conf suffix, so '*.conf' matches nothing; try '*')",
304+
include_path.display()
305+
));
306+
}
282307
}
308+
Err(e) => config.include_warnings.push(format!(
309+
"include '{}' is not a valid glob pattern: {}", include, e
310+
)),
283311
}
284312
} else if include_path.exists() {
285313
let sub_config = parse_nginx_config(&include_path);
286314
merge_configs(&mut config, sub_config);
315+
} else {
316+
// An explicit (non-glob) include of a file that doesn't exist:
317+
// real nginx treats this as a fatal `-t` error. We surface it
318+
// loudly but don't refuse to load — a proxy that's been running
319+
// shouldn't suddenly fail to start on upgrade (see main.rs).
320+
config.include_warnings.push(format!(
321+
"include '{}' refers to a file that does not exist",
322+
include_path.display()
323+
));
287324
}
288325
}
289326
}
@@ -395,12 +432,12 @@ fn parse_upstream_server(line: &str) -> Option<UpstreamServer> {
395432
// Parse additional options
396433
for part in parts.iter().skip(2) {
397434
let part = part.trim_end_matches(';');
398-
if part.starts_with("weight=") {
399-
server.weight = part[7..].parse().unwrap_or(1);
400-
} else if part.starts_with("max_fails=") {
401-
server.max_fails = part[10..].parse().unwrap_or(1);
402-
} else if part.starts_with("fail_timeout=") {
403-
let timeout_str = part[13..].trim_end_matches('s');
435+
if let Some(v) = part.strip_prefix("weight=") {
436+
server.weight = v.parse().unwrap_or(1);
437+
} else if let Some(v) = part.strip_prefix("max_fails=") {
438+
server.max_fails = v.parse().unwrap_or(1);
439+
} else if let Some(v) = part.strip_prefix("fail_timeout=") {
440+
let timeout_str = v.trim_end_matches('s');
404441
server.fail_timeout = timeout_str.parse().unwrap_or(300);
405442
} else if part == "backup" {
406443
server.backup = true;
@@ -483,15 +520,15 @@ fn parse_server_content(content: &str, base_dir: Option<&Path>) -> Option<Server
483520
};
484521

485522
// Parse line by line, handling nested blocks
486-
let mut lines = content.lines().peekable();
523+
let lines = content.lines().peekable();
487524
let mut current_location: Option<LocationBlock> = None;
488525
let mut location_depth = 0;
489526
let mut location_content = String::new();
490527
let mut in_if = false;
491528
let mut if_content = String::new();
492529
let mut if_condition = String::new();
493530

494-
while let Some(line) = lines.next() {
531+
for line in lines {
495532
let trimmed = line.trim();
496533

497534
// Handle if blocks
@@ -863,10 +900,25 @@ fn parse_server_directive(server: &mut ServerBlock, line: &str, base_dir: Option
863900
for include_line in content.lines() {
864901
let include_trimmed = include_line.trim();
865902
if !include_trimmed.is_empty() && !include_trimmed.starts_with('#') {
866-
parse_server_directive(server, include_trimmed, Some(&full_path.parent().unwrap_or(Path::new("/"))));
903+
parse_server_directive(server, include_trimmed, Some(full_path.parent().unwrap_or(Path::new("/"))));
867904
}
868905
}
869906
}
907+
} else if include_path.contains('*') {
908+
// Server-context includes pull directives (snippets) into
909+
// the surrounding server; globbing them isn't supported
910+
// here — top-level includes are the place for globs.
911+
eprintln!(
912+
"wolfproxy: include '{}' inside a server block uses a glob, which is only \
913+
supported for top-level includes — the directives were not loaded",
914+
include_path
915+
);
916+
} else {
917+
eprintln!(
918+
"wolfproxy: include '{}' inside a server block refers to a file that does \
919+
not exist — the directives were not loaded",
920+
full_path.display()
921+
);
870922
}
871923
}
872924
}
@@ -914,8 +966,8 @@ fn parse_listen_directive(line: &str) -> Option<ListenDirective> {
914966
let addr_part = parts[0].trim_end_matches(';');
915967

916968
// Check for [::] IPv6 notation
917-
if addr_part.starts_with("[::]:") {
918-
listen.port = addr_part[5..].parse().unwrap_or(80);
969+
if let Some(port_str) = addr_part.strip_prefix("[::]:") {
970+
listen.port = port_str.parse().unwrap_or(80);
919971
listen.address = Some("[::]:".to_string());
920972
} else if addr_part.starts_with("[::]") {
921973
listen.port = 80;
@@ -975,14 +1027,14 @@ fn parse_rewrite_rule(line: &str) -> Option<RewriteRule> {
9751027

9761028
fn parse_time_value(s: &str) -> Option<u64> {
9771029
let s = s.trim();
978-
if s.ends_with('s') {
979-
s[..s.len() - 1].parse().ok()
980-
} else if s.ends_with('m') {
981-
s[..s.len() - 1].parse::<u64>().ok().map(|v| v * 60)
982-
} else if s.ends_with('h') {
983-
s[..s.len() - 1].parse::<u64>().ok().map(|v| v * 3600)
984-
} else if s.ends_with('d') {
985-
s[..s.len() - 1].parse::<u64>().ok().map(|v| v * 86400)
1030+
if let Some(n) = s.strip_suffix('s') {
1031+
n.parse().ok()
1032+
} else if let Some(n) = s.strip_suffix('m') {
1033+
n.parse::<u64>().ok().map(|v| v * 60)
1034+
} else if let Some(n) = s.strip_suffix('h') {
1035+
n.parse::<u64>().ok().map(|v| v * 3600)
1036+
} else if let Some(n) = s.strip_suffix('d') {
1037+
n.parse::<u64>().ok().map(|v| v * 86400)
9861038
} else {
9871039
s.parse().ok()
9881040
}
@@ -1005,6 +1057,9 @@ fn merge_configs(main: &mut NginxConfig, other: NginxConfig) {
10051057
main.upstreams.extend(other.upstreams);
10061058
main.servers.extend(other.servers);
10071059
main.includes.extend(other.includes);
1060+
// Bubble include problems up from nested includes so `--test` and the log
1061+
// see every one, no matter how deep the include chain.
1062+
main.include_warnings.extend(other.include_warnings);
10081063
}
10091064

10101065
/// Load the proxy's configuration the way nginx itself does.
@@ -1057,7 +1112,7 @@ pub fn load_nginx_sites(nginx_dir: &Path) -> NginxConfig {
10571112
if let Ok(entries) = fs::read_dir(&conf_d) {
10581113
for entry in entries.flatten() {
10591114
let path = entry.path();
1060-
if path.extension().map_or(false, |ext| ext == "conf") {
1115+
if path.extension().is_some_and(|ext| ext == "conf") {
10611116
let sub_config = parse_nginx_config(&path);
10621117
merge_configs(&mut config, sub_config);
10631118
}
@@ -1151,4 +1206,51 @@ mod tests {
11511206
let _ = fs::remove_dir_all(&dir);
11521207
let _ = fs::remove_dir_all(&dir2);
11531208
}
1209+
1210+
#[test]
1211+
fn test_include_problems_are_surfaced_not_swallowed() {
1212+
// A root.conf whose includes resolve to nothing must produce
1213+
// include_warnings — silence is what left the operator stranded.
1214+
let dir = std::env::temp_dir().join(format!("wolfproxy-incwarn-{}", std::process::id()));
1215+
let sites = dir.join("sites-enabled");
1216+
let _ = fs::remove_dir_all(&dir);
1217+
fs::create_dir_all(&sites).unwrap();
1218+
// The file exists but has NO .conf suffix (Debian symlink style), so a
1219+
// `*.conf` glob matches nothing, and an explicit include points at a
1220+
// file that isn't there.
1221+
fs::write(sites.join("sonarr"), "server {\n listen 80;\n server_name s.example.com;\n}\n").unwrap();
1222+
fs::write(
1223+
dir.join("root.conf"),
1224+
format!(
1225+
"http {{\n include {sites}/*.conf;\n include {sites}/missing.conf;\n}}\n",
1226+
sites = sites.display()
1227+
),
1228+
)
1229+
.unwrap();
1230+
1231+
let config = load_nginx_config(&dir);
1232+
assert_eq!(config.servers.len(), 0, "neither include should have matched a server");
1233+
assert_eq!(config.include_warnings.len(), 2,
1234+
"both the zero-match glob and the missing explicit include must warn; got {:?}",
1235+
config.include_warnings);
1236+
assert!(config.include_warnings.iter().any(|w| w.contains("matched no files")),
1237+
"zero-match glob warning missing: {:?}", config.include_warnings);
1238+
assert!(config.include_warnings.iter().any(|w| w.contains("does not exist")),
1239+
"missing-file warning missing: {:?}", config.include_warnings);
1240+
1241+
// Fixing the glob to `*` (which matches the extensionless file) loads
1242+
// the server AND clears the glob warning.
1243+
fs::write(
1244+
dir.join("root.conf"),
1245+
format!("http {{\n include {sites}/*;\n}}\n", sites = sites.display()),
1246+
)
1247+
.unwrap();
1248+
let ok = load_nginx_config(&dir);
1249+
assert!(ok.servers.iter().any(|s| s.server_names.iter().any(|n| n == "s.example.com")),
1250+
"`*` should match the extensionless file and load its server");
1251+
assert!(ok.include_warnings.is_empty(),
1252+
"a healthy include must produce no warnings; got {:?}", ok.include_warnings);
1253+
1254+
let _ = fs::remove_dir_all(&dir);
1255+
}
11541256
}

0 commit comments

Comments
 (0)