Add search, update, categories, and default path to memory tool#1971
Add search, update, categories, and default path to memory tool#1971nunocoracao wants to merge 4 commits intomainfrom
Conversation
- Add search_memories tool (keyword search with AND logic, optional category filter) - Add update_memory tool (edit existing memories by ID) - Add optional category field to memories for organization - Default memory path to ~/.cagent/memory/<config-name>/memory.db (per-user, per-agent-config) - Make path optional in config validation (was required, now defaults) - Add configName parameter to ToolsetCreator for per-config-file identity - SQLite migration adds category column transparently to existing databases - Expand tool instructions with guidance on search vs get_all, update vs create Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
Assessment: 🔴 CRITICAL
Found 2 issues in the changed code that should be addressed:
- 1 HIGH severity issue: Path traversal vulnerability
- 1 MEDIUM severity issue: SQL wildcard escaping bug
Summary
This PR adds useful search, update, and category features to the memory tool. However, there are two bugs in the changed code:
-
Path traversal vulnerability in
configNameFromSource()- The function doesn't prevent".."as a config name, which could allow directory traversal when constructing the memory DB path. -
SQL wildcard escaping in
SearchMemories()- User input containing SQL wildcards (%,_) is not escaped before being used in LIKE patterns, preventing users from searching for strings containing these characters.
Both issues are in newly added code and should be fixed before merging.
This review was generated automatically. It only comments on code added in this PR.
There was a problem hiding this comment.
Review Summary
Assessment: 🟡 NEEDS ATTENTION
Found 2 medium-severity issues in the changed code that should be addressed:
- Database migration error handling: Silent failures during schema migration could leave the database in an inconsistent state
- Memory database collision: Multiple agent configs with similar names could unintentionally share the same memory database
Both issues are in new code added by this PR. The features (search, update, categories, default paths) are well-designed overall, but these edge cases should be handled to prevent potential data integrity and isolation issues.
Findings: 2 medium severity issues
Review performed by cagent - automated code review bot
…rs, 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>
|
@nunocoracao the v* packages are frozen. They shouldn't be changed. Only the |
Only pkg/config/latest/validate.go retains the optional path change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Good catch - reverted the v2-v5 changes. Only |
2 participants
Summary
search_memoriestool with keyword search (multi-word AND logic, case-insensitive) and optional category filterupdate_memorytool to edit existing memories by ID (no more delete + re-add)categoryfield on memories for organization (e.g.preference,fact,project)~/.cagent/memory/<config-name>/memory.db- per-user, per-agent-config. Path is no longer required in YAML configconfigNameparameter derived from the agent config file name, used by memory tool for per-config storagecategorycolumn added automaticallyAll existing tools (
add_memory,get_memories,delete_memory) are unchanged. Existing databases and configs work without modification.Test plan
go build ./...passesgo test ./pkg/memory/... ./pkg/tools/builtin/... ./pkg/teamloader/... ./pkg/config/... ./pkg/creator/... ./pkg/acp/...all passdocker-agent runwith- type: memory(no path) creates DB at~/.cagent/memory/<config>/memory.dbGenerated with Claude Code