Skip to content

fix: 修复文件存储配置的最大 Key 长度没有生效的问题 - #345

Merged
weimch merged 1 commit into
mainfrom
fix/issue-333-file-storage-key
Sep 23, 2026
Merged

weimch merged 1 commit into
mainfrom
fix/issue-333-file-storage-key

Conversation

@raychen911

Copy link
Copy Markdown
Contributor
  • max_key_length 现在真实参与校验
  • add/get/delete 使用一致的 key 校验
  • 配置值必须大于 0
  • Session 文件名改为固定长度 SHA-256,避免 ID 越长 key 越长
  • 自动迁移当前目录的旧明文文件名及历史目录文件
  • 增加长 key、配置生效、哈希路径和迁移测试

- max_key_length 现在真实参与校验
- add/get/delete 使用一致的 key 校验
- 配置值必须大于 0
- Session 文件名改为固定长度 SHA-256,避免 ID 越长 key 越长
- 自动迁移当前目录的旧明文文件名及历史目录文件
- 增加长 key、配置生效、哈希路径和迁移测试
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

审查结论

不通过

审查范围 b80d43a..01aef5d,共 5 个非生成文件变更(2 个实现文件 + 1 个配置 + 2 个测试)。计划意图是修复 FileStorageConfig.max_key_length 配置未生效的问题,并配套将会话文件名改为 sha256 哈希、迁移旧格式文件、为 get/delete 补 key 校验。计划符合性:核心目标(让配置生效、解决长 key 文件名问题)已达成,会话文件名哈希化正确解耦了文件系统长度限制,迁移候选路径与旧格式表达式逐字一致可正确命中旧文件。主要风险:(1) 配置激活后默认 255 与文件系统 255 字节文件名上限(NAME_MAX)按“字符数 vs 字节数”失配:实测 key=251 个 ASCII 字符(文件名 256 字节)add() 必抛 ENAMETOOLONG,非 ASCII key 约 28 个 CJK 字符即触发,且无任何捕获——这是本次变更直接引入的确定性崩溃,与修复目标自相矛盾(SEVERE)。(2) get/delete 新增校验把“缺失返回 None/幂等无操作”变为抛 ValueError,而 load_session 的 _get_value 在 try/except 之外,长 workspace 路径或收紧配置下会话恢复从降级变为硬崩溃(MODERATE)。(3) _maybe_migrate 首个候选失败后不尝试第二候选,双候选并存时 legacy 数据延迟/停止迁移(LOW)。(4) 新测试用 150 字符 key 恰好避开崩溃边界、未覆盖双候选迁移,测试缺口使上述缺陷无法被 CI 拦截(LOW)。并发 TOCTOU(多进程共享 workspace 时 target 检查与 move 之间)经评估触发条件苛刻且无数据丢失,未单独报告。门禁结论:FAILED——存在高置信、可复现的运行时崩溃缺陷,建议修复 max_key_length 与文件系统字节限制的失配后再合入。

发现的问题

严重

trpc_agent_sdk/server/openclaw/storage/_aiofile_storage.py:181-188

问题: _validate_key 改用 self._max_key_length 后按字符数(len(key))校验,而 _key_to_path 生成的文件名是 quote(key, safe='') + ".json"(按字节计),校验上限 255 与文件系统 NAME_MAX=255 字节直接冲突:默认配置下 251-255 个 ASCII 字符(或约 28 个 CJK 字符,经 quote 后每个占 9 字节)的 key 能通过校验,但落盘文件名超过 255 字节,add() 在 aiofiles.open 处抛未捕获的 OSError(36, ENAMETOOLONG)。旧代码固定使用 DEFAULT_MAX_KEY_LENGTH=128,ASCII key 文件名最多 133 字节,任何 ASCII key 都不会触发此崩溃;本次将配置激活为默认 255 后,校验放行的区间内存在必然崩溃的合法 key,且 ge=1 无上限允许用户配置更大值进一步放大受损区间。

