Files
nas-tools/dashboard/backend/routers/files.py
T
Gan, Jimmy e98cbb0abb
Deploy Dashboard (Dev) / deploy-dev (push) Successful in 3m35s
Run Tests / Backend Tests (push) Successful in 2m36s
Run Tests / Frontend Tests (push) Failing after 51s
Run Tests / Test Summary (push) Failing after 14s
feat: comprehensive test infrastructure improvements
Phase 1: Fix immediate test failures
- Fix path handling in files router (strip leading slashes)
- Fix test assertions to match actual API behavior
- Add proper error handling for missing files in download endpoint
- Reload files router in test fixtures to pick up config changes
- Fix upload endpoint test to use query params instead of form data

Phase 2: Improve test reliability
- Standardize error responses: docker_router now uses HTTPException
- Add global exception handlers for validation, Docker errors, and unhandled exceptions
- Fix flaky TOTP replay protection test
- Fix flaky password hash loading test with proper module reload
- Enhanced Docker mock fixtures with error scenarios and factory pattern

Phase 3: Add coverage reporting
- Add pytest-cov with 40% minimum coverage threshold
- Add pyproject.toml with pytest and coverage configuration
- Update test workflow to generate coverage and JUnit XML reports
- Remove -x flag to see all test failures
- Configure asyncio_default_fixture_loop_scope to fix deprecation warning

Results:
- All 144 tests passing (was 137 passed, 5 failed, 2 skipped)
- Test execution time: ~3s locally
- Coverage: 43% (above 40% threshold)
- No skipped tests
- Standardized error handling across all endpoints
2026-04-07 08:02:38 +08:00

134 lines
4.3 KiB
Python

import os
import re
import shutil
from pathlib import Path
from fastapi import APIRouter, Depends, UploadFile, File, HTTPException
from fastapi.responses import FileResponse
from config import VOLUME_ROOT
from rbac import require_admin
router = APIRouter()
BASE = Path(VOLUME_ROOT)
MAX_UPLOAD_SIZE = 100 * 1024 * 1024 # 100 MB
BLOCKED_DIRS = {"@appstore", "@docker", "@eaDir", "@S2S", "@sharesnap", "@tmp", "docker", "homes", "backups"}
_BASE_RESOLVED = str(BASE.resolve())
def _safe_path(path: str) -> Path:
# Strip leading slashes to prevent absolute path interpretation
path = path.lstrip("/")
target = (BASE / path).resolve()
if not str(target).startswith(_BASE_RESOLVED):
raise ValueError("forbidden")
# Block access to sensitive directories
rel = target.relative_to(BASE.resolve())
if rel.parts and rel.parts[0] in BLOCKED_DIRS:
raise ValueError("forbidden")
return target
def _is_safe_symlink(item: Path) -> bool:
"""Check if a symlink resolves to a path within BASE and not in blocked dirs."""
if not item.is_symlink():
return True
try:
resolved = item.resolve()
if not str(resolved).startswith(_BASE_RESOLVED):
return False
rel = resolved.relative_to(BASE.resolve())
if rel.parts and rel.parts[0] in BLOCKED_DIRS:
return False
return True
except (OSError, ValueError):
return False
def _sanitize_filename(name: str) -> str:
name = os.path.basename(name)
name = name.replace("\x00", "").replace("/", "").replace("\\", "")
name = re.sub(r'\.\.+', '.', name)
name = name.strip(". ")
if not name:
raise ValueError("invalid filename")
return name
@router.get("/browse")
def browse(path: str = ""):
try:
target = _safe_path(path)
except ValueError:
raise HTTPException(status_code=403, detail="Access denied")
entries = []
for item in sorted(target.iterdir()):
try:
if not _is_safe_symlink(item):
continue
st = item.stat()
entries.append({"name": item.name, "is_dir": item.is_dir(), "size": st.st_size if item.is_file() else 0})
except (PermissionError, OSError):
continue
return {"path": str(target.relative_to(BASE)), "entries": entries}
@router.get("/download")
def download(path: str):
try:
target = _safe_path(path)
if not target.exists():
raise HTTPException(status_code=404, detail="File not found")
return FileResponse(target)
except ValueError:
raise HTTPException(status_code=403, detail="Access denied")
@router.post("/upload", dependencies=[Depends(require_admin())])
async def upload(path: str, file: UploadFile = File(...)):
try:
safe_name = _sanitize_filename(file.filename)
target = _safe_path(path) / safe_name
except ValueError:
raise HTTPException(status_code=403, detail="Access denied")
# Stream with size check
total = 0
with open(target, "wb") as f:
while chunk := await file.read(1024 * 1024):
total += len(chunk)
if total > MAX_UPLOAD_SIZE:
f.close()
target.unlink(missing_ok=True)
raise HTTPException(status_code=413, detail="File too large (max 100MB)")
f.write(chunk)
return {"ok": True}
@router.delete("/delete", dependencies=[Depends(require_admin())])
def delete(path: str, recursive: bool = False):
try:
target = _safe_path(path)
except ValueError:
raise HTTPException(status_code=403, detail="Access denied")
if target == BASE.resolve():
raise HTTPException(status_code=403, detail="Cannot delete base root")
try:
if target.is_dir():
if not recursive:
raise HTTPException(status_code=400, detail="Directory deletion requires recursive=true")
shutil.rmtree(target)
else:
target.unlink()
except HTTPException:
raise
except FileNotFoundError:
raise HTTPException(status_code=404, detail="Path not found")
except PermissionError:
raise HTTPException(status_code=403, detail="Permission denied")
except OSError as exc:
raise HTTPException(status_code=400, detail=str(exc))
return {"ok": True}