Skip to content
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/). Versions follow
- **AppleScript injection in `_desktop_notify` on macOS** (closes #40): `src/yt_dont_recommend/state.py` interpolated an untrusted `message` string directly into an `osascript -e` AppleScript argument. A double-quote in `message` closed the string literal and allowed arbitrary shell execution via `do shell script "…"`. Reachable on macOS via channel names in a blocklist because `parse_text_blocklist` / `parse_json_blocklist` did not validate entries — failure paths in `unblock.py` build attention messages that embed channel data verbatim. Fixed by adding a private `_escape_applescript` helper that escapes backslash, double-quote, newline, carriage return, and tab before interpolation. Linux (`notify-send`) was never affected — it takes the message as a separate argv element. Follow-up issue #41 adds parse-time validation as defense in depth.
- **Channel identifiers validated at parse time** (closes #41): `parse_text_blocklist` and `parse_json_blocklist` accepted arbitrary strings after comment/prefix stripping, leaving downstream sinks (`page.goto` URLs, CSS selectors, AppleScript notifications) to trust whatever passed through. A new `_canonicalize_channel` helper enforces regex-based structural validation — only `@[A-Za-z0-9._-]+` or `UC[A-Za-z0-9_-]{22}` are accepted. Both parsers now drop invalid entries with a single `Dropped N invalid channel entries` WARN per parse call. Defense in depth for the #40 fix and reliability improvement for #46 (selector f-strings). Pre-existing test fixtures that used a 26-character placeholder `UCxxxxxxxxxxxxxxxxxxxxxxxx` were corrected to the real 24-character shape; one legacy-path test (`test_dict_url_with_other_path_falls_back_to_path`) was renamed and updated to assert that `/user/legacyName`-style paths are dropped rather than forwarded.
- **`resolve_source` rejects insecure `http://` sources** (closes #42): previously accepted `http://` and `https://` as equals, so a MITM on a plaintext user-added source could swap blocklist contents. Combined with the #40 sink path this was a full RCE before #40 landed. `resolve_source` now logs an error mentioning `https://` (or the local file path alternative) and exits with code 1. Built-in sources are both `https://` so default usage is unaffected; only users who had configured an `http://` source need to update.
- **Auto-upgrade is interactive-only** (closes #43): `do_auto_upgrade` in `src/yt_dont_recommend/cli.py` previously fired from any code path that found a new release on PyPI — including scheduled runs (`--heartbeat` → cron / launchd subprocess). A compromised PyPI release would be picked up silently the next time the scheduler fired, with immediate access to the Playwright browser-profile session cookies one directory away. `do_auto_upgrade` now returns early with an INFO log when `sys.stdin.isatty()` is False, narrowing the supply-chain blast radius to interactive sessions where the user can decide whether to upgrade. The README "Auto-upgrade" section documents the trust model and points users at `--check-update` (interactive trigger) and `--revert` (rollback).

### Fixed

Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,8 @@ The new binary takes effect on the next run. Disable at any time:
yt-dont-recommend --auto-upgrade disable
```

> **Trust model — auto-upgrade is interactive-only.** When `auto_upgrade` is enabled, the upgrade only runs in interactive sessions (`stdin` is a TTY). Scheduled runs (cron / launchd / `--heartbeat`) still notify you of a new release but do not install it: a compromised PyPI release would otherwise be picked up silently the next time the scheduler fires, with immediate access to your YouTube session cookies in the local browser profile. Run `yt-dont-recommend --check-update` interactively when you're ready to upgrade. If a release does turn out to be bad, `yt-dont-recommend --revert` rolls back to the previously recorded version (and disables auto-upgrade so it does not immediately re-upgrade).

### Reverting an upgrade

If something goes wrong after an upgrade, revert to the previous version:
Expand Down
14 changes: 14 additions & 0 deletions src/yt_dont_recommend/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,21 @@ def do_auto_upgrade(state: dict) -> bool:
Saves the current version as previous_version before upgrading so
--revert can restore it. Returns True if the upgrade succeeded.
The new binary takes effect on the next invocation.

Auto-upgrade is interactive-only: when stdin is not a TTY (cron,
launchd, or any other non-interactive scheduler), the version
notification still fires upstream but the upgrade itself is skipped.
This narrows the supply-chain blast radius — a compromised release
on PyPI cannot install itself silently from a scheduled run; the
user must opt in by running the tool interactively.
"""
if not sys.stdin.isatty():
log.info(
"Auto-upgrade skipped — non-interactive session (cron/launchd). "
"Run yt-dont-recommend interactively to upgrade."
)
return False

installer = _detect_installer()
current = _get_current_version()

Expand Down
30 changes: 30 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,13 @@ def test_notifies_via_ntfy_once_per_new_version(self, monkeypatch):
# ---------------------------------------------------------------------------

class TestDoAutoUpgrade:
@staticmethod
def _force_tty(monkeypatch, value: bool = True) -> None:
"""Pretend stdin is/is-not a TTY so the isatty() gate doesn't dominate the test."""
monkeypatch.setattr("sys.stdin.isatty", lambda: value)

def test_uv_path_success(self, monkeypatch, tmp_path):
self._force_tty(monkeypatch)
monkeypatch.setattr(ydr, "STATE_FILE", tmp_path / "state.json")
monkeypatch.setattr("yt_dont_recommend.cli._detect_installer", lambda: "uv")
monkeypatch.setattr("yt_dont_recommend.cli._get_current_version", lambda: "1.0.0")
Expand All @@ -349,6 +355,7 @@ def fake_run(cmd, capture_output, text):
assert state["previous_version"] == "1.0.0"

def test_pipx_path_success(self, monkeypatch, tmp_path):
self._force_tty(monkeypatch)
monkeypatch.setattr(ydr, "STATE_FILE", tmp_path / "state.json")
monkeypatch.setattr("yt_dont_recommend.cli._detect_installer", lambda: "pipx")
monkeypatch.setattr("yt_dont_recommend.cli._get_current_version", lambda: "1.0.0")
Expand All @@ -358,6 +365,7 @@ def test_pipx_path_success(self, monkeypatch, tmp_path):
assert do_auto_upgrade({}) is True

def test_unknown_installer_returns_false_and_warns(self, monkeypatch, caplog):
self._force_tty(monkeypatch)
monkeypatch.setattr("yt_dont_recommend.cli._detect_installer", lambda: None)
monkeypatch.setattr("yt_dont_recommend.cli._get_current_version", lambda: "1.0.0")
from yt_dont_recommend.cli import do_auto_upgrade
Expand All @@ -366,6 +374,7 @@ def test_unknown_installer_returns_false_and_warns(self, monkeypatch, caplog):
assert any("package manager" in r.message.lower() for r in caplog.records)

def test_subprocess_failure_writes_attention(self, monkeypatch, tmp_path):
self._force_tty(monkeypatch)
monkeypatch.setattr(ydr, "STATE_FILE", tmp_path / "state.json")
monkeypatch.setattr("yt_dont_recommend.cli._detect_installer", lambda: "uv")
monkeypatch.setattr("yt_dont_recommend.cli._get_current_version", lambda: "1.0.0")
Expand All @@ -377,6 +386,27 @@ def test_subprocess_failure_writes_attention(self, monkeypatch, tmp_path):
assert do_auto_upgrade({}) is False
assert wa_calls and "install failed" in wa_calls[0]

def test_non_tty_session_skips_upgrade_without_invoking_subprocess(self, monkeypatch, caplog):
"""Scheduled (cron/launchd) runs notify-only: subprocess is never called."""
self._force_tty(monkeypatch, value=False)
run_called = {"count": 0}

def fake_run(*a, **kw):
run_called["count"] += 1
return MagicMock(returncode=0)

monkeypatch.setattr("yt_dont_recommend.cli.subprocess.run", fake_run)
# _detect_installer / _get_current_version must not be reached either,
# but installing fakes is cheap insurance and avoids accidental network calls.
monkeypatch.setattr("yt_dont_recommend.cli._detect_installer", lambda: "uv")
monkeypatch.setattr("yt_dont_recommend.cli._get_current_version", lambda: "1.0.0")

from yt_dont_recommend.cli import do_auto_upgrade
with caplog.at_level(logging.INFO, logger="yt_dont_recommend.cli"):
assert do_auto_upgrade({}) is False
assert run_called["count"] == 0
assert any("non-interactive" in r.message.lower() for r in caplog.records)


# ---------------------------------------------------------------------------
# do_revert
Expand Down
Loading