feat(#3996): keep generated media workspace-only with portable session copies

pkg/acp/agent.go

@@ -635,11 +635,11 @@ func (a *Agent) readResourceLink(ctx context.Context, sessionID string, rl *acp.
 }
 
 func resourceLinkName(rl *acp.ContentBlockResourceLink) string {
-	if rl.Name != "" {
-		return rl.Name
+	if name := chat.SanitizeDisplayName(rl.Name); name != "" {
+		return name
 	}
 	if path, ok := resourceLinkPath(rl.Uri); ok {
-		if base := filepath.Base(path); base != "." && base != string(filepath.Separator) {
+		if base := chat.SanitizeDisplayName(filepath.Base(path)); base != "" && base != "." && base != string(filepath.Separator) {
 			return base
 		}
 	}

pkg/acp/agent_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"io"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	acpsdk "github.com/coder/acp-go-sdk"
@@ -70,6 +71,19 @@ func TestBuildUserContent_ResourceLinkFallbackDoesNotExposeAbsoluteURI(t *testin
 	assert.NotContains(t, content, "/var/folders")
 }
 
+func TestResourceLinkNameIsSafeAndBounded(t *testing.T) {
+	t.Parallel()
+
+	longName := strings.Repeat("é", 100) + "\nforged"
+	assert.Equal(t, chat.SanitizeDisplayName(longName), resourceLinkName(&acpsdk.ContentBlockResourceLink{Name: longName}))
+
+	got := resourceLinkName(&acpsdk.ContentBlockResourceLink{Uri: "file:///tmp/unsafe%0Aname.png"})
+	assert.Equal(t, "unsafe_name.png", got)
+	assert.LessOrEqual(t, len(got), chat.MaxSanitizedFieldBytes)
+
+	assert.Equal(t, "resource", resourceLinkName(&acpsdk.ContentBlockResourceLink{Uri: "https://example.com/private.png"}))
+}
+
 func TestBuildUserMessage_ImageContent(t *testing.T) {
 	t.Parallel()
 

pkg/acp/runagent_test.go

@@ -8,6 +8,8 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
+	"os"
+	"path/filepath"
 	"slices"
 	"strings"
 	"sync"
@@ -19,11 +21,15 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	agentpkg "github.com/docker/docker-agent/pkg/agent"
 	"github.com/docker/docker-agent/pkg/chat"
 	"github.com/docker/docker-agent/pkg/effort"
+	"github.com/docker/docker-agent/pkg/model/provider/base"
+	"github.com/docker/docker-agent/pkg/modelsdev"
 	"github.com/docker/docker-agent/pkg/runtime"
 	"github.com/docker/docker-agent/pkg/session"
 	"github.com/docker/docker-agent/pkg/sessiontitle"
+	"github.com/docker/docker-agent/pkg/team"
 	"github.com/docker/docker-agent/pkg/tools"
 	skillstool "github.com/docker/docker-agent/pkg/tools/builtin/skills"
 	"github.com/docker/docker-agent/pkg/tools/builtin/todo"
@@ -182,6 +188,84 @@ func promptRequest(text string) acpsdk.PromptRequest {
 	}
 }
 
+type escapingMediaProvider struct {
+	requested string
+}
+
+func (p *escapingMediaProvider) ID() modelsdev.ID        { return modelsdev.ParseIDOrZero("test/media") }
+func (p *escapingMediaProvider) BaseConfig() base.Config { return base.Config{} }
+func (p *escapingMediaProvider) MaxTokens() int          { return 0 }
+func (p *escapingMediaProvider) CreateChatCompletionStream(context.Context, []chat.Message, []tools.Tool) (chat.MessageStream, error) {
+	return &escapingMediaStream{responses: []chat.MessageStreamResponse{
+		{Choices: []chat.MessageStreamChoice{{Index: 0, Delta: chat.MessageDelta{
+			Content: "completed",
+			Media:   []chat.MediaDelta{{Data: []byte("png"), MimeType: "image/png", Name: "provider.png", RequestedPath: p.requested, Size: 3}},
+		}}}},
+		{Choices: []chat.MessageStreamChoice{{Index: 0, FinishReason: chat.FinishReasonStop}}, Usage: &chat.Usage{InputTokens: 1, OutputTokens: 1}},
+	}}, nil
+}
+
+type escapingMediaStream struct {
+	responses []chat.MessageStreamResponse
+	index     int
+}
+
+func (s *escapingMediaStream) Recv() (chat.MessageStreamResponse, error) {
+	if s.index == len(s.responses) {
+		return chat.MessageStreamResponse{}, io.EOF
+	}
+	response := s.responses[s.index]
+	s.index++
+	return response, nil
+}
+
+func (*escapingMediaStream) Close() {}
+
+func TestPrompt_EscapingGeneratedMediaCompletesWithoutElicitation(t *testing.T) {
+	workspace := t.TempDir()
+	external := filepath.Join(t.TempDir(), "cat.png")
+	store := session.NewInMemorySessionStore()
+	root := agentpkg.New("root", "You are a test agent", agentpkg.WithModel(&escapingMediaProvider{requested: external}))
+	rt, err := runtime.New(t.Context(), team.New(team.WithAgents(root)),
+		runtime.WithSessionCompaction(false), runtime.WithSessionStore(store))
+	require.NoError(t, err)
+	t.Cleanup(func() { require.NoError(t, rt.Close()) })
+	rt.OnElicitationRequest(func(runtime.Event) { t.Error("generated media escape must not elicit") })
+	agent, sess, peer := newPromptTestAgent(t, rt)
+	sess.sess.ID = testSessionID
+	sess.sess.WorkingDir = workspace
+
+	ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
+	defer cancel()
+	response, err := agent.Prompt(ctx, promptRequest("save the generated image"))
+	require.NoError(t, err)
+	assert.Equal(t, acpsdk.StopReasonEndTurn, response.StopReason)
+	assert.NoFileExists(t, external)
+	assert.Equal(t, []byte("png"), mustReadACPFile(t, filepath.Join(workspace, "cat.png")))
+	stored, err := store.GetSession(t.Context(), testSessionID)
+	require.NoError(t, err)
+	require.Len(t, stored.GetAllMessages(), 2)
+	assistant := stored.GetAllMessages()[1].Message
+	assert.Equal(t, "completed", assistant.Content)
+	require.Len(t, assistant.MultiContent, 2)
+	document := assistant.MultiContent[1].Document
+	require.NotNil(t, document)
+	assert.Equal(t, "cat.png", document.Source.ArtifactPath)
+	assert.Equal(t, chat.ArtifactRootWorkspace, document.Source.ArtifactRoot)
+	assert.Equal(t, testSessionID, document.Source.ArtifactOwnerSessionID)
+	out, ok := peer.out.(*captureWriter)
+	require.True(t, ok)
+	assert.Contains(t, strings.Join(out.lines(), "\n"), "outside the workspace")
+	assert.Empty(t, peer.recordedRequests())
+}
+
+func mustReadACPFile(t *testing.T, path string) []byte {
+	t.Helper()
+	b, err := os.ReadFile(path)
+	require.NoError(t, err)
+	return b
+}
+
 func TestPromptReplacementCancelsQueuedTurnWithoutSideEffects(t *testing.T) {
 	t.Parallel()
 

pkg/chat/document.go

@@ -8,8 +8,7 @@ package chat
 // deprecated but remain supported for backward compatibility.
 const MessagePartTypeDocument MessagePartType = "document"
 
-// ArtifactRootKind identifies which root a DocumentSource.ArtifactPath is
-// relative to.
+// ArtifactRootKind identifies which root a DocumentSource.ArtifactPath is relative to.
 type ArtifactRootKind string
 
 // ArtifactRootWorkspace means ArtifactPath is relative to the OWNING

pkg/chat/media.go

@@ -15,6 +15,16 @@ type MediaDelta struct {
 	// Materialization chooses a fallback when no usable name is available.
 	Name string `json:"name,omitempty"`
 
+	// RequestedPath is the prompt-directed target path the model asked for
+	// (e.g. echoed from an "as sunshine.jpg" instruction), when one exists.
+	// It is untrusted model input: the runtime routes it through
+	// workspacemedia.ClassifyRequestedPath, and a path escaping the workspace
+	// requires an explicit user confirmation before it is honored. Response
+	// marker extraction (the "[media-file: ...]" protocol) will populate it;
+	// until that lands, providers leave it empty and materialization falls
+	// back to Name.
+	RequestedPath string `json:"requested_path,omitempty"`
+
 	// Size is the byte length of Data, cached because Data itself is
 	// dropped once the artifact is materialized.
 	Size int64 `json:"size,omitempty"`

pkg/cli/runner.go

@@ -248,9 +248,10 @@ func Run(ctx context.Context, out *Printer, cfg Config, rt runtime.Runtime, sess
 			case *runtime.ElicitationRequestEvent:
 				serverURL, ok := e.Meta["docker-agent/server_url"].(string)
 				if !ok || serverURL == "" {
-					slog.WarnContext(ctx, "Skipping elicitation: missing or invalid server_url (non-interactive session?)")
+					// Keep draining after declining forms so follow-up events cannot stall the turn.
+					slog.WarnContext(ctx, "Declining elicitation without form support in CLI mode", "message", e.Message)
 					_ = rt.ResumeElicitation(ctx, "decline", nil, e.ElicitationID)
-					return nil
+					continue
 				}
 
 				result := out.PromptOAuthAuthorization(ctx, serverURL)

pkg/cli/runner_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"sync"
 	"testing"
+	"time"
 
 	"gotest.tools/v3/assert"
 
@@ -33,6 +34,10 @@ func TestMain(m *testing.M) {
 // It emits pre-configured events from RunStream and records Resume calls.
 type mockRuntime struct {
 	events []runtime.Event
+	// runStreamFn, when set, replaces the default pre-buffered RunStream —
+	// used to model a live runtime that only makes progress while the
+	// consumer keeps draining.
+	runStreamFn func(context.Context, *session.Session) <-chan runtime.Event
 
 	mu                    sync.Mutex
 	resumes               []runtime.ResumeRequest
@@ -145,7 +150,10 @@ func (m *mockRuntime) Resume(_ context.Context, req runtime.ResumeRequest) {
 	m.resumes = append(m.resumes, req)
 }
 
-func (m *mockRuntime) RunStream(_ context.Context, _ *session.Session) <-chan runtime.Event {
+func (m *mockRuntime) RunStream(ctx context.Context, sess *session.Session) <-chan runtime.Event {
+	if m.runStreamFn != nil {
+		return m.runStreamFn(ctx, sess)
+	}
 	ch := make(chan runtime.Event, len(m.events))
 	for _, e := range m.events {
 		ch <- e
@@ -596,3 +604,50 @@ func TestErrorEventReturnedNotPrinted(t *testing.T) {
 	assert.Equal(t, errors.As(err, &runtimeErr), true)
 	assert.Equal(t, strings.Contains(buf.String(), "model failed"), false)
 }
+
+// A non-OAuth MCP elicitation must be declined in CLI mode without abandoning the event
+// stream: the runtime only makes progress while the consumer drains, so
+// returning early would stall the follow-up events (redirect warning,
+// assistant response) and lose the turn. The unbuffered stream below makes
+// the test fail (bounded, not wedged) if Run stops consuming after the
+// decline.
+func TestNonOAuthElicitationDeclinedAndStreamDrained(t *testing.T) {
+	t.Parallel()
+
+	drained := make(chan struct{})
+	rt := &mockRuntime{
+		runStreamFn: func(context.Context, *session.Session) <-chan runtime.Event {
+			ch := make(chan runtime.Event) // unbuffered: every send needs a live consumer
+			go func() {
+				defer close(ch)
+				defer close(drained)
+				ch <- &runtime.ElicitationRequestEvent{Type: "elicitation_request", Message: "Choose a deployment region"}
+				ch <- runtime.Warning("The deployment choice was declined", "test")
+				ch <- runtime.AgentChoice("test", "sess", "Continuing without deployment.")
+			}()
+			return ch
+		},
+	}
+
+	var buf bytes.Buffer
+	out := NewPrinter(&buf)
+	sess := session.New()
+
+	err := Run(t.Context(), out, Config{}, rt, sess, []string{"hello"})
+	assert.NilError(t, err)
+
+	select {
+	case <-drained:
+	case <-time.After(10 * time.Second):
+		t.Fatal("the CLI stopped draining the stream after declining the elicitation")
+	}
+
+	rt.mu.Lock()
+	defer rt.mu.Unlock()
+	assert.Equal(t, rt.elicitationDeclines, 1)
+	assert.Equal(t, rt.elicitationLastAction, tools.ElicitationAction("decline"))
+	assert.Check(t, strings.Contains(buf.String(), "deployment choice was declined"),
+		"the warning must be surfaced: %q", buf.String())
+	assert.Check(t, strings.Contains(buf.String(), "Continuing without deployment."),
+		"the assistant response must still be printed: %q", buf.String())
+}

pkg/runtime/elicitation.go

@@ -455,21 +455,59 @@ func backgroundElicitationDeclinedNote(message string) string {
 	)
 }
 
+// elicitationSpec carries an MCP request through the shared waiter registry.
+type elicitationSpec struct {
+	message string
+	mode    string
+	schema  any
+	url     string
+	// serverElicitationID is the originating MCP server's wire ID, if any.
+	// Informational only — never a routing key (#3584 review item 2a).
+	serverElicitationID string
+	meta                map[string]any
+	// agentName and sessionID override the runtime-derived defaults (the
+	// shared current-agent slot and the ctx conversation ID) when the caller
+	// knows the owning agent/session more precisely.
+	agentName string
+	sessionID string
+}
+
 // elicitationHandler is the MCP-toolset-side hook that turns an inbound
 // elicitation request from a server into an ElicitationRequest event and
 // waits for the embedder's response, correlated by elicitation ID.
 func (r *LocalRuntime) elicitationHandler(ctx context.Context, req *mcp.ElicitParams) (tools.ElicitationResult, error) {
 	slog.DebugContext(ctx, "Elicitation request received from MCP server", "message", req.Message)
+	return r.requestElicitation(ctx, elicitationSpec{
+		message:             req.Message,
+		mode:                req.Mode,
+		schema:              req.RequestedSchema,
+		url:                 req.URL,
+		serverElicitationID: req.ElicitationID,
+		meta:                req.Meta,
+	})
+}
 
+// requestElicitation emits spec as an ElicitationRequest event and waits for
+// the embedder's response, correlated by elicitation ID.
+func (r *LocalRuntime) requestElicitation(ctx context.Context, spec elicitationSpec) (tools.ElicitationResult, error) {
 	// In non-interactive mode (e.g., MCP serve), there is no user to respond
 	// to elicitation requests. Decline immediately instead of blocking forever.
 	if r.nonInteractive {
-		slog.DebugContext(ctx, "Declining elicitation in non-interactive mode", "message", req.Message)
+		slog.DebugContext(ctx, "Declining elicitation in non-interactive mode", "message", spec.message)
 		return tools.ElicitationResult{
 			Action: tools.ElicitationActionDecline,
 		}, nil
 	}
 
+	sessionID := spec.sessionID
+	if sessionID == "" {
+		sessionID = genai.ConversationIDFromContext(ctx)
+	}
+	agentName := spec.agentName
+	if agentName == "" {
+		agentName = r.currentAgentName()
+	}
+
 	// A background session (run_background_agent) marks its context so
 	// toolset Start() OAuth fails fast instead of eliciting (#3200). Mid-call
 	// elicitations reach here regardless of that marker, so extend the same
@@ -479,8 +517,8 @@ func (r *LocalRuntime) elicitationHandler(ctx context.Context, req *mcp.ElicitPa
 	// all can answer this request. Decline immediately with a model-readable
 	// note instead of parking a goroutine forever (#3584).
 	if !tools.InteractivePromptsAllowed(ctx) && !r.hasElicitationSink() {
-		slog.WarnContext(ctx, "Declining elicitation: background session has no UI to answer it", "message", req.Message)
-		r.elicitationDeclines.record(genai.ConversationIDFromContext(ctx), backgroundElicitationDeclinedNote(req.Message))
+		slog.WarnContext(ctx, "Declining elicitation: background session has no UI to answer it", "message", spec.message)
+		r.elicitationDeclines.record(sessionID, backgroundElicitationDeclinedNote(spec.message))
 		return tools.ElicitationResult{
 			Action: tools.ElicitationActionDecline,
 		}, nil
@@ -494,7 +532,7 @@ func (r *LocalRuntime) elicitationHandler(ctx context.Context, req *mcp.ElicitPa
 
 	// The registry key (and the ElicitationID surfaced to clients for
 	// ResumeElicitation routing) is always a freshly generated, internal
-	// ID — never the MCP wire req.ElicitationID. The wire value is only
+	// ID — never the MCP wire elicitation ID. The wire value is only
 	// ever set for URL-mode elicitations and is chosen by the originating
 	// MCP server; two independent servers (e.g. two background jobs each
 	// talking to their own MCP process) can legitimately reuse the same
@@ -513,16 +551,15 @@ func (r *LocalRuntime) elicitationHandler(ctx context.Context, req *mcp.ElicitPa
 	defer r.elicitationWaiters.abandon(correlationID, wt)
 
 	slog.DebugContext(ctx, "Sending elicitation request event to client",
-		"message", req.Message,
-		"mode", req.Mode,
-		"requested_schema", req.RequestedSchema,
-		"url", req.URL,
+		"message", spec.message,
+		"mode", spec.mode,
+		"requested_schema", spec.schema,
+		"url", spec.url,
 		"elicitation_id", correlationID,
-		"server_elicitation_id", req.ElicitationID)
-	slog.DebugContext(ctx, "Elicitation request meta", "meta", req.Meta)
+		"server_elicitation_id", spec.serverElicitationID)
+	slog.DebugContext(ctx, "Elicitation request meta", "meta", spec.meta)
 
-	sessionID := genai.ConversationIDFromContext(ctx)
-	ev := ElicitationRequest(req.Message, req.Mode, req.RequestedSchema, req.URL, correlationID, req.ElicitationID, sessionID, req.Meta, r.currentAgentName())
+	ev := ElicitationRequest(spec.message, spec.mode, spec.schema, spec.url, correlationID, spec.serverElicitationID, sessionID, spec.meta, agentName)
 
 	// Reliable delivery: invoked synchronously, unconditionally, and exactly
 	// once, BEFORE anything that could block (#3584 review item 1). This