From ef66bfb33197929fd8f4d1773fa44a8c036b85ca Mon Sep 17 00:00:00 2001 From: weichen Date: Mon, 12 Oct 2026 15:07:29 +0000 Subject: [PATCH] Reject a rebuilt lock even when its inode is reused Why: deleting the lock and creating a new file can recycle the same inode, so an inode-only check treated the rebuilt lock as the original. Evidence: SELF go test ./internal/instanceguard ./internal/store ./internal/app, including TestClearedLockRecordRefusedWhenInodeStays. Impact: instance_guard and the lock file now share a random lock_gen. A missing or different generation refuses Bind unless adopt. Refs: docs/recovery/u8-sqlite-instance-guard.md Co-authored-by: Cursor --- internal/instanceguard/guard.go | 86 +++++++++++++++++++++------ internal/instanceguard/guard_other.go | 2 +- internal/instanceguard/guard_test.go | 29 +++++++++ internal/instanceguard/guard_unix.go | 4 +- internal/store/instance_guard.go | 13 ++-- internal/store/migrate.go | 6 +- 6 files changed, 111 insertions(+), 29 deletions(-) diff --git a/internal/instanceguard/guard.go b/internal/instanceguard/guard.go index 6a82fc6..c086727 100644 --- a/internal/instanceguard/guard.go +++ b/internal/instanceguard/guard.go @@ -3,8 +3,10 @@ package instanceguard import ( + "crypto/rand" + "encoding/hex" "errors" - "fmt" + "strconv" "strings" "agent-system-backend/internal/store" @@ -27,6 +29,7 @@ type Record struct { DBDev uint64 DBIno uint64 InstanceID string + LockGen string } // Lease 持有活动服务锁，直到 Release。关闭顺序由调用方保证：先关存储，再 Release。 @@ -94,8 +97,12 @@ func Bind(st *store.Store, lease *Lease, opt BindOptions) error { if err != nil { return err } - if ok && (marker.LockDev != lease.lockDev || marker.LockIno != lease.lockIno) && !opt.Adopt { - return ErrLockRebuilt + if ok && !opt.Adopt { + inodeMismatch := marker.LockDev != lease.lockDev || marker.LockIno != lease.lockIno + genMismatch := marker.LockGen != "" && (lease.record == nil || lease.record.LockGen != marker.LockGen) + if inodeMismatch || genMismatch { + return ErrLockRebuilt + } } id := opt.InstanceID if ok && marker.InstanceID != "" && id != "" && marker.InstanceID != id && !opt.Adopt { @@ -112,10 +119,20 @@ func Bind(st *store.Store, lease *Lease, opt BindOptions) error { return ErrDBMissing } lease.dbDev, lease.dbIno, lease.dbExists = dev, ino, true - next := store.InstanceGuardMarker{ - DBDev: dev, DBIno: ino, LockDev: lease.lockDev, LockIno: lease.lockIno, InstanceID: id, + gen := "" + if ok && marker.LockGen != "" && !opt.Adopt { + gen = marker.LockGen + } else { + gen, err = newLockGen() + if err != nil { + return err + } } - if err := lease.writeRecord(dev, ino, id); err != nil { + next := store.InstanceGuardMarker{ + DBDev: dev, DBIno: ino, LockDev: lease.lockDev, LockIno: lease.lockIno, + InstanceID: id, LockGen: gen, + } + if err := lease.writeRecord(dev, ino, id, gen); err != nil { return err } if !ok || opt.Adopt || marker != next { @@ -126,24 +143,57 @@ func Bind(st *store.Store, lease *Lease, opt BindOptions) error { return nil } +func newLockGen() (string, error) { + buf := make([]byte, 16) + if _, err := rand.Read(buf); err != nil { + return "", err + } + return hex.EncodeToString(buf), nil +} + func parseRecord(raw string) (*Record, error) { raw = strings.TrimSpace(raw) if raw == "" { return nil, nil } - var dev, ino uint64 - var id string - if _, err := fmt.Sscanf(raw, "v1\tdb_dev=%d\tdb_ino=%d\tinstance_id=%s", &dev, &ino, &id); err == nil { - want := fmt.Sprintf("v1\tdb_dev=%d\tdb_ino=%d\tinstance_id=%s", dev, ino, id) - if raw == want && !strings.ContainsAny(id, " \t") { - return &Record{DBDev: dev, DBIno: ino, InstanceID: id}, nil + parts := strings.Split(raw, "\t") + if parts[0] != "v1" { + return nil, ErrLockCorrupt + } + rec := &Record{} + seen := map[string]bool{} + for _, part := range parts[1:] { + key, val, ok := strings.Cut(part, "=") + if !ok || seen[key] || strings.ContainsAny(val, " \t") { + return nil, ErrLockCorrupt + } + seen[key] = true + switch key { + case "db_dev": + n, err := strconv.ParseUint(val, 10, 64) + if err != nil { + return nil, ErrLockCorrupt + } + rec.DBDev = n + case "db_ino": + n, err := strconv.ParseUint(val, 10, 64) + if err != nil { + return nil, ErrLockCorrupt + } + rec.DBIno = n + case "instance_id": + rec.InstanceID = val + case "lock_gen": + if val == "" { + return nil, ErrLockCorrupt + } + rec.LockGen = val + default: + return nil, ErrLockCorrupt } } - if _, err := fmt.Sscanf(raw, "v1\tdb_dev=%d\tdb_ino=%d\tinstance_id=", &dev, &ino); err == nil { - want := fmt.Sprintf("v1\tdb_dev=%d\tdb_ino=%d\tinstance_id=", dev, ino) - if raw == want { - return &Record{DBDev: dev, DBIno: ino}, nil - } + if !seen["db_dev"] || !seen["db_ino"] || !seen["instance_id"] { + return nil, ErrLockCorrupt } - return nil, ErrLockCorrupt + return rec, nil } diff --git a/internal/instanceguard/guard_other.go b/internal/instanceguard/guard_other.go index f6d6797..924f5ea 100644 --- a/internal/instanceguard/guard_other.go +++ b/internal/instanceguard/guard_other.go @@ -9,7 +9,7 @@ func Acquire(string) (*Lease, error) { return nil, ErrUnsupported } -func (l *Lease) writeRecord(uint64, uint64, string) error { +func (l *Lease) writeRecord(uint64, uint64, string, string) error { return ErrUnsupported } diff --git a/internal/instanceguard/guard_test.go b/internal/instanceguard/guard_test.go index b2c1807..4a4dfb2 100644 --- a/internal/instanceguard/guard_test.go +++ b/internal/instanceguard/guard_test.go @@ -181,6 +181,35 @@ func TestRebuiltLockRefusedUntilAdopt(t *testing.T) { } } +func TestClearedLockRecordRefusedWhenInodeStays(t *testing.T) { + dir := t.TempDir() + st, release, err := openAndBind(t, dir, "inst-a") + if err != nil { + t.Fatal(err) + } + release() + _ = st.Close() + lockPath := filepath.Join(dir, "agent-system.db.lock") + if err := os.Truncate(lockPath, 0); err != nil { + t.Fatal(err) + } + lease, err := Acquire(dir) + if err != nil { + t.Fatal(err) + } + opened, err := store.OpenSQLite(lease.DBPath()) + if err != nil { + lease.Release() + t.Fatal(err) + } + err = Bind(opened, lease, BindOptions{InstanceID: "inst-a"}) + _ = opened.Close() + lease.Release() + if !errors.Is(err, ErrLockRebuilt) { + t.Fatalf("cleared lock: %v", err) + } +} + func TestInstanceMismatchPreservesEmpty(t *testing.T) { dir := t.TempDir() st, release, err := openAndBind(t, dir, "inst-a") diff --git a/internal/instanceguard/guard_unix.go b/internal/instanceguard/guard_unix.go index 8cb31a7..ecc293f 100644 --- a/internal/instanceguard/guard_unix.go +++ b/internal/instanceguard/guard_unix.go @@ -115,11 +115,11 @@ func acquireOnce(dbPath, lockPath string) (*Lease, bool, error) { return lease, false, nil } -func (l *Lease) writeRecord(dev, ino uint64, instanceID string) error { +func (l *Lease) writeRecord(dev, ino uint64, instanceID, lockGen string) error { if strings.ContainsAny(instanceID, " \t\r\n") { return fmt.Errorf("instance guard: instance id contains whitespace") } - line := fmt.Sprintf("v1\tdb_dev=%d\tdb_ino=%d\tinstance_id=%s\n", dev, ino, instanceID) + line := fmt.Sprintf("v1\tdb_dev=%d\tdb_ino=%d\tinstance_id=%s\tlock_gen=%s\n", dev, ino, instanceID, lockGen) if _, err := syscall.Seek(l.fd, 0, 0); err != nil { return err } diff --git a/internal/store/instance_guard.go b/internal/store/instance_guard.go index f5bce17..7265ef1 100644 --- a/internal/store/instance_guard.go +++ b/internal/store/instance_guard.go @@ -12,14 +12,15 @@ type InstanceGuardMarker struct { LockDev uint64 LockIno uint64 InstanceID string + LockGen string } // LoadInstanceGuard 读取守卫标记。没有行时 ok 为 false。 func (s *Store) LoadInstanceGuard() (InstanceGuardMarker, bool, error) { var m InstanceGuardMarker var dbDev, dbIno, lockDev, lockIno int64 - err := s.db.QueryRow(`SELECT db_dev, db_ino, lock_dev, lock_ino, instance_id FROM instance_guard WHERE id=1`). - Scan(&dbDev, &dbIno, &lockDev, &lockIno, &m.InstanceID) + err := s.db.QueryRow(`SELECT db_dev, db_ino, lock_dev, lock_ino, instance_id, lock_gen FROM instance_guard WHERE id=1`). + Scan(&dbDev, &dbIno, &lockDev, &lockIno, &m.InstanceID, &m.LockGen) if errors.Is(err, sql.ErrNoRows) { return InstanceGuardMarker{}, false, nil } @@ -35,12 +36,12 @@ func (s *Store) LoadInstanceGuard() (InstanceGuardMarker, bool, error) { // SaveInstanceGuard 写入或覆盖唯一一行。 func (s *Store) SaveInstanceGuard(m InstanceGuardMarker) error { - _, err := s.db.Exec(s.q(`INSERT INTO instance_guard(id, db_dev, db_ino, lock_dev, lock_ino, instance_id) - VALUES(1,?,?,?,?,?) + _, err := s.db.Exec(s.q(`INSERT INTO instance_guard(id, db_dev, db_ino, lock_dev, lock_ino, instance_id, lock_gen) + VALUES(1,?,?,?,?,?,?) ON CONFLICT(id) DO UPDATE SET db_dev=excluded.db_dev, db_ino=excluded.db_ino, lock_dev=excluded.lock_dev, lock_ino=excluded.lock_ino, - instance_id=excluded.instance_id`), - int64(m.DBDev), int64(m.DBIno), int64(m.LockDev), int64(m.LockIno), m.InstanceID) + instance_id=excluded.instance_id, lock_gen=excluded.lock_gen`), + int64(m.DBDev), int64(m.DBIno), int64(m.LockDev), int64(m.LockIno), m.InstanceID, m.LockGen) return err } diff --git a/internal/store/migrate.go b/internal/store/migrate.go index 7b6f187..29997ec 100644 --- a/internal/store/migrate.go +++ b/internal/store/migrate.go @@ -787,7 +787,8 @@ func init() { db_ino INTEGER NOT NULL, lock_dev INTEGER NOT NULL, lock_ino INTEGER NOT NULL, - instance_id TEXT NOT NULL DEFAULT '' + instance_id TEXT NOT NULL DEFAULT '', + lock_gen TEXT NOT NULL DEFAULT '' )`, }, "postgres": { `CREATE TABLE IF NOT EXISTS instance_guard ( @@ -796,7 +797,8 @@ func init() { db_ino BIGINT NOT NULL, lock_dev BIGINT NOT NULL, lock_ino BIGINT NOT NULL, - instance_id TEXT NOT NULL DEFAULT '' + instance_id TEXT NOT NULL DEFAULT '', + lock_gen TEXT NOT NULL DEFAULT '' )`, }}, down: map[string][]string{"sqlite": {