- {!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.
+
+ {t(($) => $.tab_body.integrations.members_note)}
+
+ ) : !slackConfigured ? (
{ts(($) => $.slack.not_enabled_title)}
diff --git a/packages/views/settings/components/lark-tab.test.tsx b/packages/views/settings/components/lark-tab.test.tsx
index 4c7840cac8..9916dac311 100644
--- 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(
,
@@ -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(
+
,
+ { 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(
+
,
+ { 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(
diff --git a/packages/views/settings/components/lark-tab.tsx b/packages/views/settings/components/lark-tab.tsx
index f38edb245e..d76224335e 100644
--- 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
diff --git a/server/cmd/server/router.go b/server/cmd/server/router.go
index 1c6ecaa92f..20a78292a6 100644
--- 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
diff --git a/server/internal/handler/lark.go b/server/internal/handler/lark.go
index 3e3d5f63b5..5417ca79fc 100644
--- 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,
diff --git a/server/internal/handler/lark_test.go b/server/internal/handler/lark_test.go
index 2f1f997000..f2dfa7715b 100644
--- 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)
+ }
+}
diff --git a/server/internal/integrations/lark/registration_service.go b/server/internal/integrations/lark/registration_service.go
index 9776659dae..9aba3426c1 100644
--- 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