From 6bf00d3ea0cf0b303a0ada0b833fbe9f88ad96f5 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Sat, 10 Oct 2026 18:21:51 -0300 Subject: [PATCH 1/8] feat!: audit the `native` manager, make every manager auditable, select the manager on first use, and speed up CVE lookups and installs. --- CHANGELOG.md | 3 +- README.md | 5 +- UPGRADE.md | 89 ++++ composer.json | 2 +- docs/config.md | 18 +- docs/faqs.md | 11 +- docs/usage.md | 23 +- src/Asset/AbstractAssetManager.php | 116 +++++ src/Asset/AbstractAuditableAssetManager.php | 127 ----- src/Asset/AssetManagerInterface.php | 15 + src/Asset/AssetPackage.php | 4 +- src/Asset/BunManager.php | 9 +- src/Asset/DenoManager.php | 9 +- src/Asset/NativeManager.php | 55 ++- src/Asset/NpmManager.php | 9 +- src/Asset/PnpmManager.php | 9 +- src/Asset/YarnManager.php | 10 +- src/Audit/AuditParserFactory.php | 25 - src/Audit/AuditRunner.php | 4 +- src/Audit/AuditableAssetManagerInterface.php | 10 - src/Audit/CveEnricher.php | 75 ++- src/Audit/CveResolverInterface.php | 18 +- src/Audit/GitHubAdvisoryCveResolver.php | 94 +++- src/Audit/Parser/AbstractAuditParser.php | 3 +- .../Parser/AbstractBulkAdvisoryParser.php | 57 +++ src/Audit/Parser/BunAuditParser.php | 47 +- src/Audit/Parser/NativeAuditParser.php | 16 + src/Command/AuditCommand.php | 5 +- src/Exception/Message.php | 27 +- src/Fallback/ComposerFallback.php | 45 +- src/Foxy.php | 13 +- src/Json/JsonFile.php | 25 +- src/Json/JsonFormatter.php | 17 - src/Native/DependencyResolver.php | 10 +- src/Native/LockFile.php | 3 +- src/Native/NativeInstaller.php | 14 + src/Native/NpmRegistry.php | 129 ++++- src/Native/NpmRegistryInterface.php | 21 +- src/Util/ComposerUtil.php | 9 - src/Util/LockerUtil.php | 22 - src/Util/PackageUtil.php | 46 +- tests/Asset/AbstractAssetManagerTest.php | 5 +- tests/Asset/AuditableAssetManager.php | 43 +- tests/Asset/BunAssetManagerTest.php | 11 +- tests/Asset/DenoAssetManagerTest.php | 6 + tests/Asset/NativeManagerTest.php | 218 ++++++++- tests/Asset/NpmAssetManagerTest.php | 6 + tests/Asset/PnpmAssetManagerTest.php | 6 + tests/Asset/YarnAssetManagerTest.php | 11 +- tests/Audit/AuditParserTest.php | 123 +++-- tests/Audit/AuditRunnerTest.php | 54 ++- tests/Audit/CveEnricherTest.php | 170 ++++++- tests/Audit/GitHubAdvisoryCveResolverTest.php | 324 +++++++++---- tests/Command/AuditCommandTest.php | 14 +- tests/Command/FoxyCapabilityTest.php | 78 +-- tests/Command/FoxyCommandProviderTest.php | 4 +- tests/Fallback/ComposerFallbackTest.php | 155 +++--- .../Asset/InspectableAssetManager.php | 11 +- tests/Fixtures/Asset/StubAssetManager.php | 11 + tests/Fixtures/Native/InMemoryRegistry.php | 51 +- tests/FoxyTest.php | 5 +- tests/Json/JsonFileTest.php | 35 -- tests/Json/JsonFormatterTest.php | 22 - tests/Native/DependencyResolverTest.php | 43 ++ tests/Native/NativeInstallerTest.php | 60 +++ tests/Native/NpmRegistryTest.php | 454 +++++++++++++++++- tests/Provider/AuditParserProvider.php | 59 ++- tests/Provider/MessageProvider.php | 20 +- tests/Util/PackageUtilTest.php | 115 ++--- 69 files changed, 2462 insertions(+), 901 deletions(-) delete mode 100644 src/Asset/AbstractAuditableAssetManager.php delete mode 100644 src/Audit/AuditParserFactory.php delete mode 100644 src/Audit/AuditableAssetManagerInterface.php create mode 100644 src/Audit/Parser/AbstractBulkAdvisoryParser.php create mode 100644 src/Audit/Parser/NativeAuditParser.php delete mode 100644 src/Util/LockerUtil.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e69b40..7b3d68e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## 0.3.1 Under development +## 0.4.0 Under development - refactor: deduplicate internals, centralize exception messages in `Foxy\Exception\Message`, and normalize their wording. - feat: add the `native` asset manager that installs frontend dependencies from the npm registry in PHP, without a JavaScript manager. +- feat!: audit the `native` manager, make every manager auditable, select the manager on first use, and speed up CVE lookups and installs. ## 0.3.0 October 7, 2026 diff --git a/README.md b/README.md index b38090e..4d31fc1 100644 --- a/README.md +++ b/README.md @@ -112,10 +112,11 @@ composer foxy:audit composer foxy:audit --format=summary --no-dev --audit-level=high ``` -Foxy normalizes the current npm, pnpm, Yarn, Bun, and Deno audit reports and identifies the affected package, +Foxy normalizes the current npm, pnpm, Yarn, Bun, Deno, and native audit reports and identifies the affected package, advisory, severity, vulnerable range, and CVE identifiers when GitHub maps the advisory to a CVE. The command returns `0` when no advisory meets the configured threshold, `1` when at least one does, and `2` when the audit cannot be -completed reliably. With Deno, `--no-dev` is rejected because `deno audit` cannot exclude development dependencies. +completed reliably. With Deno and the `native` manager, `--no-dev` is rejected because neither can exclude +development dependencies. See the [usage guide](docs/usage.md#security-auditing) for formats and CI examples. ## Quick start diff --git a/UPGRADE.md b/UPGRADE.md index ecf5dfa..69fbe84 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,94 @@ # Upgrading Foxy +## Upgrading from 0.3.x to 0.4.x + +Foxy 0.4 makes every manager auditable, resolves the frontend manager on first use, moves the asset rollback into the +solver, and removes utilities that Composer `^2.10` made redundant. Projects that only configure Foxy through +`composer.json` need no change beyond the constraint; authors of custom managers, audit parsers, or CVE resolvers must +follow the sections below. + +### Composer constraint + +```bash +composer require php-forge/foxy:^0.4 --with-all-dependencies +``` + +`symfony/console` `^8.0` is accepted beside `^7.4`; PHP 8.3 projects keep resolving `7.4`. + +### Every manager audits + +`Foxy\Audit\AuditableAssetManagerInterface` and `Foxy\Asset\AbstractAuditableAssetManager` were removed. +`AssetManagerInterface` now declares `audit(bool $noDev): AuditProcessResult` and +`getAuditParser(): AuditParserInterface`, and `AbstractAssetManager` carries the process-based implementation that the +former base class held (`getAuditCommand()`, `getAuditEnvironment()`, `validateAuditConfiguration()`). Custom managers +that extended `AbstractAuditableAssetManager` extend `AbstractAssetManager` instead and add `getAuditParser()`; direct +`AssetManagerInterface` implementations must add both methods, throwing a `RuntimeException` from `audit()` when they +cannot audit. `Foxy\Audit\AuditParserFactory` was removed: the runner asks the manager for its parser. +`Foxy\Audit\Parser\BunAuditParser` now extends the new `AbstractBulkAdvisoryParser`, shared with the native manager's +`NativeAuditParser`; the message case `Message::AUDIT_BUN_PACKAGE_ADVISORIES_REQUIRED` is now +`AUDIT_BULK_PACKAGE_ADVISORIES_REQUIRED`, and `AUDIT_PARSER_UNSUPPORTED_MANAGER` and `FOXY_AUDIT_UNSUPPORTED_MANAGER` +were removed. + +The `native` manager now supports `composer foxy:audit` through the registry's bulk advisories endpoint; `--no-dev` +is rejected for it, as for Deno, because the native manager installs development dependencies together with the +others. `Foxy\Asset\NativeManager` takes the `NpmRegistryInterface` in its constructor, and `NpmRegistryInterface` +gained `getAdvisories(array $versions): string` and `prefetchTarballs(array $packages): void`; custom registries must +implement both. + +### CVE lookups resolve in batches + +`Foxy\Audit\CveResolverInterface::resolve(string): CveResolution` was replaced by +`resolveMany(array $ghsaIds): array`, which returns a `CveResolution` or a `Throwable` per identifier. +`GitHubAdvisoryCveResolver` issues its requests concurrently through Composer's HTTP downloader and therefore needs the +async-enabled downloader (`Composer::getLoop()->getHttpDownloader()`), which `composer foxy:audit` already uses. +`CveEnricher` keeps its constructor and `enrich()` signature. + +### Manager selection timing + +Foxy no longer probes manager executables when Composer loads the plugin. The manager is resolved when the first +install, update, or `composer foxy:audit` needs it, so Composer commands that do none of these (`about`, `show`, +`validate`, ...) never run ` --version`. Selection errors (an unknown `manager`, several native lockfiles, a +lockfile whose executable is missing, or no manager at all) are reported by the first command that needs the manager +instead of by every Composer command; `composer foxy:audit` reports them with exit status `2`. + +### Manager-scoped configuration + +`Foxy\Config\Config::setResolvedManager()` was removed. Probing or validating a manager no longer changes the shared +`Config`; `Config::forManager(string $manager): static` returns a copy whose `manager-*` map entries resolve for that +manager, and `AbstractManifestAssetManager::getManagerConfig()` (protected) gives each built-in manager its own view. +Custom managers that called `setResolvedManager()` remove the call and read `manager-*` keys through +`getManagerConfig()`. + +### Default commands in `AbstractAssetManager` + +`getInstallCommand()`, `getUpdateCommand()`, and `getVersionCommand()` are no longer abstract. They default to +` install`, ` update`, and ` --version`, where the binary comes from the new protected +`getDefaultBinary(): string` hook, which returns `getName()`. Custom managers whose executable differs from their name +override `getDefaultBinary()`; managers that implement the three methods keep working unchanged. + +### Asset rollback belongs to the solver + +`AssetManagerInterface::setFallback()` was removed, together with the fallback constructor argument, the protected +`$fallback` property, and `restoreAfterFailure()` of `AbstractAssetManager`. `Foxy\Solver\Solver` takes the asset +fallback as its last constructor argument and restores the manifest when the dependency merge or the manager run +fails, in the same cases as before; managers no longer restore anything. + +### Composer rollback events + +The rollback `Installer` is now built with a private event dispatcher that has no listeners and runs no scripts, so +command and autoload events of the rollback never reach the project's scripts or other plugins. Package events +(`pre-package-install`, `post-package-update`, ...) are still dispatched by Composer's shared installation manager +during a rollback, so root scripts and plugin listeners bound to those events run; projects that must not run them +during a rollback should guard those scripts. + +### Removed utilities + +`Foxy\Util\LockerUtil` was removed; the Composer fallback reads the lock through `Composer\Factory::getLockFile()`. +`Foxy\Util\ComposerUtil::getInitEventName()` was removed; use `Composer\Installer\InstallerEvents::PRE_OPERATIONS_EXEC`. +`Foxy\Util\PackageUtil::convertLockAlias()` and the `$convertAliases` parameter of `loadLockPackages()` were removed. +`Foxy\Json\JsonFile::getIndent()`, `Foxy\Json\JsonFormatter::getIndent()`, and the constant +`JsonFormatter::INDENT_REGEX` were removed. + ## Upgrading from 0.2.x to 0.3.x Foxy 0.3 raises the runtime baseline and aligns its documented integration target with the PHP 8.3 development line. diff --git a/composer.json b/composer.json index c8db0a9..377b3bc 100644 --- a/composer.json +++ b/composer.json @@ -29,7 +29,7 @@ "composer/pcre": "^3.3", "composer/semver": "^3.4", "seld/jsonlint": "^1.12", - "symfony/console": "^7.4" + "symfony/console": "^7.4 || ^8.0" }, "require-dev": { "infection/infection": "^0.35", diff --git a/docs/config.md b/docs/config.md index c617f55..ab93fd9 100644 --- a/docs/config.md +++ b/docs/config.md @@ -316,7 +316,11 @@ path is built from it. Scoped packages keep the npm layout (`vendor/npm-asset/@p The native manager requires the PHP `zlib` extension and prints `Installing`, `Updating`, and `Removing` lines like the other managers. Its registry requests go through Composer's HTTP layer: proxies, `cafile`, `disable-tls`, and the credentials stored in `auth.json` for the registry host all apply, so a private registry can be configured with -`registry-url` plus Composer authentication: +`registry-url` plus Composer authentication. Credentials are matched against the request's `host:port` exactly as +written in `registry-url`: `localhost` and `127.0.0.1` are not aliases of each other, and the `auth.json` key must +include the port when it is not the scheme's default (`npm.example.com:4873`). A plain `http://` registry also needs +Composer's `config.secure-http` set to `false`. A registry that answers 401 or 403 fails the request with Composer's +error text, as any other registry failure does: ```json { @@ -330,8 +334,15 @@ credentials stored in `auth.json` for the registry host all apply, so a private ``` The registry is expected to answer the npm metadata API (`GET /` with -`Accept: application/vnd.npm.install-v1+json`) and to serve the tarball URLs it publishes. HTTP 429 responses are -retried up to three times, honoring `Retry-After` up to 60 seconds; other failures are reported immediately. +`Accept: application/vnd.npm.install-v1+json`) and to serve the tarball URLs it publishes. `composer foxy:audit` also +needs npm's bulk advisories endpoint (`POST /-/npm/v1/security/advisories/bulk` with a JSON body); the registry must +implement it, and a registry that does not fails the audit with status `2`. Verdaccio implements it only with its +`audit` middleware enabled (`middlewares: audit: enabled: true`); its default configuration answers 404. HTTP 429 +responses are retried up to three times, honoring `Retry-After` up to 60 seconds; other failures are reported +immediately. Tarballs missing from the cache are first downloaded in parallel through Composer's HTTP downloader +(`COMPOSER_MAX_PARALLEL_HTTP` sets the limit, 12 by default) and then verified and installed one by one; a tarball +whose parallel download fails is requested again on its own, with the retries above, and its error is reported then. +With `cache-read-only` or an unwritable cache, tarballs are downloaded one by one only. The native manager implements the subset that frontend assets need. It does not support: @@ -347,7 +358,6 @@ The native manager implements the subset that frontend assets need. It does not named `alpha`, `beta`, `RC`, `dev`, and `patch` are matched with npm's rule: only a range that names a prerelease of the same `major.minor.patch` can select it. - Symbolic and hard links inside tarballs (skipped) and `..` or absolute entry paths (rejected). -- `composer foxy:audit`. Selection by availability is also disabled for this manager: without `manager: native` or a `foxy.lock` file, a project without a JavaScript manager still reports `No asset manager was found.`. diff --git a/docs/faqs.md b/docs/faqs.md index bfe8200..77bcd6f 100644 --- a/docs/faqs.md +++ b/docs/faqs.md @@ -50,8 +50,8 @@ Explicit selection and a committed lockfile are recommended for CI. Yes. Set `config.foxy.manager` to `native`. Foxy reads the merged `package.json`, resolves every dependency against the npm registry with one version per package, downloads and verifies the tarballs, extracts them into `node_modules`, and records the result in `foxy.lock`. PHP with the `zlib` extension is the only requirement. The native manager does -not run lifecycle scripts, does not create `bin` links, and does not support `composer foxy:audit`. See -[Native manager](config.md#native-manager) for the complete scope. +not run lifecycle scripts and does not create `bin` links. See [Native manager](config.md#native-manager) for the +complete scope. ## How can frontend dependencies be checked for known CVEs? @@ -61,9 +61,10 @@ identifiers. Use `--format=summary` for compact counts in CI or `--format=json` document. The native registry response commonly contains a GHSA but no CVE. Foxy resolves that relationship through GitHub's -public advisory API unless `--no-cve` or `--format=summary` is supplied. A lookup failure is shown as unavailable -without hiding the native security finding. See [Security auditing](usage.md#security-auditing) for options and exit -statuses. +public advisory API unless `--no-cve` or `--format=summary` is supplied. The lookups run concurrently through +Composer's HTTP downloader (at most 12 at once; `COMPOSER_MAX_PARALLEL_HTTP` changes the limit), and a `github-oauth` +token in Composer's `auth.json` applies to them. A lookup failure marks only its own advisory as unavailable without +hiding the native security finding. See [Security auditing](usage.md#security-auditing) for options and exit statuses. ## Why is a dependency's package.json not detected? diff --git a/docs/usage.md b/docs/usage.md index 7fbd13c..5fa281c 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -45,10 +45,16 @@ composer foxy:audit The command requires the selected manager's native lockfile, validates the manager version, and asks the manager for a security report. It reads the lockfile and does not run an install, update, fix, or fallback. Foxy supports npm audit -report version 2 starting with npm 10.9.8 and the current report schemas emitted by pnpm 11, Yarn 4, and Bun 1.4; -legacy report formats are rejected instead of being interpreted heuristically. +report version 2 starting with npm 10.9.8, the current report schemas emitted by pnpm 11, Yarn 4, and Bun 1.4, and the +npm registry's bulk advisories document read by the `native` manager; legacy report formats are rejected instead of +being interpreted heuristically. -The `native` manager does not support the audit command yet; `composer foxy:audit` returns status `2` for it. +With the `native` manager, `composer foxy:audit` reads `foxy.lock` and asks the configured `registry-url` for +advisories in one request to npm's bulk advisories endpoint (`POST /-/npm/v1/security/advisories/bulk`), +sending every locked registry package with its version; local `file:` packages are not sent, and a lock without +registry packages is clean without a request. `--no-dev` is rejected with status `2` because the native manager +installs development dependencies together with the others. A registry that does not implement the endpoint fails +the audit with status `2`. Deno does not provide a machine-readable report. Foxy runs `deno audit --level=low` with `NO_COLOR=1` against `deno.lock` and parses its text report strictly. The parser is validated against Deno 2.9.7 and returns status `2` @@ -99,9 +105,11 @@ standard error so JSON on standard output remains valid. Native npm registry audit data normally identifies advisories by a numeric ID and GHSA rather than a CVE. Unless `--no-cve` or `--format=summary` is used, Foxy resolves each unique public GHSA once through the -[GitHub global security advisories API](https://docs.github.com/en/rest/security-advisories/global-advisories). A -failed optional lookup is reported as `unavailable` and does not discard the native finding or change its audit exit -status. +[GitHub global security advisories API](https://docs.github.com/en/rest/security-advisories/global-advisories). The +lookups run concurrently through Composer's HTTP downloader, at most 12 at once by default; the +`COMPOSER_MAX_PARALLEL_HTTP` environment variable changes that limit. A `github-oauth` token for `github.com` in +Composer's `auth.json` applies to these requests and raises GitHub's rate limit. A failed optional lookup marks only +its own advisory as `unavailable` and does not discard the native finding or change its audit exit status. Bun can skip packages from a registry that does not answer its audit request while still returning a successful native status. Foxy therefore treats any diagnostic emitted by `bun audit --json` as an unreliable partial report and returns @@ -125,7 +133,8 @@ composer foxy:audit --no-dev --audit-level=high --format=summary The explicit command still runs when `run-asset-manager` is `false`; that setting controls automatic install and update execution, not a user-requested audit. Foxy must be enabled, the selected binary must satisfy its supported -version constraint, and the matching native lockfile must exist. +version constraint, and the matching native lockfile must exist. The `native` manager has no binary, so only Foxy and +`foxy.lock` are required for it. ## Library setup diff --git a/src/Asset/AbstractAssetManager.php b/src/Asset/AbstractAssetManager.php index cffc6ac..d9841ce 100644 --- a/src/Asset/AbstractAssetManager.php +++ b/src/Asset/AbstractAssetManager.php @@ -7,6 +7,7 @@ use Composer\IO\IOInterface; use Composer\Semver\{Semver, VersionParser}; use Composer\Util\{Filesystem, Platform, ProcessExecutor}; +use Foxy\Audit\AuditProcessResult; use Foxy\Config\Config; use Foxy\Converter\{SemverConverter, VersionConverterInterface}; use Foxy\Exception\{Message, RuntimeException}; @@ -14,7 +15,10 @@ use Throwable; use UnexpectedValueException; +use function array_key_exists; +use function getenv; use function is_string; +use function putenv; use function sprintf; use function trim; @@ -35,6 +39,11 @@ public function __construct( $this->versionConverter ??= new SemverConverter(); } + /** + * Get the command to audit the asset dependencies. + */ + abstract protected function getAuditCommand(bool $noDev): string; + /** * Get the command to install the asset dependencies. */ @@ -50,6 +59,37 @@ abstract protected function getUpdateCommand(): string; */ abstract protected function getVersionCommand(): string; + public function audit(bool $noDev): AuditProcessResult + { + if (!$this->hasLockFile()) { + throw new RuntimeException( + Message::ASSET_MANAGER_LOCK_FILE_MISSING->getMessage($this->getName(), $this->getLockFilePath()), + ); + } + + $this->validateAuditConfiguration($noDev); + $this->validate(); + + return $this->withManagerTimeout( + function () use ($noDev): AuditProcessResult { + $environment = $this->overrideEnvironment($this->getAuditEnvironment()); + + try { + $output = ''; + $result = $this->executor->execute( + $this->getAuditCommand($noDev), + $output, + $this->getManagerWorkingDirectory(), + ); + + return new AuditProcessResult($result, (string) $output, $this->executor->getErrorOutput()); + } finally { + $this->restoreEnvironment($environment); + } + }, + ); + } + public function isAvailable(): bool { $this->config->setResolvedManager($this->getName()); @@ -173,6 +213,16 @@ protected function buildUnconfiguredCommand(string $defaultBin, array|string $co return sprintf('%s %s', $this->getManagerBinary($defaultBin), implode(' ', (array) $command)); } + /** + * Get environment overrides required for a complete audit. + * + * @return array + */ + protected function getAuditEnvironment(): array + { + return []; + } + protected function getVersion(): string|null { if ($this->version === '' && $this->versionConverter !== null) { @@ -200,6 +250,11 @@ protected function normalizeVersionOutput(string $output): string return $output; } + /** + * Validate manager configuration that can change the audit scope. + */ + protected function validateAuditConfiguration(bool $noDev): void {} + /** * Runs the callback with the process timeout set to the `manager-timeout` option and restores the previous timeout * afterward, even when the callback throws. @@ -251,4 +306,65 @@ private function getManagerBinary(string $defaultBin): string return Platform::isWindows() ? str_replace('/', '\\', $bin) : $bin; } + + /** + * @param array $environment + * + * @return array + */ + private function overrideEnvironment(array $environment): array + { + $state = []; + + foreach ($environment as $name => $value) { + $state[$name] = [ + 'process' => getenv($name), + 'envExists' => array_key_exists($name, $_ENV), + 'env' => $_ENV[$name] ?? null, + 'serverExists' => array_key_exists($name, $_SERVER), + 'server' => $_SERVER[$name] ?? null, + ]; + + putenv("{$name}={$value}"); + + $_ENV[$name] = $value; + $_SERVER[$name] = $value; + } + + return $state; + } + + /** + * @param array $state + */ + private function restoreEnvironment(array $state): void + { + foreach ($state as $name => $values) { + putenv(false === $values['process'] ? $name : "{$name}=" . $values['process']); + + if ($values['envExists']) { + $_ENV[$name] = $values['env']; + } else { + unset($_ENV[$name]); + } + + if ($values['serverExists']) { + $_SERVER[$name] = $values['server']; + } else { + unset($_SERVER[$name]); + } + } + } } diff --git a/src/Asset/AbstractAuditableAssetManager.php b/src/Asset/AbstractAuditableAssetManager.php deleted file mode 100644 index fd19c34..0000000 --- a/src/Asset/AbstractAuditableAssetManager.php +++ /dev/null @@ -1,127 +0,0 @@ -hasLockFile()) { - throw new RuntimeException( - Message::ASSET_MANAGER_LOCK_FILE_MISSING->getMessage($this->getName(), $this->getLockFilePath()), - ); - } - - $this->validateAuditConfiguration($noDev); - $this->validate(); - - return $this->withManagerTimeout( - function () use ($noDev): AuditProcessResult { - $environment = $this->overrideEnvironment($this->getAuditEnvironment()); - - try { - $output = ''; - $result = $this->executor->execute( - $this->getAuditCommand($noDev), - $output, - $this->getManagerWorkingDirectory(), - ); - - return new AuditProcessResult($result, (string) $output, $this->executor->getErrorOutput()); - } finally { - $this->restoreEnvironment($environment); - } - }, - ); - } - - /** - * Get environment overrides required for a complete audit. - * - * @return array - */ - protected function getAuditEnvironment(): array - { - return []; - } - - /** - * Validate manager configuration that can change the audit scope. - */ - protected function validateAuditConfiguration(bool $noDev): void {} - - /** - * @param array $environment - * - * @return array - */ - private function overrideEnvironment(array $environment): array - { - $state = []; - - foreach ($environment as $name => $value) { - $state[$name] = [ - 'process' => getenv($name), - 'envExists' => array_key_exists($name, $_ENV), - 'env' => $_ENV[$name] ?? null, - 'serverExists' => array_key_exists($name, $_SERVER), - 'server' => $_SERVER[$name] ?? null, - ]; - - putenv("{$name}={$value}"); - - $_ENV[$name] = $value; - $_SERVER[$name] = $value; - } - - return $state; - } - - /** - * @param array $state - */ - private function restoreEnvironment(array $state): void - { - foreach ($state as $name => $values) { - putenv(false === $values['process'] ? $name : "{$name}=" . $values['process']); - - if ($values['envExists']) { - $_ENV[$name] = $values['env']; - } else { - unset($_ENV[$name]); - } - - if ($values['serverExists']) { - $_SERVER[$name] = $values['server']; - } else { - unset($_SERVER[$name]); - } - } - } -} diff --git a/src/Asset/AssetManagerInterface.php b/src/Asset/AssetManagerInterface.php index 1487d04..18cb883 100644 --- a/src/Asset/AssetManagerInterface.php +++ b/src/Asset/AssetManagerInterface.php @@ -5,6 +5,7 @@ namespace Foxy\Asset; use Composer\Package\RootPackageInterface; +use Foxy\Audit\{AuditParserInterface, AuditProcessResult}; use Foxy\Exception\RuntimeException; use Foxy\Fallback\FallbackInterface; @@ -18,6 +19,20 @@ interface AssetManagerInterface */ public function addDependencies(RootPackageInterface $rootPackage, array $dependencies): AssetPackageInterface; + /** + * Runs the security audit of the locked dependencies. + * + * @param bool $noDev Whether to exclude development dependencies; managers that cannot exclude them throw. + * + * @throws RuntimeException if the lock file is missing, the manager cannot honor `$noDev`, or the audit cannot run. + */ + public function audit(bool $noDev): AuditProcessResult; + + /** + * Returns the parser of this manager's audit report. + */ + public function getAuditParser(): AuditParserInterface; + /** * Get the filename of the lock file. */ diff --git a/src/Asset/AssetPackage.php b/src/Asset/AssetPackage.php index eae1db7..5e1e9c5 100644 --- a/src/Asset/AssetPackage.php +++ b/src/Asset/AssetPackage.php @@ -148,9 +148,7 @@ private function injectRequiredKeys(RootPackageInterface $rootPackage): void $license = current($rootPackage->getLicense()); if ('proprietary' === $license) { - if (!isset($this->package['private'])) { - $this->package['private'] = true; - } + $this->package['private'] ??= true; } else { $this->package['license'] = $license; } diff --git a/src/Asset/BunManager.php b/src/Asset/BunManager.php index 8689a74..69707c4 100644 --- a/src/Asset/BunManager.php +++ b/src/Asset/BunManager.php @@ -6,6 +6,8 @@ use Composer\Pcre\Preg; use Composer\Util\Platform; +use Foxy\Audit\AuditParserInterface; +use Foxy\Audit\Parser\BunAuditParser; use Foxy\Exception\{Message, RuntimeException}; use Generator; use Stringable; @@ -36,8 +38,13 @@ use const DIRECTORY_SEPARATOR; -final class BunManager extends AbstractAuditableAssetManager +final class BunManager extends AbstractAssetManager { + public function getAuditParser(): AuditParserInterface + { + return new BunAuditParser(); + } + public function getLockPackageName(): string { return 'bun.lock'; diff --git a/src/Asset/DenoManager.php b/src/Asset/DenoManager.php index 8c73ef3..09dae68 100644 --- a/src/Asset/DenoManager.php +++ b/src/Asset/DenoManager.php @@ -5,6 +5,8 @@ namespace Foxy\Asset; use Composer\Util\Platform; +use Foxy\Audit\AuditParserInterface; +use Foxy\Audit\Parser\DenoAuditParser; use Foxy\Exception\{Message, RuntimeException}; use function array_diff; @@ -15,10 +17,15 @@ use function str_starts_with; use function substr; -final class DenoManager extends AbstractAuditableAssetManager +final class DenoManager extends AbstractAssetManager { private const array AUDIT_ENVIRONMENT = ['NO_COLOR' => '1']; + public function getAuditParser(): AuditParserInterface + { + return new DenoAuditParser(); + } + public function getLockPackageName(): string { return 'deno.lock'; diff --git a/src/Asset/NativeManager.php b/src/Asset/NativeManager.php index 11c6e34..132e516 100644 --- a/src/Asset/NativeManager.php +++ b/src/Asset/NativeManager.php @@ -6,10 +6,12 @@ use Composer\IO\IOInterface; use Composer\Util\{Filesystem, Platform}; +use Foxy\Audit\{AuditParserInterface, AuditProcessResult}; +use Foxy\Audit\Parser\NativeAuditParser; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Fallback\FallbackInterface; -use Foxy\Native\NativeInstallerInterface; +use Foxy\Native\{LockFile, NativeInstallerInterface, NpmRegistryInterface}; use Throwable; use function array_unshift; @@ -18,6 +20,7 @@ use function implode; use function is_dir; use function is_string; +use function ksort; use function preg_match; use function realpath; use function sprintf; @@ -25,6 +28,8 @@ use function strtolower; use function trim; +use const SORT_STRING; + /** * Provides the `native` asset manager: installs `package.json` dependencies from the npm registry in PHP, recording * them in `foxy.lock`, without a Node.js package manager. @@ -41,6 +46,7 @@ final class NativeManager extends AbstractManifestAssetManager * @param Config $config Foxy configuration. * @param Filesystem $fs Filesystem used by the manifest handling. * @param NativeInstallerInterface $installer Installer that populates the install directory. + * @param NpmRegistryInterface $registry Registry that answers the security audit. * @param FallbackInterface|null $fallback Fallback restored when the installation fails, or `null` for none. */ public function __construct( @@ -48,11 +54,58 @@ public function __construct( Config $config, Filesystem $fs, private readonly NativeInstallerInterface $installer, + private readonly NpmRegistryInterface $registry, FallbackInterface|null $fallback = null, ) { parent::__construct($io, $config, $fs, $fallback); } + /** + * Runs the security audit of the locked registry packages with one request to the registry's bulk advisories + * endpoint. + * + * Local `file:` packages are skipped, and a lock without registry packages is clean without a request. The exit + * code is `1` when the document reports an advisory and `0` otherwise; the error output is always empty. + * + * @param bool $noDev Whether to exclude development dependencies, which the native manager cannot do. + * + * @throws RuntimeException if `foxy.lock` is missing or malformed, `$noDev` is `true`, the registry request fails, + * or the advisories document is malformed. + */ + public function audit(bool $noDev): AuditProcessResult + { + if (!$this->hasLockFile()) { + throw new RuntimeException( + Message::ASSET_MANAGER_LOCK_FILE_MISSING->getMessage($this->getName(), $this->getLockFilePath()), + ); + } + + if ($noDev) { + throw new RuntimeException( + Message::ASSET_NATIVE_AUDIT_NO_DEV_UNSUPPORTED->getMessage(), + ); + } + + $versions = []; + + foreach ((new LockFile($this->getLockFilePath()))->read()['packages'] as $package) { + if (!$package->isLocal()) { + $versions[$package->name] = [$package->version]; + } + } + + ksort($versions, SORT_STRING); + + $document = [] === $versions ? '{}' : $this->registry->getAdvisories($versions); + + return new AuditProcessResult([] === $this->getAuditParser()->parse($document) ? 0 : 1, $document, ''); + } + + public function getAuditParser(): AuditParserInterface + { + return new NativeAuditParser(); + } + public function getLockPackageName(): string { return 'foxy.lock'; diff --git a/src/Asset/NpmManager.php b/src/Asset/NpmManager.php index 079248f..feefda3 100644 --- a/src/Asset/NpmManager.php +++ b/src/Asset/NpmManager.php @@ -6,6 +6,8 @@ use Composer\Json\JsonFile; use Composer\Util\ProcessExecutor; +use Foxy\Audit\AuditParserInterface; +use Foxy\Audit\Parser\NpmAuditParser; use Foxy\Exception\{Message, RuntimeException}; use function array_is_list; @@ -16,8 +18,13 @@ use function str_replace; use function trim; -final class NpmManager extends AbstractAuditableAssetManager +final class NpmManager extends AbstractAssetManager { + public function getAuditParser(): AuditParserInterface + { + return new NpmAuditParser(); + } + public function getLockPackageName(): string { return 'package-lock.json'; diff --git a/src/Asset/PnpmManager.php b/src/Asset/PnpmManager.php index d2f5d77..99263bf 100644 --- a/src/Asset/PnpmManager.php +++ b/src/Asset/PnpmManager.php @@ -5,9 +5,16 @@ namespace Foxy\Asset; use Composer\Util\ProcessExecutor; +use Foxy\Audit\AuditParserInterface; +use Foxy\Audit\Parser\PnpmAuditParser; -final class PnpmManager extends AbstractAuditableAssetManager +final class PnpmManager extends AbstractAssetManager { + public function getAuditParser(): AuditParserInterface + { + return new PnpmAuditParser(); + } + public function getLockPackageName(): string { return 'pnpm-lock.yaml'; diff --git a/src/Asset/YarnManager.php b/src/Asset/YarnManager.php index d5c15bf..e552d76 100644 --- a/src/Asset/YarnManager.php +++ b/src/Asset/YarnManager.php @@ -4,13 +4,21 @@ namespace Foxy\Asset; -final class YarnManager extends AbstractAuditableAssetManager +use Foxy\Audit\AuditParserInterface; +use Foxy\Audit\Parser\YarnAuditParser; + +final class YarnManager extends AbstractAssetManager { private const array AUDIT_ENVIRONMENT = [ 'YARN_NPM_AUDIT_EXCLUDE_PACKAGES' => '__FOXY_AUDIT_NO_MATCH__', 'YARN_NPM_AUDIT_IGNORE_ADVISORIES' => '__FOXY_AUDIT_NO_MATCH__', ]; + public function getAuditParser(): AuditParserInterface + { + return new YarnAuditParser(); + } + public function getLockPackageName(): string { return 'yarn.lock'; diff --git a/src/Audit/AuditParserFactory.php b/src/Audit/AuditParserFactory.php deleted file mode 100644 index 477ab53..0000000 --- a/src/Audit/AuditParserFactory.php +++ /dev/null @@ -1,25 +0,0 @@ - new NpmAuditParser(), - 'pnpm' => new PnpmAuditParser(), - 'yarn' => new YarnAuditParser(), - 'bun' => new BunAuditParser(), - 'deno' => new DenoAuditParser(), - default => throw new RuntimeException( - Message::AUDIT_PARSER_UNSUPPORTED_MANAGER->getMessage($manager), - ), - }; - } -} diff --git a/src/Audit/AuditRunner.php b/src/Audit/AuditRunner.php index 20ddd33..a0d4324 100644 --- a/src/Audit/AuditRunner.php +++ b/src/Audit/AuditRunner.php @@ -14,7 +14,7 @@ final readonly class AuditRunner implements AuditRunnerInterface { - public function __construct(private AssetManagerInterface&AuditableAssetManagerInterface $manager) {} + public function __construct(private AssetManagerInterface $manager) {} public function audit(AuditRequest $request): AuditReport { @@ -34,7 +34,7 @@ public function audit(AuditRequest $request): AuditReport } try { - $findings = AuditParserFactory::create($manager)->parse($result->output); + $findings = $this->manager->getAuditParser()->parse($result->output); } catch (Throwable $exception) { $message = $exception->getMessage(); diff --git a/src/Audit/AuditableAssetManagerInterface.php b/src/Audit/AuditableAssetManagerInterface.php deleted file mode 100644 index 62f70d7..0000000 --- a/src/Audit/AuditableAssetManagerInterface.php +++ /dev/null @@ -1,10 +0,0 @@ - $cache */ - $cache = []; - $findings = []; + $findings = $report->findings; + $pending = []; - foreach ($report->findings as $finding) { + foreach ($findings as $index => $finding) { if ([] !== $finding->cves) { - $findings[] = $finding->withCveResolution($finding->cves, CveStatus::RESOLVED); + $findings[$index] = $finding->withCveResolution($finding->cves, CveStatus::RESOLVED); continue; } @@ -31,24 +41,55 @@ public function enrich(AuditReport $report, callable $warningHandler): AuditRepo $ghsaId = AuditNormalizer::normalizeGhsaId($finding->advisoryId); if (null === $ghsaId) { - $findings[] = $finding->withCveResolution([], CveStatus::UNAVAILABLE); + $findings[$index] = $finding->withCveResolution([], CveStatus::UNAVAILABLE); continue; } - if (!isset($cache[$ghsaId])) { - try { - $cache[$ghsaId] = $this->resolver->resolve($ghsaId); - } catch (Throwable $exception) { - $warningHandler(sprintf('Unable to resolve CVE identifiers for %s: %s', $ghsaId, $exception->getMessage())); - $cache[$ghsaId] = new CveResolution([], CveStatus::UNAVAILABLE); - } - } + $pending[$index] = $ghsaId; + } - $resolution = $cache[$ghsaId]; - $findings[] = $finding->withCveResolution($resolution->cves, $resolution->status); + $resolutions = [] === $pending ? [] : $this->resolveAll(array_values(array_unique($pending)), $warningHandler); + + foreach ($pending as $index => $ghsaId) { + $resolution = $resolutions[$ghsaId]; + $findings[$index] = $findings[$index]->withCveResolution($resolution->cves, $resolution->status); } return $report->withFindings($findings); } + + /** + * Returns the resolution of every GHSA identifier, marking each failed lookup as `unavailable` with one warning. + * + * @param list $ghsaIds Unique canonical GHSA identifiers, in first-seen order. + * @param callable(string): void $warningHandler Receives the warnings in the order of `$ghsaIds`. + * + * @return array Resolutions keyed by GHSA identifier. + */ + private function resolveAll(array $ghsaIds, callable $warningHandler): array + { + try { + $results = $this->resolver->resolveMany($ghsaIds); + } catch (Throwable $exception) { + $results = array_fill_keys($ghsaIds, $exception); + } + + $resolutions = []; + + foreach ($ghsaIds as $ghsaId) { + $result = $results[$ghsaId]; + + if ($result instanceof Throwable) { + $warningHandler( + sprintf('Unable to resolve CVE identifiers for %s: %s', $ghsaId, $result->getMessage()), + ); + $result = new CveResolution([], CveStatus::UNAVAILABLE); + } + + $resolutions[$ghsaId] = $result; + } + + return $resolutions; + } } diff --git a/src/Audit/CveResolverInterface.php b/src/Audit/CveResolverInterface.php index 1bb4ec5..c97f40a 100644 --- a/src/Audit/CveResolverInterface.php +++ b/src/Audit/CveResolverInterface.php @@ -4,7 +4,23 @@ namespace Foxy\Audit; +use Foxy\Exception\RuntimeException; +use Throwable; + +/** + * Resolves the CVE identifiers of GitHub security advisories. + */ interface CveResolverInterface { - public function resolve(string $ghsaId): CveResolution; + /** + * Resolves the CVE identifiers of several advisories at once. + * + * @param list $ghsaIds GHSA identifiers to resolve. + * + * @return array One result for every identifier, keyed by canonical GHSA + * identifier; a `Throwable` marks a lookup that failed. + * + * @throws RuntimeException if an identifier is not a GHSA identifier; nothing is requested in that case. + */ + public function resolveMany(array $ghsaIds): array; } diff --git a/src/Audit/GitHubAdvisoryCveResolver.php b/src/Audit/GitHubAdvisoryCveResolver.php index aecbf8c..c43e202 100644 --- a/src/Audit/GitHubAdvisoryCveResolver.php +++ b/src/Audit/GitHubAdvisoryCveResolver.php @@ -4,36 +4,99 @@ namespace Foxy\Audit; +use Composer\Util\Http\Response; use Composer\Util\HttpDownloader; use Foxy\Exception\{Message, RuntimeException}; +use Seld\JsonLint\ParsingException; +use Throwable; +use function array_map; use function is_array; use function is_string; use function sprintf; use function strtoupper; use function trim; +/** + * Resolves the CVE identifiers of GitHub security advisories through the GitHub global security advisories API. + * + * The lookups run concurrently, so the downloader must be the async-enabled instance returned by + * `Composer::getLoop()->getHttpDownloader()`; any other instance throws a `LogicException` on the first request. + * + * @see https://docs.github.com/en/rest/security-advisories/global-advisories + */ final readonly class GitHubAdvisoryCveResolver implements CveResolverInterface { private const string API_URL = 'https://api.github.com/advisories/%s'; public function __construct(private HttpDownloader $httpDownloader) {} - public function resolve(string $ghsaId): CveResolution + /** + * Resolves the CVE identifiers of several advisories with one concurrent request per identifier. + * + * @param list $ghsaIds GHSA identifiers to resolve, trimmed and compared case-insensitively. + * + * @return array Resolutions keyed by canonical GHSA identifier; a `Throwable` is + * the transport, JSON, or document error of that lookup. + * + * @throws RuntimeException if an identifier is not a GHSA identifier; nothing is requested in that case. + */ + public function resolveMany(array $ghsaIds): array { - $ghsaId = $this->normalizeGhsaId($ghsaId); - - $response = $this->httpDownloader->get( - sprintf(self::API_URL, $ghsaId), - [ - 'http' => [ - 'header' => [ - 'Accept: application/vnd.github+json', - 'X-GitHub-Api-Version: 2022-11-28', + $ghsaIds = array_map($this->normalizeGhsaId(...), $ghsaIds); + $results = []; + + foreach ($ghsaIds as $ghsaId) { + $this->httpDownloader->add( + sprintf(self::API_URL, $ghsaId), + [ + 'http' => [ + 'header' => [ + 'Accept: application/vnd.github+json', + 'X-GitHub-Api-Version: 2022-11-28', + ], ], ], - ], + )->then( + function (Response $response) use ($ghsaId, &$results): void { + try { + $results[$ghsaId] = $this->parse($ghsaId, $response); + } catch (Throwable $exception) { + $results[$ghsaId] = $exception; + } + }, + static function (Throwable $exception) use ($ghsaId, &$results): void { + $results[$ghsaId] = $exception; + }, + ); + } + + $this->httpDownloader->wait(); + + return $results; + } + + private function normalizeGhsaId(string $ghsaId): string + { + $ghsaId = trim($ghsaId); + + return AuditNormalizer::normalizeGhsaId($ghsaId) ?? throw new RuntimeException( + Message::AUDIT_GHSA_ID_INVALID->getMessage($ghsaId), ); + } + + /** + * Returns the CVE identifiers of an advisory document, unique and sorted. + * + * @param string $ghsaId Canonical GHSA identifier the document must describe. + * @param Response $response Successful response of the advisory request. + * + * @throws ParsingException if the body is not valid JSON. + * @throws RuntimeException if the document is not an object, describes another advisory, or has malformed + * identifiers. + */ + private function parse(string $ghsaId, Response $response): CveResolution + { $data = $response->decodeJson(); if (!is_array($data)) { @@ -84,13 +147,4 @@ public function resolve(string $ghsaId): CveResolution [] === $cves ? CveStatus::NONE_ASSIGNED : CveStatus::RESOLVED, ); } - - private function normalizeGhsaId(string $ghsaId): string - { - $ghsaId = trim($ghsaId); - - return AuditNormalizer::normalizeGhsaId($ghsaId) ?? throw new RuntimeException( - Message::AUDIT_GHSA_ID_INVALID->getMessage($ghsaId), - ); - } } diff --git a/src/Audit/Parser/AbstractAuditParser.php b/src/Audit/Parser/AbstractAuditParser.php index 72dce9e..b07c002 100644 --- a/src/Audit/Parser/AbstractAuditParser.php +++ b/src/Audit/Parser/AbstractAuditParser.php @@ -8,6 +8,7 @@ use Foxy\Exception\{Message, RuntimeException}; use JsonException; use stdClass; +use Throwable; use function array_map; use function get_object_vars; @@ -252,7 +253,7 @@ final protected function getStringList(mixed $value, string $context): array return $this->uniqueStrings($value); } - final protected function malformed(string $reason, \Throwable|null $previous = null): RuntimeException + final protected function malformed(string $reason, Throwable|null $previous = null): RuntimeException { return new RuntimeException( Message::AUDIT_OUTPUT_MALFORMED->getMessage($this->getManagerName(), $reason), diff --git a/src/Audit/Parser/AbstractBulkAdvisoryParser.php b/src/Audit/Parser/AbstractBulkAdvisoryParser.php new file mode 100644 index 0000000..669da51 --- /dev/null +++ b/src/Audit/Parser/AbstractBulkAdvisoryParser.php @@ -0,0 +1,57 @@ +decodeObject($output); + + $findings = []; + + foreach ($packages as $package => $advisories) { + if ($package === '' || !is_array($advisories)) { + throw $this->malformed( + Message::AUDIT_BULK_PACKAGE_ADVISORIES_REQUIRED->getMessage(), + ); + } + + $package = (string) $package; + + foreach ($advisories as $index => $advisory) { + $context = sprintf('%s.%d', $package, $index); + + $advisory = $this->getObject($advisory, $context); + $sourceId = $this->getSourceId($advisory['id'] ?? null, $context . '.id'); + $url = $this->getOptionalString($advisory, 'url', $context); + + $findings[] = new AuditFinding( + $this->sanitizeString($package), + $this->getSeverity($advisory['severity'] ?? null, $context), + $this->getAdvisoryId($sourceId, null, $url), + $sourceId, + $this->getOptionalString($advisory, 'title', $context) ?? 'Vulnerability found', + $this->getString($advisory, 'vulnerable_versions', $context), + $url, + ); + } + } + + return $findings; + } +} diff --git a/src/Audit/Parser/BunAuditParser.php b/src/Audit/Parser/BunAuditParser.php index fa84bdb..8620476 100644 --- a/src/Audit/Parser/BunAuditParser.php +++ b/src/Audit/Parser/BunAuditParser.php @@ -4,50 +4,11 @@ namespace Foxy\Audit\Parser; -use Foxy\Audit\{AuditFinding, AuditParserInterface}; -use Foxy\Exception\Message; - -use function sprintf; - -final class BunAuditParser extends AbstractAuditParser implements AuditParserInterface +/** + * Parses the `bun audit --json` report, which is the npm registry's bulk advisories document. + */ +final class BunAuditParser extends AbstractBulkAdvisoryParser { - public function parse(string $output): array - { - $packages = $this->decodeObject($output); - - $findings = []; - - foreach ($packages as $package => $advisories) { - if ($package === '' || !is_array($advisories)) { - throw $this->malformed( - Message::AUDIT_BUN_PACKAGE_ADVISORIES_REQUIRED->getMessage(), - ); - } - - $package = (string) $package; - - foreach ($advisories as $index => $advisory) { - $context = sprintf('%s.%d', $package, $index); - - $advisory = $this->getObject($advisory, $context); - $sourceId = $this->getSourceId($advisory['id'] ?? null, $context . '.id'); - $url = $this->getOptionalString($advisory, 'url', $context); - - $findings[] = new AuditFinding( - $this->sanitizeString($package), - $this->getSeverity($advisory['severity'] ?? null, $context), - $this->getAdvisoryId($sourceId, null, $url), - $sourceId, - $this->getOptionalString($advisory, 'title', $context) ?? 'Vulnerability found', - $this->getString($advisory, 'vulnerable_versions', $context), - $url, - ); - } - } - - return $findings; - } - protected function getManagerName(): string { return 'bun'; diff --git a/src/Audit/Parser/NativeAuditParser.php b/src/Audit/Parser/NativeAuditParser.php new file mode 100644 index 0000000..af77616 --- /dev/null +++ b/src/Audit/Parser/NativeAuditParser.php @@ -0,0 +1,16 @@ +vendorDirExisted = null !== $this->vendorDir && file_exists($this->vendorDir); - $im = $this->composer->getInstallationManager(); - - $composerFile = Factory::getComposerFile(); - $locker = LockerUtil::getLocker($this->io, $im, $composerFile); - - $lockFile = $locker->getJsonFile(); + $lockFile = new JsonFile(Factory::getLockFile(Factory::getComposerFile()), null, $this->io); $this->lockFile = $lockFile->getPath(); $this->lockFileExisted = $lockFile->exists(); if ($this->lockFileExisted) { - $this->lock = $locker->getLockData(); + $this->lock = $lockFile->read(); } $this->snapshotSaved = true; @@ -109,14 +107,38 @@ public function save(): self return $this; } + /** + * Creates the rollback installer over a private, subscriber-free event dispatcher with scripts disabled. + * + * Plugin listeners, Foxy's own included, and project scripts registered on the shared dispatcher never see the + * rollback; the private autoload generator keeps `pre-autoload-dump` and `post-autoload-dump` off it as well. + */ + private function createInstaller(): Installer + { + $dispatcher = (new EventDispatcher($this->composer, $this->io))->setRunScripts(false); + + return new Installer( + $this->io, + $this->composer->getConfig(), + // @phpstan-ignore argument.type (root packages all extend BasePackage, as Installer::create() assumes) + $this->composer->getPackage(), + $this->composer->getDownloadManager(), + $this->composer->getRepositoryManager(), + $this->composer->getLocker(), + $this->composer->getInstallationManager(), + $dispatcher, + new AutoloadGenerator($dispatcher, $this->io), + ); + } + private function getHydratedLock(): array { - return $this->hydratedLock ??= PackageUtil::loadLockPackages($this->lock, false); + return $this->hydratedLock ??= PackageUtil::loadLockPackages($this->lock); } private function getInstaller(): Installer { - return $this->installer ?? Installer::create($this->io, $this->composer); + return $this->installer ?? $this->createInstaller(); } private function isEnabled(): bool @@ -223,8 +245,7 @@ private function restorePreviousLockFile(): void ->setDumpAutoloader($dumpAutoloader) ->setOptimizeAutoloader($optimize) ->setClassMapAuthoritative($authoritative) - ->setApcuAutoloader($apcu) - ->setRunScripts(false); + ->setApcuAutoloader($apcu); $ignorePlatformReqs = false; diff --git a/src/Foxy.php b/src/Foxy.php index e111e42..2c80388 100644 --- a/src/Foxy.php +++ b/src/Foxy.php @@ -8,7 +8,7 @@ use Composer\DependencyResolver\Operation\InstallOperation; use Composer\EventDispatcher\EventSubscriberInterface; use Composer\Factory; -use Composer\Installer\{PackageEvent, PackageEvents}; +use Composer\Installer\{InstallerEvents, PackageEvent, PackageEvents}; use Composer\IO\IOInterface; use Composer\Plugin\Capability\CommandProvider as ComposerCommandProvider; use Composer\Plugin\{Capable, PluginInterface}; @@ -25,7 +25,7 @@ PnpmManager, YarnManager, }; -use Foxy\Audit\{AuditReport, AuditRequest, AuditRunner, AuditRunnerInterface, AuditableAssetManagerInterface}; +use Foxy\Audit\{AuditReport, AuditRequest, AuditRunner, AuditRunnerInterface}; use Foxy\Command\FoxyCommandProvider; use Foxy\Config\{Config, ConfigBuilder}; use Foxy\Exception\{Message, RuntimeException}; @@ -124,12 +124,6 @@ public function audit(AuditRequest $request): AuditReport ); } - if (!isset($this->assetManager) || !$this->assetManager instanceof AuditableAssetManagerInterface) { - throw new RuntimeException( - Message::FOXY_AUDIT_UNSUPPORTED_MANAGER->getMessage(), - ); - } - return (new AuditRunner($this->assetManager))->audit($request); } @@ -146,7 +140,7 @@ public function getCapabilities(): array public static function getSubscribedEvents(): array { return [ - ComposerUtil::getInitEventName() => [['init', 100]], + InstallerEvents::PRE_OPERATIONS_EXEC => [['init', 100]], PackageEvents::POST_PACKAGE_INSTALL => [['initOnInstall', 100]], ScriptEvents::POST_INSTALL_CMD => [['solveAssets', 100]], ScriptEvents::POST_UPDATE_CMD => [['solveAssets', 100]], @@ -249,6 +243,7 @@ private function createNativeManager( $config, $fs, new NativeInstaller($io, $fs, $registry, new DependencyResolver($registry, $io), new TarballExtractor($fs)), + $registry, ); } diff --git a/src/Json/JsonFile.php b/src/Json/JsonFile.php index 755f6cc..ab97c37 100644 --- a/src/Json/JsonFile.php +++ b/src/Json/JsonFile.php @@ -35,10 +35,10 @@ final class JsonFile extends \Composer\Json\JsonFile */ private static array $encodeArrayKeys = []; - private int|null $indent = null; - private array $mapKeys = []; + private bool $parsed = false; + /** * Encode package manifest data as JSON. * @@ -67,14 +67,6 @@ public function getArrayKeys(): array return $this->arrayKeys; } - /** - * Get the indent for this JSON file. - */ - public function getIndent(): int - { - return $this->ensureParsed(); - } - public function read(): array { $data = parent::read(); @@ -125,12 +117,14 @@ private function collectEmptyArrayKeys(array $data): array } /** - * Parses the original JSON document once and returns its detected indent. + * Parses the original JSON document once to collect its empty array and empty object keys. + * + * @throws RuntimeException if the file exists but cannot be read. */ - private function ensureParsed(): int + private function ensureParsed(): void { - if (null !== $this->indent) { - return $this->indent; + if ($this->parsed) { + return; } $content = ''; @@ -148,8 +142,7 @@ private function ensureParsed(): int $this->arrayKeys = JsonFormatter::getArrayKeys($content); $this->mapKeys = JsonFormatter::getMapKeys($content); - - return $this->indent = JsonFormatter::getIndent($content); + $this->parsed = true; } /** diff --git a/src/Json/JsonFormatter.php b/src/Json/JsonFormatter.php index 667bfb0..89d57e8 100644 --- a/src/Json/JsonFormatter.php +++ b/src/Json/JsonFormatter.php @@ -11,18 +11,15 @@ use function intdiv; use function json_decode; use function json_encode; -use function preg_match; use function preg_match_all; use function str_repeat; use function str_replace; use function strlen; -use function trim; final class JsonFormatter { public const string ARRAY_KEYS_REGEX = '/["\']([\w\-.]+)["\']\s*:\s*\[\s*]/'; public const int DEFAULT_INDENT = 4; - public const string INDENT_REGEX = '/^[{\[][\r\n]( +)["\']/'; public const string MAP_KEYS_REGEX = '/["\']([\w\-.]+)["\']\s*:\s*\{\s*}/'; /** @@ -73,20 +70,6 @@ public static function getArrayKeys(string $content): array return $matches[1]; } - /** - * Get the indent of file. - * - * @param string $content The content - */ - public static function getIndent(string $content): int - { - if (1 !== preg_match(self::INDENT_REGEX, trim($content), $matches)) { - return self::DEFAULT_INDENT; - } - - return strlen($matches[1]); - } - /** * Get the keys that were represented as empty JSON objects. * diff --git a/src/Native/DependencyResolver.php b/src/Native/DependencyResolver.php index a06cdc8..5a7e461 100644 --- a/src/Native/DependencyResolver.php +++ b/src/Native/DependencyResolver.php @@ -464,8 +464,10 @@ private static function retract(string $source, array &$constraints, array &$sel * already satisfies it. * * An optional requirement never changes an existing selection. When no version satisfies every constraint of a - * mandatory requirement, the version satisfying the mandatory constraints is selected and each optional constraint - * it violates is dropped with a warning. + * mandatory requirement, the highest version satisfying the requirement and the globally mandatory constraints + * (neither optional nor declared inside an optional subtree) is chosen: each optional constraint it violates is + * dropped with a warning, and a violated constraint from an optional subtree makes the conflict name that subtree in + * `$blockers` instead. * * @param array $metadata Metadata fetched so far, keyed by name. * @param array> $constraints Constraints recorded so far, keyed by name. @@ -584,7 +586,9 @@ private function select( foreach ($candidates as $candidate) { if (!$candidate['optional'] && !$candidate['range']->satisfies($version->version)) { - $blockers[] = (string) $candidate['root']; + /** @var string $root Every globally mandatory constraint is satisfied, so this one is conditional. */ + $root = $candidate['root']; + $blockers[] = $root; } } diff --git a/src/Native/LockFile.php b/src/Native/LockFile.php index 42e040b..2f12ab6 100644 --- a/src/Native/LockFile.php +++ b/src/Native/LockFile.php @@ -8,6 +8,7 @@ use Foxy\Exception\{Message, RuntimeException}; use Foxy\Json\JsonFile; use Seld\JsonLint\ParsingException; +use stdClass; use function count; use function is_array; @@ -206,7 +207,7 @@ private function package(string $name, mixed $entry): ResolvedPackage * * @param array $specs */ - private static function sorted(array $specs): object + private static function sorted(array $specs): stdClass { ksort($specs, SORT_STRING); diff --git a/src/Native/NativeInstaller.php b/src/Native/NativeInstaller.php index 604d935..e631b10 100644 --- a/src/Native/NativeInstaller.php +++ b/src/Native/NativeInstaller.php @@ -217,6 +217,9 @@ private static function entries(string $directory): array /** * Installs the packages into the install directory and removes the entries outside the install set. * + * The tarballs of the registry packages are first prefetched in parallel into the registry's cache; each package is + * then fetched, verified, and extracted in turn, so a failed prefetch surfaces as the error of its own fetch. + * * A symbolic link or a file standing in for the `@scope` directory of a package is removed first, so the package is * never written into the link target and a stale file cannot abort the installation. * @@ -232,6 +235,17 @@ private function installPackages(string $manifestPath, string $installDirectory, $this->fs->ensureDirectoryExists($temporary); $installed = []; + $downloads = []; + + foreach ($packages as $package) { + if (!$package->isLocal()) { + $downloads[] = $package; + } + } + + if ([] !== $downloads) { + $this->registry->prefetchTarballs($downloads); + } foreach ($packages as $package) { $target = "{$installDirectory}/{$package->name}"; diff --git a/src/Native/NpmRegistry.php b/src/Native/NpmRegistry.php index 6b03c21..256b020 100644 --- a/src/Native/NpmRegistry.php +++ b/src/Native/NpmRegistry.php @@ -16,6 +16,8 @@ use function base64_decode; use function hash_equals; use function hash_file; +use function is_file; +use function json_encode; use function min; use function preg_match; use function preg_split; @@ -27,14 +29,19 @@ use function str_starts_with; use function strlen; use function substr; +use function sys_get_temp_dir; +use function uniqid; use function unlink; +use const JSON_THROW_ON_ERROR; + /** - * Provides the npm registry access of the native manager: abbreviated metadata and integrity-verified tarballs. + * Provides the npm registry access of the native manager: abbreviated metadata, integrity-verified tarballs, and bulk + * security advisories. * * Requests answered with HTTP 429 are retried up to three times, waiting the `Retry-After` seconds (capped at 60) * or 1, 2, and 4 seconds. Tarballs are cached and verified against their Subresource Integrity value on every cache - * hit and every download. + * hit and every download; a prefetch fills the cache with parallel downloads and no retry. */ final readonly class NpmRegistry implements NpmRegistryInterface { @@ -44,14 +51,17 @@ * Hash algorithms accepted in an integrity value, strongest first. */ private const array ALGORITHMS = ['sha512', 'sha384', 'sha256', 'sha1']; + private const string BULK_ADVISORIES_PATH = '/-/npm/v1/security/advisories/bulk'; private const string METADATA_ACCEPT = 'Accept: application/vnd.npm.install-v1+json'; + private const string PREFETCH_FAILED = 'Prefetching the tarball of "%s" (%s) failed: %s'; private const int RETRIES = 3; private const int RETRY_AFTER_LIMIT = 60; /** - * @param HttpDownloader $httpDownloader Downloader that performs the requests. + * @param HttpDownloader $httpDownloader Downloader that performs the requests; {@see prefetchTarballs()} needs the + * async-enabled instance returned by `Composer::getLoop()->getHttpDownloader()`. * @param Cache $cache Tarball cache. - * @param IOInterface $io Output for the retry notices. + * @param IOInterface $io Output for the retry and failed prefetch notices. * @param string $url Registry base URL; a trailing slash is ignored. * @param (Closure(int): void)|null $delay Replacement for `sleep()` during the 429 back-off, or `null` to sleep. */ @@ -77,7 +87,7 @@ public function fetchTarball(ResolvedPackage $package, string $directory): strin { $resolved = $package->resolved ?? ''; $integrity = $package->integrity ?? ''; - $key = "tarball/{$package->name}/{$package->version}-" . sha1($resolved) . '.tgz'; + $key = $this->tarballKey($package); $file = "{$directory}/" . str_replace(['@', '/'], ['', '-'], $package->name) . "-{$package->version}.tgz"; if ($this->cache->copyTo($key, $file)) { @@ -103,6 +113,37 @@ public function fetchTarball(ResolvedPackage $package, string $directory): strin return $file; } + /** + * Returns the registry's bulk advisories document for the given versions, as the registry wrote it. + * + * Sends one `POST` to npm's bulk advisories endpoint with a JSON object body; an empty response body is returned + * as an empty string. + * + * @param array> $versions Locked versions keyed by package name. + * + * @throws RuntimeException if the request fails, including a registry that does not implement the endpoint. + */ + public function getAdvisories(array $versions): string + { + $url = $this->registryUrl() . self::BULK_ADVISORIES_PATH; + + $response = $this->send( + $url, + fn(): Response => $this->httpDownloader->get( + $url, + [ + 'http' => [ + 'method' => 'POST', + 'header' => ['Content-Type: application/json', 'Accept: application/json'], + 'content' => json_encode((object) $versions, JSON_THROW_ON_ERROR), + ], + ], + ), + ); + + return $response->getBody() ?? ''; + } + /** * Returns the abbreviated metadata of a package. * @@ -134,6 +175,64 @@ public function getMetadata(string $name): PackageMetadata return PackageMetadata::fromDocument($name, $document); } + /** + * Downloads the tarballs missing from the cache in parallel, verifying each before it is cached. + * + * Nothing is downloaded when the cache is disabled or read-only, since the downloads could not be kept. A package + * without a tarball URL is skipped. A download that fails or does not match its integrity value is discarded with + * a verbose notice; {@see fetchTarball()} downloads it again and reports the error. Every temporary file is + * removed before the method returns. + * + * @param list $packages Registry packages whose tarballs are wanted. + */ + public function prefetchTarballs(array $packages): void + { + if (!$this->cache->isEnabled() || $this->cache->isReadOnly()) { + return; + } + + $queued = false; + + foreach ($packages as $package) { + $resolved = $package->resolved ?? ''; + $key = $this->tarballKey($package); + + if ('' === $resolved || false !== $this->cache->getAge($key)) { + continue; + } + + $file = sys_get_temp_dir() . '/' . uniqid('foxy-tarball-', true) . '.tgz'; + + $this->httpDownloader->addCopy($resolved, $file)->then( + function () use ($package, $key, $file, $resolved): void { + if ($this->matchesIntegrity($file, $package->integrity ?? '')) { + $this->cache->copyFrom($key, $file); + } else { + $this->reportPrefetchFailure( + $package, + Message::NATIVE_INTEGRITY_MISMATCH->getMessage($resolved), + ); + } + + unlink($file); + }, + function (Throwable $exception) use ($package, $file): void { + if (is_file($file)) { + unlink($file); + } + + $this->reportPrefetchFailure($package, $exception->getMessage()); + }, + ); + + $queued = true; + } + + if ($queued) { + $this->httpDownloader->wait(); + } + } + /** * Returns whether the file matches any digest of the strongest known algorithm of a Subresource Integrity value. * @@ -175,6 +274,18 @@ private function registryUrl(): string return rtrim($this->url, '/'); } + /** + * Writes the verbose notice of a failed prefetch, naming the package and the cause. + */ + private function reportPrefetchFailure(ResolvedPackage $package, string $reason): void + { + $this->io->writeError( + sprintf(self::PREFETCH_FAILED, $package->name, $package->version, $reason), + true, + IOInterface::VERBOSE, + ); + } + /** * Returns the seconds to wait before a retry: the `Retry-After` integer capped at 60, else 1, 2, or 4. * @@ -235,4 +346,12 @@ private function send(string $url, Closure $request, string|null $notFound = nul } } } + + /** + * Returns the cache key of a package's tarball: name, version, and the SHA-1 of the tarball URL. + */ + private function tarballKey(ResolvedPackage $package): string + { + return "tarball/{$package->name}/{$package->version}-" . sha1($package->resolved ?? '') . '.tgz'; + } } diff --git a/src/Native/NpmRegistryInterface.php b/src/Native/NpmRegistryInterface.php index 091024f..d6b69df 100644 --- a/src/Native/NpmRegistryInterface.php +++ b/src/Native/NpmRegistryInterface.php @@ -7,7 +7,7 @@ use Foxy\Exception\RuntimeException; /** - * Provides access to an npm registry: package metadata and verified tarballs. + * Provides access to an npm registry: package metadata, verified tarballs, and security advisories. */ interface NpmRegistryInterface { @@ -21,6 +21,15 @@ interface NpmRegistryInterface */ public function fetchTarball(ResolvedPackage $package, string $directory): string; + /** + * Returns the registry's bulk advisories document for the given versions, as the registry wrote it. + * + * @param array> $versions Locked versions keyed by package name. + * + * @throws RuntimeException if the request fails. + */ + public function getAdvisories(array $versions): string; + /** * Returns the abbreviated metadata of a package. * @@ -29,4 +38,14 @@ public function fetchTarball(ResolvedPackage $package, string $directory): strin * @throws RuntimeException if the package is unknown, the request fails, or the metadata is malformed. */ public function getMetadata(string $name): PackageMetadata; + + /** + * Downloads the tarballs missing from the cache in parallel, verifying each before it is cached. + * + * The prefetch is best-effort: a tarball that fails to download or to verify is left out of the cache, and the + * later {@see fetchTarball()} downloads it again and reports the error. + * + * @param list $packages Registry packages whose tarballs are wanted. + */ + public function prefetchTarballs(array $packages): void; } diff --git a/src/Util/ComposerUtil.php b/src/Util/ComposerUtil.php index 9143bae..5eb7096 100644 --- a/src/Util/ComposerUtil.php +++ b/src/Util/ComposerUtil.php @@ -4,7 +4,6 @@ namespace Foxy\Util; -use Composer\Installer\InstallerEvents; use Composer\Semver\Semver; use Foxy\Exception\{Message, RuntimeException}; @@ -13,14 +12,6 @@ final class ComposerUtil { - /** - * Get the event name to init the plugin. - */ - public static function getInitEventName(): string - { - return InstallerEvents::PRE_OPERATIONS_EXEC; - } - /** * Validate the composer version. * diff --git a/src/Util/LockerUtil.php b/src/Util/LockerUtil.php deleted file mode 100644 index 03068bd..0000000 --- a/src/Util/LockerUtil.php +++ /dev/null @@ -1,22 +0,0 @@ - $config['alias'], - 'alias_normalized' => $config['alias_normalized'], - ]; - } - - $lockData['aliases'] = $alias; - - return $lockData; - } - /** * Load the packages in the packages section of the locker load data. * @@ -75,20 +41,18 @@ public static function loadLockPackage(ArrayLoader $loader, array $lockData, boo } /** - * Load all packages in the lock data of locker. + * Loads the `packages` and `packages-dev` entries of the lock data, leaving every other key unchanged. * - * @param array $lockData The lock data of locker. - * @param bool $convertAliases Whether the aliases should be converted to the legacy lookup map. + * @param array $lockData The raw lock data, as read from the lock file. * - * @return array The lock data. + * @return array The lock data with package entries replaced by package instances. */ - public static function loadLockPackages(array $lockData, bool $convertAliases = true): array + public static function loadLockPackages(array $lockData): array { $loader = new ArrayLoader(); $lockData = self::loadLockPackage($loader, $lockData); - $lockData = self::loadLockPackage($loader, $lockData, true); - return $convertAliases ? self::convertLockAlias($lockData) : $lockData; + return self::loadLockPackage($loader, $lockData, true); } } diff --git a/tests/Asset/AbstractAssetManagerTest.php b/tests/Asset/AbstractAssetManagerTest.php index fffec32..334806a 100644 --- a/tests/Asset/AbstractAssetManagerTest.php +++ b/tests/Asset/AbstractAssetManagerTest.php @@ -16,6 +16,7 @@ use PHPUnit\Framework\Attributes\{DataProviderExternal, PreserveGlobalState, RunInSeparateProcess}; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use RuntimeException; use Seld\JsonLint\ParsingException; use Symfony\Component\Filesystem\Filesystem as SymfonyFilesystem; use Xepozz\InternalMocker\MockerState; @@ -318,7 +319,7 @@ static function (mixed $command, mixed &$output = null) use ( $observedTimeout = ProcessExecutor::getTimeout(); - throw new \RuntimeException('Audit execution failed.'); + throw new RuntimeException('Audit execution failed.'); }, ); $this->fallback->expects(self::never())->method('restore'); @@ -340,7 +341,7 @@ static function (mixed $command, mixed &$output = null) use ( self::fail( 'Expected the audit process to fail.', ); - } catch (\RuntimeException $exception) { + } catch (RuntimeException $exception) { self::assertSame( 'Audit execution failed.', $exception->getMessage(), diff --git a/tests/Asset/AuditableAssetManager.php b/tests/Asset/AuditableAssetManager.php index 4a8af77..18219e1 100644 --- a/tests/Asset/AuditableAssetManager.php +++ b/tests/Asset/AuditableAssetManager.php @@ -4,7 +4,8 @@ namespace Foxy\Tests\Asset; -use Foxy\Audit\AuditableAssetManagerInterface; +use Foxy\Asset\AbstractAssetManager; +use Foxy\Audit\AuditParserInterface; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Tests\Provider\AuditableAssetManagerProvider; @@ -17,7 +18,7 @@ use const DIRECTORY_SEPARATOR; /** - * Base unit tests for {@see AuditableAssetManagerInterface} asset managers, shared by the audit-capable manager tests. + * Base unit tests for the audit of {@see AbstractAssetManager} subclasses, shared by the process-based manager tests. * * {@see AuditableAssetManagerProvider} for test case data providers. */ @@ -25,6 +26,11 @@ abstract class AuditableAssetManager extends AssetManager { abstract protected function getValidAuditCommand(bool $noDev): string; + /** + * @return class-string + */ + abstract protected function getValidAuditParserClass(): string; + #[DataProviderExternal(AuditableAssetManagerProvider::class, 'auditScopes')] public function testAuditBuildsExactCommandWithoutInstallOptions(bool $noDev): void { @@ -42,12 +48,6 @@ public function testAuditBuildsExactCommandWithoutInstallOptions(bool $noDev): v $this->executor->addExpectedValues(0, $this->getValidVersion()); $this->executor->addExpectedValues(1, 'AUDIT OUTPUT'); - self::assertInstanceOf( - AuditableAssetManagerInterface::class, - $this->manager, - 'The manager should implement the AuditableAssetManagerInterface', - ); - $result = $this->manager->audit($noDev); self::assertSame( @@ -87,12 +87,6 @@ public function testAuditRejectsMissingLockFileBeforeManagerAuditCommand(): void $this->manager = $this->getManager(); - self::assertInstanceOf( - AuditableAssetManagerInterface::class, - $this->manager, - 'The manager should implement the AuditableAssetManagerInterface', - ); - try { $this->manager->audit(false); @@ -136,12 +130,6 @@ public function testAuditResolvesManagerSpecificBinaryConfiguration(): void $this->executor->addExpectedValues(0, $this->getValidVersion()); $this->executor->addExpectedValues(0, '{}'); - self::assertInstanceOf( - AuditableAssetManagerInterface::class, - $this->manager, - 'The manager should implement the AuditableAssetManagerInterface', - ); - $this->manager->audit(false); [, $auditArguments] = explode(' ', $this->getValidAuditCommand(false), 2); @@ -181,12 +169,6 @@ public function testAuditUsesConfiguredRootDirectoryWithoutChangingProcessDirect $this->executor->addExpectedValues(0, $this->getValidVersion()); $this->executor->addExpectedValues(0, '{}'); - self::assertInstanceOf( - AuditableAssetManagerInterface::class, - $this->manager, - 'The manager should implement the AuditableAssetManagerInterface', - ); - $this->manager->audit(false); self::assertSame( @@ -205,4 +187,13 @@ public function testAuditUsesConfiguredRootDirectoryWithoutChangingProcessDirect 'The current working directory should not have changed after the audit', ); } + + public function testGetAuditParserReturnsParserOfManagerReport(): void + { + self::assertInstanceOf( + $this->getValidAuditParserClass(), + $this->getManager()->getAuditParser(), + 'The parser should read the report format of this manager', + ); + } } diff --git a/tests/Asset/BunAssetManagerTest.php b/tests/Asset/BunAssetManagerTest.php index c30362a..6ee5ed8 100644 --- a/tests/Asset/BunAssetManagerTest.php +++ b/tests/Asset/BunAssetManagerTest.php @@ -6,10 +6,12 @@ use Composer\Util\Platform; use Foxy\Asset\BunManager; +use Foxy\Audit\Parser\BunAuditParser; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Tests\Provider\BunAssetManagerProvider; use PHPUnit\Framework\Attributes\{DataProviderExternal, PreserveGlobalState, RunInSeparateProcess}; +use Stringable; use Xepozz\InternalMocker\MockerState; use function array_key_exists; @@ -673,8 +675,8 @@ public function testWindowsAuditUsesCaseInsensitiveEnvironmentPrecedence(): void putenv("XDG_CONFIG_HOME={$processConfig}"); unset($_ENV['XDG_CONFIG_HOME']); - $_ENV['xdg_config_home'] = new class ($environmentConfig) implements \Stringable { - public function __construct(private readonly string $value) {} + $_ENV['xdg_config_home'] = new readonly class ($environmentConfig) implements Stringable { + public function __construct(private string $value) {} public function __toString(): string { @@ -825,6 +827,11 @@ protected function getValidAuditCommand(bool $noDev): string return $binary . ' audit --json' . ($noDev ? ' --prod' : ''); } + protected function getValidAuditParserClass(): string + { + return BunAuditParser::class; + } + protected function getValidInstallCommand(): string { return Platform::isWindows() ? 'bun.exe install' : 'bun install'; diff --git a/tests/Asset/DenoAssetManagerTest.php b/tests/Asset/DenoAssetManagerTest.php index 6d5772e..1ea48c9 100644 --- a/tests/Asset/DenoAssetManagerTest.php +++ b/tests/Asset/DenoAssetManagerTest.php @@ -8,6 +8,7 @@ use Composer\Package\RootPackageInterface; use Composer\Util\{Filesystem, Platform, ProcessExecutor}; use Foxy\Asset\{AssetPackageInterface, DenoManager}; +use Foxy\Audit\Parser\DenoAuditParser; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Tests\Provider\DenoAssetManagerProvider; @@ -643,6 +644,11 @@ protected function getValidAuditCommand(bool $noDev): string return $this->getBinary() . ' audit --level=low'; } + protected function getValidAuditParserClass(): string + { + return DenoAuditParser::class; + } + protected function getValidInstallCommand(): string { return $this->getBinary() . ' install'; diff --git a/tests/Asset/NativeManagerTest.php b/tests/Asset/NativeManagerTest.php index 4b67154..5842a07 100644 --- a/tests/Asset/NativeManagerTest.php +++ b/tests/Asset/NativeManagerTest.php @@ -8,14 +8,17 @@ use Composer\Package\RootPackageInterface; use Composer\Util\{Filesystem, Platform}; use Foxy\Asset\NativeManager; +use Foxy\Audit\Parser\NativeAuditParser; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Fallback\FallbackInterface; use Foxy\Native\NativeInstallerInterface; +use Foxy\Tests\Fixtures\Native\InMemoryRegistry; use Foxy\Tests\Provider\NativeManagerProvider; use PHPUnit\Framework\Attributes\{DataProviderExternal, PreserveGlobalState, RunInSeparateProcess}; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use stdClass; use Xepozz\InternalMocker\MockerState; use function basename; @@ -26,6 +29,7 @@ use function file_get_contents; use function file_put_contents; use function getcwd; +use function json_encode; use function mkdir; use function str_replace; use function strtr; @@ -35,15 +39,18 @@ use function unlink; use const DIRECTORY_SEPARATOR; +use const JSON_THROW_ON_ERROR; /** - * Unit tests for {@see NativeManager} selection, validation, install directory resolution, and installer orchestration. + * Unit tests for {@see NativeManager} selection, validation, install directory resolution, installer orchestration, and + * the registry audit. * * {@see NativeManagerProvider} for test case data providers. */ final class NativeManagerTest extends TestCase { private const string INSTALLING = 'Installing frontend dependencies with the native manager'; + private const string INTEGRITY = 'sha512-aGVsbG8='; private const string UPDATING = 'Updating frontend dependencies with the native manager'; private string $cwd = ''; @@ -56,6 +63,7 @@ final class NativeManagerTest extends TestCase */ private array $links = []; private string $oldCwd = ''; + private InMemoryRegistry $registry; public function testAddDependenciesWritesManifest(): void { @@ -77,10 +85,112 @@ public function testAddDependenciesWritesManifest(): void ); } + public function testAuditRequestsRegistryPackagesSortedByNameWithoutLocalPackages(): void + { + $this->writeLock( + [ + 'zeta' => self::registryEntry('zeta', '2.0.0'), + '@composer-asset/foo--bar' => ['version' => '1.0.0', 'file' => './path/foo/bar'], + 'alpha' => self::registryEntry('alpha', '1.2.3'), + '9' => self::registryEntry('9', '0.9.0'), + '10' => self::registryEntry('10', '1.0.0'), + ], + ); + + $result = $this->manager()->audit(false); + + self::assertSame( + [ + [ + '10' => ['1.0.0'], + '9' => ['0.9.0'], + 'alpha' => ['1.2.3'], + 'zeta' => ['2.0.0'], + ], + ], + $this->registry->advisoryRequests, + 'One request must carry the registry versions sorted as strings.', + ); + self::assertSame( + 0, + $result->exitCode, + 'An empty document must exit with `0`.', + ); + self::assertSame( + '{}', + $result->output, + 'The document must be returned as received.', + ); + } + + public function testAuditReturnsDocumentWithFindingsAndExitOne(): void + { + $document = (string) file_get_contents(dirname(__DIR__) . '/Fixtures/Audit/bun-populated.json'); + + $this->registry->addAdvisories($document); + + $this->writeLock(['lodash' => self::registryEntry('lodash', '4.17.15')]); + + $result = $this->manager()->audit(false); + + self::assertSame( + 1, + $result->exitCode, + 'A document with advisories must exit with `1`.', + ); + self::assertSame( + $document, + $result->output, + 'The document must be returned verbatim.', + ); + self::assertSame( + '', + $result->errorOutput, + 'The error output must be empty.', + ); + } + + public function testAuditSkipsRequestWhenLockHoldsOnlyLocalPackages(): void + { + $this->writeLock(['@composer-asset/foo--bar' => ['version' => '1.0.0', 'file' => './path/foo/bar']]); + + $result = $this->manager()->audit(false); + + self::assertSame( + [], + $this->registry->advisoryRequests, + 'No request must be sent without registry packages.', + ); + self::assertSame( + 0, + $result->exitCode, + 'A lock without registry packages must exit with `0`.', + ); + self::assertSame( + '{}', + $result->output, + 'The document must be the empty object.', + ); + self::assertSame( + '', + $result->errorOutput, + 'The error output must be empty.', + ); + } + + public function testGetAuditParserReturnsNativeParser(): void + { + self::assertInstanceOf( + NativeAuditParser::class, + $this->manager()->getAuditParser(), + 'The native parser must read the bulk advisories document.', + ); + } + public function testIsAvailableRequiresLockFileAndResolvesManager(): void { $config = new Config(['manager-timeout' => ['native' => 5]]); - $manager = new NativeManager($this->io, $config, new Filesystem(), $this->installer); + $manager = new NativeManager($this->io, $config, new Filesystem(), $this->installer, $this->registry); self::assertNull( $config->get('manager-timeout'), @@ -375,6 +485,48 @@ public function testThrowRuntimeExceptionForInvalidInstallDirectory(string $valu $this->manager(['run-asset-manager' => true, 'native-install-dir' => strtr($value, $placeholders)])->run(); } + public function testThrowRuntimeExceptionForMalformedAdvisoriesDocument(): void + { + $this->registry->addAdvisories('[]'); + + $this->writeLock(['lodash' => self::registryEntry('lodash', '4.17.15')]); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + Message::AUDIT_OUTPUT_MALFORMED->getMessage( + 'native', + Message::AUDIT_REPORT_JSON_OBJECT_EXPECTED->getMessage(), + ), + ); + + $this->manager()->audit(false); + } + + public function testThrowRuntimeExceptionWhenAuditLockFileIsInvalid(): void + { + file_put_contents("{$this->cwd}/foxy.lock", '{}'); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + Message::NATIVE_LOCK_INVALID->getMessage($this->cwd . DIRECTORY_SEPARATOR . 'foxy.lock'), + ); + + $this->manager()->audit(false); + } + + public function testThrowRuntimeExceptionWhenAuditLockFileIsMissing(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + Message::ASSET_MANAGER_LOCK_FILE_MISSING->getMessage( + 'native', + $this->cwd . DIRECTORY_SEPARATOR . 'foxy.lock', + ), + ); + + $this->manager()->audit(false); + } + public function testThrowRuntimeExceptionWhenFallbackRestoreFails(): void { $failure = new RuntimeException('Install failed.'); @@ -469,6 +621,31 @@ public function testThrowRuntimeExceptionWhenInstallDirectoryIsSymlinkedRootPack )->run(); } + public function testThrowRuntimeExceptionWhenProductionOnlyAuditIsRequested(): void + { + $this->writeLock(['lodash' => self::registryEntry('lodash', '4.17.15')]); + + try { + $this->manager()->audit(true); + + self::fail( + 'Expected the production-only native audit to be rejected.', + ); + } catch (RuntimeException $exception) { + self::assertSame( + Message::ASSET_NATIVE_AUDIT_NO_DEV_UNSUPPORTED->getMessage(), + $exception->getMessage(), + 'The rejection must explain the dependency scope.', + ); + } + + self::assertSame( + [], + $this->registry->advisoryRequests, + 'The registry must not be asked.', + ); + } + public function testThrowRuntimeExceptionWhenRootPackageDirIsMissing(): void { $this->installer @@ -531,6 +708,7 @@ protected function setUp(): void $this->fallback = $this->createMock(FallbackInterface::class); $this->installer = $this->createMock(NativeInstallerInterface::class); $this->io = $this->createMock(IOInterface::class); + $this->registry = new InMemoryRegistry(); $this->oldCwd = (string) getcwd(); $directory = sys_get_temp_dir() . '/' . uniqid('foxy_native_manager_test_', true); @@ -595,7 +773,14 @@ private function link(string $target, string $link): void */ private function manager(array $config = []): NativeManager { - return new NativeManager($this->io, new Config($config), new Filesystem(), $this->installer, $this->fallback); + return new NativeManager( + $this->io, + new Config($config), + new Filesystem(), + $this->installer, + $this->registry, + $this->fallback, + ); } private function markInstalled(): void @@ -604,6 +789,20 @@ private function markInstalled(): void mkdir("{$this->cwd}/node_modules"); } + /** + * Returns a registry lock entry for the version. + * + * @return array + */ + private static function registryEntry(string $name, string $version): array + { + return [ + 'version' => $version, + 'resolved' => "https://registry.npmjs.org/{$name}/-/{$name}-{$version}.tgz", + 'integrity' => self::INTEGRITY, + ]; + } + /** * Skips the test on Windows, where `realpath()` does not resolve symbolic links. */ @@ -613,4 +812,17 @@ private static function skipUnresolvedLinks(): void self::markTestSkipped('PHP does not resolve symbolic links with realpath() on Windows.'); } } + + /** + * Writes `foxy.lock` with the given package entries and no requirements. + * + * @param array> $packages Lock entries keyed by package name, in file order. + */ + private function writeLock(array $packages): void + { + file_put_contents( + "{$this->cwd}/foxy.lock", + json_encode(['requirements' => new stdClass(), 'packages' => (object) $packages], JSON_THROW_ON_ERROR), + ); + } } diff --git a/tests/Asset/NpmAssetManagerTest.php b/tests/Asset/NpmAssetManagerTest.php index da00ec0..2454d5b 100644 --- a/tests/Asset/NpmAssetManagerTest.php +++ b/tests/Asset/NpmAssetManagerTest.php @@ -7,6 +7,7 @@ use Composer\Package\RootPackageInterface; use Composer\Util\ProcessExecutor; use Foxy\Asset\NpmManager; +use Foxy\Audit\Parser\NpmAuditParser; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Tests\Provider\NpmAssetManagerProvider; @@ -239,6 +240,11 @@ protected function getValidAuditCommand(bool $noDev): string : ' --include=dev --include=optional --include=peer'); } + protected function getValidAuditParserClass(): string + { + return NpmAuditParser::class; + } + protected function getValidInstallCommand(): string { return 'npm install'; diff --git a/tests/Asset/PnpmAssetManagerTest.php b/tests/Asset/PnpmAssetManagerTest.php index e81a45f..16e9c52 100644 --- a/tests/Asset/PnpmAssetManagerTest.php +++ b/tests/Asset/PnpmAssetManagerTest.php @@ -6,6 +6,7 @@ use Composer\Util\ProcessExecutor; use Foxy\Asset\PnpmManager; +use Foxy\Audit\Parser\PnpmAuditParser; use Foxy\Config\Config; use function file_get_contents; @@ -101,6 +102,11 @@ protected function getValidAuditCommand(bool $noDev): string . ProcessExecutor::escape('--config.auditConfig={ignoreGhsas:[]}'); } + protected function getValidAuditParserClass(): string + { + return PnpmAuditParser::class; + } + protected function getValidInstallCommand(): string { return 'pnpm install'; diff --git a/tests/Asset/YarnAssetManagerTest.php b/tests/Asset/YarnAssetManagerTest.php index 20d0960..0eb3873 100644 --- a/tests/Asset/YarnAssetManagerTest.php +++ b/tests/Asset/YarnAssetManagerTest.php @@ -6,6 +6,8 @@ use Composer\Util\ProcessExecutor; use Foxy\Asset\YarnManager; +use Foxy\Audit\Parser\YarnAuditParser; +use RuntimeException; use function array_key_exists; use function file_put_contents; @@ -168,7 +170,7 @@ static function (mixed $command, mixed &$output = null) use (&$position): int { ); } - throw new \RuntimeException( + throw new RuntimeException( 'Audit execution failed.', ); }, @@ -182,7 +184,7 @@ static function (mixed $command, mixed &$output = null) use (&$position): int { self::fail( 'Expected the audit process to fail.', ); - } catch (\RuntimeException $exception) { + } catch (RuntimeException $exception) { self::assertSame( 'Audit execution failed.', $exception->getMessage(), @@ -262,6 +264,11 @@ protected function getValidAuditCommand(bool $noDev): string return $command . ($noDev ? ' --environment production' : ''); } + protected function getValidAuditParserClass(): string + { + return YarnAuditParser::class; + } + protected function getValidInstallCommand(): string { return 'yarn install'; diff --git a/tests/Audit/AuditParserTest.php b/tests/Audit/AuditParserTest.php index 8e1080d..c8a29c6 100644 --- a/tests/Audit/AuditParserTest.php +++ b/tests/Audit/AuditParserTest.php @@ -5,8 +5,15 @@ namespace Foxy\Tests\Audit; use Closure; -use Foxy\Audit\{AuditParserFactory, AuditParserInterface, CveStatus}; -use Foxy\Audit\Parser\{BunAuditParser, DenoAuditParser, NpmAuditParser, PnpmAuditParser, YarnAuditParser}; +use Foxy\Audit\{AuditParserInterface, CveStatus}; +use Foxy\Audit\Parser\{ + BunAuditParser, + DenoAuditParser, + NativeAuditParser, + NpmAuditParser, + PnpmAuditParser, + YarnAuditParser, +}; use Foxy\Audit\Severity; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Tests\Provider\AuditParserProvider; @@ -20,7 +27,7 @@ use const JSON_THROW_ON_ERROR; /** - * Unit tests for {@see AuditParserFactory} and the npm, pnpm, Yarn, Bun, and Deno audit report parsers. + * Unit tests for the npm, pnpm, Yarn, Bun, Deno, and native audit report parsers. * * {@see AuditParserProvider} for test case data providers. */ @@ -147,6 +154,72 @@ public function testBunParserReadsRawBulkAuditReport(): void ); } + public function testNativeParserReadsRawBulkAdvisoriesDocument(): void + { + $findings = (new NativeAuditParser())->parse(self::fixture('bun-populated.json')); + + self::assertCount( + 2, + $findings, + 'The document should yield one finding per advisory', + ); + self::assertSame( + 'lodash', + $findings[0]->package, + 'The package should match the document key', + ); + self::assertSame( + Severity::HIGH, + $findings[0]->severity, + 'The severity should match the expected value', + ); + self::assertSame( + 'GHSA-35jh-r3h4-6jhm', + $findings[0]->advisoryId, + 'The GitHub advisory ID should be extracted from the advisory URL', + ); + self::assertSame( + '1106913', + $findings[0]->sourceId, + 'The integer advisory ID should become the string source ID', + ); + self::assertSame( + 'Command Injection in lodash', + $findings[0]->title, + 'The title should match the expected value', + ); + self::assertSame( + '<4.17.21', + $findings[0]->vulnerableVersions, + 'The vulnerable range should come from `vulnerable_versions`', + ); + self::assertSame( + 'https://github.com/advisories/GHSA-35jh-r3h4-6jhm', + $findings[0]->url, + 'The URL should match the expected value', + ); + self::assertSame( + [], + $findings[0]->affectedVersions, + 'The affected versions should be empty because the document does not provide them', + ); + self::assertSame( + [], + $findings[0]->dependencyPaths, + 'The dependency paths should be empty because the document does not provide them', + ); + self::assertSame( + '1107000', + $findings[1]->advisoryId, + 'The advisory ID should fall back to the source ID for a non-GitHub URL', + ); + self::assertSame( + 'Vulnerability found', + $findings[1]->title, + 'The missing title should fall back to the default title', + ); + } + public function testNpmParserAcceptsNumericPackageKeyAndObjectFixAvailability(): void { $data = json_decode(self::fixture('npm-populated.json'), true, 512, JSON_THROW_ON_ERROR); @@ -245,35 +318,6 @@ public function testNpmParserEmitsOnlyConcreteViaAdvisories(): void ); } - public function testParserFactoryCreatesEverySupportedParser(): void - { - self::assertInstanceOf( - NpmAuditParser::class, - AuditParserFactory::create('npm'), - 'The `npm` manager should resolve to the npm parser', - ); - self::assertInstanceOf( - PnpmAuditParser::class, - AuditParserFactory::create('pnpm'), - 'The `pnpm` manager should resolve to the pnpm parser', - ); - self::assertInstanceOf( - YarnAuditParser::class, - AuditParserFactory::create('yarn'), - 'The `yarn` manager should resolve to the Yarn parser', - ); - self::assertInstanceOf( - BunAuditParser::class, - AuditParserFactory::create('bun'), - 'The `bun` manager should resolve to the Bun parser', - ); - self::assertInstanceOf( - DenoAuditParser::class, - AuditParserFactory::create('deno'), - 'The `deno` manager should resolve to the Deno parser', - ); - } - public function testParsersReadCleanReports(): void { self::assertSame( @@ -301,6 +345,11 @@ public function testParsersReadCleanReports(): void (new DenoAuditParser())->parse(self::fixture('deno-clean.txt')), 'The clean Deno report should not contain any finding', ); + self::assertSame( + [], + (new NativeAuditParser())->parse(self::fixture('bun-clean.json')), + 'The clean native report should not contain any finding', + ); } public function testPnpmParserAcceptsZeroIdAndPreservesOptionalHeaderValuesAndCves(): void @@ -479,16 +528,6 @@ public function testThrowRuntimeExceptionForMalformedReport( $parser->parse($output); } - public function testThrowRuntimeExceptionForUnsupportedManager(): void - { - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage( - Message::AUDIT_PARSER_UNSUPPORTED_MANAGER->getMessage('legacy'), - ); - - AuditParserFactory::create('legacy'); - } - public function testThrowRuntimeExceptionWhenNpmMetadataCountsMismatchEntries(): void { $data = json_decode(self::fixture('npm-populated.json'), true, 512, JSON_THROW_ON_ERROR); diff --git a/tests/Audit/AuditRunnerTest.php b/tests/Audit/AuditRunnerTest.php index 6ec0b02..fde78e9 100644 --- a/tests/Audit/AuditRunnerTest.php +++ b/tests/Audit/AuditRunnerTest.php @@ -5,7 +5,15 @@ namespace Foxy\Tests\Audit; use Foxy\Asset\AssetManagerInterface; -use Foxy\Audit\{AuditProcessResult, AuditRequest, AuditRunner, AuditableAssetManagerInterface, CveStatus, Severity}; +use Foxy\Audit\{AuditParserInterface, AuditProcessResult, AuditRequest, AuditRunner, CveStatus, Severity}; +use Foxy\Audit\Parser\{ + BunAuditParser, + DenoAuditParser, + NativeAuditParser, + NpmAuditParser, + PnpmAuditParser, + YarnAuditParser, +}; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Tests\Provider\AuditRunnerProvider; use PHPUnit\Framework\Attributes\DataProviderExternal; @@ -100,6 +108,24 @@ public function testRunnerAcceptsExitOneWithValidFindingsAndForwardsNoDev(): voi ); } + public function testRunnerAcceptsNativeDocumentWithExitOne(): void + { + $manager = $this->manager('native', new AuditProcessResult(1, self::fixture('bun-populated.json'), '')); + + $report = (new AuditRunner($manager))->audit(new AuditRequest()); + + self::assertSame( + 'native', + $report->manager, + 'The report manager should match the expected value', + ); + self::assertCount( + 2, + $report->findings, + 'The native document should yield one finding per advisory', + ); + } + public function testRunnerAcceptsSuccessfulCleanReport(): void { $manager = $this->manager('bun', new AuditProcessResult(0, self::fixture('bun-clean.json'), "\n")); @@ -351,14 +377,8 @@ private function manager( string $name, AuditProcessResult $result, bool $expectedNoDev = false, - ): AssetManagerInterface&AuditableAssetManagerInterface&MockObject { - /** @var AssetManagerInterface&AuditableAssetManagerInterface&MockObject $manager */ - $manager = $this->createMockForIntersectionOfInterfaces( - [ - AssetManagerInterface::class, - AuditableAssetManagerInterface::class, - ], - ); + ): AssetManagerInterface&MockObject { + $manager = $this->createMock(AssetManagerInterface::class); $manager ->expects(self::once()) @@ -369,7 +389,23 @@ private function manager( ->method('audit') ->with($expectedNoDev) ->willReturn($result); + $manager + ->method('getAuditParser') + ->willReturn(self::parser($name)); return $manager; } + + private static function parser(string $name): AuditParserInterface + { + return match ($name) { + 'bun' => new BunAuditParser(), + 'deno' => new DenoAuditParser(), + 'native' => new NativeAuditParser(), + 'npm' => new NpmAuditParser(), + 'pnpm' => new PnpmAuditParser(), + 'yarn' => new YarnAuditParser(), + default => self::fail("No parser is registered for the {$name} manager."), + }; + } } diff --git a/tests/Audit/CveEnricherTest.php b/tests/Audit/CveEnricherTest.php index c89fd93..0a3c46b 100644 --- a/tests/Audit/CveEnricherTest.php +++ b/tests/Audit/CveEnricherTest.php @@ -10,6 +10,8 @@ use PHPUnit\Framework\TestCase; use RuntimeException; +use function array_map; + /** * Unit tests for {@see CveEnricher} GHSA to CVE enrichment of audit findings. * @@ -23,9 +25,9 @@ public function testEnricherCachesResolutionForRepeatedGhsa(): void $resolver ->expects(self::once()) - ->method('resolve') - ->with('GHSA-35jh-r3h4-6jhm') - ->willReturn(new CveResolution(['CVE-2021-23337'], CveStatus::RESOLVED)); + ->method('resolveMany') + ->with(['GHSA-35jh-r3h4-6jhm']) + ->willReturn(['GHSA-35jh-r3h4-6jhm' => new CveResolution(['CVE-2021-23337'], CveStatus::RESOLVED)]); $report = new AuditReport( 'npm', @@ -68,9 +70,9 @@ public function testEnricherDoesNotRequestExistingCves(): void $resolver ->expects(self::once()) - ->method('resolve') - ->with('GHSA-35jh-r3h4-6jhm') - ->willReturn(new CveResolution([], CveStatus::NONE_ASSIGNED)); + ->method('resolveMany') + ->with(['GHSA-35jh-r3h4-6jhm']) + ->willReturn(['GHSA-35jh-r3h4-6jhm' => new CveResolution([], CveStatus::NONE_ASSIGNED)]); $finding = new AuditFinding( 'lodash', @@ -111,13 +113,106 @@ public function testEnricherDoesNotRequestExistingCves(): void ); } + public function testEnricherIsolatesFailedLookupInMixedBatch(): void + { + $resolver = $this->createMock(CveResolverInterface::class); + + $resolver + ->expects(self::once()) + ->method('resolveMany') + ->with(['GHSA-35jh-r3h4-6jhm', 'GHSA-aaaa-bbbb-cccc']) + ->willReturn( + [ + 'GHSA-aaaa-bbbb-cccc' => new RuntimeException('not found'), + 'GHSA-35jh-r3h4-6jhm' => new CveResolution(['CVE-2021-23337'], CveStatus::RESOLVED), + ], + ); + + $warnings = []; + + $enriched = (new CveEnricher($resolver))->enrich( + new AuditReport( + 'npm', + [ + $this->finding('lodash', 'GHSA-35jh-r3h4-6jhm'), + $this->finding('example-package', 'GHSA-aaaa-bbbb-cccc'), + $this->finding('dependent-package', 'GHSA-35jh-r3h4-6jhm'), + ], + ), + static function (string $warning) use (&$warnings): void { + $warnings[] = $warning; + }, + ); + + self::assertSame( + ['lodash', 'example-package', 'dependent-package'], + array_map(static fn(AuditFinding $finding): string => $finding->package, $enriched->findings), + 'Findings must keep the report order.', + ); + self::assertSame( + [CveStatus::RESOLVED, CveStatus::UNAVAILABLE, CveStatus::RESOLVED], + array_map(static fn(AuditFinding $finding): CveStatus => $finding->cveStatus, $enriched->findings), + 'Only the failed advisory must be marked unavailable.', + ); + self::assertSame( + [['CVE-2021-23337'], [], ['CVE-2021-23337']], + array_map(static fn(AuditFinding $finding): array => $finding->cves, $enriched->findings), + 'Resolved CVE identifiers must reach both findings of the resolved advisory.', + ); + self::assertSame( + ['Unable to resolve CVE identifiers for GHSA-aaaa-bbbb-cccc: not found'], + $warnings, + 'Only the failed advisory must emit a warning.', + ); + } + + public function testEnricherMarksEveryAdvisoryUnavailableWhenBatchFails(): void + { + $resolver = $this->createMock(CveResolverInterface::class); + + $resolver + ->expects(self::once()) + ->method('resolveMany') + ->with(['GHSA-aaaa-bbbb-cccc', 'GHSA-35jh-r3h4-6jhm']) + ->willThrowException(new RuntimeException('network down')); + + $warnings = []; + + $enriched = (new CveEnricher($resolver))->enrich( + new AuditReport( + 'npm', + [ + $this->finding('example-package', 'GHSA-aaaa-bbbb-cccc'), + $this->finding('lodash', 'GHSA-35jh-r3h4-6jhm'), + ], + ), + static function (string $warning) use (&$warnings): void { + $warnings[] = $warning; + }, + ); + + self::assertSame( + [CveStatus::UNAVAILABLE, CveStatus::UNAVAILABLE], + array_map(static fn(AuditFinding $finding): CveStatus => $finding->cveStatus, $enriched->findings), + 'Every requested advisory must be marked unavailable.', + ); + self::assertSame( + [ + 'Unable to resolve CVE identifiers for GHSA-aaaa-bbbb-cccc: network down', + 'Unable to resolve CVE identifiers for GHSA-35jh-r3h4-6jhm: network down', + ], + $warnings, + 'One warning per requested advisory must be emitted in first-seen order.', + ); + } + public function testEnricherMarksNativeAdvisoryAsUnavailableWithoutNetworkRequest(): void { $resolver = $this->createMock(CveResolverInterface::class); $resolver ->expects(self::never()) - ->method('resolve'); + ->method('resolveMany'); $enriched = (new CveEnricher($resolver))->enrich( new AuditReport( @@ -153,8 +248,9 @@ public function testEnricherMarksSuccessfulResolutionWithoutCve(): void $resolver ->expects(self::once()) - ->method('resolve') - ->willReturn(new CveResolution([], CveStatus::NONE_ASSIGNED)); + ->method('resolveMany') + ->with(['GHSA-35jh-r3h4-6jhm']) + ->willReturn(['GHSA-35jh-r3h4-6jhm' => new CveResolution([], CveStatus::NONE_ASSIGNED)]); $enriched = (new CveEnricher($resolver))->enrich( new AuditReport('bun', [$this->finding('lodash', 'GHSA-35jh-r3h4-6jhm')]), @@ -180,11 +276,11 @@ public function testEnricherRejectsGhsaWithSurroundingText(string $advisoryId): $resolver ->expects(self::never()) - ->method('resolve'); + ->method('resolveMany'); $enriched = (new CveEnricher($resolver))->enrich( new AuditReport('npm', [$this->finding('example-package', $advisoryId)]), - static function (string $warning): void { + static function (string $warning): never { self::fail($warning); }, ); @@ -202,9 +298,9 @@ public function testEnricherSharesResolutionForGhsaIdsDifferingInCase(): void $resolver ->expects(self::once()) - ->method('resolve') - ->with('GHSA-35jh-r3h4-6jhm') - ->willReturn(new CveResolution(['CVE-2021-23337'], CveStatus::RESOLVED)); + ->method('resolveMany') + ->with(['GHSA-35jh-r3h4-6jhm']) + ->willReturn(['GHSA-35jh-r3h4-6jhm' => new CveResolution(['CVE-2021-23337'], CveStatus::RESOLVED)]); $report = new AuditReport( 'npm', @@ -236,14 +332,56 @@ public function testEnricherSharesResolutionForGhsaIdsDifferingInCase(): void ); } + public function testEnricherWarnsInFirstSeenOrderWhenSeveralLookupsFail(): void + { + $resolver = $this->createMock(CveResolverInterface::class); + + $resolver + ->expects(self::once()) + ->method('resolveMany') + ->with(['GHSA-aaaa-bbbb-cccc', 'GHSA-35jh-r3h4-6jhm']) + ->willReturn( + [ + 'GHSA-35jh-r3h4-6jhm' => new RuntimeException('rate limited'), + 'GHSA-aaaa-bbbb-cccc' => new RuntimeException('not found'), + ], + ); + + $warnings = []; + + (new CveEnricher($resolver))->enrich( + new AuditReport( + 'npm', + [ + $this->finding('example-package', 'GHSA-aaaa-bbbb-cccc'), + $this->finding('lodash', 'GHSA-35jh-r3h4-6jhm'), + ], + ), + static function (string $warning) use (&$warnings): void { + $warnings[] = $warning; + }, + ); + + self::assertSame( + [ + 'Unable to resolve CVE identifiers for GHSA-aaaa-bbbb-cccc: not found', + 'Unable to resolve CVE identifiers for GHSA-35jh-r3h4-6jhm: rate limited', + ], + $warnings, + 'Warnings must follow first-seen order, not result order.', + ); + } + public function testEnricherWarnsOnceAndPreservesFindingsWhenResolutionFails(): void { $resolver = $this->createMock(CveResolverInterface::class); $resolver ->expects(self::once()) - ->method('resolve') - ->willThrowException(new RuntimeException('rate limited')); + ->method('resolveMany') + ->with(['GHSA-35jh-r3h4-6jhm']) + ->willReturn(['GHSA-35jh-r3h4-6jhm' => new RuntimeException('rate limited')]); + $warnings = []; $report = new AuditReport( diff --git a/tests/Audit/GitHubAdvisoryCveResolverTest.php b/tests/Audit/GitHubAdvisoryCveResolverTest.php index c786642..aa71f26 100644 --- a/tests/Audit/GitHubAdvisoryCveResolverTest.php +++ b/tests/Audit/GitHubAdvisoryCveResolverTest.php @@ -4,17 +4,29 @@ namespace Foxy\Tests\Audit; +use Composer\Downloader\TransportException; use Composer\Util\Http\Response; use Composer\Util\HttpDownloader; -use Foxy\Audit\{CveStatus, GitHubAdvisoryCveResolver}; +use Foxy\Audit\{CveResolution, CveStatus, GitHubAdvisoryCveResolver}; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Tests\Provider\GitHubAdvisoryCveResolverProvider; use PHPUnit\Framework\Attributes\DataProviderExternal; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use React\Promise\PromiseInterface; +use Seld\JsonLint\ParsingException; +use Throwable; + +use function array_key_exists; +use function array_keys; +use function count; +use function React\Promise\reject; +use function React\Promise\resolve; +use function strlen; +use function substr; /** - * Unit tests for {@see GitHubAdvisoryCveResolver} GHSA validation and CVE resolution from GitHub advisories. + * Unit tests for {@see GitHubAdvisoryCveResolver} GHSA validation and concurrent CVE resolution from GitHub advisories. * * {@see GitHubAdvisoryCveResolverProvider} for test case data providers. */ @@ -29,14 +41,24 @@ final class GitHubAdvisoryCveResolverTest extends TestCase public function testResolverCollectsDeduplicatesAndSortsCves(): void { $resolver = new GitHubAdvisoryCveResolver( - $this->downloader( - self::fixture('github-advisory-with-cves.json'), - self::GHSA_WITH_CVES_ID, - ), + $this->downloader([self::GHSA_WITH_CVES_ID => self::fixture('github-advisory-with-cves.json')]), ); - $resolution = $resolver->resolve(' ghsa-AAaa-bBBb-CccC '); + $results = $resolver->resolveMany([' ghsa-AAaa-bBBb-CccC ']); + self::assertSame( + [self::GHSA_WITH_CVES_ID], + array_keys($results), + 'Result must be keyed by the canonical GHSA identifier.', + ); + + $resolution = $results[self::GHSA_WITH_CVES_ID]; + + self::assertInstanceOf( + CveResolution::class, + $resolution, + 'Valid document must produce a resolution.', + ); self::assertSame( CveStatus::RESOLVED, $resolution->status, @@ -52,14 +74,16 @@ public function testResolverCollectsDeduplicatesAndSortsCves(): void public function testResolverDistinguishesAdvisoryWithoutAssignedCve(): void { $resolver = new GitHubAdvisoryCveResolver( - $this->downloader( - self::fixture('github-advisory-without-cve.json'), - self::GHSA_WITHOUT_CVE_ID, - ), + $this->downloader([self::GHSA_WITHOUT_CVE_ID => self::fixture('github-advisory-without-cve.json')]), ); - $resolution = $resolver->resolve(self::GHSA_WITHOUT_CVE_ID); + $resolution = $resolver->resolveMany([self::GHSA_WITHOUT_CVE_ID])[self::GHSA_WITHOUT_CVE_ID]; + self::assertInstanceOf( + CveResolution::class, + $resolution, + 'Valid document must produce a resolution.', + ); self::assertSame( CveStatus::NONE_ASSIGNED, $resolution->status, @@ -72,29 +96,61 @@ public function testResolverDistinguishesAdvisoryWithoutAssignedCve(): void ); } + public function testResolverIsolatesTransportFailureInBatch(): void + { + $failure = new TransportException('Not Found', 404); + $resolver = new GitHubAdvisoryCveResolver( + $this->downloader( + [ + self::GHSA_ID => $failure, + self::GHSA_WITHOUT_CVE_ID => self::fixture('github-advisory-without-cve.json'), + ], + ), + ); + + $results = $resolver->resolveMany([self::GHSA_ID, self::GHSA_WITHOUT_CVE_ID]); + + self::assertSame( + $failure, + $results[self::GHSA_ID], + 'Rejected lookup must keep its own transport exception.', + ); + self::assertInstanceOf( + CveResolution::class, + $results[self::GHSA_WITHOUT_CVE_ID], + 'Sibling lookup must still resolve.', + ); + } + public function testResolverNormalizesAndFiltersCveIdentifiers(): void { $resolver = new GitHubAdvisoryCveResolver( $this->downloader( - <<<'JSON' - { - "ghsa_id": "GHSA-35JH-R3H4-6JHM", - "cve_id": " cve-2021-9999 ", - "identifiers": [ - {"type": "GHSA", "value": "CVE-2019-0001"}, - {"type": "CVE", "value": " CVE-2023-0002 "}, - {"type": "CVE", "value": "cve-2022-0001"}, - {"type": "CVE", "value": "prefixCVE-2020-0001"}, - {"type": "CVE", "value": "CVE-2020-0001suffix"} - ] - } - JSON, - self::GHSA_ID, + [ + self::GHSA_ID => <<<'JSON' + { + "ghsa_id": "GHSA-35JH-R3H4-6JHM", + "cve_id": " cve-2021-9999 ", + "identifiers": [ + {"type": "GHSA", "value": "CVE-2019-0001"}, + {"type": "CVE", "value": " CVE-2023-0002 "}, + {"type": "CVE", "value": "cve-2022-0001"}, + {"type": "CVE", "value": "prefixCVE-2020-0001"}, + {"type": "CVE", "value": "CVE-2020-0001suffix"} + ] + } + JSON, + ], ), ); - $resolution = $resolver->resolve(self::GHSA_ID); + $resolution = $resolver->resolveMany([self::GHSA_ID])[self::GHSA_ID]; + self::assertInstanceOf( + CveResolution::class, + $resolution, + 'Valid document must produce a resolution.', + ); self::assertSame( CveStatus::RESOLVED, $resolution->status, @@ -107,108 +163,206 @@ public function testResolverNormalizesAndFiltersCveIdentifiers(): void ); } - #[DataProviderExternal(GitHubAdvisoryCveResolverProvider::class, 'ghsaIdsWithSurroundingText')] - public function testResolverRejectsGhsaWithSurroundingTextBeforeRequest(string $ghsaId): void + public function testResolverReportsParsingExceptionForInvalidJsonBody(): void { - $downloader = $this->createMock(HttpDownloader::class); + $resolver = new GitHubAdvisoryCveResolver($this->downloader([self::GHSA_ID => '{'])); - $downloader - ->expects(self::never()) - ->method('get'); + self::assertInstanceOf( + ParsingException::class, + $resolver->resolveMany([self::GHSA_ID])[self::GHSA_ID], + 'Invalid JSON must be reported as that lookup\'s parsing exception.', + ); + } - $resolver = new GitHubAdvisoryCveResolver($downloader); + #[DataProviderExternal(GitHubAdvisoryCveResolverProvider::class, 'invalidIdentifiers')] + public function testResolverReportsRuntimeExceptionForInvalidIdentifiers(string $identifiers): void + { + $resolver = new GitHubAdvisoryCveResolver( + $this->downloader( + [self::GHSA_ID => '{"ghsa_id":"GHSA-35jh-r3h4-6jhm","identifiers":' . $identifiers . '}'], + ), + ); - $this->expectException(RuntimeException::class); + $result = $resolver->resolveMany([self::GHSA_ID])[self::GHSA_ID]; - $resolver->resolve($ghsaId); + self::assertInstanceOf( + RuntimeException::class, + $result, + 'Malformed identifiers must be reported as a Foxy runtime exception.', + ); + self::assertSame( + Message::AUDIT_GITHUB_IDENTIFIERS_INVALID->getMessage(self::GHSA_ID), + $result->getMessage(), + 'Exception message must name the advisory.', + ); } - public function testResolverRejectsInvalidGhsaBeforeRequest(): void + public function testResolverReportsRuntimeExceptionForMismatchedAdvisoryDocument(): void { - $downloader = $this->createMock(HttpDownloader::class); - - $downloader - ->expects(self::never()) - ->method('get'); + $resolver = new GitHubAdvisoryCveResolver( + $this->downloader([self::GHSA_ID => '{"ghsa_id":"GHSA-aaaa-bbbb-cccc","identifiers":[]}']), + ); - $resolver = new GitHubAdvisoryCveResolver($downloader); + $result = $resolver->resolveMany([self::GHSA_ID])[self::GHSA_ID]; - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage( - Message::AUDIT_GHSA_ID_INVALID->getMessage('not-a-ghsa'), + self::assertInstanceOf( + RuntimeException::class, + $result, + 'Mismatched document must be reported as a Foxy runtime exception.', + ); + self::assertSame( + Message::AUDIT_GITHUB_ADVISORY_MISMATCHED->getMessage(self::GHSA_ID), + $result->getMessage(), + 'Exception message must name the requested advisory.', ); + } + + public function testResolverReportsRuntimeExceptionForNonObjectAdvisoryDocument(): void + { + $resolver = new GitHubAdvisoryCveResolver($this->downloader([self::GHSA_ID => 'null'])); - $resolver->resolve('not-a-ghsa'); + $result = $resolver->resolveMany([self::GHSA_ID])[self::GHSA_ID]; + + self::assertInstanceOf( + RuntimeException::class, + $result, + 'Non-object document must be reported as a Foxy runtime exception.', + ); + self::assertSame( + Message::AUDIT_GITHUB_ADVISORY_INVALID->getMessage(self::GHSA_ID), + $result->getMessage(), + 'Exception message must name the advisory.', + ); } - #[DataProviderExternal(GitHubAdvisoryCveResolverProvider::class, 'invalidIdentifiers')] - public function testResolverRejectsInvalidIdentifiers(string $identifiers): void + public function testResolverRequestsEveryAdvisoryBeforeWaitingOnce(): void { $resolver = new GitHubAdvisoryCveResolver( $this->downloader( - '{"ghsa_id":"GHSA-35jh-r3h4-6jhm","identifiers":' . $identifiers . '}', - self::GHSA_ID, + [ + self::GHSA_WITH_CVES_ID => self::fixture('github-advisory-with-cves.json'), + self::GHSA_WITHOUT_CVE_ID => self::fixture('github-advisory-without-cve.json'), + ], ), ); - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage( - Message::AUDIT_GITHUB_IDENTIFIERS_INVALID->getMessage('GHSA-35jh-r3h4-6jhm'), + $results = $resolver->resolveMany([self::GHSA_WITH_CVES_ID, self::GHSA_WITHOUT_CVE_ID]); + + self::assertSame( + [self::GHSA_WITH_CVES_ID, self::GHSA_WITHOUT_CVE_ID], + array_keys($results), + 'Every requested advisory must have a result.', ); - $resolver->resolve(self::GHSA_ID); + $first = $results[self::GHSA_WITH_CVES_ID]; + $second = $results[self::GHSA_WITHOUT_CVE_ID]; + + self::assertInstanceOf( + CveResolution::class, + $first, + 'First advisory must resolve.', + ); + self::assertInstanceOf( + CveResolution::class, + $second, + 'Second advisory must resolve.', + ); + self::assertSame( + ['CVE-2020-8203', 'CVE-2021-23337'], + $first->cves, + 'First advisory must carry its own CVE identifiers.', + ); + self::assertSame( + CveStatus::NONE_ASSIGNED, + $second->status, + 'Second advisory must carry its own status.', + ); } - public function testResolverRejectsMismatchedAdvisoryDocument(): void + #[DataProviderExternal(GitHubAdvisoryCveResolverProvider::class, 'ghsaIdsWithSurroundingText')] + public function testThrowRuntimeExceptionForGhsaWithSurroundingTextBeforeRequest(string $ghsaId): void { - $resolver = new GitHubAdvisoryCveResolver( - $this->downloader( - '{"ghsa_id":"GHSA-aaaa-bbbb-cccc","identifiers":[]}', - self::GHSA_ID, - ), - ); + $resolver = new GitHubAdvisoryCveResolver($this->silentDownloader()); $this->expectException(RuntimeException::class); $this->expectExceptionMessage( - Message::AUDIT_GITHUB_ADVISORY_MISMATCHED->getMessage('GHSA-35jh-r3h4-6jhm'), + Message::AUDIT_GHSA_ID_INVALID->getMessage($ghsaId), ); - $resolver->resolve(self::GHSA_ID); + $resolver->resolveMany([$ghsaId]); } - public function testResolverRejectsNonObjectAdvisoryDocument(): void + public function testThrowRuntimeExceptionForInvalidGhsaBeforeRequest(): void { - $resolver = new GitHubAdvisoryCveResolver( - $this->downloader('null', self::GHSA_ID), - ); + $resolver = new GitHubAdvisoryCveResolver($this->silentDownloader()); $this->expectException(RuntimeException::class); $this->expectExceptionMessage( - Message::AUDIT_GITHUB_ADVISORY_INVALID->getMessage('GHSA-35jh-r3h4-6jhm'), + Message::AUDIT_GHSA_ID_INVALID->getMessage('not-a-ghsa'), ); - $resolver->resolve(self::GHSA_ID); + $resolver->resolveMany([self::GHSA_ID, 'not-a-ghsa']); } - private function downloader(string $body, string $ghsaId): HttpDownloader&MockObject + /** + * Returns a downloader that answers each advisory URL once, then expects a single `wait()`. + * + * @param array $answers Response bodies or transport failures keyed by GHSA identifier. + */ + private function downloader(array $answers): HttpDownloader&MockObject { - $url = 'https://api.github.com/advisories/' . $ghsaId; $downloader = $this->createMock(HttpDownloader::class); + $downloader - ->expects(self::once()) - ->method('get') - ->with( - $url, - [ - 'http' => [ - 'header' => [ - 'Accept: application/vnd.github+json', - 'X-GitHub-Api-Version: 2022-11-28', + ->expects(self::exactly(count($answers))) + ->method('add') + ->willReturnCallback( + static function (string $url, array $options) use (&$answers): PromiseInterface { + self::assertSame( + [ + 'http' => [ + 'header' => [ + 'Accept: application/vnd.github+json', + 'X-GitHub-Api-Version: 2022-11-28', + ], + ], ], - ], - ], - ) - ->willReturn(new Response(['url' => $url], 200, [], $body)); + $options, + 'Request must ask for the versioned GitHub JSON media type.', + ); + + $ghsaId = substr($url, strlen('https://api.github.com/advisories/')); + + self::assertTrue( + array_key_exists($ghsaId, $answers), + 'Request must target an expected advisory URL once.', + ); + + $answer = $answers[$ghsaId]; + unset($answers[$ghsaId]); + + return $answer instanceof Throwable + ? reject($answer) + : resolve(new Response(['url' => $url], 200, [], $answer)); + }, + ); + $downloader + ->expects(self::once()) + ->method('wait'); + + return $downloader; + } + + private function silentDownloader(): HttpDownloader&MockObject + { + $downloader = $this->createMock(HttpDownloader::class); + + $downloader + ->expects(self::never()) + ->method('add'); + $downloader + ->expects(self::never()) + ->method('wait'); return $downloader; } diff --git a/tests/Command/AuditCommandTest.php b/tests/Command/AuditCommandTest.php index 13cab13..5a4076d 100644 --- a/tests/Command/AuditCommandTest.php +++ b/tests/Command/AuditCommandTest.php @@ -173,9 +173,9 @@ public function testCveResolutionFailureWritesASanitizedWarningAndPreservesTheRe $resolver ->expects(self::once()) - ->method('resolve') - ->with('GHSA-aaaa-bbbb-cccc') - ->willThrowException(new RuntimeException("rate\nlimited\0now")); + ->method('resolveMany') + ->with(['GHSA-aaaa-bbbb-cccc']) + ->willReturn(['GHSA-aaaa-bbbb-cccc' => new RuntimeException("rate\nlimited\0now")]); $io = $this->createMock(IOInterface::class); @@ -308,9 +308,9 @@ public function testJsonKeepsDiagnosticsOnComposerErrorOutput(): void $resolver ->expects(self::once()) - ->method('resolve') - ->with('GHSA-aaaa-bbbb-cccc') - ->willReturn(new CveResolution(['CVE-2026-12345'], CveStatus::RESOLVED)); + ->method('resolveMany') + ->with(['GHSA-aaaa-bbbb-cccc']) + ->willReturn(['GHSA-aaaa-bbbb-cccc' => new CveResolution(['CVE-2026-12345'], CveStatus::RESOLVED)]); $io = $this->createMock(IOInterface::class); @@ -377,7 +377,7 @@ public function testNoCveSkipsResolverAndPreservesMachineReadableOutput(): void $resolver ->expects(self::never()) - ->method('resolve'); + ->method('resolveMany'); $tester = $this->createTester($runner, $resolver); diff --git a/tests/Command/FoxyCapabilityTest.php b/tests/Command/FoxyCapabilityTest.php index d7534c5..74531e5 100644 --- a/tests/Command/FoxyCapabilityTest.php +++ b/tests/Command/FoxyCapabilityTest.php @@ -7,7 +7,8 @@ use Composer\Plugin\Capability\CommandProvider as ComposerCommandProvider; use Composer\Plugin\Capable; use Foxy\Asset\AssetManagerInterface; -use Foxy\Audit\{AuditProcessResult, AuditRequest, AuditRunnerInterface, AuditableAssetManagerInterface, Severity}; +use Foxy\Audit\{AuditProcessResult, AuditRequest, AuditRunnerInterface, Severity}; +use Foxy\Audit\Parser\NpmAuditParser; use Foxy\Command\FoxyCommandProvider; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; @@ -17,11 +18,9 @@ final class FoxyCapabilityTest extends TestCase { - public function testAuditDelegatesToSelectedAuditableManager(): void + public function testAuditDelegatesToSelectedManager(): void { - $manager = $this->createMockForIntersectionOfInterfaces( - [AssetManagerInterface::class, AuditableAssetManagerInterface::class], - ); + $manager = $this->createMock(AssetManagerInterface::class); $manager ->expects(self::once()) @@ -38,6 +37,10 @@ public function testAuditDelegatesToSelectedAuditableManager(): void ' manager diagnostic ', ), ); + $manager + ->expects(self::once()) + ->method('getAuditParser') + ->willReturn(new NpmAuditParser()); $foxy = new Foxy(); @@ -63,50 +66,42 @@ public function testAuditDelegatesToSelectedAuditableManager(): void ); } - public function testAuditRejectsDisabledPlugin(): void + public function testDeclaresCommandProviderCapability(): void { $foxy = new Foxy(); - self::setProperty($foxy, 'config', new Config([], ['enabled' => false])); - - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage( - Message::FOXY_AUDIT_DISABLED->getMessage(), + self::assertInstanceOf( + Capable::class, + $foxy, + 'Plugin must expose Composer capability support.', ); - - $foxy->audit(new AuditRequest()); - } - - public function testAuditRejectsManagerWithoutAuditCapability(): void - { - $foxy = new Foxy(); - - self::setProperty($foxy, 'config', new Config([], ['enabled' => true])); - self::setProperty($foxy, 'assetManager', $this->createMock(AssetManagerInterface::class)); - - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage( - Message::FOXY_AUDIT_UNSUPPORTED_MANAGER->getMessage(), + self::assertInstanceOf( + AuditRunnerInterface::class, + $foxy, + 'Plugin must implement the audit runner contract.', + ); + self::assertSame( + [ComposerCommandProvider::class => FoxyCommandProvider::class], + $foxy->getCapabilities(), + 'Command capability must map to its provider.', ); - - $foxy->audit(new AuditRequest()); } - public function testAuditRejectsMissingAssetManager(): void + public function testThrowRuntimeExceptionWhenPluginIsDisabled(): void { $foxy = new Foxy(); - self::setProperty($foxy, 'config', new Config([], ['enabled' => true])); + self::setProperty($foxy, 'config', new Config([], ['enabled' => false])); $this->expectException(RuntimeException::class); $this->expectExceptionMessage( - Message::FOXY_AUDIT_UNSUPPORTED_MANAGER->getMessage(), + Message::FOXY_AUDIT_DISABLED->getMessage(), ); $foxy->audit(new AuditRequest()); } - public function testAuditRejectsUnactivatedPlugin(): void + public function testThrowRuntimeExceptionWhenPluginIsNotActivated(): void { $this->expectException(RuntimeException::class); $this->expectExceptionMessage( @@ -116,27 +111,6 @@ public function testAuditRejectsUnactivatedPlugin(): void (new Foxy())->audit(new AuditRequest()); } - public function testDeclaresCommandProviderCapability(): void - { - $foxy = new Foxy(); - - self::assertInstanceOf( - Capable::class, - $foxy, - 'Plugin must expose Composer capability support.', - ); - self::assertInstanceOf( - AuditRunnerInterface::class, - $foxy, - 'Plugin must implement the audit runner contract.', - ); - self::assertSame( - [ComposerCommandProvider::class => FoxyCommandProvider::class], - $foxy->getCapabilities(), - 'Command capability must map to its provider.', - ); - } - private static function setProperty(Foxy $foxy, string $property, object $value): void { (new ReflectionClass($foxy))->getProperty($property)->setValue($foxy, $value); diff --git a/tests/Command/FoxyCommandProviderTest.php b/tests/Command/FoxyCommandProviderTest.php index da15305..ee9d92a 100644 --- a/tests/Command/FoxyCommandProviderTest.php +++ b/tests/Command/FoxyCommandProviderTest.php @@ -59,7 +59,7 @@ public function testProvidesAuditCommandWithActiveComposerAndIo(): void } #[DataProviderExternal(FoxyCommandProviderProvider::class, 'invalidArguments')] - public function testRejectsInvalidCapabilityArgument(string $argument): void + public function testThrowRuntimeExceptionForInvalidCapabilityArgument(string $argument): void { $arguments = [ 'composer' => $this->createMock(Composer::class), @@ -77,7 +77,7 @@ public function testRejectsInvalidCapabilityArgument(string $argument): void new FoxyCommandProvider($arguments); } - public function testRejectsMissingCapabilityArguments(): void + public function testThrowRuntimeExceptionWhenCapabilityArgumentsAreMissing(): void { $this->expectException(RuntimeException::class); $this->expectExceptionMessage( diff --git a/tests/Fallback/ComposerFallbackTest.php b/tests/Fallback/ComposerFallbackTest.php index 3ca2180..6b5e06e 100644 --- a/tests/Fallback/ComposerFallbackTest.php +++ b/tests/Fallback/ComposerFallbackTest.php @@ -4,25 +4,30 @@ namespace Foxy\Tests\Fallback; +use Composer\Autoload\AutoloadGenerator; use Composer\Composer; +use Composer\Downloader\DownloadManager; +use Composer\EventDispatcher\{Event, EventDispatcher}; use Composer\Filter\PlatformRequirementFilter\PlatformRequirementFilterInterface; use Composer\Installer; use Composer\Installer\InstallationManager; use Composer\IO\IOInterface; -use Composer\Package\PackageInterface; +use Composer\Json\JsonFile; +use Composer\Package\{Locker, PackageInterface, RootPackage}; use Composer\Repository\RepositoryManager; +use Composer\Script\ScriptEvents; use Composer\Util\Filesystem; use Exception; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Fallback\ComposerFallback; use Foxy\Tests\Provider\ComposerFallbackProvider; -use Foxy\Util\LockerUtil; use JsonException; use PHPUnit\Framework\Attributes\DataProviderExternal; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use ReflectionClass; +use ReflectionMethod; use Symfony\Component\Console\Input\InputInterface; use function chdir; @@ -50,6 +55,91 @@ final class ComposerFallbackTest extends TestCase private string|null $oldCwd = ''; private \Symfony\Component\Filesystem\Filesystem|null $sfs = null; + public function testCreatesRollbackInstallerOverPrivateScriptFreeDispatcher(): void + { + $sharedDispatcher = $this->createMock(EventDispatcher::class); + + $sharedDispatcher + ->expects(self::never()) + ->method('setRunScripts'); + + $sharedAutoloadGenerator = $this->createMock(AutoloadGenerator::class); + $locker = $this->createMock(Locker::class); + + $this->composer + ->method('getConfig') + ->willReturn(new \Composer\Config(false)); + $this->composer + ->method('getPackage') + ->willReturn(new RootPackage('foo/root', '1.0.0.0', '1.0.0')); + $this->composer + ->method('getDownloadManager') + ->willReturn($this->createMock(DownloadManager::class)); + $this->composer + ->method('getRepositoryManager') + ->willReturn($this->createMock(RepositoryManager::class)); + $this->composer + ->method('getLocker') + ->willReturn($locker); + $this->composer + ->method('getInstallationManager') + ->willReturn($this->createMock(InstallationManager::class)); + $this->composer + ->method('getEventDispatcher') + ->willReturn($sharedDispatcher); + $this->composer + ->method('getAutoloadGenerator') + ->willReturn($sharedAutoloadGenerator); + + $composerFallback = new ComposerFallback($this->composer, $this->io, $this->config, $this->input); + + $installer = (new ReflectionMethod($composerFallback, 'getInstaller'))->invoke($composerFallback); + + self::assertInstanceOf( + Installer::class, + $installer, + 'A real installer must be built when none is injected.', + ); + + $installerReflection = new ReflectionClass(Installer::class); + $dispatcher = $installerReflection->getProperty('eventDispatcher')->getValue($installer); + $autoloadGenerator = $installerReflection->getProperty('autoloadGenerator')->getValue($installer); + + self::assertInstanceOf( + EventDispatcher::class, + $dispatcher, + 'Installer must carry an event dispatcher.', + ); + self::assertNotSame( + $sharedDispatcher, + $dispatcher, + 'Event dispatcher must be private, not the shared one.', + ); + self::assertFalse( + (new ReflectionClass(EventDispatcher::class))->getProperty('runScripts')->getValue($dispatcher), + 'Private dispatcher must have scripts disabled.', + ); + self::assertFalse( + $dispatcher->hasEventListeners(new Event(ScriptEvents::POST_UPDATE_CMD)), + 'Private dispatcher must have no listeners.', + ); + self::assertNotSame( + $sharedAutoloadGenerator, + $autoloadGenerator, + 'Autoload generator must be private, not the shared one.', + ); + self::assertSame( + $dispatcher, + (new ReflectionClass(AutoloadGenerator::class))->getProperty('eventDispatcher')->getValue($autoloadGenerator), + 'Autoload generator must dispatch on the private dispatcher.', + ); + self::assertSame( + $locker, + $installerReflection->getProperty('locker')->getValue($installer), + 'Locker must be the shared Composer locker.', + ); + } + public function testFailedSubsequentSaveInvalidatesPreviousSnapshot(): void { file_put_contents($this->cwd . '/composer.json', '{}'); @@ -79,12 +169,6 @@ static function () use (&$configCalls, $composerConfig): \Composer\Config { }, ); - $installationManager = $this->createMock(InstallationManager::class); - - $this->composer - ->expects(self::once()) - ->method('getInstallationManager') - ->willReturn($installationManager); $this->composer ->expects(self::never()) ->method('getLocker'); @@ -221,9 +305,6 @@ public function testRestoreBeforeSaveDoesNothing(): void $this->fs ->expects(self::never()) ->method('remove'); - $this->installer - ->expects(self::never()) - ->method('setRunScripts'); $this->installer ->expects(self::never()) ->method('run'); @@ -408,9 +489,6 @@ function (string $path): bool { return true; } ); - $this->installer - ->expects(self::never()) - ->method('setRunScripts'); $this->installer ->expects(self::never()) ->method('run'); @@ -522,9 +600,6 @@ function (string $path) use (&$removed): bool { return true; } ); - $this->installer - ->expects(self::never()) - ->method('setRunScripts'); $this->installer ->expects(self::never()) ->method('run'); @@ -563,9 +638,6 @@ public function testRestoreThrowsWhenCreatedLockCannotBeRemoved(): void ->method('remove') ->with('./composer.lock') ->willReturn(false); - $this->installer - ->expects(self::never()) - ->method('setRunScripts'); $this->installer ->expects(self::never()) ->method('run'); @@ -593,9 +665,6 @@ public function testRestoreThrowsWhenCreatedVendorDirectoryCannotBeRemoved(): vo ->method('remove') ->with($vendorDir) ->willReturn(false); - $this->installer - ->expects(self::never()) - ->method('setRunScripts'); $this->installer ->expects(self::never()) ->method('run'); @@ -792,9 +861,6 @@ public function testRestoreWrapsRemoveException(): void ->method('remove') ->with('./composer.lock') ->willThrowException($failure); - $this->installer - ->expects(self::never()) - ->method('setRunScripts'); $this->installer ->expects(self::never()) ->method('run'); @@ -836,13 +902,6 @@ public function testSave(bool $withLockFile): void ->method('getRepositoryManager') ->willReturn($rm); - $im = $this->createMock(InstallationManager::class); - - $this->composer - ->expects(self::any()) - ->method('getInstallationManager') - ->willReturn($im); - $config = $this->createMock(\Composer\Config::class); $config @@ -952,9 +1011,6 @@ public function testSaveWithDisabledOptionDoesNotReadComposerState(): void $this->composer ->expects(self::never()) ->method('getConfig'); - $this->composer - ->expects(self::never()) - ->method('getInstallationManager'); self::assertSame( $composerFallback, @@ -977,7 +1033,7 @@ protected function setUp(): void $this->installer = $this ->getMockBuilder(Installer::class) ->disableOriginalConstructor() - ->onlyMethods(['run', 'setRunScripts']) + ->onlyMethods(['run']) ->getMock(); $this->sfs = new \Symfony\Component\Filesystem\Filesystem(); $this->sfs->mkdir($this->cwd); @@ -1015,11 +1071,6 @@ protected function tearDown(): void private function expectInstallerRun(int $result = 0, int $times = 1): void { - $this->installer - ->expects(self::exactly($times)) - ->method('setRunScripts') - ->with(false) - ->willReturnSelf(); $this->installer ->expects(self::exactly($times)) ->method('run') @@ -1037,13 +1088,6 @@ private function setupNoLockEnvironment(int $restoreCount = 1): string file_put_contents($this->cwd . '/composer.json', '{}'); - $installationManager = $this->createMock(InstallationManager::class); - - $this->composer - ->expects(self::once()) - ->method('getInstallationManager') - ->willReturn($installationManager); - $config = $this->createMock(\Composer\Config::class); $config @@ -1108,17 +1152,16 @@ private function setupRestoreEnvironment( ->method('getRepositoryManager') ->willReturn($repositoryManager); - $installationManager = $this->createMock(InstallationManager::class); - - $this->composer - ->expects(self::any()) - ->method('getInstallationManager') - ->willReturn($installationManager); $this->io ->expects(self::exactly($restoreCount)) ->method('write'); - $locker = LockerUtil::getLocker($this->io, $installationManager, $composerFile); + $locker = new Locker( + $this->io, + new JsonFile($lockFile, null, $this->io), + $this->createMock(InstallationManager::class), + $composerContent, + ); $this->composer ->expects(self::atLeastOnce()) diff --git a/tests/Fixtures/Asset/InspectableAssetManager.php b/tests/Fixtures/Asset/InspectableAssetManager.php index 82f053e..24787d2 100644 --- a/tests/Fixtures/Asset/InspectableAssetManager.php +++ b/tests/Fixtures/Asset/InspectableAssetManager.php @@ -4,9 +4,11 @@ namespace Foxy\Tests\Fixtures\Asset; -use Foxy\Asset\{AbstractAuditableAssetManager, AssetPackageInterface}; +use Foxy\Asset\{AbstractAssetManager, AssetPackageInterface}; +use Foxy\Audit\AuditParserInterface; +use Foxy\Audit\Parser\NpmAuditParser; -final class InspectableAssetManager extends AbstractAuditableAssetManager +final class InspectableAssetManager extends AbstractAssetManager { /** * @var array|null @@ -46,6 +48,11 @@ public function getAuditCommandForTest(bool $noDev = false): string return $this->getAuditCommand($noDev); } + public function getAuditParser(): AuditParserInterface + { + return new NpmAuditParser(); + } + public function getAuditValidationForTest(): bool|null { return $this->validatedNoDev; diff --git a/tests/Fixtures/Asset/StubAssetManager.php b/tests/Fixtures/Asset/StubAssetManager.php index a2f0155..12e7060 100644 --- a/tests/Fixtures/Asset/StubAssetManager.php +++ b/tests/Fixtures/Asset/StubAssetManager.php @@ -8,6 +8,7 @@ use Composer\Package\RootPackageInterface; use Composer\Util\{Filesystem, ProcessExecutor}; use Foxy\Asset\{AssetManagerInterface, AssetPackageInterface}; +use Foxy\Audit\{AuditParserInterface, AuditProcessResult}; use Foxy\Config\Config; use Foxy\Exception\RuntimeException; use Foxy\Fallback\FallbackInterface; @@ -38,6 +39,16 @@ public function addDependencies(RootPackageInterface $rootPackage, array $depend throw new RuntimeException('Not used in this test.'); } + public function audit(bool $noDev): AuditProcessResult + { + throw new RuntimeException('Not used in this test.'); + } + + public function getAuditParser(): AuditParserInterface + { + throw new RuntimeException('Not used in this test.'); + } + public function getLockPackageName(): string { return 'stub-lock.json'; diff --git a/tests/Fixtures/Native/InMemoryRegistry.php b/tests/Fixtures/Native/InMemoryRegistry.php index dadce40..3931d80 100644 --- a/tests/Fixtures/Native/InMemoryRegistry.php +++ b/tests/Fixtures/Native/InMemoryRegistry.php @@ -7,24 +7,42 @@ use Foxy\Exception\{Message, RuntimeException}; use Foxy\Native\{NpmRegistry, NpmRegistryInterface, PackageMetadata, ResolvedPackage}; +use function count; use function file_put_contents; use function str_replace; /** - * Provides an {@see NpmRegistryInterface} backed by in-memory metadata and tarball bytes that records every request. + * Provides an {@see NpmRegistryInterface} backed by in-memory metadata, tarball bytes, and an advisories document that + * records every request. */ final class InMemoryRegistry implements NpmRegistryInterface { + /** + * @var list>> Versions passed to {@see getAdvisories()}, in request order. + */ + public array $advisoryRequests = []; + /** * @var list Tarball URLs passed to {@see fetchTarball()}, in request order. */ public array $fetched = []; + /** + * @var list, fetchedBefore: int}> Package names passed to {@see prefetchTarballs()} per + * call, with the number of {@see fetchTarball()} calls made before it. + */ + public array $prefetched = []; + /** * @var list Names passed to {@see getMetadata()}, in request order. */ public array $requested = []; + /** + * Advisories document returned by {@see getAdvisories()}. + */ + private string $advisories = '{}'; + /** * @var array Tarball bytes keyed by tarball URL. */ @@ -45,6 +63,16 @@ public function add(PackageMetadata $metadata): self return $this; } + /** + * Registers the advisories document returned for every advisories request. + */ + public function addAdvisories(string $document): self + { + $this->advisories = $document; + + return $this; + } + /** * Registers the bytes served for a tarball URL, replacing any previous entry with the same URL. */ @@ -75,6 +103,13 @@ public function fetchTarball(ResolvedPackage $package, string $directory): strin return $file; } + public function getAdvisories(array $versions): string + { + $this->advisoryRequests[] = $versions; + + return $this->advisories; + } + public function getMetadata(string $name): PackageMetadata { $this->requested[] = $name; @@ -82,4 +117,18 @@ public function getMetadata(string $name): PackageMetadata return $this->packages[$name] ?? throw new RuntimeException(Message::NATIVE_PACKAGE_NOT_FOUND->getMessage($name, 'memory')); } + + /** + * Records the package names and the number of tarballs fetched so far, without side effects. + */ + public function prefetchTarballs(array $packages): void + { + $names = []; + + foreach ($packages as $package) { + $names[] = $package->name; + } + + $this->prefetched[] = ['names' => $names, 'fetchedBefore' => count($this->fetched)]; + } } diff --git a/tests/FoxyTest.php b/tests/FoxyTest.php index c073be1..b2a87a8 100644 --- a/tests/FoxyTest.php +++ b/tests/FoxyTest.php @@ -6,7 +6,7 @@ use Composer\{Cache, Composer, Config}; use Composer\DependencyResolver\Operation\{InstallOperation, OperationInterface}; -use Composer\Installer\{InstallationManager, PackageEvent, PackageEvents}; +use Composer\Installer\{InstallationManager, InstallerEvents, PackageEvent, PackageEvents}; use Composer\IO\IOInterface; use Composer\Package\{Package, RootPackageInterface}; use Composer\Repository\RepositoryManager; @@ -21,7 +21,6 @@ use Foxy\Solver\SolverInterface; use Foxy\Tests\Fixtures\Asset\StubAssetManager; use Foxy\Tests\Provider\FoxyProvider; -use Foxy\Util\ComposerUtil; use PHPUnit\Framework\Attributes\DataProviderExternal; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -567,7 +566,7 @@ public function testGetSubscribedEvents(): void { self::assertSame( [ - ComposerUtil::getInitEventName() => [['init', 100]], + InstallerEvents::PRE_OPERATIONS_EXEC => [['init', 100]], PackageEvents::POST_PACKAGE_INSTALL => [['initOnInstall', 100]], ScriptEvents::POST_INSTALL_CMD => [['solveAssets', 100]], ScriptEvents::POST_UPDATE_CMD => [['solveAssets', 100]], diff --git a/tests/Json/JsonFileTest.php b/tests/Json/JsonFileTest.php index a83e544..08bc69d 100644 --- a/tests/Json/JsonFileTest.php +++ b/tests/Json/JsonFileTest.php @@ -134,41 +134,6 @@ public function testGetArrayKeysWithoutFile(): void ); } - public function testGetIndentWithExistingFile(): void - { - $content = self::fixture('name-two-space.json'); - - $filename = './package.json'; - - file_put_contents($filename, $content); - - self::assertFileExists( - $filename, - 'The package JSON file should exist before attempting to read it.', - ); - - $jsonFile = new JsonFile($filename); - - self::assertSame( - 2, - $jsonFile->getIndent(), - 'The indent should match the expected value.', - ); - } - - public function testGetIndentWithoutFile(): void - { - $filename = './package.json'; - - $jsonFile = new JsonFile($filename); - - self::assertSame( - 4, - $jsonFile->getIndent(), - 'The indent should default to four spaces when the file does not exist.', - ); - } - /** * @throws Exception|ParsingException */ diff --git a/tests/Json/JsonFormatterTest.php b/tests/Json/JsonFormatterTest.php index d299495..e6220a2 100644 --- a/tests/Json/JsonFormatterTest.php +++ b/tests/Json/JsonFormatterTest.php @@ -69,28 +69,6 @@ public function testGetArrayKeysWithoutSpacesBeforeArray(): void ); } - public function testGetIndent(): void - { - $content = self::fixture('package-two-space.json'); - - self::assertSame( - 2, - JsonFormatter::getIndent($content), - 'The indent extracted from the JSON content should match the expected indent.', - ); - } - - public function testGetIndentIgnoresSurroundingWhitespace(): void - { - $content = "\n " . self::fixture('name-two-space.json'); - - self::assertSame( - 2, - JsonFormatter::getIndent($content), - 'The indent extracted from the JSON content should match the expected indent.', - ); - } - public function testGetMapKeys(): void { self::assertSame( diff --git a/tests/Native/DependencyResolverTest.php b/tests/Native/DependencyResolverTest.php index 539f92d..d914ab5 100644 --- a/tests/Native/DependencyResolverTest.php +++ b/tests/Native/DependencyResolverTest.php @@ -231,6 +231,32 @@ static function (string $message) use (&$warnings): void { ); } + public function testResolveDropsOptionalSubtreesThroughSelfReferencingConstraint(): void + { + $registry = (new InMemoryRegistry()) + ->add(self::metadata('z', ['1.0.0' => ['dependencies' => ['bar' => '*', 'x' => '*']]])) + ->add(self::metadata('bar', ['1.0.0' => ['dependencies' => ['z' => '*']]])) + ->add(self::metadata('x', ['1.0.0' => ['dependencies' => ['x2' => '*']]])) + ->add(self::metadata('x2', ['1.0.0' => ['dependencies' => ['y' => '^1.0']]])) + ->add(self::metadata('aa', ['1.0.0' => ['dependencies' => ['k' => '*']]])) + ->add(self::metadata('k', ['1.0.0' => ['dependencies' => ['z' => '*']]])) + ->add(self::metadata('left', ['1.0.0' => []])); + + $io = $this->createMock(IOInterface::class); + + $io + ->expects(self::exactly(2)) + ->method('writeError'); + + self::assertEquals( + ['left' => self::package('left', '1.0.0')], + (new DependencyResolver($registry, $io))->resolve( + [new Requirement('z', '*', '', true), new Requirement('aa', '*', '', true), new Requirement('left', '*', '')], + ), + 'The subtree requiring z from outside must go too.', + ); + } + public function testResolveDropsOptionalSubtreeWithUnavailableDependency(): void { $registry = (new InMemoryRegistry()) @@ -913,6 +939,23 @@ public function testThrowRuntimeExceptionWhenDroppedOptionalRootIsRequiredAgain( $resolver->resolve([new Requirement('foo', '^1.0', '', true), new Requirement('m', '*', '')]); } + public function testThrowRuntimeExceptionWhenMandatoryConstraintsConflictBesideOptionalOne(): void + { + $registry = (new InMemoryRegistry()) + ->add(self::metadata('o', ['1.0.0' => ['optionalDependencies' => ['x' => '>=1.5']]])) + ->add(self::metadata('m', ['1.0.0' => ['dependencies' => ['x' => '^2.0']]])) + ->add(self::metadata('x', ['1.0.0' => [], '1.5.0' => [], '2.0.0' => []])); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + Message::NATIVE_VERSION_CONFLICT->getMessage('x', '"^1.0" from root, "^2.0" from m@1.0.0'), + ); + + $this->resolver($registry)->resolve( + [new Requirement('x', '^1.0', ''), new Requirement('o', '*', ''), new Requirement('m', '*', '')], + ); + } + public function testThrowRuntimeExceptionWhenMandatoryConstraintsConflictWithoutOptionalOnes(): void { $registry = (new InMemoryRegistry()) diff --git a/tests/Native/NativeInstallerTest.php b/tests/Native/NativeInstallerTest.php index eb1f280..a8ab90b 100644 --- a/tests/Native/NativeInstallerTest.php +++ b/tests/Native/NativeInstallerTest.php @@ -446,6 +446,47 @@ public function testInstallOrdersLocalPackagesAmongRegistryPackages(): void ); } + public function testInstallPrefetchesRegistryPackagesOnceBeforeFetching(): void + { + $registry = $this->registry(); + + $this->writeJson( + 'package.json', + ['dependencies' => ['left-pad' => '*', self::LOCAL_NAME => 'file:' . self::LOCAL_PATH]], + ); + $this->writeLocal(['version' => '1.0.0', 'dependencies' => ['bootstrap' => '^5.3']]); + $this->install($registry); + + self::assertSame( + [['names' => ['@popperjs/core', 'bootstrap', 'left-pad'], 'fetchedBefore' => 0]], + $registry->prefetched, + 'One prefetch must list every registry package, without the local one, before any fetch.', + ); + self::assertCount( + 3, + $registry->fetched, + 'Every prefetched tarball must still be fetched.', + ); + } + + public function testInstallPrefetchesUnchangedPackagesAgainFromFreshLock(): void + { + $registry = $this->registry(); + + $this->writeJson('package.json', ['dependencies' => ['left-pad' => '*']]); + $this->install($registry); + $this->install($registry); + + self::assertSame( + [ + ['names' => ['left-pad'], 'fetchedBefore' => 0], + ['names' => ['left-pad'], 'fetchedBefore' => 1], + ], + $registry->prefetched, + 'A reinstall from the lock must prefetch the locked packages again.', + ); + } + public function testInstallPrunesEntriesOutsideTheInstallSet(): void { $this->writeJson('package.json', ['dependencies' => ['bootstrap' => '^5.3']]); @@ -998,6 +1039,25 @@ public function testInstallSkipsOptionalLocalSourceWithoutManifestOutsideInstall ); } + public function testInstallSkipsPrefetchWithoutRegistryPackages(): void + { + $registry = $this->registry(); + + $this->writeJson('package.json', ['dependencies' => [self::LOCAL_NAME => 'file:' . self::LOCAL_PATH]]); + $this->writeLocal(['version' => '1.0.0']); + $this->install($registry); + + self::assertSame( + [], + $registry->prefetched, + 'Local packages alone must not trigger a prefetch.', + ); + self::assertFileExists( + "{$this->root}/node_modules/" . self::LOCAL_NAME . '/package.json', + 'The local package must be copied.', + ); + } + public function testInstallTreatsMissingManifestAsEmpty(): void { $this->install($this->registry()); diff --git a/tests/Native/NpmRegistryTest.php b/tests/Native/NpmRegistryTest.php index 7539244..59651bd 100644 --- a/tests/Native/NpmRegistryTest.php +++ b/tests/Native/NpmRegistryTest.php @@ -4,9 +4,10 @@ namespace Foxy\Tests\Native; +use Closure; use Composer\Cache; use Composer\Downloader\TransportException; -use Composer\IO\{BufferIO, NullIO}; +use Composer\IO\{BufferIO, IOInterface, NullIO}; use Composer\Util\Filesystem; use Composer\Util\Http\Response; use Composer\Util\HttpDownloader; @@ -16,12 +17,17 @@ use PHPUnit\Framework\Attributes\DataProviderExternal; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use React\Promise\PromiseInterface; +use Symfony\Component\Console\Output\OutputInterface; use function base64_encode; use function file_get_contents; use function file_put_contents; use function hash; use function mkdir; +use function preg_quote; +use function React\Promise\reject; +use function React\Promise\resolve; use function sha1; use function sys_get_temp_dir; use function uniqid; @@ -29,18 +35,28 @@ use const PHP_EOL; /** - * Unit tests for {@see NpmRegistry} metadata requests, 429 back-off, and cached, integrity-verified tarball downloads. + * Unit tests for {@see NpmRegistry} metadata and bulk advisories requests, 429 back-off, cached, integrity-verified + * tarball downloads, and parallel tarball prefetches. * * {@see NpmRegistryProvider} for test case data providers. */ final class NpmRegistryTest extends TestCase { private const array ACCEPT = ['http' => ['header' => ['Accept: application/vnd.npm.install-v1+json']]]; + private const string ADVISORIES_DOCUMENT = '{"lodash":[{"id":1106913,"severity":"high"}]}'; + private const string ADVISORIES_URL = 'https://registry.test/-/npm/v1/security/advisories/bulk'; + private const string LEFT_PAD_TARBALL = "package/index.js\0module.exports = 'left-pad';\n"; + private const string LEFT_PAD_URL = 'https://registry.test/left-pad/-/left-pad-1.3.0.tgz'; private const string METADATA_URL = 'https://registry.test/@scope%2Fpkg'; private const string TARBALL_URL = 'https://registry.test/@scope/pkg/-/pkg-1.0.0.tgz'; private string $cacheDir = ''; + /** + * @var list URL and target file of each parallel copy, in request order. + */ + private array $copies = []; + /** * @var list */ @@ -158,6 +174,77 @@ public function testFetchTarballUsesWarmCache(): void ); } + public function testGetAdvisoriesPostsEmptyObjectAndReturnsEmptyStringForMissingBody(): void + { + $downloader = $this->createMock(HttpDownloader::class); + + $downloader + ->expects(self::once()) + ->method('get') + ->with(self::ADVISORIES_URL, self::advisoriesOptions('{}')) + ->willReturn(new Response(['url' => self::ADVISORIES_URL], 200, [], null)); + + self::assertSame( + '', + $this->registry($downloader)->getAdvisories([]), + 'A missing body must become an empty string.', + ); + } + + public function testGetAdvisoriesPostsVersionsAsJsonObject(): void + { + $downloader = $this->createMock(HttpDownloader::class); + + $downloader + ->expects(self::once()) + ->method('get') + ->with(self::ADVISORIES_URL, self::advisoriesOptions('{"lodash":["4.17.15"],"minimist":["1.2.0"]}')) + ->willReturn(new Response(['url' => self::ADVISORIES_URL], 200, [], self::ADVISORIES_DOCUMENT)); + + self::assertSame( + self::ADVISORIES_DOCUMENT, + $this->registry($downloader)->getAdvisories(['lodash' => ['4.17.15'], 'minimist' => ['1.2.0']]), + 'The document must be returned as the registry wrote it.', + ); + } + + public function testGetAdvisoriesRetriesTooManyRequests(): void + { + $calls = 0; + $downloader = $this->createMock(HttpDownloader::class); + $exception = self::transportException(429, [], 'Too Many Requests'); + + $downloader + ->expects(self::exactly(2)) + ->method('get') + ->with(self::ADVISORIES_URL, self::advisoriesOptions('{"lodash":["4.17.15"]}')) + ->willReturnCallback( + static function () use (&$calls, $exception): Response { + if (0 === $calls++) { + throw $exception; + } + + return new Response(['url' => self::ADVISORIES_URL], 200, [], self::ADVISORIES_DOCUMENT); + }, + ); + + self::assertSame( + self::ADVISORIES_DOCUMENT, + $this->registry($downloader)->getAdvisories(['lodash' => ['4.17.15']]), + 'The document of the retried request must be returned.', + ); + self::assertSame( + [1], + $this->delays, + 'Back-off must follow the default schedule.', + ); + self::assertSame( + 'Retrying "' . self::ADVISORIES_URL . '" after a 429 response (1 s)' . PHP_EOL, + $this->io->getOutput(), + 'The retry must be announced.', + ); + } + public function testGetMetadataRequestsEscapedScopedNameWithAcceptHeader(): void { $downloader = $this->createMock(HttpDownloader::class); @@ -219,6 +306,230 @@ public function testGetMetadataSleepsWithoutDelayReplacement(): void ); } + public function testPrefetchTarballsCachesVerifiedDownloadForLaterFetch(): void + { + $registry = $this->registry($this->prefetchDownloader(1, self::serve(NpmRegistryProvider::TARBALL))); + + $registry->prefetchTarballs([$this->package()]); + + self::assertSame( + NpmRegistryProvider::TARBALL, + file_get_contents($this->cacheEntry()), + 'Verified download must be cached.', + ); + self::assertFileDoesNotExist( + $this->copies[0][1], + 'Temporary file must be removed.', + ); + self::assertSame( + NpmRegistryProvider::TARBALL, + file_get_contents($registry->fetchTarball($this->package(), $this->directory)), + 'Later fetch must be served from the cache.', + ); + } + + public function testPrefetchTarballsDiscardsDownloadWithMismatchingIntegrity(): void + { + $io = self::verboseIo(); + + $this->registry($this->prefetchDownloader(1, self::serve('corrupted')), io: $io) + ->prefetchTarballs([$this->package()]); + + self::assertFileDoesNotExist( + $this->cacheEntry(), + 'Mismatching download must not be cached.', + ); + self::assertFileDoesNotExist( + $this->copies[0][1], + 'Temporary file must be removed.', + ); + self::assertSame( + 'Prefetching the tarball of "@scope/pkg" (1.0.0) failed: ' + . Message::NATIVE_INTEGRITY_MISMATCH->getMessage(self::TARBALL_URL) . PHP_EOL, + $io->getOutput(), + 'Verbose notice must name the package and the mismatch.', + ); + } + + public function testPrefetchTarballsDiscardsPartialFileOfRejectedDownload(): void + { + $io = self::verboseIo(); + + $this->registry($this->prefetchDownloader(1, self::fail500(true)), io: $io) + ->prefetchTarballs([$this->package()]); + + self::assertFileDoesNotExist( + $this->cacheEntry(), + 'Rejected download must not be cached.', + ); + self::assertFileDoesNotExist( + $this->copies[0][1], + 'Partial temporary file must be removed.', + ); + self::assertSame( + 'Prefetching the tarball of "@scope/pkg" (1.0.0) failed: Internal Server Error' . PHP_EOL, + $io->getOutput(), + 'Verbose notice must name the package and the cause.', + ); + } + + public function testPrefetchTarballsDownloadsMissingPackagesWithOneWait(): void + { + $this->registry( + $this->prefetchDownloader( + 2, + static fn(string $url, string $to): PromiseInterface => self::serve( + self::TARBALL_URL === $url ? NpmRegistryProvider::TARBALL : self::LEFT_PAD_TARBALL, + )($url, $to), + ), + )->prefetchTarballs([$this->package(), $this->leftPad()]); + + self::assertSame( + [self::TARBALL_URL, self::LEFT_PAD_URL], + [$this->copies[0][0], $this->copies[1][0]], + 'Each missing tarball must be requested once, in order.', + ); + self::assertNotSame( + $this->copies[0][1], + $this->copies[1][1], + 'Temporary files must have unique names.', + ); + self::assertMatchesRegularExpression( + '{^' . preg_quote(sys_get_temp_dir()) . '/foxy-tarball-[0-9a-f]{13}\d\.\d{8}\.tgz$}', + $this->copies[0][1], + 'Temporary file must be a unique `.tgz` in the system temporary directory.', + ); + self::assertSame( + NpmRegistryProvider::TARBALL, + file_get_contents($this->cacheEntry()), + 'First tarball must be cached.', + ); + self::assertSame( + self::LEFT_PAD_TARBALL, + file_get_contents($this->leftPadCacheEntry()), + 'Second tarball must be cached.', + ); + self::assertFileDoesNotExist( + $this->copies[1][1], + 'Every temporary file must be removed.', + ); + } + + public function testPrefetchTarballsHidesFailureNoticeAtNormalVerbosity(): void + { + $this->registry($this->prefetchDownloader(1, self::fail500(false)))->prefetchTarballs([$this->package()]); + + self::assertSame( + '', + $this->io->getOutput(), + 'Notice must be verbose only.', + ); + } + + public function testPrefetchTarballsIgnoresRejectedDownload(): void + { + $io = self::verboseIo(); + + $this->registry($this->prefetchDownloader(1, self::fail500(false)), io: $io) + ->prefetchTarballs([$this->package()]); + + self::assertFileDoesNotExist( + $this->cacheEntry(), + 'Rejected download must not be cached.', + ); + self::assertFileDoesNotExist( + $this->copies[0][1], + 'No temporary file may be left.', + ); + self::assertSame( + 'Prefetching the tarball of "@scope/pkg" (1.0.0) failed: Internal Server Error' . PHP_EOL, + $io->getOutput(), + 'Verbose notice must name the package and the cause.', + ); + } + + public function testPrefetchTarballsRequestsNothingForCachedPackages(): void + { + $this->seedCache(NpmRegistryProvider::TARBALL); + + $this->registry($this->idleDownloader())->prefetchTarballs([$this->package()]); + + self::assertSame( + [], + $this->copies, + 'Cached tarball must not be requested.', + ); + } + + public function testPrefetchTarballsRequestsNothingForEmptyList(): void + { + $this->registry($this->idleDownloader())->prefetchTarballs([]); + + self::assertSame( + [], + $this->copies, + 'No tarball must be requested.', + ); + } + + public function testPrefetchTarballsRequestsNothingWithDisabledCache(): void + { + mkdir("{$this->root}/locked", 0o555); + + $cache = new Cache(new NullIO(), "{$this->root}/locked", 'a-z0-9_./'); + + $this->registry($this->idleDownloader(), $cache)->prefetchTarballs([$this->package()]); + + self::assertFalse( + $cache->isEnabled(), + 'Cache in a read-only directory must be disabled.', + ); + } + + public function testPrefetchTarballsRequestsNothingWithoutTarballUrl(): void + { + $this->registry($this->idleDownloader())->prefetchTarballs( + [new ResolvedPackage('no-url', '1.0.0'), new ResolvedPackage('empty-url', '1.0.0', '', 'sha512-AAAA')], + ); + + self::assertSame( + [], + $this->copies, + 'Package without a tarball URL must be skipped.', + ); + } + + public function testPrefetchTarballsRequestsNothingWithReadOnlyCache(): void + { + $cache = new Cache(new NullIO(), $this->cacheDir, 'a-z0-9_./', null, true); + + $this->registry($this->idleDownloader(), $cache)->prefetchTarballs([$this->package()]); + + self::assertFileDoesNotExist( + $this->cacheEntry(), + 'Read-only cache must stay empty.', + ); + } + + public function testPrefetchTarballsSkipsCachedPackageAndRequestsMissingOne(): void + { + $this->seedCache(NpmRegistryProvider::TARBALL); + + $this->registry($this->prefetchDownloader(1, self::serve(self::LEFT_PAD_TARBALL))) + ->prefetchTarballs([$this->package(), $this->leftPad()]); + + self::assertSame( + self::LEFT_PAD_URL, + $this->copies[0][0], + 'Only the missing tarball must be requested.', + ); + self::assertSame( + self::LEFT_PAD_TARBALL, + file_get_contents($this->leftPadCacheEntry()), + 'Missing tarball must be cached.', + ); + } + public function testThrowRuntimeExceptionForInvalidMetadataJson(): void { $downloader = $this->createMock(HttpDownloader::class); @@ -265,6 +576,23 @@ public function testThrowRuntimeExceptionForRejectedIntegrity(string $integrity) ); } + public function testThrowRuntimeExceptionWhenAdvisoriesEndpointIsMissing(): void + { + $downloader = $this->createMock(HttpDownloader::class); + + $downloader + ->expects(self::once()) + ->method('get') + ->willThrowException(self::transportException(404, [], 'Not Found')); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + Message::NATIVE_REGISTRY_REQUEST_FAILED->getMessage(self::ADVISORIES_URL, 'Not Found'), + ); + + $this->registry($downloader)->getAdvisories(['lodash' => ['4.17.15']]); + } + public function testThrowRuntimeExceptionWhenDownloadFailsAfterCorruptedCacheHit(): void { $this->seedCache('corrupted'); @@ -409,6 +737,22 @@ protected function tearDown(): void (new Filesystem())->removeDirectory($this->root); } + /** + * Returns the stream options of a bulk advisories request with the JSON body. + * + * @return array{http: array{method: string, header: list, content: string}} + */ + private static function advisoriesOptions(string $content): array + { + return [ + 'http' => [ + 'method' => 'POST', + 'header' => ['Content-Type: application/json', 'Accept: application/json'], + 'content' => $content, + ], + ]; + } + private function cache(): Cache { return new Cache(new NullIO(), $this->cacheDir, 'a-z0-9_./'); @@ -424,6 +768,54 @@ private function cacheKey(): string return 'tarball/@scope/pkg/1.0.0-' . sha1(self::TARBALL_URL) . '.tgz'; } + /** + * Returns a promise factory that rejects a parallel copy with an HTTP 500 error, leaving a partial file if asked. + * + * @return Closure(string, string): PromiseInterface + */ + private static function fail500(bool $partial): Closure + { + return static function (string $url, string $to) use ($partial): PromiseInterface { + if ($partial) { + file_put_contents($to, 'partial'); + } + + return reject(self::transportException(500, [], 'Internal Server Error')); + }; + } + + /** + * Returns a downloader that must receive neither a parallel copy nor a `wait()` call. + */ + private function idleDownloader(): HttpDownloader&MockObject + { + $downloader = $this->createMock(HttpDownloader::class); + + $downloader + ->expects(self::never()) + ->method('addCopy'); + $downloader + ->expects(self::never()) + ->method('wait'); + + return $downloader; + } + + private function leftPad(): ResolvedPackage + { + return new ResolvedPackage( + 'left-pad', + '1.3.0', + self::LEFT_PAD_URL, + 'sha512-' . base64_encode(hash('sha512', self::LEFT_PAD_TARBALL, true)), + ); + } + + private function leftPadCacheEntry(): string + { + return "{$this->cacheDir}/tarball/left-pad/1.3.0-" . sha1(self::LEFT_PAD_URL) . '.tgz'; + } + /** * Returns a downloader whose first metadata request fails with the exception and whose second one succeeds. */ @@ -471,12 +863,45 @@ private function package(string|null $integrity = null): ResolvedPackage ); } - private function registry(HttpDownloader $downloader): NpmRegistry + /** + * Returns a downloader that records the parallel copies into {@see $copies}, answers them through the factory, and + * expects one `wait()` and no sequential copy. + * + * @param Closure(string, string): PromiseInterface $answer Promise factory for each copy. + */ + private function prefetchDownloader(int $copies, Closure $answer): HttpDownloader&MockObject { + $downloader = $this->createMock(HttpDownloader::class); + + $downloader + ->expects(self::exactly($copies)) + ->method('addCopy') + ->willReturnCallback( + function (string $url, string $to) use ($answer): PromiseInterface { + $this->copies[] = [$url, $to]; + + return $answer($url, $to); + }, + ); + $downloader + ->expects(self::once()) + ->method('wait'); + $downloader + ->expects(self::never()) + ->method('copy'); + + return $downloader; + } + + private function registry( + HttpDownloader $downloader, + Cache|null $cache = null, + IOInterface|null $io = null, + ): NpmRegistry { return new NpmRegistry( $downloader, - $this->cache(), - $this->io, + $cache ?? $this->cache(), + $io ?? $this->io, 'https://registry.test/', function (int $seconds): void { $this->delays[] = $seconds; @@ -496,6 +921,20 @@ private function seedCache(string $bytes): void $this->cache()->copyFrom($this->cacheKey(), $source); } + /** + * Returns a promise factory that writes the bytes to the copy target and fulfils the copy. + * + * @return Closure(string, string): PromiseInterface + */ + private static function serve(string $bytes): Closure + { + return static function (string $url, string $to) use ($bytes): PromiseInterface { + file_put_contents($to, $bytes); + + return resolve(new Response(['url' => $url], 200, [], null)); + }; + } + /** * Returns a downloader that writes the bytes to the requested file once. */ @@ -533,4 +972,9 @@ private static function transportException( return $exception; } + + private static function verboseIo(): BufferIO + { + return new BufferIO('', OutputInterface::VERBOSITY_VERBOSE); + } } diff --git a/tests/Provider/AuditParserProvider.php b/tests/Provider/AuditParserProvider.php index a3b4389..582b1a2 100644 --- a/tests/Provider/AuditParserProvider.php +++ b/tests/Provider/AuditParserProvider.php @@ -6,7 +6,7 @@ use Closure; use Foxy\Audit\AuditParserInterface; -use Foxy\Audit\Parser\{BunAuditParser, NpmAuditParser, PnpmAuditParser, YarnAuditParser}; +use Foxy\Audit\Parser\{BunAuditParser, NativeAuditParser, NpmAuditParser, PnpmAuditParser, YarnAuditParser}; use Foxy\Exception\Message; /** @@ -165,7 +165,7 @@ public static function malformedReports(): iterable yield 'Bun advisories object' => [ new BunAuditParser(), '{"pkg":{}}', - self::malformed('bun', Message::AUDIT_BUN_PACKAGE_ADVISORIES_REQUIRED->getMessage()), + self::malformed('bun', Message::AUDIT_BULK_PACKAGE_ADVISORIES_REQUIRED->getMessage()), ]; yield 'Bun blank required string' => [ new BunAuditParser(), @@ -180,7 +180,7 @@ public static function malformedReports(): iterable yield 'Bun empty package key' => [ new BunAuditParser(), '{"":[]}', - self::malformed('bun', Message::AUDIT_BUN_PACKAGE_ADVISORIES_REQUIRED->getMessage()), + self::malformed('bun', Message::AUDIT_BULK_PACKAGE_ADVISORIES_REQUIRED->getMessage()), ]; yield 'Bun non-scalar advisory ID' => [ new BunAuditParser(), @@ -197,6 +197,59 @@ public static function malformedReports(): iterable '[]', self::malformed('bun', Message::AUDIT_REPORT_JSON_OBJECT_EXPECTED->getMessage()), ]; + yield 'native advisories object' => [ + new NativeAuditParser(), + '{"pkg":{}}', + self::malformed('native', Message::AUDIT_BULK_PACKAGE_ADVISORIES_REQUIRED->getMessage()), + ]; + yield 'native blank required string' => [ + new NativeAuditParser(), + '{"pkg":[{"id":1,"severity":"low","vulnerable_versions":" "}]}', + self::malformed( + 'native', + Message::AUDIT_FIELD_STRING_REQUIRED->getMessage('pkg.0', 'vulnerable_versions'), + ), + ]; + yield 'native empty advisory ID' => [ + new NativeAuditParser(), + '{"pkg":[{"id":" "}]}', + self::malformed('native', Message::AUDIT_VALUE_EMPTY->getMessage('pkg.0.id')), + ]; + yield 'native empty document' => [ + new NativeAuditParser(), + '', + self::malformed('native', Message::AUDIT_REPORT_JSON_OBJECT_EXPECTED->getMessage()), + ]; + yield 'native empty package key' => [ + new NativeAuditParser(), + '{"":[]}', + self::malformed('native', Message::AUDIT_BULK_PACKAGE_ADVISORIES_REQUIRED->getMessage()), + ]; + yield 'native error document' => [ + new NativeAuditParser(), + '{"statusCode":400,"error":"Bad Request","message":"Invalid request payload input"}', + self::malformed('native', Message::AUDIT_BULK_PACKAGE_ADVISORIES_REQUIRED->getMessage()), + ]; + yield 'native non-scalar advisory ID' => [ + new NativeAuditParser(), + '{"pkg":[{"id":false}]}', + self::malformed('native', Message::AUDIT_VALUE_STRING_OR_INTEGER_REQUIRED->getMessage('pkg.0.id')), + ]; + yield 'native non-string optional URL' => [ + new NativeAuditParser(), + '{"pkg":[{"id":1,"url":false}]}', + self::malformed('native', Message::AUDIT_FIELD_STRING_REQUIRED->getMessage('pkg.0', 'url')), + ]; + yield 'native root list' => [ + new NativeAuditParser(), + '[]', + self::malformed('native', Message::AUDIT_REPORT_JSON_OBJECT_EXPECTED->getMessage()), + ]; + yield 'native unsupported severity' => [ + new NativeAuditParser(), + '{"pkg":[{"id":1,"title":"Issue","severity":"unknown","vulnerable_versions":"<1"}]}', + self::malformed('native', Message::AUDIT_VALUE_SEVERITY_UNSUPPORTED->getMessage('pkg.0')), + ]; yield 'npm empty via' => [ new NpmAuditParser(), '{"auditReportVersion":2,"vulnerabilities":{"pkg":{"name":"pkg","severity":"low","isDirect":true,"via":[],"effects":[],"range":"<1","nodes":[],"fixAvailable":false}},"metadata":{}}', diff --git a/tests/Provider/MessageProvider.php b/tests/Provider/MessageProvider.php index 7e42f4c..cfbe349 100644 --- a/tests/Provider/MessageProvider.php +++ b/tests/Provider/MessageProvider.php @@ -131,6 +131,12 @@ public static function cases(): iterable ['gulp'], 'The asset manager "gulp" doesn\'t exist.', ]; + yield 'ASSET_NATIVE_AUDIT_NO_DEV_UNSUPPORTED' => [ + Message::ASSET_NATIVE_AUDIT_NO_DEV_UNSUPPORTED, + [], + 'The native audit cannot guarantee the requested dependency scope because the native manager installs ' + . 'development dependencies together with the others.', + ]; yield 'ASSET_NPM_WORKSPACE_GRAPH_UNENUMERABLE' => [ Message::ASSET_NPM_WORKSPACE_GRAPH_UNENUMERABLE, [], @@ -152,8 +158,8 @@ public static function cases(): iterable ['yarn', '1.22.19', '>=2.0.0'], 'The installed yarn version "1.22.19" doesn\'t match the supported version constraint ">=2.0.0".', ]; - yield 'AUDIT_BUN_PACKAGE_ADVISORIES_REQUIRED' => [ - Message::AUDIT_BUN_PACKAGE_ADVISORIES_REQUIRED, + yield 'AUDIT_BULK_PACKAGE_ADVISORIES_REQUIRED' => [ + Message::AUDIT_BULK_PACKAGE_ADVISORIES_REQUIRED, [], 'each package must contain a list of advisories', ]; @@ -293,11 +299,6 @@ public static function cases(): iterable ['npm', 'invalid JSON'], 'The npm audit output is malformed: invalid JSON.', ]; - yield 'AUDIT_PARSER_UNSUPPORTED_MANAGER' => [ - Message::AUDIT_PARSER_UNSUPPORTED_MANAGER, - ['deno'], - 'The asset manager "deno" does not provide a supported audit report.', - ]; yield 'AUDIT_PNPM_FINDINGS_EMPTY' => [ Message::AUDIT_PNPM_FINDINGS_EMPTY, ['advisories.1096'], @@ -443,11 +444,6 @@ public static function cases(): iterable [], 'Foxy is disabled; frontend dependencies cannot be audited.', ]; - yield 'FOXY_AUDIT_UNSUPPORTED_MANAGER' => [ - Message::FOXY_AUDIT_UNSUPPORTED_MANAGER, - [], - 'The selected asset manager does not support security audits.', - ]; yield 'JSON_FILE_UNREADABLE' => [ Message::JSON_FILE_UNREADABLE, ['/project/package.json'], diff --git a/tests/Util/PackageUtilTest.php b/tests/Util/PackageUtilTest.php index a750cf6..72540c5 100644 --- a/tests/Util/PackageUtilTest.php +++ b/tests/Util/PackageUtilTest.php @@ -4,68 +4,16 @@ namespace Foxy\Tests\Util; -use Composer\Package\CompletePackage; +use Composer\Package\{AliasPackage, CompletePackage}; use Composer\Package\Loader\ArrayLoader; use Foxy\Util\PackageUtil; use PHPUnit\Framework\TestCase; +/** + * Unit tests for {@see PackageUtil} lock package hydration. + */ final class PackageUtilTest extends TestCase { - public function testConvertLockAlias(): void - { - $lockData = [ - 'content-hash' => 'HASH_VALUE', - 'aliases' => [ - [ - 'alias' => '1.0.0', - 'alias_normalized' - => '1.0.0.0', - 'version' => 'dev-feature/1.0-test', - 'package' => 'foo/bar', - ], - [ - 'alias' => '2.2.0', - 'alias_normalized' => '2.2.0.0', - 'version' => 'dev-feature/2.2-test', - 'package' => 'foo/baz', - ], - ], - ]; - - $expectedAliases = [ - 'foo/bar' => [ - 'dev-feature/1.0-test' => [ - 'alias' => '1.0.0', - 'alias_normalized' => '1.0.0.0', - ], - ], - 'foo/baz' => [ - 'dev-feature/2.2-test' => [ - 'alias' => '2.2.0', - 'alias_normalized' => '2.2.0.0', - ], - ], - ]; - - $convertedAliases = PackageUtil::convertLockAlias($lockData); - - self::assertArrayHasKey( - 'aliases', - $convertedAliases, - 'The converted aliases array does not contain the expected key.', - ); - self::assertSame( - 'HASH_VALUE', - $convertedAliases['content-hash'], - 'The content hash does not match the expected value.', - ); - self::assertEquals( - $expectedAliases, - $convertedAliases['aliases'], - 'The converted aliases do not match the expected ones.', - ); - } - public function testLoadLockPackageLoadsOnlyRequestedSection(): void { $lockData = [ @@ -136,7 +84,7 @@ public function testLoadLockPackages(): void ); } - public function testLoadLockPackagesCanPreserveRawAliases(): void + public function testLoadLockPackagesPreservesRawAliases(): void { $aliases = [ [ @@ -153,7 +101,7 @@ public function testLoadLockPackagesCanPreserveRawAliases(): void 'aliases' => $aliases, ]; - $lockDataLoaded = PackageUtil::loadLockPackages($lockData, false); + $lockDataLoaded = PackageUtil::loadLockPackages($lockData); self::assertInstanceOf( CompletePackage::class, @@ -167,42 +115,43 @@ public function testLoadLockPackagesCanPreserveRawAliases(): void ); } - public function testLoadLockPackagesConvertsAliasesByDefault(): void + public function testLoadLockPackagesWithoutPackages(): void + { + self::assertSame( + [], + PackageUtil::loadLockPackages([]), + 'The loaded lock data for an empty input does not match the expected empty array.', + ); + } + + public function testLoadLockPackageUnwrapsBranchAliasPackage(): void { $lockData = [ 'packages' => [ - ['name' => 'foo/bar', 'version' => 'dev-feature'], - ], - 'aliases' => [ [ - 'package' => 'foo/bar', - 'version' => 'dev-feature', - 'alias' => '1.0.x-dev', - 'alias_normalized' => '1.0.9999999.9999999-dev', + 'name' => 'foo/bar', + 'version' => 'dev-main', + 'extra' => ['branch-alias' => ['dev-main' => '1.0.x-dev']], ], ], ]; - $loaded = PackageUtil::loadLockPackages($lockData); + $loaded = PackageUtil::loadLockPackage(new ArrayLoader(), $lockData); - self::assertSame( - [ - 'dev-feature' => [ - 'alias' => '1.0.x-dev', - 'alias_normalized' => '1.0.9999999.9999999-dev', - ], - ], - $loaded['aliases']['foo/bar'], - 'The loaded aliases for the package "foo/bar" do not match the expected ones.', + self::assertInstanceOf( + CompletePackage::class, + $loaded['packages'][0], + 'The loaded package should be the aliased package.', + ); + self::assertNotInstanceOf( + AliasPackage::class, + $loaded['packages'][0], + 'The branch alias wrapper should be removed.', ); - } - - public function testLoadLockPackagesWithoutPackages(): void - { self::assertSame( - [], - PackageUtil::loadLockPackages([]), - 'The loaded lock data for an empty input does not match the expected empty array.', + 'dev-main', + $loaded['packages'][0]->getPrettyVersion(), + 'The loaded package should keep the locked version.', ); } } From 12d7f83d710ffab5e278184f84ee61fbe368de89 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Sat, 10 Oct 2026 21:22:13 +0000 Subject: [PATCH 2/8] Apply fixes from StyleCI --- src/Audit/CveResolverInterface.php | 3 +-- src/Audit/GitHubAdvisoryCveResolver.php | 3 +-- tests/Audit/GitHubAdvisoryCveResolverTest.php | 5 ++--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Audit/CveResolverInterface.php b/src/Audit/CveResolverInterface.php index c97f40a..da024c6 100644 --- a/src/Audit/CveResolverInterface.php +++ b/src/Audit/CveResolverInterface.php @@ -17,10 +17,9 @@ interface CveResolverInterface * * @param list $ghsaIds GHSA identifiers to resolve. * + * @throws RuntimeException if an identifier is not a GHSA identifier; nothing is requested in that case. * @return array One result for every identifier, keyed by canonical GHSA * identifier; a `Throwable` marks a lookup that failed. - * - * @throws RuntimeException if an identifier is not a GHSA identifier; nothing is requested in that case. */ public function resolveMany(array $ghsaIds): array; } diff --git a/src/Audit/GitHubAdvisoryCveResolver.php b/src/Audit/GitHubAdvisoryCveResolver.php index c43e202..2f0b9cd 100644 --- a/src/Audit/GitHubAdvisoryCveResolver.php +++ b/src/Audit/GitHubAdvisoryCveResolver.php @@ -36,10 +36,9 @@ public function __construct(private HttpDownloader $httpDownloader) {} * * @param list $ghsaIds GHSA identifiers to resolve, trimmed and compared case-insensitively. * + * @throws RuntimeException if an identifier is not a GHSA identifier; nothing is requested in that case. * @return array Resolutions keyed by canonical GHSA identifier; a `Throwable` is * the transport, JSON, or document error of that lookup. - * - * @throws RuntimeException if an identifier is not a GHSA identifier; nothing is requested in that case. */ public function resolveMany(array $ghsaIds): array { diff --git a/tests/Audit/GitHubAdvisoryCveResolverTest.php b/tests/Audit/GitHubAdvisoryCveResolverTest.php index aa71f26..2453fce 100644 --- a/tests/Audit/GitHubAdvisoryCveResolverTest.php +++ b/tests/Audit/GitHubAdvisoryCveResolverTest.php @@ -17,7 +17,6 @@ use Seld\JsonLint\ParsingException; use Throwable; -use function array_key_exists; use function array_keys; use function count; use function React\Promise\reject; @@ -333,8 +332,8 @@ static function (string $url, array $options) use (&$answers): PromiseInterface $ghsaId = substr($url, strlen('https://api.github.com/advisories/')); - self::assertTrue( - array_key_exists($ghsaId, $answers), + self::assertArrayHasKey( + $ghsaId, $answers, 'Request must target an expected advisory URL once.', ); From 614e7dfa0334ca48272433cc1b44b2c813523cf9 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Sat, 10 Oct 2026 18:23:40 -0300 Subject: [PATCH 3/8] Fix ECS ci. --- tests/Audit/GitHubAdvisoryCveResolverTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Audit/GitHubAdvisoryCveResolverTest.php b/tests/Audit/GitHubAdvisoryCveResolverTest.php index 2453fce..dfe7ecc 100644 --- a/tests/Audit/GitHubAdvisoryCveResolverTest.php +++ b/tests/Audit/GitHubAdvisoryCveResolverTest.php @@ -333,7 +333,8 @@ static function (string $url, array $options) use (&$answers): PromiseInterface $ghsaId = substr($url, strlen('https://api.github.com/advisories/')); self::assertArrayHasKey( - $ghsaId, $answers, + $ghsaId, + $answers, 'Request must target an expected advisory URL once.', ); From 6872d5e9c5aee25f2e6cf63db233135b5e7ab6ea Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Sat, 10 Oct 2026 18:36:18 -0300 Subject: [PATCH 4/8] More rwwork code. --- README.md | 4 +- UPGRADE.md | 19 +- docs/config.md | 13 +- docs/events.md | 4 +- docs/faqs.md | 14 +- docs/index.md | 8 +- docs/usage.md | 17 +- rector.php | 8 + src/Asset/AbstractAssetManager.php | 96 +++-- src/Asset/AbstractManifestAssetManager.php | 75 ++-- src/Asset/AssetManagerInterface.php | 6 - src/Asset/BunManager.php | 27 +- src/Asset/DenoManager.php | 24 +- src/Asset/NativeManager.php | 35 +- src/Asset/NpmManager.php | 17 +- src/Asset/PnpmManager.php | 17 +- src/Asset/YarnManager.php | 14 +- src/Command/AuditCommand.php | 5 +- src/Config/Config.php | 30 +- src/Exception/Message.php | 21 +- src/Foxy.php | 133 +++++-- src/Solver/Solver.php | 59 ++- tests/Asset/AbstractAssetManagerTest.php | 186 ++++++--- tests/Asset/AssetManager.php | 102 ----- tests/Asset/BunAssetManagerTest.php | 2 +- tests/Asset/DenoAssetManagerTest.php | 28 +- tests/Asset/NativeManagerTest.php | 50 +-- tests/Asset/NpmAssetManagerTest.php | 4 +- tests/Asset/PnpmAssetManagerTest.php | 3 +- tests/Asset/YarnAssetManagerTest.php | 6 +- tests/Command/FoxyCapabilityTest.php | 17 +- tests/Config/ConfigTest.php | 114 +++++- .../Asset/CustomBinaryAssetManager.php | 57 +++ .../Asset/InspectableAssetManager.php | 17 +- tests/Fixtures/Asset/StubAssetManager.php | 6 - tests/FoxyTest.php | 284 +++++++++++--- tests/Native/NpmRegistryTest.php | 15 +- tests/Provider/AssetPackageProvider.php | 7 +- tests/Provider/MessageProvider.php | 17 +- tests/Solver/SolverTest.php | 359 +++++++++++++++++- 40 files changed, 1256 insertions(+), 664 deletions(-) create mode 100644 tests/Fixtures/Asset/CustomBinaryAssetManager.php diff --git a/README.md b/README.md index 4d31fc1..dddff8f 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ Authorize the Composer plugin and install Foxy 0.3: ```bash composer config allow-plugins.php-forge/foxy true -composer require php-forge/foxy:^0.3 +composer require php-forge/foxy:^0.4 ``` Selecting a manager explicitly is recommended for reproducible local and CI behavior: @@ -60,7 +60,7 @@ Selecting a manager explicitly is recommended for reproducible local and CI beha ```json { "require": { - "php-forge/foxy": "^0.3" + "php-forge/foxy": "^0.4" }, "config": { "allow-plugins": { diff --git a/UPGRADE.md b/UPGRADE.md index 69fbe84..29e2fc4 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -31,9 +31,10 @@ were removed. The `native` manager now supports `composer foxy:audit` through the registry's bulk advisories endpoint; `--no-dev` is rejected for it, as for Deno, because the native manager installs development dependencies together with the -others. `Foxy\Asset\NativeManager` takes the `NpmRegistryInterface` in its constructor, and `NpmRegistryInterface` -gained `getAdvisories(array $versions): string` and `prefetchTarballs(array $packages): void`; custom registries must -implement both. +others. `Foxy\Asset\NativeManager` takes the `NpmRegistryInterface` as its last constructor argument, and +`NpmRegistryInterface` gained `getAdvisories(array $versions): string` and `prefetchTarballs(array $packages): void`; +custom registries must implement both. The registry now uses the HTTP downloader of Composer's loop, so the +parallel downloads need no configuration. ### CVE lookups resolve in batches @@ -55,7 +56,8 @@ instead of by every Composer command; `composer foxy:audit` reports them with ex `Foxy\Config\Config::setResolvedManager()` was removed. Probing or validating a manager no longer changes the shared `Config`; `Config::forManager(string $manager): static` returns a copy whose `manager-*` map entries resolve for that -manager, and `AbstractManifestAssetManager::getManagerConfig()` (protected) gives each built-in manager its own view. +manager, and `AbstractManifestAssetManager::getManagerConfig()` (protected) gives each manager its own view, built +on every call. Custom managers that called `setResolvedManager()` remove the call and read `manager-*` keys through `getManagerConfig()`. @@ -69,9 +71,12 @@ override `getDefaultBinary()`; managers that implement the three methods keep wo ### Asset rollback belongs to the solver `AssetManagerInterface::setFallback()` was removed, together with the fallback constructor argument, the protected -`$fallback` property, and `restoreAfterFailure()` of `AbstractAssetManager`. `Foxy\Solver\Solver` takes the asset -fallback as its last constructor argument and restores the manifest when the dependency merge or the manager run -fails, in the same cases as before; managers no longer restore anything. +`$fallback` property, and `restoreAfterFailure()` of `AbstractAssetManager`; its constructor is now +`(IOInterface $io, Config $config, ProcessExecutor $executor, Filesystem $fs, VersionConverterInterface|null +$versionConverter = null)`, so a custom manager that passed a fallback must drop that argument. +`Foxy\Solver\Solver` takes the asset fallback as its last constructor argument and restores the manifest when the +dependency merge or the manager run throws or returns a non-zero code, before the `foxy.post-solve` event, as the +managers did; a listener of that event that throws no longer triggers an asset restore, only the Composer one. ### Composer rollback events diff --git a/docs/config.md b/docs/config.md index ab93fd9..b00659c 100644 --- a/docs/config.md +++ b/docs/config.md @@ -99,6 +99,10 @@ selected by availability; it is used only when `manager` is `native` or when `fo lockfile. Commit the lockfile generated by the selected manager. Foxy reports an error when an explicitly configured manager is unknown, or when execution is enabled and its executable is unavailable. +Foxy resolves the manager when the first install, update, or audit needs it, not when Composer loads the plugin, so +Composer commands that do none of these (for example `about`, `show`, or `validate`) never probe an executable. +Selection errors are therefore reported by the first command that needs the manager. + When `run-asset-manager` is `false`, automatic selection does not probe executables. Foxy uses the manager identified by a single recognized lockfile, or npm as the manifest adapter when no lockfile exists. Multiple lockfiles still require an explicit `manager` value. @@ -410,12 +414,15 @@ Both fallbacks are enabled by default. They can be controlled independently: } ``` -- `fallback-asset` restores the project `package.json` when package merging or manager execution fails. +- `fallback-asset` restores the project `package.json` when package merging fails, or when manager execution throws or + returns a non-zero result. - `fallback-composer` restores the captured lock data and installed Composer dependencies for any exception or non-zero manager result during asset solving. -Foxy rethrows the original solve error when Composer restoration succeeds. If restoration also fails, Foxy reports the -rollback failure and retains the original error as the previous exception. Disabled fallbacks do not create snapshots. +Foxy restores `package.json` first and Composer state second, and attempts the Composer restoration even when the +`package.json` restoration fails. It rethrows the original solve error when restoration succeeds. If a restoration also +fails, Foxy reports the rollback failure and retains the original error as the previous exception. Disabled fallbacks +do not create snapshots. Composer lock and vendor state are captured at Composer's `pre-operations-exec` event. This is not a fully atomic transaction for `composer require` or `composer remove`: Composer may update the root `composer.json` before that event, diff --git a/docs/events.md b/docs/events.md index bd9934c..6e5b89d 100644 --- a/docs/events.md +++ b/docs/events.md @@ -43,7 +43,9 @@ The path passed to `addAsset()` may identify a package definition file, which Fo `PostSolveEvent` exposes the common getters and `getRunResult()`, which returns the frontend manager process exit code. The event is dispatched before Foxy attempts to restore captured Composer lock and vendor state after a non-zero -result. It is not dispatched when manager execution throws before returning a result. +result; by then `package.json` has already been restored when `fallback-asset` is enabled. It is not dispatched when +manager execution throws before returning a result. An exception thrown by a listener restores Composer state only, not +`package.json`. ## Subscriber example diff --git a/docs/faqs.md b/docs/faqs.md index 77bcd6f..242deb7 100644 --- a/docs/faqs.md +++ b/docs/faqs.md @@ -41,7 +41,9 @@ Set `config.foxy.manager` to `bun`, `deno`, `native`, `npm`, `pnpm`, or `yarn` f omitted and manager execution is enabled, Foxy first looks for one recognized lockfile and then for an available manager executable. With execution disabled, it uses a single recognized lockfile or npm as the manifest adapter without probing binaries. Multiple recognized lockfiles require explicit selection in either mode. The `native` -manager is selected only explicitly or through its `foxy.lock` file, never by availability. +manager is selected only explicitly or through its `foxy.lock` file, never by availability. The choice is made when the +first install, update, or audit needs a manager, so other Composer commands neither probe executables nor report +selection errors. Explicit selection and a committed lockfile are recommended for CI. @@ -55,16 +57,16 @@ complete scope. ## How can frontend dependencies be checked for known CVEs? -Run `composer foxy:audit`. Foxy audits the dependency graph from the selected manager's native lockfile. The default +Run `composer foxy:audit`. Foxy audits the dependency graph from the selected manager's own lockfile. The default table, plain, and JSON formats identify the affected package, advisory, severity, vulnerable range, and available CVE identifiers. Use `--format=summary` for compact counts in CI or `--format=json` for a versioned machine-readable document. -The native registry response commonly contains a GHSA but no CVE. Foxy resolves that relationship through GitHub's +The registry's audit response commonly contains a GHSA but no CVE. Foxy resolves that relationship through GitHub's public advisory API unless `--no-cve` or `--format=summary` is supplied. The lookups run concurrently through Composer's HTTP downloader (at most 12 at once; `COMPOSER_MAX_PARALLEL_HTTP` changes the limit), and a `github-oauth` token in Composer's `auth.json` applies to them. A lookup failure marks only its own advisory as unavailable without -hiding the native security finding. See [Security auditing](usage.md#security-auditing) for options and exit statuses. +hiding the security finding itself. See [Security auditing](usage.md#security-auditing) for options and exit statuses. ## Why is a dependency's package.json not detected? @@ -82,8 +84,8 @@ during Composer install and update operations. When the manager throws or returns any non-zero status, Foxy reports the error and, by default, restores the previous project `package.json`, Composer lock data, and installed Composer dependencies. The original manager error is rethrown -when restoration succeeds. If Composer restoration also fails, the rollback error retains the original error as its -previous exception. +when restoration succeeds. If a restoration also fails, the rollback error retains the original error as its previous +exception. Set `fallback-asset` or `fallback-composer` to `false` only when retaining partial state is intentional. A Composer rollback installer failure is reported as a `RuntimeException` rather than being hidden. diff --git a/docs/index.md b/docs/index.md index 1339716..9709db1 100644 --- a/docs/index.md +++ b/docs/index.md @@ -30,8 +30,8 @@ package version only when the embedded package does not declare its own version. | `native` | No frontend manager; PHP with the `zlib` extension | The frontend manager requirements apply to automatic manager execution and explicit security audits. Manifest-only -Composer processing does not require a manager binary, but `composer foxy:audit` does. The `native` manager needs no -binary at all; see [Native manager](config.md#native-manager). +Composer processing does not require a manager binary, but `composer foxy:audit` does with a binary manager. The +`native` manager needs no binary at all, not even to audit; see [Native manager](config.md#native-manager). ## Installation @@ -39,7 +39,7 @@ Composer plugins execute code during Composer operations. Authorize Foxy explici ```bash composer config allow-plugins.php-forge/foxy true -composer require php-forge/foxy:^0.3 +composer require php-forge/foxy:^0.4 ``` The plugin is installed in the configured Composer vendor directory, normally `vendor/php-forge/foxy`. @@ -56,7 +56,7 @@ During automatic Composer processing, `run-asset-manager=false` makes Foxy selec the manifest adapter without probing executables. An explicit `composer foxy:audit` still probes and runs the selected manager. -Commit the selected manager's native lockfile and use the same explicit manager in local development and CI. +Commit the selected manager's own lockfile and use the same explicit manager in local development and CI. ## Next steps diff --git a/docs/usage.md b/docs/usage.md index 5fa281c..0e70678 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -7,7 +7,7 @@ Authorize and require the plugin in the application: ```json { "require": { - "php-forge/foxy": "^0.3" + "php-forge/foxy": "^0.4" }, "config": { "allow-plugins": { @@ -43,7 +43,7 @@ Run an explicit frontend dependency audit from Composer: composer foxy:audit ``` -The command requires the selected manager's native lockfile, validates the manager version, and asks the manager for a +The command requires the selected manager's own lockfile, validates the manager version, and asks the manager for a security report. It reads the lockfile and does not run an install, update, fix, or fallback. Foxy supports npm audit report version 2 starting with npm 10.9.8, the current report schemas emitted by pnpm 11, Yarn 4, and Bun 1.4, and the npm registry's bulk advisories document read by the `native` manager; legacy report formats are rejected instead of @@ -103,13 +103,13 @@ The default table identifies the package, severity, advisory ID, CVE, vulnerable document has `schema_version: 1` and is suitable for CI artifacts. Diagnostic and CVE lookup warnings are written to standard error so JSON on standard output remains valid. -Native npm registry audit data normally identifies advisories by a numeric ID and GHSA rather than a CVE. Unless +The npm registry's audit data normally identifies advisories by a numeric ID and GHSA rather than a CVE. Unless `--no-cve` or `--format=summary` is used, Foxy resolves each unique public GHSA once through the [GitHub global security advisories API](https://docs.github.com/en/rest/security-advisories/global-advisories). The lookups run concurrently through Composer's HTTP downloader, at most 12 at once by default; the `COMPOSER_MAX_PARALLEL_HTTP` environment variable changes that limit. A `github-oauth` token for `github.com` in Composer's `auth.json` applies to these requests and raises GitHub's rate limit. A failed optional lookup marks only -its own advisory as `unavailable` and does not discard the native finding or change its audit exit status. +its own advisory as `unavailable` and does not discard the finding itself or change its audit exit status. Bun can skip packages from a registry that does not answer its audit request while still returning a successful native status. Foxy therefore treats any diagnostic emitted by `bun audit --json` as an unreliable partial report and returns @@ -133,7 +133,7 @@ composer foxy:audit --no-dev --audit-level=high --format=summary The explicit command still runs when `run-asset-manager` is `false`; that setting controls automatic install and update execution, not a user-requested audit. Foxy must be enabled, the selected binary must satisfy its supported -version constraint, and the matching native lockfile must exist. The `native` manager has no binary, so only Foxy and +version constraint, and the manager's own lockfile must exist. The `native` manager has no binary, so only Foxy and `foxy.lock` are required for it. ## Library setup @@ -148,7 +148,7 @@ Use a runtime dependency when every consumer of the library must process its fro ```json { "require": { - "php-forge/foxy": "^0.3" + "php-forge/foxy": "^0.4" } } ``` @@ -160,7 +160,7 @@ Use a development dependency when Foxy is optional for library development: ```json { "require-dev": { - "php-forge/foxy": "^0.3" + "php-forge/foxy": "^0.4" } } ``` @@ -228,7 +228,8 @@ exclude a package that does. See [Package selection](config.md#package-selection - The `native` manager installs one version per package into a flat directory (`node_modules` unless `native-install-dir` says otherwise), verifies every tarball against its registry integrity value, and reinstalls from `foxy.lock` without registry metadata requests. -- Asset restoration occurs when package merging or manager execution fails and `fallback-asset` is enabled. +- Asset restoration occurs when package merging fails or manager execution throws or returns a non-zero result, and + `fallback-asset` is enabled. It runs before Composer restoration. - Composer lock and vendor restoration occurs after any solve exception or non-zero manager result when `fallback-composer` is enabled. - Read, write, and remove failures are reported through exceptions instead of being silently ignored. diff --git a/rector.php b/rector.php index e1d5c7e..0997b53 100644 --- a/rector.php +++ b/rector.php @@ -3,6 +3,7 @@ declare(strict_types=1); use Rector\Config\RectorConfig; +use Rector\Php74\Rector\If_\IfToNullCoalescingAssignRector; return static function (RectorConfig $rectorConfig): void { $rectorConfig->import(__DIR__ . '/vendor/php-forge/coding-standard/src/rector-83.php'); @@ -13,4 +14,11 @@ __DIR__ . '/tests', ], ); + + // `AuditCommand::sanitize()` keeps its explicit `null` check: the `??=` form yields a mutant Infection cannot kill. + $rectorConfig->skip( + [ + IfToNullCoalescingAssignRector::class => [__DIR__ . '/src/Command/AuditCommand.php'], + ], + ); }; diff --git a/src/Asset/AbstractAssetManager.php b/src/Asset/AbstractAssetManager.php index d9841ce..86394e7 100644 --- a/src/Asset/AbstractAssetManager.php +++ b/src/Asset/AbstractAssetManager.php @@ -11,8 +11,6 @@ use Foxy\Config\Config; use Foxy\Converter\{SemverConverter, VersionConverterInterface}; use Foxy\Exception\{Message, RuntimeException}; -use Foxy\Fallback\FallbackInterface; -use Throwable; use UnexpectedValueException; use function array_key_exists; @@ -31,10 +29,9 @@ public function __construct( Config $config, protected ProcessExecutor $executor, Filesystem $fs, - FallbackInterface|null $fallback = null, protected VersionConverterInterface|null $versionConverter = null, ) { - parent::__construct($io, $config, $fs, $fallback); + parent::__construct($io, $config, $fs); $this->versionConverter ??= new SemverConverter(); } @@ -44,21 +41,6 @@ public function __construct( */ abstract protected function getAuditCommand(bool $noDev): string; - /** - * Get the command to install the asset dependencies. - */ - abstract protected function getInstallCommand(): string; - - /** - * Get the command to update the asset dependencies. - */ - abstract protected function getUpdateCommand(): string; - - /** - * Get the command to retrieve the version. - */ - abstract protected function getVersionCommand(): string; - public function audit(bool $noDev): AuditProcessResult { if (!$this->hasLockFile()) { @@ -92,8 +74,6 @@ function () use ($noDev): AuditProcessResult { public function isAvailable(): bool { - $this->config->setResolvedManager($this->getName()); - return null !== $this->getVersion(); } @@ -112,36 +92,16 @@ public function run(): int $this->io->write($info); - $res = $this->withManagerTimeout( - function () use ($updatable, $managerWorkingDirectory): int { - try { - $cmd = $updatable ? $this->getUpdateCommand() : $this->getInstallCommand(); - - return $this->executeManagerCommand($cmd, $managerWorkingDirectory); - } catch (Throwable $exception) { - $this->restoreAfterFailure($exception); - - throw $exception; - } - }, + return $this->withManagerTimeout( + fn(): int => $this->executeManagerCommand( + $updatable ? $this->getUpdateCommand() : $this->getInstallCommand(), + $managerWorkingDirectory, + ), ); - - if (0 !== $res && null !== $this->fallback) { - $this->restoreAfterFailure( - new RuntimeException( - Message::ASSET_MANAGER_EXITED_WITH_STATUS->getMessage($res), - $res, - ), - ); - } - - return $res; } public function validate(): void { - $this->config->setResolvedManager($this->getName()); - $version = $this->getVersion(); if (null === $version) { @@ -173,7 +133,7 @@ public function validate(): void } /** @var string|null $constraintVersion */ - $constraintVersion = $this->config->get('manager-version'); + $constraintVersion = $this->getManagerConfig()->get('manager-version'); if ( is_string($constraintVersion) @@ -195,8 +155,8 @@ public function validate(): void */ protected function buildCommand(string $defaultBin, string $action, array|string $command): string { - $gOptions = trim((string) $this->config->get('manager-options', '')); - $options = trim((string) $this->config->get("manager-{$action}-options", '')); + $gOptions = trim((string) $this->getManagerConfig()->get('manager-options', '')); + $options = trim((string) $this->getManagerConfig()->get("manager-{$action}-options", '')); return $this->buildUnconfiguredCommand($defaultBin, $command) . ($gOptions === '' ? '' : " {$gOptions}") @@ -223,6 +183,32 @@ protected function getAuditEnvironment(): array return []; } + /** + * Returns the executable used when `manager-bin` is not configured; defaults to the manager name. + */ + protected function getDefaultBinary(): string + { + return $this->getName(); + } + + /** + * Returns the command that installs the asset dependencies, followed by the `manager-options` and + * `manager-install-options` values. + */ + protected function getInstallCommand(): string + { + return $this->buildCommand($this->getDefaultBinary(), 'install', 'install'); + } + + /** + * Returns the command that updates the asset dependencies, followed by the `manager-options` and + * `manager-update-options` values. + */ + protected function getUpdateCommand(): string + { + return $this->buildCommand($this->getDefaultBinary(), 'update', 'update'); + } + protected function getVersion(): string|null { if ($this->version === '' && $this->versionConverter !== null) { @@ -242,6 +228,14 @@ protected function getVersion(): string|null return $this->version; } + /** + * Returns the command that prints the manager version, without the install and update options. + */ + protected function getVersionCommand(): string + { + return $this->buildUnconfiguredCommand($this->getDefaultBinary(), '--version'); + } + /** * Normalizes the trimmed output of the version command before it is converted. */ @@ -270,7 +264,7 @@ final protected function withManagerTimeout(callable $callback): mixed $timeout = ProcessExecutor::getTimeout(); /** @var int $managerTimeout */ - $managerTimeout = $this->config->get('manager-timeout', PHP_INT_MAX); + $managerTimeout = $this->getManagerConfig()->get('manager-timeout', PHP_INT_MAX); ProcessExecutor::setTimeout($managerTimeout); @@ -302,7 +296,7 @@ private function executeManagerCommand(string $command, string|null $workingDire private function getManagerBinary(string $defaultBin): string { /** @var string $bin */ - $bin = $this->config->get('manager-bin', $defaultBin); + $bin = $this->getManagerConfig()->get('manager-bin', $defaultBin); return Platform::isWindows() ? str_replace('/', '\\', $bin) : $bin; } diff --git a/src/Asset/AbstractManifestAssetManager.php b/src/Asset/AbstractManifestAssetManager.php index b05d530..462e466 100644 --- a/src/Asset/AbstractManifestAssetManager.php +++ b/src/Asset/AbstractManifestAssetManager.php @@ -10,10 +10,8 @@ use Exception; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; -use Foxy\Fallback\FallbackInterface; use Foxy\Json\JsonFile; use Seld\JsonLint\ParsingException; -use Throwable; use function is_dir; use function is_string; @@ -24,8 +22,8 @@ use const DIRECTORY_SEPARATOR; /** - * Provides the asset manifest handling shared by every asset manager: dependency merging, root package paths, lock - * file detection, and fallback restoration after a failure. + * Provides the asset manifest handling shared by every asset manager: dependency merging, root package paths, and lock + * file detection. */ abstract class AbstractManifestAssetManager implements AssetManagerInterface { @@ -37,7 +35,6 @@ public function __construct( protected IOInterface $io, protected Config $config, protected Filesystem $fs, - protected FallbackInterface|null $fallback = null, ) {} /** @@ -45,32 +42,26 @@ public function __construct( */ public function addDependencies(RootPackageInterface $rootPackage, array $dependencies): AssetPackageInterface { - try { - $assetPackage = new AssetPackage( - $rootPackage, - new JsonFile($this->getPackageJsonPath(), null, $this->io), - ); - - $previousDependencies = $assetPackage->getInstalledDependencies(); + $assetPackage = new AssetPackage( + $rootPackage, + new JsonFile($this->getPackageJsonPath(), null, $this->io), + ); - $assetPackage->removeUnusedDependencies($dependencies); + $previousDependencies = $assetPackage->getInstalledDependencies(); - $alreadyInstalledDependencies = $assetPackage->addNewDependencies($dependencies); + $assetPackage->removeUnusedDependencies($dependencies); - if ($this->config->isEnabled('run-asset-manager')) { - $this->actionWhenComposerDependenciesAreAlreadyInstalled($alreadyInstalledDependencies); - } + $alreadyInstalledDependencies = $assetPackage->addNewDependencies($dependencies); - $this->actionWhenComposerDependenciesAreMerged($assetPackage, $previousDependencies); + if ($this->config->isEnabled('run-asset-manager')) { + $this->actionWhenComposerDependenciesAreAlreadyInstalled($alreadyInstalledDependencies); + } - $this->io->write('Merging Composer dependencies in the asset package'); + $this->actionWhenComposerDependenciesAreMerged($assetPackage, $previousDependencies); - return $assetPackage->write(); - } catch (Throwable $exception) { - $this->restoreAfterFailure($exception); + $this->io->write('Merging Composer dependencies in the asset package'); - throw $exception; - } + return $assetPackage->write(); } public function getPackageJsonPath(): string @@ -98,13 +89,6 @@ public function isUpdatable(): bool return $this->updatable && $this->isInstalled(); } - public function setFallback(FallbackInterface $fallback): static - { - $this->fallback = $fallback; - - return $this; - } - public function setUpdatable(bool $updatable): static { $this->updatable = $updatable; @@ -138,6 +122,16 @@ protected function getLockFilePath(): string return $this->getRootPackagePath($this->getLockPackageName()); } + /** + * Returns the configuration scoped to this manager, so `manager-*` map entries resolve to its own value. + * + * The view is built on each call because {@see getName()} must not run from a constructor. + */ + protected function getManagerConfig(): Config + { + return $this->config->forManager($this->getName()); + } + protected function getManagerWorkingDirectory(): string|null { if (null === $this->getConfiguredRootPackageDir()) { @@ -188,25 +182,6 @@ protected function getRootPackagePath(string $path): string return rtrim($this->getRootPackageDir(), '/\\') . DIRECTORY_SEPARATOR . $path; } - /** - * Restore the asset manifest without hiding the manager failure. - */ - final protected function restoreAfterFailure(Throwable $exception): void - { - if (null === $this->fallback) { - return; - } - - try { - $this->fallback->restore(); - } catch (Throwable $fallbackException) { - throw new RuntimeException( - Message::ASSET_MANAGER_FALLBACK_RESTORE_FAILED->getMessage($fallbackException->getMessage()), - previous: $exception, - ); - } - } - /** * Returns the configured `root-package-json-dir` value, or `null` when it is unset or empty. */ diff --git a/src/Asset/AssetManagerInterface.php b/src/Asset/AssetManagerInterface.php index 18cb883..af22732 100644 --- a/src/Asset/AssetManagerInterface.php +++ b/src/Asset/AssetManagerInterface.php @@ -7,7 +7,6 @@ use Composer\Package\RootPackageInterface; use Foxy\Audit\{AuditParserInterface, AuditProcessResult}; use Foxy\Exception\RuntimeException; -use Foxy\Fallback\FallbackInterface; interface AssetManagerInterface { @@ -78,11 +77,6 @@ public function isUpdatable(): bool; */ public function run(): int; - /** - * @param FallbackInterface $fallback The fallback - */ - public function setFallback(FallbackInterface $fallback): self; - /** * Define if the asset manager can be use the update command. * diff --git a/src/Asset/BunManager.php b/src/Asset/BunManager.php index 69707c4..8673872 100644 --- a/src/Asset/BunManager.php +++ b/src/Asset/BunManager.php @@ -73,22 +73,15 @@ protected function getAuditCommand(bool $noDev): string $command[] = '--prod'; } - return $this->buildUnconfiguredCommand($this->getBinary(), $command); + return $this->buildUnconfiguredCommand($this->getDefaultBinary(), $command); } - protected function getInstallCommand(): string - { - return $this->buildCommand($this->getBinary(), 'install', 'install'); - } - - protected function getUpdateCommand(): string - { - return $this->buildCommand($this->getBinary(), 'update', 'update'); - } - - protected function getVersionCommand(): string + /** + * Returns the platform-specific Bun binary name. + */ + protected function getDefaultBinary(): string { - return $this->buildUnconfiguredCommand($this->getBinary(), '--version'); + return Platform::isWindows() ? 'bun.exe' : 'bun'; } protected function validateAuditConfiguration(bool $noDev): void @@ -144,14 +137,6 @@ private function getAuditEnvironmentValue(string $name): string|null return null; } - /** - * Returns the platform-specific Bun binary name. - */ - private function getBinary(): string - { - return Platform::isWindows() ? 'bun.exe' : 'bun'; - } - /** * @return list */ diff --git a/src/Asset/DenoManager.php b/src/Asset/DenoManager.php index 09dae68..0bbca30 100644 --- a/src/Asset/DenoManager.php +++ b/src/Asset/DenoManager.php @@ -98,7 +98,7 @@ protected function actionWhenComposerDependenciesAreMerged( protected function getAuditCommand(bool $noDev): string { - return $this->buildUnconfiguredCommand($this->getBinary(), ['audit', '--level=low']); + return $this->buildUnconfiguredCommand($this->getDefaultBinary(), ['audit', '--level=low']); } protected function getAuditEnvironment(): array @@ -106,23 +106,21 @@ protected function getAuditEnvironment(): array return self::AUDIT_ENVIRONMENT; } - protected function getInstallCommand(): string + /** + * Returns the platform-specific Deno binary name. + */ + protected function getDefaultBinary(): string { - return $this->buildCommand($this->getBinary(), 'install', 'install'); + return Platform::isWindows() ? 'deno.exe' : 'deno'; } protected function getUpdateCommand(): string { - return $this->buildCommand($this->getBinary(), 'update', ['update', '--lockfile-only', '--recursive']) + return $this->buildCommand($this->getDefaultBinary(), 'update', ['update', '--lockfile-only', '--recursive']) . ' && ' . $this->getInstallCommand(); } - protected function getVersionCommand(): string - { - return $this->buildUnconfiguredCommand($this->getBinary(), '--version'); - } - protected function normalizeVersionOutput(string $output): string { return 1 === preg_match('/^deno (\S+)/', $output, $matches) ? $matches[1] : $output; @@ -137,14 +135,6 @@ protected function validateAuditConfiguration(bool $noDev): void } } - /** - * Returns the platform-specific Deno binary name. - */ - private function getBinary(): string - { - return Platform::isWindows() ? 'deno.exe' : 'deno'; - } - /** * Returns the normalized workspace member path of a `file:` dependency relative to the root package directory. */ diff --git a/src/Asset/NativeManager.php b/src/Asset/NativeManager.php index 132e516..9645682 100644 --- a/src/Asset/NativeManager.php +++ b/src/Asset/NativeManager.php @@ -10,9 +10,7 @@ use Foxy\Audit\Parser\NativeAuditParser; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; -use Foxy\Fallback\FallbackInterface; use Foxy\Native\{LockFile, NativeInstallerInterface, NpmRegistryInterface}; -use Throwable; use function array_unshift; use function basename; @@ -47,7 +45,6 @@ final class NativeManager extends AbstractManifestAssetManager * @param Filesystem $fs Filesystem used by the manifest handling. * @param NativeInstallerInterface $installer Installer that populates the install directory. * @param NpmRegistryInterface $registry Registry that answers the security audit. - * @param FallbackInterface|null $fallback Fallback restored when the installation fails, or `null` for none. */ public function __construct( IOInterface $io, @@ -55,9 +52,8 @@ public function __construct( Filesystem $fs, private readonly NativeInstallerInterface $installer, private readonly NpmRegistryInterface $registry, - FallbackInterface|null $fallback = null, ) { - parent::__construct($io, $config, $fs, $fallback); + parent::__construct($io, $config, $fs); } /** @@ -122,12 +118,10 @@ public function getVersionConstraint(): string } /** - * Records `native` as the resolved manager and returns whether `foxy.lock` exists. + * Returns whether `foxy.lock` exists; the native manager needs no executable. */ public function isAvailable(): bool { - $this->config->setResolvedManager($this->getName()); - return $this->hasLockFile(); } @@ -146,12 +140,11 @@ public function isInstalled(): bool * Installs the dependencies, or updates them when updates are allowed by {@see setUpdatable()}, as `composer update` * does, even if nothing is installed yet. * - * Returns `0` without doing anything when `run-asset-manager` is disabled. The fallback is restored when the - * installation fails, and the installer exception is rethrown. + * Returns `0` without doing anything when `run-asset-manager` is disabled. An installer exception propagates + * unchanged. * * @throws RuntimeException if the `zlib` extension is missing, the configured root package directory does not - * exist, the install directory is invalid, the installation fails, or the fallback cannot be restored after a - * failure. + * exist, the install directory is invalid, or the installation fails. */ public function run(): int { @@ -172,18 +165,12 @@ public function run(): int ), ); - try { - $this->installer->install( - $this->getPackageJsonPath(), - $this->getLockFilePath(), - $installDirectory, - $updatable, - ); - } catch (Throwable $exception) { - $this->restoreAfterFailure($exception); - - throw $exception; - } + $this->installer->install( + $this->getPackageJsonPath(), + $this->getLockFilePath(), + $installDirectory, + $updatable, + ); return 0; } diff --git a/src/Asset/NpmManager.php b/src/Asset/NpmManager.php index feefda3..37f3511 100644 --- a/src/Asset/NpmManager.php +++ b/src/Asset/NpmManager.php @@ -71,22 +71,7 @@ protected function getAuditCommand(bool $noDev): string $command = [...$command, $noDev ? '--omit=dev' : '--include=dev', '--include=optional', '--include=peer']; - return $this->buildUnconfiguredCommand('npm', $command); - } - - protected function getInstallCommand(): string - { - return $this->buildCommand('npm', 'install', 'install'); - } - - protected function getUpdateCommand(): string - { - return $this->buildCommand('npm', 'update', 'update'); - } - - protected function getVersionCommand(): string - { - return $this->buildUnconfiguredCommand('npm', '--version'); + return $this->buildUnconfiguredCommand($this->getDefaultBinary(), $command); } /** diff --git a/src/Asset/PnpmManager.php b/src/Asset/PnpmManager.php index 99263bf..e5f6b6d 100644 --- a/src/Asset/PnpmManager.php +++ b/src/Asset/PnpmManager.php @@ -51,21 +51,6 @@ protected function getAuditCommand(bool $noDev): string ProcessExecutor::escape('--config.auditConfig={ignoreGhsas:[]}'), ]; - return $this->buildUnconfiguredCommand('pnpm', $command); - } - - protected function getInstallCommand(): string - { - return $this->buildCommand('pnpm', 'install', 'install'); - } - - protected function getUpdateCommand(): string - { - return $this->buildCommand('pnpm', 'update', 'update'); - } - - protected function getVersionCommand(): string - { - return $this->buildUnconfiguredCommand('pnpm', '--version'); + return $this->buildUnconfiguredCommand($this->getDefaultBinary(), $command); } } diff --git a/src/Asset/YarnManager.php b/src/Asset/YarnManager.php index e552d76..22ffa26 100644 --- a/src/Asset/YarnManager.php +++ b/src/Asset/YarnManager.php @@ -52,7 +52,7 @@ protected function getAuditCommand(bool $noDev): string $command = [...$command, '--environment', 'production']; } - return $this->buildUnconfiguredCommand('yarn', $command); + return $this->buildUnconfiguredCommand($this->getDefaultBinary(), $command); } protected function getAuditEnvironment(): array @@ -60,18 +60,8 @@ protected function getAuditEnvironment(): array return self::AUDIT_ENVIRONMENT; } - protected function getInstallCommand(): string - { - return $this->buildCommand('yarn', 'install', 'install'); - } - protected function getUpdateCommand(): string { - return $this->buildCommand('yarn', 'update', 'up'); - } - - protected function getVersionCommand(): string - { - return $this->buildUnconfiguredCommand('yarn', '--version'); + return $this->buildCommand($this->getDefaultBinary(), 'update', 'up'); } } diff --git a/src/Command/AuditCommand.php b/src/Command/AuditCommand.php index c998c93..be44727 100644 --- a/src/Command/AuditCommand.php +++ b/src/Command/AuditCommand.php @@ -138,7 +138,10 @@ function (string $warning): void { private function sanitize(string $value): string { $sanitized = preg_replace('/[\x00-\x1F\x7F]+/u', ' ', $value); - $sanitized ??= preg_replace('/[\x00-\x1F\x7F]+/', ' ', $value) ?? $value; + + if (null === $sanitized) { + $sanitized = preg_replace('/[\x00-\x1F\x7F]+/', ' ', $value) ?? $value; + } return trim($sanitized); } diff --git a/src/Config/Config.php b/src/Config/Config.php index 4b38fb8..d5056e0 100644 --- a/src/Config/Config.php +++ b/src/Config/Config.php @@ -23,7 +23,7 @@ class Config { private array $cacheEnv = []; - private string|null $resolvedManager = null; + private string|null $scopedManager = null; /** * @param array $config The config. @@ -31,6 +31,24 @@ class Config */ public function __construct(private readonly array $config, private readonly array $defaults = []) {} + /** + * Returns a copy whose `manager-*` map entries resolve for the given manager. + * + * The current instance is never modified; environment values are returned unscoped. + * + * @param string $manager The manager name used as the key of `manager-*` maps. + * + * @return static The manager-scoped copy. + */ + public function forManager(string $manager): static + { + $view = clone $this; + + $view->scopedManager = $manager; + + return $view; + } + /** * Get the config value. * @@ -83,14 +101,6 @@ public function isEnabled(string $key): bool return true === $value || 1 === $value || '1' === $value; } - /** - * Set the manager selected during automatic discovery. - */ - public function setResolvedManager(string $manager): void - { - $this->resolvedManager = $manager; - } - /** * Convert the value of environment variable into a boolean. * @@ -172,7 +182,7 @@ private function getByManager(string $key, mixed $value, mixed $default = null): { if (str_starts_with($key, 'manager-') && is_array($value)) { /** @var int|string $manager */ - $manager = $this->resolvedManager ?? $this->get('manager', ''); + $manager = $this->scopedManager ?? $this->get('manager', ''); $value = array_key_exists($manager, $value) ? $value[$manager] diff --git a/src/Exception/Message.php b/src/Exception/Message.php index 46071e0..f2e1560 100644 --- a/src/Exception/Message.php +++ b/src/Exception/Message.php @@ -129,20 +129,6 @@ enum Message: string */ case ASSET_MANAGER_BINARY_NOT_INSTALLED = 'The "%s" binary must be installed.'; - /** - * Error when the asset manager exits with a non-zero status code. - * - * Format: "The asset manager exited with status code %d." - */ - case ASSET_MANAGER_EXITED_WITH_STATUS = 'The asset manager exited with status code %d.'; - - /** - * Error when restoring the fallback after an asset manager failure also fails. - * - * Format: "The asset manager failed and its fallback could not be restored: %s" - */ - case ASSET_MANAGER_FALLBACK_RESTORE_FAILED = 'The asset manager failed and its fallback could not be restored: %s'; - /** * Error when an audit is requested without the manager lock file. * @@ -892,6 +878,13 @@ enum Message: string */ case SOLVER_ASSET_DIR_UNMANAGED = 'The Composer asset directory "%s" is not marked as managed by Foxy.'; + /** + * Error when restoring the asset fallback after an asset manager failure also fails. + * + * Format: "The asset manager failed and its fallback could not be restored: %s" + */ + case SOLVER_ASSET_FALLBACK_RESTORE_FAILED = 'The asset manager failed and its fallback could not be restored: %s'; + /** * Error when the asset manager ends with a non-zero code. * diff --git a/src/Foxy.php b/src/Foxy.php index 2c80388..e709fc3 100644 --- a/src/Foxy.php +++ b/src/Foxy.php @@ -7,7 +7,6 @@ use Composer\{Cache, Composer}; use Composer\DependencyResolver\Operation\InstallOperation; use Composer\EventDispatcher\EventSubscriberInterface; -use Composer\Factory; use Composer\Installer\{InstallerEvents, PackageEvent, PackageEvents}; use Composer\IO\IOInterface; use Composer\Plugin\Capability\CommandProvider as ComposerCommandProvider; @@ -45,6 +44,8 @@ final class Foxy implements PluginInterface, EventSubscriberInterface, Capable, private AssetManagerInterface $assetManager; + private AssetManagerFinder $assetManagerFinder; + /** * @var list> The list of the classes of asset managers. */ @@ -81,8 +82,12 @@ final class Foxy implements PluginInterface, EventSubscriberInterface, Capable, 'native-install-dir' => 'node_modules', ]; + private Filesystem $fs; + private bool $initialized = false; + private IOInterface $io; + private SolverInterface $solver; /** @@ -103,17 +108,10 @@ public function activate(Composer $composer, IOInterface $io): void return; } - $this->assetManager = $this->getAssetManager($composer, $io, $this->config, $executor, $fs); - - $packageJsonPath = $this->assetManager instanceof AbstractManifestAssetManager - ? $this->assetManager->getPackageJsonPath() - : $this->assetManager->getPackageName(); - - $this->assetFallback = new AssetFallback($io, $this->config, $packageJsonPath, $fs); + $this->io = $io; + $this->fs = $fs; + $this->assetManagerFinder = $this->createAssetManagerFinder($composer, $io, $this->config, $executor, $fs); $this->composerFallback = new ComposerFallback($composer, $io, $this->config, $input, $fs); - $this->solver = new Solver($this->assetManager, $this->config, $fs, $this->composerFallback); - - $this->assetManager->setFallback($this->assetFallback); } public function audit(AuditRequest $request): AuditReport @@ -124,7 +122,7 @@ public function audit(AuditRequest $request): AuditReport ); } - return (new AuditRunner($this->assetManager))->audit($request); + return (new AuditRunner($this->getAssetManager()))->audit($request); } public function deactivate(Composer $composer, IOInterface $io): void @@ -162,11 +160,11 @@ public function init(): void return; } - $this->assetFallback->save(); + $this->getAssetFallback()->save(); $this->composerFallback->save(); if ($this->isEnabled('run-asset-manager')) { - $this->assetManager->validate(); + $this->getAssetManager()->validate(); } } @@ -201,8 +199,10 @@ public function solveAssets(Event $event): void return; } - $this->solver->setUpdatable(str_contains($event->getName(), 'update')); - $this->solver->solve($event->getComposer(), $event->getIO()); + $solver = $this->getSolver(); + + $solver->setUpdatable(str_contains($event->getName(), 'update')); + $solver->solve($event->getComposer(), $event->getIO()); } public function uninstall(Composer $composer, IOInterface $io): void @@ -211,10 +211,37 @@ public function uninstall(Composer $composer, IOInterface $io): void } /** - * Returns the native manager, wired to the npm registry with Composer's HTTP settings, authentication, and - * `/foxy` tarball cache. Performs no I/O. + * Returns the finder holding every built-in manager; builds the managers without probing any executable. * - * @param Composer $composer The Composer instance that provides the HTTP and cache settings. + * @param Composer $composer The Composer instance. + * @param IOInterface $io The IO interface. + * @param Config $config The config of plugin. + * @param ProcessExecutor $executor The process executor. + * @param Filesystem $fs The composer filesystem. + */ + private function createAssetManagerFinder( + Composer $composer, + IOInterface $io, + Config $config, + ProcessExecutor $executor, + Filesystem $fs, + ): AssetManagerFinder { + $amf = new AssetManagerFinder(); + + foreach (self::$assetManagers as $class) { + $amf->addManager(new $class($io, $config, $executor, $fs)); + } + + $amf->addManager($this->createNativeManager($composer, $io, $config, $fs)); + + return $amf; + } + + /** + * Returns the native manager, wired to the npm registry through the HTTP downloader of Composer's loop (its HTTP + * settings, authentication, and parallel downloads) and the `/foxy` tarball cache. Performs no I/O. + * + * @param Composer $composer The Composer instance that provides the HTTP downloader and the cache settings. * @param IOInterface $io The IO interface. * @param Config $config The config of plugin. * @param Filesystem $fs The composer filesystem. @@ -232,7 +259,7 @@ private function createNativeManager( $cache->setReadOnly(true === $composerConfig->get('cache-read-only')); $registry = new NpmRegistry( - Factory::createHttpDownloader($io, $composerConfig), + $composer->getLoop()->getHttpDownloader(), $cache, $io, (string) $config->get('registry-url'), @@ -248,33 +275,57 @@ private function createNativeManager( } /** - * @param Composer $composer The Composer instance. - * @param IOInterface $io The IO interface. - * @param Config $config The config of plugin. - * @param ProcessExecutor $executor The process executor. - * @param Filesystem $fs The composer filesystem. + * Returns the asset fallback of the resolved manager, resolving the manager first. * - * @throws RuntimeException When the asset manager is not found. + * @throws RuntimeException if the asset manager cannot be resolved. */ - private function getAssetManager( - Composer $composer, - IOInterface $io, - Config $config, - ProcessExecutor $executor, - Filesystem $fs, - ): AssetManagerInterface { - $amf = new AssetManagerFinder(); + private function getAssetFallback(): AssetFallback + { + $this->getAssetManager(); - foreach (self::$assetManagers as $class) { - $amf->addManager(new $class($io, $config, $executor, $fs)); - } + return $this->assetFallback; + } - $amf->addManager($this->createNativeManager($composer, $io, $config, $fs)); + /** + * Resolves the asset manager on first use and builds its asset fallback; later calls return the same manager. + * + * The manager is assigned last, so a selection failure leaves nothing resolved and the next call retries. + * + * @throws RuntimeException if the configured manager is unknown or no manager can be selected. + */ + private function getAssetManager(): AssetManagerInterface + { + if (!isset($this->assetManager)) { + /** @var string|null $name */ + $name = $this->config->get('manager'); + + $manager = $this->assetManagerFinder->findManager($name, $this->isEnabled('run-asset-manager')); - /** @var string|null $manager */ - $manager = $config->get('manager'); + $path = $manager instanceof AbstractManifestAssetManager + ? $manager->getPackageJsonPath() + : $manager->getPackageName(); + + $this->assetFallback = new AssetFallback($this->io, $this->config, $path, $this->fs); + $this->assetManager = $manager; + } - return $amf->findManager($manager, $this->isEnabled('run-asset-manager')); + return $this->assetManager; + } + + /** + * Returns the solver, built on first use with the resolved manager and both fallbacks unless one was injected. + * + * @throws RuntimeException if the asset manager cannot be resolved. + */ + private function getSolver(): SolverInterface + { + return $this->solver ??= new Solver( + $this->getAssetManager(), + $this->config, + $this->fs, + $this->composerFallback, + $this->getAssetFallback(), + ); } /** diff --git a/src/Solver/Solver.php b/src/Solver/Solver.php index fac0da2..c5c114e 100644 --- a/src/Solver/Solver.php +++ b/src/Solver/Solver.php @@ -42,12 +42,15 @@ * @param AssetManagerInterface $assetManager The asset manager instance. * @param Config $config The config instance. * @param FallbackInterface|null $composerFallback The composer fallback instance. + * @param FallbackInterface|null $assetFallback Fallback restored when the asset manager fails to merge the + * dependencies, throws while running, or returns a non-zero code, or `null` for none. */ public function __construct( private AssetManagerInterface $assetManager, private Config $config, private Filesystem $fs, private FallbackInterface|null $composerFallback = null, + private FallbackInterface|null $assetFallback = null, ) {} public function setUpdatable($updatable): self @@ -87,9 +90,8 @@ public function solve(Composer $composer, IOInterface $io): void $dispatcher->dispatch(FoxyEvents::PRE_SOLVE, new PreSolveEvent($assetDir, $packages)); $this->prepareAssetDirectory($assetDir, $vendorDir); $assets = $this->getAssets($composer, $assetDir, $packages); - $this->assetManager->addDependencies($composer->getPackage(), $assets); - $res = $this->assetManager->run(); + $res = $this->runAssetManager($composer, $assets); $dispatcher->dispatch(FoxyEvents::POST_SOLVE, new PostSolveEvent($assetDir, $packages, $res)); @@ -332,6 +334,28 @@ private function resolveCanonicalPath(string $existingPath, string $path, array ); } + /** + * Restores the asset fallback without hiding the asset manager failure. + * + * @throws RuntimeException if the restoration fails, with the asset manager failure as the previous exception. + */ + private function restoreAssetAfterFailure(Throwable $exception): void + { + if (null === $this->assetFallback) { + return; + } + + try { + $this->assetFallback->restore(); + } catch (Throwable $fallbackException) { + throw new RuntimeException( + Message::SOLVER_ASSET_FALLBACK_RESTORE_FAILED->getMessage($fallbackException->getMessage()), + 0, + $exception, + ); + } + } + /** * Restore Composer state without hiding the original asset failure. */ @@ -354,6 +378,37 @@ private function restoreComposerAfterFailure(Throwable $exception): never throw $exception; } + /** + * Merges the asset dependencies and runs the asset manager, restoring the asset fallback when either step throws + * or the manager returns a non-zero code. + * + * @param array $assets The asset dependencies to merge into the asset package. + * + * @return int The asset manager result code. + */ + private function runAssetManager(Composer $composer, array $assets): int + { + try { + $this->assetManager->addDependencies($composer->getPackage(), $assets); + + $res = $this->assetManager->run(); + } catch (Throwable $exception) { + $this->restoreAssetAfterFailure($exception); + + throw $exception; + } + + if (0 !== $res) { + $this->restoreAssetAfterFailure( + new RuntimeException( + Message::SOLVER_ASSET_MANAGER_FAILED->getMessage($res), + ), + ); + } + + return $res; + } + /** * Canonicalize and validate a directory before any recursive removal. */ diff --git a/tests/Asset/AbstractAssetManagerTest.php b/tests/Asset/AbstractAssetManagerTest.php index 334806a..f4752dc 100644 --- a/tests/Asset/AbstractAssetManagerTest.php +++ b/tests/Asset/AbstractAssetManagerTest.php @@ -9,8 +9,8 @@ use Composer\Util\{Filesystem, ProcessExecutor}; use Foxy\Config\Config; use Foxy\Converter\VersionConverterInterface; -use Foxy\Fallback\FallbackInterface; -use Foxy\Tests\Fixtures\Asset\InspectableAssetManager; +use Foxy\Exception\Message; +use Foxy\Tests\Fixtures\Asset\{CustomBinaryAssetManager, InspectableAssetManager}; use Foxy\Tests\Fixtures\Util\ProcessExecutorMock; use Foxy\Tests\Provider\AbstractAssetManagerProvider; use PHPUnit\Framework\Attributes\{DataProviderExternal, PreserveGlobalState, RunInSeparateProcess}; @@ -42,7 +42,6 @@ final class AbstractAssetManagerTest extends TestCase private Config|null $config = null; private string|null $cwd = null; private ProcessExecutorMock|null $executor = null; - private FallbackInterface|MockObject|null $fallback = null; private Filesystem|MockObject|null $fs = null; private IOInterface|MockObject|null $io = null; private string|null $oldCwd = null; @@ -89,35 +88,6 @@ public function testActionHookRemainsExtensible(): void ); } - public function testAddDependenciesPropagatesFailureWithoutFallback(): void - { - file_put_contents($this->cwd . DIRECTORY_SEPARATOR . 'package.json', 'invalid json'); - - $manager = new InspectableAssetManager( - $this->io, - $this->config, - $this->executor, - $this->fs, - ); - - $this->expectException(ParsingException::class); - - $manager->addDependencies($this->rootPackage, []); - } - - public function testAddDependenciesRestoresFallbackAfterFailure(): void - { - file_put_contents($this->cwd . DIRECTORY_SEPARATOR . 'package.json', 'invalid json'); - - $this->fallback - ->expects(self::once()) - ->method('restore'); - - $this->expectException(ParsingException::class); - - $this->createManager()->addDependencies($this->rootPackage, []); - } - public function testAuditCapturesStandardAndErrorOutputWithoutStreaming(): void { file_put_contents($this->cwd . DIRECTORY_SEPARATOR . 'inspectable.lock', '{}'); @@ -166,7 +136,7 @@ static function (mixed $command, mixed &$output = null, mixed $cwd = null) use ( ->expects(self::never()) ->method('writeErrorRaw'); - $manager = new InspectableAssetManager($this->io, $this->config, $executor, $this->fs, $this->fallback); + $manager = new InspectableAssetManager($this->io, $this->config, $executor, $this->fs); $result = $manager->audit(true); @@ -253,7 +223,6 @@ static function (mixed $command, mixed &$output = null) use (&$position, $name): $this->config, $executor, $this->fs, - $this->fallback, ); $manager->setAuditEnvironmentForTest([$name => 'audit-value']); @@ -322,8 +291,6 @@ static function (mixed $command, mixed &$output = null) use ( throw new RuntimeException('Audit execution failed.'); }, ); - $this->fallback->expects(self::never())->method('restore'); - try { ProcessExecutor::setTimeout($expectedTimeout); @@ -332,7 +299,6 @@ static function (mixed $command, mixed &$output = null) use ( $this->config, $executor, $this->fs, - $this->fallback, ); try { @@ -398,6 +364,35 @@ public function testBuildCommandUsesTrimmedGlobalAndActionOptions(): void ); } + public function testDefaultCommandsUseDefaultBinary(): void + { + $this->config = new Config( + [ + 'manager-options' => '--global', + 'manager-install-options' => '--frozen', + 'manager-update-options' => '--latest', + ], + ); + + $manager = new CustomBinaryAssetManager($this->io, $this->config, $this->executor, $this->fs); + + self::assertSame( + 'custom-bin install --global --frozen', + $manager->getInstallCommandForTest(), + 'Install command must start with the default binary and append install options.', + ); + self::assertSame( + 'custom-bin update --global --latest', + $manager->getUpdateCommandForTest(), + 'Update command must start with the default binary and append update options.', + ); + self::assertSame( + 'custom-bin --version', + $manager->getVersionCommandForTest(), + 'Version command must use the default binary without options.', + ); + } + public function testInjectedVersionConverterReceivesTrimmedVersion(): void { $converter = $this->createMock(VersionConverterInterface::class); @@ -463,18 +458,84 @@ public function testIsAvailableResolvesManagerSpecificConfiguration(): void $this->executor->addExpectedValues(0, '42.0.0'); + $manager = $this->createManager(); + + self::assertTrue( + $manager->isAvailable(), + 'The manager must accept its configured version constraint.', + ); self::assertNull( $this->config->get('manager-version'), - 'Manager-specific configuration must not alter the global version.', + 'Probing must not scope the shared configuration.', ); - self::assertTrue( - $this->createManager()->isAvailable(), - 'The manager must accept its configured version constraint.', + + $manager->validate(); + } + + public function testManagerBinOverridesDefaultBinary(): void + { + $this->config = new Config(['manager-bin' => ['custom' => 'configured-bin']]); + + $manager = new CustomBinaryAssetManager($this->io, $this->config, $this->executor, $this->fs); + + self::assertSame( + 'configured-bin install', + $manager->getInstallCommandForTest(), + 'Configured binary must replace the default in the install command.', ); self::assertSame( - '>=42.0.0', - $this->config->get('manager-version'), - 'The manager-specific version must be resolved.', + 'configured-bin update', + $manager->getUpdateCommandForTest(), + 'Configured binary must replace the default in the update command.', + ); + self::assertSame( + 'configured-bin --version', + $manager->getVersionCommandForTest(), + 'Configured binary must replace the default in the version command.', + ); + } + + public function testManagersSharingConfigurationReadTheirOwnEntries(): void + { + $this->config = new Config(['manager-bin' => ['custom' => 'bin-b', 'inspectable' => 'bin-a']]); + + for ($i = 0; $i < 4; $i++) { + $this->executor->addExpectedValues(0, '1.0.0'); + } + + $inspectable = $this->createManager(); + + $custom = new CustomBinaryAssetManager($this->io, $this->config, $this->executor, $this->fs); + + $inspectable->isAvailable(); + $custom->isAvailable(); + + self::assertSame( + 'bin-a install', + $inspectable->buildCommandForTest('inspectable', 'install', 'install'), + 'First manager must keep its own entry after the second probe.', + ); + + $custom = new CustomBinaryAssetManager($this->io, $this->config, $this->executor, $this->fs); + $inspectable = $this->createManager(); + + $custom->isAvailable(); + $inspectable->isAvailable(); + + self::assertSame( + 'bin-b install', + $custom->getInstallCommandForTest(), + 'Second manager must keep its own entry after the first probe.', + ); + self::assertSame( + ['bin-a --version', 'bin-b --version', 'bin-b --version', 'bin-a --version'], + [ + $this->executor->getExecutedCommand(0), + $this->executor->getExecutedCommand(1), + $this->executor->getExecutedCommand(2), + $this->executor->getExecutedCommand(3), + ], + 'Each probe must run its own configured binary.', ); } @@ -705,7 +766,7 @@ static function (mixed $command, mixed &$output = null, mixed $cwd = null) use ( ->method('writeErrorRaw') ->with('error output', false); - $manager = new InspectableAssetManager($this->io, $this->config, $executor, $this->fs, $this->fallback); + $manager = new InspectableAssetManager($this->io, $this->config, $executor, $this->fs); self::assertSame( 0, @@ -714,6 +775,36 @@ static function (mixed $command, mixed &$output = null, mixed $cwd = null) use ( ); } + public function testThrowParsingExceptionWhenPackageJsonIsInvalid(): void + { + file_put_contents($this->cwd . DIRECTORY_SEPARATOR . 'package.json', 'invalid json'); + + $manager = new InspectableAssetManager( + $this->io, + $this->config, + $this->executor, + $this->fs, + ); + + $this->expectException(ParsingException::class); + + $manager->addDependencies($this->rootPackage, []); + } + + public function testThrowRuntimeExceptionWhenManagerSpecificVersionConstraintIsNotSatisfied(): void + { + $this->config = new Config([], ['manager-version' => ['inspectable' => '>=42.0.0']]); + + $this->executor->addExpectedValues(0, '41.0.0'); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + Message::ASSET_VERSION_CONSTRAINT_MISMATCH->getMessage('inspectable', '41.0.0', '>=42.0.0'), + ); + + $this->createManager()->validate(); + } + public function testVersionCommandUsesConfiguredRootDirectory(): void { $rootPackageDir = $this->cwd . DIRECTORY_SEPARATOR . 'web'; @@ -740,7 +831,7 @@ static function (mixed $command, mixed &$output = null, mixed $cwd = null) use ( }, ); - $manager = new InspectableAssetManager($this->io, $this->config, $executor, $this->fs, $this->fallback); + $manager = new InspectableAssetManager($this->io, $this->config, $executor, $this->fs); $manager->validate(); @@ -804,7 +895,6 @@ protected function setUp(): void $this->io = $this->createMock(IOInterface::class); $this->executor = new ProcessExecutorMock($this->io); $this->fs = $this->createMock(Filesystem::class); - $this->fallback = $this->createMock(FallbackInterface::class); $this->rootPackage = $this->createMock(RootPackageInterface::class); $this->rootPackage->method('getLicense')->willReturn([]); $this->sfs = new SymfonyFilesystem(); @@ -825,7 +915,6 @@ protected function tearDown(): void $this->config = null; $this->cwd = null; $this->executor = null; - $this->fallback = null; $this->fs = null; $this->io = null; $this->oldCwd = null; @@ -841,7 +930,6 @@ private function createManager( $this->config, $this->executor, $this->fs, - $this->fallback, $versionConverter, ); } diff --git a/tests/Asset/AssetManager.php b/tests/Asset/AssetManager.php index 07a8230..d0f1d2f 100644 --- a/tests/Asset/AssetManager.php +++ b/tests/Asset/AssetManager.php @@ -11,7 +11,6 @@ use Foxy\Asset\{AbstractAssetManager, AssetManagerInterface, AssetPackageInterface}; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; -use Foxy\Fallback\FallbackInterface; use Foxy\Tests\Fixtures\Util\{ProcessExecutorMock, ThrowingProcessExecutorMock}; use Foxy\Tests\Provider\AssetManagerProvider; use PHPUnit\Framework\Attributes\{DataProviderExternal, RequiresOperatingSystemFamily}; @@ -36,7 +35,6 @@ abstract class AssetManager extends TestCase protected Config|null $config = null; protected string|null $cwd = ''; protected ProcessExecutorMock|ThrowingProcessExecutorMock|null $executor = null; - protected FallbackInterface|MockObject|null $fallback = null; protected Filesystem|MockObject|null $fs = null; protected IOInterface|MockObject|null $io = null; protected AssetManagerInterface|null $manager = null; @@ -460,12 +458,6 @@ public function testRunForInstallCommand(int $expectedRes, string $action): void ); } - if (0 === $expectedRes) { - $this->fallback->expects(self::never())->method('restore'); - } else { - $this->fallback->expects(self::once())->method('restore'); - } - $this->io ->expects(self::once()) ->method('write') @@ -496,87 +488,6 @@ public function testRunForInstallCommand(int $expectedRes, string $action): void ); } - public function testRunPreservesExecutorFailureWhenFallbackThrows(): void - { - $this->executor = new ThrowingProcessExecutorMock($this->io, $this->getValidVersion()); - $this->config = new Config([], ['run-asset-manager' => true]); - - $this->fallback - ->expects(self::once()) - ->method('restore') - ->willThrowException(new RuntimeException('Fallback failed.')); - - $this->manager = $this->getManager(); - - try { - $this->manager->run(); - - self::fail( - 'Expected fallback restoration to fail.', - ); - } catch (RuntimeException $exception) { - self::assertSame( - Message::ASSET_MANAGER_FALLBACK_RESTORE_FAILED->getMessage('Fallback failed.'), - $exception->getMessage(), - 'Fallback failure must be reported.', - ); - self::assertSame( - 'Process execution failed.', - $exception->getPrevious()?->getMessage(), - 'The original process failure must be preserved.', - ); - } - } - - public function testRunPreservesExitStatusWhenFallbackThrows(): void - { - $exitStatus = 7; - - $this->config = new Config([], ['run-asset-manager' => true]); - - $this->fallback - ->expects(self::once()) - ->method('restore') - ->willThrowException(new RuntimeException('Fallback failed.')); - - $this->actionForTestRunForInstallCommand('install'); - $this->executor->addExpectedValues($exitStatus, 'ASSET MANAGER OUTPUT'); - - $this->manager = $this->getManager(); - - try { - $this->manager->run(); - - self::fail( - 'Expected fallback restoration to fail.', - ); - } catch (RuntimeException $exception) { - self::assertSame( - Message::ASSET_MANAGER_FALLBACK_RESTORE_FAILED->getMessage('Fallback failed.'), - $exception->getMessage(), - 'Fallback failure must be reported.', - ); - - $previous = $exception->getPrevious(); - - self::assertInstanceOf( - RuntimeException::class, - $previous, - 'The exit status must be attached to a runtime exception.', - ); - self::assertSame( - $exitStatus, - $previous->getCode(), - 'The original exit status must be preserved.', - ); - self::assertSame( - Message::ASSET_MANAGER_EXITED_WITH_STATUS->getMessage($exitStatus), - $previous->getMessage(), - 'Previous exception must carry the exit status.', - ); - } - } - public function testRunPreservesWorkingDirectoryWhenExecutorThrows(): void { $rootPackageDir = $this->cwd . DIRECTORY_SEPARATOR . 'root-package'; @@ -591,10 +502,6 @@ public function testRunPreservesWorkingDirectoryWhenExecutorThrows(): void ['run-asset-manager' => true, 'root-package-json-dir' => $rootPackageDir], ); - $this->fallback - ->expects(self::once()) - ->method('restore'); - $this->manager = $this->getManager(); try { @@ -626,9 +533,6 @@ public function testRunRejectsUnsupportedManagerVersion(): void $this->io ->expects(self::never()) ->method('write'); - $this->fallback - ->expects(self::never()) - ->method('restore'); $this->executor->addExpectedValues(0, $this->getUnsupportedVersion()); $this->expectException(RuntimeException::class); @@ -658,10 +562,6 @@ public function testRunRestoresTimeoutWhenExecutorThrows(): void $this->manager = $this->getManager(); - $this->fallback - ->expects(self::once()) - ->method('restore'); - try { $this->manager->run(); self::fail( @@ -1072,7 +972,6 @@ protected function setUp(): void $this->executor = new ProcessExecutorMock($this->io); $this->fs = $this->createMock(Filesystem::class); $this->sfs = new \Symfony\Component\Filesystem\Filesystem(); - $this->fallback = $this->createMock(FallbackInterface::class); $this->manager = $this->getManager(); $this->oldCwd = getcwd(); $this->cwd = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('foxy_asset_manager_test_', true); @@ -1093,7 +992,6 @@ protected function tearDown(): void $this->executor = null; $this->fs = null; $this->sfs = null; - $this->fallback = null; $this->manager = null; $this->oldCwd = null; $this->cwd = null; diff --git a/tests/Asset/BunAssetManagerTest.php b/tests/Asset/BunAssetManagerTest.php index 6ee5ed8..b739147 100644 --- a/tests/Asset/BunAssetManagerTest.php +++ b/tests/Asset/BunAssetManagerTest.php @@ -812,7 +812,7 @@ public function testWindowsCommandsUseExecutableNameAndNormalizedCustomPath(): v protected function getManager(): BunManager { - return new BunManager($this->io, $this->config, $this->executor, $this->fs, $this->fallback); + return new BunManager($this->io, $this->config, $this->executor, $this->fs); } protected function getUnsupportedVersion(): string diff --git a/tests/Asset/DenoAssetManagerTest.php b/tests/Asset/DenoAssetManagerTest.php index 1ea48c9..51b6bef 100644 --- a/tests/Asset/DenoAssetManagerTest.php +++ b/tests/Asset/DenoAssetManagerTest.php @@ -189,30 +189,6 @@ public function testAddDependenciesRemovesStaleWorkspaceMember(): void ); } - public function testAddDependenciesRestoresFallbackWhenWorkspacesAreInvalid(): void - { - $this->fallback->expects(self::once())->method('restore'); - - try { - $this->addDependenciesFromPackage( - ['workspaces' => 'packages/*'], - ['@composer-asset/foo--bar' => 'path/foo/bar/package.json'], - ); - - self::fail( - 'Expected invalid workspaces to be rejected.', - ); - } catch (RuntimeException $exception) { - self::assertSame( - Message::ASSET_DENO_WORKSPACES_INVALID->getMessage( - $this->cwd . DIRECTORY_SEPARATOR . 'package.json', - ), - $exception->getMessage(), - 'The exception message should indicate the invalid workspaces', - ); - } - } - public function testAddDependenciesUsesRootPackageJsonDir(): void { $rootPackageDir = $this->cwd . DIRECTORY_SEPARATOR . 'root-package'; @@ -322,7 +298,7 @@ static function (mixed $command, mixed &$output = null) use (&$observed, &$posit ->method('getErrorOutput') ->willReturn(''); - (new DenoManager($this->io, $this->config, $executor, new Filesystem(), $this->fallback))->audit(false); + (new DenoManager($this->io, $this->config, $executor, new Filesystem()))->audit(false); self::assertSame( ['1', '1', '1'], @@ -631,7 +607,7 @@ protected function getExpectedPackage(array $package): array protected function getManager(): DenoManager { - return new DenoManager($this->io, $this->config, $this->executor, new Filesystem(), $this->fallback); + return new DenoManager($this->io, $this->config, $this->executor, new Filesystem()); } protected function getUnsupportedVersion(): string diff --git a/tests/Asset/NativeManagerTest.php b/tests/Asset/NativeManagerTest.php index 5842a07..3cfc5da 100644 --- a/tests/Asset/NativeManagerTest.php +++ b/tests/Asset/NativeManagerTest.php @@ -11,7 +11,6 @@ use Foxy\Audit\Parser\NativeAuditParser; use Foxy\Config\Config; use Foxy\Exception\{Message, RuntimeException}; -use Foxy\Fallback\FallbackInterface; use Foxy\Native\NativeInstallerInterface; use Foxy\Tests\Fixtures\Native\InMemoryRegistry; use Foxy\Tests\Provider\NativeManagerProvider; @@ -54,7 +53,6 @@ final class NativeManagerTest extends TestCase private const string UPDATING = 'Updating frontend dependencies with the native manager'; private string $cwd = ''; - private FallbackInterface&MockObject $fallback; private NativeInstallerInterface&MockObject $installer; private IOInterface&MockObject $io; @@ -187,23 +185,18 @@ public function testGetAuditParserReturnsNativeParser(): void ); } - public function testIsAvailableRequiresLockFileAndResolvesManager(): void + public function testIsAvailableRequiresLockFile(): void { $config = new Config(['manager-timeout' => ['native' => 5]]); $manager = new NativeManager($this->io, $config, new Filesystem(), $this->installer, $this->registry); - self::assertNull( - $config->get('manager-timeout'), - 'No manager-keyed value applies before resolution.', - ); self::assertFalse( $manager->isAvailable(), 'A missing lock file must not make the manager available.', ); - self::assertSame( - 5, + self::assertNull( $config->get('manager-timeout'), - 'The manager must be recorded as resolved.', + 'Probing must not scope the shared configuration.', ); file_put_contents("{$this->cwd}/foxy.lock", '{}'); @@ -372,7 +365,7 @@ public function testRunInstallsThroughSymlinkedInstallDirectory(): void ); } - public function testRunRestoresFallbackAndRethrowsInstallerFailure(): void + public function testRunRethrowsInstallerFailureUnchanged(): void { $failure = new RuntimeException('Install failed.'); @@ -380,9 +373,6 @@ public function testRunRestoresFallbackAndRethrowsInstallerFailure(): void ->expects(self::once()) ->method('install') ->willThrowException($failure); - $this->fallback - ->expects(self::once()) - ->method('restore'); try { $this->manager(['run-asset-manager' => true])->run(); @@ -527,36 +517,6 @@ public function testThrowRuntimeExceptionWhenAuditLockFileIsMissing(): void $this->manager()->audit(false); } - public function testThrowRuntimeExceptionWhenFallbackRestoreFails(): void - { - $failure = new RuntimeException('Install failed.'); - - $this->installer - ->method('install') - ->willThrowException($failure); - $this->fallback - ->expects(self::once()) - ->method('restore') - ->willThrowException(new RuntimeException('Fallback failed.')); - - try { - $this->manager(['run-asset-manager' => true])->run(); - - self::fail('Expected the fallback restoration to fail.'); - } catch (RuntimeException $exception) { - self::assertSame( - Message::ASSET_MANAGER_FALLBACK_RESTORE_FAILED->getMessage('Fallback failed.'), - $exception->getMessage(), - 'The fallback failure must be reported.', - ); - self::assertSame( - $failure, - $exception->getPrevious(), - 'The installer exception must be preserved.', - ); - } - } - public function testThrowRuntimeExceptionWhenInstallDirectoryIsDanglingSymlink(): void { $this->link("{$this->cwd}/missing", "{$this->cwd}/node_modules"); @@ -705,7 +665,6 @@ protected function setUp(): void { parent::setUp(); - $this->fallback = $this->createMock(FallbackInterface::class); $this->installer = $this->createMock(NativeInstallerInterface::class); $this->io = $this->createMock(IOInterface::class); $this->registry = new InMemoryRegistry(); @@ -779,7 +738,6 @@ private function manager(array $config = []): NativeManager new Filesystem(), $this->installer, $this->registry, - $this->fallback, ); } diff --git a/tests/Asset/NpmAssetManagerTest.php b/tests/Asset/NpmAssetManagerTest.php index 2454d5b..522a483 100644 --- a/tests/Asset/NpmAssetManagerTest.php +++ b/tests/Asset/NpmAssetManagerTest.php @@ -199,7 +199,7 @@ static function (mixed $command, mixed &$output = null) use (&$observedTimeout): try { ProcessExecutor::setTimeout(42); - $manager = new NpmManager($this->io, $this->config, $executor, $this->fs, $this->fallback); + $manager = new NpmManager($this->io, $this->config, $executor, $this->fs); self::assertSame( 0, @@ -223,7 +223,7 @@ static function (mixed $command, mixed &$output = null) use (&$observedTimeout): protected function getManager(): NpmManager { - return new NpmManager($this->io, $this->config, $this->executor, $this->fs, $this->fallback); + return new NpmManager($this->io, $this->config, $this->executor, $this->fs); } protected function getUnsupportedVersion(): string diff --git a/tests/Asset/PnpmAssetManagerTest.php b/tests/Asset/PnpmAssetManagerTest.php index 16e9c52..9756148 100644 --- a/tests/Asset/PnpmAssetManagerTest.php +++ b/tests/Asset/PnpmAssetManagerTest.php @@ -58,7 +58,6 @@ public function testAuditPreventsPnpmfileHooksFromMutatingTheWorkspace(): void $this->config, new ProcessExecutor($this->io), $this->fs, - $this->fallback, ); $result = $manager->audit(false); @@ -81,7 +80,7 @@ public function testAuditPreventsPnpmfileHooksFromMutatingTheWorkspace(): void protected function getManager(): PnpmManager { - return new PnpmManager($this->io, $this->config, $this->executor, $this->fs, $this->fallback); + return new PnpmManager($this->io, $this->config, $this->executor, $this->fs); } protected function getUnsupportedVersion(): string diff --git a/tests/Asset/YarnAssetManagerTest.php b/tests/Asset/YarnAssetManagerTest.php index 0eb3873..4c49109 100644 --- a/tests/Asset/YarnAssetManagerTest.php +++ b/tests/Asset/YarnAssetManagerTest.php @@ -87,7 +87,7 @@ static function (mixed $command, mixed &$output = null) use (&$position): int { ->method('getErrorOutput') ->willReturn(''); - $manager = new YarnManager($this->io, $this->config, $executor, $this->fs, $this->fallback); + $manager = new YarnManager($this->io, $this->config, $executor, $this->fs); $manager->audit(false); @@ -176,7 +176,7 @@ static function (mixed $command, mixed &$output = null) use (&$position): int { }, ); - $manager = new YarnManager($this->io, $this->config, $executor, $this->fs, $this->fallback); + $manager = new YarnManager($this->io, $this->config, $executor, $this->fs); try { $manager->audit(false); @@ -249,7 +249,7 @@ public function testIsInstalledWithPlugAndPlayState(): void protected function getManager(): YarnManager { - return new YarnManager($this->io, $this->config, $this->executor, $this->fs, $this->fallback); + return new YarnManager($this->io, $this->config, $this->executor, $this->fs); } protected function getUnsupportedVersion(): string diff --git a/tests/Command/FoxyCapabilityTest.php b/tests/Command/FoxyCapabilityTest.php index 74531e5..de8cacd 100644 --- a/tests/Command/FoxyCapabilityTest.php +++ b/tests/Command/FoxyCapabilityTest.php @@ -6,7 +6,7 @@ use Composer\Plugin\Capability\CommandProvider as ComposerCommandProvider; use Composer\Plugin\Capable; -use Foxy\Asset\AssetManagerInterface; +use Foxy\Asset\{AssetManagerFinder, AssetManagerInterface}; use Foxy\Audit\{AuditProcessResult, AuditRequest, AuditRunnerInterface, Severity}; use Foxy\Audit\Parser\NpmAuditParser; use Foxy\Command\FoxyCommandProvider; @@ -87,6 +87,21 @@ public function testDeclaresCommandProviderCapability(): void ); } + public function testThrowRuntimeExceptionWhenAuditFindsNoAssetManager(): void + { + $foxy = new Foxy(); + + self::setProperty($foxy, 'config', new Config(['enabled' => true])); + self::setProperty($foxy, 'assetManagerFinder', new AssetManagerFinder([])); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + Message::ASSET_MANAGER_NONE_FOUND->getMessage(), + ); + + $foxy->audit(new AuditRequest()); + } + public function testThrowRuntimeExceptionWhenPluginIsDisabled(): void { $foxy = new Foxy(); diff --git a/tests/Config/ConfigTest.php b/tests/Config/ConfigTest.php index 0e5d767..cd24bf4 100644 --- a/tests/Config/ConfigTest.php +++ b/tests/Config/ConfigTest.php @@ -75,6 +75,99 @@ public function testBuildIgnoresNonArrayGlobalConfig(): void } } + public function testForManagerKeepsEnvironmentValuesUnscoped(): void + { + $config = new FoxyConfig(['manager-bin' => ['npm' => '/opt/npm', 'yarn' => '/opt/yarn']]); + + putenv('FOXY__MANAGER_BIN=/env/bin'); + + try { + $value = $config->forManager('yarn')->get('manager-bin'); + } finally { + putenv('FOXY__MANAGER_BIN'); + } + + self::assertSame( + '/env/bin', + $value, + 'Environment scalar must pass through the manager view.', + ); + } + + public function testForManagerOverridesConfiguredManager(): void + { + $config = new FoxyConfig( + [ + 'manager' => 'npm', + 'manager-bin' => ['npm' => '/opt/npm', 'yarn' => '/opt/yarn'], + ], + ); + + $view = $config->forManager('yarn'); + + self::assertSame( + '/opt/yarn', + $view->get('manager-bin'), + 'Scoped manager must win over the configured `manager`.', + ); + self::assertSame( + '/opt/npm', + $config->get('manager-bin'), + 'Original configuration must keep resolving the configured `manager`.', + ); + } + + public function testForManagerReturnsDefaultForUnknownManager(): void + { + $config = new FoxyConfig(['manager-bin' => ['npm' => '/opt/npm']]); + + self::assertSame( + 'pnpm', + $config->forManager('pnpm')->get('manager-bin', 'pnpm'), + 'Missing map entry must fall back to the default value.', + ); + } + + public function testForManagerSelectsConfiguredManagerEntry(): void + { + $config = new FoxyConfig(['manager-bin' => ['npm' => '/opt/npm', 'yarn' => '/opt/yarn']]); + + self::assertSame( + '/opt/yarn', + $config->forManager('yarn')->get('manager-bin'), + 'View must select its own entry of a configured map.', + ); + self::assertNull( + $config->get('manager-bin'), + 'Unscoped configuration must not select any map entry.', + ); + } + + public function testForManagerSelectsManagerSpecificDefaults(): void + { + $config = new FoxyConfig( + [], + ['manager-version' => ['npm' => '>=10.9.8', 'yarn' => '^4.18.0']], + ); + + $view = $config->forManager('yarn'); + + self::assertNotSame( + $config, + $view, + 'Scoped view must be a distinct instance.', + ); + self::assertSame( + '^4.18.0', + $view->get('manager-version'), + 'View must select its manager-specific default.', + ); + self::assertNull( + $config->get('manager-version'), + 'Original configuration must stay unscoped.', + ); + } + /** * @param string $key The key. * @param array $expected The expected value. @@ -267,27 +360,6 @@ public function testIsEnabled(mixed $value, bool $expected): void ); } - public function testResolvedManagerSelectsManagerSpecificDefaults(): void - { - $config = new FoxyConfig( - [], - ['manager-version' => ['npm' => '>=10.9.8', 'yarn' => '^4.18.0']], - ); - - self::assertNull( - $config->get('manager-version'), - 'Manager-specific default must remain unresolved initially.', - ); - - $config->setResolvedManager('yarn'); - - self::assertSame( - '^4.18.0', - $config->get('manager-version'), - 'Resolved manager must select its configured version constraint.', - ); - } - protected function setUp(): void { $this->composer = $this->createMock(Composer::class); diff --git a/tests/Fixtures/Asset/CustomBinaryAssetManager.php b/tests/Fixtures/Asset/CustomBinaryAssetManager.php new file mode 100644 index 0000000..6ce9a70 --- /dev/null +++ b/tests/Fixtures/Asset/CustomBinaryAssetManager.php @@ -0,0 +1,57 @@ +getInstallCommand(); + } + + public function getLockPackageName(): string + { + return 'custom.lock'; + } + + public function getName(): string + { + return 'custom'; + } + + public function getUpdateCommandForTest(): string + { + return $this->getUpdateCommand(); + } + + public function getVersionCommandForTest(): string + { + return $this->getVersionCommand(); + } + + public function getVersionConstraint(): string + { + return '*'; + } + + protected function getAuditCommand(bool $noDev): string + { + return $this->buildUnconfiguredCommand($this->getDefaultBinary(), 'audit'); + } + + protected function getDefaultBinary(): string + { + return 'custom-bin'; + } +} diff --git a/tests/Fixtures/Asset/InspectableAssetManager.php b/tests/Fixtures/Asset/InspectableAssetManager.php index 24787d2..478f319 100644 --- a/tests/Fixtures/Asset/InspectableAssetManager.php +++ b/tests/Fixtures/Asset/InspectableAssetManager.php @@ -151,7 +151,7 @@ protected function actionWhenComposerDependenciesAreMerged( protected function getAuditCommand(bool $noDev): string { - return $this->buildUnconfiguredCommand('inspectable', $noDev ? ['audit', '--prod'] : ['audit']); + return $this->buildUnconfiguredCommand($this->getDefaultBinary(), $noDev ? ['audit', '--prod'] : ['audit']); } protected function getAuditEnvironment(): array @@ -159,21 +159,6 @@ protected function getAuditEnvironment(): array return [...parent::getAuditEnvironment(), ...($this->auditEnvironment ?? [])]; } - protected function getInstallCommand(): string - { - return $this->buildCommand('inspectable', 'install', 'install'); - } - - protected function getUpdateCommand(): string - { - return $this->buildCommand('inspectable', 'update', 'update'); - } - - protected function getVersionCommand(): string - { - return $this->buildUnconfiguredCommand('inspectable', '--version'); - } - protected function normalizeVersionOutput(string $output): string { $this->versionOutput = $output; diff --git a/tests/Fixtures/Asset/StubAssetManager.php b/tests/Fixtures/Asset/StubAssetManager.php index 12e7060..2a4781f 100644 --- a/tests/Fixtures/Asset/StubAssetManager.php +++ b/tests/Fixtures/Asset/StubAssetManager.php @@ -11,7 +11,6 @@ use Foxy\Audit\{AuditParserInterface, AuditProcessResult}; use Foxy\Config\Config; use Foxy\Exception\RuntimeException; -use Foxy\Fallback\FallbackInterface; final readonly class StubAssetManager implements AssetManagerInterface { @@ -94,11 +93,6 @@ public function run(): int return 0; } - public function setFallback(FallbackInterface $fallback): self - { - return $this; - } - public function setUpdatable(bool $updatable): self { return $this; diff --git a/tests/FoxyTest.php b/tests/FoxyTest.php index b2a87a8..592073d 100644 --- a/tests/FoxyTest.php +++ b/tests/FoxyTest.php @@ -11,8 +11,8 @@ use Composer\Package\{Package, RootPackageInterface}; use Composer\Repository\RepositoryManager; use Composer\Script\{Event, ScriptEvents}; -use Composer\Util\{Filesystem, ProcessExecutor}; -use Foxy\Asset\{AbstractAssetManager, AssetManagerInterface, DenoManager, NativeManager, NpmManager}; +use Composer\Util\{Filesystem, HttpDownloader, Loop, ProcessExecutor}; +use Foxy\Asset\{AssetManagerFinder, AssetManagerInterface, DenoManager, NativeManager, NpmManager}; use Foxy\Config\Config as FoxyConfig; use Foxy\Exception\{Message, RuntimeException}; use Foxy\Fallback\AssetFallback; @@ -79,10 +79,22 @@ public function testActivate(): void $this->getObjectProperty($composerFallback, 'snapshotSaved'), 'Composer snapshot must be saved.', ); + $solver = $this->invokeFoxyMethod($foxy, 'getSolver'); + + self::assertSame( + $solver, + $this->invokeFoxyMethod($foxy, 'getSolver'), + 'Solver must be built once.', + ); self::assertSame( $assetFallback, - (new ReflectionClass(AbstractAssetManager::class))->getProperty('fallback')->getValue($assetManager), - 'Manager and plugin must share the asset fallback.', + $this->getObjectProperty($solver, 'assetFallback'), + 'Solver and plugin must share the asset fallback.', + ); + self::assertSame( + $assetManager, + $this->getObjectProperty($solver, 'assetManager'), + 'Solver and plugin must share the asset manager.', ); } @@ -100,6 +112,8 @@ public function testActivateBuildsAssetFallbackWithResolvedRootPackagePath(): vo $foxy->activate($this->composer, $this->io); + $this->resolveAssetManager($foxy); + $foxyReflection = new ReflectionClass($foxy); $assetFallbackProperty = $foxyReflection->getProperty('assetFallback'); @@ -155,6 +169,35 @@ public function testActivateDefaultsNativeInstallDirToNodeModules(): void ); } + /** + * @throws ParsingException + */ + public function testActivateDoesNotResolveManagerWhenEnabled(): void + { + $this->package + ->method('getConfig') + ->willReturn(['foxy' => ['manager' => 'invalid_manager']]); + + $foxy = new Foxy(); + + $foxy->activate($this->composer, $this->io); + + $reflection = new ReflectionClass($foxy); + + self::assertFalse( + $reflection->getProperty('assetManager')->isInitialized($foxy), + 'Manager must stay unresolved after activation.', + ); + self::assertFalse( + $reflection->getProperty('assetFallback')->isInitialized($foxy), + 'Asset fallback must not be built before the manager.', + ); + self::assertFalse( + $reflection->getProperty('solver')->isInitialized($foxy), + 'Solver must not be built before the manager.', + ); + } + /** * @throws ParsingException|ReflectionException */ @@ -210,6 +253,8 @@ public function testActivateKeepsNpmFirstForAutomaticDiscovery(): void $foxy->activate($this->composer, $this->io); + $this->resolveAssetManager($foxy); + self::assertInstanceOf( NpmManager::class, $this->getFoxyProperty($foxy, 'assetManager'), @@ -366,6 +411,8 @@ public function testActivateResolvesDenoManager(): void $foxy->activate($this->composer, $this->io); + $this->resolveAssetManager($foxy); + self::assertInstanceOf( DenoManager::class, $this->getFoxyProperty($foxy, 'assetManager'), @@ -386,6 +433,8 @@ public function testActivateResolvesNativeManager(): void $foxy->activate($this->composer, $this->io); + $this->resolveAssetManager($foxy); + self::assertInstanceOf( NativeManager::class, $this->getFoxyProperty($foxy, 'assetManager'), @@ -398,6 +447,36 @@ public function testActivateResolvesNativeManager(): void ); } + /** + * @throws ParsingException|ReflectionException + */ + public function testActivateSharesComposerHttpDownloaderWithNativeRegistry(): void + { + $httpDownloader = $this->createMock(HttpDownloader::class); + $loop = $this->createMock(Loop::class); + + $loop + ->expects(self::once()) + ->method('getHttpDownloader') + ->willReturn($httpDownloader); + $this->composer + ->method('getLoop') + ->willReturn($loop); + $this->package + ->method('getConfig') + ->willReturn(['foxy' => ['manager' => 'native', 'run-asset-manager' => false]]); + + $foxy = new Foxy(); + + $foxy->activate($this->composer, $this->io); + + self::assertSame( + $httpDownloader, + $this->getObjectProperty($this->getNativeRegistry($foxy), 'httpDownloader'), + 'Registry must use the downloader of the Composer loop.', + ); + } + /** * @throws ParsingException */ @@ -413,12 +492,20 @@ public function testActivateSkipsManagerDiscoveryWhenDisabled(): void $foxy->activate($this->composer, $this->io); $foxy->init(); - $assetManager = (new ReflectionClass($foxy))->getProperty('assetManager'); + $reflection = new ReflectionClass($foxy); self::assertFalse( - $assetManager->isInitialized($foxy), + $reflection->getProperty('assetManager')->isInitialized($foxy), 'Uninitialized manager must remain undiscovered.', ); + self::assertFalse( + $reflection->getProperty('assetManagerFinder')->isInitialized($foxy), + 'Disabled plugin must not build the manager finder.', + ); + self::assertFalse( + $reflection->getProperty('composerFallback')->isInitialized($foxy), + 'Disabled plugin must not build the Composer fallback.', + ); } /** @@ -443,6 +530,8 @@ public function testActivateUsesPackageNameForNonAbstractAssetManager(): void $foxy->activate($this->composer, $this->io); + $this->resolveAssetManager($foxy); + $assetFallbackProperty = $foxyReflection->getProperty('assetFallback'); $assetFallback = $assetFallbackProperty->getValue($foxy); @@ -460,26 +549,6 @@ public function testActivateUsesPackageNameForNonAbstractAssetManager(): void } } - /** - * @throws ParsingException - */ - public function testActivateWithInvalidManager(): void - { - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage( - Message::ASSET_MANAGER_UNKNOWN->getMessage('invalid_manager'), - ); - - $this->package - ->expects(self::any()) - ->method('getConfig') - ->willReturn(['foxy' => ['manager' => 'invalid_manager']]); - - $foxy = new Foxy(); - - $foxy->activate($this->composer, $this->io); - } - public function testAutomaticManagerDiscoveryDoesNotProbeBinariesWhenExecutionIsDisabled(): void { $config = new FoxyConfig( @@ -495,22 +564,27 @@ public function testAutomaticManagerDiscoveryDoesNotProbeBinariesWhenExecutionIs ->expects(self::never()) ->method('execute'); + $fs = $this->createMock(Filesystem::class); $foxy = new Foxy(); $reflection = new ReflectionClass($foxy); - $reflection - ->getProperty('config') - ->setValue($foxy, $config); - - $manager = $reflection->getMethod('getAssetManager')->invoke( + $reflection->getProperty('config')->setValue($foxy, $config); + $reflection->getProperty('io')->setValue($foxy, $this->io); + $reflection->getProperty('fs')->setValue($foxy, $fs); + $reflection->getProperty('assetManagerFinder')->setValue( $foxy, - $this->composer, - $this->io, - $config, - $executor, - $this->createMock(Filesystem::class), + $reflection->getMethod('createAssetManagerFinder')->invoke( + $foxy, + $this->composer, + $this->io, + $config, + $executor, + $fs, + ), ); + $manager = $this->resolveAssetManager($foxy); + self::assertInstanceOf( NpmManager::class, $manager, @@ -562,6 +636,34 @@ public function testDeactivate(): void $this->expectNotToPerformAssertions(); } + /** + * @throws ParsingException + */ + public function testGetAssetManagerResolvesOnce(): void + { + $this->package + ->method('getConfig') + ->willReturn(['foxy' => ['manager' => 'npm', 'run-asset-manager' => false]]); + + $foxy = new Foxy(); + + $foxy->activate($this->composer, $this->io); + + $manager = $this->resolveAssetManager($foxy); + $assetFallback = $this->getFoxyProperty($foxy, 'assetFallback'); + + self::assertSame( + $manager, + $this->resolveAssetManager($foxy), + 'Second call must return the memoized manager.', + ); + self::assertSame( + $assetFallback, + $this->getFoxyProperty($foxy, 'assetFallback'), + 'Second call must keep the asset fallback.', + ); + } + public function testGetSubscribedEvents(): void { self::assertSame( @@ -590,13 +692,13 @@ public function testInitHonorsRunAssetManagerValues(mixed $value, bool $expected $foxy->activate($this->composer, $this->io); - $assetManager = $this->createMock(AssetManagerInterface::class); + $assetManager = $this->createNpmManagerMock(); $assetManager ->expects($expectedValidation ? self::once() : self::never()) ->method('validate'); - (new ReflectionClass($foxy))->getProperty('assetManager')->setValue($foxy, $assetManager); + $this->injectAssetManagerFinder($foxy, $assetManager); $foxy->init(); } @@ -614,13 +716,13 @@ public function testInitRunsOnlyOnce(): void $foxy->activate($this->composer, $this->io); - $assetManager = $this->createMock(AssetManagerInterface::class); + $assetManager = $this->createNpmManagerMock(); $assetManager ->expects(self::once()) ->method('validate'); - (new ReflectionClass($foxy))->getProperty('assetManager')->setValue($foxy, $assetManager); + $this->injectAssetManagerFinder($foxy, $assetManager); $foxy->init(); $foxy->init(); @@ -631,23 +733,6 @@ public function testInitRunsOnlyOnce(): void ); } - /** - * @throws ParsingException - */ - public function testIntegerOneEnablesPlugin(): void - { - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage( - Message::ASSET_MANAGER_UNKNOWN->getMessage('invalid_manager'), - ); - - $this->package - ->method('getConfig') - ->willReturn(['foxy' => ['enabled' => 1, 'manager' => 'invalid_manager']]); - - (new Foxy())->activate($this->composer, $this->io); - } - #[DataProviderExternal(FoxyProvider::class, 'solveEvents')] public function testSolveAssets(string $eventName, bool $expectedUpdatable): void { @@ -695,6 +780,49 @@ public function testSolveAssetsDoesNothingWhenDisabled(): void $foxy->solveAssets(new Event('solve_event_install', $this->composer, $this->io)); } + /** + * @throws ParsingException + */ + public function testThrowRuntimeExceptionWhenInitFindsUnknownManager(): void + { + $this->package + ->expects(self::any()) + ->method('getConfig') + ->willReturn(['foxy' => ['manager' => 'invalid_manager']]); + + $foxy = new Foxy(); + + $foxy->activate($this->composer, $this->io); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + Message::ASSET_MANAGER_UNKNOWN->getMessage('invalid_manager'), + ); + + $foxy->init(); + } + + /** + * @throws ParsingException + */ + public function testThrowRuntimeExceptionWhenIntegerOneEnabledPluginFindsUnknownManager(): void + { + $this->package + ->method('getConfig') + ->willReturn(['foxy' => ['enabled' => 1, 'manager' => 'invalid_manager']]); + + $foxy = new Foxy(); + + $foxy->activate($this->composer, $this->io); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + Message::ASSET_MANAGER_UNKNOWN->getMessage('invalid_manager'), + ); + + $foxy->init(); + } + public function testUninstall(): void { $foxy = new Foxy(); @@ -749,6 +877,20 @@ protected function setUp(): void ; } + /** + * Returns an asset manager mock named `npm` that has no lock file. + */ + private function createNpmManagerMock(): AssetManagerInterface&MockObject + { + $assetManager = $this->createMock(AssetManagerInterface::class); + + $assetManager->method('getName')->willReturn('npm'); + $assetManager->method('getPackageName')->willReturn('package.json'); + $assetManager->method('hasLockFile')->willReturn(false); + + return $assetManager; + } + private function getFoxyProperty(Foxy $foxy, string $name): mixed { return (new ReflectionClass($foxy))->getProperty($name)->getValue($foxy); @@ -761,7 +903,7 @@ private function getFoxyProperty(Foxy $foxy, string $name): mixed */ private function getNativeRegistry(Foxy $foxy): NpmRegistry { - $manager = $this->getFoxyProperty($foxy, 'assetManager'); + $manager = $this->resolveAssetManager($foxy); self::assertInstanceOf( NativeManager::class, @@ -784,4 +926,30 @@ private function getObjectProperty(object $object, string $name): mixed { return (new ReflectionClass($object))->getProperty($name)->getValue($object); } + /** + * Replaces the finder built by activation with one that holds only the given manager. + */ + private function injectAssetManagerFinder(Foxy $foxy, AssetManagerInterface $assetManager): void + { + (new ReflectionClass($foxy)) + ->getProperty('assetManagerFinder') + ->setValue($foxy, new AssetManagerFinder([$assetManager])); + } + + /** + * Invokes a private method of the plugin. + */ + private function invokeFoxyMethod(Foxy $foxy, string $name): mixed + { + return (new ReflectionClass($foxy))->getMethod($name)->invoke($foxy); + } + + /** + * Resolves the asset manager through the plugin's private memoizing accessor. + */ + private function resolveAssetManager(Foxy $foxy): AssetManagerInterface + { + return $this->invokeFoxyMethod($foxy, 'getAssetManager'); + } + } diff --git a/tests/Native/NpmRegistryTest.php b/tests/Native/NpmRegistryTest.php index 59651bd..c942a84 100644 --- a/tests/Native/NpmRegistryTest.php +++ b/tests/Native/NpmRegistryTest.php @@ -474,15 +474,18 @@ public function testPrefetchTarballsRequestsNothingForEmptyList(): void public function testPrefetchTarballsRequestsNothingWithDisabledCache(): void { - mkdir("{$this->root}/locked", 0o555); - - $cache = new Cache(new NullIO(), "{$this->root}/locked", 'a-z0-9_./'); + $cache = new class (new NullIO(), $this->cacheDir, 'a-z0-9_./') extends Cache { + public function isEnabled(): bool + { + return false; + } + }; $this->registry($this->idleDownloader(), $cache)->prefetchTarballs([$this->package()]); - self::assertFalse( - $cache->isEnabled(), - 'Cache in a read-only directory must be disabled.', + self::assertFileDoesNotExist( + $this->cacheEntry(), + 'Disabled cache must stay empty.', ); } diff --git a/tests/Provider/AssetPackageProvider.php b/tests/Provider/AssetPackageProvider.php index 4baa5ec..6206011 100644 --- a/tests/Provider/AssetPackageProvider.php +++ b/tests/Provider/AssetPackageProvider.php @@ -12,7 +12,7 @@ final class AssetPackageProvider { /** - * @return iterable, array, string}> + * @return iterable, array, string}> */ public static function requiredKeys(): iterable { @@ -31,5 +31,10 @@ public static function requiredKeys(): iterable ['name' => '@foo/bar'], 'proprietary', ]; + yield [ + ['name' => '@foo/bar', 'private' => false], + ['name' => '@foo/bar', 'private' => false], + 'proprietary', + ]; } } diff --git a/tests/Provider/MessageProvider.php b/tests/Provider/MessageProvider.php index cfbe349..37ee8db 100644 --- a/tests/Provider/MessageProvider.php +++ b/tests/Provider/MessageProvider.php @@ -95,17 +95,6 @@ public static function cases(): iterable ['npm'], 'The "npm" binary must be installed.', ]; - yield 'ASSET_MANAGER_EXITED_WITH_STATUS' => [ - Message::ASSET_MANAGER_EXITED_WITH_STATUS, - [7], - 'The asset manager exited with status code 7.', - ]; - yield 'ASSET_MANAGER_FALLBACK_RESTORE_FAILED' => [ - Message::ASSET_MANAGER_FALLBACK_RESTORE_FAILED, - ['Unable to write fallback asset file "/project/package.json".'], - 'The asset manager failed and its fallback could not be restored: Unable to write fallback asset file ' - . '"/project/package.json".', - ]; yield 'ASSET_MANAGER_LOCKED_UNAVAILABLE' => [ Message::ASSET_MANAGER_LOCKED_UNAVAILABLE, ['pnpm'], @@ -639,6 +628,12 @@ public static function cases(): iterable ['/project/vendor/foxy/composer-asset'], 'The Composer asset directory "/project/vendor/foxy/composer-asset" is not marked as managed by Foxy.', ]; + yield 'SOLVER_ASSET_FALLBACK_RESTORE_FAILED' => [ + Message::SOLVER_ASSET_FALLBACK_RESTORE_FAILED, + ['Unable to write fallback asset file "/project/package.json".'], + 'The asset manager failed and its fallback could not be restored: Unable to write fallback asset file ' + . '"/project/package.json".', + ]; yield 'SOLVER_ASSET_MANAGER_FAILED' => [ Message::SOLVER_ASSET_MANAGER_FAILED, [7], diff --git a/tests/Solver/SolverTest.php b/tests/Solver/SolverTest.php index a2bd2b9..ce86471 100644 --- a/tests/Solver/SolverTest.php +++ b/tests/Solver/SolverTest.php @@ -47,6 +47,7 @@ */ class SolverTest extends TestCase { + private FallbackInterface|MockObject|null $assetFallback = null; private Composer|MockObject|null $composer = null; private \Composer\Config|MockObject|null $composerConfig = null; private FallbackInterface|MockObject|null $composerFallback = null; @@ -445,10 +446,16 @@ public function testSolve(int $resRunManager): void ->willReturn($resRunManager); if (0 === $resRunManager) { + $this->assetFallback + ->expects(self::never()) + ->method('restore'); $this->composerFallback ->expects(self::never()) ->method('restore'); } else { + $this->assetFallback + ->expects(self::once()) + ->method('restore'); $this->composerFallback ->expects(self::once()) ->method('restore'); @@ -690,6 +697,43 @@ static function (string $name, object $event) use (&$observedEvents): int { ); } + public function testSolveDoesNotRestoreAssetFallbackWhenPostSolveListenerThrows(): void + { + $this->addInstalledPackages(); + + $failure = new \RuntimeException('Listener failed.'); + + $this->onPostSolve( + static function () use ($failure): never { + throw $failure; + }, + ); + $this->manager + ->expects(self::once()) + ->method('run') + ->willReturn(0); + $this->assetFallback + ->expects(self::never()) + ->method('restore'); + $this->composerFallback + ->expects(self::once()) + ->method('restore'); + + try { + $this->solver->solve($this->composer, $this->io); + + self::fail( + 'Expected the listener failure.', + ); + } catch (\RuntimeException $exception) { + self::assertSame( + $failure, + $exception, + 'Listener failure must be rethrown unchanged.', + ); + } + } + public function testSolveIgnoresPackageWithoutAssetActivation(): void { $package = $this->createMock(PackageInterface::class); @@ -1173,6 +1217,146 @@ public function testSolveRejectsWhitespaceOnlyAssetDirectory(): void $solver->solve($this->composer, $this->io); } + public function testSolveRestoresAssetFallbackBeforeComposerFallbackWhenMergeFails(): void + { + $this->addInstalledPackages(); + + $failure = new \RuntimeException('Merge failed.'); + $restored = []; + + $this->manager + ->expects(self::once()) + ->method('addDependencies') + ->willThrowException($failure); + $this->manager + ->expects(self::never()) + ->method('run'); + $this->assetFallback + ->expects(self::once()) + ->method('restore') + ->willReturnCallback( + static function () use (&$restored): void { + $restored[] = 'asset'; + }, + ); + $this->composerFallback + ->expects(self::once()) + ->method('restore') + ->willReturnCallback( + static function () use (&$restored): void { + $restored[] = 'composer'; + }, + ); + + try { + $this->solver->solve($this->composer, $this->io); + + self::fail( + 'Expected the merge failure.', + ); + } catch (\RuntimeException $exception) { + self::assertSame( + $failure, + $exception, + 'Merge failure must be rethrown unchanged.', + ); + } + + self::assertSame( + ['asset', 'composer'], + $restored, + 'Order: asset fallback first, Composer fallback second.', + ); + } + + public function testSolveRestoresAssetFallbackBeforePostSolveWhenManagerReturnsNonZero(): void + { + $this->addInstalledPackages(); + + $observed = []; + + $this->onPostSolve( + static function () use (&$observed): void { + $observed[] = 'post-solve'; + }, + ); + $this->manager + ->expects(self::once()) + ->method('run') + ->willReturn(3); + $this->assetFallback + ->expects(self::once()) + ->method('restore') + ->willReturnCallback( + static function () use (&$observed): void { + $observed[] = 'asset'; + }, + ); + $this->composerFallback + ->expects(self::once()) + ->method('restore') + ->willReturnCallback( + static function () use (&$observed): void { + $observed[] = 'composer'; + }, + ); + + try { + $this->solver->solve($this->composer, $this->io); + + self::fail( + 'Expected the non-zero result to fail.', + ); + } catch (RuntimeException $exception) { + self::assertSame( + Message::SOLVER_ASSET_MANAGER_FAILED->getMessage(3), + $exception->getMessage(), + 'Message must carry the manager result code.', + ); + } + + self::assertSame( + ['asset', 'post-solve', 'composer'], + $observed, + 'Order: asset restore, `foxy.post-solve`, Composer restore.', + ); + } + + public function testSolveRestoresAssetFallbackWhenManagerThrows(): void + { + $this->addInstalledPackages(); + + $failure = new \RuntimeException('Manager process failed.'); + + $this->manager + ->expects(self::once()) + ->method('addDependencies'); + $this->manager + ->expects(self::once()) + ->method('run') + ->willThrowException($failure); + $this->assetFallback + ->expects(self::once()) + ->method('restore'); + $this->composerFallback + ->expects(self::once()) + ->method('restore'); + + try { + $this->solver->solve($this->composer, $this->io); + + self::fail( + 'Expected the manager failure.', + ); + } catch (\RuntimeException $exception) { + self::assertSame( + $failure, + $exception, + 'Manager failure must be rethrown unchanged.', + ); + } + } + public function testSolveRestoresComposerWhenAssetManifestCannotBeRead(): void { $sourceContent = '{"name":"source-package","version":"1.0.0"}'; @@ -1192,6 +1376,9 @@ public function testSolveRestoresComposerWhenAssetManifestCannotBeRead(): void $this->manager ->expects(self::never()) ->method('run'); + $this->assetFallback + ->expects(self::never()) + ->method('restore'); $this->composerFallback ->expects(self::once()) ->method('restore'); @@ -1521,6 +1708,148 @@ public function testSolveWithDisableOption(): void $solver->solve($this->composer, $this->io); } + public function testThrowRuntimeExceptionWhenAssetAndComposerFallbackRestoresFail(): void + { + $this->addInstalledPackages(); + + $failure = new \RuntimeException('Manager failed.'); + + $this->manager + ->expects(self::once()) + ->method('run') + ->willThrowException($failure); + $this->assetFallback + ->expects(self::once()) + ->method('restore') + ->willThrowException(new \RuntimeException('Asset fallback failed.')); + $this->composerFallback + ->expects(self::once()) + ->method('restore') + ->willThrowException(new \RuntimeException('Composer fallback failed.')); + + try { + $this->solver->solve($this->composer, $this->io); + + self::fail( + 'Expected both fallback restorations to fail.', + ); + } catch (RuntimeException $exception) { + self::assertSame( + Message::SOLVER_COMPOSER_FALLBACK_RESTORE_FAILED->getMessage('Composer fallback failed.'), + $exception->getMessage(), + 'Outer message must embed the Composer fallback failure.', + ); + $previous = $exception->getPrevious(); + + self::assertInstanceOf( + RuntimeException::class, + $previous, + 'Previous exception must be a Foxy runtime exception.', + ); + self::assertSame( + Message::SOLVER_ASSET_FALLBACK_RESTORE_FAILED->getMessage('Asset fallback failed.'), + $previous->getMessage(), + 'Previous exception must be the asset fallback failure.', + ); + self::assertSame( + $failure, + $previous->getPrevious(), + 'Manager failure must end the previous chain.', + ); + } + } + + public function testThrowRuntimeExceptionWhenAssetFallbackRestoreFails(): void + { + $this->addInstalledPackages(); + + $failure = new \RuntimeException('Manager failed.'); + + $this->manager + ->expects(self::once()) + ->method('run') + ->willThrowException($failure); + $this->assetFallback + ->expects(self::once()) + ->method('restore') + ->willThrowException(new \RuntimeException('Asset fallback failed.')); + $this->composerFallback + ->expects(self::once()) + ->method('restore'); + + try { + $this->solver->solve($this->composer, $this->io); + + self::fail( + 'Expected the asset fallback restoration to fail.', + ); + } catch (RuntimeException $exception) { + self::assertSame( + Message::SOLVER_ASSET_FALLBACK_RESTORE_FAILED->getMessage('Asset fallback failed.'), + $exception->getMessage(), + 'Message must embed the asset fallback failure.', + ); + self::assertSame( + 0, + $exception->getCode(), + "Exception code must be '0'.", + ); + self::assertSame( + $failure, + $exception->getPrevious(), + 'Manager failure must be the previous exception.', + ); + } + } + + public function testThrowRuntimeExceptionWhenAssetFallbackRestoreFailsAfterNonZeroResult(): void + { + $this->addInstalledPackages(); + + $postSolveDispatched = false; + + $this->onPostSolve( + static function () use (&$postSolveDispatched): void { + $postSolveDispatched = true; + }, + ); + $this->manager + ->expects(self::once()) + ->method('run') + ->willReturn(7); + $this->assetFallback + ->expects(self::once()) + ->method('restore') + ->willThrowException(new \RuntimeException('Asset fallback failed.')); + $this->composerFallback + ->expects(self::once()) + ->method('restore'); + + try { + $this->solver->solve($this->composer, $this->io); + + self::fail( + 'Expected the asset fallback restoration to fail.', + ); + } catch (RuntimeException $exception) { + self::assertSame( + Message::SOLVER_ASSET_FALLBACK_RESTORE_FAILED->getMessage('Asset fallback failed.'), + $exception->getMessage(), + 'Message must embed the asset fallback failure.', + ); + self::assertSame( + Message::SOLVER_ASSET_MANAGER_FAILED->getMessage(7), + $exception->getPrevious()?->getMessage(), + 'Previous exception must carry the manager result code.', + ); + } + + self::assertFalse( + $postSolveDispatched, + 'A failed asset restore must stop before `foxy.post-solve`.', + ); + } + public function testValidateAssetDirectoryThrowsWhenProjectDirectoryIsUnavailable(): void { MockerState::addCondition('Foxy\\Solver', 'getcwd', [], false); @@ -1550,6 +1879,7 @@ protected function setUp(): void $this->package = $this->createMock(RootPackageInterface::class); $this->manager = $this->createMock(AssetManagerInterface::class); $this->composerFallback = $this->createMock(FallbackInterface::class); + $this->assetFallback = $this->createMock(FallbackInterface::class); $this->sfs->mkdir($this->cwd); $this->vendorDir = $this->cwd . '/vendor'; $this->composerConfig @@ -1597,7 +1927,13 @@ protected function setUp(): void fn(): EventDispatcher => $this->dispatcher ?? throw new LogicException('Missing event dispatcher.'), ); - $this->solver = new Solver($this->manager, $this->config, $this->fs, $this->composerFallback); + $this->solver = new Solver( + $this->manager, + $this->config, + $this->fs, + $this->composerFallback, + $this->assetFallback, + ); } protected function tearDown(): void @@ -1618,6 +1954,7 @@ protected function tearDown(): void $this->package = null; $this->manager = null; $this->composerFallback = null; + $this->assetFallback = null; $this->solver = null; $this->oldCwd = null; $this->cwd = null; @@ -1666,6 +2003,26 @@ private function invokeSolverMethodOn(SolverInterface $solver, string $method, m return (new ReflectionClass($solver))->getMethod($method)->invoke($solver, ...$arguments); } + /** + * Replaces the event dispatcher with one that runs the callback when `foxy.post-solve` is dispatched. + */ + private function onPostSolve(callable $callback): void + { + $this->dispatcher = $this->createMock(EventDispatcher::class); + + $this->dispatcher + ->method('dispatch') + ->willReturnCallback( + static function (string $name) use ($callback): int { + if (FoxyEvents::POST_SOLVE === $name) { + $callback(); + } + + return 0; + }, + ); + } + /** * @return array{source: string, target: string} */ From 224d6be649a7f99235e9347048cf8db64447a3fb Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Sat, 10 Oct 2026 21:36:49 +0000 Subject: [PATCH 5/8] Apply fixes from StyleCI --- tests/FoxyTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/FoxyTest.php b/tests/FoxyTest.php index 592073d..30cf360 100644 --- a/tests/FoxyTest.php +++ b/tests/FoxyTest.php @@ -926,6 +926,7 @@ private function getObjectProperty(object $object, string $name): mixed { return (new ReflectionClass($object))->getProperty($name)->getValue($object); } + /** * Replaces the finder built by activation with one that holds only the given manager. */ @@ -951,5 +952,4 @@ private function resolveAssetManager(Foxy $foxy): AssetManagerInterface { return $this->invokeFoxyMethod($foxy, 'getAssetManager'); } - } From 8a1354e1f18b89a011b287800a5a4d05e456ccd1 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Sat, 10 Oct 2026 18:51:32 -0300 Subject: [PATCH 6/8] Apply fixed Coderabbitai and Codex review. --- src/Audit/CveEnricher.php | 7 ++++- src/Exception/Message.php | 7 +++++ src/Native/NpmRegistry.php | 24 +++++++++-------- tests/Audit/CveEnricherTest.php | 41 ++++++++++++++++++++++++++++++ tests/Native/NpmRegistryTest.php | 24 +++++++++++++++++ tests/Provider/MessageProvider.php | 5 ++++ 6 files changed, 97 insertions(+), 11 deletions(-) diff --git a/src/Audit/CveEnricher.php b/src/Audit/CveEnricher.php index df5b1bd..eb4cdff 100644 --- a/src/Audit/CveEnricher.php +++ b/src/Audit/CveEnricher.php @@ -4,6 +4,7 @@ namespace Foxy\Audit; +use Foxy\Exception\{Message, RuntimeException}; use Throwable; use function array_fill_keys; @@ -62,6 +63,8 @@ public function enrich(AuditReport $report, callable $warningHandler): AuditRepo /** * Returns the resolution of every GHSA identifier, marking each failed lookup as `unavailable` with one warning. * + * A batch that throws, and an identifier the resolver leaves out, count as failed lookups. + * * @param list $ghsaIds Unique canonical GHSA identifiers, in first-seen order. * @param callable(string): void $warningHandler Receives the warnings in the order of `$ghsaIds`. * @@ -78,7 +81,9 @@ private function resolveAll(array $ghsaIds, callable $warningHandler): array $resolutions = []; foreach ($ghsaIds as $ghsaId) { - $result = $results[$ghsaId]; + $result = $results[$ghsaId] ?? new RuntimeException( + Message::AUDIT_CVE_RESOLUTION_MISSING->getMessage($ghsaId), + ); if ($result instanceof Throwable) { $warningHandler( diff --git a/src/Exception/Message.php b/src/Exception/Message.php index f2e1560..ed92047 100644 --- a/src/Exception/Message.php +++ b/src/Exception/Message.php @@ -235,6 +235,13 @@ enum Message: string */ case AUDIT_COMMAND_FAILED_WITH_DIAGNOSTICS = 'The %s audit command failed with status code %d. %s'; + /** + * Error when a CVE resolver answers a batch without one of the requested advisories. + * + * Format: "The CVE resolver returned no result for "%s"." + */ + case AUDIT_CVE_RESOLUTION_MISSING = 'The CVE resolver returned no result for "%s".'; + /** * Deno parser detail when advisory actions do not match the patched versions. * diff --git a/src/Native/NpmRegistry.php b/src/Native/NpmRegistry.php index 256b020..ddb297f 100644 --- a/src/Native/NpmRegistry.php +++ b/src/Native/NpmRegistry.php @@ -179,9 +179,9 @@ public function getMetadata(string $name): PackageMetadata * Downloads the tarballs missing from the cache in parallel, verifying each before it is cached. * * Nothing is downloaded when the cache is disabled or read-only, since the downloads could not be kept. A package - * without a tarball URL is skipped. A download that fails or does not match its integrity value is discarded with - * a verbose notice; {@see fetchTarball()} downloads it again and reports the error. Every temporary file is - * removed before the method returns. + * without a tarball URL is skipped. A download that fails, does not match its integrity value, or cannot be + * cached is discarded with a verbose notice; {@see fetchTarball()} downloads it again and reports the error. + * Every temporary file is removed before the method returns. * * @param list $packages Registry packages whose tarballs are wanted. */ @@ -205,13 +205,17 @@ public function prefetchTarballs(array $packages): void $this->httpDownloader->addCopy($resolved, $file)->then( function () use ($package, $key, $file, $resolved): void { - if ($this->matchesIntegrity($file, $package->integrity ?? '')) { - $this->cache->copyFrom($key, $file); - } else { - $this->reportPrefetchFailure( - $package, - Message::NATIVE_INTEGRITY_MISMATCH->getMessage($resolved), - ); + try { + if ($this->matchesIntegrity($file, $package->integrity ?? '')) { + $this->cache->copyFrom($key, $file); + } else { + $this->reportPrefetchFailure( + $package, + Message::NATIVE_INTEGRITY_MISMATCH->getMessage($resolved), + ); + } + } catch (Throwable $exception) { + $this->reportPrefetchFailure($package, $exception->getMessage()); } unlink($file); diff --git a/tests/Audit/CveEnricherTest.php b/tests/Audit/CveEnricherTest.php index 0a3c46b..f2efa25 100644 --- a/tests/Audit/CveEnricherTest.php +++ b/tests/Audit/CveEnricherTest.php @@ -5,6 +5,7 @@ namespace Foxy\Tests\Audit; use Foxy\Audit\{AuditFinding, AuditReport, CveEnricher, CveResolution, CveResolverInterface, CveStatus, Severity}; +use Foxy\Exception\Message; use Foxy\Tests\Provider\CveEnricherProvider; use PHPUnit\Framework\Attributes\DataProviderExternal; use PHPUnit\Framework\TestCase; @@ -166,6 +167,46 @@ static function (string $warning) use (&$warnings): void { ); } + public function testEnricherMarksAdvisoryUnavailableWhenResolverOmitsIt(): void + { + $resolver = $this->createMock(CveResolverInterface::class); + + $resolver + ->expects(self::once()) + ->method('resolveMany') + ->with(['GHSA-aaaa-bbbb-cccc', 'GHSA-35jh-r3h4-6jhm']) + ->willReturn(['GHSA-35jh-r3h4-6jhm' => new CveResolution(['CVE-2021-23337'], CveStatus::RESOLVED)]); + + $warnings = []; + + $enriched = (new CveEnricher($resolver))->enrich( + new AuditReport( + 'npm', + [ + $this->finding('example-package', 'GHSA-aaaa-bbbb-cccc'), + $this->finding('lodash', 'GHSA-35jh-r3h4-6jhm'), + ], + ), + static function (string $warning) use (&$warnings): void { + $warnings[] = $warning; + }, + ); + + self::assertSame( + [CveStatus::UNAVAILABLE, CveStatus::RESOLVED], + array_map(static fn(AuditFinding $finding): CveStatus => $finding->cveStatus, $enriched->findings), + 'Only the omitted advisory must be marked unavailable.', + ); + self::assertSame( + [ + 'Unable to resolve CVE identifiers for GHSA-aaaa-bbbb-cccc: ' + . Message::AUDIT_CVE_RESOLUTION_MISSING->getMessage('GHSA-aaaa-bbbb-cccc'), + ], + $warnings, + 'The omitted advisory must produce one warning.', + ); + } + public function testEnricherMarksEveryAdvisoryUnavailableWhenBatchFails(): void { $resolver = $this->createMock(CveResolverInterface::class); diff --git a/tests/Native/NpmRegistryTest.php b/tests/Native/NpmRegistryTest.php index c942a84..84fcfb9 100644 --- a/tests/Native/NpmRegistryTest.php +++ b/tests/Native/NpmRegistryTest.php @@ -328,6 +328,30 @@ public function testPrefetchTarballsCachesVerifiedDownloadForLaterFetch(): void ); } + public function testPrefetchTarballsDiscardsDownloadWhenCachingThrows(): void + { + $io = self::verboseIo(); + $cache = $this->createMock(Cache::class); + + $cache->method('isEnabled')->willReturn(true); + $cache->method('isReadOnly')->willReturn(false); + $cache->method('getAge')->willReturn(false); + $cache->method('copyFrom')->willThrowException(new RuntimeException('disk full')); + + $this->registry($this->prefetchDownloader(1, self::serve(NpmRegistryProvider::TARBALL)), $cache, $io) + ->prefetchTarballs([$this->package()]); + + self::assertFileDoesNotExist( + $this->copies[0][1], + 'Temporary file must be removed when caching fails.', + ); + self::assertSame( + 'Prefetching the tarball of "@scope/pkg" (1.0.0) failed: disk full' . PHP_EOL, + $io->getOutput(), + 'Verbose notice must name the package and the caching error.', + ); + } + public function testPrefetchTarballsDiscardsDownloadWithMismatchingIntegrity(): void { $io = self::verboseIo(); diff --git a/tests/Provider/MessageProvider.php b/tests/Provider/MessageProvider.php index 37ee8db..efa76b2 100644 --- a/tests/Provider/MessageProvider.php +++ b/tests/Provider/MessageProvider.php @@ -168,6 +168,11 @@ public static function cases(): iterable ['pnpm', 1, 'ERR_PNPM_AUDIT_BAD_RESPONSE'], 'The pnpm audit command failed with status code 1. ERR_PNPM_AUDIT_BAD_RESPONSE', ]; + yield 'AUDIT_CVE_RESOLUTION_MISSING' => [ + Message::AUDIT_CVE_RESOLUTION_MISSING, + ['GHSA-aaaa-bbbb-cccc'], + 'The CVE resolver returned no result for "GHSA-aaaa-bbbb-cccc".', + ]; yield 'AUDIT_DENO_ADVISORY_ACTIONS_MISMATCH' => [ Message::AUDIT_DENO_ADVISORY_ACTIONS_MISMATCH, ['advisory 2'], From 83c614fb7c0b01c207fdb7c05d030ed0d0acf710 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Sat, 10 Oct 2026 19:03:26 -0300 Subject: [PATCH 7/8] Apply fixed Codex review. --- UPGRADE.md | 3 +- src/Native/NativeInstaller.php | 2 +- src/Native/NpmRegistry.php | 23 ++++++++++++---- src/Native/NpmRegistryInterface.php | 6 ++-- tests/Fixtures/Native/InMemoryRegistry.php | 4 +-- tests/Native/NativeInstallerTest.php | 12 ++++++-- tests/Native/NpmRegistryTest.php | 32 +++++++++++----------- 7 files changed, 51 insertions(+), 31 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 29e2fc4..277b913 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -32,7 +32,8 @@ were removed. The `native` manager now supports `composer foxy:audit` through the registry's bulk advisories endpoint; `--no-dev` is rejected for it, as for Deno, because the native manager installs development dependencies together with the others. `Foxy\Asset\NativeManager` takes the `NpmRegistryInterface` as its last constructor argument, and -`NpmRegistryInterface` gained `getAdvisories(array $versions): string` and `prefetchTarballs(array $packages): void`; +`NpmRegistryInterface` gained `getAdvisories(array $versions): string` and +`prefetchTarballs(array $packages, string $directory): void`; custom registries must implement both. The registry now uses the HTTP downloader of Composer's loop, so the parallel downloads need no configuration. diff --git a/src/Native/NativeInstaller.php b/src/Native/NativeInstaller.php index e631b10..6032910 100644 --- a/src/Native/NativeInstaller.php +++ b/src/Native/NativeInstaller.php @@ -244,7 +244,7 @@ private function installPackages(string $manifestPath, string $installDirectory, } if ([] !== $downloads) { - $this->registry->prefetchTarballs($downloads); + $this->registry->prefetchTarballs($downloads, $temporary); } foreach ($packages as $package) { diff --git a/src/Native/NpmRegistry.php b/src/Native/NpmRegistry.php index ddb297f..5ec2f24 100644 --- a/src/Native/NpmRegistry.php +++ b/src/Native/NpmRegistry.php @@ -29,8 +29,6 @@ use function str_starts_with; use function strlen; use function substr; -use function sys_get_temp_dir; -use function uniqid; use function unlink; use const JSON_THROW_ON_ERROR; @@ -87,8 +85,9 @@ public function fetchTarball(ResolvedPackage $package, string $directory): strin { $resolved = $package->resolved ?? ''; $integrity = $package->integrity ?? ''; + $key = $this->tarballKey($package); - $file = "{$directory}/" . str_replace(['@', '/'], ['', '-'], $package->name) . "-{$package->version}.tgz"; + $file = $this->tarballFile($package, $directory, 'tgz'); if ($this->cache->copyTo($key, $file)) { if ($this->matchesIntegrity($file, $integrity)) { @@ -181,11 +180,13 @@ public function getMetadata(string $name): PackageMetadata * Nothing is downloaded when the cache is disabled or read-only, since the downloads could not be kept. A package * without a tarball URL is skipped. A download that fails, does not match its integrity value, or cannot be * cached is discarded with a verbose notice; {@see fetchTarball()} downloads it again and reports the error. - * Every temporary file is removed before the method returns. + * Each download is written into the given directory, as {@see fetchTarball()} does, and removed before the + * method returns. * * @param list $packages Registry packages whose tarballs are wanted. + * @param string $directory Existing directory that holds each tarball while it is verified. */ - public function prefetchTarballs(array $packages): void + public function prefetchTarballs(array $packages, string $directory): void { if (!$this->cache->isEnabled() || $this->cache->isReadOnly()) { return; @@ -195,13 +196,14 @@ public function prefetchTarballs(array $packages): void foreach ($packages as $package) { $resolved = $package->resolved ?? ''; + $key = $this->tarballKey($package); if ('' === $resolved || false !== $this->cache->getAge($key)) { continue; } - $file = sys_get_temp_dir() . '/' . uniqid('foxy-tarball-', true) . '.tgz'; + $file = $this->tarballFile($package, $directory, 'prefetch'); $this->httpDownloader->addCopy($resolved, $file)->then( function () use ($package, $key, $file, $resolved): void { @@ -351,6 +353,15 @@ private function send(string $url, Closure $request, string|null $notFound = nul } } + /** + * Returns the path of a package's tarball inside the directory: the name with `@` dropped and `/` as `-`, the + * version, and the extension. + */ + private function tarballFile(ResolvedPackage $package, string $directory, string $extension): string + { + return "{$directory}/" . str_replace(['@', '/'], ['', '-'], $package->name) . "-{$package->version}.{$extension}"; + } + /** * Returns the cache key of a package's tarball: name, version, and the SHA-1 of the tarball URL. */ diff --git a/src/Native/NpmRegistryInterface.php b/src/Native/NpmRegistryInterface.php index d6b69df..4cecb99 100644 --- a/src/Native/NpmRegistryInterface.php +++ b/src/Native/NpmRegistryInterface.php @@ -43,9 +43,11 @@ public function getMetadata(string $name): PackageMetadata; * Downloads the tarballs missing from the cache in parallel, verifying each before it is cached. * * The prefetch is best-effort: a tarball that fails to download or to verify is left out of the cache, and the - * later {@see fetchTarball()} downloads it again and reports the error. + * later {@see fetchTarball()} downloads it again and reports the error. Each download lands in the given + * directory until it is verified, never in a shared temporary directory. * * @param list $packages Registry packages whose tarballs are wanted. + * @param string $directory Existing directory that holds each tarball while it is verified. */ - public function prefetchTarballs(array $packages): void; + public function prefetchTarballs(array $packages, string $directory): void; } diff --git a/tests/Fixtures/Native/InMemoryRegistry.php b/tests/Fixtures/Native/InMemoryRegistry.php index 3931d80..0ca73c2 100644 --- a/tests/Fixtures/Native/InMemoryRegistry.php +++ b/tests/Fixtures/Native/InMemoryRegistry.php @@ -121,7 +121,7 @@ public function getMetadata(string $name): PackageMetadata /** * Records the package names and the number of tarballs fetched so far, without side effects. */ - public function prefetchTarballs(array $packages): void + public function prefetchTarballs(array $packages, string $directory): void { $names = []; @@ -129,6 +129,6 @@ public function prefetchTarballs(array $packages): void $names[] = $package->name; } - $this->prefetched[] = ['names' => $names, 'fetchedBefore' => count($this->fetched)]; + $this->prefetched[] = ['names' => $names, 'directory' => $directory, 'fetchedBefore' => count($this->fetched)]; } } diff --git a/tests/Native/NativeInstallerTest.php b/tests/Native/NativeInstallerTest.php index a8ab90b..537cef1 100644 --- a/tests/Native/NativeInstallerTest.php +++ b/tests/Native/NativeInstallerTest.php @@ -458,7 +458,13 @@ public function testInstallPrefetchesRegistryPackagesOnceBeforeFetching(): void $this->install($registry); self::assertSame( - [['names' => ['@popperjs/core', 'bootstrap', 'left-pad'], 'fetchedBefore' => 0]], + [ + [ + 'names' => ['@popperjs/core', 'bootstrap', 'left-pad'], + 'directory' => "{$this->root}/node_modules/.foxy-tmp", + 'fetchedBefore' => 0, + ], + ], $registry->prefetched, 'One prefetch must list every registry package, without the local one, before any fetch.', ); @@ -479,8 +485,8 @@ public function testInstallPrefetchesUnchangedPackagesAgainFromFreshLock(): void self::assertSame( [ - ['names' => ['left-pad'], 'fetchedBefore' => 0], - ['names' => ['left-pad'], 'fetchedBefore' => 1], + ['names' => ['left-pad'], 'directory' => "{$this->root}/node_modules/.foxy-tmp", 'fetchedBefore' => 0], + ['names' => ['left-pad'], 'directory' => "{$this->root}/node_modules/.foxy-tmp", 'fetchedBefore' => 1], ], $registry->prefetched, 'A reinstall from the lock must prefetch the locked packages again.', diff --git a/tests/Native/NpmRegistryTest.php b/tests/Native/NpmRegistryTest.php index 84fcfb9..668b058 100644 --- a/tests/Native/NpmRegistryTest.php +++ b/tests/Native/NpmRegistryTest.php @@ -25,7 +25,6 @@ use function file_put_contents; use function hash; use function mkdir; -use function preg_quote; use function React\Promise\reject; use function React\Promise\resolve; use function sha1; @@ -310,7 +309,7 @@ public function testPrefetchTarballsCachesVerifiedDownloadForLaterFetch(): void { $registry = $this->registry($this->prefetchDownloader(1, self::serve(NpmRegistryProvider::TARBALL))); - $registry->prefetchTarballs([$this->package()]); + $registry->prefetchTarballs([$this->package()], $this->directory); self::assertSame( NpmRegistryProvider::TARBALL, @@ -339,7 +338,7 @@ public function testPrefetchTarballsDiscardsDownloadWhenCachingThrows(): void $cache->method('copyFrom')->willThrowException(new RuntimeException('disk full')); $this->registry($this->prefetchDownloader(1, self::serve(NpmRegistryProvider::TARBALL)), $cache, $io) - ->prefetchTarballs([$this->package()]); + ->prefetchTarballs([$this->package()], $this->directory); self::assertFileDoesNotExist( $this->copies[0][1], @@ -357,7 +356,7 @@ public function testPrefetchTarballsDiscardsDownloadWithMismatchingIntegrity(): $io = self::verboseIo(); $this->registry($this->prefetchDownloader(1, self::serve('corrupted')), io: $io) - ->prefetchTarballs([$this->package()]); + ->prefetchTarballs([$this->package()], $this->directory); self::assertFileDoesNotExist( $this->cacheEntry(), @@ -380,7 +379,7 @@ public function testPrefetchTarballsDiscardsPartialFileOfRejectedDownload(): voi $io = self::verboseIo(); $this->registry($this->prefetchDownloader(1, self::fail500(true)), io: $io) - ->prefetchTarballs([$this->package()]); + ->prefetchTarballs([$this->package()], $this->directory); self::assertFileDoesNotExist( $this->cacheEntry(), @@ -406,7 +405,7 @@ public function testPrefetchTarballsDownloadsMissingPackagesWithOneWait(): void self::TARBALL_URL === $url ? NpmRegistryProvider::TARBALL : self::LEFT_PAD_TARBALL, )($url, $to), ), - )->prefetchTarballs([$this->package(), $this->leftPad()]); + )->prefetchTarballs([$this->package(), $this->leftPad()], $this->directory); self::assertSame( [self::TARBALL_URL, self::LEFT_PAD_URL], @@ -418,10 +417,10 @@ public function testPrefetchTarballsDownloadsMissingPackagesWithOneWait(): void $this->copies[1][1], 'Temporary files must have unique names.', ); - self::assertMatchesRegularExpression( - '{^' . preg_quote(sys_get_temp_dir()) . '/foxy-tarball-[0-9a-f]{13}\d\.\d{8}\.tgz$}', + self::assertSame( + "{$this->directory}/scope-pkg-1.0.0.prefetch", $this->copies[0][1], - 'Temporary file must be a unique `.tgz` in the system temporary directory.', + 'Temporary file must live in the given directory, named after the package.', ); self::assertSame( NpmRegistryProvider::TARBALL, @@ -441,7 +440,7 @@ public function testPrefetchTarballsDownloadsMissingPackagesWithOneWait(): void public function testPrefetchTarballsHidesFailureNoticeAtNormalVerbosity(): void { - $this->registry($this->prefetchDownloader(1, self::fail500(false)))->prefetchTarballs([$this->package()]); + $this->registry($this->prefetchDownloader(1, self::fail500(false)))->prefetchTarballs([$this->package()], $this->directory); self::assertSame( '', @@ -455,7 +454,7 @@ public function testPrefetchTarballsIgnoresRejectedDownload(): void $io = self::verboseIo(); $this->registry($this->prefetchDownloader(1, self::fail500(false)), io: $io) - ->prefetchTarballs([$this->package()]); + ->prefetchTarballs([$this->package()], $this->directory); self::assertFileDoesNotExist( $this->cacheEntry(), @@ -476,7 +475,7 @@ public function testPrefetchTarballsRequestsNothingForCachedPackages(): void { $this->seedCache(NpmRegistryProvider::TARBALL); - $this->registry($this->idleDownloader())->prefetchTarballs([$this->package()]); + $this->registry($this->idleDownloader())->prefetchTarballs([$this->package()], $this->directory); self::assertSame( [], @@ -487,7 +486,7 @@ public function testPrefetchTarballsRequestsNothingForCachedPackages(): void public function testPrefetchTarballsRequestsNothingForEmptyList(): void { - $this->registry($this->idleDownloader())->prefetchTarballs([]); + $this->registry($this->idleDownloader())->prefetchTarballs([], $this->directory); self::assertSame( [], @@ -505,7 +504,7 @@ public function isEnabled(): bool } }; - $this->registry($this->idleDownloader(), $cache)->prefetchTarballs([$this->package()]); + $this->registry($this->idleDownloader(), $cache)->prefetchTarballs([$this->package()], $this->directory); self::assertFileDoesNotExist( $this->cacheEntry(), @@ -517,6 +516,7 @@ public function testPrefetchTarballsRequestsNothingWithoutTarballUrl(): void { $this->registry($this->idleDownloader())->prefetchTarballs( [new ResolvedPackage('no-url', '1.0.0'), new ResolvedPackage('empty-url', '1.0.0', '', 'sha512-AAAA')], + $this->directory, ); self::assertSame( @@ -530,7 +530,7 @@ public function testPrefetchTarballsRequestsNothingWithReadOnlyCache(): void { $cache = new Cache(new NullIO(), $this->cacheDir, 'a-z0-9_./', null, true); - $this->registry($this->idleDownloader(), $cache)->prefetchTarballs([$this->package()]); + $this->registry($this->idleDownloader(), $cache)->prefetchTarballs([$this->package()], $this->directory); self::assertFileDoesNotExist( $this->cacheEntry(), @@ -543,7 +543,7 @@ public function testPrefetchTarballsSkipsCachedPackageAndRequestsMissingOne(): v $this->seedCache(NpmRegistryProvider::TARBALL); $this->registry($this->prefetchDownloader(1, self::serve(self::LEFT_PAD_TARBALL))) - ->prefetchTarballs([$this->package(), $this->leftPad()]); + ->prefetchTarballs([$this->package(), $this->leftPad()], $this->directory); self::assertSame( self::LEFT_PAD_URL, From 647982c40df4b5838b6eeaabd5ee424b9ab97a1c Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Sat, 10 Oct 2026 19:13:35 -0300 Subject: [PATCH 8/8] Apply fixed Codex review. --- src/Native/NpmRegistry.php | 13 +++++----- tests/Native/NpmRegistryTest.php | 41 +++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/Native/NpmRegistry.php b/src/Native/NpmRegistry.php index 5ec2f24..86a5406 100644 --- a/src/Native/NpmRegistry.php +++ b/src/Native/NpmRegistry.php @@ -87,7 +87,7 @@ public function fetchTarball(ResolvedPackage $package, string $directory): strin $integrity = $package->integrity ?? ''; $key = $this->tarballKey($package); - $file = $this->tarballFile($package, $directory, 'tgz'); + $file = $this->tarballFile($package, $directory, '.tgz'); if ($this->cache->copyTo($key, $file)) { if ($this->matchesIntegrity($file, $integrity)) { @@ -203,7 +203,8 @@ public function prefetchTarballs(array $packages, string $directory): void continue; } - $file = $this->tarballFile($package, $directory, 'prefetch'); + // The URL hash keeps `@scope/pkg` and `scope-pkg` apart while their downloads run side by side. + $file = $this->tarballFile($package, $directory, '-' . sha1($resolved) . '.prefetch'); $this->httpDownloader->addCopy($resolved, $file)->then( function () use ($package, $key, $file, $resolved): void { @@ -354,12 +355,12 @@ private function send(string $url, Closure $request, string|null $notFound = nul } /** - * Returns the path of a package's tarball inside the directory: the name with `@` dropped and `/` as `-`, the - * version, and the extension. + * Returns the path of a package's file inside the directory: the name with `@` dropped and `/` as `-`, the + * version, and the suffix. */ - private function tarballFile(ResolvedPackage $package, string $directory, string $extension): string + private function tarballFile(ResolvedPackage $package, string $directory, string $suffix): string { - return "{$directory}/" . str_replace(['@', '/'], ['', '-'], $package->name) . "-{$package->version}.{$extension}"; + return "{$directory}/" . str_replace(['@', '/'], ['', '-'], $package->name) . "-{$package->version}{$suffix}"; } /** diff --git a/tests/Native/NpmRegistryTest.php b/tests/Native/NpmRegistryTest.php index 668b058..19f7438 100644 --- a/tests/Native/NpmRegistryTest.php +++ b/tests/Native/NpmRegistryTest.php @@ -418,7 +418,7 @@ public function testPrefetchTarballsDownloadsMissingPackagesWithOneWait(): void 'Temporary files must have unique names.', ); self::assertSame( - "{$this->directory}/scope-pkg-1.0.0.prefetch", + "{$this->directory}/scope-pkg-1.0.0-" . sha1(self::TARBALL_URL) . '.prefetch', $this->copies[0][1], 'Temporary file must live in the given directory, named after the package.', ); @@ -471,6 +471,45 @@ public function testPrefetchTarballsIgnoresRejectedDownload(): void ); } + public function testPrefetchTarballsKeepsCollidingNamesApart(): void + { + $unscoped = new ResolvedPackage( + 'scope-pkg', + '1.0.0', + self::LEFT_PAD_URL, + 'sha512-' . base64_encode(hash('sha512', self::LEFT_PAD_TARBALL, true)), + ); + + $registry = $this->registry( + $this->prefetchDownloader( + 2, + static fn(string $url, string $to): PromiseInterface => self::serve( + self::TARBALL_URL === $url ? NpmRegistryProvider::TARBALL : self::LEFT_PAD_TARBALL, + )($url, $to), + ), + ); + $registry->prefetchTarballs([$this->package(), $unscoped], $this->directory); + + self::assertSame( + [ + "{$this->directory}/scope-pkg-1.0.0-" . sha1(self::TARBALL_URL) . '.prefetch', + "{$this->directory}/scope-pkg-1.0.0-" . sha1(self::LEFT_PAD_URL) . '.prefetch', + ], + [$this->copies[0][1], $this->copies[1][1]], + 'Packages sharing a normalized name must get distinct temporary files.', + ); + self::assertSame( + NpmRegistryProvider::TARBALL, + file_get_contents($this->cacheEntry()), + 'Scoped package must be cached.', + ); + self::assertSame( + self::LEFT_PAD_TARBALL, + file_get_contents($registry->fetchTarball($unscoped, $this->directory)), + 'Unscoped package must be served from the cache.', + ); + } + public function testPrefetchTarballsRequestsNothingForCachedPackages(): void { $this->seedCache(NpmRegistryProvider::TARBALL);