diff --git a/.gitignore b/.gitignore index 45bf57a9d..83b7f862b 100644 --- a/.gitignore +++ b/.gitignore @@ -112,3 +112,16 @@ docker-compose.override.yml # Ignore Claude and Worktrees .worktrees/** .claude/** + +# Security research artifacts (never commit) +/reproduce_ghsa_*.php +/verify_task1.php +/.gemini_security/ +# Test/build artifacts +/tests/composer.lock +/tests/e2e/playwright-report/ +/tests/e2e/test-results/ +/tests/.phpunit.result.cache +# Local autopilot/session data +/.omc/ +/docs/superpowers/ diff --git a/composer.json b/composer.json index c8439f455..ea7f30f3d 100644 --- a/composer.json +++ b/composer.json @@ -53,17 +53,20 @@ "php-mqtt/client": "^2.2", "phpmailer/phpmailer": "^7.0.2", "phpseclib/phpseclib": "^3.0", + "psr/log": "^1.1", "slim/slim": "^4.14", "stevenmaguire/oauth2-keycloak": "^6.1.0", - "stevenmaguire/oauth2-microsoft": "^2.2" + "stevenmaguire/oauth2-microsoft": "^2.2", + "symfony/filesystem": "^6.0", + "symfony/http-foundation": "^6.0" }, "require-dev": { + "composer/xdebug-handler": "^3.0", "friendsofphp/php-cs-fixer": "^3.86", "overtrue/phplint": "^9.6", - "phpstan/phpstan": "^2.1.21", "pestphp/pest": "^2", "pestphp/pest-plugin-drift": "^2.0", - "composer/xdebug-handler": "^3.0", + "phpstan/phpstan": "^2.1.21", "rector/rector": "^2.1.2" }, "scripts": { @@ -92,7 +95,7 @@ "sort-packages": true, "vendor-dir": "./include/vendor", "audit": { - "block-insecure": false + "block-insecure": false }, "allow-plugins": { "pestphp/pest-plugin": true diff --git a/docs/security/handoff-tests.md b/docs/security/handoff-tests.md new file mode 100644 index 000000000..ce82949f3 --- /dev/null +++ b/docs/security/handoff-tests.md @@ -0,0 +1,79 @@ +# Hand-off tests + +Hand-off tests live in `tests/HandOff/` and complement the in-isolation Pest +suites under `tests/Unit/`. Where a unit test exercises a function with +controlled inputs, a hand-off test exercises the boundary that function +crosses: a real file on disk, a real ZIP archive, a real `cacti_log()` call, +a real shell-quoted command line. The goal is to catch regressions that an +isolated unit test cannot see because the boundary itself is the bug surface. + +## Pattern + +Each feature PR adds exactly one file: + + tests/HandOff/HandOffTest.php + +The file requires the shared helpers and asserts on a single boundary: + +```php +require_once __DIR__ . '/HandOffHelpers.php'; + +cacti_handoff_stub_cacti_log(); + +test('import rejects zip with absolute path entry', function () { + cacti_handoff_clear_log_buffer(); + + $zip = cacti_handoff_make_zip(array( + '/etc/passwd' => 'root:x:0:0:/root:/bin/bash', + )); + + // call into the importer under test, then assert + expect(myImporter($zip))->toBeFalse(); + expect(cacti_handoff_get_log_buffer())->not->toBeEmpty(); +}); +``` + +`HandOffHelpers.php` provides: + +- `cacti_handoff_stub_cacti_log()` registers a shim that records every + `cacti_log()` call. +- `cacti_handoff_get_log_buffer()` / `cacti_handoff_clear_log_buffer()` + read and reset the captured lines. +- `cacti_handoff_temp_file($contents, $extension)` writes a unique temp file + with auto-cleanup on shutdown. +- `cacti_handoff_make_zip($entries)` builds a minimal valid ZIP from a + `path => content` map and returns its temp path. + +## Running + + composer run-script test -- --testsuite=HandOff + +The standard `composer run-script test` invocation continues to run every +suite. The `--testsuite` filter is for fast local iteration on a single +feature. + +## Mutation testing + +Infection is wired up via `infection.json5` at the repo root. The baseline +config scopes mutation to `lib/` with `minMsi: 70` / `minCoveredMsi: 80`. +Feature PRs that want to gate their files specifically should either: + +1. Copy `infection.json5` into the feature branch and edit the + `source.directories` / `filter` field to point at the changed files, or +2. Pass `--filter='lib/yourfile.php'` on the Infection CLI. + +Mutation runs are not a CI gate by default; they are slow and noisy on a +codebase Cacti's size. Operators run them locally before merging large +changes, and feature PRs may opt their files in via a per-PR job if the +maintainer requests it. + +## First wave of feature PRs + +These PRs were the source material for the helper extraction and are the +first set expected to rebase onto this infrastructure: + +- #7073, #7074, #7075, #7077, #6989, #7032, #7063, #7066 + +Once those merge, new hand-off tests should follow the same pattern: one +file under `tests/HandOff/`, helpers from `HandOffHelpers.php`, no private +copies of `cacti_log` stubs or ZIP fixture builders. diff --git a/infection.json5 b/infection.json5 new file mode 100644 index 000000000..3898d41ae --- /dev/null +++ b/infection.json5 @@ -0,0 +1,26 @@ +// Baseline mutation testing config for Cacti. +// +// Feature PRs that want to gate their own files on mutation score should +// override the `source.directories` / `filter` and `testFrameworkOptions` +// fields to scope mutation to the files they touched. Either copy this file +// into the feature branch and edit it, or pass `--filter='lib/yourfile.php'` +// on the Infection CLI. The thresholds below (minMsi / minCoveredMsi) are +// the project floor; feature PRs may raise them but should not lower them. +{ + $schema: "include/vendor/infection/infection/resources/schema.json", + source: { + directories: ["lib"], + excludes: ["vendor", "tests"] + }, + mutators: { + "@default": true + }, + testFramework: "pest", + minMsi: 70, + minCoveredMsi: 80, + timeout: 30, + tmpDir: "build/infection", + logs: { + text: "build/infection/infection.log" + } +} diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f90ce8d68..abe23b7a8 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -41,3 +41,69 @@ parameters: identifier: isset.offset count: 1 path: lib/functions.php + + - + message: '#^Undefined variable\: \$graph_template_item_id$#' + identifier: variable.undefined + count: 1 + path: aggregate_graphs.php + + - + message: '#^Variable \$graph_template_item_id in empty\(\) is never defined\.$#' + identifier: empty.variable + count: 1 + path: aggregate_graphs.php + + - + message: '#^Undefined variable\: \$color_template_item_id$#' + identifier: variable.undefined + count: 1 + path: color_templates.php + + - + message: '#^Variable \$color_template_item_id in empty\(\) is never defined\.$#' + identifier: empty.variable + count: 1 + path: color_templates.php + + - + message: '#^Undefined variable\: \$graph_template_item_id$#' + identifier: variable.undefined + count: 1 + path: graph_templates.php + + - + message: '#^Variable \$graph_template_item_id in empty\(\) is never defined\.$#' + identifier: empty.variable + count: 1 + path: graph_templates.php + + - + message: '#^Undefined variable\: \$graph_template_item_id$#' + identifier: variable.undefined + count: 1 + path: graphs.php + + - + message: '#^Variable \$graph_template_item_id in empty\(\) is never defined\.$#' + identifier: empty.variable + count: 1 + path: graphs.php + + - + message: '#^Offset ''image'' on array\{title\: string, image\: string, id\: ''tree'', url\: ''graph_view\.php…'', selected\?\: true\} in isset\(\) always exists and is not nullable\.$#' + identifier: isset.offset + count: 1 + path: lib/html.php + + - + message: '#^Offset ''image'' on array\{title\: string, image\: string, id\: ''list'', url\: ''graph_view\.php…'', selected\?\: true\} in isset\(\) always exists and is not nullable\.$#' + identifier: isset.offset + count: 1 + path: lib/html.php + + - + message: '#^Offset ''image'' on array\{title\: string, image\: string, id\: ''preview'', url\: ''graph_view\.php…'', selected\?\: true\} in isset\(\) always exists and is not nullable\.$#' + identifier: isset.offset + count: 1 + path: lib/html.php diff --git a/phpunit.xml b/phpunit.xml index 768a35492..00857c4ab 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -6,5 +6,11 @@ ./tests/Unit + + ./tests/integration + + + ./tests/HandOff + diff --git a/tests/HandOff/.gitkeep b/tests/HandOff/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/tests/HandOff/CactiDispatchHandOffTest.php b/tests/HandOff/CactiDispatchHandOffTest.php new file mode 100644 index 000000000..13005bc90 --- /dev/null +++ b/tests/HandOff/CactiDispatchHandOffTest.php @@ -0,0 +1,276 @@ + action lookup -> + * method assertion -> permission flag -> handler invocation. The + * source-scan suite in tests/Unit/CactiDispatchTest.php pins the literal + * shape of each guard; the tests here pin the runtime contract so a + * refactor that keeps the source markers but swaps the semantics still + * fails. */ + +if (!file_exists(__DIR__ . '/../../lib/cacti_dispatch.php')) { + test('cacti_dispatch hand-off: feature not present on this branch', function () {}) + ->skip('lib/cacti_dispatch.php absent — feature PR #7063 not merged into develop yet'); + return; +} + +/* --------------------------------------------------------------------- */ +/* Stubs for the Cacti runtime functions cacti_dispatch() depends on. */ +/* They are declared once and shared across every test in this file via */ +/* the $GLOBALS['cdho_*'] mailbox so each test can read what was logged, */ +/* whether the AJAX denial helper fired, and which handlers ran. */ +/* --------------------------------------------------------------------- */ + +if (!function_exists('get_nfilter_request_var')) { + function get_nfilter_request_var($name, $default = '') { + if (!isset($_REQUEST[$name])) { + return $default; + } + + return $_REQUEST[$name]; + } +} + +if (!function_exists('cacti_log')) { + function cacti_log($message, $output = false, $environ = 'CMDPHP', $level = -1) { + $GLOBALS['cdho_logs'][] = ['message' => $message, 'env' => $environ]; + } +} + +if (!function_exists('cacti_strtoupper')) { + function cacti_strtoupper($s) { + return strtoupper((string) $s); + } +} + +if (!function_exists('cacti_strtolower')) { + function cacti_strtolower($s) { + return strtolower((string) $s); + } +} + +if (!function_exists('is_realm_allowed')) { + function is_realm_allowed($realm) { + return !empty($GLOBALS['cdho_allowed_realms'][$realm]); + } +} + +if (!function_exists('get_client_addr')) { + function get_client_addr() { + return '127.0.0.1'; + } +} + +if (!function_exists('raise_ajax_permission_denied')) { + function raise_ajax_permission_denied() { + $GLOBALS['cdho_ajax_denied'] = true; + } +} + +require_once __DIR__ . '/../../lib/cacti_dispatch.php'; + +beforeEach(function () { + $_REQUEST = []; + $_SERVER = []; + $GLOBALS['cdho_logs'] = []; + $GLOBALS['cdho_handler_calls'] = []; + $GLOBALS['cdho_allowed_realms'] = []; + $GLOBALS['cdho_ajax_denied'] = false; + http_response_code(200); +}); + +/* Helper that records the action name a handler was invoked with. Used + * to prove the dispatch table actually reached (or did not reach) a + * specific entry rather than relying on global side effects. */ +function cdho_make_handler(string $tag): callable { + return function () use ($tag) { + $GLOBALS['cdho_handler_calls'][] = $tag; + }; +} + +/* --------------------------------------------------------------------- */ +/* $action request var -> action lookup */ +/* --------------------------------------------------------------------- */ + +test('string action resolves to its handler entry', function () { + $_REQUEST['action'] = 'save'; + $_SERVER['REQUEST_METHOD'] = 'GET'; + + cacti_dispatch([ + 'save' => ['callback' => cdho_make_handler('save')], + 'edit' => ['callback' => cdho_make_handler('edit')], + ], 'edit'); + + expect($GLOBALS['cdho_handler_calls'])->toBe(['save']); +}); + +test('array action input is normalized to the default action', function () { + $_REQUEST['action'] = ['x']; + $_SERVER['REQUEST_METHOD'] = 'GET'; + + cacti_dispatch([ + 'edit' => ['callback' => cdho_make_handler('edit')], + ], 'edit'); + + expect($GLOBALS['cdho_handler_calls'])->toBe(['edit']); +}); + +test('action with shell metacharacters is rejected before table lookup', function () { + /* The hostile key is also present in the table so a missed + * sanitisation step would invoke its handler. Rejection must + * happen before the isset() lookup. */ + $_REQUEST['action'] = 'save;rm -rf /'; + $_SERVER['REQUEST_METHOD'] = 'GET'; + + cacti_dispatch([ + 'save' => ['callback' => cdho_make_handler('save')], + 'save;rm -rf /' => ['callback' => cdho_make_handler('hostile')], + ], ''); + + expect($GLOBALS['cdho_handler_calls'])->toBe([]); + expect(http_response_code())->toBe(403); + + $messages = array_column($GLOBALS['cdho_logs'], 'message'); + expect($messages)->toContain('WARNING: cacti_dispatch: unknown action "" from 127.0.0.1'); +}); + +/* --------------------------------------------------------------------- */ +/* $_SERVER['REQUEST_METHOD'] -> method assertion */ +/* --------------------------------------------------------------------- */ + +test('GET request against a POST entry is rejected with method-mismatch log', function () { + $_REQUEST['action'] = 'save'; + $_SERVER['REQUEST_METHOD'] = 'GET'; + + cacti_dispatch([ + 'save' => [ + 'callback' => cdho_make_handler('save'), + 'method' => 'POST', + ], + ], ''); + + expect($GLOBALS['cdho_handler_calls'])->toBe([]); + + $messages = array_column($GLOBALS['cdho_logs'], 'message'); + expect($messages)->toContain('WARNING: cacti_dispatch: method mismatch for action "save" (expected POST, got GET)'); +}); + +test('absent REQUEST_METHOD with ANY entry still dispatches', function () { + $_REQUEST['action'] = 'save'; + unset($_SERVER['REQUEST_METHOD']); + + cacti_dispatch([ + 'save' => [ + 'callback' => cdho_make_handler('save'), + 'method' => 'ANY', + ], + ], ''); + + expect($GLOBALS['cdho_handler_calls'])->toBe(['save']); +}); + +/* --------------------------------------------------------------------- */ +/* permission flag -> handler invocation */ +/* --------------------------------------------------------------------- */ + +test('object_acl returning false suppresses handler and runs the deny path', function () { + $_REQUEST['action'] = 'save'; + $_SERVER['REQUEST_METHOD'] = 'GET'; + + cacti_dispatch([ + 'save' => [ + 'callback' => cdho_make_handler('save'), + 'object_acl' => function () { return false; }, + ], + ], ''); + + expect($GLOBALS['cdho_handler_calls'])->toBe([]); + expect(http_response_code())->toBe(403); +}); + +test('object_acl returning true invokes the handler exactly once', function () { + $_REQUEST['action'] = 'save'; + $_SERVER['REQUEST_METHOD'] = 'GET'; + $seen = []; + + cacti_dispatch([ + 'save' => [ + 'callback' => function () use (&$seen) { $seen[] = 'save'; }, + 'object_acl' => function () { return true; }, + ], + ], ''); + + expect($seen)->toBe(['save']); +}); + +/* --------------------------------------------------------------------- */ +/* object_acl -> fail-closed */ +/* --------------------------------------------------------------------- */ + +test('non-callable object_acl logs ERROR and denies instead of silently allowing', function () { + $_REQUEST['action'] = 'save'; + $_SERVER['REQUEST_METHOD'] = 'GET'; + + cacti_dispatch([ + 'save' => [ + 'callback' => cdho_make_handler('save'), + 'object_acl' => 'not_a_real_function_anywhere', + ], + ], ''); + + expect($GLOBALS['cdho_handler_calls'])->toBe([]); + expect(http_response_code())->toBe(403); + + $errors = array_filter( + $GLOBALS['cdho_logs'], + fn ($entry) => str_starts_with($entry['message'], 'ERROR: cacti_dispatch: object_acl') + ); + expect($errors)->not->toBeEmpty(); +}); + +/* --------------------------------------------------------------------- */ +/* AJAX vs non-AJAX denial */ +/* --------------------------------------------------------------------- */ + +test('AJAX denial calls raise_ajax_permission_denied', function () { + $_REQUEST['action'] = 'save'; + $_SERVER['REQUEST_METHOD'] = 'GET'; + $_SERVER['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest'; + + cacti_dispatch([ + 'save' => [ + 'callback' => cdho_make_handler('save'), + 'object_acl' => function () { return false; }, + ], + ], ''); + + expect($GLOBALS['cdho_ajax_denied'])->toBeTrue(); +}); + +test('non-AJAX denial sets an explicit 403 instead of falling through to 200', function () { + $_REQUEST['action'] = 'save'; + $_SERVER['REQUEST_METHOD'] = 'GET'; + + cacti_dispatch([ + 'save' => [ + 'callback' => cdho_make_handler('save'), + 'object_acl' => function () { return false; }, + ], + ], ''); + + expect($GLOBALS['cdho_ajax_denied'])->toBeFalse(); + expect(http_response_code())->toBe(403); +}); diff --git a/tests/HandOff/CactiMimeHandOffTest.php b/tests/HandOff/CactiMimeHandOffTest.php new file mode 100644 index 000000000..a5f8d5cac --- /dev/null +++ b/tests/HandOff/CactiMimeHandOffTest.php @@ -0,0 +1,191 @@ +skip('lib/CactiMime.php absent — feature PR #7074 not merged into develop yet'); + return; +} + +require_once __DIR__ . '/../../include/vendor/autoload.php'; +require_once __DIR__ . '/../../lib/CactiMime.php'; +require_once __DIR__ . '/../Unit/fixtures/mime/build_fixtures.php'; + +// Stub cacti_log so the libmagic-missing branch in CactiMime can run +// without pulling in lib/functions.php and its database dependencies. +if (!function_exists('cacti_log')) { + function cacti_log(mixed $string, bool $output = false, string $environ = 'CMDPHP', mixed $level = '') : bool { + return true; + } +} + +/** + * Hand-off contracts between the upload form (package_import.php) and + * CactiMime. Unit tests cover the class in isolation; these tests pin the + * boundary the production caller actually relies on: + * + * - $_FILES['import_file']['tmp_name'] is detected by content, not by + * the user-supplied filename. + * - $_FILES['import_file']['type'] (browser-supplied) never reaches the + * allowlist; the detected content MIME does. + * - The allowlist returned by packageImportMimes() drives the caller's + * accept/reject branch (the same branch that issues raise_message + + * header('Location: ...') in package_import.php form_save()). + * - When libmagic is absent, validate(strict=true) returns false and + * that boolean propagates to the caller's reject path. + */ + +beforeAll(function () { + cacti_mime_build_fixtures(); +}); + +test('detect reads tmp_name content bytes, not the user-supplied filename', function () { + // Simulate the exact upload shape package_import.php receives: a real ZIP + // arrived in tmp_name but the browser's file-picker handed it a .png name. + // PHP rewrites tmp_name to a server-controlled path, so the spoofed + // extension only lives on $_FILES['import_file']['name'] -- detect() + // must ignore it and read the magic bytes from tmp_name. + $fx = cacti_mime_build_fixtures(); + + $tmp_name = sys_get_temp_dir() . '/cacti-mime-handoff-' . getmypid() . '-phpUPLOAD'; + copy($fx['zip'], $tmp_name); + + $files_entry = [ + 'name' => 'innocent.png', + 'type' => 'image/png', + 'tmp_name' => $tmp_name, + 'error' => 0, + 'size' => filesize($tmp_name), + ]; + + expect(CactiMime::detect($files_entry['tmp_name']))->toBe('application/zip'); + + unlink($tmp_name); +}); + +test('browser-supplied $_FILES type is ignored; detect uses content', function () { + // The browser claims application/json. The bytes are XML. CactiMime + // must report the XML content type, never the browser's claim. + $fx = cacti_mime_build_fixtures(); + + $tmp_name = sys_get_temp_dir() . '/cacti-mime-handoff-' . getmypid() . '-phpBROWSER'; + copy($fx['xml'], $tmp_name); + + $files_entry = [ + 'name' => 'data.json', + 'type' => 'application/json', + 'tmp_name' => $tmp_name, + 'error' => 0, + 'size' => filesize($tmp_name), + ]; + + $detected = CactiMime::detect($files_entry['tmp_name']); + + expect($detected)->not->toBe($files_entry['type']); + expect($detected)->toBeIn(['application/xml', 'text/xml', 'application/x-xml']); + + unlink($tmp_name); +}); + +test('allowlist drives the caller accept branch for a real ZIP upload', function () { + // Mirrors the form_save() branch: + // if (!CactiMime::validate($xmlfile, CactiMime::packageImportMimes())) { reject } + // A real ZIP must traverse the accept side. + $fx = cacti_mime_build_fixtures(); + + $tmp_name = sys_get_temp_dir() . '/cacti-mime-handoff-' . getmypid() . '-phpACCEPT'; + copy($fx['zip'], $tmp_name); + + $files_entry = [ + 'name' => 'package.zip', + 'type' => 'application/zip', + 'tmp_name' => $tmp_name, + 'error' => 0, + 'size' => filesize($tmp_name), + ]; + + expect(CactiMime::validate($files_entry['tmp_name'], CactiMime::packageImportMimes()))->toBeTrue(); + + unlink($tmp_name); +}); + +test('allowlist drives the caller reject branch for a PHP payload renamed .zip', function () { + // Same form_save() branch, but with the obvious attack: a PHP file + // named import.zip. The reject side is what triggers raise_message + + // header('Location: package_import.php') in production. + $tmp_name = sys_get_temp_dir() . '/cacti-mime-handoff-' . getmypid() . '-phpREJECT'; + file_put_contents($tmp_name, "\n"); + + $files_entry = [ + 'name' => 'import.zip', + 'type' => 'application/zip', + 'tmp_name' => $tmp_name, + 'error' => 0, + 'size' => filesize($tmp_name), + ]; + + $accepted = CactiMime::validate($files_entry['tmp_name'], CactiMime::packageImportMimes()); + + expect($accepted)->toBeFalse(); + + // Model the caller's branch: validate()===false flows into a + // raise_message + header('Location: ...') reject path. Pin that the + // boolean is the trigger. + $rejected_branch_taken = ($accepted === false); + expect($rejected_branch_taken)->toBeTrue(); + + unlink($tmp_name); +}); + +test('libmagic absence drives validate(strict=true) to false at the caller boundary', function () { + // CactiMimeTest already verifies the strict-mode boolean inside the + // class via a subprocess. This hand-off variant asserts the same + // boolean reaches the caller's reject branch -- the one that emits + // the "import_mime_unavailable" raise_message and redirects the user. + $fx = cacti_mime_build_fixtures(); + $scriptPath = tempnam(sys_get_temp_dir(), 'cacti-mime-handoff-strict-'); + $libPath = realpath(__DIR__ . '/../../lib/CactiMime.php'); + + // The subprocess models the caller's boundary explicitly: it + // invokes validate() and prints the exact branch token the caller + // would take (ACCEPT or REJECT). When finfo is disabled, strict + // mode must yield REJECT. + $script = "&1'; + + $output = []; + $status = 0; + exec($command, $output, $status); + + expect($status)->toBe(0); + expect(trim(implode("\n", $output)))->toBe('REJECT'); + } finally { + if (is_file($scriptPath)) { + unlink($scriptPath); + } + } +}); diff --git a/tests/HandOff/CactiProcessHandOffTest.php b/tests/HandOff/CactiProcessHandOffTest.php new file mode 100644 index 000000000..d96015d7d --- /dev/null +++ b/tests/HandOff/CactiProcessHandOffTest.php @@ -0,0 +1,132 @@ +skip('lib/CactiProcess.php absent — feature PR #7073 not merged into develop yet'); + return; +} + +require_once dirname(__DIR__) . '/Helpers/CactiStubs.php'; +require_once dirname(__DIR__, 2) . '/lib/CactiProcess.php'; + +test('argv array reaches child as literal args without shell expansion', function () { + /* + * /usr/bin/env with no args prints the env, so we instead use it as a + * "echo every argv element on its own line" by piping argv through + * printf inside a controlled shell-free invocation. We pick printf + * directly so there is no /bin/sh layer to blame for substitution. + */ + $payload = '$(uname);`whoami`;${HOME};*;|'; + $result = CactiProcess::run(['printf', '%s\n', $payload]); + + expect($result->exitCode())->toBe(0) + ->and($result->stdout())->toContain($payload); +}); + +test('env allowlist excludes parent secret from child env', function () { + $marker = 'CACTIPROC_HANDOFF_SECRET_' . bin2hex(random_bytes(6)); + putenv($marker . '=top-secret-value'); + + try { + $result = CactiProcess::run(['/usr/bin/env']); + + expect($result->exitCode())->toBe(0) + ->and($result->stdout())->not->toContain($marker) + ->and($result->stdout())->not->toContain('top-secret-value'); + } finally { + putenv($marker); + } +}); + +test('env allowlist forwards parent variable verbatim when opted in', function () { + $marker = 'CACTIPROC_HANDOFF_OPTIN_' . bin2hex(random_bytes(6)); + $value = 'verbatim value with spaces and = signs'; + putenv($marker . '=' . $value); + + try { + $result = CactiProcess::run(['/usr/bin/env'], ['env' => [$marker]]); + + expect($result->exitCode())->toBe(0) + ->and($result->stdout())->toContain($marker . '=' . $value); + } finally { + putenv($marker); + } +}); + +test('child stdout bytes are preserved byte-for-byte including multibyte UTF-8', function () { + /* + * Cyrillic, CJK, and an emoji in one payload. printf %s leaves the bytes + * intact, so any corruption (re-encoding, locale folding, stream + * filters) shows up as a string mismatch. + */ + $payload = "\xD0\xBF\xD1\x80\xD0\xB8\xD0\xB2\xD0\xB5\xD1\x82-\xE6\x97\xA5\xE6\x9C\xAC\xE8\xAA\x9E-\xF0\x9F\x9A\x80"; + $result = CactiProcess::run(['printf', '%s', $payload]); + + expect($result->exitCode())->toBe(0) + ->and($result->stdout())->toBe($payload); +}); + +test('child exit code surfaces through CactiProcessResult exitCode', function () { + $result = CactiProcess::run( + ['/bin/sh', '-c', 'exit 7'], + ['expected_exit_codes' => [0, 7]] + ); + + expect($result->exitCode())->toBe(7); +}); + +test('outputLines matches the array shape exec() callers expect', function () { + /* + * exec($cmd, $output) populates $output with one element per line and + * does NOT include the trailing empty string from the final newline. + * outputLines() must match that shape so the migration is mechanical. + */ + $cmd = '/bin/sh -c \'printf "alpha\nbeta\ngamma\n"\''; + $exec_out = []; + $exec_exit = 0; + exec($cmd, $exec_out, $exec_exit); + + $result = CactiProcess::run(['/bin/sh', '-c', 'printf "alpha\nbeta\ngamma\n"']); + + expect($result->exitCode())->toBe($exec_exit) + ->and($result->outputLines())->toBe($exec_out); +}); + +test('timeout throws CactiProcessException carrying ProcessTimedOutException as previous', function () { + try { + CactiProcess::run(['sleep', '5'], ['timeout' => 1]); + $this->fail('expected CactiProcessException to be thrown on timeout'); + } catch (CactiProcessException $e) { + expect($e->getPrevious()) + ->toBeInstanceOf(\Symfony\Component\Process\Exception\ProcessTimedOutException::class); + } +}); diff --git a/tests/HandOff/CactiSettingsHandOffTest.php b/tests/HandOff/CactiSettingsHandOffTest.php new file mode 100644 index 000000000..358a8e04b --- /dev/null +++ b/tests/HandOff/CactiSettingsHandOffTest.php @@ -0,0 +1,282 @@ +skip('lib/CactiSettings.php absent — feature PR #7077 not merged into develop yet'); + return; +} + +require_once dirname(__DIR__) . '/Helpers/CactiStubs.php'; +require_once dirname(__DIR__, 2) . '/include/global.php'; +require_once dirname(__DIR__, 2) . '/lib/CactiSettings.php'; + +use Symfony\Component\Validator\Constraints as Assert; + +// ===================================================================== +// Hand-off tests for the Symfony Validator settings pilot. +// +// These tests pin the contract between the four moving parts that +// settings.php::save_settings() chains together: +// +// $_POST -> gnrv()-built snapshot -> CactiSettings::validate -> raise_message +// +// Each section verifies one boundary so a regression in any single +// stage is caught without needing a full HTTP round-trip. +// ===================================================================== + +beforeEach(function () { + // gnrv() reads $_CACTI_REQUEST first, then $_REQUEST. Reset both so + // state from a prior test cannot leak into the snapshot path. + $GLOBALS['_CACTI_REQUEST'] = []; + $_REQUEST = []; + $_POST = []; +}); + +// --------------------------------------------------------------------- +// Stage 1: $_POST -> gnrv()-built snapshot -> validate +// --------------------------------------------------------------------- + +test('whitespace-padded posted value round-trips through gnrv into validate', function () { + // gnrv() does not trim; the snapshot carries the raw string. The + // hand-off contract is that whatever validate() sees is byte-identical + // to what arrived in $_POST, so a Length(min:1) constraint sees the + // padded value exactly as posted. + $_POST['path_rrdtool'] = ' /usr/bin/rrdtool '; + $_REQUEST['path_rrdtool'] = $_POST['path_rrdtool']; + + $snapshot = ['path_rrdtool' => gnrv('path_rrdtool')]; + + expect($snapshot['path_rrdtool'])->toBe(' /usr/bin/rrdtool '); + + $definitions = [ + 'path_rrdtool' => [ + 'method' => 'textbox', + 'constraints' => [fn () => new Assert\Length(min: 1, max: 255)], + ], + ]; + + expect(CactiSettings::validate($snapshot, $definitions))->toBe([]); +}); + +test('array-shaped posted value reaches validate as an array via gnrv', function () { + // A crafted request like snmp_timeout[]=foo lands in $_REQUEST as an + // array. gnrv() returns it unchanged. The validator then sees an array + // where it expects a scalar; Range(min:1,max:600000) rejects it. The + // pin here is "the snapshot path does not silently coerce arrays to + // strings", which would otherwise mask injection attempts. + $_REQUEST['snmp_timeout'] = ['foo']; + + $snapshot = ['snmp_timeout' => gnrv('snmp_timeout')]; + + expect($snapshot['snmp_timeout'])->toBe(['foo']); + + $definitions = [ + 'snmp_timeout' => [ + 'method' => 'textbox', + 'constraints' => [ + fn () => new Assert\Regex(pattern: '/^\d+$/'), + fn () => new Assert\Range(min: 1, max: 600000), + ], + ], + ]; + + $result = CactiSettings::validate($snapshot, $definitions); + + expect($result)->toHaveKey('snmp_timeout') + ->and($result['snmp_timeout'])->toBeString(); +}); + +// --------------------------------------------------------------------- +// Stage 2: closure-form constraints -> validator instances +// --------------------------------------------------------------------- + +test('closure constraint defers instantiation until validate() runs', function () { + // global_settings.php loads before include/vendor/autoload.php in the + // real request flow. Constraint instances inside the array literal + // would fatal at file-load. The closure form lets the file parse + // regardless; the Assert\* class is only resolved when validate() + // invokes the closure. This counter proves the closure is NOT called + // at definition-build time and IS called exactly once per validate(). + $invocations = 0; + + $definitions = [ + 'snmp_port' => [ + 'method' => 'textbox', + 'constraints' => [ + function () use (&$invocations) { + $invocations++; + return new Assert\Range(min: 1, max: 65535); + }, + ], + ], + ]; + + // Building the definitions array did not call the closure. + expect($invocations)->toBe(0); + + $result = CactiSettings::validate(['snmp_port' => '161'], $definitions); + + expect($invocations)->toBe(1) + ->and($result)->toBe([]); + + // A second validate() call resolves the closure again. Constraints + // are not memoized inside CactiSettings, which keeps the contract + // simple: each validate() pass is independent. + CactiSettings::validate(['snmp_port' => '161'], $definitions); + + expect($invocations)->toBe(2); +}); + +test('closure returning Range produces violation on out-of-range posted value', function () { + $definitions = [ + 'snmp_port' => [ + 'method' => 'textbox', + 'constraints' => [fn () => new Assert\Range(min: 1, max: 65535)], + ], + ]; + + $result = CactiSettings::validate(['snmp_port' => '70000'], $definitions); + + expect($result)->toHaveKey('snmp_port') + ->and($result['snmp_port'])->toBeString() + ->and($result['snmp_port'])->not->toBe(''); +}); + +// --------------------------------------------------------------------- +// Stage 3: ConstraintViolationList -> first message -> raise_message key +// --------------------------------------------------------------------- + +test('two violations on one setting collapse to the first message only', function () { + // raise_message() renders one line per setting. validate() must + // pick exactly one message per setting to match that UX. The first + // violation in document order wins; subsequent violations are + // dropped so the user sees the most-immediate failure rather than + // a wall of cascading messages. + $definitions = [ + 'snmp_timeout' => [ + 'method' => 'textbox', + 'constraints' => [ + fn () => new Assert\Regex(pattern: '/^\d+$/', message: 'must be numeric'), + fn () => new Assert\Range(min: 1, max: 600000, notInRangeMessage: 'must be in range'), + ], + ], + ]; + + $result = CactiSettings::validate(['snmp_timeout' => 'abc'], $definitions); + + expect($result)->toHaveKey('snmp_timeout') + ->and($result['snmp_timeout'])->toBe('must be numeric') + ->and($result)->toHaveCount(1); +}); + +test('violation message wrapped through __() round-trips for i18n', function () { + // Constraint messages are passed through __() at definition-build + // time so the active locale's translation is what reaches + // raise_message. With no translation registered, __() returns the + // English source text unchanged; the test pins that the wrap is + // transparent and does not mangle the message. + $message = __('must be a positive integer (milliseconds).'); + $definitions = [ + 'snmp_timeout' => [ + 'method' => 'textbox', + 'constraints' => [ + fn () => new Assert\Regex(pattern: '/^\d+$/', message: __('must be a positive integer (milliseconds).')), + ], + ], + ]; + + $result = CactiSettings::validate(['snmp_timeout' => 'abc'], $definitions); + + expect($result)->toHaveKey('snmp_timeout') + ->and($result['snmp_timeout'])->toBe($message); +}); + +// --------------------------------------------------------------------- +// Stage 4: canonical-array choice derivation -> validation +// --------------------------------------------------------------------- + +test('Choice derived from $GLOBALS[poller_intervals] keys rejects unknown value', function () { + // poller_interval's allowed values are derived dynamically from + // $GLOBALS['poller_intervals'] keys. The closure form is what makes + // this safe: the global is read at validate() time, after + // global_arrays.php has populated it. + $GLOBALS['poller_intervals'] = [ + 60 => 'Every Minute', + 300 => 'Every 5 Minutes', + ]; + + $definitions = [ + 'poller_interval' => [ + 'method' => 'drop_array', + 'constraints' => [ + fn () => new Assert\Choice(choices: array_merge( + array_keys($GLOBALS['poller_intervals']), + array_map('strval', array_keys($GLOBALS['poller_intervals'])) + )), + ], + ], + ]; + + $result = CactiSettings::validate(['poller_interval' => '42'], $definitions); + + expect($result)->toHaveKey('poller_interval'); +}); + +test('Choice derived from $GLOBALS[poller_intervals] keys accepts canonical value', function () { + // $_POST values arrive as strings; the canonical keys are int. The + // constraint includes both forms via array_merge(array_keys, strval) + // so the posted '300' validates against either int 300 or string '300'. + $GLOBALS['poller_intervals'] = [ + 60 => 'Every Minute', + 300 => 'Every 5 Minutes', + ]; + + $definitions = [ + 'poller_interval' => [ + 'method' => 'drop_array', + 'constraints' => [ + fn () => new Assert\Choice(choices: array_merge( + array_keys($GLOBALS['poller_intervals']), + array_map('strval', array_keys($GLOBALS['poller_intervals'])) + )), + ], + ], + ]; + + expect(CactiSettings::validate(['poller_interval' => '300'], $definitions))->toBe([]); +}); + +test('mutating $GLOBALS[poller_intervals] between validate calls reshapes the choice set', function () { + // Confirms the closure reads the global on every invocation, not at + // definition time. A removed key on the second pass causes a value + // that previously passed to fail. + $definitions = [ + 'poller_interval' => [ + 'method' => 'drop_array', + 'constraints' => [ + fn () => new Assert\Choice(choices: array_merge( + array_keys($GLOBALS['poller_intervals']), + array_map('strval', array_keys($GLOBALS['poller_intervals'])) + )), + ], + ], + ]; + + $GLOBALS['poller_intervals'] = [60 => 'a', 300 => 'b']; + expect(CactiSettings::validate(['poller_interval' => '300'], $definitions))->toBe([]); + + $GLOBALS['poller_intervals'] = [60 => 'a']; + expect(CactiSettings::validate(['poller_interval' => '300'], $definitions))->toHaveKey('poller_interval'); +}); diff --git a/tests/HandOff/CliInvocationHandOffTest.php b/tests/HandOff/CliInvocationHandOffTest.php new file mode 100644 index 000000000..2826e7722 --- /dev/null +++ b/tests/HandOff/CliInvocationHandOffTest.php @@ -0,0 +1,270 @@ + ArgvInput -> Command::execute() arguments + * legacy shim entrypoint <-> cli/cacti.php entrypoint (parity) + * shell metacharacters in --oldrrd -> rejection before any file is touched + * Command::SUCCESS / Command::FAILURE -> subprocess exit code + * parent env CACTI_PHP_TESTING -> CactiCommand::initialize() bootstrap skip + * + * The in-process cases use Symfony's CommandTester. The cross-process cases + * reuse the runCli helper from CliInvocationTest so the bootstrap chain + * (cli_check.php -> autoload.php -> CactiApplication) is a real subprocess + * and not a mocked include graph. + */ + +use Symfony\Component\Console\Application; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Tester\CommandTester; + +if (!file_exists(dirname(__DIR__, 2) . '/lib/CactiCommand.php')) { + test('CLI invocation hand-off: feature not present on this branch', function () {}) + ->skip('lib/CactiCommand.php absent — feature PR #7075 not merged into develop yet'); + return; +} + +const CLI_HANDOFF_REPO_ROOT = __DIR__ . '/../..'; + +require_once dirname(__DIR__) . '/Helpers/CactiStubs.php'; +require_once dirname(__DIR__, 2) . '/lib/CactiCommand.php'; +require_once dirname(__DIR__, 2) . '/lib/CmdRealtimeCommand.php'; +require_once dirname(__DIR__, 2) . '/lib/SpliceRrdCommand.php'; + +/** + * Spawn a fresh PHP process for `php