Skip to content

Commit 9398f13

Browse files
chipiclaude
andcommitted
fix(security): outbox path-injection — hash the envelope id for the filename (CodeQL)
CodeQL py/path-injection (HIGH) failed PR #1441 at app_outbox_store.py:55/60/63: `envelope_id` arrives as a request path param (POST /internal/outbox/{id}/status) and flowed into the outbox filesystem path. The old char-strip sanitizer was a silent strip (and CodeQL didn't recognize it as a barrier). Fix: derive the filename from sha256(envelope_id) — the tainted string never reaches the path (a hex digest can't carry a separator), which both removes the injection surface and is a CodeQL-recognized taint sanitizer. Dedupe is preserved (same id → same hash → same file); list_pending reads the real id from file contents, so it's unaffected. Test: a traversal-style id ("../../../etc/passwd") stays inside outbox/ and round-trips by id. 16 outbox tests pass; black/flake8/mypy green. (The Snyk-Debian base-image CVEs on the PR are pre-existing infra, not this branch — that check passed; only CodeQL failed, on this finding.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019FDfbEedtYuS9PiqFoCtgK
1 parent bb0904d commit 9398f13

2 files changed

Lines changed: 21 additions & 3 deletions

File tree

src/podcast_scraper/server/app_outbox_store.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
from __future__ import annotations
2222

23+
import hashlib
2324
import json
2425
import time
2526
from pathlib import Path
@@ -45,9 +46,13 @@ def _outbox_dir(data_dir: Path) -> Path:
4546

4647

4748
def _envelope_path(data_dir: Path, envelope_id: str) -> Path:
48-
# ``id`` is app-generated (dgst_/ndg_ + hex), never raw request input; still guard traversal.
49-
safe = "".join(c for c in envelope_id if c.isalnum() or c in "._-")
50-
return _outbox_dir(data_dir) / f"{safe}.json"
49+
# The filename is a HASH of the id, never the id itself. ``envelope_id`` arrives as a request
50+
# path param (POST /internal/outbox/{id}/status), so hashing removes the path-injection surface
51+
# entirely (a hex digest can't carry a separator; CodeQL py/path-injection). Dedupe still holds:
52+
# same id → same hash → same file. The real id lives inside the file, so ``list_pending`` (which
53+
# reads file contents, not names) is unaffected.
54+
digest = hashlib.sha256(envelope_id.encode("utf-8")).hexdigest()
55+
return _outbox_dir(data_dir) / f"{digest}.json"
5156

5257

5358
def _lock(data_dir: Path, envelope_id: str) -> FileLock:

tests/unit/podcast_scraper/server/test_app_outbox_store.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,16 @@ def test_pending_excludes_expired_offset_form(tmp_path: Path) -> None:
139139
_enable_digest(tmp_path)
140140
app_outbox_store.enqueue(tmp_path, _envelope(expires_at="2000-01-01T00:00:00+00:00"))
141141
assert app_outbox_store.list_pending(tmp_path, channel="email", now=10**9) == []
142+
143+
144+
def test_path_injection_id_stays_in_outbox(tmp_path: Path) -> None:
145+
# A traversal-style id (worker path param) can't escape the outbox dir — the filename is a hash.
146+
_enable_digest(tmp_path)
147+
evil = "../../../etc/passwd"
148+
app_outbox_store.enqueue(tmp_path, _envelope(eid=evil))
149+
outbox = tmp_path / "outbox"
150+
files = list(outbox.glob("*.json"))
151+
assert len(files) == 1 # written inside outbox/, nowhere else
152+
assert not (tmp_path.parent / "passwd").exists()
153+
# and it still round-trips by the original id
154+
assert app_outbox_store.record_status(tmp_path, evil, "delivered") == "delivered"

0 commit comments

Comments
 (0)