Merge pull request #1330 from statxc/fix/session-key-sanitize-slash
fix(session): sanitize '/' and '\' in session keys so forum topic key…
This commit is contained in:
commit
755fa32336
3 changed files with 30 additions and 17 deletions
|
|
@ -86,14 +86,14 @@ func (s *JSONLStore) metaPath(key string) string {
|
||||||
|
|
||||||
// sanitizeKey converts a session key to a safe filename component.
|
// sanitizeKey converts a session key to a safe filename component.
|
||||||
// Mirrors pkg/session.sanitizeFilename so that migration paths match.
|
// Mirrors pkg/session.sanitizeFilename so that migration paths match.
|
||||||
//
|
// Replaces ':' with '_' (session key separator) and '/' and '\' with '_'
|
||||||
// Note: this is a lossy mapping — "telegram:123" and "telegram_123"
|
// so composite IDs (e.g. Telegram forum "chatID/threadID", Slack "channel/thread_ts")
|
||||||
// both produce the same filename. This is an intentional tradeoff:
|
// do not create subdirectories or break on Windows.
|
||||||
// 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.
|
|
||||||
func sanitizeKey(key string) string {
|
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.
|
// readMeta loads the metadata file for a session.
|
||||||
|
|
|
||||||
|
|
@ -146,12 +146,15 @@ func (sm *SessionManager) TruncateHistory(key string, keepLast int) {
|
||||||
}
|
}
|
||||||
|
|
||||||
// sanitizeFilename converts a session key into a cross-platform safe filename.
|
// sanitizeFilename converts a session key into a cross-platform safe filename.
|
||||||
// Session keys use "channel:chatID" (e.g. "telegram:123456") but ':' is the
|
// Replaces ':' with '_' (session key separator) and '/' and '\' with '_' so
|
||||||
// volume separator on Windows, so filepath.Base would misinterpret the key.
|
// composite IDs (e.g. Telegram forum "chatID/threadID") do not create
|
||||||
// We replace it with '_'. The original key is preserved inside the JSON file,
|
// subdirectories or break on Windows. The original key is preserved inside
|
||||||
// so loadSessions still maps back to the right in-memory key.
|
// the JSON file, so loadSessions still maps back to the right in-memory key.
|
||||||
func sanitizeFilename(key string) string {
|
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 {
|
func (sm *SessionManager) Save(key string) error {
|
||||||
|
|
@ -162,10 +165,9 @@ func (sm *SessionManager) Save(key string) error {
|
||||||
filename := sanitizeFilename(key)
|
filename := sanitizeFilename(key)
|
||||||
|
|
||||||
// filepath.IsLocal rejects empty names, "..", absolute paths, and
|
// filepath.IsLocal rejects empty names, "..", absolute paths, and
|
||||||
// OS-reserved device names (NUL, COM1 … on Windows).
|
// OS-reserved device names (NUL, COM1 … on Windows). sanitizeFilename
|
||||||
// The extra checks reject "." and any directory separators so that
|
// already replaced '/' and '\' with '_', so no subdirs are created.
|
||||||
// the session file is always written directly inside sm.storage.
|
if filename == "." || !filepath.IsLocal(filename) {
|
||||||
if filename == "." || !filepath.IsLocal(filename) || strings.ContainsAny(filename, `/\`) {
|
|
||||||
return os.ErrInvalid
|
return os.ErrInvalid
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,6 +17,7 @@ func TestSanitizeFilename(t *testing.T) {
|
||||||
{"slack:C01234", "slack_C01234"},
|
{"slack:C01234", "slack_C01234"},
|
||||||
{"no-colons-here", "no-colons-here"},
|
{"no-colons-here", "no-colons-here"},
|
||||||
{"multiple:colons:here", "multiple_colons_here"},
|
{"multiple:colons:here", "multiple_colons_here"},
|
||||||
|
{"agent:main:telegram:group:-1003822706455/12", "agent_main_telegram_group_-1003822706455_12"},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
|
|
@ -64,11 +65,21 @@ func TestSave_RejectsPathTraversal(t *testing.T) {
|
||||||
tmpDir := t.TempDir()
|
tmpDir := t.TempDir()
|
||||||
sm := NewSessionManager(tmpDir)
|
sm := NewSessionManager(tmpDir)
|
||||||
|
|
||||||
badKeys := []string{"", ".", "..", "foo/bar", "foo\\bar"}
|
// Invalid names that must still be rejected.
|
||||||
|
badKeys := []string{"", ".", ".."}
|
||||||
for _, key := range badKeys {
|
for _, key := range badKeys {
|
||||||
sm.GetOrCreate(key)
|
sm.GetOrCreate(key)
|
||||||
if err := sm.Save(key); err == nil {
|
if err := sm.Save(key); err == nil {
|
||||||
t.Errorf("Save(%q) should have failed but didn't", key)
|
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)")
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue