fix(files): Enforce DELETE action permission for file deletion

Previously, file deletion only checked READ permission via the
_lookup_file_id() method. This meant any user with READ access to
a file could also delete it, making it impossible to configure
read-only file access.

This change adds an 'action' parameter to fetch_all() and fetch_one()
in AuthorizedSqlStore, defaulting to Action.READ for backward
compatibility. The openai_delete_file() method now passes
Action.DELETE, ensuring proper RBAC enforcement.

With this fix, access policies can now distinguish between
Users who can read/list files but not delete them

Closes: #4274

Signed-off-by: Derek Higgins <derekh@redhat.com>

src/llama_stack/core/storage/sqlstore/authorized_sqlstore.py

@@ -153,6 +153,7 @@ class AuthorizedSqlStore:
         limit: int | None = None,
         order_by: list[tuple[str, Literal["asc", "desc"]]] | None = None,
         cursor: tuple[str, str] | None = None,
+        action: Action = Action.READ,
     ) -> PaginatedResponse:
         """Fetch all rows with automatic access control filtering."""
         access_where = self._build_access_control_where_clause(self.policy)
@@ -177,7 +178,7 @@ class AuthorizedSqlStore:
                 str(record_id), table, User(principal=stored_owner_principal, attributes=stored_access_attrs)
             )
 
-            if is_action_allowed(self.policy, Action.READ, sql_record, current_user):
+            if is_action_allowed(self.policy, action, sql_record, current_user):
                 filtered_rows.append(row)
 
         return PaginatedResponse(
@@ -190,6 +191,7 @@ class AuthorizedSqlStore:
         table: str,
         where: Mapping[str, Any] | None = None,
         order_by: list[tuple[str, Literal["asc", "desc"]]] | None = None,
+        action: Action = Action.READ,
     ) -> dict[str, Any] | None:
         """Fetch one row with automatic access control checking."""
         results = await self.fetch_all(
@@ -197,6 +199,7 @@ class AuthorizedSqlStore:
             where=where,
             limit=1,
             order_by=order_by,
+            action=action,
         )
 
         return results.data[0] if results.data else None

src/llama_stack/providers/inline/files/localfs/files.py

@@ -11,6 +11,7 @@ from typing import Annotated
 
 from fastapi import Depends, File, Form, Response, UploadFile
 
+from llama_stack.core.access_control.datatypes import Action
 from llama_stack.core.datatypes import AccessRule
 from llama_stack.core.id_generation import generate_object_id
 from llama_stack.core.storage.sqlstore.authorized_sqlstore import AuthorizedSqlStore
@@ -72,12 +73,12 @@ class LocalfsFilesImpl(Files):
         """Get the filesystem path for a file ID."""
         return Path(self.config.storage_dir) / file_id
 
-    async def _lookup_file_id(self, file_id: str) -> tuple[OpenAIFileObject, Path]:
+    async def _lookup_file_id(self, file_id: str, action: Action = Action.READ) -> tuple[OpenAIFileObject, Path]:
         """Look up a OpenAIFileObject and filesystem path from its ID."""
         if not self.sql_store:
             raise RuntimeError("Files provider not initialized")
 
-        row = await self.sql_store.fetch_one("openai_files", where={"id": file_id})
+        row = await self.sql_store.fetch_one("openai_files", where={"id": file_id}, action=action)
         if not row:
             raise ResourceNotFoundError(file_id, "File", "client.files.list()")
 
@@ -188,7 +189,7 @@ class LocalfsFilesImpl(Files):
     async def openai_delete_file(self, file_id: str) -> OpenAIFileDeleteResponse:
         """Delete a file."""
         # Delete physical file
-        _, file_path = await self._lookup_file_id(file_id)
+        _, file_path = await self._lookup_file_id(file_id, action=Action.DELETE)
         if file_path.exists():
             file_path.unlink()
 

src/llama_stack/providers/remote/files/openai/files.py

@@ -9,6 +9,7 @@ from typing import Annotated, Any
 
 from fastapi import Depends, File, Form, Response, UploadFile
 
+from llama_stack.core.access_control.datatypes import Action
 from llama_stack.core.datatypes import AccessRule
 from llama_stack.core.storage.sqlstore.authorized_sqlstore import AuthorizedSqlStore
 from llama_stack.core.storage.sqlstore.sqlstore import sqlstore_impl
@@ -73,11 +74,13 @@ class OpenAIFilesImpl(Files):
         """Return current UTC timestamp as int seconds."""
         return int(datetime.now(UTC).timestamp())
 
-    async def _get_file(self, file_id: str, return_expired: bool = False) -> dict[str, Any]:
+    async def _get_file(
+        self, file_id: str, return_expired: bool = False, action: Action = Action.READ
+    ) -> dict[str, Any]:
         where: dict[str, str | dict] = {"id": file_id}
         if not return_expired:
             where["expires_at"] = {">": self._now()}
-        if not (row := await self.sql_store.fetch_one("openai_files", where=where)):
+        if not (row := await self.sql_store.fetch_one("openai_files", where=where, action=action)):
             raise ResourceNotFoundError(file_id, "File", "files.list()")
         return row
 
@@ -213,7 +216,7 @@ class OpenAIFilesImpl(Files):
 
     async def openai_delete_file(self, file_id: str) -> OpenAIFileDeleteResponse:
         await self._delete_if_expired(file_id)
-        _ = await self._get_file(file_id)
+        _ = await self._get_file(file_id, action=Action.DELETE)
         await self._delete_file(file_id)
         return OpenAIFileDeleteResponse(id=file_id, deleted=True)
 

src/llama_stack/providers/remote/files/s3/files.py

@@ -15,6 +15,7 @@ from fastapi import Depends, File, Form, Response, UploadFile
 if TYPE_CHECKING:
     from mypy_boto3_s3.client import S3Client
 
+from llama_stack.core.access_control.datatypes import Action
 from llama_stack.core.datatypes import AccessRule
 from llama_stack.core.id_generation import generate_object_id
 from llama_stack.core.storage.sqlstore.authorized_sqlstore import AuthorizedSqlStore
@@ -141,11 +142,13 @@ class S3FilesImpl(Files):
         """Return current UTC timestamp as int seconds."""
         return int(datetime.now(UTC).timestamp())
 
-    async def _get_file(self, file_id: str, return_expired: bool = False) -> dict[str, Any]:
+    async def _get_file(
+        self, file_id: str, return_expired: bool = False, action: Action = Action.READ
+    ) -> dict[str, Any]:
         where: dict[str, str | dict] = {"id": file_id}
         if not return_expired:
             where["expires_at"] = {">": self._now()}
-        if not (row := await self.sql_store.fetch_one("openai_files", where=where)):
+        if not (row := await self.sql_store.fetch_one("openai_files", where=where, action=action)):
             raise ResourceNotFoundError(file_id, "File", "files.list()")
         return row
 
@@ -290,7 +293,7 @@ class S3FilesImpl(Files):
 
     async def openai_delete_file(self, file_id: str) -> OpenAIFileDeleteResponse:
         await self._delete_if_expired(file_id)
-        _ = await self._get_file(file_id)  # raises if not found
+        _ = await self._get_file(file_id, action=Action.DELETE)  # raises if not found
         await self._delete_file(file_id)
         return OpenAIFileDeleteResponse(id=file_id, deleted=True)