Deep-review round 2, on round 1's own fix commit. It found MORE than round 1 did (28 raw, against 13), and most of it was mine -- a new piece of UI attracts findings a backend-only diff never does. The pattern in almost all of them is the same: a principle applied to the site that prompted it and not to its siblings. * `display_text` reached three of the FIVE writers of a user-supplied name. `POST /api/downloads` and `POST /api/downloads/edit` still handed Postgres an unbounded string, so a 300-character name pasted into the Add-download dialog or the editor was a bare 500 (main.py re-raises every DataError but the NUL case). Four of the seven reviewers found it independently. The bound now lives in `models.py`, the three columns are DECLARED with it, and the guard for the next writer is parsed rather than remembered: every `display_name` / `display_uploader` assignment under `app/routes/` must go through the helper. That guard immediately found two more -- the Google sign-in callback writes the profile name into the same 255-wide column -- so it is six, not five. * The connection release reached one of the THREE routes that read their own body. The screenshot route (10 MiB, from a phone) and the client log sink both still held a pooled connection idle-in-transaction for the length of the upload, which is the invariant db.py sizes the pool on. All three now call `db.release_read_connection`, whose precondition -- nothing pending on the shared session -- is CHECKED rather than described, because a whole-session rollback discarding a caller's staged write is a mistake this repo has already made once. `test_body_limit.py` holds the rule for the next such route. * The AST drift registry keyed its sites by (module, limit expression), so a second route copied from the first collapsed onto the same entry and went uncovered -- the exact hole the registry exists to close. Keyed per call site now, and it carries the release requirement too. * The dialog's own refusal was announced once and never again: two over-cap picks leave an identical node with identical text, and an untouched live region says nothing. It is keyed per pick now, the drop zone reads as invalid and points at the sentence, and the reason is reachable from the control rather than only from a one-shot announcement. * Two tests rebuilt the blind spot they were written to remove: the dialog's cap test sat nowhere near the boundary (a `>` loosened to `>=` survived it, which is the mutation round 1 fixed on the server side), and the header-precedence test only covered an exact-case duplicate while a differently-cased one would have been COMBINED onto the wire rather than replaced. Both fixed, and the merge now dedupes case-insensitively because header names are. Also: the upload dialog's unstubbed usage query was firing a real request from jsdom in a neighbouring suite and leaking a delayed "connection lost" toast across test boundaries; `GET /api/downloads/usage` had no test at all though the whole client-side check hangs off it; and the comment claiming the edge 413 is readable now says what was measured -- readable to 200 MiB over loopback, a bare `onerror` through a proxy -- so the pre-send check is named as the mechanism and that branch as the backstop it is. Every test added or changed here was mutation-checked. One mutation SURVIVED and is recorded rather than papered over: the `tooBig` term inside `submit()` cannot be reached, because `ui/Button` swallows the activation itself, so it is marked as belt and braces instead of being described as the guard.
419 lines
18 KiB
Python
419 lines
18 KiB
Python
"""Debug sessions (E16): Start → reproduce → Stop, the screenshots, and the evidence under review.
|
|
|
|
The session itself — what tags a request, its cap, its level — is `app/debugsession.py`. These routes
|
|
are the user's handle on their OWN sessions only: another user's id 404s (`owned_or_404`), including
|
|
for an admin, who reads everyone's lines in the diagnostics console instead. The shared demo account
|
|
cannot record: its log would interleave every visitor under one account.
|
|
|
|
Screenshots arrive as a raw image body (`image/png`, `image/jpeg`, `image/webp`), not a multipart
|
|
form — one image per request, capped while it arrives, and kept only if its bytes really are one of
|
|
those types, whatever the Content-Type claimed.
|
|
"""
|
|
|
|
import json
|
|
from typing import Any
|
|
|
|
from fastapi import APIRouter, Depends, HTTPException, Request, Response
|
|
from pydantic import BaseModel, Field
|
|
from sqlalchemy import select
|
|
from sqlalchemy.exc import IntegrityError
|
|
from sqlalchemy.orm import Session
|
|
|
|
from app import audit, debugsession, logs, logsink
|
|
from app.audit import AuditAction
|
|
from app.auth import require_human
|
|
from app.db import get_db, release_read_connection
|
|
from app.issues import bundle, gitea
|
|
from app.ratelimit import RateLimiter
|
|
from app.refusal import refused
|
|
from app.models import DebugScreenshot, DebugSession, User
|
|
from app.routes._common import capped_body, issue_refused, not_found, owned_or_404
|
|
from app.utils import iso
|
|
|
|
router = APIRouter(prefix="/api/debug-sessions", tags=["diagnostics"])
|
|
log = logs.get_logger(__name__)
|
|
|
|
# Sessions listed in Settings → Diagnosztika, newest first: enough to reach last week's unsent draft.
|
|
LIST_MAX = 20
|
|
|
|
|
|
class StartIn(BaseModel):
|
|
level: debugsession.SessionLevel
|
|
|
|
|
|
_view = debugsession.view
|
|
|
|
# New sessions a user may open per hour (joining the running one is not counted): a session is a
|
|
# place to keep screenshots, and this — with the per-user storage cap — bounds how many there are.
|
|
_new_sessions = RateLimiter(max_events=20, window_seconds=3600)
|
|
|
|
|
|
@router.post("")
|
|
def start_session(
|
|
body: StartIn,
|
|
user: User = Depends(require_human),
|
|
db: Session = Depends(get_db),
|
|
) -> dict:
|
|
"""Start recording, or join the recording already running (another tab started it)."""
|
|
joining = debugsession.open_session(db, user) is not None
|
|
if not joining and not _new_sessions.allow(str(user.id)):
|
|
raise refused(429, "debug_session_rate_limited", limit=_new_sessions.max_events)
|
|
try:
|
|
session = debugsession.start(db, user, body.level)
|
|
db.commit()
|
|
except IntegrityError:
|
|
# A concurrent Start won the one-open-per-user index: join that one.
|
|
db.rollback()
|
|
session = debugsession.open_session(db, user)
|
|
if session is None:
|
|
raise refused(409, "debug_session_conflict") from None
|
|
# The rest of THIS request is part of the session too.
|
|
level, until = debugsession.level_for(user, session)
|
|
logs.set_user(user.id, level, until, session.id)
|
|
log.info(
|
|
"debug session started",
|
|
extra=logs.event("debug.session.started", level=session.level, until=iso(session.until)),
|
|
)
|
|
return {"ok": True, "session": _view(db, session)}
|
|
|
|
|
|
@router.post("/{sid}/stop")
|
|
def stop_session(
|
|
sid: str, user: User = Depends(require_human), db: Session = Depends(get_db)
|
|
) -> dict:
|
|
session = owned_or_404(db, DebugSession, sid, user, what="debug_session")
|
|
if session.ended_at is None:
|
|
# Logged BEFORE the session closes, while this request still carries its tag: the last line
|
|
# of the evidence is the Stop itself.
|
|
log.info("debug session stopped", extra=logs.event("debug.session.stopped"))
|
|
debugsession.stop(session)
|
|
db.commit()
|
|
logs.end_session_elevation(session.id)
|
|
return {"ok": True, "session": _view(db, session)}
|
|
|
|
|
|
@router.get("")
|
|
def list_sessions(user: User = Depends(require_human), db: Session = Depends(get_db)) -> dict:
|
|
sessions = db.scalars(
|
|
select(DebugSession)
|
|
.where(DebugSession.user_id == user.id)
|
|
.order_by(DebugSession.started_at.desc())
|
|
.limit(LIST_MAX)
|
|
).all()
|
|
return {"items": [_view(db, s) for s in sessions]}
|
|
|
|
|
|
@router.get("/{sid}")
|
|
def get_session(
|
|
sid: str, user: User = Depends(require_human), db: Session = Depends(get_db)
|
|
) -> dict:
|
|
return _view(db, owned_or_404(db, DebugSession, sid, user, what="debug_session"))
|
|
|
|
|
|
@router.get("/{sid}/events")
|
|
def session_events(
|
|
sid: str, user: User = Depends(require_human), db: Session = Depends(get_db)
|
|
) -> dict:
|
|
"""The rows an issue from this session would carry — browser and server, in time order."""
|
|
session = owned_or_404(db, DebugSession, sid, user, what="debug_session")
|
|
rows, total = debugsession.bundle(db, session)
|
|
return {"items": [logsink.serialize(r) for r in rows], "total": total}
|
|
|
|
|
|
# --- the draft and its export ---------------------------------------------------------------
|
|
# The composer's own fields (steps, description…) are its business, kept whole so it reopens as it
|
|
# was — bounded here so a draft cannot become a place to store anything else.
|
|
FORM_MAX_BYTES = 64 * 1024
|
|
# Zips a user may build per minute: each holds every screenshot in memory while it is built.
|
|
_exports = RateLimiter(max_events=6, window_seconds=60)
|
|
|
|
|
|
class DraftIn(BaseModel):
|
|
title: str = Field(max_length=200)
|
|
# The rendered markdown the export and the issue carry.
|
|
body: str = Field(max_length=50_000)
|
|
# Log rows taken out of the evidence in the review.
|
|
excluded: list[int] = Field(default_factory=list, max_length=debugsession.BUNDLE_MAX)
|
|
form: dict[str, Any] = Field(default_factory=dict)
|
|
|
|
|
|
@router.put("/{sid}/draft")
|
|
def save_draft(
|
|
sid: str,
|
|
body: DraftIn,
|
|
user: User = Depends(require_human),
|
|
db: Session = Depends(get_db),
|
|
) -> dict:
|
|
session = owned_or_404(db, DebugSession, sid, user, what="debug_session")
|
|
if len(json.dumps(body.form, ensure_ascii=False).encode()) > FORM_MAX_BYTES:
|
|
raise HTTPException(status_code=413, detail={"code": "too_large"})
|
|
# Under the lock, and refused while a send is under way: the issue is being built from the draft
|
|
# as it was saved, and this would change it under the send.
|
|
debugsession.lock(db, session)
|
|
if debugsession.sending(session):
|
|
raise refused(409, "issue_sending")
|
|
if session.issue_url is not None:
|
|
# Sent: the issue is on the tracker now, and a draft edited after it would say otherwise.
|
|
raise refused(409, "issue_already_sent")
|
|
debugsession.save_draft(session, body.model_dump())
|
|
db.commit()
|
|
return {"ok": True, "session": _view(db, session)}
|
|
|
|
|
|
@router.get("/{sid}/export")
|
|
def export_session(
|
|
sid: str, user: User = Depends(require_human), db: Session = Depends(get_db)
|
|
) -> Response:
|
|
"""The issue as a zip — the text, the two logs and the screenshots — for an instance without an
|
|
issue tracker, or for sending it by hand. Built from the SAVED draft: what the composer shows is
|
|
what it saved."""
|
|
session = owned_or_404(db, DebugSession, sid, user, what="debug_session")
|
|
if not _exports.allow(str(user.id)):
|
|
raise refused(429, "export_rate_limited")
|
|
data = bundle.zipped(bundle.files(db, session))
|
|
name = f"siftlode-debug-{session.started_at:%Y%m%d-%H%M}-{session.id[:6]}.zip"
|
|
return Response(
|
|
content=data,
|
|
media_type="application/zip",
|
|
headers={
|
|
"Content-Disposition": f'attachment; filename="{name}"',
|
|
"Cache-Control": "no-store",
|
|
},
|
|
)
|
|
|
|
|
|
# --- the issue tracker (S4) -----------------------------------------------------------------
|
|
# Issues a user may open per hour — counted once the tracker is ASKED to open one (an answer that
|
|
# goes missing may still have created it, so an attempt cannot be given back); finishing a half-sent
|
|
# one, or an attempt refused before the tracker was asked, is not counted. The composer sends one at
|
|
# a time; this is room for a bad afternoon, not a pace anything legitimate keeps up.
|
|
_new_issues = RateLimiter(max_events=5, window_seconds=3600)
|
|
# Attempts on the tracker per hour, creating or finishing. Deliberately NOT the budget above: a
|
|
# half-sent issue must always be finishable, so the pace that guards creating one must never be what
|
|
# stands between a user and their own report. This one only bounds a loop — every attempt re-reads
|
|
# the whole bundle into memory and re-offers the pending file to the tracker, and a retry that is
|
|
# refused for good (the tracker's own limits) never leaves `pending`. No hand reaches it.
|
|
_sends = RateLimiter(max_events=60, window_seconds=3600)
|
|
# The composer's live preview, rendered BY the tracker: one authenticated call each, with our token.
|
|
# The composer's toggle fires it once per switch INTO preview — the editor is the other branch, so
|
|
# no keystroke can change the query key while the preview is up — and react-query's cache makes an
|
|
# unchanged draft free. (It does NOT debounce, whatever this comment used to say: a key that changes
|
|
# per keystroke is one fetch per keystroke, measured. Showing the preview BESIDE the fields would
|
|
# need a real debounce first.) This bounds what a loop can aim at the tracker.
|
|
_previews = RateLimiter(max_events=60, window_seconds=60)
|
|
|
|
|
|
def _issues_rate_limited() -> HTTPException:
|
|
return refused(429, "issues_rate_limited", limit=_new_issues.max_events)
|
|
|
|
|
|
@router.post("/{sid}/issue")
|
|
def send_issue(
|
|
sid: str, user: User = Depends(require_human), db: Session = Depends(get_db)
|
|
) -> dict:
|
|
"""Send the session's issue to the tracker — from the SAVED draft, as the export is built — or
|
|
finish a send that stopped half-way (the issue exists; only what is missing is sent)."""
|
|
session = owned_or_404(db, DebugSession, sid, user, what="debug_session")
|
|
# The session is CLAIMED for the send rather than locked through it: the tracker is slow (a
|
|
# dozen calls, 20 s each), and a row lock held across them would queue this user's Stop, their
|
|
# autosave and their screenshots behind it. They see the claim instead and are refused at once.
|
|
debugsession.lock(db, session)
|
|
if debugsession.sending(session):
|
|
raise refused(409, "issue_sending")
|
|
new = session.issue_url is None
|
|
if not new and not debugsession.unfinished(session):
|
|
return {"ok": True, "session": _view(db, session)}
|
|
key = str(user.id)
|
|
if new:
|
|
if not ((session.draft or {}).get("title") or "").strip():
|
|
raise refused(422, "issue_title_required")
|
|
if _new_issues.exhausted(key):
|
|
raise _issues_rate_limited()
|
|
if not _sends.allow(key):
|
|
raise refused(429, "issue_rate_limited", limit=_sends.max_events)
|
|
token = debugsession.claim_send(session)
|
|
db.commit() # the claim is what everything else sees; the lock ends here
|
|
|
|
def count_new_issue() -> None:
|
|
if not _new_issues.allow(key):
|
|
raise _issues_rate_limited()
|
|
|
|
try:
|
|
gitea.deliver(
|
|
db,
|
|
session,
|
|
token=token,
|
|
checkpoint=db.commit,
|
|
before_create=count_new_issue,
|
|
on_created=lambda: _record_created(db, user, session),
|
|
)
|
|
except debugsession.SendClaimLost:
|
|
# This send outlived its lease and another one took the session over while it was still
|
|
# talking to the tracker. It stopped rather than writing over that one's progress — and a
|
|
# send IS under way, just not ours, which is what the caller needs to hear.
|
|
raise refused(409, "issue_sending") from None
|
|
except gitea.GiteaError as exc:
|
|
# Keep what did happen: an issue created before an upload failed stays linked, and the
|
|
# retry does only what is still missing.
|
|
raise issue_refused(exc) from None
|
|
except HTTPException:
|
|
raise # the pace, refused before the tracker was asked
|
|
finally:
|
|
_end_send(db, session, token)
|
|
return {"ok": True, "session": _view(db, session)}
|
|
|
|
|
|
def _end_send(db: Session, session: DebugSession, token: str) -> None:
|
|
"""Give the claim back, whatever else happened. A failure here must not replace the failure that
|
|
brought us: a dead transaction (the database went away mid-send) is rolled back, and the claim
|
|
then simply ages out."""
|
|
try:
|
|
debugsession.lock(db, session)
|
|
debugsession.release_send(session, token)
|
|
db.commit()
|
|
except Exception: # noqa: BLE001 — whatever went wrong here, the send's own failure is the news
|
|
db.rollback()
|
|
log.warning(
|
|
"a send could not give its claim back",
|
|
extra=logs.event("issues.claim.stuck", session_id=session.id),
|
|
)
|
|
|
|
|
|
def _record_created(db: Session, user: User, session: DebugSession) -> None:
|
|
"""The trail for the one act that takes a user's log off this instance. Staged inside the send,
|
|
so the checkpoint that first writes `issue_url` commits BOTH — recorded later it could be lost
|
|
on its own (the process dies mid-send), and no retry could write it afterwards: the next send
|
|
sees the link and knows the issue is not new."""
|
|
audit.record(
|
|
db,
|
|
user.id,
|
|
AuditAction.ISSUE_CREATE,
|
|
target_type="debug_session",
|
|
target_id=session.id,
|
|
summary=session.issue_url or "",
|
|
)
|
|
|
|
|
|
class PreviewIn(BaseModel):
|
|
text: str = Field(max_length=50_000)
|
|
|
|
|
|
@router.post("/{sid}/preview")
|
|
def preview_issue(
|
|
sid: str,
|
|
body: PreviewIn,
|
|
user: User = Depends(require_human),
|
|
db: Session = Depends(get_db),
|
|
) -> dict:
|
|
"""The body as the tracker will render it. Gitea sanitises its own output; the composer still
|
|
shows it in a sandboxed frame, never in the app's own document."""
|
|
owned_or_404(db, DebugSession, sid, user, what="debug_session")
|
|
if not _previews.allow(str(user.id)):
|
|
raise refused(429, "preview_rate_limited")
|
|
try:
|
|
return {"ok": True, "html": gitea.preview(db, body.text)}
|
|
except gitea.GiteaError as exc:
|
|
raise issue_refused(exc) from None
|
|
|
|
|
|
# --- screenshots ---------------------------------------------------------------------------
|
|
def _shot_view(shot: DebugScreenshot) -> dict:
|
|
return {"id": shot.id, "mime": shot.mime, "size": shot.size, "created_at": iso(shot.created_at)}
|
|
|
|
|
|
@router.get("/{sid}/screenshots")
|
|
def list_screenshots(
|
|
sid: str, user: User = Depends(require_human), db: Session = Depends(get_db)
|
|
) -> dict:
|
|
session = owned_or_404(db, DebugSession, sid, user, what="debug_session")
|
|
return {"items": [_shot_view(s) for s in debugsession.screenshots(db, session)]}
|
|
|
|
|
|
def _upload_target(
|
|
sid: str, user: User = Depends(require_human), db: Session = Depends(get_db)
|
|
) -> DebugSession:
|
|
return owned_or_404(db, DebugSession, sid, user, what="debug_session")
|
|
|
|
|
|
async def _image_body(request: Request, db: Session = Depends(get_db)) -> bytes:
|
|
# The owner check above has already run, and it was a SELECT — so this session is holding a
|
|
# pooled connection `idle in transaction` and would hold it for the whole 10 MiB upload, from a
|
|
# client that may be on a phone. Hand it back first; the handler's own work re-takes one. The
|
|
# `session` object the handler holds is expired by this and reloads on its next attribute read,
|
|
# which is the correct moment to touch the database again.
|
|
release_read_connection(db)
|
|
return await capped_body(request, debugsession.SCREENSHOT_MAX_BYTES)
|
|
|
|
|
|
@router.post("/{sid}/screenshots")
|
|
def add_screenshot(
|
|
# The owner check FIRST: dependencies resolve in order, so a stranger's upload is refused
|
|
# before a byte of it is read.
|
|
session: DebugSession = Depends(_upload_target),
|
|
data: bytes = Depends(_image_body),
|
|
db: Session = Depends(get_db),
|
|
) -> dict:
|
|
try:
|
|
shot = debugsession.add_screenshot(db, session, data)
|
|
except debugsession.ScreenshotRefused as exc:
|
|
raise _screenshot_refused(exc.code) from None
|
|
db.commit()
|
|
return {"ok": True, "screenshot": _shot_view(shot)}
|
|
|
|
|
|
def _screenshot_refused(code: str) -> HTTPException:
|
|
# Literal reasons, one per case: the frontend's contract test reads them to require a sentence
|
|
# for each in both languages.
|
|
if code == "already_sent":
|
|
return refused(409, "screenshot_already_sent")
|
|
if code == "too_many":
|
|
return refused(422, "screenshot_too_many", limit=debugsession.SCREENSHOTS_MAX)
|
|
if code == "sending":
|
|
return refused(409, "issue_sending")
|
|
if code == "storage_full":
|
|
mb = debugsession.SCREENSHOT_USER_MAX_BYTES // (1024 * 1024)
|
|
return refused(422, "screenshot_storage_full", limit=mb)
|
|
if code == "empty":
|
|
return refused(422, "screenshot_empty")
|
|
return refused(422, "screenshot_not_an_image")
|
|
|
|
|
|
def _session_screenshot(db: Session, sid: str, shot_id: int, user: User) -> DebugScreenshot:
|
|
return _screenshot_of(
|
|
db, owned_or_404(db, DebugSession, sid, user, what="debug_session"), shot_id
|
|
)
|
|
|
|
|
|
def _screenshot_of(db: Session, session: DebugSession, shot_id: int) -> DebugScreenshot:
|
|
shot = db.get(DebugScreenshot, shot_id)
|
|
if shot is None or shot.session_id != session.id:
|
|
raise not_found("screenshot")
|
|
return shot
|
|
|
|
|
|
@router.get("/{sid}/screenshots/{shot_id}")
|
|
def get_screenshot(
|
|
sid: str, shot_id: int, user: User = Depends(require_human), db: Session = Depends(get_db)
|
|
) -> Response:
|
|
shot = _session_screenshot(db, sid, shot_id, user)
|
|
# `nosniff`: the type was checked against the bytes at upload, and the browser must not guess.
|
|
return Response(
|
|
content=shot.data,
|
|
media_type=shot.mime,
|
|
headers={"Cache-Control": "private, max-age=3600", "X-Content-Type-Options": "nosniff"},
|
|
)
|
|
|
|
|
|
@router.delete("/{sid}/screenshots/{shot_id}")
|
|
def delete_screenshot(
|
|
sid: str, shot_id: int, user: User = Depends(require_human), db: Session = Depends(get_db)
|
|
) -> dict:
|
|
session = owned_or_404(db, DebugSession, sid, user, what="debug_session")
|
|
shot = _screenshot_of(db, session, shot_id)
|
|
try:
|
|
debugsession.remove_screenshot(db, session, shot)
|
|
except debugsession.ScreenshotRefused as exc:
|
|
raise _screenshot_refused(exc.code) from None
|
|
db.commit()
|
|
return {"ok": True, "deleted": shot_id}
|