L11: bind Telegram clarify callbacks to request and originating user

AGENTS.md

@@ -181,6 +181,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
 - **Telegram plan file size cap** (`internal/telegram/plan.go`) — plan files larger than 1 MiB are rejected by `ReadPlan` and `MostRecentPlan`, and `ListPlans` only reads the first 8 KiB for preview. This prevents a prompt-injected agent from causing an OOM via a multi-hundred-megabyte plan file.
 - **Telegram log file permissions** (`internal/telegram/log.go`) — Telegram log files are created with `0600`, and `os.Chmod` hardens existing files. Chat IDs and task snippets are no longer world-readable by default.
 - **Telegram chat-scoped sessions and plans** (`internal/telegram/session.go`, `internal/telegram/plan.go`, `cmd/odek/telegram.go`) — `/sessions`, `/resume`, `/prune`, `/plans`, `/plan_view`, `/plan_delete`, and `/plan_resume` are scoped to the requesting chat. Sessions are filtered by the `tg-<chatID>` prefix, `ResumeSession` rejects cross-chat IDs, and plans live under `~/.odek/plans/chat<chatID>/` so one chat cannot list, read, delete, or resume another chat's sessions/plans.
+- **Telegram clarify callback binding** (`cmd/odek/telegram.go`, `internal/telegram/handler.go`) — each `clarify` prompt now uses a random request ID in its inline-keyboard callback data (`clarify:<reqID>:yes/no`). The handler validates the request ID and binds the answer to the originating user, so a group member or stale keyboard cannot answer someone else's clarify prompt. `Handler.OnCallbackQuery` now receives the `userID` for consistent callback binding.
 - **odek self-invocation gate** (`internal/danger/classifier.go`) — any shell stage whose program basename is `odek` is classified as `system_write`. This prevents a prompt-injected agent from reaching the human-gated trust mutations (`odek memory promote`, `odek memory extended confirm`, `odek skill promote --force`) through the shell tool and flipping its own taint gates.
 - **MCP inputSchema hardening** (`cmd/odek/mcp_approval.go`) — every string in an MCP tool's `inputSchema` is recursively guard-scanned for injection patterns; schemas larger than 256 KiB are rejected; the interactive approval prompt shows a SHA-256 hash and byte size of the schema so operators can detect changes.
 - **MCP tool batch classification** (`internal/loop/loop.go`) — MCP tools (`<server>__<tool>`) are classified as `unknown` by `classifyToolCall`, so the batch approval gate shows them and untrusted sub-agents force them to `deny`.

cmd/odek/telegram.go

