mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 15:01:32 +00:00
Shankar
297ff8349e
Final PR in the six-commit M-2 sequence (PR-A: CertificateService clusterad2734c, PR-B: IssuerService+TargetService20b0e75, PR-C: Policy/Profile/ Owner/Teame5a7b45, PR-D: Job/Notification/Auditc2e9ebf, PR-E: AgentService6b2d137, PR-F: this commit). PR-A through PR-E collapsed the service-layer shim methods and deleted every in-production context.Background() / context.TODO() call from internal/service/; this PR completes the sweep across the non-service tiers (HTTP middleware + ACME connector) and wires the contextcheck linter so regressions fail CI. Three narrow edits land the D-3 pattern (context.WithoutCancel for subsidiary async writes and deferred shutdown contexts): - internal/api/middleware/audit.go -- async audit goroutine now runs on auditCtx := context.WithoutCancel(r.Context()) instead of context.Background(). Preserves request-scoped values (trace ID, auth) while detaching from the request's cancellation so the audit write does not get killed when the response completes. Goroutine is still tracked via a.wg (M-1 shutdown drain) so Flush(ctx) behaviour is unchanged. CWE-770 Missing Release (goroutine leak potential) + CWE-400 Resource Exhaustion (missed cancellation propagation). - internal/api/middleware/middleware.go -- Recovery panic path now logs via slog.ErrorContext(ctx, ...) instead of log.Printf. Request- scoped trace/auth metadata now carries through the panic log, matching every other request log. D-3 non-bypass: the context is r.Context() captured before the defer, so even a panic mid-handler propagates the ctx's trace ID into the ERROR log line. - internal/connector/issuer/acme/acme.go (HTTP-01 challenge server shutdown) -- defer shutdown context derived from context.WithTimeout(context.WithoutCancel(ctx), 5s) instead of context.Background(). Preserves parent ctx values, detaches from parent cancellation so Shutdown always gets its full 5-second budget even when the parent was cancelled. Matches the same pattern applied in ACME's solveAuthorizationsDNS01 and solveAuthorizationsDNSPersist01. Linter wiring: .golangci.yml adds `contextcheck` to the enabled set. golangci-lint v2.11.4 now fails CI on any function that takes a context.Context parameter but calls into context.Background() or context.TODO() instead of propagating -- regression guard for all five prior PRs. Verification (CI parity, GOCACHE=/tmp/gocache GOMODCACHE=/tmp/gomodcache GOLANGCI_LINT_CACHE=/tmp/lintcache): - go build ./... -> 0 - go vet ./... -> 0 - golangci-lint run (contextcheck enabled) -> 0 issues - go test -race -short ./internal/api/middleware/... -> PASS - go test -race -short ./internal/scheduler/... -> PASS - go test -race -short ./internal/connector/issuer/acme/... -> PASS - go test -race -short ./internal/service/... -> PASS - rg "context\.(Background|TODO)\(\)" internal/service/ internal/scheduler/ internal/connector/ internal/api/middleware/ -> 0 non-test hits (one pedagogical godoc reference in audit.go documenting why context.Background() would be wrong remains intentional) Wire-format invariants preserved: 0 API routes, 0 SQL migrations, 0 frontend bytes, 0 OpenAPI bytes, 0 connector interface signature changes, 0 new env vars, 0 new external dependencies (pure context stdlib). The AuditRecorder interface signature, the body-hash algorithm (SHA-256 16 hex chars), the excluded-path short-circuit, the actor-extraction path, the responseWriter status-capture wrapper, the AuditServiceAdapter, and all 116 API routes under /api/v1/, /.well-known/est/, /scep, /health, /auth are byte-identical. M-2 aggregate across PR-A through PR-F: 57 files, +635 / -613 (PR-A 12f +227/-237, PR-B 9f +150/-146, PR-C 17f +156/-148, PR-D 11f +67/-63, PR-E 4f +9/-15, PR-F 4f +26/-4). With M-2 closed, 8 of 10 Medium findings resolved; M-9, M-10, L-1..L-4, I-1..I-8 remain post-v2.1.0 hardening batch. Audit complete. Commit:855124a9d9. Sections: 12. Findings: 2/7/10/4/6.
39 lines
1.4 KiB
YAML
39 lines
1.4 KiB
YAML
version: "2"
|
|
|
|
run:
|
|
timeout: 5m
|
|
|
|
linters:
|
|
default: none
|
|
enable:
|
|
- contextcheck
|
|
- govet
|
|
- staticcheck
|
|
- unused
|
|
settings:
|
|
staticcheck:
|
|
checks:
|
|
- "all"
|
|
- "-ST1005" # error strings should not be capitalized (pre-existing style)
|
|
- "-ST1000" # package comment style (pre-existing)
|
|
- "-ST1003" # naming convention (pre-existing)
|
|
- "-ST1016" # method receiver naming (pre-existing)
|
|
- "-QF1001" # apply De Morgan's law (style suggestion)
|
|
- "-QF1003" # convert if/else to switch (style suggestion)
|
|
- "-QF1012" # use fmt.Fprintf (style suggestion)
|
|
- "-SA1019" # deprecated API usage (elliptic.Marshal — Go hasn't removed it)
|
|
- "-SA9003" # empty branch (intentional in switch stubs)
|
|
- "-S1009" # redundant nil check (pre-existing style)
|
|
- "-S1011" # use single append with spread (pre-existing style)
|
|
exclusions:
|
|
max-issues-per-linter: 0
|
|
max-same-issues: 0
|
|
|
|
# Linters temporarily disabled — re-enable incrementally as pre-existing issues are fixed:
|
|
# - errcheck (50 issues — unchecked error returns throughout codebase)
|
|
# - gocritic (50 issues — diagnostic/performance suggestions)
|
|
# - gosec (23 issues — security warnings in test/stub code)
|
|
# - ineffassign (13 issues — dead assignments)
|
|
# - noctx (25 issues — http.Get without context)
|
|
# - bodyclose (response body close missing)
|