From 6705c9ecae36780e81c1559888b9ddd2227fc5c5 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 15 May 2024 17:19:02 +0200 Subject: [PATCH 1/6] Update bootstrap scripts to check for executable correctness Signed-off-by: Danny Kopping --- provisionersdk/scripts/bootstrap_darwin.sh | 13 +++++++++++-- provisionersdk/scripts/bootstrap_linux.sh | 13 +++++++++++-- provisionersdk/scripts/bootstrap_windows.ps1 | 13 +++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/provisionersdk/scripts/bootstrap_darwin.sh b/provisionersdk/scripts/bootstrap_darwin.sh index 70158594de7d6..204eb475d7357 100644 --- a/provisionersdk/scripts/bootstrap_darwin.sh +++ b/provisionersdk/scripts/bootstrap_darwin.sh @@ -4,7 +4,7 @@ set -eux # This is to allow folks to exec into a failed workspace and poke around to # troubleshoot. waitonexit() { - echo "=== Agent script exited with non-zero code. Sleeping 24h to preserve logs..." + echo "=== Agent script exited with non-zero code ($?). Sleeping 24h to preserve logs..." sleep 86400 } trap waitonexit EXIT @@ -31,4 +31,13 @@ fi export CODER_AGENT_AUTH="${AUTH_TYPE}" export CODER_AGENT_URL="${ACCESS_URL}" -exec ./$BINARY_NAME agent + +output=$(./${BINARY_NAME} --version | head -n1) +echo "${output}" | grep -q Coder +if [ $? -ne 0 ] ; then + echo >&2 "ERROR: Downloaded agent binary is invalid" + echo >&2 "Script output: '${output}'" + exit 2 +fi + +exec ./${BINARY_NAME} agent diff --git a/provisionersdk/scripts/bootstrap_linux.sh b/provisionersdk/scripts/bootstrap_linux.sh index faf4b4a9bbfac..c2c81f8696f34 100755 --- a/provisionersdk/scripts/bootstrap_linux.sh +++ b/provisionersdk/scripts/bootstrap_linux.sh @@ -4,7 +4,7 @@ set -eux # This is to allow folks to exec into a failed workspace and poke around to # troubleshoot. waitonexit() { - echo "=== Agent script exited with non-zero code. Sleeping 24h to preserve logs..." + echo "=== Agent script exited with non-zero code ($?). Sleeping 24h to preserve logs..." sleep 86400 } trap waitonexit EXIT @@ -86,4 +86,13 @@ fi export CODER_AGENT_AUTH="${AUTH_TYPE}" export CODER_AGENT_URL="${ACCESS_URL}" -exec ./$BINARY_NAME agent + +output=$(./${BINARY_NAME} --version | head -n1) +echo "${output}" | grep -q Coder +if [ $? -ne 0 ] ; then + echo >&2 "ERROR: Downloaded agent binary is invalid" + echo >&2 "Script output: '${output}'" + exit 2 +fi + +exec ./${BINARY_NAME} agent diff --git a/provisionersdk/scripts/bootstrap_windows.ps1 b/provisionersdk/scripts/bootstrap_windows.ps1 index e51dd9415a790..fa0ec660b4f06 100644 --- a/provisionersdk/scripts/bootstrap_windows.ps1 +++ b/provisionersdk/scripts/bootstrap_windows.ps1 @@ -35,6 +35,19 @@ if (-not (Get-Command 'Set-MpPreference' -ErrorAction SilentlyContinue)) { $env:CODER_AGENT_AUTH = "${AUTH_TYPE}" $env:CODER_AGENT_URL = "${ACCESS_URL}" +$psi = [System.Diagnostics.ProcessStartInfo]::new("$env:TEMP\sshd.exe", '--version') +$psi.UseShellExecute = $false +$psi.RedirectStandardOutput = $true +$p = [System.Diagnostics.Process]::Start($psi) +$output = $p.StandardOutput.ReadToEnd() +$p.WaitForExit() + +if ($output -notlike "*Coder*") { + Write-Error "ERROR: Downloaded agent binary is invalid" + Write-Error "Script output: '$output'" + Exit 2 +} + # Check if we're running inside a Windows container! $inContainer = $false if ((Get-ItemProperty 'HKLM:\SYSTEM\CurrentControlSet\Control' -Name 'ContainerType' -ErrorAction SilentlyContinue) -ne $null) { From d11c2d3a0889a4fb6edbeb53399e71461cc01bc9 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 15 May 2024 17:19:42 +0200 Subject: [PATCH 2/6] Add comment to more easily find string replacements Signed-off-by: Danny Kopping --- provisionersdk/agent.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/provisionersdk/agent.go b/provisionersdk/agent.go index 1a285577fabda..ce7abf1c0da67 100644 --- a/provisionersdk/agent.go +++ b/provisionersdk/agent.go @@ -39,9 +39,9 @@ var ( } ) -// AgentScriptEnv returns a key-pair of scripts that are consumed -// by the Coder Terraform Provider. See: -// https://github.com/coder/terraform-provider-coder/blob/main/internal/provider/provider.go#L97 +// AgentScriptEnv returns a key-pair of scripts that are consumed by the Coder Terraform Provider. +// https://github.com/coder/terraform-provider-coder/blob/main/provider/agent.go (updateInitScript) +// performs additional string substitutions. func AgentScriptEnv() map[string]string { env := map[string]string{} for operatingSystem, scripts := range agentScripts { From b63b4790eaa01000c52c69c4a5a0d968966a1a58 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 15 May 2024 17:36:37 +0200 Subject: [PATCH 3/6] Appease shellcheck Signed-off-by: Danny Kopping --- provisionersdk/scripts/bootstrap_darwin.sh | 9 ++++----- provisionersdk/scripts/bootstrap_linux.sh | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/provisionersdk/scripts/bootstrap_darwin.sh b/provisionersdk/scripts/bootstrap_darwin.sh index 204eb475d7357..5bb44e3de85e9 100644 --- a/provisionersdk/scripts/bootstrap_darwin.sh +++ b/provisionersdk/scripts/bootstrap_darwin.sh @@ -33,11 +33,10 @@ export CODER_AGENT_AUTH="${AUTH_TYPE}" export CODER_AGENT_URL="${ACCESS_URL}" output=$(./${BINARY_NAME} --version | head -n1) -echo "${output}" | grep -q Coder -if [ $? -ne 0 ] ; then - echo >&2 "ERROR: Downloaded agent binary is invalid" - echo >&2 "Script output: '${output}'" - exit 2 +if ! echo "${output}" | grep -q Coder; then + echo >&2 "ERROR: Downloaded agent binary is invalid" + echo >&2 "Script output: '${output}'" + exit 2 fi exec ./${BINARY_NAME} agent diff --git a/provisionersdk/scripts/bootstrap_linux.sh b/provisionersdk/scripts/bootstrap_linux.sh index c2c81f8696f34..6fa6bca82b6d5 100755 --- a/provisionersdk/scripts/bootstrap_linux.sh +++ b/provisionersdk/scripts/bootstrap_linux.sh @@ -88,11 +88,10 @@ export CODER_AGENT_AUTH="${AUTH_TYPE}" export CODER_AGENT_URL="${ACCESS_URL}" output=$(./${BINARY_NAME} --version | head -n1) -echo "${output}" | grep -q Coder -if [ $? -ne 0 ] ; then - echo >&2 "ERROR: Downloaded agent binary is invalid" - echo >&2 "Script output: '${output}'" - exit 2 +if ! echo "${output}" | grep -q Coder; then + echo >&2 "ERROR: Downloaded agent binary is invalid" + echo >&2 "Script output: '${output}'" + exit 2 fi exec ./${BINARY_NAME} agent From 5438b65797bbca43ab2cd8acadcc0b741f83d0cb Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 16 May 2024 09:59:08 +0200 Subject: [PATCH 4/6] Make lint script more portable Signed-off-by: Danny Kopping --- scripts/check_site_icons.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check_site_icons.sh b/scripts/check_site_icons.sh index 3ccd6b02cac41..8b0c390a7b1e4 100755 --- a/scripts/check_site_icons.sh +++ b/scripts/check_site_icons.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash set -euo pipefail # shellcheck source=scripts/lib.sh From 8f08e000f8e1bf361720f93c40a9e50a1014549b Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 16 May 2024 09:59:15 +0200 Subject: [PATCH 5/6] Add tests Signed-off-by: Danny Kopping --- provisionersdk/agent_test.go | 137 +++++++++++++++++++++++++++++------ 1 file changed, 116 insertions(+), 21 deletions(-) diff --git a/provisionersdk/agent_test.go b/provisionersdk/agent_test.go index 96c0d531ad6f7..71d7d1754d979 100644 --- a/provisionersdk/agent_test.go +++ b/provisionersdk/agent_test.go @@ -7,6 +7,9 @@ package provisionersdk_test import ( + "bytes" + "context" + "errors" "fmt" "net/http" "net/http/httptest" @@ -14,50 +17,142 @@ import ( "os/exec" "runtime" "strings" + "sync" "testing" + "time" "github.com/go-chi/render" "github.com/stretchr/testify/require" + "github.com/coder/coder/v2/testutil" + "github.com/coder/coder/v2/provisionersdk" ) +// mimicking the --version output which we use to test the binary (see provisionersdk/scripts/bootstrap_*). +const versionOutput = `Coder v2.11.0+8979bfe Tue May 7 17:30:19 UTC 2024` + // bashEcho is a script that calls the local `echo` with the arguments. This is preferable to // sending the real `echo` binary since macOS 14.4+ immediately sigkills `echo` if it is copied to // another directory and run locally. const bashEcho = `#!/usr/bin/env bash -echo $@` +echo "` + versionOutput + `"` + +const unexpectedEcho = `#!/usr/bin/env bash +echo "this is not the agent you are looking for"` func TestAgentScript(t *testing.T) { t.Parallel() - t.Run("Run", func(t *testing.T) { + + t.Run("Valid", func(t *testing.T) { t.Parallel() - srv := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - render.Status(r, http.StatusOK) - render.Data(rw, r, []byte(bashEcho)) - })) - defer srv.Close() - srvURL, err := url.Parse(srv.URL) - require.NoError(t, err) - script, exists := provisionersdk.AgentScriptEnv()[fmt.Sprintf("CODER_AGENT_SCRIPT_%s_%s", runtime.GOOS, runtime.GOARCH)] - if !exists { - t.Skip("Agent not supported...") - return - } - script = strings.ReplaceAll(script, "${ACCESS_URL}", srvURL.String()+"/") - script = strings.ReplaceAll(script, "${AUTH_TYPE}", "token") + script := serveScript(t, bashEcho) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + t.Cleanup(cancel) + + var output bytes.Buffer // This is intentionally ran in single quotes to mimic how a customer may // embed our script. Our scripts should not include any single quotes. // nolint:gosec - output, err := exec.Command("sh", "-c", "sh -c '"+script+"'").CombinedOutput() - t.Log(string(output)) + cmd := exec.CommandContext(ctx, "sh", "-c", "sh -c '"+script+"'") + cmd.Stdout = &output + cmd.Stderr = &output + require.NoError(t, cmd.Start()) + + err := cmd.Wait() + if err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + require.Equal(t, 0, exitErr.ExitCode()) + } else { + t.Fatalf("unexpected err: %s", err) + } + } + + t.Log(output.String()) require.NoError(t, err) // Ignore debug output from `set -x`, we're only interested in the last line. - lines := strings.Split(strings.TrimSpace(string(output)), "\n") + lines := strings.Split(strings.TrimSpace(output.String()), "\n") lastLine := lines[len(lines)-1] - // Because we use the "echo" binary, we should expect the arguments provided + // When we use the "bashEcho" binary, we should expect the arguments provided // as the response to executing our script. - require.Equal(t, "agent", lastLine) + require.Equal(t, versionOutput, lastLine) }) + + t.Run("Invalid", func(t *testing.T) { + t.Parallel() + + script := serveScript(t, unexpectedEcho) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + t.Cleanup(cancel) + + var output bytes.Buffer + // This is intentionally ran in single quotes to mimic how a customer may + // embed our script. Our scripts should not include any single quotes. + // nolint:gosec + cmd := exec.CommandContext(ctx, "sh", "-c", "sh -c '"+script+"'") + cmd.WaitDelay = time.Second + cmd.Stdout = &output + cmd.Stderr = &output + require.NoError(t, cmd.Start()) + + done := make(chan error, 1) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + + // The bootstrap scripts trap exit codes to allow operators to view the script logs and debug the process + // while it is still running. We do not expect Wait() to complete. + err := cmd.Wait() + done <- err + }() + + select { + case <-ctx.Done(): + // Timeout. + break + case err := <-done: + // If done signals before context times out, script behaved in an unexpected way. + if err != nil { + t.Fatalf("unexpected err: %s", err) + } + } + + // Kill the command, wait for the command to yield. + require.NoError(t, cmd.Cancel()) + wg.Wait() + + t.Log(output.String()) + + require.Eventually(t, func() bool { + return bytes.Contains(output.Bytes(), []byte("ERROR: Downloaded agent binary is invalid")) + }, testutil.WaitShort, testutil.IntervalSlow) + }) +} + +// serveScript creates a fake HTTP server which serves a requested "agent binary" (which is actually the given input string) +// which will be attempted to run to verify that it is correct. +func serveScript(t *testing.T, in string) string { + t.Helper() + + srv := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + render.Status(r, http.StatusOK) + render.Data(rw, r, []byte(in)) + })) + t.Cleanup(srv.Close) + srvURL, err := url.Parse(srv.URL) + require.NoError(t, err) + + script, exists := provisionersdk.AgentScriptEnv()[fmt.Sprintf("CODER_AGENT_SCRIPT_%s_%s", runtime.GOOS, runtime.GOARCH)] + if !exists { + t.Skip("Agent not supported...") + return "" + } + script = strings.ReplaceAll(script, "${ACCESS_URL}", srvURL.String()+"/") + script = strings.ReplaceAll(script, "${AUTH_TYPE}", "token") + return script } From 70e3091ced28a8af2bfba5315b7d759cbc136d86 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 16 May 2024 11:24:34 +0200 Subject: [PATCH 6/6] Use more expressive error, double-quote output Signed-off-by: Danny Kopping --- provisionersdk/agent_test.go | 2 +- provisionersdk/scripts/bootstrap_darwin.sh | 4 ++-- provisionersdk/scripts/bootstrap_linux.sh | 4 ++-- provisionersdk/scripts/bootstrap_windows.ps1 | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/provisionersdk/agent_test.go b/provisionersdk/agent_test.go index 71d7d1754d979..60a973c740340 100644 --- a/provisionersdk/agent_test.go +++ b/provisionersdk/agent_test.go @@ -129,7 +129,7 @@ func TestAgentScript(t *testing.T) { t.Log(output.String()) require.Eventually(t, func() bool { - return bytes.Contains(output.Bytes(), []byte("ERROR: Downloaded agent binary is invalid")) + return bytes.Contains(output.Bytes(), []byte("ERROR: Downloaded agent binary returned unexpected version output")) }, testutil.WaitShort, testutil.IntervalSlow) }) } diff --git a/provisionersdk/scripts/bootstrap_darwin.sh b/provisionersdk/scripts/bootstrap_darwin.sh index 5bb44e3de85e9..501e43997619e 100644 --- a/provisionersdk/scripts/bootstrap_darwin.sh +++ b/provisionersdk/scripts/bootstrap_darwin.sh @@ -34,8 +34,8 @@ export CODER_AGENT_URL="${ACCESS_URL}" output=$(./${BINARY_NAME} --version | head -n1) if ! echo "${output}" | grep -q Coder; then - echo >&2 "ERROR: Downloaded agent binary is invalid" - echo >&2 "Script output: '${output}'" + echo >&2 "ERROR: Downloaded agent binary returned unexpected version output" + echo >&2 "${BINARY_NAME} --version output: \"${output}\"" exit 2 fi diff --git a/provisionersdk/scripts/bootstrap_linux.sh b/provisionersdk/scripts/bootstrap_linux.sh index 6fa6bca82b6d5..c07cbc3e01667 100755 --- a/provisionersdk/scripts/bootstrap_linux.sh +++ b/provisionersdk/scripts/bootstrap_linux.sh @@ -89,8 +89,8 @@ export CODER_AGENT_URL="${ACCESS_URL}" output=$(./${BINARY_NAME} --version | head -n1) if ! echo "${output}" | grep -q Coder; then - echo >&2 "ERROR: Downloaded agent binary is invalid" - echo >&2 "Script output: '${output}'" + echo >&2 "ERROR: Downloaded agent binary returned unexpected version output" + echo >&2 "${BINARY_NAME} --version output: \"${output}\"" exit 2 fi diff --git a/provisionersdk/scripts/bootstrap_windows.ps1 b/provisionersdk/scripts/bootstrap_windows.ps1 index fa0ec660b4f06..0c8381ef936ca 100644 --- a/provisionersdk/scripts/bootstrap_windows.ps1 +++ b/provisionersdk/scripts/bootstrap_windows.ps1 @@ -43,9 +43,9 @@ $output = $p.StandardOutput.ReadToEnd() $p.WaitForExit() if ($output -notlike "*Coder*") { - Write-Error "ERROR: Downloaded agent binary is invalid" - Write-Error "Script output: '$output'" - Exit 2 + Write-Output "$env:TEMP\sshd.exe --version output: `"$output"`" + Write-Error "ERROR: Downloaded agent binary returned unexpected version output" + Throw "unexpected binary" } # Check if we're running inside a Windows container!