Merge branch 'fix/d-008-policy-engine-drift'

This commit is contained in:
shankar0123
2026-04-18 14:56:06 +00:00
12 changed files with 823 additions and 75 deletions
+1
View File
@@ -3461,6 +3461,7 @@ components:
- RequiredMetadata
- AllowedEnvironments
- RenewalLeadTime
- CertificateLifetime
PolicySeverity:
type: string
+1
View File
@@ -145,6 +145,7 @@ func main() {
// Initialize services (following the dependency graph)
auditService := service.NewAuditService(auditRepo)
policyService := service.NewPolicyService(policyRepo, auditService)
policyService.SetCertRepo(certificateRepo) // D-008: CertificateLifetime arm needs CertificateVersion.NotBefore/NotAfter
certificateService := service.NewCertificateService(certificateRepo, policyService, auditService)
notifierRegistry := make(map[string]service.Notifier)
+2 -1
View File
@@ -71,10 +71,11 @@ func ValidatePolicyType(policyType interface{}) error {
"RequiredMetadata": true,
"AllowedEnvironments": true,
"RenewalLeadTime": true,
"CertificateLifetime": true,
}
typeStr := fmt.Sprintf("%v", policyType)
if !validTypes[typeStr] {
return ValidationError{Field: "type", Message: "type must be one of: AllowedIssuers, AllowedDomains, RequiredMetadata, AllowedEnvironments, RenewalLeadTime"}
return ValidationError{Field: "type", Message: "type must be one of: AllowedIssuers, AllowedDomains, RequiredMetadata, AllowedEnvironments, RenewalLeadTime, CertificateLifetime"}
}
return nil
}
+1
View File
@@ -26,6 +26,7 @@ const (
PolicyTypeRequiredMetadata PolicyType = "RequiredMetadata"
PolicyTypeAllowedEnvironments PolicyType = "AllowedEnvironments"
PolicyTypeRenewalLeadTime PolicyType = "RenewalLeadTime"
PolicyTypeCertificateLifetime PolicyType = "CertificateLifetime"
)
// PolicyViolation records an instance of a certificate violating a policy rule.
+206 -49
View File
@@ -2,8 +2,10 @@ package service
import (
"context"
"encoding/json"
"fmt"
"log/slog"
"strings"
"time"
"github.com/shankar0123/certctl/internal/domain"
@@ -14,6 +16,11 @@ import (
type PolicyService struct {
policyRepo repository.PolicyRepository
auditService *AuditService
// certRepo is optional and only required by the CertificateLifetime rule
// arm, which must read NotBefore/NotAfter from the latest CertificateVersion.
// Wire via SetCertRepo after construction; rules other than
// CertificateLifetime operate without it.
certRepo repository.CertificateRepository
}
// NewPolicyService creates a new policy service.
@@ -27,6 +34,16 @@ func NewPolicyService(
}
}
// SetCertRepo wires the certificate repository needed for the CertificateLifetime
// rule arm. Kept as a setter (not a constructor parameter) so the ~36 existing
// NewPolicyService call sites don't churn for a single new arm's dependency.
// Safe to call before or after construction; evaluateRule checks for nil and
// returns an error if a CertificateLifetime rule fires without a wired repo
// (the caller at ValidateCertificate logs and continues).
func (s *PolicyService) SetCertRepo(r repository.CertificateRepository) {
s.certRepo = r
}
// ValidateCertificate runs all enabled policy rules against a certificate.
func (s *PolicyService) ValidateCertificate(ctx context.Context, cert *domain.ManagedCertificate) ([]*domain.PolicyViolation, error) {
rules, err := s.policyRepo.ListRules(ctx)
@@ -43,7 +60,7 @@ func (s *PolicyService) ValidateCertificate(ctx context.Context, cert *domain.Ma
}
// Evaluate rule against certificate
v, err := s.evaluateRule(rule, cert)
v, err := s.evaluateRule(ctx, rule, cert)
if err != nil {
slog.Error("failed to evaluate rule", "rule_id", rule.ID, "error", err)
continue
@@ -58,73 +75,163 @@ func (s *PolicyService) ValidateCertificate(ctx context.Context, cert *domain.Ma
}
// evaluateRule checks if a certificate violates a single policy rule.
func (s *PolicyService) evaluateRule(rule *domain.PolicyRule, cert *domain.ManagedCertificate) (*domain.PolicyViolation, error) {
//
// D-008 closes the engine loop by:
// 1. Consuming rule.Severity on every violation (the pre-D-008 engine
// hardcoded PolicySeverityWarning, which silently defeated the D-006
// per-rule severity column).
// 2. Parsing rule.Config per-arm so rules carry real thresholds / allowlists
// instead of the pre-D-008 "metadata absent" placeholders. Empty/null
// Config preserves the pre-D-008 missing-field behavior as a
// backward-compat invariant — a rule without config still fires on the
// absent-field shape but using its configured severity.
// 3. Adding the CertificateLifetime arm, which reads NotBefore/NotAfter from
// the latest CertificateVersion (injected via SetCertRepo). Required
// because ManagedCertificate tracks ExpiresAt but not issuance date.
//
// Bad-config failure mode: json.Unmarshal error returns (nil, error) shaped
// as `invalid config for rule <id> (type=<type>): <err>`; the caller at
// ValidateCertificate logs and continues so one malformed rule doesn't fail
// the entire pass.
func (s *PolicyService) evaluateRule(ctx context.Context, rule *domain.PolicyRule, cert *domain.ManagedCertificate) (*domain.PolicyViolation, error) {
switch rule.Type {
case domain.PolicyTypeAllowedIssuers:
// Restrict to specific issuers
// Note: In a production implementation, we would parse rule.Config to extract parameters
// Config: {"allowed_issuer_ids": ["iss-a", "iss-b"]}
// Empty config = fire only on absent IssuerID (backward-compat).
var cfg struct {
AllowedIssuerIDs []string `json:"allowed_issuer_ids"`
}
if len(rule.Config) > 0 {
if err := json.Unmarshal(rule.Config, &cfg); err != nil {
return nil, fmt.Errorf("invalid config for rule %s (type=%s): %w", rule.ID, rule.Type, err)
}
}
if cert.IssuerID == "" {
return &domain.PolicyViolation{
ID: generateID("violation"),
RuleID: rule.ID,
CertificateID: cert.ID,
Severity: domain.PolicySeverityWarning,
Message: "certificate has no issuer assigned",
CreatedAt: time.Now(),
}, nil
return s.violation(rule, cert, "certificate has no issuer assigned"), nil
}
if len(cfg.AllowedIssuerIDs) > 0 && !containsString(cfg.AllowedIssuerIDs, cert.IssuerID) {
return s.violation(rule, cert, fmt.Sprintf("issuer %q is not in the allowed list", cert.IssuerID)), nil
}
case domain.PolicyTypeAllowedDomains:
// Ensure certificate domains are in allowed list
// Config: {"allowed_domains": ["example.com", "*.internal.example.com"]}
// Wildcards are literal prefix matches (*.foo matches anything ending
// in .foo). Empty config = fire only on zero SANs (backward-compat).
var cfg struct {
AllowedDomains []string `json:"allowed_domains"`
}
if len(rule.Config) > 0 {
if err := json.Unmarshal(rule.Config, &cfg); err != nil {
return nil, fmt.Errorf("invalid config for rule %s (type=%s): %w", rule.ID, rule.Type, err)
}
}
if len(cert.SANs) == 0 {
return &domain.PolicyViolation{
ID: generateID("violation"),
RuleID: rule.ID,
CertificateID: cert.ID,
Severity: domain.PolicySeverityWarning,
Message: "certificate has no subject alternative names",
CreatedAt: time.Now(),
}, nil
return s.violation(rule, cert, "certificate has no subject alternative names"), nil
}
if len(cfg.AllowedDomains) > 0 {
for _, san := range cert.SANs {
if !domainAllowed(san, cfg.AllowedDomains) {
return s.violation(rule, cert, fmt.Sprintf("SAN %q is not in the allowed domain list", san)), nil
}
}
}
case domain.PolicyTypeRequiredMetadata:
// Ensure certificate has required metadata/tags
// Config: {"required_keys": ["owner", "cost-center"]}
// Empty config = fire only on zero tags (backward-compat).
var cfg struct {
RequiredKeys []string `json:"required_keys"`
}
if len(rule.Config) > 0 {
if err := json.Unmarshal(rule.Config, &cfg); err != nil {
return nil, fmt.Errorf("invalid config for rule %s (type=%s): %w", rule.ID, rule.Type, err)
}
}
if len(cert.Tags) == 0 {
return &domain.PolicyViolation{
ID: generateID("violation"),
RuleID: rule.ID,
CertificateID: cert.ID,
Severity: domain.PolicySeverityWarning,
Message: "certificate has no tags or metadata",
CreatedAt: time.Now(),
}, nil
return s.violation(rule, cert, "certificate has no tags or metadata"), nil
}
for _, key := range cfg.RequiredKeys {
if _, ok := cert.Tags[key]; !ok {
return s.violation(rule, cert, fmt.Sprintf("certificate is missing required metadata key %q", key)), nil
}
}
case domain.PolicyTypeAllowedEnvironments:
// Restrict to specific environments
// Config: {"allowed": ["prod", "staging"]}
// Empty config = fire only on empty Environment (backward-compat).
var cfg struct {
Allowed []string `json:"allowed"`
}
if len(rule.Config) > 0 {
if err := json.Unmarshal(rule.Config, &cfg); err != nil {
return nil, fmt.Errorf("invalid config for rule %s (type=%s): %w", rule.ID, rule.Type, err)
}
}
if cert.Environment == "" {
return &domain.PolicyViolation{
ID: generateID("violation"),
RuleID: rule.ID,
CertificateID: cert.ID,
Severity: domain.PolicySeverityWarning,
Message: "certificate has no environment assigned",
CreatedAt: time.Now(),
}, nil
return s.violation(rule, cert, "certificate has no environment assigned"), nil
}
if len(cfg.Allowed) > 0 && !containsString(cfg.Allowed, cert.Environment) {
return s.violation(rule, cert, fmt.Sprintf("environment %q is not in the allowed list", cert.Environment)), nil
}
case domain.PolicyTypeRenewalLeadTime:
// Ensure renewal begins before certificate expires
// Config: {"lead_time_days": 30}
// Fires when remaining validity drops below lead_time_days and the
// cert is not already expired. Empty/zero config falls back to the
// pre-D-008 hardcoded 30-day threshold for backward compatibility.
var cfg struct {
LeadTimeDays int `json:"lead_time_days"`
}
if len(rule.Config) > 0 {
if err := json.Unmarshal(rule.Config, &cfg); err != nil {
return nil, fmt.Errorf("invalid config for rule %s (type=%s): %w", rule.ID, rule.Type, err)
}
}
leadDays := cfg.LeadTimeDays
if leadDays <= 0 {
leadDays = 30
}
daysUntilExpiry := time.Until(cert.ExpiresAt).Hours() / 24
if daysUntilExpiry < 30 && daysUntilExpiry > 0 {
return &domain.PolicyViolation{
ID: generateID("violation"),
RuleID: rule.ID,
CertificateID: cert.ID,
Severity: domain.PolicySeverityWarning,
Message: fmt.Sprintf("certificate expires in %.1f days, plan renewal soon", daysUntilExpiry),
CreatedAt: time.Now(),
}, nil
if daysUntilExpiry < float64(leadDays) && daysUntilExpiry > 0 {
return s.violation(rule, cert, fmt.Sprintf("certificate expires in %.1f days, plan renewal soon (policy lead time: %d days)", daysUntilExpiry, leadDays)), nil
}
case domain.PolicyTypeCertificateLifetime:
// Config: {"max_days": 397}
// Reads NotBefore/NotAfter from the latest CertificateVersion via the
// injected certRepo. ManagedCertificate exposes ExpiresAt but not the
// issuance date, so lifetime math requires the version record.
//
// If certRepo wasn't wired (test misconfiguration / early boot),
// returns an error so the caller logs it — better a loud failure
// than silently ignoring the rule. If GetLatestVersion errors (e.g.,
// the cert hasn't been issued yet), we skip the check — a cert with
// no version has no lifetime to measure, matching the missing-field
// backward-compat pattern used by the other arms.
if s.certRepo == nil {
return nil, fmt.Errorf("CertificateLifetime rule %s requires cert repository (not wired via SetCertRepo)", rule.ID)
}
var cfg struct {
MaxDays int `json:"max_days"`
}
if len(rule.Config) > 0 {
if err := json.Unmarshal(rule.Config, &cfg); err != nil {
return nil, fmt.Errorf("invalid config for rule %s (type=%s): %w", rule.ID, rule.Type, err)
}
}
if cfg.MaxDays <= 0 {
// No threshold configured — nothing meaningful to enforce.
return nil, nil
}
version, err := s.certRepo.GetLatestVersion(ctx, cert.ID)
if err != nil {
// No version yet — nothing to measure. Not an engine error;
// the cert simply hasn't been issued.
return nil, nil
}
lifetimeDays := version.NotAfter.Sub(version.NotBefore).Hours() / 24
if lifetimeDays > float64(cfg.MaxDays) {
return s.violation(rule, cert, fmt.Sprintf("certificate lifetime is %.1f days, exceeds policy max of %d days", lifetimeDays, cfg.MaxDays)), nil
}
default:
@@ -134,6 +241,56 @@ func (s *PolicyService) evaluateRule(rule *domain.PolicyRule, cert *domain.Manag
return nil, nil
}
// violation constructs a PolicyViolation carrying the rule's configured
// severity. Centralizing the build eliminates the pre-D-008 bug where each
// arm independently stamped PolicySeverityWarning on its violation.
func (s *PolicyService) violation(rule *domain.PolicyRule, cert *domain.ManagedCertificate, message string) *domain.PolicyViolation {
return &domain.PolicyViolation{
ID: generateID("violation"),
RuleID: rule.ID,
CertificateID: cert.ID,
Severity: rule.Severity,
Message: message,
CreatedAt: time.Now(),
}
}
// containsString reports whether needle is present in haystack.
func containsString(haystack []string, needle string) bool {
for _, s := range haystack {
if s == needle {
return true
}
}
return false
}
// domainAllowed reports whether a SAN (hostname) matches any of the allowed
// domain patterns. Patterns may be exact matches or `*.example.com` wildcards
// (the wildcard consumes a single label: `*.foo.com` matches `bar.foo.com`
// but not `baz.bar.foo.com`, mirroring X.509 SAN wildcard semantics).
func domainAllowed(san string, allowed []string) bool {
san = strings.ToLower(strings.TrimSpace(san))
for _, pattern := range allowed {
pattern = strings.ToLower(strings.TrimSpace(pattern))
if pattern == san {
return true
}
if strings.HasPrefix(pattern, "*.") {
suffix := pattern[1:] // ".foo.com"
if strings.HasSuffix(san, suffix) {
// Ensure wildcard consumes exactly one label — reject
// sub-subdomains.
head := strings.TrimSuffix(san, suffix)
if head != "" && !strings.Contains(head, ".") {
return true
}
}
}
}
return false
}
// CreateRule stores a new policy rule.
func (s *PolicyService) CreateRule(ctx context.Context, rule *domain.PolicyRule, actor string) error {
if rule.ID == "" {
+534
View File
@@ -3,6 +3,7 @@ package service
import (
"context"
"encoding/json"
"strings"
"testing"
"time"
@@ -420,3 +421,536 @@ func TestCreatePolicy(t *testing.T) {
t.Errorf("expected 1 rule in repo, got %d", len(policyRepo.Rules))
}
}
// ============================================================================
// D-008 regression tests
//
// These pin the behavior that closes the D-006 loop:
// 1. evaluateRule copies rule.Severity onto every violation (pre-D-008 the
// engine hardcoded Warning regardless of the rule's configured severity).
// 2. evaluateRule parses rule.Config per-arm so rules enforce real thresholds
// and allowlists (pre-D-008 the configs were ignored; rules fired only on
// the missing-field shape).
// 3. An empty/zero Config preserves the pre-D-008 missing-field violation
// (backward-compat invariant).
// 4. Malformed Config returns an error; the caller logs and skips the rule
// instead of producing a zero-value violation.
// 5. CertificateLifetime (new 6th arm) reads NotBefore/NotAfter from the
// latest CertificateVersion via the cert repo wired with SetCertRepo.
// ============================================================================
// mkRule is a tiny constructor used by the D-008 tests to keep the table rows
// readable. Every rule is enabled; test-specific fields layer on top.
func mkRule(id string, t domain.PolicyType, sev domain.PolicySeverity, cfg string) *domain.PolicyRule {
return &domain.PolicyRule{
ID: id,
Name: id,
Type: t,
Config: json.RawMessage(cfg),
Enabled: true,
Severity: sev,
}
}
// evalCert is a minimal cert used by the arms that don't look at much beyond
// the shape of the field they're testing. Tests shadow fields as needed.
func evalCert() *domain.ManagedCertificate {
return &domain.ManagedCertificate{
ID: "cert-001",
CommonName: "example.com",
Status: domain.CertificateStatusActive,
ExpiresAt: time.Now().AddDate(1, 0, 0),
}
}
// TestEvaluateRule_SeverityPassThrough pins invariant #1 — every arm stamps
// rule.Severity onto the violation. The pre-D-008 bug was that arms
// independently hardcoded PolicySeverityWarning. We test each arm with a
// severity that isn't the legacy default so a regression would be visible.
func TestEvaluateRule_SeverityPassThrough(t *testing.T) {
ctx := context.Background()
// Cert shaped to fail every non-empty-config check via the backward-compat
// missing-field path. Each row picks a severity intentionally ≠ Warning to
// make a stray hardcoded default obvious.
cases := []struct {
name string
rule *domain.PolicyRule
cert *domain.ManagedCertificate
setupFn func(svc *PolicyService)
expected domain.PolicySeverity
}{
{
name: "AllowedIssuers Critical via missing IssuerID",
rule: mkRule("r-ai", domain.PolicyTypeAllowedIssuers, domain.PolicySeverityCritical, ""),
cert: func() *domain.ManagedCertificate {
c := evalCert()
c.IssuerID = ""
return c
}(),
expected: domain.PolicySeverityCritical,
},
{
name: "AllowedDomains Error via empty SANs",
rule: mkRule("r-ad", domain.PolicyTypeAllowedDomains, domain.PolicySeverityError, ""),
cert: func() *domain.ManagedCertificate {
c := evalCert()
c.SANs = nil
return c
}(),
expected: domain.PolicySeverityError,
},
{
name: "RequiredMetadata Critical via empty Tags",
rule: mkRule("r-rm", domain.PolicyTypeRequiredMetadata, domain.PolicySeverityCritical, ""),
cert: func() *domain.ManagedCertificate {
c := evalCert()
c.Tags = nil
return c
}(),
expected: domain.PolicySeverityCritical,
},
{
name: "AllowedEnvironments Warning via empty Environment",
rule: mkRule("r-ae", domain.PolicyTypeAllowedEnvironments, domain.PolicySeverityWarning, ""),
cert: func() *domain.ManagedCertificate {
c := evalCert()
c.Environment = ""
return c
}(),
expected: domain.PolicySeverityWarning,
},
{
name: "RenewalLeadTime Critical via short remaining validity",
rule: mkRule("r-rl", domain.PolicyTypeRenewalLeadTime, domain.PolicySeverityCritical, `{"lead_time_days": 60}`),
cert: func() *domain.ManagedCertificate {
c := evalCert()
c.ExpiresAt = time.Now().AddDate(0, 0, 30) // 30d remaining < 60d lead
return c
}(),
expected: domain.PolicySeverityCritical,
},
{
name: "CertificateLifetime Error via 365d span vs 90d max",
rule: mkRule("r-cl", domain.PolicyTypeCertificateLifetime, domain.PolicySeverityError, `{"max_days": 90}`),
cert: evalCert(),
setupFn: func(svc *PolicyService) {
// Seed a version with 365d lifetime on the same cert ID used
// by evalCert().
cr := &mockCertRepo{
Certs: map[string]*domain.ManagedCertificate{},
Versions: map[string][]*domain.CertificateVersion{},
}
now := time.Now()
cr.Versions["cert-001"] = []*domain.CertificateVersion{{
ID: "ver-001",
CertificateID: "cert-001",
NotBefore: now.AddDate(0, 0, -10),
NotAfter: now.AddDate(1, 0, -10), // ~365d lifetime
}}
svc.SetCertRepo(cr)
},
expected: domain.PolicySeverityError,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
policyRepo := &mockPolicyRepo{
Rules: map[string]*domain.PolicyRule{tc.rule.ID: tc.rule},
Violations: []*domain.PolicyViolation{},
}
auditService := NewAuditService(&mockAuditRepo{})
svc := NewPolicyService(policyRepo, auditService)
if tc.setupFn != nil {
tc.setupFn(svc)
}
violations, err := svc.ValidateCertificate(ctx, tc.cert)
if err != nil {
t.Fatalf("ValidateCertificate failed: %v", err)
}
if len(violations) != 1 {
t.Fatalf("expected 1 violation, got %d", len(violations))
}
if violations[0].Severity != tc.expected {
t.Errorf("expected severity %q, got %q", tc.expected, violations[0].Severity)
}
if violations[0].RuleID != tc.rule.ID {
t.Errorf("expected rule ID %q, got %q", tc.rule.ID, violations[0].RuleID)
}
})
}
}
// TestEvaluateRule_ConfigConsumed pins invariant #2 — non-empty Config drives
// arm behavior (allowlists, thresholds, keys). Each subtest supplies a config
// that the cert would satisfy under the backward-compat missing-field path
// but violates under the config-aware path. A regression to the pre-D-008
// "config silently dropped" behavior would make these pass with 0 violations.
func TestEvaluateRule_ConfigConsumed(t *testing.T) {
ctx := context.Background()
t.Run("AllowedIssuers rejects issuer not in allowlist", func(t *testing.T) {
rule := mkRule("r-ai", domain.PolicyTypeAllowedIssuers, domain.PolicySeverityWarning,
`{"allowed_issuer_ids": ["iss-acme"]}`)
cert := evalCert()
cert.IssuerID = "iss-wrong"
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 1 {
t.Fatalf("expected 1 violation for disallowed issuer, got %d", len(violations))
}
if !strings.Contains(violations[0].Message, "iss-wrong") {
t.Errorf("expected message to mention issuer ID, got %q", violations[0].Message)
}
})
t.Run("AllowedIssuers accepts issuer in allowlist", func(t *testing.T) {
rule := mkRule("r-ai", domain.PolicyTypeAllowedIssuers, domain.PolicySeverityWarning,
`{"allowed_issuer_ids": ["iss-acme"]}`)
cert := evalCert()
cert.IssuerID = "iss-acme"
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 0 {
t.Errorf("expected 0 violations for allowed issuer, got %d", len(violations))
}
})
t.Run("AllowedDomains rejects SAN outside allowlist", func(t *testing.T) {
rule := mkRule("r-ad", domain.PolicyTypeAllowedDomains, domain.PolicySeverityWarning,
`{"allowed_domains": ["*.foo.com"]}`)
cert := evalCert()
cert.SANs = []string{"bar.elsewhere.com"}
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 1 {
t.Fatalf("expected 1 violation for disallowed SAN, got %d", len(violations))
}
})
t.Run("AllowedDomains wildcard matches single-label subdomain", func(t *testing.T) {
rule := mkRule("r-ad", domain.PolicyTypeAllowedDomains, domain.PolicySeverityWarning,
`{"allowed_domains": ["*.foo.com"]}`)
cert := evalCert()
cert.SANs = []string{"bar.foo.com"}
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 0 {
t.Errorf("expected 0 violations for single-label wildcard match, got %d", len(violations))
}
})
t.Run("AllowedDomains wildcard rejects multi-label subdomain", func(t *testing.T) {
// X.509 wildcard semantics: *.foo consumes exactly one label.
rule := mkRule("r-ad", domain.PolicyTypeAllowedDomains, domain.PolicySeverityWarning,
`{"allowed_domains": ["*.foo.com"]}`)
cert := evalCert()
cert.SANs = []string{"baz.bar.foo.com"}
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 1 {
t.Errorf("expected 1 violation for multi-label wildcard (X.509 semantics), got %d", len(violations))
}
})
t.Run("RequiredMetadata rejects missing key", func(t *testing.T) {
rule := mkRule("r-rm", domain.PolicyTypeRequiredMetadata, domain.PolicySeverityWarning,
`{"required_keys": ["owner"]}`)
cert := evalCert()
cert.Tags = map[string]string{"team": "platform"}
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 1 {
t.Fatalf("expected 1 violation for missing owner key, got %d", len(violations))
}
if !strings.Contains(violations[0].Message, "owner") {
t.Errorf("expected message to mention the missing key, got %q", violations[0].Message)
}
})
t.Run("RequiredMetadata accepts all required keys present", func(t *testing.T) {
rule := mkRule("r-rm", domain.PolicyTypeRequiredMetadata, domain.PolicySeverityWarning,
`{"required_keys": ["owner"]}`)
cert := evalCert()
cert.Tags = map[string]string{"owner": "alice"}
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 0 {
t.Errorf("expected 0 violations when all required keys present, got %d", len(violations))
}
})
t.Run("AllowedEnvironments rejects env outside allowlist", func(t *testing.T) {
rule := mkRule("r-ae", domain.PolicyTypeAllowedEnvironments, domain.PolicySeverityWarning,
`{"allowed": ["production", "staging"]}`)
cert := evalCert()
cert.Environment = "wild-west"
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 1 {
t.Fatalf("expected 1 violation for disallowed env, got %d", len(violations))
}
})
t.Run("RenewalLeadTime fires when remaining < configured lead", func(t *testing.T) {
rule := mkRule("r-rl", domain.PolicyTypeRenewalLeadTime, domain.PolicySeverityWarning,
`{"lead_time_days": 60}`)
cert := evalCert()
cert.ExpiresAt = time.Now().AddDate(0, 0, 30) // 30d < 60d lead
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 1 {
t.Fatalf("expected 1 violation for 30d remaining vs 60d lead, got %d", len(violations))
}
})
t.Run("RenewalLeadTime quiet when remaining > configured lead", func(t *testing.T) {
rule := mkRule("r-rl", domain.PolicyTypeRenewalLeadTime, domain.PolicySeverityWarning,
`{"lead_time_days": 14}`)
cert := evalCert()
cert.ExpiresAt = time.Now().AddDate(0, 0, 60) // 60d > 14d lead
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 0 {
t.Errorf("expected 0 violations when plenty of runway remains, got %d", len(violations))
}
})
t.Run("CertificateLifetime fires when lifetime exceeds max", func(t *testing.T) {
rule := mkRule("r-cl", domain.PolicyTypeCertificateLifetime, domain.PolicySeverityWarning,
`{"max_days": 90}`)
cert := evalCert()
now := time.Now()
certRepo := &mockCertRepo{
Certs: map[string]*domain.ManagedCertificate{},
Versions: map[string][]*domain.CertificateVersion{},
}
certRepo.Versions["cert-001"] = []*domain.CertificateVersion{{
ID: "ver-001",
CertificateID: "cert-001",
NotBefore: now.AddDate(0, 0, -1),
NotAfter: now.AddDate(1, 0, -1), // ~365d > 90d
}}
violations := runEval(ctx, t, rule, cert, certRepo)
if len(violations) != 1 {
t.Fatalf("expected 1 violation for 365d lifetime vs 90d max, got %d", len(violations))
}
if !strings.Contains(violations[0].Message, "90 days") {
t.Errorf("expected message to mention max_days threshold, got %q", violations[0].Message)
}
})
t.Run("CertificateLifetime quiet when lifetime within max", func(t *testing.T) {
rule := mkRule("r-cl", domain.PolicyTypeCertificateLifetime, domain.PolicySeverityWarning,
`{"max_days": 90}`)
cert := evalCert()
now := time.Now()
certRepo := &mockCertRepo{
Certs: map[string]*domain.ManagedCertificate{},
Versions: map[string][]*domain.CertificateVersion{},
}
certRepo.Versions["cert-001"] = []*domain.CertificateVersion{{
ID: "ver-001",
CertificateID: "cert-001",
NotBefore: now.AddDate(0, 0, -10),
NotAfter: now.AddDate(0, 0, 60), // 70d lifetime < 90d
}}
violations := runEval(ctx, t, rule, cert, certRepo)
if len(violations) != 0 {
t.Errorf("expected 0 violations for 70d lifetime under 90d max, got %d", len(violations))
}
})
}
// TestEvaluateRule_EmptyConfig_BackCompat pins invariant #3 — a rule with no
// Config (e.g., a legacy row from a pre-D-008 migration) still fires on the
// pre-D-008 missing-field shape using its configured severity. This is how
// we let existing deployments migrate without a schema rewrite.
func TestEvaluateRule_EmptyConfig_BackCompat(t *testing.T) {
ctx := context.Background()
t.Run("RequiredMetadata fires on zero tags", func(t *testing.T) {
rule := mkRule("r-rm", domain.PolicyTypeRequiredMetadata, domain.PolicySeverityError, "")
cert := evalCert()
cert.Tags = nil
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 1 {
t.Fatalf("expected 1 backcompat violation, got %d", len(violations))
}
if violations[0].Severity != domain.PolicySeverityError {
t.Errorf("expected severity Error (passed through from rule), got %q", violations[0].Severity)
}
})
t.Run("RequiredMetadata quiet when any tags present under empty config", func(t *testing.T) {
// Empty config means "only fire on missing-field shape" — so a cert
// with any tags (even not what a human would call meaningful) passes.
rule := mkRule("r-rm", domain.PolicyTypeRequiredMetadata, domain.PolicySeverityError, "")
cert := evalCert()
cert.Tags = map[string]string{"arbitrary": "value"}
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 0 {
t.Errorf("expected 0 violations under backcompat shape w/ tags set, got %d", len(violations))
}
})
t.Run("RenewalLeadTime uses 30d default under empty/zero config", func(t *testing.T) {
rule := mkRule("r-rl", domain.PolicyTypeRenewalLeadTime, domain.PolicySeverityWarning, "")
cert := evalCert()
cert.ExpiresAt = time.Now().AddDate(0, 0, 15) // 15d < 30d default
violations := runEval(ctx, t, rule, cert, nil)
if len(violations) != 1 {
t.Errorf("expected 1 violation under 30d backcompat default, got %d", len(violations))
}
})
}
// TestEvaluateRule_BadConfig_SkipsRule pins invariant #4 — malformed JSON in
// Config returns an error from evaluateRule, which ValidateCertificate logs
// and swallows. The pass continues; no zero-value violation is emitted.
// Co-located rules still fire normally.
func TestEvaluateRule_BadConfig_SkipsRule(t *testing.T) {
ctx := context.Background()
// Rule 1 has malformed JSON — should log+skip.
// Rule 2 is a healthy AllowedIssuers rule that should still emit its
// violation on the missing-IssuerID cert. If the bad rule poisoned the
// loop, we'd see 0 or 2 violations instead of exactly 1.
badRule := mkRule("r-bad", domain.PolicyTypeAllowedIssuers, domain.PolicySeverityError,
`{"allowed_issuer_ids": [`) // unterminated JSON
goodRule := mkRule("r-good", domain.PolicyTypeAllowedEnvironments, domain.PolicySeverityWarning, "")
policyRepo := &mockPolicyRepo{
Rules: map[string]*domain.PolicyRule{
badRule.ID: badRule,
goodRule.ID: goodRule,
},
Violations: []*domain.PolicyViolation{},
}
auditService := NewAuditService(&mockAuditRepo{})
svc := NewPolicyService(policyRepo, auditService)
cert := evalCert()
cert.IssuerID = "" // would trigger the bad rule if it wasn't skipped
cert.Environment = "" // triggers goodRule via missing-field backcompat
violations, err := svc.ValidateCertificate(ctx, cert)
if err != nil {
t.Fatalf("ValidateCertificate should swallow rule-eval errors, got %v", err)
}
if len(violations) != 1 {
t.Fatalf("expected exactly 1 violation (bad rule skipped, good rule fires), got %d", len(violations))
}
if violations[0].RuleID != goodRule.ID {
t.Errorf("expected violation from r-good, got %q", violations[0].RuleID)
}
}
// TestEvaluateRule_CertificateLifetime_RepoScenarios pins the setter-injection
// pattern for the 6th arm. SetCertRepo wires the dependency; without it the
// arm errors (logged+skipped by the caller). With it but no version present,
// the arm silently returns nil (matching the missing-field backcompat shape).
func TestEvaluateRule_CertificateLifetime_RepoScenarios(t *testing.T) {
ctx := context.Background()
t.Run("repo not wired logs and skips", func(t *testing.T) {
rule := mkRule("r-cl", domain.PolicyTypeCertificateLifetime, domain.PolicySeverityError,
`{"max_days": 90}`)
policyRepo := &mockPolicyRepo{
Rules: map[string]*domain.PolicyRule{rule.ID: rule},
Violations: []*domain.PolicyViolation{},
}
svc := NewPolicyService(policyRepo, NewAuditService(&mockAuditRepo{}))
// deliberately do NOT call SetCertRepo
violations, err := svc.ValidateCertificate(ctx, evalCert())
if err != nil {
t.Fatalf("ValidateCertificate should swallow the nil-repo error, got %v", err)
}
if len(violations) != 0 {
t.Errorf("expected 0 violations when repo unwired (rule skipped), got %d", len(violations))
}
})
t.Run("version missing silently skips", func(t *testing.T) {
rule := mkRule("r-cl", domain.PolicyTypeCertificateLifetime, domain.PolicySeverityError,
`{"max_days": 90}`)
policyRepo := &mockPolicyRepo{
Rules: map[string]*domain.PolicyRule{rule.ID: rule},
Violations: []*domain.PolicyViolation{},
}
svc := NewPolicyService(policyRepo, NewAuditService(&mockAuditRepo{}))
// Empty Versions map — GetLatestVersion returns errNotFound, arm skips.
svc.SetCertRepo(&mockCertRepo{
Certs: map[string]*domain.ManagedCertificate{},
Versions: map[string][]*domain.CertificateVersion{},
})
violations, err := svc.ValidateCertificate(ctx, evalCert())
if err != nil {
t.Fatalf("ValidateCertificate failed: %v", err)
}
if len(violations) != 0 {
t.Errorf("expected 0 violations when no version exists (nothing to measure), got %d", len(violations))
}
})
t.Run("max_days zero/absent means no enforcement", func(t *testing.T) {
// Even with a version, max_days=0 is a no-op (matches the
// no-threshold-configured guard in the arm).
rule := mkRule("r-cl", domain.PolicyTypeCertificateLifetime, domain.PolicySeverityError, "")
policyRepo := &mockPolicyRepo{
Rules: map[string]*domain.PolicyRule{rule.ID: rule},
Violations: []*domain.PolicyViolation{},
}
svc := NewPolicyService(policyRepo, NewAuditService(&mockAuditRepo{}))
now := time.Now()
svc.SetCertRepo(&mockCertRepo{
Certs: map[string]*domain.ManagedCertificate{},
Versions: map[string][]*domain.CertificateVersion{
"cert-001": {{
CertificateID: "cert-001",
NotBefore: now.AddDate(0, 0, -1),
NotAfter: now.AddDate(10, 0, 0), // 10 years — huge but unchecked
}},
},
})
violations, err := svc.ValidateCertificate(ctx, evalCert())
if err != nil {
t.Fatalf("ValidateCertificate failed: %v", err)
}
if len(violations) != 0 {
t.Errorf("expected 0 violations when max_days absent (no enforcement), got %d", len(violations))
}
})
}
// runEval is a test helper that exercises ValidateCertificate against a
// single-rule configuration and returns the violation slice. Optionally
// wires a cert repo for the CertificateLifetime arm.
func runEval(ctx context.Context, t *testing.T, rule *domain.PolicyRule, cert *domain.ManagedCertificate, certRepo *mockCertRepo) []*domain.PolicyViolation {
t.Helper()
policyRepo := &mockPolicyRepo{
Rules: map[string]*domain.PolicyRule{rule.ID: rule},
Violations: []*domain.PolicyViolation{},
}
svc := NewPolicyService(policyRepo, NewAuditService(&mockAuditRepo{}))
if certRepo != nil {
svc.SetCertRepo(certRepo)
}
violations, err := svc.ValidateCertificate(ctx, cert)
if err != nil {
t.Fatalf("ValidateCertificate failed: %v", err)
}
return violations
}
@@ -0,0 +1,9 @@
-- Rollback migration 000014: drop the policy_violations severity CHECK.
--
-- Drops the named CHECK constraint added by the up migration. The severity
-- column itself stays (it predates this migration — see 000001 line 183),
-- so any application code that reads/writes the column continues to work.
-- Only the DB-level enforcement of the TitleCase allowlist is removed.
ALTER TABLE policy_violations
DROP CONSTRAINT IF EXISTS policy_violations_severity_check;
@@ -0,0 +1,29 @@
-- Migration 000014: CHECK constraint on policy_violations.severity
--
-- Sibling to migration 000013, which added severity + CHECK to policy_rules.
-- policy_violations has carried a severity column since the initial schema
-- (000001, line 183) but without any CHECK. The engine used to hardcode
-- `Warning` on every violation regardless of the triggering rule's severity
-- (see pre-D-008 internal/service/policy.go:evaluateRule), so the column
-- value was uniform by accident of implementation, not by constraint.
--
-- D-008 rewrites evaluateRule to copy rule.Severity into the violation. The
-- engine now writes values drawn from the application-layer PolicySeverity
-- allowlist, but nothing at the DB level prevents a future caller — or a
-- bypassed write from a migration or psql session — from inserting casing
-- drift ('warning', 'ERROR', etc.) and re-opening the same class of bug
-- that D-005 and D-006 closed. This constraint is the defense-in-depth
-- complement to the handler validator.
--
-- Pre-existing seed_demo.sql rows use lowercase severity values. D-008
-- updates those in the same commit so this migration can apply cleanly
-- against both a fresh install and an upgraded install that has already
-- seeded the demo data.
--
-- Named constraint (policy_violations_severity_check) so the down migration
-- can DROP it by name without ambiguity; un-named CHECK constraints use
-- a synthesized PostgreSQL name that varies by environment.
ALTER TABLE policy_violations
ADD CONSTRAINT policy_violations_severity_check
CHECK (severity IN ('Warning', 'Error', 'Critical'));
+20 -14
View File
@@ -12,19 +12,25 @@ VALUES (
'[30, 14, 7, 0]'::jsonb
) ON CONFLICT (id) DO NOTHING;
-- Policy rules: Require owner assignment
-- Severity differentiated per rule to demonstrate the field means something
-- (D-006). The backend CHECK constraint (migration 000013) enforces the
-- TitleCase allowlist Warning/Error/Critical. Type-value drift
-- (ownership/environment/lifetime/renewal_window vs. the engine's TitleCase
-- canonicals) is tracked separately in d/D-008 and intentionally left
-- unchanged in this commit.
-- Policy rules: Require owner assignment, bound environments, cap lifetime,
-- and enforce a renewal lead-time.
--
-- Severity is differentiated per rule (D-006) and the types are now the
-- TitleCase canonicals the engine actually recognizes (D-008). Pre-D-008 the
-- types were lowercase strings (`ownership`, `environment`, `lifetime`,
-- `renewal_window`) that the engine silently dropped through to its
-- default-case error path — the rules looked alive in the GUI but did not
-- enforce anything. The backend CHECK constraint (migration 000013) enforces
-- the TitleCase severity allowlist Warning/Error/Critical. Configs are also
-- reshaped to match the D-008 per-arm schemas so the rules actually exercise
-- the config-consuming paths instead of falling back to the missing-field
-- placeholders.
INSERT INTO policy_rules (id, name, type, config, enabled, severity)
VALUES (
'pr-require-owner',
'require-owner',
'ownership',
'{"requirement": "owner_id must be set"}'::jsonb,
'RequiredMetadata',
'{"required_keys": ["owner"]}'::jsonb,
true,
'Warning'
) ON CONFLICT (id) DO NOTHING;
@@ -34,7 +40,7 @@ INSERT INTO policy_rules (id, name, type, config, enabled, severity)
VALUES (
'pr-allowed-environments',
'allowed-environments',
'environment',
'AllowedEnvironments',
'{"allowed": ["production", "staging", "development"]}'::jsonb,
true,
'Error'
@@ -45,19 +51,19 @@ INSERT INTO policy_rules (id, name, type, config, enabled, severity)
VALUES (
'pr-max-certificate-lifetime',
'max-certificate-lifetime',
'lifetime',
'CertificateLifetime',
'{"max_days": 90}'::jsonb,
true,
'Critical'
) ON CONFLICT (id) DO NOTHING;
-- Policy rules: Minimum renewal window
-- Policy rules: Minimum renewal window (renew at least 14 days before expiry)
INSERT INTO policy_rules (id, name, type, config, enabled, severity)
VALUES (
'pr-min-renewal-window',
'min-renewal-window',
'renewal_window',
'{"min_days": 14}'::jsonb,
'RenewalLeadTime',
'{"lead_time_days": 14}'::jsonb,
true,
'Warning'
) ON CONFLICT (id) DO NOTHING;
+13 -6
View File
@@ -478,13 +478,20 @@ ON CONFLICT (id) DO NOTHING;
-- ============================================================
-- 13. Policy Violations
-- ============================================================
-- D-008: severity values rewritten to TitleCase canonicals (Warning/Error/Critical).
-- Pre-D-008 these rows used lowercase strings ('critical', 'error', 'warning'). Those
-- values were silently tolerated by the pre-D-008 engine, which hardcoded 'Warning'
-- on every new violation regardless of the triggering rule's severity. D-008 rewires
-- evaluateRule to copy rule.Severity into the violation AND migration 000014 adds a
-- CHECK constraint enforcing the TitleCase allowlist at the DB level. Both paths now
-- round-trip correctly against these demo rows.
INSERT INTO policy_violations (id, certificate_id, rule_id, message, severity, created_at) VALUES
('pv-001', 'mc-legacy-prod', 'pr-max-certificate-lifetime', 'Certificate has expired and exceeds maximum lifetime policy', 'critical', NOW() - INTERVAL '3 days'),
('pv-002', 'mc-old-api', 'pr-max-certificate-lifetime', 'Certificate expired 15 days ago', 'critical', NOW() - INTERVAL '15 days'),
('pv-003', 'mc-vpn-prod', 'pr-min-renewal-window', 'Renewal failed within minimum renewal window', 'error', NOW() - INTERVAL '3 days'),
('pv-004', 'mc-mail-prod', 'pr-min-renewal-window', 'Certificate expiring in 5 days, below 14-day minimum window','warning', NOW() - INTERVAL '20 minutes'),
('pv-005', 'mc-wiki-prod', 'pr-max-certificate-lifetime', 'Certificate expired 7 days ago', 'critical', NOW() - INTERVAL '7 days'),
('pv-006', 'mc-compromised', 'pr-min-renewal-window', 'Certificate revoked due to key compromise', 'critical', NOW() - INTERVAL '14 days')
('pv-001', 'mc-legacy-prod', 'pr-max-certificate-lifetime', 'Certificate has expired and exceeds maximum lifetime policy', 'Critical', NOW() - INTERVAL '3 days'),
('pv-002', 'mc-old-api', 'pr-max-certificate-lifetime', 'Certificate expired 15 days ago', 'Critical', NOW() - INTERVAL '15 days'),
('pv-003', 'mc-vpn-prod', 'pr-min-renewal-window', 'Renewal failed within minimum renewal window', 'Error', NOW() - INTERVAL '3 days'),
('pv-004', 'mc-mail-prod', 'pr-min-renewal-window', 'Certificate expiring in 5 days, below 14-day minimum window','Warning', NOW() - INTERVAL '20 minutes'),
('pv-005', 'mc-wiki-prod', 'pr-max-certificate-lifetime', 'Certificate expired 7 days ago', 'Critical', NOW() - INTERVAL '7 days'),
('pv-006', 'mc-compromised', 'pr-min-renewal-window', 'Certificate revoked due to key compromise', 'Critical', NOW() - INTERVAL '14 days')
ON CONFLICT (id) DO NOTHING;
-- ============================================================
+1
View File
@@ -34,6 +34,7 @@ describe('POLICY_TYPES', () => {
'RequiredMetadata',
'AllowedEnvironments',
'RenewalLeadTime',
'CertificateLifetime',
]);
});
+1
View File
@@ -126,6 +126,7 @@ export const POLICY_TYPES = [
'RequiredMetadata',
'AllowedEnvironments',
'RenewalLeadTime',
'CertificateLifetime',
] as const;
export type PolicyType = (typeof POLICY_TYPES)[number];