Skip to content

Commit

Permalink
ast+cmd: Allowing bundle to contain calls to unknown functions when i…
Browse files Browse the repository at this point in the history
…nspected

Fixes: open-policy-agent#6457
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
  • Loading branch information
johanfylling authored and ashutosh-narkar committed Dec 11, 2023
1 parent 30fa90e commit 1e4d3e3
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 10 deletions.
25 changes: 17 additions & 8 deletions ast/check.go
Expand Up @@ -24,14 +24,15 @@ type exprChecker func(*TypeEnv, *Expr) *Error
// accumulated on the typeChecker so that a single run can report multiple
// issues.
type typeChecker struct {
builtins map[string]*Builtin
required *Capabilities
errs Errors
exprCheckers map[string]exprChecker
varRewriter varRewriter
ss *SchemaSet
allowNet []string
input types.Type
builtins map[string]*Builtin
required *Capabilities
errs Errors
exprCheckers map[string]exprChecker
varRewriter varRewriter
ss *SchemaSet
allowNet []string
input types.Type
allowUndefinedFuncs bool
}

// newTypeChecker returns a new typeChecker object that has no errors.
Expand Down Expand Up @@ -92,6 +93,11 @@ func (tc *typeChecker) WithInputType(tpe types.Type) *typeChecker {
return tc
}

func (tc *typeChecker) WithAllowUndefinedFunctionCalls(allow bool) *typeChecker {
tc.allowUndefinedFuncs = allow
return tc
}

// Env returns a type environment for the specified built-ins with any other
// global types configured on the checker. In practice, this is the default
// environment that other statements will be checked against.
Expand Down Expand Up @@ -347,6 +353,9 @@ func (tc *typeChecker) checkExprBuiltin(env *TypeEnv, expr *Expr) *Error {
tpe := env.Get(name)

if tpe == nil {
if tc.allowUndefinedFuncs {
return nil
}
return NewError(TypeErr, expr.Location, "undefined function %v", name)
}

Expand Down
14 changes: 13 additions & 1 deletion ast/compile.go
Expand Up @@ -146,6 +146,7 @@ type Compiler struct {
keepModules bool // whether to keep the unprocessed, parse modules (below)
parsedModules map[string]*Module // parsed, but otherwise unprocessed modules, kept track of when keepModules is true
useTypeCheckAnnotations bool // whether to provide annotated information (schemas) to the type checker
allowUndefinedFuncCalls bool // don't error on calls to unknown functions.
evalMode CompilerEvalMode
}

Expand Down Expand Up @@ -457,6 +458,11 @@ func (c *Compiler) WithUseTypeCheckAnnotations(enabled bool) *Compiler {
return c
}

func (c *Compiler) WithAllowUndefinedFunctionCalls(allow bool) *Compiler {
c.allowUndefinedFuncCalls = allow
return c
}

// WithEvalMode allows setting the CompilerEvalMode of the compiler
func (c *Compiler) WithEvalMode(e CompilerEvalMode) *Compiler {
c.evalMode = e
Expand Down Expand Up @@ -1513,7 +1519,8 @@ func (c *Compiler) checkTypes() {
WithInputType(c.inputType).
WithBuiltins(c.builtins).
WithRequiredCapabilities(c.Required).
WithVarRewriter(rewriteVarsInRef(c.RewrittenVars))
WithVarRewriter(rewriteVarsInRef(c.RewrittenVars)).
WithAllowUndefinedFunctionCalls(c.allowUndefinedFuncCalls)
var as *AnnotationSet
if c.useTypeCheckAnnotations {
as = c.annotationSet
Expand Down Expand Up @@ -1577,6 +1584,11 @@ func (c *Compiler) compile() {
continue // skip these stages
}
}

if c.allowUndefinedFuncCalls && s.name == "CheckUndefinedFuncs" {
continue
}

c.runStage(s.metricName, s.f)
if c.Failed() {
return
Expand Down
82 changes: 82 additions & 0 deletions cmd/inspect_test.go
Expand Up @@ -565,3 +565,85 @@ Custom:

})
}

func TestCallToUnknownBuiltInFunction(t *testing.T) {
files := [][2]string{
{"/policy.rego", `package test
p {
foo.bar(42)
contains("foo", "o")
}
`},
}

buf := archive.MustWriteTarGz(files)

test.WithTempFS(nil, func(rootDir string) {
bundleFile := filepath.Join(rootDir, "bundle.tar.gz")

bf, err := os.Create(bundleFile)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

_, err = bf.Write(buf.Bytes())
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

var out bytes.Buffer
params := newInspectCommandParams()
err = params.outputFormat.Set(evalJSONOutput)
if err != nil {
t.Fatalf("Unexpected error: %s", err)
}

err = doInspect(params, bundleFile, &out)
if err != nil {
t.Fatalf("Unexpected error %v", err)
}

bs := out.Bytes()
output := strings.TrimSpace(string(bs))
// Note: unknown foo.bar() built-in doesn't appear in the output, but also didn't cause an error.
expected := strings.TrimSpace(`{
"manifest": {
"revision": "",
"roots": [
""
]
},
"signatures_config": {},
"namespaces": {
"data.test": [
"/policy.rego"
]
},
"capabilities": {
"builtins": [
{
"name": "contains",
"decl": {
"args": [
{
"type": "string"
},
{
"type": "string"
}
],
"result": {
"type": "boolean"
},
"type": "function"
}
}
]
}
}`)

if output != expected {
t.Fatalf("Unexpected output. Expected:\n\n%s\n\nGot:\n\n%s", expected, output)
}
})
}
3 changes: 2 additions & 1 deletion internal/bundle/inspect/inspect.go
Expand Up @@ -109,7 +109,8 @@ func File(path string, includeAnnotations bool) (*Info, error) {
moduleMap[f.URL] = f.Parsed
}

c := ast.NewCompiler()
c := ast.NewCompiler().
WithAllowUndefinedFunctionCalls(true)
c.Compile(moduleMap)
if c.Failed() {
return bi, c.Errors
Expand Down

0 comments on commit 1e4d3e3

Please sign in to comment.