fischer-agentkit/docs/plans/2026-06-26-006-fix-p0-secur...

14 KiB
Raw Blame History

title date type status origin
fix: P0 安全与多实例一致性加固 2026-06-26 fix planned 代码走查报告portal-platform-evolution 合并后审查)

fix: P0 安全与多实例一致性加固

Summary

修复代码走查发现的 4 项高优先级问题,分两组:(1) 安全 fail-open 默认值 — Gateway KB cache fail-closed + MCP dangerous-tool 黑名单过滤;(2) 多实例部署一致性 — SecretsStore 与 Channels 全局状态迁移到 Redis消除多 worker 下的安全与状态共享缺口。

Problem Frame

portal-platform-evolution 合并引入了多端渠道U10-U12、MCP 协议U13/U16、LiteLLM 缓存U17等特性。代码走查发现两类系统性风险

  1. 安全 fail-openGateway 在 KB settings 读取异常时默认启用缓存(可能泄漏禁用缓存的 KB 数据MCP 端点暴露所有工具(含 ShellTool 等危险工具),绕过 chat 流程的 confirmation 机制。
  2. 多实例不一致SecretsStore 和 Channels 路由使用模块级内存字典存储凭证、nonce、限流、backpressure 状态,多 worker 部署下失效,与 README 宣称的 K8s 多实例部署目标冲突。

Requirements

  • R1Gateway KB cache 在 settings 读取失败时必须 fail-closed禁用缓存不得 fail-open
  • R2MCP /tools/call/tools/list(含 JSON-RPC 端点)必须过滤危险工具,禁止绕过 chat confirmation 流程
  • R3SecretsStore 加密凭证必须存储在 Redis多 worker 共享,保留 AES-256-GCM 加密层
  • R4Channels webhook nonce dedup、rate limit、backpressure、渠道配置必须存储在 Redis多 worker 一致
  • R5所有改动不破坏现有单进程行为Redis 不可用时降级到内存,需显式 log warning
  • R6:每个修复单元包含可运行的验证测试

Key Technical Decisions

KTD1: H4 危险工具过滤采用黑名单

决策:使用 _MCP_BLOCKED_TOOLS: frozenset[str] 黑名单,而非在 Tool 基类增加 requires_confirmation 元数据字段。

理由黑名单改动最小ponytail 原则),仅影响 mcp/server.py 一个文件。元数据方案需修改 Tool ABC + 所有工具子类 + ToolRegistry改动面过大。黑名单天花板已在代码注释标注ponytail: 黑名单需手动维护,新增危险工具需同步更新)。

KTD2: H1/H2 Redis 连接复用 app.state.working_redis_client

决策:复用 app.py lifespan 已创建的 app.state.working_redis_clientaioredis.from_url(redis_url, decode_responses=True)),通过 FastAPI Dependsrequest.app.state 注入到 channels 路由和 SecretsStore。

理由:避免新建 Redis 连接池(资源浪费),与 WorkingMemory、TaskManager 等现有子系统共享同一连接。decode_responses=True 与 RedisBus 模式一致。

KTD3: 降级策略 — Redis 不可用时 fail-closed

决策Redis 连接失败时webhook 端点返回 503而非降级到内存secrets 读取返回 None而非空字典

理由:安全优先。降级到内存会导致多 worker 间状态不一致nonce dedup 失效可能引发重放攻击。fail-closed 符合 AGENTS.md 安全约定。

Implementation Units

U1. Gateway KB cache fail-closed

Goal: KB settings 读取异常时禁用缓存,而非默认启用。

Requirements: R1

Dependencies: 无

Files:

  • src/agentkit/llm/gateway.py(修改第 137-146 行)
  • tests/unit/llm/test_gateway_cache_failclosed.py(新建)

Approach: 将 kb_caching_disabled 默认值从 False 改为 Truefail-closed仅在 settings 成功读取且 caching_disabled=False 时才启用缓存。读取异常或 settings 为 None 时保持 True

Patterns to follow: 现有 should_cache 已有 per_user_namespace + user_id=None 防护逻辑,本单元补齐 KB settings 读取层的防护。

Test scenarios:

  • Happy path: settings 正常读取 caching_disabled=False → 缓存启用,should_cache 返回 True
  • Happy path: settings 正常读取 caching_disabled=True → 缓存禁用,should_cache 返回 False
  • Error path: get_settings_store().get_settings() 抛异常 → kb_caching_disabled=Trueshould_cache 返回 Falsefail-closed
  • Edge case: settings 返回 NoneKB 不存在)→ kb_caching_disabled=True
  • Edge case: kb_id=None(非 RAG 请求)→ 不查 settings缓存正常启用

Verification: pytest tests/unit/llm/test_gateway_cache_failclosed.py -v 全绿mock settings store 抛异常时断言 should_cache 返回 False。


