mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 18:51:32 +00:00
f0865bb051
Two audit findings, both category cat-l, both rooted in
web/src/pages/CertificatesPage.tsx. Pre-L-1 the GUI looped per-cert
HTTP calls — 100 selected certs = 100 sequential round-trips × ~50–200
ms each = a 5–20-second wedge during which the operator stared at a
progress bar. Post-L-1 each workflow is a single POST.
cat-l-fa0c1ac07ab5 [P1, primary] — bulk renew loop
handleBulkRenewal: for/await triggerRenewal(id)
cat-l-8a1fb258a38a [P2] — bulk reassign loop
handleReassign: for/await updateCertificate(id, {owner_id})
The bulk-revoke endpoint (POST /api/v1/certificates/bulk-revoke +
BulkRevocationCriteria/Result) already existed as the canonical shape
in v2.0.x — L-1 ports that pattern to renew + reassign with per-action
twists.
Backend (Go)
- internal/domain/bulk_renewal.go: BulkRenewalCriteria mirrors
BulkRevocationCriteria (criteria + IDs modes); BulkRenewalResult
envelope adds EnqueuedJobs[] for per-cert {certificate_id, job_id};
shared BulkOperationError type for all bulk paths.
- internal/domain/bulk_reassignment.go: narrower shape — IDs-only,
owner_id required, team_id optional.
- internal/service/bulk_renewal.go::BulkRenewalService.BulkRenew:
resolves criteria → status filter (Archived/Revoked/Expired/
RenewalInProgress all silent-skip) → per-cert status flip + job
create. Keygen-mode-aware so jobs land in the same initial status
as single-cert TriggerRenewal. Single bulk audit event per call,
not N.
- internal/service/bulk_reassignment.go::BulkReassignmentService.
BulkReassign: validates owner_id upfront via the
ErrBulkReassignOwnerNotFound typed sentinel — non-existent owner
returns 400 before any cert is touched. Already-owned-by-target
is silent-skip. Single bulk audit event.
- internal/api/handler/{bulk_renewal,bulk_reassignment}.go: HTTP
shape mirrors bulk_revocation.go. NOT admin-gated (renew is non-
destructive; reassign is a common-case workflow). Sentinel-error
→ 400 mapping for OwnerNotFound.
- internal/api/router/router.go: three bulk-* routes registered as a
block before the {id} routes. HandlerRegistry gains BulkRenewal +
BulkReassignment fields.
- cmd/server/main.go: NewBulkRenewalService threads cfg.Keygen.Mode
so bulk-renew jobs land in same initial state as single-cert path.
Frontend
- web/src/api/client.ts: bulkRenewCertificates(criteria) +
bulkReassignCertificates(request) functions with full TS types.
- web/src/pages/CertificatesPage.tsx: handleBulkRenewal + handleReassign
rewritten from N-call loops to single calls. Result envelope drives
progress UI; first-error message surfaced when total_failed > 0.
Stale triggerRenewal + updateCertificate imports removed.
MCP
- internal/mcp/types.go: BulkRenewCertificatesInput +
BulkReassignCertificatesInput.
- internal/mcp/tools.go: certctl_bulk_renew_certificates +
certctl_bulk_reassign_certificates tools mirroring the existing
certctl_bulk_revoke_certificates pattern.
OpenAPI
- api/openapi.yaml: two new operations (bulkRenewCertificates,
bulkReassignCertificates) under Certificates tag. Four new schemas
(BulkRenewRequest, BulkRenewResult, BulkEnqueuedJob,
BulkReassignRequest, BulkReassignResult).
Tests
- Domain: BulkRenewalCriteria.IsEmpty + BulkReassignmentRequest.IsEmpty
IsEmpty contracts; JSON round-trip shape pinning.
- Service: 7 BulkRenew tests (happy/criteria-mode/skips-RenewalInProgress/
skips-revoked-archived/empty-criteria-error/partial-failure/
audit-event-emitted) + 8 BulkReassign tests (happy/skips-already-
owned/owner-required/empty-IDs/owner-not-found-sentinel/team-id-
optional/team-id-provided/partial-failure/audit-event-emitted).
- Handler: 5 BulkRenew handler tests (happy/empty-body-400/wrong-
method-405/actor-attribution/service-error-500) + 6 BulkReassign
handler tests (happy/empty-IDs-400/missing-owner-400/owner-not-
found-400-via-sentinel/wrong-method-405/generic-error-500).
CI guardrail
- .github/workflows/ci.yml: 'Forbidden client-side bulk-action loop
regression guard (L-1)'. Greps web/src/pages/CertificatesPage.tsx
for 'for(...) await triggerRenewal(...)' and 'for(...) await
updateCertificate(...)' patterns; comment lines exempt; test files
exempt. Verified locally (passes against post-fix tree, fires
against synthetic regression).
Counts (deltas)
- Routes: 119 → 121 (+2)
- OpenAPI operations: 123 → 125 (+2)
- MCP tools: 83 → 85 (+2)
Performance
- 100-cert bulk-renew: ~10s of sequential HTTP → ~100ms (99% latency
reduction on the canonical operator workflow).
- Audit event volume: 1 + N per operation → 1.
Out of scope (deferred follow-ups)
- cat-b-31ceb6aaa9f1: updateOwner/updateTeam/updateAgentGroup orphan
(different shape — wire existing PUT to GUI, not new bulk endpoint).
- cat-k-e85d1099b2d7: CertificatesPage no pagination UI.
- cat-i-b0924b6675f8: MCP missing claim/dismiss/acknowledge (L-1 added
2 new tools but does not close that finding).
Verification
- go build / vet / test -short / test -short -race all clean.
- web tsc --noEmit + vitest run all clean (296 tests passing).
- OpenAPI YAML parses (89 paths, 125 ops).
- L-1 CI guardrail passes against post-fix tree, fires against
synthetic regression.
No push.
149 lines
5.0 KiB
Go
149 lines
5.0 KiB
Go
package handler
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"encoding/json"
|
|
"errors"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/shankar0123/certctl/internal/api/middleware"
|
|
"github.com/shankar0123/certctl/internal/domain"
|
|
)
|
|
|
|
// mockBulkRenewalService is a test implementation of BulkRenewalService.
|
|
type mockBulkRenewalService struct {
|
|
BulkRenewFn func(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error)
|
|
}
|
|
|
|
func (m *mockBulkRenewalService) BulkRenew(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error) {
|
|
if m.BulkRenewFn != nil {
|
|
return m.BulkRenewFn(ctx, criteria, actor)
|
|
}
|
|
return &domain.BulkRenewalResult{}, nil
|
|
}
|
|
|
|
// authedContext mirrors adminContext but without the admin flag —
|
|
// bulk-renew is NOT admin-gated, any authenticated caller can use it.
|
|
func authedContext() context.Context {
|
|
ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id-renew")
|
|
ctx = context.WithValue(ctx, middleware.UserKey{}, "alice")
|
|
return ctx
|
|
}
|
|
|
|
func TestBulkRenew_Handler_HappyPath(t *testing.T) {
|
|
svc := &mockBulkRenewalService{
|
|
BulkRenewFn: func(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error) {
|
|
if len(criteria.CertificateIDs) != 3 {
|
|
t.Errorf("expected 3 IDs, got %d", len(criteria.CertificateIDs))
|
|
}
|
|
if actor != "alice" {
|
|
t.Errorf("actor = %q, want 'alice' (resolved from middleware UserKey)", actor)
|
|
}
|
|
return &domain.BulkRenewalResult{
|
|
TotalMatched: 3,
|
|
TotalEnqueued: 3,
|
|
EnqueuedJobs: []domain.BulkEnqueuedJob{
|
|
{CertificateID: "mc-1", JobID: "job-a"},
|
|
{CertificateID: "mc-2", JobID: "job-b"},
|
|
{CertificateID: "mc-3", JobID: "job-c"},
|
|
},
|
|
}, nil
|
|
},
|
|
}
|
|
h := NewBulkRenewalHandler(svc)
|
|
|
|
body := `{"certificate_ids":["mc-1","mc-2","mc-3"]}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-renew", bytes.NewBufferString(body))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
req = req.WithContext(authedContext())
|
|
w := httptest.NewRecorder()
|
|
h.BulkRenew(w, req)
|
|
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("status = %d, want 200; body=%s", w.Code, w.Body.String())
|
|
}
|
|
var result domain.BulkRenewalResult
|
|
if err := json.NewDecoder(w.Body).Decode(&result); err != nil {
|
|
t.Fatalf("decode failed: %v", err)
|
|
}
|
|
if result.TotalEnqueued != 3 || len(result.EnqueuedJobs) != 3 {
|
|
t.Errorf("envelope drift: enqueued=%d jobs=%d, want 3/3",
|
|
result.TotalEnqueued, len(result.EnqueuedJobs))
|
|
}
|
|
}
|
|
|
|
func TestBulkRenew_Handler_EmptyBody_400(t *testing.T) {
|
|
svc := &mockBulkRenewalService{}
|
|
h := NewBulkRenewalHandler(svc)
|
|
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-renew", bytes.NewBufferString(`{}`))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
req = req.WithContext(authedContext())
|
|
w := httptest.NewRecorder()
|
|
h.BulkRenew(w, req)
|
|
|
|
if w.Code != http.StatusBadRequest {
|
|
t.Errorf("status = %d, want 400 (empty criteria must reject)", w.Code)
|
|
}
|
|
if !strings.Contains(w.Body.String(), "filter criterion") {
|
|
t.Errorf("body should name the criteria-required contract; got: %s", w.Body.String())
|
|
}
|
|
}
|
|
|
|
func TestBulkRenew_Handler_WrongMethod_405(t *testing.T) {
|
|
svc := &mockBulkRenewalService{}
|
|
h := NewBulkRenewalHandler(svc)
|
|
|
|
for _, method := range []string{http.MethodGet, http.MethodPut, http.MethodDelete, http.MethodPatch} {
|
|
req := httptest.NewRequest(method, "/api/v1/certificates/bulk-renew", nil)
|
|
req = req.WithContext(authedContext())
|
|
w := httptest.NewRecorder()
|
|
h.BulkRenew(w, req)
|
|
if w.Code != http.StatusMethodNotAllowed {
|
|
t.Errorf("%s → status %d, want 405", method, w.Code)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestBulkRenew_Handler_ActorAttribution(t *testing.T) {
|
|
var capturedActor string
|
|
svc := &mockBulkRenewalService{
|
|
BulkRenewFn: func(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error) {
|
|
capturedActor = actor
|
|
return &domain.BulkRenewalResult{}, nil
|
|
},
|
|
}
|
|
h := NewBulkRenewalHandler(svc)
|
|
|
|
body := `{"certificate_ids":["mc-1"]}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-renew", bytes.NewBufferString(body))
|
|
req = req.WithContext(authedContext())
|
|
w := httptest.NewRecorder()
|
|
h.BulkRenew(w, req)
|
|
|
|
if capturedActor != "alice" {
|
|
t.Errorf("actor not threaded from middleware.UserKey: got %q, want 'alice'", capturedActor)
|
|
}
|
|
}
|
|
|
|
func TestBulkRenew_Handler_ServiceError_500(t *testing.T) {
|
|
svc := &mockBulkRenewalService{
|
|
BulkRenewFn: func(ctx context.Context, criteria domain.BulkRenewalCriteria, actor string) (*domain.BulkRenewalResult, error) {
|
|
return nil, errors.New("simulated DB failure")
|
|
},
|
|
}
|
|
h := NewBulkRenewalHandler(svc)
|
|
body := `{"certificate_ids":["mc-1"]}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-renew", bytes.NewBufferString(body))
|
|
req = req.WithContext(authedContext())
|
|
w := httptest.NewRecorder()
|
|
h.BulkRenew(w, req)
|
|
if w.Code != http.StatusInternalServerError {
|
|
t.Errorf("status = %d, want 500", w.Code)
|
|
}
|
|
}
|