fix: security + quality audit — 9 fixes for open-source readiness

Security:
- Add chat ID validation to callback handler (H3) — prevents
  unauthorized users from approving permissions
- Replace time-based message IDs with atomic counter (L8) —
  prevents millisecond collisions under concurrent access

Bug fix:
- Fix FormatToolName MCP parsing (H1) — extract service name
  from namespace segment (e.g., "claude_ai_Slack" → "Slack").
  Slack/Notion icons now match correctly

Code quality:
- Replace interface{} with any (Go 1.18+ canonical alias)
- Remove redundant nil checks in cmdPlan
- Replace custom contains() test helper with strings.Contains
- Update .gitignore: add .claude/, .DS_Store, organize sections
- Fix CLAUDE.md/README MAX_RETRY_COUNT default (2 → 3, matches code)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: GrapeInTheTree

.gitignore

@@ -1,11 +1,22 @@
+# Secrets
 .env
+
+# Runtime data
 bot.log
 bot.pid
 *.lock
 inbox.json
 outbox.json
 projects.json
+
+# Build output
 /pocket-claude
 
+# Claude Code session data
+.claude/
+
+# macOS
+.DS_Store
+
 # Skill eval test (isolated experiment - no impact on project)
 skill-eval-test/

CLAUDE.md

@@ -116,7 +116,7 @@ Each project gets its own `Executor` with independent session, workDir, and addD
 | `INBOX_PATH` | `./inbox.json` | Incoming messages |
 | `OUTBOX_PATH` | `./outbox.json` | Outgoing results |
 | `LOCK_TIMEOUT_MINUTES` | `5` | Stale lock threshold |
-| `MAX_RETRY_COUNT` | `2` | Error retry limit |
+| `MAX_RETRY_COUNT` | `3` | Error retry limit |
 | `OUTBOX_POLL_INTERVAL_SECONDS` | `10` | Outbox poll interval |
 | `LOG_FILE` | `./bot.log` | Log file path |
 | `MESSAGE_TTL_MINUTES` | `10` | Message expiry time |

README.md

@@ -100,7 +100,7 @@ cp .env.example .env
 | `INBOX_PATH` | `./inbox.json` | Incoming message store |
 | `OUTBOX_PATH` | `./outbox.json` | Outgoing result store |
 | `LOCK_TIMEOUT_MINUTES` | `5` | Stale lock detection threshold |
-| `MAX_RETRY_COUNT` | `2` | Max retries for failed messages |
+| `MAX_RETRY_COUNT` | `3` | Max retries for failed messages |
 | `OUTBOX_POLL_INTERVAL_SECONDS` | `10` | Outbox polling interval |
 | `LOG_FILE` | `./bot.log` | Log file path |
 | `MESSAGE_TTL_MINUTES` | `10` | Auto-expire messages older than this |

internal/bot/bot.go

