From 11ac59408dc135b39a924a5f801cf95cba993181 Mon Sep 17 00:00:00 2001
From: Nikolai Beketov <nbek@miko.ru>
Date: Fri, 23 Oct 2026 17:22:45 +0700
Subject: [PATCH] fix(modules): read the module from the path id in
 startDownload and getDownloadStatus (#1250)

REST v3 puts the module of /modules/{id}:getDownloadStatus and :startDownload in data['id'], but the processor read only data['uniqid'], so every v3 call failed with 422 "Undefined array key". Use uniqid when it is a non-empty string, otherwise the path id; non-string values become ''. Both actions still reject a bad id with 400 before touching the filesystem.

Since #1161 GET bodies are ignored, so test_04 now sends the unsafe uniqid in the query; test_05 checks that the path id reaches the action.
---
 .../Lib/ModulesManagementProcessor.php        | 25 ++++++++++++++-----
 tests/api/test_39_modules.py                  | 24 +++++++++++++++---
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/src/PBXCoreREST/Lib/ModulesManagementProcessor.php b/src/PBXCoreREST/Lib/ModulesManagementProcessor.php
index 682f48847..58865b21e 100644
--- a/src/PBXCoreREST/Lib/ModulesManagementProcessor.php
+++ b/src/PBXCoreREST/Lib/ModulesManagementProcessor.php
@@ -75,14 +75,18 @@ public static function callBack(array $request): PBXApiResult
         $res->processor = __METHOD__;
             switch ($action) {
                 case 'startDownload':
-                    $module = $request['data']['uniqid'];
-                    $url = $request['data']['url'];
-                    $md5 = $request['data']['md5'];
-                    $res = StartDownloadAction::main($module, $url, $md5);
+                    // REST v3 carries the module in the path as id; uniqid is the legacy field (#1250).
+                    // Both actions reject a bad or empty id with 400 before touching the filesystem.
+                    $res = StartDownloadAction::main(
+                        self::stringParam($data, 'uniqid') ?: self::stringParam($data, 'id'),
+                        self::stringParam($data, 'url'),
+                        self::stringParam($data, 'md5')
+                    );
                     break;
                 case 'getDownloadStatus':
-                    $module = $request['data']['uniqid'];
-                    $res = DownloadStatusAction::main($module);
+                    $res = DownloadStatusAction::main(
+                        self::stringParam($data, 'uniqid') ?: self::stringParam($data, 'id')
+                    );
                     break;
                 case 'installFromPackage':
                     $filePath = $data['filePath'];
@@ -341,6 +345,15 @@ private static function logOperationInitiator(
         SystemMessages::sysLogMsg(__CLASS__, 'Module operation started: ' . $encoded, LOG_WARNING);
     }
 
+    /**
+     * A request field as a string; missing or non-string values (e.g. uniqid[]=x) become ''.
+     */
+    private static function stringParam(array $data, string $key): string
+    {
+        $value = $data[$key] ?? '';
+        return is_string($value) ? $value : '';
+    }
+
     /**
      * Emergency switch: '1' routes install/update/uninstall back to the
      * legacy mutex-driven pipeline for one release cycle.
diff --git a/tests/api/test_39_modules.py b/tests/api/test_39_modules.py
index b6f42584b..7cd6b6b3b 100644
--- a/tests/api/test_39_modules.py
+++ b/tests/api/test_39_modules.py
@@ -73,13 +73,13 @@ def test_04_get_download_status_rejects_unsafe_uniqid(self, api_client):
         """GET /modules/{id}:getDownloadStatus must reject a uniqid that is not a safe directory name.
 
         WHY: the uniqid is used to build the download directory path under the upload dir.
-        The id goes in a JSON body: the nginx WAF answers 403 to ".." in the query string
-        before PHP runs, but it does not scan GET bodies, and the API reads them.
+        GET bodies are ignored since #1161 and the WAF answers 403 to ".." in the query
+        string, so the reachable vector is a query uniqid that is not a plain name.
         """
         # Sent raw so that a non-2xx answer does not raise
         raw = api_client.session.get(
             f"{api_client.base_url}/modules/ModuleTemplate:getDownloadStatus",
-            json={'uniqid': '../../evil'},
+            params={'uniqid': 'evil/name'},
             headers=api_client._get_headers(),
             timeout=30
         )
@@ -87,5 +87,23 @@ def test_04_get_download_status_rejects_unsafe_uniqid(self, api_client):
             f"Expected HTTP 400 for unsafe uniqid, got {raw.status_code}: {raw.text}"
         assert raw.json().get('result') is False, f"Expected result=false, got: {raw.text}"
 
+    def test_05_get_download_status_uses_path_id(self, api_client):
+        """GET /modules/{id}:getDownloadStatus must read the module from the path (#1250).
+
+        Without a download in progress the action reports NOT_FOUND; before the fix the
+        processor ignored the path id and failed with 422 "Undefined array key", and a
+        non-string uniqid (uniqid[]=x) failed with a TypeError. Such a uniqid is ignored.
+        """
+        for params in ({}, {'uniqid[]': 'ModuleTemplate'}):
+            raw = api_client.session.get(
+                f"{api_client.base_url}/modules/ModuleTemplate:getDownloadStatus",
+                params=params,
+                headers=api_client._get_headers(),
+                timeout=30
+            )
+            data = raw.json().get('data') or {}
+            assert data.get('d_status') == 'NOT_FOUND', \
+                f"Expected d_status=NOT_FOUND for {params or 'path id only'}, got {raw.status_code}: {raw.text}"
+
 if __name__ == '__main__':
     pytest.main([__file__, '-v', '-s'])