Update golangci-lint to v2.12.2 with canonical config (#187)
All checks were successful
Check / check (push) Successful in 4s
All checks were successful
Check / check (push) Successful in 4s
Bumps golangci-lint from v2.10.1 to v2.12.2 everywhere it is pinned and installs the canonical `.golangci.yml`, then fixes every finding the new linter surfaces so `make check` is green. ## Version pins - `Dockerfile` lint stage: `golangci/golangci-lint:v2.12.2` (Debian-based), tag plus digest pin - `script/bootstrap`: `GOLANGCI_LINT_VERSION=2.12.2` with updated `linux-amd64`/`linux-arm64` release-archive sha256 pins ## Config `.golangci.yml` replaced with the canonical config. Material change: the old file declared `version: "2"` but kept settings under the legacy top-level `linters-settings` key, which golangci-lint v2 ignores — so the intended thresholds (`lll` 88, `funlen` 80/50, `cyclop` 15, `dupl` 100) were not being applied. The canonical file moves them under `linters.settings` and drops `issues.exclude-use-default`. ## Lint fixes (216 findings) - `lll` (96): wrapped lines to the 88-column limit - `noctx` (46): `httptest.NewRequestWithContext` with `t.Context()` throughout the tests - `goconst` (24): shared constants for template/JSON keys in `internal/handlers` and repeated test literals - `gosec` (23): app-page redirects now go through a `redirectToApp` helper that path-escapes the app ID (G710 open redirect); `http.ServeFile` of the internally derived deployment log path annotated like the adjacent `os.Stat` (G703) - `dupl` (22): extracted a generic `findAllByAppID` in `internal/models`, a `deleteAppResource` helper in `internal/handlers`, a shared `parsePush` in `internal/service/webhook`, and table-driven/helper-based dedup in tests - `nolintlint` (5): removed `//nolint:funlen` directives made obsolete by the new limits (plus one more that became obsolete after refactoring) - `nilerr` (3, surfaced during fixing): resource-delete lookups now propagate the find error to the caller No behavior changes intended; all tests pass and `make check` is green. Note: golangci-lint v2.12 warns that `gomodguard` is deprecated in favor of `gomodguard_v2` — a future canonical-config update should address this centrally. Co-authored-by: sneak <sneak@sneak.berlin> Reviewed-on: #187 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>
This commit was merged in pull request #187.
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