diff --git a/pkg/memory/jsonl.go b/pkg/memory/jsonl.go index e12e2c5ab3..afe374166c 100644 --- a/pkg/memory/jsonl.go +++ b/pkg/memory/jsonl.go @@ -86,14 +86,14 @@ func (s *JSONLStore) metaPath(key string) string { // sanitizeKey converts a session key to a safe filename component. // Mirrors pkg/session.sanitizeFilename so that migration paths match. -// -// Note: this is a lossy mapping — "telegram:123" and "telegram_123" -// both produce the same filename. This is an intentional tradeoff: -// keys with colons (e.g. from channels) are by far the common case, -// and a bidirectional encoding (like URL-encoding) would complicate -// file listings and debugging. +// Replaces ':' with '_' (session key separator) and '/' and '\' with '_' +// so composite IDs (e.g. Telegram forum "chatID/threadID", Slack "channel/thread_ts") +// do not create subdirectories or break on Windows. func sanitizeKey(key string) string { - return strings.ReplaceAll(key, ":", "_") + s := strings.ReplaceAll(key, ":", "_") + s = strings.ReplaceAll(s, "/", "_") + s = strings.ReplaceAll(s, "\\", "_") + return s } // readMeta loads the metadata file for a session. diff --git a/pkg/session/manager.go b/pkg/session/manager.go index 07f981df17..a31dbd55c9 100644 --- a/pkg/session/manager.go +++ b/pkg/session/manager.go @@ -146,12 +146,15 @@ func (sm *SessionManager) TruncateHistory(key string, keepLast int) { } // sanitizeFilename converts a session key into a cross-platform safe filename. -// Session keys use "channel:chatID" (e.g. "telegram:123456") but ':' is the -// volume separator on Windows, so filepath.Base would misinterpret the key. -// We replace it with '_'. The original key is preserved inside the JSON file, -// so loadSessions still maps back to the right in-memory key. +// Replaces ':' with '_' (session key separator) and '/' and '\' with '_' so +// composite IDs (e.g. Telegram forum "chatID/threadID") do not create +// subdirectories or break on Windows. The original key is preserved inside +// the JSON file, so loadSessions still maps back to the right in-memory key. func sanitizeFilename(key string) string { - return strings.ReplaceAll(key, ":", "_") + s := strings.ReplaceAll(key, ":", "_") + s = strings.ReplaceAll(s, "/", "_") + s = strings.ReplaceAll(s, "\\", "_") + return s } func (sm *SessionManager) Save(key string) error { @@ -162,10 +165,9 @@ func (sm *SessionManager) Save(key string) error { filename := sanitizeFilename(key) // filepath.IsLocal rejects empty names, "..", absolute paths, and - // OS-reserved device names (NUL, COM1 … on Windows). - // The extra checks reject "." and any directory separators so that - // the session file is always written directly inside sm.storage. - if filename == "." || !filepath.IsLocal(filename) || strings.ContainsAny(filename, `/\`) { + // OS-reserved device names (NUL, COM1 … on Windows). sanitizeFilename + // already replaced '/' and '\' with '_', so no subdirs are created. + if filename == "." || !filepath.IsLocal(filename) { return os.ErrInvalid } diff --git a/pkg/session/manager_test.go b/pkg/session/manager_test.go index 5ef5f4349c..bc56159668 100644 --- a/pkg/session/manager_test.go +++ b/pkg/session/manager_test.go @@ -17,6 +17,7 @@ func TestSanitizeFilename(t *testing.T) { {"slack:C01234", "slack_C01234"}, {"no-colons-here", "no-colons-here"}, {"multiple:colons:here", "multiple_colons_here"}, + {"agent:main:telegram:group:-1003822706455/12", "agent_main_telegram_group_-1003822706455_12"}, } for _, tt := range tests { @@ -64,11 +65,21 @@ func TestSave_RejectsPathTraversal(t *testing.T) { tmpDir := t.TempDir() sm := NewSessionManager(tmpDir) - badKeys := []string{"", ".", "..", "foo/bar", "foo\\bar"} + // Invalid names that must still be rejected. + badKeys := []string{"", ".", ".."} for _, key := range badKeys { sm.GetOrCreate(key) if err := sm.Save(key); err == nil { t.Errorf("Save(%q) should have failed but didn't", key) } } + + // Keys containing path separators are sanitized (no subdirs created). + sm.GetOrCreate("foo/bar") + if err := sm.Save("foo/bar"); err != nil { + t.Fatalf("Save(\"foo/bar\") after sanitize should succeed: %v", err) + } + if _, err := os.Stat(filepath.Join(tmpDir, "foo_bar.json")); os.IsNotExist(err) { + t.Errorf("expected foo_bar.json in storage (sanitized from foo/bar)") + } }