fix(auth): guard quick-chat listing on workspace ownership (#3026)

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

apps/backend/internal/backendapp/boot_state_quick_chat_authz_test.go

@@ -0,0 +1,87 @@
+package backendapp
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/kandev/kandev/internal/auth/authn"
+	"github.com/kandev/kandev/internal/task/models"
+	"github.com/kandev/kandev/internal/webapp"
+)
+
+// seedOwnedQuickChat gives userID a workspace holding one restorable quick chat
+// whose title is the thing that must not cross an ownership boundary.
+func seedOwnedQuickChat(t *testing.T, harness bootStateTestHarness, workspaceID, userID, title string) {
+	t.Helper()
+	ctx := context.Background()
+	if err := harness.taskRepo.CreateWorkspace(ctx, &models.Workspace{
+		ID: workspaceID, Name: workspaceID, OwnerID: userID,
+	}); err != nil {
+		t.Fatalf("CreateWorkspace(%s): %v", workspaceID, err)
+	}
+	taskID := workspaceID + "-chat"
+	if err := harness.taskRepo.CreateTask(ctx, &models.Task{
+		ID: taskID, WorkspaceID: workspaceID, Title: title,
+		State: "todo", Priority: "medium", IsEphemeral: true,
+	}); err != nil {
+		t.Fatalf("CreateTask(%s): %v", taskID, err)
+	}
+	if err := harness.taskRepo.CreateTaskSession(ctx, &models.TaskSession{
+		ID: taskID + "-session", TaskID: taskID,
+		State: models.TaskSessionStateCompleted, IsPrimary: true,
+	}); err != nil {
+		t.Fatalf("CreateTaskSession(%s): %v", taskID, err)
+	}
+}
+
+func bootQuickChatNames(t *testing.T, params routeParams, userID, workspaceID string) []string {
+	t.Helper()
+	ctx := authn.WithIdentity(t.Context(), authn.Identity{UserID: userID, Role: authn.RoleMember})
+	req := httptest.NewRequest(http.MethodGet, "/?workspaceId="+workspaceID, nil).WithContext(ctx)
+	state := bootInitialState(ctx, req, params, webapp.RouteClassification{Route: webapp.RouteHome})
+
+	quickChat, ok := state["quickChat"].(map[string]any)
+	if !ok {
+		return nil
+	}
+	sessions, _ := quickChat["sessions"].([]map[string]any)
+	names := make([]string, 0, len(sessions))
+	for _, session := range sessions {
+		name, _ := session["name"].(string)
+		names = append(names, name)
+	}
+	return names
+}
+
+// TestBootStateQuickChatIsScopedToTheCaller is the regression guard for the
+// second ListQuickChatSessions caller. Its main job is the owner case: the boot
+// payload must still carry the caller's tabs now that the service can refuse.
+//
+// The foreign case documents rather than detects. resolveQuickChatWorkspaceID
+// validates every candidate against the caller's own visible workspace list, so
+// a foreign ID never reaches the service at all -- removing the service guard
+// entirely leaves this assertion green. That is the property being pinned: the
+// boot path fails closed one layer up, so a future refactor that starts
+// trusting a request-supplied workspace ID here has to break this test first.
+func TestBootStateQuickChatIsScopedToTheCaller(t *testing.T) {
+	harness := newBootStateTestHarness(t)
+	params := routeParams{taskSvc: harness.taskSvc, userCtrl: harness.userCtrl}
+	seedOwnedQuickChat(t, harness, "ws-boot-a", "user-a", "A's boot chat")
+	seedOwnedQuickChat(t, harness, "ws-boot-b", "user-b", "B's boot chat")
+
+	owned := bootQuickChatNames(t, params, "user-a", "ws-boot-a")
+	if len(owned) != 1 || owned[0] != "A's boot chat" {
+		t.Fatalf("owner boot payload quick chats = %v; want [A's boot chat]", owned)
+	}
+
+	// Asking for user-b's workspace must not put user-b's tab in user-a's boot
+	// payload -- neither directly nor by falling back to it.
+	foreign := bootQuickChatNames(t, params, "user-a", "ws-boot-b")
+	for _, name := range foreign {
+		if name == "B's boot chat" {
+			t.Fatalf("boot payload leaked a foreign quick chat: %v", foreign)
+		}
+	}
+}

apps/backend/internal/task/handlers/quick_chat_list_authz_test.go