触发条件: 存储类型为 file + 默认或更大的 max_key_length + 写入无前缀普通 key(≤255 字符但编码后文件名 >255 字节)。实测:key=250 字符(文件名 255 字节)成功,key=251 字符(256 字节)抛 OSError: [Errno 36] File name too long;约 28 个中文字符即触发。

实际影响: 写入持久化永久失败且每次重试必崩;StorageManager._set_value 与 ClawSessionService.update_session 均无 try/except,异常直接冒泡到请求层导致会话保存数据丢失、请求失败;get/delete 路径同样受影响(aio_ospath.exists 内部 stat 抛同名异常)。修复目标(放开 key 长度)与底层文件系统限制自相矛盾。

修正方向: 校验应以 _key_to_path 的最终字节数为准:在 _validate_key(或 _key_to_path)中对 quote(key, safe='') + ".json" 的 UTF-8 字节长度做上限检查(如 ≤240 预留后缀余量),并在 FileStorageConfig.max_key_length 默认值改为与文件系统兼容值(如 200)或增加 le=200 约束;测试需补 251-255 字符与 CJK key 的写入用例。

中等

trpc_agent_sdk/server/openclaw/storage/_aiofile_storage.py:142-143

问题: get/delete 新增 self._validate_key(key) 破坏了“缺失即返回 None / 幂等无操作”的既有契约:超长 key 之前静默返回 None/无操作,现在直接抛 ValueError。而 StorageManager.load_session 中 _get_value(内部调用 storage.get)位于 try/except 之外(_manager.py:160),异常直接冒泡,会话恢复从“文件缺失返回 None → 创建新会话”变为硬崩溃。session:{quote(str(path))} 类 key 的长度由 workspace 路径决定(实测 200 字符 workspace 路径时 key 达 311 字符),用户显式配置较小 max_key_length(如沿用旧文档的 128)时同样命中。

触发条件: max_key_length 配置值小于会话/记忆 key 的实际长度:包括 workspace 位于长路径(CI 沙箱、容器挂载路径等,>/120 字符)或用户在 config.yaml 中显式配置 <255 的情形。

实际影响: 升级后首次 get_session/create_session 恢复会话直接抛 ValueError: AioFileStorage key too long,无法恢复历史会话;delete 对历史遗留超长 key 的幂等清理也变为抛异常。

修正方向: 对 session:/memory:/history: 前缀 key(内部构造、不受外部输入长度影响)在 get/delete 中跳过或放宽长度校验,只保留普通 key 的长度检查;或将 max_key_length 的默认上限与 session key 最长可能长度解耦(如对内部前缀 key 不做长度限制)。

较低

trpc_agent_sdk/server/openclaw/session_memory/_claw_session_service.py:215-223

问题: _maybe_migrate 新循环中,第一个候选源(unhashed 文件)存在但 shutil.move 失败(权限、文件被占用等)时,return 位于 except 之后、循环体内,直接跳过对第二个候选(legacy 目录)的尝试;旧代码只有一个候选无此语义。且 return 在 try 外统一退出,两个候选共存时每次调用最多尝试一个。

触发条件: 磁盘同时存在 sessions/<safe_key>.jsonl(unhashed)与 legacy 目录同 key 文件,且 unhashed 候选迁移失败(如文件被其它进程占用、只读挂载)。

实际影响: legacy 目录中的历史会话数据被无限期搁置(下次调用会重试第一个候选,仍失败则始终不达 legacy),升级后这些会话恢复不到;无数据丢失但迁移停止,与注释宣称的“依次迁移两个来源”不符。

修正方向: 将循环体内的 return 改为 continue(源不存在或 move 失败时继续尝试下一候选;成功迁移后 break),并补充“unhashed 失败后仍尝试 legacy”的测试。

较低

tests/server/openclaw/storage/test_aiofile_storage.py:197-201

