From 6a9b747b651b3c39b8f562d2fc54beaecaa3309f Mon Sep 17 00:00:00 2001 From: viettranx Date: Thu, 16 Apr 2026 14:12:49 +0700 Subject: [PATCH] feat: propagate RBAC role through dispatch + bypass file-writer grant for admins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- cmd/gateway_consumer_handlers.go | 18 ++++++++++----- cmd/gateway_consumer_normal.go | 6 +++++ cmd/gateway_subagent_announce_queue.go | 5 ++++ internal/agent/loop_context.go | 6 +++++ internal/agent/loop_types.go | 1 + .../gateway/methods/teams_tasks_mutations.go | 10 ++++++++ internal/store/config_permission_store.go | 23 +++++++++++++++++++ internal/tools/announce_queue.go | 1 + internal/tools/delegate_tool.go | 5 ++++ internal/tools/subagent.go | 1 + internal/tools/subagent_exec.go | 4 ++++ internal/tools/subagent_spawn.go | 2 ++ internal/tools/team_metadata_keys.go | 4 ++++ internal/tools/team_tasks_create.go | 4 ++++ internal/tools/team_tool_dispatch.go | 3 +++ 15 files changed, 87 insertions(+), 6 deletions(-) diff --git a/cmd/gateway_consumer_handlers.go b/cmd/gateway_consumer_handlers.go index cff0cdac..66216cfe 100644 --- a/cmd/gateway_consumer_handlers.go +++ b/cmd/gateway_consumer_handlers.go @@ -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:" 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:" 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], diff --git a/cmd/gateway_consumer_normal.go b/cmd/gateway_consumer_normal.go index eee6e24c..422d2904 100644 --- a/cmd/gateway_consumer_normal.go +++ b/cmd/gateway_consumer_normal.go @@ -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, diff --git a/cmd/gateway_subagent_announce_queue.go b/cmd/gateway_subagent_announce_queue.go index 6b10613b..510947d2 100644 --- a/cmd/gateway_subagent_announce_queue.go +++ b/cmd/gateway_subagent_announce_queue.go @@ -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, diff --git a/internal/agent/loop_context.go b/internal/agent/loop_context.go index a3c8d59d..e8b5cb9e 100644 --- a/internal/agent/loop_context.go +++ b/internal/agent/loop_context.go @@ -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 { diff --git a/internal/agent/loop_types.go b/internal/agent/loop_types.go index 7c2a5b8d..8512c882 100644 --- a/internal/agent/loop_types.go +++ b/internal/agent/loop_types.go @@ -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 diff --git a/internal/gateway/methods/teams_tasks_mutations.go b/internal/gateway/methods/teams_tasks_mutations.go index f4698d38..c88b7fbc 100644 --- a/internal/gateway/methods/teams_tasks_mutations.go +++ b/internal/gateway/methods/teams_tasks_mutations.go @@ -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", diff --git a/internal/store/config_permission_store.go b/internal/store/config_permission_store.go index 3d44c86e..56f5ccbb 100644 --- a/internal/store/config_permission_store.go +++ b/internal/store/config_permission_store.go @@ -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") diff --git a/internal/tools/announce_queue.go b/internal/tools/announce_queue.go index 31f9e410..521b63c4 100644 --- a/internal/tools/announce_queue.go +++ b/internal/tools/announce_queue.go @@ -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 diff --git a/internal/tools/delegate_tool.go b/internal/tools/delegate_tool.go index 541299a9..670aa185 100644 --- a/internal/tools/delegate_tool.go +++ b/internal/tools/delegate_tool.go @@ -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 } diff --git a/internal/tools/subagent.go b/internal/tools/subagent.go index 6a4d85ee..865ee178 100644 --- a/internal/tools/subagent.go +++ b/internal/tools/subagent.go @@ -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"` diff --git a/internal/tools/subagent_exec.go b/internal/tools/subagent_exec.go index 677a1029..cfd38de1 100644 --- a/internal/tools/subagent_exec.go +++ b/internal/tools/subagent_exec.go @@ -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 } diff --git a/internal/tools/subagent_spawn.go b/internal/tools/subagent_spawn.go index 1dc33b7d..c9e43d05 100644 --- a/internal/tools/subagent_spawn.go +++ b/internal/tools/subagent_spawn.go @@ -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), diff --git a/internal/tools/team_metadata_keys.go b/internal/tools/team_metadata_keys.go index bd7ea3ab..5e52b0e9 100644 --- a/internal/tools/team_metadata_keys.go +++ b/internal/tools/team_metadata_keys.go @@ -12,6 +12,10 @@ const ( // so permission checks (e.g. CheckFileWriterPermission) attribute to the // original user rather than a synthetic "subagent:" / "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" diff --git a/internal/tools/team_tasks_create.go b/internal/tools/team_tasks_create.go index fc77f0ee..c1bc3cac 100644 --- a/internal/tools/team_tasks_create.go +++ b/internal/tools/team_tasks_create.go @@ -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, diff --git a/internal/tools/team_tool_dispatch.go b/internal/tools/team_tool_dispatch.go index c72de740..f0679475 100644 --- a/internal/tools/team_tool_dispatch.go +++ b/internal/tools/team_tool_dispatch.go @@ -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 == "" {