Files
md-vector-db/docs/代码审计/全面安全与质量审计报告-2026-07-10.md
T

370 lines
20 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# md-vector-db 全面安全与质量审计报告
> **审计日期**: 2026-07-10
> **审计范围**: 全部源代码(22 个源文件)、13 个测试文件、配置、文档
> **测试基线**: 90 个测试全部通过
> **项目版本**: commit `3b8b585`
---
## 一、审计概览
本次审计由三个并行代理分别审查 **核心模块**config/db/embedder/ingest/search/security/splitters)、**服务层与 CLI**app/auth/deps/main.py/scripts)、**测试与配置**tests/pyproject.toml/config.yaml/文档),最终整合为本报告。
### 问题统计
| 严重级别 | 数量 | 含义 |
|----------|------|------|
| **CRITICAL** | **1** | 阻断 — 数据重复处理浪费 GPU 资源 |
| **HIGH** | **13** | 警告 — 合并前应修复 |
| **MEDIUM** | **11** | 信息 — 应考虑修复 |
| **LOW** | **10** | 注意 — 可选修复 |
| **总计** | **35** | |
### 覆盖评估
| 模块 | 覆盖程度 |
|------|----------|
| `core/security.py` | ✅ 优秀 — 所有路径穿越场景已覆盖 |
| `core/config.py` | ✅ 良好 — YAML 加载/默认值/环境变量 |
| `core/embedder.py` | ✅ 良好 — 所有 provider 初始化和 batch_embed |
| `core/splitters/*` (除 markdown) | ✅ 良好 |
| `core/db.py` | ⚠️ 基本 — 缺少 list_collections、write_guard、并发测试 |
| `core/ingest.py` | ❌ 不足 — 缺少 ingest_file、ingest_directory |
| `core/search.py` | ❌ 不足 — 缺少 3/4 方法测试 |
| `server/deps.py` / `app.py` | ⚠️ 基本 — 缺少认证/速率限制/CORS 配置测试 |
| `server/auth.py` | ❌ 零覆盖 |
| `cli/main.py` | ❌ 零覆盖 |
| **整体估算** | **~55-60%**(低于 80% 目标) |
---
## 二、CRITICAL 级别(1 个)
### [CRIT-1] `ingest_obsidian.py` 顶层 .md 文件被重复入库
- **文件**: `scripts/ingest_obsidian.py`,第 54-77 行
- **根因**: 第一轮用 `rglob("*.md")` 递归扫描所有文件,第二轮用 `glob("*.md")` 再次收集顶层文件。由于 `rglob` 已经包含顶层文件,每个顶层 `.md` 被处理两次。
- **影响**: 嵌入计算浪费(GPU 资源),入库时间翻倍于顶层文件数量。第二次入库虽然会覆盖第一次(file_name 相同,先删后加),但嵌入已完成,白白消耗了 GPU。
- **修复**: 删除第二轮循环(第 67-77 行),`rglob` 已覆盖所有文件。
---
## 三、HIGH 级别(13 个)
### 安全
#### [HIGH-1] CORS `allow_credentials=True` 与 `allow_origins="*"` 冲突
- **文件**: `src/server/app.py`,第 66-72 行
- **根因**: CORS 规范禁止 `Access-Control-Allow-Origin: *``Access-Control-Allow-Credentials: true` 同时使用。浏览器会拒绝此配置。当前默认 `CORS_ORIGINS="*"``allow_credentials=True` 直接违反规范。
- **此外**: `*` 允许任意来源跨域访问,安全上过于宽松。
- **修复**:
```python
app.add_middleware(
CORSMiddleware,
allow_origins=os.environ.get("CORS_ORIGINS", "http://localhost:3000").split(","),
allow_credentials=False, # 默认关闭
...
)
```
#### [HIGH-2] `search_documents` 端点缺少异常处理
- **文件**: `src/server/app.py`,第 147-155 行
- **根因**: `ingest_document` 有完整的 `try/except` 包裹,但 `search_documents` 完全没有。如果嵌入模型加载失败或 ChromaDB 查询异常,会返回 FastAPI 默认的原始 traceback,泄露内部路径和调用栈。
- **修复**: 添加异常捕获:
```python
try:
searcher = state.get_searcher(req.collection)
results = searcher.search(req.query, top_k=req.top_k)
return {"results": results, "collection": searcher.collection_name}
except Exception:
logger.exception("检索失败")
raise HTTPException(status_code=500, detail="服务器内部错误")
```
#### [HIGH-3] `delete_document` 的 `collection` 查询参数缺少输入校验
- **文件**: `src/server/app.py`,第 163 行
- **根因**: `IngestRequest.collection` 和 `SearchRequest.collection` 都使用了 `pattern=r"^[a-zA-Z0-9_-]+$"` 校验,但 `delete_document` 的 `collection` 查询参数没有任何正则校验。ChromaDB collection 名称允许的字符集有限,传入非法字符可能导致未预期的 ChromaDB 内部错误。
- **修复**: 添加 `Query(default=None, max_length=128, pattern=r"^[a-zA-Z0-9_-]+$")`。
#### [HIGH-4] CLI 与 API 的路径安全检查逻辑不一致
- **文件**: `src/cli/main.py` 第 85 行 vs `src/server/app.py` 第 125-130 行
- **根因**: CLI 仅用 `is_safe_path()` 做基础检查(拒绝 `..` + 绝对路径),而 API 还额外做了 `os.path.commonpath` 绑定当前工作目录的检查。两处策略不同增加了安全漏洞风险。
- **修复**: 提取统一的 `is_path_within_workspace()` 函数到 `security.py`,两处共用:
```python
def is_path_within_workspace(path_str: str) -> bool:
"""检查路径是否在当前工作目录内(防路径穿越)。"""
if not is_safe_path(path_str):
return False
path = Path(path_str).resolve()
cwd = Path.cwd().resolve()
try:
return Path(os.path.commonpath([str(path), str(cwd)])) == cwd
except ValueError:
return False
```
### 功能
#### [HIGH-5] chunk 配置项被硬编码覆盖 — YAML 中的 chunk 设置完全无效
- **文件**: `src/core/ingest.py`,第 46 行
- **根因**: `get_splitter(file_path, max_size=1000, overlap=100)` 写死了值。用户无论怎么修改 `config.yaml` 里的 `chunk.max_size` 和 `chunk.overlap`,都完全不会生效。
- **影响**: 所有通过 API/CLI 正常流程的入库都使用硬编码值。`AppConfig.chunk` 数据类从未在正常入库链路中被读取。
- **修复**: `DocumentIngestor` 构造函数接受 `ChunkConfig` 参数,`deps.py` 的 `AppState.get_ingestor` 传入 `self.config.chunk`。
#### [HIGH-6] CLI `serve` 命令双重加载嵌入模型
- **文件**: `src/cli/main.py` 第 174 行 + `src/server/deps.py`
- **根因**: `serve` 命令调用 `get_state().config`,触发 `AppState.__init__` → `create_embedder()` 加载模型(第一次)。uvicorn 子进程启动后,`src.server.app:app` 生命周期内再次调用 `get_state()` → 加载模型(第二次)。对于 `serve` 命令,只需要服务配置(host, port, SSL),不需要加载 ML 模型。
- **影响**: 模型被加载两次,浪费几 GB 内存和数秒启动时间。
- **修复**: 将配置加载与模型加载解耦——提供一个不初始化模型的轻量配置读取函数给 CLI `serve` 命令使用。
### 代码质量
#### [HIGH-7] PDF 文件句柄泄漏 — fitz.Document 在异常路径下未关闭
- **文件**: `src/core/splitters/pdf.py`,第 34-43 行
- **根因**: `doc.close()` 在 `try` 块内,如果在 `for page in doc` 或 `page.get_text()` 期间抛出异常,`close()` 永远不会被调用。
- **修复**: 使用 `with fitz.open(pdf_path) as doc:`(pymupdf 支持上下文管理器协议)。
#### [HIGH-8] embedder.py 中环境变量操作不是线程安全的
- **文件**: `src/core/embedder.py`,第 80-90 行
- **根因**: `os.environ["HF_ENDPOINT"]` 的读写没有锁保护。如果多个线程同时创建 `LocalEmbedder` 实例,可能产生竞态——线程 A 设置的值被 B 覆盖,A 的 `finally` 恢复时读到 B 的值。
- **修复**: 使用线程锁保护该区域,或通过 `huggingface_hub` 的 API 在进程启动时统一设置而非按实例设置。
#### [HIGH-9] `ingest_obsidian.py` 初始化阶段无异常处理
- **文件**: `scripts/ingest_obsidian.py`,第 32-35 行
- **根因**: `load_config()` / `VectorDB()` / `create_embedder()` / `DocumentIngestor()` 任一失败都会直接崩溃,没有友好的错误信息或恢复路径。
- **修复**: 添加顶层 `try/except` 包裹,捕获异常后打印 traceback 并以 `sys.exit(1)` 退出。
### 测试缺口
#### [HIGH-10] CLI 模块零测试覆盖
- **文件**: `src/cli/main.py`216 行,5 个命令)
- **影响**: 通配符展开、stdin 输入、JSON 格式化输出、路径安全检查等逻辑无自动化验证。
- **修复**: 创建 `tests/test_cli.py`,使用 `typer.testing.CliRunner` 覆盖每个命令。
#### [HIGH-11] 认证与速率限制零单元测试
- **文件**: `src/server/auth.py`58 行)
- **影响**: `verify_api_key()` 和 `RateLimiter` 两个安全关键组件完全没有独立测试。所有 API 测试因未设置 `MD_VECTOR_API_KEY` 而跳过认证验证。
- **修复**: 创建 `tests/test_auth.py`,覆盖密钥匹配/不匹配/未提供、速率限制正常/超限/过期恢复、并发线程安全。
#### [HIGH-12] `ingest_file` / `ingest_directory` 零测试
- **文件**: `src/core/ingest.py`121 行)
- **影响**: 包含扩展名检测、路径哈希、PDF/EPUB 二进制特殊处理、目录递归扫描和过滤逻辑,全部未测试。
- **修复**: 在 `tests/test_ingest.py` 中补充文件入库和多格式目录入库的集成测试。
#### [HIGH-13] `Searcher` 三个方法零测试
- **文件**: `src/core/search.py`97 行)
- **影响**: `list_sources()`、`delete_by_source()`、`get_collection_info()` 以及 `search(source_file=...)` 过滤均无测试。
- **修复**: 在 `tests/test_search.py` 中补充。
---
## 四、MEDIUM 级别(11 个)
### [MED-1] `content` 模式默认 file_name 为 `"untitled.md"` 导致数据竞态覆盖
- **文件**: `src/server/app.py`,第 137 行
- **根因**: 多个并发请求不提供 `file_name` 时都写入 `"untitled.md"`。`ingest_content` 先 `_remove_by_source(file_name)` 再 `_add_chunks()`,且两次操作之间锁释放,存在竞态——后完成的请求覆盖先完成的请求数据。
- **修复**: 自动生成 UUID 唯一名称:`file_name = req.file_name or f"untitled_{uuid.uuid4().hex[:8]}.md"`
### [MED-2] `X-XSS-Protection` 头已过时且可能引入安全问题
- **文件**: `src/server/app.py`,第 81 行
- **根因**: 此响应头已被所有现代浏览器废弃。旧版 IE 的 XSS Auditor 自身存在安全漏洞。OWASP 等安全组织明确建议不要设置此头。
- **修复**: 替换为 `Content-Security-Policy: default-src 'self'`
### [MED-3] `EXPECTED_API_KEY` 在模块导入时求值,存在加载顺序隐患
- **文件**: `src/server/auth.py`,第 14 行
- **根因**: 该常量在模块被 import 时求值。虽然目前 `config.py` 先调用 `load_dotenv()` 再触发导入链,但这是一个隐式依赖——如果有人调整导入顺序或在测试中直接 import `auth.py``EXPECTED_API_KEY` 可能为空。
- **修复**: 改为惰性求值函数:`def _get_api_key() -> str: return os.environ.get("MD_VECTOR_API_KEY", "")`
### [MED-4] ingest.py 与 search.py 存在去重删除逻辑重复
- **文件**: `ingest.py:108-120` 和 `search.py:83-97`
- **根因**: 两处 `_remove_by_source` / `delete_by_source` 逻辑几乎一模一样:获取匹配文档、删除 IDs、捕获 `ValueError`。
- **修复**: 统一放到 `VectorDB.delete_by_metadata(where)` 方法中。
### [MED-5] `list_collections_with_stats` 静默吞掉所有异常
- **文件**: `src/server/deps.py`,第 54-59 行
- **根因**: 当 ChromaDB 不可用时,返回空列表而不是错误。调用方向用户报告"没有任何集合",而非"数据库连接失败"。错误语义被扭曲。
- **修复**: 让异常向上传播,由 API 层统一处理并返回适当的 HTTP 错误。
### [MED-6] EPUB 缺失依赖测试设计有误
- **文件**: `tests/test_splitters_epub.py`,第 51-56 行
- **根因**: `test_epub_missing_dependency_message` 声称测试"未安装 ebooklib 时应给出明确提示",但模块顶部有 `pytest.importorskip("ebooklib", ...)`,意味着当 ebooklib 未安装时整个文件被 skip,该测试永远不会在目标场景下执行。
- **修复**: 将此测试移到独立文件中(不使用 `importorskip`),或使用 `monkeypatch` 模拟 `ImportError`。
### [MED-7] 环境变量测试使用 `importlib.reload` 过于脆弱
- **文件**: `tests/test_config.py`,第 89-106 行
- **根因**: `importlib.reload(config)` 有全局副作用,可能影响后续测试。`finally` 块中的清理逻辑也不完整。
- **修复**: 使用 `monkeypatch.setenv` + 重新实例化代替 `reload`。
### [MED-8] 速率限制器的配置值硬编码
- **文件**: `src/server/auth.py`,第 58 行
- **根因**: `RateLimiter(max_requests=30, window_seconds=60)` 无法通过环境变量或配置文件调整。
- **修复**: 从环境变量读取:`max_requests=int(os.environ.get("RATE_LIMIT_MAX", "30"))`
### [MED-9] `ingest_obsidian.py` 存在硬编码的绝对回退路径
- **文件**: `scripts/ingest_obsidian.py`,第 47-51 行
- **根因**: `targets` 中的三个路径仅在本机有效,在其他开发者的机器上毫无意义。如果恰好存在同名目录,会意外入库其他数据。
- **修复**: 移除硬编码回退路径,直接报错提示用户提供参数。
### [MED-10] MarkdownSplitter 测试组织不清晰
- **文件**: 分散在 `tests/test_ingest.py` 中
- **根因**: Markdown 分块测试与入仓器测试混在一起,缺少 Markdown 特有的边界测试:无标题文档、深层标题、代码块内含 `#` 号被误识别为标题等。
- **修复**: 提取到 `tests/test_splitters_markdown.py`,补充边界测试。
### [MED-11] `splitters/__init__.py` 未导出 HTMLSplitter
- **文件**: `src/core/splitters/__init__.py`,第 10-20 行
- **根因**: `HTMLSplitter` 在第 7 行被 import 但未加入 `__all__` 列表,而 `PDFSplitter` 和 `EPUBSplitter` 都在列表中。
- **修复**: 在 `__all__` 中加入 `"HTMLSplitter"`。
---
## 五、LOW 级别(10 个)
| # | 文件 | 问题 | 建议 |
|---|------|------|------|
| L-1 | `src/core/splitters/epub.py:44` | EPUB 硬编码 UTF-8 解码,非 UTF-8 文件会丢失章节 | 添加编码检测回退(先 UTF-8,失败后 chardet |
| L-2 | `src/core/embedder.py:24` | 模块级 `import requests` 依赖传递性依赖项 | 改为惰性导入(在 DashscopeEmbedder.embed 内部) |
| L-3 | `src/core/embedder.py:176` | `list.sort()` 原地修改 API 响应 | 改为 `sorted(embeddings_raw, key=...)` |
| L-4 | `src/core/config.py:16` | `DEFAULT_CONFIG_PATH` 使用相对路径 | 统一使用 `load_config` 的路径解析逻辑 |
| L-5 | `src/core/splitters/registry.py:20` | `_custom_registry` 模块级字典无线程锁保护 | 添加 `threading.Lock` 或文档说明仅限启动时调用 |
| L-6 | `pyproject.toml:21` | `dev` 依赖缺少 `pytest-cov` 和 `ruff`/`mypy` | 添加 `pytest-cov>=5.0`、`ruff>=0.8.0`、`mypy>=1.13` 及相关 tool 配置节 |
| L-7 | `config.yaml` | `host: 0.0.0.0` 与代码默认值 `127.0.0.1` 不一致 | 统一默认值或在配置文件中添加注释说明差异 |
| L-8 | `config.yaml` | SSL 字段(ssl_keyfile/ssl_certfile)未在配置文件中暴露 | 添加注释说明可用配置 |
| L-9 | `scripts/serve.py` | `DeprecationWarning` 被 Python 默认过滤,多数用户看不到 | 改为 `print(..., file=sys.stderr)` 或 `FutureWarning` |
| L-10 | `CLAUDE.md` / `README.md` | 文档中测试数量写"46 个",实际 90 个;数据流未反映多格式支持 | 更新为实际数量,数据流加入 `get_splitter()` 描述 |
---
## 六、架构与设计评估
### 优点
1. **策略模式设计良好** — `Embedder(Protocol)` 和 `Splitter(Protocol)` 接口清晰,新增 provider/格式只需实现接口并注册
2. **扩展名自动路由** — `registry.py` 的 `get_splitter()` 工厂 + 回退到 TextSplitter 的设计优雅
3. **线程安全基础扎实** — `VectorDB._write_lock` + `deps.py` 的 `_state_lock`/`_cache_lock` 构成多层保护
4. **安全防御分层** — API Key 认证 → 速率限制 → 路径穿越防护,防御纵深合理
5. **配置热加载友好** — `load_config()` 支持文件变更检测,适合长期运行的服务
### 需要改进的架构问题
1. **配置与模型加载未解耦** — `AppState.__init__` 做了太多事(配置+模型+数据库),导致 CLI `serve` 双重加载。建议拆分为 `load_config_only()` 和 `init_full_state()`
2. **chunk 配置未贯传整个链路** — `config.yaml → AppConfig.chunk` 存在但被 `ingest.py` 硬编码覆盖。应该在 `DocumentIngestor` 构造函数中接受 `ChunkConfig`
3. **路径安全检查逻辑分散** — CLI 和 API 各有一套检查,应该提取为统一的 `is_path_within_workspace()`
4. **去重删除逻辑重复** — `ingest.py` 和 `search.py` 中几乎相同的代码应合并到 `VectorDB`
---
## 七、测试策略建议
### 当前测试分布
```
tests/
├── test_api.py # 14 个 — API 端点基本功能
├── test_config.py # 7 个 — 配置加载
├── test_db.py # 4 个 — 数据库 CRUD
├── test_deps.py # 11 个 — AppState 生命周期
├── test_embedder.py # 8 个 — 嵌入 provider
├── test_ingest.py # 11 个 — 入仓(偏 MarkdownSplitter
├── test_search.py # 8 个 — 基本搜索
├── test_security.py # 11 个 — 路径安全
├── test_splitters.py # 10 个 — TextSplitter + 注册表
├── test_splitters_html.py # 4 个
├── test_splitters_pdf.py # 3 个
├── test_splitters_epub.py # 3 个
└── (缺失)
├── test_auth.py ❌ 认证与速率限制
├── test_cli.py ❌ CLI 命令
└── test_splitters_markdown.py ❌ Markdown 边界
```
### 建议新增测试
1. **`tests/test_cli.py`** — CLI 5 个命令(使用 `CliRunner`
2. **`tests/test_auth.py`** — `verify_api_key` + `RateLimiter`
3. **`tests/test_splitters_markdown.py`** — 标题边界、代码块内 `#` 号、空文档
4. **补充 `tests/test_ingest.py`** — `ingest_file` / `ingest_directory`
5. **补充 `tests/test_search.py`** — `list_sources` / `delete_by_source` / `get_collection_info`
---
## 八、修复优先级路线图
### 第一阶段(立即修复 — 1 个 CRITICAL
| 问题 | 文件 | 工作量 |
|------|------|--------|
| CRIT-1: 顶层 .md 文件重复入库 | `scripts/ingest_obsidian.py` | 5 分钟 |
### 第二阶段(尽快修复 — 13 个 HIGH)
| 问题 | 领域 | 工作量 |
|------|------|--------|
| HIGH-5: chunk 配置被子覆盖 | 功能 | 30 分钟 |
| HIGH-7: PDF 文件句柄泄漏 | 资源 | 5 分钟 |
| HIGH-1: CORS 配置冲突 | 安全 | 5 分钟 |
| HIGH-2: search 端点无异常处理 | 安全 | 5 分钟 |
| HIGH-3: delete collection 缺少校验 | 安全 | 5 分钟 |
| HIGH-4: 路径安全检查不一致 | 安全 | 20 分钟 |
| HIGH-6: serve 双重加载模型 | 性能 | 30 分钟 |
| HIGH-8: HF_ENDPOINT 非线程安全 | 稳定性 | 15 分钟 |
| HIGH-9: ingest_obsidian 无异常处理 | 稳定性 | 10 分钟 |
| HIGH-10: CLI 零测试 | 测试 | 1-2 小时 |
| HIGH-11: auth 零测试 | 测试 | 1 小时 |
| HIGH-12: ingest_file/dir 零测试 | 测试 | 1 小时 |
| HIGH-13: Searcher 方法零测试 | 测试 | 30 分钟 |
### 第三阶段(计划修复 — 11 个 MEDIUM)
涉及竞态覆盖、过时安全头、重复代码、错误语义、测试设计等。
### 第四阶段(择机修复 — 10 个 LOW)
涉及文档过时、编码回退、线程锁、工具链配置等。
---
## 九、合规性检查
对照用户全局安全规则(`~/.claude/rules/ecc/zh/security.md`)进行逐项检查:
| 检查项 | 状态 | 说明 |
|--------|------|------|
| 无硬编码密钥 | ✅ 通过 | API Key 均从环境变量读取 |
| 所有用户输入已验证 | ⚠️ 部分 | `delete_document` 的 collection 参数缺少正则校验 |
| SQL 注入防护 | N/A | 无 SQL 数据库 |
| XSS 防护 | ⚠️ 部分 | `X-XSS-Protection` 已过时,应换为 CSP |
| CSRF 保护 | ⚠️ 部分 | CORS 配置存在规范冲突 |
| 认证/授权已验证 | ✅ 通过 | API Key 认证 + hmac.compare_digest |
| 所有端点启用速率限制 | ✅ 通过 | RateLimiter 中间件 |
| 错误消息不泄露敏感数据 | ⚠️ 部分 | `search_documents` 缺少异常处理会泄露 traceback |
---
*审计人: Claude Code(多代理协作审查)*
*下次审计建议: 在修复 CRITICAL 和 HIGH 问题后进行复验*