Skip to content

[Security] Lease.checkAlive() returns a cached boolean instead of comparing against expiresAt, and has no callers, so two nodes can both believe they hold the lease after an event-loop stall #937

Description

@pathosDev

Component: src/coordination/leases/InMemoryLease.ts, src/coordination/leases/KubernetesLease.ts
Severity (assessment): HIGH
CWE: CWE-667 (improper locking)

Lease mutual exclusion fails open after an event-loop stall. checkAlive() returns a cached boolean and is never compared against expiresAt, in both backends — so a holder whose renewal timer was starved reports itself as still holding a lease that another node has already legitimately taken.

Worse, checkAlive() has zero callers in src/. LeaseMajority — the split-brain resolver whose whole purpose is to arbitrate a partition — never re-validates the lease its decision rests on. The decision is cached for the life of the partition fingerprint (src/cluster/downing/LeaseMajority.ts:161-164), so a node that has silently lost its lease keeps downing the side that legitimately holds it.

Exploit walkthrough

No attacker required — a GC pause, a CPU-starved container, or a noisy neighbour is sufficient. Attacker position: none; this is a liveness-triggered safety failure.

  1. Node A acquires the lease with TTL T.
  2. A's event loop stalls for longer than T (GC, a synchronous parse — see the CBOR issue in this batch for a remote way to cause exactly this).
  3. A's renewal timer does not run; the lease record expires.
  4. Node B acquires it legitimately.
  5. A resumes. A.checkAlive() still returns true, because it is a field, not a check.

Both nodes now believe they hold the lease. Under LeaseMajority, both partitions believe they are the survivor.

Evidence

Identical in both backends:

src/coordination/leases/InMemoryLease.ts:106
  checkAlive(): boolean { return this.held; }
src/coordination/leases/KubernetesLease.ts:209
  checkAlive(): boolean { return this.held; }

A repo-wide grep for checkAlive returns only these two definitions and the Lease interface declaration — no call sites in src/ at all.

Why the existing guard does not cover it

The repo already detected this and mis-attributed it. tests/multi-node/lease-majority.test.ts:29-35 disables itself with:

the lease holder's renewal timer is delayed past the TTL, so both sides acquire… Not reproducible locally

That is a correct detection of a real defect, filed as a runner limitation. A GC pause or a throttled container reproduces it in production; the hosted runner was simply the first place slow enough to hit it.

#600 covers a different lease defect (release() no-ops while an acquire is in flight). Neither it nor #598 addresses the cached-liveness field.

Suggested fix

  1. Compare against the record: checkAlive() returns this.held && Date.now() < this.expiresAt, and renew() refuses on an expired record rather than extending one it no longer owns.
  2. Call it. LeaseMajority must re-validate before acting on a cached decision, and must drop the decision if the lease is no longer alive.
  3. Re-enable tests/multi-node/lease-majority.test.ts — it was right.

Acceptance criteria

  • After a stall longer than the TTL, the previous holder reports checkAlive() === false.
  • Two holders cannot both report alive for the same lease name.
  • LeaseMajority re-validates the lease before acting on a cached decision.
  • renew() on an expired record fails rather than silently re-acquiring.
  • The quarantined multi-node test runs and passes.

Verification status

Found in the ten-lens production-readiness review of 2026-08-05 (v0.13.0) and re-verified before filing: confirmed by reading — both one-line implementations are quoted verbatim, and the absence of callers is a repo-wide grep. The stall scenario was not reproduced here because the quarantined test already reproduces it on a loaded runner.

Part of the production-readiness review batch — tracked in #913.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: highTop priority — high impact, plan nextproduction-goalBlocks or defines the path to production readinesssecuritySecurity-relevant — see severity label for impact tierseverity: highSignificant impact, exploitable in standard threat model

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions