Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce validation for taskfile field in includes #1902

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
33 changes: 33 additions & 0 deletions task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1706,6 +1706,39 @@ func TestIncludesInterpolation(t *testing.T) { // nolint:paralleltest // cannot
}
}

func TestIncludesInvalidTaskfile(t *testing.T) {
t.Parallel()

const dir = "testdata/includes_invalid_taskfile"

tests := []struct {
name string
expectedErr string
}{
{"include_empty_taskfile", "taskfile field in includes cannot be empty"},
{"include_empty_value", "inline taskfile value in includes cannot be empty"},
{"include_missing_taskfile", "taskfile field in includes cannot be null"},
{"include_null_taskfile", "taskfile field in includes cannot be null"},
{"include_null_value", "inline taskfile value in includes cannot be null"},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()

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 TestIncludesWithExclude(t *testing.T) {
t.Parallel()

Expand Down
19 changes: 16 additions & 3 deletions taskfile/ast/include.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ func (includes *Includes) UnmarshalYAML(node *yaml.Node) error {
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" {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The null check is placed before decoding the valueNode because the Decode method from the yaml package does not invoke the UnmarshalYAML method if the node is null. This ensures that null values are caught early and handled with a proper error message, as the UnmarshalYAML logic would be skipped for such nodes.

return errors.NewTaskfileDecodeError(nil, valueNode).WithMessage("inline taskfile value in includes cannot be null")
}

// Decode the value node into an Include struct
var v Include
if err := valueNode.Decode(&v); err != nil {
Expand All @@ -133,18 +138,20 @@ func (includes *Includes) UnmarshalYAML(node *yaml.Node) 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("inline taskfile value in includes cannot be empty")
}
include.Taskfile = str
return nil

case yaml.MappingNode:
var includedTaskfile struct {
Taskfile string
Taskfile *string
Dir string
Optional bool
Internal bool
Expand All @@ -156,7 +163,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 includes cannot be null")
}
if *includedTaskfile.Taskfile == "" {
return errors.NewTaskfileDecodeError(nil, node).WithMessage("taskfile field in includes cannot be empty")
}
include.Taskfile = *includedTaskfile.Taskfile
include.Dir = includedTaskfile.Dir
include.Optional = includedTaskfile.Optional
include.Internal = includedTaskfile.Internal
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
version: '3'

includes:
included:
taskfile: ''

tasks:
default:
cmds:
- task: included:default
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
version: '3'

includes:
included: ''

tasks:
default:
cmds:
- task: included:default
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
version: '3'

includes:
included:
dir: '.'

tasks:
default:
cmds:
- task: included:default
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
version: '3'

includes:
included:
taskfile: null

tasks:
default:
cmds:
- task: included:default
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
version: '3'

includes:
included: null

tasks:
default:
cmds:
- task: included:default
9 changes: 6 additions & 3 deletions website/static/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -621,14 +621,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.",
Expand Down Expand Up @@ -664,7 +666,8 @@
"description": "A set of variables to apply to the included Taskfile.",
"$ref": "#/definitions/vars"
}
}
},
"required": ["taskfile"]
}
]
}
Expand Down
Loading