fix: blank inbox previews, two unauthenticated 500s, and the missing social tests
- Every conversation row in the inbox rendered an empty preview: the frontend read `last_body`, which the API has never sent. It sends `excerpt` plus `truncated`. - `GET /api/codex?limit=-1` passed a negative LIMIT to Postgres -> 500. Now bounded by Query(ge=1, le=200) -> 422. - Registering with a >255 char email hit the VARCHAR(255) column and surfaced as a 500. PATCH /api/profile already validated this; only /auth/register was open. The social layer had zero tests, despite routes/messages.py citing "tests/test_messages.py's cross-user leak tests" and routes/profile.py claiming "privacy rules that the tests pin". Neither file existed. Both now do: auth required on every route, email only on /api/profile/me, no id or password_hash on any public payload, third-party thread reads empty, payload-supplied sender inert, veiled profiles 404 and stay off the roster. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Indiana
@@ -3,7 +3,7 @@ contacted, shared across all users (spec §4)."""
|
||||
|
||||
import uuid
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, status
|
||||
from fastapi import APIRouter, Depends, HTTPException, Query, status
|
||||
from sqlalchemy import desc, func, select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
@@ -126,7 +126,9 @@ def _entity_card(entity: Entity, discoverer: str | None) -> dict:
|
||||
async def list_codex(
|
||||
rarity: str | None = None,
|
||||
sort: str = "recent",
|
||||
limit: int = 60,
|
||||
# Bounded at the edge: an unvalidated `?limit=-1` reached Postgres as a
|
||||
# negative LIMIT and returned a 500 to an unauthenticated caller.
|
||||
limit: int = Query(default=60, ge=1, le=200),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
query = select(Entity)
|
||||
|
||||
@@ -7,7 +7,9 @@ from pydantic import BaseModel, ConfigDict, Field
|
||||
class RegisterRequest(BaseModel):
|
||||
username: str = Field(min_length=3, max_length=32)
|
||||
password: str = Field(min_length=8, max_length=128)
|
||||
email: str | None = None
|
||||
# users.email is VARCHAR(255): without a cap here an over-long address
|
||||
# reached Postgres and surfaced as a 500 instead of a validation error.
|
||||
email: str | None = Field(default=None, max_length=255)
|
||||
|
||||
|
||||
class UserOut(BaseModel):
|
||||
|
||||
131
backend/tests/test_messages.py
Normal file
131
backend/tests/test_messages.py
Normal file
@@ -0,0 +1,131 @@
|
||||
"""Cross-user leak and impersonation pins for hunter-to-hunter whispers.
|
||||
|
||||
routes/messages.py's docstring points at this file ("See
|
||||
tests/test_messages.py's cross-user leak tests") — it did not exist.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
async def _register(client, username, password="spookyspooky"):
|
||||
resp = await client.post(
|
||||
"/auth/register", json={"username": username, "password": password}
|
||||
)
|
||||
assert resp.status_code == 201, resp.text
|
||||
|
||||
|
||||
async def _login(client, username, password="spookyspooky"):
|
||||
resp = await client.post(
|
||||
"/auth/login", json={"username": username, "password": password}
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"method,path",
|
||||
[("get", "/api/messages"), ("get", "/api/messages/someone")],
|
||||
)
|
||||
async def test_message_reads_require_auth(client, method, path):
|
||||
assert (await getattr(client, method)(path)).status_code == 401
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_requires_auth(client):
|
||||
resp = await client.post("/api/messages", json={"to": "anyone", "body": "hi"})
|
||||
assert resp.status_code == 401
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_third_party_cannot_read_a_thread(client):
|
||||
await _register(client, "alice")
|
||||
await _register(client, "bob")
|
||||
await _register(client, "mallory")
|
||||
|
||||
await _login(client, "alice")
|
||||
sent = await client.post("/api/messages", json={"to": "bob", "body": "the cellar is cold"})
|
||||
assert sent.status_code == 201
|
||||
|
||||
# Mallory asks for the thread with each participant: the query is pinned
|
||||
# to (caller, other) in both directions, so neither leg can match a
|
||||
# message between two other people.
|
||||
await _login(client, "mallory")
|
||||
for name in ("alice", "bob"):
|
||||
thread = await client.get(f"/api/messages/{name}")
|
||||
assert thread.status_code == 200
|
||||
assert thread.json()["messages"] == []
|
||||
assert (await client.get("/api/messages")).json()["conversations"] == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sender_is_always_the_session_user(client):
|
||||
await _register(client, "alice2")
|
||||
await _register(client, "bob2")
|
||||
await _register(client, "mallory2")
|
||||
|
||||
# An extra `from`/`sender` in the body must be inert — sender_id comes
|
||||
# from the cookie, never the payload.
|
||||
await _login(client, "mallory2")
|
||||
resp = await client.post(
|
||||
"/api/messages",
|
||||
json={"to": "bob2", "body": "trust me", "from": "alice2", "sender": "alice2"},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
|
||||
await _login(client, "bob2")
|
||||
thread = (await client.get("/api/messages/alice2")).json()
|
||||
assert thread["messages"] == []
|
||||
thread = (await client.get("/api/messages/mallory2")).json()
|
||||
assert [m["body"] for m in thread["messages"]] == ["trust me"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_body_length_is_capped_and_empty_rejected(client):
|
||||
await _register(client, "alice3")
|
||||
await _register(client, "bob3")
|
||||
await _login(client, "alice3")
|
||||
|
||||
assert (
|
||||
await client.post("/api/messages", json={"to": "bob3", "body": " "})
|
||||
).status_code == 400
|
||||
assert (
|
||||
await client.post("/api/messages", json={"to": "bob3", "body": "x" * 1001})
|
||||
).status_code == 400
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_is_rate_limited(client, monkeypatch):
|
||||
from app.routes import messages as messages_module
|
||||
from app.rate_limit import RateLimiter
|
||||
|
||||
monkeypatch.setattr(
|
||||
messages_module, "send_limiter", RateLimiter(max_requests=2, window_seconds=3600)
|
||||
)
|
||||
await _register(client, "alice4")
|
||||
await _register(client, "bob4")
|
||||
await _login(client, "alice4")
|
||||
|
||||
for _ in range(2):
|
||||
assert (
|
||||
await client.post("/api/messages", json={"to": "bob4", "body": "again"})
|
||||
).status_code == 201
|
||||
assert (
|
||||
await client.post("/api/messages", json={"to": "bob4", "body": "again"})
|
||||
).status_code == 429
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_conversation_row_carries_an_excerpt(client):
|
||||
# The inbox preview key is `excerpt` (+ `truncated`); the frontend read a
|
||||
# nonexistent `last_body` and rendered blank previews. Pin the name.
|
||||
await _register(client, "alice5")
|
||||
await _register(client, "bob5")
|
||||
await _login(client, "alice5")
|
||||
await client.post("/api/messages", json={"to": "bob5", "body": "z" * 300})
|
||||
|
||||
row = (await client.get("/api/messages")).json()["conversations"][0]
|
||||
assert row["excerpt"] == "z" * 160
|
||||
assert row["truncated"] is True
|
||||
assert row["hunter"]["username"] == "bob5"
|
||||
assert "email" not in row["hunter"]
|
||||
assert "id" not in row["hunter"]
|
||||
125
backend/tests/test_profile.py
Normal file
125
backend/tests/test_profile.py
Normal file
@@ -0,0 +1,125 @@
|
||||
"""Privacy and authorization pins for the hunter profile layer.
|
||||
|
||||
The module docstring of routes/profile.py promises three things that are
|
||||
easy to regress and were never actually tested: email appears on
|
||||
/api/profile/me and nowhere else, a veiled profile is indistinguishable
|
||||
from one that never existed, and editing is authenticated.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
async def _register(client, username, password="spookyspooky", **extra):
|
||||
resp = await client.post(
|
||||
"/auth/register", json={"username": username, "password": password, **extra}
|
||||
)
|
||||
assert resp.status_code == 201, resp.text
|
||||
return resp.json()
|
||||
|
||||
|
||||
async def _login(client, username, password="spookyspooky"):
|
||||
resp = await client.post(
|
||||
"/auth/login", json={"username": username, "password": password}
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_profile_me_requires_auth(client):
|
||||
assert (await client.get("/api/profile/me")).status_code == 401
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_profile_patch_requires_auth(client):
|
||||
resp = await client.patch("/api/profile", json={"display_name": "nobody"})
|
||||
assert resp.status_code == 401
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_email_only_on_own_profile(client):
|
||||
await _register(client, "seer_a", email="seer@example.com")
|
||||
await _login(client, "seer_a")
|
||||
|
||||
mine = (await client.get("/api/profile/me")).json()
|
||||
assert mine["email"] == "seer@example.com"
|
||||
|
||||
public = (await client.get("/api/hunters/seer_a")).json()
|
||||
assert "email" not in public
|
||||
assert "password_hash" not in public
|
||||
assert "id" not in public
|
||||
|
||||
roster = (await client.get("/api/hunters")).json()["hunters"]
|
||||
for hunter in roster:
|
||||
assert "email" not in hunter
|
||||
assert "password_hash" not in hunter
|
||||
assert "id" not in hunter
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_patch_cannot_touch_another_hunter(client):
|
||||
await _register(client, "seer_b")
|
||||
await _register(client, "seer_c")
|
||||
await _login(client, "seer_b")
|
||||
|
||||
# There is no user selector on PATCH at all — the payload is applied to
|
||||
# the session's own row, so a username in the body is inert.
|
||||
resp = await client.patch(
|
||||
"/api/profile", json={"username": "seer_c", "display_name": "impostor"}
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["username"] == "seer_b"
|
||||
|
||||
victim = (await client.get("/api/hunters/seer_c")).json()
|
||||
assert victim["display_name"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_veiled_profile_is_404_and_absent_from_roster(client):
|
||||
await _register(client, "seer_d")
|
||||
await _login(client, "seer_d")
|
||||
assert (await client.patch("/api/profile", json={"profile_public": False})).status_code == 200
|
||||
|
||||
assert (await client.get("/api/hunters/seer_d")).status_code == 404
|
||||
roster = (await client.get("/api/hunters")).json()["hunters"]
|
||||
assert all(h["username"] != "seer_d" for h in roster)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_text_fields_are_capped(client):
|
||||
await _register(client, "seer_e")
|
||||
await _login(client, "seer_e")
|
||||
|
||||
assert (
|
||||
await client.patch("/api/profile", json={"display_name": "x" * 49})
|
||||
).status_code == 422
|
||||
assert (await client.patch("/api/profile", json={"bio": "y" * 281})).status_code == 422
|
||||
assert (await client.patch("/api/profile", json={"avatar_hue": 400})).status_code == 422
|
||||
assert (await client.patch("/api/profile", json={"gender": "wraith"})).status_code == 422
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_markup_in_profile_text_is_stored_verbatim_not_interpreted(client):
|
||||
# The API is a JSON API: it stores exactly what was sent. The XSS defence
|
||||
# is that every consumer renders it as a text node (see MessagesPage /
|
||||
# HunterPage). Pinned so nobody "helpfully" starts emitting HTML here.
|
||||
await _register(client, "seer_f")
|
||||
await _login(client, "seer_f")
|
||||
payload = "<img src=x onerror=alert(1)>"
|
||||
resp = await client.patch("/api/profile", json={"bio": payload})
|
||||
assert resp.json()["bio"] == payload
|
||||
assert (await client.get("/api/hunters/seer_f")).json()["bio"] == payload
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_register_rejects_absurdly_long_email(client):
|
||||
# users.email is VARCHAR(255); without a schema cap this reached the
|
||||
# database and came back as a 500 instead of a validation error.
|
||||
resp = await client.post(
|
||||
"/auth/register",
|
||||
json={
|
||||
"username": "seer_g",
|
||||
"password": "spookyspooky",
|
||||
"email": "a" * 300 + "@example.com",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 422
|
||||
@@ -28,7 +28,11 @@ type Hunter = {
|
||||
}
|
||||
type Conversation = {
|
||||
hunter: Hunter
|
||||
last_body: string
|
||||
// GET /api/messages calls this `excerpt` (first 160 chars) with a
|
||||
// `truncated` flag beside it. This type used to say `last_body`, which the
|
||||
// API has never sent — every inbox preview rendered as an empty string.
|
||||
excerpt: string
|
||||
truncated: boolean
|
||||
last_at: string
|
||||
last_from_me: boolean
|
||||
unread: number
|
||||
@@ -132,7 +136,8 @@ function Inbox() {
|
||||
</span>
|
||||
<span className="msg-row-last dim">
|
||||
{c.last_from_me ? t('messages.youPrefix') : ''}
|
||||
{c.last_body}
|
||||
{c.excerpt}
|
||||
{c.truncated ? '…' : ''}
|
||||
</span>
|
||||
</span>
|
||||
{c.unread > 0 && (
|
||||
|
||||
Reference in New Issue
Block a user