From ceebda35ee5a519017ecba6b2098d093f93bd9df Mon Sep 17 00:00:00 2001 From: Anton Bogdanovich <27antonb@gmail.com> Date: Fri, 22 May 2026 16:20:59 -0700 Subject: [PATCH] fix(message): gate local media attachments --- docs/guides/configuration.md | 1 + pkg/agent/agent_init.go | 40 ++++++--- pkg/config/config.go | 8 +- pkg/config/config_test.go | 10 +++ pkg/config/defaults.go | 7 +- pkg/tools/integration/message.go | 124 +++++++++++++++----------- pkg/tools/integration/message_test.go | 64 +++++++++++-- 7 files changed, 178 insertions(+), 76 deletions(-) diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md index ef4802a78..d582676e7 100644 --- a/docs/guides/configuration.md +++ b/docs/guides/configuration.md @@ -400,6 +400,7 @@ Even with `restrict_to_workspace: false`, the `exec` tool blocks these dangerous |------------|------|---------|-------------| | `tools.allow_read_paths` | string[] | `[]` | Additional paths allowed for reading outside workspace | | `tools.allow_write_paths` | string[] | `[]` | Additional paths allowed for writing outside workspace | +| `tools.message.media_enabled` | bool | `false` | Allows the `message` tool to attach local media files by path. This is separate from `tools.send_file.enabled`; enable it only when unified text/media/caption delivery is intended. | ### Read File Mode diff --git a/pkg/agent/agent_init.go b/pkg/agent/agent_init.go index 14b3f8bfe..17629892d 100644 --- a/pkg/agent/agent_init.go +++ b/pkg/agent/agent_init.go @@ -161,19 +161,19 @@ func registerSharedTools( // Message tool if cfg.Tools.IsToolEnabled("message") { messageTool := tools.NewMessageTool() - messageTool.ConfigureLocalMedia( - agent.Workspace, - cfg.Agents.Defaults.RestrictToWorkspace, - cfg.Agents.Defaults.GetMaxMediaSize(), - allowReadPaths, - ) + if cfg.Tools.Message.MediaEnabled { + messageTool.ConfigureLocalMedia( + agent.Workspace, + cfg.Agents.Defaults.RestrictToWorkspace, + cfg.Agents.Defaults.GetMaxMediaSize(), + allowReadPaths, + ) + } messageTool.SetSendCallback(func( ctx context.Context, channel, chatID, content, replyToMessageID string, mediaParts []bus.MediaPart, ) error { - pubCtx, pubCancel := context.WithTimeout(context.Background(), 5*time.Second) - defer pubCancel() outboundCtx := bus.NewOutboundContext(channel, chatID, replyToMessageID) outboundAgentID, outboundSessionKey, outboundScope := outboundTurnMetadata( tools.ToolAgentID(ctx), @@ -181,22 +181,38 @@ func registerSharedTools( tools.ToolSessionScope(ctx), ) if len(mediaParts) > 0 { - return msgBus.PublishOutboundMedia(pubCtx, bus.OutboundMediaMessage{ + outboundMedia := bus.OutboundMediaMessage{ + Channel: channel, + ChatID: chatID, Context: outboundCtx, AgentID: outboundAgentID, SessionKey: outboundSessionKey, Scope: outboundScope, Parts: mediaParts, - }) + } + if al.channelManager != nil && channel != "" { + return al.channelManager.SendMedia(ctx, outboundMedia) + } + pubCtx, pubCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer pubCancel() + return msgBus.PublishOutboundMedia(pubCtx, outboundMedia) } - return msgBus.PublishOutbound(pubCtx, bus.OutboundMessage{ + outboundMessage := bus.OutboundMessage{ + Channel: channel, + ChatID: chatID, Context: outboundCtx, AgentID: outboundAgentID, SessionKey: outboundSessionKey, Scope: outboundScope, Content: content, ReplyToMessageID: replyToMessageID, - }) + } + if al.channelManager != nil && channel != "" { + return al.channelManager.SendMessage(ctx, outboundMessage) + } + pubCtx, pubCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer pubCancel() + return msgBus.PublishOutbound(pubCtx, outboundMessage) }) agent.Tools.Register(messageTool) } diff --git a/pkg/config/config.go b/pkg/config/config.go index d9608d11e..b36014b9f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -814,6 +814,12 @@ type ToolConfig struct { Enabled bool `json:"enabled" yaml:"-" env:"ENABLED"` } +type MessageToolsConfig struct { + ToolConfig `yaml:"-" envPrefix:"PICOCLAW_TOOLS_MESSAGE_"` + + MediaEnabled bool `json:"media_enabled" yaml:"-" env:"PICOCLAW_TOOLS_MESSAGE_MEDIA_ENABLED"` +} + type BraveConfig struct { Enabled bool `json:"enabled" yaml:"-" env:"PICOCLAW_TOOLS_WEB_BRAVE_ENABLED"` APIKeys SecureStrings `json:"api_keys,omitzero" yaml:"api_keys,omitempty" env:"PICOCLAW_TOOLS_WEB_BRAVE_API_KEYS"` @@ -1026,7 +1032,7 @@ type ToolsConfig struct { InstallSkill ToolConfig `json:"install_skill" yaml:"-" envPrefix:"PICOCLAW_TOOLS_INSTALL_SKILL_"` ListDir ToolConfig `json:"list_dir" yaml:"-" envPrefix:"PICOCLAW_TOOLS_LIST_DIR_"` LoadImage ToolConfig `json:"load_image" yaml:"-" envPrefix:"PICOCLAW_TOOLS_LOAD_IMAGE_"` - Message ToolConfig `json:"message" yaml:"-" envPrefix:"PICOCLAW_TOOLS_MESSAGE_"` + Message MessageToolsConfig `json:"message" yaml:"-"` ReadFile ReadFileToolConfig `json:"read_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_READ_FILE_"` Serial ToolConfig `json:"serial" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SERIAL_"` SendFile ToolConfig `json:"send_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_FILE_"` diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index e34f23895..213090a15 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1480,6 +1480,16 @@ func TestLoadConfig_LoadImageCanBeDisabled(t *testing.T) { } } +func TestDefaultConfig_MessageMediaDisabled(t *testing.T) { + cfg := DefaultConfig() + if !cfg.Tools.Message.Enabled { + t.Fatal("DefaultConfig().Tools.Message.Enabled should be true") + } + if cfg.Tools.Message.MediaEnabled { + t.Fatal("DefaultConfig().Tools.Message.MediaEnabled should be false") + } +} + func TestToolsConfig_GetFilterMinLength(t *testing.T) { tests := []struct { name string diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index c411aadf3..d7bd16875 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -447,8 +447,11 @@ func DefaultConfig() *Config { LoadImage: ToolConfig{ Enabled: true, }, - Message: ToolConfig{ - Enabled: true, + Message: MessageToolsConfig{ + ToolConfig: ToolConfig{ + Enabled: true, + }, + MediaEnabled: false, }, ReadFile: ReadFileToolConfig{ Enabled: true, diff --git a/pkg/tools/integration/message.go b/pkg/tools/integration/message.go index f7b7b7fdc..fbd8305c6 100644 --- a/pkg/tools/integration/message.go +++ b/pkg/tools/integration/message.go @@ -37,14 +37,15 @@ type sentTarget struct { } type MessageTool struct { - sendCallback SendCallbackWithContext - workspace string - restrict bool - maxFileSize int - mediaStore media.MediaStore - allowPaths []*regexp.Regexp - mu sync.Mutex - sentTargets map[string][]sentTarget + sendCallback SendCallbackWithContext + workspace string + restrict bool + maxFileSize int + mediaStore media.MediaStore + allowPaths []*regexp.Regexp + localMediaEnabled bool + mu sync.Mutex + sentTargets map[string][]sentTarget } func NewMessageTool() *MessageTool { @@ -58,57 +59,66 @@ func (t *MessageTool) Name() string { } func (t *MessageTool) Description() string { + if !t.localMediaEnabled { + return "Send a text message to the user on a chat channel." + } return "Send a message to the user on a chat channel. Supports text-only, media-only, or text with media attachments." } func (t *MessageTool) Parameters() map[string]any { - return map[string]any{ - "type": "object", - "properties": map[string]any{ - "content": map[string]any{ - "type": "string", - "description": "Optional message text. When media is present, this text is used as the caption/body for the media message.", - }, - "media": map[string]any{ - "type": "array", - "description": "Optional local media attachments to send with the message.", - "items": map[string]any{ - "type": "object", - "properties": map[string]any{ - "path": map[string]any{ - "type": "string", - "description": "Path to the local file. Relative paths are resolved from workspace.", - }, - "type": map[string]any{ - "type": "string", - "description": "Optional media type hint: image, audio, video, or file.", - }, - "filename": map[string]any{ - "type": "string", - "description": "Optional display filename. Defaults to the basename of path.", - }, - }, - "required": []string{"path"}, - }, - }, - "channel": map[string]any{ - "type": "string", - "description": "Optional: target channel (telegram, whatsapp, etc.)", - }, - "chat_id": map[string]any{ - "type": "string", - "description": "Optional: target chat/user ID", - }, - "reply_to_message_id": map[string]any{ - "type": "string", - "description": "Optional: reply target message ID for channels that support threaded replies", - }, + properties := map[string]any{ + "content": map[string]any{ + "type": "string", + "description": "Optional message text. When media is present, this text is used as the caption/body for the media message.", }, - "anyOf": []map[string]any{ - {"required": []string{"content"}}, - {"required": []string{"media"}}, + "channel": map[string]any{ + "type": "string", + "description": "Optional: target channel (telegram, whatsapp, etc.)", + }, + "chat_id": map[string]any{ + "type": "string", + "description": "Optional: target chat/user ID", + }, + "reply_to_message_id": map[string]any{ + "type": "string", + "description": "Optional: reply target message ID for channels that support threaded replies", }, } + params := map[string]any{ + "type": "object", + "properties": properties, + "required": []string{"content"}, + } + if t.localMediaEnabled { + properties["media"] = map[string]any{ + "type": "array", + "description": "Optional local media attachments to send with the message. Requires tools.message.media_enabled.", + "items": map[string]any{ + "type": "object", + "properties": map[string]any{ + "path": map[string]any{ + "type": "string", + "description": "Path to the local file. Relative paths are resolved from workspace.", + }, + "type": map[string]any{ + "type": "string", + "description": "Optional media type hint: image, audio, video, or file.", + }, + "filename": map[string]any{ + "type": "string", + "description": "Optional display filename. Defaults to the basename of path.", + }, + }, + "required": []string{"path"}, + }, + } + delete(params, "required") + params["anyOf"] = []map[string]any{ + {"required": []string{"content"}}, + {"required": []string{"media"}}, + } + } + return params } func (t *MessageTool) ConfigureLocalMedia( @@ -124,6 +134,7 @@ func (t *MessageTool) ConfigureLocalMedia( } t.maxFileSize = maxFileSize t.allowPaths = allowPaths + t.localMediaEnabled = true } func (t *MessageTool) SetMediaStore(store media.MediaStore) { @@ -173,6 +184,12 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *ToolRes if err != nil { return &ToolResult{ForLLM: err.Error(), IsError: true} } + if len(mediaArgs) > 0 && !t.localMediaEnabled { + return &ToolResult{ + ForLLM: "message media attachments are disabled; enable tools.message.media_enabled to send local media through message", + IsError: true, + } + } if content == "" && len(mediaArgs) == 0 { return &ToolResult{ForLLM: "content or media is required", IsError: true} } @@ -262,6 +279,9 @@ func (t *MessageTool) buildMediaParts( if len(mediaArgs) == 0 { return nil, nil } + if !t.localMediaEnabled { + return nil, fmt.Errorf("message media attachments are disabled") + } if t.mediaStore == nil { return nil, fmt.Errorf("media store not configured") } diff --git a/pkg/tools/integration/message_test.go b/pkg/tools/integration/message_test.go index 2d3329d3d..eea345c1c 100644 --- a/pkg/tools/integration/message_test.go +++ b/pkg/tools/integration/message_test.go @@ -250,9 +250,9 @@ func TestMessageTool_Parameters(t *testing.T) { } // Check required properties - anyOf, ok := params["anyOf"].([]map[string]any) - if !ok || len(anyOf) != 2 { - t.Fatal("Expected anyOf content/media requirement") + required, ok := params["required"].([]string) + if !ok || len(required) != 1 || required[0] != "content" { + t.Fatal("Expected content-only required schema when local media is disabled") } // Check content property @@ -264,12 +264,8 @@ func TestMessageTool_Parameters(t *testing.T) { t.Error("Expected content type to be 'string'") } - mediaProp, ok := props["media"].(map[string]any) - if !ok { - t.Fatal("Expected 'media' property") - } - if mediaProp["type"] != "array" { - t.Error("Expected media type to be 'array'") + if _, hasMedia := props["media"]; hasMedia { + t.Fatal("did not expect 'media' property when local media is disabled") } // Check channel property (optional) @@ -300,6 +296,56 @@ func TestMessageTool_Parameters(t *testing.T) { } } +func TestMessageTool_Parameters_WithLocalMediaEnabled(t *testing.T) { + tool := NewMessageTool() + tool.ConfigureLocalMedia(t.TempDir(), true, 1024*1024, nil) + params := tool.Parameters() + + props, ok := params["properties"].(map[string]any) + if !ok { + t.Fatal("Expected properties to be a map") + } + mediaProp, ok := props["media"].(map[string]any) + if !ok { + t.Fatal("Expected 'media' property") + } + if mediaProp["type"] != "array" { + t.Error("Expected media type to be 'array'") + } + anyOf, ok := params["anyOf"].([]map[string]any) + if !ok || len(anyOf) != 2 { + t.Fatal("Expected anyOf content/media requirement") + } + if _, ok := params["required"]; ok { + t.Fatal("did not expect top-level required content when media is enabled") + } +} + +func TestMessageTool_Execute_WithMediaDisabled(t *testing.T) { + tool := NewMessageTool() + tool.SetSendCallback(func( + ctx context.Context, + channel, chatID, content, replyToMessageID string, + mediaParts []bus.MediaPart, + ) error { + t.Fatal("send callback should not run when message media is disabled") + return nil + }) + + ctx := WithToolContext(context.Background(), "telegram", "-1001") + result := tool.Execute(ctx, map[string]any{ + "media": []any{ + map[string]any{"path": "photo.jpg"}, + }, + }) + if !result.IsError { + t.Fatal("expected error when message media is disabled") + } + if result.ForLLM != "message media attachments are disabled; enable tools.message.media_enabled to send local media through message" { + t.Fatalf("unexpected error: %q", result.ForLLM) + } +} + func TestMessageTool_Execute_WithReplyToMessageID(t *testing.T) { tool := NewMessageTool()