From 4ff0c25c5260e82774bf3c5f890dd50d0ad02824 Mon Sep 17 00:00:00 2001 From: Derek Higgins Date: Tue, 2 Dec 2025 17:56:59 +0000 Subject: [PATCH 1/2] fix(files): Enforce DELETE action permission for file deletion (#4275) 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 --- .../core/storage/sqlstore/authorized_sqlstore.py | 5 ++++- src/llama_stack/providers/inline/files/localfs/files.py | 7 ++++--- src/llama_stack/providers/remote/files/openai/files.py | 9 ++++++--- src/llama_stack/providers/remote/files/s3/files.py | 9 ++++++--- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/llama_stack/core/storage/sqlstore/authorized_sqlstore.py b/src/llama_stack/core/storage/sqlstore/authorized_sqlstore.py index e6cdcc543..0eac49b67 100644 --- a/src/llama_stack/core/storage/sqlstore/authorized_sqlstore.py +++ b/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 diff --git a/src/llama_stack/providers/inline/files/localfs/files.py b/src/llama_stack/providers/inline/files/localfs/files.py index 2afe2fe5e..de51b8bd9 100644 --- a/src/llama_stack/providers/inline/files/localfs/files.py +++ b/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() diff --git a/src/llama_stack/providers/remote/files/openai/files.py b/src/llama_stack/providers/remote/files/openai/files.py index 2cfd44168..9808b5402 100644 --- a/src/llama_stack/providers/remote/files/openai/files.py +++ b/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) diff --git a/src/llama_stack/providers/remote/files/s3/files.py b/src/llama_stack/providers/remote/files/s3/files.py index ec2d8b952..ada81f2c1 100644 --- a/src/llama_stack/providers/remote/files/s3/files.py +++ b/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) From 2fce5abe34d4de8308a508dd29788f6cb5ed73cd Mon Sep 17 00:00:00 2001 From: Derek Higgins Date: Tue, 2 Dec 2025 19:08:03 +0000 Subject: [PATCH 2/2] fix: Add policies to adapters (#4277) The configured policy wasn't being passed in and instead the default was being used (e.g. in the s3 file provider) Closes: #4276 Signed-off-by: Derek Higgins --- src/llama_stack/core/resolver.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/llama_stack/core/resolver.py b/src/llama_stack/core/resolver.py index 15720df95..dbab81f23 100644 --- a/src/llama_stack/core/resolver.py +++ b/src/llama_stack/core/resolver.py @@ -374,6 +374,9 @@ async def instantiate_provider( method = "get_adapter_impl" args = [config, deps] + if "policy" in inspect.signature(getattr(module, method)).parameters: + args.append(policy) + elif isinstance(provider_spec, AutoRoutedProviderSpec): method = "get_auto_router_impl"