1 Commits
Author SHA1 Message Date
Thomas VincentandTheWitness 6cbe01bff1 feat(security): add cacti_dispatch helper for action table authorisation (#7063)
* feat(security): add cacti_dispatch helper for action table authorisation

cacti_dispatch() wraps the repeated switch/case on get_request_var('action')
that every Cacti controller writes. Each action is declared with the
HTTP method it accepts, the realm id that must be allowed, and an
optional object-level ACL callback. The guards run in method then
realm then object-ACL order and each one logs with a WEBUI category
before returning a 405 or 403, so review across controllers no longer
depends on every author remembering to add the same three checks in
the same order.

The test file source-scans the helper for each guard and for the guard
ordering, so a future refactor that silently drops the realm or ACL
branch fails CI rather than regressing authorisation for every
controller that opts in.

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

* security(dispatch): address Copilot review on cacti_dispatch

Reject non-string action inputs before offset access so ?action[]=x
cannot produce a TypeError. Default REQUEST_METHOD to GET for CLI
and tests, and normalise both sides with cacti_strtoupper so the
comparison is locale-independent. Treat a declared-but-non-callable
object_acl as a misdeclaration and deny (log ERROR + 403) rather
than silently skipping the check. Split the denial emitter into
cacti_dispatch_deny() under a prefixed name to avoid colliding with
the existing raise_ajax_permission_denied() in lib/functions.php,
and always follow the AJAX helper with an explicit http_response_code()
so a denied non-AJAX request cannot fall through to a 200 render.
Reject non-callable callbacks with a 500 and ERROR log. Drop the
redundant docblock on the function and tighten the parameter types.

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

* style(dispatch): apply php-cs-fixer to cacti_dispatch

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

* fix(dispatch): drop tautological is_string checks flagged by phpstan level 6

PHPStan level 6 flagged two is_string() calls as always true:
- $default was already type-hinted as string via PHPDoc; the runtime
  cast to '' was redundant.
- The second is_string($action) at the unknown-action log happens
  after the earlier is_string guard already narrowed the type.

Tighten the signature to string $default so the first cast drops,
and remove the second is_string from the log interpolation since
$action is always a string by the time that branch runs.

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

* fix(dispatch): tighten Copilot review feedback

strspn whitelist on action; explicit AJAX detection for 403 vs redirect;
file_get_contents guard in tests.

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

* test(dispatch): hand-off coverage + infection mutation config

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

* ci(infection): allow infection/extension-installer composer plugin

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

* test(dispatch): drop hand-off test, consolidated into #7083

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

* feat(dispatch): reject mis-declared method, document CSRF scope

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>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-05-12 15:01:49 -04:00