U2. MCP dangerous-tool 黑名单过滤

Goal: 阻止危险工具通过 MCP 端点执行,绕过 chat confirmation 流程。

Requirements: R2

Dependencies: 无

Files:

  • src/agentkit/mcp/server.py(修改 _all_tools_find_tool、REST call_tool、JSON-RPC tools/call
  • tests/unit/mcp/test_server_dangerous_tools.py(新建)

Approach: 在 server.py 顶部定义 _MCP_BLOCKED_TOOLS 黑名单(shellfile_writefile_deleteterminal_execute)。_all_tools()_find_tool() 过滤黑名单工具。list_tools 和 JSON-RPC tools/list 不返回黑名单工具。call_tool 对黑名单工具返回 404。

Technical designdirectional guidance:

# ponytail: 黑名单需手动维护,新增危险工具需同步更新。
# 天花板:若工具名动态生成或别名注册,黑名单可能漏判。
# 升级路径Tool 基类增加 requires_confirmation 元数据字段。
_MCP_BLOCKED_TOOLS: frozenset[str] = frozenset({
    "shell", "file_write", "file_delete", "terminal_execute",
})

Patterns to follow: shell.py 已有 _DANGEROUS_BINARIES frozenset 模式。

Test scenarios:

  • Happy path: GET /tools/list 不返回黑名单工具
  • Happy path: POST /tools/call with {"name": "safe_tool"} 正常执行
  • Error path: POST /tools/call with {"name": "shell"} → 404
  • Error path: POST /tools/call with {"name": "file_delete"} → 404
  • Integration: JSON-RPC tools/call with {"method": "tools/call", "params": {"name": "shell"}}isError: True
  • Edge case: JSON-RPC tools/list 不包含黑名单工具

Verification: pytest tests/unit/mcp/test_server_dangerous_tools.py -v 全绿;黑名单工具在 list 和 call 端点均不可见。


U3. SecretsStore Redis 迁移

Goal: 加密凭证存储迁移到 Redis多 worker 共享,保留 AES-256-GCM 加密层。

Requirements: R3, R5

Dependencies: 无(可与 U4 并行,共享 Redis 注入基础设施)

Files:

  • src/agentkit/channels/secrets.py(修改 SecretsStore.__init__set_secretget_secretdelete_secretlist_keys
  • src/agentkit/server/routes/channels.py(修改 _get_secrets_store 注入 Redis
  • src/agentkit/server/app.pylifespan 中将 working_redis_client 传入 SecretsStore
  • tests/unit/channels/test_secrets_redis.py(新建)

Approach: SecretsStore.__init__ 新增 redis 参数(aioredis.Redis | None)。_store 字典替换为 Redis 调用:set_secretredis.set(f"secrets:{key}", entry.model_dump_json())get_secretredis.get + SecretEntry.model_validate_jsondelete_secretredis.deletelist_keysredis.scan_iter。加密层(encrypt/decrypt/_derive_key不变。Redis 为 None 时降级到内存仅开发模式log warning

Patterns to follow: RedisBus._get_redis() 懒初始化模式;SharedWorkspaceredis_client 注入模式。

Test scenarios:

  • Happy path: set_secret("k", "v")get_secret("k") 返回 "v"Redis mock
  • Happy path: delete_secret("k")get_secret("k") 返回 None
  • Happy path: list_keys(prefix="feishu:") 返回匹配前缀的 key 列表
  • Edge case: Redis 为 None降级模式→ 内存字典行为log warning
  • Error path: Redis 连接失败 → get_secret 抛异常或返回 Nonefail-closed
  • Integration: 加密-存储-读取-解密往返:明文 → encrypt → Redis SET → Redis GET → decrypt → 明文一致

Verification: pytest tests/unit/channels/test_secrets_redis.py -v 全绿;多 worker 场景下 worker A set_secret 后 worker B get_secret 能读到(需 Redis faker 或集成测试)。


U4. Channels 全局状态 Redis 迁移

Goal: nonce dedup、rate limit、backpressure、渠道配置迁移到 Redis多 worker 一致。

Requirements: R4, R5, R6

Dependencies: U3共享 Redis 注入基础设施)

Files:

  • src/agentkit/server/routes/channels.py(修改 _channels_rate_limits_seen_nonces_pending_webhook_tasks 相关逻辑)
  • tests/unit/channels/test_webhook_redis_state.py(新建)

Approach:

  1. nonce dedup → Redis SET NX EXawait redis.set(f"nonce:{nonce}", "1", ex=300, nx=True) 返回 None 表示重复。
  2. rate limit → Redis ZSET 滑动窗口:ZREMRANGEBYSCORE 清理过期 + ZADD + ZCARD 计数。
  3. backpressure → Redis INCR/DECR 共享计数器:INCR webhook:concurrent,超限 DECR 并返回 429执行完毕 DECREXPIRE 30s 防 crash 计数不归零。
  4. 渠道配置 → Redis HashHSET channel:{id} 存配置 JSONHGETALL 读取。_adapter_cache 保留进程内缓存(只读快照,配置变更时 invalidate
  5. Redis 注入webhook handler 通过 request.app.state.working_redis_client 获取连接。

Technical designdirectional guidance:

# nonce dedup — 原子操作TTL 自动过期
is_new = await redis.set(f"nonce:{nonce}", "1", ex=int(_NONCE_TTL), nx=True)
if not is_new:
    return Response(status_code=200)  # 重复事件,飞书要求 3s 内响应

# rate limit — ZSET 滑动窗口
key = f"ratelimit:{client_ip}"
now = time.time()
pipe = redis.pipeline()
pipe.zremrangebyscore(key, 0, now - _RATE_LIMIT_WINDOW)
pipe.zadd(key, {str(now): now})
pipe.zcard(key)
pipe.expire(key, int(_RATE_LIMIT_WINDOW))
_, _, count, _ = await pipe.execute()
if count > _RATE_LIMIT_MAX:
    return Response(status_code=429)

# backpressure — 共享计数器
current = await redis.incr("webhook:concurrent")
if current > _WEBHOOK_MAX_CONCURRENT * 2:
    await redis.decr("webhook:concurrent")
    return Response(status_code=429)
try:
    # ... process webhook ...
finally:
    await redis.decr("webhook:concurrent")

Patterns to follow: Redis pipeline 模式(usage_store.py 已有);_adapter_cache 保留进程内缓存(_invalidate_adapter_cache 逻辑不变)。

Test scenarios:

  • Happy path: 首次 nonce → SET NX 返回 True请求正常处理
  • Happy path: 重复 nonce → SET NX 返回 None返回 200不重复处理
  • Happy path: rate limit 窗口内 100 请求通过,第 101 请求返回 429
  • Happy path: backpressure 并发 < 2x 上限时请求通过
  • Error path: backpressure 并发 >= 2x 上限时返回 429
  • Edge case: nonce TTL 过期后相同 nonce 可再次使用mock time.sleep 或 Redis TTL
  • Edge case: rate limit 窗口滚动后计数重置
  • Integration: 渠道配置写入 Redis 后,新 worker 的 _adapter_cache miss 时从 Redis 读取并缓存

Verification: pytest tests/unit/channels/test_webhook_redis_state.py -v 全绿;ruff check src/ && ruff format src/ 通过;现有 tests/unit/channels/test_wecom.py 等不回归。


Scope Boundaries

In Scope

  • 4 项高优先级问题修复H3/H4/H1/H2
  • Redis 连接复用现有 app.state.working_redis_client
  • 单元测试覆盖每个修复单元

Out of Scope

  • 中优先级问题M1-M8feishu/slack decode 异常、dingtalk 时间戳窗口、mcp/client 性能、app.py shutdown 超时等
  • 低优先级问题L1-L3X-Forwarded-For 信任、生产 guard 触发条件、Any 类型
  • PostgreSQL 持久化迁移Redis 已满足多实例共享需求)
  • MCP confirmation 协议实现(黑名单方案足够,元数据方案为后续升级路径)
  • Tool 基类 requires_confirmation 元数据重构

Deferred to Follow-Up Work

  • M1 feishu/slack body.decode("utf-8") 异常捕获 → 独立 PR
  • M4 mcp/server.py 异常消息脱敏 + REST/JSON-RPC 代码去重 → 独立 PR
  • M6 app.py shutdown asyncio.gather 超时 → 独立 PR
  • H4 升级路径Tool 基类 requires_confirmation 元数据 → 下个迭代

Risks & Dependencies

  • 风险 1H1/H2 Redis 迁移后Redis 不可用会导致 webhook 端点完全失效fail-closed。缓解lifespan 启动时 ping Redis不可用时 log error 但不阻止启动(单进程降级模式仍可用)。
  • 风险 2U4 backpressure INCR/DECR 非原子crash 可能导致计数不归零。缓解:EXPIRE 30s 安全网 + 定期清理。
  • 依赖 1U4 依赖 U3 完成的 Redis 注入基础设施。
  • 依赖 2:所有单元测试使用 fakeredisunittest.mock.AsyncMock mock Redis不依赖真实 Redis 实例。

System-Wide Impact

  • 运维:多 worker 部署gunicorn -w 4现在可正确共享渠道状态无需 sticky session
  • 安全MCP 端点不再暴露危险工具KB 缓存 fail-closed 防止数据泄漏
  • 性能Redis 网络往返替代内存字典,单请求延迟增加 ~1ms可接受backpressure 跨 worker 生效
  • 兼容性:单进程开发模式(无 Redis降级到内存存储行为不变

Open Questions

无 — 所有技术决策已在 KTD1-KTD3 中明确。