mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-09 01:46:46 +08:00
Xichang(Seacen) Zhao
4e2afed720
* fix(channel): refuse an inbound turn from a member who may not run the agent
The web chat checks the agent's invoke permission before it opens a session: a
private agent runs only for its owner, with no admin bypass. The shared channel
Router checked the sender's binding and workspace membership and nothing else,
so any member who had bound a chat identity could run another member's private
agent through its bot — on the owner's runtime, with the owner's credentials. A
1:1 message, a group mention, /issue and /new all got through, on every channel
the engine serves.
The Router now asks service.CanMemberInvokeAgent right after identity
resolution and before anything is stored, so a refused turn reaches no Chat, no
/issue and no later run's context. It judges the SENDER, never the installer who
owns a group's route — judging the installer is what would let this through.
MemberMayInvokeAgent is that existing function keyed by agent id, since the
Router holds the installation's agent id rather than the loaded row. An agent
that no longer exists admits nobody; a lookup that FAILED is an error rather
than a denial, so the dedup claim is released and the platform's redelivery is
still the message's chance.
A refusal is audited as invocation_not_allowed. WeCom answers a 1:1 in place; a
group trigger is answered in the sender's own 1:1 and the room hears nothing,
since a line there would tell everyone present which member was refused and
that the agent is someone's private one. Other channels stay silent, as they do
for a non-member.
* fix(channel): a failed permission lookup is not a denial
The policy swallowed two of its three query errors. CanMemberInvokeAgent
returns a plain bool, so ListAgentInvocationTargets failing came back as false,
and any error from GetMemberByUserAndWorkspace — not only pgx.ErrNoRows — read
as 'not a workspace member'. Only GetAgent's failure reached the caller.
For the scheduled triggers that shipped with it, failing closed is right: an
autopilot or an issue wakeup can wait for the next tick. For a channel turn it
is not. The Router reads false as a verdict, marks the message processed so the
platform's redelivery is discarded, and WeCom tells the sender they may not run
an agent they are in fact allowed to run — all from one transient error.
So the policy is now memberMayInvokeAgent, returning (bool, error):
- (false, nil) stays a real verdict — no user id, not the owner of a private
agent, not a target of a public_to one, and a member row that is simply not
there, which is what pgx.ErrNoRows means here.
- (false, err) is 'we do not know', and every other query failure produces it.
CanMemberInvokeAgent is now a thin wrapper that fails closed, so autopilot and
issue wakeup behave exactly as before. TaskService.MemberMayInvokeAgent returns
the error, and the Router releases its dedup claim on it.
TestRouter_RealServiceLookupFailure_ReleasesInsteadOfDenying drives the REAL
TaskService against a real database with one query failed at the SQL boundary,
because the router tests stub this at the interface and can only prove what the
Router does with an error it is handed — not whether the policy ever produces
one. It asserts Release rather than Mark, no invocation_not_allowed audit, and
no denial notice, for each of the two queries.
Also moves postPrivate's doc comment back to postPrivate: sendInvokeDenied's
was inserted above it, leaving one function documented by the other's text.
77 lines
2.6 KiB
Go
77 lines
2.6 KiB
Go
package service
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"fmt"
|
|
|
|
"github.com/jackc/pgx/v5"
|
|
"github.com/jackc/pgx/v5/pgtype"
|
|
"github.com/multica-ai/multica/server/internal/util"
|
|
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
|
)
|
|
|
|
// CanMemberInvokeAgent applies the existing invocation policy for durable
|
|
// triggers, failing closed: a lookup that did not answer reads as "no".
|
|
//
|
|
// That is the right default for a trigger that fires on its own schedule —
|
|
// autopilot and issue wakeups can wait for the next tick. A caller that is
|
|
// answering a person right now wants the error instead, so it can retry rather
|
|
// than tell them they have no permission: use memberMayInvokeAgent.
|
|
func CanMemberInvokeAgent(ctx context.Context, queries *db.Queries, agent db.Agent, memberUserID pgtype.UUID, workspaceID pgtype.UUID) bool {
|
|
allowed, err := memberMayInvokeAgent(ctx, queries, agent, memberUserID, workspaceID)
|
|
return err == nil && allowed
|
|
}
|
|
|
|
// memberMayInvokeAgent is the policy itself, with query failures kept apart
|
|
// from denials.
|
|
//
|
|
// (false, nil) is a real verdict: no user id, not the owner of a private
|
|
// agent, or a public_to agent this member is not a target of — including the
|
|
// member row simply not being there, which is what pgx.ErrNoRows means here.
|
|
//
|
|
// (false, err) is "we do not know". Collapsing the two is what turns one
|
|
// transient database error into a message the platform will never redeliver
|
|
// and a person told they lack a permission they have.
|
|
func memberMayInvokeAgent(ctx context.Context, queries *db.Queries, agent db.Agent, memberUserID pgtype.UUID, workspaceID pgtype.UUID) (bool, error) {
|
|
userID := util.UUIDToString(memberUserID)
|
|
if userID == "" {
|
|
return false, nil
|
|
}
|
|
if util.UUIDToString(agent.OwnerID) == userID {
|
|
return true, nil
|
|
}
|
|
if agent.PermissionMode != "public_to" {
|
|
return false, nil
|
|
}
|
|
targets, err := queries.ListAgentInvocationTargets(ctx, agent.ID)
|
|
if err != nil {
|
|
return false, fmt.Errorf("list agent invocation targets: %w", err)
|
|
}
|
|
isWorkspaceMember := false
|
|
switch _, err := queries.GetMemberByUserAndWorkspace(ctx, db.GetMemberByUserAndWorkspaceParams{
|
|
UserID: memberUserID,
|
|
WorkspaceID: workspaceID,
|
|
}); {
|
|
case err == nil:
|
|
isWorkspaceMember = true
|
|
case errors.Is(err, pgx.ErrNoRows):
|
|
// Not a member. A fact, not a failure.
|
|
default:
|
|
return false, fmt.Errorf("load workspace member: %w", err)
|
|
}
|
|
for _, t := range targets {
|
|
switch t.TargetType {
|
|
case "workspace":
|
|
if isWorkspaceMember {
|
|
return true, nil
|
|
}
|
|
case "member":
|
|
if util.UUIDToString(t.TargetID) == userID {
|
|
return true, nil
|
|
}
|
|
}
|
|
}
|
|
return false, nil
|
|
}
|