[Security] KubernetesLease.release() deletes the Lease object unconditionally, so a holder stepping down on a stale held flag can delete its successor's record #1659

Component

src/coordination/leases/KubernetesLease.ts — release()

Severity (assessment)

Medium. CWE-667 (improper locking), the same family as #937.

Problem

release() deletes the Lease object by name, with no precondition:

src/coordination/leases/KubernetesLease.ts:591-598 (branch of #1658)
  async release(): Promise<void> {
    if (!this.held) return;
    this.held = false;
    this.stopRenewalLoop();
    this.currentLease = null;
    await this.withFreshCredentials((credentials) =>
      deleteLease(credentials, this.namespaceOf(credentials), this.leaseName, this.callOptions));
  }

The only gate is the local held flag. After a stall longer than the TTL that flag is stale until the next renewal tick reports the loss — #937 / #1658 make checkAlive() answer false in that window, but release() does not consult it. InMemoryLease does not have this problem: its store only deletes a record whose owner matches (InMemoryLease.ts:53-56).

Exploit walkthrough

No attacker needed — a GC pause or a throttled container is enough.

1. Pod A holds the lease with TTL T.
2. A's event loop stalls for longer than T. Its renewal tick does not run, so held stays true.
3. Pod B takes the lapsed record over legitimately (PUT with holderIdentity: B).
4. A resumes. Before its renewal tick fires onLost, a consumer on A calls release() — ClusterSingletonManager stepping down on a leader change, a ShardCoordinator or PersistentActor stopping.
5. release() sees held === true and DELETEs the object — B's record.
6. B still believes it holds the lease until its next renewal finds the object gone (up to one renewal interval, longer while a PUT hangs). In that window any pod, A included, can CREATE a fresh record and hold it alongside B.

Affected files

• src/coordination/leases/KubernetesLease.ts (release())
• src/coordination/leases/K8sApi.ts (deleteLease — currently sends no DeleteOptions)

Fix sketch

1. Skip the DELETE once checkAlive() is false: past the deadline the record may already belong to someone else, and leaving a record that is still ours to expire costs at most one TTL.
2. Make the DELETE conditional: send DeleteOptions.preconditions with the resourceVersion (or uid) of currentLease, so the API server refuses to delete a record that moved since this holder last wrote it. A 409 then means "no longer ours" — release is a no-op — and 404 stays success.

Either closes the race; doing both is cheap.

Acceptance criteria

• A holder whose TTL ran out does not DELETE the record on release().
• release() never deletes a record whose holderIdentity is not this owner (FakeK8sServer test: a successor's record survives the stale holder's release()).
• The DELETE carries a precondition; a precondition conflict is reported as "not held", not as a failed release.
• Docs (EN + DE) for release() updated.

Context

Found by the independent verification of #1658 (the fix for #937), round 5, and confirmed by the PR review. Pre-existing; out of that PR's scope. Related: #937, #600, #761.

Disclosure status

Already public (found in an open PR's review).