diff --git a/README_zh-CN.md b/README_zh-CN.md index 8f6c28a1..5b04c0c8 100644 --- a/README_zh-CN.md +++ b/README_zh-CN.md @@ -85,6 +85,7 @@ conda activate labelu 4. 安装 LabelU： +安装基础版本（使用SQLite） ```bash pip install labelu ``` @@ -100,7 +101,7 @@ pip install labelu[mysql] # pip install labelu mysqlclient ``` -5. 运行： +1. 运行： ```bash labelu diff --git a/labelu/internal/adapter/persistence/crud_sample.py b/labelu/internal/adapter/persistence/crud_sample.py index 5a38443b..f070e157 100644 --- a/labelu/internal/adapter/persistence/crud_sample.py +++ b/labelu/internal/adapter/persistence/crud_sample.py @@ -2,7 +2,7 @@ from typing import Any, Dict, List, Union -from sqlalchemy import case, func, text +from sqlalchemy import case, func from sqlalchemy.orm import Session from fastapi.encoders import jsonable_encoder @@ -39,17 +39,32 @@ def list_by( whens = {state.value: index for index, state in enumerate(SampleState)} sort_logic = case(whens, value=TaskSample.state).label(TaskSample.state.key) + # Allowlist of sortable columns mapped to real ORM columns. Only these + # identifiers may reach ORDER BY; anything else is rejected so request + # input can never be interpolated into raw SQL. + sortable_columns = { + "inner_id": TaskSample.inner_id, + "updated_at": TaskSample.updated_at, + "annotated_count": TaskSample.annotated_count, + } + if sorting: sort_strings = sorting.split(",") for item in sort_strings: sort_key = item.split(":") - if sort_key[0] == TaskSample.state.key: - if sort_key[1] == "asc": - query = query.order_by(sort_logic.asc()) - else: - query = query.order_by(sort_logic.desc()) + column = sort_key[0] + direction = sort_key[1] if len(sort_key) > 1 else "asc" + if direction not in ("asc", "desc"): + continue + if column == TaskSample.state.key: + order_target = sort_logic + elif column in sortable_columns: + order_target = sortable_columns[column] else: - query = query.order_by(text(f"{sort_key[0]} {sort_key[1]}")) + continue + query = query.order_by( + order_target.asc() if direction == "asc" else order_target.desc() + ) # default order by id, before need select last items if before: diff --git a/labelu/internal/adapter/routers/datasource.py b/labelu/internal/adapter/routers/datasource.py index 51020583..f45253c6 100644 --- a/labelu/internal/adapter/routers/datasource.py +++ b/labelu/internal/adapter/routers/datasource.py @@ -72,7 +72,7 @@ async def get( db: Session = Depends(db_module.get_db), current_user: User = Depends(get_current_user), ): - data = await service.get(db=db, ds_id=ds_id) + data = await service.get(db=db, ds_id=ds_id, current_user=current_user) return OkResp[DataSourceResponse](data=data) @@ -123,7 +123,7 @@ async def list_objects( current_user: User = Depends(get_current_user), ): data = await service.list_objects( - db=db, ds_id=ds_id, prefix=prefix, extension=extension, + db=db, ds_id=ds_id, current_user=current_user, prefix=prefix, extension=extension, page_token=page_token, size=size, ) return OkResp[S3ObjectListResponse](data=data) diff --git a/labelu/internal/adapter/routers/sample.py b/labelu/internal/adapter/routers/sample.py index ddd77483..6c1d8165 100644 --- a/labelu/internal/adapter/routers/sample.py +++ b/labelu/internal/adapter/routers/sample.py @@ -30,6 +30,8 @@ from labelu.internal.application.response.auto_label import AutoLabelResponse, AutoLabelJobResponse from labelu.internal.application.response.export import ExportJobResponse from labelu.internal.adapter.persistence import crud_export_job +from labelu.internal.adapter.persistence import crud_task +from labelu.internal.application.service.access import assert_task_access from labelu.internal.common.storage import get_storage_backend @@ -84,7 +86,9 @@ async def list_by( page: Union[int, None] = Query(default=None, ge=0), size: Union[int, None] = 100, sort: Union[str, None] = Query( - default=None, pattern="(annotated_count|state|inner_id|updated_at):(desc|asc)" + default=None, + pattern=r"^(annotated_count|state|inner_id|updated_at):(desc|asc)" + r"(,(annotated_count|state|inner_id|updated_at):(desc|asc))*$", ), authorization: HTTPAuthorizationCredentials = Security(security), db: Session = Depends(db_module.get_db), @@ -109,6 +113,7 @@ async def list_by( page=page, size=size, sorting=sort, + current_user=current_user, ) # response @@ -134,7 +139,7 @@ async def get( # business logic data = await service.get( - db=db, task_id=task_id, sample_id=sample_id + db=db, task_id=task_id, sample_id=sample_id, current_user=current_user ) # response @@ -329,6 +334,10 @@ async def get_export_status( status_code=status.HTTP_404_NOT_FOUND, ) + task = crud_task.get(db=db, task_id=job.task_id) + if task is not None: + assert_task_access(task, current_user) + skipped_count, warning_message = _export_progress( sample_count=job.sample_count, processed_count=job.processed_count, @@ -373,6 +382,10 @@ async def download_export( status_code=status.HTTP_404_NOT_FOUND, ) + task = crud_task.get(db=db, task_id=job.task_id) + if task is not None: + assert_task_access(task, current_user) + storage = get_storage_backend() if storage.is_remote: download_url = storage.get_read_url(job.file_path) diff --git a/labelu/internal/adapter/routers/task.py b/labelu/internal/adapter/routers/task.py index a650ecbc..fe4d5999 100644 --- a/labelu/internal/adapter/routers/task.py +++ b/labelu/internal/adapter/routers/task.py @@ -220,7 +220,7 @@ async def update( """ # business logic - data = await service.update(db=db, task_id=task_id, cmd=cmd) + data = await service.update(db=db, task_id=task_id, cmd=cmd, current_user=current_user) # response return OkResp[TaskResponse](data=data) diff --git a/labelu/internal/application/command/datasource.py b/labelu/internal/application/command/datasource.py index ee4576b6..fd1a27ce 100644 --- a/labelu/internal/application/command/datasource.py +++ b/labelu/internal/application/command/datasource.py @@ -1,12 +1,68 @@ +import ipaddress +import socket from typing import Union +from urllib.parse import urlparse -from pydantic import BaseModel, Field, model_validator +from pydantic import BaseModel, Field, field_validator, model_validator + +from labelu.internal.common.config import settings + + +def validate_s3_endpoint(endpoint: Union[str, None]) -> Union[str, None]: + """Reject S3 endpoints that could be abused for SSRF. + + Only http(s) URLs are allowed. Link-local (cloud metadata + 169.254.0.0/16, fe80::/10), multicast, reserved and unspecified + addresses are always rejected — they are never valid S3 endpoints. + + Private (RFC1918 / ULA) and loopback addresses are allowed when + ``settings.ALLOW_PRIVATE_S3_ENDPOINT`` is true (the default, for on-prem + deployments backed by an internal MinIO/S3-compatible store) and + rejected otherwise (strict mode). + """ + if endpoint is None or endpoint == "": + return endpoint + + parsed = urlparse(endpoint) + if parsed.scheme not in ("http", "https"): + raise ValueError("endpoint must be an http(s) URL") + host = parsed.hostname + if not host: + raise ValueError("endpoint must include a host") + + try: + infos = socket.getaddrinfo(host, None) + except socket.gaierror: + raise ValueError("endpoint host cannot be resolved") + + allow_private = settings.ALLOW_PRIVATE_S3_ENDPOINT + for info in infos: + addr = ipaddress.ip_address(info[4][0]) + # Never a legitimate S3 endpoint; always rejected regardless of config. + # Note: the metadata IP (169.254.x) is also flagged is_private on newer + # Python, so link-local is checked here and must win over the private + # carve-out below. Loopback is exempt from the reserved check because + # ::1 is also flagged is_reserved. + if ( + addr.is_link_local + or addr.is_multicast + or addr.is_unspecified + or (addr.is_reserved and not addr.is_loopback) + ): + raise ValueError("endpoint host resolves to a disallowed address") + # Internal networks (RFC1918 / ULA / loopback): allowed only when + # explicitly permitted via ALLOW_PRIVATE_S3_ENDPOINT. + if (addr.is_private or addr.is_loopback) and not allow_private: + raise ValueError("endpoint host resolves to a private address") + return endpoint class CreateDataSourceCommand(BaseModel): name: str = Field(max_length=128, description="Display name") type: str = Field(default="S3", max_length=32) endpoint: Union[str, None] = Field(default=None, max_length=512) + + _validate_endpoint = field_validator("endpoint")(validate_s3_endpoint) region: Union[str, None] = Field(default=None, max_length=64) bucket: str = Field(max_length=256) prefix: str = Field(default="", max_length=512) @@ -20,6 +76,8 @@ class CreateDataSourceCommand(BaseModel): class UpdateDataSourceCommand(BaseModel): name: Union[str, None] = Field(default=None, max_length=128) endpoint: Union[str, None] = Field(default=None, max_length=512) + + _validate_endpoint = field_validator("endpoint")(validate_s3_endpoint) region: Union[str, None] = Field(default=None, max_length=64) bucket: Union[str, None] = Field(default=None, max_length=256) prefix: Union[str, None] = Field(default=None, max_length=512) diff --git a/labelu/internal/application/service/access.py b/labelu/internal/application/service/access.py new file mode 100644 index 00000000..a78b25b0 --- /dev/null +++ b/labelu/internal/application/service/access.py @@ -0,0 +1,31 @@ +from fastapi import status + +from labelu.internal.common.error_code import ErrorCode, LabelUException + + +def assert_task_access(task, current_user) -> None: + """Ensure current_user may access the given task's data. + + Access is granted to the task owner or any of its collaborators. Any other + authenticated user is rejected with 403, closing IDOR/BOLA on task-scoped + resources (samples, pre-annotations, exports, ...). + """ + collaborator_ids = {c.id for c in task.collaborators} + if task.created_by != current_user.id and current_user.id not in collaborator_ids: + raise LabelUException( + code=ErrorCode.CODE_30001_NO_PERMISSION, + status_code=status.HTTP_403_FORBIDDEN, + ) + + +def assert_owner(resource, current_user) -> None: + """Ensure current_user owns the resource (created_by). + + Used for user-private resources without a collaborator concept + (e.g. data sources). + """ + if resource.created_by != current_user.id: + raise LabelUException( + code=ErrorCode.CODE_30001_NO_PERMISSION, + status_code=status.HTTP_403_FORBIDDEN, + ) diff --git a/labelu/internal/application/service/attachment.py b/labelu/internal/application/service/attachment.py index 52a42612..d25cff5a 100644 --- a/labelu/internal/application/service/attachment.py +++ b/labelu/internal/application/service/attachment.py @@ -82,11 +82,21 @@ async def create( # file relative path path_filename = cmd.file.filename.split("/") - # filename = str(uuid.uuid4())[0:8] + "-" + path_filename[-1] NOTE: If you want keep filename safe, you can use uuid as filename + # filename = str(uuid.uuid4())[0:8] + "-" + path_filename[-1] NOTE: If you want keep filename safe, you can use uuid as filename filename = path_filename[-1] sanitized = re.sub(r'%', '_pct_', filename) sanitized = re.sub(r'[\\/*?:"<>|#]', '_', sanitized) - path = "/".join(path_filename[:-1]) + # Sanitize the directory portion of the filename too: drop traversal + # components ("..", ".", empty) and apply the same character sanitization, + # so a crafted filename can never escape the task's upload directory. + safe_parts = [] + for part in path_filename[:-1]: + part = re.sub(r'%', '_pct_', part) + part = re.sub(r'[\\/*?:"<>|#]', '_', part) + if part in ("", ".", ".."): + continue + safe_parts.append(part) + path = "/".join(safe_parts) attachment_relative_base_dir = Path(settings.UPLOAD_DIR).joinpath( str(task_id), path ) diff --git a/labelu/internal/application/service/datasource.py b/labelu/internal/application/service/datasource.py index feef3f53..f14f7d02 100644 --- a/labelu/internal/application/service/datasource.py +++ b/labelu/internal/application/service/datasource.py @@ -9,6 +9,7 @@ from sqlalchemy.orm import Session from labelu.internal.adapter.persistence import crud_datasource +from labelu.internal.application.service.access import assert_owner from labelu.internal.application.command.datasource import ( CreateDataSourceCommand, UpdateDataSourceCommand, @@ -110,13 +111,14 @@ async def list_by( return [_to_response(ds) for ds in items], total -async def get(db: Session, ds_id: int) -> DataSourceResponse: +async def get(db: Session, ds_id: int, current_user: User) -> DataSourceResponse: ds = crud_datasource.get(db=db, ds_id=ds_id) if not ds: raise LabelUException( code=ErrorCode.CODE_61000_NO_DATA, status_code=status.HTTP_404_NOT_FOUND, ) + assert_owner(ds, current_user) return _to_response(ds) @@ -129,6 +131,7 @@ async def update( code=ErrorCode.CODE_61000_NO_DATA, status_code=status.HTTP_404_NOT_FOUND, ) + assert_owner(ds, current_user) obj_in = cmd.model_dump(exclude_unset=True) if "access_key_id" in obj_in and obj_in["access_key_id"] is not None: obj_in["access_key_id"] = encrypt_value(obj_in["access_key_id"]) @@ -147,6 +150,7 @@ async def delete(db: Session, ds_id: int, current_user: User) -> None: code=ErrorCode.CODE_61000_NO_DATA, status_code=status.HTTP_404_NOT_FOUND, ) + assert_owner(ds, current_user) with begin_transaction(db): crud_datasource.soft_delete(db=db, db_obj=ds) @@ -156,6 +160,7 @@ async def delete(db: Session, ds_id: int, current_user: User) -> None: async def list_objects( db: Session, ds_id: int, + current_user: User, prefix: Optional[str] = None, extension: Optional[str] = None, page_token: Optional[str] = None, @@ -167,6 +172,7 @@ async def list_objects( code=ErrorCode.CODE_61000_NO_DATA, status_code=status.HTTP_404_NOT_FOUND, ) + assert_owner(ds, current_user) client = _build_s3_client(ds) full_prefix = prefix if prefix is not None else (ds.prefix or "") diff --git a/labelu/internal/application/service/pre_annotation.py b/labelu/internal/application/service/pre_annotation.py index 297cfae2..71f0dd88 100644 --- a/labelu/internal/application/service/pre_annotation.py +++ b/labelu/internal/application/service/pre_annotation.py @@ -16,6 +16,7 @@ build_thumbnail_key, get_storage_backend, ) +from labelu.internal.application.service.access import assert_task_access from labelu.internal.adapter.persistence import crud_task from labelu.internal.adapter.persistence import crud_pre_annotation from labelu.internal.adapter.persistence import crud_attachment @@ -63,7 +64,9 @@ async def create( code=ErrorCode.CODE_50002_TASK_NOT_FOUND, status_code=status.HTTP_404_NOT_FOUND, ) - + + assert_task_access(task, current_user) + pre_annotations = [] for pre_annotation in cmd: pre_annotation_file = crud_attachment.get(db, pre_annotation.file_id) @@ -108,7 +111,12 @@ async def list_by( sorting: Optional[str], current_user: User, ) -> Tuple[List[PreAnnotationResponse], int]: - + + if task_id is not None: + task = crud_task.get(db=db, task_id=task_id) + if task is not None: + assert_task_access(task, current_user) + pre_annotations, total = crud_pre_annotation.list_by( db=db, task_id=task_id, @@ -198,13 +206,17 @@ async def get( pre_annotation_id=pre_annotation_id, ) - if not pre_annotation: + if not pre_annotation or pre_annotation.task_id != task_id: logger.error("cannot find pre_annotation: {}", pre_annotation_id) raise LabelUException( code=ErrorCode.CODE_55001_SAMPLE_NOT_FOUND, status_code=status.HTTP_404_NOT_FOUND, ) - + + task = crud_task.get(db=db, task_id=pre_annotation.task_id) + if task is not None: + assert_task_access(task, current_user) + # response return PreAnnotationResponse( id=pre_annotation.id, @@ -233,7 +245,9 @@ async def delete_pre_annotation_file( code=ErrorCode.CODE_50002_TASK_NOT_FOUND, status_code=status.HTTP_404_NOT_FOUND, ) - + + assert_task_access(task, current_user) + attachments = crud_attachment.get_by_ids( db=db, attachment_ids=[file_id] ) @@ -264,6 +278,16 @@ async def delete( ) -> CommonDataResp: with begin_transaction(db): + # authorize: caller must have access to every pre-annotation's task + pre_annotations = [ + crud_pre_annotation.get(db=db, pre_annotation_id=pid) + for pid in pre_annotation_ids + ] + for task_id in {pa.task_id for pa in pre_annotations if pa}: + task = crud_task.get(db=db, task_id=task_id) + if task is not None: + assert_task_access(task, current_user) + crud_pre_annotation.delete(db=db, pre_annotation_ids=pre_annotation_ids) # response return CommonDataResp(ok=True) diff --git a/labelu/internal/application/service/sample.py b/labelu/internal/application/service/sample.py index 4e7bac68..a4d03a77 100644 --- a/labelu/internal/application/service/sample.py +++ b/labelu/internal/application/service/sample.py @@ -19,6 +19,7 @@ build_thumbnail_key, get_storage_backend, ) +from labelu.internal.application.service.access import assert_task_access, assert_owner from labelu.internal.adapter.persistence import crud_attachment, crud_pre_annotation, crud_task from labelu.internal.adapter.persistence import crud_sample from labelu.internal.adapter.persistence import crud_export_job @@ -71,6 +72,8 @@ async def create( status_code=status.HTTP_404_NOT_FOUND, ) + assert_task_access(task, current_user) + samples = [ TaskSample( inner_id=task.last_sample_inner_id + i + 1, @@ -167,12 +170,15 @@ async def import_from_s3( status_code=status.HTTP_404_NOT_FOUND, ) + assert_task_access(task, current_user) + ds = crud_datasource.get(db=db, ds_id=cmd.data_source_id) if not ds: raise LabelUException( code=ErrorCode.CODE_61000_NO_DATA, status_code=status.HTTP_404_NOT_FOUND, ) + assert_owner(ds, current_user) # Resolve object keys: either from explicit list or by listing S3 prefix object_keys = cmd.object_keys @@ -229,7 +235,12 @@ async def list_by( page: Union[int, None], size: int, sorting: Union[str, None], + current_user: User, ) -> Tuple[List[SampleResponse], int]: + if task_id is not None: + task = crud_task.get(db=db, task_id=task_id) + if task is not None: + assert_task_access(task, current_user) samples = crud_sample.list_by( db=db, task_id=task_id, @@ -268,20 +279,24 @@ async def list_by( async def get( - db: Session, task_id: int, sample_id: int + db: Session, task_id: int, sample_id: int, current_user: User ) -> SampleResponse: sample = crud_sample.get( db=db, sample_id=sample_id, ) - if not sample: + if not sample or sample.task_id != task_id: logger.error("cannot find sample:{}", sample_id) raise LabelUException( code=ErrorCode.CODE_55001_SAMPLE_NOT_FOUND, status_code=status.HTTP_404_NOT_FOUND, ) + task = crud_task.get(db=db, task_id=sample.task_id) + if task is not None: + assert_task_access(task, current_user) + # response return SampleResponse( id=sample.id, @@ -321,9 +336,11 @@ async def patch( status_code=status.HTTP_404_NOT_FOUND, ) + assert_task_access(task, current_user) + # get sample sample = crud_sample.get(db=db, sample_id=sample_id) - if not sample: + if not sample or sample.task_id != task_id: logger.error("cannot find sample:{}", sample_id) raise LabelUException( code=ErrorCode.CODE_55001_SAMPLE_NOT_FOUND, @@ -407,6 +424,11 @@ async def delete( with begin_transaction(db): # delete media samples = crud_sample.get_by_ids(db=db, sample_ids=sample_ids) + # authorize: caller must have access to every sample's task + for task_id in {sample.task_id for sample in samples}: + task = crud_task.get(db=db, task_id=task_id) + if task is not None: + assert_task_access(task, current_user) attachment_ids = [sample.file_id for sample in samples if sample.file_id] attachments = crud_attachment.get_by_ids(db=db, attachment_ids=attachment_ids) @@ -440,6 +462,8 @@ async def create_export_job( status_code=status.HTTP_404_NOT_FOUND, ) + assert_task_access(task, current_user) + with begin_transaction(db): job = crud_export_job.create( db=db, diff --git a/labelu/internal/application/service/task.py b/labelu/internal/application/service/task.py index 6a483e8a..eee0f9c1 100644 --- a/labelu/internal/application/service/task.py +++ b/labelu/internal/application/service/task.py @@ -14,6 +14,7 @@ from labelu.internal.domain.models.task import Task from labelu.internal.domain.models.task import TaskStatus from labelu.internal.domain.models.sample import SampleState +from labelu.internal.application.service.access import assert_task_access from labelu.internal.adapter.persistence import crud_task, crud_user from labelu.internal.adapter.persistence import crud_sample from labelu.internal.application.command.task import BasicConfigCommand @@ -312,7 +313,7 @@ async def batch_remove_collaborators(db: Session, task_id: int, user_ids: List[i return CommonDataResp(ok=True) -async def update(db: Session, task_id: int, cmd: UpdateCommand) -> TaskResponse: +async def update(db: Session, task_id: int, cmd: UpdateCommand, current_user: User) -> TaskResponse: # get task task = crud_task.get(db=db, task_id=task_id) @@ -323,6 +324,8 @@ async def update(db: Session, task_id: int, cmd: UpdateCommand) -> TaskResponse: status_code=status.HTTP_404_NOT_FOUND, ) + assert_task_access(task, current_user) + # update obj_in = cmd.dict(exclude_unset=True) if cmd.config and cmd.media_type: diff --git a/labelu/internal/common/config.py b/labelu/internal/common/config.py index 553b5d5a..83094783 100644 --- a/labelu/internal/common/config.py +++ b/labelu/internal/common/config.py @@ -39,6 +39,14 @@ class Settings(BaseSettings): S3_PATH_STYLE: bool = False S3_USE_SSL: bool = True + # Allow user-configured data-source S3 endpoints to point at private + # networks (RFC1918 / ULA / loopback). Enabled by default for on-prem + # deployments that use an internal MinIO/S3-compatible store. Even when + # enabled, link-local (cloud metadata 169.254.0.0/16), multicast, + # reserved and unspecified addresses are always rejected. Set to false to + # restrict data-source endpoints to public hosts only (strict SSRF mode). + ALLOW_PRIVATE_S3_ENDPOINT: bool = True + AI_AUTO_LABEL_ENABLED: bool = False AI_PROVIDER: str = "local_http" AI_MODEL_ENDPOINT: str = "" diff --git a/labelu/internal/common/storage.py b/labelu/internal/common/storage.py index 4f1f0370..c6a25e04 100644 --- a/labelu/internal/common/storage.py +++ b/labelu/internal/common/storage.py @@ -6,9 +6,12 @@ from pathlib import Path from typing import Optional +from fastapi import status +from loguru import logger from PIL import Image from labelu.internal.common.config import settings +from labelu.internal.common.error_code import ErrorCode, LabelUException class StorageBackend(ABC): @@ -56,7 +59,16 @@ def backend_name(self) -> str: return "local" def _resolve(self, key: str) -> Path: - return settings.MEDIA_ROOT.joinpath(key.lstrip("/")) + root = settings.MEDIA_ROOT.resolve() + target = root.joinpath(key.lstrip("/")).resolve() + # Reject any key that escapes the media root via "../" traversal. + if target != root and root not in target.parents: + logger.error("rejected path traversal outside media root: {}", key) + raise LabelUException( + code=ErrorCode.CODE_51001_TASK_ATTACHMENT_NOT_FOUND, + status_code=status.HTTP_404_NOT_FOUND, + ) + return target def save_file(self, local_path: Path, key: str, content_type: Optional[str] = None) -> None: target_path = self._resolve(key) diff --git a/labelu/tests/conftest.py b/labelu/tests/conftest.py index e51ddb22..3c798709 100644 --- a/labelu/tests/conftest.py +++ b/labelu/tests/conftest.py @@ -1,13 +1,40 @@ -import pytest +import os +import shutil +import tempfile +from pathlib import Path from typing import Dict, Generator +import pytest from sqlalchemy import create_engine from sqlalchemy.orm import Session from sqlalchemy.orm import sessionmaker from fastapi.testclient import TestClient -from labelu.main import app +# Make the test suite hermetic: every run gets its own throwaway directory that +# holds BOTH the sqlite test DB and all uploaded media. Without this, tests +# wrote uploads into the developer's real appdirs data directory +# (~/Library/Application Support/labelu/media) and kept ``test.db`` in the cwd, +# so ``sqlite_sequence`` accumulated across runs and freshly minted task_ids +# collided with leftover ``upload//...`` files, intermittently failing +# uploads with a 400 "file already exists". The directory must be chosen at +# import time because the engine below is created at module import, before any +# pytest ``tmp_path``/``monkeypatch`` fixture could run. It is removed at the end +# of the session by the ``_hermetic_storage`` fixture. +_TEST_DATA_DIR = Path(tempfile.mkdtemp(prefix="labelu-test-")) + from labelu.internal.common.config import settings + +# Redirect media storage into the isolated directory and drop the memoized +# storage backend so it resolves paths against the new MEDIA_ROOT. +settings.STORAGE_BACKEND = "local" +settings.MEDIA_ROOT = _TEST_DATA_DIR / "media" +os.makedirs(settings.MEDIA_ROOT, exist_ok=True) + +from labelu.internal.common.storage import get_storage_backend + +get_storage_backend.cache_clear() + +from labelu.main import app from labelu.internal.common.db import Base from labelu.internal.common.db import begin_transaction from labelu.internal.common.db import get_db @@ -19,7 +46,7 @@ TEST_USER_PASSWORD = "test@123" -SQLALCHEMY_DATABASE_URL = "sqlite:///./test.db" +SQLALCHEMY_DATABASE_URL = f"sqlite:///{_TEST_DATA_DIR / 'test.db'}" engine = create_engine( SQLALCHEMY_DATABASE_URL, connect_args={"check_same_thread": False, "timeout": 30}, @@ -28,6 +55,23 @@ Base.metadata.create_all(bind=engine) +@pytest.fixture(autouse=True, scope="session") +def _hermetic_storage() -> Generator: + """Guarantee the isolated media root is active and clean it up afterwards. + + The redirect itself happens at import time (above); this fixture re-clears + the storage-backend cache in case anything memoized it during collection and + removes the throwaway directory (test DB + media) once the session ends. + """ + get_storage_backend.cache_clear() + try: + yield + finally: + engine.dispose() + get_storage_backend.cache_clear() + shutil.rmtree(_TEST_DATA_DIR, ignore_errors=True) + + def override_get_db(): db = None try: @@ -46,12 +90,25 @@ def db() -> Generator: db = None try: db = TestingSessionLocal() + # Seed the test user at session scope so it exists before the + # module-scoped ``testuser_token_headers`` fixture logs in. The + # per-test ``run_around_tests`` cleanup preserves the ``user`` table, so + # this single seed lasts the whole session. (Previously the user + # survived only because ``test.db`` persisted in the cwd across runs.) + init_db() yield db finally: if db is not None: db.close() +@pytest.fixture(scope="session", autouse=True) +def ensure_test_user() -> Generator: + # Initialize test user once per test session + init_db() + yield + + @pytest.fixture(autouse=True) def run_around_tests(db: Session): init_db() @@ -89,7 +146,16 @@ def get_testuser_token_headers(client: TestClient) -> Dict[str, str]: "password": TEST_USER_PASSWORD, } r = client.post(f"{settings.API_V1_STR}/users/login", json=data) - token = r.json()["data"]["token"] + response_json = r.json() + + # Add debugging info for CI failures + if "data" not in response_json: + print(f"Login failed. Response: {response_json}") + print(f"Status code: {r.status_code}") + print(f"Password length (bytes): {len(TEST_USER_PASSWORD.encode('utf-8'))}") + raise ValueError(f"Login failed with response: {response_json}") + + token = response_json["data"]["token"] headers = {"Authorization": f"{token}"} return headers diff --git a/labelu/tests/internal/adapter/persistence/test_crud_sample.py b/labelu/tests/internal/adapter/persistence/test_crud_sample.py new file mode 100644 index 00000000..77d13830 --- /dev/null +++ b/labelu/tests/internal/adapter/persistence/test_crud_sample.py @@ -0,0 +1,85 @@ +from sqlalchemy.orm import Session + +from labelu.internal.common.db import begin_transaction +from labelu.internal.domain.models.task import Task +from labelu.internal.domain.models.sample import TaskSample +from labelu.internal.adapter.persistence import crud_task +from labelu.internal.adapter.persistence import crud_sample + + +def _make_task_with_samples(db: Session, n: int = 3): + with begin_transaction(db): + task = crud_task.create( + db=db, + task=Task(name="n", description="d", tips="t", created_by=1, updated_by=1), + ) + samples = [ + TaskSample( + task_id=task.id, + file_id=1, + created_by=1, + updated_by=1, + data="{}", + annotated_count=i, + ) + for i in range(n) + ] + with begin_transaction(db): + crud_sample.batch(db=db, samples=samples) + return task + + +class TestCrudSampleSortInjection: + """Regression tests for SQL injection via the sample-listing sort param.""" + + def test_error_based_injection_is_ignored(self, db: Session): + task = _make_task_with_samples(db) + # a non-existent column injected after a valid pair must NOT reach SQL + sorting = "updated_at:asc,SQLI_MARKER_no_such_col:asc" + results = crud_sample.list_by( + db=db, task_id=task.id, after=None, before=None, + page=0, size=10, sorting=sorting, + ) + assert len(results) == 3 + + def test_boolean_based_injection_has_no_effect(self, db: Session): + task = _make_task_with_samples(db) + payload_true = ( + "state:asc,(CASE WHEN 1=1 THEN task_sample.id " + "ELSE 0-task_sample.id END):desc" + ) + payload_false = ( + "state:asc,(CASE WHEN 1=0 THEN task_sample.id " + "ELSE 0-task_sample.id END):desc" + ) + ids_true = [ + s.id for s in crud_sample.list_by( + db=db, task_id=task.id, after=None, before=None, + page=0, size=10, sorting=payload_true, + ) + ] + ids_false = [ + s.id for s in crud_sample.list_by( + db=db, task_id=task.id, after=None, before=None, + page=0, size=10, sorting=payload_false, + ) + ] + # if the injected CASE executed, the two orderings would differ + assert ids_true == ids_false + + def test_valid_sort_still_works(self, db: Session): + task = _make_task_with_samples(db) + results = crud_sample.list_by( + db=db, task_id=task.id, after=None, before=None, + page=0, size=10, sorting="annotated_count:desc", + ) + counts = [s.annotated_count for s in results] + assert counts == sorted(counts, reverse=True) + + def test_state_sort_still_works(self, db: Session): + task = _make_task_with_samples(db) + results = crud_sample.list_by( + db=db, task_id=task.id, after=None, before=None, + page=0, size=10, sorting="state:asc", + ) + assert len(results) == 3 diff --git a/labelu/tests/internal/adapter/routers/test_attachment.py b/labelu/tests/internal/adapter/routers/test_attachment.py index 1e49d1df..b5b2e454 100644 --- a/labelu/tests/internal/adapter/routers/test_attachment.py +++ b/labelu/tests/internal/adapter/routers/test_attachment.py @@ -406,6 +406,48 @@ def test_partial_content_file_not_found( assert r.status_code == 404 assert r.json()["err_code"] == 30003 + def test_upload_rejects_path_traversal_filename( + self, client: TestClient, testuser_token_headers: dict, db: Session + ) -> None: + # prepare data + data = { + "name": "traversal test", + "description": "test", + "tips": "test", + } + task = client.post( + f"{settings.API_V1_STR}/tasks", headers=testuser_token_headers, json=data + ) + task_id = task.json()["data"]["id"] + + # clean the location where a sanitized upload would actually land + empty_task_upload(task_id, "tmp/pwned_traversal_marker.txt") + + marker = "pwned_traversal_marker.txt" + escape_target = Path("/tmp").joinpath(marker) + if escape_target.exists(): + escape_target.unlink() + + # craft a filename whose directory part tries to escape the upload dir + malicious_name = "../../../../../../../../tmp/" + marker + with Path("labelu/tests/data/test.txt").open(mode="rb") as f: + res = client.post( + f"{settings.API_V1_STR}/tasks/{task_id}/attachments", + headers=testuser_token_headers, + files={"file": (malicious_name, f, "text/plain")}, + ) + + # the crafted file must NOT be written outside the media root + assert not escape_target.exists() + # the upload is handled safely and stored inside the task upload dir + assert res.status_code == 201 + assert f"upload/{task_id}/" in res.json()["data"]["url"] + # the stored key stays within the media root + parts = res.json()["data"]["url"].split("/attachment/")[-1] + assert Path(settings.MEDIA_ROOT).joinpath(parts).resolve().is_relative_to( + Path(settings.MEDIA_ROOT).resolve() + ) + def test_upload_file_with_hash_symbol( self, client: TestClient, testuser_token_headers: dict, db: Session ) -> None: diff --git a/labelu/tests/internal/adapter/routers/test_datasource.py b/labelu/tests/internal/adapter/routers/test_datasource.py new file mode 100644 index 00000000..cc83264d --- /dev/null +++ b/labelu/tests/internal/adapter/routers/test_datasource.py @@ -0,0 +1,66 @@ +from sqlalchemy.orm import Session +from fastapi.testclient import TestClient + +from labelu.internal.common.config import settings +from labelu.internal.common.db import begin_transaction +from labelu.internal.domain.models.data_source import DataSource +from labelu.internal.adapter.persistence import crud_datasource + + +class TestDataSourceIDOR: + def _other_users_datasource(self, db: Session) -> DataSource: + with begin_transaction(db): + return crud_datasource.create( + db=db, + data_source=DataSource( + name="victim-ds", + type="S3", + endpoint="https://8.8.8.8", + bucket="victim-bucket", + created_by=999, + updated_by=999, + ), + ) + + def test_get_others_datasource_forbidden( + self, client: TestClient, testuser_token_headers: dict, db: Session + ) -> None: + ds = self._other_users_datasource(db) + r = client.get( + f"{settings.API_V1_STR}/datasources/{ds.id}", + headers=testuser_token_headers, + ) + assert r.status_code == 403 + + def test_update_others_datasource_forbidden( + self, client: TestClient, testuser_token_headers: dict, db: Session + ) -> None: + ds = self._other_users_datasource(db) + r = client.patch( + f"{settings.API_V1_STR}/datasources/{ds.id}", + headers=testuser_token_headers, + json={"name": "hijacked"}, + ) + assert r.status_code == 403 + + def test_delete_others_datasource_forbidden( + self, client: TestClient, testuser_token_headers: dict, db: Session + ) -> None: + ds = self._other_users_datasource(db) + r = client.request( + "delete", + f"{settings.API_V1_STR}/datasources/{ds.id}", + headers=testuser_token_headers, + ) + assert r.status_code == 403 + + def test_list_objects_others_datasource_forbidden( + self, client: TestClient, testuser_token_headers: dict, db: Session + ) -> None: + ds = self._other_users_datasource(db) + # must be denied before any S3 request is made with the victim's creds + r = client.get( + f"{settings.API_V1_STR}/datasources/{ds.id}/objects", + headers=testuser_token_headers, + ) + assert r.status_code == 403 diff --git a/labelu/tests/internal/adapter/routers/test_pre_annotation.py b/labelu/tests/internal/adapter/routers/test_pre_annotation.py index 42f9b134..8f620200 100644 --- a/labelu/tests/internal/adapter/routers/test_pre_annotation.py +++ b/labelu/tests/internal/adapter/routers/test_pre_annotation.py @@ -20,6 +20,9 @@ class TestClassTaskPreAnnotationRouter: def test_create_pre_annotation_successful( self, client: TestClient, testuser_token_headers: dict, db: Session ) -> None: + current_user = crud_user.get_user_by_username( + db=db, username="test@example.com" + ) with begin_transaction(db): task = crud_task.create( @@ -28,8 +31,8 @@ def test_create_pre_annotation_successful( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) @@ -55,9 +58,57 @@ def test_create_pre_annotation_successful( assert r.status_code == 201 assert len(json["data"]["ids"]) == 1 + def test_pre_annotation_access_forbidden_for_non_member( + self, client: TestClient, testuser_token_headers: dict, db: Session + ) -> None: + # task + pre-annotation owned by another user + with begin_transaction(db): + task = crud_task.create( + db=db, + task=Task( + name="other", description="d", tips="t", + created_by=999, updated_by=999, + ), + ) + with begin_transaction(db): + pre = crud_pre_annotation.batch( + db=db, + pre_annotations=[ + TaskPreAnnotation( + task_id=task.id, file_id=1, sample_name="x", + data="{}", created_by=999, updated_by=999, + ) + ], + ) + pid = pre[0].id + + # list + r = client.get( + f"{settings.API_V1_STR}/tasks/{task.id}/pre_annotations", + headers=testuser_token_headers, params={"page": 0, "size": 10}, + ) + assert r.status_code == 403 + # get + r = client.get( + f"{settings.API_V1_STR}/tasks/{task.id}/pre_annotations/{pid}", + headers=testuser_token_headers, + ) + assert r.status_code == 403 + # delete + r = client.request( + "delete", + f"{settings.API_V1_STR}/tasks/{task.id}/pre_annotations", + headers=testuser_token_headers, + json={"pre_annotation_ids": [pid]}, + ) + assert r.status_code == 403 + def test_create_pre_annotation_sample_exists( self, client: TestClient, testuser_token_headers: dict, db: Session ) -> None: + current_user = crud_user.get_user_by_username( + db=db, username="test@example.com" + ) with begin_transaction(db): task = crud_task.create( @@ -66,8 +117,8 @@ def test_create_pre_annotation_sample_exists( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) @@ -144,8 +195,8 @@ def test_pre_annotation_list_by_page( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) @@ -203,8 +254,8 @@ def test_pre_annotation_list_with_sample_name( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) @@ -283,8 +334,8 @@ def test_pre_annotation_list_with_sample_name_not_found( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) empty_task_upload(task.id, "test.png") @@ -376,8 +427,8 @@ def test_sample_list_by_sample_name_error( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) # run @@ -405,8 +456,8 @@ def test_pre_annotation_get( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) with begin_transaction(db): @@ -448,8 +499,8 @@ def test_pre_annotation_delete( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) with begin_transaction(db): @@ -480,6 +531,9 @@ def test_pre_annotation_delete( def test_pre_annotations_delete_not_found( self, client: TestClient, testuser_token_headers: dict, db: Session ) -> None: + current_user = crud_user.get_user_by_username( + db=db, username="test@example.com" + ) # prepare data with begin_transaction(db): @@ -489,8 +543,8 @@ def test_pre_annotations_delete_not_found( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) @@ -595,6 +649,9 @@ def test_pre_annotation_files_list( def test_pre_annotation_files_list_with_errors( self, client: TestClient, testuser_token_headers: dict, db: Session ) -> None: + current_user = crud_user.get_user_by_username( + db=db, username="test@example.com" + ) # 测试不存在的任务 r = client.get( f"{settings.API_V1_STR}/tasks/9999/pre_annotations/files", @@ -613,8 +670,8 @@ def test_pre_annotation_files_list_with_errors( name="invalid sort task", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) @@ -687,6 +744,9 @@ def test_delete_pre_annotation_file( def test_delete_pre_annotation_file_errors( self, client: TestClient, testuser_token_headers: dict, db: Session ) -> None: + current_user = crud_user.get_user_by_username( + db=db, username="test@example.com" + ) # 测试不存在的任务 r1 = client.delete( f"{settings.API_V1_STR}/tasks/9999/pre_annotations/files/1", @@ -704,8 +764,8 @@ def test_delete_pre_annotation_file_errors( name="not found file task", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) diff --git a/labelu/tests/internal/adapter/routers/test_sample.py b/labelu/tests/internal/adapter/routers/test_sample.py index 5e349d7f..a3417f32 100644 --- a/labelu/tests/internal/adapter/routers/test_sample.py +++ b/labelu/tests/internal/adapter/routers/test_sample.py @@ -17,6 +17,9 @@ def test_create_sample_successful( ) -> None: # prepare data + current_user = crud_user.get_user_by_username( + db=db, username="test@example.com" + ) with begin_transaction(db): task = crud_task.create( db=db, @@ -24,8 +27,8 @@ def test_create_sample_successful( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) data = [{"file_id": 1, "data": {}}] @@ -50,6 +53,9 @@ def test_create_sample_task_status_not_draft( ) -> None: # prepare data + current_user = crud_user.get_user_by_username( + db=db, username="test@example.com" + ) with begin_transaction(db): task = crud_task.create( db=db, @@ -57,8 +63,8 @@ def test_create_sample_task_status_not_draft( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, status="CONFIGURED", ), ) @@ -113,8 +119,8 @@ def test_sample_list_by_page( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) samples = [ @@ -162,8 +168,8 @@ def test_sample_list_by_before( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) samples = [ @@ -212,8 +218,8 @@ def test_sample_list_by_after( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) samples = [ @@ -262,8 +268,8 @@ def test_sample_list_with_sort( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) samples = [ @@ -333,8 +339,8 @@ def test_sample_list_by_sort_error( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) # run @@ -347,6 +353,58 @@ def test_sample_list_by_sort_error( # check assert r.status_code == 422 + def test_sample_access_forbidden_for_non_member( + self, client: TestClient, testuser_token_headers: dict, db: Session + ) -> None: + # a task owned by someone else; the test user is neither owner nor collaborator + with begin_transaction(db): + task = crud_task.create( + db=db, + task=Task( + name="other", description="d", tips="t", + created_by=999, updated_by=999, + ), + ) + with begin_transaction(db): + samples = crud_sample.batch( + db=db, + samples=[ + TaskSample( + task_id=task.id, file_id=1, + created_by=999, updated_by=999, data="{}", + ) + ], + ) + sid = samples[0].id + + # list + r = client.get( + f"{settings.API_V1_STR}/tasks/{task.id}/samples", + headers=testuser_token_headers, params={"page": 0, "size": 10}, + ) + assert r.status_code == 403 + # get + r = client.get( + f"{settings.API_V1_STR}/tasks/{task.id}/samples/{sid}", + headers=testuser_token_headers, + ) + assert r.status_code == 403 + # patch + r = client.patch( + f"{settings.API_V1_STR}/tasks/{task.id}/samples/{sid}", + headers=testuser_token_headers, + json={"data": {}, "annotated_count": 0}, + ) + assert r.status_code == 403 + # delete + r = client.request( + "delete", + f"{settings.API_V1_STR}/tasks/{task.id}/samples", + headers=testuser_token_headers, + json={"sample_ids": [sid]}, + ) + assert r.status_code == 403 + def test_sample_get( self, client: TestClient, testuser_token_headers: dict, db: Session ) -> None: @@ -362,8 +420,8 @@ def test_sample_get( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) with begin_transaction(db): @@ -407,8 +465,8 @@ def test_sample_get_not_found( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) @@ -438,8 +496,8 @@ def test_sample_patch( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) with begin_transaction(db): @@ -487,8 +545,8 @@ def test_sample_patch_skip( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) with begin_transaction(db): @@ -541,6 +599,9 @@ def test_sample_patch_task_sample_not_found( ) -> None: # prepare data + current_user = crud_user.get_user_by_username( + db=db, username="test@example.com" + ) with begin_transaction(db): task = crud_task.create( db=db, @@ -548,8 +609,8 @@ def test_sample_patch_task_sample_not_found( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) @@ -580,8 +641,8 @@ def test_sample_delete( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) with begin_transaction(db): @@ -614,6 +675,9 @@ def test_sample_delete_not_found( ) -> None: # prepare data + current_user = crud_user.get_user_by_username( + db=db, username="test@example.com" + ) with begin_transaction(db): task = crud_task.create( db=db, @@ -621,8 +685,8 @@ def test_sample_delete_not_found( name="name", description="description", tips="tips", - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) @@ -653,8 +717,8 @@ def test_export_sample( description="description", tips="tips", config='{"tools":[{"tool":"rectTool","config":{"isShowCursor":false,"showConfirm":false,"skipWhileNoDependencies":false,"drawOutsideTarget":false,"copyBackwardResult":false,"minWidth":1,"attributeConfigurable":true,"textConfigurable":true,"textCheckType":4,"customFormat":"","attributes":[{"key":"rectTool","value":"rectTool"}]}},{"tool":"pointTool","config":{"upperLimit":10,"isShowCursor":false,"attributeConfigurable":true,"copyBackwardResult":false,"textConfigurable":true,"textCheckType":0,"customFormat":"","attributes":[{"key":"pointTool","value":"pointTool"}]}},{"tool":"polygonTool","config":{"isShowCursor":false,"lineType":0,"lineColor":0,"drawOutsideTarget":false,"edgeAdsorption":true,"copyBackwardResult":false,"attributeConfigurable":true,"textConfigurable":true,"textCheckType":0,"customFormat":"","attributes":[{"key":"polygonTool","value":"polygonTool"}],"lowerLimitPointNum":"4","upperLimitPointNum":100}},{"tool":"lineTool","config":{"isShowCursor":false,"lineType":0,"lineColor":1,"edgeAdsorption":true,"outOfTarget":true,"copyBackwardResult":false,"attributeConfigurable":true,"textConfigurable":true,"textCheckType":4,"customFormat":"^[\s\S]{1,3}$","lowerLimitPointNum":4,"upperLimitPointNum":"","attributes":[{"key":"lineTool","value":"lineTool"}]}},{"tool":"tagTool"},{"tool":"textTool"}],"tagList":[{"key":"类别1","value":"class1","isMulti":true,"subSelected":[{"key":"选项1","value":"option1","isDefault":true},{"key":"选项2","value":"option2","isDefault":false}]},{"key":"类别2","value":"class2","isMulti":true,"subSelected":[{"key":"a选项1","value":"aoption1","isDefault":true},{"key":"a选项2","value":"aoption2","isDefault":false}]}],"attributes":[{"key":"RT","value":"RT"}],"textConfig":[{"label":"我的描述","key":"描述的关键","required":true,"default":"","maxLength":200},{"label":"我的描述1","key":"描述的关键1","required":true,"default":"","maxLength":200}],"fileInfo":{"type":"img","list":[{"id":1,"url":"/src/img/example/bear6.webp","result":"[]"}]},"commonAttributeConfigurable":true}', - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) samples = crud_sample.batch( @@ -704,8 +768,8 @@ def test_export_includes_all_states( description="description", tips="tips", config='{"tools":[{"tool":"rectTool","config":{"attributes":[{"key":"RT","value":"RT"}]}}],"tagList":[],"attributes":[],"textConfig":[],"fileInfo":{"type":"img","list":[]},"commonAttributeConfigurable":true}', - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) result_data = '{"result": "{\"width\":100,\"height\":100,\"valid\":true,\"rotate\":0,\"rectTool\":{\"toolName\":\"rectTool\",\"result\":[{\"x\":10,\"y\":10,\"width\":50,\"height\":50,\"label\":\"RT\",\"valid\":true,\"isVisible\":true,\"id\":\"test1\",\"order\":1}]}}"}' @@ -771,8 +835,8 @@ def test_export_status_reports_skipped_count( description="description", tips="tips", config='{"tools":[{"tool":"rectTool","config":{"attributes":[{"key":"RT","value":"RT"}]}}],"tagList":[],"attributes":[],"textConfig":[],"fileInfo":{"type":"img","list":[]},"commonAttributeConfigurable":true}', - created_by=0, - updated_by=0, + created_by=current_user.id, + updated_by=current_user.id, ), ) job = crud_export_job.create( diff --git a/labelu/tests/internal/adapter/routers/test_task.py b/labelu/tests/internal/adapter/routers/test_task.py index 4688aa4c..a8e96da6 100644 --- a/labelu/tests/internal/adapter/routers/test_task.py +++ b/labelu/tests/internal/adapter/routers/test_task.py @@ -176,6 +176,27 @@ def test_task_update( assert json["data"]["config"] == "new config" assert json["data"]["media_type"] == "IMAGE" + def test_task_update_forbidden_for_non_owner( + self, client: TestClient, testuser_token_headers: dict, db: Session + ) -> None: + # task owned by another user + with begin_transaction(db): + task = crud_task.create( + db=db, + task=Task( + name="other", description="d", tips="t", + created_by=999, updated_by=999, + ), + ) + + r = client.patch( + f"{settings.API_V1_STR}/tasks/{task.id}", + headers=testuser_token_headers, + json={"name": "hijacked", "description": "x", "tips": "y", + "config": "c", "media_type": "IMAGE"}, + ) + assert r.status_code == 403 + def test_task_update_no_found_task( self, client: TestClient, testuser_token_headers: dict, db: Session ) -> None: diff --git a/labelu/tests/internal/application/__init__.py b/labelu/tests/internal/application/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/labelu/tests/internal/application/command/__init__.py b/labelu/tests/internal/application/command/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/labelu/tests/internal/application/command/test_datasource.py b/labelu/tests/internal/application/command/test_datasource.py new file mode 100644 index 00000000..a2d0fdf7 --- /dev/null +++ b/labelu/tests/internal/application/command/test_datasource.py @@ -0,0 +1,93 @@ +import pytest +from pydantic import ValidationError + +from labelu.internal.common.config import settings +from labelu.internal.application.command.datasource import ( + CreateDataSourceCommand, + UpdateDataSourceCommand, +) + + +def _create(endpoint): + return CreateDataSourceCommand( + name="ds", + bucket="b", + access_key_id="ak", + secret_access_key="sk", + endpoint=endpoint, + ) + + +class TestDataSourceEndpointSSRF: + """Regression tests for the data-source S3 endpoint SSRF guard.""" + + @pytest.mark.parametrize( + "endpoint", + [ + "http://169.254.169.254", # cloud metadata (IMDS) - always blocked + "http://169.254.169.254/latest/", # IMDS with path + "http://0.0.0.0", # unspecified - always blocked + "file:///etc/passwd", # non-http scheme + "ftp://example.com", # non-http scheme + "s3.example.com", # missing scheme + ], + ) + def test_always_rejects_metadata_and_bad_scheme(self, endpoint): + with pytest.raises(ValidationError): + _create(endpoint) + with pytest.raises(ValidationError): + UpdateDataSourceCommand(endpoint=endpoint) + + @pytest.mark.parametrize( + "endpoint", + [ + None, # optional -> AWS default + "https://8.8.8.8:9000", # public IP literal + "http://1.1.1.1", # public IP literal + ], + ) + def test_allows_public_endpoint(self, endpoint): + cmd = _create(endpoint) + assert cmd.endpoint == endpoint + + @pytest.mark.parametrize( + "endpoint", + [ + "http://10.1.2.3:9000", # private RFC1918 + "http://192.168.0.10:9000", # private RFC1918 + "http://172.16.5.5:9000", # private RFC1918 + "http://127.0.0.1:9000", # loopback (same-host MinIO) + "http://[::1]:9000", # IPv6 loopback + "http://localhost:9000", # loopback by name (same-host MinIO) + ], + ) + def test_allows_private_endpoint_by_default(self, endpoint): + # ALLOW_PRIVATE_S3_ENDPOINT defaults to True for on-prem deployments + assert settings.ALLOW_PRIVATE_S3_ENDPOINT is True + cmd = _create(endpoint) + assert cmd.endpoint == endpoint + + @pytest.mark.parametrize( + "endpoint", + [ + "http://10.1.2.3:9000", + "http://192.168.0.10:9000", + "http://127.0.0.1:9000", + "http://localhost:9000", + ], + ) + def test_strict_mode_rejects_private(self, endpoint, monkeypatch): + monkeypatch.setattr(settings, "ALLOW_PRIVATE_S3_ENDPOINT", False) + with pytest.raises(ValidationError): + _create(endpoint) + + def test_strict_mode_still_allows_public(self, monkeypatch): + monkeypatch.setattr(settings, "ALLOW_PRIVATE_S3_ENDPOINT", False) + cmd = _create("https://8.8.8.8:9000") + assert cmd.endpoint == "https://8.8.8.8:9000" + + def test_metadata_blocked_even_when_private_allowed(self): + # the flag must not re-open the cloud metadata endpoint + assert settings.ALLOW_PRIVATE_S3_ENDPOINT is True + with pytest.raises(ValidationError): + _create("http://169.254.169.254") diff --git a/labelu/tests/internal/common/test_storage.py b/labelu/tests/internal/common/test_storage.py new file mode 100644 index 00000000..a8cf653c --- /dev/null +++ b/labelu/tests/internal/common/test_storage.py @@ -0,0 +1,32 @@ +import pytest + +from labelu.internal.common.config import settings +from labelu.internal.common.storage import LocalStorageBackend +from labelu.internal.common.error_code import LabelUException + + +class TestLocalStorageResolveTraversal: + """Regression tests for path traversal in LocalStorageBackend._resolve.""" + + def test_resolve_rejects_parent_traversal(self): + backend = LocalStorageBackend() + with pytest.raises(LabelUException): + backend._resolve("../../../../../../../../etc/passwd") + + def test_resolve_rejects_nested_traversal(self): + backend = LocalStorageBackend() + with pytest.raises(LabelUException): + backend._resolve("upload/1/../../../../../../../../etc/passwd") + + def test_resolve_rejects_absolute_escape(self): + # lstrip("/") turns "/etc/passwd" into "etc/passwd" (stays inside root), + # but a backslash-free absolute traversal must still be contained. + backend = LocalStorageBackend() + resolved = backend._resolve("/etc/passwd") + assert str(resolved).startswith(str(settings.MEDIA_ROOT.resolve())) + + def test_resolve_allows_normal_key(self): + backend = LocalStorageBackend() + resolved = backend._resolve("upload/1/test.png") + assert str(resolved).startswith(str(settings.MEDIA_ROOT.resolve())) + assert resolved.name == "test.png" diff --git a/labelu/version.py b/labelu/version.py index ca4a2c90..43fc3eb6 100644 --- a/labelu/version.py +++ b/labelu/version.py @@ -1 +1 @@ -version='1.5.2' \ No newline at end of file +version='1.5.2'