问题: 新测试 test_add_honors_configured_max_key_length 仅用 150 字符 key 验证配置放宽路径,恰好避开 251-255 的崩溃边界,未覆盖“配置上限与文件系统 255 字节文件名限制冲突”的核心场景;test_get_validates_key/test_delete_validates_key 只测了超长抛错,未测 session: 前缀 key 与长 workspace 路径下 get 校验的回归;迁移测试未覆盖 unhashed 与 legacy 双候选并存的优先级及 move 失败后回退到第二候选。

触发条件: 无;为测试缺口。

实际影响: 上述 SEVERE 与 MODERATE 缺陷在测试全绿的情况下合入上线,回归无法被 CI 拦截。

修正方向: 补充 251-255 字符 ASCII key、CJK key、max_key_length 极值(1/200)的写-读往返用例;为 _maybe_migrate 增加双候选并存与“第一候选失败回退第二候选”的用例。

Comment on lines +181 to 188
def _validate_key(self, key: str) -> None:
if not key:
raise ValueError("AioFileStorage key cannot be empty")
if len(key) > DEFAULT_MAX_KEY_LENGTH:
raise ValueError(f"AioFileStorage key too long: {len(key)} > {DEFAULT_MAX_KEY_LENGTH}")
if len(key) > self._max_key_length:
raise ValueError(f"AioFileStorage key too long: {len(key)} > {self._max_key_length}")
if "/" in key or "\\" in key:
# Key is logical identifier, not filesystem path.
raise ValueError("AioFileStorage key must not contain path separators")

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.

问题: _validate_key 改用 self._max_key_length 后按字符数(len(key))校验,而 _key_to_path 生成的文件名是 quote(key, safe='') + ".json"(按字节计),校验上限 255 与文件系统 NAME_MAX=255 字节直接冲突:默认配置下 251-255 个 ASCII 字符(或约 28 个 CJK 字符,经 quote 后每个占 9 字节)的 key 能通过校验,但落盘文件名超过 255 字节,add() 在 aiofiles.open 处抛未捕获的 OSError(36, ENAMETOOLONG)。旧代码固定使用 DEFAULT_MAX_KEY_LENGTH=128,ASCII key 文件名最多 133 字节,任何 ASCII key 都不会触发此崩溃;本次将配置激活为默认 255 后,校验放行的区间内存在必然崩溃的合法 key,且 ge=1 无上限允许用户配置更大值进一步放大受损区间。

触发条件: 存储类型为 file + 默认或更大的 max_key_length + 写入无前缀普通 key(≤255 字符但编码后文件名 >255 字节)。实测:key=250 字符(文件名 255 字节)成功,key=251 字符(256 字节)抛 OSError: [Errno 36] File name too long;约 28 个中文字符即触发。

实际影响: 写入持久化永久失败且每次重试必崩;StorageManager._set_value 与 ClawSessionService.update_session 均无 try/except,异常直接冒泡到请求层导致会话保存数据丢失、请求失败;get/delete 路径同样受影响(aio_ospath.exists 内部 stat 抛同名异常)。修复目标(放开 key 长度)与底层文件系统限制自相矛盾。

修正方向: 校验应以 _key_to_path 的最终字节数为准:在 _validate_key(或 _key_to_path)中对 quote(key, safe='') + ".json" 的 UTF-8 字节长度做上限检查(如 ≤240 预留后缀余量),并在 FileStorageConfig.max_key_length 默认值改为与文件系统兼容值(如 200)或增加 le=200 约束;测试需补 251-255 字符与 CJK key 的写入用例。

Comment on lines 142 to +143
async def get(self, db: FileSession, key: str) -> Any:
self._validate_key(key)

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.

