feat(security): system-wide refactor of execution sinks to array contracts#7172
Closed
somethingwithproof wants to merge 46 commits into
Closed
feat(security): system-wide refactor of execution sinks to array contracts#7172somethingwithproof wants to merge 46 commits into
somethingwithproof wants to merge 46 commits into
Conversation
…eta CSP Route every security header through lib/headers_secure.php so the CSP has one authoritative source. Add object-src, base-uri, form-action, and manifest-src directives. Delete the weaker <meta> CSP that allowed default-src *. Ship .htaccess.dist with static-file header guidance for Apache deployments. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Extend content_security_policy_script with 'nonce' and 'nonce-report' values. Nonce mode replaces 'unsafe-inline' in script-src and style-src with a per-request base64url nonce; report-only mode ships via Content-Security-Policy-Report-Only for staging. CactiSecureHeaders gains getCspMode, isNonceMode, and buildCspPolicy as the plugin-facing contract. Pilot-migrated logout.php and permission_denied.php. Added csp_report.php endpoint + Pest unit tests. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Previous commit created the new files; this one lands the matching edits to headers_secure.php (getCspMode/isNonceMode/buildCspPolicy, base64url nonce, mode branching in emitHeaders), global_settings.php (four-value drop_array with description warning), logout.php and permission_denied.php (nonce attribute on inline scripts), and docs/ security-headers.md (nonce mode, plugin contract, rollback). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Integration tests spin up php -S with a stub read_config_option and assert on response headers for every flag value plus header/DOM nonce match. Playwright suite scaffolded under tests/e2e/ with a 6-test CSP spec, skipped by default until the docker-compose stack lands. CI workflow runs unit and integration legs on push/PR touching the CSP surface; e2e job stubbed until the stack ships. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Add report-uri directive to nonce-mode CSP (reports had nowhere to go). Replace html_escape of the alternate-sources config with a CSP source-list whitelist scrubber that drops ';', CR, LF, and other directive-terminating bytes. Strip C0 controls from CSP-report summary fields before cacti_log so a crafted violated-directive cannot forge log lines. Bump nonce entropy from 128 to 144 bits (24 base64url chars) and mark enforce-mode as [Pilot] in the UI. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…est helper Strip em dashes per repo prose rules. Register the content_security_report_uri setting in global_settings.php so the value read by emitHeaders is reachable from the UI. Delete the no-op _reset_nonce helper in HeadersSecureTest.php. Update nonce length in docs from 22 to 24 to match random_bytes(18). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
MariaDB 10.11 seeds cacti.sql at boot; PHP-FPM 7.4 container mounts the repo and runs an entrypoint that fixes up the version sentinel, creates include/config.php, and clears the default admin's must_change_password flags so Playwright can sign in. nginx fronts 8080. Workflow e2e job flips from a stub to real docker compose run with artifact upload on failure. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Live smoke: 4 runnable Playwright tests pass (2 report-only fixme's). Dockerfile gains oniguruma-dev for mbstring build; compose publishes via HOST_PORT env so local ports like 8090 work around OrbStack's 8080 pre-bind; default CACTI_CSP_MODE flips to nonce-report (matches the realistic staged rollout, un-migrated tags keep working); entrypoint reads the 1.2.x-style include/cacti_version file, sets url_path '/' so nginx serves assets without the /cacti prefix, patches in docker network creds. nginx.conf stops blocking include/js + include/themes and pulls in mime.types so JS/CSS serve with real content types. Playwright spec asserts on report-only header shape + nonce match against logout.php?action=timeout (the always-renderable pilot body). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…dmin login - CACTI_FORCE_CONFIG env overwrites include/config.php each boot so CI and dev re-runs start with creds matching the compose stack. - docker-compose.enforce.yml overlay flips the stack to nonce enforcing mode. Pair with E2E_CSP_ENFORCE=1 to run the full suite against the blocking path. - loginAsAdmin fixture in the spec uses the form's CSRF-bearing hidden inputs, unlocking the permission_denied.php nonce-match test. All 6 tests run in both modes; no more fixme skips. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Cacti's composer.json does not declare Pest as a require-dev today, so composer install in a clean CI environment does not provide the binary. Add an explicit composer require --dev step. tests/e2e does not ship package-lock.json (gitignored to keep the PR diff small) so switch from npm ci to npm install and drop the setup-node cache-dependency-path that pointed at the missing lock. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Composer 2.2+ blocks pestphp/pest-plugin by default; whitelist it scoped to the CI job before composer require. E2E wait loop now requires the Content-Security-Policy-Report-Only header to be present before declaring readiness, since a bare 200 can come from the install wizard bootstrap that skips global.php. Added a diagnostic step dumping php/mariadb logs and the header line so future CI failures surface the entrypoint state without re-running. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Cacti's main composer.json sets vendor-dir to include/vendor/, which confounds Pest 1.x's composer.json discovery walk. Scoping Pest to a tests/ composer.json with its own vendor/ directory gives Pest a path layout it understands without touching the main tree. The test files already live under tests/, so paths stay relative. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Pest needs a phpunit.xml and a Pest.php bootstrap inside the test-runner root. Ship both in tests/. E2E diagnostic step always runs now (not just on failure) and dumps settings rows, url_path line, response headers, and the first 400 bytes of the body so remaining failures surface the real state. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
CI checks out the repo as the runner uid, but php-fpm inside the container runs as www-data. The bind mount preserves host uids, so Cacti's 'System log file is not available for writing' died before emitHeaders() ran, producing a 123-byte bodied 200 with no security headers at all. chmod a+w on log/cache/rra/resource restores write access. Pest's toHaveKey does not take an error-message second argument; using one makes Pest interpret it as the expected value. Drop the message and keep the bare key assertion. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Line-number drift only after CactiSecureHeaders::emitHeaders() integration in include/global.php. No new sinks. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Same line-number drift as the sink baseline regen. No new hotspots. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Wrap CSPRNG calls in try/catch with openssl and non-CSPRNG fallbacks plus logging so an entropy failure cannot take down the web UI when nonce mode is enabled. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
content_security_alternate_sources controls source lists, not violation reporting. Drop the unbacked SRI claim about get_md5_include_js. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Drop stale unsafe-eval-only label, the dead html_escape fixture stub that emitHeaders no longer calls, and the tranche-C playwright note that no longer matches reality. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The Pest sandbox lives at tests/composer.json. The root composer install needed extensions setup-php does not enable and could fail before tests run. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Hardcoded /cacti/csp_report.php broke CSP reporting on installs at /, /cacti2, or behind a rewrite. Now derived from $url_path with fallback to the legacy literal when the global is unset. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Loading lib/functions.php alone produced undefined-$config warnings because it expects a populated $config. This endpoint is unauthenticated and reachable before include/global.php is loaded. Replaced with a self-contained csp_report_log() that prefers cacti_log() when $config is already populated and falls through to error_log() otherwise. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Spec asserted the hardcoded /cacti/csp_report.php URI; the default URI now derives from $url_path. Use a regex that matches the shim filename without pinning the base path. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Without strict-dynamic, jQuery .html()/.append()-injected scripts fail under nonce mode because they do not carry the per-request nonce. Without unsafe-eval, jQuery's globalEval and new Function() paths fail. Both break Cacti core and most plugins (thold, monitor) under enforce. Style-src keeps unsafe-inline since style XSS is a much narrower attack surface and jQuery .css() relies on inline-style execution. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Settings UI description and the security-headers doc both said inline <style> tags require a nonce; in fact only script-src enforces nonces in nonce mode and style-src keeps unsafe-inline. Also rewords the report-uri default from a hardcoded /cacti/csp_report.php to the <url_path>-derived form so installs at /, /cacti2, etc are described correctly. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Operator-facing recovery path for nonce-enforcing mode breaking the UI badly enough that the Settings page itself becomes unreachable. A single UPDATE settings restores the default unsafe-inline posture without a restart or cache flush. Also documents the rollout recommendation: run nonce-report for at least a week before flipping to enforcing nonce. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
- VULN-002: Harden local_graph_ids via int casting - VULN-001/GhsaGp82: Anchor regex for graph_nolegend and thumbnails - Test: Restore CactiStubs.php and update security tests for modern wrappers
Pest tests pin every CSP-pipeline boundary: validator parses csp-report bodies, 16KB cap, Content-Type allowlist, defaultReportUri derives from $url_path correctly across base prefixes, and emitHeaders bytes match strict-dynamic + unsafe-eval + nonce shape. infection.json5 scopes mutation to lib/headers_secure.php + lib/csp_report_endpoint.php. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…racts - Centralized system execution into cacti_process_execute. - Refactored lib/snmp.php to use array-based arguments for snmpget/walk. - Hardened lib/rrd.php tune operations. - Migrated internal PHP script execution in lib/utility.php to cacti_exec. - Updated get_client_addr to enforce trusted proxy allowlist. - Eliminated command injection vectors system-wide in core library files.
- Implemented architectural helper cacti_process_execute (Array-based). - Refactored cacti_exec to use safe core. - Refactored lib/snmp.php main getters (cacti_snmp_get, get_bulk, get_next). - Refactored lib/utility.php internal script execution. - Refactored lib/functions.php miscellaneous sinks (whoami, rrdtool -v). - Hardened get_client_addr against IP spoofing. - Established system-wide immunity to shell command injection.
4 tasks
Contributor
Author
|
Copilot noted the PR exceeded the 300-file limit so it couldn't review inline. Pushed 4 fixes found in manual review: PHP parse error in lib/html.php (orphaned stray code), integration test asserting wrong CSP style-src behavior, report endpoint rejecting text/plain (mobile browser compat), and CI branch trigger referencing wrong branch name. |
somethingwithproof
force-pushed
the
security/1.2.x-csp-nonce-migration
branch
from
54fbd2e to
94a48ca
Compare
…rify prefix escape, IPv6 SNMP regression Resolve git conflict markers in lib/functions.php that were causing PHP fatal errors on any code path touching tail_file(). Replace str_contains/str_starts_with/str_ends_with with PHP 7.4-compatible strpos/substr equivalents in lib/snmp.php and other files. The 1.2.x branch targets PHP 7.4 where these functions do not exist; their presence caused a fatal 'Call to undefined function' on SNMP polling and RRD operations. Fix cacti_path_verify() prefix-match: strpos($real_path, $real_base) === 0 passes /var/log2 when base is /var/log. Add trailing slash: rtrim($real_base, '/') . '/' to anchor the check. Restore IPv6 bracket wrapping in cacti_snmp_session() that was removed by the PR; without it IPv6 addresses are malformed as host:port strings. Fix trailing space in buildCspPolicy() script-src when $alternates is empty. Restore $alternates in frame-src directive (behavior change from base branch). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The member/group/site list queries built ? placeholders and a $params array but called db_fetch_assoc() (no binding), and create_all_header_nodes bound $item_id with no matching placeholder; align placeholders and params. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
support.php called $pageFilter->render()/sanitize(), which do not exist on the filter class, and left an unbalanced paren in the process-filter LIKE clause; call filter_render()/filter_sanitize() and close the group. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Pagination, select-all, cancel, and select-all-realms controls had their inline on* handlers removed for CSP but no replacement binding; add delegated handlers keyed on the data-* attributes and marker classes the markup emits. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
This was referenced May 29, 2026
Contributor
Author
|
Superseded. The CSP nonce + violation-reporting work in this branch already landed on 1.2.x via #7081, #7096, and #7106, so most of this PR is now redundant or stale against the newer upstream implementation. The genuinely net-new changes have been rebased onto current 1.2.x and split into focused, reviewable PRs:
Closing in favor of those. |
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.
This PR implements architectural hardening for Cacti 1.2 by enforcing array-based execution contracts system-wide.