security: fix CORS wildcard, stored XSS, CSP header, and validation error disclosure by tomj12k · Pull Request #464 · Morelitea/initiative

tomj12k · 2026-05-07T18:36:36Z

Summary

Addresses four findings from a pentest conducted on 2026-05-07 against initiative-dev.<redacted>.

Findings addressed

Severity	ID	Finding
🔴 Critical	CRIT-001	CORS origin reflection with `allow_credentials=True`
🔴 Critical	CRIT-002	Stored XSS + DoS via task title/description
🟡 Medium	MED-001	No Content-Security-Policy header
🔵 Low	LOW-001	Pydantic validation errors expose internal field paths

Changes

`backend/app/core/config.py`

Added ALLOWED_ORIGINS: list[str] | str | None env var (defaults to APP_URL)
Added cors_origins property that builds the explicit allowlist — APP_URL is always included, additional origins can be added via ALLOWED_ORIGINS=https://a.com,https://b.com

`backend/app/main.py`

CRIT-001: allow_origins=["*"] → allow_origins=settings.cors_origins. Wildcard combined with allow_credentials=True allowed any website to silently read and mutate authenticated users' data.
MED-001: Added add_security_headers HTTP middleware that injects Content-Security-Policy on every response. Covers the SPA served from the same origin.
LOW-001: Added RequestValidationError handler that returns {"detail": "Invalid request data"} instead of full Pydantic field paths and input values.

`backend/app/schemas/task.py`

CRIT-002: Added _strip_html() (removes all HTML tags — for plain-text fields) and _sanitize_html() (strips dangerous elements/attributes — for rich-text fields).
Validators applied to TaskBase.title (strip all HTML) and TaskBase.description / TaskUpdate.description (sanitize dangerous content: <script>, <iframe>, event handler attributes, javascript: URIs).
Confirmed attack: <img src=x onerror=…> in a task title was stored verbatim and crashed the React renderer for all users viewing that project.

`backend/app/schemas/task_sanitization_test.py` (new)

14 unit tests covering the sanitisation validators and the cors_origins config property.

Deployment note

After merging, set in .env:

ALLOWED_ORIGINS=https://your-frontend-origin.com

No other config changes or migrations required.

Testing

cd backend
pytest app/schemas/task_sanitization_test.py -v

…sclosure Addresses findings from pentest 2026-05-07 (initiative-dev.morels.me): CRIT-001 – CORS origin reflection with credentials `allow_origins=["*"]` combined with `allow_credentials=True` allowed any website to make authenticated requests using the victim's session cookie. Replace wildcard with `settings.cors_origins` — an explicit allowlist derived from `APP_URL` and the new `ALLOWED_ORIGINS` env var. CRIT-002 – Stored XSS + DoS via task title/description Task `title` and `description` fields accepted raw HTML/JS. A payload like `<img src=x onerror=…>` in a task title crashed the React renderer for all users viewing that project (DoS confirmed). Without a CSP, the same vector could execute arbitrary JS. Add `field_validator` sanitisers to `TaskBase` and `TaskUpdate`: - `title`: plain text only — all HTML tags stripped via regex - `description`: dangerous elements stripped (script, iframe, object, …), event handler attributes removed, javascript:/data: URIs blocked MED-001 – No Content-Security-Policy header Add an HTTP middleware that injects a CSP on every response. Covers the frontend SPA served from the same origin as the API. LOW-001 – Pydantic validation errors expose internal field paths FastAPI's default RequestValidationError handler returns full Pydantic field paths and input values. Add a custom handler that returns a single generic message ("Invalid request data") instead. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

deepsource-io · 2026-05-07T18:37:33Z

DeepSource Code Review

We reviewed changes in 20adae1...9d00d31 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade	Security Reliability Complexity Hygiene

Code Review Summary

Analyzer	Updated (UTC)	Details
Docker	May 7, 2026 10:20p.m.	Review ↗
JavaScript	May 7, 2026 10:20p.m.	Review ↗
Python	May 7, 2026 10:20p.m.	Review ↗
Shell	May 7, 2026 10:20p.m.	Review ↗
SQL	May 7, 2026 10:20p.m.	Review ↗
Secrets	May 7, 2026 10:20p.m.	Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

deepsource-io · 2026-05-07T18:38:23Z

+
+
+class TestTaskTitleSanitization:
+    def test_plain_text_unchanged(self):


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

deepsource-io · 2026-05-07T18:38:23Z

+        t = TaskBase(title="Fix login bug", project_id=1)
+        assert t.title == "Fix login bug"
+
+    def test_img_onerror_stripped(self):


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

deepsource-io · 2026-05-07T18:38:23Z

+        assert "<img" not in t.title
+        assert "onerror" not in t.title
+
+    def test_script_tag_stripped(self):


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

deepsource-io · 2026-05-07T18:38:23Z

+        assert "<script>" not in t.title
+        assert "hello" in t.title
+
+    def test_anchor_tag_stripped(self):


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

deepsource-io · 2026-05-07T18:38:23Z

+        assert "<a" not in t.title
+        assert "click" in t.title
+
+    def test_nested_html_stripped(self):


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

deepsource-io · 2026-05-07T18:38:23Z

