From 7590d53b13ebd17fffab58ea2e7e5d0988354348 Mon Sep 17 00:00:00 2001 From: Chris Amow Date: Tue, 11 Aug 2026 17:24:04 -0500 Subject: [PATCH] Put diagnostic capture retrieval behind the same auth as everything else MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uploading a capture required a token; retrieving one did not. That was a deliberate capability-URL design with a test asserting it, and the reasoning held: it lets whoever is debugging fetch a capture without the chart password. Changed because of what a capture contains. getDisplayMedia returns a picture of someone's screen, and preferCurrentTab is a preference rather than a constraint, so a mis-click shares a different window. An unguessable id stops guessing but not leakage: capability URLs escape through proxy logs, browser history and pasted links. Retrieval now uses the dependency the rest of the API uses, which already accepts the session cookie — so a logged-in browser needs nothing extra, which was the condition for making this change at all. An agent on the server reads the capture directory directly; one working over HTTP sends the API token. Both handlers moved from meta.py to routes.py. meta.py is the deliberately open router — health, version, login, logout — and a screenshot endpoint did not belong there. The existing test now asserts 401 without credentials, and a new one covers the browser path: log in, then retrieve with only the cookie. Co-Authored-By: Claude Opus 5 --- app/api/meta.py | 16 ---------------- app/api/routes.py | 28 +++++++++++++++++++++++++++- docs/implementation.md | 23 +++++++++++++++++++++++ tests/test_auth.py | 36 +++++++++++++++++++++++++++++++++--- 4 files changed, 83 insertions(+), 20 deletions(-) diff --git a/app/api/meta.py b/app/api/meta.py index 12906df..193c928 100644 --- a/app/api/meta.py +++ b/app/api/meta.py @@ -43,22 +43,6 @@ def version(): return {"commit": SOURCE_COMMIT, "started_at": STARTED_AT} -@router.get("/debug/captures/{capture_id}") -def get_debug_capture(capture_id: str): - path = capture_path(capture_id, ".png") - if path is None: - raise HTTPException(404, "Capture not found") - return FileResponse(path, media_type="image/png") - - -@router.get("/debug/captures/{capture_id}/meta") -def get_debug_capture_metadata(capture_id: str): - path = capture_path(capture_id, ".json") - if path is None: - raise HTTPException(404, "Capture not found") - return json.loads(path.read_text(encoding="utf-8")) - - @router.post("/login", status_code=status.HTTP_204_NO_CONTENT) def login(credentials: LoginRequest, request: Request, response: Response): presented_token = request.headers.get("x-chart-token", "") diff --git a/app/api/routes.py b/app/api/routes.py index 389d73a..22b9acc 100644 --- a/app/api/routes.py +++ b/app/api/routes.py @@ -1,9 +1,11 @@ +import json import base64 import json import logging import time import uuid +from fastapi.responses import FileResponse from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response from pydantic import BaseModel, Field @@ -11,7 +13,7 @@ from app.bars.models import Timeframe from app.analysis.levels import Side from app.analysis.manual_lines import ManualLine from app.api.deps import require_token -from app.api.captures import CAPTURE_MAX_BYTES, save_capture +from app.api.captures import CAPTURE_MAX_BYTES, capture_path, save_capture # Everything here needs the token when CHART_AUTH_TOKEN is set. /health and # /version live in app.api.meta and stay open on purpose. @@ -288,6 +290,30 @@ def debug_snap(payload: SnapReport): return Response(status_code=204) +@router.get("/debug/captures/{capture_id}") +def get_debug_capture(capture_id: str): + """A diagnostic capture, behind the same auth as everything else. + + These are screenshots of somebody's screen. The id is 72 bits of entropy so + the URL is unguessable, but a capability URL still leaks through anything + that records URLs — proxy logs, browser history, a pasted link. Requiring a + session costs nothing: a browser already holds the cookie, and an API client + already sends the token. + """ + path = capture_path(capture_id, ".png") + if path is None: + raise HTTPException(404, "Capture not found") + return FileResponse(path, media_type="image/png") + + +@router.get("/debug/captures/{capture_id}/meta") +def get_debug_capture_metadata(capture_id: str): + path = capture_path(capture_id, ".json") + if path is None: + raise HTTPException(404, "Capture not found") + return json.loads(path.read_text(encoding="utf-8")) + + @router.get("/drawings") def drawings(request: Request): """Every drawing, comments included, for the sidebar list.""" diff --git a/docs/implementation.md b/docs/implementation.md index 4b5eb13..b984d0b 100644 --- a/docs/implementation.md +++ b/docs/implementation.md @@ -682,3 +682,26 @@ timeframe. Holding a daily value constant is correct when projecting it over intraday candles, but on the daily chart it made the SMA itself look like a staircase. MA line type is now timeframe-aware: stepped on intraday charts and simple point-to-point lines on `1d`, with both modes pinned by browser coverage. + +### 2026-08-11 — diagnostic captures moved behind auth + +The capture endpoints were split: uploading required a token, retrieving did +not. That was deliberate — an unguessable id acting as a capability URL, with a +test asserting it — and the reasoning was sound: it lets someone debugging fetch +a capture without holding the chart password. + +Changed anyway, because of what a capture is. `getDisplayMedia` returns a +picture of somebody's screen, and `preferCurrentTab` is a preference rather than +a constraint, so a mis-click shares a different window. 72 bits of entropy stops +guessing, but a capability URL still escapes through everything that records +URLs: proxy and access logs, browser history, a link pasted into a chat. + +Retrieval now uses the same dependency as the rest of the API, which already +accepts the session cookie — so a browser that is logged in needs nothing extra, +which was the requirement. An agent on the server reads the files directly from +the capture directory, and one working over HTTP presents the API token. Neither +path got harder, which is why the trade was worth making. + +Both handlers moved from `meta.py` to `routes.py`. `meta.py` is the deliberately +unauthenticated router — health, version, login and logout — and a screenshot +endpoint did not belong in that company. diff --git a/tests/test_auth.py b/tests/test_auth.py index ee59252..b094e92 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -174,7 +174,7 @@ def test_writes_are_protected(client): assert client("s3cret").post("/api/lines", json=payload).status_code == 401 -def test_diagnostic_capture_is_authenticated_and_retrievable_by_capability_id( +def test_a_capture_needs_auth_to_upload_and_to_retrieve( client, tmp_path, monkeypatch ): monkeypatch.setattr(captures, "CAPTURE_DIR", tmp_path / "captures") @@ -202,13 +202,43 @@ def test_diagnostic_capture_is_authenticated_and_retrievable_by_capability_id( assert response.status_code == 201 saved = response.json() assert saved["id"].startswith("c-") - assert probe.get(saved["url"]).content == image - details = probe.get(saved["metadata_url"]).json() + # A capture is a picture of somebody's screen. The id is unguessable, but a + # capability URL still escapes through anything that records URLs — proxy + # logs, browser history, a link pasted into a chat. Retrieval is behind the + # same auth as the rest of the API, which costs a browser nothing because it + # already holds the session cookie. + assert probe.get(saved["url"]).status_code == 401 + assert probe.get(saved["metadata_url"]).status_code == 401 + + assert probe.get(saved["url"], headers={"X-Chart-Token": "s3cret"}).content == image + details = probe.get(saved["metadata_url"], headers={"X-Chart-Token": "s3cret"}).json() assert details["id"] == saved["id"] assert details["timeframe"] == "30m" assert details["viewport_width"] == 1440 +def test_a_capture_is_retrievable_with_the_browser_session_cookie(client, tmp_path, monkeypatch): + # The path that has to stay effortless: the person debugging is already + # logged in, so viewing a capture must need nothing extra. + monkeypatch.setattr(captures, "CAPTURE_DIR", tmp_path / "captures") + probe = client("s3cret", password="friendly passphrase") + image = b"\x89PNG\r\n\x1a\ntrimmed-test-image" + metadata = base64.b64encode(json.dumps({"timeframe": "1m"}).encode()).decode() + saved = probe.post( + "/api/debug/captures", + content=image, + headers={ + "X-Chart-Token": "s3cret", + "Content-Type": "image/png", + "X-Capture-Metadata": metadata, + }, + ).json() + + assert probe.post("/api/login", json={"password": "friendly passphrase"}).status_code == 204 + # TestClient keeps the session cookie from here on. + assert probe.get(saved["url"]).content == image + + @pytest.mark.parametrize("path", ["/api/health", "/api/version"]) def test_meta_endpoints_stay_open(client, path): """bin/wait-deploy polls /api/version without carrying the token."""