feat: cleanup on library dir creation and upload endpoints (#3069)

lnbits/app.py

@@ -163,13 +163,13 @@ def create_app() -> FastAPI:
     core_app_extra.register_new_ratelimiter = register_new_ratelimiter(app)
 
     # register static files
-    static_path = Path("lnbits", "static")
+    app.mount("/static", StaticFiles(directory=Path("lnbits", "static")), name="static")
-    static = StaticFiles(directory=static_path)
+    Path(settings.lnbits_data_folder, "images").mkdir(parents=True, exist_ok=True)
-    app.mount("/static", static, name="static")
+    app.mount(
-
+        "/library",
-    images_path = os.path.abspath(os.path.join(settings.lnbits_data_folder, "images"))
+        StaticFiles(directory=Path(settings.lnbits_data_folder, "images")),
-    os.makedirs(images_path, exist_ok=True)
+        name="library",
-    app.mount("/library", StaticFiles(directory=images_path), name="library")
+    )
 
     g().base_url = f"http://{settings.host}:{settings.port}"
 

lnbits/core/models/misc.py

@@ -46,3 +46,8 @@ class SimpleItem(BaseModel):
 class DbVersion(BaseModel):
     db: str
     version: int
+
+
+class Image(BaseModel):
+    filename: str
+    directory: str = "library"

lnbits/core/templates/admin/_tab_library.html

@@ -31,7 +31,12 @@
           class="q-pt-md q-pb-md row items-center justify-between"
         >
           <small
-            ><div class="text-caption ellipsis" v-text="image.filename"></div
+            ><div
+              class="text-caption ellipsis"
+              style="max-width: 100px"
+              :title="image.filename"
+              v-text="image.filename"
+            ></div
           ></small>
           <q-btn
             dense
@@ -43,11 +48,13 @@
             ><q-tooltip>Copy image link</q-tooltip></q-btn
           >
           <q-btn
-            color="negative"
+            dense
-            icon="delete"
             flat
             size="sm"
+            icon="delete"
+            color="negative"
             @click="deleteImage(image.filename)"
+            :title="$t('delete')"
             ><q-tooltip>Delete image</q-tooltip></q-btn
           >
         </q-card-section>

lnbits/core/views/admin_api.py

@@ -1,19 +1,19 @@
-import glob
-import imghdr
 import os
 import time
 from http import HTTPStatus
-from io import BytesIO
+from pathlib import Path
-from shutil import make_archive
+from shutil import make_archive, move
 from subprocess import Popen
-from typing import Optional
+from tempfile import NamedTemporaryFile
+from typing import IO, Optional
 from urllib.parse import urlparse
 
-import shortuuid
+import filetype
-from fastapi import APIRouter, Depends, File, HTTPException, Path, UploadFile
+from fastapi import APIRouter, Depends, File, Header, HTTPException, UploadFile
 from fastapi.responses import FileResponse
 
 from lnbits.core.models import User
+from lnbits.core.models.misc import Image, SimpleStatus
 from lnbits.core.models.notifications import NotificationType
 from lnbits.core.services import (
     enqueue_notification,
@@ -23,6 +23,7 @@ from lnbits.core.services import (
 from lnbits.core.services.notifications import send_email_notification
 from lnbits.core.services.settings import dict_to_settings
 from lnbits.decorators import check_admin, check_super_user
+from lnbits.helpers import safe_upload_file_path
 from lnbits.server import server_restart
 from lnbits.settings import AdminSettings, Settings, UpdateSettings, settings
 from lnbits.tasks import invoice_listeners
@@ -171,40 +172,53 @@ async def api_download_backup() -> FileResponse:
     status_code=HTTPStatus.OK,
     dependencies=[Depends(check_admin)],
 )
-async def upload_image(file: UploadFile = file_upload):
+async def upload_image(
-    if not file or not file.filename:
+    file: UploadFile = file_upload,
-        raise HTTPException(status_code=400, detail="No file provided")
+    content_length: int = Header(..., le=settings.lnbits_upload_size_bytes),
+) -> Image:
+    if not file.filename:
+        raise HTTPException(
+            status_code=HTTPStatus.BAD_REQUEST, detail="No filename provided."
+        )
 
-    ext = file.filename.split(".")[-1].lower()
+    # validate file types
-    if ext not in {"png", "jpg", "jpeg", "gif"}:
+    file_info = filetype.guess(file.file)
-        raise HTTPException(status_code=400, detail="Unsupported file type")
+    if file_info is None:
+        raise HTTPException(
+            status_code=HTTPStatus.UNSUPPORTED_MEDIA_TYPE,
+            detail="Unable to determine file type",
+        )
+    detected_content_type = file_info.extension.lower()
+    if (
+        file.content_type not in settings.lnbits_upload_allowed_types
+        or detected_content_type not in settings.lnbits_upload_allowed_types
+    ):
+        raise HTTPException(HTTPStatus.UNSUPPORTED_MEDIA_TYPE, "Unsupported file type")
 
-    contents = BytesIO()
+    # validate file name
-    total_size = 0
+    try:
-    max_size = 500000
+        file_path = safe_upload_file_path(file.filename)
-    while chunk := await file.read(1024 * 1024):
+    except ValueError as e:
-        total_size += len(chunk)
+        raise HTTPException(
-        if total_size > max_size:
+            status_code=HTTPStatus.FORBIDDEN,
+            detail=f"The requested filename '{file.filename}' is forbidden.",
+        ) from e
+
+    # validate file size
+    real_file_size = 0
+    temp: IO = NamedTemporaryFile(delete=False)
+    for chunk in file.file:
+        real_file_size += len(chunk)
+        if real_file_size > content_length:
             raise HTTPException(
-                status_code=413, detail=f"File too large ({max_size / 1000} KB max)"
+                status_code=HTTPStatus.REQUEST_ENTITY_TOO_LARGE,
+                detail=f"File too large ({content_length / 1000} KB max)",
             )
-        contents.write(chunk)
+        temp.write(chunk)
+    temp.close()
 
-    contents.seek(0)
+    move(temp.name, file_path)
-
+    return Image(filename=file.filename)
-    kind = imghdr.what(None, h=contents.read(512))
-    if kind not in {"png", "jpeg", "gif"}:
-        raise HTTPException(status_code=400, detail="Invalid image file")
-    contents.seek(0)
-
-    filename = f"{shortuuid.uuid()[:5]}.{ext}"
-    image_folder = os.path.join(settings.lnbits_data_folder, "images")
-    file_path = os.path.join(image_folder, filename)
-
-    with open(file_path, "wb") as f:
-        f.write(contents.read())
-
-    return {"filename": filename, "url": f"{settings.lnbits_baseurl}library/{filename}"}
 
 
 @admin_router.get(
@@ -212,23 +226,13 @@ async def upload_image(file: UploadFile = file_upload):
     status_code=HTTPStatus.OK,
     dependencies=[Depends(check_admin)],
 )
-async def list_uploaded_images():
+async def list_uploaded_images() -> list[Image]:
-    image_folder = os.path.join(settings.lnbits_data_folder, "images")
+    image_folder = Path(settings.lnbits_data_folder, "images")
-    if not os.path.exists(image_folder):
+    files = image_folder.glob("*")
-        return []
-
-    files = glob.glob(os.path.join(image_folder, "*"))
     images = []
-
+    for file in files:
-    for file_path in files:
+        if file.is_file():
-        if os.path.isfile(file_path):
+            images.append(Image(filename=file.name))
-            filename = os.path.basename(file_path)
-            images.append(
-                {
-                    "filename": filename,
-                    "url": f"{settings.lnbits_baseurl}library/{filename}",
-                }
-            )
     return images
 
 
@@ -237,18 +241,17 @@ async def list_uploaded_images():
     status_code=HTTPStatus.OK,
     dependencies=[Depends(check_admin)],
 )
-async def delete_uploaded_image(
+async def delete_uploaded_image(filename: str) -> SimpleStatus:
-    filename: str = Path(..., description="Name of the image file to delete")
+    try:
-):
+        file_path = safe_upload_file_path(filename)
-    image_folder = os.path.join(settings.lnbits_data_folder, "images")
+    except ValueError as e:
-    file_path = os.path.join(image_folder, filename)
+        raise HTTPException(
+            status_code=HTTPStatus.FORBIDDEN,
+            detail=f"The requested filename '{filename}' is forbidden.",
+        ) from e
 
-    # Prevent dir traversal attack
+    if not file_path.exists():
-    if not os.path.abspath(file_path).startswith(os.path.abspath(image_folder)):
+        raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Image not found.")
-        raise HTTPException(status_code=400, detail="Invalid filename")
 
-    if not os.path.exists(file_path):
+    file_path.unlink()
-        raise HTTPException(status_code=404, detail="Image not found")
+    return SimpleStatus(success=True, message=f"{filename} deleted")
-
-    os.remove(file_path)
-    return {"status": "success", "message": f"{filename} deleted"}

lnbits/helpers.py

@@ -347,3 +347,14 @@ def path_segments(path: str) -> list[str]:
 def normalize_path(path: Optional[str]) -> str:
     path = path or ""
     return "/" + "/".join(path_segments(path))
+
+
+def safe_upload_file_path(filename: str, directory: str = "images") -> Path:
+    image_folder = Path(settings.lnbits_data_folder, directory)
+    file_path = image_folder / filename
+    # Prevent dir traversal attack
+    if image_folder.resolve() not in file_path.resolve().parents:
+        raise ValueError("Unsafe filename.")
+    # Prevent filename with subdirectories
+    file_path = image_folder / filename.split("/")[-1]
+    return file_path.resolve()

lnbits/settings.py

@@ -271,6 +271,23 @@ class ThemesSettings(LNbitsSettings):
 class OpsSettings(LNbitsSettings):
     lnbits_baseurl: str = Field(default="http://127.0.0.1:5000/")
     lnbits_hide_api: bool = Field(default=False)
+    lnbits_upload_size_bytes: int = Field(default=512_000, ge=0)  # 500kb
+    lnbits_upload_allowed_types: list[str] = Field(
+        default=[
+            "image/png",
+            "image/jpeg",
+            "image/jpg",
+            "image/heic",
+            "image/heif",
+            "image/heics",
+            "png",
+            "jpeg",
+            "jpg",
+            "heic",
+            "heif",
+            "heics",
+        ]
+    )
 
 
 class FeeSettings(LNbitsSettings):

lnbits/static/js/admin.js

@@ -560,59 +560,57 @@ window.AdminPageLogic = {
         this.uploadImage(file)
       }
     },
