Feat/audit remediation - #11
Merged
Merged
Conversation
…l, tests Implements all four phases of the codebase audit in docs/improvements/. Every fix was verified end to end against a reproduction fixture, and the result was exercised against 22 consumer projects before landing. Data safety (critical) - remove() no longer follows symlinks. Composer symlinks `path` repositories into vendor/ by default, and the recursive delete walked through them into the developer's real working copy. Reproduced: the released 3.2.21 deletes the source tree, this build leaves it intact. - The deps/ swap is now atomic. The old tree is moved aside first and restored if the rename fails, with a copy+verify fallback for cross-device moves. The backup lives in the temp dir, not next to deps/, because several projects point `folder` inside vendor/ where a stray .bak ships to wordpress.org. Scoping correctness - The un-prefixing patcher is anchored. It matched substrings with no right-hand boundary, so any scoped symbol merely starting with an excluded name (WP, PO, MO, ftp, wpdb...) was silently de-prefixed into a dangling reference. Matching is now exact for exclude-classes and segment-wise for exclude-namespaces, so children of an excluded namespace still resolve. - The lookup tables are built once instead of per file, behind a fast guard. - The autoload_static.php rewrite is scoped to the $files array. - Symbol extraction walks the whole AST: symbols declared inside function bodies, top-level const, class_alias() targets and else/try/switch branches were all missed. WordPress gains 116 symbols; wp-cli sheds 28 test-only ones. Generated files now carry source and version provenance. Execution model - The nested Composer runs as a subprocess. It was built in-process, and Symfony's autoExit meant run() called exit(): the outer install never resumed, so its security audit, its own post-install-cmd and any later plugin were all skipped. Verified: an outer post-install-cmd that never fired on 3.2.21 now runs. - The nested exit code propagates, so a failed scoping run fails the build instead of reporting success. - Re-entrancy is guarded explicitly rather than by the exit() side effect. - The user's composer-deps.json is read, never rewritten. The derived manifest and the injected script commands are gone, along with the %%placeholder%% templating that broke on Windows paths. - Adds a real `composer wpify-scoper install|update [--no-dev]` command via Capable/CommandProvider. --no-dev was unreachable, so dev dependencies were always scoped and shipped. - Plugin is decomposed into Configuration, Scoper, ScoperConfigFactory, ComposerRunner, ScopedTreeInstaller and SymbolUnprefixer. Configuration - Requires PHP ^8.2: the declared ^8.1 was unsatisfiable against php-scoper and 8.1 is EOL. - A missing or malformed prefix is now an error instead of silence, but only when extra.wpify-scoper is present. The plugin is installed globally, so an unconditional check would break every unrelated project on the machine. - Drops the built-in plugin-update-checker symbol list, which held only dead PUC v4 names. The $checkerClass patcher is deliberately kept: it compensates for php-scoper prefixing PUC's string-literal registry keys, and removing it fatals every plugin using wpify/updates. Its VCS branch is now patched too, fixing GitHub/GitLab-hosted update checks in scoped builds. Foundation - 163 unit and golden-file tests, 17 end-to-end tests, PHPStan level 9 clean. - CI across PHP 8.2-8.5, plus a scheduled symbol refresh guarded against silent count drops. - .gitattributes, CHANGELOG, CONTRIBUTING, README corrections.
Self-review of the previous commit. Two of these were regressions the remediation itself introduced. - Scoper: the workspace was deleted in a `finally`, but a failed deps swap parks the project's previous dependencies in a backup inside it. A failure therefore destroyed the tree the error message told the user to recover from. The workspace is now kept on failure and its path reported. - ScopedTreeInstaller: moveTree() reported a successful cross-filesystem copy as failed when it could not unlink the source, so the caller "restored" the old tree over the new one and left deps/ a merge of both. Copy+verify now counts as success and an uncleaned source only warns. - Plugin: activate() threw on invalid configuration, and Composer does not guard activate(), so a partial extra.wpify-scoper block aborted every command in the project - including the ones needed to fix it. The error is carried and thrown from execute() instead. - ScopedTreeInstaller: publishLock() deleted the project's lock before checking the replacement existed; a scoper.custom.php overriding `finders` could destroy a committed composer-deps.lock. - SymbolUnprefixer, ScopedTreeInstaller: preg_replace() returning null on a PCRE limit was cast to '' and written out, silently emptying a scoped source file or an autoload file. Guarded with `?? $original`, as the patchers in config/scoper.inc.php already were. - Scoper: the nested Composer inherited COMPOSER and COMPOSER_VENDOR_DIR, which point it at the wrong manifest and the wrong vendor dir. Both are cleared for the child and restored afterwards. - ProcessComposerRunner: moving off the in-process Application subjected the pipeline to Composer's 300s process-timeout, killing large installs mid-write. Disabled for these steps and restored. - Configuration: a composerjson without a .json suffix derived a lock path equal to the manifest, so publishing the lock overwrote the manifest. The suffix is appended now, with a hard error if the two still collide. - extract-symbols: a truncated symbol list was written before exiting on a parse error, leaving a corrupt symbols/*.php behind. 166 unit tests, 17 integration tests, PHPStan level 9, all green. Re-verified against the seven consumer projects: all rescope cleanly and all sites serve.
The scoped-namespace check used four backslashes, which grep's BRE reads as two literal ones, so it could never match `namespace Test\Deps\Psr\Log;`. The scoping itself was correct - only the assertion was wrong. Switched to grep -F so the separator needs no escaping at all.
v4 targets Node 20, which GitHub now force-runs on Node 24 and annotates on every job. The remaining annotation comes from actions/cache inside ramsey/composer-install@v3 and needs a release on their side.
The README had grown into the whole manual: a broken numbered list (steps 1-5, then a config table, "Running it manually" and "What to commit", then steps 6-7), a troubleshooting matrix whose rows explained bugs fixed in 4.0, and paragraphs of release-note history sitting in what a new reader takes for current behaviour. It is now a front door - pitch, requirements, one-screen quickstart, link table - and the depth moved into docs/, split by what each page is for. - docs/getting-started.md is a tutorial: one path from an empty directory to a scoped Guzzle, verifiable at every step. Guzzle because it is the collision people actually arrive with. - docs/configuration.md is the reference: every extra.wpify-scoper key, both commands, and the four environment variables. COMPOSER, COMPOSER_VENDOR_DIR and WPIFY_SCOPER_RUNNING were undocumented. - docs/troubleshooting.md keeps the matrix, current behaviour only, with the new composerlock-points-at-the-manifest error and the version-drift case. - docs/deployment.md adds the layouts the consumer audit found in real projects - Bedrock's web/app/deps, scoping into vendor/<name>, one plugin per directory - to the two CI recipes, now pinned to :^4.0. - docs/customizing.md documents that scoper.custom.php must append to `patchers` rather than replace it: dropping the built-in entry removes the un-prefixer and breaks every WordPress call in the scoped tree. - docs/how-it-works.md explains the symbol lists, the pipeline and the swap. - docs/upgrading-to-4.md absorbs the history the reference pages were carrying, and leads with the constraint bump - a global "^3.2" never resolves to 4.0, and nothing tells the user that. Global install stays the documented default, but pinned, with the drift risk stated: the scoper version that produced deps/ is recorded in no lock file, so two machines can emit different bytes from one commit. Adds the community health files the project never had. There was no LICENSE at all, so GitHub reported the repository as unlicensed even though composer.json has declared GPL-2.0-or-later since the first commit in 2021; the file is the verbatim GPL-2.0 text under a copyright notice, no relicensing. SECURITY.md states 4.x-only support and treats a wrongly scoped symbol as in scope. symbol_report.yml is a separate issue form because that report needs different information from an ordinary bug. Deletes docs/improvements/. It was a point-in-time audit superseded by the remediation commits, and its consumer reports named real client projects and carried absolute host paths on a public branch. .gitattributes export-ignores CONTRIBUTING.md, CODE_OF_CONDUCT.md and SECURITY.md alongside docs/ - nobody reads those from inside a consumer's vendor/. README.md, LICENSE and CHANGELOG.md still ship. .github/workflows/docs.yml link-checks every Markdown change, because eight cross-linked pages and a README link table is exactly the surface that rots on the first rename.
A scoping run now ends by reporting a newer published version of the plugin, with a command for a patch or minor and a link to the upgrade guide for a major, since `composer update` cannot cross one on its own. The source is Packagist's package metadata rather than the GitHub tag API, which is a deliberate deviation recorded in docs/adr/0001: a version on Packagist is one that can actually be installed, and GitHub's unauthenticated API is rate limited per IP in a way that breaks on shared office and CI addresses. It behaves like the courtesy it is. The answer is cached for 24 hours in Composer's machine-global cache dir, and only a successful lookup resets that clock, so being offline in the morning does not cost the check in the afternoon. The request has its own 3s timeout because Composer's default is 300, and a size cap. Anything that goes wrong is swallowed, with the reason shown only under -v, so a Packagist outage or a corporate proxy can never slow down or fail a run. Nothing about the user or their project is sent. Silent whenever it would be noise: non-interactive runs (CI, Docker builds, deploy scripts), WPIFY_SCOPER_NO_UPDATE_CHECK, COMPOSER_DISABLE_NETWORK, and any installed version that is not stable, which keeps contributors working from a checkout and anyone deliberately testing an RC from being nagged. The short-circuits are ordered so nothing reaches the network before every reason to stay quiet is ruled out. ReleaseSource exists as an interface for the same reason ComposerRunner does: so the decision can be exercised without touching the network. The notice is emitted from the finally in Scoper::run(), which puts it after the environment is restored, gives it to failed runs as well as successful ones, and keeps the nested Composer process from ever emitting one. The major-version URL is generated from a template, so docs.yml grows a job asserting that a guide exists for every major from 4 up to the newest tag. Its paths filter had to go: a release tag is precisely the push that touches no markdown, which is the one moment the guard needs to run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.