Skip to content

Commit db1fe89

Browse files
authored
Merge pull request #2004 from Automattic/fix/concurrent-wp-cli-temp-files
Fix concurrent WP-CLI wrapper collisions
2 parents 5a4cce5 + 3237ca7 commit db1fe89

7 files changed

Lines changed: 144 additions & 21 deletions

File tree

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@
269269
"test:wordpress-hotspots-contracts": "tsx tests/wordpress-hotspots-contracts.test.ts",
270270
"test:cache-churn-observation-contracts": "tsx tests/cache-churn-observation-contracts.test.ts",
271271
"test:wordpress-db-contracts": "tsx tests/wordpress-db-contracts.test.ts",
272+
"test:wp-cli-temporary-script": "tsx tests/wp-cli-temporary-script.test.ts",
272273
"test:browser-sdk-facade": "tsx tests/browser-sdk-facade.test.ts",
273274
"check": "npm run test:production-boundary-enforcement && npm run smoke -- --group=check"
274275
},

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

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { browserWordPressDiagnosticProvider, isBrowserCommandArtifactError, runB
1414
import type { PluginCheckArtifact, ThemeCheckArtifact } from "./check-artifacts.js"
1515
import { executePlaygroundCommand } from "./command-router.js"
1616
import { firstCommandWordPressAdminAuthRequirement } from "./command-auth-requirements.js"
17-
import { argValue, cleanWpCliOutput, shellArgv, wordpressBlockExerciseInputFromArgs, wordpressBlockExercisePhpCode, wordpressCrudOperationFromArgs, wordpressCrudOperationPhpCode, wordpressDbOperationFromArgs, wordpressDbOperationPhpCode, wpCliCommandFromArgs, wpCliPhpScript } from "./commands.js"
17+
import { argValue, cleanWpCliOutput, runWithTemporaryWpCliScript, shellArgv, wordpressBlockExerciseInputFromArgs, wordpressBlockExercisePhpCode, wordpressCrudOperationFromArgs, wordpressCrudOperationPhpCode, wordpressDbOperationFromArgs, wordpressDbOperationPhpCode, wpCliCommandFromArgs } from "./commands.js"
1818
import { bootstrapPhpCode } from "./php-bootstrap.js"
1919
import { observeHttpResponse as observeHttpResponseArtifact, observeWordPressState as observeWordPressStateArtifact } from "./observation-artifacts.js"
2020
import { PlaygroundCommandCrashError, assertPlaygroundResponseOk, errorMessage, type PlaygroundRunResponse } from "./playground-command-errors.js"
@@ -1054,13 +1054,7 @@ class PlaygroundRuntime implements Runtime {
10541054
throw new Error("wordpress.wp-cli requires a non-empty command")
10551055
}
10561056

1057-
if (!server.playground.writeFile) {
1058-
throw new Error("wordpress.wp-cli requires a Playground backend with writeFile support")
1059-
}
1060-
1061-
const scriptPath = `/tmp/wp-codebox-wp-cli-${this.commands.length}.php`
1062-
await server.playground.writeFile(scriptPath, wpCliPhpScript(argv))
1063-
const response = await this.runPlaygroundCommand("wordpress.wp-cli", server, { scriptPath })
1057+
const response = await this.runWpCliCommand(server, argv)
10641058
assertPlaygroundResponseOk("wordpress.wp-cli", response)
10651059

10661060
return cleanWpCliOutput(response.text)
@@ -1471,13 +1465,18 @@ class PlaygroundRuntime implements Runtime {
14711465
}
14721466

14731467
private async runWpCliCommand(server: PlaygroundCliServer, argv: string[]): Promise<PlaygroundRunResponse> {
1474-
if (!server.playground.writeFile) {
1475-
throw new Error("WP-CLI commands require a Playground backend with writeFile support")
1468+
if (!server.playground.writeFile || !server.playground.unlink) {
1469+
throw new Error("WP-CLI commands require a Playground backend with writeFile and unlink support")
14761470
}
1477-
1478-
const scriptPath = `/tmp/wp-codebox-wp-cli-${this.commands.length}-${Date.now().toString(36)}.php`
1479-
await server.playground.writeFile(scriptPath, wpCliPhpScript(argv))
1480-
return this.runPlaygroundCommand("wordpress.wp-cli", server, { scriptPath })
1471+
return runWithTemporaryWpCliScript(
1472+
{
1473+
writeFile: (path, contents) => server.playground.writeFile!(path, contents),
1474+
unlink: (path) => server.playground.unlink!(path),
1475+
},
1476+
this.runtimeId,
1477+
argv,
1478+
(scriptPath) => this.runPlaygroundCommand("wordpress.wp-cli", server, { scriptPath }),
1479+
)
14811480
}
14821481

14831482
private async createRuntimeWpCliBridge(server: PlaygroundCliServer): Promise<RuntimeWpCliBridge> {
@@ -1758,13 +1757,7 @@ class PlaygroundRuntime implements Runtime {
17581757
}
17591758

17601759
private async runWpCliArgv(server: PlaygroundCliServer, argv: string[]): Promise<PlaygroundRunResponse> {
1761-
if (!server.playground.writeFile) {
1762-
throw new Error("WP-CLI commands require a Playground backend with writeFile support")
1763-
}
1764-
1765-
const scriptPath = `/tmp/wp-codebox-wp-cli-${this.commands.length}-${Date.now().toString(36)}.php`
1766-
await server.playground.writeFile(scriptPath, wpCliPhpScript(argv))
1767-
return this.runPlaygroundCommand("wordpress.wp-cli", server, { scriptPath })
1760+
return this.runWpCliCommand(server, argv)
17681761
}
17691762

17701763
private async runPlaygroundCommand(command: string, server: PlaygroundCliServer, options: { code: string } | { scriptPath: string }): Promise<PlaygroundRunResponse> {

packages/runtime-playground/src/preview-server.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export interface PlaygroundCliServer {
1313
run(options: { code: string } | { scriptPath: string }): Promise<PlaygroundServerRunResponse>
1414
onMessage?(listener: (data: string) => Promise<string | void> | string | void): Promise<(() => Promise<void> | void) | void> | (() => Promise<void> | void) | void
1515
readFileAsText?(path: string): string | Promise<string>
16+
unlink?(path: string): Promise<void> | void
1617
writeFile?(path: string, contents: string): Promise<void>
1718
}
1819
serverUrl: string

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ interface ProgrammaticPHP {
2323
onMessage(listener: (data: string) => Promise<string | void> | string | void): () => Promise<void>
2424
readFileAsText(path: string): string
2525
run(options: { code: string } | { scriptPath: string }): Promise<ProgrammaticPHPResponse>
26+
unlink(path: string): void
2627
writeFile(path: string, contents: string): void
2728
}
2829

@@ -91,6 +92,9 @@ export async function startProgrammaticPlaygroundServer(spec: RuntimeCreateSpec,
9192
run: (runOptions) => runPhp(primaryPhp, runOptions),
9293
onMessage: (listener) => primaryPhp.onMessage(listener),
9394
readFileAsText: (path) => primaryPhp.readFileAsText(path),
95+
unlink(path) {
96+
primaryPhp.unlink(path)
97+
},
9498
async writeFile(path, contents) {
9599
primaryPhp.writeFile(path, contents)
96100
},

packages/runtime-playground/src/wp-cli-command-handlers.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
1+
import { randomBytes } from "node:crypto"
12
import { argValue } from "./command-args.js"
23
import { phpCliStreamConstants } from "./php-snippets.js"
34

5+
interface WpCliTemporaryScriptFilesystem {
6+
writeFile(path: string, contents: string): Promise<void>
7+
unlink(path: string): Promise<void> | void
8+
}
9+
410
export function wpCliCommandFromArgs(args: string[]): string {
511
const explicit = argValue(args, "command")
612
if (explicit) {
@@ -58,6 +64,17 @@ require '/tmp/wp-cli.phar';
5864
`
5965
}
6066

67+
export async function runWithTemporaryWpCliScript<T>(filesystem: WpCliTemporaryScriptFilesystem, runtimeId: string, argv: string[], run: (scriptPath: string) => Promise<T>): Promise<T> {
68+
const runtimeNamespace = runtimeId.replace(/[^A-Za-z0-9_-]/g, "-").slice(0, 80) || "runtime"
69+
const scriptPath = `/tmp/wp-codebox-wp-cli-${runtimeNamespace}-${randomBytes(16).toString("hex")}.php`
70+
await filesystem.writeFile(scriptPath, wpCliPhpScript(argv))
71+
try {
72+
return await run(scriptPath)
73+
} finally {
74+
await Promise.resolve(filesystem.unlink(scriptPath)).catch(() => undefined)
75+
}
76+
}
77+
6178
export function cleanWpCliOutput(output: string): string {
6279
return output.replace(/^#!\/usr\/bin\/env php\r?\n/, "")
6380
}

scripts/smoke-manifest.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ export const smokeGroups = {
120120
tsxSmoke("replay-export-snapshot-scoping-smoke"),
121121
tsxSmoke("runtime-overlay-validation-smoke"),
122122
npmScript("test:runtime-php-snippets"),
123+
npmScript("test:wp-cli-temporary-script"),
123124
npmScript("test:php-runtime-provider-registry"),
124125
tsxSmoke("composer-backed-source-hydration-smoke"),
125126
tsxSmoke("composer-package-overlay-autoload-layout-smoke"),
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import assert from "node:assert/strict"
2+
import { runWithTemporaryWpCliScript, shellArgv } from "../packages/runtime-playground/dist/wp-cli-command-handlers.js"
3+
4+
class MemoryFilesystem {
5+
readonly files = new Map<string, string>()
6+
readonly writtenPaths: string[] = []
7+
readonly unlinkedPaths: string[] = []
8+
9+
async writeFile(path: string, contents: string): Promise<void> {
10+
if (this.files.has(path)) {
11+
throw new Error(`Could not write to ${JSON.stringify(path)}: File exists.`)
12+
}
13+
this.files.set(path, contents)
14+
this.writtenPaths.push(path)
15+
}
16+
17+
unlink(path: string): void {
18+
this.files.delete(path)
19+
this.unlinkedPaths.push(path)
20+
}
21+
}
22+
23+
const oldPaths = Array.from({ length: 16 }, () => "/tmp/wp-codebox-wp-cli-2.php")
24+
assert.equal(new Set(oldPaths).size, 1, "the old command-count suffix reproduces the shared wrapper path")
25+
26+
const filesystem = new MemoryFilesystem()
27+
let invocationCount = 0
28+
for (const { concurrency, iterations, repetitions } of [
29+
{ concurrency: 2, iterations: 128, repetitions: 3 },
30+
{ concurrency: 6, iterations: 256, repetitions: 3 },
31+
{ concurrency: 16, iterations: 512, repetitions: 2 },
32+
{ concurrency: 64, iterations: 1_024, repetitions: 1 },
33+
]) {
34+
for (let repetition = 0; repetition < repetitions; repetition++) {
35+
const markers = Array.from({ length: iterations }, (_, index) => `marker-${concurrency}-${repetition}-${index.toString().padStart(6, "0")}`)
36+
const outputs = await runBounded(markers, concurrency, async (marker, index) => {
37+
invocationCount++
38+
return runWithTemporaryWpCliScript(
39+
filesystem,
40+
"runtime/shared/../../unsafe",
41+
["eval", `echo ${JSON.stringify(marker)};`],
42+
async (scriptPath) => {
43+
await new Promise((resolve) => setTimeout(resolve, index % 5))
44+
const script = filesystem.files.get(scriptPath)
45+
assert.ok(script, `wrapper must exist while ${marker} executes`)
46+
assert.match(scriptPath, /^\/tmp\/wp-codebox-wp-cli-runtime-shared-------unsafe-[a-f0-9]{32}\.php$/)
47+
assert.ok(script.includes(marker), `${marker} must execute only its own payload`)
48+
const otherMarker = markers[(index + 1) % markers.length]
49+
assert.equal(script.includes(otherMarker), false, `${marker} must not execute ${otherMarker}`)
50+
return marker
51+
},
52+
)
53+
})
54+
assert.deepEqual(outputs, markers)
55+
assert.equal(filesystem.files.size, 0, `concurrency ${concurrency} must return the temp directory to baseline`)
56+
}
57+
}
58+
59+
assert.equal(new Set(filesystem.writtenPaths).size, invocationCount)
60+
assert.deepEqual(new Set(filesystem.unlinkedPaths), new Set(filesystem.writtenPaths))
61+
assert.throws(() => shellArgv("eval 'unterminated"), /Unclosed quote/, "malformed WP-CLI input must fail before allocating a wrapper")
62+
63+
const nonzero = await runWithTemporaryWpCliScript(filesystem, "runtime-nonzero", ["eval", "exit(7)"], async () => ({ exitCode: 7, text: "", errors: "intentional" }))
64+
assert.deepEqual(nonzero, { exitCode: 7, text: "", errors: "intentional" }, "structured nonzero responses must remain compatible")
65+
assert.equal(filesystem.files.size, 0, "structured nonzero responses must clean up their wrapper")
66+
67+
for (const failure of [
68+
new Error("wrapper execution failure"),
69+
new Error("runtime command exceeded timeout"),
70+
new Error("runtime execution was aborted"),
71+
new Error("adapter exception"),
72+
]) {
73+
await assert.rejects(
74+
runWithTemporaryWpCliScript(filesystem, "runtime-failures", ["eval", "broken"], async () => { throw failure }),
75+
failure,
76+
)
77+
assert.equal(filesystem.files.size, 0, `${failure.message} must clean up its owned wrapper`)
78+
}
79+
80+
let unlinkedAfterRejectedWrite = false
81+
await assert.rejects(
82+
runWithTemporaryWpCliScript({
83+
async writeFile() {
84+
throw new Error("File exists")
85+
},
86+
unlink() {
87+
unlinkedAfterRejectedWrite = true
88+
},
89+
}, "runtime-write-failure", ["version"], async () => "unreachable"),
90+
/File exists/,
91+
)
92+
assert.equal(unlinkedAfterRejectedWrite, false, "a rejected write must not claim or remove another invocation's file")
93+
94+
console.log(`WP-CLI temporary wrapper stress ok: ${invocationCount} isolated invocations at concurrency 2, 6, 16, and 64`)
95+
96+
async function runBounded<T, R>(items: T[], concurrency: number, operation: (item: T, index: number) => Promise<R>): Promise<R[]> {
97+
const outputs = new Array<R>(items.length)
98+
let nextIndex = 0
99+
await Promise.all(Array.from({ length: Math.min(concurrency, items.length) }, async () => {
100+
while (nextIndex < items.length) {
101+
const index = nextIndex++
102+
outputs[index] = await operation(items[index], index)
103+
}
104+
}))
105+
return outputs
106+
}

0 commit comments

Comments
 (0)