+        )
+        assert "<iframe" not in t.description
+
+    def test_onerror_attribute_stripped(self):


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

deepsource-io · 2026-05-07T18:38:23Z

+        )
+        assert "onerror" not in t.description
+
+    def test_task_update_description_sanitized(self):


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

deepsource-io · 2026-05-07T18:38:23Z

+
+
+class TestCorsOrigins:
+    def test_defaults_to_app_url(self, monkeypatch):


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

deepsource-io · 2026-05-07T18:38:23Z

+        )
+        assert "https://app.example.com" in s.cors_origins
+
+    def test_allowed_origins_string_parsed(self, monkeypatch):


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

deepsource-io · 2026-05-07T18:38:23Z

+        assert "https://www.example.com" in origins
+        assert "https://staging.example.com" in origins
+
+    def test_wildcard_not_in_default_origins(self):


Method doesn't use the class instance and could be converted into a static method

The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

Three bugs introduced in the previous commit: 1. RequestValidationError handler returned a plain string instead of a list, breaking existing tests that iterate over `detail` expecting error objects with custom codes (PROPERTY_OPTIONS_REQUIRED, PROPERTY_DUPLICATE_OPTION_VALUE). Fix: strip only the `input` and `url` keys from each error entry rather than collapsing everything to a single string — application-level error codes are preserved for callers, raw user input is no longer echoed back. 2. _sanitize_html() stripped the <script> open/close tags but left the JS content between them (`window.__desc_xss=1` survived tag removal). Fix: use a paired regex that matches tag-content-closing-tag as a unit so the entire block is removed in one pass. 3. test_empty_string_stays_empty expected TaskBase(title="") to raise ValidationError, but there is no min_length constraint on `title` so Pydantic allows it. Replaced with test_html_only_title_becomes_empty_string which verifies the validator does not crash on degenerate input. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

deepsource-io · 2026-05-07T21:52:33Z

+  (pentest 2026-05-07, initiative-dev.morels.me)
+"""
+
+import pytest


Unused import pytest

An object has been imported but is not used anywhere in the file.
It should either be used or the import should be removed.

deepsource-io · 2026-05-07T21:52:33Z

+"""
+
+import pytest
+from pydantic import ValidationError


Unused ValidationError imported from pydantic

An object has been imported but is not used anywhere in the file.
It should either be used or the import should be removed.

…peError)

LeeJMorel

I think my only feedback for this is that sanitizing fields most definitely impacts more than just tasks. We should probably address this on a more systemic level, given its an app that ingests a LOT of data by its nature. If you want for now you can just remove that part of the PR since systemic stuff is a lot more of a pain haha.

Ideally id like to make _strip_html / _sanitize_html to typed annotations: PlainText, RichText, RawText, ect, and then do a DB migration from str and maybe get a sanitizer library (I'm think nh3? RUST FOR LYFE)

LeeJMorel

okay I added a sanitizer at the schema level, remove ti from here and submit the CORS fix?

tomj12k and others added 2 commits May 3, 2026 19:15

Add .worktrees to gitignore

8391447

tomj12k requested review from LeeJMorel and jordandrako as code owners May 7, 2026 18:36

deepsource-io Bot reviewed May 7, 2026

View reviewed changes

tomj12k added 3 commits May 7, 2026 14:59

fix: remove unused pytest and ValidationError imports (ruff F401)

5384f1b

fix: stringify exception objects in ctx before JSON serialisation (Ty…

62e5d1f

…peError)

fix: convert test classes to plain functions (PYL-R0201 x25)

9d00d31

tomj12k mentioned this pull request May 7, 2026

fix: pre-existing TYP-005 type errors in permissions, queues, and oidc_sync services #465

Open

LeeJMorel reviewed May 8, 2026

View reviewed changes

LeeJMorel requested changes May 14, 2026

View reviewed changes

This was referenced May 23, 2026

Finish typed text annotations from #464 (PlainTextStr / RawTextStr + DB backfill) #527

Open

Polymorphic recent-items bar + command center expansion #526

Merged



		class TestTaskTitleSanitization:
		def test_plain_text_unchanged(self):



		class TestCorsOrigins:
		def test_defaults_to_app_url(self, monkeypatch):

Conversation

tomj12k commented May 7, 2026 • edited by jordandrako Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Findings addressed

Changes

backend/app/core/config.py

backend/app/main.py

backend/app/schemas/task.py

backend/app/schemas/task_sanitization_test.py (new)

Deployment note

Testing

Uh oh!

deepsource-io Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepSource Code Review

PR Report Card

Code Review Summary

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Method doesn't use the class instance and could be converted into a static method

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Unused import pytest

Uh oh!

deepsource-io Bot May 7, 2026

Choose a reason for hiding this comment

Unused ValidationError imported from pydantic

Uh oh!

LeeJMorel left a comment

Choose a reason for hiding this comment

Uh oh!

LeeJMorel left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

tomj12k commented May 7, 2026 •

edited by jordandrako

Loading

`backend/app/core/config.py`

`backend/app/main.py`

`backend/app/schemas/task.py`

`backend/app/schemas/task_sanitization_test.py` (new)

deepsource-io Bot commented May 7, 2026 •

edited

Loading