From 884b19ccb5fbcaa6b29cb41dc978386cf7b1b3f9 Mon Sep 17 00:00:00 2001
From: asepharyana <superaseph@gmail.com>
Date: Thu, 27 Aug 2026 22:09:22 +0700
Subject: [PATCH] fix(api): cegah race condition pada register users.json
 (TOCTOU)

- Tambah users_lock (Mutex) di ApiState untuk serialisasi read-modify-write
  users.json pada endpoint register; lock hanya dipegang selama operasi
  file sinkron (tidak pernah lintas .await, menjaga future tetap Send)
- Hash password dihitung sebelum lock sehingga request concurrent tidak
  saling blokir selama hashing Argon2
---
 apps/interfaces/api/src/handlers/auth.rs | 33 +++++++++++++++---------
 apps/interfaces/api/src/state.rs         |  7 +++++
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/apps/interfaces/api/src/handlers/auth.rs b/apps/interfaces/api/src/handlers/auth.rs
index b7ce363..44555ef 100644
--- a/apps/interfaces/api/src/handlers/auth.rs
+++ b/apps/interfaces/api/src/handlers/auth.rs
@@ -15,6 +15,7 @@
 use std::sync::Arc;
 
 use axum::extract::State;
+use axum::http::HeaderMap;
 use axum::routing::post;
 use axum::{Json, Router};
 
@@ -39,7 +40,7 @@ pub fn router() -> Router<Arc<ApiState>> {
 
 /// Extract a coarse client identity from the request headers (IP via
 /// X-Forwarded-For fallback). Used as the rate-limit key.
-fn client_id(headers: &axum::http::HeaderMap) -> String {
+fn client_id(headers: &HeaderMap) -> String {
     headers
         .get("x-forwarded-for")
         .and_then(|v| v.to_str().ok())
@@ -50,7 +51,7 @@ fn client_id(headers: &axum::http::HeaderMap) -> String {
 }
 
 /// Enforce the auth rate limit, returning `true` if the request is allowed.
-fn rate_limited(state: &ApiState, headers: &axum::http::HeaderMap) -> bool {
+fn rate_limited(state: &ApiState, headers: &HeaderMap) -> bool {
     !state
         .auth_rate_limiter
         .check_rate_limit(
@@ -79,7 +80,7 @@ fn rate_limited(state: &ApiState, headers: &axum::http::HeaderMap) -> bool {
 #[tracing::instrument(skip(state))]
 pub async fn login_handler(
     State(state): State<Arc<ApiState>>,
-    headers: axum::http::HeaderMap,
+    headers: HeaderMap,
     Json(req): Json<LoginRequest>,
 ) -> Result<Json<AuthResponse>, ApiError> {
     // Rate-limit login attempts (brute-force protection).
@@ -156,7 +157,7 @@ pub async fn login_handler(
 #[tracing::instrument(skip(state))]
 pub async fn register_handler(
     State(state): State<Arc<ApiState>>,
-    headers: axum::http::HeaderMap,
+    headers: HeaderMap,
     Json(req): Json<RegisterRequest>,
 ) -> Result<Json<AuthResponse>, ApiError> {
     // Rate-limit registration (abuse protection).
@@ -176,6 +177,21 @@ pub async fn register_handler(
         ));
     }
 
+    // Compute the password hash first (async, no lock held).
+    let hash = state
+        .password_service
+        .hash(&req.password)
+        .await
+        .map_err(|e| ApiError::Internal(format!("Password hashing failed: {e}")))?;
+
+    // Serialize read-modify-write of users.json to avoid losing concurrent
+    // registers (TOCTOU race). The lock is held only across the sync
+    // file operations — never across an `.await` (keeps the future `Send`).
+    let _users_guard = state
+        .users_lock
+        .lock()
+        .map_err(|_| ApiError::Internal("users lock poisoned".into()))?;
+
     // Load existing users
     let users_path = state.store_base_dir.join("users.json");
     let mut users: std::collections::HashMap<String, String> = if users_path.exists() {
@@ -194,13 +210,6 @@ pub async fn register_handler(
         ));
     }
 
-    // Hash the password
-    let hash = state
-        .password_service
-        .hash(&req.password)
-        .await
-        .map_err(|e| ApiError::Internal(format!("Password hashing failed: {e}")))?;
-
     // Persist
     users.insert(req.username.clone(), hash);
     let content = serde_json::to_string_pretty(&users)
@@ -239,7 +248,7 @@ pub async fn register_handler(
 #[tracing::instrument(skip(state))]
 pub async fn refresh_handler(
     State(state): State<Arc<ApiState>>,
-    headers: axum::http::HeaderMap,
+    headers: HeaderMap,
     Json(req): Json<RefreshRequest>,
 ) -> Result<Json<AuthResponse>, ApiError> {
     // Rate-limit refresh attempts.
diff --git a/apps/interfaces/api/src/state.rs b/apps/interfaces/api/src/state.rs
index 3194285..7544150 100644
--- a/apps/interfaces/api/src/state.rs
+++ b/apps/interfaces/api/src/state.rs
@@ -23,6 +23,7 @@
 use std::fmt;
 use std::future::Future;
 use std::path::PathBuf;
+use std::sync::Arc;
 
 use zesdex_application::ports::{PasswordService, TokenService};
 
@@ -208,6 +209,10 @@ pub struct ApiState {
     /// Shared sliding-window limiter for auth endpoints (login/register/refresh).
     pub auth_rate_limiter: zesdex_infrastructure::middleware::rate_limit::RateLimiter,
 
+    /// Serializes read-modify-write of `users.json` so concurrent register
+    /// requests cannot lose writes (TOCTOU race).
+    pub users_lock: Arc<std::sync::Mutex<()>>,
+
     /// LLM provider client for chat completions.
     pub llm_client: zesdex_infrastructure::llm::provider::LlmClient,
 }
@@ -288,6 +293,7 @@ impl ApiState {
 
         let token_service = JwtTokenService::new(&jwt_secret);
         let auth_rate_limiter = zesdex_infrastructure::middleware::rate_limit::RateLimiter::new();
+        let users_lock = Arc::new(std::sync::Mutex::new(()));
         let llm_client = zesdex_infrastructure::llm::provider::LlmClient::new(
             llm_api_key.into(),
             llm_model.into(),
@@ -304,6 +310,7 @@ impl ApiState {
             password_service: Argon2PasswordService,
             token_service,
             auth_rate_limiter,
+            users_lock,
             llm_client,
         }
     }