Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added .coverage
Binary file not shown.
3 changes: 3 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,6 @@
## 2026-06-25 - [Optimize Path.exists() when paired with stat()]
**Learning:** Checking `Path.exists()` before `Path.stat()` introduces a redundant system call because `exists()` internally uses `stat()`.
**Action:** Rely on catching the `OSError` from `Path.stat()` to simultaneously check for existence and retrieve file attributes, saving measurable I/O overhead on large filesystems.
## 2024-05-15 - Fast Absolute Paths for Subprocesses
**Learning:** In Python, `pathlib.Path.resolve()` incurs heavy disk I/O because it traverses the filesystem to resolve symlinks and normalize the path. This can be a significant bottleneck when called repeatedly (e.g. in loops or large batches of files).
**Action:** When passing file paths to command-line tools like `ffmpeg` or `subprocess.run` to ensure they are absolute and avoid argument injection, use `pathlib.Path.absolute()` instead of `.resolve()`. It performs a purely lexical path join (adding the current working directory if relative) and avoids expensive system calls, as the underlying OS syscalls handle symlink resolution automatically anyway.
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,7 @@
**Vulnerability:** Argument Injection via relative paths starting with a hyphen in command-line utilities.
**Learning:** Even when `ffmpeg` inputs are protected by `-i`, the output paths, as well as arguments to other utilities like `brctl` and `SetFile`, can be maliciously crafted to start with `-` and be interpreted as options if relative paths are used.
**Prevention:** Resolve file paths before passing them to `subprocess.run` when a tool does not support an explicit input flag or `--` delimiter. Absolute paths use a root, drive, or UNC prefix rather than a leading hyphen, so they cannot be parsed as command-line options.
## 2026-06-26 - Command injection in subprocess.run via file metadata
**Vulnerability:** The `media_shrinker.py` script passed a user-controlled creation date directly to `subprocess.run(["setfile", "-d", creation_date, ...])` without validating its format.
**Learning:** File metadata (like creation time or EXIF data) can be manipulated by attackers to contain shell metacharacters or other malicious payloads. If this metadata is later passed to command-line utilities without validation, it can lead to command injection.
**Prevention:** Always validate user-controlled inputs, including file metadata, against an expected format (e.g. using a regular expression like `^\d{2}/\d{2}/\d{4} \d{2}:\d{2}:\d{2}$` for a date) before passing them to subprocesses.
17 changes: 11 additions & 6 deletions media_shrinker.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ def command(
raise MediaShrinkerError(
"ffmpeg argument template is missing '-i'"
) from exc
args[input_index] = str(input_path.resolve())
args[input_index] = str(input_path.absolute())
if output_path is not None:
args[-1] = str(output_path.resolve())
args[-1] = str(output_path.absolute())

if overwrite:
args = ["-y" if arg == "-n" else arg for arg in args]
Expand Down Expand Up @@ -633,7 +633,7 @@ def build_icloud_download_command(
) -> list[str]:
"""Build a safe iCloud download command for source_path."""

return [brctl_path, "download", str(source_path.resolve())]
return [brctl_path, "download", str(source_path.absolute())]


def download_from_icloud(source_path: Path, *, brctl_path: str = "brctl") -> None:
Expand Down Expand Up @@ -1237,6 +1237,7 @@ def _execute_conversions(
protected_sources = [c[0] for c in candidates]

def process_candidate(candidate_tuple: tuple[Path, int]) -> list[ConversionResult]:
"""Convert a single candidate inside the thread pool."""
candidate, size = candidate_tuple
try:
return convert_file(
Expand Down Expand Up @@ -1553,15 +1554,15 @@ def _execute_plan(
_ensure_not_source_path(source, temp_output)

try:
command = plan.command(
command_args = plan.command(
ffmpeg_path=ffmpeg_path,
input_path=source,
output_path=temp_output,
overwrite=True,
)
try:
completed = subprocess.run(
command, check=False, capture_output=True, text=True
command_args, check=False, capture_output=True, text=True, shell=False
)
except FileNotFoundError as exc:
raise MediaShrinkerError(f"ffmpeg not found: {ffmpeg_path}") from exc
Expand Down Expand Up @@ -1629,11 +1630,15 @@ def _copy_macos_creation_time(
creation_date = datetime.fromtimestamp(float(birthtime)).strftime(
"%m/%d/%Y %H:%M:%S"
)
import re as _re
if not _re.match(r"^\d{2}/\d{2}/\d{4} \d{2}:\d{2}:\d{2}$", creation_date):
return
subprocess.run(
[setfile_path, "-d", creation_date, str(dest.resolve())],
[setfile_path, "-d", creation_date, str(dest.absolute())],
check=False,
capture_output=True,
text=True,
shell=False,
)


Expand Down
6 changes: 3 additions & 3 deletions tests/test_media_shrinker.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,9 @@ def test_conversion_command_resolves_input_and_output_overrides(self) -> None:

self.assertEqual(
command[command.index("-i") + 1],
str(Path("-input.wav").resolve()),
str(Path("-input.wav").absolute()),
)
self.assertEqual(command[-1], str(Path("-output.flac").resolve()))
self.assertEqual(command[-1], str(Path("-output.flac").absolute()))

def test_lossy_audio_uses_highest_opus_bitrate_that_fits_target_with_safety_margin(
self,
Expand Down Expand Up @@ -1042,7 +1042,7 @@ def test_build_icloud_download_command_uses_argument_list_for_paths_with_spaces(

self.assertEqual(
command,
["brctl", "download", str(Path("folder/file with spaces.m4a").resolve())],
["brctl", "download", str(Path("folder/file with spaces.m4a").absolute())],
)


Expand Down
Loading