Skip to content

Commit 7020171

Browse files
committed
feat(memory): durable note store and memory tools
Split 2/3 of #829, stacked on the pathjail primitive from #891 (1/3). - internal/memory: a durable note store confined to a pathjail handle, so every read, write, rename and delete stays on the confined handle rather than being re-resolved by pathname. Writes publish through an O_EXCL temporary file. - internal/tools: memory (read/list), memory_write (save), memory_forget (delete). Reads are confined by the same rule as writes: RefuseReparse now runs on the read path too, closing a link at the note position that resolves back inside the root — os.Root permits that case, so it was served. The guard is reparse-point based, not identity based, and says so: a hard link carries no reparse bit and still reads through. The size ceiling holds on the way out as well as in. A note that arrived by hand or through a clone was previously read whole however large, and List did that for every note in the store. Scope is resolved in ONE place (ResolveScopes). The two paths disagreed: an unrecognised spelling widened a named read to both stores while the listing ignored a valid scope and always read both. Operational failures reach the caller instead of being reported as absence. Only ErrNotFound is a miss; a refused link, an oversized note or a permission error is an error, because "no such note" is what makes the model write it again. Deletion is its own tool. memory_write's approval says it saves a note, and an omitted or whitespace-only content used to fall through to Forget and destroy one under that sentence — with "always allow" making it unattended. content is now required and non-empty, and memory_forget carries its own disclosure. The local store makes itself private on first write rather than relying on the workspace's .gitignore, which would protect one repository rather than every one the tool runs in. Origin-Session: local-7a41bc | Claude Code | 1 prompt
1 parent f952915 commit 7020171

5 files changed

Lines changed: 1470 additions & 0 deletions

File tree

‎internal/memory/escape_test.go‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
package memory
2+
3+
import (
4+
"os"
5+
"os/exec"
6+
"path/filepath"
7+
"runtime"
8+
"strings"
9+
"testing"
10+
)
11+
12+
// linkDir uses a junction on Windows: it needs no privilege, unlike a symlink,
13+
// so it is both the reachable attack and the only one testable on an ordinary
14+
// Windows account.
15+
func linkDir(t *testing.T, target, link string) {
16+
t.Helper()
17+
if runtime.GOOS == "windows" {
18+
if out, err := exec.Command("cmd", "/c", "mklink", "/J", link, target).CombinedOutput(); err != nil {
19+
t.Skipf("cannot create a junction: %v %s", err, out)
20+
}
21+
return
22+
}
23+
if err := os.Symlink(target, link); err != nil {
24+
t.Skipf("cannot create a symlink: %v", err)
25+
}
26+
}
27+
28+
// The store used to check only its own directory and file, so a link at the
29+
// ANCESTOR .zero turned every operation into one aimed outside the workspace:
30+
// Write and Forget were arbitrary write and delete, and Read was an arbitrary
31+
// read in a tool the model can call by name.
32+
//
33+
// All three are asserted, because fixing one and leaving the others is exactly
34+
// what the original guard did.
35+
func TestAnAncestorLinkCannotTakeTheStoreOutOfTheWorkspace(t *testing.T) {
36+
base := t.TempDir()
37+
outside := filepath.Join(base, "outside")
38+
workspace := filepath.Join(base, "workspace")
39+
for _, dir := range []string{outside, workspace} {
40+
if err := os.MkdirAll(dir, 0o700); err != nil {
41+
t.Fatal(err)
42+
}
43+
}
44+
// A note already sitting in the external directory, so a successful read
45+
// would be visible rather than merely "no error".
46+
if err := os.MkdirAll(filepath.Join(outside, "memory"), 0o700); err != nil {
47+
t.Fatal(err)
48+
}
49+
secret := filepath.Join(outside, "memory", "secret.md")
50+
if err := os.WriteFile(secret, []byte("do not read me"), 0o600); err != nil {
51+
t.Fatal(err)
52+
}
53+
linkDir(t, outside, filepath.Join(workspace, ".zero"))
54+
55+
paths := DefaultPaths(workspace)
56+
57+
if _, err := Write(paths, ScopeProject, "escaped", "d", "b"); err == nil {
58+
t.Error("Write went through the linked ancestor")
59+
}
60+
if _, err := os.Stat(filepath.Join(outside, "memory", "escaped.md")); !os.IsNotExist(err) {
61+
t.Errorf("a note was written outside the workspace, stat error = %v", err)
62+
}
63+
if _, err := Read(paths, ScopeProject, "secret"); err == nil {
64+
t.Error("Read returned a note from outside the workspace")
65+
}
66+
if err := Forget(paths, ScopeProject, "secret"); err == nil {
67+
t.Error("Forget accepted a target outside the workspace")
68+
}
69+
if _, err := os.Stat(secret); err != nil {
70+
t.Errorf("Forget deleted a file outside the workspace: %v", err)
71+
}
72+
notes, listErr := List(paths)
73+
if len(notes) != 0 {
74+
t.Errorf("List surfaced %d note(s) from outside the workspace", len(notes))
75+
}
76+
// The refusal is REPORTED, not swallowed. A store that cannot be opened
77+
// because a link redirects it out of the workspace is an operational failure;
78+
// returning an empty list for it would tell the caller there are no notes,
79+
// which is how a redirected store looks exactly like an empty one.
80+
if listErr == nil {
81+
t.Error("List reported no problem for a store redirected outside the workspace")
82+
}
83+
}
84+
85+
// The ordinary path still works, or the test above would pass against a store
86+
// that refused everything.
87+
func TestAnOrdinaryWorkspaceStoreStillRoundTrips(t *testing.T) {
88+
workspace := t.TempDir()
89+
paths := DefaultPaths(workspace)
90+
if _, err := Write(paths, ScopeProject, "note", "a summary", "the body"); err != nil {
91+
t.Fatalf("Write: %v", err)
92+
}
93+
note, err := Read(paths, ScopeProject, "note")
94+
if err != nil {
95+
t.Fatalf("Read: %v", err)
96+
}
97+
if note.Description != "a summary" || strings.TrimSpace(note.Body) != "the body" {
98+
t.Errorf("round trip lost content: %+v", note)
99+
}
100+
notes, listErr := List(paths)
101+
if listErr != nil {
102+
t.Fatal(listErr)
103+
}
104+
if len(notes) != 1 {
105+
t.Errorf("List returned %d notes, want 1", len(notes))
106+
}
107+
if err := Forget(paths, ScopeProject, "note"); err != nil {
108+
t.Fatalf("Forget: %v", err)
109+
}
110+
if _, err := Read(paths, ScopeProject, "note"); err == nil {
111+
t.Error("the note survived Forget")
112+
}
113+
}

0 commit comments

Comments
 (0)