From d712afbc04538ccf0df34a9178fad69098c95ca3 Mon Sep 17 00:00:00 2001 From: Saim Hashmi Date: Thu, 15 Oct 2026 18:18:36 -0400 Subject: [PATCH] fix(media): stop building unauthenticated public URLs for S3-compatible avatars/icons (#11865) --- bootstrap/helpers/shared.php | 13 ++++++++++--- tests/Feature/ProfileAvatarTest.php | 6 ++++-- tests/Feature/ProjectIconTest.php | 9 +++++++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 6c359212af..6e937ca91b 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -915,6 +915,15 @@ function s3_image_url(?int $storageId, ?string $path, int $version): ?string return null; } + // S3-compatible endpoints (e.g. Cloudflare R2) require authenticated requests for + // every object, including plain GETs, so an unauthenticated direct link only works + // when a public CDN has been configured in front of the bucket. Without one, callers + // must fall back to the app's own authenticated proxy route. + $cdnUrl = instanceSettings()->image_cdn_url; + if (blank($cdnUrl)) { + return null; + } + $storage = S3Storage::query() ->whereKey($storageId) ->whereTeamId(0) @@ -925,9 +934,7 @@ function s3_image_url(?int $storageId, ?string $path, int $version): ?string return null; } - $baseUrl = instanceSettings()->image_cdn_url ?: $storage->awsUrl(); - - return rtrim($baseUrl, '/').'/'.ltrim($path, '/').'?v='.$version; + return rtrim($cdnUrl, '/').'/'.ltrim($path, '/').'?v='.$version; } /** diff --git a/tests/Feature/ProfileAvatarTest.php b/tests/Feature/ProfileAvatarTest.php index 1265040a1f..822b78a4c9 100644 --- a/tests/Feature/ProfileAvatarTest.php +++ b/tests/Feature/ProfileAvatarTest.php @@ -100,7 +100,7 @@ it('loads an S3 profile picture from the configured CDN', function () { expect(profile_avatar_url($user))->toBe("https://avatars.example.com/media/avatars/1/avatar.jpg?v={$user->updated_at->timestamp}"); }); -it('loads an S3 profile picture directly from S3 when the CDN is not configured', function () { +it('falls back to the authenticated proxy route for an S3 profile picture when the CDN is not configured', function () { Team::factory()->create(['id' => 0]); $storage = S3Storage::query()->create([ 'team_id' => 0, @@ -118,7 +118,9 @@ it('loads an S3 profile picture directly from S3 when the CDN is not configured' 'avatar_s3_storage_id' => $storage->id, ]); - expect(profile_avatar_url($user))->toBe("https://s3.example.com/avatars/avatars/1/avatar.jpg?v={$user->updated_at->timestamp}"); + // Direct S3-compatible endpoints (e.g. Cloudflare R2) require authenticated requests, + // so without a CDN the browser must use the app's authenticated proxy route. + expect(profile_avatar_url($user))->toBe(route('profile.avatar', ['v' => $user->updated_at->timestamp])); }); it('does not use an unrelated S3 storage URL for a profile picture', function () { diff --git a/tests/Feature/ProjectIconTest.php b/tests/Feature/ProjectIconTest.php index 99a6c0ac5b..ff4615ccb4 100644 --- a/tests/Feature/ProjectIconTest.php +++ b/tests/Feature/ProjectIconTest.php @@ -129,7 +129,7 @@ it('loads an S3 project icon from the configured CDN', function () { ->assertViewHas('projectsJs', fn (array $projects): bool => $projects[0]['iconUrl'] === "https://avatars.example.com/media/project-icons/{$this->project->uuid}/icon.jpg?v={$this->project->updated_at->timestamp}"); }); -it('loads an S3 project icon directly from S3 when the CDN is not configured', function () { +it('falls back to the authenticated proxy route for an S3 project icon when the CDN is not configured', function () { Team::factory()->create(['id' => 0]); $storage = S3Storage::query()->create([ 'team_id' => 0, @@ -147,7 +147,12 @@ it('loads an S3 project icon directly from S3 when the CDN is not configured', f 'icon_s3_storage_id' => $storage->id, ])->save(); - expect(project_icon_url($this->project))->toBe("https://s3.example.com/avatars/project-icons/{$this->project->uuid}/icon.jpg?v={$this->project->updated_at->timestamp}"); + // Direct S3-compatible endpoints (e.g. Cloudflare R2) require authenticated requests, + // so without a CDN the browser must use the app's authenticated proxy route. + expect(project_icon_url($this->project))->toBe(route('project.icon', [ + 'project_uuid' => $this->project->uuid, + 'v' => $this->project->updated_at->timestamp, + ])); }); it('does not use an unusable S3 storage URL for a project icon', function () {