805f41c757
Il Content-Type del client non era verificato: un file "x.html" dichiarato image/png veniva salvato e poi servito per estensione come text/html, eseguito dal browser. Nuova detect_file_type() (PNG/JPEG/GIF/WebP/PDF) in files.py, usata anche dal logo in settings.py; l'estensione salvata segue sempre il tipo rilevato, mai il nome file dichiarato dal client. Il logo rifiuta SVG a prescindere (puo' portare script). Upload letti a blocchi da 1MB, interrotti appena superano il limite invece di bufferizzare tutto prima di controllare la dimensione. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
534 lines
19 KiB
Python
534 lines
19 KiB
Python
"""Tests for files router (/api/files)."""
|
|
import io
|
|
import os
|
|
import tempfile
|
|
from pathlib import Path
|
|
from unittest.mock import patch
|
|
|
|
import pytest
|
|
from httpx import AsyncClient
|
|
from sqlalchemy import select
|
|
from sqlalchemy.ext.asyncio import AsyncSession
|
|
|
|
from src.backend.models.orm.recipe import RecipeVersion
|
|
from src.backend.models.orm.task import RecipeTask
|
|
from src.backend.models.orm.user import User
|
|
from src.backend.tests.conftest import auth_headers, create_test_recipe
|
|
|
|
|
|
class TestUploadFile:
|
|
"""POST /api/files/upload tests."""
|
|
|
|
async def test_upload_image(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
"""Maker can upload a valid image."""
|
|
# Create a minimal valid JPEG (1x1 pixel)
|
|
jpeg_bytes = (
|
|
b"\xff\xd8\xff\xe0\x00\x10JFIF\x00\x01\x01\x00\x00\x01"
|
|
b"\x00\x01\x00\x00\xff\xdb\x00C\x00\x08\x06\x06\x07\x06"
|
|
b"\x05\x08\x07\x07\x07\t\t\x08\n\x0c\x14\r\x0c\x0b\x0b"
|
|
b"\x0c\x19\x12\x13\x0f\x14\x1d\x1a\x1f\x1e\x1d\x1a\x1c"
|
|
b"\x1c $.\' \",#\x1c\x1c(7),01444\x1f\'9=82<.342\xff\xc0"
|
|
b"\x00\x0b\x08\x00\x01\x00\x01\x01\x01\x11\x00\xff\xc4"
|
|
b"\x00\x1f\x00\x00\x01\x05\x01\x01\x01\x01\x01\x01\x00"
|
|
b"\x00\x00\x00\x00\x00\x00\x00\x01\x02\x03\x04\x05\x06"
|
|
b"\x07\x08\t\n\x0b\xff\xc4\x00\xb5\x10\x00\x02\x01\x03"
|
|
b"\x03\x02\x04\x03\x05\x05\x04\x04\x00\x00\x01}\x01\x02"
|
|
b"\x03\x00\x04\x11\x05\x12!1A\x06\x13Qa\x07\"q\x142\x81"
|
|
b"\x91\xa1\x08#B\xb1\xc1\x15R\xd1\xf0$3br\x82\t\n\x16"
|
|
b"\x17\x18\x19\x1a%&\'()*456789:CDEFGHIJSTUVWXYZcdefghij"
|
|
b"stuvwxyz\x83\x84\x85\x86\x87\x88\x89\x8a\x92\x93\x94"
|
|
b"\x95\x96\x97\x98\x99\x9a\xa2\xa3\xa4\xa5\xa6\xa7\xa8"
|
|
b"\xa9\xaa\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xc2\xc3"
|
|
b"\xc4\xc5\xc6\xc7\xc8\xc9\xca\xd2\xd3\xd4\xd5\xd6\xd7"
|
|
b"\xd8\xd9\xda\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea"
|
|
b"\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xff\xda\x00"
|
|
b"\x08\x01\x01\x00\x00?\x00T\xdb\xae\x8e\xd3H\xa5(?"
|
|
b"\xff\xd9"
|
|
)
|
|
|
|
# Patch settings.upload_path to use tmp_path
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
mock_settings.max_upload_size_mb = 50
|
|
|
|
resp = await client.post(
|
|
"/api/files/upload",
|
|
headers=auth_headers(maker_user),
|
|
files={"file": ("test.jpg", io.BytesIO(jpeg_bytes), "image/jpeg")},
|
|
)
|
|
|
|
assert resp.status_code == 200
|
|
data = resp.json()
|
|
assert "file_path" in data
|
|
assert data["file_type"] == "image/jpeg"
|
|
assert data["file_size"] > 0
|
|
|
|
async def test_extension_forced_to_detected_type_ignores_filename(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
"""gitea#36 point 2: the stored extension follows the detected magic
|
|
bytes, not the client-supplied filename — even a misleading one."""
|
|
jpeg_bytes = b"\xff\xd8\xff\xe0\x00\x10JFIF" + b"\x00" * 20 + b"\xff\xd9"
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
mock_settings.max_upload_size_mb = 50
|
|
|
|
resp = await client.post(
|
|
"/api/files/upload",
|
|
headers=auth_headers(maker_user),
|
|
# Filename claims .txt; content is a real JPEG.
|
|
files={"file": ("photo.txt", io.BytesIO(jpeg_bytes), "text/plain")},
|
|
)
|
|
|
|
assert resp.status_code == 200
|
|
data = resp.json()
|
|
assert data["file_type"] == "image/jpeg"
|
|
assert data["file_path"].endswith(".jpg")
|
|
|
|
async def test_upload_declared_type_lies_about_html_content(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
"""gitea#36 point 2: a file whose bytes are HTML must be rejected even
|
|
when the client declares Content-Type: image/png and names it .png —
|
|
this is what previously let a stored file be served back as
|
|
text/html by extension and executed by the browser."""
|
|
html_bytes = b"<html><body><script>alert(1)</script></body></html>"
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
mock_settings.max_upload_size_mb = 50
|
|
|
|
resp = await client.post(
|
|
"/api/files/upload",
|
|
headers=auth_headers(maker_user),
|
|
files={"file": ("x.png", io.BytesIO(html_bytes), "image/png")},
|
|
)
|
|
|
|
assert resp.status_code == 400
|
|
assert not any(tmp_path.rglob("x.png"))
|
|
|
|
async def test_upload_invalid_type(
|
|
self, client: AsyncClient, maker_user: User
|
|
):
|
|
"""Uploading a disallowed file type returns 400."""
|
|
resp = await client.post(
|
|
"/api/files/upload",
|
|
headers=auth_headers(maker_user),
|
|
files={
|
|
"file": ("malware.exe", io.BytesIO(b"MZ\x00\x00"), "application/x-msdownload")
|
|
},
|
|
)
|
|
assert resp.status_code == 400
|
|
assert "not allowed" in resp.json()["detail"]
|
|
|
|
async def test_upload_too_large(
|
|
self, client: AsyncClient, maker_user: User
|
|
):
|
|
"""Uploading a file that exceeds size limit returns 400."""
|
|
# Create a large file content
|
|
large_content = b"\x00" * (51 * 1024 * 1024) # 51MB
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.max_upload_size_mb = 50
|
|
mock_settings.upload_path = Path(tempfile.mkdtemp())
|
|
|
|
resp = await client.post(
|
|
"/api/files/upload",
|
|
headers=auth_headers(maker_user),
|
|
files={
|
|
"file": ("big.jpg", io.BytesIO(large_content), "image/jpeg")
|
|
},
|
|
)
|
|
|
|
assert resp.status_code == 400
|
|
assert "exceeds" in resp.json()["detail"]
|
|
|
|
|
|
class TestGetFile:
|
|
"""GET /api/files/{file_path} tests."""
|
|
|
|
async def test_get_file(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
"""Authenticated user can retrieve an uploaded file."""
|
|
# Create a test file in tmp_path
|
|
test_file = tmp_path / "testfile.txt"
|
|
test_file.write_text("hello test")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.get(
|
|
"/api/files/testfile.txt",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 200
|
|
|
|
async def test_get_file_not_found(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
"""Requesting a non-existent file returns 404."""
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.get(
|
|
"/api/files/nonexistent.png",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 404
|
|
|
|
|
|
class TestGetRestrictedFile:
|
|
"""gitea#21: GET statistics/* and reports/* need Metrologist/Supervisor/admin."""
|
|
|
|
async def test_measurement_tec_forbidden_on_statistics(
|
|
self, client: AsyncClient, measurement_tec_user: User, tmp_path: Path
|
|
):
|
|
stats_dir = tmp_path / "statistics"
|
|
stats_dir.mkdir()
|
|
(stats_dir / "production_1_x.csv").write_text("a,b\n1,2\n")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.get(
|
|
"/api/files/statistics/production_1_x.csv",
|
|
headers=auth_headers(measurement_tec_user),
|
|
)
|
|
|
|
assert resp.status_code == 403
|
|
|
|
async def test_metrologist_allowed_on_statistics(
|
|
self, client: AsyncClient, metrologist_user: User, tmp_path: Path
|
|
):
|
|
stats_dir = tmp_path / "statistics"
|
|
stats_dir.mkdir()
|
|
(stats_dir / "production_1_x.csv").write_text("a,b\n1,2\n")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.get(
|
|
"/api/files/statistics/production_1_x.csv",
|
|
headers=auth_headers(metrologist_user),
|
|
)
|
|
|
|
assert resp.status_code == 200
|
|
|
|
async def test_admin_allowed_on_reports(
|
|
self, client: AsyncClient, admin_user: User, tmp_path: Path
|
|
):
|
|
reports_dir = tmp_path / "reports"
|
|
reports_dir.mkdir()
|
|
(reports_dir / "spc_1.pdf").write_bytes(b"%PDF-1.4")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.get(
|
|
"/api/files/reports/spc_1.pdf",
|
|
headers=auth_headers(admin_user),
|
|
)
|
|
|
|
assert resp.status_code == 200
|
|
|
|
async def test_maker_unrestricted_path_still_allowed(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
"""The restriction is scoped to statistics/reports, not every file."""
|
|
test_file = tmp_path / "1" / "1" / "drawing.png"
|
|
test_file.parent.mkdir(parents=True)
|
|
test_file.write_bytes(b"\x89PNG")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.get(
|
|
"/api/files/1/1/drawing.png",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 200
|
|
|
|
|
|
class TestDeleteFile:
|
|
"""DELETE /api/files/{file_path} tests."""
|
|
|
|
async def test_delete_file(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
"""Maker can delete a file."""
|
|
test_file = tmp_path / "deleteme.txt"
|
|
test_file.write_text("delete me")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.delete(
|
|
"/api/files/deleteme.txt",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 204
|
|
assert not test_file.exists()
|
|
|
|
async def test_delete_unreferenced_recipe_image_allowed(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
"""A file under a recipe/version path but referenced nowhere is safe."""
|
|
test_file = tmp_path / "5" / "12" / "images" / "orphan.png"
|
|
test_file.parent.mkdir(parents=True)
|
|
test_file.write_bytes(b"\x89PNG")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.delete(
|
|
"/api/files/5/12/images/orphan.png",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 204
|
|
assert not test_file.exists()
|
|
|
|
|
|
class TestDeleteProtectedFolders:
|
|
"""gitea#21: statistics/, reports/, logos/ can never be deleted here."""
|
|
|
|
async def test_maker_delete_statistics_forbidden(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
stats_dir = tmp_path / "statistics"
|
|
stats_dir.mkdir()
|
|
test_file = stats_dir / "production_1_x.csv"
|
|
test_file.write_text("a,b\n1,2\n")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.delete(
|
|
"/api/files/statistics/production_1_x.csv",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 403
|
|
assert test_file.exists()
|
|
|
|
async def test_admin_delete_reports_forbidden(
|
|
self, client: AsyncClient, admin_user: User, tmp_path: Path
|
|
):
|
|
"""Even an admin cannot delete through this route: not just a role gate."""
|
|
reports_dir = tmp_path / "reports"
|
|
reports_dir.mkdir()
|
|
test_file = reports_dir / "spc_1.pdf"
|
|
test_file.write_bytes(b"%PDF-1.4")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.delete(
|
|
"/api/files/reports/spc_1.pdf",
|
|
headers=auth_headers(admin_user),
|
|
)
|
|
|
|
assert resp.status_code == 403
|
|
assert test_file.exists()
|
|
|
|
async def test_delete_logo_forbidden(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
"""The logo is only ever replaced via POST /api/settings/logo."""
|
|
logos_dir = tmp_path / "logos"
|
|
logos_dir.mkdir()
|
|
test_file = logos_dir / "company_logo.png"
|
|
test_file.write_bytes(b"\x89PNG")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.delete(
|
|
"/api/files/logos/company_logo.png",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 403
|
|
assert test_file.exists()
|
|
|
|
|
|
class TestDeleteReferencedRecipeFile:
|
|
"""gitea#21: a recipe file backing the current version, a historical
|
|
version, or measured data must survive a DELETE (409)."""
|
|
|
|
async def test_current_version_no_measurements_is_allowed(
|
|
self,
|
|
client: AsyncClient,
|
|
maker_user: User,
|
|
db_session: AsyncSession,
|
|
tmp_path: Path,
|
|
):
|
|
"""Still a draft (current version, nothing measured against it yet):
|
|
swapping the drawing out is exactly what editing a recipe does."""
|
|
recipe = await create_test_recipe(db_session, maker_user.id, code="FILE-409-A")
|
|
version = recipe.versions[0]
|
|
task = (await db_session.execute(
|
|
select(RecipeTask).where(RecipeTask.version_id == version.id)
|
|
)).scalar_one()
|
|
task.file_path = "recipe-a/1/drawing.png"
|
|
await db_session.commit()
|
|
|
|
test_file = tmp_path / "recipe-a" / "1" / "drawing.png"
|
|
test_file.parent.mkdir(parents=True)
|
|
test_file.write_bytes(b"\x89PNG")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.delete(
|
|
"/api/files/recipe-a/1/drawing.png",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 204
|
|
assert not test_file.exists()
|
|
|
|
async def test_current_version_with_measurements_is_409(
|
|
self,
|
|
client: AsyncClient,
|
|
maker_user: User,
|
|
db_session: AsyncSession,
|
|
tmp_path: Path,
|
|
):
|
|
"""Once something has been measured against this version, its drawing
|
|
must stay: the audit trail needs it to make sense of the numbers."""
|
|
recipe = await create_test_recipe(db_session, maker_user.id, code="FILE-409-D")
|
|
version = recipe.versions[0]
|
|
task = (await db_session.execute(
|
|
select(RecipeTask).where(RecipeTask.version_id == version.id)
|
|
)).scalar_one()
|
|
task.file_path = "recipe-d/1/drawing.png"
|
|
|
|
from src.backend.models.orm.measurement import Measurement
|
|
from src.backend.models.orm.task import RecipeSubtask
|
|
|
|
subtask = (await db_session.execute(
|
|
select(RecipeSubtask).where(RecipeSubtask.task_id == task.id)
|
|
)).scalar_one()
|
|
db_session.add(Measurement(
|
|
subtask_id=subtask.id,
|
|
version_id=version.id,
|
|
measured_by=maker_user.id,
|
|
value=10.0,
|
|
pass_fail="pass",
|
|
input_method="manual",
|
|
))
|
|
await db_session.commit()
|
|
|
|
test_file = tmp_path / "recipe-d" / "1" / "drawing.png"
|
|
test_file.parent.mkdir(parents=True)
|
|
test_file.write_bytes(b"\x89PNG")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.delete(
|
|
"/api/files/recipe-d/1/drawing.png",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 409
|
|
assert test_file.exists()
|
|
|
|
async def test_noncurrent_version_task_file_is_409(
|
|
self,
|
|
client: AsyncClient,
|
|
maker_user: User,
|
|
db_session: AsyncSession,
|
|
tmp_path: Path,
|
|
):
|
|
"""A file kept only for a superseded version is history, not garbage."""
|
|
recipe = await create_test_recipe(db_session, maker_user.id, code="FILE-409-B")
|
|
|
|
old_version = RecipeVersion(
|
|
recipe_id=recipe.id,
|
|
version_number=2,
|
|
is_current=False,
|
|
created_by=maker_user.id,
|
|
change_notes="Superseded",
|
|
)
|
|
db_session.add(old_version)
|
|
await db_session.flush()
|
|
old_task = RecipeTask(
|
|
version_id=old_version.id,
|
|
order_index=0,
|
|
title="Old drawing",
|
|
task_type="drawing",
|
|
file_path="recipe-b/2/old-drawing.png",
|
|
)
|
|
db_session.add(old_task)
|
|
await db_session.commit()
|
|
|
|
test_file = tmp_path / "recipe-b" / "2" / "old-drawing.png"
|
|
test_file.parent.mkdir(parents=True)
|
|
test_file.write_bytes(b"\x89PNG")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.delete(
|
|
"/api/files/recipe-b/2/old-drawing.png",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 409
|
|
assert test_file.exists()
|
|
|
|
async def test_recipe_cover_image_is_409(
|
|
self,
|
|
client: AsyncClient,
|
|
maker_user: User,
|
|
db_session: AsyncSession,
|
|
tmp_path: Path,
|
|
):
|
|
recipe = await create_test_recipe(db_session, maker_user.id, code="FILE-409-C")
|
|
recipe.image_path = "recipe-c/cover.png"
|
|
await db_session.commit()
|
|
|
|
test_file = tmp_path / "recipe-c" / "cover.png"
|
|
test_file.parent.mkdir(parents=True)
|
|
test_file.write_bytes(b"\x89PNG")
|
|
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.delete(
|
|
"/api/files/recipe-c/cover.png",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
assert resp.status_code == 409
|
|
assert test_file.exists()
|
|
|
|
|
|
class TestFilenameSanitization:
|
|
"""Path traversal protection tests."""
|
|
|
|
async def test_path_traversal_blocked(
|
|
self, client: AsyncClient, maker_user: User, tmp_path: Path
|
|
):
|
|
"""Path traversal attempt returns 403 or 404."""
|
|
with patch("src.backend.api.routers.files.settings") as mock_settings:
|
|
mock_settings.upload_path = tmp_path
|
|
|
|
resp = await client.get(
|
|
"/api/files/../../etc/passwd",
|
|
headers=auth_headers(maker_user),
|
|
)
|
|
|
|
# Should be blocked by security check
|
|
assert resp.status_code in (403, 404)
|