diff --git a/cmd/drone-server/config/config.go b/cmd/drone-server/config/config.go index b871ab564..2b683a4e4 100644 --- a/cmd/drone-server/config/config.go +++ b/cmd/drone-server/config/config.go @@ -148,6 +148,7 @@ type ( Starlark struct { Enabled bool `envconfig:"DRONE_STARLARK_ENABLED"` StepLimit uint64 `envconfig:"DRONE_STARLARK_STEP_LIMIT"` + SizeLimit uint64 `envconfig:"DRONE_STARLARK_SIZE_LIMIT" default:"0"` } // License provides license configuration diff --git a/cmd/drone-server/inject_plugin.go b/cmd/drone-server/inject_plugin.go index dfd80a9b8..a97e8e8da 100644 --- a/cmd/drone-server/inject_plugin.go +++ b/cmd/drone-server/inject_plugin.go @@ -83,6 +83,7 @@ func provideConvertPlugin(client *scm.Client, fileService core.FileService, conf converter.Starlark( conf.Starlark.Enabled, conf.Starlark.StepLimit, + conf.Starlark.SizeLimit, ), converter.Jsonnet( conf.Jsonnet.Enabled, @@ -92,6 +93,7 @@ func provideConvertPlugin(client *scm.Client, fileService core.FileService, conf converter.Template( templateStore, conf.Starlark.StepLimit, + conf.Starlark.SizeLimit, ), converter.Memoize( converter.Remote( diff --git a/plugin/converter/starlark.go b/plugin/converter/starlark.go index 8238a950e..67659254b 100644 --- a/plugin/converter/starlark.go +++ b/plugin/converter/starlark.go @@ -26,16 +26,18 @@ import ( // Starlark returns a conversion service that converts the // starlark file to a yaml file. -func Starlark(enabled bool, stepLimit uint64) core.ConvertService { +func Starlark(enabled bool, stepLimit uint64, sizeLimit uint64) core.ConvertService { return &starlarkPlugin{ enabled: enabled, stepLimit: stepLimit, + sizeLimit: sizeLimit, } } type starlarkPlugin struct { enabled bool stepLimit uint64 + sizeLimit uint64 } func (p *starlarkPlugin) Convert(ctx context.Context, req *core.ConvertArgs) (*core.Config, error) { @@ -53,7 +55,7 @@ func (p *starlarkPlugin) Convert(ctx context.Context, req *core.ConvertArgs) (*c return nil, nil } - file, err := starlark.Parse(req, nil, nil, p.stepLimit) + file, err := starlark.Parse(req, nil, nil, p.stepLimit, p.sizeLimit) if err != nil { return nil, err } diff --git a/plugin/converter/starlark/starlark.go b/plugin/converter/starlark/starlark.go index 3ad3239b4..5e7a79904 100644 --- a/plugin/converter/starlark/starlark.go +++ b/plugin/converter/starlark/starlark.go @@ -29,8 +29,8 @@ const ( newline = "\n" ) -// limits generated configuration file size. -const limit = 1000000 +// default limit for generated configuration file size. +const defaultSizeLimit = 1000000 var ( // ErrMainMissing indicates the starlark script is missing @@ -54,7 +54,7 @@ var ( ErrCannotLoad = errors.New("starlark: cannot load external scripts") ) -func Parse(req *core.ConvertArgs, template *core.Template, templateData map[string]interface{}, stepLimit uint64) (string, error) { +func Parse(req *core.ConvertArgs, template *core.Template, templateData map[string]interface{}, stepLimit uint64, sizeLimit uint64) (string, error) { thread := &starlark.Thread{ Name: "drone", Load: noLoad, @@ -132,9 +132,13 @@ func Parse(req *core.ConvertArgs, template *core.Template, templateData map[stri return "", ErrMainReturn } + if sizeLimit == 0 { + sizeLimit = defaultSizeLimit + } + // this is a temporary workaround until we // implement a LimitWriter. - if b := buf.Bytes(); len(b) > limit { + if b := buf.Bytes(); uint64(len(b)) > sizeLimit { return "", ErrMaximumSize } return buf.String(), nil diff --git a/plugin/converter/starlark/starlark_test.go b/plugin/converter/starlark/starlark_test.go index 6700fdd5e..8e7871c97 100644 --- a/plugin/converter/starlark/starlark_test.go +++ b/plugin/converter/starlark/starlark_test.go @@ -57,7 +57,7 @@ func TestParseStarlark(t *testing.T) { req.Config.Data = string(before) - parsedFile, err := Parse(req, template, templateData, 0) + parsedFile, err := Parse(req, template, templateData, 0, 0) if err != nil { t.Error(err) return @@ -95,7 +95,7 @@ func TestParseStarlarkNotTemplateFile(t *testing.T) { req.Repo.Config = "plugin.starlark.star" req.Config.Data = string(before) - parsedFile, err := Parse(req, nil, nil, 0) + parsedFile, err := Parse(req, nil, nil, 0, 0) if err != nil { t.Error(err) return diff --git a/plugin/converter/starlark_oss.go b/plugin/converter/starlark_oss.go index 865d9b6c5..0444be33d 100644 --- a/plugin/converter/starlark_oss.go +++ b/plugin/converter/starlark_oss.go @@ -18,6 +18,6 @@ package converter import "github.com/drone/drone/core" -func Starlark(enabled bool, stepLimit uint64) core.ConvertService { +func Starlark(enabled bool, stepLimit uint64, sizeLimit uint64) core.ConvertService { return new(noop) } diff --git a/plugin/converter/starlark_test.go b/plugin/converter/starlark_test.go index 72a86b017..6f3c8fd36 100644 --- a/plugin/converter/starlark_test.go +++ b/plugin/converter/starlark_test.go @@ -24,7 +24,7 @@ import ( ) func TestStarlarkConvert(t *testing.T) { - plugin := Starlark(true, 0) + plugin := Starlark(true, 0, 0) req := &core.ConvertArgs{ Build: &core.Build{ @@ -102,7 +102,7 @@ func TestConvert_Multi(t *testing.T) { }, } - plugin := Starlark(true, 0) + plugin := Starlark(true, 0, 0) config, err := plugin.Convert(noContext, req) if err != nil { t.Error(err) @@ -134,7 +134,7 @@ func TestConvert_Multi(t *testing.T) { // this test verifies the plugin is skipped when it has // not been explicitly enabled. func TestConvert_Skip(t *testing.T) { - plugin := Starlark(false, 0) + plugin := Starlark(false, 0, 0) config, err := plugin.Convert(noContext, nil) if err != nil { t.Error(err) @@ -154,7 +154,7 @@ func TestConvert_SkipYaml(t *testing.T) { }, } - plugin := Starlark(true, 0) + plugin := Starlark(true, 0, 0) config, err := plugin.Convert(noContext, req) if err != nil { t.Error(err) @@ -164,3 +164,35 @@ func TestConvert_SkipYaml(t *testing.T) { t.Errorf("Expect nil config returned for non-starlark files") } } + +// this test verifies the plugin returns error +// if the generated file size is exceeded. +func TestConvert_SizeLimit(t *testing.T) { + smallFileSizeLimit := uint64(1) + plugin := Starlark(true, 0, smallFileSizeLimit) + + req := &core.ConvertArgs{ + Build: &core.Build{ + After: "3d21ec53a331a6f037a91c368710b99387d012c1", + }, + Repo: &core.Repository{ + Slug: "octocat/hello-world", + Config: ".drone.yml", + }, + Config: &core.Config{}, + } + + before, err := ioutil.ReadFile("testdata/single.star") + if err != nil { + t.Error(err) + return + } + + req.Repo.Config = "single.star" + req.Config.Data = string(before) + _, expectedError := plugin.Convert(noContext, req) + if expectedError == nil { + t.Error("Expected 'starlark: maximum file size exceeded' error") + return + } +} \ No newline at end of file diff --git a/plugin/converter/template.go b/plugin/converter/template.go index 3ea60fb0f..a8d2ab595 100644 --- a/plugin/converter/template.go +++ b/plugin/converter/template.go @@ -41,16 +41,18 @@ var ( errTemplateExtensionInvalid = errors.New("template extension invalid. must be yaml, starlark or jsonnet") ) -func Template(templateStore core.TemplateStore, stepLimit uint64) core.ConvertService { +func Template(templateStore core.TemplateStore, stepLimit uint64, sizeLimit uint64) core.ConvertService { return &templatePlugin{ templateStore: templateStore, stepLimit: stepLimit, + sizeLimit: sizeLimit, } } type templatePlugin struct { templateStore core.TemplateStore stepLimit uint64 + sizeLimit uint64 } func (p *templatePlugin) Convert(ctx context.Context, req *core.ConvertArgs) (*core.Config, error) { @@ -84,7 +86,7 @@ func (p *templatePlugin) Convert(ctx context.Context, req *core.ConvertArgs) (*c case ".yml", ".yaml": return parseYaml(req, template, templateArgs) case ".star", ".starlark", ".script": - return parseStarlark(req, template, templateArgs, p.stepLimit) + return parseStarlark(req, template, templateArgs, p.stepLimit, p.sizeLimit) case ".jsonnet": return parseJsonnet(req, template, templateArgs) default: @@ -122,8 +124,8 @@ func parseJsonnet(req *core.ConvertArgs, template *core.Template, templateArgs c }, nil } -func parseStarlark(req *core.ConvertArgs, template *core.Template, templateArgs core.TemplateArgs, stepLimit uint64) (*core.Config, error) { - file, err := starlark.Parse(req, template, templateArgs.Data, stepLimit) +func parseStarlark(req *core.ConvertArgs, template *core.Template, templateArgs core.TemplateArgs, stepLimit uint64, sizeLimit uint64) (*core.Config, error) { + file, err := starlark.Parse(req, template, templateArgs.Data, stepLimit, sizeLimit) if err != nil { return nil, err } diff --git a/plugin/converter/template_oss.go b/plugin/converter/template_oss.go index f1b604e8e..29ca662e8 100644 --- a/plugin/converter/template_oss.go +++ b/plugin/converter/template_oss.go @@ -22,7 +22,7 @@ import ( "github.com/drone/drone/core" ) -func Template(templateStore core.TemplateStore, stepLimit uint64) core.ConvertService { +func Template(templateStore core.TemplateStore, stepLimit uint64, sizeLimit uint64) core.ConvertService { return &templatePlugin{ templateStore: templateStore, } diff --git a/plugin/converter/template_test.go b/plugin/converter/template_test.go index 5103184e8..f78d2a86c 100644 --- a/plugin/converter/template_test.go +++ b/plugin/converter/template_test.go @@ -73,7 +73,7 @@ func TestTemplatePluginConvertStarlark(t *testing.T) { templates := mock.NewMockTemplateStore(controller) templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil) - plugin := Template(templates, 0) + plugin := Template(templates, 0, 0) config, err := plugin.Convert(noContext, req) if err != nil { t.Error(err) @@ -92,7 +92,7 @@ func TestTemplatePluginConvertStarlark(t *testing.T) { func TestTemplatePluginConvertNotYamlFile(t *testing.T) { - plugin := Template(nil, 0) + plugin := Template(nil, 0, 0) req := &core.ConvertArgs{ Build: &core.Build{ After: "3d21ec53a331a6f037a91c368710b99387d012c1", @@ -120,7 +120,7 @@ func TestTemplatePluginConvertDroneFileTypePipeline(t *testing.T) { t.Error(err) return } - plugin := Template(nil, 0) + plugin := Template(nil, 0, 0) req := &core.ConvertArgs{ Build: &core.Build{ After: "3d21ec53a331a6f037a91c368710b99387d012c1", @@ -161,7 +161,7 @@ func TestTemplatePluginConvertDroneFileYamlExtensions(t *testing.T) { templates := mock.NewMockTemplateStore(controller) templates.EXPECT().FindName(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, dummyErr) - plugin := Template(templates, 0) + plugin := Template(templates, 0, 0) req := &core.ConvertArgs{ Build: &core.Build{ After: "3d21ec53a331a6f037a91c368710b99387d012c1", @@ -214,7 +214,7 @@ func TestTemplatePluginConvertTemplateNotFound(t *testing.T) { templates := mock.NewMockTemplateStore(controller) templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(nil, nil) - plugin := Template(templates, 0) + plugin := Template(templates, 0, 0) config, err := plugin.Convert(noContext, req) if config != nil { @@ -267,7 +267,7 @@ func TestTemplatePluginConvertJsonnet(t *testing.T) { templates := mock.NewMockTemplateStore(controller) templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil) - plugin := Template(templates, 0) + plugin := Template(templates, 0, 0) config, err := plugin.Convert(noContext, req) if err != nil { t.Error(err) @@ -346,7 +346,7 @@ func TestTemplateNestedValuesPluginConvertStarlark(t *testing.T) { templates := mock.NewMockTemplateStore(controller) templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil) - plugin := Template(templates, 0) + plugin := Template(templates, 0, 0) config, err := plugin.Convert(noContext, req) if err != nil { t.Error(err) @@ -424,7 +424,7 @@ func TestTemplatePluginConvertYaml(t *testing.T) { templates := mock.NewMockTemplateStore(controller) templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil) - plugin := Template(templates, 0) + plugin := Template(templates, 0, 0) config, err := plugin.Convert(noContext, req) if err != nil { t.Error(err) @@ -483,7 +483,7 @@ func TestTemplatePluginConvertInvalidTemplateExtension(t *testing.T) { templates := mock.NewMockTemplateStore(controller) templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil) - plugin := Template(templates, 0) + plugin := Template(templates, 0, 0) config, err := plugin.Convert(noContext, req) if config != nil { t.Errorf("template extension invalid. must be yaml, starlark or jsonnet") @@ -535,7 +535,7 @@ func TestTemplatePluginConvertYamlWithComment(t *testing.T) { templates := mock.NewMockTemplateStore(controller) templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil) - plugin := Template(templates, 0) + plugin := Template(templates, 0, 0) config, err := plugin.Convert(noContext, req) if err != nil { t.Error(err)