Skip to content

Commit b7719a6

Browse files
nunocoracaoclaude
andcommitted
Address review comments: path traversal, SQL escaping, migration errors, config collisions
- Reject ".." in configNameFromSource to prevent directory traversal - Escape SQL wildcards (%, _, \) in LIKE patterns with ESCAPE clause - Only ignore "duplicate column name" errors during migration, surface real failures - Append short SHA-256 hash of full source path to config name to prevent collisions between identically named configs in different directories Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c2520db commit b7719a6

File tree

2 files changed

+21
-8
lines changed

2 files changed

+21
-8
lines changed

pkg/memory/database/sqlite/sqlite.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ func NewMemoryDatabase(path string) (database.Database, error) {
2929
}
3030

3131
// Add category column if it doesn't exist (transparent migration)
32-
_, _ = db.ExecContext(context.Background(), "ALTER TABLE memories ADD COLUMN category TEXT DEFAULT ''")
32+
if _, err := db.ExecContext(context.Background(), "ALTER TABLE memories ADD COLUMN category TEXT DEFAULT ''"); err != nil {
33+
if !strings.Contains(err.Error(), "duplicate column name") {
34+
db.Close()
35+
return nil, fmt.Errorf("memory database migration failed: %w", err)
36+
}
37+
}
3338

3439
return &MemoryDatabase{db: db}, nil
3540
}
@@ -79,8 +84,11 @@ func (m *MemoryDatabase) SearchMemories(ctx context.Context, query, category str
7984
if query != "" {
8085
words := strings.Fields(query)
8186
for _, word := range words {
82-
conditions = append(conditions, "LOWER(memory) LIKE LOWER(?)")
83-
args = append(args, "%"+word+"%")
87+
conditions = append(conditions, "LOWER(memory) LIKE LOWER(?) ESCAPE '\\'")
88+
escaped := strings.ReplaceAll(word, `\`, `\\`)
89+
escaped = strings.ReplaceAll(escaped, `%`, `\%`)
90+
escaped = strings.ReplaceAll(escaped, `_`, `\_`)
91+
args = append(args, "%"+escaped+"%")
8492
}
8593
}
8694

pkg/teamloader/teamloader.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package teamloader
33
import (
44
"cmp"
55
"context"
6+
"crypto/sha256"
7+
"encoding/hex"
68
"errors"
79
"fmt"
810
"log/slog"
@@ -483,18 +485,21 @@ func getToolsForAgent(ctx context.Context, a *latest.AgentConfig, parentDir stri
483485
}
484486

485487
// configNameFromSource extracts a clean config name from a source name.
486-
// For file sources this strips the directory and extension (e.g. "/path/to/memory_agent.yaml" -> "memory_agent").
487-
// For non-file sources (OCI refs, URLs) it returns the full name sanitised for use as a directory.
488+
// The result is "<basename>-<hash>" where basename comes from the file name
489+
// (e.g. "memory_agent" from "/path/to/memory_agent.yaml") and hash is a short
490+
// SHA-256 of the full source name to prevent collisions between identically
491+
// named configs in different directories.
488492
func configNameFromSource(sourceName string) string {
489493
base := filepath.Base(sourceName)
490494
ext := filepath.Ext(base)
491495
if ext != "" {
492496
base = base[:len(base)-len(ext)]
493497
}
494-
if base == "" || base == "." {
495-
return "default"
498+
if base == "" || base == "." || base == ".." {
499+
base = "default"
496500
}
497-
return base
501+
h := sha256.Sum256([]byte(sourceName))
502+
return base + "-" + hex.EncodeToString(h[:4])
498503
}
499504

500505
// resolveAgentRefs resolves a list of agent references to agent instances.

0 commit comments

Comments
 (0)