Conversation
…space-owned Media row
TPX-02/A0 found that CreatePost and UpdatePost write posts.media straight
from client input with no check that media.*.id/path resolves to a real
medias row: tests/Feature/Api/PostApiTest.php created a post with
media[0].id = "media-1" (no matching medias row) and it was accepted.
Root cause confirmed as a bug, not a feature: every legitimate write path
(the 3 dedicated MCP/API attach tools, the web asset gallery, Unsplash/
Giphy save-from-url, AI regeneration) always echoes back a real, server-
issued Media id, scoped to the workspace via FindWorkspaceAsset or
MediaAttacher's download-and-host flow. The web StorePostRequest client
(createPostFromAsset in GalleryBrowser.vue) sends only asset.id from a
prior server response, never a client-generated id.
App\Support\PostMediaRules::assertHostedMediaExists mirrors the existing
FindWorkspaceAsset lookup used by attach-existing-asset: any media item
that claims to already be hosted (id and/or path set) must resolve to a
medias row with mediable_type=workspace, mediable_id=<caller's workspace>,
collection=assets, or the request is rejected with a 422 on media.N.id.
A bare external url with no id/path (API-only fresh-download case) is left
alone, HostInlineMedia/MediaAttacher still download and host it before
anything is persisted.
Wired into all four request classes:
- App\Http\Requests\App\Post\StorePostRequest previously had NO item-level
media validation at all ('media' => ['nullable','array']); now uses
PostMediaRules::rules(hosted: true) plus the new check.
- App\Http\Requests\App\Post\UpdatePostRequest, Api StorePostRequest and
Api UpdatePostRequest already used PostMediaRules::rules() for shape but
never checked existence; added a withValidator() closure.
Also closes a related crash: a non-UUID id (e.g. "media-1") reached
Media::whereKey() and threw an unhandled Postgres "invalid input syntax
for type uuid" 500 instead of a 422. assertHostedMediaExists now checks
Str::isUuid() first.
Security: this closes a cross-tenant IDOR. Nothing previously verified
that a referenced media id belonged to the requesting workspace, so a
crafted id/path could reference (or claim to reference) another
workspace's asset. Verified against production (web02, trypost-pgsql,
read-only): 0 of 32 posts with non-empty media have an orphaned or
cross-tenant media id today, so this closes the gap going forward without
any known bad data to migrate.
Updated 6 existing tests (PostApiTest, PostMediaApiTest x5,
PostMediaAltTextValidationTest x3, UpdatePostRequestTest) that used
fabricated ids like "media-1"/"hosted-1"/"m1" as a fixture shortcut for
the "already-hosted" case; they now create a real Media::factory()
->assets() row first, which is what those code paths were always meant
to receive.
Added tests/Feature/Api/PostMediaExistsValidationTest.php and
tests/Feature/PostMediaExistsValidationWebTest.php (11 new tests) that
prove the fix red-before/green-after: fabricated id rejected, cross-
tenant id rejected, real workspace id accepted, bare url still downloads,
path without id rejected.
NOT FOR MERGE. Reference implementation for A1 (media_post_platform
pivot with a hard FK to medias.id) from the plan at
~/.claude/plans/proud-bubbling-dewdrop.md. See TPX-03 completion report
in the Vault for the full design writeup, verification results, and
IDOR scope.
5 tasks
Cryptoom
added a commit
that referenced
this pull request
Sep 16, 2026
…idation (#9) * feat(media): per-platform media pivot + workspace-scoped media id validation Adds media_post_platform (uuid PK, unique on media_id+post_platform_id, cascadeOnDelete on both FKs) and PostPlatform::media()/scopedMediaItems(), the single place publishers and validators will read per-platform media selection from once the publisher rollout (A3) lands. An empty pivot set for a platform means "all of the post's media", today's behavior, so nothing else needs a special case. Combines this with the media-id validation fix identified in TPX-03 (A0b): posts.media accepted a client-supplied id/path with no check that it resolved to a real medias row owned by the caller's workspace, a cross-tenant IDOR, plus an unhandled 500 on a non-UUID id. Both request classes for create/update (web + API) now validate via PostMediaRules::assertHostedMediaExists. Ported the 11 tests from PR #8 (TPX-03 reference branch) plus 8 new tests for the pivot/relation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(media): tighten pivot docblock rationale, add collection-boundary test Review round 1 feedback (2 independent code-reviewer passes): the composite-key pivot post_workspace_label has the identical many-to-many shape and works fine with a composite PK, so "no natural single key" overstated the uuid-PK justification. Rewrote both docblocks around the actual reason (row addressability for debugging), the unique index still does the uniqueness enforcement. Also closes a minor coverage gap: no test asserted the collection === 'assets' boundary in assertHostedMediaExists (a real workspace-owned medias row in the wrong collection, e.g. a logo, must still 422). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE
This is a reference implementation for TPX-03 (A0b Media-ID-Design-Vertiefung),
handed to Paket A1 as the design basis for closing the FK-safety gap in the
media_post_platformpivot table plan. It is not intended to land as-is; A1should review, adapt as needed, and land it as part of its own work (or cherry-
pick from this branch).
Summary
App\Actions\Post\CreatePost/UpdatePostwriteposts.mediastraight fromclient input with no check that
media.*.id/pathresolves to a realmediasrow. Confirmed as a bug (not an intentional feature): everylegitimate write path already resolves media server-side and scoped to the
workspace (the 3 dedicated attach tools, the web asset gallery, Unsplash/
Giphy save-from-url, AI regeneration). No client code anywhere generates its
own media id.
App\Support\PostMediaRules::assertHostedMediaExists(), mirroring theexisting
FindWorkspaceAssetlookup used byattach-existing-asset. Wiredinto all four request classes (App/Api Store/UpdatePostRequest) via
withValidator().invalid input syntax for type uuid)when a non-UUID id like
"media-1"reachedMedia::whereKey().another workspace's asset). Verified against production (web02,
read-only): 0 of 32 posts with media have an orphaned or cross-tenant media
id today, so no bad data needs migrating.
media-1/hosted-1/m1)as a fixture shortcut; they now create a real
Media::factory()->assets()row first.
PostMediaExistsValidationTest,PostMediaExistsValidationWebTest) proving red-before/green-after.Test plan
php artisan test tests/Feature/Api/PostMediaExistsValidationTest.php tests/Feature/PostMediaExistsValidationWebTest.php(11 passed)--filter=Postsuite: 967 passed, 0 regressionsgit stashof the fix-only files