问题: get/delete 新增 self._validate_key(key) 破坏了“缺失即返回 None / 幂等无操作”的既有契约:超长 key 之前静默返回 None/无操作,现在直接抛 ValueError。而 StorageManager.load_session 中 _get_value(内部调用 storage.get)位于 try/except 之外(_manager.py:160),异常直接冒泡,会话恢复从“文件缺失返回 None → 创建新会话”变为硬崩溃。session:{quote(str(path))} 类 key 的长度由 workspace 路径决定(实测 200 字符 workspace 路径时 key 达 311 字符),用户显式配置较小 max_key_length(如沿用旧文档的 128)时同样命中。

触发条件: max_key_length 配置值小于会话/记忆 key 的实际长度:包括 workspace 位于长路径(CI 沙箱、容器挂载路径等,>/120 字符)或用户在 config.yaml 中显式配置 <255 的情形。

实际影响: 升级后首次 get_session/create_session 恢复会话直接抛 ValueError: AioFileStorage key too long,无法恢复历史会话;delete 对历史遗留超长 key 的幂等清理也变为抛异常。

修正方向: 对 session:/memory:/history: 前缀 key(内部构造、不受外部输入长度影响)在 get/delete 中跳过或放宽长度校验,只保留普通 key 的长度检查;或将 max_key_length 的默认上限与 session key 最长可能长度解耦(如对内部前缀 key 不做长度限制)。

Comment on lines +215 to 223
for source in candidates:
if not source.exists():
continue
try:
shutil.move(str(source), str(target))
logger.info("Migrated session %s from %s", save_key, source)
except Exception as exc: # pylint: disable=broad-except
logger.error("Failed to migrate session %s from %s: %s", save_key, source, exc)
return

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.

问题: _maybe_migrate 新循环中,第一个候选源(unhashed 文件)存在但 shutil.move 失败(权限、文件被占用等)时,return 位于 except 之后、循环体内,直接跳过对第二个候选(legacy 目录)的尝试;旧代码只有一个候选无此语义。且 return 在 try 外统一退出,两个候选共存时每次调用最多尝试一个。

触发条件: 磁盘同时存在 sessions/<safe_key>.jsonl(unhashed)与 legacy 目录同 key 文件,且 unhashed 候选迁移失败(如文件被其它进程占用、只读挂载)。

实际影响: legacy 目录中的历史会话数据被无限期搁置(下次调用会重试第一个候选,仍失败则始终不达 legacy),升级后这些会话恢复不到;无数据丢失但迁移停止,与注释宣称的“依次迁移两个来源”不符。

修正方向: 将循环体内的 return 改为 continue(源不存在或 move 失败时继续尝试下一候选;成功迁移后 break),并补充“unhashed 失败后仍尝试 legacy”的测试。

Comment on lines +197 to +201
async def test_add_honors_configured_max_key_length(self, tmp_path):
storage = AioFileStorage(FileStorageConfig(base_dir=str(tmp_path), max_key_length=255))
db = FileSession(base_dir=tmp_path)
key = "k" * 150

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.

问题: 新测试 test_add_honors_configured_max_key_length 仅用 150 字符 key 验证配置放宽路径,恰好避开 251-255 的崩溃边界,未覆盖“配置上限与文件系统 255 字节文件名限制冲突”的核心场景;test_get_validates_key/test_delete_validates_key 只测了超长抛错,未测 session: 前缀 key 与长 workspace 路径下 get 校验的回归;迁移测试未覆盖 unhashed 与 legacy 双候选并存的优先级及 move 失败后回退到第二候选。

触发条件: 无;为测试缺口。

实际影响: 上述 SEVERE 与 MODERATE 缺陷在测试全绿的情况下合入上线,回归无法被 CI 拦截。

修正方向: 补充 251-255 字符 ASCII key、CJK key、max_key_length 极值(1/200)的写-读往返用例;为 _maybe_migrate 增加双候选并存与“第一候选失败回退第二候选”的用例。

@weimch weimch left a comment

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.

Approve

@weimch
weimch merged commit 6155210 into main Sep 23, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants