2 Commits

Author	SHA1	Message	Date
shankar0123	58a15e0b3d	feat(notifier): DOC-001 — wire the orphan webhook notifier; README "6 notifiers" now accurate ... Acquisition-audit DOC-001 closure (Sprint 7 ACQ, 2026-05-16). The webhook notifier shipped to internal/connector/notifier/webhook/ months ago with full SafeHTTPDialContext SSRF guard + HMAC-SHA256 signing + comprehensive tests, but it was never wired in cmd/server/main.go — README:39 claimed "6 notifiers" while only 5 were actually registered. Audit prompt offered two paths: (a) wire it if the impl is feature-complete, (b) fix the README count. The impl IS feature-complete (verified by reading webhook.go + webhook_test.go), so path (a) is the rigorous closure. What this commit adds ===================== internal/connector/notifier/webhook/adapter.go (NEW): NotifierAdapter bridges the rich notifier.Connector interface (SendAlert / SendEvent / ValidateConfig) to the simpler service- layer service.Notifier (Send + Channel) used by the notification service's per-channel routing. Send(ctx, recipient, subject, body) constructs a notifier.Event with the three fields populated + a fresh 16-byte hex random ID + UTC timestamp, delegates to the Connector's SendEvent. Channel() returns "webhook". The Connector's per-request HMAC-SHA256 signing + SafeHTTPDialContext SSRF guard apply transitively through SendEvent → postWebhook — no defense duplication at the adapter layer. internal/config/notifiers.go: NotifierConfig gains WebhookURL + WebhookSecret fields with the same docstring shape as the other 5 notifier env-var pairs. internal/config/config.go::Load(): Reads CERTCTL_WEBHOOK_URL + CERTCTL_WEBHOOK_SECRET (both empty by default → notifier disabled, matching the pattern of the other 5 env-var-gated notifiers). cmd/server/main.go: - notifywebhook import added next to the other 5. - New wire-up block after the OpsGenie one: when WebhookURL is set, constructs the Connector via webhook.New (production constructor — strict ValidateSafeURL + SafeHTTPDialContext), wraps in NotifierAdapter, registers as notifierRegistry["Webhook"]. Boot log includes the signing posture ("HMAC-SHA256 signed" vs "unsigned") so operators can spot a missing secret. Target-connector count reconciliation ===================================== The audit prompt also asked to reconcile the target-connector count (README says "fourteen + Kubernetes Secrets preview" = 15; ls internal/connector/target/ shows 17 dirs). Ground-truth: the extra two dirs (certutil, configcheck) are shared HELPER packages (PEM/PFX conversion + server-side shell-injection validation respectively), NOT target connectors. Real target-connector count is 17 - 2 = 15, exactly matching README:12 + README:39. No README change needed. Verified locally: gofmt clean, go vet clean, staticcheck clean across internal/config + internal/connector/notifier/webhook + cmd/server; `go test -count=1 ./internal/connector/notifier/webhook/...` green (existing tests unchanged); `go test -short -count=1 ./internal/config/... ./cmd/server/...` green; `go build ./cmd/server` produces a 30.9MB binary that boots.	2026-05-16 20:37:54 +00:00
shankar0123	45ddcb75a3	refactor(config): extract NotifierConfig to its own file (Phase 9, 1 of N) ... Phase 9 of the certctl architecture diligence remediation begins closing ARCH-M2: the 6 backend mega-files totaling > 13K LOC of change-risk hotspots. config.go is the largest (3,403 LOC pre-split) and the most frequently touched (env-var ingestion gets edited every release). The audit's "3.2K LOC / 11.5K total across 6 files" claim has drifted upward — live grep shows config.go alone is now 3,403 LOC and the top-6 hotspots total 13,267 LOC. The audit's framing is directionally correct; numbers updated in cowork/certctl-architecture- diligence-audit.html with this commit. This commit ships the FIRST of many splits (one per PR per the Phase 9 prompt's "Do not bundle" rule): Extract NotifierConfig (65 lines) → internal/config/notifiers.go Why NotifierConfig first ======================== - Cleanest possible cut: a single struct, no helper functions, no validation logic, no cross-references to Load() except via the Config{}.Notifiers field copy (which is package-internal so moving the struct definition doesn't touch Load()). - Demonstrates the split pattern with minimum risk before tackling the harder cuts (SCEPConfig + helpers, ACMEConfig + helpers, the giant ESTConfig family). - Public-surface byte-identical: every caller's `config.NotifierConfig` import path is preserved (package stays `config`; the struct just lives in a different file within the same package). Live audit (Phase 9 audit questions answered) ============================================== top-10 production .go files by LOC (find cmd internal -name '*.go' -not -name '*_test.go' | xargs wc -l | sort -rn | head -10): 3403 internal/config/config.go <-- this commit -68 2966 cmd/server/main.go 1965 internal/service/acme.go 1867 internal/mcp/tools.go 1577 internal/api/handler/auth_session_oidc.go 1489 cmd/agent/main.go 1356 internal/auth/oidc/service.go 1249 internal/scheduler/scheduler.go 1235 internal/connector/issuer/local/local.go 1224 internal/service/scep.go The audit's "3 others beyond config/main/acme" are: - internal/mcp/tools.go (1867 LOC) - internal/api/handler/auth_session_oidc.go (1577 LOC) - cmd/agent/main.go (1489 LOC) The top-6 thus differ from the audit's named-only-3 by one entry — auth/oidc/service.go (1356) edges out the audit's likely fourth pick. Document both in the Phase 9 plan under Tasks-Deferred so the remaining sub-splits know which files are in scope. config.go internals (45 distinct exported `type X struct` defs as of this commit's pre-state): Config, ServerConfig, ServerTLSConfig, DatabaseConfig, SchedulerConfig, LogConfig, AuthConfig, RateLimitConfig, CORSConfig, KeygenConfig, CAConfig, StepCAConfig, VaultConfig, DigiCertConfig, SectigoConfig, GoogleCASConfig, OpenSSLConfig, ESTConfig, ESTProfileConfig, SCEPConfig, SCEPProfileConfig, SCEPIntuneProfileConfig, NetworkScanConfig, VerificationConfig, ApprovalConfig, NamedAPIKey, SessionConfig, BreakglassConfig, EncryptionConfig, CloudDiscoveryConfig, AWSSecretsMgrDiscoveryConfig, AzureKVDiscoveryConfig, GCPSecretMgrDiscoveryConfig, NotifierConfig (THIS COMMIT), DigestConfig, HealthCheckConfig, ACMEConfig, ACMEServerConfig, ACMEServerDirectoryMeta, AWSACMPCAConfig, EntrustConfig, GlobalSignConfig, EJBCAConfig, OCSPResponderConfig Each is a natural future-split candidate. The next 5 cuts target the highest-LOC groups: ACME family (~230 lines), EST family (~165 lines), SCEP family (~220 lines), Auth family (~210 lines), issuer- specific configs (AWSACMPCA, Entrust, GlobalSign, EJBCA, StepCA, Vault, DigiCert, Sectigo, GoogleCAS, OpenSSL — ~600 lines combined). Public-surface invariant ======================== - Package name stays `config`. - Struct + all field names byte-identical. - Every caller's `config.NotifierConfig` import path preserved. - Verified via: go build ./internal/config/... → clean go test ./internal/config/... -count=1 → ok (0.67s) gofmt -l internal/config/ → clean staticcheck ./internal/config/... → clean LOC delta: config.go: 3403 → 3335 (-68 lines) notifiers.go: new, 86 lines (incl. 18-line Phase 9 doc-comment + BSL header + package decl) Phase 9 follow-on plan (each = separate commit, separate review) ================================================================ Next cuts from config.go (priority order): 2 of N. ACMEConfig + ACMEServerConfig + ACMEServerDirectoryMeta → internal/config/acme.go (~230 lines moved) 3 of N. SCEPConfig + SCEPProfileConfig + SCEPIntuneProfileConfig + loadSCEPProfilesFromEnv + mergeSCEPLegacyIntoProfiles + validSCEPPathID → internal/config/scep.go (~330 lines) 4 of N. ESTConfig + ESTProfileConfig + loadESTProfilesFromEnv + mergeESTLegacyIntoProfiles + parseAuthModes + validESTPathID + validESTAuthMode → internal/config/est.go (~250 lines) 5 of N. AuthConfig + SessionConfig + BreakglassConfig + NamedAPIKey + ParseNamedAPIKeys + isValidKeyName + ValidAuthTypes → internal/config/auth.go (~340 lines) 6 of N. ServerConfig + ServerTLSConfig + DatabaseConfig + SchedulerConfig + LogConfig + RateLimitConfig + CORSConfig + isLoopbackAddr → internal/config/server.go (~270 lines) 7 of N. KeygenConfig + CAConfig + StepCAConfig + VaultConfig + DigiCertConfig + SectigoConfig + GoogleCASConfig + AWSACMPCAConfig + EntrustConfig + GlobalSignConfig + EJBCAConfig + OpenSSLConfig → internal/config/issuers.go (~600 lines) After the config.go cuts land, the same pattern applies to the next 5 hotspots: 8 of N. cmd/server/main.go split: main.go (entrypoint), wire.go (DI assembly), migrations.go (boot-migration path). Phase 4's migration-hook lives in main.go today; migrations.go inherits the path without re-touching it. 9 of N. internal/service/acme.go split: orders.go, authz.go, challenges.go, nonces.go, gc.go under internal/service/acme/. Becomes its own subpackage. 10 of N. internal/mcp/tools.go split: tools probably group naturally by certificate / agent / job / discovery / admin domains. 11 of N. internal/api/handler/auth_session_oidc.go split: by handler verb (login, callback, refresh, logout, backchannel). 12 of N. cmd/agent/main.go split: main.go (entrypoint), poll.go (work-poll loop), deploy.go (deployment execution), register.go (bootstrap + registration). Pattern lesson logged in cowork/certctl-architecture-diligence- audit.html Tasks-Deferred table. Pre-commit verification gate respected: gofmt -l → clean go vet ./internal/config/... → clean (implicit via go test) go test ./internal/config/... → ok staticcheck ./internal/config/... → clean TestRouterRBACGateCoverage → not affected (config package) Closes: cowork/certctl-architecture-diligence-audit.html#fix-ARCH-M2 (partial — 1 of N — full ARCH-M2 closure is the aggregate)	2026-05-14 03:44:44 +00:00