Reject dangling and non-absolute paths in AttachedFiles

Two correctness fixes uncovered while reviewing the AttachedFiles plumbing:

1. App.Run was recording att.FilePath on the session before

   processFileAttachment had a chance to verify the file existed or

   was a regular file. A user typing @some/missing/path or

   @some/directory in the editor would silently propagate that

   broken reference to every sub-agent created in the rest of the

   session.

   processFileAttachment now returns a bool meaning "the path resolves

   to a real, regular file we attempted to surface" and App.Run only

   records on true. Content-handling failures (too large to inline,

   unsupported MIME, transient read errors) still record the path

   because a sub-agent may have larger limits or different tools than

   the parent.

2. AddAttachedFile is documented as taking an absolute path but

   accepted any string. A buggy caller could leak a relative path

   into the sub-agent's system prompt, where it would be ambiguous.

   Add a filepath.IsAbs guard that silently drops non-absolute paths

   (with a debug log) and update the doc comment.

Also clarify the Session.AttachedFiles field comment to mention all

three entry points (editor @-mention, /attach directive, --attach CLI

flag) and extend TestAddAttachedFile / TestWithAttachedFiles to cover

the new validation.

Assisted-By: docker-agent

Author: dgageot

pkg/app/app.go

@@ -302,12 +302,14 @@ func (a *App) Run(ctx context.Context, cancel context.CancelFunc, message string
 			for _, att := range attachments {
 				switch {
 				case att.FilePath != "":
-					// The editor resolves @-mentions to absolute paths before this
-					// point; remember them so sub-agents created via task transfer
-					// inherit the same file references.
-					a.session.AddAttachedFile(att.FilePath)
 					// File-reference attachment: read and classify from disk.
-					a.processFileAttachment(ctx, att, &textBuilder, &binaryParts)
+					// Only remember the path on the session when the file actually
+					// exists as a regular file — we don't want sub-agents to inherit
+					// dangling references to directories or missing paths. The editor
+					// resolves @-mentions to absolute paths before this point.
+					if a.processFileAttachment(ctx, att, &textBuilder, &binaryParts) {
+						a.session.AddAttachedFile(att.FilePath)
+					}
 				case att.Content != "":
 					// Inline content attachment (e.g. pasted text).
 					a.processInlineAttachment(att, &textBuilder)
@@ -348,7 +350,14 @@ func (a *App) Run(ctx context.Context, cancel context.CancelFunc, message string
 
 // processFileAttachment reads a file from disk, classifies it, and either
 // appends its text content to textBuilder or adds a binary part to binaryParts.
-func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment, textBuilder *strings.Builder, binaryParts *[]chat.MessagePart) {
+// Returns true when the path resolved to a real, regular file that we attempted
+// to surface to the model — even if the content itself was rejected (too
+// large, unsupported MIME, transient read error, etc.). The boolean is meant
+// for callers that want to record the path on the session for later reuse by
+// sub-agents; we don't want those references to point at directories or
+// missing files, but we do want them to cover "the agent has bigger tools
+// than us" cases.
+func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment, textBuilder *strings.Builder, binaryParts *[]chat.MessagePart) bool {
 	absPath := att.FilePath
 
 	fi, err := os.Stat(absPath)
@@ -364,20 +373,20 @@ func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment
 		}
 		slog.Warn("skipping attachment", "path", absPath, "reason", reason)
 		a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: %s", att.Name, reason), ""))
-		return
+		return false
 	}
 
 	if !fi.Mode().IsRegular() {
 		slog.Warn("skipping attachment: not a regular file", "path", absPath, "mode", fi.Mode().String())
 		a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: not a regular file", att.Name), ""))
-		return
+		return false
 	}
 
 	const maxAttachmentSize = 100 * 1024 * 1024 // 100MB
 	if fi.Size() > maxAttachmentSize {
 		slog.Warn("skipping attachment: file too large", "path", absPath, "size", fi.Size(), "max", maxAttachmentSize)
 		a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: file too large (max 100MB)", att.Name), ""))
-		return
+		return true
 	}
 
 	mimeType := chat.DetectMimeType(absPath)
@@ -387,13 +396,13 @@ func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment
 		if fi.Size() > chat.MaxInlineFileSize {
 			slog.Warn("skipping attachment: text file too large to inline", "path", absPath, "size", fi.Size(), "max", chat.MaxInlineFileSize)
 			a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: text file too large to inline (max 5MB)", att.Name), ""))
-			return
+			return true
 		}
 		content, err := chat.ReadFileForInline(absPath)
 		if err != nil {
 			slog.Warn("skipping attachment: failed to read file", "path", absPath, "error", err)
 			a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: failed to read file", att.Name), ""))
-			return
+			return true
 		}
 		textBuilder.WriteString("\n\n")
 		textBuilder.WriteString(content)
@@ -406,14 +415,14 @@ func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment
 			if readErr != nil {
 				slog.Warn("skipping attachment: failed to read image", "path", absPath, "error", readErr)
 				a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: failed to read image", att.Name), ""))
-				return
+				return true
 			}
 			resized, resizeErr := chat.ResizeImage(imgData, mimeType)
 			if resizeErr != nil {
 				// Don't bypass security checks - reject the file if resize failed
 				slog.Warn("skipping attachment: image resize failed", "path", absPath, "error", resizeErr)
 				a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: %s", att.Name, resizeErr), ""))
-				return
+				return true
 			}
 			dataURL := fmt.Sprintf("data:%s;base64,%s", resized.MimeType, base64.StdEncoding.EncodeToString(resized.Data))
 			*binaryParts = append(*binaryParts, chat.MessagePart{
@@ -441,6 +450,8 @@ func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment
 		slog.Warn("skipping attachment: unsupported file type", "path", absPath, "mime_type", mimeType)
 		a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: unsupported file type", att.Name), ""))
 	}
+
+	return true
 }
 
 // sendEvent sends an event to the TUI, respecting context cancellation to

pkg/session/session.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"log/slog"
+	"path/filepath"
 	"slices"
 	"strings"
 	"sync"
@@ -137,10 +138,11 @@ type Session struct {
 	CustomModelsUsed []string `json:"custom_models_used,omitempty"`
 
 	// AttachedFiles records absolute paths of files the user attached to this
-	// session via @-mentions in the editor or /attach directives. Sub-sessions
-	// created via task transfer inherit this list so that delegated agents can
-	// reference the same files without having to scan the workspace or guess
-	// from a bare filename. Paths are deduplicated and order-preserved.
+	// session via the editor's @-mentions, the in-message /attach directive, or
+	// the CLI --attach flag. Sub-sessions created via task transfer inherit
+	// this list so that delegated agents can reference the same files without
+	// having to scan the workspace or guess from a bare filename. Paths are
+	// deduplicated and order-preserved.
 	AttachedFiles []string `json:"attached_files,omitempty"`
 
 	// ExcludedTools lists tool names that should be filtered out of the agent's
@@ -489,14 +491,22 @@ func (s *Session) AddMessageUsageRecord(agentName, model string, cost float64, u
 }
 
 // AddAttachedFile records absPath as a file the user attached to this session.
-// Empty paths are ignored; duplicates (already in AttachedFiles) are dropped.
+// The path must be absolute; relative paths are silently dropped (with a debug
+// log) since they would be ambiguous to sub-agents started in a fresh working
+// directory. Empty paths and duplicates already present in AttachedFiles are
+// also dropped.
+//
 // The recorded paths are propagated to sub-sessions created via task transfer
 // so that delegated agents can read the same files without having to scan the
 // workspace or guess from a bare filename.
 func (s *Session) AddAttachedFile(absPath string) {
 	if absPath == "" {
 		return
 	}
+	if !filepath.IsAbs(absPath) {
+		slog.Debug("ignoring non-absolute attached file path", "session_id", s.ID, "path", absPath)
+		return
+	}
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	if slices.Contains(s.AttachedFiles, absPath) {

pkg/session/session_options_test.go

@@ -83,6 +83,14 @@ func TestAddAttachedFile(t *testing.T) {
 		assert.Empty(t, s.AttachedFilesSnapshot())
 	})
 
+	t.Run("ignores non-absolute paths", func(t *testing.T) {
+		s := New()
+		s.AddAttachedFile("foo.go")
+		s.AddAttachedFile("./bar.go")
+		s.AddAttachedFile("../baz.go")
+		assert.Empty(t, s.AttachedFilesSnapshot())
+	})
+
 	t.Run("snapshot is independent of session storage", func(t *testing.T) {
 		s := New()
 		s.AddAttachedFile("/abs/foo.go")
@@ -93,6 +101,6 @@ func TestAddAttachedFile(t *testing.T) {
 }
 
 func TestWithAttachedFiles(t *testing.T) {
-	s := New(WithAttachedFiles([]string{"/abs/foo.go", "", "/abs/bar.go", "/abs/foo.go"}))
+	s := New(WithAttachedFiles([]string{"/abs/foo.go", "", "relative/path.go", "/abs/bar.go", "/abs/foo.go"}))
 	assert.Equal(t, []string{"/abs/foo.go", "/abs/bar.go"}, s.AttachedFilesSnapshot())
 }