Skip to content

Commit cb190ff

Browse files
authored
H10: harden Telegram outbound-media allowlist and add approval gate
- ResolveMediaPath now rejects well-known secret subtrees and .env* files even when they sit inside an otherwise allowed base directory. - Added BroadBaseWarning to warn when the bot is launched from /Users/kyberneees or /. - Added TelegramApprover.PromptMedia for explicit user approval of outbound media uploads, with the broad-base warning surfaced in the prompt. - Wired the gate into Handler.sendMedia (MEDIA: prefix) and the send_message tool sender closure. - Updated affected tests and added regression tests for secret-subtree rejection, .env rejection, broad-base warning, and PromptMedia behavior. - Updated AGENTS.md and docs/SECURITY.md.
1 parent fbb9ced commit cb190ff

10 files changed

Lines changed: 405 additions & 6 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
170170
- **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.
171171
- **`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.
172172
- **Telegram class-trust guard** (`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. This matches the TTY/Web approver policy and prevents one batch approval from blanket-authorizing later destructive operations.
173-
- **Telegram outbound media path allowlist** (`internal/telegram/media_path.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, preventing prompt-injection-driven exfiltration of arbitrary files.
173+
- **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 `/`.
174174
- **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.
175175
- **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.
176176
- **Session ID entropy + session-scoped auth tokens** (`internal/session/session.go`, `cmd/odek/serve.go`) — session IDs now carry 128 bits of randomness (16 bytes / 32 hex chars); each session stores a 256-bit `AuthToken` required by `GET/DELETE/POST /api/sessions/<id>`, `POST /api/cancel`, and WebSocket session-resume messages via `X-Session-Token` header, `session_token` cookie, or `auth_token` WS field. Per-IP rate limiting (60/min) on session lookups adds a brute-force backstop.

cmd/odek/telegram.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,6 +1512,11 @@ func handleChatMessage(
15121512
safeText := telegram.EscapeMarkdown(text)
15131513

15141514
if file != "" {
1515+
// Outbound media can exfiltrate arbitrary files; require explicit
1516+
// user approval before resolving or uploading the path.
1517+
if err := approver.PromptMedia(file); err != nil {
1518+
return fmt.Errorf("send_message: media upload denied: %w", err)
1519+
}
15151520
// Detect media type from extension.
15161521
mediaType := mediaTypeFromExt(file)
15171522
return sendTelegramMedia(bot, chatID, mediaType, file, safeText, buttons)

docs/SECURITY.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -407,15 +407,19 @@ The Telegram bot previously used a PID file at `~/.odek/telegram.pid` to enforce
407407

408408
The `send_message` tool lets the agent send inline keyboard buttons. Each button's `callback_data` is validated by the tool and again by the Telegram sender closure: any value that starts with a reserved internal prefix (`apr:`, `den:`, `trs:`, `clarify:`, `skill_save:`, `skill_skip:`) is rejected. Only user-facing `cb:` callbacks are allowed. This prevents a compromised or prompt-injected agent from presenting a button that, when clicked, would forge an approval decision or trigger a skill action.
409409

410-
### 23. Telegram outbound media path allowlist
410+
### 23. Telegram outbound media hardening
411411

412412
When the agent emits `MEDIA:photo:/path`, `MEDIA:voice:/path`, `MEDIA:document:/path`, or `send_message` with a `file`, the path is validated by `internal/telegram.ResolveMediaPath` before upload. Only paths inside an allowed base directory are permitted:
413413

414414
- the current working directory,
415415
- `~/.odek/media/`, and
416416
- the system temporary directory.
417417

418-
The path is resolved to an absolute, cleaned form with `filepath.Abs`, symlinks are resolved with `filepath.EvalSymlinks`, and the final component is verified with an atomic `O_NOFOLLOW` open + `fstat` (Unix). If the final component is a symlink, or if the resolved path escapes the allowlist, the upload is rejected. This closes the arbitrary-file-read/exfiltration vector where a prompt-injected agent asks the bot to send files such as `/home/user/.ssh/id_rsa`.
418+
The path is resolved to an absolute, cleaned form with `filepath.Abs`, symlinks are resolved with `filepath.EvalSymlinks`, and the final component is verified with an atomic `O_NOFOLLOW` open + `fstat` (Unix). If the final component is a symlink, or if the resolved path escapes the allowlist, the upload is rejected.
419+
420+
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 `/`.
421+
422+
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.
419423

420424
### 24. Session ID entropy + session-scoped auth tokens
421425

@@ -663,7 +667,7 @@ A prompt-injected agent could overwrite `schedules.json` to install persistent c
663667
| Local process brute-forces session IDs to read transcripts | 128-bit IDs + session-scoped auth tokens + per-IP rate limiting |
664668
| Telegram bot scanned by random user | Allowlist enforced before any tool call |
665669
| Agent sends fake approval/skill button via `send_message` | Reserved internal callback prefixes rejected; only `cb:` allowed |
666-
| Agent exfiltrates arbitrary file via Telegram media | Outbound paths restricted to cwd, `~/.odek/media/`, and temp dir; symlinks rejected |
670+
| Agent exfiltrates arbitrary file via Telegram media | Outbound paths restricted to cwd, `~/.odek/media/`, and temp dir; secret subtrees and `.env*` files rejected; explicit user approval required for every upload |
667671
| Auto-saved skill auto-activates on next session | Provenance gate pins NeedsReview skills to Lazy |
668672
| Memory replays a previously-injected episode forever | Tainted episodes filtered from `Search` |
669673
| User reflex-approves a destructive class after many benign ones | Friction mode requires typed `approve` + 1.5 s pause |

internal/telegram/approver.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,17 @@ func (a *TelegramApprover) PromptOperation(op danger.ToolOperation) error {
222222
return a.PromptCommand(op.Risk, desc, "")
223223
}
224224

225+
// PromptMedia asks the user to approve an outbound Telegram media upload.
226+
// The file path is shown in full and, when the bot was launched from a broad
227+
// base such as $HOME or /, an explicit warning is added to the prompt.
228+
func (a *TelegramApprover) PromptMedia(path string) error {
229+
desc := "Outbound Telegram media upload"
230+
if w := BroadBaseWarning(); w != "" {
231+
desc += "\n⚠️ " + w
232+
}
233+
return a.PromptCommand(danger.NetworkEgress, path, desc)
234+
}
235+
225236
// HandleCallback processes a callback query from an inline keyboard approval.
226237
// It parses the callback data, looks up the pending request, and unblocks
227238
// the waiting goroutine. Callbacks are only accepted from the originating

internal/telegram/approver_test.go

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,3 +616,139 @@ func extractCallbackID(body, prefix string) string {
616616
}
617617
return rest
618618
}
619+
620+
// TestPromptMedia_Approves verifies that PromptMedia sends an approval prompt
621+
// for an outbound media upload and returns nil when the user approves.
622+
func TestPromptMedia_Approves(t *testing.T) {
623+
rec := new(requestRecorder)
624+
ts := testServer(t, rec)
625+
defer ts.Close()
626+
bot := testBot(t, ts)
627+
628+
a := NewTelegramApprover(bot, 1, 0)
629+
done := make(chan error, 1)
630+
go func() { done <- a.PromptMedia("/tmp/photo.jpg") }()
631+
632+
// Wait for the prompt to be registered.
633+
var body string
634+
deadline := time.Now().Add(2 * time.Second)
635+
for time.Now().Before(deadline) {
636+
rec.mu.Lock()
637+
if len(rec.requests) > 0 {
638+
body = rec.requests[len(rec.requests)-1].Body
639+
}
640+
rec.mu.Unlock()
641+
if body != "" {
642+
break
643+
}
644+
time.Sleep(10 * time.Millisecond)
645+
}
646+
if body == "" {
647+
t.Fatal("prompt request was not sent")
648+
}
649+
if !strings.Contains(body, "/tmp/photo.jpg") {
650+
t.Errorf("approval prompt must show the media path, got body:\n%s", body)
651+
}
652+
if !strings.Contains(body, "network_egress") {
653+
t.Errorf("approval prompt must show the network_egress risk class, got body:\n%s", body)
654+
}
655+
656+
id := extractCallbackID(body, cbPrefixApprove)
657+
if id == "" {
658+
t.Fatal("could not extract approve callback id")
659+
}
660+
a.HandleCallback(cbPrefixApprove+id, 0)
661+
if err := <-done; err != nil {
662+
t.Fatalf("approve should succeed: %v", err)
663+
}
664+
}
665+
666+
// TestPromptMedia_Deny verifies that PromptMedia returns an error when the
667+
// user denies the upload.
668+
func TestPromptMedia_Deny(t *testing.T) {
669+
rec := new(requestRecorder)
670+
ts := testServer(t, rec)
671+
defer ts.Close()
672+
bot := testBot(t, ts)
673+
674+
a := NewTelegramApprover(bot, 1, 0)
675+
done := make(chan error, 1)
676+
go func() { done <- a.PromptMedia("/tmp/photo.jpg") }()
677+
678+
var body string
679+
deadline := time.Now().Add(2 * time.Second)
680+
for time.Now().Before(deadline) {
681+
rec.mu.Lock()
682+
if len(rec.requests) > 0 {
683+
body = rec.requests[len(rec.requests)-1].Body
684+
}
685+
rec.mu.Unlock()
686+
if body != "" {
687+
break
688+
}
689+
time.Sleep(10 * time.Millisecond)
690+
}
691+
if body == "" {
692+
t.Fatal("prompt request was not sent")
693+
}
694+
695+
id := extractCallbackID(body, cbPrefixDeny)
696+
if id == "" {
697+
t.Fatal("could not extract deny callback id")
698+
}
699+
a.HandleCallback(cbPrefixDeny+id, 0)
700+
err := <-done
701+
if err == nil {
702+
t.Fatal("deny should return an error")
703+
}
704+
if !strings.Contains(err.Error(), "denied") {
705+
t.Errorf("expected 'denied' in error, got: %v", err)
706+
}
707+
}
708+
709+
// TestPromptMedia_BroadBaseWarning verifies that the approval prompt includes
710+
// a warning when the bot is launched from $HOME.
711+
func TestPromptMedia_BroadBaseWarning(t *testing.T) {
712+
home := t.TempDir()
713+
t.Setenv("HOME", home)
714+
t.Setenv("USERPROFILE", home)
715+
t.Chdir(home)
716+
717+
rec := new(requestRecorder)
718+
ts := testServer(t, rec)
719+
defer ts.Close()
720+
bot := testBot(t, ts)
721+
722+
a := NewTelegramApprover(bot, 1, 0)
723+
done := make(chan error, 1)
724+
go func() { done <- a.PromptMedia("/home/user/project/plot.png") }()
725+
726+
var body string
727+
deadline := time.Now().Add(2 * time.Second)
728+
for time.Now().Before(deadline) {
729+
rec.mu.Lock()
730+
if len(rec.requests) > 0 {
731+
body = rec.requests[len(rec.requests)-1].Body
732+
}
733+
rec.mu.Unlock()
734+
if body != "" {
735+
break
736+
}
737+
time.Sleep(10 * time.Millisecond)
738+
}
739+
if body == "" {
740+
t.Fatal("prompt request was not sent")
741+
}
742+
if !strings.Contains(body, "$HOME") {
743+
t.Errorf("approval prompt must warn when cwd == $HOME, got body:\n%s", body)
744+
}
745+
746+
id := extractCallbackID(body, cbPrefixApprove)
747+
if id == "" {
748+
t.Fatal("could not extract approve callback id")
749+
}
750+
a.HandleCallback(cbPrefixApprove+id, 0)
751+
if err := <-done; err != nil {
752+
t.Fatalf("approve should succeed: %v", err)
753+
}
754+
}

internal/telegram/e2e_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,11 @@ func TestE2E_MediaFlow(t *testing.T) {
620620
handler := NewHandler(bot)
621621
handler.Config.AllowAllUsers = true // routing test
622622

623+
// Auto-approve outbound media so the test verifies the upload path.
624+
approver := NewTelegramApprover(bot, 777, 0)
625+
approver.SetTrustAll(true)
626+
handler.SetApprover(777, approver)
627+
623628
// OnTextMessage returns a MEDIA:photo response.
624629
handler.OnTextMessage = func(chatID int64, messageID int, text string, _ bool, _ int64) (string, error) {
625630
return "MEDIA:photo:" + tmpPath, nil
@@ -722,6 +727,11 @@ func TestE2E_VoiceMediaFlow(t *testing.T) {
722727
handler := NewHandler(bot)
723728
handler.Config.AllowAllUsers = true // routing test
724729

730+
// Auto-approve outbound media so the test verifies the upload path.
731+
approver := NewTelegramApprover(bot, 888, 0)
732+
approver.SetTrustAll(true)
733+
handler.SetApprover(888, approver)
734+
725735
handler.OnTextMessage = func(chatID int64, messageID int, text string, _ bool, _ int64) (string, error) {
726736
return "MEDIA:voice:" + tmpPath, nil
727737
}

internal/telegram/handler.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,26 @@ func (h *Handler) sendMedia(chatID int64, text string, replyToMessageID int) {
473473
mediaType := parts[0]
474474
filePath := parts[1]
475475

476+
// Outbound media can exfiltrate arbitrary files, so it requires an
477+
// explicit user approval before the path is resolved or uploaded. The
478+
// Telegram bot registers a per-chat approver in production; without one
479+
// we fail closed.
480+
a := h.GetApprover(chatID)
481+
if a == nil {
482+
h.log.Error("media upload rejected: no approver configured", "chat_id", chatID, "path", filePath)
483+
if h.OnError != nil {
484+
h.OnError(chatID, fmt.Errorf("telegram: media upload rejected: no approver configured"))
485+
}
486+
return
487+
}
488+
if err := a.PromptMedia(filePath); err != nil {
489+
h.log.Error("media upload denied", "chat_id", chatID, "path", filePath, "error", err)
490+
if h.OnError != nil {
491+
h.OnError(chatID, fmt.Errorf("telegram: media upload denied: %s: %w", filePath, err))
492+
}
493+
return
494+
}
495+
476496
// Validate and resolve the media path against the allowlist.
477497
resolved, err := ResolveMediaPath(filePath)
478498
if err != nil {

internal/telegram/handler_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,6 +1133,9 @@ func TestSendResponse_MediaPhoto(t *testing.T) {
11331133
defer ts.Close()
11341134
bot := testBot(t, ts)
11351135
h := NewHandler(bot)
1136+
approver := NewTelegramApprover(bot, 123, 0)
1137+
approver.SetTrustAll(true)
1138+
h.SetApprover(123, approver)
11361139

11371140
h.SendResponse(123, "MEDIA:photo:"+tmpPath, 0)
11381141

@@ -1163,6 +1166,9 @@ func TestSendResponse_MediaVoice(t *testing.T) {
11631166
defer ts.Close()
11641167
bot := testBot(t, ts)
11651168
h := NewHandler(bot)
1169+
approver := NewTelegramApprover(bot, 456, 0)
1170+
approver.SetTrustAll(true)
1171+
h.SetApprover(456, approver)
11661172

11671173
h.SendResponse(456, "MEDIA:voice:"+tmpPath, 0)
11681174

@@ -1185,6 +1191,9 @@ func TestSendResponse_MediaFileNotFound(t *testing.T) {
11851191
defer ts.Close()
11861192
bot := testBot(t, ts)
11871193
h := NewHandler(bot)
1194+
approver := NewTelegramApprover(bot, 123, 0)
1195+
approver.SetTrustAll(true)
1196+
h.SetApprover(123, approver)
11881197

11891198
errCalled := false
11901199
h.OnError = func(_ int64, err error) {
@@ -1218,6 +1227,9 @@ func TestSendResponse_MediaUnknownType(t *testing.T) {
12181227
defer ts.Close()
12191228
bot := testBot(t, ts)
12201229
h := NewHandler(bot)
1230+
approver := NewTelegramApprover(bot, 123, 0)
1231+
approver.SetTrustAll(true)
1232+
h.SetApprover(123, approver)
12211233

12221234
h.SendResponse(123, "MEDIA:video:"+tmpPath, 0)
12231235

@@ -1248,6 +1260,9 @@ func TestSendResponse_MediaDocument(t *testing.T) {
12481260
defer ts.Close()
12491261
bot := testBot(t, ts)
12501262
h := NewHandler(bot)
1263+
approver := NewTelegramApprover(bot, 456, 0)
1264+
approver.SetTrustAll(true)
1265+
h.SetApprover(456, approver)
12511266

12521267
h.SendResponse(456, "MEDIA:document:"+tmpPath, 0)
12531268

0 commit comments

Comments
 (0)