diff --git a/pkg/policydomain/lint/diagnostic.go b/pkg/policydomain/lint/diagnostic.go index 7953c47..79ddbd8 100644 --- a/pkg/policydomain/lint/diagnostic.go +++ b/pkg/policydomain/lint/diagnostic.go @@ -59,6 +59,9 @@ const ( // SourceRegistry indicates a domain-loading or registry-construction failure // that is not attributable to a specific YAML syntax error. SourceRegistry Source = "registry" + // SourceSelector indicates an invalid regular expression in a selector field + // on an operation, mapper, or resource entity. + SourceSelector Source = "selector" ) // Position is a 1-based line/column location within a file. diff --git a/pkg/policydomain/lint/lint.go b/pkg/policydomain/lint/lint.go index 8aba8a5..708da9e 100644 --- a/pkg/policydomain/lint/lint.go +++ b/pkg/policydomain/lint/lint.go @@ -157,6 +157,10 @@ func lintCore(ctx context.Context, src DataSource, opts Options) (*Result, error for _, key := range validKeys { data := rawData[key] + // Phase 1.5: Selector regex validation (runs on raw YAML before parse so + // entity-aware diagnostics are produced even when LoadFromBytes would fail). + diagnostics = append(diagnostics, lintSelectors(data, key)...) + domain, err := parsers.LoadFromBytes(key, data) if err != nil { diagnostics = append(diagnostics, Diagnostic{ diff --git a/pkg/policydomain/lint/selector.go b/pkg/policydomain/lint/selector.go new file mode 100644 index 0000000..1bd6203 --- /dev/null +++ b/pkg/policydomain/lint/selector.go @@ -0,0 +1,117 @@ +// +// Copyright © Manetu Inc. All rights reserved. +// + +package lint + +import ( + "fmt" + "regexp" + "strings" + + "gopkg.in/yaml.v3" +) + +// lintSelectors validates selector regex patterns on operations, mappers, and +// resources within a raw PolicyDomain YAML document. +// +// It walks the YAML node tree to find selector sequences and attempts to compile +// each pattern, emitting a structured Diagnostic for any invalid regex. This +// phase runs on the raw bytes before LoadFromBytes so that entity-aware +// diagnostics (with entity type, ID, and YAML line number) are produced even +// when the full parse would fail due to the bad pattern. +func lintSelectors(data []byte, key string) []Diagnostic { + var root yaml.Node + if err := yaml.Unmarshal(data, &root); err != nil { + // YAML syntax errors are reported by lintYAML; nothing to do here. + return nil + } + + if root.Kind != yaml.DocumentNode || len(root.Content) == 0 { + return nil + } + + doc := root.Content[0] + + domainName := "" + if meta := findMappingValue(doc, "metadata"); meta != nil { + domainName = findScalarValue(meta, "name") + } + + spec := findMappingValue(doc, "spec") + if spec == nil || spec.Kind != yaml.MappingNode { + return nil + } + + var diagnostics []Diagnostic + + for _, section := range []struct { + key string + entityType string + }{ + {"operations", "operation"}, + {"mappers", "mapper"}, + {"resources", "resource"}, + } { + sectionNode := findMappingValue(spec, section.key) + if sectionNode == nil || sectionNode.Kind != yaml.SequenceNode { + continue + } + for i, item := range sectionNode.Content { + if item.Kind != yaml.MappingNode { + continue + } + entityID := findScalarValue(item, "name") + if entityID == "" { + entityID = findScalarValue(item, "mrn") + } + if entityID == "" { + entityID = fmt.Sprintf("%s[%d]", section.entityType, i) + } + + selectorNode := findMappingValue(item, "selector") + if selectorNode == nil || selectorNode.Kind != yaml.SequenceNode { + continue + } + + for _, sel := range selectorNode.Content { + if sel.Kind != yaml.ScalarNode { + continue + } + pattern := selectorAnchorPattern(sel.Value) + if _, err := regexp.Compile(pattern); err != nil { + diagnostics = append(diagnostics, Diagnostic{ + Source: SourceSelector, + Severity: SeverityError, + Location: Location{ + File: key, + Start: Position{Line: sel.Line, Column: sel.Column}, + }, + Entity: Entity{ + Domain: domainName, + Type: section.entityType, + ID: entityID, + Field: "selector", + }, + Message: fmt.Sprintf("invalid selector regex %q: %s", sel.Value, err.Error()), + }) + } + } + } + } + + return diagnostics +} + +// selectorAnchorPattern ensures the pattern is anchored with ^ and $, +// matching the behaviour of the v1alpha3/v1alpha4/v1beta1 parsers. +func selectorAnchorPattern(pattern string) string { + result := pattern + if !strings.HasPrefix(result, "^") { + result = "^" + result + } + if !strings.HasSuffix(result, "$") { + result = result + "$" + } + return result +} diff --git a/pkg/policydomain/lint/selector_test.go b/pkg/policydomain/lint/selector_test.go new file mode 100644 index 0000000..d45111b --- /dev/null +++ b/pkg/policydomain/lint/selector_test.go @@ -0,0 +1,330 @@ +// +// Copyright © Manetu Inc. All rights reserved. +// + +package lint + +import ( + "context" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// validSelectorDomain is a well-formed v1beta1 PolicyDomain with valid selector +// patterns on an operation, a mapper, and a resource. +const validSelectorDomain = ` +apiVersion: iamlite.manetu.io/v1beta1 +kind: PolicyDomain +metadata: + name: test-domain +spec: + policies: + - mrn: "mrn:iam:policy:allow-all" + rego: | + package authz + default allow = 1 + + resource-groups: + - mrn: "mrn:iam:resource-group:default" + policy: "mrn:iam:policy:allow-all" + default: true + + operations: + - name: default-op + selector: + - ".*" + policy: "mrn:iam:policy:allow-all" + + mappers: + - name: default-mapper + selector: + - ".*" + rego: | + package mapper + porc := {"principal": {}, "operation": "op", "resource": {"id": "r", "group": "mrn:iam:resource-group:default"}, "context": input} + + resources: + - name: default-resource + selector: + - "mrn:data:.*" + group: "mrn:iam:resource-group:default" +` + +// domainWithInvalidSelector builds a minimal domain with one invalid selector +// on the named entity type. +func domainWithInvalidSelector(entityType, name, badPattern string) string { + var sb strings.Builder + sb.WriteString(` +apiVersion: iamlite.manetu.io/v1beta1 +kind: PolicyDomain +metadata: + name: test-domain +spec: + policies: + - mrn: "mrn:iam:policy:allow-all" + rego: | + package authz + default allow = 1 + + resource-groups: + - mrn: "mrn:iam:resource-group:default" + policy: "mrn:iam:policy:allow-all" + default: true + +`) + switch entityType { + case "operation": + sb.WriteString(" operations:\n") + sb.WriteString(" - name: " + name + "\n") + sb.WriteString(" selector:\n") + sb.WriteString(" - " + `"` + badPattern + `"` + "\n") + sb.WriteString(" policy: \"mrn:iam:policy:allow-all\"\n") + case "mapper": + sb.WriteString(" mappers:\n") + sb.WriteString(" - name: " + name + "\n") + sb.WriteString(" selector:\n") + sb.WriteString(" - " + `"` + badPattern + `"` + "\n") + sb.WriteString(" rego: |\n") + sb.WriteString(" package mapper\n") + sb.WriteString(` porc := {"principal": {}, "operation": "op", "resource": {"id": "r", "group": "mrn:iam:resource-group:default"}, "context": input}`) + sb.WriteString("\n") + case "resource": + sb.WriteString(" resources:\n") + sb.WriteString(" - name: " + name + "\n") + sb.WriteString(" selector:\n") + sb.WriteString(" - " + `"` + badPattern + `"` + "\n") + sb.WriteString(" group: \"mrn:iam:resource-group:default\"\n") + } + return sb.String() +} + +// filterSelector returns only diagnostics with SourceSelector. +func filterSelector(diags []Diagnostic) []Diagnostic { + var out []Diagnostic + for _, d := range diags { + if d.Source == SourceSelector { + out = append(out, d) + } + } + return out +} + +// --------------------------------------------------------------------------- +// lintSelectors unit tests +// --------------------------------------------------------------------------- + +func TestLintSelectors_ValidSelectorsProduceNoDiagnostics(t *testing.T) { + diags := lintSelectors([]byte(validSelectorDomain), "test.yml") + selectorDiags := filterSelector(diags) + assert.Empty(t, selectorDiags) +} + +func TestLintSelectors_InvalidOperationSelector(t *testing.T) { + yaml := domainWithInvalidSelector("operation", "my-op", "[invalid") + diags := lintSelectors([]byte(yaml), "test.yml") + + require.Len(t, diags, 1) + d := diags[0] + assert.Equal(t, SourceSelector, d.Source) + assert.Equal(t, SeverityError, d.Severity) + assert.Equal(t, "operation", d.Entity.Type) + assert.Equal(t, "my-op", d.Entity.ID) + assert.Equal(t, "selector", d.Entity.Field) + assert.Equal(t, "test-domain", d.Entity.Domain) + assert.Equal(t, "test.yml", d.Location.File) + assert.NotZero(t, d.Location.Start.Line) + assert.Contains(t, d.Message, "[invalid") +} + +func TestLintSelectors_InvalidMapperSelector(t *testing.T) { + yaml := domainWithInvalidSelector("mapper", "my-mapper", "[bad") + diags := lintSelectors([]byte(yaml), "test.yml") + + require.Len(t, diags, 1) + d := diags[0] + assert.Equal(t, SourceSelector, d.Source) + assert.Equal(t, SeverityError, d.Severity) + assert.Equal(t, "mapper", d.Entity.Type) + assert.Equal(t, "my-mapper", d.Entity.ID) + assert.Equal(t, "selector", d.Entity.Field) + assert.Contains(t, d.Message, "[bad") +} + +func TestLintSelectors_InvalidResourceSelector(t *testing.T) { + yaml := domainWithInvalidSelector("resource", "my-resource", "[broken") + diags := lintSelectors([]byte(yaml), "test.yml") + + require.Len(t, diags, 1) + d := diags[0] + assert.Equal(t, SourceSelector, d.Source) + assert.Equal(t, SeverityError, d.Severity) + assert.Equal(t, "resource", d.Entity.Type) + assert.Equal(t, "my-resource", d.Entity.ID) + assert.Equal(t, "selector", d.Entity.Field) + assert.Contains(t, d.Message, "[broken") +} + +func TestLintSelectors_MultipleInvalidSelectorsOnOneEntity(t *testing.T) { + yaml := ` +apiVersion: iamlite.manetu.io/v1beta1 +kind: PolicyDomain +metadata: + name: test-domain +spec: + operations: + - name: multi-op + selector: + - "[bad1" + - "[bad2" + - "valid.*" + policy: "mrn:iam:policy:allow-all" +` + diags := lintSelectors([]byte(yaml), "test.yml") + require.Len(t, diags, 2) + assert.Contains(t, diags[0].Message, "[bad1") + assert.Contains(t, diags[1].Message, "[bad2") +} + +func TestLintSelectors_MixedValidAndInvalidSelectors(t *testing.T) { + yaml := ` +apiVersion: iamlite.manetu.io/v1beta1 +kind: PolicyDomain +metadata: + name: test-domain +spec: + operations: + - name: ok-op + selector: + - ".*" + policy: "mrn:iam:policy:allow-all" + - name: bad-op + selector: + - "[invalid" + policy: "mrn:iam:policy:allow-all" +` + diags := lintSelectors([]byte(yaml), "test.yml") + require.Len(t, diags, 1) + assert.Equal(t, "bad-op", diags[0].Entity.ID) +} + +func TestLintSelectors_EntityIDFromNameField(t *testing.T) { + yaml := domainWithInvalidSelector("mapper", "named-mapper", "[bad") + diags := lintSelectors([]byte(yaml), "test.yml") + require.Len(t, diags, 1) + assert.Equal(t, "named-mapper", diags[0].Entity.ID) +} + +func TestLintSelectors_EntityIDFallbackToIndex(t *testing.T) { + // Entity with no name field gets an index-based fallback ID. + yaml := ` +apiVersion: iamlite.manetu.io/v1beta1 +kind: PolicyDomain +metadata: + name: test-domain +spec: + operations: + - selector: + - "[invalid" + policy: "mrn:iam:policy:allow-all" +` + diags := lintSelectors([]byte(yaml), "test.yml") + require.Len(t, diags, 1) + assert.Equal(t, "operation[0]", diags[0].Entity.ID) +} + +func TestLintSelectors_DomainNamePopulated(t *testing.T) { + yaml := domainWithInvalidSelector("operation", "op", "[bad") + diags := lintSelectors([]byte(yaml), "test.yml") + require.Len(t, diags, 1) + assert.Equal(t, "test-domain", diags[0].Entity.Domain) +} + +func TestLintSelectors_LineNumberAccuracy(t *testing.T) { + // The bad selector is on a known line; verify Start.Line is non-zero and + // points somewhere in the selector block (not line 1). + yaml := domainWithInvalidSelector("operation", "op", "[bad") + diags := lintSelectors([]byte(yaml), "test.yml") + require.Len(t, diags, 1) + assert.Greater(t, diags[0].Location.Start.Line, 1) +} + +func TestLintSelectors_InvalidYAMLReturnsNoDiagnostics(t *testing.T) { + // If the YAML is unparseable, lintSelectors should not panic and return empty. + diags := lintSelectors([]byte(":\t: bad yaml {{{"), "test.yml") + assert.Empty(t, diags) +} + +func TestLintSelectors_EmptyInputReturnsNoDiagnostics(t *testing.T) { + diags := lintSelectors([]byte(""), "test.yml") + assert.Empty(t, diags) +} + +// --------------------------------------------------------------------------- +// selectorAnchorPattern unit tests +// --------------------------------------------------------------------------- + +func TestSelectorAnchorPattern_AddsAnchors(t *testing.T) { + assert.Equal(t, "^foo$", selectorAnchorPattern("foo")) +} + +func TestSelectorAnchorPattern_PreservesExistingAnchors(t *testing.T) { + assert.Equal(t, "^foo$", selectorAnchorPattern("^foo$")) + assert.Equal(t, "^foo$", selectorAnchorPattern("^foo")) + assert.Equal(t, "^foo$", selectorAnchorPattern("foo$")) +} + +// --------------------------------------------------------------------------- +// Integration tests via LintFromStrings +// --------------------------------------------------------------------------- + +func TestLintFromStrings_InvalidOperationSelectorProducesSelectorDiagnostic(t *testing.T) { + yaml := domainWithInvalidSelector("operation", "bad-op", "[broken") + result, err := LintFromStrings(context.Background(), map[string]string{"test.yml": yaml}, DefaultOptions()) + require.NoError(t, err) + + selectorDiags := filterSelector(result.Diagnostics) + require.NotEmpty(t, selectorDiags, "expected at least one SourceSelector diagnostic") + d := selectorDiags[0] + assert.Equal(t, SourceSelector, d.Source) + assert.Equal(t, SeverityError, d.Severity) + assert.Equal(t, "operation", d.Entity.Type) + assert.Equal(t, "bad-op", d.Entity.ID) + assert.Equal(t, "selector", d.Entity.Field) + assert.Equal(t, "test-domain", d.Entity.Domain) + assert.Equal(t, "test.yml", d.Location.File) + assert.NotZero(t, d.Location.Start.Line) + assert.Contains(t, d.Message, "[broken") +} + +func TestLintFromStrings_InvalidMapperSelectorProducesSelectorDiagnostic(t *testing.T) { + yaml := domainWithInvalidSelector("mapper", "bad-mapper", "[oops") + result, err := LintFromStrings(context.Background(), map[string]string{"test.yml": yaml}, DefaultOptions()) + require.NoError(t, err) + + selectorDiags := filterSelector(result.Diagnostics) + require.NotEmpty(t, selectorDiags) + assert.Equal(t, "mapper", selectorDiags[0].Entity.Type) + assert.Equal(t, "bad-mapper", selectorDiags[0].Entity.ID) +} + +func TestLintFromStrings_InvalidResourceSelectorProducesSelectorDiagnostic(t *testing.T) { + yaml := domainWithInvalidSelector("resource", "bad-resource", "[nope") + result, err := LintFromStrings(context.Background(), map[string]string{"test.yml": yaml}, DefaultOptions()) + require.NoError(t, err) + + selectorDiags := filterSelector(result.Diagnostics) + require.NotEmpty(t, selectorDiags) + assert.Equal(t, "resource", selectorDiags[0].Entity.Type) + assert.Equal(t, "bad-resource", selectorDiags[0].Entity.ID) +} + +func TestLintFromStrings_ValidSelectorsProduceNoSelectorDiagnostics(t *testing.T) { + result, err := LintFromStrings(context.Background(), map[string]string{"test.yml": validSelectorDomain}, DefaultOptions()) + require.NoError(t, err) + + selectorDiags := filterSelector(result.Diagnostics) + assert.Empty(t, selectorDiags) +}