-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: at sender users not working in dingtalk #4219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的代码评审。
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 insend_with_clientwill 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 ofsend_with_clientbut still re-cast in theclient.reply_markdowncall; reuse theicmvariable 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>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) |
There was a problem hiding this comment.
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.
fixes: #4218
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
Bug Fixes:
sender_staff_id而不是sender_id来构造 @ 提及,以确保机器人在回复时能够正确提及原始发送者。Original summary in English
Summary by Sourcery
Bug Fixes: