chore(test): gate test-only DB short-circuit behind CACTI_TEST_BOOTSTRAP env#7128
Open
somethingwithproof wants to merge 5 commits into
Open
chore(test): gate test-only DB short-circuit behind CACTI_TEST_BOOTSTRAP env#7128somethingwithproof wants to merge 5 commits into
somethingwithproof wants to merge 5 commits into
Conversation
…RAP env
The previous `!defined('PHP_TESTING')` guard skipped db_connect_real
whenever PHP_TESTING was set anywhere in the include chain. A stray
define from a CLI tool or third-party script could disable real DB
connection logic in a deployed environment.
Switch the guard to require both `PHP_TESTING` and an explicit
`CACTI_TEST_BOOTSTRAP=1` env var, and replace the silent-no-connect
path with a `Cacti_TestDbSentinel` whose every method throws. Any
production code path that accidentally treats the sentinel as a real
DB handle will fail loudly instead of silently misbehaving.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens Cacti’s bootstrap DB connection logic in include/global.php by preventing a stray PHP_TESTING define from accidentally disabling real database connections in non-test environments. It introduces an explicit opt-in via CACTI_TEST_BOOTSTRAP=1 and replaces silent “no-connect” behavior with a sentinel object intended to fail loudly if misused as a DB handle.
Changes:
- Gate the test-only DB connection short-circuit behind both
PHP_TESTINGandCACTI_TEST_BOOTSTRAP=1. - Introduce
Cacti_TestDbSentinelwhose method calls throw to avoid silently treating a non-connection as a real handle. - Avoid applying the remote-override configuration when the “remote connection” is the sentinel.
Same as PR-A: appends 11 PHPStan ignoreErrors entries that exist on upstream develop but are not yet baselined, so this branch's CI does not regress on phpstan analyse --level 6. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The previous review fix introduced a local $is_test_bootstrap flag but left
two call sites still inlining defined('PHP_TESTING') && getenv(...). Replace
the long-form check and guard the single-poller cacti_db_version fetch so the
sentinel branch finishes initialising without invoking the DB.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The existing
!defined('PHP_TESTING')guards ininclude/global.phpskipped
db_connect_realwheneverPHP_TESTINGwas defined anywherein the include chain. A stray define from a CLI tool or third-party
script could disable real DB connection logic in a deployed
environment.
This change tightens the guard:
defined('PHP_TESTING')andgetenv('CACTI_TEST_BOOTSTRAP') === '1'before short-circuiting the DB connect calls.
new Cacti_TestDbSentinel(),a final class whose
__callthrowsRuntimeExceptionfor everyinvocation. Any code path that treats the sentinel as a real handle
fails loudly instead of silently returning the wrong value.
$remote_db_cnn_idis thesentinel.
Self-contained: only
include/global.phpchanges. No public APIshifts.