diff --git a/api/openapi.yaml b/api/openapi.yaml
index a205b54..5fc3b36 100644
--- a/api/openapi.yaml
+++ b/api/openapi.yaml
@@ -3326,6 +3326,7 @@ components:
DeploymentTarget:
type: object
+ required: [name, type, agent_id]
properties:
id:
type: string
@@ -3335,6 +3336,12 @@ components:
$ref: "#/components/schemas/TargetType"
agent_id:
type: string
+ description: |
+ ID of the agent that manages this target. Required because
+ deployment_targets.agent_id is a NOT NULL foreign key to agents(id)
+ (migration 000001). Empty or nonexistent agent IDs are rejected
+ with HTTP 400 by the service layer (see C-002 in the coverage-gap
+ audit).
config:
type: object
description: Target-specific configuration (varies by type)
diff --git a/internal/api/handler/certificate_handler_test.go b/internal/api/handler/certificate_handler_test.go
index 7da514f..55663e3 100644
--- a/internal/api/handler/certificate_handler_test.go
+++ b/internal/api/handler/certificate_handler_test.go
@@ -432,6 +432,66 @@ func TestCreateCertificate_ServiceError(t *testing.T) {
}
}
+// TestCreateCertificate_MissingRequiredField_Returns400 pins the C-001 handler
+// contract: handler MUST reject a create payload that omits any of the five
+// required fields (name, common_name, owner_id, team_id, issuer_id,
+// renewal_policy_id) with HTTP 400 before the service is invoked. The mock
+// service here would succeed if called; every subtest proving 400 therefore
+// proves the handler guard fires.
+func TestCreateCertificate_MissingRequiredField_Returns400(t *testing.T) {
+ baseBody := map[string]interface{}{
+ "name": "API Prod",
+ "common_name": "api.example.com",
+ "owner_id": "o-alice",
+ "team_id": "t-platform",
+ "issuer_id": "iss-local",
+ "renewal_policy_id": "rp-standard",
+ }
+
+ cases := []struct {
+ name string
+ missingField string
+ }{
+ {"missing name", "name"},
+ {"missing common_name", "common_name"},
+ {"missing owner_id", "owner_id"},
+ {"missing team_id", "team_id"},
+ {"missing issuer_id", "issuer_id"},
+ {"missing renewal_policy_id", "renewal_policy_id"},
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ body := make(map[string]interface{}, len(baseBody))
+ for k, v := range baseBody {
+ body[k] = v
+ }
+ delete(body, tc.missingField)
+ bodyBytes, _ := json.Marshal(body)
+
+ mock := &MockCertificateService{
+ CreateCertificateFn: func(_ context.Context, cert domain.ManagedCertificate) (*domain.ManagedCertificate, error) {
+ // Would succeed if handler guard did not fire.
+ cert.ID = "mc-would-be-created"
+ return &cert, nil
+ },
+ }
+ handler := NewCertificateHandler(mock)
+
+ req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates", bytes.NewReader(bodyBytes))
+ req = req.WithContext(contextWithRequestID())
+ req.Header.Set("Content-Type", "application/json")
+ w := httptest.NewRecorder()
+
+ handler.CreateCertificate(w, req)
+
+ if w.Code != http.StatusBadRequest {
+ t.Fatalf("%s: expected 400, got %d — body=%s", tc.name, w.Code, w.Body.String())
+ }
+ })
+ }
+}
+
// Test UpdateCertificate - success case
func TestUpdateCertificate_Success(t *testing.T) {
updated := &domain.ManagedCertificate{
diff --git a/internal/api/handler/target_handler_test.go b/internal/api/handler/target_handler_test.go
index 8d80f0c..756fdbf 100644
--- a/internal/api/handler/target_handler_test.go
+++ b/internal/api/handler/target_handler_test.go
@@ -10,6 +10,7 @@ import (
"time"
"github.com/shankar0123/certctl/internal/domain"
+ "github.com/shankar0123/certctl/internal/service"
)
// MockTargetService is a mock implementation of TargetService interface.
@@ -239,8 +240,9 @@ func TestCreateTarget_Success(t *testing.T) {
}
body := map[string]interface{}{
- "name": "New Target",
- "type": "nginx",
+ "name": "New Target",
+ "type": "nginx",
+ "agent_id": "agent-001",
}
bodyBytes, _ := json.Marshal(body)
@@ -258,7 +260,8 @@ func TestCreateTarget_Success(t *testing.T) {
func TestCreateTarget_MissingName(t *testing.T) {
body := map[string]interface{}{
- "type": "nginx",
+ "type": "nginx",
+ "agent_id": "agent-001",
}
bodyBytes, _ := json.Marshal(body)
@@ -276,7 +279,8 @@ func TestCreateTarget_MissingName(t *testing.T) {
func TestCreateTarget_MissingType(t *testing.T) {
body := map[string]interface{}{
- "name": "New Target",
+ "name": "New Target",
+ "agent_id": "agent-001",
}
bodyBytes, _ := json.Marshal(body)
@@ -311,8 +315,9 @@ func TestCreateTarget_NameTooLong(t *testing.T) {
longName += "x"
}
body := map[string]interface{}{
- "name": longName,
- "type": "nginx",
+ "name": longName,
+ "type": "nginx",
+ "agent_id": "agent-001",
}
bodyBytes, _ := json.Marshal(body)
@@ -340,6 +345,65 @@ func TestCreateTarget_MethodNotAllowed(t *testing.T) {
}
}
+// TestCreateTarget_MissingAgentID_Returns400 pins the C-002 handler contract:
+// handler MUST reject a create payload that omits agent_id with HTTP 400
+// before the service is invoked. Using a mock that would return 201-worthy
+// success proves the guard fires.
+func TestCreateTarget_MissingAgentID_Returns400(t *testing.T) {
+ body := map[string]interface{}{
+ "name": "New Target",
+ "type": "nginx",
+ // agent_id intentionally omitted
+ }
+ bodyBytes, _ := json.Marshal(body)
+
+ mock := &MockTargetService{
+ CreateTargetFn: func(_ context.Context, target domain.DeploymentTarget) (*domain.DeploymentTarget, error) {
+ // Would succeed if handler guard did not fire.
+ target.ID = "t-would-be-created"
+ return &target, nil
+ },
+ }
+ handler := NewTargetHandler(mock)
+ req := httptest.NewRequest(http.MethodPost, "/api/v1/targets", bytes.NewReader(bodyBytes))
+ req = req.WithContext(contextWithRequestID())
+ w := httptest.NewRecorder()
+
+ handler.CreateTarget(w, req)
+
+ if w.Code != http.StatusBadRequest {
+ t.Fatalf("expected 400, got %d — body=%s", w.Code, w.Body.String())
+ }
+}
+
+// TestCreateTarget_NonexistentAgent_Returns400 pins the C-002 handler↔service
+// translation: when the service returns service.ErrAgentNotFound, the handler
+// MUST map it to HTTP 400, not the generic 500 used for other service errors.
+func TestCreateTarget_NonexistentAgent_Returns400(t *testing.T) {
+ mock := &MockTargetService{
+ CreateTargetFn: func(_ context.Context, target domain.DeploymentTarget) (*domain.DeploymentTarget, error) {
+ return nil, service.ErrAgentNotFound
+ },
+ }
+ body := map[string]interface{}{
+ "name": "New Target",
+ "type": "nginx",
+ "agent_id": "agent-does-not-exist",
+ }
+ bodyBytes, _ := json.Marshal(body)
+
+ handler := NewTargetHandler(mock)
+ req := httptest.NewRequest(http.MethodPost, "/api/v1/targets", bytes.NewReader(bodyBytes))
+ req = req.WithContext(contextWithRequestID())
+ w := httptest.NewRecorder()
+
+ handler.CreateTarget(w, req)
+
+ if w.Code != http.StatusBadRequest {
+ t.Fatalf("expected 400 for nonexistent agent, got %d — body=%s", w.Code, w.Body.String())
+ }
+}
+
func TestUpdateTarget_Success(t *testing.T) {
now := time.Now()
mock := &MockTargetService{
diff --git a/internal/api/handler/targets.go b/internal/api/handler/targets.go
index a6e23db..d6d5b35 100644
--- a/internal/api/handler/targets.go
+++ b/internal/api/handler/targets.go
@@ -3,12 +3,14 @@ package handler
import (
"context"
"encoding/json"
+ "errors"
"net/http"
"strconv"
"strings"
"github.com/shankar0123/certctl/internal/api/middleware"
"github.com/shankar0123/certctl/internal/domain"
+ "github.com/shankar0123/certctl/internal/service"
)
// TargetService defines the service interface for deployment target operations.
@@ -125,9 +127,23 @@ func (h TargetHandler) CreateTarget(w http.ResponseWriter, r *http.Request) {
ErrorWithRequestID(w, http.StatusBadRequest, "type is required", requestID)
return
}
+ // C-002: agent_id is a NOT NULL FK in deployment_targets (migration 000001
+ // line 104). Reject empty values at the boundary so callers get a clean 400
+ // with the field name rather than a generic "Failed to create target" 500.
+ if err := ValidateRequired("agent_id", target.AgentID); err != nil {
+ ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID)
+ return
+ }
created, err := h.svc.CreateTarget(r.Context(), target)
if err != nil {
+ // C-002: a nonexistent agent_id is a client error, not a server error.
+ // The service returns ErrAgentNotFound (wrapped via fmt.Errorf %w) when
+ // agentRepo.Get fails; we translate that to 400 via errors.Is.
+ if errors.Is(err, service.ErrAgentNotFound) {
+ ErrorWithRequestID(w, http.StatusBadRequest, err.Error(), requestID)
+ return
+ }
ErrorWithRequestID(w, http.StatusInternalServerError, "Failed to create target", requestID)
return
}
diff --git a/internal/mcp/types.go b/internal/mcp/types.go
index 16e0503..d14c2fe 100644
--- a/internal/mcp/types.go
+++ b/internal/mcp/types.go
@@ -35,7 +35,7 @@ type CreateCertificateInput struct {
TeamID string `json:"team_id" jsonschema:"Team ID (required)"`
IssuerID string `json:"issuer_id" jsonschema:"Issuer connector ID"`
TargetIDs []string `json:"target_ids,omitempty" jsonschema:"Deployment target IDs"`
- RenewalPolicyID string `json:"renewal_policy_id,omitempty" jsonschema:"Renewal policy ID"`
+ RenewalPolicyID string `json:"renewal_policy_id" jsonschema:"Renewal policy ID (required)"`
ProfileID string `json:"certificate_profile_id,omitempty" jsonschema:"Certificate profile ID"`
Tags map[string]string `json:"tags,omitempty" jsonschema:"Key-value tags"`
}
@@ -112,7 +112,7 @@ type CreateTargetInput struct {
ID string `json:"id,omitempty" jsonschema:"Target ID"`
Name string `json:"name" jsonschema:"Target display name"`
Type string `json:"type" jsonschema:"Target type: NGINX, Apache, HAProxy, F5, IIS"`
- AgentID string `json:"agent_id,omitempty" jsonschema:"Agent ID that manages this target"`
+ AgentID string `json:"agent_id" jsonschema:"Agent ID that manages this target (required)"`
Config interface{} `json:"config,omitempty" jsonschema:"Target-specific configuration"`
Enabled bool `json:"enabled,omitempty" jsonschema:"Whether the target is enabled"`
}
diff --git a/internal/service/target.go b/internal/service/target.go
index 0b7f464..bfbcc9e 100644
--- a/internal/service/target.go
+++ b/internal/service/target.go
@@ -3,6 +3,7 @@ package service
import (
"context"
"encoding/json"
+ "errors"
"fmt"
"log/slog"
"time"
@@ -12,6 +13,13 @@ import (
"github.com/shankar0123/certctl/internal/repository"
)
+// ErrAgentNotFound is returned by [TargetService.CreateTarget] when the caller
+// references an agent_id that is empty or does not correspond to a registered
+// agent. The handler layer maps this to HTTP 400 via [errors.Is]. See C-002 in
+// cowork/certctl-coverage-gap-audit.md — this sentinel replaces a silent
+// Postgres FK violation (23503 → HTTP 500) with a deterministic 400.
+var ErrAgentNotFound = errors.New("referenced agent does not exist")
+
// validTargetTypes is the set of allowed target types for validation.
var validTargetTypes = map[domain.TargetType]bool{
domain.TargetTypeNGINX: true,
@@ -276,6 +284,19 @@ func (s *TargetService) CreateTarget(ctx context.Context, target domain.Deployme
if !isValidTargetType(target.Type) {
return nil, fmt.Errorf("unsupported target type: %s", target.Type)
}
+
+ // C-002: enforce agent_id FK at service layer so we return a clean 400
+ // instead of bubbling a Postgres 23503 foreign-key violation out as 500.
+ // The schema (migrations/000001 line 104) declares agent_id TEXT NOT NULL
+ // with a FK to agents(id); we mirror that contract here for deterministic
+ // error mapping.
+ if target.AgentID == "" {
+ return nil, fmt.Errorf("%w: agent_id is required", ErrAgentNotFound)
+ }
+ if _, err := s.agentRepo.Get(ctx, target.AgentID); err != nil {
+ return nil, fmt.Errorf("%w: %s", ErrAgentNotFound, target.AgentID)
+ }
+
if target.ID == "" {
target.ID = generateID("target")
}
diff --git a/internal/service/target_test.go b/internal/service/target_test.go
index 21e473e..cbca9fa 100644
--- a/internal/service/target_test.go
+++ b/internal/service/target_test.go
@@ -3,6 +3,7 @@ package service
import (
"context"
"encoding/json"
+ "errors"
"log/slog"
"os"
"testing"
@@ -377,11 +378,17 @@ func TestTargetService_GetTarget_Success(t *testing.T) {
}
func TestTargetService_CreateTarget_Success(t *testing.T) {
- svc, targetRepo, _, _ := newTestTargetService()
+ svc, targetRepo, _, agentRepo := newTestTargetService()
+
+ // C-002: CreateTarget now pre-validates agent_id against agentRepo. Seed a
+ // real agent so the happy path still exercises the normal creation flow
+ // without tripping the new ErrAgentNotFound guard.
+ agentRepo.AddAgent(&domain.Agent{ID: "a-1", Name: "test-agent"})
target := domain.DeploymentTarget{
- Name: "New Target",
- Type: domain.TargetTypeNGINX,
+ Name: "New Target",
+ Type: domain.TargetTypeNGINX,
+ AgentID: "a-1",
}
ctx := context.Background()
@@ -415,6 +422,53 @@ func TestTargetService_CreateTarget_InvalidType(t *testing.T) {
}
}
+// TestTargetService_CreateTarget_MissingAgentID verifies the C-002 service-layer
+// guard: an empty agent_id must be rejected with ErrAgentNotFound before the
+// repository layer is ever consulted. The handler maps this sentinel to HTTP
+// 400, so a 500 from a Postgres 23503 FK violation is never surfaced.
+func TestTargetService_CreateTarget_MissingAgentID(t *testing.T) {
+ svc, _, _, _ := newTestTargetService()
+
+ target := domain.DeploymentTarget{
+ Name: "No Agent",
+ Type: domain.TargetTypeNGINX,
+ // AgentID intentionally empty
+ }
+
+ ctx := context.Background()
+ _, err := svc.CreateTarget(ctx, target)
+ if err == nil {
+ t.Fatalf("expected error for missing agent_id, got nil")
+ }
+ if !errors.Is(err, ErrAgentNotFound) {
+ t.Errorf("expected errors.Is(err, ErrAgentNotFound) to be true, got err=%v", err)
+ }
+}
+
+// TestTargetService_CreateTarget_NonexistentAgentID verifies the second half of
+// the C-002 guard: a non-empty agent_id that does not resolve in agentRepo
+// still returns ErrAgentNotFound rather than letting the FK violation escape to
+// Postgres. This is the realistic failure mode for a GUI sending a stale
+// agent_id or a CLI caller with a typo.
+func TestTargetService_CreateTarget_NonexistentAgentID(t *testing.T) {
+ svc, _, _, _ := newTestTargetService()
+
+ target := domain.DeploymentTarget{
+ Name: "Bad Agent Ref",
+ Type: domain.TargetTypeNGINX,
+ AgentID: "a-does-not-exist",
+ }
+
+ ctx := context.Background()
+ _, err := svc.CreateTarget(ctx, target)
+ if err == nil {
+ t.Fatalf("expected error for nonexistent agent_id, got nil")
+ }
+ if !errors.Is(err, ErrAgentNotFound) {
+ t.Errorf("expected errors.Is(err, ErrAgentNotFound) to be true, got err=%v", err)
+ }
+}
+
func TestTargetService_UpdateTarget_Success(t *testing.T) {
svc, targetRepo, _, _ := newTestTargetService()
diff --git a/web/src/pages/CertificatesPage.tsx b/web/src/pages/CertificatesPage.tsx
index a864629..62e539f 100644
--- a/web/src/pages/CertificatesPage.tsx
+++ b/web/src/pages/CertificatesPage.tsx
@@ -1,7 +1,7 @@
import { useState } from 'react';
import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
import { useNavigate } from 'react-router-dom';
-import { getCertificates, createCertificate, triggerRenewal, revokeCertificate, updateCertificate, getOwners, getProfiles, getIssuers, bulkRevokeCertificates } from '../api/client';
+import { getCertificates, createCertificate, triggerRenewal, revokeCertificate, updateCertificate, getOwners, getTeams, getPolicies, getProfiles, getIssuers, bulkRevokeCertificates } from '../api/client';
import { REVOCATION_REASONS } from '../api/types';
import PageHeader from '../components/PageHeader';
import DataTable from '../components/DataTable';
@@ -35,8 +35,27 @@ function CreateCertificateModal({ onClose, onSuccess }: { onClose: () => void; o
queryKey: ['issuers'],
queryFn: () => getIssuers(),
});
+ // C-001: owner_id, team_id, and renewal_policy_id are required by the
+ // server (handler in internal/api/handler/certificates.go) and by OpenAPI.
+ // Load the catalog so the user selects valid FKs instead of typing free-text
+ // IDs that would 400 at the server.
+ const { data: ownersResp } = useQuery({
+ queryKey: ['owners', 'form'],
+ queryFn: () => getOwners({ per_page: '500' }),
+ });
+ const { data: teamsResp } = useQuery({
+ queryKey: ['teams', 'form'],
+ queryFn: () => getTeams({ per_page: '500' }),
+ });
+ const { data: policiesResp } = useQuery({
+ queryKey: ['renewal-policies', 'form'],
+ queryFn: () => getPolicies({ per_page: '500' }),
+ });
const profiles = profilesResp?.data || [];
const issuers = issuersResp?.data || [];
+ const owners = ownersResp?.data || [];
+ const teams = teamsResp?.data || [];
+ const policies = policiesResp?.data || [];
const selectedProfile = profiles.find(p => p.id === form.certificate_profile_id);
const ttlLabel = selectedProfile
@@ -143,24 +162,36 @@ function CreateCertificateModal({ onClose, onSuccess }: { onClose: () => void; o
-
- setForm(f => ({ ...f, renewal_policy_id: e.target.value }))}
- className={inputClass}
- placeholder="rp-standard" />
+
+
@@ -175,7 +206,15 @@ function CreateCertificateModal({ onClose, onSuccess }: { onClose: () => void; o