语法糖 #61

MixBadGun · 2026-03-18T17:34:57+08:00

MixBadGun commented

2026-03-18 17:34:57 +08:00

User description

糖糖

PR Type

Enhancement

Description

新增语法糖插件，支持命令别名存储与展开
支持模板变量渲染（位置参数与命名参数）
通过事件克隆实现命令回注执行
更新 VSCode 配置使用 Poetry 环境管理

Diagram Walkthrough

flowchart LR
  User["用户输入命令"]
  Cmd["语法糖命令处理"]
  Store["存入/删除/查看"]
  DB["SQLite 数据库"]
  Render["模板变量渲染"]
  Reinject["命令回注执行"]
  User -- "糖 <action>" --> Cmd
  Cmd -- "存入/删除/查看" --> Store
  Store -- "读写" --> DB
  Cmd -- "匹配糖名" --> Render
  Render -- "展开结果" --> Reinject

File Walkthrough

Relevant files

Enhancement

__init__.py `语法糖插件核心逻辑：CRUD 与命令回注` konabot/plugins/syntactic_sugar/init.py 注册 `语法糖`/`糖`/`sugar` 命令，支持存入、删除、查看操作实现模板渲染，支持 `{1}` 位置参数和 `{key}` 命名参数通过 `_reinject_command` 克隆事件并回注展开后的命令，限制递归深度为 3 数据库初始化时自动迁移 `channel_id` 列和唯一索引	+210/-0
create_table.sql `语法糖表结构与唯一索引定义` konabot/plugins/syntactic_sugar/sql/create_table.sql 创建 `syntactic_sugar` 表，包含 `name`、`content`、`belong_to`、`channel_id` 等字段创建基于 `name`、`channel_id`、`belong_to` 的唯一索引	+12/-0
insert_sugar.sql `语法糖 upsert 插入语句` konabot/plugins/syntactic_sugar/sql/insert_sugar.sql 使用 `INSERT ... ON CONFLICT DO UPDATE` 实现 upsert 语义	+5/-0

Configuration changes

settings.json

VSCode 默认使用 Poetry 环境管理

.vscode/settings.json

新增 python-envs.defaultEnvManager 和 python-envs.defaultPackageManager
配置为 Poetry

+3/-1

### **User description** 糖糖 ___ ### **PR Type** Enhancement ___ ### **Description** - 新增语法糖插件，支持命令别名存储与展开 - 支持模板变量渲染（位置参数与命名参数） - 通过事件克隆实现命令回注执行 - 更新 VSCode 配置使用 Poetry 环境管理 ___ ### Diagram Walkthrough ```mermaid flowchart LR User["用户输入命令"] Cmd["语法糖命令处理"] Store["存入/删除/查看"] DB["SQLite 数据库"] Render["模板变量渲染"] Reinject["命令回注执行"] User -- "糖 <action>" --> Cmd Cmd -- "存入/删除/查看" --> Store Store -- "读写" --> DB Cmd -- "匹配糖名" --> Render Render -- "展开结果" --> Reinject ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>__init__.py</strong><dd><code>语法糖插件核心逻辑：CRUD 与命令回注</code>                                                                          </dd></summary> <hr> konabot/plugins/syntactic_sugar/__init__.py <ul><li>注册 <code>语法糖</code>/<code>糖</code>/<code>sugar</code> 命令，支持存入、删除、查看操作<br> <li> 实现模板渲染，支持 <code>{1}</code> 位置参数和 <code>{key}</code> 命名参数<br> <li> 通过 <code>_reinject_command</code> 克隆事件并回注展开后的命令，限制递归深度为 3<br> <li> 数据库初始化时自动迁移 <code>channel_id</code> 列和唯一索引</ul> </details> </td> <td><a href="https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/__init__.py">+210/-0</a>  </td> </tr> <tr> <td> <details> <summary><strong>create_table.sql</strong><dd><code>语法糖表结构与唯一索引定义</code>                                                                                        </dd></summary> <hr> konabot/plugins/syntactic_sugar/sql/create_table.sql <ul><li>创建 <code>syntactic_sugar</code> 表，包含 <code>name</code>、<code>content</code>、<code>belong_to</code>、<code>channel_id</code> 等字段<br> <li> 创建基于 <code>name</code>、<code>channel_id</code>、<code>belong_to</code> 的唯一索引</ul> </details> </td> <td><a href="https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/sql/create_table.sql">+12/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>insert_sugar.sql</strong><dd><code>语法糖 upsert 插入语句</code>                                                                                    </dd></summary> <hr> konabot/plugins/syntactic_sugar/sql/insert_sugar.sql - 使用 `INSERT ... ON CONFLICT DO UPDATE` 实现 upsert 语义 </details> </td> <td><a href="https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/sql/insert_sugar.sql">+5/-0</a>      </td> </tr> </table></td></tr><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>settings.json</strong><dd><code>VSCode 默认使用 Poetry 环境管理</code>                                                                    </dd></summary> <hr> .vscode/settings.json <ul><li>新增 <code>python-envs.defaultEnvManager</code> 和 <code>python-envs.defaultPackageManager</code> <br>配置为 Poetry</ul> </details> </td> <td><a href="https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/.vscode/settings.json">+3/-1</a>      </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

