workspace-server/internal/handlers/chat_files.go

@@ -196,10 +196,15 @@ func resolveWorkspaceForwardCreds(c *gin.Context, ctx context.Context, workspace
 		c.JSON(http.StatusServiceUnavailable, gin.H{"error": "workspace url not registered yet"})
 		return "", "", false
 	}
-	// Trust note: workspaces.url passes validateAgentURL at /registry/
-	// register write time, blocking SSRF-shaped URLs. We rely on that
-	// upstream gate rather than re-validating here. Tracked at #2316
-	// for follow-up: forward-time re-validation as defense-in-depth.
+	// Defense-in-depth: re-validate the URL at forward time even though
+	// registration-time validation already ran. The DB value could have
+	// been tampered with or the registration-time policy could have
+	// changed since the row was written (#2316).
+	if err := isSafeURL(wsURL); err != nil {
+		log.Printf("chat_files %s: workspace %s URL rejected: %v", op, workspaceID, err)
+		c.JSON(http.StatusBadRequest, gin.H{"error": "workspace URL not allowed"})
+		return "", "", false
+	}
 
 	secret, healed, err := readOrLazyHealInboundSecret(ctx, workspaceID, "chat_files "+op)
 	if err != nil {

workspace-server/internal/handlers/chat_files_test.go

@@ -717,3 +717,78 @@ func TestChatDownload_404FromWorkspacePropagated(t *testing.T) {
 		t.Errorf("expected 404 propagated, got %d", w.Code)
 	}
 }
+
+// SSRF defense-in-depth tests for #2316 — forward-time URL validation in
+// resolveWorkspaceForwardCreds. Upload + Download share the helper; both
+// paths are pinned so a partial regression (one handler drifting away from
+// the shared helper) fails in CI.
+
+func TestChatUpload_RejectsMetadataURL(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	setSSRFCheckForTest(true)
+
+	wsID := "00000000-0000-0000-0000-000000000060"
+	expectURL(mock, wsID, "http://169.254.169.254/latest/meta-data/")
+
+	h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil, nil))
+	body, ct := uploadFixture(t)
+	c, w := makeUploadRequest(t, wsID, body, ct)
+	h.Upload(c)
+
+	if w.Code != http.StatusBadRequest {
+		t.Errorf("expected 400 for metadata URL in upload, got %d: %s", w.Code, w.Body.String())
+	}
+}
+
+func TestChatUpload_RejectsNonHTTPScheme(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	setSSRFCheckForTest(true)
+
+	wsID := "00000000-0000-0000-0000-000000000061"
+	expectURL(mock, wsID, "file:///etc/passwd")
+
+	h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil, nil))
+	body, ct := uploadFixture(t)
+	c, w := makeUploadRequest(t, wsID, body, ct)
+	h.Upload(c)
+
+	if w.Code != http.StatusBadRequest {
+		t.Errorf("expected 400 for file:// scheme in upload, got %d: %s", w.Code, w.Body.String())
+	}
+}
+
+func TestChatDownload_RejectsMetadataURL(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	setSSRFCheckForTest(true)
+
+	wsID := "00000000-0000-0000-0000-000000000062"
+	expectURL(mock, wsID, "http://169.254.169.254/latest/meta-data/")
+
+	h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil, nil))
+	c, w := makeDownloadRequest(t, wsID, "/workspace/foo.txt")
+	h.Download(c)
+
+	if w.Code != http.StatusBadRequest {
+		t.Errorf("expected 400 for metadata URL in download, got %d: %s", w.Code, w.Body.String())
+	}
+}
+
+func TestChatDownload_RejectsNonHTTPScheme(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	setSSRFCheckForTest(true)
+
+	wsID := "00000000-0000-0000-0000-000000000063"
+	expectURL(mock, wsID, "file:///etc/passwd")
+
+	h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil, nil))
+	c, w := makeDownloadRequest(t, wsID, "/workspace/foo.txt")
+	h.Download(c)
+
+	if w.Code != http.StatusBadRequest {
+		t.Errorf("expected 400 for file:// scheme in download, got %d: %s", w.Code, w.Body.String())
+	}
+}