fix: validate that a flag key is valid UTF-8 & implemented fuzzing tests

Signed-off-by: Skye Gill <[email protected]>
open-feature · Jan 26, 2023 · f9cb653 · f9cb653
1 parent 1b9fd94
commit f9cb653
Show file tree

Hide file tree

Showing 3 changed files with 144 additions and 8 deletions.
diff --git a/README.md b/README.md
@@ -105,7 +105,7 @@ Run unit tests with `make test`.
 
 #### Integration tests
 
-The continuous integration runs a set of [gherkin integration tests](https://github.com/open-feature/test-harness/blob/main/features) using [`flagd`](https://github.com/open-feature/flagd).
+The continuous integration runs a set of [gherkin integration tests](https://github.com/open-feature/test-harness/blob/main/features) using the [flagd provider](https://github.com/open-feature/go-sdk-contrib/tree/main/providers/flagd) and [flagd](https://github.com/open-feature/flagd).
 If you'd like to run them locally, first pull the `test-harness` git submodule
 ```
 git submodule update --init --recursive
@@ -119,6 +119,20 @@ docker run -p 8013:8013 -v $PWD/test-harness/testing-flags.json:/testing-flags.j
 make integration-test
 ```
 
+#### Fuzzing
+
+[Go supports fuzzing natively as of 1.18](https://go.dev/security/fuzz/).
+The fuzzing suite is implemented as an integration of `go-sdk` with the [flagd provider](https://github.com/open-feature/go-sdk-contrib/tree/main/providers/flagd) and [flagd](https://github.com/open-feature/flagd).
+The fuzzing tests are found in [./integration/evaluation_fuzz_test.go](./integration/evaluation_fuzz_test.go), they are dependent on the flagd testbed running, you can start it with
+```
+docker run -p 8013:8013 ghcr.io/open-feature/flagd-testbed:latest
+```
+then, to execute a fuzzing test, run the following
+```
+go test -fuzz=FuzzBooleanEvaluation ./integration/evaluation_fuzz_test.go
+```
+substituting the name of the fuzz as appropriate.
+
 ### Releases
 
 This repo uses Release Please to release packages. Release Please sets up a running PR that tracks all changes for the library components, and maintains the versions according to conventional commits, generated when PRs are merged. When Release Please's running PR is merged, any changed artifacts are published.

diff --git a/integration/evaluation_fuzz_test.go b/integration/evaluation_fuzz_test.go
@@ -0,0 +1,116 @@
+package integration_test
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"time"
+
+	flagd "github.com/open-feature/go-sdk-contrib/providers/flagd/pkg"
+	"github.com/open-feature/go-sdk/pkg/openfeature"
+)
+
+func setupFuzzClient(f *testing.F) *openfeature.Client {
+	f.Helper()
+
+	provider := flagd.NewProvider(flagd.WithPort(8013), flagd.WithoutCache())
+	openfeature.SetProvider(provider)
+
+	select {
+	case <-provider.IsReady():
+	case <-time.After(500 * time.Millisecond):
+		f.Fatal("provider not ready after 500 milliseconds")
+	}
+
+	return openfeature.NewClient("fuzzing")
+}
+
+func FuzzBooleanEvaluation(f *testing.F) {
+	client := setupFuzzClient(f)
+
+	f.Add("foo", false)
+	f.Fuzz(func(t *testing.T, flagKey string, defaultValue bool) {
+		res, err := client.BooleanValueDetails(context.Background(), flagKey, defaultValue, openfeature.EvaluationContext{})
+		if err != nil {
+			if res.ErrorCode == openfeature.FlagNotFoundCode {
+				return
+			}
+			if strings.Contains(err.Error(), string(openfeature.ParseErrorCode)) {
+				return
+			}
+			t.Error(err)
+		}
+	})
+}
+
+func FuzzStringEvaluation(f *testing.F) {
+	client := setupFuzzClient(f)
+
+	f.Add("foo", "bar")
+	f.Fuzz(func(t *testing.T, flagKey string, defaultValue string) {
+		res, err := client.StringValueDetails(context.Background(), flagKey, defaultValue, openfeature.EvaluationContext{})
+		if err != nil {
+			if res.ErrorCode == openfeature.FlagNotFoundCode {
+				return
+			}
+			if strings.Contains(err.Error(), string(openfeature.ParseErrorCode)) {
+				return
+			}
+			t.Error(err)
+		}
+	})
+}
+
+func FuzzIntEvaluation(f *testing.F) {
+	client := setupFuzzClient(f)
+
+	f.Add("foo", int64(1))
+	f.Fuzz(func(t *testing.T, flagKey string, defaultValue int64) {
+		res, err := client.IntValueDetails(context.Background(), flagKey, defaultValue, openfeature.EvaluationContext{})
+		if err != nil {
+			if res.ErrorCode == openfeature.FlagNotFoundCode {
+				return
+			}
+			if strings.Contains(err.Error(), string(openfeature.ParseErrorCode)) {
+				return
+			}
+			t.Error(err)
+		}
+	})
+}
+
+func FuzzFloatEvaluation(f *testing.F) {
+	client := setupFuzzClient(f)
+
+	f.Add("foo", float64(1))
+	f.Fuzz(func(t *testing.T, flagKey string, defaultValue float64) {
+		res, err := client.FloatValueDetails(context.Background(), flagKey, defaultValue, openfeature.EvaluationContext{})
+		if err != nil {
+			if res.ErrorCode == openfeature.FlagNotFoundCode {
+				return
+			}
+			if strings.Contains(err.Error(), string(openfeature.ParseErrorCode)) {
+				return
+			}
+			t.Error(err)
+		}
+	})
+}
+
+func FuzzObjectEvaluation(f *testing.F) {
+	client := setupFuzzClient(f)
+
+	f.Add("foo", "{}")
+	f.Fuzz(func(t *testing.T, flagKey string, defaultValue string) { // interface{} is not supported, using a string
+		res, err := client.ObjectValueDetails(context.Background(), flagKey, defaultValue, openfeature.EvaluationContext{})
+		if err != nil {
+			if res.ErrorCode == openfeature.FlagNotFoundCode {
+				return
+			}
+			if strings.Contains(err.Error(), string(openfeature.ParseErrorCode)) {
+				return
+			}
+			t.Error(err)
+		}
+	})
+}
diff --git a/pkg/openfeature/client.go b/pkg/openfeature/client.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"sync"
+	"unicode/utf8"
 
 	"github.com/go-logr/logr"
 )
@@ -563,6 +564,18 @@ func (c *Client) evaluate(
 		"evaluationContext", evalCtx, "evaluationOptions", options,
 	)
 
+	evalDetails := InterfaceEvaluationDetails{
+		Value: defaultValue,
+		EvaluationDetails: EvaluationDetails{
+			FlagKey:  flag,
+			FlagType: flagType,
+		},
+	}
+
+	if !utf8.Valid([]byte(flag)) {
+		return evalDetails, NewParseErrorResolutionError("flag key is not a UTF-8 encoded string")
+	}
+
 	// ensure that the same provider & hooks are used across this transaction to avoid unexpected behaviour
 	api.RLock()
 	provider := api.prvder
@@ -583,13 +596,6 @@ func (c *Client) evaluate(
 		providerMetadata:  provider.Metadata(),
 		evaluationContext: evalCtx,
 	}
-	evalDetails := InterfaceEvaluationDetails{
-		Value: defaultValue,
-		EvaluationDetails: EvaluationDetails{
-			FlagKey:  flag,
-			FlagType: flagType,
-		},
-	}
 
 	defer func() {
 		c.finallyHooks(hookCtx, providerInvocationClientApiHooks, options)