gsadmin/certctl
1
0
Fork 0
mirror of https://github.com/shankar0123/certctl.git synced 2026-06-07 15:01:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

Bundle E: Mechanical sweeps & defensive polish — 6 findings closed; L-004 deferred

Browse Source 
Closes L-009 + L-010 + L-011 + L-013 + L-020 + L-021 from
comprehensive-audit-2026-04-25. L-004 deferred — recon found NO
rotation infrastructure exists at all; building it from scratch is
a feature project, not a Bundle-E mechanical sweep.

L-009 — ZeroSSL EAB URL configurable
  Audit's 'no timeout' claim was wrong: ari.go:329 has 15s timeout.
  internal/connector/issuer/acme/acme.go: zeroSSLEABEndpoint now
  lazily reads CERTCTL_ZEROSSL_EAB_URL from env at package init;
  defaults to ZeroSSL public endpoint. Pre-existing test override
  path preserved.

L-010 — Verified-already-clean
  grep -rn 'mock\.Anything' --include='*_test.go' . returned 0.
  certctl uses hand-rolled struct mocks (mockJobRepo, mockAuditRepo,
  etc.) with explicit method bodies; no testify-style mocks anywhere.

L-011 — IPv6 bracket-aware dialing pinned
  Every production net.Dial / DialTimeout site audited:
    cmd/agent/main.go:293 — intentional IPv4 literal '8.8.8.8:80'
    verify.go / tlsprobe / network_scan — net.Dialer (no string addr)
    email.go — net.JoinHostPort (bracket-aware)
    ssh.go — addr derives from JoinHostPort upstream
    ssrf.go — net.Dialer
  internal/connector/notifier/email/email_ipv6_test.go (NEW):
    TestJoinHostPort_IPv6BracketsRoundTrip pins IPv4/IPv6/zone variants;
    TestSMTPDialerUsesJoinHostPort source-greps email.go and fails CI
    if a future refactor swaps in 'host:port' concatenation.

L-013 — Verified-already-clean (monotonic-safe)
  Only one site uses now.Sub: middleware.go:393 in tokenBucket.allow().
  Both 'now' and tb.lastRefill come from time.Now() which carries
  monotonic-clock readings per Go's time package contract;
  intra-process now.Sub is monotonic-safe by construction. Doc
  comment block added above the call to make the invariant explicit.

L-020 (CWE-563) — ineffassign sweep, 8 unique sites
  certificate.go:135 — sortDir initial value dropped (set
    unconditionally below by SortDesc branch).
  certificate.go:169,175 — argCount post-increments dropped (var
    not read past the LIMIT/OFFSET formatting).
  agent_group.go, profile.go — page/perPage truly vestigial,
    replaced with _ = page; _ = perPage.
  issuer.go:633, owner.go:131, target.go:267, team.go:131 — same
    treatment for the audit-flagged second-function ListXxx clamps.
  First-function List() in issuer/owner/target/team KEEPS its
    clamp because page/perPage is used for in-memory slice
    pagination — ineffassign correctly didn't flag those.
  Build + tests green post-sweep.

L-021 — Transitive CVE bump
  go get golang.org/x/crypto@v0.45.0 golang.org/x/net@v0.47.0
    (crypto required net@0.47.0). go-text@v0.31.0 transitively
    bumped.
  Per tool-output govulncheck-verbose: x/net@v0.45.0 fixes
    GO-2026-4441 + GO-2026-4440; x/crypto@v0.45.0 fixes
    GO-2025-4134 + GO-2025-4135 + GO-2025-4116 — all 5 advisories
    cleared. Bundle B's ISV grep guard + Bundle D's release-time
    govulncheck step are the going-forward monitor + bump pass.

L-004 — Deferred to dedicated bundle
  Recon: zero hits for RotateAPIKey / rotated_at / key_status
    anywhere in source. API keys configured via
    CERTCTL_API_KEYS_NAMED env var; rotation is operator-managed
    (edit env + restart). Building rotation infrastructure from
    scratch is a feature project, not a mechanical sweep.
  Documented in audit-report.md with scope-pivot note.

