feat(saas): deliver platform_inbound_secret via /registry/register (RFC #2312, PR-F)

scripts/build_runtime_package.py

@@ -66,6 +66,7 @@
     "heartbeat",
     "initial_prompt",
     "internal_chat_uploads",
+    "internal_file_read",
     "main",
     "molecule_ai_status",
     "platform_auth",

workspace-server/internal/handlers/chat_files.go

@@ -21,13 +21,12 @@ package handlers
 // conversation payloads.
 
 import (
-	"archive/tar"
 	"errors"
 	"fmt"
 	"io"
 	"log"
-	"mime"
 	"net/http"
+	"net/url"
 	"path/filepath"
 	"strings"
 	"time"
@@ -245,16 +244,20 @@ func (h *ChatFilesHandler) Upload(c *gin.Context) {
 }
 
 // Download handles GET /workspaces/:id/chat/download?path=<abs path>.
-// Streams the file bytes from the container with a correct
-// Content-Type and attachment Content-Disposition. Binary-safe —
-// unlike the existing JSON ReadFile endpoint which carries content
-// as a string (lossy for non-UTF-8 bytes).
+// Forwards over HTTP to the workspace's own /internal/file/read endpoint
+// (RFC #2312 PR-D), replacing the docker-cp tar-stream extraction that
+// only worked when the platform binary had local Docker socket access.
 //
-// TODO(#2312, follow-up PR): migrate Download to the same HTTP-forward
-// pattern as Upload. For now keeping the docker-cp path so this PR is
-// reviewable as a single-surface change. SaaS download is broken
-// today the same way SaaS upload was broken before this PR — the next
-// PR closes that gap.
+// Same path-safety contract as the legacy version: caller-side validation
+// is duplicated on the workspace side (internal_file_read.py) so a
+// platform bug or malicious caller bypassing one layer still hits the
+// other. This is "defence in depth via two parallel checks," not "trust
+// the workspace to validate" — the workspace doesn't trust the platform
+// either.
+//
+// Body is streamed end-to-end (no buffering on the platform), preserving
+// binary safety and arbitrary file size (the 50 MB cap on Upload doesn't
+// apply to artefacts the agent produced).
 func (h *ChatFilesHandler) Download(c *gin.Context) {
 	workspaceID := c.Param("id")
 	if err := validateWorkspaceID(workspaceID); err != nil {
@@ -293,54 +296,72 @@ func (h *ChatFilesHandler) Download(c *gin.Context) {
 	}
 
 	ctx := c.Request.Context()
-	if h.templates.docker == nil {
-		c.JSON(http.StatusServiceUnavailable, gin.H{"error": "docker unavailable"})
+
+	// Resolve workspace URL + inbound secret. Same shape as Upload —
+	// see chat_files.go::Upload for the rationale on why each missing-
+	// piece path surfaces as 404 / 503.
+	var wsURL string
+	if err := db.DB.QueryRowContext(ctx,
+		`SELECT COALESCE(url, '') FROM workspaces WHERE id = $1`, workspaceID,
+	).Scan(&wsURL); err != nil {
+		log.Printf("chat_files Download: workspace lookup failed for %s: %v", workspaceID, err)
+		c.JSON(http.StatusNotFound, gin.H{"error": "workspace not found"})
 		return
 	}
-	containerName := h.templates.findContainer(ctx, workspaceID)
-	if containerName == "" {
-		c.JSON(http.StatusServiceUnavailable, gin.H{"error": "workspace container not running"})
+	if wsURL == "" {
+		c.JSON(http.StatusServiceUnavailable, gin.H{"error": "workspace url not registered yet"})
 		return
 	}
 
-	// docker cp returns a tar stream containing the requested path.
-	// For a regular file that's a single tar entry; we extract and
-	// stream the body through.
-	reader, _, err := h.templates.docker.CopyFromContainer(ctx, containerName, path)
+	secret, err := wsauth.ReadPlatformInboundSecret(ctx, db.DB, workspaceID)
 	if err != nil {
-		c.JSON(http.StatusNotFound, gin.H{"error": "file not found"})
+		if errors.Is(err, wsauth.ErrNoInboundSecret) {
+			log.Printf("chat_files Download: no platform_inbound_secret for %s — workspace needs reprovision (#2312)", workspaceID)
+			c.JSON(http.StatusServiceUnavailable, gin.H{
+				"error":  "workspace not yet enrolled in v2 download (RFC #2312)",
+				"detail": "Reprovisioning the workspace will mint the platform_inbound_secret it's missing.",
+			})
+			return
+		}
+		log.Printf("chat_files Download: read platform_inbound_secret failed for %s: %v", workspaceID, err)
+		c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to read workspace secret"})
 		return
 	}
-	defer reader.Close()
 
-	tr := tar.NewReader(reader)
-	hdr, err := tr.Next()
+	// Build forward URL with the validated path encoded as a query param.
+	// url.Values handles all the percent-encoding correctly — a path with
+	// special chars (spaces, &, +) round-trips through both the platform's
+	// validator and the workspace-side validator.
+	forwardURL := strings.TrimRight(wsURL, "/") + "/internal/file/read?path=" + url.QueryEscape(path)
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, forwardURL, nil)
 	if err != nil {
-		c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to read archive"})
+		log.Printf("chat_files Download: build request failed for %s: %v", workspaceID, err)
+		c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to construct forward request"})
 		return
 	}
-	if hdr.Typeflag != tar.TypeReg {
-		c.JSON(http.StatusBadRequest, gin.H{"error": "path is not a regular file"})
+	req.Header.Set("Authorization", "Bearer "+secret)
+
+	resp, err := h.httpClient.Do(req)
+	if err != nil {
+		log.Printf("chat_files Download: forward to %s failed: %v", forwardURL, err)
+		c.JSON(http.StatusBadGateway, gin.H{"error": "workspace unreachable"})
 		return
 	}
+	defer resp.Body.Close()
 
-	name := filepath.Base(path)
-	mt := mime.TypeByExtension(filepath.Ext(name))
-	if mt == "" {
-		mt = "application/octet-stream"
+	// Stream response back, including the workspace's headers so the
+	// client gets the correct Content-Type + Content-Disposition (the
+	// workspace constructs them from the actual file's extension +
+	// basename — keeping that logic on the workspace side avoids a
+	// double-source-of-truth on filename encoding rules).
+	for _, hdr := range []string{"Content-Type", "Content-Length", "Content-Disposition"} {
+		if v := resp.Header.Get(hdr); v != "" {
+			c.Header(hdr, v)
+		}
 	}
-	c.Header("Content-Type", mt)
-	c.Header("Content-Length", fmt.Sprintf("%d", hdr.Size))
-	c.Header("Content-Disposition", contentDispositionAttachment(name))
-	c.Status(http.StatusOK)
-
-	// Stream exactly hdr.Size bytes. CopyN was chosen over LimitReader
-	// because it returns an error when the source is short — that
-	// surfaces a bug in the tar extraction path immediately instead
-	// of silently truncating. Agents can legitimately produce files
-	// larger than the 50 MB upload cap (that's a per-request inbound
-	// cap, not a per-artifact one), so we cannot clamp here.
-	if _, err := io.CopyN(c.Writer, tr, hdr.Size); err != nil {
-		log.Printf("Chat download stream error for %s (%s): %v", workspaceID, path, err)
+	c.Status(resp.StatusCode)
+	if _, err := io.Copy(c.Writer, resp.Body); err != nil {
+		log.Printf("chat_files Download: stream response back failed for %s: %v", workspaceID, err)
 	}
 }
+

workspace-server/internal/handlers/chat_files_test.go

@@ -343,22 +343,123 @@ func TestContentDispositionAttachment_Escapes(t *testing.T) {
 	}
 }
 
-func TestChatDownload_DockerUnavailable(t *testing.T) {
-	setupTestDB(t)
+// makeDownloadRequest builds a gin context for GET /workspaces/:id/chat/download
+// with the given path query param.
+func makeDownloadRequest(t *testing.T, workspaceID, path string) (*gin.Context, *httptest.ResponseRecorder) {
+	t.Helper()
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: workspaceID}}
+	c.Request = httptest.NewRequest("GET", "/workspaces/"+workspaceID+"/chat/download?path="+path, nil)
+	return c, w
+}
+
+func TestChatDownload_WorkspaceNotInDB(t *testing.T) {
+	mock := setupTestDB(t)
 	setupTestRedis(t)
 
-	tmplh := NewTemplatesHandler(t.TempDir(), nil) // docker=nil
-	h := NewChatFilesHandler(tmplh)
+	wsID := "00000000-0000-0000-0000-000000000099"
+	mock.ExpectQuery(`SELECT COALESCE\(url, ''\) FROM workspaces WHERE id = \$1`).
+		WithArgs(wsID).
+		WillReturnError(sql.ErrNoRows)
 
-	w := httptest.NewRecorder()
-	c, _ := gin.CreateTestContext(w)
-	c.Params = gin.Params{{Key: "id", Value: "00000000-0000-0000-0000-000000000001"}}
-	req := httptest.NewRequest("GET", "/workspaces/xxx/chat/download?path=/workspace/report.pdf", nil)
-	c.Request = req
+	h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil))
+	c, w := makeDownloadRequest(t, wsID, "/workspace/foo.txt")
+	h.Download(c)
+
+	if w.Code != http.StatusNotFound {
+		t.Errorf("expected 404 when workspace row missing, got %d", w.Code)
+	}
+}
+
+func TestChatDownload_NoInboundSecret(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
 
+	wsID := "00000000-0000-0000-0000-000000000051"
+	expectURL(mock, wsID, "http://127.0.0.1:1")
+	expectInboundSecret(mock, wsID, nil)
+
+	h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil))
+	c, w := makeDownloadRequest(t, wsID, "/workspace/foo.txt")
 	h.Download(c)
 
 	if w.Code != http.StatusServiceUnavailable {
-		t.Errorf("expected 503 when docker is nil, got %d: %s", w.Code, w.Body.String())
+		t.Errorf("expected 503 when platform_inbound_secret missing, got %d: %s", w.Code, w.Body.String())
+	}
+	if !strings.Contains(w.Body.String(), "RFC #2312") {
+		t.Errorf("expected detail to reference RFC #2312, got: %s", w.Body.String())
+	}
+}
+
+func TestChatDownload_ForwardsToWorkspace_HappyPath(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+
+	body := []byte("file-contents-here\nmultiline\n")
+	cap := &captured{}
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		cap.authorization = r.Header.Get("Authorization")
+		cap.method = r.Method
+		cap.path = r.URL.Path
+		w.Header().Set("Content-Type", "text/plain")
+		w.Header().Set("Content-Disposition", `attachment; filename="report.txt"`)
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write(body)
+	}))
+	t.Cleanup(srv.Close)
+
+	wsID := "00000000-0000-0000-0000-000000000052"
+	expectURL(mock, wsID, srv.URL)
+	expectInboundSecret(mock, wsID, "the-secret")
+
+	h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil))
+	c, w := makeDownloadRequest(t, wsID, "/workspace/report.txt")
+	h.Download(c)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	if cap.authorization != "Bearer the-secret" {
+		t.Errorf("expected secret in Authorization header, got %q", cap.authorization)
+	}
+	if cap.method != "GET" {
+		t.Errorf("expected GET, got %s", cap.method)
+	}
+	if cap.path != "/internal/file/read" {
+		t.Errorf("expected /internal/file/read, got %s", cap.path)
+	}
+	if got := w.Header().Get("Content-Type"); got != "text/plain" {
+		t.Errorf("Content-Type not forwarded: %q", got)
+	}
+	if got := w.Header().Get("Content-Disposition"); got != `attachment; filename="report.txt"` {
+		t.Errorf("Content-Disposition not forwarded: %q", got)
+	}
+	if got := w.Body.Bytes(); !bytes.Equal(got, body) {
+		t.Errorf("body mismatch: got %q, want %q", got, body)
+	}
+}
+
+func TestChatDownload_404FromWorkspacePropagated(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusNotFound)
+		_, _ = w.Write([]byte(`{"error":"file not found"}`))
+	}))
+	t.Cleanup(srv.Close)
+
+	wsID := "00000000-0000-0000-0000-000000000053"
+	expectURL(mock, wsID, srv.URL)
+	expectInboundSecret(mock, wsID, "tok")
+
+	h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil))
+	c, w := makeDownloadRequest(t, wsID, "/workspace/missing.txt")
+	h.Download(c)
+
+	if w.Code != http.StatusNotFound {
+		t.Errorf("expected 404 propagated, got %d", w.Code)
 	}
 }

