max / makenotwork
9 files changed,
+112 insertions,
-19 deletions
| @@ -60,7 +60,7 @@ Two-pass fuzz: initial scan + deep verification. Items marked REFUTED were dispr | |||
| 60 | 60 | - [x] MINOR: Tips lack `check_not_suspended()` — added (`routes/stripe/checkout/tips.rs`) | |
| 61 | 61 | - [ ] MINOR: Orphaned pending transactions on partial cart failure — mitigated by 24h stale cleanup (`routes/stripe/checkout/cart.rs:317-353`) | |
| 62 | 62 | - [x] MINOR: `CartLineItem.amount_cents` is `i32` while `Cents` wraps `i64` — changed to `i64`, removed redundant cast (`payments/checkout.rs`) | |
| 63 | - | - [ ] MINOR: No Stripe minimum amount ($0.50) enforcement before creating session (`payments/checkout.rs:108,244`) | |
| 63 | + | - [x] MINOR: No Stripe minimum amount ($0.50) enforcement before creating session — added `STRIPE_MINIMUM_CHARGE_CENTS` check to all 3 checkout methods (`payments/checkout.rs`, `constants.rs`) | |
| 64 | 64 | - [ ] MINOR: Cart items removed before Stripe session completes — cancel = empty cart, intentional UX trade-off (`routes/stripe/checkout/cart.rs:356,673`) | |
| 65 | 65 | - [ ] MINOR: Project checkout uses `ItemId::nil()` — needs schema change to support per-project unique constraint (`routes/stripe/checkout/project.rs:135,148`) | |
| 66 | 66 | - [ ] MINOR: v2 webhook handler swallows account update failures — needs v2 retry queue infrastructure (`routes/stripe/webhook_v2.rs:95-106`) | |
| @@ -106,8 +106,8 @@ Two-pass fuzz: initial scan + deep verification. Items marked REFUTED were dispr | |||
| 106 | 106 | - [x] SERIOUS: Presigned URLs have no S3 lifecycle cleanup — added `pending_uploads` table, reaper job (24h), S3 lifecycle rule (36h) in human_todo (`routes/storage/uploads.rs`) | |
| 107 | 107 | - [ ] MINOR: Storage counter decrement-then-increment for file replacement not transactional — mitigated by `recalculate_all_storage_batch` cron (`routes/storage/uploads.rs:206-225`) | |
| 108 | 108 | - [ ] MINOR: `classify_media` trusts client-supplied `content_type` for size limits — attacker wastes own quota (`media.rs:80-91`) | |
| 109 | - | - [ ] MINOR: Creator cannot preview own draft content via stream/download — needs `is_creator` check in access control (`routes/storage/downloads.rs:76-78`) | |
| 110 | - | - [ ] LOW: Play count inflatable via unauthenticated spam — needs per-IP dedup or rate limiting (`routes/storage/downloads.rs:121`) | |
| 109 | + | - [x] MINOR: Creator cannot preview own draft content via stream/download — added `is_creator` bypass for draft, scan status, and payment checks (`routes/storage/downloads.rs`) | |
| 110 | + | - [x] LOW: Play count inflatable via unauthenticated spam — added per-IP rate limit (10 burst, 1/3s) on stream/download routes + unique_play_count tracking for authenticated users (migration 103, `user_plays` table) | |
| 111 | 111 | - [ ] NOTE: `extract_s3_key_from_url` may include bucket name for path-style URLs — only used for project images which handle it (`storage.rs:427-446`) | |
| 112 | 112 | - [ ] NOTE: Project image old-file cleanup relies on URL-to-key extraction — fragile but functional (`routes/storage/images.rs:169-177`) | |
| 113 | 113 |
| @@ -0,0 +1,11 @@ | |||
| 1 | + | -- Track unique listeners per item (authenticated users only). | |
| 2 | + | -- play_count remains total plays (including replays -- valuable for creators). | |
| 3 | + | -- unique_play_count tracks distinct listeners. | |
| 4 | + | CREATE TABLE IF NOT EXISTS user_plays ( | |
| 5 | + | user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE, | |
| 6 | + | item_id UUID NOT NULL REFERENCES items(id) ON DELETE CASCADE, | |
| 7 | + | played_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), | |
| 8 | + | PRIMARY KEY (user_id, item_id) | |
| 9 | + | ); | |
| 10 | + | ||
| 11 | + | ALTER TABLE items ADD COLUMN IF NOT EXISTS unique_play_count INTEGER NOT NULL DEFAULT 0; |
| @@ -29,6 +29,8 @@ pub const ACCOUNT_DELETION_EXPIRY_SECS: i64 = 3600; // 1 hour | |||
| 29 | 29 | // -- Stripe fees (for display only — actual fees set by Stripe) -- | |
| 30 | 30 | pub const STRIPE_FEE_PERCENTAGE: f64 = 0.029; // 2.9% | |
| 31 | 31 | pub const STRIPE_FEE_FIXED_CENTS: f64 = 30.0; // $0.30 | |
| 32 | + | /// Stripe minimum charge amount in cents (USD). Charges below this are rejected. | |
| 33 | + | pub const STRIPE_MINIMUM_CHARGE_CENTS: i64 = 50; // $0.50 | |
| 32 | 34 | ||
| 33 | 35 | // -- Page / query limits -- | |
| 34 | 36 | pub const DASHBOARD_TRANSACTION_LIMIT: i64 = 100; | |
| @@ -189,6 +191,9 @@ pub const BUILD_ALLOWED_TARGETS: &[&str] = &[ | |||
| 189 | 191 | ||
| 190 | 192 | // -- Streaming -- | |
| 191 | 193 | pub const STREAMING_CACHE_MAX_SECS: u64 = 86400; // 24 hours max presigned URL lifetime | |
| 194 | + | /// Rate limit for stream/download URL requests: 1 per 3 seconds, burst of 10. | |
| 195 | + | pub const STREAM_RATE_LIMIT_MS: u64 = 3000; | |
| 196 | + | pub const STREAM_RATE_LIMIT_BURST: u32 = 10; | |
| 192 | 197 | ||
| 193 | 198 | // -- Date display formats -- | |
| 194 | 199 | pub const DATE_FMT_SHORT: &str = "%b %d"; // "Mar 25" |
| @@ -713,7 +713,7 @@ pub async fn has_public_item_by_user(pool: &PgPool, user_id: UserId) -> Result<b | |||
| 713 | 713 | Ok(exists) | |
| 714 | 714 | } | |
| 715 | 715 | ||
| 716 | - | /// Increment the play count for an item (called on audio stream request). | |
| 716 | + | /// Increment the play count for an item (called on audio/video stream request). | |
| 717 | 717 | #[tracing::instrument(skip_all)] | |
| 718 | 718 | pub async fn increment_play_count(pool: &PgPool, item_id: ItemId) -> Result<()> { | |
| 719 | 719 | sqlx::query("UPDATE items SET play_count = play_count + 1 WHERE id = $1") | |
| @@ -724,6 +724,29 @@ pub async fn increment_play_count(pool: &PgPool, item_id: ItemId) -> Result<()> | |||
| 724 | 724 | Ok(()) | |
| 725 | 725 | } | |
| 726 | 726 | ||
| 727 | + | /// Record a unique listener and increment unique_play_count if this is the first | |
| 728 | + | /// play by this user. Returns true if new unique listener, false if already counted. | |
| 729 | + | #[tracing::instrument(skip_all)] | |
| 730 | + | pub async fn record_unique_play(pool: &PgPool, user_id: super::UserId, item_id: ItemId) -> Result<bool> { | |
| 731 | + | let result = sqlx::query( | |
| 732 | + | "INSERT INTO user_plays (user_id, item_id) VALUES ($1, $2) ON CONFLICT DO NOTHING", | |
| 733 | + | ) | |
| 734 | + | .bind(user_id) | |
| 735 | + | .bind(item_id) | |
| 736 | + | .execute(pool) | |
| 737 | + | .await?; | |
| 738 | + | ||
| 739 | + | if result.rows_affected() > 0 { | |
| 740 | + | sqlx::query("UPDATE items SET unique_play_count = unique_play_count + 1 WHERE id = $1") | |
| 741 | + | .bind(item_id) | |
| 742 | + | .execute(pool) | |
| 743 | + | .await?; | |
| 744 | + | Ok(true) | |
| 745 | + | } else { | |
| 746 | + | Ok(false) | |
| 747 | + | } | |
| 748 | + | } | |
| 749 | + | ||
| 727 | 750 | /// Increment the item-level download count (called alongside per-version increment). | |
| 728 | 751 | #[tracing::instrument(skip_all)] | |
| 729 | 752 | pub async fn increment_item_download_count(pool: &PgPool, item_id: ItemId) -> Result<()> { |
| @@ -59,8 +59,10 @@ pub struct DbItem { | |||
| 59 | 59 | pub default_max_activations: Option<i32>, | |
| 60 | 60 | /// Denormalized count of completed purchases. | |
| 61 | 61 | pub sales_count: i32, | |
| 62 | - | /// Number of audio stream requests. | |
| 62 | + | /// Number of audio/video stream requests (total, including replays). | |
| 63 | 63 | pub play_count: i32, | |
| 64 | + | /// Number of unique authenticated listeners. | |
| 65 | + | pub unique_play_count: i32, | |
| 64 | 66 | /// Aggregate download count across all versions. | |
| 65 | 67 | pub download_count: i32, | |
| 66 | 68 | /// Whether Pay What You Want pricing is enabled. | |
| @@ -299,6 +301,7 @@ mod tests { | |||
| 299 | 301 | default_max_activations: None, | |
| 300 | 302 | sales_count: 0, | |
| 301 | 303 | play_count: 0, | |
| 304 | + | unique_play_count: 0, | |
| 302 | 305 | download_count: 0, | |
| 303 | 306 | pwyw_enabled: false, | |
| 304 | 307 | pwyw_min_cents: None, |
| @@ -6,6 +6,7 @@ use stripe::{ | |||
| 6 | 6 | CreateCheckoutSessionLineItems, CreateCheckoutSessionLineItemsPriceData, | |
| 7 | 7 | CreateCheckoutSessionLineItemsPriceDataProductData, Currency, | |
| 8 | 8 | }; | |
| 9 | + | use crate::constants; | |
| 9 | 10 | use crate::db::{Cents, CheckoutType, ItemId, ProjectId, PromoCodeId, SubscriptionTierId, SyncAppId, UserId}; | |
| 10 | 11 | use crate::error::{AppError, Result}; | |
| 11 | 12 | use super::StripeClient; | |
| @@ -92,6 +93,13 @@ impl StripeClient { | |||
| 92 | 93 | &self, | |
| 93 | 94 | checkout: &GuestCheckoutParams<'_>, | |
| 94 | 95 | ) -> Result<CheckoutSession> { | |
| 96 | + | if checkout.amount_cents.as_i64() > 0 && checkout.amount_cents.as_i64() < constants::STRIPE_MINIMUM_CHARGE_CENTS { | |
| 97 | + | return Err(AppError::BadRequest(format!( | |
| 98 | + | "Minimum purchase amount is ${:.2}", | |
| 99 | + | constants::STRIPE_MINIMUM_CHARGE_CENTS as f64 / 100.0 | |
| 100 | + | ))); | |
| 101 | + | } | |
| 102 | + | ||
| 95 | 103 | let mut params = CreateCheckoutSession::new(); | |
| 96 | 104 | params.mode = Some(CheckoutSessionMode::Payment); | |
| 97 | 105 | params.success_url = Some(checkout.success_url); | |
| @@ -157,6 +165,13 @@ impl StripeClient { | |||
| 157 | 165 | &self, | |
| 158 | 166 | checkout: &CheckoutParams<'_>, | |
| 159 | 167 | ) -> Result<CheckoutSession> { | |
| 168 | + | if checkout.amount_cents.as_i64() > 0 && checkout.amount_cents.as_i64() < constants::STRIPE_MINIMUM_CHARGE_CENTS { | |
| 169 | + | return Err(AppError::BadRequest(format!( | |
| 170 | + | "Minimum purchase amount is ${:.2}", | |
| 171 | + | constants::STRIPE_MINIMUM_CHARGE_CENTS as f64 / 100.0 | |
| 172 | + | ))); | |
| 173 | + | } | |
| 174 | + | ||
| 160 | 175 | let mut params = CreateCheckoutSession::new(); | |
| 161 | 176 | params.mode = Some(CheckoutSessionMode::Payment); | |
| 162 | 177 | params.success_url = Some(checkout.success_url); | |
| @@ -226,6 +241,14 @@ impl StripeClient { | |||
| 226 | 241 | &self, | |
| 227 | 242 | cart: &CartCheckoutParams<'_>, | |
| 228 | 243 | ) -> Result<CheckoutSession> { | |
| 244 | + | let total_cents: i64 = cart.line_items.iter().map(|li| li.amount_cents).sum(); | |
| 245 | + | if total_cents > 0 && total_cents < constants::STRIPE_MINIMUM_CHARGE_CENTS { | |
| 246 | + | return Err(AppError::BadRequest(format!( | |
| 247 | + | "Minimum purchase amount is ${:.2}", | |
| 248 | + | constants::STRIPE_MINIMUM_CHARGE_CENTS as f64 / 100.0 | |
| 249 | + | ))); | |
| 250 | + | } | |
| 251 | + | ||
| 229 | 252 | let mut params = CreateCheckoutSession::new(); | |
| 230 | 253 | params.mode = Some(CheckoutSessionMode::Payment); | |
| 231 | 254 | params.success_url = Some(cart.success_url); |
| @@ -812,6 +812,7 @@ mod tests { | |||
| 812 | 812 | default_max_activations: None, | |
| 813 | 813 | sales_count: 0, | |
| 814 | 814 | play_count: 0, | |
| 815 | + | unique_play_count: 0, | |
| 815 | 816 | download_count: 0, | |
| 816 | 817 | pwyw_enabled, | |
| 817 | 818 | pwyw_min_cents, |
| @@ -72,14 +72,23 @@ pub(super) async fn stream_url( | |||
| 72 | 72 | .await? | |
| 73 | 73 | .ok_or(AppError::NotFound)?; | |
| 74 | 74 | ||
| 75 | - | // Draft items cannot be streamed (unless owner - but stream is for consumers) | |
| 76 | - | if !item.is_public { | |
| 75 | + | // Check if the current user is the item's creator (for draft preview + access bypass) | |
| 76 | + | let is_creator = if let Some(ref user) = maybe_user { | |
| 77 | + | db::items::get_item_owner(&state.db, item_id).await? | |
| 78 | + | .is_some_and(|owner_id| owner_id == user.id) | |
| 79 | + | } else { | |
| 80 | + | false | |
| 81 | + | }; | |
| 82 | + | ||
| 83 | + | // Draft items can only be streamed by their creator (for preview) | |
| 84 | + | if !item.is_public && !is_creator { | |
| 77 | 85 | return Err(AppError::NotFound); | |
| 78 | 86 | } | |
| 79 | 87 | ||
| 80 | 88 | // Only allow files with Clean scan status to be streamed. | |
| 81 | 89 | // Blocks Pending (not yet scanned), Quarantined, and HeldForReview. | |
| 82 | - | if item.scan_status != db::FileScanStatus::Clean { | |
| 90 | + | // Creators can stream their own HeldForReview content for preview. | |
| 91 | + | if item.scan_status != db::FileScanStatus::Clean && !is_creator { | |
| 83 | 92 | return Err(AppError::NotFound); | |
| 84 | 93 | } | |
| 85 | 94 | ||
| @@ -90,14 +99,14 @@ pub(super) async fn stream_url( | |||
| 90 | 99 | _ => return Err(AppError::NotFound), | |
| 91 | 100 | }; | |
| 92 | 101 | ||
| 93 | - | // Access control | |
| 102 | + | // Access control — creators always have access to their own content | |
| 94 | 103 | let item_pricing = pricing::for_item(&item); | |
| 95 | 104 | let is_free = item_pricing.is_free(); | |
| 96 | 105 | ||
| 97 | - | if !is_free { | |
| 106 | + | if !is_free && !is_creator { | |
| 98 | 107 | let user = maybe_user.as_ref().ok_or(AppError::Unauthorized)?; | |
| 99 | 108 | let ctx = pricing::AccessContext { | |
| 100 | - | is_creator: false, // stream is for consumers | |
| 109 | + | is_creator: false, | |
| 101 | 110 | has_purchased: db::transactions::has_purchased_item(&state.db, user.id, item_id).await?, | |
| 102 | 111 | has_active_subscription: db::subscriptions::has_active_subscription_to_item(&state.db, user.id, item_id).await?, | |
| 103 | 112 | }; | |
| @@ -117,9 +126,14 @@ pub(super) async fn stream_url( | |||
| 117 | 126 | s3.as_ref(), state.config.cdn_base_url.as_deref(), &s3_key, is_free, expiry_secs, | |
| 118 | 127 | ).await?; | |
| 119 | 128 | ||
| 120 | - | // Increment item-level play count | |
| 129 | + | // Increment total play count (includes replays -- valuable for creators) | |
| 121 | 130 | db::items::increment_play_count(&state.db, item_id).await?; | |
| 122 | 131 | ||
| 132 | + | // Track unique listeners for authenticated users | |
| 133 | + | if let Some(ref user) = maybe_user { | |
| 134 | + | let _ = db::items::record_unique_play(&state.db, user.id, item_id).await; | |
| 135 | + | } | |
| 136 | + | ||
| 123 | 137 | Ok(Json(StreamUrlResponse { | |
| 124 | 138 | stream_url, | |
| 125 | 139 | expires_in, | |
| @@ -156,20 +170,28 @@ pub(super) async fn version_download( | |||
| 156 | 170 | .await? | |
| 157 | 171 | .ok_or(AppError::NotFound)?; | |
| 158 | 172 | ||
| 159 | - | // Unpublished items are not downloadable | |
| 160 | - | if !item.is_public { | |
| 173 | + | // Check if the current user is the item's creator | |
| 174 | + | let is_creator = if let Some(ref user) = maybe_user { | |
| 175 | + | db::items::get_item_owner(&state.db, version.item_id).await? | |
| 176 | + | .is_some_and(|owner_id| owner_id == user.id) | |
| 177 | + | } else { | |
| 178 | + | false | |
| 179 | + | }; | |
| 180 | + | ||
| 181 | + | // Unpublished items are only downloadable by their creator | |
| 182 | + | if !item.is_public && !is_creator { | |
| 161 | 183 | return Err(AppError::NotFound); | |
| 162 | 184 | } | |
| 163 | 185 | ||
| 164 | - | // Only allow files with Clean scan status to be downloaded | |
| 165 | - | if version.scan_status != db::FileScanStatus::Clean { | |
| 186 | + | // Only allow files with Clean scan status to be downloaded (creators can access their own) | |
| 187 | + | if version.scan_status != db::FileScanStatus::Clean && !is_creator { | |
| 166 | 188 | return Err(AppError::NotFound); | |
| 167 | 189 | } | |
| 168 | 190 | ||
| 169 | - | // Access control | |
| 191 | + | // Access control — creators always have access to their own content | |
| 170 | 192 | let item_pricing = pricing::for_item(&item); | |
| 171 | 193 | let is_free = item_pricing.is_free(); | |
| 172 | - | if !is_free { | |
| 194 | + | if !is_free && !is_creator { | |
| 173 | 195 | let user = maybe_user.as_ref().ok_or(AppError::Unauthorized)?; | |
| 174 | 196 | let ctx = pricing::AccessContext { | |
| 175 | 197 | is_creator: false, |
| @@ -43,9 +43,14 @@ pub fn storage_routes() -> Router<AppState> { | |||
| 43 | 43 | config: upload_rate_limit, | |
| 44 | 44 | }); | |
| 45 | 45 | ||
| 46 | + | let stream_rate_limit = crate::helpers::rate_limiter_ms(constants::STREAM_RATE_LIMIT_MS, constants::STREAM_RATE_LIMIT_BURST); | |
| 47 | + | ||
| 46 | 48 | let stream_routes = Router::new() | |
| 47 | 49 | .route("/api/stream/{item_id}", get(downloads::stream_url)) | |
| 48 | - | .route("/api/versions/{version_id}/download", get(downloads::version_download)); | |
| 50 | + | .route("/api/versions/{version_id}/download", get(downloads::version_download)) | |
| 51 | + | .route_layer(GovernorLayer { | |
| 52 | + | config: stream_rate_limit, | |
| 53 | + | }); | |
| 49 | 54 | ||
| 50 | 55 | upload_routes.merge(stream_routes) | |
| 51 | 56 | } |