feat(lark): let an agent's owner bind/manage its Lark bot (MUL-4213) (#5079)

* feat(lark): let an agent's owner bind/manage its Lark bot (MUL-4213) Scan-to-bind was authorized by workspace role only, so a non-admin member could not bind a Lark bot even to an agent they own. Authorize the device-flow install, status poll, and revoke by the same rule that governs every other agent-management op — canManageAgent: the agent's owner OR a workspace owner/admin. Backend: - router: begin/status/revoke drop to workspace-member level; the per-agent check moves into the handlers (agent_id is a query param / installation id, which the role middleware can't see). - BeginLarkInstall + RevokeLarkInstallation load the target agent and run canManageAgent. - GetLarkInstallStatus scopes the read to the session initiator or a workspace owner/admin; others get 404 (no existence leak). Session state now carries InitiatorID for this. Frontend: - LarkAgentBindButton takes agentOwnerId and lets the agent owner through (mirrors canEditAgent). - Agent Integrations tab gates Lark per-agent (owner or admin) while Slack stays workspace-admin-only, since its routes are unchanged. Tests: begin/status/revoke authorization (owner, agent owner, unrelated member) on the backend; agent-owner bind visibility on the frontend. Co-authored-by: multica-agent <github@multica.ai> * fix(lark): keep orphan installation revoke available to workspace admins (MUL-4213) RevokeLarkInstallation loaded the bound agent and ran canManageAgent unconditionally, so once the agent was hard-deleted the load 404'd and a workspace owner/admin could no longer disconnect the orphan Lark installation — a documented cleanup path (ListByWorkspace lists orphans; the active-connection query filters them; Settings surfaces "Unknown Agent" Disconnect). Fall back to workspace owner/admin-only revoke when GetAgentInWorkspace finds no agent; agents that still exist keep the owner-OR-admin canManageAgent check. A plain member gains no orphan-row cleanup rights. No FK/cascade — resolved in the application layer. Adds a backend regression test: orphan installation is revocable by a workspace owner but not a plain member. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>
2026-07-23 18:17:44 +02:00 · 2026-07-08 16:19:41 +08:00
parent 947a54b674
commit fd3216fd6b
9 changed files with 475 additions and 56 deletions
--- a/packages/views/agents/components/agent-detail-inspector.tsx
+++ b/packages/views/agents/components/agent-detail-inspector.tsx
@@ -245,6 +245,7 @@ export function AgentDetailInspector({
            <LarkAgentBindButton
              agentId={agent.id}
              agentName={agent.name}
+              agentOwnerId={agent.owner_id}
              onShowConnectedDetails={onShowIntegrations}
            />
            <SlackAgentBindButton
--- a/packages/views/agents/components/tabs/integrations-tab.test.tsx
+++ b/packages/views/agents/components/tabs/integrations-tab.test.tsx
@@ -71,8 +71,18 @@ vi.mock("@multica/core/auth", () => {
 });

 vi.mock("../../../settings/components/lark-tab", () => ({
-  LarkAgentBindButton: ({ agentId }: { agentId: string }) => (
-    <div data-testid="lark-bind-button" data-agent-id={agentId} />
+  LarkAgentBindButton: ({
+    agentId,
+    agentOwnerId,
+  }: {
+    agentId: string;
+    agentOwnerId?: string | null;
+  }) => (
+    <div
+      data-testid="lark-bind-button"
+      data-agent-id={agentId}
+      data-agent-owner-id={agentOwnerId ?? ""}
+    />
  ),
 }));

@@ -166,11 +176,11 @@ describe("IntegrationsTab", () => {
    expect(screen.queryByTestId("lark-bind-button")).toBeNull();
  });

-  it("points members at Settings with one role notice (not per-platform) when they can't manage", () => {
+  it("points members at Settings when they are neither an admin nor the agent owner", () => {
+    // A plain member viewing an agent owned by someone else can manage
+    // neither platform, so the read-only note replaces both sections.
    membersRef.current = [{ user_id: "user-1", role: "member" }];
-    renderTab(<IntegrationsTab agent={agent} />);
-    // The role gate is hoisted above the per-platform sections, so the notice
-    // appears exactly once and neither bind entry renders.
+    renderTab(<IntegrationsTab agent={{ ...agent, owner_id: "user-2" }} />);
    expect(
      screen.getByText(/Only workspace owners and admins can connect an agent/i),
    ).toBeTruthy();
@@ -178,6 +188,23 @@ describe("IntegrationsTab", () => {
    expect(screen.queryByTestId("slack-bind-button")).toBeNull();
  });

+  it("lets a non-admin agent owner bind Lark but keeps Slack admin-only", () => {
+    // The agent's owner (user-1) is only a plain workspace member. Lark
+    // authorizes the agent owner (canManageAgent), so the Lark bind entry
+    // renders and receives owner_id; Slack's routes stay admin-only, so it
+    // shows the read-only note instead of a CTA (MUL-4213).
+    membersRef.current = [{ user_id: "user-1", role: "member" }];
+    renderTab(<IntegrationsTab agent={agent} />);
+    const larkButton = screen.getByTestId("lark-bind-button");
+    expect(larkButton.getAttribute("data-agent-id")).toBe("agent-1");
+    expect(larkButton.getAttribute("data-agent-owner-id")).toBe("user-1");
+    expect(screen.queryByTestId("slack-bind-button")).toBeNull();
+    // The Slack section falls back to the shared members note.
+    expect(
+      screen.getByText(/Only workspace owners and admins can connect an agent/i),
+    ).toBeTruthy();
+  });
+
  it("renders the bind entry (not coming-soon) when installs are unavailable but the agent is already bound", () => {
    // install_supported governs only NEW installs; an already-bound agent
    // must still surface its connected state instead of "coming soon"
--- a/packages/views/agents/components/tabs/integrations-tab.tsx
+++ b/packages/views/agents/components/tabs/integrations-tab.tsx
@@ -51,8 +51,17 @@ export function IntegrationsTab({ agent }: { agent: Agent }) {
  const configured = listing?.configured === true;
  const installSupported = listing?.install_supported === true;
  const currentMember = members.find((m) => m.user_id === user?.id) ?? null;
-  const canManage =
+  const isWorkspaceAdmin =
    currentMember?.role === "owner" || currentMember?.role === "admin";
+  const isAgentOwner =
+    !!user?.id && agent.owner_id != null && agent.owner_id === user.id;
+  // Lark bind/manage is authorized for the agent's owner OR a workspace
+  // owner/admin (server/internal/handler/lark.go canManageAgent, MUL-4213).
+  // Slack's install/revoke routes are still workspace owner/admin-only, so
+  // its gate stays admin-only — the agent owner must not see a Slack CTA the
+  // backend would 403.
+  const canManageLark = isWorkspaceAdmin || isAgentOwner;
+  const canManageSlack = isWorkspaceAdmin;
  const hasActiveInstall =
    listing?.installations.some(
      (inst) => inst.agent_id === agent.id && inst.status === "active",
@@ -65,11 +74,11 @@ export function IntegrationsTab({ agent }: { agent: Agent }) {
      (inst) => inst.agent_id === agent.id && inst.status === "active",
    ) ?? false;

-  // Install / manage is gated on workspace owner/admin for every platform, so
-  // the role notice is hoisted above the per-platform sections — one note
-  // instead of repeating it under each integration. Members can still view
-  // connected bots in the (member-visible) Settings → Integrations listing.
-  if (!canManage) {
+  // A member who can manage neither platform (not a workspace admin and not
+  // this agent's owner) gets the read-only note instead of the sections.
+  // Members can still view connected bots in the (member-visible)
+  // Settings → Integrations listing.
+  if (!canManageLark && !canManageSlack) {
    return (
      <div className="space-y-6">
        <p className="text-xs text-muted-foreground">
@@ -122,10 +131,15 @@ export function IntegrationsTab({ agent }: { agent: Agent }) {
              </p>
            </div>
          ) : (
-            // Owner/admin with either a supported transport or an existing
-            // bot: the shared button renders the scan-to-bind CTA or the
-            // already-connected "Manage in Lark" badge.
-            <LarkAgentBindButton agentId={agent.id} agentName={agent.name} />
+            // Agent owner or workspace owner/admin with either a supported
+            // transport or an existing bot: the shared button renders the
+            // scan-to-bind CTA or the already-connected "Manage in Lark"
+            // badge. It self-authorizes on agentOwnerId + role.
+            <LarkAgentBindButton
+              agentId={agent.id}
+              agentName={agent.name}
+              agentOwnerId={agent.owner_id}
+            />
          )}
        </div>
      </section>
@@ -143,7 +157,14 @@ export function IntegrationsTab({ agent }: { agent: Agent }) {
          </div>
        </div>
        <div className="border-t px-4 py-3">
-          {!slackConfigured ? (
+          {!canManageSlack ? (
+            // Slack install/revoke stay workspace owner/admin-only, so an
+            // agent owner who is not an admin only gets the read-only note
+            // here (unlike Lark above). Reuses the shared members note.
+            <p className="text-xs text-muted-foreground">
+              {t(($) => $.tab_body.integrations.members_note)}
+            </p>
+          ) : !slackConfigured ? (
            <p className="text-xs text-muted-foreground">
              {ts(($) => $.slack.not_enabled_title)}
            </p>
--- a/packages/views/settings/components/lark-tab.test.tsx
+++ b/packages/views/settings/components/lark-tab.test.tsx
@@ -207,7 +207,7 @@ describe("LarkAgentBindButton (CTA gate)", () => {
    expect(screen.queryByRole("button", { name: /Bind to Lark/i })).toBeNull();
  });

-  it("hides both bind CTAs for a non-admin agent owner (matches backend admin gate)", () => {
+  it("hides both bind CTAs for a plain member when no agentOwnerId is supplied (stays admin-only)", () => {
    membersRef.current = [{ user_id: "user-1", role: "member" }];
    const { container } = render(
      <LarkAgentBindButton agentId="agent-1" agentName="Bot" />,
@@ -216,6 +216,36 @@ describe("LarkAgentBindButton (CTA gate)", () => {
    expect(container.querySelector("button")).toBeNull();
  });

+  it("shows the Feishu bind CTA for a non-admin agent owner (agentOwnerId matches the user, MUL-4213)", () => {
+    // The backend authorizes the agent's owner via canManageAgent even when
+    // they are only a plain workspace member, so the CTA must render for
+    // them once the caller threads the agent's owner_id.
+    membersRef.current = [{ user_id: "user-1", role: "member" }];
+    render(
+      <LarkAgentBindButton
+        agentId="agent-1"
+        agentName="Bot"
+        agentOwnerId="user-1"
+      />,
+      { wrapper: I18nWrapper },
+    );
+    expect(screen.getByRole("button", { name: /Bind to Feishu/i })).toBeTruthy();
+  });
+
+  it("still hides the CTAs for a non-admin member who is NOT the agent owner", () => {
+    // agentOwnerId belongs to someone else, so a plain member gets nothing.
+    membersRef.current = [{ user_id: "user-1", role: "member" }];
+    const { container } = render(
+      <LarkAgentBindButton
+        agentId="agent-1"
+        agentName="Bot"
+        agentOwnerId="user-2"
+      />,
+      { wrapper: I18nWrapper },
+    );
+    expect(container.querySelector("button")).toBeNull();
+  });
+
  it("hides both bind CTAs when the device-flow install path is not wired on the server", () => {
    installationsRef.current.install_supported = false;
    const { container } = render(
--- a/packages/views/settings/components/lark-tab.tsx
+++ b/packages/views/settings/components/lark-tab.tsx
@@ -275,13 +275,14 @@ function InstallationRow({
 // button is the entry point.
 //
 // Visibility rules, in order:
-//   1. Non-owner/admin viewers see nothing — the backend gates
-//      `POST /lark/install/begin`, the status poll, AND disconnect on
-//      those roles (see server/cmd/server/router.go), and `canEditAgent`
-//      lets agent owners through even when they're not workspace admins,
-//      so the parent's `canEdit` gate alone would expose controls that
-//      are guaranteed to 403.
-//   2. If this agent ALREADY has an active installation, owner/admins see
+//   1. Only the agent's owner or a workspace owner/admin see anything —
+//      the backend authorizes `POST /lark/install/begin`, the status
+//      poll, AND disconnect with canManageAgent (agent owner OR ws
+//      owner/admin; see server/internal/handler/lark.go, MUL-4213), so
+//      the gate here mirrors that. `agentOwnerId` is what lets a
+//      non-admin owner through; when it is omitted the button stays
+//      workspace owner/admin-only.
+//   2. If this agent ALREADY has an active installation, they see
 //      the "Connected + Manage in Lark" badge — regardless of
 //      install_supported. install_supported governs only whether NEW
 //      scan-installs can complete; already-installed bots stay manageable
@@ -295,11 +296,20 @@ function InstallationRow({
 export function LarkAgentBindButton({
  agentId,
  agentName,
+  agentOwnerId,
  className,
  onShowConnectedDetails,
 }: {
  agentId: string;
  agentName?: string;
+  /**
+   * The bound agent's owner (`agent.owner_id`). When it matches the
+   * current user, the button treats them as able to manage the bot even
+   * if they are not a workspace owner/admin — mirroring the backend's
+   * canManageAgent authorization (MUL-4213). Omit it to keep the button
+   * workspace owner/admin-only.
+   */
+  agentOwnerId?: string | null;
  className?: string;
  /**
   * When set, the connected state renders as a compact read-only status
@@ -335,8 +345,13 @@ export function LarkAgentBindButton({
    enabled: !!wsId,
  });
  const currentMember = members.find((m) => m.user_id === user?.id) ?? null;
-  const canManage =
+  const isWorkspaceAdmin =
    currentMember?.role === "owner" || currentMember?.role === "admin";
+  const isAgentOwner =
+    !!user?.id && agentOwnerId != null && agentOwnerId === user.id;
+  // Mirror the backend canManageAgent gate: the agent's owner OR a
+  // workspace owner/admin may bind/manage the bot (MUL-4213).
+  const canManage = isWorkspaceAdmin || isAgentOwner;

  if (!canManage) return null;

@@ -480,10 +495,10 @@ function LarkAgentBotStatusRow({
 // (new tab). Disconnect removes the installation after a confirm dialog.
 //
 // Visibility rules carry over from the parent `LarkAgentBindButton`:
-// only owners and admins ever reach this component, so the unbind
-// affordance is unconditionally shown — the backend gates DELETE on
-// the same role and would 403 anyone else, which makes a redundant
-// `canManage` check here dead code.
+// only the agent's owner or a workspace owner/admin ever reach this
+// component, so the unbind affordance is unconditionally shown — the
+// backend authorizes DELETE with the same canManageAgent check (MUL-4213)
+// and would 403 anyone else, which makes a redundant gate here dead code.
 //
 // The dev-console host depends on which Lark cloud the bot lives on:
 // Feishu (mainland) bots are managed at open.feishu.cn, Lark
--- a/server/cmd/server/router.go
+++ b/server/cmd/server/router.go
@@ -844,18 +844,24 @@ func NewRouterWithOptions(pool *pgxpool.Pool, hub *realtime.Hub, bus *events.Bus
 					r.Delete("/github/installations/{installationId}", h.DeleteGitHubInstallation)
 				})

-				// Lark integration. Listing is member-visible (same
-				// rationale as GitHub: the Integrations tab must
-				// render for non-admins so they see "wired up by whom").
-				// Install / revoke require admin to prevent a non-admin
-				// from binding a Bot to a workspace agent or yanking
-				// an installation out from under one.
+				// Lark integration. Every endpoint here only requires
+				// workspace membership at the router; the real authorization
+				// is per-agent and enforced inside each handler via
+				// canManageAgent (agent owner OR workspace owner/admin), so an
+				// agent's owner can bind/manage their own agent's Bot without
+				// being a workspace admin (MUL-4213). The router can't make
+				// that call itself: begin identifies the agent by an
+				// `agent_id` query param and revoke by an installation id,
+				// neither of which is a URL param the role middleware sees.
+				//   - Listing stays member-visible (same rationale as GitHub:
+				//     the Integrations tab must render for non-admins so they
+				//     see "wired up by whom").
+				//   - Begin / status / revoke each load the target agent and
+				//     run canManageAgent (status gates on the session
+				//     initiator or an admin) before doing anything.
 				r.Group(func(r chi.Router) {
 					r.Use(middleware.RequireWorkspaceMemberFromURL(queries, "id"))
 					r.Get("/lark/installations", h.ListLarkInstallations)
-				})
-				r.Group(func(r chi.Router) {
-					r.Use(middleware.RequireWorkspaceRoleFromURL(queries, "id", "owner", "admin"))
 					r.Delete("/lark/installations/{installationId}", h.RevokeLarkInstallation)
 					// Device-flow scan-to-install. Begin opens a new
 					// registration session against Lark and returns
--- a/server/internal/handler/lark.go
+++ b/server/internal/handler/lark.go
@@ -110,6 +110,16 @@ func (h *Handler) ListLarkInstallations(w http.ResponseWriter, r *http.Request)
 // flips status to 'revoked' so the WS hub drops the connection on its
 // next sweep. The row itself is preserved for audit; a re-install via
 // the device-flow path flips status back to 'active' atomically.
+//
+// Membership is checked at the router; the per-agent authorization
+// (canManageAgent: the bound agent's owner OR a workspace owner/admin)
+// is enforced here, symmetric with BeginLarkInstall so an agent owner
+// can unbind the bot they bound (MUL-4213). When the bound agent has
+// been hard-deleted the installation is an orphan (the active-connection
+// query skips it and disconnecting it is the documented cleanup path);
+// revoke then falls back to workspace owner/admin only, so the cleanup
+// entry point keeps working without handing a plain member orphan-row
+// rights.
 func (h *Handler) RevokeLarkInstallation(w http.ResponseWriter, r *http.Request) {
 	if h.LarkInstallations == nil {
 		writeError(w, http.StatusServiceUnavailable, "lark integration not configured")
@@ -129,7 +139,8 @@ func (h *Handler) RevokeLarkInstallation(w http.ResponseWriter, r *http.Request)
 	}
 	// Workspace-scoped lookup ensures one workspace cannot revoke
 	// another's installation by guessing the UUID.
-	if _, err := h.LarkInstallations.GetInWorkspace(r.Context(), instUUID, wsUUID); err != nil {
+	inst, err := h.LarkInstallations.GetInWorkspace(r.Context(), instUUID, wsUUID)
+	if err != nil {
 		if errors.Is(err, lark.ErrInstallationNotFound) {
 			writeError(w, http.StatusNotFound, "lark installation not found")
 			return
@@ -137,6 +148,24 @@ func (h *Handler) RevokeLarkInstallation(w http.ResponseWriter, r *http.Request)
 		writeError(w, http.StatusInternalServerError, "failed to load installation")
 		return
 	}
+	// Authorize against the bound agent. Normally its owner or a workspace
+	// owner/admin may revoke (canManageAgent writes the 403/404 itself).
+	// If the agent has been hard-deleted the installation is an orphan, so
+	// fall back to workspace owner/admin-only cleanup instead of 404-ing
+	// the disconnect entry point (see ListByWorkspace vs the orphan-
+	// filtered active list). No FK/cascade: the missing agent is handled
+	// in the application layer.
+	agent, agentErr := h.Queries.GetAgentInWorkspace(r.Context(), db.GetAgentInWorkspaceParams{
+		ID:          inst.AgentID,
+		WorkspaceID: wsUUID,
+	})
+	if agentErr != nil {
+		if _, ok := h.requireWorkspaceRole(w, r, uuidToString(wsUUID), "lark installation not found", "owner", "admin"); !ok {
+			return
+		}
+	} else if !h.canManageAgent(w, r, agent) {
+		return
+	}
 	if err := h.LarkInstallations.Revoke(r.Context(), instUUID); err != nil {
 		writeError(w, http.StatusInternalServerError, "failed to revoke installation")
 		return
@@ -235,10 +264,13 @@ type BeginLarkInstallResponse struct {
 }

 // BeginLarkInstall (POST /api/workspaces/{id}/lark/install/begin)
-// opens a new device-flow registration session against Lark. Admin-only
-// at the router. The agent_id query param picks which Multica Agent
-// the new Bot will be bound to; the agent must belong to this
-// workspace (RegistrationService re-checks that defense-in-depth).
+// opens a new device-flow registration session against Lark. The router
+// only requires workspace membership; this handler authorizes per-agent
+// via canManageAgent (the agent's owner OR a workspace owner/admin), so
+// an agent owner can bind their own agent's Bot without being a
+// workspace admin (MUL-4213). The agent_id query param picks which
+// Multica Agent the new Bot will be bound to; the agent must belong to
+// this workspace (RegistrationService re-checks that defense-in-depth).
 //
 // Returns 503 when the integration is not wired (no at-rest key, no
 // HTTP client, no RegistrationService); the UI hides the bind button
@@ -288,13 +320,21 @@ func (h *Handler) BeginLarkInstall(w http.ResponseWriter, r *http.Request) {
 	// Ownership pre-check at the HTTP boundary so a malformed
 	// agent_id surfaces 404 here (not an opaque service error from
 	// inside the service's own re-check).
-	if _, err := h.Queries.GetAgentInWorkspace(r.Context(), db.GetAgentInWorkspaceParams{
+	agent, err := h.Queries.GetAgentInWorkspace(r.Context(), db.GetAgentInWorkspaceParams{
 		ID:          agentUUID,
 		WorkspaceID: wsUUID,
-	}); err != nil {
+	})
+	if err != nil {
 		writeError(w, http.StatusNotFound, "agent not found in this workspace")
 		return
 	}
+	// Authorize the initiator against the target agent: its owner or a
+	// workspace owner/admin may bind. canManageAgent writes the 403/404
+	// itself, so a member who is neither is stopped here rather than at
+	// the (now member-level) router.
+	if !h.canManageAgent(w, r, agent) {
+		return
+	}
 	initiatorUUID, ok := parseUUIDOrBadRequest(w, userID, "user id")
 	if !ok {
 		return
@@ -330,9 +370,14 @@ type LarkInstallStatusResponse struct {
 }

 // GetLarkInstallStatus (GET /api/workspaces/{id}/lark/install/{sessionId}/status)
-// returns the current state of an in-flight install session. Admin-
-// only at the router. Unknown / cross-workspace / GC'd sessions return
-// 404 — the frontend treats it as "session lost, please restart".
+// returns the current state of an in-flight install session. The router
+// only requires workspace membership; this handler scopes the read to
+// the session's initiator OR a workspace owner/admin, so a member who
+// began an install (as its agent's owner) can poll their own session
+// without exposing it to unrelated members (MUL-4213). Unknown /
+// cross-workspace / GC'd sessions — and sessions the caller may not read
+// — return 404, which the frontend treats as "session lost, please
+// restart".
 //
 // On success this handler does NOT clean up the session — the
 // frontend may poll once more after the dialog closes to confirm
@@ -343,6 +388,10 @@ func (h *Handler) GetLarkInstallStatus(w http.ResponseWriter, r *http.Request) {
 		writeError(w, http.StatusServiceUnavailable, "lark install not configured")
 		return
 	}
+	userID, ok := requireUserID(w, r)
+	if !ok {
+		return
+	}
 	wsUUID, ok := parseUUIDOrBadRequest(w, chi.URLParam(r, "id"), "workspace id")
 	if !ok {
 		return
@@ -361,6 +410,17 @@ func (h *Handler) GetLarkInstallStatus(w http.ResponseWriter, r *http.Request) {
 		writeError(w, http.StatusInternalServerError, "failed to load install session")
 		return
 	}
+	// Only the initiator or a workspace owner/admin may read the
+	// session. The session id is handed back only to the initiator, so
+	// treating anyone else as "session lost" (404, no existence leak) is
+	// consistent with the cross-workspace case above.
+	if uuidToString(state.InitiatorID) != userID {
+		member, mErr := h.getWorkspaceMember(r.Context(), userID, uuidToString(wsUUID))
+		if mErr != nil || !roleAllowed(member.Role, "owner", "admin") {
+			writeError(w, http.StatusNotFound, "install session not found")
+			return
+		}
+	}
 	resp := LarkInstallStatusResponse{
 		Status:       string(state.Status),
 		ErrorReason:  state.ErrorReason,
--- a/server/internal/handler/lark_test.go
+++ b/server/internal/handler/lark_test.go
@@ -10,6 +10,7 @@ import (
 	"testing"

 	"github.com/multica-ai/multica/server/internal/integrations/lark"
+	"github.com/multica-ai/multica/server/internal/util/secretbox"
 )

 // Lark-handler unit tests focus on the no-config short-circuits —
@@ -218,3 +219,253 @@ VALUES ($1, $2, $3, jsonb_build_object('app_id', $4::text), $5, 'active')
 		t.Fatal("non-Feishu installation must not be listed by the Feishu hub")
 	}
 }
+
+// wireLarkInstallServices attaches a live InstallationService and a
+// RegistrationService (pointed at a hermetic fake device-flow server) to the
+// shared testHandler, restoring the previous values on cleanup. The fake
+// answers the RFC 8628 begin call with a canned device_code so an authorized
+// BeginLarkInstall reaches a 200; poll calls get authorization_pending so the
+// background poller stays quiet for the life of the test.
+func wireLarkInstallServices(t *testing.T) {
+	t.Helper()
+	if testHandler == nil {
+		t.Skip("database not available")
+	}
+	box, err := secretbox.New(make([]byte, secretbox.KeySize))
+	if err != nil {
+		t.Fatalf("secretbox.New: %v", err)
+	}
+	installSvc, err := lark.NewInstallationService(testHandler.Queries, box)
+	if err != nil {
+		t.Fatalf("NewInstallationService: %v", err)
+	}
+	fake := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_ = r.ParseForm()
+		w.Header().Set("Content-Type", "application/json")
+		if r.FormValue("action") == "begin" {
+			_ = json.NewEncoder(w).Encode(map[string]any{
+				"device_code":               "dc_test",
+				"verification_uri_complete": "https://accounts.feishu.cn/oauth/v1/qrcode?code=abc",
+				"verification_uri":          "https://accounts.feishu.cn/oauth/v1/qrcode",
+				"user_code":                 "ABCD-EFGH",
+				"interval":                  1,
+				"expire_in":                 2,
+			})
+			return
+		}
+		_ = json.NewEncoder(w).Encode(map[string]any{"error": "authorization_pending"})
+	}))
+	t.Cleanup(fake.Close)
+
+	regSvc, err := lark.NewRegistrationService(
+		lark.RegistrationServiceConfig{},
+		lark.NewRegistrationClient(lark.RegistrationConfig{Domain: fake.URL}),
+		lark.NewHTTPAPIClient(lark.HTTPClientConfig{}),
+		testHandler.Queries,
+		testPool,
+		installSvc,
+		lark.NewBindingTokenService(testHandler.Queries, testPool),
+	)
+	if err != nil {
+		t.Fatalf("NewRegistrationService: %v", err)
+	}
+
+	prevInstall, prevReg := testHandler.LarkInstallations, testHandler.LarkRegistration
+	testHandler.LarkInstallations = installSvc
+	testHandler.LarkRegistration = regSvc
+	t.Cleanup(func() {
+		testHandler.LarkInstallations = prevInstall
+		testHandler.LarkRegistration = prevReg
+	})
+}
+
+func beginLarkInstallAs(userID, agentID string) *httptest.ResponseRecorder {
+	req := newRequestAs(userID, http.MethodPost,
+		"/api/workspaces/"+testWorkspaceID+"/lark/install/begin?agent_id="+agentID, nil)
+	req = withURLParams(req, "id", testWorkspaceID)
+	w := httptest.NewRecorder()
+	testHandler.BeginLarkInstall(w, req)
+	return w
+}
+
+// TestBeginLarkInstall_AuthorizesAgentOwnerAndAdmins is the core of MUL-4213:
+// the device-flow scan-to-bind is authorized by canManageAgent, so the agent's
+// owner (a plain workspace member) and workspace owner/admins may begin an
+// install, while a member who is neither is forbidden.
+func TestBeginLarkInstall_AuthorizesAgentOwnerAndAdmins(t *testing.T) {
+	wireLarkInstallServices(t)
+	agentID, ownerID, memberID := privateAgentTestFixture(t)
+
+	cases := []struct {
+		name   string
+		userID string
+		want   int
+	}{
+		{"workspace owner (admin path)", testUserID, http.StatusOK},
+		{"agent owner (plain member)", ownerID, http.StatusOK},
+		{"unrelated plain member", memberID, http.StatusForbidden},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			w := beginLarkInstallAs(tc.userID, agentID)
+			if w.Code != tc.want {
+				t.Fatalf("BeginLarkInstall as %s: want %d, got %d: %s",
+					tc.name, tc.want, w.Code, w.Body.String())
+			}
+			if tc.want == http.StatusOK {
+				var resp BeginLarkInstallResponse
+				if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+					t.Fatalf("decode begin response: %v", err)
+				}
+				if resp.SessionID == "" || resp.QRCodeURL == "" {
+					t.Fatalf("expected a session id and QR URL, got %+v", resp)
+				}
+			}
+		})
+	}
+}
+
+// TestGetLarkInstallStatus_ScopedToInitiatorOrAdmin verifies the status poll is
+// readable by the session's initiator (the agent owner who began it) and by a
+// workspace owner/admin, but returns 404 (no existence leak) to an unrelated
+// member (MUL-4213).
+func TestGetLarkInstallStatus_ScopedToInitiatorOrAdmin(t *testing.T) {
+	wireLarkInstallServices(t)
+	agentID, ownerID, memberID := privateAgentTestFixture(t)
+
+	// The agent owner begins the install, so they are the session initiator.
+	begin := beginLarkInstallAs(ownerID, agentID)
+	if begin.Code != http.StatusOK {
+		t.Fatalf("begin as agent owner: want 200, got %d: %s", begin.Code, begin.Body.String())
+	}
+	var beginResp BeginLarkInstallResponse
+	if err := json.Unmarshal(begin.Body.Bytes(), &beginResp); err != nil {
+		t.Fatalf("decode begin response: %v", err)
+	}
+
+	status := func(userID string) int {
+		req := newRequestAs(userID, http.MethodGet,
+			"/api/workspaces/"+testWorkspaceID+"/lark/install/"+beginResp.SessionID+"/status", nil)
+		req = withURLParams(req, "id", testWorkspaceID, "sessionId", beginResp.SessionID)
+		w := httptest.NewRecorder()
+		testHandler.GetLarkInstallStatus(w, req)
+		return w.Code
+	}
+
+	if code := status(ownerID); code != http.StatusOK {
+		t.Fatalf("status as initiator (agent owner): want 200, got %d", code)
+	}
+	if code := status(testUserID); code != http.StatusOK {
+		t.Fatalf("status as workspace owner/admin: want 200, got %d", code)
+	}
+	if code := status(memberID); code != http.StatusNotFound {
+		t.Fatalf("status as unrelated member: want 404, got %d", code)
+	}
+}
+
+// TestRevokeLarkInstallation_AuthorizesAgentOwnerAndAdmins mirrors the bind
+// authorization on the unbind path: the bound agent's owner (a plain member)
+// and workspace owner/admins may revoke, an unrelated member may not (MUL-4213).
+func TestRevokeLarkInstallation_AuthorizesAgentOwnerAndAdmins(t *testing.T) {
+	wireLarkInstallServices(t)
+	agentID, ownerID, memberID := privateAgentTestFixture(t)
+
+	// A unique (workspace_id, agent_id, channel_type) constraint means at
+	// most one Feishu row per agent, so each subcase starts from a clean
+	// active row (delete any prior, then insert).
+	seedInstallation := func() string {
+		if _, err := testPool.Exec(context.Background(),
+			`DELETE FROM channel_installation WHERE workspace_id = $1 AND agent_id = $2 AND channel_type = 'feishu'`,
+			testWorkspaceID, agentID); err != nil {
+			t.Fatalf("clear prior installation: %v", err)
+		}
+		var instID string
+		if err := testPool.QueryRow(context.Background(), `
+INSERT INTO channel_installation (workspace_id, agent_id, channel_type, config, installer_user_id, status)
+VALUES ($1, $2, 'feishu', jsonb_build_object('app_id', 'cli_revoke_test'), $3, 'active')
+RETURNING id
+`, testWorkspaceID, agentID, ownerID).Scan(&instID); err != nil {
+			t.Fatalf("seed installation: %v", err)
+		}
+		t.Cleanup(func() {
+			testPool.Exec(context.Background(), `DELETE FROM channel_installation WHERE id = $1`, instID)
+		})
+		return instID
+	}
+
+	revoke := func(userID, instID string) int {
+		req := newRequestAs(userID, http.MethodDelete,
+			"/api/workspaces/"+testWorkspaceID+"/lark/installations/"+instID, nil)
+		req = withURLParams(req, "id", testWorkspaceID, "installationId", instID)
+		w := httptest.NewRecorder()
+		testHandler.RevokeLarkInstallation(w, req)
+		return w.Code
+	}
+
+	// Unrelated member: forbidden.
+	if code := revoke(memberID, seedInstallation()); code != http.StatusForbidden {
+		t.Fatalf("revoke as unrelated member: want 403, got %d", code)
+	}
+	// Agent owner: allowed.
+	if code := revoke(ownerID, seedInstallation()); code != http.StatusNoContent {
+		t.Fatalf("revoke as agent owner: want 204, got %d", code)
+	}
+	// Workspace owner/admin: allowed.
+	if code := revoke(testUserID, seedInstallation()); code != http.StatusNoContent {
+		t.Fatalf("revoke as workspace owner: want 204, got %d", code)
+	}
+}
+
+// TestRevokeLarkInstallation_OrphanCleanableByAdminNotMember pins the
+// orphan-cleanup path (Elon review on MUL-4213 / PR #5079): when the bound
+// agent has been hard-deleted, the agent-owner authorization has no agent to
+// resolve, so revoke must fall back to workspace owner/admin — a workspace
+// owner can still disconnect the orphan (the documented cleanup entry point),
+// while a plain member cannot.
+func TestRevokeLarkInstallation_OrphanCleanableByAdminNotMember(t *testing.T) {
+	wireLarkInstallServices(t)
+	// Only need a plain member; the installation binds a deleted agent.
+	_, _, memberID := privateAgentTestFixture(t)
+
+	// agent_id references no agent row (hard-deleted / never existed). There
+	// is no FK, so the row persists as an orphan — exactly the ListByWorkspace
+	// case the active-connection query filters out.
+	const orphanAgent = "5d0a0000-0000-4000-8000-0000000000aa"
+	seedOrphan := func() string {
+		if _, err := testPool.Exec(context.Background(),
+			`DELETE FROM channel_installation WHERE workspace_id = $1 AND agent_id = $2 AND channel_type = 'feishu'`,
+			testWorkspaceID, orphanAgent); err != nil {
+			t.Fatalf("clear prior orphan: %v", err)
+		}
+		var instID string
+		if err := testPool.QueryRow(context.Background(), `
+INSERT INTO channel_installation (workspace_id, agent_id, channel_type, config, installer_user_id, status)
+VALUES ($1, $2, 'feishu', jsonb_build_object('app_id', 'cli_orphan'), $3, 'active')
+RETURNING id
+`, testWorkspaceID, orphanAgent, testUserID).Scan(&instID); err != nil {
+			t.Fatalf("seed orphan installation: %v", err)
+		}
+		t.Cleanup(func() {
+			testPool.Exec(context.Background(), `DELETE FROM channel_installation WHERE id = $1`, instID)
+		})
+		return instID
+	}
+
+	revoke := func(userID, instID string) int {
+		req := newRequestAs(userID, http.MethodDelete,
+			"/api/workspaces/"+testWorkspaceID+"/lark/installations/"+instID, nil)
+		req = withURLParams(req, "id", testWorkspaceID, "installationId", instID)
+		w := httptest.NewRecorder()
+		testHandler.RevokeLarkInstallation(w, req)
+		return w.Code
+	}
+
+	// Plain member: no agent to own, so orphan cleanup is denied.
+	if code := revoke(memberID, seedOrphan()); code != http.StatusForbidden {
+		t.Fatalf("orphan revoke as plain member: want 403, got %d", code)
+	}
+	// Workspace owner/admin: can disconnect the orphan.
+	if code := revoke(testUserID, seedOrphan()); code != http.StatusNoContent {
+		t.Fatalf("orphan revoke as workspace owner: want 204, got %d", code)
+	}
+}
--- a/server/internal/integrations/lark/registration_service.go
+++ b/server/internal/integrations/lark/registration_service.go
@@ -244,6 +244,10 @@ func (s *registrationSession) snapshot() RegistrationSessionState {
 		InstallationID: s.installationID,
 		ErrorReason:    s.errorReason,
 		ErrorMessage:   s.errorMessage,
+		// InitiatorID is immutable after BeginInstall, so it is safe to
+		// read outside the mutex too — it is included here so the status
+		// handler can scope reads to the session's initiator.
+		InitiatorID: s.initiatorID,
 	}
 }

@@ -272,18 +276,22 @@ func (s *registrationSession) markError(reason, msg string, gcAfter time.Time) {

 // RegistrationSessionState is the read-only snapshot the handler
 // serializes to the frontend. Internal mutex is hidden by construction.
+// InitiatorID is not serialized to the client — the handler uses it only
+// to authorize the status read (session initiator or workspace admin).
 type RegistrationSessionState struct {
 	ID             string
 	Status         RegistrationSessionStatus
 	InstallationID pgtype.UUID
 	ErrorReason    string
 	ErrorMessage   string
+	InitiatorID    pgtype.UUID
 }

 // BeginInstallParams is the trusted input from the handler — the
 // workspace, agent, and initiating user have already been authenticated
-// and authorized at the router (admin role on the workspace; agent
-// belongs to the workspace).
+// and authorized by the handler (canManageAgent: the agent's owner OR a
+// workspace owner/admin; agent belongs to the workspace). The service
+// re-checks agent↔workspace membership below as defense-in-depth.
 type BeginInstallParams struct {
 	WorkspaceID pgtype.UUID
 	AgentID     pgtype.UUID
@@ -323,11 +331,11 @@ func (s *RegistrationService) BeginInstall(ctx context.Context, p BeginInstallPa
 	if !p.WorkspaceID.Valid || !p.AgentID.Valid || !p.InitiatorID.Valid {
 		return BeginInstallResult{}, errors.New("lark registration: workspace, agent, and initiator are required")
 	}
-	// Agent ownership pre-check — without this, a workspace admin
-	// could open an install session against another workspace's agent
-	// by guessing the UUID, and the device_code minted against Lark
-	// would still produce credentials. The handler does the same
-	// check; doing it here too keeps the service self-defending.
+	// Agent↔workspace pre-check — without this, a caller could open an
+	// install session against another workspace's agent by guessing the
+	// UUID, and the device_code minted against Lark would still produce
+	// credentials. The handler already loads this agent to run
+	// canManageAgent; re-checking here keeps the service self-defending.
 	//
 	// We keep the agent: its name pre-fills the bot name on Lark's
 	// PersonalAgent creation form (see botNamePreset) so the installed