search

open-policy-agent / opa

Open Policy Agent (OPA) is an open source, general-purpose policy engine.
https://www.openpolicyagent.org
Apache License 2.0
9.68k stars 1.34k forks source link

panic: Type checking panics in custom built-in functions #5582

Closed wata727 closed 1 year ago

wata727 commented 1 year ago

Short description

I'm building a custom CLI to extend OPA. It offers some custom built-in functions, but I've noticed frequent panics when combining them.

Steps To Reproduce

Panic occurs when you run go run main.go with the code below:

package main

import (
        "context"
        "fmt"

        "github.com/open-policy-agent/opa/ast"
        "github.com/open-policy-agent/opa/rego"
        "github.com/open-policy-agent/opa/types"
)

var rangeTy = types.Named("range", types.NewObject(
        []*types.StaticProperty{
                types.NewStaticProperty("filename", types.S),
                types.NewStaticProperty("start", posTy),
                types.NewStaticProperty("end", posTy),
        },
        nil,
)).Description("range of a source file")

var posTy = types.Named("pos", types.NewObject(
        []*types.StaticProperty{
                types.NewStaticProperty("line", types.N),
                types.NewStaticProperty("column", types.N),
                types.NewStaticProperty("byte", types.N),
        },
        nil,
)).Description("position of a source file")

var issueTy = types.Named("issue", types.NewObject(
        []*types.StaticProperty{
                types.NewStaticProperty("msg", types.S),
                types.NewStaticProperty("range", rangeTy),
        },
        nil,
)).Description("message and source range")

func main() {
        objectFunc := rego.Function2(
                &rego.Function{
                        Name: "custom.object",
                        Decl: types.NewFunction(types.Args(types.S, rangeTy), issueTy),
                },
                func(_ rego.BuiltinContext, msg, rng *ast.Term) (*ast.Term, error) {
                        return ast.ObjectTerm(
                                ast.Item(ast.StringTerm("msg"), msg),
                                ast.Item(ast.StringTerm("range"), rng),
                        ), nil
                },
        )
        rangeFunc := rego.FunctionDyn(
                &rego.Function{
                        Name: "custom.range",
                        Decl: types.NewFunction(types.Args(), rangeTy),
                },
                func(_ rego.BuiltinContext, _ []*ast.Term) (*ast.Term, error) {
                        rng := map[string]any{
                                "filename": "example.txt",
                                "start":    map[string]int{"line": 1, "end": 1, "byte": 1},
                                "end":      map[string]int{"line": 1, "end": 1, "byte": 1},
                        }
                        v, err := ast.InterfaceToValue(rng)
                        if err != nil {
                                return nil, err
                        }
                        return ast.NewTerm(v), nil
                },
        )

        r := rego.New(
                rego.Query(`ret := custom.object("foo", custom.range())`),
                objectFunc,
                rangeFunc,
        )

        rs, err := r.Eval(context.Background())
        if err != nil {
                panic(err.Error())
        }

        fmt.Printf("%#v", rs)
}
$ go run main.go
panic: unreachable

