security: harden path-traversal class across routes + services; fifth CI backstop

Two attacker-controlled strings were being joined to library_dir with no
  resolve + containment check in the project ZIP import endpoint:

    - linked_folders[*].name from the request's project.json
    - per-entry zf.namelist() paths from the ZIP itself

  An absolute path in either field collapsed the join (Path("/lib") / "/etc"
  becomes Path("/etc") because pathlib discards the left side when the right
  is absolute) and the next write_bytes landed wherever the attacker chose.

  Adjacent finding from the routes audit: GET /archives/{id}/photos/{filename}
  had NO validation on filename and FileResponse-served arbitrary paths -
  the DELETE counterpart at least gated on the photos membership check.

  Adjacent finding from the services audit: ArchiveService.attach_timelapse
  wrote archive_dir / filename where filename ultimately came from a printer's
  FTP listing (compromised-printer threat model) or the /timelapse/select
  query param. A malicious printer that exposes a directory entry with ..
  segments could write the timelapse outside the archive directory.

  New backend/app/utils/safe_path.py::safe_join_under(parent, *parts) is the
  single source of truth: rejects empty / null-byte / absolute parts up-front,
  joins under parent, resolves both sides, asserts is_relative_to. Returns the
  resolved canonical path on success, raises HTTPException(400) on escape, or
  PathTraversalError when http=False (for service-layer callers that need to
  match a non-HTTP return contract).

  Wired into the import vectors, both archive photo handlers, and the
  attach_timelapse service. The full audit sweep inspected every Path/Name
  join in backend/app/api/routes/ AND backend/app/services/ - 25 route-layer
  sites + 8 service-layer sites confirmed safe and tagged with
  # SEC-PATH-OK: <reason> so future audits trust the inline guard at a glance.

  Fifth CI backstop test_route_path_arithmetic_is_safe_joined_or_marked
  AST-walks both layers and fails the build on any <dir-like>/<bare variable>
  join that doesn't either route through safe_join_under or carry the marker.
  The services layer is in scope because it receives values verbatim from the
  routes AND from external sources Bambuddy has no control over (the printer
  FTP-listing case above).

  SECURITY.md gets a fifth rule + a fifth row in the CI test mapping table;
  the rule now names the printer FTP-listing case explicitly so future
  services-layer audits set the right expectation.

--------------

  fix(library): suppress warning storm when bulk-uploading ZIPs of empty/stub STL files

  Uploading a ZIP of stub or empty STL files (e.g. the 24-byte
  "solid test\nendsolid test" shape) produced one WARNING per file in
  stl_thumbnail.py::generate_stl_thumbnail. The warnings were technically
  correct - trimesh returns a valid Mesh with zero vertices, the safeguard
  matches, and the function returns None so the library entry is still
  created without a thumbnail - but the volume turned a successful upload
  into thousands of WARNING lines in the journal.

  Two changes:

  1. The per-file "Failed to load STL or empty mesh" message in
     stl_thumbnail.py is now logger.debug instead of logger.warning. It's
     a per-file content observation, not an actionable error; the caller
     already handles None correctly. The branch now catches the rare
     "large enough but trimesh still can't parse it" case, visible in
     debug logs without spamming production.

  2. New module constant MIN_USABLE_STL_BYTES = 200 (smallest binary STL
     with one triangle is 134B, smallest ASCII ~150B; 200 is a safe floor
     below any real STL). The three thumbnail call sites in library.py
     (extract_zip_file, single-file upload, _backfill_external_stl_thumbnails)
     pre-skip files below this size before calling generate_stl_thumbnail.
     Stubs never enter the trimesh pipeline at all.

  Behavior is unchanged for real STLs: any file >=200 bytes runs through
  the existing pipeline, MAX_VERTICES still triggers simplification at
  100k vertices for the 256x256 thumbnail render, large files still get
  thumbnails.

------------

  fix(stl-thumbnail): silence matplotlib first-import noise (writable cache + font_manager log level)

  On first STL upload, three matplotlib-internal log lines surfaced:

    WARNING [matplotlib] /opt/claude/.config/matplotlib is not a writable directory
    INFO    [matplotlib.font_manager] Failed to extract font properties from NotoColorEmoji.ttf
    INFO    [matplotlib.font_manager] generated new fontManager

  The writable-dir warning fired because Bambuddy's $HOME isn't writable for
  matplotlib's default config path; matplotlib fell back to /tmp/matplotlib-XXX
  which lost the font cache on every host reboot, so font_manager rebuilt it
  each cold start - producing another batch of INFO lines.

  Fix is two small additions in stl_thumbnail.py before the matplotlib import:

  1. New _configure_matplotlib_cache() sets MPLCONFIGDIR to
     settings.base_dir/.cache/matplotlib (mkdir if missing) so the cache
     persists across container restarts and the writable-dir warning never
     fires. Respects an externally-set MPLCONFIGDIR so operators who chose
     their own path aren't overridden. Best-effort with a debug fallback if
     settings can't be imported or the mkdir fails.

  2. logging.getLogger("matplotlib.font_manager").setLevel(WARNING) at module
     import demotes the per-font INFO scan that fires when font_manager
     builds its cache cold. Real font warnings (>= WARNING) still surface.

  3 new tests: font_manager logger at WARNING after module import;
  _configure_matplotlib_cache creates the directory under base_dir and sets
  MPLCONFIGDIR; an externally-set MPLCONFIGDIR is preserved verbatim.
  5516 backend tests green, frontend gates clean.
This commit is contained in:
maziggy 2026-06-02 12:12:54 +02:00
parent db9b20631c
commit 396e9aa09e
20 changed files with 1045 additions and 56 deletions

5
.gitignore vendored
View file

@ -62,6 +62,11 @@ node_modules/
data/
# Local-dev runtime caches (matplotlib MPLCONFIGDIR lands here when DATA_DIR
# is unset, so base_dir resolves to the repo root). In Docker this sits
# inside the /app/data volume and is already covered.
.cache/
# JWT secret file (should be in data dir, but protect project root too)
.jwt_secret

File diff suppressed because one or more lines are too long

View file

@ -147,6 +147,28 @@ The failure modes are where the vulnerabilities live. The structural
backstops above catch *categories* of regression; the negative-path
tests catch *specific* regressions in the new code.
### 5. Path joins under a trusted parent use the safe-join helper
Anywhere a Bambuddy code path joins a string from outside the function's
scope (request body, query/path param, `UploadFile.filename`, ZIP
`namelist()` entry, tarfile member, **printer FTP-listing entry**) under
a trusted directory, the join must route through
`backend.app.utils.safe_path.safe_join_under(parent, *parts)`. The helper
resolves the joined path and asserts it is a descendant of the parent —
defeating both absolute-path collapse (`Path("/a") / "/b"``Path("/b")`)
and `..` traversal.
Sites that have an inline guard (an explicit resolve + `is_relative_to`,
a basename-stripping helper like `_safe_filename`, or a pre-validated
alphanumeric filter) carry a `# SEC-PATH-OK: <reason>` marker on the
same line. CI walks **both** `backend/app/api/routes/` and
`backend/app/services/` and fails the build on any
``<dir-like> / <variable>`` join without either the helper or the
marker. The services layer is in scope because it receives values from
the routes verbatim and from external sources Bambuddy has no control
over (the compromised-printer threat model: a malicious printer can
serve crafted FTP-listing entries that flow straight into a path join).
### Where these rules live in the codebase
| Rule | Enforcement | Location |
@ -156,6 +178,7 @@ tests catch *specific* regressions in the new code.
| 2. Fail-closed in auth code | `test_no_fail_open_in_auth_modules` | `backend/tests/unit/test_no_fail_open_in_auth.py` |
| 3. No hardcoded fallback secrets | `test_no_hardcoded_secrets` | `backend/tests/unit/test_no_hardcoded_secrets.py` |
| 4. Negative-path tests required | Reviewer responsibility (no automated CI gate yet) | PR review |
| 5. Safe-join under trusted parent | `test_route_path_arithmetic_is_safe_joined_or_marked` | `backend/tests/unit/test_no_unsafe_path_joins.py` |
If you are adding a CI rule, update this table. If you are removing a
CI rule, you are removing a security backstop and the PR description

View file

