fix(http): share loopback nets and redact more query secrets
- commit
- 7cfa412aa8d7114e650a7eec02b897ae658b0344
- author
- brice · · 2026-04-08 18:18 UTC
- parent
- be42624
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +9 -13
M internal/adapters/in/http/httphelper/trust.go +3 -3
| @@ -7,8 +7,8 @@ import ( | |||
| 7 | 7 | "strings" | |
| 8 | 8 | ) | |
| 9 | 9 | ||
| 10 | - | // localhostNets contains IPv4 and IPv6 loopback ranges. | |
| 11 | - | var localhostNets = ParseTrustedProxies([]string{"127.0.0.0/8", "::1"}) | |
| 10 | + | // LocalhostNets contains IPv4 and IPv6 loopback ranges. | |
| 11 | + | var LocalhostNets = ParseTrustedProxies([]string{"127.0.0.0/8", "::1"}) | |
| 12 | 12 | ||
| 13 | 13 | // ParseTrustedProxies converts a list of IP addresses and CIDR ranges to [net.IPNet]. | |
| 14 | 14 | // It accepts both single IPs (e.g., "10.0.0.1") and CIDR notation (e.g., "10.0.0.0/8"). | |
| @@ -62,7 +62,7 @@ func IsTrustedProxy(ip string, trustedNets []*net.IPNet) bool { | |||
| 62 | 62 | // IsTrustedOrLocal reports whether an IP belongs to localhost or the given | |
| 63 | 63 | // trusted proxy networks. | |
| 64 | 64 | func IsTrustedOrLocal(ip string, proxyNets []*net.IPNet) bool { | |
| 65 | - | return IsTrustedProxy(ip, localhostNets) || IsTrustedProxy(ip, proxyNets) | |
| 65 | + | return IsTrustedProxy(ip, LocalhostNets) || IsTrustedProxy(ip, proxyNets) | |
| 66 | 66 | } | |
| 67 | 67 | ||
| 68 | 68 | // ExtractRemoteIP returns the IP portion of an [http.Request.RemoteAddr], | |
| @@ -7,8 +7,8 @@ import ( | |||||
| 7 | "strings" | 7 | "strings" | ||
| 8 | ) | 8 | ) | ||
| 9 | 9 | ||||
| 10 | - | // localhostNets contains IPv4 and IPv6 loopback ranges. | 10 | + | // LocalhostNets contains IPv4 and IPv6 loopback ranges. |
| 11 | - | var localhostNets = ParseTrustedProxies([]string{"127.0.0.0/8", "::1"}) | 11 | + | var LocalhostNets = ParseTrustedProxies([]string{"127.0.0.0/8", "::1"}) |
| 12 | 12 | ||||
| 13 | // ParseTrustedProxies converts a list of IP addresses and CIDR ranges to [net.IPNet]. | 13 | // ParseTrustedProxies converts a list of IP addresses and CIDR ranges to [net.IPNet]. | ||
| 14 | // It accepts both single IPs (e.g., "10.0.0.1") and CIDR notation (e.g., "10.0.0.0/8"). | 14 | // It accepts both single IPs (e.g., "10.0.0.1") and CIDR notation (e.g., "10.0.0.0/8"). | ||
| @@ -62,7 +62,7 @@ func IsTrustedProxy(ip string, trustedNets []*net.IPNet) bool { | |||||
| 62 | // IsTrustedOrLocal reports whether an IP belongs to localhost or the given | 62 | // IsTrustedOrLocal reports whether an IP belongs to localhost or the given | ||
| 63 | // trusted proxy networks. | 63 | // trusted proxy networks. | ||
| 64 | func IsTrustedOrLocal(ip string, proxyNets []*net.IPNet) bool { | 64 | func IsTrustedOrLocal(ip string, proxyNets []*net.IPNet) bool { | ||
| 65 | - | return IsTrustedProxy(ip, localhostNets) || IsTrustedProxy(ip, proxyNets) | 65 | + | return IsTrustedProxy(ip, LocalhostNets) || IsTrustedProxy(ip, proxyNets) |
| 66 | } | 66 | } | ||
| 67 | 67 | ||||
| 68 | // ExtractRemoteIP returns the IP portion of an [http.Request.RemoteAddr], | 68 | // ExtractRemoteIP returns the IP portion of an [http.Request.RemoteAddr], | ||
M internal/adapters/in/http/middleware/accesslog.go +3 -4
| @@ -42,11 +42,10 @@ func AccessLogger(writer out.AccessLogWriter, excludeHealthChecks bool, log zero | |||
| 42 | 42 | // Apply health-check exclusion after the response so the request | |
| 43 | 43 | // always completes — we just skip writing the log entry. | |
| 44 | 44 | // UA check uses the shared domain constant so the prober and this | |
| 45 | - | // filter can never drift. Loopback check reuses localhostNets from | |
| 46 | - | // cidr.go (same package) — no separate loopback definition needed. | |
| 45 | + | // filter can never drift. Loopback check uses the shared helper list. | |
| 47 | 46 | if excludeHealthChecks { | |
| 48 | 47 | if strings.HasPrefix(r.UserAgent(), out.HealthCheckUserAgentPrefix) || | |
| 49 | - | httphelper.IsTrustedProxy(clientIP, localhostNets) { | |
| 48 | + | httphelper.IsTrustedProxy(clientIP, httphelper.LocalhostNets) { | |
| 50 | 49 | return | |
| 51 | 50 | } | |
| 52 | 51 | } | |
| @@ -121,7 +120,7 @@ func sanitizeLoggedQuery(rawQuery string) string { | |||
| 121 | 120 | ||
| 122 | 121 | func isSensitiveQueryKey(key string) bool { | |
| 123 | 122 | switch strings.ToLower(key) { | |
| 124 | - | case "access_token", "auth", "code", "key", "password", "refresh_token", "reset", "secret", "sig", "signature", "token": | |
| 123 | + | case "access_token", "api_key", "apikey", "auth", "bearer", "code", "credential", "credentials", "key", "password", "private_key", "refresh_token", "reset", "secret", "session", "session_id", "sig", "signature", "token": | |
| 125 | 124 | return true | |
| 126 | 125 | default: | |
| 127 | 126 | return false | |
| @@ -42,11 +42,10 @@ func AccessLogger(writer out.AccessLogWriter, excludeHealthChecks bool, log zero | |||||
| 42 | // Apply health-check exclusion after the response so the request | 42 | // Apply health-check exclusion after the response so the request | ||
| 43 | // always completes — we just skip writing the log entry. | 43 | // always completes — we just skip writing the log entry. | ||
| 44 | // UA check uses the shared domain constant so the prober and this | 44 | // UA check uses the shared domain constant so the prober and this | ||
| 45 | - | // filter can never drift. Loopback check reuses localhostNets from | 45 | + | // filter can never drift. Loopback check uses the shared helper list. |
| 46 | - | // cidr.go (same package) — no separate loopback definition needed. | |||
| 47 | if excludeHealthChecks { | 46 | if excludeHealthChecks { | ||
| 48 | if strings.HasPrefix(r.UserAgent(), out.HealthCheckUserAgentPrefix) || | 47 | if strings.HasPrefix(r.UserAgent(), out.HealthCheckUserAgentPrefix) || | ||
| 49 | - | httphelper.IsTrustedProxy(clientIP, localhostNets) { | 48 | + | httphelper.IsTrustedProxy(clientIP, httphelper.LocalhostNets) { |
| 50 | return | 49 | return | ||
| 51 | } | 50 | } | ||
| 52 | } | 51 | } | ||
| @@ -121,7 +120,7 @@ func sanitizeLoggedQuery(rawQuery string) string { | |||||
| 121 | 120 | ||||
| 122 | func isSensitiveQueryKey(key string) bool { | 121 | func isSensitiveQueryKey(key string) bool { | ||
| 123 | switch strings.ToLower(key) { | 122 | switch strings.ToLower(key) { | ||
| 124 | - | case "access_token", "auth", "code", "key", "password", "refresh_token", "reset", "secret", "sig", "signature", "token": | 123 | + | case "access_token", "api_key", "apikey", "auth", "bearer", "code", "credential", "credentials", "key", "password", "private_key", "refresh_token", "reset", "secret", "session", "session_id", "sig", "signature", "token": |
| 125 | return true | 124 | return true | ||
| 126 | default: | 125 | default: | ||
| 127 | return false | 126 | return false | ||
M internal/adapters/in/http/middleware/accesslog_test.go +2 -2
| @@ -69,7 +69,7 @@ func TestAccessLogger_RedactsSensitiveQueryParams(t *testing.T) { | |||
| 69 | 69 | server := httptest.NewServer(handler) | |
| 70 | 70 | defer server.Close() | |
| 71 | 71 | ||
| 72 | - | resp, err := http.Get(server.URL + "/oauth/callback?code=secret-code&state=ok&token=secret-token") | |
| 72 | + | resp, err := http.Get(server.URL + "/oauth/callback?code=secret-code&state=ok&token=secret-token&api_key=top-secret&session_id=session-secret") | |
| 73 | 73 | require.NoError(t, err) | |
| 74 | 74 | defer func() { | |
| 75 | 75 | assert.NoError(t, resp.Body.Close()) | |
| @@ -78,7 +78,7 @@ func TestAccessLogger_RedactsSensitiveQueryParams(t *testing.T) { | |||
| 78 | 78 | require.NoError(t, err) | |
| 79 | 79 | ||
| 80 | 80 | require.Len(t, mock.entries, 1) | |
| 81 | - | assert.Equal(t, "code=[REDACTED]&state=ok&token=[REDACTED]", mock.entries[0].Query) | |
| 81 | + | assert.Equal(t, "code=[REDACTED]&state=ok&token=[REDACTED]&api_key=[REDACTED]&session_id=[REDACTED]", mock.entries[0].Query) | |
| 82 | 82 | } | |
| 83 | 83 | ||
| 84 | 84 | func TestAccessLogger_RequestIDReused(t *testing.T) { | |
| @@ -69,7 +69,7 @@ func TestAccessLogger_RedactsSensitiveQueryParams(t *testing.T) { | |||||
| 69 | server := httptest.NewServer(handler) | 69 | server := httptest.NewServer(handler) | ||
| 70 | defer server.Close() | 70 | defer server.Close() | ||
| 71 | 71 | ||||
| 72 | - | resp, err := http.Get(server.URL + "/oauth/callback?code=secret-code&state=ok&token=secret-token") | 72 | + | resp, err := http.Get(server.URL + "/oauth/callback?code=secret-code&state=ok&token=secret-token&api_key=top-secret&session_id=session-secret") |
| 73 | require.NoError(t, err) | 73 | require.NoError(t, err) | ||
| 74 | defer func() { | 74 | defer func() { | ||
| 75 | assert.NoError(t, resp.Body.Close()) | 75 | assert.NoError(t, resp.Body.Close()) | ||
| @@ -78,7 +78,7 @@ func TestAccessLogger_RedactsSensitiveQueryParams(t *testing.T) { | |||||
| 78 | require.NoError(t, err) | 78 | require.NoError(t, err) | ||
| 79 | 79 | ||||
| 80 | require.Len(t, mock.entries, 1) | 80 | require.Len(t, mock.entries, 1) | ||
| 81 | - | assert.Equal(t, "code=[REDACTED]&state=ok&token=[REDACTED]", mock.entries[0].Query) | 81 | + | assert.Equal(t, "code=[REDACTED]&state=ok&token=[REDACTED]&api_key=[REDACTED]&session_id=[REDACTED]", mock.entries[0].Query) |
| 82 | } | 82 | } | ||
| 83 | 83 | ||||
| 84 | func TestAccessLogger_RequestIDReused(t *testing.T) { | 84 | func TestAccessLogger_RequestIDReused(t *testing.T) { | ||
M internal/adapters/in/http/middleware/cidr.go +1 -4
| @@ -13,9 +13,6 @@ import ( | |||
| 13 | 13 | "github.com/bnema/gordon/internal/adapters/in/http/httphelper" | |
| 14 | 14 | ) | |
| 15 | 15 | ||
| 16 | - | // localhostNets contains IPv4 and IPv6 loopback ranges that are always allowed. | |
| 17 | - | var localhostNets = httphelper.ParseTrustedProxies([]string{"127.0.0.0/8", "::1"}) | |
| 18 | - | ||
| 19 | 16 | // cidrAllowlist is the shared implementation for CIDR-based access control middleware. | |
| 20 | 17 | // ipExtractor determines how the client IP is obtained from the request. | |
| 21 | 18 | // logLabel is used in the deny log message (e.g. "registry", "proxy origin"). | |
| @@ -27,7 +24,7 @@ func cidrAllowlist(allowedNets []*net.IPNet, ipExtractor func(*http.Request) str | |||
| 27 | 24 | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 28 | 25 | clientIP := ipExtractor(r) | |
| 29 | 26 | ||
| 30 | - | if httphelper.IsTrustedProxy(clientIP, localhostNets) { | |
| 27 | + | if httphelper.IsTrustedProxy(clientIP, httphelper.LocalhostNets) { | |
| 31 | 28 | next.ServeHTTP(w, r) | |
| 32 | 29 | return | |
| 33 | 30 | } | |
| @@ -13,9 +13,6 @@ import ( | |||||
| 13 | "github.com/bnema/gordon/internal/adapters/in/http/httphelper" | 13 | "github.com/bnema/gordon/internal/adapters/in/http/httphelper" | ||
| 14 | ) | 14 | ) | ||
| 15 | 15 | ||||
| 16 | - | // localhostNets contains IPv4 and IPv6 loopback ranges that are always allowed. | |||
| 17 | - | var localhostNets = httphelper.ParseTrustedProxies([]string{"127.0.0.0/8", "::1"}) | |||
| 18 | - | ||||
| 19 | // cidrAllowlist is the shared implementation for CIDR-based access control middleware. | 16 | // cidrAllowlist is the shared implementation for CIDR-based access control middleware. | ||
| 20 | // ipExtractor determines how the client IP is obtained from the request. | 17 | // ipExtractor determines how the client IP is obtained from the request. | ||
| 21 | // logLabel is used in the deny log message (e.g. "registry", "proxy origin"). | 18 | // logLabel is used in the deny log message (e.g. "registry", "proxy origin"). | ||
| @@ -27,7 +24,7 @@ func cidrAllowlist(allowedNets []*net.IPNet, ipExtractor func(*http.Request) str | |||||
| 27 | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 24 | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| 28 | clientIP := ipExtractor(r) | 25 | clientIP := ipExtractor(r) | ||
| 29 | 26 | ||||
| 30 | - | if httphelper.IsTrustedProxy(clientIP, localhostNets) { | 27 | + | if httphelper.IsTrustedProxy(clientIP, httphelper.LocalhostNets) { |
| 31 | next.ServeHTTP(w, r) | 28 | next.ServeHTTP(w, r) | ||
| 32 | return | 29 | return | ||
| 33 | } | 30 | } | ||