goroutine 1 [running]:
github.com/open-policy-agent/opa/ast.unifies({0xbcbd90, 0xc0001567e0?}, {0xbcbd90?, 0xc0001567e0?})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/check.go:828 +0x46c
github.com/open-policy-agent/opa/ast.unifiesObjectsStatic(0xc000170320, 0xc00018c090?)
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/check.go:882 +0x1d3
github.com/open-policy-agent/opa/ast.unifiesObjects(0xc000170320, 0xc000170320)
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/check.go:869 +0x27
github.com/open-policy-agent/opa/ast.unifies({0xbcbdc0, 0xc000170320?}, {0xbcbdc0?, 0xc000170320?})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/check.go:807 +0x390
github.com/open-policy-agent/opa/ast.unify1(0xc000187668, 0xc000187368, {0xbcbdc0?, 0xc000170320?}, 0x0)
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/check.go:529 +0x628
github.com/open-policy-agent/opa/ast.(*typeChecker).checkExprBuiltin(0xa1ecc0?, 0xc000189f50?, 0xc0001783c0)
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/check.go:337 +0xb05
github.com/open-policy-agent/opa/ast.(*typeChecker).checkExpr(0xc00007c900, 0xaa8740?, 0xc0001783c0)
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/check.go:286 +0x105
github.com/open-policy-agent/opa/ast.(*typeChecker).CheckBody.func1(0xb?)
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/check.go:119 +0x334
github.com/open-policy-agent/opa/ast.WalkExprs.func1({0xaa8740?, 0xc0001783c0?})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/visit.go:217 +0x36
github.com/open-policy-agent/opa/ast.(*GenericVisitor).Walk(0xc000014480, {0xaa8740?, 0xc0001783c0?})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/visit.go:282 +0x5f
github.com/open-policy-agent/opa/ast.(*GenericVisitor).Walk(0xc000014480, {0xa920e0?, 0xc000187680?})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/visit.go:323 +0xedb
github.com/open-policy-agent/opa/ast.WalkExprs({0xa920e0, 0xc000187680}, 0xc0001851e0)
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/visit.go:221 +0x9a
github.com/open-policy-agent/opa/ast.(*typeChecker).CheckBody(0xc00007c900, 0x2?, {0xc000187638, 0x3, 0x3})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/check.go:102 +0x107
github.com/open-policy-agent/opa/ast.(*queryCompiler).checkTypes(0xc0001782c0, 0xc000184ea0?, {0xc000187638, 0x3, 0x3})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/compile.go:2667 +0x1f6
github.com/open-policy-agent/opa/ast.(*queryCompiler).runStage(0xa1e7e0?, {0xacd7a0?, 0xac33b5?}, 0x13?, {0xc000187638?, 0x28?, 0x7fa15d82e108?}, 0x30?)
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/compile.go:2482 +0x173
github.com/open-policy-agent/opa/ast.(*queryCompiler).Compile(0xc0001782c0, {0xc0000140e0, 0x1, 0xac4f24?})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/ast/compile.go:2526 +0x6bb
github.com/open-policy-agent/opa/rego.(*Rego).compileQuery(0xc000166600, {0xc0000140e0, 0x1, 0x1}, {0x0, 0x0, 0x0}, {0x0?, 0xc0000140e0?}, {0xc00012f5a0, ...})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/rego/rego.go:1948 +0x251
github.com/open-policy-agent/opa/rego.(*Rego).compileAndCacheQuery(0xc000166600, 0xc000170440?, {0xc0000140e0, 0x1, 0x1}, {0x0, 0x0, 0x0}, {0xbceb20, 0xc000170440}, ...)
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/rego/rego.go:1891 +0x265
github.com/open-policy-agent/opa/rego.(*Rego).prepare(0xc000166600, {0xbcc6c0, 0xc00002a0c8}, 0x0?, {0xc00012f5a0, 0x1, 0x1})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/rego/rego.go:1683 +0x47a
github.com/open-policy-agent/opa/rego.(*Rego).PrepareForEval(0xc000166600, {0xbcc6c0, 0xc00002a0c8}, {0x0, 0x0, 0x78a145?})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/rego/rego.go:1529 +0x538
github.com/open-policy-agent/opa/rego.(*Rego).Eval(0xc000166600, {0xbcc6c0, 0xc00002a0c8})
        /go/pkg/mod/github.com/open-policy-agent/opa@v0.48.0/rego/rego.go:1203 +0xb0
main.main()
        /workspaces/tflint-ruleset-opa/work/main.go:76 +0x298
exit status 2

Expected behavior

It doesn't panic and returns successfully.

Additional context

This error seems to be caused by a condition that is not supposed to be reached: https://github.com/open-policy-agent/opa/blob/v0.48.0/ast/check.go#L828

Unfortunately, I didn't know how to fix it...

Environments:

srenatus commented 1 year ago

I think the problem is this: nesting named types is possible: it's allowed by the type signatures and the compiler will let you do it -- but it's not supported: the type checker doesn't know what to do with it.

For example, nesting happens here:


var rangeTy = types.Named("range", types.NewObject(
        []*types.StaticProperty{
                types.NewStaticProperty("filename", types.S),
                types.NewStaticProperty("start", posTy), #  <<<<<< nested named type
                types.NewStaticProperty("end", posTy),
        },
        nil,
)).Description("range of a source file")

var posTy = types.Named("pos", types.NewObject(
        []*types.StaticProperty{
                types.NewStaticProperty("line", types.N),
                types.NewStaticProperty("column", types.N),
                types.NewStaticProperty("byte", types.N),
        },
        nil,
)).Description("position of a source file")

Named types, as used for generating the builtin function documentation, can only appear on the top level. So, the first argument to a function, or some other argument; and its return value can be named. A property of an object type cannot.

wata727 commented 1 year ago

Thank you for your reply. Fair enough. In that case, it would be nice if types.Named() had documentation for that.