mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 06:17:41 +00:00
feat(tools): fail-closed cross-target guard in message tool
Production trace 019d9fcf: agent in DM session called
message(target="-1003787954683:topic:1") and leaked a user's image to
the wrong Telegram group topic. No guardrail caught it.
Add explicit forward opt-in: in ENFORCED sessions, cross-target sends
require forward=true + non-empty forward_reason. Otherwise blocked with
a self-explanatory error that tells the LLM how to retry correctly.
Successful forwards emit slog.Warn("message.cross_target_forward") and
post a user-facing notice back to the origin chat via noticeOnSuccess
wrapper — notice only fires when the send itself succeeds, so failed
forwards no longer announce phantom deliveries.
Schema gains forward and forward_reason fields; description explains
when to set target vs omit.
This commit is contained in:
1 parent
3fa8a02d4b
commit
4658014ac8
1 file changed
+53
-8
@@ -34,13 +34,13 @@ func NewMessageTool(workspace string, restrict bool) *MessageTool {
|
||||
return &MessageTool{workspace: workspace, restrict: restrict}
|
||||
}
|
||||
|
||||
func (t *MessageTool) SetChannelSender(s ChannelSender) { t.sender = s }
|
||||
func (t *MessageTool) SetMessageBus(b *bus.MessageBus) { t.msgBus = b }
|
||||
func (t *MessageTool) SetChannelSender(s ChannelSender) { t.sender = s }
|
||||
func (t *MessageTool) SetMessageBus(b *bus.MessageBus) { t.msgBus = b }
|
||||
func (t *MessageTool) SetChannelTenantChecker(c ChannelTenantChecker) { t.tenantChecker = c }
|
||||
|
||||
func (t *MessageTool) Name() string { return "message" }
|
||||
func (t *MessageTool) Description() string {
|
||||
return "Send a message to a channel (Telegram, Discord, Slack, Zalo, Feishu/Lark, WhatsApp, etc.) or the current chat. Channel and target are auto-filled from context."
|
||||
return "Send a message to a channel (Telegram, Discord, Slack, Zalo, Feishu/Lark, WhatsApp, etc.). In a DM/group, omit `target` to reply to the current chat — DO NOT set a different target unless the user explicitly asked you to forward (then set `forward=true` + `forward_reason` quoting the request). In cron/heartbeat/subagent/team contexts, set `target` per job spec."
|
||||
}
|
||||
|
||||
func (t *MessageTool) Parameters() map[string]any {
|
||||
@@ -64,6 +64,14 @@ func (t *MessageTool) Parameters() map[string]any {
|
||||
"type": "string",
|
||||
"description": "Message content to send. To send a file as attachment, use the prefix MEDIA: followed by the file path, e.g. 'MEDIA:docs/report.pdf' or 'MEDIA:/tmp/image.png'. The file will be uploaded as a document/photo/audio depending on its type.",
|
||||
},
|
||||
"forward": map[string]any{
|
||||
"type": "boolean",
|
||||
"description": "Set true ONLY when the user explicitly asked to forward to a different chat than the current one. Required when target ≠ current chat in DM/group sessions.",
|
||||
},
|
||||
"forward_reason": map[string]any{
|
||||
"type": "string",
|
||||
"description": "Quote the user's literal request when forward=true (e.g. 'gửi báo cáo này sang group dev'). Required when forward=true.",
|
||||
},
|
||||
},
|
||||
"required": []string{"action", "message"},
|
||||
}
|
||||
@@ -134,9 +142,46 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
return err
|
||||
}
|
||||
|
||||
// Cross-target guard: in DM/group/default sessions, prevent the agent from
|
||||
// sending to a chat other than the one bound to its context. FREE session
|
||||
// kinds (cron/heartbeat/subagent/team) compose targets per job spec and
|
||||
// bypass this guard. Opt-in forwarding requires forward=true +
|
||||
// non-empty forward_reason; notice is posted back to the origin chat and
|
||||
// an slog audit line is emitted.
|
||||
sessionKey := ToolSessionKeyFromCtx(ctx)
|
||||
var forwardReason string // non-empty ⇒ guard allowed a cross-target forward; post notice on success
|
||||
if MessageTargetEnforced(sessionKey) {
|
||||
crossTarget := channel != ctxChannel || target != ctxChatID
|
||||
if crossTarget && (ctxChannel != "" || ctxChatID != "") {
|
||||
forward, _ := args["forward"].(bool)
|
||||
reason := strings.TrimSpace(argString(args, "forward_reason"))
|
||||
if !forward || reason == "" {
|
||||
return ErrorResult(fmt.Sprintf(
|
||||
"Cross-target send blocked. You are bound to %s/%s but tried to send to %s/%s. "+
|
||||
"If the user explicitly asked you to forward, retry with forward=true AND forward_reason=\"<quote user's literal request>\".",
|
||||
ctxChannel, ctxChatID, channel, target))
|
||||
}
|
||||
slog.Warn("message.cross_target_forward",
|
||||
"session", sessionKey,
|
||||
"from_channel", ctxChannel, "from", ctxChatID,
|
||||
"to_channel", channel, "to", target,
|
||||
"reason", reason)
|
||||
forwardReason = reason
|
||||
}
|
||||
}
|
||||
// noticeOnSuccess posts the cross-target breadcrumb back to origin iff the
|
||||
// forward succeeded (res.IsError == false). Guarantees we never announce
|
||||
// a fake delivery when the downstream sender/bus publish fails.
|
||||
noticeOnSuccess := func(res *Result) *Result {
|
||||
if forwardReason != "" && res != nil && !res.IsError {
|
||||
t.postCrossTargetNotice(ctx, target, forwardReason)
|
||||
}
|
||||
return res
|
||||
}
|
||||
|
||||
// Handle MEDIA: prefix — send file as attachment instead of text.
|
||||
if filePath, ok := t.resolveMediaPath(ctx, message); ok {
|
||||
return t.sendMedia(ctx, channel, target, filePath)
|
||||
return noticeOnSuccess(t.sendMedia(ctx, channel, target, filePath))
|
||||
}
|
||||
|
||||
// Extract embedded MEDIA: paths from multi-line messages.
|
||||
@@ -155,7 +200,7 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
outMsg.Metadata = map[string]string{"group_id": target}
|
||||
}
|
||||
t.msgBus.PublishOutbound(outMsg)
|
||||
return SilentResult(fmt.Sprintf(`{"status":"sent","channel":"%s","target":"%s"}`, channel, target))
|
||||
return noticeOnSuccess(SilentResult(fmt.Sprintf(`{"status":"sent","channel":"%s","target":"%s"}`, channel, target)))
|
||||
}
|
||||
|
||||
// Prefer direct channel sender for immediate delivery.
|
||||
@@ -164,7 +209,7 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
if err := t.sender(ctx, channel, target, message); err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to send message: %v", err))
|
||||
}
|
||||
return SilentResult(fmt.Sprintf(`{"status":"sent","channel":"%s","target":"%s"}`, channel, target))
|
||||
return noticeOnSuccess(SilentResult(fmt.Sprintf(`{"status":"sent","channel":"%s","target":"%s"}`, channel, target)))
|
||||
}
|
||||
|
||||
// Publish via message bus outbound queue.
|
||||
@@ -180,7 +225,7 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
outMsg.Metadata = map[string]string{"group_id": target}
|
||||
}
|
||||
t.msgBus.PublishOutbound(outMsg)
|
||||
return SilentResult(fmt.Sprintf(`{"status":"sent","channel":"%s","target":"%s"}`, channel, target))
|
||||
return noticeOnSuccess(SilentResult(fmt.Sprintf(`{"status":"sent","channel":"%s","target":"%s"}`, channel, target)))
|
||||
}
|
||||
|
||||
// Last resort: direct sender without group metadata.
|
||||
@@ -188,7 +233,7 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
if err := t.sender(ctx, channel, target, message); err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to send message: %v", err))
|
||||
}
|
||||
return SilentResult(fmt.Sprintf(`{"status":"sent","channel":"%s","target":"%s"}`, channel, target))
|
||||
return noticeOnSuccess(SilentResult(fmt.Sprintf(`{"status":"sent","channel":"%s","target":"%s"}`, channel, target)))
|
||||
}
|
||||
|
||||
return ErrorResult("no channel sender or message bus available")
|
||||
|
||||
Reference in new issue
Block a user