From 5096e320fe5c3b422c9e3e4911bdb032d21fbad1 Mon Sep 17 00:00:00 2001 From: vince Date: Sat, 8 Aug 2026 23:58:34 +0200 Subject: [PATCH] docs: tighten voice filename safety plan --- docs/superpowers/plans/2026-08-08-voice-studio.md | 10 ++++++---- .../specs/2026-08-08-voice-studio-design.md | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/superpowers/plans/2026-08-08-voice-studio.md b/docs/superpowers/plans/2026-08-08-voice-studio.md index 2099ff5..5359c57 100644 --- a/docs/superpowers/plans/2026-08-08-voice-studio.md +++ b/docs/superpowers/plans/2026-08-08-voice-studio.md @@ -32,7 +32,7 @@ **Interfaces:** - Produces: `VoiceStore.list() -> list[VoiceMetadata]`, `VoiceStore.save(filename: str, payload: bytes) -> VoiceMetadata`, `VoiceStore.delete(name: str) -> None`, and `VoiceStore.resolve(name: str) -> Path`. -- Produces: `GET /v1/voices`, `POST /v1/voices`, and `DELETE /v1/voices/{name}`. +- Produces: `GET /v1/voices`, `POST /v1/voices`, and `DELETE /v1/voices/{name:path}` so encoded separators reach the validator and return 400. - [ ] **Step 1: Write failing voice-management tests** @@ -40,7 +40,7 @@ def test_list_voices_returns_safe_wav_metadata(client, voices_dir): write_wav(voices_dir / "warm voice.wav") payload = client.get("/v1/voices").json() - assert payload["data"][0]["name"] == "warm-voice.wav" + assert payload["data"][0]["name"] == "warm voice.wav" assert payload["data"][0]["sample_rate"] == 24000 def test_upload_rejects_non_wav_and_oversized_files(client): @@ -49,7 +49,9 @@ def test_upload_rejects_non_wav_and_oversized_files(client): def test_upload_and_delete_never_escape_voice_directory(client): assert client.post("/v1/voices", files={"file": ("../../bad.wav", valid_wav_bytes())}).status_code == 400 + assert client.post("/v1/voices", files={"file": ("..%2Fbad.wav", valid_wav_bytes())}).status_code == 400 assert client.delete("/v1/voices/../reference.wav").status_code == 400 + assert client.delete("/v1/voices/%2E%2E%2Freference.wav").status_code == 400 ``` - [ ] **Step 2: Run the new tests and verify RED** @@ -60,7 +62,7 @@ Expected: FAIL because the voice-store module and endpoints do not exist. - [ ] **Step 3: Implement the safe store and endpoints** -Use `Path(filename).name` as a starting point, reject an empty/path-bearing name, normalize displayed/stored names to a safe ASCII `a-z`, `0-9`, `-`, `_`, `.` set, and preserve only the `.wav` suffix. In the endpoint, read the upload then reject payloads over the exact byte cap before calling `save()`. Validate nonempty WAV headers/frames with `wave.open(BytesIO(payload))`. Write first to a unique file inside `VOICES_DIR`, then atomically rename it to the final path after rechecking containment and collision. `list()` must skip symlinks/non-WAV files and return sorted metadata. `delete()` must resolve containment and refuse missing/non-regular files. +Reject an empty upload name and any name containing `/`, `\\`, percent-encoded path separators/traversal, or a path component that differs from its basename; never strip a path-like name into a valid filename. For accepted plain filenames, normalize new storage names to safe ASCII `a-z`, `0-9`, `-`, `_`, `.` characters and preserve only the `.wav` suffix. Listing returns the stored basename unchanged, escaping only at the browser rendering boundary. In the endpoint, read the upload then reject payloads over the exact byte cap before calling `save()`. Validate nonempty WAV headers/frames with `wave.open(BytesIO(payload))`. Write first to a unique file inside `VOICES_DIR`, then atomically rename it to the final path after rechecking containment and collision. `list()` must skip symlinks/non-WAV files and return sorted metadata. `delete()` must URL-decode once, then resolve containment and refuse encoded traversal, missing, and non-regular files. ```python @app.post("/v1/voices", status_code=201) @@ -68,7 +70,7 @@ async def upload_voice(file: UploadFile) -> dict[str, VoiceMetadata]: payload = await file.read() return {"data": voice_store.save(file.filename or "", payload)} -@app.delete("/v1/voices/{name}", status_code=204) +@app.delete("/v1/voices/{name:path}", status_code=204) def delete_voice(name: str) -> Response: voice_store.delete(name) return Response(status_code=204) diff --git a/docs/superpowers/specs/2026-08-08-voice-studio-design.md b/docs/superpowers/specs/2026-08-08-voice-studio-design.md index dc8e52c..e04fe26 100644 --- a/docs/superpowers/specs/2026-08-08-voice-studio-design.md +++ b/docs/superpowers/specs/2026-08-08-voice-studio-design.md @@ -31,11 +31,13 @@ its local-network exposure. Voice files continue to live exclusively under `VOICES_DIR`. - `GET /v1/voices` returns safe metadata for each managed file: relative name, - byte size, duration, sample rate, and channel count. + byte size, duration, sample rate, and channel count. It preserves existing + stored filenames rather than renaming them during listing. - `POST /v1/voices` accepts a multipart `file` upload. The server sanitizes - the filename for storage and display, rejects paths and collisions, accepts - WAV only in this first release, validates that the decoded audio is nonempty, - and saves inside `VOICES_DIR` only. + the filename for storage and display, rejects any path-like name (including + encoded traversal) and collisions, accepts WAV only in this first release, + validates that the decoded audio is nonempty, and saves inside `VOICES_DIR` + only. - `DELETE /v1/voices/{name}` deletes one validated, regular file below `VOICES_DIR`; it rejects traversal, absolute paths, symlink escapes, and missing files.