diff --git a/internal/agent/loop_media.go b/internal/agent/loop_media.go index ab6e1c6c..42255d42 100644 --- a/internal/agent/loop_media.go +++ b/internal/agent/loop_media.go @@ -65,9 +65,17 @@ func extractMediaFromContent(content, workspace string) []MediaResult { if len(matches) == 0 { return nil } + // Resolve workspace to its real path (follows symlinks). Required because + // macOS uses symlinks for /tmp → /private/tmp; if we only Clean the + // workspace but EvalSymlinks the extracted paths, the Rel check below + // would spuriously fail even for legitimate files. wsRoot := "" if abs, err := filepath.Abs(workspace); err == nil { - wsRoot = filepath.Clean(abs) + if resolved, err := filepath.EvalSymlinks(abs); err == nil { + wsRoot = filepath.Clean(resolved) + } else { + wsRoot = filepath.Clean(abs) + } } if wsRoot == "" { return nil @@ -90,18 +98,24 @@ func extractMediaFromContent(content, workspace string) []MediaResult { path = filepath.Join(wsRoot, path) } cleaned := filepath.Clean(path) - // Workspace containment check — blocks "../" escapes and absolute paths - // outside the agent's allowed area. - rel, err := filepath.Rel(wsRoot, cleaned) - if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - continue - } - // Existence + regular-file check prevents hallucinated paths and - // directory/symlink-device shenanigans from reaching the outbound queue. + // Existence + regular-file check. Lstat (not Stat) so a symlink at + // the leaf is rejected outright. info, err := os.Lstat(cleaned) if err != nil || !info.Mode().IsRegular() { continue } + // Resolve ancestor symlinks THEN check containment. A purely lexical + // Rel check would pass "//secret" when symlink-dir + // points outside the workspace; EvalSymlinks closes that escape. + resolved, err := filepath.EvalSymlinks(cleaned) + if err != nil { + continue + } + rel, err := filepath.Rel(wsRoot, resolved) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + continue + } + cleaned = resolved if _, dup := seen[cleaned]; dup { continue } @@ -114,7 +128,12 @@ func extractMediaFromContent(content, workspace string) []MediaResult { return results } -// deduplicateMedia removes duplicate media results by path, keeping the first occurrence. +// deduplicateMedia removes duplicate media results, normalizing each path +// so legacy entries from parseMediaResult (raw tool output — may contain +// "./" segments or relative forms) collapse cleanly with entries from +// extractMediaFromContent (already Clean'd + Abs'd). Prevents double-send +// when a tool returns MEDIA:./x.mp3 and the LLM echoes it back in a form +// that resolves to the same file. func deduplicateMedia(media []MediaResult) []MediaResult { if len(media) <= 1 { return media @@ -122,10 +141,16 @@ func deduplicateMedia(media []MediaResult) []MediaResult { seen := make(map[string]bool, len(media)) result := make([]MediaResult, 0, len(media)) for _, m := range media { - if seen[m.Path] { + key := m.Path + if abs, err := filepath.Abs(key); err == nil { + key = filepath.Clean(abs) + } else { + key = filepath.Clean(key) + } + if seen[key] { continue } - seen[m.Path] = true + seen[key] = true result = append(result, m) } return result diff --git a/internal/agent/loop_media_test.go b/internal/agent/loop_media_test.go index 4d4d65eb..7b32d06e 100644 --- a/internal/agent/loop_media_test.go +++ b/internal/agent/loop_media_test.go @@ -20,7 +20,13 @@ func writeTempFile(t *testing.T, workspace, relPath string) string { } func TestExtractMediaFromContent(t *testing.T) { - ws := t.TempDir() + wsRaw := t.TempDir() + // Resolve workspace symlinks up front (macOS has /var → /private/var) so + // expected paths match what the extractor returns after EvalSymlinks. + ws, err := filepath.EvalSymlinks(wsRaw) + if err != nil { + t.Fatal(err) + } reportPath := writeTempFile(t, ws, "deliver/report.pdf") audioA := writeTempFile(t, ws, "a.mp3") audioB := writeTempFile(t, ws, "b.mp3") @@ -33,6 +39,23 @@ func TestExtractMediaFromContent(t *testing.T) { t.Fatal(err) } + // Symlink inside workspace pointing to outside: must be rejected by + // EvalSymlinks-then-Rel containment. Covers the P0 ancestor-symlink + // escape the lexical-only Rel check would have allowed. + symlinkFile := filepath.Join(ws, "shortcut-to-leak.pdf") + if err := os.Symlink(outsidePath, symlinkFile); err != nil { + t.Skipf("symlink not supported: %v", err) + } + // Ancestor symlink case: dir symlink inside ws pointing outside. + symDirParent := t.TempDir() + if err := os.WriteFile(filepath.Join(symDirParent, "victim.pdf"), nil, 0o644); err != nil { + t.Fatal(err) + } + ancestorSym := filepath.Join(ws, "shared") + if err := os.Symlink(symDirParent, ancestorSym); err != nil { + t.Skipf("symlink not supported: %v", err) + } + tests := []struct { name string content string @@ -84,6 +107,16 @@ func TestExtractMediaFromContent(t *testing.T) { name: "absolute path with no workspace dropped", content: "MEDIA:" + reportPath, }, + { + name: "symlink leaf rejected by Lstat", + content: "MEDIA:shortcut-to-leak.pdf", + workspace: ws, + }, + { + name: "ancestor symlink escape blocked (P0)", + content: "MEDIA:shared/victim.pdf", + workspace: ws, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/tools/tts.go b/internal/tools/tts.go index e756de57..ed89cf49 100644 --- a/internal/tools/tts.go +++ b/internal/tools/tts.go @@ -242,11 +242,17 @@ func (t *TtsTool) Execute(ctx context.Context, args map[string]any) *Result { // Set Result.Media explicitly (matching create_audio) so the agent loop's // media collector uses the authoritative path even when the ForLLM // MEDIA: prefix is reshaped by a provider bridge (e.g. claude_cli MCP). + // Prefer the provider-supplied MimeType ("audio/mpeg", "audio/ogg") over + // "audio/"+Extension — the latter yields the non-standard "audio/mp3". + mimeType := result.MimeType + if mimeType == "" { + mimeType = "audio/" + result.Extension + } r := &Result{ ForLLM: forLLM, Media: []bus.MediaFile{{ Path: audioPath, - MimeType: "audio/" + result.Extension, + MimeType: mimeType, }}, } r.Deliverable = fmt.Sprintf("[Generated audio: %s]\nText: %s", filepath.Base(audioPath), text)