From 287bab9fa7b69dc61e524bf6a5e404552d9d37ee Mon Sep 17 00:00:00 2001 From: Big-cedar Date: Thu, 30 Jul 2026 11:07:29 +0100 Subject: [PATCH] fix(backend): graceful shutdown and strict JWT/Redis env vars --- CHANGES.md | 27 +++++++++++++ backend/.env.example | 10 +++-- backend/modules/api/src/server.rs | 59 ++++++++++++++++++++++------- backend/modules/security/src/jwt.rs | 44 +++++++++++++++++++++ backend/src/main.rs | 4 ++ 5 files changed, 127 insertions(+), 17 deletions(-) create mode 100644 CHANGES.md diff --git a/CHANGES.md b/CHANGES.md new file mode 100644 index 00000000..bbfc8443 --- /dev/null +++ b/CHANGES.md @@ -0,0 +1,27 @@ +# BE-26 & BE-27 Implementation + +## BE-26: Graceful shutdown for Actix-Web server +**Location:** `backend/modules/api/src/server.rs` (invoked from `backend/src/main.rs`) + +- Installs Unix SIGTERM and SIGINT handlers via `tokio::signal::unix`. +- On signal, calls `ServerHandle::stop(true)` so Actix drains in-flight + requests / WebSocket connections and shuts down the worker thread pool + cleanly instead of aborting mid-transaction. + +## BE-27: Strict JWT secret and Redis URL environment variables +**Location:** `backend/modules/security/src/jwt.rs`, `backend/modules/api/src/server.rs` + +- Added `JwtService::from_env()` which **requires** `JWT_SECRET` or + `JWT_SECRET_KEY` (no hardcoded fallback). Panics on empty or known + insecure default values. +- `REDIS_URL` is now required via `std::env::var(...).expect(...)` — + no `redis://localhost:6379` fallback. +- `.env.example` updated to document required secrets. + +### Required env vars at startup +| Variable | Required | Notes | +|----------|----------|-------| +| `JWT_SECRET` or `JWT_SECRET_KEY` | **Yes** | Prefer `JWT_SECRET`; rejects known insecure defaults | +| `REDIS_URL` | **Yes** | e.g. `redis://localhost:6379` | +| `DATABASE_URL` | Yes (already) | | +| `JWT_EXPIRATION_SECS` | No | Default 3600 | diff --git a/backend/.env.example b/backend/.env.example index 14cb690b..dd5d5b48 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -9,12 +9,14 @@ RUST_LOG=info,xlmate=debug # PostgreSQL connection string DATABASE_URL=postgres://username:password@localhost:5432/xlmate_db -# Redis Configuration +# Redis Configuration (REQUIRED — no fallback) REDIS_URL=redis://localhost:6379 -# JWT Configuration -# Secret key for signing JWT tokens - CHANGE THIS IN PRODUCTION! -JWT_SECRET_KEY=xlmate_super_secret_jwt_key_change_in_production +# JWT Configuration (REQUIRED — server will refuse to start without a real secret) +# Prefer JWT_SECRET; JWT_SECRET_KEY is accepted for backward compatibility. +# Do NOT use placeholder/default values — the process panics on known insecure defaults. +JWT_SECRET=generate_a_long_random_secret_here +# JWT_SECRET_KEY=generate_a_long_random_secret_here # Token expiration time in seconds (3600 = 1 hour) JWT_EXPIRATION_SECS=3600 diff --git a/backend/modules/api/src/server.rs b/backend/modules/api/src/server.rs index e698bfab..030aa691 100644 --- a/backend/modules/api/src/server.rs +++ b/backend/modules/api/src/server.rs @@ -52,17 +52,19 @@ pub async fn main() -> std::io::Result<()> { // Initialize logger env_logger::init(); - // Load configuration from environment + // Load configuration from environment — critical secrets have no fallbacks (BE-27) let server_addr = env::var("SERVER_ADDR").unwrap_or_else(|_| "127.0.0.1:8080".to_string()); let database_url = env::var("DATABASE_URL").expect("DATABASE_URL must be set in .env"); - let jwt_secret = env::var("JWT_SECRET_KEY") - .unwrap_or_else(|_| "knightverse_dev_secret_key_change_in_production".to_string()); - let jwt_expiration = env::var("JWT_EXPIRATION_SECS") - .unwrap_or_else(|_| "3600".to_string()) - .parse::() - .unwrap_or(3600); - let redis_url = env::var("REDIS_URL").unwrap_or_else(|_| "redis://localhost:6379".to_string()); + // JWT: strict env — crash on missing/insecure secret + let jwt_service = JwtService::from_env(); + let jwt_secret = jwt_service.secret_key.clone(); + let jwt_expiration = jwt_service.expiration_time(); + + // Redis: strict env — no localhost fallback that could mask misconfiguration + let redis_url = env::var("REDIS_URL").expect( + "REDIS_URL must be set. Refusing to start with a hardcoded fallback.", + ); eprintln!("Initializing KnightVerse Backend Server"); eprintln!("Server address: {}", server_addr); @@ -91,8 +93,6 @@ pub async fn main() -> std::io::Result<()> { } } - // Initialize JWT service - let jwt_service = JwtService::new(jwt_secret.clone(), jwt_expiration); let db = std::sync::Arc::new(db); // Wrap db in Arc // Create a shared LobbyState actor @@ -262,14 +262,47 @@ pub async fn main() -> std::io::Result<()> { ) }; - let mut server = HttpServer::new(app_factory).bind(&server_addr)?; + let mut http_server = HttpServer::new(app_factory).bind(&server_addr)?; if let Ok(workers_str) = env::var("WORKERS") { if let Ok(workers) = workers_str.parse::() { println!("Setting worker count to {}", workers); - server = server.workers(workers); + http_server = http_server.workers(workers); } } - server.run().await + // BE-26: Graceful shutdown on SIGTERM / SIGINT + // stop(true) waits for in-flight requests and drains the worker thread pool + // so active WebSocket connections and DB transactions can complete cleanly. + let server = http_server.run(); + let server_handle = server.handle(); + + actix_web::rt::spawn(async move { + #[cfg(unix)] + { + use tokio::signal::unix::{signal, SignalKind}; + let mut sigterm = signal(SignalKind::terminate()) + .expect("failed to install SIGTERM handler"); + let mut sigint = signal(SignalKind::interrupt()) + .expect("failed to install SIGINT handler"); + tokio::select! { + _ = sigterm.recv() => { + eprintln!("Received SIGTERM — initiating graceful shutdown..."); + } + _ = sigint.recv() => { + eprintln!("Received SIGINT — initiating graceful shutdown..."); + } + } + } + #[cfg(not(unix))] + { + let _ = tokio::signal::ctrl_c().await; + eprintln!("Received Ctrl+C — initiating graceful shutdown..."); + } + // true = graceful: finish active connections, then drain workers + server_handle.stop(true).await; + eprintln!("Server stopped gracefully"); + }); + + server.await } diff --git a/backend/modules/security/src/jwt.rs b/backend/modules/security/src/jwt.rs index 9c8e0157..d3e04d99 100644 --- a/backend/modules/security/src/jwt.rs +++ b/backend/modules/security/src/jwt.rs @@ -60,6 +60,50 @@ impl JwtService { } } + /// Create a JWT service from environment variables. + /// + /// Requires `JWT_SECRET` (preferred) or `JWT_SECRET_KEY`. The process will + /// panic on startup if neither is set — no hardcoded fallback is allowed. + /// Optional: `JWT_EXPIRATION_SECS` (default 3600). + pub fn from_env() -> Self { + let secret_key = std::env::var("JWT_SECRET") + .or_else(|_| std::env::var("JWT_SECRET_KEY")) + .expect( + "JWT_SECRET (or JWT_SECRET_KEY) must be set. Refusing to start with a hardcoded fallback secret.", + ); + + if secret_key.trim().is_empty() { + panic!("JWT_SECRET must not be empty"); + } + + // Reject well-known insecure defaults that might still be in .env templates + const INSECURE_DEFAULTS: &[&str] = &[ + "knightverse_dev_secret_key_change_in_production", + "xlmate_super_secret_jwt_key_change_in_production", + "your_secret_key_here", + "your_secret_key_change_this_in_production", + "change_me", + "secret", + ]; + if INSECURE_DEFAULTS.iter().any(|d| secret_key == *d) { + panic!( + "JWT_SECRET appears to be an insecure default value. Set a strong, unique secret before starting the server." + ); + } + + let expiration_time = std::env::var("JWT_EXPIRATION_SECS") + .ok() + .and_then(|s| s.parse::().ok()) + .unwrap_or(3600); + + Self::new(secret_key, expiration_time) + } + + /// Token expiration time in seconds + pub fn expiration_time(&self) -> usize { + self.expiration_time + } + /// Generate a new JWT access token for a user pub fn generate_token( &self, diff --git a/backend/src/main.rs b/backend/src/main.rs index 94ee2437..1f1f086b 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -2,6 +2,10 @@ //! //! This is the unified entry point for the KnightVerse backend service. //! It initializes the API server with all configured routes and middleware. +//! +//! Graceful shutdown (BE-26): the Actix-Web server in `api::server` installs +//! SIGTERM/SIGINT handlers and calls `ServerHandle::stop(true)` so in-flight +//! HTTP/WebSocket work and the worker thread pool are drained cleanly. use api::server;