Skip to content

fix(files): support a fully private S3 media bucket - #287

Merged
roncodes merged 1 commit into
release/v1.6.69from
fix/private-media-bucket
Oct 7, 2026
Merged

roncodes merged 1 commit into
release/v1.6.69from
fix/private-media-bucket

Conversation

@roncodes

@roncodes roncodes commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Why

Lets Fleetbase run against an S3 bucket that is fully private: no public-read bucket policy, BlockPublicAcls/RestrictPublicBuckets on, and ObjectOwnership=BucketOwnerEnforced. The app already serves S3/GCS files through presigned URLs (File::getUrlAttribute() → temporaryUrl()). A few paths still assumed a public bucket:

  • some writes passed 'public' visibility, i.e. a public-read ACL. A bucket with BucketOwnerEnforced rejects any PUT that carries an ACL, and Laravel's put() returned false without an error, so the object was never stored;
  • some values are stored as an absolute URL string rather than a File reference. A plain bucket URL only works while the bucket is public; a stored signed URL stops working once its signature expires.

Self-hosters on local/public disks are unaffected.

Changes

  • Utils::urlToStorefrontFile: upload without 'public' visibility.
  • File::signStoredUrl() / File::s3KeyFromUrl(): recognise an absolute URL that points into the configured s3 bucket (virtual-hosted, legacy s3-region, path-style, or the disk's url), strip any old query string and sign the key again. Every other value passes through unchanged: other buckets, other hosts, UUIDs and relative values.
  • TemplateRenderService: re-signs template-builder image src at render time.
  • Signed URLs are cached for 60 of their 120 minutes (previously 115). Every URL handed out now has at least an hour left, where a cached URL could previously be handed out with 5 minutes to live.
  • Extension::icon_url cache cut from 24 h to 30 min. It was caching a 2-hour signed URL for a day.

Companion PRs: fleetbase/fleetops#353 · fleetbase/storefront#112

Making a bucket private (operators)

  1. Deploy core-api, fleetops and storefront with these changes.
  2. Check that the identity the app signs with (task role or the services.aws keys) has s3:GetObject on the bucket. Presigned URLs carry that identity's permissions.
  3. Remove any Principal: * s3:GetObject statement, then turn on all four Block Public Access settings.
  4. Verify: an unsigned object URL returns 403, and a URL from the console (signed) returns 200.

Known follow-ups (not policy-dependent)

These break once a signed URL expires, whether or not the bucket is public:

  • logos embedded in emails (mail layout, ledger invoice mail);
  • signed URLs persisted in JSON (storefront cart product_image_url, entity meta.image_url, campaign notification payloads, AI task attachments, rich-text <img> HTML);
  • fleetops Ember models sending the computed avatar_url back on save.

Branding and logos may warrant a deliberately public prefix or a CDN path with a long-lived URL.

Tests

FileModelTest covers key extraction for every URL form, rejection of lookalike hosts and other buckets, re-signing with per-key caching, and pass-through for other values. Verified via CI.

- Drop the 'public' visibility from Utils::urlToStorefrontFile: a bucket with
  BucketOwnerEnforced rejects any PUT carrying an ACL, so put() returned false.
- Add File::signStoredUrl()/s3KeyFromUrl(): turn absolute URLs stored as strings
  (legacy unsigned bucket URLs, expired signed URLs) back into a key and re-sign.
- Re-sign template builder image src at render time.
- Cache signed URLs for 60 of their 120 minutes so every URL handed out has at
  least an hour left; cut Extension icon_url cache from 24h to 30m.
@codecov

codecov Bot commented Oct 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (ab33100) to head (383c8f4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##                main      #287   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity      7931      7947   +16     
===========================================
  Files            438       438           
  Lines          25665     25693   +28     
===========================================
+ Hits           25665     25693   +28     
Flag Coverage Δ
backend 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roncodes roncodes mentioned this pull request Oct 7, 2026
@roncodes
roncodes changed the base branch from main to release/v1.6.69 October 7, 2026 05:51
@roncodes
roncodes merged commit 5e902f2 into release/v1.6.69 Oct 7, 2026
7 checks passed
@roncodes
roncodes deleted the fix/private-media-bucket branch October 7, 2026 05:53
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