refactor: pass verifier payload via stdin instead of base64 argv - #34
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Updated per review: the deprecated base64 argv fallback is gone entirely — the verifier now accepts stdin JSON only. Early-alpha, single in-tree caller; no reason to carry the legacy transport. |
fe413d8 to
1fc0865
Compare
The runtime verification payload (candidate code plus checks) was JSON-encoded, base64-encoded, and passed as a command-line argument to wp eval-file. That caps payload size at OS argv limits and exposes the full payload in process listings — both blockers for evaluating realistic artifacts (multi-file plugins, themes) and a needless information leak on shared hosts. - execute_code() now pipes plain JSON (with payload_version: 1.0) through stdin; the command line carries only the verifier path. - _exec()/_run_process() gained stdin plumbing across all three runtime paths (wp-env, CLI, docker exec -i). - verify-runtime.php reads php://stdin first; the base64 argument path remains as a deprecated fallback with explicit errors, so any external callers keep working while migrating. - Payloads are no longer base64-wrapped anywhere. Verified: pytest python (106 passed, 4 new transport tests: stdin delivery, ~2MB payload with argv under 1KB, plain-JSON payloads, docker-exec piping), php -l on the verifier, ruff clean, mypy (2 pre-existing trunk errors only).
The project is early alpha with a single in-tree caller, which now speaks stdin JSON exclusively. Keeping the base64 argument path would preserve exactly the flaws the stdin transport removed (argv size cap, payload visible in process listings) behind a code path nothing uses. The verifier now requires stdin JSON and errors clearly otherwise. Verified: php -l clean, pytest python (106 passed).
a07567e to
2255969
Compare
|
rebased on trunk, 108 unit tests green. verifier reads php://stdin cleanly. transport's solid. |
## Why this matters WP-Bench wants to answer 'which model is best at WordPress work' — but real WordPress work isn't isolated named-function snippets fed to `eval`. It's plugins with headers and file layout, activation hooks, includes, and eventually blocks and themes. A benchmark that only tests snippet-writing measures a narrow (and increasingly unrepresentative) slice of what people actually ask models to build, and models that are strong at structuring real projects get no credit for it. This adds the first realistic artifact mode: multi-file plugin generation, installed and verified in a live WordPress. It also establishes the parsing/validation/install pipeline that block, theme, and patch tracks can reuse. Since candidate artifacts are now written to the runtime filesystem, this is also a security boundary: everything is validated twice (harness and runtime), confined to a dedicated directory, size-capped, and cleaned up. ## Changes **Harness** - New artifact layer: `php_snippet` (default, unchanged behavior) and `wp_plugin_files`, where the model must return a JSON `files` map. Validation rejects absolute paths, `..`/backslash traversal, hostile characters, oversized files (256KB), oversized artifacts (1MB / 20 files), and artifacts missing a top-level `Plugin Name:` header. - Tests declare `artifact_kind` (plus `reference_files` for reference-solution runs); dataset loaders and the Parquet export carry both, and prompts render kind-specific output instructions. - **A malformed completion is a scored failure, not a crash**: unparseable/invalid artifacts record `execution_pass: false` with the artifact error in the grader payload, and the run continues — a model that can't produce a valid plugin has failed the task. **Runtime** - New `Artifact_Installer` writes candidate files into an isolated `wp-content/plugins/wp-bench-candidate-*` directory (independently re-validating every path server-side — defense in depth against a compromised or buggy harness), loads the main plugin file, and deletes the directory after verification. - `Verifier` routes on `artifact_kind`: plugin files are installed before assertions run; static analysis covers all files concatenated; install failures come back as structured `artifact_install_error` results. - Per-test environment resets (from earlier in this series) guarantee candidate plugin state can't leak between tests. ## Verification - `pytest python`: **122 passed** (17 new: parsing incl. fenced JSON, traversal/absolute/backslash rejection, size/count caps, main-file requirement, unknown-kind rejection, end-to-end runner integration, parse-failure continuation across tests, reference-files execution, kind-specific prompts) - `php -l` on all touched runtime files: clean - `ruff check python datasets`: clean - `mypy python`: 2 pre-existing trunk errors only - Live wp-env install/verify round-trip not run here (no local runtime up) — flagged for a runtime smoke test before this ships in an official run. ## Notes - Stacked on #34 (stdin transport, which this depends on for payload size). - Scope is deliberately `wp_plugin_files` only; `block_plugin`, `wp_theme_files`, `js_module`, and `patch` reuse this pipeline in future PRs.
Why this matters
Every execution test ships its payload — the model's generated code plus all verification checks — to the WordPress runtime as a single base64 command-line argument. That transport has two hard problems. First, argv has an OS-level size cap, which silently bounds how large a candidate solution can be; that's tolerable for one-function snippets but a dead end for where the benchmark is headed (multi-file plugins, block plugins, themes). Second, command arguments are visible to anyone who can read a process listing, so the full payload — including checks a task author may not want exposed — leaks on any shared machine. Moving transport to stdin removes both constraints now, so the upcoming artifact-execution work builds on a transport that can already carry it.
Changes
payload_version: 1.0) piped through stdin; the command line carries only thewp eval-file <verifier>invocation. Stdin plumbing runs through all three runtime paths (wp-env, local CLI,docker exec -i).php://stdinexclusively and errors clearly when the payload is missing or not valid JSON. The base64-argument transport is removed entirely — the project is early alpha with a single in-tree caller, so carrying a legacy path would preserve exactly the flaws (argv cap, process-listing leak) this PR exists to eliminate.Verification
pytest python: 106 passed (4 new: payload arrives via stdin and never in argv, a ~2MB payload keeps the command line under 1KB, payload is plain JSON with no base64 layer, docker path pipes throughdocker exec -i)php -l runtime/verify-runtime.php: no syntax errorsruff check python: cleanmypy python: 2 pre-existing trunk errors onlyNotes
payload_versionfield.