Issues derived from SECURITY-AUDIT.md findings and tooling setup.
- Priority: high
- Labels: [security, hardening, priority: high]
- Branch: hardening/1-thold-column-name-allowlist
- Evidence: setup.php:345,350,386,391
- Acceptance criteria:
- A constant or function defines the exact set of allowed column suffixes:
['hi', 'low', 'warning_hi', 'warning_low'] - Each call site checks
in_array($matches[1], $allowed, true)before interpolation - An unexpected value logs a warning via
cacti_log()and skips the replacement - Existing behaviour is unchanged for valid suffixes
- Unit test in
tests/Security/XssOutputTest.phppasses (allowlist describe block)
- A constant or function defines the exact set of allowed column suffixes:
- Dependencies: none
Issue #2: hardening(sql): replace concatenated DELETE/UPDATE with prepared statements in notify_lists.php
- Priority: high
- Labels: [security, hardening, priority: high, tech-debt]
- Branch: hardening/2-notify-lists-prepared-statements
- Evidence: notify_lists.php:484,488
- Acceptance criteria:
-
db_execute('DELETE ... thold_id=' . $selected_items[$i])replaced withdb_execute_prepared(..., [(int) $selected_items[$i]]) -
db_execute('UPDATE thold_data SET notify_alert=' . ...)replaced with prepared equivalent - All other concatenated queries in notify_lists.php audited and converted
- Existing test suite passes
-
- Dependencies: none
- Priority: high
- Labels: [security, hardening, priority: high]
- Branch: hardening/3-thold-webapi-context-guard
- Evidence: thold_webapi.php:1 (missing context guard)
- Acceptance criteria:
- A
defined('IN_CACTI_CONTEXT') or die(...)guard is added at the top of thold_webapi.php -
thold.phpdefinesIN_CACTI_CONTEXTbefore including thold_webapi.php - Direct HTTP GET to thold_webapi.php returns a non-200 response or no output
- Existing functionality via thold.php is unaffected
- A
- Dependencies: none
- Priority: medium
- Labels: [security, hardening, priority: medium]
- Branch: hardening/4-thold-csrf-tokens
- Evidence: notify_lists.php:537, thold.php:386, thold_templates.php:241
- Acceptance criteria:
-
form_verify_token()(or Cacti equivalent) called before each bulk-action handler - Invalid token results in HTTP 400 and no state change
- CSRF test in
tests/Security/XssOutputTest.phppasses (after seam is created)
-
- Dependencies: Issue #5 (seam)
- Priority: medium
- Labels: [tech-debt, testability]
- Branch: tech-debt/5-thold-src-seams
- Evidence: notify_lists.php:484, setup.php:345, thold_webapi.php action handlers
- Acceptance criteria:
-
src/TholdColumnResolver.phpprovidesresolve(string $suffix): stringwith allowlist -
src/NotificationListRepository.phpprovides typed methods for delete/update operations -
src/WebApiDispatcher.phpprovides auth-gated action dispatch - All existing behaviour preserved
- PHPStan level 6 passes on
src/
-
- Dependencies: none
- Priority: medium
- Labels: [tooling, ci]
- Branch: tooling/6-thold-composer-ci
- Evidence: composer.json, phpstan.neon, infection.json, pest.php created in this audit
- Acceptance criteria:
-
composer installsucceeds -
vendor/bin/pestruns and reports test results -
vendor/bin/phpstan analyseruns at level 6 onsrc/andtests/ - CI workflow runs on push and pull_request
- Coverage threshold 80% enforced in CI
-
- Dependencies: Issue #5 (src/ must have testable code)