Skip to content

Fix concurrent WP-CLI wrapper collisions#2004

Merged
chubes4 merged 3 commits into
mainfrom
fix/concurrent-wp-cli-temp-files
Jul 24, 2026
Merged

Fix concurrent WP-CLI wrapper collisions#2004
chubes4 merged 3 commits into
mainfrom
fix/concurrent-wp-cli-temp-files

Conversation

@chubes4

@chubes4 chubes4 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • give every WP-CLI invocation a runtime-scoped wrapper path with 128 bits of cryptographic randomness
  • centralize the three WP-CLI execution paths on one ownership-safe write, execute, and cleanup lifecycle
  • add deterministic collision regression coverage and a 3,200-invocation bounded stress matrix

Root cause

runWpCli() derived /tmp/wp-codebox-wp-cli-<n>.php from this.commands.length. Executions are appended to this.commands only after completion, so all concurrent invocations starting from the same runtime state observed the same value (2 in the reported fuzz run). The bridge and argv helper variants added only Date.now(), which retained the same collision class for calls in one process tick. Concurrent writes could therefore fail with File exists or expose a partially written payload to another worker.

Ownership and atomicity

Every wrapper now uses:

/tmp/wp-codebox-wp-cli-<sanitized-runtime-id>-<128-bit-random-token>.php

The runtime namespace is restricted to fixed safe filename characters and the token comes from crypto.randomBytes(16). The existing Playground writeFile operation completes before execution starts. Ownership begins only after that write resolves, so a rejected write never triggers deletion of a path the invocation did not create.

All public, bridge, theme/plugin helper, and argv-based WP-CLI calls use the same helper. The sibling audit found no non-WP-CLI adapter reusing this wrapper allocation pattern.

Cleanup and errors

The runtime owner unlinks its exact path in finally, covering successful execution, structured nonzero responses, wrapper crashes, timeouts/cancellation, and adapter exceptions without changing existing WP-CLI output cleaning or error classification.

An initial revision also deleted the wrapper from a PHP shutdown handler. Fresh CI exposed that as a runtime-source integration regression because it could run before WP-CLI's own later shutdown handlers. Commit 3237ca76 removed that redundant in-script cleanup and leaves lifecycle ownership exclusively with the host invocation. The base SHA's contracts run was green (30029484653), the focused integration passed twice locally after the correction, and the next fresh CI run passed.

The programmatic Playground adapter now exposes the existing PHP WASM unlink primitive; no parallel temp subsystem was introduced.

Original evidence

The isolated two-factor bounded plan used concurrency 6 with 13 entries and produced 6 successes / 7 failures. Concurrent workers reused /tmp/wp-codebox-wp-cli-2.php, including:

Could not write to "/tmp/wp-codebox-wp-cli-2.php": File exists.
PHP Parse error: Unclosed '{' on line 10 in /tmp/wp-codebox-wp-cli-2.php on line 11

The regression test explicitly reproduces the old deterministic allocator yielding one path for 16 concurrent allocations before exercising the replacement seam.

Stress results

Surface Runtime Concurrency Invocations Result
deterministic memory filesystem test seam 2 384 across 3 repetitions passed
deterministic memory filesystem test seam 6 768 across 3 repetitions passed
deterministic memory filesystem test seam 16 1,024 across 2 repetitions passed
deterministic memory filesystem test seam 64 1,024 passed
real Playground PHP 8.3 / WP 6.8 2, 6, 16 48 passed
real Playground PHP 8.4 / WP 6.8 2, 6, 16 48 passed

Each invocation used a unique payload marker. All outputs matched only their own marker; no File exists, parse errors, missing scripts, cross-payload execution, or orphan wrappers were observed. Both real runtimes also exercised intentional WP-CLI failure and returned /tmp/wp-codebox-wp-cli-*.php to an empty baseline.

The exact two-factor recipe was attempted as a final replay but Playground failed during bootstrap, before any workload command, because https://playground.wordpress.net/wp-cli.phar could not be downloaded. A cached WP 6.8 attempt reached plugin activation but the current two-factor branch requires WP 6.9. This is an external bootstrap limitation, not a wrapper failure; the same 13-entry/concurrency-6 execution shape is covered by the successful real-runtime stress above.

Checks

  • npm run test:runtime-sources-playground-integration passed twice consecutively after the cleanup correction
  • npm run test:wp-cli-temporary-script passed: 3,200 invocations
  • node --test tests/wp-cli-temporary-script.test.ts passed
  • npx tsx tests/runtime-wp-cli-bridge.test.ts passed
  • npm run build passed
  • real Playground PHP 8.3 and 8.4 stress passed: 96 invocations, mixed failure, zero orphans
  • homeboy review --changed-since origin/main passed audit, lint, and changed tests with no introduced findings (run 2d0f10a3-50c4-4ae0-a325-051cba924dad)
  • GitHub Actions run 30059238805 passed contracts and workflow-lint, including test:runtime-sources-playground-integration
  • npm run check reached a pre-existing untouched command-registry-smoke failure: wordpress.collect-workload-result outputShape should mention outputSchema id
  • runtime smoke reached a pre-existing untouched redaction expectation failure in playground-command-errors-smoke

Closes #2003

@chubes4
chubes4 merged commit db1fe89 into main Jul 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Concurrent wordpress.wp-cli commands collide on shared temporary script

1 participant