fix: harden cache retention race checks

Author: chubes4

README.md

@@ -519,7 +519,7 @@ kimaki tunnel -- sh -c 'npm run wp-codebox -- run \
 
 When a caller exposes the local Playground through a tunnel or proxy, pass `--preview-public-url <url>` to report that public URL in `artifacts.preview.url`, `metadata.json`, and `files/review.json`. WP Codebox also passes the same URL to Playground as `site-url` and defines `WP_HOME` / `WP_SITEURL` in the sandbox config, so WordPress-generated links and canonical redirects align with the public preview URL. The local proxy URL remains recorded as `preview.localUrl`. If the fixed port is already occupied, WP Codebox fails clearly with `EADDRINUSE` and the requested `--preview-port` value.
 
-Playground's content-addressed `custom-*.zip` WordPress archives are retained separately from stable release archives and cleaned after runtime use. The custom cache defaults to a seven-day age limit, 1 GiB, and 20 archives. Configure those bounds with `WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_AGE_MS`, `WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_BYTES`, and `WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT`. Active locks and runtimes use token-owned renewable leases; configure their duration with `WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_LEASE_MS` and crashed legacy lock recovery with `WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_STALE_LOCK_MS`. All maintenance uses `WP_CODEBOX_PLAYGROUND_WORDPRESS_CACHE_DIR` when set. The Playground runtime package exports `maintainPlaygroundCustomArchiveCache()` for deterministic `diagnose`, `dry-run`, and `apply` evidence, including filesystem protections, orphan sidecars, logical bytes removed, estimated allocated bytes removed, and the verified filesystem free-space delta.
+Playground's content-addressed `custom-*.zip` WordPress archives are retained separately from stable release archives and cleaned after runtime use. The custom cache defaults to a seven-day age limit, 1 GiB, and 20 archives. Configure those bounds with `WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_AGE_MS`, `WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_BYTES`, and `WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT`. Active locks and runtimes use token-owned renewable leases; configure their duration with `WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_LEASE_MS` and crashed legacy lock recovery with `WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_STALE_LOCK_MS`. All maintenance uses `WP_CODEBOX_PLAYGROUND_WORDPRESS_CACHE_DIR` when set. The Playground runtime package exports `maintainPlaygroundCustomArchiveCache()` for deterministic `diagnose`, `dry-run`, and `apply` evidence, including filesystem protections, orphan sidecars, logical bytes removed, estimated allocated inode bytes removed, and the concurrently observed filesystem free-space delta.
 
 Remote-host previews can opt into `--preview-bind <host>` with `--preview-port`. The flag changes the WP Codebox preview proxy bind address only; the upstream runtime server remains loopback-bound until backend bind-host control is available. The default stays `127.0.0.1`. Use `--preview-bind 0.0.0.0` only behind trusted firewall, tunnel, or reverse-proxy controls because the sandbox preview is reachable for the hold duration.
 

packages/runtime-playground/src/playground-cli-runner.ts

