fix: escape control characters when substituting multiline params

pkg/template/resource.go

@@ -121,30 +121,119 @@ func escapeTektonVariables(value string) string {
 }
 
 // applyParamsToResourceTemplate returns the TriggerResourceTemplate with the
-// param values substituted for all matching param variables in the template
+// param values substituted for all matching param variables in the template.
+// All params are substituted in a single pass over rt, rather than one pass
+// per param: rescanning already-substituted output for the next param would
+// let a raw, unescaped quote from one param's value (e.g. from #823-preserved
+// pass-through behavior) desync the JSON-string-boundary tracking used to
+// decide whether a later param's control characters need escaping.
 func applyParamsToResourceTemplate(params []triggersv1.Param, rt json.RawMessage, oldEscape bool) json.RawMessage {
 	// Assume the params are valid
-	for _, param := range params {
-		rt = applyParamToResourceTemplate(param, rt, oldEscape)
+	return substituteParamsInResourceTemplate(params, rt, oldEscape)
+}
+
+// escapeJSONControlChars escapes literal control characters (e.g. a raw
+// newline from a multiline TriggerBinding value) so the string can be
+// embedded inside a JSON string literal without producing invalid JSON.
+// Unlike the old-escape-quotes behavior, this does not touch quote or
+// backslash characters, since a raw control character is never valid JSON
+// (regardless of context) while a bare quote or backslash may already be
+// part of an intentionally pre-escaped value. See #257 and #823.
+func escapeJSONControlChars(value string) string {
+	var b strings.Builder
+	for _, r := range value {
+		switch r {
+		case '\n':
+			b.WriteString(`\n`)
+		case '\r':
+			b.WriteString(`\r`)
+		case '\t':
+			b.WriteString(`\t`)
+		default:
+			if r < 0x20 {
+				fmt.Fprintf(&b, `\u%04x`, r)
+			} else {
+				b.WriteRune(r)
+			}
+		}
 	}
-	return rt
+	return b.String()
 }
 
 // applyParamToResourceTemplate returns the TriggerResourceTemplate with the
 // param value substituted for all matching param variables in the template
 func applyParamToResourceTemplate(param triggersv1.Param, rt json.RawMessage, oldEscape bool) json.RawMessage {
 	// Assume the param is valid
-	paramVariable := fmt.Sprintf("$(tt.params.%s)", param.Name)
-	// Escape quotes so that JSON strings can be appended to regular strings.
-	// See #257 for discussion on this behavior.
-	paramValue := param.Value
-	if oldEscape {
-		paramValue = strings.ReplaceAll(paramValue, `"`, `\"`)
+	return substituteParamsInResourceTemplate([]triggersv1.Param{param}, rt, oldEscape)
+}
+
+// substituteParamsInResourceTemplate walks rt once, replacing every
+// $(tt.params.NAME) token with its matching param value. It tracks whether
+// the current scan position is inside a JSON string literal, so that control
+// characters (e.g. a literal newline from a multiline value) are only
+// escaped when they would otherwise land inside a quoted string and break
+// JSON parsing; values substituted outside of a string (e.g. a raw JSON
+// object, see #823) are left untouched. Doing this in a single pass over the
+// original rt - rather than one pass per param - ensures a raw, unescaped
+// quote in one param's value can't desync the string-boundary tracking used
+// for a different param's token later in rt.
+func substituteParamsInResourceTemplate(params []triggersv1.Param, rt json.RawMessage, oldEscape bool) json.RawMessage {
+	tokens := make([][]byte, len(params))
+	rawValues := make([][]byte, len(params))
+	quotedValues := make([][]byte, len(params))
+	for i, param := range params {
+		// Escape quotes so that JSON strings can be appended to regular strings.
+		// See #257 for discussion on this behavior.
+		paramValue := param.Value
+		if oldEscape {
+			paramValue = strings.ReplaceAll(paramValue, `"`, `\"`)
+		}
+		// Escape Tekton variable syntax to prevent validation errors
+		// when parameter values contain literal $(tasks.*) or similar patterns
+		paramValue = escapeTektonVariables(paramValue)
+
+		tokens[i] = []byte(fmt.Sprintf("$(tt.params.%s)", param.Name))
+		rawValues[i] = []byte(paramValue)
+		quotedValues[i] = []byte(escapeJSONControlChars(paramValue))
+	}
+
+	var out bytes.Buffer
+	inString, escaped := false, false
+	for i := 0; i < len(rt); {
+		matched := -1
+		for t, token := range tokens {
+			if bytes.HasPrefix(rt[i:], token) {
+				matched = t
+				break
+			}
+		}
+		if matched != -1 {
+			if inString {
+				out.Write(quotedValues[matched])
+			} else {
+				out.Write(rawValues[matched])
+			}
+			i += len(tokens[matched])
+			escaped = false
+			continue
+		}
+		c := rt[i]
+		out.WriteByte(c)
+		if inString {
+			switch {
+			case escaped:
+				escaped = false
+			case c == '\\':
+				escaped = true
+			case c == '"':
+				inString = false
+			}
+		} else if c == '"' {
+			inString = true
+		}
+		i++
 	}
-	// Escape Tekton variable syntax to prevent validation errors
-	// when parameter values contain literal $(tasks.*) or similar patterns
-	paramValue = escapeTektonVariables(paramValue)
-	return bytes.ReplaceAll(rt, []byte(paramVariable), []byte(paramValue))
+	return out.Bytes()
 }
 
 // UUID generates a Universally Unique IDentifier following RFC 4122.

pkg/template/resource_test.go

@@ -243,6 +243,36 @@ func Test_applyParamToResourceTemplate(t *testing.T) {
 				rt: json.RawMessage(`{"spec": {"params": [{"name": "pr-body", "value": "$(tt.params.pr-body)"}]}}`),
 			},
 			want: json.RawMessage(`{"spec": {"params": [{"name": "pr-body", "value": "name: $$(context.pipelineRun.name)"}]}}`),
+		}, {
+			name: "escape literal newline embedded in a multiline value",
+			args: args{
+				param: triggersv1.Param{
+					Name:  "sdk_config_file",
+					Value: "test1\ntest2\n",
+				},
+				rt: json.RawMessage(`{"script": "echo $(tt.params.sdk_config_file) | tee test.txt"}`),
+			},
+			want: json.RawMessage(`{"script": "echo test1\ntest2\n | tee test.txt"}`),
+		}, {
+			name: "escape literal newline when value fills the whole quoted string",
+			args: args{
+				param: triggersv1.Param{
+					Name:  "sdk_config_file",
+					Value: "test1\ntest2\n",
+				},
+				rt: json.RawMessage(`{"foo": "$(tt.params.sdk_config_file)"}`),
+			},
+			want: json.RawMessage(`{"foo": "test1\ntest2\n"}`),
+		}, {
+			name: "leave literal newline untouched in unquoted (raw JSON) substitution",
+			args: args{
+				param: triggersv1.Param{
+					Name:  "p1",
+					Value: "{\n  \"a\": \"b\"\n}",
+				},
+				rt: json.RawMessage(`{"foo": $(tt.params.p1)}`),
+			},
+			want: json.RawMessage("{\"foo\": {\n  \"a\": \"b\"\n}}"),
 		},
 	}
 	for _, tt := range tests {
@@ -330,6 +360,22 @@ func Test_ApplyParamsToResourceTemplate(t *testing.T) {
 			},
 			want: json.RawMessage(`{"actualParam": "actualValue", "invalidParam": "$(tt.params1.invalidid)", "deprecatedParam": "$(params.twoid)"`),
 		},
+		{
+			// A regression test: an earlier param whose raw (pass-through, see
+			// #823) value contains an odd number of unescaped quotes must not
+			// throw off whether a later param's control characters get
+			// escaped. Both params are substituted into quoted JSON string
+			// positions here.
+			name: "an earlier param with an unescaped quote does not corrupt a later multiline param",
+			args: args{
+				params: []triggersv1.Param{
+					{Name: "msg", Value: `5" screen`},
+					{Name: "log", Value: "line1\nline2"},
+				},
+				rt: json.RawMessage(`{"message": "$(tt.params.msg)", "log": "$(tt.params.log)"}`),
+			},
+			want: json.RawMessage(`{"message": "5" screen", "log": "line1\nline2"}`),
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {