Skip to content

Conversation

@Soulter
Copy link
Member

@Soulter Soulter commented Dec 27, 2025

fixes: #4218

Modifications / 改动点

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果


Checklist / 检查清单

  • 😊 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。/ If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  • 👀 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。/ My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  • 🤓 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到了 requirements.txtpyproject.toml 文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
  • 😮 我的更改没有引入恶意代码。/ My changes do not introduce malicious code.

Summary by Sourcery

Bug Fixes:

  • 使用 DingTalk 的 sender_staff_id 而不是 sender_id 来构造 @ 提及,以确保机器人在回复时能够正确提及原始发送者。
Original summary in English

Summary by Sourcery

Bug Fixes:

  • Use DingTalk sender_staff_id instead of sender_id when constructing @ mentions to ensure the bot correctly mentions the original sender in replies.

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Dec 27, 2025
@Soulter Soulter merged commit f0fff68 into master Dec 27, 2025
4 of 5 checks passed
@Soulter Soulter deleted the fix/4218 branch December 27, 2025 03:26
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - 我发现了 1 个问题,并留下了一些总体反馈:

  • send_with_client 中的调试语句 print(i.qq, icm.sender_id, icm.sender_staff_id) 会在生产环境中把内部 ID 打印到 stdout;建议移除它,或者改为使用现有的日志机制并选择合适的日志级别。
  • 你已经在 send_with_client 开头通过 icm = cast(dingtalk_stream.ChatbotMessage, self.message_obj.raw_message) 赋值了 icm,但在调用 client.reply_markdown 时又重新做了一次类型转换;建议在那里的调用中复用 icm 变量,以避免重复转换并让代码更清晰。
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `print(i.qq, icm.sender_id, icm.sender_staff_id)` debug statement in `send_with_client` will leak internal IDs to stdout in production; consider removing it or switching to the existing logging mechanism with an appropriate log level.
- You already assign `icm = cast(dingtalk_stream.ChatbotMessage, self.message_obj.raw_message)` at the top of `send_with_client` but still re-cast in the `client.reply_markdown` call; reuse the `icm` variable there to avoid redundant casting and keep the code clearer.

## Individual Comments

### Comment 1
<location> `astrbot/core/platform/sources/dingtalk/dingtalk_event.py:34` </location>
<code_context>
+        # 钉钉 at 机器人需要使用 sender_staff_id 而不是 sender_id
+        for i in message.chain:
+            if isinstance(i, Comp.At):
+                print(i.qq, icm.sender_id, icm.sender_staff_id)
+                if str(i.qq) in str(icm.sender_id or ""):
+                    # 适配器会将开头的 $:LWCP_v1:$ 去掉,因此我们用 in 判断
</code_context>

<issue_to_address>
**🚨 issue (security):** Remove the debug `print` to avoid noisy logs and potential PII leakage.

These IDs (including `sender_staff_id`) are sensitive. Logging them directly can expose user information and clutter production logs. Please remove this `print`, or use the project logger with an appropriate level and redaction if you need this information for debugging.
</issue_to_address>

Sourcery 对开源项目免费使用——如果你觉得我们的代码评审有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的代码评审。
Original comment in English

Hey - I've found 1 issue, and left some high level feedback:

  • The print(i.qq, icm.sender_id, icm.sender_staff_id) debug statement in send_with_client will leak internal IDs to stdout in production; consider removing it or switching to the existing logging mechanism with an appropriate log level.
  • You already assign icm = cast(dingtalk_stream.ChatbotMessage, self.message_obj.raw_message) at the top of send_with_client but still re-cast in the client.reply_markdown call; reuse the icm variable there to avoid redundant casting and keep the code clearer.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `print(i.qq, icm.sender_id, icm.sender_staff_id)` debug statement in `send_with_client` will leak internal IDs to stdout in production; consider removing it or switching to the existing logging mechanism with an appropriate log level.
- You already assign `icm = cast(dingtalk_stream.ChatbotMessage, self.message_obj.raw_message)` at the top of `send_with_client` but still re-cast in the `client.reply_markdown` call; reuse the `icm` variable there to avoid redundant casting and keep the code clearer.

## Individual Comments

### Comment 1
<location> `astrbot/core/platform/sources/dingtalk/dingtalk_event.py:34` </location>
<code_context>
+        # 钉钉 at 机器人需要使用 sender_staff_id 而不是 sender_id
+        for i in message.chain:
+            if isinstance(i, Comp.At):
+                print(i.qq, icm.sender_id, icm.sender_staff_id)
+                if str(i.qq) in str(icm.sender_id or ""):
+                    # 适配器会将开头的 $:LWCP_v1:$ 去掉,因此我们用 in 判断
</code_context>

<issue_to_address>
**🚨 issue (security):** Remove the debug `print` to avoid noisy logs and potential PII leakage.

These IDs (including `sender_staff_id`) are sensitive. Logging them directly can expose user information and clutter production logs. Please remove this `print`, or use the project logger with an appropriate level and redaction if you need this information for debugging.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

# 钉钉 at 机器人需要使用 sender_staff_id 而不是 sender_id
for i in message.chain:
if isinstance(i, Comp.At):
print(i.qq, icm.sender_id, icm.sender_staff_id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): 删除调试用的 print,以避免产生噪声日志并防止潜在的个人敏感信息泄露。

这些 ID(包括 sender_staff_id)都是敏感信息。直接记录到日志中可能会暴露用户信息,并让生产环境的日志变得杂乱。请移除这个 print,或者在确实需要这些信息进行调试时,使用项目中的日志记录器、设置合适的日志级别并对敏感信息做脱敏处理。

Original comment in English

🚨 issue (security): Remove the debug print to avoid noisy logs and potential PII leakage.

These IDs (including sender_staff_id) are sensitive. Logging them directly can expose user information and clutter production logs. Please remove this print, or use the project logger with an appropriate level and redaction if you need this information for debugging.

@dosubot dosubot bot added the area:platform The bug / feature is about IM platform adapter, such as QQ, Lark, Telegram, WebChat and so on. label Dec 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:platform The bug / feature is about IM platform adapter, such as QQ, Lark, Telegram, WebChat and so on. size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]

2 participants