From 2aeb889574a826f89e6e4690f5b58e9f3808fcd9 Mon Sep 17 00:00:00 2001
From: Ivan Bochkarev <ivanx86@gmail.com>
Date: Fri, 17 Jul 2026 15:00:58 +0600
Subject: [PATCH 1/3] fix(manager): align order update response with GET, hide
 secret token

PUT /api/mgr/orders/{id} returned a raw $order->toArray(): the secret
order token leaked and the shape diverged from GET (no address merge,
no formatted cost). Route update() through buildOrderPayloadFromModel()
like GET and add a null-guard after the status-change reload.

Strip the web-order secret token from every Manager order payload via
HIDDEN_ORDER_FIELDS inside the formatOrder() funnel (get, list, create,
finalize, recalculate-cost, update), closing the leak consistently.
---
 .../Api/Manager/OrdersController.php          | 20 ++++++++++-
 .../tests/OrderPayloadSecretFieldsTest.php    | 36 +++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 core/components/minishop3/tests/OrderPayloadSecretFieldsTest.php

diff --git a/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php b/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php
index ce13d719..aa4a4b20 100644
--- a/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php
+++ b/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php
@@ -71,6 +71,16 @@ class OrdersController
         'phone',
     ];
 
+    /**
+     * Internal order fields never exposed in Manager API responses.
+     *
+     * `token` is the secret used to authorize web order actions (see Web OrderController
+     * `ms3_token`); the Vue manager never consumes it, so it must not leak in any order payload.
+     */
+    protected const HIDDEN_ORDER_FIELDS = [
+        'token',
+    ];
+
     protected modX $modx;
     protected ?OrderLogService $orderLog = null;
     protected ?Utils $ms3Utils = null;
@@ -970,9 +980,13 @@ public function update(array $params = []): array
 
             // Reload order to get updated data
             $order = $this->modx->getObject(msOrder::class, $id);
+            if (!$order instanceof msOrder) {
+                return Response::error('Order not found after update', HttpStatus::INTERNAL_SERVER_ERROR)->getData();
+            }
         }
 
-        return Response::success($order->toArray(), 'Order updated successfully')->getData();
+        // Return the same shape as GET (address-merge, formatted cost, no secret fields)
+        return Response::success($this->buildOrderPayloadFromModel($order), 'Order updated successfully')->getData();
     }
 
     /**
@@ -1778,6 +1792,10 @@ protected function formatOrder(array $data): array
             }
         }
 
+        foreach (self::HIDDEN_ORDER_FIELDS as $field) {
+            unset($data[$field]);
+        }
+
         return $data;
     }
 
diff --git a/core/components/minishop3/tests/OrderPayloadSecretFieldsTest.php b/core/components/minishop3/tests/OrderPayloadSecretFieldsTest.php
new file mode 100644
index 00000000..eaf52a36
--- /dev/null
+++ b/core/components/minishop3/tests/OrderPayloadSecretFieldsTest.php
@@ -0,0 +1,36 @@
+<?php
+
+/**
+ * Guards the Manager order payload secret denylist (issue #421).
+ *
+ * `formatOrder()` strips every key in HIDDEN_ORDER_FIELDS from all order responses
+ * (get / list / create / finalize / recalculate-cost / update). This static check
+ * fails if the web-order secret `token` is dropped from that denylist.
+ *
+ * Run: php tests/OrderPayloadSecretFieldsTest.php
+ */
+
+declare(strict_types=1);
+
+require __DIR__ . '/../vendor/autoload.php';
+
+use MiniShop3\Controllers\Api\Manager\OrdersController;
+
+$fail = static function (string $message): never {
+    fwrite(STDERR, "FAIL: {$message}\n");
+    exit(1);
+};
+
+$reflection = new ReflectionClass(OrdersController::class);
+$hiddenFields = $reflection->getConstant('HIDDEN_ORDER_FIELDS');
+
+if (!is_array($hiddenFields)) {
+    $fail('HIDDEN_ORDER_FIELDS must be an array');
+}
+
+if (!in_array('token', $hiddenFields, true)) {
+    $fail('HIDDEN_ORDER_FIELDS must contain "token" (web-order secret must never leak)');
+}
+
+fwrite(STDOUT, "OK OrderPayloadSecretFieldsTest\n");
+exit(0);

