feat: split config into providers.json and platforms.json #4740
Conversation
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
dosubot
bot
added
size:XL
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个安全问题、3 个其他问题，并留下了一些总体反馈：
安全问题：
- 检测到一个通用 API Key，可能会暴露对各类服务和敏感操作的访问权限。(link)
总体评论：
- provider/platform 配置的分类逻辑在
AstrBotConfig._save_categorized_config和config_migration.save_config_by_category中都有重复实现；建议将其集中到一个公共 helper 中，以减少偏差，并在未来键值变更时只需改动单一位置。 - 在
_save_categorized_config中，如果写入providers.json或platforms.json在部分成功后失败，该方法仍然会写入核心配置，并在无提示的情况下让系统处于部分更新的状态；建议添加错误处理/回滚机制，或返回状态值，以便调用方能对部分失败做出响应。 - 新增的
CORE_DEFAULT_CONFIG和PROVIDERS_DEFAULT_CONFIG/PLATFORMS_DEFAULT_CONFIG引入了多个默认值来源；为了避免默认值随时间发生偏离，可能更安全的做法是基于这些配置以编程方式派生出DEFAULT_CONFIG。
给 AI Agents 的 Prompt
请根据这次代码审查中的评论进行修改：
## 总体评论
- provider/platform 配置的分类逻辑在 `AstrBotConfig._save_categorized_config` 和 `config_migration.save_config_by_category` 中都有重复实现；建议将其集中到一个公共 helper 中，以减少偏差，并在未来键值变更时只需改动单一位置。
- 在 `_save_categorized_config` 中，如果写入 `providers.json` 或 `platforms.json` 在部分成功后失败，该方法仍然会写入核心配置，并在无提示的情况下让系统处于部分更新的状态；建议添加错误处理/回滚机制，或返回状态值，以便调用方能对部分失败做出响应。
- 新增的 `CORE_DEFAULT_CONFIG` 和 `PROVIDERS_DEFAULT_CONFIG`/`PLATFORMS_DEFAULT_CONFIG` 引入了多个默认值来源；为了避免默认值随时间发生偏离，可能更安全的做法是基于这些配置以编程方式派生出 `DEFAULT_CONFIG`。
## 单独评论
### 评论 1
<location> `astrbot/core/config/config_migration.py:85-94` </location>
<code_context>
+def migrate_config() -> tuple[bool, str]:
</code_context>
<issue_to_address>
**issue (bug_risk):** 在迁移过程中创建默认 provider/platform 文件失败时，只记录日志但不会向调用方暴露该失败。
当核心配置中没有 provider/platform 数据时，你会回退到写入 `PROVIDERS_DEFAULT_CONFIG` / `PLATFORMS_DEFAULT_CONFIG`。如果此处的 `save_json_file` 调用失败，你目前只会记录日志，却仍将迁移视为成功，这会导致部分迁移的状态，而一旦从 `cmd_config.json` 中移除了相关键，`needs_migration()` 就不会再触发。请在这些写入失败时也返回 `(False, ...)`，就像你对非空提取配置的失败处理那样。
</issue_to_address>
### 评论 2
<location> `astrbot/core/config/astrbot_config.py:84` </location>
<code_context>
self.update(conf)
+
+ def _load_merged_config(self, core_conf: dict) -> dict:
+ """从多个配置文件合并加载配置"""
+ merged = dict(core_conf)
</code_context>
<issue_to_address>
**issue (complexity):** 建议将所有配置拆分/合并以及多文件布局的逻辑委托给 `config_migration` 中的共享 helper，而不是在这个类中重新实现。
你可以通过将所有拆分/合并/文件布局逻辑委托给 `config_migration`，而不是在本地重新实现，来降低新增复杂度。
### 1. 用 `config_migration.get_merged_config` 替换 `_load_merged_config`
当前做法：
```python
# __init__
if is_default_config:
conf = self._load_merged_config(conf)
def _load_merged_config(self, core_conf: dict) -> dict:
merged = dict(core_conf)
# ... open PROVIDERS_CONFIG_PATH, PLATFORMS_CONFIG_PATH, merge, log errors ...
return merged
```
建议改为使用迁移 helper，并完全移除 `_load_merged_config`：
```python
# __init__
if is_default_config:
# 将合并行为和错误处理交给 config_migration
conf = config_migration.get_merged_config(
core_conf=conf,
core_path=ASTRBOT_CONFIG_PATH,
providers_path=PROVIDERS_CONFIG_PATH,
platforms_path=PLATFORMS_CONFIG_PATH,
)
```
如果 `get_merged_config` 目前还不支持传入路径参数，可以在那里增加可选参数，而不是在这里重复文件处理逻辑。
### 2. 用 `config_migration.save_config_by_category` 替换 `_save_categorized_config`
当前做法：
```python
def save_config(self, replace_config: dict | None = None):
if replace_config:
self.update(replace_config)
is_default_config = self.config_path == ASTRBOT_CONFIG_PATH
if is_default_config:
self._save_categorized_config()
else:
with open(self.config_path, "w", encoding="utf-8-sig") as f:
json.dump(self, f, indent=2, ensure_ascii=False)
def _save_categorized_config(self):
providers_config = {}
for key in PROVIDERS_CONFIG_KEYS:
if key in self:
providers_config[key] = self[key]
platforms_config = {}
for key in PLATFORMS_CONFIG_KEYS:
if key in self:
platforms_config[key] = self[key]
core_config = {}
excluded_keys = set(PROVIDERS_CONFIG_KEYS + PLATFORMS_CONFIG_KEYS)
for key, value in self.items():
if key not in excluded_keys:
core_config[key] = value
# write providers/platforms/core files ...
```
建议委托给已经具备分类和写入能力的迁移模块：
```python
def save_config(self, replace_config: dict | None = None):
if replace_config:
self.update(replace_config)
is_default_config = self.config_path == ASTRBOT_CONFIG_PATH
if is_default_config:
# 将拆分与文件布局逻辑集中在 config_migration 中
config_migration.save_config_by_category(
full_config=dict(self),
core_path=ASTRBOT_CONFIG_PATH,
providers_path=PROVIDERS_CONFIG_PATH,
platforms_path=PLATFORMS_CONFIG_PATH,
)
else:
with open(self.config_path, "w", encoding="utf-8-sig") as f:
json.dump(self, f, indent=2, ensure_ascii=False)
```
这样你就可以移除 `_save_categorized_config`，并将 `PROVIDERS_CONFIG_KEYS` / `PLATFORMS_CONFIG_KEYS` 的定义集中到一个地方（理想情况下是 `config_migration` 或一个共享模块），从而让**策略 + 行为**绑定在一起。
### 3. 让此类专注于内存中的配置
完成上述修改后，`AstrBotConfig` 只需要：
- 在需要时执行迁移（`needs_migration` / `migrate_config`）。
- 通过 `config_migration.get_merged_config` 加载配置。
- 通过 `config_migration.save_config_by_category` 或简单的单文件写入来保存配置。
这样可以保留所有新的多文件行为，同时避免出现两个配置拆分/合并的实现而产生偏差。
</issue_to_address>
### 评论 3
<location> `astrbot/core/config/config_migration.py:138` </location>
<code_context>
+ return True, "配置迁移成功"
+
+
+def get_merged_config() -> dict:
+ """获取合并后的完整配置（用于向后兼容）
+
</code_context>
<issue_to_address>
**issue (complexity):** 建议将该模块限定为只处理迁移逻辑，并把共享的配置拆分/合并 helper 和编排 API 集中到一个由 AstrBotConfig 使用的工具中。
你可以通过将此模块的职责严格限定为迁移，并将通用的拆分/合并 helper 移动到一个集中使用于 `AstrBotConfig` 的位置，来减少接口表面积和重复。
### 1. 从此模块中移除未使用/通用用途的 API
如果 `get_merged_config` / `save_config_by_category` 目前尚未被使用，建议从这个专注于迁移的模块中移除它们，以避免出现第二套“配置编排” API：
```python
# 暂时从此模块中移除：
def get_merged_config() -> dict:
...
def save_config_by_category(full_config: dict) -> bool:
...
```
如果未来需要它们，可以在负责运行时配置行为的同一个模块中重新引入（很可能是 `astrbot_config.py`），这样就只会有一种合并/拆分配置的方式。
### 2. 将拆分/合并 helper 移动到一个聚焦的共享工具中
目前 `extract_config_keys` / `remove_config_keys` 实现的概念操作与 `astrbot_config.py` 中的逻辑是相同的。为避免双重实现，可以抽取一个精简的共享 helper 模块，并在两处共用：
```python
# astrbot/core/utils/config_split_merge.py
from typing import Iterable
def extract_config_keys(source: dict, keys: Iterable[str]) -> dict:
return {k: source[k] for k in keys if k in source}
def remove_config_keys(source: dict, keys: Iterable[str]) -> dict:
keys_set = set(keys)
return {k: v for k, v in source.items() if k not in keys_set}
```
然后在迁移模块中：
```python
from astrbot.core.utils.config_split_merge import (
extract_config_keys,
remove_config_keys,
)
```
在 `astrbot_config.py` 中，用同样的 helper 替换本地的拆分/合并逻辑，例如：
```python
from astrbot.core.utils.config_split_merge import (
extract_config_keys,
remove_config_keys,
)
def split_config(full_config: dict) -> tuple[dict, dict, dict]:
providers = extract_config_keys(full_config, PROVIDERS_CONFIG_KEYS)
platforms = extract_config_keys(full_config, PLATFORMS_CONFIG_KEYS)
core = remove_config_keys(full_config, PROVIDERS_CONFIG_KEYS + PLATFORMS_CONFIG_KEYS)
return core, providers, platforms
```
这样既能保持迁移流程不变（`needs_migration` / `migrate_config`），又能消除并行的 API 和重复的拆分/合并行为实现。
</issue_to_address>
### 评论 4
<location> `astrbot/core/config/default.py:82` </location>
<code_context>
77b90590a8945a7d36c963981a307dc9
</code_context>
<issue_to_address>
**security (generic-api-key):** 检测到一个通用 API Key，可能会暴露对各类服务和敏感操作的访问权限。
*来源: gitleaks*
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的审查。
Original comment in English
Hey - I've found 1 security issue, 3 other issues, and left some high level feedback:
Security issues:
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
General comments:
- The categorization logic for provider/platform configs is duplicated between
AstrBotConfig._save_categorized_configandconfig_migration.save_config_by_category; consider centralizing this in one helper to reduce drift and keep future key changes in a single place. - In
_save_categorized_config, if writingproviders.jsonorplatforms.jsonfails after partially succeeding, the method still writes the core config and silently leaves the system in a partially-updated state; consider adding error handling/rollback or returning a status so callers can react to partial failures. - The new
CORE_DEFAULT_CONFIGandPROVIDERS_DEFAULT_CONFIG/PLATFORMS_DEFAULT_CONFIGintroduce multiple sources of default values; it may be safer to deriveDEFAULT_CONFIGprogrammatically from these to avoid the defaults diverging over time.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The categorization logic for provider/platform configs is duplicated between `AstrBotConfig._save_categorized_config` and `config_migration.save_config_by_category`; consider centralizing this in one helper to reduce drift and keep future key changes in a single place.
- In `_save_categorized_config`, if writing `providers.json` or `platforms.json` fails after partially succeeding, the method still writes the core config and silently leaves the system in a partially-updated state; consider adding error handling/rollback or returning a status so callers can react to partial failures.
- The new `CORE_DEFAULT_CONFIG` and `PROVIDERS_DEFAULT_CONFIG`/`PLATFORMS_DEFAULT_CONFIG` introduce multiple sources of default values; it may be safer to derive `DEFAULT_CONFIG` programmatically from these to avoid the defaults diverging over time.
## Individual Comments
### Comment 1
<location> `astrbot/core/config/config_migration.py:85-94` </location>
<code_context>
+def migrate_config() -> tuple[bool, str]:
</code_context>
<issue_to_address>
**issue (bug_risk):** Failure to create default provider/platform files during migration is logged but not surfaced to the caller.
When there is no provider/platform data in the core config, you fall back to writing `PROVIDERS_DEFAULT_CONFIG` / `PLATFORMS_DEFAULT_CONFIG`. If `save_json_file` fails here, you only log and still treat migration as successful, which can leave a partially migrated state while `needs_migration()` stops triggering once keys are removed from `cmd_config.json`. Please return `(False, ...)` on these write failures as you do for non-empty extracted configs.
</issue_to_address>
### Comment 2
<location> `astrbot/core/config/astrbot_config.py:84` </location>
<code_context>
self.update(conf)
+
+ def _load_merged_config(self, core_conf: dict) -> dict:
+ """从多个配置文件合并加载配置"""
+ merged = dict(core_conf)
</code_context>
<issue_to_address>
**issue (complexity):** Consider delegating all config split/merge and multi-file layout logic to shared helpers in `config_migration` instead of reimplementing it in this class.
You can reduce the new complexity by delegating all split/merge/file-layout logic to `config_migration` instead of re‑implementing it locally.
### 1. Replace `_load_merged_config` with `config_migration.get_merged_config`
Instead of:
```python
# __init__
if is_default_config:
conf = self._load_merged_config(conf)
def _load_merged_config(self, core_conf: dict) -> dict:
merged = dict(core_conf)
# ... open PROVIDERS_CONFIG_PATH, PLATFORMS_CONFIG_PATH, merge, log errors ...
return merged
```
Use the migration helper and drop `_load_merged_config` entirely:
```python
# __init__
if is_default_config:
# Let config_migration own merge behavior & error handling
conf = config_migration.get_merged_config(
core_conf=conf,
core_path=ASTRBOT_CONFIG_PATH,
providers_path=PROVIDERS_CONFIG_PATH,
platforms_path=PLATFORMS_CONFIG_PATH,
)
```
If `get_merged_config` doesn’t yet accept paths, you can add optional parameters there instead of duplicating file logic here.
### 2. Replace `_save_categorized_config` with `config_migration.save_config_by_category`
Instead of:
```python
def save_config(self, replace_config: dict | None = None):
if replace_config:
self.update(replace_config)
is_default_config = self.config_path == ASTRBOT_CONFIG_PATH
if is_default_config:
self._save_categorized_config()
else:
with open(self.config_path, "w", encoding="utf-8-sig") as f:
json.dump(self, f, indent=2, ensure_ascii=False)
def _save_categorized_config(self):
providers_config = {}
for key in PROVIDERS_CONFIG_KEYS:
if key in self:
providers_config[key] = self[key]
platforms_config = {}
for key in PLATFORMS_CONFIG_KEYS:
if key in self:
platforms_config[key] = self[key]
core_config = {}
excluded_keys = set(PROVIDERS_CONFIG_KEYS + PLATFORMS_CONFIG_KEYS)
for key, value in self.items():
if key not in excluded_keys:
core_config[key] = value
# write providers/platforms/core files ...
```
Delegate to the migration module, which already knows how to categorize and write:
```python
def save_config(self, replace_config: dict | None = None):
if replace_config:
self.update(replace_config)
is_default_config = self.config_path == ASTRBOT_CONFIG_PATH
if is_default_config:
# Centralize split & file layout logic in config_migration
config_migration.save_config_by_category(
full_config=dict(self),
core_path=ASTRBOT_CONFIG_PATH,
providers_path=PROVIDERS_CONFIG_PATH,
platforms_path=PLATFORMS_CONFIG_PATH,
)
else:
with open(self.config_path, "w", encoding="utf-8-sig") as f:
json.dump(self, f, indent=2, ensure_ascii=False)
```
Then you can remove `_save_categorized_config` and keep `PROVIDERS_CONFIG_KEYS` / `PLATFORMS_CONFIG_KEYS` definition in a single place (ideally `config_migration` or a shared module), so **policy + behavior** live together.
### 3. Keep this class focused on in‑memory config
After these changes, `AstrBotConfig` only needs to:
- Run migration when needed (`needs_migration` / `migrate_config`).
- Load via `config_migration.get_merged_config`.
- Save via `config_migration.save_config_by_category` or a simple single‑file write.
This preserves all new multi‑file behavior while avoiding two divergent implementations of splitting/merging configs.
</issue_to_address>
### Comment 3
<location> `astrbot/core/config/config_migration.py:138` </location>
<code_context>
+ return True, "配置迁移成功"
+
+
+def get_merged_config() -> dict:
+ """获取合并后的完整配置（用于向后兼容）
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider limiting this module to migration-only logic and centralizing shared config split/merge helpers and orchestration APIs in a single utility used by AstrBotConfig.
You can reduce surface area and duplication by scoping this module strictly to migration and moving the general split/merge helpers into a single place used by `AstrBotConfig`.
### 1. Drop unused/general-purpose APIs from this module
If `get_merged_config` / `save_config_by_category` are not used yet, remove them from this migration-focused module to avoid a second “config orchestration” API:
```python
# Remove these from this module for now:
def get_merged_config() -> dict:
...
def save_config_by_category(full_config: dict) -> bool:
...
```
If you need them later, reintroduce them in the same module that owns runtime config behavior (likely `astrbot_config.py`) so there is only one way to merge/split config.
### 2. Move split/merge helpers to a shared, focused utility
Right now `extract_config_keys` / `remove_config_keys` implement the same conceptual operation that `astrbot_config.py` also performs. To avoid two implementations, extract a tiny shared helper module and use it from both:
```python
# astrbot/core/utils/config_split_merge.py
from typing import Iterable
def extract_config_keys(source: dict, keys: Iterable[str]) -> dict:
return {k: source[k] for k in keys if k in source}
def remove_config_keys(source: dict, keys: Iterable[str]) -> dict:
keys_set = set(keys)
return {k: v for k, v in source.items() if k not in keys_set}
```
Then in your migration module:
```python
from astrbot.core.utils.config_split_merge import (
extract_config_keys,
remove_config_keys,
)
```
And in `astrbot_config.py`, replace its local merge/split logic with calls to the same helpers, e.g.:
```python
from astrbot.core.utils.config_split_merge import (
extract_config_keys,
remove_config_keys,
)
def split_config(full_config: dict) -> tuple[dict, dict, dict]:
providers = extract_config_keys(full_config, PROVIDERS_CONFIG_KEYS)
platforms = extract_config_keys(full_config, PLATFORMS_CONFIG_KEYS)
core = remove_config_keys(full_config, PROVIDERS_CONFIG_KEYS + PLATFORMS_CONFIG_KEYS)
return core, providers, platforms
```
This keeps the migration flow intact (`needs_migration` / `migrate_config`) while eliminating parallel APIs and duplicated split/merge behavior.
</issue_to_address>
### Comment 4
<location> `astrbot/core/config/default.py:82` </location>
<code_context>
77b90590a8945a7d36c963981a307dc9
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return True, "配置迁移成功" | ||
|
|
||
|
|
||
| def get_merged_config() -> dict: |
Contributor
There was a problem hiding this comment.
issue (complexity): 建议将该模块限定为只处理迁移逻辑，并把共享的配置拆分/合并 helper 和编排 API 集中到一个由 AstrBotConfig 使用的工具中。
你可以通过将此模块的职责严格限定为迁移，并将通用的拆分/合并 helper 移动到一个集中使用于 AstrBotConfig 的位置，来减少接口表面积和重复。
1. 从此模块中移除未使用/通用用途的 API
如果 get_merged_config / save_config_by_category 目前尚未被使用，建议从这个专注于迁移的模块中移除它们，以避免出现第二套“配置编排” API：
# 暂时从此模块中移除：
def get_merged_config() -> dict:
...
def save_config_by_category(full_config: dict) -> bool:
...如果未来需要它们，可以在负责运行时配置行为的同一个模块中重新引入（很可能是 astrbot_config.py），这样就只会有一种合并/拆分配置的方式。
2. 将拆分/合并 helper 移动到一个聚焦的共享工具中
目前 extract_config_keys / remove_config_keys 实现的概念操作与 astrbot_config.py 中的逻辑是相同的。为避免双重实现，可以抽取一个精简的共享 helper 模块，并在两处共用：
# astrbot/core/utils/config_split_merge.py
from typing import Iterable
def extract_config_keys(source: dict, keys: Iterable[str]) -> dict:
return {k: source[k] for k in keys if k in source}
def remove_config_keys(source: dict, keys: Iterable[str]) -> dict:
keys_set = set(keys)
return {k: v for k, v in source.items() if k not in keys_set}然后在迁移模块中：
from astrbot.core.utils.config_split_merge import (
extract_config_keys,
remove_config_keys,
)在 astrbot_config.py 中，用同样的 helper 替换本地的拆分/合并逻辑，例如：
from astrbot.core.utils.config_split_merge import (
extract_config_keys,
remove_config_keys,
)
def split_config(full_config: dict) -> tuple[dict, dict, dict]:
providers = extract_config_keys(full_config, PROVIDERS_CONFIG_KEYS)
platforms = extract_config_keys(full_config, PLATFORMS_CONFIG_KEYS)
core = remove_config_keys(full_config, PROVIDERS_CONFIG_KEYS + PLATFORMS_CONFIG_KEYS)
return core, providers, platforms这样既能保持迁移流程不变（needs_migration / migrate_config），又能消除并行的 API 和重复的拆分/合并行为实现。
Original comment in English
issue (complexity): Consider limiting this module to migration-only logic and centralizing shared config split/merge helpers and orchestration APIs in a single utility used by AstrBotConfig.
You can reduce surface area and duplication by scoping this module strictly to migration and moving the general split/merge helpers into a single place used by AstrBotConfig.
1. Drop unused/general-purpose APIs from this module
If get_merged_config / save_config_by_category are not used yet, remove them from this migration-focused module to avoid a second “config orchestration” API:
# Remove these from this module for now:
def get_merged_config() -> dict:
...
def save_config_by_category(full_config: dict) -> bool:
...If you need them later, reintroduce them in the same module that owns runtime config behavior (likely astrbot_config.py) so there is only one way to merge/split config.
2. Move split/merge helpers to a shared, focused utility
Right now extract_config_keys / remove_config_keys implement the same conceptual operation that astrbot_config.py also performs. To avoid two implementations, extract a tiny shared helper module and use it from both:
# astrbot/core/utils/config_split_merge.py
from typing import Iterable
def extract_config_keys(source: dict, keys: Iterable[str]) -> dict:
return {k: source[k] for k in keys if k in source}
def remove_config_keys(source: dict, keys: Iterable[str]) -> dict:
keys_set = set(keys)
return {k: v for k, v in source.items() if k not in keys_set}Then in your migration module:
from astrbot.core.utils.config_split_merge import (
extract_config_keys,
remove_config_keys,
)And in astrbot_config.py, replace its local merge/split logic with calls to the same helpers, e.g.:
from astrbot.core.utils.config_split_merge import (
extract_config_keys,
remove_config_keys,
)
def split_config(full_config: dict) -> tuple[dict, dict, dict]:
providers = extract_config_keys(full_config, PROVIDERS_CONFIG_KEYS)
platforms = extract_config_keys(full_config, PLATFORMS_CONFIG_KEYS)
core = remove_config_keys(full_config, PROVIDERS_CONFIG_KEYS + PLATFORMS_CONFIG_KEYS)
return core, providers, platformsThis keeps the migration flow intact (needs_migration / migrate_config) while eliminating parallel APIs and duplicated split/merge behavior.
tooplick
force-pushed
the
feature/config-split
branch
2 times, most recently
from
0e8330c to
192086d
Compare
- Create providers_default.py with LLM provider config defaults - Create platforms_default.py with platform adapter config defaults - Create config_migration.py for auto-migration from single file - Update AstrBotConfig to load/save from multiple config files - Update backup exporter/importer to handle new config files
tooplick
force-pushed
the
feature/config-split
branch
from
192086d to
c667b8a
Compare
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个安全问题，并给出了一些高层次的反馈：
安全问题：
- 检测到一个通用 API Key，可能会暴露对各种服务和敏感操作的访问权限。(link)
总体评论：
save_config_by_category中的多文件保存路径不是原子性的——如果保存 providers/platforms 成功，但更新cmd_config.json失败，就可能导致迁移/更新只完成了一部分；建议先写入临时文件再重命名，或者加入回滚策略，以保持配置的一致性。importer.py和exporter.py各自独立构造了providers.json和platforms.json的路径；你可能需要将这些路径常量集中管理（例如复用config_migration.py中的常量），以避免将来文件位置变化时出现偏差。
AI 代理提示词
Please address the comments from this code review:
## Overall Comments
- `save_config_by_category` 中的多文件保存路径不是原子性的——如果保存 providers/platforms 成功，但更新 `cmd_config.json` 失败，就可能导致迁移/更新只完成了一部分；建议先写入临时文件再重命名，或者加入回滚策略，以保持配置的一致性。
- `importer.py` 和 `exporter.py` 各自独立构造了 `providers.json` 和 `platforms.json` 的路径；你可能需要将这些路径常量集中管理（例如复用 `config_migration.py` 中的常量），以避免将来文件位置变化时出现偏差。
## Individual Comments
### Comment 1
<location> `astrbot/core/config/default.py:82` </location>
<code_context>
77b90590a8945a7d36c963981a307dc9
</code_context>
<issue_to_address>
**security (generic-api-key):** 检测到一个通用 API Key，可能会暴露对各种服务和敏感操作的访问权限。
*Source: gitleaks*
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 security issue, and left some high level feedback:
Security issues:
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
General comments:
- The multi-file save path in
save_config_by_categoryis not atomic—if saving providers/platforms succeeds but updatingcmd_config.jsonfails you can end up in a partially migrated/updated state; consider writing to temporary files and renaming or adding a rollback strategy to keep the configuration consistent. importer.pyandexporter.pyreconstructproviders.jsonandplatforms.jsonpaths independently; you might want to centralize these path constants (e.g., reuse those fromconfig_migration.py) to avoid future drift if file locations change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The multi-file save path in `save_config_by_category` is not atomic—if saving providers/platforms succeeds but updating `cmd_config.json` fails you can end up in a partially migrated/updated state; consider writing to temporary files and renaming or adding a rollback strategy to keep the configuration consistent.
- `importer.py` and `exporter.py` reconstruct `providers.json` and `platforms.json` paths independently; you might want to centralize these path constants (e.g., reuse those from `config_migration.py`) to avoid future drift if file locations change.
## Individual Comments
### Comment 1
<location> `astrbot/core/config/default.py:82` </location>
<code_context>
77b90590a8945a7d36c963981a307dc9
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</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.
Hey - 我发现了 1 个安全问题、1 个其他问题，并留下了一些整体性的反馈：
Security issues:
- 检测到一个通用 API 密钥（Generic API Key），可能会暴露对各种服务和敏感操作的访问权限。(link)
General comments:
- 在
config_migration.save_config_by_category中，建议将写入操作改为原子写（例如先写入临时文件再重命名，或者把对cmd_config.json的更新放在最后），以避免在某一次写入失败时导致配置处于部分更新的状态。 migrate_config中的迁移流程会就地修改cmd_config.json，但不会先创建备份；考虑到该逻辑会在启动时自动对默认配置运行，建议在写入变更前先将原始文件复制为.bak备份，以更安全的方式处理。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `config_migration.save_config_by_category`, consider making the writes atomic (e.g., write to temp files and rename, or update `cmd_config.json` last) to avoid leaving configs in a partially-updated state if one of the writes fails.
- The migration flow in `migrate_config` modifies `cmd_config.json` in place without creating a backup; given this runs automatically on startup for default configs, it would be safer to copy the original file to a `.bak` before writing changes.
## Individual Comments
### Comment 1
<location> `astrbot/core/backup/importer.py:378-381` </location>
<code_context>
+ config_count = 0
+ data_path = get_astrbot_data_path()
+
+ # 导入核心配置文件
if "config/cmd_config.json" in zf.namelist():
try:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 建议同时备份现有核心配置，以便与 providers/platforms 的处理方式保持一致。
`providers.json` 和 `platforms.json` 在被覆盖前都会备份为 `*.bak`，但 `cmd_config.json` 是直接就地覆盖写入的。这样会让核心配置比 provider/platform 配置更难恢复。除非有强理由必须区别对待，否则请在写入导入内容之前，同样备份 `self.config_path`。
建议实现方式：
```python
# 导入核心配置文件
if "config/cmd_config.json" in zf.namelist():
try:
config_content = zf.read("config/cmd_config.json")
# 在覆盖写入前备份现有核心配置文件（与 providers/platforms 备份策略保持一致）
try:
if os.path.exists(self.config_path):
backup_path = f"{self.config_path}.bak"
shutil.copy2(self.config_path, backup_path)
except Exception as backup_exc:
# 备份失败不阻止导入，只记录警告
result.add_warning(f"备份核心配置文件失败，将继续覆盖写入: {backup_exc}")
with open(self.config_path, "wb") as f:
f.write(config_content)
config_count += 1
except Exception as e:
result.add_warning(f"导入核心配置文件失败: {e}")
```
此修改假定 `astrbot/core/backup/importer.py` 文件开头已经导入了 `os` 和 `shutil`（如果现有的 provider/platform 备份逻辑实现类似的话，这很可能已经存在）。如果尚未导入，请在本文件的其它 import 语句附近添加：
```python
import os
import shutil
```
</issue_to_address>
### Comment 2
<location> `astrbot/core/config/default.py:82` </location>
<code_context>
77b90590a8945a7d36c963981a307dc9
</code_context>
<issue_to_address>
**security (generic-api-key):** 检测到一个通用 API 密钥（Generic API Key），可能会暴露对各种服务和敏感操作的访问权限。
*Source: gitleaks*
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进之后的评审。
Original comment in English
Hey - I've found 1 security issue, 1 other issue, and left some high level feedback:
Security issues:
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
General comments:
- In
config_migration.save_config_by_category, consider making the writes atomic (e.g., write to temp files and rename, or updatecmd_config.jsonlast) to avoid leaving configs in a partially-updated state if one of the writes fails. - The migration flow in
migrate_configmodifiescmd_config.jsonin place without creating a backup; given this runs automatically on startup for default configs, it would be safer to copy the original file to a.bakbefore writing changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `config_migration.save_config_by_category`, consider making the writes atomic (e.g., write to temp files and rename, or update `cmd_config.json` last) to avoid leaving configs in a partially-updated state if one of the writes fails.
- The migration flow in `migrate_config` modifies `cmd_config.json` in place without creating a backup; given this runs automatically on startup for default configs, it would be safer to copy the original file to a `.bak` before writing changes.
## Individual Comments
### Comment 1
<location> `astrbot/core/backup/importer.py:378-381` </location>
<code_context>
+ config_count = 0
+ data_path = get_astrbot_data_path()
+
+ # 导入核心配置文件
if "config/cmd_config.json" in zf.namelist():
try:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider backing up the existing core config as well for consistency with providers/platforms.
`providers.json` and `platforms.json` are backed up as `*.bak` before overwrite, but `cmd_config.json` is overwritten in-place. This makes core config harder to recover than provider/platform configs. Unless there’s a strong reason to treat it differently, please also back up `self.config_path` before writing the imported content.
Suggested implementation:
```python
# 导入核心配置文件
if "config/cmd_config.json" in zf.namelist():
try:
config_content = zf.read("config/cmd_config.json")
# 在覆盖写入前备份现有核心配置文件（与 providers/platforms 备份策略保持一致）
try:
if os.path.exists(self.config_path):
backup_path = f"{self.config_path}.bak"
shutil.copy2(self.config_path, backup_path)
except Exception as backup_exc:
# 备份失败不阻止导入，只记录警告
result.add_warning(f"备份核心配置文件失败，将继续覆盖写入: {backup_exc}")
with open(self.config_path, "wb") as f:
f.write(config_content)
config_count += 1
except Exception as e:
result.add_warning(f"导入核心配置文件失败: {e}")
```
This change assumes that `os` and `shutil` are already imported at the top of `astrbot/core/backup/importer.py`, which is likely if existing provider/platform backups are implemented similarly. If they are not yet imported, add:
```python
import os
import shutil
```
near the other imports in this file.
</issue_to_address>
### Comment 2
<location> `astrbot/core/config/default.py:82` </location>
<code_context>
77b90590a8945a7d36c963981a307dc9
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+378
to
381
| # 导入核心配置文件 | ||
| if "config/cmd_config.json" in zf.namelist(): | ||
| try: | ||
| config_content = zf.read("config/cmd_config.json") |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): 建议同时备份现有核心配置，以便与 providers/platforms 的处理方式保持一致。
providers.json 和 platforms.json 在被覆盖前都会备份为 *.bak，但 cmd_config.json 是直接就地覆盖写入的。这样会让核心配置比 provider/platform 配置更难恢复。除非有强理由必须区别对待，否则请在写入导入内容之前，同样备份 self.config_path。
建议实现方式：
# 导入核心配置文件
if "config/cmd_config.json" in zf.namelist():
try:
config_content = zf.read("config/cmd_config.json")
# 在覆盖写入前备份现有核心配置文件（与 providers/platforms 备份策略保持一致）
try:
if os.path.exists(self.config_path):
backup_path = f"{self.config_path}.bak"
shutil.copy2(self.config_path, backup_path)
except Exception as backup_exc:
# 备份失败不阻止导入，只记录警告
result.add_warning(f"备份核心配置文件失败，将继续覆盖写入: {backup_exc}")
with open(self.config_path, "wb") as f:
f.write(config_content)
config_count += 1
except Exception as e:
result.add_warning(f"导入核心配置文件失败: {e}")此修改假定 astrbot/core/backup/importer.py 文件开头已经导入了 os 和 shutil（如果现有的 provider/platform 备份逻辑实现类似的话，这很可能已经存在）。如果尚未导入，请在本文件的其它 import 语句附近添加：
import os
import shutilOriginal comment in English
suggestion (bug_risk): Consider backing up the existing core config as well for consistency with providers/platforms.
providers.json and platforms.json are backed up as *.bak before overwrite, but cmd_config.json is overwritten in-place. This makes core config harder to recover than provider/platform configs. Unless there’s a strong reason to treat it differently, please also back up self.config_path before writing the imported content.
Suggested implementation:
# 导入核心配置文件
if "config/cmd_config.json" in zf.namelist():
try:
config_content = zf.read("config/cmd_config.json")
# 在覆盖写入前备份现有核心配置文件（与 providers/platforms 备份策略保持一致）
try:
if os.path.exists(self.config_path):
backup_path = f"{self.config_path}.bak"
shutil.copy2(self.config_path, backup_path)
except Exception as backup_exc:
# 备份失败不阻止导入，只记录警告
result.add_warning(f"备份核心配置文件失败，将继续覆盖写入: {backup_exc}")
with open(self.config_path, "wb") as f:
f.write(config_content)
config_count += 1
except Exception as e:
result.add_warning(f"导入核心配置文件失败: {e}")This change assumes that os and shutil are already imported at the top of astrbot/core/backup/importer.py, which is likely if existing provider/platform backups are implemented similarly. If they are not yet imported, add:
import os
import shutilnear the other imports in this file.
| "dashboard": { | ||
| "enable": True, | ||
| "username": "astrbot", | ||
| "password": "77b90590a8945a7d36c963981a307dc9", |
Contributor
There was a problem hiding this comment.
security (generic-api-key): 检测到一个通用 API 密钥（Generic API Key），可能会暴露对各种服务和敏感操作的访问权限。
Source: gitleaks
Original comment in English
security (generic-api-key): Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
Source: gitleaks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
feat: split config into providers.json and platforms.json
根据 issue #4548 将单一的
cmd_config.json配置文件拆分为模块化配置（cmd_config.json、providers.json、platforms.json），提高配置管理的可维护性。支持自动迁移，透明兼容现有功能。Modifications / 改动点
新增文件：
astrbot/core/config/providers_default.py- LLM 供应商配置默认值astrbot/core/config/platforms_default.py- 平台适配器配置默认值astrbot/core/config/config_migration.py- 自动迁移工具修改文件：
astrbot/core/config/astrbot_config.py- 多文件加载/保存astrbot/core/config/default.py- 核心配置定义astrbot/core/backup/exporter.py- 导出新配置文件astrbot/core/backup/importer.py- 恢复新配置文件This is NOT a breaking change. / 这不是一个破坏性变更。
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
在保持向后兼容的同时，将配置拆分为核心（core）、提供方（provider）和平台（platform）配置文件，并支持从旧版单文件配置自动迁移。
新功能：
providers.json和platforms.json中。providers.json和platforms.json与核心配置文件一并包含在内。增强：
Original summary in English
Summary by Sourcery
Split configuration into core, provider, and platform files while preserving backward compatibility and supporting automatic migration from the legacy single-file config.
New Features:
Enhancements:
新功能：
providers.json和platforms.json中。providers.json和platforms.json与核心配置文件一起包含在内。增强改进：
Original summary in English
Summary by Sourcery
在保持向后兼容的同时，将配置拆分为核心（core）、提供方（provider）和平台（platform）配置文件，并支持从旧版单文件配置自动迁移。
新功能：
providers.json和platforms.json中。providers.json和platforms.json与核心配置文件一并包含在内。增强：
Original summary in English
Summary by Sourcery
Split configuration into core, provider, and platform files while preserving backward compatibility and supporting automatic migration from the legacy single-file config.
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
在保持向后兼容的同时，将配置拆分为核心（core）、提供方（provider）和平台（platform）配置文件，并支持从旧版单文件配置自动迁移。
新功能：
providers.json和platforms.json中。providers.json和platforms.json与核心配置文件一并包含在内。增强：
Original summary in English
Summary by Sourcery
Split configuration into core, provider, and platform files while preserving backward compatibility and supporting automatic migration from the legacy single-file config.
New Features:
Enhancements:
新功能：
providers.json和platforms.json中。providers.json和platforms.json与核心配置文件一起包含在内。增强改进：
Original summary in English
Summary by Sourcery
在保持向后兼容的同时，将配置拆分为核心（core）、提供方（provider）和平台（platform）配置文件，并支持从旧版单文件配置自动迁移。
新功能：
providers.json和platforms.json中。providers.json和platforms.json与核心配置文件一并包含在内。增强：
Original summary in English
Summary by Sourcery
Split configuration into core, provider, and platform files while preserving backward compatibility and supporting automatic migration from the legacy single-file config.
New Features:
Enhancements: