From d9fb0e7f85c339d53e756c2cf45a8b0bc72ca8c6 Mon Sep 17 00:00:00 2001
From: MechaCat02 <fabian@diekaulbachs.de>
Date: Fri, 12 Jun 2026 18:26:46 +0200
Subject: [PATCH] fix(audit-2026-06-11/F-FS-002): belt-and-suspenders
 collection validation on files read/delete
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The admin files endpoints hit FsFilesRepo directly (not via
FilesServiceImpl, which validates), so only create/update guarded the
collection — head/get/list/delete built FS paths from an unvalidated
value. Today nothing can store a traversal-shaped collection, but one
future bad migration / restore tool inserting collection='../../etc'
would give the read/delete paths arbitrary host-file reach.

- FsFilesRepo::{head,get,list,delete} now call guard_collection (create
  and update already did).
- The three admin endpoints (list/get/delete) call
  validate_files_collection up front for a clean 422 instead of the
  repo guard surfacing as an opaque 500.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 crates/manager-core/src/files_api.rs  | 16 ++++++++++------
 crates/manager-core/src/files_repo.rs |  4 ++++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/crates/manager-core/src/files_api.rs b/crates/manager-core/src/files_api.rs
index dd4f90c..d63f0a4 100644
--- a/crates/manager-core/src/files_api.rs
+++ b/crates/manager-core/src/files_api.rs
@@ -22,7 +22,7 @@ use axum::http::StatusCode;
 use axum::response::{IntoResponse, Json, Response};
 use axum::routing::get;
 use axum::{Extension, Router};
-use picloud_shared::{AppId, Principal};
+use picloud_shared::{validate_files_collection, AppId, Principal};
 use serde::{Deserialize, Serialize};
 use serde_json::json;
 use uuid::Uuid;
@@ -88,11 +88,13 @@ async fn list_files(
         Capability::AppFilesRead(app_id),
     )
     .await?;
-    if q.collection.trim().is_empty() {
-        return Err(FilesApiError::Invalid(
-            "collection must not be empty".into(),
-        ));
-    }
+    // Audit 2026-06-11 (F-FS-002) — the admin endpoints hit the repo
+    // directly (not via FilesServiceImpl, which validates), so a
+    // traversal-shaped collection would otherwise only be caught by the
+    // repo's `guard_collection` as an opaque 500. Validate here for a
+    // clean 422 and as the outer layer of the belt-and-suspenders guard.
+    validate_files_collection(&q.collection)
+        .map_err(|e| FilesApiError::Invalid(e.to_string()))?;
     let page = s
         .files
         .list(
@@ -149,6 +151,7 @@ async fn get_file(
         Capability::AppFilesRead(app_id),
     )
     .await?;
+    validate_files_collection(&collection).map_err(|e| FilesApiError::Invalid(e.to_string()))?;
     let id = Uuid::parse_str(&file_id).map_err(|_| FilesApiError::NotFound)?;
     let meta = s
         .files
@@ -193,6 +196,7 @@ async fn delete_file(
         Capability::AppFilesWrite(app_id),
     )
     .await?;
+    validate_files_collection(&collection).map_err(|e| FilesApiError::Invalid(e.to_string()))?;
     let id = Uuid::parse_str(&file_id).map_err(|_| FilesApiError::NotFound)?;
     if s.files.delete(app_id, &collection, id).await?.is_none() {
         return Err(FilesApiError::NotFound);
diff --git a/crates/manager-core/src/files_repo.rs b/crates/manager-core/src/files_repo.rs
index d20225c..7579102 100644
--- a/crates/manager-core/src/files_repo.rs
+++ b/crates/manager-core/src/files_repo.rs
@@ -350,6 +350,7 @@ impl FilesRepo for FsFilesRepo {
         collection: &str,
         id: Uuid,
     ) -> Result<Option<FileMeta>, FilesRepoError> {
+        Self::guard_collection(collection)?;
         let row: Option<FileRow> = sqlx::query_as(
             "SELECT id, collection, name, content_type, size_bytes, \
                     checksum_sha256, created_at, updated_at \
@@ -369,6 +370,7 @@ impl FilesRepo for FsFilesRepo {
         collection: &str,
         id: Uuid,
     ) -> Result<Option<Vec<u8>>, FilesRepoError> {
+        Self::guard_collection(collection)?;
         let row: Option<(String,)> = sqlx::query_as(
             "SELECT checksum_sha256 FROM files \
              WHERE app_id = $1 AND collection = $2 AND id = $3",
@@ -434,6 +436,7 @@ impl FilesRepo for FsFilesRepo {
         collection: &str,
         id: Uuid,
     ) -> Result<Option<FileMeta>, FilesRepoError> {
+        Self::guard_collection(collection)?;
         // SELECT + DELETE in one tx; unlink afterwards (outside the tx).
         let mut tx = self.pool.begin().await?;
         let row: Option<FileRow> = sqlx::query_as(
@@ -479,6 +482,7 @@ impl FilesRepo for FsFilesRepo {
         cursor: Option<&str>,
         limit: u32,
     ) -> Result<FilesListPage, FilesRepoError> {
+        Self::guard_collection(collection)?;
         let limit = if limit == 0 {
             FILES_LIST_DEFAULT_LIMIT
         } else {