Skip to content

Issue direct upload URLs from Cloud filesystems - #216

Open
brandonkelly wants to merge 1 commit into
3.xfrom
direct-upload-urls
Open

brandonkelly wants to merge 1 commit into
3.xfrom
direct-upload-urls

Conversation

@brandonkelly

@brandonkelly brandonkelly commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Summary

Craft's MCP server can now upload files straight to S3 on Cloud, the way the control panel uploader does. Today they go through Craft's mcp/upload action, which is capped by Lambda's 6 MB request limit.

assets_upload_url (Craft)
  └─ temp upload fs = TmpFs            (via Temp => TmpFs, needs craftcms/cms#20628)
       └─ Fs::getDirectUploadUrl('mcp-uploads/<id>', expiresAt)
            → { url: presigned S3 PutObject,
                method: 'PUT',
                headers: { x-amz-meta-visibility: private } }   ← every signed header except host
client ── PUT file + headers ──▶ S3     (no Lambda in the path)
assets_create { upload_id } ── Craft checks existence + size, then claims
  • Fs implements Craft's new craft\base\DirectUploadFsInterface (5.12.2+). It returns null under useLocalFs, so Craft falls back to its own PUT endpoint.
  • src/fs/DirectUploadFsInterface.php shims the interface for older Craft versions, the same way ReleasableQueueInterface does.
  • presignedUrl() and getDirectUploadUrl() now share one presign helper, so the returned headers come from the same signed request as the URL.

S3 can't enforce a size limit on a presigned PUT. Craft checks maxUploadFileSize when the upload is claimed.

Evidence

  • New tests/unit/FsTest.php: TmpFs URL key <env>/storage/tmp/mcp-uploads/abc, headers equal the signed headers (x-amz-meta-visibility: private, plus x-amz-meta-max-age on AssetsFs), null under useLocalFs. Full suite: 103 tests pass; ECS and PHPStan clean.
  • End to end against SeaweedFS's S3 API (scratch test, not committed):
    • Without the returned headers: PUT → 403 SignatureDoesNotMatch
    • With them (and an arbitrary Content-Type): a 7 MB PUT → 200; fileExists, getFileSize and read return the bytes; object metadata {"visibility":"private"}
  • With a stub craft\base\DirectUploadFsInterface defined, TmpFs implements it (the 5.12.2+ branch of the shim).

Related issues

Both Craft PRs need to ship before this has any effect on Cloud.

Implement Craft's DirectUploadFsInterface (5.12.2+) on Fs, so Craft's MCP
server can send files straight to S3 instead of through Lambda's 6 MB
request limit. The interface is shimmed for older Craft versions.

Amp-Thread-ID: https://ampcode.com/threads/T-54e734d2-d69f-42d7-9f3b-a6a8e281ad1d
Co-authored-by: Amp <amp@ampcode.com>
Copilot AI balanced review requested due to automatic review settings November 12, 2026 12:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Compatibility and the MCP upload/claim flow still require validation against the upstream DirectUploadFsInterface, which is unavailable on Craft’s 5.x branch; the reported stub does not establish that contract.

No open issues 🎉

What changed in this PR

Adds direct S3 upload URLs to Cloud filesystems so Craft’s MCP uploads can bypass Lambda’s 6 MB request limit, while retaining the local-filesystem fallback.

Changes:

  • Implements direct-upload support with URLs and required headers derived from a shared presigned request.
  • Adds an interface shim for compatibility with older Craft versions.
  • Adds unit tests for temporary-upload paths, signed metadata headers, and local-filesystem fallback.
File Description
tests/​unit/​FsTest.php Tests direct-upload URLs, required headers, and local fallback.
src/​fs/​Fs.php Implements direct-upload URL generation and shares presigning logic.
src/​fs/​DirectUploadFsInterface.php Adds a compatibility shim for Craft’s direct-upload interface.

Review effort: Balanced

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.

2 participants