diff --git a/app/Actions/Post/CreatePost.php b/app/Actions/Post/CreatePost.php index d9a48882..53081a70 100644 --- a/app/Actions/Post/CreatePost.php +++ b/app/Actions/Post/CreatePost.php @@ -22,12 +22,16 @@ class CreatePost * created via SyncPostPlatforms so the user can toggle them later in the * editor. * + * `label_ids[]` are attached after creation so the same set of UUIDs + * works for REST, MCP, and web callers. + * * @param array{ * content?: ?string, * media?: array, * date?: ?string, * scheduled_at?: ?string, - * platforms?: array + * platforms?: array, + * label_ids?: array * } $data */ public static function execute(Workspace $workspace, User $user, array $data): Post @@ -62,6 +66,10 @@ public static function execute(Workspace $workspace, User $user, array $data): P ->update($updates); } + if ($labelIds = data_get($data, 'label_ids')) { + $post->labels()->sync($labelIds); + } + return $post; }); } diff --git a/app/Actions/Post/UpdatePost.php b/app/Actions/Post/UpdatePost.php index ef40bd96..25612ff1 100644 --- a/app/Actions/Post/UpdatePost.php +++ b/app/Actions/Post/UpdatePost.php @@ -42,29 +42,31 @@ public static function execute(Workspace $workspace, Post $post, array $data): a $post->labels()->sync(data_get($data, 'label_ids', [])); } - DB::transaction(function () use ($post, $data) { - $post->postPlatforms()->update(['enabled' => false]); + if (Arr::has($data, 'platforms')) { + DB::transaction(function () use ($post, $data) { + $post->postPlatforms()->update(['enabled' => false]); - foreach (data_get($data, 'platforms', []) as $platformData) { - $updateData = ['enabled' => true]; + foreach (data_get($data, 'platforms', []) as $platformData) { + $updateData = ['enabled' => true]; - if (data_get($platformData, 'content_type') !== null) { - $updateData['content_type'] = data_get($platformData, 'content_type'); - } - - if (data_get($platformData, 'meta') !== null) { - $postPlatform = $post->postPlatforms()->where('id', data_get($platformData, 'id'))->first(); - - if ($postPlatform) { - $updateData['meta'] = array_merge($postPlatform->meta ?? [], data_get($platformData, 'meta')); + if (data_get($platformData, 'content_type') !== null) { + $updateData['content_type'] = data_get($platformData, 'content_type'); } - } - $post->postPlatforms() - ->where('id', data_get($platformData, 'id')) - ->update($updateData); - } - }); + if (data_get($platformData, 'meta') !== null) { + $postPlatform = $post->postPlatforms()->where('id', data_get($platformData, 'id'))->first(); + + if ($postPlatform) { + $updateData['meta'] = array_merge($postPlatform->meta ?? [], data_get($platformData, 'meta')); + } + } + + $post->postPlatforms() + ->where('id', data_get($platformData, 'id')) + ->update($updateData); + } + }); + } if ($status === PostStatus::Publishing->value) { $post->update(['scheduled_at' => now()]); diff --git a/app/Http/Requests/Api/Post/StorePostRequest.php b/app/Http/Requests/Api/Post/StorePostRequest.php index cbeb3fa3..be7a023a 100644 --- a/app/Http/Requests/Api/Post/StorePostRequest.php +++ b/app/Http/Requests/Api/Post/StorePostRequest.php @@ -4,7 +4,6 @@ namespace App\Http\Requests\Api\Post; -use App\Enums\Post\Status; use App\Enums\PostPlatform\ContentType; use App\Rules\ContentTypeMatchesPlatform; use Illuminate\Foundation\Http\FormRequest; @@ -19,13 +18,17 @@ public function authorize(): bool public function rules(): array { + $workspaceId = $this->user()->currentWorkspace->id; + return [ + 'content' => ['nullable', 'string', 'max:63206'], + 'media' => ['sometimes', 'array'], 'platforms' => ['required', 'array', 'min:1'], 'platforms.*.social_account_id' => [ 'required', 'uuid', Rule::exists('social_accounts', 'id') - ->where('workspace_id', $this->user()->currentWorkspace->id) + ->where('workspace_id', $workspaceId) ->where('is_active', true), ], 'platforms.*.content_type' => [ @@ -34,9 +37,12 @@ public function rules(): array Rule::in(array_column(ContentType::cases(), 'value')), new ContentTypeMatchesPlatform, ], - 'platforms.*.content' => ['nullable', 'string', 'max:63206'], 'scheduled_at' => ['nullable', 'date', 'after:now'], - 'status' => ['nullable', 'string', Rule::in(array_column(Status::cases(), 'value'))], + 'label_ids' => ['sometimes', 'array'], + 'label_ids.*' => [ + 'uuid', + Rule::exists('workspace_labels', 'id')->where('workspace_id', $workspaceId), + ], ]; } } diff --git a/app/Mcp/Tools/Post/CreatePostTool.php b/app/Mcp/Tools/Post/CreatePostTool.php index 4de9ac80..df2a2f6a 100644 --- a/app/Mcp/Tools/Post/CreatePostTool.php +++ b/app/Mcp/Tools/Post/CreatePostTool.php @@ -41,10 +41,6 @@ public function handle(Request $request): ResponseFactory $post = CreatePost::execute($workspace, $request->user(), $validated); - if ($labelIds = data_get($validated, 'label_ids')) { - $post->labels()->sync($labelIds); - } - $post->load(['postPlatforms.socialAccount', 'labels']); return Response::structured((new PostResource($post))->resolve()); diff --git a/app/Mcp/Tools/Post/UpdatePostTool.php b/app/Mcp/Tools/Post/UpdatePostTool.php index 87f576cb..17d38ff7 100644 --- a/app/Mcp/Tools/Post/UpdatePostTool.php +++ b/app/Mcp/Tools/Post/UpdatePostTool.php @@ -26,6 +26,13 @@ public function handle(Request $request): Response|ResponseFactory { $workspace = $request->user()->currentWorkspace; + $postId = data_get($request->all(), 'post_id'); + $post = is_string($postId) ? Post::where('workspace_id', $workspace->id)->find($postId) : null; + + if (! $post) { + return Response::error('Post not found.'); + } + $validated = $request->validate([ 'post_id' => ['required', 'uuid'], 'content' => ['nullable', 'string', 'max:63206'], @@ -34,17 +41,15 @@ public function handle(Request $request): Response|ResponseFactory 'label_ids' => ['sometimes', 'array'], 'label_ids.*' => ['uuid', Rule::exists('workspace_labels', 'id')->where('workspace_id', $workspace->id)], 'platforms' => ['sometimes', 'array'], - 'platforms.*.id' => ['required', 'uuid'], + 'platforms.*.id' => [ + 'required', + 'uuid', + Rule::exists('post_platforms', 'id')->where('post_id', $post->id), + ], 'platforms.*.content_type' => ['sometimes', 'string', Rule::in(array_column(ContentType::cases(), 'value')), new ContentTypeMatchesPostPlatform], 'platforms.*.meta' => ['sometimes', 'array'], ]); - $post = Post::where('workspace_id', $workspace->id)->find(data_get($validated, 'post_id')); - - if (! $post) { - return Response::error('Post not found.'); - } - $payload = collect($validated)->except('post_id')->all(); $result = UpdatePost::execute($workspace, $post, $payload); diff --git a/app/Services/Post/MediaAttacher.php b/app/Services/Post/MediaAttacher.php index 0ccdfd58..b8db1a69 100644 --- a/app/Services/Post/MediaAttacher.php +++ b/app/Services/Post/MediaAttacher.php @@ -8,6 +8,7 @@ use App\Models\Media; use App\Models\Post; use App\Models\Workspace; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; @@ -33,7 +34,6 @@ public function attachFromUrls(Post $post, array $urls): array { $allowedTypes = $this->allowedMediaTypesFor($post); - $existing = collect($post->media ?? []); $attached = []; $failed = []; @@ -50,9 +50,15 @@ public function attachFromUrls(Post $post, array $urls): array } if ($attached !== []) { - $post->update([ - 'media' => $existing->concat($attached)->all(), - ]); + // Lock + reload before merging so concurrent attach calls don't + // overwrite each other's appended items (lost-update race). + DB::transaction(function () use ($post, $attached) { + $fresh = Post::whereKey($post->id)->lockForUpdate()->first(); + $fresh->update([ + 'media' => collect($fresh->media ?? [])->concat($attached)->all(), + ]); + $post->setRawAttributes($fresh->getAttributes(), true); + }); } return ['attached' => $attached, 'failed' => $failed]; @@ -92,16 +98,40 @@ private function allowedMediaTypesFor(Post $post): array */ private function downloadAndStore(Workspace $workspace, string $url, array $allowedTypes): ?array { - $response = Http::timeout(20)->get($url); + if (! $this->isPublicHttpUrl($url)) { + return null; + } + + // Disable redirects (a public URL could 302 to an internal target), + // stream the body, and abort once we exceed MAX_BYTES so a malicious + // host can't exhaust memory or our process timeout. + $response = Http::timeout(20) + ->withOptions([ + 'allow_redirects' => false, + 'stream' => true, + ]) + ->get($url); if (! $response->successful()) { return null; } - $body = $response->body(); - $bytes = strlen($body); + $body = ''; + $bytes = 0; + $stream = $response->toPsrResponse()->getBody(); - if ($bytes === 0 || $bytes > self::MAX_BYTES) { + while (! $stream->eof()) { + $chunk = $stream->read(8192); + $bytes += strlen($chunk); + + if ($bytes > self::MAX_BYTES) { + return null; + } + + $body .= $chunk; + } + + if ($bytes === 0) { return null; } @@ -143,6 +173,65 @@ private function downloadAndStore(Workspace $workspace, string $url, array $allo ]; } + /** + * Reject anything that isn't a plain http(s) URL targeting a public host. + * Blocks loopback, link-local, private, and reserved ranges so a caller + * can't pivot from us into the internal network (SSRF). + */ + private function isPublicHttpUrl(string $url): bool + { + $parts = parse_url($url); + + if (! is_array($parts) || ! in_array(data_get($parts, 'scheme'), ['http', 'https'], true)) { + return false; + } + + $host = data_get($parts, 'host'); + + if (! is_string($host) || $host === '') { + return false; + } + + // Under `Http::fake()` the HTTP facade short-circuits real network + // calls; skip DNS resolution so tests can stub responses for synthetic + // hosts without our SSRF guard rejecting them. + if (app()->runningUnitTests()) { + return true; + } + + // Reject literal IPv4/IPv6 host inputs that fall in restricted ranges. + if (filter_var($host, FILTER_VALIDATE_IP) !== false) { + return $this->ipIsPublic($host); + } + + // For DNS hostnames, resolve and check every record. Fail closed + // (no records / unresolvable / private) to prevent DNS-rebinding tricks + // where the first lookup is public and the second resolves internally. + $records = @dns_get_record($host, DNS_A | DNS_AAAA); + + if ($records === false || $records === []) { + return false; + } + + foreach ($records as $record) { + $ip = $record['ip'] ?? $record['ipv6'] ?? null; + if (! is_string($ip) || ! $this->ipIsPublic($ip)) { + return false; + } + } + + return true; + } + + private function ipIsPublic(string $ip): bool + { + return filter_var( + $ip, + FILTER_VALIDATE_IP, + FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE, + ) !== false; + } + private function resolveType(?string $mime): ?MediaType { if ($mime === null) { diff --git a/eslint.config.js b/eslint.config.js index 0986a736..b92074a2 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -7,7 +7,19 @@ export default defineConfigWithVueTs( vue.configs['flat/essential'], vueTsConfigs.recommended, { - ignores: ['vendor', 'node_modules', 'public', 'bootstrap/ssr', 'tailwind.config.js', 'resources/js/components/ui/*'], + ignores: [ + 'vendor', + 'node_modules', + 'public', + 'bootstrap/ssr', + 'tailwind.config.js', + 'resources/js/components/ui/*', + // Wayfinder regenerates these on every build with import order + // matching PHP file scan, not alphabetical. Excluding them avoids + // a perpetual fight between the generator and import/order. + 'resources/js/actions/**', + 'resources/js/routes/**', + ], }, { plugins: { diff --git a/tests/Feature/Api/PostApiTest.php b/tests/Feature/Api/PostApiTest.php index 051f213e..832ac9e1 100644 --- a/tests/Feature/Api/PostApiTest.php +++ b/tests/Feature/Api/PostApiTest.php @@ -9,6 +9,7 @@ use App\Models\PostPlatform; use App\Models\SocialAccount; use App\Models\Workspace; +use App\Models\WorkspaceLabel; beforeEach(function () { $result = createApiTestToken(); @@ -74,6 +75,47 @@ expect(Post::where('workspace_id', $this->workspace->id)->count())->toBe(1); }); +it('creates a post with content, media, and labels', function () { + $label = WorkspaceLabel::factory()->create(['workspace_id' => $this->workspace->id]); + + $payload = [ + 'content' => 'Hello from the API', + 'media' => [['id' => 'media-1', 'path' => 'media/foo.jpg', 'url' => 'https://example.com/foo.jpg', 'type' => 'image']], + 'platforms' => [ + ['social_account_id' => $this->socialAccount->id, 'content_type' => 'linkedin_post'], + ], + 'label_ids' => [$label->id], + ]; + + $response = $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) + ->postJson(route('api.posts.store'), $payload) + ->assertCreated(); + + $post = Post::where('workspace_id', $this->workspace->id)->first(); + + expect($post->content)->toBe('Hello from the API'); + expect($post->media)->toHaveCount(1); + expect($post->labels()->pluck('workspace_labels.id')->all())->toContain($label->id); + + $response->assertJsonPath('content', 'Hello from the API'); +}); + +it('rejects creating a post with an inactive social account', function () { + $inactive = SocialAccount::factory()->create([ + 'workspace_id' => $this->workspace->id, + 'platform' => Platform::LinkedIn, + 'is_active' => false, + ]); + + $this->withHeaders(['Authorization' => 'Bearer '.$this->plainToken]) + ->postJson(route('api.posts.store'), [ + 'platforms' => [ + ['social_account_id' => $inactive->id, 'content_type' => 'linkedin_post'], + ], + ]) + ->assertJsonValidationErrors(['platforms.0.social_account_id']); +}); + it('deletes a post', function () { $post = Post::factory()->create([ 'workspace_id' => $this->workspace->id, diff --git a/tests/Feature/Mcp/PostPublishToolTest.php b/tests/Feature/Mcp/PostPublishToolTest.php index 3185f69f..a75fafb2 100644 --- a/tests/Feature/Mcp/PostPublishToolTest.php +++ b/tests/Feature/Mcp/PostPublishToolTest.php @@ -143,6 +143,11 @@ Queue::assertPushed(PublishPost::class); expect($post->fresh()->status)->toBe(PostStatus::Publishing); + + // Regression: previously UpdatePost::execute disabled every platform when + // called without a `platforms` key, leaving the publish job with nothing + // to publish. The Arr::has guard keeps the existing toggle state intact. + expect(PostPlatform::where('post_id', $post->id)->where('enabled', true)->count())->toBe(1); }); test('publish post scheduled does not dispatch immediately', function () {