Skip to content

Commit 080d7ae

Browse files
zerox80zerox80
committed
refactor: enhance security headers and comment validation
- Added configurable comment author display names via environment variable - Implemented content sanitization with control character filtering and length validation - Enhanced security headers (CSP, HSTS, Referrer-Policy, Permissions-Policy) across backend and nginx - Added proxy IP header controls with TRUST_PROXY_IP_HEADERS flag to prevent spoofing
1 parent efbe47c commit 080d7ae

File tree

3 files changed

+153
-31
lines changed

3 files changed

+153
-31
lines changed

‎backend/src/handlers/comments.rs‎

Lines changed: 64 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use axum::{
55
Json,
66
};
77
use serde::{Deserialize, Serialize};
8+
use std::{env, sync::OnceLock};
89

910
#[derive(Deserialize)]
1011
pub struct CreateCommentRequest {
@@ -32,6 +33,63 @@ pub struct Comment {
3233
pub created_at: String,
3334
}
3435

36+
fn comment_author_display_name(claims: &auth::Claims) -> String {
37+
static DISPLAY_NAME: OnceLock<Option<String>> = OnceLock::new();
38+
39+
let configured = DISPLAY_NAME.get_or_init(|| {
40+
env::var("COMMENT_AUTHOR_DISPLAY_NAME")
41+
.ok()
42+
.and_then(|value| {
43+
let trimmed = value.trim();
44+
if trimmed.is_empty() {
45+
None
46+
} else {
47+
Some(trimmed.to_string())
48+
}
49+
})
50+
});
51+
52+
configured
53+
.as_ref()
54+
.cloned()
55+
.unwrap_or_else(|| claims.sub.clone())
56+
}
57+
58+
fn sanitize_comment_content(
59+
raw: &str,
60+
) -> Result<String, (StatusCode, Json<ErrorResponse>)> {
61+
let trimmed = raw.trim();
62+
63+
if trimmed.is_empty() {
64+
return Err((
65+
StatusCode::BAD_REQUEST,
66+
Json(ErrorResponse {
67+
error: "Comment content cannot be empty".to_string(),
68+
}),
69+
));
70+
}
71+
72+
if trimmed.len() > 1_000 {
73+
return Err((
74+
StatusCode::BAD_REQUEST,
75+
Json(ErrorResponse {
76+
error: "Comment too long (max 1000 characters)".to_string(),
77+
}),
78+
));
79+
}
80+
81+
let sanitized: String = trimmed
82+
.chars()
83+
.filter(|c| match c {
84+
'\n' | '\r' | '\t' => true,
85+
c if c.is_control() => false,
86+
_ => true,
87+
})
88+
.collect();
89+
90+
Ok(sanitized)
91+
}
92+
3593
pub async fn list_comments(
3694
State(pool): State<DbPool>,
3795
Path(tutorial_id): Path<String>,
@@ -141,24 +199,8 @@ pub async fn create_comment(
141199
));
142200
}
143201

144-
// Validate content
145-
if payload.content.trim().is_empty() {
146-
return Err((
147-
StatusCode::BAD_REQUEST,
148-
Json(ErrorResponse {
149-
error: "Comment content cannot be empty".to_string(),
150-
}),
151-
));
152-
}
153-
154-
if payload.content.len() > 1000 {
155-
return Err((
156-
StatusCode::BAD_REQUEST,
157-
Json(ErrorResponse {
158-
error: "Comment too long (max 1000 characters)".to_string(),
159-
}),
160-
));
161-
}
202+
let comment_content = sanitize_comment_content(&payload.content)?;
203+
let author = comment_author_display_name(&claims);
162204

163205
let id = uuid::Uuid::new_v4().to_string();
164206
let now = chrono::Utc::now().to_rfc3339();
@@ -168,8 +210,8 @@ pub async fn create_comment(
168210
)
169211
.bind(&id)
170212
.bind(&tutorial_id)
171-
.bind(&claims.sub)
172-
.bind(&payload.content)
213+
.bind(&author)
214+
.bind(&comment_content)
173215
.bind(&now)
174216
.execute(&pool)
175217
.await
@@ -186,8 +228,8 @@ pub async fn create_comment(
186228
Ok(Json(Comment {
187229
id,
188230
tutorial_id,
189-
author: claims.sub,
190-
content: payload.content,
231+
author,
232+
content: comment_content,
191233
created_at: now,
192234
}))
193235
}

