Skip to content

DO NOT MERGE: TPX-03 reference fix, media id existence check for A1 - #359

Closed
Cryptoom wants to merge 14 commits into
trypostit:mainfrom
Cryptoom:claude/tpx-03-mediaid-design
Closed

Cryptoom wants to merge 14 commits into
trypostit:mainfrom
Cryptoom:claude/tpx-03-mediaid-design

Conversation

@Cryptoom

Copy link
Copy Markdown

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_platform pivot table plan. It is not intended to land as-is; A1
should review, adapt as needed, and land it as part of its own work (or cherry-
pick from this branch).

Summary

  • App\Actions\Post\CreatePost / UpdatePost write posts.media straight from
    client input with no check that media.*.id/path resolves to a real
    medias row. Confirmed as a bug (not an intentional feature): every
    legitimate 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.
  • Added App\Support\PostMediaRules::assertHostedMediaExists(), mirroring the
    existing FindWorkspaceAsset lookup used by attach-existing-asset. Wired
    into all four request classes (App/Api Store/UpdatePostRequest) via
    withValidator().
  • Also fixes an unhandled 500 (Postgres invalid input syntax for type uuid)
    when a non-UUID id like "media-1" reached Media::whereKey().
  • Security: closes a cross-tenant IDOR (a crafted id/path could reference
    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.
  • Updated 6 existing tests that used fabricated ids (media-1/hosted-1/m1)
    as a fixture shortcut; they now create a real Media::factory()->assets()
    row first.
  • Added 11 new tests (PostMediaExistsValidationTest,
    PostMediaExistsValidationWebTest) proving red-before/green-after.

Test plan

  • php artisan test tests/Feature/Api/PostMediaExistsValidationTest.php tests/Feature/PostMediaExistsValidationWebTest.php (11 passed)
  • Full --filter=Post suite: 967 passed, 0 regressions
  • MCP media tool tests unaffected (87 passed)
  • Confirmed red-before-fix via git stash of the fix-only files

Cryptoom and others added 14 commits August 25, 2026 10:33
Infrastructure prep, no active patch yet. Same pattern as
Cryptoom/whatsapp-mcp and Cryptoom/telegram-mcp forks.
* feat(mcp): add update-workspace-tool for brand settings

GetWorkspaceTool is read-only; there was no API or MCP path to write a
workspace's brand settings (name, colors, voice traits, font, image
style, content language). Adds UpdateWorkspaceTool mirroring
UpdateWorkspaceRequest's validation with PATCH semantics (only passed
fields change), gated on the same 'update' ability as the settings UI.

Registered as fork Patch 1 in PATCHES.md.

* fix(mcp): coerce brand_voice_traits and rename misleading test

UpdateWorkspaceTool validated brand_voice_traits against the enum but
never ran BrandVoiceTrait::coerce(), so an update could store an
incoherent set (e.g. both formal and casual). This is the first
programmatic/unattended MCP write path, so coerce it here before the
values land in the update payload.

Also rename a test whose name implied a cross-workspace injection
vector that the tool schema does not expose. The tool always resolves
the caller's current workspace; there is no workspace_id parameter to
attack.
…oyed

is_aigc already exists upstream for TikTok (Composer checkbox, meta
rules, publisher, MCP tool) - no patch needed. Also corrects the
stale note about the server checkout still running against upstream;
it now runs against Cryptoom/trypost since Patch 1.
* feat(design): madevisible brand token concept for app.css

Concept-only reskin: swaps the upstream Gumroad-style warm cream +
violet identity for madevisible.io brand tokens (deep teal primary,
navy text, gold accent, soft tinted shadows instead of hard offset
shadows, Geist font stack instead of Figtree/Instrument Serif).
Not pushed, local concept branch only.

* fix(design): darken chart-3 gold for WCAG 3:1 UI-component contrast

