From e7ad034a5596deb77124b9828f0d5f358688cdb1 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Fri, 3 Jul 2026 15:45:40 -0600 Subject: [PATCH] =?UTF-8?q?fix(speakers):=20serve=5Faudio=20404s=20in=20pr?= =?UTF-8?q?od=20=E2=80=94=20resolve=20media=20under=20chronicle=20day=20di?= =?UTF-8?q?r?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit serve_audio built paths under the journal ROOT (state.journal_root//...) instead of the chronicle day dir (journal/chronicle//...), so every request 404'd in production. Its correct sibling serve_file (transcripts) had a divergent, hand-rolled containment idiom; that divergence is what let the bug hide. Extract one day-scoped containment helper safe_day_path(day, rel_path) in solstone/convey/utils.py, sibling to safe_journal_path. It contains a request rel_path under day_path(day), symlink-aware via contained_path, returning the standard INVALID_PATH envelope at HTTP 403 on any escape. Route both serve_audio and serve_file through it, fixing the base bug and removing the duplicated containment logic. Tests rework serve_audio to exercise the real journal root with chronicle-only media and no flat-day symlink, add safe_day_path unit tests including a date-like first-segment containment guard, and strengthen the serve_file traversal assertion to 403 + invalid_path. Co-Authored-By: Claude Opus 4.8 (1M context) --- solstone/apps/speakers/routes.py | 41 +++----- solstone/apps/speakers/tests/test_routes.py | 93 +++++++++++-------- solstone/apps/transcripts/routes.py | 41 +++----- .../apps/transcripts/tests/test_serve_file.py | 3 +- solstone/convey/utils.py | 18 +++- tests/test_convey_utils.py | 60 ++++++++++++ 6 files changed, 159 insertions(+), 97 deletions(-) diff --git a/solstone/apps/speakers/routes.py b/solstone/apps/speakers/routes.py index a167137ac..a3722c0d6 100644 --- a/solstone/apps/speakers/routes.py +++ b/solstone/apps/speakers/routes.py @@ -11,7 +11,6 @@ from __future__ import annotations import json import logging -import os import re from datetime import date from pathlib import Path @@ -69,15 +68,12 @@ from solstone.apps.speakers.suggest import format_suggestions, suggest_opportuni from solstone.apps.speakers.time import segment_start_ts_ms from solstone.apps.speakers.wipe import wipe_speaker_artifacts from solstone.apps.utils import log_app_action -from solstone.convey import state from solstone.convey.reasons import ( ENTITY_BLOCKED, ENTITY_NOT_FOUND, FILE_NOT_FOUND, - FILE_READ_FAILED, INVALID_DAY, INVALID_MONTH, - INVALID_PATH, INVALID_REQUEST_VALUE, INVALID_SEGMENT_OR_STREAM, MISSING_REQUEST_BODY, @@ -92,7 +88,13 @@ from solstone.convey.reasons import ( SPEAKER_SENTENCE_MISSING, SPEAKER_VOICEPRINT_BUSY, ) -from solstone.convey.utils import DATE_RE, error_response, format_date, success_response +from solstone.convey.utils import ( + DATE_RE, + error_response, + format_date, + safe_day_path, + success_response, +) from solstone.think.awareness import get_current, owner_detection_ready from solstone.think.entities import find_matching_entity from solstone.think.entities.journal import ( @@ -1926,28 +1928,9 @@ def serve_audio(day: str, rel_path: str) -> Any: """Serve audio files for playback.""" if not DATE_RE.fullmatch(day): return error_response(INVALID_DAY, detail="Day not found", status=404) - - full_path = os.path.join(state.journal_root, day, rel_path) - try: - base = Path(state.journal_root, day).resolve() - candidate = Path(full_path).resolve() - except (OSError, ValueError): - logger.warning( - "serve_audio path resolution failed for %s/%s", - day, - rel_path, - exc_info=True, - ) - return error_response( - FILE_READ_FAILED, detail="Failed to serve file", status=404 - ) - - try: - candidate.relative_to(base) - except ValueError: - return error_response(INVALID_PATH, detail="Invalid file path", status=403) - - if not os.path.isfile(full_path): + path, error = safe_day_path(day, rel_path) + if error is not None: + return error + if not path.is_file(): return error_response(FILE_NOT_FOUND, detail="File not found") - - return send_file(full_path, mimetype="audio/flac") + return send_file(path, mimetype="audio/flac") diff --git a/solstone/apps/speakers/tests/test_routes.py b/solstone/apps/speakers/tests/test_routes.py index 8bc988440..c45560b2d 100644 --- a/solstone/apps/speakers/tests/test_routes.py +++ b/solstone/apps/speakers/tests/test_routes.py @@ -7,8 +7,44 @@ import json from datetime import datetime import numpy as np +import pytest from flask import Flask +SERVE_AUDIO_DAY = "20240101" +SERVE_AUDIO_STREAM = "test" +SERVE_AUDIO_SEGMENT = "143022_300" +SERVE_AUDIO_SOURCE = "mic_audio" +SERVE_AUDIO_URL = ( + f"/app/speakers/api/serve_audio/{SERVE_AUDIO_DAY}/" + f"{SERVE_AUDIO_STREAM}/{SERVE_AUDIO_SEGMENT}/{SERVE_AUDIO_SOURCE}.flac" +) + + +@pytest.fixture +def serve_audio_client(tmp_path, monkeypatch): + from solstone.convey import create_app + + journal = tmp_path / "journal" + config_dir = journal / "config" + config_dir.mkdir(parents=True) + (config_dir / "journal.json").write_text( + json.dumps({"setup": {"completed_at": 1700000000000}}) + "\n", + encoding="utf-8", + ) + segment_dir = ( + journal + / "chronicle" + / SERVE_AUDIO_DAY + / SERVE_AUDIO_STREAM + / SERVE_AUDIO_SEGMENT + ) + segment_dir.mkdir(parents=True) + (segment_dir / f"{SERVE_AUDIO_SOURCE}.flac").write_bytes(b"fLaC") + monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal)) + + app = create_app(str(journal)) + return app.test_client(), journal + def _read_action_entries(journal_root): """Read journal-level action log entries for today.""" @@ -695,61 +731,38 @@ def test_discovery_identify_route_is_idempotent_after_success( assert second.get_json()["voiceprints_saved"] == 0 -def test_serve_audio_sets_flac_mimetype(speakers_env, monkeypatch): +def test_serve_audio_sets_flac_mimetype(serve_audio_client): """Serve audio endpoint returns FLAC mimetype for sample playback.""" - from solstone.apps.speakers.routes import speakers_bp - from solstone.convey import state + client, _journal = serve_audio_client - env = speakers_env() - env.create_segment("20240101", "143022_300", ["mic_audio"]) - monkeypatch.setattr(state, "journal_root", str(env.journal / "chronicle")) + response = client.get(SERVE_AUDIO_URL) - app = Flask(__name__) - app.register_blueprint(speakers_bp) - - with app.test_client() as client: - response = client.get( - "/app/speakers/api/serve_audio/20240101/test/143022_300/mic_audio.flac" - ) - assert response.status_code == 200 - assert response.mimetype == "audio/flac" + assert response.status_code == 200 + assert response.mimetype == "audio/flac" -def test_serve_audio_path_traversal_is_forbidden(speakers_env, monkeypatch): +def test_serve_audio_path_traversal_is_forbidden(serve_audio_client): """A rel_path resolving to a real file outside the day dir is refused 403.""" - from solstone.apps.speakers.routes import speakers_bp - from solstone.convey import state - - env = speakers_env() - monkeypatch.setattr(state, "journal_root", str(env.journal / "chronicle")) - chronicle = env.journal / "chronicle" - (chronicle / "20240101").mkdir(parents=True, exist_ok=True) + client, journal = serve_audio_client + chronicle = journal / "chronicle" # Real file OUTSIDE the day dir, reachable via ../ from it. (chronicle / "leak.flac").write_bytes(b"secret") - app = Flask(__name__) - app.register_blueprint(speakers_bp) + response = client.get( + f"/app/speakers/api/serve_audio/{SERVE_AUDIO_DAY}/../leak.flac" + ) - with app.test_client() as client: - response = client.get("/app/speakers/api/serve_audio/20240101/../leak.flac") - assert response.status_code == 403 - assert response.get_json()["reason_code"] == "invalid_path" + assert response.status_code == 403 + assert response.get_json()["reason_code"] == "invalid_path" -def test_serve_audio_malformed_day_returns_404(speakers_env, monkeypatch): +def test_serve_audio_malformed_day_returns_404(serve_audio_client): """A day segment that doesn't match the YYYYMMDD regex returns 404.""" - from solstone.apps.speakers.routes import speakers_bp - from solstone.convey import state + client, _journal = serve_audio_client - env = speakers_env() - monkeypatch.setattr(state, "journal_root", str(env.journal / "chronicle")) + response = client.get("/app/speakers/api/serve_audio/notadate/foo") - app = Flask(__name__) - app.register_blueprint(speakers_bp) - - with app.test_client() as client: - response = client.get("/app/speakers/api/serve_audio/notadate/foo") - assert response.status_code == 404 + assert response.status_code == 404 def test_confirm_attribution_rejects_escaping_stream(speakers_env): diff --git a/solstone/apps/transcripts/routes.py b/solstone/apps/transcripts/routes.py index 90a220d20..cff33bbab 100644 --- a/solstone/apps/transcripts/routes.py +++ b/solstone/apps/transcripts/routes.py @@ -39,13 +39,18 @@ from solstone.convey.reasons import ( INVALID_DAY, INVALID_MONTH, INVALID_OPERATION_FOR_STATE, - INVALID_PATH, INVALID_REQUEST_VALUE, INVALID_SEGMENT_OR_STREAM, OPERATION_NO_LONGER_AVAILABLE, RAW_MEDIA_NOT_AVAILABLE, ) -from solstone.convey.utils import DATE_RE, error_response, format_date, success_response +from solstone.convey.utils import ( + DATE_RE, + error_response, + format_date, + safe_day_path, + success_response, +) from solstone.observe.hear import format_audio from solstone.observe.screen import format_screen from solstone.observe.utils import AUDIO_EXTENSIONS, VIDEO_EXTENSIONS @@ -297,31 +302,15 @@ def serve_file(day: str, rel_path: str) -> Any: """Serve actual media files for embedding.""" if not DATE_RE.fullmatch(day): return error_response(INVALID_DAY, status=404, detail="Day not found") - - try: - day_dir = day_path(day, create=False).resolve() - full_path = (day_dir / rel_path).resolve() - if os.path.commonpath([str(full_path), str(day_dir)]) != str(day_dir): - return error_response(INVALID_PATH, status=403, detail="Invalid file path") - if not full_path.is_file(): - return error_response(FILE_NOT_FOUND, detail="File not found") - except (OSError, ValueError): - logger.warning( - "serve_file path validation failed for %s/%s", - day, - rel_path, - exc_info=True, - ) - return error_response( - FILE_READ_FAILED, status=404, detail="Failed to serve file" - ) - - mimetype = MIME_TYPES.get(full_path.suffix.lower()) + path, error = safe_day_path(day, rel_path) + if error is not None: + return error + if not path.is_file(): + return error_response(FILE_NOT_FOUND, detail="File not found") + mimetype = MIME_TYPES.get(path.suffix.lower()) if mimetype is None: - raise ValueError( - f"unregistered media extension for serve_file: {full_path.suffix}" - ) - return send_file(full_path, conditional=True, mimetype=mimetype) + raise ValueError(f"unregistered media extension for serve_file: {path.suffix}") + return send_file(path, conditional=True, mimetype=mimetype) @transcripts_bp.route("/api/stats/") diff --git a/solstone/apps/transcripts/tests/test_serve_file.py b/solstone/apps/transcripts/tests/test_serve_file.py index d4966f156..58ea786cf 100644 --- a/solstone/apps/transcripts/tests/test_serve_file.py +++ b/solstone/apps/transcripts/tests/test_serve_file.py @@ -50,7 +50,8 @@ def test_serve_file_path_traversal_returns_non_200(client): "/app/transcripts/api/serve_file/20240101/../../../etc/passwd" ) - assert response.status_code != 200 + assert response.status_code == 403 + assert response.get_json()["reason_code"] == "invalid_path" def test_serve_file_malformed_day_returns_404(client): diff --git a/solstone/convey/utils.py b/solstone/convey/utils.py index 462de1da7..0bac322f4 100644 --- a/solstone/convey/utils.py +++ b/solstone/convey/utils.py @@ -14,7 +14,7 @@ from typing import Any, Optional from flask import Response, jsonify from solstone.convey.reasons import INVALID_PATH, Reason -from solstone.think.journal_io import contained_path, get_journal +from solstone.think.journal_io import contained_path, day_path, get_journal DATE_RE = re.compile(r"\d{8}") _REQUEST_ID_ALPHABET = "0123456789ABCDEFGHJKMNPQRSTVWXYZ" @@ -273,6 +273,22 @@ def safe_journal_path(relpath: str) -> tuple[Path | None, tuple[Response, int] | return None, error_response(INVALID_PATH) +def safe_day_path( + day: str, rel_path: str +) -> tuple[Path | None, tuple[Response, int] | None]: + """Contain a request-supplied path at the chronicle-day HTTP boundary. + + Thin wrapper over contained_path: validates and contains under the chronicle + day directory, never writes. Rejections converge on the standard + INVALID_PATH envelope at 403. + """ + try: + base = day_path(day, create=False) + return contained_path(base, rel_path), None + except ValueError: + return None, error_response(INVALID_PATH, status=403) + + def error_response( reason: Reason, status: int | None = None, diff --git a/tests/test_convey_utils.py b/tests/test_convey_utils.py index e1064bd1c..c403ee27d 100644 --- a/tests/test_convey_utils.py +++ b/tests/test_convey_utils.py @@ -14,6 +14,7 @@ from solstone.convey.utils import ( format_month_day, relative_time, respond_collection, + safe_day_path, safe_journal_path, time_since, ) @@ -171,6 +172,17 @@ def _assert_invalid_path_error(error): } +def _assert_invalid_path_error_403(error): + assert error is not None + response, status = error + assert status == 403 + assert response.get_json() == { + "error": "I couldn't use that path.", + "reason_code": "invalid_path", + "detail": "", + } + + def test_safe_journal_path_accepts_contained_path(tmp_path, monkeypatch): monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) @@ -203,3 +215,51 @@ def test_safe_journal_path_rejects_symlink_escape(tmp_path, monkeypatch): assert path is None _assert_invalid_path_error(error) + + +def test_safe_day_path_accepts_contained_path(tmp_path, monkeypatch): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) + day = "20240101" + day_dir = day_path(day) + + path, error = safe_day_path(day, "test/143022_300/mic_audio.flac") + + assert error is None + assert path == day_dir / "test" / "143022_300" / "mic_audio.flac" + assert path.is_absolute() + + +def test_safe_day_path_rejects_invalid_relpaths(tmp_path, monkeypatch): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) + + with _app_context(): + for relpath in ("..", "../escape", "/etc/passwd", "a\\b", ""): + path, error = safe_day_path("20240101", relpath) + + assert path is None + _assert_invalid_path_error_403(error) + + +def test_safe_day_path_rejects_symlink_escape(tmp_path, monkeypatch): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) + day_dir = day_path("20240101") + outside = tmp_path.parent / f"{tmp_path.name}_outside" + outside.mkdir() + os.symlink(outside, day_dir / "out") + + with _app_context(): + path, error = safe_day_path("20240101", "out/secret.txt") + + assert path is None + _assert_invalid_path_error_403(error) + + +def test_safe_day_path_keeps_date_like_first_segment_contained(tmp_path, monkeypatch): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) + day_dir = day_path("20240101") + + path, error = safe_day_path("20240101", "20260101/foo.flac") + + assert error is None + assert path is not None + assert path.is_relative_to(day_dir) -- 2.51.2