@@ -6,6 +6,7 @@ import (
 	"log/slog"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/GrapeInTheTree/pocket-claude/internal/config"
@@ -17,6 +18,13 @@ import (
 // maxConcurrentCallbacks limits goroutine spawning for callback handlers.
 const maxConcurrentCallbacks = 10
 
+// msgCounter provides unique message IDs without millisecond collisions.
+var msgCounter atomic.Int64
+
+func init() {
+	msgCounter.Store(time.Now().UnixMilli())
+}
+
 type Bot struct {
 	api    *tgbotapi.BotAPI
 	cfg    config.Config
@@ -263,7 +271,7 @@ func (b *Bot) handleMessage(msg *tgbotapi.Message) {
 	}
 
 	inboxMsg := store.InboxMessage{
-		ID:                fmt.Sprintf("msg_%d", time.Now().UnixMilli()),
+		ID:                fmt.Sprintf("msg_%d", msgCounter.Add(1)),
 		Text:              text,
 		Status:            store.StatusPending,
 		Timestamp:         time.Now().UTC().Format(time.RFC3339),
@@ -283,6 +291,11 @@ func (b *Bot) handleMessage(msg *tgbotapi.Message) {
 }
 
 func (b *Bot) handleCallback(cq *tgbotapi.CallbackQuery) {
+	// Security: verify callback is from authorized chat
+	if cq.Message != nil && cq.Message.Chat.ID != b.cfg.TelegramChatID {
+		return
+	}
+
 	data := cq.Data
 	parts := strings.SplitN(data, ":", 2)
 	if len(parts) != 2 {

internal/bot/commands.go

@@ -111,7 +111,7 @@ func (b *Bot) cmdBtw(msg *tgbotapi.Message) {
 	}
 
 	inboxMsg := store.InboxMessage{
-		ID:                fmt.Sprintf("msg_%d", time.Now().UnixMilli()),
+		ID:                fmt.Sprintf("msg_%d", msgCounter.Add(1)),
 		Text:              "[BTW context note, just acknowledge briefly] " + text,
 		Status:            store.StatusPending,
 		Timestamp:         time.Now().UTC().Format(time.RFC3339),
@@ -551,7 +551,7 @@ func (b *Bot) cmdResearch(msg *tgbotapi.Message) {
 		btwText := fmt.Sprintf("[Background task %s result from project %q] %s", taskID, projectName, injected)
 
 		inboxMsg := store.InboxMessage{
-			ID:        fmt.Sprintf("msg_%d", time.Now().UnixMilli()),
+			ID:        fmt.Sprintf("msg_%d", msgCounter.Add(1)),
 			Text:      "[Injected context — do NOT respond to this, just silently remember it for future reference] " + btwText,
 			Status:    store.StatusPending,
 			Timestamp: time.Now().UTC().Format(time.RFC3339),
@@ -718,18 +718,15 @@ func (b *Bot) cmdPlan(msg *tgbotapi.Message) {
 		return
 	}
 
-	var projectName string
-	if b.worker != nil {
-		projectName = b.worker.ActiveProject()
-	}
+	projectName := b.worker.ActiveProject()
 
 	planPrompt := "[Plan mode: Create a detailed implementation plan for the following task. " +
 		"Analyze the codebase and outline specific steps with file paths. " +
 		"Do NOT execute anything yet — only plan. " +
 		"Wait for my approval before making any changes.] " + text
 
 	inboxMsg := store.InboxMessage{
-		ID:                fmt.Sprintf("msg_%d", time.Now().UnixMilli()),
+		ID:                fmt.Sprintf("msg_%d", msgCounter.Add(1)),
 		Text:              planPrompt,
 		Status:            store.StatusPending,
 		Timestamp:         time.Now().UTC().Format(time.RFC3339),
@@ -742,7 +739,5 @@ func (b *Bot) cmdPlan(msg *tgbotapi.Message) {
 		return
 	}
 
-	if b.worker != nil {
-		b.worker.Enqueue(inboxMsg)
-	}
+	b.worker.Enqueue(inboxMsg)
 }

internal/store/models.go

@@ -74,6 +74,6 @@ type CLIResult struct {
 }
 
 type PermissionDenial struct {
-	ToolName  string                 `json:"tool_name"`
-	ToolInput map[string]interface{} `json:"tool_input,omitempty"`
+	ToolName  string         `json:"tool_name"`
+	ToolInput map[string]any `json:"tool_input,omitempty"`
 }

internal/worker/approval.go

@@ -135,12 +135,26 @@ func extractToolDetail(d store.PermissionDenial) string {
 }
 
 // FormatToolName converts internal tool names to readable labels.
+// MCP tool names follow the pattern: mcp__<namespace>__<action>
+// e.g., mcp__claude_ai_Slack__slack_send_message → 💬 Slack → Send Message
 func FormatToolName(raw string) string {
 	if strings.HasPrefix(raw, "mcp__") {
 		parts := strings.Split(raw, "__")
 		if len(parts) >= 3 {
-			service := parts[2]
+			// Extract service name from namespace (e.g., "claude_ai_Slack" → "Slack")
+			namespace := parts[1]
+			service := namespace
+			if idx := strings.LastIndex(namespace, "_"); idx >= 0 {
+				service = namespace[idx+1:]
+			}
+
+			// Extract and format action (e.g., "slack_send_message" → "Send Message")
 			action := parts[len(parts)-1]
+			// Remove service prefix from action if present (e.g., "slack_send_message" → "send_message")
+			serviceLower := strings.ToLower(service)
+			if strings.HasPrefix(action, serviceLower+"_") {
+				action = action[len(serviceLower)+1:]
+			}
 			action = strings.ReplaceAll(action, "_", " ")
 			words := strings.Fields(action)
 			for i, w := range words {

internal/worker/approval_test.go

@@ -1,6 +1,7 @@
 package worker
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/GrapeInTheTree/pocket-claude/internal/store"
@@ -84,14 +85,11 @@ func TestFormatToolName(t *testing.T) {
 		{"Edit", "Edit", "✏️ File Edit"},
 		{"Read", "Read", "📖 File Read"},
 		{"unknown tool", "CustomTool", "🔧 CustomTool"},
-		// mcp__claude_ai_Slack__slack_send_message splits on "__" to:
-		// ["mcp", "claude_ai_Slack", "slack_send_message"]
-		// service=parts[2]="slack_send_message", action=last="slack_send_message"
-		{"MCP Slack", "mcp__claude_ai_Slack__slack_send_message", "🔌 slack_send_message → Slack Send Message"},
-		{"MCP Notion", "mcp__claude_ai_Notion__notion_search", "🔌 notion_search → Notion Search"},
-		// mcp__foo__bar__do_thing splits to ["mcp", "foo", "bar", "do_thing"]
-		// service=parts[2]="bar", action=last="do_thing"
-		{"MCP unknown service", "mcp__foo__bar__do_thing", "🔌 bar → Do Thing"},
+		// mcp__claude_ai_Slack__slack_send_message → service="Slack", action="Send Message"
+		{"MCP Slack", "mcp__claude_ai_Slack__slack_send_message", "💬 Slack → Send Message"},
+		{"MCP Notion", "mcp__claude_ai_Notion__notion_search", "📝 Notion → Search"},
+		// mcp__foo__bar__do_thing → namespace="foo", service="foo", action="do_thing"
+		{"MCP unknown service", "mcp__foo__bar__do_thing", "🔌 foo → Do Thing"},
 	}
 
 	for _, tt := range tests {
@@ -109,11 +107,11 @@ func TestBuildPermissionMessage(t *testing.T) {
 		PermissionDenials: []store.PermissionDenial{
 			{
 				ToolName:  "Bash",
-				ToolInput: map[string]interface{}{"command": "rm -rf /tmp/test"},
+				ToolInput: map[string]any{"command": "rm -rf /tmp/test"},
 			},
 			{
 				ToolName:  "Write",
-				ToolInput: map[string]interface{}{"file_path": "/home/user/file.txt"},
+				ToolInput: map[string]any{"file_path": "/home/user/file.txt"},
 			},
 		},
 		Result: "I need to run a command",
@@ -142,10 +140,10 @@ func TestBuildPermissionMessageDedup(t *testing.T) {
 	// Multiple denials for the same tool should be grouped
 	result := &store.CLIResult{
 		PermissionDenials: []store.PermissionDenial{
-			{ToolName: "Bash", ToolInput: map[string]interface{}{"command": "ls"}},
-			{ToolName: "Bash", ToolInput: map[string]interface{}{"command": "pwd"}},
-			{ToolName: "Bash", ToolInput: map[string]interface{}{"command": "cat file"}},
-			{ToolName: "Bash", ToolInput: map[string]interface{}{"command": "extra"}}, // 4th, should be capped at 3
+			{ToolName: "Bash", ToolInput: map[string]any{"command": "ls"}},
+			{ToolName: "Bash", ToolInput: map[string]any{"command": "pwd"}},
+			{ToolName: "Bash", ToolInput: map[string]any{"command": "cat file"}},
+			{ToolName: "Bash", ToolInput: map[string]any{"command": "extra"}}, // 4th, should be capped at 3
 		},
 	}
 
@@ -172,14 +170,5 @@ func TestSanitizeUTF8(t *testing.T) {
 }
 
 func contains(s, substr string) bool {
-	return len(s) >= len(substr) && containsSubstr(s, substr)
-}
-
-func containsSubstr(s, substr string) bool {
-	for i := 0; i <= len(s)-len(substr); i++ {
-		if s[i:i+len(substr)] == substr {
-			return true
-		}
-	}
-	return false
+	return strings.Contains(s, substr)
 }