fix: escape shell metacharacters in expanded env files

internal/cli/run.go

@@ -950,11 +950,14 @@ func bootstrapEnv(sandboxName, repoDir string, h *harness.Harness) error {
 
 		if hf.Expand {
 			// Read file, expand ${VAR} in content, write expanded version.
+			// Uses shell-safe quoting so user-authored values (e.g.
+			// HUMAN_INSTRUCTION) containing shell metacharacters do not
+			// cause syntax errors when the file is sourced. (#408, #615)
 			raw, err := os.ReadFile(hostPath)
 			if err != nil {
 				return fmt.Errorf("reading host file %s for expansion: %w", hf.Src, err)
 			}
-			expanded := os.ExpandEnv(string(raw))
+			expanded := shellSafeExpandEnv(string(raw))
 
 			tmp, err := os.CreateTemp("", "fullsend-expand-*")
 			if err != nil {
@@ -994,6 +997,30 @@ func bootstrapEnv(sandboxName, repoDir string, h *harness.Harness) error {
 	return nil
 }
 
+// shellSafeExpandEnv expands ${VAR} references in text using the host
+// environment, escaping characters that are special inside double quotes
+// (", $, `, \) so the result is safe to source as a shell script.
+// Templates use the standard export FOO="${FOO}" pattern; this function
+// ensures substituted values cannot break out of the double-quote context.
+// Fixes #408, #615.
+func shellSafeExpandEnv(text string) string {
+	return os.Expand(text, func(key string) string {
+		return escapeForDoubleQuotes(os.Getenv(key))
+	})
+}
+
+// escapeForDoubleQuotes escapes the four characters that have special
+// meaning inside double-quoted shell strings: backslash, double quote,
+// dollar sign, and backtick. Order matters: backslash must be escaped
+// first to avoid double-escaping the others.
+func escapeForDoubleQuotes(s string) string {
+	s = strings.ReplaceAll(s, `\`, `\\`)
+	s = strings.ReplaceAll(s, `"`, `\"`)
+	s = strings.ReplaceAll(s, `$`, `\$`)
+	s = strings.ReplaceAll(s, "`", "\\`")
+	return s
+}
+
 // envToList converts a map of env vars to a sorted list of KEY=VALUE strings.
 func envToList(env map[string]string) []string {
 	keys := make([]string, 0, len(env))

internal/cli/run_test.go

@@ -13,6 +13,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -430,6 +431,209 @@ func TestEnvToList_Sorted(t *testing.T) {
 	assert.Equal(t, "Z_VAR=z", list[2])
 }
 
+func TestShellSafeExpandEnv(t *testing.T) {
+	tests := []struct {
+		name     string
+		template string
+		env      map[string]string
+		want     string
+	}{
+		{
+			name:     "simple value",
+			template: `export FOO="${FOO}"`,
+			env:      map[string]string{"FOO": "bar"},
+			want:     `export FOO="bar"`,
+		},
+		{
+			name:     "value with double quotes",
+			template: `export MSG="${MSG}"`,
+			env:      map[string]string{"MSG": `say "hello"`},
+			want:     `export MSG="say \"hello\""`,
+		},
+		{
+			name:     "value with parentheses",
+			template: `export MSG="${MSG}"`,
+			env:      map[string]string{"MSG": "fix (example) thing"},
+			want:     `export MSG="fix (example) thing"`,
+		},
+		{
+			name:     "value with single quotes",
+			template: `export MSG="${MSG}"`,
+			env:      map[string]string{"MSG": "it's broken"},
+			want:     `export MSG="it's broken"`,
+		},
+		{
+			name:     "value with dollar sign",
+			template: `export V="${V}"`,
+			env:      map[string]string{"V": "$HOME"},
+			want:     `export V="\$HOME"`,
+		},
+		{
+			name:     "value with backticks",
+			template: `export CMD="${CMD}"`,
+			env:      map[string]string{"CMD": "use `grep` here"},
+			want:     "export CMD=\"use \\`grep\\` here\"",
+		},
+		{
+			name:     "value with backslashes",
+			template: `export P="${P}"`,
+			env:      map[string]string{"P": `C:\Users\test`},
+			want:     `export P="C:\\Users\\test"`,
+		},
+		{
+			name:     "value with all four special chars",
+			template: `export V="${V}"`,
+			env:      map[string]string{"V": "a]\" $x `y` \\z"},
+			want:     `export V="a]\" \$x ` + "\\`y\\`" + ` \\z"`,
+		},
+		{
+			name:     "value with shell metacharacters safe inside double quotes",
+			template: `export CMD="${CMD}"`,
+			env:      map[string]string{"CMD": "foo || true && bar; baz > /dev/null"},
+			want:     `export CMD="foo || true && bar; baz > /dev/null"`,
+		},
+		{
+			name:     "empty value",
+			template: `export FOO="${FOO}"`,
+			env:      map[string]string{"FOO": ""},
+			want:     `export FOO=""`,
+		},
+		{
+			name:     "undefined variable",
+			template: `export FOO="${UNDEFINED}"`,
+			env:      map[string]string{},
+			want:     `export FOO=""`,
+		},
+		{
+			name:     "static lines unchanged",
+			template: "export STATIC='hello world'",
+			env:      map[string]string{},
+			want:     "export STATIC='hello world'",
+		},
+		{
+			name:     "multiple variables",
+			template: "export A=\"${A}\"\nexport B=\"${B}\"",
+			env:      map[string]string{"A": "1", "B": "two (2)"},
+			want:     "export A=\"1\"\nexport B=\"two (2)\"",
+		},
+		{
+			name:     "unquoted template still safe",
+			template: "export FOO=${FOO}",
+			env:      map[string]string{"FOO": "bar"},
+			want:     "export FOO=bar",
+		},
+		{
+			name:     "real-world HUMAN_INSTRUCTION from issue 615",
+			template: `export HUMAN_INSTRUCTION="${HUMAN_INSTRUCTION}"`,
+			env:      map[string]string{"HUMAN_INSTRUCTION": `replacing --search "$ISSUE_NUMBER in:body,title" with timeline API || true`},
+			want:     `export HUMAN_INSTRUCTION="replacing --search \"\$ISSUE_NUMBER in:body,title\" with timeline API || true"`,
+		},
+		{
+			name:     "real-world instruction with parentheses from failing run",
+			template: `export HUMAN_INSTRUCTION="${HUMAN_INSTRUCTION}"`,
+			env:      map[string]string{"HUMAN_INSTRUCTION": `An administrator with elevated access to the GCP project (for example, with the ability to set IAM policy) can grant all required roles`},
+			want:     `export HUMAN_INSTRUCTION="An administrator with elevated access to the GCP project (for example, with the ability to set IAM policy) can grant all required roles"`,
+		},
+		{
+			name:     "injection attempt: break out of double quotes",
+			template: `export V="${V}"`,
+			env:      map[string]string{"V": `"; rm -rf /; echo "`},
+			want:     `export V="\"; rm -rf /; echo \""`,
+		},
+		{
+			name:     "injection attempt: command substitution",
+			template: `export V="${V}"`,
+			env:      map[string]string{"V": `$(cat /etc/passwd)`},
+			want:     `export V="\$(cat /etc/passwd)"`,
+		},
+		{
+			name:     "injection attempt: backtick substitution",
+			template: `export V="${V}"`,
+			env:      map[string]string{"V": "`cat /etc/passwd`"},
+			want:     "export V=\"\\`cat /etc/passwd\\`\"",
+		},
+		{
+			name:     "newlines in value",
+			template: `export V="${V}"`,
+			env:      map[string]string{"V": "line1\nline2\nline3"},
+			want:     "export V=\"line1\nline2\nline3\"",
+		},
+		{
+			name:     "tabs and special whitespace",
+			template: `export V="${V}"`,
+			env:      map[string]string{"V": "col1\tcol2"},
+			want:     "export V=\"col1\tcol2\"",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			for k, v := range tt.env {
+				t.Setenv(k, v)
+			}
+			got := shellSafeExpandEnv(tt.template)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+// TestShellSafeExpandEnv_ShellRoundtrip verifies that expanded env files
+// produce the original value when sourced by a real shell. This is the
+// definitive safety test: if the value survives a roundtrip through
+// sh -c '. file && printf "%s" "$VAR"', the escaping is correct.
+func TestShellSafeExpandEnv_ShellRoundtrip(t *testing.T) {
+	values := []struct {
+		name  string
+		value string
+	}{
+		{"simple", "hello world"},
+		{"double quotes", `say "hello" to "world"`},
+		{"single quotes", "it's a test"},
+		{"parentheses", "fix (example) thing"},
+		{"pipes and logic", "foo || true && bar"},
+		{"dollar sign", "cost is $100 or $HOME"},
+		{"command substitution", "$(rm -rf /)"},
+		{"backtick substitution", "`rm -rf /`"},
+		{"backslashes", `path\to\file`},
+		{"semicolons", "cmd1; cmd2; cmd3"},
+		{"redirects", "echo foo > /tmp/evil"},
+		{"glob chars", "match *.go and file?.txt"},
+		{"mixed injection", `"; $(evil) ` + "`more`" + ` && rm -rf / #`},
+		{"all four special chars", `quote" dollar$ tick` + "`" + ` slash\`},
+		{"newlines", "line1\nline2\nline3"},
+		{"tabs", "col1\tcol2"},
+		{"empty", ""},
+		{"unicode", "こんにちは 🎉"},
+		{"real issue 615", `replacing --search "$ISSUE_NUMBER in:body,title" with timeline API || true`},
+		{"real failing run", `An administrator with elevated access to the GCP project (for example, with the ability to set IAM policy) can grant all required roles with a single script:`},
+		{"already escaped backslash", `already \" escaped`},
+		{"nested quotes", `He said "she said 'hello'" today`},
+		{"hash comment char", "value # not a comment"},
+		{"exclamation mark", "hello! world!"},
+		{"curly braces", "use ${VAR} syntax"},
+		{"square brackets", "array[0] = value"},
+		{"tilde", "~user/path"},
+		{"ampersand", "Tom & Jerry"},
+	}
+
+	for _, tt := range values {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Setenv("TEST_VAL", tt.value)
+			expanded := shellSafeExpandEnv(`export TEST_VAL="${TEST_VAL}"`)
+
+			// Write expanded content to a temp file and source it in sh.
+			envFile := filepath.Join(t.TempDir(), "test.env")
+			require.NoError(t, os.WriteFile(envFile, []byte(expanded+"\n"), 0o644))
+
+			// Use printf "%s" (not echo) to avoid interpretation of \n etc.
+			cmd := exec.Command("sh", "-c", fmt.Sprintf(`. %s && printf '%%s' "$TEST_VAL"`, envFile))
+			out, err := cmd.Output()
+			require.NoError(t, err, "shell failed to source expanded env file; expanded content:\n%s", expanded)
+			assert.Equal(t, tt.value, string(out), "value did not survive shell roundtrip")
+		})
+	}
+}
+
 func TestNeedsCrossCompilation(t *testing.T) {
 	result := needsCrossCompilation()
 	if runtime.GOOS == "linux" {