From 8d73d0811fc2a966b62668ae5f95d6f20fe76d15 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Sat, 20 Sep 2025 16:08:55 -0600 Subject: [PATCH 1/7] fix(VirtualIP): make carp vhid unique per interface #754 This commit fixes an issue where a CARP virtual IP's VHID was forced to be unique across all existing CARP virtual IPs regardless of interface. This change allows the VHID to be reused by other interfaces. --- .../local/pkg/RESTAPI/Models/VirtualIP.inc | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/VirtualIP.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/VirtualIP.inc index 020342fb5..5b2ce29f1 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/VirtualIP.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/VirtualIP.inc @@ -10,6 +10,7 @@ use RESTAPI\Fields\IntegerField; use RESTAPI\Fields\InterfaceField; use RESTAPI\Fields\StringField; use RESTAPI\Fields\UIDField; +use RESTAPI\Responses\ConflictError; use RESTAPI\Responses\ValidationError; use RESTAPI\Validators\IPAddressValidator; use RESTAPI\Validators\UniqueFromForeignModelValidator; @@ -89,7 +90,6 @@ class VirtualIP extends Model { ); $this->vhid = new IntegerField( required: true, - unique: true, minimum: 1, maximum: 255, conditions: ['mode' => 'carp'], @@ -178,6 +178,27 @@ class VirtualIP extends Model { return $subnet_bits; } + /** + * Adds extra validation to the vhid field. + * @param int $vhid The incoming `vhid` value to be validated. + * @return int The validated `vhid` value to be set. + * @throws ValidationError When the `vhid` value is already used by another CARP virtual IP on the same interface. + */ + public function validate_vhid(int $vhid): int { + # Check for an existing CARP virtual IP with the same VHID on this interface + $vip_q = $this->query(id__except: $this->id, mode: 'carp', interface: $this->interface->value, vhid: $vhid); + + # Ensure no other CARP virtual IP on this interface is using the same VHID + if ($vip_q->exists()) { + $vip = $vip_q->first(); + throw new ConflictError( + message: "Virtual IP with ID '$vip->id' is already using VHID '$vhid' on interface '{$this->interface->value}'", + response_id: 'VIRTUALIP_VHID_ALREADY_IN_USE', + ); + } + return $vhid; + } + /** * Obtains the current internal CARP status of this object * @return string|null Returns a string that indicates the current CARP status of this virtual IP, or null From e2f8a553d81199648530c05897a4659b0e948495 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Sat, 20 Sep 2025 16:09:45 -0600 Subject: [PATCH 2/7] test(VirtualIP): add tests for unique-per-interface vhids #754 --- .../Tests/APIModelsVirtualIPTestCase.inc | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsVirtualIPTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsVirtualIPTestCase.inc index bbf7b36b5..1d594a415 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsVirtualIPTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsVirtualIPTestCase.inc @@ -242,4 +242,45 @@ class APIModelsVirtualIPTestCase extends TestCase { $carp_status->update(); $carp_vip->delete(apply: true); } + + public function test_carp_vhid_must_be_unique_per_interface(): void { + # Create a virtual IP to test with + $vip = new VirtualIP( + mode: 'carp', + interface: 'lan', + subnet: '127.1.2.3', + subnet_bits: 32, + password: 'testpasswd', + vhid: 5, + ); + $vip->create(); + + # Ensure we can update the existing VIP with the same VHID without issue + $this->assert_does_not_throw( + callable: function () use ($vip) { + $vip->validate_vhid(vhid: 5); + }, + ); + + # Ensure we cannot create a new VIP with the same VHID on the same interface + $this->assert_throws_response( + response_id: 'VIRTUALIP_VHID_ALREADY_IN_USE', + code: 409, + callable: function () { + $vip = new VirtualIP(mode: 'carp', interface: 'lan'); + $vip->validate_vhid(vhid: 5); + }, + ); + + # Ensure we can create a new VIP with the same VHID on a different interface + $this->assert_does_not_throw( + callable: function () { + $vip = new VirtualIP(mode: 'carp', interface: 'wan'); + $vip->validate_vhid(vhid: 5); + }, + ); + + # Clean up the VIP we created + $vip->delete(); + } } From e63f440a6e16fdc9e3fd5aa74bbddb6463d59e4b Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Sun, 21 Sep 2025 09:42:24 -0600 Subject: [PATCH 3/7] docs(mkdocs): add repo url to config for edit on gh --- mkdocs.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/mkdocs.yml b/mkdocs.yml index d50975458..fca1f06b3 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -1,4 +1,5 @@ site_name: pfSense REST API Guide +repo_url: https://github.com/jaredhendrickson13/pfsense-api nav: - General: - Home: index.md From 3e95cb1a085c9ab6d4c425eec7be512bb71fcd93 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Sun, 21 Sep 2025 11:13:05 -0600 Subject: [PATCH 4/7] chore(deps): add prettier to package.json --- package-lock.json | 11 ++++++----- package.json | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index eac56056e..c7c4b406b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,7 +6,8 @@ "": { "devDependencies": { "@prettier/plugin-php": "^0.24.0", - "@stoplight/spectral-cli": "^6.15.0" + "@stoplight/spectral-cli": "^6.15.0", + "prettier": "^3.6.2" } }, "node_modules/@asyncapi/specs": { @@ -2154,11 +2155,11 @@ } }, "node_modules/prettier": { - "version": "3.2.4", - "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.2.4.tgz", - "integrity": "sha512-FWu1oLHKCrtpO1ypU6J0SbK2d9Ckwysq6bHj/uaCP26DxrPpppCLQRGVuqAxSTvhF00AcvDRyYrLNW7ocBhFFQ==", + "version": "3.6.2", + "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.6.2.tgz", + "integrity": "sha512-I7AIg5boAr5R0FFtJ6rCfD+LFsWHp81dolrFD8S79U9tb8Az2nGrJncnMSnys+bpQJfRUzqs9hnA81OAA3hCuQ==", "dev": true, - "peer": true, + "license": "MIT", "bin": { "prettier": "bin/prettier.cjs" }, diff --git a/package.json b/package.json index 990f4f6eb..20dfe64cf 100644 --- a/package.json +++ b/package.json @@ -1,5 +1,6 @@ { "devDependencies": { + "prettier": "^3.6.2", "@prettier/plugin-php": "^0.24.0", "@stoplight/spectral-cli": "^6.15.0" } From 7f49b481ffc198e9798050b0fdb812d8637310fd Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Sun, 21 Sep 2025 11:15:14 -0600 Subject: [PATCH 5/7] test: add retries to speed sensitive tests --- .../pkg/RESTAPI/Tests/APIModelsFirewallAliasTestCase.inc | 4 +++- .../usr/local/pkg/RESTAPI/Tests/APIModelsTableTestCase.inc | 1 + 2 files changed, 4 insertions(+), 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 c98d8e425..94a984058 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 @@ -3,13 +3,15 @@ namespace RESTAPI\Tests; use RESTAPI\Core\TestCase; +use RESTAPI\Core\TestCaseRetry; use RESTAPI\Models\FirewallAlias; class APIModelsFirewallAliasTestCase extends TestCase { /** * Checks that aliases with hostnames correctly populate a pfctl table */ - public function test_fqdn_alias_populates_pfctl_table() { + #[TestCaseRetry(retries: 3, delay: 1)] + public function test_fqdn_alias_populates_pfctl_table(): void { # Create an alias that includes dns.google as an alias item $test_alias = new FirewallAlias( data: [ diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsTableTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsTableTestCase.inc index 45dee568f..f95abac4a 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsTableTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsTableTestCase.inc @@ -82,6 +82,7 @@ class APIModelsTableTestCase extends TestCase { /** * Checks that we can successfully delete (flush) entrries from a table */ + #[TestCaseRetry(retries: 3, delay: 1)] public function test_delete(): void { # Create a new pf table to test with $this->add_table(table_name: 'pfrest_test_table', entries: ['1.2.3.4', '4.3.2.1']); From ce760ddabbca421abed730a41c31130ce8c59e99 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Sun, 21 Sep 2025 12:30:25 -0600 Subject: [PATCH 6/7] fix(ForeignModelField): don't query if target and local fields are the same #749 This addresses an issue where the ForeignModelField could unnecessarily make query when the target and local field names are the same. In large rulesets this can result in a timeout. --- .../files/usr/local/pkg/RESTAPI/Fields/ForeignModelField.inc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/ForeignModelField.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/ForeignModelField.inc index 0a5d8f32f..e79734fbb 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/ForeignModelField.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/ForeignModelField.inc @@ -217,6 +217,11 @@ class ForeignModelField extends Field { * @return mixed The field value in its representation form. */ protected function _from_internal(mixed $internal_value): mixed { + # If the model_field_internal, and the model_field are the same, return the internal value as-is. + if ($this->model_field_internal === $this->model_field) { + return $internal_value; + } + # Ensure the internal field value is converted to its representation value before querying if ($this->model_field_internal !== 'id') { $internal_value = $this->models[0]->{$this->model_field_internal}->_from_internal($internal_value); From 5af3236accd3afae3e8b331d3a5e0bd40ca534d0 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Mon, 22 Sep 2025 21:28:38 -0600 Subject: [PATCH 7/7] fix(ForeignModelField): always load from internal before returning --- .../usr/local/pkg/RESTAPI/Fields/ForeignModelField.inc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/ForeignModelField.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/ForeignModelField.inc index e79734fbb..6020a7521 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/ForeignModelField.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/ForeignModelField.inc @@ -217,16 +217,16 @@ class ForeignModelField extends Field { * @return mixed The field value in its representation form. */ protected function _from_internal(mixed $internal_value): mixed { - # If the model_field_internal, and the model_field are the same, return the internal value as-is. - if ($this->model_field_internal === $this->model_field) { - return $internal_value; - } - # Ensure the internal field value is converted to its representation value before querying if ($this->model_field_internal !== 'id') { $internal_value = $this->models[0]->{$this->model_field_internal}->_from_internal($internal_value); } + # If the model_field_internal, and the model_field are the same, return the internal value as-is. + if ($this->model_field_internal === $this->model_field) { + return $internal_value; + } + # Query for the Model object this value relates to. $query_modelset = $this->__get_matches($this->model_field_internal, $internal_value);