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
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
## 4.9.0

### Fix: ActivityLogger no longer bloats KyteActivityLog, and the admin log list stops dragging blobs (KYTE-#182)

`KyteActivityLog` grew to **10GB on one install and OOM'd the admin log query**. Two independent causes, both fixed here — pure code, no schema change, instant rollback.

1. **Write-cap (the durable bloat fix).** `request_data` and `changes` are `LONGTEXT` and `ActivityLogger` stored the **full** json-encoded request body / diff. A single page or script save carries 300KB+ of HTML/JS/CSS, so each logged row could be hundreds of KB — multiplied across every write, that is what filled the table. `ActivityLogger::log()` now caps each of those two fields at `KYTE_ACTIVITY_LOG_MAX_FIELD_BYTES` (default **16384** bytes, auto-defaulted in `Api`). When the encoded value exceeds the cap it is replaced with a small audit-preserving marker — `{"_truncated":true,"_original_bytes":N,"_fields":[...]}` — so you still see *what* was sent or changed (the top-level field names and the original size), just not the megabyte of content. Set the constant to `0` to disable the limit. Redaction (SensitivityPolicy + the hardcoded `SENSITIVE_FIELDS` baseline) is unchanged and still runs before the cap.

2. **List-view projection (the OOM fix).** The framework `SELECT *` (`DBI::select`) pulls both `LONGTEXT` columns for every row, and the admin log **list** never uses them — only the single-record **detail** view does. `KyteActivityLogController` now detects a by-`id` detail fetch in `hook_prequery` and, on every other (list) response, omits `request_data`/`changes` and skips the JSON decode. The detail view (Shipyard `get("KyteActivityLog","id",idx)`) still returns the full decoded payload, so no Shipyard change is required. Severity/action colors are still added to both list and detail rows.

