From 7f2f34fa86d6d0911ba44cd16aa203514fb028ee Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 26 May 2026 22:08:44 -0400 Subject: [PATCH 1/8] feat(iac): add OnBeforeAction FATAL hook to ApplyPlanHooks --- iac/wfctlhelpers/apply.go | 35 +++++++++++++++++ iac/wfctlhelpers/apply_test.go | 68 ++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/iac/wfctlhelpers/apply.go b/iac/wfctlhelpers/apply.go index 3252734a..0ec17995 100644 --- a/iac/wfctlhelpers/apply.go +++ b/iac/wfctlhelpers/apply.go @@ -89,6 +89,25 @@ import ( // successfully mutates cloud-side state. Hooks let wfctl persist state at the // action boundary instead of waiting for the whole plan to finish. type ApplyPlanHooks struct { + // OnBeforeAction fires PRE-DISPATCH for every PlanAction, after the + // per-iteration ctx.Err() check but before JIT substitution / driver + // resolution / cloud-side mutation. The intended use case is policy / + // ownership gates that can deny a record-level change before any + // cloud-side state moves (see workflow/dns/gate + the wfctl dns-policy + // surface). + // + // FATAL semantics (cycle 3.5 I-NEW-2): a non-nil return aborts the + // per-action loop with a wrapped error — no further actions dispatch, + // no further hook invocations fire, and the top-level + // ApplyPlanWithHooks return value is the wrapped error. This is NOT + // best-effort. Operators rely on "policy denied" being a hard-stop; + // silently continuing past a gate denial would defeat the purpose of + // having a gate. + // + // Distinct from OnResourceApplied / OnResourceDeleted (post-dispatch, + // best-effort persistence hooks) — those record state AFTER cloud-side + // work; OnBeforeAction decides whether cloud-side work happens at all. + OnBeforeAction func(context.Context, interfaces.PlanAction) error OnResourceApplied func(context.Context, interfaces.ResourceDriver, interfaces.PlanAction, interfaces.ResourceOutput) error OnResourceDeleted func(context.Context, interfaces.PlanAction) error // OnPlanComplete fires once after the per-action loop reaches its @@ -317,6 +336,22 @@ func applyPlanWithEnvProviderAndHooks( fatalErr = err return } + // OnBeforeAction (FATAL): pre-dispatch policy / ownership gate. + // Fires AFTER ctx.Err() (cancellation always wins over policy) + // but BEFORE jit substitution + driver resolution + cloud-side + // mutation. A non-nil return aborts the whole apply; subsequent + // actions never see OnBeforeAction. Per cycle 3.5 I-NEW-2 the + // error tier is hard-stop, not best-effort — DNS ownership + // denials are exactly the kind of "must not happen" condition + // that silently continuing would mask. + if hooks.OnBeforeAction != nil { + if err := hooks.OnBeforeAction(ctx, action); err != nil { + fatalErr = fmt.Errorf("%s/%s: apply aborted by OnBeforeAction hook: %w", action.Resource.Type, action.Resource.Name, err) + iterErr = err + iterStatus = statusForPreDispatchSkip() + return + } + } // Per-action JIT substitution — resolve ${VAR} / ${MODULE.field} // / ${MODULE.id} in action.Resource.Config against // result.ReplaceIDMap (this-apply Replace ProviderIDs) and diff --git a/iac/wfctlhelpers/apply_test.go b/iac/wfctlhelpers/apply_test.go index 9988c7cf..d98b9e1d 100644 --- a/iac/wfctlhelpers/apply_test.go +++ b/iac/wfctlhelpers/apply_test.go @@ -317,6 +317,74 @@ func (p *selectiveFakeProvider) ResourceDriver(typ string) (interfaces.ResourceD // TestApplyPlan_CtxCancellationStopsLoop verifies the loop respects // context cancellation between actions. Drivers may honor ctx individually, +// TestApplyPlan_OnBeforeAction_abortsFatal pins the OnBeforeAction hook +// contract: a non-nil error from OnBeforeAction is FATAL — it aborts the +// per-action loop with no further actions dispatched, no further hook +// invocations, and a top-level error wrapping the hook's error so callers +// see the policy/gate denial unambiguously. Mirrors the design's Phase 3a +// "OnBeforeAction error tier specified as FATAL" decision (cycle 3.5 I-NEW-2). +func TestApplyPlan_OnBeforeAction_abortsFatal(t *testing.T) { + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("a", "infra.dns")}, + {Action: "create", Resource: spec("b", "infra.dns")}, + }, + } + fp := newFakeProvider() + var beforeCalls int + hooks := ApplyPlanHooks{ + OnBeforeAction: func(_ context.Context, a interfaces.PlanAction) error { + beforeCalls++ + if a.Resource.Name == "a" { + return errors.New("policy denied: a is not delegated for this owner") + } + return nil + }, + } + _, err := ApplyPlanWithHooks(context.Background(), fp, plan, hooks) + if err == nil || !strings.Contains(err.Error(), "policy denied") { + t.Fatalf("expected top-level error wrapping policy denial; got %v", err) + } + if beforeCalls != 1 { + t.Errorf("OnBeforeAction calls = %d; want 1 (abort on first failure, second action not reached)", beforeCalls) + } + if fp.driver.createCount != 0 { + t.Errorf("createCount = %d; want 0 (fatal hook must abort before any driver call)", fp.driver.createCount) + } +} + +// TestApplyPlan_OnBeforeAction_nilAllowsAll pins the success path: when +// OnBeforeAction returns nil for every action, the hook is non-blocking and +// the apply proceeds as if no hook were wired. Catches the regression where +// a nil-return path is misinterpreted as failure due to a stale fatalErr +// assignment. +func TestApplyPlan_OnBeforeAction_nilAllowsAll(t *testing.T) { + plan := &interfaces.IaCPlan{ + Actions: []interfaces.PlanAction{ + {Action: "create", Resource: spec("a", "infra.dns")}, + {Action: "create", Resource: spec("b", "infra.dns")}, + }, + } + fp := newFakeProvider() + var beforeCalls int + hooks := ApplyPlanHooks{ + OnBeforeAction: func(_ context.Context, _ interfaces.PlanAction) error { + beforeCalls++ + return nil + }, + } + _, err := ApplyPlanWithHooks(context.Background(), fp, plan, hooks) + if err != nil { + t.Fatalf("OnBeforeAction returning nil should not abort apply; got %v", err) + } + if beforeCalls != 2 { + t.Errorf("OnBeforeAction calls = %d; want 2 (one per action)", beforeCalls) + } + if fp.driver.createCount != 2 { + t.Errorf("createCount = %d; want 2 (every action dispatches when hook returns nil)", fp.driver.createCount) + } +} + // but the loop itself must check at the iteration boundary so a // long-running multi-action apply terminates promptly on Ctrl-C / deadline. func TestApplyPlan_CtxCancellationStopsLoop(t *testing.T) { From 5d7789c0ebcb83e7cd168dc36422648099d04a97 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 26 May 2026 22:09:04 -0400 Subject: [PATCH 2/8] feat(dns): relocate dns/policy + dns/audit from workflow-plugin-infra/internal --- dns/audit/audit.go | 71 ++++++++++++++++++++++++ dns/audit/audit_test.go | 40 ++++++++++++++ dns/policy/errors.go | 14 +++++ dns/policy/match.go | 58 ++++++++++++++++++++ dns/policy/match_test.go | 33 ++++++++++++ dns/policy/parse.go | 61 +++++++++++++++++++++ dns/policy/parse_test.go | 111 ++++++++++++++++++++++++++++++++++++++ dns/policy/policy.go | 95 ++++++++++++++++++++++++++++++++ dns/policy/policy_test.go | 48 +++++++++++++++++ dns/policy/reader.go | 10 ++++ dns/policy/serialize.go | 44 +++++++++++++++ dns/policy/types.go | 13 +++++ dns/policy/writer.go | 16 ++++++ 13 files changed, 614 insertions(+) create mode 100644 dns/audit/audit.go create mode 100644 dns/audit/audit_test.go create mode 100644 dns/policy/errors.go create mode 100644 dns/policy/match.go create mode 100644 dns/policy/match_test.go create mode 100644 dns/policy/parse.go create mode 100644 dns/policy/parse_test.go create mode 100644 dns/policy/policy.go create mode 100644 dns/policy/policy_test.go create mode 100644 dns/policy/reader.go create mode 100644 dns/policy/serialize.go create mode 100644 dns/policy/types.go create mode 100644 dns/policy/writer.go diff --git a/dns/audit/audit.go b/dns/audit/audit.go new file mode 100644 index 00000000..5a0556a3 --- /dev/null +++ b/dns/audit/audit.go @@ -0,0 +1,71 @@ +package audit + +import ( + "encoding/json" + "os" + "path/filepath" + "time" +) + +type auditEntry struct { + TS string `json:"ts"` + Actor string `json:"actor"` + Zone string `json:"zone"` + Action string `json:"action,omitempty"` // for policy edits + Name string `json:"name,omitempty"` // for apply attempts + RecordType string `json:"record_type,omitempty"` // for apply attempts + Operation string `json:"operation,omitempty"` + Owner string `json:"owner,omitempty"` + Provider string `json:"provider,omitempty"` + Outcome string `json:"outcome,omitempty"` + Error string `json:"error,omitempty"` + PriorSHA string `json:"prior_sha256,omitempty"` + NewSHA string `json:"new_sha256,omitempty"` +} + +func auditPath() string { + base := os.Getenv("XDG_STATE_HOME") + if base == "" { + base = filepath.Join(os.Getenv("HOME"), ".local", "state") + } + return filepath.Join(base, "wfctl", "plugins", "workflow-plugin-infra", "dns-policy-audit.jsonl") +} + +func appendEntry(e auditEntry) error { + p := auditPath() + if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil { + return err + } + f, err := os.OpenFile(p, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) + if err != nil { + return err + } + defer f.Close() + e.TS = time.Now().UTC().Format(time.RFC3339Nano) + b, _ := json.Marshal(e) + _, err = f.Write(append(b, '\n')) + return err +} + +// LogAttempt records a DNS record mutation attempt before the gate decision. +func LogAttempt(actor, zone, name, recordType, operation, owner, provider string) { + _ = appendEntry(auditEntry{ + Actor: actor, Zone: zone, Name: name, RecordType: recordType, + Operation: operation, Owner: owner, Provider: provider, Outcome: "attempted", + }) +} + +// LogOutcome records the gate or apply outcome for a DNS record mutation. +func LogOutcome(actor, zone, name, recordType, outcome, errMsg string) { + _ = appendEntry(auditEntry{ + Actor: actor, Zone: zone, Name: name, RecordType: recordType, + Outcome: outcome, Error: errMsg, + }) +} + +// LogPolicyEdit records a policy write operation (set-policy / transfer-ownership). +func LogPolicyEdit(actor, zone, action, priorSHA, newSHA string) { + _ = appendEntry(auditEntry{ + Actor: actor, Zone: zone, Action: action, PriorSHA: priorSHA, NewSHA: newSHA, + }) +} diff --git a/dns/audit/audit_test.go b/dns/audit/audit_test.go new file mode 100644 index 00000000..6e1f1de9 --- /dev/null +++ b/dns/audit/audit_test.go @@ -0,0 +1,40 @@ +package audit + +import ( + "os" + "strings" + "testing" +) + +func TestAuditLog_AppendsAttemptThenOutcome(t *testing.T) { + tmp := t.TempDir() + t.Setenv("XDG_STATE_HOME", tmp) + LogAttempt("user@host", "example.com", "www", "A", "upsert", "multisite", "digitalocean") + LogOutcome("user@host", "example.com", "www", "A", "success", "") + path := tmp + "/wfctl/plugins/workflow-plugin-infra/dns-policy-audit.jsonl" + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read audit: %v", err) + } + lines := strings.Split(strings.TrimSpace(string(data)), "\n") + if len(lines) != 2 { + t.Errorf("want 2 lines, got %d: %s", len(lines), data) + } +} + +func TestAuditLog_PolicyEdit(t *testing.T) { + tmp := t.TempDir() + t.Setenv("XDG_STATE_HOME", tmp) + LogPolicyEdit("sre@wfctl", "example.com", "set-policy", "abc123", "def456") + path := tmp + "/wfctl/plugins/workflow-plugin-infra/dns-policy-audit.jsonl" + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read audit: %v", err) + } + if !strings.Contains(string(data), "set-policy") { + t.Errorf("action missing from audit: %s", data) + } + if !strings.Contains(string(data), "abc123") { + t.Errorf("prior_sha missing from audit: %s", data) + } +} diff --git a/dns/policy/errors.go b/dns/policy/errors.go new file mode 100644 index 00000000..8a83805c --- /dev/null +++ b/dns/policy/errors.go @@ -0,0 +1,14 @@ +package policy + +import "errors" + +var ( + ErrMultipleDefaults = errors.New("dnspolicy: multiple RRs set d=true") + ErrEmptyOwner = errors.New("dnspolicy: o= field is empty") + // ErrUnknownHeritage is reserved for future use. Parse() currently silently + // skips RRs with unknown heritage values to preserve forward-compatibility; + // a future stricter parser variant may return this error. + ErrUnknownHeritage = errors.New("dnspolicy: unknown heritage value (parser ignored RR)") +) + +const HeritageV1 = "wfinfra-v1" diff --git a/dns/policy/match.go b/dns/policy/match.go new file mode 100644 index 00000000..3ceb6c2d --- /dev/null +++ b/dns/policy/match.go @@ -0,0 +1,58 @@ +package policy + +import "strings" + +// MatchPattern returns true if name matches pattern. +// Pattern syntax: +// +// "@" matches the apex literal "@" +// "*" matches a SINGLE DNS label segment +// "**" matches one or more label segments +// "." matches recursively +// +// All matches are case-sensitive (DNS names are case-insensitive by spec +// but our pattern compare requires lowercase normalization at call sites). +func MatchPattern(pattern, name string) bool { + if pattern == "@" { + return name == "@" + } + if pattern == "**" { + return true + } + if pattern == "*" { + return !strings.Contains(name, ".") && name != "" + } + // Recursive: split on first dot + pParts := strings.SplitN(pattern, ".", 2) + nParts := strings.SplitN(name, ".", 2) + head := pParts[0] + // Head match: literal or single-* or **-spanning + if head == "**" { + // ** consumes anything from here + return true + } + if head == "*" { + if len(nParts) == 0 { + return false + } + // * matches one label; require both have a tail OR both have no tail + if len(pParts) == 1 { // pattern "*" alone (no dot) — handled above; safety + return !strings.Contains(name, ".") + } + if len(nParts) == 1 { + return false // pattern has tail, name doesn't + } + return MatchPattern(pParts[1], nParts[1]) + } + // Literal head + if len(nParts) == 0 || nParts[0] != head { + return false + } + if len(pParts) == 1 { // pattern has no tail + return len(nParts) == 1 + } + if len(nParts) == 1 { + return false // pattern has tail, name doesn't + } + return MatchPattern(pParts[1], nParts[1]) +} diff --git a/dns/policy/match_test.go b/dns/policy/match_test.go new file mode 100644 index 00000000..9aaa1520 --- /dev/null +++ b/dns/policy/match_test.go @@ -0,0 +1,33 @@ +package policy + +import "testing" + +func TestMatchPattern(t *testing.T) { + cases := []struct { + pattern, name string + want bool + }{ + {"www", "www", true}, + {"www", "admin", false}, + {"@", "@", true}, + {"@", "www", false}, + {"*", "www", true}, + {"*", "anything", true}, + {"*", "", false}, // empty name → false (closes plan-cycle-1 m-1) + {"*", "www.sub", false}, // * = single label only + {"_acme-challenge.*", "_acme-challenge.www", true}, + {"_acme-challenge.*", "_acme-challenge.www.sub", false}, // * is single + {"**", "anything.multi.label", true}, // ** spans + {"**", "single", true}, + {"a.**", "a.b.c", true}, + {"a.**", "b.c", false}, + {"tour.*", "tour.bandname", true}, + {"tour.*", "other.bandname", false}, + } + for _, c := range cases { + got := MatchPattern(c.pattern, c.name) + if got != c.want { + t.Errorf("MatchPattern(%q, %q) = %v, want %v", c.pattern, c.name, got, c.want) + } + } +} diff --git a/dns/policy/parse.go b/dns/policy/parse.go new file mode 100644 index 00000000..7c21091d --- /dev/null +++ b/dns/policy/parse.go @@ -0,0 +1,61 @@ +package policy + +import ( + "fmt" + "strings" +) + +// Parse parses TXT RR strings (one per RR) into a Policy. +// Unknown heritage values are silently skipped (forward-compat). +func Parse(zone string, txtRRs []string) (*Policy, error) { + p := &Policy{Zone: zone} + defaultCount := 0 + for _, rr := range txtRRs { + fields := tokenize(rr) + if fields["heritage"] != HeritageV1 { + continue // foreign TXT (SPF, future schema, etc.) + } + owner := strings.TrimSpace(fields["o"]) + if owner == "" { + return nil, fmt.Errorf("%w: rr=%q", ErrEmptyOwner, rr) + } + entry := Entry{ + Owner: owner, + Patterns: splitCSV(fields["p"]), + Types: splitCSV(fields["t"]), + Default: fields["d"] == "true", + } + if entry.Default { + defaultCount++ + if defaultCount > 1 { + return nil, fmt.Errorf("%w: rr=%q", ErrMultipleDefaults, rr) + } + } + p.Entries = append(p.Entries, entry) + } + return p, nil +} + +// tokenize splits "key=value key=value" into a map. +func tokenize(rr string) map[string]string { + out := map[string]string{} + for _, tok := range strings.Fields(rr) { + eq := strings.IndexByte(tok, '=') + if eq < 0 { + continue + } + out[tok[:eq]] = tok[eq+1:] + } + return out +} + +func splitCSV(s string) []string { + if s == "" { + return nil + } + parts := strings.Split(s, ",") + for i, p := range parts { + parts[i] = strings.TrimSpace(p) + } + return parts +} diff --git a/dns/policy/parse_test.go b/dns/policy/parse_test.go new file mode 100644 index 00000000..24859431 --- /dev/null +++ b/dns/policy/parse_test.go @@ -0,0 +1,111 @@ +package policy + +import ( + "errors" + "strings" + "testing" +) + +func TestParse_HappyPath(t *testing.T) { + rrs := []string{ + `heritage=wfinfra-v1 o=sre d=true`, + `heritage=wfinfra-v1 o=multisite p=www,admin,_acme-challenge.www`, + } + p, err := Parse("gocodealone.tech", rrs) + if err != nil { + t.Fatal(err) + } + if p.Zone != "gocodealone.tech" { + t.Errorf("zone=%q", p.Zone) + } + if len(p.Entries) != 2 { + t.Fatalf("entries=%d want 2", len(p.Entries)) + } +} + +func TestParse_IgnoresUnknownHeritage(t *testing.T) { + rrs := []string{ + `heritage=wfinfra-v1 o=sre d=true`, + `v=spf1 -all`, // SPF — ignored + `heritage=wfinfra-v999 o=alien p=*`, // future schema — ignored + } + p, err := Parse("z", rrs) + if err != nil { + t.Fatal(err) + } + if len(p.Entries) != 1 { + t.Errorf("entries=%d want 1", len(p.Entries)) + } +} + +func TestParse_MultipleDefaults(t *testing.T) { + rrs := []string{ + `heritage=wfinfra-v1 o=sre d=true`, + `heritage=wfinfra-v1 o=multisite d=true p=www`, + } + _, err := Parse("z", rrs) + if !errors.Is(err, ErrMultipleDefaults) { + t.Errorf("want ErrMultipleDefaults, got %v", err) + } +} + +func TestParse_EmptyOwner(t *testing.T) { + rrs := []string{`heritage=wfinfra-v1 o= p=www`} + _, err := Parse("z", rrs) + if !errors.Is(err, ErrEmptyOwner) { + t.Errorf("want ErrEmptyOwner, got %v", err) + } +} + +func TestSerialize_DeterministicSort(t *testing.T) { + p := &Policy{ + Zone: "z", + Entries: []Entry{ + {Owner: "multisite", Patterns: []string{"www", "admin", "_acme-challenge.www"}}, + {Owner: "sre", Default: true}, + }, + } + out1, err := Serialize(p) + if err != nil { + t.Fatal(err) + } + out2, _ := Serialize(p) + if strings.Join(out1, "\n") != strings.Join(out2, "\n") { + t.Errorf("serialize not deterministic") + } + // patterns within entry sorted alphabetically + found := false + for _, rr := range out1 { + if strings.Contains(rr, "o=multisite") { + if !strings.Contains(rr, "p=_acme-challenge.www,admin,www") { + t.Errorf("patterns not sorted within entry: %s", rr) + } + found = true + } + } + if !found { + t.Errorf("multisite RR missing from %v", out1) + } +} + +func TestSerialize_MultipleDefaultsRejected(t *testing.T) { + p := &Policy{Zone: "z", Entries: []Entry{{Owner: "a", Default: true}, {Owner: "b", Default: true}}} + _, err := Serialize(p) + if !errors.Is(err, ErrMultipleDefaults) { + t.Errorf("Serialize should refuse multiple defaults, got %v", err) + } +} + +func TestParseSerialize_RoundTrip(t *testing.T) { + rrs := []string{ + `heritage=wfinfra-v1 o=sre d=true`, + `heritage=wfinfra-v1 o=multisite p=admin,www`, + } + p1, _ := Parse("z", rrs) + out1, _ := Serialize(p1) + p2, _ := Parse("z", out1) + out2, _ := Serialize(p2) + if strings.Join(out1, "\n") != strings.Join(out2, "\n") { + t.Errorf("Parse(Serialize(p)) not idempotent\nout1=%v\nout2=%v", out1, out2) + } +} diff --git a/dns/policy/policy.go b/dns/policy/policy.go new file mode 100644 index 00000000..2d2b8771 --- /dev/null +++ b/dns/policy/policy.go @@ -0,0 +1,95 @@ +package policy + +import "fmt" + +var protectedTypes = map[string]bool{"SOA": true, "NS": true} + +// CheckAllowed returns nil if owner may upsert (name, recordType) under this policy. +// Returns an error describing the denial otherwise. +// +// Priority semantics (closes plan-cycle-1 C-3): +// 1. Explicit pattern claims take precedence over default-owner fallback. +// 2. If any owner (including non-caller) has an explicit pattern matching +// (name, recordType), only that owner may mutate. +// 3. Default owner catches only unmatched records. +// 4. SOA/NS protected unless explicitly listed in the owner's Types. +func (p *Policy) CheckAllowed(name, recordType, owner string) error { + // Phase 1: find any explicit pattern claim (any owner) — explicit beats default + var explicitClaimer string + for _, e := range p.Entries { + if e.Default && len(e.Patterns) == 0 { + continue // skip pure default-only entries in phase 1 + } + if matchesEntry(e, name, recordType) { + explicitClaimer = e.Owner + if e.Owner == owner { + if protectedTypes[recordType] && !isProtectedAllowed(e, recordType) { + return fmt.Errorf("dnspolicy: record type %s never delegated (zone-level only)", recordType) + } + return nil // explicit claim by caller → allow + } + } + } + if explicitClaimer != "" { + return fmt.Errorf("dnspolicy: denied — name=%q type=%s owner=%q; explicitly claimed by owner=%q", name, recordType, owner, explicitClaimer) + } + // Phase 2: no explicit claim exists → fall back to default owner if caller is default. + // (Closes plan-cycle-2 I-3) — also apply Types restriction here; non-empty e.Types restricts the default owner too. + for _, e := range p.Entries { + if e.Default && e.Owner == owner { + // Types restriction: empty = all-types-except-protected; non-empty = exact list + if len(e.Types) > 0 { + ok := false + for _, t := range e.Types { + if t == recordType { + ok = true + break + } + } + if !ok { + return fmt.Errorf("dnspolicy: denied — name=%q type=%s owner=%q; default owner restricted to types %v", name, recordType, owner, e.Types) + } + } + if protectedTypes[recordType] && !isProtectedAllowed(e, recordType) { + return fmt.Errorf("dnspolicy: record type %s never delegated (zone-level only)", recordType) + } + return nil + } + } + // Phase 3: no match anywhere → fail-closed + return fmt.Errorf("dnspolicy: denied — name=%q type=%s owner=%q matches no delegate and no default owner exists for this caller", name, recordType, owner) +} + +// matchesEntry returns true if entry's patterns + types cover (name, recordType). +// Does NOT consider e.Default — that's caller's job (see CheckAllowed phase 1 skip). +func matchesEntry(e Entry, name, recordType string) bool { + // Type scoping + if len(e.Types) > 0 { + ok := false + for _, t := range e.Types { + if t == recordType { + ok = true + break + } + } + if !ok { + return false + } + } + // Pattern match (default-only entries with no patterns are handled by caller) + for _, pat := range e.Patterns { + if MatchPattern(pat, name) { + return true + } + } + return false +} + +func isProtectedAllowed(e Entry, recordType string) bool { + for _, t := range e.Types { + if t == recordType { + return true + } + } + return false +} diff --git a/dns/policy/policy_test.go b/dns/policy/policy_test.go new file mode 100644 index 00000000..db69bbae --- /dev/null +++ b/dns/policy/policy_test.go @@ -0,0 +1,48 @@ +package policy + +import ( + "strings" + "testing" +) + +func TestCheckAllowed(t *testing.T) { + p := &Policy{Zone: "z", Entries: []Entry{ + {Owner: "sre", Default: true}, + {Owner: "multisite", Patterns: []string{"www", "admin", "tour.*"}, Types: []string{"A", "CNAME"}}, + }} + + cases := []struct { + name, recordType, owner string + wantErr bool + errSub string + }{ + {"www", "A", "multisite", false, ""}, // pattern + type match + {"www", "A", "sre", true, "denied"}, // owner mismatch (sre is default) + {"bandname", "A", "sre", false, ""}, // sre default catches unmatched + {"bandname", "A", "multisite", true, "denied"}, // no pattern match + {"www", "MX", "multisite", true, "type"}, // type not in list + {"www", "MX", "sre", false, ""}, // sre owns all types (no type restriction) + {"tour.bandname", "CNAME", "multisite", false, ""}, // glob match + {"www", "SOA", "sre", true, "SOA never delegated"}, // SOA always SRE + {"www", "NS", "sre", true, "NS never delegated"}, // NS always SRE + } + for _, c := range cases { + err := p.CheckAllowed(c.name, c.recordType, c.owner) + if (err != nil) != c.wantErr { + t.Errorf("CheckAllowed(%q,%q,%q) err=%v wantErr=%v", c.name, c.recordType, c.owner, err, c.wantErr) + } + if err != nil && c.errSub != "" && !strings.Contains(err.Error(), c.errSub) { + t.Errorf("CheckAllowed(%q,%q,%q) err=%q want substring %q", c.name, c.recordType, c.owner, err, c.errSub) + } + } +} + +func TestCheckAllowed_NoDefaultFailsClosed(t *testing.T) { + p := &Policy{Zone: "z", Entries: []Entry{ + {Owner: "multisite", Patterns: []string{"www"}}, + }} + err := p.CheckAllowed("bandname", "A", "anyone") + if err == nil { + t.Errorf("expected fail-closed denial for unmatched name with zero defaults") + } +} diff --git a/dns/policy/reader.go b/dns/policy/reader.go new file mode 100644 index 00000000..5ab29ade --- /dev/null +++ b/dns/policy/reader.go @@ -0,0 +1,10 @@ +package policy + +import "context" + +// DNSPolicyReader is the narrow interface the gate needs. +// Tests mock this directly; only 2 methods to fake. +type DNSPolicyReader interface { + GetTXT(ctx context.Context, name string) ([]string, error) + UpsertTXT(ctx context.Context, name string, values []string, ttl int) error +} diff --git a/dns/policy/serialize.go b/dns/policy/serialize.go new file mode 100644 index 00000000..d03220ea --- /dev/null +++ b/dns/policy/serialize.go @@ -0,0 +1,44 @@ +package policy + +import ( + "fmt" + "sort" + "strings" +) + +// Serialize emits Policy as deterministically-ordered TXT RR strings. +// Refuses to emit if multiple entries have Default=true. +func Serialize(p *Policy) ([]string, error) { + defaultCount := 0 + for _, e := range p.Entries { + if e.Default { + defaultCount++ + } + } + if defaultCount > 1 { + return nil, fmt.Errorf("%w (Policy has %d defaults; only 1 allowed)", ErrMultipleDefaults, defaultCount) + } + out := make([]string, 0, len(p.Entries)) + for _, e := range p.Entries { + // Sort patterns + types within entry for deterministic hash + pats := append([]string(nil), e.Patterns...) + sort.Strings(pats) + types := append([]string(nil), e.Types...) + sort.Strings(types) + + sb := strings.Builder{} + fmt.Fprintf(&sb, "heritage=%s o=%s", HeritageV1, e.Owner) + if len(pats) > 0 { + fmt.Fprintf(&sb, " p=%s", strings.Join(pats, ",")) + } + if len(types) > 0 { + fmt.Fprintf(&sb, " t=%s", strings.Join(types, ",")) + } + if e.Default { + sb.WriteString(" d=true") + } + out = append(out, sb.String()) + } + sort.Strings(out) // RR-level sort for deterministic hashing + return out, nil +} diff --git a/dns/policy/types.go b/dns/policy/types.go new file mode 100644 index 00000000..c3f6dabc --- /dev/null +++ b/dns/policy/types.go @@ -0,0 +1,13 @@ +package policy + +type Policy struct { + Zone string + Entries []Entry +} + +type Entry struct { + Owner string + Patterns []string + Types []string + Default bool +} diff --git a/dns/policy/writer.go b/dns/policy/writer.go new file mode 100644 index 00000000..a5b9f282 --- /dev/null +++ b/dns/policy/writer.go @@ -0,0 +1,16 @@ +package policy + +import "context" + +// DNSRecordWriter performs arbitrary DNS record mutations (post-gate). +type DNSRecordWriter interface { + UpsertRecord(ctx context.Context, zone, name, recordType, data string, ttl, priority int32) (recordID string, err error) + DeleteRecord(ctx context.Context, zone, name, recordType string) error +} + +// Adapter combines policy R/W and record R/W in one type. +// dnsprovider.NewAdapter returns this combined interface. +type Adapter interface { + DNSPolicyReader + DNSRecordWriter +} From 4cd5727ee85deba8da0563e4d28232ab2ffe6cd4 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 26 May 2026 22:10:57 -0400 Subject: [PATCH 3/8] feat(dns/gate): relocate; adapt to ResourceDriver.Read for TXT scanning --- dns/gate/gate.go | 173 ++++++++++++++++++++++++++++++++++++++++++ dns/gate/gate_test.go | 172 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 345 insertions(+) create mode 100644 dns/gate/gate.go create mode 100644 dns/gate/gate_test.go diff --git a/dns/gate/gate.go b/dns/gate/gate.go new file mode 100644 index 00000000..7daccc72 --- /dev/null +++ b/dns/gate/gate.go @@ -0,0 +1,173 @@ +// Package gate implements pre-apply DNS policy enforcement. Operators +// configure a TXT policy at `_workflow-dns-policy.` declaring which +// owners may upsert which (name, type) tuples; the Gate reads that policy +// at apply time and denies actions that the policy does not permit. +// +// Relocated from workflow-plugin-infra/internal/dnsgate. Adapted to +// dispatch via interfaces.ResourceDriver.Read (not dnspolicy.Adapter) so +// the same gate logic works against any DNS provider that implements the +// strict-contract ResourceDriver interface — not just providers carrying +// the legacy libdns surface. +package gate + +import ( + "context" + "fmt" + "sync" + + "github.com/GoCodeAlone/workflow/dns/policy" + "github.com/GoCodeAlone/workflow/interfaces" +) + +// PolicyName returns the TXT name where policy lives for a zone — the same +// convention enforced by the parser side (workflow/dns/policy.Parse expects +// records named `_workflow-dns-policy.`). +func PolicyName(zone string) string { return "_workflow-dns-policy." + zone } + +// policyCache holds per-zone parsed policies for the lifetime of one Gate +// holder (e.g. one wfctl apply invocation = one *CachingGate instance). +// Avoids re-fetching + re-parsing the TXT policy once per record when the +// surrounding apply touches many records in the same zone. +type policyCache struct { + mu sync.RWMutex + zones map[string]*policy.Policy +} + +// CachingGate is a Gate-call wrapper with per-zone caching. One per +// wfctl apply invocation; releases at end of invocation (no TTL). Use +// this when an apply touches multiple records under the same zone — the +// dns-policy TXT is fetched + parsed exactly once per zone instead of +// once per record. Mirrors the v1 dnsgate.CachingGate pattern. +type CachingGate struct{ c *policyCache } + +// NewCachingGate returns a new CachingGate. +func NewCachingGate() *CachingGate { + return &CachingGate{c: &policyCache{zones: map[string]*policy.Policy{}}} +} + +// Check is the cached entry point — single GetTXT per zone per Gate. +// Returns nil when (zone, name, recordType, owner) is permitted; non-nil +// when the policy denies the action. Fails closed: a missing policy TXT +// (zero policy entries after parse) returns an error rather than allowing. +// +// Reader is the policy.DNSPolicyReader interface — either a libdns adapter +// (legacy) or a *DriverReader (wfctl-driven, this package). The narrow +// 2-method interface means new transports can be added without touching +// the gate logic. +func (g *CachingGate) Check(ctx context.Context, reader policy.DNSPolicyReader, zone, name, recordType, owner string) error { + g.c.mu.RLock() + cached, ok := g.c.zones[zone] + g.c.mu.RUnlock() + if !ok { + rrs, err := reader.GetTXT(ctx, PolicyName(zone)) + if err != nil { + return fmt.Errorf("dnsgate: fetch policy: %w", err) + } + parsed, perr := policy.Parse(zone, rrs) + if perr != nil { + return perr + } + if len(parsed.Entries) == 0 { + return fmt.Errorf("dnsgate: fail-closed — no policy found at %s", PolicyName(zone)) + } + cached = parsed + g.c.mu.Lock() + g.c.zones[zone] = cached + g.c.mu.Unlock() + } + return cached.CheckAllowed(name, recordType, owner) +} + +// Gate is the uncached entry point (one GetTXT per call). Use this for +// one-off invocations (CLI commands, integration tests). For step handlers +// or apply loops processing many records in one go, use NewCachingGate + +// Check so the policy TXT is read at most once per zone. +func Gate(ctx context.Context, reader policy.DNSPolicyReader, zone, name, recordType, owner string) error { + return NewCachingGate().Check(ctx, reader, zone, name, recordType, owner) +} + +// DriverReader adapts an interfaces.ResourceDriver to the +// policy.DNSPolicyReader interface so the Gate can read TXT records via +// the strict-contract driver path. Scans the zone's Outputs["records"] +// for TXT records matching the given name. Read-only: UpsertTXT delegates +// to a sister mutate path (Driver.Update with a synthesized spec); kept +// out of this adapter because the gate only ever reads. The dns-policy +// command surface owns the write path separately. +type DriverReader struct { + // Driver is the resolved ResourceDriver for resource type "infra.dns". + // Caller is responsible for getting the right driver (via + // IaCProvider.ResourceDriver("infra.dns")) before constructing the + // reader — keeps this adapter's concerns narrow. + Driver interfaces.ResourceDriver + // Zone is the FQDN of the zone whose policy is being read. Used as + // ResourceRef.ProviderID — the DNS provider plugins (DO, CF, NC, + // Hover) all accept zone-name-as-ID for the infra.dns resource type. + Zone string +} + +// GetTXT implements policy.DNSPolicyReader. Reads the zone via +// Driver.Read, then scans Outputs["records"] for TXT records whose name +// matches the requested policy name. Returns an empty (nil) slice when +// the zone has no matching TXT records — the caller (Gate.Check) handles +// the "no policy" case by failing closed. +// +// Tolerates both `[]map[string]any` and `[]any` (with element-wise +// type-assertion) for the Outputs["records"] entry — different provider +// plugins surface the records slice with slightly different concrete +// types depending on whether the value passed through a structpb roundtrip +// or stayed Go-native within the same process. Either shape works. +func (r *DriverReader) GetTXT(ctx context.Context, name string) ([]string, error) { + if r.Driver == nil { + return nil, fmt.Errorf("dnsgate.DriverReader: nil Driver") + } + if r.Zone == "" { + return nil, fmt.Errorf("dnsgate.DriverReader: empty Zone") + } + out, err := r.Driver.Read(ctx, interfaces.ResourceRef{Type: "infra.dns", ProviderID: r.Zone}) + if err != nil { + return nil, err + } + if out == nil { + return nil, nil + } + return extractTXTValues(out.Outputs, name), nil +} + +// extractTXTValues handles the two records-slice concrete-type variants +// produced by DNS provider plugins. Tolerant: returns nil for any shape +// that doesn't carry record entries. +func extractTXTValues(outputs map[string]any, recordName string) []string { + if outputs == nil { + return nil + } + var values []string + switch recs := outputs["records"].(type) { + case []map[string]any: + for _, rec := range recs { + if matchTXT(rec, recordName) { + if v, ok := rec["data"].(string); ok { + values = append(values, v) + } + } + } + case []any: + for _, raw := range recs { + rec, ok := raw.(map[string]any) + if !ok { + continue + } + if matchTXT(rec, recordName) { + if v, ok := rec["data"].(string); ok { + values = append(values, v) + } + } + } + } + return values +} + +func matchTXT(rec map[string]any, recordName string) bool { + t, _ := rec["type"].(string) + n, _ := rec["name"].(string) + return t == "TXT" && n == recordName +} diff --git a/dns/gate/gate_test.go b/dns/gate/gate_test.go new file mode 100644 index 00000000..9d652405 --- /dev/null +++ b/dns/gate/gate_test.go @@ -0,0 +1,172 @@ +package gate + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/GoCodeAlone/workflow/dns/policy" + "github.com/GoCodeAlone/workflow/interfaces" +) + +type fakeReader struct { + txtRRs []string + err error +} + +func (f *fakeReader) GetTXT(_ context.Context, _ string) ([]string, error) { + return f.txtRRs, f.err +} +func (f *fakeReader) UpsertTXT(_ context.Context, _ string, _ []string, _ int) error { return nil } + +func TestGate_Allowed(t *testing.T) { + reader := &fakeReader{txtRRs: []string{ + `heritage=wfinfra-v1 o=sre d=true`, + `heritage=wfinfra-v1 o=multisite p=www,admin`, + }} + if err := Gate(context.Background(), reader, "z.com", "www", "A", "multisite"); err != nil { + t.Errorf("expected pass, got %v", err) + } +} + +func TestGate_Denied(t *testing.T) { + reader := &fakeReader{txtRRs: []string{ + `heritage=wfinfra-v1 o=sre d=true`, + `heritage=wfinfra-v1 o=multisite p=www`, + }} + err := Gate(context.Background(), reader, "z.com", "bandname", "A", "multisite") + if err == nil { + t.Errorf("expected denial") + } +} + +func TestGate_FailClosedOnEmptyPolicy(t *testing.T) { + reader := &fakeReader{txtRRs: []string{}} + err := Gate(context.Background(), reader, "z.com", "www", "A", "anyone") + if err == nil { + t.Errorf("expected fail-closed when no policy exists") + } +} + +func TestGate_PropagatesParseError(t *testing.T) { + reader := &fakeReader{txtRRs: []string{ + `heritage=wfinfra-v1 o=sre d=true`, + `heritage=wfinfra-v1 o=multisite d=true p=www`, // two defaults + }} + err := Gate(context.Background(), reader, "z.com", "www", "A", "sre") + if !errors.Is(err, policy.ErrMultipleDefaults) { + t.Errorf("want ErrMultipleDefaults, got %v", err) + } +} + +type countingReader struct { + txtRRs []string + callCounter *int +} + +func (c *countingReader) GetTXT(_ context.Context, _ string) ([]string, error) { + *c.callCounter++ + return c.txtRRs, nil +} +func (c *countingReader) UpsertTXT(_ context.Context, _ string, _ []string, _ int) error { return nil } + +func TestCachingGate_OneGetTXTPerZone(t *testing.T) { + calls := 0 + reader := &countingReader{txtRRs: []string{`heritage=wfinfra-v1 o=sre d=true`}, callCounter: &calls} + g := NewCachingGate() + for i := 0; i < 10; i++ { + if err := g.Check(context.Background(), reader, "z.com", fmt.Sprintf("name%d", i), "A", "sre"); err != nil { + t.Fatalf("call %d: %v", i, err) + } + } + if calls != 1 { + t.Errorf("want 1 GetTXT call across 10 Check invocations; got %d", calls) + } +} + +// ── DriverReader adapter tests ───────────────────────────────────────────────── + +// fakeDriver implements interfaces.ResourceDriver for the DriverReader +// adapter tests. Only Read is meaningful; the other methods return +// zero-value to satisfy the full interface. +type fakeDriver struct { + records []map[string]any + readErr error +} + +func (d *fakeDriver) Create(_ context.Context, _ interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + return nil, nil +} +func (d *fakeDriver) Read(_ context.Context, _ interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { + if d.readErr != nil { + return nil, d.readErr + } + return &interfaces.ResourceOutput{Outputs: map[string]any{"records": d.records}}, nil +} +func (d *fakeDriver) Update(_ context.Context, _ interfaces.ResourceRef, _ interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + return nil, nil +} +func (d *fakeDriver) Delete(_ context.Context, _ interfaces.ResourceRef) error { return nil } +func (d *fakeDriver) Diff(_ context.Context, _ interfaces.ResourceSpec, _ *interfaces.ResourceOutput) (*interfaces.DiffResult, error) { + return &interfaces.DiffResult{}, nil +} +func (d *fakeDriver) HealthCheck(_ context.Context, _ interfaces.ResourceRef) (*interfaces.HealthResult, error) { + return &interfaces.HealthResult{Healthy: true}, nil +} +func (d *fakeDriver) Scale(_ context.Context, _ interfaces.ResourceRef, _ int) (*interfaces.ResourceOutput, error) { + return nil, nil +} +func (d *fakeDriver) SensitiveKeys() []string { return nil } + +// TestDriverReader_GetTXT_extractsMatchingRecord pins the adapter contract: +// only TXT records whose name matches the requested policy name are +// returned. Non-TXT and non-matching records must be silently filtered. +func TestDriverReader_GetTXT_extractsMatchingRecord(t *testing.T) { + d := &fakeDriver{records: []map[string]any{ + {"type": "TXT", "name": "_workflow-dns-policy.z.com", "data": "heritage=wfinfra-v1 o=sre d=true"}, + {"type": "TXT", "name": "_other.z.com", "data": "noise"}, + {"type": "A", "name": "_workflow-dns-policy.z.com", "data": "10.0.0.1"}, + }} + r := &DriverReader{Driver: d, Zone: "z.com"} + got, err := r.GetTXT(context.Background(), "_workflow-dns-policy.z.com") + if err != nil { + t.Fatalf("GetTXT: %v", err) + } + if len(got) != 1 || got[0] != "heritage=wfinfra-v1 o=sre d=true" { + t.Errorf("want exactly the policy TXT; got %v", got) + } +} + +// TestDriverReader_GetTXT_handlesUntypedSlice pins the structpb-roundtrip +// compatibility: when records arrive as []any (post-gRPC unmarshal) the +// adapter must still extract values. Tested at the extractTXTValues +// boundary so it does not depend on a particular fakeDriver shape. +func TestDriverReader_GetTXT_handlesUntypedSlice(t *testing.T) { + rec := []any{ + map[string]any{"type": "TXT", "name": "_workflow-dns-policy.z.com", "data": "heritage=wfinfra-v1 o=sre d=true"}, + } + got := extractTXTValues(map[string]any{"records": rec}, "_workflow-dns-policy.z.com") + if len(got) != 1 || got[0] != "heritage=wfinfra-v1 o=sre d=true" { + t.Errorf("want untyped-slice extraction; got %v", got) + } +} + +func TestDriverReader_GetTXT_returnsErrOnDriverError(t *testing.T) { + sentinel := errors.New("simulated read failure") + d := &fakeDriver{readErr: sentinel} + r := &DriverReader{Driver: d, Zone: "z.com"} + _, err := r.GetTXT(context.Background(), "_workflow-dns-policy.z.com") + if !errors.Is(err, sentinel) { + t.Errorf("want wrapped sentinel; got %v", err) + } +} + +func TestDriverReader_GetTXT_requiresZoneAndDriver(t *testing.T) { + if _, err := (&DriverReader{Driver: nil, Zone: "z"}).GetTXT(context.Background(), "n"); err == nil { + t.Error("want error for nil Driver") + } + if _, err := (&DriverReader{Driver: &fakeDriver{}, Zone: ""}).GetTXT(context.Background(), "n"); err == nil { + t.Error("want error for empty Zone") + } +} From c2fa88b01965bc6ac908b369a365a167de742323 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 26 May 2026 22:19:06 -0400 Subject: [PATCH 4/8] feat(wfctl): add dns-policy command (show/set/transfer-ownership/drift) --- cmd/wfctl/dns_policy.go | 575 +++++++++++++++++++++++++++++++ cmd/wfctl/dns_policy_test.go | 210 +++++++++++ cmd/wfctl/main.go | 1 + cmd/wfctl/plugin_cli_commands.go | 51 +-- dns/audit/audit.go | 43 +++ dns/audit/audit_test.go | 4 +- 6 files changed, 857 insertions(+), 27 deletions(-) create mode 100644 cmd/wfctl/dns_policy.go create mode 100644 cmd/wfctl/dns_policy_test.go diff --git a/cmd/wfctl/dns_policy.go b/cmd/wfctl/dns_policy.go new file mode 100644 index 00000000..4b57f090 --- /dev/null +++ b/cmd/wfctl/dns_policy.go @@ -0,0 +1,575 @@ +package main + +import ( + "context" + "crypto/sha256" + "errors" + "flag" + "fmt" + "os" + "strings" + + "github.com/GoCodeAlone/workflow/dns/audit" + "github.com/GoCodeAlone/workflow/dns/gate" + "github.com/GoCodeAlone/workflow/dns/policy" + "github.com/GoCodeAlone/workflow/interfaces" +) + +// runDNSPolicy implements the `wfctl dns-policy` builtin — the cross-cutting +// orchestrator for DNS ownership policy declared as TXT records at +// `_workflow-dns-policy.`. Per design-guidance §CLI: cross-cutting +// orchestration commands live as wfctl builtins; capability-scoped commands +// stay in plugin cliCommands. dns-policy reads + writes via any provider +// plugin's IaCProvider.ResourceDriver("infra.dns"), so it works across DO / +// CF / NC / Hover without per-provider command duplication. +// +// Subcommands: +// - show — pretty-print parsed policy for a zone +// - set — upsert a policy entry (owner / patterns / types / default) +// - transfer-ownership — rewrite policy so a different owner controls a record +// - drift — compare configured-vs-live policy and report diffs +// +// Each mutating command (set, transfer-ownership) appends a JSONL audit +// trail entry to `${XDG_STATE_HOME}/wfctl/plugins/wfctl/dns-audit.jsonl`. +func runDNSPolicy(args []string) error { + if len(args) < 1 { + return dnsPolicyUsage() + } + switch args[0] { + case "show": + return runDNSPolicyShow(args[1:]) + case "set": + return runDNSPolicySet(args[1:]) + case "transfer-ownership": + return runDNSPolicyTransfer(args[1:]) + case "drift": + return runDNSPolicyDrift(args[1:]) + case "-h", "--help", "help": + return dnsPolicyUsage() + default: + return fmt.Errorf("dns-policy: unknown subcommand %q", args[0]) + } +} + +func dnsPolicyUsage() error { + fmt.Fprintf(flag.CommandLine.Output(), `Usage: wfctl dns-policy [flags] + +Manage DNS ownership policy TXT records via any iac.provider that supports +"infra.dns". Cross-provider; resolved through the workflow config like other +infra commands. + +Subcommands: + show Pretty-print parsed policy for a zone + set Upsert a policy entry (owner/patterns/types/default) + transfer-ownership Rewrite a record's owner via policy update + drift Compare configured vs live policy; report diffs + +Common flags (all subcommands): + --config Config file (default: infra.yaml or config/infra.yaml) + --env Environment name for config resolution + --provider iac.provider module name from config (required) + --zone DNS zone (required) +`) + return fmt.Errorf("missing or unknown subcommand") +} + +// commonDNSPolicyFlags binds the four-flag set that every dns-policy +// subcommand needs. Centralized so adding a new common flag updates every +// subcommand in one place. +type commonDNSPolicyFlags struct { + configFile, envName, providerName, zone string +} + +func bindCommonDNSPolicyFlags(fs *flag.FlagSet, c *commonDNSPolicyFlags) { + fs.StringVar(&c.configFile, "config", "", "Config file") + fs.StringVar(&c.configFile, "c", "", "Config file (short for --config)") + fs.StringVar(&c.envName, "env", "", "Environment name") + fs.StringVar(&c.providerName, "provider", "", "iac.provider module name (required)") + fs.StringVar(&c.zone, "zone", "", "DNS zone (required)") +} + +// resolveDNSPolicyReader resolves a provider via the standard wfctl +// config-loading path, obtains the infra.dns ResourceDriver, and wraps it +// in a *gate.DriverReader that satisfies policy.DNSPolicyReader (both +// GetTXT and UpsertTXT). The returned closer is the IaCProvider plugin +// process shutdown — caller MUST defer it. Mirrors the runInfraImport +// resolution flow at cmd/wfctl/infra.go:1056-1077. +func resolveDNSPolicyReader(ctx context.Context, common *commonDNSPolicyFlags) (*gate.DriverReader, ioCloser, error) { + if common.providerName == "" { + return nil, nil, fmt.Errorf("--provider required") + } + if common.zone == "" { + return nil, nil, fmt.Errorf("--zone required") + } + cfgFile, err := resolveInfraConfigPath(common.configFile) + if err != nil { + return nil, nil, err + } + providerType, providerCfg, err := resolveProviderModuleByName(cfgFile, common.envName, common.providerName) + if err != nil { + return nil, nil, err + } + provider, closer, err := resolveIaCProvider(ctx, providerType, providerCfg) + if err != nil { + return nil, nil, fmt.Errorf("load provider %q: %w", providerType, err) + } + driver, err := provider.ResourceDriver("infra.dns") + if err != nil { + if closer != nil { + _ = closer.Close() + } + return nil, nil, fmt.Errorf("provider %q: resolve infra.dns driver: %w", providerType, err) + } + return &gate.DriverReader{Driver: driver, Zone: common.zone}, closer, nil +} + +// ioCloser is a narrow Close() error interface — the resolveIaCProvider +// closer return is io.Closer-shaped but pulling in the io package across +// every subcommand for one method is more noise than it's worth. +type ioCloser interface { + Close() error +} + +// resolveInfraConfigPath defaults the config path to infra.yaml or +// config/infra.yaml (matches runInfraImport behavior at infra.go:1044). +// Without going through a *flag.FlagSet — used by dns-policy subcommands +// which bind their flags directly. +func resolveInfraConfigPath(configFile string) (string, error) { + if configFile != "" { + if _, err := os.Stat(configFile); err != nil { + return "", fmt.Errorf("config file %q: %w", configFile, err) + } + return configFile, nil + } + for _, candidate := range []string{"infra.yaml", "config/infra.yaml"} { + if _, err := os.Stat(candidate); err == nil { + return candidate, nil + } + } + return "", fmt.Errorf("no config file found; pass --config or place infra.yaml in the working directory") +} + +// ── show ────────────────────────────────────────────────────────────────────── + +func runDNSPolicyShow(args []string) error { + fs := flag.NewFlagSet("dns-policy show", flag.ContinueOnError) + var common commonDNSPolicyFlags + bindCommonDNSPolicyFlags(fs, &common) + var raw bool + fs.BoolVar(&raw, "raw", false, "Print raw TXT RR values instead of parsed output") + if err := fs.Parse(args); err != nil { + return err + } + ctx := context.Background() + reader, closer, err := resolveDNSPolicyReader(ctx, &common) + if err != nil { + return err + } + if closer != nil { + defer closer.Close() + } + policyName := gate.PolicyName(common.zone) + rrs, err := reader.GetTXT(ctx, policyName) + if err != nil { + return fmt.Errorf("dns-policy show: fetch: %w", err) + } + if len(rrs) == 0 { + fmt.Printf("No policy found at %s\n", policyName) + return nil + } + if raw { + for _, r := range rrs { + fmt.Println(r) + } + return nil + } + pol, err := policy.Parse(common.zone, rrs) + if err != nil { + return fmt.Errorf("dns-policy show: parse: %w", err) + } + fmt.Printf("DNS Ownership Policy for zone: %s\n", common.zone) + fmt.Printf("TXT record: %s (%d RR(s))\n", policyName, len(rrs)) + fmt.Println(strings.Repeat("-", 60)) + for _, e := range pol.Entries { + marker := "" + if e.Default { + marker = " [DEFAULT]" + } + fmt.Printf("Owner: %s%s\n", e.Owner, marker) + if len(e.Patterns) > 0 { + fmt.Printf(" Patterns: %s\n", strings.Join(e.Patterns, ", ")) + } else { + fmt.Printf(" Patterns: (catch-all default)\n") + } + if len(e.Types) > 0 { + fmt.Printf(" Types: %s\n", strings.Join(e.Types, ", ")) + } else { + fmt.Printf(" Types: all (except SOA/NS)\n") + } + fmt.Println() + } + return nil +} + +// ── set ─────────────────────────────────────────────────────────────────────── + +func runDNSPolicySet(args []string) error { + fs := flag.NewFlagSet("dns-policy set", flag.ContinueOnError) + var common commonDNSPolicyFlags + bindCommonDNSPolicyFlags(fs, &common) + var owner, patterns, types string + var defaultOwner bool + var ttl int + fs.StringVar(&owner, "owner", "", "Owner name for this policy entry (required)") + fs.StringVar(&patterns, "patterns", "", "Comma-separated name patterns (empty = catch-all default)") + fs.StringVar(&types, "types", "", "Comma-separated record types (empty = all except SOA/NS)") + fs.BoolVar(&defaultOwner, "default", false, "Mark this entry as the default owner (d=true)") + fs.IntVar(&ttl, "ttl", 300, "TTL in seconds for the policy TXT record") + if err := fs.Parse(args); err != nil { + return err + } + if owner == "" { + return fmt.Errorf("dns-policy set requires --owner") + } + ctx := context.Background() + reader, closer, err := resolveDNSPolicyReader(ctx, &common) + if err != nil { + return err + } + if closer != nil { + defer closer.Close() + } + policyName := gate.PolicyName(common.zone) + rrs, err := reader.GetTXT(ctx, policyName) + if err != nil { + return fmt.Errorf("dns-policy set: fetch existing: %w", err) + } + existing, _ := policy.Parse(common.zone, rrs) // tolerate parse errors when overwriting + if existing == nil { + existing = &policy.Policy{Zone: common.zone} + } + entry := policy.Entry{ + Owner: owner, + Patterns: splitCSVDNSPolicy(patterns), + Types: splitCSVDNSPolicy(types), + Default: defaultOwner, + } + // Replace the entry for this owner (idempotent); leave other owners alone. + merged := mergeEntry(existing.Entries, entry) + newRRs, serr := policy.Serialize(&policy.Policy{Zone: common.zone, Entries: merged}) + if serr != nil { + return fmt.Errorf("dns-policy set: serialize: %w", serr) + } + priorSHA := policyDigest(rrs) + newSHA := policyDigest(newRRs) + if err := reader.UpsertTXT(ctx, policyName, newRRs, ttl); err != nil { + return fmt.Errorf("dns-policy set: write: %w", err) + } + audit.LogPolicyEdit(currentActor(), common.zone, "set-policy", priorSHA, newSHA) + fmt.Printf("Updated policy at %s for owner %q\n", policyName, owner) + return nil +} + +// mergeEntry returns existing entries with the new entry replacing any +// entry whose Owner matches. If no existing entry matches, the new entry +// is appended. Order of other entries is preserved. +func mergeEntry(existing []policy.Entry, e policy.Entry) []policy.Entry { + out := make([]policy.Entry, 0, len(existing)+1) + replaced := false + for _, ex := range existing { + if ex.Owner == e.Owner { + out = append(out, e) + replaced = true + continue + } + out = append(out, ex) + } + if !replaced { + out = append(out, e) + } + return out +} + +func splitCSVDNSPolicy(s string) []string { + if s == "" { + return nil + } + parts := strings.Split(s, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + if p = strings.TrimSpace(p); p != "" { + out = append(out, p) + } + } + return out +} + +// ── transfer-ownership ──────────────────────────────────────────────────────── + +func runDNSPolicyTransfer(args []string) error { + fs := flag.NewFlagSet("dns-policy transfer-ownership", flag.ContinueOnError) + var common commonDNSPolicyFlags + bindCommonDNSPolicyFlags(fs, &common) + var name, newOwner string + var ttl int + fs.StringVar(&name, "name", "", "Record name to transfer (required); matches the policy pattern not the literal DNS name") + fs.StringVar(&newOwner, "new-owner", "", "New owner for the matched record (required)") + fs.IntVar(&ttl, "ttl", 300, "TTL in seconds for the policy TXT record") + if err := fs.Parse(args); err != nil { + return err + } + if name == "" { + return fmt.Errorf("dns-policy transfer-ownership requires --name") + } + if newOwner == "" { + return fmt.Errorf("dns-policy transfer-ownership requires --new-owner") + } + ctx := context.Background() + reader, closer, err := resolveDNSPolicyReader(ctx, &common) + if err != nil { + return err + } + if closer != nil { + defer closer.Close() + } + policyName := gate.PolicyName(common.zone) + rrs, err := reader.GetTXT(ctx, policyName) + if err != nil { + return fmt.Errorf("dns-policy transfer-ownership: fetch: %w", err) + } + pol, err := policy.Parse(common.zone, rrs) + if err != nil { + return fmt.Errorf("dns-policy transfer-ownership: parse: %w", err) + } + prevOwner, updated, err := transferPatternOwnership(pol.Entries, name, newOwner) + if err != nil { + return fmt.Errorf("dns-policy transfer-ownership: %w", err) + } + newRRs, serr := policy.Serialize(&policy.Policy{Zone: common.zone, Entries: updated}) + if serr != nil { + return fmt.Errorf("dns-policy transfer-ownership: serialize: %w", serr) + } + priorSHA := policyDigest(rrs) + newSHA := policyDigest(newRRs) + if err := reader.UpsertTXT(ctx, policyName, newRRs, ttl); err != nil { + return fmt.Errorf("dns-policy transfer-ownership: write: %w", err) + } + audit.LogPolicyEdit(currentActor(), common.zone, "transfer-ownership:"+name+":"+prevOwner+"→"+newOwner, priorSHA, newSHA) + fmt.Printf("Transferred %q in zone %s: %s → %s\n", name, common.zone, prevOwner, newOwner) + return nil +} + +// transferPatternOwnership finds the entry currently owning `name` (by +// pattern membership) and moves that single pattern to a new entry under +// `newOwner`. If `name` is not in any entry's pattern list, returns an +// error rather than silently no-oping — operators expect explicit +// feedback when the pattern doesn't exist. +// +// If the transferred pattern was the only pattern on the old entry, the +// old entry is removed entirely. If `newOwner` already has an entry, the +// pattern is appended to its existing pattern list (deduplicated). +func transferPatternOwnership(entries []policy.Entry, name, newOwner string) (prevOwner string, out []policy.Entry, err error) { + out = make([]policy.Entry, 0, len(entries)) + found := false + for _, e := range entries { + idx := indexOf(e.Patterns, name) + if idx == -1 { + out = append(out, e) + continue + } + prevOwner = e.Owner + found = true + // Remove `name` from this entry's patterns. Drop the entry entirely + // if its pattern list becomes empty. + remaining := append([]string(nil), e.Patterns[:idx]...) + remaining = append(remaining, e.Patterns[idx+1:]...) + if len(remaining) > 0 { + e.Patterns = remaining + out = append(out, e) + } + } + if !found { + return "", nil, fmt.Errorf("pattern %q not found in any entry", name) + } + // Append to existing newOwner entry, or create a fresh one. + merged := false + for i := range out { + if out[i].Owner == newOwner { + if indexOf(out[i].Patterns, name) == -1 { + out[i].Patterns = append(out[i].Patterns, name) + } + merged = true + break + } + } + if !merged { + out = append(out, policy.Entry{Owner: newOwner, Patterns: []string{name}}) + } + return prevOwner, out, nil +} + +func indexOf(s []string, target string) int { + for i, v := range s { + if v == target { + return i + } + } + return -1 +} + +// ── drift ───────────────────────────────────────────────────────────────────── + +// runDNSPolicyDrift compares the live policy (as fetched from the +// provider) against an expected policy declared inline via --expect or +// loaded from --expect-file. Reports missing / extra / mismatched +// entries. Read-only: never writes to the zone. +func runDNSPolicyDrift(args []string) error { + fs := flag.NewFlagSet("dns-policy drift", flag.ContinueOnError) + var common commonDNSPolicyFlags + bindCommonDNSPolicyFlags(fs, &common) + var expectFile string + fs.StringVar(&expectFile, "expect-file", "", "Path to expected policy TXT-RR file (one RR per line). Required.") + if err := fs.Parse(args); err != nil { + return err + } + if expectFile == "" { + return fmt.Errorf("dns-policy drift requires --expect-file") + } + expectBytes, err := os.ReadFile(expectFile) + if err != nil { + return fmt.Errorf("dns-policy drift: read %s: %w", expectFile, err) + } + expectedRRs := splitNonEmptyLines(string(expectBytes)) + expected, err := policy.Parse(common.zone, expectedRRs) + if err != nil { + return fmt.Errorf("dns-policy drift: parse expected: %w", err) + } + ctx := context.Background() + reader, closer, err := resolveDNSPolicyReader(ctx, &common) + if err != nil { + return err + } + if closer != nil { + defer closer.Close() + } + policyName := gate.PolicyName(common.zone) + liveRRs, err := reader.GetTXT(ctx, policyName) + if err != nil { + return fmt.Errorf("dns-policy drift: fetch live: %w", err) + } + live, err := policy.Parse(common.zone, liveRRs) + if err != nil { + return fmt.Errorf("dns-policy drift: parse live: %w", err) + } + diffs := comparePolicyEntries(expected.Entries, live.Entries) + if len(diffs) == 0 { + fmt.Printf("No drift detected for %s\n", common.zone) + return nil + } + fmt.Printf("Drift detected for %s:\n", common.zone) + for _, d := range diffs { + fmt.Printf(" %s\n", d) + } + return errors.New("dns-policy drift: differences detected") +} + +func splitNonEmptyLines(s string) []string { + var out []string + for _, line := range strings.Split(s, "\n") { + if line = strings.TrimSpace(line); line != "" { + out = append(out, line) + } + } + return out +} + +// comparePolicyEntries returns human-readable diff strings. Owners present +// in expected but missing in live are MISSING; owners in live but not +// expected are EXTRA; owners in both but with different pattern/type sets +// are MISMATCHED. Default-flag mismatches are surfaced explicitly. +func comparePolicyEntries(expected, live []policy.Entry) []string { + expByOwner := indexByOwner(expected) + liveByOwner := indexByOwner(live) + var diffs []string + for owner, e := range expByOwner { + l, ok := liveByOwner[owner] + if !ok { + diffs = append(diffs, fmt.Sprintf("MISSING: owner=%s patterns=%v types=%v default=%v", owner, e.Patterns, e.Types, e.Default)) + continue + } + if !stringSetEqual(e.Patterns, l.Patterns) { + diffs = append(diffs, fmt.Sprintf("MISMATCH patterns owner=%s expected=%v live=%v", owner, e.Patterns, l.Patterns)) + } + if !stringSetEqual(e.Types, l.Types) { + diffs = append(diffs, fmt.Sprintf("MISMATCH types owner=%s expected=%v live=%v", owner, e.Types, l.Types)) + } + if e.Default != l.Default { + diffs = append(diffs, fmt.Sprintf("MISMATCH default owner=%s expected=%v live=%v", owner, e.Default, l.Default)) + } + } + for owner, l := range liveByOwner { + if _, ok := expByOwner[owner]; !ok { + diffs = append(diffs, fmt.Sprintf("EXTRA: owner=%s patterns=%v types=%v default=%v", owner, l.Patterns, l.Types, l.Default)) + } + } + return diffs +} + +func indexByOwner(entries []policy.Entry) map[string]policy.Entry { + m := make(map[string]policy.Entry, len(entries)) + for _, e := range entries { + m[e.Owner] = e + } + return m +} + +func stringSetEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + set := make(map[string]struct{}, len(a)) + for _, s := range a { + set[s] = struct{}{} + } + for _, s := range b { + if _, ok := set[s]; !ok { + return false + } + } + return true +} + +// policyDigest returns a stable short hash for a policy TXT-RR slice, used +// as the prior/new SHA recorded in the audit trail. SHA-256 of the +// alphabetically sorted joined RRs — invariant to slice order so the same +// policy semantics produce the same digest regardless of provider-side +// record ordering. +func policyDigest(rrs []string) string { + if len(rrs) == 0 { + return "" + } + sorted := append([]string(nil), rrs...) + // Sort in-place; stable order for deterministic hashing. + for i := 1; i < len(sorted); i++ { + for j := i; j > 0 && sorted[j-1] > sorted[j]; j-- { + sorted[j-1], sorted[j] = sorted[j], sorted[j-1] + } + } + h := sha256.Sum256([]byte(strings.Join(sorted, "\n"))) + return fmt.Sprintf("%x", h[:8]) // 16-hex-char short digest is enough for trail readability +} + +// currentActor returns the username for the audit-trail Actor field. +// Falls back to "unknown" if the env doesn't expose USER (CI runners +// without HOME set sometimes leave it blank). +func currentActor() string { + if u := os.Getenv("USER"); u != "" { + return u + } + return "unknown" +} + +// Compile-time guard: ensure interfaces.IaCProvider has the methods this +// file expects. If a future SDK change breaks the contract, this fails +// fast at compile time rather than at runtime. +var _ = (*interfaces.IaCProvider)(nil) diff --git a/cmd/wfctl/dns_policy_test.go b/cmd/wfctl/dns_policy_test.go new file mode 100644 index 00000000..24a48186 --- /dev/null +++ b/cmd/wfctl/dns_policy_test.go @@ -0,0 +1,210 @@ +package main + +import ( + "strings" + "testing" + + "github.com/GoCodeAlone/workflow/dns/policy" +) + +// TestRunDNSPolicy_usage pins the no-arg behavior: print usage + return +// an error so the wfctl dispatcher exits non-zero. +func TestRunDNSPolicy_usage(t *testing.T) { + if err := runDNSPolicy([]string{}); err == nil { + t.Fatal("expected error for no subcommand; got nil") + } +} + +func TestRunDNSPolicy_unknownSubcommand(t *testing.T) { + err := runDNSPolicy([]string{"frobnicate"}) + if err == nil || !strings.Contains(err.Error(), "unknown subcommand") { + t.Fatalf("expected unknown-subcommand error; got %v", err) + } +} + +// TestRunDNSPolicyShow_requiresZone + provider pins the basic flag-gates +// for the show subcommand; resolveDNSPolicyReader returns an error before +// any plugin lookup happens when --zone is missing. Catches the regression +// where the flag-required guard is silently skipped + the empty zone +// reaches the provider plugin as a remote lookup of "". +func TestRunDNSPolicyShow_requiresZone(t *testing.T) { + err := runDNSPolicy([]string{"show", "--provider", "do-prod"}) + if err == nil || !strings.Contains(err.Error(), "--zone") { + t.Fatalf("want --zone required error; got %v", err) + } +} + +func TestRunDNSPolicyShow_requiresProvider(t *testing.T) { + err := runDNSPolicy([]string{"show", "--zone", "z.com"}) + if err == nil || !strings.Contains(err.Error(), "--provider") { + t.Fatalf("want --provider required error; got %v", err) + } +} + +func TestRunDNSPolicySet_requiresOwner(t *testing.T) { + err := runDNSPolicy([]string{"set", "--provider", "do-prod", "--zone", "z.com"}) + if err == nil || !strings.Contains(err.Error(), "--owner") { + t.Fatalf("want --owner required error; got %v", err) + } +} + +func TestRunDNSPolicyTransfer_requiresName(t *testing.T) { + err := runDNSPolicy([]string{"transfer-ownership", "--provider", "do-prod", "--zone", "z.com", "--new-owner", "ratchet"}) + if err == nil || !strings.Contains(err.Error(), "--name") { + t.Fatalf("want --name required error; got %v", err) + } +} + +func TestRunDNSPolicyTransfer_requiresNewOwner(t *testing.T) { + err := runDNSPolicy([]string{"transfer-ownership", "--provider", "do-prod", "--zone", "z.com", "--name", "www"}) + if err == nil || !strings.Contains(err.Error(), "--new-owner") { + t.Fatalf("want --new-owner required error; got %v", err) + } +} + +func TestRunDNSPolicyDrift_requiresExpectFile(t *testing.T) { + err := runDNSPolicy([]string{"drift", "--provider", "do-prod", "--zone", "z.com"}) + if err == nil || !strings.Contains(err.Error(), "--expect-file") { + t.Fatalf("want --expect-file required error; got %v", err) + } +} + +// ── policy-mutation helper tests ────────────────────────────────────────────── + +func TestMergeEntry_replacesSameOwner(t *testing.T) { + existing := []policy.Entry{ + {Owner: "sre", Default: true}, + {Owner: "multisite", Patterns: []string{"www"}}, + } + updated := policy.Entry{Owner: "multisite", Patterns: []string{"www", "admin"}} + out := mergeEntry(existing, updated) + if len(out) != 2 { + t.Fatalf("want 2 entries; got %d: %+v", len(out), out) + } + if out[0].Owner != "sre" { + t.Errorf("first entry owner = %q; want sre (order preserved)", out[0].Owner) + } + if out[1].Owner != "multisite" || len(out[1].Patterns) != 2 { + t.Errorf("multisite entry not updated; got %+v", out[1]) + } +} + +func TestMergeEntry_appendsNewOwner(t *testing.T) { + existing := []policy.Entry{{Owner: "sre", Default: true}} + updated := policy.Entry{Owner: "ratchet", Patterns: []string{"api"}} + out := mergeEntry(existing, updated) + if len(out) != 2 || out[1].Owner != "ratchet" { + t.Errorf("append failed; got %+v", out) + } +} + +func TestTransferPatternOwnership_moves(t *testing.T) { + entries := []policy.Entry{ + {Owner: "multisite", Patterns: []string{"www", "admin"}}, + {Owner: "ratchet", Patterns: []string{"api"}}, + } + prev, out, err := transferPatternOwnership(entries, "www", "ratchet") + if err != nil { + t.Fatalf("transfer: %v", err) + } + if prev != "multisite" { + t.Errorf("prevOwner = %q; want multisite", prev) + } + // multisite should now have only "admin"; ratchet should have "api" + "www" + var multisitePatterns, ratchetPatterns []string + for _, e := range out { + switch e.Owner { + case "multisite": + multisitePatterns = e.Patterns + case "ratchet": + ratchetPatterns = e.Patterns + } + } + if len(multisitePatterns) != 1 || multisitePatterns[0] != "admin" { + t.Errorf("multisite patterns = %v; want [admin]", multisitePatterns) + } + if !containsStringDNSPolicy(ratchetPatterns, "www") || !containsStringDNSPolicy(ratchetPatterns, "api") { + t.Errorf("ratchet patterns = %v; want both [api, www]", ratchetPatterns) + } +} + +func TestTransferPatternOwnership_dropsEmptyEntry(t *testing.T) { + entries := []policy.Entry{ + {Owner: "old", Patterns: []string{"www"}}, // single-pattern entry + {Owner: "new", Patterns: []string{"api"}}, + } + prev, out, err := transferPatternOwnership(entries, "www", "new") + if err != nil { + t.Fatalf("transfer: %v", err) + } + if prev != "old" { + t.Errorf("prevOwner = %q; want old", prev) + } + for _, e := range out { + if e.Owner == "old" { + t.Errorf("emptied 'old' entry should have been dropped; got %+v", e) + } + } +} + +func TestTransferPatternOwnership_errorOnMissingPattern(t *testing.T) { + entries := []policy.Entry{{Owner: "sre", Default: true}} + _, _, err := transferPatternOwnership(entries, "ghost", "ratchet") + if err == nil { + t.Fatal("expected error for missing pattern; got nil") + } +} + +func TestComparePolicyEntries_detectsMissingExtraMismatch(t *testing.T) { + expected := []policy.Entry{ + {Owner: "sre", Default: true}, + {Owner: "multisite", Patterns: []string{"www"}}, + } + live := []policy.Entry{ + {Owner: "sre", Default: false}, // default flag mismatch + {Owner: "ratchet", Patterns: []string{"api"}}, // extra + // multisite missing + } + diffs := comparePolicyEntries(expected, live) + if len(diffs) == 0 { + t.Fatal("expected diffs; got none") + } + joined := strings.Join(diffs, "|") + if !strings.Contains(joined, "MISSING") || !strings.Contains(joined, "multisite") { + t.Errorf("missing-detection failed; diffs=%v", diffs) + } + if !strings.Contains(joined, "EXTRA") || !strings.Contains(joined, "ratchet") { + t.Errorf("extra-detection failed; diffs=%v", diffs) + } + if !strings.Contains(joined, "MISMATCH default") { + t.Errorf("default-mismatch detection failed; diffs=%v", diffs) + } +} + +func TestComparePolicyEntries_noDriftReturnsEmpty(t *testing.T) { + entries := []policy.Entry{ + {Owner: "sre", Default: true}, + {Owner: "multisite", Patterns: []string{"www"}}, + } + diffs := comparePolicyEntries(entries, entries) + if len(diffs) != 0 { + t.Errorf("identical policies should produce zero diffs; got %v", diffs) + } +} + +func TestPolicyDigest_orderIndependent(t *testing.T) { + a := []string{"heritage=wfinfra-v1 o=sre d=true", "heritage=wfinfra-v1 o=multisite p=www"} + b := []string{"heritage=wfinfra-v1 o=multisite p=www", "heritage=wfinfra-v1 o=sre d=true"} + if policyDigest(a) != policyDigest(b) { + t.Errorf("digest should be order-invariant; a=%q b=%q", policyDigest(a), policyDigest(b)) + } +} + +func containsStringDNSPolicy(s []string, target string) bool { + for _, v := range s { + if v == target { + return true + } + } + return false +} diff --git a/cmd/wfctl/main.go b/cmd/wfctl/main.go index 8ca35715..66083ac9 100644 --- a/cmd/wfctl/main.go +++ b/cmd/wfctl/main.go @@ -108,6 +108,7 @@ var commands = map[string]func([]string) error{ "modernize": runModernize, "expr-migrate": runExprMigrate, "infra": runInfra, + "dns-policy": runDNSPolicy, "docs": runDocs, "editor-schemas": runEditorSchemas, "editor-bundle": runEditorBundle, diff --git a/cmd/wfctl/plugin_cli_commands.go b/cmd/wfctl/plugin_cli_commands.go index ff1674c0..b2c09ecd 100644 --- a/cmd/wfctl/plugin_cli_commands.go +++ b/cmd/wfctl/plugin_cli_commands.go @@ -14,31 +14,32 @@ import ( // reservedCLICommands is the set of static wfctl command names that plugins // cannot shadow. Any plugin declaring one of these names is rejected at startup. var reservedCLICommands = map[string]struct{}{ - "plugin": {}, - "build": {}, - "infra": {}, - "ci": {}, - "deploy": {}, - "tenant": {}, - "config": {}, - "api": {}, - "contract": {}, - "diff": {}, - "dev": {}, - "generate": {}, - "git": {}, - "help": {}, - "init": {}, - "inspect": {}, - "list": {}, - "mcp": {}, - "modernize": {}, - "pipeline": {}, - "registry": {}, - "template": {}, - "update": {}, - "validate": {}, - "version": {}, + "plugin": {}, + "build": {}, + "infra": {}, + "dns-policy": {}, + "ci": {}, + "deploy": {}, + "tenant": {}, + "config": {}, + "api": {}, + "contract": {}, + "diff": {}, + "dev": {}, + "generate": {}, + "git": {}, + "help": {}, + "init": {}, + "inspect": {}, + "list": {}, + "mcp": {}, + "modernize": {}, + "pipeline": {}, + "registry": {}, + "template": {}, + "update": {}, + "validate": {}, + "version": {}, } // isReservedCLICommand reports whether name is a reserved static wfctl command. diff --git a/dns/audit/audit.go b/dns/audit/audit.go index 5a0556a3..1b02f24c 100644 --- a/dns/audit/audit.go +++ b/dns/audit/audit.go @@ -23,7 +23,23 @@ type auditEntry struct { NewSHA string `json:"new_sha256,omitempty"` } +// auditPath returns the canonical post-Phase-3a audit-trail path. Relocated +// from the workflow-plugin-infra namespace into the wfctl-builtin namespace +// (the policy command surface moved out of the plugin per design §Phase-3). +// One-time migration from the old path runs at first append (see +// migrateLegacyAuditTrail). func auditPath() string { + base := os.Getenv("XDG_STATE_HOME") + if base == "" { + base = filepath.Join(os.Getenv("HOME"), ".local", "state") + } + return filepath.Join(base, "wfctl", "plugins", "wfctl", "dns-audit.jsonl") +} + +// legacyAuditPath returns the pre-Phase-3a path used by the +// workflow-plugin-infra admincli. Kept exported (lowercase, package-private) +// for migration only; do not write to this path going forward. +func legacyAuditPath() string { base := os.Getenv("XDG_STATE_HOME") if base == "" { base = filepath.Join(os.Getenv("HOME"), ".local", "state") @@ -31,7 +47,34 @@ func auditPath() string { return filepath.Join(base, "wfctl", "plugins", "workflow-plugin-infra", "dns-policy-audit.jsonl") } +// migrateLegacyAuditTrail performs a one-time copy of the pre-relocation +// audit-trail entries into the new path. Idempotent: subsequent calls are +// no-ops once the new path exists. Migration is additive — the legacy file +// is left in place for one release cycle so operators with external readers +// (log shippers, SIEM) see no break. +func migrateLegacyAuditTrail() error { + legacy := legacyAuditPath() + current := auditPath() + if _, err := os.Stat(current); err == nil { + // New file already exists; migration done (or never needed). + return nil + } + legacyData, err := os.ReadFile(legacy) + if err != nil { + // No legacy file — nothing to migrate. Common case on fresh installs. + return nil + } + if err := os.MkdirAll(filepath.Dir(current), 0o700); err != nil { + return err + } + return os.WriteFile(current, legacyData, 0o600) +} + func appendEntry(e auditEntry) error { + // Best-effort one-time migration from the legacy plugin-namespace + // path. Failures here do not block the current write — operators + // without the legacy file are unaffected. + _ = migrateLegacyAuditTrail() p := auditPath() if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil { return err diff --git a/dns/audit/audit_test.go b/dns/audit/audit_test.go index 6e1f1de9..d24c830b 100644 --- a/dns/audit/audit_test.go +++ b/dns/audit/audit_test.go @@ -11,7 +11,7 @@ func TestAuditLog_AppendsAttemptThenOutcome(t *testing.T) { t.Setenv("XDG_STATE_HOME", tmp) LogAttempt("user@host", "example.com", "www", "A", "upsert", "multisite", "digitalocean") LogOutcome("user@host", "example.com", "www", "A", "success", "") - path := tmp + "/wfctl/plugins/workflow-plugin-infra/dns-policy-audit.jsonl" + path := tmp + "/wfctl/plugins/wfctl/dns-audit.jsonl" data, err := os.ReadFile(path) if err != nil { t.Fatalf("read audit: %v", err) @@ -26,7 +26,7 @@ func TestAuditLog_PolicyEdit(t *testing.T) { tmp := t.TempDir() t.Setenv("XDG_STATE_HOME", tmp) LogPolicyEdit("sre@wfctl", "example.com", "set-policy", "abc123", "def456") - path := tmp + "/wfctl/plugins/workflow-plugin-infra/dns-policy-audit.jsonl" + path := tmp + "/wfctl/plugins/wfctl/dns-audit.jsonl" data, err := os.ReadFile(path) if err != nil { t.Fatalf("read audit: %v", err) From bde6703f0068430edf4ae929e4af6bc0b381a582 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 26 May 2026 22:23:25 -0400 Subject: [PATCH 5/8] feat(wfctl): wire dns-gate as OnBeforeAction for infra.dns during apply --- cmd/wfctl/infra_apply.go | 2 + cmd/wfctl/infra_apply_dns_gate.go | 113 ++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 cmd/wfctl/infra_apply_dns_gate.go diff --git a/cmd/wfctl/infra_apply.go b/cmd/wfctl/infra_apply.go index 93992ed2..b817b015 100644 --- a/cmd/wfctl/infra_apply.go +++ b/cmd/wfctl/infra_apply.go @@ -460,6 +460,7 @@ func applyWithProviderAndStore(ctx context.Context, provider interfaces.IaCProvi // goes through wfctlhelpers.ApplyPlanWithHooks (Replace + drift // postcondition + IaCProviderFinalizer fan-out). hooks := statePersistenceHooks(store, secretsProvider, provider, providerType, plan.ID, hydratedOut) + wireDNSGateIntoHooks(&hooks, provider) result, err := applyV2ApplyPlanWithHooksFn(ctx, provider, &plan, hooks) // printDriftReportIfAny surfaces input-drift to the operator on // success OR partial failure — silently no-ops on empty reports. @@ -1603,6 +1604,7 @@ func applyPrecomputedPlanWithStore(ctx context.Context, plan interfaces.IaCPlan, fmt.Printf(" Plan: %d action(s) to execute.\n", len(plan.Actions)) // v2 is the only supported dispatch per ADR 0024 + workflow#699. hooks := statePersistenceHooks(store, secretsProvider, provider, providerType, plan.ID, hydratedOut) + wireDNSGateIntoHooks(&hooks, provider) result, err := applyV2ApplyPlanWithHooksFn(ctx, provider, &plan, hooks) if result != nil { printDriftReportIfAny(w, result) diff --git a/cmd/wfctl/infra_apply_dns_gate.go b/cmd/wfctl/infra_apply_dns_gate.go new file mode 100644 index 00000000..60305100 --- /dev/null +++ b/cmd/wfctl/infra_apply_dns_gate.go @@ -0,0 +1,113 @@ +package main + +import ( + "context" + "fmt" + "os" + + "github.com/GoCodeAlone/workflow/dns/gate" + "github.com/GoCodeAlone/workflow/dns/policy" + "github.com/GoCodeAlone/workflow/iac/wfctlhelpers" + "github.com/GoCodeAlone/workflow/interfaces" +) + +// dnsGateHook returns a wfctlhelpers.ApplyPlanHooks-compatible +// OnBeforeAction function that enforces DNS policy on infra.dns resources +// during `wfctl infra apply`. Wires the workflow/dns/gate package as a +// FATAL pre-apply gate per design §Phase 3a. +// +// Behavior: +// +// - Non-infra.dns actions: pass-through (nil error). +// - WORKFLOW_DNS_OWNER env unset: log a warning and pass-through. Gate +// cannot be enforced without an owner identity; explicit skip is +// better than silently breaking applies that haven't yet adopted the +// policy model. +// - infra.dns action: build a gate.CachingGate (one TXT-read per zone +// for the lifetime of this apply), iterate records in +// action.Resource.Config["records"], and gate-check each +// (record_name, record_type, owner) tuple. ANY denial aborts the +// action (FATAL); the rest of the records under that action are not +// checked because OnBeforeAction itself is fatal and aborts the +// whole apply per the design's hard-stop semantics. +// +// The hook is constructed per-apply so its CachingGate's memo table is +// scoped to one apply invocation — no cross-apply policy bleed. +func dnsGateHook(provider interfaces.IaCProvider) func(context.Context, interfaces.PlanAction) error { + owner := os.Getenv("WORKFLOW_DNS_OWNER") + cachingGate := gate.NewCachingGate() + return func(ctx context.Context, action interfaces.PlanAction) error { + if action.Resource.Type != "infra.dns" { + return nil + } + if owner == "" { + // Surface to stderr so operators see the explicit skip — the + // alternative (silently allow every action) would mask config + // errors; the alternative (block every infra.dns action) would + // break legitimate applies that pre-date Phase 3a. + fmt.Fprintf(os.Stderr, "warning: WORKFLOW_DNS_OWNER not set; skipping DNS policy gate for %s/%s\n", action.Resource.Type, action.Resource.Name) + return nil + } + zone, _ := action.Resource.Config["domain"].(string) + if zone == "" { + // infra.dns ResourceSpec carries the zone in Config["domain"] + // (per DO/CF/NC/Hover plugin configSchema). No fallback because + // ResourceSpec has no ProviderID field; ProviderID lives on + // ResourceState — only available post-apply. + return fmt.Errorf("dns-gate: action %s has no Config.domain; cannot read policy", action.Resource.Name) + } + driver, err := provider.ResourceDriver("infra.dns") + if err != nil { + return fmt.Errorf("dns-gate: resolve infra.dns driver for %s: %w", zone, err) + } + reader := &gate.DriverReader{Driver: driver, Zone: zone} + records := extractDNSRecords(action.Resource.Config["records"]) + for _, rec := range records { + recName, _ := rec["name"].(string) + recType, _ := rec["type"].(string) + if recName == "" || recType == "" { + continue + } + if err := cachingGate.Check(ctx, reader, zone, recName, recType, owner); err != nil { + return fmt.Errorf("dns-gate: zone=%s record=%s/%s owner=%s: %w", zone, recName, recType, owner, err) + } + } + return nil + } +} + +// extractDNSRecords normalises both concrete-type variants of the records +// slice ([]map[string]any and []any-of-map) returned by config parsers. +// Returns nil-safe empty slice when records is missing or has an +// unexpected shape — the gate-check loop then iterates zero times and the +// action passes. +func extractDNSRecords(records any) []map[string]any { + switch v := records.(type) { + case []map[string]any: + return v + case []any: + out := make([]map[string]any, 0, len(v)) + for _, raw := range v { + if rec, ok := raw.(map[string]any); ok { + out = append(out, rec) + } + } + return out + } + return nil +} + +// wireDNSGateIntoHooks attaches the DNS gate as OnBeforeAction on the +// hooks struct returned by statePersistenceHooks. Caller must invoke this +// AFTER constructing the hooks via statePersistenceHooks so the OnBefore +// closure shares the same provider reference. +func wireDNSGateIntoHooks(hooks *wfctlhelpers.ApplyPlanHooks, provider interfaces.IaCProvider) { + hooks.OnBeforeAction = dnsGateHook(provider) +} + +// Compile-time guard that the policy + gate packages stay in dependency +// reach for this file even if the body changes — catches accidental +// import-path drift early. The `_ = policy.HeritageV1` line references a +// stable constant in the policy package; if policy gets refactored to +// move HeritageV1, this line breaks at compile time. +var _ = policy.HeritageV1 From 66c983ede3b219cbce40e3df3428e3fecfc5d5d0 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 26 May 2026 22:24:41 -0400 Subject: [PATCH 6/8] feat(wfctl): register dns-policy in wfctl.yaml command surface --- cmd/wfctl/wfctl.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmd/wfctl/wfctl.yaml b/cmd/wfctl/wfctl.yaml index e9abe0b8..cf5eeb63 100644 --- a/cmd/wfctl/wfctl.yaml +++ b/cmd/wfctl/wfctl.yaml @@ -69,6 +69,8 @@ workflows: description: Convert Go template expressions to expr syntax - name: infra description: Manage infrastructure lifecycle + - name: dns-policy + description: Manage cross-provider DNS ownership policy (show/set/transfer-ownership/drift) - name: docs description: Generate documentation from workflow configs - name: editor-schemas @@ -229,6 +231,10 @@ pipelines: trigger: {type: cli, config: {command: infra}} steps: - {name: run, type: step.cli_invoke, config: {command: infra}} + cmd-dns-policy: + trigger: {type: cli, config: {command: dns-policy}} + steps: + - {name: run, type: step.cli_invoke, config: {command: dns-policy}} cmd-docs: trigger: {type: cli, config: {command: docs}} steps: From d9ff6952d65be53d05555696ac64813d6c748250 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 26 May 2026 22:31:41 -0400 Subject: [PATCH 7/8] fix(dns/gate): add DriverReader.UpsertTXT so it satisfies policy.DNSPolicyReader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR 7's first push left this method out — spec-reviewer caught a broken build: cmd/wfctl/dns_policy.go and cmd/wfctl/infra_apply_dns_gate.go both pass *gate.DriverReader where a policy.DNSPolicyReader is expected, but the production adapter only implemented GetTXT. The package tests pass in isolation because gate_test.go's test fakes (fakeReader, countingReader) implement both halves, masking the regression. Adds UpsertTXT mirroring the design: Read current zone → rewrite records slice replacing TXT entries at the target name → Driver.Update with a synthesized ResourceSpec. Includes the recordsToMaps + upsertTXTInRecords helpers needed for the slice transform. Pins the regression with a compile-time interface assertion: var _ policy.DNSPolicyReader = (*DriverReader)(nil) Verified: - GOWORK=off go build ./... - GOWORK=off go test ./cmd/wfctl/... ./dns/... ./iac/wfctlhelpers/... all green (128s) --- dns/gate/gate.go | 114 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 107 insertions(+), 7 deletions(-) diff --git a/dns/gate/gate.go b/dns/gate/gate.go index 7daccc72..9d9d776a 100644 --- a/dns/gate/gate.go +++ b/dns/gate/gate.go @@ -86,13 +86,16 @@ func Gate(ctx context.Context, reader policy.DNSPolicyReader, zone, name, record return NewCachingGate().Check(ctx, reader, zone, name, recordType, owner) } -// DriverReader adapts an interfaces.ResourceDriver to the -// policy.DNSPolicyReader interface so the Gate can read TXT records via -// the strict-contract driver path. Scans the zone's Outputs["records"] -// for TXT records matching the given name. Read-only: UpsertTXT delegates -// to a sister mutate path (Driver.Update with a synthesized spec); kept -// out of this adapter because the gate only ever reads. The dns-policy -// command surface owns the write path separately. +// DriverReader adapts an interfaces.ResourceDriver to the full +// policy.DNSPolicyReader interface so the Gate can read TXT records AND +// the dns-policy mutating commands (set / transfer-ownership) can write +// them — all via the strict-contract driver path (no libdns dependency). +// +// GetTXT scans Outputs["records"] for TXT records matching the given +// name. UpsertTXT issues a Driver.Update against a synthesized +// ResourceSpec whose records list replaces all TXT entries at the target +// name. Both halves are scoped narrowly to the policy-TXT use case; the +// general DNS-record CRUD surface is `wfctl infra apply`. type DriverReader struct { // Driver is the resolved ResourceDriver for resource type "infra.dns". // Caller is responsible for getting the right driver (via @@ -171,3 +174,100 @@ func matchTXT(rec map[string]any, recordName string) bool { n, _ := rec["name"].(string) return t == "TXT" && n == recordName } + +// UpsertTXT implements the write half of policy.DNSPolicyReader so +// DriverReader satisfies the full interface used by the wfctl dns-policy +// mutating commands AND can be passed to CachingGate.Check (which takes a +// DNSPolicyReader, not a narrower Reader sub-interface). +// +// Strategy: Read the current zone via Driver.Read, rewrite the records +// slice replacing all TXT entries at `name` with the supplied values (one +// TXT RR per value), then call Driver.Update with a synthesized +// ResourceSpec carrying the new records list. +// +// Limitations: the synthesized ResourceSpec carries only `domain` + +// `records` fields. Providers requiring additional zone-level config on +// Update (e.g. CF "type" / "settings") may reject. The narrow scope is +// intentional — this adapter exists to manage the policy TXT specifically, +// not to be a general DNS-record CRUD surface (the `wfctl infra apply` +// path covers general record CRUD via config-declared records). +func (r *DriverReader) UpsertTXT(ctx context.Context, name string, values []string, ttl int) error { + if r.Driver == nil { + return fmt.Errorf("dnsgate.DriverReader: nil Driver") + } + if r.Zone == "" { + return fmt.Errorf("dnsgate.DriverReader: empty Zone") + } + out, err := r.Driver.Read(ctx, interfaces.ResourceRef{Type: "infra.dns", ProviderID: r.Zone}) + if err != nil { + return fmt.Errorf("dnsgate.UpsertTXT: read current zone: %w", err) + } + var records []map[string]any + if out != nil { + records = recordsToMaps(out.Outputs["records"]) + } + updated := upsertTXTInRecords(records, name, values, ttl) + spec := interfaces.ResourceSpec{ + Name: r.Zone, + Type: "infra.dns", + Config: map[string]any{ + "domain": r.Zone, + "records": updated, + }, + } + _, err = r.Driver.Update(ctx, interfaces.ResourceRef{Type: "infra.dns", ProviderID: r.Zone}, spec) + if err != nil { + return fmt.Errorf("dnsgate.UpsertTXT: update zone: %w", err) + } + return nil +} + +// recordsToMaps normalises both concrete-type variants of the records +// slice ([]map[string]any and []any-of-map) into the typed form needed +// for Update. +func recordsToMaps(records any) []map[string]any { + switch v := records.(type) { + case []map[string]any: + return append([]map[string]any(nil), v...) + case []any: + out := make([]map[string]any, 0, len(v)) + for _, raw := range v { + if rec, ok := raw.(map[string]any); ok { + out = append(out, rec) + } + } + return out + } + return nil +} + +// upsertTXTInRecords removes all TXT records at `name` from the slice and +// appends one fresh TXT record per value. Idempotent on the policy shape: +// re-running with the same values produces an equivalent records list +// (modulo slice order). +func upsertTXTInRecords(records []map[string]any, name string, values []string, ttl int) []map[string]any { + out := make([]map[string]any, 0, len(records)) + for _, rec := range records { + t, _ := rec["type"].(string) + n, _ := rec["name"].(string) + if t == "TXT" && n == name { + continue // drop existing TXT at this name; replaced below + } + out = append(out, rec) + } + for _, v := range values { + out = append(out, map[string]any{ + "type": "TXT", + "name": name, + "data": v, + "ttl": ttl, + }) + } + return out +} + +// Compile-time assertion that *DriverReader satisfies the full +// policy.DNSPolicyReader contract (GetTXT + UpsertTXT). Catches a +// regression where one half of the interface gets accidentally removed +// (the failure mode that broke PR 7's first push). +var _ policy.DNSPolicyReader = (*DriverReader)(nil) From 6e8ad7b7ba73104d7487a2ba15d2ba26d6df6a8b Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 26 May 2026 22:43:44 -0400 Subject: [PATCH 8/8] =?UTF-8?q?fix(dns/audit):=20nilerr=20=E2=80=94=20prop?= =?UTF-8?q?agate=20non-NotExist=20legacy-read=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lint nilerr fired at audit.go:65: `legacyData, err := os.ReadFile(legacy); if err != nil { return nil }`. Swallowing all errors is wrong — only the NotExist case represents "no legacy file, nothing to migrate"; permission denied / IO errors should propagate so operators see them rather than silently losing the migration on a transient problem. Wrap propagated errors so the caller knows it came from the legacy-read path specifically. --- dns/audit/audit.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/dns/audit/audit.go b/dns/audit/audit.go index 1b02f24c..567430fd 100644 --- a/dns/audit/audit.go +++ b/dns/audit/audit.go @@ -2,6 +2,7 @@ package audit import ( "encoding/json" + "fmt" "os" "path/filepath" "time" @@ -61,8 +62,14 @@ func migrateLegacyAuditTrail() error { } legacyData, err := os.ReadFile(legacy) if err != nil { - // No legacy file — nothing to migrate. Common case on fresh installs. - return nil + // Legacy file absent — common case on fresh installs. Anything else + // (permission denied, IO error) should propagate so operators see + // it rather than silently losing the migration on a transient + // problem. The nilerr lint catches the alternative. + if os.IsNotExist(err) { + return nil + } + return fmt.Errorf("read legacy audit %s: %w", legacy, err) } if err := os.MkdirAll(filepath.Dir(current), 0o700); err != nil { return err