Palash Debnath
18793a99a7
* feat(audiobook): durable crash-resume for interrupted longform renders
Chapter WAVs were already content-addressed (a re-run reused finished chapters),
but resume only worked if the user could re-submit the EXACT script — impossible
for Stories, whose plan is compiled from cast+lines. This persists the plan
itself so an interrupted render is resumable without the original input.
- New services/longform_resume.py (pure file/JSON): on render start, write a
resume.json manifest (compiled plan + render params + title) into the job work
dir, atomically; clear it on successful completion. read/has/clear/build
helpers, schema-versioned (a foreign/corrupt manifest is ignored, never
resumed).
- _render_longform_sse: accepts an optional job_id + resume flag (resume reuses
the original job row + cached chapters instead of creating a new one); writes
the manifest at start, clears it on done. Both front doors (/audiobook,
/longform/render) unchanged for callers.
- GET /audiobook/jobs — lists interrupted renders (running/failed longform jobs
that still have a manifest; a job left "running" across an app restart is
interrupted by definition), with title + total/done chapter counts for the UI.
- POST /audiobook/resume/{job_id} — rebuilds the plan from the manifest and
replays _render_longform_sse under the original job_id; the content-addressed
cache makes finished chapters instant, so only the unrendered ones synthesize.
404 on unknown id / missing manifest.
Resume durability is best-effort — a manifest failure never blocks the render.
The resume UI affordance is a follow-up (the endpoints are ready for it).
Tests: tests/test_longform_resume.py (7, pure manifest round-trip / version &
corrupt rejection / atomic write — monkeypatches OUTPUTS_DIR, no global
core.config stub so the shared tests/ session isn't polluted) +
backend/tests/test_audiobook_resume_api.py (6, config-stub: jobs-list with
progress, failed-included, done/manifestless/non-longform excluded, resume
404s). 13 passed. CJK green. Stale module docstring updated.
* fix(audiobook): confine resume paths — py/path-injection (CodeQL) + quality
The default-setup CodeQL (security-and-quality suite) flagged the crash-resume
work: longform_resume built filesystem paths from job_id, which on the
POST /audiobook/resume/{job_id} endpoint is a request-supplied path param →
py/path-injection (10 high-severity sinks: open/replace/remove/makedirs/isfile).
- longform_resume.work_dir now confines like profiles._voices_path: reject an
unknown job_type or an id that isn't a bare safe token (^[A-Za-z0-9_-]{1,64}$),
then realpath + startswith(OUTPUTS_DIR + os.sep) — a crafted id (`../`, NUL,
separators) can never escape OUTPUTS_DIR. Returns None on violation; all
callers (manifest_path/read/write/clear/has) degrade gracefully.
- The resume endpoint also gates the path-param id up front (404 on a bad
token) — barrier at the source as well as the sink.
Also cleared the quality alerts the same diff introduced:
- py/repeated-import: the 4 inline `from services import longform_resume` calls
collapse to one module-top import (it's pure, no torch).
- py/empty-except: the best-effort manifest blocks now logger.debug instead of
a bare `pass`.
13 resume tests still pass; all job ids in tests are safe tokens.
* fix(audiobook): sanitize resume job_id at the source (path + log injection)
The first CodeQL pass wasn't enough: resume made job_id request-controlled, so
it tainted not just the manifest paths but the EXISTING work-dir join and the
progress log lines too (py/path-injection + py/log-injection, ~14 alerts).
Fix at the source so the whole dataflow is clean:
- _render_longform_sse strips job_id to a safe token (`re.sub` removing anything
but [A-Za-z0-9_-], capped 64) right after it's resolved — no path separator,
no CR/LF can survive, whether the id came from the resume path param or a
fresh uuid.
- The work dir now routes through longform_resume.work_dir, which adds the
proven os.path.basename(seg)==seg barrier (the shape CodeQL accepts in
_voices_path) on top of the realpath+startswith confinement — so the join and
every path derived from it (meta/concat/out) is sanitized.
- The best-effort manifest-write log no longer interpolates the raw exception
(uses exc_info); clear_manifest's OSError handler returns instead of bare pass
(py/empty-except).
13 resume tests still pass.
* fix(audiobook): launder resume job_id via trusted FS scan (CodeQL path/log-injection)
The custom realpath/regex barriers weren't in CodeQL's recognized sanitizer set,
so the request-supplied resume job_id kept tainting the work-dir/manifest paths
and the progress logs. Switch to the pattern CodeQL does accept — launder the id
through a trusted filesystem enumeration:
- longform_resume.scan_resumable() lists resumable jobs by scanning OUTPUTS_DIR
for <type>_<id>/resume.json; every id it returns is sourced from os.listdir
(never request input).
- POST /audiobook/resume/{job_id} now only resumes an id that scan_resumable()
reports (membership match), and uses the (job_type, job_id) pair FROM that
trusted list for everything downstream — so nothing request-controlled reaches
a filesystem path or a log line.
- GET /audiobook/jobs lists from scan_resumable() too (filesystem-sourced ids).
work_dir keeps the realpath+startswith+basename confinement as genuine defense;
the render path's job_id is now always either a fresh uuid or a laundered id.
13 resume tests still pass.
* fix(audiobook): exact-match allowlist on the work-dir name (CodeQL path-injection)
The remaining 4 path-injection alerts were inside work_dir: I validated job_id
with an anchored regex but then joined a DIFFERENT f-string (`{job_type}_{job_id}`),
so CodeQL didn't carry the sanitization to the joined value. Mirror the pattern
the repo's _safe_cover_path uses (which CodeQL accepts): validate the WHOLE
joined component against an exact-match allowlist regex (_SAFE_SEG_RE), then
confine with os.path.commonpath containment (the recognized barrier) instead of
startswith. 13 resume tests still pass.
* fix(audiobook): basename-sanitize the work-dir name for CodeQL path-injection
The exact-match regex alone wasn't credited; route the joined value through os.path.basename() first — the sanitizer CodeQL recognizes (mirrors _safe_cover_path) — then the regex + commonpath. Functionally identical (no separator in the name) but clears the 4 remaining alerts. 13 tests pass.
* fix(audiobook): allow-list membership guard launders resume job_id (CodeQL)
The next(... if pair[1]==job_id) comparison-select didn't sanitize for CodeQL. Build a dict of resumable ids from the trusted scan and gate with 'if job_id not in resumable' — the membership barrier CodeQL recognizes — then use job_id directly downstream. 13 tests pass.
* fix(audiobook): eliminate request→path flow in resume (definitive CodeQL fix)
Five rounds of recognized path-injection barriers (regex, basename, exact-match,
commonpath, membership-guard) still left CodeQL flagging the resume job_id →
work-dir/manifest/log flow. Remove the flow entirely instead of guarding it:
- scan_resumable() now returns {job_type, job_id, manifest_path} where
manifest_path is built from the os.listdir dir name (trusted), plus
load_manifest_file(path) / discard_manifest_file(path) that operate on those
trusted paths. The request job_id is used ONLY to *select* a scan entry, never
to build a path.
- POST /audiobook/resume/{job_id} reads the manifest via the trusted scan path
and renders under a FRESH server uuid (job_id=None). The chapter cache is
content-addressed (keyed by chapter content, not the job id), so finished
chapters still hit instantly — resume works, but the request's id never names
a work dir, output file, or log line.
- The interrupted job's manifest is discarded (trusted path) once the fresh-id
resume kicks off, so it stops showing as resumable.
Net: no request-controlled value reaches any file operation or log on the
render path (job_id there is always a server uuid). work_dir keeps its
confinement barriers as defence-in-depth. 13 resume tests pass.
82 lines
3.0 KiB
Python
82 lines
3.0 KiB
Python
"""Pure tests for the durable-resume manifest (services.longform_resume).
|
|
|
|
Uses a monkeypatch fixture to point OUTPUTS_DIR at a tmp dir — deliberately NOT
|
|
a module-level ``sys.modules["core.config"]`` stub, which would pollute the rest
|
|
of the shared ``pytest tests/`` session (e.g. test_longform_e2e)."""
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
|
|
import pytest
|
|
|
|
from services import longform_resume as lr
|
|
|
|
_PLAN = [
|
|
{"title": "One", "spans": [
|
|
{"voice_id": "v1", "text": "hello", "pause_ms_after": 0, "speed": None}]},
|
|
{"title": "Two", "spans": [
|
|
{"voice_id": "v1", "text": "world", "pause_ms_after": 500, "speed": 0.9}]},
|
|
]
|
|
_PARAMS = {"default_voice": "v1", "fmt": "m4b", "bitrate": "128k",
|
|
"loudness": "acx", "cover_path": None, "metadata": {"title": "Bk"}, "lexicon": None}
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _tmp_outputs(tmp_path, monkeypatch):
|
|
import core.config as cfg
|
|
monkeypatch.setattr(cfg, "OUTPUTS_DIR", str(tmp_path), raising=False)
|
|
return tmp_path
|
|
|
|
|
|
def test_build_manifest_shape():
|
|
m = lr.build_manifest(job_id="abc", job_type="audiobook", plan_chapters=_PLAN,
|
|
params=_PARAMS, title="Bk")
|
|
assert m["version"] == lr.MANIFEST_VERSION
|
|
assert m["job_id"] == "abc" and m["job_type"] == "audiobook"
|
|
assert m["total_chapters"] == 2 and m["title"] == "Bk"
|
|
assert m["plan"] == _PLAN and m["params"] == _PARAMS
|
|
|
|
|
|
def test_write_read_roundtrip():
|
|
m = lr.build_manifest(job_id="rt", job_type="story", plan_chapters=_PLAN,
|
|
params=_PARAMS, title="S")
|
|
path = lr.write_manifest(m)
|
|
assert path and os.path.isfile(path)
|
|
assert lr.has_manifest("story", "rt") is True
|
|
assert lr.read_manifest("story", "rt") == m
|
|
|
|
|
|
def test_clear_manifest():
|
|
lr.write_manifest(lr.build_manifest(job_id="cl", job_type="audiobook",
|
|
plan_chapters=_PLAN, params=_PARAMS))
|
|
assert lr.has_manifest("audiobook", "cl")
|
|
lr.clear_manifest("audiobook", "cl")
|
|
assert not lr.has_manifest("audiobook", "cl")
|
|
lr.clear_manifest("audiobook", "cl") # idempotent, no raise
|
|
|
|
|
|
def test_read_missing_returns_none():
|
|
assert lr.read_manifest("audiobook", "nope") is None
|
|
assert lr.has_manifest("audiobook", "nope") is False
|
|
|
|
|
|
def test_read_rejects_wrong_version():
|
|
m = lr.build_manifest(job_id="ver", job_type="audiobook", plan_chapters=_PLAN, params=_PARAMS)
|
|
m["version"] = 999
|
|
lr.write_manifest(m)
|
|
assert lr.read_manifest("audiobook", "ver") is None # foreign schema → not resumed
|
|
|
|
|
|
def test_read_rejects_corrupt_json():
|
|
path = lr.manifest_path("audiobook", "corrupt")
|
|
os.makedirs(os.path.dirname(path), exist_ok=True)
|
|
with open(path, "w") as f:
|
|
f.write("{ not json")
|
|
assert lr.read_manifest("audiobook", "corrupt") is None
|
|
|
|
|
|
def test_atomic_write_leaves_no_tmp():
|
|
lr.write_manifest(lr.build_manifest(job_id="atom", job_type="story",
|
|
plan_chapters=_PLAN, params=_PARAMS))
|
|
assert not any(n.endswith(".tmp") for n in os.listdir(lr.work_dir("story", "atom")))
|