🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

1. 🟠 Race condition: orphan-self-pid reclaim can delete a freshly created lock and allow double acquisition

Description

The new shouldTreatAsOrphanSelfLock logic can force a contended lock to be treated as stale: true purely because the lock payload has pid === process.pid and missing/invalid starttime, even if the lock is fresh and the owning process is alive.

Because acquireSessionWriteLock() passes these forced reclaimDetails into shouldReclaimContendedLockFile(), this bypasses the mtime-based safety fallback in shouldReclaimContendedLockFile() and causes immediate deletion of the lock file during contention.

Impact scenarios:

• Concurrent acquisitions in the same process (race window): if getProcessStartTime() returns null (non-Linux platforms or environments where /proc is unavailable), newly created locks will have no starttime. A second concurrent call that hits EEXIST before HELD_LOCKS is populated will:

  • read a valid JSON payload with pid + createdAt but no starttime
  • classify it as orphanSelfLock
  • delete the lock file even though the first call still holds an open handle
  • then create a new lock file, resulting in two callers simultaneously believing they hold the lock, risking session store corruption.

• PID-namespace collisions (containers + shared volume): a lock file created by another live container/process using the same PID (common PID=1) but missing starttime (legacy lock format) can be reclaimed by a different container instance, breaking mutual exclusion.

Vulnerable behavior is introduced by forcing stale: true + adding a non-fallback staleReason ("orphan-self-pid"), which makes shouldReclaimContendedLockFile() return true immediately when contended.

Recommendation

Do not immediately reclaim/delete a contended lock solely because pid === process.pid and starttime is missing.

Safer options:

1. Re-check for in-process ownership during the contention loop (fixes the race where HELD_LOCKS becomes populated after another concurrent call acquires the lock):

​// inside the while loop, before attempting fs.open or right after EEXIST
if (allowReentrant) {
  const heldNow = HELD_LOCKS.get(normalizedSessionFile);
  if (heldNow) {
    heldNow.count += 1;
    return { release: async () => await releaseHeldLock(normalizedSessionFile, heldNow) };
  }
}

2. If you still need legacy reclamation, gate it on age rather than forcing immediate stale:

• require lock mtime/createdAt to be older than some minimum (or older than the current process start wall-clock time derived from process.uptime()), and/or
• keep the mtime-fallback path by treating this condition as requiring additional validation, not unconditional staleness.

This prevents deleting a freshly created lock that is merely missing starttime due to platform/proc limitations or transient acquisition timing.

Analyzed PR: #32081 at commit 960ee03

_{Last updated on: 2026-03-02T20:18:01Z}

Greptile Summary

Added logic to reclaim orphan lock files when the lock file's PID matches the current process but lacks a valid starttime field and is not actively held in-process.

Key changes:

• Introduced shouldTreatAsOrphanSelfLock() to detect legacy lock files from previous process instances with recycled PIDs
• Modified lock acquisition to mark orphan same-PID locks as stale and reclaim them automatically
• Preserved safety by checking HELD_LOCKS to prevent reclaiming actively-held locks even when starttime is missing or malformed

Testing:

• Updated existing test to verify orphan lock reclamation succeeds
• Added regression tests ensuring active in-process locks are never reclaimed, even with missing/malformed starttime
• All scenarios (orphan detection, active lock protection, PID recycling) covered by comprehensive test cases

Confidence Score: 5/5

• This PR is safe to merge with high confidence
• The fix is well-targeted, addressing a specific edge case (orphan same-PID legacy locks) with clear conditions. The logic correctly distinguishes between orphan locks and actively-held locks using the HELD_LOCKS registry. Comprehensive test coverage includes both the fix scenario and regression tests for active lock protection. The implementation preserves existing PID-recycling detection and adds an additional safety check without changing lock file format or timeout behavior.
• No files require special attention

_{Last reviewed commit: a1e919b}

Landed via temp rebase onto main.

• Gate: pnpm -s vitest run src/agents/session-write-lock.test.ts
• Land commit: 960ee03
• Merge commit: 0cf533a

Thanks @bmendonca3!