-    async uploadImage(file) {
+    uploadImage(file) {
       const formData = new FormData()
       formData.append('file', file)
-
+      LNbits.api
-      try {
+        .request(
-        const response = await LNbits.api.request(
           'POST',
           '/admin/api/v1/images',
           this.g.user.wallets[0].adminkey,
           formData,
           {headers: {'Content-Type': 'multipart/form-data'}}
         )
-        this.$q.notify({
+        .then(() => {
-          type: 'positive',
+          this.$q.notify({
-          message: 'Image uploaded!',
+            type: 'positive',
-          icon: null
+            message: 'Image uploaded!',
+            icon: null
+          })
+          this.getUploadedImages()
         })
-        await this.getUploadedImages()
+        .catch(LNbits.utils.notifyApiError)
-      } catch (error) {
-        LNbits.utils.notifyApiError(error)
-      }
     },
-    async getUploadedImages() {
+    getUploadedImages() {
-      try {
+      LNbits.api
-        const response = await LNbits.api.request(
+        .request('GET', '/admin/api/v1/images', this.g.user.wallets[0].inkey)
-          'GET',
+        .then(response => {
-          '/admin/api/v1/images',
+          this.library_images = response.data.map(image => ({
-          this.g.user.wallets[0].inkey
+            ...image,
-        )
+            url: `${window.origin}/${image.directory}/${image.filename}`
-        this.library_images = response.data
+          }))
-      } catch (error) {
+        })
-        LNbits.utils.notifyApiError(error)
+        .catch(LNbits.utils.notifyApiError)
-      }
     },
-    async deleteImage(filename) {
+    deleteImage(filename) {
       LNbits.utils
         .confirmDialog('Are you sure you want to delete this image?')
-        .onOk(async () => {
+        .onOk(() => {
-          try {
+          LNbits.api
-            await LNbits.api.request(
+            .request(
               'DELETE',
               `/admin/api/v1/images/${filename}`,
               this.g.user.wallets[0].adminkey
             )
-            this.$q.notify({
+            .then(() => {
-              type: 'positive',
+              this.$q.notify({
-              message: 'Image deleted!',
+                type: 'positive',
-              icon: null
+                message: 'Image deleted!',
+                icon: null
+              })
+              this.getUploadedImages()
             })
-            await this.getUploadedImages()
+            .catch(LNbits.utils.notifyApiError)
-          } catch (error) {
-            LNbits.utils.notifyApiError(error)
-          }
         })
     },
     downloadBackup() {

poetry.lock

@@ -1083,6 +1083,17 @@ docs = ["furo (>=2023.9.10)", "sphinx (>=7.2.6)", "sphinx-autodoc-typehints (>=1
 testing = ["covdefaults (>=2.3)", "coverage (>=7.3.2)", "diff-cover (>=8)", "pytest (>=7.4.3)", "pytest-cov (>=4.1)", "pytest-mock (>=3.12)", "pytest-timeout (>=2.2)"]
 typing = ["typing-extensions (>=4.8)"]
 
+[[package]]
+name = "filetype"
+version = "1.2.0"
+description = "Infer file type and MIME type of any file/buffer. No external dependencies."
+optional = false
+python-versions = "*"
+files = [
+    {file = "filetype-1.2.0-py2.py3-none-any.whl", hash = "sha256:7ce71b6880181241cf7ac8697a2f1eb6a8bd9b429f7ad6d27b8db9ba5f1c2d25"},
+    {file = "filetype-1.2.0.tar.gz", hash = "sha256:66b56cd6474bf41d8c54660347d37afcc3f7d1970648de365c102ef77548aadb"},
+]
+
 [[package]]
 name = "greenlet"
 version = "3.0.3"
@@ -3403,4 +3414,4 @@ liquid = ["wallycore"]
 [metadata]
 lock-version = "2.0"
 python-versions = "~3.12 | ~3.11 | ~3.10"
-content-hash = "96dd180aaa4fbfeb34fa6f9647c8684fce183a72b1b41d22101a9dd4b962fa2e"
+content-hash = "f56154a228bfd11ca92c1818dd2b7d71ff67b218225103f4b701e6523ba499ed"

pyproject.toml

@@ -63,6 +63,8 @@ breez-sdk = {version = "0.6.6", optional = true}
 jsonpath-ng = "^1.7.0"
 pynostr = "^0.6.2"
 python-multipart = "^0.0.20"
+filetype = "^1.2.0"
+
 [tool.poetry.extras]
 breez = ["breez-sdk"]
 liquid = ["wallycore"]
@@ -145,6 +147,7 @@ module = [
   "fastapi_sso.sso.*",
   "json5.*",
   "jsonpath_ng.*",
+  "filetype.*",
 ]
 ignore_missing_imports = "True"
 

tests/unit/test_upload_file_path.py

@@ -0,0 +1,42 @@
+from pathlib import Path
+
+import pytest
+
+from lnbits.helpers import safe_upload_file_path
+from lnbits.settings import settings
+
+
+@pytest.mark.parametrize(
+    "filepath",
+    [
+        "test.txt",
+        "test/test.txt",
+        "test/test/test.txt",
+        "test/../test.txt",
+        "*/test.txt",
+        "test/**/test.txt",
+        "./test.txt",
+    ],
+)
+def test_safe_upload_file_path(filepath: str):
+    safe_path = safe_upload_file_path(filepath)
+    assert safe_path.name == "test.txt"
+
+    # check if subdirectories got removed
+    images_folder = Path(settings.lnbits_data_folder) / "images"
+    assert images_folder.resolve() / "test.txt" == safe_path
+
+
+@pytest.mark.parametrize(
+    "filepath",
+    [
+        "../test.txt",
+        "test/../../test.txt",
+        "../../test.txt",
+        "test/../../../test.txt",
+        "../../../test.txt",
+    ],
+)
+def test_unsafe_upload_file_path(filepath: str):
+    with pytest.raises(ValueError):
+        safe_upload_file_path(filepath)