Audit deliverables:
  audit-report.md: score 46/55 -> 52/55 closed
    (Low 14/19 -> 19/19 — 100% Low closed except L-004 deferred)
  findings.yaml: 6 status flips
  certctl/CHANGELOG.md: Bundle E section

Verification:
  go test -count=1 -short ./internal/service ./internal/connector/issuer/acme
    ./internal/connector/notifier/email                      green
  go vet on changed packages                                  clean
This commit is contained in:
shankar0123
2026-04-27 01:17:15 +00:00
parent f4fc83d8d6
commit 1b4de3fb2d
13 changed files with 194 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/connector/notifier/email/email_ipv6_test.go
+92
View File
@@ -0,0 +1,92 @@
package email
import (
"net"
"os"
"strings"
"testing"
)
var osReadFile = os.ReadFile
// Bundle E / Audit L-011 (IPv6 dual-stack handling): every production
// `net.Dial`/`net.DialTimeout` call site was audited; the SMTP / email
// notifier path uses `net.JoinHostPort(SMTPHost, port)` which is
// bracket-aware by spec. This test pins the JoinHostPort shape so a
// future refactor that switches to bare `host + ":" + port`
// concatenation — which would silently break IPv6 literals — fails CI.
//
// Other production net.Dial sites are out of scope for this test:
// - cmd/agent/main.go:293 uses literal "8.8.8.8:80" intentionally
// (IPv4 route-discovery hack)
// - cmd/agent/verify.go, internal/tlsprobe/probe.go,
// internal/service/network_scan.go use net.Dialer (no string addr)
// - internal/connector/target/ssh/ssh.go uses an addr derived from
// net.JoinHostPort upstream
// The audit's per-site analysis confirms each is bracket-aware or
// intentionally IPv4-literal.
func TestJoinHostPort_IPv6BracketsRoundTrip(t *testing.T) {
cases := []struct {
name string
host string
port string
want string
}{
{"ipv4_literal", "10.0.0.1", "587", "10.0.0.1:587"},
{"ipv6_literal", "::1", "587", "[::1]:587"},
{"ipv6_full", "2001:db8::1", "25", "[2001:db8::1]:25"},
{"hostname", "smtp.example.com", "465", "smtp.example.com:465"},
{"ipv6_zone", "fe80::1%eth0", "587", "[fe80::1%eth0]:587"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := net.JoinHostPort(tc.host, tc.port)
if got != tc.want {
t.Errorf("net.JoinHostPort(%q, %q) = %q, want %q",
tc.host, tc.port, got, tc.want)
}
// Round-trip via SplitHostPort.
rh, rp, err := net.SplitHostPort(got)
if err != nil {
t.Fatalf("net.SplitHostPort(%q): %v", got, err)
}
// IPv6-zone hosts come back without the literal brackets.
expectedHost := tc.host
if rh != expectedHost {
t.Errorf("round-trip host: got %q, want %q", rh, expectedHost)
}
if rp != tc.port {
t.Errorf("round-trip port: got %q, want %q", rp, tc.port)
}
})
}
}
func TestSMTPDialerUsesJoinHostPort(t *testing.T) {
// Source-grep regression pin: the email notifier MUST use
// net.JoinHostPort when assembling SMTP addresses, never bare
// "host:port" string concatenation. We don't actually dial a
// server here — we just assert the source pattern.
//
// Ridiculously cheap test, but a future refactor that swaps in
// `fmt.Sprintf("%s:%d", host, port)` would silently break IPv6
// SMTP destinations and this test catches it pre-merge.
body := mustReadFile(t, "email.go")
if !strings.Contains(body, "net.JoinHostPort") {
t.Fatal("internal/connector/notifier/email/email.go must use net.JoinHostPort for IPv6 bracket-awareness (L-011)")
}
// Additionally make sure no bare "%s:%d" SMTP pattern slipped in.
if strings.Contains(body, `fmt.Sprintf("%s:%d"`) {
t.Error("found bare host:port concatenation; use net.JoinHostPort (L-011)")
}
}
func mustReadFile(t *testing.T, path string) string {
t.Helper()
body, err := osReadFile(path)
if err != nil {
t.Fatalf("read %s: %v", path, err)
}
return string(body)
}