diff --git a/docs/README.md b/docs/README.md index 3c86314..a11ec45 100644 --- a/docs/README.md +++ b/docs/README.md @@ -70,7 +70,7 @@ Finally, it will execute the plan, running tasks sequentially until the end.
-All scripts include set -euo pipefail. script includes set -x. +All scripts include set -euo pipefail. Before running any Bash script in `script`, `when.output_changes` or `when.check_fails`, Ebro will prepend to the script the lines `set -euo pipefail` to ensure sane defaults: @@ -79,10 +79,6 @@ Before running any Bash script in `script`, `when.output_changes` or `when.check - `-u`: Usage of unset variables is considered an error - `-o pipefail`: The pipeline’s return status is the value of the last (rightmost) command to exit with a non-zero status, or zero if all commands exit successfully -In case of `script`, it will also add `set -x`: - -- `-x`: Print a trace of simple commands. - More on Bash's documentation: [The Set Builtin](https://www.gnu.org/software/bash/manual/bash.html#The-Set-Builtin).
@@ -91,15 +87,10 @@ During the first execution it will execute everything, with no skips, which soul ``` ███ [:cache_dir] running -+ mkdir -p cache ███ [:produce_a] running -+ echo 'this is A' ███ [:produce_b] running -+ echo 'this is B' ███ [:echoer] running -+ cat cache/A.txt this is A -+ cat cache/B.txt this is B ███ [:producer] satisfied ███ [:default] satisfied diff --git a/docs/changelog/0.3.0.md b/docs/changelog/0.3.0.md new file mode 100644 index 0000000..d7c1d37 --- /dev/null +++ b/docs/changelog/0.3.0.md @@ -0,0 +1,5 @@ +## Breaking changes + +### `set -x` added in previous version have been reverted + +It's more secure to let users call themselves `set -x` whenever they need it instead of enabling it by default. It's easy to leak secrets on stdout with it enabled. diff --git a/src/internal/runner/runner.go b/src/internal/runner/runner.go index fd4c8a2..9fb9490 100644 --- a/src/internal/runner/runner.go +++ b/src/internal/runner/runner.go @@ -34,7 +34,7 @@ func Run(inv inventory.Inventory, plan planner.Plan, force bool) error { if task.When.CheckFails != "" { output := bytes.Buffer{} outputWriter := bufio.NewWriter(&output) - status, err := runScriptWithIO(task.When.CheckFails, task.WorkingDirectory, task.Environment, outputWriter, outputWriter, false) + status, err := runScriptWithIO(task.When.CheckFails, task.WorkingDirectory, task.Environment, outputWriter, outputWriter) if err != nil { return fmt.Errorf("running task %v when.check_fails: %w", taskName, err) } @@ -48,7 +48,7 @@ func Run(inv inventory.Inventory, plan planner.Plan, force bool) error { if task.When.OutputChanges != "" { output := bytes.Buffer{} outputWriter := bufio.NewWriter(&output) - status, err := runScriptWithIO(task.When.OutputChanges, task.WorkingDirectory, task.Environment, outputWriter, outputWriter, false) + status, err := runScriptWithIO(task.When.OutputChanges, task.WorkingDirectory, task.Environment, outputWriter, outputWriter) if err != nil { return fmt.Errorf("running task %v when.output_changes: %w", taskName, err) } @@ -101,14 +101,11 @@ func logLine(taskName string, message string) string { } func runScript(script string, workingDirectory string, environment map[string]string) (uint8, error) { - return runScriptWithIO(script, workingDirectory, environment, os.Stdout, os.Stdout, true) + return runScriptWithIO(script, workingDirectory, environment, os.Stdout, os.Stdout) } -func runScriptWithIO(script string, workingDirectory string, environment map[string]string, stdout io.Writer, stderr io.Writer, simpleCommandTracing bool) (uint8, error) { +func runScriptWithIO(script string, workingDirectory string, environment map[string]string, stdout io.Writer, stderr io.Writer) (uint8, error) { script_header := []string{"set -euo pipefail"} - if simpleCommandTracing { - script_header = append(script_header, "set -x") - } file, err := syntax.NewParser().Parse(strings.NewReader(strings.Join(script_header, "\n")+"\n"+script), "") if err != nil { diff --git a/tests/src/git_import_test.py b/tests/src/git_import_test.py index 5a41e1f..af3e280 100644 --- a/tests/src/git_import_test.py +++ b/tests/src/git_import_test.py @@ -17,18 +17,13 @@ def test_git_import_works(self, repository_url): f""" ███ cloning {repository_url} ███ [:apt:pre-config] running - + mkdir -p {self.workdir}/.cache/apt/packages ███ [:caddy:package-apt-config] running - + echo caddy ███ [:apt:default] running - + echo 'Installing apt packages' Installing apt packages - + cat {self.workdir}/.cache/apt/packages/caddy.txt caddy ███ [:caddy:package] satisfied ███ [:caddy:default] satisfied ███ [:default] running - + echo Done! Done! """, ) @@ -49,7 +44,6 @@ def test_git_import_works(self, repository_url): ███ [:caddy:package] satisfied ███ [:caddy:default] satisfied ███ [:default] running - + echo Done! Done! """, ) diff --git a/tests/src/run_test.py b/tests/src/run_test.py index e06f929..d7a69bc 100644 --- a/tests/src/run_test.py +++ b/tests/src/run_test.py @@ -10,15 +10,10 @@ def test_execution_is_correct(self): stdout, f""" ███ [:apt:pre-config] running - + mkdir -p {self.workdir}/.cache/apt/packages ███ [:caddy:package-apt-config] running - + echo caddy ███ [:docker:package-apt-config] running - + echo 'docker==2.0.0-1-apt' ███ [:apt:default] running - + echo 'Installing apt packages' Installing apt packages - + cat {self.workdir}/.cache/apt/packages/caddy.txt {self.workdir}/.cache/apt/packages/docker.txt caddy docker==2.0.0-1-apt ███ [:caddy:package] satisfied @@ -26,7 +21,6 @@ def test_execution_is_correct(self): ███ [:caddy:default] satisfied ███ [:docker:default] satisfied ███ [:default] running - + echo Done! Done! """, ) @@ -46,7 +40,6 @@ def test_execution_is_correct(self): ███ [:caddy:default] satisfied ███ [:docker:default] satisfied ███ [:default] running - + echo Done! Done! """, ) @@ -58,15 +51,10 @@ def test_execution_is_correct(self): stdout, f""" ███ [:apt:pre-config] running - + mkdir -p {self.workdir}/.cache/apt/packages ███ [:caddy:package-apt-config] running - + echo caddy ███ [:docker:package-apt-config] running - + echo 'docker==2.0.0-1-apt' ███ [:apt:default] running - + echo 'Installing apt packages' Installing apt packages - + cat {self.workdir}/.cache/apt/packages/caddy.txt {self.workdir}/.cache/apt/packages/docker.txt caddy docker==2.0.0-1-apt ███ [:caddy:package] satisfied @@ -74,7 +62,6 @@ def test_execution_is_correct(self): ███ [:caddy:default] satisfied ███ [:docker:default] satisfied ███ [:default] running - + echo Done! Done! """, ) @@ -86,7 +73,6 @@ def test_scripts_fail_asap(self): stdout, f""" ███ [:default] running - + echo 'This should print' This should print UNBOUND_VARIABLE: unbound variable ███ ERROR: task :default returned status code 1 @@ -100,9 +86,7 @@ def test_failing_scripts_are_not_cached_by_output_changes(self): stdout, f""" ███ [:default] running - + echo 'This should print all the time' This should print all the time - + exit 1 ███ ERROR: task :default returned status code 1 """, ) @@ -113,9 +97,7 @@ def test_failing_scripts_are_not_cached_by_output_changes(self): stdout, f""" ███ [:default] running - + echo 'This should print all the time' This should print all the time - + exit 1 ███ ERROR: task :default returned status code 1 """, ) @@ -127,7 +109,6 @@ def test_required_by_does_not_include_referenced_task_in_plan(self): stdout, f""" ███ [:b] running - + echo B B ███ [:default] satisfied """, @@ -140,10 +121,8 @@ def test_when_checkers_behave_correctly(self): stdout, f""" ███ [:always_fails] running - + echo Running Running ███ [:never_fails] running - + echo Running Running ███ [:default] satisfied """, @@ -155,7 +134,6 @@ def test_when_checkers_behave_correctly(self): stdout, f""" ███ [:always_fails] running - + echo Running Running ███ [:never_fails] skipping ███ [:default] satisfied