fix: Mark published S3 objects public-read

Nothing has ever made an artifact readable by the world: CannedACL appears nowhere in this repository's history, so every S3 deployment has depended on a bucket policy to grant it and silently served unreachable firmware where there was none. PublishedArtifactVerifier is what surfaced it, refusing a publish whose every artifact answered 403.

The caller now states whether an object is world-readable, because it is the only thing that knows: staged bytes stay private, the published copy and a module zip do not. The parameter has no default, since a forgotten argument is a release nobody can download and nothing fails until a hub tries.

Sending the ACL is opt-in via Storage:S3:PublicReadAcl. R2 does not implement ACLs and an AWS bucket on the default Object Ownership rejects them, and on both the write fails rather than being ignored, so a bucket granting public read by policy must keep sending none.

Author: hhvrc

RepositoryServer/Config/StorageConfig.cs

@@ -50,4 +50,19 @@ public sealed class S3StorageConfig
     /// AWS region. Required for AWS S3, optional for most S3-compatible services.
     /// </summary>
     public string? Region { get; init; }
+
+    /// <summary>
+    /// Whether to mark world-readable objects public-read with a canned ACL as they are written.
+    /// </summary>
+    /// <remarks>
+    /// Off by default, because ACLs are not universally available and sending one where they are
+    /// disabled fails the write rather than being ignored: Cloudflare R2 does not implement them at
+    /// all, and an AWS bucket on the default Object Ownership of "bucket owner enforced" answers
+    /// AccessControlListNotSupported.
+    ///
+    /// Leave it off where the bucket grants public read by policy, which is the only option on those
+    /// stores. Turn it on for a bucket that has ACLs enabled and no such policy. Either way the
+    /// staging prefix is never marked public: only published artifacts and module zips are.
+    /// </remarks>
+    public bool PublicReadAcl { get; init; }
 }

RepositoryServer/Controllers/V1/CiCdController.cs

