[Mac] [3.45.5] Task panics if user places 'pipefail' into `shopt` by mistake (without `-o` mode turned on in shopts)

[posidoni]

Description

Hello! First of all, I would like to say thank you for your work, I have 'sold' task to my teammates in multiple companies for go projects instead of make. This is my first issue for public OSS projet

Context: if I include pipefail in shopt - task panics without clear error message.

this code panics for me:

set:
  - pipefail
  - nounset
  - errexit
shopt:
  - pipefail

and this one

# set: <- may be empty.
shopt:
  - pipefail

It is clear here the problem is invalid pipefail that is in shopt, however it has to be in set.

task: [default] echo 'I will not work'
shopt: invalid option name "abcd"
task: Failed to run task "default": exit status 1

This issue happens only with pipefail and not with other invalid non existing options.

Issue: task panics with weird 'panic: runtime error: index out of range [-1]'.

Expected: give user more clear and friendly information of what happened. Maybe add helpful message that tells move pipefail into set and explanation of possible options.

Or give user this message: shopt: invalid option name "pipefail"

Something bad happens here: https://github.com/mvdan/sh/blob/b717ad599e17a6975e15a67ed8937bd8d4909146/interp/builtin.go#L793

I suppose, the issue is with mvdan/sh library, but anyway I think it should be handled in this project.

Also, could you please explain, why don't you use recovery while running cmd that potentially panics? I have seen panics from sh interpreter in multiple versions, however now I have only this issue.

In recovery handler you could, for example, capture error message from CMD, but if you want stacktraces, you could manually add them to recovery logs.

Full panic stacktraces are below.

 > task                                                                                                                                                                               
task: [default] nginx -p "/Users/posidoni/powx/cicd/nginx" -c ./nginx.conf -T
panic: runtime error: index out of range [-1]

goroutine 1 gp=0x140000021c0 m=4 mp=0x14000100008 [running]:
panic({0x1030acd60?, 0x1400003a588?})
        /opt/homebrew/opt/go/libexec/src/runtime/panic.go:802 +0x150 fp=0x14000593a50 sp=0x140005939a0 pc=0x1026384b0
runtime.goPanicIndex(0xffffffffffffffff, 0x36)
        /opt/homebrew/opt/go/libexec/src/runtime/panic.go:115 +0x70 fp=0x14000593a90 sp=0x14000593a50 pc=0x102600910
mvdan.cc/sh/v3/interp.(*Runner).builtin(0x140002c38c8, {0x103107db0, 0x1400017c780}, {0x0, 0x4001}, {0x140004964c8, 0x5}, {0x140003dc410, 0x2, 0x3})
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/builtin.go:729 +0x4d70 fp=0x14000594bb0 sp=0x14000593a90 pc=0x1027e29e0
mvdan.cc/sh/v3/interp.(*Runner).call(0x140002c38c8, {0x103107db0, 0x1400017c780}, {0x3dc410?, 0x140?}, {0x140003dc400, 0x3, 0x4})
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/runner.go:1031 +0x5a4 fp=0x14000594ca0 sp=0x14000594bb0 pc=0x1027ed414
mvdan.cc/sh/v3/interp.(*Runner).cmd(0x140002c38c8, {0x103107db0, 0x1400017c780}, {0x103106b90, 0x1400045aaa0})
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/runner.go:458 +0x1c5c fp=0x140005951f0 sp=0x14000594ca0 pc=0x1027eabcc
mvdan.cc/sh/v3/interp.(*Runner).stmtSync(0x140002c38c8, {0x103107db0, 0x1400017c780}, 0x140003862a0)
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/runner.go:340 +0x158 fp=0x14000595290 sp=0x140005951f0 pc=0x1027e8dc8
mvdan.cc/sh/v3/interp.(*Runner).stmt(0x140002c38c8, {0x103107db0, 0x1400017c780}, 0x140003862a0)
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/runner.go:322 +0x22c fp=0x14000595310 sp=0x14000595290 pc=0x1027e8b5c
mvdan.cc/sh/v3/interp.(*Runner).stmts(...)
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/runner.go:829
mvdan.cc/sh/v3/interp.(*Runner).Run(0x140002c38c8, {0x103107db0, 0x1400017c780}, {0x1031043f8, 0x140003dc340})
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/api.go:901 +0x32c fp=0x140005953b0 sp=0x14000595310 pc=0x1027dd29c
github.com/go-task/task/v3/internal/execext.RunCommand({0x103107db0, 0x1400017c780}, 0x140005957b8)
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/internal/execext/exec.go:80 +0x5a4 fp=0x140005955b0 sp=0x140005953b0 pc=0x102989024
github.com/go-task/task/v3.(*Executor).runCommand(0x140001ca420, {0x103107db0, 0x1400017c780}, 0x14000372248, 0x140002def00?, 0x0?)
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/task.go:345 +0x578 fp=0x14000595860 sp=0x140005955b0 pc=0x102bcbaa8
github.com/go-task/task/v3.(*Executor).RunTask.func1({0x103107db0, 0x1400017c780})
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/task.go:217 +0x680 fp=0x14000595a20 sp=0x14000595860 pc=0x102bca770
github.com/go-task/task/v3.(*Executor).startExecution(0x140001ca420, {0x103107db0, 0x1400017c780}, 0x1025dc2bc?, 0x14000595b80)
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/task.go:376 +0x348 fp=0x14000595af0 sp=0x14000595a20 pc=0x102bcc1e8
github.com/go-task/task/v3.(*Executor).RunTask(0x140001ca420, {0x103107db0, 0x1400017c780}, 0x140002def00)
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/task.go:153 +0x188 fp=0x14000595bb0 sp=0x14000595af0 pc=0x102bc9ee8
github.com/go-task/task/v3.(*Executor).Run(0x140001ca420, {0x103107d40, 0x1036600a0}, {0x140003c5d88, 0x1, 0x4})

