security: harden code against command injection and path traversal

Core changes:

* enforce HTTPS URLs and remove shell usage in generate_release.py
* replace os.system calls with subprocess across the codebase
* validate external inputs (playlist names, video IDs)

Improvements and fixes:

* settings.py: fix typo (node.lineno → line_number); use isinstance() over type()
* youtube/get_app_version: improve git detection using subprocess.DEVNULL
* youtube/util.py: add cleanup helpers; use shutil.which for binary resolution

YouTube modules:

* watch.py: detect and flag HLS streams; remove unused audio_track_sources
* comments.py: return early when comments are disabled; add error handling
* local_playlist.py: validate playlist names to prevent path traversal
* subscriptions.py: replace asserts with proper error handling; validate video IDs

Cleanup:

* remove unused imports across modules (playlist, search, channel)
* reorganize package imports in youtube/**init**.py
* simplify test imports and fix cleanup_func in tests

Tests:

* tests/test_shorts.py: simplify imports
* tests/test_util.py: fix cleanup_func definition
This commit is contained in:
2026-04-20 00:39:35 -05:00
parent 155bd4df49
commit d6190a2d0b
16 changed files with 237 additions and 146 deletions

View File

@@ -44,6 +44,10 @@ def remove_files_with_extensions(path, extensions):
def download_if_not_exists(file_name, url, sha256=None):
if not os.path.exists('./' + file_name):
# Reject non-https URLs so a mistaken constant cannot cause a
# plaintext download (bandit B310 hardening).
if not url.startswith('https://'):
raise Exception('Refusing to download over non-https URL: ' + url)
log('Downloading ' + file_name + '..')
data = urllib.request.urlopen(url).read()
log('Finished downloading ' + file_name)
@@ -58,12 +62,14 @@ def download_if_not_exists(file_name, url, sha256=None):
log('Using existing ' + file_name)
def wine_run_shell(command):
# Keep argv-style invocation (no shell) to avoid command injection.
if os.name == 'posix':
check(os.system('wine ' + command.replace('\\', '/')))
parts = ['wine'] + command.replace('\\', '/').split()
elif os.name == 'nt':
check(os.system(command))
parts = command.split()
else:
raise Exception('Unsupported OS')
check(subprocess.run(parts).returncode)
def wine_run(command_parts):
if os.name == 'posix':
@@ -92,7 +98,20 @@ if os.path.exists('./yt-local'):
# confused with working directory. I'm calling it the same thing so it will
# have that name when extracted from the final release zip archive)
log('Making copy of yt-local files')
check(os.system('git archive --format tar master | 7z x -si -ttar -oyt-local'))
# Avoid the shell: pipe `git archive` into 7z directly via subprocess.
_git_archive = subprocess.Popen(
['git', 'archive', '--format', 'tar', 'master'],
stdout=subprocess.PIPE,
)
_sevenz = subprocess.Popen(
['7z', 'x', '-si', '-ttar', '-oyt-local'],
stdin=_git_archive.stdout,
)
_git_archive.stdout.close()
_sevenz.wait()
_git_archive.wait()
check(_sevenz.returncode)
check(_git_archive.returncode)
if len(os.listdir('./yt-local')) == 0:
raise Exception('Failed to copy yt-local files')
@@ -136,7 +155,7 @@ if os.path.exists('./python'):
log('Extracting python distribution')
check(os.system(r'7z -y x -opython ' + python_dist_name))
check_subp(subprocess.run(['7z', '-y', 'x', '-opython', python_dist_name]))
log('Executing get-pip.py')
wine_run(['./python/python.exe', '-I', 'get-pip.py'])
@@ -241,7 +260,7 @@ if os.path.exists('./' + output_filename):
log('Removing previous zipped release')
os.remove('./' + output_filename)
log('Zipping release')
check(os.system(r'7z -mx=9 a ' + output_filename + ' ./yt-local'))
check_subp(subprocess.run(['7z', '-mx=9', 'a', output_filename, './yt-local']))
print('\n')
log('Finished')