Fix audit findings: security, robustness, and dead code

- Remove dead get_analysis_service() function in main.py
- Scope file deletion to UPLOAD_DIR to prevent path traversal
- Parse datetime strings back to datetime objects in repos
- Add 10-minute polling timeout in frontend analysis store
- Accept .pdf extension (not just MIME type) on drag-and-drop
- Guard localStorage access for private browsing compatibility

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Pier-Jean Malandrino 2026-03-31 11:28:42 +02:00
parent 6b0fc45e5d
commit 001cf6807c
7 changed files with 60 additions and 18 deletions

View file

@ -14,7 +14,7 @@ from __future__ import annotations
import logging import logging
from contextlib import asynccontextmanager from contextlib import asynccontextmanager
from fastapi import FastAPI, Request from fastapi import FastAPI
from fastapi.middleware.cors import CORSMiddleware from fastapi.middleware.cors import CORSMiddleware
from api.analyses import router as analyses_router from api.analyses import router as analyses_router
@ -89,11 +89,6 @@ app.include_router(documents_router)
app.include_router(analyses_router) app.include_router(analyses_router)
def get_analysis_service(request: Request) -> AnalysisService:
"""FastAPI dependency — retrieve the AnalysisService from app.state."""
return request.app.state.analysis_service
@app.get("/health") @app.get("/health")
def health(): def health():
"""Health check endpoint.""" """Health check endpoint."""

View file

@ -2,10 +2,19 @@
from __future__ import annotations from __future__ import annotations
from datetime import datetime
from domain.models import AnalysisJob, AnalysisStatus from domain.models import AnalysisJob, AnalysisStatus
from persistence.database import get_connection from persistence.database import get_connection
def _parse_dt(value: str | None) -> datetime | None:
"""Parse an ISO-format datetime string back into a datetime object."""
if not value:
return None
return datetime.fromisoformat(value)
def _row_to_job(row) -> AnalysisJob: def _row_to_job(row) -> AnalysisJob:
return AnalysisJob( return AnalysisJob(
id=row["id"], id=row["id"],
@ -15,9 +24,9 @@ def _row_to_job(row) -> AnalysisJob:
content_html=row["content_html"], content_html=row["content_html"],
pages_json=row["pages_json"], pages_json=row["pages_json"],
error_message=row["error_message"], error_message=row["error_message"],
started_at=row["started_at"], started_at=_parse_dt(row["started_at"]),
completed_at=row["completed_at"], completed_at=_parse_dt(row["completed_at"]),
created_at=row["created_at"], created_at=_parse_dt(row["created_at"]) or datetime.now(),
document_filename=row["filename"] if "filename" in row.keys() else None, # noqa: SIM118 document_filename=row["filename"] if "filename" in row.keys() else None, # noqa: SIM118
) )

View file

@ -2,11 +2,16 @@
from __future__ import annotations from __future__ import annotations
from datetime import datetime
from domain.models import Document from domain.models import Document
from persistence.database import get_connection from persistence.database import get_connection
def _row_to_document(row) -> Document: def _row_to_document(row) -> Document:
created = row["created_at"]
if isinstance(created, str):
created = datetime.fromisoformat(created)
return Document( return Document(
id=row["id"], id=row["id"],
filename=row["filename"], filename=row["filename"],
@ -14,7 +19,7 @@ def _row_to_document(row) -> Document:
file_size=row["file_size"], file_size=row["file_size"],
page_count=row["page_count"], page_count=row["page_count"],
storage_path=row["storage_path"], storage_path=row["storage_path"],
created_at=row["created_at"], created_at=created,
) )

View file

@ -70,10 +70,14 @@ async def delete(doc_id: str) -> bool:
# Delete associated analyses first (cascade) # Delete associated analyses first (cascade)
await analysis_repo.delete_by_document(doc_id) await analysis_repo.delete_by_document(doc_id)
# Delete file from disk # Delete file from disk (only if inside UPLOAD_DIR)
try: try:
if os.path.exists(doc.storage_path): real_path = os.path.realpath(doc.storage_path)
os.unlink(doc.storage_path) real_upload_dir = os.path.realpath(UPLOAD_DIR)
if real_path.startswith(real_upload_dir + os.sep) and os.path.exists(real_path):
os.unlink(real_path)
elif os.path.exists(doc.storage_path):
logger.warning("Refused to delete file outside upload dir: %s", doc.storage_path)
except OSError: except OSError:
logger.warning("Could not delete file: %s", doc.storage_path) logger.warning("Could not delete file: %s", doc.storage_path)