@@ -151,7 +151,7 @@ public async Task<IActionResult> PublishVersion(
         // Upload to storage
         var storagePath = $"modules/{moduleId}/{moduleVersion}/module.zip";
         using var uploadStream = new MemoryStream(fileBytes);
-        await _storage.UploadFileAsync(storagePath, uploadStream);
+        await _storage.UploadFileAsync(storagePath, uploadStream, publicRead: true);
 
         // Construct public download URL
         var cdnBase = _apiConfig.Modules.CdnBaseUrl.TrimEnd('/');

RepositoryServer/Controllers/V2/Firmware/ReleasesController.cs

@@ -389,7 +389,7 @@ await _db.FirmwareStagedArtifacts
             var stagingPath = FirmwareArtifactFileNames.BuildStagingPath(releaseId, boardId, artifactType);
 
             await using var uploadStream = new MemoryStream(bytes);
-            await _storage.UploadFileAsync(stagingPath, uploadStream, ct);
+            await _storage.UploadFileAsync(stagingPath, uploadStream, publicRead: false, ct);
 
             _db.FirmwareStagedArtifacts.Add(new FirmwareStagedArtifact
             {
@@ -509,7 +509,7 @@ public async Task<IActionResult> PublishRelease([FromRoute] Guid releaseId, Canc
                 var publishedPath = FirmwareArtifactFileNames.BuildStoragePath(
                     release.Version, staged.BoardId, staged.ArtifactType);
 
-                await _storage.CopyFileAsync(stagingPath, publishedPath, ct);
+                await _storage.CopyFileAsync(stagingPath, publishedPath, publicRead: true, ct);
                 promoted.Add(publishedPath);
             }
         }

RepositoryServer/Services/BunnyCdnStorageService.cs

@@ -18,7 +18,7 @@ public BunnyCdnStorageService(HttpClient httpClient, BunnyCdnStorageConfig confi
     }
 
     /// <inheritdoc />
-    public async Task UploadFileAsync(string path, Stream content, CancellationToken cancellationToken = default)
+    public async Task UploadFileAsync(string path, Stream content, bool publicRead, CancellationToken cancellationToken = default)
     {
         var url = $"{_storageUrl}/{path}";
 
@@ -30,7 +30,7 @@ public async Task UploadFileAsync(string path, Stream content, CancellationToken
     }
 
     /// <inheritdoc />
-    public async Task CopyFileAsync(string sourcePath, string destinationPath, CancellationToken cancellationToken = default)
+    public async Task CopyFileAsync(string sourcePath, string destinationPath, bool publicRead, CancellationToken cancellationToken = default)
     {
         // The Bunny storage API has no server-side copy, so the object is streamed back through the
         // server. Firmware artifacts are a few MB, and this runs once per artifact at publish.
@@ -39,7 +39,7 @@ public async Task CopyFileAsync(string sourcePath, string destinationPath, Cance
         response.EnsureSuccessStatusCode();
 
         await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken);
-        await UploadFileAsync(destinationPath, stream, cancellationToken);
+        await UploadFileAsync(destinationPath, stream, publicRead, cancellationToken);
     }
 
     /// <inheritdoc />

RepositoryServer/Services/IStorageService.cs

@@ -7,8 +7,9 @@ public interface IStorageService
     /// </summary>
     /// <param name="path">Path relative to storage root, e.g. "1.0.0/board-name/app.bin"</param>
     /// <param name="content">File content stream</param>
+    /// <param name="publicRead">Whether the object is served to the world at <c>CdnBaseUrl</c>.</param>
     /// <param name="cancellationToken">Cancellation token</param>
-    Task UploadFileAsync(string path, Stream content, CancellationToken cancellationToken = default);
+    Task UploadFileAsync(string path, Stream content, bool publicRead, CancellationToken cancellationToken = default);
 
     /// <summary>
     /// Copies a file within storage, overwriting the destination.
@@ -20,8 +21,9 @@ public interface IStorageService
     /// </remarks>
     /// <param name="sourcePath">Existing path relative to storage root.</param>
     /// <param name="destinationPath">Path to copy to, relative to storage root.</param>
+    /// <param name="publicRead">Whether the destination is served to the world at <c>CdnBaseUrl</c>.</param>
     /// <param name="cancellationToken">Cancellation token</param>
-    Task CopyFileAsync(string sourcePath, string destinationPath, CancellationToken cancellationToken = default);
+    Task CopyFileAsync(string sourcePath, string destinationPath, bool publicRead, CancellationToken cancellationToken = default);
 
     /// <summary>
     /// Deletes a file from storage.

RepositoryServer/Services/LocalStorageService.cs

@@ -14,7 +14,7 @@ public LocalStorageService(LocalStorageConfig config)
     }
 
     /// <inheritdoc />
-    public async Task UploadFileAsync(string path, Stream content, CancellationToken cancellationToken = default)
+    public async Task UploadFileAsync(string path, Stream content, bool publicRead, CancellationToken cancellationToken = default)
     {
         var fullPath = ResolvePath(path);
         Directory.CreateDirectory(Path.GetDirectoryName(fullPath)!);
@@ -23,7 +23,7 @@ public async Task UploadFileAsync(string path, Stream content, CancellationToken
     }
 
     /// <inheritdoc />
-    public Task CopyFileAsync(string sourcePath, string destinationPath, CancellationToken cancellationToken = default)
+    public Task CopyFileAsync(string sourcePath, string destinationPath, bool publicRead, CancellationToken cancellationToken = default)
     {
         var source = ResolvePath(sourcePath);
         var destination = ResolvePath(destinationPath);

RepositoryServer/Services/S3StorageService.cs

@@ -10,11 +10,13 @@ public sealed class S3StorageService : IStorageService, IDisposable
     private readonly IAmazonS3 _s3Client;
     private readonly string _bucketName;
     private readonly string? _keyPrefix;
+    private readonly bool _publicReadAcl;
 
     public S3StorageService(S3StorageConfig config)
     {
         _bucketName = config.BucketName;
         _keyPrefix = config.KeyPrefix?.TrimEnd('/');
+        _publicReadAcl = config.PublicReadAcl;
 
         var s3Config = new AmazonS3Config();
 
@@ -32,21 +34,22 @@ public S3StorageService(S3StorageConfig config)
     }
 
     /// <inheritdoc />
-    public async Task UploadFileAsync(string path, Stream content, CancellationToken cancellationToken = default)
+    public async Task UploadFileAsync(string path, Stream content, bool publicRead, CancellationToken cancellationToken = default)
     {
         var request = new PutObjectRequest
         {
             BucketName = _bucketName,
             Key = ResolveKey(path),
             InputStream = content,
             ContentType = "application/octet-stream",
+            CannedACL = AclFor(publicRead),
         };
 
         await _s3Client.PutObjectAsync(request, cancellationToken);
     }
 
     /// <inheritdoc />
-    public async Task CopyFileAsync(string sourcePath, string destinationPath, CancellationToken cancellationToken = default)
+    public async Task CopyFileAsync(string sourcePath, string destinationPath, bool publicRead, CancellationToken cancellationToken = default)
     {
         // Server-side copy — the object never travels through this process.
         var request = new CopyObjectRequest
@@ -55,6 +58,7 @@ public async Task CopyFileAsync(string sourcePath, string destinationPath, Cance
             SourceKey = ResolveKey(sourcePath),
             DestinationBucket = _bucketName,
             DestinationKey = ResolveKey(destinationPath),
+            CannedACL = AclFor(publicRead),
         };
 
         await _s3Client.CopyObjectAsync(request, cancellationToken);
@@ -125,6 +129,17 @@ public async IAsyncEnumerable<string> ListFilesAsync(string prefix = "", [Enumer
         } while (response.IsTruncated == true);
     }
 
+    /// <summary>
+    /// The canned ACL to send, or null to send none and leave the bucket's own rules to decide.
+    /// </summary>
+    /// <remarks>
+    /// Null is not "private": it means this server states nothing about who may read the object.
+    /// On a bucket whose Object Ownership is "bucket owner enforced", or on any store that does not
+    /// implement ACLs, sending one is an error rather than a no-op, so it has to be opt-in.
+    /// </remarks>
+    private S3CannedACL? AclFor(bool publicRead)
+        => _publicReadAcl && publicRead ? S3CannedACL.PublicRead : null;
+
     private string ResolveKey(string path) => _keyPrefix != null ? $"{_keyPrefix}/{path}" : path;
 
     public void Dispose() => _s3Client.Dispose();