diff --git a/.gitignore b/.gitignore index 3561c12..40a32f3 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,5 @@ vendor composer.lock .idea/ .eslintrc +CODE_QUALITY_SCAN.md +SECURITY_SCAN.md diff --git a/CODE_QUALITY_SCAN.md b/CODE_QUALITY_SCAN.md new file mode 100644 index 0000000..636f869 --- /dev/null +++ b/CODE_QUALITY_SCAN.md @@ -0,0 +1,36 @@ +# Code quality and PHP 8.1-8.5 compatibility scan + +## Scope + +- Module: `postcode-nl/api-magento2-module` +- Scan date: 2026-06-12 +- Runtime available in this environment: PHP 8.4.18 + +## Validation performed + +1. `php -l` across every PHP file in the module +2. `composer validate --no-check-lock --no-check-publish` +3. Static review focused on PHP 8.1-8.5 compatibility risks and runtime errors +4. Follow-up code review after the fixes in this branch + +## Result + +The module is in good shape for PHP 8.1-8.5 after the fixes in this branch. + +## Findings addressed in this branch + +| Severity | File | Issue | Status | +| --- | --- | --- | --- | +| High | `Service/PostcodeApiClient.php` | Invalid JSON error path used `sprintf()` incorrectly, which could raise `ArgumentCountError` on PHP 8.1+ instead of the intended module exception. | Fixed | +| High | `Helper/ApiClientHelper.php` | ISO-2 country codes that do not map to ISO-3 could flow into a strictly typed method as `null`, causing `TypeError` on PHP 8.1+. | Fixed | +| Medium | `Controller/Adminhtml/Address/Api.php` | Missing or malformed admin request parameters could lead to PHP argument errors instead of a handled bad-request response. | Fixed | + +## Notes + +- The reported nullable-parameter deprecation is already addressed in this fork: `Helper/ApiClientHelper.php` uses an explicit nullable type for `$labelSuffix`. +- `composer validate` reports one metadata warning only: the package keeps a `"version"` field in `composer.json`. That is not a PHP 8.1-8.5 compatibility problem. +- This environment only provided PHP 8.4 for execution, so the compatibility conclusion is based on PHP 8.4 validation plus static review against known PHP 8.1-8.5 incompatibility patterns. + +## Remaining concerns + +No further actionable PHP 8.1-8.5 compatibility issues were identified during this scan. diff --git a/Controller/Adminhtml/Address/Api.php b/Controller/Adminhtml/Address/Api.php index bc27138..87bdc25 100644 --- a/Controller/Adminhtml/Address/Api.php +++ b/Controller/Adminhtml/Address/Api.php @@ -61,8 +61,23 @@ public function execute(): Json throw new \Exception('Invalid service method'); } - $values = array_filter($request->getParams(), fn($key) => in_array($key, $params), ARRAY_FILTER_USE_KEY); - $result = $this->_postcodeModel->$serviceMethod(...array_values($values)); + $values = []; + + foreach ($params as $param) { + $value = $request->getParam($param); + + if ($value === null) { + throw new \InvalidArgumentException(sprintf('Missing required parameter `%s`.', $param)); + } + + if (!is_scalar($value)) { + throw new \InvalidArgumentException(sprintf('Invalid parameter `%s`.', $param)); + } + + $values[] = (string) $value; + } + + $result = $this->_postcodeModel->$serviceMethod(...$values); $result = $this->_serviceOutputProcessor->process($result, PostcodeModelInterface::class, $serviceMethod); return $resultJson->setData($result); } catch (\Exception $e) { diff --git a/Helper/ApiClientHelper.php b/Helper/ApiClientHelper.php index ad9daec..20eac75 100644 --- a/Helper/ApiClientHelper.php +++ b/Helper/ApiClientHelper.php @@ -4,6 +4,7 @@ use Exception; use PostcodeEu\AddressValidation\Helper\StoreConfigHelper; +use PostcodeEu\AddressValidation\Service\Exception\BadRequestException; use PostcodeEu\AddressValidation\Service\Exception\NotFoundException; use PostcodeEu\AddressValidation\Service\PostcodeApiClient; use Magento\Customer\Helper\Address as AddressHelper; @@ -469,11 +470,18 @@ public function getSupportedCountries(): array public function validateAddress(): array { $args = func_get_args(); - if (strlen($args[0]) === 2) { - $args[0] = $this->getCountryIso3Code($args[0]); // Support country ISO 2 code. - } try { + if (strlen($args[0]) === 2) { + $countryIso3Code = $this->getCountryIso3Code($args[0]); // Support country ISO 2 code. + + if ($countryIso3Code === null) { + throw new BadRequestException('Country not supported', 400); + } + + $args[0] = $countryIso3Code; + } + $client = $this->getApiClient(); $response = $client->validateAddress(...$args); diff --git a/SECURITY_SCAN.md b/SECURITY_SCAN.md new file mode 100644 index 0000000..d196b6f --- /dev/null +++ b/SECURITY_SCAN.md @@ -0,0 +1,29 @@ +# Security scan + +## Scope + +- Module: `postcode-nl/api-magento2-module` +- Scan date: 2026-06-12 + +## Review performed + +1. Static review of request handling, outbound API usage, credential handling, and exception paths +2. Pattern scan for high-risk PHP functions and insecure legacy APIs +3. Follow-up code review after the fixes in this branch + +## Findings addressed in this branch + +| Severity | File | Issue | Status | +| --- | --- | --- | --- | +| High | `Service/PostcodeApiClient.php` | The module forwarded the incoming `HTTP_REFERER` header to `api.postcode.eu`, which could leak storefront or admin URLs and query strings to a third party. | Fixed by removing referer forwarding | +| Medium | `Controller/Adminhtml/Address/Api.php` | The admin endpoint accepted unchecked parameter shapes before dispatching them into typed service methods. | Fixed by validating presence and scalar input before dispatch | + +## Reviewed items + +- API credentials remain stored through Magento configuration and decrypted through Magento's encryptor. +- Cached status data uses Magento's serializer interface, not native PHP `unserialize()`. +- No dangerous command-execution helpers or similar high-risk sinks were found in the module source during the scan. + +## Remaining concerns + +No additional meaningful security issues were identified after the fixes in this branch. diff --git a/Service/PostcodeApiClient.php b/Service/PostcodeApiClient.php index 9183552..38cc80b 100644 --- a/Service/PostcodeApiClient.php +++ b/Service/PostcodeApiClient.php @@ -5,8 +5,6 @@ use PostcodeEu\AddressValidation\Helper\StoreConfigHelper; use Magento\Framework\App\ProductMetadataInterface; -use Magento\Framework\App\Request\Http as HttpRequest; - use PostcodeEu\AddressValidation\HTTP\Client\Curl; use PostcodeEu\AddressValidation\Service\Exception\AuthenticationException; use PostcodeEu\AddressValidation\Service\Exception\BadRequestException; @@ -55,7 +53,6 @@ class PostcodeApiClient public function __construct( Curl $curl, - HttpRequest $request, ProductMetadataInterface $productMetadata, StoreConfigHelper $storeConfigHelper ) { @@ -67,10 +64,6 @@ public function __construct( CURLOPT_CONNECTTIMEOUT => 2, CURLOPT_TIMEOUT => 5, ]); - - if (null !== $request->getServer('HTTP_REFERER')) { - $curl->setOption(CURLOPT_REFERER, $request->getServer('HTTP_REFERER')); - } } public function getUserAgent(): string @@ -294,7 +287,7 @@ protected function _fetch(string $path, ?string $session = null): array $jsonResponse = json_decode($response, true); if (!is_array($jsonResponse)) { throw new InvalidJsonResponseException( - sprintf('Invalid JSON response from the server for request: `%s`.' . $url) + sprintf('Invalid JSON response from the server for request: `%s`.', $url) ); }