diff --git a/CHANGELOG.md b/CHANGELOG.md index a52e566..b4104b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `LAST30DAYS_YT_SSH_HOST` env var: when set, yt-dlp YouTube search invocations are routed through `ssh ` for residential-IP egress. Bypasses YouTube's bot-wall on datacenter IPs (Hetzner/DigitalOcean/AWS) where `ytsearch:` returns 0 results regardless of cookies (the IP fingerprint is checked first). The named host must be configured in `~/.ssh/config` and have yt-dlp installed. The transcript path is unchanged (uses the existing HTTP fallback when SSH-routing is on, since the timedtext API isn't bot-walled). +- `LAST30DAYS_YOUTUBE_SSH_HOST` env var: when set, yt-dlp YouTube search invocations are routed through `ssh ` for residential-IP egress. Bypasses YouTube's bot-wall on datacenter IPs (Hetzner/DigitalOcean/AWS) where `ytsearch:` returns 0 results regardless of cookies (the IP fingerprint is checked first). The named host must be configured in `~/.ssh/config` and have yt-dlp installed. Host value is validated against `^[a-zA-Z0-9._-]+$` to reject SSH option-injection (e.g. a leading `-` masquerading as a flag). The transcript path is unchanged (uses the existing HTTP fallback when SSH-routing is on, since the timedtext API isn't bot-walled). ### Changed diff --git a/skills/last30days/scripts/last30days.py b/skills/last30days/scripts/last30days.py index a873f05..da026c8 100644 --- a/skills/last30days/scripts/last30days.py +++ b/skills/last30days/scripts/last30days.py @@ -545,8 +545,8 @@ def main() -> int: # youtube_yt) can read it without taking a config dependency. This # routes yt-dlp through `ssh ` to bypass YouTube's bot-wall on # datacenter IPs (see lib/youtube_yt.py for details). - if config.get("LAST30DAYS_YT_SSH_HOST") and "LAST30DAYS_YT_SSH_HOST" not in os.environ: - os.environ["LAST30DAYS_YT_SSH_HOST"] = config["LAST30DAYS_YT_SSH_HOST"] + if config.get("LAST30DAYS_YOUTUBE_SSH_HOST") and "LAST30DAYS_YOUTUBE_SSH_HOST" not in os.environ: + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = config["LAST30DAYS_YOUTUBE_SSH_HOST"] # Handle setup subcommand topic = " ".join(args.topic).strip() diff --git a/skills/last30days/scripts/lib/youtube_yt.py b/skills/last30days/scripts/lib/youtube_yt.py index d805124..fe033d1 100644 --- a/skills/last30days/scripts/lib/youtube_yt.py +++ b/skills/last30days/scripts/lib/youtube_yt.py @@ -100,7 +100,7 @@ def _log(msg: str): def is_ytdlp_installed() -> bool: """Check if yt-dlp is available locally, or if SSH routing is configured. - When LAST30DAYS_YT_SSH_HOST is set, returns True without a local check — + When LAST30DAYS_YOUTUBE_SSH_HOST is set, returns True without a local check — yt-dlp lives on the remote host. Failures surface naturally on first use. """ if _ytdlp_ssh_host(): @@ -108,10 +108,16 @@ def is_ytdlp_installed() -> bool: return shutil.which("yt-dlp") is not None +# Host aliases must be plain hostnames / SSH config aliases — no flags, no +# shell metacharacters. Rejects any value that could be reinterpreted by ssh +# (or the surrounding shell) as something other than a destination. +_SSH_HOST_ALIAS_RE = re.compile(r"^[a-zA-Z0-9._-]+$") + + def _ytdlp_ssh_host() -> Optional[str]: """Return SSH host alias if yt-dlp should be routed via SSH, else None. - Set LAST30DAYS_YT_SSH_HOST= (e.g. 'macmini') in the environment + Set LAST30DAYS_YOUTUBE_SSH_HOST= (e.g. 'macmini') in the environment to route yt-dlp through SSH for residential IP egress. This bypasses YouTube's bot-wall on datacenter IPs (Hetzner, DigitalOcean, AWS, etc.) where ytsearch returns 0 results regardless of cookies. @@ -121,13 +127,30 @@ def _ytdlp_ssh_host() -> Optional[str]: add brew shellenv to ~/.zshenv (not just ~/.zprofile) so non-login SSH shells find yt-dlp on PATH. + Validation: host value must match ``[A-Za-z0-9._-]+``. Anything starting + with ``-`` or containing shell/SSH metacharacters is rejected with a + stderr warning and treated as unset, so a misconfigured or attacker- + controlled value can't slip through as an SSH option flag or proxy command. + The ``--`` option terminator in ``_wrap_ytdlp_cmd`` is a second line of + defense; this regex closes the door on the env var ever reaching ssh + in the first place. + To use a value from ~/.config/last30days/.env, export it into the environment before invoking the engine, e.g. in a wrapper: set -a; source ~/.config/last30days/.env; set +a python3 last30days.py "..." """ - host = os.environ.get("LAST30DAYS_YT_SSH_HOST", "").strip() - return host or None + host = os.environ.get("LAST30DAYS_YOUTUBE_SSH_HOST", "").strip() + if not host: + return None + if not _SSH_HOST_ALIAS_RE.match(host): + sys.stderr.write( + f"[youtube_yt] WARNING: LAST30DAYS_YOUTUBE_SSH_HOST={host!r} " + "does not look like a plain hostname/alias; ignoring. " + "Expected pattern: letters, digits, dot, underscore, hyphen.\n" + ) + return None + return host def _wrap_ytdlp_cmd(cmd: List[str]) -> List[str]: @@ -136,7 +159,7 @@ def _wrap_ytdlp_cmd(cmd: List[str]) -> List[str]: Args are shell-quoted to survive the remote shell. Uses BatchMode=yes so a misconfigured key fails fast instead of hanging on a password prompt. The `--` option terminator prevents an SSH option-injection if - LAST30DAYS_YT_SSH_HOST were ever set to a value starting with `-`. + LAST30DAYS_YOUTUBE_SSH_HOST were ever set to a value starting with `-`. """ host = _ytdlp_ssh_host() if not host: diff --git a/tests/test_youtube_yt.py b/tests/test_youtube_yt.py index ed8d1d4..be55b83 100644 --- a/tests/test_youtube_yt.py +++ b/tests/test_youtube_yt.py @@ -409,34 +409,34 @@ class TestSearchAndTranscribe(unittest.TestCase): class TestYtdlpSSHRouting(unittest.TestCase): - """LAST30DAYS_YT_SSH_HOST routes yt-dlp invocations through SSH for residential IP.""" + """LAST30DAYS_YOUTUBE_SSH_HOST routes yt-dlp invocations through SSH for residential IP.""" def setUp(self): # Ensure clean env for each test - self._saved_env = os.environ.pop("LAST30DAYS_YT_SSH_HOST", None) + self._saved_env = os.environ.pop("LAST30DAYS_YOUTUBE_SSH_HOST", None) def tearDown(self): - os.environ.pop("LAST30DAYS_YT_SSH_HOST", None) + os.environ.pop("LAST30DAYS_YOUTUBE_SSH_HOST", None) if self._saved_env is not None: - os.environ["LAST30DAYS_YT_SSH_HOST"] = self._saved_env + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = self._saved_env def test_no_env_var_returns_none(self): """Without the env var set, _ytdlp_ssh_host returns None.""" self.assertIsNone(youtube_yt._ytdlp_ssh_host()) def test_env_var_returns_host(self): - """With LAST30DAYS_YT_SSH_HOST set, _ytdlp_ssh_host returns it.""" - os.environ["LAST30DAYS_YT_SSH_HOST"] = "macmini" + """With LAST30DAYS_YOUTUBE_SSH_HOST set, _ytdlp_ssh_host returns it.""" + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = "macmini" self.assertEqual(youtube_yt._ytdlp_ssh_host(), "macmini") def test_env_var_whitespace_stripped(self): """Whitespace around the host alias is stripped.""" - os.environ["LAST30DAYS_YT_SSH_HOST"] = " macmini " + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = " macmini " self.assertEqual(youtube_yt._ytdlp_ssh_host(), "macmini") def test_empty_env_var_falls_back_to_none(self): """An empty env var is treated as unset.""" - os.environ["LAST30DAYS_YT_SSH_HOST"] = "" + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = "" self.assertIsNone(youtube_yt._ytdlp_ssh_host()) def test_wrap_cmd_passthrough_when_unset(self): @@ -446,7 +446,7 @@ class TestYtdlpSSHRouting(unittest.TestCase): def test_wrap_cmd_prepends_ssh_when_set(self): """_wrap_ytdlp_cmd prepends ssh when SSH routing is on.""" - os.environ["LAST30DAYS_YT_SSH_HOST"] = "macmini" + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = "macmini" cmd = ["yt-dlp", "--ignore-config", "ytsearch5:test"] wrapped = youtube_yt._wrap_ytdlp_cmd(cmd) self.assertEqual(wrapped[0], "ssh") @@ -462,29 +462,52 @@ class TestYtdlpSSHRouting(unittest.TestCase): def test_wrap_cmd_quotes_args_with_spaces(self): """Args containing spaces or special chars are shell-quoted.""" - os.environ["LAST30DAYS_YT_SSH_HOST"] = "macmini" + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = "macmini" cmd = ["yt-dlp", "ytsearch5:hello world", "--dump-json"] wrapped = youtube_yt._wrap_ytdlp_cmd(cmd) # shlex.quote wraps the whole arg in single quotes when it contains spaces self.assertIn("'ytsearch5:hello world'", wrapped[5]) def test_wrap_cmd_uses_option_terminator(self): - """`--` is inserted before host to prevent SSH option injection. - - Without `--`, an env var like `LAST30DAYS_YT_SSH_HOST=-oProxyCommand=...` - would be parsed by ssh as an option flag. The terminator forces it - to be treated as a hostname (which will then fail clean if invalid). - """ - os.environ["LAST30DAYS_YT_SSH_HOST"] = "-oFoo=bar" + """`--` is inserted before host as defense-in-depth even for valid hosts.""" + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = "macmini" cmd = ["yt-dlp", "--version"] wrapped = youtube_yt._wrap_ytdlp_cmd(cmd) - # Find the `--` terminator and verify the host comes immediately after dash_idx = wrapped.index("--") - self.assertEqual(wrapped[dash_idx + 1], "-oFoo=bar") + self.assertEqual(wrapped[dash_idx + 1], "macmini") + + def test_host_alias_with_dash_prefix_is_rejected(self): + """A host value starting with `-` is rejected by the alias validator. + + Without validation, ssh could parse `-oProxyCommand=...` as a flag + instead of a hostname. The `--` terminator in _wrap_ytdlp_cmd is + defense-in-depth; this regex on _ytdlp_ssh_host() rejects the value + before it ever reaches the ssh command line. + """ + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = "-oProxyCommand=evil" + self.assertIsNone(youtube_yt._ytdlp_ssh_host()) + # And the wrap function falls back to the local-execution path. + cmd = ["yt-dlp", "--version"] + self.assertEqual(youtube_yt._wrap_ytdlp_cmd(cmd), cmd) + + def test_host_alias_with_shell_metacharacters_is_rejected(self): + """Host values containing spaces, semicolons, $, etc. are rejected.""" + for bad in ("host;rm -rf /", "host name", "host$IFS", "host`whoami`", "host&cmd"): + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = bad + self.assertIsNone( + youtube_yt._ytdlp_ssh_host(), + msg=f"validator should reject {bad!r}", + ) + + def test_host_alias_validator_accepts_realistic_aliases(self): + """Valid SSH config aliases are accepted: bare names, FQDNs, IPs.""" + for good in ("macmini", "home-server", "pi5.local", "192.168.1.10", "homelab_box"): + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = good + self.assertEqual(youtube_yt._ytdlp_ssh_host(), good) def test_is_ytdlp_installed_short_circuits_with_ssh(self): """is_ytdlp_installed returns True without local check when SSH routing is on.""" - os.environ["LAST30DAYS_YT_SSH_HOST"] = "macmini" + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = "macmini" with mock.patch("lib.youtube_yt.shutil.which", return_value=None) as which_mock: self.assertTrue(youtube_yt.is_ytdlp_installed()) which_mock.assert_not_called() @@ -498,7 +521,7 @@ class TestYtdlpSSHRouting(unittest.TestCase): def test_search_call_routes_through_ssh(self): """search_youtube wraps the yt-dlp invocation when SSH routing is on.""" - os.environ["LAST30DAYS_YT_SSH_HOST"] = "macmini" + os.environ["LAST30DAYS_YOUTUBE_SSH_HOST"] = "macmini" from lib.subproc import SubprocResult fake_result = SubprocResult(returncode=0, stdout="", stderr="") with mock.patch.object(youtube_yt.subproc, "run_with_timeout",