feat(M35): dynamic target configuration with encrypted config, test connection, and GUI updates

Mirror M34's dynamic issuer config pattern for deployment targets: AES-256-GCM
encrypted config storage, sensitive field redaction in API responses, agent
heartbeat-based test connection endpoint, and full frontend updates including
test status indicators, source badges, and removal of stale hostname/status
fields from the Target interface.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

internal/service/concurrent_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"os"
 	"sync"
 	"testing"
 
@@ -193,7 +194,7 @@ func TestConcurrentTargetCRUD(t *testing.T) {
 		Targets: make(map[string]*domain.DeploymentTarget),
 	}
 
-	targetSvc := NewTargetService(mockTargetRepo, nil)
+	targetSvc := NewTargetService(mockTargetRepo, nil, nil, nil, slog.New(slog.NewTextHandler(os.Stderr, nil)))
 
 	var mu sync.Mutex
 	createdTargets := make([]string, 0)
@@ -402,7 +403,7 @@ func TestConcurrentMixedOperations(t *testing.T) {
 	// Setup services
 	auditSvc := &AuditService{auditRepo: mockAuditRepo}
 	certSvc := NewCertificateService(mockCertRepo, nil, auditSvc)
-	targetSvc := NewTargetService(mockTargetRepo, auditSvc)
+	targetSvc := NewTargetService(mockTargetRepo, auditSvc, nil, nil, slog.New(slog.NewTextHandler(os.Stderr, nil)))
 
 	var wg sync.WaitGroup
 	errChan := make(chan error, 30)

internal/service/config_helpers.go

@@ -0,0 +1,42 @@
+package service
+
+import (
+	"encoding/json"
+	"strings"
+)
+
+// sensitiveKeys are config key substrings that should be redacted in API responses.
+var sensitiveKeys = []string{"password", "secret", "token", "key", "hmac", "private", "credentials"}
+
+// isSensitiveConfigKey checks if a config key contains sensitive substrings.
+func isSensitiveConfigKey(key string) bool {
+	lower := strings.ToLower(key)
+	for _, s := range sensitiveKeys {
+		if strings.Contains(lower, s) {
+			return true
+		}
+	}
+	return false
+}
+
+// redactConfigJSON replaces sensitive values in a JSON config with "********".
+func redactConfigJSON(configJSON json.RawMessage) json.RawMessage {
+	var m map[string]interface{}
+	if err := json.Unmarshal(configJSON, &m); err != nil {
+		return configJSON // Not a JSON object, return as-is
+	}
+
+	for k, v := range m {
+		if isSensitiveConfigKey(k) {
+			if str, ok := v.(string); ok && str != "" {
+				m[k] = "********"
+			}
+		}
+	}
+
+	redacted, err := json.Marshal(m)
+	if err != nil {
+		return configJSON
+	}
+	return json.RawMessage(redacted)
+}

internal/service/context_test.go

@@ -3,6 +3,7 @@ package service
 import (
 	"context"
 	"log/slog"
+	"os"
 	"testing"
 	"time"
 
@@ -141,7 +142,7 @@ func TestTargetService_ListWithCancelledContext(t *testing.T) {
 	mockTargetRepo := &mockTargetRepo{
 		Targets: make(map[string]*domain.DeploymentTarget),
 	}
-	targetSvc := NewTargetService(mockTargetRepo, nil)
+	targetSvc := NewTargetService(mockTargetRepo, nil, nil, nil, slog.New(slog.NewTextHandler(os.Stderr, nil)))
 
 	_, _, err := targetSvc.List(ctx, 1, 50)
 

internal/service/issuer.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"log/slog"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/shankar0123/certctl/internal/config"
@@ -16,9 +15,6 @@ import (
 	"github.com/shankar0123/certctl/internal/repository"
 )
 
-// sensitiveKeys are config key substrings that should be redacted in API responses.
-var sensitiveKeys = []string{"password", "secret", "token", "key", "hmac", "private", "credentials"}
-
 // IssuerService provides business logic for certificate issuer management.
 type IssuerService struct {
 	issuerRepo    repository.IssuerRepository
@@ -703,39 +699,6 @@ func (s *IssuerService) updateTestStatus(ctx context.Context, iss *domain.Issuer
 	}
 }
 
-// redactConfigJSON replaces sensitive values in a JSON config with "********".
-func redactConfigJSON(configJSON json.RawMessage) json.RawMessage {
-	var m map[string]interface{}
-	if err := json.Unmarshal(configJSON, &m); err != nil {
-		return configJSON // Not a JSON object, return as-is
-	}
-
-	for k, v := range m {
-		if isSensitiveConfigKey(k) {
-			if str, ok := v.(string); ok && str != "" {
-				m[k] = "********"
-			}
-		}
-	}
-
-	redacted, err := json.Marshal(m)
-	if err != nil {
-		return configJSON
-	}
-	return json.RawMessage(redacted)
-}
-
-// isSensitiveConfigKey checks if a config key contains sensitive substrings.
-func isSensitiveConfigKey(key string) bool {
-	lower := strings.ToLower(key)
-	for _, s := range sensitiveKeys {
-		if strings.Contains(lower, s) {
-			return true
-		}
-	}
-	return false
-}
-
 // getEnvForSeed reads an environment variable for seed data construction.
 func getEnvForSeed(key string) string {
 	return os.Getenv(key)

internal/service/target.go

@@ -2,28 +2,58 @@ package service
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"log/slog"
 	"time"
 
+	"github.com/shankar0123/certctl/internal/crypto"
 	"github.com/shankar0123/certctl/internal/domain"
 	"github.com/shankar0123/certctl/internal/repository"
 )
 
+// validTargetTypes is the set of allowed target types for validation.
+var validTargetTypes = map[domain.TargetType]bool{
+	domain.TargetTypeNGINX:   true,
+	domain.TargetTypeApache:  true,
+	domain.TargetTypeHAProxy: true,
+	domain.TargetTypeF5:      true,
+	domain.TargetTypeIIS:     true,
+	domain.TargetTypeTraefik: true,
+	domain.TargetTypeCaddy:   true,
+	domain.TargetTypeEnvoy:   true,
+	domain.TargetTypePostfix: true,
+	domain.TargetTypeDovecot: true,
+}
+
+// isValidTargetType checks if a type string is a known target type.
+func isValidTargetType(t domain.TargetType) bool {
+	return validTargetTypes[t]
+}
+
 // TargetService provides business logic for deployment target management.
 type TargetService struct {
-	targetRepo   repository.TargetRepository
-	auditService *AuditService
+	targetRepo    repository.TargetRepository
+	agentRepo     repository.AgentRepository
+	auditService  *AuditService
+	encryptionKey []byte
+	logger        *slog.Logger
 }
 
 // NewTargetService creates a new target service.
 func NewTargetService(
 	targetRepo repository.TargetRepository,
 	auditService *AuditService,
+	agentRepo repository.AgentRepository,
+	encryptionKey []byte,
+	logger *slog.Logger,
 ) *TargetService {
 	return &TargetService{
-		targetRepo:   targetRepo,
-		auditService: auditService,
+		targetRepo:    targetRepo,
+		agentRepo:     agentRepo,
+		auditService:  auditService,
+		encryptionKey: encryptionKey,
+		logger:        logger,
 	}
 }
 
@@ -61,11 +91,14 @@ func (s *TargetService) Get(ctx context.Context, id string) (*domain.DeploymentT
 	return target, nil
 }
 
-// Create validates and stores a new deployment target.
+// Create validates and stores a new deployment target, encrypting sensitive config.
 func (s *TargetService) Create(ctx context.Context, target *domain.DeploymentTarget, actor string) error {
 	if target.Name == "" {
 		return fmt.Errorf("target name is required")
 	}
+	if !isValidTargetType(target.Type) {
+		return fmt.Errorf("unsupported target type: %s", target.Type)
+	}
 
 	if target.ID == "" {
 		target.ID = generateID("target")
@@ -77,33 +110,68 @@ func (s *TargetService) Create(ctx context.Context, target *domain.DeploymentTar
 	if target.UpdatedAt.IsZero() {
 		target.UpdatedAt = now
 	}
+	if target.TestStatus == "" {
+		target.TestStatus = "untested"
+	}
+	if target.Source == "" {
+		target.Source = "database"
+	}
+
+	// Encrypt the full config and store redacted version in config column
+	if len(target.Config) > 0 {
+		encrypted, _, err := crypto.EncryptIfKeySet([]byte(target.Config), s.encryptionKey)
+		if err != nil {
+			return fmt.Errorf("failed to encrypt config: %w", err)
+		}
+		target.EncryptedConfig = encrypted
+		target.Config = redactConfigJSON(target.Config)
+	}
+
 	if err := s.targetRepo.Create(ctx, target); err != nil {
 		return fmt.Errorf("failed to create target: %w", err)
 	}
 
 	if s.auditService != nil {
 		if auditErr := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "create_target", "target", target.ID, nil); auditErr != nil {
-			slog.Error("failed to record audit event", "error", auditErr)
+			s.logger.Error("failed to record audit event", "error", auditErr)
 		}
 	}
 
 	return nil
 }
 
-// Update modifies an existing deployment target.
+// Update modifies an existing deployment target. Handles "********" preservation for sensitive fields.
 func (s *TargetService) Update(ctx context.Context, id string, target *domain.DeploymentTarget, actor string) error {
 	if target.Name == "" {
 		return fmt.Errorf("target name is required")
 	}
 
 	target.ID = id
+	target.UpdatedAt = time.Now()
+
+	// If config contains "********" values, merge with existing decrypted config
+	if len(target.Config) > 0 {
+		mergedConfig, err := s.mergeRedactedConfig(ctx, id, target.Config)
+		if err != nil {
+			return fmt.Errorf("failed to merge config: %w", err)
+		}
+
+		// Encrypt the merged config
+		encrypted, _, encErr := crypto.EncryptIfKeySet(mergedConfig, s.encryptionKey)
+		if encErr != nil {
+			return fmt.Errorf("failed to encrypt config: %w", encErr)
+		}
+		target.EncryptedConfig = encrypted
+		target.Config = redactConfigJSON(json.RawMessage(mergedConfig))
+	}
+
 	if err := s.targetRepo.Update(ctx, target); err != nil {
 		return fmt.Errorf("failed to update target %s: %w", id, err)
 	}
 
 	if s.auditService != nil {
 		if auditErr := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "update_target", "target", id, nil); auditErr != nil {
-			slog.Error("failed to record audit event", "error", auditErr)
+			s.logger.Error("failed to record audit event", "error", auditErr)
 		}
 	}
 
@@ -118,13 +186,50 @@ func (s *TargetService) Delete(ctx context.Context, id string, actor string) err
 
 	if s.auditService != nil {
 		if auditErr := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "delete_target", "target", id, nil); auditErr != nil {
-			slog.Error("failed to record audit event", "error", auditErr)
+			s.logger.Error("failed to record audit event", "error", auditErr)
 		}
 	}
 
 	return nil
 }
 
+// TestConnection tests a target's connectivity by checking the assigned agent's heartbeat status.
+// Target connectors run on agents, not on the server, so we can't instantiate a connector here.
+// Instead, we verify the agent is online and reachable.
+func (s *TargetService) TestConnection(ctx context.Context, id string) error {
+	target, err := s.targetRepo.Get(ctx, id)
+	if err != nil {
+		return fmt.Errorf("target not found: %w", err)
+	}
+
+	if target.AgentID == "" {
+		s.updateTestStatus(ctx, target, "failed")
+		return fmt.Errorf("target has no assigned agent")
+	}
+
+	agent, err := s.agentRepo.Get(ctx, target.AgentID)
+	if err != nil {
+		s.updateTestStatus(ctx, target, "failed")
+		return fmt.Errorf("assigned agent not found: %w", err)
+	}
+
+	if agent.Status != domain.AgentStatusOnline {
+		s.updateTestStatus(ctx, target, "failed")
+		return fmt.Errorf("assigned agent %s is %s (expected Online)", agent.ID, agent.Status)
+	}
+
+	// Check heartbeat freshness (agent must have heartbeated within the last 5 minutes)
+	if agent.LastHeartbeatAt != nil {
+		if time.Since(*agent.LastHeartbeatAt) > 5*time.Minute {
+			s.updateTestStatus(ctx, target, "failed")
+			return fmt.Errorf("assigned agent %s last heartbeat was %s ago (stale)", agent.ID, time.Since(*agent.LastHeartbeatAt).Round(time.Second))
+		}
+	}
+
+	s.updateTestStatus(ctx, target, "success")
+	return nil
+}
+
 // ListTargets returns paginated targets (handler interface method).
 func (s *TargetService) ListTargets(page, perPage int) ([]domain.DeploymentTarget, int64, error) {
 	if page < 1 {
@@ -157,6 +262,9 @@ func (s *TargetService) GetTarget(id string) (*domain.DeploymentTarget, error) {
 
 // CreateTarget creates a new target (handler interface method).
 func (s *TargetService) CreateTarget(target domain.DeploymentTarget) (*domain.DeploymentTarget, error) {
+	if !isValidTargetType(target.Type) {
+		return nil, fmt.Errorf("unsupported target type: %s", target.Type)
+	}
 	if target.ID == "" {
 		target.ID = generateID("target")
 	}
@@ -167,6 +275,23 @@ func (s *TargetService) CreateTarget(target domain.DeploymentTarget) (*domain.De
 	if target.UpdatedAt.IsZero() {
 		target.UpdatedAt = now
 	}
+	if target.TestStatus == "" {
+		target.TestStatus = "untested"
+	}
+	if target.Source == "" {
+		target.Source = "database"
+	}
+
+	// Encrypt config
+	if len(target.Config) > 0 {
+		encrypted, _, err := crypto.EncryptIfKeySet([]byte(target.Config), s.encryptionKey)
+		if err != nil {
+			return nil, fmt.Errorf("failed to encrypt config: %w", err)
+		}
+		target.EncryptedConfig = encrypted
+		target.Config = redactConfigJSON(target.Config)
+	}
+
 	if err := s.targetRepo.Create(context.Background(), &target); err != nil {
 		return nil, fmt.Errorf("failed to create target: %w", err)
 	}
@@ -176,6 +301,23 @@ func (s *TargetService) CreateTarget(target domain.DeploymentTarget) (*domain.De
 // UpdateTarget modifies a target (handler interface method).
 func (s *TargetService) UpdateTarget(id string, target domain.DeploymentTarget) (*domain.DeploymentTarget, error) {
 	target.ID = id
+	target.UpdatedAt = time.Now()
+
+	// Merge redacted fields with existing config
+	if len(target.Config) > 0 {
+		mergedConfig, err := s.mergeRedactedConfig(context.Background(), id, target.Config)
+		if err != nil {
+			return nil, fmt.Errorf("failed to merge config: %w", err)
+		}
+
+		encrypted, _, encErr := crypto.EncryptIfKeySet(mergedConfig, s.encryptionKey)
+		if encErr != nil {
+			return nil, fmt.Errorf("failed to encrypt config: %w", encErr)
+		}
+		target.EncryptedConfig = encrypted
+		target.Config = redactConfigJSON(json.RawMessage(mergedConfig))
+	}
+
 	if err := s.targetRepo.Update(context.Background(), &target); err != nil {
 		return nil, fmt.Errorf("failed to update target: %w", err)
 	}
@@ -186,3 +328,100 @@ func (s *TargetService) UpdateTarget(id string, target domain.DeploymentTarget)
 func (s *TargetService) DeleteTarget(id string) error {
 	return s.targetRepo.Delete(context.Background(), id)
 }
+
+// TestTargetConnection tests target connectivity (handler interface method).
+func (s *TargetService) TestTargetConnection(id string) error {
+	return s.TestConnection(context.Background(), id)
+}
+
+// --- Internal helpers ---
+
+// getDecryptedConfig returns the decrypted config JSON for a target.
+func (s *TargetService) getDecryptedConfig(target *domain.DeploymentTarget) (json.RawMessage, error) {
+	if len(target.EncryptedConfig) > 0 {
+		decrypted, err := crypto.DecryptIfKeySet(target.EncryptedConfig, s.encryptionKey)
+		if err != nil {
+			return nil, err
+		}
+		return json.RawMessage(decrypted), nil
+	}
+	if len(target.Config) > 0 {
+		return target.Config, nil
+	}
+	return json.RawMessage("{}"), nil
+}
+
+// mergeRedactedConfig merges incoming config (which may have "********" values)
+// with the existing decrypted config so sensitive fields are preserved.
+func (s *TargetService) mergeRedactedConfig(ctx context.Context, id string, incoming json.RawMessage) ([]byte, error) {
+	// Parse incoming config
+	var incomingMap map[string]interface{}
+	if err := json.Unmarshal(incoming, &incomingMap); err != nil {
+		s.logger.Warn("mergeRedactedConfig: incoming config is not a JSON object, using as-is", "target", id, "error", err)
+		return incoming, nil
+	}
+
+	// Check if any values are "********"
+	hasRedacted := false
+	for _, v := range incomingMap {
+		if str, ok := v.(string); ok && str == "********" {
+			hasRedacted = true
+			break
+		}
+	}
+
+	if !hasRedacted {
+		return incoming, nil // No redacted values, use incoming as-is
+	}
+
+	// Load existing target to get real values
+	existing, err := s.targetRepo.Get(ctx, id)
+	if err != nil {
+		s.logger.Warn("mergeRedactedConfig: could not load existing target, redacted values will be lost", "target", id, "error", err)
+		return incoming, nil
+	}
+
+	existingConfig, err := s.getDecryptedConfig(existing)
+	if err != nil {
+		s.logger.Warn("mergeRedactedConfig: could not decrypt existing config, redacted values will be lost", "target", id, "error", err)
+		return incoming, nil
+	}
+
+	var existingMap map[string]interface{}
+	if err := json.Unmarshal(existingConfig, &existingMap); err != nil {
+		s.logger.Warn("mergeRedactedConfig: existing config is not a JSON object, redacted values will be lost", "target", id, "error", err)
+		return incoming, nil
+	}
+
+	// Merge: for each "********" value in incoming, use existing value
+	for k, v := range incomingMap {
+		if str, ok := v.(string); ok && str == "********" {
+			if existingVal, exists := existingMap[k]; exists {
+				incomingMap[k] = existingVal
+			}
+		}
+	}
+
+	return json.Marshal(incomingMap)
+}
+
+// updateTestStatus updates the test_status and last_tested_at fields in the database
+// and records an audit event.
+func (s *TargetService) updateTestStatus(ctx context.Context, target *domain.DeploymentTarget, status string) {
+	now := time.Now()
+	target.TestStatus = status
+	target.LastTestedAt = &now
+	target.UpdatedAt = now
+	if err := s.targetRepo.Update(ctx, target); err != nil {
+		s.logger.Error("failed to update test status", "target", target.ID, "status", status, "error", err)
+	}
+
+	// Record audit event for connection test
+	if s.auditService != nil {
+		action := "target_test_connection_" + status
+		details := map[string]interface{}{"target_type": string(target.Type), "result": status, "agent_id": target.AgentID}
+		if auditErr := s.auditService.RecordEvent(ctx, "system", domain.ActorTypeSystem, action, "target", target.ID, details); auditErr != nil {
+			s.logger.Error("failed to record test connection audit event", "error", auditErr)
+		}
+	}
+}

internal/service/target_test.go

@@ -3,21 +3,26 @@ package service
 import (
 	"context"
 	"encoding/json"
+	"log/slog"
+	"os"
 	"testing"
+	"time"
 
 	"github.com/shankar0123/certctl/internal/domain"
 )
 
 // newTestTargetService creates a TargetService with mock repositories for testing.
-func newTestTargetService() (*TargetService, *mockTargetRepo, *mockAuditRepo) {
+func newTestTargetService() (*TargetService, *mockTargetRepo, *mockAuditRepo, *mockAgentRepo) {
 	targetRepo := &mockTargetRepo{Targets: make(map[string]*domain.DeploymentTarget)}
 	auditRepo := newMockAuditRepository()
 	auditSvc := NewAuditService(auditRepo)
-	return NewTargetService(targetRepo, auditSvc), targetRepo, auditRepo
+	agentRepo := &mockAgentRepo{Agents: make(map[string]*domain.Agent), HeartbeatUpdates: make(map[string]time.Time)}
+	logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError}))
+	return NewTargetService(targetRepo, auditSvc, agentRepo, nil, logger), targetRepo, auditRepo, agentRepo
 }
 
 func TestTargetService_List_Success(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 	ctx := context.Background()
 
 	// Add 3 targets
@@ -44,7 +49,7 @@ func TestTargetService_List_Success(t *testing.T) {
 }
 
 func TestTargetService_List_DefaultPagination(t *testing.T) {
-	svc, _, _ := newTestTargetService()
+	svc, _, _, _ := newTestTargetService()
 	ctx := context.Background()
 
 	// Call with invalid pagination (page=0, perPage=0)
@@ -60,7 +65,7 @@ func TestTargetService_List_DefaultPagination(t *testing.T) {
 }
 
 func TestTargetService_List_EmptyPage(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 	ctx := context.Background()
 
 	// Add 3 targets
@@ -87,7 +92,7 @@ func TestTargetService_List_EmptyPage(t *testing.T) {
 }
 
 func TestTargetService_List_RepoError(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 	ctx := context.Background()
 
 	// Set repo to return error
@@ -104,7 +109,7 @@ func TestTargetService_List_RepoError(t *testing.T) {
 }
 
 func TestTargetService_Get_Success(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 	ctx := context.Background()
 
 	target := &domain.DeploymentTarget{ID: "t-1", Name: "Target 1", Type: domain.TargetTypeNGINX}
@@ -121,7 +126,7 @@ func TestTargetService_Get_Success(t *testing.T) {
 }
 
 func TestTargetService_Get_NotFound(t *testing.T) {
-	svc, _, _ := newTestTargetService()
+	svc, _, _, _ := newTestTargetService()
 	ctx := context.Background()
 
 	result, err := svc.Get(ctx, "nonexistent")
@@ -135,7 +140,7 @@ func TestTargetService_Get_NotFound(t *testing.T) {
 }
 
 func TestTargetService_Create_Success(t *testing.T) {
-	svc, targetRepo, auditRepo := newTestTargetService()
+	svc, targetRepo, auditRepo, _ := newTestTargetService()
 	ctx := context.Background()
 
 	target := &domain.DeploymentTarget{
@@ -168,6 +173,14 @@ func TestTargetService_Create_Success(t *testing.T) {
 		t.Errorf("expected timestamps to be set, CreatedAt=%v, UpdatedAt=%v", target.CreatedAt, target.UpdatedAt)
 	}
 
+	// Verify test status and source defaults
+	if target.TestStatus != "untested" {
+		t.Errorf("expected test_status 'untested', got %s", target.TestStatus)
+	}
+	if target.Source != "database" {
+		t.Errorf("expected source 'database', got %s", target.Source)
+	}
+
 	// Verify audit event
 	if len(auditRepo.Events) == 0 {
 		t.Fatalf("expected audit event, got none")
@@ -184,7 +197,7 @@ func TestTargetService_Create_Success(t *testing.T) {
 }
 
 func TestTargetService_Create_MissingName(t *testing.T) {
-	svc, _, _ := newTestTargetService()
+	svc, _, _, _ := newTestTargetService()
 	ctx := context.Background()
 
 	target := &domain.DeploymentTarget{
@@ -197,8 +210,23 @@ func TestTargetService_Create_MissingName(t *testing.T) {
 	}
 }
 
+func TestTargetService_Create_InvalidType(t *testing.T) {
+	svc, _, _, _ := newTestTargetService()
+	ctx := context.Background()
+
+	target := &domain.DeploymentTarget{
+		Name: "Bad Target",
+		Type: domain.TargetType("InvalidType"),
+	}
+
+	err := svc.Create(ctx, target, "test-actor")
+	if err == nil {
+		t.Fatalf("expected error for invalid type, got nil")
+	}
+}
+
 func TestTargetService_Create_RepoError(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 	ctx := context.Background()
 
 	targetRepo.CreateErr = errNotFound
@@ -215,7 +243,7 @@ func TestTargetService_Create_RepoError(t *testing.T) {
 }
 
 func TestTargetService_Update_Success(t *testing.T) {
-	svc, targetRepo, auditRepo := newTestTargetService()
+	svc, targetRepo, auditRepo, _ := newTestTargetService()
 	ctx := context.Background()
 
 	// Create initial target
@@ -251,7 +279,7 @@ func TestTargetService_Update_Success(t *testing.T) {
 }
 
 func TestTargetService_Update_MissingName(t *testing.T) {
-	svc, _, _ := newTestTargetService()
+	svc, _, _, _ := newTestTargetService()
 	ctx := context.Background()
 
 	target := &domain.DeploymentTarget{
@@ -265,7 +293,7 @@ func TestTargetService_Update_MissingName(t *testing.T) {
 }
 
 func TestTargetService_Delete_Success(t *testing.T) {
-	svc, targetRepo, auditRepo := newTestTargetService()
+	svc, targetRepo, auditRepo, _ := newTestTargetService()
 	ctx := context.Background()
 
 	// Create initial target
@@ -295,7 +323,7 @@ func TestTargetService_Delete_Success(t *testing.T) {
 }
 
 func TestTargetService_Delete_RepoError(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 	ctx := context.Background()
 
 	targetRepo.DeleteErr = errNotFound
@@ -307,7 +335,7 @@ func TestTargetService_Delete_RepoError(t *testing.T) {
 }
 
 func TestTargetService_ListTargets_Success(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 
 	// Add targets
 	target1 := &domain.DeploymentTarget{ID: "t-1", Name: "Target 1", Type: domain.TargetTypeNGINX}
@@ -331,7 +359,7 @@ func TestTargetService_ListTargets_Success(t *testing.T) {
 }
 
 func TestTargetService_GetTarget_Success(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 
 	target := &domain.DeploymentTarget{ID: "t-1", Name: "Target 1", Type: domain.TargetTypeNGINX}
 	targetRepo.AddTarget(target)
@@ -347,7 +375,7 @@ func TestTargetService_GetTarget_Success(t *testing.T) {
 }
 
 func TestTargetService_CreateTarget_Success(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 
 	target := domain.DeploymentTarget{
 		Name: "New Target",
@@ -369,8 +397,22 @@ func TestTargetService_CreateTarget_Success(t *testing.T) {
 	}
 }
 
+func TestTargetService_CreateTarget_InvalidType(t *testing.T) {
+	svc, _, _, _ := newTestTargetService()
+
+	target := domain.DeploymentTarget{
+		Name: "Bad Target",
+		Type: domain.TargetType("Unknown"),
+	}
+
+	_, err := svc.CreateTarget(target)
+	if err == nil {
+		t.Fatalf("expected error for invalid type, got nil")
+	}
+}
+
 func TestTargetService_UpdateTarget_Success(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 
 	// Create initial target
 	target := &domain.DeploymentTarget{ID: "t-1", Name: "Old Name", Type: domain.TargetTypeNGINX}
@@ -393,7 +435,7 @@ func TestTargetService_UpdateTarget_Success(t *testing.T) {
 }
 
 func TestTargetService_DeleteTarget_Success(t *testing.T) {
-	svc, targetRepo, _ := newTestTargetService()
+	svc, targetRepo, _, _ := newTestTargetService()
 
 	// Create initial target
 	target := &domain.DeploymentTarget{ID: "t-1", Name: "Target To Delete", Type: domain.TargetTypeNGINX}
@@ -410,3 +452,130 @@ func TestTargetService_DeleteTarget_Success(t *testing.T) {
 		t.Errorf("target should be deleted from repo")
 	}
 }
+
+func TestTargetService_TestConnection_AgentOnline(t *testing.T) {
+	svc, targetRepo, _, agentRepo := newTestTargetService()
+	ctx := context.Background()
+
+	// Set up agent
+	heartbeat := time.Now()
+	agent := &domain.Agent{
+		ID:              "agent-1",
+		Name:            "Test Agent",
+		Status:          domain.AgentStatusOnline,
+		LastHeartbeatAt: &heartbeat,
+	}
+	agentRepo.Create(ctx, agent)
+
+	// Set up target assigned to agent
+	target := &domain.DeploymentTarget{
+		ID:      "t-1",
+		Name:    "Test Target",
+		Type:    domain.TargetTypeNGINX,
+		AgentID: "agent-1",
+	}
+	targetRepo.AddTarget(target)
+
+	// Test connection should succeed
+	err := svc.TestConnection(ctx, "t-1")
+	if err != nil {
+		t.Fatalf("expected success, got error: %v", err)
+	}
+
+	// Verify test status was updated
+	stored := targetRepo.Targets["t-1"]
+	if stored.TestStatus != "success" {
+		t.Errorf("expected test_status 'success', got %s", stored.TestStatus)
+	}
+	if stored.LastTestedAt == nil {
+		t.Error("expected last_tested_at to be set")
+	}
+}
+
+func TestTargetService_TestConnection_AgentOffline(t *testing.T) {
+	svc, targetRepo, _, agentRepo := newTestTargetService()
+	ctx := context.Background()
+
+	// Set up offline agent
+	agent := &domain.Agent{
+		ID:     "agent-1",
+		Name:   "Offline Agent",
+		Status: domain.AgentStatusOffline,
+	}
+	agentRepo.Create(ctx, agent)
+
+	// Set up target
+	target := &domain.DeploymentTarget{
+		ID:      "t-1",
+		Name:    "Test Target",
+		Type:    domain.TargetTypeNGINX,
+		AgentID: "agent-1",
+	}
+	targetRepo.AddTarget(target)
+
+	err := svc.TestConnection(ctx, "t-1")
+	if err == nil {
+		t.Fatal("expected error for offline agent, got nil")
+	}
+
+	stored := targetRepo.Targets["t-1"]
+	if stored.TestStatus != "failed" {
+		t.Errorf("expected test_status 'failed', got %s", stored.TestStatus)
+	}
+}
+
+func TestTargetService_TestConnection_NoAgent(t *testing.T) {
+	svc, targetRepo, _, _ := newTestTargetService()
+	ctx := context.Background()
+
+	target := &domain.DeploymentTarget{
+		ID:      "t-1",
+		Name:    "Test Target",
+		Type:    domain.TargetTypeNGINX,
+		AgentID: "",
+	}
+	targetRepo.AddTarget(target)
+
+	err := svc.TestConnection(ctx, "t-1")
+	if err == nil {
+		t.Fatal("expected error for missing agent, got nil")
+	}
+}
+
+func TestTargetService_TestConnection_TargetNotFound(t *testing.T) {
+	svc, _, _, _ := newTestTargetService()
+	ctx := context.Background()
+
+	err := svc.TestConnection(ctx, "nonexistent")
+	if err == nil {
+		t.Fatal("expected error for nonexistent target, got nil")
+	}
+}
+
+func TestTargetService_TestConnection_StaleHeartbeat(t *testing.T) {
+	svc, targetRepo, _, agentRepo := newTestTargetService()
+	ctx := context.Background()
+
+	// Set up agent with stale heartbeat (10 minutes ago)
+	staleTime := time.Now().Add(-10 * time.Minute)
+	agent := &domain.Agent{
+		ID:              "agent-1",
+		Name:            "Stale Agent",
+		Status:          domain.AgentStatusOnline,
+		LastHeartbeatAt: &staleTime,
+	}
+	agentRepo.Create(ctx, agent)
+
+	target := &domain.DeploymentTarget{
+		ID:      "t-1",
+		Name:    "Test Target",
+		Type:    domain.TargetTypeNGINX,
+		AgentID: "agent-1",
+	}
+	targetRepo.AddTarget(target)
+
+	err := svc.TestConnection(ctx, "t-1")
+	if err == nil {
+		t.Fatal("expected error for stale heartbeat, got nil")
+	}
+}

internal/service/testutil_test.go

@@ -637,6 +637,19 @@ func (m *mockTargetRepo) Create(ctx context.Context, target *domain.DeploymentTa
 	return nil
 }
 
+func (m *mockTargetRepo) CreateIfNotExists(ctx context.Context, target *domain.DeploymentTarget) (bool, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.CreateErr != nil {
+		return false, m.CreateErr
+	}
+	if _, exists := m.Targets[target.ID]; exists {
+		return false, nil
+	}
+	m.Targets[target.ID] = target
+	return true, nil
+}
+
 func (m *mockTargetRepo) Update(ctx context.Context, target *domain.DeploymentTarget) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()