Skip to content

hardening: move to argument array pattern for shell execution (#6854)#6855

Merged
TheWitness merged 10 commits intoCacti:developfrom
somethingwithproof:fix/hardening-arg-array-6854
Mar 24, 2026
Merged

hardening: move to argument array pattern for shell execution (#6854)#6855
TheWitness merged 10 commits intoCacti:developfrom
somethingwithproof:fix/hardening-arg-array-6854

Conversation

@somethingwithproof
Copy link
Copy Markdown
Contributor

@somethingwithproof somethingwithproof commented Mar 20, 2026

This PR introduces a structural hardening change to the shell execution layer by allowing __rrd_execute() and exec_background() to accept an array of arguments.

When an array is provided, the function automatically applies cacti_escapeshellarg() to each element and joins them with spaces. This provides a safe, idiomatic path for executing commands without risky string concatenation.

Fixes #6854

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof marked this pull request as draft March 20, 2026 10:00
@somethingwithproof somethingwithproof marked this pull request as ready for review March 23, 2026 13:59
TheWitness
TheWitness previously approved these changes Mar 23, 2026
@TheWitness
Copy link
Copy Markdown
Member

@somethingwithproof, Still failing php-cs-fixer texted. Might be from a different pull.

image

- Auto-fix php-cs-fixer issues in lib/rrd.php and lib/poller.php
- Replace undefined cacti_redirect() with header()+exit in oauth2.php

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof added a commit to somethingwithproof/cacti that referenced this pull request Mar 24, 2026
- Revert lib/poller.php and lib/rrd.php arg-array changes (belong in PR Cacti#6855)
- Remove ArgArrayHardeningTest.php (belongs in PR Cacti#6855)
- Revert vendor/composer updates (should be a separate PR)

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
TheWitness added a commit that referenced this pull request Mar 24, 2026
* fix: shellcheck fixes, codespell config, and actionlint config

- Add .github/actionlint.yaml with no-shellcheck to disable
  false positives in run: blocks
- Update .codespell.cfg to skip third-party assets, themes,
  mock data; add intentional words to ignore list
- Fix shellcheck issues in all shell scripts: quoting, egrep
  to grep -E, glob iteration, bare redirects, SIGKILL trap

Does not touch workflow YAML files (handled in #6874).

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

* hardening: move to argument array pattern for shell execution (#6854)

* docs: update exec_background docblock for string|array param types

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

* docs: add PHPDoc to __rrd_execute for PHPStan

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

* fix: resolve php-cs-fixer formatting issues

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

* revert: remove oauth2.php fix (moved to standalone PR #6886)

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

* chore: remove files that belong in other PRs

- Revert lib/poller.php and lib/rrd.php arg-array changes (belong in PR #6855)
- Remove ArgArrayHardeningTest.php (belongs in PR #6855)
- Revert vendor/composer updates (should be a separate PR)

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

* chore: remove composer.lock from tracking

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
@TheWitness
Copy link
Copy Markdown
Member

Darn, this likely slipped in on one of the other pull requests. This is why pre-commit hooks are so important.

image

@TheWitness TheWitness merged commit c869d9b into Cacti:develop Mar 24, 2026
5 checks passed
@somethingwithproof somethingwithproof deleted the fix/hardening-arg-array-6854 branch March 24, 2026 22:58
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.

hardening: move to argument array pattern for shell execution

2 participants