Skip to content

Conversation

@nquinquenel
Copy link
Member

No description provided.

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title WIP MCP-215 WIP Dec 4, 2025
@hashicorp-vault-sonar-prod
Copy link

hashicorp-vault-sonar-prod bot commented Dec 4, 2025

MCP-215

@nquinquenel nquinquenel force-pushed the task/nq/http-improvements branch 2 times, most recently from 41fdf10 to c2af3ed Compare December 4, 2025 16:37
@nquinquenel nquinquenel requested a review from Copilot December 5, 2025 08:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements session-token binding to prevent session hijacking attacks in HTTP mode. The changes introduce a SessionTokenStore that maps MCP session IDs to authentication tokens, ensuring that a session can only be used with the token that created it. Additionally, the security filter is enhanced to prevent subdomain bypass attacks by validating exact host matches instead of prefix matching.

Key changes:

  • New SessionTokenStore component with TTL-based expiration for secure session-token mappings
  • Modified AuthenticationFilter to validate session-token bindings and reject hijacking attempts
  • Enhanced McpSecurityFilter to prevent subdomain bypass attacks (e.g., localhost.evil.com)
  • Updated RequestContext to store session IDs instead of tokens

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
SessionTokenStore.java New thread-safe store for session-token mappings with automatic TTL-based cleanup
SessionTokenStoreTest.java Comprehensive test coverage for the new SessionTokenStore component
AuthenticationFilter.java Modified to validate session-token bindings and prevent hijacking attempts
AuthenticationFilterTest.java Added tests for session binding validation and hijacking prevention
RequestContext.java Changed from storing tokens to storing session IDs
SonarQubeMcpServer.java Updated to look up tokens from SessionTokenStore and handle session context
SonarQubeMcpServerGenericTest.java Tests for server behavior with new session-based token lookup
McpSecurityFilter.java Enhanced to prevent subdomain bypass attacks with exact host matching
McpSecurityFilterTest.java Added tests for subdomain bypass attack prevention
http-authentication-architecture.md Updated documentation to reflect new session-token binding architecture

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nquinquenel nquinquenel force-pushed the task/nq/http-improvements branch from c2af3ed to 1bf2ce2 Compare December 5, 2025 08:28
@nquinquenel nquinquenel marked this pull request as ready for review December 5, 2025 08:29
@nquinquenel nquinquenel force-pushed the task/nq/http-improvements branch from 1bf2ce2 to 5f64bc3 Compare December 5, 2025 08:51
@nquinquenel nquinquenel force-pushed the task/nq/http-improvements branch from 5f64bc3 to 3e101ea Compare December 5, 2025 14:03
@nquinquenel nquinquenel changed the title MCP-215 WIP MCP-215 SSF Dec 5, 2025
Copy link
Contributor

@damien-urruty-sonarsource damien-urruty-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The McpServer class is becoming huge, I think we should act on that. Lot of things going on there

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 5, 2025

SonarQube reviewer guide

Important

We are currently testing different models for AI Summary.
Please give us your feedback by filling this form.

Model A:

Summary: Refactors HTTP authentication to prevent session hijacking using SessionTokenStore with session-to-token binding.

Review Focus: The new SessionTokenStore implements critical security logic that validates session-token bindings to prevent hijacking attacks. Pay special attention to the setTokenIfValid() method's concurrent access handling and the token lookup flow from AuthenticationFilterSessionTokenStoreSonarQubeMcpServer.get(). Also review the origin validation fix in McpSecurityFilter that now uses exact host matching to prevent subdomain bypass attacks.

Start review at: src/main/java/org/sonarsource/sonarqube/mcp/authentication/SessionTokenStore.java. This is the core new component that enforces session security and is central to understanding how the authentication flow has changed throughout the system.

Model B:

Summary: Refactor HTTP authentication to use session-token binding instead of ThreadLocal storage, improving security against session hijacking and simplifying async request handling.

Review Focus:

  • SessionTokenStore.java (new file): Core security mechanism - validates that tokens are bound to sessions and rejects hijacking attempts. Verify the TTL-based expiration logic and thread-safety of the ConcurrentHashMap operations.
  • AuthenticationFilter.java: Changes how tokens are stored - now delegates to SessionTokenStore instead of RequestContext. Ensure the session ID extraction and validation flow is correct.
  • SonarQubeMcpServer.java (ServerApiProvider.get()): Critical change in token retrieval - now looks up from SessionTokenStore by session ID. Verify error handling for missing sessions/tokens is robust.
  • Documentation updates: Verify the security posture changes are accurately reflected (Stdio recommended for dev, HTTPS for production, HTTP discouraged).

Start review at: src/main/java/org/sonarsource/sonarqube/mcp/authentication/SessionTokenStore.java. This is the foundation of the security model - it prevents session hijacking by binding tokens to sessions and enforcing token matching on subsequent requests. Understanding this store's guarantees is essential before reviewing how it's used in the filters and server code.

Quality Gate Passed Quality Gate passed

Issues
0 New issues
1 Accepted issue
0 Dependency risks

Measures
0 Security Hotspots
83.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@nquinquenel nquinquenel merged commit d8d7a7c into master Dec 5, 2025
3 checks passed
@nquinquenel nquinquenel deleted the task/nq/http-improvements branch December 5, 2025 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants