From 27fb70cf06bff84250e1a71137ad89edc9be419e Mon Sep 17 00:00:00 2001 From: "R.I.Pienaar" Date: Tue, 12 Sep 2023 19:42:37 +0200 Subject: [PATCH] (#151) Support experimental flag and argument validation Signed-off-by: R.I.Pienaar --- builder/generic.go | 39 ++++++--- docs/content/experiments/_index.md | 35 +++++++- example/example_test.go | 9 ++ example/sample-app.yaml | 1 + go.mod | 5 +- go.sum | 10 ++- validator/validator.go | 132 +++++++++++++++++++++++++++++ validator/validator_test.go | 79 +++++++++++++++++ 8 files changed, 289 insertions(+), 21 deletions(-) create mode 100644 validator/validator.go create mode 100644 validator/validator_test.go diff --git a/builder/generic.go b/builder/generic.go index ab257b3..becdea1 100644 --- a/builder/generic.go +++ b/builder/generic.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/AlecAivazis/survey/v2" + "github.com/choria-io/appbuilder/validator" "github.com/choria-io/fisk" ) @@ -75,24 +76,26 @@ func (c *GenericCommand) Validate(logger Logger) error { // GenericArgument is a standard command line argument type GenericArgument struct { - Name string `json:"name"` - Description string `json:"description"` - Required bool `json:"required"` - Enum []string `json:"enum"` - Default string `json:"default"` + Name string `json:"name"` + Description string `json:"description"` + Required bool `json:"required"` + Enum []string `json:"enum"` + Default string `json:"default"` + ValidationExpression string `json:"validate"` } // GenericFlag is a standard command line flag type GenericFlag struct { - Name string `json:"name"` - Description string `json:"description"` - Required bool `json:"required"` - PlaceHolder string `json:"placeholder"` - Enum []string `json:"enum"` - Default any `json:"default"` - Bool bool `json:"bool"` - EnvVar string `json:"env"` - Short string `json:"short"` + Name string `json:"name"` + Description string `json:"description"` + Required bool `json:"required"` + PlaceHolder string `json:"placeholder"` + Enum []string `json:"enum"` + Default any `json:"default"` + Bool bool `json:"bool"` + EnvVar string `json:"env"` + Short string `json:"short"` + ValidationExpression string `json:"validate"` } // CreateGenericCommand can be used to add all the typical flags and arguments etc if your command is based on GenericCommand. Values set in flags and arguments @@ -128,6 +131,10 @@ func CreateGenericCommand(app KingpinCommand, sc *GenericCommand, arguments map[ arg.Default(a.Default) } + if a.ValidationExpression != "" { + arg.Validator(validator.FiskValidator(a.ValidationExpression)) + } + switch { case len(a.Enum) > 0: arguments[a.Name] = arg.Enum(a.Enum...) @@ -160,6 +167,10 @@ func CreateGenericCommand(app KingpinCommand, sc *GenericCommand, arguments map[ flag.Short([]rune(f.Short)[0]) } + if f.ValidationExpression != "" { + flag.Validator(validator.FiskValidator(f.ValidationExpression)) + } + switch { case len(f.Enum) > 0: flags[f.Name] = flag.Enum(f.Enum...) diff --git a/docs/content/experiments/_index.md b/docs/content/experiments/_index.md index c01f7cf..344af35 100644 --- a/docs/content/experiments/_index.md +++ b/docs/content/experiments/_index.md @@ -72,6 +72,39 @@ An example can be found in the source repository for this project. Configuration is looked for in the local directory in the `.abtenv` file. At present this is not searched for in parent directories. +## Argument and Flag Validations + +One might need to ensure that the input provided by a user passes some validation, for example when passing commands +to shell scripts one has to be careful about [Shell Injection](https://en.wikipedia.org/wiki/Code_injection#Shell_injection). + +We support custom validators on Arguments and Flags using the [Expr Language](https://expr.medv.io/docs/Language-Definition) + +{{% notice secondary "Version Hint" code-branch %}} +This is available since version `0.8.0`. +{{% /notice %}} + +Based on the Getting Started example that calls `cowsay` we might wish to limit the length of the message to what +would work well with `cowsay` and also ensure there is no shell escaping happening. + +```yaml +arguments: + - name: message + description: The message to display + required: true + validate: len(value) < 20 && is_shellsafe(value) +``` +We support the standard `expr` language grammar - that has a large number of functions that can assist the +validation needs - we then add a few extra functions that makes sense for operation teams. + +In each case accessing `value` would be the value passed from the user + +| Function | Description | +|-----------------------|-----------------------------------------------------------------| +| `is_ip(value)` | Checks if `value` is a IPv4 or IPv6 address | +| `is_ipv4(value)` | Checks if `value` is a IPv4 address | +| `is_ipv6(value)` | Checks if `value` is a IPv6 address | +| `is_shellsafe(value)` | Checks if `value` is a attempting to to do shell escape attacks | + ## Compiled Applications It's nice that you do not need to compile App Builder apps into binaries as it allows for fast iteration, but sometimes @@ -113,7 +146,7 @@ func main() { When you compile this as a normal Go application your binary will be an executable version of the app. Here we mount the application at the top level of the `myapp` binary, but you could also mount it later on - perhaps you -have other compiled in behaviours you wish to surface: +have other compiled in behaviors you wish to surface: ```go func main() { diff --git a/example/example_test.go b/example/example_test.go index 349ea1c..da60ec0 100644 --- a/example/example_test.go +++ b/example/example_test.go @@ -75,6 +75,15 @@ var _ = Describe("Example Application", func() { }) }) + Describe("Validation", func() { + It("Should correctly validate options", func() { + usageBuf.Reset() + + cmd.MustParseWithUsage(strings.Fields("basics required ginkgoginkgoginkgoginkgoginkgoginkgo")) + Expect(usageBuf.String()).To(ContainSubstring(`name: validation using "len(value) < 20" did not pass`)) + }) + }) + Describe("Basics", func() { Describe("required", func() { It("Should require a name", func() { diff --git a/example/sample-app.yaml b/example/sample-app.yaml index e9b73e8..a3355da 100644 --- a/example/sample-app.yaml +++ b/example/sample-app.yaml @@ -35,6 +35,7 @@ commands: - name: name description: The name of the person to greet required: true + validate: len(value) < 20 - name: surname description: An optional surname of the person to greet # We add an optional flag to override the "Hello" greeting diff --git a/go.mod b/go.mod index e855c02..4b423d0 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,8 @@ require ( github.com/AlecAivazis/survey/v2 v2.3.7 github.com/Masterminds/sprig/v3 v3.2.3 github.com/adrg/xdg v0.4.0 - github.com/choria-io/fisk v0.5.3 + github.com/antonmedv/expr v1.15.2 + github.com/choria-io/fisk v0.6.0 github.com/choria-io/goform v0.0.2 github.com/dustin/go-humanize v1.0.1 github.com/ghodss/yaml v1.0.0 @@ -28,7 +29,7 @@ require ( github.com/go-logr/logr v1.2.4 // indirect github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect github.com/google/go-cmp v0.5.9 // indirect - github.com/google/pprof v0.0.0-20230907193218-d3ddc7976beb // indirect + github.com/google/pprof v0.0.0-20230912144702-c363fe2c2ed8 // indirect github.com/google/uuid v1.3.1 // indirect github.com/huandu/xstrings v1.4.0 // indirect github.com/imdario/mergo v0.3.16 // indirect diff --git a/go.sum b/go.sum index 2dd35e1..5d542f1 100644 --- a/go.sum +++ b/go.sum @@ -13,8 +13,10 @@ github.com/adrg/xdg v0.4.0 h1:RzRqFcjH4nE5C6oTAxhBtoE2IRyjBSa62SCbyPidvls= github.com/adrg/xdg v0.4.0/go.mod h1:N6ag73EX4wyxeaoeHctc1mas01KZgsj5tYiAIwqJE/E= github.com/alessio/shellescape v1.4.1 h1:V7yhSDDn8LP4lc4jS8pFkt0zCnzVJlG5JXy9BVKJUX0= github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30= -github.com/choria-io/fisk v0.5.3 h1:n3BaCDRuhbtGjtwIdFukjm9xpngp9Gb9co9Hcpzjvig= -github.com/choria-io/fisk v0.5.3/go.mod h1:SutwJ9AoAJ9R/bmi4pzpGYXNgru8rqJeQnQ2jNX9IT0= +github.com/antonmedv/expr v1.15.2 h1:afFXpDWIC2n3bF+kTZE1JvFo+c34uaM3sTqh8z0xfdU= +github.com/antonmedv/expr v1.15.2/go.mod h1:0E/6TxnOlRNp81GMzX9QfDPAmHo2Phg00y4JUv1ihsE= +github.com/choria-io/fisk v0.6.0 h1:0x5zLzFR2CHCbSIIeuZUKryCzls/Bduw1i+8w82a1E8= +github.com/choria-io/fisk v0.6.0/go.mod h1:m6kd61ycRGwkyb0SDdgmcQXW9fQJuqeH4DKEjRxJewg= github.com/choria-io/goform v0.0.2 h1:6xVVCM8xtwSVFnn+XY5Aqsq6PWU4sxllkDAbkIoOVd4= github.com/choria-io/goform v0.0.2/go.mod h1:duNtNRvWQS+Hf2U4VjM7LDleQOOLGyJjx+OQH+tLM5s= github.com/creack/pty v1.1.17 h1:QeVUsEDNrLBW4tMgZHvxy18sKtr6VI492kBhUfhDJNI= @@ -34,8 +36,8 @@ github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4 github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/google/pprof v0.0.0-20230907193218-d3ddc7976beb h1:LCMfzVg3sflxTs4UvuP4D8CkoZnfHLe2qzqgDn/4OHs= -github.com/google/pprof v0.0.0-20230907193218-d3ddc7976beb/go.mod h1:czg5+yv1E0ZGTi6S6vVK1mke0fV+FaUhNGcd6VRS9Ik= +github.com/google/pprof v0.0.0-20230912144702-c363fe2c2ed8 h1:gpptm606MZYGaMHMsB4Srmb6EbW/IVHnt04rcMXnkBQ= +github.com/google/pprof v0.0.0-20230912144702-c363fe2c2ed8/go.mod h1:czg5+yv1E0ZGTi6S6vVK1mke0fV+FaUhNGcd6VRS9Ik= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.3.1 h1:KjJaJ9iWZ3jOFZIf1Lqf4laDRCasjl0BCmnEGxkdLb4= github.com/google/uuid v1.3.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= diff --git a/validator/validator.go b/validator/validator.go new file mode 100644 index 0000000..304d6c3 --- /dev/null +++ b/validator/validator.go @@ -0,0 +1,132 @@ +// Copyright (c) 2023, R.I. Pienaar and the Choria Project contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package validator + +import ( + "fmt" + "net" + "strings" + + "github.com/antonmedv/expr" + "github.com/choria-io/fisk" +) + +// FiskValidator is a fisk.OptionValidator that compatible with Validator() on arguments and flags +func FiskValidator(validation string) fisk.OptionValidator { + return func(value string) error { + ok, err := Validate(value, validation) + if err != nil { + return fmt.Errorf("validation using %q failed: %w", validation, err) + } + + if !ok { + return fmt.Errorf("validation using %q did not pass", validation) + } + + return nil + } +} + +// Validate validates value using the expr expression validation +func Validate(value any, validation string) (bool, error) { + var env any + + vs, ok := value.(string) + if ok { + env = map[string]any{ + "value": vs, + "Value": vs, + } + } else { + env = value + } + + program, err := expr.Compile(validation, expr.Env(env), expr.AsBool(), + ShellSafeValidator(), + IPv4Validator(), + IPv6Validator(), + IPvValidator(), + ) + if err != nil { + return false, err + } + + output, err := expr.Run(program, env) + if err != nil { + return false, err + } + + return output.(bool), nil +} + +func IPvValidator() expr.Option { + return expr.Function( + "is_ip", + func(params ...any) (any, error) { + val := params[0].(string) + ip := net.ParseIP(val) + + if ip == nil { + return false, fmt.Errorf("%s is not an IP address", val) + } + + return true, nil + }, + new(func(string) (bool, error))) +} + +func IPv4Validator() expr.Option { + return expr.Function( + "is_ipv4", + func(params ...any) (any, error) { + val := params[0].(string) + ip := net.ParseIP(val).To4() + + if ip == nil { + return false, fmt.Errorf("%s is not an IPv4 address", val) + } + + return true, nil + }, + new(func(string) (bool, error))) +} + +func IPv6Validator() expr.Option { + return expr.Function( + "is_ipv6", + func(params ...any) (any, error) { + val := params[0].(string) + ip := net.ParseIP(val) + + if ip == nil { + return false, fmt.Errorf("%s is not an IPv6 address", val) + } + + if ip.To4() != nil { + return false, fmt.Errorf("%s is not an IPv6 address", val) + } + + return true, nil + }, + new(func(string) (bool, error))) +} + +func ShellSafeValidator() expr.Option { + return expr.Function( + "is_shellsafe", + func(params ...any) (any, error) { + val := strings.TrimSpace(params[0].(string)) + badchars := []string{"`", "$", ";", "|", "&&", ">", "<"} + + for _, c := range badchars { + if strings.Contains(val, c) { + return false, fmt.Errorf("may not contain '%s'", c) + } + } + + return true, nil + }, + new(func(string) (bool, error))) +} diff --git a/validator/validator_test.go b/validator/validator_test.go new file mode 100644 index 0000000..588e0c8 --- /dev/null +++ b/validator/validator_test.go @@ -0,0 +1,79 @@ +// Copyright (c) 2023, R.I. Pienaar and the Choria Project contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package validator + +import ( + "fmt" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestBuilder(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Validator") +} + +var _ = Describe("Validator", func() { + Describe("is_ip", func() { + It("Should validate correctly", func() { + ok, err := Validate("1.1.1.1", "is_ip(value)") + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + + ok, err = Validate("2a00:1450:4002:405::20", "is_ip(value)") + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + + ok, err = Validate("bob", "is_ip(value)") + Expect(err.Error()).To(ContainSubstring("bob is not an IP address")) + Expect(ok).To(BeFalse()) + }) + }) + + Describe("is_ipv4", func() { + It("Should validate correctly", func() { + ok, err := Validate("1.1.1.1", "is_ipv4(value)") + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + + ok, err = Validate("2a00:1450:4002:405::20", "is_ipv4(value)") + Expect(err.Error()).To(ContainSubstring("2a00:1450:4002:405::20 is not an IPv4 address")) + Expect(ok).To(BeFalse()) + }) + }) + + Describe("is_ipv6", func() { + It("Should validate correctly", func() { + ok, err := Validate("2a00:1450:4002:405::20", "is_ipv6(value)") + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + + ok, err = Validate("1.1.1.1", "is_ipv6(value)") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("1.1.1.1 is not an IPv6 address")) + Expect(ok).To(BeFalse()) + }) + }) + + Describe("shellsafe", func() { + It("Should match bad strings", func() { + badchars := []string{"`", "$", ";", "|", "&&", ">", "<"} + + for _, c := range badchars { + ok, err := Validate(fmt.Sprintf("thing%sthing", c), "is_shellsafe(value)") + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("may not contain '%s'", c))) + Expect(ok).To(BeFalse()) + } + }) + + It("Should allow good things", func() { + Expect(Validate("ok", "is_shellsafe(value)")).To(BeTrue()) + Expect(Validate("", "is_shellsafe(value)")).To(BeTrue()) + Expect(Validate("ok ok ok", "is_shellsafe(value)")).To(BeTrue()) + }) + }) +})