feat(#3996): gate workspace-escaping generated-media paths behind user confirmation

pkg/chat/document.go

@@ -21,6 +21,15 @@ type ArtifactRootKind string
 // reference is never resolved and surfaces as unavailable.
 const ArtifactRootWorkspace ArtifactRootKind = "workspace"
 
+// ArtifactRootExternal means ArtifactPath is the ABSOLUTE path of a target
+// OUTSIDE the workspace that the user explicitly confirmed via a runtime
+// elicitation (see pkg/runtime's generated-media escape confirmation). It is
+// the one root kind whose path is not workspace-relative. Resolution must
+// still verify the (owner session, path) pair against the generated-media
+// manifest — an external reference in tampered session JSON selects nothing
+// without a matching manifest record.
+const ArtifactRootExternal ArtifactRootKind = "external"
+
 // DocumentSource holds the actual content of a document. Exactly one of the
 // fields should be set.
 type DocumentSource struct {
@@ -44,6 +53,9 @@ type DocumentSource struct {
 	//     resolution must also verify the (owner session, path) pair against
 	//     the generated-media manifest (session.GeneratedMediaManifest),
 	//     which only materialization writes.
+	//   - ArtifactRootExternal: the ABSOLUTE user-confirmed path, exactly as
+	//     returned by workspacemedia.WriteExternal, and equally gated on a
+	//     matching manifest record.
 	//   - empty: the root is unknown — never resolved; the part surfaces
 	//     as unavailable.
 	ArtifactPath string `json:"artifact_path,omitempty"`

pkg/chat/media.go

@@ -23,6 +23,16 @@ type MediaDelta struct {
 	// runtime accumulator synthesizes one when needed.
 	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,14 @@ 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?)")
+					// Non-OAuth elicitation (e.g. an MCP form or the runtime's
+					// workspace-escape confirmation): the CLI has no form UI, so
+					// decline — but keep draining the stream, otherwise the
+					// runtime blocks emitting the follow-up events (redirect
+					// warning, assistant response) and the turn never persists.
+					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
@@ -127,7 +132,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
@@ -578,3 +586,51 @@ 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 elicitation (e.g. the runtime's workspace-escape
+// confirmation) 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: "Save outside the workspace?"}
+				ch <- runtime.Warning("Requested save location for generated media item 1/1 is outside the workspace and was not confirmed; saved as cat.png in the workspace instead", "test")
+				ch <- runtime.AgentChoice("test", "sess", "Saved your image.")
+			}()
+			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(), "saved as cat.png in the workspace instead"),
+		"the redirect warning must be surfaced: %q", buf.String())
+	assert.Check(t, strings.Contains(buf.String(), "Saved your image."),
+		"the assistant response must still be printed: %q", buf.String())
+}

pkg/runtime/elicitation.go

@@ -455,21 +455,63 @@ func backgroundElicitationDeclinedNote(message string) string {
 	)
 }
 
+// elicitationSpec describes one elicitation request independently of its
+// origin: an MCP server (see elicitationHandler) or the runtime itself
+// (e.g. the generated-media workspace-escape confirmation in
+// media_escape.go). Both are answered through the same waiter registry and
+// ResumeElicitation plumbing.
+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 +521,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 +536,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 +555,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

pkg/runtime/loop.go

@@ -2,10 +2,8 @@ package runtime
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"log/slog"
-	"path"
 	"path/filepath"
 	"reflect"
 	"regexp"
@@ -1317,13 +1315,21 @@ func sanitizeToolCallName(name string) string {
 // generated-media manifest ([session.GeneratedMediaManifest]), the trust
 // anchor a resolver must consult before reading a workspace path back.
 //
-// The requested filename is the sanitized provider display name when one
-// exists, otherwise a generic "generated-N"; the writer owns MIME/extension
+// The requested filename is the prompt-directed path when the provider
+// surfaced one ([chat.MediaDelta.RequestedPath], populated by marker
+// extraction once that lands), otherwise the sanitized provider display
+// name, otherwise a generic "generated-N"; the writer owns MIME/extension
 // correction and collision suffixing, and the part persists the exact final
-// relative path it returns. A corrected extension additionally surfaces a
-// bounded user-visible notice naming the final path. Explicit
-// prompt-directed naming (and its out-of-workspace confirmation flow) is
-// intentionally not implemented here yet.
+// path it returns. A corrected extension additionally surfaces a bounded
+// user-visible notice naming the final path. A prompt-directed path that
+// escapes the workspace (absolute, "..", or "~"-rooted) is refused unless
+// the user confirms the resolved target through a runtime-native
+// elicitation; refusal redirects the bytes into the workspace root under
+// the sanitized basename with a sanitized warning — see
+// [LocalRuntime.writeGeneratedMedia] in media_escape.go for the policy. An
+// accepted external write persists [chat.ArtifactRootExternal] plus the
+// confirmed absolute path and is manifest-gated exactly like a workspace
+// write.
 //
 // When no workspace root is available (no provenance anywhere in the parent
 // chain, or a malformed stored value) every item fails with the same
@@ -1383,25 +1389,26 @@ func (r *LocalRuntime) materializeGeneratedMedia(ctx context.Context, sess *sess
 			continue
 		}
 
-		requested := safeName
-		generic := fmt.Sprintf("generated-%d", i+1)
-		if requested == "" {
-			requested = generic
-		}
-		res, err := workspacemediaWrite(root, requested, m.Data, m.MimeType)
-		if err != nil && requested != generic && errors.Is(err, workspacemedia.ErrPathEscape) {
-			// A provider display name the writer refuses even after display
-			// sanitization (e.g. a Windows-reserved name like "CON.png") must
-			// not cost the user the item; there is no user-chosen path to
-			// honor at this stage, so fall back to the generic name.
-			res, err = workspacemediaWrite(root, generic, m.Data, m.MimeType)
-		}
+		write, err := r.writeGeneratedMedia(ctx, generatedMediaItem{
+			workspaceRoot: root,
+			requestedPath: m.RequestedPath,
+			providerName:  safeName,
+			genericName:   fmt.Sprintf("generated-%d", i+1),
+			data:          m.Data,
+			mimeType:      m.MimeType,
+			safeMimeType:  safeMimeType,
+			agentName:     agentName,
+			sessionID:     sess.ID,
+			index:         i + 1,
+			total:         len(media),
+		}, events)
 		if err != nil {
 			warnItemFailed(err)
 			continue
 		}
+		res := write.res
 
-		if err := r.recordGeneratedFile(ctx, sess.ID, res.RelPath, safeMimeType); err != nil {
+		if err := r.recordGeneratedFile(ctx, sess.ID, write.root, res.RelPath, safeMimeType); err != nil {
 			// The file is already a real workspace deliverable, so keep the
 			// reference; without the manifest record inline display will
 			// refuse to render it (fail closed), which the user should hear
@@ -1423,12 +1430,12 @@ func (r *LocalRuntime) materializeGeneratedMedia(ctx context.Context, sess *sess
 		parts = append(parts, chat.MessagePart{
 			Type: chat.MessagePartTypeDocument,
 			Document: &chat.Document{
-				Name:     path.Base(res.RelPath),
+				Name:     generatedDocumentName(write.root, res.RelPath),
 				MimeType: safeMimeType,
 				Size:     m.Size,
 				Source: chat.DocumentSource{
 					ArtifactPath:           res.RelPath,
-					ArtifactRoot:           chat.ArtifactRootWorkspace,
+					ArtifactRoot:           write.root,
 					ArtifactOwnerSessionID: sess.ID,
 				},
 			},
@@ -1447,15 +1454,16 @@ func (r *LocalRuntime) sessionLookup() session.Lookup {
 }
 
 // recordGeneratedFile writes one manifest record after a successful
-// workspace write — materialization is the only writer of the manifest.
-func (r *LocalRuntime) recordGeneratedFile(ctx context.Context, sessionID, relPath, mimeType string) error {
+// write — materialization is the only writer of the manifest.
+func (r *LocalRuntime) recordGeneratedFile(ctx context.Context, sessionID string, root chat.ArtifactRootKind, finalPath, mimeType string) error {
 	manifest, ok := r.sessionStore.(session.GeneratedMediaManifest)
 	if !ok {
 		return fmt.Errorf("session store %T does not implement the generated-media manifest", r.sessionStore)
 	}
 	return manifest.AddGeneratedFile(ctx, session.GeneratedFile{
 		SessionID: sessionID,
-		RelPath:   relPath,
+		RelPath:   finalPath,
+		Root:      root,
 		MimeType:  mimeType,
 		CreatedAt: r.now(),
 	})