fix: [CI-24318]: preserve multiline action output values

utils/workflow.go

@@ -1,9 +1,12 @@
 package utils
 
 import (
+	"crypto/rand"
+	"encoding/hex"
 	"fmt"
 	"io/ioutil"
 	"os"
+	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -84,22 +87,60 @@ func getWorkflowEvent() string {
 	return "custom"
 }
 
+// newEOFMarker returns a per-run heredoc terminator that is extremely unlikely
+// to collide with any output value content. Falls back to a static suffix if
+// the crypto reader fails (should be effectively impossible on real systems).
+func newEOFMarker() string {
+	buf := make([]byte, 8)
+	if _, err := rand.Read(buf); err != nil {
+ return "HARNESS_EOF_FALLBACK"
+	}
+	return "HARNESS_EOF_" + hex.EncodeToString(buf)
+}
+
+// setOutputVariables generates a workflow step that appends each declared
+// action output to $DRONE_OUTPUT as a single line "key=__B64__<payload>".
+// The Harness delegate's output-var reader (harness/godotenv) detects the
+// __B64__ prefix and base64-decodes the payload transparently, so multiline
+// values and values containing any special characters round-trip cleanly.
+//
+// The raw value is ingested via a quoted heredoc so bash does not perform
+// variable expansion or escape processing on it.
 func setOutputVariables(prevStepId, outputFile string, outputVars []string) step {
 	skip := len(outputFile) == 0 || len(outputVars) == 0
 	if skip {
  logrus.Infof("No output variables detected in action.yml; skipping output file generation.")
+ return step{
+ Name: "output variables",
+ Run: fmt.Sprintf(": > %s", outputFile),
+ If: "false",
+ }
 	}
 
-	cmd := ""
+	marker := newEOFMarker()
+	var b strings.Builder
+	// Truncate the file first so re-runs do not append onto stale content.
+	fmt.Fprintf(&b, ": > %s\n", outputFile)
 	for _, outputVar := range outputVars {
- cmd += fmt.Sprintf("%s=${{ steps.%s.outputs.%s }}\n", outputVar, prevStepId, outputVar)
+ // Read the substituted value into __val via a quoted heredoc kept at
+ // the top level (not inside $(...)) to sidestep a bash 3.2 parser bug
+ // with double-quotes inside `$(cat <<'EOF' ... EOF)`. The heredoc
+ // terminator is generated per plugin run so it will not collide with
+ // value content. IFS= and -r prevent whitespace or backslash mangling;
+ // -d '' reads until NUL (never present), so read consumes the full
+ // heredoc and always returns non-zero, hence `|| true`.
+ fmt.Fprintf(&b, `IFS= read -r -d '' __val <<'%s' || true
+${{ steps.%s.outputs.%s }}
+%s
+__val="${__val%%$'\n'}"
+__b64=$(printf '%%s' "$__val" | base64 | tr -d '\n')
+echo "%s=__B64__$__b64" >> %s
+`, marker, prevStepId, outputVar, marker, outputVar, outputFile)
 	}
 
-	cmd = fmt.Sprintf("echo \"%s\" > %s", cmd, outputFile)
-	s := step{
+	return step{
  Name: "output variables",
- Run: cmd,
- If: fmt.Sprintf("%t", !skip),
+ Run: b.String(),
+ If: "true",
 	}
-	return s
 }

utils/workflow_test.go

@@ -1,8 +1,14 @@
 package utils
 
 import (
+	"bufio"
+	"encoding/base64"
 	"fmt"
 	"os"
+	"os/exec"
+	"path/filepath"
+	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -30,35 +36,173 @@ func TestCreateWorkflowFile(t *testing.T) {
 	assert.Contains(t, string(content), "steps:")
 	assert.Contains(t, string(content), "id: stepIdentifier")
 
-	// Check the `run` command
-	assert.Contains(t, string(content), "out1=${{ steps.stepIdentifier.outputs.out1 }}")
-	assert.Contains(t, string(content), "out-2=${{ steps.stepIdentifier.outputs.out-2 }}")
-	assert.Contains(t, string(content), fmt.Sprintf("> %s", outputFile))
+	// Check the run command references each output and appends to the file
+	assert.Contains(t, string(content), "steps.stepIdentifier.outputs.out1")
+	assert.Contains(t, string(content), "steps.stepIdentifier.outputs.out-2")
+	assert.Contains(t, string(content), fmt.Sprintf(">> %s", outputFile))
+	assert.Contains(t, string(content), "__B64__")
 
 	// Without output variables
 	err = CreateWorkflowFile(workflowFile, action, with, env, outputFile, []string{})
 	assert.NoError(t, err)
 	content, err = os.ReadFile(workflowFile)
 	assert.NoError(t, err)
 	assert.Contains(t, string(content), "name: output variables")
-	assert.Contains(t, string(content), "run: echo \"\" >")
+	assert.Contains(t, string(content), fmt.Sprintf(": > %s", outputFile))
 	assert.Contains(t, string(content), "if: \"false\"")
 }
 
+// TestSetOutputVariables_MultilineValueRoundTrip is the CI-24318 fix verification.
+//
+// It renders setOutputVariables, substitutes a multiline value the way act
+// would, runs the real bash, and parses the resulting file with the same
+// contract Harness's delegate reader uses (key=__B64__<payload> lines with
+// automatic base64 decode). The full multiline value must survive.
+func TestSetOutputVariables_MultilineValueRoundTrip(t *testing.T) {
+	testDir := t.TempDir()
+	outputFile := filepath.Join(testDir, "output.env")
+
+	prevStepId := "stepIdentifier"
+	outputVars := []string{"changelog", "new_tag"}
+
+	step := setOutputVariables(prevStepId, outputFile, outputVars)
+
+	multilineChangelog := "* commit A\n* commit B\n* commit C"
+	newTag := "v1.2.3"
+
+	shellCmd := substituteAll(step.Run, prevStepId, map[string]string{
+ "changelog": multilineChangelog,
+ "new_tag": newTag,
+	})
+
+	t.Logf("shell command the plugin would run:\n%s", shellCmd)
+
+	cmd := exec.Command("bash", "-c", shellCmd)
+	out, err := cmd.CombinedOutput()
+	assert.NoError(t, err, "bash exec failed: %s", string(out))
+
+	raw, err := os.ReadFile(outputFile)
+	assert.NoError(t, err)
+	t.Logf("resulting DRONE_OUTPUT file:\n---\n%s\n---", string(raw))
+
+	parsed := parseB64Output(t, outputFile)
+	assert.Equal(t, multilineChangelog, parsed["changelog"],
+ "multiline changelog must round-trip intact")
+	assert.Equal(t, newTag, parsed["new_tag"],
+ "single-line value must round-trip intact")
+}
+
+// TestSetOutputVariables_MultilineValuePreservesBlankLines confirms that
+// empty lines inside a multiline value are preserved.
+func TestSetOutputVariables_MultilineValuePreservesBlankLines(t *testing.T) {
+	testDir := t.TempDir()
+	outputFile := filepath.Join(testDir, "output.env")
+
+	prevStepId := "s"
+	outputVars := []string{"body"}
+
+	step := setOutputVariables(prevStepId, outputFile, outputVars)
+
+	val := "line 1\n\nline 3\n\n\nline 6"
+	shellCmd := substituteAll(step.Run, prevStepId, map[string]string{"body": val})
+
+	cmd := exec.Command("bash", "-c", shellCmd)
+	out, err := cmd.CombinedOutput()
+	assert.NoError(t, err, "bash exec failed: %s", string(out))
+
+	parsed := parseB64Output(t, outputFile)
+	assert.Equal(t, val, parsed["body"])
+}
+
+// TestSetOutputVariables_ValuesWithSpecialChars covers characters that
+// broke the old writer: double quotes, dollar signs, backticks, and
+// backslashes. All must survive because the payload is base64-encoded.
+func TestSetOutputVariables_ValuesWithSpecialChars(t *testing.T) {
+	testDir := t.TempDir()
+	outputFile := filepath.Join(testDir, "output.env")
+
+	prevStepId := "s"
+	outputVars := []string{"tricky"}
+
+	step := setOutputVariables(prevStepId, outputFile, outputVars)
+
+	trickyVal := "quote:\" dollar:$USER backtick:`id` backslash:\\ end"
+	shellCmd := substituteAll(step.Run, prevStepId, map[string]string{"tricky": trickyVal})
+
+	t.Logf("shell command:\n%s", shellCmd)
+
+	cmd := exec.Command("bash", "-c", shellCmd)
+	out, err := cmd.CombinedOutput()
+	assert.NoError(t, err, "bash exec failed: %s", string(out))
+
+	parsed := parseB64Output(t, outputFile)
+	assert.Equal(t, trickyVal, parsed["tricky"])
+}
+
 func TestSetOutputVariables(t *testing.T) {
 	outputVars := []string{"var1", "var2"}
 	prevStepId := "prevStep"
 	outputFile := "/tmp/output"
 
-	// With output variables
+	// With output variables: one per-variable block that appends __B64__ line.
 	step := setOutputVariables(prevStepId, outputFile, outputVars)
 	assert.Equal(t, "output variables", step.Name)
-	assert.Contains(t, step.Run, "var1=${{ steps.prevStep.outputs.var1 }}")
-	assert.Contains(t, step.Run, "var2=${{ steps.prevStep.outputs.var2 }}")
+	assert.Equal(t, "true", step.If)
+	assert.Contains(t, step.Run, "steps.prevStep.outputs.var1")
+	assert.Contains(t, step.Run, "steps.prevStep.outputs.var2")
+	assert.Contains(t, step.Run, fmt.Sprintf(">> %s", outputFile))
+	assert.Contains(t, step.Run, "__B64__")
+	// Each variable gets its own heredoc marker for ingestion.
+	assert.Regexp(t, regexp.MustCompile(`<<'HARNESS_EOF_[0-9a-f]{16}'`), step.Run)
+	assert.Contains(t, step.Run, `echo "var1=__B64__$__b64"`)
+	assert.Contains(t, step.Run, `echo "var2=__B64__$__b64"`)
 
-	// No output variables
+	// No output variables: file is truncated, step is gated off.
 	step = setOutputVariables(prevStepId, outputFile, []string{})
 	assert.Equal(t, "output variables", step.Name)
-	assert.Contains(t, step.Run, "echo \"\" > /tmp/output")
-	assert.Contains(t, step.If, "false")
+	assert.Contains(t, step.Run, ": > /tmp/output")
+	assert.Equal(t, "false", step.If)
+}
+
+// substituteAll replaces every ${{ steps.<id>.outputs.<name> }} placeholder
+// in shellCmd with its corresponding value, mimicking act's substitution.
+func substituteAll(shellCmd, prevStepId string, values map[string]string) string {
+	out := shellCmd
+	for name, val := range values {
+ placeholder := fmt.Sprintf("${{ steps.%s.outputs.%s }}", prevStepId, name)
+ out = strings.ReplaceAll(out, placeholder, val)
+	}
+	return out
+}
+
+// parseB64Output reads a DRONE_OUTPUT file in the same shape Harness's
+// delegate reader does: parse key=value lines, then base64-decode any value
+// starting with the __B64__ prefix.
+func parseB64Output(t *testing.T, path string) map[string]string {
+	t.Helper()
+	f, err := os.Open(path)
+	assert.NoError(t, err)
+	defer f.Close()
+
+	parsed := map[string]string{}
+	scanner := bufio.NewScanner(f)
+	scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024)
+	for scanner.Scan() {
+ line := scanner.Text()
+ idx := strings.Index(line, "=")
+ if idx <= 0 {
+ continue
+ }
+ key := line[:idx]
+ val := line[idx+1:]
+ if strings.HasPrefix(val, "__B64__") {
+ raw, err := base64.StdEncoding.DecodeString(val[len("__B64__"):])
+ assert.NoError(t, err)
+ parsed[key] = string(raw)
+ continue
+ }
+ parsed[key] = val
+	}
+	assert.NoError(t, scanner.Err())
+	return parsed
 }