❯❯3_  ❯❯3… /powx/cicd/nginx  21:14   /main  +/- [⚡IOT]    
 > task -v                                                                                                                                                                            
task: "default" started
task: [default] nginx -p "/Users/posidoni/powx/cicd/nginx" -c ./nginx.conf -T
panic: runtime error: index out of range [-1]

goroutine 1 gp=0x140000021c0 m=10 mp=0x14000386008 [running]:
panic({0x101100d60?, 0x1400051a150?})
        /opt/homebrew/opt/go/libexec/src/runtime/panic.go:802 +0x150 fp=0x14000413a50 sp=0x140004139a0 pc=0x10068c4b0
runtime.goPanicIndex(0xffffffffffffffff, 0x36)
        /opt/homebrew/opt/go/libexec/src/runtime/panic.go:115 +0x70 fp=0x14000413a90 sp=0x14000413a50 pc=0x100654910
mvdan.cc/sh/v3/interp.(*Runner).builtin(0x14000295b88, {0x10115bdb0, 0x1400017c780}, {0x0, 0x4001}, {0x140004bc620, 0x5}, {0x140003d2550, 0x2, 0x3})
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/builtin.go:729 +0x4d70 fp=0x14000414bb0 sp=0x14000413a90 pc=0x1008369e0
mvdan.cc/sh/v3/interp.(*Runner).call(0x14000295b88, {0x10115bdb0, 0x1400017c780}, {0x3d2550?, 0x140?}, {0x140003d2540, 0x3, 0x4})
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/runner.go:1031 +0x5a4 fp=0x14000414ca0 sp=0x14000414bb0 pc=0x100841414
mvdan.cc/sh/v3/interp.(*Runner).cmd(0x14000295b88, {0x10115bdb0, 0x1400017c780}, {0x10115ab90, 0x14000312870})
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/runner.go:458 +0x1c5c fp=0x140004151f0 sp=0x14000414ca0 pc=0x10083ebcc
mvdan.cc/sh/v3/interp.(*Runner).stmtSync(0x14000295b88, {0x10115bdb0, 0x1400017c780}, 0x14000280180)
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/runner.go:340 +0x158 fp=0x14000415290 sp=0x140004151f0 pc=0x10083cdc8
mvdan.cc/sh/v3/interp.(*Runner).stmt(0x14000295b88, {0x10115bdb0, 0x1400017c780}, 0x14000280180)
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/runner.go:322 +0x22c fp=0x14000415310 sp=0x14000415290 pc=0x10083cb5c
mvdan.cc/sh/v3/interp.(*Runner).stmts(...)
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/runner.go:829
mvdan.cc/sh/v3/interp.(*Runner).Run(0x14000295b88, {0x10115bdb0, 0x1400017c780}, {0x1011583f8, 0x140003d24c0})
        /Users/posidoni/.cache/go/modcache/mvdan.cc/sh/v3@v3.12.0/interp/api.go:901 +0x32c fp=0x140004153b0 sp=0x14000415310 pc=0x10083129c
