Update golangci-lint to v2.12.2 with canonical config
All checks were successful
Check / check (pull_request) Successful in 3m22s
All checks were successful
Check / check (pull_request) Successful in 3m22s
Bump golangci-lint from v2.10.1 to v2.12.2 in the Dockerfile lint stage (tag+digest pin) and script/bootstrap release-archive pins (linux amd64/arm64 sha256s). Replace .golangci.yml with the canonical v2-layout config so linter settings (lll 88, funlen 80/50, cyclop 15, dupl 100) actually apply. Fix all findings surfaced by the new linter and config: - noctx: use httptest.NewRequestWithContext in all tests - gosec G710/G703: route app redirects through a path-escaping redirectToApp helper; annotate internal log path usage - goconst: introduce shared constants for template/JSON keys and repeated test literals - lll: wrap lines to the 88-column limit - dupl: extract shared helpers (generic findAllByAppID in models, deleteAppResource in handlers, parsePush in webhook payloads, table-driven/helper-based test dedup) - nolintlint: drop nolint directives made obsolete by the new limits Record the change in TODO.md; make check is green.
This commit is contained in:
@@ -24,21 +24,30 @@ func newCORSTestMiddleware(corsOrigins string) *Middleware {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCORS_NoOriginsConfigured_NoCORSHeaders(t *testing.T) {
|
||||
t.Parallel()
|
||||
// assertNoCORSHeaders runs a request with the given Origin header through
|
||||
// CORS middleware configured with corsOrigins and asserts that no
|
||||
// Access-Control-Allow-Origin header is set.
|
||||
func assertNoCORSHeaders(t *testing.T, corsOrigins, origin, msg string) {
|
||||
t.Helper()
|
||||
|
||||
m := newCORSTestMiddleware("")
|
||||
m := newCORSTestMiddleware(corsOrigins)
|
||||
handler := m.CORS()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Set("Origin", "https://evil.com")
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
|
||||
req.Header.Set("Origin", origin)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin"),
|
||||
assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin"), msg)
|
||||
}
|
||||
|
||||
func TestCORS_NoOriginsConfigured_NoCORSHeaders(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assertNoCORSHeaders(t, "", "https://evil.com",
|
||||
"expected no CORS headers when no origins configured")
|
||||
}
|
||||
|
||||
@@ -50,7 +59,7 @@ func TestCORS_OriginsConfigured_AllowsMatchingOrigin(t *testing.T) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
|
||||
req.Header.Set("Origin", "https://app.example.com")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
@@ -65,17 +74,6 @@ func TestCORS_OriginsConfigured_AllowsMatchingOrigin(t *testing.T) {
|
||||
func TestCORS_OriginsConfigured_RejectsNonMatchingOrigin(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := newCORSTestMiddleware("https://app.example.com")
|
||||
handler := m.CORS()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Set("Origin", "https://evil.com")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin"),
|
||||
assertNoCORSHeaders(t, "https://app.example.com", "https://evil.com",
|
||||
"expected no CORS headers for non-matching origin")
|
||||
}
|
||||
|
||||
@@ -370,8 +370,9 @@ func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler {
|
||||
}
|
||||
}
|
||||
|
||||
// APISessionAuth returns middleware that requires session authentication for API routes.
|
||||
// Unlike SessionAuth, it returns JSON 401 responses instead of redirecting to /login.
|
||||
// APISessionAuth returns middleware that requires session authentication
|
||||
// for API routes. Unlike SessionAuth, it returns JSON 401 responses instead
|
||||
// of redirecting to /login.
|
||||
func (m *Middleware) APISessionAuth() func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(
|
||||
|
||||
@@ -30,13 +30,15 @@ func TestLoginRateLimitAllowsUpToBurst(t *testing.T) {
|
||||
|
||||
mw := newTestMiddleware(t)
|
||||
|
||||
handler := mw.LoginRateLimit()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
handler := mw.LoginRateLimit()(http.HandlerFunc(
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
},
|
||||
))
|
||||
|
||||
// First 5 requests should succeed (burst)
|
||||
for i := range 5 {
|
||||
req := httptest.NewRequest(http.MethodPost, "/login", nil)
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil)
|
||||
req.RemoteAddr = "192.168.1.1:12345"
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
@@ -44,11 +46,12 @@ func TestLoginRateLimitAllowsUpToBurst(t *testing.T) {
|
||||
}
|
||||
|
||||
// 6th request should be rate limited
|
||||
req := httptest.NewRequest(http.MethodPost, "/login", nil)
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil)
|
||||
req.RemoteAddr = "192.168.1.1:12345"
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
assert.Equal(t, http.StatusTooManyRequests, rec.Code, "6th request should be rate limited")
|
||||
assert.Equal(t, http.StatusTooManyRequests, rec.Code,
|
||||
"6th request should be rate limited")
|
||||
}
|
||||
|
||||
//nolint:paralleltest // mutates global loginLimiter
|
||||
@@ -57,27 +60,29 @@ func TestLoginRateLimitIsolatesIPs(t *testing.T) {
|
||||
|
||||
mw := newTestMiddleware(t)
|
||||
|
||||
handler := mw.LoginRateLimit()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
handler := mw.LoginRateLimit()(http.HandlerFunc(
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
},
|
||||
))
|
||||
|
||||
// Exhaust IP1's budget
|
||||
for range 5 {
|
||||
req := httptest.NewRequest(http.MethodPost, "/login", nil)
|
||||
req.RemoteAddr = "10.0.0.1:1234"
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil)
|
||||
req.RemoteAddr = testProxyAddr
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
}
|
||||
|
||||
// IP1 should be blocked
|
||||
req := httptest.NewRequest(http.MethodPost, "/login", nil)
|
||||
req.RemoteAddr = "10.0.0.1:1234"
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil)
|
||||
req.RemoteAddr = testProxyAddr
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
assert.Equal(t, http.StatusTooManyRequests, rec.Code)
|
||||
|
||||
// IP2 should still work
|
||||
req2 := httptest.NewRequest(http.MethodPost, "/login", nil)
|
||||
req2 := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil)
|
||||
req2.RemoteAddr = "10.0.0.2:1234"
|
||||
rec2 := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec2, req2)
|
||||
@@ -90,25 +95,28 @@ func TestLoginRateLimitReturns429Body(t *testing.T) {
|
||||
|
||||
mw := newTestMiddleware(t)
|
||||
|
||||
handler := mw.LoginRateLimit()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
handler := mw.LoginRateLimit()(http.HandlerFunc(
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
},
|
||||
))
|
||||
|
||||
// Exhaust burst
|
||||
for range 5 {
|
||||
req := httptest.NewRequest(http.MethodPost, "/login", nil)
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil)
|
||||
req.RemoteAddr = "172.16.0.1:5555"
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodPost, "/login", nil)
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil)
|
||||
req.RemoteAddr = "172.16.0.1:5555"
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
assert.Equal(t, http.StatusTooManyRequests, rec.Code)
|
||||
assert.Contains(t, rec.Body.String(), "Too Many Requests")
|
||||
assert.NotEmpty(t, rec.Header().Get("Retry-After"), "should include Retry-After header")
|
||||
assert.NotEmpty(t, rec.Header().Get("Retry-After"),
|
||||
"should include Retry-After header")
|
||||
}
|
||||
|
||||
func TestIPLimiterEvictsStaleEntries(t *testing.T) {
|
||||
|
||||
@@ -7,6 +7,16 @@ import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
// Shared test addresses (also used by ratelimit_test.go).
|
||||
const (
|
||||
testProxyAddr = "10.0.0.1:1234"
|
||||
testRealIP = "203.0.113.5"
|
||||
testXFFIP = "198.51.100.1"
|
||||
testPrivateIP = "192.168.1.1"
|
||||
testPublicIP = "93.184.216.34"
|
||||
testPublicDNSIP = "8.8.8.8"
|
||||
)
|
||||
|
||||
func TestRealIP(t *testing.T) { //nolint:funlen // table-driven test
|
||||
t.Parallel()
|
||||
|
||||
@@ -20,63 +30,63 @@ func TestRealIP(t *testing.T) { //nolint:funlen // table-driven test
|
||||
// === Trusted proxy (RFC1918 / loopback) — headers ARE honoured ===
|
||||
{
|
||||
name: "trusted: X-Real-IP from 10.x",
|
||||
remoteAddr: "10.0.0.1:1234",
|
||||
xRealIP: "203.0.113.5",
|
||||
remoteAddr: testProxyAddr,
|
||||
xRealIP: testRealIP,
|
||||
xff: "198.51.100.1, 10.0.0.1",
|
||||
want: "203.0.113.5",
|
||||
want: testRealIP,
|
||||
},
|
||||
{
|
||||
name: "trusted: XFF from 10.x when no X-Real-IP",
|
||||
remoteAddr: "10.0.0.1:1234",
|
||||
remoteAddr: testProxyAddr,
|
||||
xff: "198.51.100.1, 10.0.0.1",
|
||||
want: "198.51.100.1",
|
||||
want: testXFFIP,
|
||||
},
|
||||
{
|
||||
name: "trusted: XFF single IP from 10.x",
|
||||
remoteAddr: "10.0.0.1:1234",
|
||||
remoteAddr: testProxyAddr,
|
||||
xff: "203.0.113.10",
|
||||
want: "203.0.113.10",
|
||||
},
|
||||
{
|
||||
name: "trusted: falls back to RemoteAddr (192.168.x)",
|
||||
remoteAddr: "192.168.1.1:5678",
|
||||
want: "192.168.1.1",
|
||||
want: testPrivateIP,
|
||||
},
|
||||
{
|
||||
name: "trusted: RemoteAddr without port",
|
||||
remoteAddr: "192.168.1.1",
|
||||
want: "192.168.1.1",
|
||||
remoteAddr: testPrivateIP,
|
||||
want: testPrivateIP,
|
||||
},
|
||||
{
|
||||
name: "trusted: X-Real-IP with whitespace from 10.x",
|
||||
remoteAddr: "10.0.0.1:1234",
|
||||
remoteAddr: testProxyAddr,
|
||||
xRealIP: " 203.0.113.5 ",
|
||||
want: "203.0.113.5",
|
||||
want: testRealIP,
|
||||
},
|
||||
{
|
||||
name: "trusted: XFF with whitespace from 10.x",
|
||||
remoteAddr: "10.0.0.1:1234",
|
||||
remoteAddr: testProxyAddr,
|
||||
xff: " 198.51.100.1 , 10.0.0.1",
|
||||
want: "198.51.100.1",
|
||||
want: testXFFIP,
|
||||
},
|
||||
{
|
||||
name: "trusted: empty X-Real-IP falls through to XFF from 10.x",
|
||||
remoteAddr: "10.0.0.1:1234",
|
||||
remoteAddr: testProxyAddr,
|
||||
xRealIP: " ",
|
||||
xff: "198.51.100.1",
|
||||
want: "198.51.100.1",
|
||||
xff: testXFFIP,
|
||||
want: testXFFIP,
|
||||
},
|
||||
{
|
||||
name: "trusted: loopback honours X-Real-IP",
|
||||
remoteAddr: "127.0.0.1:9999",
|
||||
xRealIP: "93.184.216.34",
|
||||
want: "93.184.216.34",
|
||||
xRealIP: testPublicIP,
|
||||
want: testPublicIP,
|
||||
},
|
||||
{
|
||||
name: "trusted: 172.16.x honours XFF",
|
||||
remoteAddr: "172.16.0.1:4321",
|
||||
xff: "8.8.8.8",
|
||||
want: "8.8.8.8",
|
||||
xff: testPublicDNSIP,
|
||||
want: testPublicDNSIP,
|
||||
},
|
||||
|
||||
// === Untrusted proxy (public IP) — headers IGNORED, use RemoteAddr ===
|
||||
@@ -97,17 +107,17 @@ func TestRealIP(t *testing.T) { //nolint:funlen // table-driven test
|
||||
remoteAddr: "8.8.8.8:443",
|
||||
xRealIP: "1.2.3.4",
|
||||
xff: "5.6.7.8",
|
||||
want: "8.8.8.8",
|
||||
want: testPublicDNSIP,
|
||||
},
|
||||
{
|
||||
name: "untrusted: no headers, public RemoteAddr",
|
||||
remoteAddr: "93.184.216.34:8080",
|
||||
want: "93.184.216.34",
|
||||
want: testPublicIP,
|
||||
},
|
||||
{
|
||||
name: "untrusted: public RemoteAddr without port",
|
||||
remoteAddr: "93.184.216.34",
|
||||
want: "93.184.216.34",
|
||||
remoteAddr: testPublicIP,
|
||||
want: testPublicIP,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -139,7 +149,9 @@ func TestIsTrustedProxy(t *testing.T) {
|
||||
|
||||
trusted := []string{"10.0.0.1", "10.255.255.255", "172.16.0.1", "172.31.255.255",
|
||||
"192.168.0.1", "192.168.255.255", "127.0.0.1", "127.255.255.255", "::1"}
|
||||
untrusted := []string{"8.8.8.8", "203.0.113.1", "172.32.0.1", "11.0.0.1", "2001:db8::1"}
|
||||
untrusted := []string{
|
||||
testPublicDNSIP, "203.0.113.1", "172.32.0.1", "11.0.0.1", "2001:db8::1",
|
||||
}
|
||||
|
||||
for _, addr := range trusted {
|
||||
ip := net.ParseIP(addr)
|
||||
|
||||
Reference in New Issue
Block a user