Harden HTTP defaults and improve stop-server fallback#136
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces HTTP endpoint security policies with scope-aware URL normalization, enabling opt-in controls for LAN HTTP binding and insecure remote HTTP. New validation helpers enforce security policies across server management, transport initialization, and UI components, with settings persisted in EditorPrefs. Changes
Sequence DiagramsequenceDiagram
participant UI as User/UI
participant McpConn as McpConnectionSection
participant McpAdv as McpAdvancedSection
participant Prefs as EditorPrefs
participant Util as HttpEndpointUtility
participant Server as ServerManagementService
participant Transport as WebSocketTransportClient
UI->>McpAdv: Toggle AllowLanHttpBind/AllowInsecureRemoteHttp
McpAdv->>Prefs: Save toggle state
McpAdv->>McpConn: OnHttpServerCommandUpdateRequested
UI->>McpConn: Click Start Server/Connect
McpConn->>Util: IsHttpLocalUrlAllowedForLaunch(url)
Util->>Prefs: Read AllowLanHttpBind
Util-->>McpConn: Result + error message
alt Local URL Allowed
McpConn->>Server: CanStartLocalServer()
Server->>Util: IsHttpLocalUrlAllowedForLaunch(url)
Util->>Prefs: Read AllowLanHttpBind
Util-->>Server: true/false
Server-->>McpConn: true/false
McpConn->>Server: StartLocalHttpServer()
else Local URL Blocked
McpConn->>UI: Show error dialog (policy violation)
end
UI->>McpConn: Click Connect (Remote)
McpConn->>Util: IsCurrentRemoteUrlAllowed()
Util->>Prefs: Read AllowInsecureRemoteHttp
Util-->>McpConn: Result + error message
alt Remote URL Allowed
McpConn->>Transport: StartAsync()
Transport->>Util: IsCurrentRemoteUrlAllowed()
Util->>Prefs: Read AllowInsecureRemoteHttp
Util-->>Transport: true/false
Transport-->>McpConn: Connected
else Remote URL Blocked
Transport->>UI: Log error, set Disconnected state
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Greptile OverviewGreptile SummaryHardens HTTP endpoint security by implementing safer defaults and explicit opt-in mechanisms for potentially risky configurations. The PR centralizes URL policy validation in Key security improvements:
Server management improvements:
Test coverage:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant UI as McpConnectionSection
participant Util as HttpEndpointUtility
participant Server as ServerManagementService
participant WS as WebSocketTransportClient
Note over User,WS: HTTP Local Launch Flow
User->>UI: Click "Start Server"
UI->>Util: IsHttpLocalUrlAllowedForLaunch(url)
Util->>Util: IsLoopbackHost(host)?
alt Loopback (localhost/127.0.0.1/::1)
Util-->>UI: Allowed ✓
else Bind-all (0.0.0.0/::)
Util->>Util: AllowLanHttpBind()?
alt Opt-in disabled
Util-->>UI: Blocked ✗ (security policy)
UI->>User: Show error dialog
else Opt-in enabled
Util-->>UI: Allowed ✓
end
end
UI->>Server: StartLocalHttpServer()
Server->>Server: Launch process
Note over User,WS: HTTP Remote Connection Flow
User->>UI: Click "Start Session" (HTTP Remote)
UI->>Util: IsCurrentRemoteUrlAllowed()
Util->>Util: Parse URL scheme
alt HTTPS scheme
Util-->>UI: Allowed ✓
else HTTP scheme
Util->>Util: AllowInsecureRemoteHttp()?
alt Opt-in disabled
Util-->>UI: Blocked ✗ (requires HTTPS)
UI->>User: Show error dialog
else Opt-in enabled
Util-->>UI: Allowed ✓
end
end
UI->>WS: ConnectAsync()
WS->>Util: IsCurrentRemoteUrlAllowed()
Util-->>WS: Validate again
WS->>WS: Establish connection
Note over Server: Server Stop with Stale PID
Server->>Server: StopLocalHttpServer(port)
Server->>Server: Read pidfile
Server->>Server: Validate PID identity
alt PID not active listener
Server->>Server: Delete stale pidfile
Server->>Server: Fall back to port heuristics
Server->>Server: Stop actual listener
else PID is listener but validation fails
Server->>Server: Fail closed (refuse stop)
end
Last reviewed commit: f423605 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue Coverage
Validation