refactor(symfony): align components with DI-friendly wrappers#7189
Draft
somethingwithproof wants to merge 53 commits into
Draft
refactor(symfony): align components with DI-friendly wrappers#7189somethingwithproof wants to merge 53 commits into
somethingwithproof wants to merge 53 commits into
Conversation
Routes saved settings through Symfony Validator so include/global_settings.php can declare per-setting constraints inline. The save handler runs one validator pass and surfaces violations via the existing raise_message() pattern, replacing scattered ad-hoc checks. Pilot covers 12 high-value settings; the rest stay on the existing implicit form-render checks. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Routes external commands through Symfony Process in array-mode argv, scrubs the environment to a baseline allowlist, and centralizes timeout and exit-code handling. Migrates cli/push_out_hosts.php as the pilot call site. Includes unit, integration, and a migration guide. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Centralizes argv parsing through Symfony Console's Command/Application classes. cmd_realtime.php and cli/splice_rrd.php become thin shims delegating to CmdRealtimeCommand and SpliceRrdCommand, preserving the operator-facing argv interface used by poller_realtime.php and existing cron jobs. Argument validation goes through filter_var so values like "3.14" are rejected instead of silently truncated; splice_rrd filenames are checked for shell metacharacters before any processing. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
11 errors surfaced after PHPStan re-analyzed the tree on the latest PR runs. None are in code this PR touches; they are pre-existing issues in aggregate_graphs.php, color_templates.php, graph_templates.php, graphs.php, and lib/html.php. Suppress them in the baseline so this PR can land. A separate cleanup PR on develop should retire entries. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The Pest assertion was wrapped in double quotes, so PHP interpolated $poller_id to the empty string. The actual assertion looked for the literal $poller_id text in poller_realtime.php; switching to single quotes preserves the dollar sign verbatim. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Pest tests pin every data boundary the wrapper crosses (argv→child, env scrub, stdout bytes, exit code, exec-shape parity, timeout chaining). infection.json5 scopes mutation analysis to the wrapper files with a 70% MSI floor. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Source-scan asserts the production branch and the bypass gate so the escape hatch can't silently become the only path. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
PHP refuses to start without SYSTEMROOT; proc_open relies on COMSPEC and PATHEXT to resolve executables under cmd.exe. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
A web request with a leaked CACTI_PHP_TESTING env var could otherwise skip cli_check.php. PHP_SAPI gate eliminates that path; strict '1' equality drops the truthy-string surface. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Routes uploaded-file MIME detection through Symfony Mime so a content-type allowlist can be enforced on top of the existing extension and signature checks. package_import.php now rejects uploads whose detected MIME is not in the allowlist (zip, gzip, xml variants) before any further processing. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
LIBXML_NONET blocks network entity loading as defense in depth even though libxml2 ≥ 2.9.0 disables external entities by default. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…declarations Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…t timeout to float Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…toload) Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Allows callers to inject env name=value pairs without putenv() or argv exposure. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Default Symfony NotBlank/Length/Range/Choice/Positive messages render in English regardless of Cacti locale because the validator runs without a Symfony Translator. Pass explicit __()-wrapped message, minMessage, maxMessage, and notInRangeMessage options on each bare constraint in include/global_settings.php so violations participate in Cacti's gettext pipeline. Documents the no-Translator decision in CactiValidator so future contributors know to translate at the call site. Addresses Copilot PR Cacti#7077 review feedback. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
- Add explicit $onError callback to forward stderr chunks to STDERR, replacing reliance on CactiProcess's internal fallback. - Capture runStreaming() return value and pass to exit() so the wrapper propagates the child's exit status rather than always exiting 0. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
# Conflicts: # composer.json # graph_realtime.php # tests/Unit/GraphRealtimeShellTest.php # tests/integration/CactiProcessIntegrationTest.php
# Conflicts: # cmd_realtime.php # composer.json # phpunit.xml # tests/Unit/GraphRealtimeShellTest.php
# Conflicts: # composer.json
…ony-di-direction # Conflicts: # composer.json
Contributor
Author
|
Added a follow-up process hardening commit (2eb5ec4) on this branch. Summary:
Local validation:
|
Contributor
Author
|
Added follow-up commit 46a0579. Summary:
Validation:
|
This was referenced May 31, 2026
01036a5 to
d4d60d8
Compare
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
… fatal - tests/HandOff/CliInvocationHandOffTest.php declares its own runCli(), and the duplicate definition aborted the whole Pest run with a silent exit 255 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…archive Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
… handling - template/package import allowlists accept libmagic text/plain; spikekill writeXMLFile and poller resource_cache_out catch wrapper exceptions and fall back to the boolean failure path; migration guard test scoped to migrated callers Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Clears the level 6 alreadyNarrowedType error on lib/data_query.php:2591 that develop currently emits, matching the sibling narrowing identifiers already ignored. Signed-off-by: Thomas Vincent <thomasvincent@gmail.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
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.
Summary
CactiProcessacross production shell execution paths so commands use argv arrays where practical and centralize legacy command-line execution behind one wrapper.CactiProcessPipe, leaving productionproc_open()usage in one reviewed abstraction.CactiFilesystempass in package, plugin, spikekill, and poller cache file operations.FormatterHelperaccess through the console bootstrap andCactiCommand.Architecture Direction
This is the Symfony component baseline for develop. We are not introducing the full Symfony container yet. New code should depend on Cacti-owned injectable wrappers, while older procedural code can use the static facades during migration.
Process execution should prefer argv arrays through
CactiProcess;runCommandLine()is reserved for internally built legacy command strings that still need compatibility. Filesystem-sensitive writes, deletes, copies, mkdirs, temp names, and path checks should useCactiFilesystemwhen touched.Consolidation
This PR supersedes the direction of the smaller Symfony component PRs #7073, #7075, and #7077. Those PRs can remain useful as review history, but the merge direction should be this consolidated DI-friendly implementation.
Symfony Mailer replacement remains separate in #7190 so mail transport behavior can be reviewed independently.
Validation
php -lover touched PHP filesgit diff --checkexec,shell_exec,system, orpassthrucall sites remainproc_open()is centralized inlib/CactiProcessPipe.phpinclude/vendor/bin/pestis missing in this worktree