Skip to content

Commit cf282dd

Browse files
fix(cellstore): never capture a folder that holds the home folder (#1976)
* test(cellstore): a folder holding home is never planned at startup Red: a seat opened in the home folder (or its parent, or a filesystem root) asks the engine for a full capture plan of it. The root case walks the whole disk in the guard before the engine is asked. * fix(cellstore): never capture a folder that holds the home folder A chat opened in the home folder, a folder above it or a filesystem root seals its own folder instead: the conversation is kept, the folder's files are not. Engine.workspace is the one place every seal, rewind and take reads the tree from, so all of them agree. The seat says so once. * docs(changes): a home-folder chat saves the conversation only * test(cellstore): a home chat never captures the sender's home path elsewhere Red: the same chat, on a machine whose home is not the sender's, plans the sender's home path as its project. The syncsetup guard covers the take: B keeps the chat in a folder of its own and A's path is untouched. * fix(cellstore): a home chat never lands in the sender's home path elsewhere A chat that sealed only its own folder because its folder held a home folder records that in .cell/own-folder, which travels with every snapshot. Wherever the chat is continued, a workspace outside the chat's own folder that holds no home folder there is foreign: the engine seals and restores the chat's own folder instead, and the take side never takes the chat in place there. * docs(changes): the own-folder mark travels with a home chat * test(cellstore): the home notice test declares the engine available It seals over a fake transport, so it needs no engine binary. A build with no engine inside (CI's touched rest leg) gave it a plain seat and no notice.
1 parent 6ef871a commit cf282dd

6 files changed

Lines changed: 468 additions & 3 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
kind: fixed
3+
title: A chat opened in the home folder saves the conversation, not every file in the home folder
4+
pr: 1976
5+
surface: [chat, engine]
6+
invalidates:
7+
- "A chat whose folder was the home folder, a folder above it or a filesystem root captured that whole folder: the warm capture asked furrow for a plan of every file, a full copy went into the store and each unsealed plan stayed on disk. That folder is now never captured; the chat seals its own folder instead."
8+
- "Rewind in a chat opened in the home folder restored the folder's files along with the conversation. It now brings back the conversation only, and the chat says so once when it opens."
9+
---
10+
11+
The rule is where the folder is, never what is in it: a folder that is the home
12+
folder, holds it, or is a filesystem root is not a project. Counting files or
13+
bytes to find a folder that is too big would itself be the walk this avoids.
14+
Every seal, rewind and take reads the tree from the same place, so they agree.
15+
The chat records that it saves only its own folder (`.cell/own-folder`), so a
16+
machine where the same path is an ordinary folder, not its home, still seals and
17+
restores only the chat's own folder and never takes the chat in place there.
18+
19+
On a copy of a real profile used as the home folder, each launch there wrote
20+
about 9 MB of capture plan and added 2.6 to 3.1 GB to the store before this
21+
change, and wrote no plan and under 1 MB after. Time to the first frame did not
22+
move (about 0.45 s), since the capture was already off the paint path.

‎internal/cellstore/engine.go‎

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"os"
99
"path/filepath"
10+
"strings"
1011
"time"
1112

1213
"github.com/Agent-Field/codeaf/internal/cell"
@@ -107,6 +108,9 @@ func (e Engine) Seal(ctx context.Context, c cell.Cell, info TurnInfo) (Sealed, e
107108
if err != nil {
108109
return Sealed{}, fmt.Errorf("seal: encode receipt: %w", err)
109110
}
111+
if err := e.markOwnFolder(c); err != nil {
112+
return Sealed{}, fmt.Errorf("seal: compose: %w", err)
113+
}
110114
if err := e.compose(c, raw, rid, info); err != nil {
111115
return Sealed{}, fmt.Errorf("seal: compose: %w", err)
112116
}
@@ -191,14 +195,99 @@ func (e Engine) tree(c cell.Cell) string {
191195
}
192196

193197
// workspace is the folder the tools of c work in, "" when c's own folder is
194-
// what is sealed.
198+
// what is sealed. A folder that holds the home folder is never sealed (see
199+
// [holdsHome]): its chat seals its own folder, so the conversation is kept and
200+
// the folder's files are not.
195201
func (e Engine) workspace(c cell.Cell) string {
202+
w := e.named(c)
203+
if w != "" && (holdsHome(w) || ForeignFolder(c.Root, w)) {
204+
return ""
205+
}
206+
return w
207+
}
208+
209+
// named is the workspace the caller names for c, before the rules above.
210+
func (e Engine) named(c cell.Cell) string {
196211
if e.WorkspaceOf != nil {
197212
return e.WorkspaceOf(c)
198213
}
199214
return e.Workspace
200215
}
201216

217+
// OwnFolderPath marks a chat that has sealed only its own folder because the
218+
// folder it was opened in held a home folder. It lives in the cell's .cell/
219+
// directory, so it travels with every snapshot and arrives wherever the chat
220+
// is continued.
221+
const OwnFolderPath = cell.StateDir + "/own-folder"
222+
223+
// ForeignFolder reports whether workspace is a folder the chat at root must
224+
// never be sealed or restored in here: the chat carries only its own folder
225+
// (OwnFolderPath), and workspace is outside that folder and holds no home
226+
// folder here. A folder that holds this machine's home is the chat's working
227+
// folder as on the machine it came from: its tools run there, and only its
228+
// files are never sealed.
229+
//
230+
// THE RECEIVER CANNOT JUDGE THE SENDER'S HOME. A path that was somebody's home
231+
// on the machine that sealed the chat can be an ordinary folder here, holding
232+
// files the snapshot never had, and restoring the chat's own folder over it
233+
// would replace them. [holdsHome] answers for this machine's home; the mark
234+
// answers for the machine the chat came from. A workspace inside the chat's
235+
// own folder (an owned work/) is still the chat's.
236+
func ForeignFolder(root, workspace string) bool {
237+
if workspace == "" || holdsHome(workspace) {
238+
return false
239+
}
240+
if _, err := os.Stat(filepath.Join(root, filepath.FromSlash(OwnFolderPath))); err != nil {
241+
return false
242+
}
243+
rel, err := filepath.Rel(canonicalTree(root), canonicalTree(workspace))
244+
return err != nil || filepath.IsAbs(rel) || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator))
245+
}
246+
247+
// markOwnFolder records, before a seal composes the cell, that this chat's
248+
// folder holds a home folder and so only the chat's own folder is sealed.
249+
func (e Engine) markOwnFolder(c cell.Cell) error {
250+
if w := e.named(c); w == "" || !holdsHome(w) {
251+
return nil
252+
}
253+
path := rel(c, OwnFolderPath)
254+
if _, err := os.Stat(path); err == nil {
255+
return nil
256+
}
257+
if err := writeFile(path, []byte("this chat saves its own folder, never the folder it was opened in\n")); err != nil {
258+
return err
259+
}
260+
commits.mark(c.Root, path, true)
261+
return nil
262+
}
263+
264+
// holdsHome reports whether dir is the home folder, a folder above it, or a
265+
// filesystem root: the places a person opens a general chat, which are not a
266+
// project and whose files are everything they own.
267+
//
268+
// IT IS DECIDED BY WHERE THE FOLDER IS, NEVER BY WHAT IS IN IT. Counting files
269+
// or bytes to find a folder too big to save is itself the walk this rule
270+
// exists to avoid, and a cap would draw its line through somebody's real
271+
// project. A folder that holds the home folder needs no look inside: a home of
272+
// 2.5 GB took a capture that never finished inside SealWithin, and every try
273+
// left its whole plan on disk. Every caller that seals, rewinds or takes a chat
274+
// goes through [Engine.workspace], so all of them agree on the tree.
275+
func holdsHome(dir string) bool {
276+
tree := canonicalTree(dir)
277+
if filepath.Dir(tree) == tree {
278+
return true
279+
}
280+
house, err := os.UserHomeDir()
281+
if err != nil || strings.TrimSpace(house) == "" {
282+
return false
283+
}
284+
rel, err := filepath.Rel(tree, canonicalTree(house))
285+
if err != nil || filepath.IsAbs(rel) {
286+
return false
287+
}
288+
return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
289+
}
290+
202291
// policyDir is where the harness keeps its exclusions: the cell's directory
203292
// that is composed into a workspace, or the cell's own folder when that is
204293
// what is sealed. The engine reads a .furrowpolicy from either.
Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
package cellstore
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"fmt"
7+
"io/fs"
8+
"os"
9+
"path/filepath"
10+
"runtime"
11+
"strings"
12+
"sync"
13+
"testing"
14+
15+
"github.com/Agent-Field/codeaf/internal/cell"
16+
"github.com/Agent-Field/codeaf/internal/executor"
17+
)
18+
19+
// useHome makes dir the home folder this process reports.
20+
func useHome(t *testing.T, dir string) {
21+
t.Helper()
22+
t.Setenv("HOME", dir)
23+
if runtime.GOOS == "windows" {
24+
t.Setenv("USERPROFILE", dir)
25+
}
26+
}
27+
28+
// fillHome writes files spread over dirs folders under root.
29+
func fillHome(t testing.TB, root string, dirs, files int) {
30+
t.Helper()
31+
for d := 0; d < dirs; d++ {
32+
dir := filepath.Join(root, fmt.Sprintf("d%03d", d))
33+
if err := os.MkdirAll(dir, 0o700); err != nil {
34+
t.Fatal(err)
35+
}
36+
for f := 0; f < files; f++ {
37+
if err := os.WriteFile(filepath.Join(dir, fmt.Sprintf("f%04d.txt", f)), []byte("x"), 0o600); err != nil {
38+
t.Fatal(err)
39+
}
40+
}
41+
}
42+
}
43+
44+
// planLedger counts what the engine was asked to plan and what it planned.
45+
type planLedger struct {
46+
mu sync.Mutex
47+
trees []string // the tree of every capture-plan asked for
48+
paths int // the paths every plan answered with, summed
49+
}
50+
51+
func (l *planLedger) over(inner Transport) Transport {
52+
return transportFunc(func(ctx context.Context, target Target, op Op) ([]byte, error) {
53+
out, err := inner.Do(ctx, target, op)
54+
if op.Verb() == "capture-plan" {
55+
var plan capturePlan
56+
_ = json.Unmarshal(out, &plan)
57+
l.mu.Lock()
58+
l.trees = append(l.trees, target.Tree)
59+
l.paths += len(plan.Paths)
60+
l.mu.Unlock()
61+
}
62+
return out, err
63+
})
64+
}
65+
66+
func (l *planLedger) asked(tree string) int {
67+
l.mu.Lock()
68+
defer l.mu.Unlock()
69+
n := 0
70+
for _, asked := range l.trees {
71+
if asked == canonicalTree(tree) {
72+
n++
73+
}
74+
}
75+
return n
76+
}
77+
78+
// fakePlans answers capture-plan with an empty full plan and every other verb
79+
// with a snapshot, so the rule is seen without an engine binary.
80+
func fakePlans() Transport {
81+
return transportFunc(func(_ context.Context, _ Target, op Op) ([]byte, error) {
82+
if op.Verb() == "capture-plan" {
83+
return []byte(`{"token":"t","full":true,"paths":[]}`), nil
84+
}
85+
return []byte(`{"snapshot":"s"}`), nil
86+
})
87+
}
88+
89+
// A FOLDER THAT HOLDS THE HOME FOLDER IS NOT A PROJECT. A chat opened there
90+
// still seals (its own folder), but the engine is never asked to plan the home,
91+
// its parent or a filesystem root, while a project folder beside the home is
92+
// planned exactly as before.
93+
func TestAFolderHoldingHomeIsNeverPlanned(t *testing.T) {
94+
base := canonicalTree(t.TempDir())
95+
house := filepath.Join(base, "people", "someone")
96+
project := filepath.Join(base, "people", "someone-else", "api")
97+
for _, dir := range []string{house, project} {
98+
if err := os.MkdirAll(dir, 0o700); err != nil {
99+
t.Fatal(err)
100+
}
101+
}
102+
useHome(t, house)
103+
root := filepath.VolumeName(base) + string(filepath.Separator)
104+
for _, tc := range []struct {
105+
name, workspace string
106+
planned bool
107+
}{
108+
{"home", house, false},
109+
{"home with a trailing separator", house + string(filepath.Separator), false},
110+
{"parent of home", filepath.Dir(house), false},
111+
{"filesystem root", root, false},
112+
{"project beside home", project, true},
113+
} {
114+
t.Run(tc.name, func(t *testing.T) {
115+
c := newCell(t)
116+
ledger := &planLedger{}
117+
e := EngineFor(tc.workspace)
118+
e.DataRoot, e.Transport = t.TempDir(), ledger.over(fakePlans())
119+
if _, err := e.Seal(t.Context(), c, TurnInfo{}); err != nil {
120+
t.Fatal(err)
121+
}
122+
if got := ledger.asked(tc.workspace); (got > 0) != tc.planned {
123+
t.Fatalf("plans of %s = %d, want planned=%v (asked for %v)", tc.workspace, got, tc.planned, ledger.trees)
124+
}
125+
if !tc.planned && ledger.asked(c.Root) == 0 {
126+
t.Fatalf("the chat's own folder was not sealed instead (asked for %v)", ledger.trees)
127+
}
128+
})
129+
}
130+
}
131+
132+
// The seat says once, in plain words, that a home folder's files are not kept.
133+
func TestAHomeSeatSaysOnceThatItsFilesAreNotSaved(t *testing.T) {
134+
// The seat is built over a fake transport, so no engine binary is needed;
135+
// without this a build with no engine inside gets a plain seat and no notice.
136+
t.Cleanup(cell.UseAvailabilityInTests(true))
137+
house := t.TempDir()
138+
useHome(t, house)
139+
for _, tc := range []struct {
140+
workspace string
141+
notices int
142+
}{{house, 1}, {t.TempDir(), 0}} {
143+
var said []string
144+
_, err := SeatOver(executor.HostBound, newCell(t), tc.workspace, nil, nil, nil,
145+
WithNotices(func(s string) { said = append(said, s) }),
146+
func(e *Engine) { e.DataRoot, e.Transport = t.TempDir(), fakePlans() })
147+
if err != nil {
148+
t.Fatal(err)
149+
}
150+
home := 0
151+
for _, s := range said {
152+
if strings.Contains(s, "home folder") {
153+
home++
154+
}
155+
}
156+
if home != tc.notices {
157+
t.Fatalf("workspace %s: home notices = %d, want %d (%q)", tc.workspace, home, tc.notices, said)
158+
}
159+
}
160+
}
161+
162+
// TestStartupInALargeHomeWalksNoHome is the startup gate: a seat opened with
163+
// warm capture in a home of many files, through the real engine, finishes its
164+
// first capture having planned none of the home's files and having written no
165+
// plan that lists them. It counts paths and plan bytes; it never times.
166+
func TestStartupInALargeHomeWalksNoHome(t *testing.T) {
167+
const dirs, files = 100, 200 // 20,000 files
168+
house := t.TempDir()
169+
fillHome(t, house, dirs, files)
170+
useHome(t, house)
171+
real := realEngine(t)
172+
ledger := &planLedger{}
173+
c := newCell(t)
174+
seat, err := SeatOver(executor.HostBound, c, house, nil, func(err error) {
175+
if err != nil {
176+
t.Errorf("seal: %v", err)
177+
}
178+
}, nil, WithPipeline(), WithWarmCapture(), WithNotices(func(string) {}), func(e *Engine) {
179+
e.Binary, e.DataRoot, e.Transport = real.Binary, real.DataRoot, ledger.over(real.Transport)
180+
})
181+
if err != nil {
182+
t.Fatal(err)
183+
}
184+
if err := executor.Drain(t.Context(), seat); err != nil {
185+
t.Fatal(err)
186+
}
187+
if head, err := Head(c); err != nil || head == nil {
188+
t.Fatalf("the chat was not sealed: %v", err)
189+
}
190+
if n := ledger.asked(house); n != 0 {
191+
t.Fatalf("the home was planned %d times", n)
192+
}
193+
if ledger.paths >= dirs*files/10 {
194+
t.Fatalf("startup planned %d paths for a home of %d files", ledger.paths, dirs*files)
195+
}
196+
var planBytes int64
197+
_ = filepath.WalkDir(real.DataRoot, func(path string, d fs.DirEntry, err error) error {
198+
if err == nil && !d.IsDir() && strings.Contains(filepath.ToSlash(path), "/capture-plans/") {
199+
if info, err := d.Info(); err == nil {
200+
planBytes += info.Size()
201+
}
202+
}
203+
return nil
204+
})
205+
// One plan line per home file would be at least ten bytes each.
206+
if planBytes >= int64(dirs*files) {
207+
t.Fatalf("startup wrote %d bytes of capture plans for a home of %d files", planBytes, dirs*files)
208+
}
209+
}
210+
211+
// A CHAT THAT LEFT ITS FOLDER'S FILES UNSAVED ARRIVES THAT WAY. A chat sealed in
212+
// its sender's home folder carries only its own folder. On a machine whose home
213+
// is elsewhere, the sender's home path may be an ordinary folder that exists
214+
// (somebody else's files, or the same path by coincidence). The receiver must
215+
// still seal and restore only the chat's own folder, never that path.
216+
func TestAHomeChatNeverCapturesTheSendersHomeOnAnotherMachine(t *testing.T) {
217+
base := canonicalTree(t.TempDir())
218+
sender, receiver := filepath.Join(base, "sender"), filepath.Join(base, "receiver")
219+
for _, dir := range []string{sender, receiver} {
220+
if err := os.MkdirAll(dir, 0o700); err != nil {
221+
t.Fatal(err)
222+
}
223+
}
224+
c := newCell(t)
225+
useHome(t, sender)
226+
e := EngineFor(sender)
227+
e.DataRoot, e.Transport = t.TempDir(), fakePlans()
228+
if _, err := e.Seal(t.Context(), c, TurnInfo{}); err != nil {
229+
t.Fatal(err)
230+
}
231+
// The same chat, its .cell/ as it arrived, on a machine whose home is not
232+
// the sender's: the session record still names the sender's path.
233+
useHome(t, receiver)
234+
ledger := &planLedger{}
235+
there := EngineFor(sender)
236+
there.DataRoot, there.Transport = t.TempDir(), ledger.over(fakePlans())
237+
if _, err := there.Seal(t.Context(), c, TurnInfo{}); err != nil {
238+
t.Fatal(err)
239+
}
240+
if n := ledger.asked(sender); n != 0 {
241+
t.Fatalf("the receiver planned the sender's home path %d times (asked for %v)", n, ledger.trees)
242+
}
243+
if got := there.tree(c); got != canonicalTree(c.Root) {
244+
t.Fatalf("the receiver restores into %s, want the chat's own folder %s", got, c.Root)
245+
}
246+
// A work/ of the chat's own is still the chat's, and on the sender, where the
247+
// path is home, the path stays the chat's working folder.
248+
if ForeignFolder(c.Root, filepath.Join(c.Root, "work")) {
249+
t.Fatal("the chat's own work/ is called foreign")
250+
}
251+
useHome(t, sender)
252+
if ForeignFolder(c.Root, sender) {
253+
t.Fatal("the sender's own home is called foreign on the sender")
254+
}
255+
}

0 commit comments

Comments
 (0)