@@ -0,0 +1,123 @@
+package handlers
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/kandev/kandev/internal/auth/authn"
+	"github.com/kandev/kandev/internal/task/models"
+	"github.com/kandev/kandev/internal/task/repository/repoerrors"
+	"github.com/kandev/kandev/internal/task/service"
+)
+
+// quickChatAuthzRepo owns two workspaces belonging to different users and
+// serves user-a's quick chat regardless of the workspace asked for, so any tab
+// reaching a foreign caller is a leak rather than an artifact of the fixture.
+type quickChatAuthzRepo struct {
+	quickChatListRepo
+	workspaces map[string]*models.Workspace
+}
+
+// GetWorkspace mirrors the sqlite repository's error shape exactly: a missing
+// row wraps the sentinel with the requested ID (workspace.go:351). Returning the
+// bare sentinel here would make the byte-identity assertion below pass for the
+// wrong reason -- the point is that handleNotFound substitutes its own fallback
+// message, so the ID the caller probed for never reaches the response.
+func (r *quickChatAuthzRepo) GetWorkspace(_ context.Context, id string) (*models.Workspace, error) {
+	workspace, ok := r.workspaces[id]
+	if !ok {
+		return nil, fmt.Errorf("%w: %s", repoerrors.ErrWorkspaceNotFound, id)
+	}
+	return workspace, nil
+}
+
+func newQuickChatAuthzHandler(t *testing.T) *TaskHandlers {
+	t.Helper()
+	gin.SetMode(gin.TestMode)
+	log := newTestLogger(t)
+	repo := &quickChatAuthzRepo{
+		quickChatListRepo: quickChatListRepo{
+			tasks: []*models.Task{{
+				ID: "task-chat", WorkspaceID: "ws-a", Title: "A's secret chat", IsEphemeral: true,
+				Metadata: map[string]interface{}{models.MetaKeyAgentProfileID: "agent-1"},
+			}},
+			sessions: map[string]*models.TaskSession{
+				"task-chat": {ID: "session-chat", TaskID: "task-chat"},
+			},
+		},
+		workspaces: map[string]*models.Workspace{
+			"ws-a": {ID: "ws-a", Name: "A's", OwnerID: "user-a"},
+			"ws-b": {ID: "ws-b", Name: "B's", OwnerID: "user-b"},
+		},
+	}
+	svc := service.NewService(service.Repos{
+		Workspaces: repo, Tasks: repo, TaskRepos: repo,
+		Workflows: repo, Messages: repo, Turns: repo,
+		Sessions: repo, GitSnapshots: repo, RepoEntities: repo,
+		Executors: repo, Environments: repo, TaskEnvironments: repo,
+		Reviews: repo,
+	}, nil, log, service.RepositoryDiscoveryConfig{})
+	return &TaskHandlers{service: svc, logger: log}
+}
+
+func listQuickChats(t *testing.T, h *TaskHandlers, identity *authn.Identity, workspaceID string) *httptest.ResponseRecorder {
+	t.Helper()
+	rec := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(rec)
+	req := httptest.NewRequest(http.MethodGet, "/workspaces/"+workspaceID+"/quick-chats", nil)
+	if identity != nil {
+		req = req.WithContext(authn.WithIdentity(req.Context(), *identity))
+	}
+	c.Request = req
+	c.Params = gin.Params{{Key: "id", Value: workspaceID}}
+	h.httpListQuickChatSessions(c)
+	return rec
+}
+
+// TestHTTPListQuickChatSessionsDeniesForeignWorkspace verifies end to end that
+// the service denial reaches the wire as a plain 404 -- the whole
+// no-existence-leak guarantee rests on the response, not on the sentinel.
+func TestHTTPListQuickChatSessionsDeniesForeignWorkspace(t *testing.T) {
+	h := newQuickChatAuthzHandler(t)
+	userB := &authn.Identity{UserID: "user-b", Role: authn.RoleMember}
+
+	foreign := listQuickChats(t, h, userB, "ws-a")
+	missing := listQuickChats(t, h, userB, "ws-nonexistent")
+
+	assert.Equal(t, http.StatusNotFound, foreign.Code)
+	assert.NotContains(t, foreign.Body.String(), "A's secret chat")
+	assert.NotContains(t, foreign.Body.String(), "session-chat")
+
+	// The probed ID must not survive into either body; that is what lets the
+	// two cases collapse into one response.
+	assert.Equal(t, http.StatusNotFound, missing.Code)
+	assert.NotContains(t, missing.Body.String(), "ws-nonexistent")
+
+	// A workspace the caller may not see must be indistinguishable from one
+	// that does not exist, byte for byte.
+	assert.Equal(t, missing.Body.String(), foreign.Body.String())
+}
+
+// TestHTTPListQuickChatSessionsOwnerAndUnscopedCallers keeps the legitimate
+// paths intact: the owner still gets their tabs, and with authentication
+// disabled (synthetic identity, or no identity at all) nothing changes.
+func TestHTTPListQuickChatSessionsOwnerAndUnscopedCallers(t *testing.T) {
+	h := newQuickChatAuthzHandler(t)
+
+	for name, identity := range map[string]*authn.Identity{
+		"owner":         {UserID: "user-a", Role: authn.RoleMember},
+		"auth disabled": {UserID: "default-user", Role: authn.RoleAdmin, Synthetic: true},
+		"no identity":   nil,
+	} {
+		rec := listQuickChats(t, h, identity, "ws-a")
+		require.Equal(t, http.StatusOK, rec.Code, "%s: body %s", name, rec.Body.String())
+		assert.Contains(t, rec.Body.String(), "A's secret chat", "%s", name)
+	}
+}

apps/backend/internal/task/service/quick_chat_sessions.go

@@ -40,7 +40,16 @@ type QuickChatSession struct {
 // most recently active first. It is the single source of truth for the quick
 // chat tab strip: the boot payload and the runtime resync endpoint both read it,
 // so a client that reloads and a client that resyncs converge on the same list.
+//
+// Quick-chat names are user-authored text, so the workspace check belongs here
+// rather than in the callers: the resync endpoint takes its workspace ID
+// straight from the URL. Today listQuickChatTasks also happens to authorize on
+// the way through ListTasksByWorkspace, but that is an implementation detail of
+// how this list is assembled, not a guarantee this function should lean on.
 func (s *Service) ListQuickChatSessions(ctx context.Context, workspaceID string) ([]QuickChatSession, error) {
+	if err := s.authorizeWorkspaceID(ctx, workspaceID); err != nil {
+		return nil, err
+	}
 	tasks, err := s.listQuickChatTasks(ctx, workspaceID)
 	if err != nil {
 		return nil, err

apps/backend/internal/task/service/quick_chat_sessions_access_test.go

@@ -0,0 +1,145 @@
+package service
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/jmoiron/sqlx"
+
+	"github.com/kandev/kandev/internal/task/models"
+	"github.com/kandev/kandev/internal/task/repository"
+	"github.com/kandev/kandev/internal/task/repository/repoerrors"
+)
+
+// countingTaskRepo records whether the quick-chat listing ever reached the task
+// store. A denial must be decided before the query runs, so the counter staying
+// at zero is what proves the guard is an authorization check and not a filter
+// applied to rows we already read.
+type countingTaskRepo struct {
+	repository.TaskRepository
+	listCalls int
+}
+
+func (r *countingTaskRepo) ListTasksByWorkspace(
+	ctx context.Context,
+	workspaceID, workflowID, repositoryID, query string,
+	page, pageSize int,
+	sort string,
+	includeArchived, includeEphemeral, onlyEphemeral, excludeConfig bool,
+) ([]*models.Task, int, error) {
+	r.listCalls++
+	return r.TaskRepository.ListTasksByWorkspace(
+		ctx, workspaceID, workflowID, repositoryID, query,
+		page, pageSize, sort, includeArchived, includeEphemeral, onlyEphemeral, excludeConfig,
+	)
+}
+
+// seedScopedQuickChats gives user-a and user-b one owned workspace each, with a
+// single restorable quick chat apiece.
+func seedScopedQuickChats(t *testing.T, svc *Service, sqlxDB *sqlx.DB) {
+	t.Helper()
+	ctx := context.Background()
+	for _, ws := range []*models.Workspace{
+		{ID: "ws-qc-a", Name: "A's", OwnerID: "user-a"},
+		{ID: "ws-qc-b", Name: "B's", OwnerID: "user-b"},
+	} {
+		if err := svc.workspaces.CreateWorkspace(ctx, ws); err != nil {
+			t.Fatalf("CreateWorkspace(%s): %v", ws.ID, err)
+		}
+	}
+	base := time.Date(2026, 8, 25, 12, 0, 0, 0, time.UTC)
+	seedQuickChatTask(t, svc, sqlxDB, quickChatFixture{
+		id: "qc-task-a", workspaceID: "ws-qc-a", title: "A's secret chat",
+		agentProfileID: "agent-a", updatedAt: base,
+	})
+	seedQuickChatTask(t, svc, sqlxDB, quickChatFixture{
+		id: "qc-task-b", workspaceID: "ws-qc-b", title: "B's secret chat",
+		agentProfileID: "agent-b", updatedAt: base,
+	})
+}
+
+// TestListQuickChatSessionsDeniesForeignWorkspace is the core regression: quick
+// chat names are user-authored text, so a foreign listing must fail closed and
+// must fail before the task query runs.
+func TestListQuickChatSessionsDeniesForeignWorkspace(t *testing.T) {
+	svc, sqlxDB := createOfficeIntegrationServiceWithDB(t)
+	seedScopedQuickChats(t, svc, sqlxDB)
+
+	counter := &countingTaskRepo{TaskRepository: svc.tasks}
+	svc.tasks = counter
+
+	items, err := svc.ListQuickChatSessions(ctxAs("user-b"), "ws-qc-a")
+	if !errors.Is(err, repoerrors.ErrWorkspaceNotFound) {
+		t.Fatalf("foreign quick-chat listing: got (%v, %v); want ErrWorkspaceNotFound", items, err)
+	}
+	if len(items) != 0 {
+		t.Fatalf("denied listing leaked %d tabs", len(items))
+	}
+	if counter.listCalls != 0 {
+		t.Fatalf("quick-chat query ran %d time(s) for a denied caller; want 0", counter.listCalls)
+	}
+}
+
+// TestListQuickChatSessionsForeignMatchesNonexistent pins the no-existence-leak
+// guarantee at the service boundary: both cases resolve to the same sentinel,
+// which is what makes them collapse into one response. The sentinels carry
+// different detail text (the repository names the missing ID), so the byte-level
+// identity of the two responses is pinned at the HTTP boundary instead, in
+// TestHTTPListQuickChatSessionsDeniesForeignWorkspace.
+func TestListQuickChatSessionsForeignMatchesNonexistent(t *testing.T) {
+	svc, sqlxDB := createOfficeIntegrationServiceWithDB(t)
+	seedScopedQuickChats(t, svc, sqlxDB)
+
+	_, foreignErr := svc.ListQuickChatSessions(ctxAs("user-b"), "ws-qc-a")
+	_, missingErr := svc.ListQuickChatSessions(ctxAs("user-b"), "ws-does-not-exist")
+
+	if !errors.Is(foreignErr, repoerrors.ErrWorkspaceNotFound) {
+		t.Fatalf("foreign workspace: got %v; want ErrWorkspaceNotFound", foreignErr)
+	}
+	if !errors.Is(missingErr, repoerrors.ErrWorkspaceNotFound) {
+		t.Fatalf("nonexistent workspace: got %v; want ErrWorkspaceNotFound", missingErr)
+	}
+}
+
+// TestListQuickChatSessionsOwnerStillSeesTabs guards against the fix turning
+// into a denial for the legitimate owner.
+func TestListQuickChatSessionsOwnerStillSeesTabs(t *testing.T) {
+	svc, sqlxDB := createOfficeIntegrationServiceWithDB(t)
+	seedScopedQuickChats(t, svc, sqlxDB)
+
+	items, err := svc.ListQuickChatSessions(ctxAs("user-a"), "ws-qc-a")
+	if err != nil {
+		t.Fatalf("owner listing: %v", err)
+	}
+	if len(items) != 1 || items[0].Name != "A's secret chat" {
+		t.Fatalf("owner listing = %+v; want one tab named %q", items, "A's secret chat")
+	}
+}
+
+// TestListQuickChatSessionsUnscopedCallersUnchanged pins single-user behavior:
+// the synthetic identity (auth disabled) and identity-less internal callers see
+// every workspace's tabs exactly as before.
+func TestListQuickChatSessionsUnscopedCallersUnchanged(t *testing.T) {
+	svc, sqlxDB := createOfficeIntegrationServiceWithDB(t)
+	seedScopedQuickChats(t, svc, sqlxDB)
+
+	for name, ctx := range map[string]context.Context{
+		"auth disabled": ctxSynthetic(),
+		"internal":      context.Background(),
+	} {
+		for _, ws := range []struct{ id, want string }{
+			{"ws-qc-a", "A's secret chat"},
+			{"ws-qc-b", "B's secret chat"},
+		} {
+			items, err := svc.ListQuickChatSessions(ctx, ws.id)
+			if err != nil {
+				t.Fatalf("%s caller listing %s: %v", name, ws.id, err)
+			}
+			if len(items) != 1 || items[0].Name != ws.want {
+				t.Fatalf("%s caller listing %s = %+v; want one tab named %q", name, ws.id, items, ws.want)
+			}
+		}
+	}
+}