Skip to main content

max / makenotwork

Require CDN in production and tighten storage key checks (ultra-fuzz Run 10 Storage) - S-1: CDN_BASE_URL is now required in production (Config::from_env returns MissingCdnBaseUrl when HOST=0.0.0.0 / HTTPS HOST_URL and no CDN is set). Without a CDN, cover/download URLs are path-style presigned S3 URLs that migration 152's cover_s3_key backfill and other key-from-URL logic don't expect; refusing to start closes that whole config-conditional class. Migration 152 comment updated to state the now-enforced invariant; guard tests added. - M-1: version upload confirm now pins the full key prefix including the download/{version_id}/ segment, so a creator can't confirm one version's row against another version's key. - N-1: add a regression test pinning the S3_KEY_REFS liveness registry so a new key-bearing column can't be added without registering it for the deletion worker's delete-then-reupload protection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-30 14:13 UTC
Commit: 5716d14a05af66d5ab6484765954ab69d9d54571
Parent: b2e6376
4 files changed, +108 insertions, -8 deletions
M server/migrations/152_cover_s3_key_backfill.sql +8 -4
@@ -13,10 +13,14 @@
13 13 ALTER TABLE projects ADD COLUMN IF NOT EXISTS cover_s3_key TEXT;
14 14
15 15 -- Backfill from the canonical URL form: strip scheme + host (and any query
16 - -- string) to recover the object key. Stored cover URLs are always
17 - -- `{cdn_base}/{s3_key}`, so the path after the host IS the key. Only touch rows
18 - -- that have a URL but no key yet; a URL that isn't an http(s) path with a
19 - -- non-empty path is left NULL (nothing for the worker to protect).
16 + -- string) to recover the object key. Stored cover URLs are `{cdn_base}/{s3_key}`,
17 + -- so the path after the host IS the key. This holds because production REQUIRES
18 + -- a CDN (Config::from_env returns MissingCdnBaseUrl otherwise, ultra-fuzz Run 10
19 + -- Sto S-1); the no-CDN dev fallback stores path-style presigned URLs
20 + -- (`{endpoint}/{bucket}/{key}`) that this regex would mis-key, but that mode is
21 + -- not allowed in production where this backfill runs. Only touch rows that have a
22 + -- URL but no key yet; a URL that isn't an http(s) path with a non-empty path is
23 + -- left NULL (nothing for the worker to protect).
20 24 UPDATE projects
21 25 SET cover_s3_key = substring(cover_image_url from '^https?://[^/]+/([^?]*)')
22 26 WHERE cover_image_url IS NOT NULL
M server/src/config.rs +57 -2
@@ -331,8 +331,22 @@ impl Config {
331 331 let build_host_linux = std::env::var("BUILD_HOST_LINUX").ok();
332 332 let build_host_darwin = std::env::var("BUILD_HOST_DARWIN").ok();
333 333
334 - // CDN base URL - optional, when unset all downloads use presigned S3 URLs
335 - let cdn_base_url = std::env::var("CDN_BASE_URL").ok();
334 + // CDN base URL — REQUIRED in production, presigned-S3 fallback in dev.
335 + // Without a CDN, cover/download URLs are path-style presigned S3 URLs
336 + // (`{endpoint}/{bucket}/{key}`), a shape the cover_s3_key backfill
337 + // (migration 152) and other key-from-URL derivation do not expect. The
338 + // platform assumes a CDN; refuse to start "green" in production without
339 + // one rather than silently run an unsupported mode (ultra-fuzz Run 10 Sto S-1).
340 + let cdn_base_url = std::env::var("CDN_BASE_URL").ok().filter(|s| !s.is_empty());
341 + {
342 + let is_production = host == std::net::IpAddr::V4(std::net::Ipv4Addr::UNSPECIFIED)
343 + || std::env::var("HOST_URL")
344 + .map(|u| u.starts_with("https://"))
345 + .unwrap_or(false);
346 + if is_production && cdn_base_url.is_none() {
347 + return Err(ConfigError::MissingCdnBaseUrl);
348 + }
349 + }
336 350
337 351 let user_pages_host = std::env::var("USER_PAGES_HOST")
338 352 .ok()
@@ -672,6 +686,8 @@ pub enum ConfigError {
672 686 WeakInternalSecret,
673 687 #[error("CLI_SERVICE_TOKEN must be at least 32 characters long")]
674 688 WeakCliServiceToken,
689 + #[error("CDN_BASE_URL is required in production (HOST=0.0.0.0 or HTTPS HOST_URL detected). Without a CDN, cover/download URLs are presigned S3 URLs the storage key-derivation logic does not support. Set CDN_BASE_URL to your CDN origin.")]
690 + MissingCdnBaseUrl,
675 691 }
676 692
677 693 #[cfg(test)]
@@ -855,6 +871,45 @@ mod tests {
855 871 }
856 872
857 873 #[test]
874 + fn from_env_fails_in_production_without_cdn_base_url() {
875 + let guard = EnvGuard::new();
876 + guard.clear_all();
877 +
878 + // SAFETY: test-only, serialized by EnvGuard mutex
879 + unsafe {
880 + std::env::set_var("DATABASE_URL", "postgres://localhost/test_db");
881 + std::env::set_var("SIGNING_SECRET", "x".repeat(32)); // pass the pre-CDN gate
882 + std::env::set_var("HOST", "0.0.0.0"); // production indicator
883 + // CDN_BASE_URL deliberately unset.
884 + }
885 +
886 + let err = Config::from_env().unwrap_err();
887 + assert!(
888 + matches!(err, ConfigError::MissingCdnBaseUrl),
889 + "expected MissingCdnBaseUrl, got: {err}"
890 + );
891 + drop(guard);
892 + }
893 +
894 + #[test]
895 + fn from_env_accepts_production_with_cdn_base_url() {
896 + let guard = EnvGuard::new();
897 + guard.clear_all();
898 +
899 + // SAFETY: test-only, serialized by EnvGuard mutex
900 + unsafe {
901 + std::env::set_var("DATABASE_URL", "postgres://localhost/test_db");
902 + std::env::set_var("SIGNING_SECRET", "x".repeat(32));
903 + std::env::set_var("HOST", "0.0.0.0");
904 + std::env::set_var("CDN_BASE_URL", "https://cdn.makenot.work");
905 + }
906 +
907 + let config = Config::from_env().expect("production config with CDN should succeed");
908 + assert_eq!(config.cdn_base_url.as_deref(), Some("https://cdn.makenot.work"));
909 + drop(guard);
910 + }
911 +
912 + #[test]
858 913 fn from_env_fails_with_https_host_url_without_signing_secret() {
859 914 let guard = EnvGuard::new();
860 915 guard.clear_all();
M server/src/db/pending_s3_deletions.rs +37
@@ -188,3 +188,40 @@ pub async fn get_stale_pending(
188 188 .await?;
189 189 Ok(rows)
190 190 }
191 +
192 + #[cfg(test)]
193 + mod tests {
194 + use super::*;
195 + use std::collections::BTreeSet;
196 +
197 + /// Pin the set of S3 key-bearing columns the deletion-worker liveness check
198 + /// consults. A new key-bearing column added anywhere must be registered in
199 + /// `S3_KEY_REFS` too, or a freshly re-uploaded object reusing a queued key
200 + /// could be torpedoed by the worker. This forces any change to be deliberate
201 + /// (ultra-fuzz Run 10 Sto N-1).
202 + #[test]
203 + fn s3_key_ref_registry_covers_every_known_class() {
204 + let actual: BTreeSet<(&str, &str, &str)> =
205 + S3_KEY_REFS.iter().map(|r| (r.bucket, r.table, r.column)).collect();
206 + let expected: BTreeSet<(&str, &str, &str)> = [
207 + ("main", "media_files", "s3_key"),
208 + ("main", "versions", "s3_key"),
209 + ("main", "items", "audio_s3_key"),
210 + ("main", "items", "cover_s3_key"),
211 + ("main", "items", "video_s3_key"),
212 + ("main", "projects", "cover_s3_key"),
213 + ("main", "item_images", "s3_key"),
214 + ("main", "project_images", "s3_key"),
215 + ("main", "content_insertions", "storage_key"),
216 + ("synckit", "sync_blobs", "s3_key"),
217 + ("synckit", "ota_artifacts", "s3_key"),
218 + ]
219 + .into_iter()
220 + .collect();
221 + assert_eq!(
222 + actual, expected,
223 + "S3_KEY_REFS changed: if you added or removed a key-bearing column, update this \
224 + test and confirm the deletion-worker liveness check still covers every key class"
225 + );
226 + }
227 + }
M server/src/routes/storage/versions.rs +6 -2
@@ -127,8 +127,12 @@ pub(super) async fn version_confirm_upload(
127 127 return Err(AppError::Forbidden);
128 128 }
129 129
130 - // Validate S3 key belongs to this user + item (prevent cross-user file reference)
131 - let expected_prefix = format!("{}/{}/", user.id, version.item_id);
130 + // Validate the S3 key belongs to this user + item + THIS version. The
131 + // generated key is `{user}/{item}/download/{version_id}/{filename}`, so pin
132 + // the full prefix including the version segment — a looser `{user}/{item}/`
133 + // check would let a creator confirm one version's row against another
134 + // version's key (ultra-fuzz Run 10 Sto M-1).
135 + let expected_prefix = format!("{}/{}/download/{}/", user.id, version.item_id, version_id);
132 136 if !req.s3_key.starts_with(&expected_prefix) {
133 137 return Err(AppError::BadRequest(
134 138 "Invalid upload key".to_string(),