From 01aef5d5b030f2f7ebcd13a9162a98e9aee6ea39 Mon Sep 17 00:00:00 2001 From: raychen <815315825@qq.com> Date: Wed, 23 Sep 2026 15:10:32 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8D=E6=96=87=E4=BB=B6?= =?UTF-8?q?=E5=AD=98=E5=82=A8=E9=85=8D=E7=BD=AE=E7=9A=84=E6=9C=80=E5=A4=A7?= =?UTF-8?q?=20Key=20=E9=95=BF=E5=BA=A6=E6=B2=A1=E6=9C=89=E7=94=9F=E6=95=88?= =?UTF-8?q?=E7=9A=84=E9=97=AE=E9=A2=98?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - max_key_length 现在真实参与校验 - add/get/delete 使用一致的 key 校验 - 配置值必须大于 0 - Session 文件名改为固定长度 SHA-256，避免 ID 越长 key 越长 - 自动迁移当前目录的旧明文文件名及历史目录文件 - 增加长 key、配置生效、哈希路径和迁移测试 --- .../test_claw_session_service.py | 34 ++++++++++-- .../openclaw/storage/test_aiofile_storage.py | 55 +++++++++++++------ .../server/openclaw/config/_config.py | 2 +- .../session_memory/_claw_session_service.py | 27 ++++++--- .../openclaw/storage/_aiofile_storage.py | 10 ++-- 5 files changed, 93 insertions(+), 35 deletions(-) diff --git a/tests/server/openclaw/session_memory/test_claw_session_service.py b/tests/server/openclaw/session_memory/test_claw_session_service.py index 9e3e3084b..ae1d92a6e 100644 --- a/tests/server/openclaw/session_memory/test_claw_session_service.py +++ b/tests/server/openclaw/session_memory/test_claw_session_service.py @@ -2,6 +2,7 @@ from __future__ import annotations +import hashlib import shutil from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch @@ -47,15 +48,26 @@ class TestGetSessionPath: def test_constructs_correct_path(self, tmp_path): svc = _make_service(tmp_path) - path = svc._get_session_path("app:user:session") + save_key = "app:user:session" + path = svc._get_session_path(save_key) assert path.parent == svc.sessions_dir assert path.suffix == ".jsonl" - assert "app_user_session" in path.stem or "app" in path.stem + assert path.stem == hashlib.sha256(save_key.encode("utf-8")).hexdigest() - def test_replaces_colons(self, tmp_path): + def test_filename_length_is_bounded(self, tmp_path): svc = _make_service(tmp_path) - path = svc._get_session_path("a:b:c") - assert ":" not in path.name + short_path = svc._get_session_path("a:b:c") + long_path = svc._get_session_path("app:" + "user" * 1000 + ":session") + + assert len(short_path.name) == len(long_path.name) == 70 + assert short_path != long_path + + def test_unhashed_path_preserves_previous_format(self, tmp_path): + svc = _make_service(tmp_path) + path = svc._get_unhashed_session_path("app:user:session") + + assert path.parent == svc.sessions_dir + assert "app_user_session" in path.stem or "app" in path.stem # --------------------------------------------------------------------------- @@ -110,6 +122,18 @@ def test_migration_success(self, tmp_path): assert target.read_text() == "legacy data" assert not legacy.exists() + def test_migrates_unhashed_current_session(self, tmp_path): + svc = _make_service(tmp_path) + save_key = "app:user:session" + target = svc._get_session_path(save_key) + unhashed = svc._get_unhashed_session_path(save_key) + unhashed.write_text("current session data") + + svc._maybe_migrate(save_key, target) + + assert target.read_text() == "current session data" + assert not unhashed.exists() + @patch("trpc_agent_sdk.server.openclaw.session_memory._claw_session_service.shutil.move", side_effect=OSError("permission denied")) def test_migration_failure_logs_error(self, mock_move, tmp_path): diff --git a/tests/server/openclaw/storage/test_aiofile_storage.py b/tests/server/openclaw/storage/test_aiofile_storage.py index e712b63be..5e20531d5 100644 --- a/tests/server/openclaw/storage/test_aiofile_storage.py +++ b/tests/server/openclaw/storage/test_aiofile_storage.py @@ -17,7 +17,6 @@ HISTORY_FILENAME, MEMORY_FILENAME, ) -from trpc_agent_sdk.storage import DEFAULT_MAX_KEY_LENGTH class TestAioFileStorageInit: @@ -42,28 +41,31 @@ def test_max_key_length(self, tmp_path): class TestValidateKey: """Tests for AioFileStorage._validate_key.""" - def test_empty_key_raises(self): + def test_empty_key_raises(self, tmp_path): + storage = _make_storage(tmp_path) with pytest.raises(ValueError, match="cannot be empty"): - AioFileStorage._validate_key("") + storage._validate_key("") - def test_too_long_key_raises(self): - long_key = "a" * (DEFAULT_MAX_KEY_LENGTH + 1) - with pytest.raises(ValueError, match="too long"): - AioFileStorage._validate_key(long_key) + def test_configured_max_key_length_is_enforced(self, tmp_path): + storage = AioFileStorage(FileStorageConfig(base_dir=str(tmp_path), max_key_length=140)) - def test_forward_slash_raises(self): - with pytest.raises(ValueError, match="path separators"): - AioFileStorage._validate_key("a/b") + storage._validate_key("a" * 140) + with pytest.raises(ValueError, match="141 > 140"): + storage._validate_key("a" * 141) - def test_backslash_raises(self): + def test_forward_slash_raises(self, tmp_path): + storage = _make_storage(tmp_path) with pytest.raises(ValueError, match="path separators"): - AioFileStorage._validate_key("a\\b") + storage._validate_key("a/b") - def test_valid_key(self): - AioFileStorage._validate_key("valid-key_123") + def test_backslash_raises(self, tmp_path): + storage = _make_storage(tmp_path) + with pytest.raises(ValueError, match="path separators"): + storage._validate_key("a\\b") - def test_max_length_key_ok(self): - AioFileStorage._validate_key("a" * DEFAULT_MAX_KEY_LENGTH) + def test_valid_key(self, tmp_path): + storage = _make_storage(tmp_path) + storage._validate_key("valid-key_123") class TestKeyToPathAndPathToKey: @@ -192,6 +194,15 @@ async def test_add_validates_key(self, tmp_path): with pytest.raises(ValueError): await storage.add(db, {"key": "", "value": "data"}) + 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 + + await storage.add(db, {"key": key, "value": "persisted"}) + + assert await storage.get(db, key) == "persisted" + class TestGet: """Tests for AioFileStorage.get.""" @@ -209,6 +220,12 @@ async def test_get_nonexistent(self, tmp_path): result = await storage.get(db, "nope") assert result is None + async def test_get_validates_key(self, tmp_path): + storage = AioFileStorage(FileStorageConfig(base_dir=str(tmp_path), max_key_length=3)) + db = FileSession(base_dir=tmp_path) + with pytest.raises(ValueError, match="4 > 3"): + await storage.get(db, "long") + async def test_get_text_file(self, tmp_path): storage = _make_storage(tmp_path) db = FileSession(base_dir=tmp_path) @@ -241,6 +258,12 @@ async def test_delete_nonexistent_no_error(self, tmp_path): db = FileSession(base_dir=tmp_path) await storage.delete(db, "nosuchkey") + async def test_delete_validates_key(self, tmp_path): + storage = AioFileStorage(FileStorageConfig(base_dir=str(tmp_path), max_key_length=3)) + db = FileSession(base_dir=tmp_path) + with pytest.raises(ValueError, match="4 > 3"): + await storage.delete(db, "long") + class TestQuery: """Tests for AioFileStorage.query.""" diff --git a/trpc_agent_sdk/server/openclaw/config/_config.py b/trpc_agent_sdk/server/openclaw/config/_config.py index 87884a693..6de2d4c33 100644 --- a/trpc_agent_sdk/server/openclaw/config/_config.py +++ b/trpc_agent_sdk/server/openclaw/config/_config.py @@ -120,7 +120,7 @@ class MemoryConfig(BaseModel): class FileStorageConfig(BaseModel): """trpc_claw file storage config.""" base_dir: str = "" - max_key_length: int = 255 + max_key_length: int = Field(default=255, ge=1) class SqlStorageConfig(BaseModel): diff --git a/trpc_agent_sdk/server/openclaw/session_memory/_claw_session_service.py b/trpc_agent_sdk/server/openclaw/session_memory/_claw_session_service.py index 92a614027..40158c2c6 100644 --- a/trpc_agent_sdk/server/openclaw/session_memory/_claw_session_service.py +++ b/trpc_agent_sdk/server/openclaw/session_memory/_claw_session_service.py @@ -10,6 +10,7 @@ from __future__ import annotations import copy +import hashlib import shutil import uuid from pathlib import Path @@ -191,6 +192,11 @@ async def update_session(self, session: Session) -> None: # ------------------------------------------------------------------ def _get_session_path(self, save_key: str) -> Path: + digest = hashlib.sha256(save_key.encode("utf-8")).hexdigest() + return self.sessions_dir / f"{digest}.jsonl" + + def _get_unhashed_session_path(self, save_key: str) -> Path: + """Return the session path used before filenames were hashed.""" safe_key = safe_filename(save_key.replace(":", "_")) return self.sessions_dir / f"{safe_key}.jsonl" @@ -199,14 +205,19 @@ def _get_legacy_session_path(self, save_key: str) -> Path: return self.legacy_sessions_dir / f"{safe_key}.jsonl" def _maybe_migrate(self, save_key: str, target: Path) -> None: - """Move a legacy session file to *target* if one exists and target is absent.""" + """Move an older session file to *target* when one exists.""" if target.exists(): return - legacy = self._get_legacy_session_path(save_key) - if not legacy.exists(): + candidates = ( + self._get_unhashed_session_path(save_key), + self._get_legacy_session_path(save_key), + ) + 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 - try: - shutil.move(str(legacy), str(target)) - logger.info("Migrated session %s from legacy path", save_key) - except Exception as exc: # pylint: disable=broad-except - logger.error("Failed to migrate session %s: %s", save_key, exc) diff --git a/trpc_agent_sdk/server/openclaw/storage/_aiofile_storage.py b/trpc_agent_sdk/server/openclaw/storage/_aiofile_storage.py index c9bf518f5..0543e5c88 100644 --- a/trpc_agent_sdk/server/openclaw/storage/_aiofile_storage.py +++ b/trpc_agent_sdk/server/openclaw/storage/_aiofile_storage.py @@ -27,7 +27,6 @@ from aiofiles import ospath as aio_ospath from nanobot.utils.helpers import safe_filename from trpc_agent_sdk.storage import BaseStorage -from trpc_agent_sdk.storage import DEFAULT_MAX_KEY_LENGTH from ..config import FileStorageConfig from ._constants import HISTORY_FILENAME @@ -113,6 +112,7 @@ async def add(self, db: FileSession, data: FileData | dict[str, Any]) -> None: @override async def delete(self, db: FileSession, key: str, conditions: Optional[FileCondition] = None) -> None: + self._validate_key(key) file_path = await self._resolve_key_path(db.base_dir, key) if not await aio_ospath.exists(file_path): return @@ -140,6 +140,7 @@ async def query(self, @override async def get(self, db: FileSession, key: str) -> Any: + self._validate_key(key) file_path = await self._resolve_key_path(db.base_dir, key) if not await aio_ospath.exists(file_path): return None @@ -177,12 +178,11 @@ def _key_to_path(base_dir: Path, key: str) -> Path: def _path_to_key(path: Path) -> str: return unquote(path.stem) - @staticmethod - def _validate_key(key: str) -> None: + 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")