Skip to content

Commit 3cd158b

Browse files
fix(Dispatcher): accept devel variants when allow_development_packages is enabled #905
1 parent e81b30c commit 3cd158b

2 files changed

Lines changed: 108 additions & 2 deletions

File tree

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Dispatcher.inc

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace RESTAPI\Core;
44

55
use RESTAPI\Models\CronJob;
6+
use RESTAPI\Models\RESTAPISettings;
67
use RESTAPI\Responses\FailedDependencyError;
78
use RESTAPI\Responses\ServerError;
89
use RESTAPI\Responses\ServiceUnavailableError;
@@ -119,10 +120,18 @@ class Dispatcher {
119120
* @throws FailedDependencyError When a package requires a PHP include file that could not be found.
120121
*/
121122
private function check_required_packages(): void {
123+
# Check if the user has opted in to using development (-devel) package variants
124+
$pkg_config = RESTAPISettings::get_pkg_config();
125+
$allow_development_packages = ($pkg_config['allow_development_packages'] ?? '') === 'enabled';
126+
122127
# Loop through each required package and ensure it is present on the system.
123128
foreach ($this->required_packages as $pkg) {
124-
# Return an error if the package is not installed
125-
if (!is_pkg_installed($pkg)) {
129+
# When the development packages setting is enabled, also accept the -devel variant of the package
130+
$devel_pkg = $pkg . '-devel';
131+
$pkg_installed = is_pkg_installed($pkg) || ($allow_development_packages && is_pkg_installed($devel_pkg));
132+
133+
# Return an error if neither the package nor its -devel variant (when allowed) is installed
134+
if (!$pkg_installed) {
126135
throw new FailedDependencyError(
127136
message: "The requested action requires the '$pkg' package but it is not installed.",
128137
response_id: 'DISPATCHER_MISSING_REQUIRED_PACKAGE',

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreDispatcherTestCase.inc

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use RESTAPI\Core\Dispatcher;
66
use RESTAPI\Core\TestCase;
77
use RESTAPI\Dispatchers\WireGuardApplyDispatcher;
88
use RESTAPI\Models\Package;
9+
use RESTAPI\Models\RESTAPISettings;
910

1011
class APICoreDispatcherTestCase extends TestCase {
1112
/**
@@ -176,4 +177,100 @@ class APICoreDispatcherTestCase extends TestCase {
176177
# Remove the installed package
177178
$package->delete();
178179
}
180+
181+
/**
182+
* Checks that a Dispatcher requiring a package that is not installed throws DISPATCHER_MISSING_REQUIRED_PACKAGE
183+
* when the `allow_development_packages` setting is disabled (default).
184+
*/
185+
public function test_required_package_missing_throws_by_default(): void {
186+
# Ensure the allow_development_packages setting is disabled
187+
$settings = new RESTAPISettings();
188+
$settings->from_internal();
189+
$original = $settings->allow_development_packages->value;
190+
191+
if ($original) {
192+
$settings->allow_development_packages->value = false;
193+
$settings->update(apply: false);
194+
}
195+
196+
# A Dispatcher requiring a package that is not installed should fail
197+
$this->assert_throws_response(
198+
response_id: 'DISPATCHER_MISSING_REQUIRED_PACKAGE',
199+
code: 424,
200+
callable: function () {
201+
$dispatcher = new RequiredPackagesTestDispatcher();
202+
$dispatcher->required_packages = ['pfSense-pkg-some_package_we_dont_have'];
203+
$dispatcher->process();
204+
},
205+
);
206+
}
207+
208+
/**
209+
* Checks that when `allow_development_packages` is enabled, a Dispatcher whose required package is not installed
210+
* and whose -devel counterpart is also not installed still throws DISPATCHER_MISSING_REQUIRED_PACKAGE. Since we
211+
* cannot install arbitrary packages in tests, we verify the inverse: a package whose -devel name also doesn't
212+
* exist still fails even with the setting enabled.
213+
*/
214+
public function test_devel_variant_still_fails_when_neither_installed(): void {
215+
# Enable the allow_development_packages setting
216+
$settings = new RESTAPISettings();
217+
$settings->from_internal();
218+
$settings->allow_development_packages->value = true;
219+
$settings->update(apply: false);
220+
221+
# Even with the setting on, if neither the base package nor the -devel variant is installed, it should fail
222+
$this->assert_throws_response(
223+
response_id: 'DISPATCHER_MISSING_REQUIRED_PACKAGE',
224+
code: 424,
225+
callable: function () {
226+
$dispatcher = new RequiredPackagesTestDispatcher();
227+
$dispatcher->required_packages = ['pfSense-pkg-some_package_we_dont_have'];
228+
$dispatcher->process();
229+
},
230+
);
231+
232+
# Restore the setting to its original value
233+
$settings->allow_development_packages->value = false;
234+
$settings->update(apply: false);
235+
}
236+
237+
/**
238+
* Checks that when `allow_development_packages` is enabled, a Dispatcher whose required base package is installed
239+
* (using pfSense-pkg-RESTAPI as a proxy for the base package) still passes the required package check.
240+
*/
241+
public function test_base_package_accepted_when_devel_enabled(): void {
242+
# Enable the allow_development_packages setting
243+
$settings = new RESTAPISettings();
244+
$settings->from_internal();
245+
$settings->allow_development_packages->value = true;
246+
$settings->update(apply: false);
247+
248+
# The base package (pfSense-pkg-RESTAPI) is installed; the package check should still pass
249+
$this->assert_does_not_throw(
250+
callable: function () {
251+
$dispatcher = new RequiredPackagesTestDispatcher();
252+
$dispatcher->required_packages = ['pfSense-pkg-RESTAPI'];
253+
$dispatcher->process();
254+
},
255+
);
256+
257+
# Restore the setting to its original value
258+
$settings->allow_development_packages->value = false;
259+
$settings->update(apply: false);
260+
}
261+
}
262+
263+
/**
264+
* Defines a test-only Dispatcher used to exercise the $required_packages check without spawning a real background
265+
* process. The $required_packages property is exposed publicly so individual tests can assign the package(s) to
266+
* check, and _process() is intentionally a no-op so calling process() only runs the required package validation.
267+
*/
268+
class RequiredPackagesTestDispatcher extends Dispatcher {
269+
public array $required_packages = [];
270+
271+
/**
272+
* Overrides the default _process() with a no-op so tests can validate the package check in isolation.
273+
* @param mixed ...$arguments Unused arguments accepted to match the parent signature.
274+
*/
275+
protected function _process(mixed ...$arguments): void {}
179276
}

0 commit comments

Comments
 (0)