WASM "glob" is caching results

[xescugc]

Expected Behavior

To be able to run multiple times the same policy on the JS lib

Actual Behavior

Once it fails, all the rest are false

Steps to Reproduce the Problem

I've copied the example on https://github.com/open-policy-agent/npm-opa-wasm/tree/master/examples/nodejs-app and made my own small example to display it:

https://github.com/xescugc/opa-wasm-error

This was build using master just to be sure as the globs where added on #2682

~ $> opa version
Version: 0.25.0-dev
Build Commit: 
Build Timestamp: 
Build Hostname: 
Go Version: go1.15.4
WebAssembly: available

The steps are the ones on the README but it's basically to run node app.js.

Additional Info

If you need more info just ask 😄

[srenatus]

Thanks for reporting this with a repro example 👍 Haven't yet dug into this, but the behaviour surely isn't correct. I wonder if it's a problem with the npm-opa-sdk or the WASM module. Investigation needed

[srenatus]

diff --git a/internal/wasm/sdk/opa/opa_test.go b/internal/wasm/sdk/opa/opa_test.go
index daf9163c..9199f5cb 100644
--- a/internal/wasm/sdk/opa/opa_test.go
+++ b/internal/wasm/sdk/opa/opa_test.go
@@ -52,6 +52,22 @@ func TestOPA(t *testing.T) {
 			},
 			WantErr: "",
 		},
+		{
+			Description: "Only input changing, glob.match",
+			Policy: `
+			default hello = false
+			hello {
+				x := input.message
+				glob.match("world", [":"], x)
+			}`,
+			Query: "data.p.hello = x",
+			Evals: []Eval{
+				Eval{Input: `{"message": "world"}`, Result: `{{"x": true}}`},
+				Eval{Input: `{"message": "no-world"}`, Result: `{{"x": false}}`},
+				Eval{Input: `{"message": "world"}`, Result: `{{"x": true}}`},
+			},
+			WantErr: "",
+		},
 		{
 			Description: "Only data changing",
 			Policy:      `a = data.q`,
@@ -145,6 +161,9 @@ a = "c" { input > 2 }`,
 	}
 
 	for _, test := range tests {
+		if test.Description != "Only input changing, glob.match" {
+			continue
+		}
 		t.Run(test.Description, func(t *testing.T) {
 			policy := compileRegoToWasm(test.Policy, test.Query)
 			data := []byte(test.Data)

I was able to replicate the problem without node and the npm-opa-wasm package, with the included WASM SDK (Run go test -v -count=1 ./internal/wasm/sdk/opa -run TestOPA)

[srenatus]

Sharing my notes while digging into this here:

• Changing "no-world" to "nope" makes the issue disappear.
• Running with the inputs "world", "no-world", "nope", "world" gives the expected true, false, false, true.
• glob.match is not subject to memoization (the call has more than the two default args, input and data), and
• memoization seems to have nothing to do with it, since disabling it in the WASM compiler still shows this issue

Progress:

• printing the match argument (the third arg to glob.match), and running the "world"/"no-world" sequence, we find that the second time, the match arg ends up being "worldrld"
  • this indicates that we're maybe not writing a string terminating NULL into glob.match's arguments
  • running "world", "no-world", "nope", "world", we don't find "nope" turned into "nopeorld", but running "no-world"/"nopey" turns "nopey" into "nopeyrld"
    • so this has something to do with string length, too.
• in opa_regex_match, we use an opa_cast_string(value) by just using its v char pointer, disregarding its len.
  • the same problem can be triggered by using regex.match("^world$", input.message) with varying inputs as above

Working on the PR now, expect it shortly 😃