@ -30,6 +30,7 @@ from backend.app.schemas.print_log import PrintLogResponse
from backend.app.schemas.slicer import SliceRequest
from backend.app.services.archive import ArchiveService
from backend.app.utils.http import build_content_disposition
from backend.app.utils.safe_path import safe_join_under
from backend.app.utils.threemf_tools import (
extract_embedded_presets_from_3mf,
extract_nozzle_mapping_from_3mf,
@ -2373,7 +2374,7 @@ async def process_timelapse(
filename = f"timelapse_{archive_id}_edited"
if not filename.endswith(".mp4"):
filename += ".mp4"
output_path = archive_dir / filename
output_path = archive_dir / filename # SEC-PATH-OK: filename alnum-filtered + .. rejected above
success = await processor.process(
output_path=output_path,
@ -2444,7 +2445,7 @@ async def upload_photo(
ext = Path(file.filename).suffix.lower()
photo_filename = f"{uuid.uuid4().hex[:8]}{ext}"
photo_path = photos_dir / photo_filename
photo_path = photos_dir / photo_filename # SEC-PATH-OK: photo_filename = uuid.uuid4().hex[:8] + ext
# Save file
content = await file.read()
@ -2477,8 +2478,20 @@ async def get_photo(
if not archive:
raise HTTPException(404, "Archive not found")
# Membership check first — UUID-generated names on upload mean any URL
# filename that doesn't appear here is by definition not a real photo.
# Mirrors the delete handler below; previously this endpoint had no
# membership check at all and joined `filename` straight to disk.
if not archive.photos or filename not in archive.photos:
raise HTTPException(404, "Photo not found")
archive_dir = settings.base_dir / Path(archive.file_path).parent
photo_path = archive_dir / "photos" / filename
photos_dir = archive_dir / "photos"
# Defence-in-depth: even though the membership check above already
# constrains `filename` to UUID-generated names from upload, the
# resolve + containment check guards against future code paths that
# might populate `archive.photos` from a less-trusted source.
photo_path = safe_join_under(photos_dir, filename)
if not photo_path.exists():
raise HTTPException(404, "Photo not found")
@ -2512,9 +2525,10 @@ async def delete_photo(
if not archive.photos or filename not in archive.photos:
raise HTTPException(404, "Photo not found")
# Delete file
# Delete file — same defence-in-depth as get_photo above.
archive_dir = settings.base_dir / Path(archive.file_path).parent
photo_path = archive_dir / "photos" / filename
photos_dir = archive_dir / "photos"
photo_path = safe_join_under(photos_dir, filename)
if photo_path.exists():
photo_path.unlink()
@ -2960,7 +2974,9 @@ async def upload_archive(
# Save uploaded file temporarily — strip directory components to prevent path traversal
safe_filename = _safe_filename(file.filename)
temp_path = settings.archive_dir / "temp" / safe_filename
temp_path = (
settings.archive_dir / "temp" / safe_filename
) # SEC-PATH-OK: safe_filename = _safe_filename(...) basename-stripped above
temp_path.parent.mkdir(parents=True, exist_ok=True)
try:
@ -3008,7 +3024,9 @@ async def upload_archives_bulk(
continue
safe_filename = _safe_filename(file.filename)
temp_path = settings.archive_dir / "temp" / safe_filename
temp_path = (
settings.archive_dir / "temp" / safe_filename
) # SEC-PATH-OK: safe_filename = _safe_filename(...) basename-stripped above
temp_path.parent.mkdir(parents=True, exist_ok=True)
try:
@ -3639,7 +3657,9 @@ async def slice_archive(
detail="Archive has no source file to slice",
)
src_path = Path(settings.base_dir) / src_relative
src_path = (
Path(settings.base_dir) / src_relative
) # SEC-PATH-OK: src_relative is archive.source_3mf_path from DB, set by _resolve_source_3mf_path which already does resolve+relative_to containment
if not src_path.exists():
raise HTTPException(status_code=404, detail="Archive source file missing on disk")
@ -3947,7 +3967,9 @@ def _resolve_source_3mf_path(archive: PrintArchive, source_filename: str) -> Pat
) from exc
source_dir.mkdir(parents=True, exist_ok=True)
return source_dir / source_filename
return (
source_dir / source_filename
) # SEC-PATH-OK: callers pass _safe_filename(...) basename-stripped; source_dir resolve+relative_to checked above
@router.post("/{archive_id}/source")
@ -4262,7 +4284,7 @@ async def upload_f3d(
# Save the F3D file - preserve original filename, strip directory components
f3d_filename = _safe_filename(file.filename)
f3d_path = f3d_dir / f3d_filename
f3d_path = f3d_dir / f3d_filename # SEC-PATH-OK: f3d_filename = _safe_filename(...) basename-stripped above
content = await file.read()
f3d_path.write_bytes(content)

View file

@ -63,7 +63,7 @@ from backend.app.schemas.library import (
)
from backend.app.schemas.slicer import SliceRequest, SliceResponse
from backend.app.services.archive import ThreeMFParser
from backend.app.services.stl_thumbnail import generate_stl_thumbnail
from backend.app.services.stl_thumbnail import MIN_USABLE_STL_BYTES, generate_stl_thumbnail
from backend.app.utils.filename import InvalidFilenameError, validate_print_filename
from backend.app.utils.threemf_tools import (
extract_embedded_presets_from_3mf,
@ -219,7 +219,7 @@ def _resolve_upload_destination(target_folder: LibraryFolder | None, filename: s
)
# Guard against path-traversal via a pathological filename — join then
# verify the resolved destination is still inside the external dir.
dest = (ext_dir / filename).resolve()
dest = (ext_dir / filename).resolve() # SEC-PATH-OK: resolve + relative_to containment check on next line
try:
dest.relative_to(ext_dir.resolve())
except ValueError:
@ -302,7 +302,7 @@ def _move_file_bytes(file: LibraryFile, target_folder: LibraryFolder | None) ->
raise _MoveSkip("target_inaccessible", f"target path not accessible: {ext_dir}")
if not os.access(ext_dir, os.W_OK):
raise _MoveSkip("target_unwritable", f"target path not writable: {ext_dir}")
dest = (ext_dir / file.filename).resolve()
dest = (ext_dir / file.filename).resolve() # SEC-PATH-OK: resolve + relative_to containment check on next line
try:
dest.relative_to(ext_dir.resolve())
except ValueError:
@ -433,7 +433,9 @@ async def save_3mf_bytes_to_library(
# extension so downstream logic (ThreeMFParser, thumbnail viewer) works.
ext = os.path.splitext(filename)[1].lower() or ".3mf"
unique_filename = f"{uuid.uuid4().hex}{ext}"
file_path = get_library_files_dir() / unique_filename
file_path = (
get_library_files_dir() / unique_filename
) # SEC-PATH-OK: unique_filename = uuid.uuid4().hex + ext, generated on the previous line
with open(file_path, "wb") as fh:
fh.write(file_bytes)
@ -451,7 +453,7 @@ async def save_3mf_bytes_to_library(
if thumb_data:
thumbs_dir = get_library_thumbnails_dir()
thumb_filename = f"{uuid.uuid4().hex}{thumb_ext}"
thumb_path = thumbs_dir / thumb_filename
thumb_path = thumbs_dir / thumb_filename # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + thumb_ext
with open(thumb_path, "wb") as fh:
fh.write(thumb_data)
thumbnail_path = str(thumb_path)
@ -554,7 +556,7 @@ def create_image_thumbnail(file_path: Path, thumbnails_dir: Path, max_size: int
from PIL import Image
thumb_filename = f"{uuid.uuid4().hex}.png"
thumb_path = thumbnails_dir / thumb_filename
thumb_path = thumbnails_dir / thumb_filename # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + ".png"
with Image.open(file_path) as img:
# Convert to RGB if necessary (for PNG with transparency, etc.)
@ -582,7 +584,9 @@ def create_image_thumbnail(file_path: Path, thumbnails_dir: Path, max_size: int
file_size = file_path.stat().st_size
if file_size < 500000: # Less than 500KB
thumb_filename = f"{uuid.uuid4().hex}{file_path.suffix}"
thumb_path = thumbnails_dir / thumb_filename
thumb_path = (
thumbnails_dir / thumb_filename
) # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + file_path.suffix
shutil.copy2(file_path, thumb_path)
return str(thumb_path)
except OSError:
@ -636,6 +640,14 @@ async def _backfill_external_stl_thumbnails(folder_ids: list[int]) -> None:
abs_path = to_absolute_path(stl_file.file_path)
if not abs_path or not abs_path.exists():
continue
# Pre-skip files too small to contain even a single triangle.
# Bulk-uploaded ZIPs of stub STLs would otherwise trigger one
# trimesh.load() call + one debug log line per stub.
try:
if abs_path.stat().st_size < MIN_USABLE_STL_BYTES:
continue
except OSError:
continue
try:
thumb_path = generate_stl_thumbnail(abs_path, thumbnails_dir)
except Exception as exc: # noqa: BLE001 — never let one bad STL kill the rest
@ -1393,7 +1405,9 @@ async def scan_external_folder(
name=part,
parent_id=current_parent,
is_external=True,
external_path=str(ext_path / current_path),
external_path=str(
ext_path / current_path
), # SEC-PATH-OK: current_path built from Path(rel_dir).parts of an os.walk descent under ext_path
external_readonly=folder.external_readonly,
external_show_hidden=folder.external_show_hidden,
)
@ -1409,7 +1423,9 @@ async def scan_external_folder(
if not folder.external_show_hidden and filename.startswith("."):
continue
filepath = Path(dirpath) / filename
filepath = (
Path(dirpath) / filename
) # SEC-PATH-OK: dirpath + filename from os.walk(ext_path); filesystem-discovered, not user input
ext = filepath.suffix.lower()
# Check for compound extensions like .gcode.3mf
@ -1459,7 +1475,9 @@ async def scan_external_folder(
if thumb_data:
thumb_dir = get_library_thumbnails_dir()
thumb_filename = f"{uuid.uuid4().hex}{thumbnail_ext}"
thumb_full = thumb_dir / thumb_filename
thumb_full = (
thumb_dir / thumb_filename
) # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + thumbnail_ext
thumb_full.write_bytes(thumb_data)
thumbnail_path = to_relative_path(thumb_full)
@ -1492,7 +1510,7 @@ async def scan_external_folder(
if thumb_data:
thumb_dir = get_library_thumbnails_dir()
thumb_filename = f"{uuid.uuid4().hex}.png"
thumb_full = thumb_dir / thumb_filename
thumb_full = thumb_dir / thumb_filename # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + ".png"
thumb_full.write_bytes(thumb_data)
thumbnail_path = to_relative_path(thumb_full)
@ -1737,7 +1755,9 @@ async def upload_file(
# Save thumbnail if extracted
if thumbnail_data:
thumb_filename = f"{uuid.uuid4().hex}{thumbnail_ext}"
thumb_path = thumbnails_dir / thumb_filename
thumb_path = (
thumbnails_dir / thumb_filename
) # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + thumbnail_ext
with open(thumb_path, "wb") as f:
f.write(thumbnail_data)
thumbnail_path = str(thumb_path)
@ -1766,7 +1786,9 @@ async def upload_file(
thumbnail_data = extract_gcode_thumbnail(file_path)
if thumbnail_data:
thumb_filename = f"{uuid.uuid4().hex}.png"
thumb_path = thumbnails_dir / thumb_filename
thumb_path = (
thumbnails_dir / thumb_filename
) # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + ".png"
with open(thumb_path, "wb") as f:
f.write(thumbnail_data)
thumbnail_path = str(thumb_path)
@ -1778,9 +1800,16 @@ async def upload_file(
thumbnail_path = create_image_thumbnail(file_path, thumbnails_dir)
elif ext == ".stl":
# Generate STL thumbnail if enabled
# Generate STL thumbnail if enabled. Same MIN_USABLE_STL_BYTES
# pre-skip as extract_zip_file — stubs / placeholders below this
# size can't contain a triangle so trimesh would return an empty
# mesh anyway.
if generate_stl_thumbnails:
thumbnail_path = generate_stl_thumbnail(file_path, thumbnails_dir)
try:
if file_path.stat().st_size >= MIN_USABLE_STL_BYTES:
thumbnail_path = generate_stl_thumbnail(file_path, thumbnails_dir)
except OSError:
pass
# Create database entry (managed files store relative paths for portability;
# external files store the absolute mount path — same shape as scan produces)
@ -1970,7 +1999,9 @@ async def extract_zip_file(
# Generate unique filename for storage
unique_filename = f"{uuid.uuid4().hex}{ext}"
file_path = get_library_files_dir() / unique_filename
file_path = (
get_library_files_dir() / unique_filename
) # SEC-PATH-OK: unique_filename = uuid.uuid4().hex + ext
# Extract and save file
file_content = zf.read(zip_path)
@ -1995,7 +2026,9 @@ async def extract_zip_file(
if thumbnail_data:
thumb_filename = f"{uuid.uuid4().hex}{thumbnail_ext}"
thumb_path = thumbnails_dir / thumb_filename
thumb_path = (
thumbnails_dir / thumb_filename
) # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + thumbnail_ext
with open(thumb_path, "wb") as f:
f.write(thumbnail_data)
thumbnail_path = str(thumb_path)
@ -2022,7 +2055,9 @@ async def extract_zip_file(
thumbnail_data = extract_gcode_thumbnail(file_path)
if thumbnail_data:
thumb_filename = f"{uuid.uuid4().hex}.png"
thumb_path = thumbnails_dir / thumb_filename
thumb_path = (
thumbnails_dir / thumb_filename
) # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + ".png"
with open(thumb_path, "wb") as f:
f.write(thumbnail_data)
thumbnail_path = str(thumb_path)
@ -2033,8 +2068,12 @@ async def extract_zip_file(
thumbnail_path = create_image_thumbnail(file_path, thumbnails_dir)
elif ext == ".stl":
# Generate STL thumbnail if enabled
if generate_stl_thumbnails:
# Generate STL thumbnail if enabled. Pre-skip files
# below MIN_USABLE_STL_BYTES — they can't contain
# even a single triangle, and bulk-uploaded ZIPs of
# stub STLs would otherwise log one debug line per
# file via the empty-mesh branch in trimesh.load.
if generate_stl_thumbnails and len(file_content) >= MIN_USABLE_STL_BYTES:
thumbnail_path = generate_stl_thumbnail(file_path, thumbnails_dir)
# Create database entry (store relative paths for portability)
@ -3540,7 +3579,7 @@ async def slice_and_persist(
base_name = model_filename.rsplit(".", 1)[0]
out_filename = f"{base_name}.gcode.3mf"
unique_name = f"{uuid.uuid4().hex}.gcode.3mf"
out_path = get_library_files_dir() / unique_name
out_path = get_library_files_dir() / unique_name # SEC-PATH-OK: unique_name = uuid.uuid4().hex + ".gcode.3mf"
out_path.write_bytes(result.content)
# Extract thumbnail from the produced 3MF so the library card shows a
@ -3657,9 +3696,13 @@ async def slice_and_persist_as_archive(
timestamp = datetime.now(timezone.utc).strftime("%Y%m%d_%H%M%S")
printer_folder = str(source_archive.printer_id) if source_archive.printer_id is not None else "unassigned"
archive_subdir = f"{timestamp}_{base_name}_sliced"
archive_dir = app_settings.archive_dir / printer_folder / archive_subdir
archive_dir = (
app_settings.archive_dir / printer_folder / archive_subdir
) # SEC-PATH-OK: printer_folder = str(int|None), archive_subdir = f"{timestamp}_{base_name}_sliced" where base_name went through _safe_filename
archive_dir.mkdir(parents=True, exist_ok=True)
out_path = archive_dir / out_filename
out_path = (
archive_dir / out_filename
) # SEC-PATH-OK: out_filename = f"{base_name}.gcode.3mf" where base_name went through _safe_filename
out_path.write_bytes(result.content)
# Extract a thumbnail for the new archive card. Priority order:

View file

@ -41,6 +41,7 @@ from backend.app.schemas.project import (
TimelineEvent,
)
from backend.app.utils.http import build_content_disposition
from backend.app.utils.safe_path import safe_join_under
logger = logging.getLogger(__name__)
@ -892,7 +893,7 @@ async def upload_attachment(
# Generate unique filename
unique_filename = f"{uuid.uuid4().hex}{ext}"
file_path = attachments_dir / unique_filename
file_path = attachments_dir / unique_filename # SEC-PATH-OK: unique_filename = uuid.uuid4().hex + ext
# Save file
try:
@ -964,7 +965,9 @@ async def download_attachment(
raise HTTPException(status_code=404, detail="Attachment not found")
# Check file exists
file_path = get_project_attachments_dir(project_id) / filename
file_path = (
get_project_attachments_dir(project_id) / filename
) # SEC-PATH-OK: filename validated above (no /, \\, .., empty) + attachment membership check
if not file_path.exists():
raise HTTPException(status_code=404, detail="Attachment file not found")
@ -1004,7 +1007,9 @@ async def delete_attachment(
project.attachments = attachments if attachments else None
# Delete file
file_path = get_project_attachments_dir(project_id) / filename
file_path = (
get_project_attachments_dir(project_id) / filename
) # SEC-PATH-OK: filename validated above (no /, \\, .., empty) + attachment membership check
if file_path.exists():
try:
os.remove(file_path)
@ -1066,7 +1071,7 @@ async def upload_project_cover_image(
logger.warning("Failed to delete old cover image %s: %s", old_path, e)
unique_filename = f"cover_{uuid.uuid4().hex}{ext}"
file_path = attachments_dir / unique_filename
file_path = attachments_dir / unique_filename # SEC-PATH-OK: unique_filename = f"cover_{uuid.uuid4().hex}{ext}"
try:
with open(file_path, "wb") as f:
content = await file.read()
@ -1827,6 +1832,13 @@ async def import_project_file(
if not folder_name:
continue
# Containment check on the folder name — refuses absolute paths and
# ``..`` traversal in ``project.json[linked_folders[*].name]``. The
# previous code did ``library_dir / folder_name`` directly, which
# collapses to ``Path(folder_name)`` when folder_name is absolute
# and lets ``..`` escape after mkdir.
folder_path = safe_join_under(library_dir, folder_name)
# Check if folder exists
existing_result = await db.execute(
select(LibraryFolder).where(
@ -1853,7 +1865,6 @@ async def import_project_file(
await db.flush()
# Create folder on disk
folder_path = library_dir / folder_name
folder_path.mkdir(parents=True, exist_ok=True)
# Import files for this folder from ZIP
@ -1868,8 +1879,18 @@ async def import_project_file(
if not relative_path:
continue
# Write file to disk
file_disk_path = library_dir / folder_name / relative_path
# Containment check on the per-entry relative path. ZIP names
# can carry ``..`` segments by spec; without resolve + parent
# containment, ``files/<folder>/../../../etc/x`` escapes
# ``library_dir`` entirely. ``relative_path`` is split into
# parts because ``safe_join_under`` rejects parts that start
# with ``/``, and a single combined string would hide an
# embedded ``..`` segment behind a forward slash.
file_disk_path = safe_join_under(
library_dir,
folder_name,
*Path(relative_path).parts,
)
file_disk_path.parent.mkdir(parents=True, exist_ok=True)
file_disk_path.write_bytes(file_content)

View file

@ -573,7 +573,9 @@ async def create_backup_zip(output_path: Path | None = None) -> tuple[Path, str]
for name, src_dir in dirs_to_backup:
if src_dir.exists() and any(src_dir.iterdir()):
try:
shutil.copytree(src_dir, temp_path / name)
shutil.copytree(
src_dir, temp_path / name
) # SEC-PATH-OK: name iterates the dirs_to_backup tuple of constant strings ("archive", "virtual_printer", ...)
except shutil.Error as e:
logger.warning("Some files in %s could not be copied: %s", name, e)
except PermissionError as e:
@ -599,7 +601,9 @@ async def create_backup_zip(output_path: Path | None = None) -> tuple[Path, str]
# Create ZIP
if output_path is not None:
zip_file = output_path / filename
zip_file = (
output_path / filename
) # SEC-PATH-OK: filename = f"bambuddy-backup-{datetime.now()...}.zip" generated in create_backup_zip itself
else:
fd, tmp = tempfile.mkstemp(suffix=".zip")
os.close(fd)
@ -869,7 +873,9 @@ async def restore_backup(
# Reject path-traversal payloads: any entry whose resolved
# path escapes temp_path would allow writing arbitrary files
# on the host (ZipSlip / CVE-2006-5456).
dest = (temp_path / name).resolve()
dest = (
temp_path / name
).resolve() # SEC-PATH-OK: is_relative_to containment check below before extractall
# is_relative_to (Python 3.9+) covers both relative
# path-traversal (../etc/passwd) and absolute-path overrides
# (/etc/passwd) — str.startswith was vulnerable to
@ -1003,7 +1009,9 @@ async def restore_backup(
skipped_dirs = []
for name, dest_dir in dirs_to_restore:
src_dir = temp_path / name
src_dir = (
temp_path / name
) # SEC-PATH-OK: name iterates the dirs_to_restore tuple of constant strings ("archive", "virtual_printer", ...)
if src_dir.exists():
logger.info("Restoring %s directory...", name)
try:

View file

@ -16,6 +16,7 @@ from backend.app.core.config import settings
from backend.app.models.archive import PrintArchive
from backend.app.models.filament import Filament
from backend.app.models.printer import Printer
from backend.app.utils.safe_path import PathTraversalError, safe_join_under
logger = logging.getLogger(__name__)
@ -1084,7 +1085,9 @@ class ArchiveService:
archive_name = f"{timestamp}_{display_stem}"
# Use "unassigned" folder for archives without a printer
printer_folder = str(printer_id) if printer_id is not None else "unassigned"
archive_dir = settings.archive_dir / printer_folder / archive_name
archive_dir = (
settings.archive_dir / printer_folder / archive_name
) # SEC-PATH-OK: printer_folder = str(int|None) → digits or "unassigned"; archive_name = f"{timestamp}_{display_stem}" where resolve_display_stem strips path components via Path(filename).name
archive_dir.mkdir(parents=True, exist_ok=True)
# Copy 3MF file with an explicit fsync'd loop (avoids a sendfile
@ -1448,12 +1451,29 @@ class ArchiveService:
return False
# Get archive directory
file_path = settings.base_dir / archive.file_path
file_path = (
settings.base_dir / archive.file_path
) # SEC-PATH-OK: archive.file_path is DB-stored, set by archive_print() under settings.archive_dir
archive_dir = file_path.parent
# Save timelapse - use thread pool to avoid blocking event loop
# (timelapse files can be 100MB+, sync write blocks for seconds)
timelapse_file = archive_dir / filename
# (timelapse files can be 100MB+, sync write blocks for seconds).
# `filename` ultimately comes from a printer's FTP listing (compromised-
# printer threat model) or a query param on /archives/{id}/timelapse/select;
# the safe-join helper rejects ``..`` segments and absolute paths so a
# crafted name can't escape the archive directory. Use http=False so a
# service-layer reject surfaces as a return False (matching the existing
# not-found contract) rather than a 400 raised from inside a background
# task.
try:
timelapse_file = safe_join_under(archive_dir, filename, http=False)
except PathTraversalError:
logger.warning(
"Refusing to attach timelapse with unsafe filename %r to archive %s",
filename,
archive_id,
)
return False
await asyncio.to_thread(timelapse_file.write_bytes, timelapse_data)
# Update archive record

View file

@ -651,7 +651,9 @@ async def capture_finish_photo(
# Generate filename with timestamp
timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
filename = f"finish_{timestamp}_{uuid.uuid4().hex[:8]}.jpg"
output_path = photos_dir / filename
output_path = (
photos_dir / filename
) # SEC-PATH-OK: filename = f"finish_{timestamp}_{uuid.uuid4().hex[:8]}.jpg" generated above
success = await capture_camera_frame(
ip_address=ip_address,

View file

@ -64,7 +64,9 @@ def _to_absolute_path(relative_path: str | None) -> Path | None:
path = Path(relative_path)
if path.is_absolute():
return path
return Path(app_settings.base_dir) / path
return (
Path(app_settings.base_dir) / path
) # SEC-PATH-OK: relative_path is LibraryFile.file_path / LibraryFile.thumbnail_path — DB-stored, internally generated by the upload pipeline
def _age_cutoff(now: datetime, older_than_days: int) -> datetime:

View file

@ -223,7 +223,9 @@ class LocalBackupService:
if not filename.startswith("bambuddy-backup-") or not filename.endswith(".zip"):
return None
backup_dir = self._resolve_backup_dir(path_setting)
target = backup_dir / filename
target = (
backup_dir / filename
) # SEC-PATH-OK: filename rejected above on /, \\, .., plus startswith "bambuddy-backup-" + endswith ".zip" gate
if not target.exists():
return None
return target
@ -253,7 +255,9 @@ class LocalBackupService:
return {"success": False, "message": "Invalid filename"}
backup_dir = self._resolve_backup_dir(path_setting)
target = backup_dir / filename
target = (
backup_dir / filename
) # SEC-PATH-OK: filename rejected above on /, \\, .., plus startswith "bambuddy-backup-" + endswith ".zip" gate below
if not target.exists():
return {"success": False, "message": "Backup not found"}

View file

@ -212,7 +212,9 @@ async def store_print_data(
return
# Get 3MF file path
full_path = app_settings.base_dir / file_path
full_path = (
app_settings.base_dir / file_path
) # SEC-PATH-OK: file_path is archive.file_path / library_file.file_path — DB-stored, internally generated
if not full_path.exists():
logger.debug("[SPOOLMAN] 3MF file not found: %s", full_path)
return

View file

@ -4,11 +4,52 @@ Generates thumbnail images from STL files using trimesh and matplotlib.
"""
import logging
import os
import uuid
from pathlib import Path
logger = logging.getLogger(__name__)
# Matplotlib's font_manager emits one INFO line per font on first import
# while it builds its cache, including a noisy "Failed to extract font
# properties from NotoColorEmoji.ttf" for the COLR/COLR1 emoji format it
# doesn't support. These are not actionable — demote to WARNING so real
# font issues still surface but the first STL upload doesn't produce a
# multi-line matplotlib preamble in the journal.
logging.getLogger("matplotlib.font_manager").setLevel(logging.WARNING)
def _configure_matplotlib_cache() -> None:
"""Point matplotlib's config/cache directory at a writable persistent path.
Without this, matplotlib falls back to ``/tmp/matplotlib-XXXXXX`` whenever
``$HOME/.config/matplotlib`` isn't writable — which is the case under
Bambuddy's container / systemd-service deployments where ``$HOME`` is set
to a non-writable path. The fallback emits a WARNING on every cold start
AND loses the font cache on host reboot, so font_manager rebuilds it
every time another batch of INFO lines.
Setting ``MPLCONFIGDIR`` to ``settings.base_dir / .cache / matplotlib``
eliminates both: the warning never fires, and the cache survives across
restarts so the per-font scan only runs once per deployment.
Idempotent respects an externally-set ``MPLCONFIGDIR`` if the operator
chose their own path.
"""
if os.environ.get("MPLCONFIGDIR"):
return
try:
from backend.app.core.config import settings
cache_dir = Path(settings.base_dir) / ".cache" / "matplotlib"
cache_dir.mkdir(parents=True, exist_ok=True)
os.environ["MPLCONFIGDIR"] = str(cache_dir)
except Exception as exc:
# Best-effort. If settings isn't importable or the mkdir fails (read-only
# FS, permission denied), let matplotlib fall back to /tmp with its
# built-in warning — same as today's behaviour, no worse.
logger.debug("Could not configure MPLCONFIGDIR: %s", exc)
# Bambu green color for rendering
BAMBU_GREEN = "#00AE42"
BACKGROUND_COLOR = "#1a1a1a"
@ -16,6 +57,14 @@ BACKGROUND_COLOR = "#1a1a1a"
# Maximum vertices before simplification
MAX_VERTICES = 100000
# Minimum STL file size that could possibly contain a usable mesh:
# - Binary STL with one triangle: 80B header + 4B count + 50B triangle = 134B
# - ASCII STL with one triangle: header + "facet ... endfacet" + footer ≈ 150B
# Files below this are stubs / placeholders / corrupted; trimesh would return an
# empty mesh anyway. Pre-skipping at the call sites suppresses the warning storm
# bulk-uploaded ZIPs of small test STLs used to produce.
MIN_USABLE_STL_BYTES = 200
def generate_stl_thumbnail(
stl_path: Path,
@ -39,6 +88,10 @@ def generate_stl_thumbnail(
thumbnails_dir = Path(thumbnails_dir)
try:
# Must precede the matplotlib import — MPLCONFIGDIR is read at
# matplotlib import time, not on subsequent attribute access.
_configure_matplotlib_cache()
import matplotlib
import trimesh
@ -52,7 +105,14 @@ def generate_stl_thumbnail(
mesh = trimesh.load(str(stl_path), force="mesh")
if mesh is None or not hasattr(mesh, "vertices") or len(mesh.vertices) == 0:
logger.warning("Failed to load STL or empty mesh: %s", stl_path)
# Demoted from warning to debug: this is a per-file content
# observation (the STL is empty / stub / corrupted), not an
# actionable error. The caller proceeds correctly with no
# thumbnail. The call sites also pre-skip files below
# MIN_USABLE_STL_BYTES so the common stub-STL case never gets
# this far — this branch now catches only the rare "large
# enough but trimesh still can't parse it" case.
logger.debug("Failed to load STL or empty mesh: %s", stl_path)
return None
# Simplify large meshes for performance
@ -122,7 +182,7 @@ def generate_stl_thumbnail(
# Save thumbnail
thumb_filename = f"{uuid.uuid4().hex}.png"
thumb_path = thumbnails_dir / thumb_filename
thumb_path = thumbnails_dir / thumb_filename # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + ".png"
fig.savefig(
thumb_path,

View file

@ -414,7 +414,9 @@ class FTPSession:
return
filename = Path(arg).name # Sanitize filename
file_path = self.upload_dir / filename
file_path = (
self.upload_dir / filename
) # SEC-PATH-OK: filename = Path(arg).name strips every path component above
logger.info("FTP receiving file: %s from %s", filename, self.remote_ip)

View file

@ -0,0 +1,114 @@
"""Containment-checked path joining.
Single source of truth for joining a user-controlled string under a trusted
parent directory. The two-vector arbitrary-file-write reported against
``backend/app/api/routes/projects.py::import_project_file`` traced to plain
``Path / user_string`` arithmetic with no resolve + containment check
attacker passed an absolute path, ``Path("/lib") / "/etc"`` collapsed to
``Path("/etc")``, and the next ``write_bytes`` landed wherever the attacker
chose. This module is the answer.
Every site that joins a path component coming from a request body, a ZIP
``namelist()``, an ``UploadFile.filename``, or any other attacker-controlled
source MUST route through ``safe_join_under``. Sites that join trusted
constants (settings paths, hardcoded subdirs) are not in scope those should
carry a ``# SEC-PATH-OK: <reason>`` marker so the CI backstop knows.
"""
from __future__ import annotations
from pathlib import Path
from fastapi import HTTPException
class PathTraversalError(ValueError):
"""Raised when a join attempt would escape the trusted parent.
Callers in API-route context catch this and translate to ``HTTPException``
via ``safe_join_under`` (which already raises HTTPException directly when
invoked with ``http=True``). Non-route callers can catch the
``PathTraversalError`` and decide their own response shape.
"""
def safe_join_under(parent: Path, *parts: str, http: bool = True) -> Path:
"""Join *parts* under *parent* and assert the result stays under it.
Rejects:
- empty / None / non-str parts;
- parts containing NUL (``\\x00``);
- parts starting with ``/`` or ``\\`` (absolute paths;
``Path("/lib") / "/etc"`` discards ``/lib``);
- any sequence whose resolved form is not a descendant of *parent*'s
resolved form (defeats ``..`` traversal even when the literal join
doesn't look suspicious).
Returns the resolved absolute path on success.
When ``http=True`` (default; suitable for FastAPI routes), failures raise
``HTTPException(400, "Invalid path in upload")``. Set ``http=False`` to
raise ``PathTraversalError`` instead for non-route callers that need
finer control over the response.
"""
if not parts:
_fail("safe_join_under called with no parts", http)
for part in parts:
if not isinstance(part, str):
_fail(f"Path part has type {type(part).__name__}, expected str", http)
if not part:
_fail("Empty path part", http)
if "\x00" in part:
_fail("NUL byte in path part", http)
# Reject literal absolute markers: pathlib collapses ``Path("/a") /
# "/b"`` to ``Path("/b")`` so the catch-after-resolve below would also
# fire, but rejecting up-front gives a clearer error and avoids
# touching the filesystem.
if part.startswith("/") or part.startswith("\\"):
_fail("Absolute path part not allowed", http)
parent_resolved = parent.resolve()
candidate = parent
for part in parts:
candidate = candidate / part
candidate_resolved = candidate.resolve()
if not _is_relative_to(candidate_resolved, parent_resolved):
_fail("Path escapes the parent directory", http)
return candidate_resolved
def assert_under(parent: Path, candidate: Path, *, http: bool = True) -> Path:
"""Assert that an already-joined *candidate* path is under *parent*.
Use when you have an existing ``Path`` (e.g. from another helper that
builds the path itself) and need a containment check before writing or
deleting. Equivalent to ``safe_join_under`` minus the per-part input
validation.
"""
parent_resolved = parent.resolve()
candidate_resolved = candidate.resolve()
if not _is_relative_to(candidate_resolved, parent_resolved):
_fail("Path escapes the parent directory", http)
return candidate_resolved
def _is_relative_to(child: Path, parent: Path) -> bool:
# ``Path.is_relative_to`` exists in Python 3.9+. Bambuddy targets 3.11+
# (per pyproject and the bug-report system info) so this is safe.
try:
return child.is_relative_to(parent)
except AttributeError: # pragma: no cover - defensive
try:
child.relative_to(parent)
return True
except ValueError:
return False
def _fail(reason: str, http: bool) -> None:
if http:
raise HTTPException(status_code=400, detail="Invalid path in upload")
raise PathTraversalError(reason)

View file

@ -953,3 +953,122 @@ class TestProjectExportImport:
response = await async_client.post("/api/v1/projects/import/file", files=files)
assert response.status_code == 400
assert "Invalid JSON" in response.json()["detail"]
@pytest.mark.asyncio
@pytest.mark.integration
async def test_import_rejects_absolute_path_in_folder_name(self, async_client: AsyncClient, tmp_path):
"""Absolute paths in `linked_folders[*].name` must not escape library_dir.
Verbatim shape from the upstream advisory: attacker sets folder name to
an absolute path, expecting Python's ``Path("/lib") / "/anywhere"`` to
collapse to ``Path("/anywhere")`` and let the next file write land
outside the library directory.
"""
import io
import json
import zipfile
target_outside = tmp_path / "outside" / "owned"
# Build a ZIP whose folder name points outside library_dir entirely.
zip_buffer = io.BytesIO()
with zipfile.ZipFile(zip_buffer, "w", zipfile.ZIP_DEFLATED) as zf:
zf.writestr(
"project.json",
json.dumps(
{
"name": "innocent",
"linked_folders": [{"name": str(target_outside)}],
}
),
)
zf.writestr(f"files/{target_outside}/evil.pth", b"import os; os.system('echo pwned > /tmp/owned')\n")
zip_buffer.seek(0)
files = {"file": ("evil.zip", zip_buffer, "application/zip")}
response = await async_client.post("/api/v1/projects/import/file", files=files)
assert response.status_code == 400, response.text
assert not target_outside.exists(), "Attacker payload landed outside library_dir"
@pytest.mark.asyncio
@pytest.mark.integration
async def test_import_rejects_dotdot_in_folder_name(self, async_client: AsyncClient):
"""`..` segments in folder name must be rejected."""
import io
import json
import zipfile
zip_buffer = io.BytesIO()
with zipfile.ZipFile(zip_buffer, "w", zipfile.ZIP_DEFLATED) as zf:
zf.writestr(
"project.json",
json.dumps(
{
"name": "innocent",
"linked_folders": [{"name": "../../../etc"}],
}
),
)
zf.writestr("files/../../../etc/x.txt", b"x")
zip_buffer.seek(0)
files = {"file": ("evil.zip", zip_buffer, "application/zip")}
response = await async_client.post("/api/v1/projects/import/file", files=files)
assert response.status_code == 400, response.text
@pytest.mark.asyncio
@pytest.mark.integration
async def test_import_rejects_dotdot_in_relative_path(self, async_client: AsyncClient):
"""`..` segments in the per-entry path (Vector B in the advisory) must
be rejected even when the folder name itself is fine."""
import io
import json
import zipfile
zip_buffer = io.BytesIO()
with zipfile.ZipFile(zip_buffer, "w", zipfile.ZIP_DEFLATED) as zf:
zf.writestr(
"project.json",
json.dumps(
{
"name": "innocent",
"linked_folders": [{"name": "ok"}],
}
),
)
# Folder name is benign, but the file path inside attempts to
# escape via ``..``.
zf.writestr("files/ok/../../../etc/x.txt", b"x")
zip_buffer.seek(0)
files = {"file": ("evil.zip", zip_buffer, "application/zip")}
response = await async_client.post("/api/v1/projects/import/file", files=files)
assert response.status_code == 400, response.text
@pytest.mark.asyncio
@pytest.mark.integration
async def test_import_legit_nested_zip_still_works(self, async_client: AsyncClient):
"""A legitimate ZIP with a nested file path inside the folder must
continue to import cleanly. Guards against the fix being over-strict."""
import io
import json
import zipfile
zip_buffer = io.BytesIO()
with zipfile.ZipFile(zip_buffer, "w", zipfile.ZIP_DEFLATED) as zf:
zf.writestr(
"project.json",
json.dumps(
{
"name": "nested-ok",
"linked_folders": [{"name": "OkFolder"}],
}
),
)
zf.writestr("files/OkFolder/sub/dir/inside.txt", b"hello")
zip_buffer.seek(0)
files = {"file": ("nested.zip", zip_buffer, "application/zip")}
response = await async_client.post("/api/v1/projects/import/file", files=files)
assert response.status_code == 200, response.text
data = response.json()
assert data["name"] == "nested-ok"

View file

@ -0,0 +1,116 @@
"""Regression tests for ArchiveService.attach_timelapse path-traversal guard.
``filename`` ultimately comes from a printer's FTP listing or a query
parameter on ``POST /archives/{id}/timelapse/select``. A compromised printer
that returns a malicious filename (e.g. ``"../../etc/passwd"``) used to land
the write outside the archive directory. The safe-join helper now rejects
such names; this test locks the behaviour in.
"""
from __future__ import annotations
from pathlib import Path
from unittest.mock import AsyncMock, MagicMock
import pytest
from backend.app.services.archive import ArchiveService
@pytest.mark.asyncio
async def test_attach_timelapse_rejects_dotdot_filename(tmp_path: Path, monkeypatch):
"""A ``..`` traversal in filename must not land bytes outside archive_dir."""
# Stage an archive directory that the service thinks is owned.
archive_dir = tmp_path / "archive" / "1" / "20260101_test"
archive_dir.mkdir(parents=True)
# Repoint settings.base_dir so attach_timelapse's archive_dir = file_path.parent
# resolves to our tmp directory.
monkeypatch.setattr(
"backend.app.services.archive.settings",
MagicMock(base_dir=tmp_path),
)
db = MagicMock()
db.commit = AsyncMock()
service = ArchiveService(db)
# Mock the archive lookup to return a row whose file_path resolves under tmp_path.
fake_archive = MagicMock()
fake_archive.file_path = "archive/1/20260101_test/file.3mf"
service.get_archive = AsyncMock(return_value=fake_archive)
# The attacker-controlled filename in the threat model.
malicious = "../../etc/passwd_pwned"
result = await service.attach_timelapse(
archive_id=1,
timelapse_data=b"would-be-attacker-payload",
filename=malicious,
)
# The helper rejected the join → service returns False.
assert result is False
# And no payload landed at the target outside archive_dir.
target_outside = tmp_path / "etc" / "passwd_pwned"
assert not target_outside.exists(), "Attacker payload landed outside archive_dir"
# And no payload landed under archive_dir either (since we rejected before write).
assert not list(archive_dir.glob("*"))
@pytest.mark.asyncio
async def test_attach_timelapse_rejects_absolute_filename(tmp_path: Path, monkeypatch):
"""An absolute path in filename must not collapse the join."""
archive_dir = tmp_path / "archive" / "1" / "20260101_test"
archive_dir.mkdir(parents=True)
monkeypatch.setattr(
"backend.app.services.archive.settings",
MagicMock(base_dir=tmp_path),
)
db = MagicMock()
db.commit = AsyncMock()
service = ArchiveService(db)
fake_archive = MagicMock()
fake_archive.file_path = "archive/1/20260101_test/file.3mf"
service.get_archive = AsyncMock(return_value=fake_archive)
result = await service.attach_timelapse(
archive_id=1,
timelapse_data=b"x",
filename="/tmp/owned_via_absolute",
)
assert result is False
assert not Path("/tmp/owned_via_absolute").exists()
@pytest.mark.asyncio
async def test_attach_timelapse_accepts_legit_filename(tmp_path: Path, monkeypatch):
"""The legitimate happy path must still work — the fix isn't over-strict."""
archive_dir = tmp_path / "archive" / "1" / "20260101_test"
archive_dir.mkdir(parents=True)
monkeypatch.setattr(
"backend.app.services.archive.settings",
MagicMock(base_dir=tmp_path),
)
db = MagicMock()
db.commit = AsyncMock()
service = ArchiveService(db)
fake_archive = MagicMock()
fake_archive.file_path = "archive/1/20260101_test/file.3mf"
fake_archive.timelapse_path = None
service.get_archive = AsyncMock(return_value=fake_archive)
result = await service.attach_timelapse(
archive_id=1,
timelapse_data=b"hello-timelapse",
filename="timelapse_2026-01-01_12-00-00.mp4",
)
assert result is True
landed = archive_dir / "timelapse_2026-01-01_12-00-00.mp4"
assert landed.exists()
assert landed.read_bytes() == b"hello-timelapse"

View file

@ -1,5 +1,6 @@
"""Unit tests for the STL thumbnail service."""
import os
import tempfile
from pathlib import Path
@ -230,3 +231,97 @@ class TestStlThumbnailConstants:
from backend.app.services.stl_thumbnail import MAX_VERTICES
assert MAX_VERTICES == 100000
def test_min_usable_stl_bytes_threshold(self):
"""MIN_USABLE_STL_BYTES is the call-site pre-skip floor.
Binary STL with one triangle = 80B header + 4B count + 50B triangle
= 134B. ASCII STL with one triangle 150B. Anything below this size
cannot contain a usable mesh.
"""
from backend.app.services.stl_thumbnail import MIN_USABLE_STL_BYTES
assert MIN_USABLE_STL_BYTES == 200
# Verify it sits between "smaller than smallest real STL" and
# "common stub size" — the 24-byte ``solid test\nendsolid test``
# stubs that triggered the warning storm.
assert MIN_USABLE_STL_BYTES > 134 # smallest binary STL with one triangle
assert MIN_USABLE_STL_BYTES > 150 # smallest ASCII STL with one triangle
assert MIN_USABLE_STL_BYTES > 24 # the ZIP-stub case in the bug report
def test_font_manager_logger_demoted_to_warning(self):
"""matplotlib.font_manager's per-font INFO scan is demoted at module
import so the first STL upload doesn't surface a multi-line preamble
of matplotlib internals in the journal."""
import logging
# Importing the module sets the level as a side effect.
import backend.app.services.stl_thumbnail # noqa: F401
assert logging.getLogger("matplotlib.font_manager").level >= logging.WARNING
def test_configure_matplotlib_cache_sets_mplconfigdir(self, tmp_path, monkeypatch):
"""``_configure_matplotlib_cache`` points matplotlib at a writable
persistent path so it doesn't fall back to ``/tmp/matplotlib-XXX``
on every cold start."""
from backend.app.services.stl_thumbnail import _configure_matplotlib_cache
# Ensure we start with no value so the helper actually runs.
monkeypatch.delenv("MPLCONFIGDIR", raising=False)
monkeypatch.setattr(
"backend.app.services.stl_thumbnail.Path",
__import__("pathlib").Path,
)
# Stub settings.base_dir to point inside tmp_path.
from backend.app.core import config as core_config
monkeypatch.setattr(core_config.settings, "base_dir", tmp_path, raising=False)
_configure_matplotlib_cache()
assert "MPLCONFIGDIR" in os.environ
configured = Path(os.environ["MPLCONFIGDIR"])
assert configured.exists()
assert configured.is_dir()
# And the directory sits under base_dir, not /tmp/matplotlib-XXX.
assert tmp_path in configured.parents
def test_configure_matplotlib_cache_respects_externally_set_value(self, tmp_path, monkeypatch):
"""If the operator (or container init) has set MPLCONFIGDIR already,
the helper must leave it alone they made a deliberate choice."""
from backend.app.services.stl_thumbnail import _configure_matplotlib_cache
external = str(tmp_path / "external-mpl-cache")
monkeypatch.setenv("MPLCONFIGDIR", external)
_configure_matplotlib_cache()
assert os.environ["MPLCONFIGDIR"] == external
def test_empty_mesh_logged_at_debug_not_warning(self, caplog):
"""An empty STL (header present, no triangles) must log at DEBUG, not
WARNING bulk uploads used to log thousands of WARNING lines per
ZIP. Per-file content observations stay observable in debug logs
but don't spam production journals."""
import logging
import tempfile
from pathlib import Path
from backend.app.services.stl_thumbnail import generate_stl_thumbnail
# The exact 24-byte stub from the bug report
stub_content = b"solid test\nendsolid test"
with tempfile.TemporaryDirectory() as tmpdir:
tmpdir_path = Path(tmpdir)
stl_path = tmpdir_path / "stub.stl"
stl_path.write_bytes(stub_content)
with caplog.at_level(logging.DEBUG, logger="backend.app.services.stl_thumbnail"):
result = generate_stl_thumbnail(stl_path, tmpdir_path)
assert result is None
# The empty-mesh message must NOT appear at WARNING level.
warning_records = [r for r in caplog.records if r.levelno >= logging.WARNING and "empty mesh" in r.getMessage()]
assert warning_records == [], (
f"Empty-mesh path still logs at WARNING: {[r.getMessage() for r in warning_records]}"
)

View file

@ -0,0 +1,203 @@
"""Backstop: every Path-arithmetic site in the API routes that joins a
variable to a directory-like parent must either use ``safe_join_under`` or
carry a ``# SEC-PATH-OK: <reason>`` marker.
A critical advisory traced to plain ``Path / user_string`` arithmetic in
``import_project_file`` the join had no resolve + containment check, and an
attacker-supplied absolute path collapsed the left side. This test catches the
same shape in any new route added later: it AST-walks every Python file under
``backend/app/api/routes/`` and flags every ``a / b`` where ``a`` looks like a
directory variable and ``b`` is a non-constant (i.e. variable / call result).
False positives are intentionally cheap to silence (add a one-line
``# SEC-PATH-OK: <reason>`` justifying the existing guard) so that *future*
unsafe joins are noisy by default.
"""
from __future__ import annotations
import ast
from pathlib import Path
import pytest
# The route surface receives external input directly; the services layer is
# called by the routes and routinely receives values that originated from a
# request (filenames, query params) or from an untrusted external source
# (printer FTP listings — the printer is part of the threat surface in the
# compromised-printer model). Both layers need the strictest gate.
_BACKEND_APP = Path(__file__).resolve().parents[2] / "app"
SCAN_DIRS = [
_BACKEND_APP / "api" / "routes",
_BACKEND_APP / "services",
]
# Identifier substrings that suggest the LHS is a filesystem directory. Heuristic
# but tuned to Bambuddy's conventions — every actual directory variable in the
# routes hits one of these.
_DIR_NAME_HINTS = (
"_dir",
"_path",
"dir_",
"path_",
"temp_path",
"library_dir",
"archive_dir",
"photos_dir",
"base_dir",
"ext_dir",
"attachments_dir",
"static_dir",
"log_dir",
"data_dir",
"folder_path",
"file_disk_path",
"photo_path",
"dest",
"output_path",
)
# Function calls whose return value is a Path under our control. Hits to these
# don't need scrutiny — they're constructed by Bambuddy code, not by the request.
_KNOWN_PATH_FACTORIES = (
"Path",
"get_library_dir",
"get_library_files_dir",
"get_archive_dir",
"get_project_attachments_dir",
"get_project_cover_dir",
"resolve",
)
_MARKER = "# SEC-PATH-OK:"
def _looks_path_like(node: ast.AST) -> bool:
"""Heuristic for whether *node* evaluates to a ``pathlib.Path``."""
if isinstance(node, ast.Call):
func = node.func
if isinstance(func, ast.Name) and func.id in _KNOWN_PATH_FACTORIES:
return True
return bool(isinstance(func, ast.Attribute) and func.attr in _KNOWN_PATH_FACTORIES)
if isinstance(node, ast.Name):
return any(hint in node.id for hint in _DIR_NAME_HINTS)
if isinstance(node, ast.Attribute):
# `settings.base_dir`, `cls.archive_dir`, etc.
return any(hint in node.attr for hint in _DIR_NAME_HINTS)
if isinstance(node, ast.BinOp) and isinstance(node.op, ast.Div):
# Chains like ``base_dir / "x" / variable`` — keep looking left.
return _looks_path_like(node.left)
return False
def _is_constant_string(node: ast.AST) -> bool:
return isinstance(node, ast.Constant) and isinstance(node.value, str)
def _rhs_is_attacker_shape(node: ast.AST) -> bool:
"""The high-risk shape is ``path / Name`` — RHS is a bare variable that
came from somewhere outside this scope (a function parameter, request
body field, ZIP namelist entry).
Attribute (``lib_file.file_path``), Subscript (``photos[i]``), Call
(``str(vp_id)``), and JoinedStr (f-strings) all have *some* structure
that the audit can reason about those are caught by the broader audit
sweep, not the regression backstop. This narrows the noise to the exact
shape that produced the path-traversal class so the backstop only fires
when something that *looks* like that bug appears.
"""
return isinstance(node, ast.Name)
def _line_has_marker(source_lines: list[str], lineno: int, end_lineno: int | None) -> bool:
# Walk every line spanned by the BinOp — the marker can sit anywhere in the
# expression (start, end, or any continuation line of a multi-line join).
start = max(1, lineno)
end = max(start, end_lineno or lineno)
for i in range(start, end + 1):
if i - 1 >= len(source_lines):
continue
if _MARKER in source_lines[i - 1]:
return True
return False
def _enclosing_call_is_safe_join(stack: list[ast.AST]) -> bool:
"""True if the BinOp is being passed directly into ``safe_join_under(...)``.
Tracking parent links keeps the test conservative a ``base_dir / x``
expression that's already inside ``safe_join_under(base_dir / x, ...)``
is fine because the helper does its own containment check. This rarely
happens in practice but keeps the test from yelling about an idiomatic
arrangement.
"""
for ancestor in reversed(stack):
if isinstance(ancestor, ast.Call):
func = ancestor.func
if isinstance(func, ast.Name) and func.id == "safe_join_under":
return True
if isinstance(func, ast.Attribute) and func.attr == "safe_join_under":
return True
return False
def _scan_file(py_file: Path) -> list[str]:
source = py_file.read_text()
source_lines = source.splitlines()
tree = ast.parse(source, filename=str(py_file))
findings: list[str] = []
# Walk with parent stack so we can detect "inside safe_join_under" and
# skip such nodes.
stack: list[ast.AST] = []
def visit(node: ast.AST) -> None:
stack.append(node)
try:
if (
isinstance(node, ast.BinOp)
and isinstance(node.op, ast.Div)
and _looks_path_like(node.left)
and _rhs_is_attacker_shape(node.right)
and not _is_constant_string(node.right)
and not _enclosing_call_is_safe_join(stack)
and not _line_has_marker(source_lines, node.lineno, node.end_lineno)
):
line = source_lines[node.lineno - 1].strip() if node.lineno - 1 < len(source_lines) else "<?>"
findings.append(f"{py_file.name}:{node.lineno} {line}")
for child in ast.iter_child_nodes(node):
visit(child)
finally:
stack.pop()
visit(tree)
return findings
def test_route_path_arithmetic_is_safe_joined_or_marked():
"""Every ``<dir-like> / <non-constant>`` join in a route handler must
either route through ``safe_join_under(...)`` or carry a
``# SEC-PATH-OK: <reason>`` marker on one of its source lines.
Adding ``# SEC-PATH-OK: <reason>`` is the escape hatch for sites where
the input has already been validated (e.g. a denylist + membership
check, a pre-sanitised alphanumeric filter, or an explicit resolve +
``relative_to`` containment check inline). The marker MUST explain the
existing guard silent suppression defeats the backstop's purpose.
"""
findings: list[str] = []
for scan_dir in SCAN_DIRS:
for py_file in sorted(scan_dir.rglob("*.py")):
if py_file.name == "__init__.py":
continue
findings.extend(_scan_file(py_file))
if findings:
pytest.fail(
"Found Path-arithmetic sites in api/routes/ or services/ that "
"join a non-constant value to a directory-like parent without "
"using safe_join_under() or carrying a # SEC-PATH-OK: marker. "
"Each site must either be refactored to "
"safe_join_under(parent, *parts) or tagged with the marker "
"explaining why the existing guard is sufficient.\n\nFindings:\n" + "\n".join(findings)
)

View file

@ -0,0 +1,124 @@
"""Tests for ``backend.app.utils.safe_path.safe_join_under``.
Cover every escape vector documented in the helper plus the legitimate
nested-path use case so the helper's behaviour is locked in.
"""
from __future__ import annotations
from pathlib import Path
import pytest
from fastapi import HTTPException
from backend.app.utils.safe_path import (
PathTraversalError,
assert_under,
safe_join_under,
)
@pytest.fixture()
def library(tmp_path: Path) -> Path:
"""A real on-disk directory that mimics the "trusted parent" role."""
lib = tmp_path / "library"
lib.mkdir()
return lib
class TestSafeJoinUnder:
def test_simple_filename_is_joined(self, library: Path):
result = safe_join_under(library, "model.3mf")
assert result == (library / "model.3mf").resolve()
def test_nested_path_components_are_joined(self, library: Path):
result = safe_join_under(library, "myfolder", "sub", "file.3mf")
assert result == (library / "myfolder" / "sub" / "file.3mf").resolve()
def test_absolute_path_rejected(self, library: Path):
# The exact shape that produced the original CVE — ``Path("/lib") / "/etc/passwd"``
# collapses to ``Path("/etc/passwd")`` in Python's pathlib.
with pytest.raises(HTTPException) as exc:
safe_join_under(library, "/etc/passwd")
assert exc.value.status_code == 400
def test_absolute_windows_path_rejected(self, library: Path):
with pytest.raises(HTTPException):
safe_join_under(library, "\\\\evil\\share\\x")
def test_parent_traversal_rejected(self, library: Path):
with pytest.raises(HTTPException):
safe_join_under(library, "..", "etc", "passwd")
def test_embedded_parent_traversal_rejected(self, library: Path):
# ``library/foo/../../etc/passwd`` resolves outside ``library``.
with pytest.raises(HTTPException):
safe_join_under(library, "foo", "..", "..", "etc", "passwd")
def test_null_byte_rejected(self, library: Path):
with pytest.raises(HTTPException):
safe_join_under(library, "evil\x00.3mf")
def test_empty_string_part_rejected(self, library: Path):
with pytest.raises(HTTPException):
safe_join_under(library, "")
def test_no_parts_rejected(self, library: Path):
with pytest.raises(HTTPException):
safe_join_under(library)
def test_non_string_part_rejected(self, library: Path):
with pytest.raises(HTTPException):
safe_join_under(library, 42) # type: ignore[arg-type]
def test_http_false_raises_path_traversal_error(self, library: Path):
with pytest.raises(PathTraversalError):
safe_join_under(library, "/etc/passwd", http=False)
def test_http_false_allows_clean_join(self, library: Path):
result = safe_join_under(library, "ok.txt", http=False)
assert result == (library / "ok.txt").resolve()
def test_returned_path_is_resolved(self, library: Path):
# The helper returns a resolved path so callers don't need to do it
# themselves — every downstream is_relative_to/parent check assumes
# a canonical form.
result = safe_join_under(library, "x.txt")
assert result == result.resolve()
class TestAssertUnder:
def test_inside_passes(self, library: Path):
candidate = library / "x" / "y" / "z.txt"
out = assert_under(library, candidate)
assert out == candidate.resolve()
def test_outside_rejects(self, library: Path, tmp_path: Path):
outside = tmp_path / "elsewhere" / "evil.txt"
with pytest.raises(HTTPException):
assert_under(library, outside)
def test_outside_raises_path_traversal_error_with_http_false(self, library: Path, tmp_path: Path):
outside = tmp_path / "elsewhere" / "evil.txt"
with pytest.raises(PathTraversalError):
assert_under(library, outside, http=False)
class TestPocReproducer:
"""The exact attacker payload from the advisory.
A directly attacker-controlled folder name pointing at a venv's
site-packages directory used to land a ``.pth`` file on disk. With the
helper in place the join now raises before any write.
"""
def test_advisory_poc_target_dir_rejected(self, library: Path):
# Verbatim shape from the advisory POC.
target_dir = "BAMBUDDY_BASE_DIR/bambuddy/venv/lib/python3.14/site-packages"
# Leading slash → absolute → rejected up-front.
with pytest.raises(HTTPException):
safe_join_under(library, "/" + target_dir)
# No leading slash but with ``..`` traversal embedded in the
# follow-up file path — also rejected.
with pytest.raises(HTTPException):
safe_join_under(library, "innocent", "..", "..", "evil.pth")