diff --git a/.gitignore b/.gitignore
index c0d9b26..7b1bc15 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,3 @@
configs/
output/
+.omc/
diff --git a/BACKLOG.md b/BACKLOG.md
new file mode 100644
index 0000000..539b550
--- /dev/null
+++ b/BACKLOG.md
@@ -0,0 +1,117 @@
+# Backlog: plugin_weathermap
+
+Issues derived from SECURITY-AUDIT.md. Do not push to GitHub until contrib-ledger gate passes.
+
+---
+
+## Issue #1: refactor: extract FpingRunner and RrdCommandBuilder seams
+**Priority**: medium (blocker for security tests)
+**Labels**: refactor, tech-debt
+**Branch**: `refactor/1-shell-command-seams`
+**Evidence**: WeatherMapDataSource_fping.php:103; WeatherMapDataSource_rrd.php:296–325
+**Acceptance criteria**:
+- [ ] `src/Shell/FpingRunner.php` wraps popen call; accepts pre-built command string
+- [ ] `src/Rrd/RrdCommandBuilder.php` assembles rrdtool command string from typed args
+- [ ] Both classes instantiable without Cacti bootstrap
+- [ ] PHPStan level 6 passes on `src/`
+
+**Dependencies**: none
+
+---
+
+## Issue #2: security(cmd): escapeshellarg fping target and validate as IP/hostname
+**Priority**: medium
+**Labels**: security
+**Branch**: `security/2-escapeshellarg-fping-target`
+**Evidence**: lib/datasources/WeatherMapDataSource_fping.php:103 (WM-CMD-01)
+**Acceptance criteria**:
+- [ ] `$target` validated with `filter_var($target, FILTER_VALIDATE_IP)` or strict hostname regex before use
+- [ ] `escapeshellarg($target)` applied in command string
+- [ ] `src/Shell/FpingRunner.php` seam extracted and unit-tested
+- [ ] Security test WM-CMD-01 passes (todo removed)
+- [ ] Targets that fail IP/hostname validation are rejected with a logged warning
+
+**Dependencies**: Issue #1
+
+---
+
+## Issue #3: security(cmd): fix RRD popen argument quoting
+**Priority**: low
+**Labels**: security, tech-debt
+**Branch**: `security/3-fix-rrd-arg-quoting`
+**Evidence**: lib/datasources/WeatherMapDataSource_rrd.php:325, 429 (WM-CMD-02)
+**Acceptance criteria**:
+- [ ] Manual double-quote wrapping replaced with `escapeshellarg()` per argument
+- [ ] `$rrdfile` validated as absolute filesystem path with no shell metacharacters
+- [ ] `src/Rrd/RrdCommandBuilder.php` seam extracted and unit-tested
+- [ ] Security test WM-CMD-02 passes (todo removed)
+
+**Dependencies**: Issue #1
+
+---
+
+## Issue #4: security(path): add backslash check to wm_editor_sanitize_conffile
+**Priority**: low
+**Labels**: security, tech-debt
+**Branch**: `security/4-sanitize-conffile-backslash`
+**Evidence**: lib/editor.inc.php:232–246 (WM-PATH-01)
+**Acceptance criteria**:
+- [ ] `wm_editor_sanitize_conffile` rejects filenames containing `\`
+- [ ] Path traversal test WM-PATH-01 (backslash case) todo removed
+- [ ] Existing positive test cases remain green
+
+**Dependencies**: none
+
+---
+
+## Issue #5: security(input): validate $action against allowed-list in editor
+**Priority**: medium
+**Labels**: security, tech-debt
+**Branch**: `security/5-validate-editor-action`
+**Evidence**: weathermap-cacti-plugin-editor.php:91 — `$action = get_nfilter_request_var('action')` without allowed-list validation
+**Acceptance criteria**:
+- [ ] `$action` validated against an explicit list of valid editor action strings
+- [ ] Unknown actions produce a logged warning and abort
+- [ ] PHPStan level 6 passes
+
+**Dependencies**: none
+
+---
+
+## Issue #6: refactor: extract MapnameValidator as testable class
+**Priority**: low
+**Labels**: refactor, tech-debt
+**Branch**: `refactor/6-mapname-validator-class`
+**Evidence**: lib/editor.inc.php:232–246
+**Acceptance criteria**:
+- [ ] `src/Config/MapnameValidator.php` wraps `wm_editor_sanitize_conffile` logic
+- [ ] All existing sanitization cases covered by unit tests
+- [ ] PHPStan level 6 passes
+
+**Dependencies**: none
+
+---
+
+## Issue #7: ci: add Pest 4 + PHPStan CI workflow
+**Priority**: medium
+**Labels**: ci
+**Branch**: `ci/7-pest-phpstan-workflow`
+**Acceptance criteria**:
+- [ ] `.github/workflows/test.yml` runs `composer test` and `composer analyse`
+- [ ] PHPStan level 6 clean on `src/` and `tests/`
+- [ ] Actions pinned to full commit SHA
+
+**Dependencies**: Issues #1–#5
+
+---
+
+## Issue #8: docs: complete audit of weathermap-cacti-plugin-editor.php action dispatch
+**Priority**: medium
+**Labels**: security, docs
+**Branch**: `docs/8-editor-action-audit`
+**Evidence**: editor action variable not fully traced in this audit
+**Acceptance criteria**:
+- [ ] All action values and their code paths documented
+- [ ] Any state-changing actions that lack CSRF or auth checks identified and filed as separate issues
+
+**Dependencies**: none
diff --git a/SECURITY-AUDIT.md b/SECURITY-AUDIT.md
new file mode 100644
index 0000000..f12100c
--- /dev/null
+++ b/SECURITY-AUDIT.md
@@ -0,0 +1,174 @@
+# Security Audit: plugin_weathermap
+
+**Date**: 2026-03-09
+**Auditor**: automated static analysis + manual review
+**Scope**: all PHP files under plugin_weathermap/ excluding vendor/ (none present)
+
+---
+
+## Summary
+
+Weathermap is the most security-mature of the four plugins audited. The Cacti Group
+rewrite uses prepared statements throughout the management UI (weathermap-cacti-plugin-mgmt.php),
+consistently applies html_escape() on DB-sourced output, and enforces api_plugin_user_realm_auth
+at well-defined entry points. The two remaining surface areas are in the map-rendering engine
+(datasource plugins) where admin-controlled config values flow into shell commands without
+escapeshellarg, and a residual Windows-only path traversal gap in the config filename sanitizer.
+
+---
+
+## Findings
+
+### WM-CMD-01 — OS Command Injection via map config TARGET value (MEDIUM)
+**File**: `lib/datasources/WeatherMapDataSource_fping.php:103`
+**Category**: OS Command Injection
+**Severity**: MEDIUM (requires write access to a .conf map file; admin-controlled input)
+
+```php
+$command = $this->fping_cmd . " -t100 -r1 -p20 -u -C $ping_count -i10 -q $target 2>&1";
+$pipe = popen($command, 'r');
+```
+
+`$target` is extracted from the TARGET line in a weathermap `.conf` file via the regex
+`/^fping:(\S+)$/`. The regex captures any non-whitespace string, including shell
+metacharacters (`;`, `|`, `&`, `` ` ``, `$`). A crafted TARGET line such as
+`TARGET fping:192.0.2.1;id>/tmp/x` injects into the shell command.
+
+**Attack path**: Admin writes (or attacker achieves file-write on) a .conf file containing
+a malicious TARGET line → weathermap poller processes it → `popen` executes the injected command
+as the web/poller process user.
+
+**Remediation**: Wrap `$target` with `escapeshellarg()`. Additionally, validate that
+`$target` is a valid IP address or hostname before use (reject if it fails `filter_var`
+with `FILTER_VALIDATE_IP` or a strict hostname regex).
+
+---
+
+### WM-CMD-02 — OS Command Injection via RRD file path in popen command (LOW-MEDIUM)
+**File**: `lib/datasources/WeatherMapDataSource_rrd.php:325, 429`
+**Category**: OS Command Injection
+**Severity**: LOW-MEDIUM (rrdfile sourced from Cacti DB, admin-controlled)
+
+```php
+// args array assembled with:
+$args[] = "DEF:in=$rrdfile:" . $dsnames[IN] . ":$cf";
+// then:
+foreach ($args as $arg) {
+ if (strchr($arg, ' ') != false) {
+ $command .= ' "' . $arg . '"';
+ } else {
+ $command .= ' ' . $arg;
+ }
+}
+$pipe = popen($command, 'r');
+```
+
+`$rrdfile` and `$dsnames` originate from Cacti's `data_template_data` table (admin-set
+RRD paths). The quoting logic uses double quotes around args containing spaces, but does
+not escape double-quote characters within the value itself. A `$rrdfile` containing `"`
+or `` ` `` breaks out of the double-quote boundary.
+
+The `db_fetch_row_prepared` calls that source `$rrdfile` use parameterized queries, so
+the DB-layer is safe. The risk is an admin storing a crafted RRD path.
+
+**Remediation**: Validate `$rrdfile` as an absolute filesystem path with no shell
+metacharacters before embedding in command args. Use `escapeshellarg()` per argument
+rather than manual double-quoting.
+
+---
+
+### WM-PATH-01 — Path traversal in map config filename (LOW — mitigated, residual Windows risk)
+**File**: `weathermap-cacti-plugin-editor.php:96`, `lib/editor.inc.php:232–246`
+**Category**: Path Traversal
+**Severity**: LOW (effectively mitigated on Linux/macOS; residual risk on Windows)
+
+```php
+function wm_editor_sanitize_conffile($filename) {
+ $filename = wm_editor_sanitize_uri($filename);
+ if (substr($filename, -5, 5) != '.conf') { $filename = ''; }
+ if (strstr($filename, '/') !== false) { $filename = ''; }
+ return $filename;
+}
+```
+
+The sanitizer enforces `.conf` extension and rejects any `/` character, preventing
+Unix path traversal (e.g., `../../etc/passwd.conf` is rejected). PHP null-byte injection
+(`file.conf\0.php`) was fixed in PHP 8.0; with PHP 8.4 as the requirement it is not
+exploitable.
+
+Residual gap: Windows path separator `\` is not checked. On Windows hosts,
+`..\\configs\\other.conf` passes both checks. Since Cacti targets Linux, this is
+informational.
+
+**Remediation**: Add a backslash check for defense-in-depth: `if (strstr($filename, '\\') !== false) { $filename = ''; }`.
+
+---
+
+### WM-POSITIVE-01 — Management UI uses prepared statements throughout (INFORMATIONAL)
+**File**: `weathermap-cacti-plugin-mgmt.php:354–390, 467–476`
+
+The `weathermap_form_actions()` function iterates `$_POST`, extracts checkbox keys via
+`preg_match('/^chk_([0-9:a-z]+)$/', ...)`, validates numeric parts with
+`input_validate_input_number()`, and uses `db_execute_prepared` with `?` placeholders
+for all INSERT/DELETE operations. DB-sourced output is wrapped with `html_escape()`.
+No SQL injection or XSS issues found in this file.
+
+---
+
+### WM-POSITIVE-02 — Auth enforcement is consistent (INFORMATIONAL)
+**File**: `weathermap-cacti-plugin.php:418, 445`, `setup.php:766`
+
+`api_plugin_user_realm_auth` is called at the correct entry points for both viewer and
+management realms. No unauthenticated paths to state-modifying actions were found.
+
+---
+
+### WM-POSITIVE-03 — Editor mapname sanitization prevents path traversal (INFORMATIONAL)
+**File**: `weathermap-cacti-plugin-editor.php:91–96`
+
+```php
+$mapname = get_nfilter_request_var('mapname');
+$mapname = wm_editor_sanitize_conffile($mapname);
+```
+
+The sanitizer is applied immediately after reading `$mapname`. The `wm_editor_sanitize_uri`
+call within `wm_editor_sanitize_conffile` additionally strips dangerous URI characters.
+This is a well-implemented control.
+
+---
+
+## Unknowns and Blind Spots
+
+- **weathermap-cacti-plugin-editor.php** was only partially read. The editor has a large
+ surface area (map config editing, node/link add/remove). The `$action` variable is read
+ via `get_nfilter_request_var('action')` without validation against an allowed-list;
+ tracing what each action value triggers requires a full read.
+- **`lib/editor.actions.php`** was not fully read. It uses `wm_editor_sanitize_conffile`
+ on `$sourcemapname` (line 68) which is correct, but other action parameters may not
+ be sanitized.
+- **`weathermap-cacti-rebuild.php`** was not examined. Rebuild scripts that process
+ map configs offline may have additional shell execution paths.
+- **`check.php`** was not examined.
+- **fping target IP validation**: whether `$target` is validated as a valid IP/hostname
+ anywhere before reaching the popen call was not confirmed beyond the `\S+` regex.
+
+---
+
+## Seams Required Before TDD Can Fully Proceed
+
+| Seam | Files Affected | Description |
+|------|---------------|-------------|
+| `src/Shell/FpingRunner.php` | WeatherMapDataSource_fping.php:103 | Extract popen call; test command assembly without executing fping |
+| `src/Rrd/RrdCommandBuilder.php` | WeatherMapDataSource_rrd.php:296–325 | Extract arg assembly; test escapeshellarg application |
+| `src/Config/MapnameValidator.php` | editor.inc.php:232–246 | Expose sanitize logic as a testable class method |
+
+---
+
+## Estimated Effort
+
+| Finding | Fix Effort |
+|---------|-----------|
+| WM-CMD-01 (fping escapeshellarg + IP validation) | 1 h |
+| WM-CMD-02 (rrd arg quoting) | 1 h |
+| WM-PATH-01 (backslash check) | 15 min |
+| Full test coverage via seams | 1 day |
diff --git a/SECURITY.md b/SECURITY.md
new file mode 100644
index 0000000..a1a8a46
--- /dev/null
+++ b/SECURITY.md
@@ -0,0 +1,27 @@
+# Security Policy
+
+## Supported Versions
+
+This plugin follows the Cacti project's support policy. Security fixes are
+applied to the current development branch and backported per project policy.
+
+## Reporting a Vulnerability
+
+Report security vulnerabilities via the Cacti project's private security
+disclosure process:
+
+- GitHub Security Advisories: https://github.com/Cacti/plugin_weathermap/security/advisories
+- Do NOT open public issues for security vulnerabilities.
+
+Please include:
+- Description of the vulnerability
+- Steps to reproduce
+- Affected versions
+- Suggested remediation (if known)
+
+A maintainer will acknowledge the report within 72 hours and provide a
+remediation timeline.
+
+## Security Hardening Notes
+
+See SECURITY-AUDIT.md for the current known finding backlog and remediation status.
diff --git a/check.php b/check.php
index 30a82a5..6d7067f 100644
--- a/check.php
+++ b/check.php
@@ -1,4 +1,6 @@
datasourceclasses = array();
+ // $this->datasourceclasses = [];
$dh = opendir($dir);
if (!$dh) {
@@ -907,7 +909,7 @@ function DatasourceInit() {
wm_debug("Running $ds_class" . '->Init()');
- // $ret = call_user_func(array($ds_class, 'Init'), $this);
+ // $ret = call_user_func([$ds_class, 'Init'], $this);
// assert('isset($this->plugins["data"][$ds_class])');
$ret = $this->plugins['data'][$ds_class]->Init($this);
@@ -965,7 +967,7 @@ function ProcessTargets() {
foreach ($this->datasourceclasses as $ds_class) {
if (!$matched) {
- // $recognised = call_user_func(array($ds_class, 'Recognise'), $targetstring);
+ // $recognised = call_user_func([$ds_class, 'Recognise'], $targetstring);
$recognised = $this->plugins['data'][$ds_class]->Recognise($targetstring);
if ($recognised) {
@@ -2389,11 +2391,11 @@ function ReadConfig($input, $is_include = false) {
['NODE', '/^\s*LABELFONT\s+(\d+)\s*$/i', ['labelfont'=>1]],
['NODE', '/^\s*LABELANGLE\s+(0|90|180|270)\s*$/i', ['labelangle'=>1]],
- // array('(NODE|LINK)', '/^\s*TEMPLATE\s+(\S+)\s*$/i', array('template'=>1)),
+ // array('(NODE|LINK)', '/^\s*TEMPLATE\s+(\S+)\s*$/i', ['template'=>1]),
['LINK', '/^\s*OUTBWFORMAT\s+(.*)\s*$/i', ['bwlabelformats[OUT]'=>1, 'labelstyle'=>'--']],
['LINK', '/^\s*INBWFORMAT\s+(.*)\s*$/i', ['bwlabelformats[IN]'=>1, 'labelstyle'=>'--']],
- // array('NODE','/^\s*ICON\s+none\s*$/i',array('iconfile'=>'')),
+ // array('NODE','/^\s*ICON\s+none\s*$/i',['iconfile'=>'']),
['NODE', '/^\s*ICON\s+(\S+)\s*$/i', ['iconfile'=>1, 'iconscalew'=>'#0', 'iconscaleh'=>'#0']],
['NODE', '/^\s*ICON\s+(\S+)\s*$/i', ['iconfile'=>1]],
['NODE', '/^\s*ICON\s+(\d+)\s+(\d+)\s+(inpie|outpie|box|rbox|round|gauge|nink)\s*$/i', ['iconfile'=>3, 'iconscalew'=>1, 'iconscaleh'=>2]],
@@ -2654,7 +2656,7 @@ function ReadConfig($input, $is_include = false) {
}
}
- // array('(NODE|LINK)', '/^\s*TEMPLATE\s+(\S+)\s*$/i', array('template'=>1)),
+ // array('(NODE|LINK)', '/^\s*TEMPLATE\s+(\S+)\s*$/i', ['template'=>1]),
if (($last_seen == 'NODE' || $last_seen == 'LINK') && preg_match('/^\s*TEMPLATE\s+(\S+)\s*$/i', $buffer, $matches)) {
$tname = $matches[1];
@@ -3549,7 +3551,7 @@ function DrawMap($filename = '', $thumbnailfile = '', $thumbnailmax = 250, $with
foreach ($this->postprocessclasses as $post_class) {
wm_debug("Running $post_class" . '->run()');
- // call_user_func_array(array($post_class, 'run'), array(&$this));
+ // call_user_func_array([$post_class, 'run'], [&$this]);
$this->plugins['post'][$post_class]->run($this);
}
@@ -3987,7 +3989,7 @@ function PreloadMapHTML() {
$n = 0;
if (cacti_sizeof($myobj->overliburl[$dir]) > 0) {
- // print "ARRAY:".is_array($link->overliburl[$dir])."\n";
+ // print "ARRAY:".is_array($link->overliburl[$dir)]."\n";
foreach ($myobj->overliburl[$dir] as $url) {
if ($n > 0) {
$data_hover .= '
';
diff --git a/lib/WeatherMap.functions.php b/lib/WeatherMap.functions.php
index 964deb1..faef1d3 100644
--- a/lib/WeatherMap.functions.php
+++ b/lib/WeatherMap.functions.php
@@ -1,4 +1,6 @@
a_offset = 'C';
// $this->b_offset = 'C';
- // $this->targets = array();
+ // $this->targets = [];
}
function Reset(&$newowner) {
@@ -270,7 +272,7 @@ function DrawComments($image, $col, $widths) {
}
if ($comment != '') {
- // print "\n\n----------------------------------------------------------------\nComment $dir for ".$this->name."\n";;
+ // print "\n\n----------------------------------------------------------------\nComment $dir for ".$this->name."\n";
[$textlength, $textheight] = $this->owner->myimagestringsize($this->commentfont, $comment);
@@ -623,7 +625,7 @@ function WriteConfig() {
['duplex', 'DUPLEX', CONFIG_TYPE_LITERAL],
['commentstyle', 'COMMENTSTYLE', CONFIG_TYPE_LITERAL],
['labelboxstyle', 'BWSTYLE', CONFIG_TYPE_LITERAL],
- // array('usescale', 'USESCALE', CONFIG_TYPE_LITERAL),
+ // ['usescale', 'USESCALE', CONFIG_TYPE_LITERAL],
['bwfont', 'BWFONT', CONFIG_TYPE_LITERAL],
['commentfont', 'COMMENTFONT', CONFIG_TYPE_LITERAL],
diff --git a/lib/WeatherMapNode.class.php b/lib/WeatherMapNode.class.php
index ab0614a..e2604d2 100644
--- a/lib/WeatherMapNode.class.php
+++ b/lib/WeatherMapNode.class.php
@@ -1,4 +1,6 @@
nodes[$this->name]->width = imagesx($icon_im);
$map->nodes[$this->name]->height = imagesy($icon_im);
- // $map->imap->addArea("Rectangle", "NODE:" . $this->name . ':0', '', array($icon_x1, $icon_y1, $icon_x2, $icon_y2));
+ // $map->imap->addArea("Rectangle", "NODE:" . $this->name . ':0', '', [$icon_x1, $icon_y1, $icon_x2, $icon_y2]);
$map->nodes[$this->name]->boundingboxes[] = [$icon_x1, $icon_y1, $icon_x2, $icon_y2];
}
}
@@ -577,7 +579,7 @@ function pre_render($image, &$map) {
$label_y2 += ($this->labeloffsety + $dy);
if ($this->label != '') {
- // $map->imap->addArea("Rectangle", "NODE:" . $this->name .':1', '', array($label_x1, $label_y1, $label_x2, $label_y2));
+ // $map->imap->addArea("Rectangle", "NODE:" . $this->name .':1', '', [$label_x1, $label_y1, $label_x2, $label_y2]);
$map->nodes[$this->name]->boundingboxes[] = [$label_x1, $label_y1, $label_x2, $label_y2];
}
@@ -796,7 +798,7 @@ function WriteConfig() {
// $field = 'zorder'; $keyword = 'ZORDER';
$basic_params = [
- // array('template','TEMPLATE',CONFIG_TYPE_LITERAL),
+ // ['template','TEMPLATE',CONFIG_TYPE_LITERAL],
['label', 'LABEL', CONFIG_TYPE_LITERAL],
['zorder', 'ZORDER', CONFIG_TYPE_LITERAL],
['labeloffset', 'LABELOFFSET', CONFIG_TYPE_LITERAL],
diff --git a/lib/datasources/WeatherMapDataSource_cacti.php b/lib/datasources/WeatherMapDataSource_cacti.php
index 9ca5e22..ee05077 100644
--- a/lib/datasources/WeatherMapDataSource_cacti.php
+++ b/lib/datasources/WeatherMapDataSource_cacti.php
@@ -1,4 +1,6 @@
context === 'cacti') {
diff --git a/lib/datasources/WeatherMapDataSource_cactihost.php b/lib/datasources/WeatherMapDataSource_cactihost.php
index 5dbded1..9e9146c 100644
--- a/lib/datasources/WeatherMapDataSource_cactihost.php
+++ b/lib/datasources/WeatherMapDataSource_cactihost.php
@@ -1,4 +1,6 @@
+
+
+
+ tests
+
+
+
diff --git a/setup.php b/setup.php
index bab278f..f853a36 100644
--- a/setup.php
+++ b/setup.php
@@ -1,4 +1,6 @@
not->toContain("if (isset_request_var('action')) {\n\t\$action = get_nfilter_request_var('action');\n}");
+ expect($entry)->toContain("wm_editor_sanitize_action(get_nfilter_request_var('action'), [");
+ });
+});
diff --git a/tests/Helpers/GlobalStubs.php b/tests/Helpers/GlobalStubs.php
new file mode 100644
index 0000000..f58d78a
--- /dev/null
+++ b/tests/Helpers/GlobalStubs.php
@@ -0,0 +1,8 @@
+toContain("switch(\$action) {");
+ expect($entry)->toContain("\$action = wm_editor_sanitize_action(get_nfilter_request_var('action'), [");
+ });
+});
diff --git a/tests/Security/CommandInjectionTest.php b/tests/Security/CommandInjectionTest.php
new file mode 100644
index 0000000..2a66115
--- /dev/null
+++ b/tests/Security/CommandInjectionTest.php
@@ -0,0 +1,162 @@
+fping_cmd . " -t100 -r1 -p20 -u -C $ping_count -i10 -q $target 2>&1";
+ // $pipe = popen($command, 'r');
+ //
+ // A crafted target value with shell metacharacters produces an injectable command.
+ $fping_cmd = '/usr/local/sbin/fping';
+ $ping_count = 5;
+ $target = '192.0.2.1; id > /tmp/wm_pwned';
+
+ $command = $fping_cmd . " -t100 -r1 -p20 -u -C $ping_count -i10 -q $target 2>&1";
+
+ expect($command)->toContain('; id > /tmp/wm_pwned');
+
+ // Fixed form: validate then escapeshellarg.
+ $safe_command = $fping_cmd
+ . ' -t100 -r1 -p20 -u -C ' . (int) $ping_count
+ . ' -i10 -q ' . escapeshellarg($target)
+ . ' 2>&1';
+
+ // escapeshellarg wraps the target; metachar inside quotes is neutralised
+ expect($safe_command)->toContain("'");
+ });
+
+ it('hostname regex \S+ does not prevent shell metacharacter injection', function (): void {
+ // The regex /^fping:(\S+)$/ captures any non-whitespace string.
+ // Shell metacharacters like ; | & ` $ are non-whitespace and pass the regex.
+ $targetstring = 'fping:192.0.2.1;id';
+ preg_match('/^fping:(\S+)$/', $targetstring, $matches);
+
+ expect($matches[1])->toBe('192.0.2.1;id');
+ // The regex provides no shell safety — only the allowlist validation does.
+ expect($matches[1])->toContain(';');
+ });
+
+ it('escapeshellarg neutralises semicolon in fping target', function (): void {
+ $target = '192.0.2.1; rm -rf /';
+ $safe = escapeshellarg($target);
+
+ expect($safe)->toStartWith("'")->toEndWith("'");
+ });
+
+ it('allowlist regex accepts valid IPv4 address', function (): void {
+ $target = '192.0.2.1';
+ expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(1);
+ });
+
+ it('allowlist regex accepts valid IPv6 address', function (): void {
+ $target = '2001:db8::1';
+ expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(1);
+ });
+
+ it('allowlist regex accepts valid hostname', function (): void {
+ $target = 'router-01.example.com';
+ expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(1);
+ });
+
+ it('allowlist regex rejects target containing semicolon', function (): void {
+ $target = '192.0.2.1; id';
+ expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(0);
+ });
+
+ it('allowlist regex rejects target containing pipe', function (): void {
+ $target = '192.0.2.1|cat /etc/passwd';
+ expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(0);
+ });
+
+ it('allowlist regex rejects target containing backtick', function (): void {
+ $target = '`id`';
+ expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(0);
+ });
+
+ it('allowlist regex rejects target containing dollar sign', function (): void {
+ $target = '$(/bin/sh)';
+ expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(0);
+ });
+
+ it('allowlist regex rejects target containing space', function (): void {
+ $target = '192.0.2.1 -r 1';
+ expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(0);
+ });
+});
+
+describe('Weathermap RRD popen command injection (WM-CMD-02)', function (): void {
+ it('demonstrates that rrdfile path without escapeshellarg allows metacharacter injection (pre-fix)', function (): void {
+ // WeatherMapDataSource_rrd.php (pre-fix) built args then assembled with manual strchr quoting:
+ // if (strchr($arg, ' ') != false) { $command .= ' "' . $arg . '"'; }
+ // A $rrdfile with embedded double-quote breaks out of the quoted arg boundary.
+ $rrdtool = '/usr/bin/rrdtool';
+ $rrdfile = '/var/rra/host.rrd" --daemon /tmp/attacker.sock "';
+ $ds = 'traffic_in';
+ $cf = 'AVERAGE';
+
+ $arg = "DEF:in=$rrdfile:$ds:$cf";
+ expect($arg)->toContain('"');
+ });
+
+ it('escapeshellarg neutralises double-quote in rrdfile path', function (): void {
+ $rrdfile = '/var/rra/host.rrd" --daemon /tmp/attacker.sock "';
+ $safe = escapeshellarg($rrdfile);
+
+ expect($safe)->toStartWith("'");
+ // The injected double-quote is contained inside single-quoted shell argument.
+ expect($safe)->toContain('"');
+ // No unquoted shell metacharacters remain outside the single-quote wrapper.
+ expect(substr($safe, 0, 1))->toBe("'");
+ expect(substr($safe, -1))->toBe("'");
+ });
+
+ it('escapeshellarg neutralises backtick in rrdfile path', function (): void {
+ $rrdfile = '/var/rra/`id`.rrd';
+ $safe = escapeshellarg($rrdfile);
+
+ expect($safe)->toStartWith("'");
+ // Backtick is inert inside single quotes.
+ expect($safe)->toContain('`');
+ });
+
+ it('escapeshellarg on a path with spaces produces a single-quoted string', function (): void {
+ $rrdfile = '/var/rra/my file.rrd';
+ $safe = escapeshellarg($rrdfile);
+
+ expect($safe)->toBe("'/var/rra/my file.rrd'");
+ });
+
+ it('per-arg escapeshellarg on clean args preserves them functionally', function (): void {
+ // Verify that escapeshellarg on benign args round-trips correctly when unquoted by shell.
+ // We can only check the string form here; the shell would strip the surrounding quotes.
+ $arg = 'AVERAGE';
+ $safe = escapeshellarg($arg);
+ expect($safe)->toBe("'AVERAGE'");
+
+ $arg2 = '--start';
+ $safe2 = escapeshellarg($arg2);
+ expect($safe2)->toBe("'--start'");
+ });
+});
diff --git a/tests/Security/PathTraversalTest.php b/tests/Security/PathTraversalTest.php
new file mode 100644
index 0000000..7fd2a82
--- /dev/null
+++ b/tests/Security/PathTraversalTest.php
@@ -0,0 +1,81 @@
+toBe('my-map.conf');
+ });
+
+ it('allows filenames with hyphens and underscores', function (): void {
+ expect(sanitize_conffile_stub('site_network-01.conf'))->toBe('site_network-01.conf');
+ });
+
+ it('rejects filenames that do not end in .conf', function (): void {
+ expect(sanitize_conffile_stub('../../etc/passwd'))->toBe('');
+ expect(sanitize_conffile_stub('map.conf.php'))->toBe('');
+ expect(sanitize_conffile_stub('map'))->toBe('');
+ });
+
+ it('rejects filenames containing forward slash', function (): void {
+ expect(sanitize_conffile_stub('../other/map.conf'))->toBe('');
+ expect(sanitize_conffile_stub('/absolute/path.conf'))->toBe('');
+ });
+
+ it('rejects filenames containing Windows backslash (WM-PATH-01 fix)', function (): void {
+ // Prior to the fix, '..\\configs\\other.conf' passed because only '/' was checked.
+ // The backslash check now closes this gap for Windows host deployments.
+ expect(sanitize_conffile_stub('..\\configs\\other.conf'))->toBe('');
+ expect(sanitize_conffile_stub('..\\..\\windows\\system32\\evil.conf'))->toBe('');
+ expect(sanitize_conffile_stub('sub\\map.conf'))->toBe('');
+ });
+
+ it('rejects empty filename', function (): void {
+ expect(sanitize_conffile_stub(''))->toBe('');
+ });
+
+ it('rejects filename that is only .conf extension', function (): void {
+ // Edge case: substr('.conf', -5, 5) === '.conf' passes the extension check.
+ // That is acceptable — the sanitizer is a filename guard, not a name-quality check.
+ $result = sanitize_conffile_stub('.conf');
+ expect($result)->toBe('.conf');
+ });
+});
diff --git a/tests/Unit/EditorActionNormalizationTest.php b/tests/Unit/EditorActionNormalizationTest.php
new file mode 100644
index 0000000..a81740c
--- /dev/null
+++ b/tests/Unit/EditorActionNormalizationTest.php
@@ -0,0 +1,21 @@
+toContain('function wm_editor_sanitize_action($action, $valid = [])');
+ expect($entry)->toContain("'draw'");
+ expect($entry)->toContain("'load_map_javascript'");
+ });
+
+ it('normalizes invalid editor actions to empty string at the entrypoint', function (): void {
+ $entry = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-editor.php');
+
+ expect($entry)->toContain("\$action = wm_editor_sanitize_action(get_nfilter_request_var('action'), [");
+ expect($entry)->not->toContain("\$action = get_nfilter_request_var('action');");
+ });
+});
diff --git a/weathermap-cacti-plugin-editor.php b/weathermap-cacti-plugin-editor.php
index 99f29f3..d5c5fb6 100644
--- a/weathermap-cacti-plugin-editor.php
+++ b/weathermap-cacti-plugin-editor.php
@@ -1,4 +1,6 @@