mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-10 12:58:53 +00:00
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.
This commit is contained in:
@@ -88,12 +88,105 @@ func TestClient_RenewCertificate(t *testing.T) {
|
||||
defer server.Close()
|
||||
|
||||
client, _ := NewClient(server.URL, "", "table", "", false)
|
||||
err := client.RenewCertificate("mc-1")
|
||||
err := client.RenewCertificate("mc-1", false)
|
||||
if err != nil {
|
||||
t.Fatalf("RenewCertificate failed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestClient_RenewCertificate_ForceFlag pins the 2026-05-05 parity-defaults-
|
||||
// cleanup (P3-1) wire: the CLI sends `?force=true` on the renew URL when
|
||||
// the operator passes --force. The pre-2026-05-05 hardcoded `force=false`
|
||||
// body field is gone (the API never read it — it was a "lying field").
|
||||
func TestClient_RenewCertificate_ForceFlag(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
force bool
|
||||
wantQuery string
|
||||
}{
|
||||
{"no-force", false, ""},
|
||||
{"force-true", true, "force=true"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
var gotQuery string
|
||||
var gotBody string
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
gotQuery = r.URL.RawQuery
|
||||
buf := make([]byte, 1024)
|
||||
n, _ := r.Body.Read(buf)
|
||||
gotBody = string(buf[:n])
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(map[string]interface{}{"job_id": "job-1"})
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client, _ := NewClient(server.URL, "", "table", "", false)
|
||||
if err := client.RenewCertificate("mc-1", tc.force); err != nil {
|
||||
t.Fatalf("RenewCertificate: %v", err)
|
||||
}
|
||||
if gotQuery != tc.wantQuery {
|
||||
t.Errorf("query: got %q want %q", gotQuery, tc.wantQuery)
|
||||
}
|
||||
// Body must be empty — the lying `force=false` field is gone.
|
||||
if gotBody != "" {
|
||||
t.Errorf("body should be empty (no lying force field), got %q", gotBody)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestNormalizeRevokeReason pins the 2026-05-05 parity-defaults-cleanup
|
||||
// (P3-2) reason-code validator: canonical camelCase passes through, snake_
|
||||
// case normalises to camelCase, anything else returns ok=false.
|
||||
func TestNormalizeRevokeReason(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
in string
|
||||
want string
|
||||
wantOK bool
|
||||
}{
|
||||
{"keyCompromise", "keyCompromise", true},
|
||||
{"key_compromise", "keyCompromise", true},
|
||||
{"superseded", "superseded", true},
|
||||
{"cessation_of_operation", "cessationOfOperation", true},
|
||||
{"unspecified", "unspecified", true},
|
||||
{"BogusReason", "BogusReason", false},
|
||||
{"", "", false},
|
||||
{"key compromise", "key compromise", false},
|
||||
} {
|
||||
t.Run(tc.in, func(t *testing.T) {
|
||||
got, ok := NormalizeRevokeReason(tc.in)
|
||||
if got != tc.want || ok != tc.wantOK {
|
||||
t.Errorf("NormalizeRevokeReason(%q) = (%q, %v), want (%q, %v)",
|
||||
tc.in, got, ok, tc.want, tc.wantOK)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidRevokeReasons pins that the canonical RFC 5280 §5.3.1 reason
|
||||
// list is non-empty and contains the operator-critical entries (the help
|
||||
// menu printed when --reason is missing depends on this).
|
||||
func TestValidRevokeReasons(t *testing.T) {
|
||||
got := ValidRevokeReasons()
|
||||
if len(got) < 9 {
|
||||
t.Errorf("expected ≥9 RFC 5280 §5.3.1 reasons, got %d: %v", len(got), got)
|
||||
}
|
||||
want := []string{"keyCompromise", "caCompromise", "superseded",
|
||||
"cessationOfOperation", "certificateHold", "privilegeWithdrawn"}
|
||||
for _, w := range want {
|
||||
found := false
|
||||
for _, g := range got {
|
||||
if g == w {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Errorf("missing canonical reason %q in %v", w, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestClient_RevokeCertificate(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != "POST" || r.URL.Path != "/api/v1/certificates/mc-1/revoke" {
|
||||
|
||||
Reference in New Issue
Block a user