diff --git a/beetsplug/badfiles.py b/beetsplug/badfiles.py index beec8eae33..2f58ac6d1d 100644 --- a/beetsplug/badfiles.py +++ b/beetsplug/badfiles.py @@ -18,7 +18,13 @@ import os import shlex import sys -from subprocess import STDOUT, CalledProcessError, check_output, list2cmdline +from subprocess import ( + DEVNULL, + STDOUT, + CalledProcessError, + check_output, + list2cmdline, +) from typing import Literal import confuse @@ -66,7 +72,10 @@ def run_command(self, cmd): "running command: {}", displayable_path(list2cmdline(cmd)) ) try: - output = check_output(cmd, stderr=STDOUT) + # The checker commands are non-interactive, so detach stdin from + # the controlling terminal. Otherwise a checker may leave the TTY + # in a modified state (e.g. with echo disabled). See #6750. + output = check_output(cmd, stderr=STDOUT, stdin=DEVNULL) errors = 0 status = 0 except CalledProcessError as e: diff --git a/docs/changelog.rst b/docs/changelog.rst index 2609ce8fa9..b989ac0fa2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -34,6 +34,9 @@ Bug fixes example a multi-disc album whose cover lives in the album root rather than a per-disc directory); the missing art is skipped instead. :bug:`4692` - :doc:`plugins/tidal`: Normalize Tidal album types to lowercase. +- :doc:`plugins/badfiles`: Detach stdin from the controlling terminal when + running checker commands, so a checker can no longer leave the terminal in a + modified state (e.g. with input echo disabled) after ``beet bad``. :bug:`6750` .. For plugin developers diff --git a/test/plugins/test_badfiles.py b/test/plugins/test_badfiles.py index 8b88f72041..6aec21c45c 100644 --- a/test/plugins/test_badfiles.py +++ b/test/plugins/test_badfiles.py @@ -14,6 +14,7 @@ """Tests for the 'badfiles' plugin.""" +from subprocess import DEVNULL from types import SimpleNamespace from unittest.mock import patch @@ -46,3 +47,16 @@ def test_non_quiet_import_calls_prompt(self): result = plugin.on_import_task_before_choice(task, session=None) assert result == importer.Action.SKIP + + def test_run_command_detaches_stdin(self): + # The checker commands are non-interactive; run_command must detach + # stdin from the terminal so a checker cannot leave the TTY in a + # modified state (e.g. with echo disabled). See #6750. + plugin = BadFiles() + + with patch( + "beetsplug.badfiles.check_output", return_value=b"" + ) as mock_check_output: + plugin.run_command(["mp3val", "foo.mp3"]) + + assert mock_check_output.call_args.kwargs["stdin"] is DEVNULL