@@ -631,7 +631,13 @@ function errorDetail(error: unknown): Record<string, unknown> {
 
 async function automaticPlaygroundCustomArchiveCacheMaintenance(warnOnFailure = false): Promise<{ result?: PlaygroundCustomArchiveCacheMaintenance; diagnostics: PlaygroundCustomArchiveCacheDiagnostic[] }> {
   try {
-    return { result: await maintainPlaygroundCustomArchiveCache(), diagnostics: [] }
+    const result = await maintainPlaygroundCustomArchiveCache()
+    if (warnOnFailure) {
+      for (const diagnostic of result.diagnostics) {
+        console.warn(`[wp-codebox] ${diagnostic.code}: ${diagnostic.message}`)
+      }
+    }
+    return { result, diagnostics: result.diagnostics }
   } catch (error) {
     const diagnostic = {
       code: "playground-custom-archive-cache-maintenance-failed",

packages/runtime-playground/src/playground-wordpress-archive-cache.ts

@@ -70,6 +70,7 @@ export interface PlaygroundCustomArchiveCachePolicy {
 export interface PlaygroundCustomArchiveCacheMaintenanceOptions extends Partial<PlaygroundCustomArchiveCachePolicy> {
   mode?: "diagnose" | "dry-run" | "apply"
   now?: number
+  candidateInterlock?: (archivePath: string) => void | Promise<void>
 }
 
 export interface PlaygroundCustomArchiveCacheDiagnostic {
@@ -105,7 +106,7 @@ export interface PlaygroundCustomArchiveCacheMaintenance {
   removedCount: number
   removedBytes: number
   estimatedAllocatedBytesRemoved: number
-  verifiedReclaimedBytes: number
+  observedFilesystemFreeBytesDelta: number
   retainedCount: number
   retainedBytes: number
   diagnostics: PlaygroundCustomArchiveCacheDiagnostic[]
@@ -115,6 +116,8 @@ interface CustomArchiveEntry {
   path: string
   size: number
   mtimeMs: number
+  dev: number
+  ino: number
   lockPath: string
   activeLock: boolean
   activeReferences: number
@@ -211,6 +214,8 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play
         path: archivePath,
         size: regular ? archiveStat.size : 0,
         mtimeMs: archiveStat.mtimeMs,
+        dev: archiveStat.dev,
+        ino: archiveStat.ino,
         lockPath,
         activeLock,
         activeReferences,
@@ -246,6 +251,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play
   const filesystemBefore = mode === "apply" ? await filesystemAvailableBytes(cacheDirectory) : undefined
   if (mode === "apply") {
     for (const candidate of candidates.sort(oldestFirst)) {
+      await options.candidateInterlock?.(candidate.path)
       const lease = await tryAcquireCacheLock(candidate.lockPath, policy.leaseMs)
       if (!lease) {
         continue
@@ -267,6 +273,10 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play
           diagnostics.push({ code: "custom-archive-changed-before-removal", message: "Custom archive changed to an unsafe filesystem entry before removal and was protected.", severity: "warning", path: candidate.path })
           continue
         }
+        if (current.dev !== candidate.dev || current.ino !== candidate.ino || current.mtimeMs !== candidate.mtimeMs || current.size !== candidate.size) {
+          diagnostics.push({ code: "custom-archive-generation-changed", message: "Custom archive was replaced or modified after candidate selection and was protected from removal.", severity: "warning", path: candidate.path })
+          continue
+        }
         const currentAllocatedBytes = allocatedBytes(current.blocks)
         await unlink(candidate.path)
         try {
@@ -289,7 +299,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play
 
   const sidecars = await maintainOrphanSidecars(cacheDirectory, directoryEntries.map((entry) => entry.name), now, policy, mode, diagnostics)
   const filesystemAfter = mode === "apply" ? await filesystemAvailableBytes(cacheDirectory) : undefined
-  const verifiedReclaimedBytes = filesystemBefore === undefined || filesystemAfter === undefined ? 0 : Math.max(0, filesystemAfter - filesystemBefore)
+  const observedFilesystemFreeBytesDelta = filesystemBefore === undefined || filesystemAfter === undefined ? 0 : filesystemAfter - filesystemBefore
   const totalBytes = entries.reduce((total, entry) => total + entry.size, 0)
   return {
     schema: "wp-codebox/playground-custom-archive-cache-maintenance/v1",
@@ -312,7 +322,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play
     removedCount,
     removedBytes,
     estimatedAllocatedBytesRemoved,
-    verifiedReclaimedBytes,
+    observedFilesystemFreeBytesDelta,
     retainedCount: entries.length - removedCount,
     retainedBytes: totalBytes - removedBytes,
     diagnostics,
@@ -440,7 +450,7 @@ async function cacheLockIsActive(lockPath: string, now: number, policy: Pick<Pla
         }
         continue
       }
-      const state = await leaseState(path, path, now, policy.staleLockMs)
+      const state = await inspectLeaseFile(path, now, policy.staleLockMs, removeStale)
       if (state.active) {
         active = true
       } else if (removeStale) {
@@ -482,13 +492,9 @@ async function activeArchiveReferenceCount(archivePath: string, now: number, pol
   try {
     for (const name of names.sort()) {
       const path = join(anchoredDirectory, name)
-      const state = await leaseState(path, path, now, policy.staleLockMs)
+      const state = await inspectLeaseFile(path, now, policy.staleLockMs, removeStale, diagnostics)
       if (state.active) {
         active += 1
-      } else if (removeStale) {
-        await unlink(path).catch((error) => {
-          if (!errorHasCode(error, "ENOENT")) throw error
-        })
       }
     }
   } finally {
@@ -497,20 +503,47 @@ async function activeArchiveReferenceCount(archivePath: string, now: number, pol
   return active
 }
 
-async function leaseState(ownerPath: string, sidecarPath: string, now: number, legacyStaleMs: number): Promise<LeaseState> {
-  const owner = await readOwnerRecord(ownerPath)
-  if (owner) {
-    const expiresAt = Date.parse(owner.expiresAt)
-    return { active: Number.isFinite(expiresAt) && expiresAt > now, token: owner.token }
+async function inspectLeaseFile(path: string, now: number, legacyStaleMs: number, removeUnsafe: boolean, diagnostics: PlaygroundCustomArchiveCacheDiagnostic[] = []): Promise<LeaseState> {
+  let pathStat
+  try {
+    pathStat = await lstat(path)
+  } catch (error) {
+    if (errorHasCode(error, "ENOENT")) return { active: false }
+    throw error
   }
+  if (!pathStat.isFile() || pathStat.isSymbolicLink() || pathStat.nlink !== 1) {
+    diagnostics.push({ code: "custom-archive-ref-entry-unsafe", message: "Archive lease entry is not a singly-linked regular file and was not followed.", severity: "warning", path })
+    if (removeUnsafe && !pathStat.isDirectory()) await unlink(path)
+    return { active: !removeUnsafe || pathStat.isDirectory() }
+  }
+
+  let handle: FileHandle | undefined
   try {
-    const sidecarStat = await lstat(sidecarPath)
-    return { active: now - sidecarStat.mtimeMs <= legacyStaleMs }
+    handle = await open(path, constants.O_RDONLY | constants.O_NOFOLLOW)
+    const openedStat = await handle.stat()
+    if (openedStat.dev !== pathStat.dev || openedStat.ino !== pathStat.ino) {
+      diagnostics.push({ code: "custom-archive-ref-entry-changed", message: "Archive lease entry changed while opening and was conservatively protected.", severity: "warning", path })
+      return { active: true }
+    }
+    const owner = await readOwnerFromHandle(handle)
+    if (owner) {
+      const expiresAt = Date.parse(owner.expiresAt)
+      if (Number.isFinite(expiresAt) && expiresAt > now) return { active: true, token: owner.token }
+      if (removeUnsafe) await unlink(path)
+      return { active: false, token: owner.token }
+    }
+    const active = now - pathStat.mtimeMs <= legacyStaleMs
+    if (!active && removeUnsafe) await unlink(path)
+    return { active }
   } catch (error) {
-    if (errorHasCode(error, "ENOENT")) {
-      return { active: false }
+    if (errorHasCode(error, "ENOENT")) return { active: false }
+    if (errorHasCode(error, "ELOOP")) {
+      diagnostics.push({ code: "custom-archive-ref-entry-unsafe", message: "Archive lease entry became a symlink and was conservatively protected.", severity: "warning", path })
+      return { active: true }
     }
     throw error
+  } finally {
+    await handle?.close()
   }
 }
 
@@ -534,7 +567,7 @@ async function maintainOrphanSidecars(cacheDirectory: string, names: string[], n
       }
       continue
     }
-    const active = await activeReferenceCountInDirectory(path, now, policy, mode === "apply")
+    const active = await activeReferenceCountInDirectory(path, now, policy, mode === "apply", diagnostics)
     if (active > 0) {
       activeCount += 1
     } else if (mode === "apply" && !await pathExists(path)) {
@@ -546,8 +579,8 @@ async function maintainOrphanSidecars(cacheDirectory: string, names: string[], n
   return { orphanCount: paths.length, activeCount, removedCount, paths }
 }
 
-async function activeReferenceCountInDirectory(path: string, now: number, policy: Pick<PlaygroundCustomArchiveCachePolicy, "staleLockMs">, removeStale: boolean): Promise<number> {
-  return activeArchiveReferenceCount(path.slice(0, -".refs".length), now, policy, removeStale)
+async function activeReferenceCountInDirectory(path: string, now: number, policy: Pick<PlaygroundCustomArchiveCachePolicy, "staleLockMs">, removeStale: boolean, diagnostics: PlaygroundCustomArchiveCacheDiagnostic[] = []): Promise<number> {
+  return activeArchiveReferenceCount(path.slice(0, -".refs".length), now, policy, removeStale, diagnostics)
 }
 
 async function removeOrphanReferencesDirectory(path: string, now: number, policy: Pick<PlaygroundCustomArchiveCachePolicy, "staleLockMs">, diagnostics: PlaygroundCustomArchiveCacheDiagnostic[]): Promise<boolean> {
@@ -563,7 +596,7 @@ async function removeOrphanReferencesDirectory(path: string, now: number, policy
     await unlink(path)
     return true
   }
-  if (await activeReferenceCountInDirectory(path, now, policy, true) > 0) {
+  if (await activeReferenceCountInDirectory(path, now, policy, true, diagnostics) > 0) {
     return false
   }
   try {
@@ -578,28 +611,6 @@ async function removeOrphanReferencesDirectory(path: string, now: number, policy
   }
 }
 
-async function readOwnerRecord(path: string): Promise<CacheOwnerRecord | undefined> {
-  try {
-    const value = JSON.parse(await readFile(path, "utf8")) as Partial<CacheOwnerRecord>
-    return value.schema === "wp-codebox/playground-cache-lease/v1"
-      && typeof value.token === "string"
-      && typeof value.hostname === "string"
-      && typeof value.bootId === "string"
-      && Number.isInteger(value.pid)
-      && typeof value.processStart === "string"
-      && typeof value.createdAt === "string"
-      && typeof value.heartbeatAt === "string"
-      && typeof value.expiresAt === "string"
-      ? value as CacheOwnerRecord
-      : undefined
-  } catch (error) {
-    if (errorHasCode(error, "ENOENT") || errorHasCode(error, "EISDIR") || error instanceof SyntaxError) {
-      return undefined
-    }
-    throw error
-  }
-}
-
 async function openSafeLeaseDirectory(path: string, create: boolean): Promise<FileHandle> {
   if (create) {
     await mkdir(path).catch((error) => {

tests/playground-custom-archive-cache.integration.test.ts

@@ -1,7 +1,7 @@
 import assert from "node:assert/strict"
 import { existsSync } from "node:fs"
 import { createServer } from "node:http"
-import { mkdtemp, readdir, rm, utimes, writeFile } from "node:fs/promises"
+import { mkdtemp, readdir, rm, symlink, utimes, writeFile } from "node:fs/promises"
 import { tmpdir } from "node:os"
 import { join } from "node:path"
 
@@ -61,11 +61,13 @@ try {
   assert.equal((cacheValidation?.retention as Record<string, unknown> | undefined)?.schema, "wp-codebox/playground-custom-archive-cache-maintenance/v1")
   assert.equal((cacheValidation?.retention as Record<string, unknown>).removedCount, 1)
   assert.equal(((cacheValidation?.retention as Record<string, unknown>).activeProtection as Record<string, unknown>).referenceCount, 0)
+  console.warn = (message?: unknown) => { warnings.push(String(message)) }
+  await symlink(root, join(root, "custom-teardown-warning.zip.refs"))
   await server[Symbol.asyncDispose]()
+  assert.ok(warnings.some((warning) => warning.includes("custom-archive-refs-not-directory")), "successful teardown maintenance diagnostics must emit bounded warning evidence")
   assert.deepEqual((await readdir(root)).filter((name) => /^custom-.*\.zip(?:\.(?:refs|lock))?$/.test(name)), [], "teardown must release and retain no zero-bound custom cache entries or sidecars")
 
   process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT = "invalid"
-  console.warn = (message?: unknown) => { warnings.push(String(message)) }
   const invalidProgress: Array<Record<string, unknown>> = []
   const invalidServer = await startPlaygroundCliServer(spec, [], { cliModule, onProgress(event) { invalidProgress.push(event as unknown as Record<string, unknown>) } })
   await new Promise((resolve) => setTimeout(resolve, 0))

tests/playground-custom-archive-cache.test.ts

@@ -52,7 +52,7 @@ try {
   assert.equal(applied.removedCount, 2)
   assert.equal(applied.removedBytes, 24)
   assert.equal(applied.estimatedAllocatedBytesRemoved, staleAllocated)
-  assert.ok(applied.verifiedReclaimedBytes >= 0)
+  assert.ok(Number.isFinite(applied.observedFilesystemFreeBytesDelta))
   assert.deepEqual(await customArchiveNames(), ["custom-locked.zip", "custom-referenced.zip"])
   assert.ok((await readdir(root)).includes("6.9.1.zip"), "stable release archive must remain intact")
 
@@ -104,6 +104,18 @@ try {
   assert.equal(await readFile(attackMarker, "utf8"), "outside cache")
   await rm(attackTarget, { recursive: true, force: true })
 
+  const childAttackTarget = await mkdtemp(join(tmpdir(), "wp-codebox-playground-ref-child-attack-"))
+  const externalLease = join(childAttackTarget, "external-lease.json")
+  await writeLease(externalLease, { token: "external-symlink-lease", expiresAt: now + 60_000, ownerHostname: "outside-cache" })
+  const childAttackArchive = await archive("custom-ref-child-attack.zip", 42, now - 100_000)
+  await mkdir(`${childAttackArchive}.refs`)
+  await symlink(externalLease, join(`${childAttackArchive}.refs`, "malicious.json"))
+  const childAttackCleanup = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1 })
+  assert.ok(childAttackCleanup.diagnostics.some((item) => item.code === "custom-archive-ref-entry-unsafe"))
+  assert.ok(!await exists(childAttackArchive), "unsafe child lease entries must not protect stale archives")
+  assert.ok(await exists(externalLease), "lease child inspection must never follow or remove an external symlink target")
+  await rm(childAttackTarget, { recursive: true, force: true })
+
   const generationArchive = await archive("custom-generation-race.zip", 43, now - 100_000)
   const oldGeneration = await acquirePlaygroundArchiveReference(generationArchive, { leaseMs: 100, heartbeat: false })
   await new Promise((resolve) => setTimeout(resolve, 125))
@@ -117,6 +129,22 @@ try {
   assert.ok(await exists(replacementGeneration.path), "old generation release must not remove replacement ownership")
   await replacementGeneration.release()
 
+  const replacedCandidate = await archive("custom-replaced-candidate.zip", 47, now - 100_000)
+  let replaced = false
+  const replacementResult = await maintainPlaygroundCustomArchiveCache(root, {
+    mode: "apply",
+    now,
+    maxAgeMs: 1,
+    async candidateInterlock(path) {
+      if (path !== replacedCandidate || replaced) return
+      replaced = true
+      await rm(path)
+      await writeFile(path, "fresh replacement generation")
+    },
+  })
+  assert.equal(await readFile(replacedCandidate, "utf8"), "fresh replacement generation")
+  assert.ok(replacementResult.diagnostics.some((item) => item.code === "custom-archive-generation-changed"), "replacement generation must be revalidated after lock acquisition")
+
   const allocationRoot = await mkdtemp(join(tmpdir(), "wp-codebox-playground-allocation-"))
   const openArchive = join(allocationRoot, "custom-open-file.zip")
   await writeFile(openArchive, Buffer.alloc(1024 * 1024, "a"))
@@ -125,7 +153,7 @@ try {
   const openRemoval = await maintainPlaygroundCustomArchiveCache(allocationRoot, { mode: "apply", maxAgeMs: 1 })
   assert.equal(openRemoval.removedBytes, 1024 * 1024)
   assert.ok(openRemoval.estimatedAllocatedBytesRemoved >= 1024 * 1024)
-  assert.equal(openRemoval.verifiedReclaimedBytes, 0, "open unlinked allocation is not verified as reclaimed until the final handle closes")
+  assert.equal(openRemoval.observedFilesystemFreeBytesDelta, 0, "open unlinked allocation should not increase observed free space until the final handle closes")
   await openHandle.close()
   await rm(allocationRoot, { recursive: true, force: true })