Skip to content

Commit 7a39c64

Browse files
committed
fix(security): constrain preview cache metadata paths
Keep preview-cache metadata scoped to each cache directory and reject metadata-loaded entries whose files resolve outside that directory before cleanup can unlink them.
1 parent b7a1670 commit 7a39c64

3 files changed

Lines changed: 80 additions & 21 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ record also lives in the git commit messages.
2828
metadata, and the C2PA claim-generator string so release smoke fails when
2929
those public version surfaces drift.
3030

31+
### Security — cache safety
32+
33+
- Preview-cache metadata is now scoped to the active cache directory and ignores
34+
metadata entries whose files resolve outside that directory before eviction,
35+
invalidation, or flush can delete them.
36+
3137
## [1.33.0] — 2026-06-11 — June hardening & quality pass
3238

3339
### Fixed — Docker/runtime

opencut/core/preview_cache.py

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from typing import Callable, Dict, Optional
2121

2222
from opencut.helpers import OPENCUT_DIR
23+
from opencut.security import is_path_within
2324

2425
logger = logging.getLogger("opencut")
2526

@@ -100,11 +101,20 @@ def __init__(self, cache_dir: str = PREVIEW_CACHE_DIR,
100101
self._miss_count = 0
101102
self._cleanup_thread: Optional[threading.Thread] = None
102103
self._shutdown = threading.Event()
104+
self._metadata_file = os.path.join(self._cache_dir, "metadata.json")
103105

104106
os.makedirs(self._cache_dir, exist_ok=True)
105107
self._load_metadata()
106108
self._start_cleanup()
107109

110+
def _is_cache_path(self, path: str) -> bool:
111+
"""Return True when *path* resolves inside this cache directory."""
112+
return bool(path) and is_path_within(path, self._cache_dir)
113+
114+
def _is_cache_file(self, path: str) -> bool:
115+
"""Return True when *path* is an existing file inside this cache directory."""
116+
return self._is_cache_path(path) and os.path.isfile(path)
117+
108118
# ------------------------------------------------------------------
109119
# Key generation
110120
# ------------------------------------------------------------------
@@ -163,7 +173,7 @@ def put(self, key: str, file_path: str, source_path: str = "",
163173

164174
# Copy file to cache directory if not already there
165175
cache_file = file_path
166-
if not file_path.startswith(self._cache_dir):
176+
if not self._is_cache_path(file_path):
167177
ext = os.path.splitext(file_path)[1] or ".jpg"
168178
cache_file = os.path.join(self._cache_dir, f"{key}{ext}")
169179
try:
@@ -215,8 +225,11 @@ def _remove_entry(self, key: str) -> bool:
215225
if entry is None:
216226
return False
217227
try:
218-
if os.path.isfile(entry.file_path):
228+
if self._is_cache_file(entry.file_path):
219229
os.unlink(entry.file_path)
230+
elif entry.file_path:
231+
logger.warning("Refusing to delete preview cache path outside cache dir: %s", entry.file_path)
232+
return False
220233
except OSError:
221234
pass
222235
return True
@@ -258,14 +271,9 @@ def flush(self) -> int:
258271
"""Remove all cache entries and delete cache directory contents."""
259272
count = 0
260273
with self._lock:
261-
count = len(self._entries)
262-
for entry in self._entries.values():
263-
try:
264-
if os.path.isfile(entry.file_path):
265-
os.unlink(entry.file_path)
266-
except OSError:
267-
pass
268-
self._entries.clear()
274+
for key in list(self._entries):
275+
if self._remove_entry(key):
276+
count += 1
269277
self._hit_count = 0
270278
self._miss_count = 0
271279
self._save_metadata_unlocked()
@@ -283,13 +291,11 @@ def _evict_lru(self) -> None:
283291
# Find LRU entry
284292
lru_key = min(self._entries,
285293
key=lambda k: self._entries[k].last_accessed)
286-
entry = self._entries.pop(lru_key)
294+
entry = self._entries.get(lru_key)
295+
if entry is None:
296+
break
287297
total -= entry.file_size
288-
try:
289-
if os.path.isfile(entry.file_path):
290-
os.unlink(entry.file_path)
291-
except OSError:
292-
pass
298+
self._remove_entry(lru_key)
293299
logger.debug("Evicted cache entry: %s", lru_key)
294300

295301
# ------------------------------------------------------------------
@@ -433,16 +439,16 @@ def stats(self) -> CacheStats:
433439
# ------------------------------------------------------------------
434440
def _load_metadata(self) -> None:
435441
"""Load cache metadata from disk."""
436-
if not os.path.isfile(CACHE_METADATA_FILE):
442+
if not os.path.isfile(self._metadata_file):
437443
return
438444
try:
439-
with open(CACHE_METADATA_FILE, "r", encoding="utf-8") as fh:
445+
with open(self._metadata_file, "r", encoding="utf-8") as fh:
440446
data = json.load(fh)
441447
entries = data.get("entries", {})
442448
for k, v in entries.items():
443449
entry = CacheEntry.from_dict(v)
444-
# Verify file still exists
445-
if os.path.isfile(entry.file_path):
450+
# Verify file still exists and belongs to this cache.
451+
if self._is_cache_file(entry.file_path):
446452
self._entries[k] = entry
447453
self._hit_count = data.get("hit_count", 0)
448454
self._miss_count = data.get("miss_count", 0)
@@ -460,7 +466,7 @@ def _save_metadata_unlocked(self) -> None:
460466
}
461467
try:
462468
os.makedirs(self._cache_dir, exist_ok=True)
463-
with open(CACHE_METADATA_FILE, "w", encoding="utf-8") as fh:
469+
with open(self._metadata_file, "w", encoding="utf-8") as fh:
464470
json.dump(data, fh, indent=2)
465471
except OSError as e:
466472
logger.warning("Failed to save cache metadata: %s", e)

tests/test_preview_realtime.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
Uses Flask test client. No real FFmpeg, no GPU, no PIL — all mocked.
1313
"""
1414

15+
import json
1516
import os
1617
import sys
1718
import tempfile
@@ -946,6 +947,52 @@ def test_flush(self):
946947
finally:
947948
mgr.shutdown()
948949

950+
def test_metadata_is_scoped_to_custom_cache_dir(self):
951+
from opencut.core.preview_cache import PreviewCacheManager
952+
with tempfile.TemporaryDirectory() as td:
953+
source = os.path.join(td, "src.jpg")
954+
with open(source, "wb") as f:
955+
f.write(b"data")
956+
mgr = PreviewCacheManager(cache_dir=td, max_size_mb=10,
957+
cleanup_interval=9999)
958+
try:
959+
mgr.put("k1", source, source_path="/video/a.mp4")
960+
mgr.save_metadata()
961+
assert os.path.isfile(os.path.join(td, "metadata.json"))
962+
finally:
963+
mgr.shutdown()
964+
965+
mgr2 = PreviewCacheManager(cache_dir=td, max_size_mb=10,
966+
cleanup_interval=9999)
967+
try:
968+
assert mgr2.stats().entry_count == 1
969+
finally:
970+
mgr2.shutdown()
971+
972+
def test_metadata_paths_outside_cache_dir_are_ignored(self):
973+
from opencut.core.preview_cache import CacheEntry, PreviewCacheManager
974+
with tempfile.TemporaryDirectory() as td:
975+
with tempfile.NamedTemporaryFile(delete=False) as outside:
976+
outside.write(b"do-not-delete")
977+
outside_path = outside.name
978+
metadata = {
979+
"entries": {
980+
"bad": CacheEntry(cache_key="bad", file_path=outside_path).to_dict(),
981+
},
982+
"hit_count": 0,
983+
"miss_count": 0,
984+
}
985+
with open(os.path.join(td, "metadata.json"), "w", encoding="utf-8") as f:
986+
json.dump(metadata, f)
987+
mgr = PreviewCacheManager(cache_dir=td, max_size_mb=10,
988+
cleanup_interval=9999)
989+
try:
990+
assert mgr.stats().entry_count == 0
991+
assert os.path.isfile(outside_path)
992+
finally:
993+
mgr.shutdown()
994+
os.unlink(outside_path)
995+
949996
def test_invalidate_by_file(self):
950997
from opencut.core.preview_cache import PreviewCacheManager
951998
with tempfile.TemporaryDirectory() as td:

0 commit comments

Comments
 (0)