**Not in this release (deferred to the data-model initiative, KYTE-#190):** index coverage on `KyteActivityLog`'s filter/sort columns and a retention policy. The earlier one-off ETOM purge (10.4GB → 1.56GB) was the band-aid; the write-cap above is what prevents recurrence.

## 4.8.1

### Fix: republish is now fault-isolated (KYTE-#181) + partial content saves no longer blank `block_layout` (KYTE-#189)
Expand Down
44 changes: 42 additions & 2 deletions src/Core/ActivityLogger.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public function log($action, $modelName, $field, $value, $requestData, $response
$redacted = is_array($requestData)
? SensitivityPolicy::getInstance()->redactFields($requestData, $modelName, $this->accountId)
: $requestData;
$requestDataForLog = json_encode($this->redactSensitive($redacted));
$requestDataForLog = $this->capField(json_encode($this->redactSensitive($redacted)));
}

$logData = [
Expand Down Expand Up @@ -172,7 +172,7 @@ public function log($action, $modelName, $field, $value, $requestData, $response
if ($action === 'PUT' && !$shouldDrop && $this->preUpdateState !== null && is_array($requestData)) {
$changes = $this->computeChanges($this->preUpdateState, $requestData, $modelName);
if (!empty($changes)) {
$logData['changes'] = json_encode($changes);
$logData['changes'] = $this->capField(json_encode($changes));
}
}

Expand Down Expand Up @@ -232,6 +232,46 @@ public function logAuth($action, $email, $success, $errorMessage = null) {
$this->isLogging = false;
}

/**
* Cap an encoded JSON field to KYTE_ACTIVITY_LOG_MAX_FIELD_BYTES.
*
* `request_data` and `changes` are LONGTEXT and otherwise store the FULL
* request body / diff — a single page or script save carries 300KB+ of
* HTML/JS/CSS, which is what let KyteActivityLog grow to 10GB and OOM the
* admin log query (KYTE-#182). When the encoded value exceeds the cap it
* is replaced with a small audit-preserving marker: the original byte
* size and the top-level field names (so you still see WHAT was sent /
* changed, just not the megabyte of content). A cap <= 0 disables the
* limit. Bytes, not characters — we are capping storage size.
*/
private function capField($encoded) {
if ($encoded === null) {
return null;
}
$max = defined('KYTE_ACTIVITY_LOG_MAX_FIELD_BYTES')
? (int)KYTE_ACTIVITY_LOG_MAX_FIELD_BYTES
: 16384;
if ($max <= 0 || strlen($encoded) <= $max) {
return $encoded;
}

$fields = null;
$decoded = json_decode($encoded, true);
if (is_array($decoded)) {
$fields = array_keys($decoded);
// Guard against the marker itself ballooning on a very wide payload.
if (count($fields) > 50) {
$fields = array_slice($fields, 0, 50);
}
}

return json_encode([
'_truncated' => true,
'_original_bytes' => strlen($encoded),
'_fields' => $fields,
]);
}

/**
* Redact sensitive fields from data
*/
Expand Down
1 change: 1 addition & 0 deletions src/Core/Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ class Api
'STRICT_TYPING' => true,
'KYTE_USE_SNS' => false,
'AUTH_STRATEGY_DISPATCHER' => 'off',
'KYTE_ACTIVITY_LOG_MAX_FIELD_BYTES' => 16384,
];

/**
Expand Down
21 changes: 21 additions & 0 deletions src/Mvc/Controller/KyteActivityLogController.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ class KyteActivityLogController extends ModelController
'LOGIN_FAIL' => '#dc3545'
];

// Set true in hook_prequery when a single record is fetched by primary
// key (the Shipyard detail view does `get("KyteActivityLog","id",idx)`).
// The heavy LONGTEXT columns (request_data, changes) are returned only in
// that case; list responses omit them so a page of logs doesn't drag a
// blob per row into memory. See KYTE-#182.
private $isDetailView = false;

public function hook_init() {
$this->requireAccount = false;
$this->dateformat = 'm/d/Y H:i:s';
Expand All @@ -54,6 +61,10 @@ public function hook_prequery($method, &$field, &$value, &$conditions, &$all, &$
throw new \Exception("Unauthorized request method: {$method}");
}

// Single-record detail fetch (by primary key) gets the full payload;
// everything else is treated as a list and projected down. KYTE-#182.
$this->isDetailView = ($field === 'id' && $value !== null && $value !== '');

$query = [];

// Account scoping
Expand Down Expand Up @@ -179,6 +190,16 @@ public function hook_response_data($method, $o, &$r = null, &$d = null) {
$r['action_color'] = self::ACTION_COLORS[$o->action] ?? '#6c757d';
}

// List projection (KYTE-#182): the heavy LONGTEXT columns are only
// consumed by the single-record detail view. On a list response, drop
// the raw blobs and skip the decode entirely so a page of rows can't
// pull a request body per row into memory. The detail fetch (by id)
// still returns the decoded payload below.
if (!$this->isDetailView) {
unset($r['request_data'], $r['changes']);
return;
}

// Decode request_data JSON if present
$r['request_data_decoded'] = null;
if (!empty($o->request_data)) {
Expand Down
136 changes: 136 additions & 0 deletions tests/ActivityLoggerWriteCapTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
<?php
namespace Kyte\Test;

use Kyte\Core\ActivityLogger;
use Kyte\Core\Api;
use Kyte\Core\SensitivityPolicy;
use PHPUnit\Framework\TestCase;

/**
* Coverage for the ActivityLogger write-cap (KYTE-#182).
*
* request_data and changes are LONGTEXT. Before the cap, ActivityLogger
* stored the FULL json-encoded request body / diff — a single page or
* script save carries 300KB+ of HTML/JS/CSS, which is what let
* KyteActivityLog grow to 10GB and OOM the admin log query. The cap
* (KYTE_ACTIVITY_LOG_MAX_FIELD_BYTES, default 16KB) replaces an
* over-limit value with a small audit-preserving marker that records the
* original byte size and the top-level field names.
*
* These tests drive the real ActivityLogger::log() against the real
* KyteActivityLog table and read the row back to assert what was stored.
* The model name used here is NOT flagged sensitive, so the payload is
* captured (then capped) rather than dropped — exercising exactly the cap
* path. Sensitive-drop behavior is covered separately in
* ActivityLoggerSensitivityTest.
*/
class ActivityLoggerWriteCapTest extends TestCase
{
private const ACCOUNT = 'al-cap-test';
private const MODEL = 'AlCapTestModel';

private Api $api;
private int $accountId;

protected function setUp(): void
{
\Kyte\Core\DBI::dbInit(KYTE_DB_USERNAME, KYTE_DB_PASSWORD, KYTE_DB_HOST, KYTE_DB_DATABASE, KYTE_DB_CHARSET, 'InnoDB');

$this->api = new Api();

\Kyte\Core\DBI::createTable(KyteAccount);
\Kyte\Core\DBI::createTable(Controller);
\Kyte\Core\DBI::createTable(DataModel);
\Kyte\Core\DBI::createTable(ModelAttribute);
\Kyte\Core\DBI::createTable(KyteActivityLog);

\Kyte\Core\DBI::query("DELETE FROM `KyteAccount` WHERE number = '" . self::ACCOUNT . "'");
\Kyte\Core\DBI::query("DELETE FROM `KyteActivityLog` WHERE model_name = '" . self::MODEL . "'");

$acct = new \Kyte\Core\ModelObject(KyteAccount);
$acct->create(['number' => self::ACCOUNT, 'name' => 'AL Cap Test']);
$this->accountId = (int)$acct->id;

SensitivityPolicy::resetForTests();

$this->api->account = new \Kyte\Core\ModelObject(KyteAccount);
$this->api->account->retrieve('id', $this->accountId);
ActivityLogger::getInstance()->setContext($this->api);
}

public function testOversizedRequestDataIsTruncatedToMarker(): void
{
// ~20KB of content in one field — comfortably over the 16KB cap.
$big = str_repeat('A', 20000);

ActivityLogger::getInstance()->log(
'POST', self::MODEL, null, null,
['html' => $big, 'route' => '/some/page'],
200, 'success'
);

$row = $this->latestLogRow();
$this->assertNotNull($row, 'log row was written');

$decoded = json_decode($row['request_data'], true);
$this->assertIsArray($decoded);
$this->assertTrue($decoded['_truncated'] ?? false, 'oversized request_data → truncation marker');
$this->assertGreaterThan(20000, $decoded['_original_bytes'], 'marker records original byte size');
$this->assertContains('html', $decoded['_fields'], 'marker preserves which fields were present');
$this->assertContains('route', $decoded['_fields']);
// The stored value itself must be small — that is the whole point.
$this->assertLessThan(2000, strlen($row['request_data']), 'stored marker is small');
}

public function testWithinLimitRequestDataIsStoredVerbatim(): void
{
ActivityLogger::getInstance()->log(
'POST', self::MODEL, null, null,
['note' => 'a small, normal payload', 'count' => 3],
200, 'success'
);

$row = $this->latestLogRow();
$this->assertNotNull($row);

$decoded = json_decode($row['request_data'], true);
$this->assertArrayNotHasKey('_truncated', $decoded, 'within-limit payload is not marked truncated');
$this->assertSame('a small, normal payload', $decoded['note']);
$this->assertSame(3, $decoded['count']);
}

public function testOversizedChangesDiffIsTruncatedToMarker(): void
{
$logger = ActivityLogger::getInstance();

// Simulate the pre-update capture Api.php performs before a PUT.
$reflection = new \ReflectionClass($logger);
$prop = $reflection->getProperty('preUpdateState');
$prop->setAccessible(true);
$prop->setValue($logger, ['javascript' => 'old']);

$logger->log(
'PUT', self::MODEL, 'id', 1,
['javascript' => str_repeat('B', 20000)],
200, 'success'
);

$row = $this->latestLogRow();
$this->assertNotNull($row);

$decoded = json_decode($row['changes'], true);
$this->assertTrue($decoded['_truncated'] ?? false, 'oversized changes diff → truncation marker');
$this->assertContains('javascript', $decoded['_fields']);
}

private function latestLogRow(): ?array
{
$rows = \Kyte\Core\DBI::query(
"SELECT * FROM `KyteActivityLog` WHERE model_name = '" . self::MODEL . "' ORDER BY id DESC LIMIT 1"
);
if (!is_array($rows) || count($rows) === 0) {
return null;
}
return $rows[0];
}
}
Loading