Unescaped backslashes in escapeForTemplateLiteral corrupt JS template literal evaluation

[jeanlaurent]

Summary

• Context: The escapeForTemplateLiteral function in pkg/js/expand.go is used to escape strings before evaluating them as JavaScript template literals, enabling safe variable expansion in agent descriptions, welcome messages, and commands.
• Bug: The function only escapes backticks but does not escape backslashes, causing backslashes in the input to be interpreted as JavaScript escape sequences.
• Actual vs. expected: When input contains backslashes (e.g., C:\Users\Alice or \d+), they are interpreted as escape sequences (producing C:UsersAlice or d+) instead of being preserved literally.
• Impact: Data corruption occurs when expanding strings containing backslashes, affecting agent descriptions, welcome messages, and commands that may include file paths, regex patterns, or escape sequences.

Code with bug

// escapeForTemplateLiteral escapes characters that have special meaning in
// JavaScript template literals
func escapeForTemplateLiteral(s string) string {
	// Escape backticks so they don't terminate the template literal.
	// Also escape backslashes that precede backticks to avoid double-escaping issues.
	s = strings.ReplaceAll(s, "\\`", "\\\\`") // First escape already-escaped backticks
	s = strings.ReplaceAll(s, "`", "\\`")     // Then escape remaining backticks
	return s  // <-- BUG 🔴 Missing: backslashes not preceded by backticks are not escaped
}

Evidence

Example

Consider the input string: path\to\file

1. After line 19 (strings.ReplaceAll(s, "\\", "\\")): No change, since there's no \`` sequence → path\to\file`
2. After line 20 (strings.ReplaceAll(s, "", "\")): No change, since there are no backticks → path\to\file
3. Result: The escaped string is path\to\file (unchanged)
4. In JavaScript: The template literal `path\to\file` interprets \t as a tab character and \f as a form feed
5. Final output: path<TAB>o<FORM_FEED>ile (corrupted)

Expected behavior: All backslashes should be doubled (path\\to\\file) so the JavaScript template literal preserves them literally.

Inconsistency with API documentation

Reference API documentation

JavaScript template literals follow the same escape sequence rules as regular strings. From the ECMAScript specification and MDN documentation:

JavaScript Escape Sequences in Template Literals:

• \n → newline
• \t → tab
• \\ → literal backslash
• ``` → literal backtick
• \$ → literal dollar sign (when followed by {)

To preserve a literal backslash, it must be escaped as \\.

Reference: MDN - Template literals

Current code usage

func (exp *Expander) Expand(ctx context.Context, text string) string {
	vm := exp.jsRuntime(ctx)

	result, err := vm.RunString("`" + escapeForTemplateLiteral(text) + "`")  // <-- BUG 🔴
	if err != nil {
		return text
	}

	return fmt.Sprintf("%v", result.Export())
}

Contradiction

The escapeForTemplateLiteral function must escape all backslashes to prevent them from being interpreted as escape sequences. Currently, it only escapes backslashes immediately before backticks (line 19), leaving all other backslashes unescaped. This violates JavaScript's template literal escaping rules.

Failing test

Test script

package js

import (
	"context"
	"testing"

	"github.com/stretchr/testify/assert"
)

func TestExpand_Backslashes(t *testing.T) {
	t.Parallel()

	tests := []struct {
		name     string
		input    string
		expected string
	}{
		{
			name:     "single backslash",
			input:    "test\\value",
			expected: "test\\value",
		},
		{
			name:     "backslash n (not newline)",
			input:    "test\\nvalue",
			expected: "test\\nvalue",
		},
		{
			name:     "backslash t (not tab)",
			input:    "test\\tvalue",
			expected: "test\\tvalue",
		},
		{
			name:     "windows path",
			input:    "C:\\Users\\Alice\\Documents",
			expected: "C:\\Users\\Alice\\Documents",
		},
		{
			name:     "network path",
			input:    "\\\\server\\share\\file",
			expected: "\\\\server\\share\\file",
		},
		{
			name:     "multiple backslashes",
			input:    "test\\\\value",
			expected: "test\\\\value",
		},
		{
			name:     "regex pattern with backslashes",
			input:    "\\d+\\.\\d+",
			expected: "\\d+\\.\\d+",
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			t.Parallel()

			env := testEnvProvider(map[string]string{})
			expander := NewJsExpander(&env)

			result := expander.Expand(context.Background(), tt.input)

			assert.Equal(t, tt.expected, result)
		})
	}
}

Test output

=== RUN   TestExpand_Backslashes
=== PAUSE TestExpand_Backslashes
=== CONT  TestExpand_Backslashes
=== RUN   TestExpand_Backslashes/single_backslash
=== PAUSE TestExpand_Backslashes/single_backslash
=== RUN   TestExpand_Backslashes/backslash_n_(not_newline)
=== PAUSE TestExpand_Backslashes/backslash_n_(not_newline)
=== RUN   TestExpand_Backslashes/backslash_t_(not_tab)
=== PAUSE TestExpand_Backslashes/backslash_t_(not_tab)
=== RUN   TestExpand_Backslashes/windows_path
=== PAUSE TestExpand_Backslashes/windows_path
=== RUN   TestExpand_Backslashes/network_path
=== PAUSE TestExpand_Backslashes/network_path
=== RUN   TestExpand_Backslashes/trailing_backslash
=== PAUSE TestExpand_Backslashes/trailing_backslash
=== RUN   TestExpand_Backslashes/multiple_backslashes
=== PAUSE TestExpand_Backslashes/multiple_backslashes
=== RUN   TestExpand_Backslashes/regex_pattern_with_backslashes
=== PAUSE TestExpand_Backslashes/regex_pattern_with_backslashes
=== CONT  TestExpand_Backslashes/single_backslash
    expand_backslash_test.go:69:
        	Error Trace:	/home/user/cagent/pkg/js/expand_backslash_test.go:69
        	Error:      	Not equal:
        	            	expected: "test\\value"
        	            	actual  : "test\value"

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-test\value
        	            	+testalue
        	Test:       	TestExpand_Backslashes/single_backslash
=== CONT  TestExpand_Backslashes/regex_pattern_with_backslashes
    expand_backslash_test.go:69:
        	Error Trace:	/home/user/cagent/pkg/js/expand_backslash_test.go:69
        	Error:      	Not equal:
        	            	expected: "\\d+\\.\\d+"
        	            	actual  : "d+.d+"

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-\d+\.\d+
        	            	+d+.d+
        	Test:       	TestExpand_Backslashes/regex_pattern_with_backslashes
=== CONT  TestExpand_Backslashes/multiple_backslashes
    expand_backslash_test.go:69:
        	Error Trace:	/home/user/cagent/pkg/js/expand_backslash_test.go:69
        	Error:      	Not equal:
        	            	expected: "test\\\\value"
        	            	actual  : "test\\value"

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-test\\value
        	            	+test\value
        	Test:       	TestExpand_Backslashes/multiple_backslashes
=== CONT  TestExpand_Backslashes/trailing_backslash
=== CONT  TestExpand_Backslashes/network_path
    expand_backslash_test.go:69:
        	Error Trace:	/home/user/cagent/pkg/js/expand_backslash_test.go:69
        	Error:      	Not equal:
        	            	expected: "\\\\server\\share\\file"
        	            	actual  : "\\servershare\file"

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-\\server\share\file
        	            	+\servershareile
        	Test:       	TestExpand_Backslashes/network_path
=== CONT  TestExpand_Backslashes/windows_path
    expand_backslash_test.go:69:
        	Error Trace:	/home/user/cagent/pkg/js/expand_backslash_test.go:69
        	Error:      	Not equal:
        	            	expected: "C:\\Users\\Alice\\Documents"
        	            	actual  : "C:UsersAliceDocuments"

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-C:\Users\Alice\Documents
        	            	+C:UsersAliceDocuments
        	Test:       	TestExpand_Backslashes/windows_path
=== CONT  TestExpand_Backslashes/backslash_t_(not_tab)
=== CONT  TestExpand_Backslashes/backslash_n_(not_newline)
    expand_backslash_test.go:69:
        	Error Trace:	/home/user/cagent/pkg/js/expand_backslash_test.go:69
        	Error:      	Not equal:
        	            	expected: "test\\nvalue"
        	            	actual  : "test\nvalue"

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1,2 @@
        	            	-test\nvalue
        	            	+test
        	            	+value
        	Test:       	TestExpand_Backslashes/backslash_n_(not_newline)
=== NAME  TestExpand_Backslashes/backslash_t_(not_tab)
    expand_backslash_test.go:69:
        	Error Trace:	/home/user/cagent/pkg/js/expand_backslash_test.go:69
        	Error:      	Not equal:
        	            	expected: "test\\tvalue"
        	            	actual  : "test\tvalue"

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-test\tvalue
        	            	+test	value
        	Test:       	TestExpand_Backslashes/backslash_t_(not_tab)
--- FAIL: TestExpand_Backslashes (0.00s)
    --- FAIL: TestExpand_Backslashes/single_backslash (0.00s)
    --- FAIL: TestExpand_Backslashes/regex_pattern_with_backslashes (0.00s)
    --- FAIL: TestExpand_Backslashes/multiple_backslashes (0.00s)
    --- PASS: TestExpand_Backslashes/trailing_backslash (0.00s)
    --- FAIL: TestExpand_Backslashes/network_path (0.00s)
    --- FAIL: TestExpand_Backslashes/windows_path (0.00s)
    --- FAIL: TestExpand_Backslashes/backslash_n_(not_newline) (0.00s)
    --- FAIL: TestExpand_Backslashes/backslash_t_(not_tab) (0.00s)
FAIL
FAIL	github.com/docker/cagent/pkg/js	0.005s
FAIL

Full context

The escapeForTemplateLiteral function is part of the JavaScript template expansion system used throughout the cagent codebase. It was introduced in commit c51dcb9a ("escape js template literals") to handle backticks in markdown code blocks.

Where it's used

1. Expander.Expand() - Expands agent descriptions and welcome messages with environment variables

   • Called from pkg/teamloader/teamloader.go lines 101-102:
     • agent.WithDescription(expander.Expand(ctx, agentConfig.Description))
     • agent.WithWelcomeMessage(expander.Expand(ctx, agentConfig.WelcomeMessage))

2. Expander.ExpandMap() - Expands agent commands and API headers

   • Called from pkg/teamloader/teamloader.go line 108:
     • agent.WithCommands(expander.ExpandMap(ctx, agentConfig.Commands))
   • Called from pkg/teamloader/registry.go lines 173, 233:
     • Expanding API headers for API tools and A2A tools

3. ExpandString() - Expands template strings with arbitrary values

   • Called from pkg/rag/strategy/semantic_embeddings.go line 248:
     • Expanding semantic embedding prompts that include code content via the ${content} variable
   • Called from pkg/tools/builtin/api.go line 46:
     • Expanding API endpoint URLs with parameters

How JavaScript template expansion works

The expansion system wraps input strings in JavaScript template literals (backticks) and evaluates them using the goja JavaScript runtime. For example:

• Input: "Say hello to ${env.USER}"
• Wrapped: `Say hello to ${env.USER}`
• Evaluated: "Say hello to alice" (if env.USER = "alice")

The escapeForTemplateLiteral function is responsible for escaping any special characters in the input string before wrapping it, so they're treated as literal text rather than code.

The bug's manifestation

When the input text contains backslashes directly in the template (not in JavaScript variables), those backslashes are interpreted as escape sequences:

• \n becomes a newline
• \t becomes a tab
• \r becomes a carriage return
• \\ becomes a single backslash
• Unknown sequences like \a have the backslash silently removed

This affects:

• Agent descriptions/welcome messages containing backslashes
• Commands containing backslashes
• API headers containing backslashes (though unlikely)

Note: The ExpandString() function is less affected in practice because values are typically passed as JavaScript variables (via vm.Set()), and JavaScript preserves backslashes in variable values. However, if the template string itself contains backslashes (outside of ${} expressions), those will still be corrupted.

External documentation

• MDN - Template literals

Template literals are enclosed by backtick (`) characters instead of double or single quotes.

Along with having normal strings, template literals can also contain other parts called placeholders, which are embedded expressions delimited by a dollar sign and curly braces: ${expression}.

Template literals are string literals that can span multiple lines and have embedded expressions. They are enclosed by the back-tick (`) character instead of double or single quotes, and they can contain placeholders indicated by the dollar sign and curly braces (${expression}).

Like regular string literals, template literals can include escape sequences:
- \n: newline
- \t: tab
- \\: backslash
- \`: backtick
- \${: literal ${ sequence

• MDN - Escape sequences

JavaScript strings can include escape sequences. A backslash followed by a character produces an escape sequence. Common escape sequences include:
- \0: null character
- \b: backspace
- \t: horizontal tab
- \n: line feed (newline)
- \v: vertical tab
- \f: form feed
- \r: carriage return
- \\: backslash

Why has this bug gone undetected?

Several factors have allowed this bug to go undetected:

1. Limited backslash usage in typical agent configs: Agent descriptions and welcome messages are typically natural language prose without backslashes. Looking at example configs like examples/podcastgenerator_githubmodel.yaml, descriptions are plain English sentences.

2. Error handling masks some cases: When a trailing backslash causes a JavaScript syntax error (e.g., test\ becomes `test\` with an unterminated escape), the error handler returns the original text unchanged (see line 81 in expand.go). This makes trailing backslash cases appear to work correctly, though for the wrong reason.

3. Variable values work correctly: In ExpandString(), values passed as parameters are set as JavaScript variables (line 91), which preserves backslashes correctly. Only the template string itself is subject to corruption. Since most dynamic content (like code chunks) is passed as variables, not in the template, this masks the issue.

4. Test gaps: The existing tests in expand_test.go added with commit c51dcb9a focus on backtick handling and don't test backslashes that aren't followed by backticks. The test cases include:

   • Backticks in markdown code fences (lines 63-67)
   • Multiple backticks (lines 69-73)
   • Backticks in values for ExpandString (lines 167-183)

   None test standalone backslashes like \n, \t, or Windows paths.

5. Platform-specific: The most obvious manifestation would be Windows file paths (e.g., C:\Users\Alice), but development and testing likely occur primarily on Unix-like systems where backslashes in paths are rare.

Recommended fix

Replace the escapeForTemplateLiteral function with proper escaping:

func escapeForTemplateLiteral(s string) string {
	// Escape backslashes first (must be done before backticks)
	s = strings.ReplaceAll(s, "\\", "\\\\")  // <-- FIX 🟢
	// Then escape backticks
	s = strings.ReplaceAll(s, "`", "\\`")    // <-- FIX 🟢
	return s
}

Order matters: Backslashes must be escaped first. If backticks were escaped first, then the backslashes in the resulting \`` would get doubled, producing \ instead of the intended `\.

Alternative approach for completeness (also escaping ${} expressions, though this would change the current behavior where ${} is intentionally evaluated):

func escapeForTemplateLiteral(s string) string {
	s = strings.ReplaceAll(s, "\\", "\\\\")
	s = strings.ReplaceAll(s, "`", "\\`")
	s = strings.ReplaceAll(s, "${", "\\${")  // Optional: prevent template expression evaluation
	return s
}

The current behavior intentionally allows ${} expressions (e.g., ${env.USER}) to be evaluated, so the ${} escaping should likely not be added unless the intended behavior changes.