diff --git a/src/application/services/file_management_service.rs b/src/application/services/file_management_service.rs index 8a1a8a82..d617357f 100644 --- a/src/application/services/file_management_service.rs +++ b/src/application/services/file_management_service.rs @@ -43,6 +43,13 @@ pub struct FileManagementService { /// that case the cross-drive move check is skipped (the policy /// is silently off). Production DI wires it in. drive_repo: Option>, + /// Storage-usage service — used to pre-check the destination + /// drive's `used_bytes + delta ≤ quota_bytes` invariant on + /// cross-drive MOVE, matching the pre-write check the upload path + /// already performs. Without it, the check is silently skipped + /// (stub/test builders); production DI wires it in. + storage_usage: + Option>, } impl FileManagementService { @@ -67,6 +74,7 @@ impl FileManagementService { file_lifecycle_hook: None, resource_access_hook: None, drive_repo: None, + storage_usage: None, } } @@ -100,6 +108,18 @@ impl FileManagementService { self } + /// Wires the storage-usage service so `move_file_with_perms` can + /// pre-check the destination drive's quota on cross-drive moves. + pub fn with_storage_usage( + mut self, + storage_usage: Arc< + crate::application::services::storage_usage_service::StorageUsageService, + >, + ) -> Self { + self.storage_usage = Some(storage_usage); + self + } + /// Engine check for a file resource. Parses the id into a `Uuid` and /// requires the specified permission. async fn require_file_perm( @@ -338,6 +358,22 @@ impl FileManagementUseCase for FileManagementService { dst_drive_id, }, )?; + // Destination drive quota: same pre-write check the + // upload path already runs (`file_upload_service.rs` + // `check_storage_quota`), applied here so a caller + // can't sneak content past the drive cap via MOVE. + // Denial → `DomainError::QuotaExceeded` → 507 + // Insufficient Storage. Skipped when `storage_usage` + // isn't wired (stub builders) — same shape as the + // upload path's skip semantics. + if let Some(storage_usage) = &self.storage_usage + && let Some(size_bytes) = storage_usage.file_bytes(file_uuid).await? + && let Ok(size_u64) = u64::try_from(size_bytes) + { + storage_usage + .check_drive_quota(dst_drive_id, size_u64) + .await?; + } cross_drive = Some((src_drive_id, dst_drive_id)); } } @@ -375,6 +411,31 @@ impl FileManagementUseCase for FileManagementService { .await?; self.require_target_folder_perm(target_folder_id.as_deref(), Permission::Create, caller_id) .await?; + + // Destination drive quota: COPY creates a new file row that + // counts against the destination drive's `used_bytes` even + // though blob dedup means no new bytes hit the store. Same + // pre-flight shape the delta-upload path already uses. + // Skipped when `storage_usage` isn't wired (stub builders) or + // `target_folder_id` is None (root namespace — same-drive + // semantics inherit the source's cap coverage). Denial → + // `QuotaExceeded` → 507. + if let (Some(storage_usage), Some(target_folder)) = + (&self.storage_usage, target_folder_id.as_deref()) + { + let file_uuid = + Uuid::parse_str(file_id).map_err(|_| DomainError::not_found("File", file_id))?; + let target_folder_uuid = Uuid::parse_str(target_folder) + .map_err(|_| DomainError::not_found("Folder", target_folder))?; + if let Some(size_bytes) = storage_usage.file_bytes(file_uuid).await? + && let Ok(size_u64) = u64::try_from(size_bytes) + { + storage_usage + .check_drive_quota_by_folder(target_folder_uuid, size_u64) + .await?; + } + } + self.copy_file(file_id, target_folder_id, new_name.as_deref(), caller_id) .await } @@ -453,6 +514,26 @@ impl FileManagementUseCase for FileManagementService { .await?; self.require_target_folder_perm(target_parent_id.as_deref(), Permission::Create, caller_id) .await?; + + // Destination drive quota: sum the subtree's non-trashed files + // and refuse if the destination couldn't hold them. Skipped + // when `storage_usage` isn't wired or the target is root + // (same rationale as `copy_file_with_perms`). + if let (Some(storage_usage), Some(target_parent)) = + (&self.storage_usage, target_parent_id.as_deref()) + { + let source_uuid = Uuid::parse_str(source_folder_id) + .map_err(|_| DomainError::not_found("Folder", source_folder_id))?; + let target_parent_uuid = Uuid::parse_str(target_parent) + .map_err(|_| DomainError::not_found("Folder", target_parent))?; + let subtree_bytes = storage_usage.folder_subtree_bytes(source_uuid).await?; + if let Ok(subtree_u64) = u64::try_from(subtree_bytes) { + storage_usage + .check_drive_quota_by_folder(target_parent_uuid, subtree_u64) + .await?; + } + } + self.copy_folder_tree(source_folder_id, target_parent_id, dest_name) .await } diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index 20c16681..4503cf8f 100644 --- a/src/application/services/folder_service.rs +++ b/src/application/services/folder_service.rs @@ -31,6 +31,11 @@ pub struct FolderService { /// that case the cross-drive move check is skipped (the policy is /// silently off). Production DI wires it via `with_drive_repo`. drive_repo: Option>, + /// Storage-usage service — used to pre-check the destination + /// drive's `used_bytes + subtree_bytes ≤ quota_bytes` invariant + /// on cross-drive MOVE. Silently skipped when unwired (stubs). + storage_usage: + Option>, } impl FolderService { @@ -45,6 +50,7 @@ impl FolderService { authz, file_lifecycle, drive_repo: None, + storage_usage: None, } } @@ -60,6 +66,19 @@ impl FolderService { self } + /// Wires the storage-usage service so `move_folder_with_perms` + /// can pre-check the destination drive's quota on cross-drive + /// folder moves. + pub fn with_storage_usage( + mut self, + storage_usage: Arc< + crate::application::services::storage_usage_service::StorageUsageService, + >, + ) -> Self { + self.storage_usage = Some(storage_usage); + self + } + /// Batch counterpart of `get_folder`: resolve many folder ids in ONE /// query instead of one per id. Like `get_folder` it performs no /// per-folder authorization — both current callers (ACL grant listing, @@ -593,6 +612,19 @@ impl FolderUseCase for FolderService { dst_drive_id, }, )?; + // Destination drive quota: sum the moved subtree's + // non-trashed files and refuse if the destination + // couldn't hold them. Same 507 shape as the file + // path + upload path — DomainError::QuotaExceeded + // maps at the AppError boundary. + if let Some(storage_usage) = &self.storage_usage { + let subtree_bytes = storage_usage.folder_subtree_bytes(src_folder_uuid).await?; + if let Ok(subtree_u64) = u64::try_from(subtree_bytes) { + storage_usage + .check_drive_quota(dst_drive_id, subtree_u64) + .await?; + } + } cross_drive = Some((src_drive_id, dst_drive_id)); } } diff --git a/src/application/services/storage_usage_service.rs b/src/application/services/storage_usage_service.rs index 56ed625e..637419a2 100644 --- a/src/application/services/storage_usage_service.rs +++ b/src/application/services/storage_usage_service.rs @@ -213,6 +213,47 @@ impl StorageUsageService { Ok(()) } + /// Return the size in bytes of a single non-trashed file. `None` + /// if the file is trashed or absent. Used by cross-drive MOVE to + /// know how many bytes will land on the destination drive so the + /// pre-move `check_drive_quota` call can fire. + pub async fn file_bytes(&self, file_id: Uuid) -> Result, DomainError> { + let row: Option<(i64,)> = sqlx::query_as( + "SELECT size::bigint FROM storage.files WHERE id = $1 AND NOT is_trashed", + ) + .bind(file_id) + .fetch_optional(self.pool.as_ref()) + .await + .map_err(|e| DomainError::internal_error("StorageUsage", format!("file_bytes: {e}")))?; + Ok(row.map(|(s,)| s)) + } + + /// Sum the sizes of every non-trashed file whose parent folder is + /// `folder_id` itself or a descendant of it via the `lpath` ltree. + /// Used by cross-drive MOVE to know how many bytes would land on + /// the destination drive — necessary for the pre-move + /// `check_drive_quota` call. + /// + /// Returns 0 for an empty subtree AND for a non-existent + /// `folder_id` (the JOIN silently drops); callers that need to + /// distinguish those two cases must probe the folder separately. + pub async fn folder_subtree_bytes(&self, folder_id: Uuid) -> Result { + let (bytes,): (Option,) = sqlx::query_as( + "SELECT COALESCE(SUM(f.size), 0)::bigint + FROM storage.files f + JOIN storage.folders fo ON fo.id = f.folder_id + WHERE fo.lpath <@ (SELECT lpath FROM storage.folders WHERE id = $1) + AND NOT f.is_trashed", + ) + .bind(folder_id) + .fetch_one(self.pool.as_ref()) + .await + .map_err(|e| { + DomainError::internal_error("StorageUsage", format!("folder_subtree_bytes: {e}")) + })?; + Ok(bytes.unwrap_or(0)) + } + /// Same as [`Self::add_drive_storage_usage_delta`] but resolves /// the drive id from a parent folder id in a single statement. /// Avoids a separate `SELECT drive_id FROM storage.folders` round diff --git a/src/common/di.rs b/src/common/di.rs index 959ad7c2..b3a777e6 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -536,7 +536,12 @@ impl AppServiceFactory { // drive repo every other policy uses. Wired here so // `move_folder_with_perms` can enforce // `forbid_cross_drive_move` without a separate construction path. - .with_drive_repo(drive_repo.clone()), + .with_drive_repo(drive_repo.clone()) + // Destination-drive quota pre-check on cross-drive folder + // MOVE. Reuses the `check_drive_quota` the upload path + // already runs. Without this, a Move that would push the + // destination past its cap succeeds silently. + .with_storage_usage(storage_usage.clone()), ); // Built before the upload/management services so the plugin lifecycle @@ -618,7 +623,10 @@ impl AppServiceFactory { // drive repo every other policy uses. Wired here so // `move_file_with_perms` can enforce `forbid_cross_drive_move` // without a separate construction path. - .with_drive_repo(drive_repo.clone()); + .with_drive_repo(drive_repo.clone()) + // Destination-drive quota pre-check on cross-drive file + // MOVE. Same rationale as the folder side above. + .with_storage_usage(storage_usage.clone()); if let Some(hook) = resource_access_hook.clone() { svc = svc.with_resource_access_hook(hook); } diff --git a/src/infrastructure/services/webdav_lock_service.rs b/src/infrastructure/services/webdav_lock_service.rs index 919b6bfc..9bedaf99 100644 --- a/src/infrastructure/services/webdav_lock_service.rs +++ b/src/infrastructure/services/webdav_lock_service.rs @@ -321,7 +321,7 @@ mod tests { let store = WebDavLockStore::new(16); let info = lock_info("urn:token-1", Some("Second-600"), LockScope::Exclusive); - let acquired = store.acquire("/a.txt", info).expect("acquire"); + let acquired = store.acquire("/a.txt", info, None).expect("acquire"); assert_eq!(acquired.info.token, "urn:token-1"); // Resolvable by both indexes. @@ -348,12 +348,14 @@ mod tests { .acquire( "/a.txt", lock_info("urn:token-1", Some("Second-600"), LockScope::Exclusive), + None, ) .expect("first acquire"); let conflict = store.acquire( "/a.txt", lock_info("urn:token-2", Some("Second-600"), LockScope::Exclusive), + None, ); assert!(conflict.is_err()); // The original holder is returned so the caller can report it. @@ -367,6 +369,7 @@ mod tests { .acquire( "/a.txt", lock_info("urn:token-1", Some("Infinite"), LockScope::Exclusive), + None, ) .expect("acquire"); diff --git a/tests/api/drive_quota.hurl b/tests/api/drive_quota.hurl index ec7a5540..018ad6ce 100644 --- a/tests/api/drive_quota.hurl +++ b/tests/api/drive_quota.hurl @@ -302,6 +302,94 @@ jsonpath "$.blobs_deleted" exists jsonpath "$.bytes_freed" exists +# ───────────────────────────────────────────────────────────── +# Step 11 — Pre-flight quota gate on MOVE and COPY. +# +# Silent gap before 2026-07-06: +# `move_file_with_perms` / `move_folder_with_perms` +# / `copy_file_with_perms` / `copy_folder_tree_with_perms` +# never called `check_drive_quota` on the destination. +# A user could bypass a tight drive's cap by uploading +# to their unlimited personal drive first and MOVE-ing +# (or COPY-ing) into the tight drive afterwards. +# +# Fix landed in the service layer, so both REST + WebDAV + +# NC WebDAV surfaces got the check for free. This step +# locks in the 507 shape on the REST path: +# +# a) MOVE a 5 MiB file from unlimited → tight → 507. +# b) COPY a 5 MiB file from unlimited → tight → 507. +# c) Sanity — same MOVE targeted at unlimited still 200. +# ───────────────────────────────────────────────────────────── + +# Capture the 5 MiB file id currently living in the unlimited drive +# (uploaded at Step 7). We'll try to relocate it into the 100-byte +# tight drive. +GET {{base_url}}/api/files?folder_id={{unlimited_root_id}} +Authorization: Bearer {{owner_token}} + +HTTP 200 +[Captures] +big_file_id: jsonpath "$[0].id" + + +# 11a — MOVE 5 MiB file into the tight (100-byte quota) drive. +# Refused at the service pre-check: 5_242_880 + 32 > 100. +PUT {{base_url}}/api/files/{{big_file_id}}/move +Authorization: Bearer {{owner_token}} +Content-Type: application/json +{ + "folder_id": "{{tight_root_id}}" +} + +HTTP 507 + + +# 11b — COPY same file into tight drive. Same refusal shape as MOVE +# — COPY creates a NEW file row that counts against +# `drives.used_bytes` even when blob dedup means no new bytes +# hit the store. +POST {{base_url}}/api/files/copy +Authorization: Bearer {{owner_token}} +Content-Type: application/json +{ + "file_ids": ["{{big_file_id}}"], + "target_folder_id": "{{tight_root_id}}" +} + +# Batch endpoint returns 206 Partial when at least one item fails +# with a per-item error. Per-item quota rejection is the wire +# shape here — assert the 507 landed in the per-item results, +# not on the envelope. +HTTP 206 +[Asserts] +jsonpath "$.results[?(@.file_id=='{{big_file_id}}')].error" exists + + +# 11c — Sanity: the file MOVE isn't universally broken. Targeting +# the unlimited drive's own root succeeds (it's already +# there, but MOVE is idempotent for same-parent — service +# returns 200 without re-doing storage work). +PUT {{base_url}}/api/files/{{big_file_id}}/move +Authorization: Bearer {{owner_token}} +Content-Type: application/json +{ + "folder_id": "{{unlimited_root_id}}" +} + +HTTP 200 + + +# `used_bytes` on the tight drive is unchanged — the two refused +# operations above never wrote anything. +GET {{base_url}}/api/drives +Authorization: Bearer {{owner_token}} + +HTTP 200 +[Asserts] +jsonpath "$[?(@.id=='{{tight_drive_id}}')].used_bytes" == 32 + + # No cleanup tail here — `tests/api/storage_cleanup_check.sh` enumerates # every drive via `GET /api/admin/drives` and drains+deletes any that # isn't admin's default. This keeps individual Hurl tests focused on