Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ vendor
composer.lock
.idea/
.eslintrc
CODE_QUALITY_SCAN.md
SECURITY_SCAN.md
36 changes: 36 additions & 0 deletions CODE_QUALITY_SCAN.md
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 17 additions & 2 deletions Controller/Adminhtml/Address/Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 11 additions & 3 deletions Helper/ApiClientHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
29 changes: 29 additions & 0 deletions SECURITY_SCAN.md
Original file line number Diff line number Diff line change
@@ -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.
9 changes: 1 addition & 8 deletions Service/PostcodeApiClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -55,7 +53,6 @@ class PostcodeApiClient

public function __construct(
Curl $curl,
HttpRequest $request,
ProductMetadataInterface $productMetadata,
StoreConfigHelper $storeConfigHelper
) {
Expand All @@ -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
Expand Down Expand Up @@ -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)
);
}

Expand Down