Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 91 additions & 7 deletions astrbot/dashboard/plugin_page_bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const SELF_ORIGIN = window.location.origin;
const pendingRequests = new Map();
const sseHandlers = new Map();
const contextHandlers = new Set();
let requestCounter = 0;
let subscriptionCounter = 0;
let context = null;
Expand All @@ -13,7 +14,11 @@
});

function getTargetOrigin() {
if (typeof parentOrigin === "string" && parentOrigin && parentOrigin !== "null") {
if (
typeof parentOrigin === "string" &&
parentOrigin &&
parentOrigin !== "null"
) {
return parentOrigin;
}
if (SELF_ORIGIN !== "null") {
Expand Down Expand Up @@ -70,6 +75,63 @@
}
}

function getByPath(source, key) {
if (!source || typeof source !== "object" || !key) {
return undefined;
}

return String(key)
.split(".")
.reduce((current, part) => {
if (!current || typeof current !== "object" || !(part in current)) {
return undefined;
}
return current[part];
}, source);
}

function translate(key, fallback) {
const locale = context?.locale;
const messages = context?.i18n;
const locales = [locale, "zh-CN", "en-US"].filter(Boolean);
let value;
for (const candidateLocale of locales) {
value = getByPath(messages?.[candidateLocale], key);
if (value !== undefined && value !== null) {
break;
}
}
if (value === undefined || value === null) {
return fallback || "";
}
return typeof value === "string" ? value : String(value);
}

function notifyContextHandlers() {
contextHandlers.forEach((handler) => {
try {
handler(context);
} catch (error) {
console.error("AstrBotPluginPage context handler failed:", error);
}
});
}
Comment on lines +110 to +118

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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.


function applyContext(nextContext) {
if (!nextContext || typeof nextContext !== "object") {
return;
}
context = {
...(context || {}),
...nextContext,
};
if (resolveReady) {
resolveReady(context);
resolveReady = null;
}
notifyContextHandlers();
}

window.addEventListener("message", (event) => {
if (event.source !== window.parent) {
return;
Expand All @@ -87,11 +149,7 @@
}

if (message.kind === "context") {
context = message.context || null;
if (resolveReady) {
resolveReady(context);
resolveReady = null;
}
applyContext(message.context);
return;
}

Expand All @@ -104,7 +162,9 @@
if (message.ok) {
pending.resolve(message.data);
} else {
pending.reject(new Error(message.error || "Plugin bridge request failed."));
pending.reject(
new Error(message.error || "Plugin bridge request failed."),
);
}
return;
}
Expand Down Expand Up @@ -139,6 +199,30 @@
getContext() {
return context;
},
getLocale() {
return context?.locale || "zh-CN";
},
getI18n() {
return context?.i18n || {};
},
t(key, fallback) {
return translate(key, fallback);
},
onContext(handler) {
if (typeof handler !== "function") {
return () => {};
}
contextHandlers.add(handler);
if (context) {
handler(context);
}
return () => {
contextHandlers.delete(handler);
};
},
__setInitialContext(nextContext) {
applyContext(nextContext);
},
apiGet(endpoint, params) {
return makeRequest("api:get", { endpoint, params });
},
Expand Down
87 changes: 86 additions & 1 deletion astrbot/dashboard/routes/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,13 @@ async def get_plugin_page_bridge_sdk(self):
return await self._plugin_page_error_response(
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 += (
Comment on lines +192 to +196

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚨 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.

f"\n;window.AstrBotPluginPage?.__setInitialContext({context_json});\n"
)
response = cast(
QuartResponse,
await make_response(
Expand All @@ -204,6 +210,82 @@ def _get_plugin_metadata_by_name(self, plugin_name: str) -> StarMetadata | None:
return plugin
return None

@staticmethod
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
Comment on lines +214 to +222

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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.


@staticmethod
def _get_request_locale(default: str = "zh-CN") -> str:
raw_locale = request.headers.get("Accept-Language", "").strip()
locale = raw_locale.split(",", 1)[0].split(";", 1)[0].strip()
if not locale or len(locale) > 32:
return default
return locale

def _get_plugin_page_initial_context(self) -> dict | None:
asset_token = request.args.get("asset_token", "").strip()
if not asset_token:
return None
jwt_secret = self.config.get("dashboard", {}).get("jwt_secret")
if not isinstance(jwt_secret, str) or not jwt_secret.strip():
return None

try:
payload = jwt.decode(asset_token, jwt_secret, algorithms=["HS256"])
except jwt.InvalidTokenError:
return None
if payload.get("token_type") != _PLUGIN_PAGE_ASSET_TOKEN_TYPE:
return None

plugin_name = payload.get("plugin_name")
page_name = payload.get("page_name")
if not isinstance(plugin_name, str) or not isinstance(page_name, str):
return None

plugin = self._get_plugin_metadata_by_name(plugin_name)
if not plugin:
return None

locale = (
payload.get("locale")
if isinstance(payload.get("locale"), str)
else self._get_request_locale()
)
plugin_i18n = plugin.i18n or {}
try:
plugin_root = self._get_plugin_root_dir(plugin)
fresh_i18n = PluginManager._load_plugin_i18n(str(plugin_root))
if fresh_i18n:
plugin_i18n = fresh_i18n
except (OSError, ValueError):
pass

locale_data = plugin_i18n.get(locale)
display_name = (
self._get_by_path(locale_data, "metadata.display_name")
or plugin.display_name
or plugin.name
)
page_title = (
self._get_by_path(locale_data, f"pages.{page_name}.title") or page_name
)

return {
"pluginName": plugin.name,
"displayName": display_name,
"pageName": page_name,
"pageTitle": page_title,
"locale": locale,
"i18n": plugin_i18n,
}

@staticmethod
def _normalize_plugin_page_path(
raw_path: str,
Expand Down Expand Up @@ -634,6 +716,7 @@ async def _serialize_plugin_page(
page_data = {
"name": page.name,
"title": page.title,
"i18n_key": f"pages.{page.name}",
}
if include_content_path:
asset_token = (
Expand Down Expand Up @@ -675,6 +758,7 @@ def _issue_plugin_page_asset_token(
"token_type": _PLUGIN_PAGE_ASSET_TOKEN_TYPE,
"plugin_name": plugin_name,
"page_name": page_name,
"locale": self._get_request_locale(),
"iat": now,
"exp": now + timedelta(seconds=_PLUGIN_PAGE_ASSET_TOKEN_TTL_SECONDS),
}
Expand Down Expand Up @@ -1285,6 +1369,7 @@ async def get_plugin_page_components(self, plugin) -> list[dict]:
"name": page["title"],
"title": page["title"],
"page_name": page["name"],
"i18n_key": page["i18n_key"],
"description": "Plugin Page entry",
"plugin_name": plugin.name,
}
Expand Down
Loading
Loading