Skip to content

Commit ef6360e

Browse files
committed
docs+test(telegram): expand docs and coverage for #62, #75, #76
- docs/TELEGRAM.md: document plan size cap, O_NOFOLLOW media verification, and 0600 log file permissions - internal/telegram/plan_test.go: add at-cap, MostRecentPlan oversize, and ListPlans preview tests - internal/telegram/media_path_test.go: add tilde expansion and symlink-to- allowed-file rejection tests - internal/telegram/log_test.go: add existing-file permission hardening test - full suite and race detector pass
1 parent 56f065d commit ef6360e

4 files changed

Lines changed: 154 additions & 1 deletion

File tree

docs/TELEGRAM.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ for {
166166
}
167167
```
168168

169+
> **Log file permissions.** `telegram.NewFileLogger` creates log files with `0600` permissions (owner read/write only). Existing files created by earlier versions are also hardened to `0600` on open, so chat IDs and task snippets are not world-readable.
170+
169171
## Message Handler (`handler.go`)
170172

171173
The `Handler` struct routes incoming updates to the appropriate callback based on message type. It is the bridge between raw Telegram updates and the agent.
@@ -212,7 +214,7 @@ The agent can send files back to the chat either by emitting a `MEDIA:` prefix i
212214

213215
- Allowed directories: current working directory, `~/.odek/media/`, and the system temporary directory.
214216
- The path is resolved to an absolute, cleaned form and checked against the allowlist.
215-
- Symlinks are rejected: the final component is verified with `os.Lstat` and the resolved path must not escape the allowlist.
217+
- 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.
216218
- Files outside the allowlist (e.g. `/home/user/.ssh/id_rsa`) are refused, closing prompt-injection-driven exfiltration.
217219

218220
## Slash Commands (`commands.go`)
@@ -278,6 +280,13 @@ The `clarifyChannels` sync.Map provides per-chat channels for the agent to ask t
278280

279281
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.
280282

283+
### Size Cap
284+
285+
To prevent a prompt-injected agent from causing an out-of-memory condition, plan files are subject to a **1 MiB** size cap:
286+
287+
- `ReadPlan` and `MostRecentPlan` reject files larger than 1 MiB.
288+
- `ListPlans` only reads the first 8 KiB of each plan to generate the preview, so listing plans never loads multi-megabyte files.
289+
281290
### Key Functions
282291

283292
| Function | Purpose |

internal/telegram/log_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,28 @@ func TestNewFileLogger_filePath(t *testing.T) {
6161
}
6262
}
6363

64+
func TestNewFileLogger_hardensExistingFile(t *testing.T) {
65+
dir := t.TempDir()
66+
path := filepath.Join(dir, "existing.log")
67+
68+
// Simulate an existing world-readable log file from an earlier version.
69+
if err := os.WriteFile(path, []byte("old log\n"), 0644); err != nil {
70+
t.Fatalf("create existing file: %v", err)
71+
}
72+
73+
l := NewFileLogger(LogInfo, path)
74+
fl := l.(*fileLogger)
75+
fl.file.Close()
76+
77+
info, err := os.Stat(path)
78+
if err != nil {
79+
t.Fatalf("stat log file: %v", err)
80+
}
81+
if info.Mode().Perm()&0077 != 0 {
82+
t.Errorf("existing log file permissions = %04o, want no group/other bits", info.Mode().Perm())
83+
}
84+
}
85+
6486
func TestNewFileLogger_invalidPath(t *testing.T) {
6587
// An invalid path should fall back to stderr without panicking.
6688
l := NewFileLogger(LogDebug, "/nonexistent/dir/log.log")

internal/telegram/media_path_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,3 +257,61 @@ func TestResolveMediaPath_RelativeInCWD(t *testing.T) {
257257
t.Errorf("expected absolute resolved path, got %q", resolved)
258258
}
259259
}
260+
261+
// TestResolveMediaPath_TildeExpansion verifies that a leading ~ is expanded to
262+
// the home directory and resolved inside the odek media dir.
263+
func TestResolveMediaPath_TildeExpansion(t *testing.T) {
264+
setupMediaPathTest(t)
265+
266+
dir, err := MediaDir()
267+
if err != nil {
268+
t.Fatal(err)
269+
}
270+
f := filepath.Join(dir, "tilde-media.txt")
271+
if err := os.WriteFile(f, []byte("x"), 0644); err != nil {
272+
t.Fatal(err)
273+
}
274+
275+
resolved, err := ResolveMediaPath("~/.odek/media/tilde-media.txt")
276+
if err != nil {
277+
t.Fatalf("ResolveMediaPath with tilde: %v", err)
278+
}
279+
if !filepath.IsAbs(resolved) {
280+
t.Errorf("expected absolute resolved path, got %q", resolved)
281+
}
282+
}
283+
284+
// TestResolveMediaPath_RejectsSymlinkToAllowedFile verifies that the final
285+
// component being a symlink is rejected even when the symlink target is inside
286+
// the allowlist. This exercises the O_NOFOLLOW / lstat check.
287+
func TestResolveMediaPath_RejectsSymlinkToAllowedFile(t *testing.T) {
288+
if runtime.GOOS == "windows" {
289+
t.Skip("symlink tests skipped on windows")
290+
}
291+
292+
setupMediaPathTest(t)
293+
294+
cwd, err := os.Getwd()
295+
if err != nil {
296+
t.Fatal(err)
297+
}
298+
299+
// Both target and link are inside the allowed cwd.
300+
target := filepath.Join(cwd, "allowed-target.txt")
301+
if err := os.WriteFile(target, []byte("x"), 0644); err != nil {
302+
t.Fatal(err)
303+
}
304+
link := filepath.Join(cwd, "allowed-link.txt")
305+
if err := os.Symlink(target, link); err != nil {
306+
t.Fatal(err)
307+
}
308+
t.Cleanup(func() { _ = os.Remove(link) })
309+
310+
_, err = ResolveMediaPath(link)
311+
if err == nil {
312+
t.Fatal("expected rejection for symlink final component")
313+
}
314+
if !strings.Contains(err.Error(), "symlinks are not allowed") {
315+
t.Errorf("expected 'symlinks are not allowed' in error, got: %v", err)
316+
}
317+
}

internal/telegram/plan_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,70 @@ func TestReadPlan_OversizeRejected(t *testing.T) {
199199
}
200200
}
201201

202+
func TestReadPlan_AtSizeCapAllowed(t *testing.T) {
203+
tmp := t.TempDir()
204+
t.Setenv("HOME", tmp)
205+
206+
dir := filepath.Join(tmp, ".odek", "plans")
207+
os.MkdirAll(dir, 0755)
208+
path := filepath.Join(dir, "max-plan.md")
209+
content := strings.Repeat("x", maxPlanBytes)
210+
os.WriteFile(path, []byte(content), 0644)
211+
212+
slug, got, err := ReadPlan("max-plan")
213+
if err != nil {
214+
t.Fatalf("ReadPlan at size cap: %v", err)
215+
}
216+
if slug != "max-plan" {
217+
t.Errorf("slug = %q, want max-plan", slug)
218+
}
219+
if got != content {
220+
t.Errorf("content length = %d, want %d", len(got), len(content))
221+
}
222+
}
223+
224+
func TestMostRecentPlan_OversizeRejected(t *testing.T) {
225+
tmp := t.TempDir()
226+
t.Setenv("HOME", tmp)
227+
228+
dir := filepath.Join(tmp, ".odek", "plans")
229+
os.MkdirAll(dir, 0755)
230+
path := filepath.Join(dir, "huge-plan.md")
231+
os.WriteFile(path, []byte(strings.Repeat("x", maxPlanBytes+1)), 0644)
232+
233+
_, _, err := MostRecentPlan()
234+
if err == nil {
235+
t.Fatal("expected error for oversized most-recent plan")
236+
}
237+
if !strings.Contains(err.Error(), "too large") {
238+
t.Errorf("error = %q, want 'too large'", err)
239+
}
240+
}
241+
242+
func TestListPlans_OversizePreview(t *testing.T) {
243+
tmp := t.TempDir()
244+
t.Setenv("HOME", tmp)
245+
246+
dir := filepath.Join(tmp, ".odek", "plans")
247+
os.MkdirAll(dir, 0755)
248+
path := filepath.Join(dir, "huge-plan.md")
249+
os.WriteFile(path, []byte(strings.Repeat("x", maxPlanBytes+1)), 0644)
250+
251+
infos, err := ListPlans(0)
252+
if err != nil {
253+
t.Fatalf("ListPlans: %v", err)
254+
}
255+
if len(infos) != 1 {
256+
t.Fatalf("len(infos) = %d, want 1", len(infos))
257+
}
258+
if infos[0].Slug != "huge-plan" {
259+
t.Errorf("slug = %q, want huge-plan", infos[0].Slug)
260+
}
261+
if len(infos[0].Preview) > 100 {
262+
t.Errorf("preview length = %d, expected short preview", len(infos[0].Preview))
263+
}
264+
}
265+
202266
func TestDeletePlan(t *testing.T) {
203267
tmp := t.TempDir()
204268
t.Setenv("HOME", tmp)

0 commit comments

Comments
 (0)