amadan

carlos / platform Public

stopevidence, lease: StepFDs by file identity; lock absence only on NoSuchKey (moves task 3 fix round 1)

Reconstruct's step 3 now uses fdHoldersByIdentity: each database file
(and -wal, -shm) is stat'ed for device and inode and compared with a stat
of every /proc/<pid>/task/<tid>/fd/<n>, besides the path-text match (which
keeps " (deleted)"). A hard link, a symlinked database path and a holder
in another mount namespace's bind path are now holders. An unreadable or
unstat-able descriptor, or an unstat-able database path, is ErrIncomplete.
FDHolders, which the claim code owns, is unchanged.

LockState treats a lock as absent only on HTTP 404 with a parsed NoSuchKey
code. A malformed, empty or unrecognised 404, NotFound, NoSuchVersion,
NoSuchBucket or NoSuchKey on another status is an error. mapReadErr is
unchanged for its other callers.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude ef001dd50959b263cec33d9024f7e6d13160ee28 parent 0900c76
7 files changed, +612 −8
  • approved_moves_copy_test.go +1 −0
  • internal/lease/lockstate.go +17 −2
  • internal/lease/lockstate_test.go +49 −0
  • internal/stopevidence/fdidentity_linux.go +177 −0
  • internal/stopevidence/fdidentity_linux_test.go +333 −0
  • internal/stopevidence/reconstruct_linux.go +7 −5
  • internal/stopevidence/reconstruct_linux_test.go +28 −1
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go
index 9eb8afdc7..a2cdc2c3b 100644
--- a/approved_moves_copy_test.go
+++ b/approved_moves_copy_test.go
@@ -599,6 +599,7 @@ var movesScanFiles = []string{
"internal/release/materializeheld.go",
"internal/release/proofgate.go",
"internal/release/refusal.go",
+ "internal/stopevidence/fdidentity_linux.go",
"internal/stopevidence/reconstruct.go",
"internal/stopevidence/reconstruct_linux.go",
"internal/stopevidence/reconstruct_other.go",
diff --git a/internal/lease/lockstate.go b/internal/lease/lockstate.go
index 53d268104..95db5c0cf 100644
--- a/internal/lease/lockstate.go
+++ b/internal/lease/lockstate.go
@@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
+ "net/http"
"path"
"strings"
"time"
@@ -39,7 +40,8 @@ const lockBodyLimit = 64 << 10
// more than RenewMargin, so a holder whose clock runs slightly behind
// still counts as holding. An absent lock (never acquired, or released:
// the leaser deletes it) is held=false with a zero owner and expiry and a
-// nil error. An expired lock is held=false with its owner and expiry, for
+// nil error; only S3's explicit NoSuchKey counts as absent (lockAbsent).
+// An expired lock is held=false with its owner and expiry, for
// the record. Anything else (a missing bucket, an unreadable or garbled
// body, a lock with no expiry) is an error, never "not held".
func LockState(ctx context.Context, bucket, location, region string) (held bool, owner string, expires time.Time, err error) {
@@ -53,7 +55,7 @@ func LockState(ctx context.Context, bucket, location, region string) (held bool,
}
out, err := client.GetObject(ctx, &awss3.GetObjectInput{Bucket: &bucket, Key: &key})
if err != nil {
- if errors.Is(mapReadErr(err), ErrMirrorNotFound) {
+ if lockAbsent(err) {
return false, "", time.Time{}, nil
}
return false, "", time.Time{}, fmt.Errorf("lease: read %s: %w", key, err)
@@ -76,6 +78,19 @@ func LockState(ctx context.Context, bucket, location, region string) (held bool,
return lockHeld(lock.ExpiresAt, time.Now()), lock.Owner, lock.ExpiresAt, nil
}
+// lockAbsent reports an explicit S3 object-absence answer: HTTP 404 with an
+// error body the SDK parsed to the code NoSuchKey. Absence is what lets a
+// cut call the lease free, so nothing weaker counts: not a 404 with a
+// malformed, empty or unrecognised body (an endpoint that is not the store,
+// a proxy), not NotFound or NoSuchVersion, not NoSuchBucket, and not
+// NoSuchKey on any other status. mapReadErr is deliberately not used here:
+// its looser reading (any 404 but NoSuchBucket) suits its callers, which
+// treat absence as "nothing to copy", not as authority.
+func lockAbsent(err error) bool {
+ code, status := apiCode(err)
+ return code == "NoSuchKey" && status == http.StatusNotFound
+}
+
// lockHeld is §5.3 step 4's rule: a lock is released only once its expiry
// has passed by more than the renewal margin.
func lockHeld(expires, now time.Time) bool {
diff --git a/internal/lease/lockstate_test.go b/internal/lease/lockstate_test.go
index 4aaa2dc2d..f1f87ac8c 100644
--- a/internal/lease/lockstate_test.go
+++ b/internal/lease/lockstate_test.go
@@ -2,6 +2,8 @@ package lease_test
import (
"context"
+ "io"
+ "net/http"
"net/http/httptest"
"testing"
"time"
@@ -107,3 +109,50 @@ func TestLockStateFailsClosed(t *testing.T) {
}
}
}
+
+// faultLockServer answers every request with status and body, as an S3
+// endpoint (or something pretending to be one) might.
+func faultLockServer(t *testing.T, status int, body string) {
+ t.Helper()
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if body != "" {
+ w.Header().Set("Content-Type", "application/xml")
+ }
+ w.WriteHeader(status)
+ io.WriteString(w, body)
+ }))
+ t.Cleanup(srv.Close)
+ lockFixture(t)
+ t.Setenv("CARLOS_S3_ENDPOINT", srv.URL)
+}
+
+// Absence is an authority-bearing answer ("the lease is free"), so only S3's
+// explicit, parseable NoSuchKey counts. Any other 404 is an error.
+func TestLockStateAbsenceNeedsAnExplicitNoSuchKey(t *testing.T) {
+ ctx := context.Background()
+ faultLockServer(t, http.StatusNotFound, `<?xml version="1.0" encoding="UTF-8"?><Error><Code>NoSuchKey</Code><Message>The specified key does not exist.</Message></Error>`)
+ if got, _, _, err := lease.LockState(ctx, "data", "instances/h.example.com", "test-region"); err != nil || got {
+ t.Fatalf("explicit NoSuchKey: held %v err %v, want absent", got, err)
+ }
+ for name, c := range map[string]struct {
+ status int
+ body string
+ }{
+ "malformed 404": {http.StatusNotFound, `this is not an S3 error body`},
+ "truncated 404": {http.StatusNotFound, `<Error><Code>NoSuchKey`},
+ "empty 404": {http.StatusNotFound, ``},
+ "unrecognised 404": {http.StatusNotFound, `<Error><Code>NoSuchThing</Code><Message>x</Message></Error>`},
+ "NotFound 404": {http.StatusNotFound, `<Error><Code>NotFound</Code><Message>x</Message></Error>`},
+ "NoSuchVersion": {http.StatusNotFound, `<Error><Code>NoSuchVersion</Code><Message>x</Message></Error>`},
+ "NoSuchBucket": {http.StatusNotFound, `<Error><Code>NoSuchBucket</Code><Message>x</Message></Error>`},
+ "NoSuchKey on 200": {http.StatusOK, `<Error><Code>NoSuchKey</Code></Error>`},
+ "NoSuchKey on 403": {http.StatusForbidden, `<Error><Code>NoSuchKey</Code></Error>`},
+ } {
+ t.Run(name, func(t *testing.T) {
+ faultLockServer(t, c.status, c.body)
+ if got, _, _, err := lease.LockState(ctx, "data", "instances/h.example.com", "test-region"); err == nil || got {
+ t.Fatalf("held %v err %v, want an error", got, err)
+ }
+ })
+ }
+}
diff --git a/internal/stopevidence/fdidentity_linux.go b/internal/stopevidence/fdidentity_linux.go
new file mode 100644
index 000000000..a65e063cd
--- /dev/null
+++ b/internal/stopevidence/fdidentity_linux.go
@@ -0,0 +1,177 @@
+//go:build linux
+
+package stopevidence
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "sort"
+ "strconv"
+ "strings"
+ "syscall"
+)
+
+// fileID is a file's identity: the device and inode a stat reports.
+type fileID struct{ dev, ino uint64 }
+
+// identityOps are fdHoldersByIdentity's filesystem calls: the real ones,
+// or faults in tests.
+type identityOps struct {
+ readDir func(string) ([]os.DirEntry, error)
+ readlink func(string) (string, error)
+ stat func(string) (os.FileInfo, error)
+ lstat func(string) (os.FileInfo, error)
+}
+
+func realIdentityOps() identityOps {
+ return identityOps{os.ReadDir, os.Readlink, os.Stat, os.Lstat}
+}
+
+// fdHoldersByIdentity is Reconstruct's step 3: the pids under procRoot with
+// a descriptor on one of paths, matched by file identity (device and inode)
+// as well as by path text. Identity catches what the path-text scan
+// (FDHolders, which the claim code owns and which this does not change)
+// cannot: a hard link to the database, a database path that runs through a
+// symlink, and a holder in another mount namespace whose descriptor names
+// a bind-mount path. os.Stat on /proc/<pid>/task/<tid>/fd/<n> follows the
+// kernel's magic link to the open file itself, whatever its name.
+//
+// It fails closed. A descriptor counts as a hold when its link text names
+// one of paths, a " (deleted)" suffix included (a writer holding a database
+// that was deleted and replaced is still a writer). A descriptor that
+// cannot be read or stat'ed while its entry is still there, a stat with no
+// device and inode, or a database path that cannot be stat'ed for any
+// reason but absence, makes the scan incomplete: ErrIncomplete, never a
+// pass. A database file that does not exist (no WAL yet) has no identity
+// to hold and is matched by its path text only. Descriptors whose link
+// text is not a path (socket:[...], pipe:[...], anon_inode:[...]) are not
+// files on disk and are never followed.
+func fdHoldersByIdentity(ctx context.Context, procRoot string, paths []string, ops identityOps) ([]int, error) {
+ if ctx == nil || !filepath.IsAbs(procRoot) || len(paths) == 0 {
+ return nil, ErrInvalidInput
+ }
+ wanted := make(map[string]bool, len(paths))
+ ids := map[fileID]bool{}
+ for _, p := range paths {
+ if !filepath.IsAbs(p) {
+ return nil, ErrInvalidInput
+ }
+ p = filepath.Clean(p)
+ wanted[p] = true
+ fi, err := ops.stat(p)
+ if errors.Is(err, fs.ErrNotExist) {
+ continue
+ }
+ if err != nil {
+ return nil, fmt.Errorf("%w: %s: %w", ErrIncomplete, p, err)
+ }
+ id, ok := identityOf(fi)
+ if !ok {
+ return nil, fmt.Errorf("%w: %s has no device and inode", ErrIncomplete, p)
+ }
+ ids[id] = true
+ }
+ if err := ctx.Err(); err != nil {
+ return nil, err
+ }
+ dirOps := fdOps{readDir: ops.readDir, readlink: ops.readlink, stat: ops.stat}
+ entries, err := ops.readDir(procRoot)
+ if err != nil {
+ return nil, fmt.Errorf("%w: %w", ErrIncomplete, err)
+ }
+ var holders []int
+ for _, entry := range entries {
+ pid, err := strconv.Atoi(entry.Name())
+ if err != nil || pid <= 0 {
+ continue
+ }
+ pidDir := filepath.Join(procRoot, entry.Name())
+ tasks, err := readProcDir(filepath.Join(pidDir, "task"), pidDir, dirOps)
+ if err != nil {
+ return nil, fmt.Errorf("%w: pid %d: %w", ErrIncomplete, pid, err)
+ }
+ found := false
+ for _, task := range tasks {
+ if found {
+ break
+ }
+ tid, err := strconv.Atoi(task.Name())
+ if err != nil || tid <= 0 {
+ continue
+ }
+ taskDir := filepath.Join(pidDir, "task", task.Name())
+ fdDir := filepath.Join(taskDir, "fd")
+ fds, err := readProcDir(fdDir, taskDir, dirOps)
+ if err != nil {
+ return nil, fmt.Errorf("%w: pid %d tid %d: %w", ErrIncomplete, pid, tid, err)
+ }
+ for _, fd := range fds {
+ if err := ctx.Err(); err != nil {
+ return nil, err
+ }
+ held, err := fdHolds(filepath.Join(fdDir, fd.Name()), wanted, ids, ops)
+ if err != nil {
+ return nil, fmt.Errorf("%w: pid %d tid %d fd %s: %w", ErrIncomplete, pid, tid, fd.Name(), err)
+ }
+ if held {
+ found = true
+ break
+ }
+ }
+ }
+ if found {
+ holders = append(holders, pid)
+ }
+ }
+ if err := ctx.Err(); err != nil {
+ return nil, err
+ }
+ sort.Ints(holders)
+ return holders, nil
+}
+
+// fdHolds reports whether one descriptor entry holds a wanted file. A
+// descriptor that closed between the listing and the reads holds nothing.
+func fdHolds(fdPath string, wanted map[string]bool, ids map[fileID]bool, ops identityOps) (bool, error) {
+ target, err := ops.readlink(fdPath)
+ if procGone(err) {
+ return false, nil
+ }
+ if err != nil {
+ return false, err
+ }
+ if !filepath.IsAbs(target) {
+ return false, nil
+ }
+ if wanted[filepath.Clean(target)] || wanted[filepath.Clean(strings.TrimSuffix(target, " (deleted)"))] {
+ return true, nil
+ }
+ fi, err := ops.stat(fdPath)
+ if err != nil {
+ if procGone(err) {
+ if _, lerr := ops.lstat(fdPath); procGone(lerr) {
+ return false, nil
+ } else if lerr != nil {
+ return false, lerr
+ }
+ }
+ return false, err
+ }
+ id, ok := identityOf(fi)
+ if !ok {
+ return false, errors.New("no device and inode")
+ }
+ return ids[id], nil
+}
+
+func identityOf(fi os.FileInfo) (fileID, bool) {
+ st, ok := fi.Sys().(*syscall.Stat_t)
+ if !ok || st == nil {
+ return fileID{}, false
+ }
+ return fileID{dev: uint64(st.Dev), ino: uint64(st.Ino)}, true
+}
diff --git a/internal/stopevidence/fdidentity_linux_test.go b/internal/stopevidence/fdidentity_linux_test.go
new file mode 100644
index 000000000..939b26054
--- /dev/null
+++ b/internal/stopevidence/fdidentity_linux_test.go
@@ -0,0 +1,333 @@
+//go:build linux
+
+package stopevidence
+
+import (
+ "context"
+ "errors"
+ "io/fs"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "reflect"
+ "strconv"
+ "strings"
+ "syscall"
+ "testing"
+ "time"
+)
+
+// holdOpen starts a child that holds path open as descriptor 3 and sleeps.
+func holdOpen(t *testing.T, path string) int {
+ t.Helper()
+ f, err := os.Open(path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ cmd := exec.Command("sleep", "60")
+ cmd.ExtraFiles = []*os.File{f}
+ if err := cmd.Start(); err != nil {
+ t.Fatal(err)
+ }
+ f.Close()
+ t.Cleanup(func() { cmd.Process.Kill(); cmd.Wait() })
+ return cmd.Process.Pid
+}
+
+// projectPIDs is a scan root holding only these processes' real /proc
+// entries, so the scan needs no access to other uids.
+func projectPIDs(t *testing.T, pids ...int) string {
+ t.Helper()
+ root := t.TempDir()
+ for _, pid := range pids {
+ p := strconv.Itoa(pid)
+ if err := os.Symlink(filepath.Join("/proc", p), filepath.Join(root, p)); err != nil {
+ t.Fatal(err)
+ }
+ }
+ return root
+}
+
+func newDB(t *testing.T, dir string) string {
+ t.Helper()
+ if err := os.MkdirAll(dir, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ db := filepath.Join(dir, "h.example.com.db")
+ if err := os.WriteFile(db, []byte("SQLite format 3\x00"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ return db
+}
+
+func dbFilesOf(t *testing.T, db string) []string {
+ t.Helper()
+ files, err := dbPathsWithSidecars([]string{db})
+ if err != nil {
+ t.Fatal(err)
+ }
+ return files
+}
+
+// A hard link is the same file under another name: a holder that opened
+// the link holds the database. The path-text scan (main's FDHolders) does
+// not see it; the identity scan must.
+func TestFDIdentityFindsAHardLinkHeldByAChild(t *testing.T) {
+ dir := t.TempDir()
+ db := newDB(t, filepath.Join(dir, "data"))
+ alias := filepath.Join(dir, "elsewhere", "alias.db")
+ os.MkdirAll(filepath.Dir(alias), 0o755)
+ if err := os.Link(db, alias); err != nil {
+ t.Skip("no hard links here:", err)
+ }
+ pid := holdOpen(t, alias)
+ root := projectPIDs(t, pid)
+ got, err := fdHoldersByIdentity(context.Background(), root, dbFilesOf(t, db), realIdentityOps())
+ if err != nil || !reflect.DeepEqual(got, []int{pid}) {
+ t.Fatalf("identity scan: %v %v, want [%d]", got, err, pid)
+ }
+ if byText, err := FDHolders(root, dbFilesOf(t, db)); err != nil || len(byText) != 0 {
+ t.Logf("path-text scan now sees the hard link (%v %v); the identity scan is still required", byText, err)
+ }
+}
+
+// The database path the caller passes may run through a symlink (a data
+// directory linked elsewhere) while the holder's descriptor names the real
+// path.
+func TestFDIdentityFindsAHolderThroughASymlinkedDBPath(t *testing.T) {
+ dir := t.TempDir()
+ db := newDB(t, filepath.Join(dir, "real"))
+ link := filepath.Join(dir, "data")
+ if err := os.Symlink(filepath.Join(dir, "real"), link); err != nil {
+ t.Fatal(err)
+ }
+ pid := holdOpen(t, db)
+ root := projectPIDs(t, pid)
+ viaLink := filepath.Join(link, filepath.Base(db))
+ got, err := fdHoldersByIdentity(context.Background(), root, dbFilesOf(t, viaLink), realIdentityOps())
+ if err != nil || !reflect.DeepEqual(got, []int{pid}) {
+ t.Fatalf("identity scan through the symlinked path: %v %v, want [%d]", got, err, pid)
+ }
+}
+
+// A holder in another mount namespace sees the database through a bind
+// mount at a different path; its descriptor's link text is that path. An
+// unprivileged user and mount namespace builds it; where the box forbids
+// that, the test skips.
+func TestFDIdentityFindsAHolderInAnotherMountNamespace(t *testing.T) {
+ if _, err := exec.LookPath("unshare"); err != nil {
+ t.Skip("no unshare:", err)
+ }
+ dir := t.TempDir()
+ db := newDB(t, filepath.Join(dir, "real"))
+ alias := filepath.Join(dir, "alias")
+ if err := os.MkdirAll(alias, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ aliased := filepath.Join(alias, filepath.Base(db))
+ cmd := exec.Command("unshare", "--user", "--map-root-user", "--mount", "sh", "-c",
+ `mount --bind "$1" "$2" && exec sleep 60 3<"$3"`, "sh", filepath.Join(dir, "real"), alias, aliased)
+ if err := cmd.Start(); err != nil {
+ t.Skip("cannot start unshare:", err)
+ }
+ t.Cleanup(func() { cmd.Process.Kill(); cmd.Wait() })
+ pid := cmd.Process.Pid
+ fd3 := filepath.Join("/proc", strconv.Itoa(pid), "fd", "3")
+ var target string
+ for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(20 * time.Millisecond) {
+ if target, _ = os.Readlink(fd3); target != "" {
+ break
+ }
+ }
+ if target == "" {
+ t.Skip("the namespace holder never came up (unprivileged user namespaces refused?)")
+ }
+ if target != aliased {
+ t.Logf("descriptor text %q (expected the bind path %q)", target, aliased)
+ }
+ root := projectPIDs(t, pid)
+ got, err := fdHoldersByIdentity(context.Background(), root, dbFilesOf(t, db), realIdentityOps())
+ if err != nil || !reflect.DeepEqual(got, []int{pid}) {
+ t.Fatalf("identity scan across a mount namespace: %v %v, want [%d]", got, err, pid)
+ }
+}
+
+// A writer still holding a database that was deleted and replaced holds a
+// file that is no longer at the path; its link text names the path with
+// " (deleted)", and that is a hold, never a pass.
+func TestFDIdentityDeletedAndReplacedDatabaseIsAHold(t *testing.T) {
+ db := newDB(t, t.TempDir())
+ pid := holdOpen(t, db)
+ if err := os.Remove(db); err != nil {
+ t.Fatal(err)
+ }
+ newDB(t, filepath.Dir(db))
+ root := projectPIDs(t, pid)
+ got, err := fdHoldersByIdentity(context.Background(), root, dbFilesOf(t, db), realIdentityOps())
+ if err != nil || !reflect.DeepEqual(got, []int{pid}) {
+ t.Fatalf("deleted and replaced: %v %v, want [%d]", got, err, pid)
+ }
+}
+
+// A process holding something else is not a holder, and a database with
+// no WAL or SHM on disk is checked by the files that exist.
+func TestFDIdentityOtherFilesAreNotHolders(t *testing.T) {
+ dir := t.TempDir()
+ db := newDB(t, dir)
+ other := filepath.Join(dir, "other.db")
+ os.WriteFile(other, []byte("x"), 0o644)
+ pid := holdOpen(t, other)
+ got, err := fdHoldersByIdentity(context.Background(), projectPIDs(t, pid), dbFilesOf(t, db), realIdentityOps())
+ if err != nil || len(got) != 0 {
+ t.Fatalf("other file: %v %v", got, err)
+ }
+}
+
+type fakeInfo struct{ fs.FileInfo }
+
+func (fakeInfo) Sys() any { return nil }
+
+// Every way the scan could fail to see a descriptor is an error, never a
+// pass.
+func TestFDIdentityFailsClosed(t *testing.T) {
+ dir := t.TempDir()
+ db := newDB(t, filepath.Join(dir, "data"))
+ other := filepath.Join(dir, "other")
+ os.WriteFile(other, []byte("x"), 0o644)
+ root := filepath.Join(dir, "proc")
+ fdDir := filepath.Join(root, "20", "task", "20", "fd")
+ os.MkdirAll(fdDir, 0o755)
+ fdLink := filepath.Join(fdDir, "0")
+ if err := os.Symlink(other, fdLink); err != nil {
+ t.Fatal(err)
+ }
+ real := realIdentityOps()
+ files := dbFilesOf(t, db)
+ for _, c := range []struct {
+ name string
+ mutate func(*identityOps)
+ wantErr bool
+ }{
+ {"fd stat refused", func(o *identityOps) {
+ o.stat = func(p string) (os.FileInfo, error) {
+ if p == fdLink {
+ return nil, syscall.EACCES
+ }
+ return real.stat(p)
+ }
+ }, true},
+ {"fd stat gone but the entry is there", func(o *identityOps) {
+ o.stat = func(p string) (os.FileInfo, error) {
+ if p == fdLink {
+ return nil, syscall.ENOENT
+ }
+ return real.stat(p)
+ }
+ }, true},
+ {"fd closed between the reads", func(o *identityOps) {
+ o.stat = func(p string) (os.FileInfo, error) {
+ if p == fdLink {
+ return nil, syscall.ENOENT
+ }
+ return real.stat(p)
+ }
+ o.lstat = func(p string) (os.FileInfo, error) { return nil, syscall.ENOENT }
+ }, false},
+ {"fd lstat refused", func(o *identityOps) {
+ o.stat = func(p string) (os.FileInfo, error) {
+ if p == fdLink {
+ return nil, syscall.ENOENT
+ }
+ return real.stat(p)
+ }
+ o.lstat = func(p string) (os.FileInfo, error) { return nil, syscall.EACCES }
+ }, true},
+ {"fd link refused", func(o *identityOps) {
+ o.readlink = func(string) (string, error) { return "", syscall.EACCES }
+ }, true},
+ {"fd closed before the link read", func(o *identityOps) {
+ o.readlink = func(string) (string, error) { return "", syscall.ENOENT }
+ }, false},
+ {"fd with no identity", func(o *identityOps) {
+ o.stat = func(p string) (os.FileInfo, error) {
+ fi, err := real.stat(p)
+ if p == fdLink && err == nil {
+ return fakeInfo{fi}, nil
+ }
+ return fi, err
+ }
+ }, true},
+ {"database stat refused", func(o *identityOps) {
+ o.stat = func(p string) (os.FileInfo, error) {
+ if p == db {
+ return nil, syscall.EACCES
+ }
+ return real.stat(p)
+ }
+ }, true},
+ {"database with no identity", func(o *identityOps) {
+ o.stat = func(p string) (os.FileInfo, error) {
+ fi, err := real.stat(p)
+ if p == db && err == nil {
+ return fakeInfo{fi}, nil
+ }
+ return fi, err
+ }
+ }, true},
+ } {
+ ops := realIdentityOps()
+ c.mutate(&ops)
+ got, err := fdHoldersByIdentity(context.Background(), root, files, ops)
+ if c.wantErr {
+ if !errors.Is(err, ErrIncomplete) || got != nil {
+ t.Fatalf("%s: %v %v, want ErrIncomplete", c.name, got, err)
+ }
+ continue
+ }
+ if err != nil || len(got) != 0 {
+ t.Fatalf("%s: %v %v, want no holder", c.name, got, err)
+ }
+ }
+}
+
+// Descriptors that are not paths (sockets, pipes, anonymous inodes) are
+// never followed: they cannot be the database, and a stat could block.
+func TestFDIdentitySkipsNonPathDescriptors(t *testing.T) {
+ db := newDB(t, t.TempDir())
+ root := t.TempDir()
+ fdDir := filepath.Join(root, "20", "task", "20", "fd")
+ os.MkdirAll(fdDir, 0o755)
+ for n, target := range []string{"socket:[123]", "pipe:[9]", "anon_inode:[eventfd]"} {
+ os.Symlink(target, filepath.Join(fdDir, strconv.Itoa(n)))
+ }
+ ops := realIdentityOps()
+ real := ops.stat
+ ops.stat = func(p string) (os.FileInfo, error) {
+ if strings.HasPrefix(p, root) {
+ t.Fatalf("stat of a non-path descriptor %s", p)
+ }
+ return real(p)
+ }
+ if got, err := fdHoldersByIdentity(context.Background(), root, dbFilesOf(t, db), ops); err != nil || len(got) != 0 {
+ t.Fatalf("%v %v", got, err)
+ }
+}
+
+func TestFDIdentityInvalidInput(t *testing.T) {
+ db := newDB(t, t.TempDir())
+ ctx := context.Background()
+ for name, c := range map[string]struct {
+ ctx context.Context
+ root string
+ paths []string
+ }{
+ "nil context": {nil, "/proc", []string{db}},
+ "relative root": {ctx, "proc", []string{db}},
+ "no paths": {ctx, "/proc", nil},
+ "relative path": {ctx, "/proc", []string{"h.db"}},
+ } {
+ if _, err := fdHoldersByIdentity(c.ctx, c.root, c.paths, realIdentityOps()); !errors.Is(err, ErrInvalidInput) {
+ t.Fatalf("%s: %v", name, err)
+ }
+ }
+}
diff --git a/internal/stopevidence/reconstruct_linux.go b/internal/stopevidence/reconstruct_linux.go
index bce6e089c..fe3d162a1 100644
--- a/internal/stopevidence/reconstruct_linux.go
+++ b/internal/stopevidence/reconstruct_linux.go
@@ -117,12 +117,14 @@ func reconstruct(ctx context.Context, in ReconstructInputs, open func(string) (t
}
}
- // 3. No open database. The scan covers every process /proc shows. On a
- // box the agent runs under ProtectProc=invisible (carlos-edge.service),
- // so /proc shows only the carlos uid's processes: §5.3's "restricted to
- // the carlos uid". A process it can see but not read makes the scan
+ // 3. No open database, by file identity (fdHoldersByIdentity): a hard
+ // link, a symlinked path or another mount namespace's bind path is the
+ // same file. The scan covers every process /proc shows. On a box the
+ // agent runs under ProtectProc=invisible (carlos-edge.service), so /proc
+ // shows only the carlos uid's processes: §5.3's "restricted to the
+ // carlos uid". A process it can see but not read makes the scan
// incomplete, which is an error, never a pass.
- holders, err := scanFDs(ctx, procRoot, dbFiles)
+ holders, err := fdHoldersByIdentity(ctx, procRoot, dbFiles, realIdentityOps())
if err != nil {
return rep, err
}
diff --git a/internal/stopevidence/reconstruct_linux_test.go b/internal/stopevidence/reconstruct_linux_test.go
index d33058aca..0f7cd5dd8 100644
--- a/internal/stopevidence/reconstruct_linux_test.go
+++ b/internal/stopevidence/reconstruct_linux_test.go
@@ -360,10 +360,37 @@ func TestReconstructDatabaseHolders(t *testing.T) {
}
// A descriptor on some other file is not a holder.
f := newBoxFixture(t)
- f.holdDB(t, 5150, filepath.Join(filepath.Dir(f.db), "other.db"))
+ other := filepath.Join(filepath.Dir(f.db), "other.db")
+ write(t, other, "x")
+ f.holdDB(t, 5150, other)
if rep, err := reconstruct(context.Background(), f.inputs(), opener(&fakeTree{}, nil)); err != nil || !rep.Steps[moves.StepFDs].OK {
t.Fatalf("other file: %+v %v", rep.Steps, err)
}
+ // A descriptor whose file cannot be identified is not a pass.
+ f = newBoxFixture(t)
+ f.holdDB(t, 5150, filepath.Join(filepath.Dir(f.db), "vanished.db"))
+ if _, err := reconstruct(context.Background(), f.inputs(), opener(&fakeTree{}, nil)); !errors.Is(err, ErrIncomplete) {
+ t.Fatalf("unidentifiable descriptor: %v", err)
+ }
+}
+
+// Step 3 compares file identity, not path text: a holder of a hard link
+// to the database (another name, same file) fails StepFDs.
+func TestReconstructDatabaseHeldThroughAHardLink(t *testing.T) {
+ f := newBoxFixture(t)
+ write(t, f.db, "SQLite format 3")
+ alias := filepath.Join(filepath.Dir(f.db), "..", "alias.db")
+ if err := os.Link(f.db, alias); err != nil {
+ t.Skip("no hard links here:", err)
+ }
+ f.holdDB(t, 5150, filepath.Clean(alias))
+ rep, err := reconstruct(context.Background(), f.inputs(), opener(&fakeTree{}, nil))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if c := rep.Steps[moves.StepFDs]; c.OK || c.Reason != "1 process(es) hold the database open" || rep.TreeStopped() {
+ t.Fatalf("hard-link holder passed: %+v", rep.Steps)
+ }
}
func TestReconstructLeaseStep(t *testing.T) {