‎backend/src/main.rs‎

Lines changed: 83 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use axum::http::{
1717
AUTHORIZATION, CACHE_CONTROL, CONTENT_SECURITY_POLICY, CONTENT_TYPE, EXPIRES,
1818
PRAGMA, STRICT_TRANSPORT_SECURITY, X_CONTENT_TYPE_OPTIONS, X_FRAME_OPTIONS,
1919
},
20-
HeaderValue, Method,
20+
HeaderName, HeaderValue, Method,
2121
};
2222
use tower_http::cors::{AllowHeaders, AllowMethods, AllowOrigin, CorsLayer};
2323
use tower_http::limit::RequestBodyLimitLayer;
@@ -29,7 +29,45 @@ use tracing_subscriber;
2929
use tokio::signal;
3030
use std::net::SocketAddr;
3131
use std::io::ErrorKind;
32-
use tower_governor::key_extractor::SmartIpKeyExtractor;
32+
use tower_governor::key_extractor::{PeerIpKeyExtractor, SmartIpKeyExtractor};
33+
34+
const PERMISSIONS_POLICY: HeaderName = HeaderName::from_static("permissions-policy");
35+
const REFERRER_POLICY: HeaderName = HeaderName::from_static("referrer-policy");
36+
const X_XSS_PROTECTION: HeaderName = HeaderName::from_static("x-xss-protection");
37+
const FORWARDED_HEADER: HeaderName = HeaderName::from_static("forwarded");
38+
const X_FORWARDED_FOR_HEADER: HeaderName = HeaderName::from_static("x-forwarded-for");
39+
const X_FORWARDED_PROTO_HEADER: HeaderName = HeaderName::from_static("x-forwarded-proto");
40+
const X_FORWARDED_HOST_HEADER: HeaderName = HeaderName::from_static("x-forwarded-host");
41+
const X_REAL_IP_HEADER: HeaderName = HeaderName::from_static("x-real-ip");
42+
43+
fn parse_env_bool(key: &str, default: bool) -> bool {
44+
env::var(key)
45+
.ok()
46+
.and_then(|value| {
47+
match value.trim().to_ascii_lowercase().as_str() {
48+
"1" | "true" | "yes" | "on" => Some(true),
49+
"0" | "false" | "no" | "off" => Some(false),
50+
_ => {
51+
tracing::warn!(key = %key, value = %value, "Invalid boolean env value; using default");
52+
None
53+
}
54+
}
55+
})
56+
.unwrap_or(default)
57+
}
58+
59+
async fn strip_untrusted_forwarded_headers(mut request: Request, next: Next) -> Response {
60+
{
61+
let headers = request.headers_mut();
62+
headers.remove(FORWARDED_HEADER);
63+
headers.remove(X_FORWARDED_FOR_HEADER);
64+
headers.remove(X_FORWARDED_PROTO_HEADER);
65+
headers.remove(X_FORWARDED_HOST_HEADER);
66+
headers.remove(X_REAL_IP_HEADER);
67+
}
68+
69+
next.run(request).await
70+
}
3371