Review Round 1 (PR #2) flagged --chart-3 (#c99a5c, same hue as --accent)
against --background (#fbf7f1) at 2.38:1, below the 3:1 threshold for
isolated/meaningful data elements. Darkened to #a67c3f (~3.6:1).
Olli asked to remove the "Earn 30% referral" and "Discord community"
bottom-nav items. Also drops the now-unused IconGift/IconBrandDiscord
imports. Docs link and IconAffiliate (used elsewhere) untouched.
Patch 2 (madevisible brand token reskin, PR #2) and Patch 3 (sidebar
referral/Discord removal, PR #3) modify upstream files but were not
captured in PATCHES.md. A future upstream merge could silently
overwrite them without conflict. Add marker strings and break
conditions for both, following the existing Patch 1 format.
…isible.io (#5)

* fix(auth): show Privacy/ToS footer links on login too, point at madevisible.io

TikTok's App Review rejected the connected app "madevisible.io Social"
on 04.09.2026: the site had no visible Privacy Policy/Terms of Service
links at all, without opening a menu or logging in. Live-verified via
Playwright with a fresh, cookie-cleared session: the login page shows
only a "Forgot password?" link, nothing else.

Root cause: the only legal footer (auth.legal translation) was gated
behind v-if="!isSelfHosted" in Register.vue, and Login.vue never had
it. This deployment runs trypost.self_hosted=true (single-operator
model, not multi-tenant SaaS like trypost.it itself), so the footer
was unconditionally hidden on both pages that matter for App Review.

- lang/*/auth.php (all 16 locales): auth.legal's hardcoded
  https://trypost.it/terms and /privacy URLs -> https://madevisible.io/agb/
  and /privacy/ (both now name "madevisible.io Social" explicitly,
  see Cryptoom/digital-mind-agency#396).
- Register.vue: dropped the isSelfHosted guard, footer always shows.
- Login.vue: added the same footer block (didn't exist before).

See PATCHES.md Patch 4 for the full merge-safety marker strings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(patches): note the vite-build deploy trap for Patch 4

Round-2 review flagged that lang/*/auth.php only take effect after a
real `docker compose up -d --build`: laravel-vue-i18n compiles the
PHP locale files to gitignored lang/php_*.json at build time, so a
plain file-copy-and-restart on web02 would silently keep serving the
old trypost.it URLs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
TikTok app review rejected the app icon submission because it did not
match the icon shown at the declared Web/Desktop URL. That URL was
still serving TryPosts stock diamond favicon instead of the
madevisible.io teal M mark used everywhere else (TikTok Basic Info,
website, browser tab).
Assessed feasibility after a PlayCraft story draft got both image and
video attached simultaneously (facebook_story rejects images and both
story formats cap at 1 media item). Media is currently a single pool
shared by every enabled platform (Post::media(), no PostPlatform
relation), verified across the validation rule, all 13 publisher
services and the Vue editor. Documents a pivot-table design and flags
that the AGPL network-copyleft disclosure duty is already live now
that PlayCraft is a real paying workspace on this instance, not only
"before the first customer" as the earlier note said.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
publishStory() ignored the upload_url returned by the video_stories
start phase and instead POSTed the media's hosted URL as a body field
called video_file_chunk to /{video_id} on the regular Graph host. That
is not a valid transfer step for this API: Facebook's Video Stories
API always returns {video_id, upload_url}, and the transfer phase has
to reach the rupload host at upload_url, not graph.facebook.com.

Every Facebook Story publish failed with Graph API error 6000
("Problem with file. Try with another file."), regardless of the
actual video (reproduced with and without an audio track). Fixed by
mirroring the already-working publishReel() transfer step: download
the hosted media and stream raw bytes to upload_url with the
Offset/file_size headers rupload requires, since the documented
file_url-header shortcut is the same shortcut publishReel()'s own
comment already flags as unreliable.

Verified against Meta's documented start/transfer/finish flow (Ayrshare's
Facebook Stories API writeup, which matches the official docs). Could not
run the local test suite (no working Postgres test database for this repo
on this machine, port 5432 is held by an unrelated project's container);
`php -l` passes on both changed files, and the updated test mirrors the
already-passing reel test's assertions against the new transfer contract.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Port 5432 is occupied by another project's Docker container on this
machine, so .env.testing's DB_HOST=127.0.0.1 pointed at the wrong
database and the entire test suite failed with a bad auth error.

Adds compose.test.yml, a minimal, isolated Postgres 16 container
(binds 127.0.0.1:5433 only) dedicated to the test suite, independent
from the full compose.yaml dev stack. Reuses docker/postgres-init.sh
to create the trypost_test database, matching the existing convention.

Updates .env.testing to point at the new port and credentials
(DB_PASSWORD already matched phpunit.xml's committed value, DB_USERNAME
now matches the container's postgres user).

Documents the setup, the passport:keys requirement (storage/oauth-*.key
are gitignored and not generated by default, causing every OAuth-backed
test to fail with "Invalid key supplied" until generated), the memory
limit needed for image-processing tests, and why php artisan test
cannot be used locally (it boots the full console kernel against the
default .env before phpunit's env vars apply) in a new CLAUDE.md
section.

Full suite verified green: 4553 passed, 2 skipped, 0 failed.
Koeder test: inverted an assertion in the newly landed Facebook story
publisher test (a30d83c), confirmed it failed red, reverted, confirmed
green again.
…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.
@Cryptoom

Copy link
Copy Markdown
Author

Opened against the wrong repo by mistake (gh pr create picked the upstream instead of our fork). Closing immediately, no review needed. Correct PR: will be opened against Cryptoom/trypost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant