mirror of
https://github.com/henrygd/beszel.git
synced 2026-03-27 07:56:19 +01:00
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.
This commit is contained in:
@@ -28,6 +28,7 @@ import (
|
|||||||
// ansiEscapePattern matches ANSI escape sequences (colors, cursor movement, etc.)
|
// ansiEscapePattern matches ANSI escape sequences (colors, cursor movement, etc.)
|
||||||
// This includes CSI sequences like \x1b[...m and simple escapes like \x1b[K
|
// 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 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 (
|
const (
|
||||||
// Docker API timeout in milliseconds
|
// Docker API timeout in milliseconds
|
||||||
@@ -649,9 +650,34 @@ func getDockerHost() string {
|
|||||||
return scheme + socks[0]
|
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
|
// getContainerInfo fetches the inspection data for a container
|
||||||
func (dm *dockerManager) getContainerInfo(ctx context.Context, containerID string) ([]byte, error) {
|
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)
|
req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
@@ -682,7 +708,15 @@ func (dm *dockerManager) getContainerInfo(ctx context.Context, containerID strin
|
|||||||
|
|
||||||
// getLogs fetches the logs for a container
|
// getLogs fetches the logs for a container
|
||||||
func (dm *dockerManager) getLogs(ctx context.Context, containerID string) (string, error) {
|
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)
|
req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", err
|
return "", err
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import (
|
|||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"io"
|
||||||
"net"
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
@@ -25,6 +26,37 @@ import (
|
|||||||
|
|
||||||
var defaultCacheTimeMs = uint16(60_000)
|
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
|
// cycleCpuDeltas cycles the CPU tracking data for a specific cache time interval
|
||||||
func (dm *dockerManager) cycleCpuDeltas(cacheTimeMs uint16) {
|
func (dm *dockerManager) cycleCpuDeltas(cacheTimeMs uint16) {
|
||||||
// Clear the CPU tracking maps for this cache time interval
|
// 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) {
|
func TestValidateCpuPercentage(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import (
|
|||||||
"net/url"
|
"net/url"
|
||||||
"os"
|
"os"
|
||||||
"path"
|
"path"
|
||||||
|
"regexp"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -41,6 +42,8 @@ type Hub struct {
|
|||||||
appURL string
|
appURL string
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var containerIDPattern = regexp.MustCompile(`^[a-fA-F0-9]{12,64}$`)
|
||||||
|
|
||||||
// NewHub creates a new Hub instance with default configuration
|
// NewHub creates a new Hub instance with default configuration
|
||||||
func NewHub(app core.App) *Hub {
|
func NewHub(app core.App) *Hub {
|
||||||
hub := &Hub{}
|
hub := &Hub{}
|
||||||
@@ -461,6 +464,9 @@ func (h *Hub) containerRequestHandler(e *core.RequestEvent, fetchFunc func(*syst
|
|||||||
if systemID == "" || containerID == "" {
|
if systemID == "" || containerID == "" {
|
||||||
return e.JSON(http.StatusBadRequest, map[string]string{"error": "system and container parameters are required"})
|
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)
|
system, err := h.sm.GetSystem(systemID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -545,7 +545,7 @@ func TestApiRoutesAuthentication(t *testing.T) {
|
|||||||
{
|
{
|
||||||
Name: "GET /containers/logs - with auth but invalid system should fail",
|
Name: "GET /containers/logs - with auth but invalid system should fail",
|
||||||
Method: http.MethodGet,
|
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{
|
Headers: map[string]string{
|
||||||
"Authorization": userToken,
|
"Authorization": userToken,
|
||||||
},
|
},
|
||||||
@@ -553,6 +553,39 @@ func TestApiRoutesAuthentication(t *testing.T) {
|
|||||||
ExpectedContent: []string{"system not found"},
|
ExpectedContent: []string{"system not found"},
|
||||||
TestAppFactory: testAppFactory,
|
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
|
// Auth Optional Routes - Should work without authentication
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user