fix: security hardening, bug fixes, and test suite (PR #215 follow-up)

.gitignore

@@ -1,2 +1,3 @@
 configs/**
 output/**
+.omc/

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

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 |

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.

check.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /*
  +-------------------------------------------------------------------------+
  | Copyright (C) 2022-2026 The Cacti Group, Inc.                           |

cli/bristle.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /*
  +-------------------------------------------------------------------------+
  | Copyright (C) 2022-2026 The Cacti Group, Inc.                           |

cli/cacti-integrate.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /*
  +-------------------------------------------------------------------------+
  | Copyright (C) 2022-2026 The Cacti Group, Inc.                           |

cli/cacti-mapper.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /*
  +-------------------------------------------------------------------------+
  | Copyright (C) 2022-2026 The Cacti Group, Inc.                           |
@@ -147,7 +149,7 @@
 			unset($interfaces[$key]);
 			$cleaned++;
 		} else {
-			$interfaces[$key]['nicename'] = (isset($int['name']) ? $int['name'] : (isset($int['descr']) ? $int['descr'] : (isset($int['alias']) ? $int['alias'] : 'Interface #' . $int['index'])));
+			$interfaces[$key]['nicename'] = ($int['name'] ?? ($int['descr'] ?? ($int['alias'] ?? 'Interface #' . $int['index'])));
 		}
 	}
 }

cli/convert-to-dsstats.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /*
  +-------------------------------------------------------------------------+
  | Copyright (C) 2022-2026 The Cacti Group, Inc.                           |

cli/index.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /*
  +-------------------------------------------------------------------------+
  | Copyright (C) 2022-2026 The Cacti Group, Inc.                           |

cli/map-tidyup.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /*
  +-------------------------------------------------------------------------+
  | Copyright (C) 2022-2026 The Cacti Group, Inc.                           |

cli/upgrade_configs.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /*
  +-------------------------------------------------------------------------+
  | Copyright (C) 2022-2026 The Cacti Group, Inc.                           |