From 0cb59cf3fdb8e9c81eabbf08d160f1d073c07a24 Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Sat, 22 Aug 2026 13:50:40 -0400
Subject: [PATCH] fix(auth): stop heartbeating a lock that is no longer ours
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Found on a re-read of the round-4 liveness work, not by review.

The heartbeat refreshed the lock directory's mtime unconditionally. After
a steal — possible when this process looked dead, or across hosts where
the pid check cannot apply — the directory belongs to someone else, and
touching it keeps *their* lock looking alive. If they then die, the stale
timestamp nobody is maintaining is being maintained by us, and a third
writer can never break in: a stale lock turned unbreakable, which is the
permanent outage the stealing exists to avoid.

The heartbeat now re-reads the owner stamp and clears itself when the
token no longer matches, so it only ever refreshes a lock we still hold.
The release path already checked the token; this closes the same hole on
the other side.

Tested by stamping a different owner onto a held lock and aging it: the
mtime stays old across several heartbeat intervals, and release leaves the
thief's lock intact.

Signed-off-by: cliffhall <cliff@futurescale.com>
---
 .../auth/node/file-secret-store.test.ts       | 26 +++++++++++++++++++
 core/auth/node/file-secret-store.ts           | 23 +++++++++++++---
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/clients/web/src/test/integration/auth/node/file-secret-store.test.ts b/clients/web/src/test/integration/auth/node/file-secret-store.test.ts
index 12e2b97e..bd2c9820 100644
--- a/clients/web/src/test/integration/auth/node/file-secret-store.test.ts
+++ b/clients/web/src/test/integration/auth/node/file-secret-store.test.ts
@@ -879,6 +879,32 @@ describe("acquireLock", () => {
     await release();
   });
 
+  it("stops heartbeating a lock that is no longer ours", async () => {
+    // After a steal, the directory belongs to someone else. Continuing to
+    // touch it would keep *their* lock looking alive after they died, which
+    // turns a stale lock into an unbreakable one.
+    const target = path.join(tmpDir, "stolen.json");
+    const lockDir = `${target}.lock`;
+    const release = await acquireLock(target, { heartbeatMs: 10 });
+    // Someone judges us dead and takes it.
+    await fs.writeFile(
+      path.join(lockDir, "owner"),
+      JSON.stringify({ token: "thief", pid: process.pid, host: os.hostname() }),
+      "utf-8",
+    );
+    const afterSteal = new Date(Date.now() - 30_000);
+    await fs.utimes(lockDir, afterSteal, afterSteal);
+    // Give the heartbeat several chances to (wrongly) refresh it.
+    await new Promise((r) => setTimeout(r, 80));
+    const mtime = (await fs.stat(lockDir)).mtimeMs;
+    expect(Date.now() - mtime).toBeGreaterThan(1_000);
+    // And our release leaves their lock alone, since the token no longer
+    // matches.
+    await release();
+    expect(existsSync(lockDir)).toBe(true);
+    await fs.rm(lockDir, { recursive: true, force: true });
+  });
+
   it("cleans up and reports when it cannot stamp ownership", async () => {
     // mkdir succeeded, the owner write did not. Returning a no-op release
     // here would enter the critical section unlocked *and* leave the
diff --git a/core/auth/node/file-secret-store.ts b/core/auth/node/file-secret-store.ts
index 1c0b169f..9b954a91 100644
--- a/core/auth/node/file-secret-store.ts
+++ b/core/auth/node/file-secret-store.ts
@@ -336,10 +336,25 @@ export async function acquireLock(
       // never keeps the process alive on its own — the interval is a
       // side-channel, not work anyone is waiting for.
       const heartbeat = setInterval(() => {
-        const now = new Date();
-        void fs.utimes(lockDir, now, now).catch(() => {
-          // Lock already released or stolen; the release below copes.
-        });
+        // Confirm the lock is still ours before refreshing it. If it was
+        // stolen (only possible when we looked dead, or across hosts where
+        // the pid check cannot apply), the directory now belongs to someone
+        // else — and touching it would keep *their* lock looking alive after
+        // they died, which is how a stale lock becomes an unbreakable one.
+        void (async () => {
+          try {
+            const held = parseOwner(await fs.readFile(ownerFile, "utf-8"));
+            if (held?.token !== token) {
+              clearInterval(heartbeat);
+              return;
+            }
+            const now = new Date();
+            await fs.utimes(lockDir, now, now);
+          } catch {
+            // Released, stolen, or unreadable. The release below copes, and
+            // there is nothing useful to do from a timer.
+          }
+        })();
       }, heartbeatMs);
       heartbeat.unref?.();
       return async () => {