github.com/go-task/task/v3/internal/execext.RunCommand({0x10115bdb0, 0x1400017c780}, 0x140004157b8)
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/internal/execext/exec.go:80 +0x5a4 fp=0x140004155b0 sp=0x140004153b0 pc=0x1009dd024
github.com/go-task/task/v3.(*Executor).runCommand(0x140001ca420, {0x10115bdb0, 0x1400017c780}, 0x14000332248, 0x140002b4f40?, 0x0?)
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/task.go:345 +0x578 fp=0x14000415860 sp=0x140004155b0 pc=0x100c1faa8
github.com/go-task/task/v3.(*Executor).RunTask.func1({0x10115bdb0, 0x1400017c780})
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/task.go:217 +0x680 fp=0x14000415a20 sp=0x14000415860 pc=0x100c1e770
github.com/go-task/task/v3.(*Executor).startExecution(0x140001ca420, {0x10115bdb0, 0x1400017c780}, 0x1006302bc?, 0x14000415b80)
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/task.go:376 +0x348 fp=0x14000415af0 sp=0x14000415a20 pc=0x100c201e8
github.com/go-task/task/v3.(*Executor).RunTask(0x140001ca420, {0x10115bdb0, 0x1400017c780}, 0x140002b4f40)
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/task.go:153 +0x188 fp=0x14000415bb0 sp=0x14000415af0 pc=0x100c1dee8
github.com/go-task/task/v3.(*Executor).Run(0x140001ca420, {0x10115bd40, 0x1016b40a0}, {0x14000325d88, 0x1, 0x4})
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/task.go:87 +0x314 fp=0x14000415c70 sp=0x14000415bb0 pc=0x100c1d9f4
main.run()
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/cmd/task/task.go:181 +0x740 fp=0x14000415e90 sp=0x14000415c70 pc=0x100c2cb30
main.main()
        /Users/posidoni/.cache/go/modcache/github.com/go-task/task/v3@v3.45.4/cmd/task/task.go:23 +0x20 fp=0x14000415f40 sp=0x14000415e90 pc=0x100c2c110
runtime.main()
        /opt/homebrew/opt/go/libexec/src/runtime/proc.go:285 +0x278 fp=0x14000415fd0 sp=0x14000415f40 pc=0x100659918
runtime.goexit({})

Version

3.45.4

Operating system

mac

Experiments Enabled

No response

Example Taskfile

---
version: '3.40'

set:
  - e
  - u
shopt:
  - pipefail ### INFO: issue is here

tasks:
    default:
      cmds:
        - echo 123

[posidoni]

I'm also aware the user is at fault here. but this is inconsistent with other invalid shopts.

If this issue is already documented or I did not see some docs, well, I apologize, but there is quite a lot of things to process.

also combinations below crash (I think, it does not depend on min version in taskfile, but depennds on binary version installed):

shopt:
  - globstar
  - pipefail

I found crash only with pipefail, but maybe different opts also panic inside mvdan/sh library.

192:nginx posidoni$ task
task: [default] nginx -p "/Users/posidoni/powx/cicd/nginx" -c ./nginx.conf -T
shopt: invalid option name "errorexit"

[posidoni]

mvdan/sh library returns some regular errors as panics, so this is why I mentioned adding recovery. Consider line below:

				panic(fmt.Sprintf("unhandled shopt flag: %s", flag))

generally, I don't understand how it differs from other errors and why panic is needed.

it happens, for example, if we add -p to shopt. This panic is not handled by client:

 > task
task: [default] nginx -p "/Users/posidoni/powx/cicd/nginx" -c ./nginx.conf -T
panic: unhandled shopt flag: -p

also, this method is really clever, it is worth checking out / refactoring. IDK, should I also open this one for mvdan/sh project.

func (r *Runner) builtin(ctx context.Context, pos syntax.Pos, name string, args []string) (exit exitStatus) {
	failf := func(code uint8, format string, args ...any) exitStatus {
		r.errf(format, args...)
		exit.code = code
		return exit
	}
        ....
}

If possible, I would be happy to contribute with code by opening PR. We should just discuss:

• should we fix it
• shoold we fix it either by:
  • adding recovery
  • adding shopt validation
  • both or some other way.

[pd93]

It may be worth checking with @mvdan if this is intended behavior. However, regardless of this, we can probably add some validation on our side to stop sending invalid options and improve error messages. I'm generally against using recover when possible.

[posidoni]

Alright, sounds fair.

If we prefer validation during parsing, it is worth checking out are there any exported enums / consts from mvdan/sh project. Or at least some kind of validation.

Also, by researching code I have found that:

shopt:
  - '-o'
  - 'pipefail'
shopt:
  - 'pipefail'
  - '-o'

works fine, because it turns out there are different modes for shopt.

so, pipefail is completely valid here, but requires special -o flag that we could search for / ask user to provide it if he wants to use there features.

