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.

Sign up for GitHub

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

Jump to bottom

panic on opa fmt when calling another rule #2430

Closed
kevinsimons-wf opened this issue May 22, 2020 · 1 comment
Closed

panic on opa fmt when calling another rule #2430

kevinsimons-wf opened this issue May 22, 2020 · 1 comment
Assignees
@patrick-east
Labels
bug

Comments

@kevinsimons-wf
Copy link

kevinsimons-wf commented May 22, 2020 •
edited
Loading

Expected Behavior

Using opa fmt on the following policy

package example

operations = {"CREATE"}

violation[{"msg": msg}] {
	operations[input.review.operation]
	general_violation[{"msg": msg}]
}

violation[{"msg": msg}] {
	# On audits the operation is blank
	not input.review.operation
	general_violation[{"msg": msg}]
}

general_violation[{"msg": msg}] {
	not input.review.object.metadata.annotations["management"]
	msg := "This operation has been disabled for non management"
}

Actual Behavior

A panic is hit

$ opa fmt ./example.rego
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x1525c51]

goroutine 1 [running]:
github.com/open-policy-agent/opa/format.closingLoc(0xc07d7b0000, 0x0, 0x1)
        /go/src/github.com/open-policy-agent/opa/format/format.go:946 +0x41
github.com/open-policy-agent/opa/format.(*writer).writeObject(0xc000086780, 0x19d15c0, 0xc000165650, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        /go/src/github.com/open-policy-agent/opa/format/format.go:615 +0x154
github.com/open-policy-agent/opa/format.(*writer).writeTermParens(0xc000086780, 0x0, 0xc0000c5278, 0x0, 0x0, 0x0, 0x18a1d84, 0x1, 0x1)
        /go/src/github.com/open-policy-agent/opa/format/format.go:509 +0x85f
github.com/open-policy-agent/opa/format.(*writer).writeTerm(0xc000086780, 0xc0000c5278, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        /go/src/github.com/open-policy-agent/opa/format/format.go:496 +0x58
github.com/open-policy-agent/opa/format.(*writer).writeRef(0xc000086780, 0xc0001608e0, 0x2, 0x2)
        /go/src/github.com/open-policy-agent/opa/format/format.go:554 +0x1d6
github.com/open-policy-agent/opa/format.(*writer).writeTermParens(0xc000086780, 0x0, 0xc000085480, 0xc000162220, 0x1, 0x1, 0x0, 0x0, 0x0)
        /go/src/github.com/open-policy-agent/opa/format/format.go:507 +0x88b
github.com/open-policy-agent/opa/format.(*writer).writeTerm(0xc000086780, 0xc000085480, 0xc000162218, 0x1, 0x1, 0xc000162218, 0x1, 0x1)
        /go/src/github.com/open-policy-agent/opa/format/format.go:496 +0x58
github.com/open-policy-agent/opa/format.(*writer).writeExpr(0xc000086780, 0xc00016c280, 0xc000162218, 0x1, 0x1, 0x0, 0x0, 0x0)
        /go/src/github.com/open-policy-agent/opa/format/format.go:396 +0x33b
github.com/open-policy-agent/opa/format.(*writer).writeBody(0xc000086780, 0xc0001608f0, 0x2, 0x2, 0xc000162210, 0x1, 0x1, 0x1, 0x1, 0x0)
        /go/src/github.com/open-policy-agent/opa/format/format.go:374 +0xcb
github.com/open-policy-agent/opa/format.(*writer).writeRule(0xc000086780, 0xc000088c00, 0x0, 0xc0001621f8, 0x1, 0x1, 0x1, 0x1, 0x1)
        /go/src/github.com/open-policy-agent/opa/format/format.go:242 +0x305
github.com/open-policy-agent/opa/format.(*writer).writeRules(0xc000086780, 0xc000085bc0, 0x4, 0x4, 0xc0001621d8, 0x1, 0x1, 0x0, 0x0, 0x1)
        /go/src/github.com/open-policy-agent/opa/format/format.go:206 +0xb3
github.com/open-policy-agent/opa/format.(*writer).writeModule(0xc000086780, 0xc00016c4b0)
        /go/src/github.com/open-policy-agent/opa/format/format.go:173 +0x418
github.com/open-policy-agent/opa/format.Ast(0x1803ce0, 0xc00016c4b0, 0xc0001bc000, 0x1a8, 0xc00016c4b0, 0x0, 0x0)
        /go/src/github.com/open-policy-agent/opa/format/format.go:84 +0x439
github.com/open-policy-agent/opa/format.Source(0x7ffeefbfe7db, 0x30, 0xc0001ba000, 0x1a8, 0x3a8, 0x0, 0x0, 0x0, 0x0, 0x1)
        /go/src/github.com/open-policy-agent/opa/format/format.go:26 +0xc3
github.com/open-policy-agent/opa/cmd.formatFile(0x1f17f94, 0x19b3ba0, 0xc0000aa008, 0x7ffeefbfe7db, 0x30, 0x19c9cc0, 0xc00015b380, 0x0, 0x0, 0x0, ...)
        /go/src/github.com/open-policy-agent/opa/cmd/fmt.go:108 +0x21b
github.com/open-policy-agent/opa/cmd.opaFmt.func1(0x7ffeefbfe7db, 0x30, 0x19c9cc0, 0xc00015b380, 0x0, 0x0, 0xc, 0xc0000c5c00)
        /go/src/github.com/open-policy-agent/opa/cmd/fmt.go:73 +0x8e
path/filepath.walk(0x7ffeefbfe7db, 0x30, 0x19c9cc0, 0xc00015b380, 0x18d95d8, 0x0, 0x119a0f4)
        /usr/local/go/src/path/filepath/path.go:358 +0x425
path/filepath.Walk(0x7ffeefbfe7db, 0x30, 0x18d95d8, 0x30, 0x0)
        /usr/local/go/src/path/filepath/path.go:404 +0xff
github.com/open-policy-agent/opa/cmd.opaFmt(0xc000084ee0, 0x1, 0x2, 0x0)
        /go/src/github.com/open-policy-agent/opa/cmd/fmt.go:72 +0xa0
github.com/open-policy-agent/opa/cmd.glob..func3(0x1ee7600, 0xc000084ee0, 0x1, 0x2)
        /go/src/github.com/open-policy-agent/opa/cmd/fmt.go:50 +0x3f
github.com/spf13/cobra.(*Command).execute(0x1ee7600, 0xc000084ec0, 0x2, 0x2, 0x1ee7600, 0xc000084ec0)
        /go/src/github.com/open-policy-agent/opa/vendor/github.com/spf13/cobra/command.go:766 +0x2aa
github.com/spf13/cobra.(*Command).ExecuteC(0x1ee73a0, 0xc000056750, 0xc0000c5f50, 0x100545f)
        /go/src/github.com/open-policy-agent/opa/vendor/github.com/spf13/cobra/command.go:852 +0x2ea
github.com/spf13/cobra.(*Command).Execute(...)
        /go/src/github.com/open-policy-agent/opa/vendor/github.com/spf13/cobra/command.go:800
main.main()
        /go/src/github.com/open-policy-agent/opa/main.go:15 +0x31

Steps to Reproduce the Problem

$ opa version
Version: 0.20.3
Build Commit: f19a044b
Build Timestamp: 2020-05-22T02:06:24Z
Build Hostname: fe931b74376a

Additional Info

The following policy does work and I believe would be functionally the same

package example

operations = {"CREATE"}

violation[{"msg": msg}] {
	operations[input.review.operation]
	obj := {"msg": msg}
	general_violation[obj]
}

violation[{"msg": msg}] {
	# On audits the operation is blank
	not input.review.operation
	obj := {"msg": msg}
	general_violation[obj]
}

general_violation[{"msg": msg}] {
	not input.review.object.metadata.annotations["management"]
	msg := "This operation has been disabled for non management"
}

I am also fairly new to rego, so if I am making some large mistake by calling the general_violation like this, please let me know.
@patrick-east patrick-east self-assigned this May 22, 2020
@patrick-east
Copy link
Contributor

Thanks for filing the issue! There is a PR that fixes this over at #2429 , I've just linked the commit to this issue to it (there wasn't an issue earlier)

@patrick-east patrick-east mentioned this issue May 22, 2020
Merged
patrick-east added a commit to patrick-east/opa that referenced this issue May 22, 2020
@patrick-east
format: Refactor wildcard names to rewrite early
d4a4e23 
Instead of playing games with trying to format them nicely, we instead
can just rewrite them right at the start to no longer be wildcards if
they show up >1 time in the policy.

This fixes a panic being caused if a ref had an array path segment, by
making a new term we were passing along a nil Location. While we could
correct this by just passing the original term from the ref we can
avoid all this special handling altogether by rewriting the wildcards
earlier.

Fixes: open-policy-agent#2430
Signed-off-by: Patrick East <east.patrick@gmail.com>
patrick-east added a commit that referenced this issue May 22, 2020
@patrick-east
format: Refactor wildcard names to rewrite early
68e57b1 
Instead of playing games with trying to format them nicely, we instead
can just rewrite them right at the start to no longer be wildcards if
they show up >1 time in the policy.

This fixes a panic being caused if a ref had an array path segment, by
making a new term we were passing along a nil Location. While we could
correct this by just passing the original term from the ref we can
avoid all this special handling altogether by rewriting the wildcards
earlier.

Fixes: #2430
Signed-off-by: Patrick East <east.patrick@gmail.com>
patrick-east added a commit to patrick-east/opa that referenced this issue May 22, 2020
@patrick-east
format: Refactor wildcard names to rewrite early
7fa4656 
Instead of playing games with trying to format them nicely, we instead
can just rewrite them right at the start to no longer be wildcards if
they show up >1 time in the policy.

This fixes a panic being caused if a ref had an array path segment, by
making a new term we were passing along a nil Location. While we could
correct this by just passing the original term from the ref we can
avoid all this special handling altogether by rewriting the wildcards
earlier.

Fixes: open-policy-agent#2430
Signed-off-by: Patrick East <east.patrick@gmail.com>
(cherry picked from commit 68e57b1)
patrick-east added a commit that referenced this issue May 22, 2020
@patrick-east
format: Refactor wildcard names to rewrite early
b949d89 
Instead of playing games with trying to format them nicely, we instead
can just rewrite them right at the start to no longer be wildcards if
they show up >1 time in the policy.

This fixes a panic being caused if a ref had an array path segment, by
making a new term we were passing along a nil Location. While we could
correct this by just passing the original term from the ref we can
avoid all this special handling altogether by rewriting the wildcards
earlier.

Fixes: #2430
Signed-off-by: Patrick East <east.patrick@gmail.com>
(cherry picked from commit 68e57b1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@patrick-east patrick-east

Labels
bug
Projects
Open Policy Agent
Archived in project
Milestone
No milestone
Development

No branches or pull requests
2 participants
@patrick-east @kevinsimons-wf