amadan
stopevidence: re-read the agent's leaf after the descriptor scan (moves task 3 fix round 2)
StepStray read the agent's cgroup once, before the descriptor scan and the lease read, so a process that joined the leaf meanwhile and held no database descriptor passed. Reconstruct now reads the leaf again after step 4, under the same rules (unreadable or without the agent is an error); any pid but the agent in either read fails StepStray for the pass, and Stray lists the strays of both reads.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+97
−12
internal/stopevidence/reconstruct_linux.go+46 −12internal/stopevidence/reconstruct_linux_test.go+51 −0
diff --git a/internal/stopevidence/reconstruct_linux.go b/internal/stopevidence/reconstruct_linux.go| index fe3d162a1..f383348ac 100644 |
| --- a/internal/stopevidence/reconstruct_linux.go |
| +++ b/internal/stopevidence/reconstruct_linux.go |
| @@ -101,21 +101,13 @@ func reconstruct(ctx context.Context, in ReconstructInputs, open func(string) (t |
| // alongside this step. So does any short-lived child the agent forks |
| // for itself: it fails StepStray for this pass, and the cut's retry |
| // until its deadline is the intended behaviour. Only a repeat is worth |
| - // the alert (M-C20). |
| - pids, err := ownLeafProcs(procRoot) |
| + // the alert (M-C20). The leaf is read again after steps 3 and 4 (below). |
| + self := os.Getpid() |
| + first, err := agentLeaf(procRoot, self) |
| if err != nil { |
| return rep, err |
| } |
| - self := os.Getpid() |
| - if !slices.Contains(pids, self) { |
| - return rep, fmt.Errorf("stopevidence: the agent (pid %d) is not in the cgroup its /proc entry names", self) |
| - } |
| - rep.Steps[moves.StepStray] = strayCheck(pids, self) |
| - for _, p := range pids { |
| - if p != self { |
| - rep.Stray = append(rep.Stray, p) |
| - } |
| - } |
| + rep.Steps[moves.StepStray] = strayCheck(first, self) |
| // 3. No open database, by file identity (fdHoldersByIdentity): a hard |
| // link, a symlinked path or another mount namespace's bind path is the |
| @@ -145,6 +137,21 @@ func reconstruct(ctx context.Context, in ReconstructInputs, open func(string) (t |
| rep.Steps[moves.StepLease] = moves.Check{Reason: "lock.json still held"} |
| } |
| + // 2, again. The first read is a snapshot taken before the descriptor |
| + // scan and the lease read, which take time; a process that joined the |
| + // agent's leaf meanwhile and holds no database descriptor would pass |
| + // both. Any pid in the second read but the agent fails StepStray for |
| + // this pass, and the cut's retry covers it. Rep.Stray lists the strays |
| + // of both reads. |
| + second, err := agentLeaf(procRoot, self) |
| + if err != nil { |
| + return rep, err |
| + } |
| + if rep.Steps[moves.StepStray].OK { |
| + rep.Steps[moves.StepStray] = strayCheck(second, self) |
| + } |
| + rep.Stray = strays(self, first, second) |
| + |
| // The evidence time is when the last step was observed, not when the |
| // pass began: §5.3's late-completion horizon runs from it, and a later |
| // time is the safe direction. |
| @@ -188,6 +195,33 @@ func strayCheck(pids []int, self int) moves.Check { |
| return moves.Check{OK: true} |
| } |
| +// agentLeaf reads the agent's own leaf and requires the agent in it: a |
| +// listing without the agent is the wrong directory, an error. |
| +func agentLeaf(procRoot string, self int) ([]int, error) { |
| + pids, err := ownLeafProcs(procRoot) |
| + if err != nil { |
| + return nil, err |
| + } |
| + if !slices.Contains(pids, self) { |
| + return nil, fmt.Errorf("stopevidence: the agent (pid %d) is not in the cgroup its /proc entry names", self) |
| + } |
| + return pids, nil |
| +} |
| + |
| +// strays is every pid but self across the reads, sorted, each once. |
| +func strays(self int, reads ...[]int) []int { |
| + var out []int |
| + for _, pids := range reads { |
| + for _, p := range pids { |
| + if p != self && !slices.Contains(out, p) { |
| + out = append(out, p) |
| + } |
| + } |
| + } |
| + sort.Ints(out) |
| + return out |
| +} |
| + |
| // dbPathsWithSidecars is every database file with its -wal and -shm, clean, |
| // sorted and without repeats. Each path must be absolute, and there must be |
| // at least one: a cut always has a database path, so an empty list is a |
diff --git a/internal/stopevidence/reconstruct_linux_test.go b/internal/stopevidence/reconstruct_linux_test.go| index 0f7cd5dd8..09a1972d5 100644 |
| --- a/internal/stopevidence/reconstruct_linux_test.go |
| +++ b/internal/stopevidence/reconstruct_linux_test.go |
| @@ -494,3 +494,54 @@ func (d deadlineTree) KillAndEmpty(ctx context.Context) error { |
| return nil |
| } |
| func (deadlineTree) Close() error { return nil } |
| + |
| +// The agent's leaf is read again after the descriptor scan and the lease |
| +// read, so a process that joins it while those run fails StepStray for |
| +// this pass. LeaseFree runs between the two reads, so it is the hook that |
| +// adds the member: deterministic, no timing. |
| +func TestReconstructStrayJoiningDuringTheScanFails(t *testing.T) { |
| + f := newBoxFixture(t) |
| + in := f.inputs() |
| + in.LeaseFree = func(context.Context) (bool, error) { |
| + f.setLeaf(t, os.Getpid(), 4321) |
| + return true, nil |
| + } |
| + rep, err := reconstruct(context.Background(), in, opener(&fakeTree{}, nil)) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if c := rep.Steps[moves.StepStray]; c.OK || c.Reason != "pid 4321 in the agent's cgroup is not the agent" || rep.TreeStopped() { |
| + t.Fatalf("a pid that joined during the scan passed: %+v", rep.Steps) |
| + } |
| + if !reflect.DeepEqual(rep.Stray, []int{4321}) { |
| + t.Fatalf("Stray %v", rep.Stray) |
| + } |
| + // A stray in both reads is listed once. |
| + f = newBoxFixture(t, os.Getpid(), 4243) |
| + in = f.inputs() |
| + in.LeaseFree = func(context.Context) (bool, error) { |
| + f.setLeaf(t, os.Getpid(), 4243, 4321) |
| + return true, nil |
| + } |
| + rep, err = reconstruct(context.Background(), in, opener(&fakeTree{}, nil)) |
| + if err != nil || rep.Steps[moves.StepStray].OK || !reflect.DeepEqual(rep.Stray, []int{4243, 4321}) { |
| + t.Fatalf("stray in both reads: %+v %v %v", rep.Steps, rep.Stray, err) |
| + } |
| +} |
| + |
| +// The second read is held to the first's standard: unreadable, or without |
| +// the agent, is an error. |
| +func TestReconstructSecondLeafReadFailsClosed(t *testing.T) { |
| + for name, mutate := range map[string]func(f *boxFixture){ |
| + "leaf gone": func(f *boxFixture) { os.Remove(filepath.Join(f.leafDir, "cgroup.procs")) }, |
| + "agent gone": func(f *boxFixture) { f.setLeaf(t, 4321) }, |
| + "garbled listing": func(f *boxFixture) { write(t, filepath.Join(f.leafDir, "cgroup.procs"), "x\n") }, |
| + } { |
| + f := newBoxFixture(t) |
| + in := f.inputs() |
| + in.LeaseFree = func(context.Context) (bool, error) { mutate(f); return true, nil } |
| + if _, err := reconstruct(context.Background(), in, opener(&fakeTree{}, nil)); err == nil { |
| + t.Fatalf("%s: passed", name) |
| + } |
| + } |
| +} |