Skip to content

Commit 2aaa73a

Browse files
authored
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.
1 parent b17bf34 commit 2aaa73a

File tree

3 files changed

+67
-17
lines changed

3 files changed

+67
-17
lines changed

docs/spec/openapi/paths/server_info.yaml

-8
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,3 @@ get:
1515
responses:
1616
'200':
1717
$ref: '../components/responses/200_server_info.yaml'
18-
post:
19-
summary: Get general server information
20-
operationId: postServerHealth
21-
tags:
22-
- server info
23-
responses:
24-
'200':
25-
$ref: '../components/responses/200_server_info.yaml'

internal/beater/api/mux_root_test.go

+52-7
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@ package api
2020
import (
2121
"encoding/json"
2222
"net/http"
23+
"net/http/httptest"
2324
"testing"
2425

2526
"github.com/stretchr/testify/assert"
2627
"github.com/stretchr/testify/require"
2728

2829
"github.com/elastic/apm-server/internal/beater/config"
2930
"github.com/elastic/apm-server/internal/beater/headers"
31+
"github.com/elastic/apm-server/internal/beater/monitoringtest"
3032
"github.com/elastic/apm-server/internal/beater/request"
3133
"github.com/elastic/apm-server/internal/version"
3234
)
@@ -36,7 +38,7 @@ func TestRootHandler_AuthorizationMiddleware(t *testing.T) {
3638
cfg.AgentAuth.SecretToken = "1234"
3739

3840
t.Run("No auth", func(t *testing.T) {
39-
rec, err := requestToMuxerWithPattern(cfg, RootPath)
41+
rec, err := requestToMuxerWithHeader(cfg, RootPath, http.MethodGet, nil)
4042
require.NoError(t, err)
4143
assert.Equal(t, http.StatusOK, rec.Code)
4244
assert.Empty(t, rec.Body.String())
@@ -63,10 +65,53 @@ func TestRootHandler_PanicMiddleware(t *testing.T) {
6365
}
6466

6567
func TestRootHandler_MonitoringMiddleware(t *testing.T) {
66-
testMonitoringMiddleware(t, "/", map[string]any{
67-
"http.server." + string(request.IDRequestCount): 1,
68-
"http.server." + string(request.IDResponseCount): 1,
69-
"http.server." + string(request.IDResponseValidCount): 1,
70-
"http.server." + string(request.IDResponseValidOK): 1,
71-
})
68+
validMethodMetrics := map[string]any{
69+
"http.server." + string(request.IDRequestCount): 1,
70+
"http.server." + string(request.IDResponseCount): 1,
71+
"http.server." + string(request.IDResponseValidCount): 1,
72+
"http.server." + string(request.IDResponseValidOK): 1,
73+
"apm-server.root." + string(request.IDRequestCount): 1,
74+
"apm-server.root." + string(request.IDResponseCount): 1,
75+
"apm-server.root." + string(request.IDResponseValidCount): 1,
76+
"apm-server.root." + string(request.IDResponseValidOK): 1,
77+
}
78+
invalidMethodMetrics := map[string]any{
79+
"http.server." + string(request.IDRequestCount): 1,
80+
"http.server." + string(request.IDResponseCount): 1,
81+
"http.server." + string(request.IDResponseErrorsCount): 1,
82+
"http.server." + string(request.IDResponseErrorsMethodNotAllowed): 1,
83+
"apm-server.root." + string(request.IDRequestCount): 1,
84+
"apm-server.root." + string(request.IDResponseCount): 1,
85+
"apm-server.root." + string(request.IDResponseErrorsCount): 1,
86+
"apm-server.root." + string(request.IDResponseErrorsMethodNotAllowed): 1,
87+
}
88+
for _, tc := range []struct {
89+
method string
90+
wantMetrics map[string]any
91+
}{
92+
{
93+
method: http.MethodGet,
94+
wantMetrics: validMethodMetrics,
95+
},
96+
{
97+
method: http.MethodHead,
98+
wantMetrics: validMethodMetrics,
99+
},
100+
{
101+
method: http.MethodPut,
102+
wantMetrics: invalidMethodMetrics,
103+
},
104+
{
105+
method: http.MethodPost,
106+
wantMetrics: invalidMethodMetrics,
107+
},
108+
} {
109+
t.Run(tc.method, func(t *testing.T) {
110+
h, reader := newTestMux(t, config.DefaultConfig())
111+
req := httptest.NewRequest(tc.method, "/", nil)
112+
h.ServeHTTP(httptest.NewRecorder(), req)
113+
114+
monitoringtest.ExpectContainOtelMetrics(t, reader, tc.wantMetrics)
115+
})
116+
}
72117
}

internal/beater/api/root/handler.go

+15-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
package root
1919

2020
import (
21+
"errors"
22+
"net/http"
2123
"time"
2224

2325
"github.com/elastic/beats/v7/libbeat/version"
@@ -38,10 +40,21 @@ type HandlerConfig struct {
3840

3941
// Handler returns error if route does not exist,
4042
// otherwise returns information about the server. The detail level differs for authenticated and anonymous requests.
41-
// TODO: only allow GET, HEAD requests (breaking change)
4243
func Handler(cfg HandlerConfig) request.Handler {
43-
4444
return func(c *request.Context) {
45+
// Only allow GET, HEAD requests
46+
switch c.Request.Method {
47+
case http.MethodGet, http.MethodHead:
48+
default:
49+
c.Result.SetWithError(
50+
request.IDResponseErrorsMethodNotAllowed,
51+
// include a verbose error message to alert users about a common misconfiguration
52+
errors.New("this is the server information endpoint; did you mean to send data to another endpoint instead?"),
53+
)
54+
c.WriteResult()
55+
return
56+
}
57+
4558
serverInfo := mapstr.M{
4659
"build_date": version.BuildTime().Format(time.RFC3339),
4760
"build_sha": version.Commit(),

0 commit comments

Comments
 (0)