feat: enhance plugin page internationalization#7998
Conversation
- Updated PluginRoute to read initial context from JWT and set it in the bridge SDK. - Added methods to retrieve locale and plugin metadata for better i18n support. - Enhanced pluginI18n utility to resolve page-specific translations and added new functions for page titles and descriptions. - Modified PluginPagePage and PluginDetailPage to utilize new i18n features for dynamic content rendering. - Improved documentation for plugin page i18n structure and usage. - Added tests to verify the correct integration of i18n in plugin pages and context handling.
dosubot
Bot
added
size:XL
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
_get_request_localehelper and the bridge’sgetLocale()/t()logic both work with rawAccept-Languagevalues; consider normalizing these to your supported locales (e.g., mappingen→en-US) so that server-side page titles/descriptions and client-sidet()resolutions consistently hit the same i18n entries instead of falling back to metadata in some locales.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `_get_request_locale` helper and the bridge’s `getLocale()/t()` logic both work with raw `Accept-Language` values; consider normalizing these to your supported locales (e.g., mapping `en` → `en-US`) so that server-side page titles/descriptions and client-side `t()` resolutions consistently hit the same i18n entries instead of falling back to metadata in some locales.
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/routes/plugin.py" line_range="192-196" />
<code_context>
404, "Plugin Page bridge SDK not found"
)
- bridge_js = await self._read_plugin_page_binary(_PLUGIN_PAGE_BRIDGE_FILE)
+ bridge_js = await self._read_plugin_page_text(_PLUGIN_PAGE_BRIDGE_FILE)
+ initial_context = self._get_plugin_page_initial_context()
+ if initial_context:
+ context_json = json.dumps(initial_context, ensure_ascii=False)
+ bridge_js += (
+ f"\n;window.AstrBotPluginPage?.__setInitialContext({context_json});\n"
+ )
</code_context>
<issue_to_address>
**🚨 issue (security):** Inline injection of JSON context into bridge JS can break the script or introduce XSS edge cases.
Here `initial_context` (including `plugin_i18n`) is `json.dumps`’d and directly concatenated into the JS: `;window.AstrBotPluginPage?.__setInitialContext({context_json});`. If any value contains `</script>` or U+2028/U+2029, the browser can treat this as script-terminating or inject arbitrary JS.
To harden this:
- Prefer not to inline the i18n payload; let the iframe fetch it via the bridge API or a dedicated endpoint.
- If you must inline, post-process the JSON string (e.g., replace `</script` with `<\/script` and escape U+2028/U+2029) before appending so the resulting script is JS-safe.
Since plugin authors control this content, this is a realistic XSS vector and should be addressed before shipping.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| bridge_js = await self._read_plugin_page_text(_PLUGIN_PAGE_BRIDGE_FILE) | ||
| initial_context = self._get_plugin_page_initial_context() | ||
| if initial_context: | ||
| context_json = json.dumps(initial_context, ensure_ascii=False) | ||
| bridge_js += ( |
There was a problem hiding this comment.
🚨 issue (security): Inline injection of JSON context into bridge JS can break the script or introduce XSS edge cases.
Here initial_context (including plugin_i18n) is json.dumps’d and directly concatenated into the JS: ;window.AstrBotPluginPage?.__setInitialContext({context_json});. If any value contains </script> or U+2028/U+2029, the browser can treat this as script-terminating or inject arbitrary JS.
To harden this:
- Prefer not to inline the i18n payload; let the iframe fetch it via the bridge API or a dedicated endpoint.
- If you must inline, post-process the JSON string (e.g., replace
</scriptwith<\/scriptand escape U+2028/U+2029) before appending so the resulting script is JS-safe.
Since plugin authors control this content, this is a realistic XSS vector and should be addressed before shipping.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements internationalization (i18n) for plugin pages, enabling localized titles and descriptions through an enhanced bridge SDK and backend context injection. Key changes include new bridge methods for locale access, backend logic to provide initial context, and UI integration in the dashboard, supported by updated documentation and tests. Feedback focuses on improving code idiomaticity, such as using for...of loops, leveraging standard library functions for path traversal, simplifying logic with modern JavaScript syntax, and optimizing data serialization for performance.
| function notifyContextHandlers() { | ||
| contextHandlers.forEach((handler) => { | ||
| try { | ||
| handler(context); | ||
| } catch (error) { | ||
| console.error("AstrBotPluginPage context handler failed:", error); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
The notifyContextHandlers function uses a try-catch block inside a forEach loop. While this prevents one failing handler from stopping others, it is generally better to use a for...of loop for better readability and to avoid potential issues with forEach and async/await if handlers were to become asynchronous in the future.
| def _get_by_path(source: dict | None, key: str): | ||
| if not isinstance(source, dict) or not key: | ||
| return None | ||
| current = source | ||
| for part in key.split("."): | ||
| if not isinstance(current, dict) or part not in current: | ||
| return None | ||
| current = current[part] | ||
| return current |
There was a problem hiding this comment.
The _get_by_path method is implemented manually. Python's functools.reduce or a similar approach could make this more concise, though the current implementation is clear. Consider using a more robust approach if this pattern is used elsewhere.
| function getPluginPageI18nBase(page) { | ||
| if (page && typeof page === "object") { | ||
| if (typeof page.i18n_key === "string" && page.i18n_key.trim()) { | ||
| return page.i18n_key.trim(); | ||
| } | ||
| const pageName = page.page_name || page.name; | ||
| return pageName ? `pages.${pageName}` : ""; | ||
| } | ||
|
|
||
| return page ? `pages.${page}` : ""; | ||
| } |
There was a problem hiding this comment.
The getPluginPageI18nBase function could be simplified by using optional chaining and nullish coalescing operators to reduce nested if statements.
function getPluginPageI18nBase(page) {
if (typeof page === "object" && page !== null) {
if (page.i18n_key?.trim()) return page.i18n_key.trim();
const pageName = page.page_name ?? page.name;
return pageName ? "pages." + pageName : "";
}
return page ? "pages." + page : "";
}| const toPostMessageData = (value, fallback = null) => { | ||
| try { | ||
| return JSON.parse(JSON.stringify(toRaw(value))); | ||
| } catch { | ||
| return fallback; | ||
| } | ||
| }; |
There was a problem hiding this comment.
The toPostMessageData function uses JSON.parse(JSON.stringify(toRaw(value))) to deep clone and strip reactivity. While effective, this is a heavy operation. If the data structure is known to be simple, consider a more efficient approach or ensure this is only called when necessary.
* feat: enhance plugin page internationalization - Updated PluginRoute to read initial context from JWT and set it in the bridge SDK. - Added methods to retrieve locale and plugin metadata for better i18n support. - Enhanced pluginI18n utility to resolve page-specific translations and added new functions for page titles and descriptions. - Modified PluginPagePage and PluginDetailPage to utilize new i18n features for dynamic content rendering. - Improved documentation for plugin page i18n structure and usage. - Added tests to verify the correct integration of i18n in plugin pages and context handling. * fix test
1 participant
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。
🤓 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.txtandpyproject.toml./ 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Enhance plugin page internationalization by propagating locale-aware i18n context through the bridge, updating dashboard views and utilities to use page-specific translations, expanding bridge APIs for locale and i18n access, and documenting the new behavior in both English and Chinese guides.
New Features:
Enhancements:
Tests: