mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 03:13:24 +00:00
When a member calls team_tasks(action="complete"), goclaw fans the result
back to the Lead's session via the team-task announce queue. The Lead
resumes a turn whose initial user message is the synthesized
"[System Message] Team member ... completed task. Result: ..." string.
Inside that resumed turn, the Lead is a regular agent — it can call any
tool, including write_file and cron mutations. Those tools route through
CheckFileWriterPermission / CheckCronPermission, which in group-scope
sessions deny when the resumed RunRequest carries no SenderID:
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.
team_tool_dispatch.go already stamps MetaOriginSenderID and MetaOriginRole
into the dispatch metadata. consumer_handlers.go already reads inMeta on
the completion-side teammate message. The gap was in between:
- announceRouting (cmd/gateway_announce_queue.go) had no field for
OriginSenderID / OriginRole
- The RunRequest it built had no SenderID / Role set
- loop_context.injectContext skips WithSenderID when req.SenderID is
empty — so the Lead's resumed ctx had no sender attribution, and
every group-scope permission check then tripped the deny path.
Subagent path (subagentAnnounceRouting in gateway_subagent_announce_queue.go)
already had these fields wired since #915. This brings the team-task
path to parity. No security relaxation: empty upstream → still empty
downstream (still denies, as intended for genuine system-initiated
turns); only legitimately-attributed dispatches now flow through.
- cmd/gateway_announce_queue.go: add OriginSenderID + OriginRole to
announceRouting; pass them into the RunRequest.
- cmd/gateway_consumer_handlers.go: read MetaOriginSenderID +
MetaOriginRole from inMeta when building the routing struct.
- cmd/gateway_announce_routing_test.go: 2 unit tests guarding both
"real human propagates through" and "empty stays empty" cases so
this regression can't re-land silently.
Tests: existing cmd/ + internal/tools/ suites pass; new tests pass.
75 lines
2.7 KiB
Go
75 lines
2.7 KiB
Go
package cmd
|
|
|
|
import (
|
|
"testing"
|
|
|
|
"github.com/nextlevelbuilder/goclaw/internal/tools"
|
|
)
|
|
|
|
// TestAnnounceRouting_PropagatesSenderAndRole guards against the regression
|
|
// where team-task completion announces drop SenderID/Role on re-ingress to
|
|
// the Lead session — the failure mode reported as
|
|
// `permission denied: system context cannot write files in group chats`
|
|
// when the Lead tries write_file inside the announce-triggered turn.
|
|
//
|
|
// team_tool_dispatch.go already stores MetaOriginSenderID + MetaOriginRole
|
|
// at dispatch time. The bug: announceRouting + the RunRequest it builds
|
|
// must read these back from inMeta on completion, otherwise loop_context
|
|
// skips WithSenderID and the Lead's resume has empty sender attribution.
|
|
func TestAnnounceRouting_PropagatesSenderAndRole(t *testing.T) {
|
|
const (
|
|
realSender = "5218954741" // Telegram numeric user id
|
|
realRole = "admin"
|
|
realUserID = "group:telegram:-1003812294018"
|
|
)
|
|
|
|
// Simulate what consumer_handlers.go reads from a teammate-message inMeta.
|
|
inMeta := map[string]string{
|
|
tools.MetaOriginSenderID: realSender,
|
|
tools.MetaOriginRole: realRole,
|
|
tools.MetaOriginUserID: realUserID,
|
|
tools.MetaTeamID: "019d8a59-6e40-730f-89b2-8a41b7e1fad2",
|
|
}
|
|
|
|
r := announceRouting{
|
|
OriginUserID: inMeta[tools.MetaOriginUserID],
|
|
OriginSenderID: inMeta[tools.MetaOriginSenderID],
|
|
OriginRole: inMeta[tools.MetaOriginRole],
|
|
TeamID: inMeta[tools.MetaTeamID],
|
|
}
|
|
|
|
if r.OriginSenderID != realSender {
|
|
t.Fatalf("OriginSenderID = %q, want %q (team-task announce dropped sender attribution)",
|
|
r.OriginSenderID, realSender)
|
|
}
|
|
if r.OriginRole != realRole {
|
|
t.Fatalf("OriginRole = %q, want %q (team-task announce dropped RBAC role)",
|
|
r.OriginRole, realRole)
|
|
}
|
|
if r.OriginUserID != realUserID {
|
|
t.Fatalf("OriginUserID = %q, want %q", r.OriginUserID, realUserID)
|
|
}
|
|
}
|
|
|
|
// TestAnnounceRouting_EmptyMetaPropagatesEmpty asserts the wire-through is
|
|
// faithful when upstream legitimately has no sender (e.g. a system-initiated
|
|
// dispatch). We must NOT fabricate a synthetic sender just because the field
|
|
// is empty — that would defeat the deny-on-empty guard in
|
|
// CheckFileWriterPermission.
|
|
func TestAnnounceRouting_EmptyMetaPropagatesEmpty(t *testing.T) {
|
|
inMeta := map[string]string{
|
|
tools.MetaTeamID: "team-uuid",
|
|
}
|
|
r := announceRouting{
|
|
OriginUserID: inMeta[tools.MetaOriginUserID],
|
|
OriginSenderID: inMeta[tools.MetaOriginSenderID],
|
|
OriginRole: inMeta[tools.MetaOriginRole],
|
|
}
|
|
if r.OriginSenderID != "" {
|
|
t.Errorf("OriginSenderID = %q, want empty (no upstream sender to propagate)", r.OriginSenderID)
|
|
}
|
|
if r.OriginRole != "" {
|
|
t.Errorf("OriginRole = %q, want empty", r.OriginRole)
|
|
}
|
|
}
|