Files
cacti/tests/tools/check_hardening_patterns.php
Thomas VincentandTheWitness 7c5e4c102a feat(support): consolidate diagnostics, lockout CSRF, process-table hook, and safe-triage (#7514)
* feat(support): add redaction toggle and recent-log panel

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

* test: cover support safe triage

Unit-test the redaction helper, severity log tail and environment table, and add a Docker probe that the redacted report masks the RSA fingerprint and hides the database host and password.

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

* fix(support): escape rsa fingerprint and default unknown poller type

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

* ci: refresh security baselines

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

* fix(support): redact compressed IPv6 addresses

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

* ci: refresh security baselines

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

* ci: skip tests tree in hardening pattern guard

Test fixtures assign request superglobals to exercise product code; the guard only protects product code.

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

* fix(support): drop redundant isset flagged by phpstan

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

* feat(support): guard snmpget path before executing

Reject a null byte and require a regular executable file for path_snmpget
so a misconfigured path cannot throw a ValueError or match a directory.

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

* ci: refresh security baselines for snmpget path guard

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

* fix(support): redact FQDNs before the node name and stop truncating tool output

Masking the node name first could leave a domain suffix unmasked when
the name was a prefix of an FQDN, and strip_tags() on installed SNMP
output dropped legitimate angle-bracket content instead of just
masking it. Also tighten the redact toggle to a strict '1' match,
carry it through page-refresh URLs, and correct the diagnostics report
comment to describe what actually gets masked.

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

* fix(support): guard poller type lookup, log stats, and binary path checks

Fall back to an "Unknown" label when poller_type doesn't match a known
poller instead of an undefined-index warning, suppress warnings from
filesize()/fopen() if the log file disappears between the caller's
check and the open, and require is_file() plus a null-byte guard on
the required-binaries version check (matches the snmpget check next
to it).

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

* docs(tests): align triage probe comments with actual assertions

The header comments claimed the unredacted report exposes the DB host
and password, but the probe only ever asserts the RSA fingerprint;
DB credentials are checked for absence when present, not required to
appear unmasked. Correct the wording in both files.

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

* fix(support): require POST + CSRF token for maintenance lockout, compare-and-set

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

* fix(support): require a valid expected state for lockout compare-and-set

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

* test(security): refresh sink/hotspot baselines for upstream line shifts

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

* fix(support): clamp page offset, quote SQL literals, suppress lockout URL state

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

* test: cover support lockout csrf guard

Exercise the POST-only guard and compare-and-set branches, and add a Docker probe that a GET or tokenless POST cannot change the lockout state.

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

* test(support): guard lockout probe extraction and correct probe scope note

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

* ci: refresh security baselines

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

* ci: skip tests tree in hardening pattern guard

Test fixtures assign request superglobals to exercise product code; the guard only protects product code.

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

* fix(support): keep lockout e2e health poll alive under pipefail

Tolerate grep returning non-zero before the Health field appears so the
poll loop waits instead of exiting the script.

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

* test(support): add integration coverage for the lockout compare-and-set

The Unit test cannot reach the "currently locked" branch (forced reads
return the PHP_TESTING default) and the e2e probe deliberately never
locks the system, so the real read/write round trip against
cacti_lockout_status has no coverage. Add a fast integration test that
drives the lock, a stale-expected rejection, and the unlock against a
real database, skipping gracefully when none is reachable.

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

* test(support): match the page-clamp assertion whitespace-tolerantly

Whitespace-exact substr_count() breaks on reformatting; the regex enforces
the same guard without depending on exact spacing.

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

* fix(support): harden the lockout e2e probe's auth and cookie-jar handling

Assert a CSRF token was extracted and the session actually authenticated
before running the lockout checks, otherwise a failed login still passes.
Also handle tempnam() failure and clean up the cookie jar on every exit
path via a shutdown handler.

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

* feat(support): add diagnostics export and environment checks

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

* test: cover support diagnostics export

Unit-test the environment table and report redaction, and add a Docker probe that checks the report never leaks the database host, password or full RSA fingerprint.

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

* fix(support): guard null snmp version and copy-status styling

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

* ci: refresh security baselines

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

* fix(support): keep diagnostics textarea visible on copy failure and report real extension load state

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

* ci: refresh security baselines

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

* fix(support): verify PHP extensions in both cli and web contexts

show_tech_environment() only checked utility_php_verify_extensions()'s
web flag, which seeds CLI-only extensions like pcntl as satisfied
without checking whether they're actually loaded. Shell out to
cli_check.php for the real CLI state and combine it with the web
state the same way lib/installer.php does, so status reflects reality
in both directions.

Also guard the source-extraction helpers in SupportDiagnosticsTest and
the e2e probe against a moved/unreadable support.php instead of
letting strpos(false, ...)/eval() fail confusingly.

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

* fix(support): require POST + CSRF token for maintenance lockout, compare-and-set

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

* refactor(support): register process tables via support_process_tables hook

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

* fix(support): guard non-array hook result and empty process-table set

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

* fix(support): cast poller_interval to int in SQL and validate hook entry types

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

* test(security): refresh sink/hotspot baselines for upstream line shifts

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

* fix(support): reuse forced admin lookup and coerce expected state

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

* test: cover support process-table hook

Verify the hook drops malformed plugin definitions and keeps valid ones, and add a Docker probe that the hook returns well-formed tables against a real database.

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

* test(support): guard hook extraction and tolerate empty health probe

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

* ci: refresh security baselines

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

* fix(support): escape plugin-contributed process-table cells

Plugins can now supply the process-table SELECT fragments via the
support_process_tables hook, so escape the rendered cell values, and
install the e2e cleanup trap before backing up config.php.

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

* ci: refresh security baselines for support.php line shift

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

* fix(support): use form_selectable_ecell for process rows and cast pid in row id

Plugin-contributed process-table values were only escaped via manual
html_escape() calls, and the row id was built from an unsanitized pid;
switch to the file's existing form_selectable_ecell() helper and cast
pid to int before it goes into the id attribute.

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

* test(support): harden hook-call rewrite and narrow validation comment

Replace the exact-string str_replace() with a regex plus a match-count
assertion so the probe fails loudly instead of silently no-op'ing if
support_process_tables() formatting drifts, and correct the header
comment's claim that malformed definitions can't corrupt the UNION --
the validation only checks types, not SQL validity.

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

* test(support): make e2e hook extraction regex whitespace-tolerant

The extraction pattern required exact spacing around the function
signature; match flexible whitespace instead and fail loudly if
preg_match doesn't return exactly one match.

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

* test(support): add integration coverage for the process tables SQL path

Unit tests eval() support_process_tables() with the plugin hook stubbed
and never touch a database; the e2e Docker probe calls the same function
directly but never builds or executes the UNION show_cacti_processes()
runs. Add a fast, DB-only integration test that executes the real query
against a live schema, skipping gracefully when no database is reachable.

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

* fix(support): harden process table hook contracts

* docs: add changelog entry

* style: satisfy support hook fixer rules

* fix(support): reconcile escaping contract and diagnostics test after merge

tech_env_status() now escapes its $text argument (safe-triage's design,
tested by SupportSafeTriageTest); strip the now-redundant html_escape()
calls at its call sites in show_tech_environment() to avoid double
encoding. Update SupportDiagnosticsTest's source-extraction marker and
eval() inputs to match the merged, redact-toggle-aware report block from
safe-triage. Refresh the sink inventory baseline for the resulting line
shift.

* fix(support): escape the PHP uname value on both redact paths

Hostnames and kernel strings can carry markup, and support_redact() itself substitutes <host>, so redact first and escape the result either way.

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

* test(support): locate the diagnostics report block by regex, not literal

The $snmp_line assignment became a conditional and the literal start marker stopped matching, aborting the probe before it built a report.

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

* docs: append the process tables entry to the feature block

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-08-05 10:42:46 -04:00

181 lines
5.5 KiB
PHP

<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
| |
| This program is free software; you can redistribute it and/or |
| modify it under the terms of the GNU General Public License |
| as published by the Free Software Foundation; either version 2 |
| of the License, or (at your option) any later version. |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/
$options = getopt('', ['base:', 'head:']);
$base = $options['base'] ?? '';
$head = $options['head'] ?? 'HEAD';
$files = cli_file_args($argv);
/* Explicit file arguments get a whole-file scan for local use. CI runs
derive the file list from the diff and must only flag lines a PR added
or modified, so legacy patterns elsewhere in a touched file do not
block unrelated work. */
$changed_lines = null;
if ($files === []) {
$changed_lines = changed_php_lines($base, $head);
$files = array_keys($changed_lines);
}
$patterns = [
'raw-superglobal-request' => [
'pattern' => '/\$_(?:GET|POST|REQUEST)\s*\[/',
'message' => 'Use Cacti request helpers or typed filter objects instead of raw request superglobals.',
'allow' => [
// pre-boot code; the request helper layer is not loaded yet
'include/global.php',
'lib/PackageListFilter.php',
// tests/integration/AggregateConvertTemplateGuardIntegrationTest.php: simulates request
// state to exercise the real request-consuming function, not a production sanitization bypass
'tests/integration/AggregateConvertTemplateGuardIntegrationTest.php',
]
],
// 'manual-request-query-url' => [
// 'pattern' => '/[?&][A-Za-z0-9_-]+=\s*[\'"]?\s*\.\s*g(?:r|fr|nr)v\s*\(/',
// 'message' => 'Use a URL builder or typed page-state URL method instead of concatenating request values into query strings.'
// ],
'inline-js-php-string' => [
'pattern' => '/(?:var|let|const)\s+[A-Za-z0-9_$]+\s*=\s*[\'"][^\'"]*<\?php\s+print\s+/',
'message' => 'Use JSON encoding for PHP values injected into JavaScript.'
],
'raw-request-value-attribute' => [
'pattern' => '/value\s*=\s*[\'"][^\'"]*<\?php\s+print\s+g(?:r|fr|nr)v\s*\(/',
'message' => 'Use form rendering helpers for request-backed input values.'
],
'serialized-hidden-state' => [
'pattern' => '/(?:type\s*=\s*[\'"]hidden[\'"].*serialize\s*\(|html_hidden_input\s*\([^;]*serialize\s*\()/',
'message' => 'Do not round-trip serialized PHP state through hidden fields.'
],
];
$violations = [];
foreach ($files as $file) {
if (!is_file($file) || pathinfo($file, PATHINFO_EXTENSION) !== 'php') {
continue;
}
/* The guard protects product code. Test fixtures legitimately assign
request superglobals and embed inline patterns to exercise the code
under test, so the tests tree is out of scope. */
if (str_starts_with(preg_replace('#^\./#', '', $file), 'tests/')) {
continue;
}
$lines = file($file, FILE_IGNORE_NEW_LINES);
if ($lines === false) {
continue;
}
foreach ($lines as $index => $line) {
if ($changed_lines !== null && !isset($changed_lines[$file][$index + 1])) {
continue;
}
foreach ($patterns as $name => $rule) {
if (isset($rule['allow']) && in_array($file, $rule['allow'], true)) {
continue;
}
if (preg_match($rule['pattern'], $line) === 1) {
$violations[] = sprintf('%s:%d: %s: %s', $file, $index + 1, $name, $rule['message']);
}
}
}
}
if ($violations !== []) {
fwrite(STDERR, "Hardening pattern guard found unsafe changed-file patterns:\n");
fwrite(STDERR, implode("\n", $violations) . "\n");
exit(1);
}
exit(0);
function changed_php_lines(string $base, string $head) : array {
if ($base === '') {
$base = trim((string) shell_exec('git merge-base HEAD upstream/develop 2>/dev/null'));
}
if ($base === '') {
$base = trim((string) shell_exec('git merge-base HEAD origin/develop 2>/dev/null'));
}
if ($base === '') {
return [];
}
/* -U0 keeps hunk headers tight so only added/modified new-side lines
are recorded; --no-ext-diff and the explicit prefixes guard against
local diff drivers or diff.noprefix breaking the parse. */
$command = sprintf(
'git diff -U0 --no-ext-diff --src-prefix=a/ --dst-prefix=b/ --diff-filter=ACMR %s...%s -- %s 2>/dev/null',
escapeshellarg($base),
escapeshellarg($head),
escapeshellarg('*.php')
);
$output = shell_exec($command);
if (!is_string($output) || trim($output) === '') {
return [];
}
$changed = [];
$file = '';
foreach (preg_split('/\R/', $output) as $line) {
if (preg_match('/^\+\+\+ b\/(.+)$/', $line, $matches) === 1) {
$file = $matches[1];
} elseif ($file !== '' && preg_match('/^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/', $line, $matches) === 1) {
$start = (int) $matches[1];
$count = isset($matches[2]) ? (int) $matches[2] : 1;
for ($i = 0; $i < $count; $i++) {
$changed[$file][$start + $i] = true;
}
}
}
return $changed;
}
function cli_file_args(array $argv) : array {
$files = [];
for ($i = 1; $i < count($argv); $i++) {
$arg = $argv[$i];
if ($arg === '--base' || $arg === '--head') {
$i++;
continue;
}
if (str_starts_with($arg, '--base=') || str_starts_with($arg, '--head=')) {
continue;
}
if (str_starts_with($arg, '--')) {
continue;
}
$files[] = $arg;
}
return $files;
}