Add clarifying comments for correct-but-non-obvious behaviors (C3, W8, W10, W11, S9)
This commit is contained in:
@@ -181,6 +181,10 @@ pub fn validate(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Health check always binds to 127.0.0.1 (hardcoded in src/health.rs), so this
|
||||||
|
// conflict check is conservative — it warns even when the health check port
|
||||||
|
// wouldn't actually conflict (e.g., health check on 127.0.0.1:80 vs listener
|
||||||
|
// on 203.0.113.10:80). This is acceptable for Phase 1.
|
||||||
if static_config.health_check_port > 0 {
|
if static_config.health_check_port > 0 {
|
||||||
for listener in &static_config.listeners {
|
for listener in &static_config.listeners {
|
||||||
if static_config.health_check_port == listener.https_port {
|
if static_config.health_check_port == listener.https_port {
|
||||||
|
|||||||
@@ -23,6 +23,10 @@ pub async fn body_limit_middleware(
|
|||||||
limit
|
limit
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// Early rejection: if Content-Length is present and exceeds the limit, reject
|
||||||
|
// immediately without reading the body. For requests without Content-Length
|
||||||
|
// (chunked, HTTP/2), the Limited body wrapper below enforces the limit during
|
||||||
|
// streaming. This is a two-layer defense.
|
||||||
if let Some(content_length) = request.headers().get("content-length") {
|
if let Some(content_length) = request.headers().get("content-length") {
|
||||||
if let Ok(length_str) = content_length.to_str() {
|
if let Ok(length_str) = content_length.to_str() {
|
||||||
if let Ok(length) = length_str.parse::<u64>() {
|
if let Ok(length) = length_str.parse::<u64>() {
|
||||||
|
|||||||
@@ -110,6 +110,9 @@ async fn proxy_handler(
|
|||||||
);
|
);
|
||||||
let (mut parts, body) = upstream_resp.into_parts();
|
let (mut parts, body) = upstream_resp.into_parts();
|
||||||
remove_hop_by_hop(&mut parts.headers);
|
remove_hop_by_hop(&mut parts.headers);
|
||||||
|
// The upstream Server header is intentionally removed. As a security-focused
|
||||||
|
// reverse proxy, we hide upstream server identity as a defense-in-depth measure.
|
||||||
|
// The proxy does not add its own Server header either. See W8 in review #002.
|
||||||
parts.headers.remove("server");
|
parts.headers.remove("server");
|
||||||
let body = Body::new(body);
|
let body = Body::new(body);
|
||||||
Response::from_parts(parts, body)
|
Response::from_parts(parts, body)
|
||||||
|
|||||||
@@ -25,6 +25,10 @@ pub fn inject_proxy_headers(headers: &mut HeaderMap, remote_addr: SocketAddr) {
|
|||||||
|
|
||||||
headers.insert(HeaderName::from_static("x-real-ip"), ip_value.clone());
|
headers.insert(HeaderName::from_static("x-real-ip"), ip_value.clone());
|
||||||
|
|
||||||
|
// X-Forwarded-For is SET (not appended) because this proxy is the outermost
|
||||||
|
// edge proxy. Any existing X-Forwarded-For from the client is untrusted and
|
||||||
|
// must be replaced with the actual client IP from ConnectInfo.
|
||||||
|
// See ADR-021 for the edge proxy model rationale.
|
||||||
headers.insert(HeaderName::from_static("x-forwarded-for"), ip_value);
|
headers.insert(HeaderName::from_static("x-forwarded-for"), ip_value);
|
||||||
|
|
||||||
// X-Forwarded-Proto is always "https" because this proxy only forwards requests
|
// X-Forwarded-Proto is always "https" because this proxy only forwards requests
|
||||||
|
|||||||
@@ -76,6 +76,11 @@ pub async fn rate_limit_middleware(
|
|||||||
});
|
});
|
||||||
|
|
||||||
let Some(ip) = client_ip else {
|
let Some(ip) = client_ip else {
|
||||||
|
// If no client IP can be identified, the request passes through without rate
|
||||||
|
// limiting. In practice, ConnectInfo is always set by the server's
|
||||||
|
// ConnectInfoService, so this branch is unreachable. If the proxy were ever
|
||||||
|
// deployed without ConnectInfo propagation, rate limiting would silently become
|
||||||
|
// a no-op. Consider adding a warning log or returning 429 in a future phase.
|
||||||
return next.run(req).await;
|
return next.run(req).await;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user