fix(security): refuse DASH/IMF manifests and subfile URLs, refuse line breaks in concat paths, never write temps through a planted link

CHANGELOG.md

@@ -4,7 +4,44 @@
 
 ## Unreleased
 
-(nothing yet)
+### Security
+
+- **A DASH manifest or an IMF composition playlist given as media is refused, whatever it is
+  called.** The playlist guard knew HLS and ffconcat by name and first bytes only, so a DASH
+  manifest saved as `clip.mp4` (with a `dash:profile` or the 3GPP `PSS:profile:DASH1`) made every
+  tool follow its `BaseURL`: to another local file, which
+  the output then contained (reported completed and verified), or over HTTP, which FFmpeg 6.1's
+  `-protocol_whitelist file` did not stop. Both are now told the way ffmpeg's own probes tell
+  them, by content, and refused as `media_path_is_playlist`.
+- **`subfile,<options>:<url>` is refused as a URL.** ffmpeg opens it through the subfile protocol;
+  the URL guard stopped reading the scheme at the comma and let it through, so the guards
+  described the named file while ffmpeg read another.
+- **A path with a line break is refused where it would go into a concat list** (`loop.py`,
+  `join.py --copy`, `sequence.py`, multi-part cuts): `media_path_has_line_break`. Each line of a
+  concat list is a directive, so such a name added entries and pulled a file named nowhere on the
+  command line into the output.
+- **The hidden temp files in the output directory are created exclusively.** The staging temp of
+  an `--overwrite` run, `place_output()`'s temp, `export.py`'s key/fill temp and `batch.py`'s
+  cross-filesystem delivery temp had fixed names and were opened through
+  whatever was already there, a planted symlink included: the encode was written into the link's
+  target and the output left as a symlink to it. A copy the tool makes itself is written
+  through the descriptor that created the temp, never by reopening its name, and a directory that
+  refuses the exclusive create fails the run (`output_dir_not_writable`) instead of handing ffmpeg
+  a name nobody reserved. A lock's default audit log is opened without following a symlink.
+- **`render.py` refuses an output that is any file the project names.** The guard compared the
+  output with the clips and overlay videos only, so with `--overwrite` a project whose output was
+  its own music bed, brand file (or the brand's logo or font), overlay image or font, caption
+  file, beat source, or the project file itself replaced it. A `--stop-after` preview, which
+  never writes the output, is not refused. `audiogram.image` and an overlay's `font_file` are now passed
+  to their stages resolved against the project file, as every other project path is, instead of
+  against the caller's working directory.
+
+### Fixed
+
+- **A colour given to `caption.py`, `graphics.py` or a brand must be six hex digits.** The check
+  was on the length only: a six-letter name such as `yellow` or `orange` drew black text and was
+  reported verified, and a quote or comma reached the filter graph and failed as `kind: ffmpeg`.
+  It is now a `kind: input` refusal.
 
 ## 3.6.0
 

docs/contract.md

@@ -897,7 +897,7 @@ here claims otherwise until real exit-code/stderr sniffing exists to back that u
 A refusal also carries `error.reason`, a stable snake_case code for *why*
 (`contract --json` → `json_output.error_reasons` lists them: `output_exists`, `output_is_input`,
 `output_is_directory`, `output_dir_missing`, `output_dir_not_writable`, `output_locked`,
-`media_path_is_url`, `media_path_is_playlist` (an HLS or ffconcat playlist given as media),
+`media_path_is_url` (`subfile,...:` included), `media_path_is_playlist` (an HLS, DASH, IMF or ffconcat playlist given as media, DASH and IMF told by content whatever the file is called), `media_path_has_line_break` (a path with a line break, which a concat list cannot hold),
 `output_looks_like_option` (an output starting with `-`, other than `-` for stdout), `output_path_taken` (two files one run writes share a path), `media_path_not_a_file` (a FIFO or device as input), `silence.py --timemap`'s `timemap_path_taken`, `join.py --copy`'s `join_copy_mismatch`,
 `render.py`'s `plan_*` reasons, `--codec av1`/`prores`'s `encoder_missing` and
 `encoder_list_unreadable`, and `--transcribe`'s `asr_engine_missing`, `no_speech`, `engine_failed`

docs/design-decisions.md

@@ -407,7 +407,29 @@ exists. When a decision changes, edit the entry in the same PR.
 - **An existing output is written through a hidden sibling temp file and replaced only on
   success**, so a failed run never costs the caller the file that was there (FFmpeg 5.x truncates
   the output before a filter error). The temp name `.<stem>.ffskill-<pid><ext>` is expected in the
-  output directory during a run. Code: `_common._stage_existing_output()`.
+  output directory during a run; it is created exclusively (`O_EXCL`), so an entry already at that
+  name, a planted symlink included, is never written through, and the next free
+  `.<stem>.ffskill-<pid>-<n><ext>` is used instead. A copy the tool makes itself (`place_output`,
+  `batch.py`'s cross-filesystem delivery) writes the descriptor that created the temp
+  (`copy_to_sibling()`); ffmpeg opens its temp by name, so a process that may delete entries in
+  the output directory could still swap the file between the create and that open, which a
+  sticky shared directory (`/tmp`) and a directory only the caller can write both rule out. A lock's default audit log beside the output
+  is opened without following a symlink; a log named with `--audit-log` is opened as named. Code:
+  `_common._stage_existing_output()`, `runner.reserve_sibling()`, `emit.audit()`. Test:
+  `test_hidden_temp_names_are_never_written_through_a_planted_link`.
+- **A file ffmpeg would read as a playlist is refused whatever it is called.** HLS and ffconcat
+  are told by name or their first bytes; a DASH manifest and an IMF composition playlist by name
+  or by the strings ffmpeg's own probes look for (`<MPD` with `dash:profile`, `<CompositionPlaylist`
+  with `ContentTitle>`), anywhere before the first NUL byte, because ffmpeg probes the whole probe
+  buffer: a manifest saved as `clip.mp4` made every tool follow its BaseURL to another local file
+  or over HTTP, and FFmpeg 6.1's `-protocol_whitelist file` did not stop the HTTP request. A media
+  file has a NUL byte within its first bytes, so the scan stops there. `subfile,<options>:<url>`
+  is refused as a URL like any other protocol prefix. A path with a line break is refused where it
+  would be written into a concat list (`media_path_has_line_break`): the format has no escape for
+  one. Code: `runner._refuse_playlist()`, `runner.looks_like_url()`, `decision.concat_list_line()`.
+  Tests: `test_a_playlist_input_and_an_option_shaped_output_are_refused`,
+  `test_a_url_never_reaches_ffmpeg_as_an_input_or_an_output`,
+  `test_a_path_with_a_line_break_never_reaches_a_concat_list`.
 - **Subtitle/data tracks are kept where the picture's timeline is untouched and dropped where
   it is retimed; both cases report `dropped_non_av_streams`.** Tools that re-encode the picture
   or audio in place (`fit`, `color`, `graphics`, `overlay`, `audio`, `loudness`, `proxy`,

scripts/_ass_overlay.py

@@ -54,7 +54,7 @@ def ass_time(sec: float) -> str:
 
 def ass_color(hex_rgb: str, alpha: int = 0) -> str:
     h = str(hex_rgb).lstrip("#")
-    if len(h) != 6:
+    if len(h) != 6 or any(c not in "0123456789abcdefABCDEF" for c in h):
         raise ValueError(f"colour must be RRGGBB hex, got '{hex_rgb}'")
     return f"&H{alpha:02X}{h[4:6]}{h[2:4]}{h[0:2]}".upper()
 

scripts/_common/color.py

@@ -312,13 +312,20 @@ def through(m: "re.Match[str]") -> str:
     return out[:at] + ["-vf", conv] + out[at:]
 
 
+_HEX6 = re.compile(r"^[0-9A-Fa-f]{6}$")
+
+
 def color_hex(value: str) -> str:
-    """Normalise '#ffd200' / 'ffd200' / '0xFFD200' to 'FFD200'."""
+    """Normalise '#ffd200' / 'ffd200' / '0xFFD200' to 'FFD200'. Six hex digits only: the result is
+    written into libass styles and drawtext/color options, so a six-letter colour name ("yellow")
+    used to pass the length check and draw black, reported verified, and a quote or comma reached
+    the filter graph."""
     v = str(value).strip().lstrip("#")
     if v.lower().startswith("0x"):
         v = v[2:]
-    if len(v) != 6:
-        die(f"colour must be RRGGBB, got '{value}'")
+    if not _HEX6.match(v):
+        die(f"colour must be six hex digits RRGGBB (e.g. FFD200 or #ffd200), got '{value}'; colour names are not "
+            "accepted here")
     return v.upper()
 
 

scripts/_common/decision.py

@@ -148,7 +148,15 @@ def concat_list_line(path: str) -> str:
     """One `file '...'` line for the concat demuxer. The demuxer reads backslash as an escape
     inside the quoted form, so a Windows path (C:\\Users\\...\\part000.mp4) must be written
     with forward slashes -- ffmpeg opens either spelling on Windows -- and a single quote in the
-    name is closed, escaped and reopened. Shared by cut.py (multi-segment) and sequence.py."""
+    name is closed, escaped and reopened. Shared by cut.py (multi-segment) and sequence.py.
+
+    A line break has no escape in this format: the demuxer reads each line as its own directive,
+    so a name containing one would add `file` entries to the list and pull in files the caller
+    never named. Such a path is refused."""
+    if "\n" in str(path) or "\r" in str(path):
+        refuse("input", "media_path_has_line_break",
+               f"{str(path)!r} contains a line break, which ffmpeg's concat list cannot hold (each line is a "
+               "directive of its own); rename the file", path=str(path))
     # only on Windows: on POSIX the quoted form takes a backslash literally, and it is part of the name (#408)
     escaped = (str(path).replace("\\", "/") if _on_windows() else str(path)).replace("'", "'\\''")
     return f"file '{escaped}'"

scripts/_common/emit.py

@@ -8,6 +8,7 @@
 import json
 import math
 import os
+import stat
 import sys
 from typing import Any, Dict, List, Optional, Sequence, Tuple
 from _common.runner import Context, ERROR_CODE, ERROR_RETRYABLE, STATE
@@ -319,7 +320,28 @@ def audit(event: str, details: Dict[str, Any], *, default_path: Optional[str] =
         rec["plan_sha256"] = plan_sha256
     rec["details"] = details
     try:
-        with open(path, "a", encoding="utf-8") as f:
+        # a default path is a hidden name in someone's output directory: never appended to through a
+        # symlink planted there. A log the caller named (--audit-log) is opened as named.
+        flags = os.O_WRONLY | os.O_APPEND | os.O_CREAT | (0 if ctx.audit_log else getattr(os, "O_NOFOLLOW", 0))
+        checked = not ctx.audit_log and not hasattr(os, "O_NOFOLLOW")
+        if checked and os.path.islink(path):
+            raise OSError(f"{path} is a symbolic link")
+        fd = os.open(path, flags, 0o666)
+        if checked:
+            # no O_NOFOLLOW (Windows): a link that appeared between the look above and the open is
+            # caught after it -- the descriptor must be the file now at that name, not a link's
+            # target -- and nothing is written through it
+            try:
+                st, at = os.fstat(fd), os.lstat(path)
+                # st_nlink: a link swapped for a hard link to its target before the lstat would
+                # otherwise pass the identity check; a log this tool created has one name
+                if (stat.S_ISLNK(at.st_mode) or (st.st_dev, st.st_ino) != (at.st_dev, at.st_ino)
+                        or not stat.S_ISREG(st.st_mode) or st.st_nlink != 1):
+                    raise OSError(f"{path} was replaced by a link while it was opened")
+            except BaseException:
+                os.close(fd)
+                raise
+        with os.fdopen(fd, "a", encoding="utf-8") as f:
             f.write(json.dumps(json_safe(rec), ensure_ascii=False, sort_keys=True) + "\n")
     except OSError as exc:
         warn(f"could not append the {event} record to the audit log {path} ({exc}); the run is unaffected",