MCP-420 Remove redundant warning about bind address#338
MCP-420 Remove redundant warning about bind address#338sophio-japharidze-sonarsource wants to merge 1 commit intomasterfrom
Conversation
SummaryThis PR removes a redundant security warning about binding to 0.0.0.0. Two warnings existed:
The fix removes the first warning (which was redundant) and refines the second to only trigger when HTTPS is disabled. The logic is: binding to 0.0.0.0 with HTTPS enabled is acceptable, so only warn the user when both 0.0.0.0 binding AND HTTP (unencrypted) are active. The What reviewers should knowWhat to review:
Key insight: This is a refinement of security messaging, not a removal of security. The single remaining warning is more nuanced — it only flags the risky case (unencrypted + exposed to all interfaces).
|
|
![sonar-review-alpha[bot]](https://avatars.githubusercontent.com/in/3025063?s=60&v=4){kind=small}
There was a problem hiding this comment.
The warning logic refinement itself is correct — httpsEnabled is already in scope at that point (line 146 uses it to set protocol), and suppressing the 0.0.0.0 warning when HTTPS is active is a reasonable security trade-off. The removal of the duplicate warning in McpSecurityFilter is clean.
The main problem is an unrelated file that was accidentally committed.
| @@ -0,0 +1,12 @@ | |||
| { | |||
There was a problem hiding this comment.
This is a local Claude Code IDE settings file and should not be committed to the repository. It contains developer-specific tool permissions (Atlassian MCP, Bash) that are irrelevant to other contributors and could grant unintended permissions if consumed by CI or other automation environments.
Add .claude/settings.local.json to .gitignore and remove this file from the PR.
- Mark as noise
nquinquenel
left a comment
nquinquenel
left a comment
There was a problem hiding this comment.
This warning should be kept if the MCP is running via a JAR, it's a big security risk (DNS rebinding). 0.0.0.0 is only necessary in Docker environment.
Also, we should probably mention something new in the README + Doc:
- If you are running the server locally on your machine, then add
-p 127.0.0.1:8443:8443 - If it's meant to run remotely, then we can keep
-p 8443:8443
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "mcp__atlassian__search", | ||
| "mcp__atlassian__getJiraIssue", | ||
| "mcp__atlassian__getJiraIssueRemoteIssueLinks", | ||
| "mcp__atlassian__getVisibleJiraProjects", | ||
| "mcp__atlassian__createJiraIssue", | ||
| "Bash(./gradlew test *)" | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you remove this please? 😅
|
|
||
| // Warn about security risk when binding to all interfaces | ||
| if ("0.0.0.0".equals(host)) { | ||
| if ("0.0.0.0".equals(host) && !httpsEnabled) { |
There was a problem hiding this comment.
Ideally, this warning should be removed only in a Docker environment, it's still valid if the MCP is run via a JAR
There was a problem hiding this comment.
Also it doesn't matter whether it's HTTP or HTTPS, this binding impacts the surface attack, TLS just encrypt the transport but doesn't reduce the surface.
2 participants
No description provided.