10 Commits
Author SHA1 Message Date
Thomas Vincent 08d0f94f24 Fix develop client behavior, popup isolation and PHP method visibility (#8292)
* fix(core): correct client state and declare public method contracts

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

* fix(security): isolate Midwinter external homepage popup

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

* fix(client): isolate menu state and simplify validated JavaScript guards

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

* test(auth): use certificate expiry truth in rendering assertions

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

* refactor(js): remove unused local storage while retaining initializer calls

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

* fix(js): encode inline translated values through the shared JSON boundary

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

* style(js): wrap encoded translation literals without changing messages

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

* fix(client): remove unused checkbox reads from graph resizing

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

* refactor(client): declare repeated function locals once

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

* test(ui): run column width regression in browser CI

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-10-11 21:15:39 -04:00
TheWitness 7b551eea72 chore: normalize CRLF line endings to LF across Cacti-owned files (#8156)
Excludes vendored packages, third-party JS, patches, and the Win32 probe fixture. Pure EOL change; guards pass and php -l is clean.
2026-10-02 17:22:42 -04:00
TheWitnessandci 9facd9b500 security: batch 7 develop - consolidate remaining develop-only GHSA advisories (12 fixes + 8 confirmations) (#8149)
* security: batch 7 develop - consolidate remaining develop-only GHSA advisories (12 fixes + 8 regression confirmations)

Fixes: f7jw (automation preview XSS), rp5g (db_column_exists LIKE), vx2m (automation pivot CASE WHEN), h5wg (plugin DDL identifiers), 929h (aggregate LIKE filter), mfw8 (add_graphs REGEXP), m49v (user_admin sort_column allowlist), mpfm (rrd error-image theme allowlist), m3fh (SNMPv3 control chars), cg45 (remember-me row revoke), xq26 (remember-me retention window), m67r (import xml_path containment).

Confirmed already fixed on develop, pinned with regression tests: 4gph, 977w, mgcq, 5p79, cfhh, wfcf, pjq5, wp33.

Excluded: 227p (1.2.x-only), mx5c (maintainer private branch).

* security: address batch-7 review feedback (lexical-first xml_path containment, session-wide sort allowlist, connection-aware db_column_exists quoting, reject-not-normalize plugin DDL identifiers, behavior-based test names)

* style: apply php-cs-fixer single-quote fix to api_automation pivot builders; add WSL dev-environment note to copilot instructions

* fix phpstan string typing

* test: refresh SQL interpolation baseline for db_column_exists db_qstr change (database.php 9->8)

---------

Co-authored-by: ci <ci@local>
2026-10-02 15:27:34 -04:00
TheWitness 41e6d9e033 security: consolidate verified GHSA fixes for develop (batch 4) (#8145)
* security: consolidate verified GHSA fixes for develop (batch 4)

Develop counterparts of the 1.2.x batch-4 fixes in #8144, re-verified against
current develop HEAD.

- GHSA-4rmr-wvjq-qxc2: include/auth.php Basic Auth and guest branches fall
  through to the per-page realm check instead of returning early.
- GHSA-j67j-wpm4-9g3x: the pre-dispatch update_policy branch in user_admin.php
  and user_group_admin.php requires a CSRF-token POST. The denylist -> allowlist
  redesign is tracked for 1.3 in #8143.
- GHSA-2p3x-f5jm-rg6p: the View-Graphs -> poller RCE chain is broken by the
  realm check (4rmr), the update_policy guard (j67j) and shell-free webhits (x5pw).
- GHSA-x5pw-cg7j-mj4r: scripts/webhits.pl runs wc through a shell-free list-form open.
- GHSA-9737-rqh3-h7fh: names interpolated into raise_message() now use __esc()
  across the UI; URL- and HTML-bearing messages are excluded.
- GHSA-9x9h-2577-9w86: the colordropdown input is built as a node and the label
  is no longer round-tripped through parseHTML (the list item already used
  createTextNode on develop).
- GHSA-h96j-8xhc-2p3w: remote_agent.php authorizes on $_SERVER['REMOTE_ADDR'],
  not get_client_addr(), which honours client-settable proxy headers.

Regression guards under tests/Unit/Security/{Auth,Csrf,Xss,ShellInjection}.
php -l, check-changelog.py and the sink/architectural baselines all pass.

* chore: reference PR #8145 on the batch-4 1.3.0-dev CHANGELOG entries

* security: resolve develop merge - keep id-only xmlerror unescaped (matches #8141 h745 test)

* security(GHSA-j67j): use REQUEST_METHOD POST guard instead of raw \ (hardening-pattern-guard)

* security(GHSA-9737): fix review findings - drop double-encoding in __esc, escape missed name sinks, correct plugins format-arg count
2026-10-02 13:10:42 -04:00
TheWitness 0055dd00cc security: consolidate verified GHSA fixes for develop (batch 3) + CHANGELOG CVE reconciliation (#8141)
* security: consolidate verified GHSA fixes for develop (batch 3)

Develop counterparts of the 1.2.x fixes in #8138, each re-verified against
develop HEAD.

- GHSA-77pr-g949-4v5f: spikekill.php also requires is_graph_allowed() on the
  target graph, not just the Spike Kill realm.
- GHSA-w3qw-762w-6g9v: the include/global.php GET action guard fails closed on
  an array-form action[] before the scalar denylist comparison.
- GHSA-xhpr-w454-cc9w: update_poller_cache() casts the stored output_type to
  int before interpolation.
- GHSA-j3px-vw6r-g25x: graphs_new.php escapes stored snmp_index via db_qstr()
  in the data-query IN() pivot.
- GHSA-h745-3r76-g253: graphs_new.php escapes the data-query and field names in
  the XML parse error messages (__ -> __esc).

GHSA-297h-95ff-r767 does not apply on develop: the clog log-file selector
renders through the drop_array filter framework, which escapes.

tests/Unit/Security/ConsolidatedBatch3Test.php adds source-scanning regression
guards. CHANGELOG (1.3.0-dev) updated; php -l, the security baselines, and
check-changelog.py all pass.

* chore: lead 1.3.0-dev security CHANGELOG entries with the GHSA id

Normalize the 1.3.0-dev -security entries to the approved
"-security: GHSA-xxxx [CVE-xxxx] Description" form, moving the advisory id to
the front. CVE tokens are omitted because none of these advisories have a CVE
issued yet (verified via the repository security-advisories API). Covers the
five batch-3 entries plus the pre-existing entries that still used the trailing
(GHSA-xxxx) form.

* chore: use the -security#GHSA-xxxx: form for the 1.3.0-dev CHANGELOG entries

Correct the batch-3 and reconciled 1.3.0-dev -security entries to the approved
"-security#GHSA-xxxx: CVE-yyyy Description" form (GHSA id in the prefix). No CVE
tokens are added because none of these advisories have a CVE issued yet
(verified via the repository security-advisories API).

* test: split batch-3 security guards into vulnerability-class files; reconcile CHANGELOG

- Move the ConsolidatedBatch3Test.php guards into the matching Auth, Csrf,
  SqlInjection and Xss folders with behavior-based filenames, and strengthen
  them per review: the GET-guard test asserts the array branch actually returns
  405 and exits; the snmp_index test rejects both raw concatenation branches;
  the parse-error test asserts every name-bearing message uses __esc().
- Append this pull request number to the five new 1.3.0-dev security entries.
- Add issued CVE identifiers to the older -security#GHSA CHANGELOG entries
  (verified via the repository security-advisories API). Entries with no CVE
  yet, entries that already carried a CVE, and non-GHSA lines are unchanged.
2026-10-02 11:03:52 -04:00
TheWitness 1a37594ad6 Replace unmaintained phpdiff vendor lib with jfcherng/php-diff (develop) (#8050)
* chore: replace unmaintained phpdiff vendor lib with jfcherng/php-diff

The manually-vendored include/vendor/phpdiff (chrisboulton/php-diff v1.1,
2009 - unmaintained since ~2013) backed the package-import file diff
viewer (package_import.php's package_diff_file()), its only call site.

Replace it with jfcherng/php-diff (^6.16, BSD-3-Clause), an actively
maintained continuation of the same library, declared as a real Composer
dependency in composer.json/composer.lock (installed at build/CI time,
same as this branch's other composer.json dependencies - not committed
to include/vendor, unlike the old hand-vendored phpdiff).

package_import.php now uses Jfcherng\Diff\Differ + RendererFactory in
place of the old Diff/Diff_Renderer_Html_Inline classes. Preserved the
existing nosemgrep justification comment, updated to reference the new
library's AbstractHtml::htmlSafe() escaping.

Adopted the vendor's own diff-table.css as the Diff.css in all 10 themes,
replacing the bespoke per-theme stylesheets that were tied to the old
library's class names.

* fix: address review feedback on jfcherng/php-diff migration

- package_import.php: narrow $oldfile/$newfile to array before
  constructing Jfcherng\Diff\Differ so PHPStan can see the
  false branch is excluded (fixes level 6/8 analysis failure).
- tests/tools/check_vendor_policy.php: drop the removed
  include/vendor/phpdiff/ path from the legacy allowlist.
- tests/Unit/Security/Xss/PackageDiffRendererTest.php: rewrite
  against the new Jfcherng\Diff\Differ/RendererFactory API instead
  of the deleted vendored phpdiff classes.
2026-09-21 13:59:50 -04:00
Thomas Vincent 08c02d3322 security: resolve open code scanning alerts (#8003)
* security: resolve open code scanning alerts

* test(security): constrain SQL helper sanitizer

* fix(security): address follow-up review
2026-09-06 08:41:47 -07:00
Thomas Vincent 95060c49ba Close test-suite discovery and coverage gaps (#7970) 2026-09-05 18:38:42 -07:00
Thomas Vincent 619b33142e feat(rendering): add HTML, URL, and inline-data output helpers (#7252) 2026-08-22 17:15:06 -07:00
Sean Mancini 96033267cb refactor(tests): reorganize unit tests into domain category folders (#7733)
* refactor(tests): reorganize unit tests into domain category folders

- Consolidate per-advisory GHSA regression tests into Security/ subfolders by vuln class
- Group all 186 test files into domain folders: DataCollection/, Ui/, Core/,
  Database/, Installer/, Scripts/, Security/, Plugin/
- Strip GitHub issue/GHSA numbers from filenames; keep IDs in test descriptions
- Fix all __DIR__ relative paths to match new folder nesting depth
- Update CI workflow path triggers and phpunit XML configs for new file locations
- Update .github/copilot-instructions.md with unit test organization guidelines
- Add tests/vendor/ to .gitignore

* add gaurds

* fix up guard for unit tests

* fix redeclaration of cacti_log in tests

* fix updates to unit tests

* fix test patjhs

* fix: update test __DIR__ paths for reorganized test structure

* fix: conditionally define read_user_setting to prevent redeclaration in tests

* fix: conditionally define functions to prevent redeclaration in tests

- Wrap clean_up_lines, raise_message, cacti_debug_backtrace, and get_debug_prefix
  with function_exists() checks to prevent conflicts when test stubs are loaded
  before lib/functions.php in the same PHP process

* fix: add unit test bootstrap to prevent function redeclaration errors

- Create tests/bootstrap-unit.php to load core libraries before tests
- Ensures lib/functions.php loads before test stubs define stub functions
- Test stubs use function_exists() checks, so they detect pre-loaded functions
- Update phpunit.xml to use bootstrap-unit.php for Unit testsuite
- Fixes 'Cannot redeclare read_user_setting()' errors when tests run in same process

* fix: load test stubs before global.php in unit test bootstrap

- Reorder bootstrap to load CactiStubs.php and UnitStubs.php first
- Ensures stub functions like __() are available when global.php loads
- global.php calls get_cacti_version_text() which needs __() for translations
- Fixes 'Call to undefined function __()' error during bootstrap

* fix: remove function conflicts from test stubs

- Remove stub definitions for functions already in lib/functions.php:
  cacti_sizeof, cacti_log, cacti_strtolower, read_config_option,
  read_user_setting, cacti_count
- Keep translation function stubs (__(), __esc()) needed by global.php
- Update bootstrap to load lib/functions.php after stubs
- This prevents function redeclaration errors while maintaining test isolation

* fix: prevent constant and variable redeclaration in bootstrap

- Remove explicit PHP_TESTING and CACTI_TEST_BOOTSTRAP defines from bootstrap
  (CactiStubs.php now handles this with guards)
- Add guards to CactiStubs.php to prevent redefinition of PHP_TESTING
- Only set CACTI_TEST_BOOTSTRAP env vars if not already set
- Fixes PHP warnings about 'Constant X already defined'

* fix: remove redundant lib/functions.php load from bootstrap

include/global.php already loads lib/functions.php at lines 287-288.
The bootstrap was trying to load it again after global.php, but global.php
needs format_cacti_version() to be available on lines 293-294 to define
CACTI_VERSION and CACTI_VERSION_FULL constants.

Solution: Let global.php handle loading lib/functions.php itself, and don't
load it separately in the bootstrap.

* fix: remove more conflicting function stubs from test helpers

- Remove stubs for functions also defined in lib/functions.php:
  raise_message, clean_up_lines, cacti_debug_backtrace, get_debug_prefix
- Remove POLLER_ID constant stub (defined unconditionally by global.php)
- Keep only translation functions and test-specific helpers
- This prevents function/constant redeclaration errors

* fix: minimize test stubs to only translation functions

Remove ALL helper functions and constants from UnitStubs.php that are:
- Defined in lib/functions.php (raise_message, clean_up_lines, etc.)
- Defined in lib/html_utility.php (srv, set_request_var)
- Defined in global_constants.php (POLLER_VERBOSITY_*, MESSAGE_LEVEL_*)
- Defined in global.php (POLLER_ID)

Keep ONLY translation functions (__(), __esc()) that are:
- Called by global.php during initialization
- Not defined elsewhere
- Required for proper bootstrap sequence

This eliminates all constant and function redeclaration errors.

* fix: remove __esc stub - already defined in global_languages.php

Keep only __() translation function stub, which is needed by global.php
and not defined elsewhere in Cacti core.

* fix: handle no-arg case in __() translation stub

The __() function may be called with just a string and no format arguments.
Previously, vsprintf() would fail with 'arguments array must contain 2 items'.
Now, if no args are provided, return the text as-is.
2026-08-16 19:23:31 -07:00