3472
// Security headers middleware
3573
async fn security_headers(
@@ -75,10 +113,10 @@ async fn security_headers(
75113
// Content Security Policy - Environment-dependent for dev mode support
76114
let csp = if cfg!(debug_assertions) {
77115
// Development mode: Allow WebSocket for Vite HMR
78-
"default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self' ws: wss:; object-src 'none'; base-uri 'self'; form-action 'self';"
116+
"default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self' ws: wss:; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none';"
79117
} else {
80118
// Production mode: Strict CSP (no inline styles)
81-
"default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data:; connect-src 'self'; object-src 'none'; base-uri 'self'; form-action 'self';"
119+
"default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data:; connect-src 'self'; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'; upgrade-insecure-requests;"
82120
};
83121

84122
headers.insert(
@@ -105,6 +143,21 @@ async fn security_headers(
105143
X_FRAME_OPTIONS,
106144
HeaderValue::from_static("DENY"),
107145
);
146+
147+
headers.insert(
148+
REFERRER_POLICY,
149+
HeaderValue::from_static("no-referrer"),
150+
);
151+
152+
headers.insert(
153+
PERMISSIONS_POLICY,
154+
HeaderValue::from_static("geolocation=(), microphone=(), camera=()"),
155+
);
156+
157+
headers.insert(
158+
X_XSS_PROTECTION,
159+
HeaderValue::from_static("0"),
160+
);
108161

109162
response
110163
}
@@ -199,12 +252,25 @@ async fn main() {
199252

200253
tracing::info!(origins = ?allowed_origins, "Configured CORS origins");
201254

255+
let trust_proxy_ip_headers = parse_env_bool("TRUST_PROXY_IP_HEADERS", false);
256+
if trust_proxy_ip_headers {
257+
tracing::info!("Trusting X-Forwarded-* headers for client IP extraction");
258+
} else {
259+
tracing::info!("Proxy headers will be stripped before rate limiting to prevent spoofing");
260+
}
261+
262+
let login_key_extractor = if trust_proxy_ip_headers {
263+
SmartIpKeyExtractor
264+
} else {
265+
PeerIpKeyExtractor
266+
};
267+
202268
// Configure rate limiting (average 1 request/sec for login, burst up to 5)
203269
let rate_limit_config = std::sync::Arc::new(
204270
GovernorConfigBuilder::default()
205271
.per_second(1)
206272
.burst_size(5)
207-
.key_extractor(SmartIpKeyExtractor)
273+
.key_extractor(login_key_extractor)
208274
.finish()
209275
.expect("Failed to build governor config"),
210276
);
@@ -217,11 +283,17 @@ async fn main() {
217283
.layer(RequestBodyLimitLayer::new(LOGIN_BODY_LIMIT))
218284
.layer(GovernorLayer::new(rate_limit_config));
219285

286+
let admin_key_extractor = if trust_proxy_ip_headers {
287+
SmartIpKeyExtractor
288+
} else {
289+
PeerIpKeyExtractor
290+
};
291+
220292
let admin_rate_limit_config = std::sync::Arc::new(
221293
GovernorConfigBuilder::default()
222294
.per_second(1)
223295
.burst_size(3)
224-
.key_extractor(SmartIpKeyExtractor)
296+
.key_extractor(admin_key_extractor)
225297
.finish()
226298
.expect("Failed to build governor config for write routes"),
227299
);
@@ -276,7 +348,7 @@ async fn main() {
276348
.layer(RequestBodyLimitLayer::new(ADMIN_BODY_LIMIT))
277349
.layer(GovernorLayer::new(admin_rate_limit_config.clone()));
278350

279-
let app = Router::new()
351+
let mut app = Router::new()
280352
.merge(login_router)
281353
// Auth routes
282354
.route("/api/auth/me", get(handlers::auth::me))
@@ -321,6 +393,10 @@ async fn main() {
321393
.layer(middleware::from_fn(security_headers))
322394
.with_state(pool);
323395

396+
if !trust_proxy_ip_headers {
397+
app = app.layer(middleware::from_fn(strip_untrusted_forwarded_headers));
398+
}
399+
324400
// Get port from environment or use default
325401
let port_str = env::var("PORT").unwrap_or_else(|_| "8489".to_string());
326402
let port: u16 = match port_str.parse() {

‎nginx/frontend.conf‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,13 @@ server {
1111
gzip_types text/plain text/css text/xml text/javascript application/javascript application/xml+rss application/json;
1212

1313
# Security Headers
14-
add_header X-Frame-Options "SAMEORIGIN" always;
14+
add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data:; connect-src 'self'; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'; upgrade-insecure-requests;" always;
15+
add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;
16+
add_header X-Frame-Options "DENY" always;
1517
add_header X-Content-Type-Options "nosniff" always;
16-
add_header X-XSS-Protection "1; mode=block" always;
18+
add_header Referrer-Policy "no-referrer" always;
19+
add_header Permissions-Policy "geolocation=(), microphone=(), camera=()" always;
20+
add_header X-XSS-Protection "0" always;
1721

1822
# Cache static assets
1923
location ~* \.(js|css|png|jpg|jpeg|gif|ico|svg|woff|woff2|ttf|eot)$ {

0 commit comments

Comments
 (0)