fix: guard api_plugin_moveup/movedown against NULL prior/next id (1.2.x backport)

[somethingwithproof]

Backport of #7157 to 1.2.x. The same two bugs exist identically in both branches.

Root cause

api_plugin_moveup() / api_plugin_movedown() rotate plugin ids via a three-step swap. At the boundary (first plugin moved up, last moved down), MAX(id) WHERE id < ? / MIN(id) WHERE id > ? returns NULL. With strict SQL modes stripped on connect, UPDATE plugin_config SET id = NULL on a NOT NULL column silently stores 0, corrupting the primary key. Subsequent calls to plugins_load_temp_table() then fail with ERROR 1062 because the id=0 row gets AUTO_INCREMENT-reassigned to the next sequence value, colliding with the row already at that id.

Fixes

• api_plugin_moveup: add !empty($prior_id) guard — moving the first plugin is a no-op.
• api_plugin_movedown: add outer !empty($id) guard (was missing) and !empty($next_id) guard — moving the last plugin is a no-op.
• plugins_load_temp_table: set NO_AUTO_VALUE_ON_ZERO for the single bulk copy INSERT, then restore the original session sql_mode, so any existing id=0 row is preserved literally.

Includes tests/Unit/PluginMoveorderBugTest.php with 12 regression tests (all pass).

[Copilot]

Pull request overview

Backports a fix to prevent plugin_config.id corruption when moving plugins up/down at ordering boundaries, and adds a defensive sql_mode tweak to preserve literal id=0 rows during temp-table population in the plugin management UI.

Changes:

• Add guards in api_plugin_moveup() / api_plugin_movedown() to avoid swapping when there is no prior/next row (boundary no-op).
• Temporarily enable NO_AUTO_VALUE_ON_ZERO during the plugin_config -> temp bulk copy, then restore the original session sql_mode.
• Add Pest unit tests that assert the presence/order of the new guards and the sql_mode save/restore around the bulk copy.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
lib/plugins.php	Adds boundary guards around the 3-step id swap in move-up/move-down to avoid NULL→0 corruption.
plugins.php	Adjusts session sql_mode during temp-table copy to preserve literal id=0 rows, then restores it.
tests/Unit/PluginMoveorderBugTest.php	Adds regression tests (source-scan invariants) asserting the guards and sql_mode save/restore behavior.

Comments suppressed due to low confidence (2)

lib/plugins.php:1021

• Same issue here: empty($id)/empty($next_id) will evaluate to true for id=0 or next_id=0, preventing reordering when an id=0 row exists (which this PR otherwise supports by preserving id=0 during temp-table copies). Use explicit comparisons against NULL/false instead of empty().

	$id = db_fetch_cell_prepared('SELECT id FROM plugin_config WHERE directory = ?', array($plugin));

	if (!empty($id)) {
		$next_id = db_fetch_cell_prepared('SELECT MIN(id) FROM plugin_config WHERE id > ?', array($id));

		/* MIN() on an empty set returns NULL; same NULL->0 corruption risk as moveup. */
		if (!empty($next_id)) {
			$temp_id = db_fetch_cell('SELECT MAX(id) FROM plugin_config') + 1;

			db_execute_prepared('UPDATE plugin_config SET id = ? WHERE id = ?', array($temp_id, $next_id));

tests/Unit/PluginMoveorderBugTest.php:113

• These movedown tests also lock in empty()-based guards ("if (!empty($id))" / "if (!empty($next_id))"), which will treat id=0 as empty and prevent reordering around a legitimate id=0 row. If the intent is only to guard against NULL (empty set) and false (no row), the assertions should be updated accordingly.

test('api_plugin_movedown has outer !empty($id) guard', function () use ($libPluginsPath) {
	$source = file_get_contents($libPluginsPath);

	$fn_pos   = strpos($source, 'function api_plugin_movedown(');
	$id_guard = strpos($source, 'if (!empty($id))', $fn_pos);

	expect($fn_pos)->not->toBeFalse();
	expect($id_guard)->not->toBeFalse('!empty($id) guard missing from api_plugin_movedown');
});

test('api_plugin_movedown has !empty($next_id) guard around the three-step swap', function () use ($libPluginsPath) {
	$source = file_get_contents($libPluginsPath);

	$fn_pos    = strpos($source, 'function api_plugin_movedown(');
	$guard_pos = strpos($source, 'if (!empty($next_id))', $fn_pos);

	expect($fn_pos)->not->toBeFalse();
	expect($guard_pos)->not->toBeFalse('!empty($next_id) guard missing from api_plugin_movedown');
});