From 1e2705d332f29408bed00d21013f0b432eb2ccb8 Mon Sep 17 00:00:00 2001 From: wollomatic Date: Sat, 25 Jul 2026 18:02:10 +0200 Subject: [PATCH 1/8] Refresh Docker label allowlist before denying --- cmd/socket-proxy/handlehttprequest.go | 10 +++++ internal/config/config.go | 24 ++++++++++ internal/config/config_test.go | 65 +++++++++++++++++++++++++++ 3 files changed, 99 insertions(+) diff --git a/cmd/socket-proxy/handlehttprequest.go b/cmd/socket-proxy/handlehttprequest.go index f366135..c00bce1 100644 --- a/cmd/socket-proxy/handlehttprequest.go +++ b/cmd/socket-proxy/handlehttprequest.go @@ -74,6 +74,16 @@ func determineAllowList(r *http.Request) (config.AllowList, bool) { slog.Warn("cannot get valid IP address for client allowlist check", "reason", err, "method", r.Method, "URL", r.URL, "client", r.RemoteAddr) // #nosec G706 - structured logging (slog) safely encodes values } if !allowedIP { + // A container can send its first request before Docker's start event has + // updated the per-container allowlist. Refresh it once before denying + // the request, while still failing closed if Docker cannot be queried. + if cfg.ProxyContainerName != "" { + if err := cfg.RefreshAllowLists(r.Context()); err != nil { + slog.Warn("failed to refresh per-container allowlists", "error", err) + } else if allowList, found := cfg.AllowLists.FindByIP(clientIPStr); found { + return allowList, true + } + } return config.AllowList{}, false } } diff --git a/internal/config/config.go b/internal/config/config.go index f929bf1..1e4187a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -44,6 +44,7 @@ const ( ) type Config struct { + allowListsRefreshMutex sync.Mutex // serialises synchronous allowlist refreshes triggered by requests AllowLists *AllowListRegistry AllowFrom []string AllowHealthcheck bool @@ -367,6 +368,29 @@ func (cfg *Config) UpdateAllowLists() { } } +// RefreshAllowLists updates the per-container allowlists from Docker. It is +// used when a request arrives before its corresponding Docker start event has +// been processed. +func (cfg *Config) RefreshAllowLists(ctx context.Context) error { + cfg.allowListsRefreshMutex.Lock() + defer cfg.allowListsRefreshMutex.Unlock() + + dockerClient, err := client.NewClientWithOpts( + client.WithHost("unix://"+cfg.SocketPath), + client.WithAPIVersionNegotiation(), + ) + if err != nil { + return err + } + defer func() { + if closeErr := dockerClient.Close(); closeErr != nil { + slog.Error("failed to close Docker client", "error", closeErr) + } + }() + + return cfg.AllowLists.initByIP(ctx, dockerClient) +} + // PrintNetworks prints the allowed networks func (allowLists *AllowListRegistry) PrintNetworks() { if len(allowLists.networks) > 0 { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 116408a..c4f33be 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,9 +1,14 @@ package config import ( + "context" + "encoding/json" "flag" "math" + "net" + "net/http" "os" + "path/filepath" "reflect" "regexp" "sort" @@ -11,6 +16,7 @@ import ( "testing" "github.com/wollomatic/socket-proxy/internal/docker/api/types/container" + "github.com/wollomatic/socket-proxy/internal/docker/api/types/network" ) func resetFlagsForTest(t *testing.T, args []string) func() { @@ -175,3 +181,62 @@ func TestInitConfig_WatchdogIntervalTooLarge(t *testing.T) { t.Fatal("InitConfig() unexpectedly succeeded") } } + +func TestRefreshAllowLists(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "docker.sock") + listener, err := net.Listen("unix", socketPath) + if err != nil { + t.Fatalf("listen on Docker test socket: %v", err) + } + t.Cleanup(func() { _ = listener.Close() }) + + go func() { + _ = http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/_ping": + w.Header().Set("Api-Version", "1.51") + case "/v1.51/containers/json": + if err := json.NewEncoder(w).Encode([]container.Summary{{ + ID: "container-id", + Labels: map[string]string{ + "socket-proxy.allow.get": "/version", + }, + NetworkSettings: &container.NetworkSettingsSummary{Networks: map[string]*network.EndpointSettings{ + "proxy-network": {IPAddress: "172.20.0.2"}, + }}, + }}); err != nil { + t.Errorf("encode container list: %v", err) + } + default: + http.NotFound(w, r) + } + })) + }() + + cfg := Config{ + SocketPath: socketPath, + AllowLists: &AllowListRegistry{ + networks: []string{"proxy-network"}, + }, + } + if err := cfg.RefreshAllowLists(context.Background()); err != nil { + t.Fatalf("RefreshAllowLists() error = %v", err) + } + + allowList, found := cfg.AllowLists.FindByIP("172.20.0.2") + if !found { + t.Fatal("allowlist was not added for container IP") + } + if !matchAny(allowList.AllowedRequests[http.MethodGet], "/version") { + t.Fatal("refreshed allowlist does not contain the container label") + } +} + +func matchAny(regexes []*regexp.Regexp, value string) bool { + for _, regex := range regexes { + if regex.MatchString(value) { + return true + } + } + return false +} From eb7280d62ed12ff97ca21af81b66b7265abcb914 Mon Sep 17 00:00:00 2001 From: wollomatic Date: Sat, 25 Jul 2026 21:14:57 +0200 Subject: [PATCH 2/8] Refactor Docker label handling and harden allowlist refreshes --- cmd/socket-proxy/handlehttprequest.go | 5 +- internal/config/config.go | 362 +-------------------- internal/config/config_test.go | 169 ---------- internal/config/dockerlabels.go | 433 ++++++++++++++++++++++++++ internal/config/dockerlabels_test.go | 320 +++++++++++++++++++ 5 files changed, 757 insertions(+), 532 deletions(-) create mode 100644 internal/config/dockerlabels.go create mode 100644 internal/config/dockerlabels_test.go diff --git a/cmd/socket-proxy/handlehttprequest.go b/cmd/socket-proxy/handlehttprequest.go index c00bce1..a70461b 100644 --- a/cmd/socket-proxy/handlehttprequest.go +++ b/cmd/socket-proxy/handlehttprequest.go @@ -78,9 +78,10 @@ func determineAllowList(r *http.Request) (config.AllowList, bool) { // updated the per-container allowlist. Refresh it once before denying // the request, while still failing closed if Docker cannot be queried. if cfg.ProxyContainerName != "" { - if err := cfg.RefreshAllowLists(r.Context()); err != nil { + refreshedAllowLists, err := cfg.RefreshAllowLists(r.Context()) + if err != nil { slog.Warn("failed to refresh per-container allowlists", "error", err) - } else if allowList, found := cfg.AllowLists.FindByIP(clientIPStr); found { + } else if allowList, found := refreshedAllowLists.FindByIP(clientIPStr); found { return allowList, true } } diff --git a/internal/config/config.go b/internal/config/config.go index 1e4187a..36372ec 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,7 +1,6 @@ package config import ( - "context" "errors" "flag" "fmt" @@ -15,17 +14,8 @@ import ( "slices" "strconv" "strings" - "sync" - "time" - - "github.com/wollomatic/socket-proxy/internal/docker/api/types/container" - "github.com/wollomatic/socket-proxy/internal/docker/api/types/events" - "github.com/wollomatic/socket-proxy/internal/docker/api/types/filters" - "github.com/wollomatic/socket-proxy/internal/docker/client" ) -const allowedDockerLabelPrefix = "socket-proxy.allow." - const ( defaultAllowFrom = "127.0.0.1/32" // allowed IPs to connect to the proxy defaultAllowHealthcheck = false // allow health check requests (HEAD http://localhost:55555/health) @@ -44,7 +34,7 @@ const ( ) type Config struct { - allowListsRefreshMutex sync.Mutex // serialises synchronous allowlist refreshes triggered by requests + allowListsRefresh allowListsRefreshState AllowLists *AllowListRegistry AllowFrom []string AllowHealthcheck bool @@ -60,13 +50,6 @@ type Config struct { ProxyContainerName string } -type AllowListRegistry struct { - mutex sync.RWMutex // mutex to control read/write of byIP - networks []string // names of networks in which socket proxy access is allowed for non-default allowlists - Default AllowList // default allowlist - byIP map[string]AllowList // map container IP address to allowlist for that container -} - type AllowList struct { ID string // Container ID (empty for the default allowlist) AllowedRequests map[string][]*regexp.Regexp // map of request methods to request path regex patterns (no requests allowed if empty) @@ -296,272 +279,6 @@ func InitConfig() (*Config, error) { return &cfg, nil } -// UpdateAllowLists populates the byIP allowlists then keeps them updated -func (cfg *Config) UpdateAllowLists() { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - dockerClient, err := client.NewClientWithOpts( - client.WithHost("unix://"+cfg.SocketPath), - client.WithAPIVersionNegotiation(), - ) - if err != nil { - slog.Error("failed to create Docker client", "error", err) - return - } - defer func(dockerClient *client.Client) { - err := dockerClient.Close() - if err != nil { - slog.Error("failed to close Docker client", "error", err) - } - }(dockerClient) - - err = cfg.AllowLists.initByIP(ctx, dockerClient) - if err != nil { - slog.Error("failed to initialise non-default allowlists", "error", err) - return - } - slog.Debug("initialised non-default allowlists") - - filter := filters.NewArgs() - filter.Add("type", "container") - filter.Add("event", "start") - filter.Add("event", "restart") - filter.Add("event", "die") - eventsChan, errChan := dockerClient.Events(ctx, events.ListOptions{Filters: filter}) - slog.Debug("subscribed to Docker event stream to update allowlists") - - // print non-default request allowlists - cfg.AllowLists.PrintByIP(cfg.LogJSON) - - // handle Docker events to update allowlists - for { - select { - case event, ok := <-eventsChan: - if !ok { - slog.Info("Docker event stream closed") - return - } - slog.Debug("received Docker container event", "action", event.Action, "id", event.Actor.ID[:12]) - addedIPs, removedIPs, updateErr := cfg.AllowLists.updateFromEvent(ctx, dockerClient, event) - if updateErr != nil { - slog.Warn("failed to update allowlists from container event", "error", updateErr) - continue - } - for _, ip := range addedIPs { - cfg.AllowLists.mutex.RLock() - allowList, found := cfg.AllowLists.byIP[ip] - cfg.AllowLists.mutex.RUnlock() - if found { - allowList.Print(ip, cfg.LogJSON) - } - } - for _, ip := range removedIPs { - slog.Info("removed allowlist for container", "id", event.Actor.ID[:12], "ip", ip) - } - case err := <-errChan: - if err != nil { - slog.Error("received error from Docker event stream", "error", err) - return - } - } - } -} - -// RefreshAllowLists updates the per-container allowlists from Docker. It is -// used when a request arrives before its corresponding Docker start event has -// been processed. -func (cfg *Config) RefreshAllowLists(ctx context.Context) error { - cfg.allowListsRefreshMutex.Lock() - defer cfg.allowListsRefreshMutex.Unlock() - - dockerClient, err := client.NewClientWithOpts( - client.WithHost("unix://"+cfg.SocketPath), - client.WithAPIVersionNegotiation(), - ) - if err != nil { - return err - } - defer func() { - if closeErr := dockerClient.Close(); closeErr != nil { - slog.Error("failed to close Docker client", "error", closeErr) - } - }() - - return cfg.AllowLists.initByIP(ctx, dockerClient) -} - -// PrintNetworks prints the allowed networks -func (allowLists *AllowListRegistry) PrintNetworks() { - if len(allowLists.networks) > 0 { - slog.Info("socket proxy networks detected", "socketproxynetworks", allowLists.networks) - } else { - // we only log this on DEBUG level because the socket proxy networks are used for per-container allowlists - slog.Debug("no socket proxy networks detected") - } -} - -// PrintDefault prints the default allowlist -func (allowLists *AllowListRegistry) PrintDefault(logJSON bool) { - allowLists.Default.Print("", logJSON) -} - -// PrintByIP prints the non-default allowlists -func (allowLists *AllowListRegistry) PrintByIP(logJSON bool) { - allowLists.mutex.RLock() - defer allowLists.mutex.RUnlock() - for ip, allowList := range allowLists.byIP { - allowList.Print(ip, logJSON) - } -} - -// FindByIP returns the allowlist corresponding to the given IP address if found -func (allowLists *AllowListRegistry) FindByIP(ip string) (AllowList, bool) { - allowLists.mutex.RLock() - defer allowLists.mutex.RUnlock() - allowList, found := allowLists.byIP[ip] - return allowList, found -} - -// initialise allowlist registry byIP allowlists -func (allowLists *AllowListRegistry) initByIP(ctx context.Context, dockerClient *client.Client) error { - filter := filters.NewArgs() - for _, network := range allowLists.networks { - filter.Add("network", network) - } - containers, err := dockerClient.ContainerList(ctx, container.ListOptions{Filters: filter}) - if err != nil { - return err - } - - allowLists.mutex.Lock() - defer allowLists.mutex.Unlock() - - allowLists.byIP = make(map[string]AllowList) - - for _, cntr := range containers { - allowedRequests, allowedBindMounts, err := extractLabelData(cntr) - if err != nil { - allowLists.byIP = nil - return err - } - - if len(allowedRequests) > 0 || len(allowedBindMounts) > 0 { - for networkID, cntrNetwork := range cntr.NetworkSettings.Networks { - if slices.Contains(allowLists.networks, networkID) { - allowList := AllowList{ - ID: cntr.ID, - AllowedRequests: allowedRequests, - AllowedBindMounts: allowedBindMounts, - } - - if len(cntrNetwork.IPAddress) > 0 { - allowLists.byIP[cntrNetwork.IPAddress] = allowList - } - if len(cntrNetwork.GlobalIPv6Address) > 0 { - allowLists.byIP[cntrNetwork.GlobalIPv6Address] = allowList - } - } - } - } - } - - return nil -} - -// update the allowlist registry based on the Docker event -func (allowLists *AllowListRegistry) updateFromEvent( - ctx context.Context, dockerClient *client.Client, event events.Message, -) ([]string, []string, error) { - containerID := event.Actor.ID - var ( - addedIPs []string - removedIPs []string - err error - ) - - switch event.Action { - case "start", "restart": - addedIPs, err = allowLists.add(ctx, dockerClient, containerID) - if err != nil { - return nil, nil, err - } - case "die": - removedIPs = allowLists.remove(containerID) - } - return addedIPs, removedIPs, nil -} - -// add the allowlist for the container with the given ID to the allowlist registry -// if it has at least one socket-proxy allow label and is in a same network as the socket-proxy -func (allowLists *AllowListRegistry) add( - ctx context.Context, dockerClient *client.Client, containerID string, -) ([]string, error) { - filter := filters.NewArgs() - filter.Add("id", containerID) - for _, network := range allowLists.networks { - filter.Add("network", network) - } - containers, err := dockerClient.ContainerList(ctx, container.ListOptions{Filters: filter}) - if err != nil { - return nil, err - } - if len(containers) == 0 { - slog.Debug("container is not in a network with socket-proxy or may have stopped", "id", containerID[:12]) - return nil, nil - } - cntr := containers[0] - - allowedRequests, allowedBindMounts, err := extractLabelData(cntr) - if err != nil { - return nil, err - } - - var ips []string - if len(allowedRequests) > 0 || len(allowedBindMounts) > 0 { - allowList := AllowList{ - ID: cntr.ID, - AllowedRequests: allowedRequests, - AllowedBindMounts: allowedBindMounts, - } - - allowLists.mutex.Lock() - defer allowLists.mutex.Unlock() - - for networkID, cntrNetwork := range cntr.NetworkSettings.Networks { - if slices.Contains(allowLists.networks, networkID) { - ipv4Address := cntrNetwork.IPAddress - if len(ipv4Address) > 0 { - allowLists.byIP[ipv4Address] = allowList - ips = append(ips, ipv4Address) - } - ipv6Address := cntrNetwork.GlobalIPv6Address - if len(ipv6Address) > 0 { - allowLists.byIP[ipv6Address] = allowList - ips = append(ips, ipv6Address) - } - } - } - } - - return ips, nil -} - -// remove allowlists having the given container ID from the allowlist registry -func (allowLists *AllowListRegistry) remove(containerID string) []string { - allowLists.mutex.Lock() - defer allowLists.mutex.Unlock() - - var removedIPs []string - for ip, allowList := range allowLists.byIP { - if allowList.ID == containerID { - delete(allowLists.byIP, ip) - removedIPs = append(removedIPs, ip) - } - } - return removedIPs -} - // Print prints the allowlist, including the IP address of the associated container if it is not empty, // and in JSON format if logJSON is true func (allowList AllowList) Print(ip string, logJSON bool) { @@ -655,80 +372,3 @@ func parseAllowedBindMounts(allowBindMountFromString string) ([]string, error) { } return allowedBindMounts, nil } - -// return list of docker networks that the socket-proxy container is in -func listSocketProxyNetworks(socketPath, proxyContainerName string) ([]string, error) { - cntr, err := getSocketProxyContainerSummary(socketPath, proxyContainerName) - if err != nil { - return nil, err - } - - networks := make([]string, 0, len(cntr.NetworkSettings.Networks)) - for networkID := range cntr.NetworkSettings.Networks { - networks = append(networks, networkID) - } - return networks, nil -} - -// return Docker container summary for the socket proxy container -func getSocketProxyContainerSummary(socketPath, proxyContainerName string) (container.Summary, error) { - const maxTries = 3 - - dockerClient, err := client.NewClientWithOpts( - client.WithHost("unix://"+socketPath), - client.WithAPIVersionNegotiation(), - ) - if err != nil { - return container.Summary{}, err - } - defer func(dockerClient *client.Client) { - err := dockerClient.Close() - if err != nil { - slog.Error("failed to close Docker client", "error", err) - } - }(dockerClient) - - ctx := context.Background() - filter := filters.NewArgs() - filter.Add("name", proxyContainerName) - var containers []container.Summary - for i := 1; i <= maxTries; i++ { - containers, err = dockerClient.ContainerList(ctx, container.ListOptions{Filters: filter}) - if err != nil { - return container.Summary{}, err - } - if len(containers) > 0 { - return containers[0], nil - } - if i < maxTries { - time.Sleep(time.Duration(i) * time.Second) - } - } - return container.Summary{}, fmt.Errorf("socket-proxy container \"%s\" was not found after %d attempts; verify the container name is correct and the container is running", proxyContainerName, maxTries) -} - -// extract Docker container allowlist label data from the container summary -func extractLabelData(cntr container.Summary) (map[string][]*regexp.Regexp, []string, error) { - allowedRequests := make(map[string][]*regexp.Regexp) - var allowedBindMounts []string - for labelName, labelValue := range cntr.Labels { - if strings.HasPrefix(labelName, allowedDockerLabelPrefix) && labelValue != "" { - allowSpec := strings.ToUpper(strings.TrimPrefix(labelName, allowedDockerLabelPrefix)) - method, _, _ := strings.Cut(allowSpec, ".") - if slices.Contains(supportedHTTPMethods, method) { - r, err := compileRegexp(labelValue, method, "docker container label") - if err != nil { - return nil, nil, err - } - allowedRequests[method] = append(allowedRequests[method], r) - } else if allowSpec == "BINDMOUNTFROM" { - var err error - allowedBindMounts, err = parseAllowedBindMounts(labelValue) - if err != nil { - return nil, nil, err - } - } - } - } - return allowedRequests, allowedBindMounts, nil -} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c4f33be..ec7cb5c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,22 +1,11 @@ package config import ( - "context" - "encoding/json" "flag" "math" - "net" - "net/http" "os" - "path/filepath" - "reflect" - "regexp" - "sort" "strconv" "testing" - - "github.com/wollomatic/socket-proxy/internal/docker/api/types/container" - "github.com/wollomatic/socket-proxy/internal/docker/api/types/network" ) func resetFlagsForTest(t *testing.T, args []string) func() { @@ -35,105 +24,6 @@ func resetFlagsForTest(t *testing.T, args []string) func() { } } -func Test_extractLabelData(t *testing.T) { - tests := []struct { - name string // description of this test case - // Named input parameters for target function. - cntr container.Summary - want map[string][]*regexp.Regexp - want2 []string - wantErr bool - }{ - { - name: "valid labels with multiple methods and regexes", - cntr: container.Summary{ - Labels: map[string]string{ - "socket-proxy.allow.get.0": "regex1", - "socket-proxy.allow.get.1": "regex2", - "socket-proxy.allow.post": "regex3", - }, - }, - want: map[string][]*regexp.Regexp{ - "GET": {regexp.MustCompile("^regex1$"), regexp.MustCompile("^regex2$")}, - "POST": {regexp.MustCompile("^regex3$")}, - }, - want2: nil, - wantErr: false, - }, - { - name: "invalid regex in label value", - cntr: container.Summary{ - Labels: map[string]string{ - "socket-proxy.allow.get": "invalid[regex", - }, - }, - want: nil, - want2: nil, - wantErr: true, - }, - { - name: "non-allow labels are ignored", - cntr: container.Summary{ - Labels: map[string]string{ - "socket-proxy.allow.get": "regex1", - "other.label": "value", - }, - }, - want: map[string][]*regexp.Regexp{ - "GET": {regexp.MustCompile("^regex1$")}, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, got2, gotErr := extractLabelData(tt.cntr) - if gotErr != nil { - if !tt.wantErr { - t.Errorf("extractLabelData() failed: %v", gotErr) - } - return - } - if tt.wantErr { - t.Fatal("extractLabelData() succeeded unexpectedly") - } - if !regexMapsEqual(got, tt.want) { - t.Errorf("extractLabelData() = %v, want %v", got, tt.want) - } - if !reflect.DeepEqual(got2, tt.want2) { - t.Errorf("extractLabelData() = %v, want %v", got2, tt.want2) - } - }) - } -} - -func regexMapsEqual(a, b map[string][]*regexp.Regexp) bool { - if len(a) != len(b) { - return false - } - for method, aRegexes := range a { - bRegexes, ok := b[method] - if !ok || len(aRegexes) != len(bRegexes) { - return false - } - aRegexStrings := make([]string, 0, len(aRegexes)) - for _, ar := range aRegexes { - aRegexStrings = append(aRegexStrings, ar.String()) - } - bRegexStrings := make([]string, 0, len(bRegexes)) - for _, br := range bRegexes { - bRegexStrings = append(bRegexStrings, br.String()) - } - sort.Strings(aRegexStrings) - sort.Strings(bRegexStrings) - for i, ar := range aRegexStrings { - if ar != bRegexStrings[i] { - return false - } - } - } - return true -} - func TestInitConfig_AllowMethodFlagOverridesEnv(t *testing.T) { t.Setenv("SP_ALLOW_GET", "/from-env") restore := resetFlagsForTest(t, []string{"socket-proxy", "-allowGET=/from-flag"}) @@ -181,62 +71,3 @@ func TestInitConfig_WatchdogIntervalTooLarge(t *testing.T) { t.Fatal("InitConfig() unexpectedly succeeded") } } - -func TestRefreshAllowLists(t *testing.T) { - socketPath := filepath.Join(t.TempDir(), "docker.sock") - listener, err := net.Listen("unix", socketPath) - if err != nil { - t.Fatalf("listen on Docker test socket: %v", err) - } - t.Cleanup(func() { _ = listener.Close() }) - - go func() { - _ = http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/_ping": - w.Header().Set("Api-Version", "1.51") - case "/v1.51/containers/json": - if err := json.NewEncoder(w).Encode([]container.Summary{{ - ID: "container-id", - Labels: map[string]string{ - "socket-proxy.allow.get": "/version", - }, - NetworkSettings: &container.NetworkSettingsSummary{Networks: map[string]*network.EndpointSettings{ - "proxy-network": {IPAddress: "172.20.0.2"}, - }}, - }}); err != nil { - t.Errorf("encode container list: %v", err) - } - default: - http.NotFound(w, r) - } - })) - }() - - cfg := Config{ - SocketPath: socketPath, - AllowLists: &AllowListRegistry{ - networks: []string{"proxy-network"}, - }, - } - if err := cfg.RefreshAllowLists(context.Background()); err != nil { - t.Fatalf("RefreshAllowLists() error = %v", err) - } - - allowList, found := cfg.AllowLists.FindByIP("172.20.0.2") - if !found { - t.Fatal("allowlist was not added for container IP") - } - if !matchAny(allowList.AllowedRequests[http.MethodGet], "/version") { - t.Fatal("refreshed allowlist does not contain the container label") - } -} - -func matchAny(regexes []*regexp.Regexp, value string) bool { - for _, regex := range regexes { - if regex.MatchString(value) { - return true - } - } - return false -} diff --git a/internal/config/dockerlabels.go b/internal/config/dockerlabels.go new file mode 100644 index 0000000..30d2250 --- /dev/null +++ b/internal/config/dockerlabels.go @@ -0,0 +1,433 @@ +package config + +import ( + "context" + "fmt" + "log/slog" + "regexp" + "slices" + "strings" + "sync" + "time" + + "github.com/wollomatic/socket-proxy/internal/docker/api/types/container" + "github.com/wollomatic/socket-proxy/internal/docker/api/types/events" + "github.com/wollomatic/socket-proxy/internal/docker/api/types/filters" + "github.com/wollomatic/socket-proxy/internal/docker/client" +) + +const ( + allowedDockerLabelPrefix = "socket-proxy.allow." + allowListsRefreshCooldown = time.Second + defaultAllowListsRefreshTimeout = 10 * time.Second +) + +type allowListsRefreshState struct { + mutex sync.Mutex + inFlight *allowListsRefresh + completed time.Time + lastResult *AllowListRegistry + lastError error + timeout time.Duration // zero uses defaultAllowListsRefreshTimeout +} + +type allowListsRefresh struct { + done chan struct{} + allowLists *AllowListRegistry + err error +} + +type AllowListRegistry struct { + mutex sync.RWMutex // mutex to control read/write of byIP + networks []string // names of networks in which socket proxy access is allowed for non-default allowlists + Default AllowList // default allowlist + byIP map[string]AllowList // map container IP address to allowlist for that container +} + +// UpdateAllowLists populates the byIP allowlists then keeps them updated. +func (cfg *Config) UpdateAllowLists() { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + dockerClient, err := client.NewClientWithOpts( + client.WithHost("unix://"+cfg.SocketPath), + client.WithAPIVersionNegotiation(), + ) + if err != nil { + slog.Error("failed to create Docker client", "error", err) + return + } + defer func(dockerClient *client.Client) { + err := dockerClient.Close() + if err != nil { + slog.Error("failed to close Docker client", "error", err) + } + }(dockerClient) + + err = cfg.AllowLists.initByIP(ctx, dockerClient) + if err != nil { + slog.Error("failed to initialise non-default allowlists", "error", err) + return + } + slog.Debug("initialised non-default allowlists") + + filter := filters.NewArgs() + filter.Add("type", "container") + filter.Add("event", "start") + filter.Add("event", "restart") + filter.Add("event", "die") + eventsChan, errChan := dockerClient.Events(ctx, events.ListOptions{Filters: filter}) + slog.Debug("subscribed to Docker event stream to update allowlists") + + // print non-default request allowlists + cfg.AllowLists.PrintByIP(cfg.LogJSON) + + // handle Docker events to update allowlists + for { + select { + case event, ok := <-eventsChan: + if !ok { + slog.Info("Docker event stream closed") + return + } + slog.Debug("received Docker container event", "action", event.Action, "id", event.Actor.ID[:12]) + addedIPs, removedIPs, updateErr := cfg.AllowLists.updateFromEvent(ctx, dockerClient, event) + if updateErr != nil { + slog.Warn("failed to update allowlists from container event", "error", updateErr) + continue + } + for _, ip := range addedIPs { + cfg.AllowLists.mutex.RLock() + allowList, found := cfg.AllowLists.byIP[ip] + cfg.AllowLists.mutex.RUnlock() + if found { + allowList.Print(ip, cfg.LogJSON) + } + } + for _, ip := range removedIPs { + slog.Info("removed allowlist for container", "id", event.Actor.ID[:12], "ip", ip) + } + case err := <-errChan: + if err != nil { + slog.Error("received error from Docker event stream", "error", err) + return + } + } + } +} + +// RefreshAllowLists updates the per-container allowlists from Docker. Concurrent +// callers share an in-flight refresh, and recently completed results are reused +// to avoid repeatedly scanning Docker during bursts of rejected requests. +func (cfg *Config) RefreshAllowLists(ctx context.Context) (*AllowListRegistry, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + state := &cfg.allowListsRefresh + state.mutex.Lock() + if refresh := state.inFlight; refresh != nil { + state.mutex.Unlock() + return waitForAllowListsRefresh(ctx, refresh) + } + if time.Since(state.completed) < allowListsRefreshCooldown { + allowLists, err := state.lastResult, state.lastError + state.mutex.Unlock() + return allowLists, err + } + refresh := &allowListsRefresh{done: make(chan struct{})} + state.inFlight = refresh + timeout := state.timeout + if timeout <= 0 { + timeout = defaultAllowListsRefreshTimeout + } + state.mutex.Unlock() + + refreshCtx, cancel := context.WithTimeout(context.Background(), timeout) + go func() { + defer cancel() + cfg.refreshAllowLists(refreshCtx, refresh) + }() + return waitForAllowListsRefresh(ctx, refresh) +} + +func (cfg *Config) refreshAllowLists(ctx context.Context, refresh *allowListsRefresh) { + dockerClient, err := client.NewClientWithOpts( + client.WithHost("unix://"+cfg.SocketPath), + client.WithAPIVersionNegotiation(), + ) + if err == nil { + err = cfg.AllowLists.initByIP(ctx, dockerClient) + if closeErr := dockerClient.Close(); closeErr != nil { + slog.Error("failed to close Docker client", "error", closeErr) + } + } + + state := &cfg.allowListsRefresh + state.mutex.Lock() + refresh.allowLists = cfg.AllowLists + refresh.err = err + state.lastResult = refresh.allowLists + state.lastError = err + state.completed = time.Now() + state.inFlight = nil + close(refresh.done) + state.mutex.Unlock() +} + +func waitForAllowListsRefresh(ctx context.Context, refresh *allowListsRefresh) (*AllowListRegistry, error) { + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-refresh.done: + return refresh.allowLists, refresh.err + } +} + +// PrintNetworks prints the allowed networks. +func (allowLists *AllowListRegistry) PrintNetworks() { + if len(allowLists.networks) > 0 { + slog.Info("socket proxy networks detected", "socketproxynetworks", allowLists.networks) + } else { + // we only log this on DEBUG level because the socket proxy networks are used for per-container allowlists + slog.Debug("no socket proxy networks detected") + } +} + +// PrintDefault prints the default allowlist. +func (allowLists *AllowListRegistry) PrintDefault(logJSON bool) { + allowLists.Default.Print("", logJSON) +} + +// PrintByIP prints the non-default allowlists. +func (allowLists *AllowListRegistry) PrintByIP(logJSON bool) { + allowLists.mutex.RLock() + defer allowLists.mutex.RUnlock() + for ip, allowList := range allowLists.byIP { + allowList.Print(ip, logJSON) + } +} + +// FindByIP returns the allowlist corresponding to the given IP address if found. +func (allowLists *AllowListRegistry) FindByIP(ip string) (AllowList, bool) { + allowLists.mutex.RLock() + defer allowLists.mutex.RUnlock() + allowList, found := allowLists.byIP[ip] + return allowList, found +} + +// initialise allowlist registry byIP allowlists +func (allowLists *AllowListRegistry) initByIP(ctx context.Context, dockerClient *client.Client) error { + filter := filters.NewArgs() + for _, network := range allowLists.networks { + filter.Add("network", network) + } + containers, err := dockerClient.ContainerList(ctx, container.ListOptions{Filters: filter}) + if err != nil { + return err + } + + allowLists.mutex.Lock() + defer allowLists.mutex.Unlock() + + allowLists.byIP = make(map[string]AllowList) + + for _, cntr := range containers { + allowedRequests, allowedBindMounts, err := extractLabelData(cntr) + if err != nil { + allowLists.byIP = nil + return err + } + + if len(allowedRequests) > 0 || len(allowedBindMounts) > 0 { + for networkID, cntrNetwork := range cntr.NetworkSettings.Networks { + if slices.Contains(allowLists.networks, networkID) { + allowList := AllowList{ + ID: cntr.ID, + AllowedRequests: allowedRequests, + AllowedBindMounts: allowedBindMounts, + } + + if len(cntrNetwork.IPAddress) > 0 { + allowLists.byIP[cntrNetwork.IPAddress] = allowList + } + if len(cntrNetwork.GlobalIPv6Address) > 0 { + allowLists.byIP[cntrNetwork.GlobalIPv6Address] = allowList + } + } + } + } + } + + return nil +} + +// update the allowlist registry based on the Docker event +func (allowLists *AllowListRegistry) updateFromEvent( + ctx context.Context, dockerClient *client.Client, event events.Message, +) ([]string, []string, error) { + containerID := event.Actor.ID + var ( + addedIPs []string + removedIPs []string + err error + ) + + switch event.Action { + case "start", "restart": + addedIPs, err = allowLists.add(ctx, dockerClient, containerID) + if err != nil { + return nil, nil, err + } + case "die": + removedIPs = allowLists.remove(containerID) + } + return addedIPs, removedIPs, nil +} + +// add the allowlist for the container with the given ID to the allowlist registry +// if it has at least one socket-proxy allow label and is in a same network as the socket-proxy +func (allowLists *AllowListRegistry) add( + ctx context.Context, dockerClient *client.Client, containerID string, +) ([]string, error) { + filter := filters.NewArgs() + filter.Add("id", containerID) + for _, network := range allowLists.networks { + filter.Add("network", network) + } + containers, err := dockerClient.ContainerList(ctx, container.ListOptions{Filters: filter}) + if err != nil { + return nil, err + } + if len(containers) == 0 { + slog.Debug("container is not in a network with socket-proxy or may have stopped", "id", containerID[:12]) + return nil, nil + } + cntr := containers[0] + + allowedRequests, allowedBindMounts, err := extractLabelData(cntr) + if err != nil { + return nil, err + } + + var ips []string + if len(allowedRequests) > 0 || len(allowedBindMounts) > 0 { + allowList := AllowList{ + ID: cntr.ID, + AllowedRequests: allowedRequests, + AllowedBindMounts: allowedBindMounts, + } + + allowLists.mutex.Lock() + defer allowLists.mutex.Unlock() + + for networkID, cntrNetwork := range cntr.NetworkSettings.Networks { + if slices.Contains(allowLists.networks, networkID) { + ipv4Address := cntrNetwork.IPAddress + if len(ipv4Address) > 0 { + allowLists.byIP[ipv4Address] = allowList + ips = append(ips, ipv4Address) + } + ipv6Address := cntrNetwork.GlobalIPv6Address + if len(ipv6Address) > 0 { + allowLists.byIP[ipv6Address] = allowList + ips = append(ips, ipv6Address) + } + } + } + } + + return ips, nil +} + +// remove allowlists having the given container ID from the allowlist registry +func (allowLists *AllowListRegistry) remove(containerID string) []string { + allowLists.mutex.Lock() + defer allowLists.mutex.Unlock() + + var removedIPs []string + for ip, allowList := range allowLists.byIP { + if allowList.ID == containerID { + delete(allowLists.byIP, ip) + removedIPs = append(removedIPs, ip) + } + } + return removedIPs +} + +// return list of docker networks that the socket proxy container is in +func listSocketProxyNetworks(socketPath, proxyContainerName string) ([]string, error) { + cntr, err := getSocketProxyContainerSummary(socketPath, proxyContainerName) + if err != nil { + return nil, err + } + + networks := make([]string, 0, len(cntr.NetworkSettings.Networks)) + for networkID := range cntr.NetworkSettings.Networks { + networks = append(networks, networkID) + } + return networks, nil +} + +// return Docker container summary for the socket proxy container +func getSocketProxyContainerSummary(socketPath, proxyContainerName string) (container.Summary, error) { + const maxTries = 3 + + dockerClient, err := client.NewClientWithOpts( + client.WithHost("unix://"+socketPath), + client.WithAPIVersionNegotiation(), + ) + if err != nil { + return container.Summary{}, err + } + defer func(dockerClient *client.Client) { + err := dockerClient.Close() + if err != nil { + slog.Error("failed to close Docker client", "error", err) + } + }(dockerClient) + + ctx := context.Background() + filter := filters.NewArgs() + filter.Add("name", proxyContainerName) + var containers []container.Summary + for i := 1; i <= maxTries; i++ { + containers, err = dockerClient.ContainerList(ctx, container.ListOptions{Filters: filter}) + if err != nil { + return container.Summary{}, err + } + if len(containers) > 0 { + return containers[0], nil + } + if i < maxTries { + time.Sleep(time.Duration(i) * time.Second) + } + } + return container.Summary{}, fmt.Errorf("socket-proxy container \"%s\" was not found after %d attempts; verify the container name is correct and the container is running", proxyContainerName, maxTries) +} + +// extract Docker container allowlist label data from the container summary +func extractLabelData(cntr container.Summary) (map[string][]*regexp.Regexp, []string, error) { + allowedRequests := make(map[string][]*regexp.Regexp) + var allowedBindMounts []string + for labelName, labelValue := range cntr.Labels { + if strings.HasPrefix(labelName, allowedDockerLabelPrefix) && labelValue != "" { + allowSpec := strings.ToUpper(strings.TrimPrefix(labelName, allowedDockerLabelPrefix)) + method, _, _ := strings.Cut(allowSpec, ".") + if slices.Contains(supportedHTTPMethods, method) { + r, err := compileRegexp(labelValue, method, "docker container label") + if err != nil { + return nil, nil, err + } + allowedRequests[method] = append(allowedRequests[method], r) + } else if allowSpec == "BINDMOUNTFROM" { + var err error + allowedBindMounts, err = parseAllowedBindMounts(labelValue) + if err != nil { + return nil, nil, err + } + } + } + } + return allowedRequests, allowedBindMounts, nil +} diff --git a/internal/config/dockerlabels_test.go b/internal/config/dockerlabels_test.go new file mode 100644 index 0000000..4604508 --- /dev/null +++ b/internal/config/dockerlabels_test.go @@ -0,0 +1,320 @@ +package config + +import ( + "context" + "encoding/json" + "errors" + "net" + "net/http" + "path/filepath" + "reflect" + "regexp" + "sort" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/wollomatic/socket-proxy/internal/docker/api/types/container" + "github.com/wollomatic/socket-proxy/internal/docker/api/types/network" +) + +func Test_extractLabelData(t *testing.T) { + tests := []struct { + name string // description of this test case + // Named input parameters for target function. + cntr container.Summary + want map[string][]*regexp.Regexp + want2 []string + wantErr bool + }{ + { + name: "valid labels with multiple methods and regexes", + cntr: container.Summary{ + Labels: map[string]string{ + "socket-proxy.allow.get.0": "regex1", + "socket-proxy.allow.get.1": "regex2", + "socket-proxy.allow.post": "regex3", + }, + }, + want: map[string][]*regexp.Regexp{ + "GET": {regexp.MustCompile("^regex1$"), regexp.MustCompile("^regex2$")}, + "POST": {regexp.MustCompile("^regex3$")}, + }, + want2: nil, + wantErr: false, + }, + { + name: "invalid regex in label value", + cntr: container.Summary{ + Labels: map[string]string{ + "socket-proxy.allow.get": "invalid[regex", + }, + }, + want: nil, + want2: nil, + wantErr: true, + }, + { + name: "non-allow labels are ignored", + cntr: container.Summary{ + Labels: map[string]string{ + "socket-proxy.allow.get": "regex1", + "other.label": "value", + }, + }, + want: map[string][]*regexp.Regexp{ + "GET": {regexp.MustCompile("^regex1$")}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, got2, gotErr := extractLabelData(tt.cntr) + if gotErr != nil { + if !tt.wantErr { + t.Errorf("extractLabelData() failed: %v", gotErr) + } + return + } + if tt.wantErr { + t.Fatal("extractLabelData() succeeded unexpectedly") + } + if !regexMapsEqual(got, tt.want) { + t.Errorf("extractLabelData() = %v, want %v", got, tt.want) + } + if !reflect.DeepEqual(got2, tt.want2) { + t.Errorf("extractLabelData() = %v, want %v", got2, tt.want2) + } + }) + } +} + +func regexMapsEqual(a, b map[string][]*regexp.Regexp) bool { + if len(a) != len(b) { + return false + } + for method, aRegexes := range a { + bRegexes, ok := b[method] + if !ok || len(aRegexes) != len(bRegexes) { + return false + } + aRegexStrings := make([]string, 0, len(aRegexes)) + for _, ar := range aRegexes { + aRegexStrings = append(aRegexStrings, ar.String()) + } + bRegexStrings := make([]string, 0, len(bRegexes)) + for _, br := range bRegexes { + bRegexStrings = append(bRegexStrings, br.String()) + } + sort.Strings(aRegexStrings) + sort.Strings(bRegexStrings) + for i, ar := range aRegexStrings { + if ar != bRegexStrings[i] { + return false + } + } + } + return true +} + +func TestRefreshAllowLists(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "docker.sock") + listener, err := (&net.ListenConfig{}).Listen(context.Background(), "unix", socketPath) + if err != nil { + t.Fatalf("listen on Docker test socket: %v", err) + } + t.Cleanup(func() { _ = listener.Close() }) + + go func() { + _ = http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/_ping": + w.Header().Set("Api-Version", "1.51") + case "/v1.51/containers/json": + if err := json.NewEncoder(w).Encode([]container.Summary{{ + ID: "container-id", + Labels: map[string]string{ + "socket-proxy.allow.get": "/version", + }, + NetworkSettings: &container.NetworkSettingsSummary{Networks: map[string]*network.EndpointSettings{ + "proxy-network": {IPAddress: "172.20.0.2"}, + }}, + }}); err != nil { + t.Errorf("encode container list: %v", err) + } + default: + http.NotFound(w, r) + } + })) + }() + + cfg := Config{ + SocketPath: socketPath, + AllowLists: &AllowListRegistry{ + networks: []string{"proxy-network"}, + }, + } + refreshedAllowLists, err := cfg.RefreshAllowLists(context.Background()) + if err != nil { + t.Fatalf("RefreshAllowLists() error = %v", err) + } + + allowList, found := refreshedAllowLists.FindByIP("172.20.0.2") + if !found { + t.Fatal("allowlist was not added for container IP") + } + if !matchAny(allowList.AllowedRequests[http.MethodGet], "/version") { + t.Fatal("refreshed allowlist does not contain the container label") + } +} + +func TestRefreshCoalescing(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "docker.sock") + listener, err := net.Listen("unix", socketPath) + if err != nil { + t.Fatalf("listen on Docker test socket: %v", err) + } + t.Cleanup(func() { _ = listener.Close() }) + + var ( + containerListRequests atomic.Int32 + requestStarted = make(chan struct{}) + releaseRequest = make(chan struct{}) + startOnce sync.Once + ) + go func() { + _ = http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/_ping": + w.Header().Set("Api-Version", "1.51") + case "/v1.51/containers/json": + containerListRequests.Add(1) + startOnce.Do(func() { close(requestStarted) }) + <-releaseRequest + http.Error(w, "Docker unavailable", http.StatusServiceUnavailable) + default: + http.NotFound(w, r) + } + })) + }() + + cfg := Config{ + SocketPath: socketPath, + AllowLists: &AllowListRegistry{}, + } + firstResult := make(chan error, 1) + go func() { + _, refreshErr := cfg.RefreshAllowLists(context.Background()) + firstResult <- refreshErr + }() + + select { + case <-requestStarted: + case <-time.After(time.Second): + t.Fatal("timed out waiting for Docker refresh to start") + } + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) + defer cancel() + if _, err := cfg.RefreshAllowLists(ctx); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("RefreshAllowLists() error = %v, want context.DeadlineExceeded", err) + } + + const callers = 8 + results := make(chan error, callers) + for range callers { + go func() { + _, refreshErr := cfg.RefreshAllowLists(context.Background()) + results <- refreshErr + }() + } + close(releaseRequest) + + if err := <-firstResult; err == nil { + t.Fatal("first RefreshAllowLists() unexpectedly succeeded") + } + for range callers { + if err := <-results; err == nil { + t.Fatal("shared RefreshAllowLists() unexpectedly succeeded") + } + } + if got := containerListRequests.Load(); got != 1 { + t.Fatalf("Docker container list requests = %d, want 1", got) + } + + if _, err := cfg.RefreshAllowLists(context.Background()); err == nil { + t.Fatal("cached RefreshAllowLists() unexpectedly succeeded") + } + if got := containerListRequests.Load(); got != 1 { + t.Fatalf("Docker container list requests during cooldown = %d, want 1", got) + } +} + +func TestRefreshTimeoutClearsInFlightRefresh(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "docker.sock") + listener, err := net.Listen("unix", socketPath) + if err != nil { + t.Fatalf("listen on Docker test socket: %v", err) + } + t.Cleanup(func() { _ = listener.Close() }) + + var containerListRequests atomic.Int32 + firstRequestCanceled := make(chan struct{}) + go func() { + _ = http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/_ping": + w.Header().Set("Api-Version", "1.51") + case "/v1.51/containers/json": + if containerListRequests.Add(1) == 1 { + <-r.Context().Done() + close(firstRequestCanceled) + return + } + if err := json.NewEncoder(w).Encode([]container.Summary{}); err != nil { + t.Errorf("encode container list: %v", err) + } + default: + http.NotFound(w, r) + } + })) + }() + + cfg := Config{ + SocketPath: socketPath, + AllowLists: &AllowListRegistry{}, + allowListsRefresh: allowListsRefreshState{ + timeout: 100 * time.Millisecond, + }, + } + if _, err := cfg.RefreshAllowLists(context.Background()); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("RefreshAllowLists() error = %v, want context.DeadlineExceeded", err) + } + select { + case <-firstRequestCanceled: + case <-time.After(time.Second): + t.Fatal("timed out waiting for Docker request cancellation") + } + + // Bypass the result cooldown so the retry exercises the cleared in-flight state. + cfg.allowListsRefresh.mutex.Lock() + cfg.allowListsRefresh.completed = time.Time{} + cfg.allowListsRefresh.mutex.Unlock() + + if _, err := cfg.RefreshAllowLists(context.Background()); err != nil { + t.Fatalf("retry RefreshAllowLists() error = %v", err) + } + if got := containerListRequests.Load(); got != 2 { + t.Fatalf("Docker container list requests = %d, want 2", got) + } +} + +func matchAny(regexes []*regexp.Regexp, value string) bool { + for _, regex := range regexes { + if regex.MatchString(value) { + return true + } + } + return false +} From bfdd44add123ca37c93d4d6efae0235e729f43b2 Mon Sep 17 00:00:00 2001 From: wollomatic Date: Sat, 25 Jul 2026 21:34:49 +0200 Subject: [PATCH 3/8] Fix stale Docker allowlist refresh races --- internal/config/dockerlabels.go | 36 ++++++++-- internal/config/dockerlabels_test.go | 101 +++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 6 deletions(-) diff --git a/internal/config/dockerlabels.go b/internal/config/dockerlabels.go index 30d2250..091b534 100644 --- a/internal/config/dockerlabels.go +++ b/internal/config/dockerlabels.go @@ -38,7 +38,8 @@ type allowListsRefresh struct { } type AllowListRegistry struct { - mutex sync.RWMutex // mutex to control read/write of byIP + mutex sync.RWMutex // mutex to control read/write of byIP and revision + revision uint64 // generation used to detect updates during Docker snapshots networks []string // names of networks in which socket proxy access is allowed for non-default allowlists Default AllowList // default allowlist byIP map[string]AllowList // map container IP address to allowlist for that container @@ -222,12 +223,25 @@ func (allowLists *AllowListRegistry) initByIP(ctx context.Context, dockerClient for _, network := range allowLists.networks { filter.Add("network", network) } - containers, err := dockerClient.ContainerList(ctx, container.ListOptions{Filters: filter}) - if err != nil { - return err - } - allowLists.mutex.Lock() + var containers []container.Summary + for { + allowLists.mutex.RLock() + snapshotRevision := allowLists.revision + allowLists.mutex.RUnlock() + + var err error + containers, err = dockerClient.ContainerList(ctx, container.ListOptions{Filters: filter}) + if err != nil { + return err + } + + allowLists.mutex.Lock() + if allowLists.revision == snapshotRevision { + break + } + allowLists.mutex.Unlock() + } defer allowLists.mutex.Unlock() allowLists.byIP = make(map[string]AllowList) @@ -236,6 +250,7 @@ func (allowLists *AllowListRegistry) initByIP(ctx context.Context, dockerClient allowedRequests, allowedBindMounts, err := extractLabelData(cntr) if err != nil { allowLists.byIP = nil + allowLists.revision++ return err } @@ -259,6 +274,7 @@ func (allowLists *AllowListRegistry) initByIP(ctx context.Context, dockerClient } } + allowLists.revision++ return nil } @@ -266,6 +282,10 @@ func (allowLists *AllowListRegistry) initByIP(ctx context.Context, dockerClient func (allowLists *AllowListRegistry) updateFromEvent( ctx context.Context, dockerClient *client.Client, event events.Message, ) ([]string, []string, error) { + allowLists.mutex.Lock() + allowLists.revision++ + allowLists.mutex.Unlock() + containerID := event.Actor.ID var ( addedIPs []string @@ -321,6 +341,10 @@ func (allowLists *AllowListRegistry) add( allowLists.mutex.Lock() defer allowLists.mutex.Unlock() + if allowLists.byIP == nil { + allowLists.byIP = make(map[string]AllowList) + } + for networkID, cntrNetwork := range cntr.NetworkSettings.Networks { if slices.Contains(allowLists.networks, networkID) { ipv4Address := cntrNetwork.IPAddress diff --git a/internal/config/dockerlabels_test.go b/internal/config/dockerlabels_test.go index 4604508..53e2d99 100644 --- a/internal/config/dockerlabels_test.go +++ b/internal/config/dockerlabels_test.go @@ -16,6 +16,7 @@ import ( "time" "github.com/wollomatic/socket-proxy/internal/docker/api/types/container" + "github.com/wollomatic/socket-proxy/internal/docker/api/types/events" "github.com/wollomatic/socket-proxy/internal/docker/api/types/network" ) @@ -169,6 +170,106 @@ func TestRefreshAllowLists(t *testing.T) { } } +func TestRefreshDoesNotOverwriteNewerEventUpdate(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "docker.sock") + listener, err := net.Listen("unix", socketPath) + if err != nil { + t.Fatalf("listen on Docker test socket: %v", err) + } + t.Cleanup(func() { _ = listener.Close() }) + + requestStarted := make(chan struct{}) + releaseRequest := make(chan struct{}) + var ( + containerListRequests atomic.Int32 + startOnce sync.Once + releaseOnce sync.Once + ) + release := func() { + releaseOnce.Do(func() { close(releaseRequest) }) + } + t.Cleanup(release) + + go func() { + _ = http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/_ping": + w.Header().Set("Api-Version", "1.51") + case "/v1.51/containers/json": + if containerListRequests.Add(1) != 1 { + if err := json.NewEncoder(w).Encode([]container.Summary{}); err != nil { + t.Errorf("encode container list: %v", err) + } + return + } + startOnce.Do(func() { close(requestStarted) }) + <-releaseRequest + if err := json.NewEncoder(w).Encode([]container.Summary{{ + ID: "stopped-container-id", + Labels: map[string]string{ + "socket-proxy.allow.get": "/version", + }, + NetworkSettings: &container.NetworkSettingsSummary{Networks: map[string]*network.EndpointSettings{ + "proxy-network": {IPAddress: "172.20.0.2"}, + }}, + }}); err != nil { + t.Errorf("encode container list: %v", err) + } + default: + http.NotFound(w, r) + } + })) + }() + + cfg := Config{ + SocketPath: socketPath, + AllowLists: &AllowListRegistry{ + networks: []string{"proxy-network"}, + byIP: map[string]AllowList{ + "172.20.0.2": {ID: "stopped-container-id"}, + }, + }, + } + refreshResult := make(chan error, 1) + go func() { + _, refreshErr := cfg.RefreshAllowLists(context.Background()) + refreshResult <- refreshErr + }() + + select { + case <-requestStarted: + case <-time.After(time.Second): + t.Fatal("timed out waiting for Docker refresh to start") + } + + _, removedIPs, err := cfg.AllowLists.updateFromEvent(context.Background(), nil, events.Message{ + Action: events.ActionDie, + Actor: events.Actor{ID: "stopped-container-id"}, + }) + if err != nil { + t.Fatalf("updateFromEvent() error = %v", err) + } + if !reflect.DeepEqual(removedIPs, []string{"172.20.0.2"}) { + t.Fatalf("removed IPs = %v, want [172.20.0.2]", removedIPs) + } + + release() + select { + case err := <-refreshResult: + if err != nil { + t.Fatalf("RefreshAllowLists() error = %v", err) + } + case <-time.After(time.Second): + t.Fatal("timed out waiting for Docker refresh retry") + } + if got := containerListRequests.Load(); got != 2 { + t.Fatalf("Docker container list requests = %d, want 2", got) + } + if _, found := cfg.AllowLists.FindByIP("172.20.0.2"); found { + t.Fatal("refresh restored the allowlist removed by a newer event") + } +} + func TestRefreshCoalescing(t *testing.T) { socketPath := filepath.Join(t.TempDir(), "docker.sock") listener, err := net.Listen("unix", socketPath) From 35ce9f495de7f387ce4f4b28ad1d8bac473c5706 Mon Sep 17 00:00:00 2001 From: wollomatic Date: Sat, 25 Jul 2026 21:42:12 +0200 Subject: [PATCH 4/8] include fix for issue #132 in refactored code --- internal/config/config.go | 9 +++-- internal/config/dockerlabels.go | 36 +++++++++++++++-- internal/config/dockerlabels_test.go | 59 ++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 7 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 36372ec..59e27a2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -52,6 +52,7 @@ type Config struct { type AllowList struct { ID string // Container ID (empty for the default allowlist) + ContainerName string // Container name (empty for the default allowlist) AllowedRequests map[string][]*regexp.Regexp // map of request methods to request path regex patterns (no requests allowed if empty) AllowedBindMounts []string // list of from portion of allowed bind mounts (all bind mounts allowed if empty) } @@ -291,7 +292,7 @@ func (allowList AllowList) Print(ip string, logJSON bool) { } else { for method, regex := range allowList.AllowedRequests { slog.Info("configured request allowlist", - "id", allowList.ID[:12], + "container", allowList.ContainerName, "ip", ip, "method", method, "regex", regex, @@ -304,7 +305,7 @@ func (allowList AllowList) Print(ip string, logJSON bool) { if ip == "" { fmt.Printf("Default request allowlist:\n %-8s %s\n", "Method", "Regex") } else { - fmt.Printf("Request allowlist for %s (%s):\n %-8s %s\n", allowList.ID[:12], ip, "Method", "Regex") + fmt.Printf("Request allowlist for %s (%s):\n %-8s %s\n", allowList.ContainerName, ip, "Method", "Regex") } for method, regex := range allowList.AllowedRequests { fmt.Printf(" %-8s %s\n", method, regex) @@ -319,7 +320,7 @@ func (allowList AllowList) Print(ip string, logJSON bool) { } else { slog.Info("Docker bind mount restrictions enabled", "allowbindmountfrom", allowList.AllowedBindMounts, - "id", allowList.ID[:12], + "container", allowList.ContainerName, "ip", ip, ) } @@ -328,7 +329,7 @@ func (allowList AllowList) Print(ip string, logJSON bool) { if ip == "" { slog.Debug("no default Docker bind mount restrictions") } else { - slog.Debug("no Docker bind mount restrictions", "id", allowList.ID[:12], "ip", ip) + slog.Debug("no Docker bind mount restrictions", "container", allowList.ContainerName, "ip", ip) } } } diff --git a/internal/config/dockerlabels.go b/internal/config/dockerlabels.go index 091b534..27b23e4 100644 --- a/internal/config/dockerlabels.go +++ b/internal/config/dockerlabels.go @@ -91,7 +91,8 @@ func (cfg *Config) UpdateAllowLists() { slog.Info("Docker event stream closed") return } - slog.Debug("received Docker container event", "action", event.Action, "id", event.Actor.ID[:12]) + containerName := eventContainerName(event) + slog.Debug("received Docker container event", "action", event.Action, "container", containerName) addedIPs, removedIPs, updateErr := cfg.AllowLists.updateFromEvent(ctx, dockerClient, event) if updateErr != nil { slog.Warn("failed to update allowlists from container event", "error", updateErr) @@ -106,7 +107,7 @@ func (cfg *Config) UpdateAllowLists() { } } for _, ip := range removedIPs { - slog.Info("removed allowlist for container", "id", event.Actor.ID[:12], "ip", ip) + slog.Info("removed allowlist for container", "container", containerName, "ip", ip) } case err := <-errChan: if err != nil { @@ -259,6 +260,7 @@ func (allowLists *AllowListRegistry) initByIP(ctx context.Context, dockerClient if slices.Contains(allowLists.networks, networkID) { allowList := AllowList{ ID: cntr.ID, + ContainerName: containerName(cntr), AllowedRequests: allowedRequests, AllowedBindMounts: allowedBindMounts, } @@ -320,7 +322,7 @@ func (allowLists *AllowListRegistry) add( return nil, err } if len(containers) == 0 { - slog.Debug("container is not in a network with socket-proxy or may have stopped", "id", containerID[:12]) + slog.Debug("container is not in a network with socket-proxy or may have stopped", "id", shortContainerID(containerID)) return nil, nil } cntr := containers[0] @@ -334,6 +336,7 @@ func (allowLists *AllowListRegistry) add( if len(allowedRequests) > 0 || len(allowedBindMounts) > 0 { allowList := AllowList{ ID: cntr.ID, + ContainerName: containerName(cntr), AllowedRequests: allowedRequests, AllowedBindMounts: allowedBindMounts, } @@ -379,6 +382,33 @@ func (allowLists *AllowListRegistry) remove(containerID string) []string { return removedIPs } +// containerName returns Docker's container name without its leading slash. +// It falls back to the short container ID for unusual responses without a name. +func containerName(cntr container.Summary) string { + for _, name := range cntr.Names { + if name = strings.TrimPrefix(name, "/"); name != "" { + return name + } + } + return shortContainerID(cntr.ID) +} + +// eventContainerName returns the name Docker includes with container events. +// It falls back to the short container ID when the event has no name. +func eventContainerName(event events.Message) string { + if name := event.Actor.Attributes["name"]; name != "" { + return name + } + return shortContainerID(event.Actor.ID) +} + +func shortContainerID(id string) string { + if len(id) > 12 { + return id[:12] + } + return id +} + // return list of docker networks that the socket proxy container is in func listSocketProxyNetworks(socketPath, proxyContainerName string) ([]string, error) { cntr, err := getSocketProxyContainerSummary(socketPath, proxyContainerName) diff --git a/internal/config/dockerlabels_test.go b/internal/config/dockerlabels_test.go index 53e2d99..105346c 100644 --- a/internal/config/dockerlabels_test.go +++ b/internal/config/dockerlabels_test.go @@ -91,6 +91,65 @@ func Test_extractLabelData(t *testing.T) { } } +func TestContainerName(t *testing.T) { + tests := []struct { + name string + cntr container.Summary + want string + }{ + { + name: "uses the first Docker container name", + cntr: container.Summary{ID: "0123456789abcdef", Names: []string{"/traefik", "/ignored"}}, + want: "traefik", + }, + { + name: "falls back to the short ID when Docker provides no name", + cntr: container.Summary{ID: "0123456789abcdef"}, + want: "0123456789ab", + }, + { + name: "does not panic for a short fallback ID", + cntr: container.Summary{ID: "short"}, + want: "short", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := containerName(tt.cntr); got != tt.want { + t.Errorf("containerName() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestEventContainerName(t *testing.T) { + tests := []struct { + name string + event events.Message + want string + }{ + { + name: "uses the container name from event attributes", + event: events.Message{Actor: events.Actor{ID: "0123456789abcdef", Attributes: map[string]string{"name": "traefik"}}}, + want: "traefik", + }, + { + name: "falls back to the short ID when the event has no name", + event: events.Message{Actor: events.Actor{ID: "0123456789abcdef"}}, + want: "0123456789ab", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := eventContainerName(tt.event); got != tt.want { + t.Errorf("eventContainerName() = %q, want %q", got, tt.want) + } + }) + } +} + func regexMapsEqual(a, b map[string][]*regexp.Regexp) bool { if len(a) != len(b) { return false From 0dd4a37157ddb3b3cc383251a7569830deaa388b Mon Sep 17 00:00:00 2001 From: wollomatic Date: Sat, 25 Jul 2026 22:03:31 +0200 Subject: [PATCH 5/8] fix Coderabbit findings --- internal/config/dockerlabels.go | 44 ++++++---- internal/config/dockerlabels_test.go | 117 ++++++++++++++++++++++++++- 2 files changed, 143 insertions(+), 18 deletions(-) diff --git a/internal/config/dockerlabels.go b/internal/config/dockerlabels.go index 27b23e4..a66fe86 100644 --- a/internal/config/dockerlabels.go +++ b/internal/config/dockerlabels.go @@ -19,6 +19,7 @@ import ( const ( allowedDockerLabelPrefix = "socket-proxy.allow." allowListsRefreshCooldown = time.Second + allowListsRefreshRetryBackoff = 10 * time.Millisecond defaultAllowListsRefreshTimeout = 10 * time.Second ) @@ -225,39 +226,53 @@ func (allowLists *AllowListRegistry) initByIP(ctx context.Context, dockerClient filter.Add("network", network) } - var containers []container.Summary for { + if err := ctx.Err(); err != nil { + return err + } + allowLists.mutex.RLock() snapshotRevision := allowLists.revision allowLists.mutex.RUnlock() - var err error - containers, err = dockerClient.ContainerList(ctx, container.ListOptions{Filters: filter}) + containers, err := dockerClient.ContainerList(ctx, container.ListOptions{Filters: filter}) + if err != nil { + return err + } + + byIP, err := buildByIP(containers, allowLists.networks) if err != nil { return err } allowLists.mutex.Lock() if allowLists.revision == snapshotRevision { - break + allowLists.byIP = byIP + allowLists.revision++ + allowLists.mutex.Unlock() + return nil } allowLists.mutex.Unlock() - } - defer allowLists.mutex.Unlock() - allowLists.byIP = make(map[string]AllowList) + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(allowListsRefreshRetryBackoff): + } + } +} +func buildByIP(containers []container.Summary, networks []string) (map[string]AllowList, error) { + byIP := make(map[string]AllowList) for _, cntr := range containers { allowedRequests, allowedBindMounts, err := extractLabelData(cntr) if err != nil { - allowLists.byIP = nil - allowLists.revision++ - return err + return nil, err } if len(allowedRequests) > 0 || len(allowedBindMounts) > 0 { for networkID, cntrNetwork := range cntr.NetworkSettings.Networks { - if slices.Contains(allowLists.networks, networkID) { + if slices.Contains(networks, networkID) { allowList := AllowList{ ID: cntr.ID, ContainerName: containerName(cntr), @@ -266,18 +281,17 @@ func (allowLists *AllowListRegistry) initByIP(ctx context.Context, dockerClient } if len(cntrNetwork.IPAddress) > 0 { - allowLists.byIP[cntrNetwork.IPAddress] = allowList + byIP[cntrNetwork.IPAddress] = allowList } if len(cntrNetwork.GlobalIPv6Address) > 0 { - allowLists.byIP[cntrNetwork.GlobalIPv6Address] = allowList + byIP[cntrNetwork.GlobalIPv6Address] = allowList } } } } } - allowLists.revision++ - return nil + return byIP, nil } // update the allowlist registry based on the Docker event diff --git a/internal/config/dockerlabels_test.go b/internal/config/dockerlabels_test.go index 105346c..a3a05b0 100644 --- a/internal/config/dockerlabels_test.go +++ b/internal/config/dockerlabels_test.go @@ -229,9 +229,67 @@ func TestRefreshAllowLists(t *testing.T) { } } +func TestRefreshAllowListsErrorPreservesRegistry(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "docker.sock") + listener, err := (&net.ListenConfig{}).Listen(context.Background(), "unix", socketPath) + if err != nil { + t.Fatalf("listen on Docker test socket: %v", err) + } + t.Cleanup(func() { _ = listener.Close() }) + + go func() { + _ = http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/_ping": + w.Header().Set("Api-Version", "1.51") + case "/v1.51/containers/json": + if err := json.NewEncoder(w).Encode([]container.Summary{{ + ID: "container-id", + Labels: map[string]string{ + "socket-proxy.allow.get": "invalid[regex", + }, + NetworkSettings: &container.NetworkSettingsSummary{Networks: map[string]*network.EndpointSettings{ + "proxy-network": {IPAddress: "172.20.0.3"}, + }}, + }}); err != nil { + t.Errorf("encode container list: %v", err) + } + default: + http.NotFound(w, r) + } + })) + }() + + const initialRevision = 7 + cfg := Config{ + SocketPath: socketPath, + AllowLists: &AllowListRegistry{ + revision: initialRevision, + networks: []string{"proxy-network"}, + byIP: map[string]AllowList{ + "172.20.0.2": {ID: "existing-container-id"}, + }, + }, + } + if _, err := cfg.RefreshAllowLists(context.Background()); err == nil { + t.Fatal("RefreshAllowLists() unexpectedly succeeded") + } + + allowList, found := cfg.AllowLists.FindByIP("172.20.0.2") + if !found || allowList.ID != "existing-container-id" { + t.Fatal("RefreshAllowLists() modified the existing allowlist after an error") + } + cfg.AllowLists.mutex.RLock() + revision := cfg.AllowLists.revision + cfg.AllowLists.mutex.RUnlock() + if revision != initialRevision { + t.Fatalf("revision = %d, want %d", revision, initialRevision) + } +} + func TestRefreshDoesNotOverwriteNewerEventUpdate(t *testing.T) { socketPath := filepath.Join(t.TempDir(), "docker.sock") - listener, err := net.Listen("unix", socketPath) + listener, err := (&net.ListenConfig{}).Listen(context.Background(), "unix", socketPath) if err != nil { t.Fatalf("listen on Docker test socket: %v", err) } @@ -329,9 +387,62 @@ func TestRefreshDoesNotOverwriteNewerEventUpdate(t *testing.T) { } } +func TestRefreshRevisionRetriesRespectTimeout(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "docker.sock") + listener, err := (&net.ListenConfig{}).Listen(context.Background(), "unix", socketPath) + if err != nil { + t.Fatalf("listen on Docker test socket: %v", err) + } + t.Cleanup(func() { _ = listener.Close() }) + + const refreshTimeout = 50 * time.Millisecond + var containerListRequests atomic.Int32 + cfg := Config{ + SocketPath: socketPath, + AllowLists: &AllowListRegistry{ + byIP: map[string]AllowList{ + "172.20.0.2": {ID: "existing-container-id"}, + }, + }, + allowListsRefresh: allowListsRefreshState{ + timeout: refreshTimeout, + }, + } + + go func() { + _ = http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/_ping": + w.Header().Set("Api-Version", "1.51") + case "/v1.51/containers/json": + containerListRequests.Add(1) + cfg.AllowLists.mutex.Lock() + cfg.AllowLists.revision++ + cfg.AllowLists.mutex.Unlock() + if err := json.NewEncoder(w).Encode([]container.Summary{}); err != nil { + t.Errorf("encode container list: %v", err) + } + default: + http.NotFound(w, r) + } + })) + }() + + if _, err := cfg.RefreshAllowLists(context.Background()); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("RefreshAllowLists() error = %v, want context.DeadlineExceeded", err) + } + maxRequests := int32(refreshTimeout/allowListsRefreshRetryBackoff) + 1 + if got := containerListRequests.Load(); got > maxRequests { + t.Fatalf("Docker container list requests = %d, want at most %d", got, maxRequests) + } + if _, found := cfg.AllowLists.FindByIP("172.20.0.2"); !found { + t.Fatal("timed-out refresh replaced the existing allowlist") + } +} + func TestRefreshCoalescing(t *testing.T) { socketPath := filepath.Join(t.TempDir(), "docker.sock") - listener, err := net.Listen("unix", socketPath) + listener, err := (&net.ListenConfig{}).Listen(context.Background(), "unix", socketPath) if err != nil { t.Fatalf("listen on Docker test socket: %v", err) } @@ -413,7 +524,7 @@ func TestRefreshCoalescing(t *testing.T) { func TestRefreshTimeoutClearsInFlightRefresh(t *testing.T) { socketPath := filepath.Join(t.TempDir(), "docker.sock") - listener, err := net.Listen("unix", socketPath) + listener, err := (&net.ListenConfig{}).Listen(context.Background(), "unix", socketPath) if err != nil { t.Fatalf("listen on Docker test socket: %v", err) } From 4c6077ea7910a90ab2892054cd09e8189ca692ad Mon Sep 17 00:00:00 2001 From: wollomatic Date: Sat, 25 Jul 2026 22:09:34 +0200 Subject: [PATCH 6/8] release blocked mock handler during cleanup --- internal/config/dockerlabels_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/config/dockerlabels_test.go b/internal/config/dockerlabels_test.go index a3a05b0..e1280e8 100644 --- a/internal/config/dockerlabels_test.go +++ b/internal/config/dockerlabels_test.go @@ -453,7 +453,13 @@ func TestRefreshCoalescing(t *testing.T) { requestStarted = make(chan struct{}) releaseRequest = make(chan struct{}) startOnce sync.Once + releaseOnce sync.Once ) + release := func() { + releaseOnce.Do(func() { close(releaseRequest) }) + } + t.Cleanup(release) + go func() { _ = http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { @@ -500,7 +506,7 @@ func TestRefreshCoalescing(t *testing.T) { results <- refreshErr }() } - close(releaseRequest) + release() if err := <-firstResult; err == nil { t.Fatal("first RefreshAllowLists() unexpectedly succeeded") From 4b26d2e2c2fc94d8b795eb12304a4d1b0160bf0f Mon Sep 17 00:00:00 2001 From: wollomatic Date: Sun, 26 Jul 2026 17:12:41 +0200 Subject: [PATCH 7/8] extremely decrease allowListsRefreshCooldown --- internal/config/dockerlabels.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/dockerlabels.go b/internal/config/dockerlabels.go index a66fe86..8633602 100644 --- a/internal/config/dockerlabels.go +++ b/internal/config/dockerlabels.go @@ -18,7 +18,7 @@ import ( const ( allowedDockerLabelPrefix = "socket-proxy.allow." - allowListsRefreshCooldown = time.Second + allowListsRefreshCooldown = 50 * time.Millisecond allowListsRefreshRetryBackoff = 10 * time.Millisecond defaultAllowListsRefreshTimeout = 10 * time.Second ) From 719d070a22c2a47bd9d84a1a4c98e33a2b1c441f Mon Sep 17 00:00:00 2001 From: wollomatic Date: Sun, 26 Jul 2026 17:37:52 +0200 Subject: [PATCH 8/8] make Docker label configurable (fixes #165) --- README.md | 3 ++- internal/config/config.go | 14 ++++++++++++ internal/config/config_test.go | 34 ++++++++++++++++++++++++++++ internal/config/dockerlabels.go | 3 ++- internal/config/dockerlabels_test.go | 20 ++++++++++++++++ 5 files changed, 72 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3eee4f2..837857a 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,7 @@ Allowlists for both requests and bind mount restrictions can be specified for pa 1. Set `-proxycontainername` or the environment variable `SP_PROXYCONTAINERNAME` to the name of the socket proxy container. 2. Make sure that each container that will use the socket proxy is in a Docker network that the socket proxy container is also in. -3. Use the same regex syntax for request allowlists and for bind mount restrictions that were discussed earlier, but for labels on each container that will use the socket proxy. Each label name will have the prefix of `socket-proxy.allow.`, with `socket-proxy.allow.bindmountfrom` for bind mount restrictions. For example: +3. Use the same regex syntax for request allowlists and for bind mount restrictions that were discussed earlier, but for labels on each container that will use the socket proxy. Each label name has the prefix `.allow.`; by default this is `socket-proxy.allow.`, with `socket-proxy.allow.bindmountfrom` for bind mount restrictions. Set `-dockerlabelprefix` or `SP_DOCKERLABELPREFIX` when multiple socket proxies share a Docker daemon. For example, `-dockerlabelprefix=traefik-socket-proxy` uses labels beginning with `traefik-socket-proxy.allow.`. ```yaml services: @@ -254,6 +254,7 @@ socket-proxy can be configured via command-line parameters or via environment va | `-watchdoginterval` | `SP_WATCHDOGINTERVAL` | `0` | Check for socket availability every x seconds (disable checks, if not set or value is 0) | | `-proxysocketendpoint` | `SP_PROXYSOCKETENDPOINT` | (not set) | Proxy to the given unix socket instead of a TCP port | | `-proxysocketendpointfilemode` | `SP_PROXYSOCKETENDPOINTFILEMODE` | `0600` | Explicitly set the file mode for the filtered unix socket endpoint (only useful with `-proxysocketendpoint`) | +| `-dockerlabelprefix` | `SP_DOCKERLABELPREFIX` | `socket-proxy` | Specifies the prefix before `.allow.` in Docker container labels used for per-container allowlists. It must contain only lowercase letters, digits, dots, and hyphens. For example, `-dockerlabelprefix=traefik-socket-proxy` recognizes `traefik-socket-proxy.allow.get`. | | `-proxycontainername` | `SP_PROXYCONTAINERNAME` | (not set) | Provides the name of the socket proxy container to enable per-container allowlists specified by Docker container labels (not available with `-proxysocketendpoint`) | ### Changelog diff --git a/internal/config/config.go b/internal/config/config.go index 59e27a2..503f55b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -30,6 +30,7 @@ const ( defaultProxySocketEndpoint = "" // empty string means no socket listener, but regular TCP listener defaultProxySocketEndpointFileMode = uint(0o600) // set the file mode of the unix socket endpoint defaultAllowBindMountFrom = "" // empty string means no bind mount restrictions + defaultDockerLabelPrefix = "socket-proxy" // prefix before .allow. in per-container allowlist labels defaultProxyContainerName = "" // socket-proxy Docker container name (empty string disables container labels for allowlists) ) @@ -75,6 +76,8 @@ var supportedHTTPMethods = []string{ http.MethodOptions, } +var dockerLabelPrefixRegexp = regexp.MustCompile(`^[a-z0-9]+(?:[.-][a-z0-9]+)*$`) + // InitConfig reads configuration from environment variables and command-line // flags, validates the resulting values, and returns the initialized Config. func InitConfig() (*Config, error) { @@ -86,6 +89,7 @@ func InitConfig() (*Config, error) { logLevel string endpointFileMode uint allowBindMountFromString string + dockerLabelPrefix string defaultAllowFromValue = defaultAllowFrom defaultAllowHealthcheckValue = defaultAllowHealthcheck defaultLogJSONValue = defaultLogJSON @@ -99,6 +103,7 @@ func InitConfig() (*Config, error) { defaultProxySocketEndpointValue = defaultProxySocketEndpoint defaultProxySocketEndpointFileModeValue = defaultProxySocketEndpointFileMode defaultAllowBindMountFromValue = defaultAllowBindMountFrom + defaultDockerLabelPrefixValue = defaultDockerLabelPrefix defaultProxyContainerNameValue = defaultProxyContainerName ) @@ -155,6 +160,9 @@ func InitConfig() (*Config, error) { if val, ok := os.LookupEnv("SP_ALLOWBINDMOUNTFROM"); ok && val != "" { defaultAllowBindMountFromValue = val } + if val, ok := os.LookupEnv("SP_DOCKERLABELPREFIX"); ok && val != "" { + defaultDockerLabelPrefixValue = val + } if val, ok := os.LookupEnv("SP_PROXYCONTAINERNAME"); ok && val != "" { defaultProxyContainerNameValue = val } @@ -185,12 +193,17 @@ func InitConfig() (*Config, error) { flag.StringVar(&cfg.ProxySocketEndpoint, "proxysocketendpoint", defaultProxySocketEndpointValue, "unix socket endpoint (if set, used instead of the TCP listener)") flag.UintVar(&endpointFileMode, "proxysocketendpointfilemode", defaultProxySocketEndpointFileModeValue, "set the file mode of the unix socket endpoint") flag.StringVar(&allowBindMountFromString, "allowbindmountfrom", defaultAllowBindMountFromValue, "allowed directories for bind mounts (comma-separated)") + flag.StringVar(&dockerLabelPrefix, "dockerlabelprefix", defaultDockerLabelPrefixValue, "prefix before .allow. in Docker container allowlist labels") flag.StringVar(&cfg.ProxyContainerName, "proxycontainername", defaultProxyContainerNameValue, "socket-proxy Docker container name") for i := range methodAllowLists { flag.Var(&methodAllowLists[i].regexStrings, "allow"+methodAllowLists[i].method, "regex for "+methodAllowLists[i].method+" requests (not set means method is not allowed)") } flag.Parse() + if !dockerLabelPrefixRegexp.MatchString(dockerLabelPrefix) { + return nil, fmt.Errorf("invalid dockerlabelprefix %q: use lowercase letters, digits, dots, and hyphens only; dots and hyphens must separate alphanumeric parts", dockerLabelPrefix) + } + // init allowlist registry to configure default allowlist cfg.AllowLists = &AllowListRegistry{} @@ -276,6 +289,7 @@ func InitConfig() (*Config, error) { return nil, err } } + allowedDockerLabelPrefix = dockerLabelPrefix + ".allow." return &cfg, nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ec7cb5c..0dc774d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -13,6 +13,7 @@ func resetFlagsForTest(t *testing.T, args []string) func() { prevCommandLine := flag.CommandLine prevArgs := os.Args + prevDockerLabelPrefix := allowedDockerLabelPrefix flag.CommandLine = flag.NewFlagSet(args[0], flag.ContinueOnError) flag.CommandLine.SetOutput(os.Stderr) @@ -21,6 +22,7 @@ func resetFlagsForTest(t *testing.T, args []string) func() { return func() { flag.CommandLine = prevCommandLine os.Args = prevArgs + allowedDockerLabelPrefix = prevDockerLabelPrefix } } @@ -46,6 +48,38 @@ func TestInitConfig_AllowMethodFlagOverridesEnv(t *testing.T) { } } +func TestInitConfig_DockerLabelPrefixFlagOverridesEnv(t *testing.T) { + t.Setenv("SP_DOCKERLABELPREFIX", "from-env") + restore := resetFlagsForTest(t, []string{"socket-proxy", "-dockerlabelprefix=gameserver-socket-proxy"}) + defer restore() + + _, err := InitConfig() + if err != nil { + t.Fatalf("InitConfig() error = %v", err) + } + + if got, want := allowedDockerLabelPrefix, "gameserver-socket-proxy.allow."; got != want { + t.Errorf("allowedDockerLabelPrefix = %q, want %q", got, want) + } +} + +func TestInitConfig_InvalidDockerLabelPrefix(t *testing.T) { + for _, prefix := range []string{ + "Traefik", + "traefik..proxy", + "traefik-", + } { + t.Run(prefix, func(t *testing.T) { + restore := resetFlagsForTest(t, []string{"socket-proxy", "-dockerlabelprefix=" + prefix}) + defer restore() + + if _, err := InitConfig(); err == nil { + t.Fatalf("InitConfig() with dockerlabelprefix %q unexpectedly succeeded", prefix) + } + }) + } +} + func TestInitConfig_ShutdownGraceTimeTooLarge(t *testing.T) { restore := resetFlagsForTest(t, []string{ "socket-proxy", diff --git a/internal/config/dockerlabels.go b/internal/config/dockerlabels.go index 8633602..f4f720b 100644 --- a/internal/config/dockerlabels.go +++ b/internal/config/dockerlabels.go @@ -17,12 +17,13 @@ import ( ) const ( - allowedDockerLabelPrefix = "socket-proxy.allow." allowListsRefreshCooldown = 50 * time.Millisecond allowListsRefreshRetryBackoff = 10 * time.Millisecond defaultAllowListsRefreshTimeout = 10 * time.Second ) +var allowedDockerLabelPrefix = defaultDockerLabelPrefix + ".allow." + type allowListsRefreshState struct { mutex sync.Mutex inFlight *allowListsRefresh diff --git a/internal/config/dockerlabels_test.go b/internal/config/dockerlabels_test.go index e1280e8..04dddf3 100644 --- a/internal/config/dockerlabels_test.go +++ b/internal/config/dockerlabels_test.go @@ -25,6 +25,7 @@ func Test_extractLabelData(t *testing.T) { name string // description of this test case // Named input parameters for target function. cntr container.Summary + prefix string want map[string][]*regexp.Regexp want2 []string wantErr bool @@ -56,6 +57,19 @@ func Test_extractLabelData(t *testing.T) { want2: nil, wantErr: true, }, + { + name: "custom label prefix ignores the default prefix", + cntr: container.Summary{ + Labels: map[string]string{ + "socket-proxy.allow.get": "default", + "gameserver-socket-proxy.allow.get": "custom", + }, + }, + prefix: "gameserver-socket-proxy", + want: map[string][]*regexp.Regexp{ + "GET": {regexp.MustCompile("^custom$")}, + }, + }, { name: "non-allow labels are ignored", cntr: container.Summary{ @@ -71,6 +85,12 @@ func Test_extractLabelData(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + previousPrefix := allowedDockerLabelPrefix + defer func() { allowedDockerLabelPrefix = previousPrefix }() + allowedDockerLabelPrefix = defaultDockerLabelPrefix + ".allow." + if tt.prefix != "" { + allowedDockerLabelPrefix = tt.prefix + ".allow." + } got, got2, gotErr := extractLabelData(tt.cntr) if gotErr != nil { if !tt.wantErr {