diff --git a/Makefile b/Makefile index 3da2c03a..b04c0e5a 100644 --- a/Makefile +++ b/Makefile @@ -21,6 +21,10 @@ fake_assets: test: fake_assets generate go test -cover -race -count 1 -timeout 40s ./... +.PHONY: test-update +test-update: fake_assets generate + go test -cover -race -count 1 -timeout 5s ./... -- -- -u + .PHONY: build build: dist generate CGO_ENABLED=0 go build -ldflags "-s -w -X github.com/amir20/dozzle/internal/support/cli.Version=local" diff --git a/internal/web/__snapshots__/web.snapshot b/internal/web/__snapshots__/web.snapshot index c4ee6e9d..91501950 100644 --- a/internal/web/__snapshots__/web.snapshot +++ b/internal/web/__snapshots__/web.snapshot @@ -119,6 +119,27 @@ X-Accel-Buffering: no event: containers-changed data: [] +/* snapshot: Test_handler_streamEvents_filtered */ +HTTP/1.1 200 OK +Connection: close +Cache-Control: no-transform +Cache-Control: no-cache +Connection: keep-alive +Content-Security-Policy: default-src 'self' 'wasm-unsafe-eval' blob: https://cdn.jsdelivr.net https://*.duckdb.org; style-src 'self' 'unsafe-inline' blob:; img-src 'self' data:; font-src 'self' data:; +Content-Type: text/event-stream +X-Accel-Buffering: no + +event: containers-changed +data: [{"id":"visible","name":"visible","image":"test","command":"","created":"0001-01-01T00:00:00Z","startedAt":"0001-01-01T00:00:00Z","finishedAt":"0001-01-01T00:00:00Z","state":"","stats":[],"memoryLimit":0,"cpuLimit":0}] + + +event: containers-changed +data: [{"id":"visible","name":"visible","image":"test","command":"","created":"0001-01-01T00:00:00Z","startedAt":"0001-01-01T00:00:00Z","finishedAt":"0001-01-01T00:00:00Z","state":"","stats":[],"memoryLimit":0,"cpuLimit":0}] + + +event: container-event +data: {"name":"start","host":"localhost","actorId":"visible","time":"0001-01-01T00:00:00Z"} + /* snapshot: Test_handler_streamEvents_happy */ HTTP/1.1 200 OK Connection: close @@ -130,11 +151,11 @@ Content-Type: text/event-stream X-Accel-Buffering: no event: containers-changed -data: [] +data: [{"id":"1234","name":"test","image":"test","command":"","created":"0001-01-01T00:00:00Z","startedAt":"0001-01-01T00:00:00Z","finishedAt":"0001-01-01T00:00:00Z","state":"","stats":[],"memoryLimit":0,"cpuLimit":0}] event: containers-changed -data: [] +data: [{"id":"1234","name":"test","image":"test","command":"","created":"0001-01-01T00:00:00Z","startedAt":"0001-01-01T00:00:00Z","finishedAt":"0001-01-01T00:00:00Z","state":"","stats":[],"memoryLimit":0,"cpuLimit":0}] event: container-event diff --git a/internal/web/events.go b/internal/web/events.go index 676b25ce..3dffac3f 100644 --- a/internal/web/events.go +++ b/internal/web/events.go @@ -38,6 +38,42 @@ func (h *handler) streamEvents(w http.ResponseWriter, r *http.Request) { allContainers, errors := h.hostService.ListAllContainers(userLabels) + // per-host set of container IDs the caller may see, so stat/event channels stay filtered like the list + visibleByHost := make(map[string]map[string]struct{}) + setVisible := func(host string, containers []container.Container) { + ids := make(map[string]struct{}, len(containers)) + for _, c := range containers { + ids[c.ID] = struct{}{} + } + visibleByHost[host] = ids + } + isVisible := func(host, id string) bool { + if host != "" { + ids, ok := visibleByHost[host] + if !ok { + return false + } + _, ok = ids[id] + return ok + } + // container-stat payloads carry no host, so fall back to scanning all hosts + for _, ids := range visibleByHost { + if _, ok := ids[id]; ok { + return true + } + } + return false + } + + for _, c := range allContainers { + ids, ok := visibleByHost[c.Host] + if !ok { + ids = make(map[string]struct{}) + visibleByHost[c.Host] = ids + } + ids[c.ID] = struct{}{} + } + for _, err := range errors { log.Warn().Err(err).Msg("error listing containers") if hostNotAvailableError, ok := err.(*docker_support.HostUnavailableError); ok { @@ -61,6 +97,9 @@ func (h *handler) streamEvents(w http.ResponseWriter, r *http.Request) { return } case stat := <-stats: + if !isVisible("", stat.ID) { + continue + } if err := sseWriter.Event("container-stat", stat); err != nil { log.Error().Err(err).Msg("error writing event to event stream") return @@ -72,13 +111,25 @@ func (h *handler) streamEvents(w http.ResponseWriter, r *http.Request) { log.Trace().Str("event", event.Name).Str("id", event.ActorID).Msg("container event from store") switch event.Name { case "start", "die", "destroy", "rename", "pause", "unpause": + var refreshed []container.Container if event.Name == "start" || event.Name == "rename" { if containers, err := h.hostService.ListContainersForHost(event.Host, userLabels); err == nil { log.Debug().Str("host", event.Host).Int("count", len(containers)).Msg("updating containers for host") - if err := sseWriter.Event("containers-changed", containers); err != nil { - log.Error().Err(err).Msg("error writing containers to event stream") - return - } + setVisible(event.Host, containers) + refreshed = containers + } + } + + // gate both containers-changed and the raw event so out-of-scope + // containers don't leak via payload or as a timing side-channel + if !isVisible(event.Host, event.ActorID) { + continue + } + + if refreshed != nil { + if err := sseWriter.Event("containers-changed", refreshed); err != nil { + log.Error().Err(err).Msg("error writing containers to event stream") + return } } @@ -88,11 +139,17 @@ func (h *handler) streamEvents(w http.ResponseWriter, r *http.Request) { } case "update": + if event.Container == nil || !isVisible(event.Host, event.Container.ID) { + continue + } if err := sseWriter.Event("container-updated", event.Container); err != nil { log.Error().Err(err).Msg("error writing event to event stream") return } case "health_status: healthy", "health_status: unhealthy": + if !isVisible(event.Host, event.ActorID) { + continue + } healthy := "unhealthy" if event.Name == "health_status: healthy" { healthy = "healthy" diff --git a/internal/web/events_test.go b/internal/web/events_test.go index a32c1697..48ad7bcf 100644 --- a/internal/web/events_test.go +++ b/internal/web/events_test.go @@ -24,7 +24,9 @@ func Test_handler_streamEvents_happy(t *testing.T) { mockedClient := new(MockedClient) - mockedClient.On("ListContainers", mock.Anything, mock.Anything).Return([]container.Container{}, nil) + mockedClient.On("ListContainers", mock.Anything, mock.Anything).Return([]container.Container{ + {ID: "1234", Name: "test", Image: "test", Host: "localhost"}, + }, nil) mockedClient.On("ContainerEvents", mock.Anything, mock.AnythingOfType("chan<- container.ContainerEvent")).Return(nil).Run(func(args mock.Arguments) { messages := args.Get(1).(chan<- container.ContainerEvent) @@ -66,3 +68,62 @@ func Test_handler_streamEvents_happy(t *testing.T) { abide.AssertHTTPResponse(t, t.Name(), rr.Result()) mockedClient.AssertExpectations(t) } + +// Test_handler_streamEvents_filtered asserts that container-event for a container +// outside the caller's label scope is not forwarded. The mocked ListContainers +// only ever returns the in-scope container ("visible"), so the out-of-scope +// container ("secret") is never added to the visible set and its lifecycle event +// must be dropped. See GHSA-xcw9-qmmf-vqxj. +func Test_handler_streamEvents_filtered(t *testing.T) { + context, cancel := context.WithCancel(context.Background()) + req, err := http.NewRequestWithContext(context, "GET", "/api/events/stream", nil) + require.NoError(t, err, "NewRequest should not return an error.") + + mockedClient := new(MockedClient) + + mockedClient.On("ListContainers", mock.Anything, mock.Anything).Return([]container.Container{ + {ID: "visible", Name: "visible", Image: "test", Host: "localhost"}, + }, nil) + mockedClient.On("ContainerEvents", mock.Anything, mock.AnythingOfType("chan<- container.ContainerEvent")).Return(nil).Run(func(args mock.Arguments) { + messages := args.Get(1).(chan<- container.ContainerEvent) + + time.Sleep(50 * time.Millisecond) + // out-of-scope container: must be dropped + messages <- container.ContainerEvent{ + Name: "start", + ActorID: "secret", + Host: "localhost", + } + // in-scope container: must be forwarded + messages <- container.ContainerEvent{ + Name: "start", + ActorID: "visible", + Host: "localhost", + } + time.Sleep(50 * time.Millisecond) + cancel() + }) + + mockedClient.On("FindContainer", mock.Anything, mock.Anything).Return(container.Container{ + ID: "visible", + Name: "visible", + Image: "test", + Stats: utils.NewRingBuffer[container.ContainerStat](300), + }, nil) + + mockedClient.On("Host").Return(container.Host{ + ID: "localhost", + }) + + manager := docker_support.NewRetriableClientManager(nil, 3*time.Second, tls.Certificate{}, docker_support.NewDockerClientService(mockedClient, container.ContainerLabels{})) + multiHostService := docker_support.NewMultiHostService(manager, 3*time.Second) + + server := CreateServer(multiHostService, nil, Config{Base: "/", Authorization: Authorization{Provider: NONE}}) + + handler := server.Handler + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + abide.AssertHTTPResponse(t, t.Name(), rr.Result()) + mockedClient.AssertExpectations(t) +}