Skip to main content

max / makenotwork

server: seal S3 keys behind a typed builder; close storage A+ gaps (ultra-fuzz Run #1 --deep Phase 3) Introduce an S3Key newtype with exactly two origins: a S3Client::generate_* constructor (the single reviewed home for key layout) or S3Key::from_stored (the named DB-trust boundary for keys read back from a row). Every write/presign/delete on StorageBackend now takes &S3Key, so a hand-built format!() key can no longer reach S3 — the OTA, synckit-blob, and content-export sites that built keys inline now route through new generators (generate_ota_artifact_key, generate_synckit_blob_key, generate_content_export_key). sqlx(transparent) keeps binds/decodes transparent; Deref<str> keeps reads ergonomic; Deref coercion means &str-taking helpers are unaffected. Add a UNIQUE index on ota_artifacts.s3_key (migration 149) as the multi-row backstop, mirroring the versions (148) and gallery (147) indexes. Move the item audio/video/cover replace's old-key deletion enqueue inside the confirm transaction (was best-effort post-commit), matching the project-image path and closing the crash-between-commit-and-enqueue orphan window. Reviewed the quarantine path: a mid-quarantine crash is covered by scan-job retry (mark_done runs only after quarantine; purge is idempotent), and gated kinds correctly use immediate-delete + WAM because the durable queue cannot delete a still-referenced key. No change warranted. Gate: storage/gallery/versions/scanning/synckit/ota/media workflows 125, storage units 57 green; migration 149 applies; clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-22 22:25 UTC
Commit: 150f0c25b72841571b50557e735136e45f2a9ee2
Parent: ffe89f4
20 files changed, +230 insertions, -86 deletions
A server/migrations/149_ota_artifacts_s3_key_unique.sql +31
@@ -0,0 +1,31 @@
1 + -- Backstop: one OTA artifact row per S3 object (ultra-fuzz Run #1 --deep, Storage).
2 + --
3 + -- OTA artifact keys are `ota/{app_id}/{version}/{target}/{arch}/artifact`. They
4 + -- are already unique by construction — `ota_releases` enforces
5 + -- UNIQUE(app_id, version) and `ota_artifacts` enforces UNIQUE(release_id, target,
6 + -- arch), so two artifact rows cannot derive the same key today. But the s3_key
7 + -- column itself carried no constraint, so a future change to the key layout (or a
8 + -- hand-built key bypassing `generate_ota_artifact_key`) could silently point two
9 + -- rows at one object and overwrite a published artifact's bytes — the same class
10 + -- the versions/gallery unique indexes (147, 148) closed. Centralizing key
11 + -- construction in `S3Client::generate_ota_artifact_key` plus this index makes a
12 + -- regression a write error, not a silent overwrite.
13 + --
14 + -- Surface any pre-existing legacy collision loudly: an OTA artifact row is a
15 + -- published release binary; dropping one automatically would lose a release, so
16 + -- it needs a human decision, not a DELETE.
17 + DO $$
18 + DECLARE
19 + dupes int;
20 + BEGIN
21 + SELECT count(*) INTO dupes FROM (
22 + SELECT s3_key FROM ota_artifacts
23 + GROUP BY s3_key HAVING count(*) > 1
24 + ) d;
25 + IF dupes > 0 THEN
26 + RAISE EXCEPTION 'Cannot enforce unique ota_artifacts.s3_key: % colliding key(s) exist. Resolve manually before applying this migration.', dupes;
27 + END IF;
28 + END $$;
29 +
30 + CREATE UNIQUE INDEX IF NOT EXISTS idx_ota_artifacts_s3_key
31 + ON ota_artifacts (s3_key);
M server/src/build_runner.rs +4 -2
@@ -384,7 +384,9 @@ async fn execute_target(
384 384 let _ = append_log_bounded(state, build.id, &format!("[{target}] {}\n", output.trim())).await;
385 385
386 386 // SCP artifact back and upload to S3
387 - let s3_key = format!("ota/{}/{}/{target_os}/{arch}/artifact", build.app_id, build.version);
387 + let s3_key = crate::storage::S3Client::generate_ota_artifact_key(
388 + build.app_id, &build.version, target_os, arch,
389 + );
388 390
389 391 // Copy artifact from remote to local temp
390 392 let local_tmp = format!("/tmp/mnw-artifact-{}-{target_os}-{arch}", build.id);
@@ -466,7 +468,7 @@ async fn execute_target(
466 468 .await;
467 469 }
468 470
469 - Ok((s3_key, signature))
471 + Ok((s3_key.into_string(), signature))
470 472 }
471 473
472 474 /// Path to a known_hosts file for build SSH connections.
M server/src/routes/api/content_insertions.rs +1 -1
@@ -105,7 +105,7 @@ pub(super) async fn presign_insertion(
105 105
106 106 Ok(Json(InsertionPresignResponse {
107 107 upload_url,
108 - s3_key,
108 + s3_key: s3_key.into_string(),
109 109 expires_in,
110 110 }))
111 111 }
M server/src/routes/api/exports/content.rs +2 -1
@@ -334,7 +334,8 @@ async fn build_content_export(
334 334
335 335 // Upload ZIP to S3 via multipart upload (streams from disk in 10 MB parts)
336 336 let timestamp = chrono::Utc::now().format("%Y%m%d-%H%M%S");
337 - let export_key = format!("{}/exports/content-{}.zip", user_id, timestamp);
337 + let export_key =
338 + crate::storage::S3Client::generate_content_export_key(user_id, &timestamp.to_string());
338 339 if let Err(e) = s3.upload_multipart(&export_key, "application/zip", &zip_path).await {
339 340 tracing::error!(error = ?e, "Failed to upload content export ZIP to S3");
340 341 return Err("Failed to upload the export to storage.".to_string());
M server/src/routes/api/guest_checkout.rs +1 -1
@@ -268,7 +268,7 @@ pub(super) async fn guest_download(
268 268 let s3 = state.s3.as_ref()
269 269 .ok_or_else(|| AppError::ServiceUnavailable("File storage is not configured".to_string()))?;
270 270
271 - let download_url = s3.presign_download(s3_key, Some(3600)).await?;
271 + let download_url = s3.presign_download(&crate::storage::S3Key::from_stored(s3_key), Some(3600)).await?;
272 272
273 273 Ok(Redirect::temporary(&download_url).into_response())
274 274 }
M server/src/routes/api/internal/uploads.rs +1 -1
@@ -82,7 +82,7 @@ pub(super) async fn presign_upload(
82 82
83 83 Ok(Json(InternalPresignResponse {
84 84 upload_url,
85 - s3_key,
85 + s3_key: s3_key.into_string(),
86 86 expires_in,
87 87 cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_string()),
88 88 }))
M server/src/routes/ota.rs +4 -5
@@ -287,9 +287,8 @@ async fn upload_artifact(
287 287 .find(|r| r.id == release_id)
288 288 .ok_or(AppError::NotFound)?;
289 289
290 - let s3_key = format!(
291 - "ota/{}/{}/{}/{}/artifact",
292 - app.id, release.version, req.target, req.arch
290 + let s3_key = crate::storage::S3Client::generate_ota_artifact_key(
291 + app.id, &release.version, &req.target, &req.arch,
293 292 );
294 293
295 294 let synckit_s3 = state.require_synckit_s3()?;
@@ -313,7 +312,7 @@ async fn upload_artifact(
313 312
314 313 Ok((
315 314 axum::http::StatusCode::CREATED,
316 - Json(UploadArtifactResponse { upload_url, s3_key }),
315 + Json(UploadArtifactResponse { upload_url, s3_key: s3_key.into_string() }),
317 316 ))
318 317 }
319 318
@@ -404,7 +403,7 @@ async fn artifact_download(
404 403 let synckit_s3 = state.require_synckit_s3()?;
405 404
406 405 let download_url = synckit_s3
407 - .presign_download(&artifact.s3_key, Some(constants::OTA_PRESIGN_EXPIRY_SECS))
406 + .presign_download(&crate::storage::S3Key::from_stored(&artifact.s3_key), Some(constants::OTA_PRESIGN_EXPIRY_SECS))
408 407 .await?;
409 408
410 409 Ok((
M server/src/routes/pages/public/content/item.rs +1 -1
@@ -344,7 +344,7 @@ pub(super) async fn build_segments_json(
344 344 cached.clone()
345 345 } else {
346 346 match s3
347 - .presign_download(&p.insertion_storage_key, Some(3600))
347 + .presign_download(&crate::storage::S3Key::from_stored(&p.insertion_storage_key), Some(3600))
348 348 .await
349 349 {
350 350 Ok(url) => {
M server/src/routes/pages/public/content/library.rs +2 -2
@@ -265,7 +265,7 @@ async fn render_audio_library(
265 265 .clamp(3600, constants::STREAMING_CACHE_MAX_SECS),
266 266 None => 3600,
267 267 };
268 - match s3.presign_download(s3_key, Some(expiry_secs)).await {
268 + match s3.presign_download(&crate::storage::S3Key::from_stored(s3_key), Some(expiry_secs)).await {
269 269 Ok(url) => Some(url),
270 270 Err(e) => {
271 271 tracing::warn!(s3_key = %s3_key, error = ?e, "failed to generate presigned url");
@@ -334,7 +334,7 @@ async fn render_video_library(
334 334 .clamp(3600, constants::STREAMING_CACHE_MAX_SECS),
335 335 None => 3600,
336 336 };
337 - match s3.presign_download(s3_key, Some(expiry_secs)).await {
337 + match s3.presign_download(&crate::storage::S3Key::from_stored(s3_key), Some(expiry_secs)).await {
338 338 Ok(url) => Some(url),
339 339 Err(e) => {
340 340 tracing::warn!(s3_key = %s3_key, error = ?e, "failed to generate presigned video url");
M server/src/routes/storage/downloads.rs +1 -1
@@ -43,7 +43,7 @@ async fn resolve_content_url(
43 43 if is_free && let Some(cdn_base) = cdn_base_url {
44 44 return Ok((format!("{}/{}", cdn_base, s3_key), 0));
45 45 }
46 - let url = s3.presign_download(s3_key, Some(expiry_secs))
46 + let url = s3.presign_download(&crate::storage::S3Key::from_stored(s3_key), Some(expiry_secs))
47 47 .await
48 48 .context("presign download for content")?;
49 49 Ok((url, expiry_secs))
M server/src/routes/storage/gallery.rs +1 -1
@@ -167,7 +167,7 @@ pub(super) async fn gallery_presign(
167 167
168 168 Ok(Json(PresignUploadResponse {
169 169 upload_url,
170 - s3_key,
170 + s3_key: s3_key.into_string(),
171 171 expires_in,
172 172 cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_string()),
173 173 max_file_bytes: None,
M server/src/routes/storage/images.rs +2 -2
@@ -96,7 +96,7 @@ pub(super) async fn project_image_presign(
96 96
97 97 Ok(Json(PresignUploadResponse {
98 98 upload_url,
99 - s3_key,
99 + s3_key: s3_key.into_string(),
100 100 expires_in,
101 101 cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_string()),
102 102 max_file_bytes: None,
@@ -354,7 +354,7 @@ pub(super) async fn item_image_presign(
354 354
355 355 Ok(Json(PresignUploadResponse {
356 356 upload_url,
357 - s3_key,
357 + s3_key: s3_key.into_string(),
358 358 expires_in,
359 359 cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_string()),
360 360 max_file_bytes: None,
M server/src/routes/storage/media.rs +1 -1
@@ -182,7 +182,7 @@ pub(super) async fn media_presign(
182 182
183 183 Ok(Json(PresignUploadResponse {
184 184 upload_url,
185 - s3_key,
185 + s3_key: s3_key.into_string(),
186 186 expires_in,
187 187 cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_string()),
188 188 max_file_bytes: None,
M server/src/routes/storage/uploads.rs +18 -7
@@ -105,7 +105,7 @@ pub(super) async fn presign_upload(
105 105
106 106 Ok(Json(PresignUploadResponse {
107 107 upload_url,
108 - s3_key,
108 + s3_key: s3_key.into_string(),
109 109 expires_in,
110 110 cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_string()),
111 111 max_file_bytes,
@@ -261,6 +261,21 @@ pub(super) async fn confirm_upload(
261 261 Ok(0)
262 262 }
263 263 db::items::FileConfirmOutcome::Committed => {
264 + // Enqueue the OLD object for deletion in the SAME tx as the row
265 + // swap, closing the crash-between-commit-and-enqueue orphan
266 + // window (ultra-fuzz Run #1 Storage LOW; mirrors the project-
267 + // image replace path in images.rs). After commit the row points
268 + // at the new key, so the old key is non-live; the worker's
269 + // is_s3_key_live check is the backstop if a row still references
270 + // it.
271 + if let Some(old_key) = old_s3_key.as_deref() {
272 + db::pending_s3_deletions::enqueue_deletions(
273 + &mut *tx,
274 + &[(old_key.to_string(), "main".to_string())],
275 + "item_upload_replace",
276 + )
277 + .await?;
278 + }
264 279 tx.commit().await?;
265 280 Ok(1)
266 281 }
@@ -307,12 +322,8 @@ pub(super) async fn confirm_upload(
307 322 file_size_bytes,
308 323 ).await?;
309 324
310 - // Delete the old S3 object now that the replacement is committed. Route
311 - // through the orphan queue so a transient S3 failure here doesn't leak the
312 - // object forever — the worker retries until the delete succeeds (or 404s).
313 - if let Some(old_key) = &old_s3_key {
314 - super::enqueue_s3_orphan(&state.db, old_key, "item_upload_replace").await;
315 - }
325 + // (The old S3 object on a replace was enqueued for deletion inside the
326 + // confirm tx above, so a crash here can't orphan it.)
316 327
317 328 // Clear the pending upload record now that the upload is confirmed
318 329 db::pending_uploads::remove_pending_upload(&state.db, user.id, &req.s3_key, "main").await?;
M server/src/routes/storage/versions.rs +1 -1
@@ -92,7 +92,7 @@ pub(super) async fn version_presign_upload(
92 92
93 93 Ok(Json(PresignUploadResponse {
94 94 upload_url,
95 - s3_key,
95 + s3_key: s3_key.into_string(),
96 96 expires_in,
97 97 cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_string()),
98 98 max_file_bytes,
M server/src/routes/synckit/blobs.rs +7 -3
@@ -71,7 +71,9 @@ pub(super) async fn blob_upload_url(
71 71 }));
72 72 }
73 73
74 - let s3_key = format!("{}/{}/{}", sync_user.app_id, sync_user.user_id, req.hash);
74 + let s3_key = crate::storage::S3Client::generate_synckit_blob_key(
75 + sync_user.app_id, sync_user.user_id, &req.hash,
76 + );
75 77
76 78 // Track the pending upload so the reaper can clean it up if never confirmed
77 79 db::pending_uploads::record_pending_upload(&state.db, sync_user.user_id, &s3_key, "synckit").await?;
@@ -144,7 +146,9 @@ pub(super) async fn blob_confirm_upload(
144 146 }
145 147 }
146 148
147 - let s3_key = format!("{}/{}/{}", sync_user.app_id, sync_user.user_id, req.hash);
149 + let s3_key = crate::storage::S3Client::generate_synckit_blob_key(
150 + sync_user.app_id, sync_user.user_id, &req.hash,
151 + );
148 152
149 153 // Verify the object actually exists in S3
150 154 if !synckit_s3.object_exists(&s3_key).await? {
@@ -237,7 +241,7 @@ pub(super) async fn blob_download_url(
237 241
238 242 let download_url = synckit_s3
239 243 .presign_download(
240 - &blob.s3_key,
244 + &crate::storage::S3Key::from_stored(&blob.s3_key),
241 245 Some(constants::SYNCKIT_BLOB_PRESIGN_EXPIRY_SECS),
242 246 )
243 247 .await
M server/src/scanning/worker.rs +1 -1
@@ -361,7 +361,7 @@ async fn run_pipeline_and_decide(
361 361 // still-referenced gated key). Mint the authority the sealed delete
362 362 // API requires.
363 363 let auth = crate::storage::S3DeleteAuthority::new();
364 - match ctx.s3.delete_object(&auth, &job.s3_key).await {
364 + match ctx.s3.delete_object(&auth, &crate::storage::S3Key::from_stored(&job.s3_key)).await {
365 365 Ok(()) => tracing::warn!(
366 366 s3_key = %job.s3_key, target_kind = %kind.as_str(),
367 367 "purged quarantined object from storage"
M server/src/scheduler/cleanup.rs +5 -2
@@ -359,7 +359,10 @@ pub(super) async fn purge_expired_deleted_items(state: &AppState) {
359 359 if let Some(ref s3) = state.s3
360 360 && !all_s3_keys.is_empty()
361 361 {
362 - let keys_only: Vec<String> = all_s3_keys.iter().map(|(k, _)| k.clone()).collect();
362 + let keys_only: Vec<crate::storage::S3Key> = all_s3_keys
363 + .iter()
364 + .map(|(k, _)| crate::storage::S3Key::from_stored(k))
365 + .collect();
363 366 if let Err(e) = s3.delete_objects(&S3DeleteAuthority::new(), &keys_only).await {
364 367 tracing::warn!(error = ?e, "batch S3 delete failed for purged items; pending_s3_deletions queue will retry");
365 368 }
@@ -434,7 +437,7 @@ async fn delete_orphan_key_guarded(
434 437 "live-key check failed; proceeding with delete attempt");
435 438 }
436 439 }
437 - match s3.delete_object(&S3DeleteAuthority::new(), s3_key).await {
440 + match s3.delete_object(&S3DeleteAuthority::new(), &crate::storage::S3Key::from_stored(s3_key)).await {
438 441 Ok(()) => GuardedDelete::Deleted,
439 442 Err(e) => {
440 443 tracing::warn!(s3_key = %s3_key, bucket = %bucket, error = ?e, "orphan S3 delete failed");
M server/src/storage.rs +136 -43
@@ -8,9 +8,71 @@
8 8 use std::str::FromStr;
9 9
10 10 use crate::config::StorageConfig;
11 - use crate::db::{ItemId, ProjectId, UserId, VersionId};
11 + use crate::db::{ItemId, ProjectId, SyncAppId, UserId, VersionId};
12 12 use crate::error::{AppError, Result};
13 13
14 + /// A storage object key. There are exactly two ways to obtain one, and an
15 + /// ad-hoc `format!("...")` is neither:
16 + ///
17 + /// 1. A `S3Client::generate_*` constructor — the single, reviewed home for key
18 + /// *layout*. Multi-instance kinds (versions, gallery, media) take their
19 + /// uniqueness segment (a table PK or a fresh uuid) as a required argument, so
20 + /// a collidable key cannot be built; singleton kinds (audio/cover/video, OTA
21 + /// artifacts) are one-per-parent and correctly overwrite-on-replace.
22 + /// 2. [`S3Key::from_stored`] — the named trust boundary for a key that already
23 + /// exists in our storage (read back from a DB row). The caller asserts it was
24 + /// minted by a generator at write time; this is how delete/download/re-presign
25 + /// paths address objects without re-deriving their layout.
26 + ///
27 + /// Because every write/presign/delete on [`StorageBackend`] takes `&S3Key`, a
28 + /// hand-built string can never reach S3 — the OTA-style inline `format!` key
29 + /// (which bypassed the generators) is now uncompilable.
30 + #[derive(Debug, Clone, PartialEq, Eq, Hash, sqlx::Type)]
31 + #[sqlx(transparent)]
32 + pub struct S3Key(String);
33 +
34 + impl S3Key {
35 + /// Wrap a key read back from durable storage (a DB row). Names the trust
36 + /// boundary: the caller asserts this key was minted by a `generate_*`
37 + /// constructor when the object was written, not freshly invented here.
38 + pub fn from_stored(key: impl AsRef<str>) -> Self {
39 + S3Key(key.as_ref().to_string())
40 + }
41 +
42 + pub fn as_str(&self) -> &str {
43 + &self.0
44 + }
45 +
46 + pub fn into_string(self) -> String {
47 + self.0
48 + }
49 + }
50 +
51 + impl std::ops::Deref for S3Key {
52 + type Target = str;
53 + fn deref(&self) -> &str {
54 + &self.0
55 + }
56 + }
57 +
58 + impl AsRef<str> for S3Key {
59 + fn as_ref(&self) -> &str {
60 + &self.0
61 + }
62 + }
63 +
64 + impl std::fmt::Display for S3Key {
65 + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
66 + f.write_str(&self.0)
67 + }
68 + }
69 +
70 + impl PartialEq<&str> for S3Key {
71 + fn eq(&self, other: &&str) -> bool {
72 + self.0 == *other
73 + }
74 + }
75 +
14 76 /// Allowed audio file extensions and their MIME types
15 77 const ALLOWED_AUDIO_TYPES: &[(&str, &str)] = &[
16 78 ("mp3", "audio/mpeg"),
@@ -246,8 +308,8 @@ pub trait StorageBackend: Send + Sync {
246 308 /// the URL as `Content-Length` so S3 itself enforces the size cap at the
247 309 /// protocol level (prevents oversized PUTs from burning bandwidth before
248 310 /// hitting the post-PUT delete-and-charge fallback).
249 - async fn presign_upload(&self, s3_key: &str, content_type: &str, expiry_secs: Option<u64>, cache_control: Option<&str>, max_bytes: Option<i64>) -> Result<String>;
250 - async fn presign_download(&self, s3_key: &str, expiry_secs: Option<u64>) -> Result<String>;
311 + async fn presign_upload(&self, s3_key: &S3Key, content_type: &str, expiry_secs: Option<u64>, cache_control: Option<&str>, max_bytes: Option<i64>) -> Result<String>;
312 + async fn presign_download(&self, s3_key: &S3Key, expiry_secs: Option<u64>) -> Result<String>;
251 313 async fn object_exists(&self, s3_key: &str) -> Result<bool>;
252 314 async fn object_size(&self, s3_key: &str) -> Result<Option<i64>>;
253 315 async fn download_object(&self, s3_key: &str) -> Result<Vec<u8>>;
@@ -255,15 +317,15 @@ pub trait StorageBackend: Send + Sync {
255 317 /// drive the stream to disk (scanner spool) or to a layer that consumes
256 318 /// chunks directly.
257 319 async fn download_stream(&self, s3_key: &str) -> Result<s3_storage::ByteStream>;
258 - async fn upload_object(&self, s3_key: &str, content_type: &str, data: Vec<u8>, cache_control: Option<&str>) -> Result<()>;
320 + async fn upload_object(&self, s3_key: &S3Key, content_type: &str, data: Vec<u8>, cache_control: Option<&str>) -> Result<()>;
259 321 /// Delete an object. Requires an [`S3DeleteAuthority`] — route handlers
260 322 /// cannot mint one, so they must enqueue through `pending_s3_deletions`
261 323 /// instead of deleting directly (Run #18 CHRONIC B′).
262 - async fn delete_object(&self, auth: &S3DeleteAuthority, s3_key: &str) -> Result<()>;
324 + async fn delete_object(&self, auth: &S3DeleteAuthority, s3_key: &S3Key) -> Result<()>;
263 325 /// Delete a batch of objects in a single S3 `DeleteObjects` request
264 326 /// (up to 1000 keys/call). Default loops `delete_object` so test backends
265 327 /// don't have to implement it, but production should override.
266 - async fn delete_objects(&self, auth: &S3DeleteAuthority, keys: &[String]) -> Result<()> {
328 + async fn delete_objects(&self, auth: &S3DeleteAuthority, keys: &[S3Key]) -> Result<()> {
267 329 for k in keys {
268 330 if let Err(e) = self.delete_object(auth, k).await {
269 331 tracing::warn!(key = %k, error = ?e, "delete_objects: per-key delete failed");
@@ -280,7 +342,7 @@ pub trait StorageBackend: Send + Sync {
280 342 /// default that `tokio::fs::read`s the whole file into RAM + single PUT
281 343 /// silently defeats streaming, so a future backend that forgot to override
282 344 /// it would quietly lose multipart. Every backend must declare its strategy.
283 - async fn upload_multipart(&self, s3_key: &str, content_type: &str, file_path: &std::path::Path) -> Result<()>;
345 + async fn upload_multipart(&self, s3_key: &S3Key, content_type: &str, file_path: &std::path::Path) -> Result<()>;
284 346 async fn check_connectivity(&self) -> std::result::Result<(), String>;
285 347 fn bucket(&self) -> &str;
286 348 }
@@ -322,15 +384,15 @@ impl S3Client {
322 384 item_id: ItemId,
323 385 file_type: FileType,
324 386 filename: &str,
325 - ) -> String {
387 + ) -> S3Key {
326 388 let safe_filename = sanitize_filename(filename);
327 - format!(
389 + S3Key(format!(
328 390 "{}/{}/{}/{}",
329 391 user_id,
330 392 item_id,
331 393 file_type.as_str(),
332 394 safe_filename
333 - )
395 + ))
334 396 }
335 397
336 398 /// Generate an S3 key for a version download file. The version's own id is
@@ -345,42 +407,72 @@ impl S3Client {
345 407 item_id: ItemId,
346 408 version_id: VersionId,
347 409 filename: &str,
348 - ) -> String {
410 + ) -> S3Key {
349 411 let safe_filename = sanitize_filename(filename);
350 - format!(
412 + S3Key(format!(
351 413 "{}/{}/{}/{}/{}",
352 414 user_id,
353 415 item_id,
354 416 FileType::Download.as_str(),
355 417 version_id,
356 418 safe_filename
357 - )
419 + ))
358 420 }
359 421
360 422 /// Generate an S3 key for a reusable insertion clip (not tied to any item).
361 423 /// Format: {user_id}/insertions/{filename}
362 - pub fn generate_insertion_key(user_id: UserId, filename: &str) -> String {
424 + pub fn generate_insertion_key(user_id: UserId, filename: &str) -> S3Key {
363 425 let safe_filename = sanitize_filename(filename);
364 - format!("{}/insertions/{}", user_id, safe_filename)
426 + S3Key(format!("{}/insertions/{}", user_id, safe_filename))
365 427 }
366 428
367 429 /// Generate an S3 key for a media library file.
368 430 /// Format: `{user_id}/media/{folder}/{filename}` (or `{user_id}/media/{filename}` for root folder).
369 - pub fn generate_media_key(user_id: UserId, folder: &str, filename: &str) -> String {
431 + pub fn generate_media_key(user_id: UserId, folder: &str, filename: &str) -> S3Key {
370 432 let safe_filename = sanitize_filename(filename);
371 433 let safe_folder = sanitize_folder(folder);
372 434 if safe_folder.is_empty() {
373 - format!("{}/media/{}", user_id, safe_filename)
435 + S3Key(format!("{}/media/{}", user_id, safe_filename))
374 436 } else {
375 - format!("{}/media/{}/{}", user_id, safe_folder, safe_filename)
437 + S3Key(format!("{}/media/{}/{}", user_id, safe_folder, safe_filename))
376 438 }
377 439 }
378 440
379 441 /// Generate an S3 key for a project image (logo/avatar).
380 442 /// Format: projects/{project_id}/image/{sanitized_filename}
381 - pub fn generate_project_image_key(project_id: ProjectId, filename: &str) -> String {
443 + pub fn generate_project_image_key(project_id: ProjectId, filename: &str) -> S3Key {
382 444 let safe_filename = sanitize_filename(filename);
383 - format!("projects/{}/image/{}", project_id, safe_filename)
445 + S3Key(format!("projects/{}/image/{}", project_id, safe_filename))
446 + }
447 +
448 + /// Generate an S3 key for an OTA release artifact. Singleton per
449 + /// (app, version, target, arch) — the release row already enforces
450 + /// `UNIQUE(app_id, version)` and the artifact row `UNIQUE(release_id, target,
451 + /// arch)`, so re-uploading the same artifact correctly overwrites in place.
452 + /// Centralized here so OTA keys are no longer hand-built at the call site.
453 + /// Format: ota/{app_id}/{version}/{target}/{arch}/artifact
454 + pub fn generate_ota_artifact_key(
455 + app_id: SyncAppId,
456 + version: &str,
457 + target: &str,
458 + arch: &str,
459 + ) -> S3Key {
460 + S3Key(format!("ota/{app_id}/{version}/{target}/{arch}/artifact"))
461 + }
462 +
463 + /// Generate an S3 key for a SyncKit content-addressed blob. The hash is the
464 + /// uniqueness segment (and `UNIQUE(app_id, user_id, hash)` backs it), so two
465 + /// uploads of identical bytes resolve to one object by design.
466 + /// Format: {app_id}/{user_id}/{hash}
467 + pub fn generate_synckit_blob_key(app_id: SyncAppId, user_id: UserId, hash: &str) -> S3Key {
468 + S3Key(format!("{app_id}/{user_id}/{hash}"))
469 + }
470 +
471 + /// Generate an S3 key for a generated content-export archive. Ephemeral
472 + /// (presigned, then reaped); the timestamp keeps repeat exports distinct.
473 + /// Format: {user_id}/exports/content-{timestamp}.zip
474 + pub fn generate_content_export_key(user_id: UserId, timestamp: &str) -> S3Key {
475 + S3Key(format!("{user_id}/exports/content-{timestamp}.zip"))
384 476 }
385 477
386 478 /// Generate an S3 key for an item gallery image. A per-image uuid segment
@@ -392,9 +484,9 @@ impl S3Client {
392 484 item_id: ItemId,
393 485 image_uuid: uuid::Uuid,
394 486 filename: &str,
395 - ) -> String {
487 + ) -> S3Key {
396 488 let safe_filename = sanitize_filename(filename);
397 - format!("{}/{}/gallery/{}/{}", user_id, item_id, image_uuid, safe_filename)
489 + S3Key(format!("{}/{}/gallery/{}/{}", user_id, item_id, image_uuid, safe_filename))
398 490 }
399 491
400 492 /// Generate an S3 key for a project gallery image.
@@ -403,9 +495,9 @@ impl S3Client {
403 495 project_id: ProjectId,
404 496 image_uuid: uuid::Uuid,
405 497 filename: &str,
406 - ) -> String {
498 + ) -> S3Key {
407 499 let safe_filename = sanitize_filename(filename);
408 - format!("projects/{}/gallery/{}/{}", project_id, image_uuid, safe_filename)
500 + S3Key(format!("projects/{}/gallery/{}/{}", project_id, image_uuid, safe_filename))
409 501 }
410 502
411 503 /// Validate content type for the given file type
@@ -478,14 +570,14 @@ impl S3Client {
478 570 /// whose actual body length differs from `max_bytes`.
479 571 pub async fn presign_upload(
480 572 &self,
481 - s3_key: &str,
573 + s3_key: &S3Key,
482 574 content_type: &str,
483 575 expiry_secs: Option<u64>,
484 576 cache_control: Option<&str>,
485 577 max_bytes: Option<i64>,
486 578 ) -> Result<String> {
487 579 self.inner
488 - .presign_upload(s3_key, content_type, expiry_secs.unwrap_or(PRESIGN_EXPIRY_SECS), cache_control, max_bytes)
580 + .presign_upload(s3_key.as_str(), content_type, expiry_secs.unwrap_or(PRESIGN_EXPIRY_SECS), cache_control, max_bytes)
489 581 .await
490 582 .map_err(AppError::Storage)
491 583 }
@@ -493,11 +585,11 @@ impl S3Client {
493 585 /// Generate a presigned URL for downloading/streaming a file
494 586 pub async fn presign_download(
495 587 &self,
496 - s3_key: &str,
588 + s3_key: &S3Key,
497 589 expiry_secs: Option<u64>,
498 590 ) -> Result<String> {
499 591 self.inner
500 - .presign_download(s3_key, expiry_secs.unwrap_or(PRESIGN_EXPIRY_SECS))
592 + .presign_download(s3_key.as_str(), expiry_secs.unwrap_or(PRESIGN_EXPIRY_SECS))
501 593 .await
502 594 .map_err(AppError::Storage)
503 595 }
@@ -532,31 +624,32 @@ impl S3Client {
532 624 /// Upload an object to S3 from bytes
533 625 pub async fn upload_object(
534 626 &self,
535 - s3_key: &str,
627 + s3_key: &S3Key,
536 628 content_type: &str,
537 629 data: Vec<u8>,
538 630 cache_control: Option<&str>,
539 631 ) -> Result<()> {
540 632 self.inner
541 - .upload(s3_key, content_type, data, cache_control)
633 + .upload(s3_key.as_str(), content_type, data, cache_control)
542 634 .await
543 635 .map_err(AppError::Storage)
544 636 }
545 637
546 638 /// Delete an object from S3
547 - pub async fn delete_object(&self, s3_key: &str) -> Result<()> {
548 - self.inner.delete(s3_key).await.map_err(AppError::Storage)
639 + pub async fn delete_object(&self, s3_key: &S3Key) -> Result<()> {
640 + self.inner.delete(s3_key.as_str()).await.map_err(AppError::Storage)
549 641 }
550 642
551 643 /// Batched S3 delete (`DeleteObjects`, up to 1000 keys per call).
552 644 /// Chunks larger slices into 1000-key batches and logs per-key failures
553 645 /// without bubbling — the pending_s3_deletions queue is the safety net.
554 - pub async fn delete_objects(&self, keys: &[String]) -> Result<()> {
646 + pub async fn delete_objects(&self, keys: &[S3Key]) -> Result<()> {
555 647 if keys.is_empty() {
556 648 return Ok(());
557 649 }
558 650 for chunk in keys.chunks(1000) {
559 - match self.inner.delete_objects(chunk).await {
651 + let chunk: Vec<String> = chunk.iter().map(|k| k.as_str().to_string()).collect();
652 + match self.inner.delete_objects(&chunk).await {
560 653 Ok(failures) => {
561 654 for (k, msg) in failures {
562 655 tracing::warn!(key = %k, error = %msg, "S3 delete_objects: key-level failure");
@@ -569,9 +662,9 @@ impl S3Client {
569 662 }
570 663
571 664 /// Upload a file to S3 using multipart upload (10 MB parts).
572 - pub async fn upload_multipart(&self, s3_key: &str, content_type: &str, file_path: &std::path::Path) -> Result<()> {
665 + pub async fn upload_multipart(&self, s3_key: &S3Key, content_type: &str, file_path: &std::path::Path) -> Result<()> {
573 666 self.inner
574 - .upload_multipart(s3_key, content_type, file_path, None)
667 + .upload_multipart(s3_key.as_str(), content_type, file_path, None)
575 668 .await
576 669 .map_err(AppError::Storage)
577 670 }
@@ -701,16 +794,16 @@ pub async fn build_project_image_url(
701 794 if let Some(cdn_base) = cdn_base_url {
702 795 return Ok(format!("{}/{}", cdn_base, s3_key));
703 796 }
704 - s3.presign_download(s3_key, Some(86400)).await
797 + s3.presign_download(&S3Key::from_stored(s3_key), Some(86400)).await
705 798 }
706 799
707 800 #[async_trait::async_trait]
708 801 impl StorageBackend for S3Client {
709 - async fn presign_upload(&self, s3_key: &str, content_type: &str, expiry_secs: Option<u64>, cache_control: Option<&str>, max_bytes: Option<i64>) -> Result<String> {
802 + async fn presign_upload(&self, s3_key: &S3Key, content_type: &str, expiry_secs: Option<u64>, cache_control: Option<&str>, max_bytes: Option<i64>) -> Result<String> {
710 803 self.presign_upload(s3_key, content_type, expiry_secs, cache_control, max_bytes).await
711 804 }
712 805
713 - async fn presign_download(&self, s3_key: &str, expiry_secs: Option<u64>) -> Result<String> {
806 + async fn presign_download(&self, s3_key: &S3Key, expiry_secs: Option<u64>) -> Result<String> {
714 807 self.presign_download(s3_key, expiry_secs).await
715 808 }
716 809
@@ -730,16 +823,16 @@ impl StorageBackend for S3Client {
730 823 self.download_stream(s3_key).await
731 824 }
732 825
733 - async fn upload_object(&self, s3_key: &str, content_type: &str, data: Vec<u8>, cache_control: Option<&str>) -> Result<()> {
826 + async fn upload_object(&self, s3_key: &S3Key, content_type: &str, data: Vec<u8>, cache_control: Option<&str>) -> Result<()> {
734 827 self.upload_object(s3_key, content_type, data, cache_control).await
735 828 }
736 829
737 - async fn delete_object(&self, _auth: &S3DeleteAuthority, s3_key: &str) -> Result<()> {
830 + async fn delete_object(&self, _auth: &S3DeleteAuthority, s3_key: &S3Key) -> Result<()> {
738 831 // Authority proven by the caller; delegate to the inherent impl.
739 832 self.delete_object(s3_key).await
740 833 }
741 834
742 - async fn delete_objects(&self, _auth: &S3DeleteAuthority, keys: &[String]) -> Result<()> {
835 + async fn delete_objects(&self, _auth: &S3DeleteAuthority, keys: &[S3Key]) -> Result<()> {
743 836 self.delete_objects(keys).await
744 837 }
745 838
@@ -748,7 +841,7 @@ impl StorageBackend for S3Client {
748 841 .map_err(AppError::Storage)
749 842 }
750 843
751 - async fn upload_multipart(&self, s3_key: &str, content_type: &str, file_path: &std::path::Path) -> Result<()> {
844 + async fn upload_multipart(&self, s3_key: &S3Key, content_type: &str, file_path: &std::path::Path) -> Result<()> {
752 845 self.upload_multipart(s3_key, content_type, file_path).await
753 846 }
754 847
M server/tests/harness/storage.rs +10 -10
@@ -1,7 +1,7 @@
1 1 //! In-memory storage backend for integration tests.
2 2
3 3 use makenotwork::error::{AppError, Result};
4 - use makenotwork::storage::{S3DeleteAuthority, StorageBackend};
4 + use makenotwork::storage::{S3DeleteAuthority, S3Key, StorageBackend};
5 5 use std::collections::HashMap;
6 6 use std::sync::Mutex;
7 7
@@ -35,12 +35,12 @@ impl InMemoryStorage {
35 35
36 36 #[async_trait::async_trait]
37 37 impl StorageBackend for InMemoryStorage {
38 - async fn presign_upload(&self, s3_key: &str, _content_type: &str, _expiry_secs: Option<u64>, _cache_control: Option<&str>, _max_bytes: Option<i64>) -> Result<String> {
38 + async fn presign_upload(&self, s3_key: &S3Key, _content_type: &str, _expiry_secs: Option<u64>, _cache_control: Option<&str>, _max_bytes: Option<i64>) -> Result<String> {
39 39 Ok(format!("http://test-storage/{}", s3_key))
40 40 }
41 41
42 - async fn presign_download(&self, s3_key: &str, _expiry_secs: Option<u64>) -> Result<String> {
43 - if self.objects.lock().unwrap().contains_key(s3_key) {
42 + async fn presign_download(&self, s3_key: &S3Key, _expiry_secs: Option<u64>) -> Result<String> {
43 + if self.objects.lock().unwrap().contains_key(s3_key.as_str()) {
44 44 Ok(format!("http://test-storage/{}", s3_key))
45 45 } else {
46 46 Err(AppError::Storage(format!("Object not found: {}", s3_key)))
@@ -75,21 +75,21 @@ impl StorageBackend for InMemoryStorage {
75 75 Ok(s3_storage::ByteStream::from(bytes))
76 76 }
77 77
78 - async fn upload_object(&self, s3_key: &str, _content_type: &str, data: Vec<u8>, _cache_control: Option<&str>) -> Result<()> {
79 - self.objects.lock().unwrap().insert(s3_key.to_string(), data);
78 + async fn upload_object(&self, s3_key: &S3Key, _content_type: &str, data: Vec<u8>, _cache_control: Option<&str>) -> Result<()> {
79 + self.objects.lock().unwrap().insert(s3_key.as_str().to_string(), data);
80 80 Ok(())
81 81 }
82 82
83 - async fn upload_multipart(&self, s3_key: &str, _content_type: &str, file_path: &std::path::Path) -> Result<()> {
83 + async fn upload_multipart(&self, s3_key: &S3Key, _content_type: &str, file_path: &std::path::Path) -> Result<()> {
84 84 let data = tokio::fs::read(file_path)
85 85 .await
86 86 .map_err(|e| AppError::Storage(format!("read multipart source: {e}")))?;
87 - self.objects.lock().unwrap().insert(s3_key.to_string(), data);
87 + self.objects.lock().unwrap().insert(s3_key.as_str().to_string(), data);
88 88 Ok(())
89 89 }
90 90
91 - async fn delete_object(&self, _auth: &S3DeleteAuthority, s3_key: &str) -> Result<()> {
92 - self.objects.lock().unwrap().remove(s3_key);
91 + async fn delete_object(&self, _auth: &S3DeleteAuthority, s3_key: &S3Key) -> Result<()> {
92 + self.objects.lock().unwrap().remove(s3_key.as_str());
93 93 Ok(())
94 94 }
95 95