docs: tighten voice filename safety plan

This commit is contained in:
vince 2026-08-08 23:58:34 +02:00
parent 710139443c
commit 5096e320fe
2 changed files with 12 additions and 8 deletions

View file

@ -32,7 +32,7 @@
**Interfaces:** **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: `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** - [ ] **Step 1: Write failing voice-management tests**
@ -40,7 +40,7 @@
def test_list_voices_returns_safe_wav_metadata(client, voices_dir): def test_list_voices_returns_safe_wav_metadata(client, voices_dir):
write_wav(voices_dir / "warm voice.wav") write_wav(voices_dir / "warm voice.wav")
payload = client.get("/v1/voices").json() 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 assert payload["data"][0]["sample_rate"] == 24000
def test_upload_rejects_non_wav_and_oversized_files(client): 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): 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": ("../../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/../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** - [ ] **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** - [ ] **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 ```python
@app.post("/v1/voices", status_code=201) @app.post("/v1/voices", status_code=201)
@ -68,7 +70,7 @@ async def upload_voice(file: UploadFile) -> dict[str, VoiceMetadata]:
payload = await file.read() payload = await file.read()
return {"data": voice_store.save(file.filename or "", payload)} 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: def delete_voice(name: str) -> Response:
voice_store.delete(name) voice_store.delete(name)
return Response(status_code=204) return Response(status_code=204)

View file

@ -31,11 +31,13 @@ its local-network exposure.
Voice files continue to live exclusively under `VOICES_DIR`. Voice files continue to live exclusively under `VOICES_DIR`.
- `GET /v1/voices` returns safe metadata for each managed file: relative name, - `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 - `POST /v1/voices` accepts a multipart `file` upload. The server sanitizes
the filename for storage and display, rejects paths and collisions, accepts the filename for storage and display, rejects any path-like name (including
WAV only in this first release, validates that the decoded audio is nonempty, encoded traversal) and collisions, accepts WAV only in this first release,
and saves inside `VOICES_DIR` only. validates that the decoded audio is nonempty, and saves inside `VOICES_DIR`
only.
- `DELETE /v1/voices/{name}` deletes one validated, regular file below - `DELETE /v1/voices/{name}` deletes one validated, regular file below
`VOICES_DIR`; it rejects traversal, absolute paths, symlink escapes, and `VOICES_DIR`; it rejects traversal, absolute paths, symlink escapes, and
missing files. missing files.