From 8c0d6eabdc82114e1a0739e923793f13354373db Mon Sep 17 00:00:00 2001 From: semihbkgr Date: Sat, 2 Nov 2024 22:13:11 +0300 Subject: [PATCH 1/5] not allow undeclared or empty taskfile in include --- task_test.go | 29 +++++++++++++++++++ taskfile/reader.go | 4 +++ .../inlined_taskfile_empty/Taskfile.yml | 9 ++++++ .../inlined_taskfile_null/Taskfile.yml | 9 ++++++ .../taskfile_empty/Taskfile.yml | 10 +++++++ .../taskfile_null/Taskfile.yml | 10 +++++++ 6 files changed, 71 insertions(+) create mode 100644 testdata/includes_missing_taskfile/inlined_taskfile_empty/Taskfile.yml create mode 100644 testdata/includes_missing_taskfile/inlined_taskfile_null/Taskfile.yml create mode 100644 testdata/includes_missing_taskfile/taskfile_empty/Taskfile.yml create mode 100644 testdata/includes_missing_taskfile/taskfile_null/Taskfile.yml diff --git a/task_test.go b/task_test.go index 599277c8c5..53e7433521 100644 --- a/task_test.go +++ b/task_test.go @@ -1527,6 +1527,35 @@ func TestIncludesInterpolation(t *testing.T) { } } +func TestIncludesMissingTaskfile(t *testing.T) { + const dir = "testdata/includes_missing_taskfile" + const error = "no taskfile is specified for include \"included\"" + + tests := []struct { + name string + task string + }{ + {"inlined_taskfile_empty", "empty inline taskfile"}, + {"inlined_taskfile_null", "null inline taskfile"}, + {"taskfile_empty", "empty taskfile"}, + {"taskfile_null", "null taskfile"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var buff bytes.Buffer + e := task.Executor{ + Dir: filepath.Join(dir, test.name), + Stdout: &buff, + Stderr: &buff, + Silent: true, + } + err := e.Setup() + assert.ErrorContains(t, err, error) + }) + } +} + func TestIncludedTaskfileVarMerging(t *testing.T) { const dir = "testdata/included_taskfile_var_merging" tests := []struct { diff --git a/taskfile/reader.go b/taskfile/reader.go index 69f75b1d19..0d48f400a2 100644 --- a/taskfile/reader.go +++ b/taskfile/reader.go @@ -121,6 +121,10 @@ func (r *Reader) include(node Node) error { return err } + if include.Taskfile == "" { + return errors.New(fmt.Sprintf("no taskfile is specified for include %q", include.Namespace)) + } + entrypoint, err := node.ResolveEntrypoint(include.Taskfile) if err != nil { return err diff --git a/testdata/includes_missing_taskfile/inlined_taskfile_empty/Taskfile.yml b/testdata/includes_missing_taskfile/inlined_taskfile_empty/Taskfile.yml new file mode 100644 index 0000000000..58f3516ced --- /dev/null +++ b/testdata/includes_missing_taskfile/inlined_taskfile_empty/Taskfile.yml @@ -0,0 +1,9 @@ +version: '3' + +includes: + included: '' + +tasks: + task-1: + cmds: + - task: included:default diff --git a/testdata/includes_missing_taskfile/inlined_taskfile_null/Taskfile.yml b/testdata/includes_missing_taskfile/inlined_taskfile_null/Taskfile.yml new file mode 100644 index 0000000000..d48191c23d --- /dev/null +++ b/testdata/includes_missing_taskfile/inlined_taskfile_null/Taskfile.yml @@ -0,0 +1,9 @@ +version: '3' + +includes: + included: + +tasks: + task-1: + cmds: + - task: included:default diff --git a/testdata/includes_missing_taskfile/taskfile_empty/Taskfile.yml b/testdata/includes_missing_taskfile/taskfile_empty/Taskfile.yml new file mode 100644 index 0000000000..7805221521 --- /dev/null +++ b/testdata/includes_missing_taskfile/taskfile_empty/Taskfile.yml @@ -0,0 +1,10 @@ +version: '3' + +includes: + included: + taskfile: '' + +tasks: + task-1: + cmds: + - task: included:default diff --git a/testdata/includes_missing_taskfile/taskfile_null/Taskfile.yml b/testdata/includes_missing_taskfile/taskfile_null/Taskfile.yml new file mode 100644 index 0000000000..5c0a933a64 --- /dev/null +++ b/testdata/includes_missing_taskfile/taskfile_null/Taskfile.yml @@ -0,0 +1,10 @@ +version: '3' + +includes: + included: + taskfile: + +tasks: + task-1: + cmds: + - task: included:default From 8c8edfe89a6bf02ad56e1789247cc53c2e912890 Mon Sep 17 00:00:00 2001 From: semihbkgr Date: Thu, 21 Nov 2024 15:00:54 +0300 Subject: [PATCH 2/5] Revert "not allow undeclared or empty taskfile in include" This reverts commit 8c0d6eabdc82114e1a0739e923793f13354373db. --- task_test.go | 29 ------------------- taskfile/reader.go | 4 --- .../inlined_taskfile_empty/Taskfile.yml | 9 ------ .../inlined_taskfile_null/Taskfile.yml | 9 ------ .../taskfile_empty/Taskfile.yml | 10 ------- .../taskfile_null/Taskfile.yml | 10 ------- 6 files changed, 71 deletions(-) delete mode 100644 testdata/includes_missing_taskfile/inlined_taskfile_empty/Taskfile.yml delete mode 100644 testdata/includes_missing_taskfile/inlined_taskfile_null/Taskfile.yml delete mode 100644 testdata/includes_missing_taskfile/taskfile_empty/Taskfile.yml delete mode 100644 testdata/includes_missing_taskfile/taskfile_null/Taskfile.yml diff --git a/task_test.go b/task_test.go index 53e7433521..599277c8c5 100644 --- a/task_test.go +++ b/task_test.go @@ -1527,35 +1527,6 @@ func TestIncludesInterpolation(t *testing.T) { } } -func TestIncludesMissingTaskfile(t *testing.T) { - const dir = "testdata/includes_missing_taskfile" - const error = "no taskfile is specified for include \"included\"" - - tests := []struct { - name string - task string - }{ - {"inlined_taskfile_empty", "empty inline taskfile"}, - {"inlined_taskfile_null", "null inline taskfile"}, - {"taskfile_empty", "empty taskfile"}, - {"taskfile_null", "null taskfile"}, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - var buff bytes.Buffer - e := task.Executor{ - Dir: filepath.Join(dir, test.name), - Stdout: &buff, - Stderr: &buff, - Silent: true, - } - err := e.Setup() - assert.ErrorContains(t, err, error) - }) - } -} - func TestIncludedTaskfileVarMerging(t *testing.T) { const dir = "testdata/included_taskfile_var_merging" tests := []struct { diff --git a/taskfile/reader.go b/taskfile/reader.go index 0d48f400a2..69f75b1d19 100644 --- a/taskfile/reader.go +++ b/taskfile/reader.go @@ -121,10 +121,6 @@ func (r *Reader) include(node Node) error { return err } - if include.Taskfile == "" { - return errors.New(fmt.Sprintf("no taskfile is specified for include %q", include.Namespace)) - } - entrypoint, err := node.ResolveEntrypoint(include.Taskfile) if err != nil { return err diff --git a/testdata/includes_missing_taskfile/inlined_taskfile_empty/Taskfile.yml b/testdata/includes_missing_taskfile/inlined_taskfile_empty/Taskfile.yml deleted file mode 100644 index 58f3516ced..0000000000 --- a/testdata/includes_missing_taskfile/inlined_taskfile_empty/Taskfile.yml +++ /dev/null @@ -1,9 +0,0 @@ -version: '3' - -includes: - included: '' - -tasks: - task-1: - cmds: - - task: included:default diff --git a/testdata/includes_missing_taskfile/inlined_taskfile_null/Taskfile.yml b/testdata/includes_missing_taskfile/inlined_taskfile_null/Taskfile.yml deleted file mode 100644 index d48191c23d..0000000000 --- a/testdata/includes_missing_taskfile/inlined_taskfile_null/Taskfile.yml +++ /dev/null @@ -1,9 +0,0 @@ -version: '3' - -includes: - included: - -tasks: - task-1: - cmds: - - task: included:default diff --git a/testdata/includes_missing_taskfile/taskfile_empty/Taskfile.yml b/testdata/includes_missing_taskfile/taskfile_empty/Taskfile.yml deleted file mode 100644 index 7805221521..0000000000 --- a/testdata/includes_missing_taskfile/taskfile_empty/Taskfile.yml +++ /dev/null @@ -1,10 +0,0 @@ -version: '3' - -includes: - included: - taskfile: '' - -tasks: - task-1: - cmds: - - task: included:default diff --git a/testdata/includes_missing_taskfile/taskfile_null/Taskfile.yml b/testdata/includes_missing_taskfile/taskfile_null/Taskfile.yml deleted file mode 100644 index 5c0a933a64..0000000000 --- a/testdata/includes_missing_taskfile/taskfile_null/Taskfile.yml +++ /dev/null @@ -1,10 +0,0 @@ -version: '3' - -includes: - included: - taskfile: - -tasks: - task-1: - cmds: - - task: included:default From 5f9f163b3cf002ab8cecc711a56704e6af23f709 Mon Sep 17 00:00:00 2001 From: semihbkgr Date: Thu, 21 Nov 2024 20:52:29 +0300 Subject: [PATCH 3/5] validation of include taskfile on decoding --- taskfile/ast/include.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/taskfile/ast/include.go b/taskfile/ast/include.go index 659070de4a..fb8cacb81e 100644 --- a/taskfile/ast/include.go +++ b/taskfile/ast/include.go @@ -29,13 +29,18 @@ type Includes struct { func (includes *Includes) UnmarshalYAML(node *yaml.Node) error { switch node.Kind { case yaml.MappingNode: - // NOTE(@andreynering): on this style of custom unmarshalling, + // NOTE(@andreynering): on this style of custom unmarshaling, // even number contains the keys, while odd numbers contains // the values. for i := 0; i < len(node.Content); i += 2 { keyNode := node.Content[i] valueNode := node.Content[i+1] + // Ensure the include value is not null, as it must be either a string or an include object. + if valueNode.Kind == yaml.ScalarNode && valueNode.Tag == "!!null" { + return errors.NewTaskfileDecodeError(nil, valueNode).WithMessage("value of the include cannot be null") + } + var v Include if err := valueNode.Decode(&v); err != nil { return errors.NewTaskfileDecodeError(err, node) @@ -67,18 +72,20 @@ func (includes *Includes) Range(f func(k string, v *Include) error) error { func (include *Include) UnmarshalYAML(node *yaml.Node) error { switch node.Kind { - case yaml.ScalarNode: var str string if err := node.Decode(&str); err != nil { return errors.NewTaskfileDecodeError(err, node) } + if str == "" { + return errors.NewTaskfileDecodeError(nil, node).WithMessage("taskfile of the include cannot be empty") + } include.Taskfile = str return nil case yaml.MappingNode: var includedTaskfile struct { - Taskfile string + Taskfile *string Dir string Optional bool Internal bool @@ -89,7 +96,13 @@ func (include *Include) UnmarshalYAML(node *yaml.Node) error { if err := node.Decode(&includedTaskfile); err != nil { return errors.NewTaskfileDecodeError(err, node) } - include.Taskfile = includedTaskfile.Taskfile + if includedTaskfile.Taskfile == nil { + return errors.NewTaskfileDecodeError(nil, node).WithMessage("taskfile field in the include cannot be null") + } + if *includedTaskfile.Taskfile == "" { + return errors.NewTaskfileDecodeError(nil, node).WithMessage("taskfile field in the include cannot be empty") + } + include.Taskfile = *includedTaskfile.Taskfile include.Dir = includedTaskfile.Dir include.Optional = includedTaskfile.Optional include.Internal = includedTaskfile.Internal From 755feb6d042254a8cdd8cb0bb67967de35ca0217 Mon Sep 17 00:00:00 2001 From: semihbkgr Date: Thu, 21 Nov 2024 20:53:48 +0300 Subject: [PATCH 4/5] update schema json --- website/static/schema.json | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/website/static/schema.json b/website/static/schema.json index b2b2d875aa..2f605b426b 100644 --- a/website/static/schema.json +++ b/website/static/schema.json @@ -614,14 +614,16 @@ "^.*$": { "anyOf": [ { - "type": "string" + "type": "string", + "minLength": 1 }, { "type": "object", "properties": { "taskfile": { "description": "The path for the Taskfile or directory to be included. If a directory, Task will look for files named `Taskfile.yml` or `Taskfile.yaml` inside that directory. If a relative path, resolved relative to the directory containing the including Taskfile.", - "type": "string" + "type": "string", + "minLength": 1 }, "dir": { "description": "The working directory of the included tasks when run.", @@ -650,7 +652,8 @@ "description": "A set of variables to apply to the included Taskfile.", "$ref": "#/definitions/vars" } - } + }, + "required": ["taskfile"] } ] } From f0f8da994b0e341604c72521487c6d528e41eaef Mon Sep 17 00:00:00 2001 From: semihbkgr Date: Thu, 21 Nov 2024 21:19:46 +0300 Subject: [PATCH 5/5] unit tests for include's taskfile validation --- task_test.go | 29 +++++++++++++++++++ .../include_empty_taskfile/Taskfile.yml | 10 +++++++ .../include_empty_value/Taskfile.yml | 9 ++++++ .../include_missing_taskfile/Taskfile.yml | 10 +++++++ .../include_null_taskfile/Taskfile.yml | 10 +++++++ .../include_null_value/Taskfile.yml | 9 ++++++ 6 files changed, 77 insertions(+) create mode 100644 testdata/includes_invalid_taskfile/include_empty_taskfile/Taskfile.yml create mode 100644 testdata/includes_invalid_taskfile/include_empty_value/Taskfile.yml create mode 100644 testdata/includes_invalid_taskfile/include_missing_taskfile/Taskfile.yml create mode 100644 testdata/includes_invalid_taskfile/include_null_taskfile/Taskfile.yml create mode 100644 testdata/includes_invalid_taskfile/include_null_value/Taskfile.yml diff --git a/task_test.go b/task_test.go index 599277c8c5..0ddf2ea1c6 100644 --- a/task_test.go +++ b/task_test.go @@ -1527,6 +1527,35 @@ func TestIncludesInterpolation(t *testing.T) { } } +func TestIncludesInvalidTaskfile(t *testing.T) { + const dir = "testdata/includes_invalid_taskfile" + + tests := []struct { + name string + expectedErr string + }{ + {"include_empty_taskfile", "taskfile field in the include cannot be empty"}, + {"include_empty_value", "taskfile of the include cannot be empty"}, + {"include_missing_taskfile", "taskfile field in the include cannot be null"}, + {"include_null_taskfile", "taskfile field in the include cannot be null"}, + {"include_null_value", "value of the include cannot be null"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var buff bytes.Buffer + e := task.Executor{ + Dir: filepath.Join(dir, test.name), + Stdout: &buff, + Stderr: &buff, + Silent: true, + } + err := e.Setup() + assert.ErrorContains(t, err, test.expectedErr) + }) + } +} + func TestIncludedTaskfileVarMerging(t *testing.T) { const dir = "testdata/included_taskfile_var_merging" tests := []struct { diff --git a/testdata/includes_invalid_taskfile/include_empty_taskfile/Taskfile.yml b/testdata/includes_invalid_taskfile/include_empty_taskfile/Taskfile.yml new file mode 100644 index 0000000000..a4d25a185c --- /dev/null +++ b/testdata/includes_invalid_taskfile/include_empty_taskfile/Taskfile.yml @@ -0,0 +1,10 @@ +version: '3' + +includes: + included: + taskfile: '' + +tasks: + default: + cmds: + - task: included:default diff --git a/testdata/includes_invalid_taskfile/include_empty_value/Taskfile.yml b/testdata/includes_invalid_taskfile/include_empty_value/Taskfile.yml new file mode 100644 index 0000000000..1f9fac9d8a --- /dev/null +++ b/testdata/includes_invalid_taskfile/include_empty_value/Taskfile.yml @@ -0,0 +1,9 @@ +version: '3' + +includes: + included: '' + +tasks: + default: + cmds: + - task: included:default diff --git a/testdata/includes_invalid_taskfile/include_missing_taskfile/Taskfile.yml b/testdata/includes_invalid_taskfile/include_missing_taskfile/Taskfile.yml new file mode 100644 index 0000000000..ddb86f0229 --- /dev/null +++ b/testdata/includes_invalid_taskfile/include_missing_taskfile/Taskfile.yml @@ -0,0 +1,10 @@ +version: '3' + +includes: + included: + dir: '.' + +tasks: + default: + cmds: + - task: included:default diff --git a/testdata/includes_invalid_taskfile/include_null_taskfile/Taskfile.yml b/testdata/includes_invalid_taskfile/include_null_taskfile/Taskfile.yml new file mode 100644 index 0000000000..d8d32fd855 --- /dev/null +++ b/testdata/includes_invalid_taskfile/include_null_taskfile/Taskfile.yml @@ -0,0 +1,10 @@ +version: '3' + +includes: + included: + taskfile: null + +tasks: + default: + cmds: + - task: included:default diff --git a/testdata/includes_invalid_taskfile/include_null_value/Taskfile.yml b/testdata/includes_invalid_taskfile/include_null_value/Taskfile.yml new file mode 100644 index 0000000000..b5344e6328 --- /dev/null +++ b/testdata/includes_invalid_taskfile/include_null_value/Taskfile.yml @@ -0,0 +1,9 @@ +version: '3' + +includes: + included: null + +tasks: + default: + cmds: + - task: included:default