From c8a78efbcd99f6f0e3696dd7d5b0a7e29d9e7013 Mon Sep 17 00:00:00 2001 From: Carson Ip Date: Wed, 5 Mar 2025 14:19:49 +0000 Subject: [PATCH] Change server information endpoint `/` to only accept GET and HEAD requests (#15976) Breaking change to change server information endpoint / to only accept GET and HEAD requests, and return 405 Method Not Allowed otherwise. This will surface any agent misconfiguration, e.g. configuring otlphttp to send to / instead of /v1/traces. (cherry picked from commit 2aaa73a56538c97c83f4b9f73e0c516631d04b42) --- docs/spec/openapi/paths/server_info.yaml | 8 ---- internal/beater/api/mux_root_test.go | 59 +++++++++++++++++++++--- internal/beater/api/root/handler.go | 17 ++++++- 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/docs/spec/openapi/paths/server_info.yaml b/docs/spec/openapi/paths/server_info.yaml index a70149acfce..9a4a5365422 100644 --- a/docs/spec/openapi/paths/server_info.yaml +++ b/docs/spec/openapi/paths/server_info.yaml @@ -15,11 +15,3 @@ get: responses: '200': $ref: '../components/responses/200_server_info.yaml' -post: - summary: Get general server information - operationId: postServerHealth - tags: - - server info - responses: - '200': - $ref: '../components/responses/200_server_info.yaml' diff --git a/internal/beater/api/mux_root_test.go b/internal/beater/api/mux_root_test.go index 2b2af7dcc47..5c814786abd 100644 --- a/internal/beater/api/mux_root_test.go +++ b/internal/beater/api/mux_root_test.go @@ -20,6 +20,7 @@ package api import ( "encoding/json" "net/http" + "net/http/httptest" "testing" "github.com/stretchr/testify/assert" @@ -27,6 +28,7 @@ import ( "github.com/elastic/apm-server/internal/beater/config" "github.com/elastic/apm-server/internal/beater/headers" + "github.com/elastic/apm-server/internal/beater/monitoringtest" "github.com/elastic/apm-server/internal/beater/request" "github.com/elastic/apm-server/internal/version" ) @@ -36,7 +38,7 @@ func TestRootHandler_AuthorizationMiddleware(t *testing.T) { cfg.AgentAuth.SecretToken = "1234" t.Run("No auth", func(t *testing.T) { - rec, err := requestToMuxerWithPattern(cfg, RootPath) + rec, err := requestToMuxerWithHeader(cfg, RootPath, http.MethodGet, nil) require.NoError(t, err) assert.Equal(t, http.StatusOK, rec.Code) assert.Empty(t, rec.Body.String()) @@ -63,10 +65,53 @@ func TestRootHandler_PanicMiddleware(t *testing.T) { } func TestRootHandler_MonitoringMiddleware(t *testing.T) { - testMonitoringMiddleware(t, "/", map[string]any{ - "http.server." + string(request.IDRequestCount): 1, - "http.server." + string(request.IDResponseCount): 1, - "http.server." + string(request.IDResponseValidCount): 1, - "http.server." + string(request.IDResponseValidOK): 1, - }) + validMethodMetrics := map[string]any{ + "http.server." + string(request.IDRequestCount): 1, + "http.server." + string(request.IDResponseCount): 1, + "http.server." + string(request.IDResponseValidCount): 1, + "http.server." + string(request.IDResponseValidOK): 1, + "apm-server.root." + string(request.IDRequestCount): 1, + "apm-server.root." + string(request.IDResponseCount): 1, + "apm-server.root." + string(request.IDResponseValidCount): 1, + "apm-server.root." + string(request.IDResponseValidOK): 1, + } + invalidMethodMetrics := map[string]any{ + "http.server." + string(request.IDRequestCount): 1, + "http.server." + string(request.IDResponseCount): 1, + "http.server." + string(request.IDResponseErrorsCount): 1, + "http.server." + string(request.IDResponseErrorsMethodNotAllowed): 1, + "apm-server.root." + string(request.IDRequestCount): 1, + "apm-server.root." + string(request.IDResponseCount): 1, + "apm-server.root." + string(request.IDResponseErrorsCount): 1, + "apm-server.root." + string(request.IDResponseErrorsMethodNotAllowed): 1, + } + for _, tc := range []struct { + method string + wantMetrics map[string]any + }{ + { + method: http.MethodGet, + wantMetrics: validMethodMetrics, + }, + { + method: http.MethodHead, + wantMetrics: validMethodMetrics, + }, + { + method: http.MethodPut, + wantMetrics: invalidMethodMetrics, + }, + { + method: http.MethodPost, + wantMetrics: invalidMethodMetrics, + }, + } { + t.Run(tc.method, func(t *testing.T) { + h, reader := newTestMux(t, config.DefaultConfig()) + req := httptest.NewRequest(tc.method, "/", nil) + h.ServeHTTP(httptest.NewRecorder(), req) + + monitoringtest.ExpectContainOtelMetrics(t, reader, tc.wantMetrics) + }) + } } diff --git a/internal/beater/api/root/handler.go b/internal/beater/api/root/handler.go index 2ef21a67bc4..adce51c3421 100644 --- a/internal/beater/api/root/handler.go +++ b/internal/beater/api/root/handler.go @@ -18,6 +18,8 @@ package root import ( + "errors" + "net/http" "time" "github.com/elastic/beats/v7/libbeat/version" @@ -38,10 +40,21 @@ type HandlerConfig struct { // Handler returns error if route does not exist, // otherwise returns information about the server. The detail level differs for authenticated and anonymous requests. -// TODO: only allow GET, HEAD requests (breaking change) func Handler(cfg HandlerConfig) request.Handler { - return func(c *request.Context) { + // Only allow GET, HEAD requests + switch c.Request.Method { + case http.MethodGet, http.MethodHead: + default: + c.Result.SetWithError( + request.IDResponseErrorsMethodNotAllowed, + // include a verbose error message to alert users about a common misconfiguration + errors.New("this is the server information endpoint; did you mean to send data to another endpoint instead?"), + ) + c.WriteResult() + return + } + serverInfo := mapstr.M{ "build_date": version.BuildTime().Format(time.RFC3339), "build_sha": version.Commit(),