From cac17609754f48dce18e5d0a188cd8ed47b1e6ed Mon Sep 17 00:00:00 2001 From: Shane Rosenthal Date: Fri, 11 Sep 2026 21:11:39 -0400 Subject: [PATCH] Harden admin-upload-media base64 against transport mangling. Normalize content before strict decode (strip whitespace, recover space-substituted +, accept base64url), document JSON-arg usage for agents, and cover the recovery paths in AdminUploadMediaTest. --- app/Mcp/Servers/AdminNativePhpServer.php | 4 +- app/Mcp/Tools/Admin/AdminUploadMedia.php | 4 +- app/Services/AdminArticleMediaService.php | 13 +++- tests/Feature/Mcp/AdminUploadMediaTest.php | 71 +++++++++++++++++++++- 4 files changed, 86 insertions(+), 6 deletions(-) diff --git a/app/Mcp/Servers/AdminNativePhpServer.php b/app/Mcp/Servers/AdminNativePhpServer.php index a1fcdc7e6..f42a59bb0 100644 --- a/app/Mcp/Servers/AdminNativePhpServer.php +++ b/app/Mcp/Servers/AdminNativePhpServer.php @@ -32,12 +32,12 @@ class AdminNativePhpServer extends Server Rules: - Never return passwords, remember tokens, GitHub tokens, license keys, Stripe secrets, or raw API credentials. - Blog: create/update drafts; publish via admin-publish-blog-post (sets published_at like Filament; idempotent if already live). Slug changes refused once published. No unpublish tool yet. - - Media: upload via admin-upload-media to the public disk (default directory website-images; pass directory: "blog/heroes" for Filament article heroes). Optionally attach in one shot with article_id/slug only when directory is blog/heroes; or set hero on admin-update-blog-post via media_id/path or URL. + - Media: upload via admin-upload-media to the public disk. Pass `content` as a JSON string arg (raw base64, no data: prefix). Prefer image/webp. Default directory is website-images; pass directory: "blog/heroes" for Filament article heroes. Optionally attach in one shot with article_id/slug only when directory is blog/heroes. Do not put base64 in freeform prose (transport may mangle + into spaces; on invalid base64 the tool notes this). Or set hero on admin-update-blog-post via media_id/path or URL. - Other tools are read-only ops helpers (signups, users, companies, plugins, sales summaries, support). Tools: 1. admin-create-blog-post — create an unpublished article. - 2. admin-upload-media — upload an image (base64 → public disk WebP); optional directory (default website-images); for heroes pass directory: "blog/heroes" and optional article_id/slug attach. + 2. admin-upload-media — upload an image (JSON-arg raw base64 → public disk WebP; prefer image/webp; no data: prefix; do not put base64 in prose); optional directory (default website-images); for heroes pass directory: "blog/heroes" with slug/article_id in the same call to attach. 3. admin-update-blog-post — patch title/content/excerpt/slug/hero (no publish/unpublish). 4. admin-publish-blog-post — publish by id/slug (optional published_at; default now; idempotent). 5. admin-get-blog-post / admin-list-blog-posts — inspect drafts and published posts. diff --git a/app/Mcp/Tools/Admin/AdminUploadMedia.php b/app/Mcp/Tools/Admin/AdminUploadMedia.php index 96a98b7b1..e9e0a51dd 100644 --- a/app/Mcp/Tools/Admin/AdminUploadMedia.php +++ b/app/Mcp/Tools/Admin/AdminUploadMedia.php @@ -16,7 +16,7 @@ use RuntimeException; #[Name('admin-upload-media')] -#[Description('Upload an image to the public media disk (WebP re-encode). Optional directory is a relative path under the public disk (no "..", no absolute paths); nested paths like blog/heroes are allowed. Default directory is website-images. Attaching via article_id/slug does NOT force blog/heroes — pass directory: "blog/heroes" when uploading a blog hero/featured image. Prefer image/webp; jpeg/png allowed. Does not publish.')] +#[Description('Upload an image to the public media disk (WebP re-encode). Pass content as a JSON string argument: raw base64 only (no data: URI prefix). Prefer image/webp; jpeg/png allowed. Do not put base64 in freeform prose — transport may turn + into spaces (server recovers spaces→+ and accepts base64url). Optional directory is a relative path under the public disk (no "..", no absolute paths); nested paths like blog/heroes are allowed. Default directory is website-images. For blog heroes pass directory: "blog/heroes" and optional article_id/slug in the same call to attach; attach does NOT force that folder. Does not publish.')] class AdminUploadMedia extends Tool { use RequiresAdmin; @@ -123,7 +123,7 @@ public function schema(JsonSchema $schema): array return [ 'filename' => $schema->string()->description('Original filename (used for logging; stored name is a UUID .webp).')->required(), 'contentType' => $schema->string()->description('MIME type: image/webp (preferred), image/jpeg, or image/png.')->required(), - 'content' => $schema->string()->description('Base64-encoded image bytes with no data: URI prefix. Decoded size hard-capped at a few MB; re-encoded to WebP.')->required(), + 'content' => $schema->string()->description('Raw base64 image bytes as a JSON string argument (no data: URI prefix). Prefer image/webp. Do not paste base64 into freeform prose — + may become spaces. Server strips whitespace, recovers space→+, and accepts base64url (-_). Decoded size hard-capped at a few MB; re-encoded to WebP.')->required(), 'directory' => $schema->string()->description('Optional relative path under the public media disk (no "..", no absolute paths). Nested paths allowed (e.g. blog/heroes). Defaults to website-images. When attaching as an article hero via article_id/slug, pass directory: "blog/heroes" — attach does not force that folder.'), 'article_id' => $schema->integer()->description('Optional article id to attach this upload as the hero/featured image. Requires directory "blog/heroes" (or a path already under it).'), 'slug' => $schema->string()->description('Optional article slug to attach as hero when article_id is omitted. Requires directory "blog/heroes".'), diff --git a/app/Services/AdminArticleMediaService.php b/app/Services/AdminArticleMediaService.php index 1819ece64..eaa27c0e0 100644 --- a/app/Services/AdminArticleMediaService.php +++ b/app/Services/AdminArticleMediaService.php @@ -144,6 +144,17 @@ public function storeHeroFromBase64(string $base64, string $contentType, ?string throw new RuntimeException('content must be raw base64 without a data: URI prefix.'); } + // Normalize before strict decode: + // 1) strip whitespace (newlines/tabs/wrapping), 2) recover '+' mangled to spaces, + // 3) if -/_ present, accept base64url via strtr to the standard alphabet. + // Spaces are preserved through the whitespace strip so they can be restored to '+'. + $base64 = preg_replace('/[\t\n\r\f\v]+/', '', $base64) ?? $base64; + $base64 = str_replace(' ', '+', $base64); + + if (str_contains($base64, '-') || str_contains($base64, '_')) { + $base64 = strtr($base64, '-_', '+/'); + } + // Reject before decode so oversized payloads cannot exhaust memory. // Base64 expands 3 bytes → 4 chars; floor(len*3/4) is a safe decoded upper bound. if ((int) floor(strlen($base64) * 3 / 4) > self::MAX_DECODED_BYTES) { @@ -153,7 +164,7 @@ public function storeHeroFromBase64(string $base64, string $contentType, ?string $binary = base64_decode($base64, true); if ($binary === false) { - throw new RuntimeException('content is not valid base64.'); + throw new RuntimeException('content is not valid base64. Transport may have mangled + into spaces — pass content as a JSON string argument (raw base64, no data: prefix), not freeform prose.'); } return $this->storeHeroFromBinary($binary, $contentType, $filename, $directory); diff --git a/tests/Feature/Mcp/AdminUploadMediaTest.php b/tests/Feature/Mcp/AdminUploadMediaTest.php index 1aeb8c77f..37d6782c6 100644 --- a/tests/Feature/Mcp/AdminUploadMediaTest.php +++ b/tests/Feature/Mcp/AdminUploadMediaTest.php @@ -141,7 +141,76 @@ public function test_upload_media_rejects_data_uri_prefix_and_invalid_base64(): 'content' => '%%%not-base64%%%', ])->assertOk(); $this->assertTrue($bad->json('result.isError')); - $this->assertStringContainsString('base64', (string) data_get($bad->json(), 'result.content.0.text')); + $message = (string) data_get($bad->json(), 'result.content.0.text'); + $this->assertStringContainsString('base64', $message); + $this->assertStringContainsString('mangled', $message); + } + + public function test_upload_media_recovers_space_substituted_pluses(): void + { + $payload = $this->makeHeroPayload(); + $tokens = $this->issueMcpOAuthTokens($this->admin); + + // Simulate form/transport layers turning '+' into spaces. + $this->assertStringContainsString('+', $payload['base64'], 'Fixture base64 must include + so space recovery is exercised.'); + $mangled = str_replace('+', ' ', $payload['base64']); + $this->assertNotSame($payload['base64'], $mangled); + $this->assertFalse(base64_decode($mangled, true)); + + $response = $this->callMcpTool($tokens['access_token'], 'admin-upload-media', [ + 'filename' => 'hero.jpg', + 'contentType' => 'image/jpeg', + 'content' => $mangled, + ])->assertOk(); + + $this->assertFalse($response->json('result.isError')); + $result = json_decode((string) data_get($response->json(), 'result.content.0.text'), true); + $this->assertSame('image/webp', $result['content_type']); + Storage::disk('public')->assertExists($result['path']); + } + + public function test_upload_media_accepts_base64url(): void + { + $payload = $this->makeHeroPayload(); + $tokens = $this->issueMcpOAuthTokens($this->admin); + + $this->assertTrue( + str_contains($payload['base64'], '+') || str_contains($payload['base64'], '/'), + 'Fixture base64 must include + or / so base64url mapping is exercised.' + ); + + $base64url = strtr($payload['base64'], '+/', '-_'); + $this->assertTrue(str_contains($base64url, '-') || str_contains($base64url, '_')); + $this->assertFalse(base64_decode($base64url, true)); + + $response = $this->callMcpTool($tokens['access_token'], 'admin-upload-media', [ + 'filename' => 'hero.jpg', + 'contentType' => 'image/jpeg', + 'content' => $base64url, + ])->assertOk(); + + $this->assertFalse($response->json('result.isError')); + $result = json_decode((string) data_get($response->json(), 'result.content.0.text'), true); + Storage::disk('public')->assertExists($result['path']); + } + + public function test_upload_media_strips_whitespace_from_base64(): void + { + $payload = $this->makeHeroPayload(); + $tokens = $this->issueMcpOAuthTokens($this->admin); + + $wrapped = implode("\n", str_split($payload['base64'], 76)); + $this->assertStringContainsString("\n", $wrapped); + + $response = $this->callMcpTool($tokens['access_token'], 'admin-upload-media', [ + 'filename' => 'hero.jpg', + 'contentType' => 'image/jpeg', + 'content' => $wrapped, + ])->assertOk(); + + $this->assertFalse($response->json('result.isError')); + $result = json_decode((string) data_get($response->json(), 'result.content.0.text'), true); + Storage::disk('public')->assertExists($result['path']); } public function test_upload_media_rejects_oversized_decoded_payload(): void