fix: delimit remote resolve records safely

Author: wesm

internal/ssh/resolve.go

@@ -3,6 +3,7 @@ package ssh
 import (
 	"context"
 	"fmt"
+	"path"
 	"strings"
 
 	"go.kenn.io/agentsview/internal/parser"
@@ -13,6 +14,8 @@ import (
 // is not a valid agent type, so parseResolvedDirs routes it separately.
 const resolveFilePrefix = "@file"
 
+const resolveRecordSep = "\x00"
+
 func aiderSkipDirCasePattern() string {
 	return strings.Join(parser.AiderDiscoverySkipDirNames(), "|")
 }
@@ -34,7 +37,7 @@ func buildAiderResolveSnippet(envVar string) string {
 			"[ \"$av_aider_files\" -ge %d ] && return; "+
 			"[ \"$av_aider_dirs\" -ge %d ] && return; "+
 			"elif [ -f \"$av_entry\" ] && [ \"$av_base\" = '%s' ]; then "+
-			"echo \"%s:$av_entry\"; "+
+			"printf '%%s\\000' \"%s:$av_entry\"; "+
 			"av_aider_files=$((av_aider_files + 1)); "+
 			"[ \"$av_aider_files\" -ge %d ] && return; "+
 			"fi; "+
@@ -100,7 +103,8 @@ func buildResolveScript() string {
 				dirExpr = fmt.Sprintf("${%s:-%s}", def.EnvVar, defaultDir)
 			}
 			fmt.Fprintf(&b,
-				"dir=\"%s\"; [ -d \"$dir\" ] && echo \"%s:$dir\"\n",
+				"dir=\"%s\"; [ -d \"$dir\" ] && "+
+					"printf '%%s\\000' \"%s:$dir\"\n",
 				dirExpr, string(def.Type),
 			)
 			// Codex stores renameable session titles in
@@ -110,7 +114,8 @@ func buildResolveScript() string {
 			if def.Type == parser.AgentCodex {
 				fmt.Fprintf(&b,
 					"idx=\"${dir%%/*}/%s\"; "+
-						"[ -f \"$idx\" ] && echo \"%s:$idx\"\n",
+						"[ -f \"$idx\" ] && "+
+						"printf '%%s\\000' \"%s:$idx\"\n",
 					parser.CodexSessionIndexFilename,
 					resolveFilePrefix,
 				)
@@ -124,23 +129,26 @@ func buildResolveScript() string {
 }
 
 // parseResolvedDirs parses script output into a map of agent type to transfer
-// target paths plus a deduplicated list of extra files (lines tagged with
-// resolveFilePrefix). Most agent targets are directories; Aider targets are
-// individual .aider.chat.history.md files. Skips empty lines and entries with
-// empty values.
+// target paths plus a deduplicated list of extra files (records tagged with
+// resolveFilePrefix). Generated resolver output is NUL-delimited so remote
+// paths containing newlines cannot inject extra records; newline-delimited input
+// is accepted only for older tests and defensive compatibility. Most agent
+// targets are directories; Aider targets are individual .aider.chat.history.md
+// files. Skips empty records, empty values, and values containing record
+// separators.
 func parseResolvedDirs(
 	output string,
 ) (map[parser.AgentType][]string, []string) {
 	dirs := make(map[parser.AgentType][]string)
 	var extraFiles []string
 	seenFile := make(map[string]struct{})
-	for line := range strings.SplitSeq(output, "\n") {
-		line = strings.TrimSpace(line)
-		if line == "" {
+	for _, record := range resolveOutputRecords(output) {
+		record = strings.TrimSpace(record)
+		if record == "" {
 			continue
 		}
-		key, value, ok := strings.Cut(line, ":")
-		if !ok || value == "" {
+		key, value, ok := strings.Cut(record, ":")
+		if !ok || invalidResolvedPath(value) {
 			continue
 		}
 		if key == resolveFilePrefix {
@@ -152,11 +160,26 @@ func parseResolvedDirs(
 			continue
 		}
 		at := parser.AgentType(key)
+		if at == parser.AgentAider &&
+			path.Base(value) != parser.AiderHistoryFileName() {
+			continue
+		}
 		dirs[at] = append(dirs[at], value)
 	}
 	return dirs, extraFiles
 }
 
+func resolveOutputRecords(output string) []string {
+	if strings.Contains(output, resolveRecordSep) {
+		return strings.Split(output, resolveRecordSep)
+	}
+	return strings.Split(output, "\n")
+}
+
+func invalidResolvedPath(value string) bool {
+	return value == "" || strings.ContainsAny(value, "\x00\r\n")
+}
+
 // resolveDirs runs the resolve script on the remote host via SSH and
 // returns the discovered agent directories plus extra sibling files
 // (such as Codex's session_index.jsonl) to include in the transfer.

internal/ssh/resolve_test.go

@@ -23,7 +23,7 @@ func TestBuildResolveScript(t *testing.T) {
 		if def.FileBased || def.DiscoverFunc != nil {
 			continue
 		}
-		marker := "echo \"" + string(def.Type) + ":"
+		marker := "\"" + string(def.Type) + ":"
 		assert.NotContains(t, script, marker,
 			"non-file-based agent %s in script", def.Type)
 	}
@@ -33,7 +33,7 @@ func TestBuildResolveScript(t *testing.T) {
 		if !def.FileBased || def.DiscoverFunc == nil {
 			continue
 		}
-		marker := "echo \"" + string(def.Type) + ":"
+		marker := "\"" + string(def.Type) + ":"
 		assert.Contains(t, script, marker,
 			"file-based agent %s missing from script", def.Type)
 	}
@@ -183,6 +183,30 @@ func TestResolveScriptAiderScopedByEnvFindsHistoryFiles(t *testing.T) {
 		"remote aider discovery must enforce the local depth cap")
 }
 
+func TestResolveScriptAiderNewlinePathCannotInjectTarget(t *testing.T) {
+	home := t.TempDir()
+	codeRoot := filepath.Join(home, "code")
+	injected := "/home/victim/" + parser.AiderHistoryFileName()
+	maliciousDir := filepath.Join(codeRoot, "repo\naider:", "home", "victim")
+	require.NoError(t, os.MkdirAll(maliciousDir, 0o755), "mkdir malicious dir")
+	maliciousHistory := filepath.Join(maliciousDir, parser.AiderHistoryFileName())
+	require.NoError(t, os.WriteFile(maliciousHistory, []byte("# aider\n"), 0o644))
+
+	script := buildResolveScript()
+	cmd := exec.Command("sh", "-c", script)
+	cmd.Env = []string{"HOME=" + home, "AIDER_DIR=" + codeRoot}
+	out, err := cmd.CombinedOutput()
+	require.NoError(t, err, "resolve script failed: output: %s", out)
+
+	dirs, _ := parseResolvedDirs(string(out))
+	assert.NotContains(t, dirs[parser.AgentAider], injected,
+		"newline-bearing repository paths must not inject a second transfer target")
+	for _, target := range dirs[parser.AgentAider] {
+		assert.NotContains(t, target, "\n",
+			"aider transfer target must not contain record separators")
+	}
+}
+
 // TestResolveScriptAiderRejectsHomeOverride verifies that setting AIDER_DIR
 // to literal $HOME (the very thing the home-default skip prevents) is also
 // dropped, so an unscoped override cannot reintroduce a whole-home tar.
@@ -227,3 +251,18 @@ func TestParseResolvedDirs(t *testing.T) {
 	assert.Equal(t,
 		[]string{"/home/wes/.codex/session_index.jsonl"}, extraFiles)
 }
+
+func TestParseResolvedDirsNULRecords(t *testing.T) {
+	input := "claude:/home/wes/.claude/projects\x00" +
+		"aider:/home/wes/code/repo/.aider.chat.history.md\x00" +
+		"@file:/home/wes/.codex/session_index.jsonl\x00"
+
+	dirs, extraFiles := parseResolvedDirs(input)
+
+	assert.Equal(t, []string{"/home/wes/.claude/projects"}, dirs[parser.AgentClaude])
+	assert.Equal(t,
+		[]string{"/home/wes/code/repo/.aider.chat.history.md"},
+		dirs[parser.AgentAider])
+	assert.Equal(t,
+		[]string{"/home/wes/.codex/session_index.jsonl"}, extraFiles)
+}