feat: add password confirmation when changing password#5247
Merged
Soulter merged 3 commits intoAstrBotDevs:masterfrom Feb 20, 2026
Merged
feat: add password confirmation when changing password#5247Soulter merged 3 commits intoAstrBotDevs:masterfrom
Soulter merged 3 commits intoAstrBotDevs:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
Hey - 我发现了 4 个问题,并留下了一些总体反馈:
confirmPasswordRules目前在必填检查中使用!!newPassword.value,这会导致当newPassword为空时,确认密码字段显示“必填”错误;建议基于确认密码字段本身来做必填校验,和/或在没有输入新密码时完全跳过确认密码的校验。- 在
accountEdit中,你在发起请求前把newPassword和confirmPassword这两个 ref 直接改成了它们的 MD5 哈希值,这会在响应清空之前短暂地在输入框中显示哈希值;更干净的做法是保持 ref 内还是明文密码,然后单独构造用于请求载荷的哈希变量。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- `confirmPasswordRules` 目前在必填检查中使用 `!!newPassword.value`,这会导致当 `newPassword` 为空时,确认密码字段显示“必填”错误;建议基于确认密码字段本身来做必填校验,和/或在没有输入新密码时完全跳过确认密码的校验。
- 在 `accountEdit` 中,你在发起请求前把 `newPassword` 和 `confirmPassword` 这两个 ref 直接改成了它们的 MD5 哈希值,这会在响应清空之前短暂地在输入框中显示哈希值;更干净的做法是保持 ref 内还是明文密码,然后单独构造用于请求载荷的哈希变量。
## Individual Comments
### Comment 1
<location> `dashboard/src/components/extension/McpServersSection.vue:492-500` </location>
<code_context>
+ }
+ return normalized;
+ },
+ validateAgentScope() {
+ const normalized = this.normalizeAgentScope(this.currentServer.agent_scope);
+ const invalidScopes = normalized.filter((scope) => {
+ if (scope === '*') {
+ return false;
+ }
+ return !/^[a-z][a-z0-9_]{0,63}$/.test(scope);
+ });
+ if (invalidScopes.length > 0) {
+ this.scopeError = this.tm('dialogs.addServer.errors.agentScopeInvalid', {
+ value: invalidScopes.join(', ')
+ });
+ return false;
+ }
+ this.scopeError = '';
+ this.currentServer.agent_scope = normalized;
+ return true;
+ },
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 使前端的 scope 规范化/校验行为与后端保持一致,避免出现出乎意料的拒绝。
在前端,`normalizeAgentScope` 会保留原始大小写(除了 `'all'` → `'*'`),但 `validateAgentScope` 使用 `^[a-z][a-z0-9_]{0,63}$` 进行校验,会拒绝任何包含大写字符的 scope。而在后端,`normalize_mcp_scope_value` 会先把 scope token 转为小写再做匹配,所以像 `Main` 这样的值会被接受为 `main`。为避免“只在 UI 端被拒绝”的情况,建议在 `normalizeAgentScope` 中先统一转为小写再进行校验,或调整正则以匹配服务端的行为。
```suggestion
}
// Normalize case to align with backend behavior:
// - Preserve the special "*" token as-is
// - Lowercase all other scope values
const normalizedValue = value === '*' ? '*' : value.toLowerCase();
seen.add(normalizedValue);
normalized.push(normalizedValue);
}
if (normalized.includes('*')) {
return ['*'];
}
return normalized;
},
```
</issue_to_address>
### Comment 2
<location> `dashboard/src/layouts/full/vertical-header/VerticalHeader.vue:93-95` </location>
<code_context>
(v: string) => !!v || t('core.header.accountDialog.validation.passwordRequired'),
(v: string) => v.length >= 8 || t('core.header.accountDialog.validation.passwordMinLength')
]);
+const confirmPasswordRules = computed(() => [
+ (v: string) => !!newPassword.value || t('core.header.accountDialog.validation.passwordRequired'),
+ (v: string) => v === newPassword.value || t('core.header.accountDialog.validation.passwordMatch')
+]);
const usernameRules = computed(() => [
</code_context>
<issue_to_address>
**issue (bug_risk):** 确认密码规则的逻辑很可能与预期的交互体验不一致。
第一条规则使用的是 `!!newPassword.value` 而不是确认字段自身的值 `v`,这会导致当 `newPassword` 为空时确认字段被标记为“必填”,但在输入了新密码时又没有真正要求填写确认密码。建议改为:仅当 `newPassword.value` 有值时才要求确认密码必填(例如使用 `!newPassword.value || !!v`),然后在第二条规则中再进行相等性检查。
</issue_to_address>
### Comment 3
<location> `tests/test_plugin_tool_fix.py:183` </location>
<code_context>
+ assert len(req.func_tool.tools) == 1
+ assert req.func_tool.tools[0].name == "mcp_tool"
+
+ def test_plugins_name_set_filters_regular_tools(self):
+ """When plugins_name is set, only tools from those plugins should be kept."""
+ event = MockEvent(plugins_name=["plugin_a"])
</code_context>
<issue_to_address>
**suggestion (testing):** 为 `plugins_name=[]`(空列表)的行为添加测试
当前测试覆盖了 `plugins_name=None` 和非空列表的情况,但没有覆盖 `plugins_name=[]`。在该情况下逻辑依然会走过滤分支(因为 `event.plugins_name is not None`)。请添加一个 `plugins_name=[]` 的测试,并混合使用 plugin 工具和 MCP 工具,以确认在未选择任何插件时 MCP 工具会被保留,而插件工具会被过滤掉。
建议在:
```python
def test_plugins_name_set_filters_regular_tools(self):
```
这一节中进行扩展。
变更如下:
<file_operations>
<file_operation operation="edit" file_path="tests/test_plugin_tool_fix.py">
<<<<<<< SEARCH
def test_plugins_name_set_filters_regular_tools(self):
=======
def test_plugins_name_set_filters_regular_tools(self):
>>>>>>> REPLACE
</file_operation>
</file_operations>
<additional_changes>
请在 `tests/test_plugin_tool_fix.py` 中其他与 `plugins_name` 相关的测试附近(例如紧接在 `test_plugins_name_set_filters_regular_tools` 之后)添加如下新的测试函数:
```python
def test_plugins_name_empty_list_filters_all_plugin_tools_but_keeps_mcp(self):
"""When plugins_name is [], all plugin tools are filtered out but MCP tools are preserved."""
event = MockEvent(plugins_name=[])
req = MockProviderRequest()
# Create tools with different plugin associations
tool_a = MockFunctionTool("tool_a", handler_module_path="plugins.plugin_a.main")
tool_b = MockFunctionTool("tool_b", handler_module_path="plugins.plugin_b.main")
mcp_tool = MockMCPTool("mcp_tool")
# Start with both plugin-backed tools and an MCP tool
req.func_tool = ToolSet([tool_a, tool_b, mcp_tool])
# Map tool names to plugin names so plugin_tool_fix_logic can filter them
star_map = {
"tool_a": "plugin_a",
"tool_b": "plugin_b",
}
# Run the logic; with plugins_name=[], we go down the filtering path
# but no plugin names are selected, so all plugin-backed tools should be removed.
plugin_tool_fix_logic(event, req, star_map, [mcp_tool])
# Only MCP tool should remain
assert len(req.func_tool.tools) == 1
assert req.func_tool.tools[0].name == "mcp_tool"
```
该测试假设:
- `MockEvent`、`MockProviderRequest`、`MockFunctionTool`、`MockMCPTool`、`ToolSet` 和 `plugin_tool_fix_logic` 已在该测试模块中导入/可用(与现有测试保持一致)。
- 过滤逻辑是通过 `event.plugins_name is not None` 来决定是否执行过滤,并将 `plugins_name=[]` 视为“未选择任何插件”,即不保留任何基于插件的工具,但仍保留 MCP 工具。
</issue_to_address>
### Comment 4
<location> `astrbot/core/astr_main_agent.py:765` </location>
<code_context>
-def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None:
+def _plugin_tool_fix(
+ event: AstrMessageEvent,
+ req: ProviderRequest,
</code_context>
<issue_to_address>
**issue (complexity):** 建议抽取一个共享的辅助函数,用于根据 agent 的可见性过滤 MCP 工具,并在 `_plugin_tool_fix` 末尾统一调用,这样可见性只在一个地方处理,而不是在每个分支中交织处理。
你可以通过将 MCP 可见性集中到一个辅助函数中,并在 `_plugin_tool_fix` 的最后统一应用,来简化当前的实现,而不是在各个分支中混合可见性检查。
### 1. 抽取可见性辅助函数
```python
def _filter_mcp_visibility(tool_set: ToolSet | None, agent_name: str) -> ToolSet | None:
if not tool_set:
return tool_set
filtered = ToolSet()
for tool in tool_set.tools:
if isinstance(tool, MCPTool) and not is_mcp_tool_visible_to_agent(tool, agent_name):
continue
filtered.add_tool(tool)
return filtered
```
### 2. 简化 `_plugin_tool_fix`
移除顶层的 MCP 过滤和各分支中的可见性检查,只在函数末尾做一次统一处理:
```python
def _plugin_tool_fix(
event: AstrMessageEvent,
req: ProviderRequest,
inject_mcp: bool = True,
*,
agent_name: str = "main",
) -> None:
"""
根据事件中的插件设置,过滤请求中的工具列表。
注意:没有 handler_module_path 的工具(如 MCP 工具)会被保留,
因为它们不属于任何插件,不应被插件过滤逻辑影响。
Args:
event: 消息事件
req: 提供者请求
inject_mcp: 是否自动注入全局 MCP 工具,默认 True(向后兼容)
"""
if event.plugins_name is not None and req.func_tool:
new_tool_set = ToolSet()
for tool in req.func_tool.tools:
if isinstance(tool, MCPTool):
# MCP 工具不受插件过滤影响,先全部保留
new_tool_set.add_tool(tool)
continue
mp = tool.handler_module_path
if not mp:
continue
plugin = star_map.get(mp)
if not plugin:
continue
if plugin.name in event.plugins_name or plugin.reserved:
new_tool_set.add_tool(tool)
req.func_tool = new_tool_set
elif inject_mcp:
tool_set = req.func_tool or ToolSet()
for tool in llm_tools.func_list:
if isinstance(tool, MCPTool):
tool_set.add_tool(tool)
req.func_tool = tool_set
# 统一在最后按 agent 过滤 MCP 可见性
req.func_tool = _filter_mcp_visibility(req.func_tool, agent_name)
```
这样可以保持当前所有行为:
- 仍然会对以下情况强制执行 MCP 可见性检查:
- 已存在的工具,
- 经过插件过滤后的工具,
- 注入的全局 MCP 工具。
- 插件过滤逻辑保持不变,但不再重复检查可见性。
- 控制流更加清晰:先做插件过滤/注入,再作为单独步骤处理可见性。
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 4 issues, and left some high level feedback:
- The
confirmPasswordRulescurrently use!!newPassword.valuefor the required check, which will make the confirm field show a required error whenevernewPasswordis empty; consider basing the required rule on the confirm field itself and/or skipping confirm validation entirely when no new password is entered. - In
accountEdityou mutatenewPasswordandconfirmPasswordrefs to their MD5 hashes before the request, which briefly shows the hashed value in the input until the response clears it; it would be cleaner to leave the refs as plain text and build separate hashed variables for the payload.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `confirmPasswordRules` currently use `!!newPassword.value` for the required check, which will make the confirm field show a required error whenever `newPassword` is empty; consider basing the required rule on the confirm field itself and/or skipping confirm validation entirely when no new password is entered.
- In `accountEdit` you mutate `newPassword` and `confirmPassword` refs to their MD5 hashes before the request, which briefly shows the hashed value in the input until the response clears it; it would be cleaner to leave the refs as plain text and build separate hashed variables for the payload.
## Individual Comments
### Comment 1
<location> `dashboard/src/components/extension/McpServersSection.vue:492-500` </location>
<code_context>
+ }
+ return normalized;
+ },
+ validateAgentScope() {
+ const normalized = this.normalizeAgentScope(this.currentServer.agent_scope);
+ const invalidScopes = normalized.filter((scope) => {
+ if (scope === '*') {
+ return false;
+ }
+ return !/^[a-z][a-z0-9_]{0,63}$/.test(scope);
+ });
+ if (invalidScopes.length > 0) {
+ this.scopeError = this.tm('dialogs.addServer.errors.agentScopeInvalid', {
+ value: invalidScopes.join(', ')
+ });
+ return false;
+ }
+ this.scopeError = '';
+ this.currentServer.agent_scope = normalized;
+ return true;
+ },
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Align front-end scope normalization/validation with back-end behavior to avoid surprising rejections.
On the front end, `normalizeAgentScope` keeps the original casing (aside from `'all'` → `'*'`), but `validateAgentScope` enforces `^[a-z][a-z0-9_]{0,63}$`, which rejects any scopes with uppercase characters. On the back end, `normalize_mcp_scope_value` lowercases scope tokens before matching, so a value like `Main` is accepted as `main`. To avoid UI-only rejections, consider lowercasing in `normalizeAgentScope` before validation or adjusting the regex to match the server’s behavior.
```suggestion
}
// Normalize case to align with backend behavior:
// - Preserve the special "*" token as-is
// - Lowercase all other scope values
const normalizedValue = value === '*' ? '*' : value.toLowerCase();
seen.add(normalizedValue);
normalized.push(normalizedValue);
}
if (normalized.includes('*')) {
return ['*'];
}
return normalized;
},
```
</issue_to_address>
### Comment 2
<location> `dashboard/src/layouts/full/vertical-header/VerticalHeader.vue:93-95` </location>
<code_context>
(v: string) => !!v || t('core.header.accountDialog.validation.passwordRequired'),
(v: string) => v.length >= 8 || t('core.header.accountDialog.validation.passwordMinLength')
]);
+const confirmPasswordRules = computed(() => [
+ (v: string) => !!newPassword.value || t('core.header.accountDialog.validation.passwordRequired'),
+ (v: string) => v === newPassword.value || t('core.header.accountDialog.validation.passwordMatch')
+]);
const usernameRules = computed(() => [
</code_context>
<issue_to_address>
**issue (bug_risk):** The confirm password rule logic likely doesn’t match the intended UX.
The first rule uses `!!newPassword.value` instead of the confirm field value `v`, which makes the confirm field “required” when `newPassword` is empty, and doesn’t actually require the confirm field when a new password is entered. Consider instead making confirm required only when `newPassword.value` is set (e.g. `!newPassword.value || !!v`) and then applying the equality check in the second rule.
</issue_to_address>
### Comment 3
<location> `tests/test_plugin_tool_fix.py:183` </location>
<code_context>
+ assert len(req.func_tool.tools) == 1
+ assert req.func_tool.tools[0].name == "mcp_tool"
+
+ def test_plugins_name_set_filters_regular_tools(self):
+ """When plugins_name is set, only tools from those plugins should be kept."""
+ event = MockEvent(plugins_name=["plugin_a"])
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for `plugins_name=[]` (empty list) behavior
Current tests cover `plugins_name=None` and a non-empty list, but not `plugins_name=[]`, which still goes down the filtering path (`event.plugins_name is not None`). Please add a test with `plugins_name=[]` and a mix of plugin and MCP tools to confirm MCP tools are preserved while plugin tools are filtered out when no plugin names are selected.
Suggested implementation:
```python
def test_plugins_name_set_filters_regular_tools(self):
```
` section.
Here are the changes:
<file_operations>
<file_operation operation="edit" file_path="tests/test_plugin_tool_fix.py">
<<<<<<< SEARCH
def test_plugins_name_set_filters_regular_tools(self):
=======
def test_plugins_name_set_filters_regular_tools(self):
>>>>>>> REPLACE
</file_operation>
</file_operations>
<additional_changes>
Please add the following new test function near the other `plugins_name`-related tests in `tests/test_plugin_tool_fix.py` (for example, immediately after `test_plugins_name_set_filters_regular_tools`):
```python
def test_plugins_name_empty_list_filters_all_plugin_tools_but_keeps_mcp(self):
"""When plugins_name is [], all plugin tools are filtered out but MCP tools are preserved."""
event = MockEvent(plugins_name=[])
req = MockProviderRequest()
# Create tools with different plugin associations
tool_a = MockFunctionTool("tool_a", handler_module_path="plugins.plugin_a.main")
tool_b = MockFunctionTool("tool_b", handler_module_path="plugins.plugin_b.main")
mcp_tool = MockMCPTool("mcp_tool")
# Start with both plugin-backed tools and an MCP tool
req.func_tool = ToolSet([tool_a, tool_b, mcp_tool])
# Map tool names to plugin names so plugin_tool_fix_logic can filter them
star_map = {
"tool_a": "plugin_a",
"tool_b": "plugin_b",
}
# Run the logic; with plugins_name=[], we go down the filtering path
# but no plugin names are selected, so all plugin-backed tools should be removed.
plugin_tool_fix_logic(event, req, star_map, [mcp_tool])
# Only MCP tool should remain
assert len(req.func_tool.tools) == 1
assert req.func_tool.tools[0].name == "mcp_tool"
```
This test assumes:
- `MockEvent`, `MockProviderRequest`, `MockFunctionTool`, `MockMCPTool`, `ToolSet`, and `plugin_tool_fix_logic` are already imported/available in this test module (as they are for the existing tests you showed).
- The filtering logic uses `event.plugins_name is not None` to trigger filtering, and treats `plugins_name=[]` as “no plugins selected”, meaning no plugin-backed tools should be retained while MCP tools are still kept.
</issue_to_address>
### Comment 4
<location> `astrbot/core/astr_main_agent.py:765` </location>
<code_context>
-def _plugin_tool_fix(event: AstrMessageEvent, req: ProviderRequest) -> None:
+def _plugin_tool_fix(
+ event: AstrMessageEvent,
+ req: ProviderRequest,
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting a shared helper to filter MCP tools by agent visibility and calling it once at the end of `_plugin_tool_fix` so visibility is handled in a single place rather than interleaved in each branch.
You can simplify `_plugin_tool_fix` by centralizing MCP visibility into a single helper and applying it once at the end, instead of mixing visibility checks into each branch.
### 1. Extract a visibility helper
```python
def _filter_mcp_visibility(tool_set: ToolSet | None, agent_name: str) -> ToolSet | None:
if not tool_set:
return tool_set
filtered = ToolSet()
for tool in tool_set.tools:
if isinstance(tool, MCPTool) and not is_mcp_tool_visible_to_agent(tool, agent_name):
continue
filtered.add_tool(tool)
return filtered
```
### 2. Simplify `_plugin_tool_fix`
Remove the top-level MCP filtering and per-branch visibility checks, and do a single pass at the end:
```python
def _plugin_tool_fix(
event: AstrMessageEvent,
req: ProviderRequest,
inject_mcp: bool = True,
*,
agent_name: str = "main",
) -> None:
"""
根据事件中的插件设置,过滤请求中的工具列表。
注意:没有 handler_module_path 的工具(如 MCP 工具)会被保留,
因为它们不属于任何插件,不应被插件过滤逻辑影响。
Args:
event: 消息事件
req: 提供者请求
inject_mcp: 是否自动注入全局 MCP 工具,默认 True(向后兼容)
"""
if event.plugins_name is not None and req.func_tool:
new_tool_set = ToolSet()
for tool in req.func_tool.tools:
if isinstance(tool, MCPTool):
# MCP 工具不受插件过滤影响,先全部保留
new_tool_set.add_tool(tool)
continue
mp = tool.handler_module_path
if not mp:
continue
plugin = star_map.get(mp)
if not plugin:
continue
if plugin.name in event.plugins_name or plugin.reserved:
new_tool_set.add_tool(tool)
req.func_tool = new_tool_set
elif inject_mcp:
tool_set = req.func_tool or ToolSet()
for tool in llm_tools.func_list:
if isinstance(tool, MCPTool):
tool_set.add_tool(tool)
req.func_tool = tool_set
# 统一在最后按 agent 过滤 MCP 可见性
req.func_tool = _filter_mcp_visibility(req.func_tool, agent_name)
```
This keeps all current behavior:
- MCP visibility is still enforced for:
- existing tools,
- plugin-filtered tools,
- injected global MCP tools.
- Plugin filtering logic is unchanged, but no longer rechecks visibility.
- The control flow is clearer: plugin filtering / injection first, then visibility as a separate concern.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request implements password confirmation functionality when changing passwords (fixes #5179) to prevent accidental typos. Additionally, it includes significant enhancements to the MCP (Model Context Protocol) scope filtering system, allowing fine-grained control over which agents can access specific MCP tools.
Changes:
- Added password confirmation field and validation in the account settings dialog
- Implemented comprehensive MCP scope management with agent-level visibility controls
- Added test coverage for MCP scope functionality and plugin tool filtering logic
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| astrbot/dashboard/routes/auth.py | Added backend validation for password confirmation matching |
| dashboard/src/layouts/full/vertical-header/VerticalHeader.vue | Added confirmation password input field with validation rules |
| dashboard/src/i18n/locales/zh-CN/core/header.json | Added Chinese translations for password confirmation labels and validation messages |
| dashboard/src/i18n/locales/en-US/core/header.json | Added English translations for password confirmation labels and validation messages |
| astrbot/core/agent/mcp_scope.py | New module implementing MCP scope normalization and visibility checking logic |
| astrbot/core/agent/mcp_client.py | Added mcp_server_scopes parameter to MCPTool and scope field stripping |
| astrbot/core/provider/func_tool_manager.py | Integrated scope extraction and assignment to MCP tools during initialization |
| astrbot/core/astr_main_agent.py | Enhanced tool filtering with scope-based visibility checks for agents |
| astrbot/core/astr_agent_tool_exec.py | Added scope validation when building tool sets for handoff agents |
| astrbot/core/config/default.py | Added inject_global_mcp_tools configuration option |
| dashboard/src/components/extension/McpServersSection.vue | Implemented comprehensive UI for MCP scope selection and management |
| dashboard/src/i18n/locales/zh-CN/features/tool-use.json | Added Chinese translations for scope-related UI elements |
| dashboard/src/i18n/locales/en-US/features/tool-use.json | Added English translations for scope-related UI elements |
| astrbot/dashboard/routes/tools.py | Enhanced tool list API to include MCP server name and scopes |
| tests/test_plugin_tool_fix.py | New comprehensive test suite for plugin tool filtering logic |
| tests/test_mcp_scope.py | New test suite for MCP scope normalization and visibility functions |
Fixes AstrBotDevs#5177 Adds a password confirmation field to prevent accidental password typos. Changes: - Backend: validate confirm_password matches new_password - Frontend: add confirmation input with validation - i18n: add labels and error messages for password mismatch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
a37d691 to
6ce8507
Compare
Contributor
There was a problem hiding this comment.
Hey - 我给出了一些整体性的反馈:
confirmPasswordRules的第一个规则检查的是!!newPassword.value而不是确认密码字段的值,这会导致即使确认字段本身为空时,也只显示一个通用的「密码必填」错误;建议把必填校验和对应的提示信息基于v(以及一个确认密码专用的 i18n 文案 key),这样该字段的校验行为会更符合预期。- 后端的不一致提示信息
"两次输入的新密码不一致,健忘症患者?"是用户可见的,而且略带贬义;最好将其调整为与前端使用的更为中性的 i18n 校验文案保持一致,以统一语气和风格。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The `confirmPasswordRules` first rule checks `!!newPassword.value` instead of the confirm field value, which will surface a generic password-required error even when the confirm field itself is empty; consider basing the required check and message on `v` (and a confirm-specific i18n key) so validation behaves as expected for that field.
- The backend mismatch message `"两次输入的新密码不一致,健忘症患者?"` is user-visible and a bit pejorative; it would be better to align it with the more neutral i18n validation copy used on the frontend for consistency and tone.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- The
confirmPasswordRulesfirst rule checks!!newPassword.valueinstead of the confirm field value, which will surface a generic password-required error even when the confirm field itself is empty; consider basing the required check and message onv(and a confirm-specific i18n key) so validation behaves as expected for that field. - The backend mismatch message
"两次输入的新密码不一致,健忘症患者?"is user-visible and a bit pejorative; it would be better to align it with the more neutral i18n validation copy used on the frontend for consistency and tone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `confirmPasswordRules` first rule checks `!!newPassword.value` instead of the confirm field value, which will surface a generic password-required error even when the confirm field itself is empty; consider basing the required check and message on `v` (and a confirm-specific i18n key) so validation behaves as expected for that field.
- The backend mismatch message `"两次输入的新密码不一致,健忘症患者?"` is user-visible and a bit pejorative; it would be better to align it with the more neutral i18n validation copy used on the frontend for consistency and tone.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Soulter
approved these changes
Feb 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5179
Adds password confirmation when changing password to prevent accidental typos.
Changes
Backend (
astrbot/dashboard/routes/auth.py)confirm_passwordparameter validationFrontend (
dashboard/src/layouts/full/vertical-header/VerticalHeader.vue)confirmPasswordRulesvalidationshowConfirmPasswordtogglei18n
confirmPasswordlabelconfirmPasswordHinthintpasswordMatchvalidation errorVerification
Summary by Sourcery
为账户密码修改流程添加密码确认支持,以防止保存非预期的密码。
新功能:
confirm_password,当确认密码与新密码不匹配时拒绝更新账户信息。增强改进:
Original summary in English
Summary by Sourcery
Add password confirmation support to the account password change flow to prevent saving unintended passwords.
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
为账户密码修改流程添加密码确认支持,以防止保存非预期的密码。
新功能:
confirm_password,当确认密码与新密码不匹配时拒绝更新账户信息。增强改进:
Original summary in English
Summary by Sourcery
Add password confirmation support to the account password change flow to prevent saving unintended passwords.
New Features:
Enhancements: