feat(agentconfig): overlay validation (5/15)

pkg/agentconfig/classify_test.go

@@ -2,6 +2,9 @@ package agentconfig
 
 import (
 	"encoding/json"
+	"fmt"
+	"slices"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -332,3 +335,120 @@ func TestClassifyReenableKeptParts(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, []Change{{Path: "/plugins/x/labels", Safety: Safe, Reason: ChangeReasonDataOnly}}, changes)
 }
+
+// applySafeCases predict, per field, whether one apply_safe host applies a change at path
+// (Classify + WillApply): the field-level rule the UI re-implements (field-access.ts). They
+// are shared with the conformance golden file (conformance_test.go). probes are concrete
+// overlays that change the field; state is "editable" when the host applies every probe,
+// "readonly" when it applies none and "restricted" otherwise.
+var applySafeCases = []struct {
+	name    string
+	trusted []string
+	file    map[string]*Plugin
+	path    string
+	probes  []string
+	state   string
+}{
+	{
+		name: "re-enable, untrusted source",
+		file: map[string]*Plugin{"x": {Enabled: boolPtr(false), Source: srcDisabled}},
+		path: "/plugins/x/enabled", probes: []string{`{"plugins":{"x":{"enabled":true}}}`},
+		state: "readonly",
+	},
+	{
+		name: "re-enable, trusted source", trusted: []string{"ghcr.io/trusted/*"},
+		file: map[string]*Plugin{"x": {Enabled: boolPtr(false), Source: "ghcr.io/trusted/p:v1"}},
+		path: "/plugins/x/enabled", probes: []string{`{"plugins":{"x":{"enabled":true}}}`},
+		state: "editable",
+	},
+	{
+		name: "re-enable keeps untrusted and local policies (TestClassifyReenableKeptParts)", trusted: []string{"ghcr.io/trusted/*"},
+		file: map[string]*Plugin{"x": {Enabled: boolPtr(false), Source: "ghcr.io/trusted/p:v1", Policies: []string{"ghcr.io/evil/pol:v9", "/tmp/local-policy"}}},
+		path: "/plugins/x/enabled", probes: []string{`{"plugins":{"x":{"enabled":true}}}`},
+		state: "readonly",
+	},
+	{
+		name: "re-enable keeps ${env:} references (TestClassifyReenableKeptParts)", trusted: []string{"ghcr.io/trusted/*"},
+		file: map[string]*Plugin{"x": {Enabled: boolPtr(false), Source: "ghcr.io/trusted/p:v1", Config: map[string]string{"token": "${env:DB_TOKEN}"}}},
+		path: "/plugins/x/enabled", probes: []string{`{"plugins":{"x":{"enabled":true}}}`},
+		state: "readonly",
+	},
+	{
+		name: "re-enable keeps a local plugin source (fp 2db275ed2d26)", trusted: []string{"ghcr.io/trusted/*"},
+		file: map[string]*Plugin{"x": {Enabled: boolPtr(false), Source: "./bin/local-plugin"}},
+		path: "/plugins/x/enabled", probes: []string{`{"plugins":{"x":{"enabled":true}}}`},
+		state: "readonly",
+	},
+	{
+		name: "disabling is data-only",
+		file: map[string]*Plugin{"x": {Source: "ghcr.io/other/p:v1"}},
+		path: "/plugins/x/enabled", probes: []string{`{"plugins":{"x":{"enabled":false}}}`},
+		state: "editable",
+	},
+	{
+		name: "a new source needs trusted_sources, but reusing one is already-used",
+		file: map[string]*Plugin{"x": {Source: "ghcr.io/a/x:v1"}, "y": {Source: "ghcr.io/a/y:v1"}},
+		path: "/plugins/x/source",
+		probes: []string{
+			`{"plugins":{"x":{"source":"ghcr.io/a/y:v1"}}}`,
+			`{"plugins":{"x":{"source":"ghcr.io/a/new:v1"}}}`,
+		},
+		state: "restricted",
+	},
+	{
+		name: "a disabled plugin's sources are not already used",
+		file: map[string]*Plugin{"x": {Enabled: boolPtr(false), Source: "ghcr.io/a/x:v1"}},
+		path: "/plugins/x/source", probes: []string{`{"plugins":{"x":{"source":"ghcr.io/a/new:v1"}}}`},
+		state: "readonly",
+	},
+	{
+		name: "config key needs an overridable_config_flags entry",
+		file: map[string]*Plugin{"local-ssh": {Source: "s"}},
+		path: "/plugins/local-ssh/config/host", probes: []string{`{"plugins":{"local-ssh":{"config":{"host":"h"}}}}`},
+		state: "readonly",
+	},
+	{
+		name: "data-only fields",
+		file: map[string]*Plugin{"local-ssh": {Source: "s"}},
+		path: "/plugins/local-ssh/policy_data/threshold", probes: []string{`{"plugins":{"local-ssh":{"policy_data":{"threshold":5}}}}`},
+		state: "editable",
+	},
+}
+
+// applySafeState classifies every probe of a case on an apply_safe host trusting trusted
+// and returns the field state (see applySafeCases). It fails when a probe does not change
+// the field at path.
+func applySafeState(trusted []string, file map[string]*Plugin, path string, probes []string) (string, error) {
+	rc := RemoteConfig{Mode: ModeApplySafe, TrustedSources: trusted}.Normalize(true)
+	applied := 0
+	for _, probe := range probes {
+		changes, err := Classify(Config{Plugins: file}, json.RawMessage(probe), rc)
+		if err != nil {
+			return "", err
+		}
+		if !slices.ContainsFunc(changes, func(c Change) bool { return c.Path == path || strings.HasPrefix(path, c.Path+"/") }) {
+			return "", fmt.Errorf("probe %s does not change %s: %v", probe, path, changes)
+		}
+		if ok, _ := WillApply(rc, changes); ok {
+			applied++
+		}
+	}
+	switch applied {
+	case len(probes):
+		return "editable", nil
+	case 0:
+		return "readonly", nil
+	default:
+		return "restricted", nil
+	}
+}
+
+func TestClassifyApplySafeFields(t *testing.T) {
+	for _, tt := range applySafeCases {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := applySafeState(tt.trusted, tt.file, tt.path, tt.probes)
+			require.NoError(t, err)
+			assert.Equal(t, tt.state, got)
+		})
+	}
+}

pkg/agentconfig/conformance_test.go

@@ -0,0 +1,163 @@
+package agentconfig
+
+import (
+	"bytes"
+	"encoding/json"
+	"flag"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// conformanceGolden is the conformance file for the clients that re-implement some of this
+// package's rules (the UI: glob.ts, cron5.ts, field-access.ts, validation.ts). It is
+// generated from the same tables the unit tests use, with every expected value computed by
+// the real functions, and TestConformanceGolden fails when it is stale.
+const conformanceGolden = "testdata/conformance.json"
+
+var updateConformance = flag.Bool("update", false, "rewrite "+conformanceGolden+" (TestConformanceGolden)")
+
+type conformanceDoc struct {
+	Comment        string `json:"_comment"`
+	Source         string `json:"_source"`
+	TrustedSources struct {
+		Patterns []string                   `json:"patterns"`
+		Cases    [][2]any                   `json:"cases"`
+		Extra    []conformanceTrustedSource `json:"extra"`
+	} `json:"trustedSources"`
+	OverridableConfigFlags []conformanceConfigFlag `json:"overridableConfigFlags"`
+	SourceKinds            [][2]string             `json:"sourceKinds"`
+	Schedules              conformanceValidity     `json:"schedules"`
+	ApplySafe              struct {
+		Comment string                 `json:"_comment"`
+		Cases   []conformanceApplySafe `json:"cases"`
+	} `json:"applySafe"`
+	PluginNames conformanceValidity `json:"pluginNames"`
+}
+
+type conformanceTrustedSource struct {
+	Patterns []string `json:"patterns"`
+	Source   string   `json:"source"`
+	Want     bool     `json:"want"`
+}
+
+type conformanceConfigFlag struct {
+	Name   string   `json:"name"`
+	Flags  []string `json:"flags"`
+	Plugin string   `json:"plugin"`
+	Key    string   `json:"key"`
+	Want   bool     `json:"want"`
+}
+
+type conformanceValidity struct {
+	Valid   []string `json:"valid"`
+	Invalid []string `json:"invalid"`
+}
+
+type conformanceApplySafe struct {
+	Name    string             `json:"name"`
+	Trusted []string           `json:"trusted"`
+	File    map[string]*Plugin `json:"file"`
+	Overlay json.RawMessage    `json:"overlay"` // the UI's draft overlay: always null here
+	Path    string             `json:"path"`
+	State   string             `json:"state"`
+}
+
+func nonNilStrings(s []string) []string {
+	if s == nil {
+		return []string{}
+	}
+	return s
+}
+
+// conformanceDocument builds the golden file from the unit-test tables, computing every
+// expected value with the real functions.
+func conformanceDocument() ([]byte, error) {
+	var doc conformanceDoc
+	doc.Comment = "Expected results of the pkg/agentconfig rules that clients re-implement (the UI's glob.ts, cron5.ts, field-access.ts, validation.ts). " +
+		"Generated from the API's own test tables (remoteconfig_test.go, sources_test.go incl. TestNamePatterns, cron_test.go, classify_test.go), " +
+		"every expected value computed by the real functions. Do not edit: regenerate with go test ./pkg/agentconfig -run TestConformanceGolden -update."
+	doc.Source = "compliance-framework/api pkg/agentconfig/testdata/conformance.json"
+
+	doc.TrustedSources.Patterns = trustedSourcePatterns
+	rc := RemoteConfig{TrustedSources: trustedSourcePatterns}
+	for _, c := range trustedSourceCases {
+		doc.TrustedSources.Cases = append(doc.TrustedSources.Cases, [2]any{c.source, MatchTrustedSource(rc, c.source)})
+	}
+	for _, c := range trustedSourceExtraCases {
+		doc.TrustedSources.Extra = append(doc.TrustedSources.Extra, conformanceTrustedSource{
+			Patterns: nonNilStrings(c.patterns),
+			Source:   c.source,
+			Want:     MatchTrustedSource(RemoteConfig{TrustedSources: c.patterns}, c.source),
+		})
+	}
+
+	for _, c := range overridableConfigFlagCases {
+		doc.OverridableConfigFlags = append(doc.OverridableConfigFlags, conformanceConfigFlag{
+			Name: c.name, Flags: nonNilStrings(c.flags), Plugin: c.plugin, Key: c.key,
+			Want: MatchOverridableConfigFlag(RemoteConfig{OverridableConfigFlags: c.flags}, c.plugin, c.key),
+		})
+	}
+
+	for _, c := range sourceKindCases {
+		doc.SourceKinds = append(doc.SourceKinds, [2]string{c.source, string(KindOf(c.source))})
+	}
+
+	doc.Schedules = conformanceValidity{Valid: []string{}, Invalid: []string{}}
+	for _, expr := range append(append([]string{}, schedulesValid...), schedulesInvalid...) {
+		if _, err := ParseSchedule(expr); err == nil {
+			doc.Schedules.Valid = append(doc.Schedules.Valid, expr)
+		} else {
+			doc.Schedules.Invalid = append(doc.Schedules.Invalid, expr)
+		}
+	}
+
+	doc.PluginNames = conformanceValidity{Valid: []string{}, Invalid: []string{}}
+	for _, name := range append(append([]string{}, pluginNamesValid...), pluginNamesInvalid...) {
+		if PluginNamePattern.MatchString(name) {
+			doc.PluginNames.Valid = append(doc.PluginNames.Valid, name)
+		} else {
+			doc.PluginNames.Invalid = append(doc.PluginNames.Invalid, name)
+		}
+	}
+
+	doc.ApplySafe.Comment = "classify_test.go applySafeCases: whether an apply_safe host applies a change at `path` (Classify + WillApply over the case's probe overlays), for the field-level prediction of field-access.ts. `state` is fieldAccess over that one host: editable (every probe applies), restricted (some do), readonly (none do)."
+	for _, c := range applySafeCases {
+		state, err := applySafeState(c.trusted, c.file, c.path, c.probes)
+		if err != nil {
+			return nil, err
+		}
+		doc.ApplySafe.Cases = append(doc.ApplySafe.Cases, conformanceApplySafe{
+			Name: c.name, Trusted: nonNilStrings(c.trusted), File: c.file,
+			Overlay: json.RawMessage("null"), Path: c.path, State: state,
+		})
+	}
+
+	var buf bytes.Buffer
+	enc := json.NewEncoder(&buf)
+	enc.SetEscapeHTML(false)
+	enc.SetIndent("", "  ")
+	if err := enc.Encode(doc); err != nil {
+		return nil, err
+	}
+	return buf.Bytes(), nil
+}
+
+// TestConformanceGolden fails when testdata/conformance.json no longer matches the rules.
+// Regenerate it with: go test ./pkg/agentconfig -run TestConformanceGolden -update
+func TestConformanceGolden(t *testing.T) {
+	got, err := conformanceDocument()
+	require.NoError(t, err)
+	if *updateConformance {
+		require.NoError(t, os.MkdirAll(filepath.Dir(conformanceGolden), 0o755))
+		require.NoError(t, os.WriteFile(conformanceGolden, got, 0o644))
+		return
+	}
+	want, err := os.ReadFile(conformanceGolden)
+	require.NoError(t, err, "run: go test ./pkg/agentconfig -run TestConformanceGolden -update")
+	assert.Equal(t, string(want), string(got),
+		"%s is stale: a rule or its test table changed. Regenerate it with go test ./pkg/agentconfig -run TestConformanceGolden -update, and update the clients that consume it", conformanceGolden)
+}

pkg/agentconfig/cron_test.go

@@ -0,0 +1,32 @@
+package agentconfig
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// schedulesValid and schedulesInvalid are shared with the conformance golden file
+// (conformance_test.go). robfig/cron panics on the invalid time-zone prefixes; ParseSchedule
+// must return an error instead.
+var (
+	schedulesValid   = []string{"TZ=UTC 0 * * * *", "CRON_TZ=Europe/London 0 * * * *", "*/5 * * * *", "@hourly"}
+	schedulesInvalid = []string{"TZ=UTC", "CRON_TZ=UTC", "TZ=", "CRON_TZ="}
+)
+
+func TestParseScheduleTimeZonePrefix(t *testing.T) {
+	for _, expr := range schedulesValid {
+		_, err := ParseSchedule(expr)
+		assert.NoError(t, err, expr)
+	}
+	for _, expr := range schedulesInvalid {
+		require.NotPanics(t, func() {
+			_, err := ParseSchedule(expr)
+			assert.Error(t, err, expr)
+		}, expr)
+	}
+	// The overlay rule O7 reports it as a validation error, not a crash.
+	err := ValidateOverlay([]byte(`{"plugins":{"p":{"schedule":"TZ=UTC"}}}`))
+	requireFieldError(t, err, "/plugins/p/schedule", FieldCodeCron)
+}

pkg/agentconfig/errors.go

@@ -1,7 +1,9 @@
 package agentconfig
 
 import (
+	"cmp"
 	"fmt"
+	"slices"
 	"strings"
 )
 
@@ -53,3 +55,20 @@ func (v ValidationErrors) Error() string {
 	}
 	return strings.Join(parts, "; ")
 }
+
+// sortFieldErrors orders errors by path, then code, then message, and drops exact
+// duplicates.
+func sortFieldErrors(errs []FieldError) []FieldError {
+	slices.SortFunc(errs, func(a, b FieldError) int {
+		return cmp.Or(strings.Compare(a.Path, b.Path), strings.Compare(a.Code, b.Code), strings.Compare(a.Message, b.Message))
+	})
+	return slices.Compact(errs)
+}
+
+// asError returns nil for an empty list and the sorted ValidationErrors otherwise.
+func asError(errs []FieldError) error {
+	if len(errs) == 0 {
+		return nil
+	}
+	return ValidationErrors(sortFieldErrors(errs))
+}