mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-11 03:09:57 +00:00
fix(career): path-traversal guard on prepare; a cap test that actually tests the cap
CodeRabbit again, and the first one is a real hole I put there. 1. PATH TRAVERSAL. sloppak.resolve_source_dir() does a bare `dlc_root / filename` with NO containment guard — so `../../x` walks straight out of the library, and my new endpoint handed it attacker-supplied filenames. Every filename now goes through _resolve_dlc_path first, the same check every other filename-bound handler applies. Pinned: `..`, backslash traversal, an absolute POSIX path and a Windows drive path are all refused, and nothing outside the library is unpacked. 2. THE CAP TEST WAS VACUOUS. It asserted `prepared == 0` against a fixture with no library — where the endpoint exits before extraction — so it passed whether or not MAX_GIG_SONGS existed. It now runs against a real library and asserts the endpoint CONSIDERED at most MAX_GIG_SONGS of the 82 it was handed. Verified to fail when the cap is removed. Same class of mistake as the notedetect gigBlock: a test that passes for the wrong reason. Worth saying out loud since it is twice in one day. 3. E702 — semicolon-joined statements in the new tests, split. 51 career tests; full suite green.
This commit is contained in:
@@ -47,6 +47,7 @@ from fastapi import Body, HTTPException
|
||||
from fastapi.responses import FileResponse
|
||||
|
||||
import sloppak
|
||||
from dlc_paths import _resolve_dlc_path
|
||||
from progression import instrument_for_arrangement
|
||||
|
||||
PLUGIN_ID = "career"
|
||||
@@ -774,10 +775,20 @@ def setup(app, context):
|
||||
if dlc_root is None or cache_root is None:
|
||||
return {"ok": False, "prepared": 0, "failed": files, "error": "no library"}
|
||||
|
||||
root = Path(dlc_root)
|
||||
prepared, failed = 0, []
|
||||
for fn in files:
|
||||
# CONTAINMENT FIRST. resolve_source_dir() does a bare
|
||||
# `dlc_root / filename` with no guard, so a crafted `../..` would
|
||||
# walk straight out of the library. Every other filename-bound
|
||||
# handler validates through _resolve_dlc_path; so does this one.
|
||||
safe = _resolve_dlc_path(root, fn)
|
||||
if safe is None:
|
||||
_state["log"].warning("career: gig pre-extract rejected unsafe path %r", fn)
|
||||
failed.append(fn)
|
||||
continue
|
||||
try:
|
||||
sloppak.resolve_source_dir(fn, Path(dlc_root), Path(cache_root))
|
||||
sloppak.resolve_source_dir(fn, root, Path(cache_root))
|
||||
prepared += 1
|
||||
except Exception as exc: # noqa: BLE001 — one bad pak can't sink the set
|
||||
_state["log"].warning("career: gig pre-extract failed for %s: %s", fn, exc)
|
||||
|
||||
Reference in New Issue
Block a user