From 6402c6e33d75a8ebab6050bbe4ffcf9ab7493db1 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Wed, 11 Dec 2024 00:33:59 -0800 Subject: [PATCH 1/7] Add support for base and target trees in git ingest, add .tar.gz bundler --- internal/engine/eval/rego/lib.go | 625 ++++++++++++++++------- internal/engine/eval/rego/lib_test.go | 89 ++++ internal/engine/ingester/deps/deps.go | 2 +- internal/engine/ingester/git/git.go | 121 +++-- internal/engine/ingester/git/git_test.go | 14 +- pkg/engine/v1/interfaces/interfaces.go | 3 + 6 files changed, 596 insertions(+), 258 deletions(-) diff --git a/internal/engine/eval/rego/lib.go b/internal/engine/eval/rego/lib.go index 9ce398a87f..6dfe9b4e79 100644 --- a/internal/engine/eval/rego/lib.go +++ b/internal/engine/eval/rego/lib.go @@ -4,6 +4,9 @@ package rego import ( + "archive/tar" + "bytes" + "compress/gzip" "context" "errors" "fmt" @@ -12,6 +15,7 @@ import ( "net/http" "os" "path/filepath" + "time" "github.com/go-git/go-billy/v5" billyutil "github.com/go-git/go-billy/v5/util" @@ -34,7 +38,16 @@ var MinderRegoLib = []func(res *interfaces.Result) func(*rego.Rego){ FileHTTPType, FileRead, FileWalk, + FileBundle, ListGithubActions, + BaseFileExists, + BaseFileLs, + BaseFileLsGlob, + BaseFileHTTPType, + BaseFileRead, + BaseFileWalk, + BaseListGithubActions, + BaseFileBundle, ParseYaml, JQIsTrue, } @@ -57,30 +70,47 @@ func FileExists(res *interfaces.Result) func(*rego.Rego) { Name: "file.exists", Decl: types.NewFunction(types.Args(types.S), types.B), }, - func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { - var path string - if err := ast.As(op1.Value, &path); err != nil { - return nil, err - } + fsExists(res.Fs), + ) +} - if res.Fs == nil { - return nil, fmt.Errorf("cannot check file existence without a filesystem") - } +// BaseFileExists is a rego function that checks if a file exists +// in the base filesystem from the ingester. Base filesystems are +// typically associated with pull requests. +// It takes one argument, the path to the file to check. +// It's exposed as `base_file.exists`. +func BaseFileExists(res *interfaces.Result) func(*rego.Rego) { + return rego.Function1( + ®o.Function{ + Name: "base_file.exists", + Decl: types.NewFunction(types.Args(types.S), types.B), + }, + fsExists(res.BaseFs), + ) +} - fs := res.Fs +func fsExists(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.Term, error) { + return func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { + var path string + if err := ast.As(op1.Value, &path); err != nil { + return nil, err + } - cpath := filepath.Clean(path) - _, err := fs.Stat(cpath) - if err != nil { - if errors.Is(err, os.ErrNotExist) { - return ast.BooleanTerm(false), nil - } - return nil, err + if vfs == nil { + return nil, fmt.Errorf("cannot check file existence without a filesystem") + } + + cpath := filepath.Clean(path) + _, err := vfs.Stat(cpath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return ast.BooleanTerm(false), nil } + return nil, err + } - return ast.BooleanTerm(true), nil - }, - ) + return ast.BooleanTerm(true), nil + } } // FileRead is a rego function that reads a file from the filesystem @@ -92,35 +122,51 @@ func FileRead(res *interfaces.Result) func(*rego.Rego) { Name: "file.read", Decl: types.NewFunction(types.Args(types.S), types.S), }, - func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { - var path string - if err := ast.As(op1.Value, &path); err != nil { - return nil, err - } + fsRead(res.Fs), + ) +} - if res.Fs == nil { - return nil, fmt.Errorf("cannot read file without a filesystem") - } +// BaseFileRead is a rego function that reads a file from the +// base filesystem in a pull_request or other diff context. +// It takes one argument, the path to the file to read. +// It's exposed as `base_file.read`. +func BaseFileRead(res *interfaces.Result) func(*rego.Rego) { + return rego.Function1( + ®o.Function{ + Name: "base_file.read", + Decl: types.NewFunction(types.Args(types.S), types.S), + }, + fsRead(res.BaseFs), + ) +} - fs := res.Fs +func fsRead(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.Term, error) { + return func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { + var path string + if err := ast.As(op1.Value, &path); err != nil { + return nil, err + } - cpath := filepath.Clean(path) - f, err := fs.Open(cpath) - if err != nil { - return nil, err - } + if vfs == nil { + return nil, fmt.Errorf("cannot read file without a filesystem") + } - defer f.Close() + cpath := filepath.Clean(path) + f, err := vfs.Open(cpath) + if err != nil { + return nil, err + } - all, rerr := io.ReadAll(f) - if rerr != nil { - return nil, rerr - } + defer f.Close() - allstr := ast.String(all) - return ast.NewTerm(allstr), nil - }, - ) + all, rerr := io.ReadAll(f) + if rerr != nil { + return nil, rerr + } + + allstr := ast.String(all) + return ast.NewTerm(allstr), nil + } } // FileLs is a rego function that lists the files in a directory @@ -137,64 +183,84 @@ func FileLs(res *interfaces.Result) func(*rego.Rego) { Name: "file.ls", Decl: types.NewFunction(types.Args(types.S), types.A), }, - func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { - var path string - if err := ast.As(op1.Value, &path); err != nil { - return nil, err - } + fsLs(res.Fs), + ) +} - if res.Fs == nil { - return nil, fmt.Errorf("cannot walk file without a filesystem") - } +// BaseFileLs is a rego function that lists the files in a directory +// in the base filesystem being evaluated (in a pull_request or other +// diff context). It takes one argument, the path to the directory to list. +// It's exposed as `base_file.ls`. +// If the file is a file, it returns the file itself. +// If the file is a directory, it returns the files in the directory. +// If the file is a symlink, it follows the symlink and returns the files +// in the target. +func BaseFileLs(res *interfaces.Result) func(*rego.Rego) { + return rego.Function1( + ®o.Function{ + Name: "base_file.ls", + Decl: types.NewFunction(types.Args(types.S), types.A), + }, + fsLs(res.BaseFs), + ) +} - fs := res.Fs +func fsLs(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.Term, error) { + return func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { + var path string + if err := ast.As(op1.Value, &path); err != nil { + return nil, err + } + + if vfs == nil { + return nil, fmt.Errorf("cannot walk file without a filesystem") + } + + // Check file information and return a list of files + // and directories + finfo, err := vfs.Lstat(path) + if err != nil { + return fileLsHandleError(err) + } + + // If the file is a file return the file itself + if finfo.Mode().IsRegular() { + return fileLsHandleFile(path) + } + + // If the file is a directory return the files in the directory + if finfo.Mode().IsDir() { + return fileLsHandleDir(path, vfs) + } + + // If the file is a symlink, follow it + if finfo.Mode()&os.ModeSymlink != 0 { + // Get the target of the symlink + target, err := vfs.Readlink(path) + if err != nil { + return nil, err + } - // Check file information and return a list of files - // and directories - finfo, err := fs.Lstat(path) + // Get the file information of the target + // NOTE: This overwrites the previous finfo + finfo, err = vfs.Lstat(target) if err != nil { return fileLsHandleError(err) } - // If the file is a file return the file itself + // If the target is a file return the file itself if finfo.Mode().IsRegular() { - return fileLsHandleFile(path) + return fileLsHandleFile(target) } - // If the file is a directory return the files in the directory + // If the target is a directory return the files in the directory if finfo.Mode().IsDir() { - return fileLsHandleDir(path, fs) - } - - // If the file is a symlink, follow it - if finfo.Mode()&os.ModeSymlink != 0 { - // Get the target of the symlink - target, err := fs.Readlink(path) - if err != nil { - return nil, err - } - - // Get the file information of the target - // NOTE: This overwrites the previous finfo - finfo, err = fs.Lstat(target) - if err != nil { - return fileLsHandleError(err) - } - - // If the target is a file return the file itself - if finfo.Mode().IsRegular() { - return fileLsHandleFile(target) - } - - // If the target is a directory return the files in the directory - if finfo.Mode().IsDir() { - return fileLsHandleDir(target, fs) - } + return fileLsHandleDir(target, vfs) } + } - return nil, fmt.Errorf("cannot handle file type %s", finfo.Mode()) - }, - ) + return nil, fmt.Errorf("cannot handle file type %s", finfo.Mode()) + } } // FileLsGlob is a rego function that lists the files matching a glob in a directory @@ -207,33 +273,50 @@ func FileLsGlob(res *interfaces.Result) func(*rego.Rego) { Name: "file.ls_glob", Decl: types.NewFunction(types.Args(types.S), types.A), }, - func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { - var path string - if err := ast.As(op1.Value, &path); err != nil { - return nil, err - } + fsLsGlob(res.Fs), + ) +} - if res.Fs == nil { - return nil, fmt.Errorf("cannot walk file without a filesystem") - } +// BaseFileLsGlob is a rego function that lists the files matching a glob +// in a directory in the base filesystem being evaluated (in a pull_request +// or other diff context). +// It takes one argument, the path to the pattern to match. It's exposed +// as `base_file.ls_glob`. +func BaseFileLsGlob(res *interfaces.Result) func(*rego.Rego) { + return rego.Function1( + ®o.Function{ + Name: "base_file.ls_glob", + Decl: types.NewFunction(types.Args(types.S), types.A), + }, + fsLsGlob(res.BaseFs), + ) +} - rfs := res.Fs +func fsLsGlob(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.Term, error) { + return func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { + var path string + if err := ast.As(op1.Value, &path); err != nil { + return nil, err + } - matches, err := billyutil.Glob(rfs, path) - files := []*ast.Term{} + if vfs == nil { + return nil, fmt.Errorf("cannot walk file without a filesystem") + } - for _, m := range matches { - files = append(files, ast.NewTerm(ast.String(m))) - } + matches, err := billyutil.Glob(vfs, path) + files := []*ast.Term{} - if err != nil { - return nil, err - } + for _, m := range matches { + files = append(files, ast.NewTerm(ast.String(m))) + } - return ast.NewTerm( - ast.NewArray(files...)), nil - }, - ) + if err != nil { + return nil, err + } + + return ast.NewTerm( + ast.NewArray(files...)), nil + } } // FileWalk is a rego function that walks the files in a directory @@ -246,54 +329,71 @@ func FileWalk(res *interfaces.Result) func(*rego.Rego) { Name: "file.walk", Decl: types.NewFunction(types.Args(types.S), types.A), }, - func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { - var path string - if err := ast.As(op1.Value, &path); err != nil { - return nil, err - } - - if res.Fs == nil { - return nil, fmt.Errorf("cannot walk file without a filesystem") - } + fsWalk(res.Fs), + ) +} - rfs := res.Fs +// BaseFileWalk is a rego function that walks the files in a directory +// in the base filesystem being evaluated (in a pull_request or other +// diff context). +// It takes one argument, the path to the directory to walk. It's exposed +// as `base_file.walk`. +func BaseFileWalk(res *interfaces.Result) func(*rego.Rego) { + return rego.Function1( + ®o.Function{ + Name: "base_file.walk", + Decl: types.NewFunction(types.Args(types.S), types.A), + }, + fsWalk(res.BaseFs), + ) +} - // if the path is a file, return the file itself - // Check file information and return a list of files - // and directories - finfo, err := rfs.Lstat(path) +func fsWalk(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.Term, error) { + return func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { + var path string + if err := ast.As(op1.Value, &path); err != nil { + return nil, err + } + + if vfs == nil { + return nil, fmt.Errorf("cannot walk file without a filesystem") + } + + // if the path is a file, return the file itself + // Check file information and return a list of files + // and directories + finfo, err := vfs.Lstat(path) + if err != nil { + return fileLsHandleError(err) + } + + // If the file is a file return the file itself + if finfo.Mode().IsRegular() { + return fileLsHandleFile(path) + } + + files := []*ast.Term{} + err = billyutil.Walk(vfs, path, func(path string, info fs.FileInfo, err error) error { + // skip if error if err != nil { - return fileLsHandleError(err) - } - - // If the file is a file return the file itself - if finfo.Mode().IsRegular() { - return fileLsHandleFile(path) + return nil } - files := []*ast.Term{} - err = billyutil.Walk(rfs, path, func(path string, info fs.FileInfo, err error) error { - // skip if error - if err != nil { - return nil - } - - // skip if directory - if info.IsDir() { - return nil - } - - files = append(files, ast.NewTerm(ast.String(path))) + // skip if directory + if info.IsDir() { return nil - }) - if err != nil { - return nil, err } - return ast.NewTerm( - ast.NewArray(files...)), nil - }, - ) + files = append(files, ast.NewTerm(ast.String(path))) + return nil + }) + if err != nil { + return nil, err + } + + return ast.NewTerm( + ast.NewArray(files...)), nil + } } func fileLsHandleError(err error) (*ast.Term, error) { @@ -328,6 +428,99 @@ func fileLsHandleDir(path string, bfs billy.Filesystem) (*ast.Term, error) { ast.NewArray(files...)), nil } +// FileBundle packages a set of files form the the specified directory into +// a tarball. It takes one argument: a list of file or directory paths to +// include, and outputs the tarball as a string. +// It's exposed as 'file.bundle`. +func FileBundle(res *interfaces.Result) func(*rego.Rego) { + return rego.Function1( + ®o.Function{ + Name: "file.bundle", + Decl: types.NewFunction(types.Args(types.NewArray(nil, types.S)), types.S), + }, + fsBundle(res.Fs), + ) +} + +// BaseFileBundle packages a set of files form the the specified directory +// in the base filesystem (from a pull_request or other diff context) into +// a tarball. It takes one argument: a list of file or directory paths to +// include, and outputs the tarball as a string. +// It's exposed as 'base_file.bundle`. +func BaseFileBundle(res *interfaces.Result) func(*rego.Rego) { + return rego.Function1( + ®o.Function{ + Name: "base_file.bundle", + Decl: types.NewFunction(types.Args(types.NewArray(nil, types.S)), types.S), + }, + fsBundle(res.BaseFs), + ) +} + +func fsBundle(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.Term, error) { + return func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { + var paths []string + if err := ast.As(op1.Value, &paths); err != nil { + return nil, err + } + + if vfs == nil { + return nil, fmt.Errorf("cannot bundle files without a filesystem") + } + + out := bytes.Buffer{} + gzWriter := gzip.NewWriter(&out) + defer gzWriter.Close() + tarWriter := tar.NewWriter(gzWriter) + defer tarWriter.Close() + + for _, f := range paths { + fmt.Printf("++ Adding %s\n", f) + err := billyutil.Walk(vfs, f, func(path string, info fs.FileInfo, err error) error { + if err != nil { + return err + } + fileHeader, err := tar.FileInfoHeader(info, "") + if err != nil { + return err + } + fileHeader.Name = path + // memfs doesn't return change times anyway, so zero them for consistency + fileHeader.ModTime = time.Time{} + fileHeader.AccessTime = time.Time{} + fileHeader.ChangeTime = time.Time{} + if err := tarWriter.WriteHeader(fileHeader); err != nil { + return err + } + if info.Mode().IsRegular() { + file, err := vfs.Open(path) + if err != nil { + return err + } + defer file.Close() + if _, err := io.Copy(tarWriter, file); err != nil { + return err + } + } + fmt.Printf(" -> Added %s (%s): %d\n", path, info.Mode(), out.Len()) + return nil + }) + if err != nil { + return nil, err + } + } + + if err := tarWriter.Close(); err != nil { + return nil, err + } + if err := gzWriter.Close(); err != nil { + return nil, err + } + + return ast.StringTerm(out.String()), nil + } +} + // ListGithubActions is a rego function that lists the actions in a directory // in the filesystem being evaluated (which comes from the ingester). // It takes one argument, the path to the directory to list. It's exposed @@ -340,33 +533,53 @@ func ListGithubActions(res *interfaces.Result) func(*rego.Rego) { Name: "github_workflow.ls_actions", Decl: types.NewFunction(types.Args(types.S), types.NewSet(types.S)), }, - func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { - var base string - if err := ast.As(op1.Value, &base); err != nil { - return nil, err - } + fsListGithubActions(res.Fs), + ) +} - if res.Fs == nil { - return nil, fmt.Errorf("cannot list actions without a filesystem") - } +// BaseListGithubActions is a rego function that lists the actions in a directory +// in the base filesystem being evaluated (in a pull_request or diff context). +// It takes one argument, the path to the directory to list. It's exposed +// as `github_workflow.base_ls_actions`. +// The function returns a set of strings, each string being the name of an action. +// The frizbee library guarantees that the actions are unique. +func BaseListGithubActions(res *interfaces.Result) func(*rego.Rego) { + return rego.Function1( + ®o.Function{ + Name: "github_workflow.base_ls_actions", + Decl: types.NewFunction(types.Args(types.S), types.NewSet(types.S)), + }, + fsListGithubActions(res.BaseFs), + ) +} - var terms []*ast.Term +func fsListGithubActions(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.Term, error) { + return func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { + var base string + if err := ast.As(op1.Value, &base); err != nil { + return nil, err + } - // Parse the ingested file system and extract all action references - r := replacer.NewGitHubActionsReplacer(&config.Config{}) - actions, err := r.ListPathInFS(res.Fs, base) - if err != nil { - return nil, err - } + if vfs == nil { + return nil, fmt.Errorf("cannot list actions without a filesystem") + } - // Save the action names - for _, a := range actions.Entities { - terms = append(terms, ast.StringTerm(a.Name)) - } + var terms []*ast.Term - return ast.SetTerm(terms...), nil - }, - ) + // Parse the ingested file system and extract all action references + r := replacer.NewGitHubActionsReplacer(&config.Config{}) + actions, err := r.ListPathInFS(vfs, base) + if err != nil { + return nil, err + } + + // Save the action names + for _, a := range actions.Entities { + terms = append(terms, ast.StringTerm(a.Name)) + } + + return ast.SetTerm(terms...), nil + } } // FileHTTPType is a rego function that returns the HTTP type of a file @@ -379,39 +592,55 @@ func FileHTTPType(res *interfaces.Result) func(*rego.Rego) { Name: "file.http_type", Decl: types.NewFunction(types.Args(types.S), types.S), }, - func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { - var path string - if err := ast.As(op1.Value, &path); err != nil { - return nil, err - } - - if res.Fs == nil { - return nil, fmt.Errorf("cannot list actions without a filesystem") - } - - bfs := res.Fs - - cpath := filepath.Clean(path) - f, err := bfs.Open(cpath) - if err != nil { - return nil, err - } - - defer f.Close() - - buffer := make([]byte, 512) - n, err := f.Read(buffer) - if err != nil && err != io.EOF { - return nil, err - } + fsHTTPType(res.Fs), + ) +} - httpTyp := http.DetectContentType(buffer[:n]) - astHTTPTyp := ast.String(httpTyp) - return ast.NewTerm(astHTTPTyp), nil +// BaseFileHTTPType is a rego function that returns the HTTP type of a file +// in the filesystem being evaluated (which comes from the ingester). +// It takes one argument, the path to the file to check. It's exposed +// as `base_file.http_type`. +func BaseFileHTTPType(res *interfaces.Result) func(*rego.Rego) { + return rego.Function1( + ®o.Function{ + Name: "base_file.http_type", + Decl: types.NewFunction(types.Args(types.S), types.S), }, + fsHTTPType(res.BaseFs), ) } +func fsHTTPType(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.Term, error) { + return func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { + var path string + if err := ast.As(op1.Value, &path); err != nil { + return nil, err + } + + if vfs == nil { + return nil, fmt.Errorf("cannot list actions without a filesystem") + } + + cpath := filepath.Clean(path) + f, err := vfs.Open(cpath) + if err != nil { + return nil, err + } + + defer f.Close() + + buffer := make([]byte, 512) + n, err := f.Read(buffer) + if err != nil && err != io.EOF { + return nil, err + } + + httpTyp := http.DetectContentType(buffer[:n]) + astHTTPTyp := ast.String(httpTyp) + return ast.NewTerm(astHTTPTyp), nil + } +} + // JQIsTrue is a rego function that accepts parsed YAML data and runs a jq query on it. // The query is a string in jq format that returns a boolean. // It returns a boolean indicating whether the jq query matches the parsed YAML data. diff --git a/internal/engine/eval/rego/lib_test.go b/internal/engine/eval/rego/lib_test.go index 6d53620d52..e652361951 100644 --- a/internal/engine/eval/rego/lib_test.go +++ b/internal/engine/eval/rego/lib_test.go @@ -5,12 +5,14 @@ package rego_test import ( "context" + "encoding/base64" "errors" "fmt" "testing" "time" memfs "github.com/go-git/go-billy/v5/memfs" + billyutil "github.com/go-git/go-billy/v5/util" "github.com/stretchr/testify/require" engerrors "github.com/mindersec/minder/internal/engine/errors" @@ -53,6 +55,37 @@ allow { require.NoError(t, err, "could not evaluate") } +func TestFileExistsInBase(t *testing.T) { + t.Parallel() + fs := memfs.New() + + _, err := fs.Create("foo") + require.NoError(t, err, "could not create file") + + e, err := rego.NewRegoEvaluator( + &minderv1.RuleType_Definition_Eval_Rego{ + Type: rego.DenyByDefaultEvaluationType.String(), + Def: ` +package minder + +default allow = false + +allow { + base_file.exists("foo") +}`, + }, + ) + require.NoError(t, err, "could not create evaluator") + + emptyPol := map[string]any{} + + // Matches + err = e.Eval(context.Background(), emptyPol, nil, &interfaces.Result{ + BaseFs: fs, + }) + require.NoError(t, err, "could not evaluate") +} + func TestFileExistsWithNonExistentFile(t *testing.T) { t.Parallel() @@ -750,6 +783,62 @@ allow { require.NoError(t, err, "could not evaluate") } +func TestFileBundle(t *testing.T) { + t.Parallel() + fs := memfs.New() + require.NoError(t, fs.MkdirAll("foo", 0755), "could not create directory") + require.NoError(t, fs.MkdirAll("bar", 0755), "could not create directory") + + require.NoError(t, billyutil.WriteFile(fs, "foo/bar", []byte("bar"), 0644)) + require.NoError(t, billyutil.WriteFile(fs, "foo/baz", []byte("bar"), 0644)) + require.NoError(t, billyutil.WriteFile(fs, "file.txt", []byte("words"), 0644)) + require.NoError(t, billyutil.WriteFile(fs, "README", []byte("docs"), 0644)) + + // N.B. This was constructed by examining the output of the tarball, and + // and verifying by untarring the data with `cat file | tar -tzvf -` + expectedTarball := []byte{ + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 236, 147, 65, 10, 2, 49, 12, 69, 115, + 148, 57, 129, 254, 98, 211, 158, 103, 68, 11, 130, 16, 232, 84, 20, 79, + 47, 234, 202, 34, 10, 74, 170, 67, 243, 54, 153, 213, 252, 52, 159, 151, + 68, 72, 27, 0, 136, 204, 183, 9, 160, 158, 79, 190, 99, 116, 142, 6, 86, + 223, 140, 136, 14, 83, 25, 51, 1, 223, 254, 167, 126, 220, 76, 72, 34, + 203, 245, 152, 85, 51, 174, 247, 8, 222, 191, 232, 127, 245, 216, 191, + 131, 103, 208, 208, 228, 136, 157, 247, 175, 221, 189, 241, 223, 220, + 253, 63, 171, 102, 124, 226, 127, 48, 255, 155, 96, 254, 247, 77, 218, + 237, 183, 139, 114, 42, 154, 25, 239, 253, 231, 218, 255, 96, 254, 183, + 225, 40, 121, 51, 253, 122, 9, 195, 48, 12, 163, 57, 151, 0, 0, 0, 255, + 255, 203, 184, 208, 59, 0, 18, 0, 0, + } + + e, err := rego.NewRegoEvaluator( + &minderv1.RuleType_Definition_Eval_Rego{ + Type: rego.ConstraintsEvaluationType.String(), + Def: ` +package minder +import rego.v1 + +tarball := file.bundle(["foo", "file.txt"]) +encoded := base64.encode(tarball) +expectedTar := base64.decode(input.profile.expected) +violations contains {"msg": sprintf("Expected: %s", [input.profile.expected])} if tarball != expectedTar +violations contains {"msg": sprintf("Got : %s", [encoded])} if tarball != expectedTar +`, + }, + ) + require.NoError(t, err, "could not create evaluator") + + policy := map[string]any{ + // Encode to string in Go, to force checking that Go & Rego perform the same encoding + "expected": base64.StdEncoding.EncodeToString(expectedTarball), + } + + err = e.Eval(context.Background(), policy, nil, &interfaces.Result{ + Object: nil, + Fs: fs, + }) + require.NoError(t, err, "could not evaluate") +} + func TestJQIsTrue(t *testing.T) { t.Parallel() diff --git a/internal/engine/ingester/deps/deps.go b/internal/engine/ingester/deps/deps.go index 8c4fee55cc..a9398a4e15 100644 --- a/internal/engine/ingester/deps/deps.go +++ b/internal/engine/ingester/deps/deps.go @@ -87,7 +87,7 @@ func (gi *Deps) Ingest(ctx context.Context, ent protoreflect.ProtoMessage, param case *pbinternal.PullRequest: return gi.ingestPullRequest(ctx, entity, params) default: - return nil, fmt.Errorf("deps is only supported for repositories") + return nil, fmt.Errorf("deps is only supported for repositories and pull requests") } } diff --git a/internal/engine/ingester/git/git.go b/internal/engine/ingester/git/git.go index 4708663a9e..39ce5fd31c 100644 --- a/internal/engine/ingester/git/git.go +++ b/internal/engine/ingester/git/git.go @@ -5,10 +5,14 @@ package git import ( + "cmp" "context" "errors" "fmt" + "github.com/go-git/go-billy/v5" + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/storage" "github.com/go-viper/mapstructure/v2" "google.golang.org/protobuf/reflect/protoreflect" @@ -60,41 +64,31 @@ func (gi *Git) GetConfig() protoreflect.ProtoMessage { // Ingest does the actual data ingestion for a rule type by cloning a git repo func (gi *Git) Ingest(ctx context.Context, ent protoreflect.ProtoMessage, params map[string]any) (*interfaces.Result, error) { + switch entity := ent.(type) { + case *pb.Repository: + return gi.ingestRepository(ctx, entity, params) + case *pbinternal.PullRequest: + return gi.ingestPullRequest(ctx, entity, params) + default: + return nil, fmt.Errorf("git is only supported for repositories and pull requests") + } +} + +func (gi *Git) ingestRepository(ctx context.Context, repo *pb.Repository, params map[string]any) (*interfaces.Result, error) { userCfg := &IngesterConfig{} if err := mapstructure.Decode(params, userCfg); err != nil { return nil, fmt.Errorf("failed to read git ingester configuration from params: %w", err) } - url := getCloneUrl(ent, userCfg) + url := cmp.Or(userCfg.CloneURL, repo.GetCloneUrl()) if url == "" { return nil, fmt.Errorf("could not get clone url") } - branch := gi.getBranch(ent, userCfg) - - // We clone to the memfs go-billy filesystem driver, which doesn't - // allow for direct access to the underlying filesystem. This is - // because we want to be able to run this in a sandboxed environment - // where we don't have access to the underlying filesystem. - r, err := gi.gitprov.Clone(ctx, url, branch) + branch := cmp.Or(userCfg.Branch, gi.cfg.Branch, repo.GetDefaultBranch(), defaultBranch) + fs, storer, head, err := gi.fetchClone(ctx, url, branch) if err != nil { - if errors.Is(err, provifv1.ErrProviderGitBranchNotFound) { - return nil, fmt.Errorf("%w: %s: branch %s", engerrors.ErrEvaluationFailed, - provifv1.ErrProviderGitBranchNotFound, branch) - } else if errors.Is(err, provifv1.ErrRepositoryEmpty) { - return nil, fmt.Errorf("%w: %s", engerrors.ErrEvaluationSkipped, provifv1.ErrRepositoryEmpty) - } - return nil, err - } - - wt, err := r.Worktree() - if err != nil { - return nil, fmt.Errorf("could not get worktree: %w", err) - } - - head, err := r.Head() - if err != nil { - return nil, fmt.Errorf("could not get head: %w", err) + return nil, fmt.Errorf("failed to clone %s from %s: %w", branch, url, err) } hsh := head.Hash() @@ -105,47 +99,70 @@ func (gi *Git) Ingest(ctx context.Context, ent protoreflect.ProtoMessage, params return &interfaces.Result{ Object: nil, - Fs: wt.Filesystem, - Storer: r.Storer, + Fs: fs, + Storer: storer, Checkpoint: chkpoint, }, nil } -func (gi *Git) getBranch(ent protoreflect.ProtoMessage, userCfg *IngesterConfig) string { - // If the user has specified a branch, use that - if userCfg.Branch != "" { - return userCfg.Branch +func (gi *Git) ingestPullRequest(ctx context.Context, ent *pbinternal.PullRequest, params map[string]any) (*interfaces.Result, error) { + userCfg := &IngesterConfig{} + if err := mapstructure.Decode(params, userCfg); err != nil { + return nil, fmt.Errorf("failed to read git ingester configuration from params: %w", err) } - // If the branch is provided in the rule-type - // configuration, use that - if gi.cfg.Branch != "" { - return gi.cfg.Branch + if ent.GetBaseCloneUrl() == "" || ent.GetBaseRef() == "" { + return nil, fmt.Errorf("could not get PR base branch %q from %q", ent.GetBaseRef(), ent.GetBaseCloneUrl()) + } + if ent.GetTargetCloneUrl() == "" || ent.GetTargetRef() == "" { + return nil, fmt.Errorf("could not get PR target branch %q from %q", ent.GetTargetRef(), ent.GetTargetCloneUrl()) } - if repo, ok := ent.(*pb.Repository); ok { - if repo.GetDefaultBranch() != "" { - return repo.GetDefaultBranch() - } - } else if pr, ok := ent.(*pbinternal.PullRequest); ok { - return pr.GetTargetRef() + baseFs, _, _, err := gi.fetchClone(ctx, ent.GetBaseCloneUrl(), ent.GetBaseRef()) + if err != nil { + return nil, fmt.Errorf("failed to clone base branch %s from %s: %w", ent.GetBaseRef(), ent.GetBaseCloneUrl(), err) + } + targetFs, storer, head, err := gi.fetchClone(ctx, ent.GetTargetCloneUrl(), ent.GetTargetRef()) + if err != nil { + return nil, fmt.Errorf("failed to clone target branch %s from %s: %w", ent.GetTargetRef(), ent.GetTargetCloneUrl(), err) } - // If the branch is not provided in the rule-type - // configuration, use the default branch - return defaultBranch + checkpoint := checkpoints.NewCheckpointV1Now().WithBranch(ent.GetTargetRef()).WithCommitHash(head.Hash().String()) + + return &interfaces.Result{ + Object: nil, + Fs: targetFs, + Storer: storer, + BaseFs: baseFs, + Checkpoint: checkpoint, + }, nil } -func getCloneUrl(ent protoreflect.ProtoMessage, cfg *IngesterConfig) string { - if cfg.CloneURL != "" { - return cfg.CloneURL +func (gi *Git) fetchClone(ctx context.Context, url, branch string) (billy.Filesystem, storage.Storer, *plumbing.Reference, error) { + // We clone to the memfs go-billy filesystem driver, which doesn't + // allow for direct access to the underlying filesystem. This is + // because we want to be able to run this in a sandboxed environment + // where we don't have access to the underlying filesystem. + r, err := gi.gitprov.Clone(ctx, url, branch) + if err != nil { + if errors.Is(err, provifv1.ErrProviderGitBranchNotFound) { + return nil, nil, nil, fmt.Errorf("%w: %s: branch %s", engerrors.ErrEvaluationFailed, + provifv1.ErrProviderGitBranchNotFound, branch) + } else if errors.Is(err, provifv1.ErrRepositoryEmpty) { + return nil, nil, nil, fmt.Errorf("%w: %s", engerrors.ErrEvaluationSkipped, provifv1.ErrRepositoryEmpty) + } + return nil, nil, nil, err + } + + wt, err := r.Worktree() + if err != nil { + return nil, nil, nil, fmt.Errorf("could not get worktree: %w", err) } - if repo, ok := ent.(*pb.Repository); ok { - return repo.GetCloneUrl() - } else if pr, ok := ent.(*pbinternal.PullRequest); ok { - return pr.GetTargetCloneUrl() + head, err := r.Head() + if err != nil { + return nil, nil, nil, fmt.Errorf("could not get head: %w", err) } - return "" + return wt.Filesystem, r.Storer, head, err } diff --git a/internal/engine/ingester/git/git_test.go b/internal/engine/ingester/git/git_test.go index ee71007bc3..a73b8b5834 100644 --- a/internal/engine/ingester/git/git_test.go +++ b/internal/engine/ingester/git/git_test.go @@ -72,7 +72,7 @@ func TestGitIngestWithCloneURLFromParams(t *testing.T) { ) require.NoError(t, err, "expected no error") - got, err := gi.Ingest(context.Background(), &pb.Artifact{}, map[string]any{ + got, err := gi.Ingest(context.Background(), &pb.Repository{}, map[string]any{ "clone_url": "https://github.com/octocat/Hello-World.git", }) require.NoError(t, err, "expected no error") @@ -100,7 +100,7 @@ func TestGitIngestWithCustomBranchFromParams(t *testing.T) { ) require.NoError(t, err, "expected no error") - got, err := gi.Ingest(context.Background(), &pb.Artifact{}, map[string]any{ + got, err := gi.Ingest(context.Background(), &pb.Repository{}, map[string]any{ "clone_url": "https://github.com/octocat/Hello-World.git", "branch": "test", }) @@ -161,7 +161,7 @@ func TestGitIngestWithUnexistentBranchFromParams(t *testing.T) { require.NoError(t, err, "expected no error") - got, err := gi.Ingest(context.Background(), &pb.Artifact{}, map[string]any{ + got, err := gi.Ingest(context.Background(), &pb.Repository{}, map[string]any{ "clone_url": "https://github.com/octocat/Hello-World.git", "branch": "unexistent-branch", }) @@ -181,7 +181,7 @@ func TestGitIngestFailsBecauseOfAuthorization(t *testing.T) { require.NoError(t, err, "expected no error") - got, err := gi.Ingest(context.Background(), &pb.Artifact{}, map[string]any{ + got, err := gi.Ingest(context.Background(), &pb.Repository{}, map[string]any{ "clone_url": "https://github.com/mindersec/minder.git", }) require.Error(t, err, "expected error") @@ -195,7 +195,7 @@ func TestGitIngestFailsBecauseOfUnexistentCloneUrl(t *testing.T) { &pb.GitType{}, testproviders.NewGitProvider(credentials.NewEmptyCredential())) require.NoError(t, err, "expected no error") - got, err := gi.Ingest(context.Background(), &pb.Artifact{}, map[string]any{ + got, err := gi.Ingest(context.Background(), &pb.Repository{}, map[string]any{ "clone_url": "https://github.com/octocat/unexistent-git-repo.git", }) require.Error(t, err, "expected error") @@ -222,7 +222,7 @@ func TestGitIngestFailsWhenRepoTooLarge(t *testing.T) { require.NoError(t, err, "expected no error") - got, err := gi.Ingest(context.Background(), &pb.Artifact{}, map[string]any{ + got, err := gi.Ingest(context.Background(), &pb.Repository{}, map[string]any{ "clone_url": "https://github.com/octocat/Hello-World.git", }) require.Error(t, err, "expected error") @@ -250,7 +250,7 @@ func TestGitIngestFailsWhenRepoHasTooManyFiles(t *testing.T) { require.NoError(t, err, "expected no error") - got, err := gi.Ingest(context.Background(), &pb.Artifact{}, map[string]any{ + got, err := gi.Ingest(context.Background(), &pb.Repository{}, map[string]any{ "clone_url": "https://github.com/octocat/Hello-World.git", }) require.Error(t, err, "expected error") diff --git a/pkg/engine/v1/interfaces/interfaces.go b/pkg/engine/v1/interfaces/interfaces.go index f7f499ac6a..8c3e400ada 100644 --- a/pkg/engine/v1/interfaces/interfaces.go +++ b/pkg/engine/v1/interfaces/interfaces.go @@ -38,6 +38,9 @@ type Result struct { // is normally used by the evaluator to do rule evaluation. The filesystem // may be a git repo, or a memory filesystem. Fs billy.Filesystem + // BaseFs is the base filesystem for a pull request. It can be used in the + // evaluator for diffing the PR target files against the base files. + BaseFs billy.Filesystem // Storer is the git storer that was created as a result of the ingestion. // FIXME: It might be cleaner to either wrap both Fs and Storer in a struct // or pass out the git.Repository structure instead of the storer. From 335c6b9d2fb3cbb2634bc6f8d81b1c8f9ad997ae Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Wed, 11 Dec 2024 06:12:45 -0800 Subject: [PATCH 2/7] Fix line-too-long lint error --- internal/engine/ingester/git/git.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/engine/ingester/git/git.go b/internal/engine/ingester/git/git.go index 39ce5fd31c..78269646f4 100644 --- a/internal/engine/ingester/git/git.go +++ b/internal/engine/ingester/git/git.go @@ -105,7 +105,8 @@ func (gi *Git) ingestRepository(ctx context.Context, repo *pb.Repository, params }, nil } -func (gi *Git) ingestPullRequest(ctx context.Context, ent *pbinternal.PullRequest, params map[string]any) (*interfaces.Result, error) { +func (gi *Git) ingestPullRequest( + ctx context.Context, ent *pbinternal.PullRequest, params map[string]any) (*interfaces.Result, error) { userCfg := &IngesterConfig{} if err := mapstructure.Decode(params, userCfg); err != nil { return nil, fmt.Errorf("failed to read git ingester configuration from params: %w", err) @@ -138,7 +139,8 @@ func (gi *Git) ingestPullRequest(ctx context.Context, ent *pbinternal.PullReques }, nil } -func (gi *Git) fetchClone(ctx context.Context, url, branch string) (billy.Filesystem, storage.Storer, *plumbing.Reference, error) { +func (gi *Git) fetchClone( + ctx context.Context, url, branch string) (billy.Filesystem, storage.Storer, *plumbing.Reference, error) { // We clone to the memfs go-billy filesystem driver, which doesn't // allow for direct access to the underlying filesystem. This is // because we want to be able to run this in a sandboxed environment From 889ac19807a1960078d471fc50135082dd3fc3c5 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Wed, 11 Dec 2024 10:29:17 -0800 Subject: [PATCH 3/7] Rename 'bundle' to 'archive' per feedback from Ed --- internal/engine/eval/rego/lib.go | 28 +++++++++++++-------------- internal/engine/eval/rego/lib_test.go | 4 ++-- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/internal/engine/eval/rego/lib.go b/internal/engine/eval/rego/lib.go index 6dfe9b4e79..97e80c6ca2 100644 --- a/internal/engine/eval/rego/lib.go +++ b/internal/engine/eval/rego/lib.go @@ -38,7 +38,7 @@ var MinderRegoLib = []func(res *interfaces.Result) func(*rego.Rego){ FileHTTPType, FileRead, FileWalk, - FileBundle, + FileArchive, ListGithubActions, BaseFileExists, BaseFileLs, @@ -47,7 +47,7 @@ var MinderRegoLib = []func(res *interfaces.Result) func(*rego.Rego){ BaseFileRead, BaseFileWalk, BaseListGithubActions, - BaseFileBundle, + BaseFileArchive, ParseYaml, JQIsTrue, } @@ -428,36 +428,36 @@ func fileLsHandleDir(path string, bfs billy.Filesystem) (*ast.Term, error) { ast.NewArray(files...)), nil } -// FileBundle packages a set of files form the the specified directory into +// FileArchive packages a set of files form the the specified directory into // a tarball. It takes one argument: a list of file or directory paths to // include, and outputs the tarball as a string. -// It's exposed as 'file.bundle`. -func FileBundle(res *interfaces.Result) func(*rego.Rego) { +// It's exposed as 'file.archive`. +func FileArchive(res *interfaces.Result) func(*rego.Rego) { return rego.Function1( ®o.Function{ - Name: "file.bundle", + Name: "file.archive", Decl: types.NewFunction(types.Args(types.NewArray(nil, types.S)), types.S), }, - fsBundle(res.Fs), + fsArchive(res.Fs), ) } -// BaseFileBundle packages a set of files form the the specified directory +// BaseFileArchive packages a set of files form the the specified directory // in the base filesystem (from a pull_request or other diff context) into // a tarball. It takes one argument: a list of file or directory paths to // include, and outputs the tarball as a string. -// It's exposed as 'base_file.bundle`. -func BaseFileBundle(res *interfaces.Result) func(*rego.Rego) { +// It's exposed as 'base_file.archive`. +func BaseFileArchive(res *interfaces.Result) func(*rego.Rego) { return rego.Function1( ®o.Function{ - Name: "base_file.bundle", + Name: "base_file.archive", Decl: types.NewFunction(types.Args(types.NewArray(nil, types.S)), types.S), }, - fsBundle(res.BaseFs), + fsArchive(res.BaseFs), ) } -func fsBundle(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.Term, error) { +func fsArchive(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.Term, error) { return func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) { var paths []string if err := ast.As(op1.Value, &paths); err != nil { @@ -465,7 +465,7 @@ func fsBundle(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast.T } if vfs == nil { - return nil, fmt.Errorf("cannot bundle files without a filesystem") + return nil, fmt.Errorf("cannot archive files without a filesystem") } out := bytes.Buffer{} diff --git a/internal/engine/eval/rego/lib_test.go b/internal/engine/eval/rego/lib_test.go index e652361951..47fa078ab1 100644 --- a/internal/engine/eval/rego/lib_test.go +++ b/internal/engine/eval/rego/lib_test.go @@ -783,7 +783,7 @@ allow { require.NoError(t, err, "could not evaluate") } -func TestFileBundle(t *testing.T) { +func TestFileArchive(t *testing.T) { t.Parallel() fs := memfs.New() require.NoError(t, fs.MkdirAll("foo", 0755), "could not create directory") @@ -817,7 +817,7 @@ func TestFileBundle(t *testing.T) { package minder import rego.v1 -tarball := file.bundle(["foo", "file.txt"]) +tarball := file.archive(["foo", "file.txt"]) encoded := base64.encode(tarball) expectedTar := base64.decode(input.profile.expected) violations contains {"msg": sprintf("Expected: %s", [input.profile.expected])} if tarball != expectedTar From ed1cb1d262c7474444fe29e9b87210c8f974d663 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Thu, 12 Dec 2024 13:42:42 -0800 Subject: [PATCH 4/7] Clean up some extra logging, add more details on PR evaluation --- internal/engine/eval/rego/lib.go | 2 -- internal/engine/ingester/git/git.go | 1 + internal/providers/github/webhook/handlers_pull_requests.go | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/engine/eval/rego/lib.go b/internal/engine/eval/rego/lib.go index 97e80c6ca2..8eea00ef7d 100644 --- a/internal/engine/eval/rego/lib.go +++ b/internal/engine/eval/rego/lib.go @@ -475,7 +475,6 @@ func fsArchive(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast. defer tarWriter.Close() for _, f := range paths { - fmt.Printf("++ Adding %s\n", f) err := billyutil.Walk(vfs, f, func(path string, info fs.FileInfo, err error) error { if err != nil { return err @@ -502,7 +501,6 @@ func fsArchive(vfs billy.Filesystem) func(rego.BuiltinContext, *ast.Term) (*ast. return err } } - fmt.Printf(" -> Added %s (%s): %d\n", path, info.Mode(), out.Len()) return nil }) if err != nil { diff --git a/internal/engine/ingester/git/git.go b/internal/engine/ingester/git/git.go index 78269646f4..c9f6993fe5 100644 --- a/internal/engine/ingester/git/git.go +++ b/internal/engine/ingester/git/git.go @@ -107,6 +107,7 @@ func (gi *Git) ingestRepository(ctx context.Context, repo *pb.Repository, params func (gi *Git) ingestPullRequest( ctx context.Context, ent *pbinternal.PullRequest, params map[string]any) (*interfaces.Result, error) { + // TODO: we don't actually have any configuration here. Do we need to read the configuration? userCfg := &IngesterConfig{} if err := mapstructure.Decode(params, userCfg); err != nil { return nil, fmt.Errorf("failed to read git ingester configuration from params: %w", err) diff --git a/internal/providers/github/webhook/handlers_pull_requests.go b/internal/providers/github/webhook/handlers_pull_requests.go index 59fa2e9823..13390e9139 100644 --- a/internal/providers/github/webhook/handlers_pull_requests.go +++ b/internal/providers/github/webhook/handlers_pull_requests.go @@ -150,7 +150,7 @@ func processPullRequestEvent( WithOriginator(pb.Entity_ENTITY_REPOSITORIES, repoProps). WithProviderImplementsHint(string(db.ProviderTypeGithub)) - l.Info().Msgf("evaluating PR %s\n", event.GetPullRequest().GetURL()) + l.Info().Msgf("evaluating PR %s: %s => %s\n", event.GetPullRequest().GetURL(), event.GetAction(), topic) return &processingResult{topic: topic, wrapper: prMsg}, nil } From b50e41eb475c5d27e2c5005d3898281d6574ea53 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 16 Dec 2024 11:08:45 -0800 Subject: [PATCH 5/7] Flag-protect new functionality --- cmd/dev/app/rule_type/rttst.go | 2 +- internal/controlplane/handlers_ruletype.go | 40 ++++++++++++++++------ internal/engine/eval/eval.go | 4 ++- internal/engine/eval/eval_test.go | 4 +-- internal/engine/eval/rego/eval.go | 18 ++++++---- internal/engine/eval/rego/fuzz_test.go | 1 + internal/engine/eval/rego/lib.go | 33 ++++++++++++------ internal/engine/eval/rego/lib_test.go | 31 +++++++++++++++-- internal/engine/eval/rego/rego_test.go | 19 ++++++++-- internal/engine/executor.go | 1 + internal/engine/rtengine/cache.go | 36 +++++++++++-------- internal/engine/rtengine/cache_test.go | 2 +- internal/flags/constants.go | 4 +++ pkg/engine/v1/rtengine/engine.go | 4 ++- pkg/engine/v1/rtengine/engine_test.go | 2 +- 15 files changed, 146 insertions(+), 55 deletions(-) diff --git a/cmd/dev/app/rule_type/rttst.go b/cmd/dev/app/rule_type/rttst.go index 8e99978fd8..6720bdca7d 100644 --- a/cmd/dev/app/rule_type/rttst.go +++ b/cmd/dev/app/rule_type/rttst.go @@ -197,7 +197,7 @@ func testCmdRun(cmd *cobra.Command, _ []string) error { // TODO: use cobra context here ctx := context.Background() - eng, err := rtengine.NewRuleTypeEngine(ctx, ruletype, prov, options.WithDataSources(dsRegistry)) + eng, err := rtengine.NewRuleTypeEngine(ctx, ruletype, prov, nil /*experiments*/, options.WithDataSources(dsRegistry)) if err != nil { return fmt.Errorf("cannot create rule type engine: %w", err) } diff --git a/internal/controlplane/handlers_ruletype.go b/internal/controlplane/handlers_ruletype.go index 0bc89c4108..42fbf75424 100644 --- a/internal/controlplane/handlers_ruletype.go +++ b/internal/controlplane/handlers_ruletype.go @@ -14,6 +14,7 @@ import ( "github.com/google/uuid" "github.com/microcosm-cc/bluemonday" + "github.com/open-feature/go-sdk/openfeature" "github.com/yuin/goldmark" "github.com/yuin/goldmark/extension" "github.com/yuin/goldmark/parser" @@ -23,6 +24,7 @@ import ( "github.com/mindersec/minder/internal/db" "github.com/mindersec/minder/internal/engine/engcontext" + "github.com/mindersec/minder/internal/engine/ingester/git" "github.com/mindersec/minder/internal/flags" "github.com/mindersec/minder/internal/logger" "github.com/mindersec/minder/internal/util" @@ -175,14 +177,9 @@ func (s *Server) CreateRuleType( return nil, util.UserVisibleError(codes.InvalidArgument, "%s", err) } - ruleDS := crt.GetRuleType().GetDef().GetEval().GetDataSources() - if len(ruleDS) > 0 && !flags.Bool(ctx, s.featureFlags, flags.DataSources) { - return nil, util.UserVisibleError(codes.InvalidArgument, "DataSources feature is disabled") - } - - prCommentAlert := crt.GetRuleType().GetDef().GetAlert().GetPullRequestComment() - if prCommentAlert != nil && !flags.Bool(ctx, s.featureFlags, flags.PRCommentAlert) { - return nil, util.UserVisibleError(codes.InvalidArgument, "Pull request comment alert type is disabled") + ruleDef := crt.GetRuleType().GetDef() + if err := checkRuleDefinitionFlags(ctx, s.featureFlags, ruleDef); err != nil { + return nil, err } newRuleType, err := db.WithTransaction(s.store, func(qtx db.ExtendQuerier) (*minderv1.RuleType, error) { @@ -204,6 +201,27 @@ func (s *Server) CreateRuleType( }, nil } +func checkRuleDefinitionFlags( + ctx context.Context, featureFlags openfeature.IClient, ruleDef *minderv1.RuleType_Definition) *util.NiceStatus { + ruleDS := ruleDef.GetEval().GetDataSources() + if len(ruleDS) > 0 && !flags.Bool(ctx, featureFlags, flags.DataSources) { + return util.UserVisibleError(codes.InvalidArgument, "DataSources feature is disabled") + } + + prCommentAlert := ruleDef.GetAlert().GetPullRequestComment() + if prCommentAlert != nil && !flags.Bool(ctx, featureFlags, flags.PRCommentAlert) { + return util.UserVisibleError(codes.InvalidArgument, "Pull request comment alert type is disabled") + } + + usesGitPR := ruleDef.GetIngest().GetType() == git.GitRuleDataIngestType && + ruleDef.GetInEntity() == minderv1.PullRequestEntity.String() + if usesGitPR && !flags.Bool(ctx, featureFlags, flags.GitPRDiffs) { + return util.UserVisibleError(codes.InvalidArgument, "Git pull request ingest is disabled") + } + + return nil +} + // UpdateRuleType is a method to update a rule type func (s *Server) UpdateRuleType( ctx context.Context, @@ -227,9 +245,9 @@ func (s *Server) UpdateRuleType( return nil, util.UserVisibleError(codes.InvalidArgument, "%s", err) } - ruleDS := urt.GetRuleType().GetDef().GetEval().GetDataSources() - if len(ruleDS) > 0 && !flags.Bool(ctx, s.featureFlags, flags.DataSources) { - return nil, util.UserVisibleError(codes.InvalidArgument, "DataSources feature is disabled") + ruleDef := urt.GetRuleType().GetDef() + if err := checkRuleDefinitionFlags(ctx, s.featureFlags, ruleDef); err != nil { + return nil, err } updatedRuleType, err := db.WithTransaction(s.store, func(qtx db.ExtendQuerier) (*minderv1.RuleType, error) { diff --git a/internal/engine/eval/eval.go b/internal/engine/eval/eval.go index 6d13f6aca1..5f81a4ef3e 100644 --- a/internal/engine/eval/eval.go +++ b/internal/engine/eval/eval.go @@ -19,6 +19,7 @@ import ( minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" "github.com/mindersec/minder/pkg/engine/v1/interfaces" provinfv1 "github.com/mindersec/minder/pkg/providers/v1" + "github.com/open-feature/go-sdk/openfeature" ) // NewRuleEvaluator creates a new rule data evaluator @@ -26,6 +27,7 @@ func NewRuleEvaluator( ctx context.Context, ruletype *minderv1.RuleType, provider provinfv1.Provider, + featureFlags openfeature.IClient, opts ...eoptions.Option, ) (interfaces.Evaluator, error) { e := ruletype.Def.GetEval() @@ -41,7 +43,7 @@ func NewRuleEvaluator( } return jq.NewJQEvaluator(e.GetJq(), opts...) case rego.RegoEvalType: - return rego.NewRegoEvaluator(e.GetRego(), opts...) + return rego.NewRegoEvaluator(e.GetRego(), featureFlags, opts...) case vulncheck.VulncheckEvalType: client, err := provinfv1.As[provinfv1.GitHub](provider) if err != nil { diff --git a/internal/engine/eval/eval_test.go b/internal/engine/eval/eval_test.go index 6939b479ce..4603eccd16 100644 --- a/internal/engine/eval/eval_test.go +++ b/internal/engine/eval/eval_test.go @@ -71,7 +71,7 @@ func TestNewRuleEvaluatorWorks(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, err := eval.NewRuleEvaluator(context.Background(), tt.args.rt, nil) + got, err := eval.NewRuleEvaluator(context.Background(), tt.args.rt, nil, nil) assert.NoError(t, err, "unexpected error") assert.NotNil(t, got, "unexpected nil") }) @@ -146,7 +146,7 @@ func TestNewRuleEvaluatorFails(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, err := eval.NewRuleEvaluator(context.Background(), tt.args.rt, nil) + got, err := eval.NewRuleEvaluator(context.Background(), tt.args.rt, nil, nil) assert.Error(t, err, "should have errored") assert.Nil(t, got, "should be nil") }) diff --git a/internal/engine/eval/rego/eval.go b/internal/engine/eval/rego/eval.go index 7dea5f37e9..ff9628cd6f 100644 --- a/internal/engine/eval/rego/eval.go +++ b/internal/engine/eval/rego/eval.go @@ -9,6 +9,7 @@ import ( "fmt" "os" + "github.com/open-feature/go-sdk/openfeature" "github.com/open-policy-agent/opa/rego" "github.com/open-policy-agent/opa/topdown/print" "google.golang.org/protobuf/reflect/protoreflect" @@ -37,10 +38,11 @@ const ( // It initializes the rego engine and evaluates the rules // The default rego package is "minder" type Evaluator struct { - cfg *Config - regoOpts []func(*rego.Rego) - reseval resultEvaluator - datasources *v1datasources.DataSourceRegistry + cfg *Config + featureFlags openfeature.IClient + regoOpts []func(*rego.Rego) + reseval resultEvaluator + datasources *v1datasources.DataSourceRegistry } // Input is the input for the rego evaluator @@ -66,6 +68,7 @@ var _ print.Hook = (*hook)(nil) // NewRegoEvaluator creates a new rego evaluator func NewRegoEvaluator( cfg *minderv1.RuleType_Definition_Eval_Rego, + featureFlags openfeature.IClient, opts ...eoptions.Option, ) (*Evaluator, error) { c, err := parseConfig(cfg) @@ -76,8 +79,9 @@ func NewRegoEvaluator( re := c.getEvalType() eval := &Evaluator{ - cfg: c, - reseval: re, + cfg: c, + featureFlags: featureFlags, + reseval: re, regoOpts: []func(*rego.Rego){ re.getQuery(), rego.Module(MinderRegoFile, c.Def), @@ -119,7 +123,7 @@ func (e *Evaluator) Eval( regoFuncOptions := []func(*rego.Rego){} // Initialize the built-in minder library rego functions - regoFuncOptions = append(regoFuncOptions, instantiateRegoLib(res)...) + regoFuncOptions = append(regoFuncOptions, instantiateRegoLib(ctx, e.featureFlags, res)...) // If the evaluator has data sources defined, expose their functions regoFuncOptions = append(regoFuncOptions, buildDataSourceOptions(res, e.datasources)...) diff --git a/internal/engine/eval/rego/fuzz_test.go b/internal/engine/eval/rego/fuzz_test.go index 53fc19a16e..8988b17dc1 100644 --- a/internal/engine/eval/rego/fuzz_test.go +++ b/internal/engine/eval/rego/fuzz_test.go @@ -20,6 +20,7 @@ func FuzzRegoEval(f *testing.F) { Type: ConstraintsEvaluationType.String(), Def: policy, }, + nil, ) if err != nil { return diff --git a/internal/engine/eval/rego/lib.go b/internal/engine/eval/rego/lib.go index 8eea00ef7d..b3f1472b5e 100644 --- a/internal/engine/eval/rego/lib.go +++ b/internal/engine/eval/rego/lib.go @@ -19,6 +19,7 @@ import ( "github.com/go-git/go-billy/v5" billyutil "github.com/go-git/go-billy/v5/util" + "github.com/open-feature/go-sdk/openfeature" "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/rego" "github.com/open-policy-agent/opa/types" @@ -26,6 +27,7 @@ import ( "github.com/stacklok/frizbee/pkg/utils/config" "gopkg.in/yaml.v3" + "github.com/mindersec/minder/internal/flags" "github.com/mindersec/minder/internal/util" "github.com/mindersec/minder/pkg/engine/v1/interfaces" ) @@ -38,25 +40,36 @@ var MinderRegoLib = []func(res *interfaces.Result) func(*rego.Rego){ FileHTTPType, FileRead, FileWalk, - FileArchive, ListGithubActions, - BaseFileExists, - BaseFileLs, - BaseFileLsGlob, - BaseFileHTTPType, - BaseFileRead, - BaseFileWalk, - BaseListGithubActions, - BaseFileArchive, ParseYaml, JQIsTrue, } -func instantiateRegoLib(res *interfaces.Result) []func(*rego.Rego) { +var MinderRegoLibExperiments = map[flags.Experiment][]func(res *interfaces.Result) func(*rego.Rego){ + flags.TarGzFunctions: {FileArchive, BaseFileArchive}, + flags.GitPRDiffs: { + BaseFileExists, + BaseFileLs, + BaseFileLsGlob, + BaseFileHTTPType, + BaseFileRead, + BaseFileWalk, + BaseListGithubActions, + }, +} + +func instantiateRegoLib(ctx context.Context, featureFlags openfeature.IClient, res *interfaces.Result) []func(*rego.Rego) { var lib []func(*rego.Rego) for _, f := range MinderRegoLib { lib = append(lib, f(res)) } + for flag, funcs := range MinderRegoLibExperiments { + if flags.Bool(ctx, featureFlags, flag) { + for _, f := range funcs { + lib = append(lib, f(res)) + } + } + } return lib } diff --git a/internal/engine/eval/rego/lib_test.go b/internal/engine/eval/rego/lib_test.go index 47fa078ab1..c736fa58d9 100644 --- a/internal/engine/eval/rego/lib_test.go +++ b/internal/engine/eval/rego/lib_test.go @@ -17,6 +17,7 @@ import ( engerrors "github.com/mindersec/minder/internal/engine/errors" "github.com/mindersec/minder/internal/engine/eval/rego" + "github.com/mindersec/minder/internal/flags" minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" "github.com/mindersec/minder/pkg/engine/v1/interfaces" ) @@ -42,6 +43,7 @@ allow { file.exists("foo") }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -62,6 +64,8 @@ func TestFileExistsInBase(t *testing.T) { _, err := fs.Create("foo") require.NoError(t, err, "could not create file") + featureClient := &flags.FakeClient{} + featureClient.Data = map[string]any{"git_pr_diffs": true} e, err := rego.NewRegoEvaluator( &minderv1.RuleType_Definition_Eval_Rego{ Type: rego.DenyByDefaultEvaluationType.String(), @@ -74,6 +78,7 @@ allow { base_file.exists("foo") }`, }, + featureClient, ) require.NoError(t, err, "could not create evaluator") @@ -103,6 +108,7 @@ allow { file.exists("unexistent") }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -140,6 +146,7 @@ allow { contents == "bar" }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -177,6 +184,7 @@ allow { contents == "bar" }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -207,6 +215,7 @@ allow { is_null(files) }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -239,6 +248,7 @@ allow { count(files) == 0 }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -276,6 +286,7 @@ allow { files[0] == "foo/bar" }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -313,6 +324,7 @@ allow { files[0] == "foo/bar" }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -353,6 +365,7 @@ allow { count(files) == 3 }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -396,6 +409,7 @@ allow { count(files) == 3 }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -434,6 +448,7 @@ allow { count(files) == 1 }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -531,6 +546,7 @@ allow { actions == expected_set }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -567,6 +583,7 @@ allow { actions == expected_set }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -602,6 +619,7 @@ allow { count(files) == 1 }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -648,6 +666,7 @@ allow { count(files) == 3 }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -684,6 +703,7 @@ allow { htype == "text/plain; charset=utf-8" }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -721,6 +741,7 @@ allow { htype == "application/octet-stream" }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -771,6 +792,7 @@ allow { count(files) == 7 }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -810,6 +832,8 @@ func TestFileArchive(t *testing.T) { 255, 203, 184, 208, 59, 0, 18, 0, 0, } + featureClient := &flags.FakeClient{} + featureClient.Data = map[string]any{"tar_gz_functions": true} e, err := rego.NewRegoEvaluator( &minderv1.RuleType_Definition_Eval_Rego{ Type: rego.ConstraintsEvaluationType.String(), @@ -824,6 +848,7 @@ violations contains {"msg": sprintf("Expected: %s", [input.profile.expected])} i violations contains {"msg": sprintf("Got : %s", [encoded])} if tarball != expectedTar `, }, + featureClient, ) require.NoError(t, err, "could not create evaluator") @@ -977,7 +1002,8 @@ allow { Type: rego.DenyByDefaultEvaluationType.String(), Def: regoCode, }, - ) + nil, + ) require.NoError(t, err, "could not create evaluator") emptyPol := map[string]any{} @@ -1082,7 +1108,8 @@ allow { Type: rego.DenyByDefaultEvaluationType.String(), Def: regoCode, }, - ) + nil, + ) require.NoError(t, err, "could not create evaluator") err = e.Eval(context.Background(), map[string]any{}, nil, &interfaces.Result{}) diff --git a/internal/engine/eval/rego/rego_test.go b/internal/engine/eval/rego/rego_test.go index cf91c9f140..0cd49f1085 100644 --- a/internal/engine/eval/rego/rego_test.go +++ b/internal/engine/eval/rego/rego_test.go @@ -41,6 +41,7 @@ allow { input.ingested.data == "foo" }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -83,6 +84,7 @@ skip { } `, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -111,6 +113,7 @@ violations[{"msg": msg}] { msg := "data did not contain foo" }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -154,6 +157,7 @@ violations[{"msg": msg}] { } `, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -190,6 +194,7 @@ allow { input.profile.data == input.ingested.data }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -228,6 +233,7 @@ violations[{"msg": msg}] { msg := sprintf("data did not match profile: %s", [input.profile.data]) }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -292,6 +298,7 @@ func TestConstraintsJSONOutput(t *testing.T) { ViolationFormat: &violationFormat, Def: jsonPolicyDef, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -333,6 +340,7 @@ violations[{"msg": msg}] { msg := sprintf("data did not match profile: %s", [input.profile.data]) }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -365,6 +373,7 @@ func TestOutputTypePassedIntoRule(t *testing.T) { Type: rego.ConstraintsEvaluationType.String(), Def: jsonPolicyDef, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -391,14 +400,14 @@ func TestCantCreateEvaluatorWithInvalidConfig(t *testing.T) { t.Run("nil", func(t *testing.T) { t.Parallel() - _, err := rego.NewRegoEvaluator(nil) + _, err := rego.NewRegoEvaluator(nil, nil) require.Error(t, err, "should have failed to create evaluator") }) t.Run("empty", func(t *testing.T) { t.Parallel() - _, err := rego.NewRegoEvaluator(&minderv1.RuleType_Definition_Eval_Rego{}) + _, err := rego.NewRegoEvaluator(&minderv1.RuleType_Definition_Eval_Rego{}, nil) require.Error(t, err, "should have failed to create evaluator") }) @@ -409,7 +418,8 @@ func TestCantCreateEvaluatorWithInvalidConfig(t *testing.T) { &minderv1.RuleType_Definition_Eval_Rego{ Type: "invalid", }, - ) + nil, + ) require.Error(t, err, "should have failed to create evaluator") }) } @@ -427,6 +437,7 @@ package minder violations[{"msg": msg}] {`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -451,6 +462,7 @@ violations[{"msg": msg}] { msg := "data did not contain foo" }`, }, + nil, ) require.NoError(t, err, "could not create evaluator") @@ -490,6 +502,7 @@ allow { minder.datasource.fake.source({"datasourcetest": input.ingested.data}) == "foo" }`, }, + nil, options.WithDataSources(fdsr), ) require.NoError(t, err, "could not create evaluator") diff --git a/internal/engine/executor.go b/internal/engine/executor.go index 7850926f21..b750ede90e 100644 --- a/internal/engine/executor.go +++ b/internal/engine/executor.go @@ -123,6 +123,7 @@ func (e *executor) EvalEntityEvent(ctx context.Context, inf *entities.EntityInfo entityType, inf.ProjectID, provider, + e.featureFlags, ingestCache, dssvc, eoptions.WithFlagsClient(e.featureFlags), diff --git a/internal/engine/rtengine/cache.go b/internal/engine/rtengine/cache.go index cb0fcd5f9e..2cc730adc9 100644 --- a/internal/engine/rtengine/cache.go +++ b/internal/engine/rtengine/cache.go @@ -10,6 +10,7 @@ import ( "fmt" "github.com/google/uuid" + "github.com/open-feature/go-sdk/openfeature" datasourceservice "github.com/mindersec/minder/internal/datasources/service" "github.com/mindersec/minder/internal/db" @@ -29,12 +30,13 @@ type Cache interface { type cacheType = map[uuid.UUID]*rtengine2.RuleTypeEngine type ruleEngineCache struct { - store db.Store - provider provinfv1.Provider - ingestCache ingestcache.Cache - engines cacheType - dssvc datasourceservice.DataSourcesService - opts []eoptions.Option + store db.Store + provider provinfv1.Provider + featureFlags openfeature.IClient + ingestCache ingestcache.Cache + engines cacheType + dssvc datasourceservice.DataSourcesService + opts []eoptions.Option } // NewRuleEngineCache creates the rule engine cache @@ -46,6 +48,7 @@ func NewRuleEngineCache( entityType db.Entities, projectID uuid.UUID, provider provinfv1.Provider, + featureFlags openfeature.IClient, ingestCache ingestcache.Cache, dssvc datasourceservice.DataSourcesService, opts ...eoptions.Option, @@ -71,7 +74,7 @@ func NewRuleEngineCache( engines := make(cacheType, len(ruleTypes)) for _, ruleType := range ruleTypes { ruleEngine, err := cacheRuleEngine( - ctx, &ruleType, provider, ingestCache, engines, dssvc, opts...) + ctx, &ruleType, provider, featureFlags, ingestCache, engines, dssvc, opts...) if err != nil { return nil, err } @@ -79,12 +82,13 @@ func NewRuleEngineCache( } return &ruleEngineCache{ - store: store, - provider: provider, - ingestCache: ingestCache, - engines: engines, - opts: opts, - dssvc: dssvc, + store: store, + provider: provider, + featureFlags: featureFlags, + ingestCache: ingestCache, + engines: engines, + opts: opts, + dssvc: dssvc, }, nil } @@ -112,7 +116,8 @@ func (r *ruleEngineCache) GetRuleEngine(ctx context.Context, ruleTypeID uuid.UUI } // If we find the rule type, insert into the cache and return. - ruleTypeEngine, err := cacheRuleEngine(ctx, &ruleType, r.provider, r.ingestCache, r.engines, r.dssvc, r.opts...) + ruleTypeEngine, err := cacheRuleEngine( + ctx, &ruleType, r.provider, r.featureFlags, r.ingestCache, r.engines, r.dssvc, r.opts...) if err != nil { return nil, fmt.Errorf("error while caching rule type engine: %w", err) } @@ -123,6 +128,7 @@ func cacheRuleEngine( ctx context.Context, ruleType *db.RuleType, provider provinfv1.Provider, + experiments openfeature.IClient, ingestCache ingestcache.Cache, engineCache cacheType, dssvc datasourceservice.DataSourcesService, @@ -150,7 +156,7 @@ func cacheRuleEngine( opts = append(opts, eoptions.WithDataSources(dsreg)) // Create the rule type engine - ruleEngine, err := rtengine2.NewRuleTypeEngine(ctx, pbRuleType, provider, opts...) + ruleEngine, err := rtengine2.NewRuleTypeEngine(ctx, pbRuleType, provider, experiments, opts...) if err != nil { return nil, fmt.Errorf("error creating rule type engine: %w", err) } diff --git a/internal/engine/rtengine/cache_test.go b/internal/engine/rtengine/cache_test.go index a492934415..5f6a348eb5 100644 --- a/internal/engine/rtengine/cache_test.go +++ b/internal/engine/rtengine/cache_test.go @@ -120,7 +120,7 @@ func TestNewRuleTypeEngineCacheConstructor(t *testing.T) { cache, err := NewRuleEngineCache( ctx, store, db.EntitiesRepository, uuid.New(), - testproviders.NewGitProvider(nil), ingestcache.NewNoopCache(), + testproviders.NewGitProvider(nil), nil, ingestcache.NewNoopCache(), dssvc) if scenario.ExpectedError != "" { require.ErrorContains(t, err, scenario.ExpectedError) diff --git a/internal/flags/constants.go b/internal/flags/constants.go index 4325b90ee2..4d909612d0 100644 --- a/internal/flags/constants.go +++ b/internal/flags/constants.go @@ -19,4 +19,8 @@ const ( DataSources Experiment = "data_sources" // PRCommentAlert enables the pull request comment alert engine. PRCommentAlert Experiment = "pr_comment_alert" + // GitPRDiffs enables the git ingester for pull requests. + GitPRDiffs Experiment = "git_pr_diffs" + // Add tar.gz functions to the rego evaluation environment. + TarGzFunctions Experiment = "tar_gz_functions" ) diff --git a/pkg/engine/v1/rtengine/engine.go b/pkg/engine/v1/rtengine/engine.go index b17209d070..cab4aaf953 100644 --- a/pkg/engine/v1/rtengine/engine.go +++ b/pkg/engine/v1/rtengine/engine.go @@ -10,6 +10,7 @@ import ( "runtime/debug" "strings" + "github.com/open-feature/go-sdk/openfeature" "github.com/rs/zerolog" "google.golang.org/protobuf/reflect/protoreflect" @@ -67,6 +68,7 @@ func NewRuleTypeEngine( ctx context.Context, ruletype *minderv1.RuleType, provider provinfv1.Provider, + experiments openfeature.IClient, opts ...eoptions.Option, ) (*RuleTypeEngine, error) { rval, err := profiles.NewRuleValidator(ruletype) @@ -79,7 +81,7 @@ func NewRuleTypeEngine( return nil, fmt.Errorf("cannot create rule data ingest: %w", err) } - evaluator, err := eval.NewRuleEvaluator(ctx, ruletype, provider, opts...) + evaluator, err := eval.NewRuleEvaluator(ctx, ruletype, provider, experiments, opts...) if err != nil { return nil, fmt.Errorf("cannot create rule evaluator: %w", err) } diff --git a/pkg/engine/v1/rtengine/engine_test.go b/pkg/engine/v1/rtengine/engine_test.go index 9aa97169fc..d8389035c8 100644 --- a/pkg/engine/v1/rtengine/engine_test.go +++ b/pkg/engine/v1/rtengine/engine_test.go @@ -92,7 +92,7 @@ allow { } tk := tkv1.NewTestKit(tkv1.WithGitDir(tdir)) - rte, err := NewRuleTypeEngine(ctx, tt.ruleType, tk) + rte, err := NewRuleTypeEngine(ctx, tt.ruleType, tk, nil) require.NoError(t, err, "NewRuleTypeEngine() failed") // Override ingester. This is needed for the test. From 082416cdf2fdcf6bb23eecda69d318fd3d9c3d06 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 16 Dec 2024 12:08:17 -0800 Subject: [PATCH 6/7] Update new tests for changes to Eval method --- cmd/dev/app/rule_type/rttst.go | 1 - internal/engine/eval/rego/lib_test.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/dev/app/rule_type/rttst.go b/cmd/dev/app/rule_type/rttst.go index 9523922edc..7c263bb956 100644 --- a/cmd/dev/app/rule_type/rttst.go +++ b/cmd/dev/app/rule_type/rttst.go @@ -326,7 +326,6 @@ func selectAndEval( } var evalErr error - //var result *interfaces.EvaluationResult if selected { _, evalErr = eng.Eval(ctx, inf.Entity, evalStatus.GetRule().Def, evalStatus.GetRule().Params, evalStatus) } else { diff --git a/internal/engine/eval/rego/lib_test.go b/internal/engine/eval/rego/lib_test.go index 80b85dd84f..716a801614 100644 --- a/internal/engine/eval/rego/lib_test.go +++ b/internal/engine/eval/rego/lib_test.go @@ -85,7 +85,7 @@ allow { emptyPol := map[string]any{} // Matches - err = e.Eval(context.Background(), emptyPol, nil, &interfaces.Result{ + _, err = e.Eval(context.Background(), emptyPol, nil, &interfaces.Result{ BaseFs: fs, }) require.NoError(t, err, "could not evaluate") @@ -857,7 +857,7 @@ violations contains {"msg": sprintf("Got : %s", [encoded])} if tarball != ex "expected": base64.StdEncoding.EncodeToString(expectedTarball), } - err = e.Eval(context.Background(), policy, nil, &interfaces.Result{ + _, err = e.Eval(context.Background(), policy, nil, &interfaces.Result{ Object: nil, Fs: fs, }) From 9c14b194bb886e1ef1890dd3f35a0dfbecd389b2 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 16 Dec 2024 12:19:57 -0800 Subject: [PATCH 7/7] Fix lint /tableflip --- internal/engine/eval/eval.go | 3 ++- internal/engine/eval/rego/lib.go | 2 ++ internal/engine/eval/rego/lib_test.go | 4 ++-- internal/engine/eval/rego/rego_test.go | 2 +- internal/engine/ingester/deps/deps.go | 1 + internal/engine/rtengine/cache.go | 4 ++-- internal/flags/constants.go | 3 ++- 7 files changed, 12 insertions(+), 7 deletions(-) diff --git a/internal/engine/eval/eval.go b/internal/engine/eval/eval.go index 5f81a4ef3e..80aa3643b9 100644 --- a/internal/engine/eval/eval.go +++ b/internal/engine/eval/eval.go @@ -10,6 +10,8 @@ import ( "errors" "fmt" + "github.com/open-feature/go-sdk/openfeature" + "github.com/mindersec/minder/internal/engine/eval/homoglyphs/application" "github.com/mindersec/minder/internal/engine/eval/jq" "github.com/mindersec/minder/internal/engine/eval/rego" @@ -19,7 +21,6 @@ import ( minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" "github.com/mindersec/minder/pkg/engine/v1/interfaces" provinfv1 "github.com/mindersec/minder/pkg/providers/v1" - "github.com/open-feature/go-sdk/openfeature" ) // NewRuleEvaluator creates a new rule data evaluator diff --git a/internal/engine/eval/rego/lib.go b/internal/engine/eval/rego/lib.go index b3f1472b5e..77752e09d4 100644 --- a/internal/engine/eval/rego/lib.go +++ b/internal/engine/eval/rego/lib.go @@ -45,6 +45,8 @@ var MinderRegoLib = []func(res *interfaces.Result) func(*rego.Rego){ JQIsTrue, } +// MinderRegoLibExperiments contains Minder-specific functions which +// should only be exposed when the given experiment is enabled. var MinderRegoLibExperiments = map[flags.Experiment][]func(res *interfaces.Result) func(*rego.Rego){ flags.TarGzFunctions: {FileArchive, BaseFileArchive}, flags.GitPRDiffs: { diff --git a/internal/engine/eval/rego/lib_test.go b/internal/engine/eval/rego/lib_test.go index 716a801614..e30579bc3d 100644 --- a/internal/engine/eval/rego/lib_test.go +++ b/internal/engine/eval/rego/lib_test.go @@ -1003,7 +1003,7 @@ allow { Def: regoCode, }, nil, - ) + ) require.NoError(t, err, "could not create evaluator") emptyPol := map[string]any{} @@ -1109,7 +1109,7 @@ allow { Def: regoCode, }, nil, - ) + ) require.NoError(t, err, "could not create evaluator") _, err = e.Eval(context.Background(), map[string]any{}, nil, &interfaces.Result{}) diff --git a/internal/engine/eval/rego/rego_test.go b/internal/engine/eval/rego/rego_test.go index 808a5b8aa9..2ad41f4588 100644 --- a/internal/engine/eval/rego/rego_test.go +++ b/internal/engine/eval/rego/rego_test.go @@ -419,7 +419,7 @@ func TestCantCreateEvaluatorWithInvalidConfig(t *testing.T) { Type: "invalid", }, nil, - ) + ) require.Error(t, err, "should have failed to create evaluator") }) } diff --git a/internal/engine/ingester/deps/deps.go b/internal/engine/ingester/deps/deps.go index a9398a4e15..50b019f958 100644 --- a/internal/engine/ingester/deps/deps.go +++ b/internal/engine/ingester/deps/deps.go @@ -262,6 +262,7 @@ func (gi *Deps) ingestPullRequest( }, nil } +// TODO: this first part is fairly shared with fetchClone from ../git/git.go. func (gi *Deps) scanFromUrl(ctx context.Context, url string, branch string) (*sbom.NodeList, *plumbing.Reference, error) { // We clone to the memfs go-billy filesystem driver, which doesn't // allow for direct access to the underlying filesystem. This is diff --git a/internal/engine/rtengine/cache.go b/internal/engine/rtengine/cache.go index 2cc730adc9..a03129c1f0 100644 --- a/internal/engine/rtengine/cache.go +++ b/internal/engine/rtengine/cache.go @@ -128,7 +128,7 @@ func cacheRuleEngine( ctx context.Context, ruleType *db.RuleType, provider provinfv1.Provider, - experiments openfeature.IClient, + featureFlags openfeature.IClient, ingestCache ingestcache.Cache, engineCache cacheType, dssvc datasourceservice.DataSourcesService, @@ -156,7 +156,7 @@ func cacheRuleEngine( opts = append(opts, eoptions.WithDataSources(dsreg)) // Create the rule type engine - ruleEngine, err := rtengine2.NewRuleTypeEngine(ctx, pbRuleType, provider, experiments, opts...) + ruleEngine, err := rtengine2.NewRuleTypeEngine(ctx, pbRuleType, provider, featureFlags, opts...) if err != nil { return nil, fmt.Errorf("error creating rule type engine: %w", err) } diff --git a/internal/flags/constants.go b/internal/flags/constants.go index 4d909612d0..0b3be69d78 100644 --- a/internal/flags/constants.go +++ b/internal/flags/constants.go @@ -21,6 +21,7 @@ const ( PRCommentAlert Experiment = "pr_comment_alert" // GitPRDiffs enables the git ingester for pull requests. GitPRDiffs Experiment = "git_pr_diffs" - // Add tar.gz functions to the rego evaluation environment. + // TarGzFunctions enables functions to produce tar.gz data in the rego + // evaluation environment. TarGzFunctions Experiment = "tar_gz_functions" )