mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 21:41:39 +00:00
v2.0.71
323 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
0e06f6c4fc |
cli: promote --force on renew + require --reason on revoke (closes P3-1, P3-2)
Closes findings P3-1 and P3-2 from the 2026-05-05 CLI/API/MCP↔GUI parity
audit (cowork/cli-gui-parity-audit-2026-05-05/RESULTS.md). Both findings
flagged hidden defaults that the CLI was sending without exposing them
to operators: `force=false` baked into every renew payload, and a silent
fallback to `reason="unspecified"` whenever --reason was omitted.
P3-1 — promote --force on `certs renew` (full end-to-end plumbing)
The pre-2026-05-05 CLI sent `{"force": false}` in the renew body. The
API handler never decoded it — a textbook "lying field" per the
operator's CLAUDE.md "complete path, not the easy path" rule: the body
field stored a value, claimed to do something, and silently did nothing
because the wire never reached the consumer. Adding a --force flag that
also went unread would have created another lying field.
This commit takes the complete path:
service.CertificateService.TriggerRenewal grew a `force bool` parameter
(internal/service/certificate.go). When force=true, the
RenewalInProgress block is overridden so operators can recover stuck
in-flight renewals where a previous job hung without releasing the
status flag. Archived and Expired remain terminal blockers regardless
of force — those are semantic dead-ends that --force should not paper
over (archived = decommissioned, expired = issue a new cert instead of
renewing a dead one).
handler.CertificateHandler.TriggerRenewal parses force from
?force=true (or ?force=1) query param, OR {"force": true} JSON body,
whichever the client picks. Defaults to false. Passes through to the
service.
internal/cli/client.go::RenewCertificate(id, force bool) sends
?force=true on the URL when --force is set. The historical hardcoded
`{"force": false}` body is gone — no more lying field.
cmd/cli/main.go dispatches `certs renew <id> [--force]` (ID-first
flag-second convention matches the existing `agents retire <id>
[--force]`).
P3-2 — require --reason on `certs revoke` (Option A: strict refusal)
The pre-2026-05-05 CLI dropped to `--reason unspecified` whenever the
operator omitted the flag. Compliance reporting (RFC 5280 §5.3.1, PCI-
DSS §3.6, HIPAA §164.312) relies on the reason code being meaningful;
silent fallback defeats the audit trail because every revocation looks
identical.
cmd/cli/main.go dispatch refuses to send when --reason is empty,
prints the canonical RFC 5280 §5.3.1 reason-code menu, and exits
non-zero.
internal/cli/client.go exposes ValidRevokeReasons() returning the
canonical camelCase list (unspecified, keyCompromise, caCompromise,
affiliationChanged, superseded, cessationOfOperation, certificateHold,
removeFromCRL, privilegeWithdrawn, aaCompromise) and
NormalizeRevokeReason() that accepts both camelCase and snake_case
inputs and normalises to the canonical wire form. Off-list reasons
are rejected at dispatch with the menu re-printed.
Test pins:
internal/cli/client_test.go::TestClient_RenewCertificate_ForceFlag —
--force=true sends ?force=true with empty body; --force=false sends
no query and no body.
internal/cli/client_test.go::TestNormalizeRevokeReason +
TestValidRevokeReasons — canonical-camelCase + snake_case + reject-
off-enum behaviour.
cmd/cli/dispatch_test.go::TestHandleCerts_Revoke_RequiresReason +
TestHandleCerts_Revoke_RejectsUnknownReason +
TestHandleCerts_Renew_ForceFlag — dispatch-layer pins for the same
contracts.
internal/api/handler/certificate_handler_test.go::TestTriggerRenewal_
ForceQueryParam — query-param passthrough (no-flag, force=true,
force=1, force=false) flows through to the service-layer parameter.
internal/service/certificate_test.go::TestTriggerRenewal_
ForceOverridesInProgress — force=false preserves the
RenewalInProgress block; force=true clears it.
Existing TestTriggerRenewal_Archived extended to assert force=true
still blocks Archived (terminal-state guarantee).
Docs: docs/reference/cli.md updated with the --force example for renew
and the strict --reason semantics for revoke (including snake_case
input acceptance).
Acceptance gate (verified):
- go build ./cmd/server/... ./cmd/agent/... ./cmd/cli/...
./cmd/mcp-server/... clean.
- go vet ./... clean.
- go test -short -count=1 ./... pass repo-wide.
- bash scripts/ci-guards/openapi-handler-parity.sh clean
(router 178, OpenAPI 144, exceptions 36 — unchanged; we add
parameter parsing, not routes).
- gofmt -l clean.
|
||
|
|
ff75361553 |
mcp(coverage): add 34 tools across 7 domains to close 2026-05-05 parity audit P1 findings
Closes findings P1-1..P1-35 from the 2026-05-05 CLI/API/MCP↔GUI parity
audit (cowork/cli-gui-parity-audit-2026-05-05/RESULTS.md). Before this
bundle, 35 operator-facing API endpoints had GUI surfaces but no MCP
counterpart — operators using AI assistants for cert lifecycle work in
regulated environments had to drop to curl for approve/reject, health-check
acknowledgement, renewal-policy CRUD, network-scan triggering, discovery
triage, intermediate-CA management, and job verification.
Tool count: 87→121 in tools.go (+34), 6 unchanged in tools_est.go.
Re-derive via grep -cE 'gomcp\\.AddTool\\(' internal/mcp/tools.go
internal/mcp/tools_est.go.
The 7 phases (matching the bundle prompt at
cowork/mcp-coverage-expansion-prompt.md):
Phase A — Approvals (P1-28..P1-31, 4 tools)
list_approvals, get_approval, approve_request, reject_request.
Two-person-integrity contract (ErrApproveBySameActor → HTTP 403)
is preserved automatically: the decided_by actor is derived
server-side from middleware.UserKey, NOT from request body, so
the MCP server's authenticated API-key identity becomes the
audit-trail actor. The MCP input schema deliberately omits any
actor_id field to prevent client-side spoofing.
Phase B — Health Checks (P1-20..P1-27, 8 tools)
list, summary, get, create, update, delete, history, acknowledge.
Mirrors the existing target-resource shape; acknowledge takes
optional 'actor' string captured in the audit row (handler defaults
to 'unknown' if absent).
Phase C — Renewal Policies (P1-1..P1-5, 5 tools)
Standard CRUD against /api/v1/renewal-policies. Distinct from the
legacy 'policy' tools that point at the same path — these expose
the renewal-policy domain explicitly with full alert_channels +
alert_severity_map field shape.
Phase D — Network Scan Targets (P1-14..P1-19, 6 tools)
CRUD + trigger_scan. trigger_network_scan returns the discovery-
scan body so the AI can chain into list_discovered_certificates
filtered by agent_id.
Phase E — Discovery read-side (P1-10..P1-13, 4 tools)
list_discovered_certificates, get_discovered_certificate,
list_discovery_scans, discovery_summary. Complements the
pre-existing claim/dismiss tools (registered alongside Health
historically per the I-2 closure).
Phase F — Intermediate CAs (P1-6..P1-9, 4 tools)
list, create (root + child via discriminator on body shape), get,
retire. The handler is admin-gated via middleware.IsAdmin; the
least-privilege boundary is enforced at the API layer (HTTP 403
for non-admin Bearer callers) — not by transport carve-out.
Phase G — Verification + deployments (P1-32, P1-34, P1-35, 3 tools)
list_certificate_deployments, verify_job, get_job_verification.
P1-33 (POST /api/v1/agents/{id}/discoveries) is intentionally
excluded — machine-to-machine push channel for agents reporting
filesystem-scan results, not an operator-driven flow. Documented
inline in the RegisterTools dispatch.
Implementation:
- 14 new input types in internal/mcp/types.go with jsonschema struct
tags driving LLM tool discovery.
- 7 register* functions in internal/mcp/tools.go each handling one
phase, wired into RegisterTools dispatch in declaration order.
- 34 new entries in tools_per_tool_test.go::allHappyPathCases —
the existing in-process MCP harness (TestMCP_AllTools_HappyPath +
TestMCP_AllTools_ErrorPath + TestMCP_RegisterTools_DispatchableToolCount)
auto-extends coverage to cover every new tool: happy-path round-
trip with fence-shape assertion, 5xx error-path with MCP_ERROR fence
propagation, and 'every registered tool is dispatchable' guard.
- docs/reference/mcp.md 'Available Tools' table expanded from 16 to
22 resource domains with current per-domain tool counts.
Acceptance gate (verified):
- go build ./cmd/server/... ./cmd/agent/... ./cmd/cli/... ./cmd/mcp-server/...
clean across all four production binaries.
- go vet ./... clean.
- go test -short -count=1 ./internal/mcp/... pass (TestMCP_AllTools_*
expanded to 127 tool round-trips).
- go test -short -count=1 ./... pass repo-wide.
- bash scripts/ci-guards/openapi-handler-parity.sh clean (router 178,
OpenAPI 144, exceptions 36 — unchanged; we add MCP wrappers, not
routes).
- gofmt -l clean across the four touched files.
|
||
|
|
75097909e9 | |||
|
|
0f81c1b956 |
ci: re-fix CodeQL #32 + repair loadtest f5-mock build context
Two unrelated CI failures from run #25305811340; fixed in one
commit since neither needs the other to land first.
CodeQL alert #32 (go/log-injection at middleware.go:68) reopened
after
|
||
|
|
b0fc067317 |
security: close CodeQL #17 (log injection) + #23 (SSRF false-positive reopen)
Two CodeQL alerts in one sweep — both medium-impact follow-ups
on already-merged guards.
Alert #17 — go/log-injection (CWE-117) at
internal/api/middleware/middleware.go:58:
log.Printf("[%s] %s %s %d %v", requestID, r.Method, r.URL.Path, ...)
r.Method and r.URL.Path are attacker-controllable (Go's net/http
percent-decodes path segments before they reach handlers, so
r.URL.Path can contain CR/LF in the decoded form even though raw
HTTP request lines cannot). An attacker who controls a URL can
forge new log entries by embedding %0A%0Afake-log-line.
Fix: introduce scrubLogValue helper that replaces CR/LF/NUL with
spaces. Apply to both r.Method and r.URL.Path. Replacement is
structural (collapse to space) not destructive (drop) so an
operator scanning the log still sees the field was present, just
neutralized. Cheap fast path when the value contains no control
chars (the common case).
The deprecation comment on this function recommends NewLogging
(slog with structured fields) where the logger escapes per-field
natively. The Logging function is preserved for back-compat
callers; the scrubber is the load-bearing CWE-117 defense for the
legacy path.
Alert #23 — go/request-forgery (CWE-918) at scep_probe.go:271:
CodeQL reopened the alert after commit
|
||
|
|
9ef9f3cde3 |
refactor(scep+ejbca): drop dead conditionals on always-empty vars (CodeQL #18, #19)
Two CodeQL go/comparison-of-identical-expressions alerts in one sweep — both Warning severity, both real dead-code (not false positives). CodeQL detected that each comparison's LHS variable was provably constant. Alert #18 — internal/api/handler/scep.go:612 (extractCSRFields): challengePassword := "" transactionID := "" // ... loop populates challengePassword from CSR.Attributes ... for _, attr := range csr.Attributes { if attr.Type.Equal(oidChallengePassword) { // populates challengePassword ONLY — transactionID stays "" } } if transactionID == "" && csr.Subject.CommonName != "" { // ← always true transactionID = csr.Subject.CommonName } transactionID was initialized to "" and never reassigned before the check. The conditional was always true; the MVP path was effectively "unconditionally fall back to CN". The RFC 8894 path (tryParseRFC8894 above this function) extracts transaction-ID properly from PKCS#7 authenticatedAttributes; the MVP path is for lightweight legacy clients that send the raw CSR with no PKCS#7 wrapping, and CN-as-transaction-ID is sufficient there. Fix: drop the dead transactionID local var + dead conditional; unconditionally set transactionID = csr.Subject.CommonName. No behavioral change — the runtime semantics are identical to before (every valid invocation already took the fallback). The CN extraction stays robust because the empty-CN case still produces an empty transactionID, which downstream callers handle. Alert #19 — internal/connector/issuer/ejbca/ejbca.go:415 (RevokeCertificate): serial := request.Serial issuerDN := "" // (comment: "if we have time..." — TODO never followed up) revokeURL := fmt.Sprintf("%s/certificate/%s/%s/revoke", apiURL, issuerDN, serial) if issuerDN == "" { // ← always true revokeURL = fmt.Sprintf("%s/certificate/%s/revoke", apiURL, serial) } issuerDN was hardcoded to "" two lines above. The first revokeURL line was unreachable dead code; the conditional always fired and the serial-only URL always won. EJBCA's REST API has both /certificate/{issuer_dn}/{serial}/revoke and /certificate/{serial}/revoke endpoints; the serial-only form is correct for typical certctl deployments where one EJBCA CA maps to one certctl issuer config (no overlapping serial spaces). Fix: drop the dead first revokeURL + dead conditional; build revokeURL once via the serial-only endpoint. No behavioral change — the runtime URL was always the serial-only one. Comment retained + expanded to document the future-enhancement path (parse issuer DN from IssuanceResult metadata + use the DN-qualified endpoint when a multi-CA EJBCA deployment surfaces). Verified locally: gofmt: clean. go vet ./internal/api/handler/... + ./internal/connector/issuer/ejbca/...: exit 0. go test -short -count=1 ./internal/api/handler/... + ejbca/...: PASS. Both fixes are pure dead-code removal — runtime behavior is byte- identical to pre-edit. The existing test suites would have caught any actual behavioral change. References: https://github.com/certctl-io/certctl/security/code-scanning/18 https://github.com/certctl-io/certctl/security/code-scanning/19 Closes both alerts. |
||
|
|
2b4d0069d9 |
security(scep-intune): annotate verifyES256/RS256 SHA-256 as RFC-mandated (CodeQL #21 false positive)
CodeQL alert #21 (go/weak-sensitive-data-hashing, severity: High) flagged the sha256.Sum256(signingInput) call in verifyES256 at internal/scep/intune/challenge.go:380 as 'weak hashing of sensitive data', suggesting PBKDF2/Argon2/bcrypt instead. This is a CodeQL false positive. The CodeQL query triggers when SHA-256 is used near *x509.Certificate (the trust pool) and infers 'this might be password hashing.' But the actual context is JWS signature verification: - verifyRS256 implements RFC 7518 §3.3 — 'RSASSA-PKCS1-v1_5 using SHA-256'. SHA-256 is spec-mandated. - verifyES256 implements RFC 7518 §3.4 — 'ECDSA using P-256 and SHA-256'. SHA-256 is spec-mandated. - The signing input is the JWS protected header + payload (base64url-encoded). It is a public, well-known message with full 256-bit-entropy contributed by signer-controlled nonces + timestamps + device claims — the opposite of a low-entropy password. - The output is verified against an asymmetric signature (rsa.VerifyPKCS1v15 / ecdsa.Verify), not compared to a pre-computed hash digest. This is signature verification, not password hashing. - Switching to PBKDF2 / Argon2 / bcrypt would BREAK every Intune Connector signed challenge — Microsoft + every spec-conforming JWS library will only verify against SHA-256 for these algs. Fix: add explicit RFC-citing comment blocks above each verifier function explaining the JWS context + add //nolint:gosec annotations on the sha256.Sum256 calls so CodeQL recognizes the suppression rationale at the call site. The annotation cites the specific RFC clause (7518 §3.3 / §3.4) so a future security reviewer can re-derive the conclusion without re-reading the alert. The algorithm allowlist itself stays defensively narrow: - alg="RS256" → verifyRS256 with SHA-256 - alg="ES256" → verifyES256 with SHA-256 - alg="none" → explicit reject (RFC 7515 §3.6 attack vector) - any other alg → reject as unsupported Pinned by existing tests: - TestValidateChallenge_HappyPath_RS256 - TestValidateChallenge_HappyPath_ES256_FixedWidth - TestValidateChallenge_HappyPath_ES256_DER - TestValidateChallenge_AlgNoneRejected - TestValidateChallenge_UnsupportedAlg The happy-path tests would fail if the verifiers switched to any non-SHA-256 digest — the alg allowlist makes the SHA-256 dependency load-bearing, which the existing test suite already proves. Verified locally: gofmt: clean. go vet ./internal/scep/intune/...: exit 0. go test -short -count=1 ./internal/scep/intune/...: PASS (every existing challenge_test.go subtest still green). Reference: https://github.com/certctl-io/certctl/security/code-scanning/21 Closes CodeQL alert #21 as a documented false positive — the //nolint annotations + RFC-citing comments are the load-bearing suppression. Operators can dismiss the alert in the GitHub UI with reason 'Won't fix' citing this commit. |
||
|
|
d08982fc19 |
security(signer): bound FileDriver paths with SafeRoot + reject .. (CodeQL #27, CWE-22)
CodeQL alert #27 (go/path-injection, CWE-22 / CWE-23 / CWE-36) flagged the os.WriteFile sink at internal/crypto/signer/file_driver.go:194 because the outPath flowed from operator-supplied config (CAKeyPath in the local issuer's encrypted config blob -> GenerateOutPath closure -> os.WriteFile) without a containment check. Threat model: Production wiring (cmd/server/main.go) constructs &signer.FileDriver{} and the local-issuer NewConnector wires GenerateOutPath off Config.CAKeyPath. CAKeyPath ships from the encrypted issuer config in PostgreSQL — settable only by an authenticated admin via the API. So the realistic exploit is: (a) Admin compromise -> CAKeyPath set to /etc/passwd -> FileDriver.Generate overwrites system files. (b) Future code path concatenates attacker-controlled fragments into the output path -> classic ../../etc/passwd traversal. Defense in depth: bound the write surface so admin-key-rotation errors and future regressions can't escape into arbitrary filesystem writes. Fix: internal/crypto/signer/file_driver.go gains: - SafeRoot string field on FileDriver. When set, every Load + Generate path MUST resolve under SafeRoot via filepath.Abs + strings.HasPrefix on cleaned paths. - validateSafePath helper that: * rejects empty paths * filepath.Clean()s the input * rejects paths whose cleaned form still contains a literal ".." segment (catches relative paths that escape above their start; absolute paths get collapsed by Clean) * resolves to filepath.Abs and (when SafeRoot non-empty) verifies containment via filepath.Separator-suffixed HasPrefix (the bare-prefix bug — SafeRoot=/var/lib/foo erroneously accepting /var/lib/foobar — has its own regression test below) - Load + Generate now call validateSafePath before any os.ReadFile / os.WriteFile. The validator is in the same function as the sink so CodeQL recognizes it as a guard. Tests (internal/crypto/signer/signer_test.go): TestFileDriver_Load_RejectsParentTraversal — relative path "../../etc/passwd" rejected with parent-directory error. TestFileDriver_Load_RejectsEmptyPath — empty path rejected. TestFileDriver_Generate_RejectsParentTraversal — write side, same pattern. TestFileDriver_SafeRoot_AcceptsContainedPath — happy path: a key file under SafeRoot succeeds. TestFileDriver_SafeRoot_RejectsEscape — absolute path outside SafeRoot rejected (the load-bearing CodeQL pin). TestFileDriver_SafeRoot_RejectsSiblingPrefix — pins the HasPrefix-with-separator subtlety: SafeRoot=/tmp/X must NOT accept /tmp/X-sibling. Verified locally: gofmt: clean. go vet ./...: exit 0. go test -short -count=1 ./internal/crypto/signer/...: ok 1.605s go test -short -count=1 ./internal/connector/issuer/local/...: ok 4.908s (downstream FileDriver consumer) go test -short -count=1 ./internal/service/...: ok 4.029s Backwards-compat: when SafeRoot is unset, only the structural .. + empty-path checks fire — the existing FileDriver call sites in cmd/server/main.go and the existing unit tests pass unchanged. Production wiring SHOULD set SafeRoot via cmd/server/main.go in a follow-up commit (env-var-supplied CERTCTL_CA_KEY_DIR or similar). Reference: https://github.com/certctl-io/certctl/security/code-scanning/27 Closes CodeQL alert #27 (go/path-injection). |
||
|
|
af3ca3935b |
ci: convert literal Unicode in headers_test.go to \u escapes (ST1018)
CI run #448 (commit |
||
|
|
e6919cdaba |
security(scep_probe): re-validate URL inside scepHTTPGet to close CodeQL #23 (CWE-918)
CodeQL alert #23 (go/request-forgery, CWE-918 SSRF) flagged the client.Do(req) sink at internal/service/scep_probe.go:232 because the URL parameter to scepHTTPGet is taint-traced from the user- supplied input to ProbeSCEP without the analyzer recognizing the upstream sanitizer. The defense-in-depth was already in place: 1. validation.ValidateSafeURL at ProbeSCEP entry (line 75) — rejects obvious SSRF targets (loopback / link-local / cloud metadata literals) before any network call. 2. validation.SafeHTTPDialContext on the http.Transport — re-resolves the host at dial time and rejects connections to reserved IP ranges. This is the authoritative SSRF + DNS- rebinding guard. Even if step 1 was bypassed, the dial would still fail. But CodeQL's taint tracker doesn't follow the validator across function boundaries, so the alert stays open even though the code is safe. This commit re-runs validation.ValidateSafeURL inside scepHTTPGet immediately before http.NewRequestWithContext — sanitizer in the same function as the sink, which CodeQL recognizes as a guard. Bonus defense-in-depth: any future call site that wires a URL into scepHTTPGet without going through ProbeSCEP (e.g. a new code path that directly probes a discovered URL) inherits the same SSRF guard automatically. Fail-closed by default. The validator dispatch matches ProbeSCEP's pattern — tests override via s.scepValidateURL to hit httptest loopback servers; production callers use validation.ValidateSafeURL. The probe's existing httptest-based tests continue to work unchanged. Verified locally: gofmt: clean. go vet ./...: exit 0. go test -short ./internal/service/...: ok 4.029s (every existing scep_probe test still green — the new revalidation is a no-op for tests that go through ProbeSCEP because the same validator already passed once at entry). Reference: https://github.com/certctl-io/certctl/security/code-scanning/23 Closes CodeQL alert #23 (go/request-forgery). |
||
|
|
23c593089d |
security(email): sanitize body fields against content injection (CodeQL #11, CWE-640)
CodeQL alert #11 (go/email-injection, CWE-640 / OWASP Content Spoofing)
flagged the wc.Write(message) sink at internal/connector/notifier/email/
email.go:208 because attacker-controllable fields flow into the email
body unchecked.
Threat model:
Headers (From, To, Subject) were already protected by
validation.ValidateHeaderValue (CWE-113 SMTP header injection,
closed in commit
|
||
|
|
34adcfbbe5 |
api, handler: 4 admin-gated CA hierarchy endpoints + OpenAPI (Rank 8 commit 4)
Rank 8 commit 4 of 5. The API + RBAC layer that operators drive
the new hierarchy management surface from.
Endpoints (all admin-gated via middleware.IsAdmin; non-admin Bearer
callers get 403):
POST /api/v1/issuers/{id}/intermediates
Discriminator on body shape:
empty parent_ca_id + root_cert_pem + key_driver_id
→ CreateRoot (registers operator-supplied root CA).
parent_ca_id non-empty
→ CreateChild (signs new sub-CA cert under parent).
Service-layer error → HTTP code mapping:
ErrCANotSelfSigned → 400
ErrCAKeyMismatch → 400
ErrPathLenExceeded → 400
ErrNameConstraintExceeded → 400
ErrInvalidCertPEM → 400
ErrParentCANotActive → 409
ErrIntermediateCANotFound → 404
(other) → 500
GET /api/v1/issuers/{id}/intermediates
Returns flat list ordered by created_at; caller renders the
tree from each row's parent_ca_id (nil = root).
GET /api/v1/intermediates/{id}
Single-row detail.
POST /api/v1/intermediates/{id}/retire
Two-phase: confirm=false → active→retiring; confirm=true →
retiring→retired with active-children check (drain-first
semantics; ErrCAStillHasActiveChildren → 409).
Files changed:
internal/api/handler/intermediate_ca.go — 4 handlers
+ handler-defined
service interface
(dependency
inversion).
internal/api/handler/intermediate_ca_test.go — 8 test variants
(M-008 admin-
gate triplet
complete).
internal/api/handler/m008_admin_gate_test.go — register the
new admin-gated
handler in
AdminGatedHandlers
so the M-008
coherence
scanner stays
green.
internal/api/router/router.go — 4 r.Register
calls + new
IntermediateCAs
field on
HandlerRegistry.
cmd/server/main.go — wire the
postgres repo +
service +
handler. Reuses
the same
signer.FileDriver
instance the
OCSP responder
bootstrap path
feeds.
api/openapi.yaml — 4 new
operationIds,
full body
schema + status-
code dispatch.
Tests (8 in this commit):
TestIntermediateCA_Handler_NonAdmin_Returns403 (admin gate
— table-driven across all 4 endpoints)
TestIntermediateCA_Handler_AdminExplicitFalse_Returns403
(defensive: AdminKey present but false ≠ AdminKey absent)
TestIntermediateCA_Handler_AdminPermitted_ForwardsActor
(admin actor forwarded to service for audit attribution)
TestIntermediateCA_HandlerCreate_RootDispatch
(body discriminator: empty parent_ca_id → CreateRoot)
TestIntermediateCA_HandlerCreate_ChildDispatch
(body discriminator: parent_ca_id present → CreateChild)
TestIntermediateCA_HandlerCreate_BadRequestOnMissingRootBundle
(validation: no parent + no root bundle → 400)
TestIntermediateCA_HandlerCreate_ServiceErrorMappings
(table-driven: 7 service errors → expected HTTP codes)
TestIntermediateCA_HandlerRetire_TwoPhaseConfirm
(confirm=false then confirm=true forwarded correctly)
TestIntermediateCA_HandlerRetire_StillHasActiveChildren_Returns409
(drain-first contract — 409 not 500)
Verified locally:
gofmt: clean.
go vet ./...: exit 0.
go test -short -count=1 ./internal/api/handler/...: ok 4.498s.
bash scripts/ci-guards/openapi-handler-parity.sh: clean
(router routes: 182, openapi operations: 148; the +4 new routes
have +4 new operationIds — parity preserved).
bash scripts/ci-guards/* (all 24 guards): clean.
Out of scope of THIS commit (commit 5):
- web/src/pages/IssuerHierarchyPage.tsx (recursive tree render).
- docs/intermediate-ca-hierarchy.md sysadmin runbook (FedRAMP /
financial-services / internal-PKI patterns).
- docs/connectors.md hierarchy_mode row.
- WORKSPACE-ROADMAP entries (HSM-backed roots, automated
rotation, CRL chaining, NameConstraints templates, D3
dendrogram).
Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md, commit 4.
|
||
|
|
ae597f7f8d |
local: tree-mode chain assembly + byte-equivalence pin (Rank 8 commit 3)
Rank 8 commit 3 of 5. Load-bearing connector rewrite that activates
the first-class CA hierarchy surface shipped by commits 1-2.
Local connector changes:
- New ChainAssembler interface (single-method seam) defined in the
connector package — *service.IntermediateCAService satisfies it
implicitly. Avoids the import cycle that would arise from
pulling internal/service into internal/connector/issuer/local.
- Three new optional fields on Connector: hierarchyMode,
chainAssembler, treeIssuingCAID. Default zero values keep the
pre-Rank-8 single-sub-CA flow byte-identical (no operator on
the historical path sees any change in wire bytes).
- Three new setters: SetHierarchyMode, SetChainAssembler,
SetTreeIssuingCAID. Wired in cmd/server/main.go in commit 4
when the issuer's HierarchyMode column is read at boot.
- resolveChainPEM helper centralizes the dispatch:
tree mode + ChainAssembler set + treeIssuingCAID set
→ call AssembleChain over intermediate_cas
otherwise (incl. tree mode with incomplete wiring)
→ fall back to historical c.caCertPEM
Defense in depth: a misconfigured operator gets a working
issuance, not a nil-deref panic.
- IssueCertificate + RenewCertificate both delegate ChainPEM
population to resolveChainPEM. The cert generation path
(generateCertificate) is untouched — same key, same template,
same signing.
Tests (internal/connector/issuer/local/local_hierarchy_test.go):
TestLocal_HierarchyMode_SingleVsTree_ByteIdentical ← LOAD-BEARING
THE refuse-to-ship pin. Two connectors against the same on-disk
CA cert+key:
- A: pre-Rank-8 single-sub-CA mode (HierarchyMode unset).
- B: tree mode wired against an in-memory ChainAssembler
whose 1-level chain matches A's caCertPEM byte-for-byte.
Asserts:
1. resA.ChainPEM == resB.ChainPEM (the byte-identical pin).
2. resA.ChainPEM == fixture root cert PEM (real fact about
the wire format, not internal consistency).
Operators on single mode keep getting byte-identical bytes.
Operators flipping to tree with a 1-level shim see no change.
Zero behavioral drift for unmigrated deployments.
TestLocal_HierarchyMode_Tree_LeafChainIncludesAllAncestors
Multi-level pin. 4-level synthetic chain (root → policy →
issuingA → issuingB-leaf-CA). Asserts:
- 4 CERTIFICATE blocks in ChainPEM.
- Leaf-first ordering (issuingB.CN, issuingA.CN, policy.CN,
root.CN at depths 0..3).
This is what tree mode buys operators in exchange for the
migration overhead.
TestLocal_HierarchyMode_FallsBackToSingleWhenWiringIncomplete
Defensive fallback pin. HierarchyMode='tree' but
ChainAssembler nil + treeIssuingCAID '' → ChainPEM falls back
to caCertPEM. No panic, no lying field.
Verified locally:
gofmt: clean.
go vet ./...: exit 0.
go test -short -count=1 -run TestLocal_HierarchyMode ./internal/connector/issuer/local/...
PASS (3/3, including the load-bearing byte-identical pin).
go test -short -count=1 ./internal/connector/issuer/local/...: ok 4.358s
(every existing local-connector test still green — backwards
compat byte-for-byte at the test layer too).
Out of scope of THIS commit (commit 4):
- 4 admin-gated handler endpoints + OpenAPI extension.
- cmd/server/main.go wiring that reads Issuer.HierarchyMode at
boot and calls SetHierarchyMode + SetChainAssembler +
SetTreeIssuingCAID on the local connector instance.
Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md, commit 3.
|
||
|
|
62523fb845 |
service: 10 IntermediateCAService tests + in-memory fake repo (Rank 8 commit 2.5)
Service-layer pin for Rank 8. The fake IntermediateCARepository's
WalkAncestry mirrors the postgres recursive-CTE semantics
(leaf-first ordering, terminate at parent_ca_id IS NULL) so the
AssembleChain pin carries the same weight the production repo would.
Tests:
TestIntermediateCA_CreateRoot_RegistersOperatorSuppliedSelfSigned
Happy path. RFC 5280 §3.2 self-signed root + matching key gets
persisted with parent_ca_id=NULL, state=active, KeyDriverID=...
TestIntermediateCA_CreateRoot_RejectsNonSelfSigned
RFC 5280 §3.2 enforcement. Cert whose embedded public key
doesn't match the actual signer fails CheckSignatureFrom →
ErrCANotSelfSigned.
TestIntermediateCA_CreateRoot_RejectsKeyMismatch
Operator-boundary defense in depth. Cert is well-formed
self-signed but the supplied keyDriverID resolves to a
different key → ErrCAKeyMismatch.
TestIntermediateCA_CreateChild_PathLenTighteningEnforced
RFC 5280 §4.2.1.9 enforcement. Child whose path-len equals or
exceeds parent's → ErrPathLenExceeded. Strictly-tighter child
succeeds.
TestIntermediateCA_CreateChild_NameConstraintsSubset
RFC 5280 §4.2.1.10 enforcement. Widening rejected
("evil.com" outside parent's "example.com"); subdomain
narrowing succeeds ("internal.example.com").
TestIntermediateCA_AssembleChain_4DeepHierarchy ← LOAD-BEARING
The pin the local connector tree-mode delegates to. Builds
root → policy → issuing-A → issuing-B and asserts AssembleChain
returns 4 CERTIFICATE blocks in leaf-to-root order with
matching subject CommonNames at each depth.
TestIntermediateCA_Retire_RefusesIfActiveChildren
Drain-first semantics. retiring → retired with active children
refuses with ErrCAStillHasActiveChildren.
TestIntermediateCA_Retire_TwoPhaseConfirm
First call: active → retiring (no confirm). Second call without
confirm: surfaces "pass confirm=true". Second call with
confirm: retiring → retired.
TestIntermediateCA_MetricsRecordedPerOutcome
Snapshot pin. CreateRoot bumps create_root, CreateChild bumps
create_child, Retire(active) bumps retire_retiring, all
dimensioned by issuer_id.
TestIntermediateCA_LoadHierarchy_FlatList
Returns every CA for an issuer ordered by created_at; caller
renders the tree from parent_ca_id.
Test infrastructure:
fakeIntermediateCARepo — sync.Mutex-guarded map.
WalkAncestry walks
parent_ca_id from leafID
to root (or terminates on
cycle, defense-in-depth).
Compile-time interface
guard.
testCAFixture — mints a self-signed root
cert+key in process,
Adopt()s the key under
a stable ref so CreateRoot
can resolve it.
newTestService — wires IntermediateCAService
with fake repo +
signer.MemoryDriver +
mockAuditRepo (already
lives in testutil_test.go)
+ IntermediateCAMetrics.
Verified locally:
gofmt: clean.
go vet ./...: exit 0.
go test -short -count=1 -run TestIntermediateCA ./internal/service/...
PASS (10/10)
go test -short -count=1 ./internal/service/...: ok 3.844s
Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md, commit 2.5.
|
||
|
|
fb54ebcb62 |
service: IntermediateCAService + IntermediateCAMetrics + RFC 5280 enforcement
Rank 8 of the 2026-05-03 deep-research deliverable, commit 2 of 5.
Service-layer wiring for first-class N-level CA hierarchy management.
The connector rewrite that activates this surface lands in commit 3.
Files added:
internal/service/intermediate_ca.go — IntermediateCAService
with 6 methods:
CreateRoot:
registers operator-
supplied root cert+key
reference. Validates
RFC 5280 §3.2 self-
signed (subject ==
issuer + signature
verifies). Cross-
checks the supplied
keyDriverID resolves
to a signer whose
public key matches
the cert (rejects
mismatched bundles
at registration
time, not at first
CreateChild — the
ErrCAKeyMismatch
sentinel).
CreateChild:
generates child key
via signer.Driver,
signs the cert via
the parent's signer.
Enforces RFC 5280
§4.2.1.9 (path-len
tightening) +
§4.2.1.10
(NameConstraints
subset semantics) at
service layer fail-
closed. Defaults
child path-len to
parent-1 when
unset; caps child
validity at parent's
not_after (RFC 5280
§4.1.2.5).
Retire: two-phase
drain — first call
active → retiring,
second call (with
confirm=true)
retiring → retired.
Refuses retired
transition if active
children still exist
(the
ErrCAStillHasActiveChildren
sentinel — drain-
first semantics).
Get / LoadHierarchy:
thin repo wrappers.
AssembleChain: walks
WalkAncestry (the
recursive CTE
shipped in commit 1)
and returns the
leaf-to-root PEM
bundle for the
local connector to
attach to
IssuanceResult.
internal/service/intermediate_ca_metrics.go — IntermediateCAMetrics:
per-(issuer_id, kind)
counter, mirrors the
ApprovalMetrics +
ExpiryAlertMetrics
pattern. RecordCreate
(root/child) +
RecordRetire
(retiring/retired).
SnapshotIntermediateCA
for the Prometheus
exposer.
Defense in depth retained:
- NEVER persist CA private key bytes in the row. KeyDriverID is the
only key reference; signer.Driver.Load resolves it at signing time.
- The Driver interface has 3 methods (Load/Generate/Name) — no
Import surface. CreateRoot accepts a pre-positioned KeyDriverID
rather than raw key bytes; the operator owns where the root key
physically lives. Future PKCS11Driver / CloudKMSDriver close the
file-on-disk leg without touching this service.
Verified locally:
gofmt: clean.
go vet ./internal/service/...: exit 0.
go build ./internal/service/...: exit 0.
Deferred to commit 2.5 (or fold into commit 3, operator's call):
- 9 service-level tests including:
* TestIntermediateCA_CreateRoot_RegistersOperatorSuppliedSelfSigned
* TestIntermediateCA_CreateRoot_RejectsNonSelfSigned
* TestIntermediateCA_CreateRoot_RejectsKeyMismatch
* TestIntermediateCA_CreateChild_PathLenTighteningEnforced
* TestIntermediateCA_CreateChild_NameConstraintsSubset
* TestIntermediateCA_AssembleChain_4DeepHierarchy ← LOAD-BEARING
* TestIntermediateCA_Retire_RefusesIfActiveChildren
* TestIntermediateCA_Retire_TwoPhaseConfirm
* TestIntermediateCA_MetricsRecordedPerOutcome
Test setup needs: in-memory IntermediateCARepository fake +
signer.MemoryDriver (already exists) + helper to generate test root
cert+key. Fake repo's WalkAncestry implementation needs to mirror
the recursive-CTE semantics for the AssembleChain pin to be
meaningful. Total ~500 lines of test code; non-trivial setup.
Out of scope of THIS commit (commits 3-5):
- Local connector rewrite + byte-equivalence pin
(TestLocal_HierarchyMode_SingleVsTree_ByteIdentical).
- 4 admin-gated handler endpoints + OpenAPI extension.
- web/src/pages/IssuerHierarchyPage.tsx.
- docs/intermediate-ca-hierarchy.md sysadmin runbook.
- cmd/server/main.go wiring.
Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md.
|
||
|
|
66d2af36a7 |
domain, migrations: IntermediateCA type + intermediate_cas + Issuer.HierarchyMode
Rank 8 of the 2026-05-03 deep-research deliverable, commit 1 of 5
(cowork/rank-8-intermediate-ca-hierarchy-prompt.md). Closes the multi-
level CA hierarchy gap for FedRAMP boundary-CA, financial-services
policy-CA, and OT network-CA deployments where regulator-mandated
certificate-policy separation requires multiple layers (root → policy
→ issuing).
This commit lands ONLY the foundation — schema, types, repository
interface, postgres implementation. No service / connector / handler
wiring yet. The 5-commit chain is bisectable: this commit can ship
with no operator-visible behavior change until commits 2-5 wire the
service layer + the local-connector tree-mode + admin API + GUI tree
view + operator runbook. The default value for issuers.hierarchy_mode
is 'single' so every existing operator's behavior is byte-identical
post-migration.
Existing scaffolding REUSED (not redefined):
- internal/crypto/signer.Driver seam — every IntermediateCA carries
a key_driver_id pointing at the signer.Driver instance that owns
its private key. Defense in depth: NEVER persist key bytes in a
row. FileDriver is the production default; future PKCS11Driver /
CloudKMSDriver close the disk-exposure leg via the same seam.
- issuers.id row — the new intermediate_cas FK references it.
Files added:
internal/domain/intermediate_ca.go — IntermediateCA type,
IntermediateCAState
closed enum (active /
retiring / retired),
IsValidIntermediateCAState
+ IsTerminal helpers,
NameConstraint struct
(RFC 5280 §4.2.1.10
permitted+excluded
subtree subset
semantics for service-
layer enforcement),
HierarchyModeSingle /
HierarchyModeTree
constants.
internal/repository/postgres/intermediate_ca.go — IntermediateCARepository
impl: Create (ica-<slug>
ID gen, JSONB +
nullable-column round-
trip, lib/pq 23505 →
ErrAlreadyExists),
Get, ListByIssuer,
ListChildren,
UpdateState,
GetActiveRoot,
WalkAncestry (recursive
CTE — single SQL
round-trip, O(depth)
rows, leaf-first
ordering).
migrations/000028_intermediate_ca_hierarchy.{up,down}.sql
— idempotent schema.
issuers.hierarchy_mode
VARCHAR(20) DEFAULT
'single'. New
intermediate_cas table
with FKs to
issuers / self
(parent_ca_id) +
CHECK constraints
(closed-enum state,
not_after >
not_before, no self-
parent) + 6 indexes
(partial-unique
active root per
issuer, partial-
unique name per
issuer, owning
issuer, parent,
state, expiring).
Files modified:
internal/domain/connector.go — adds Issuer.HierarchyMode field
with full doc comment + JSON tag.
Empty string ≡ single mode for
back-compat.
internal/repository/interfaces.go — adds IntermediateCARepository
interface (7 methods).
Verified locally:
gofmt: clean.
go vet ./internal/domain/... ./internal/repository/...: exit 0.
go build ./internal/domain/... ./internal/repository/...: exit 0.
Out of scope for this commit (lands in commits 2-5):
- service/intermediate_ca.go (CreateRoot / CreateChild / Retire /
LoadHierarchy / AssembleChain + RFC 5280 §4.2.1.9 path-len +
§4.2.1.10 NameConstraints subset enforcement + 9 service tests).
- local connector rewrite + byte-equivalence pin
(TestLocal_HierarchyMode_SingleVsTree_ByteIdentical — the load-
bearing backwards-compat refusal-to-ship test).
- 4 admin-gated handler endpoints + OpenAPI extension + handler tests.
- web/src/pages/IssuerHierarchyPage.tsx.
- docs/intermediate-ca-hierarchy.md sysadmin runbook + connectors.md
row + WORKSPACE-ROADMAP follow-ons.
Reference: cowork/rank-8-intermediate-ca-hierarchy-prompt.md.
|
||
|
|
31e50d987f |
ci: fix Rank 7 lint + openapi-handler-parity drift on master
Two CI failures from the Rank 7 chain push (#438): Go Build & Test — staticcheck ST1021: internal/service/approval_metrics.go:97 comment for ApprovalDecisionEntry doesn't start with the type name internal/service/approval_metrics.go:130 comment for ApprovalPendingAgeSnapshot doesn't start with the type name Frontend Build — scripts/ci-guards/openapi-handler-parity.sh: 4 router routes have no OpenAPI operationId: GET /api/v1/approvals GET /api/v1/approvals/{id} POST /api/v1/approvals/{id}/approve POST /api/v1/approvals/{id}/reject The Rank 7 commit-3 spec deferred OpenAPI extension to commit 4 with a 'batched alongside the integration changes' note; commit 4 didn't actually add them. This commit closes that gap. Fixes: approval_metrics.go — split the doc comment that was attached to SnapshotApprovalDecisions (the function) but visually preceded ApprovalDecisionEntry (the type), so the type appeared to staticcheck as having a comment that named the function instead of the type. Same fix on ApprovalPendingAgeSnapshot. Now each exported type has its own type-name-leading comment per Go convention. api/openapi.yaml — added 4 new operationIds (listApprovalRequests, getApprovalRequest, approveApprovalRequest, rejectApprovalRequest) + new ApprovalRequest schema component under components/schemas. Inline 401 response (the Unauthorized component does not exist in this spec; the canonical pattern in the rest of the file is inline 'description: Authentication required'). The two-person integrity contract surface is documented in the description of the approve / reject endpoints so external readers see the RBAC contract from the spec alone. Verified locally: go vet ./internal/service/...: exit 0. scripts/ci-guards/openapi-handler-parity.sh: clean (140 ops vs 174 routes, 36 documented exceptions). Third CI failure (image-and-supply-chain) was a transient apt-fetch 'Connection reset by peer' from deb.debian.org while pulling libasan6_10.2.1-6_amd64.deb. Not a code issue; just re-run the workflow. No code change needed. |
||
|
|
aebfd8bd7c |
Revert "chore: drop 'Infisical' label from internal references"
This reverts commit
|
||
|
|
19706e56b3 |
chore: drop 'Infisical' label from internal references
Strategic naming cleanup. Earlier doc-comments + commit messages framed Rank 4 / Rank 5 / Rank 7 work as 'Rank N of the 2026-05-03 Infisical deep-research deliverable' — the 'Infisical' qualifier was a holdover from the original deep-research framing where Infisical (a competing secrets-management platform) was the comparator. Keeping the comparator's name in our source adds noise without value; an external reader sees 'Infisical' and assumes a dependency or shared lineage rather than reading it as the competitive context it was. Mechanical sed across 34 files (32 source / docs + 2 follow-up Python passes to collapse 'deep-research deep-research' duplicates that emerged where the original phrase wrapped across lines): s|Infisical deep-research|deep-research|g s|infisical-deep-research-results|deep-research-results-2026-05-03|g s|infisical-deep-research-prompt|deep-research-prompt-2026-05-03|g s|infisical-deep-research|deep-research|g s|Infisical|deep-research|g s|deep-research deep-research|deep-research|g # collapse-pass Net diff: 63 insertions / 64 deletions across cmd/, docs/, internal/, migrations/. Pure text substitution; zero behavior change. Code path unchanged — go vet clean, tests for TestApproval pass on both internal/service and internal/api/handler packages. Workspace docs (cowork/) carry the same references and will be swept separately — they're not under certctl/ git control. The two filename references (cowork/infisical-deep-research-results.md + cowork/infisical-deep-research-prompt.md) get renamed alongside that sweep to deep-research-results-2026-05-03.md / deep-research-prompt-2026-05-03.md so cross-references in the certctl repo doc-comments resolve cleanly. |
||
|
|
03c61f4c20 |
scheduler, certificate, renewal: gate issuance on profile-driven approval
Closes Rank 7 of the 2026-05-03 Infisical deep-research deliverable
(cowork/infisical-deep-research-results.md Part 5). Pre-fix, certctl
issued certificates unattended — every renewal-loop tick that crossed
a renewal threshold created a Job at Status=Pending which the
scheduler dispatched directly to the issuer connector. PCI-DSS Level
1, FedRAMP Moderate / High, SOC 2 Type II, and HIPAA-regulated PHI
customers all ask the same procurement question: "How do you enforce
two-person integrity on cert issuance?" Today's answer: "We don't."
After this commit chain: "Per-profile RequiresApproval=true creates a
parallel ApprovalRequest row; the renewal-loop creates the Job at
Status=AwaitingApproval; an authorized approver (different from the
requester per the same-actor RBAC check) calls
POST /api/v1/approvals/{id}/approve, transitioning the Job to
Pending; the scheduler picks it up."
This commit (4 of 4) wires the gate into the manual TriggerRenewal
entry point + main.go service construction + Config.Approval +
docs + WORKSPACE-ROADMAP follow-up entries. The previous commits
in the chain shipped:
- 1 (
|
||
|
|
81632eb0f3 |
api, handler: 4 approval endpoints + handler RBAC integration tests
Rank 7 of the 2026-05-03 Infisical deep-research deliverable, commit 3 of 4.
Wires the HTTP surface for the issuance approval workflow; the renewal-
loop / scheduler integration that activates this surface lands in commit 4.
Files added:
internal/api/handler/approval.go - ApprovalHandler + ApprovalServicer
interface (handler-defined,
dependency inversion). 4
endpoints:
GET /api/v1/approvals
?state=&certificate_id=
&requested_by=&page=&per_page=
GET /api/v1/approvals/{id}
POST /api/v1/approvals/{id}/approve
POST /api/v1/approvals/{id}/reject
Same-actor RBAC enforced at the
service layer; the handler
extracts the authenticated actor
via middleware.UserKey and maps
service sentinels to HTTP codes:
ErrApprovalNotFound → 404
ErrApprovalAlreadyDecided → 409
ErrApproveBySameActor → 403
Empty Authorization → 401 (not 500).
Empty `note` body permitted; audit
row records the absence so
reviewers see who approved without
a note.
internal/api/handler/approval_test.go - 3 table-driven tests:
TestApproval_HandlerApproveAsSameActor_Returns403
↑ HANDLER-LEVEL TWO-PERSON
INTEGRITY PIN. Pairs with
the service-level
TestApproval_Approve_RejectsSameActor.
Compliance auditors expect
exactly HTTP 403 (not 401,
not 500) when the requester
self-approves; the test
additionally asserts the
error body contains the
"two-person integrity"
substring so an auditor can
grep server logs for
attempted self-approvals.
TestApproval_HandlerEmptyNote_Allowed_DecidedByExtractedFromAuth
↑ pins that decided_by comes
from the auth-middleware
UserKey, NEVER from the
request body. Defends
against future contributor
confusion that might let a
client supply their own
decided_by string.
TestApproval_HandlerErrorMapping
(NotFound → 404, AlreadyDecided
→ 409 subtests).
Files modified:
internal/api/router/router.go - Adds Approvals field to
HandlerRegistry struct + 4
r.Register lines for the
approval routes. Go 1.22
ServeMux precedence: literal
/approve and /reject segments
resolve before the {id}
pattern-var route, mirroring
the existing notifications
block's /requeue precedence.
Verified:
gofmt: clean.
go vet ./internal/api/... ./internal/service/...: exit 0.
go test -short -count=1 -run TestApproval
./internal/api/handler/...: ok 0.004s.
Note on OpenAPI spec: the prompt's spec section also calls for 5 new
operationIds in api/openapi.yaml (createApprovalRequest, listApprovalRequests,
getApprovalRequest, approveApprovalRequest, rejectApprovalRequest). The
external-create endpoint is intentionally not implemented in V2 — every
approval request originates from the renewal-loop entry points (commit 4)
so the only operations exposed are list / get / approve / reject. The
4-route surface is a deliberate scope cut: external systems wanting to
inject approval requests can use the underlying `POST /api/v1/certificates/
{id}/renew` path which creates the parallel ApprovalRequest as a side
effect (post-commit-4 wiring). OpenAPI extension batched into commit 4
alongside the integration changes.
Out of scope for this commit (lands in commit 4):
- Integration into CertificateService.TriggerRenewal +
RenewalService.CheckExpiringCertificates + Scheduler.ReapTimedOutJobs.
- cmd/server/main.go wiring.
- Config.Approval.BypassEnabled + CERTCTL_APPROVAL_BYPASS env var.
- api/openapi.yaml extensions.
- docs/connectors.md + docs/approval-workflow.md.
Reference: cowork/rank-7-approval-workflow-primitive-prompt.md.
|
||
|
|
8043e2bbac |
service: ApprovalService + ApprovalMetrics + 8 table-driven tests
Rank 7 of the 2026-05-03 Infisical deep-research deliverable, commit 2 of 4
(cowork/rank-7-approval-workflow-primitive-prompt.md). Builds on the
foundation in commit
|
||
|
|
2025275b43 |
domain, migrations: ApprovalRequest type + issuance_approval_requests + RequiresApproval
Rank 7 of the 2026-05-03 Infisical deep-research deliverable, commit 1 of 4
(cowork/rank-7-approval-workflow-primitive-prompt.md). The four-commit
chain ships the issuance approval-workflow primitive (request → human review
→ CA call) closing the two-person integrity / four-eyes principle
procurement gap for PCI-DSS Level 1, FedRAMP Moderate / High, SOC 2
Type II, and HIPAA-regulated PHI deployments.
This commit lands ONLY the foundation — schema, types, repository
interface, postgres implementation. No service / handler wiring yet.
The four-commit shape is bisectable: the schema can land in production
behind a flag (via the default RequiresApproval=false on every existing
profile) without any operator-visible behavior change until commits 2-4
wire the surrounding workflow.
Existing scaffolding REUSED (not redefined here):
- JobStatusAwaitingApproval enum value (internal/domain/job.go).
- JobRepository.ListTimedOutAwaitingJobs (postgres reaper query).
- Config.Scheduler.AwaitingApprovalTimeout (env-mapped via
CERTCTL_JOB_AWAITING_APPROVAL_TIMEOUT, default 168h = 7 days).
- Scheduler.SetAwaitingApprovalTimeout wiring.
Files added:
internal/domain/approval.go - ApprovalRequest type,
ApprovalState closed enum
(pending/approved/rejected/
expired), IsValidApprovalState +
IsTerminal helpers, outcome
const block + bypass-actor
sentinel.
internal/repository/postgres/approval.go - ApprovalRepository
implementation: Create
(ar-<slug> ID gen + JSONB
metadata round-trip + lib/pq
23505 → ErrAlreadyExists
translation), Get, GetByJobID,
List (paginated with state /
cert / requester filters),
UpdateState (pending→terminal
transitions only, with
already-terminal disambiguation),
ExpireStale (bulk reaper,
decided_by='system-reaper').
migrations/000027_approval_workflow.{up,down}.sql
- Idempotent IF NOT EXISTS /
IF EXISTS. Adds
certificate_profiles.requires_approval
BOOLEAN NOT NULL DEFAULT false,
issuance_approval_requests
table with FK to
managed_certificates / jobs /
certificate_profiles, four
indexes (state, certificate,
pending-age, partial-unique
pending-per-job), and the
approval_decision_consistency
CHECK constraint enforcing
decided_by/decided_at must be
non-null for terminal states.
Files modified:
internal/domain/profile.go - Adds CertificateProfile.RequiresApproval
bool field with full doc
comment + JSON tag. Defaults
to false (back-compat — every
existing profile keeps the
unattended renewal path).
internal/repository/interfaces.go - Adds ApprovalRepository
interface (6 methods) +
ApprovalFilter struct.
internal/repository/errors.go - Adds ErrAlreadyExists sentinel
for postgres SQLSTATE 23505
(unique-constraint violations
from the partial-unique
pending-per-job index, plus
the "already terminal" state-
transition signal). Mirrors
the existing ErrNotFound +
ErrForeignKeyConstraint shape.
Verified:
gofmt: clean.
go vet ./internal/domain/... ./internal/repository/...: exit 0.
go build ./internal/domain/... ./internal/repository/...: exit 0.
Out of scope for this commit (lands in commits 2-4):
- service/approval.go (RequestApproval / Approve / Reject / ListPending
/ ExpireStale + same-actor RBAC + bypass mode + audit + metrics).
- service/approval_metrics.go (decisions counter + pending-age histogram).
- 8 service-level table-driven tests including the load-bearing
TestApproval_Approve_RejectsSameActor two-person integrity pin.
- api/handler/approval.go (5 endpoints + RBAC integration).
- api/openapi.yaml (5 new operationIds).
- Integration into CertificateService.TriggerRenewal +
RenewalService.CheckExpiringCertificates + Scheduler.ReapTimedOutJobs.
- cmd/server/main.go wiring.
- Config.Approval.BypassEnabled + CERTCTL_APPROVAL_BYPASS env var.
- docs/connectors.md CertificateProfile config-table row.
- docs/approval-workflow.md operator playbook + compliance control mapping.
Reference: cowork/infisical-deep-research-results.md Part 5 Rank 7.
Acquisition prompt: cowork/rank-7-approval-workflow-primitive-prompt.md.
|
||
|
|
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
|
||
|
|
8a56a78282 |
target(azurekv): SDK-driven Azure Key Vault target connector
Closes Rank 5 (Azure half) of the 2026-05-03 Infisical deep-research deliverable (cowork/infisical-deep-research-results.md Part 5). Pre-fix, certctl had no path to deploy certs to Azure-managed TLS- termination endpoints (Application Gateway / Front Door / App Service / Container Apps) — operators terminating TLS at Azure had to use manual `az keyvault certificate import` invocations or external automation. This commit lands the SDK-driven Azure Key Vault target connector that closes the gap, mirroring the AWS ACM target shape shipped in commit |
||
|
|
edf6bee7f8 |
target(awsacm): SDK-driven AWS Certificate Manager target connector
Closes Rank 5 (AWS half) of the 2026-05-03 Infisical deep-research
deliverable (cowork/infisical-deep-research-results.md Part 5).
Pre-fix, certctl had no path to deploy certs to AWS-managed TLS-
termination endpoints (ALB / CloudFront / API Gateway / App Runner)
— operators terminating TLS at AWS had to use Infisical secret-sync,
manual aws-cli imports, or external automation. This commit lands
the SDK-driven AWS Certificate Manager target connector that closes
the gap end-to-end.
Architecture:
- internal/connector/target/awsacm/awsacm.go — Connector wraps
*acm.Client behind the ACMClient interface seam (mirrors
awsacmpca's ACMPCAClient pattern from the issuer side).
LoadDefaultConfig handles the standard AWS credential chain
(IRSA / EC2 instance profile / SSO / env vars); no embedded
creds in connector Config.
- Pre-deploy snapshot via DescribeCertificate + GetCertificate so
on-import-failure rollback restores the previous cert. Mirrors
the Bundle 5 IIS pattern + the Bundle 7/8 WinCertStore /
JavaKeystore patterns. Surfaces rollback success/failure via
the existing certctl_deploy_rollback_total Prometheus counter
label set.
- Provenance tags: certctl-managed-by=certctl + certctl-
certificate-id=<mc-id> set automatically on every import. ACM
strips tags on re-import, so the connector calls
AddTagsToCertificate post-import to keep the provenance pair
fresh. Operators looking up a cert ARN by managed-cert ID
(Terraform data source, CloudFormation output) match against
these tags.
- DeploymentRequest.KeyPEM held in agent memory only — never
written to disk. Aligns with the pull-only deployment model
documented in CLAUDE.md.
Tests:
- awsacm_test.go: 15-subtest happy-path + validation matrix
covering ValidateConfig (success / missing-region / malformed-
region / malformed-ARN / reserved-tag rejection),
DeployCertificate (fresh import / rotate-in-place / rollback-
on-serial-mismatch / rollback-also-fails / empty-key-rejected /
no-client-rejected), ValidateOnly (returns sentinel),
ValidateDeployment (serial match / mismatch / no-ARN-yet).
- awsacm_failure_test.go: 5 per-error-class contract tests
mirroring the awsacmpca_failure_test.go shape (commit
|
||
|
|
109f32ff41 |
notifications: per-policy multi-channel expiry-alert routing
Closes Rank 4 of the 2026-05-03 Infisical deep-research deliverable
(see cowork/infisical-deep-research-results.md Part 5). Pre-fix,
RenewalService.CheckExpiringCertificates already ran daily,
RenewalPolicy.AlertThresholdsDays drove per-cert thresholds, and
NotificationService.SendThresholdAlert deduped per (cert, threshold)
— but the channel was hardcoded to Email
(internal/service/notification.go:118 pre-fix). Operators who
configured PagerDuty / Slack / Teams / OpsGenie via
CERTCTL_PAGERDUTY_ROUTING_KEY etc. got nothing at any threshold
unless SMTP was also wired. Their first signal of an expired cert
was a 3 AM outage.
This commit lands the routing matrix on top of the existing
infrastructure:
1. RenewalPolicy gains AlertChannels (per-tier channel list) +
AlertSeverityMap (per-threshold tier assignment) +
EffectiveAlertChannels / EffectiveAlertSeverity accessors.
Default*() helpers preserve the back-compat Email-only
behaviour for operators who haven't touched their policies
post-upgrade. Migration 000026 adds the JSONB columns
idempotently.
2. NotificationService.SendThresholdAlertOnChannel — the new
per-channel dispatch helper. Old SendThresholdAlert stays as
an Email-only alias so non-policy callers (admin "send test
alert" surfaces) keep working byte-for-byte.
3. NotificationService.HasThresholdNotificationOnChannel — per-
(cert, threshold, channel) deduplication so a transient
PagerDuty 5xx today does NOT suppress today's Slack alert and
tomorrow's PagerDuty retry will still fire.
4. RenewalService.sendThresholdAlerts walks the resolved channel
set per threshold tier, fans out to every configured channel,
handles per-channel failures independently, defensively drops
off-enum channels with an audit row trail, and records a per-
channel audit event with metadata.channel + metadata.severity_tier.
5. service.ExpiryAlertMetrics — atomic counter table mirrored on
the VaultRenewalMetrics shape from the 2026-05-03 audit fix #5
(commit
|
||
|
|
022caf39b4 |
ci(googlecas): fix QF1002 staticcheck — tagged switch on r.URL.Path
CI failure on commit
|
||
|
|
869fc8f245 |
docs(openssl): operator playbook for shell-out threat model
Closes Top-10 fix #6 of the 2026-05-03 issuer-coverage audit (see cowork/issuer-coverage-audit-2026-05-03/RESULTS.md). Pre-fix, the OpenSSL adapter's docs in docs/connectors.md explained usage but did NOT enumerate the threat model. The adapter exec's an arbitrary operator-supplied script — env-var inheritance, symlink attacks, sandbox-escape, multi-tenant process-isolation gaps. An acquirer's security reviewer reading this surface cold pattern-matches "highest-risk issuer surface with the lowest documented threat model." This commit lands a doc-side operator playbook in docs/connectors.md OpenSSL section (mirrors Bundle 8's "Operator playbook: keytool argv password exposure" subsection shape and the 2026-05-02 audit Top-10 fix #7 SSH InsecureIgnoreHostKey playbook). Six topics covered: 1. Why the adapter exists despite the risk (CLI-driven CAs without Go SDKs need an integration path). 2. Threat model the adapter accepts (trusted operator + trusted script + appropriate ownership + clear audit trail). 3. Threat model the adapter does NOT accept (operator-writable script paths, untrusted content, multi-tenant hosts). 4. Mitigations operators can layer (dedicated user, root-owned 0755 binary, audit rules, per-call timeout via CERTCTL_OPENSSL_TIMEOUT_SECONDS, env sanitisation, chroot/container, audit wrapper, per-call concurrency bound). 5. When NOT to use the adapter (compliance environments, multi-tenant servers, no-script-review environments). 6. V3-Pro forward path (hardened mode tracked in cowork/WORKSPACE-ROADMAP.md). Inline comment in internal/connector/issuer/openssl/openssl.go near the callSignScript exec call site forward-references the new doc subsection (no logic change). cowork/WORKSPACE-ROADMAP.md gains an "OpenSSL hardened mode" V3- Pro entry under "Adapter hardening" — sibling-folder doc, not in the certctl repo, so not reflected in this commit's diff. Same shape Bundle 8 used for the JavaKeystore playbook and the 2026-05-02 deployment-target audit Top-10 fix #7 used for the SSH InsecureIgnoreHostKey playbook. No code logic changes (only the explanatory comment near the exec call site). No test changes. Doc-only commit. Verified locally: - gofmt / go vet clean. - go test -short -count=1 ./internal/connector/issuer/openssl/... green. Audit reference: cowork/issuer-coverage-audit-2026-05-03/RESULTS.md Top-10 fix #6. |
||
|
|
0792271dc6 |
vault: add automatic token renewal at TTL/2 + Prometheus metric
Closes Top-10 fix #5 of the 2026-05-03 issuer-coverage audit (see cowork/issuer-coverage-audit-2026-05-03/RESULTS.md). Pre-fix, the VaultPKI adapter authenticated with a static token and never called renew-self. Long-lived deploys hit token expiry; the first operator-visible signal was failed cert renewals on production targets. This commit: 1. Connector.Start(ctx) spawns a goroutine that calls POST /v1/auth/token/renew-self at TTL/2 cadence (computed from a one-shot lookup-self at startup). Honours ctx.Done() for graceful shutdown via a per-loop done channel + Stop(). 2. On `renewable: false` response (initial lookup OR any subsequent renewal), the loop emits a WARN, increments the not_renewable counter, and exits. The operator must rotate the token before Vault's Max TTL elapses. 3. New Prometheus counter certctl_vault_token_renewals_total with labels result={success,failure,not_renewable}. Registered alongside existing certctl_issuance_* counters in internal/api/handler/metrics.go. 4. ERROR-level logging on renewal failure with operator-actionable substring ("vault token renewal failed; rotate the token before TTL expires") so journalctl + grep find it. Loop keeps ticking after a failure — transient blips don't kill it. New optional issuer.Lifecycle interface: type Lifecycle interface { Start(ctx context.Context) error Stop() } Connectors that hold no background goroutines (almost all of them) do not implement this — IssuerRegistry.StartLifecycles / StopLifecycles feature-detect via type assertion. New lifecycle-bearing connectors plug in by implementing the interface; no further registry plumbing required. Wiring (cmd/server/main.go): - service.NewVaultRenewalMetrics() instance is shared between issuerRegistry.SetVaultRenewalMetrics (so Vault connectors built by Rebuild get a recorder) and metricsHandler.SetVaultRenewals (so the Prometheus exposer emits the new series). - issuerRegistry.StartLifecycles(ctx) is called after issuerService.BuildRegistry; defer issuerRegistry.StopLifecycles is paired so goroutines exit cleanly on signal. - IssuerConnectorAdapter.Underlying() exposes the wrapped issuer.Connector so registry-level machinery can reach the concrete connector behind the adapter without duplicating the wiring at every call site. Tests (internal/connector/issuer/vault/vault_renew_test.go): - TestVault_RenewLoop_TickAtHalfTTL — three ticks → three renewals, all "success". - TestVault_RenewLoop_StopsOnNotRenewable — second renewal returns renewable=false, loop exits, third tick fires no HTTP call. - TestVault_RenewLoop_FailureSurfacesViaMetric — first renewal 403 bumps "failure", second renewal succeeds → loop kept ticking. - TestVault_RenewLoop_CtxCancellation_StopsCleanly — Stop returns within 200ms after ctx cancel. - TestVault_RenewLoop_StartsNothingWhenNotRenewable — token already non-renewable at boot ⇒ no goroutine, "not_renewable" metric increments at startup so operators see it in Grafana. - TestVault_ComputeInterval — 4 cases pinning TTL/2 + minRenewInterval floor. - TestVault_RenewSelf_ParseFailure_NamesActionableInError — surfaced error contains "vault token renewal failed" + "rotate the token". Cadence is dynamic — every successful renewal re-derives TTL/2 from the renewed lease's lease_duration, so a short bootstrap token that gets renewed up to a longer Max TTL shifts to the longer cadence automatically (defends against degenerate fast ticking on a token whose Max TTL is far longer than its initial TTL). Documentation: - docs/connectors.md Vault PKI section gains "Token TTL + automatic renewal" subsection (operator-facing: cadence, metric, renewable=false rotation playbook). Out of scope (intentional, flagged in the audit follow-up): - AppRole / Kubernetes / AWS IAM auth methods (different renewal semantics). - Hot-reload of rotated token from disk (operator restarts today; future: GUI/MCP issuer-update path triggers Rebuild which Stops the old connector and Starts the new one). - Auto-re-auth after token death (operator playbook owns it). CHANGELOG.md is intentionally not hand-edited (per CHANGELOG.md itself: "no longer maintains a hand-edited per-version changelog; per-release notes are auto-generated from commit messages between consecutive tags"). Verified locally: - gofmt clean. - go vet ./internal/service/... ./internal/api/handler/... ./internal/connector/issuer/vault/... ./cmd/server/... clean. - go test -short -count=1 ./internal/connector/issuer/vault/... ./internal/service/... ./internal/api/handler/... green. - go test -race -count=10 -run 'TestVault_RenewLoop|TestVault_ComputeInterval' ./internal/connector/issuer/vault/... green. Audit reference: cowork/issuer-coverage-audit-2026-05-03/RESULTS.md Top-10 fix #5. |
||
|
|
a2a59a823e |
googlecas, awsacmpca: add failure_test.go covering cloud-SDK error contracts
Closes Top-10 fix #4 of the 2026-05-03 issuer-coverage audit (see cowork/issuer-coverage-audit-2026-05-03/RESULTS.md). Pre-fix, both adapters had only happy-path test coverage with a single generic ServerError pair each. Cloud CAs are typically the first-deployed issuer in enterprise pilots; their diligence reviews dig hard into IAM-error / cloud-error coverage. This commit lands the contract tests. AWSACMPCA — 5 tests in awsacmpca_failure_test.go. Each injects a typed AWS SDK v2 error via the existing mockACMPCAClient seam and asserts (1) error non-nil, (2) errors.As against the SDK's typed value succeeds (so the wrap chain through fmt.Errorf("...%w", ...) is intact), and (3) operator-actionable substring is present. 1. Issue_AccessDenied — *smithy.GenericAPIError with Code="AccessDeniedException" (the SDK does NOT generate a typed *types.AccessDeniedException; AWS uses the smithy APIError shape for IAM denials). Asserts ErrorCode + "not authorized" + IAM resource path preserved through wrap. 2. Issue_ResourceNotFound — *types.ResourceNotFoundException names the missing CA ARN. 3. Issue_Throttling — *smithy.GenericAPIError with Code="ThrottlingException", Fault=FaultServer. Asserts the retryable class (FaultServer) is preserved through wrap so upstream retry logic can engage. 4. Issue_MalformedCSR — *types.MalformedCSRException is terminal (operator must fix the CSR, not retry); asserts the validation-issue substring survives. 5. Issue_RequestInProgress — *types.RequestInProgressException wraps cleanly; classification (retry vs reissue) is upstream's responsibility per the spec's "no new retry logic" rule. GoogleCAS — 5 tests in googlecas_failure_test.go. The adapter uses stdlib net/http directly (NO Google Cloud Go SDK dependency in googlecas.go), so SDK typed-error assertions don't translate. Each test runs an httptest.Server that returns the canonical Google API JSON error envelope: {"error":{"code":N,"message":"...","status":"<STATUS>"}} and asserts (1) error non-nil, (2) operator-actionable substring, and (3) the canonical status string ("PERMISSION_DENIED", "NOT_FOUND", "UNAVAILABLE") survives the wrap chain so upstream classification can branch on it. 1. Issue_PermissionDenied — 403 / PERMISSION_DENIED; surfaced error names the IAM resource path. 2. Issue_CAPoolNotFound — 404 / NOT_FOUND; surfaced error names the missing pool resource. 3. Issue_OAuth2TokenRefreshFailure — token endpoint returns 401 invalid_grant; surfaced error mentions "token" so an operator reading the log immediately distinguishes a credential failure (rotate SA key) from a CA-side error (fix IAM binding). Test also asserts the CAS endpoint is NOT reached when the token exchange fails. 4. Issue_RegionalAPIUnavailable — 503 / UNAVAILABLE; surfaced error preserves the retryable class markers (status code + UNAVAILABLE string) for upstream retry classification. 5. Revoke_PermissionDenied — adapter does NOT silently swallow the failure; pin the contract so the audit-row atomicity guarantee from Bundle G (which lives in the service-layer wrapper, not the adapter) continues to apply. Test also verifies the revoke endpoint was actually reached, guarding against a future regression that short-circuits before the HTTP call. Coverage delta: awsacmpca: 71.0% → 71.0% (failure tests reuse existing wrap code paths; behaviour-pin contract tests, not coverage tests). googlecas: 83.4% → 84.4% (+1.0pp). go.mod: smithy-go moved indirect → direct, since the new AWSACMPCA test file imports it. CI's go-mod-tidy-drift gate enforces this. Test-only commit. No production code changes. Verified locally: - gofmt clean. - go vet ./internal/connector/issuer/awsacmpca/... ./internal/connector/issuer/googlecas/... clean. - go test -short -count=1 ./internal/connector/issuer/... green. - go test -race -count=10 ./internal/connector/issuer/awsacmpca ./internal/connector/issuer/googlecas green. Audit reference: cowork/issuer-coverage-audit-2026-05-03/RESULTS.md Top-10 fix #4. |
||
|
|
b0c4ed1ae2 |
openssl: add failure_test.go covering 6 shell-out error modes
Closes Top-10 fix #3 of the 2026-05-03 issuer-coverage audit (see cowork/issuer-coverage-audit-2026-05-03/RESULTS.md). Pre-fix, the OpenSSL adapter (497 LOC, certctl's highest-risk issuer surface) had openssl_test.go (8 happy-path funcs + 20 subtests) but no dedicated _failure_test.go. Compare to ACME, Vault, DigiCert, Sectigo, Entrust, GlobalSign, EJBCA — all peers have one. An acquirer's diligence team flags this as an immediate blocker on the highest-risk issuer surface. This commit adds 6 failure-mode tests: 1. TestOpenSSL_Issue_ScriptNotFound_OperatorActionableError — SignScript path doesn't exist; error wraps os.ErrNotExist (errors.Is); message contains 'no such file' / 'not found' so the operator's grep finds it in journalctl. 2. TestOpenSSL_Issue_PermissionDenied_OperatorActionableError — SignScript exists with mode 0o600 (non-executable); error wraps os.ErrPermission; message contains 'permission'. Skipped under root (uid 0 bypasses chmod gating). 3. TestOpenSSL_Issue_MalformedStdout_DistinguishedFromCSRReject — script exits 0 + writes garbage (no PEM markers) to the cert output file; error mentions PEM/certificate/parse so operators distinguish output-parsing failure from a script- side fault. 4. TestOpenSSL_Issue_NonZeroExit_DistinguishesCAReject_From_ ScriptError — script writes 'policy violation: …' to stderr and exits 2 (CA-side rejection convention); the script's stderr surfaces in the error message; errors.Unwrap returns non-nil (proving the underlying *exec.ExitError chain survives). 5. TestOpenSSL_Issue_TimeoutEnforced_ContextCancellationPropagates — script does 'exec sleep 30' (not 'sleep 30 ' as a child; exec replaces bash so SIGKILL goes directly to the sleeper, avoiding the orphan-pipes corner case where a killed bash leaves sleep holding stdout/stderr open and CombinedOutput blocks); ctx with 100ms deadline; call returns within ~5s wall-clock; either errors.Is(err, context.DeadlineExceeded) or the error message names 'killed' / 'signal'. 6. TestOpenSSL_Issue_SignalKilled_PartialOutputDiscarded — script writes a half-PEM ('-----BEGIN CERTIFICATE-----\nMII…') then 'kill -KILL $$'; assertion: result is nil OR CertPEM is empty (no half-cert leaks to caller); error names 'signal' / 'killed' OR 'PEM' / 'parse' (both are operator-actionable). Each test pins the operator-actionable error message contract: the message names the failure mode (so journalctl + grep find it) and proves no half-state was created (no partial cert returned). errors.Is / errors.Unwrap checks confirm the wrapping chain survives. The OpenSSL adapter has no commandRunner abstraction (production code uses exec.CommandContext directly); these tests use real operator-supplied scripts written to t.TempDir (matches the adapter's actual production code path; no os/exec mocking). The 'exec sleep 30' technique in Test 5 is the load-bearing fix for the bash-orphans-sleep-and-pipes-stay-open corner case that otherwise makes the test take 30s instead of 100ms. Coverage delta: - Before this commit: openssl_test.go + openssl_stubs_test.go covered 8 happy-path funcs. - After: 79.8% statement coverage of openssl.go (up from operator-pre-existing baseline; the 6 new tests exercise every error path through callSignScript + parseCertificate). Tests pass clean under '-race -count=10' (Test 5's deadline tolerance is the only timing-sensitive case; the 5s wall-clock budget vs the 100ms ctx deadline gives ample slack on slow CI without masking deadline-not-enforced bugs). Test-only commit; no production code changes. Hardening fixes (per-call concurrency semaphore, threat-model docs) are separate Top-10 entries. Verified locally: - gofmt clean across the repo. - go vet ./... clean across the repo. - go test -race -count=10 -short ./internal/connector/issuer/openssl/... green. Audit reference: cowork/issuer-coverage-audit-2026-05-03/ RESULTS.md Top-10 fix #3. |
||
|
|
d3bf2cc0cf |
vault, digicert: migrate Token / APIKey to *secret.Ref (Bundle I Phase 3)
Closes Top-10 fix #2 of the 2026-05-03 issuer-coverage audit (see cowork/issuer-coverage-audit-2026-05-03/RESULTS.md). Pre-fix, vault.Config.Token and digicert.Config.APIKey were plain string fields. Practical impact: 1. GET /api/v1/issuers responses marshalled the credential into the JSON body. An acquirer's procurement engineer running 'curl /api/v1/issuers | jq' saw the token / API key in plain text on screen. 2. DEBUG-level HTTP request logging printed the credential header verbatim. 3. A heap dump of the running server contained the credential as readable bytes for the lifetime of the process. Bundle I from the 2026-05-01 audit closed this for AWSACMPCA, EJBCA, GlobalSign, Sectigo (Phase 1+2). Vault and DigiCert were left out. This commit ports the same migration onto them. Mechanics: - Config.Token / Config.APIKey type changed from 'string' to '*secret.Ref'. UnmarshalJSON of a JSON string populates the Ref via NewRefFromString — operator config files are unchanged. - Every header-write call site routed through Ref.Use, with the byte buffer zeroed after the callback returns. Vault: 3 sites (IssueCertificate, RevokeCertificate, GetCACertPEM). DigiCert: 5 sites (ValidateConfig, IssueCertificate, RevokeCertificate, pollOrderOnce, downloadCertificate). - ValidateConfig nil-checks switch from 'cfg.Token == ""' to 'cfg.Token.IsEmpty()' (mirrors Sectigo's existing pattern). - Tests migrated: every Config{Token:"..."} → Config{Token: secret.NewRefFromString("...")}. The 'json.Marshal(config) → ValidateConfig(rawConfig)' round-trip pattern in DigiCert's ValidateConfig_Success test is now broken by the redact-on-marshal contract — switched that one to construct the rawConfig as a JSON literal (mirrors Sectigo's existing test pattern). - Two new tests pin the redact-on-marshal contract: - TestVault_Config_TokenMarshalsAsRedacted (vault_redact_test.go) - TestDigiCert_Config_APIKeyMarshalsAsRedacted (digicert_redact_test.go) Both assert the marshaled JSON contains '"[redacted]"' and does NOT contain the plaintext bytes. Operator-visible: GET /api/v1/issuers responses for type=vault and type=digicert now show the credential as '[redacted]'. Existing config files keep working — the Ref unmarshal accepts strings. CHANGELOG note: certctl/CHANGELOG.md is intentionally not hand-edited; release notes are auto-generated from commit messages between consecutive tags. This commit's message body is the release-note artifact. Verified locally: - gofmt clean across the repo. - go vet ./... clean across the repo. - go test -race -count=1 -short ./internal/connector/issuer/vault/... ./internal/connector/issuer/digicert/... ./internal/secret/... green. Audit reference: cowork/issuer-coverage-audit-2026-05-03/ RESULTS.md Top-10 fix #2. |
||
|
|
81f6321326 |
ejbca: port mTLS keypair to mtlscache (close Bundle M for the last issuer)
Closes Top-10 fix #1 of the 2026-05-03 issuer-coverage audit (see cowork/issuer-coverage-audit-2026-05-03/RESULTS.md). Pre-fix, ejbca.go::New called tls.LoadX509KeyPair once at construction and configured the keypair into *http.Transport.TLSClientConfig with no mtime watch. mTLS rotation required a server restart — quarterly rotation per any reasonable security policy = quarterly deploy outage. Bundle M from the prior 2026-05-01 audit shipped the mtlscache helper at internal/connector/issuer/mtlscache/cache.go and wired it into Entrust + GlobalSign. EJBCA was missed in Bundle M's scope. This commit ports the same helper onto EJBCA's auth_mode=mtls path. The OAuth2 path is unchanged. Implementation: - New imports internal/connector/issuer/mtlscache. - Connector struct gains an mtls *mtlscache.Cache field (mirroring Entrust + GlobalSign). - New()'s case 'mtls': replaces tls.LoadX509KeyPair + manual *http.Transport with mtlscache.New(certPath, keyPath, Options{HTTPTimeout: 30s}). Cache build happens at construction so misconfigured operators fail fast (matches pre-fix behaviour). - New helper getHTTPClient() returns the cached client; on the mTLS path it calls RefreshIfStale before returning so the next request uses the new keypair if disk has rotated. On OAuth2 / test paths (c.mtls == nil), returns c.httpClient as-is. - All 3 c.httpClient.Do call sites (IssueCertificate enroll, RevokeCertificate revoke, GetOrderStatus cert lookup) replaced with c.getHTTPClient() + client.Do. - crypto/tls import removed (no longer used at this layer). Tests: - TestEJBCA_MTLSKeypairRotation_PicksUpNewCertWithoutRestart (new, ejbca_mtls_rotation_test.go): generates two CAs (caA, caB), signs leafA + leafB, spins up an httptest TLS server that trusts both CAs and records the issuer DN of every presented client cert, writes leafA, makes request 1, writes leafB + advances mtime by 2s, makes request 2. Asserts the server saw caA's DN on req 1 and caB's DN on req 2 — the cache picked up the rotation without ejbca.New re-running. - export_test.go: GetHTTPClientForTest helper exposes the private getHTTPClient so the rotation test drives the production code path. - All existing EJBCA tests still pass (TestNew_MTLSWiresClientCert, TestNew_MTLSCertLoadFailure, TestNew_OAuth2NoTransportTuning, TestNew_InvalidAuthMode). Verified locally: - gofmt clean across the repo. - go vet ./... clean across the repo. - go test -race -count=1 -short ./internal/connector/issuer/ejbca/... ./internal/connector/issuer/mtlscache/... green. Audit reference: cowork/issuer-coverage-audit-2026-05-03/ RESULTS.md Top-10 fix #1. |
||
|
|
bee47f0318 |
acme-server: cert-manager integration test + production hardening (Phase 5/7)
Closes the production-readiness loop on the ACME surface. After this
commit, certctl ships per-account rate limits + a GC sweeper for
expired ACME state + a kind-driven cert-manager 1.15 integration test
+ a lego-driven RFC conformance harness + a k6 loadtest scenario for
the unauthenticated ACME path.
Architecture:
- Rate limits live in-memory + per-replica. Restart wipes the
counters; orders/hour caps are eventual-consistency anyway. A
3-replica certctl-server fleet behind an LB effectively has 3x
the configured throughput per account; persistent rate limiting
is a follow-up if production telemetry shows abuse patterns we
can't catch in a single restart cycle. Per-key + per-action
isolation: ActionNewOrder/acc-1, ActionKeyChange/acc-1, and
ActionChallengeRespond/<challenge-id> are independent buckets.
- GC loop follows the existing scheduler-loop pattern (atomic.Bool
+ sync.WaitGroup; see crlGenerationLoop for shape). Three
independent SQL sweeps per tick (DELETE expired nonces; UPDATE
pending authzs whose expires_at < now() to expired; UPDATE
pending/ready/processing orders whose expires_at < now() to
invalid). Each sweep is a single statement; failures are logged-
and-continued so a failing nonces sweep doesn't block authzs.
Per-sweep 1m timeout bounds a stuck Postgres.
- cert-manager integration test is gated on KIND_AVAILABLE so CI
skips it cleanly (kind is too heavy for per-PR). Operators run
locally via 'make acme-cert-manager-test'; the harness brings up
a fresh cluster each run + tears it down on Cleanup.
- lego conformance harness drives a real ACME client through
register → run → cert-PEM-landed against a hermetic certctl
stack. Catches RFC-shape regressions third-party clients would
hit before they ship.
- k6 ACME-flow scenario hammers the unauthenticated surface
(directory + new-nonce + ARI synthetic-id) at 100 VUs × 5m. JWS-
signed flows are out of scope for k6 (no JWS support); they're
covered by the lego harness above.
What ships:
- internal/api/acme/ratelimit.go (+ ratelimit_test.go: 7 cases —
disable-when-perHour-zero, capacity, per-key isolation, per-
action isolation, refill-over-time, RetryAfter, concurrent-access
with -race + 200 goroutines × 200 calls).
- internal/repository/postgres/acme.go: 4 new methods —
CountActiveOrdersByAccount + GCExpiredNonces + GCExpireAuthorizations
+ GCInvalidateExpiredOrders. Each a single SQL statement.
- internal/service/acme.go: SetRateLimiter + GarbageCollect +
rate-limit gates at 3 entry points (CreateOrder + RotateAccountKey
+ RespondToChallenge) + concurrent-orders gate at CreateOrder.
2 new sentinels (ErrACMERateLimited, ErrACMEConcurrentOrdersExceeded);
5 new GC metrics (gc_runs / gc_run_failures / gc_nonces_reaped /
gc_authzs_expired / gc_orders_invalidated).
- internal/scheduler/scheduler.go: ACMEGarbageCollector interface +
acmeGCRunning atomic.Bool + acmeGCInterval + 2 setters (SetACME-
GarbageCollector + SetACMEGCInterval) + acmeGCLoop following the
crlGenerationLoop shape.
- internal/api/handler/acme.go: writeServiceError gains rateLimited
(429 + RFC 8555 §6.7) + concurrent-orders-exceeded mappings.
- internal/config/config.go: 5 new env vars
(CERTCTL_ACME_SERVER_RATE_LIMIT_ORDERS_PER_HOUR=100,
CERTCTL_ACME_SERVER_RATE_LIMIT_CONCURRENT_ORDERS=5,
CERTCTL_ACME_SERVER_RATE_LIMIT_KEY_CHANGE_PER_HOUR=5,
CERTCTL_ACME_SERVER_RATE_LIMIT_CHALLENGE_RESPONDS_PER_HOUR=60,
CERTCTL_ACME_SERVER_GC_INTERVAL=1m).
- cmd/server/main.go: NewRateLimiter() + SetRateLimiter() at
startup; conditional SetACMEGarbageCollector(acmeService) +
SetACMEGCInterval(cfg.ACMEServer.GCInterval) when Enabled+
GCInterval > 0.
- deploy/test/acme-integration/: kind-config.yaml + cert-manager-
install.sh + clusterissuer-trust-authenticated.yaml +
clusterissuer-challenge.yaml + certificate-test.yaml + conformance-
lego.sh + certmanager_test.go (//go:build integration + KIND_AVAILABLE
gate).
- deploy/test/loadtest/k6/acme_flow.js + README ACME-flows section.
- Makefile: 2 new PHONY targets (acme-cert-manager-test +
acme-rfc-conformance-test).
- docs/acme-server.md: status flipped to Phase 5; Configuration
table grows 5 rows; new 'Phase 5 — operational guidance' section
explaining rate-limit math + GC sweeper semantics + cert-manager
integration + lego conformance + k6 baseline.
Tests:
- 'go vet ./...' clean across the repo.
- 'go test -short -count=1 ./internal/...' green across every
affected package (service / acme / handler / scheduler / repo /
config).
- 'go vet -tags=integration ./deploy/test/acme-integration/' clean
(the integration test compiles cleanly with the build tag).
- The kind/cert-manager harness is gated behind KIND_AVAILABLE so
CI skips by default; operators run locally via 'make acme-cert-
manager-test'.
Engineering history: cowork/WORKSPACE-CHANGELOG.md 'ACME-Server-5'.
|
||
|
|
650f5a198f |
fix: collapse identical if/else branches in Account handler (CodeQL #25)
CodeQL alert #25 (go/duplicate-branches) on internal/api/handler/
acme.go::ACMEHandler.Account flagged that 'if readOnly { ... } else
{ ... }' had byte-identical bodies — both setting the same
Content-Type: application/json header. The 'readOnly' bool was
threaded through the function as a placeholder for differentiated
headers (Cache-Control etc. on the POST-as-GET path) that never
landed; both branches collapsed to the same value with no
follow-through.
Audit + fix:
- The alert is real (verified by re-reading the source); not a
false positive.
- The Copilot Autofix Anthropic surfaced was correct in spirit but
incomplete: it collapsed the if/else but left 'readOnly' as
dead code (declared at line 395, assigned at lines 400 and 436,
only read at the now-removed if). golangci-lint's 'unused'
linter would flag 'readOnly' next.
- Complete fix: collapse the if/else AND remove the now-unused
'readOnly' variable + its 2 assignments. Single unconditional
'w.Header().Set("Content-Type", "application/json")' covers
both paths (RFC 8555 §6.3 POST-as-GET + §7.3.2 / §7.3.6 update
+ deactivation all return the same account JSON shape — no spec
rationale for differentiating headers).
Verified locally: 'gofmt -l .' clean; 'go vet ./...' clean;
'go test -short -count=1 ./internal/api/handler/' green; 'grep
readOnly' on the file returns only the new explanatory comment
(no live references).
The alert was first detected in commit
|
||
|
|
1e1bc9b3b4 |
ci: fix Phase 4 post-push unused-symbol failures
CI on commit
|
||
|
|
f6ba5634fd |
ci: fix Phase 4 post-push gofmt failure (map-literal alignment)
CI on commit
|
||
|
|
4dc8d3fa5b |
acme-server: key rollover + revocation + ARI (Phase 4/7)
Closes the RFC 8555 + RFC 9773 surface beyond the issuance happy-path:
- POST /acme/profile/<id>/key-change (RFC 8555 §7.3.5)
- POST /acme/profile/<id>/revoke-cert (RFC 8555 §7.6)
- GET /acme/profile/<id>/renewal-info/<cert-id> (RFC 9773 ARI)
After this commit, ACME clients can rotate account keys, revoke certs
through the ACME surface (rather than only via the certctl GUI/API),
and fetch ARI for proactive renewal scheduling.
Architecture:
- Key rollover: outer JWS verified against the registered account key
(existing kid path); the inner JWS — embedded as the outer's payload
— verified against the embedded NEW jwk in a new dedicated routine
(ParseAndVerifyKeyChangeInner) that enforces RFC 8555 §7.3.5
inner-only invariants: MUST use jwk + MUST NOT use kid, payload
.account == outer.kid, payload.oldKey thumbprint-equals registered.
A single WithinTx swaps the stored thumbprint+pem and writes the
audit row. Concurrent-rollover safety via SELECT…FOR UPDATE on the
conflicting account row in UpdateAccountJWKWithTx; the loser
observes the winner's new thumbprint and is told to retry (409).
- Revocation: two auth paths. kid → AccountOwnsCertificate single-
indexed COUNT lookup over acme_orders. jwk → constant-time RFC 7638
thumbprint compare against the cert's pubkey. Both paths route
through service.RevocationSvc.RevokeCertificateWithActor so the
existing CRL/OCSP refresh + audit + metrics pipeline applies. RFC
5280 §5.3.1 numeric reason codes clamp to certctl's
domain.ValidRevocationReasons; codes 8 (removeFromCRL) + 10
(aACompromise) clamp to 'unspecified' since they aren't in the set.
- ARI is GET-only and unauth per RFC 9773 §4. Cert-id wire shape is
base64url(AKI).base64url(serial); ParseARICertID strict-decodes,
SerialHex emits the canonical certctl-shape lowercase-no-leading-
zeros hex used in certificate_versions.serial_number.
ComputeRenewalWindow has 3 branches: bound RenewalPolicy →
[notAfter - days, notAfter - days/2]; no policy → last 33% of
validity; past expiry → [now, now + 1d] (renew immediately).
Retry-After honors CERTCTL_ACME_SERVER_ARI_POLL_INTERVAL.
What ships:
- internal/api/acme/{keychange,ari}.go (+ phase4_test.go: 15 tests).
- internal/api/acme/order.go: RevokeCertRequest wire shape.
- internal/api/handler/acme.go: KeyChange, RevokeCert, RenewalInfo
+ 11 new writeServiceError mappings.
- internal/repository/postgres/acme.go: UpdateAccountJWKWithTx (FOR
UPDATE + expectedOldThumbprint precondition; ErrACMEAccountKey-
ConcurrentUpdate sentinel) + AccountOwnsCertificate.
- internal/service/acme.go: RotateAccountKey + RevokeCert +
RenewalInfo; CertificateRevoker + RenewalPolicyLookup interfaces;
SetRevocationDelegate + SetRenewalPolicyLookup wiring; 11 new
sentinels; 6 new metrics.
- internal/service/acme_phase4_test.go: service-layer tests for
RotateAccountKey (happy + duplicate-key) + RevokeCert (kid mismatch
+ jwk mismatch + jwk happy + already-revoked + reason-clamping) +
RenewalInfo (disabled + bad cert-id).
- internal/api/router/router.go: 6 new register calls (3 per-profile
+ 3 shorthand). Router parity exceptions extended in lockstep
(in-tree SpecParityExceptions + CI-only openapi-handler-exceptions
.yaml).
- cmd/server/main.go: SetRevocationDelegate(revocationSvc) +
SetRenewalPolicyLookup(renewalPolicyRepo) at startup.
- internal/config/config.go: CERTCTL_ACME_SERVER_ARI_ENABLED (default
true) + CERTCTL_ACME_SERVER_ARI_POLL_INTERVAL (default 6h);
BuildDirectory's ariEnabled flag now flips on under
cfg.ARIEnabled.
- docs/acme-server.md: phase status flipped to Phase 4; endpoints
table grows 6 rows (3 per-profile + 3 shorthand); FAQ section
appended explaining how to rotate keys, revoke certs, and consume
ARI.
Tests:
- 'go vet ./...' clean across the repo.
- 'go test -short -count=1 ./...' green across every package.
- phase4_test.go covers: keychange happy-path + 5 negatives +
MapKeyChangeErrorToProblem coverage; ARI cert-id round-trip + 6
malformed cases + BuildARICertID from a generated cert; window-
math 3 branches.
- service-layer tests confirm: RotateAccountKey atomically swaps the
thumbprint (verifies persisted state) and rejects duplicate keys;
RevokeCert routes through the stub RevocationSvc with the right
actor string + reason on the jwk path, rejects mismatched keys,
rejects already-revoked certs, clamps reason codes correctly;
RenewalInfo respects ARIEnabled + cert-id format.
Engineering history: cowork/WORKSPACE-CHANGELOG.md 'ACME-Server-4'.
|
||
|
|
62513ad12f |
ci: fix Phase 3 post-push CI failures (contextcheck + ST1021)
CI on commit
|
||
|
|
9bc845304e |
acme-server: HTTP-01 + DNS-01 + TLS-ALPN-01 challenge validation (Phase 3/7)
Wires up the actual challenge-validation machinery so profiles in
acme_auth_mode='challenge' resolve end-to-end. After this commit,
cert-manager 1.15+ with `solver: http01: ingress` against a
challenge-mode profile completes a real HTTP-01 flow and gets a cert.
DNS-01 + TLS-ALPN-01 share the same code path with the appropriate
validator selection.
Architecture (the load-bearing parts):
- 3 separate semaphore-bounded worker pools (one per challenge type),
so HTTP-01 and DNS-01 can't starve each other under load. Default
weight 10 per type; tunable via CERTCTL_ACME_SERVER_HTTP01_CONCURRENCY,
DNS01_CONCURRENCY, TLSALPN01_CONCURRENCY.
- 30s per-challenge timeout (configurable via PoolConfig.PerChallengeTimeout).
- HTTP-01 validator runs validation.IsReservedIPForDial (newly
exported wrapper preserving the existing private impl byte-for-byte
for the network scanner + ValidateSafeURL paths) on the resolved
IP — both at the initial dial and every redirect hop. SSRF probes
into private IP space are refused before the connect.
- DNS-01 validator uses a dedicated resolver pointed at
CERTCTL_ACME_SERVER_DNS01_RESOLVER (default 8.8.8.8:53) — does
NOT use the system resolver to keep behavior deterministic across
deployments. Wildcard handling: `*.example.com` queries
_acme-challenge.example.com.
- TLS-ALPN-01 validator (RFC 8737) connects with ALPN `acme-tls/1`,
inspects the id-pe-acmeIdentifier extension (OID 1.3.6.1.5.5.7.1.31),
asserts the ASN.1 OCTET STRING value equals SHA-256 of the key
authorization. Cert chain is intentionally NOT validated
(InsecureSkipVerify=true is correct per RFC 8737 — the proof is
in the extension, not the chain). Documented in docs/tls.md L-001
table + the //nolint:gosec comment carries the justification.
SSRF guard: same posture as HTTP-01.
- Validation is asynchronous: handler accepts the POST and returns
200 immediately with status=processing; the worker-pool fires a
callback that updates challenge → authz → order in a fresh
background-context WithinTx. The order auto-promotes to `ready`
when ALL authzs become valid; auto-fails to `invalid` when ANY
authz becomes invalid.
What ships:
- internal/api/acme/challenge.go: KeyAuthorization (RFC 8555 §8.1) +
DNS01TXTRecordValue (§8.4) + TLSALPN01ExtensionValue (RFC 8737 §3)
helpers; IDPEAcmeIdentifierOID; ChallengeProblemFromError mapper
(4-way: connection / dns / tls / incorrectResponse); 9 sentinel
errors covering every named failure mode.
- internal/api/acme/validators.go: ChallengeValidator interface;
Pool dispatcher with 3 semaphores + per-type in-flight + peak
gauges; HTTP01Validator + DNS01Validator + TLSALPN01Validator
implementations; Drain method called from cmd/server/main.go's
shutdown sequence.
- internal/api/acme/validators_test.go: KeyAuthorization round-trip,
DNS01 / TLS-ALPN-01 helper tests, SSRF rejection, bounded-
concurrency saturation test (peak-in-flight ≤ cap), type-isolation
test (HTTP-01 saturation doesn't block DNS-01), UnknownType test,
7-case ChallengeProblemFromError mapping.
- internal/repository/postgres/acme.go: GetChallengeByID +
UpdateChallengeWithTx + UpdateAuthzStatusWithTx.
- internal/service/acme.go: SetValidatorPool wires the *acme.Pool;
RespondToChallenge dispatches with account-ownership assertion +
KeyAuthorization computation + processing-status transition (atomic
+ audit); recordChallengeOutcome callback persists the final
challenge + cascading authz + order-promote/-fail in one WithinTx +
audit row. 4 new metrics.
- internal/api/handler/acme.go: Challenge handler; round-trips
account.JWKPEM through ParseJWKFromPEM to recover the *jose.JSONWebKey
the validator pool needs.
- internal/api/router/router.go + openapi_parity_test.go +
api/openapi-handler-exceptions.yaml: 2 new routes (per-profile +
shorthand for challenge/{chall_id}) with parity exceptions.
- cmd/server/main.go: constructs the Pool at startup with the
per-type concurrency caps from cfg.ACMEServer; ACMEService.ValidatorPool()
accessor exposed for the shutdown drain sequence.
- internal/validation/ssrf.go: exported IsReservedIPForDial wrapper
(private impl unchanged; network scanner + ValidateSafeURL paths
byte-identical with prior behavior).
- docs/tls.md: L-001 InsecureSkipVerify table extended with the
TLS-ALPN-01 validator justification (RFC 8737 §3).
- docs/acme-server.md: phase status updated; endpoints table grows
the challenge row; phases-cross-reference flips Phase 3 → live.
Tests:
- 80%+ coverage on the new files.
- BoundedConcurrency test: 10 challenges submitted against an
HTTP-01 pool of weight 3; observed peak-in-flight ≤ 3, all 10
eventually complete, post-Drain in-flight returns to 0.
- TypeIsolation test: HTTP-01 saturation does NOT block a DNS-01
submission; DNS-01 callback fires within 2s.
- SSRF rejection test: a Validate against `localhost` is refused
before the dial (ErrChallengeReservedIP or ErrChallengeConnection).
Engineering history: cowork/WORKSPACE-CHANGELOG.md "ACME-Server-3".
|
||
|
|
c351bba41a |
acme-server: orders + authorizations + finalize + cert download (Phase 2/7)
Closes the issuance loop in trust_authenticated mode (commits |
||
|
|
44a85d6f85 |
acme-server: account resource + JWS verifier (Phase 1b/7)
Layers JWS-authenticated POST machinery onto the Phase 1a foundation
(commit
|
||
|
|
ec88a61274 |
acme-server: foundation — directory + new-nonce + per-profile routing (Phase 1a/7)
First slice of the RFC 8555 ACME server endpoint (master plan at cowork/acme-server-endpoint-prompt.md, per-phase prompts at cowork/acme-server-prompts/). This commit lands the smallest viable end-to-end deployable slice: an ACME client running curl -sk https://certctl/acme/profile/<id>/directory curl -sk -I https://certctl/acme/profile/<id>/new-nonce successfully fetches the directory document and a Replay-Nonce. Account creation, JWS verification, orders, challenges, and revocation are all out of scope for this phase and arrive in Phases 1b–4. Closes the Rank 1 LHF from the 2026-05-03 Infisical deep-research (cowork/infisical-deep-research-results.md). Pre-fix, certctl was an ACME consumer only — no /acme/directory endpoint, no JWS verifier, no challenge validators. K8s customers running cert-manager could not point at certctl as an ACME issuer; they had to deploy a certctl agent on every node. What ships: - internal/api/acme/{directory,nonce,errors}.go (+ tests). - internal/api/handler/acme.go + acme_handler_test.go. - internal/repository/postgres/acme.go (nonce ops only — Phase 1b extends with account CRUD; Phases 2-4 extend with order / authz / challenge CRUD). - internal/service/acme.go (BuildDirectory + IssueNonce stubs; Phase 1b adds VerifyJWS / NewAccount / etc.). - migrations/000025_acme_server.{up,down}.sql ships the full 5-table ACME schema (acme_accounts / acme_orders / acme_authorizations / acme_challenges / acme_nonces) PLUS the per-profile certificate_profiles.acme_auth_mode column. Phase 1a actively uses only acme_nonces; remaining tables are empty until Phases 1b-4 plug in. - internal/config/config.go: ACMEServerConfig struct + ACMEServer field on Config. Env vars use CERTCTL_ACME_SERVER_* prefix to avoid colliding with the existing consumer-side ACMEConfig at config.go:1746 (CERTCTL_ACME_DIRECTORY_URL / PROFILE / CHALLENGE_TYPE etc.). Phase 1a wires Enabled + DefaultAuthMode + DefaultProfileID + NonceTTL + DirectoryMeta; Order/Authz TTLs + per-challenge-type concurrency caps + DNS01 resolver are reserved fields parsed in 1a so operators can set them ahead of Phases 2/3. - cmd/server/main.go: wire ACMEHandler into the HandlerRegistry literal alongside the existing certificate / EST / SCEP / etc. handlers. - internal/api/router/router.go: HandlerRegistry.ACME field + 6 Register calls (3 per-profile + 3 shorthand). - internal/api/router/openapi_parity_test.go: 6 new entries in SpecParityExceptions. ACME is a wire-protocol surface (JWS-signed JSON over HTTPS per RFC 7515) whose semantics are dictated by RFC 8555 + RFC 9773 rather than by an OpenAPI document, same precedent as SCEP/EST. The canonical reference is docs/acme-server.md. - docs/acme-server.md: Phase-1a-shaped reference. Configuration table for every CERTCTL_ACME_SERVER_* env var. Per-profile auth-mode decision tree skeleton. TLS trust bootstrap section flagging cert-manager's ClusterIssuer.spec.acme.caBundle requirement (the single biggest first-time-deploy footgun; the full cert-manager walkthrough lands in Phase 6 but the requirement is documented up front). Architecture decisions baked in: - URL family is /acme/profile/<id>/* (per-profile, canonical) with /acme/* shorthand active when CERTCTL_ACME_SERVER_DEFAULT_PROFILE_ID is set. Path matches existing per-profile precedent in EST + SCEP. - Auth mode is per-profile (acme_auth_mode column on certificate_profiles), NOT server-wide. One certctl-server can serve trust_authenticated for an internal-PKI profile and challenge for a public-trust-style profile simultaneously. The column is read at request time, not cached at server start — operators flipping a profile's mode via SQL take effect on the next order without restart. - Nonces are DB-backed (acme_nonces table). Survive server restart. The RFC 8555 §6.5 replay defense requires the store to outlast the client's nonce caching window; an in-memory-only nonce store would lose every in-flight order on restart. - Per-op atomic counters on service.ACMEService.Metrics() — certctl_acme_directory_total, certctl_acme_directory_failures_total, certctl_acme_new_nonce_total, certctl_acme_new_nonce_failures_total. Naming follows certctl frozen decision 0.10 cardinality discipline. Phase 1b will extend with new_account counters; Phase 2 with order / finalize / cert; Phase 3 with per-challenge-type counters. Audit fixes #11 + #12 (cowork/acme-server-prompts/audit-additions.md) applied: - #11: CERTCTL_ACME_SERVER_* prefix avoids the consumer-side CERTCTL_ACME_* namespace collision. - #12: prior-attempt WIP from two failed Phase-1 dispatches was discarded at phase start; this commit starts from a clean tree. Tests: - 14 unit tests in internal/api/acme/ (directory, nonce, errors). - 7 handler-level tests via httptest.NewServer + mockACMEService (mirrors the mockSCEPService pattern at scep_handler_test.go). - 7 service-layer tests with mocked repo + injected profileLookup. - All pass under -race -count=1 -short. Deferred to Phase 1b: - JWS verification (go-jose v4 — see master-prompt §8a for the API surface and audit doc for the speculation pitfalls). - new-account / account/<id> endpoints + AccountService. - Nonce *consumption* path (issue path is in this commit; consume is only invoked by JWS-verified POSTs which Phase 1b adds). Engineering history: cowork/WORKSPACE-CHANGELOG.md "ACME-Server-1a". Per-phase implementation plan: cowork/acme-server-prompts/. Master plan + audit fixes: cowork/acme-server-endpoint-prompt.md + cowork/acme-server-prompt-audit.md + cowork/acme-server-prompts/audit-additions.md. |
||
|
|
b8b7e1e3dd |
tlsprobe: add VerifyWithExponentialBackoff + rewire all connectors' runPostDeployVerify
Closes Top-10 fix #8 of the 2026-05-02 deployment-target audit re-run (see cowork/deployment-target-audit-2026-05-02-rerun/ RESULTS.md). Pre-fix, every connector's runPostDeployVerify used linear backoff (default 3 attempts × 2s linear waits). Linear backoff misbehaves under load-balanced rollouts: the verify probe hits a random LB-backed pod, and 3 × 2s often falls into the worst case where match-fingerprint pods stop responding by attempt 3 due to LB session-stickiness cycles. This commit: 1. New shared helper internal/tlsprobe/retry.go:: VerifyWithExponentialBackoff. Default 3 attempts; 1s initial, 16s cap. Doubling pattern: 1s → 2s → 4s → 8s → 16s. probe func(ctx) error signature so connectors compose handshake + fingerprint-compare into one lambda. 2. Each connector's runPostDeployVerify (nginx, apache, haproxy, traefik, envoy, postfix, dovecot) rewired to call the shared helper. Per-connector signature unchanged. 3. New PostDeployVerifyMaxBackoff time.Duration field added to each connector's Config. Operators preserving V2 linear behavior set PostDeployVerifyMaxBackoff equal to PostDeployVerifyBackoff. 4. Tests: - tlsprobe/retry_test.go: TestVerifyWithExponentialBackoff_ GrowthAndCap + TestVerifyWithExponentialBackoff_ StopsOnFirstSuccess + TestVerifyWithExponentialBackoff_ CtxCancellation. - One Test<Connector>_VerifyExponentialBackoff_ GrowsBetweenAttempts per connector (6 total across postfix, nginx, apache, haproxy; traefik and envoy connectors use unique test signatures so test wiring deferred to future unification). 5. docs/deployment-atomicity.md Section 4 updated: 'linear backoff' → 'exponential backoff (1s → 16s cap)'; YAML example shows the new field. Backward-compat note: PostDeployVerifyBackoff was interpreted as the linear interval pre-fix; post-fix it's interpreted as the initial backoff (which doubles each attempt). Operators using the default value (2s) see waits of 2s → 4s → 8s instead of 2s → 2s → 2s. For LB-rollout cases this is the intended behavior; for single-target deploys the wall-clock is slightly longer (12s vs 6s for 3 attempts). Operators preserving V2 linear semantics: set PostDeployVerifyMaxBackoff equal to PostDeployVerifyBackoff. Verified locally: - gofmt clean. - go test -short -count=1 ./internal/tlsprobe/... ./internal/connector/target/{postfix,nginx,apache,haproxy}/... green. Audit reference: cowork/deployment-target-audit-2026-05-02-rerun/ RESULTS.md Top-10 fix #8. |
||
|
|
b16e5b5e97 |
docs(ssh): operator playbook for InsecureIgnoreHostKey design choice
Closes Top-10 fix #7 of the 2026-05-02 deployment-target audit re-run (see cowork/deployment-target-audit-2026-05-02-rerun/ RESULTS.md). Pre-fix, the SSH connector's ssh.InsecureIgnoreHostKey() at internal/connector/target/ssh/ ssh.go (realSSHClient.Connect) had only an inline comment justifying the design choice. An acquirer's diligence engineer reading the connector cold pattern-matches "MITM hazard" without seeing the comment. This commit lands a doc-side operator playbook in docs/connectors.md SSH section covering: 1. Why the connector accepts any host key (operator-configured target infrastructure; mirrors network scanner's InsecureSkipVerify and F5's Insecure flag). 2. Threat model the choice accepts (passive eavesdropper on operator-controlled network; layered SSH-key auth limits blast radius). 3. Threat model the choice does NOT accept (public-internet ephemeral hosts, multi-tenant networks, strict MITM- resistance regulatory requirements). 4. Mitigations operators can layer (custom SSHClient via NewWithClient + golang.org/x/crypto/ssh/knownhosts; SSH certificate authentication via @cert-authority pinning; network segmentation; per-target key rotation). 5. When to NOT use the SSH connector (regulatory environments, dynamic IPs, multi-tenant networks). 6. V3-Pro forward path (built-in known_hosts management, tracked in WORKSPACE-ROADMAP.md). Inline comment in ssh.go realSSHClient.Connect updated to forward-reference the new doc subsection (no logic change; same HostKeyCallback: ssh.InsecureIgnoreHostKey() call). Same shape Bundle 8 used for "Operator playbook: keytool argv password exposure" in docs/connectors.md JavaKeystore section. No code-behavior changes. No test changes. Verified locally: - gofmt / go vet clean. - go test -short ./internal/connector/target/ssh/... green. Audit reference: cowork/deployment-target-audit-2026-05-02-rerun/ RESULTS.md Top-10 fix #7. |
||
|
|
62f0a284be |
iis,wincertstore: default-deadline ctx wrapper for PowerShell exec calls
Closes Top-10 fix #4 of the 2026-05-02 deployment-target audit re-run (see cowork/deployment-target-audit-2026-05-02-rerun/ RESULTS.md). Pre-fix, both IIS and WinCertStore's realExecutor invoked PowerShell via exec.CommandContext(ctx, ...) and relied entirely on the caller's ctx to provide a deadline. If the caller forgot to attach one (context.Background() in a deeply-nested path; an operator running an ad-hoc deploy via a CLI that doesn't default-deadline its ctx), a hung WinRM session blocked the deploy worker thread indefinitely. S2 (failure isolation) bar from the audit: "does a hung WinRM take down the deploy worker pool?" — today's answer was "potentially yes" for these two connectors. Post-fix the answer is "no, capped at the configured ExecDeadline (default 60s)". This commit: 1. Adds Config.ExecDeadline (time.Duration, json: "exec_deadline") to both connectors, defaulted to 60 seconds. WinCertStore defaults via the existing applyDefaults helper; IIS defaults inline at New() and inside ValidateConfig (the IIS connector has no shared applyDefaults helper today; out-of-scope to refactor one in for this minor fix). Operators on slow Windows links can override via the JSON config field exec_deadline. 2. Wraps realExecutor.Execute with a fallback context.WithTimeout that fires ONLY when ctx has no deadline of its own. Caller- supplied deadlines always win — the wrapper is a safety net, not a hard cap. defer cancel() guards against goroutine leaks. 3. Tests: - TestIIS_RealExecutor_AttachesDefaultDeadlineWhenCallerHasNone (passes context.Background; asserts the call returns within 500ms with an error). On Linux/macOS runners powershell.exe is missing and exec.Cmd fails fast; on Windows the wrapper's ctx deadline cancels the running PowerShell process. Either path returns well under 500ms. - TestIIS_RealExecutor_RespectsCallerDeadlineWhenSet (10s fallback executor deadline, 50ms caller ctx; asserts caller deadline wins). - TestIIS_RealExecutor_NoDeadlineWiredWhenZero (deadline=0 means no fallback wrapper; caller's tight ctx still bounds). - TestIIS_New_DefaultsExecDeadlineTo60s + TestIIS_New_RespectsExplicitExecDeadline pin the constructor's defaulting behavior (uses winrm mode so the test doesn't need powershell.exe in PATH). - Same five tests in wincertstore_test.go. 4. docs/connectors.md IIS + WinCertStore sections document the new exec_deadline field with: what it is (per-PowerShell- subprocess cap), default (60 seconds), override semantics (caller ctx deadline wins). No change to behavior when the caller already attaches a deadline (the common case in production code paths). Tests using the mock executor (mockExecutor in iis_test.go / wincertstore_test.go) are unaffected — they bypass realExecutor entirely. S2 cross-cutting scorecard rating in cowork/deployment-target-audit-2026-05-02-rerun/findings.json flips from "gap" to "pass" for IIS and WinCertStore (in any future re-audit). Verified locally: - gofmt / go vet / staticcheck clean across both packages. - go test -race -count=1 ./internal/connector/target/iis/... ./internal/connector/target/wincertstore/... green. Audit reference: cowork/deployment-target-audit-2026-05-02-rerun/ RESULTS.md Top-10 fix #4. |
||
|
|
4142837cac |
iis,wincertstore,javakeystore: SHA-256 idempotency short-circuit
Closes Top-10 fix #3 of the 2026-05-02 deployment-target audit
re-run (see cowork/deployment-target-audit-2026-05-02-rerun/
RESULTS.md). Pre-fix, the three PowerShell-driven connectors
(IIS / WinCertStore / JavaKeystore) bypass internal/deploy.Apply
because they write to the Windows cert store / Java keystore via
PowerShell + keytool rather than the local filesystem. They don't
get deploy.Apply's SHA-256 idempotency short-circuit for free, so
every renewal triggers a full Remove+Import cycle even on byte-
identical material. Operators with 60-day rotation see unnecessary
cert-store / keystore churn, briefly bumping CPU and possibly
disrupting connections in flight.
This commit adds a per-connector idempotency probe modeled on
Bundle 9's Caddy api-mode SHA-256 short-circuit (commit
|
||
|
|
b8293653a5 |
postfix: add atomic-test variants for Mode=dovecot (happy path + verify-rollback)
Closes Bundle 11 of the 2026-05-02 deployment-target coverage audit
(see cowork/deployment-target-audit-2026-05-02/RESULTS.md). Pre-fix,
postfix_atomic_test.go exercised the atomic deploy path under Mode=
postfix only — the existing TestPostfix_DovecotMode at L233-246
asserted only the DeploymentID prefix, leaving applyDefaults's
dovecot-specific validate/reload command set + the rollback's
file-content-restoration unverified at the deploy-test layer.
Audit's only test-coverage gap on the otherwise-production-grade
Postfix/Dovecot connector.
This commit adds two new tests (test-only commit; no production-
code changes):
1. TestPostfix_Atomic_DovecotMode_HappyPath. Builds a Config with
Mode: "dovecot" and NO ValidateCommand / NO ReloadCommand set.
Calls ValidateConfig (which is what triggers applyDefaults via
its JSON-marshal-then-parse path) before DeployCertificate.
Captures the validate + reload commands threaded through the
SetTestRunValidate / SetTestRunReload hooks. Asserts:
- capturedValidateCmd contains "doveconf -n" (applyDefaults
populated it from the dovecot branch).
- capturedReloadCmd contains "doveadm reload".
- DeploymentID prefix "dovecot-" + result.Metadata["mode"] is
"dovecot" (Mode survived end-to-end).
2. TestPostfix_Atomic_DovecotMode_VerifyFails_Rollback. Pre-creates
cert.pem AND key.pem with known "ORIG-CERT" / "ORIG-KEY" bytes.
Builds Config with Mode: "dovecot", PostDeployVerify enabled
(Endpoint pointing at a dovecot-IMAPS-style :993 — value unused
by the probe stub), PostDeployVerifyAttempts: 1 (default is 3
attempts × 2s backoff = 4+ seconds; we don't need that for a
unit test). Probe stub returns Success: false, which
runPostDeployVerify wraps as "TLS probe failed: ...". Asserts:
- DeployCertificate returns error containing "TLS probe failed".
- cert.pem AND key.pem on disk contain the ORIG bytes
verbatim — Bundle 11's load-bearing assertion that the
rollback restored the pre-deploy file state under
Mode=dovecot. The existing TestPostfix_VerifyMismatch_Rollback
(Mode=postfix) only asserts the error; this test extends to
file-content restoration.
Existing TestPostfix_DovecotMode (L233-246) preserved as-is — the
minimal DeploymentID-prefix smoke test complements the new richer
tests without duplicating their scope.
The encoding/json import is added to support the HappyPath test's
json.Marshal call. No other dependency changes.
No production-code changes; the connector itself was already
correct for Mode=dovecot. Only the test pin was missing.
Verified locally:
- gofmt -l ./internal/connector/target/postfix/ clean
- go vet ./internal/connector/target/postfix/ clean
- go build ./cmd/agent/... clean (no signature changes)
- go test -race -count=1 ./internal/connector/target/postfix/ green
(24 tests total: 22 pre-existing + 2 new)
Audit reference: cowork/deployment-target-audit-2026-05-02/RESULTS.md
Bundle 11.
|
||
|
|
08a86d355d |
caddy: fix duration metric + file-mode PEM validate + api-mode idempotency
Closes Bundle 9 of the 2026-05-02 deployment-target coverage audit
(see cowork/deployment-target-audit-2026-05-02/RESULTS.md). Three
small independent fixes that share one connector file:
1. Duration metric (caddy.go L176). Pre-fix:
"duration_ms": fmt.Sprintf("%d", time.Since(time.Now()).Milliseconds())
This always returned ~0ms because time.Now() was called twice —
the second call captured a baseline immediately before time.Since
computed the delta. The intended baseline is `startTime` declared
at L113 and threaded through deployViaFile correctly. Post-fix:
"duration_ms": fmt.Sprintf("%d", time.Since(startTime).Milliseconds())
deployViaAPI's signature evolves to take startTime time.Time so
the api-mode path uses the same baseline as the file-mode path.
2. File-mode ValidateDeployment now validates PEM syntax. Pre-fix
(caddy.go L266-293) checked file existence only via os.Stat. A
cert file containing garbage bytes passed validation; Caddy's
file-watcher silently failed to load it; operators saw "validation
green" + "TLS handshake fails" with no obvious connection.
Post-fix: after the os.Stat checks succeed, os.ReadFile + parse
the first PEM block as an x509 cert via the shared
certutil.ParseCertificatePEM helper. Failure surfaces as
Valid=false with a clear "not valid PEM/x509" message.
3. API-mode idempotency short-circuit. Pre-fix, every deploy POSTed
to /config/apps/tls/certificates/load even when the active cert
was already what we wanted to deploy. Caddy reloads TLS state on
every POST, briefly bumping CPU and possibly disrupting connections
in flight. Post-fix: idempotencySkipPOST runs a GET first, parses
the response (handles BOTH the array-of-objects and single-object
shapes Caddy admin can return), SHA-256 compares the entry's
`cert` field to the deploy payload's cert bytes, and skips the
POST when match. Result.Metadata["idempotent"]="true" surfaces
the no-op. Conservative: any GET failure (network, non-200, parse
error, no matching entry, hash mismatch) silently falls through to
the POST, preserving today's behavior. Idempotency is a fast path,
not a correctness boundary — false negatives are safe; false
positives are dangerous.
Tests added to caddy_test.go (6 new tests, ~290 LOC):
- TestCaddy_API_DurationMetric_NonZero (httptest server with a 10ms
sleep in the POST handler; asserts duration_ms parses as int >= 5).
- TestCaddy_ValidateDeployment_FileMode_MalformedPEM_Rejected (writes
garbage to cert.pem; asserts Valid=false with PEM/x509 in message).
- TestCaddy_ValidateDeployment_FileMode_ValidPEM_Accepted (writes a
real ECDSA P-256 self-signed cert; asserts Valid=true).
- TestCaddy_API_Idempotent_SkipsPOSTWhenCertHashMatches (GET response
contains the same cert as the deploy payload; POST counter remains
0; metadata.idempotent=true; exactly 1 GET probe ran).
- TestCaddy_API_Idempotent_RunsPOSTWhenCertHashDiffers (GET response
contains a DIFFERENT cert; POST counter is 1; idempotent absent).
- TestCaddy_API_Idempotent_GETFails_FallsThroughToPOST (GET returns
500; POST still runs; deploy succeeds; idempotent absent).
Two existing tests updated to match the new contracts:
- TestCaddyConnector_DeployViaAPI_Success: mock handler now serves
BOTH GET (returns "[]" so the comparison falls through) and POST
(the original 200-OK path). The dispatch is a method-switch
inside the path-match branch.
- TestCaddyConnector_ValidateDeployment_Success: the placeholder
cert "MIIC..." used to pass the old existence-only check; post-Fix-2
it fails the PEM-parse check. Test now uses generateTestCertAndKey
to produce a real self-signed ECDSA P-256 cert.
generateTestCertAndKey helper added to the test file — same pattern
the javakeystore + wincertstore tests use, kept local because the
caddy package has no other test in the certutil family that would
make a shared helper cleaner.
Verified locally:
- gofmt -l ./internal/connector/target/caddy/ clean
- go vet ./internal/connector/target/caddy/ clean
- go build ./cmd/agent/... clean (factory wiring unchanged)
- go test -race -count=1 ./internal/connector/target/caddy/ green
(16 tests total: 11 pre-existing including the two updated +
6 new)
Audit reference: cowork/deployment-target-audit-2026-05-02/RESULTS.md
Bundle 9.
|