workspace-server/internal/handlers/registry.go

@@ -341,6 +341,27 @@ func (h *RegistryHandler) Register(c *gin.Context) {
 		log.Printf("Registry: token existence check failed for %s: %v", payload.ID, hasLiveErr)
 	}
 
+	// RFC #2312 PR-F: return the workspace's platform_inbound_secret so SaaS
+	// workspaces (which have no persistent /configs volume across container
+	// restarts) can re-populate /configs/.platform_inbound_secret on every
+	// register call. Docker-mode workspaces also receive it — the workspace-
+	// side write is idempotent (same value every call until a future
+	// rotation flow lands), so the duplication is harmless.
+	//
+	// NOT gated by hasLive: the inbound secret is minted at workspace
+	// creation in workspace_provision.go (PR-A), independent of the
+	// outbound auth_token's "issue once" lifecycle. Returning it here is
+	// the only delivery path for SaaS, where the platform's CP provisioner
+	// has no volume to write into.
+	if secret, secretErr := wsauth.ReadPlatformInboundSecret(ctx, db.DB, payload.ID); secretErr == nil {
+		response["platform_inbound_secret"] = secret
+	} else if !errors.Is(secretErr, wsauth.ErrNoInboundSecret) {
+		// ErrNoInboundSecret is the expected case for legacy workspaces
+		// that predate migration 044 — quiet. Other errors (DB hiccup, etc.)
+		// log loud so ops notices.
+		log.Printf("Registry: read platform_inbound_secret for %s failed: %v", payload.ID, secretErr)
+	}
+
 	c.JSON(http.StatusOK, response)
 }