mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 16:31:33 +00:00
shankar0123
b3cc7cbdb2
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.
70 lines
2.2 KiB
SQL
70 lines
2.2 KiB
SQL
-- Seed data for certificate control plane
|
|
|
|
-- Default renewal policy
|
|
INSERT INTO renewal_policies (id, name, renewal_window_days, auto_renew, max_retries, retry_interval_minutes, alert_thresholds_days)
|
|
VALUES (
|
|
'rp-default',
|
|
'default',
|
|
30,
|
|
true,
|
|
3,
|
|
60,
|
|
'[30, 14, 7, 0]'::jsonb
|
|
) ON CONFLICT (id) DO NOTHING;
|
|
|
|
-- 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',
|
|
'RequiredMetadata',
|
|
'{"required_keys": ["owner"]}'::jsonb,
|
|
true,
|
|
'Warning'
|
|
) ON CONFLICT (id) DO NOTHING;
|
|
|
|
-- Policy rules: Allowed environments
|
|
INSERT INTO policy_rules (id, name, type, config, enabled, severity)
|
|
VALUES (
|
|
'pr-allowed-environments',
|
|
'allowed-environments',
|
|
'AllowedEnvironments',
|
|
'{"allowed": ["production", "staging", "development"]}'::jsonb,
|
|
true,
|
|
'Error'
|
|
) ON CONFLICT (id) DO NOTHING;
|
|
|
|
-- Policy rules: Maximum certificate lifetime
|
|
INSERT INTO policy_rules (id, name, type, config, enabled, severity)
|
|
VALUES (
|
|
'pr-max-certificate-lifetime',
|
|
'max-certificate-lifetime',
|
|
'CertificateLifetime',
|
|
'{"max_days": 90}'::jsonb,
|
|
true,
|
|
'Critical'
|
|
) ON CONFLICT (id) DO NOTHING;
|
|
|
|
-- 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',
|
|
'RenewalLeadTime',
|
|
'{"lead_time_days": 14}'::jsonb,
|
|
true,
|
|
'Warning'
|
|
) ON CONFLICT (id) DO NOTHING;
|