From 91a491bf04ec3233fa8239d5fee0c4a319ac584f Mon Sep 17 00:00:00 2001 From: MichaIng Date: Tue, 28 May 2024 19:27:29 +0200 Subject: [PATCH] enhance motion startup logging and update "safety" workflow (#2983) Print motion path and version in debug log when detecting and when starting motion. Print error code when motion failed to start. Align variable names and avoid unused variables. Safety "check" has been superseded by "scan": https://docs.safetycli.com/safety-docs/safety-cli-3/migrating-from-safety-cli-2.x-to-safety-cli-3.x#switching-to-the-new-scan-command But it requires to create an account and authenticate. Until we decide whether we want to create a motionEye account for this, and when we know how to authenticate non-interactively, we stick with "check": https://github.com/pyupio/safety/issues/525 Ignore disputed CVE-2018-20225. pip (intentioanlly) pulls the latest version of a module from PyPI, if an older version is available in "extra" indexes added via "extra-index-url" config/arg. If the module does not exist on PyPI at all, an attacker could upload one with the same name, injecting an unintended module into the user's project. This is of course naturally true when installing one module with multiple indexes, same as when installing an APT package with multiple APT repositories present. "extra"-index-url is not meant to override, but extend the indexes. To enforce a different index, and mitigate this potential risk for modules not uploaded to PyPI, use "index-url" arg/config instead. Remove obsolete workaround. Fix error log when ffmpeg executable could NOT print version. Add executable path to debug log. Quote motion executable path, which is not assured to work in shell without quotation. Align variable and structuring code comments. Signed-off-by: MichaIng --- .github/workflows/python_safety.yml | 6 ++++-- motioneye/mediafiles.py | 4 ++-- motioneye/motionctl.py | 30 +++++++++++++++-------------- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/.github/workflows/python_safety.yml b/.github/workflows/python_safety.yml index ec02ef0fe..25c245ee7 100644 --- a/.github/workflows/python_safety.yml +++ b/.github/workflows/python_safety.yml @@ -24,5 +24,7 @@ jobs: check-latest: true - run: pip install --upgrade pip setuptools - run: pip install safety . - - run: rm -Rfv /opt/hostedtoolcache/Python/3.12.1/x64/lib/python3.12/site-packages/pip-23.2.1.dist-info # Workaround: https://github.com/motioneye-project/motioneye/pull/2883 - - run: safety check + # Ignore CVE-2018-20225, which is IMO reasonably disputed: https://data.safetycli.com/v/67599/97c/ + # "extra"-index-url means an index to "additionally" look for newer versions, pre-compiled wheels, or similar, not to force this index being used. + # There is "index-url" to enforce a different index: https://pip.pypa.io/en/stable/cli/pip_install/#cmdoption-i + - run: safety check --ignore 67599 diff --git a/motioneye/mediafiles.py b/motioneye/mediafiles.py index 1ddc82498..8bca304be 100644 --- a/motioneye/mediafiles.py +++ b/motioneye/mediafiles.py @@ -240,7 +240,7 @@ def find_ffmpeg() -> tuple: output = utils.call_subprocess([quote(binary), '-version']) except subprocess.CalledProcessError as e: - logging.error(f'ffmpeg: could find version: {e}') + logging.error(f'ffmpeg: could not find version: {e}') return None, None, None result = re.findall('ffmpeg version (.+?) ', output, re.IGNORECASE) @@ -278,7 +278,7 @@ def find_ffmpeg() -> tuple: codecs[codec] = {'encoders': encoders, 'decoders': decoders} - logging.debug(f'using ffmpeg version {version}') + logging.debug(f'found ffmpeg executable "{binary}" version "{version}"') _ffmpeg_binary_cache = (binary, version, codecs) diff --git a/motioneye/motionctl.py b/motioneye/motionctl.py index 272e487af..bc29abe3e 100644 --- a/motioneye/motionctl.py +++ b/motioneye/motionctl.py @@ -21,6 +21,7 @@ import signal import subprocess import time +from shlex import quote from tornado.httpclient import AsyncHTTPClient, HTTPRequest from tornado.ioloop import IOLoop @@ -40,6 +41,7 @@ def find_motion(): if _motion_binary_cache: return _motion_binary_cache + # binary if settings.MOTION_BINARY: if os.path.exists(settings.MOTION_BINARY): binary = settings.MOTION_BINARY @@ -54,16 +56,18 @@ def find_motion(): except subprocess.CalledProcessError: # not found return None, None + # version try: - help = utils.call_subprocess(binary + ' -h || true', shell=True) + output = utils.call_subprocess(quote(binary) + ' -h || true', shell=True) - except subprocess.CalledProcessError: # not found + except subprocess.CalledProcessError as e: # not found as + logging.error(f'motion version could not be found: {e}') return None, None - result = re.findall('motion Version ([^,]+)', help, re.IGNORECASE) + result = re.findall('motion Version ([^,]+)', output, re.IGNORECASE) version = result and result[0] or '' - logging.debug(f'using motion version {version}') + logging.debug(f'found motion executable "{binary}" version "{version}"') _motion_binary_cache = (binary, version) @@ -85,21 +89,19 @@ def start(deferred=False): if running() or not enabled_local_motion_cameras: return - logging.debug('starting motion') + logging.debug('searching motion executable') - program = find_motion() - if not program[0]: + binary, version = find_motion() + if not binary: raise Exception('motion executable could not be found') - program, version = program # @UnusedVariable - - logging.debug(f'starting motion binary "{program}"') + logging.debug(f'starting motion executable "{binary}" version "{version}"') - motion_config_path = os.path.join(settings.CONF_PATH, 'motion.conf') + motion_cfg_path = os.path.join(settings.CONF_PATH, 'motion.conf') motion_log_path = os.path.join(settings.LOG_PATH, 'motion.log') motion_pid_path = os.path.join(settings.RUN_PATH, 'motion.pid') - args = [program, '-n', '-c', motion_config_path, '-d'] + args = [binary, '-n', '-c', motion_cfg_path, '-d'] if settings.LOG_LEVEL <= logging.DEBUG: args.append('9') @@ -120,11 +122,11 @@ def start(deferred=False): ) # wait 2 seconds to see that the process has successfully started - for i in range(20): # @UnusedVariable + for _ in range(20): time.sleep(0.1) exit_code = process.poll() if exit_code is not None and exit_code != 0: - raise Exception('motion failed to start') + raise Exception(f'motion failed to start with exit code "{exit_code}"') pid = process.pid