From 99094ea402be7ab66a5a7766636be220837b3627 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 12 Nov 2024 17:03:45 +0200 Subject: [PATCH 1/3] Rule Type schema validation: change library and apply defaults This changes the underlying library to something that's actually maintained: github.com/santhosh-tekuri/jsonschema/v6 It also adds the feature of applying defaults if they were defined in the JSON schema. Signed-off-by: Juan Antonio Osorio --- go.mod | 2 +- go.sum | 3 +- pkg/profiles/rule_validator.go | 67 +++++++++++++++++---------- pkg/profiles/rule_validator_test.go | 72 +++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 26 deletions(-) diff --git a/go.mod b/go.mod index c311e2ae2f..c3be9c0983 100644 --- a/go.mod +++ b/go.mod @@ -60,6 +60,7 @@ require ( github.com/puzpuzpuz/xsync/v3 v3.4.0 github.com/robfig/cron/v3 v3.0.1 github.com/rs/zerolog v1.33.0 + github.com/santhosh-tekuri/jsonschema/v6 v6.0.1 github.com/signalfx/splunk-otel-go/instrumentation/database/sql/splunksql v1.22.0 github.com/signalfx/splunk-otel-go/instrumentation/github.com/lib/pq/splunkpq v1.22.0 github.com/sigstore/protobuf-specs v0.3.2 @@ -74,7 +75,6 @@ require ( github.com/styrainc/regal v0.29.1 github.com/thomaspoignant/go-feature-flag v1.38.0 github.com/xanzy/go-gitlab v0.113.0 - github.com/xeipuuv/gojsonschema v1.2.0 github.com/yuin/goldmark v1.7.8 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.57.0 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.57.0 diff --git a/go.sum b/go.sum index 8d925389dd..4895b38507 100644 --- a/go.sum +++ b/go.sum @@ -948,6 +948,8 @@ github.com/sagikazarmark/slog-shim v0.1.0/go.mod h1:SrcSrq8aKtyuqEI1uvTDTK1arOWR github.com/sahilm/fuzzy v0.1.1 h1:ceu5RHF8DGgoi+/dR5PsECjCDH1BE3Fnmpo7aVXOdRA= github.com/sahilm/fuzzy v0.1.1/go.mod h1:VFvziUEIMCrT6A6tw2RFIXPXXmzXbOsSHF0DOI8ZK9Y= github.com/saintfish/chardet v0.0.0-20120816061221-3af4cd4741ca/go.mod h1:uugorj2VCxiV1x+LzaIdVa9b4S4qGAcH6cbhh4qVxOU= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.1 h1:PKK9DyHxif4LZo+uQSgXNqs0jj5+xZwwfKHgph2lxBw= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.1/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= github.com/sassoftware/relic v7.2.1+incompatible h1:Pwyh1F3I0r4clFJXkSI8bOyJINGqpgjJU3DYAZeI05A= github.com/sassoftware/relic v7.2.1+incompatible/go.mod h1:CWfAxv73/iLZ17rbyhIEq3K9hs5w6FpNMdUT//qR+zk= github.com/sassoftware/relic/v7 v7.6.2 h1:rS44Lbv9G9eXsukknS4mSjIAuuX+lMq/FnStgmZlUv4= @@ -1074,7 +1076,6 @@ github.com/xanzy/go-gitlab v0.113.0 h1:v5O4R+YZbJGxKqa9iIZxjMyeKkMKBN8P6sZsNl+Yc github.com/xanzy/go-gitlab v0.113.0/go.mod h1:wKNKh3GkYDMOsGmnfuX+ITCmDuSDWFO0G+C4AygL9RY= github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM= github.com/xanzy/ssh-agent v0.3.3/go.mod h1:6dzNDKs0J9rVPHPhaGCukekBHKqfl+L3KghI1Bc68Uw= -github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb h1:zGWFAtiMcyryUHoUjUJX0/lt1H2+i2Ka2n+D3DImSNo= github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0= diff --git a/pkg/profiles/rule_validator.go b/pkg/profiles/rule_validator.go index e0c7a168dd..f50f89a712 100644 --- a/pkg/profiles/rule_validator.go +++ b/pkg/profiles/rule_validator.go @@ -7,7 +7,7 @@ import ( "fmt" "strings" - "github.com/xeipuuv/gojsonschema" + "github.com/santhosh-tekuri/jsonschema/v6" minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" ) @@ -18,24 +18,24 @@ type RuleValidator struct { ruleTypeName string // schema is the schema that this rule type must conform to - schema *gojsonschema.Schema + schema *jsonschema.Schema // paramSchema is the schema that the parameters for this rule type must conform to - paramSchema *gojsonschema.Schema + paramSchema *jsonschema.Schema } // NewRuleValidator creates a new rule validator func NewRuleValidator(rt *minderv1.RuleType) (*RuleValidator, error) { - // Load schemas - schemaLoader := gojsonschema.NewGoLoader(rt.Def.RuleSchema) - schema, err := gojsonschema.NewSchema(schemaLoader) + // Create a new schema compiler + // Compile the main rule schema + mainSchema, err := compileSchema(rt.GetDef().GetRuleSchema().AsMap()) if err != nil { return nil, fmt.Errorf("cannot create json schema: %w", err) } - var paramSchema *gojsonschema.Schema + // Compile the parameter schema if it exists + var paramSchema *jsonschema.Schema if rt.Def.ParamSchema != nil { - paramSchemaLoader := gojsonschema.NewGoLoader(rt.Def.ParamSchema) - paramSchema, err = gojsonschema.NewSchema(paramSchemaLoader) + paramSchema, err = compileSchema(rt.GetDef().GetParamSchema().AsMap()) if err != nil { return nil, fmt.Errorf("cannot create json schema for params: %w", err) } @@ -43,7 +43,7 @@ func NewRuleValidator(rt *minderv1.RuleType) (*RuleValidator, error) { return &RuleValidator{ ruleTypeName: rt.Name, - schema: schema, + schema: mainSchema, paramSchema: paramSchema, }, nil } @@ -57,7 +57,7 @@ func (r *RuleValidator) ValidateRuleDefAgainstSchema(contextualProfile map[strin Err: err.Error(), } } - + applyDefaults(r.schema, contextualProfile) return nil } @@ -67,36 +67,55 @@ func (r *RuleValidator) ValidateParamsAgainstSchema(params map[string]any) error if r.paramSchema == nil { return nil } - if err := validateAgainstSchema(r.paramSchema, params); err != nil { return &RuleValidationError{ RuleType: r.ruleTypeName, Err: err.Error(), } } - + applyDefaults(r.paramSchema, params) return nil } -func validateAgainstSchema(schema *gojsonschema.Schema, obj map[string]any) error { - documentLoader := gojsonschema.NewGoLoader(obj) - result, err := schema.Validate(documentLoader) - if err != nil { - return fmt.Errorf("cannot validate json schema: %s", err) +func compileSchema(schemaData interface{}) (*jsonschema.Schema, error) { + compiler := jsonschema.NewCompiler() + if err := compiler.AddResource("schema.json", schemaData); err != nil { + return nil, fmt.Errorf("invalid schema: %w", err) } + return compiler.Compile("schema.json") +} - if !result.Valid() { - return buildValidationError(result.Errors()) +func validateAgainstSchema(schema *jsonschema.Schema, obj map[string]any) error { + if err := schema.Validate(obj); err != nil { + return buildValidationError(err.(*jsonschema.ValidationError).Causes) } - return nil } -func buildValidationError(errs []gojsonschema.ResultError) error { +func buildValidationError(errs []*jsonschema.ValidationError) error { problems := make([]string, 0, len(errs)) for _, desc := range errs { - problems = append(problems, desc.String()) + problems = append(problems, desc.Error()) } - return fmt.Errorf("invalid json schema: %s", strings.TrimSpace(strings.Join(problems, "\n"))) } + +// applyDefaults recursively applies default values from the schema to the object. +func applyDefaults(schema *jsonschema.Schema, obj map[string]any) { + for key, def := range schema.Properties { + // If the key does not exist in obj, apply the default value from the schema if present + if _, exists := obj[key]; !exists && def.Default != nil { + obj[key] = *def.Default + } + + // If def has properties, apply defaults to the nested object + if def.Properties != nil { + o, ok := obj[key].(map[string]any) + if !ok { + // cannot apply defaults to non-object types + continue + } + applyDefaults(def, o) + } + } +} diff --git a/pkg/profiles/rule_validator_test.go b/pkg/profiles/rule_validator_test.go index ffb57311cb..aa2eba0ffd 100644 --- a/pkg/profiles/rule_validator_test.go +++ b/pkg/profiles/rule_validator_test.go @@ -15,6 +15,78 @@ import ( "github.com/mindersec/minder/pkg/profiles" ) +func TestSetDefaultValuesOnValidation(t *testing.T) { + t.Parallel() + + rtstr := ` + +--- +version: v1 +release_phase: alpha +type: rule-type +name: foo +display_name: Foo +short_failure_message: Foo failed +severity: + value: medium +context: + provider: github +description: Very important rule +guidance: | + This is how you should do it. +def: + in_entity: repository + rule_schema: + type: object + properties: + schedule_interval: + type: string + description: | + Sets the schedule interval in cron format for the workflow. Only applicable for remediation. + publish_results: + type: boolean + description: | + Publish the results of the analysis. + default: true + retention_days: + type: integer + description: | + Number of days to retain the SARIF file. + default: 5 + sarif_file: + type: string + description: | + Name of the SARIF file. + default: "results.sarif" + required: + - schedule_interval + - publish_results +` + + rt := &minderv1.RuleType{} + require.NoError(t, minderv1.ParseResource(strings.NewReader(rtstr), rt), "failed to parse rule type") + + rval, err := profiles.NewRuleValidator(rt) + require.NoError(t, err, "failed to create rule validator") + + obj := map[string]any{ + "schedule_interval": "0 0 * * *", + "publish_results": false, + "retention_days": 10, + } + + // Validation should pass + require.NoError(t, rval.ValidateRuleDefAgainstSchema(obj), "failed to validate rule definition") + + // Value is left as is + require.Equal(t, "0 0 * * *", obj["schedule_interval"]) + require.Equal(t, 10, obj["retention_days"]) + require.Equal(t, false, obj["publish_results"]) + + // default is set + require.Equal(t, "results.sarif", obj["sarif_file"]) +} + func TestExampleRulesAreValidatedCorrectly(t *testing.T) { t.Parallel() From 8fa87cd2183ffaf78b07f76837bd838777828aae Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 12 Nov 2024 17:22:55 +0200 Subject: [PATCH 2/3] Hook rule validation into engine so defaults are set. Signed-off-by: Juan Antonio Osorio --- pkg/engine/v1/rtengine/engine.go | 16 ++++++++++++++++ pkg/profiles/rule_validator_test.go | 1 - 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/engine/v1/rtengine/engine.go b/pkg/engine/v1/rtengine/engine.go index d72fa698b4..b17209d070 100644 --- a/pkg/engine/v1/rtengine/engine.go +++ b/pkg/engine/v1/rtengine/engine.go @@ -146,6 +146,22 @@ func (r *RuleTypeEngine) Eval( } }() + // The rule type has already been validated at creation time. However, + // re-validating it here is a good idea to ensure that the rule type + // has not been tampered with. Also, this sets the defaults for the + // rule definition. + if ruleDef != nil { + if err := r.ruleValidator.ValidateRuleDefAgainstSchema(ruleDef); err != nil { + return fmt.Errorf("rule definition validation failed: %w", err) + } + } + + if ruleParams != nil { + if err := r.ruleValidator.ValidateParamsAgainstSchema(ruleParams); err != nil { + return fmt.Errorf("rule parameters validation failed: %w", err) + } + } + logger.Info().Msg("entity evaluation - ingest started") // Try looking at the ingesting cache first result, ok := r.ingestCache.Get(r.ingester, entity, ruleParams) diff --git a/pkg/profiles/rule_validator_test.go b/pkg/profiles/rule_validator_test.go index aa2eba0ffd..f08cec5024 100644 --- a/pkg/profiles/rule_validator_test.go +++ b/pkg/profiles/rule_validator_test.go @@ -19,7 +19,6 @@ func TestSetDefaultValuesOnValidation(t *testing.T) { t.Parallel() rtstr := ` - --- version: v1 release_phase: alpha From 49a86d6fb56d1b280ee0c588c6a74dba0950a13e Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Fri, 15 Nov 2024 15:09:23 +0200 Subject: [PATCH 3/3] Address comments Signed-off-by: Juan Antonio Osorio --- pkg/profiles/rule_validator.go | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/pkg/profiles/rule_validator.go b/pkg/profiles/rule_validator.go index f50f89a712..35b75106ab 100644 --- a/pkg/profiles/rule_validator.go +++ b/pkg/profiles/rule_validator.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/santhosh-tekuri/jsonschema/v6" + "google.golang.org/protobuf/types/known/structpb" minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" ) @@ -25,20 +26,20 @@ type RuleValidator struct { // NewRuleValidator creates a new rule validator func NewRuleValidator(rt *minderv1.RuleType) (*RuleValidator, error) { + if rt.GetDef().GetRuleSchema() == nil { + return nil, fmt.Errorf("rule type %s does not have a rule schema", rt.Name) + } // Create a new schema compiler // Compile the main rule schema - mainSchema, err := compileSchema(rt.GetDef().GetRuleSchema().AsMap()) + mainSchema, err := compileSchemaFromPB(rt.GetDef().GetRuleSchema()) if err != nil { return nil, fmt.Errorf("cannot create json schema: %w", err) } // Compile the parameter schema if it exists - var paramSchema *jsonschema.Schema - if rt.Def.ParamSchema != nil { - paramSchema, err = compileSchema(rt.GetDef().GetParamSchema().AsMap()) - if err != nil { - return nil, fmt.Errorf("cannot create json schema for params: %w", err) - } + paramSchema, err := compileSchemaFromPB(rt.GetDef().GetParamSchema()) + if err != nil { + return nil, fmt.Errorf("cannot create json schema for params: %w", err) } return &RuleValidator{ @@ -77,7 +78,15 @@ func (r *RuleValidator) ValidateParamsAgainstSchema(params map[string]any) error return nil } -func compileSchema(schemaData interface{}) (*jsonschema.Schema, error) { +func compileSchemaFromPB(schemaData *structpb.Struct) (*jsonschema.Schema, error) { + if schemaData == nil { + return nil, nil + } + + return compileSchemaFromMap(schemaData.AsMap()) +} + +func compileSchemaFromMap(schemaData map[string]any) (*jsonschema.Schema, error) { compiler := jsonschema.NewCompiler() if err := compiler.AddResource("schema.json", schemaData); err != nil { return nil, fmt.Errorf("invalid schema: %w", err) @@ -87,7 +96,10 @@ func compileSchema(schemaData interface{}) (*jsonschema.Schema, error) { func validateAgainstSchema(schema *jsonschema.Schema, obj map[string]any) error { if err := schema.Validate(obj); err != nil { - return buildValidationError(err.(*jsonschema.ValidationError).Causes) + if verror, ok := err.(*jsonschema.ValidationError); ok { + return buildValidationError(verror.Causes) + } + return fmt.Errorf("invalid json schema: %s", err) } return nil }