From b045d378f0d3fac6e7f4e8b3eb9dabb8315ee4a8 Mon Sep 17 00:00:00 2001
From: technosexual <92812073+atljh@users.noreply.github.com>
Date: Sun, 30 Aug 2026 18:11:09 +0300
Subject: [PATCH] fix(security): stop granting every origin credentialed access
 to the API (#5)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`AURUM_CORS_ORIGINS` defaults to `*`, and the middleware was configured with
`allow_credentials=True`. Starlette does not answer that combination with a
literal `*` — it echoes back whatever `Origin` the caller sent and adds
`Access-Control-Allow-Credentials: true`, so any page open in the same
browser can call the instance, with the user's Basic Auth attached, and read
the response:

    GET /api/accounts   Origin: https://evil.example
    -> Access-Control-Allow-Origin: https://evil.example
       Access-Control-Allow-Credentials: true

That undercuts the README's advice that leaving Basic Auth off is fine while
the instance is only reachable from localhost: reachable-from-localhost
includes every website the user visits.

Nothing in the shipped setup is cross-origin. Compose serves the UI and the
API from the same nginx, and the Vite dev server proxies `/api`
(frontend/vite.config.ts), so neither deployment nor development needs CORS
at all. The default is therefore empty — no cross-origin access — and the
middleware is only installed when origins are configured deliberately.

An explicit `*` is still honoured for a genuinely public read-only setup, but
never together with credentials, which is what the spec intends `*` to mean
anyway.
---
 .env.example               |  9 +++++++--
 backend/app/core/config.py |  7 +++++--
 backend/app/main.py        | 22 +++++++++++++++-------
 backend/tests/test_cors.py | 37 +++++++++++++++++++++++++++++++++++++
 docker-compose.yml         |  2 +-
 5 files changed, 65 insertions(+), 12 deletions(-)
 create mode 100644 backend/tests/test_cors.py

diff --git a/.env.example b/.env.example
index 70a9191..472ce46 100644
--- a/.env.example
+++ b/.env.example
@@ -8,8 +8,13 @@ AURUM_POSTGRES_HOST=db
 AURUM_POSTGRES_PORT=5432
 
 # --- Backend ---
-# Comma-separated list of allowed origins, or * for any (fine for local self-hosting).
-AURUM_CORS_ORIGINS=*
+# Comma-separated list of browser origins allowed to call the API. Leave it
+# empty unless you serve the frontend from a different origin than the API:
+# docker compose serves both from the same nginx, and the Vite dev server
+# proxies /api, so neither needs this. "*" is honoured but then credentials
+# are never granted, because "*" plus credentials lets any site you visit
+# read this instance.
+AURUM_CORS_ORIGINS=
 AURUM_DEFAULT_CURRENCY=USD
 
 # --- Crypto tab (live prices via CoinGecko) ---
diff --git a/backend/app/core/config.py b/backend/app/core/config.py
index 0aa8fef..1e59a34 100644
--- a/backend/app/core/config.py
+++ b/backend/app/core/config.py
@@ -22,8 +22,11 @@ class Settings(BaseSettings):
     # Default currency shown across the UI when an account doesn't override it
     default_currency: str = "USD"
 
-    # Comma-separated list of origins allowed to call the API (frontend dev server, etc.)
-    cors_origins: str = "*"
+    # Comma-separated list of browser origins allowed to call the API. Empty
+    # by default, which allows none: the shipped compose serves the UI and the
+    # API from one nginx, and the Vite dev server proxies /api, so neither is
+    # a cross-origin caller. Set it only for a genuinely separate frontend.
+    cors_origins: str = ""
 
     # CoinGecko Demo API key (free, no card required — https://www.coingecko.com/en/api/pricing)
     # for services/crypto_service.py's price lookups. Empty by default; the
diff --git a/backend/app/main.py b/backend/app/main.py
index 5b98f21..1f1bdac 100644
--- a/backend/app/main.py
+++ b/backend/app/main.py
@@ -40,13 +40,21 @@ async def lifespan(app: FastAPI):
 
 app = FastAPI(title="Aurum API", version=APP_VERSION, lifespan=lifespan)
 
-app.add_middleware(
-    CORSMiddleware,
-    allow_origins=settings.cors_origins_list,
-    allow_credentials=True,
-    allow_methods=["*"],
-    allow_headers=["*"],
-)
+# CORS stays off unless someone deliberately opens it, and credentials are
+# only granted to a pinned list. Starlette answers a credentialed "*" by
+# echoing back whatever Origin asked instead of a literal "*" — so the two
+# together turn any page the user happens to have open into an authenticated
+# client of their instance, which is exactly what the README's "fine if it's
+# only reachable from localhost" advice assumes can't happen.
+cors_origins = settings.cors_origins_list
+if cors_origins:
+    app.add_middleware(
+        CORSMiddleware,
+        allow_origins=cors_origins,
+        allow_credentials="*" not in cors_origins,
+        allow_methods=["*"],
+        allow_headers=["*"],
+    )
 
 app.include_router(dashboard.router, prefix="/api")
 app.include_router(accounts.router, prefix="/api")
diff --git a/backend/tests/test_cors.py b/backend/tests/test_cors.py
new file mode 100644
index 0000000..1c5c07d
--- /dev/null
+++ b/backend/tests/test_cors.py
@@ -0,0 +1,37 @@
+"""CORS defaults.
+
+The README tells self-hosters that leaving basic auth off is fine as long as
+the instance is only reachable from localhost. That promise only holds if a
+random page open in the same browser can't call the API on their behalf —
+which is what these tests pin down.
+"""
+from httpx import AsyncClient
+
+from app.core.config import Settings
+
+EVIL = "https://evil.example"
+
+
+async def test_a_foreign_origin_is_never_echoed_back(client: AsyncClient):
+    """Starlette answers a credentialed wildcard by echoing whatever Origin
+    asked, which is what makes the wildcard dangerous rather than merely
+    permissive — the answer must be `*` or nothing, never the caller's own
+    origin."""
+    resp = await client.get("/accounts", headers={"Origin": EVIL})
+
+    assert resp.headers.get("access-control-allow-origin") != EVIL
+
+
+async def test_credentials_are_not_granted_to_every_origin(client: AsyncClient):
+    """With allow-credentials, a page the user visits can call the instance
+    with their Basic Auth attached and read the response."""
+    resp = await client.get("/accounts", headers={"Origin": EVIL})
+
+    assert resp.headers.get("access-control-allow-credentials") != "true"
+
+
+def test_shipped_default_allows_no_cross_origin_access():
+    """Nothing in the shipped setup needs CORS: compose serves UI and API
+    from one origin, and the Vite dev server proxies /api. So the default
+    should be closed, not `*`."""
+    assert Settings.model_fields["cors_origins"].default == ""
diff --git a/docker-compose.yml b/docker-compose.yml
index 6144f95..5e278ad 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -23,7 +23,7 @@ services:
       AURUM_POSTGRES_DB: ${AURUM_POSTGRES_DB:-aurum}
       AURUM_POSTGRES_HOST: db
       AURUM_POSTGRES_PORT: 5432
-      AURUM_CORS_ORIGINS: ${AURUM_CORS_ORIGINS:-*}
+      AURUM_CORS_ORIGINS: ${AURUM_CORS_ORIGINS:-}
       AURUM_DEFAULT_CURRENCY: ${AURUM_DEFAULT_CURRENCY:-USD}
       # Free CoinGecko Demo key (coingecko.com/en/api/pricing) — the Crypto
       # tab's price sync 400s with a clear message until this is set.