Skip to content

Relocate schema into includes/database.php - #61

Merged
cigamit merged 9 commits into
developfrom
refactor/schema-includes-database
Oct 1, 2026
Merged

cigamit merged 9 commits into
developfrom
refactor/schema-includes-database

Conversation

@TheWitness

@TheWitness TheWitness commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Relocate schema into includes/database.php and add the fleet file manifest

Aligns plugin_hmib with the fleet schema/includes convention and adds the fleet-wide manifest.json + upgrade-time file-pruning mechanism.

  • Relocated schema provisioning into includes/database.php, loaded via a top-level require_once in setup.php (behavior unchanged).
  • File manifest + upgrade pruning — new root manifest.json with tombstones (empty — this relocation moved a function, not files), expected (top-level files/directories shipping today, directories with a trailing /), and whitelist (empty). hmib_prune_files() (in setup.php, called from the version-change block of hmib_check_upgrade()) removes the dev-only tests/ tree on upgrade, refuses any path that resolves outside the plugin directory (a tampered manifest.json), warns on any file/directory it cannot remove, leaves whitelist/.git* alone, and logs — without removing — any top-level entry the manifest does not account for.
  • tests/bin/validate-manifest.php (wired into plugin-ci-workflow.yml) fails on drift between expected and the real top-level tree.
  • Tests — added PruneFilesTest (tombstone/file/dir removal, path-escape refusal, permission warnings, whitelist/.git protection, missing/malformed no-ops); sandboxed base_path in HmibLifecycleTest (beforeEach/afterEach) so its upgrade-path tests run the prune against a throwaway tree.

Validation

Full Pest suite green (33 passed) and patch-coverage passes at 100% (102/102 changed lines); manifest drift-check passes and the translation template is up to date.

Revision: hardening & fleet cleanup

Since the initial description, this PR also:

  • Prunes phpunit.xml on upgrade (alongside the dev-only tests/ tree) and leaves .md* lint configs in place — the drift check now ignores tests/, phpunit.xml, .git*, .md*, and whitelisted paths.
  • Hardens the prune against tampered manifests: refuses any tombstone whose normalized path contains a ./.. traversal segment or resolves outside the plugin directory (including via a symlinked directory), and protects a directory when a whitelisted entry lives beneath it — each with added unit coverage.
  • Renames the prune helpers to the documented naming convention: hmib_prune_files() / hmib_rmtree() (the plugin_hmib_ prefix is reserved for lifecycle/hook-registration functions).
  • Gives tests/Unit/PruneFilesTest.php the full standard GPL v2 header and aligns the Project Structure block in .github/copilot-instructions.md.

- Move hmib_setup_table() verbatim into includes/database.php, loaded once
  via a top-level require_once in setup.php.
- Measure includes/database.php and repoint the index-guard source scan at
  its new location.

Keeps the raw CREATE TABLE definitions as-is (lighter relocation).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The relocated schema conflicts with mandatory schema API, header, PHPDoc, and changelog conventions.

Review effort: Balanced
Findings: 2 Low severity

Open (2)
What changed in this PR

Relocates Host MIB schema provisioning into the fleet-standard database include while preserving behavior.

Changes:

  • Moves hmib_setup_table() into includes/database.php.
  • Loads the schema include from setup.php.
  • Updates coverage configuration and the index-guard test path.
File Description
includes/​database.php Contains relocated schema provisioning.
setup.php Loads the database include.
phpunit.xml Adds the include to coverage sources.
tests/​Security/​IndexGuardConsistencyTest.php Scans the relocated function.

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

Comment thread includes/database.php
Comment thread setup.php
Copilot and others added 8 commits September 30, 2026 13:48
Restores the warranty disclaimer and Cacti Group maintainer lines that
the abbreviated header omitted, matching setup.php and the other PHP
files in the repository.
Adds a develop-section entry for moving schema provisioning into
includes/database.php and introducing the manifest.json file-pruning
mechanism.
phpunit.xml ships in the repo for CI but is a dev-only test artifact, so it
is removed from manifest.json 'expected', pruned from installs on upgrade
(like tests/), and excluded from the manifest drift check.

The retired markdown-lint configs (.mdlrc, .md_style.rb) are deleted, and
.md* is now ignored like .git*: protected from pruning and excluded from
drift if it reappears.
The upgrade-time prune now rejects any tombstone containing '.'/'..' segments
(which could escape the plugin directory or resolve to its root) and treats
ancestors of whitelist entries as protected, so a tombstone on a parent
directory can no longer delete a whitelisted file beneath it.
The trailing '# ...' comments in the Project Structure block drifted further
right down the tree; align them all to a single column.
- Use the full license header (from the plugin's own setup.php) in
  tests/Unit/PruneFilesTest.php instead of the abbreviated copyright banner.
- Document that the manifest drift check and the upgrade-time prune also
  handle phpunit.xml and .md* files, matching the implemented behavior.
The repository naming contract reserves the plugin_<name>_ prefix for plugin
lifecycle / hook-registration functions; all other functions use the plain
<name>_ prefix. Rename the internal upgrade helpers accordingly:

  plugin_<name>_prune_files() -> <name>_prune_files()
  plugin_<name>_rmtree()      -> <name>_rmtree()

The call site, unit tests, and the copilot-instructions.md references are
updated to match. No behavioral change.
@cigamit
cigamit merged commit d88a7fa into develop Oct 1, 2026
3 checks passed
@cigamit
cigamit deleted the refactor/schema-includes-database branch October 1, 2026 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants