From 48f9178a946fc523e7ed994bd25434705aa2b72b Mon Sep 17 00:00:00 2001 From: Kenneth Hough Date: Thu, 28 May 2026 02:54:04 -0500 Subject: [PATCH] fix(shipyard-api): sensitive toggle / metadata-only PUT fails on DataModel + ModelAttribute Toggling sensitive=1 on a model or field never persisted: PUT returned 200 with an empty body and the toggle reverted with "Save failed." Two update hooks assumed every update carries the full record: - DataModelController::hook_preprocess(update): `$o->name != $r['name']` ran the table-rename path. A partial PUT of just {sensitive:1} omits name, so the compare was true vs null -> renameTable($o->name, null) throws "New table name cannot be empty" BEFORE $obj->save() persists sensitive. Now gated on isset($r['name']) && $o->name != $r['name']. - ModelAttributeController::hook_preprocess(update): unconditionally ran changeColumn($tbl->name, $o->name, $r['name'], $attrs) on every update. A metadata-only PUT (no name) tried to rename the column to an empty name with an incomplete definition and threw. CHANGE COLUMN now gated on isset($r['name']); the field-edit form (always sends name) is unaffected. The empty-200 was the swallowed hook exception. ControllerController has no schema-altering update hook, so Controller sensitive toggles were never affected. Blocks enabling model/field-level sensitive-data redaction (policy reads these flags at runtime). No schema change. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 14 +++++ src/Mvc/Controller/DataModelController.php | 9 +++- .../Controller/ModelAttributeController.php | 53 +++++++++++-------- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56cd4dee..62e8acf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,17 @@ +## 4.5.1 + +### Bug Fix: `sensitive` toggle (and any metadata-only PUT) silently fails on DataModel / ModelAttribute + +Toggling `sensitive=1` on a model (Settings tab) or field never persisted: the PUT returned HTTP 200 with an empty body, and the Shipyard toggle reverted with "Save failed." Root cause is in two update hooks that assumed every update carries the full record: + +- **`DataModelController::hook_preprocess` (update):** `if ($o->name != $r['name'])` ran the table-rename path. A partial PUT of just `{sensitive:1}` omits `name`, so the comparison was true against `null` → `DBI::renameTable($o->name, null)` → throws *"New table name cannot be empty"* — **before** `$obj->save()` persisted `sensitive`. Now gated on `isset($r['name']) && $o->name != $r['name']`. + +- **`ModelAttributeController::hook_preprocess` (update):** unconditionally ran `DBI::changeColumn($tbl->name, $o->name, $r['name'], $attrs)` on every update. A metadata-only PUT (no `name`) tried to rename the column to an empty name with an incomplete definition and threw. The CHANGE COLUMN path is now gated on `isset($r['name'])`; the field-edit form (which always sends `name`) is unaffected. + +Why it surfaced now: this is the first feature to PUT a single metadata field on these meta-models. The empty-200 was the swallowed hook exception (thrown after the 200 status path but before the response body was serialized). `Controller` sensitive toggles were never affected — `ControllerController` has no schema-altering update hook. + +Impact: blocks enabling sensitive-data redaction at the model/field level (the redaction policy reads these flags at runtime). No schema change. Composer upgrade is sufficient. + ## 4.5.0 ### Feature: JWT session lifetime caps (inactivity + absolute) diff --git a/src/Mvc/Controller/DataModelController.php b/src/Mvc/Controller/DataModelController.php index 7ace880d..97087bbe 100644 --- a/src/Mvc/Controller/DataModelController.php +++ b/src/Mvc/Controller/DataModelController.php @@ -124,7 +124,14 @@ public function hook_preprocess($method, &$r, &$o = null) { throw new \Exception("Unknown model definition"); } - if ($o->name != $r['name']) { + // Only run the table-rename path when the request actually + // carries a (different) name. A metadata-only partial PUT — + // e.g. the Settings tab toggling `sensitive` — omits `name`, + // so `$o->name != $r['name']` would be true against a null and + // wrongly trigger renameTable($o->name, null), which throws + // "New table name cannot be empty" and aborts the save before + // `sensitive` is ever persisted. + if (isset($r['name']) && $o->name != $r['name']) { // check if new name is unique if (defined($r['name'])) { throw new \Exception("New model name conflicts with existing model or system model name."); diff --git a/src/Mvc/Controller/ModelAttributeController.php b/src/Mvc/Controller/ModelAttributeController.php index ce00438a..3192994f 100644 --- a/src/Mvc/Controller/ModelAttributeController.php +++ b/src/Mvc/Controller/ModelAttributeController.php @@ -50,28 +50,39 @@ public function hook_preprocess($method, &$r, &$o = null) { break; case 'update': - $tbl = new \Kyte\Core\ModelObject(DataModel); - if (!$tbl->retrieve('id', $o->dataModel)) { - throw new \Exception("Unable to find associated data model."); - } - - $attrs = \Kyte\Mvc\Controller\DataModelController::prepareModelDef((object)$r); - - // switch dbs - $app = new \Kyte\Core\ModelObject(Application); - if (!$app->retrieve('id', $tbl->application)) { - throw new \Exception("CRITICAL ERROR: Unable to find application and perform context switch."); + // Only touch the physical column when the request actually + // carries the column `name`. The CHANGE COLUMN path needs the + // full column definition; a metadata-only partial PUT — e.g. + // toggling `sensitive` on a field — omits `name`, so the old + // unconditional changeColumn($tbl->name, $o->name, null, ...) + // tried to rename the column to an empty name with an + // incomplete definition and threw, aborting the save before + // `sensitive` was persisted. The field-edit form always sends + // `name` for real schema changes, so this preserves that path. + if (isset($r['name'])) { + $tbl = new \Kyte\Core\ModelObject(DataModel); + if (!$tbl->retrieve('id', $o->dataModel)) { + throw new \Exception("Unable to find associated data model."); + } + + $attrs = \Kyte\Mvc\Controller\DataModelController::prepareModelDef((object)$r); + + // switch dbs + $app = new \Kyte\Core\ModelObject(Application); + if (!$app->retrieve('id', $tbl->application)) { + throw new \Exception("CRITICAL ERROR: Unable to find application and perform context switch."); + } + + \Kyte\Core\Api::dbappconnect($app->db_name, $app->db_username, $app->db_password, $app->db_host ? $app->db_host : null); + \Kyte\Core\Api::dbswitch(true); + + // alter the underlying column to match the new definition + if (!\Kyte\Core\DBI::changeColumn($tbl->name, $o->name, $r['name'], $attrs)) { + throw new \Exception("Failed to change column {$o->name} to {$r['name']} in table {$tbl->name}..."); + } + // return to kyte db + \Kyte\Core\Api::dbswitch(); } - - \Kyte\Core\Api::dbappconnect($app->db_name, $app->db_username, $app->db_password, $app->db_host ? $app->db_host : null); - \Kyte\Core\Api::dbswitch(true); - - // create new table with basic kyte info - if (!\Kyte\Core\DBI::changeColumn($tbl->name, $o->name, $r['name'], $attrs)) { - throw new \Exception("Failed to change column {$o->name} to {$r['name']} in table {$tbl->name}..."); - } - // return to kyte db - \Kyte\Core\Api::dbswitch(); break; default: