From 1bfd3032962b89b006d34f053617e4e2e9d9578f Mon Sep 17 00:00:00 2001 From: check <137591557+yifenliwu@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:47:17 +0800 Subject: [PATCH] =?UTF-8?q?fix(plugin):=207z=20=E5=8A=A0=E5=AF=86=E5=8E=8B?= =?UTF-8?q?=E7=BC=A9=E6=94=B9=E6=88=90=E5=8F=82=E6=95=B0=E5=88=97=E8=A1=A8?= =?UTF-8?q?=E8=B0=83=E7=94=A8=EF=BC=8C=E4=BE=9D=E8=B5=96=E9=A2=84=E6=A3=80?= =?UTF-8?q?=E5=85=88=E8=BF=87=20kwargs=20=E6=A0=A1=E9=AA=8C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 接着 #579 的 review 收尾。那两条 CodeRabbit 意见当时没人回（PR 已经合了，行内也回不了），代码在 dev 上还是原样，所以另开一个 PR。 ## zip_with_password 在拼 shell 命令串 `save_dir` / `zip_path` / `zip_password` 都来自 option 配置，原来这么拼： ```python cmd_list = f''' cd {self.save_dir} 7z a "{zip_path}" {file_args} -p{self.zip_password} -mhe=on > "../7z_output.txt" ''' self.execute_multi_line_cmd(cmd_list) # subprocess.run(cmd, shell=True, check=True) ``` `shlex.quote` 只包了文件名，另外三个值是裸拼进去的。密码里放个分号就能跑出去，跑一下当前代码看到的就是： ``` 7z a "...\o.7z" a.csv -pp w; rm -rf / # -mhe=on > "../7z_output.txt" ``` `-pp w` 之后整串都被 shell 当成新命令了。 改成参数列表加 `cwd=save_dir`，不走 shell： ```python cmd = ['7z', 'a', zip_path] cmd += [of_file_name(f) for f in files] cmd += [f'-p{self.zip_password}', '-mhe=on'] with open(os.path.join(self.save_dir, '..', '7z_output.txt'), 'w') as out: subprocess.run(cmd, cwd=self.save_dir, stdout=out, stderr=subprocess.STDOUT, check=True) ``` `7z_output.txt` 的位置和原来一致（还是在 `save_dir` 上一级）。密码里的空格、分号、引号现在都只是普通字符。 ## check_plugins_dependencies 没走 kwargs 校验 原来直接把 `pinfo.get('kwargs') or {}` 丢给 `check_plugin_dependency`。`kwargs` 写成真值标量（`kwargs: enabled`）时字符串被原样带下去，`required_dependencies_for` 里 `kwargs.get(...)` 就抛 `AttributeError`，把本该给出的「kwargs 必须为 dict」配置错误盖掉了。 改成先走一遍 `self.fix_kwargs(...)`，跟 `invoke_plugin` 用同一套校验，报错文案也就一致了。`fix_kwargs` 返回新 dict、不回写 `pinfo`，所以只是多解析一次，没有副作用。 ## 测试 `tests/test_jmcomic/test_jm_plugin.py` 改了 2 条、加了 2 条。只把 `src/jmcomic` 回滚到改动前跑一遍： ``` 3 failed, 1 passed, 11 deselected ``` 改动全上之后 `15 passed, 6 subtests passed`。 base 取的是 dev 当前 HEAD。和 #580 没有重叠改动，不过两个 PR 都碰到 `jm_plugin.py`，先合一个再合另一个可能要 rebase，需要的话我来处理。 --- src/jmcomic/jm_option.py | 7 ++- src/jmcomic/jm_plugin.py | 35 ++++++----- tests/test_jmcomic/test_jm_plugin.py | 87 +++++++++++++++++++++++++--- 3 files changed, 104 insertions(+), 25 deletions(-) diff --git a/src/jmcomic/jm_option.py b/src/jmcomic/jm_option.py index a712a5cf..a581a2b9 100644 --- a/src/jmcomic/jm_option.py +++ b/src/jmcomic/jm_option.py @@ -667,7 +667,12 @@ def check_plugins_dependencies(self) -> None: if pclass is None: continue - pclass.check_plugin_dependency(pinfo.get('kwargs') or {}, strategy=strategy) + # 与 invoke_plugin 保持一致，先过 fix_kwargs 校验参数类型： + # kwargs 写成真值标量（如 kwargs: enabled）时，required_dependencies_for + # 里的 kwargs.get(...) 会抛 AttributeError，把这里本该给出的 + # “kwargs 必须为 dict”配置错误盖掉。 + plugin_kwargs = self.fix_kwargs(pinfo.get('kwargs')) + pclass.check_plugin_dependency(plugin_kwargs, strategy=strategy) def call_all_plugin(self, group: str, safe=None, **extra): plugin_list: List[dict] = self.plugins.get(group, []) diff --git a/src/jmcomic/jm_plugin.py b/src/jmcomic/jm_plugin.py index 859061e2..4bf990a8 100644 --- a/src/jmcomic/jm_plugin.py +++ b/src/jmcomic/jm_plugin.py @@ -1410,6 +1410,10 @@ def zip_with_password(self, files, zip_path): 以及失败收藏夹写了一半的 csv 一起塞进包里，而这些文件并不在 execute_deletion 的删除范围内，等于往产物里混入无关数据。 + 以参数列表直接调用 7z，不经过 shell：save_dir / zip_path / zip_password + 都来自 option 配置，拼进 shell 命令串会引入命令注入（密码里带空格或 + 分号就会改变命令语义），shlex.quote 只能护住文件名，护不住这几个值。 + :param files: 要压缩的文件的绝对路径的列表 :param zip_path: 压缩文件的保存路径 """ @@ -1417,22 +1421,23 @@ def zip_with_password(self, files, zip_path): if not files: return - import shlex - - # 在 save_dir 中逐个列举本次成功导出的文件。 - file_args = ' '.join( - shlex.quote(of_file_name(f)) for f in files - ) - - cmd_list = f''' - cd {self.save_dir} - 7z a "{zip_path}" {file_args} -p{self.zip_password} -mhe=on > "../7z_output.txt" - - ''' - self.log(f'运行命令: {cmd_list}') + import subprocess - # 执行 - self.execute_multi_line_cmd(cmd_list) + # 以参数列表直接调用 7z，不经过 shell。 + # save_dir / zip_path / zip_password 都来自 option 配置，拼进 shell 命令串 + # 会引入命令注入（密码里带空格或分号就会改变命令语义）， + # shlex.quote 只护得住文件名，护不住这几个值。 + # 工作目录设为 save_dir，所以传相对 save_dir 的文件名即可。 + cmd = ['7z', 'a', zip_path] + cmd += [of_file_name(f) for f in files] + cmd += [f'-p{self.zip_password}', '-mhe=on'] + self.log(f'运行命令: {cmd}') + + # 输出重定向位置与原实现一致：save_dir 的上一级 + output_filepath = os.path.join(self.save_dir, '..', '7z_output.txt') + with open(output_filepath, 'w') as out: + subprocess.run(cmd, cwd=self.save_dir, stdout=out, + stderr=subprocess.STDOUT, check=True) class Img2pdfPlugin(JmOptionPlugin): diff --git a/tests/test_jmcomic/test_jm_plugin.py b/tests/test_jmcomic/test_jm_plugin.py index 9529e4a5..ccd649bd 100644 --- a/tests/test_jmcomic/test_jm_plugin.py +++ b/tests/test_jmcomic/test_jm_plugin.py @@ -336,9 +336,9 @@ def test_favorite_folder_export_empty_encrypted_zip_skips_command(self): plugin = FavoriteFolderExportPlugin(self.new_option()) plugin.max_retry = 0 plugin.failed_folders = [('bad', '失败收藏夹', RuntimeError('导出失败'))] - with patch.object(plugin, 'execute_multi_line_cmd') as execute: + with patch('subprocess.run') as run: plugin.zip_with_password([], 'export.7z') - execute.assert_not_called() + run.assert_not_called() with self.assertRaises(JmcomicException): plugin.raise_if_failed_folders() @@ -577,15 +577,84 @@ def test_zip_with_password_does_not_archive_whole_save_dir(self): plugin.zip_password = 'secret' good = os.path.join(tmp, 'good.csv') - cmds = [] - with patch.object(plugin, 'execute_multi_line_cmd', side_effect=cmds.append): + calls = [] + with patch('subprocess.run', side_effect=lambda *a, **kw: calls.append((a, kw))): plugin.zip_with_password([good], plugin.zip_filepath) - self.assertEqual(1, len(cmds)) - cmd = cmds[0] + self.assertEqual(1, len(calls)) + args, kwargs = calls[0] + cmd = args[0] + # 参数列表形式调用，不经过 shell + self.assertIsInstance(cmd, list) + self.assertEqual('7z', cmd[0]) self.assertIn('good.csv', cmd) - self.assertNotIn('"./"', cmd) - self.assertNotIn("'./'", cmd) - print('✅ 7z command enumerates files instead of archiving "./".') + # 不能再用 './' 或通配把整个 save_dir 打包进去 + self.assertNotIn('./', cmd) + self.assertNotIn('.', cmd) + self.assertNotIn('*', cmd) + # 工作目录指向 save_dir，且不再走 shell + self.assertEqual(tmp, kwargs.get('cwd')) + self.assertNotIn('shell', kwargs) + print('✅ 7z invoked with an argv list (cwd=save_dir, no shell).') finally: shutil.rmtree(tmp, ignore_errors=True) + + def test_zip_with_password_does_not_go_through_shell(self): + """ + source: https://github.com/hect0x7/JMComic-Crawler-Python/pull/579 + + zip_password / save_dir / zip_path 都来自 option 配置。之前拼成 shell 命令串 + （execute_multi_line_cmd → subprocess.run(shell=True)），密码里带空格或分号 + 就能改变命令语义；shlex.quote 只护住了文件名。改成参数列表后，这些值原样 + 作为单个 argv 元素传下去。 + """ + import tempfile + import shutil + from unittest.mock import patch + + from jmcomic.jm_plugin import FavoriteFolderExportPlugin + + option = self.new_option() + tmp = tempfile.mkdtemp(prefix='jm_test_7z_noshell_') + try: + plugin = FavoriteFolderExportPlugin(option) + plugin.save_dir = tmp + evil_password = 'p w; rm -rf / #' + plugin.zip_password = evil_password + good = os.path.join(tmp, 'a.csv') + calls = [] + with patch('subprocess.run', side_effect=lambda *a, **kw: calls.append((a, kw))): + plugin.zip_with_password([good], os.path.join(tmp, 'o.7z')) + + args, kwargs = calls[0] + cmd = args[0] + self.assertIsInstance(cmd, list) + self.assertIn(f'-p{evil_password}', cmd) + self.assertNotIn('shell', kwargs) + # 没有任何一个参数是拼好的整条命令串 + self.assertFalse(any('7z a' in str(c) for c in cmd)) + print('✅ 7z receives an argv list; password is never shell-interpreted.') + finally: + shutil.rmtree(tmp, ignore_errors=True) + + def test_dependencies_reject_non_mapping_kwargs(self): + """ + source: https://github.com/hect0x7/JMComic-Crawler-Python/pull/579 + + kwargs 必须是映射。写成真值标量（kwargs: enabled）时，之前的 + check_plugins_dependencies 直接把字符串丢给 required_dependencies_for， + 里面 kwargs.get(...) 抛 AttributeError，把「kwargs 必须为 dict」这条 + 配置错误盖掉了。现在前置 fix_kwargs，与 invoke 路径共用同一套校验。 + """ + from jmcomic import JmOption, JmcomicException + + dic = {'plugins': {'after_album': [{'plugin': 'zip', 'kwargs': 'enabled'}]}} + try: + JmOption.construct(dic) + except JmcomicException as e: + self.assertIn('kwargs', str(e)) + except AttributeError as e: + self.fail(f'非 mapping 的 kwargs 仍抛 AttributeError: {e}') + else: + self.fail('非 mapping 的 kwargs 应当抛配置错误，实际构建成功') + print('✅ non-mapping kwargs rejected with a readable config error.')