fix(http): serve /status/diagnostics only on the internal interface - #4442
Open
stevenvegt wants to merge 2 commits into
Open
fix(http): serve /status/diagnostics only on the internal interface#4442stevenvegt wants to merge 2 commits into
stevenvegt wants to merge 2 commits into
Conversation
The diagnostics endpoint discloses the exact software version and git commit (which answers "is this node patched" for version-scoped advisories), the gRPC peer list with remote addresses and node DIDs, and data store counts. The deployment documentation lists it as an internal endpoint, but it was served on the public interface as well because it shares its first path segment with /status, and interface binding resolves on the first path segment only. Requests reaching the path through any interface other than the internal one are now refused with 403. The interface is identified by comparing the port of the connection's local address (stored by net/http under http.LocalAddrContextKey) against the configured internal address, since interfaces always listen on distinct ports. /status itself deliberately stays public: external load balancers and uptime monitors probe it as a liveness signal, and breaking those in a security patch is a bad trade. A 403 with an explicit message is returned rather than a 404. Hiding the endpoint gains nothing (its existence is public knowledge given the open source), and the message tells an operator whose monitor probes diagnostics exactly why it broke. Assisted-by: AI
stevenvegt
requested review from
Dirklectisch,
JorisHeadease,
gerardsn,
reinkrul and
woutslakhorst
as code owners
July 30, 2026 15:28
Assisted-by: AI
Contributor
1 new issue
|
Contributor
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (1)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
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.
/status/diagnosticsis now refused with 403 on any interface other than the internal one. The endpoint discloses the exact software version and git commit (which answers "is this node patched" for version-scoped advisories), the gRPC peer list including remote addresses, node DIDs and authentication state, and DID document/credential store counts. The deployment documentation has always listed it under internal endpoints, but it was actually served publicly as well.Why middleware instead of binding the route internally: interface binding (
MultiEcho) resolves on the first path segment only and rejects subpath binds, so/statusand/status/diagnosticscannot be bound to different interfaces through the bind table; the route is registered on every interface that serves/status. Teaching the bind table longest-prefix subpath matching is the cleaner end state but changes how every route resolves to an interface, which is a refactor for a minor release, not a security patch. When that refactor lands, this path should become a regular internal-only bind and the middleware deleted. This is documented at thediagnosticsPathconstant.How the interface is identified: Go's net/http server stores the connection's local address in the request context under
http.LocalAddrContextKey. Interfaces always listen on distinct ports (binding the same port twice fails at startup), so comparing the local address port with the configured internal address port identifies the arrival interface. Host is deliberately not compared, since the configured bind host (:8081) need not equal the connection's concrete local IP. If public and internal are configured to the same address there is one server and the ports always match, honoring that explicit setup. A missing or unparseable local address refuses the request (fail closed), though net/http always sets it.Why 403 with a message instead of 404: hiding the endpoint gains nothing, its existence is public knowledge given the open source, and any behavior change equally reveals the patch level. The explicit "only available on the internal interface" message tells an operator whose external monitor probes diagnostics exactly why it broke after a patch upgrade.
/statusitself deliberately stays public as the liveness signal for load balancers and uptime monitors.Testing: new
TestEngine_DiagnosticsInternalOnlystarts the engine on both interfaces with routes registered as the status engine does, and asserts/statusreturns 200 on the public interface,/status/diagnosticsreturns 200 with its body on the internal interface, and/status/diagnosticsreturns 403 without the diagnostics body on the public interface. Before the fix the public request returned 200 with the full body.The monitoring documentation now states the endpoint is internal-only and why.