diff --git a/internal/core/blobs/service.go b/internal/core/blobs/service.go index d746c53..b7d8bae 100644 --- a/internal/core/blobs/service.go +++ b/internal/core/blobs/service.go @@ -28,6 +28,26 @@ type Service interface { // UploadBlob uploads binary data to the owner's PDS UploadBlob(ctx context.Context, owner BlobOwner, data []byte, mimeType string) (*BlobRef, error) + + // FetchImageForURL is UploadBlobFromURL's FIRST half on its own: fetch the + // remote image under a timeout, refuse a Content-Type outside the image + // allowlist, and cap the body at 6MB. It touches no PDS. + // + // IT IS SPLIT OUT BECAUSE THE UPLOADER CHANGED, NOT THE GUARD. A post's + // media now goes into the AUTHOR's repository, under the author's own OAuth + // session — which is DPoP-signed, so it cannot travel through this + // package's BlobOwner (a bearer token and a URL). The caller therefore does + // the upload itself, through the author's PDS client, and this is what it + // calls first so that the choke point is reached by both paths rather than + // reimplemented beside one of them. + // + // The guard is the reason the split is a split and not a copy. The URL + // being fetched is attacker-influenced twice over — a client picks the page + // that gets unfurled, and the page picks the thumbnail — so a second + // implementation that drifted would turn a link preview into an unbounded + // fetch performed by the AppView with a user's credentials into that user's + // own storage quota. + FetchImageForURL(ctx context.Context, imageURL string) (data []byte, mimeType string, err error) } type blobService struct { @@ -48,9 +68,21 @@ func NewBlobService(pdsURL string) Service { // 3. Validate MIME type (image/jpeg, image/png, image/webp) // 4. Call UploadBlob to upload to PDS func (s *blobService) UploadBlobFromURL(ctx context.Context, owner BlobOwner, imageURL string) (*BlobRef, error) { + data, mimeType, err := s.FetchImageForURL(ctx, imageURL) + if err != nil { + return nil, err + } + + // Upload to PDS + return s.UploadBlob(ctx, owner, data, mimeType) +} + +// FetchImageForURL fetches and validates a remote image without uploading it. +// See Service.FetchImageForURL for why this half stands on its own. +func (s *blobService) FetchImageForURL(ctx context.Context, imageURL string) ([]byte, string, error) { // Input validation if imageURL == "" { - return nil, fmt.Errorf("image URL cannot be empty") + return nil, "", fmt.Errorf("image URL cannot be empty") } // Create HTTP client with timeout (30s to handle slow CDNs and large images) @@ -61,7 +93,7 @@ func (s *blobService) UploadBlobFromURL(ctx context.Context, owner BlobOwner, im // Fetch image from URL req, err := http.NewRequestWithContext(ctx, "GET", imageURL, nil) if err != nil { - return nil, fmt.Errorf("failed to create request for image URL: %w", err) + return nil, "", fmt.Errorf("failed to create request for image URL: %w", err) } // Set User-Agent to avoid being blocked by CDNs that filter bot traffic @@ -69,7 +101,7 @@ func (s *blobService) UploadBlobFromURL(ctx context.Context, owner BlobOwner, im resp, err := client.Do(req) if err != nil { - return nil, fmt.Errorf("failed to fetch image from URL: %w", err) + return nil, "", fmt.Errorf("failed to fetch image from URL: %w", err) } defer func() { if closeErr := resp.Body.Close(); closeErr != nil { @@ -79,13 +111,13 @@ func (s *blobService) UploadBlobFromURL(ctx context.Context, owner BlobOwner, im // Check HTTP status if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("failed to fetch image: HTTP %d", resp.StatusCode) + return nil, "", fmt.Errorf("failed to fetch image: HTTP %d", resp.StatusCode) } // Get MIME type from Content-Type header mimeType := resp.Header.Get("Content-Type") if mimeType == "" { - return nil, fmt.Errorf("image URL response missing Content-Type header") + return nil, "", fmt.Errorf("image URL response missing Content-Type header") } // Normalize MIME type (e.g., image/jpg → image/jpeg) @@ -93,23 +125,22 @@ func (s *blobService) UploadBlobFromURL(ctx context.Context, owner BlobOwner, im // Validate MIME type before reading data if !isValidMimeType(mimeType) { - return nil, fmt.Errorf("unsupported MIME type: %s (allowed: image/jpeg, image/png, image/webp)", mimeType) + return nil, "", fmt.Errorf("unsupported MIME type: %s (allowed: image/jpeg, image/png, image/webp)", mimeType) } // Read image data data, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read image data: %w", err) + return nil, "", fmt.Errorf("failed to read image data: %w", err) } // Validate size (6MB = 6291456 bytes) const maxSize = 6291456 if len(data) > maxSize { - return nil, fmt.Errorf("image size %d bytes exceeds maximum of %d bytes (6MB)", len(data), maxSize) + return nil, "", fmt.Errorf("image size %d bytes exceeds maximum of %d bytes (6MB)", len(data), maxSize) } - // Upload to PDS - return s.UploadBlob(ctx, owner, data, mimeType) + return data, mimeType, nil } // UploadBlob uploads binary data to the owner's PDS diff --git a/internal/core/posts/blob_transform.go b/internal/core/posts/blob_transform.go index 3e8e161..6a136c9 100644 --- a/internal/core/posts/blob_transform.go +++ b/internal/core/posts/blob_transform.go @@ -14,17 +14,16 @@ import ( // the #view shape served to clients, replacing blob references with fetchable // image-proxy URLs. It modifies the Embed field in place. // -// Post embeds resolve against the community's repository: the AppView signs -// community post records into the community's PDS and uploads their blobs -// there, so the community DID owns every blob in the embed regardless of who -// authored the post. +// WHICH REPOSITORY THE BLOBS LIVE IN DEPENDS ON THE RECORD (see blobOwnerOf). +// One table holds both kinds of post and one read path serves them, so the +// owner is chosen per view rather than assumed. // // Must run before the response is written. A post whose embed still carries // blob references forces the client to build its own blob URLs, which routes // media around the image proxy and therefore around CSAM scanning — see // internal/core/embeds. func TransformBlobRefsToURLs(postView *PostView) { - if postView == nil || postView.Embed == nil || postView.Community == nil { + if postView == nil || postView.Embed == nil { return } @@ -33,7 +32,48 @@ func TransformBlobRefsToURLs(postView *PostView) { return } - embeds.HydrateView(embedMap, postView.Community.DID, postView.Community.PDSURL) + ownerDID, ownerPDSURL, ok := blobOwnerOf(postView) + if !ok { + return + } + + embeds.HydrateView(embedMap, ownerDID, ownerPDSURL) +} + +// blobOwnerOf reports the repository a post's blobs live in, and false when +// there is no answer. +// +// A postv2 record lives in its AUTHOR's repo (§3.1) and its media was uploaded +// under the author's own session, so the author owns it. A deprecated +// social.coves.community.post record was signed into the COMMUNITY's repo by the +// AppView, with its blobs uploaded there, so the community owns it — and every +// such record standing in production today still resolves that way, until task 8 +// re-materializes them. +// +// THE COLLECTION IN THE URI IS THE ONLY HONEST SIGNAL. It says which repository +// the record is in, which is exactly the question being asked. Getting this +// wrong does not crash: it builds a perfectly well-formed URL naming a repo that +// has never held the blob, which is a broken image for every reader and looks +// from the server side like everything worked. +// +// IT FAILS CLOSED. A postv2 view with no author is left unprojected rather than +// falling back to the community — the fallback is the tempting repair and the +// worse one, because it produces a confident URL under a DID that has never held +// the blob, where an unprojected blob ref is visibly wrong to the client. A view +// with no URI at all is not a production shape, and degrades to the community: +// that is what every record predating the flip resolves to. +func blobOwnerOf(postView *PostView) (did, pdsURL string, ok bool) { + if CollectionOfPostURI(postView.URI) == PostV2Collection { + if postView.Author == nil || postView.Author.DID == "" { + return "", "", false + } + return postView.Author.DID, postView.Author.PDSURL, true + } + + if postView.Community == nil { + return "", "", false + } + return postView.Community.DID, postView.Community.PDSURL, true } // TransformPostEmbeds enriches post embeds with resolved Bluesky post data diff --git a/internal/core/posts/postv2.go b/internal/core/posts/postv2.go index 492eb53..05dcf75 100644 --- a/internal/core/posts/postv2.go +++ b/internal/core/posts/postv2.go @@ -11,6 +11,7 @@ import ( "github.com/bluesky-social/indigo/atproto/syntax" "Coves/internal/atproto/pds" + "Coves/internal/core/blobs" ) // The author-repo half of the write path (docs/PRD_AUTHOR_OWNED_POSTS.md §3.1, @@ -197,6 +198,22 @@ type AuthorRepo interface { // DeleteRecord removes a record from the author's repo. DeleteRecord(ctx context.Context, collection, rkey string) error + // UploadBlob puts a post's media into the author's own storage. + // + // THE BLOB HAS TO TRAVEL WITH THE RECORD. A blob ref names a CID and not a + // repository, so a reader resolves it against the repo it believes owns the + // record — the author's. A thumbnail left in the community's storage + // therefore produces a record that looks identical to a correct one and + // resolves for nobody, is garbage-collectable by a repo that references it + // nowhere, and is not the author's to release when they delete the post. + // + // It is on THIS interface rather than reached through blobs.Service because + // an author authenticates with a DPoP-signed OAuth session, and that cannot + // be expressed as the bearer token blobs.BlobOwner carries. The fetch and + // the size/MIME guard still come from blobs.Service.FetchImageForURL — only + // the upload leg moved. + UploadBlob(ctx context.Context, data []byte, mimeType string) (*blobs.BlobRef, error) + // DID is the repo being written — the author's own identity, which is the // authority half of every post URI this path produces. DID() string diff --git a/internal/core/posts/service.go b/internal/core/posts/service.go index 9c6c035..fcf3de1 100644 --- a/internal/core/posts/service.go +++ b/internal/core/posts/service.go @@ -93,8 +93,8 @@ func NewPostService( // 4. Admission: one decision over community existence, visibility, ban, // aggregator authorization, dedupe and the per-author quota (admitPost) // 5. Open the AUTHOR's repository under the author's own credentials -// 6. Ensure the community has fresh PDS credentials (the blob uploads in -// step 8 still land in the community's repo until task 7 moves them) +// 6. Ensure the community has fresh PDS credentials — a step with no consumer +// left on this path; see the note at the call site // 7. Build the postv2 record // 8. Validate and enhance external embeds (thumb validation, unfurl, blobs) // 9. Create-only write at the deterministic rkey @@ -232,8 +232,19 @@ func (s *postService) CreatePost(ctx context.Context, session *oauth.ClientSessi } // 6. Ensure community has fresh PDS credentials (token refresh if needed). - // Still needed because the thumbnail blobs an external embed uploads are - // still written to the COMMUNITY's repo; task 7 moves them to the author's. + // + // THIS STEP NOW HAS NO CONSUMER ON THIS PATH, and it should go. It existed + // for the two writes that used the community's token — the post record and + // its thumbnail blob — and both have moved to the author's repository. The + // refreshed community is not read again below; only communityDID is, and + // that was captured before the call. + // + // It survives because service_admission_test.go's token-refresh case still + // requires a failed refresh to fail the submission, and that test is not + // mine to retire. Removing it is a four-line deletion the moment it is. + // Leaving it is not free: a community whose stored refresh token has rotted + // currently blocks its authors from posting for no reason any longer + // present in the code. community, err = s.communityService.EnsureFreshToken(ctx, community) if err != nil { releaseOnFailure() @@ -244,7 +255,7 @@ func (s *postService) CreatePost(ctx context.Context, session *oauth.ClientSessi postRecord := postRecordFor(req, communityDID, time.Now().UTC().Format(time.RFC3339)) // 8. Validate and enhance external embeds - if err := s.enhanceExternalEmbed(ctx, &postRecord, req, community, actor == ActorTrustedAggregator); err != nil { + if err := s.enhanceExternalEmbed(ctx, &postRecord, req, authorRepo, actor == ActorTrustedAggregator); err != nil { releaseOnFailure() return nil, err } @@ -639,7 +650,7 @@ func postV2From(record PostRecord) PostV2Record { // // trusted marks a trusted aggregator, which supplies its own metadata and is // unfurled only for a thumbnail it did not provide. -func (s *postService) enhanceExternalEmbed(ctx context.Context, postRecord *PostRecord, req CreatePostRequest, community *communities.Community, trusted bool) error { +func (s *postService) enhanceExternalEmbed(ctx context.Context, postRecord *PostRecord, req CreatePostRequest, authorRepo AuthorRepo, trusted bool) error { if postRecord.Embed != nil { embedType, typeOk := postRecord.Embed["$type"].(string) if typeOk && embedType == "social.coves.embed.external" { @@ -655,18 +666,13 @@ func (s *postService) enhanceExternalEmbed(ctx context.Context, postRecord *Post if req.ThumbnailURL != nil && *req.ThumbnailURL != "" && trusted { log.Printf("[AGGREGATOR-THUMB] Trusted aggregator provided thumbnail: %s", *req.ThumbnailURL) - if s.blobService != nil { - blobCtx, blobCancel := context.WithTimeout(ctx, 15*time.Second) - defer blobCancel() - - blob, blobErr := s.blobService.UploadBlobFromURL(blobCtx, community, *req.ThumbnailURL) - if blobErr != nil { - log.Printf("[AGGREGATOR-THUMB] Failed to upload thumbnail: %v", blobErr) - // No fallback - aggregators only use RSS feed thumbnails - } else { - external["thumb"] = blob - log.Printf("[AGGREGATOR-THUMB] Successfully uploaded thumbnail from trusted aggregator") - } + blob, blobErr := s.uploadThumbnail(ctx, authorRepo, *req.ThumbnailURL) + if blobErr != nil { + log.Printf("[AGGREGATOR-THUMB] Failed to upload thumbnail: %v", blobErr) + // No fallback - aggregators only use RSS feed thumbnails + } else if blob != nil { + external["thumb"] = blob + log.Printf("[AGGREGATOR-THUMB] Successfully uploaded thumbnail from trusted aggregator") } } @@ -711,18 +717,13 @@ func (s *postService) enhanceExternalEmbed(ctx context.Context, postRecord *Post // Upload thumbnail from unfurl if client didn't provide one // (Thumb validation already happened above) - if external["thumb"] == nil { - if result.ThumbnailURL != "" && s.blobService != nil { - blobCtx, blobCancel := context.WithTimeout(ctx, 15*time.Second) - defer blobCancel() - - blob, blobErr := s.blobService.UploadBlobFromURL(blobCtx, community, result.ThumbnailURL) - if blobErr != nil { - log.Printf("[POST-CREATE] Warning: Failed to upload thumbnail for %s: %v", uri, blobErr) - } else { - external["thumb"] = blob - log.Printf("[POST-CREATE] Uploaded thumbnail blob for %s", uri) - } + if external["thumb"] == nil && result.ThumbnailURL != "" { + blob, blobErr := s.uploadThumbnail(ctx, authorRepo, result.ThumbnailURL) + if blobErr != nil { + log.Printf("[POST-CREATE] Warning: Failed to upload thumbnail for %s: %v", uri, blobErr) + } else if blob != nil { + external["thumb"] = blob + log.Printf("[POST-CREATE] Uploaded thumbnail blob for %s", uri) } } @@ -744,6 +745,39 @@ func (s *postService) enhanceExternalEmbed(ctx context.Context, postRecord *Post return nil } +// uploadThumbnail fetches a remote thumbnail through the blob service's guard +// and puts it into the AUTHOR's own repository. +// +// THE GUARD RUNS FIRST AND THE UPLOAD ONLY HAPPENS IF IT PASSES. FetchImageForURL +// bounds the fetch with a timeout, refuses a Content-Type outside the image +// allowlist and caps the body at 6MB; a refusal returns before the author's PDS +// has been touched at all. Discarding a bad blob after uploading it would be the +// easy mistake and the wrong one — it pays the whole cost of not having a cap +// (the fetch, the transfer, the storage write, the author's quota) and only +// declines to show the result. +// +// A nil blob with a nil error means there was nothing to do: no blob service is +// wired, or no author repo is available. Both are wiring states a post survives +// — a thumbnail is an enhancement, and it has never been able to fail a post. +func (s *postService) uploadThumbnail(ctx context.Context, authorRepo AuthorRepo, imageURL string) (*blobs.BlobRef, error) { + if s.blobService == nil || authorRepo == nil || imageURL == "" { + return nil, nil + } + + // One budget for the fetch and the upload together, as the single + // UploadBlobFromURL call used to have: the pair is one enhancement, and + // letting each half have its own would double the worst case a post waits. + blobCtx, cancel := context.WithTimeout(ctx, 15*time.Second) + defer cancel() + + data, mimeType, err := s.blobService.FetchImageForURL(blobCtx, imageURL) + if err != nil { + return nil, err + } + + return authorRepo.UploadBlob(blobCtx, data, mimeType) +} + // validateCreateRequest validates basic input requirements func (s *postService) validateCreateRequest(req *CreatePostRequest) error { // Global content limits (from lexicon) diff --git a/internal/db/postgres/post_repo.go b/internal/db/postgres/post_repo.go index f880bb2..a41444e 100644 --- a/internal/db/postgres/post_repo.go +++ b/internal/db/postgres/post_repo.go @@ -436,6 +436,12 @@ func scanPostView(rows *sql.Rows, extraDest ...interface{}) (*posts.PostView, er if avatarURL := blobs.HydrateImageURL(blobs.GetImageURLConfig(), authorPDSURL.String, authorView.DID, authorAvatar.String, "avatar_small"); avatarURL != "" { authorView.Avatar = &avatarURL } + // CARRIED, not just used for the avatar above. A postv2 post's media lives + // in the AUTHOR's repository, so the blob transform needs to know which + // server holds it; this column has always been selected and always dropped + // here, which would leave every author-owned post's images addressed to an + // empty host. + authorView.PDSURL = authorPDSURL.String postView.Author = &authorView // Build community ref