diff --git a/.env-template b/.env-template index afff3835..81ba3d42 100644 --- a/.env-template +++ b/.env-template @@ -40,7 +40,8 @@ MICROSOFT_AUTHORITY=https://{tenantId}.ciamlogin.com/{tenantId} # Authentication (optional - default is no auth; see docs: Deploying -> App Configuration) # AUTH_TYPE=None|simple_jwt|session_jwt|oidc -# JWT_SECRET_KEY= +# JWT_SECRET_KEY= +# Local development auto-generates .jwt_secret_key when this is unset. # OIDC SSO (only when AUTH_TYPE=oidc; works with Authentik, Keycloak, Okta, ...) # OIDC_ISSUER= diff --git a/application/api/user/agents/routes.py b/application/api/user/agents/routes.py index 62cc6ee7..938ea855 100644 --- a/application/api/user/agents/routes.py +++ b/application/api/user/agents/routes.py @@ -244,7 +244,11 @@ def _format_agent_output( "slug": agent.get("slug", "") or "", "description": agent.get("description", "") or "", "image": ( - generate_image_url(agent["image"]) if agent.get("image") else "" + generate_image_url( + agent["image"], agent["id"], agent.get("user_id") + ) + if agent.get("image") + else "" ), "source": source_value, "sources": sources_list, @@ -308,7 +312,7 @@ def _build_create_kwargs(data: dict, *, image_url: str, agent_type: str) -> dict allowed_fields = set(schema["fields"]) for key in ( - "description", "agent_type", "key", "image", "retriever", + "description", "agent_type", "key", "retriever", "default_model_id", ): if key in allowed_fields and data.get(key) not in (None, ""): @@ -787,8 +791,8 @@ class UpdateAgent(Resource): "description": fields.String( required=True, description="New description of the agent" ), - "image": fields.String( - required=False, description="New image URL or identifier" + "image": fields.Raw( + required=False, description="Image file upload", type="file" ), "source": fields.String( required=False, description="Source ID (legacy single source)" @@ -913,8 +917,12 @@ class UpdateAgent(Resource): 404, ) pg_agent_id = str(existing_agent["id"]) + existing_image = existing_agent.get("image", "") or "" image_url, image_error = handle_image_upload( - request, existing_agent.get("image", "") or "", user, storage, + request, + existing_image, + existing_agent.get("user_id") or user, + storage, ) if image_error: return image_error @@ -923,7 +931,6 @@ class UpdateAgent(Resource): allowed_fields = [ "name", "description", - "image", "source", "sources", "chunks", @@ -1133,7 +1140,7 @@ class UpdateAgent(Resource): f"Field '{field}' cannot be empty", user, field ) update_fields[field] = value - if image_url: + if image_url and image_url != existing_image: update_fields["image"] = image_url if not update_fields: return _reject("No valid update data provided", user) @@ -1543,7 +1550,11 @@ class PinnedAgents(Resource): "name": agent.get("name", ""), "description": agent.get("description", ""), "image": ( - generate_image_url(agent["image"]) if agent.get("image") else "" + generate_image_url( + agent["image"], agent["id"], agent.get("user_id") + ) + if agent.get("image") + else "" ), "source": str(source_id) if source_id else "", "chunks": str(agent["chunks"]) if agent.get("chunks") is not None else "", @@ -1591,7 +1602,13 @@ class GetTemplateAgents(Resource): "id": str(agent["id"]), "name": agent.get("name"), "description": agent.get("description") or "", - "image": agent.get("image") or "", + "image": ( + generate_image_url( + agent["image"], agent["id"], agent.get("user_id") + ) + if agent.get("image") + else "" + ), } for agent in template_rows ] diff --git a/application/api/user/agents/sharing.py b/application/api/user/agents/sharing.py index 2d1758b4..df797733 100644 --- a/application/api/user/agents/sharing.py +++ b/application/api/user/agents/sharing.py @@ -29,7 +29,11 @@ def _serialize_agent_basic(agent: dict) -> dict: "user": agent.get("user_id", ""), "name": agent.get("name", ""), "image": ( - generate_image_url(agent["image"]) if agent.get("image") else "" + generate_image_url( + agent["image"], agent["id"], agent.get("user_id") + ) + if agent.get("image") + else "" ), "description": agent.get("description", ""), "source": str(source_id) if source_id else "", @@ -158,7 +162,11 @@ class SharedAgents(Resource): "name": agent.get("name", ""), "description": agent.get("description", ""), "image": ( - generate_image_url(agent["image"]) if agent.get("image") else "" + generate_image_url( + agent["image"], agent["id"], agent.get("user_id") + ) + if agent.get("image") + else "" ), "tools": agent.get("tools", []) or [], "tool_details": resolve_tool_details( diff --git a/application/api/user/attachments/routes.py b/application/api/user/attachments/routes.py index 32e876e7..d384cb9f 100644 --- a/application/api/user/attachments/routes.py +++ b/application/api/user/attachments/routes.py @@ -6,12 +6,13 @@ from pathlib import Path import uuid -from flask import current_app, jsonify, make_response, request +from flask import Response, current_app, jsonify, make_response, redirect, request from flask_restx import fields, Namespace, Resource from application.api import api from application.cache import get_redis_instance from application.core.settings import settings +from application.storage.db.base_repository import looks_like_uuid from application.storage.db.repositories.agents import AgentsRepository from application.storage.db.session import db_readonly from application.stt.constants import ( @@ -35,13 +36,37 @@ from application.stt.live_session import ( ) from application.stt.stt_creator import STTCreator from application.tts.tts_creator import TTSCreator -from application.utils import safe_filename +from application.utils import ( + get_agent_image_content_type, + is_external_image_url, + is_safe_agent_image_path, + safe_filename, + verify_agent_image_capability, +) attachments_ns = Namespace( "attachments", description="File attachments and media operations", path="/api" ) +_AGENT_IMAGE_CHUNK_BYTES = 64 * 1024 +_AGENT_IMAGE_PRESIGNED_TTL_SECONDS = 300 +_AGENT_IMAGE_REDIRECT_CACHE_SECONDS = _AGENT_IMAGE_PRESIGNED_TTL_SECONDS - 60 + + +def _stream_file(file_obj, size_bytes: int): + """Yield exactly the size-checked file range and always close the handle.""" + remaining = size_bytes + try: + while remaining > 0: + chunk = file_obj.read(min(_AGENT_IMAGE_CHUNK_BYTES, remaining)) + if not chunk: + break + remaining -= len(chunk) + yield chunk + finally: + file_obj.close() + def _resolve_authenticated_user(): decoded_token = getattr(request, "decoded_token", None) @@ -630,25 +655,71 @@ class LiveSpeechToTextFinish(Resource): ) -@attachments_ns.route("/images/") +@attachments_ns.route("/images//") class ServeImage(Resource): - @api.doc(description="Serve an image from storage") - def get(self, image_path): - if ".." in image_path or image_path.startswith("/") or "\x00" in image_path: - return make_response( - jsonify({"success": False, "message": "Invalid image path"}), 400 - ) + @api.doc(description="Serve an agent image using an opaque capability URL") + def get(self, agent_id, capability): try: + if not looks_like_uuid(agent_id): + return make_response( + jsonify({"success": False, "message": "Image not found"}), 404 + ) + with db_readonly() as conn: + agent = AgentsRepository(conn).find_image_record(agent_id) + if not agent: + return make_response( + jsonify({"success": False, "message": "Image not found"}), 404 + ) + + image_path = agent.get("image") + user_id = agent.get("user_id") + content_type = get_agent_image_content_type(image_path) + if ( + is_external_image_url(image_path) + or not content_type + or not is_safe_agent_image_path(image_path, user_id) + or not verify_agent_image_capability( + capability, agent_id, image_path, user_id + ) + ): + return make_response( + jsonify({"success": False, "message": "Image not found"}), 404 + ) + from application.api.user.base import storage + size_bytes = storage.get_file_size(image_path) + if size_bytes < 0 or size_bytes > settings.AGENT_IMAGE_MAX_BYTES: + current_app.logger.warning( + "Rejected oversized legacy agent image for agent %s", agent_id + ) + return make_response( + jsonify({"success": False, "message": "Image not found"}), 404 + ) + + if settings.STORAGE_TYPE == "s3": + image_url = storage.generate_presigned_url( + image_path, + expires_in=_AGENT_IMAGE_PRESIGNED_TTL_SECONDS, + content_type=content_type, + ) + response = redirect(image_url, code=302) + response.headers.set( + "Cache-Control", + f"private, max-age={_AGENT_IMAGE_REDIRECT_CACHE_SECONDS}", + ) + response.headers.set("X-Content-Type-Options", "nosniff") + return response + file_obj = storage.get_file(image_path) - extension = image_path.split(".")[-1].lower() - content_type = f"image/{extension}" - if extension == "jpg": - content_type = "image/jpeg" - response = make_response(file_obj.read()) - response.headers.set("Content-Type", content_type) - response.headers.set("Cache-Control", "max-age=86400") + response = Response( + _stream_file(file_obj, size_bytes), + content_type=content_type, + direct_passthrough=True, + ) + response.content_length = size_bytes + response.headers.set("Cache-Control", "public, max-age=86400, immutable") + response.headers.set("X-Content-Type-Options", "nosniff") return response except FileNotFoundError: @@ -657,7 +728,7 @@ class ServeImage(Resource): ) except ValueError: return make_response( - jsonify({"success": False, "message": "Invalid image path"}), 400 + jsonify({"success": False, "message": "Image not found"}), 404 ) except Exception as e: current_app.logger.error(f"Error serving image: {e}") diff --git a/application/api/user/base.py b/application/api/user/base.py index 5aaa478e..96c6a80d 100644 --- a/application/api/user/base.py +++ b/application/api/user/base.py @@ -6,9 +6,11 @@ import datetime import os import uuid from functools import wraps +from pathlib import PurePosixPath from typing import Optional, Tuple from flask import current_app, jsonify, make_response, Response +from PIL import Image, UnidentifiedImageError from werkzeug.utils import secure_filename from sqlalchemy import text as _sql_text @@ -18,6 +20,7 @@ from application.storage.db.base_repository import looks_like_uuid, row_to_dict from application.storage.db.repositories.users import UsersRepository from application.storage.db.session import db_readonly, db_session from application.storage.storage_creator import StorageCreator +from application.utils import AGENT_IMAGE_FORMATS, safe_user_storage_component from application.vectorstore.vector_creator import VectorCreator @@ -28,7 +31,6 @@ current_dir = os.path.dirname( os.path.dirname(os.path.dirname(os.path.abspath(__file__))) ) - def generate_minute_range(start_date, end_date): """Generate a dictionary with minute-level time ranges.""" return { @@ -237,8 +239,63 @@ def get_vector_store(source_id): return store +def _validate_agent_image_upload(file, filename: str) -> None: + """Validate an uploaded agent avatar and rewind it for storage. + + Args: + file: Werkzeug file upload object. + filename: Sanitized upload filename. + + Raises: + OSError: If the upload stream cannot be inspected. + ValueError: If the file exceeds a limit or is not an allowed raster image. + """ + extension = PurePosixPath(filename).suffix.lower() + image_policy = AGENT_IMAGE_FORMATS.get(extension) + if not filename or image_policy is None: + raise ValueError("Unsupported image extension") + + stream = file.stream + stream.seek(0, os.SEEK_END) + size_bytes = stream.tell() + stream.seek(0) + if size_bytes > settings.AGENT_IMAGE_MAX_BYTES: + raise ValueError("Image exceeds the upload size limit") + + with Image.open(stream) as image: + image_format = (image.format or "").upper() + if image_format != image_policy[0]: + raise ValueError("Image content does not match its extension") + if image.width * image.height > settings.AGENT_IMAGE_MAX_PIXELS: + raise ValueError("Image exceeds the pixel limit") + image.verify() + stream.seek(0) + + +def _safe_agent_image_filename(filename: str) -> str: + """Build an ASCII storage filename without discarding a Unicode file's suffix. + + Args: + filename: Original multipart upload filename. + + Returns: + A traversal-safe filename with a normalized, allow-listed extension. + + Raises: + ValueError: If the original filename has no supported image extension. + """ + normalized_path = filename.replace("\\", "/") + original_path = PurePosixPath(normalized_path) + extension = original_path.suffix.lower() + if extension not in AGENT_IMAGE_FORMATS: + raise ValueError("Unsupported image extension") + + safe_stem = secure_filename(original_path.stem).strip("._") or "avatar" + return f"{safe_stem}{extension}" + + def handle_image_upload( - request, existing_url: str, user: str, storage, base_path: str = "attachments/" + request, existing_url: str, user: str, storage ) -> Tuple[str, Optional[Response]]: """ Handle image file upload from request. @@ -248,7 +305,6 @@ def handle_image_upload( existing_url: Existing image URL (fallback) user: User ID storage: Storage instance - base_path: Base path for upload Returns: Tuple of (image_url, error_response) @@ -258,8 +314,26 @@ def handle_image_upload( if "image" in request.files: file = request.files["image"] if file.filename != "": - filename = secure_filename(file.filename) - upload_path = f"{settings.UPLOAD_FOLDER.rstrip('/')}/{user}/{base_path.rstrip('/')}/{uuid.uuid4()}_{filename}" + try: + filename = _safe_agent_image_filename(file.filename) + _validate_agent_image_upload(file, filename) + except ( + Image.DecompressionBombError, + UnidentifiedImageError, + ValueError, + OSError, + ) as e: + current_app.logger.warning(f"Invalid agent image upload: {e}") + return None, make_response( + jsonify({"success": False, "message": "Invalid image upload"}), + 400, + ) + + owner_component = safe_user_storage_component(user) + upload_path = ( + f"{settings.UPLOAD_FOLDER.rstrip('/')}/{owner_component}/" + f"attachments/{uuid.uuid4()}_{filename}" + ) try: storage.save_file(file, upload_path, storage_class="STANDARD") image_url = upload_path diff --git a/application/app.py b/application/app.py index bf27bf52..b717ba9c 100644 --- a/application/app.py +++ b/application/app.py @@ -28,6 +28,7 @@ from application.api.user.routes import user # noqa: E402 from application.api.connector.routes import connector # noqa: E402 from application.api.v1 import v1_bp # noqa: E402 from application.celery_init import celery # noqa: E402 +from application.core.secret_key import resolve_jwt_secret_key # noqa: E402 from application.core.settings import settings # noqa: E402 from application.storage.db.bootstrap import ensure_database_ready # noqa: E402 from application.stt.upload_limits import ( # noqa: E402 @@ -84,18 +85,13 @@ app.config.update( celery.config_from_object("application.celeryconfig") api.init_app(app) -if settings.AUTH_TYPE in ("simple_jwt", "session_jwt", "oidc") and not settings.JWT_SECRET_KEY: - key_file = ".jwt_secret_key" - try: - with open(key_file, "r") as f: - settings.JWT_SECRET_KEY = f.read().strip() - except FileNotFoundError: - new_key = os.urandom(32).hex() - with open(key_file, "w") as f: - f.write(new_key) - settings.JWT_SECRET_KEY = new_key - except Exception as e: - raise RuntimeError(f"Failed to setup JWT_SECRET_KEY: {e}") +# The same stable secret also signs opaque agent-image capabilities, including +# in no-auth mode. Production replicas must receive one shared configured key; +# only local development may use the atomic filesystem fallback. +settings.JWT_SECRET_KEY = resolve_jwt_secret_key( + settings.JWT_SECRET_KEY, + os.getenv("DEPLOYMENT_TYPE"), +) if settings.AUTH_TYPE == "oidc": _missing_oidc = [ name diff --git a/application/core/secret_key.py b/application/core/secret_key.py new file mode 100644 index 00000000..fdc50f93 --- /dev/null +++ b/application/core/secret_key.py @@ -0,0 +1,89 @@ +"""Resolve stable application signing secrets for local and production use.""" + +from __future__ import annotations + +import os +import tempfile +from pathlib import Path + + +_SHARED_SECRET_DEPLOYMENTS = {"cloud", "production"} + + +def _read_secret_file(key_path: Path) -> str: + """Read and validate a generated local signing secret.""" + secret = key_path.read_text(encoding="utf-8").strip() + if not secret: + raise RuntimeError(f"Signing secret file is empty: {key_path}") + return secret + + +def _create_secret_file_atomically(key_path: Path) -> str: + """Create a complete mode-0600 secret before atomically publishing it.""" + new_secret = os.urandom(32).hex() + descriptor, temporary_name = tempfile.mkstemp( + dir=key_path.parent, + prefix=f".{key_path.name}.", + text=True, + ) + temporary_path = Path(temporary_name) + try: + if hasattr(os, "fchmod"): + os.fchmod(descriptor, 0o600) + with os.fdopen(descriptor, "w", encoding="utf-8") as temporary_file: + descriptor = -1 + temporary_file.write(new_secret) + temporary_file.flush() + os.fsync(temporary_file.fileno()) + + try: + # A hard link publishes the fully written file without replacing a + # secret another process may have won the race to create. + os.link(temporary_path, key_path) + except FileExistsError: + return _read_secret_file(key_path) + return new_secret + finally: + if descriptor >= 0: + os.close(descriptor) + temporary_path.unlink(missing_ok=True) + + +def resolve_jwt_secret_key( + configured_secret: str | None, + deployment_type: str | None, + key_file: str | Path = ".jwt_secret_key", +) -> str: + """Return a shared configured secret or a stable local-development secret. + + Args: + configured_secret: Operator-supplied signing secret. + deployment_type: Deployment class, such as ``cloud`` or ``production``. + key_file: Local fallback file used outside production deployments. + + Returns: + The configured or locally persisted signing secret. + + Raises: + RuntimeError: If production lacks a shared secret or local persistence fails. + """ + if configured_secret and configured_secret.strip(): + return configured_secret + + normalized_deployment = (deployment_type or "").strip().lower() + if normalized_deployment in _SHARED_SECRET_DEPLOYMENTS: + raise RuntimeError( + "JWT_SECRET_KEY must be set to the same strong random value on every " + f"{normalized_deployment} API and worker replica" + ) + + key_path = Path(key_file) + try: + return _read_secret_file(key_path) + except FileNotFoundError: + try: + return _create_secret_file_atomically(key_path) + except Exception as exc: + raise RuntimeError(f"Failed to create signing secret: {exc}") from exc + except Exception as exc: + raise RuntimeError(f"Failed to read signing secret: {exc}") from exc diff --git a/application/core/settings.py b/application/core/settings.py index 12d8924f..b170fb22 100644 --- a/application/core/settings.py +++ b/application/core/settings.py @@ -99,6 +99,8 @@ class Settings(BaseSettings): DOCLING_TABULAR_MAX_BYTES: int = 2_000_000 DOCLING_MARKUP_MAX_BYTES: int = 8_000_000 ATTACHMENT_TEXT_MAX_BYTES: int = 5_000_000 + AGENT_IMAGE_MAX_BYTES: int = 5_000_000 + AGENT_IMAGE_MAX_PIXELS: int = 16_777_216 VECTOR_STORE: str = "faiss" # "faiss" or "elasticsearch" or "qdrant" or "milvus" or "lancedb" or "pgvector" # Allow-list of retriever keys an agent may use. Values must match the # ``RetrieverCreator.retrievers`` registry keys (``classic`` / ``default``), diff --git a/application/storage/base.py b/application/storage/base.py index 1dae6506..af505014 100644 --- a/application/storage/base.py +++ b/application/storage/base.py @@ -1,7 +1,7 @@ """Base storage class for file system abstraction.""" from abc import ABC, abstractmethod -from typing import BinaryIO, List, Callable +from typing import BinaryIO, Callable, List, Optional class BaseStorage(ABC): @@ -37,12 +37,41 @@ class BaseStorage(ABC): """ pass - def generate_presigned_url(self, path: str, expires_in: int = 300) -> str: + def get_file_size(self, path: str) -> int: + """Return a file's encoded size without retaining its bytes in memory. + + Storage backends should override this with a metadata-only lookup. The + compatibility fallback seeks an already-open file and is suitable for + file-like backends that do not provide object metadata. + + Args: + path: Path to the file. + + Returns: + File size in bytes. + """ + file_obj = self.get_file(path) + try: + current = file_obj.tell() + file_obj.seek(0, 2) + size = file_obj.tell() + file_obj.seek(current) + return size + finally: + file_obj.close() + + def generate_presigned_url( + self, + path: str, + expires_in: int = 300, + content_type: Optional[str] = None, + ) -> str: """Return a short-lived presigned download URL; not all backends support it. Args: path: Path to the file expires_in: TTL of the signed URL in seconds + content_type: Optional response Content-Type override Returns: str: A presigned URL granting time-limited read access diff --git a/application/storage/db/repositories/agents.py b/application/storage/db/repositories/agents.py index 22cb5653..cdec8a2a 100644 --- a/application/storage/db/repositories/agents.py +++ b/application/storage/db/repositories/agents.py @@ -176,6 +176,25 @@ class AgentsRepository: row = result.fetchone() return row_to_dict(row) if row is not None else None + def find_image_record(self, agent_id: str) -> Optional[dict]: + """Fetch only the fields needed to verify a public image capability. + + This lookup is intentionally not owner-scoped because the resulting + row is not authorization: callers must verify its HMAC capability + before reading or returning any storage bytes. + """ + if not looks_like_uuid(agent_id): + return None + result = self._conn.execute( + text( + "SELECT id, user_id, image FROM agents " + "WHERE id = CAST(:id AS uuid)" + ), + {"id": agent_id}, + ) + row = result.fetchone() + return row_to_dict(row) if row is not None else None + def list_by_ids(self, agent_ids) -> list[dict]: """Fetch agents whose id is in ``agent_ids`` (team-shared listing path).""" ids = [str(a) for a in agent_ids if looks_like_uuid(str(a))] diff --git a/application/storage/local.py b/application/storage/local.py index a74f6181..a8ec5340 100644 --- a/application/storage/local.py +++ b/application/storage/local.py @@ -60,6 +60,14 @@ class LocalStorage(BaseStorage): return open(full_path, 'rb') + def get_file_size(self, path: str) -> int: + """Return the size of a local file without opening and buffering it.""" + full_path = self._get_full_path(path) + try: + return os.path.getsize(full_path) + except FileNotFoundError as exc: + raise FileNotFoundError(f"File not found: {full_path}") from exc + def delete_file(self, path: str) -> bool: """Delete a file from local storage.""" full_path = self._get_full_path(path) diff --git a/application/storage/s3.py b/application/storage/s3.py index 9c16fab2..d6bf7d96 100644 --- a/application/storage/s3.py +++ b/application/storage/s3.py @@ -121,13 +121,33 @@ class S3Storage(BaseStorage): file_obj.seek(0) return file_obj - def generate_presigned_url(self, path: str, expires_in: int = 300) -> str: + def get_file_size(self, path: str) -> int: + """Return an S3 object's size using HEAD without downloading its body.""" + path = self._validate_path(path) + try: + metadata = self.s3.head_object(Bucket=self.bucket_name, Key=path) + except ClientError as exc: + error_code = str(exc.response.get("Error", {}).get("Code", "")) + if error_code in {"404", "NoSuchKey", "NotFound"}: + raise FileNotFoundError(f"File not found: {path}") from exc + raise + return int(metadata["ContentLength"]) + + def generate_presigned_url( + self, + path: str, + expires_in: int = 300, + content_type: Optional[str] = None, + ) -> str: """Return a short-lived presigned GET URL for a private object (TTL <= 1h).""" path = self._validate_path(path) expires_in = min(expires_in, 3600) + params = {"Bucket": self.bucket_name, "Key": path} + if content_type: + params["ResponseContentType"] = content_type return self.s3.generate_presigned_url( "get_object", - Params={"Bucket": self.bucket_name, "Key": path}, + Params=params, ExpiresIn=expires_in, ) diff --git a/application/utils.py b/application/utils.py index 60032893..3c3e95dd 100644 --- a/application/utils.py +++ b/application/utils.py @@ -1,12 +1,13 @@ import base64 import hashlib +import hmac import io import logging import os import re import uuid +from pathlib import PurePosixPath from typing import List -from urllib.parse import urlparse import tiktoken from flask import jsonify, make_response @@ -237,31 +238,117 @@ def validate_function_name(function_name): return True -def generate_image_url(image_path): +AGENT_IMAGE_FORMATS = { + ".gif": ("GIF", "image/gif"), + ".jpeg": ("JPEG", "image/jpeg"), + ".jpg": ("JPEG", "image/jpeg"), + ".png": ("PNG", "image/png"), + ".webp": ("WEBP", "image/webp"), +} + + +def is_external_image_url(image_path: object) -> bool: + """Return whether an image value is an externally hosted HTTP(S) URL.""" + return isinstance(image_path, str) and image_path.startswith(("http://", "https://")) + + +def safe_user_storage_component(user_id: object) -> str: + """Return a deterministic, traversal-safe directory component for a user ID.""" + raw_user_id = str(user_id or "") + component = secure_filename(raw_user_id)[:80] or "user" + digest = hashlib.sha256(raw_user_id.encode("utf-8")).hexdigest()[:16] + return f"{component}-{digest}" + + +def get_agent_image_content_type(image_path: object) -> str | None: + """Return an allow-listed raster MIME type for an internal image path.""" + if not isinstance(image_path, str): + return None + policy = AGENT_IMAGE_FORMATS.get(PurePosixPath(image_path).suffix.lower()) + return policy[1] if policy else None + + +def is_safe_agent_image_path(image_path: object, user_id: object) -> bool: + """Validate that a path is an agent avatar under its owner's upload directory.""" + if not isinstance(image_path, str) or not image_path or not user_id: + return False + if is_external_image_url(image_path) or "\\" in image_path or "\x00" in image_path: + return False + + candidate = PurePosixPath(image_path) + upload_root = PurePosixPath(str(settings.UPLOAD_FOLDER).rstrip("/")) + if candidate.is_absolute() or upload_root.is_absolute() or ".." in candidate.parts: + return False + if get_agent_image_content_type(image_path) is None: + return False + + owner_components = {safe_user_storage_component(user_id)} + raw_user_id = str(user_id) + raw_owner = PurePosixPath(raw_user_id) + if ( + len(raw_owner.parts) == 1 + and raw_user_id not in {"", ".", ".."} + and "\\" not in raw_user_id + and "\x00" not in raw_user_id + ): + # Compatibility for avatars written before user IDs were sanitized. + owner_components.add(raw_user_id) + + for owner_component in owner_components: + expected_parent = upload_root / owner_component / "attachments" + if candidate.parent == expected_parent and candidate.name: + return True + return False + + +def generate_agent_image_capability( + agent_id: object, image_path: object, user_id: object +) -> str: + """Create an HMAC capability for one agent's current internal image.""" + secret = getattr(settings, "JWT_SECRET_KEY", "") + if not isinstance(secret, str) or not secret: + return "" + try: + canonical_agent_id = str(uuid.UUID(str(agent_id))) + except (TypeError, ValueError, AttributeError): + return "" + if not isinstance(image_path, str) or not user_id: + return "" + payload = ( + f"docsgpt-agent-image-v1\0{canonical_agent_id}\0{user_id}\0{image_path}" + ).encode("utf-8") + return hmac.new(secret.encode("utf-8"), payload, hashlib.sha256).hexdigest() + + +def verify_agent_image_capability( + capability: object, agent_id: object, image_path: object, user_id: object +) -> bool: + """Verify an agent image capability without timing-leaky string comparison.""" + if not isinstance(capability, str) or not re.fullmatch(r"[0-9a-f]{64}", capability): + return False + expected = generate_agent_image_capability(agent_id, image_path, user_id) + return bool(expected) and hmac.compare_digest(capability, expected) + + +def generate_image_url(image_path, agent_id=None, user_id=None): + """Return an external URL or an opaque capability URL for an agent image. + + Internal storage paths are never included in the returned URL. Invalid or + unscoped paths fail closed so a poisoned database row cannot become a file + read capability. + """ if isinstance(image_path, str) and ( image_path.startswith("http://") or image_path.startswith("https://") ): return image_path - strategy = getattr(settings, "URL_STRATEGY", "backend") - if strategy == "s3": - bucket_name = settings.S3_BUCKET_NAME - endpoint_url = settings.S3_ENDPOINT_URL - if endpoint_url: - # S3-compatible service (MinIO, R2, B2, Spaces, ...). - base = endpoint_url.rstrip("/") - if settings.S3_PATH_STYLE: - return f"{base}/{bucket_name}/{image_path}" - parsed = urlparse(base) - return f"{parsed.scheme}://{bucket_name}.{parsed.netloc}/{image_path}" - region_name = ( - settings.S3_REGION - or getattr(settings, "SAGEMAKER_REGION", None) - or "eu-central-1" - ) - return f"https://{bucket_name}.s3.{region_name}.amazonaws.com/{image_path}" - else: - base_url = getattr(settings, "API_URL", "http://localhost:7091") - return f"{base_url}/api/images/{image_path}" + if not is_safe_agent_image_path(image_path, user_id): + return "" + capability = generate_agent_image_capability(agent_id, image_path, user_id) + if not capability: + return "" + canonical_agent_id = str(uuid.UUID(str(agent_id))) + base_url = getattr(settings, "API_URL", "http://localhost:7091").rstrip("/") + return f"{base_url}/api/images/{canonical_agent_id}/{capability}" def calculate_compression_threshold( diff --git a/deployment/k8s/deployments/docsgpt-deploy.yaml b/deployment/k8s/deployments/docsgpt-deploy.yaml index 345b1449..fc7631fd 100644 --- a/deployment/k8s/deployments/docsgpt-deploy.yaml +++ b/deployment/k8s/deployments/docsgpt-deploy.yaml @@ -99,6 +99,8 @@ spec: - secretRef: name: docsgpt-secrets env: + - name: DEPLOYMENT_TYPE + value: "cloud" - name: API_URL value: "http://" - name: POSTGRES_URI diff --git a/deployment/k8s/docsgpt-secrets.yaml b/deployment/k8s/docsgpt-secrets.yaml index 6d558a0f..8fc04bdc 100644 --- a/deployment/k8s/docsgpt-secrets.yaml +++ b/deployment/k8s/docsgpt-secrets.yaml @@ -12,6 +12,9 @@ type: Opaque # manually add a MONGO_URI key below (base64-encoded) and apply the # opt-in manifests under deployment/k8s/optional-mongo/. Example: # MONGO_URI: +# - Before deploying, add JWT_SECRET_KEY below using the base64 encoding of a +# strong random value. Every API and worker replica must receive the same key. +# Example generation: openssl rand -hex 32 | base64 data: LLM_PROVIDER: ZG9jc2dwdA== INTERNAL_KEY: aW50ZXJuYWw= @@ -23,3 +26,4 @@ data: POSTGRES_URI: cG9zdGdyZXNxbDovL2RvY3NncHQ6ZG9jc2dwdEBwb3N0Z3Jlczo1NDMyL2RvY3NncHQ= postgres-user: ZG9jc2dwdA== postgres-password: ZG9jc2dwdA== + # JWT_SECRET_KEY: diff --git a/docs/content/Deploying/DocsGPT-Settings.mdx b/docs/content/Deploying/DocsGPT-Settings.mdx index 42234d92..3e25ee7c 100644 --- a/docs/content/Deploying/DocsGPT-Settings.mdx +++ b/docs/content/Deploying/DocsGPT-Settings.mdx @@ -217,6 +217,17 @@ STT_ENABLE_DIARIZATION=false `faster_whisper` is an optional backend dependency. Install it in the Python environment used by the DocsGPT API and worker before selecting this provider. +## Agent Image Settings + +Agent avatars uploaded to DocsGPT must be valid PNG, JPEG, GIF, or WebP files. +The decoded image and its file extension must agree; SVG and other formats are +rejected. These limits apply before the image is written to local or S3 storage. + +| Setting | Default | Description | +| --- | --- | --- | +| `AGENT_IMAGE_MAX_BYTES` | `5000000` | Maximum encoded avatar file size in bytes. | +| `AGENT_IMAGE_MAX_PIXELS` | `16777216` | Maximum decoded width × height, limiting decompression-bomb images. | + ## Authentication Settings DocsGPT includes a JWT (JSON Web Token) based authentication feature for managing sessions or securing local deployments while allowing access. @@ -237,27 +248,28 @@ The `AUTH_TYPE` setting in your `.env` file or `settings.py` determines the auth Add the following to your `.env` file (or set in `settings.py`): ```env +# Shared signing key (required in production for every authentication mode) +JWT_SECRET_KEY= + # No authentication (default) AUTH_TYPE=None # OR: Simple JWT (shared token) AUTH_TYPE=simple_jwt -JWT_SECRET_KEY=your_secret_key_here # OR: Session JWT (per-user/session tokens) AUTH_TYPE=session_jwt -JWT_SECRET_KEY=your_secret_key_here # OR: SSO via an OpenID Connect provider (Authentik, Keycloak, Okta, ...) AUTH_TYPE=oidc OIDC_ISSUER=https://auth.example.com/application/o/docsgpt/ OIDC_CLIENT_ID=your_client_id OIDC_FRONTEND_URL=https://docsgpt.example.com -JWT_SECRET_KEY=your_secret_key_here ``` -- If `AUTH_TYPE` is set to `simple_jwt` or `session_jwt`, a `JWT_SECRET_KEY` is required. -- If `JWT_SECRET_KEY` is not set, DocsGPT will generate one and store it in `.jwt_secret_key` in the project root. +- `JWT_SECRET_KEY` signs authentication tokens where applicable and opaque agent-avatar capabilities in every authentication mode, including no-auth mode. +- Cloud and production deployments must set a strong `JWT_SECRET_KEY`, shared unchanged by every API and worker replica. Startup fails rather than creating replica-local keys when it is missing. +- Local development may omit it. DocsGPT atomically generates an owner-readable `.jwt_secret_key` in the project root and reuses it on later starts. #### How Each Method Works @@ -283,7 +295,7 @@ JWT_SECRET_KEY=your_secret_key_here - Always keep your `JWT_SECRET_KEY` secure and private. - If you set it manually, use a strong, random string. -- If not set, DocsGPT will generate a secure key and persist it in `.jwt_secret_key`. +- Keep the value stable. Rotating it invalidates active JWTs and previously generated agent-avatar URLs. #### Checking Current Auth Type @@ -317,7 +329,7 @@ By default DocsGPT stores files locally. Set `STORAGE_TYPE=s3` to use Amazon S3 | `S3_REGION` | Region (use `auto` for Cloudflare R2) | — | | `S3_ENDPOINT_URL` | Custom endpoint for S3-compatible services; leave unset for AWS S3 | — | | `S3_PATH_STYLE` | Use path-style addressing (required by most non-AWS services) | `false` | -| `URL_STRATEGY` | `backend` (proxy through API) or `s3` (direct object URLs) | `backend` | +| `URL_STRATEGY` | Artifact-download delivery: `backend` proxies bytes through the API; `s3` returns short-lived presigned object URLs. Agent avatars always use capability URLs and, with S3 storage, redirect to a size-checked presigned URL. | `backend` | ### AWS S3 diff --git a/docs/content/Deploying/Kubernetes-Deploying.mdx b/docs/content/Deploying/Kubernetes-Deploying.mdx index e671be1a..e0c73633 100644 --- a/docs/content/Deploying/Kubernetes-Deploying.mdx +++ b/docs/content/Deploying/Kubernetes-Deploying.mdx @@ -33,9 +33,11 @@ The `deployment/k8s` folder contains the necessary deployment and service config cd docsgpt/deployment/k8s ``` -2. **Configure Secrets (optional)** +2. **Configure Secrets** - Ensure that you have all the necessary secrets in `docsgpt-secrets.yaml`. Update it with your secrets before applying if you want. By default we will use qdrant as a vectorstore and public docsgpt llm as llm for inference. + Add a strong, base64-encoded `JWT_SECRET_KEY` to `docsgpt-secrets.yaml` before applying it. The API and worker deployments share this value so authentication tokens and agent-avatar capability URLs remain valid across pod restarts and replicas. You can generate a value with `openssl rand -hex 32 | base64`. + + Configure any additional provider secrets you need. By default, the manifests use Qdrant as the vector store and the public DocsGPT LLM for inference. Alternatively, you can use [Neon](https://get.neon.com/docsgpt) as an easy way to set up your vector store with pgvector, which is highly recommended for quick deployments. @@ -106,4 +108,4 @@ If you encounter any issues, you can check the logs of the pods for more details kubectl logs ``` -Replace `` with the actual name of your DocsGPT pod. \ No newline at end of file +Replace `` with the actual name of your DocsGPT pod. diff --git a/docs/content/Deploying/OIDC-SSO.mdx b/docs/content/Deploying/OIDC-SSO.mdx index 377ea929..8851994e 100644 --- a/docs/content/Deploying/OIDC-SSO.mdx +++ b/docs/content/Deploying/OIDC-SSO.mdx @@ -45,7 +45,7 @@ Sessions last `OIDC_SESSION_LIFETIME_SECONDS` (8 hours by default) and renew wit | `OIDC_ALLOWED_GROUPS` | no | — | Comma-separated group allowlist. Unset, any authenticated IdP user may sign in — see [Restricting sign-in by group](#restricting-sign-in-by-group). | | `OIDC_ADMIN_GROUPS` | no | — | Comma-separated groups whose members are granted the global `admin` role. Re-checked at every login and renewal — see [Granting admin via groups](#granting-admin-via-groups). | | `OIDC_GROUPS_CLAIM` | no | `groups` | ID-token/userinfo claim carrying the user's group membership. | -| `JWT_SECRET_KEY` | recommended | auto-generated | Signs DocsGPT session tokens. Set it explicitly in production — required when running multiple API replicas. | +| `JWT_SECRET_KEY` | required in production | auto-generated for local development | Signs DocsGPT session tokens and agent-avatar capabilities. Every API and worker replica must use the same value. | `SCIM_ENABLED` and `SCIM_TOKEN` are listed in the [SCIM section](#scim-user-provisioning). diff --git a/tests/api/user/agents/test_routes_happy.py b/tests/api/user/agents/test_routes_happy.py index 24483dd2..94aaa559 100644 --- a/tests/api/user/agents/test_routes_happy.py +++ b/tests/api/user/agents/test_routes_happy.py @@ -465,6 +465,28 @@ class TestUpdateAgent: got = AgentsRepository(pg_conn).get(str(agent["id"]), user) assert got["name"] == "new name" + def test_ignores_client_supplied_image_path(self, app, pg_conn): + from application.api.user.agents.routes import UpdateAgent + from application.storage.db.repositories.agents import AgentsRepository + + user = "u-upd-image-path" + agent = _seed_agent( + pg_conn, user=user, status="draft", with_source=False, + ) + + with _patch_db(pg_conn), app.test_request_context( + f"/api/update_agent/{agent['id']}", method="PUT", + json={"description": "safe update", "image": ".env"}, + ): + from flask import request + request.decoded_token = {"sub": user} + response = UpdateAgent().put(str(agent["id"])) + + assert response.status_code == 200 + got = AgentsRepository(pg_conn).get(str(agent["id"]), user) + assert got["description"] == "safe update" + assert got["image"] is None + def test_invalid_status_returns_400(self, app, pg_conn): from application.api.user.agents.routes import UpdateAgent diff --git a/tests/api/user/agents/test_routes_helpers.py b/tests/api/user/agents/test_routes_helpers.py index a7572a07..ecbf2bb1 100644 --- a/tests/api/user/agents/test_routes_helpers.py +++ b/tests/api/user/agents/test_routes_helpers.py @@ -258,3 +258,11 @@ class TestBuildCreateKwargs: {}, image_url="/upload/img.png", agent_type="classic", ) assert out.get("image") == "/upload/img.png" + + def test_client_image_path_is_never_persisted(self): + from application.api.user.agents.routes import _build_create_kwargs + + out = _build_create_kwargs( + {"image": ".env"}, image_url="", agent_type="classic", + ) + assert "image" not in out diff --git a/tests/api/user/attachments/test_routes.py b/tests/api/user/attachments/test_routes.py index c010918e..02949aa4 100644 --- a/tests/api/user/attachments/test_routes.py +++ b/tests/api/user/attachments/test_routes.py @@ -1520,23 +1520,53 @@ class TestLiveSpeechToTextAdditional: class TestServeImage: """Tests for ServeImage endpoint.""" + agent_id = "00000000-0000-0000-0000-000000000001" + image_path = "inputs/user123/attachments/avatar.png" + + @staticmethod + def _capability(agent_id, image_path, user_id="user123"): + from application.utils import generate_agent_image_capability + + return generate_agent_image_capability(agent_id, image_path, user_id) + def test_serve_image_success(self, flask_app): from application.api.user.attachments.routes import ServeImage app = Flask(__name__) mock_storage = MagicMock() mock_file_obj = io.BytesIO(b"\x89PNG\r\n") + mock_storage.get_file_size.return_value = len(mock_file_obj.getvalue()) mock_storage.get_file.return_value = mock_file_obj - with patch("application.api.user.base.storage", mock_storage): + agent = { + "id": self.agent_id, + "user_id": "user123", + "image": self.image_path, + } + + with patch( + "application.utils.settings.JWT_SECRET_KEY", "test-image-secret" + ), patch( + "application.api.user.attachments.routes.AgentsRepository.find_image_record", + return_value=agent, + ), patch( + "application.api.user.attachments.routes.db_readonly", _fake_readonly + ), patch( + "application.api.user.attachments.routes.settings.STORAGE_TYPE", "local" + ), patch("application.api.user.base.storage", mock_storage): + capability = self._capability(self.agent_id, self.image_path) with app.test_request_context( - "/api/images/test/image.png", + f"/api/images/{self.agent_id}/{capability}", method="GET", ): resource = ServeImage() - response = resource.get("test/image.png") + response = resource.get(self.agent_id, capability) assert _get_response_status(response) == 200 assert response.headers.get("Content-Type") == "image/png" + assert response.headers.get("X-Content-Type-Options") == "nosniff" + assert response.is_streamed + assert b"".join(response.response) == b"\x89PNG\r\n" + mock_storage.get_file.assert_called_once_with(self.image_path) def test_serve_image_jpg_content_type(self, flask_app): from application.api.user.attachments.routes import ServeImage @@ -1544,32 +1574,184 @@ class TestServeImage: app = Flask(__name__) mock_storage = MagicMock() mock_file_obj = io.BytesIO(b"\xff\xd8\xff\xe0") + mock_storage.get_file_size.return_value = len(mock_file_obj.getvalue()) mock_storage.get_file.return_value = mock_file_obj - with patch("application.api.user.base.storage", mock_storage): + image_path = "inputs/user123/attachments/photo.jpg" + agent = { + "id": self.agent_id, + "user_id": "user123", + "image": image_path, + } + + with patch( + "application.utils.settings.JWT_SECRET_KEY", "test-image-secret" + ), patch( + "application.api.user.attachments.routes.AgentsRepository.find_image_record", + return_value=agent, + ), patch( + "application.api.user.attachments.routes.db_readonly", _fake_readonly + ), patch( + "application.api.user.attachments.routes.settings.STORAGE_TYPE", "local" + ), patch("application.api.user.base.storage", mock_storage): + capability = self._capability(self.agent_id, image_path) with app.test_request_context( - "/api/images/test/photo.jpg", + f"/api/images/{self.agent_id}/{capability}", method="GET", ): resource = ServeImage() - response = resource.get("test/photo.jpg") + response = resource.get(self.agent_id, capability) assert _get_response_status(response) == 200 assert response.headers.get("Content-Type") == "image/jpeg" - def test_serve_image_not_found(self, flask_app): + def test_s3_avatar_redirects_without_downloading_object(self, flask_app): from application.api.user.attachments.routes import ServeImage app = Flask(__name__) mock_storage = MagicMock() - mock_storage.get_file.side_effect = FileNotFoundError("not found") + mock_storage.get_file_size.return_value = 1024 + mock_storage.generate_presigned_url.return_value = ( + "https://bucket.example/avatar.png?signature=short-lived" + ) + agent = { + "id": self.agent_id, + "user_id": "user123", + "image": self.image_path, + } - with patch("application.api.user.base.storage", mock_storage): + with patch( + "application.utils.settings.JWT_SECRET_KEY", "test-image-secret" + ), patch( + "application.api.user.attachments.routes.AgentsRepository.find_image_record", + return_value=agent, + ), patch( + "application.api.user.attachments.routes.db_readonly", _fake_readonly + ), patch( + "application.api.user.attachments.routes.settings.STORAGE_TYPE", "s3" + ), patch("application.api.user.base.storage", mock_storage): + capability = self._capability(self.agent_id, self.image_path) with app.test_request_context( - "/api/images/missing/image.png", + f"/api/images/{self.agent_id}/{capability}", method="GET" + ): + response = ServeImage().get(self.agent_id, capability) + + assert response.status_code == 302 + assert response.headers["Location"].startswith("https://bucket.example/") + assert response.headers["Cache-Control"] == "private, max-age=240" + mock_storage.generate_presigned_url.assert_called_once_with( + self.image_path, expires_in=300, content_type="image/png" + ) + mock_storage.get_file.assert_not_called() + + def test_legacy_oversized_avatar_is_rejected_before_read(self, flask_app): + from application.api.user.attachments.routes import ServeImage + + app = Flask(__name__) + mock_storage = MagicMock() + mock_storage.get_file_size.return_value = 5001 + agent = { + "id": self.agent_id, + "user_id": "user123", + "image": self.image_path, + } + + with patch( + "application.utils.settings.JWT_SECRET_KEY", "test-image-secret" + ), patch( + "application.api.user.attachments.routes.AgentsRepository.find_image_record", + return_value=agent, + ), patch( + "application.api.user.attachments.routes.db_readonly", _fake_readonly + ), patch( + "application.api.user.attachments.routes.settings.AGENT_IMAGE_MAX_BYTES", 5000 + ), patch("application.api.user.base.storage", mock_storage): + capability = self._capability(self.agent_id, self.image_path) + with app.test_request_context( + f"/api/images/{self.agent_id}/{capability}", method="GET" + ): + response = ServeImage().get(self.agent_id, capability) + + assert response.status_code == 404 + mock_storage.get_file.assert_not_called() + mock_storage.generate_presigned_url.assert_not_called() + + def test_unknown_capability_never_reads_storage(self, flask_app): + from application.api.user.attachments.routes import ServeImage + + app = Flask(__name__) + mock_storage = MagicMock() + + agent = { + "id": self.agent_id, + "user_id": "user123", + "image": self.image_path, + } + + with patch( + "application.utils.settings.JWT_SECRET_KEY", "test-image-secret" + ), patch( + "application.api.user.attachments.routes.AgentsRepository.find_image_record", + return_value=agent, + ), patch( + "application.api.user.attachments.routes.db_readonly", _fake_readonly + ), patch("application.api.user.base.storage", mock_storage): + with app.test_request_context( + f"/api/images/{self.agent_id}/{'0' * 64}", method="GET", ): resource = ServeImage() - response = resource.get("missing/image.png") + response = resource.get(self.agent_id, "0" * 64) + assert _get_response_status(response) == 404 + mock_storage.get_file.assert_not_called() + + def test_poisoned_agent_path_never_reads_storage(self, flask_app): + from application.api.user.attachments.routes import ServeImage + + app = Flask(__name__) + mock_storage = MagicMock() + agent = {"id": self.agent_id, "user_id": "user123", "image": ".env"} + + with patch( + "application.utils.settings.JWT_SECRET_KEY", "test-image-secret" + ), patch( + "application.api.user.attachments.routes.AgentsRepository.find_image_record", + return_value=agent, + ), patch( + "application.api.user.attachments.routes.db_readonly", _fake_readonly + ), patch("application.api.user.base.storage", mock_storage): + capability = self._capability(self.agent_id, ".env") + with app.test_request_context( + f"/api/images/{self.agent_id}/{capability}", method="GET" + ): + response = ServeImage().get(self.agent_id, capability) + assert _get_response_status(response) == 404 + mock_storage.get_file.assert_not_called() + + def test_serve_image_file_not_found(self, flask_app): + from application.api.user.attachments.routes import ServeImage + + app = Flask(__name__) + mock_storage = MagicMock() + mock_storage.get_file_size.side_effect = FileNotFoundError("not found") + agent = { + "id": self.agent_id, + "user_id": "user123", + "image": self.image_path, + } + + with patch( + "application.utils.settings.JWT_SECRET_KEY", "test-image-secret" + ), patch( + "application.api.user.attachments.routes.AgentsRepository.find_image_record", + return_value=agent, + ), patch( + "application.api.user.attachments.routes.db_readonly", _fake_readonly + ), patch("application.api.user.base.storage", mock_storage): + capability = self._capability(self.agent_id, self.image_path) + with app.test_request_context( + f"/api/images/{self.agent_id}/{capability}", method="GET" + ): + response = ServeImage().get(self.agent_id, capability) assert _get_response_status(response) == 404 def test_serve_image_generic_error(self, flask_app): @@ -1577,15 +1759,29 @@ class TestServeImage: app = Flask(__name__) mock_storage = MagicMock() - mock_storage.get_file.side_effect = Exception("storage error") + mock_storage.get_file_size.side_effect = Exception("storage error") - with patch("application.api.user.base.storage", mock_storage): + agent = { + "id": self.agent_id, + "user_id": "user123", + "image": self.image_path, + } + + with patch( + "application.utils.settings.JWT_SECRET_KEY", "test-image-secret" + ), patch( + "application.api.user.attachments.routes.AgentsRepository.find_image_record", + return_value=agent, + ), patch( + "application.api.user.attachments.routes.db_readonly", _fake_readonly + ), patch("application.api.user.base.storage", mock_storage): + capability = self._capability(self.agent_id, self.image_path) with app.test_request_context( - "/api/images/broken/image.png", + f"/api/images/{self.agent_id}/{capability}", method="GET", ): resource = ServeImage() - response = resource.get("broken/image.png") + response = resource.get(self.agent_id, capability) assert _get_response_status(response) == 500 diff --git a/tests/api/user/test_base.py b/tests/api/user/test_base.py index 06a1545d..abdabdd4 100644 --- a/tests/api/user/test_base.py +++ b/tests/api/user/test_base.py @@ -4,6 +4,7 @@ from contextlib import contextmanager from unittest.mock import Mock, patch import pytest +from PIL import Image from werkzeug.datastructures import FileStorage @@ -112,7 +113,19 @@ class TestGetVectorStore: @pytest.mark.unit class TestHandleImageUpload: - pass + @staticmethod + def _image_bytes( + width: int = 1, height: int = 1, image_format: str = "PNG" + ) -> bytes: + buffer = io.BytesIO() + Image.new("RGB", (width, height), color="white").save( + buffer, format=image_format + ) + return buffer.getvalue() + + @classmethod + def _png_bytes(cls) -> bytes: + return cls._image_bytes() def test_returns_existing_url_when_no_file(self, flask_app): from application.api.user.base import handle_image_upload @@ -135,7 +148,7 @@ class TestHandleImageUpload: with flask_app.test_request_context(): mock_file = FileStorage( - stream=io.BytesIO(b"fake image data"), filename="test_image.png" + stream=io.BytesIO(self._png_bytes()), filename="test_image.png" ) mock_request = Mock() mock_request.files = {"image": mock_file} @@ -152,6 +165,23 @@ class TestHandleImageUpload: assert "user123" in url mock_storage.save_file.assert_called_once() + def test_uploads_image_with_non_ascii_basename(self, flask_app): + from application.api.user.base import handle_image_upload + + with flask_app.test_request_context(): + mock_file = FileStorage( + stream=io.BytesIO(self._png_bytes()), filename="上传.png" + ) + mock_storage = Mock() + + url, error = handle_image_upload( + Mock(files={"image": mock_file}), "", "user123", mock_storage + ) + + assert error is None + assert url.endswith("_avatar.png") + mock_storage.save_file.assert_called_once() + def test_ignores_empty_filename(self, flask_app): from application.api.user.base import handle_image_upload @@ -175,7 +205,9 @@ class TestHandleImageUpload: from application.api.user.base import handle_image_upload with flask_app.app_context(): - mock_file = FileStorage(stream=io.BytesIO(b"data"), filename="test.png") + mock_file = FileStorage( + stream=io.BytesIO(self._png_bytes()), filename="test.png" + ) mock_request = Mock() mock_request.files = {"image": mock_file} mock_storage = Mock() @@ -189,6 +221,135 @@ class TestHandleImageUpload: assert error is not None assert error.status_code == 400 + def test_rejects_non_image_content(self, flask_app): + from application.api.user.base import handle_image_upload + + with flask_app.app_context(): + mock_file = FileStorage( + stream=io.BytesIO(b"not an image"), filename="test.png" + ) + mock_request = Mock(files={"image": mock_file}) + mock_storage = Mock() + + url, error = handle_image_upload( + mock_request, "old.jpg", "user123", mock_storage + ) + + assert url is None + assert error.status_code == 400 + mock_storage.save_file.assert_not_called() + + def test_accepts_image_at_encoded_byte_limit(self, flask_app): + from application.api.user.base import handle_image_upload + + payload = self._png_bytes() + with patch( + "application.api.user.base.settings.AGENT_IMAGE_MAX_BYTES", + len(payload), + ), flask_app.app_context(): + mock_file = FileStorage( + stream=io.BytesIO(payload), filename="at-limit.png" + ) + mock_storage = Mock() + + url, error = handle_image_upload( + Mock(files={"image": mock_file}), "", "user123", mock_storage + ) + + assert error is None + assert url.endswith("_at-limit.png") + mock_storage.save_file.assert_called_once() + + def test_rejects_image_over_encoded_byte_limit(self, flask_app): + from application.api.user.base import handle_image_upload + + payload = self._png_bytes() + with patch( + "application.api.user.base.settings.AGENT_IMAGE_MAX_BYTES", + len(payload) - 1, + ), flask_app.app_context(): + mock_file = FileStorage( + stream=io.BytesIO(payload), filename="too-large.png" + ) + mock_storage = Mock() + + url, error = handle_image_upload( + Mock(files={"image": mock_file}), "old.png", "user123", mock_storage + ) + + assert url is None + assert error.status_code == 400 + mock_storage.save_file.assert_not_called() + + @pytest.mark.parametrize( + ("width", "height", "expected_status"), + [(4, 4, None), (5, 4, 400)], + ) + def test_enforces_decoded_pixel_limit( + self, flask_app, width, height, expected_status + ): + from application.api.user.base import handle_image_upload + + payload = self._image_bytes(width, height) + with patch( + "application.api.user.base.settings.AGENT_IMAGE_MAX_PIXELS", 16 + ), flask_app.app_context(): + mock_file = FileStorage( + stream=io.BytesIO(payload), filename="dimensions.png" + ) + mock_storage = Mock() + + url, error = handle_image_upload( + Mock(files={"image": mock_file}), "", "user123", mock_storage + ) + + if expected_status is None: + assert error is None + assert url + mock_storage.save_file.assert_called_once() + else: + assert url is None + assert error.status_code == expected_status + mock_storage.save_file.assert_not_called() + + def test_rejects_image_whose_content_does_not_match_extension( + self, flask_app + ): + from application.api.user.base import handle_image_upload + + with flask_app.app_context(): + mock_file = FileStorage( + stream=io.BytesIO(self._png_bytes()), filename="disguised.jpg" + ) + mock_storage = Mock() + + url, error = handle_image_upload( + Mock(files={"image": mock_file}), "", "user123", mock_storage + ) + + assert url is None + assert error.status_code == 400 + mock_storage.save_file.assert_not_called() + + def test_sanitizes_user_directory_component(self, flask_app): + from application.api.user.base import handle_image_upload + + with flask_app.app_context(): + mock_file = FileStorage( + stream=io.BytesIO(self._png_bytes()), filename="test.png" + ) + mock_request = Mock(files={"image": mock_file}) + mock_storage = Mock() + + url, error = handle_image_upload( + mock_request, "", "../../secrets", mock_storage + ) + + assert error is None + assert ".." not in url + assert url.startswith("inputs/secrets-") + assert "/attachments/" in url + @pytest.mark.unit class TestRequireAgentDecorator: @@ -321,4 +482,3 @@ class TestRequireAgentPgConn: with _patch_base_db(pg_conn), flask_app.app_context(): result = fn(webhook_token="webhook-123") assert result["got"] == str(agent["id"]) - diff --git a/tests/core/test_secret_key.py b/tests/core/test_secret_key.py new file mode 100644 index 00000000..75195cde --- /dev/null +++ b/tests/core/test_secret_key.py @@ -0,0 +1,36 @@ +import stat + +import pytest + + +def test_configured_secret_is_used_without_touching_the_filesystem(tmp_path): + from application.core.secret_key import resolve_jwt_secret_key + + key_file = tmp_path / "missing" / "jwt-secret" + + assert ( + resolve_jwt_secret_key("configured-secret", None, key_file) + == "configured-secret" + ) + assert not key_file.exists() + + +def test_cloud_deployment_requires_an_explicit_shared_secret(tmp_path): + from application.core.secret_key import resolve_jwt_secret_key + + with pytest.raises(RuntimeError, match="JWT_SECRET_KEY must be set"): + resolve_jwt_secret_key("", "cloud", tmp_path / "jwt-secret") + + +def test_local_secret_is_created_once_with_owner_only_permissions(tmp_path): + from application.core.secret_key import resolve_jwt_secret_key + + key_file = tmp_path / "jwt-secret" + + first = resolve_jwt_secret_key("", None, key_file) + second = resolve_jwt_secret_key("", None, key_file) + + assert second == first + assert key_file.read_text(encoding="utf-8") == first + assert len(first) == 64 + assert stat.S_IMODE(key_file.stat().st_mode) == 0o600 diff --git a/tests/e2e/specs/tier-b/images.spec.ts b/tests/e2e/specs/tier-b/images.spec.ts index 6c09e59d..8eaa1669 100644 --- a/tests/e2e/specs/tier-b/images.spec.ts +++ b/tests/e2e/specs/tier-b/images.spec.ts @@ -1,34 +1,6 @@ -// Tier-B · B17 image serve endpoint. -/** - * Exercises `GET /api/images/` in - * `application/api/user/attachments/routes.py` — the static-ish endpoint - * that fronts `storage.get_file()` for user-uploaded agent images and - * anything else the UI references via `generate_image_url(...)`. The route - * is not authenticated (no `@token_required` wrapping) by design — the - * image URLs are shared publicly e.g. in the agents marketplace — so we - * don't set up a `newUserContext` here, unlike every other Tier-B spec. - * - * Storage contract gotcha: `LocalStorage.__init__` sets `base_dir` to the - * repo root (three `os.path.dirname()` calls up from - * `application/storage/local.py`). The e2e env sets `UPLOAD_FOLDER=.e2e-tmp/inputs` - * but that is the directory the upload handlers write INTO; it is not the - * storage base. So an image request for `/api/images/foo/bar.png` resolves - * to `/foo/bar.png`. The tests below write their fixture files - * under `/.e2e-tmp/inputs/e2e-images/` and request - * `/api/images/.e2e-tmp/inputs/e2e-images/` — the `.` prefix is - * NOT `..` so it passes the route's literal-substring guard. - * - * Covered: - * 1. served file: write a small PNG under storage, GET returns bytes + - * correct Content-Type + Cache-Control header. - * 2. path traversal: `/api/images/../../etc/passwd` — rejected 400 before - * hitting storage. Also URL-encoded %2E%2E variant. - * 3. missing file: GET against a known-absent path → 404. - * 4. non-image extensions serve with a predictable content-type (the - * route trusts the extension and returns `image/`). - * 5. jpg alias: extension `.jpg` is served as `image/jpeg` (not `image/jpg`). - */ +/** Tier-B · public agent-image capability endpoint. */ +import { createHmac } from 'node:crypto'; import { mkdir, rm, writeFile } from 'node:fs/promises'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -36,146 +8,139 @@ import { fileURLToPath } from 'node:url'; import * as playwright from '@playwright/test'; const { expect, test } = playwright; +import { pg } from '../../helpers/db.js'; +import { resetDb } from '../../helpers/reset.js'; + const HERE = dirname(fileURLToPath(import.meta.url)); -// Repo root = tests/e2e/specs/tier-b/ → up 4 = repo root. const REPO_ROOT = resolve(HERE, '..', '..', '..', '..'); -// Scratch dir we own — writes go here, TRUNCATE doesn't touch the filesystem. -const IMAGES_DIR = resolve(REPO_ROOT, '.e2e-tmp', 'inputs', 'e2e-images'); -// Path used in URLs — relative-to-repo-root, matches what `storage.get_file` -// will resolve via its `base_dir` join. -const IMAGES_URL_PREFIX = '.e2e-tmp/inputs/e2e-images'; - +const IMAGE_USER = 'e2e-images'; +const IMAGES_DIR = resolve( + REPO_ROOT, + '.e2e-tmp', + 'inputs', + IMAGE_USER, + 'attachments', +); +const STORAGE_PREFIX = `.e2e-tmp/inputs/${IMAGE_USER}/attachments`; const API_URL = process.env.API_URL ?? 'http://127.0.0.1:7099'; +const IMAGE_SECRET = + process.env.JWT_SECRET_KEY ?? 'e2e-fixed-secret-never-use-in-prod'; -// Minimal valid PNG — 1x1 transparent pixel. Borrowed from the commonly- -// reproduced public-domain snippet; it's a real PNG that decoders accept. -// Using a real PNG (vs. a fake 8-byte header) means a future test that -// actually tries to render it won't break. -const TINY_PNG_BASE64 = - 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII='; +const TINY_PNG = Buffer.from( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=', + 'base64', +); -async function seedImage( - relName: string, - data: Buffer = Buffer.from(TINY_PNG_BASE64, 'base64'), -): Promise { - await mkdir(IMAGES_DIR, { recursive: true }); - const fullPath = resolve(IMAGES_DIR, relName); - await writeFile(fullPath, data); - return `${IMAGES_URL_PREFIX}/${relName}`; +function imageCapability( + agentId: string, + imagePath: string, + userId = IMAGE_USER, +): string { + const payload = `docsgpt-agent-image-v1\0${agentId}\0${userId}\0${imagePath}`; + return createHmac('sha256', IMAGE_SECRET).update(payload).digest('hex'); } -test.describe('tier-b · images (public image serve)', () => { - // Filesystem is shared across tests in this file; each test writes under - // a unique filename to avoid collision. After the suite we clean the dir. +async function seedAgentImage( + filename: string, + data: Buffer = TINY_PNG, +): Promise<{ agentId: string; imagePath: string; url: string }> { + await mkdir(IMAGES_DIR, { recursive: true }); + await writeFile(resolve(IMAGES_DIR, filename), data); + const imagePath = `${STORAGE_PREFIX}/${filename}`; + const { rows } = await pg.query<{ id: string }>( + `INSERT INTO agents (user_id, name, status, image) + VALUES ($1, 'image-capability-e2e', 'draft', $2) + RETURNING id::text AS id`, + [IMAGE_USER, imagePath], + ); + const agentId = rows[0]?.id; + if (!agentId) throw new Error('Failed to seed image agent'); + const capability = imageCapability(agentId, imagePath); + return { + agentId, + imagePath, + url: `/api/images/${agentId}/${capability}`, + }; +} + +test.describe('tier-b · agent image capabilities', () => { + test.beforeEach(async () => { + await resetDb(); + }); + test.afterAll(async () => { - await rm(IMAGES_DIR, { recursive: true, force: true }); + await rm(resolve(IMAGES_DIR, '..'), { recursive: true, force: true }); }); - test('serves a seeded PNG with correct content-type and cache header', async () => { + test('valid capability serves exact PNG bytes with hardened headers', async () => { const api = await playwright.request.newContext({ baseURL: API_URL }); try { - const imagePath = await seedImage('served.png'); - const res = await api.get(`/api/images/${imagePath}`); + const seeded = await seedAgentImage('served.png'); + const res = await api.get(seeded.url); expect(res.status()).toBe(200); - - const headers = res.headers(); - expect(headers['content-type']).toBe('image/png'); - - // The route sets `Cache-Control: max-age=86400`. Spec asserts presence - // (don't pin the specific value — a future 1h/1d swap shouldn't fail). - const cacheControl = headers['cache-control']; - expect(cacheControl, 'Cache-Control header should be present').toBeTruthy(); - expect(cacheControl).toMatch(/max-age=\d+/); - - // Body is the exact bytes we wrote. - const expected = Buffer.from(TINY_PNG_BASE64, 'base64'); - const body = await res.body(); - expect(body.equals(expected)).toBe(true); + expect(res.headers()['content-type']).toBe('image/png'); + expect(res.headers()['cache-control']).toMatch(/max-age=\d+/); + expect(res.headers()['x-content-type-options']).toBe('nosniff'); + expect((await res.body()).equals(TINY_PNG)).toBe(true); } finally { await api.dispose(); } }); - test('path traversal rejected (literal ..) → 400', async () => { - // Playwright's APIRequestContext resolves URLs through the WHATWG URL - // parser which collapses `..` segments before sending. To actually hit - // Flask with `..` in the path we must URL-encode the dots so Playwright - // passes them through unmodified — Werkzeug decodes them back to `..` - // and the route's substring guard then triggers 400. + test('raw storage paths are no longer accepted', async () => { const api = await playwright.request.newContext({ baseURL: API_URL }); try { - const encoded = await api.get( - '/api/images/%2e%2e%2f%2e%2e%2fetc%2fpasswd', - ); - expect(encoded.status()).toBe(400); - const body = (await encoded.json()) as { success: boolean; message?: string }; - expect(body.success).toBe(false); - expect(body.message ?? '').toMatch(/invalid/i); - - // Also the mixed-path traversal case where the literal `..` sits - // inside the trailing path (no leading-slash collapse because of the - // non-dot first segment). Encoded so Playwright can't normalise it away. - const encoded2 = await api.get( - '/api/images/e2e-images/%2e%2e%2fbadfile', - ); - expect(encoded2.status()).toBe(400); - } finally { - await api.dispose(); - } - }); - - test('missing file returns 404', async () => { - const api = await playwright.request.newContext({ baseURL: API_URL }); - try { - // Ensure the directory exists (so the error is truly "file not found", - // not a directory-missing surfaced as FileNotFoundError from a missing - // intermediate segment). - await mkdir(IMAGES_DIR, { recursive: true }); - const res = await api.get( - `/api/images/${IMAGES_URL_PREFIX}/definitely-not-here-${Date.now()}.png`, - ); + const res = await api.get('/api/images/.env'); expect(res.status()).toBe(404); - const body = (await res.json()) as { success: boolean; message?: string }; - expect(body.success).toBe(false); } finally { await api.dispose(); } }); - test('non-image extension is served with image/ content-type (route trusts the extension)', async () => { - // The handler does not inspect bytes — it splits the path by `.` and - // returns `image/`. This is a deliberate contract (legacy agents - // may reference `.webp`, `.gif`, etc.) but a weird one: `.txt` would - // also be served as `image/txt`. Pin that behaviour so a future MIME- - // sniffer refactor trips this test loudly rather than silently changing - // what browsers see. + test('forged capability is rejected before storage access', async () => { const api = await playwright.request.newContext({ baseURL: API_URL }); try { - const imagePath = await seedImage( - 'not-really.webp', - Buffer.from('RIFFxxxxWEBP', 'utf8'), + const seeded = await seedAgentImage('forged.png'); + const res = await api.get(`/api/images/${seeded.agentId}/${'0'.repeat(64)}`); + expect(res.status()).toBe(404); + } finally { + await api.dispose(); + } + }); + + test('even a signed poisoned database path is rejected', async () => { + const api = await playwright.request.newContext({ baseURL: API_URL }); + try { + const { rows } = await pg.query<{ id: string }>( + `INSERT INTO agents (user_id, name, status, image) + VALUES ($1, 'poisoned-image-e2e', 'draft', '.env') + RETURNING id::text AS id`, + [IMAGE_USER], ); - const res = await api.get(`/api/images/${imagePath}`); - expect(res.status()).toBe(200); - expect(res.headers()['content-type']).toBe('image/webp'); + const agentId = rows[0]?.id; + if (!agentId) throw new Error('Failed to seed poisoned image agent'); + const capability = imageCapability(agentId, '.env'); + const res = await api.get(`/api/images/${agentId}/${capability}`); + expect(res.status()).toBe(404); } finally { await api.dispose(); } }); - test('jpg extension is aliased to image/jpeg', async () => { + test('jpg uses image/jpeg while non-image extensions fail closed', async () => { const api = await playwright.request.newContext({ baseURL: API_URL }); try { - const imagePath = await seedImage( + const jpg = await seedAgentImage( 'aliased.jpg', - Buffer.from('not-a-real-jpeg-just-bytes', 'utf8'), + Buffer.from([0xff, 0xd8, 0xff, 0xe0]), ); - const res = await api.get(`/api/images/${imagePath}`); - expect(res.status()).toBe(200); - // The route explicitly branches `jpg -> image/jpeg` — verifying here - // so a refactor to a generic mime-type map can't quietly regress to - // `image/jpg` (which Safari rejects). - expect(res.headers()['content-type']).toBe('image/jpeg'); + const jpgRes = await api.get(jpg.url); + expect(jpgRes.status()).toBe(200); + expect(jpgRes.headers()['content-type']).toBe('image/jpeg'); + + const text = await seedAgentImage('not-an-image.txt', Buffer.from('secret')); + const textRes = await api.get(text.url); + expect(textRes.status()).toBe(404); } finally { await api.dispose(); } diff --git a/tests/storage/db/repositories/test_agents.py b/tests/storage/db/repositories/test_agents.py index 394e5364..4a20c6b5 100644 --- a/tests/storage/db/repositories/test_agents.py +++ b/tests/storage/db/repositories/test_agents.py @@ -71,6 +71,18 @@ class TestGet: created = repo.create("user-1", "a", "draft") assert repo.get(created["id"], "user-other") is None + def test_find_image_record_returns_only_capability_fields(self, pg_conn): + repo = _repo(pg_conn) + created = repo.create( + "user-1", "a", "draft", image="inputs/user-1/attachments/a.png" + ) + + image_record = repo.find_image_record(str(created["id"])) + + assert set(image_record) >= {"id", "user_id", "image"} + assert image_record["image"] == "inputs/user-1/attachments/a.png" + assert "name" not in image_record + def test_get_by_legacy_id(self, pg_conn): repo = _repo(pg_conn) created = repo.create( diff --git a/tests/storage/test_local_storage.py b/tests/storage/test_local_storage.py index 196b0621..0c68de0e 100644 --- a/tests/storage/test_local_storage.py +++ b/tests/storage/test_local_storage.py @@ -123,6 +123,23 @@ class TestLocalStorageGetFile: ) +@pytest.mark.unit +class TestLocalStorageFileSize: + + @patch("os.path.getsize", return_value=42) + def test_get_file_size_uses_metadata_without_opening( + self, mock_getsize, local_storage + ): + assert local_storage.get_file_size("avatars/test.png") == 42 + + expected_path = os.path.join( + os.path.realpath("/tmp/test_storage"), "avatars/test.png" + ) + assert os.path.normpath(mock_getsize.call_args[0][0]) == os.path.normpath( + expected_path + ) + + @pytest.mark.unit class TestLocalStorageDeleteFile: diff --git a/tests/storage/test_s3_storage.py b/tests/storage/test_s3_storage.py index aed2a9b2..e13ca805 100644 --- a/tests/storage/test_s3_storage.py +++ b/tests/storage/test_s3_storage.py @@ -242,6 +242,60 @@ class TestS3StorageGetFile: s3_storage.get_file(path) +class TestS3StorageFileSize: + """Test metadata-only size lookup used by bounded public delivery.""" + + @pytest.mark.unit + def test_get_file_size_uses_head_without_downloading( + self, s3_storage, mock_boto3_client + ): + mock_boto3_client.head_object.return_value = {"ContentLength": 1234} + + assert s3_storage.get_file_size("avatars/test.png") == 1234 + + mock_boto3_client.head_object.assert_called_once_with( + Bucket="test-bucket", Key="avatars/test.png" + ) + mock_boto3_client.download_fileobj.assert_not_called() + + @pytest.mark.unit + def test_get_file_size_maps_missing_object_to_file_not_found( + self, s3_storage, mock_boto3_client + ): + mock_boto3_client.head_object.side_effect = ClientError( + {"Error": {"Code": "NoSuchKey", "Message": "Not found"}}, + "head_object", + ) + + with pytest.raises(FileNotFoundError, match="File not found"): + s3_storage.get_file_size("avatars/missing.png") + + +class TestS3StoragePresignedUrl: + """Test response metadata on short-lived direct avatar delivery URLs.""" + + @pytest.mark.unit + def test_generate_presigned_url_can_override_content_type( + self, s3_storage, mock_boto3_client + ): + mock_boto3_client.generate_presigned_url.return_value = "https://signed" + + result = s3_storage.generate_presigned_url( + "avatars/test.png", expires_in=300, content_type="image/png" + ) + + assert result == "https://signed" + mock_boto3_client.generate_presigned_url.assert_called_once_with( + "get_object", + Params={ + "Bucket": "test-bucket", + "Key": "avatars/test.png", + "ResponseContentType": "image/png", + }, + ExpiresIn=300, + ) + + class TestS3StorageDeleteFile: """Test file deletion functionality.""" diff --git a/tests/test_utils.py b/tests/test_utils.py index b8e15048..b4c89de7 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -317,42 +317,54 @@ class TestGenerateImageUrl: assert generate_image_url("http://example.com/img.png") == "http://example.com/img.png" @pytest.mark.unit - def test_s3_strategy(self): + def test_internal_image_uses_opaque_capability(self): with patch("application.utils.settings") as s: - s.URL_STRATEGY = "s3" - s.S3_BUCKET_NAME = "my-bucket" - s.S3_ENDPOINT_URL = None - s.S3_REGION = "us-west-2" - result = generate_image_url("path/to/img.png") - assert result == "https://my-bucket.s3.us-west-2.amazonaws.com/path/to/img.png" + s.JWT_SECRET_KEY = "test-image-secret" + s.UPLOAD_FOLDER = "inputs" + s.API_URL = "https://api.example.com" + result = generate_image_url( + "inputs/user-1/attachments/avatar.png", + "00000000-0000-0000-0000-000000000001", + "user-1", + ) + assert result.startswith( + "https://api.example.com/api/images/" + "00000000-0000-0000-0000-000000000001/" + ) + assert "inputs" not in result + assert "avatar.png" not in result @pytest.mark.unit - def test_s3_strategy_custom_endpoint_path_style(self): + def test_internal_image_fails_closed_without_agent_identity(self): with patch("application.utils.settings") as s: - s.URL_STRATEGY = "s3" - s.S3_BUCKET_NAME = "my-bucket" - s.S3_ENDPOINT_URL = "https://account.r2.cloudflarestorage.com" - s.S3_PATH_STYLE = True - result = generate_image_url("path/to/img.png") - assert result == "https://account.r2.cloudflarestorage.com/my-bucket/path/to/img.png" + s.JWT_SECRET_KEY = "test-image-secret" + s.UPLOAD_FOLDER = "inputs" + assert generate_image_url("inputs/user-1/attachments/avatar.png") == "" @pytest.mark.unit - def test_s3_strategy_custom_endpoint_virtual_host(self): + def test_internal_image_fails_closed_without_secret(self): with patch("application.utils.settings") as s: - s.URL_STRATEGY = "s3" - s.S3_BUCKET_NAME = "my-bucket" - s.S3_ENDPOINT_URL = "https://minio.example.com" - s.S3_PATH_STYLE = False - result = generate_image_url("path/to/img.png") - assert result == "https://my-bucket.minio.example.com/path/to/img.png" + s.JWT_SECRET_KEY = "" + s.UPLOAD_FOLDER = "inputs" + result = generate_image_url( + "inputs/user-1/attachments/avatar.png", + "00000000-0000-0000-0000-000000000001", + "user-1", + ) + assert result == "" @pytest.mark.unit - def test_backend_strategy(self): + def test_internal_image_rejects_path_outside_owner_uploads(self): with patch("application.utils.settings") as s: - s.URL_STRATEGY = "backend" + s.JWT_SECRET_KEY = "test-image-secret" + s.UPLOAD_FOLDER = "inputs" s.API_URL = "http://localhost:7091" - result = generate_image_url("path/to/img.png") - assert result == "http://localhost:7091/api/images/path/to/img.png" + result = generate_image_url( + ".env", + "00000000-0000-0000-0000-000000000001", + "user-1", + ) + assert result == "" class TestCalculateCompressionThreshold: @@ -641,17 +653,20 @@ class TestGenerateImageUrlEdgeCases: @pytest.mark.unit def test_non_string_input(self): result = generate_image_url(123) - # Not a string, not starting with http, uses default strategy - assert "/api/images/" in result or "s3" in result + assert result == "" @pytest.mark.unit - def test_default_strategy_is_backend(self): + def test_email_owner_legacy_path_is_supported(self): with patch("application.utils.settings") as s: - # Simulate missing URL_STRATEGY attribute - del s.URL_STRATEGY + s.JWT_SECRET_KEY = "test-image-secret" + s.UPLOAD_FOLDER = "inputs" s.API_URL = "http://localhost:7091" - result = generate_image_url("img.png") - assert "localhost:7091" in result + result = generate_image_url( + "inputs/person@example.com/attachments/img.png", + "00000000-0000-0000-0000-000000000001", + "person@example.com", + ) + assert result.startswith("http://localhost:7091/api/images/") class TestGetHashEdgeCases: