Files
dstalk/agents/audits/W11.2-config-audit.md
XiuChengWu bb2e8c0220
Some checks failed
CI / Determine matrix (push) Has been cancelled
CI / ${{ matrix.os }} / ${{ matrix.build_type }} (push) Has been cancelled
Wave 8: tech-debt audits, core unit tests, CLI pipe input (W11.1-W11.7)
- W11.1 context_plugin audit (architect-huang): 3 findings on ABI exception
  safety, strdup null checks, dead g_max_tokens variable. Rating: B.
- W11.2 config audit (engineer-chen): identified 74-line TOML parser
  duplication between config_plugin and config_store, dual-store data
  isolation, dangling c_str() risk. Rating: C.
- W11.3 event_bus + service_registry unit tests (qa-liu): 12 cases total,
  ctest coverage 2 -> 4 targets, 100% pass.
- W11.4 CLI stdin pipe mode (engineer-zhao): isatty detection, single-shot
  inference path with exit codes 0/1/2/3.
- W11.6 scripts/refresh_status.py (engineer-li): 431-line generator that
  scans 16 profile.md + 5 group.md to regenerate STATUS.md.
- W11.7 destructive testing (qa-xu): 10 input scenarios PASS, found bin
  copy mismatch (BUG-1) plus 3 minor UX bugs for follow-up.

Verified: cmake build 0 error, ctest 4/4 pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2026-05-27 09:06:25 +08:00

7.0 KiB
Raw Permalink Blame History

W11.2 Config Plugin / ConfigStore 职责与跨 DLL 堆审计

审计人: 陈风 (engineer-chen)
日期: 2026-05-27
审计范围: plugins/config/src/config_plugin.cpp (146行) + dstalk-core/src/config_store.cpp (83行)


1. 职责划分

文件 职责 所在层
config_store.cpp (dstalk::ConfigStore) Host 自身的键值配置存储。dstalk_init 加载初始配置,通过 host->config_get/set 供所有插件调用 core
config_plugin.cpp (匿名 ConfigStore) 注册 "config" 服务 (vtable dstalk_config_service_t),提供 load_file 运行时调用接口,供其他插件通过 query_service 发现 plugin

结论: 职责有交集但不完全重复——plugin 提供了 core 未暴露的 load_file 运行时接口。然而,两个 ConfigStore 的 TOML 解析逻辑74行逐字符完全相同,且两个 config 存储在运行时相互独立host 的 config 与 plugin 的 config 是不同的 map存在以下问题

  • 同一 key 在两个 store 中可能具有不同值
  • host->config_get("plugin_dir") 读的是 core storequery_service("config")->get("plugin_dir") 读的是 plugin store
  • 用户不知道该用哪个接口

建议边界: core 的 ConfigStore 作为唯一真相源config plugin 不再持有自己的 store改为将 load_file / get / set 委托给 g_host->config_get / g_host->config_setload_file 则需要 core 侧新增 config_load_file 到 host API 或 config plugin 自行实现(仅保留解析逻辑,写入 host store


2. 跨 DLL 堆合规

检查项 config_store.cpp config_plugin.cpp
直接 malloc/free
直接 strdup
new/delete 跨边界 无(仅内部 STL 容器)
get() 返回 const char* 的所有权 指向内部 std::string,调用方不得释放 同左
set() 的值类型 const char* 输入,拷贝到 std::string 同左

结论: 无跨 DLL 堆违规。两个文件均完全使用 STL 容器(std::unordered_map<std::string, std::string>)管理内存,所有分配/释放均在各自 DLL 的 CRT 堆内完成。返回的 const char* 指向内部 string buffer调用方只读不释放符合 ABI 契约 2.1。

但需注意:config_plugin.cpp:77 返回的 c_str() 指向 plugin DLL 内部的 std::string。调用方若持有该指针跨越 plugin unload将导致 use-after-free。这在当前设计中是低风险config plugin 通常不会被卸载),但建议在 ABI 文档中明确说明。


3. 线程安全

文件 锁机制 评估
config_store.cpp mutable std::mutexget/set 均持锁 get/set 单独调用安全
config_plugin.cpp 同上(匿名类内 std::mutex 同左

已知问题:

  • Dangling pointer (config_store.cpp:72, config_plugin.cpp:77): get() 在锁内获取 it->second.c_str(),锁释放后返回。并发 set() 同一 key 会触发 std::string 重分配使外部持有的指针悬垂。ABI 文档 6.4 已记录此风险但代码未做防护(如返回 std::string 副本或用 host->strdup 分配)。

  • load_file 非原子 (config_store.cpp:57-60, config_plugin.cpp:63-66): load_file 逐行解析,每写入一个 key-value 就释放锁。并发 get() 可观察到新旧配置混合的状态。设计上这是合理的(避免持锁做磁盘 I/O但调用方需知晓。

  • ServiceRegistry 与 ConfigStore 之间无锁协调: plugin 注册 "config" 服务时(config_plugin.cpp:126ServiceRegistry 的写锁和 ConfigStore 的 mutex 是独立的。这在当前无问题(注册只存 vtable 指针),但如果未来 config service 的注册/卸载与 store 生命周期联动,需注意锁序。


4. 服务暴露

config_plugin.cpp:124-127:

static int on_init(const dstalk_host_api_t* host) {
    g_host = host;
    return host->register_service("config", 1, &g_service);
}
  • 正确: 在 on_init 期间注册,符合 ABI 契约 4.1(仅此时可注册)
  • 正确: vtable 为 static 全局变量生命周期覆盖插件整个加载期ABI 4.4
  • 正确: service name "config" 与 dstalk_services.h:64 定义一致
  • 注意: 未检查 register_service 返回值。如果另一个插件也注册 "config",返回 -2 会被静默忽略
  • 去重: ServiceRegistry 已实现重复注册检测(service_registry.cpp:12-14),但插件自身不感知失败

5. 三个最严重发现

发现 1: 完整 TOML 解析器代码重复 (Critical)

  • config_plugin.cpp:16-90config_store.cpp:10-83 的解析逻辑完全相同
  • 影响: 任何 bug 修复需双份维护;已发现 load_file 不处理 section 名前后空格、不支持 inline table 等问题,需在两个文件中分别修复
  • 修复方向: config plugin 消除自己的 ConfigStore改为委托 host store通过 g_host->config_get/set),仅保留 load_file 且将解析结果写入 host store

发现 2: 双配置存储导致数据孤岛 (High)

  • 位置: host.cpp:176 创建 dstalk::ConfigStorehost storeconfig_plugin.cpp:98 创建匿名 ConfigStoreplugin store
  • 影响: host->config_get("key")query_service("config")->get("key") 返回不同数据,用户困惑
  • 修复方向: 合并为唯一 store建议保留 host 侧 dstalk::ConfigStoreconfig plugin 只做服务注册包装)

发现 3: get() 返回悬垂指针 (High)

  • config_store.cpp:72 / config_plugin.cpp:77: return it->second.c_str() 在锁释放后,并发 set 同一 key 触发 realloc
  • 影响: 调用方持有的 const char* 可能指向已释放内存,导致 crash 或静默数据损坏
  • 修复方向: 选项 A — 返回值拷贝(调用方用 host->strdup 在锁内复制);选项 B — 文档明确规定调用方必须立即复制(当前 ABI spec 6.4 方向);选项 C — 用 std::string 作为返回值类型(需改 vtable 签名,涉及 API 版本 bump

6. 整体评级

评级: C

理由: 无跨 DLL 堆违规(本次审计核心关注点通过),线程安全基础正确但 get() 存在悬垂指针缺陷。最严重的问题是 74 行 TOML 解析器完全重复 + 双存储架构混乱,属于设计层面而非实现 bug但维护代价和用户困惑程度显著。建议优先解决发现 1 和 2合并 store发现 3 在合并时一并修复。


7. 附加建议

  1. config plugin 的 on_init 检查返回值 (config_plugin.cpp:126): register_service 返回 -2 时应有日志警告
  2. load_file 前清空已有数据: 当前 load_file 不先 data_.clear()新文件与旧数据混合。如果这是预期行为merge 语义),应在注释中说明
  3. config_store.hpp 缺少 load_file 文档: 未说明返回值约定0=成功, -1=文件不存在/解析失败)