By the way, like that it prints all these options:

shopt:
	- '-o' # executes `shopt -o` literally in shell

output:

errexit on
noexec  off
nounset off
pipefail        off
xtrace  off
noglob  off
allexport       off 

These flags don't conflict with set features generally. We just have to provide proper validation and not panic.

Also, it is worth updating the doc, as it doesn't mention them.

Maybe we could include some tips, e.g.

# You may consider using `xtrace` for detailed line by line logging of long scripts.

 > task
task: [default] ./long.sh
+ ./long.sh
1 i am really long shell script
2 i am really long shell script
3 i am really long shell script

and update json schema as well.

bash options may be printed like that

shopt:
    - ' '

task: [default] ./long.sh

expand_aliases  off
globstar        off
nocaseglob      off
nullglob        off
. others not supported.

UPD:

found one more crash (segfault).

set:
    - ''

 > task -v
task: "default" started
task: [default] ./long.sh
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x18 pc=0x100d613d8]

OK. To conclude:

1. This is not mvdan/sh bug, but intended behaviour.
2. Panics are expected there (sigsegv is not, but here it is clearly invalid input).
3. In my opinion, we should just add more validation to flags + update json schema in order to avoid passing invalid things to sh lib.

[mvdan]

While the interp package does have some panic calls which might need to be tweaked, this panic was simply a bug; I've fixed it in mvdan/sh@6bd6eb6.

[posidoni]

Awesome, thanks for such a quick fix! @mvdan

Currently don't have time re-test, because the build fails.
I will debug it and tell the status soon.

I think, there are still things to improve in terms of UX of task cli, however it will be more of a feature suggestion than bug.

> GOBIN=task go install github.com/go-task/task/cmd/task@latest

go: finding module for package github.com/Masterminds/semver
go: finding module for package mvdan.cc/sh/syntax
go: finding module for package github.com/radovskyb/watcher
go: finding module for package mvdan.cc/sh/shell
go: finding module for package mvdan.cc/sh/interp
go: finding module for package golang.org/x/sync/errgroup
go: finding module for package github.com/mattn/go-zglob
go: finding module for package github.com/spf13/pflag
go: finding module for package github.com/Masterminds/sprig
go: finding module for package gopkg.in/yaml.v2
go: found github.com/spf13/pflag in github.com/spf13/pflag v1.0.10
go: found github.com/Masterminds/semver in github.com/Masterminds/semver v1.5.0
go: found github.com/mattn/go-zglob in github.com/mattn/go-zglob v0.0.6
go: found github.com/radovskyb/watcher in github.com/radovskyb/watcher v1.0.7
go: found golang.org/x/sync/errgroup in golang.org/x/sync v0.17.0
go: found mvdan.cc/sh/shell in mvdan.cc/sh v2.6.4+incompatible
go: found mvdan.cc/sh/interp in mvdan.cc/sh v2.6.4+incompatible
go: found mvdan.cc/sh/syntax in mvdan.cc/sh v2.6.4+incompatible
go: found gopkg.in/yaml.v2 in gopkg.in/yaml.v2 v2.4.0
go: found github.com/Masterminds/sprig in github.com/Masterminds/sprig v2.22.0+incompatible
go: finding module for package golang.org/x/crypto/ssh/terminal
go: finding module for package golang.org/x/crypto/scrypt
go: finding module for package github.com/Masterminds/goutils
go: finding module for package github.com/google/uuid
go: finding module for package github.com/mitchellh/copystructure
go: finding module for package github.com/huandu/xstrings
go: finding module for package github.com/imdario/mergo
go: found golang.org/x/crypto/ssh/terminal in golang.org/x/crypto v0.42.0
go: found github.com/Masterminds/goutils in github.com/Masterminds/goutils v1.1.1
go: found github.com/google/uuid in github.com/google/uuid v1.6.0
go: found github.com/huandu/xstrings in github.com/huandu/xstrings v1.5.0
go: found github.com/imdario/mergo in github.com/imdario/mergo v1.0.2
go: found github.com/mitchellh/copystructure in github.com/mitchellh/copystructure v1.2.0
go: found golang.org/x/crypto/scrypt in golang.org/x/crypto v0.42.0
go: github.com/go-task/task/cmd/task imports
        github.com/go-task/task imports
        github.com/go-task/task/internal/templater imports
        github.com/Masterminds/sprig imports
        github.com/imdario/mergo: github.com/imdario/mergo@v1.0.2: parsing go.mod:
        module declares its path as: dario.cat/mergo
                but was required as: github.com/imdario/mergo

[trulede]

The fix for this should be available in the current version of task.