diff --git a/README.md b/README.md index 073e8ea..65ea3f4 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ cd frontend pnpm test ``` -当前回归基线为后端 116 项测试、前端 29 项测试，且 TypeScript 类型检查和生产构建通过。测试数量会随功能增长，以本地实际输出和 CI 为准。 +当前回归基线为后端 121 项测试、前端 29 项测试，且 TypeScript 类型检查和生产构建通过。测试数量会随功能增长，以本地实际输出和 CI 为准。 构建产物位于 `frontend/dist`，该目录不提交到 Git。 diff --git a/backend/README.md b/backend/README.md index 9d32774..2f32253 100644 --- a/backend/README.md +++ b/backend/README.md @@ -23,7 +23,7 @@ uv run uvicorn app.main:app --reload --host 127.0.0.1 --port 8000 uv run pytest ``` -当前基线为 116 项测试通过。Provider API Key 可通过前端设置页写入，也可用 `OPENAI_API_KEY`、`DEEPSEEK_API_KEY` 或 `AINOTE_CREDENTIAL_` 注入；不要把真实密钥写入仓库。`plugin.*` 是 Plugin Settings 的保留凭据命名空间，通用 Provider 凭据接口不能读写。 +当前基线为 121 项测试通过。Provider API Key 可通过前端设置页写入，也可用 `OPENAI_API_KEY`、`DEEPSEEK_API_KEY` 或 `AINOTE_CREDENTIAL_` 注入；不要把真实密钥写入仓库。`plugin.*` 是 Plugin Settings 的保留凭据命名空间，通用 Provider 凭据接口不能读写。 团队接口清单见 `../docs/contracts/后端接口契约-开发版.md`，机器可读契约以运行时的 `/openapi.json` 为准。 diff --git a/backend/app/extensions/contributions.py b/backend/app/extensions/contributions.py index a496db5..783a7d5 100644 --- a/backend/app/extensions/contributions.py +++ b/backend/app/extensions/contributions.py @@ -326,6 +326,7 @@ class PluginSettingsStore: secret_refs = entry.get("secret_refs", {}) if not isinstance(stored_values, dict) or not isinstance(secret_refs, dict): raise self._storage_format_error(plugin_id) + validated_refs = self._validate_secret_refs(plugin_id, secret_refs) values = { field.key: field.default for field in definition.fields @@ -349,7 +350,7 @@ class PluginSettingsStore: for field in definition.fields: if field.type != PluginSettingType.secret: continue - reference = secret_refs.get(field.key) + reference = validated_refs.get(field.key) secrets[field.key] = PluginSecretState( configured=isinstance(reference, str) and self._has_secret(reference) ) @@ -457,6 +458,7 @@ class PluginSettingsStore: refs = entry.get("secret_refs", {}) if not isinstance(refs, dict): raise self._storage_format_error(plugin_id) + self._validate_secret_refs(plugin_id, refs) entry["secret_refs"] = refs try: previous = self.credentials.resolve(reference) @@ -494,16 +496,28 @@ class PluginSettingsStore: refs = entry.get("secret_refs", {}) if not isinstance(refs, dict): raise self._storage_format_error(plugin_id) - reference = refs.pop(key, None) - if reference: - try: - self.credentials.delete(reference) - except CredentialStoreError as exc: - raise ExtensionError( - "PLUGIN_SECRET_STORE_ERROR", str(exc), status_code=500 - ) from exc - if plugin_id in data: + self._validate_secret_refs(plugin_id, refs) + reference = _secret_reference(plugin_id, key) + had_reference = refs.pop(key, None) is not None + if plugin_id in data and had_reference: self._write(data) + try: + self.credentials.delete(reference) + except CredentialStoreError as exc: + if had_reference: + refs[key] = reference + try: + self._write(data) + except ExtensionError as rollback_exc: + raise ExtensionError( + "PLUGIN_STORAGE_ERROR", + "Plugin Secret deletion failed and its reference could not be restored.", + status_code=500, + details={"plugin_id": plugin_id, "key": key}, + ) from rollback_exc + raise ExtensionError( + "PLUGIN_SECRET_STORE_ERROR", str(exc), status_code=500 + ) from exc return PluginSecretStatus(plugin_id=plugin_id, key=key, configured=False) def resolve_secret( @@ -515,7 +529,7 @@ class PluginSettingsStore: refs = entry.get("secret_refs", {}) if not isinstance(refs, dict): raise self._storage_format_error(plugin_id) - reference = refs.get(key) + reference = self._validate_secret_refs(plugin_id, refs).get(key) try: return self.credentials.resolve(reference) if isinstance(reference, str) else None except CredentialStoreError as exc: @@ -527,21 +541,48 @@ class PluginSettingsStore: with self._lock: data = self._read() entry = data.pop(plugin_id, None) - if isinstance(entry, dict): + references: list[str] = [] + if entry is not None and not isinstance(entry, dict): + raise self._storage_format_error(plugin_id) + if entry is not None: refs = entry.get("secret_refs", {}) - if isinstance(refs, dict): - for reference in refs.values(): - if isinstance(reference, str): - try: - self.credentials.delete(reference) - except CredentialStoreError as exc: - raise ExtensionError( - "PLUGIN_SECRET_STORE_ERROR", - str(exc), - status_code=500, - ) from exc + if not isinstance(refs, dict): + raise self._storage_format_error(plugin_id) + references = list(self._validate_secret_refs(plugin_id, refs).values()) if entry is not None: self._write(data) + try: + self.credentials.delete_many(references) + except CredentialStoreError as exc: + if entry is not None: + data[plugin_id] = entry + try: + self._write(data) + except ExtensionError as rollback_exc: + raise ExtensionError( + "PLUGIN_STORAGE_ERROR", + "Plugin uninstall failed and its Settings namespace could not be restored.", + status_code=500, + details={"plugin_id": plugin_id}, + ) from rollback_exc + raise ExtensionError( + "PLUGIN_SECRET_STORE_ERROR", str(exc), status_code=500 + ) from exc + + def _validate_secret_refs( + self, plugin_id: str, refs: dict[Any, Any] + ) -> dict[str, str]: + validated: dict[str, str] = {} + for key, reference in refs.items(): + if ( + not isinstance(key, str) + or not _SETTING_KEY.fullmatch(key) + or not isinstance(reference, str) + or reference != _secret_reference(plugin_id, key) + ): + raise self._storage_format_error(plugin_id) + validated[key] = reference + return validated def _has_secret(self, reference: str) -> bool: try: @@ -599,15 +640,19 @@ class PluginSettingsStore: def _write(self, value: dict[str, dict[str, Any]]) -> None: path = self._path() - path.parent.mkdir(parents=True, exist_ok=True) temporary = path.with_suffix(".tmp") try: + path.parent.mkdir(parents=True, exist_ok=True) temporary.write_text( json.dumps(value, ensure_ascii=False, sort_keys=True), encoding="utf-8", ) temporary.replace(path) except OSError as exc: + try: + temporary.unlink(missing_ok=True) + except OSError: + pass raise ExtensionError( "PLUGIN_STORAGE_ERROR", "Plugin settings storage cannot be written.", diff --git a/backend/app/providers/credentials.py b/backend/app/providers/credentials.py index ae1cca6..88d9485 100644 --- a/backend/app/providers/credentials.py +++ b/backend/app/providers/credentials.py @@ -119,17 +119,26 @@ class EncryptedCredentialStore: def _write_tokens(self, tokens: dict[str, str]) -> None: _, store_path = self._paths() - store_path.parent.mkdir(parents=True, exist_ok=True) - self._restrict(store_path.parent, 0o700) temporary = store_path.with_suffix(".tmp") - temporary.write_text( - json.dumps(tokens, ensure_ascii=True, sort_keys=True), - encoding="utf-8", - ) - self._restrict(temporary, 0o600) - # 凭据表同样使用原子替换，确保并发读取只会看到完整 JSON。 - temporary.replace(store_path) - self._restrict(store_path, 0o600) + try: + store_path.parent.mkdir(parents=True, exist_ok=True) + self._restrict(store_path.parent, 0o700) + temporary.write_text( + json.dumps(tokens, ensure_ascii=True, sort_keys=True), + encoding="utf-8", + ) + self._restrict(temporary, 0o600) + # 凭据表同样使用原子替换，确保并发读取只会看到完整 JSON。 + temporary.replace(store_path) + self._restrict(store_path, 0o600) + except OSError as exc: + try: + temporary.unlink(missing_ok=True) + except OSError: + pass + raise CredentialStoreError( + "Encrypted credential store cannot be written." + ) from exc def put(self, credential_id: str, secret: str) -> None: self._validate_id(credential_id) @@ -168,6 +177,24 @@ class EncryptedCredentialStore: self._write_tokens(tokens) return removed + def delete_many(self, credential_ids: list[str]) -> set[str]: + """用一次原子替换删除多个凭据，避免插件卸载只删除部分 Secret。""" + + for credential_id in credential_ids: + self._validate_id(credential_id) + with self._lock: + tokens = self._read_tokens() + removed = { + credential_id + for credential_id in credential_ids + if credential_id in tokens + } + if removed: + for credential_id in removed: + del tokens[credential_id] + self._write_tokens(tokens) + return removed + class ChainedCredentialResolver: def __init__(self, *resolvers: CredentialResolver) -> None: diff --git a/backend/tests/test_credentials.py b/backend/tests/test_credentials.py index 37ac388..ca09176 100644 --- a/backend/tests/test_credentials.py +++ b/backend/tests/test_credentials.py @@ -1,4 +1,5 @@ import asyncio +from pathlib import Path import httpx import pytest @@ -35,6 +36,33 @@ def test_encrypted_credential_store_round_trip_without_plaintext_on_disk() -> No assert store.resolve("deepseek") is None +def test_encrypted_credential_store_deletes_multiple_credentials_atomically() -> None: + store = EncryptedCredentialStore() + store.put("plugin.first", "first") + store.put("plugin.second", "second") + store.put("openai", "keep") + + removed = store.delete_many(["plugin.first", "plugin.second"]) + + assert removed == {"plugin.first", "plugin.second"} + assert store.resolve("plugin.first") is None + assert store.resolve("plugin.second") is None + assert store.resolve("openai") == "keep" + + +def test_credential_write_os_error_uses_stable_store_error(monkeypatch) -> None: + store = EncryptedCredentialStore() + store.put("existing", "value") + + def fail_replace(_path: Path, _target: Path) -> Path: + raise OSError("injected replace failure") + + monkeypatch.setattr(Path, "replace", fail_replace) + + with pytest.raises(CredentialStoreError, match="cannot be written"): + store.put("new", "value") + + def test_credential_api_never_returns_secret() -> None: written = asyncio.run( put_credential( diff --git a/backend/tests/test_plugin_contributions.py b/backend/tests/test_plugin_contributions.py index cd994f4..f3e77c2 100644 --- a/backend/tests/test_plugin_contributions.py +++ b/backend/tests/test_plugin_contributions.py @@ -15,6 +15,7 @@ from app.contracts import ( from app.extensions import ExtensionError, PluginRuntime from app.extensions.contributions import _secret_reference from app.extensions.runtime import DeclarativePluginHost +from app.providers.credentials import CredentialStoreError TEXT_TOOLS = BACKEND_DIR / "extensions" / "plugins" / "text-tools" @@ -296,6 +297,80 @@ def test_plugin_secret_reference_has_fixed_credential_safe_length() -> None: assert len(reference) <= 128 +def test_tampered_secret_reference_cannot_cross_credential_namespace() -> None: + container = build_container() + container.credentials.put("openai", "provider-private-secret") + settings_path = get_settings().data_dir / "plugins" / "settings.json" + settings_path.parent.mkdir(parents=True, exist_ok=True) + settings_path.write_text( + json.dumps( + { + "text-tools": { + "schema_version": 1, + "values": {}, + "secret_refs": {"api_key": "openai"}, + } + } + ), + encoding="utf-8", + ) + + with pytest.raises(ExtensionError) as read_error: + container.plugins.get_settings("text-tools") + with pytest.raises(ExtensionError) as uninstall_error: + container.plugins.uninstall("text-tools") + + assert read_error.value.code == "PLUGIN_STORAGE_ERROR" + assert uninstall_error.value.code == "PLUGIN_STORAGE_ERROR" + assert container.credentials.resolve("openai") == "provider-private-secret" + + +def test_secret_delete_restores_reference_when_credential_delete_fails( + monkeypatch, +) -> None: + container = build_container() + container.plugins.put_setting_secret("text-tools", "api_key", "keep-me") + settings_path = get_settings().data_dir / "plugins" / "settings.json" + original = settings_path.read_text(encoding="utf-8") + reference = _secret_reference("text-tools", "api_key") + + def fail_delete(_credential_id: str) -> bool: + raise CredentialStoreError("injected delete failure") + + monkeypatch.setattr(container.credentials, "delete", fail_delete) + + with pytest.raises(ExtensionError) as exc: + container.plugins.delete_setting_secret("text-tools", "api_key") + + assert exc.value.code == "PLUGIN_SECRET_STORE_ERROR" + assert settings_path.read_text(encoding="utf-8") == original + assert container.credentials.resolve(reference) == "keep-me" + + +def test_uninstall_restores_settings_when_atomic_secret_delete_fails( + monkeypatch, +) -> None: + container = build_container() + container.plugins.update_settings("text-tools", 1, {"result_limit": 12}) + container.plugins.put_setting_secret("text-tools", "api_key", "keep-me") + settings_path = get_settings().data_dir / "plugins" / "settings.json" + original = settings_path.read_text(encoding="utf-8") + reference = _secret_reference("text-tools", "api_key") + + def fail_delete_many(_credential_ids: list[str]) -> set[str]: + raise CredentialStoreError("injected batch delete failure") + + monkeypatch.setattr(container.credentials, "delete_many", fail_delete_many) + + with pytest.raises(ExtensionError) as exc: + container.plugins.uninstall("text-tools") + + assert exc.value.code == "PLUGIN_SECRET_STORE_ERROR" + assert settings_path.read_text(encoding="utf-8") == original + assert container.credentials.resolve(reference) == "keep-me" + assert container.plugins.get("text-tools").manifest.plugin_id == "text-tools" + + def test_invalid_command_and_settings_manifest_are_rejected(tmp_path: Path) -> None: invalid_command = tmp_path / "invalid-command" invalid_command.mkdir()