From 311095cfddda113863ca9656cf9e99411be1cef5 Mon Sep 17 00:00:00 2001 From: henrygd Date: Wed, 18 Feb 2026 17:33:00 -0500 Subject: [PATCH] harden against docker api path traversal Validate container IDs (12-64 hex) in hub container endpoints and agent Docker requests, and build Docker URLs with escaped path segments. Add regression tests for traversal/malformed container inputs and safe endpoint construction. --- agent/docker.go | 38 +++++++++++++++- agent/docker_test.go | 98 ++++++++++++++++++++++++++++++++++++++++ internal/hub/hub.go | 6 +++ internal/hub/hub_test.go | 35 +++++++++++++- 4 files changed, 174 insertions(+), 3 deletions(-) diff --git a/agent/docker.go b/agent/docker.go index c02c038a..0962c391 100644 --- a/agent/docker.go +++ b/agent/docker.go @@ -28,6 +28,7 @@ import ( // ansiEscapePattern matches ANSI escape sequences (colors, cursor movement, etc.) // This includes CSI sequences like \x1b[...m and simple escapes like \x1b[K var ansiEscapePattern = regexp.MustCompile(`\x1b\[[0-9;]*[a-zA-Z]|\x1b\][^\x07]*\x07|\x1b[@-Z\\-_]`) +var dockerContainerIDPattern = regexp.MustCompile(`^[a-fA-F0-9]{12,64}$`) const ( // Docker API timeout in milliseconds @@ -649,9 +650,34 @@ func getDockerHost() string { return scheme + socks[0] } +func validateContainerID(containerID string) error { + if !dockerContainerIDPattern.MatchString(containerID) { + return fmt.Errorf("invalid container id") + } + return nil +} + +func buildDockerContainerEndpoint(containerID, action string, query url.Values) (string, error) { + if err := validateContainerID(containerID); err != nil { + return "", err + } + u := &url.URL{ + Scheme: "http", + Host: "localhost", + Path: fmt.Sprintf("/containers/%s/%s", url.PathEscape(containerID), action), + } + if len(query) > 0 { + u.RawQuery = query.Encode() + } + return u.String(), nil +} + // getContainerInfo fetches the inspection data for a container func (dm *dockerManager) getContainerInfo(ctx context.Context, containerID string) ([]byte, error) { - endpoint := fmt.Sprintf("http://localhost/containers/%s/json", containerID) + endpoint, err := buildDockerContainerEndpoint(containerID, "json", nil) + if err != nil { + return nil, err + } req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { return nil, err @@ -682,7 +708,15 @@ func (dm *dockerManager) getContainerInfo(ctx context.Context, containerID strin // getLogs fetches the logs for a container func (dm *dockerManager) getLogs(ctx context.Context, containerID string) (string, error) { - endpoint := fmt.Sprintf("http://localhost/containers/%s/logs?stdout=1&stderr=1&tail=%d", containerID, dockerLogsTail) + query := url.Values{ + "stdout": []string{"1"}, + "stderr": []string{"1"}, + "tail": []string{fmt.Sprintf("%d", dockerLogsTail)}, + } + endpoint, err := buildDockerContainerEndpoint(containerID, "logs", query) + if err != nil { + return "", err + } req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { return "", err diff --git a/agent/docker_test.go b/agent/docker_test.go index 4a4ad8c2..28508aeb 100644 --- a/agent/docker_test.go +++ b/agent/docker_test.go @@ -9,6 +9,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net" "net/http" "net/http/httptest" @@ -25,6 +26,37 @@ import ( var defaultCacheTimeMs = uint16(60_000) +type recordingRoundTripper struct { + statusCode int + body string + contentType string + called bool + lastPath string + lastQuery map[string]string +} + +func (rt *recordingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + rt.called = true + rt.lastPath = req.URL.EscapedPath() + rt.lastQuery = map[string]string{} + for key, values := range req.URL.Query() { + if len(values) > 0 { + rt.lastQuery[key] = values[0] + } + } + resp := &http.Response{ + StatusCode: rt.statusCode, + Status: "200 OK", + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader(rt.body)), + Request: req, + } + if rt.contentType != "" { + resp.Header.Set("Content-Type", rt.contentType) + } + return resp, nil +} + // cycleCpuDeltas cycles the CPU tracking data for a specific cache time interval func (dm *dockerManager) cycleCpuDeltas(cacheTimeMs uint16) { // Clear the CPU tracking maps for this cache time interval @@ -116,6 +148,72 @@ func TestCalculateMemoryUsage(t *testing.T) { } } +func TestBuildDockerContainerEndpoint(t *testing.T) { + t.Run("valid container ID builds escaped endpoint", func(t *testing.T) { + endpoint, err := buildDockerContainerEndpoint("0123456789ab", "json", nil) + require.NoError(t, err) + assert.Equal(t, "http://localhost/containers/0123456789ab/json", endpoint) + }) + + t.Run("invalid container ID is rejected", func(t *testing.T) { + _, err := buildDockerContainerEndpoint("../../version", "json", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid container id") + }) +} + +func TestContainerDetailsRequestsValidateContainerID(t *testing.T) { + rt := &recordingRoundTripper{ + statusCode: 200, + body: `{"Config":{"Env":["SECRET=1"]}}`, + } + dm := &dockerManager{ + client: &http.Client{Transport: rt}, + } + + _, err := dm.getContainerInfo(context.Background(), "../version") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid container id") + assert.False(t, rt.called, "request should be rejected before dispatching to Docker API") +} + +func TestContainerDetailsRequestsUseExpectedDockerPaths(t *testing.T) { + t.Run("container info uses container json endpoint", func(t *testing.T) { + rt := &recordingRoundTripper{ + statusCode: 200, + body: `{"Config":{"Env":["SECRET=1"]},"Name":"demo"}`, + } + dm := &dockerManager{ + client: &http.Client{Transport: rt}, + } + + body, err := dm.getContainerInfo(context.Background(), "0123456789ab") + require.NoError(t, err) + assert.True(t, rt.called) + assert.Equal(t, "/containers/0123456789ab/json", rt.lastPath) + assert.NotContains(t, string(body), "SECRET=1", "sensitive env vars should be removed") + }) + + t.Run("container logs uses expected endpoint and query params", func(t *testing.T) { + rt := &recordingRoundTripper{ + statusCode: 200, + body: "line1\nline2\n", + } + dm := &dockerManager{ + client: &http.Client{Transport: rt}, + } + + logs, err := dm.getLogs(context.Background(), "abcdef123456") + require.NoError(t, err) + assert.True(t, rt.called) + assert.Equal(t, "/containers/abcdef123456/logs", rt.lastPath) + assert.Equal(t, "1", rt.lastQuery["stdout"]) + assert.Equal(t, "1", rt.lastQuery["stderr"]) + assert.Equal(t, "200", rt.lastQuery["tail"]) + assert.Equal(t, "line1\nline2\n", logs) + }) +} + func TestValidateCpuPercentage(t *testing.T) { tests := []struct { name string diff --git a/internal/hub/hub.go b/internal/hub/hub.go index bc58c76f..3a5b9504 100644 --- a/internal/hub/hub.go +++ b/internal/hub/hub.go @@ -9,6 +9,7 @@ import ( "net/url" "os" "path" + "regexp" "strings" "time" @@ -41,6 +42,8 @@ type Hub struct { appURL string } +var containerIDPattern = regexp.MustCompile(`^[a-fA-F0-9]{12,64}$`) + // NewHub creates a new Hub instance with default configuration func NewHub(app core.App) *Hub { hub := &Hub{} @@ -461,6 +464,9 @@ func (h *Hub) containerRequestHandler(e *core.RequestEvent, fetchFunc func(*syst if systemID == "" || containerID == "" { return e.JSON(http.StatusBadRequest, map[string]string{"error": "system and container parameters are required"}) } + if !containerIDPattern.MatchString(containerID) { + return e.JSON(http.StatusBadRequest, map[string]string{"error": "invalid container parameter"}) + } system, err := h.sm.GetSystem(systemID) if err != nil { diff --git a/internal/hub/hub_test.go b/internal/hub/hub_test.go index dafc1548..3bdf6ac0 100644 --- a/internal/hub/hub_test.go +++ b/internal/hub/hub_test.go @@ -545,7 +545,7 @@ func TestApiRoutesAuthentication(t *testing.T) { { Name: "GET /containers/logs - with auth but invalid system should fail", Method: http.MethodGet, - URL: "/api/beszel/containers/logs?system=invalid-system&container=test-container", + URL: "/api/beszel/containers/logs?system=invalid-system&container=0123456789ab", Headers: map[string]string{ "Authorization": userToken, }, @@ -553,6 +553,39 @@ func TestApiRoutesAuthentication(t *testing.T) { ExpectedContent: []string{"system not found"}, TestAppFactory: testAppFactory, }, + { + Name: "GET /containers/logs - traversal container should fail validation", + Method: http.MethodGet, + URL: "/api/beszel/containers/logs?system=" + system.Id + "&container=..%2F..%2Fversion", + Headers: map[string]string{ + "Authorization": userToken, + }, + ExpectedStatus: 400, + ExpectedContent: []string{"invalid container parameter"}, + TestAppFactory: testAppFactory, + }, + { + Name: "GET /containers/info - traversal container should fail validation", + Method: http.MethodGet, + URL: "/api/beszel/containers/info?system=" + system.Id + "&container=../../version?x=", + Headers: map[string]string{ + "Authorization": userToken, + }, + ExpectedStatus: 400, + ExpectedContent: []string{"invalid container parameter"}, + TestAppFactory: testAppFactory, + }, + { + Name: "GET /containers/info - non-hex container should fail validation", + Method: http.MethodGet, + URL: "/api/beszel/containers/info?system=" + system.Id + "&container=container_name", + Headers: map[string]string{ + "Authorization": userToken, + }, + ExpectedStatus: 400, + ExpectedContent: []string{"invalid container parameter"}, + TestAppFactory: testAppFactory, + }, // Auth Optional Routes - Should work without authentication {