MixBadGun added 1 commit 2026-03-18 17:34:58 +08:00

语法糖 23b9f101b3

kagami-review commented

2026-03-18 17:35:46 +08:00

PR Reviewer Guide 🔍

(Review updated until commit `9064b31fe9`)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 Security concerns

命令注入风险：用户可以通过 存入 操作将任意命令文本存储为语法糖，之后通过 _reinject_command 回注执行。这意味着任何用户都可以创建一个语法糖，使其展开后触发 bot 的任意已注册命令（包括可能的管理命令）。虽然递归深度限制为3层，但这不能阻止横向的命令权限绕过。建议对可回注的命令进行白名单过滤，或校验调用者是否有权限执行展开后的目标命令。

⚡ Recommended focus areas for review

注入安全

_reinject_command 将用户可控的模板渲染结果直接作为新命令回注执行。虽然有递归深度限制（3层），但恶意用户仍可通过精心构造的语法糖内容触发任意已注册命令，可能导致权限绕过或意外操作。建议增加命令白名单或对展开内容做进一步校验。

async def _reinject_command(bot: Bot, evt: Event, command_text: str) -> bool:
    depth = int(getattr(evt, "_syntactic_sugar_depth", 0))
    if depth >= 3:
        return False

    try:
        cloned_evt = copy.deepcopy(evt)
    except Exception:
        logger.exception("语法糖克隆事件失败")
        return False

    message = getattr(cloned_evt, "message", None)
    if message is None:
        return False

    try:
        msg_obj = type(message)(command_text)
    except Exception:
        msg_obj = command_text

    setattr(cloned_evt, "message", msg_obj)
    if hasattr(cloned_evt, "original_message"):
        setattr(cloned_evt, "original_message", msg_obj)
    if hasattr(cloned_evt, "raw_message"):
        setattr(cloned_evt, "raw_message", command_text)

    setattr(cloned_evt, "_syntactic_sugar_depth", depth + 1)

    try:
        await handle_event(bot, cloned_evt)
    except Exception:
        logger.exception("语法糖回注事件失败")
        return False
    return True

逻辑缺陷

"查看"分支中，当 _find_sugar 返回 None 时调用 cmd.finish 后，代码继续执行到第200行的 fallback 逻辑。虽然 cmd.finish 会抛异常中断流程，但"存入"、"删除"、"查看"三个分支使用的是独立的 if 而非 elif，如果 cmd.finish 的行为发生变化或被捕获，会导致意外的 fall-through 执行。建议改为 elif 链。

if action == "存入":
    if len(tokens) < 2:
        await cmd.finish("请提供要存入的名称")
    name = tokens[1].strip()
    content = " ".join(tokens[2:]).strip()
    if not content:
        content = _extract_reply_plain_text(evt)
    if not content:
        await cmd.finish("请提供要存入的内容")

    await _store_sugar(name, content, target_id, channel_id)
    await cmd.finish(f"糖已存入：「{name}」！")

if action == "删除":
    if len(tokens) < 2:
        await cmd.finish("请提供要删除的名称")
    name = tokens[1].strip()
    await _delete_sugar(name, target_id, channel_id)
    await cmd.finish(f"已删除糖：「{name}」！")

if action == "查看":
    if len(tokens) < 2:
        await cmd.finish("请提供要查看的名称")
    name = tokens[1].strip()
    content = await _find_sugar(name, target_id, channel_id)
    if content is None:
        await cmd.finish(f"没有糖：「{name}」")
    await cmd.finish(f"糖的内容：「{content}」")


name = action
content = await _find_sugar(name, target_id, channel_id)
if content is None:
    await cmd.finish(f"没有糖：「{name}」")

数据库迁移

init_db 中先无条件 DROP 旧索引再 CREATE 新索引，且通过 PRAGMA table_info 做列迁移。这种手动迁移方式在多次启动或并发场景下较脆弱，且没有版本追踪。如果未来 schema 继续演进，建议引入简单的迁移版本管理机制。

async def init_db():
    await db_manager.execute_by_sql_file(ROOT_PATH / "sql" / "create_table.sql")

    table_info = await db_manager.query("PRAGMA table_info(syntactic_sugar)")
    columns = {str(row.get("name")) for row in table_info}
    if "channel_id" not in columns:
        await db_manager.execute(
            "ALTER TABLE syntactic_sugar ADD COLUMN channel_id VARCHAR(255) NOT NULL DEFAULT ''"
        )

    await db_manager.execute("DROP INDEX IF EXISTS idx_syntactic_sugar_name_belong_to")
    await db_manager.execute(
        "CREATE UNIQUE INDEX IF NOT EXISTS idx_syntactic_sugar_name_channel_target "
        "ON syntactic_sugar(name, channel_id, belong_to)"
    )

## PR Reviewer Guide 🔍 #### (Review updated until commit https://gitea.service.jazzwhom.top/mttu-developers/konabot/commit/9064b31fe9ab116df24f3fc2ba1944c175039a05) Here are some key observations to aid the review process: <table> <tr><td>⏱️ <strong>Estimated effort to review</strong>: 3 🔵🔵🔵⚪⚪</td></tr> <tr><td>🧪 <strong>No relevant tests</strong></td></tr> <tr><td>🔒 <strong>Security concerns</strong><br><br> 命令注入风险：用户可以通过 `存入` 操作将任意命令文本存储为语法糖，之后通过 `_reinject_command` 回注执行。这意味着任何用户都可以创建一个语法糖，使其展开后触发 bot 的任意已注册命令（包括可能的管理命令）。虽然递归深度限制为3层，但这不能阻止横向的命令权限绕过。建议对可回注的命令进行白名单过滤，或校验调用者是否有权限执行展开后的目标命令。</td></tr> <tr><td>⚡ <strong>Recommended focus areas for review</strong><br><br> <details><summary><a href='https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/__init__.py#L123-L156'><strong>注入安全</strong></a> `_reinject_command` 将用户可控的模板渲染结果直接作为新命令回注执行。虽然有递归深度限制（3层），但恶意用户仍可通过精心构造的语法糖内容触发任意已注册命令，可能导致权限绕过或意外操作。建议增加命令白名单或对展开内容做进一步校验。 </summary> ```python async def _reinject_command(bot: Bot, evt: Event, command_text: str) -> bool: depth = int(getattr(evt, "_syntactic_sugar_depth", 0)) if depth >= 3: return False try: cloned_evt = copy.deepcopy(evt) except Exception: logger.exception("语法糖克隆事件失败") return False message = getattr(cloned_evt, "message", None) if message is None: return False try: msg_obj = type(message)(command_text) except Exception: msg_obj = command_text setattr(cloned_evt, "message", msg_obj) if hasattr(cloned_evt, "original_message"): setattr(cloned_evt, "original_message", msg_obj) if hasattr(cloned_evt, "raw_message"): setattr(cloned_evt, "raw_message", command_text) setattr(cloned_evt, "_syntactic_sugar_depth", depth + 1) try: await handle_event(bot, cloned_evt) except Exception: logger.exception("语法糖回注事件失败") return False return True ``` </details> <details><summary><a href='https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/__init__.py#L170-L203'><strong>逻辑缺陷</strong></a> "查看"分支中，当 `_find_sugar` 返回 `None` 时调用 `cmd.finish` 后，代码继续执行到第200行的 fallback 逻辑。虽然 `cmd.finish` 会抛异常中断流程，但"存入"、"删除"、"查看"三个分支使用的是独立的 `if` 而非 `elif`，如果 `cmd.finish` 的行为发生变化或被捕获，会导致意外的 fall-through 执行。建议改为 `elif` 链。 </summary> ```python if action == "存入": if len(tokens) < 2: await cmd.finish("请提供要存入的名称") name = tokens[1].strip() content = " ".join(tokens[2:]).strip() if not content: content = _extract_reply_plain_text(evt) if not content: await cmd.finish("请提供要存入的内容") await _store_sugar(name, content, target_id, channel_id) await cmd.finish(f"糖已存入：「{name}」！") if action == "删除": if len(tokens) < 2: await cmd.finish("请提供要删除的名称") name = tokens[1].strip() await _delete_sugar(name, target_id, channel_id) await cmd.finish(f"已删除糖：「{name}」！") if action == "查看": if len(tokens) < 2: await cmd.finish("请提供要查看的名称") name = tokens[1].strip() content = await _find_sugar(name, target_id, channel_id) if content is None: await cmd.finish(f"没有糖：「{name}」") await cmd.finish(f"糖的内容：「{content}」") name = action content = await _find_sugar(name, target_id, channel_id) if content is None: await cmd.finish(f"没有糖：「{name}」") ``` </details> <details><summary><a href='https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/__init__.py#L33-L47'><strong>数据库迁移</strong></a> `init_db` 中先无条件 DROP 旧索引再 CREATE 新索引，且通过 `PRAGMA table_info` 做列迁移。这种手动迁移方式在多次启动或并发场景下较脆弱，且没有版本追踪。如果未来 schema 继续演进，建议引入简单的迁移版本管理机制。 </summary> ```python async def init_db(): await db_manager.execute_by_sql_file(ROOT_PATH / "sql" / "create_table.sql") table_info = await db_manager.query("PRAGMA table_info(syntactic_sugar)") columns = {str(row.get("name")) for row in table_info} if "channel_id" not in columns: await db_manager.execute( "ALTER TABLE syntactic_sugar ADD COLUMN channel_id VARCHAR(255) NOT NULL DEFAULT ''" ) await db_manager.execute("DROP INDEX IF EXISTS idx_syntactic_sugar_name_belong_to") await db_manager.execute( "CREATE UNIQUE INDEX IF NOT EXISTS idx_syntactic_sugar_name_channel_target " "ON syntactic_sugar(name, channel_id, belong_to)" ) ``` </details> </td></tr> </table>

kagami-review commented

2026-03-18 17:36:22 +08:00

PR Code Suggestions ✨

Latest suggestions up to 8f40572
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	finish后缺少显式return保护在"查看"分支中，当 `content` 不为 `None` 时，代码会继续执行到 `await cmd.finish(...)` 并正常结束。但当 `content is None` 时，`cmd.finish` 虽然会抛出异常来终止流程，但从代码可读性和安全性角度看，缺少 `return` 或 `else` 会让后续的"糖展开"逻辑在 `finish` 未能正确终止时被意外执行。同样的问题也出现在最后的糖展开部分的 `None` 检查中。建议添加显式的 `return` 以确保流程安全。 konabot/plugins/syntactic_sugar/init.py [194-197] `content = await _find_sugar(name, target_id, channel_id) if content is None: await cmd.finish(f"没有糖：「{name}」") + return await cmd.finish(f"糖的内容：「{content}」")` Suggestion importance[1-10]: 4 __ Why: In nonebot, `cmd.finish` raises a `FinishedException` to terminate the handler, so the `return` is not strictly necessary. However, adding a defensive `return` improves readability and guards against potential framework behavior changes. This is a minor defensive coding improvement, not a real bug.	Low
Possible issue	糖展开时缺少None防护在糖展开逻辑中，如果 `_find_sugar` 返回 `None`，`cmd.finish` 通过抛出异常来终止处理流程。但如果异常未被正确传播（例如框架行为变更），`content` 为 `None` 时代码会继续执行 `_render_template`，导致对 `None` 进行正则替换而产生错误。应在 `cmd.finish` 后添加 `return` 作为防御性编程。 konabot/plugins/syntactic_sugar/init.py [201-205] `content = await _find_sugar(name, target_id, channel_id) if content is None: await cmd.finish(f"没有糖：「{name}」") + return positional, named = _split_variables(tokens[1:])` Suggestion importance[1-10]: 4 __ Why: Same reasoning as suggestion 1 — `cmd.finish` raises an exception in nonebot so `content` being `None` won't actually reach `_render_template`. Adding `return` is a reasonable defensive measure but not fixing an actual bug. Minor improvement.	Low
Security	模板展开存在命令注入风险 `_render_template` 中用户可以通过模板变量 `{name}` 注入任意内容，而展开后的结果会被 `_reinject_command` 作为新命令重新注入事件处理。虽然递归深度限制为 3，但恶意用户仍可通过构造糖内容来执行非预期的命令（如删除其他糖或触发其他插件命令）。建议对渲染后的结果进行命令白名单校验，或至少在文档中明确说明此安全风险。 konabot/plugins/syntactic_sugar/init.py [82-89] `+def replace(match: re.Match[str]) -> str: + key = match.group(1).strip() + if key.isdigit(): + idx = int(key) - 1 + if 0 <= idx < len(positional): + return positional[idx] + return match.group(0) + return named.get(key, match.group(0)) -` Suggestion importance[1-10]: 3 __ Why: The security concern about command injection via template rendering into `_reinject_command` is valid conceptually, but the `existing_code` and `improved_code` are identical — no actual fix is proposed. The recursion depth is already capped at 3, and the feature is inherently designed to re-inject commands. Without a concrete code change, this is more of a discussion point than an actionable suggestion.	Low

Previous suggestions

Suggestions up to commit 9064b31

