28 fr support granular api permissions on a per host basis via allowfrom#75
Merged
wollomatic merged 45 commits intoDec 4, 2025
Conversation
… container name is provided
Bumps golang from 1.25.1-alpine3.22 to 1.25.3-alpine3.22. --- updated-dependencies: - dependency-name: golang dependency-version: 1.25.3-alpine3.22 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…3-alpine3.22 Bump golang from 1.25.1-alpine3.22 to 1.25.3-alpine3.22
Updated usage instructions for Traefik due to API version changes. Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
Added information about allowing requests to /_ping in README. Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
Updated usage instructions for Traefik configuration to reflect changes in how Traefik retrieves the Docker API version. Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
Updated Traefik usage instructions and removed outdated information. Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
To fix number of retries when querying Docker Engine API for proxy container summary. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Amanda Wee <amanda@aranel.net>
To fix formatting of SP_PROXYCONTAINERNAME Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Amanda Wee <amanda@aranel.net>
implement per-container allowlists specified by Docker container labels
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request introduces per-container allowlist support to socket-proxy, enabling fine-grained control over Docker API access on a per-container basis through Docker labels. The implementation includes a custom Docker client library (extracted from the official Docker SDK), comprehensive allowlist management with event-driven updates, and refactored bind mount restrictions to support the new allowlist architecture.
- Adds Docker client library under
internal/dockerandinternal/go-connectionswith API version negotiation and event streaming capabilities - Implements
AllowListRegistrywith thread-safe per-IP allowlist management, automatically updated via Docker events (container start/restart/die) - Refactors bind mount restriction functions to accept allowlist parameters, enabling per-container bind mount policies
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/go-connections/sockets/sockets.go | Helper functions for Unix/TCP socket configuration extracted from Docker's go-connections library |
| internal/docker/client/*.go | Custom Docker API client implementation with request handling, ping, events, and container listing |
| internal/docker/api/**/*.go | Docker API type definitions for containers, events, filters, and version comparison |
| internal/config/config.go | Major refactoring introducing AllowListRegistry with per-container allowlist support via Docker labels, event-driven updates, and mutex-protected IP-based lookups |
| cmd/socket-proxy/main.go | Initializes non-default allowlists as a goroutine when proxy container name is provided |
| cmd/socket-proxy/handlehttprequest.go | Refactored to determine appropriate allowlist (default or per-container) based on client IP |
| cmd/socket-proxy/bindmount.go | Updated all bind mount validation functions to accept allowedBindMounts as a parameter |
| cmd/socket-proxy/bindmount_test.go | Updated tests to pass allowedBindMounts directly to validation functions |
| README.md | Documents per-container allowlist feature, Traefik compatibility note, and fixes spelling error |
| LICENSE | Updates Apache 2.0 attribution to include new internal/docker and internal/go-connections files |
| Dockerfile | Updates Go base image from 1.25.1 to 1.25.3 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+278
to
+342
| 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 dockerClient.Close() | ||
|
|
||
| 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 | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The UpdateAllowLists function runs in an infinite loop (lines 311-341) and is designed to be called as a goroutine. However, there's no mechanism to gracefully shut down this goroutine. Consider accepting a context parameter from the caller (e.g., from main) instead of creating one internally, so the goroutine can be properly cancelled on application shutdown. This would prevent potential resource leaks and allow for cleaner shutdowns.
Owner
Author
There was a problem hiding this comment.
Let's keep this suggestion for the next release
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
…ssions-on-a-per-host-basis-via-allowfrom' into 28-fr-support-granular-api-permissions-on-a-per-host-basis-via-allowfrom
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Wolfgang Ellsässer <67168186+wollomatic@users.noreply.github.com>
wollomatic
deleted the
28-fr-support-granular-api-permissions-on-a-per-host-basis-via-allowfrom
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces support for per-container allowlists in
socket-proxy, allowing more granular control over allowed HTTP methods and bind mount restrictions based on Docker container labels. It refactors the bind mount restriction logic to use allowlists passed per request, updates documentation to reflect these new features, and makes minor improvements to licensing and dependency references.Per-container allowlist support:
-proxycontainernameparameter. (README.md,cmd/socket-proxy/handlehttprequest.go, [1] [2] [3]cmd/socket-proxy/bindmount.goto accept the allowed bind mounts as a parameter, supporting the new per-container allowlist logic. [1] [2] [3] [4] [5]Documentation updates:
README.mdto document per-container allowlists, new configuration options, and usage notes for compatibility with recent Traefik versions. [1] [2] [3] [4]Testing and code maintenance:
cmd/socket-proxy/bindmount_test.goto use the new function signatures for bind mount validation and restriction checks. [1] [2] [3] [4] [5] [6] [7]Licensing and dependencies:
LICENSEandREADME.mdto clarify that files underinternal/dockerandinternal/go-connectionsare licensed under Apache 2.0. [1] [2]Dockerfilefrom1.25.1-alpine3.22to1.25.3-alpine3.22.