Skip to content

Commit 465a433

Browse files
authored
M2: scope Telegram sessions/plans to requesting chat (#59)
1 parent 5fa47f7 commit 465a433

13 files changed

Lines changed: 445 additions & 325 deletions

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
173173
- **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.
176+
- **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.
176177
- **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.
177178
- **Skill/episode untrusted wrapper** (`internal/loop/loop.go` + `odek.go`) — skill context and retrieved session-episode context are passed through the caller-provided untrusted wrapper (the same nonce'd `<untrusted_content_*>` boundary used for tool output) before being injected into the model's system context. This prevents a compromised or tainted skill/episode from being treated as trusted system instructions.
178179
- **`env` / `printenv` environment-dump gate** (`internal/danger/classifier.go`) — bare `env` and `printenv` invocations are classified as `system_write` because they can leak process-environment secrets that the redaction scanner does not recognise. `env VAR=value <cmd>` still classifies `<cmd>` normally.

cmd/odek/telegram.go

Lines changed: 70 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -385,9 +385,9 @@ func telegramCmd(args []string) error {
385385
return formatStats(cs), nil
386386
}
387387

388-
// Handle /sessions — list recent sessions from the store.
388+
// Handle /sessions — list recent sessions belonging to this chat.
389389
if cmdName == "sessions" {
390-
infos, err := sessionManager.ListSessions(10)
390+
infos, err := sessionManager.ListSessions(chatID, 10)
391391
if err != nil {
392392
return fmt.Sprintf("❌ Failed to list sessions: %v", err), nil
393393
}
@@ -411,7 +411,7 @@ func telegramCmd(args []string) error {
411411
return b.String(), nil
412412
}
413413

414-
// Handle /resume <id> — switch to a different session.
414+
// Handle /resume <id> — switch to a different session owned by this chat.
415415
if cmdName == "resume" {
416416
sessionID := strings.TrimSpace(argsStr)
417417
if sessionID == "" {
@@ -431,7 +431,7 @@ func telegramCmd(args []string) error {
431431
), nil
432432
}
433433

434-
// Handle /prune [days] — clean up old sessions and plans.
434+
// Handle /prune [days] — clean up old sessions and plans for this chat.
435435
if cmdName == "prune" {
436436
days := 30
437437
if strings.TrimSpace(argsStr) != "" {
@@ -441,11 +441,11 @@ func telegramCmd(args []string) error {
441441
return "❗ Usage: `/prune [days]`\n\nExample: `/prune 7` to remove sessions and plans older than 7 days.", nil
442442
}
443443
}
444-
sessionsRemoved, err := sessionManager.PruneSessions(days)
444+
sessionsRemoved, err := sessionManager.PruneSessions(chatID, days)
445445
if err != nil {
446446
return fmt.Sprintf("❌ Failed to prune sessions: %v", err), nil
447447
}
448-
plansRemoved, err := sessionManager.PrunePlans(days)
448+
plansRemoved, err := sessionManager.PrunePlans(chatID, days)
449449
if err != nil {
450450
return fmt.Sprintf("❌ Failed to prune plans: %v", err), nil
451451
}
@@ -471,26 +471,78 @@ func telegramCmd(args []string) error {
471471
return "❗ Usage: `/plan <description>`\n\nExample: `/plan Add user authentication with OAuth2`", nil
472472
}
473473
slug := telegram.Slugify(description)
474+
planFile := fmt.Sprintf("~/.odek/plans/chat%d/%s.md", chatID, slug)
474475
prompt := fmt.Sprintf(
475476
"Create a detailed implementation plan for: %s\n\n"+
476-
"Save the plan as a markdown file to `~/.odek/plans/%s.md`. "+
477+
"Save the plan as a markdown file to `%s`. "+
477478
"The plan should include:\n"+
478479
"- Overview and goals\n"+
479480
"- Architecture / design\n"+
480481
"- Implementation steps (bite-sized tasks)\n"+
481482
"- File paths and key code locations\n"+
482483
"- Testing strategy\n\n"+
483484
"Use your write_file tool to save the plan.",
484-
description, slug,
485+
description, planFile,
485486
)
486487
go handleChatMessage(chatID, messageID, userID, prompt, bot, handler, sessionManager,
487488
resolved, systemMessage, handlerLog)
488489
return fmt.Sprintf("📝 *Planning* `%s`…\n\n_Generating plan for: %s_", slug, description), nil
489490
}
490491

491-
// Handle /plan-resume — inject most recent plan into session context.
492+
// Handle /plans — list saved plans for this chat.
493+
if cmdName == "plans" {
494+
infos, err := telegram.ListPlans(chatID, 20)
495+
if err != nil {
496+
return fmt.Sprintf("❌ Failed to list plans: %v", err), nil
497+
}
498+
if len(infos) == 0 {
499+
return "📋 *Plans* — No plans found.\n\nCreate one with `/plan <description>`", nil
500+
}
501+
var b strings.Builder
502+
b.WriteString("📋 *Plans*\n\n")
503+
for _, p := range infos {
504+
ago := time.Since(p.ModTime).Round(time.Minute)
505+
fmt.Fprintf(&b, "`%s` — %s ago\n", p.Slug, ago)
506+
if p.Preview != "" {
507+
fmt.Fprintf(&b, " _%s_\n", truncateStr(p.Preview, 60))
508+
}
509+
}
510+
b.WriteString("\nUse `/plan_view <slug>` to read a plan.")
511+
return b.String(), nil
512+
}
513+
514+
// Handle /plan_view <slug> — read a plan for this chat.
515+
if cmdName == "plan_view" {
516+
slug := strings.TrimSpace(argsStr)
517+
if slug == "" {
518+
return "❗ Usage: `/plan_view <slug>`\n\nUse `/plans` to see available plans.", nil
519+
}
520+
matched, content, err := telegram.ReadPlan(chatID, slug)
521+
if err != nil {
522+
return fmt.Sprintf("❌ %v", err), nil
523+
}
524+
if len(content) > 3900 {
525+
content = content[:3900] + "\n\n… _(truncated — plan too long for Telegram)_"
526+
}
527+
return fmt.Sprintf("📄 *Plan: `%s`*\n\n%s", matched, content), nil
528+
}
529+
530+
// Handle /plan_delete <slug> — delete a plan for this chat.
531+
if cmdName == "plan_delete" {
532+
slug := strings.TrimSpace(argsStr)
533+
if slug == "" {
534+
return "❗ Usage: `/plan_delete <slug>`\n\nUse `/plans` to see available plans.", nil
535+
}
536+
matched, err := telegram.DeletePlan(chatID, slug)
537+
if err != nil {
538+
return fmt.Sprintf("❌ %v", err), nil
539+
}
540+
return fmt.Sprintf("🗑️ *Plan deleted*: `%s`", matched), nil
541+
}
542+
543+
// Handle /plan-resume — inject most recent plan for this chat into session context.
492544
if cmdName == "plan_resume" {
493-
slug, content, err := telegram.MostRecentPlan()
545+
slug, content, err := telegram.MostRecentPlan(chatID)
494546
if err != nil {
495547
return fmt.Sprintf("❌ %v", err), nil
496548
}
@@ -2280,6 +2332,14 @@ func buttonsToMarkup(buttons [][]map[string]string) *telegram.InlineKeyboardMark
22802332
return markup
22812333
}
22822334

2335+
// truncateStr shortens s to maxLen, appending "…" if trimmed.
2336+
func truncateStr(s string, maxLen int) string {
2337+
if len(s) <= maxLen {
2338+
return s
2339+
}
2340+
return s[:maxLen] + "…"
2341+
}
2342+
22832343
// deleteToolTraceMessages deletes all individual tool trace messages for a chat.
22842344
// Used to clean up after the agent finishes (success or error).
22852345
func deleteToolTraceMessages(bot *telegram.Bot, chatID int64, msgIDs *sync.Map) {

cmd/odek/telegram_test.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -744,14 +744,6 @@ func narratorToolCallMessage(name, args string) string {
744744
}
745745
}
746746

747-
// truncateStr truncates a string for display in test logs.
748-
func truncateStr(s string, n int) string {
749-
if len(s) <= n {
750-
return s
751-
}
752-
return s[:n] + "..."
753-
}
754-
755747
// ── Tool Latency ────────────────────────────────────────────────────
756748

757749
// TestToolLatencyTracking verifies that recordToolStart and popToolLatency

docs/SECURITY.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,18 @@ Plan files live in `~/.odek/plans/` and are loaded by `/plan_view` and injected
543543

544544
Telegram log files were created with world-readable `0644` permissions, exposing chat IDs and task snippets to other local users. `NewFileLogger` now creates log files with `0600` and `os.Chmod`'s existing files to the same mode.
545545

546+
### 39c. Telegram chat-scoped sessions and plans
547+
548+
The Telegram bot's `/sessions`, `/resume`, `/prune`, `/plans`, `/plan_view`, `/plan_delete`, and `/plan_resume` commands previously operated on the global `~/.odek/sessions` and `~/.odek/plans` stores that are shared with the CLI. Any allowed chat could list the operator's CLI sessions (including task snippets that often contain secrets), resume one so its history entered the attacker's chat context, prune the operator's history, or read/delete plans created by other chats.
549+
550+
These commands are now scoped to the requesting chat:
551+
552+
- Each Telegram chat owns sessions with IDs of the form `tg-<chatID>` (and timestamped archives `tg-<chatID>-<YYYYMMDD>-<HHMMSS>`). `ListSessions`, `ResumeSession`, and `PruneSessions` only consider sessions whose ID starts with the caller's `tg-<chatID>` prefix.
553+
- `ResumeSession` explicitly rejects a direct ID that belongs to a different chat.
554+
- Plans are stored under `~/.odek/plans/chat<chatID>/`. `ListPlans`, `ReadPlan`, `DeletePlan`, and `MostRecentPlan` only look inside the caller's per-chat directory. Chat ID `0` is reserved as a global/admin scope mapping to the root `~/.odek/plans/` directory.
555+
556+
This removes the cross-chat session/plan disclosure path while keeping the CLI and admin flows functional.
557+
546558
### 40. `/api/resources` result limit cap
547559

548560
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.

docs/TELEGRAM.md

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -231,12 +231,12 @@ The agent can send files back to the chat either by emitting a `MEDIA:` prefix i
231231
| `/mode` | Show current agent modes (interaction_mode, sandbox, skills) |
232232
| `/restart` | Gracefully restart the bot process. Restricted to operator chats/users and rate-limited to once per 60 seconds. |
233233
| `/plan <description>` | Create a new plan from a natural language description |
234-
| `/plans` | List all saved plans |
235-
| `/plan-view <slug>` | View a specific plan's content |
236-
| `/plan-delete <slug>` | Delete a saved plan |
237-
| `/sessions` | List recent conversation sessions |
238-
| `/resume <session_id>` | Resume a previous session by ID |
239-
| `/prune [days]` | Clean up old sessions (default: 30 days) |
234+
| `/plans` | List saved plans for this chat |
235+
| `/plan-view <slug>` | View a specific plan's content for this chat |
236+
| `/plan-delete <slug>` | Delete a saved plan for this chat |
237+
| `/sessions` | List recent conversation sessions for this chat |
238+
| `/resume <session_id>` | Resume a previous session owned by this chat |
239+
| `/prune [days]` | Clean up old sessions and plans for this chat (default: 30 days) |
240240
| `/schedules` | List scheduled tasks (id, on/off, cron, next fire, last status) |
241241
| `/schedule <subcommand>` | Manage scheduled tasks — `add`, `rm`, `enable`, `disable`, `run`, `next`, `view`. Mutating commands are restricted to configured operator chats/users. See [Managing schedules from Telegram](SCHEDULES.md#managing-from-telegram) |
242242

@@ -262,6 +262,7 @@ The `SessionManager` manages per-chat Telegram agent conversations, backed by th
262262
5. **Session recall** — the user message is saved to the session store *before* the agent loop runs, enabling `session_search` to find the current conversation's data during the same turn
263263
6. Active sessions survive bot restarts — on reconnect, the session is loaded from disk
264264
7. **/now archives** — using `/new` archives the current session with a timestamped ID (`tg-<chatID>-<YYYYMMDD>-<HHMMSS>`) before starting fresh. Archived sessions remain on disk and are visible via `odek session list`.
265+
8. **Chat-scoped listing/resume/prune**`/sessions`, `/resume`, and `/prune` only operate on sessions belonging to the requesting chat. A chat cannot list, resume, or delete another chat's sessions.
265266

266267
### Key Methods
267268

@@ -278,7 +279,7 @@ The `clarifyChannels` sync.Map provides per-chat channels for the agent to ask t
278279

279280
## Plan Management (`plan.go`)
280281

281-
Plans are stored as markdown files in `~/.odek/plans/<slug>.md`. Each plan is created from a natural language description and persisted for later review.
282+
Plans are stored as markdown files in `~/.odek/plans/chat<chatID>/<slug>.md`, isolating each Telegram chat's plans. Each plan is created from a natural language description and persisted for later review. The `/plan` command instructs the agent to save plans into the caller's per-chat directory, and `/plans`, `/plan_view`, `/plan_delete`, and `/plan_resume` only see plans belonging to that chat.
282283

283284
### Size Cap
284285

@@ -292,11 +293,11 @@ To prevent a prompt-injected agent from causing an out-of-memory condition, plan
292293
| Function | Purpose |
293294
|---|---|
294295
| `Slugify(description)` | Convert description to filesystem-safe slug (max 60 chars) |
295-
| `ListPlans(limit)` | List plans sorted by modification time (newest first) |
296-
| `ReadPlan(slug)` | Load plan content by exact slug or prefix match |
297-
| `DeletePlan(slug)` | Delete plan by exact slug or prefix match |
298-
| `MostRecentPlan()` | Return the most recently modified plan's content |
299-
| `EnsurePlansDir()` | Create plans directory if it doesn't exist |
296+
| `ListPlans(chatID, limit)` | List plans for a chat sorted by modification time (newest first) |
297+
| `ReadPlan(chatID, slug)` | Load a chat's plan content by exact slug or prefix match |
298+
| `DeletePlan(chatID, slug)` | Delete a chat's plan by exact slug or prefix match |
299+
| `MostRecentPlan(chatID)` | Return the most recently modified plan's content for a chat |
300+
| `EnsurePlansDir(chatID)` | Create per-chat plans directory if it doesn't exist |
300301

301302
### Slug Generation
302303

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
package telegram
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
"time"
9+
10+
"github.com/BackendStack21/odek/internal/llm"
11+
"github.com/BackendStack21/odek/internal/session"
12+
)
13+
14+
// Regression tests for M-2: Telegram session/plan commands must be scoped
15+
// to the requesting chat. Cross-chat access must be rejected.
16+
17+
func TestResumeSession_CrossChatRejected(t *testing.T) {
18+
sm, _ := setupTestSessionManager(t)
19+
20+
const ownerChat int64 = 999
21+
const attackerChat int64 = 100
22+
23+
if err := sm.Save(ownerChat, []llm.Message{{Role: "user", Content: "secret"}}); err != nil {
24+
t.Fatalf("Save failed: %v", err)
25+
}
26+
27+
_, err := sm.ResumeSession(attackerChat, "tg-999")
28+
if err == nil {
29+
t.Fatal("ResumeSession should reject a session belonging to a different chat")
30+
}
31+
if !strings.Contains(err.Error(), "different chat") {
32+
t.Errorf("error = %q, want 'different chat'", err)
33+
}
34+
}
35+
36+
func TestListSessions_ChatScoped(t *testing.T) {
37+
sm, _ := setupTestSessionManager(t)
38+
39+
for _, chatID := range []int64{111, 222, 333} {
40+
if err := sm.Save(chatID, []llm.Message{{Role: "user", Content: "msg"}}); err != nil {
41+
t.Fatalf("Save(%d) failed: %v", chatID, err)
42+
}
43+
}
44+
45+
infos, err := sm.ListSessions(222, 0)
46+
if err != nil {
47+
t.Fatalf("ListSessions failed: %v", err)
48+
}
49+
if len(infos) != 1 {
50+
t.Errorf("ListSessions returned %d, want 1", len(infos))
51+
}
52+
if len(infos) == 1 && infos[0].ID != "tg-222" {
53+
t.Errorf("session ID = %q, want tg-222", infos[0].ID)
54+
}
55+
}
56+
57+
func TestPruneSessions_ChatScoped(t *testing.T) {
58+
sm, st := setupTestSessionManager(t)
59+
60+
makeOldSession := func(chatID int64, id string) {
61+
sess := &session.Session{
62+
ID: id,
63+
CreatedAt: time.Now().Add(-60 * 24 * time.Hour),
64+
UpdatedAt: time.Now().Add(-60 * 24 * time.Hour),
65+
Task: id,
66+
Messages: nil,
67+
}
68+
if err := st.Save(sess); err != nil {
69+
t.Fatalf("store.Save(%q) failed: %v", id, err)
70+
}
71+
}
72+
73+
makeOldSession(1, "tg-1-old")
74+
makeOldSession(2, "tg-2-old")
75+
76+
removed, err := sm.PruneSessions(1, 30)
77+
if err != nil {
78+
t.Fatalf("PruneSessions failed: %v", err)
79+
}
80+
if removed != 1 {
81+
t.Errorf("PruneSessions removed %d, want 1", removed)
82+
}
83+
84+
// Chat 2's session must remain untouched.
85+
_, err = st.Load("tg-2-old")
86+
if err != nil {
87+
t.Errorf("chat 2 session should not have been pruned: %v", err)
88+
}
89+
}
90+
91+
func TestReadPlan_ChatScoped(t *testing.T) {
92+
tmp := t.TempDir()
93+
t.Setenv("HOME", tmp)
94+
95+
ownerDir := filepath.Join(tmp, ".odek", "plans", "chat111")
96+
if err := os.MkdirAll(ownerDir, 0755); err != nil {
97+
t.Fatalf("MkdirAll: %v", err)
98+
}
99+
os.WriteFile(filepath.Join(ownerDir, "secret.md"), []byte("secret plan"), 0644)
100+
101+
_, _, err := ReadPlan(222, "secret")
102+
if err == nil {
103+
t.Fatal("ReadPlan should reject a plan belonging to a different chat")
104+
}
105+
if !strings.Contains(err.Error(), "no plan") && !strings.Contains(err.Error(), "no plans found") {
106+
t.Errorf("error = %q, want plan-not-found", err)
107+
}
108+
}
109+
110+
func TestDeletePlan_ChatScoped(t *testing.T) {
111+
tmp := t.TempDir()
112+
t.Setenv("HOME", tmp)
113+
114+
ownerDir := filepath.Join(tmp, ".odek", "plans", "chat111")
115+
if err := os.MkdirAll(ownerDir, 0755); err != nil {
116+
t.Fatalf("MkdirAll: %v", err)
117+
}
118+
os.WriteFile(filepath.Join(ownerDir, "secret.md"), []byte("secret plan"), 0644)
119+
120+
_, err := DeletePlan(222, "secret")
121+
if err == nil {
122+
t.Fatal("DeletePlan should reject a plan belonging to a different chat")
123+
}
124+
if !strings.Contains(err.Error(), "no plan") && !strings.Contains(err.Error(), "no plans found") {
125+
t.Errorf("error = %q, want plan-not-found", err)
126+
}
127+
128+
// Ensure the owner's file is still there.
129+
if _, err := os.Stat(filepath.Join(ownerDir, "secret.md")); os.IsNotExist(err) {
130+
t.Error("owner's plan file was deleted by cross-chat call")
131+
}
132+
}

0 commit comments

Comments
 (0)