diff --git a/.gitignore b/.gitignore index 73d5009..b18af88 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,6 @@ # uv.lock коммитим (это приложение, не библиотека) — не игнорируем # TS build artifact tsconfig.tsbuildinfo + +# runtime-данные приложения (вложения и пр.) +backend/data/ diff --git a/backend/app/api/attachments.py b/backend/app/api/attachments.py index e616eaa..abc3304 100644 --- a/backend/app/api/attachments.py +++ b/backend/app/api/attachments.py @@ -5,7 +5,6 @@ уже сохранённый markdown-текст документов. """ -import shutil import uuid from pathlib import Path from typing import Any, cast @@ -22,6 +21,12 @@ router = APIRouter(prefix="/api", tags=["attachments"]) ALLOWED_PREFIXES = ("image/",) +# SVG не принимаем вовсе: это скриптуемый формат, его inline-отдача в origin +# приложения = stored XSS (кука сессии уходит на скрипт). Растровые форматы +# (png/jpeg/gif/webp) скриптовать нельзя. Лимит размера — без него загрузка +# льёт файлы на диск бесконтрольно. +FORBIDDEN_MIMES = ("image/svg+xml",) +MAX_FILE_BYTES = 10 * 1024 * 1024 def _attachments_dir() -> Path: @@ -53,13 +58,25 @@ saved: list[Attachment] = [] for file in files: mime = file.content_type or "application/octet-stream" - if not mime.startswith(ALLOWED_PREFIXES): + if not mime.startswith(ALLOWED_PREFIXES) or mime in FORBIDDEN_MIMES: raise HTTPException(status_code=415, detail=f"Unsupported type: {mime}") ext = Path(file.filename or "image").suffix or ".bin" stored_name = f"{uuid.uuid4().hex}{ext}" target = _attachments_dir() / stored_name + size = 0 + too_large = False with target.open("wb") as out: - shutil.copyfileobj(file.file, out) + # copyfileobj не умеет лимит (третий аргумент — размер буфера): + # копируем кусками и следим за объёмом сами + while chunk := file.file.read(1024 * 1024): + size += len(chunk) + if size > MAX_FILE_BYTES: + too_large = True + break + out.write(chunk) + if too_large: + target.unlink() + raise HTTPException(status_code=413, detail="File too large") att = Attachment( document_id=document.id, filename=stored_name, @@ -93,7 +110,8 @@ path = _attachments_dir() / att.filename if not path.is_file(): raise HTTPException(status_code=404, detail="File missing on disk") - return FileResponse(path, media_type=att.mime) + # nosniff: браузер не должен угадывать тип иначе, чем записанный mime + return FileResponse(path, media_type=att.mime, headers={"X-Content-Type-Options": "nosniff"}) @router.delete("/attachments/{attachment_id}") diff --git a/backend/app/api/tags.py b/backend/app/api/tags.py index 6063f53..f641ad4 100644 --- a/backend/app/api/tags.py +++ b/backend/app/api/tags.py @@ -27,7 +27,7 @@ @router.delete("/{tag_id}") -async def delete_tag(tag_id: int, db: DbDep) -> dict[str, bool]: +async def delete_tag(tag_id: int, db: DbDep, user: UserDep) -> dict[str, bool]: tag = db.get(Tag, tag_id) if tag is None: raise HTTPException(status_code=404, detail="Tag not found") diff --git a/backend/app/auth/routes.py b/backend/app/auth/routes.py index 8ffdbc6..2bd02a0 100644 --- a/backend/app/auth/routes.py +++ b/backend/app/auth/routes.py @@ -21,8 +21,16 @@ SCOPES = ["openid", "email", "profile"] +def _safe_return_to(return_to: str) -> str: + """Только внутренние пути: open redirect через ?return_to=https://evil.com закрыт.""" + if return_to.startswith("/") and not return_to.startswith("//"): + return return_to + return "/" + + @router.get("/login") async def login(request: Request, return_to: str = "/") -> RedirectResponse: + return_to = _safe_return_to(return_to) client = get_gauth_client() auth_request = client.build_authorization_request(return_to=return_to, scopes=SCOPES) # return_to запоминаем в сессии и забираем в callback после обмена code. @@ -51,7 +59,7 @@ "locale": user.profile.get("locale") or "", } - return_to = request.session.pop("return_to", "/") + return_to = _safe_return_to(request.session.pop("return_to", "/")) return RedirectResponse(return_to) diff --git a/backend/app/config.py b/backend/app/config.py index 0d39e0b..b0b305e 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -2,8 +2,14 @@ from functools import lru_cache +from pydantic import model_validator from pydantic_settings import BaseSettings, SettingsConfigDict +# Дефолт в репозитории — подделываемая кука, если при деплое забыть SESSION_SECRET. +# Падаем на старте вместо молчаливо небезопасной сессии (остальные секреты и так +# обязательны — без них gauth не работает). +DEFAULT_SESSION_SECRET = "change-me-session-secret" + class Settings(BaseSettings): model_config = SettingsConfigDict(env_file=".env", env_file_encoding="utf-8", extra="ignore") @@ -18,7 +24,7 @@ database_url: str = "postgresql+psycopg://gntodo:gntodo@localhost:5432/gntodo" # Session cookie - session_secret: str = "change-me-session-secret" + session_secret: str = DEFAULT_SESSION_SECRET session_cookie_name: str = "gntodo_session" # Ollama (M2) @@ -31,6 +37,15 @@ # MCP-сервер (M5): bearer-токен для агентов; пусто — /mcp закрыт (401) mcp_token: str = "" + @model_validator(mode="after") + def _session_secret_must_be_set(self) -> "Settings": + if self.session_secret == DEFAULT_SESSION_SECRET: + raise ValueError( + "SESSION_SECRET не задан: дефолтный секрет из репозитория позволяет " + "подделать куку сессии. Задайте SESSION_SECRET в .env." + ) + return self + @lru_cache def get_settings() -> Settings: diff --git a/backend/app/main.py b/backend/app/main.py index 2de5d34..33575cb 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1,5 +1,6 @@ """Точка входа FastAPI: health + OAuth-флоу gnexus-gauth + защищённое /api + /mcp.""" +import hmac from collections.abc import AsyncIterator, Awaitable, Callable from contextlib import asynccontextmanager @@ -49,7 +50,8 @@ if request.url.path.startswith("/mcp"): token = get_settings().mcp_token auth = request.headers.get("authorization", "") - if not token or auth != f"Bearer {token}": + # сравнение constant-time: обычный == позволяет timing-атаку на токен + if not token or not hmac.compare_digest(auth, f"Bearer {token}"): return JSONResponse({"detail": "Not authenticated"}, status_code=401) return await call_next(request) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 3726c32..15a5d14 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -9,6 +9,7 @@ _tmpdir = tempfile.mkdtemp(prefix="gntodo-att-") os.environ.setdefault("ATTACHMENTS_PATH", _tmpdir) os.environ.setdefault("MCP_TOKEN", "test-mcp-token") +os.environ.setdefault("SESSION_SECRET", "test-session-secret") MCP_TOKEN = os.environ["MCP_TOKEN"] import pytest # noqa: E402 diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py index ebc8c79..65d6c68 100644 --- a/backend/tests/test_auth.py +++ b/backend/tests/test_auth.py @@ -25,6 +25,26 @@ assert "state=" in location and "code_challenge=" in location +def test_safe_return_to() -> None: + """Open redirect: наружный return_to превращается в корень приложения.""" + from app.auth.routes import _safe_return_to + + assert _safe_return_to("/tasks/5") == "/tasks/5" + assert _safe_return_to("https://evil.com") == "/" + assert _safe_return_to("//evil.com") == "/" + assert _safe_return_to("") == "/" + + +def test_default_session_secret_rejected() -> None: + """Забытый SESSION_SECRET не должен поднимать приложение молча.""" + import pytest as _pytest + + from app.config import Settings + + with _pytest.raises(ValueError, match="SESSION_SECRET"): + Settings(_env_file=None, session_secret="change-me-session-secret") + + def test_api_requires_auth() -> None: """Регрессия: все /api эндпоинты должны требовать сессию.""" from app.dependencies import require_user @@ -38,6 +58,26 @@ assert c.get("/api/projects").status_code == 401 assert c.get("/api/tags").status_code == 401 assert c.post("/api/tasks", json={"title": "x"}).status_code == 401 + assert c.delete("/api/tags/1").status_code == 401 finally: if saved is not None: app.dependency_overrides[require_user] = saved + + +def test_every_api_route_requires_user() -> None: + """Каждый /api-роут обязан иметь require_user в зависимостях: один новый + эндпоинт без UserDep (как было с DELETE /api/tags) не должен проходить мимо.""" + from fastapi.routing import APIRoute + + from app.dependencies import require_user + + bare = [] + for route in app.routes: + if not isinstance(route, APIRoute): + continue + if not route.path.startswith("/api/") or route.path == "/api/health": + continue + deps = [d.call for d in route.dependant.dependencies] + if require_user not in deps: + bare.append(route.path) + assert bare == [], f"роуты без авторизации: {bare}" diff --git a/backend/tests/test_detailing.py b/backend/tests/test_detailing.py index f016fa6..2cbc469 100644 --- a/backend/tests/test_detailing.py +++ b/backend/tests/test_detailing.py @@ -246,3 +246,42 @@ files={"files": ("doc.pdf", b"%PDF-1.4", "application/pdf")}, ) assert up.status_code == 415 + + +def test_attachments_reject_svg(client: TestClient) -> None: + """SVG — скриптуемый формат: inline-отдача в origin приложения = stored XSS.""" + task = create_task(client) + up = client.post( + f"/api/documents/{task['document_id']}/attachments", + files={ + "files": ( + "evil.svg", + b'', + "image/svg+xml", + ) + }, + ) + assert up.status_code == 415 + + +def test_attachments_reject_oversize(client: TestClient) -> None: + from app.api.attachments import MAX_FILE_BYTES + + task = create_task(client) + big = b"\x89PNG" + b"\x00" * (MAX_FILE_BYTES + 1) + up = client.post( + f"/api/documents/{task['document_id']}/attachments", + files={"files": ("big.png", big, "image/png")}, + ) + assert up.status_code == 413 + + +def test_attachment_file_has_nosniff(client: TestClient) -> None: + task = create_task(client) + up = client.post( + f"/api/documents/{task['document_id']}/attachments", + files={"files": ("pic.png", b"\x89PNG\r\n\x1a\nx", "image/png")}, + ) + att = up.json()[0] + got = client.get(f"/api/attachments/{att['id']}/file") + assert got.headers["x-content-type-options"] == "nosniff" diff --git a/frontend/src/App.vue b/frontend/src/App.vue index c34c202..c577a0c 100644 --- a/frontend/src/App.vue +++ b/frontend/src/App.vue @@ -47,11 +47,20 @@ } onMounted(async () => { - const res = await fetch('/auth/me') - if (res.ok) { - const data = await res.json() - user.value = data.user - accountUrl.value = data.account_url ?? '' + // оффлайн-старт PWA (ТЗ 3.15): оболочка открывается без сети — /auth/me + // реджектится, и надо не уронить onMounted молча, а просто остаться гостем + let me: { user: UserInfo; account_url?: string } | null = null + try { + const res = await fetch('/auth/me') + if (res.ok) { + me = await res.json() + } + } catch { + me = null + } + if (me) { + user.value = me.user + accountUrl.value = me.account_url ?? '' // SSE-события (ТЗ 3.14): только для авторизованных — иначе EventSource // будет бесконечно ловить 401 startRealtime() diff --git a/frontend/src/components/GardenScene.vue b/frontend/src/components/GardenScene.vue index dcc53cf..d8e90d4 100644 --- a/frontend/src/components/GardenScene.vue +++ b/frontend/src/components/GardenScene.vue @@ -173,11 +173,16 @@ // --- Жизненный цикл --- +// флаг «компонент размонтирован» — init Pixi резолвится асинхронно +let unmounted = false + onMounted(async () => { window.addEventListener('keydown', onKeydown) window.addEventListener('scroll', onScroll, { capture: true, passive: true }) document.addEventListener('fullscreenchange', onFullscreenChange) const { GardenRenderer: R } = await import('../game/GardenRenderer') + // unmount во время загрузки модуля/init: create() ниже может уже не выполняться + if (unmounted) return if (!hostEl.value) return renderer.value = await R.create(hostEl.value, props.state, { onMove: (id, x, y) => emit('move', id, x, y), @@ -187,9 +192,16 @@ onDragChange: (dragging) => emit('dragging', dragging), inventoryRect: () => invEl.value?.getBoundingClientRect() ?? null, }) + // init мог резолвнуться уже после unmount — уничтожаем сразу, иначе ticker + // и ResizeObserver живут вечно + if (unmounted) { + renderer.value.destroy() + renderer.value = null + } }) onBeforeUnmount(() => { + unmounted = true window.removeEventListener('keydown', onKeydown) window.removeEventListener('scroll', onScroll, { capture: true }) document.removeEventListener('fullscreenchange', onFullscreenChange) diff --git a/frontend/src/components/TaskForm.vue b/frontend/src/components/TaskForm.vue index c23da1b..b5e9b41 100644 --- a/frontend/src/components/TaskForm.vue +++ b/frontend/src/components/TaskForm.vue @@ -153,8 +153,12 @@ const submitText = computed(() => props.submitLabel ?? t(props.task ? 'common.save' : 'common.create')) const formEl = ref(null) +// busy: двойной клик по «Создать» не должен плодить две задачи +const busy = ref(false) async function save() { + if (busy.value) return + busy.value = true try { // GnTagInput подтверждает набираемый тег только по Enter/вставке: если // текст остался в поле (клик по кнопке сразу после ввода), фиксируем его @@ -210,6 +214,8 @@ emit('saved', task) } catch (e) { toast.error({ title: t('common.error'), text: String(e) }) + } finally { + busy.value = false } } @@ -369,7 +375,7 @@
- + {{ submitText }} diff --git a/frontend/src/realtime.ts b/frontend/src/realtime.ts index 32c1578..97686a5 100644 --- a/frontend/src/realtime.ts +++ b/frontend/src/realtime.ts @@ -2,7 +2,7 @@ // События грубозернистые (kind + данные): подписчики реагируют refetch'ем. // Reconnect: браузер сам переподключает EventSource, но при CLOSED-состоянии // (сессия истекла / сеть лежит долго) включается backoff 1→2→5→15 с. -import { onScopeDispose } from 'vue' +import { getCurrentScope, onScopeDispose } from 'vue' export interface RealtimeEvent { kind: string @@ -90,10 +90,15 @@ handlers.set(kind, set) } set.add(handler) - return () => { + const off = () => { set?.delete(handler) if (set && set.size === 0) handlers.delete(kind) } + // Подписка из setup-контекста компонента (типичный вид) умирает вместе с + // компонентом: без этого N визитов на страницу оставляли N живых обработчиков + // в модульном Map — каждое SSE-событие дёргало load() на отмонтированных. + if (getCurrentScope()) onScopeDispose(off) + return off } // Дедуп XP-тостов (окно 3 с): собственные действия празднуются по заголовкам diff --git a/frontend/src/views/ListView.vue b/frontend/src/views/ListView.vue index fff3a3b..d2fc81c 100644 --- a/frontend/src/views/ListView.vue +++ b/frontend/src/views/ListView.vue @@ -30,31 +30,50 @@ const pagedTasks = computed(() => tasks.value.slice((page.value - 1) * 30, page.value * 30), ) -watch(filters, () => { - page.value = 1 -}) +watch( + filters, + () => { + page.value = 1 + }, + // селекты мутируют вложенные поля — без deep watch молчит (ТЗ 3.6: смена + // фильтра возвращает на первую страницу) + { deep: true }, +) + +// Токен запроса: поздний ответ старого фильтра не перетирает список нового +let loadSeq = 0 async function load() { + const seq = ++loadSeq loading.value = true error.value = '' try { - tasks.value = await api.listTasks({ + const fetched = await api.listTasks({ detail_state: 'approved', status: filters.value.status, project_id: filters.value.project_id, tag_id: filters.value.tag_id, }) + if (seq !== loadSeq) return + tasks.value = fetched } catch (e) { + if (seq !== loadSeq) return error.value = String(e) } finally { - loading.value = false + if (seq === loadSeq) loading.value = false } } onMounted(async () => { await load() - projects.value = await api.listProjects() - tags.value = await api.listTags() + try { + // опции фильтров: грузятся вместе со списком (не в try load) — но и они + // должны показывать ошибку, а не падать молча + projects.value = await api.listProjects() + tags.value = await api.listTags() + } catch (e) { + error.value = String(e) + } }) // Реактивность (ТЗ 3.14): изменения задач/проектов (MCP, соседняя вкладка) — diff --git a/frontend/src/views/ProjectView.vue b/frontend/src/views/ProjectView.vue index 62c0023..103c584 100644 --- a/frontend/src/views/ProjectView.vue +++ b/frontend/src/views/ProjectView.vue @@ -177,22 +177,34 @@ } } +// Токен запроса: поздний ответ для старого id (быстрая навигация по проектам) +// не должен перетирать данные нового +let loadSeq = 0 + async function load() { + const seq = ++loadSeq loading.value = true notFound.value = false error.value = '' try { - project.value = await api.getProject(projectId.value) - tasks.value = await api.listTasks({ project_id: projectId.value }) - tags.value = await api.listTags() + const [fetched, projectTasks, tagList] = await Promise.all([ + api.getProject(projectId.value), + api.listTasks({ project_id: projectId.value }), + api.listTags(), + ]) + if (seq !== loadSeq) return + project.value = fetched + tasks.value = projectTasks + tags.value = tagList } catch (e) { + if (seq !== loadSeq) return if (e instanceof ApiError && e.status === 404) { notFound.value = true } else { error.value = String(e) } } finally { - loading.value = false + if (seq === loadSeq) loading.value = false } } diff --git a/frontend/src/views/StackView.vue b/frontend/src/views/StackView.vue index e320a44..27cfa85 100644 --- a/frontend/src/views/StackView.vue +++ b/frontend/src/views/StackView.vue @@ -70,9 +70,14 @@ onMounted(() => loadAll()) +// busy: двойной Enter до очистки поля не должен создавать дубликаты +const quickBusy = ref(false) + async function quickAdd() { + if (quickBusy.value) return const title = quickTitle.value.trim() if (!title) return + quickBusy.value = true try { await api.createTask(title) quickTitle.value = '' @@ -83,6 +88,8 @@ await loadAll() } catch (e) { toast.error({ title: t('common.error'), text: String(e) }) + } finally { + quickBusy.value = false } } diff --git a/frontend/src/views/TaskView.vue b/frontend/src/views/TaskView.vue index 80ade65..4c56074 100644 --- a/frontend/src/views/TaskView.vue +++ b/frontend/src/views/TaskView.vue @@ -69,27 +69,43 @@ setPageTitle(task.value?.title ?? t('nav.task')) }) +// Токен запроса: при быстрой навигации (id сменился) ответ старого getTask, +// пришедший позже, не должен перетереть данные новой задачи +let loadSeq = 0 + async function load() { + const seq = ++loadSeq loading.value = true notFound.value = false error.value = '' try { - task.value = await api.getTask(taskId.value) - if (task.value.document_id !== null) { - attachments.value = await api.listAttachments(task.value.document_id) + const fetched = await api.getTask(taskId.value) + if (seq !== loadSeq) return + task.value = fetched + if (fetched.document_id !== null) { + const atts = await api.listAttachments(fetched.document_id) + if (seq !== loadSeq) return + attachments.value = atts } // все задачи (не только утверждённые) — подзадача видна сразу после создания - tasks.value = await api.listTasks() - tagNames.value = task.value.tags.map((t0) => t0.name) - if (!tagsCatalog.value.length) tagsCatalog.value = await api.listTags() + const all = await api.listTasks() + if (seq !== loadSeq) return + tasks.value = all + tagNames.value = fetched.tags.map((t0) => t0.name) + if (!tagsCatalog.value.length) { + const catalog = await api.listTags() + if (seq !== loadSeq) return + tagsCatalog.value = catalog + } } catch (e) { + if (seq !== loadSeq) return if (e instanceof ApiError && e.status === 404) { notFound.value = true } else { error.value = String(e) } } finally { - loading.value = false + if (seq === loadSeq) loading.value = false } } @@ -105,9 +121,13 @@ }) // Переход «подзадача → родитель» (и любой /tasks/:id → /tasks/:id) переиспользует -// этот же инстанс компонента — onMounted не сработает; перезагружаемся по смене id +// этот же инстанс компонента — onMounted не сработает; перезагружаемся по смене id. +// Черновики инлайн-правок сбрасываем обязательно: иначе открытый редактор описания +// старой задачи уедет PATCH'ем в новую (тихая порча данных). watch(taskId, () => { editing.value = false + descEditing.value = false + draftDescription.value = '' void load() }) @@ -564,7 +584,7 @@