Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule Type schema validation: change library and apply defaults #4953

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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.21.0
github.com/signalfx/splunk-otel-go/instrumentation/github.com/lib/pq/splunkpq v1.21.0
github.com/sigstore/protobuf-specs v0.3.2
Expand All @@ -74,7 +75,6 @@ require (
github.com/styrainc/regal v0.28.0
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
Expand Down
3 changes: 2 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
16 changes: 16 additions & 0 deletions pkg/engine/v1/rtengine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
67 changes: 43 additions & 24 deletions pkg/profiles/rule_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -18,32 +18,32 @@ 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())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can compileSchema return a nil schema if the ParamSchema is empty?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In particular, that could make this:

paramSchema, err := compileSchema(rt.GetDef().GetParamSchema())
if err != nil {
  ...
}

if err != nil {
return nil, fmt.Errorf("cannot create json schema for params: %w", err)
}
}

return &RuleValidator{
ruleTypeName: rt.Name,
schema: schema,
schema: mainSchema,
paramSchema: paramSchema,
}, nil
}
Expand All @@ -57,7 +57,7 @@ func (r *RuleValidator) ValidateRuleDefAgainstSchema(contextualProfile map[strin
Err: err.Error(),
}
}

applyDefaults(r.schema, contextualProfile)
return nil
}

Expand All @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to worry that this error might have some other type? (And, does this need to be a separate method?)

}

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")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why TrimSpace here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all honesty, I don't remember. It may not be needed anymore.

}

// applyDefaults recursively applies default values from the schema to the object.
func applyDefaults(schema *jsonschema.Schema, obj map[string]any) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish the library had this function, but it looks like it doesn't. 😞

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No library I found has one 😢

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)
}
}
}
71 changes: 71 additions & 0 deletions pkg/profiles/rule_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,77 @@ 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()

Expand Down