Put diagnostic capture retrieval behind the same auth as everything else
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 <noreply@anthropic.com>
This commit is contained in:
parent
7d559f47f2
commit
7590d53b13
4 changed files with 83 additions and 20 deletions
|
|
@ -43,22 +43,6 @@ def version():
|
||||||
return {"commit": SOURCE_COMMIT, "started_at": STARTED_AT}
|
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)
|
@router.post("/login", status_code=status.HTTP_204_NO_CONTENT)
|
||||||
def login(credentials: LoginRequest, request: Request, response: Response):
|
def login(credentials: LoginRequest, request: Request, response: Response):
|
||||||
presented_token = request.headers.get("x-chart-token", "")
|
presented_token = request.headers.get("x-chart-token", "")
|
||||||
|
|
|
||||||
|
|
@ -1,9 +1,11 @@
|
||||||
|
import json
|
||||||
import base64
|
import base64
|
||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
import time
|
import time
|
||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
|
from fastapi.responses import FileResponse
|
||||||
from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response
|
from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response
|
||||||
from pydantic import BaseModel, Field
|
from pydantic import BaseModel, Field
|
||||||
|
|
||||||
|
|
@ -11,7 +13,7 @@ from app.bars.models import Timeframe
|
||||||
from app.analysis.levels import Side
|
from app.analysis.levels import Side
|
||||||
from app.analysis.manual_lines import ManualLine
|
from app.analysis.manual_lines import ManualLine
|
||||||
from app.api.deps import require_token
|
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
|
# Everything here needs the token when CHART_AUTH_TOKEN is set. /health and
|
||||||
# /version live in app.api.meta and stay open on purpose.
|
# /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)
|
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")
|
@router.get("/drawings")
|
||||||
def drawings(request: Request):
|
def drawings(request: Request):
|
||||||
"""Every drawing, comments included, for the sidebar list."""
|
"""Every drawing, comments included, for the sidebar list."""
|
||||||
|
|
|
||||||
|
|
@ -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
|
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
|
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.
|
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.
|
||||||
|
|
|
||||||
|
|
@ -174,7 +174,7 @@ def test_writes_are_protected(client):
|
||||||
assert client("s3cret").post("/api/lines", json=payload).status_code == 401
|
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
|
client, tmp_path, monkeypatch
|
||||||
):
|
):
|
||||||
monkeypatch.setattr(captures, "CAPTURE_DIR", tmp_path / "captures")
|
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
|
assert response.status_code == 201
|
||||||
saved = response.json()
|
saved = response.json()
|
||||||
assert saved["id"].startswith("c-")
|
assert saved["id"].startswith("c-")
|
||||||
assert probe.get(saved["url"]).content == image
|
# A capture is a picture of somebody's screen. The id is unguessable, but a
|
||||||
details = probe.get(saved["metadata_url"]).json()
|
# 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["id"] == saved["id"]
|
||||||
assert details["timeframe"] == "30m"
|
assert details["timeframe"] == "30m"
|
||||||
assert details["viewport_width"] == 1440
|
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"])
|
@pytest.mark.parametrize("path", ["/api/health", "/api/version"])
|
||||||
def test_meta_endpoints_stay_open(client, path):
|
def test_meta_endpoints_stay_open(client, path):
|
||||||
"""bin/wait-deploy polls /api/version without carrying the token."""
|
"""bin/wait-deploy polls /api/version without carrying the token."""
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue