From 498e4395c5322ab4dbd97f8226b5534065bf442c Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Tue, 21 Jul 2026 13:42:25 +0000 Subject: [PATCH 1/6] fix: harden Playground cache retention leases --- .github/workflows/agent-task-contracts.yml | 9 + README.md | 2 +- package.json | 2 +- .../src/playground-cli-runner.ts | 32 +- .../src/playground-wordpress-archive-cache.ts | 450 ++++++++++++++---- .../fixtures/playground-cache-lease-child.ts | 29 ++ ...round-custom-archive-cache-process.test.ts | 111 +++++ ...d-custom-archive-cache.integration.test.ts | 81 ++++ tests/playground-custom-archive-cache.test.ts | 200 +++++--- 9 files changed, 768 insertions(+), 148 deletions(-) create mode 100644 tests/fixtures/playground-cache-lease-child.ts create mode 100644 tests/playground-custom-archive-cache-process.test.ts create mode 100644 tests/playground-custom-archive-cache.integration.test.ts diff --git a/.github/workflows/agent-task-contracts.yml b/.github/workflows/agent-task-contracts.yml index 567b14d8..3c7cabac 100644 --- a/.github/workflows/agent-task-contracts.yml +++ b/.github/workflows/agent-task-contracts.yml @@ -24,6 +24,10 @@ on: - "tests/production-boundary-enforcement.test.ts" - "tests/runtime-tool-policy.test.ts" - "packages/runtime-playground/src/phpunit-command-handlers.ts" + - "packages/runtime-playground/src/playground-cli-runner.ts" + - "packages/runtime-playground/src/playground-wordpress-archive-cache.ts" + - "tests/playground-custom-archive-cache*.test.ts" + - "tests/fixtures/playground-cache-lease-child.ts" - "tests/playground-phpunit-readonly-cache.integration.test.ts" - "tests/fixtures/phpunit-playground-harness/**" - "packages/cli/src/bounded-recipe-plan.ts" @@ -53,6 +57,10 @@ on: - "tests/production-boundary-enforcement.test.ts" - "tests/runtime-tool-policy.test.ts" - "packages/runtime-playground/src/phpunit-command-handlers.ts" + - "packages/runtime-playground/src/playground-cli-runner.ts" + - "packages/runtime-playground/src/playground-wordpress-archive-cache.ts" + - "tests/playground-custom-archive-cache*.test.ts" + - "tests/fixtures/playground-cache-lease-child.ts" - "tests/playground-phpunit-readonly-cache.integration.test.ts" - "tests/fixtures/phpunit-playground-harness/**" - "packages/cli/src/bounded-recipe-plan.ts" @@ -84,6 +92,7 @@ jobs: - run: npm run test:disposable-mysql-mysqli-e2e - run: npm run test:runtime-sources-playground-integration - run: npm run test:playground-phpunit-readonly-cache-integration + - run: npm run test:playground-custom-archive-cache - run: npm run test:native-agent-task-playground-e2e - run: npm run test:native-agent-task-interruption - run: npm run test:trusted-apply-artifact-channel diff --git a/README.md b/README.md index 883d503c..5de6d2f6 100644 --- a/README.md +++ b/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 ` 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`; configure 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. +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, and allocated bytes verified as reclaimed. Remote-host previews can opt into `--preview-bind ` 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. diff --git a/package.json b/package.json index 714288fe..05fbae67 100644 --- a/package.json +++ b/package.json @@ -108,7 +108,7 @@ "test:playground-readonly-mounts-integration": "tsx tests/playground-readonly-mounts-integration.test.ts", "test:playground-phpunit-readonly-cache-integration": "tsx tests/playground-phpunit-readonly-cache.integration.test.ts", "test:playground-phpunit-bootstrap-failure-integration": "tsx tests/playground-phpunit-bootstrap-failure.integration.test.ts", - "test:playground-custom-archive-cache": "tsx tests/playground-custom-archive-cache.test.ts", + "test:playground-custom-archive-cache": "tsx tests/playground-custom-archive-cache.test.ts && tsx tests/playground-custom-archive-cache-process.test.ts && tsx tests/playground-custom-archive-cache.integration.test.ts", "test:phpunit-runtime-failure-diagnostics": "tsx tests/phpunit-runtime-failure-diagnostics.test.ts", "test:php-wasm-extension-manifests": "tsx tests/php-wasm-extension-manifests.test.ts", "test:browser-task-builder": "tsx tests/browser-task-builder.test.ts", diff --git a/packages/runtime-playground/src/playground-cli-runner.ts b/packages/runtime-playground/src/playground-cli-runner.ts index 1ce71626..63c6d70c 100644 --- a/packages/runtime-playground/src/playground-cli-runner.ts +++ b/packages/runtime-playground/src/playground-cli-runner.ts @@ -13,7 +13,7 @@ import * as PlaygroundStorage from "@wp-playground/storage" import { resolveWordPressRelease } from "@wp-playground/wordpress" import { phpEnvAssignments, phpLiteral, phpWpConfigDefineAssignments } from "./php-snippets.js" import { stageReadonlyPlaygroundMounts, type ReadonlyMountStaging } from "./mount-materialization.js" -import { acquirePlaygroundArchiveReference, isCustomPlaygroundWordPressArchive, maintainPlaygroundCustomArchiveCache, playgroundWordPressArchiveCacheDirectory, withPlaygroundArchiveCacheLock, type PlaygroundArchiveReference, type PlaygroundCustomArchiveCacheMaintenance } from "./playground-wordpress-archive-cache.js" +import { acquirePlaygroundArchiveReference, isCustomPlaygroundWordPressArchive, maintainPlaygroundCustomArchiveCache, playgroundWordPressArchiveCacheDirectory, withPlaygroundArchiveCacheLock, type PlaygroundArchiveReference, type PlaygroundCustomArchiveCacheDiagnostic, type PlaygroundCustomArchiveCacheMaintenance } from "./playground-wordpress-archive-cache.js" const DEFAULT_RUNTIME_PHP_INI_ENTRIES = { memory_limit: "512M" } @@ -66,6 +66,7 @@ export async function startPlaygroundCliServer(spec: RuntimeCreateSpec, mounts: let readonlyMountStaging: ReadonlyMountStaging | undefined let archiveReference: PlaygroundArchiveReference | undefined let cacheMaintenance: PlaygroundCustomArchiveCacheMaintenance | undefined + let cacheMaintenanceDiagnostics: PlaygroundCustomArchiveCacheDiagnostic[] = [] let usesArchiveCache = false try { if (spec.preview?.port) { @@ -92,7 +93,9 @@ export async function startPlaygroundCliServer(spec: RuntimeCreateSpec, mounts: const wordpressStartupAsset = wordpressDirectory ? undefined : await resolvePlaygroundWordPressStartupAsset(spec.environment.version, spec.environment.assets?.wordpressZip) archiveReference = wordpressStartupAsset?.archiveReference if (usesArchiveCache) { - cacheMaintenance = await maintainPlaygroundCustomArchiveCache().catch(() => undefined) + const maintenance = await automaticPlaygroundCustomArchiveCacheMaintenance() + cacheMaintenance = maintenance.result + cacheMaintenanceDiagnostics = maintenance.diagnostics } const cacheValidation = wordpressStartupAsset?.cacheValidation ?? { version: spec.environment.version ?? "mounted-wordpress-source", @@ -103,6 +106,9 @@ export async function startPlaygroundCliServer(spec: RuntimeCreateSpec, mounts: if (cacheMaintenance) { cacheValidation.retention = cacheMaintenance } + if (cacheMaintenanceDiagnostics.length > 0) { + cacheValidation.retentionDiagnostics = cacheMaintenanceDiagnostics + } const blueprintSummary = summarizeBlueprint(spec.environment.blueprint) if (blueprintSummary.steps > 0) { emitProgress("preview:applying-blueprint", "running", "Applying site setup", blueprintSummary) @@ -187,7 +193,7 @@ export async function startPlaygroundCliServer(spec: RuntimeCreateSpec, mounts: } finally { await archiveReference?.release().catch(() => undefined) if (usesArchiveCache) { - await maintainPlaygroundCustomArchiveCache().catch(() => undefined) + await automaticPlaygroundCustomArchiveCacheMaintenance(true) } await readonlyMountStaging?.[Symbol.asyncDispose]() } @@ -200,7 +206,7 @@ export async function startPlaygroundCliServer(spec: RuntimeCreateSpec, mounts: await archiveReference?.release().catch(() => undefined) if (usesArchiveCache) { - await maintainPlaygroundCustomArchiveCache().catch(() => undefined) + await automaticPlaygroundCustomArchiveCacheMaintenance(true) } await readonlyMountStaging?.[Symbol.asyncDispose]() if (spec.preview?.port && errorHasCode(error, "EADDRINUSE")) { @@ -623,6 +629,23 @@ function errorDetail(error: unknown): Record { return { message: String(error) } } +async function automaticPlaygroundCustomArchiveCacheMaintenance(warnOnFailure = false): Promise<{ result?: PlaygroundCustomArchiveCacheMaintenance; diagnostics: PlaygroundCustomArchiveCacheDiagnostic[] }> { + try { + return { result: await maintainPlaygroundCustomArchiveCache(), diagnostics: [] } + } catch (error) { + const diagnostic = { + code: "playground-custom-archive-cache-maintenance-failed", + message: error instanceof Error ? error.message.slice(0, 500) : String(error).slice(0, 500), + severity: "warning" as const, + path: playgroundWordPressArchiveCacheDirectory(), + } + if (warnOnFailure) { + console.warn(`[wp-codebox] ${diagnostic.code}: ${diagnostic.message}`) + } + return { diagnostics: [diagnostic] } + } +} + export interface PlaygroundWordPressArchiveCacheValidation { version: string sourceUrl: string @@ -640,6 +663,7 @@ export interface PlaygroundWordPressArchiveCacheValidation { deleted: boolean }> retention?: PlaygroundCustomArchiveCacheMaintenance + retentionDiagnostics?: PlaygroundCustomArchiveCacheDiagnostic[] } export interface PlaygroundWordPressArchiveCacheValidationOptions { diff --git a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts index c892169a..7c11cffa 100644 --- a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts +++ b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts @@ -1,24 +1,43 @@ import { randomUUID } from "node:crypto" -import { existsSync } from "node:fs" -import { mkdir, readFile, readdir, rm, stat, unlink, writeFile } from "node:fs/promises" -import { homedir } from "node:os" -import { basename, join } from "node:path" +import { hostname, homedir } from "node:os" +import { lstat, mkdir, readFile, readdir, rename, rm, unlink, writeFile } from "node:fs/promises" +import { basename, dirname, join } from "node:path" const CUSTOM_ARCHIVE_PATTERN = /^custom-[A-Za-z0-9._-]+\.zip$/ +const CUSTOM_SIDECAR_PATTERN = /^(custom-[A-Za-z0-9._-]+\.zip)\.(lock|refs)$/ const DEFAULT_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1_000 const DEFAULT_MAX_BYTES = 1024 * 1024 * 1024 const DEFAULT_MAX_COUNT = 20 +const DEFAULT_LEASE_MS = 120_000 const DEFAULT_STALE_LOCK_MS = 10 * 60 * 1_000 const CACHE_DIRECTORY_ENV = "WP_CODEBOX_PLAYGROUND_WORDPRESS_CACHE_DIR" const CACHE_MAX_AGE_MS_ENV = "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_AGE_MS" const CACHE_MAX_BYTES_ENV = "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_BYTES" const CACHE_MAX_COUNT_ENV = "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT" +const CACHE_LEASE_MS_ENV = "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_LEASE_MS" const CACHE_STALE_LOCK_MS_ENV = "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_STALE_LOCK_MS" interface CacheOwnerRecord { + schema: "wp-codebox/playground-cache-lease/v1" + token: string + hostname: string + bootId: string pid: number + processStart: string createdAt: string + heartbeatAt: string + expiresAt: string +} + +interface LeaseHandle { + token: string + release(): Promise +} + +interface LeaseState { + active: boolean + token?: string } export interface PlaygroundArchiveCacheLock { @@ -36,6 +55,7 @@ export interface PlaygroundCustomArchiveCachePolicy { maxAgeMs: number maxBytes: number maxCount: number + leaseMs: number staleLockMs: number } @@ -44,6 +64,13 @@ export interface PlaygroundCustomArchiveCacheMaintenanceOptions extends Partial< now?: number } +export interface PlaygroundCustomArchiveCacheDiagnostic { + code: string + message: string + severity: "warning" | "error" + path?: string +} + export interface PlaygroundCustomArchiveCacheMaintenance { schema: "wp-codebox/playground-custom-archive-cache-maintenance/v1" mode: "diagnose" | "dry-run" | "apply" @@ -58,6 +85,13 @@ export interface PlaygroundCustomArchiveCacheMaintenance { bytes: number lockCount: number referenceCount: number + filesystemCount: number + paths: string[] + } + sidecars: { + orphanCount: number + activeCount: number + removedCount: number paths: string[] } removedCount: number @@ -65,6 +99,7 @@ export interface PlaygroundCustomArchiveCacheMaintenance { verifiedReclaimedBytes: number retainedCount: number retainedBytes: number + diagnostics: PlaygroundCustomArchiveCacheDiagnostic[] } interface CustomArchiveEntry { @@ -74,18 +109,24 @@ interface CustomArchiveEntry { lockPath: string activeLock: boolean activeReferences: number + filesystemProtected: boolean } +const processIdentityPromise = processIdentity() + export async function withPlaygroundArchiveCacheLock(cacheDirectory: string, version: string, callback: (lock: PlaygroundArchiveCacheLock) => Promise, timeoutError?: (lockPath: string) => Error): Promise { await mkdir(cacheDirectory, { recursive: true }) const lockPath = join(cacheDirectory, `${version}.zip.lock`) const startedAt = Date.now() + const policy = cacheLeasePolicy() + let lease: LeaseHandle | undefined for (;;) { - if (await tryAcquireCacheLock(lockPath)) { + lease = await tryAcquireCacheLock(lockPath, policy.leaseMs) + if (lease) { break } - if (!await cacheLockIsActive(lockPath, Date.now(), customArchiveCachePolicy({}).staleLockMs, true)) { + if (!await cacheLockIsActive(lockPath, Date.now(), policy, true)) { continue } if (Date.now() - startedAt > 120_000) { @@ -97,27 +138,22 @@ export async function withPlaygroundArchiveCacheLock(cacheDirectory: string, try { return await callback({ path: lockPath, waitedMs: Date.now() - startedAt }) } finally { - await rm(lockPath, { recursive: true, force: true }) + await lease.release() } } export async function acquirePlaygroundArchiveReference(archivePath: string): Promise { + const policy = cacheLeasePolicy() const referencesDirectory = `${archivePath}.refs` await mkdir(referencesDirectory, { recursive: true }) - const referencePath = join(referencesDirectory, `${process.pid}-${randomUUID()}.json`) - await writeFile(referencePath, `${JSON.stringify(ownerRecord())}\n`, { flag: "wx" }) - let released = false + const token = randomUUID() + const referencePath = join(referencesDirectory, `${token}.json`) + const lease = await createFileLease(referencePath, token, policy.leaseMs) return { archivePath, path: referencePath, - async release() { - if (released) { - return - } - released = true - await rm(referencePath, { force: true }) - }, + release: lease.release, } } @@ -129,21 +165,39 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play const mode = options.mode ?? "apply" const now = options.now ?? Date.now() const policy = customArchiveCachePolicy(options) + const diagnostics: PlaygroundCustomArchiveCacheDiagnostic[] = [] await mkdir(cacheDirectory, { recursive: true }) - const archiveNames = (await readdir(cacheDirectory)).filter((name) => CUSTOM_ARCHIVE_PATTERN.test(name)).sort() + const directoryEntries = await readdir(cacheDirectory, { withFileTypes: true }) + const archiveNames = directoryEntries.map((entry) => entry.name).filter((name) => CUSTOM_ARCHIVE_PATTERN.test(name)).sort() const entries: CustomArchiveEntry[] = [] for (const name of archiveNames) { const archivePath = join(cacheDirectory, name) try { - const archiveStat = await stat(archivePath) - if (!archiveStat.isFile()) { - continue + const archiveStat = await lstat(archivePath) + const regular = archiveStat.isFile() + const singlyLinked = archiveStat.nlink === 1 + const filesystemProtected = !regular || !singlyLinked + if (filesystemProtected) { + diagnostics.push({ + code: regular ? "custom-archive-multiply-linked" : "custom-archive-not-regular", + message: regular ? "Custom archive has multiple hard links and was protected from retention." : "Custom archive is not a regular file and was protected from retention.", + severity: "warning", + path: archivePath, + }) } const lockPath = `${archivePath}.lock` - const activeLock = await cacheLockIsActive(lockPath, now, policy.staleLockMs, mode === "apply") - const activeReferences = await activeArchiveReferenceCount(archivePath, mode === "apply") - entries.push({ path: archivePath, size: archiveStat.size, mtimeMs: archiveStat.mtimeMs, lockPath, activeLock, activeReferences }) + const activeLock = await cacheLockIsActive(lockPath, now, policy, mode === "apply") + const activeReferences = await activeArchiveReferenceCount(archivePath, now, policy, mode === "apply") + entries.push({ + path: archivePath, + size: regular ? archiveStat.size : 0, + mtimeMs: archiveStat.mtimeMs, + lockPath, + activeLock, + activeReferences, + filesystemProtected, + }) } catch (error) { if (!errorHasCode(error, "ENOENT")) { throw error @@ -151,21 +205,20 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play } } - const protectedEntries = entries.filter((entry) => entry.activeLock || entry.activeReferences > 0) - let retainedCount = protectedEntries.length - let retainedBytes = protectedEntries.reduce((total, entry) => total + entry.size, 0) + const protectedEntries = entries.filter((entry) => entry.activeLock || entry.activeReferences > 0 || entry.filesystemProtected) + let retainedCountForBounds = protectedEntries.length + let retainedBytesForBounds = protectedEntries.reduce((total, entry) => total + entry.size, 0) const candidates: CustomArchiveEntry[] = [] - const eligible = entries.filter((entry) => !entry.activeLock && entry.activeReferences === 0).sort(newestFirst) - + const eligible = entries.filter((entry) => !protectedEntries.includes(entry)).sort(newestFirst) for (const entry of eligible) { const stale = now - entry.mtimeMs > policy.maxAgeMs - const exceedsCount = retainedCount + 1 > policy.maxCount - const exceedsBytes = retainedBytes + entry.size > policy.maxBytes + const exceedsCount = retainedCountForBounds + 1 > policy.maxCount + const exceedsBytes = retainedBytesForBounds + entry.size > policy.maxBytes if (stale || exceedsCount || exceedsBytes) { candidates.push(entry) } else { - retainedCount += 1 - retainedBytes += entry.size + retainedCountForBounds += 1 + retainedBytesForBounds += entry.size } } @@ -174,42 +227,56 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play let verifiedReclaimedBytes = 0 if (mode === "apply") { for (const candidate of candidates.sort(oldestFirst)) { - if (!await tryAcquireCacheLock(candidate.lockPath)) { + const lease = await tryAcquireCacheLock(candidate.lockPath, policy.leaseMs) + if (!lease) { continue } try { - if (await activeArchiveReferenceCount(candidate.path, true) > 0) { + if (await activeArchiveReferenceCount(candidate.path, Date.now(), policy, true) > 0) { continue } - let beforeSize: number + let current try { - beforeSize = (await stat(candidate.path)).size + current = await lstat(candidate.path) } catch (error) { if (errorHasCode(error, "ENOENT")) { continue } throw error } + if (!current.isFile() || current.nlink !== 1) { + 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 + } + const currentAllocatedBytes = allocatedBytes(current.blocks) await unlink(candidate.path) - if (!existsSync(candidate.path)) { + try { + await lstat(candidate.path) + diagnostics.push({ code: "custom-archive-removal-unverified", message: "Custom archive path still exists after unlink and reclaimed bytes could not be verified.", severity: "warning", path: candidate.path }) + } catch (error) { + if (!errorHasCode(error, "ENOENT")) { + throw error + } removedCount += 1 - removedBytes += beforeSize - verifiedReclaimedBytes += beforeSize - await rm(`${candidate.path}.refs`, { recursive: true, force: true }) + removedBytes += current.size + verifiedReclaimedBytes += currentAllocatedBytes + await removeOrphanReferencesDirectory(`${candidate.path}.refs`, Date.now(), policy, diagnostics) } } finally { - await rm(candidate.lockPath, { recursive: true, force: true }) + await lease.release() } } } + const sidecars = await maintainOrphanSidecars(cacheDirectory, directoryEntries.map((entry) => entry.name), now, policy, mode, diagnostics) + const totalBytes = entries.reduce((total, entry) => total + entry.size, 0) return { schema: "wp-codebox/playground-custom-archive-cache-maintenance/v1", mode, cacheDirectory, policy, customArchiveCount: entries.length, - customArchiveBytes: entries.reduce((total, entry) => total + entry.size, 0), + customArchiveBytes: totalBytes, candidateCount: candidates.length, candidateBytes: candidates.reduce((total, entry) => total + entry.size, 0), activeProtection: { @@ -217,13 +284,16 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play bytes: protectedEntries.reduce((total, entry) => total + entry.size, 0), lockCount: protectedEntries.filter((entry) => entry.activeLock).length, referenceCount: protectedEntries.reduce((total, entry) => total + entry.activeReferences, 0), + filesystemCount: protectedEntries.filter((entry) => entry.filesystemProtected).length, paths: protectedEntries.map((entry) => entry.path).sort(), }, + sidecars, removedCount, removedBytes, verifiedReclaimedBytes, retainedCount: entries.length - removedCount, - retainedBytes: entries.reduce((total, entry) => total + entry.size, 0) - removedBytes, + retainedBytes: totalBytes - removedBytes, + diagnostics, } } @@ -232,99 +302,303 @@ function customArchiveCachePolicy(options: PlaygroundCustomArchiveCacheMaintenan maxAgeMs: boundedNumber(options.maxAgeMs, CACHE_MAX_AGE_MS_ENV, DEFAULT_MAX_AGE_MS), maxBytes: boundedNumber(options.maxBytes, CACHE_MAX_BYTES_ENV, DEFAULT_MAX_BYTES), maxCount: boundedNumber(options.maxCount, CACHE_MAX_COUNT_ENV, DEFAULT_MAX_COUNT), + leaseMs: boundedNumber(options.leaseMs, CACHE_LEASE_MS_ENV, DEFAULT_LEASE_MS, 100), staleLockMs: boundedNumber(options.staleLockMs, CACHE_STALE_LOCK_MS_ENV, DEFAULT_STALE_LOCK_MS), } } -async function tryAcquireCacheLock(lockPath: string): Promise { +function cacheLeasePolicy(): Pick { + return { + leaseMs: boundedNumber(undefined, CACHE_LEASE_MS_ENV, DEFAULT_LEASE_MS, 100), + staleLockMs: boundedNumber(undefined, CACHE_STALE_LOCK_MS_ENV, DEFAULT_STALE_LOCK_MS), + } +} + +async function tryAcquireCacheLock(lockPath: string, leaseMs: number): Promise { + const token = randomUUID() try { await mkdir(lockPath) - await writeFile(join(lockPath, "owner.json"), `${JSON.stringify(ownerRecord())}\n`) - return true } catch (error) { if (errorHasCode(error, "EEXIST")) { - return false + return undefined } - await rm(lockPath, { recursive: true, force: true }) + throw error + } + try { + return await createDirectoryLease(lockPath, token, leaseMs) + } catch (error) { + await quarantineDirectory(lockPath) throw error } } -async function cacheLockIsActive(lockPath: string, now: number, staleLockMs: number, removeStale: boolean): Promise { - try { - const lockStat = await stat(lockPath) - const owner = await readOwnerRecord(join(lockPath, "owner.json")) - const active = owner ? processIsAlive(owner.pid) : now - lockStat.mtimeMs <= staleLockMs - if (!active && removeStale) { - await rm(lockPath, { recursive: true, force: true }) +async function createDirectoryLease(lockPath: string, token: string, leaseMs: number): Promise { + const ownerPath = join(lockPath, "owner.json") + await writeLease(ownerPath, await ownerRecord(token, leaseMs)) + return heartbeatLease(ownerPath, token, leaseMs, async () => { + await quarantineDirectoryIfOwned(lockPath, token) + }) +} + +async function createFileLease(path: string, token: string, leaseMs: number): Promise { + await writeLease(path, await ownerRecord(token, leaseMs), true) + return heartbeatLease(path, token, leaseMs, async () => { + const owner = await readOwnerRecord(path) + if (owner?.token === token) { + await rm(path, { force: true }) } - return active - } catch (error) { - if (errorHasCode(error, "ENOENT")) { - return false + }) +} + +function heartbeatLease(path: string, token: string, leaseMs: number, removeOwned: () => Promise): LeaseHandle { + let stopped = false + let heartbeat: Promise | undefined + const interval = setInterval(() => { + if (stopped || heartbeat) { + return } - throw error + heartbeat = refreshLease(path, token, leaseMs).catch(() => undefined).finally(() => { + heartbeat = undefined + }) + }, Math.max(50, Math.floor(leaseMs / 3))) + interval.unref() + + return { + token, + async release() { + stopped = true + clearInterval(interval) + await heartbeat?.catch(() => undefined) + await removeOwned() + }, } } -async function activeArchiveReferenceCount(archivePath: string, removeStale: boolean): Promise { +async function refreshLease(path: string, token: string, leaseMs: number): Promise { + const owner = await readOwnerRecord(path) + if (owner?.token !== token) { + return + } + const now = new Date() + await writeLease(path, { ...owner, heartbeatAt: now.toISOString(), expiresAt: new Date(now.getTime() + leaseMs).toISOString() }) +} + +async function cacheLockIsActive(lockPath: string, now: number, policy: Pick, removeStale: boolean): Promise { + const state = await leaseState(join(lockPath, "owner.json"), lockPath, now, policy.staleLockMs) + if (!state.active && removeStale) { + if (state.token) { + await quarantineDirectoryIfOwned(lockPath, state.token) + } else { + await quarantineLegacyDirectoryIfStale(lockPath, now, policy.staleLockMs) + } + } + return state.active +} + +async function activeArchiveReferenceCount(archivePath: string, now: number, policy: Pick, removeStale: boolean): Promise { const referencesDirectory = `${archivePath}.refs` let names: string[] try { names = await readdir(referencesDirectory) } catch (error) { - if (errorHasCode(error, "ENOENT")) { + if (errorHasCode(error, "ENOENT") || errorHasCode(error, "ENOTDIR")) { return 0 } throw error } - let active = 0 for (const name of names.sort()) { const path = join(referencesDirectory, name) - const owner = await readOwnerRecord(path) - if (owner && processIsAlive(owner.pid)) { + if (name.startsWith(".") && name.endsWith(".tmp")) { + if (removeStale) { + await rm(path, { force: true }) + } + continue + } + const state = await leaseState(path, path, now, policy.staleLockMs) + if (state.active) { active += 1 } else if (removeStale) { - await rm(path, { recursive: true, force: true }) + const current = await readOwnerRecord(path) + if (!state.token || current?.token === state.token) { + await rm(path, { recursive: true, force: true }) + } } } return active } -async function readOwnerRecord(path: string): Promise { +async function leaseState(ownerPath: string, sidecarPath: string, now: number, legacyStaleMs: number): Promise { + const owner = await readOwnerRecord(ownerPath) + if (owner) { + const expiresAt = Date.parse(owner.expiresAt) + return { active: Number.isFinite(expiresAt) && expiresAt > now, token: owner.token } + } try { - const value = JSON.parse(await readFile(path, "utf8")) as Partial - return Number.isInteger(value.pid) && typeof value.createdAt === "string" ? value as CacheOwnerRecord : undefined + const sidecarStat = await lstat(sidecarPath) + return { active: now - sidecarStat.mtimeMs <= legacyStaleMs } } catch (error) { - if (errorHasCode(error, "ENOENT") || errorHasCode(error, "EISDIR") || error instanceof SyntaxError) { - return undefined + if (errorHasCode(error, "ENOENT")) { + return { active: false } } throw error } } -function ownerRecord(): CacheOwnerRecord { - return { pid: process.pid, createdAt: new Date().toISOString() } +async function maintainOrphanSidecars(cacheDirectory: string, names: string[], now: number, policy: PlaygroundCustomArchiveCachePolicy, mode: PlaygroundCustomArchiveCacheMaintenance["mode"], diagnostics: PlaygroundCustomArchiveCacheDiagnostic[]): Promise { + const paths: string[] = [] + let activeCount = 0 + let removedCount = 0 + for (const name of names.sort()) { + const match = CUSTOM_SIDECAR_PATTERN.exec(name) + if (!match || names.includes(match[1])) { + continue + } + const path = join(cacheDirectory, name) + paths.push(path) + if (match[2] === "lock") { + const active = await cacheLockIsActive(path, now, policy, mode === "apply") + if (active) { + activeCount += 1 + } else if (mode === "apply" && !await pathExists(path)) { + removedCount += 1 + } + continue + } + const active = await activeReferenceCountInDirectory(path, now, policy, mode === "apply") + if (active > 0) { + activeCount += 1 + } else if (mode === "apply" && await removeOrphanReferencesDirectory(path, now, policy, diagnostics)) { + removedCount += 1 + } + } + return { orphanCount: paths.length, activeCount, removedCount, paths } } -function processIsAlive(pid: number): boolean { +async function activeReferenceCountInDirectory(path: string, now: number, policy: Pick, removeStale: boolean): Promise { + return activeArchiveReferenceCount(path.slice(0, -".refs".length), now, policy, removeStale) +} + +async function removeOrphanReferencesDirectory(path: string, now: number, policy: Pick, diagnostics: PlaygroundCustomArchiveCacheDiagnostic[]): Promise { + if (await activeReferenceCountInDirectory(path, now, policy, true) > 0) { + return false + } try { - process.kill(pid, 0) + const quarantined = `${path}.cleanup-${randomUUID()}` + await rename(path, quarantined) + await rm(quarantined, { recursive: true, force: true }) return true } catch (error) { - return errorHasCode(error, "EPERM") + if (errorHasCode(error, "ENOENT")) { + return false + } + diagnostics.push({ code: "orphan-reference-cleanup-failed", message: errorMessage(error), severity: "warning", path }) + return false + } +} + +async function quarantineDirectoryIfOwned(path: string, token: string): Promise { + if ((await readOwnerRecord(join(path, "owner.json")))?.token !== token) { + return false + } + return quarantineDirectory(path) +} + +async function quarantineLegacyDirectoryIfStale(path: string, now: number, staleMs: number): Promise { + try { + if (now - (await lstat(path)).mtimeMs <= staleMs || await readOwnerRecord(join(path, "owner.json"))) { + return false + } + return quarantineDirectory(path) + } catch (error) { + if (errorHasCode(error, "ENOENT")) { + return false + } + throw error + } +} + +async function quarantineDirectory(path: string): Promise { + const quarantined = `${path}.cleanup-${randomUUID()}` + try { + await rename(path, quarantined) + } catch (error) { + if (errorHasCode(error, "ENOENT")) { + return false + } + throw error + } + await rm(quarantined, { recursive: true, force: true }) + return true +} + +async function writeLease(path: string, owner: CacheOwnerRecord, exclusive = false): Promise { + if (exclusive) { + await writeFile(path, `${JSON.stringify(owner)}\n`, { flag: "wx" }) + return + } + const temporaryPath = join(dirname(path), `.${basename(path)}.${owner.token}.tmp`) + await writeFile(temporaryPath, `${JSON.stringify(owner)}\n`) + try { + await rename(temporaryPath, path) + } finally { + await rm(temporaryPath, { force: true }) + } +} + +async function readOwnerRecord(path: string): Promise { + try { + const value = JSON.parse(await readFile(path, "utf8")) as Partial + 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 } } -function boundedNumber(explicit: number | undefined, environmentName: string, fallback: number): number { +async function ownerRecord(token: string, leaseMs: number): Promise { + const identity = await processIdentityPromise + const now = new Date() + return { + schema: "wp-codebox/playground-cache-lease/v1", + token, + ...identity, + createdAt: now.toISOString(), + heartbeatAt: now.toISOString(), + expiresAt: new Date(now.getTime() + leaseMs).toISOString(), + } +} + +async function processIdentity(): Promise> { + const bootId = await readFile("/proc/sys/kernel/random/boot_id", "utf8").then((value) => value.trim()).catch(() => "unavailable") + const processStart = await readFile(`/proc/${process.pid}/stat`, "utf8").then((value) => value.trim().split(/\s+/)[21] ?? "unavailable").catch(() => String(process.uptime())) + return { hostname: hostname(), bootId, pid: process.pid, processStart } +} + +function boundedNumber(explicit: number | undefined, environmentName: string, fallback: number, minimum = 0): number { const candidate = explicit ?? (process.env[environmentName] === undefined ? fallback : Number(process.env[environmentName])) - if (!Number.isFinite(candidate) || candidate < 0) { - throw new Error(`${environmentName} must be a non-negative finite number`) + if (!Number.isFinite(candidate) || candidate < minimum) { + throw new Error(`${environmentName} must be a finite number greater than or equal to ${minimum}`) } return Math.floor(candidate) } +function allocatedBytes(blocks: number): number { + return Math.max(0, blocks) * 512 +} + function newestFirst(left: CustomArchiveEntry, right: CustomArchiveEntry): number { return right.mtimeMs - left.mtimeMs || left.path.localeCompare(right.path) } @@ -337,6 +611,22 @@ function errorHasCode(error: unknown, code: string): boolean { return Boolean(error && typeof error === "object" && "code" in error && error.code === code) } +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message.slice(0, 500) : String(error).slice(0, 500) +} + +async function pathExists(path: string): Promise { + try { + await lstat(path) + return true + } catch (error) { + if (errorHasCode(error, "ENOENT")) { + return false + } + throw error + } +} + async function delay(ms: number): Promise { await new Promise((resolveDelay) => setTimeout(resolveDelay, ms)) } diff --git a/tests/fixtures/playground-cache-lease-child.ts b/tests/fixtures/playground-cache-lease-child.ts new file mode 100644 index 00000000..8ce3ea69 --- /dev/null +++ b/tests/fixtures/playground-cache-lease-child.ts @@ -0,0 +1,29 @@ +import { lstat, writeFile } from "node:fs/promises" + +import { acquirePlaygroundArchiveReference } from "../../packages/runtime-playground/src/playground-wordpress-archive-cache.js" + +const [archivePath, readyPath, stopPath] = process.argv.slice(2) +if (!archivePath || !readyPath || !stopPath) { + throw new Error("archivePath, readyPath, and stopPath are required") +} + +const reference = await acquirePlaygroundArchiveReference(archivePath) +await writeFile(readyPath, reference.path) + +while (!await exists(stopPath)) { + await new Promise((resolve) => setTimeout(resolve, 25)) +} + +await reference.release() + +async function exists(path: string): Promise { + try { + await lstat(path) + return true + } catch (error) { + if (error && typeof error === "object" && "code" in error && error.code === "ENOENT") { + return false + } + throw error + } +} diff --git a/tests/playground-custom-archive-cache-process.test.ts b/tests/playground-custom-archive-cache-process.test.ts new file mode 100644 index 00000000..dd274b79 --- /dev/null +++ b/tests/playground-custom-archive-cache-process.test.ts @@ -0,0 +1,111 @@ +import assert from "node:assert/strict" +import { spawn, type ChildProcess } from "node:child_process" +import { lstat, mkdtemp, readFile, readdir, rm, utimes, writeFile } from "node:fs/promises" +import { tmpdir } from "node:os" +import { join } from "node:path" + +import { maintainPlaygroundCustomArchiveCache } from "../packages/runtime-playground/src/playground-wordpress-archive-cache.js" + +const root = await mkdtemp(join(tmpdir(), "wp-codebox-playground-cache-process-")) +const fixture = join(process.cwd(), "tests/fixtures/playground-cache-lease-child.ts") +const tsx = join(process.cwd(), "node_modules/.bin/tsx") +const children: ChildProcess[] = [] + +try { + const archivePath = join(root, "custom-shared-process.zip") + await writeFile(archivePath, Buffer.alloc(8_192, "p")) + await utimes(archivePath, 1, 1) + + const first = await startChild("first", archivePath) + const second = await startChild("second", archivePath) + const shared = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", maxAgeMs: 1 }) + assert.equal(shared.activeProtection.referenceCount, 2) + assert.ok(await exists(archivePath), "two separate processes must share archive protection") + + await stopChild(first) + const oneRemaining = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", maxAgeMs: 1 }) + assert.equal(oneRemaining.activeProtection.referenceCount, 1) + assert.ok(await exists(archivePath), "one remaining process must keep the archive protected") + + killChildGroup(second.child) + await childExit(second.child) + const immediatelyAfterCrash = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", maxAgeMs: 1 }) + assert.equal(immediatelyAfterCrash.activeProtection.referenceCount, 1, "a crashed process remains protected until its bounded lease expires") + assert.ok(await exists(archivePath), "archive must remain while crashed-process lease is unexpired") + await waitForReferenceExpiry(`${archivePath}.refs`) + const afterExpiry = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", maxAgeMs: 1 }) + assert.ok(afterExpiry.removedCount >= 1, JSON.stringify(afterExpiry)) + assert.ok(!await exists(archivePath), "expired crashed-process lease must become reclaimable") + + console.log("playground custom archive separate-process leases passed") +} finally { + for (const child of children) { + if (child.exitCode === null && child.signalCode === null) killChildGroup(child) + } + await rm(root, { recursive: true, force: true }) +} + +async function startChild(name: string, archivePath: string): Promise<{ child: ChildProcess; stopPath: string }> { + const readyPath = join(root, `${name}.ready`) + const stopPath = join(root, `${name}.stop`) + const child = spawn(tsx, [fixture, archivePath, readyPath, stopPath], { + env: { ...process.env, WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_LEASE_MS: "300" }, + stdio: ["ignore", "pipe", "pipe"], + detached: true, + }) + children.push(child) + await waitForPath(readyPath, child) + const referencePath = await readFile(readyPath, "utf8") + const owner = JSON.parse(await readFile(referencePath, "utf8")) as { heartbeatAt: string; expiresAt: string } + assert.ok(Date.parse(owner.expiresAt) - Date.parse(owner.heartbeatAt) <= 350, "child must use the configured short lease") + return { child, stopPath } +} + +async function stopChild(entry: { child: ChildProcess; stopPath: string }): Promise { + await writeFile(entry.stopPath, "stop") + await childExit(entry.child) +} + +async function waitForPath(path: string, child: ChildProcess): Promise { + for (let attempt = 0; attempt < 100; attempt += 1) { + if (await exists(path)) return + if (child.exitCode !== null) throw new Error(`lease child exited early with ${child.exitCode}`) + await new Promise((resolve) => setTimeout(resolve, 25)) + } + throw new Error(`timed out waiting for lease child: ${path}`) +} + +async function childExit(child: ChildProcess): Promise { + if (child.exitCode !== null || child.signalCode !== null) return + await new Promise((resolve, reject) => { + child.once("exit", () => resolve()) + child.once("error", reject) + }) +} + +function killChildGroup(child: ChildProcess): void { + if (child.pid) { + process.kill(-child.pid, "SIGKILL") + } +} + +async function exists(path: string): Promise { + try { + await lstat(path) + return true + } catch (error) { + if (error && typeof error === "object" && "code" in error && error.code === "ENOENT") return false + throw error + } +} + +async function referenceRecords(path: string): Promise { + if (!await exists(path)) return [] + return Promise.all((await readdir(path)).map(async (name) => JSON.parse(await readFile(join(path, name), "utf8")))) +} + +async function waitForReferenceExpiry(path: string): Promise { + const records = await referenceRecords(path) as Array<{ expiresAt?: string }> + const latestExpiry = Math.max(Date.now(), ...records.map((record) => Date.parse(record.expiresAt ?? ""))).valueOf() + await new Promise((resolve) => setTimeout(resolve, Math.max(0, latestExpiry - Date.now()) + 100)) +} diff --git a/tests/playground-custom-archive-cache.integration.test.ts b/tests/playground-custom-archive-cache.integration.test.ts new file mode 100644 index 00000000..3b9a3400 --- /dev/null +++ b/tests/playground-custom-archive-cache.integration.test.ts @@ -0,0 +1,81 @@ +import assert from "node:assert/strict" +import { createServer } from "node:http" +import { mkdtemp, readdir, rm } from "node:fs/promises" +import { tmpdir } from "node:os" +import { join } from "node:path" + +import { startPlaygroundCliServer, type PlaygroundCliModule } from "../packages/runtime-playground/src/playground-cli-runner.js" +import type { RuntimeCreateSpec } from "../packages/runtime-core/src/index.js" + +const root = await mkdtemp(join(tmpdir(), "wp-codebox-playground-cache-lifecycle-")) +const environmentNames = ["WP_CODEBOX_PLAYGROUND_WORDPRESS_CACHE_DIR", "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_AGE_MS", "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT", "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_LEASE_MS"] as const +const originalEnvironment = Object.fromEntries(environmentNames.map((name) => [name, process.env[name]])) +const zip = Buffer.alloc(22) +zip.set([0x50, 0x4b, 0x05, 0x06]) +const source = createServer((_request, response) => { + response.writeHead(200, { "content-type": "application/zip", "content-length": String(zip.length) }) + response.end(zip) +}) +await new Promise((resolve) => source.listen(0, "127.0.0.1", resolve)) +const address = source.address() +if (!address || typeof address === "string") throw new Error("archive source did not bind") +const archiveUrl = `http://127.0.0.1:${address.port}/wordpress.zip` +const progress: Array> = [] +const warnings: string[] = [] +const originalWarn = console.warn + +const spec: RuntimeCreateSpec = { + backend: "wordpress-playground", + environment: { version: archiveUrl, phpVersion: "8.4", blueprint: {} }, + policy: { network: "deny", filesystem: "none", commands: ["wordpress.run-php"], secrets: "none", approvals: "never" }, +} +const cliModule: PlaygroundCliModule = { + async runCLI() { + const names = await readdir(root) + assert.ok(names.some((name) => name.endsWith(".zip.refs")), "startup must hold an archive lease before Playground consumes it") + return { + serverUrl: "http://127.0.0.1:65535", + playground: { async run() { return { text: "" } } }, + async [Symbol.asyncDispose]() {}, + } + }, +} + +try { + process.env.WP_CODEBOX_PLAYGROUND_WORDPRESS_CACHE_DIR = root + process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_AGE_MS = "0" + process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT = "0" + process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_LEASE_MS = "300" + + const server = await startPlaygroundCliServer(spec, [], { cliModule, onProgress(event) { progress.push(event as unknown as Record) } }) + await new Promise((resolve) => setTimeout(resolve, 0)) + const connecting = progress.find((event) => event.phase === "preview:connecting-client") + const cacheValidation = (connecting?.detail as Record | undefined)?.cacheValidation as Record | undefined + assert.equal((cacheValidation?.retention as Record | undefined)?.schema, "wp-codebox/playground-custom-archive-cache-maintenance/v1") + assert.equal(((cacheValidation?.retention as Record).activeProtection as Record).referenceCount, 1) + await server[Symbol.asyncDispose]() + 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> = [] + const invalidServer = await startPlaygroundCliServer(spec, [], { cliModule, onProgress(event) { invalidProgress.push(event as unknown as Record) } }) + await new Promise((resolve) => setTimeout(resolve, 0)) + const invalidConnecting = invalidProgress.find((event) => event.phase === "preview:connecting-client") + const invalidValidation = (invalidConnecting?.detail as Record).cacheValidation as Record + const diagnostics = invalidValidation.retentionDiagnostics as Array> + assert.equal(diagnostics[0]?.code, "playground-custom-archive-cache-maintenance-failed") + await invalidServer[Symbol.asyncDispose]() + assert.ok(warnings.some((warning) => warning.includes("playground-custom-archive-cache-maintenance-failed")), "teardown policy failures must emit a bounded warning") + + console.log("playground custom archive startup and teardown integration passed") +} finally { + console.warn = originalWarn + for (const name of environmentNames) { + const value = originalEnvironment[name] + if (value === undefined) delete process.env[name] + else process.env[name] = value + } + await new Promise((resolve, reject) => source.close((error) => error ? reject(error) : resolve())) + await rm(root, { recursive: true, force: true }) +} diff --git a/tests/playground-custom-archive-cache.test.ts b/tests/playground-custom-archive-cache.test.ts index 3f835a9c..2643dcf6 100644 --- a/tests/playground-custom-archive-cache.test.ts +++ b/tests/playground-custom-archive-cache.test.ts @@ -1,17 +1,25 @@ import assert from "node:assert/strict" -import { mkdir, mkdtemp, readdir, rm, utimes, writeFile } from "node:fs/promises" -import { tmpdir } from "node:os" +import { link, lstat, mkdir, mkdtemp, readdir, readFile, rm, symlink, utimes, writeFile } from "node:fs/promises" +import { hostname, tmpdir } from "node:os" import { join } from "node:path" -import { acquirePlaygroundArchiveReference, maintainPlaygroundCustomArchiveCache, withPlaygroundArchiveCacheLock } from "../packages/runtime-playground/src/playground-wordpress-archive-cache.js" +import { acquirePlaygroundArchiveReference, maintainPlaygroundCustomArchiveCache, playgroundWordPressArchiveCacheDirectory, withPlaygroundArchiveCacheLock } from "../packages/runtime-playground/src/playground-wordpress-archive-cache.js" const root = await mkdtemp(join(tmpdir(), "wp-codebox-playground-custom-cache-")) const now = Date.now() +const environmentNames = [ + "WP_CODEBOX_PLAYGROUND_WORDPRESS_CACHE_DIR", + "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_AGE_MS", + "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_BYTES", + "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT", + "WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_LEASE_MS", +] as const +const originalEnvironment = Object.fromEntries(environmentNames.map((name) => [name, process.env[name]])) try { const stableArchive = await archive("6.9.1.zip", 101, now - 100_000) - const staleArchive = await archive("custom-stale.zip", 11, now - 100_000) - const invalidArchive = await archive("custom-invalid.zip", 13, now - 100_000, "not a zip") + await archive("custom-stale.zip", 11, now - 100_000) + await archive("custom-invalid.zip", 13, now - 100_000, "not a zip") const referencedArchive = await archive("custom-referenced.zip", 17, now - 100_000) const lockedArchive = await archive("custom-locked.zip", 19, now - 100_000) @@ -19,15 +27,9 @@ try { const secondReference = await acquirePlaygroundArchiveReference(referencedArchive) const activeLock = `${lockedArchive}.lock` await mkdir(activeLock) - await writeFile(join(activeLock, "owner.json"), JSON.stringify({ pid: process.pid, createdAt: new Date(now).toISOString() })) - - const dryRun = await maintainPlaygroundCustomArchiveCache(root, { - mode: "dry-run", - now, - maxAgeMs: 1_000, - maxBytes: 1_000, - maxCount: 20, - }) + await writeLease(join(activeLock, "owner.json"), { token: "foreign-active-lock", expiresAt: now + 60_000, ownerHostname: "foreign-host" }) + + const dryRun = await maintainPlaygroundCustomArchiveCache(root, { mode: "dry-run", now, maxAgeMs: 1_000, maxBytes: 1_000, maxCount: 20 }) assert.equal(dryRun.customArchiveCount, 4, "stable release archives must not enter custom retention") assert.equal(dryRun.candidateCount, 2, "stale unprotected archives must be candidates regardless of archive validity") assert.equal(dryRun.candidateBytes, 24) @@ -36,28 +38,20 @@ try { bytes: 36, lockCount: 1, referenceCount: 2, + filesystemCount: 0, paths: [lockedArchive, referencedArchive].sort(), }) assert.equal(dryRun.removedCount, 0) - assert.deepEqual(await customArchiveNames(), ["custom-invalid.zip", "custom-locked.zip", "custom-referenced.zip", "custom-stale.zip"]) await firstReference.release() const sharedUse = await maintainPlaygroundCustomArchiveCache(root, { mode: "diagnose", now, maxAgeMs: 1_000 }) assert.equal(sharedUse.activeProtection.referenceCount, 1, "one shared user must keep the archive protected") - assert.ok(sharedUse.activeProtection.paths.includes(referencedArchive)) - - const applied = await maintainPlaygroundCustomArchiveCache(root, { - mode: "apply", - now, - maxAgeMs: 1_000, - maxBytes: 1_000, - maxCount: 20, - }) + + const staleAllocated = await allocatedBytes(join(root, "custom-stale.zip")) + await allocatedBytes(join(root, "custom-invalid.zip")) + const applied = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1_000, maxBytes: 1_000, maxCount: 20 }) assert.equal(applied.removedCount, 2) assert.equal(applied.removedBytes, 24) - assert.equal(applied.verifiedReclaimedBytes, 24) - assert.equal(applied.retainedCount, 2) - assert.equal(applied.retainedBytes, 36) + assert.equal(applied.verifiedReclaimedBytes, staleAllocated) 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") @@ -65,66 +59,120 @@ try { await rm(activeLock, { recursive: true }) const afterRelease = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1_000 }) assert.equal(afterRelease.removedCount, 2, "teardown must make formerly active archives reclaimable") - assert.deepEqual(await customArchiveNames(), []) for (let index = 0; index < 5; index += 1) { await archive(`custom-bound-${index}.zip`, 10, now - (5 - index) * 1_000) } - const bounds = await maintainPlaygroundCustomArchiveCache(root, { - mode: "apply", - now, - maxAgeMs: 1_000_000, - maxBytes: 25, - maxCount: 2, - }) - assert.equal(bounds.candidateCount, 3, "count and byte bounds must retain only the newest fitting archives") + const bounds = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1_000_000, maxBytes: 25, maxCount: 2 }) + assert.equal(bounds.candidateCount, 3) assert.equal(bounds.removedCount, 3) - assert.equal(bounds.verifiedReclaimedBytes, 30) assert.deepEqual(await customArchiveNames(), ["custom-bound-3.zip", "custom-bound-4.zip"]) - - const idempotent = await maintainPlaygroundCustomArchiveCache(root, { - mode: "apply", - now, - maxAgeMs: 1_000_000, - maxBytes: 25, - maxCount: 2, - }) + const idempotent = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1_000_000, maxBytes: 25, maxCount: 2 }) assert.equal(idempotent.candidateCount, 0) assert.equal(idempotent.removedCount, 0) - assert.equal(idempotent.verifiedReclaimedBytes, 0) - - const crashedArchive = await archive("custom-crashed.zip", 23, now - 100_000) - const referencesDirectory = `${crashedArchive}.refs` - await mkdir(referencesDirectory) - await writeFile(join(referencesDirectory, "stale.json"), JSON.stringify({ pid: 2_147_483_647, createdAt: new Date(now - 100_000).toISOString() })) - const crashedLock = `${crashedArchive}.lock` - await mkdir(crashedLock) - await writeFile(join(crashedLock, "owner.json"), JSON.stringify({ pid: 2_147_483_647, createdAt: new Date(now - 100_000).toISOString() })) - const crashRecovery = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1_000 }) - assert.ok(crashRecovery.removedCount >= 1, "dead process locks and references must not retain stale archives forever") - assert.ok(!(await customArchiveNames()).includes("custom-crashed.zip")) - assert.ok((await readdir(root)).includes(stableArchive.split("/").at(-1)!)) + const linkedTarget = join(root, "linked-target.zip") + await writeFile(linkedTarget, Buffer.alloc(8_192, "h")) + const hardLink = join(root, "custom-hard-link.zip") + const symbolicLink = join(root, "custom-symbolic-link.zip") + await link(linkedTarget, hardLink) + await symlink(linkedTarget, symbolicLink) + await utimes(hardLink, (now - 100_000) / 1_000, (now - 100_000) / 1_000) + const links = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1 }) + assert.equal(links.activeProtection.filesystemCount, 2) + assert.ok(links.diagnostics.some((item) => item.code === "custom-archive-multiply-linked")) + assert.ok(links.diagnostics.some((item) => item.code === "custom-archive-not-regular")) + assert.equal((await lstat(linkedTarget)).size, 8_192) + assert.ok((await readdir(root)).includes("custom-hard-link.zip")) + assert.ok((await readdir(root)).includes("custom-symbolic-link.zip")) + + const pidReuseArchive = await archive("custom-pid-reuse.zip", 23, now - 100_000) + await mkdir(`${pidReuseArchive}.refs`) + await writeLease(join(`${pidReuseArchive}.refs`, "reused-pid.json"), { token: "different-process-token", expiresAt: now + 60_000, pid: process.pid, processStart: "different-start" }) + const pidReuse = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1 }) + assert.ok(pidReuse.activeProtection.paths.includes(pidReuseArchive), "PID reuse must not invalidate an unexpired token lease") + + const expiredArchive = await archive("custom-expired-lease.zip", 29, now - 100_000) + await mkdir(`${expiredArchive}.refs`) + await writeLease(join(`${expiredArchive}.refs`, "expired.json"), { token: "expired-token", expiresAt: now - 1, pid: process.pid }) + const expired = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1 }) + assert.ok(!await exists(expiredArchive), "expiry, not PID identity, must reclaim crashed leases") + + const orphanArchive = join(root, "custom-orphan.zip") + await mkdir(`${orphanArchive}.refs`) + await writeLease(join(`${orphanArchive}.refs`, "active.json"), { token: "foreign-reference", expiresAt: now + 60_000, ownerHostname: "remote-container" }) + const orphanLock = `${join(root, "custom-orphan-lock.zip")}.lock` + await mkdir(orphanLock) + await writeLease(join(orphanLock, "owner.json"), { token: "expired-orphan-lock", expiresAt: now - 1 }) + const orphanFirst = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now }) + assert.equal(orphanFirst.sidecars.orphanCount, 2) + assert.equal(orphanFirst.sidecars.activeCount, 1) + assert.equal(orphanFirst.sidecars.removedCount, 1) + assert.ok(await exists(`${orphanArchive}.refs`), "foreign unexpired orphan lease must remain protected") + await writeLease(join(`${orphanArchive}.refs`, "active.json"), { token: "foreign-reference", expiresAt: now - 1, ownerHostname: "remote-container" }) + const orphanExpired = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now }) + assert.equal(orphanExpired.sidecars.removedCount, 1) + assert.ok(!await exists(`${orphanArchive}.refs`)) + + const concurrentArchive = await archive("custom-concurrent.zip", 31, now - 100_000) + const concurrentReference = await acquirePlaygroundArchiveReference(concurrentArchive) + const concurrentMaintenance = await Promise.all(Array.from({ length: 4 }, () => maintainPlaygroundCustomArchiveCache(root, { mode: "apply", maxAgeMs: 1 }))) + assert.ok(await exists(concurrentArchive), "concurrent maintenance must not race an acquired reference") + assert.ok(concurrentMaintenance.some((result) => result.activeProtection.paths.includes(concurrentArchive))) + await concurrentReference.release() + + const lockRaceArchive = await archive("custom-lock-race.zip", 37, now - 100_000) + let lockAcquired!: () => void + let releaseLock!: () => void + const acquired = new Promise((resolve) => { lockAcquired = resolve }) + const held = new Promise((resolve) => { releaseLock = resolve }) + const lockHolder = withPlaygroundArchiveCacheLock(root, "custom-lock-race", async () => { + lockAcquired() + await held + }) + await acquired + const duringAcquisition = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", maxAgeMs: 1 }) + assert.ok(duringAcquisition.activeProtection.paths.includes(lockRaceArchive), "maintenance must protect an archive while another actor holds its acquisition lock") + releaseLock() + await lockHolder + + process.env.WP_CODEBOX_PLAYGROUND_WORDPRESS_CACHE_DIR = root + process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_AGE_MS = "0" + process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_BYTES = "1" + process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT = "0" + assert.equal(playgroundWordPressArchiveCacheDirectory(), root) + const environmentPolicy = await maintainPlaygroundCustomArchiveCache(undefined, { mode: "dry-run" }) + assert.equal(environmentPolicy.policy.maxAgeMs, 0) + assert.equal(environmentPolicy.policy.maxBytes, 1) + assert.equal(environmentPolicy.policy.maxCount, 0) + process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT = "invalid" + await assert.rejects(() => maintainPlaygroundCustomArchiveCache(), /must be a finite number/) + delete process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT const recoveredVersion = "custom-recovered" const recoveredLock = join(root, `${recoveredVersion}.zip.lock`) await mkdir(recoveredLock) - await writeFile(join(recoveredLock, "owner.json"), JSON.stringify({ pid: 2_147_483_647, createdAt: new Date(now - 100_000).toISOString() })) + await writeLease(join(recoveredLock, "owner.json"), { token: "expired-producer", expiresAt: Date.now() - 1 }) let acquiredRecoveredLock = false await withPlaygroundArchiveCacheLock(root, recoveredVersion, async () => { acquiredRecoveredLock = true }) - assert.equal(acquiredRecoveredLock, true, "a crashed archive producer must not block future cache users") + assert.equal(acquiredRecoveredLock, true, "an expired producer lease must not block future cache users") + assert.ok((await readdir(root)).includes(stableArchive.split("/").at(-1)!)) console.log("playground custom archive cache retention passed") } finally { + for (const name of environmentNames) { + const value = originalEnvironment[name] + if (value === undefined) delete process.env[name] + else process.env[name] = value + } await rm(root, { recursive: true, force: true }) } async function archive(name: string, size: number, mtimeMs: number, content = "x"): Promise { const path = join(root, name) - const bytes = Buffer.alloc(size, content) - await writeFile(path, bytes) + await writeFile(path, Buffer.alloc(size, content)) await utimes(path, mtimeMs / 1_000, mtimeMs / 1_000) return path } @@ -132,3 +180,31 @@ async function archive(name: string, size: number, mtimeMs: number, content = "x async function customArchiveNames(): Promise { return (await readdir(root)).filter((name) => /^custom-.*\.zip$/.test(name)).sort() } + +async function allocatedBytes(path: string): Promise { + return (await lstat(path)).blocks * 512 +} + +async function exists(path: string): Promise { + try { + await lstat(path) + return true + } catch (error) { + return Boolean(error && typeof error === "object" && "code" in error && error.code === "ENOENT") ? false : Promise.reject(error) + } +} + +async function writeLease(path: string, options: { token: string; expiresAt: number; ownerHostname?: string; pid?: number; processStart?: string }): Promise { + const timestamp = new Date(now).toISOString() + await writeFile(path, JSON.stringify({ + schema: "wp-codebox/playground-cache-lease/v1", + token: options.token, + hostname: options.ownerHostname ?? hostname(), + bootId: "test-boot", + pid: options.pid ?? 987_654, + processStart: options.processStart ?? "test-start", + createdAt: timestamp, + heartbeatAt: timestamp, + expiresAt: new Date(options.expiresAt).toISOString(), + })) +} From 5dbba3c4747ccf603c0c1f2391fdae2dbd947857 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Tue, 21 Jul 2026 14:08:08 +0000 Subject: [PATCH 2/6] fix: make Playground cache retention generation-safe --- README.md | 2 +- .../src/playground-cli-runner.ts | 4 +- .../src/playground-wordpress-archive-cache.ts | 331 ++++++++++++------ ...d-custom-archive-cache.integration.test.ts | 12 +- tests/playground-custom-archive-cache.test.ts | 58 ++- 5 files changed, 284 insertions(+), 123 deletions(-) diff --git a/README.md b/README.md index 5de6d2f6..9b3bcf3f 100644 --- a/README.md +++ b/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 ` 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, and allocated bytes verified as reclaimed. +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. Remote-host previews can opt into `--preview-bind ` 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. diff --git a/packages/runtime-playground/src/playground-cli-runner.ts b/packages/runtime-playground/src/playground-cli-runner.ts index 63c6d70c..5767df1a 100644 --- a/packages/runtime-playground/src/playground-cli-runner.ts +++ b/packages/runtime-playground/src/playground-cli-runner.ts @@ -90,13 +90,13 @@ export async function startPlaygroundCliServer(spec: RuntimeCreateSpec, mounts: const stagedMounts = readonlyMountStaging.mounts const preinstallMounts = stagedMounts.filter((mount) => mount.target === "/wordpress/wp-config.php") const postinstallMounts = stagedMounts.filter((mount) => mount.target !== "/wordpress/wp-config.php") - const wordpressStartupAsset = wordpressDirectory ? undefined : await resolvePlaygroundWordPressStartupAsset(spec.environment.version, spec.environment.assets?.wordpressZip) - archiveReference = wordpressStartupAsset?.archiveReference if (usesArchiveCache) { const maintenance = await automaticPlaygroundCustomArchiveCacheMaintenance() cacheMaintenance = maintenance.result cacheMaintenanceDiagnostics = maintenance.diagnostics } + const wordpressStartupAsset = wordpressDirectory ? undefined : await resolvePlaygroundWordPressStartupAsset(spec.environment.version, spec.environment.assets?.wordpressZip) + archiveReference = wordpressStartupAsset?.archiveReference const cacheValidation = wordpressStartupAsset?.cacheValidation ?? { version: spec.environment.version ?? "mounted-wordpress-source", sourceUrl: wordpressDirectory ?? "", diff --git a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts index 7c11cffa..8e8b4f6a 100644 --- a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts +++ b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts @@ -1,7 +1,8 @@ import { randomUUID } from "node:crypto" +import { constants } from "node:fs" import { hostname, homedir } from "node:os" -import { lstat, mkdir, readFile, readdir, rename, rm, unlink, writeFile } from "node:fs/promises" -import { basename, dirname, join } from "node:path" +import { lstat, mkdir, open, readFile, readdir, rmdir, statfs, unlink, type FileHandle } from "node:fs/promises" +import { basename, join } from "node:path" const CUSTOM_ARCHIVE_PATTERN = /^custom-[A-Za-z0-9._-]+\.zip$/ const CUSTOM_SIDECAR_PATTERN = /^(custom-[A-Za-z0-9._-]+\.zip)\.(lock|refs)$/ @@ -32,6 +33,7 @@ interface CacheOwnerRecord { interface LeaseHandle { token: string + renew(): Promise release(): Promise } @@ -48,9 +50,15 @@ export interface PlaygroundArchiveCacheLock { export interface PlaygroundArchiveReference { archivePath: string path: string + renew(): Promise release(): Promise } +export interface PlaygroundArchiveReferenceOptions { + leaseMs?: number + heartbeat?: boolean +} + export interface PlaygroundCustomArchiveCachePolicy { maxAgeMs: number maxBytes: number @@ -96,6 +104,7 @@ export interface PlaygroundCustomArchiveCacheMaintenance { } removedCount: number removedBytes: number + estimatedAllocatedBytesRemoved: number verifiedReclaimedBytes: number retainedCount: number retainedBytes: number @@ -142,17 +151,26 @@ export async function withPlaygroundArchiveCacheLock(cacheDirectory: string, } } -export async function acquirePlaygroundArchiveReference(archivePath: string): Promise { +export async function acquirePlaygroundArchiveReference(archivePath: string, options: PlaygroundArchiveReferenceOptions = {}): Promise { const policy = cacheLeasePolicy() + const leaseMs = options.leaseMs ?? policy.leaseMs const referencesDirectory = `${archivePath}.refs` - await mkdir(referencesDirectory, { recursive: true }) + const directoryHandle = await openSafeLeaseDirectory(referencesDirectory, true) const token = randomUUID() - const referencePath = join(referencesDirectory, `${token}.json`) - const lease = await createFileLease(referencePath, token, policy.leaseMs) + const fileName = `${token}.json` + const referencePath = join(referencesDirectory, fileName) + let lease: LeaseHandle + try { + lease = await createFileLease(directoryHandle, fileName, referencePath, token, leaseMs, referencesDirectory, options.heartbeat !== false) + } catch (error) { + await directoryHandle.close() + throw error + } return { archivePath, path: referencePath, + renew: lease.renew, release: lease.release, } } @@ -188,7 +206,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play } const lockPath = `${archivePath}.lock` const activeLock = await cacheLockIsActive(lockPath, now, policy, mode === "apply") - const activeReferences = await activeArchiveReferenceCount(archivePath, now, policy, mode === "apply") + const activeReferences = await activeArchiveReferenceCount(archivePath, now, policy, mode === "apply", diagnostics) entries.push({ path: archivePath, size: regular ? archiveStat.size : 0, @@ -224,7 +242,8 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play let removedCount = 0 let removedBytes = 0 - let verifiedReclaimedBytes = 0 + let estimatedAllocatedBytesRemoved = 0 + const filesystemBefore = mode === "apply" ? await filesystemAvailableBytes(cacheDirectory) : undefined if (mode === "apply") { for (const candidate of candidates.sort(oldestFirst)) { const lease = await tryAcquireCacheLock(candidate.lockPath, policy.leaseMs) @@ -232,7 +251,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play continue } try { - if (await activeArchiveReferenceCount(candidate.path, Date.now(), policy, true) > 0) { + if (await activeArchiveReferenceCount(candidate.path, Date.now(), policy, true, diagnostics) > 0) { continue } let current @@ -259,7 +278,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play } removedCount += 1 removedBytes += current.size - verifiedReclaimedBytes += currentAllocatedBytes + estimatedAllocatedBytesRemoved += currentAllocatedBytes await removeOrphanReferencesDirectory(`${candidate.path}.refs`, Date.now(), policy, diagnostics) } } finally { @@ -269,6 +288,8 @@ 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 totalBytes = entries.reduce((total, entry) => total + entry.size, 0) return { schema: "wp-codebox/playground-custom-archive-cache-maintenance/v1", @@ -290,6 +311,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play sidecars, removedCount, removedBytes, + estimatedAllocatedBytesRemoved, verifiedReclaimedBytes, retainedCount: entries.length - removedCount, retainedBytes: totalBytes - removedBytes, @@ -324,106 +346,153 @@ async function tryAcquireCacheLock(lockPath: string, leaseMs: number): Promise undefined) + await rmdir(lockPath).catch(() => undefined) throw error } } -async function createDirectoryLease(lockPath: string, token: string, leaseMs: number): Promise { - const ownerPath = join(lockPath, "owner.json") - await writeLease(ownerPath, await ownerRecord(token, leaseMs)) - return heartbeatLease(ownerPath, token, leaseMs, async () => { - await quarantineDirectoryIfOwned(lockPath, token) - }) +async function createDirectoryLease(directoryHandle: FileHandle, lockPath: string, token: string, leaseMs: number): Promise { + const fileName = `${token}.lease.json` + return createFileLease(directoryHandle, fileName, join(lockPath, fileName), token, leaseMs, lockPath, true) } -async function createFileLease(path: string, token: string, leaseMs: number): Promise { - await writeLease(path, await ownerRecord(token, leaseMs), true) - return heartbeatLease(path, token, leaseMs, async () => { - const owner = await readOwnerRecord(path) - if (owner?.token === token) { - await rm(path, { force: true }) - } - }) +async function createFileLease(directoryHandle: FileHandle, fileName: string, displayPath: string, token: string, leaseMs: number, directoryPath: string, automaticHeartbeat: boolean): Promise { + const anchoredPath = join(fileDescriptorPath(directoryHandle), fileName) + const fileHandle = await open(anchoredPath, constants.O_CREAT | constants.O_EXCL | constants.O_RDWR | constants.O_NOFOLLOW, 0o600) + await writeOwnerToHandle(fileHandle, await ownerRecord(token, leaseMs)) + return heartbeatLease(fileHandle, token, leaseMs, async () => { + await unlink(anchoredPath).catch((error) => { + if (!errorHasCode(error, "ENOENT")) throw error + }) + await fileHandle.close() + await directoryHandle.close() + await rmdir(directoryPath).catch((error) => { + if (!errorHasCode(error, "ENOENT") && !errorHasCode(error, "ENOTEMPTY")) throw error + }) + }, displayPath, automaticHeartbeat) } -function heartbeatLease(path: string, token: string, leaseMs: number, removeOwned: () => Promise): LeaseHandle { +function heartbeatLease(fileHandle: FileHandle, token: string, leaseMs: number, removeOwned: () => Promise, displayPath: string, automaticHeartbeat: boolean): LeaseHandle { let stopped = false let heartbeat: Promise | undefined - const interval = setInterval(() => { + const renew = async () => refreshLease(fileHandle, token, leaseMs) + const interval = automaticHeartbeat ? setInterval(() => { if (stopped || heartbeat) { return } - heartbeat = refreshLease(path, token, leaseMs).catch(() => undefined).finally(() => { + heartbeat = refreshLease(fileHandle, token, leaseMs).catch((error) => { + console.warn(`[wp-codebox] playground-cache-lease-refresh-failed: ${displayPath}: ${errorMessage(error)}`) + }).finally(() => { heartbeat = undefined }) - }, Math.max(50, Math.floor(leaseMs / 3))) - interval.unref() + }, Math.max(50, Math.floor(leaseMs / 3))) : undefined + interval?.unref() return { token, + renew, async release() { stopped = true - clearInterval(interval) + if (interval) clearInterval(interval) await heartbeat?.catch(() => undefined) await removeOwned() }, } } -async function refreshLease(path: string, token: string, leaseMs: number): Promise { - const owner = await readOwnerRecord(path) +async function refreshLease(fileHandle: FileHandle, token: string, leaseMs: number): Promise { + const owner = await readOwnerFromHandle(fileHandle) if (owner?.token !== token) { return } const now = new Date() - await writeLease(path, { ...owner, heartbeatAt: now.toISOString(), expiresAt: new Date(now.getTime() + leaseMs).toISOString() }) + await writeOwnerToHandle(fileHandle, { ...owner, heartbeatAt: now.toISOString(), expiresAt: new Date(now.getTime() + leaseMs).toISOString() }) } async function cacheLockIsActive(lockPath: string, now: number, policy: Pick, removeStale: boolean): Promise { - const state = await leaseState(join(lockPath, "owner.json"), lockPath, now, policy.staleLockMs) - if (!state.active && removeStale) { - if (state.token) { - await quarantineDirectoryIfOwned(lockPath, state.token) - } else { - await quarantineLegacyDirectoryIfStale(lockPath, now, policy.staleLockMs) + let lockStat + try { + lockStat = await lstat(lockPath) + } catch (error) { + if (errorHasCode(error, "ENOENT")) return false + throw error + } + if (!lockStat.isDirectory() || lockStat.isSymbolicLink()) return true + const directoryHandle = await openSafeLeaseDirectory(lockPath, false) + let active = false + try { + const anchoredDirectory = fileDescriptorPath(directoryHandle) + const names = await readdir(anchoredDirectory) + for (const name of names.sort()) { + const path = join(anchoredDirectory, name) + if (name !== "owner.json" && !name.endsWith(".lease.json")) { + const unknownStat = await lstat(path) + if (unknownStat.isDirectory() || now - lockStat.mtimeMs <= policy.staleLockMs || !removeStale) { + active = true + } else { + await unlink(path) + } + continue + } + const state = await leaseState(path, path, now, policy.staleLockMs) + if (state.active) { + active = true + } else if (removeStale) { + await unlink(path).catch((error) => { + if (!errorHasCode(error, "ENOENT")) throw error + }) + } + } + if (names.length === 0 && now - lockStat.mtimeMs <= policy.staleLockMs) active = true + } finally { + await directoryHandle.close() + if (removeStale && !active) { + await rmdir(lockPath).catch((error) => { + if (!errorHasCode(error, "ENOENT") && !errorHasCode(error, "ENOTEMPTY")) throw error + }) } } - return state.active + return active } -async function activeArchiveReferenceCount(archivePath: string, now: number, policy: Pick, removeStale: boolean): Promise { +async function activeArchiveReferenceCount(archivePath: string, now: number, policy: Pick, removeStale: boolean, diagnostics: PlaygroundCustomArchiveCacheDiagnostic[] = []): Promise { const referencesDirectory = `${archivePath}.refs` - let names: string[] + let referencesStat try { - names = await readdir(referencesDirectory) + referencesStat = await lstat(referencesDirectory) } catch (error) { - if (errorHasCode(error, "ENOENT") || errorHasCode(error, "ENOTDIR")) { - return 0 - } + if (errorHasCode(error, "ENOENT")) return 0 throw error } + if (!referencesStat.isDirectory() || referencesStat.isSymbolicLink()) { + diagnostics.push({ code: "custom-archive-refs-not-directory", message: "Archive reference sidecar is not a real directory and was not followed.", severity: "warning", path: referencesDirectory }) + if (removeStale) await unlink(referencesDirectory) + return removeStale ? 0 : 1 + } + const directoryHandle = await openSafeLeaseDirectory(referencesDirectory, false) + const anchoredDirectory = fileDescriptorPath(directoryHandle) + const names = await readdir(anchoredDirectory) let active = 0 - for (const name of names.sort()) { - const path = join(referencesDirectory, name) - if (name.startsWith(".") && name.endsWith(".tmp")) { - if (removeStale) { - await rm(path, { force: true }) - } - continue - } - const state = await leaseState(path, path, now, policy.staleLockMs) - if (state.active) { - active += 1 - } else if (removeStale) { - const current = await readOwnerRecord(path) - if (!state.token || current?.token === state.token) { - await rm(path, { recursive: true, force: true }) + try { + for (const name of names.sort()) { + const path = join(anchoredDirectory, name) + const state = await leaseState(path, path, now, policy.staleLockMs) + if (state.active) { + active += 1 + } else if (removeStale) { + await unlink(path).catch((error) => { + if (!errorHasCode(error, "ENOENT")) throw error + }) } } + } finally { + await directoryHandle.close() } return active } @@ -468,6 +537,8 @@ async function maintainOrphanSidecars(cacheDirectory: string, names: string[], n const active = await activeReferenceCountInDirectory(path, now, policy, mode === "apply") if (active > 0) { activeCount += 1 + } else if (mode === "apply" && !await pathExists(path)) { + removedCount += 1 } else if (mode === "apply" && await removeOrphanReferencesDirectory(path, now, policy, diagnostics)) { removedCount += 1 } @@ -480,16 +551,26 @@ async function activeReferenceCountInDirectory(path: string, now: number, policy } async function removeOrphanReferencesDirectory(path: string, now: number, policy: Pick, diagnostics: PlaygroundCustomArchiveCacheDiagnostic[]): Promise { + let sidecarStat + try { + sidecarStat = await lstat(path) + } catch (error) { + if (errorHasCode(error, "ENOENT")) return false + throw error + } + if (!sidecarStat.isDirectory() || sidecarStat.isSymbolicLink()) { + diagnostics.push({ code: "custom-archive-refs-not-directory", message: "Archive reference sidecar is not a real directory and was unlinked without following it.", severity: "warning", path }) + await unlink(path) + return true + } if (await activeReferenceCountInDirectory(path, now, policy, true) > 0) { return false } try { - const quarantined = `${path}.cleanup-${randomUUID()}` - await rename(path, quarantined) - await rm(quarantined, { recursive: true, force: true }) + await rmdir(path) return true } catch (error) { - if (errorHasCode(error, "ENOENT")) { + if (errorHasCode(error, "ENOENT") || errorHasCode(error, "ENOTEMPTY")) { return false } diagnostics.push({ code: "orphan-reference-cleanup-failed", message: errorMessage(error), severity: "warning", path }) @@ -497,55 +578,6 @@ async function removeOrphanReferencesDirectory(path: string, now: number, policy } } -async function quarantineDirectoryIfOwned(path: string, token: string): Promise { - if ((await readOwnerRecord(join(path, "owner.json")))?.token !== token) { - return false - } - return quarantineDirectory(path) -} - -async function quarantineLegacyDirectoryIfStale(path: string, now: number, staleMs: number): Promise { - try { - if (now - (await lstat(path)).mtimeMs <= staleMs || await readOwnerRecord(join(path, "owner.json"))) { - return false - } - return quarantineDirectory(path) - } catch (error) { - if (errorHasCode(error, "ENOENT")) { - return false - } - throw error - } -} - -async function quarantineDirectory(path: string): Promise { - const quarantined = `${path}.cleanup-${randomUUID()}` - try { - await rename(path, quarantined) - } catch (error) { - if (errorHasCode(error, "ENOENT")) { - return false - } - throw error - } - await rm(quarantined, { recursive: true, force: true }) - return true -} - -async function writeLease(path: string, owner: CacheOwnerRecord, exclusive = false): Promise { - if (exclusive) { - await writeFile(path, `${JSON.stringify(owner)}\n`, { flag: "wx" }) - return - } - const temporaryPath = join(dirname(path), `.${basename(path)}.${owner.token}.tmp`) - await writeFile(temporaryPath, `${JSON.stringify(owner)}\n`) - try { - await rename(temporaryPath, path) - } finally { - await rm(temporaryPath, { force: true }) - } -} - async function readOwnerRecord(path: string): Promise { try { const value = JSON.parse(await readFile(path, "utf8")) as Partial @@ -568,6 +600,64 @@ async function readOwnerRecord(path: string): Promise { + if (create) { + await mkdir(path).catch((error) => { + if (!errorHasCode(error, "EEXIST")) throw error + }) + } + const pathStat = await lstat(path) + if (!pathStat.isDirectory() || pathStat.isSymbolicLink()) { + throw new Error(`Playground cache lease sidecar must be a real directory: ${path}`) + } + const handle = await open(path, constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW) + const handleStat = await handle.stat() + if (handleStat.dev !== pathStat.dev || handleStat.ino !== pathStat.ino) { + await handle.close() + throw new Error(`Playground cache lease sidecar changed while opening: ${path}`) + } + return handle +} + +function fileDescriptorPath(handle: FileHandle): string { + if (process.platform === "linux") return `/proc/self/fd/${handle.fd}` + if (process.platform === "darwin" || process.platform === "freebsd") return `/dev/fd/${handle.fd}` + throw new Error("Playground cache leases require a platform with file-descriptor paths") +} + +async function readOwnerFromHandle(handle: FileHandle): Promise { + try { + const fileStat = await handle.stat() + const contents = Buffer.alloc(fileStat.size) + await handle.read(contents, 0, contents.length, 0) + return normalizeOwnerRecord(JSON.parse(contents.toString("utf8"))) + } catch (error) { + if (error instanceof SyntaxError || errorHasCode(error, "ENOENT")) return undefined + throw error + } +} + +async function writeOwnerToHandle(handle: FileHandle, owner: CacheOwnerRecord): Promise { + const contents = Buffer.from(`${JSON.stringify(owner)}\n`) + await handle.write(contents, 0, contents.length, 0) + await handle.truncate(contents.length) + await handle.sync() +} + +function normalizeOwnerRecord(value: Partial): CacheOwnerRecord | undefined { + 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 +} + async function ownerRecord(token: string, leaseMs: number): Promise { const identity = await processIdentityPromise const now = new Date() @@ -599,6 +689,15 @@ function allocatedBytes(blocks: number): number { return Math.max(0, blocks) * 512 } +async function filesystemAvailableBytes(path: string): Promise { + try { + const filesystem = await statfs(path) + return filesystem.bavail * filesystem.bsize + } catch { + return undefined + } +} + function newestFirst(left: CustomArchiveEntry, right: CustomArchiveEntry): number { return right.mtimeMs - left.mtimeMs || left.path.localeCompare(right.path) } diff --git a/tests/playground-custom-archive-cache.integration.test.ts b/tests/playground-custom-archive-cache.integration.test.ts index 3b9a3400..5518d986 100644 --- a/tests/playground-custom-archive-cache.integration.test.ts +++ b/tests/playground-custom-archive-cache.integration.test.ts @@ -1,6 +1,7 @@ import assert from "node:assert/strict" +import { existsSync } from "node:fs" import { createServer } from "node:http" -import { mkdtemp, readdir, rm } from "node:fs/promises" +import { mkdtemp, readdir, rm, utimes, writeFile } from "node:fs/promises" import { tmpdir } from "node:os" import { join } from "node:path" @@ -12,7 +13,10 @@ const environmentNames = ["WP_CODEBOX_PLAYGROUND_WORDPRESS_CACHE_DIR", "WP_CODEB const originalEnvironment = Object.fromEntries(environmentNames.map((name) => [name, process.env[name]])) const zip = Buffer.alloc(22) zip.set([0x50, 0x4b, 0x05, 0x06]) +const staleBeforeDownload = join(root, "custom-stale-before-download.zip") +let requestObservedAfterCleanup = false const source = createServer((_request, response) => { + requestObservedAfterCleanup = !existsSync(staleBeforeDownload) response.writeHead(200, { "content-type": "application/zip", "content-length": String(zip.length) }) response.end(zip) }) @@ -46,13 +50,17 @@ try { process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_AGE_MS = "0" process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_MAX_COUNT = "0" process.env.WP_CODEBOX_PLAYGROUND_CUSTOM_ARCHIVE_LEASE_MS = "300" + await writeFile(staleBeforeDownload, Buffer.alloc(8_192, "s")) + await utimes(staleBeforeDownload, 1, 1) const server = await startPlaygroundCliServer(spec, [], { cliModule, onProgress(event) { progress.push(event as unknown as Record) } }) + assert.equal(requestObservedAfterCleanup, true, "bounded maintenance must run before resolving or downloading the next archive") await new Promise((resolve) => setTimeout(resolve, 0)) const connecting = progress.find((event) => event.phase === "preview:connecting-client") const cacheValidation = (connecting?.detail as Record | undefined)?.cacheValidation as Record | undefined assert.equal((cacheValidation?.retention as Record | undefined)?.schema, "wp-codebox/playground-custom-archive-cache-maintenance/v1") - assert.equal(((cacheValidation?.retention as Record).activeProtection as Record).referenceCount, 1) + assert.equal((cacheValidation?.retention as Record).removedCount, 1) + assert.equal(((cacheValidation?.retention as Record).activeProtection as Record).referenceCount, 0) await server[Symbol.asyncDispose]() 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") diff --git a/tests/playground-custom-archive-cache.test.ts b/tests/playground-custom-archive-cache.test.ts index 2643dcf6..4dac550c 100644 --- a/tests/playground-custom-archive-cache.test.ts +++ b/tests/playground-custom-archive-cache.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict" -import { link, lstat, mkdir, mkdtemp, readdir, readFile, rm, symlink, utimes, writeFile } from "node:fs/promises" +import { link, lstat, mkdir, mkdtemp, open, readdir, readFile, rm, symlink, utimes, writeFile } from "node:fs/promises" import { hostname, tmpdir } from "node:os" import { join } from "node:path" @@ -51,7 +51,8 @@ try { const applied = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1_000, maxBytes: 1_000, maxCount: 20 }) assert.equal(applied.removedCount, 2) assert.equal(applied.removedBytes, 24) - assert.equal(applied.verifiedReclaimedBytes, staleAllocated) + assert.equal(applied.estimatedAllocatedBytesRemoved, staleAllocated) + assert.ok(applied.verifiedReclaimedBytes >= 0) 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") @@ -86,6 +87,48 @@ try { assert.ok((await readdir(root)).includes("custom-hard-link.zip")) assert.ok((await readdir(root)).includes("custom-symbolic-link.zip")) + const attackTarget = await mkdtemp(join(tmpdir(), "wp-codebox-playground-refs-attack-")) + const attackMarker = join(attackTarget, "do-not-delete.txt") + await writeFile(attackMarker, "outside cache") + const attackedArchive = await archive("custom-refs-attack.zip", 41, now - 100_000) + await symlink(attackTarget, `${attackedArchive}.refs`) + await assert.rejects(() => acquirePlaygroundArchiveReference(attackedArchive), /must be a real directory/) + const attackCleanup = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now, maxAgeMs: 1 }) + assert.ok(attackCleanup.diagnostics.some((item) => item.code === "custom-archive-refs-not-directory")) + assert.equal(await readFile(attackMarker, "utf8"), "outside cache", "reference cleanup must never follow a sidecar symlink") + assert.ok(!await exists(`${attackedArchive}.refs`)) + const orphanAttack = join(root, "custom-orphan-attack.zip.refs") + await symlink(attackTarget, orphanAttack) + const orphanAttackCleanup = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now }) + assert.ok(!await exists(orphanAttack)) + assert.equal(await readFile(attackMarker, "utf8"), "outside cache") + await rm(attackTarget, { 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)) + await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", maxAgeMs: 1 }) + await archive("custom-generation-race.zip", 43, Date.now()) + const replacementGeneration = await acquirePlaygroundArchiveReference(generationArchive, { leaseMs: 1_000, heartbeat: false }) + const replacementBefore = await readFile(replacementGeneration.path, "utf8") + await oldGeneration.renew() + assert.equal(await readFile(replacementGeneration.path, "utf8"), replacementBefore, "an old generation heartbeat must not overwrite its replacement") + await oldGeneration.release() + assert.ok(await exists(replacementGeneration.path), "old generation release must not remove replacement ownership") + await replacementGeneration.release() + + 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")) + await utimes(openArchive, 1, 1) + const openHandle = await open(openArchive, "r") + 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") + await openHandle.close() + await rm(allocationRoot, { recursive: true, force: true }) + const pidReuseArchive = await archive("custom-pid-reuse.zip", 23, now - 100_000) await mkdir(`${pidReuseArchive}.refs`) await writeLease(join(`${pidReuseArchive}.refs`, "reused-pid.json"), { token: "different-process-token", expiresAt: now + 60_000, pid: process.pid, processStart: "different-start" }) @@ -158,6 +201,17 @@ try { acquiredRecoveredLock = true }) assert.equal(acquiredRecoveredLock, true, "an expired producer lease must not block future cache users") + + const recoveredTempVersion = "custom-recovered-temp" + const recoveredTempLock = join(root, `${recoveredTempVersion}.zip.lock`) + await mkdir(recoveredTempLock) + await writeFile(join(recoveredTempLock, ".owner.json.crashed.tmp"), "partial") + await utimes(recoveredTempLock, 1, 1) + let acquiredRecoveredTempLock = false + await withPlaygroundArchiveCacheLock(root, recoveredTempVersion, async () => { + acquiredRecoveredTempLock = true + }) + assert.equal(acquiredRecoveredTempLock, true, "stale heartbeat temp files from an older generation must not wedge archive acquisition") assert.ok((await readdir(root)).includes(stableArchive.split("/").at(-1)!)) console.log("playground custom archive cache retention passed") From 4e4a83968ed8465b79a45a9c53495403336f8ac5 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Tue, 21 Jul 2026 14:24:45 +0000 Subject: [PATCH 3/6] fix: harden cache retention race checks --- README.md | 2 +- .../src/playground-cli-runner.ts | 8 +- .../src/playground-wordpress-archive-cache.ts | 99 ++++++++++--------- ...d-custom-archive-cache.integration.test.ts | 6 +- tests/playground-custom-archive-cache.test.ts | 32 +++++- 5 files changed, 97 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 9b3bcf3f..c3dff3ba 100644 --- a/README.md +++ b/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 ` 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 ` 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. diff --git a/packages/runtime-playground/src/playground-cli-runner.ts b/packages/runtime-playground/src/playground-cli-runner.ts index 5767df1a..d4bf1022 100644 --- a/packages/runtime-playground/src/playground-cli-runner.ts +++ b/packages/runtime-playground/src/playground-cli-runner.ts @@ -631,7 +631,13 @@ function errorDetail(error: unknown): Record { 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", diff --git a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts index 8e8b4f6a..4831b907 100644 --- a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts +++ b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts @@ -70,6 +70,7 @@ export interface PlaygroundCustomArchiveCachePolicy { export interface PlaygroundCustomArchiveCacheMaintenanceOptions extends Partial { mode?: "diagnose" | "dry-run" | "apply" now?: number + candidateInterlock?: (archivePath: string) => void | Promise } 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 { - 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 { - 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 { + 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, removeStale: boolean): Promise { - return activeArchiveReferenceCount(path.slice(0, -".refs".length), now, policy, removeStale) +async function activeReferenceCountInDirectory(path: string, now: number, policy: Pick, removeStale: boolean, diagnostics: PlaygroundCustomArchiveCacheDiagnostic[] = []): Promise { + return activeArchiveReferenceCount(path.slice(0, -".refs".length), now, policy, removeStale, diagnostics) } async function removeOrphanReferencesDirectory(path: string, now: number, policy: Pick, diagnostics: PlaygroundCustomArchiveCacheDiagnostic[]): Promise { @@ -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 { - try { - const value = JSON.parse(await readFile(path, "utf8")) as Partial - 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 { if (create) { await mkdir(path).catch((error) => { diff --git a/tests/playground-custom-archive-cache.integration.test.ts b/tests/playground-custom-archive-cache.integration.test.ts index 5518d986..5de55ab8 100644 --- a/tests/playground-custom-archive-cache.integration.test.ts +++ b/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 | undefined)?.schema, "wp-codebox/playground-custom-archive-cache-maintenance/v1") assert.equal((cacheValidation?.retention as Record).removedCount, 1) assert.equal(((cacheValidation?.retention as Record).activeProtection as Record).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> = [] const invalidServer = await startPlaygroundCliServer(spec, [], { cliModule, onProgress(event) { invalidProgress.push(event as unknown as Record) } }) await new Promise((resolve) => setTimeout(resolve, 0)) diff --git a/tests/playground-custom-archive-cache.test.ts b/tests/playground-custom-archive-cache.test.ts index 4dac550c..47819112 100644 --- a/tests/playground-custom-archive-cache.test.ts +++ b/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 }) From 32d0032e780cc46852edb6b0f72324c91aee794f Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Tue, 21 Jul 2026 15:10:25 +0000 Subject: [PATCH 4/6] fix: preserve replacement cache leases on release --- .../src/playground-wordpress-archive-cache.ts | 28 +++++++++++++++++-- tests/playground-custom-archive-cache.test.ts | 12 +++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts index 4831b907..2dc3e590 100644 --- a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts +++ b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts @@ -377,9 +377,7 @@ async function createFileLease(directoryHandle: FileHandle, fileName: string, di const fileHandle = await open(anchoredPath, constants.O_CREAT | constants.O_EXCL | constants.O_RDWR | constants.O_NOFOLLOW, 0o600) await writeOwnerToHandle(fileHandle, await ownerRecord(token, leaseMs)) return heartbeatLease(fileHandle, token, leaseMs, async () => { - await unlink(anchoredPath).catch((error) => { - if (!errorHasCode(error, "ENOENT")) throw error - }) + await unlinkOwnedLeasePath(anchoredPath, fileHandle, token) await fileHandle.close() await directoryHandle.close() await rmdir(directoryPath).catch((error) => { @@ -388,6 +386,30 @@ async function createFileLease(directoryHandle: FileHandle, fileName: string, di }, displayPath, automaticHeartbeat) } +async function unlinkOwnedLeasePath(path: string, ownedHandle: FileHandle, token: string): Promise { + let currentHandle: FileHandle | undefined + try { + const ownedStat = await ownedHandle.stat() + currentHandle = await open(path, constants.O_RDONLY | constants.O_NOFOLLOW) + const currentStat = await currentHandle.stat() + const currentOwner = await readOwnerFromHandle(currentHandle) + if (currentStat.dev !== ownedStat.dev || currentStat.ino !== ownedStat.ino || currentOwner?.token !== token) { + return false + } + const pathStat = await lstat(path) + if (pathStat.dev !== ownedStat.dev || pathStat.ino !== ownedStat.ino || !pathStat.isFile() || pathStat.isSymbolicLink()) { + return false + } + await unlink(path) + return true + } catch (error) { + if (errorHasCode(error, "ENOENT") || errorHasCode(error, "ELOOP")) return false + throw error + } finally { + await currentHandle?.close() + } +} + function heartbeatLease(fileHandle: FileHandle, token: string, leaseMs: number, removeOwned: () => Promise, displayPath: string, automaticHeartbeat: boolean): LeaseHandle { let stopped = false let heartbeat: Promise | undefined diff --git a/tests/playground-custom-archive-cache.test.ts b/tests/playground-custom-archive-cache.test.ts index 47819112..116dfda1 100644 --- a/tests/playground-custom-archive-cache.test.ts +++ b/tests/playground-custom-archive-cache.test.ts @@ -129,6 +129,16 @@ try { assert.ok(await exists(replacementGeneration.path), "old generation release must not remove replacement ownership") await replacementGeneration.release() + const samePathArchive = await archive("custom-same-path-release.zip", 45, Date.now()) + const staleSamePath = await acquirePlaygroundArchiveReference(samePathArchive, { leaseMs: 1_000, heartbeat: false }) + await rm(staleSamePath.path) + await writeLease(staleSamePath.path, { token: "same-path-replacement-token", expiresAt: Date.now() + 60_000 }) + const samePathReplacement = await readFile(staleSamePath.path, "utf8") + await staleSamePath.release() + assert.equal(await readFile(staleSamePath.path, "utf8"), samePathReplacement, "stale release must not unlink a replacement at the exact same pathname") + await rm(staleSamePath.path) + await rm(`${samePathArchive}.refs`, { recursive: true }) + const replacedCandidate = await archive("custom-replaced-candidate.zip", 47, now - 100_000) let replaced = false const replacementResult = await maintainPlaygroundCustomArchiveCache(root, { @@ -153,7 +163,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.observedFilesystemFreeBytesDelta, 0, "open unlinked allocation should not increase observed free space until the final handle closes") + assert.ok(Number.isFinite(openRemoval.observedFilesystemFreeBytesDelta), "filesystem free-space delta is observational and may include concurrent allocation changes") await openHandle.close() await rm(allocationRoot, { recursive: true, force: true }) From 8cb0630567741607b008b51bcc5235b8c9ff41cd Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Tue, 21 Jul 2026 15:23:33 +0000 Subject: [PATCH 5/6] fix: expire cache leases without pathname unlink --- .../src/playground-wordpress-archive-cache.ts | 46 ++++++------------- tests/playground-custom-archive-cache.test.ts | 33 ++++++++++--- 2 files changed, 40 insertions(+), 39 deletions(-) diff --git a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts index 2dc3e590..63c6a01f 100644 --- a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts +++ b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts @@ -57,6 +57,7 @@ export interface PlaygroundArchiveReference { export interface PlaygroundArchiveReferenceOptions { leaseMs?: number heartbeat?: boolean + releaseInterlock?: (leasePath: string) => void | Promise } export interface PlaygroundCustomArchiveCachePolicy { @@ -164,7 +165,7 @@ export async function acquirePlaygroundArchiveReference(archivePath: string, opt const referencePath = join(referencesDirectory, fileName) let lease: LeaseHandle try { - lease = await createFileLease(directoryHandle, fileName, referencePath, token, leaseMs, referencesDirectory, options.heartbeat !== false) + lease = await createFileLease(directoryHandle, fileName, referencePath, token, leaseMs, options.heartbeat !== false, options.releaseInterlock) } catch (error) { await directoryHandle.close() throw error @@ -297,7 +298,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play } } - const sidecars = await maintainOrphanSidecars(cacheDirectory, directoryEntries.map((entry) => entry.name), now, policy, mode, diagnostics) + const sidecars = await maintainOrphanSidecars(cacheDirectory, await readdir(cacheDirectory), now, policy, mode, diagnostics) const filesystemAfter = mode === "apply" ? await filesystemAvailableBytes(cacheDirectory) : undefined const observedFilesystemFreeBytesDelta = filesystemBefore === undefined || filesystemAfter === undefined ? 0 : filesystemAfter - filesystemBefore const totalBytes = entries.reduce((total, entry) => total + entry.size, 0) @@ -369,47 +370,20 @@ async function tryAcquireCacheLock(lockPath: string, leaseMs: number): Promise { const fileName = `${token}.lease.json` - return createFileLease(directoryHandle, fileName, join(lockPath, fileName), token, leaseMs, lockPath, true) + return createFileLease(directoryHandle, fileName, join(lockPath, fileName), token, leaseMs, true) } -async function createFileLease(directoryHandle: FileHandle, fileName: string, displayPath: string, token: string, leaseMs: number, directoryPath: string, automaticHeartbeat: boolean): Promise { +async function createFileLease(directoryHandle: FileHandle, fileName: string, displayPath: string, token: string, leaseMs: number, automaticHeartbeat: boolean, releaseInterlock?: (leasePath: string) => void | Promise): Promise { const anchoredPath = join(fileDescriptorPath(directoryHandle), fileName) const fileHandle = await open(anchoredPath, constants.O_CREAT | constants.O_EXCL | constants.O_RDWR | constants.O_NOFOLLOW, 0o600) await writeOwnerToHandle(fileHandle, await ownerRecord(token, leaseMs)) return heartbeatLease(fileHandle, token, leaseMs, async () => { - await unlinkOwnedLeasePath(anchoredPath, fileHandle, token) + await releaseInterlock?.(displayPath) await fileHandle.close() await directoryHandle.close() - await rmdir(directoryPath).catch((error) => { - if (!errorHasCode(error, "ENOENT") && !errorHasCode(error, "ENOTEMPTY")) throw error - }) }, displayPath, automaticHeartbeat) } -async function unlinkOwnedLeasePath(path: string, ownedHandle: FileHandle, token: string): Promise { - let currentHandle: FileHandle | undefined - try { - const ownedStat = await ownedHandle.stat() - currentHandle = await open(path, constants.O_RDONLY | constants.O_NOFOLLOW) - const currentStat = await currentHandle.stat() - const currentOwner = await readOwnerFromHandle(currentHandle) - if (currentStat.dev !== ownedStat.dev || currentStat.ino !== ownedStat.ino || currentOwner?.token !== token) { - return false - } - const pathStat = await lstat(path) - if (pathStat.dev !== ownedStat.dev || pathStat.ino !== ownedStat.ino || !pathStat.isFile() || pathStat.isSymbolicLink()) { - return false - } - await unlink(path) - return true - } catch (error) { - if (errorHasCode(error, "ENOENT") || errorHasCode(error, "ELOOP")) return false - throw error - } finally { - await currentHandle?.close() - } -} - function heartbeatLease(fileHandle: FileHandle, token: string, leaseMs: number, removeOwned: () => Promise, displayPath: string, automaticHeartbeat: boolean): LeaseHandle { let stopped = false let heartbeat: Promise | undefined @@ -433,11 +407,19 @@ function heartbeatLease(fileHandle: FileHandle, token: string, leaseMs: number, stopped = true if (interval) clearInterval(interval) await heartbeat?.catch(() => undefined) + await expireLease(fileHandle, token) await removeOwned() }, } } +async function expireLease(fileHandle: FileHandle, token: string): Promise { + const owner = await readOwnerFromHandle(fileHandle) + if (owner?.token !== token) return + const now = new Date().toISOString() + await writeOwnerToHandle(fileHandle, { ...owner, heartbeatAt: now, expiresAt: new Date(0).toISOString() }) +} + async function refreshLease(fileHandle: FileHandle, token: string, leaseMs: number): Promise { const owner = await readOwnerFromHandle(fileHandle) if (owner?.token !== token) { diff --git a/tests/playground-custom-archive-cache.test.ts b/tests/playground-custom-archive-cache.test.ts index 116dfda1..ad0a986b 100644 --- a/tests/playground-custom-archive-cache.test.ts +++ b/tests/playground-custom-archive-cache.test.ts @@ -130,10 +130,16 @@ try { await replacementGeneration.release() const samePathArchive = await archive("custom-same-path-release.zip", 45, Date.now()) - const staleSamePath = await acquirePlaygroundArchiveReference(samePathArchive, { leaseMs: 1_000, heartbeat: false }) - await rm(staleSamePath.path) - await writeLease(staleSamePath.path, { token: "same-path-replacement-token", expiresAt: Date.now() + 60_000 }) - const samePathReplacement = await readFile(staleSamePath.path, "utf8") + let samePathReplacement = "" + const staleSamePath = await acquirePlaygroundArchiveReference(samePathArchive, { + leaseMs: 1_000, + heartbeat: false, + async releaseInterlock(path) { + await rm(path) + await writeLease(path, { token: "same-path-replacement-token", expiresAt: Date.now() + 60_000 }) + samePathReplacement = await readFile(path, "utf8") + }, + }) await staleSamePath.release() assert.equal(await readFile(staleSamePath.path, "utf8"), samePathReplacement, "stale release must not unlink a replacement at the exact same pathname") await rm(staleSamePath.path) @@ -186,13 +192,13 @@ try { await mkdir(orphanLock) await writeLease(join(orphanLock, "owner.json"), { token: "expired-orphan-lock", expiresAt: now - 1 }) const orphanFirst = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now }) - assert.equal(orphanFirst.sidecars.orphanCount, 2) + assert.ok(orphanFirst.sidecars.orphanCount >= 2) assert.equal(orphanFirst.sidecars.activeCount, 1) - assert.equal(orphanFirst.sidecars.removedCount, 1) + assert.ok(orphanFirst.sidecars.removedCount >= 1) assert.ok(await exists(`${orphanArchive}.refs`), "foreign unexpired orphan lease must remain protected") await writeLease(join(`${orphanArchive}.refs`, "active.json"), { token: "foreign-reference", expiresAt: now - 1, ownerHostname: "remote-container" }) const orphanExpired = await maintainPlaygroundCustomArchiveCache(root, { mode: "apply", now }) - assert.equal(orphanExpired.sidecars.removedCount, 1) + assert.ok(orphanExpired.sidecars.removedCount >= 1) assert.ok(!await exists(`${orphanArchive}.refs`)) const concurrentArchive = await archive("custom-concurrent.zip", 31, now - 100_000) @@ -250,6 +256,19 @@ try { acquiredRecoveredTempLock = true }) assert.equal(acquiredRecoveredTempLock, true, "stale heartbeat temp files from an older generation must not wedge archive acquisition") + + const samePathLockVersion = "custom-same-path-lock-release" + const samePathLockDirectory = join(root, `${samePathLockVersion}.zip.lock`) + let replacementLockEntry = "" + let replacementLockContents = "" + await withPlaygroundArchiveCacheLock(root, samePathLockVersion, async () => { + replacementLockEntry = join(samePathLockDirectory, (await readdir(samePathLockDirectory)).find((name) => name.endsWith(".lease.json"))!) + await rm(replacementLockEntry) + await writeLease(replacementLockEntry, { token: "same-path-lock-replacement", expiresAt: Date.now() + 60_000 }) + replacementLockContents = await readFile(replacementLockEntry, "utf8") + }) + assert.equal(await readFile(replacementLockEntry, "utf8"), replacementLockContents, "producer lock release must not unlink a replacement at the same generation pathname") + await rm(samePathLockDirectory, { recursive: true }) assert.ok((await readdir(root)).includes(stableArchive.split("/").at(-1)!)) console.log("playground custom archive cache retention passed") From 545d0ff84d6bc593e0582278626eeacb0bba954c Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Tue, 21 Jul 2026 15:38:23 +0000 Subject: [PATCH 6/6] fix: tolerate concurrent cache lock reclamation --- .../src/playground-wordpress-archive-cache.ts | 143 +++++++++++++----- tests/playground-custom-archive-cache.test.ts | 10 ++ 2 files changed, 112 insertions(+), 41 deletions(-) diff --git a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts index 63c6a01f..c2333e68 100644 --- a/packages/runtime-playground/src/playground-wordpress-archive-cache.ts +++ b/packages/runtime-playground/src/playground-wordpress-archive-cache.ts @@ -42,6 +42,8 @@ interface LeaseState { token?: string } +class LeaseSidecarUnsafeError extends Error {} + export interface PlaygroundArchiveCacheLock { path: string waitedMs: number @@ -223,7 +225,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play filesystemProtected, }) } catch (error) { - if (!errorHasCode(error, "ENOENT")) { + if (!isConcurrentDisappearance(error)) { throw error } } @@ -265,7 +267,7 @@ export async function maintainPlaygroundCustomArchiveCache(cacheDirectory = play try { current = await lstat(candidate.path) } catch (error) { - if (errorHasCode(error, "ENOENT")) { + if (isConcurrentDisappearance(error)) { continue } throw error @@ -364,6 +366,7 @@ async function tryAcquireCacheLock(lockPath: string, leaseMs: number): Promise undefined) await rmdir(lockPath).catch(() => undefined) + if (isConcurrentDisappearance(error)) return undefined throw error } } @@ -434,33 +437,48 @@ async function cacheLockIsActive(lockPath: string, now: number, policy: Pick { - if (!errorHasCode(error, "ENOENT")) throw error - }) } } if (names.length === 0 && now - lockStat.mtimeMs <= policy.staleLockMs) active = true @@ -468,7 +486,7 @@ async function cacheLockIsActive(lockPath: string, now: number, policy: Pick { - if (!errorHasCode(error, "ENOENT") && !errorHasCode(error, "ENOTEMPTY")) throw error + if (!isConcurrentDisappearance(error) && !errorHasCode(error, "ENOTEMPTY")) throw error }) } } @@ -481,17 +499,34 @@ async function activeArchiveReferenceCount(archivePath: string, now: number, pol try { referencesStat = await lstat(referencesDirectory) } catch (error) { - if (errorHasCode(error, "ENOENT")) return 0 + if (isConcurrentDisappearance(error)) return 0 throw error } if (!referencesStat.isDirectory() || referencesStat.isSymbolicLink()) { diagnostics.push({ code: "custom-archive-refs-not-directory", message: "Archive reference sidecar is not a real directory and was not followed.", severity: "warning", path: referencesDirectory }) - if (removeStale) await unlink(referencesDirectory) + if (removeStale) await unlinkIfPresent(referencesDirectory) return removeStale ? 0 : 1 } - const directoryHandle = await openSafeLeaseDirectory(referencesDirectory, false) + let directoryHandle: FileHandle + try { + directoryHandle = await openSafeLeaseDirectory(referencesDirectory, false) + } catch (error) { + if (isConcurrentDisappearance(error)) return 0 + if (error instanceof LeaseSidecarUnsafeError || errorHasCode(error, "ELOOP") || errorHasCode(error, "ENOTDIR")) { + diagnostics.push({ code: "custom-archive-refs-changed", message: "Archive reference sidecar changed while opening and was conservatively protected.", severity: "warning", path: referencesDirectory }) + return 1 + } + throw error + } const anchoredDirectory = fileDescriptorPath(directoryHandle) - const names = await readdir(anchoredDirectory) + let names: string[] + try { + names = await readdir(anchoredDirectory) + } catch (error) { + await directoryHandle.close() + if (isConcurrentDisappearance(error)) return 0 + throw error + } let active = 0 try { for (const name of names.sort()) { @@ -512,12 +547,12 @@ async function inspectLeaseFile(path: string, now: number, legacyStaleMs: number try { pathStat = await lstat(path) } catch (error) { - if (errorHasCode(error, "ENOENT")) return { active: false } + if (isConcurrentDisappearance(error)) 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) + if (removeUnsafe && !pathStat.isDirectory()) await unlinkIfPresent(path) return { active: !removeUnsafe || pathStat.isDirectory() } } @@ -533,14 +568,14 @@ async function inspectLeaseFile(path: string, now: number, legacyStaleMs: number 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) + if (removeUnsafe) await unlinkIfPresent(path) return { active: false, token: owner.token } } const active = now - pathStat.mtimeMs <= legacyStaleMs - if (!active && removeUnsafe) await unlink(path) + if (!active && removeUnsafe) await unlinkIfPresent(path) return { active } } catch (error) { - if (errorHasCode(error, "ENOENT")) return { active: false } + if (isConcurrentDisappearance(error)) 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 } @@ -592,13 +627,12 @@ async function removeOrphanReferencesDirectory(path: string, now: number, policy try { sidecarStat = await lstat(path) } catch (error) { - if (errorHasCode(error, "ENOENT")) return false + if (isConcurrentDisappearance(error)) return false throw error } if (!sidecarStat.isDirectory() || sidecarStat.isSymbolicLink()) { diagnostics.push({ code: "custom-archive-refs-not-directory", message: "Archive reference sidecar is not a real directory and was unlinked without following it.", severity: "warning", path }) - await unlink(path) - return true + return unlinkIfPresent(path) } if (await activeReferenceCountInDirectory(path, now, policy, true, diagnostics) > 0) { return false @@ -607,7 +641,7 @@ async function removeOrphanReferencesDirectory(path: string, now: number, policy await rmdir(path) return true } catch (error) { - if (errorHasCode(error, "ENOENT") || errorHasCode(error, "ENOTEMPTY")) { + if (isConcurrentDisappearance(error) || errorHasCode(error, "ENOTEMPTY")) { return false } diagnostics.push({ code: "orphan-reference-cleanup-failed", message: errorMessage(error), severity: "warning", path }) @@ -616,22 +650,35 @@ async function removeOrphanReferencesDirectory(path: string, now: number, policy } async function openSafeLeaseDirectory(path: string, create: boolean): Promise { - if (create) { - await mkdir(path).catch((error) => { - if (!errorHasCode(error, "EEXIST")) throw error - }) - } - const pathStat = await lstat(path) - if (!pathStat.isDirectory() || pathStat.isSymbolicLink()) { - throw new Error(`Playground cache lease sidecar must be a real directory: ${path}`) - } - const handle = await open(path, constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW) - const handleStat = await handle.stat() - if (handleStat.dev !== pathStat.dev || handleStat.ino !== pathStat.ino) { - await handle.close() - throw new Error(`Playground cache lease sidecar changed while opening: ${path}`) + for (let attempt = 0; attempt < (create ? 3 : 1); attempt += 1) { + try { + if (create) { + await mkdir(path).catch((error) => { + if (!errorHasCode(error, "EEXIST")) throw error + }) + } + const pathStat = await lstat(path) + if (!pathStat.isDirectory() || pathStat.isSymbolicLink()) { + throw new LeaseSidecarUnsafeError(`Playground cache lease sidecar must be a real directory: ${path}`) + } + const handle = await open(path, constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW) + let handleStat + try { + handleStat = await handle.stat() + } catch (error) { + await handle.close() + throw error + } + if (handleStat.dev !== pathStat.dev || handleStat.ino !== pathStat.ino) { + await handle.close() + throw new LeaseSidecarUnsafeError(`Playground cache lease sidecar changed while opening: ${path}`) + } + return handle + } catch (error) { + if (!create || !isConcurrentDisappearance(error) || attempt === 2) throw error + } } - return handle + throw new Error(`Unable to create Playground cache lease sidecar: ${path}`) } function fileDescriptorPath(handle: FileHandle): string { @@ -725,6 +772,20 @@ function errorHasCode(error: unknown, code: string): boolean { return Boolean(error && typeof error === "object" && "code" in error && error.code === code) } +function isConcurrentDisappearance(error: unknown): boolean { + return errorHasCode(error, "ENOENT") || errorHasCode(error, "ESTALE") +} + +async function unlinkIfPresent(path: string): Promise { + try { + await unlink(path) + return true + } catch (error) { + if (isConcurrentDisappearance(error)) return false + throw error + } +} + function errorMessage(error: unknown): string { return error instanceof Error ? error.message.slice(0, 500) : String(error).slice(0, 500) } @@ -734,7 +795,7 @@ async function pathExists(path: string): Promise { await lstat(path) return true } catch (error) { - if (errorHasCode(error, "ENOENT")) { + if (isConcurrentDisappearance(error)) { return false } throw error diff --git a/tests/playground-custom-archive-cache.test.ts b/tests/playground-custom-archive-cache.test.ts index ad0a986b..18934d3e 100644 --- a/tests/playground-custom-archive-cache.test.ts +++ b/tests/playground-custom-archive-cache.test.ts @@ -257,6 +257,16 @@ try { }) assert.equal(acquiredRecoveredTempLock, true, "stale heartbeat temp files from an older generation must not wedge archive acquisition") + const stressRoot = await mkdtemp(join(tmpdir(), "wp-codebox-playground-maintenance-stress-")) + for (let iteration = 0; iteration < 25; iteration += 1) { + const stressArchive = join(stressRoot, `custom-stress-${iteration}.zip`) + await writeFile(stressArchive, Buffer.alloc(4_096, "s")) + await utimes(stressArchive, 1, 1) + await Promise.all(Array.from({ length: 4 }, () => maintainPlaygroundCustomArchiveCache(stressRoot, { mode: "apply", maxAgeMs: 1, maxCount: 0 }))) + } + assert.deepEqual((await readdir(stressRoot)).filter((name) => name.startsWith("custom-")), [], "concurrent maintenance must converge without inspect-open disappearance failures") + await rm(stressRoot, { recursive: true, force: true }) + const samePathLockVersion = "custom-same-path-lock-release" const samePathLockDirectory = join(root, `${samePathLockVersion}.zip.lock`) let replacementLockEntry = ""