mirror of
https://github.com/supermemoryai/supermemory.git
synced 2026-10-27 06:15:15 +00:00
fix(tools): refuse claude-memory str_replace when old_str is not unique
str_replace replaced the first match and reported success when old_str appeared several times, so Claude could silently edit the wrong part of a memory file. Anthropic's memory tool requires old_str to be unique and returns an error listing the matching lines; do the same and leave the file untouched.
This commit is contained in:
parent
f81fad00e4
commit
7c1be91512
2 changed files with 68 additions and 0 deletions
|
|
@ -292,6 +292,57 @@ describe("ClaudeMemoryTool str_replace replacement literalness", () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe("ClaudeMemoryTool str_replace uniqueness", () => {
|
||||
let tool: ClaudeMemoryTool
|
||||
|
||||
beforeEach(() => {
|
||||
documentsGetMock.mockReset()
|
||||
updateMock.mockReset().mockResolvedValue({ id: "doc", status: "queued" })
|
||||
mockDocument("status: todo\nnotes\nstatus: todo")
|
||||
tool = new ClaudeMemoryTool("test-api-key")
|
||||
})
|
||||
|
||||
it("refuses to replace an old_str that occurs more than once", async () => {
|
||||
const result = await tool.handleCommand({
|
||||
command: "str_replace",
|
||||
path: FILE_PATH,
|
||||
old_str: "status: todo",
|
||||
new_str: "status: done",
|
||||
})
|
||||
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("Multiple occurrences")
|
||||
expect(result.error).toContain("lines: 1, 3")
|
||||
expect(updateMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("counts overlapping occurrences", async () => {
|
||||
mockDocument("aaa")
|
||||
const result = await tool.handleCommand({
|
||||
command: "str_replace",
|
||||
path: FILE_PATH,
|
||||
old_str: "aa",
|
||||
new_str: "b",
|
||||
})
|
||||
|
||||
expect(result.success).toBe(false)
|
||||
expect(updateMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("still replaces a unique old_str", async () => {
|
||||
const result = await tool.handleCommand({
|
||||
command: "str_replace",
|
||||
path: FILE_PATH,
|
||||
old_str: "notes",
|
||||
new_str: "updated notes",
|
||||
})
|
||||
|
||||
expect(result.success).toBe(true)
|
||||
const stored = updateMock.mock.calls[0]?.[2]?.content as string
|
||||
expect(stored).toBe("status: todo\nupdated notes\nstatus: todo")
|
||||
})
|
||||
})
|
||||
|
||||
describe("ClaudeMemoryTool path traversal", () => {
|
||||
let tool: ClaudeMemoryTool
|
||||
|
||||
|
|
|
|||
|
|
@ -389,6 +389,23 @@ export class ClaudeMemoryTool {
|
|||
}
|
||||
}
|
||||
|
||||
// old_str must be unique, like Anthropic's memory tool: replacing
|
||||
// only the first of several matches would silently edit the wrong spot.
|
||||
const matchingLines: number[] = []
|
||||
for (
|
||||
let index = originalContent.indexOf(oldStr);
|
||||
index !== -1;
|
||||
index = originalContent.indexOf(oldStr, index + 1)
|
||||
) {
|
||||
matchingLines.push(originalContent.slice(0, index).split("\n").length)
|
||||
}
|
||||
if (matchingLines.length > 1) {
|
||||
return {
|
||||
success: false,
|
||||
error: `No replacement was performed. Multiple occurrences of old_str "${oldStr}" in lines: ${matchingLines.join(", ")}. Please ensure it is unique`,
|
||||
}
|
||||
}
|
||||
|
||||
// Replace the string. The function replacer keeps `$` sequences
|
||||
// in the replacement literal — a bare string here would expand
|
||||
// patterns like $&, $', and $` and silently corrupt the file.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue