mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 03:13:24 +00:00
feat: propagate RBAC role through dispatch + bypass file-writer grant for admins
Dashboard users (and other tenant-authenticated admin/operator/owner
actors) dispatching team tasks to agents in Telegram/Discord groups
previously hit the empty-sender DENY rule when the assigned agent
tried to write files — because dashboard users have tenant identities
(e.g. "viettx") that don't match the numeric file-writer grants stored
per channel.
Two-part fix:
1. CheckFileWriterPermission / CheckCronPermission add an RBAC bypass
at the store layer: if ctx carries RoleFromContext in {admin,
operator, owner}, skip the per-user grant check. Tenant RBAC already
pre-authenticated these callers at the gateway edge; requiring a
redundant per-channel grant blocks legitimate admin work.
2. Role propagation through the full dispatch chain:
ctx (RoleFromContext)
→ RunRequest.Role
→ SubagentTask.OriginRole / AnnounceMetadata.OriginRole
/ DelegateRequest.Role
→ InboundMessage.Metadata[MetaOriginRole]
→ subagentAnnounceRouting.Role / teammateRole
→ RunRequest.Role (re-ingress)
→ store.WithRole(ctx, ...) in loop_context
Covers subagent announce, delegate announce, teammate dispatch
(WS dashboard, tool), session_send, and processNormalMessage's
synthetic-sender override path. team_tasks_create persists
origin_role for deferred dispatches.
Viewer and empty roles fall through to the existing sender-based path
(no bypass).
Regression tests:
- A.10 AdminRoleBypass: admin/operator/owner in ctx allowed even with
empty sender (normally DENY).
- A.11 ViewerRoleDoesNotBypass: viewer and empty roles still hit DENY.
Builds + integration + unit tests green.
This commit is contained in:
1 parent
51cf103714
commit
6a9b747b65
15 files changed
+87
-6
No files matched your search
@@ -109,10 +109,12 @@ func handleSubagentAnnounce(
|
||||
Iterations: iterations,
|
||||
}
|
||||
|
||||
// Preserve real acting sender from original turn so permission checks
|
||||
// (e.g. write_file in group chat) attribute to the user, not the
|
||||
// synthetic "subagent:<id>" sender of the announce message itself (#915).
|
||||
// Preserve real acting sender + RBAC role from original turn so permission
|
||||
// checks (e.g. write_file in group chat) attribute to the user and can
|
||||
// bypass per-user grants for authenticated admins, not the synthetic
|
||||
// "subagent:<id>" sender of the announce message itself (#915).
|
||||
originSenderID := msg.Metadata[tools.MetaOriginSenderID]
|
||||
originRole := msg.Metadata[tools.MetaOriginRole]
|
||||
|
||||
queueKey := fmt.Sprintf("%s:%s", msg.TenantID, sessionKey)
|
||||
routing := subagentAnnounceRouting{
|
||||
@@ -126,6 +128,7 @@ func handleSubagentAnnounce(
|
||||
OrigLocalKey: origLocalKey,
|
||||
UserID: announceUserID,
|
||||
SenderID: originSenderID,
|
||||
Role: originRole,
|
||||
ParentAgent: parentAgent,
|
||||
ParentTraceID: parentTraceID,
|
||||
ParentRootSpanID: parentRootSpanID,
|
||||
@@ -211,10 +214,12 @@ func handleTeammateMessage(
|
||||
announceUserID = fmt.Sprintf("group:%s:%s", origChannel, origChatID)
|
||||
}
|
||||
|
||||
// Preserve real acting sender through teammate dispatch so permission
|
||||
// checks during the teammate's turn (e.g. write_file in group chat)
|
||||
// attribute to the original user (#915).
|
||||
// Preserve real acting sender + RBAC role through teammate dispatch so
|
||||
// permission checks during the teammate's turn (e.g. write_file in group
|
||||
// chat) attribute to the original user and can bypass per-user grants
|
||||
// for authenticated admins (#915).
|
||||
teammateSenderID := msg.Metadata[tools.MetaOriginSenderID]
|
||||
teammateRole := msg.Metadata[tools.MetaOriginRole]
|
||||
|
||||
outMeta := buildAnnounceOutMeta(origLocalKey)
|
||||
|
||||
@@ -245,6 +250,7 @@ func handleTeammateMessage(
|
||||
LocalKey: origLocalKey,
|
||||
UserID: announceUserID,
|
||||
SenderID: teammateSenderID, // real user who triggered the teammate dispatch (#915)
|
||||
Role: teammateRole, // RBAC role for admin bypass during teammate turn (#915)
|
||||
RunID: fmt.Sprintf("teammate-%s-%s", msg.Metadata[tools.MetaFromAgent], msg.Metadata[tools.MetaToAgent]),
|
||||
Stream: false,
|
||||
TeamTaskID: msg.Metadata[tools.MetaTeamTaskID],
|
||||
|
||||
@@ -363,6 +363,11 @@ func processNormalMessage(
|
||||
effectiveSenderID = realSender
|
||||
}
|
||||
}
|
||||
// Role propagation: carry the RBAC role of the originating actor so
|
||||
// permission checks during the re-ingress turn can bypass per-user
|
||||
// grants for authenticated admins (#915). Only present when the
|
||||
// upstream dispatch set MetaOriginRole.
|
||||
effectiveRole := msg.Metadata[tools.MetaOriginRole]
|
||||
|
||||
// Schedule through main lane (per-session concurrency controlled by maxConcurrent)
|
||||
outCh := deps.Sched.ScheduleWithOpts(schedCtx, "main", agent.RunRequest{
|
||||
@@ -378,6 +383,7 @@ func processNormalMessage(
|
||||
LocalKey: msg.Metadata["local_key"],
|
||||
UserID: userID,
|
||||
SenderID: effectiveSenderID,
|
||||
Role: effectiveRole,
|
||||
SenderName: resolveSenderName(msg),
|
||||
RunID: runID,
|
||||
Stream: enableStream,
|
||||
|
||||
@@ -50,6 +50,9 @@ func makeDelegateAnnounceCallback(
|
||||
if meta.OriginSenderID != "" {
|
||||
batchMeta[tools.MetaOriginSenderID] = meta.OriginSenderID
|
||||
}
|
||||
if meta.OriginRole != "" {
|
||||
batchMeta[tools.MetaOriginRole] = meta.OriginRole
|
||||
}
|
||||
if meta.OriginUserID != "" {
|
||||
batchMeta[tools.MetaOriginUserID] = meta.OriginUserID
|
||||
}
|
||||
@@ -102,6 +105,7 @@ type subagentAnnounceRouting struct {
|
||||
OrigLocalKey string
|
||||
UserID string
|
||||
SenderID string // real acting sender (preserves permission attribution through re-ingress, #915)
|
||||
Role string // caller's RBAC role; bypasses per-user grants for admin/operator/owner (#915)
|
||||
ParentAgent string
|
||||
ParentTraceID uuid.UUID
|
||||
ParentRootSpanID uuid.UUID
|
||||
@@ -176,6 +180,7 @@ func processSubagentAnnounceLoop(
|
||||
LocalKey: r.OrigLocalKey,
|
||||
UserID: r.UserID,
|
||||
SenderID: r.SenderID, // preserves real acting sender for permission checks (#915)
|
||||
Role: r.Role, // preserves RBAC role for admin bypass in group writes (#915)
|
||||
RunID: fmt.Sprintf("subagent-announce-%s-%d", r.ParentAgent, len(entries)),
|
||||
RunKind: "announce",
|
||||
HideInput: true,
|
||||
|
||||
@@ -67,6 +67,12 @@ func (l *Loop) injectContext(ctx context.Context, req *RunRequest) (contextSetup
|
||||
if req.SenderName != "" {
|
||||
ctx = store.WithSenderName(ctx, req.SenderName)
|
||||
}
|
||||
// Inject caller role so RBAC-aware permission checks (CheckFileWriterPermission,
|
||||
// CheckCronPermission) can bypass per-user grants for authenticated admins
|
||||
// dispatched from dashboard or other trusted sources (#915).
|
||||
if req.Role != "" {
|
||||
ctx = store.WithRole(ctx, req.Role)
|
||||
}
|
||||
// Inject global + per-agent builtin tool settings (tier 1+3).
|
||||
// Media/provider-chain tools read the merged view via BuiltinToolSettingsFromCtx.
|
||||
if l.builtinToolSettings != nil {
|
||||
|
||||
@@ -562,6 +562,7 @@ type RunRequest struct {
|
||||
UserID string // external user ID (TEXT, free-form) for multi-tenant scoping
|
||||
SenderID string // original individual sender ID (preserved in group chats for permission checks)
|
||||
SenderName string // display name from channel metadata (for bootstrap auto-contact)
|
||||
Role string // caller's RBAC role (admin/operator/viewer/owner); bypasses per-user grants for authenticated admins (#915)
|
||||
Stream bool // whether to stream response chunks
|
||||
ExtraSystemPrompt string // optional: injected into system prompt (skills, subagent context, etc.)
|
||||
SkillFilter []string // per-request skill override: nil=use agent default, []=no skills, ["x","y"]=whitelist
|
||||
|
||||
@@ -410,6 +410,16 @@ func (m *TeamsMethods) dispatchTaskToAgent(ctx context.Context, task *store.Team
|
||||
meta["origin_sender_id"] = taskSender
|
||||
}
|
||||
}
|
||||
// Propagate RBAC role so the teammate's permission checks can bypass
|
||||
// per-user grants for authenticated admin dispatchers (#915). Live WS
|
||||
// caller's role wins; falls back to role stored on the task at create.
|
||||
if dispatchRole := store.RoleFromContext(ctx); dispatchRole != "" {
|
||||
meta["origin_role"] = dispatchRole
|
||||
} else if task.Metadata != nil {
|
||||
if taskRole, _ := task.Metadata["origin_role"].(string); taskRole != "" {
|
||||
meta["origin_role"] = taskRole
|
||||
}
|
||||
}
|
||||
|
||||
m.msgBus.PublishInbound(bus.InboundMessage{
|
||||
Channel: "system",
|
||||
|
||||
@@ -72,6 +72,13 @@ func CheckFileWriterPermission(ctx context.Context, permStore ConfigPermissionSt
|
||||
if agentID == uuid.Nil {
|
||||
return nil // no agent context
|
||||
}
|
||||
// RBAC bypass: admin / operator / owner roles are pre-authenticated by
|
||||
// the tenant RBAC system (dashboard users, tenant admins). File-writer
|
||||
// grants exist to gate random group members; authenticated admins
|
||||
// shouldn't trip over them when dispatching work that writes files.
|
||||
if isAdminRole(ctx) {
|
||||
return nil
|
||||
}
|
||||
senderID := SenderIDFromContext(ctx)
|
||||
if senderID == "" || isSyntheticSender(senderID) {
|
||||
return fmt.Errorf("permission denied: system context cannot write files in group chats. If this is a legitimate user action, ensure the acting sender is preserved through the tool chain")
|
||||
@@ -87,6 +94,19 @@ func CheckFileWriterPermission(ctx context.Context, permStore ConfigPermissionSt
|
||||
return nil
|
||||
}
|
||||
|
||||
// isAdminRole reports whether ctx carries an elevated RBAC role
|
||||
// (admin / operator / owner) that should bypass per-user file-writer
|
||||
// grants. Tenant-authenticated identities pre-pass RBAC at the gateway
|
||||
// edge; re-checking per-channel grants here is redundant and blocks
|
||||
// legitimate dashboard-dispatched work (#915).
|
||||
func isAdminRole(ctx context.Context) bool {
|
||||
switch RoleFromContext(ctx) {
|
||||
case "admin", "operator", RoleOwner:
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// isSyntheticSender reports whether senderID is an internal system component
|
||||
// (not a real user). Mirrors bus.IsInternalSender — kept here to avoid the
|
||||
// store→bus import dependency. If prefixes change, update both.
|
||||
@@ -114,6 +134,9 @@ func CheckCronPermission(ctx context.Context, permStore ConfigPermissionStore) e
|
||||
if agentID == uuid.Nil {
|
||||
return nil // no agent context
|
||||
}
|
||||
if isAdminRole(ctx) {
|
||||
return nil // RBAC bypass (admin/operator/owner)
|
||||
}
|
||||
senderID := SenderIDFromContext(ctx)
|
||||
if senderID == "" || isSyntheticSender(senderID) {
|
||||
return fmt.Errorf("permission denied: system context cannot manage cron jobs in group chats")
|
||||
|
||||
@@ -32,6 +32,7 @@ type AnnounceMetadata struct {
|
||||
OriginLocalKey string // composite key with topic/thread suffix for routing
|
||||
OriginUserID string
|
||||
OriginSenderID string // real acting sender; preserves permission attribution through re-ingress (#915)
|
||||
OriginRole string // caller's RBAC role; bypasses per-user grants for admin/operator/owner (#915)
|
||||
OriginSessionKey string // exact parent session key (WS uses non-standard format)
|
||||
OriginTenantID uuid.UUID // parent tenant for announce routing
|
||||
ParentAgent string
|
||||
|
||||
@@ -35,6 +35,7 @@ type DelegateRequest struct {
|
||||
DelegationID string
|
||||
UserID string
|
||||
SenderID string // real acting sender preserved through delegate announce re-ingress (#915)
|
||||
Role string // caller's RBAC role; bypasses per-user grants for admin/operator/owner (#915)
|
||||
TenantID string
|
||||
Channel string
|
||||
ChatID string
|
||||
@@ -150,6 +151,7 @@ func (t *DelegateTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
DelegationID: delegationID,
|
||||
UserID: actorID,
|
||||
SenderID: store.SenderIDFromContext(ctx),
|
||||
Role: store.RoleFromContext(ctx),
|
||||
TenantID: store.TenantIDFromContext(ctx).String(),
|
||||
Channel: ToolChannelFromCtx(ctx),
|
||||
ChatID: ToolChatIDFromCtx(ctx),
|
||||
@@ -299,6 +301,9 @@ func (t *DelegateTool) announceToParent(req DelegateRequest, content string, med
|
||||
if req.SenderID != "" {
|
||||
meta[MetaOriginSenderID] = req.SenderID
|
||||
}
|
||||
if req.Role != "" {
|
||||
meta[MetaOriginRole] = req.Role
|
||||
}
|
||||
if req.UserID != "" {
|
||||
meta[MetaOriginUserID] = req.UserID
|
||||
}
|
||||
|
||||
@@ -56,6 +56,7 @@ type SubagentTask struct {
|
||||
OriginLocalKey string `json:"originLocalKey,omitempty"` // composite key with topic/thread suffix for routing
|
||||
OriginUserID string `json:"originUserId,omitempty"` // parent's userID for per-user scoping propagation
|
||||
OriginSenderID string `json:"originSenderId,omitempty"` // real acting sender; preserves permission attribution in announce re-ingress (#915)
|
||||
OriginRole string `json:"originRole,omitempty"` // parent's RBAC role; bypasses per-user grants for admin/operator/owner in re-ingress (#915)
|
||||
OriginSessionKey string `json:"originSessionKey,omitempty"` // exact parent session key for announce routing (WS uses non-standard format)
|
||||
CreatedAt int64 `json:"createdAt"`
|
||||
CompletedAt int64 `json:"completedAt,omitempty"`
|
||||
|
||||
@@ -43,6 +43,7 @@ func (sm *SubagentManager) runTask(ctx context.Context, task *SubagentTask, call
|
||||
OriginLocalKey: task.OriginLocalKey,
|
||||
OriginUserID: task.OriginUserID,
|
||||
OriginSenderID: task.OriginSenderID,
|
||||
OriginRole: task.OriginRole,
|
||||
OriginSessionKey: task.OriginSessionKey,
|
||||
OriginTenantID: task.OriginTenantID,
|
||||
ParentAgent: task.ParentID,
|
||||
@@ -83,6 +84,9 @@ func (sm *SubagentManager) runTask(ctx context.Context, task *SubagentTask, call
|
||||
if task.OriginSenderID != "" {
|
||||
announceMeta[MetaOriginSenderID] = task.OriginSenderID
|
||||
}
|
||||
if task.OriginRole != "" {
|
||||
announceMeta[MetaOriginRole] = task.OriginRole
|
||||
}
|
||||
if task.OriginUserID != "" {
|
||||
announceMeta[MetaOriginUserID] = task.OriginUserID
|
||||
}
|
||||
|
||||
@@ -85,6 +85,7 @@ func (sm *SubagentManager) Spawn(
|
||||
OriginLocalKey: ToolLocalKeyFromCtx(ctx),
|
||||
OriginUserID: store.UserIDFromContext(ctx),
|
||||
OriginSenderID: store.SenderIDFromContext(ctx),
|
||||
OriginRole: store.RoleFromContext(ctx),
|
||||
OriginSessionKey: ToolSessionKeyFromCtx(ctx),
|
||||
OriginTenantID: store.TenantIDFromContext(ctx),
|
||||
OriginTraceID: tracing.TraceIDFromContext(ctx),
|
||||
@@ -163,6 +164,7 @@ func (sm *SubagentManager) RunSync(
|
||||
OriginLocalKey: ToolLocalKeyFromCtx(ctx),
|
||||
OriginUserID: store.UserIDFromContext(ctx),
|
||||
OriginSenderID: store.SenderIDFromContext(ctx),
|
||||
OriginRole: store.RoleFromContext(ctx),
|
||||
OriginSessionKey: ToolSessionKeyFromCtx(ctx),
|
||||
OriginTenantID: store.TenantIDFromContext(ctx),
|
||||
OriginTraceID: tracing.TraceIDFromContext(ctx),
|
||||
|
||||
@@ -12,6 +12,10 @@ const (
|
||||
// so permission checks (e.g. CheckFileWriterPermission) attribute to the
|
||||
// original user rather than a synthetic "subagent:<id>" / "notification:system" string.
|
||||
MetaOriginSenderID = "origin_sender_id"
|
||||
// MetaOriginRole carries the caller's RBAC role through dispatch + re-ingress
|
||||
// so permission checks can bypass per-user grants for authenticated admins
|
||||
// (e.g. dashboard user dispatches a task that writes files in a group chat).
|
||||
MetaOriginRole = "origin_role"
|
||||
MetaOriginLocalKey = "origin_local_key"
|
||||
MetaOriginSessionKey = "origin_session_key"
|
||||
MetaOriginTraceID = "origin_trace_id"
|
||||
|
||||
@@ -209,6 +209,10 @@ func (t *TeamTasksTool) executeCreate(ctx context.Context, args map[string]any)
|
||||
if sender := store.SenderIDFromContext(ctx); sender != "" {
|
||||
taskMeta["origin_sender_id"] = sender
|
||||
}
|
||||
// Persist caller role for RBAC-aware bypass at dispatch time.
|
||||
if role := store.RoleFromContext(ctx); role != "" {
|
||||
taskMeta["origin_role"] = role
|
||||
}
|
||||
|
||||
task := &store.TeamTaskData{
|
||||
TeamID: team.ID,
|
||||
|
||||
@@ -171,6 +171,9 @@ func (m *TeamToolManager) dispatchTaskToAgent(ctx context.Context, task *store.T
|
||||
if originSenderID != "" {
|
||||
meta[MetaOriginSenderID] = originSenderID
|
||||
}
|
||||
if originRole := store.RoleFromContext(ctx); originRole != "" {
|
||||
meta[MetaOriginRole] = originRole
|
||||
}
|
||||
// Resolve local key from context; fallback to task metadata for deferred dispatches.
|
||||
localKey := ToolLocalKeyFromCtx(ctx)
|
||||
if localKey == "" {
|
||||
|
||||
Reference in new issue
Block a user