View file

@ -9,6 +9,8 @@ export const useAnalysisStore = defineStore('analysis', () => {
const running = ref(false) const running = ref(false)
const error = ref<string | null>(null) const error = ref<string | null>(null)
const pollingInterval = ref<ReturnType<typeof setInterval> | null>(null) const pollingInterval = ref<ReturnType<typeof setInterval> | null>(null)
const pollingTimeout = ref<ReturnType<typeof setTimeout> | null>(null)
const MAX_POLLING_DURATION = 10 * 60 * 1000 // 10 minutes
const currentPages = computed<Page[]>(() => { const currentPages = computed<Page[]>(() => {
if (!currentAnalysis.value?.pagesJson) return [] if (!currentAnalysis.value?.pagesJson) return []
@ -72,6 +74,13 @@ export const useAnalysisStore = defineStore('analysis', () => {
running.value = false running.value = false
} }
}, 2000) }, 2000)
pollingTimeout.value = setTimeout(() => {
if (pollingInterval.value) {
error.value = 'Analysis timed out'
stopPolling()
running.value = false
}
}, MAX_POLLING_DURATION)
} }
function stopPolling(): void { function stopPolling(): void {
@ -79,6 +88,10 @@ export const useAnalysisStore = defineStore('analysis', () => {
clearInterval(pollingInterval.value) clearInterval(pollingInterval.value)
pollingInterval.value = null pollingInterval.value = null
} }
if (pollingTimeout.value) {
clearTimeout(pollingTimeout.value)
pollingTimeout.value = null
}
} }
async function select(id: string): Promise<void> { async function select(id: string): Promise<void> {

View file

@ -57,7 +57,7 @@ async function onFileSelect(e: Event) {
async function onDrop(e: DragEvent) { async function onDrop(e: DragEvent) {
dragging.value = false dragging.value = false
const file = e.dataTransfer?.files?.[0] const file = e.dataTransfer?.files?.[0]
if (file && file.type === 'application/pdf') { if (file && (file.type === 'application/pdf' || file.name.toLowerCase().endsWith('.pdf'))) {
try { try {
store.clearError() store.clearError()
const doc = await store.upload(file) const doc = await store.upload(file)

View file

@ -2,13 +2,29 @@ import { defineStore } from 'pinia'
import { ref, watch, watchEffect } from 'vue' import { ref, watch, watchEffect } from 'vue'
import type { Locale, Theme } from '../../shared/types' import type { Locale, Theme } from '../../shared/types'
function safeGetItem(key: string): string | null {
try {
return localStorage.getItem(key)
} catch {
return null
}
}
function safeSetItem(key: string, value: string): void {
try {
localStorage.setItem(key, value)
} catch {
// localStorage unavailable (private browsing, quota exceeded)
}
}
export const useSettingsStore = defineStore('settings', () => { export const useSettingsStore = defineStore('settings', () => {
const apiUrl = ref('http://localhost:8000') const apiUrl = ref('http://localhost:8000')
const theme = ref<Theme>((localStorage.getItem('docling-theme') as Theme) || 'dark') const theme = ref<Theme>((safeGetItem('docling-theme') as Theme) || 'dark')
const locale = ref<Locale>((localStorage.getItem('docling-locale') as Locale) || 'fr') const locale = ref<Locale>((safeGetItem('docling-locale') as Locale) || 'fr')
watch(theme, (v) => localStorage.setItem('docling-theme', v)) watch(theme, (v) => safeSetItem('docling-theme', v))
watch(locale, (v) => localStorage.setItem('docling-locale', v)) watch(locale, (v) => safeSetItem('docling-locale', v))
watchEffect(() => { watchEffect(() => {
document.documentElement.classList.toggle('light', theme.value === 'light') document.documentElement.classList.toggle('light', theme.value === 'light')