Fix Telegram send failures for oversized messages (#244)

README.md

@@ -528,6 +528,21 @@ When `restrict_to_workspace: true`, the following tools are sandboxed:
 | `append_file` | Append to files | Only files within workspace |
 | `exec` | Execute commands | Command paths must be within workspace |
 
+#### Web Fetch Network Boundary
+
+`web_fetch` enforces an outbound network boundary independent of `restrict_to_workspace`.
+
+Blocked destination classes include:
+
+* loopback
+* private RFC1918 / unique-local ranges
+* link-local
+* multicast
+* unspecified / non-routable internal targets
+* redirect hops that resolve to blocked targets
+
+This policy is applied both before connect and during redirect handling, so a public URL cannot bounce into private infrastructure through redirects.
+
 #### Additional Exec Protection
 
 Even with `restrict_to_workspace: false`, the `exec` tool blocks these dangerous commands:
@@ -551,6 +566,11 @@ Even with `restrict_to_workspace: false`, the `exec` tool blocks these dangerous
 {tool=exec, error=Command blocked by safety guard (dangerous pattern detected)}
 ```
 
+```
+[ERROR] tool: Tool execution failed
+{tool=web_fetch, error=blocked destination: host "127.0.0.1" resolves to non-public IP 127.0.0.1}
+```
+
 #### Disabling Restrictions (Security Risk)
 
 If you need the agent to access paths outside the workspace:
@@ -577,7 +597,12 @@ export PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_WORKSPACE=false
 
 #### Security Boundary Consistency
 
-The `restrict_to_workspace` setting applies consistently across all execution paths:
+PicoClaw enforces complementary boundaries:
+
+* Filesystem + shell boundary via `restrict_to_workspace`
+* Network egress boundary via `web_fetch` public-target validation
+
+The workspace boundary (`restrict_to_workspace`) applies consistently across all execution paths:
 
 | Execution Path | Security Boundary |
 |----------------|-------------------|

pkg/channels/telegram.go

@@ -39,6 +39,10 @@ type thinkingCancel struct {
 	fn context.CancelFunc
 }
 
+const (
+	telegramMaxMessageLength = 4096
+)
+
 func (c *thinkingCancel) Cancel() {
 	if c != nil && c.fn != nil {
 		c.fn()
@@ -157,35 +161,66 @@ func (c *TelegramChannel) Send(ctx context.Context, msg bus.OutboundMessage) err
 		c.stopThinking.Delete(msg.ChatID)
 	}
 
-	htmlContent := markdownToTelegramHTML(msg.Content)
+	chunks := splitTelegramMessageContent(msg.Content, telegramMaxMessageLength)
+	if len(chunks) == 0 {
+		return nil
+	}
 
 	// Try to edit placeholder
 	if pID, ok := c.placeholders.Load(msg.ChatID); ok {
 		c.placeholders.Delete(msg.ChatID)
-		editMsg := tu.EditMessageText(tu.ID(chatID), pID.(int), htmlContent)
-		editMsg.ParseMode = telego.ModeHTML
-
-		if _, err = c.bot.EditMessageText(ctx, editMsg); err == nil {
+		if err := c.editMessageChunk(ctx, chatID, pID.(int), chunks[0]); err == nil {
+			for i := 1; i < len(chunks); i++ {
+				if sendErr := c.sendMessageChunk(ctx, chatID, chunks[i]); sendErr != nil {
+					return sendErr
+				}
+			}
 			return nil
 		}
 		// Fallback to new message if edit fails
 	}
 
+	for _, chunk := range chunks {
+		if err := c.sendMessageChunk(ctx, chatID, chunk); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (c *TelegramChannel) sendMessageChunk(ctx context.Context, chatID int64, content string) error {
+	htmlContent := markdownToTelegramHTML(content)
 	tgMsg := tu.Message(tu.ID(chatID), htmlContent)
 	tgMsg.ParseMode = telego.ModeHTML
 
-	if _, err = c.bot.SendMessage(ctx, tgMsg); err != nil {
+	if _, err := c.bot.SendMessage(ctx, tgMsg); err != nil {
 		logger.ErrorCF("telegram", "HTML parse failed, falling back to plain text", map[string]interface{}{
 			"error": err.Error(),
 		})
-		tgMsg.ParseMode = ""
-		_, err = c.bot.SendMessage(ctx, tgMsg)
-		return err
+		plainMsg := tu.Message(tu.ID(chatID), content)
+		_, fallbackErr := c.bot.SendMessage(ctx, plainMsg)
+		return fallbackErr
 	}
 
 	return nil
 }
 
+func (c *TelegramChannel) editMessageChunk(ctx context.Context, chatID int64, messageID int, content string) error {
+	htmlContent := markdownToTelegramHTML(content)
+	editMsg := tu.EditMessageText(tu.ID(chatID), messageID, htmlContent)
+	editMsg.ParseMode = telego.ModeHTML
+	if _, err := c.bot.EditMessageText(ctx, editMsg); err != nil {
+		logger.ErrorCF("telegram", "HTML edit parse failed, falling back to plain text", map[string]interface{}{
+			"error": err.Error(),
+		})
+		plainEdit := tu.EditMessageText(tu.ID(chatID), messageID, content)
+		_, fallbackErr := c.bot.EditMessageText(ctx, plainEdit)
+		return fallbackErr
+	}
+	return nil
+}
+
 func (c *TelegramChannel) handleMessage(ctx context.Context, message *telego.Message) error {
 	if message == nil {
 		return fmt.Errorf("message is nil")
@@ -453,6 +488,67 @@ func markdownToTelegramHTML(text string) string {
 	return text
 }
 
+func splitTelegramMessageContent(text string, maxLen int) []string {
+	text = strings.TrimSpace(text)
+	if text == "" {
+		return nil
+	}
+
+	if maxLen <= 0 {
+		return []string{text}
+	}
+
+	chunks := utils.SplitMessage(text, maxLen)
+	return enforceTelegramMessageHTMLLimit(chunks, maxLen)
+}
+
+func enforceTelegramMessageHTMLLimit(chunks []string, maxLen int) []string {
+	out := make([]string, 0, len(chunks))
+	for _, chunk := range chunks {
+		chunk = strings.TrimSpace(chunk)
+		if chunk == "" {
+			continue
+		}
+
+		if runeLen(markdownToTelegramHTML(chunk)) <= maxLen {
+			out = append(out, chunk)
+			continue
+		}
+
+		runes := []rune(chunk)
+		if len(runes) <= 1 {
+			out = append(out, chunk)
+			continue
+		}
+
+		splitLimit := len(runes) / 2
+		if splitLimit > maxLen {
+			splitLimit = maxLen
+		}
+		if splitLimit < 1 {
+			splitLimit = 1
+		}
+
+		subChunks := utils.SplitMessage(chunk, splitLimit)
+		if len(subChunks) <= 1 {
+			mid := len(runes) / 2
+			if mid < 1 {
+				mid = 1
+			}
+			subChunks = []string{
+				string(runes[:mid]),
+				string(runes[mid:]),
+			}
+		}
+		out = append(out, enforceTelegramMessageHTMLLimit(subChunks, maxLen)...)
+	}
+	return out
+}
+
+func runeLen(text string) int {
+	return len([]rune(text))
+}
+
 type codeBlockMatch struct {
 	text  string
 	codes []string

pkg/channels/telegram_test.go

@@ -0,0 +1,54 @@
+package channels
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestSplitTelegramMessageContentShortMessage(t *testing.T) {
+	input := "hello world"
+	chunks := splitTelegramMessageContent(input, telegramMaxMessageLength)
+
+	if len(chunks) != 1 {
+		t.Fatalf("len(chunks) = %d, want 1", len(chunks))
+	}
+	if chunks[0] != input {
+		t.Fatalf("chunk[0] = %q, want %q", chunks[0], input)
+	}
+}
+
+func TestSplitTelegramMessageContentLongMessage(t *testing.T) {
+	input := strings.Repeat("This is a long telegram message chunk. ", 300)
+	chunks := splitTelegramMessageContent(input, telegramMaxMessageLength)
+
+	if len(chunks) < 2 {
+		t.Fatalf("expected multiple chunks, got %d", len(chunks))
+	}
+
+	for i, chunk := range chunks {
+		if strings.TrimSpace(chunk) == "" {
+			t.Fatalf("chunk %d is empty", i)
+		}
+		html := markdownToTelegramHTML(chunk)
+		if runeLen(html) > telegramMaxMessageLength {
+			t.Fatalf("chunk %d HTML length = %d, want <= %d", i, runeLen(html), telegramMaxMessageLength)
+		}
+	}
+}
+
+func TestSplitTelegramMessageContentEscapingExpansion(t *testing.T) {
+	// '&' expands to '&amp;' in HTML, so this validates recursive splitting safety.
+	input := strings.Repeat("&", 5000)
+	chunks := splitTelegramMessageContent(input, telegramMaxMessageLength)
+
+	if len(chunks) < 2 {
+		t.Fatalf("expected multiple chunks, got %d", len(chunks))
+	}
+
+	for i, chunk := range chunks {
+		html := markdownToTelegramHTML(chunk)
+		if runeLen(html) > telegramMaxMessageLength {
+			t.Fatalf("chunk %d escaped HTML length = %d, want <= %d", i, runeLen(html), telegramMaxMessageLength)
+		}
+	}
+}

pkg/tools/README.md

@@ -0,0 +1,9 @@
+# Tool Security Checklist
+
+When adding a new built-in tool, include these minimum safety checks:
+
+1. Path boundary: if the tool reads/writes files or executes commands with paths, enforce canonical workspace membership when `restrict_to_workspace=true`.
+2. Network boundary: if the tool performs outbound network calls, reject loopback/private/link-local/multicast/unspecified/internal targets and validate redirect hops.
+3. Timeout behavior: long-running operations must use deterministic timeout/cancel handling and terminate child processes where process trees are possible.
+4. Regression tests: add explicit tests for blocked behavior (not just happy-path errors), including redirect/path traversal/process-leak scenarios where relevant.
+5. Error clarity: return explicit denial reasons (`blocked destination`, `outside workspace`, `timed out`) so behavior is auditable in logs.