Skip to content
Merged
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
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
## 4.4.2

### Bug Fix: ErrorHandler crash when `apiContext->key` is a ModelObject

The enhanced v4.4 ErrorHandler bound `$this->apiContext->key` directly into the `KyteError.api_key` string column. `$this->key` is set in `Api.php` as `new \Kyte\Core\ModelObject(KyteAPIKey)` — an object, not a string. `mysqli_stmt::execute()` then threw `Object of class Kyte\Core\ModelObject could not be converted to string` *inside* the error handler. That secondary fatal swallowed the original error and surfaced to clients as a **blank HTTP 500** with no log row written and no stack trace recoverable.

Customer-visible impact: any request that triggered the error handler — including expected exceptions on routes like POST `/Session`, GET on a controller hitting a runtime error, etc. — returned 500 with no body. Production traffic at ETOM was affected from the v4.4.0 deploy onward.

Fix: `'api_key' => isset($this->apiContext->key->public_key) ? (string)$this->apiContext->key->public_key : null` — extract the scalar `public_key` string (which is the audit-relevant value anyway). Falls back to `null` when the ModelObject hasn't been retrieved or when there's no key context at all.

Regression test `ErrorHandlerSensitivityTest::testApiContextKeyAsModelObjectLogsPublicKeyString` exercises the previously-broken ModelObject path end to end. Pre-existing tests set `$context->key = null` and never hit the crash, which is why the regression slipped through 4.4.0 and 4.4.1.

No schema changes. Composer upgrade is sufficient.

## 4.4.1

### Bug Fix: CORS preflight on `/jwt/*` endpoints
Expand Down
13 changes: 11 additions & 2 deletions src/Exception/ErrorHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,14 @@ public function handleException($exception) {
'kyte_account' => isset($this->apiContext->account->id) ? $this->apiContext->account->id : null,
'user_id' => isset($this->apiContext->user->id) ? $this->apiContext->user->id : null,
'app_id' => isset($this->apiContext->appId) ? $this->apiContext->appId : null,
'api_key' => isset($this->apiContext->key) ? $this->apiContext->key : null,
// $apiContext->key is a ModelObject(KyteAPIKey) — extract the
// scalar public_key string for logging. Binding the bare object
// would crash mysqli_stmt::execute() with
// "Object of class Kyte\Core\ModelObject could not be converted
// to string" because KyteError.api_key is a varchar column.
// That secondary fatal inside the error handler swallows the
// original error and surfaces as a blank HTTP 500 to the client.
'api_key' => isset($this->apiContext->key->public_key) ? (string)$this->apiContext->key->public_key : null,
// Additional details
'signature' => isset($this->apiContext->signature) ? $this->apiContext->signature : null,
'contentType' => isset($this->apiContext->contentType) ? $this->apiContext->contentType : null,
Expand Down Expand Up @@ -133,7 +140,9 @@ public function handleError($errno, $errstr, $errfile, $errline) {
'kyte_account' => isset($this->apiContext->account->id) ? $this->apiContext->account->id : null,
'user_id' => isset($this->apiContext->user->id) ? $this->apiContext->user->id : null,
'app_id' => isset($this->apiContext->appId) ? $this->apiContext->appId : null,
'api_key' => isset($this->apiContext->key) ? $this->apiContext->key : null,
// See handleException — $apiContext->key is a ModelObject;
// extract the scalar public_key string to avoid the same crash.
'api_key' => isset($this->apiContext->key->public_key) ? (string)$this->apiContext->key->public_key : null,
'request' => isset($this->apiContext->request) ? $this->apiContext->request : null,
'model' => $modelName,
'message' => $errstr,
Expand Down
43 changes: 43 additions & 0 deletions tests/ErrorHandlerSensitivityTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,49 @@ public function testNoFlagsPreservesExistingBehavior(): void
$this->assertStringContainsString('ok', $row['response']);
}

/**
* Regression: $apiContext->key is a ModelObject(KyteAPIKey) in real
* Api flow, but the enhanced v4.4 ErrorHandler bound it directly into
* the KyteError.api_key string column. mysqli_stmt::execute() then
* threw "Object of class Kyte\Core\ModelObject could not be converted
* to string" — a secondary fatal that swallowed the original error
* and surfaced as a blank HTTP 500 to the client.
*
* Pre-existing tests set $context->key = null, so they didn't catch
* the regression. This test exercises the ModelObject path and
* asserts the api_key column gets the scalar public_key value.
*/
public function testApiContextKeyAsModelObjectLogsPublicKeyString(): void
{
\Kyte\Core\DBI::createTable(KyteAPIKey);
\Kyte\Core\DBI::query("DELETE FROM `KyteAPIKey` WHERE public_key = 'eh-key-test-pub-12345'");

$key = new \Kyte\Core\ModelObject(KyteAPIKey);
$key->create([
'identifier' => 'eh-key-test-iden',
'public_key' => 'eh-key-test-pub-12345',
'secret_key' => 'eh-key-test-sec',
'epoch' => time(),
'kyte_account' => $this->accountId,
]);

$context = $this->buildContext(self::PLAIN_MODEL);
$context->key = $key; // ← the real Api.php flow
$context->data = ['note' => 'whatever'];
$context->response = ['status' => 'ok'];

$handler = ErrorHandler::getInstance($context);
// Without the fix, this throws ValueError/TypeError from mysqli
// and the error row is never written.
$handler->handleException(new \RuntimeException('regression check'));

$row = $this->latestErrorRow(self::PLAIN_MODEL);
$this->assertNotNull($row, 'error row must be written even when context->key is a ModelObject');
$this->assertSame('eh-key-test-pub-12345', $row['api_key'],
'api_key column must hold the scalar public_key string, not a stringified ModelObject');
$this->assertSame('regression check', $row['message'], 'original error message must survive');
}

public function testAIDefenseInDepthGateBlocksSensitiveContext(): void
{
// Drive AIErrorCorrection::queueForAnalysis directly with a
Expand Down
Loading