Clean up

Author: nquinquenel

README.md

@@ -556,8 +556,8 @@ Connecting your SonarQube MCP Server with your SonarQube Cloud **US** instance r
 
 The SonarQube MCP Server supports three transport modes:
 
-#### 1. **Stdio** (Default - Single User)
-Default mode for single-user setups via command-line tools or MCP clients.
+#### 1. **Stdio** (Default - Recommended for Local Development)
+The recommended mode for local development and single-user setups, used by all MCP clients.
 
 **Example - Docker with SonarQube Cloud:**
 ```json
@@ -575,49 +575,29 @@ Default mode for single-user setups via command-line tools or MCP clients.
 }
 ```
 
-#### 2. **HTTP** (Multi-User - Recommended for Local Development)
-Enables multiple clients to connect to a shared server. Each client provides their own token.
+#### 2. **HTTP**
+Unencrypted HTTP transport. Use HTTPS instead for multi-user deployments.
+
+> ⚠️ **Not Recommended:** Use **Stdio** for local development or **HTTPS** for multi-user production deployments.
 
 | Environment variable | Description                                                      | Default         |
 |----------------------|------------------------------------------------------------------|-----------------|
 | `SONARQUBE_TRANSPORT`| Set to `http` to enable HTTP transport                           | Not set (stdio) |
 | `SONARQUBE_HTTP_PORT`| Port number (1024-65535)                                         | `8080`          |
-| `SONARQUBE_HTTP_HOST`| Host to bind (`127.0.0.1` for localhost, `0.0.0.0` for Docker) | `127.0.0.1`     |
-
-**Example - Docker with SonarQube Cloud:**
-```bash
-docker run --name sonarqube-mcp-server -p 8080:8080 \
-  -e SONARQUBE_TRANSPORT=http \
-  -e SONARQUBE_HTTP_HOST=0.0.0.0 \
-  -e SONARQUBE_TOKEN="<init-token>" \
-  -e SONARQUBE_ORG="<your-org>" \
-  mcp/sonarqube
-```
-
-**Client Configuration:**
-```json
-{
-  "mcpServers": {
-    "sonarqube-http": {
-      "url": "http://127.0.0.1:8080/mcp",
-      "headers": {
-        "SONARQUBE_TOKEN": "<your-token>"
-      }
-    }
-  }
-}
-```
+| `SONARQUBE_HTTP_HOST`| Host to bind (defaults to localhost for security)                | `127.0.0.1`     |
 
 **Note:** In HTTP(S) mode, each client sends their own token via the `SONARQUBE_TOKEN` header. The server's `SONARQUBE_TOKEN` is only used for initialization.
 
-#### 3. **HTTPS** (Multi-User - Production)
-Same as HTTP but with TLS encryption. Requires SSL certificates.
+#### 3. **HTTPS** (Recommended for Multi-User Production Deployments)
+Secure multi-user transport with TLS encryption. Requires SSL certificates.
+
+> ✅ **Recommended for Production:** Use HTTPS when deploying the MCP server for multiple users. The server binds to `127.0.0.1` (localhost) by default for security.
 
 | Environment variable | Description                                                      | Default         |
 |----------------------|------------------------------------------------------------------|-----------------|
 | `SONARQUBE_TRANSPORT`| Set to `https` to enable HTTPS transport                         | Not set (stdio) |
 | `SONARQUBE_HTTP_PORT`| Port number (typically 8443 for HTTPS)                          | `8080`          |
-| `SONARQUBE_HTTP_HOST`| Host to bind (`127.0.0.1` for localhost, `0.0.0.0` for Docker) | `127.0.0.1`     |
+| `SONARQUBE_HTTP_HOST`| Host to bind (defaults to localhost for security)               | `127.0.0.1`     |
 
 **SSL Certificate Configuration (Optional):**
 