Category	Suggestion	Impact
Possible issue	分支条件应互斥避免穿透 "存入"、"删除"、"查看"三个分支都使用 `if` 而非 `elif`，依赖 `cmd.finish` 抛异常来终止流程。如果 `cmd.finish` 在某些边界情况下未能抛出异常（例如被上层捕获），代码会穿透到下方的默认分支，导致"存入"或"删除"操作后还会尝试查找并执行语法糖。应改为 `elif` 链以确保互斥。 konabot/plugins/syntactic_sugar/init.py [190-200] `-if action == "查看": +elif action == "查看": if len(tokens) < 2: await cmd.finish("请提供要查看的名称") name = tokens[1].strip() content = await _find_sugar(name, target_id, channel_id) if content is None: await cmd.finish(f"没有糖：「{name}」") + return await cmd.finish(f"糖的内容：「{content}」") +else: + name = action - -name = action -` Suggestion importance[1-10]: 6 __ Why: Using `elif` instead of separate `if` statements is a valid structural improvement. In nonebot, `cmd.finish` raises `FinishedException` so fall-through won't happen in normal operation, but using `elif`/`else` makes the mutual exclusivity explicit and is more robust and readable. It's a good defensive coding practice but not a real bug given the framework's behavior.	Low
Possible issue	finish 后缺少 return 防护在"查看"分支和默认分支中，`_find_sugar` 返回 `None` 后调用 `cmd.finish` 虽然会抛出异常终止流程，但类型检查器无法感知这一点，`content` 在后续 `_render_template` 中仍可能被推断为 `None`。建议在 `cmd.finish` 后显式 `return`，以确保代码健壮性，避免在 `cmd.finish` 行为变更时 `content` 为 `None` 传入 `_render_template` 导致运行时错误。"查看"分支同理。 konabot/plugins/syntactic_sugar/init.py [201-205] `content = await _find_sugar(name, target_id, channel_id) if content is None: await cmd.finish(f"没有糖：「{name}」") + return positional, named = _split_variables(tokens[1:])` Suggestion importance[1-10]: 5 __ Why: The suggestion is valid — `cmd.finish` in nonebot raises `FinishedException` to terminate flow, so in practice the code won't fall through. However, adding `return` after `cmd.finish` is a reasonable defensive practice for type safety and resilience against future framework changes. It's a minor robustness improvement, not a real bug.	Low
Security	模板替换可能被递归注入 `_render_template` 的模板变量由用户输入控制，恶意用户可以构造包含特殊正则字符的 `content`，虽然当前正则本身是安全的，但更关键的问题是：用户提供的 `positional` 值如果包含 `{...}` 模式，替换结果会在同一次 `re.sub` 中被再次匹配处理，可能导致非预期的递归替换。建议对替换后的结果不再进行二次匹配，或使用迭代方式逐段构建输出。 konabot/plugins/syntactic_sugar/init.py [81-91] def _render_template(content: str, positional: list[str], named: dict[str, str]) -> str: - def replace(match: re.Match[str]) -> str: + result: list[str] = [] + last_end = 0 + for match in re.finditer(r"\{([^{}]+)\}", content): + result.append(content[last_end:match.start()]) key = match.group(1).strip() if key.isdigit(): idx = int(key) - 1 if 0 <= idx < len(positional): - return positional[idx] - return match.group(0) - return named.get(key, match.group(0)) + result.append(positional[idx]) + else: + result.append(match.group(0)) + else: + result.append(named.get(key, match.group(0))) + last_end = match.end() + result.append(content[last_end:]) + return "".join(result) - return re.sub(r"\{([^{}]+)\}", replace, content) - Suggestion importance[1-10]: 6 __ Why: The concern about recursive substitution is technically valid — `re.sub` processes the entire string in one pass, so replacement values containing `{...}` patterns won't actually be re-matched within the same `re.sub` call. However, `re.sub` replaces left-to-right and doesn't re-scan replaced text, so the "recursive replacement" claim is actually incorrect for a single `re.sub` call. The iterative approach is still slightly safer and more explicit, but the stated vulnerability doesn't exist in practice.	Low

