fix(audit-2026-06-11/F-FS-002): belt-and-suspenders collection validation on files read/delete

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>
This commit is contained in:
MechaCat02
2026-06-12 18:26:46 +02:00
parent e676eb9ba7
commit d9fb0e7f85
2 changed files with 14 additions and 6 deletions

View File

@@ -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);