From edd5fdcd57b71c89a23ce6345bfd52c28a2471aa Mon Sep 17 00:00:00 2001 From: Will Baker Date: Wed, 29 Dec 2021 11:46:20 -0500 Subject: [PATCH] fix(templates): disable use of jsonnet with `/api/v2/templates/apply` --- pkger/doc.go | 5 +- pkger/http_server_template.go | 35 +++++ pkger/http_server_template_test.go | 220 +++++++++++++++++++++++++++-- pkger/parser.go | 23 ++- pkger/parser_test.go | 30 +++- pkger/service.go | 30 ++++ pkger/service_test.go | 189 +++++++++++++++++++++++++ 7 files changed, 512 insertions(+), 20 deletions(-) diff --git a/pkger/doc.go b/pkger/doc.go index e54b06f611a..074244e95b3 100644 --- a/pkger/doc.go +++ b/pkger/doc.go @@ -4,7 +4,10 @@ templates for what will eventually come to support all influxdb resources. The parser supports JSON, Jsonnet, and YAML encodings as well as a number -of different ways to read the file/reader/string as it may. +of different ways to read the file/reader/string as it may. While the parser +supports Jsonnet, due to issues in the go-jsonnet implementation, only trusted +input should be given to the parser and therefore the EnableJsonnet() option +must be used with Parse() to enable Jsonnet support. As an example, you can use the following to parse and validate a YAML file and see a summary of its contents: diff --git a/pkger/http_server_template.go b/pkger/http_server_template.go index a15579d0d3c..e5f6004a77a 100644 --- a/pkger/http_server_template.go +++ b/pkger/http_server_template.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "net/url" "path" "strings" @@ -332,6 +333,40 @@ func (s *HTTPServerTemplates) apply(w http.ResponseWriter, r *http.Request) { return } + // Reject use of server-side jsonnet with /api/v2/templates/apply + if encoding == EncodingJsonnet { + s.api.Err(w, r, &errors.Error{ + Code: errors.EUnprocessableEntity, + Msg: fmt.Sprintf("template from source(s) had an issue: %s", ErrInvalidEncoding.Error()), + }) + return + } + + var remotes []string + for _, rem := range reqBody.Remotes { + remotes = append(remotes, rem.URL) + } + remotes = append(remotes, reqBody.RawTemplate.Sources...) + + for _, rem := range remotes { + // While things like '.%6Aonnet' evaluate to the default encoding (yaml), let's unescape and catch those too + decoded, err := url.QueryUnescape(rem) + if err != nil { + s.api.Err(w, r, &errors.Error{ + Code: errors.EInvalid, + Msg: fmt.Sprintf("template from url[%q] had an issue", rem), + }) + return + } + if len(decoded) > 0 && strings.HasSuffix(strings.ToLower(decoded), "jsonnet") { + s.api.Err(w, r, &errors.Error{ + Code: errors.EUnprocessableEntity, + Msg: fmt.Sprintf("template from url[%q] had an issue: %s", rem, ErrInvalidEncoding.Error()), + }) + return + } + } + var stackID platform.ID if reqBody.StackID != nil { if err := stackID.DecodeFromString(*reqBody.StackID); err != nil { diff --git a/pkger/http_server_template_test.go b/pkger/http_server_template_test.go index 2a68ad51ba0..28533dd8f6e 100644 --- a/pkger/http_server_template_test.go +++ b/pkger/http_server_template_test.go @@ -18,6 +18,7 @@ import ( "github.com/influxdata/influxdb/v2" pcontext "github.com/influxdata/influxdb/v2/context" "github.com/influxdata/influxdb/v2/kit/platform" + influxerror "github.com/influxdata/influxdb/v2/kit/platform/errors" kithttp "github.com/influxdata/influxdb/v2/kit/transport/http" "github.com/influxdata/influxdb/v2/mock" "github.com/influxdata/influxdb/v2/pkg/testttp" @@ -25,6 +26,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" "gopkg.in/yaml.v3" ) @@ -106,6 +108,138 @@ func TestPkgerHTTPServerTemplate(t *testing.T) { }) t.Run("dry run pkg", func(t *testing.T) { + t.Run("jsonnet disabled", func(t *testing.T) { + tests := []struct { + name string + contentType string + reqBody pkger.ReqApply + }{ + { + name: "app jsonnet disabled", + contentType: "application/x-jsonnet", + reqBody: pkger.ReqApply{ + DryRun: true, + OrgID: platform.ID(9000).String(), + RawTemplate: bucketPkgKinds(t, pkger.EncodingJsonnet), + }, + }, + { + name: "retrieves package from a URL (jsonnet disabled)", + reqBody: pkger.ReqApply{ + DryRun: true, + OrgID: platform.ID(9000).String(), + Remotes: []pkger.ReqTemplateRemote{{ + URL: newPkgURL(t, filesvr.URL, "testdata/bucket_associates_labels_one.jsonnet"), + }}, + }, + }, + { + name: "app json with jsonnet disabled remote", + contentType: "application/json", + reqBody: pkger.ReqApply{ + DryRun: true, + OrgID: platform.ID(9000).String(), + RawTemplate: bucketPkgJsonWithJsonnetRemote(t), + }, + }, + } + + for _, tt := range tests { + fn := func(t *testing.T) { + svc := &fakeSVC{ + dryRunFn: func(ctx context.Context, orgID, userID platform.ID, opts ...pkger.ApplyOptFn) (pkger.ImpactSummary, error) { + var opt pkger.ApplyOpt + for _, o := range opts { + o(&opt) + } + pkg, err := pkger.Combine(opt.Templates) + if err != nil { + return pkger.ImpactSummary{}, err + } + + if err := pkg.Validate(); err != nil { + return pkger.ImpactSummary{}, err + } + sum := pkg.Summary() + var diff pkger.Diff + for _, b := range sum.Buckets { + diff.Buckets = append(diff.Buckets, pkger.DiffBucket{ + DiffIdentifier: pkger.DiffIdentifier{ + MetaName: b.Name, + }, + }) + } + return pkger.ImpactSummary{ + Summary: sum, + Diff: diff, + }, nil + }, + } + + core, sink := observer.New(zap.InfoLevel) + pkgHandler := pkger.NewHTTPServerTemplates(zap.New(core), svc) + svr := newMountedHandler(pkgHandler, 1) + + ctx := context.Background() + testttp. + PostJSON(t, "/api/v2/templates/apply", tt.reqBody). + Headers("Content-Type", tt.contentType). + WithCtx(ctx). + Do(svr). + ExpectStatus(http.StatusUnprocessableEntity). + ExpectBody(func(buf *bytes.Buffer) { + var resp pkger.RespApply + decodeBody(t, buf, &resp) + + assert.Len(t, resp.Summary.Buckets, 0) + assert.Len(t, resp.Diff.Buckets, 0) + }) + + // Verify logging when jsonnet is disabled + entries := sink.TakeAll() // resets to 0 + if tt.contentType == "application/x-jsonnet" { + require.Equal(t, 1, len(entries)) + // message 0 + require.Equal(t, zap.ErrorLevel, entries[0].Entry.Level) + require.Equal(t, "api error encountered", entries[0].Entry.Message) + assert.ElementsMatch(t, []zap.Field{ + zap.Error(&influxerror.Error{ + Code: influxerror.EUnprocessableEntity, + Msg: "template from source(s) had an issue: invalid encoding provided", + }, + )}, entries[0].Context) + } else if len(tt.reqBody.Remotes) == 1 && strings.HasSuffix(tt.reqBody.Remotes[0].URL, "jsonnet") { + require.Equal(t, 1, len(entries)) + // message 0 + require.Equal(t, zap.ErrorLevel, entries[0].Entry.Level) + require.Equal(t, "api error encountered", entries[0].Entry.Message) + expMsg := fmt.Sprintf("template from url[\"%s\"] had an issue: invalid encoding provided", tt.reqBody.Remotes[0].URL) + assert.ElementsMatch(t, []zap.Field{ + zap.Error(&influxerror.Error{ + Code: influxerror.EUnprocessableEntity, + Msg: expMsg, + }, + )}, entries[0].Context) + } else if len(tt.reqBody.RawTemplate.Sources) == 1 && strings.HasSuffix(tt.reqBody.RawTemplate.Sources[0], "jsonnet") { + require.Equal(t, 1, len(entries)) + // message 0 + require.Equal(t, zap.ErrorLevel, entries[0].Entry.Level) + require.Equal(t, "api error encountered", entries[0].Entry.Message) + expMsg := fmt.Sprintf("template from url[\"%s\"] had an issue: invalid encoding provided", tt.reqBody.RawTemplate.Sources[0]) + assert.ElementsMatch(t, []zap.Field{ + zap.Error(&influxerror.Error{ + Code: influxerror.EUnprocessableEntity, + Msg: expMsg, + }, + )}, entries[0].Context) + } else { + require.Equal(t, 0, len(entries)) + } + } + t.Run(tt.name, fn) + } + }) + t.Run("json", func(t *testing.T) { tests := []struct { name string @@ -130,7 +264,7 @@ func TestPkgerHTTPServerTemplate(t *testing.T) { }, }, { - name: "retrieves package from a URL", + name: "retrieves package from a URL (json)", reqBody: pkger.ReqApply{ DryRun: true, OrgID: platform.ID(9000).String(), @@ -139,15 +273,6 @@ func TestPkgerHTTPServerTemplate(t *testing.T) { }}, }, }, - { - name: "app jsonnet", - contentType: "application/x-jsonnet", - reqBody: pkger.ReqApply{ - DryRun: true, - OrgID: platform.ID(9000).String(), - RawTemplate: bucketPkgKinds(t, pkger.EncodingJsonnet), - }, - }, } for _, tt := range tests { @@ -182,7 +307,8 @@ func TestPkgerHTTPServerTemplate(t *testing.T) { }, } - pkgHandler := pkger.NewHTTPServerTemplates(zap.NewNop(), svc) + core, _ := observer.New(zap.InfoLevel) + pkgHandler := pkger.NewHTTPServerTemplates(zap.New(core), svc) svr := newMountedHandler(pkgHandler, 1) testttp. @@ -198,7 +324,6 @@ func TestPkgerHTTPServerTemplate(t *testing.T) { assert.Len(t, resp.Diff.Buckets, 1) }) } - t.Run(tt.name, fn) } }) @@ -566,6 +691,48 @@ func TestPkgerHTTPServerTemplate(t *testing.T) { }) }) }) + + t.Run("Templates()", func(t *testing.T) { + tests := []struct { + name string + reqBody pkger.ReqApply + encoding pkger.Encoding + }{ + { + name: "jsonnet disabled", + reqBody: pkger.ReqApply{ + OrgID: platform.ID(9000).String(), + RawTemplate: bucketPkgKinds(t, pkger.EncodingJsonnet), + }, + encoding: pkger.EncodingJsonnet, + }, + { + name: "jsonnet remote disabled", + reqBody: pkger.ReqApply{ + OrgID: platform.ID(9000).String(), + Remotes: []pkger.ReqTemplateRemote{{ + URL: newPkgURL(t, filesvr.URL, "testdata/bucket_associates_labels_one.jsonnet"), + }}, + }, + encoding: pkger.EncodingJsonnet, + }, + { + name: "jsonnet disabled remote source", + reqBody: pkger.ReqApply{ + OrgID: platform.ID(9000).String(), + RawTemplate: bucketPkgJsonWithJsonnetRemote(t), + }, + encoding: pkger.EncodingJSON, + }, + } + + for _, tt := range tests { + tmpl, err := tt.reqBody.Templates(tt.encoding) + assert.Nil(t, tmpl) + require.Error(t, err) + assert.Equal(t, "unprocessable entity", influxerror.ErrorCode(err)) + } + }) } func assertNonZeroApplyResp(t *testing.T, resp pkger.RespApply) { @@ -644,7 +811,7 @@ spec: require.FailNow(t, "invalid encoding provided: "+encoding.String()) } - pkg, err := pkger.Parse(encoding, pkger.FromString(fmt.Sprintf(pkgStr, pkger.APIVersion))) + pkg, err := pkger.Parse(encoding, pkger.FromString(fmt.Sprintf(pkgStr, pkger.APIVersion)), pkger.EnableJsonnet()) require.NoError(t, err) b, err := pkg.Encode(encoding) @@ -656,6 +823,33 @@ spec: } } +func bucketPkgJsonWithJsonnetRemote(t *testing.T) pkger.ReqRawTemplate { + pkgStr := `[ + { + "apiVersion": "%[1]s", + "kind": "Bucket", + "metadata": { + "name": "rucket-11" + }, + "spec": { + "description": "bucket 1 description" + } + } +] +` + // Create a json template and then add a jsonnet remote raw template + pkg, err := pkger.Parse(pkger.EncodingJSON, pkger.FromString(fmt.Sprintf(pkgStr, pkger.APIVersion))) + require.NoError(t, err) + + b, err := pkg.Encode(pkger.EncodingJSON) + require.NoError(t, err) + return pkger.ReqRawTemplate{ + ContentType: pkger.EncodingJsonnet.String(), + Sources: []string{"file:///nonexistent.jsonnet"}, + Template: b, + } +} + func newReqApplyYMLBody(t *testing.T, orgID platform.ID, dryRun bool) *bytes.Buffer { t.Helper() diff --git a/pkger/parser.go b/pkger/parser.go index 20b447f0208..0b1e5e4e935 100644 --- a/pkger/parser.go +++ b/pkger/parser.go @@ -209,7 +209,16 @@ func parseJSON(r io.Reader, opts ...ValidateOptFn) (*Template, error) { } func parseJsonnet(r io.Reader, opts ...ValidateOptFn) (*Template, error) { - return parse(jsonnet.NewDecoder(r), opts...) + opt := &validateOpt{} + for _, o := range opts { + o(opt) + } + // For security, we'll default to disabling parsing jsonnet but allow callers to override the behavior via + // EnableJsonnet(). Enabling jsonnet might be useful for client code where parsing jsonnet could be acceptable. + if opt.enableJsonnet { + return parse(jsonnet.NewDecoder(r), opts...) + } + return nil, fmt.Errorf("%s: jsonnet", ErrInvalidEncoding) } func parseSource(r io.Reader, opts ...ValidateOptFn) (*Template, error) { @@ -536,14 +545,22 @@ func Combine(pkgs []*Template, validationOpts ...ValidateOptFn) (*Template, erro type ( validateOpt struct { - minResources bool - skipValidate bool + minResources bool + skipValidate bool + enableJsonnet bool } // ValidateOptFn provides a means to disable desired validation checks. ValidateOptFn func(*validateOpt) ) +// Jsonnet parsing is disabled by default. EnableJsonnet turns it back on. +func EnableJsonnet() ValidateOptFn { + return func(opt *validateOpt) { + opt.enableJsonnet = true + } +} + // ValidWithoutResources ignores the validation check for minimum number // of resources. This is useful for the service Create to ignore this and // allow the creation of a pkg without resources. diff --git a/pkger/parser_test.go b/pkger/parser_test.go index 91b7a2e79c1..982c373e6ad 100644 --- a/pkger/parser_test.go +++ b/pkger/parser_test.go @@ -4441,8 +4441,13 @@ spec: }) }) - t.Run("jsonnet support", func(t *testing.T) { + t.Run("jsonnet support disabled by default", func(t *testing.T) { template := validParsedTemplateFromFile(t, "testdata/bucket_associates_labels.jsonnet", EncodingJsonnet) + require.Equal(t, &Template{}, template) + }) + + t.Run("jsonnet support", func(t *testing.T) { + template := validParsedTemplateFromFile(t, "testdata/bucket_associates_labels.jsonnet", EncodingJsonnet, EnableJsonnet()) sum := template.Summary() @@ -4934,7 +4939,7 @@ func nextField(t *testing.T, field string) (string, int) { return "", -1 } -func validParsedTemplateFromFile(t *testing.T, path string, encoding Encoding) *Template { +func validParsedTemplateFromFile(t *testing.T, path string, encoding Encoding, opts ...ValidateOptFn) *Template { t.Helper() var readFn ReaderFn @@ -4946,7 +4951,17 @@ func validParsedTemplateFromFile(t *testing.T, path string, encoding Encoding) * atomic.AddInt64(&missedTemplateCacheCounter, 1) } - template := newParsedTemplate(t, readFn, encoding) + opt := &validateOpt{} + for _, o := range opts { + o(opt) + } + + template := newParsedTemplate(t, readFn, encoding, opts...) + if encoding == EncodingJsonnet && !opt.enableJsonnet { + require.Equal(t, &Template{}, template) + return template + } + u := url.URL{ Scheme: "file", Path: path, @@ -4958,7 +4973,16 @@ func validParsedTemplateFromFile(t *testing.T, path string, encoding Encoding) * func newParsedTemplate(t *testing.T, fn ReaderFn, encoding Encoding, opts ...ValidateOptFn) *Template { t.Helper() + opt := &validateOpt{} + for _, o := range opts { + o(opt) + } + template, err := Parse(encoding, fn, opts...) + if encoding == EncodingJsonnet && !opt.enableJsonnet { + require.Error(t, err) + return &Template{} + } require.NoError(t, err) for _, k := range template.Objects { diff --git a/pkger/service.go b/pkger/service.go index 0ce80ecffc2..b620bfc335c 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -362,6 +362,21 @@ func (s *Service) InitStack(ctx context.Context, userID platform.ID, stCreate St return Stack{}, err } + // Reject use of server-side jsonnet with stack templates + for _, u := range stCreate.TemplateURLs { + // While things like '.%6Aonnet' evaluate to the default encoding (yaml), let's unescape and catch those too + decoded, err := url.QueryUnescape(u) + if err != nil { + msg := fmt.Sprintf("stack template from url[%q] had an issue", u) + return Stack{}, influxErr(errors2.EInvalid, msg) + } + + if strings.HasSuffix(strings.ToLower(decoded), "jsonnet") { + msg := fmt.Sprintf("stack template from url[%q] had an issue: %s", u, ErrInvalidEncoding.Error()) + return Stack{}, influxErr(errors2.EUnprocessableEntity, msg) + } + } + if _, err := s.orgSVC.FindOrganizationByID(ctx, stCreate.OrgID); err != nil { if errors2.ErrorCode(err) == errors2.ENotFound { msg := fmt.Sprintf("organization dependency does not exist for id[%q]", stCreate.OrgID.String()) @@ -476,6 +491,21 @@ func (s *Service) UpdateStack(ctx context.Context, upd StackUpdate) (Stack, erro return Stack{}, err } + // Reject use of server-side jsonnet with stack templates + for _, u := range upd.TemplateURLs { + // While things like '.%6Aonnet' evaluate to the default encoding (yaml), let's unescape and catch those too + decoded, err := url.QueryUnescape(u) + if err != nil { + msg := fmt.Sprintf("stack template from url[%q] had an issue", u) + return Stack{}, influxErr(errors2.EInvalid, msg) + } + + if strings.HasSuffix(strings.ToLower(decoded), "jsonnet") { + msg := fmt.Sprintf("stack template from url[%q] had an issue: %s", u, ErrInvalidEncoding.Error()) + return Stack{}, influxErr(errors2.EUnprocessableEntity, msg) + } + } + updatedStack := s.applyStackUpdate(existing, upd) if err := s.store.UpdateStack(ctx, updatedStack); err != nil { return Stack{}, err diff --git a/pkger/service_test.go b/pkger/service_test.go index 76876b309a0..5c8b94468ca 100644 --- a/pkger/service_test.go +++ b/pkger/service_test.go @@ -5044,6 +5044,96 @@ func TestService(t *testing.T) { t.Run(tt.name, fn) } }) + + t.Run("jsonnet template url", func(t *testing.T) { + tests := []struct { + name string + create StackCreate + expectedErrCode string + }{ + // always valid + { + name: "no templates", + create: StackCreate{OrgID: 3333}, + }, + { + name: "one json template", + create: StackCreate{ + OrgID: 3333, + TemplateURLs: []string{"http://fake/some.json"}, + }, + }, + { + name: "one yaml template", + create: StackCreate{ + OrgID: 3333, + TemplateURLs: []string{"http://fake/some.yaml"}, + }, + }, + { + name: "multiple templates", + create: StackCreate{ + OrgID: 3333, + TemplateURLs: []string{ + "http://fake/some.yaml", + "http://fake/some.json", + "http://fake/other.yaml", + }, + }, + }, + // invalid + { + name: "one jsonnet template", + create: StackCreate{ + OrgID: 3333, + TemplateURLs: []string{"http://fake/some.jsonnet"}, + }, + expectedErrCode: "unprocessable entity", + }, + { + name: "multiple with one jsonnet template", + create: StackCreate{ + OrgID: 3333, + TemplateURLs: []string{ + "http://fake/some.json", + "http://fake/some.jsonnet", + "http://fake/some.yaml", + }, + }, + expectedErrCode: "unprocessable entity", + }, + { + name: "one weird jsonnet template", + create: StackCreate{ + OrgID: 3333, + TemplateURLs: []string{"http://fake/some.%6asonnet"}, + }, + expectedErrCode: "unprocessable entity", + }, + } + + svc := newTestService( + WithIDGenerator(newFakeIDGen(3)), + WithTimeGenerator(newTimeGen(now)), + WithStore(newFakeStore(safeCreateFn)), + ) + + for _, tt := range tests { + ctx := context.Background() + stack, err := svc.InitStack(ctx, 9000, tt.create) + if tt.expectedErrCode == "" { + require.NoError(t, err) + assert.Equal(t, platform.ID(3), stack.ID) + assert.Equal(t, platform.ID(3333), stack.OrgID) + assert.Equal(t, now, stack.CreatedAt) + assert.Equal(t, now, stack.LatestEvent().UpdatedAt) + } else { + require.Error(t, err) + assert.Equal(t, tt.expectedErrCode, errors2.ErrorCode(err)) + assert.Equal(t, Stack{}, stack) + } + } + }) }) t.Run("UpdateStack", func(t *testing.T) { @@ -5229,6 +5319,105 @@ func TestService(t *testing.T) { t.Run(tt.name, fn) } }) + t.Run("jsonnet template url", func(t *testing.T) { + tests := []struct { + name string + update StackUpdate + expectedErrCode string + }{ + // always valid + { + name: "no templates", + update: StackUpdate{ID: 3333}, + }, + { + name: "one json template", + update: StackUpdate{ + ID: 3333, + TemplateURLs: []string{"http://fake/some.json"}, + }, + }, + { + name: "one yaml template", + update: StackUpdate{ + ID: 3333, + TemplateURLs: []string{"http://fake/some.yaml"}, + }, + }, + { + name: "multiple templates", + update: StackUpdate{ + ID: 3333, + TemplateURLs: []string{ + "http://fake/some.yaml", + "http://fake/some.json", + "http://fake/other.yaml", + }, + }, + }, + // invalid + { + name: "one jsonnet template", + update: StackUpdate{ + ID: 3333, + TemplateURLs: []string{"http://fake/some.jsonnet"}, + }, + expectedErrCode: "unprocessable entity", + }, + { + name: "multiple with one jsonnet template", + update: StackUpdate{ + ID: 3333, + TemplateURLs: []string{ + "http://fake/some.json", + "http://fake/some.jsonnet", + "http://fake/some.yaml", + }, + }, + expectedErrCode: "unprocessable entity", + }, + { + name: "one weird jsonnet template", + update: StackUpdate{ + ID: 3333, + TemplateURLs: []string{"http://fake/some.%6asonnet"}, + }, + expectedErrCode: "unprocessable entity", + }, + } + + for _, tt := range tests { + svc := newTestService( + WithIDGenerator(mock.IDGenerator{ + IDFn: func() platform.ID { + return 333 + }, + }), + WithTimeGenerator(newTimeGen(now)), + WithStore(&fakeStore{ + readFn: func(ctx context.Context, id platform.ID) (Stack, error) { + return Stack{ID: id, OrgID: 3}, nil + }, + updateFn: func(ctx context.Context, stack Stack) error { + return nil + }, + }), + ) + + ctx := context.Background() + stack, err := svc.UpdateStack(ctx, tt.update) + if tt.expectedErrCode == "" { + require.NoError(t, err) + assert.Equal(t, platform.ID(3333), stack.ID) + assert.Equal(t, platform.ID(3), stack.OrgID) + assert.Equal(t, now, stack.LatestEvent().UpdatedAt) + } else { + require.Error(t, err) + assert.Equal(t, tt.expectedErrCode, errors2.ErrorCode(err)) + assert.Equal(t, Stack{}, stack) + } + } + }) }) }