diff --git a/apps/cinc/cmd/node_test.go b/apps/cinc/cmd/node_test.go index 64d1e77..0486f3b 100644 --- a/apps/cinc/cmd/node_test.go +++ b/apps/cinc/cmd/node_test.go @@ -535,7 +535,7 @@ func TestNodeBootstrapDryRunCommand(t *testing.T) { } got := buf.String() for _, want := range []string{ - "curl -L 'https://omnitruck.cinc.sh/install.sh'", + "curl -fsSL 'https://omnitruck.cinc.sh/install.sh'", "chef_server_url 'https://cinc.example.test/organizations/acme'", "node_name 'web01'", "\"run_list\": [", diff --git a/cli/remote/bootstrap.go b/cli/remote/bootstrap.go index c3ea3e6..2e45639 100644 --- a/cli/remote/bootstrap.go +++ b/cli/remote/bootstrap.go @@ -78,10 +78,20 @@ func BootstrapCommand(opts BootstrapOptions) (string, error) { } commands := []string{ "set -e", - // Trust assumption: this pipes the installer script straight into a - // shell, so the target trusts opts.BootstrapURL (HTTPS omnitruck by - // default) and its TLS chain — standard Chef/Cinc bootstrap behavior. - "curl -L " + shellQuote(opts.BootstrapURL) + " | " + prefix + "bash -s --" + installArgs, + // The installer is downloaded to a file and then run, rather than + // piped into a shell. A pipeline reports only the exit status of its + // last command, so `curl ... | bash` hides a failed download from + // `set -e`: the script would carry on and run a cinc-client that was + // never installed. curl -f turns an HTTP error into a failure instead + // of saving the error page as the "installer". + // + // Trust assumption is unchanged: the target executes whatever + // opts.BootstrapURL serves (HTTPS omnitruck by default), so it trusts + // that host and its TLS chain, as with any Chef/Cinc bootstrap. + `CINC_INSTALLER="$(mktemp)"`, + `trap 'rm -f "$CINC_INSTALLER"' EXIT`, + "curl -fsSL " + shellQuote(opts.BootstrapURL) + ` -o "$CINC_INSTALLER"`, + prefix + `bash "$CINC_INSTALLER"` + installArgs, prefix + "mkdir -p /etc/cinc", // Create the private key 0600 *before* writing, so it is never // world-readable on the target (tee would otherwise create it with the diff --git a/cli/remote/bootstrap_test.go b/cli/remote/bootstrap_test.go index f0261bb..5ade03d 100644 --- a/cli/remote/bootstrap_test.go +++ b/cli/remote/bootstrap_test.go @@ -21,7 +21,8 @@ func TestBootstrapCommandBuildsCincClientScript(t *testing.T) { t.Fatalf("BootstrapCommand: %v", err) } for _, want := range []string{ - "curl -L 'https://omnitruck.cinc.sh/install.sh' | sudo bash -s -- -v '18'", + "curl -fsSL 'https://omnitruck.cinc.sh/install.sh' -o", + "sudo bash \"$CINC_INSTALLER\" -v '18'", "sudo mkdir -p /etc/cinc", "sudo install -m 0600 /dev/null /etc/cinc/client.pem", "chef_server_url 'https://cinc.example.test/organizations/acme'", @@ -37,6 +38,47 @@ func TestBootstrapCommandBuildsCincClientScript(t *testing.T) { } } +// TestBootstrapCommandStopsWhenTheInstallerCannotBeFetched pins that a failed +// download aborts the bootstrap. `curl ... | bash` reports only bash's status, +// so under plain `set -e` a 404 or a TLS failure was swallowed and the script +// carried on to run a cinc-client that had never been installed. +func TestBootstrapCommandStopsWhenTheInstallerCannotBeFetched(t *testing.T) { + cmd, err := BootstrapCommand(BootstrapOptions{ + NodeName: "web01", + ServerURL: "https://cinc.example.test/organizations/acme", + ClientKeyPEM: "PRIVATE KEY", + Sudo: true, + }) + if err != nil { + t.Fatalf("BootstrapCommand: %v", err) + } + if strings.Contains(cmd, "| sudo bash") || strings.Contains(cmd, "| bash") { + t.Errorf("installer is still piped into a shell, so its exit status is lost:\n%s", cmd) + } + // -f makes curl fail on an HTTP error rather than saving the error page. + if !strings.Contains(cmd, "curl -fsSL") { + t.Errorf("curl should use -f so HTTP errors are failures:\n%s", cmd) + } + if !strings.Contains(cmd, "set -e") { + t.Errorf("script should still abort on the first failing command:\n%s", cmd) + } +} + +// The downloaded installer is removed even when a later step fails. +func TestBootstrapCommandCleansUpTheInstaller(t *testing.T) { + cmd, err := BootstrapCommand(BootstrapOptions{ + NodeName: "web01", + ServerURL: "https://cinc.example.test/organizations/acme", + ClientKeyPEM: "PRIVATE KEY", + }) + if err != nil { + t.Fatalf("BootstrapCommand: %v", err) + } + if !strings.Contains(cmd, "trap") || !strings.Contains(cmd, "rm -f") { + t.Errorf("script should remove the downloaded installer on exit:\n%s", cmd) + } +} + // firstBoot extracts and decodes the first-boot.json payload embedded in the // generated bootstrap script so tests can assert on the actual JSON keys. func firstBoot(t *testing.T, opts BootstrapOptions) map[string]any {