@@ -632,7 +612,6 @@ Same as HTTP but with TLS encryption. Requires SSL certificates.
 docker run --name sonarqube-mcp-server -p 8443:8443 \
   -v $(pwd)/keystore.p12:/etc/ssl/mcp/keystore.p12:ro \
   -e SONARQUBE_TRANSPORT=https \
-  -e SONARQUBE_HTTP_HOST=0.0.0.0 \
   -e SONARQUBE_HTTP_PORT=8443 \
   -e SONARQUBE_TOKEN="<init-token>" \
   -e SONARQUBE_ORG="<your-org>" \
@@ -644,7 +623,7 @@ docker run --name sonarqube-mcp-server -p 8443:8443 \
 {
   "mcpServers": {
     "sonarqube-https": {
-      "url": "https://127.0.0.1:8443/mcp",
+      "url": "https://your-server:8443/mcp",
       "headers": {
         "SONARQUBE_TOKEN": "<your-token>"
       }
@@ -653,7 +632,7 @@ docker run --name sonarqube-mcp-server -p 8443:8443 \
 }
 ```
 
-**Note:** For local development, use HTTP instead of HTTPS to avoid certificate issues. For production deployments with proper SSL certificates from a trusted CA, use HTTPS.
+**Note:** For local development, use Stdio transport instead (the default). HTTPS is intended for multi-user production deployments with proper SSL certificates.
 
 ### Custom Certificates
 

docs/http-authentication-architecture.md

@@ -14,12 +14,13 @@
 
 ## Overview
 
-The SonarQube MCP Server supports two transport modes:
+The SonarQube MCP Server supports three transport modes:
 
-- **Stdio Transport**: Direct process communication (stdin/stdout)
-- **HTTP Transport**: Network-based communication using MCP Streamable HTTP specification
+- **Stdio Transport** (Recommended for local development): Direct process communication (stdin/stdout)
+- **HTTP Transport** (Not recommended): Unencrypted network communication
+- **HTTPS Transport** (Recommended for production): Secure network-based communication with TLS encryption
 
-This document focuses on the **HTTP Transport** and its authentication mechanism.
+This document focuses on the **HTTP/HTTPS Transport** and its authentication mechanism.
 
 ---
 
@@ -209,8 +210,9 @@ The flow is:
 **Mitigation**: `McpSecurityFilter` validates `Origin` header:
 
 **Allowed Origins**:
-- When bound to `127.0.0.1`: Only `http://localhost`, `http://127.0.0.1`, etc.
-- When bound to `0.0.0.0`: All origins allowed (less secure, logs warning)
+- When bound to `127.0.0.1` (default): Only `http://localhost`, `http://127.0.0.1`, etc.
+
+> ⚠️ **Important**: The server defaults to binding to `127.0.0.1` (localhost) for security. This is the recommended configuration. Binding to other interfaces is not supported for production use.
 
 ### Token Security
 

src/main/java/org/sonarsource/sonarqube/mcp/SonarQubeMcpServer.java

@@ -429,51 +429,69 @@ public void shutdown() {
     }
     isShutdown = true;
 
-    // Wait for background initialization to complete or cancel it
-    if (!initializationFuture.isDone()) {
-      LOG.info("Waiting for background initialization to complete before shutdown...");
-      try {
-        initializationFuture.get(30, java.util.concurrent.TimeUnit.SECONDS);
-      } catch (TimeoutException | ExecutionException e) {
-        LOG.warn("Background initialization did not complete within 30 seconds, proceeding with shutdown");
-        initializationFuture.cancel(true);
-      } catch (Exception e) {
-        LOG.error("Background initialization failed or was interrupted", e);
-      }
+    awaitBackgroundInitialization();
+    shutdownHttpServer();
+    shutdownHttpClient();
+    shutdownMcpServer();
+    shutdownBackend();
+  }
+
+  private void awaitBackgroundInitialization() {
+    if (initializationFuture.isDone()) {
+      return;
     }
+    LOG.info("Waiting for background initialization to complete before shutdown...");
+    try {
+      initializationFuture.get(30, java.util.concurrent.TimeUnit.SECONDS);
+    } catch (TimeoutException | ExecutionException e) {
+      LOG.warn("Background initialization did not complete within 30 seconds, proceeding with shutdown");
+      initializationFuture.cancel(true);
+    } catch (Exception e) {
+      LOG.error("Background initialization failed or was interrupted", e);
+    }
+  }
 
-    // Stop HTTP server if running
-    if (httpServerManager != null) {
-      try {
-        LOG.info("Stopping HTTP server...");
-        httpServerManager.stopServer().join();
-        LOG.info("HTTP server stopped");
-      } catch (Exception e) {
-        LOG.error("Error shutting down HTTP server", e);
-      }
-      
-      // Clean up session token store (only used in HTTP mode)
-      try {
-        SessionTokenStore.getInstance().shutdown();
-      } catch (Exception e) {
-        LOG.error("Error shutting down session token store", e);
-      }
+  private void shutdownHttpServer() {
+    if (httpServerManager == null) {
+      return;
+    }
+    try {
+      LOG.info("Stopping HTTP server...");
+      httpServerManager.stopServer().join();
+      LOG.info("HTTP server stopped");
+    } catch (Exception e) {
+      LOG.error("Error shutting down HTTP server", e);
     }
+    try {
+      SessionTokenStore.getInstance().shutdown();
+    } catch (Exception e) {
+      LOG.error("Error shutting down session token store", e);
+    }
+  }
 
+  private void shutdownHttpClient() {
+    if (httpClientProvider == null) {
+      return;
+    }
     try {
-      if (httpClientProvider != null) {
-        httpClientProvider.shutdown();
-      }
+      httpClientProvider.shutdown();
     } catch (Exception e) {
       LOG.error("Error shutting down HTTP client", e);
     }
+  }
+
+  private void shutdownMcpServer() {
+    if (syncServer == null) {
+      return;
+    }
     try {
-      if (syncServer != null) {
-        syncServer.closeGracefully();
-      }
+      syncServer.closeGracefully();
     } catch (Exception e) {
       LOG.error("Error shutting down MCP server", e);
     }
+  }
+
+  private void shutdownBackend() {
     try {
       backendService.shutdown();
     } catch (Exception e) {

src/main/java/org/sonarsource/sonarqube/mcp/authentication/AuthenticationFilter.java

@@ -136,10 +136,6 @@ private static void sendUnauthorizedResponse(HttpServletResponse response, Strin
     response.getWriter().write(errorJson);
   }
 
-  /**
-   * Send 403 Forbidden response for session hijacking attempts.
-   * Per MCP Security Best Practices: reject requests where token doesn't match session binding.
-   */
   private static void sendForbiddenResponse(HttpServletResponse response) throws IOException {
     response.setStatus(HttpServletResponse.SC_FORBIDDEN);
     response.setContentType("application/json");

src/main/java/org/sonarsource/sonarqube/mcp/authentication/SessionTokenStore.java

@@ -22,16 +22,11 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nullable;
-import org.sonarsource.sonarqube.mcp.log.McpLogger;
 
 /**
  * Thread-safe store for session-to-token mappings with TTL-based expiration.
- * <p>
- * This store is the key to preventing session hijacking. Instead of relying on
- * InheritableThreadLocal (which can leak tokens across requests when threads are
- * reused from pools), we store tokens by session ID and look them up at execution time.
- * <p>
  * Sessions are automatically expired after a period of inactivity to prevent
  * memory leaks and ensure stale sessions are cleaned up.
  * <p>
@@ -48,20 +43,21 @@ public class SessionTokenStore {
   private static final Duration SESSION_TTL = Duration.ofHours(1);
   private static final Duration CLEANUP_INTERVAL = Duration.ofMinutes(5);
   private static final SessionTokenStore INSTANCE = new SessionTokenStore();
-  
-  /** Session entry with token and last access timestamp */
+
+  /**
+   * Session entry with token and last access timestamp
+   */
   private record SessionEntry(String token, Instant lastAccess) {
     SessionEntry touch() {
       return new SessionEntry(token, Instant.now());
     }
   }
-  
+
   // Maps sessionId → SessionEntry (token + last access time)
   private final ConcurrentHashMap<String, SessionEntry> sessionTokens = new ConcurrentHashMap<>();
   private final ScheduledExecutorService cleanupScheduler;
 
   private SessionTokenStore() {
-    // Start background cleanup task
     cleanupScheduler = Executors.newSingleThreadScheduledExecutor(r -> {
       var thread = new Thread(r, "session-cleanup");
       thread.setDaemon(true);
@@ -82,16 +78,10 @@ public static SessionTokenStore getInstance() {
   /**
    * Store the token for a NEW session only.
    * For existing sessions, validates that the token matches and refreshes the TTL.
-   * 
-   * @param sessionId The MCP session ID
-   * @param token The token from the request
-   * @return true if token was stored/matches, false if token mismatch (hijacking attempt)
    */
   public boolean setTokenIfValid(String sessionId, String token) {
     var newEntry = new SessionEntry(token, Instant.now());
-    
-    // Use compute to atomically check-and-set or validate
-    var result = new boolean[]{true};
+    var result = new AtomicBoolean(true);
     sessionTokens.compute(sessionId, (key, existing) -> {
       if (existing == null) {
         // New session - store the entry
@@ -103,31 +93,28 @@ public boolean setTokenIfValid(String sessionId, String token) {
         return existing.touch();
       }
       // Token mismatch
-      result[0] = false;
-      return existing; // Keep existing entry unchanged
+      result.set(false);
+      return existing;
     });
-    
-    return result[0];
+
+    return result.get();
   }
 
   /**
    * Get the token for a session, refreshing its TTL.
    * Called by tool execution to get the correct token.
-   * 
-   * @param sessionId The MCP session ID
-   * @return The token, or null if session not found or expired
    */
   @Nullable
   public String getToken(String sessionId) {
     var entry = sessionTokens.computeIfPresent(sessionId, (key, existing) -> {
       // Check if expired
       if (isExpired(existing)) {
-        return null; // Remove expired entry
+        return null;
       }
       // Refresh TTL on access
       return existing.touch();
     });
-    
+
     return entry != null ? entry.token() : null;
   }
 
@@ -157,21 +144,13 @@ public void shutdown() {
     }
     clear();
   }
-  
-  private boolean isExpired(SessionEntry entry) {
+
+  private static boolean isExpired(SessionEntry entry) {
     return Duration.between(entry.lastAccess(), Instant.now()).compareTo(SESSION_TTL) > 0;
   }
-  
+
   private void cleanupExpiredSessions() {
-    var expiredCount = 0;
-    var iterator = sessionTokens.entrySet().iterator();
-    while (iterator.hasNext()) {
-      var entry = iterator.next();
-      if (isExpired(entry.getValue())) {
-        iterator.remove();
-        expiredCount++;
-      }
-    }
+    sessionTokens.entrySet().removeIf(entry -> isExpired(entry.getValue()));
   }
 
 }

src/main/java/org/sonarsource/sonarqube/mcp/context/RequestContext.java

@@ -39,17 +39,14 @@ public static RequestContext current() {
   /**
    * Set the request context for the current thread.
    * This should be called by the tool execution handler with the session ID from the MCP exchange.
-   *
-   * @param sessionId the MCP session ID
    */
   public static void set(String sessionId) {
     CONTEXT.set(new RequestContext(sessionId));
   }
 
   /**
    * Clear the request context for the current thread.
-   * This MUST be called after request processing completes to avoid memory leaks
-   * and thread pool contamination.
+   * This MUST be called after request processing completes to avoid memory leak and thread pool contamination.
    */
   public static void clear() {
     CONTEXT.remove();