From 8aeeec93c03bbbc467c772b28586abd2f6be7d04 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Mon, 11 May 2026 13:31:13 +0000 Subject: [PATCH] chore(lint): close 5 golangci-lint v2 findings surfaced by v2.1.0 release-gate Phase 1.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five golangci-lint v2 findings surfaced when running the v2.1.0 release gate (auth-bundle-2 → master pre-flight). Each is mechanical: 1. govet/printf-style misuse — internal/auth/oidc/service_test.go used integer literal 501 in http.Error; switched to http.StatusNotImplemented. 2. staticcheck SA1019 — internal/auth/breakglass/reflect_helper_test.go referenced reflect.Ptr; the canonical name since Go 1.18 is reflect.Pointer. 3. staticcheck ST1020 — internal/repository/postgres/auth.go ActorRoleRepository.Revoke had a doc comment that did not begin with the method name. Prepended 'Revoke drops actor_roles rows.' to the comment so it now starts with the method name. 4. staticcheck ST1022 — internal/api/handler/auth_session_oidc.go DefaultBCLVerifierMaxAge docstring was attached to the DefaultBCLVerifier type docstring. Moved the const docstring directly above the const declaration, separated by a blank line. 5. unused — internal/auth/session/bench_test.go declared benchSessionMinSamples and never referenced it; the bench loop relies on Go's default b.N scaling. Replaced the const block with a comment describing the rationale. Lint clean (golangci-lint v2.12.2 with the .golangci.yml config) on the five edited packages. --- internal/api/handler/auth_session_oidc.go | 8 ++++---- internal/auth/breakglass/reflect_helper_test.go | 2 +- internal/auth/oidc/service_test.go | 2 +- internal/auth/session/bench_test.go | 12 +++++------- internal/repository/postgres/auth.go | 13 +++++++------ 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/internal/api/handler/auth_session_oidc.go b/internal/api/handler/auth_session_oidc.go index f865279..8b80e2e 100644 --- a/internal/api/handler/auth_session_oidc.go +++ b/internal/api/handler/auth_session_oidc.go @@ -1393,16 +1393,16 @@ func defaultIntIfZero(v, def int) int { // Default BackChannelLogoutVerifier — wraps go-oidc/v3. // ============================================================================= -// DefaultBCLVerifier is the production BackChannelLogoutVerifier. It -// resolves the IdP by issuer (matched against the OIDCProviderRepository), -// fetches the IdP's JWKS via gooidc.Provider, and validates the -// logout_token JWT signature + required claims. // DefaultBCLVerifierMaxAge is the default iat-freshness skew window // (60 seconds; tokens older or newer than this are rejected). Override // per-server via CERTCTL_OIDC_BCL_MAX_AGE_SECONDS. Audit 2026-05-10 // HIGH-3 closure. const DefaultBCLVerifierMaxAge = 60 * time.Second +// DefaultBCLVerifier is the production BackChannelLogoutVerifier. It +// resolves the IdP by issuer (matched against the OIDCProviderRepository), +// fetches the IdP's JWKS via gooidc.Provider, and validates the +// logout_token JWT signature + required claims. type DefaultBCLVerifier struct { providerRepo repository.OIDCProviderRepository tenantID string diff --git a/internal/auth/breakglass/reflect_helper_test.go b/internal/auth/breakglass/reflect_helper_test.go index 7b5a56c..70dc0f9 100644 --- a/internal/auth/breakglass/reflect_helper_test.go +++ b/internal/auth/breakglass/reflect_helper_test.go @@ -11,7 +11,7 @@ import ( // wire-leak the Argon2id hash. Test-only. func reflectJSONTag(v interface{}, fieldName string) string { rv := reflect.ValueOf(v) - if rv.Kind() == reflect.Ptr { + if rv.Kind() == reflect.Pointer { rv = rv.Elem() } if rv.Kind() != reflect.Struct { diff --git a/internal/auth/oidc/service_test.go b/internal/auth/oidc/service_test.go index 15f1131..fcd0f82 100644 --- a/internal/auth/oidc/service_test.go +++ b/internal/auth/oidc/service_test.go @@ -290,7 +290,7 @@ func newMockIdPWithTB(t testing.TB) *mockIdP { mux.HandleFunc("/authorize", func(w http.ResponseWriter, r *http.Request) { // Tests call HandleCallback directly; this endpoint exists for // completeness but the test never round-trips through it. - http.Error(w, "test fixture: not implemented", 501) + http.Error(w, "test fixture: not implemented", http.StatusNotImplemented) }) idp.server = httptest.NewServer(mux) diff --git a/internal/auth/session/bench_test.go b/internal/auth/session/bench_test.go index 112eab4..9e59399 100644 --- a/internal/auth/session/bench_test.go +++ b/internal/auth/session/bench_test.go @@ -47,13 +47,11 @@ import ( // The full Phase 14 result table lives at docs/operator/auth-benchmarks.md. // ============================================================================= -// benchSessionConfig caps b.N to keep the benchmark tractable; for -// p99 we want at least ~1000 samples but not so many that the -// benchmark takes >10s on a CI runner. Go's default benchmark scaling -// already handles this. -const ( - benchSessionMinSamples = 1000 -) +// Bench config: Go's default benchmark scaling caps b.N to keep the +// benchmark tractable. For p99 we want at least ~1000 samples but not +// so many that the benchmark takes >10s on a CI runner. We let the +// runtime handle it rather than enforcing a const that lint can't +// trace through to a use site. // setupBenchSession boots a session.Service with a warm in-memory // repo + a single active signing key, mints one session row, and diff --git a/internal/repository/postgres/auth.go b/internal/repository/postgres/auth.go index 699611a..70ab1eb 100644 --- a/internal/repository/postgres/auth.go +++ b/internal/repository/postgres/auth.go @@ -406,12 +406,13 @@ func (r *ActorRoleRepository) Grant(ctx context.Context, ar *authdomain.ActorRol return nil } -// Audit 2026-05-11 A-4 — scope-aware revoke. The pre-fix SQL omitted -// (scope_type, scope_id) from the WHERE clause; combined with HIGH-10's -// UNIQUE (actor_id, actor_type, role_id, scope_type, scope_id, tenant_id) -// uniqueness extension, an operator who granted the same role to the -// same actor at two different scopes had no selective-revoke path — -// every Revoke call nuked both rows. The new behaviour: +// Revoke drops actor_roles rows. Audit 2026-05-11 A-4 — scope-aware +// revoke. The pre-fix SQL omitted (scope_type, scope_id) from the +// WHERE clause; combined with HIGH-10's UNIQUE (actor_id, actor_type, +// role_id, scope_type, scope_id, tenant_id) uniqueness extension, an +// operator who granted the same role to the same actor at two +// different scopes had no selective-revoke path — every Revoke call +// nuked both rows. The new behaviour: // // - opts.ScopeType == "" (legacy call shape): drop the scope from the // WHERE clause; delete every variant. Zero-row delete is NOT an