From 5f472fad90c4599403d0fe3a58098a056a0cf890 Mon Sep 17 00:00:00 2001
From: Ivan Bochkarev <ivanx86@gmail.com>
Date: Fri, 17 Jul 2026 15:06:55 +0600
Subject: [PATCH 2/3] test(manager): add live smoke for order GET/PUT payload
 parity

Optional MODX-backed smoke that asserts update() matches GET shape and
that the web-order token never appears in Manager order responses.
---
 .../tests/smoke_order_update_payload.php      | 205 ++++++++++++++++++
 1 file changed, 205 insertions(+)
 create mode 100644 core/components/minishop3/tests/smoke_order_update_payload.php

diff --git a/core/components/minishop3/tests/smoke_order_update_payload.php b/core/components/minishop3/tests/smoke_order_update_payload.php
new file mode 100644
index 00000000..8488c7ec
--- /dev/null
+++ b/core/components/minishop3/tests/smoke_order_update_payload.php
@@ -0,0 +1,205 @@
+<?php
+
+/**
+ * Live smoke for Manager order GET/PUT payload shape (issue #421).
+ *
+ * Bootstraps a local MODX install and calls OrdersController directly
+ * (no HTTP session required). Optional — needs a real DB with orders.
+ *
+ * Usage:
+ *   MODX_CONFIG_CORE=/path/to/config.core.php \
+ *     php tests/smoke_order_update_payload.php [--order-id=N]
+ *
+ * If MODX_CONFIG_CORE is unset, looks for ../../../../config.core.php
+ * relative to this file (core/components/minishop3/tests → site root).
+ */
+
+declare(strict_types=1);
+
+$configCore = getenv('MODX_CONFIG_CORE') ?: '';
+if ($configCore === '') {
+    $candidate = dirname(__DIR__, 4) . '/config.core.php';
+    if (is_file($candidate)) {
+        $configCore = $candidate;
+    }
+}
+
+if ($configCore === '' || !is_file($configCore)) {
+    fwrite(STDERR, "SKIP: set MODX_CONFIG_CORE to a MODX config.core.php (live DB required)\n");
+    exit(0);
+}
+
+require $configCore;
+require MODX_CORE_PATH . 'config/' . MODX_CONFIG_KEY . '.inc.php';
+require MODX_CORE_PATH . 'vendor/autoload.php';
+
+use MiniShop3\Controllers\Api\Manager\OrdersController;
+use MiniShop3\Model\msOrder;
+
+$fail = static function (string $message): never {
+    fwrite(STDERR, "FAIL: {$message}\n");
+    exit(1);
+};
+
+$pass = static function (string $message): void {
+    fwrite(STDOUT, "OK: {$message}\n");
+};
+
+$orderId = null;
+foreach ($argv as $arg) {
+    if (str_starts_with($arg, '--order-id=')) {
+        $orderId = (int) substr($arg, 11);
+    }
+}
+
+$modx = new \MODX\Revolution\modX();
+$modx->initialize('mgr');
+
+if (!$modx->services->has('ms3')) {
+    $fail('ms3 service is not registered');
+}
+
+if ($orderId === null || $orderId <= 0) {
+    // Prefer an order that still has a web-order secret in DB (stronger leak check).
+    $c = $modx->newQuery(msOrder::class);
+    $c->where(['token:!=' => '', 'AND:token:IS NOT' => null]);
+    $c->select('id');
+    $c->sortby('id', 'DESC');
+    $c->limit(1);
+    $c->prepare();
+    $c->stmt->execute();
+    $orderId = (int) $c->stmt->fetchColumn();
+    if ($orderId <= 0) {
+        $c = $modx->newQuery(msOrder::class);
+        $c->select('id');
+        $c->sortby('id', 'DESC');
+        $c->limit(1);
+        $c->prepare();
+        $c->stmt->execute();
+        $orderId = (int) $c->stmt->fetchColumn();
+    }
+}
+
+if ($orderId <= 0) {
+    $fail('No order found to smoke-test');
+}
+
+/** @var msOrder|null $before */
+$before = $modx->getObject(msOrder::class, $orderId);
+if (!$before) {
+    $fail("Order #{$orderId} not found");
+}
+
+$dbToken = (string) $before->get('token');
+$requireDbToken = $dbToken !== '';
+
+$controller = new OrdersController($modx);
+
+$get = $controller->get(['id' => $orderId]);
+if (empty($get['success'])) {
+    $fail('GET failed: ' . json_encode($get, JSON_UNESCAPED_UNICODE));
+}
+$getData = $get['data'] ?? null;
+if (!is_array($getData)) {
+    $fail('GET data is not an array');
+}
+
+$hasTokenInGet = array_key_exists('token', $getData);
+$pass(sprintf(
+    'GET #%d success; token in payload: %s',
+    $orderId,
+    $hasTokenInGet ? 'YES (leak)' : 'no'
+));
+
+$oldComment = (string) $before->get('order_comment');
+$marker = '[smoke-421 ' . date('H:i:s') . ']';
+$newComment = $oldComment === '' ? $marker : $oldComment;
+
+$put = $controller->update([
+    'id' => $orderId,
+    'order_comment' => $newComment,
+]);
+
+if (empty($put['success'])) {
+    $fail('PUT failed: ' . json_encode($put, JSON_UNESCAPED_UNICODE));
+}
+$putData = $put['data'] ?? null;
+if (!is_array($putData)) {
+    $fail('PUT data is not an array');
+}
+
+$hasTokenInPut = array_key_exists('token', $putData);
+$pass(sprintf(
+    'PUT #%d success; token in payload: %s',
+    $orderId,
+    $hasTokenInPut ? 'YES (leak)' : 'no'
+));
+
+if ($newComment !== $oldComment) {
+    $controller->update(['id' => $orderId, 'order_comment' => $oldComment]);
+}
+
+$shapeKeysExpected = [
+    'status_name',
+    'delivery_name',
+    'payment_name',
+    'customer',
+    'cost_formatted',
+    'cart_cost_formatted',
+    'delivery_cost_formatted',
+];
+
+$missingInPut = [];
+foreach ($shapeKeysExpected as $key) {
+    if (!array_key_exists($key, $putData)) {
+        $missingInPut[] = $key;
+    }
+}
+
+$putKeys = array_keys($putData);
+$getKeys = array_keys($getData);
+sort($putKeys);
+sort($getKeys);
+
+$onlyInPut = array_values(array_diff($putKeys, $getKeys));
+$onlyInGet = array_values(array_diff($getKeys, $putKeys));
+
+fwrite(STDOUT, 'GET keys count: ' . count($getKeys) . "\n");
+fwrite(STDOUT, 'PUT keys count: ' . count($putKeys) . "\n");
+if ($onlyInPut) {
+    fwrite(STDOUT, 'Only in PUT: ' . implode(', ', $onlyInPut) . "\n");
+}
+if ($onlyInGet) {
+    fwrite(STDOUT, 'Only in GET: ' . implode(', ', $onlyInGet) . "\n");
+}
+
+$errors = [];
+if ($hasTokenInGet) {
+    $errors[] = 'GET still exposes token';
+}
+if ($hasTokenInPut) {
+    $errors[] = 'PUT still exposes token';
+}
+if ($missingInPut) {
+    $errors[] = 'PUT missing GET-shape keys: ' . implode(', ', $missingInPut);
+}
+if ($onlyInPut || $onlyInGet) {
+    $errors[] = 'GET/PUT key sets differ';
+}
+
+$after = $modx->getObject(msOrder::class, $orderId);
+if (!$after) {
+    $errors[] = 'Order missing after update';
+} elseif ($requireDbToken && (string) $after->get('token') !== $dbToken) {
+    $errors[] = 'DB token changed after update';
+}
+
+if ($errors) {
+    foreach ($errors as $error) {
+        fwrite(STDERR, "FAIL: {$error}\n");
+    }
+    exit(1);
+}
+
+$pass('GET and PUT payloads match (no token, shared shape keys)');
+exit(0);

