Delete unused configuration files with named confirmation

[alcatron]

Add a red minus action beside unused configuration files. Confirmation names the file and explains that deletion is permanent; the action uses a CSRF-protected POST.

Before deleting, validate the basename, extension, canonical path, regular-file status and symlink status. Protect configurations referenced by registered maps and stop when the database usage query fails. Report success only after unlink succeeds. Escape filename-bearing status messages for HTML and use the existing Cacti-compatible attribute helper for buttons and confirmation text.

Validation: 229 PHP 8.3 tests passed in an isolated fixture layout. Temporary-file tests cover successful deletion, in-use files, database false/null failures, invalid paths, symlinks and filesystem failures. An action-level regression verifies escaping in both successful and in-use messages; nonce/CSRF checks also pass. Tests only delete their own temporary files. Focused PHPStan level 8 passed for the deletion helper. Syntax, whitespace and manifest checks passed; measured changed-line coverage is 100%, with the existing management-entry exemption. Full running-installation permission integration remains unverified for this revision.

Latest review follow-up

Registration and deletion now take the same exclusive nonblocking config-file lock and fail safely when the file is busy or unavailable. The lock spans the usage check/unlink and registration insertion, verifies the opened inode still matches the path and releases in finally blocks. Regression checks invoke the actual registration function during deletion and attempt deletion during registration, plus missing-file and exception paths. Latest local validation: 230 PHP tests (5466 assertions), measured changed-line coverage 40/40 and PHPStan level 8 on the helper with installation symbols supplied. The translation template includes the retry message.

These follow-up checks use isolated local fixtures, not a running Cacti installation. PHP 8.3 syntax, whitespace, manifest and translation-template checks passed. GitHub CI must still confirm the revised commits.

[Copilot]

🟡 Changes recommended

Database failures can permit deletion, and filename-bearing messages introduce XSS risk.

5 open findings

• Fail closed when usage query fails · New
• Escape request-derived filename in UI message · New
• Use standard GPL v2 header in new PHP file · New
• Use Cacti attribute-context encoder · New
• Use standard GPL v2 header in new PHP test · New

What changed in this PR

Adds secure deletion controls for unused Weathermap configuration files.

Changes:

• Adds confirmed, CSRF-protected deletion UI and backend handling.
• Validates files and protects configurations referenced by maps.
• Adds tests, translations, and changelog documentation.

File	Description
weathermap-cacti-plugin-mgmt.php	Integrates deletion actions and status messages.
lib/​editor.config-delete.php	Implements validation, deletion, and UI helpers.
tests/​Unit/​ConfigFileDeleteTest.php	Tests validation, deletion, failure handling, and rendering.
locales/​po/​cacti.pot	Adds translatable deletion messages.
CHANGELOG.md	Documents the feature.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Non-canonical registered paths and concurrent registration can allow deletion of an in-use configuration.

1 open finding

• Race between usage check and config deletion · New

5 resolved since last review

• Escape request-derived filename in UI message
• Fail closed when usage query fails
• Use standard GPL v2 header in new PHP test
• Use Cacti attribute-context encoder
• Use standard GPL v2 header in new PHP file

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

The destructive filesystem and concurrency behavior still requires validation in a running Cacti installation.

0 open findings

1 resolved since last review

• Race between usage check and config deletion

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

Functional safeguards and tests are comprehensive, with only a minor PHPDoc formatting issue remaining.

1 open finding

• Add blank PHPDoc line before @​return · New

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

The destructive filesystem and concurrency behavior still lacks validation in a running Cacti installation.

0 open findings

1 resolved since last review

• Add blank PHPDoc line before @​return

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Equivalent non-normalized registered paths can bypass the in-use check and allow deletion of an active configuration.

1 open finding

• Canonicalize config aliases before checking usage · New

🧠 Review effort: Balanced

[alcatron]

Resolved the new changelog conflict after #292 merged. All 239 local tests pass (5549 assertions), with syntax, manifest and whitespace checks passing. The feature changes remain intact; validation uses isolated fixtures.

[Copilot]

🟡 Changes recommended

Windows absolute registered paths can be misclassified as unused, allowing deletion of an active configuration.

2 open findings

• Handle Windows absolute paths in configuration registrations · New
• Canonicalize Windows paths when identifying used configuration files · New

1 resolved since last review

• Canonicalize config aliases before checking usage

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

An expected unlink failure can emit output that exposes paths and disrupts the redirect.

0 open findings

2 resolved since last review

• Handle Windows absolute paths in configuration registrations
• Canonicalize Windows paths when identifying used configuration files

Previously missed (1)

In code that hasn't changed since last review

Suppress unlink warning to preserve redirect and avoid path disclosure

lib/​editor.config-delete.php:128

When unlink() fails (for example because the directory is read-only or the file disappears concurrently), it emits an E_WARNING before this function returns failed. With error display enabled that output can expose the server path and prevent the subsequent Location header from being sent; the new failure test has to install an error handler specifically to hide it. Suppress this expected filesystem warning and rely on the return value for the user-facing failure message.

🧠 Review effort: Balanced

[alcatron]

Addressed the latest review-summary finding about unlink warnings. Expected unlink failure is suppressed and still returns failed for the existing user-facing message. The read-only-directory regression now throws on any unsuppressed warning and asserts empty output, while verifying the file remains intact. All 249 local tests pass (5560 assertions); manifest and whitespace checks pass. These remain isolated fixture checks, not native Windows/live-installation verification.

[alcatron]

Resolved the CHANGELOG.md conflict after #293 merged, preserving both entries. The feature code merged cleanly; all 250 local tests pass, plus manifest and whitespace checks. Validation uses isolated fixtures.

[alcatron]

Updated again after #294 merged during the previous push. Resolved the changelog/translation metadata conflicts; code merged cleanly. All 276 local tests pass on this combined revision. Manifest and whitespace checks pass; validation uses isolated fixtures.

[alcatron]

Resolved the latest CHANGELOG.md conflict after #295 merged, preserving both entries. Code merged cleanly. All 282 local tests pass (5684 assertions), including deletion/path/locking and new-map preset regressions; manifest and whitespace checks pass. Validation uses isolated fixtures.

[alcatron]

Resolved the CHANGELOG.md conflict after #297 merged, preserving both entries in numerical order. The code merged cleanly. All 284 local tests pass (5689 assertions) on the combined revision; PHP syntax, manifest and whitespace checks pass. Validation uses isolated fixtures.