diff --git a/.gitignore b/.gitignore index 24a406a5..272b2a91 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,6 @@ history/ # Docker docker-compose.yml + +# Git worktrees +.worktrees/ diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 2a36caca..7b057217 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -30,6 +30,27 @@ def cookie_secure(self) -> bool: AUTO_APPROVED_EMAIL_DOMAINS: list[str] = Field(default_factory=list) # APP_URL should point to the frontend entry so redirect URIs resolve correctly APP_URL: str = "http://localhost:5173" + # ALLOWED_ORIGINS controls the CORS allowlist. Defaults to APP_URL so a + # single-origin deploy works out of the box. Override with a + # comma-separated list (e.g. "https://app.example.com,https://www.example.com") + # when the frontend is served from a different origin than APP_URL. + # Never set this to "*" in production — that combined with + # allow_credentials=True allows any site to make authenticated requests. + ALLOWED_ORIGINS: list[str] | str | None = None + + @property + def cors_origins(self) -> list[str]: + """Return the resolved CORS origin allowlist.""" + if isinstance(self.ALLOWED_ORIGINS, list) and self.ALLOWED_ORIGINS: + origins = self.ALLOWED_ORIGINS + elif isinstance(self.ALLOWED_ORIGINS, str) and self.ALLOWED_ORIGINS.strip(): + origins = [o.strip() for o in self.ALLOWED_ORIGINS.split(",") if o.strip()] + else: + origins = [] + # Always include APP_URL so single-origin deploys need no extra config + if self.APP_URL not in origins: + origins = [self.APP_URL] + origins + return origins OIDC_ENABLED: bool = False OIDC_ISSUER: str | None = None OIDC_CLIENT_ID: str | None = None diff --git a/backend/app/main.py b/backend/app/main.py index 335cd89f..6362a5fe 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -6,9 +6,10 @@ from typing import Annotated from fastapi import Depends, FastAPI, HTTPException, Request, status +from fastapi.exceptions import RequestValidationError from fastapi.middleware.cors import CORSMiddleware from fastapi.openapi.utils import get_openapi -from fastapi.responses import FileResponse +from fastapi.responses import FileResponse, JSONResponse from slowapi import _rate_limit_exceeded_handler from slowapi.errors import RateLimitExceeded @@ -52,12 +53,59 @@ app.add_middleware( CORSMiddleware, - allow_origins=["*"], # Update with frontend URL(s) in production + allow_origins=settings.cors_origins, allow_credentials=True, - allow_methods=["*"], - allow_headers=["*"], + allow_methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"], + allow_headers=["Content-Type", "Authorization", "X-Guild-ID"], ) + +@app.middleware("http") +async def add_security_headers(request: Request, call_next): + """Add security headers to every response.""" + response = await call_next(request) + response.headers["Content-Security-Policy"] = ( + "default-src 'self'; " + "script-src 'self'; " + "style-src 'self' 'unsafe-inline'; " + "connect-src 'self'; " + "font-src 'self' https://fonts.gstatic.com https://fonts.googleapis.com; " + "img-src 'self' data: blob:; " + "frame-ancestors 'none'" + ) + return response + + +@app.exception_handler(RequestValidationError) +async def validation_exception_handler(request: Request, exc: RequestValidationError): + """Sanitise validation errors before returning them to clients. + + FastAPI's default handler echoes the raw ``input`` value that triggered + each error back in the response body, which leaks user-supplied data and + exposes the internal Pydantic field structure. We preserve the error + messages (including application-level codes such as + ``PROPERTY_OPTIONS_REQUIRED`` that callers legitimately depend on) but + strip the ``input`` and ``url`` keys from every entry. + """ + def _clean(err: dict) -> dict: + cleaned: dict = {} + for k, v in err.items(): + if k in ("input", "url"): + continue + # Pydantic v2 puts the raw exception object in ctx["error"], which + # is not JSON-serialisable. Stringify any exception values. + if k == "ctx" and isinstance(v, dict): + cleaned[k] = { + ck: str(cv) if isinstance(cv, Exception) else cv + for ck, cv in v.items() + } + else: + cleaned[k] = v + return cleaned + + sanitized = [_clean(err) for err in exc.errors()] + return JSONResponse(status_code=422, content={"detail": sanitized}) + @app.get("/uploads/{filename:path}", include_in_schema=False) @limiter.limit("600/minute") async def serve_upload_file( diff --git a/backend/app/schemas/task.py b/backend/app/schemas/task.py index 4b7b1815..3882f669 100644 --- a/backend/app/schemas/task.py +++ b/backend/app/schemas/task.py @@ -1,3 +1,4 @@ +import re from datetime import datetime from typing import List, Literal, Optional @@ -106,10 +107,78 @@ def validate_combinations(self) -> "TaskRecurrence": return self +_HTML_TAG_RE = re.compile(r"<[^>]+>", re.DOTALL) +# Event handler attributes e.g. onerror=, onclick=, onmouseover= … +_EVENT_HANDLER_RE = re.compile( + r'\bon\w+\s*=\s*(?:"[^"]*"|\'[^\']*\'|[^\s>]*)', re.IGNORECASE +) +# javascript: and data: URI schemes inside attribute values +_DANGEROUS_URI_RE = re.compile( + r'(?:javascript|data|vbscript)\s*:', re.IGNORECASE +) + + +def _strip_html(value: str) -> str: + """Remove all HTML tags from a plain-text field (e.g. task title).""" + return _HTML_TAG_RE.sub("", value).strip() + + +def _sanitize_html(value: str) -> str: + """Strip dangerous HTML from a rich-text field (e.g. task description). + + Removes: + - All inline event handlers (onerror=, onclick=, …) + - javascript: / data: / vbscript: URI schemes + - → stripped entirely (tags + content) + # → stripped (self-closing / void) + _dangerous_names = ( + r"script|iframe|object|embed|form|input|base|link|meta|style" + ) + # Paired tags: strip open tag, content, and close tag together + paired = re.compile( + rf"<\s*({_dangerous_names})(?:\s[^>]*)?>.*?", + re.IGNORECASE | re.DOTALL, + ) + value = paired.sub("", value) + # Remaining unpaired / self-closing open tags (e.g. ) + unpaired = re.compile( + rf"<\s*/?\s*(?:{_dangerous_names})(?:\s[^>]*)?>", + re.IGNORECASE | re.DOTALL, + ) + value = unpaired.sub("", value) + # Strip event handler attributes from remaining tags + value = _EVENT_HANDLER_RE.sub("", value) + # Strip dangerous URI schemes + value = _DANGEROUS_URI_RE.sub("blocked:", value) + return value.strip() + + class TaskBase(BaseModel): title: str description: Optional[str] = None priority: TaskPriority = TaskPriority.medium + + @field_validator("title", mode="before") + @classmethod + def sanitize_title(cls, v: object) -> object: + """Task titles are plain text — strip all HTML tags.""" + if isinstance(v, str): + return _strip_html(v) + return v + + @field_validator("description", mode="before") + @classmethod + def sanitize_description(cls, v: object) -> object: + """Strip dangerous HTML from task descriptions.""" + if isinstance(v, str): + return _sanitize_html(v) + return v start_date: Optional[datetime] = None due_date: Optional[datetime] = None recurrence: Optional[TaskRecurrence] = None @@ -126,6 +195,20 @@ class TaskUpdate(BaseModel): title: Optional[str] = None description: Optional[str] = None task_status_id: Optional[int] = None + + @field_validator("title", mode="before") + @classmethod + def sanitize_title(cls, v: object) -> object: + if isinstance(v, str): + return _strip_html(v) + return v + + @field_validator("description", mode="before") + @classmethod + def sanitize_description(cls, v: object) -> object: + if isinstance(v, str): + return _sanitize_html(v) + return v priority: Optional[TaskPriority] = None assignee_ids: Optional[List[int]] = None start_date: Optional[datetime] = None diff --git a/backend/app/schemas/task_sanitization_test.py b/backend/app/schemas/task_sanitization_test.py new file mode 100644 index 00000000..d7cf59a3 --- /dev/null +++ b/backend/app/schemas/task_sanitization_test.py @@ -0,0 +1,164 @@ +"""Tests for XSS sanitisation on task title and description fields. + +Findings addressed: + CRIT-002 – Stored XSS + DoS via task title/description + (pentest 2026-05-07, initiative-dev.morels.me) +""" + +from app.schemas.task import TaskBase, TaskUpdate + + +# --------------------------------------------------------------------------- +# Title sanitisation – plain text only, all HTML stripped +# --------------------------------------------------------------------------- + + +def test_title_plain_text_unchanged(): + t = TaskBase(title="Fix login bug") + assert t.title == "Fix login bug" + + +def test_title_img_onerror_stripped(): + """The exact payload used in the pentest.""" + payload = "" + t = TaskBase(title=payload) + assert "alert(1)hello") + assert "hover' + t = TaskBase(title="Task", description=payload) + assert "notes") + assert "