## PR Code Suggestions ✨  Latest suggestions up to 8f40572 Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=2>Possible issue</td> <td> <details><summary>finish后缺少显式return保护</summary> ___ **在"查看"分支中，当 <code>content</code> 不为 <code>None</code> 时，代码会继续执行到 <code>await cmd.finish(...)</code> 并正常结束。但当 <code>content is None</code> <br>时，<code>cmd.finish</code> 虽然会抛出异常来终止流程，但从代码可读性和安全性角度看，缺少 <code>return</code> 或 <code>else</code> 会让后续的"糖展开"逻辑在 <code>finish</code> <br>未能正确终止时被意外执行。同样的问题也出现在最后的糖展开部分的 <code>None</code> 检查中。建议添加显式的 <code>return</code> 以确保流程安全。** [konabot/plugins/syntactic_sugar/__init__.py [194-197]](https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/__init__.py#L194-L197) ```diff content = await _find_sugar(name, target_id, channel_id) if content is None: await cmd.finish(f"没有糖：「{name}」") + return await cmd.finish(f"糖的内容：「{content}」") ``` <details><summary>Suggestion importance[1-10]: 4</summary> __ Why: In nonebot, `cmd.finish` raises a `FinishedException` to terminate the handler, so the `return` is not strictly necessary. However, adding a defensive `return` improves readability and guards against potential framework behavior changes. This is a minor defensive coding improvement, not a real bug. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>糖展开时缺少None防护</summary> ___ **在糖展开逻辑中，如果 <code>_find_sugar</code> 返回 <code>None</code>，<code>cmd.finish</code> <br>通过抛出异常来终止处理流程。但如果异常未被正确传播（例如框架行为变更），<code>content</code> 为 <code>None</code> 时代码会继续执行 <code>_render_template</code>，导致对 <br><code>None</code> 进行正则替换而产生错误。应在 <code>cmd.finish</code> 后添加 <code>return</code> 作为防御性编程。** [konabot/plugins/syntactic_sugar/__init__.py [201-205]](https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/__init__.py#L201-L205) ```diff content = await _find_sugar(name, target_id, channel_id) if content is None: await cmd.finish(f"没有糖：「{name}」") + return positional, named = _split_variables(tokens[1:]) ``` <details><summary>Suggestion importance[1-10]: 4</summary> __ Why: Same reasoning as suggestion 1 — `cmd.finish` raises an exception in nonebot so `content` being `None` won't actually reach `_render_template`. Adding `return` is a reasonable defensive measure but not fixing an actual bug. Minor improvement. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=1>Security</td> <td> <details><summary>模板展开存在命令注入风险</summary> ___ **<code>_render_template</code> 中用户可以通过模板变量 <code>{name}</code> 注入任意内容，而展开后的结果会被 <code>_reinject_command</code> <br>作为新命令重新注入事件处理。虽然递归深度限制为 <br>3，但恶意用户仍可通过构造糖内容来执行非预期的命令（如删除其他糖或触发其他插件命令）。建议对渲染后的结果进行命令白名单校验，或至少在文档中明确说明此安全风险。** [konabot/plugins/syntactic_sugar/__init__.py [82-89]](https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/__init__.py#L82-L89) ```diff +def replace(match: re.Match[str]) -> str: + key = match.group(1).strip() + if key.isdigit(): + idx = int(key) - 1 + if 0 <= idx < len(positional): + return positional[idx] + return match.group(0) + return named.get(key, match.group(0)) - ``` <details><summary>Suggestion importance[1-10]: 3</summary> __ Why: The security concern about command injection via template rendering into `_reinject_command` is valid conceptually, but the `existing_code` and `improved_code` are identical — no actual fix is proposed. The recursion depth is already capped at 3, and the feature is inherently designed to re-inject commands. Without a concrete code change, this is more of a discussion point than an actionable suggestion. </details></details></td><td align=center>Low </td></tr></tr></tbody></table> ___ #### Previous suggestions <details><summary>Suggestions up to commit 9064b31</summary> <br><table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=2>Possible issue</td> <td> <details><summary>分支条件应互斥避免穿透</summary> ___ **"存入"、"删除"、"查看"三个分支都使用 <code>if</code> 而非 <code>elif</code>，依赖 <code>cmd.finish</code> 抛异常来终止流程。如果 <code>cmd.finish</code> <br>在某些边界情况下未能抛出异常（例如被上层捕获），代码会穿透到下方的默认分支，导致"存入"或"删除"操作后还会尝试查找并执行语法糖。应改为 <code>elif</code> 链以确保互斥。** [konabot/plugins/syntactic_sugar/__init__.py [190-200]](https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/__init__.py#L190-L200) ```diff -if action == "查看": +elif action == "查看": if len(tokens) < 2: await cmd.finish("请提供要查看的名称") name = tokens[1].strip() content = await _find_sugar(name, target_id, channel_id) if content is None: await cmd.finish(f"没有糖：「{name}」") + return await cmd.finish(f"糖的内容：「{content}」") +else: + name = action - -name = action - ``` <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: Using `elif` instead of separate `if` statements is a valid structural improvement. In nonebot, `cmd.finish` raises `FinishedException` so fall-through won't happen in normal operation, but using `elif`/`else` makes the mutual exclusivity explicit and is more robust and readable. It's a good defensive coding practice but not a real bug given the framework's behavior. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>finish 后缺少 return 防护</summary> ___ **在"查看"分支和默认分支中，<code>_find_sugar</code> 返回 <code>None</code> 后调用 <code>cmd.finish</code> 虽然会抛出异常终止流程，但类型检查器无法感知这一点，<code>content</code> <br>在后续 <code>_render_template</code> 中仍可能被推断为 <code>None</code>。建议在 <code>cmd.finish</code> 后显式 <code>return</code>，以确保代码健壮性，避免在 <code>cmd.finish</code> <br>行为变更时 <code>content</code> 为 <code>None</code> 传入 <code>_render_template</code> 导致运行时错误。"查看"分支同理。** [konabot/plugins/syntactic_sugar/__init__.py [201-205]](https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/__init__.py#L201-L205) ```diff content = await _find_sugar(name, target_id, channel_id) if content is None: await cmd.finish(f"没有糖：「{name}」") + return positional, named = _split_variables(tokens[1:]) ``` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion is valid — `cmd.finish` in nonebot raises `FinishedException` to terminate flow, so in practice the code won't fall through. However, adding `return` after `cmd.finish` is a reasonable defensive practice for type safety and resilience against future framework changes. It's a minor robustness improvement, not a real bug. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=1>Security</td> <td> <details><summary>模板替换可能被递归注入</summary> ___ **<code>_render_template</code> 的模板变量由用户输入控制，恶意用户可以构造包含特殊正则字符的 <code>content</code>，虽然当前正则本身是安全的，但更关键的问题是：用户提供的 <br><code>positional</code> 值如果包含 <code>\{...\}</code> 模式，替换结果会在同一次 <code>re.sub</code> <br>中被再次匹配处理，可能导致非预期的递归替换。建议对替换后的结果不再进行二次匹配，或使用迭代方式逐段构建输出。** [konabot/plugins/syntactic_sugar/__init__.py [81-91]](https://gitea.service.jazzwhom.top/mttu-developers/konabot/src/branch/feature/sugar/konabot/plugins/syntactic_sugar/__init__.py#L81-L91) ```diff def _render_template(content: str, positional: list[str], named: dict[str, str]) -> str: - def replace(match: re.Match[str]) -> str: + result: list[str] = [] + last_end = 0 + for match in re.finditer(r"\{([^{}]+)\}", content): + result.append(content[last_end:match.start()]) key = match.group(1).strip() if key.isdigit(): idx = int(key) - 1 if 0 <= idx < len(positional): - return positional[idx] - return match.group(0) - return named.get(key, match.group(0)) + result.append(positional[idx]) + else: + result.append(match.group(0)) + else: + result.append(named.get(key, match.group(0))) + last_end = match.end() + result.append(content[last_end:]) + return "".join(result) - return re.sub(r"\{([^{}]+)\}", replace, content) - ``` <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The concern about recursive substitution is technically valid — `re.sub` processes the entire string in one pass, so replacement values containing `{...}` patterns won't actually be re-matched within the same `re.sub` call. However, `re.sub` replaces left-to-right and doesn't re-scan replaced text, so the "recursive replacement" claim is actually incorrect for a single `re.sub` call. The iterative approach is still slightly safer and more explicit, but the stated vulnerability doesn't exist in practice. </details></details></td><td align=center>Low </td></tr></tr></tbody></table> </details>

Passthem added 1 commit 2026-03-18 17:39:01 +08:00

Merge branch 'master' into feature/sugar d37c4870d8

Passthem merged commit 00f42dbdf1 into master

2026-03-18 17:39:36 +08:00

Passthem referenced this issue from a commit

2026-03-18 17:39:38 +08:00

Merge pull request '语法糖' (#61) from feature/sugar into master

kagami-review commented

2026-03-18 17:39:42 +08:00

Persistent review updated to latest commit 9064b31fe9

**[Persistent review](https://gitea.service.jazzwhom.top/mttu-developers/konabot/pulls/61#issuecomment-346)** updated to latest commit https://gitea.service.jazzwhom.top/mttu-developers/konabot/commit/9064b31fe9ab116df24f3fc2ba1944c175039a05

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mttu-developers/konabot#61

语法糖 #61

User description

PR Type

Description

Diagram Walkthrough

File Walkthrough

PR Reviewer Guide 🔍

(Review updated until commit 9064b31fe9)

PR Code Suggestions ✨

Previous suggestions

(Review updated until commit `9064b31fe9`)