detached c9b8983

test: add outer-deny access log coverage for loopback and fail-closed paths

commit
c9b89835808be21b64951472fa36346ef9957968
author
brice · · 2026-04-03 15:24 UTC
parent
2147872
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
1 file · +104 -0
Unified
UnifiedSplit
M internal/app/run_http_handlers_test.go +104 -0
@@ -7,6 +7,7 @@ import (
77 "net/http/httptest"
88 "os"
99 "path/filepath"
10+ "sync"
1011 "testing"
1112
1213 "github.com/bnema/zerowrap"
@@ -14,9 +15,31 @@ import (
1415 "github.com/stretchr/testify/require"
1516
1617 adminhttp "github.com/bnema/gordon/internal/adapters/in/http/admin"
18+ "github.com/bnema/gordon/internal/adapters/out/accesslog"
1719 pkiadapter "github.com/bnema/gordon/internal/adapters/out/pki"
1820 )
1921
22+// testAccessLogWriter is a thread-safe mock AccessLogWriter for tests.
23+type testAccessLogWriter struct {
24+ mu sync.Mutex
25+ entries []accesslog.Entry
26+}
27+
28+func (w *testAccessLogWriter) Write(entry accesslog.Entry) error {
29+ w.mu.Lock()
30+ defer w.mu.Unlock()
31+ w.entries = append(w.entries, entry)
32+ return nil
33+}
34+
35+func (w *testAccessLogWriter) snapshot() []accesslog.Entry {
36+ w.mu.Lock()
37+ defer w.mu.Unlock()
38+ out := make([]accesslog.Entry, len(w.entries))
39+ copy(out, w.entries)
40+ return out
41+}
42+
2043 func TestCreateAuthService_Disabled_ReturnsNilServices(t *testing.T) {
2144 t.Parallel()
2245
@@ -399,3 +422,84 @@ func TestBuildRegistryCIDRAllowlistMiddleware_InvalidEntries_DenyAll(t *testing.
399422 t.Fatalf("expected status %d, got %d", http.StatusForbidden, rec.Code)
400423 }
401424 }
425+
426+// TestAccessLog_AdminRejectedByLoopbackOnly_IsLogged verifies that requests
427+// to /admin/ blocked by loopbackOnly() still produce an access-log entry.
428+// This exercises the outer-deny path: AccessLogger wraps the top-level
429+// handler so even gates that run before any inner middleware are logged.
430+func TestAccessLog_AdminRejectedByLoopbackOnly_IsLogged(t *testing.T) {
431+ t.Parallel()
432+
433+ cfg := Config{}
434+ cfg.Auth.Enabled = true // required for admin routes to be registered
435+
436+ svc := &services{adminHandler: &adminhttp.Handler{}}
437+ aw := &testAccessLogWriter{}
438+ registryHandler, _, _ := createHTTPHandlers(svc, cfg, zerowrap.Default(), aw)
439+
440+ // Non-loopback IP — loopbackOnly() will reject this with 403 before any
441+ // inner middleware runs.
442+ req := httptest.NewRequest(http.MethodGet, "/admin/status", nil)
443+ req.RemoteAddr = "192.0.2.10:12345"
444+
445+ rec := httptest.NewRecorder()
446+ registryHandler.ServeHTTP(rec, req)
447+
448+ require.Equal(t, http.StatusForbidden, rec.Code)
449+ entries := aw.snapshot()
450+ require.Len(t, entries, 1, "access log must capture the loopback-rejected admin request")
451+ assert.Equal(t, http.StatusForbidden, entries[0].Status)
452+ assert.Equal(t, "/admin/status", entries[0].Path)
453+}
454+
455+// TestAccessLog_RegistryRejectedByLoopbackOnly_IsLogged verifies that requests
456+// to /v2/ blocked by loopbackOnly() in local mode still produce an access-log entry.
457+func TestAccessLog_RegistryRejectedByLoopbackOnly_IsLogged(t *testing.T) {
458+ t.Parallel()
459+
460+ cfg := Config{}
461+ cfg.Auth.Enabled = false // local mode: /v2/ is wrapped by loopbackOnly()
462+
463+ svc := &services{}
464+ aw := &testAccessLogWriter{}
465+ registryHandler, _, _ := createHTTPHandlers(svc, cfg, zerowrap.Default(), aw)
466+
467+ // Non-loopback IP — loopbackOnly() rejects with 403.
468+ req := httptest.NewRequest(http.MethodGet, "/v2/", nil)
469+ req.RemoteAddr = "192.0.2.20:12345"
470+
471+ rec := httptest.NewRecorder()
472+ registryHandler.ServeHTTP(rec, req)
473+
474+ require.Equal(t, http.StatusForbidden, rec.Code)
475+ entries := aw.snapshot()
476+ require.Len(t, entries, 1, "access log must capture the loopback-rejected registry request")
477+ assert.Equal(t, http.StatusForbidden, entries[0].Status)
478+}
479+
480+// TestAccessLog_DenyAllProxy_IsLogged verifies that requests rejected by the
481+// fail-closed denyAllHandler (triggered by an all-invalid proxy_allowed_ips
482+// config) still produce an access-log entry.
483+func TestAccessLog_DenyAllProxy_IsLogged(t *testing.T) {
484+ t.Parallel()
485+
486+ cfg := Config{}
487+ // All-invalid entries cause buildProxyCIDRAllowlistMiddleware to return
488+ // denyAllHandler, which wraps the proxy mux unconditionally with 403.
489+ cfg.Server.ProxyAllowedIPs = []string{"not-a-valid-ip"}
490+
491+ svc := &services{}
492+ aw := &testAccessLogWriter{}
493+ _, httpProxyHandler, _ := createHTTPHandlers(svc, cfg, zerowrap.Default(), aw)
494+
495+ req := httptest.NewRequest(http.MethodGet, "/", nil)
496+ req.RemoteAddr = "203.0.113.5:54321"
497+
498+ rec := httptest.NewRecorder()
499+ httpProxyHandler.ServeHTTP(rec, req)
500+
501+ require.Equal(t, http.StatusForbidden, rec.Code)
502+ entries := aw.snapshot()
503+ require.Len(t, entries, 1, "access log must capture the fail-closed proxy denial")
504+ assert.Equal(t, http.StatusForbidden, entries[0].Status)
505+}
@@ -7,6 +7,7 @@ import (
7 "net/http/httptest"7 "net/http/httptest"
8 "os"8 "os"
9 "path/filepath"9 "path/filepath"
10+ "sync"
10 "testing"11 "testing"
11 12
12 "github.com/bnema/zerowrap"13 "github.com/bnema/zerowrap"
@@ -14,9 +15,31 @@ import (
14 "github.com/stretchr/testify/require"15 "github.com/stretchr/testify/require"
15 16
16 adminhttp "github.com/bnema/gordon/internal/adapters/in/http/admin"17 adminhttp "github.com/bnema/gordon/internal/adapters/in/http/admin"
18+ "github.com/bnema/gordon/internal/adapters/out/accesslog"
17 pkiadapter "github.com/bnema/gordon/internal/adapters/out/pki"19 pkiadapter "github.com/bnema/gordon/internal/adapters/out/pki"
18 )20 )
19 21
22+// testAccessLogWriter is a thread-safe mock AccessLogWriter for tests.
23+type testAccessLogWriter struct {
24+ mu sync.Mutex
25+ entries []accesslog.Entry
26+}
27+
28+func (w *testAccessLogWriter) Write(entry accesslog.Entry) error {
29+ w.mu.Lock()
30+ defer w.mu.Unlock()
31+ w.entries = append(w.entries, entry)
32+ return nil
33+}
34+
35+func (w *testAccessLogWriter) snapshot() []accesslog.Entry {
36+ w.mu.Lock()
37+ defer w.mu.Unlock()
38+ out := make([]accesslog.Entry, len(w.entries))
39+ copy(out, w.entries)
40+ return out
41+}
42+
20 func TestCreateAuthService_Disabled_ReturnsNilServices(t *testing.T) {43 func TestCreateAuthService_Disabled_ReturnsNilServices(t *testing.T) {
21 t.Parallel()44 t.Parallel()
22 45
@@ -399,3 +422,84 @@ func TestBuildRegistryCIDRAllowlistMiddleware_InvalidEntries_DenyAll(t *testing.
399 t.Fatalf("expected status %d, got %d", http.StatusForbidden, rec.Code)422 t.Fatalf("expected status %d, got %d", http.StatusForbidden, rec.Code)
400 }423 }
401 }424 }
425+
426+// TestAccessLog_AdminRejectedByLoopbackOnly_IsLogged verifies that requests
427+// to /admin/ blocked by loopbackOnly() still produce an access-log entry.
428+// This exercises the outer-deny path: AccessLogger wraps the top-level
429+// handler so even gates that run before any inner middleware are logged.
430+func TestAccessLog_AdminRejectedByLoopbackOnly_IsLogged(t *testing.T) {
431+ t.Parallel()
432+
433+ cfg := Config{}
434+ cfg.Auth.Enabled = true // required for admin routes to be registered
435+
436+ svc := &services{adminHandler: &adminhttp.Handler{}}
437+ aw := &testAccessLogWriter{}
438+ registryHandler, _, _ := createHTTPHandlers(svc, cfg, zerowrap.Default(), aw)
439+
440+ // Non-loopback IP — loopbackOnly() will reject this with 403 before any
441+ // inner middleware runs.
442+ req := httptest.NewRequest(http.MethodGet, "/admin/status", nil)
443+ req.RemoteAddr = "192.0.2.10:12345"
444+
445+ rec := httptest.NewRecorder()
446+ registryHandler.ServeHTTP(rec, req)
447+
448+ require.Equal(t, http.StatusForbidden, rec.Code)
449+ entries := aw.snapshot()
450+ require.Len(t, entries, 1, "access log must capture the loopback-rejected admin request")
451+ assert.Equal(t, http.StatusForbidden, entries[0].Status)
452+ assert.Equal(t, "/admin/status", entries[0].Path)
453+}
454+
455+// TestAccessLog_RegistryRejectedByLoopbackOnly_IsLogged verifies that requests
456+// to /v2/ blocked by loopbackOnly() in local mode still produce an access-log entry.
457+func TestAccessLog_RegistryRejectedByLoopbackOnly_IsLogged(t *testing.T) {
458+ t.Parallel()
459+
460+ cfg := Config{}
461+ cfg.Auth.Enabled = false // local mode: /v2/ is wrapped by loopbackOnly()
462+
463+ svc := &services{}
464+ aw := &testAccessLogWriter{}
465+ registryHandler, _, _ := createHTTPHandlers(svc, cfg, zerowrap.Default(), aw)
466+
467+ // Non-loopback IP — loopbackOnly() rejects with 403.
468+ req := httptest.NewRequest(http.MethodGet, "/v2/", nil)
469+ req.RemoteAddr = "192.0.2.20:12345"
470+
471+ rec := httptest.NewRecorder()
472+ registryHandler.ServeHTTP(rec, req)
473+
474+ require.Equal(t, http.StatusForbidden, rec.Code)
475+ entries := aw.snapshot()
476+ require.Len(t, entries, 1, "access log must capture the loopback-rejected registry request")
477+ assert.Equal(t, http.StatusForbidden, entries[0].Status)
478+}
479+
480+// TestAccessLog_DenyAllProxy_IsLogged verifies that requests rejected by the
481+// fail-closed denyAllHandler (triggered by an all-invalid proxy_allowed_ips
482+// config) still produce an access-log entry.
483+func TestAccessLog_DenyAllProxy_IsLogged(t *testing.T) {
484+ t.Parallel()
485+
486+ cfg := Config{}
487+ // All-invalid entries cause buildProxyCIDRAllowlistMiddleware to return
488+ // denyAllHandler, which wraps the proxy mux unconditionally with 403.
489+ cfg.Server.ProxyAllowedIPs = []string{"not-a-valid-ip"}
490+
491+ svc := &services{}
492+ aw := &testAccessLogWriter{}
493+ _, httpProxyHandler, _ := createHTTPHandlers(svc, cfg, zerowrap.Default(), aw)
494+
495+ req := httptest.NewRequest(http.MethodGet, "/", nil)
496+ req.RemoteAddr = "203.0.113.5:54321"
497+
498+ rec := httptest.NewRecorder()
499+ httpProxyHandler.ServeHTTP(rec, req)
500+
501+ require.Equal(t, http.StatusForbidden, rec.Code)
502+ entries := aw.snapshot()
503+ require.Len(t, entries, 1, "access log must capture the fail-closed proxy denial")
504+ assert.Equal(t, http.StatusForbidden, entries[0].Status)
505+}

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 7

Tags 106

↑↓ moveenter openesc close