fix: scope ~/.odek/media to originating chat to block cross-chat re-disclosure (M-14) (#71)

Author: jkyberneees

AGENTS.md

@@ -170,7 +170,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
 - **Telegram photo caption wrapping** (`cmd/odek/telegram.go`) — photo captions cross the Telegram trust boundary, so they are wrapped as untrusted content both when passed to the local vision model and when injected into the main agent's user message.
 - **`send_message` callback prefix restriction** (`internal/tool/send_message.go` + `cmd/odek/telegram.go`) — the `send_message` tool rejects any button whose `callback_data` starts with a reserved internal prefix (`apr:`, `den:`, `trs:`, `clarify:`, `skill_save:`, `skill_skip:`); only user-facing `cb:` callbacks are allowed. The Telegram sender closure validates again as defense-in-depth, preventing a forged approval or skill button.
 - **Telegram class-trust guard + friction** (`internal/telegram/approver.go`) — the "Trust Session" button is hidden for `destructive`, `blocked`, `unknown`, and the synthetic `tool_batch` class, and a trust callback for those classes is treated as a denial. After 3 approvals of the same class in 60 seconds, friction mode hides the Trust Session shortcut and adds a warning, forcing per-call approval and breaking reflexive tap-through.
-- **Telegram outbound media hardening** (`internal/telegram/media_path.go` + `internal/telegram/approver.go` + `internal/telegram/handler.go` + `internal/tool/send_message.go` + `cmd/odek/telegram.go`) — paths supplied to `MEDIA:...` prefixes or `send_message(file=...)` are resolved to an absolute path and verified against an allowlist (cwd, `~/.odek/media/`, system temp dir). On Unix the final component is opened with `O_NOFOLLOW` and `fstat`'d to avoid a symlink TOCTOU race; `filepath.EvalSymlinks` ensures the resolved path does not escape the allowlist. Additionally, well-known secret subtrees (`~/.ssh`, `~/.aws`, `~/.gnupg`, `~/.odek` trust anchors, etc.) and any file whose basename starts with `.env` are rejected outright, and every outbound media upload now requires explicit user approval via `TelegramApprover.PromptMedia`, with an extra warning when the bot was launched from `$HOME` or `/`.
+- **Telegram outbound media hardening** (`internal/telegram/media_path.go` + `internal/telegram/approver.go` + `internal/telegram/handler.go` + `internal/tool/send_message.go` + `cmd/odek/telegram.go`) — paths supplied to `MEDIA:...` prefixes or `send_message(file=...)` are resolved to an absolute path and verified against an allowlist (cwd, `~/.odek/media/`, system temp dir). On Unix the final component is opened with `O_NOFOLLOW` and `fstat`'d to avoid a symlink TOCTOU race; `filepath.EvalSymlinks` ensures the resolved path does not escape the allowlist. Additionally, well-known secret subtrees (`~/.ssh`, `~/.aws`, `~/.gnupg`, `~/.odek` trust anchors, etc.) and any file whose basename starts with `.env` are rejected outright, and every outbound media upload now requires explicit user approval via `TelegramApprover.PromptMedia`, with an extra warning when the bot was launched from `$HOME` or `/`. The shared `~/.odek/media/` directory is further scoped per chat: `ResolveMediaPathForChat` accepts a file there only when its basename contains the originating chat's `_chat<chatID>_` tag (matching downloads produced by `DownloadVoice`/`DownloadPhoto`/`DownloadDocument`) or it lives under `~/.odek/media/chat<chatID>/`, preventing one chat from re-disclosing another chat's uploaded documents or media.
 - **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.

cmd/odek/telegram.go

@@ -1550,7 +1550,7 @@ func handleChatMessage(
 	// Wire the send_message tool so the agent can send intermediate
 	// messages, files, and interactive keyboards mid-task — not just
 	// at the final answer.
-	agentTools = append(agentTools, toolpkg.NewSendMessageTool(
+	agentTools = append(agentTools, toolpkg.NewSendMessageToolForChat(chatID,
 		func(text string, file string, buttons [][]map[string]string) error {
 			// Defense-in-depth: never send buttons that use reserved internal
 			// callback prefixes, even if the tool validation was bypassed.
@@ -2272,8 +2272,9 @@ func mediaTypeFromExt(path string) string {
 // sendTelegramMedia sends a file as a Telegram media message with caption
 // and optional inline keyboard. Detects the media type from file extension.
 func sendTelegramMedia(bot *telegram.Bot, chatID int64, mediaType, path, caption string, buttons [][]map[string]string) error {
-	// Defense-in-depth: validate the path against the media allowlist.
-	resolved, err := telegram.ResolveMediaPath(path)
+	// Defense-in-depth: validate the path against the media allowlist, scoped to
+	// this chat so one chat cannot request another chat's downloaded media.
+	resolved, err := telegram.ResolveMediaPathForChat(path, chatID)
 	if err != nil {
 		return fmt.Errorf("telegram media: %w", err)
 	}

docs/SECURITY.md

@@ -428,6 +428,8 @@ The path is resolved to an absolute, cleaned form with `filepath.Abs`, symlinks
 
 On top of the allowlist, `ResolveMediaPath` now rejects well-known secret subtrees (`~/.ssh`, `~/.aws`, `~/.gnupg`, `~/.odek` trust anchors, etc.) and any file whose basename starts with `.env`, so project API keys and host secrets cannot be uploaded even when the bot is launched from a broad base such as `$HOME` or `/`.
 
+The shared `~/.odek/media/` directory is additionally scoped per chat. Telegram callers use `ResolveMediaPathForChat`, which accepts a file inside `~/.odek/media/` only when its basename contains the originating chat's tag (`_chat<chatID>_`, matching the names produced by `DownloadVoice`, `DownloadPhoto`, and `DownloadDocument`) or when it lives under `~/.odek/media/chat<chatID>/`. This prevents a chat from asking the bot to re-send documents or media that were uploaded by a different chat.
+
 Finally, every outbound media upload requires explicit user approval via `TelegramApprover.PromptMedia` (`internal/telegram/approver.go`). The approval card shows the full file path and the `network_egress` risk class, and adds an explicit warning when the current working directory is `$HOME` or `/`. If no approver is registered (e.g. a standalone `Handler` outside the bot runtime), the upload is denied outright.
 
 ### 24. Session ID entropy + session-scoped auth tokens

docs/TELEGRAM.md

@@ -210,12 +210,13 @@ The handler uses `sync.Map` for `TelegramApprover` instances, keyed by `chatID`.
 
 ### Outbound Media
 
-The agent can send files back to the chat either by emitting a `MEDIA:` prefix in its final answer (`MEDIA:photo:/path`, `MEDIA:voice:/path`, `MEDIA:document:/path`) or by calling `send_message` with the `file` parameter. Before any upload, the path is validated by `internal/telegram.ResolveMediaPath`:
+The agent can send files back to the chat either by emitting a `MEDIA:` prefix in its final answer (`MEDIA:photo:/path`, `MEDIA:voice:/path`, `MEDIA:document:/path`) or by calling `send_message` with the `file` parameter. Before any upload, the path is validated by `internal/telegram.ResolveMediaPathForChat`:
 
 - Allowed directories: current working directory, `~/.odek/media/`, and the system temporary directory.
 - The path is resolved to an absolute, cleaned form and checked against the allowlist.
 - Symlinks are rejected: on Unix the final component is opened with `O_NOFOLLOW` and verified with `fstat` to close a TOCTOU race; on other platforms it is checked with `os.Lstat`. The resolved path must not escape the allowlist.
 - Files outside the allowlist (e.g. `/home/user/.ssh/id_rsa`) are refused, closing prompt-injection-driven exfiltration.
+- Files inside the shared `~/.odek/media/` directory are additionally scoped to the originating chat. A file is accepted only when its basename contains the chat's `_chat<chatID>_` tag (matching files downloaded by the bot) or when it lives under `~/.odek/media/chat<chatID>/`. This prevents one chat from asking the bot to re-send documents or media uploaded by another chat.
 
 ## Slash Commands (`commands.go`)
 

internal/telegram/handler.go

@@ -493,8 +493,9 @@ func (h *Handler) sendMedia(chatID int64, text string, replyToMessageID int) {
 		return
 	}
 
-	// Validate and resolve the media path against the allowlist.
-	resolved, err := ResolveMediaPath(filePath)
+	// Validate and resolve the media path against the allowlist, scoped to this
+	// chat so one chat cannot request another chat's downloaded media.
+	resolved, err := ResolveMediaPathForChat(filePath, chatID)
 	if err != nil {
 		h.log.Error("media file rejected", "chat_id", chatID, "path", filePath, "error", err)
 		if h.OnError != nil {

internal/telegram/media_path.go

@@ -28,7 +28,33 @@ import (
 // otherwise allowed. This prevents a prompt-injected agent from exfiltrating
 // arbitrary files such as /home/user/.ssh/id_rsa via MEDIA:... or
 // send_message(file=...), especially when the bot was launched from $HOME or /.
+//
+// For Telegram callers that know the originating chat, use
+// ResolveMediaPathForChat instead. It scopes the ~/.odek/media directory to
+// files whose names contain the chat-specific "_chat<chatID>_" tag, preventing
+// one chat from requesting media downloaded for another chat.
 func ResolveMediaPath(path string) (string, error) {
+	return resolveMediaPath(path, 0)
+}
+
+// ResolveMediaPathForChat is like ResolveMediaPath but additionally binds files
+// inside ~/.odek/media to the supplied chat ID. Files downloaded from Telegram
+// are named with a "_chat<chatID>_" prefix (e.g. "doc_chat12345_report.pdf"),
+// so a path inside ~/.odek/media is accepted only when its basename contains
+// that prefix or it lives under ~/.odek/media/chat<chatID>/. This prevents a
+// chat from re-disclosing documents or media that originated in another chat.
+//
+// A chatID of 0 disables the chat-scoped check and behaves like
+// ResolveMediaPath. Real Telegram chat IDs are never zero, so callers should
+// pass the actual chat ID whenever it is available.
+func ResolveMediaPathForChat(path string, chatID int64) (string, error) {
+	return resolveMediaPath(path, chatID)
+}
+
+// resolveMediaPath implements the shared validation for ResolveMediaPath and
+// ResolveMediaPathForChat. When chatID != 0, paths inside the odek media
+// directory must be scoped to that chat.
+func resolveMediaPath(path string, chatID int64) (string, error) {
 	if path == "" {
 		return "", fmt.Errorf("media path is empty")
 	}
@@ -66,7 +92,7 @@ func ResolveMediaPath(path string) (string, error) {
 	}
 	resolved = filepath.Clean(resolved)
 
-	allowed, err := mediaBaseDirs()
+	allowed, mediaDir, err := mediaBaseDirs()
 	if err != nil {
 		return "", fmt.Errorf("media path: allowed dirs: %w", err)
 	}
@@ -76,6 +102,12 @@ func ResolveMediaPath(path string) (string, error) {
 			if err := checkMediaPathSensitivity(resolved); err != nil {
 				return "", err
 			}
+			// Enforce per-chat isolation inside the shared media directory.
+			if chatID != 0 && mediaDir != "" && isPathInside(resolved, mediaDir) {
+				if err := checkChatMediaScope(resolved, mediaDir, chatID); err != nil {
+					return "", err
+				}
+			}
 			return resolved, nil
 		}
 	}
@@ -133,20 +165,43 @@ func BroadBaseWarning() string {
 	return ""
 }
 
+// checkChatMediaScope rejects media files inside ~/.odek/media that do not
+// belong to the requesting chat. Telegram downloads are tagged with
+// "_chat<chatID>_" in the basename; future per-chat subdirectories are also
+// allowed under ~/.odek/media/chat<chatID>/.
+func checkChatMediaScope(resolved, mediaDir string, chatID int64) error {
+	// Allow files under a per-chat subdirectory.
+	chatSubdir := filepath.Join(mediaDir, fmt.Sprintf("chat%d", chatID))
+	if isPathInside(resolved, chatSubdir) {
+		return nil
+	}
+
+	// Allow files whose basename contains the chat tag (e.g. doc_chat12345_x.pdf).
+	base := filepath.Base(resolved)
+	tag := fmt.Sprintf("_chat%d_", chatID)
+	if strings.Contains(base, tag) {
+		return nil
+	}
+
+	return fmt.Errorf("media path: file belongs to a different chat: %s", resolved)
+}
+
 // mediaBaseDirs returns the resolved, cleaned allowed base directories for
-// outbound media paths. Errors retrieving individual directories are ignored
+// outbound media paths and the resolved odek media directory (empty string if
+// it cannot be located). Errors retrieving individual directories are ignored
 // where safe to do so (a directory that cannot be located cannot contain a
 // valid media file), but the current working directory and temp directory are
 // always included.
-func mediaBaseDirs() ([]string, error) {
+func mediaBaseDirs() (dirs []string, mediaDir string, err error) {
 	cwd, err := os.Getwd()
 	if err != nil {
-		return nil, fmt.Errorf("getwd: %w", err)
+		return nil, "", fmt.Errorf("getwd: %w", err)
 	}
 
-	dirs := []string{cwd}
+	dirs = []string{cwd}
 
-	if mediaDir, err := MediaDir(); err == nil {
+	if md, err := MediaDir(); err == nil {
+		mediaDir = md
 		dirs = append(dirs, mediaDir)
 	}
 
@@ -160,7 +215,14 @@ func mediaBaseDirs() ([]string, error) {
 		}
 		resolved = append(resolved, d)
 	}
-	return resolved, nil
+	// Return the resolved media dir, not the unresolved one.
+	if mediaDir != "" {
+		mediaDir = filepath.Clean(mediaDir)
+		if real, err := filepath.EvalSymlinks(mediaDir); err == nil {
+			mediaDir = filepath.Clean(real)
+		}
+	}
+	return resolved, mediaDir, nil
 }
 
 // isPathInside reports whether child is equal to or inside parent, using

internal/telegram/media_path_test.go

@@ -1,6 +1,7 @@
 package telegram
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -422,6 +423,118 @@ func TestResolveMediaPath_RejectsOdekTrustAnchors(t *testing.T) {
 	}
 }
 
+// TestResolveMediaPathForChat_AllowsOwnChat verifies that files tagged for the
+// requesting chat inside ~/.odek/media are accepted.
+func TestResolveMediaPathForChat_AllowsOwnChat(t *testing.T) {
+	setupMediaPathTest(t)
+
+	mediaDir, err := MediaDir()
+	if err != nil {
+		t.Fatalf("MediaDir: %v", err)
+	}
+
+	chatID := int64(12345)
+	cases := []string{
+		fmt.Sprintf("doc_chat%d_report.pdf", chatID),
+		fmt.Sprintf("photo_chat%d_abc.jpg", chatID),
+		fmt.Sprintf("voice_chat%d_def.ogg", chatID),
+	}
+
+	for _, name := range cases {
+		t.Run(name, func(t *testing.T) {
+			f := filepath.Join(mediaDir, name)
+			if err := os.WriteFile(f, []byte("x"), 0644); err != nil {
+				t.Fatalf("write media file: %v", err)
+			}
+			if _, err := ResolveMediaPathForChat(f, chatID); err != nil {
+				t.Fatalf(" ResolveMediaPathForChat(%q, %d) error: %v", f, chatID, err)
+			}
+		})
+	}
+}
+
+// TestResolveMediaPathForChat_RejectsOtherChat verifies that files tagged for a
+// different chat inside ~/.odek/media are rejected, preventing cross-chat
+// re-disclosure of downloaded documents or media.
+func TestResolveMediaPathForChat_RejectsOtherChat(t *testing.T) {
+	setupMediaPathTest(t)
+
+	mediaDir, err := MediaDir()
+	if err != nil {
+		t.Fatalf("MediaDir: %v", err)
+	}
+
+	ownerChat := int64(12345)
+	attackerChat := int64(99999)
+
+	cases := []string{
+		fmt.Sprintf("doc_chat%d_report.pdf", ownerChat),
+		fmt.Sprintf("photo_chat%d_abc.jpg", ownerChat),
+		fmt.Sprintf("voice_chat%d_def.ogg", ownerChat),
+	}
+
+	for _, name := range cases {
+		t.Run(name, func(t *testing.T) {
+			f := filepath.Join(mediaDir, name)
+			if err := os.WriteFile(f, []byte("x"), 0644); err != nil {
+				t.Fatalf("write media file: %v", err)
+			}
+			_, err := ResolveMediaPathForChat(f, attackerChat)
+			if err == nil {
+				t.Fatalf("expected rejection for other chat's file: %s", f)
+			}
+			if !strings.Contains(err.Error(), "different chat") {
+				t.Errorf("expected 'different chat' in error, got: %v", err)
+			}
+		})
+	}
+}
+
+// TestResolveMediaPathForChat_AllowsChatSubdir verifies that files under a
+// per-chat subdirectory inside ~/.odek/media are accepted.
+func TestResolveMediaPathForChat_AllowsChatSubdir(t *testing.T) {
+	setupMediaPathTest(t)
+
+	mediaDir, err := MediaDir()
+	if err != nil {
+		t.Fatalf("MediaDir: %v", err)
+	}
+
+	chatID := int64(12345)
+	subdir := filepath.Join(mediaDir, fmt.Sprintf("chat%d", chatID))
+	if err := os.MkdirAll(subdir, 0755); err != nil {
+		t.Fatalf("mkdir chat subdir: %v", err)
+	}
+
+	f := filepath.Join(subdir, "shared-notes.pdf")
+	if err := os.WriteFile(f, []byte("x"), 0644); err != nil {
+		t.Fatalf("write media file: %v", err)
+	}
+	if _, err := ResolveMediaPathForChat(f, chatID); err != nil {
+		t.Fatalf("ResolveMediaPathForChat(%q, %d) error: %v", f, chatID, err)
+	}
+}
+
+// TestResolveMediaPathForChat_BackwardCompatibility verifies that the
+// unscoped ResolveMediaPath still accepts any file in ~/.odek/media, preserving
+// behavior for callers that do not know the originating chat.
+func TestResolveMediaPathForChat_BackwardCompatibility(t *testing.T) {
+	setupMediaPathTest(t)
+
+	mediaDir, err := MediaDir()
+	if err != nil {
+		t.Fatalf("MediaDir: %v", err)
+	}
+
+	f := filepath.Join(mediaDir, "doc_chat12345_report.pdf")
+	if err := os.WriteFile(f, []byte("x"), 0644); err != nil {
+		t.Fatalf("write media file: %v", err)
+	}
+	if _, err := ResolveMediaPath(f); err != nil {
+		t.Fatalf("ResolveMediaPath(%q) should still allow any media file: %v", f, err)
+	}
+}
+
 // TestBroadBaseWarning verifies that a warning is produced when the bot is
 // launched from $HOME or /.
 func TestBroadBaseWarning(t *testing.T) {