Fix: MCP commit_draft shipped raw bzip2 bytes into published HTML (4.10.1)

CHANGELOG.md

@@ -1,3 +1,19 @@
+## 4.10.1
+
+### Fix: MCP commit_draft published raw bzip2 bytes into page HTML (header/footer section CSS not decompressed)
+
+Publishing a page through the MCP `commit_draft` flow could inject raw bzip2-compressed binary (magic `BZh9…`) into the published HTML, inside `<style>`/`<footer>` blocks, producing a browser UTF-8 decode error and garbled header/footer CSS. Surfaced on FrameVTO / doctor.etometry.com (page 58, published to v13).
+
+**Root cause.** A page's `header`/`footer` are FKs to `KyteSectionTemplate`, which stores `html`/`stylesheet`/`javascript`/`block_layout` **bzip2-compressed**. `getObject()`'s FK expansion returns those fields RAW. The page-assembly path (`createHtml` → `buildHeaderFooterStyles` etc.) concatenates them straight into the output, so they must be decompressed first. The **human publish** path (HTTP `update`, `state=1`) was saved only incidentally — `KytePageController::hook_response_data()` runs first and decompresses `$r['header']`/`$r['footer']`. The **MCP commit** path (`DraftService::commitDraft` → `publishForSurface` → `KytePageController::publishFromContent`) calls `getObject()` and goes straight to `publishPage()`, never invoking that hook — so the compressed bytes shipped. Deterministic per-path (not a race): any page with a populated header/footer section template was affected; the "re-publish cleared it" report was a re-publish via the human path.
+
+**Fixes (`KytePageController`, `S3`):**
+1. **Path-independent decompression.** New `decompressSectionTemplate(&$section)` decompresses header/footer content via `Bz2Codec::decompressIfBz2`, called from BOTH `hook_response_data()` and `publishFromContent()`. The page's own html/stylesheet/javascript were already correct (decompressed by `DraftService::versionContent`); only the section templates leaked.
+2. **Latent guard bug fixed.** The old `hook_response_data` block required *all four* fields (`html`, `stylesheet`, `javascript`, `block_layout`) to be set or it decompressed *none* — a null `block_layout` would have leaked the other three even on the HTTP path. The new helper decompresses each field independently.
+3. **Output integrity guard.** `publishPage()` now runs `hasBinaryContamination()` on the assembled HTML (bzip2 stream magic `BZh[1-9]1AY&SY`, or invalid UTF-8) and **aborts before the S3 write** (returns false → MCP commit reports `committed:false`, draft left intact) rather than shipping corrupt HTML.
+4. **Charset.** Published HTML objects are now written with `Content-Type: text/html; charset=utf-8` (`S3::write()` gained an optional content-type passed through the stream-wrapper context; other callers unchanged).
+
+Tests: `tests/PublishIntegrityTest.php` covers the per-field decompression (incl. the null-`block_layout` regression) and the contamination detector (embedded bzip2 stream, invalid UTF-8, and no false-positive on literal "BZh" prose).
+
 ## 4.10.0
 
 ### Feature: MCP draft/write — AI can draft and commit pages, controller functions, and scripts

phpunit.xml.dist

@@ -22,6 +22,7 @@
       <file>tests/McpTokenControllerTest.php</file>
       <file>tests/McpTokenStrategyTest.php</file>
       <file>tests/ModelTest.php</file>
+      <file>tests/PublishIntegrityTest.php</file>
       <file>tests/RefreshTokenStoreTest.php</file>
       <file>tests/SaveSafeSessionTest.php</file>
       <file>tests/SensitivityPolicyTest.php</file>

src/Aws/S3.php

@@ -203,13 +203,13 @@ public function deleteBucket() {
     }
 
     // use S3 stream wrapper to write to bucket path
-    public function write($key, $data) {
+    public function write($key, $data, $contentType = null) {
         // check if bucket exists
         if (!$this->bucket) {
             throw new \Exception('bucket must be defined');
         }
 
-        return $this->streamWrite($key, $data, 'w');
+        return $this->streamWrite($key, $data, 'w', $contentType);
     }
 
     // use S3 stream wrapper to append to bucket path
@@ -223,10 +223,19 @@ public function append($key, $data) {
     }
 
     // use S3 stream wrapper to write/append to bucket path
-    private function streamWrite($key, $data, $flag) {
+    private function streamWrite($key, $data, $flag, $contentType = null) {
         $this->client->registerStreamWrapper();
 
-        $stream = fopen('s3://'.$this->bucket.'/'.$key, $flag);
+        // When a content type is supplied, pass it through the stream context so
+        // the underlying PutObject sets Content-Type (e.g. published HTML gets
+        // "text/html; charset=utf-8" instead of S3's extension-inferred default
+        // without a charset). Other callers keep the prior behaviour.
+        if ($contentType !== null) {
+            $context = stream_context_create(['s3' => ['ContentType' => $contentType]]);
+            $stream = fopen('s3://'.$this->bucket.'/'.$key, $flag, false, $context);
+        } else {
+            $stream = fopen('s3://'.$this->bucket.'/'.$key, $flag);
+        }
         fwrite($stream, $data);
         return fclose($stream);
     }

src/Mvc/Controller/KytePageController.php

@@ -49,24 +49,14 @@ public function hook_preprocess($method, &$r, &$o = null) {
     }
 
     public function hook_response_data($method, $o, &$r = null, &$d = null) {
-        if (isset($r['footer']) && is_array($r['footer']) &&
-            isset($r['footer']['html'], $r['footer']['stylesheet'], $r['footer']['javascript'],
-                $r['footer']['block_layout'])) {
-
-            $r['footer']['html'] = bzdecompress($r['footer']['html']);
-            $r['footer']['stylesheet'] = bzdecompress($r['footer']['stylesheet']);
-            $r['footer']['javascript'] = bzdecompress($r['footer']['javascript']);
-            $r['footer']['block_layout'] = bzdecompress($r['footer']['block_layout']);
+        // The header/footer section templates are FK-expanded by getObject()
+        // with their content still bzip2-compressed; decompress in place so
+        // callers (API consumers AND the publish/assembly path) get plaintext.
+        if (isset($r['footer'])) {
+            self::decompressSectionTemplate($r['footer']);
         }
-
-        if (isset($r['header']) && is_array($r['header']) &&
-            isset($r['header']['html'], $r['header']['stylesheet'], $r['header']['javascript'],
-                $r['header']['block_layout'])) {
-
-            $r['header']['html'] = bzdecompress($r['header']['html']);
-            $r['header']['stylesheet'] = bzdecompress($r['header']['stylesheet']);
-            $r['header']['javascript'] = bzdecompress($r['header']['javascript']);
-            $r['header']['block_layout'] = bzdecompress($r['header']['block_layout']);
+        if (isset($r['header'])) {
+            self::decompressSectionTemplate($r['header']);
         }
 
         switch ($method) {
@@ -262,9 +252,65 @@ public function hook_response_data($method, $o, &$r = null, &$d = null) {
         }
     }
 
+    /**
+     * Decompress a FK-expanded header/footer section template in place.
+     *
+     * KyteSectionTemplate stores html/stylesheet/javascript/block_layout
+     * bzip2-compressed (compress-on-write). getObject()'s FK expansion returns
+     * those fields RAW, so every code path that assembles a page from an
+     * expanded $page array — the HTTP response hook AND the MCP commit publish
+     * path (publishFromContent) — must decompress them first. Skipping this is
+     * what let raw "BZh…" bytes land inside the published <style>/<footer>
+     * blocks when committing a draft over MCP (the human publish path was
+     * saved only by hook_response_data running first).
+     *
+     * Each field is decompressed INDEPENDENTLY via Bz2Codec (idempotent and
+     * safe on already-plaintext or non-bzip2 rows) — unlike the previous
+     * all-or-nothing isset() guard, a null block_layout no longer suppresses
+     * decompression of the other three fields.
+     *
+     * @param mixed $section The expanded section array (by reference). Non-array
+     *                       values (e.g. an unexpanded id) are left untouched.
+     */
+    private static function decompressSectionTemplate(&$section) {
+        if (!is_array($section)) {
+            return;
+        }
+        foreach (['html', 'stylesheet', 'javascript', 'block_layout'] as $field) {
+            if (isset($section[$field])) {
+                $section[$field] = \Kyte\Mcp\Util\Bz2Codec::decompressIfBz2($section[$field]);
+            }
+        }
+    }
+
+    /**
+     * Detect whether assembled page HTML has been contaminated with binary
+     * content that must never be published. Two independent signals:
+     *
+     *  1. A bzip2 stream header — "BZh" + a level digit (1-9) + the block
+     *     magic "1AY&SY" (0x314159265359). This full signature avoids
+     *     false-positives on legitimate prose that merely contains "BZh".
+     *  2. The string is not valid UTF-8 — a catch-all for any non-text bytes
+     *     (raw compressed payloads are never valid UTF-8).
+     *
+     * @param string $html The fully assembled page HTML.
+     * @return bool True if the HTML must NOT be published.
+     */
+    public static function hasBinaryContamination($html) {
+        if (!is_string($html) || $html === '') {
+            return false;
+        }
+        if (strpos($html, "BZh") !== false &&
+            preg_match('/BZh[1-9]1AY&SY/', $html) === 1) {
+            return true;
+        }
+        // preg_match with the /u modifier returns false (not 0) on invalid UTF-8.
+        return preg_match('//u', $html) !== 1;
+    }
+
     /**
      * Publish a page to S3 and invalidate CloudFront cache
-     * 
+     *
      * @param object $pageObj The page object being published
      * @param array $params Parameters including page content and metadata
      * @param array $responseData Response data containing site information
@@ -289,6 +335,19 @@ public function publishFromContent($pageObj, array $content) {
         // Expand the page exactly as the normal publish path does (FK chain:
         // site → application, header/footer/navigation templates, etc.).
         $r = $this->getObject($pageObj);
+
+        // getObject() returns the FK-expanded header/footer with their content
+        // still bzip2-compressed. The HTTP publish path is decompressed by
+        // hook_response_data() before it reaches publishPage(); this internal
+        // path never runs that hook, so decompress here — otherwise the raw
+        // compressed bytes get assembled straight into the published HTML.
+        if (isset($r['header'])) {
+            self::decompressSectionTemplate($r['header']);
+        }
+        if (isset($r['footer'])) {
+            self::decompressSectionTemplate($r['footer']);
+        }
+
         $params = $r;
         $params['html']       = isset($content['html']) ? $content['html'] : '';
         $params['stylesheet'] = isset($content['stylesheet']) ? $content['stylesheet'] : '';
@@ -358,12 +417,22 @@ public static function publishPage($pageObj, $params, &$responseData) {
 
         // Compile HTML file
         $compiledHtml = self::createHtml($params);
-
+
+        // Integrity guard: never ship corrupt output. If an upstream decompress
+        // failed and raw bzip2 bytes (or any non-UTF-8) made it into the
+        // assembled HTML, abort BEFORE writing to S3/CloudFront. Returning
+        // false (rather than throwing) lets the MCP commit_draft flow surface
+        // committed:false and leave the draft intact for a retry.
+        if (self::hasBinaryContamination($compiledHtml)) {
+            error_log("KytePageController::publishPage aborted for s3key '{$pageObj->s3key}': assembled HTML failed integrity check (bzip2 magic or invalid UTF-8). Publish NOT written.");
+            return false;
+        }
+
         // Write compiled HTML to S3. Capture the result: the S3 stream wrapper
         // returns false on a failed upload (e.g. invalid credentials) rather
         // than throwing, so callers that need to know whether the publish
         // actually landed (the MCP commit_draft flow) can check the return.
-        $publishOk = $s3->write($pageObj->s3key, $compiledHtml);
+        $publishOk = $s3->write($pageObj->s3key, $compiledHtml, 'text/html; charset=utf-8');
 
         // Update sitemap
         $siteDomain = $responseData['site']['aliasDomain'] ?: $responseData['site']['cfDomain'];

tests/PublishIntegrityTest.php

@@ -0,0 +1,109 @@
+<?php
+namespace Kyte\Test;
+
+use Kyte\Mvc\Controller\KytePageController;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Tests for the publish-time defenses against shipping bzip2-compressed
+ * section-template content into published HTML (the MCP commit_draft bug):
+ *
+ *  - decompressSectionTemplate(): the FK-expanded header/footer arrays carry
+ *    their content still bzip2-compressed; this must decompress every field
+ *    independently (a null block_layout must NOT suppress the others, the old
+ *    all-or-nothing isset() guard's latent failure).
+ *  - hasBinaryContamination(): the last-line guard in publishPage() that
+ *    aborts a publish if raw bzip2 bytes / non-UTF-8 reached the output.
+ */
+class PublishIntegrityTest extends TestCase
+{
+    private function decompressSection(array $section): array
+    {
+        $m = new \ReflectionMethod(KytePageController::class, 'decompressSectionTemplate');
+        $m->setAccessible(true);
+        $args = [&$section];
+        $m->invokeArgs(null, $args);
+        return $section;
+    }
+
+    public function testDecompressesEveryCompressedField(): void
+    {
+        $section = [
+            'html'         => bzcompress('<b>hi</b>', 9),
+            'stylesheet'   => bzcompress('footer{color:red}', 9),
+            'javascript'   => bzcompress('var x=1;', 9),
+            'block_layout' => bzcompress('{}', 9),
+        ];
+
+        $out = $this->decompressSection($section);
+
+        $this->assertSame('<b>hi</b>', $out['html']);
+        $this->assertSame('footer{color:red}', $out['stylesheet']);
+        $this->assertSame('var x=1;', $out['javascript']);
+        $this->assertSame('{}', $out['block_layout']);
+    }
+
+    public function testNullBlockLayoutDoesNotSuppressOtherFields(): void
+    {
+        // Regression for the latent all-or-nothing guard: a missing/null
+        // block_layout used to skip decompression of html/stylesheet/javascript
+        // entirely, shipping their raw compressed bytes.
+        $section = [
+            'html'         => bzcompress('<b>hi</b>', 9),
+            'stylesheet'   => bzcompress('footer{color:red}', 9),
+            'javascript'   => bzcompress('var x=1;', 9),
+            'block_layout' => null,
+        ];
+
+        $out = $this->decompressSection($section);
+
+        $this->assertSame('<b>hi</b>', $out['html']);
+        $this->assertSame('footer{color:red}', $out['stylesheet']);
+        $this->assertSame('var x=1;', $out['javascript']);
+        $this->assertNull($out['block_layout']);
+    }
+
+    public function testLeavesPlaintextSectionUntouched(): void
+    {
+        $section = [
+            'html'       => '<b>hi</b>',
+            'stylesheet' => 'footer{color:red}',
+        ];
+
+        $this->assertSame($section, $this->decompressSection($section));
+    }
+
+    public function testCleanHtmlIsNotFlaggedAsContaminated(): void
+    {
+        $html = '<!DOCTYPE html><html><head><style>footer{color:#023f81}</style>'
+              . '</head><body><p>café — 日本語</p></body></html>';
+        $this->assertFalse(KytePageController::hasBinaryContamination($html));
+    }
+
+    public function testDetectsEmbeddedBzip2Stream(): void
+    {
+        // The exact failure mode: a compressed stylesheet concatenated into a
+        // <style> block instead of being decompressed first.
+        $html = '<style>footer{}' . bzcompress('body{color:red}', 9) . '</style>';
+        $this->assertTrue(KytePageController::hasBinaryContamination($html));
+    }
+
+    public function testDetectsInvalidUtf8(): void
+    {
+        $html = '<p>' . chr(0xFF) . '</p>'; // 0xFF is never valid UTF-8
+        $this->assertTrue(KytePageController::hasBinaryContamination($html));
+    }
+
+    public function testDoesNotFalsePositiveOnLiteralBZhText(): void
+    {
+        // Prose that merely contains "BZh" (no full bzip2 block magic) and is
+        // valid UTF-8 must publish normally.
+        $html = '<p>The BZh2 compression algorithm is great.</p>';
+        $this->assertFalse(KytePageController::hasBinaryContamination($html));
+    }
+
+    public function testEmptyStringIsClean(): void
+    {
+        $this->assertFalse(KytePageController::hasBinaryContamination(''));
+    }
+}