From 03f997f04347ad0ea83140e9e4fe827beb4a4983 Mon Sep 17 00:00:00 2001
From: Ivan Bochkarev <ivanx86@gmail.com>
Date: Fri, 17 Jul 2026 15:07:31 +0600
Subject: [PATCH 3/3] chore(tests): drop local smoke script from PR

Live MODX smoke stays out of the repo; it is environment-specific.
---
 .../tests/smoke_order_update_payload.php      | 205 ------------------
 1 file changed, 205 deletions(-)
 delete mode 100644 core/components/minishop3/tests/smoke_order_update_payload.php

diff --git a/core/components/minishop3/tests/smoke_order_update_payload.php b/core/components/minishop3/tests/smoke_order_update_payload.php
deleted file mode 100644
index 8488c7ec..00000000
--- a/core/components/minishop3/tests/smoke_order_update_payload.php
+++ /dev/null
@@ -1,205 +0,0 @@
-<?php
-
-/**
- * Live smoke for Manager order GET/PUT payload shape (issue #421).
- *
- * Bootstraps a local MODX install and calls OrdersController directly
- * (no HTTP session required). Optional — needs a real DB with orders.
- *
- * Usage:
- *   MODX_CONFIG_CORE=/path/to/config.core.php \
- *     php tests/smoke_order_update_payload.php [--order-id=N]
- *
- * If MODX_CONFIG_CORE is unset, looks for ../../../../config.core.php
- * relative to this file (core/components/minishop3/tests → site root).
- */
-
-declare(strict_types=1);
-
-$configCore = getenv('MODX_CONFIG_CORE') ?: '';
-if ($configCore === '') {
-    $candidate = dirname(__DIR__, 4) . '/config.core.php';
-    if (is_file($candidate)) {
-        $configCore = $candidate;
-    }
-}
-
-if ($configCore === '' || !is_file($configCore)) {
-    fwrite(STDERR, "SKIP: set MODX_CONFIG_CORE to a MODX config.core.php (live DB required)\n");
-    exit(0);
-}
-
-require $configCore;
-require MODX_CORE_PATH . 'config/' . MODX_CONFIG_KEY . '.inc.php';
-require MODX_CORE_PATH . 'vendor/autoload.php';
-
-use MiniShop3\Controllers\Api\Manager\OrdersController;
-use MiniShop3\Model\msOrder;
-
-$fail = static function (string $message): never {
-    fwrite(STDERR, "FAIL: {$message}\n");
-    exit(1);
-};
-
-$pass = static function (string $message): void {
-    fwrite(STDOUT, "OK: {$message}\n");
-};
-
-$orderId = null;
-foreach ($argv as $arg) {
-    if (str_starts_with($arg, '--order-id=')) {
-        $orderId = (int) substr($arg, 11);
-    }
-}
-
-$modx = new \MODX\Revolution\modX();
-$modx->initialize('mgr');
-
-if (!$modx->services->has('ms3')) {
-    $fail('ms3 service is not registered');
-}
-
-if ($orderId === null || $orderId <= 0) {
-    // Prefer an order that still has a web-order secret in DB (stronger leak check).
-    $c = $modx->newQuery(msOrder::class);
-    $c->where(['token:!=' => '', 'AND:token:IS NOT' => null]);
-    $c->select('id');
-    $c->sortby('id', 'DESC');
-    $c->limit(1);
-    $c->prepare();
-    $c->stmt->execute();
-    $orderId = (int) $c->stmt->fetchColumn();
-    if ($orderId <= 0) {
-        $c = $modx->newQuery(msOrder::class);
-        $c->select('id');
-        $c->sortby('id', 'DESC');
-        $c->limit(1);
-        $c->prepare();
-        $c->stmt->execute();
-        $orderId = (int) $c->stmt->fetchColumn();
-    }
-}
-
-if ($orderId <= 0) {
-    $fail('No order found to smoke-test');
-}
-
-/** @var msOrder|null $before */
-$before = $modx->getObject(msOrder::class, $orderId);
-if (!$before) {
-    $fail("Order #{$orderId} not found");
-}
-
-$dbToken = (string) $before->get('token');
-$requireDbToken = $dbToken !== '';
-
-$controller = new OrdersController($modx);
-
-$get = $controller->get(['id' => $orderId]);
-if (empty($get['success'])) {
-    $fail('GET failed: ' . json_encode($get, JSON_UNESCAPED_UNICODE));
-}
-$getData = $get['data'] ?? null;
-if (!is_array($getData)) {
-    $fail('GET data is not an array');
-}
-
-$hasTokenInGet = array_key_exists('token', $getData);
-$pass(sprintf(
-    'GET #%d success; token in payload: %s',
-    $orderId,
-    $hasTokenInGet ? 'YES (leak)' : 'no'
-));
-
-$oldComment = (string) $before->get('order_comment');
-$marker = '[smoke-421 ' . date('H:i:s') . ']';
-$newComment = $oldComment === '' ? $marker : $oldComment;
-
-$put = $controller->update([
-    'id' => $orderId,
-    'order_comment' => $newComment,
-]);
-
-if (empty($put['success'])) {
-    $fail('PUT failed: ' . json_encode($put, JSON_UNESCAPED_UNICODE));
-}
-$putData = $put['data'] ?? null;
-if (!is_array($putData)) {
-    $fail('PUT data is not an array');
-}
-
-$hasTokenInPut = array_key_exists('token', $putData);
-$pass(sprintf(
-    'PUT #%d success; token in payload: %s',
-    $orderId,
-    $hasTokenInPut ? 'YES (leak)' : 'no'
-));
-
-if ($newComment !== $oldComment) {
-    $controller->update(['id' => $orderId, 'order_comment' => $oldComment]);
-}
-
-$shapeKeysExpected = [
-    'status_name',
-    'delivery_name',
-    'payment_name',
-    'customer',
-    'cost_formatted',
-    'cart_cost_formatted',
-    'delivery_cost_formatted',
-];
-
-$missingInPut = [];
-foreach ($shapeKeysExpected as $key) {
-    if (!array_key_exists($key, $putData)) {
-        $missingInPut[] = $key;
-    }
-}
-
-$putKeys = array_keys($putData);
-$getKeys = array_keys($getData);
-sort($putKeys);
-sort($getKeys);
-
-$onlyInPut = array_values(array_diff($putKeys, $getKeys));
-$onlyInGet = array_values(array_diff($getKeys, $putKeys));
-
-fwrite(STDOUT, 'GET keys count: ' . count($getKeys) . "\n");
-fwrite(STDOUT, 'PUT keys count: ' . count($putKeys) . "\n");
-if ($onlyInPut) {
-    fwrite(STDOUT, 'Only in PUT: ' . implode(', ', $onlyInPut) . "\n");
-}
-if ($onlyInGet) {
-    fwrite(STDOUT, 'Only in GET: ' . implode(', ', $onlyInGet) . "\n");
-}
-
-$errors = [];
-if ($hasTokenInGet) {
-    $errors[] = 'GET still exposes token';
-}
-if ($hasTokenInPut) {
-    $errors[] = 'PUT still exposes token';
-}
-if ($missingInPut) {
-    $errors[] = 'PUT missing GET-shape keys: ' . implode(', ', $missingInPut);
-}
-if ($onlyInPut || $onlyInGet) {
-    $errors[] = 'GET/PUT key sets differ';
-}
-
-$after = $modx->getObject(msOrder::class, $orderId);
-if (!$after) {
-    $errors[] = 'Order missing after update';
-} elseif ($requireDbToken && (string) $after->get('token') !== $dbToken) {
-    $errors[] = 'DB token changed after update';
-}
-
-if ($errors) {
-    foreach ($errors as $error) {
-        fwrite(STDERR, "FAIL: {$error}\n");
-    }
-    exit(1);
-}
-
-$pass('GET and PUT payloads match (no token, shared shape keys)');
-exit(0);