fix: handle gh security comments

Author: o-az

skills/multilingual-caption-video/SKILL.md

@@ -5,7 +5,7 @@ license: "GPL-3.0-or-Later"
 compatibility: Requires ffmpeg and ffprobe with libass and H.264 support, uv, yt-dlp for URL inputs, and curl for URL delivery.
 metadata:
   author: o-az
-  version: "1.1.0"
+  version: "1.1.1"
 ---
 
 # multilingual-caption-video
@@ -44,6 +44,7 @@ Run any script with `uv run <script> --help` for its interface.
 - Deliver only the generated MP4. A request for a URL authorizes uploading that generated file, not unrelated local files.
 - Write preferences only after explicit consent. An explicit request always overrides a saved preference for that job.
 - Delete only marked working directories created by `scripts/cleanup.py`.
+- Run bundled scripts as the current unprivileged user. Never invoke them through `sudo` or another privilege-elevation mechanism.
 
 ## Workflow
 
@@ -55,7 +56,7 @@ Resolve the directory containing this `SKILL.md` as `SKILL_ROOT`, then inspect s
 uv run "$SKILL_ROOT/scripts/preferences.py" show
 ```
 
-Preferences live at `${XDG_CONFIG_HOME:-~/.config}/multilingual-caption-video/preferences.json`. The file may contain `delivery`, `language`, `font`, and `font_size`.
+Preferences live at `$XDG_CONFIG_HOME/multilingual-caption-video/preferences.json` when `XDG_CONFIG_HOME` is an absolute path, or `~/.config/multilingual-caption-video/preferences.json` otherwise. The file may contain `delivery`, `language`, `font`, and `font_size`.
 
 Resolve each setting in this order: the current request, a saved preference, then the documented default. If neither the request nor saved preferences specify `delivery`, ask whether the user wants the finished video as a file or URL. If neither specifies the target language, ask for it. Do not infer either choice.
 

skills/multilingual-caption-video/scripts/cleanup.py

@@ -72,8 +72,7 @@ def main() -> None:
         description="Create and safely clean caption-video work directories."
     )
     subparsers = parser.add_subparsers(dest="command", required=True)
-    create_parser = subparsers.add_parser("create")
-    create_parser.add_argument("--parent", type=Path)
+    subparsers.add_parser("create")
     schedule_parser = subparsers.add_parser("schedule")
     schedule_parser.add_argument("path", type=Path)
     schedule_parser.add_argument("--delay", type=float, default=300)
@@ -85,7 +84,7 @@ def main() -> None:
     args = parser.parse_args()
 
     if args.command == "create":
-        print(create_workdir(args.parent))
+        print(create_workdir())
     elif args.command == "schedule":
         print(
             json.dumps(

skills/multilingual-caption-video/scripts/preferences.py

@@ -3,6 +3,7 @@
 import argparse
 import json
 import os
+import tempfile
 from collections.abc import Mapping
 from pathlib import Path
 from typing import cast
@@ -12,9 +13,11 @@
 
 
 def default_preferences_path() -> Path:
-    config_home = Path(
-        os.environ.get("XDG_CONFIG_HOME", Path.home() / ".config")
-    )
+    default_config_home = Path.home() / ".config"
+    configured = os.environ.get("XDG_CONFIG_HOME")
+    config_home = Path(configured) if configured else default_config_home
+    if not config_home.is_absolute():
+        config_home = default_config_home
     return config_home / "multilingual-caption-video" / "preferences.json"
 
 
@@ -45,23 +48,36 @@ def validate_preferences(preferences: Mapping[str, object]) -> Preferences:
 
 def load_preferences(path: Path | None = None) -> Preferences:
     path = path or default_preferences_path()
-    if not path.exists():
+    try:
+        serialized = path.read_text(encoding="utf-8")
+    except FileNotFoundError:
         return {}
-    return validate_preferences(json.loads(path.read_text(encoding="utf-8")))
+    return validate_preferences(json.loads(serialized))
 
 
 def save_preferences(
     updates: Mapping[str, object], path: Path | None = None
 ) -> Preferences:
     path = path or default_preferences_path()
     preferences = validate_preferences({**load_preferences(path), **updates})
-    path.parent.mkdir(parents=True, exist_ok=True)
-    temporary_path = path.with_suffix(".tmp")
-    temporary_path.write_text(
-        f"{json.dumps(preferences, indent=2, ensure_ascii=False, sort_keys=True)}\n",
-        encoding="utf-8",
-    )
-    os.replace(temporary_path, path)
+    path.parent.mkdir(mode=0o700, parents=True, exist_ok=True)
+    serialized = f"{json.dumps(preferences, indent=2, ensure_ascii=False, sort_keys=True)}\n"
+    temporary_path: Path | None = None
+    try:
+        with tempfile.NamedTemporaryFile(
+            mode="w",
+            encoding="utf-8",
+            dir=path.parent,
+            prefix=f".{path.name}.",
+            suffix=".tmp",
+            delete=False,
+        ) as temporary_file:
+            temporary_path = Path(temporary_file.name)
+            temporary_file.write(serialized)
+        os.replace(temporary_path, path)
+    finally:
+        if temporary_path is not None:
+            temporary_path.unlink(missing_ok=True)
     return preferences
 
 

tests/multilingual-caption-video/test_caption_video.py

@@ -1,10 +1,13 @@
 #!/usr/bin/env -S uv run
 
 import json
+import os
+import subprocess
 import sys
 import tempfile
 import time
 from pathlib import Path
+from unittest.mock import patch
 
 SCRIPTS = (
     Path(__file__).parents[2]
@@ -17,7 +20,11 @@
 
 from cleanup import create_workdir, keep_workdir, schedule_cleanup
 from make_ass import build_ass
-from preferences import load_preferences, save_preferences
+from preferences import (
+    default_preferences_path,
+    load_preferences,
+    save_preferences,
+)
 from transcribe import transcript_payload
 
 
@@ -53,6 +60,48 @@ def __init__(self, start: float, end: float, text: str) -> None:
 assert "Style: Default,Noto Naskh Arabic UI,35," in ass
 assert "Dialogue: 0,0:00:01.25,0:00:03.50" in ass
 
+cleanup_script = SCRIPTS / "cleanup.py"
+with tempfile.TemporaryDirectory() as temporary_directory:
+    unsupported_parent = subprocess.run(
+        [
+            sys.executable,
+            str(cleanup_script),
+            "create",
+            "--parent",
+            temporary_directory,
+        ],
+        capture_output=True,
+        text=True,
+        check=False,
+    )
+    assert unsupported_parent.returncode != 0
+    assert "unrecognized arguments: --parent" in unsupported_parent.stderr
+
+original_xdg_config_home = os.environ.get("XDG_CONFIG_HOME")
+try:
+    with tempfile.TemporaryDirectory() as temporary_directory:
+        absolute_config_home = Path(temporary_directory)
+        os.environ["XDG_CONFIG_HOME"] = str(absolute_config_home)
+        assert default_preferences_path() == (
+            absolute_config_home
+            / "multilingual-caption-video"
+            / "preferences.json"
+        )
+
+    for invalid_config_home in ("", "relative/config"):
+        os.environ["XDG_CONFIG_HOME"] = invalid_config_home
+        assert default_preferences_path() == (
+            Path.home()
+            / ".config"
+            / "multilingual-caption-video"
+            / "preferences.json"
+        )
+finally:
+    if original_xdg_config_home is None:
+        os.environ.pop("XDG_CONFIG_HOME", None)
+    else:
+        os.environ["XDG_CONFIG_HOME"] = original_xdg_config_home
+
 with tempfile.TemporaryDirectory() as temporary_directory:
     preferences_path = Path(temporary_directory) / "preferences.json"
     assert load_preferences(preferences_path) == {}
@@ -70,6 +119,34 @@ def __init__(self, start: float, end: float, text: str) -> None:
     else:
         raise AssertionError("Unsupported delivery preference should fail")
 
+    victim_path = Path(temporary_directory) / "victim.txt"
+    victim_path.write_text("unchanged\n")
+    predictable_temporary_path = preferences_path.with_suffix(".tmp")
+    predictable_temporary_path.symlink_to(victim_path)
+    save_preferences({"language": "Spanish"}, preferences_path)
+    assert victim_path.read_text() == "unchanged\n"
+    assert predictable_temporary_path.is_symlink()
+
+    destination_target = Path(temporary_directory) / "destination-target.json"
+    destination_target.write_text('{"language": "English"}\n')
+    preferences_path.unlink()
+    preferences_path.symlink_to(destination_target)
+    save_preferences({"language": "Arabic"}, preferences_path)
+    assert not preferences_path.is_symlink()
+    assert destination_target.read_text() == '{"language": "English"}\n'
+
+    failed_preferences_path = (
+        Path(temporary_directory) / "failed" / "preferences.json"
+    )
+    with patch("preferences.os.replace", side_effect=OSError("replace failed")):
+        try:
+            save_preferences({"language": "French"}, failed_preferences_path)
+        except OSError as error:
+            assert str(error) == "replace failed"
+        else:
+            raise AssertionError("A failed preferences replacement should fail")
+    assert list(failed_preferences_path.parent.glob("*.tmp")) == []
+
     cleanup_root = Path(temporary_directory) / "cleanup"
     cleanup_root.mkdir()