fix(app,cli): fail closed on invalid allowlist and log close errors

Remove unused internal auth setup config arg, deny registry traffic when registry_allowed_ips contains only invalid entries, and surface local control-plane cleanup errors. Also simplify remote control-plane interface test by removing self-helper usage.

commit: 1dae7a96126445cb25f3a208a63f8df71906dc1b
author: brice · 8mo ago · 2026-02-09 06:18 UTC
parent: 25e14b0
signature: unsigned

4 files · +45 -5

M internal/adapters/in/cli/controlplane_remote_test.go +0 -2


@@ -9,8 +9,6 @@ import (
9	9		var _ ControlPlane = (*remoteControlPlane)(nil)
10	10
11	11		func TestRemoteControlPlane_ImplementsInterface(t *testing.T) {
12		-	t.Helper()
13		-
14	12		client := remote.NewClient("https://gordon.example.com")
15	13		if NewRemoteControlPlane(client) == nil {
16	14		t.Fatal("expected non-nil remote control-plane")


@@ -9,8 +9,6 @@ import (
9		var _ ControlPlane = (*remoteControlPlane)(nil)	9	var _ ControlPlane = (*remoteControlPlane)(nil)
10			10
11		func TestRemoteControlPlane_ImplementsInterface(t *testing.T) {	11	func TestRemoteControlPlane_ImplementsInterface(t *testing.T) {
12	-	t.Helper()
13	-
14		client := remote.NewClient("https://gordon.example.com")	12	client := remote.NewClient("https://gordon.example.com")
15		if NewRemoteControlPlane(client) == nil {	13	if NewRemoteControlPlane(client) == nil {
16		t.Fatal("expected non-nil remote control-plane")	14	t.Fatal("expected non-nil remote control-plane")

M internal/adapters/in/cli/controlplane_resolver.go +6 -1


@@ -3,6 +3,8 @@ package cli
3	3		import (
4	4		"fmt"
5	5
	6	+	"github.com/bnema/zerowrap"
	7	+
6	8		"github.com/bnema/gordon/internal/app"
7	9		)
8	10

@@ -16,7 +18,10 @@ func (h *controlPlaneHandle) close() {
16	18		if h == nil \|\| h.closeFn == nil {
17	19		return
18	20		}
19		-	_ = h.closeFn()
	21	+	if err := h.closeFn(); err != nil {
	22	+	log := zerowrap.Default()
	23	+	log.Warn().Err(err).Msg("failed to close control-plane resources")
	24	+	}
20	25		}
21	26
22	27		func resolveControlPlane(configPath string) (*controlPlaneHandle, error) {

 import (
 	"fmt"
+	"github.com/bnema/zerowrap"
 	"github.com/bnema/gordon/internal/app"
 )
 	if h == nil || h.closeFn == nil {
 		return
 	}
-	_ = h.closeFn()
+	if err := h.closeFn(); err != nil {
+		log := zerowrap.Default()
+		log.Warn().Err(err).Msg("failed to close control-plane resources")
+	}
 }
 func resolveControlPlane(configPath string) (*controlPlaneHandle, error) {

M internal/app/run.go +13 -2


@@ -434,7 +434,7 @@ func createServices(ctx context.Context, v *viper.Viper, cfg Config, log zerowra
434	434		return nil, err
435	435		}
436	436
437		-	if err := setupInternalRegistryAuth(cfg, svc, log); err != nil {
	437	+	if err := setupInternalRegistryAuth(svc, log); err != nil {
438	438		return nil, err
439	439		}
440	440

@@ -516,7 +516,7 @@ func createServices(ctx context.Context, v *viper.Viper, cfg Config, log zerowra
516	516		return svc, nil
517	517		}
518	518
519		-	func setupInternalRegistryAuth(_ Config, svc *services, log zerowrap.Logger) error {
	519	+	func setupInternalRegistryAuth(svc *services, log zerowrap.Logger) error {
520	520		var err error
521	521		svc.internalRegUser, svc.internalRegPass, err = generateInternalRegistryAuth()
522	522		if err != nil {

@@ -1381,6 +1381,17 @@ func buildRegistryCIDRAllowlistMiddleware(cfg Config, trustedNets []*net.IPNet,
1381	1381		log.Error().
1382	1382		Strs("registry_allowed_ips", cfg.Server.RegistryAllowedIPs).
1383	1383		Msg("registry_allowed_ips is set but no valid entries were parsed; registry will allow all traffic")
	1384	+	return func(next http.Handler) http.Handler {
	1385	+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	1386	+	log.Warn().
	1387	+	Str(zerowrap.FieldPath, r.URL.Path).
	1388	+	Str(zerowrap.FieldClientIP, middleware.GetClientIP(r, trustedNets)).
	1389	+	Msg("registry access denied due to invalid registry_allowed_ips configuration")
	1390	+	w.Header().Set("Content-Type", "application/json")
	1391	+	w.WriteHeader(http.StatusForbidden)
	1392	+	_ = json.NewEncoder(w).Encode(dto.ErrorResponse{Error: "Forbidden"})
	1393	+	})
	1394	+	}
1384	1395		}
1385	1396
1386	1397		return middleware.RegistryCIDRAllowlist(allowedNets, trustedNets, log)

 		return nil, err
 	}
-	if err := setupInternalRegistryAuth(cfg, svc, log); err != nil {
+	if err := setupInternalRegistryAuth(svc, log); err != nil {
 		return nil, err
 	}
 	return svc, nil
 }
-func setupInternalRegistryAuth(_ Config, svc *services, log zerowrap.Logger) error {
+func setupInternalRegistryAuth(svc *services, log zerowrap.Logger) error {
 	var err error
 	svc.internalRegUser, svc.internalRegPass, err = generateInternalRegistryAuth()
 	if err != nil {
 		log.Error().
 			Strs("registry_allowed_ips", cfg.Server.RegistryAllowedIPs).
 			Msg("registry_allowed_ips is set but no valid entries were parsed; registry will allow all traffic")
+		return func(next http.Handler) http.Handler {
+			return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				log.Warn().
+					Str(zerowrap.FieldPath, r.URL.Path).
+					Str(zerowrap.FieldClientIP, middleware.GetClientIP(r, trustedNets)).
+					Msg("registry access denied due to invalid registry_allowed_ips configuration")
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(http.StatusForbidden)
+				_ = json.NewEncoder(w).Encode(dto.ErrorResponse{Error: "Forbidden"})
+			})
+		}
 	}
 	return middleware.RegistryCIDRAllowlist(allowedNets, trustedNets, log)

M internal/app/run_http_handlers_test.go +26 -0


@@ -86,3 +86,29 @@ func TestCreateHTTPHandlers_AuthEnabled_ExposesAdminOnProxy(t *testing.T) {
86	86		t.Fatalf("expected status %d, got %d", http.StatusServiceUnavailable, rec.Code)
87	87		}
88	88		}
	89	+
	90	+	func TestBuildRegistryCIDRAllowlistMiddleware_InvalidEntries_DenyAll(t *testing.T) {
	91	+	t.Parallel()
	92	+
	93	+	cfg := Config{}
	94	+	cfg.Server.RegistryAllowedIPs = []string{"not-a-cidr"}
	95	+
	96	+	mw := buildRegistryCIDRAllowlistMiddleware(cfg, nil, zerowrap.Default())
	97	+	if mw == nil {
	98	+	t.Fatal("expected non-nil allowlist middleware")
	99	+	}
	100	+
	101	+	handler := mw(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
	102	+	w.WriteHeader(http.StatusOK)
	103	+	}))
	104	+
	105	+	req := httptest.NewRequest(http.MethodGet, "/v2/test/manifests/latest", nil)
	106	+	req.RemoteAddr = "127.0.0.1:12345"
	107	+
	108	+	rec := httptest.NewRecorder()
	109	+	handler.ServeHTTP(rec, req)
	110	+
	111	+	if rec.Code != http.StatusForbidden {
	112	+	t.Fatalf("expected status %d, got %d", http.StatusForbidden, rec.Code)
	113	+	}
	114	+	}


@@ -86,3 +86,29 @@ func TestCreateHTTPHandlers_AuthEnabled_ExposesAdminOnProxy(t *testing.T) {
86	t.Fatalf("expected status %d, got %d", http.StatusServiceUnavailable, rec.Code)	86		t.Fatalf("expected status %d, got %d", http.StatusServiceUnavailable, rec.Code)
87	}	87		}
88	}	88		}
		89	+
		90	+	func TestBuildRegistryCIDRAllowlistMiddleware_InvalidEntries_DenyAll(t *testing.T) {
		91	+	t.Parallel()
		92	+
		93	+	cfg := Config{}
		94	+	cfg.Server.RegistryAllowedIPs = []string{"not-a-cidr"}
		95	+
		96	+	mw := buildRegistryCIDRAllowlistMiddleware(cfg, nil, zerowrap.Default())
		97	+	if mw == nil {
		98	+	t.Fatal("expected non-nil allowlist middleware")
		99	+	}
		100	+
		101	+	handler := mw(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
		102	+	w.WriteHeader(http.StatusOK)
		103	+	}))
		104	+
		105	+	req := httptest.NewRequest(http.MethodGet, "/v2/test/manifests/latest", nil)
		106	+	req.RemoteAddr = "127.0.0.1:12345"
		107	+
		108	+	rec := httptest.NewRecorder()
		109	+	handler.ServeHTTP(rec, req)
		110	+
		111	+	if rec.Code != http.StatusForbidden {
		112	+	t.Fatalf("expected status %d, got %d", http.StatusForbidden, rec.Code)
		113	+	}
		114	+	}