Added the possibility to block the MCP server from outside.#18
Added the possibility to block the MCP server from outside.#18romankassebaum wants to merge 1 commit into
Conversation
cmgeuze
left a comment
There was a problem hiding this comment.
Hi Roman,
Thanks for this! Binding the server to a single host is a really useful addition, and the hooks are a nice approach. A few things I'd like to sort out before we merge.
-
The allow/deny check ends up in two places that can disagree. InternalAllowTool is applied when FTools gets built (constructor and now ListTools), but CallTool just reads FTools as it stands and never rebuilds. So if the filter is dynamic, a call can run against a stale list. I think the cleaner fix is to not rebuild FTools at all: register every tool once, then apply InternalAllowTool at the two read points, skip disallowed tools in the list response and reject them in CallTool. That keeps list and call consistent and drops the per-list rebuild below.
-
Related to that, ListTools now clears and recreates the whole dictionary on every call. With the approach above that rebuild isn't needed anymore.
-
When InternalVerify returns False it just Exits, and Indy defaults ResponseNo to 200, so a blocked request gets an empty 200 instead of a 403. Could you document that an override needs to set ResponseInfo itself, and maybe give the base a safe default?
-
In Start, GStack.ResolveHost runs before FHTTPServer.Active, where GStack can still be nil and cause an AV when OnlyHost is on. That same block also reads FSettings.Host without the Assigned(FSettings) guard you use just above it. Could you guard both, and resolve once the stack is up? It's also hardcoded to IPv4.
-
Small one: OnlyHost can only be set in code right now, exposing it as a [Server] setting would make it usable out of the box.
Thanks!
Sometimes it is necessary to block the MSC server from outside.