mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 12:41:30 +00:00
fix(policies): close the D-006 loop — TitleCase seed canonicals + severity-aware, config-consuming rule engine (D-008)
D-008 was a three-part drift in the policy engine that made the
D-005/D-006 remediation cosmetic below the DB layer:
(a) migrations/seed.sql INSERTed rules with pre-D-005 lowercase
types ('ownership', 'environment', 'lifetime', 'renewal_window')
that the handler validator rejects on Create/Update but that
raw SQL INSERTs bypassed entirely. At runtime evaluateRule's
switch fell through to the default "unknown policy rule type"
error branch on every demo rule × every cert × every cycle,
flooding logs while emitting zero violations.
(b) migrations/seed_demo.sql persisted lowercase severity values
('critical', 'error', 'warning') on policy_violations rows.
INSERT succeeded because that column had no CHECK, but any
frontend comparing against the canonical PolicySeverity enum
mis-categorized every seeded violation.
(c) evaluateRule hardcoded Severity: PolicySeverityWarning on
every emitted violation and ignored rule.Config entirely —
so the D-006 per-rule severity column (000013) and every
per-arm Config JSON ({allowed_issuer_ids, allowed_domains,
required_keys, allowed, lead_time_days, max_days}) was dead
data below the evaluation layer.
This commit lands (a)+(b)+(c) atomically. Shipping any subset
leaves the feature half-working.
## Changes
Domain (internal/domain/policy.go):
* Add PolicyTypeCertificateLifetime as the 6th TitleCase canonical.
Pre-D-008 the seeded "max-certificate-lifetime" rule had no engine
arm — routing it through RenewalLeadTime would conflate "how
close to expiry before we renew" with "how long can the cert
possibly be", two distinct semantics. The new type accepts
config {"max_days": int} and flags certs whose
NotAfter - NotBefore exceeds the cap.
Handler validator (internal/api/handler/validation.go):
* ValidatePolicyType allowlist grown to 6 canonicals
(AllowedIssuers, AllowedDomains, RequiredMetadata,
AllowedEnvironments, RenewalLeadTime, CertificateLifetime).
OpenAPI (api/openapi.yaml):
* PolicyType enum grown to match domain.
Frontend (web/src/api/types.ts, types.test.ts):
* POLICY_TYPES tuple gains CertificateLifetime; pin test asserts
all 6 canonicals and rejects casing drift.
Migration 000014 (policy_violations severity CHECK):
* Named CHECK constraint (policy_violations_severity_check)
mirroring 000013's allowlist, defense-in-depth at the DB layer
against future drift from bypassed writes (migrations, psql
sessions, future callers). Symmetric down migration drops by
name.
Seed data:
* migrations/seed.sql rewritten to emit TitleCase canonicals with
per-arm config JSON that actually exercises the config-consuming
paths (not the missing-field backstops):
- pr-require-owner → RequiredMetadata {"required_keys":["owner"]} Warning
- pr-allowed-environments → AllowedEnvironments {"allowed":["production","staging","development"]} Error
- pr-max-certificate-lifetime → CertificateLifetime {"max_days":90} Critical
- pr-min-renewal-window → RenewalLeadTime {"lead_time_days":14} Warning
Severities are now differentiated per rule (D-006 intent).
* migrations/seed_demo.sql violation rows flipped to TitleCase
severity ('Critical', 'Error', 'Warning') so migration 000014
applies cleanly on upgrade paths.
Engine rewrite (internal/service/policy.go):
* evaluateRule rewritten. All six arms now:
1. Parse rule.Config into the per-arm typed struct.
2. Bad JSON → log at ValidateCertificate boundary and skip
this rule (no co-located poisoning of other rules in the
same batch).
3. Empty/null Config → emit the pre-D-008 missing-field
violation (backwards compat invariant — operators who
haven't reconfigured still see the same output).
4. Violations emitted carry rule.Severity (no more hardcoded
Warning); D-006 column is now load-bearing.
* CertificateLifetime arm reads NotBefore/NotAfter from the
certificate's latest version via CertRepo. Injected via
PolicyService.SetCertRepo() setter — avoids churning ~36
NewPolicyService call sites while keeping the lifetime arm
optional (degrades to a log+skip if the setter is not wired).
Server wiring (cmd/server/main.go):
* policyService.SetCertRepo(certRepo) wired after construction.
Tests (internal/service/policy_test.go):
* 25 new subtests across 5 groups:
- TestEvaluateRule_SeverityPassThrough (6): every rule type
emits violations carrying rule.Severity, not hardcoded.
- TestEvaluateRule_ConfigConsumed (12): every per-arm Config
path exercised positive + negative.
- TestEvaluateRule_EmptyConfig_BackCompat (3): empty/null
Config still emits pre-D-008 missing-field violations.
- TestEvaluateRule_BadConfig_SkipsRule: malformed JSON logs
and skips cleanly without poisoning neighbors.
- TestEvaluateRule_CertificateLifetime_RepoScenarios (3):
ok when repo wired, log+skip when not, handles missing
NotBefore/NotAfter edges.
Provenance: D-008 surfaced during D-005/D-006 remediation review
in eef1db0. That commit added persistence and CI pins for the
severity field but did not re-verify the evaluation layer
consumed it; this finding and fix close the audit-process gap.
This commit is contained in:
@@ -3461,6 +3461,7 @@ components:
|
|||||||
- RequiredMetadata
|
- RequiredMetadata
|
||||||
- AllowedEnvironments
|
- AllowedEnvironments
|
||||||
- RenewalLeadTime
|
- RenewalLeadTime
|
||||||
|
- CertificateLifetime
|
||||||
|
|
||||||
PolicySeverity:
|
PolicySeverity:
|
||||||
type: string
|
type: string
|
||||||
|
|||||||
@@ -145,6 +145,7 @@ func main() {
|
|||||||
// Initialize services (following the dependency graph)
|
// Initialize services (following the dependency graph)
|
||||||
auditService := service.NewAuditService(auditRepo)
|
auditService := service.NewAuditService(auditRepo)
|
||||||
policyService := service.NewPolicyService(policyRepo, auditService)
|
policyService := service.NewPolicyService(policyRepo, auditService)
|
||||||
|
policyService.SetCertRepo(certificateRepo) // D-008: CertificateLifetime arm needs CertificateVersion.NotBefore/NotAfter
|
||||||
certificateService := service.NewCertificateService(certificateRepo, policyService, auditService)
|
certificateService := service.NewCertificateService(certificateRepo, policyService, auditService)
|
||||||
notifierRegistry := make(map[string]service.Notifier)
|
notifierRegistry := make(map[string]service.Notifier)
|
||||||
|
|
||||||
|
|||||||
@@ -71,10 +71,11 @@ func ValidatePolicyType(policyType interface{}) error {
|
|||||||
"RequiredMetadata": true,
|
"RequiredMetadata": true,
|
||||||
"AllowedEnvironments": true,
|
"AllowedEnvironments": true,
|
||||||
"RenewalLeadTime": true,
|
"RenewalLeadTime": true,
|
||||||
|
"CertificateLifetime": true,
|
||||||
}
|
}
|
||||||
typeStr := fmt.Sprintf("%v", policyType)
|
typeStr := fmt.Sprintf("%v", policyType)
|
||||||
if !validTypes[typeStr] {
|
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
|
return nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -21,11 +21,12 @@ type PolicyRule struct {
|
|||||||
type PolicyType string
|
type PolicyType string
|
||||||
|
|
||||||
const (
|
const (
|
||||||
PolicyTypeAllowedIssuers PolicyType = "AllowedIssuers"
|
PolicyTypeAllowedIssuers PolicyType = "AllowedIssuers"
|
||||||
PolicyTypeAllowedDomains PolicyType = "AllowedDomains"
|
PolicyTypeAllowedDomains PolicyType = "AllowedDomains"
|
||||||
PolicyTypeRequiredMetadata PolicyType = "RequiredMetadata"
|
PolicyTypeRequiredMetadata PolicyType = "RequiredMetadata"
|
||||||
PolicyTypeAllowedEnvironments PolicyType = "AllowedEnvironments"
|
PolicyTypeAllowedEnvironments PolicyType = "AllowedEnvironments"
|
||||||
PolicyTypeRenewalLeadTime PolicyType = "RenewalLeadTime"
|
PolicyTypeRenewalLeadTime PolicyType = "RenewalLeadTime"
|
||||||
|
PolicyTypeCertificateLifetime PolicyType = "CertificateLifetime"
|
||||||
)
|
)
|
||||||
|
|
||||||
// PolicyViolation records an instance of a certificate violating a policy rule.
|
// PolicyViolation records an instance of a certificate violating a policy rule.
|
||||||
|
|||||||
+206
-49
@@ -2,8 +2,10 @@ package service
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/shankar0123/certctl/internal/domain"
|
"github.com/shankar0123/certctl/internal/domain"
|
||||||
@@ -14,6 +16,11 @@ import (
|
|||||||
type PolicyService struct {
|
type PolicyService struct {
|
||||||
policyRepo repository.PolicyRepository
|
policyRepo repository.PolicyRepository
|
||||||
auditService *AuditService
|
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.
|
// 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.
|
// ValidateCertificate runs all enabled policy rules against a certificate.
|
||||||
func (s *PolicyService) ValidateCertificate(ctx context.Context, cert *domain.ManagedCertificate) ([]*domain.PolicyViolation, error) {
|
func (s *PolicyService) ValidateCertificate(ctx context.Context, cert *domain.ManagedCertificate) ([]*domain.PolicyViolation, error) {
|
||||||
rules, err := s.policyRepo.ListRules(ctx)
|
rules, err := s.policyRepo.ListRules(ctx)
|
||||||
@@ -43,7 +60,7 @@ func (s *PolicyService) ValidateCertificate(ctx context.Context, cert *domain.Ma
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Evaluate rule against certificate
|
// Evaluate rule against certificate
|
||||||
v, err := s.evaluateRule(rule, cert)
|
v, err := s.evaluateRule(ctx, rule, cert)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Error("failed to evaluate rule", "rule_id", rule.ID, "error", err)
|
slog.Error("failed to evaluate rule", "rule_id", rule.ID, "error", err)
|
||||||
continue
|
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.
|
// 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 {
|
switch rule.Type {
|
||||||
case domain.PolicyTypeAllowedIssuers:
|
case domain.PolicyTypeAllowedIssuers:
|
||||||
// Restrict to specific issuers
|
// Config: {"allowed_issuer_ids": ["iss-a", "iss-b"]}
|
||||||
// Note: In a production implementation, we would parse rule.Config to extract parameters
|
// 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 == "" {
|
if cert.IssuerID == "" {
|
||||||
return &domain.PolicyViolation{
|
return s.violation(rule, cert, "certificate has no issuer assigned"), nil
|
||||||
ID: generateID("violation"),
|
}
|
||||||
RuleID: rule.ID,
|
if len(cfg.AllowedIssuerIDs) > 0 && !containsString(cfg.AllowedIssuerIDs, cert.IssuerID) {
|
||||||
CertificateID: cert.ID,
|
return s.violation(rule, cert, fmt.Sprintf("issuer %q is not in the allowed list", cert.IssuerID)), nil
|
||||||
Severity: domain.PolicySeverityWarning,
|
|
||||||
Message: "certificate has no issuer assigned",
|
|
||||||
CreatedAt: time.Now(),
|
|
||||||
}, nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
case domain.PolicyTypeAllowedDomains:
|
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 {
|
if len(cert.SANs) == 0 {
|
||||||
return &domain.PolicyViolation{
|
return s.violation(rule, cert, "certificate has no subject alternative names"), nil
|
||||||
ID: generateID("violation"),
|
}
|
||||||
RuleID: rule.ID,
|
if len(cfg.AllowedDomains) > 0 {
|
||||||
CertificateID: cert.ID,
|
for _, san := range cert.SANs {
|
||||||
Severity: domain.PolicySeverityWarning,
|
if !domainAllowed(san, cfg.AllowedDomains) {
|
||||||
Message: "certificate has no subject alternative names",
|
return s.violation(rule, cert, fmt.Sprintf("SAN %q is not in the allowed domain list", san)), nil
|
||||||
CreatedAt: time.Now(),
|
}
|
||||||
}, nil
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
case domain.PolicyTypeRequiredMetadata:
|
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 {
|
if len(cert.Tags) == 0 {
|
||||||
return &domain.PolicyViolation{
|
return s.violation(rule, cert, "certificate has no tags or metadata"), nil
|
||||||
ID: generateID("violation"),
|
}
|
||||||
RuleID: rule.ID,
|
for _, key := range cfg.RequiredKeys {
|
||||||
CertificateID: cert.ID,
|
if _, ok := cert.Tags[key]; !ok {
|
||||||
Severity: domain.PolicySeverityWarning,
|
return s.violation(rule, cert, fmt.Sprintf("certificate is missing required metadata key %q", key)), nil
|
||||||
Message: "certificate has no tags or metadata",
|
}
|
||||||
CreatedAt: time.Now(),
|
|
||||||
}, nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
case domain.PolicyTypeAllowedEnvironments:
|
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 == "" {
|
if cert.Environment == "" {
|
||||||
return &domain.PolicyViolation{
|
return s.violation(rule, cert, "certificate has no environment assigned"), nil
|
||||||
ID: generateID("violation"),
|
}
|
||||||
RuleID: rule.ID,
|
if len(cfg.Allowed) > 0 && !containsString(cfg.Allowed, cert.Environment) {
|
||||||
CertificateID: cert.ID,
|
return s.violation(rule, cert, fmt.Sprintf("environment %q is not in the allowed list", cert.Environment)), nil
|
||||||
Severity: domain.PolicySeverityWarning,
|
|
||||||
Message: "certificate has no environment assigned",
|
|
||||||
CreatedAt: time.Now(),
|
|
||||||
}, nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
case domain.PolicyTypeRenewalLeadTime:
|
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
|
daysUntilExpiry := time.Until(cert.ExpiresAt).Hours() / 24
|
||||||
if daysUntilExpiry < 30 && daysUntilExpiry > 0 {
|
if daysUntilExpiry < float64(leadDays) && daysUntilExpiry > 0 {
|
||||||
return &domain.PolicyViolation{
|
return s.violation(rule, cert, fmt.Sprintf("certificate expires in %.1f days, plan renewal soon (policy lead time: %d days)", daysUntilExpiry, leadDays)), nil
|
||||||
ID: generateID("violation"),
|
}
|
||||||
RuleID: rule.ID,
|
|
||||||
CertificateID: cert.ID,
|
case domain.PolicyTypeCertificateLifetime:
|
||||||
Severity: domain.PolicySeverityWarning,
|
// Config: {"max_days": 397}
|
||||||
Message: fmt.Sprintf("certificate expires in %.1f days, plan renewal soon", daysUntilExpiry),
|
// Reads NotBefore/NotAfter from the latest CertificateVersion via the
|
||||||
CreatedAt: time.Now(),
|
// injected certRepo. ManagedCertificate exposes ExpiresAt but not the
|
||||||
}, nil
|
// 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:
|
default:
|
||||||
@@ -134,6 +241,56 @@ func (s *PolicyService) evaluateRule(rule *domain.PolicyRule, cert *domain.Manag
|
|||||||
return nil, nil
|
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.
|
// CreateRule stores a new policy rule.
|
||||||
func (s *PolicyService) CreateRule(ctx context.Context, rule *domain.PolicyRule, actor string) error {
|
func (s *PolicyService) CreateRule(ctx context.Context, rule *domain.PolicyRule, actor string) error {
|
||||||
if rule.ID == "" {
|
if rule.ID == "" {
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package service
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -420,3 +421,536 @@ func TestCreatePolicy(t *testing.T) {
|
|||||||
t.Errorf("expected 1 rule in repo, got %d", len(policyRepo.Rules))
|
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
@@ -12,19 +12,25 @@ VALUES (
|
|||||||
'[30, 14, 7, 0]'::jsonb
|
'[30, 14, 7, 0]'::jsonb
|
||||||
) ON CONFLICT (id) DO NOTHING;
|
) ON CONFLICT (id) DO NOTHING;
|
||||||
|
|
||||||
-- Policy rules: Require owner assignment
|
-- Policy rules: Require owner assignment, bound environments, cap lifetime,
|
||||||
-- Severity differentiated per rule to demonstrate the field means something
|
-- and enforce a renewal lead-time.
|
||||||
-- (D-006). The backend CHECK constraint (migration 000013) enforces the
|
--
|
||||||
-- TitleCase allowlist Warning/Error/Critical. Type-value drift
|
-- Severity is differentiated per rule (D-006) and the types are now the
|
||||||
-- (ownership/environment/lifetime/renewal_window vs. the engine's TitleCase
|
-- TitleCase canonicals the engine actually recognizes (D-008). Pre-D-008 the
|
||||||
-- canonicals) is tracked separately in d/D-008 and intentionally left
|
-- types were lowercase strings (`ownership`, `environment`, `lifetime`,
|
||||||
-- unchanged in this commit.
|
-- `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)
|
INSERT INTO policy_rules (id, name, type, config, enabled, severity)
|
||||||
VALUES (
|
VALUES (
|
||||||
'pr-require-owner',
|
'pr-require-owner',
|
||||||
'require-owner',
|
'require-owner',
|
||||||
'ownership',
|
'RequiredMetadata',
|
||||||
'{"requirement": "owner_id must be set"}'::jsonb,
|
'{"required_keys": ["owner"]}'::jsonb,
|
||||||
true,
|
true,
|
||||||
'Warning'
|
'Warning'
|
||||||
) ON CONFLICT (id) DO NOTHING;
|
) ON CONFLICT (id) DO NOTHING;
|
||||||
@@ -34,7 +40,7 @@ INSERT INTO policy_rules (id, name, type, config, enabled, severity)
|
|||||||
VALUES (
|
VALUES (
|
||||||
'pr-allowed-environments',
|
'pr-allowed-environments',
|
||||||
'allowed-environments',
|
'allowed-environments',
|
||||||
'environment',
|
'AllowedEnvironments',
|
||||||
'{"allowed": ["production", "staging", "development"]}'::jsonb,
|
'{"allowed": ["production", "staging", "development"]}'::jsonb,
|
||||||
true,
|
true,
|
||||||
'Error'
|
'Error'
|
||||||
@@ -45,19 +51,19 @@ INSERT INTO policy_rules (id, name, type, config, enabled, severity)
|
|||||||
VALUES (
|
VALUES (
|
||||||
'pr-max-certificate-lifetime',
|
'pr-max-certificate-lifetime',
|
||||||
'max-certificate-lifetime',
|
'max-certificate-lifetime',
|
||||||
'lifetime',
|
'CertificateLifetime',
|
||||||
'{"max_days": 90}'::jsonb,
|
'{"max_days": 90}'::jsonb,
|
||||||
true,
|
true,
|
||||||
'Critical'
|
'Critical'
|
||||||
) ON CONFLICT (id) DO NOTHING;
|
) 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)
|
INSERT INTO policy_rules (id, name, type, config, enabled, severity)
|
||||||
VALUES (
|
VALUES (
|
||||||
'pr-min-renewal-window',
|
'pr-min-renewal-window',
|
||||||
'min-renewal-window',
|
'min-renewal-window',
|
||||||
'renewal_window',
|
'RenewalLeadTime',
|
||||||
'{"min_days": 14}'::jsonb,
|
'{"lead_time_days": 14}'::jsonb,
|
||||||
true,
|
true,
|
||||||
'Warning'
|
'Warning'
|
||||||
) ON CONFLICT (id) DO NOTHING;
|
) ON CONFLICT (id) DO NOTHING;
|
||||||
|
|||||||
@@ -478,13 +478,20 @@ ON CONFLICT (id) DO NOTHING;
|
|||||||
-- ============================================================
|
-- ============================================================
|
||||||
-- 13. Policy Violations
|
-- 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
|
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-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-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-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-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-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-006', 'mc-compromised', 'pr-min-renewal-window', 'Certificate revoked due to key compromise', 'Critical', NOW() - INTERVAL '14 days')
|
||||||
ON CONFLICT (id) DO NOTHING;
|
ON CONFLICT (id) DO NOTHING;
|
||||||
|
|
||||||
-- ============================================================
|
-- ============================================================
|
||||||
|
|||||||
@@ -34,6 +34,7 @@ describe('POLICY_TYPES', () => {
|
|||||||
'RequiredMetadata',
|
'RequiredMetadata',
|
||||||
'AllowedEnvironments',
|
'AllowedEnvironments',
|
||||||
'RenewalLeadTime',
|
'RenewalLeadTime',
|
||||||
|
'CertificateLifetime',
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -126,6 +126,7 @@ export const POLICY_TYPES = [
|
|||||||
'RequiredMetadata',
|
'RequiredMetadata',
|
||||||
'AllowedEnvironments',
|
'AllowedEnvironments',
|
||||||
'RenewalLeadTime',
|
'RenewalLeadTime',
|
||||||
|
'CertificateLifetime',
|
||||||
] as const;
|
] as const;
|
||||||
export type PolicyType = (typeof POLICY_TYPES)[number];
|
export type PolicyType = (typeof POLICY_TYPES)[number];
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user