@@ -2,6 +2,8 @@ package main
 
 import (
 	"context"
+	"crypto/rand"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -47,10 +49,89 @@ var chatCancels sync.Map // map[int64]context.CancelFunc
 // interrupted task.
 var chatRunInfos sync.Map // map[int64]loop.IterationInfo
 
-// clarifyMsgIDs stores the message ID of the active clarify prompt
-// per chat, so the callback handler can edit/remove it after the user
-// responds. Cleared when the clarify tool completes or times out.
-var clarifyMsgIDs sync.Map // map[int64]int
+// pendingClarifyReqs stores active clarify prompts keyed by a random
+// request ID embedded in the callback data. Each entry records the
+// originating user, the response channel, and the prompt message ID so
+// callback queries can be validated against the user who triggered the
+// clarify and rejected if the prompt has expired or was answered by
+// someone else. Cleared when the clarify tool completes, times out, or
+// the user responds.
+type pendingClarifyReq struct {
+	userID int64
+	ch     chan string
+	msgID  int
+}
+
+var pendingClarifyReqs sync.Map // map[string]*pendingClarifyReq
+
+// generateClarifyReqID returns a random request ID for a clarify prompt.
+// The ID is embedded in the callback data so each prompt is independent
+// and stale keyboards cannot answer later clarifies.
+func generateClarifyReqID() string {
+	buf := make([]byte, 8)
+	if _, err := rand.Read(buf); err != nil {
+		// crypto/rand.Read only fails on catastrophic system failure. Fail
+		// closed rather than minting a predictable ID, which would let an
+		// attacker pre-compute callback data.
+		panic(fmt.Sprintf("telegram: crypto/rand unavailable: %v", err))
+	}
+	return hex.EncodeToString(buf)
+}
+
+// parseClarifyCallback parses callback data of the form
+// "clarify:<reqID>:<yes|no>". It returns the request ID, the answer, and
+// true if the data is a valid clarify callback.
+func parseClarifyCallback(data string) (reqID, answer string, ok bool) {
+	rest, ok := strings.CutPrefix(data, "clarify:")
+	if !ok {
+		return "", "", false
+	}
+	idx := strings.LastIndex(rest, ":")
+	if idx < 0 {
+		return "", "", false
+	}
+	reqID = rest[:idx]
+	answer = rest[idx+1:]
+	if reqID == "" || (answer != "yes" && answer != "no") {
+		return "", "", false
+	}
+	return reqID, answer, true
+}
+
+// handleClarifyCallback routes a Telegram inline-button press for a clarify
+// prompt. It validates the embedded request ID, binds the answer to the
+// originating user, and unblocks the waiting agent goroutine. The returned
+// bool is true when the callback data was a clarify callback (whether valid
+// or expired); callers should not process it further.
+func handleClarifyCallback(chatID, userID int64, data string, bot *telegram.Bot) (string, bool) {
+	reqID, answer, ok := parseClarifyCallback(data)
+	if !ok {
+		return "", false
+	}
+	v, ok := pendingClarifyReqs.Load(reqID)
+	if !ok {
+		return "⚠️ This clarify prompt has expired or already been answered.", true
+	}
+	req := v.(*pendingClarifyReq)
+	// Bind the callback to the user who triggered the clarify.
+	if req.userID != 0 && req.userID != userID {
+		return "⚠️ This button was meant for another user.", true
+	}
+	// Accept the answer and remove the request atomically.
+	pendingClarifyReqs.Delete(reqID)
+	select {
+	case req.ch <- answer:
+	default:
+		// Channel full or closed — clarify already resolved.
+	}
+	// Remove the inline keyboard and update text to show the answer.
+	if req.msgID != 0 && bot != nil {
+		bot.EditMessageText(chatID, req.msgID,
+			"✅ *User answered:* "+answer,
+			&telegram.SendOpts{ParseMode: telegram.ParseModeMarkdownV2, ReplyMarkup: &telegram.InlineKeyboardMarkup{InlineKeyboard: [][]telegram.InlineKeyboardButton{}}})
+	}
+	return "", true
+}
 
 // pendingSuggestions stores SkillSuggestion values keyed by skill name,
 // awaiting user approval via inline keyboard callbacks.
@@ -594,24 +675,10 @@ func telegramCmd(args []string) error {
 		return cmd.Handler(argsStr)
 	}
 
-	handler.OnCallbackQuery = func(chatID int64, data string) (string, error) {
+	handler.OnCallbackQuery = func(chatID int64, data string, userID int64) (string, error) {
 		// Route clarify callbacks — the user clicked Yes/No on a clarify question.
-		if answer, ok := strings.CutPrefix(data, "clarify:"); ok {
-			if ch, ok := sessionManager.GetClarifyChannel(chatID); ok {
-				select {
-				case ch <- answer:
-				default:
-					// Channel full or closed — clarify already resolved.
-				}
-			}
-			// Remove the inline keyboard and update text to show the answer.
-			if msgID, ok := clarifyMsgIDs.Load(chatID); ok {
-				bot.EditMessageText(chatID, msgID.(int),
-					"✅ *User answered:* "+answer,
-					&telegram.SendOpts{ParseMode: telegram.ParseModeMarkdownV2, ReplyMarkup: &telegram.InlineKeyboardMarkup{InlineKeyboard: [][]telegram.InlineKeyboardButton{}}})
-				clarifyMsgIDs.Delete(chatID)
-			}
-			return "", nil
+		if resp, ok := handleClarifyCallback(chatID, userID, data, bot); ok {
+			return resp, nil
 		}
 
 		// Route skill suggestion callbacks — Save or Skip.
@@ -1514,26 +1581,29 @@ func handleChatMessage(
 	// keyboard message and blocks until the user responds.
 	agentTools := append([]odek.Tool{}, tools...)
 	agentTools = append(agentTools, toolpkg.NewClarifyTool(func(question string) (string, error) {
+		reqID := generateClarifyReqID()
 		ch := make(chan string, 1)
-		sessionManager.SetClarifyChannel(chatID, ch)
-		defer sessionManager.DeleteClarifyChannel(chatID)
+		req := &pendingClarifyReq{userID: userID, ch: ch}
+		pendingClarifyReqs.Store(reqID, req)
+		defer pendingClarifyReqs.Delete(reqID)
 
-		// Send the question with Yes/No buttons.
+		// Send the question with Yes/No buttons. Each prompt gets a unique
+		// request ID so stale keyboards cannot answer later clarifies and
+		// the callback can be bound to the originating user.
 		replyMarkup := &telegram.InlineKeyboardMarkup{
 			InlineKeyboard: [][]telegram.InlineKeyboardButton{
 				{
-					{Text: "Yes", CallbackData: "clarify:yes"},
-					{Text: "No", CallbackData: "clarify:no"},
+					{Text: "Yes", CallbackData: fmt.Sprintf("clarify:%s:yes", reqID)},
+					{Text: "No", CallbackData: fmt.Sprintf("clarify:%s:no", reqID)},
 				},
 			},
 		}
-		if msg, err := bot.SendMessage(chatID, "❓ "+question,
-			&telegram.SendOpts{ReplyMarkup: replyMarkup, ParseMode: "Markdown", ReplyToMessageID: messageID}); err != nil {
+		msg, err := bot.SendMessage(chatID, "❓ "+question,
+			&telegram.SendOpts{ReplyMarkup: replyMarkup, ParseMode: "Markdown", ReplyToMessageID: messageID})
+		if err != nil {
 			return "", fmt.Errorf("clarify: send message: %w", err)
-		} else {
-			clarifyMsgIDs.Store(chatID, msg.ID)
-			defer clarifyMsgIDs.Delete(chatID)
 		}
+		req.msgID = msg.ID
 
 		// Wait for the user to click a button (or timeout).
 		select {

cmd/odek/telegram_clarify_test.go

@@ -0,0 +1,121 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/BackendStack21/odek/internal/telegram"
+)
+
+func TestParseClarifyCallback_Valid(t *testing.T) {
+	reqID, answer, ok := parseClarifyCallback("clarify:abcd1234:yes")
+	if !ok {
+		t.Fatal("expected valid clarify callback")
+	}
+	if reqID != "abcd1234" {
+		t.Errorf("reqID = %q, want abcd1234", reqID)
+	}
+	if answer != "yes" {
+		t.Errorf("answer = %q, want yes", answer)
+	}
+}
+
+func TestParseClarifyCallback_Invalid(t *testing.T) {
+	invalid := []string{
+		"clarify:yes",
+		"clarify:",
+		"clarify:abcd1234:maybe",
+		"skill_save:foo",
+		"apr:123",
+		"",
+	}
+	for _, data := range invalid {
+		if _, _, ok := parseClarifyCallback(data); ok {
+			t.Errorf("expected %q to be invalid", data)
+		}
+	}
+}
+
+func TestGenerateClarifyReqID_Unique(t *testing.T) {
+	seen := make(map[string]bool)
+	for i := 0; i < 100; i++ {
+		id := generateClarifyReqID()
+		if seen[id] {
+			t.Fatalf("duplicate request ID: %q", id)
+		}
+		seen[id] = true
+	}
+}
+
+// TestHandleClarifyCallback_BindsToOriginatingUser verifies that a clarify
+// callback from the correct user unblocks the channel and rejects a
+// callback from a different user.
+func TestHandleClarifyCallback_BindsToOriginatingUser(t *testing.T) {
+	// Start a fake Telegram server so EditMessageText does not fail.
+	var edits sync.Map
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if strings.HasSuffix(r.URL.Path, "/editMessageText") {
+			edits.Store(1, true)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.Write([]byte(`{"ok":true,"result":true}`))
+	}))
+	defer srv.Close()
+
+	bot := telegram.NewBot("test-token")
+	bot.BaseURL = srv.URL + "/bot" + bot.Token
+
+	reqID := generateClarifyReqID()
+	ch := make(chan string, 1)
+	pendingClarifyReqs.Store(reqID, &pendingClarifyReq{userID: 42, ch: ch, msgID: 123})
+	defer pendingClarifyReqs.Delete(reqID)
+
+	// Wrong user should be rejected without sending an answer.
+	resp, ok := handleClarifyCallback(1, 99, "clarify:"+reqID+":yes", bot)
+	if !ok {
+		t.Fatal("expected clarify callback to be recognized")
+	}
+	if !strings.Contains(resp, "meant for another user") {
+		t.Errorf("expected cross-user rejection, got: %q", resp)
+	}
+	select {
+	case <-ch:
+		t.Fatal("answer should not have been sent for wrong user")
+	default:
+	}
+
+	// Correct user should unblock the channel and update the message.
+	resp, ok = handleClarifyCallback(1, 42, "clarify:"+reqID+":yes", bot)
+	if !ok {
+		t.Fatal("expected clarify callback to be recognized")
+	}
+	if resp != "" {
+		t.Errorf("expected empty response for accepted callback, got: %q", resp)
+	}
+	select {
+	case ans := <-ch:
+		if ans != "yes" {
+			t.Errorf("answer = %q, want yes", ans)
+		}
+	default:
+		t.Fatal("expected answer to be delivered to channel")
+	}
+	if _, ok := edits.Load(1); !ok {
+		t.Error("expected EditMessageText to be called")
+	}
+}
+
+// TestHandleClarifyCallback_ExpiredRequest verifies that a callback for an
+// unknown/expired request returns a friendly expiration message.
+func TestHandleClarifyCallback_ExpiredRequest(t *testing.T) {
+	resp, ok := handleClarifyCallback(1, 42, "clarify:"+generateClarifyReqID()+":no", nil)
+	if !ok {
+		t.Fatal("expected clarify callback to be recognized")
+	}
+	if !strings.Contains(resp, "expired") {
+		t.Errorf("expected expiration message, got: %q", resp)
+	}
+}

docs/SECURITY.md

@@ -667,6 +667,12 @@ Now:
 
 `persist()` now hardens any existing file with `os.Chmod(path, 0600)` and opens the file with `os.O_WRONLY|os.O_CREATE|os.O_TRUNC` and mode `0600`, matching the permission model already applied to sessions, audit logs, and Telegram logs.
 
+### 39n. Telegram clarify callback binding
+
+The Telegram bot's `clarify` tool sent inline keyboard buttons with literal callback data `clarify:yes` and `clarify:no`. Because the data carried no request identifier or user binding, any group member (or a stale keyboard from an earlier task) could answer a clarify prompt intended for someone else, and a later clarify could be answered by a stale button from an earlier one.
+
+Each clarify prompt now generates a random request ID (embedded in callback data as `clarify:<reqID>:yes/no`). The handler validates the request ID, rejects callbacks from a different user than the one who triggered the prompt, and ignores callbacks for expired or already-answered prompts. The `Handler.OnCallbackQuery` signature now receives the originating `userID` so other callback handlers can apply the same binding.
+
 ### 40. `/api/resources` result limit cap
 
 The `/api/resources?q=...&limit=N` autocomplete endpoint previously accepted any positive `limit` value. It is now capped to 100 results both in the HTTP handler and in `Registry.Search`. This prevents a prompt-injected or attacker-forged request from forcing an unbounded directory walk and returning a multi-megabyte JSON response.

internal/telegram/e2e_test.go

@@ -336,7 +336,7 @@ func TestE2E_FullCallbackFlow(t *testing.T) {
 		capturedChatID int64
 		capturedData   string
 	)
-	handler.OnCallbackQuery = func(chatID int64, callbackData string) (string, error) {
+	handler.OnCallbackQuery = func(chatID int64, callbackData string, userID int64) (string, error) {
 		capturedChatID = chatID
 		capturedData = callbackData
 		return "You selected option 1!", nil

internal/telegram/handler.go

@@ -57,9 +57,11 @@ type Handler struct {
 	OnTextMessage func(chatID int64, messageID int, text string, forwarded bool, userID int64) (string, error)
 
 	// OnCallbackQuery is called when a callback query is received and
-	// it was NOT handled by the TelegramApprover. Returns the response
-	// text (may be empty).
-	OnCallbackQuery func(chatID int64, callbackData string) (string, error)
+	// it was NOT handled by the TelegramApprover. userID is the Telegram
+	// user who pressed the button, so callers can bind callbacks to the
+	// originating user and reject stale or cross-user button presses.
+	// Returns the response text (may be empty).
+	OnCallbackQuery func(chatID int64, callbackData string, userID int64) (string, error)
 
 	// OnCommand is called when a bot command (e.g. /start) is received.
 	// userID is the Telegram user who sent the command.
@@ -149,8 +151,8 @@ func defaultTextHandler() func(int64, int, string, bool, int64) (string, error)
 }
 
 // defaultCallbackHandler returns a default OnCallbackQuery callback.
-func defaultCallbackHandler() func(int64, string) (string, error) {
-	return func(_ int64, _ string) (string, error) {
+func defaultCallbackHandler() func(int64, string, int64) (string, error) {
+	return func(_ int64, _ string, _ int64) (string, error) {
 		return "Not implemented yet: callback query", nil
 	}
 }
@@ -409,7 +411,7 @@ func (h *Handler) handleCallback(cq *CallbackQuery) {
 	}
 
 	if h.OnCallbackQuery != nil {
-		resp, err := h.OnCallbackQuery(cq.Message.Chat.ID, cq.Data)
+		resp, err := h.OnCallbackQuery(cq.Message.Chat.ID, cq.Data, userID)
 		if err != nil {
 			h.log.Error("callback query handler failed", "chat_id", cq.Message.Chat.ID, "data", cq.Data, "error", err)
 			if h.OnError != nil {

internal/telegram/handler_error_test.go

@@ -60,7 +60,7 @@ func TestHandleCallback_ErrorSentToUser(t *testing.T) {
 	h := NewHandler(bot)
 	h.Config.AllowAllUsers = true // routing test
 
-	h.OnCallbackQuery = func(chatID int64, data string) (string, error) {
+	h.OnCallbackQuery = func(chatID int64, data string, userID int64) (string, error) {
 		return "", fmt.Errorf("simulated callback failure: %s", data)
 	}
 

internal/telegram/handler_recover_test.go

@@ -65,7 +65,7 @@ func TestHandleUpdate_RecoverFromPanicCallback(t *testing.T) {
 	h := NewHandler(bot)
 	h.Config.AllowAllUsers = true // routing test
 
-	h.OnCallbackQuery = func(chatID int64, data string) (string, error) {
+	h.OnCallbackQuery = func(chatID int64, data string, userID int64) (string, error) {
 		panic("simulated callback panic")
 	}