Skip to content

feat(cli): add Symfony Console plumbing and migrate two pilot scripts#7075

Closed
somethingwithproof wants to merge 12 commits into
Cacti:developfrom
somethingwithproof:feat/develop-symfony-console-cli
Closed

feat(cli): add Symfony Console plumbing and migrate two pilot scripts#7075
somethingwithproof wants to merge 12 commits into
Cacti:developfrom
somethingwithproof:feat/develop-symfony-console-cli

Conversation

@somethingwithproof
Copy link
Copy Markdown
Contributor

@somethingwithproof somethingwithproof commented Apr 28, 2026

Current Status

Consolidated into #7189. Keep this PR only as historical review context for the original Symfony Console extraction; the merge direction should be the DI-friendly component baseline in #7189.

Original Scope

  • Introduced Symfony Console integration for Cacti command entry points.
  • Started the Cacti command abstraction used by later Symfony component work.

Recommendation

Do not merge this independently unless it is deliberately rebased to match #7189. Console work should follow the #7189 direction, including Cacti-owned command abstractions and helper access such as FormatterHelper.

Copilot AI review requested due to automatic review settings April 28, 2026 03:37
@somethingwithproof somethingwithproof force-pushed the feat/develop-symfony-console-cli branch from 98b2352 to 97aa3f6 Compare April 28, 2026 03:45
@somethingwithproof somethingwithproof force-pushed the feat/develop-symfony-console-cli branch 4 times, most recently from b45c410 to a69c598 Compare April 28, 2026 07:45
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>
@somethingwithproof somethingwithproof force-pushed the feat/develop-symfony-console-cli branch from a69c598 to a27fbb8 Compare April 28, 2026 08:20
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>
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>
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>
somethingwithproof added a commit to somethingwithproof/cacti that referenced this pull request May 8, 2026
Match the flat un-namespaced lib/CactiX.php convention used by Cacti#7088,
Cacti#7073, Cacti#7077, Cacti#7075. Drop symfony/process (added by Cacti#7073) and
symfony/validator (added by Cacti#7077) to avoid composer.json conflicts at
merge time.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof added a commit to somethingwithproof/cacti that referenced this pull request May 8, 2026
Match the un-namespaced lib/CactiX.php convention used by canonical PRs
(Cacti#7088 CactiProcess, Cacti#7073 CactiMime, Cacti#7077 CactiSettings, Cacti#7075
CactiApplication/CactiCommand). The previous nested PSR-4 namespace
required composer autoload changes that conflicted with those PRs.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof added a commit to somethingwithproof/cacti that referenced this pull request May 8, 2026
Rename lib/Security/CactiOAuth.php to lib/CactiOAuth.php and drop the
Cacti\Security namespace. Update the two callers (oauth2.php and
lib/functions.php mailer) to use the unqualified class name. Behavior
is unchanged: only the FQCN was rewritten.

The flat layout matches every other lib/Cacti*.php class on develop and
in the canonical PRs (Cacti#7088, Cacti#7073, Cacti#7077, Cacti#7075).

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof
Copy link
Copy Markdown
Contributor Author

Queue note: this should follow #7073. The Process/Mime helper PR is the lower-level security foundation; this Symfony Console migration has a larger CLI blast radius and should be reviewed after that direction is accepted.

TheWitness added a commit that referenced this pull request May 12, 2026
…7124)

* test: shared hand-off + mutation infrastructure for feature PRs

Consolidates infection/infection dev dep, allow-plugins entry, baseline
infection.json5, tests/HandOff/HandOffHelpers.php with reusable stubs
(cacti_log capture, temp-file fixtures, minimal-zip builder), HandOff
testsuite registration, and the documented pattern. Feature PRs that
add hand-off coverage rebase onto this and contribute only ONE
HandOffTest file plus an optional infection.json5 filter override.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: consolidate per-feature hand-off suites into shared infra

Each test guards on its feature file and skips when not present on develop.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(test): replace eval stub, unify log buffer ref, guard ZipArchive add

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(ci): correct infection vendor path in infection.json5

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci(phpstan): baseline upstream undefined-variable and isset errors

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: add HandOff and Integration testsuites; add regression guards

- phpunit.xml: add Integration testsuite alongside existing Unit/HandOff
- tests/Integration/: DbDumpIntegrationTest, PingIntegrationTest (from develop), CactiProcessIntegrationTest, SqlScriptsIntegrationTest
- tests/HandOff/RegressionGuardTest: guards against backsliding on shell_exec, password-in-argv, RLIKE concat, and open-redirect patterns

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* chore(test): pest test infrastructure and PSR-4 autoload

Add Pest 2 dev dependency, regenerate composer.lock under PHP 8.1, declare PSR-4 autoload for the Cacti namespace, and seed the test bootstrap with three unit tests plus an orb integration check. Pest stays on the v2 line because v3 transitively requires PHP 8.2 and breaks the 8.1 matrix entry.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* chore(phpstan): catch up baseline with 11 upstream-detected entries

Cacti Commit Audit fails on develop with 11 PHPStan errors that are
not yet in phpstan-baseline.neon. Append the matching entries so the
PR-A branch passes Run PHPStan at Level 6. Same set of entries
already exists on PR #7077; this is a transient catch-up that
upstream will absorb when those entries land on develop.

Files: aggregate_graphs.php, color_templates.php, graph_templates.php,
graphs.php, lib/html.php (3 entries).

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* chore(test): drop composer.lock; let CI resolve per PHP version

The lock file pinned brianium/paratest v7.3.1 which only supports
PHP 8.1-8.3, so the 8.4 matrix entry rejected the lock with
"Your lock file does not contain a compatible set of packages".
A single lock cannot satisfy 8.1 (paratest 7.3.x) and 8.4 (paratest
7.4.x) at the same time because pestphp/pest 2.36.0 is the last
2.x release that supports 8.1 and the 8.1-compatible paratest tag
predates 8.4 support.

Match upstream develop's behaviour: no committed lock; let
`composer install` in CI resolve per matrix entry. Pest stays on
the v2 line in composer.json so 8.1 still gets a compatible Pest.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(test): apply Copilot review feedback for test infra

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* chore(test): drop psr-4 autoload and align deps with canonical PRs

Match the flat un-namespaced lib/CactiX.php convention used by #7088,
#7073, #7077, #7075. Drop symfony/process (added by #7073) and
symfony/validator (added by #7077) to avoid composer.json conflicts at
merge time.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: skip integration suites and regression guards pending feature PRs

Guards integration tests and regression guards from #7083 against
absence of CactiProcess and other feature-PR hardening, so the
consolidated foundation PR is green on develop. Suites activate
automatically once their feature PRs merge.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(ci): drop infection/infection to unbreak PHP 8.4 matrix

infection/infection ^0.27 transitively pins thecodingmachine/safe
^2.1.2, resolving to v2.5.0. Its generated stubs use implicit-nullable
parameter declarations, which PHP 8.4 emits as deprecations into
cacti.log and trips the log-quiet assertion. infection has no config
or test wiring in this repo, so removing the dev dep clears the
transitive pin without losing functionality.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
TheWitness added a commit that referenced this pull request May 13, 2026
* refactor(oauth): extract Cacti\Security\CactiOAuth provider factory

Replace the inline provider switch in oauth2.php and the OAuth
branch of mailer() in lib/functions.php with two helpers:

- CactiOAuth::getProvider($name, $params) returns the configured
  provider instance for `google`, `azure`, `yahoo`, or `microsoft`,
  or null when the configured provider is unknown.
- CactiOAuth::getDefaultOptions($name) returns the scope set each
  provider expects.

Behaviour is preserved; both call sites now branch on a null
return rather than relying on a `$provider` variable that may have
fallen through the switch unset.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* chore(phpstan): catch up baseline with 11 upstream-detected entries

Same as PR-A: appends 11 PHPStan ignoreErrors entries that exist on
upstream develop but are not yet baselined, so this branch's CI does
not regress on phpstan analyse --level 6.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor: flatten CactiOAuth to global namespace

Rename lib/Security/CactiOAuth.php to lib/CactiOAuth.php and drop the
Cacti\Security namespace. Update the two callers (oauth2.php and
lib/functions.php mailer) to use the unqualified class name. Behavior
is unchanged: only the FQCN was rewritten.

The flat layout matches every other lib/Cacti*.php class on develop and
in the canonical PRs (#7088, #7073, #7077, #7075).

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: add shared unit stubs for OAuth tests

* fix(oauth): load provider factory in runtime paths

* test(oauth): pin provider factory runtime loading

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
# Conflicts:
#	cmd_realtime.php
#	composer.json
#	phpunit.xml
#	tests/Unit/GraphRealtimeShellTest.php
@somethingwithproof
Copy link
Copy Markdown
Contributor Author

Closing as superseded by #7189, which carries this work forward as the DI-friendly component baseline. Leaving the branch in place so the review history stays accessible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant