From 3daaf92d4c38bdaa4a04b9c1131008e4b39ebcba Mon Sep 17 00:00:00 2001 From: Pete Davison Date: Mon, 28 Apr 2025 16:31:12 +0000 Subject: [PATCH] feat: checksum pinning --- errors/errors.go | 1 + errors/errors_taskfile.go | 21 ++++++++ executor_test.go | 20 +++++++ taskfile/ast/include.go | 5 ++ taskfile/node.go | 2 + taskfile/node_base.go | 19 ++++++- taskfile/reader.go | 53 +++++++++++++++---- .../includes_checksum/correct/Taskfile.yml | 12 +++++ .../TestIncludeChecksum-correct.golden | 2 + .../correct_remote/Taskfile.yml | 12 +++++ testdata/includes_checksum/included.yml | 6 +++ .../includes_checksum/incorrect/Taskfile.yml | 12 +++++ ...IncludeChecksum-incorrect-err-setup.golden | 3 ++ .../TestIncludeChecksum-incorrect.golden | 0 website/docs/experiments/remote_taskfiles.mdx | 38 ++++++++++++- website/docs/reference/schema.mdx | 1 + website/static/schema.json | 4 ++ 17 files changed, 196 insertions(+), 15 deletions(-) create mode 100644 testdata/includes_checksum/correct/Taskfile.yml create mode 100644 testdata/includes_checksum/correct/testdata/TestIncludeChecksum-correct.golden create mode 100644 testdata/includes_checksum/correct_remote/Taskfile.yml create mode 100644 testdata/includes_checksum/included.yml create mode 100644 testdata/includes_checksum/incorrect/Taskfile.yml create mode 100644 testdata/includes_checksum/incorrect/testdata/TestIncludeChecksum-incorrect-err-setup.golden create mode 100644 testdata/includes_checksum/incorrect/testdata/TestIncludeChecksum-incorrect.golden diff --git a/errors/errors.go b/errors/errors.go index ea3d7ce024..1ed50e8741 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -26,6 +26,7 @@ const ( CodeTaskfileNetworkTimeout CodeTaskfileInvalid CodeTaskfileCycle + CodeTaskfileDoesNotMatchChecksum ) // Task related exit codes diff --git a/errors/errors_taskfile.go b/errors/errors_taskfile.go index cbb160aebe..a2f0bad790 100644 --- a/errors/errors_taskfile.go +++ b/errors/errors_taskfile.go @@ -187,3 +187,24 @@ func (err TaskfileCycleError) Error() string { func (err TaskfileCycleError) Code() int { return CodeTaskfileCycle } + +// TaskfileDoesNotMatchChecksum is returned when a Taskfile's checksum does not +// match the one pinned in the parent Taskfile. +type TaskfileDoesNotMatchChecksum struct { + URI string + ExpectedChecksum string + ActualChecksum string +} + +func (err *TaskfileDoesNotMatchChecksum) Error() string { + return fmt.Sprintf( + "task: The checksum of the Taskfile at %q does not match!\ngot: %q\nwant: %q", + err.URI, + err.ActualChecksum, + err.ExpectedChecksum, + ) +} + +func (err *TaskfileDoesNotMatchChecksum) Code() int { + return CodeTaskfileDoesNotMatchChecksum +} diff --git a/executor_test.go b/executor_test.go index 8f3c794c9d..4d1677db21 100644 --- a/executor_test.go +++ b/executor_test.go @@ -958,3 +958,23 @@ func TestFuzzyModel(t *testing.T) { WithTask("install"), ) } + +func TestIncludeChecksum(t *testing.T) { + t.Parallel() + + NewExecutorTest(t, + WithName("correct"), + WithExecutorOptions( + task.WithDir("testdata/includes_checksum/correct"), + ), + ) + + NewExecutorTest(t, + WithName("incorrect"), + WithExecutorOptions( + task.WithDir("testdata/includes_checksum/incorrect"), + ), + WithSetupError(), + WithPostProcessFn(PPRemoveAbsolutePaths), + ) +} diff --git a/taskfile/ast/include.go b/taskfile/ast/include.go index 894fd23d1d..6902b5a61d 100644 --- a/taskfile/ast/include.go +++ b/taskfile/ast/include.go @@ -24,6 +24,7 @@ type ( AdvancedImport bool Vars *Vars Flatten bool + Checksum string } // Includes is an ordered map of namespaces to includes. Includes struct { @@ -165,6 +166,7 @@ func (include *Include) UnmarshalYAML(node *yaml.Node) error { Aliases []string Excludes []string Vars *Vars + Checksum string } if err := node.Decode(&includedTaskfile); err != nil { return errors.NewTaskfileDecodeError(err, node) @@ -178,6 +180,7 @@ func (include *Include) UnmarshalYAML(node *yaml.Node) error { include.AdvancedImport = true include.Vars = includedTaskfile.Vars include.Flatten = includedTaskfile.Flatten + include.Checksum = includedTaskfile.Checksum return nil } @@ -200,5 +203,7 @@ func (include *Include) DeepCopy() *Include { AdvancedImport: include.AdvancedImport, Vars: include.Vars.DeepCopy(), Flatten: include.Flatten, + Aliases: deepcopy.Slice(include.Aliases), + Checksum: include.Checksum, } } diff --git a/taskfile/node.go b/taskfile/node.go index 357ba1a522..3dc3344c6c 100644 --- a/taskfile/node.go +++ b/taskfile/node.go @@ -17,6 +17,8 @@ type Node interface { Parent() Node Location() string Dir() string + Checksum() string + Verify(checksum string) bool ResolveEntrypoint(entrypoint string) (string, error) ResolveDir(dir string) (string, error) } diff --git a/taskfile/node_base.go b/taskfile/node_base.go index c712de6c46..7e641efdba 100644 --- a/taskfile/node_base.go +++ b/taskfile/node_base.go @@ -7,8 +7,9 @@ type ( // designed to be embedded in other node types so that this boilerplate code // does not need to be repeated. baseNode struct { - parent Node - dir string + parent Node + dir string + checksum string } ) @@ -32,6 +33,12 @@ func WithParent(parent Node) NodeOption { } } +func WithChecksum(checksum string) NodeOption { + return func(node *baseNode) { + node.checksum = checksum + } +} + func (node *baseNode) Parent() Node { return node.parent } @@ -39,3 +46,11 @@ func (node *baseNode) Parent() Node { func (node *baseNode) Dir() string { return node.dir } + +func (node *baseNode) Checksum() string { + return node.checksum +} + +func (node *baseNode) Verify(checksum string) bool { + return node.checksum == "" || node.checksum == checksum +} diff --git a/taskfile/reader.go b/taskfile/reader.go index 3230807639..3f36ad62b2 100644 --- a/taskfile/reader.go +++ b/taskfile/reader.go @@ -250,6 +250,7 @@ func (r *Reader) include(ctx context.Context, node Node) error { AdvancedImport: include.AdvancedImport, Excludes: include.Excludes, Vars: include.Vars, + Checksum: include.Checksum, } if err := cache.Err(); err != nil { return err @@ -267,6 +268,7 @@ func (r *Reader) include(ctx context.Context, node Node) error { includeNode, err := NewNode(entrypoint, include.Dir, r.insecure, WithParent(node), + WithChecksum(include.Checksum), ) if err != nil { if include.Optional { @@ -362,7 +364,24 @@ func (r *Reader) readNodeContent(ctx context.Context, node Node) ([]byte, error) if node, isRemote := node.(RemoteNode); isRemote { return r.readRemoteNodeContent(ctx, node) } - return node.Read() + + // Read the Taskfile + b, err := node.Read() + if err != nil { + return nil, err + } + + // If the given checksum doesn't match the sum pinned in the Taskfile + checksum := checksum(b) + if !node.Verify(checksum) { + return nil, &errors.TaskfileDoesNotMatchChecksum{ + URI: node.Location(), + ExpectedChecksum: node.Checksum(), + ActualChecksum: checksum, + } + } + + return b, nil } func (r *Reader) readRemoteNodeContent(ctx context.Context, node RemoteNode) ([]byte, error) { @@ -427,17 +446,29 @@ func (r *Reader) readRemoteNodeContent(ctx context.Context, node RemoteNode) ([] } r.debugf("found remote file at %q\n", node.Location()) + + // If the given checksum doesn't match the sum pinned in the Taskfile checksum := checksum(downloadedBytes) - prompt := cache.ChecksumPrompt(checksum) - - // Prompt the user if required - if prompt != "" { - if err := func() error { - r.promptMutex.Lock() - defer r.promptMutex.Unlock() - return r.promptf(prompt, node.Location()) - }(); err != nil { - return nil, &errors.TaskfileNotTrustedError{URI: node.Location()} + if !node.Verify(checksum) { + return nil, &errors.TaskfileDoesNotMatchChecksum{ + URI: node.Location(), + ExpectedChecksum: node.Checksum(), + ActualChecksum: checksum, + } + } + + // If there is no manual checksum pin, run the automatic checks + if node.Checksum() == "" { + // Prompt the user if required + prompt := cache.ChecksumPrompt(checksum) + if prompt != "" { + if err := func() error { + r.promptMutex.Lock() + defer r.promptMutex.Unlock() + return r.promptf(prompt, node.Location()) + }(); err != nil { + return nil, &errors.TaskfileNotTrustedError{URI: node.Location()} + } } } diff --git a/testdata/includes_checksum/correct/Taskfile.yml b/testdata/includes_checksum/correct/Taskfile.yml new file mode 100644 index 0000000000..ef1121dec1 --- /dev/null +++ b/testdata/includes_checksum/correct/Taskfile.yml @@ -0,0 +1,12 @@ +version: '3' + +includes: + included: + taskfile: ../included.yml + internal: true + checksum: c97f39eb96fe3fa5fe2a610d244b8449897b06f0c93821484af02e0999781bf5 + +tasks: + default: + cmds: + - task: included:default diff --git a/testdata/includes_checksum/correct/testdata/TestIncludeChecksum-correct.golden b/testdata/includes_checksum/correct/testdata/TestIncludeChecksum-correct.golden new file mode 100644 index 0000000000..a2d7af2d34 --- /dev/null +++ b/testdata/includes_checksum/correct/testdata/TestIncludeChecksum-correct.golden @@ -0,0 +1,2 @@ +task: [included:default] echo "Hello, World!" +Hello, World! diff --git a/testdata/includes_checksum/correct_remote/Taskfile.yml b/testdata/includes_checksum/correct_remote/Taskfile.yml new file mode 100644 index 0000000000..34a5cda654 --- /dev/null +++ b/testdata/includes_checksum/correct_remote/Taskfile.yml @@ -0,0 +1,12 @@ +version: '3' + +includes: + included: + taskfile: https://taskfile.dev + internal: true + checksum: c153e97e0b3a998a7ed2e61064c6ddaddd0de0c525feefd6bba8569827d8efe9 + +tasks: + default: + cmds: + - task: included:default diff --git a/testdata/includes_checksum/included.yml b/testdata/includes_checksum/included.yml new file mode 100644 index 0000000000..f97528ea21 --- /dev/null +++ b/testdata/includes_checksum/included.yml @@ -0,0 +1,6 @@ +version: '3' + +tasks: + default: + cmds: + - echo "Hello, World!" diff --git a/testdata/includes_checksum/incorrect/Taskfile.yml b/testdata/includes_checksum/incorrect/Taskfile.yml new file mode 100644 index 0000000000..56c13cca10 --- /dev/null +++ b/testdata/includes_checksum/incorrect/Taskfile.yml @@ -0,0 +1,12 @@ +version: '3' + +includes: + included: + taskfile: ../included.yml + internal: true + checksum: foo + +tasks: + default: + cmds: + - task: included:default diff --git a/testdata/includes_checksum/incorrect/testdata/TestIncludeChecksum-incorrect-err-setup.golden b/testdata/includes_checksum/incorrect/testdata/TestIncludeChecksum-incorrect-err-setup.golden new file mode 100644 index 0000000000..58dedf8795 --- /dev/null +++ b/testdata/includes_checksum/incorrect/testdata/TestIncludeChecksum-incorrect-err-setup.golden @@ -0,0 +1,3 @@ +task: The checksum of the Taskfile at "/testdata/includes_checksum/included.yml" does not match! +got: "c97f39eb96fe3fa5fe2a610d244b8449897b06f0c93821484af02e0999781bf5" +want: "foo" \ No newline at end of file diff --git a/testdata/includes_checksum/incorrect/testdata/TestIncludeChecksum-incorrect.golden b/testdata/includes_checksum/incorrect/testdata/TestIncludeChecksum-incorrect.golden new file mode 100644 index 0000000000..e69de29bb2 diff --git a/website/docs/experiments/remote_taskfiles.mdx b/website/docs/experiments/remote_taskfiles.mdx index b9c2d1f2cb..b815422462 100644 --- a/website/docs/experiments/remote_taskfiles.mdx +++ b/website/docs/experiments/remote_taskfiles.mdx @@ -182,9 +182,11 @@ includes: ## Security +### Automatic checksums + Running commands from sources that you do not control is always a potential -security risk. For this reason, we have added some checks when using remote -Taskfiles: +security risk. For this reason, we have added some automatic checks when using +remote Taskfiles: 1. When running a task from a remote Taskfile for the first time, Task will print a warning to the console asking you to check that you are sure that you @@ -209,6 +211,38 @@ flag. Before enabling this flag, you should: containing a commit hash) to prevent Task from automatically accepting a prompt that says a remote Taskfile has changed. +### Manual checksum pinning + +Alternatively, if you expect the contents of your remote files to be a constant +value, you can pin the checksum of the included file instead: + +```yaml +version: '3' + +includes: + included: + taskfile: https://taskfile.dev + checksum: c153e97e0b3a998a7ed2e61064c6ddaddd0de0c525feefd6bba8569827d8efe9 +``` + +This will disable the automatic checksum prompts discussed above. However, if +the checksums do not match, Task will exit immediately with an error. When +setting this up for the first time, you may not know the correct value of the +checksum. There are a couple of ways you can obtain this: + +1. Add the include normally without the `checksum` key. The first time you run + the included Taskfile, a `.task/remote` temporary directory is created. Find + the correct set of files for your included Taskfile and open the file that + ends with `.checksum`. You can copy the contents of this file and paste it + into the `checksum` key of your include. This method is safest as it allows + you to inspect the downloaded Taskfile before you pin it. +2. Alternatively, add the include with a temporary random value in the + `checksum` key. When you try to run the Taskfile, you will get an error that + will report the incorrect expected checksum and the actual checksum. You can + copy the actual checksum and replace your temporary random value. + +### TLS + Task currently supports both `http` and `https` URLs. However, the `http` requests will not execute by default unless you run the task with the `--insecure` flag. This is to protect you from accidentally running a remote diff --git a/website/docs/reference/schema.mdx b/website/docs/reference/schema.mdx index f426e3faab..8339f5100e 100644 --- a/website/docs/reference/schema.mdx +++ b/website/docs/reference/schema.mdx @@ -34,6 +34,7 @@ toc_max_heading_level: 5 | `internal` | `bool` | `false` | Stops any task in the included Taskfile from being callable on the command line. These commands will also be omitted from the output when used with `--list`. | | `aliases` | `[]string` | | Alternative names for the namespace of the included Taskfile. | | `vars` | `map[string]Variable` | | A set of variables to apply to the included Taskfile. | +| `checksum` | `string` | | The checksum of the file you expect to include. If the checksum does not match, the file will not be included. | :::info diff --git a/website/static/schema.json b/website/static/schema.json index 0a942697d6..0a229bedc9 100644 --- a/website/static/schema.json +++ b/website/static/schema.json @@ -684,6 +684,10 @@ "vars": { "description": "A set of variables to apply to the included Taskfile.", "$ref": "#/definitions/vars" + }, + "checksum": { + "description": "The checksum of the file you expect to include. If the checksum does not match, the file will not be included.", + "type": "string" } } }