From b5eb2a7e6826d9460c999b4ead692d9e79447f2c Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 20:47:04 -0600 Subject: [PATCH 01/32] fix: implement missing auto-escaping in Command class The intention for this wrapper class was to force escaping at a central location. This was either never implemented or was removed early in v2's development cycle. This commit implements the intended logic and closes command injection gaps at a core level. --- .../usr/local/pkg/RESTAPI/Core/Command.inc | 94 +++++- .../RESTAPI/Tests/APICoreCommandTestCase.inc | 284 +++++++++++++++++- 2 files changed, 372 insertions(+), 6 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Command.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Command.inc index 3bed26144..09bb15b23 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Command.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Command.inc @@ -33,29 +33,119 @@ class Command { */ public bool $trim_whitespace = false; + /** + * @var bool $escape Whether to automatically escape each token in the command string before execution. + * Defaults to true. Set to false only when the caller has already performed its own escaping or when + * shell features such as pipes, redirects, or glob expansion are intentionally required. + */ + public bool $escape = true; + + /** + * @var string $pipe An optional command string to pipe the output of $command into. When set, the shell + * pipeline ` | ` is constructed. The pipe command is escaped independently according to + * $escape_pipe. Use this instead of embedding `|` in the raw $command string, which would be neutralised + * by auto-escaping. + */ + public string $pipe = ''; + + /** + * @var bool $escape_pipe Whether to automatically escape the tokens of the $pipe command string. Defaults + * to true. Set to false when the pipe command contains shell features (e.g. `grep "some pattern"`) that + * must be passed verbatim. + */ + public bool $escape_pipe = true; + /** * Defines the Command object including the shell command to execute and optional modifiers. Note: By default, * the command output will redirect stderr to stdout so error message will be included in the output. * @param string $command The shell command to execute. * @param bool $trim_whitespace Remove excessive whitespace from the command output. * @param string $redirect An optional shell redirect to append to the end of the $command. + * @param bool $escape Automatically escape each token in the command string before execution. Defaults to + * true. Set to false only when the caller has already performed its own escaping or when shell features + * such as pipes, redirects, or glob expansion are intentionally required. + * @param string $pipe An optional command string to pipe the output of $command into. Escaped independently + * according to $escape_pipe. + * @param bool $escape_pipe Whether to automatically escape the tokens of the $pipe command. Defaults to true. * @return Command Returns this object containing the results of the executed command. Note: the object returned * cannot be used to initiate new commands. A new Command object should be created for any additional commands. */ - public function __construct(string $command, bool $trim_whitespace = false, string $redirect = '2>&1') { + public function __construct( + string $command, + bool $trim_whitespace = false, + string $redirect = '2>&1', + bool $escape = true, + string $pipe = '', + bool $escape_pipe = true, + ) { $this->command = $command; $this->trim_whitespace = $trim_whitespace; $this->redirect = $redirect; + $this->escape = $escape; + $this->pipe = $pipe; + $this->escape_pipe = $escape_pipe; $this->run_command(); return $this; } + /** + * Parses a raw command string into an array of argument tokens, respecting single- and double-quoted + * substrings so that spaces inside quotes are not treated as delimiters. Surrounding quotes are stripped + * from each token so that the value can be re-escaped uniformly by escape_command(). + * @param string $command The raw command string to tokenize. + * @return array An ordered list of unquoted token strings. + */ + public static function tokenize_command(string $command): array { + $tokens = []; + + # Match single-quoted strings, double-quoted strings, or unquoted non-whitespace sequences + preg_match_all("/'[^']*'|\"[^\"]*\"|[^\s]+/", $command, $matches); + + foreach ($matches[0] as $token) { + # Strip surrounding single quotes + if (str_starts_with($token, "'") && str_ends_with($token, "'")) { + $tokens[] = substr($token, 1, -1); + } + # Strip surrounding double quotes + elseif (str_starts_with($token, '"') && str_ends_with($token, '"')) { + $tokens[] = substr($token, 1, -1); + } + # Unquoted token — use as-is + else { + $tokens[] = $token; + } + } + + return $tokens; + } + + /** + * Escapes a raw command string by tokenizing it (respecting quoted substrings) and applying + * escapeshellarg() to every token before re-joining with spaces. This prevents command injection + * regardless of whether the original string contained pre-quoted or unquoted arguments. + * @param string $command The raw command string to escape. + * @return string The fully escaped command string ready for exec(). + */ + public static function escape_command(string $command): string { + $tokens = self::tokenize_command($command); + $escaped = array_map('escapeshellarg', $tokens); + return implode(' ', $escaped); + } + /** * Executes the assigned $command. The $output and $result_code properties will be set after running * this method. */ private function run_command(): void { - exec(command: "$this->command $this->redirect", output: $raw_output, result_code: $this->result_code); + $safe_command = $this->escape ? self::escape_command($this->command) : $this->command; + + # Append a pipe stage when one is specified, escaping it independently + if ($this->pipe !== '') { + $safe_pipe = $this->escape_pipe ? self::escape_command($this->pipe) : $this->pipe; + $safe_command = "$safe_command | $safe_pipe"; + } + + exec(command: "$safe_command $this->redirect", output: $raw_output, result_code: $this->result_code); $this->output = implode(PHP_EOL, $raw_output); # Normalize output's whitespace if requested diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc index 40829a406..77513a750 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc @@ -2,23 +2,299 @@ namespace RESTAPI\Tests; +require_once 'RESTAPI/autoloader.inc'; + use RESTAPI\Core\Command; use RESTAPI\Core\TestCase; class APICoreCommandTestCase extends TestCase { /** * Checks that the Command object successfully runs the requested command and returns the correct output and - * result code. + * result code. Escaping is disabled here because these commands rely on raw shell behaviour (e.g. a bare + * unrecognised token and a pre-quoted echo argument). */ - public function test_command() { + public function test_command(): void { # Run a command that doesn't exist, ensure it's output and result code are expected - $cmd = new Command('asdf'); + $cmd = new Command('asdf', escape: false); $this->assert_equals($cmd->output, 'sh: asdf: not found'); $this->assert_equals($cmd->result_code, 127); # Run an echo command, ensure it's output and result code are expected - $cmd = new Command("echo 'test'"); + $cmd = new Command("echo 'test'", escape: false); $this->assert_equals($cmd->output, 'test'); $this->assert_equals($cmd->result_code, 0); } + + /** + * Checks that escape defaults to true when not explicitly specified. + */ + public function test_escape_defaults_to_true(): void { + $cmd = new Command('echo hello'); + $this->assert_is_true($cmd->escape); + } + + /** + * Checks that escape can be explicitly disabled via named argument. + */ + public function test_escape_can_be_disabled(): void { + $cmd = new Command('echo hello', escape: false); + $this->assert_is_false($cmd->escape); + } + + /** + * Checks that a simple auto-escaped command runs successfully and returns expected output. + */ + public function test_auto_escape_basic_execution(): void { + $cmd = new Command('echo hello'); + $this->assert_equals($cmd->output, 'hello'); + $this->assert_equals($cmd->result_code, 0); + } + + /** + * Checks that a Command with escape disabled (false) allows compound shell expressions to run. + */ + public function test_escape_false_allows_raw_shell_string(): void { + $cmd = new Command('echo foo && echo bar', escape: false); + $this->assert_str_contains($cmd->output, 'foo'); + $this->assert_str_contains($cmd->output, 'bar'); + $this->assert_equals($cmd->result_code, 0); + } + + /** + * Checks that tokenize_command splits a simple unquoted command into the expected tokens. + */ + public function test_tokenize_command_simple(): void { + $tokens = Command::tokenize_command('/sbin/ifconfig em0 -group mygroup'); + $this->assert_equals($tokens, ['/sbin/ifconfig', 'em0', '-group', 'mygroup']); + } + + /** + * Checks that tokenize_command returns an empty array for an empty string. + */ + public function test_tokenize_command_empty_string(): void { + $tokens = Command::tokenize_command(''); + $this->assert_equals($tokens, []); + } + + /** + * Checks that tokenize_command preserves a space inside single-quoted substrings as one token + * and strips the surrounding quotes. + */ + public function test_tokenize_command_single_quoted_token(): void { + $tokens = Command::tokenize_command("echo 'hello world'"); + $this->assert_equals($tokens, ['echo', 'hello world']); + } + + /** + * Checks that tokenize_command preserves a space inside double-quoted substrings as one token + * and strips the surrounding quotes. + */ + public function test_tokenize_command_double_quoted_token(): void { + $tokens = Command::tokenize_command('echo "hello world"'); + $this->assert_equals($tokens, ['echo', 'hello world']); + } + + /** + * Checks that tokenize_command handles a mix of quoted and unquoted tokens in one string. + */ + public function test_tokenize_command_mixed_quoted_and_unquoted(): void { + $tokens = Command::tokenize_command('/bin/cmd --flag "value with spaces" bare'); + $this->assert_equals($tokens, ['/bin/cmd', '--flag', 'value with spaces', 'bare']); + } + + /** + * Checks that escape_command wraps each unquoted token with escapeshellarg() single-quote quoting. + */ + public function test_escape_command_wraps_tokens_in_single_quotes(): void { + $escaped = Command::escape_command('/sbin/ifconfig em0'); + $this->assert_equals($escaped, "'/sbin/ifconfig' 'em0'"); + } + + /** + * Checks that escape_command re-escapes a token that was already single-quoted in the input. + */ + public function test_escape_command_re_escapes_pre_single_quoted_token(): void { + $escaped = Command::escape_command("/sbin/ifconfig em0 -group 'mygroup'"); + $this->assert_equals($escaped, "'/sbin/ifconfig' 'em0' '-group' 'mygroup'"); + } + + /** + * Checks that escape_command re-escapes a token that was already double-quoted in the input. + */ + public function test_escape_command_re_escapes_pre_double_quoted_token(): void { + $escaped = Command::escape_command('/sbin/ifconfig em0 -group "mygroup"'); + $this->assert_equals($escaped, "'/sbin/ifconfig' 'em0' '-group' 'mygroup'"); + } + + /** + * Checks that escape_command neutralises a semicolon injection payload so it cannot act as a + * shell command separator. + */ + public function test_escape_command_neutralises_semicolon_injection(): void { + $escaped = Command::escape_command('/bin/cmd arg; rm -rf /'); + $this->assert_str_does_not_contain($escaped, ';'); + } + + /** + * Checks that escape_command neutralises $(...) command substitution syntax. + */ + public function test_escape_command_neutralises_command_substitution(): void { + $escaped = Command::escape_command('/bin/cmd $(evil)'); + $this->assert_str_does_not_contain($escaped, '$('); + } + + /** + * Checks that escape_command neutralises backtick command substitution syntax. + */ + public function test_escape_command_neutralises_backtick_substitution(): void { + $escaped = Command::escape_command('/bin/cmd `evil`'); + $this->assert_str_does_not_contain($escaped, '`'); + } + + /** + * Checks that escape_command neutralises a pipe character so it cannot chain commands. + */ + public function test_escape_command_neutralises_pipe(): void { + $escaped = Command::escape_command('/bin/cmd arg | cat /etc/passwd'); + $this->assert_str_does_not_contain($escaped, '|'); + } + + /** + * Checks that escape_command neutralises an ampersand so it cannot background or chain commands. + */ + public function test_escape_command_neutralises_ampersand(): void { + $escaped = Command::escape_command('/bin/cmd arg && evil'); + # The && must not survive as shell syntax + $this->assert_str_does_not_contain($escaped, '&&'); + } + + /** + * Checks that auto-escaping prevents injection via a malicious value interpolated into the + * command string — the injected shell code must not be executed. + */ + public function test_auto_escape_prevents_injection_via_interpolated_value(): void { + $sentinel = '/tmp/restapi_cmd_injection_test_' . getmypid(); + + # Simulate a field value that contains an injection payload + $malicious_value = "em0; touch $sentinel"; + new Command("/sbin/ifconfig $malicious_value -group testgroup"); + + $this->assert_is_false(file_exists($sentinel)); + } + + /** + * Checks that auto-escaping correctly handles a token whose inner value contains a single quote, + * producing a safely escaped shell argument rather than a broken quoting sequence. + */ + public function test_auto_escape_handles_embedded_single_quote_in_value(): void { + # escapeshellarg() should encode the apostrophe safely + $escaped = Command::escape_command("/bin/echo it's"); + $this->assert_is_true(strlen($escaped) > 0); + + # The shell must not crash and must exit 0 + $cmd = new Command("/bin/echo it's"); + $this->assert_equals($cmd->result_code, 0); + } + + /** + * Checks that auto-escaping correctly handles a token whose inner value contains a double quote. + */ + public function test_auto_escape_handles_embedded_double_quote_in_value(): void { + $cmd = new Command('/bin/echo say "hello"'); + $this->assert_equals($cmd->result_code, 0); + } + + /** + * Checks that trim_whitespace still works correctly when escape is disabled. + */ + public function test_trim_whitespace_with_escape_disabled(): void { + $cmd = new Command('printf "foo bar"', trim_whitespace: true, escape: false); + $this->assert_equals($cmd->output, 'foo bar'); + } + + /** + * Checks that a basic pipe is constructed and both commands run — output of the first feeds the second. + */ + public function test_pipe_basic(): void { + # echo three lines; grep filters to only lines containing 'keep' + $cmd = new Command( + command: 'printf "keep\nskip\nkeep2"', + escape: false, + pipe: 'grep keep', + escape_pipe: false, + ); + $this->assert_str_contains($cmd->output, 'keep'); + $this->assert_str_does_not_contain($cmd->output, 'skip'); + } + + /** + * Checks that the pipe command receives the correct output when the primary command is auto-escaped. + */ + public function test_pipe_with_escaped_primary_command(): void { + $cmd = new Command( + command: 'echo hello', + pipe: 'grep hello', + escape_pipe: false, + ); + $this->assert_str_contains($cmd->output, 'hello'); + $this->assert_equals($cmd->result_code, 0); + } + + /** + * Checks that the pipe stage itself is auto-escaped when escape_pipe is true (default), preventing + * injection through the pipe argument. + */ + public function test_pipe_escape_pipe_default_true(): void { + $sentinel = '/tmp/restapi_pipe_injection_test_' . getmypid(); + + # Attempt injection through the pipe string — if escape_pipe works, touch must not run + $cmd = new Command( + command: 'echo hello', + pipe: "grep hello; touch $sentinel", + escape_pipe: true, + ); + + $this->assert_is_false(file_exists($sentinel)); + } + + /** + * Checks that escape_pipe defaults to true when not explicitly specified. + */ + public function test_pipe_escape_pipe_defaults_to_true(): void { + $cmd = new Command('echo hello', pipe: 'grep hello'); + $this->assert_is_true($cmd->escape_pipe); + } + + /** + * Checks that escape_pipe can be set to false, allowing raw shell syntax in the pipe stage. + */ + public function test_pipe_escape_pipe_false_allows_raw_syntax(): void { + # grep with a quoted pattern only works when escape_pipe is false + $cmd = new Command( + command: 'printf "hello world\ngoodbye"', + escape: false, + pipe: 'grep "hello world"', + escape_pipe: false, + ); + $this->assert_str_contains($cmd->output, 'hello world'); + $this->assert_str_does_not_contain($cmd->output, 'goodbye'); + } + + /** + * Checks that an empty pipe string does not alter the command — no pipe is inserted. + */ + public function test_pipe_empty_string_does_not_pipe(): void { + $cmd = new Command('echo hello', pipe: ''); + $this->assert_equals($cmd->output, 'hello'); + $this->assert_equals($cmd->result_code, 0); + } + + /** + * Checks that the $pipe property is stored correctly on the object. + */ + public function test_pipe_property_stored(): void { + $cmd = new Command('echo hello', pipe: 'grep hello', escape_pipe: false); + $this->assert_equals($cmd->pipe, 'grep hello'); + $this->assert_is_false($cmd->escape_pipe); + } } From 5055f4f86bc76fff86f5a927814204d4a7f4a941 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 20:47:21 -0600 Subject: [PATCH 02/32] chore: CommandPrompt should not auto-escape --- .../files/usr/local/pkg/RESTAPI/Models/CommandPrompt.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/CommandPrompt.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/CommandPrompt.inc index 7ffe497ac..ac28dd94d 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/CommandPrompt.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/CommandPrompt.inc @@ -43,7 +43,7 @@ class CommandPrompt extends Model { * Execute the requested command and populate the output and result code. */ public function _create(): void { - $cmd = new Command($this->command->value); + $cmd = new Command($this->command->value, escape: false); $this->output->value = $cmd->output; $this->result_code->value = $cmd->result_code; } From 0a4eb7d19b5cf33a1010559d402105516c3099ce Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 20:48:24 -0600 Subject: [PATCH 03/32] chore: replace outlier exec calls with Command calls --- .../usr/local/pkg/RESTAPI/Models/NetworkInterface.inc | 8 +++++++- .../files/usr/local/pkg/RESTAPI/Models/RESTAPIVersion.inc | 5 ++++- .../files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc | 6 +++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/NetworkInterface.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/NetworkInterface.inc index 9c43f4df7..2ecb0a678 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/NetworkInterface.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/NetworkInterface.inc @@ -2,6 +2,7 @@ namespace RESTAPI\Models; +use RESTAPI\Core\Command; use RESTAPI\Core\Model; use RESTAPI\Dispatchers\InterfaceApplyDispatcher; use RESTAPI\Fields\BooleanField; @@ -894,7 +895,12 @@ class NetworkInterface extends Model { $if = $this->if->value; # Run ifconfig to determine what media and media options are supported - exec("/sbin/ifconfig -m $if | grep \"media \"", $media_options_output); + $ifconfig_cmd = new Command( + command: "/sbin/ifconfig -m $if", + pipe: 'grep "media "', + escape_pipe: false, + ); + $media_options_output = explode(PHP_EOL, $ifconfig_cmd->output); # Loop through each supported media from the ifconfig output foreach ($media_options_output as $media_option) { diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIVersion.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIVersion.inc index 97438805a..c2e1db46d 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIVersion.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIVersion.inc @@ -119,7 +119,10 @@ class RESTAPIVersion extends Model { */ public static function get_api_version(): string { # Pull the raw pkg info for the API package into an array for each line - $pkg_info = explode(PHP_EOL, shell_exec('pkg-static info pfSense-pkg-RESTAPI')); + $pkg_info = explode( + PHP_EOL, + (new Command('pkg-static info pfSense-pkg-RESTAPI'))->output + ); # Loop through each line and check the version foreach ($pkg_info as $pkg_line) { diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc index 676f1a4d7..ac339cebd 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc @@ -217,7 +217,11 @@ class SystemStatus extends Model { */ private function get_bios_info(string $bios_field): string { # Run the kenv command to obtain the requested BIOS information - $kenv = new Command(command: "/bin/kenv -q smbios.bios.$bios_field 2>/dev/null", trim_whitespace: true); + $kenv = new Command( + command: "/bin/kenv -q smbios.bios.$bios_field", + trim_whitespace: true, + redirect: "2>/dev/null" + ); return $kenv->result_code === 0 ? $kenv->output : ''; } From 06d46d9dbe74fecd9d8e2ac40327470bcc100f83 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 20:49:30 -0600 Subject: [PATCH 04/32] fix: enforce character validation in FilterNameValidator --- ...IValidatorsFilterNameValidatorTestCase.inc | 161 ++++++++++++++++-- .../Validators/FilterNameValidator.inc | 9 + 2 files changed, 159 insertions(+), 11 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIValidatorsFilterNameValidatorTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIValidatorsFilterNameValidatorTestCase.inc index f244458e7..21f555d6e 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIValidatorsFilterNameValidatorTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIValidatorsFilterNameValidatorTestCase.inc @@ -2,20 +2,130 @@ namespace RESTAPI\Tests; +require_once 'RESTAPI/autoloader.inc'; + use RESTAPI\Core\TestCase; use RESTAPI\Validators\FilterNameValidator; class APIValidatorsFilterNameValidatorTestCase extends TestCase { + /** + * Checks that a valid filter name consisting only of A-Z, a-z, 0-9, and _ passes validation. + */ + public function test_valid_name_passes(): void { + $this->assert_does_not_throw(callable: function () { + $v = new FilterNameValidator(); + $v->validate('valid_Name123'); + }); + } + + /** + * Checks that a name containing a hyphen is rejected as invalid. + */ + public function test_rejects_hyphen(): void { + $this->assert_throws_response( + response_id: 'FILTER_NAME_VALIDATOR_INVALID_CHARACTERS', + code: 400, + callable: function () { + $v = new FilterNameValidator(); + $v->validate('bad-name'); + }, + ); + } + + /** + * Checks that a name containing a space is rejected as invalid. + */ + public function test_rejects_space(): void { + $this->assert_throws_response( + response_id: 'FILTER_NAME_VALIDATOR_INVALID_CHARACTERS', + code: 400, + callable: function () { + $v = new FilterNameValidator(); + $v->validate('bad name'); + }, + ); + } + + /** + * Checks that a name containing a dot is rejected as invalid. + */ + public function test_rejects_dot(): void { + $this->assert_throws_response( + response_id: 'FILTER_NAME_VALIDATOR_INVALID_CHARACTERS', + code: 400, + callable: function () { + $v = new FilterNameValidator(); + $v->validate('bad.name'); + }, + ); + } + + /** + * Checks that a name containing a slash is rejected as invalid. + */ + public function test_rejects_slash(): void { + $this->assert_throws_response( + response_id: 'FILTER_NAME_VALIDATOR_INVALID_CHARACTERS', + code: 400, + callable: function () { + $v = new FilterNameValidator(); + $v->validate('bad/name'); + }, + ); + } + + /** + * Checks that a name containing a shell metacharacter (semicolon) is rejected as invalid. + */ + public function test_rejects_semicolon(): void { + $this->assert_throws_response( + response_id: 'FILTER_NAME_VALIDATOR_INVALID_CHARACTERS', + code: 400, + callable: function () { + $v = new FilterNameValidator(); + $v->validate('bad;name'); + }, + ); + } + + /** + * Checks that a name containing an at-sign is rejected as invalid. + */ + public function test_rejects_at_sign(): void { + $this->assert_throws_response( + response_id: 'FILTER_NAME_VALIDATOR_INVALID_CHARACTERS', + code: 400, + callable: function () { + $v = new FilterNameValidator(); + $v->validate('bad@name'); + }, + ); + } + + /** + * Checks that an empty string is rejected as invalid (no allowed characters present). + */ + public function test_rejects_empty_string(): void { + $this->assert_throws_response( + response_id: 'FILTER_NAME_VALIDATOR_INVALID_CHARACTERS', + code: 400, + callable: function () { + $v = new FilterNameValidator(); + $v->validate(''); + }, + ); + } + /** * Checks that system reserved names are not allowed as filter names. */ - public function test_cannot_use_reserved_names() { + public function test_cannot_use_reserved_names(): void { $this->assert_throws_response( response_id: 'FILTER_NAME_VALIDATOR_RESERVED_NAME_USED', code: 400, callable: function () { - $test_validator = new FilterNameValidator(); - $test_validator->validate('sshguard'); + $v = new FilterNameValidator(); + $v->validate('sshguard'); }, ); } @@ -23,30 +133,59 @@ class APIValidatorsFilterNameValidatorTestCase extends TestCase { /** * Checks that filter names cannot begin with `pkg_`. */ - public function test_cannot_start_with_pkg() { + public function test_cannot_start_with_pkg(): void { $this->assert_throws_response( response_id: 'FILTER_NAME_VALIDATOR_CANNOT_START_WITH_PKG', code: 400, callable: function () { - $test_validator = new FilterNameValidator(); - $test_validator->validate('pkg_test'); + $v = new FilterNameValidator(); + $v->validate('pkg_test'); }, ); } /** - * Checks that filter names cannot be entirely numeric. + * Checks that a filter name that matches an existing network interface is rejected. */ - public function test_cannot_be_pfsense_interface_id() { + public function test_cannot_be_pfsense_interface_id(): void { $this->assert_throws_response( response_id: 'FILTER_NAME_VALIDATOR_NAME_IN_USE_BY_INTERFACE', code: 400, callable: function () { - $test_validator = new FilterNameValidator(); - $test_validator->validate('wan'); + $v = new FilterNameValidator(); + $v->validate('wan'); }, ); } - # TODO: Add test to ensure filter names cannot be existing ifgroup names + /** + * Checks that a filter name that is entirely numerical is rejected. + */ + public function test_cannot_be_entirely_numerical(): void { + $this->assert_throws_response( + response_id: 'FILTER_NAME_VALIDATOR_CANNOT_BE_NUMERICAL', + code: 400, + callable: function () { + $v = new FilterNameValidator(); + $v->validate('12345'); + }, + ); + } + + /** + * Checks that the invalid-characters check fires before the reserved-name check, ensuring + * character validation is the outermost guard. + */ + public function test_invalid_characters_checked_before_reserved_names(): void { + $this->assert_throws_response( + response_id: 'FILTER_NAME_VALIDATOR_INVALID_CHARACTERS', + code: 400, + callable: function () { + # 'ssh guard' (with a space) would also be a reserved name pattern, but the + # invalid-character error must be raised first. + $v = new FilterNameValidator(); + $v->validate('ssh guard'); + }, + ); + } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Validators/FilterNameValidator.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Validators/FilterNameValidator.inc index 24d55a173..3ceedd0a2 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Validators/FilterNameValidator.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Validators/FilterNameValidator.inc @@ -24,6 +24,15 @@ class FilterNameValidator extends Validator { public function validate(mixed $value, string $field_name = ''): void { $reserved_names = get_pf_reserved(); + # Throw an exception if the name contains characters outside A-Z, a-z, 0-9, and _ + if (!preg_match('/^[A-Za-z0-9_]+$/', $value)) { + throw new ValidationError( + message: "Field '$field_name' cannot be '$value' because it contains invalid characters. " . + 'Only A-Z, a-z, 0-9, and _ are allowed.', + response_id: 'FILTER_NAME_VALIDATOR_INVALID_CHARACTERS', + ); + } + # Throw an exception if this name is reserved if (in_array($value, $reserved_names)) { throw new ValidationError( From e5221c7fe91611a77837bc5fdbabf1bb5eabf8ff Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 20:53:55 -0600 Subject: [PATCH 05/32] fix: add sensitive flag to several model fields Some fields were not marked as sensitive during the initial implementation of the sensitive flag. This commit adds the flag to applicable fields. --- .../pkg/RESTAPI/Models/OpenVPNClient.inc | 29 ++++++++++--------- .../usr/local/pkg/RESTAPI/Models/User.inc | 7 +++-- .../pkg/RESTAPI/Models/WireGuardPeer.inc | 5 ++-- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNClient.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNClient.inc index b3590536f..c1f0e44b3 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNClient.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNClient.inc @@ -205,16 +205,17 @@ class OpenVPNClient extends Model { $this->auth_pass = new StringField( default: null, allow_null: true, - conditions: ['!auth_user' => null], + sensitive: true, verbose_name: 'Auth Pass', + conditions: ['!auth_user' => null], help_text: 'The password used to authenticate with the OpenVPN server.', ); $this->auth_retry_none = new BooleanField( default: false, indicates_true: 'yes', indicates_false: '', - internal_name: 'auth-retry-none', verbose_name: 'Auth Retry None', + internal_name: 'auth-retry-none', help_text: 'Disables retrying authentication if an authentication failed error is received from the server', ); $this->tls = new Base64Field( @@ -227,8 +228,8 @@ class OpenVPNClient extends Model { $this->tls_type = new StringField( required: true, choices: ['auth', 'crypt'], - conditions: ['!tls' => null], verbose_name: 'TLS Type', + conditions: ['!tls' => null], help_text: 'The TLS key usage type. In `auth` mode, the TLS key is used only as HMAC authentication for ' . 'the control channel, protecting the peers from unauthorized connections. The `crypt` mode encrypts ' . 'the control channel communication in addition to providing authentication, providing more privacy ' . @@ -237,8 +238,8 @@ class OpenVPNClient extends Model { $this->tlsauth_keydir = new StringField( default: 'default', choices: ['default', '0', '1', '2'], - conditions: ['!tls' => null], verbose_name: 'Tlsauth Keydir', + conditions: ['!tls' => null], help_text: 'The TLS key direction. This must be set to complementary values on the client and client. ' . 'For example, if the client is set to 0, the client must be set to 1. Both may be set to omit the ' . 'direction, in which case the TLS Key will be used bidirectionally.', @@ -288,23 +289,23 @@ class OpenVPNClient extends Model { $this->tunnel_network = new StringField( default: '', allow_empty: true, - validators: [new SubnetValidator(allow_ipv4: true, allow_ipv6: false, allow_alias: false)], verbose_name: 'Tunnel Network', + validators: [new SubnetValidator(allow_ipv4: true, allow_ipv6: false, allow_alias: false)], help_text: 'The IPv4 virtual network used for private communications between this client and client hosts.', ); $this->tunnel_networkv6 = new StringField( default: '', allow_empty: true, - validators: [new SubnetValidator(allow_ipv4: false, allow_ipv6: true, allow_alias: false)], verbose_name: 'IPv6 Tunnel Network', + validators: [new SubnetValidator(allow_ipv4: false, allow_ipv6: true, allow_alias: false)], help_text: 'The IPv6 virtual network used for private communications between this client and client hosts.', ); $this->remote_network = new StringField( default: [], allow_empty: true, many: true, - validators: [new SubnetValidator(allow_ipv4: true, allow_ipv6: false, allow_alias: true)], verbose_name: 'Remote Network', + validators: [new SubnetValidator(allow_ipv4: true, allow_ipv6: false, allow_alias: true)], help_text: 'IPv4 networks that will be routed through the tunnel, so that a site-to-site VPN can be ' . 'established without manually changing the routing tables. Expressed as a list of ' . 'one or more CIDR ranges or host/network type aliases. If this is a site-to-site VPN, enter the ' . @@ -314,8 +315,8 @@ class OpenVPNClient extends Model { default: [], allow_empty: true, many: true, - validators: [new SubnetValidator(allow_ipv4: false, allow_ipv6: true, allow_alias: true)], verbose_name: 'IPv6 Remote Network', + validators: [new SubnetValidator(allow_ipv4: false, allow_ipv6: true, allow_alias: true)], help_text: 'IPv6 networks that will be routed through the tunnel, so that a site-to-site VPN can be ' . 'established without manually changing the routing tables. Expressed as a list of ' . 'one or more CIDR ranges or host/network type aliases. If this is a site-to-site VPN, enter the ' . @@ -340,8 +341,8 @@ class OpenVPNClient extends Model { $this->topology = new StringField( default: 'subnet', choices: ['subnet', 'net30'], - conditions: ['dev_mode' => 'tun'], verbose_name: 'Topology', + conditions: ['dev_mode' => 'tun'], help_text: 'The method used to supply a virtual adapter IP address to clients when using TUN mode on IPv4.', ); $this->passtos = new BooleanField( @@ -386,34 +387,34 @@ class OpenVPNClient extends Model { ); $this->keepalive_interval = new IntegerField( default: 10, - conditions: ['ping_method' => 'keepalive'], verbose_name: 'Keepalive Interval', + conditions: ['ping_method' => 'keepalive'], help_text: 'The keepalive interval parameter.', ); $this->keepalive_timeout = new IntegerField( default: 60, - conditions: ['ping_method' => 'keepalive'], verbose_name: 'Keepalive Timeout', + conditions: ['ping_method' => 'keepalive'], help_text: 'The keepalive timeout parameter.', ); $this->ping_seconds = new IntegerField( default: 10, - conditions: ['ping_method' => 'ping'], verbose_name: 'Ping Seconds', + conditions: ['ping_method' => 'ping'], help_text: 'The number of seconds to accept no packets before sending a ping to the ' . 'remote peer over the TCP/UDP control channel.', ); $this->ping_action = new StringField( default: 'ping_restart', choices: ['ping_restart', 'ping_exit'], - conditions: ['ping_method' => 'ping'], verbose_name: 'Ping Action', + conditions: ['ping_method' => 'ping'], help_text: 'The action to take after a ping to the remote peer times-out.', ); $this->ping_action_seconds = new IntegerField( default: 60, - conditions: ['ping_method' => 'ping'], verbose_name: 'Ping Action Seconds', + conditions: ['ping_method' => 'ping'], help_text: 'The number of seconds that must elapse before the ping is considered a timeout and the ' . 'configured `ping_action` is performed.', ); diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/User.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/User.inc index 4a308f740..51ab37d38 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/User.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/User.inc @@ -41,17 +41,17 @@ class User extends Model { required: true, unique: true, maximum_length: 32, + verbose_name: 'Name', validators: [ new RegexValidator(pattern: "/^[a-zA-Z0-9\.\-_]+$/", error_msg: 'Value contains invalid characters.'), ], - verbose_name: 'Name', help_text: 'The username of this local user.', ); $this->password = new StringField( required: true, sensitive: true, - internal_name: $this->get_config('system/webgui/pwhash', 'bcrypt') . '-hash', verbose_name: 'Password', + internal_name: $this->get_config('system/webgui/pwhash', 'bcrypt') . '-hash', help_text: 'The password of this local user.', ); $this->uid = new IntegerField( @@ -115,10 +115,11 @@ class User extends Model { $this->ipsecpsk = new StringField( default: '', allow_empty: true, + sensitive: true, + verbose_name: 'IPsec Pre-Shared Key', validators: [ new RegexValidator(pattern: "/^[[:ascii:]]*$/", error_msg: 'Value contains invalid characters.'), ], - verbose_name: 'IPsec Pre-Shared Key', help_text: 'The IPsec pre-shared key to assign this user.', ); parent::__construct($id, $parent_id, $data, ...$options); diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/WireGuardPeer.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/WireGuardPeer.inc index f983e1467..2db0e4e81 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/WireGuardPeer.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/WireGuardPeer.inc @@ -55,14 +55,14 @@ class WireGuardPeer extends Model { $this->endpoint = new StringField( default: null, allow_null: true, - validators: [new IPAddressValidator(allow_ipv4: true, allow_ipv6: true, allow_fqdn: true)], verbose_name: 'Endpoint', + validators: [new IPAddressValidator(allow_ipv4: true, allow_ipv6: true, allow_fqdn: true)], help_text: 'The IP address or hostname of the remote peer. Set to `null` to make this a dynamic endpoint.', ); $this->port = new PortField( default: '51820', - conditions: ['!endpoint' => null], verbose_name: 'Port', + conditions: ['!endpoint' => null], help_text: 'The port used by the remote peer.', ); $this->descr = new StringField( @@ -89,6 +89,7 @@ class WireGuardPeer extends Model { allow_empty: true, allow_null: true, write_only: true, + sensitive: true, verbose_name: 'Pre-Shared Key', help_text: 'The pre-shared key for this tunnel.', ); From b812d560a3ed5fc370c857acf62de5ef5b4eb67f Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 20:54:25 -0600 Subject: [PATCH 06/32] style: run prettier on changed files --- .../pkg/RESTAPI/Models/NetworkInterface.inc | 6 +----- .../pkg/RESTAPI/Models/RESTAPIVersion.inc | 5 +---- .../local/pkg/RESTAPI/Models/SystemStatus.inc | 2 +- .../RESTAPI/Tests/APICoreCommandTestCase.inc | 19 +++---------------- ...IValidatorsFilterNameValidatorTestCase.inc | 10 ++++++---- 5 files changed, 12 insertions(+), 30 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/NetworkInterface.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/NetworkInterface.inc index 2ecb0a678..a9bc9b70f 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/NetworkInterface.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/NetworkInterface.inc @@ -895,11 +895,7 @@ class NetworkInterface extends Model { $if = $this->if->value; # Run ifconfig to determine what media and media options are supported - $ifconfig_cmd = new Command( - command: "/sbin/ifconfig -m $if", - pipe: 'grep "media "', - escape_pipe: false, - ); + $ifconfig_cmd = new Command(command: "/sbin/ifconfig -m $if", pipe: 'grep "media "', escape_pipe: false); $media_options_output = explode(PHP_EOL, $ifconfig_cmd->output); # Loop through each supported media from the ifconfig output diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIVersion.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIVersion.inc index c2e1db46d..b3934894f 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIVersion.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIVersion.inc @@ -119,10 +119,7 @@ class RESTAPIVersion extends Model { */ public static function get_api_version(): string { # Pull the raw pkg info for the API package into an array for each line - $pkg_info = explode( - PHP_EOL, - (new Command('pkg-static info pfSense-pkg-RESTAPI'))->output - ); + $pkg_info = explode(PHP_EOL, (new Command('pkg-static info pfSense-pkg-RESTAPI'))->output); # Loop through each line and check the version foreach ($pkg_info as $pkg_line) { diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc index ac339cebd..04ee603e0 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc @@ -220,7 +220,7 @@ class SystemStatus extends Model { $kenv = new Command( command: "/bin/kenv -q smbios.bios.$bios_field", trim_whitespace: true, - redirect: "2>/dev/null" + redirect: '2>/dev/null', ); return $kenv->result_code === 0 ? $kenv->output : ''; } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc index 77513a750..5c71d6fb2 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc @@ -217,12 +217,7 @@ class APICoreCommandTestCase extends TestCase { */ public function test_pipe_basic(): void { # echo three lines; grep filters to only lines containing 'keep' - $cmd = new Command( - command: 'printf "keep\nskip\nkeep2"', - escape: false, - pipe: 'grep keep', - escape_pipe: false, - ); + $cmd = new Command(command: 'printf "keep\nskip\nkeep2"', escape: false, pipe: 'grep keep', escape_pipe: false); $this->assert_str_contains($cmd->output, 'keep'); $this->assert_str_does_not_contain($cmd->output, 'skip'); } @@ -231,11 +226,7 @@ class APICoreCommandTestCase extends TestCase { * Checks that the pipe command receives the correct output when the primary command is auto-escaped. */ public function test_pipe_with_escaped_primary_command(): void { - $cmd = new Command( - command: 'echo hello', - pipe: 'grep hello', - escape_pipe: false, - ); + $cmd = new Command(command: 'echo hello', pipe: 'grep hello', escape_pipe: false); $this->assert_str_contains($cmd->output, 'hello'); $this->assert_equals($cmd->result_code, 0); } @@ -248,11 +239,7 @@ class APICoreCommandTestCase extends TestCase { $sentinel = '/tmp/restapi_pipe_injection_test_' . getmypid(); # Attempt injection through the pipe string — if escape_pipe works, touch must not run - $cmd = new Command( - command: 'echo hello', - pipe: "grep hello; touch $sentinel", - escape_pipe: true, - ); + $cmd = new Command(command: 'echo hello', pipe: "grep hello; touch $sentinel", escape_pipe: true); $this->assert_is_false(file_exists($sentinel)); } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIValidatorsFilterNameValidatorTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIValidatorsFilterNameValidatorTestCase.inc index 21f555d6e..68de5f2b0 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIValidatorsFilterNameValidatorTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIValidatorsFilterNameValidatorTestCase.inc @@ -12,10 +12,12 @@ class APIValidatorsFilterNameValidatorTestCase extends TestCase { * Checks that a valid filter name consisting only of A-Z, a-z, 0-9, and _ passes validation. */ public function test_valid_name_passes(): void { - $this->assert_does_not_throw(callable: function () { - $v = new FilterNameValidator(); - $v->validate('valid_Name123'); - }); + $this->assert_does_not_throw( + callable: function () { + $v = new FilterNameValidator(); + $v->validate('valid_Name123'); + }, + ); } /** From e3e039abd992585a77042f24fb75d4793daa8ce4 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 20:56:40 -0600 Subject: [PATCH 07/32] chore: don't make presharedkey write only The field is marked as sensitive so it is not necessary. Admins can override the sensitive flag if they want to accept the risk of exposing it through the API. --- .../files/usr/local/pkg/RESTAPI/Models/WireGuardPeer.inc | 1 - 1 file changed, 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/WireGuardPeer.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/WireGuardPeer.inc index 2db0e4e81..ecd73eb60 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/WireGuardPeer.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/WireGuardPeer.inc @@ -88,7 +88,6 @@ class WireGuardPeer extends Model { default: '', allow_empty: true, allow_null: true, - write_only: true, sensitive: true, verbose_name: 'Pre-Shared Key', help_text: 'The pre-shared key for this tunnel.', From 787b5efe7d01c450df00e111b7983fc1f5d444ca Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 21:09:13 -0600 Subject: [PATCH 08/32] test: replace usage of shell_exec in tests with Command --- .../Tests/APIModelsFirewallAliasTestCase.inc | 3 ++- .../Tests/APIModelsInterfaceVLANTestCase.inc | 21 ++++++++++--------- .../APIModelsNetworkInterfaceTestCase.inc | 17 ++++++++------- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc index 94a984058..c98727b69 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc @@ -2,6 +2,7 @@ namespace RESTAPI\Tests; +use RESTAPI\Core\Command; use RESTAPI\Core\TestCase; use RESTAPI\Core\TestCaseRetry; use RESTAPI\Models\FirewallAlias; @@ -25,7 +26,7 @@ class APIModelsFirewallAliasTestCase extends TestCase { # Wait up to 30 seconds for the filter to reload and the table to be create foreach (range(0, 30) as $attempt) { # Check pfctl for the table - $pfctl_output = shell_exec('pfctl -t TEST_GOOGLE_DNS -Ts'); + $pfctl_output = (new Command('pfctl -t TEST_GOOGLE_DNS -Ts'))->output; if ($pfctl_output) { break; } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsInterfaceVLANTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsInterfaceVLANTestCase.inc index da9b25dde..92043bbcb 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsInterfaceVLANTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsInterfaceVLANTestCase.inc @@ -2,6 +2,7 @@ namespace RESTAPI\Tests; +use RESTAPI\Core\Command; use RESTAPI\Core\TestCase; use RESTAPI\Models\InterfaceVLAN; @@ -21,9 +22,9 @@ class APIModelsInterfaceVLANTestCase extends TestCase { $test_vlan->create(); # Ensure the newly created VLAN is correctly configuring by checking ifconfig - $ifconfig_output = shell_exec("ifconfig {$this->env['PFREST_LAN_IF']}.2"); - $this->assert_str_contains($ifconfig_output, 'vlan: 2'); - $this->assert_str_contains($ifconfig_output, 'vlanpcp: 4'); + $ifconfig_cmd = new Command("ifconfig {$this->env['PFREST_LAN_IF']}.2"); + $this->assert_str_contains($ifconfig_cmd->output, 'vlan: 2'); + $this->assert_str_contains($ifconfig_cmd->output, 'vlanpcp: 4'); # Update the VLAN interface to change the tag and pcp values $test_vlan->tag->value = 55; @@ -31,20 +32,20 @@ class APIModelsInterfaceVLANTestCase extends TestCase { $test_vlan->update(); # Ensure the old VLAN is no longer present - $ifconfig_output = shell_exec("ifconfig {$this->env['PFREST_LAN_IF']}.2"); - $this->assert_equals($ifconfig_output, null); + $ifconfig_cmd = new Command("ifconfig {$this->env['PFREST_LAN_IF']}.2"); + $this->assert_not_equals($ifconfig_cmd->result_code, 0); # Ensure the new VLAN is present - $ifconfig_output = shell_exec("ifconfig {$this->env['PFREST_LAN_IF']}.55"); - $this->assert_str_contains($ifconfig_output, 'vlan: 55'); - $this->assert_str_contains($ifconfig_output, 'vlanpcp: 7'); + $ifconfig_cmd = new Command("ifconfig {$this->env['PFREST_LAN_IF']}.55"); + $this->assert_str_contains($ifconfig_cmd->output, 'vlan: 55'); + $this->assert_str_contains($ifconfig_cmd->output, 'vlanpcp: 7'); # Delete the VLAN interface $test_vlan->delete(); # Ensure the VLAN is no longer present - $ifconfig_output = shell_exec("ifconfig {$this->env['PFREST_LAN_IF']}.55"); - $this->assert_equals($ifconfig_output, null); + $ifconfig_cmd = new Command("ifconfig {$this->env['PFREST_LAN_IF']}.55"); + $this->assert_not_equals($ifconfig_cmd->result_code, 0); } /** diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsNetworkInterfaceTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsNetworkInterfaceTestCase.inc index 6e162f1e3..ea6bf6c79 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsNetworkInterfaceTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsNetworkInterfaceTestCase.inc @@ -2,6 +2,7 @@ namespace RESTAPI\Tests; +use RESTAPI\Core\Command; use RESTAPI\Core\Model; use RESTAPI\Core\TestCase; use RESTAPI\Models\DHCPServer; @@ -678,7 +679,12 @@ class APIModelsNetworkInterfaceTestCase extends TestCase { $test_model->if->value = $this->env['PFREST_WAN_IF']; # Run ifconfig to gather interface details - exec("/sbin/ifconfig -m {$test_model->if->value} | grep \"media \"", $ifconfig_output); + $ifconfig_cmd = new Command( + command: "/sbin/ifconfig -m {$test_model->if->value}", + pipe: 'grep "media "', + escape_pipe: false, + ); + $ifconfig_output = explode(PHP_EOL, $ifconfig_cmd->output); # Loop through each line in the ifconfig output and ensure it is found in the `get_supported_media()` output foreach ($ifconfig_output as $media_line) { @@ -803,8 +809,7 @@ class APIModelsNetworkInterfaceTestCase extends TestCase { $static_if->create(apply: true); # Run ifconfig to check the configured interface's values - exec("ifconfig {$this->env['PFREST_OPT1_IF']}", $ifconfig_output); - $ifconfig_output = implode(PHP_EOL, $ifconfig_output); + $ifconfig_output = (new Command("ifconfig {$this->env['PFREST_OPT1_IF']}"))->output; # Ensure the interface's mtu is shown by ifconfig $this->assert_str_contains($ifconfig_output, 'mtu 1501'); @@ -832,8 +837,7 @@ class APIModelsNetworkInterfaceTestCase extends TestCase { $static_if->update(apply: true); # Run ifconfig to check the configured interface's new values - exec("ifconfig {$this->env['PFREST_OPT1_IF']}", $ifconfig_output); - $ifconfig_output = implode(PHP_EOL, $ifconfig_output); + $ifconfig_output = (new Command("ifconfig {$this->env['PFREST_OPT1_IF']}"))->output; # Ensure the interface's new mtu is shown by ifconfig $this->assert_str_contains($ifconfig_output, 'mtu 1500'); @@ -850,8 +854,7 @@ class APIModelsNetworkInterfaceTestCase extends TestCase { $static_if->delete(); # Run ifconfig to check the configured interface's deleted values - exec("ifconfig {$this->env['PFREST_OPT1_IF']}", $ifconfig_output); - $ifconfig_output = implode(PHP_EOL, $ifconfig_output); + $ifconfig_output = (new Command("ifconfig {$this->env['PFREST_OPT1_IF']}"))->output; # Ensure the new static IPv4 address is not shown by ifconfig $this->assert_str_does_not_contain( From ca05fa95f16eb10a748e77e3953700c7e399c235 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 21:16:24 -0600 Subject: [PATCH 09/32] chore: replace redundant TestCase::run_command method with Command calls --- .../usr/local/pkg/RESTAPI/Core/TestCase.inc | 20 ------------------- .../Tests/APIModelsRoutingGatewayTestCase.inc | 7 ++++--- .../Tests/APIModelsStaticRouteTestCase.inc | 9 +++++---- 3 files changed, 9 insertions(+), 27 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/TestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/TestCase.inc index e87a2ac76..30b13e5a7 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/TestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/TestCase.inc @@ -130,26 +130,6 @@ class TestCase { } } - /** - * Runs a shell command and returns its output and return code. - * @param string $command The command to execute. - * @param bool $trim_whitespace Remove excess whitespace from the command output. This is sometimes helpful when - * the output of commands that do not have consistent whitespace formatting. - * @return array An array where the `output` key contains the commands output and the `result_code` key contains the - * resulting result code of the command. - */ - function run_command(string $command, bool $trim_whitespace = false): array { - $results = ['output' => null, 'code' => null]; - exec(command: "$command 2>/dev/null", output: $results['output'], result_code: $results['result_code']); - $results['output'] = implode(PHP_EOL, $results['output']); - - # Normalize output's whitespace if requested - if ($trim_whitespace) { - $results['output'] = preg_replace('/\s+/', ' ', $results['output']); - } - - return $results; - } /** * Sets up the test case before tests are run. This can be overridden by your TestCase to setup shared resources diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsRoutingGatewayTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsRoutingGatewayTestCase.inc index 68710f93a..abfd5a77d 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsRoutingGatewayTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsRoutingGatewayTestCase.inc @@ -2,6 +2,7 @@ namespace RESTAPI\Tests; +use RESTAPI\Core\Command; use RESTAPI\Core\TestCase; use RESTAPI\Models\NetworkInterface; use RESTAPI\Models\RoutingGateway; @@ -400,19 +401,19 @@ class APIModelsRoutingGatewayTestCase extends TestCase { $test_gw->create(apply: true); # Ensure it is now found in the routing table - $netstat_output = $this->run_command('netstat -rn')['output']; + $netstat_output = (new Command('netstat -rn'))->output; $this->assert_str_contains($netstat_output, '1.2.3.4'); # Update the RoutingGateway's IP and ensure it is now present in the routing table and 1.2.3.4 is not $test_gw->gateway->value = '4.3.2.1'; $test_gw->update(apply: true); - $netstat_output = $this->run_command('netstat -rn')['output']; + $netstat_output = (new Command('netstat -rn'))->output; $this->assert_str_does_not_contain($netstat_output, '1.2.3.4'); $this->assert_str_contains($netstat_output, '4.3.2.1'); # Delete the RoutingGateway and ensure 4.3.2.1 is no longer in the routing table $test_gw->delete(apply: true); - $netstat_output = $this->run_command('netstat -rn')['output']; + $netstat_output = (new Command('netstat -rn'))->output; $this->assert_str_does_not_contain($netstat_output, '4.3.2.1'); } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsStaticRouteTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsStaticRouteTestCase.inc index 5914d1b7b..4f8b30bda 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsStaticRouteTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsStaticRouteTestCase.inc @@ -2,6 +2,7 @@ namespace RESTAPI\Tests; +use RESTAPI\Core\Command; use RESTAPI\Core\TestCase; use RESTAPI\Models\FirewallAlias; use RESTAPI\Models\NetworkInterface; @@ -312,7 +313,7 @@ class APIModelsStaticRouteTestCase extends TestCase { # Ensure the route is found in the routing table after applying $this->assert_str_contains( - $this->run_command('netstat -rn', trim_whitespace: true)['output'], + (new Command('netstat -rn', trim_whitespace: true))->output, "{$test_route->network->value} {$test_gw->gateway->value}", ); @@ -322,11 +323,11 @@ class APIModelsStaticRouteTestCase extends TestCase { # Ensure the new route is found in the routing table and the old one is not after applying $this->assert_str_does_not_contain( - $this->run_command('netstat -rn', trim_whitespace: true)['output'], + (new Command('netstat -rn', trim_whitespace: true))->output, "1.2.3.0/24 {$test_gw->gateway->value}", ); $this->assert_str_contains( - $this->run_command('netstat -rn', trim_whitespace: true)['output'], + (new Command('netstat -rn', trim_whitespace: true))->output, "{$test_route->network->value} {$test_gw->gateway->value}", ); @@ -334,7 +335,7 @@ class APIModelsStaticRouteTestCase extends TestCase { $test_route->delete(apply: true); $this->assert_str_does_not_contain( - $this->run_command('netstat -rn', trim_whitespace: true)['output'], + (new Command('netstat -rn', trim_whitespace: true))->output, "{$test_route->network->value} {$test_gw->gateway->value}", ); From 7f1e6a9181fe04be3aeba24aaab13406bbc75093 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 21:17:42 -0600 Subject: [PATCH 10/32] chore: optimize imports for models --- .../files/usr/local/pkg/RESTAPI/Models/Certificate.inc | 2 +- .../files/usr/local/pkg/RESTAPI/Models/CertificateAuthority.inc | 1 - pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/Enum.inc | 2 -- .../files/usr/local/pkg/RESTAPI/Models/IPsecChildSAStatus.inc | 2 -- .../files/usr/local/pkg/RESTAPI/Models/PortForward.inc | 1 - .../files/usr/local/pkg/RESTAPI/Models/SystemHalt.inc | 1 - .../files/usr/local/pkg/RESTAPI/Models/SystemReboot.inc | 1 - .../files/usr/local/pkg/RESTAPI/Models/SystemUpdate.inc | 1 - 8 files changed, 1 insertion(+), 10 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/Certificate.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/Certificate.inc index 246c49487..93f5b512a 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/Certificate.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/Certificate.inc @@ -4,9 +4,9 @@ namespace RESTAPI\Models; use RESTAPI\Core\Model; use RESTAPI\Fields\Base64Field; -use RESTAPI\Fields\StringField; use RESTAPI\Fields\DateTimeField; use RESTAPI\Fields\IntegerField; +use RESTAPI\Fields\StringField; use RESTAPI\Fields\UIDField; use RESTAPI\Responses\ForbiddenError; use RESTAPI\Responses\ValidationError; diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/CertificateAuthority.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/CertificateAuthority.inc index 0436ee319..59270e336 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/CertificateAuthority.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/CertificateAuthority.inc @@ -5,7 +5,6 @@ namespace RESTAPI\Models; use RESTAPI\Core\Model; use RESTAPI\Fields\Base64Field; use RESTAPI\Fields\BooleanField; -use RESTAPI\Fields\ForeignModelField; use RESTAPI\Fields\IntegerField; use RESTAPI\Fields\StringField; use RESTAPI\Fields\UIDField; diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/Enum.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/Enum.inc index c3ac252dc..5ac47a617 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/Enum.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/Enum.inc @@ -3,9 +3,7 @@ namespace RESTAPI\Models; use RESTAPI; -use RESTAPI\Core\Command; use RESTAPI\Core\Model; -use RESTAPI\Fields\BooleanField; use RESTAPI\Fields\ForeignModelField; use RESTAPI\Fields\ObjectField; use RESTAPI\Fields\StringField; diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/IPsecChildSAStatus.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/IPsecChildSAStatus.inc index cfeaedadf..4d45ebe41 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/IPsecChildSAStatus.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/IPsecChildSAStatus.inc @@ -5,8 +5,6 @@ namespace RESTAPI\Models; use RESTAPI\Core\Model; use RESTAPI\Fields\BooleanField; use RESTAPI\Fields\IntegerField; -use RESTAPI\Fields\NestedModelField; -use RESTAPI\Fields\PortField; use RESTAPI\Fields\StringField; /** diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/PortForward.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/PortForward.inc index ea47bb3e7..d7e73f69c 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/PortForward.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/PortForward.inc @@ -15,7 +15,6 @@ use RESTAPI\Fields\SpecialNetworkField; use RESTAPI\Fields\StringField; use RESTAPI\Fields\UnixTimeField; use RESTAPI\Responses\ServerError; -use RESTAPI\Validators\IPAddressValidator; /** * Defines a Model that represents port forward rules. diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemHalt.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemHalt.inc index fae5f0681..3632700f5 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemHalt.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemHalt.inc @@ -4,7 +4,6 @@ namespace RESTAPI\Models; use RESTAPI\Core\Model; use RESTAPI\Dispatchers\SystemHaltDispatcher; -use RESTAPI\Fields\BooleanField; /** * Defines a Model that performs a system halt operation. diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemReboot.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemReboot.inc index b6cc5000b..a0e1fc783 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemReboot.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemReboot.inc @@ -4,7 +4,6 @@ namespace RESTAPI\Models; use RESTAPI\Core\Model; use RESTAPI\Dispatchers\SystemRebootDispatcher; -use RESTAPI\Fields\BooleanField; /** * Defines a Model that performs a system reboot operation. diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemUpdate.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemUpdate.inc index 8b58836b2..bce0803f4 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemUpdate.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemUpdate.inc @@ -6,7 +6,6 @@ require_once 'RESTAPI/autoloader.inc'; use RESTAPI\Core\Model; use RESTAPI\Dispatchers\SystemUpdateDispatcher; -use RESTAPI\Fields\BooleanField; /** * Defines a Model that performs a pfSense update operation. From eee85bf688100edea0f01c5b1d36e341b81fc69b Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Wed, 5 Aug 2026 21:17:57 -0600 Subject: [PATCH 11/32] style: run prettier on changed files --- .../files/usr/local/pkg/RESTAPI/Core/TestCase.inc | 1 - 1 file changed, 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/TestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/TestCase.inc index 30b13e5a7..eab81197f 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/TestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/TestCase.inc @@ -130,7 +130,6 @@ class TestCase { } } - /** * Sets up the test case before tests are run. This can be overridden by your TestCase to setup shared resources * required for your tests. From a4161a3d39b35cae234ca49ee734625f48f340d9 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 09:00:30 -0600 Subject: [PATCH 12/32] test: use redirect param instead of redirect literal --- .../pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc index c206d8a75..343203388 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc @@ -34,15 +34,15 @@ class APIModelsSystemStatusTestCase extends TestCase { $this->assert_is_less_than_or_equal($system_status->disk_usage->value, 100); $this->assert_equals( $system_status->bios_vendor->value, - (new Command(command: '/bin/kenv -q smbios.bios.vendor 2>/dev/null', trim_whitespace: true))->output, + (new Command(command: '/bin/kenv -q smbios.bios.vendor', trim_whitespace: true, redirect: '2>/dev/null'))->output, ); $this->assert_equals( $system_status->bios_version->value, - (new Command(command: '/bin/kenv -q smbios.bios.version 2>/dev/null', trim_whitespace: true))->output, + (new Command(command: '/bin/kenv -q smbios.bios.version', trim_whitespace: true, redirect: '2>/dev/null'))->output, ); $this->assert_equals( $system_status->bios_date->value, - (new Command(command: '/bin/kenv -q smbios.bios.reldate 2>/dev/null', trim_whitespace: true))->output, + (new Command(command: '/bin/kenv -q smbios.bios.reldate', trim_whitespace: true, redirect: '2>/dev/null'))->output, ); } From ecb06a74a7df8f6743c8bf6c9d785eba22a07ac8 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 09:02:05 -0600 Subject: [PATCH 13/32] test: use pipe parameter instead of pipe literal --- .../usr/local/pkg/RESTAPI/Tests/APIModelsDHCPRelayTestCase.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsDHCPRelayTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsDHCPRelayTestCase.inc index 1f723fd7b..efdb783d7 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsDHCPRelayTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsDHCPRelayTestCase.inc @@ -85,7 +85,7 @@ class APIModelsDHCPRelayTestCase extends TestCase { $dhcp_relay->update(); # Ensure the DHCP relay service is running with the correct arguments - $dhcrelay_process = new Command('ps aux | grep dhcrelay'); + $dhcrelay_process = new Command('ps aux', pipe: 'grep dhcrelay'); $lan_if = $this->env['PFREST_LAN_IF']; $wan_if = $this->env['PFREST_WAN_IF']; $this->assert_str_contains( From 52cbc733dd0b7244487ad7193ef0f0c255860c94 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 09:16:10 -0600 Subject: [PATCH 14/32] test: sanitization wraps args in single-quotes, not individual char escapes --- .../RESTAPI/Tests/APICoreCommandTestCase.inc | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc index 5c71d6fb2..8775556f3 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreCommandTestCase.inc @@ -127,45 +127,49 @@ class APICoreCommandTestCase extends TestCase { } /** - * Checks that escape_command neutralises a semicolon injection payload so it cannot act as a - * shell command separator. + * Checks that escape_command neutralizes a semicolon injection payload so it cannot act as a + * shell command separator — the semicolon is present but enclosed inside a single-quoted token. */ - public function test_escape_command_neutralises_semicolon_injection(): void { + public function test_escape_command_neutralizes_semicolon_injection(): void { $escaped = Command::escape_command('/bin/cmd arg; rm -rf /'); - $this->assert_str_does_not_contain($escaped, ';'); + # The semicolon must still be present (not stripped), but enclosed in quotes + $this->assert_str_contains($escaped, "'arg;'"); } /** - * Checks that escape_command neutralises $(...) command substitution syntax. + * Checks that escape_command neutralizes $(...) command substitution syntax — the $( sequence + * is present but enclosed inside a single-quoted token where the shell cannot evaluate it. */ - public function test_escape_command_neutralises_command_substitution(): void { + public function test_escape_command_neutralizes_command_substitution(): void { $escaped = Command::escape_command('/bin/cmd $(evil)'); - $this->assert_str_does_not_contain($escaped, '$('); + $this->assert_str_contains($escaped, "'$(evil)'"); } /** - * Checks that escape_command neutralises backtick command substitution syntax. + * Checks that escape_command neutralizes backtick command substitution syntax — the backticks + * are present but enclosed inside a single-quoted token. */ - public function test_escape_command_neutralises_backtick_substitution(): void { + public function test_escape_command_neutralizes_backtick_substitution(): void { $escaped = Command::escape_command('/bin/cmd `evil`'); - $this->assert_str_does_not_contain($escaped, '`'); + $this->assert_str_contains($escaped, "'`evil`'"); } /** - * Checks that escape_command neutralises a pipe character so it cannot chain commands. + * Checks that escape_command neutralizes a pipe character so it cannot chain commands — + * the pipe is present but enclosed inside a single-quoted token. */ - public function test_escape_command_neutralises_pipe(): void { + public function test_escape_command_neutralizes_pipe(): void { $escaped = Command::escape_command('/bin/cmd arg | cat /etc/passwd'); - $this->assert_str_does_not_contain($escaped, '|'); + $this->assert_str_contains($escaped, "'|'"); } /** - * Checks that escape_command neutralises an ampersand so it cannot background or chain commands. + * Checks that escape_command neutralizes an ampersand so it cannot background or chain commands — + * the && sequence is present but enclosed inside a single-quoted token. */ - public function test_escape_command_neutralises_ampersand(): void { + public function test_escape_command_neutralizes_ampersand(): void { $escaped = Command::escape_command('/bin/cmd arg && evil'); - # The && must not survive as shell syntax - $this->assert_str_does_not_contain($escaped, '&&'); + $this->assert_str_contains($escaped, "'&&'"); } /** From 10a308459c37f2965d041a7d22069887d47a2e02 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 09:21:41 -0600 Subject: [PATCH 15/32] test: use redirect and pipe literals for LogTraits tests --- .../Tests/APIModelTraitsLogFileModelTraitsTestCase.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelTraitsLogFileModelTraitsTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelTraitsLogFileModelTraitsTestCase.inc index 4ccdb2d3b..cb9851d34 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelTraitsLogFileModelTraitsTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelTraitsLogFileModelTraitsTestCase.inc @@ -158,8 +158,8 @@ class APIModelTraitsLogFileModelTraitsTestCase extends TestCase { # Create some mock log files that are xz compressed file_put_contents('/tmp/test_xz.log', "Line 7\nLine 8\nLine 9\n"); - new Command('printf "Line 4\nLine 5\nLine 6\n" | xz -c > /tmp/test_xz.log.0.xz'); - new Command('printf "Line 1\nLine 2\nLine 3\n" | xz -c > /tmp/test_xz.log.1.xz'); + new Command(command: 'printf "Line 4\nLine 5\nLine 6\n"', redirect: '> /tmp/test_xz.log.0.xz', pipe: 'xz -c'); + new Command(command: 'printf "Line 1\nLine 2\nLine 3\n"', redirect: '> /tmp/test_xz.log.1.xz', pipe: 'xz -c'); # Read the system log file $log = $model->read_log('/tmp/test_xz.log'); From f716a68f967d5282139d2116ee262c41ee3f8460 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 13:14:52 -0600 Subject: [PATCH 16/32] fix: derive acme results from issue log There have been out of band changes added to the acme package that are no publicly available. The issue_certificate command no longer prints the logs and the implementation broke because of this. To capture the results, this commit tracks the issueance log and extracts only the new log items as the result. --- .../ACMECertificateIssueDispatcher.inc | 20 ++++++++++++++----- .../ACMECertificateRenewDispatcher.inc | 20 ++++++++++++++----- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Dispatchers/ACMECertificateIssueDispatcher.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Dispatchers/ACMECertificateIssueDispatcher.inc index 6d137577d..c36f2936a 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Dispatchers/ACMECertificateIssueDispatcher.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Dispatchers/ACMECertificateIssueDispatcher.inc @@ -2,6 +2,7 @@ namespace RESTAPI\Dispatchers; +use RESTAPI\Core\Command; use RESTAPI\Core\Dispatcher; use RESTAPI\Responses\ServerError; @@ -37,14 +38,23 @@ class ACMECertificateIssueDispatcher extends Dispatcher { result_log: '', ); - # pfSense prints the output of the acme.sh instead of returning it, start output buffering to capture it - ob_start(); + # Record the current byte size of the acme.sh issue log before issuance so we can + # extract only the new content appended during this run. + $log_file = "/tmp/acme/{$arguments['certificate']}/acme_issuecert.log"; + $pre_size = file_exists($log_file) ? filesize($log_file) : 0; # Issue the ACME certificate \pfsense_pkg\acme\issue_certificate(id: $arguments['certificate'], force: true, renew: false); - # Log the results of the ACME certificate issue - $result = ob_get_clean(); + # Extract only the bytes appended to the log since before the issuance started. + # tail -c +N outputs from byte N onward (1-indexed), so +($pre_size+1) skips + # all pre-existing content and returns only what was written during this run. + $start_byte = $pre_size + 1; + if (file_exists($log_file) && filesize($log_file) >= $start_byte) { + $result = (new Command("/usr/bin/tail -c +$start_byte $log_file"))->output; + } else { + $result = ''; + } # Replace the issuance result file $this->set_issuance_result( @@ -55,7 +65,7 @@ class ACMECertificateIssueDispatcher extends Dispatcher { ); # Wait a bit to ensure the issue log is written before proceeding - sleep(1); + sleep(5); } /** diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Dispatchers/ACMECertificateRenewDispatcher.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Dispatchers/ACMECertificateRenewDispatcher.inc index 66ed2b98c..cabfde331 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Dispatchers/ACMECertificateRenewDispatcher.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Dispatchers/ACMECertificateRenewDispatcher.inc @@ -2,6 +2,7 @@ namespace RESTAPI\Dispatchers; +use RESTAPI\Core\Command; use RESTAPI\Core\Dispatcher; use RESTAPI\Responses\ServerError; @@ -37,14 +38,23 @@ class ACMECertificateRenewDispatcher extends Dispatcher { result_log: '', ); - # pfSense prints the output of the acme.sh instead of returning it, start output buffering to capture it - ob_start(); + # Record the current byte size of the acme issue log before renewal so we can + # extract only the new content appended during this run. + $log_file = "/tmp/acme/{$arguments['certificate']}/acme_issuecert.log"; + $pre_size = file_exists($log_file) ? filesize($log_file) : 0; - # Issue the ACME certificate + # Renew the ACME certificate \pfsense_pkg\acme\issue_certificate(id: $arguments['certificate'], force: true, renew: true); - # Log the results of the ACME certificate renewal - $result = ob_get_clean(); + # Extract only the bytes appended to the log since before the renewal started. + # tail -c +N outputs from byte N onward (1-indexed), so +($pre_size+1) skips + # all pre-existing content and returns only what was written during this run. + $start_byte = $pre_size + 1; + if (file_exists($log_file) && filesize($log_file) >= $start_byte) { + $result = (new Command("/usr/bin/tail -c +$start_byte $log_file"))->output; + } else { + $result = ''; + } # Replace the renewal result file $this->set_renewal_result( From cae11cdf763077f9d2fd3b498d5504122c76045c Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 13:15:12 -0600 Subject: [PATCH 17/32] style: run prettier on changed files --- .../pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc index 343203388..351bc5a4e 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc @@ -34,15 +34,18 @@ class APIModelsSystemStatusTestCase extends TestCase { $this->assert_is_less_than_or_equal($system_status->disk_usage->value, 100); $this->assert_equals( $system_status->bios_vendor->value, - (new Command(command: '/bin/kenv -q smbios.bios.vendor', trim_whitespace: true, redirect: '2>/dev/null'))->output, + (new Command(command: '/bin/kenv -q smbios.bios.vendor', trim_whitespace: true, redirect: '2>/dev/null')) + ->output, ); $this->assert_equals( $system_status->bios_version->value, - (new Command(command: '/bin/kenv -q smbios.bios.version', trim_whitespace: true, redirect: '2>/dev/null'))->output, + (new Command(command: '/bin/kenv -q smbios.bios.version', trim_whitespace: true, redirect: '2>/dev/null')) + ->output, ); $this->assert_equals( $system_status->bios_date->value, - (new Command(command: '/bin/kenv -q smbios.bios.reldate', trim_whitespace: true, redirect: '2>/dev/null'))->output, + (new Command(command: '/bin/kenv -q smbios.bios.reldate', trim_whitespace: true, redirect: '2>/dev/null')) + ->output, ); } From 41a3f588531a0e3ae9cd82742e9d8cbe97314f9b Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 16:02:42 -0600 Subject: [PATCH 18/32] test: check table status code in firewall alias tests --- .../pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc index c98727b69..3ad0aa4b4 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc @@ -26,8 +26,8 @@ class APIModelsFirewallAliasTestCase extends TestCase { # Wait up to 30 seconds for the filter to reload and the table to be create foreach (range(0, 30) as $attempt) { # Check pfctl for the table - $pfctl_output = (new Command('pfctl -t TEST_GOOGLE_DNS -Ts'))->output; - if ($pfctl_output) { + $pfctl_ret = (new Command('pfctl -t TEST_GOOGLE_DNS -Ts')); + if ($pfctl_ret->output and $pfctl_ret->result_code == 0) { break; } @@ -36,8 +36,8 @@ class APIModelsFirewallAliasTestCase extends TestCase { } # Ensure expected IPs were resolved and stored in a pfctl with the same name as our alias - $this->assert_str_contains($pfctl_output, '8.8.8.8'); - $this->assert_str_contains($pfctl_output, '8.8.4.4'); + $this->assert_str_contains($pfctl_ret->output, '8.8.8.8'); + $this->assert_str_contains($pfctl_ret->output, '8.8.4.4'); # Delete the alias $test_alias->delete(apply: true); From b3fea0ddf6240f9aa10bf4c8c8d14b63c73d1f7f Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 16:02:58 -0600 Subject: [PATCH 19/32] style: run prettier on changed files --- .../local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc index 3ad0aa4b4..fed1388c5 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc @@ -26,7 +26,7 @@ class APIModelsFirewallAliasTestCase extends TestCase { # Wait up to 30 seconds for the filter to reload and the table to be create foreach (range(0, 30) as $attempt) { # Check pfctl for the table - $pfctl_ret = (new Command('pfctl -t TEST_GOOGLE_DNS -Ts')); + $pfctl_ret = new Command('pfctl -t TEST_GOOGLE_DNS -Ts'); if ($pfctl_ret->output and $pfctl_ret->result_code == 0) { break; } From b46d10b7bfe315d7c05cb741d684ce38431e4fdb Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 16:14:33 -0600 Subject: [PATCH 20/32] fix(RESTAPIKey): use hash_equals for key comparison --- .../files/usr/local/pkg/RESTAPI/Models/RESTAPIKey.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIKey.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIKey.inc index 2375748d0..26bbcef6e 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIKey.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPIKey.inc @@ -133,7 +133,7 @@ class RESTAPIKey extends Model { $hash = hash($this->hash_algo->value, $key); # Authentication is successful when incoming key matches the hashed key stored in config - if ($hash === $this->hash->value) { + if (hash_equals($hash, $this->hash->value)) { return true; } From 04d9f828c32bbb516b385fb7921ea39fbcd572ab Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 16:18:16 -0600 Subject: [PATCH 21/32] chore: opt for static analysis of requested auth method --- .../files/usr/local/pkg/RESTAPI/Auth/JWTAuth.inc | 8 ++++++++ .../files/usr/local/pkg/RESTAPI/Auth/KeyAuth.inc | 10 ++++++++++ 2 files changed, 18 insertions(+) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/JWTAuth.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/JWTAuth.inc index b0c8743f9..30d9338a8 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/JWTAuth.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/JWTAuth.inc @@ -36,4 +36,12 @@ class JWTAuth extends Auth { return false; } + + /** + * Checks if the client is requesting JWT authentication by detecting an Authorization: Bearer + * header. Credential validation is intentionally deferred to _authenticate(). + */ + public function is_requested(): bool { + return !empty($this->get_auth_key(identifier: 'bearer')); + } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/KeyAuth.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/KeyAuth.inc index 0f8aae1a3..713451386 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/KeyAuth.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/KeyAuth.inc @@ -21,6 +21,16 @@ class KeyAuth extends Auth { */ public array $security_scheme = ['type' => 'apiKey', 'in' => 'header', 'name' => 'x-api-key']; + + + /** + * Checks if the client is requesting key authentication by detecting the x-api-key header. + * Credential validation is intentionally deferred to _authenticate(). + */ + public function is_requested(): bool { + return !empty($_SERVER['HTTP_X_API_KEY'] ?? ''); + } + /** * Performs REST API key authentication and obtains the username of the user who owns the provided key. * @return bool Returns true if match for this client's key found a match stored in config, returns false From f10a9160a7374d29a08364f0f21591dd79195bd0 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 16:19:29 -0600 Subject: [PATCH 22/32] chore: require both a username and a password to request basic auth --- .../files/usr/local/pkg/RESTAPI/Auth/BasicAuth.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/BasicAuth.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/BasicAuth.inc index 7a3267c37..a8aa18d53 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/BasicAuth.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/BasicAuth.inc @@ -39,7 +39,7 @@ class BasicAuth extends Auth { * Checks if the remote client is requesting BasicAuth by checking for the necessary headers. */ public function is_requested(): bool { - return $_SERVER['PHP_AUTH_USER'] or $_SERVER['PHP_AUTH_PW']; + return $_SERVER['PHP_AUTH_USER'] and $_SERVER['PHP_AUTH_PW']; } /** From 98d3aa52ba042e112952484b3e213167f1b9684e Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 16:21:01 -0600 Subject: [PATCH 23/32] chore: guard against empty usernames in Auth class --- .../files/usr/local/pkg/RESTAPI/Core/Auth.inc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Auth.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Auth.inc index 1afa99e30..565ecd779 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Auth.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Auth.inc @@ -195,6 +195,11 @@ class Auth { * @return bool `true` if the user is disabled, `false` if it is not. */ public static function is_user_enabled(string $username): bool { + # If username is somehow empty, consider them disabled + if (empty($username)) { + return false; + } + return User::query(name: $username, disabled: false)->exists(); } From 275bf63de24e78c4509a38f80df9a4dd5c8fe85e Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 16:22:39 -0600 Subject: [PATCH 24/32] chore: guard against empty usernames in JWTAuth class --- .../files/usr/local/pkg/RESTAPI/Auth/JWTAuth.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/JWTAuth.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/JWTAuth.inc index 30d9338a8..9d3e3f2c7 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/JWTAuth.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/JWTAuth.inc @@ -28,7 +28,7 @@ class JWTAuth extends Auth { $decoded_jwt = RESTAPIJWT::decode($this->get_auth_key(identifier: 'bearer')); # Check that the JWT from our Authorization header is valid - if ($decoded_jwt) { + if ($decoded_jwt and !empty($decoded_jwt['data'])) { # Set the username of the authenticating client embedded in the decoded JWT data payload. $this->username = $decoded_jwt['data']; return true; From 311b65121790a2922a19a4c3e3f0b56e1dc7bbe7 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 16:23:27 -0600 Subject: [PATCH 25/32] chore: use PHP 8+ safe default syntax --- .../files/usr/local/pkg/RESTAPI/Auth/BasicAuth.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/BasicAuth.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/BasicAuth.inc index a8aa18d53..ce489b699 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/BasicAuth.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/BasicAuth.inc @@ -28,8 +28,8 @@ class BasicAuth extends Auth { */ public function _authenticate(): bool { # Obtain the username and password from the client via Basic authentication - $this->username = $_SERVER['PHP_AUTH_USER'] ?: ''; - $password = $_SERVER['PHP_AUTH_PW'] ?: ''; + $this->username = $_SERVER['PHP_AUTH_USER'] ?? ''; + $password = $_SERVER['PHP_AUTH_PW'] ?? ''; # Authenticate against the local user database and return the result return (bool) authenticate_user($this->username, $password); From 448a0d3ef1df0ec6968b344c331fe7e6d1c8de06 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 16:24:07 -0600 Subject: [PATCH 26/32] style: run prettier on changed files --- .../files/usr/local/pkg/RESTAPI/Auth/KeyAuth.inc | 2 -- 1 file changed, 2 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/KeyAuth.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/KeyAuth.inc index 713451386..7c5b7cfe1 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/KeyAuth.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Auth/KeyAuth.inc @@ -21,8 +21,6 @@ class KeyAuth extends Auth { */ public array $security_scheme = ['type' => 'apiKey', 'in' => 'header', 'name' => 'x-api-key']; - - /** * Checks if the client is requesting key authentication by detecting the x-api-key header. * Credential validation is intentionally deferred to _authenticate(). From 2b549f274206342b46029e30aa9c2565bbbda071 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 17:45:08 -0600 Subject: [PATCH 27/32] chore: prevent redundant validation during deletions --- .../files/usr/local/pkg/RESTAPI/Core/Endpoint.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Endpoint.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Endpoint.inc index 15857358f..ac556bf51 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Endpoint.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Endpoint.inc @@ -1299,7 +1299,7 @@ class Endpoint { } # Otherwise, delete a single object - $this->model->from_representation(data: $this->request_data); + $this->model ->from_representation(id: $this->request_data["id"], parent_id: $this->request_data["parent_id"]); return $this->model->delete(apply: $this->request_data['apply'] === true); } From 1908d8b5b4698ce38eeb45052ba60e1ec3f00d02 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 17:47:17 -0600 Subject: [PATCH 28/32] test: invert context of when basic auth is requested without username AND password --- .../local/pkg/RESTAPI/Tests/APIAuthBasicAuthTestCase.inc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIAuthBasicAuthTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIAuthBasicAuthTestCase.inc index 998b2526f..aeb2264e7 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIAuthBasicAuthTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIAuthBasicAuthTestCase.inc @@ -33,16 +33,16 @@ class APIAuthBasicAuthTestCase extends TestCase { $auth = new BasicAuth(); $this->assert_is_false($auth->is_requested()); - # Ensure is_requested() returns true if either a basic username or password are specified + # Ensure is_requested() returns false if either a basic username or password are specified $_SERVER['PHP_AUTH_USER'] = 'admin'; $_SERVER['PHP_AUTH_PW'] = null; $auth = new BasicAuth(); - $this->assert_is_true($auth->is_requested()); + $this->assert_is_false($auth->is_requested()); $_SERVER['PHP_AUTH_USER'] = null; $_SERVER['PHP_AUTH_PW'] = 'pfsense'; - $this->assert_is_true($auth->is_requested()); + $this->assert_is_false($auth->is_requested()); $_SERVER['PHP_AUTH_USER'] = 'admin'; $_SERVER['PHP_AUTH_PW'] = 'pfsense'; - $this->assert_is_true($auth->is_requested()); + $this->assert_is_false($auth->is_requested()); } } From e2f3bcdecba4d978f7943df3e25e317f03b7b968 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 17:48:38 -0600 Subject: [PATCH 29/32] style: run prettier on changed files --- .../files/usr/local/pkg/RESTAPI/Core/Endpoint.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Endpoint.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Endpoint.inc index ac556bf51..205f6a0e8 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Endpoint.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Endpoint.inc @@ -1299,7 +1299,7 @@ class Endpoint { } # Otherwise, delete a single object - $this->model ->from_representation(id: $this->request_data["id"], parent_id: $this->request_data["parent_id"]); + $this->model->from_representation(id: $this->request_data['id'], parent_id: $this->request_data['parent_id']); return $this->model->delete(apply: $this->request_data['apply'] === true); } From 0268aa7dec7eaf7a600dc253a9120ec0c16c7d4b Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 19:45:12 -0600 Subject: [PATCH 30/32] test: correct accidental false assertion for valid basic auth --- .../usr/local/pkg/RESTAPI/Tests/APIAuthBasicAuthTestCase.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIAuthBasicAuthTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIAuthBasicAuthTestCase.inc index aeb2264e7..f74805518 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIAuthBasicAuthTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIAuthBasicAuthTestCase.inc @@ -43,6 +43,6 @@ class APIAuthBasicAuthTestCase extends TestCase { $this->assert_is_false($auth->is_requested()); $_SERVER['PHP_AUTH_USER'] = 'admin'; $_SERVER['PHP_AUTH_PW'] = 'pfsense'; - $this->assert_is_false($auth->is_requested()); + $this->assert_is_true($auth->is_requested()); } } From 77500f7dbe18853cbc66aca68bee340492f53c04 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 20:40:15 -0600 Subject: [PATCH 31/32] ci: do not build for CE 2.8.0 We do not release for 2.8.0 anymore, so we should not do test builds for it either. --- .github/workflows/build.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7fe6bb7a1..0c62a0325 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -57,8 +57,6 @@ jobs: strategy: matrix: include: - - PFSENSE_VERSION: pfSense-2.8.0-RELEASE - FREEBSD_ID: freebsd15 - PFSENSE_VERSION: pfSense-2.8.1-RELEASE FREEBSD_ID: freebsd15 steps: @@ -107,8 +105,6 @@ jobs: strategy: matrix: include: - - PFSENSE_VERSION: pfSense-2.8.0-RELEASE - FREEBSD_ID: freebsd15 - PFSENSE_VERSION: pfSense-2.8.1-RELEASE FREEBSD_ID: freebsd15 steps: @@ -136,8 +132,6 @@ jobs: strategy: matrix: include: - - PFSENSE_VERSION: pfSense-2.8.0-RELEASE - FREEBSD_ID: freebsd15 - PFSENSE_VERSION: pfSense-2.8.1-RELEASE FREEBSD_ID: freebsd15 From fcc9b8fa49f5053a5f636b998ad5e658733823df Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Thu, 6 Aug 2026 20:51:21 -0600 Subject: [PATCH 32/32] chore: change validator expansion method for StringFields --- .../files/usr/local/pkg/RESTAPI/Fields/StringField.inc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/StringField.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/StringField.inc index 45565b28d..64ebf30b5 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/StringField.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/StringField.inc @@ -123,7 +123,8 @@ class StringField extends Field { internal_namespace: $internal_namespace, referenced_by: $referenced_by, conditions: $conditions, - validators: $validators + [ + validators: [ + ...$validators, new RESTAPI\Validators\LengthValidator(minimum: $minimum_length, maximum: $maximum_length), ], help_text: $help_text,