4 Commits

Author	SHA1	Message	Date
shankar0123	8b75e0311b	chore: rename Go module path to github.com/certctl-io/certctl ... Mechanical sed across the main go.mod's module declaration, the f5-mock-icontrol sub-module's go.mod, every Go file's import path (361 files), and a rebuild of the checked-in f5-mock-icontrol binary so its embedded build-info reflects the new module path. No behavior change. Choice B from cowork/transfer-certctl-to-org.md, executed 2026-05-04. Choice A (keep module path declared as github.com/shankar0123/certctl regardless of repo URL) shipped on the day of the org transfer (2026-05-03) since we had no external Go consumers; this commit closes that deferral. Backward-compat: GitHub HTTP redirects continue to forward github.com/shankar0123/certctl → github.com/certctl-io/certctl at the URL level, but Go's module proxy uses the path declared in go.mod as the canonical name. Pre-fix, anyone trying `go get github.com/certctl-io/certctl/...` hit a "module path mismatch" error because go.mod said github.com/shankar0123/certctl and the URL they fetched it from said certctl-io/certctl. Post-fix, the canonical name and the URL agree, so go get / go install / external Go consumers / Go-tooling integrations work cleanly via either the new path (preferred) or the old path (which redirects and Go follows the redirect for source fetch). Anyone still importing the old path inside their own code keeps working provided they update their go.mod's `require` line to match — the module path declared in their consumer's go.sum / go.mod is the authoritative import name, so a mass sed across their import statements is the migration on the consumer side. No external consumers exist today. Diff shape: 361 *.go files — import path replacement only 2 go.mod — module declaration replacement only 1 binary — deploy/test/f5-mock-icontrol/f5-mock-icontrol rebuilt so embedded build-info reflects the new path (8618965 vs 8618933 bytes; 32-byte diff is the build-info change) Total: 364 files, 730 insertions / 730 deletions, net-zero size, pure mechanical substitution. Verification: gofmt: 17 files needed re-alignment after sed (the new path is one char shorter than the old, so column-aligned import groups drifted). Applied `gofmt -w` to fix. go mod tidy: clean exit on both modules. go vet ./...: clean exit. go build ./...: clean exit. go test -short -count=1 on representative packages: all green (internal/domain, internal/validation, internal/crypto, internal/crypto/signer, cmd/agent). Test output now reads `ok github.com/certctl-io/certctl/...` confirming the module path resolves correctly. binary: f5-mock-icontrol rebuilt; `strings | grep shankar0123` returns nothing; `strings | grep certctl-io/certctl` shows the new module path embedded in build-info. Files intentionally NOT touched in this commit: README.md / CHANGELOG.md / docs/ / etc. — already swept to certctl-io URLs in commit 0729ee4 (the post-transfer URL refresh). This commit is purely the Go-tooling layer. Scarf pixels (`shankar0123.docker.scarf.sh/...`) — Scarf-account namespace, not a Go import or GitHub repo URL. Stays. This is a non-blocking, non-customer-impacting change. Operators pulling container images, running `make verify`, hitting the API, or installing the agent see no functional difference. Only Go-tooling consumers (none today) are affected, and they're enabled — not broken — by this commit.	2026-05-04 00:30:29 +00:00
shankar0123	fefa5a5fd7	acme: support serial-only revocation via local cert-version lookup ... Closes the #7 acquisition-readiness blocker from the 2026-05-01 issuer coverage audit. Pre-fix, ACME RevokeCertificate at acme.go:L519-L529 returned the literal error "ACME revocation by serial not supported in V1; provide certificate DER". RFC 8555 §7.6 genuinely requires the cert DER bytes (not just the serial), but a CLM platform's job is to abstract over that limitation. Operators routinely have only the serial in hand: lost PEM, rotated key, GUI revoke action driven by a row in the certs list. This commit: - Adds CertificateLookupRepo interface at the ACME connector boundary (connector boundary, NOT a service/repository import — the connector accepts whatever satisfies the shape). Production wiring in cmd/server/main.go injects the postgres CertificateRepository; tests inject a fake. - Adds CertificateRepository.GetVersionBySerial(ctx, issuerID, serial) + interface declaration in repository/interfaces.go, returning the certificate_versions row whose SerialNumber matches, scoped to the issuer via JOIN on managed_certificates. Mirrors the existing GetByIssuerAndSerial shape but returns the version (where PEMChain lives). Per RFC 5280 §5.2.3 the issuer scope is required for determinism. - Adds SetCertificateLookup + SetIssuerID setters on *acme.Connector. Mirror the pattern local.Connector already uses for OCSP responder wiring. Both must be wired before serial-only revoke works; unwired state falls back to a more actionable error pointing at the wiring requirement (the historical "not supported" wording is retired). - Rewrites RevokeCertificate end-to-end: lookup → empty-PEM check → pem.Decode → block.Type == "CERTIFICATE" check → ensureClient → golang.org/x/crypto/acme.Client.RevokeCert(ctx, accountKey, der, reasonCode). RFC 8555 §7.6 case 1 (revocation request signed with account key) — the same account key issued the cert, so authority is intrinsic. The not-found path returns an actionable operator- facing error pointing at the local-store requirement. - Adds mapRevocationReason translating RFC 5280 §5.3.1 reason strings (unspecified, keyCompromise, cACompromise, affiliationChanged, superseded, cessationOfOperation, certificateHold, removeFromCRL, privilegeWithdrawn, aACompromise) into golang.org/x/crypto/acme. CRLReasonCode. Accepts canonical camelCase + underscore_lower + ALL_CAPS_UNDERSCORE. Nil reason → 0 (unspecified). Unknown reason errors rather than silently demoting (operators rely on the reason for compliance reporting). - Wiring update in service/issuer_registry.go: SetACMECertLookup setter on the registry; Rebuild type-asserts *acme.Connector and calls SetCertificateLookup + SetIssuerID, mirroring the existing *local.Connector branch. cmd/server/main.go calls issuerRegistry.SetACMECertLookup(certificateRepo) immediately after SetIssuanceMetrics — the postgres repo satisfies the interface via GetVersionBySerial. - Tests: * acme_revoke_test.go (new): TestRevokeCertificate_NoCertLookupWired, TestRevokeCertificate_NoIssuerIDWired, TestRevokeCertificate_LookupReturnsNotFound (operator-facing "may not have been issued through certctl" hint pinned), TestRevokeCertificate_LookupArbitraryError, TestRevokeCertificate_VersionPEMEmpty (corrupt-row guard), TestRevokeCertificate_PEMMalformed_NoBlock, TestRevokeCertificate_PEMMalformed_WrongType (PRIVATE KEY block rejected as not a CERTIFICATE). * TestMapRevocationReason_TableDriven: full RFC 5280 reason set plus camelCase / underscore / ALL-CAPS variants plus nil-reason and unknown-reason cases. * acme_failure_test.go: renamed TestRevokeCertificate_AlwaysError → TestRevokeCertificate_UnwiredCertLookupFallback; the test still exercises the same backward-compat branch but now asserts the new "CertificateLookup wiring" error wording. - Mock-repo updates (3 sites): mockCertificateRepository in internal/integration/lifecycle_test.go, mockCertRepo in internal/service/testutil_test.go, mockCertRepoWithGetError in internal/service/shortlived_test.go each gain a GetVersionBySerial implementation that mirrors the GetByIssuerAndSerial logic but returns the version row. - docs/connectors.md ACME section: new "Revocation by serial number" subsection covering the workflow, the local-store requirement (cert was issued through certctl, not imported), the reason-code mapping with the three accepted spelling variants, and a pointer to the audit reference. Out of scope (intentional, per spec): - Recovering the DER from outside the local cert store (CT logs, CSR + signature reconstruction). If the cert wasn't issued through certctl, revoke-by-serial via certctl isn't possible. - Revocation via the cert's private key (RFC 8555 §7.6 case 2). The account-key path covers all certctl-issued certs because the same account key issued them. - Pebble-backed integration test for the happy path. Pebble integration is the right home for that — the unit tests in this commit pin all failure-mode branches before the network call, and the wiring branch in Rebuild is exercised by the existing TestIssuerRegistryRebuild paths. Verified locally: - gofmt -l . clean - go vet ./... clean - staticcheck ./... clean - go test -short -count=1 across connector, service, repository, integration, api/middleware, api/handler: green Audit reference: cowork/issuer-coverage-audit-2026-05-01/RESULTS.md Top-10 fix #7.	2026-05-02 13:09:30 +00:00
shankar0123	c22ce0fcd2	Bundle J follow-up: fix CI staticcheck QF1002 in acme_failure_test.go ... CI on the Bundle J merge (18e46f0) failed at golangci-lint: internal/connector/issuer/acme/acme_failure_test.go:244:3: QF1002: could use tagged switch on r.URL.Path (staticcheck) TestGetRenewalInfo_ARI5xx had a switch{} with case r.URL.Path == ... which staticcheck QF1002 flags as a quick-fix candidate (use tagged switch instead). The function also accumulated dead ts/ts2/ts3 setup from earlier iteration — only ts3 was actually used by the assertion. This commit: - Collapses the 3-server scaffold into a single ts using if/return instead of switch (sidesteps QF1002 entirely + removes ~25 LoC of dead code) - Verifies via 'staticcheck -checks all' (which includes QF*) that the package is clean except for pre-existing ST1000 hits in acme.go/ari.go/dns.go/profile.go (out of scope for this fix) Verification: staticcheck -checks all internal/connector/issuer/acme/... clean (excluding 4 pre-existing ST1000 'missing package comment') go vet ./internal/connector/issuer/acme/... clean go test -short ./internal/connector/issuer/acme/... PASS Coverage unchanged at 55.6% (the test logic was already correct; this commit only removes lint friction).	2026-04-27 16:31:37 +00:00
shankar0123	29d853d641	Bundle J (Coverage Audit Closure): ACME failure-mode test batch — C-001 partial-closed ... internal/connector/issuer/acme line coverage 41.8% -> 55.6% (+13.8pp) via internal/connector/issuer/acme/acme_failure_test.go (~700 LoC, 23 tests). Failure modes pinned (all hermetic via httptest.Server, no live ACME): EAB auto-fetch: network-error, malformed-JSON, 5xx, 401, success=false ARI: dir-unreachable, 5xx, 404 (nil/nil), malformed-JSON, empty-suggestedWindow, dir-malformed-falls-to-fallback, invalid-PEM, happy-path with explanationURL Profile-order: directory-discovery-failure on JWS-POST branch empty-profile fast-path delegation fetchNonce: no-URL, no-Replay-Nonce, network-error, happy-path Always-error V1: RevokeCertificate, GenerateCRL, SignOCSPResponse, GetCACertPEM ensureClient propagation: IssueCertificate / RenewCertificate / GetOrderStatus surface 'ACME client init' wrap Challenge handler (HTTP-01): known-token serves, unknown-token 404 presentPersistRecord: no-solver + DNSSolver-fallback Defense-in-depth: error messages do not leak HMAC key bytes Per-function deltas: GetRenewalInfo 11.4% -> 91.4% getARIEndpoint 0.0% -> 82.4% computeARICertID 50.0% -> 100.0% RenewCertificate 0.0% -> 100.0% RevokeCertificate 0.0% -> 80.0% presentPersistRecord 0.0% -> 80.0% fetchNonce 78.6% -> 92.9% ensureClient 79.3% -> 86.2% fetchZeroSSLEAB 80.8% -> 88.5% Engineering: preWiredConnector fixture pre-sets c.client + c.accountKey so ensureClient short-circuits, letting tests exercise post-init paths (ARI/profile/revoke/getOrderStatus) without a full registration mock. Why partial-closed: residual ~30pp gap to >=85% target lives in IssueCertificate (~115 LoC) + solveAuthorizations[HTTP01|DNS01|DNSPersist01] (~280 LoC) + authorizeOrderWithProfile JWS-POST branch — all require a Pebble-style ACME mock (~300-500 LoC infra + ~500 LoC tests). Tracked as follow-on 'Bundle J-extended'. C-001 status open -> partial_closed. Verification: go vet ./internal/connector/issuer/acme/... clean staticcheck ./internal/connector/issuer/acme/... clean go test -short ./internal/connector/issuer/acme/ PASS, 55.6% coverage go test -race ./internal/connector/issuer/acme/ PASS, 0 races Audit deliverables: findings.yaml: C-001 status open -> partial_closed with closure_note gap-backlog.md: closure log + C-001 row updated coverage-matrix.md: ACME 41.8 -> 55.6 closure-plan.md: Bundle J [~] partial-closed CHANGELOG.md: [unreleased] Bundle J entry with per-function table	2026-04-27 16:26:24 +00:00