diff --git a/database/migrations/000117_rule_type_rego_version.down.sql b/database/migrations/000117_rule_type_rego_version.down.sql new file mode 100644 index 0000000000..7c8a225a1f --- /dev/null +++ b/database/migrations/000117_rule_type_rego_version.down.sql @@ -0,0 +1,4 @@ +-- SPDX-FileCopyrightText: Copyright 2024 The Minder Authors +-- SPDX-License-Identifier: Apache-2.0 + +ALTER TABLE rule_type DROP COLUMN rego_version; diff --git a/database/migrations/000117_rule_type_rego_version.up.sql b/database/migrations/000117_rule_type_rego_version.up.sql new file mode 100644 index 0000000000..e5030dbbf7 --- /dev/null +++ b/database/migrations/000117_rule_type_rego_version.up.sql @@ -0,0 +1,6 @@ +-- SPDX-FileCopyrightText: Copyright 2024 The Minder Authors +-- SPDX-License-Identifier: Apache-2.0 + +-- Add rego_version column to rule_type table to cache the detected Rego +-- language version. All existing rule types are assumed to be V0. +ALTER TABLE rule_type ADD COLUMN rego_version TEXT NOT NULL DEFAULT 'v0'; diff --git a/database/query/rule_types.sql b/database/query/rule_types.sql index da30381b2a..ffede402d1 100644 --- a/database/query/rule_types.sql +++ b/database/query/rule_types.sql @@ -9,7 +9,8 @@ INSERT INTO rule_type ( subscription_id, display_name, release_phase, - short_failure_message + short_failure_message, + rego_version ) VALUES ( $1, $2, @@ -20,7 +21,8 @@ INSERT INTO rule_type ( sqlc.narg(subscription_id), sqlc.arg(display_name), sqlc.arg(release_phase), - sqlc.arg(short_failure_message) + sqlc.arg(short_failure_message), + sqlc.arg(rego_version) ) RETURNING *; -- name: ListRuleTypesByProject :many @@ -37,7 +39,7 @@ DELETE FROM rule_type WHERE id = $1; -- name: UpdateRuleType :one UPDATE rule_type - SET description = $2, definition = sqlc.arg(definition)::jsonb, severity_value = sqlc.arg(severity_value), display_name = sqlc.arg(display_name), release_phase = sqlc.arg(release_phase), short_failure_message = sqlc.arg(short_failure_message) + SET description = $2, definition = sqlc.arg(definition)::jsonb, severity_value = sqlc.arg(severity_value), display_name = sqlc.arg(display_name), release_phase = sqlc.arg(release_phase), short_failure_message = sqlc.arg(short_failure_message), rego_version = sqlc.arg(rego_version) WHERE id = $1 RETURNING *; diff --git a/internal/db/models.go b/internal/db/models.go index 70d8df5caf..1a37982421 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -718,6 +718,7 @@ type RuleType struct { DisplayName string `json:"display_name"` ReleasePhase ReleaseStatus `json:"release_phase"` ShortFailureMessage string `json:"short_failure_message"` + RegoVersion string `json:"rego_version"` } type RuleTypeDataSource struct { diff --git a/internal/db/rule_types.sql.go b/internal/db/rule_types.sql.go index 6b5d863e89..d6013b8de6 100644 --- a/internal/db/rule_types.sql.go +++ b/internal/db/rule_types.sql.go @@ -24,7 +24,8 @@ INSERT INTO rule_type ( subscription_id, display_name, release_phase, - short_failure_message + short_failure_message, + rego_version ) VALUES ( $1, $2, @@ -35,8 +36,9 @@ INSERT INTO rule_type ( $7, $8, $9, - $10 -) RETURNING id, name, provider, project_id, description, guidance, definition, created_at, updated_at, severity_value, provider_id, subscription_id, display_name, release_phase, short_failure_message + $10, + $11 +) RETURNING id, name, provider, project_id, description, guidance, definition, created_at, updated_at, severity_value, provider_id, subscription_id, display_name, release_phase, short_failure_message, rego_version ` type CreateRuleTypeParams struct { @@ -50,6 +52,7 @@ type CreateRuleTypeParams struct { DisplayName string `json:"display_name"` ReleasePhase ReleaseStatus `json:"release_phase"` ShortFailureMessage string `json:"short_failure_message"` + RegoVersion string `json:"rego_version"` } func (q *Queries) CreateRuleType(ctx context.Context, arg CreateRuleTypeParams) (RuleType, error) { @@ -64,6 +67,7 @@ func (q *Queries) CreateRuleType(ctx context.Context, arg CreateRuleTypeParams) arg.DisplayName, arg.ReleasePhase, arg.ShortFailureMessage, + arg.RegoVersion, ) var i RuleType err := row.Scan( @@ -82,6 +86,7 @@ func (q *Queries) CreateRuleType(ctx context.Context, arg CreateRuleTypeParams) &i.DisplayName, &i.ReleasePhase, &i.ShortFailureMessage, + &i.RegoVersion, ) return i, err } @@ -96,7 +101,7 @@ func (q *Queries) DeleteRuleType(ctx context.Context, id uuid.UUID) error { } const getRuleTypeByID = `-- name: GetRuleTypeByID :one -SELECT id, name, provider, project_id, description, guidance, definition, created_at, updated_at, severity_value, provider_id, subscription_id, display_name, release_phase, short_failure_message FROM rule_type WHERE id = $1 +SELECT id, name, provider, project_id, description, guidance, definition, created_at, updated_at, severity_value, provider_id, subscription_id, display_name, release_phase, short_failure_message, rego_version FROM rule_type WHERE id = $1 ` func (q *Queries) GetRuleTypeByID(ctx context.Context, id uuid.UUID) (RuleType, error) { @@ -118,12 +123,13 @@ func (q *Queries) GetRuleTypeByID(ctx context.Context, id uuid.UUID) (RuleType, &i.DisplayName, &i.ReleasePhase, &i.ShortFailureMessage, + &i.RegoVersion, ) return i, err } const getRuleTypeByName = `-- name: GetRuleTypeByName :one -SELECT id, name, provider, project_id, description, guidance, definition, created_at, updated_at, severity_value, provider_id, subscription_id, display_name, release_phase, short_failure_message FROM rule_type WHERE project_id = ANY($1::uuid[]) AND lower(name) = lower($2) +SELECT id, name, provider, project_id, description, guidance, definition, created_at, updated_at, severity_value, provider_id, subscription_id, display_name, release_phase, short_failure_message, rego_version FROM rule_type WHERE project_id = ANY($1::uuid[]) AND lower(name) = lower($2) ` type GetRuleTypeByNameParams struct { @@ -150,6 +156,7 @@ func (q *Queries) GetRuleTypeByName(ctx context.Context, arg GetRuleTypeByNamePa &i.DisplayName, &i.ReleasePhase, &i.ShortFailureMessage, + &i.RegoVersion, ) return i, err } @@ -169,7 +176,7 @@ func (q *Queries) GetRuleTypeNameByID(ctx context.Context, id uuid.UUID) (string } const getRuleTypesByEntityInHierarchy = `-- name: GetRuleTypesByEntityInHierarchy :many -SELECT rt.id, rt.name, rt.provider, rt.project_id, rt.description, rt.guidance, rt.definition, rt.created_at, rt.updated_at, rt.severity_value, rt.provider_id, rt.subscription_id, rt.display_name, rt.release_phase, rt.short_failure_message FROM rule_type AS rt +SELECT rt.id, rt.name, rt.provider, rt.project_id, rt.description, rt.guidance, rt.definition, rt.created_at, rt.updated_at, rt.severity_value, rt.provider_id, rt.subscription_id, rt.display_name, rt.release_phase, rt.short_failure_message, rt.rego_version FROM rule_type AS rt JOIN rule_instances AS ri ON ri.rule_type_id = rt.id WHERE ri.entity_type = $1 AND ri.project_id = ANY($2::uuid[]) @@ -205,6 +212,7 @@ func (q *Queries) GetRuleTypesByEntityInHierarchy(ctx context.Context, arg GetRu &i.DisplayName, &i.ReleasePhase, &i.ShortFailureMessage, + &i.RegoVersion, ); err != nil { return nil, err } @@ -220,7 +228,7 @@ func (q *Queries) GetRuleTypesByEntityInHierarchy(ctx context.Context, arg GetRu } const listRuleTypesByProject = `-- name: ListRuleTypesByProject :many -SELECT id, name, provider, project_id, description, guidance, definition, created_at, updated_at, severity_value, provider_id, subscription_id, display_name, release_phase, short_failure_message FROM rule_type WHERE project_id = $1 +SELECT id, name, provider, project_id, description, guidance, definition, created_at, updated_at, severity_value, provider_id, subscription_id, display_name, release_phase, short_failure_message, rego_version FROM rule_type WHERE project_id = $1 ` func (q *Queries) ListRuleTypesByProject(ctx context.Context, projectID uuid.UUID) ([]RuleType, error) { @@ -248,6 +256,7 @@ func (q *Queries) ListRuleTypesByProject(ctx context.Context, projectID uuid.UUI &i.DisplayName, &i.ReleasePhase, &i.ShortFailureMessage, + &i.RegoVersion, ); err != nil { return nil, err } @@ -264,9 +273,9 @@ func (q *Queries) ListRuleTypesByProject(ctx context.Context, projectID uuid.UUI const updateRuleType = `-- name: UpdateRuleType :one UPDATE rule_type - SET description = $2, definition = $3::jsonb, severity_value = $4, display_name = $5, release_phase = $6, short_failure_message = $7 + SET description = $2, definition = $3::jsonb, severity_value = $4, display_name = $5, release_phase = $6, short_failure_message = $7, rego_version = $8 WHERE id = $1 - RETURNING id, name, provider, project_id, description, guidance, definition, created_at, updated_at, severity_value, provider_id, subscription_id, display_name, release_phase, short_failure_message + RETURNING id, name, provider, project_id, description, guidance, definition, created_at, updated_at, severity_value, provider_id, subscription_id, display_name, release_phase, short_failure_message, rego_version ` type UpdateRuleTypeParams struct { @@ -277,6 +286,7 @@ type UpdateRuleTypeParams struct { DisplayName string `json:"display_name"` ReleasePhase ReleaseStatus `json:"release_phase"` ShortFailureMessage string `json:"short_failure_message"` + RegoVersion string `json:"rego_version"` } func (q *Queries) UpdateRuleType(ctx context.Context, arg UpdateRuleTypeParams) (RuleType, error) { @@ -288,6 +298,7 @@ func (q *Queries) UpdateRuleType(ctx context.Context, arg UpdateRuleTypeParams) arg.DisplayName, arg.ReleasePhase, arg.ShortFailureMessage, + arg.RegoVersion, ) var i RuleType err := row.Scan( @@ -306,6 +317,7 @@ func (q *Queries) UpdateRuleType(ctx context.Context, arg UpdateRuleTypeParams) &i.DisplayName, &i.ReleasePhase, &i.ShortFailureMessage, + &i.RegoVersion, ) return i, err } diff --git a/internal/engine/eval/rego/eval.go b/internal/engine/eval/rego/eval.go index ff8c27f9b2..711290ba67 100644 --- a/internal/engine/eval/rego/eval.go +++ b/internal/engine/eval/rego/eval.go @@ -45,6 +45,7 @@ type Evaluator struct { regoOpts []func(*rego.Rego) reseval resultEvaluator datasources *v1datasources.DataSourceRegistry + regoVersion ast.RegoVersion } // Input is the input for the rego evaluator @@ -83,8 +84,9 @@ func NewRegoEvaluator( re := c.getEvalType() eval := &Evaluator{ - cfg: c, - reseval: re, + cfg: c, + reseval: re, + regoVersion: ast.RegoV0, regoOpts: []func(*rego.Rego){ rego.Query(RegoQueryPrefix), rego.Module(MinderRegoFile, c.Def), @@ -140,8 +142,7 @@ func (e *Evaluator) Eval( // Register options to expose functions regoFuncOptions := []func(*rego.Rego){ - // TODO: figure out a Rego V1 migration path (https://github.com/mindersec/minder/issues/5262) - rego.SetRegoVersion(ast.RegoV0), + rego.SetRegoVersion(e.regoVersion), } // Initialize the built-in minder library rego functions @@ -188,6 +189,18 @@ func enrichInputWithEntityProps( } } +// WithRegoVersion returns an Option that sets the Rego language version used +// for evaluation. The version is typically determined at rule type creation +// time via dual-parse detection and stored in the database. +func WithRegoVersion(v ast.RegoVersion) interfaces.Option { + return func(eval interfaces.Evaluator) error { + if e, ok := eval.(*Evaluator); ok { + e.regoVersion = v + } + return nil + } +} + // WithShortFailureMessage returns an Option that sets the short failure message for deny-by-default evaluations. // This message will be used as a fallback when the rego policy doesn't provide a custom "message" field, // but before defaulting to the generic "denied" message. diff --git a/internal/engine/eval/rego/version.go b/internal/engine/eval/rego/version.go new file mode 100644 index 0000000000..c522d7b530 --- /dev/null +++ b/internal/engine/eval/rego/version.go @@ -0,0 +1,40 @@ +// SPDX-FileCopyrightText: Copyright 2024 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package rego + +import ( + "github.com/open-policy-agent/opa/v1/ast" +) + +// DetectRegoVersion attempts to parse the Rego source with the V1 parser first. +// If V1 parsing succeeds, it returns ast.RegoV1. If V1 parsing fails, it falls +// back to ast.RegoV0. This allows the system to accept both V0 and V1 policies +// without requiring any user-facing changes. +func DetectRegoVersion(def string) ast.RegoVersion { + _, err := ast.ParseModuleWithOpts(MinderRegoFile, def, + ast.ParserOptions{RegoVersion: ast.RegoV1}) + if err == nil { + return ast.RegoV1 + } + + return ast.RegoV0 +} + +// VersionToString converts an ast.RegoVersion to the string stored in the +// database. +func VersionToString(v ast.RegoVersion) string { + if v == ast.RegoV1 { + return "v1" + } + return "v0" +} + +// VersionFromString converts a stored database string back to an +// ast.RegoVersion. +func VersionFromString(s string) ast.RegoVersion { + if s == "v1" { + return ast.RegoV1 + } + return ast.RegoV0 +} diff --git a/internal/engine/eval/rego/version_test.go b/internal/engine/eval/rego/version_test.go new file mode 100644 index 0000000000..da0fc4c2fe --- /dev/null +++ b/internal/engine/eval/rego/version_test.go @@ -0,0 +1,144 @@ +// SPDX-FileCopyrightText: Copyright 2024 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package rego_test + +import ( + "testing" + + "github.com/open-policy-agent/opa/v1/ast" + "github.com/stretchr/testify/require" + + rego "github.com/mindersec/minder/internal/engine/eval/rego" +) + +func TestDetectRegoVersion(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + def string + want ast.RegoVersion + wantStr string + }{ + { + name: "v0 policy without imports", + def: ` +package minder + +default allow = false + +allow { + file.exists("foo") +}`, + want: ast.RegoV0, + wantStr: "v0", + }, + { + name: "v1 policy with import rego.v1", + def: ` +package minder + +import rego.v1 + +default allow := false + +allow if { + file.exists("foo") +}`, + want: ast.RegoV1, + wantStr: "v1", + }, + { + name: "v0 policy with future.keywords parses as v1", + def: ` +package minder + +import future.keywords.if +import future.keywords.in + +default allow = false + +allow if { + "admin" in input.profile.roles +}`, + // future.keywords policies are accepted by OPA's V1 parser + // for backward compatibility during migration. + want: ast.RegoV1, + wantStr: "v1", + }, + { + name: "v1 constraints policy", + def: ` +package minder + +import rego.v1 + +violations contains {"msg": msg} if { + input.ingested.name == "" + msg := "name is required" +}`, + want: ast.RegoV1, + wantStr: "v1", + }, + { + name: "v0 constraints policy", + def: ` +package minder + +violations[{"msg": msg}] { + input.ingested.name == "" + msg = "name is required" +}`, + want: ast.RegoV0, + wantStr: "v0", + }, + { + name: "empty string defaults to v0", + def: "", + want: ast.RegoV0, + wantStr: "v0", + }, + { + name: "valid in both v0 and v1 returns v1", + def: ` +package minder + +default allow := false`, + want: ast.RegoV1, + wantStr: "v1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := rego.DetectRegoVersion(tt.def) + require.Equal(t, tt.want, got, "DetectRegoVersion() returned unexpected version") + require.Equal(t, tt.wantStr, rego.VersionToString(got)) + }) + } +} + +func TestRegoVersionRoundTrip(t *testing.T) { + t.Parallel() + + tests := []struct { + str string + version ast.RegoVersion + }{ + {"v0", ast.RegoV0}, + {"v1", ast.RegoV1}, + {"", ast.RegoV0}, // unknown defaults to v0 + {"v2", ast.RegoV0}, // unknown defaults to v0 + {"invalid", ast.RegoV0}, // unknown defaults to v0 + } + + for _, tt := range tests { + t.Run(tt.str, func(t *testing.T) { + t.Parallel() + got := rego.VersionFromString(tt.str) + require.Equal(t, tt.version, got) + }) + } +} diff --git a/internal/engine/rtengine/cache.go b/internal/engine/rtengine/cache.go index 1dd9df6ccf..bd59a4df81 100644 --- a/internal/engine/rtengine/cache.go +++ b/internal/engine/rtengine/cache.go @@ -13,6 +13,7 @@ import ( datasourceservice "github.com/mindersec/minder/internal/datasources/service" "github.com/mindersec/minder/internal/db" + regoeval "github.com/mindersec/minder/internal/engine/eval/rego" "github.com/mindersec/minder/internal/engine/ingestcache" eoptions "github.com/mindersec/minder/internal/engine/options" "github.com/mindersec/minder/pkg/engine/v1/interfaces" @@ -164,6 +165,14 @@ func cacheRuleEngine( opts = append(opts, eoptions.WithDataSources(dsreg), eoptions.WithFlagsClient(featureFlags)) + // Pass the stored Rego version to the evaluator when the dual-parse + // feature flag is enabled. When the flag is off, the evaluator defaults + // to Rego V0 (the zero value of ast.RegoVersion). + if flags.Bool(ctx, featureFlags, flags.RegoV1DualParse) { + opts = append(opts, regoeval.WithRegoVersion( + regoeval.VersionFromString(ruleType.RegoVersion))) + } + // Create the rule type engine ruleEngine, err := rtengine2.NewRuleTypeEngine(ctx, pbRuleType, provider, opts...) if err != nil { diff --git a/internal/service/service.go b/internal/service/service.go index 87856e9d82..e1d63f3d0d 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -98,7 +98,7 @@ func AllInOneServerService( inviteSvc := invites.NewInviteService() selChecker := selectors.NewEnv() profileSvc := profiles.NewProfileService(evt, selChecker) - ruleSvc := ruletypes.NewRuleTypeService() + ruleSvc := ruletypes.NewRuleTypeService(featureFlagClient) roleScv := roles.NewRoleService() dataSourcesSvc := datasourcessvc.NewDataSourceService(store) marketplace, err := marketplaces.NewMarketplaceFromServiceConfig(cfg.Marketplace, profileSvc, ruleSvc, dataSourcesSvc) diff --git a/pkg/api/protobuf/go/minder/v1/validators.go b/pkg/api/protobuf/go/minder/v1/validators.go index 965f28bf47..053716614c 100644 --- a/pkg/api/protobuf/go/minder/v1/validators.go +++ b/pkg/api/protobuf/go/minder/v1/validators.go @@ -11,7 +11,6 @@ import ( "github.com/go-playground/validator/v10" "github.com/itchyny/gojq" - "github.com/open-policy-agent/opa/v1/ast" "k8s.io/apimachinery/pkg/util/sets" "github.com/mindersec/minder/internal/util" @@ -251,12 +250,9 @@ func (rego *RuleType_Definition_Eval_Rego) Validate() error { return fmt.Errorf("%w: rego definition is empty", ErrInvalidRuleTypeDefinition) } - // TODO: figure out a Rego V1 migration path (https://github.com/mindersec/minder/issues/5262) - _, err := ast.ParseModuleWithOpts("minder-ruletype-def.rego", rego.Def, - ast.ParserOptions{RegoVersion: ast.RegoV0}) - if err != nil { - return fmt.Errorf("%w: rego definition is invalid: %s", ErrInvalidRuleTypeDefinition, err) - } + // Rego language-level validation (syntax checking, version gating) is + // intentionally handled server-side in ruletypes/service.go so that it + // can be controlled via feature flags without requiring client updates. return nil } diff --git a/pkg/api/protobuf/go/minder/v1/validators_test.go b/pkg/api/protobuf/go/minder/v1/validators_test.go index 4e945c6b7b..c38e3843e1 100644 --- a/pkg/api/protobuf/go/minder/v1/validators_test.go +++ b/pkg/api/protobuf/go/minder/v1/validators_test.go @@ -302,18 +302,11 @@ func TestRuleType_Definition_Eval_Rego_Validate(t *testing.T) { wantErr: true, }, { - name: "invalid syntax rego definition", + name: "any non-empty rego definition passes protobuf validation", rego: &RuleType_Definition_Eval_Rego{ Def: "package example.policy\n\nallow {", }, - wantErr: true, - }, - { - name: "missing import rego definition", - rego: &RuleType_Definition_Eval_Rego{ - Def: "package example.policy\n\nallow if { input.ingested.url != \"\" }", - }, - wantErr: true, + wantErr: false, }, } diff --git a/pkg/flags/constants.go b/pkg/flags/constants.go index 8730be78f5..f2e5946daa 100644 --- a/pkg/flags/constants.go +++ b/pkg/flags/constants.go @@ -14,4 +14,8 @@ const ( ProjectCreateDelete Experiment = "project_create_delete" // AuthenticatedDataSources enables provider authentication for data sources. AuthenticatedDataSources Experiment = "authenticated_datasources" + // RegoV1DualParse enables dual-parse version detection (try V1, fall back + // to V0) when creating or updating rule types. When disabled, all rule + // types are treated as Rego V0. + RegoV1DualParse Experiment = "rego_v1_dual_parse" ) diff --git a/pkg/querier/querier.go b/pkg/querier/querier.go index 2e6c228760..175f588905 100644 --- a/pkg/querier/querier.go +++ b/pkg/querier/querier.go @@ -88,7 +88,7 @@ func New(ctx context.Context, config *server.Config) (Store, Closer, error) { return &querierType{ store: store, querier: store, // use store by default - ruleSvc: ruletypes.NewRuleTypeService(), + ruleSvc: ruletypes.NewRuleTypeService(nil), profileSvc: profiles.NewProfileService(evt, selectors.NewEnv()), dataSourceSvc: datasourceservice.NewDataSourceService(store), }, dbCloser, nil diff --git a/pkg/ruletypes/service.go b/pkg/ruletypes/service.go index 7fdb062b91..e47dc118cd 100644 --- a/pkg/ruletypes/service.go +++ b/pkg/ruletypes/service.go @@ -13,6 +13,7 @@ import ( "regexp" "github.com/google/uuid" + "github.com/open-policy-agent/opa/v1/ast" "github.com/mindersec/minder/internal/db" "github.com/mindersec/minder/internal/logger" @@ -20,6 +21,7 @@ import ( "github.com/mindersec/minder/internal/util" "github.com/mindersec/minder/internal/util/schemaupdate" pb "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" + "github.com/mindersec/minder/pkg/flags" ) //go:generate go run go.uber.org/mock/mockgen -package mock_$GOPACKAGE -destination=./mock/$GOFILE -source=./$GOFILE @@ -68,11 +70,13 @@ type RuleTypeService interface { ) error } -type ruleTypeService struct{} +type ruleTypeService struct { + featureFlags flags.Interface +} // NewRuleTypeService creates a new instance of RuleTypeService -func NewRuleTypeService() RuleTypeService { - return &ruleTypeService{} +func NewRuleTypeService(featureFlags flags.Interface) RuleTypeService { + return &ruleTypeService{featureFlags: featureFlags} } var ( @@ -89,7 +93,7 @@ var ( uuidLike = regexp.MustCompile(`^[[:alnum:]]{8}-([[:alnum:]]{4}-){3}[[:alnum:]]{12}$`) ) -func (*ruleTypeService) CreateRuleType( +func (s *ruleTypeService) CreateRuleType( ctx context.Context, projectID uuid.UUID, subscriptionID uuid.UUID, @@ -102,6 +106,11 @@ func (*ruleTypeService) CreateRuleType( if err := ruleType.Validate(); err != nil { return nil, errors.Join(ErrRuleTypeInvalid, err) } + + regoVersion, err := s.validateAndDetectRegoVersion(ctx, ruleType) + if err != nil { + return nil, errors.Join(ErrRuleTypeInvalid, err) + } if uuidLike.MatchString(ruleType.Name) { return nil, errors.Join(ErrRuleTypeInvalid, errors.New("rule type name must not be UUID-like")) } @@ -156,6 +165,7 @@ func (*ruleTypeService) CreateRuleType( SeverityValue: *severity, SubscriptionID: uuid.NullUUID{UUID: subscriptionID, Valid: subscriptionID != uuid.Nil}, ReleasePhase: *releasePhase, + RegoVersion: regoVersion, }) if err != nil { return nil, fmt.Errorf("failed to create rule type: %w", err) @@ -179,7 +189,7 @@ func (*ruleTypeService) CreateRuleType( return rt, nil } -func (*ruleTypeService) UpdateRuleType( +func (s *ruleTypeService) UpdateRuleType( ctx context.Context, projectID uuid.UUID, subscriptionID uuid.UUID, @@ -193,6 +203,11 @@ func (*ruleTypeService) UpdateRuleType( return nil, errors.Join(ErrRuleTypeInvalid, err) } + regoVersion, err := s.validateAndDetectRegoVersion(ctx, ruleType) + if err != nil { + return nil, errors.Join(ErrRuleTypeInvalid, err) + } + ruleTypeName := ruleType.GetName() ruleTypeDef := ruleType.GetDef() @@ -243,6 +258,7 @@ func (*ruleTypeService) UpdateRuleType( DisplayName: ruleType.GetDisplayName(), ShortFailureMessage: ruleType.GetShortFailureMessage(), ReleasePhase: *releasePhase, + RegoVersion: regoVersion, }) if err != nil { return nil, fmt.Errorf("failed to update rule type: %w", err) @@ -297,6 +313,46 @@ func (s *ruleTypeService) UpsertRuleType( return nil } +// validateAndDetectRegoVersion validates the rego definition syntax, gates +// V1-only policies behind the feature flag, and returns the detected version +// string ("v0" or "v1") for database storage. This avoids parsing the rego +// definition multiple times across separate validation and detection steps. +func (s *ruleTypeService) validateAndDetectRegoVersion(ctx context.Context, ruleType *pb.RuleType) (string, error) { + def := ruleType.GetDef().GetEval().GetRego().GetDef() + if def == "" { + return "v0", nil + } + + flagOn := flags.Bool(ctx, s.featureFlags, flags.RegoV1DualParse) + + _, errV0 := ast.ParseModuleWithOpts("minder-ruletype-def.rego", def, + ast.ParserOptions{RegoVersion: ast.RegoV0}) + _, errV1 := ast.ParseModuleWithOpts("minder-ruletype-def.rego", def, + ast.ParserOptions{RegoVersion: ast.RegoV1}) + + switch { + case errV0 == nil && errV1 == nil: + // Valid in both dialects — store as V1 when flag is on. + if flagOn { + return "v1", nil + } + return "v0", nil + case errV0 == nil: + // V0 only — always accepted. + return "v0", nil + case errV1 == nil: + // V1 only — gate behind feature flag. + if !flagOn { + return "", fmt.Errorf("rego definition uses V1-only syntax which is not yet enabled") + } + return "v1", nil + default: + // Invalid in both dialects — return V0 error as it's the more + // likely intent for authors not yet on V1. + return "", fmt.Errorf("rego definition is invalid: %w", errV0) + } +} + func getRuleTypeSeverity(severity *pb.Severity) (*db.Severity, error) { sev := severity.InitializedStringValue() var seval db.Severity diff --git a/pkg/ruletypes/service_test.go b/pkg/ruletypes/service_test.go index 496e056cc6..46f6d9c7bc 100644 --- a/pkg/ruletypes/service_test.go +++ b/pkg/ruletypes/service_test.go @@ -19,6 +19,7 @@ import ( dbf "github.com/mindersec/minder/internal/db/fixtures" "github.com/mindersec/minder/internal/util/ptr" pb "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" + "github.com/mindersec/minder/pkg/flags" "github.com/mindersec/minder/pkg/ruletypes" ) @@ -34,6 +35,7 @@ func TestRuleTypeService(t *testing.T) { ExpectedError string TestMethod method SubscriptionID uuid.UUID + FeatureFlags map[string]any }{ { Name: "CreateRuleType rejects nil rule", @@ -298,6 +300,44 @@ func TestRuleTypeService(t *testing.T) { DBSetup: dbf.NewDBMock(withHierarchyGet, withSuccessfulGet, withSuccessfulUpdate, withSuccessfulGetDataSourcesByName(2), withSuccessfulDeleteRuleTypeDataSource, withSuccessfulAddRuleTypeDataSourceReference), TestMethod: update, }, + { + Name: "CreateRuleType rejects V1-only rego when flag is off", + RuleType: newRuleType(withBasicStructure, withV1OnlyRego), + ExpectedError: "V1-only syntax", + TestMethod: create, + }, + { + Name: "UpdateRuleType rejects V1-only rego when flag is off", + RuleType: newRuleType(withBasicStructure, withV1OnlyRego), + ExpectedError: "V1-only syntax", + TestMethod: update, + }, + { + Name: "CreateRuleType allows V1-only rego when flag is on", + RuleType: newRuleType(withBasicStructure, withV1OnlyRego), + DBSetup: dbf.NewDBMock(withHierarchyGet, withNotFoundGet, withSuccessfulCreate, withSuccessfulDeleteRuleTypeDataSource), + TestMethod: create, + FeatureFlags: map[string]any{string(flags.RegoV1DualParse): true}, + }, + { + Name: "UpdateRuleType allows V1-only rego when flag is on", + RuleType: newRuleType(withBasicStructure, withV1OnlyRego), + DBSetup: dbf.NewDBMock(withHierarchyGet, withSuccessfulGet, withSuccessfulUpdate, withSuccessfulDeleteRuleTypeDataSource), + TestMethod: update, + FeatureFlags: map[string]any{string(flags.RegoV1DualParse): true}, + }, + { + Name: "CreateRuleType rejects invalid rego syntax", + RuleType: newRuleType(withBasicStructure, withInvalidRego), + ExpectedError: "rego definition is invalid", + TestMethod: create, + }, + { + Name: "UpdateRuleType rejects invalid rego syntax", + RuleType: newRuleType(withBasicStructure, withInvalidRego), + ExpectedError: "rego definition is invalid", + TestMethod: update, + }, } for _, scenario := range scenarios { @@ -312,9 +352,14 @@ func TestRuleTypeService(t *testing.T) { store = scenario.DBSetup(ctrl) } + var featureFlags flags.Interface + if scenario.FeatureFlags != nil { + featureFlags = &flags.FakeClient{Data: scenario.FeatureFlags} + } + var err error var res *pb.RuleType - svc := ruletypes.NewRuleTypeService() + svc := ruletypes.NewRuleTypeService(featureFlags) switch scenario.TestMethod { case create: res, err = svc.CreateRuleType( @@ -449,6 +494,30 @@ func withIncompatibleParams(ruleType *pb.RuleType) { ruleType.Def.ParamSchema = incompatibleSchema } +// withV1OnlyRego sets a Rego definition that uses V1-only syntax (the +// `contains` keyword), which parses under V1 but fails under V0. +func withV1OnlyRego(ruleType *pb.RuleType) { + ruleType.Def.Eval = &pb.RuleType_Definition_Eval{ + Type: "rego", + Rego: &pb.RuleType_Definition_Eval_Rego{ + Type: "deny", + Def: "package minder\n\nviolations contains msg if {\n msg := \"something bad\"\n}\n", + }, + } +} + +// withInvalidRego sets a Rego definition with a syntax error that is +// invalid in both V0 and V1. +func withInvalidRego(ruleType *pb.RuleType) { + ruleType.Def.Eval = &pb.RuleType_Definition_Eval{ + Type: "rego", + Rego: &pb.RuleType_Definition_Eval_Rego{ + Type: "deny", + Def: "package minder\n\nallow {", + }, + } +} + func withHierarchyGet(mock dbf.DBMock) { mock.EXPECT(). GetParentProjects(gomock.Any(), gomock.Any()).