From 3cf29f4633e323cee691af213eb0927cb9d255eb Mon Sep 17 00:00:00 2001 From: Chiguyong Date: Mon, 6 Jul 2026 08:36:59 +0800 Subject: [PATCH] =?UTF-8?q?fix(enterprise):=20ce-code-review=20P1=20fixes?= =?UTF-8?q?=20=E2=80=94=20SSO=20is=5Factive=20+=20span=20leak=20+=20Helm?= =?UTF-8?q?=20chart=20integration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 5c applied safe P1/P2/P3 fixes from multi-agent code review: - SEC-1 (P1, security): SSO 登录绕过 is_active 检查 — _sso_issue_token_pair 入口加显式校验，与本地 login（auth.py:377）一致。已停用用户无法再通过 IDP 回调拿 JWT，封堵 R1b deprovisioning 绕过。 - PERF-2/3 (P2, reliability): gateway chat() span 泄漏 + 异常未记录 — try 块 扩展到覆盖整个 span 生命周期（含 cache 设置），新增 except 分支调用 record_exception + set_status(ERROR)，失败调用在 trace 中不再显示为成功。 - API-3 (P1, test isolation): SCIM router 使用 import-time 捕获的 DEFAULT_AUTH_DB_PATH — 改为 resolve_auth_db_path() 在调用时读取 env var， 让测试 monkeypatch.setenv 可见。 - MAINT-1 (P3, dead code): 删除 BitableRepository.delete_view（被 delete_view_if_not_last 取代，无生产调用方，仅 docs 残留引用）。 - PERF-4/5 (P3, performance): pii_filter._record_pii_metrics 的 from import 提升至模块顶部 + except: pass 改为 logger.debug 记录失败原因。 - DEPLOY-3 (P1, chart integration): ConfigMap 不可达 — deployment.yaml args 增加 --config /etc/agentkit/agentkit.yaml，让应用能加载 chart 渲染 的非密配置。 - DEPLOY-6/7 (P3, doc/chart consistency): values.yaml 注释残留旧 slot 名 redisPgPasswords → redisPassword；slot 计数 10 → 11（5 处 chart + 3 处 deployment docs 同步修正）。 Verification: - ruff check + format clean (7 modified .py files) - pytest tests/unit/{auth,bitable} + test_{pii_filter,oidc_provider, saml_provider,scim_router,auth_audit,telemetry,prompt_cache_layers}.py → 336 passed, 145 skipped - helm lint + helm template 验证 secret key 一致性 Residual findings (deferred to follow-up): - PERF-1: PII filter 未接入 gateway 生产路径（manual, 需配置接入决策） - API-1/2/4/5: SCIM RFC 7644 合规性（totalResults/error format/Location header/SSO 预配联动）— manual, 需设计决策 - DEPLOY-1/2/4: Helm chart ↔ app 集成断裂（部署名引用/Redis 鉴权/PG 密码双源） — manual, 需 app+chart 联动修改 --- .../helm/agentkit/templates/deployment.yaml | 2 + .../agentkit/templates/external-secret.yaml | 2 +- .../agentkit/templates/sealed-secret.yaml | 2 +- deploy/helm/agentkit/values.yaml | 6 +- docs/deployment/k8s-install.md | 2 +- docs/deployment/key-rotation-runbook.md | 2 +- docs/deployment/secret-inventory.md | 2 +- src/agentkit/bitable/repository.py | 7 -- src/agentkit/llm/gateway.py | 107 ++++++++++-------- src/agentkit/llm/pii_filter.py | 21 ++-- src/agentkit/server/auth/scim/router.py | 11 +- src/agentkit/server/routes/auth.py | 4 + 12 files changed, 96 insertions(+), 72 deletions(-) diff --git a/deploy/helm/agentkit/templates/deployment.yaml b/deploy/helm/agentkit/templates/deployment.yaml index 4446d9f..d87e3ec 100644 --- a/deploy/helm/agentkit/templates/deployment.yaml +++ b/deploy/helm/agentkit/templates/deployment.yaml @@ -32,6 +32,8 @@ spec: {{- toYaml .Values.securityContext | nindent 12 }} args: - serve + - --config + - /etc/agentkit/agentkit.yaml - --host - 0.0.0.0 - --port diff --git a/deploy/helm/agentkit/templates/external-secret.yaml b/deploy/helm/agentkit/templates/external-secret.yaml index ed8a879..cddda56 100644 --- a/deploy/helm/agentkit/templates/external-secret.yaml +++ b/deploy/helm/agentkit/templates/external-secret.yaml @@ -1,7 +1,7 @@ {{- /* External Secrets Operator 模板 — 当 secrets.backend=external-secrets 时渲染。 ExternalSecret 引用运维预创建的 SecretStore（指向 Vault / AWS SM / GCP SM / Azure KV）， -从外部密钥管理服务拉取全部 10 个 secret slot（KTD-4）。 +从外部密钥管理服务拉取全部 11 个 secret slot（KTD-4）。 */ -}} {{- if eq .Values.secrets.backend "external-secrets" }} apiVersion: external-secrets.io/v1beta1 diff --git a/deploy/helm/agentkit/templates/sealed-secret.yaml b/deploy/helm/agentkit/templates/sealed-secret.yaml index b22d926..18616d1 100644 --- a/deploy/helm/agentkit/templates/sealed-secret.yaml +++ b/deploy/helm/agentkit/templates/sealed-secret.yaml @@ -1,6 +1,6 @@ {{- /* Sealed Secrets 模板（Bitnami Sealed Secrets controller）— 当 secrets.backend=sealed-secrets 时渲染。 -覆盖全部 10 个 secret slot（KTD-4）。encryptedData 由运维通过 kubeseal 生成后以 --set 或 +覆盖全部 11 个 secret slot（KTD-4）。encryptedData 由运维通过 kubeseal 生成后以 --set 或 override values 文件注入。禁止明文 Kubernetes Secret + Git 提交。 */ -}} {{- if eq .Values.secrets.backend "sealed-secrets" }} diff --git a/deploy/helm/agentkit/values.yaml b/deploy/helm/agentkit/values.yaml index e01ee5f..b473fad 100644 --- a/deploy/helm/agentkit/values.yaml +++ b/deploy/helm/agentkit/values.yaml @@ -110,7 +110,7 @@ otel: # --------------------------------------------------------------------------- redis: url: redis://redis:6379/0 - # 实际密码通过 env REDIS_PASSWORD 注入（来自 secrets.redisPgPasswords） + # 实际密码通过 env REDIS_PASSWORD 注入（来自 secrets.redisPassword） postgres: # DATABASE_URL（不含密码，密码通过 ${POSTGRES_PASSWORD} 占位由 env 注入） @@ -124,7 +124,7 @@ auth: idps: [] # IDP 配置列表（非密部分），密钥引用 secrets.idpSigningCerts # --------------------------------------------------------------------------- -# 密钥清单 — 10 个 secret slot（KTD-4 / R2a） +# 密钥清单 — 11 个 secret slot（KTD-4 / R2a） # --------------------------------------------------------------------------- # backend 选择： # sealed-secrets — 使用 Bitnami Sealed Secrets（模板 templates/sealed-secret.yaml） @@ -140,7 +140,7 @@ secrets: # 后端类型：vault | aws | gcp | azure backend: vault - # --- 10 个 secret slot --- + # --- 11 个 secret slot --- # 每个 slot：name=Kubernetes Secret key；description=用途；rotation=轮换周期 # 1) JWT 签名密钥（HS256）— access(15m) + refresh(7d) token 签名 diff --git a/docs/deployment/k8s-install.md b/docs/deployment/k8s-install.md index 7f3e070..2ef0b38 100644 --- a/docs/deployment/k8s-install.md +++ b/docs/deployment/k8s-install.md @@ -32,7 +32,7 @@ ### 2.1 准备密钥 -按 `docs/deployment/secret-inventory.md` 准备 10 个 secret slot 的明文值， +按 `docs/deployment/secret-inventory.md` 准备 11 个 secret slot 的明文值， 然后根据所选 backend 生成密钥资源。 **Sealed Secrets 方式**（推荐单集群 / 离线场景）： diff --git a/docs/deployment/key-rotation-runbook.md b/docs/deployment/key-rotation-runbook.md index 7995a3b..9caf123 100644 --- a/docs/deployment/key-rotation-runbook.md +++ b/docs/deployment/key-rotation-runbook.md @@ -1,6 +1,6 @@ # Fischer AgentKit — 密钥轮换 Runbook -本文档给出全部 10 个 secret slot（KTD-4 / R2a）的轮换操作流程。 +本文档给出全部 11 个 secret slot（KTD-4 / R2a）的轮换操作流程。 所有轮换遵循「双密钥并行 → 切换 → 旧密钥退役」原则，避免服务中断。 ## 通用轮换原则 diff --git a/docs/deployment/secret-inventory.md b/docs/deployment/secret-inventory.md index 96b8724..371e752 100644 --- a/docs/deployment/secret-inventory.md +++ b/docs/deployment/secret-inventory.md @@ -1,6 +1,6 @@ # Fischer AgentKit — 密钥清单（Secret Inventory） -本文档列出 Helm Chart 涉及的全部 10 个 secret slot（KTD-4 / R2a）， +本文档列出 Helm Chart 涉及的全部 11 个 secret slot（KTD-4 / R2a）， 每个 slot 含：名称、用途、来源、轮换周期、归属角色、对应环境变量。 > 密钥后端二选一：Sealed Secrets（`secrets.backend: sealed-secrets`）或 diff --git a/src/agentkit/bitable/repository.py b/src/agentkit/bitable/repository.py index 426780b..afaee6d 100644 --- a/src/agentkit/bitable/repository.py +++ b/src/agentkit/bitable/repository.py @@ -486,13 +486,6 @@ class BitableRepository: await session.commit() return View.model_validate(entity) if entity else None - async def delete_view(self, view_id: str) -> bool: - """Delete a view. Returns True if a row was deleted.""" - async with self._session_factory() as session: - result = await session.execute(delete(ViewModel).where(ViewModel.id == view_id)) - await session.commit() - return result.rowcount > 0 - async def delete_view_if_not_last(self, view_id: str, table_id: str) -> bool: """Atomically delete a view only if it's not the last one in its table (DR-4 TOCTOU fix). diff --git a/src/agentkit/llm/gateway.py b/src/agentkit/llm/gateway.py index d74d753..dbc9cf5 100644 --- a/src/agentkit/llm/gateway.py +++ b/src/agentkit/llm/gateway.py @@ -157,54 +157,55 @@ class LLMGateway: _span = _span_cm.__enter__() start = time.monotonic() - - # ── Cache check (U17 — LiteLLM cache via cache_key in request) ── - # LiteLLM 在 litellm.acompletion 内部处理缓存读写，gateway 只需： - # 1. 构建 per-user + ACL-scoped cache_key（安全要求 a, b） - # 2. 将 cache 参数注入 kwargs 透传到 provider - # 3. 检测响应的 cache_hit 标志，用于 usage tracking（cost=0） - if self._cache_manager is not None: - from agentkit.llm.cache import LitellmCacheManager - - # 解析 KB caching_disabled（安全要求 c） - # 非 RAG 请求（kb_id=None）→ 默认启用缓存（无 KB 数据需保护）。 - # RAG 请求（kb_id!=None）→ fail-closed：默认禁用缓存，仅在 settings - # 明确返回 caching_disabled=False 时启用。防止 DB 异常时 fail-open - # 导致禁用缓存的 KB 数据泄漏到缓存。 - kb_caching_disabled = kb_id is not None - if kb_id is not None: - try: - from agentkit.rag_platform.settings import get_settings_store - - settings = await get_settings_store().get_settings(kb_id) - if settings is not None: - kb_caching_disabled = settings.caching_disabled - # settings 为 None（KB 不存在）→ 保持 True（fail-closed） - except Exception as e: - logger.warning(f"Failed to read KB cache settings for kb_id={kb_id}: {e}") - # 读取异常 → 保持 True（fail-closed，禁用缓存） - - if self._cache_manager.should_cache(kb_caching_disabled, user_id): - cache_key = self._cache_manager.build_cache_key( - model=resolved_model, - messages=messages, - temperature=kwargs.get("temperature", 0.7), - tools=tools, - tool_choice=tool_choice, - max_tokens=kwargs.get("max_tokens", 2000), - user_id=user_id, - kb_acl_hash=kb_acl_hash, - ) - kwargs["cache"] = LitellmCacheManager.cache_params_for_hit(cache_key) - else: - kwargs["cache"] = LitellmCacheManager.cache_params_for_no_cache() - - # ── Normal provider call ── - models_to_try = self._get_models_to_try(resolved_model) - last_error: LLMProviderError | None = None - response: LLMResponse | None = None - + # PERF-2: try 块必须覆盖整个 span 生命周期（含 cache 设置）， + # 否则 cache_key 构建异常会泄漏 span。PERF-3: except 记录异常到 span。 try: + # ── Cache check (U17 — LiteLLM cache via cache_key in request) ── + # LiteLLM 在 litellm.acompletion 内部处理缓存读写，gateway 只需： + # 1. 构建 per-user + ACL-scoped cache_key（安全要求 a, b） + # 2. 将 cache 参数注入 kwargs 透传到 provider + # 3. 检测响应的 cache_hit 标志，用于 usage tracking（cost=0） + if self._cache_manager is not None: + from agentkit.llm.cache import LitellmCacheManager + + # 解析 KB caching_disabled（安全要求 c） + # 非 RAG 请求（kb_id=None）→ 默认启用缓存（无 KB 数据需保护）。 + # RAG 请求（kb_id!=None）→ fail-closed：默认禁用缓存，仅在 settings + # 明确返回 caching_disabled=False 时启用。防止 DB 异常时 fail-open + # 导致禁用缓存的 KB 数据泄漏到缓存。 + kb_caching_disabled = kb_id is not None + if kb_id is not None: + try: + from agentkit.rag_platform.settings import get_settings_store + + settings = await get_settings_store().get_settings(kb_id) + if settings is not None: + kb_caching_disabled = settings.caching_disabled + # settings 为 None（KB 不存在）→ 保持 True（fail-closed） + except Exception as e: + logger.warning(f"Failed to read KB cache settings for kb_id={kb_id}: {e}") + # 读取异常 → 保持 True（fail-closed，禁用缓存） + + if self._cache_manager.should_cache(kb_caching_disabled, user_id): + cache_key = self._cache_manager.build_cache_key( + model=resolved_model, + messages=messages, + temperature=kwargs.get("temperature", 0.7), + tools=tools, + tool_choice=tool_choice, + max_tokens=kwargs.get("max_tokens", 2000), + user_id=user_id, + kb_acl_hash=kb_acl_hash, + ) + kwargs["cache"] = LitellmCacheManager.cache_params_for_hit(cache_key) + else: + kwargs["cache"] = LitellmCacheManager.cache_params_for_no_cache() + + # ── Normal provider call ── + models_to_try = self._get_models_to_try(resolved_model) + last_error: LLMProviderError | None = None + response: LLMResponse | None = None + for model_name in models_to_try: try: provider, actual_model = self._resolve_model(model_name) @@ -305,6 +306,18 @@ class LLMGateway: self._record_prompt_cache_metric(response.usage, resolved_model) return response + except Exception as e: + # PERF-3: 记录异常到 span — __exit__(None, None, None) 会丢失异常信息， + # 失败的 LLM 调用在 trace 中显示为成功。手动 record_exception + 设 ERROR 状态。 + if _span is not None: + try: + _span.record_exception(e) + from opentelemetry.trace import Status, StatusCode + + _span.set_status(Status(StatusCode.ERROR, str(e))) + except Exception: # noqa: BLE001 — span 记录失败不影响异常向上传播 + pass + raise finally: if _span_cm is not None: _span_cm.__exit__(None, None, None) diff --git a/src/agentkit/llm/pii_filter.py b/src/agentkit/llm/pii_filter.py index 633f108..04691f9 100644 --- a/src/agentkit/llm/pii_filter.py +++ b/src/agentkit/llm/pii_filter.py @@ -20,6 +20,11 @@ import logging import re from dataclasses import dataclass, field +from agentkit.telemetry.metrics import ( + pii_filter_coverage_counter, + pii_filter_redacted_counter, +) + logger = logging.getLogger(__name__) @@ -120,18 +125,18 @@ def _redact(text: str, ner_hook=None) -> PIIFilterResult: def _record_pii_metrics(status: str, redacted_count: int | None = None) -> None: - """记录 PII 脱敏 OTel 指标 — 失败静默降级（不影响主流程）。""" - try: - from agentkit.telemetry.metrics import ( - pii_filter_coverage_counter, - pii_filter_redacted_counter, - ) + """记录 PII 脱敏 OTel 指标 — 失败静默降级（不影响主流程）。 + PERF-4: 用 logger.debug 记录失败原因（而非 except: pass）， + 保留静默降级语义同时让排障可见。 + PERF-5: import 提升至模块顶部，避免热路径每次 from import 开销。 + """ + try: pii_filter_coverage_counter().add(1, {"pii_filter.status": status}) if redacted_count is not None: pii_filter_redacted_counter().add(redacted_count) - except Exception: - pass + except Exception as e: # noqa: BLE001 — 指标失败不影响 PII 过滤主流程 + logger.debug("PII metric recording failed: %s", e) def filter_pii( diff --git a/src/agentkit/server/auth/scim/router.py b/src/agentkit/server/auth/scim/router.py index 9645c27..d53057c 100644 --- a/src/agentkit/server/auth/scim/router.py +++ b/src/agentkit/server/auth/scim/router.py @@ -23,7 +23,12 @@ import aiosqlite from fastapi import APIRouter, Depends, HTTPException, Query, Request, status from pydantic import BaseModel, ConfigDict -from ...auth.models import DEFAULT_AUTH_DB_PATH, init_auth_db, user_row_to_dict, _now_iso +from ...auth.models import ( + init_auth_db, + resolve_auth_db_path as _resolve_default_db_path, + user_row_to_dict, + _now_iso, +) from ...auth.password import hash_password from .models import SCIMListResponse, SCIMPatchRequest, SCIMUser @@ -36,7 +41,9 @@ _SCIM_USER_SCHEMA = "urn:ietf:params:scim:schemas:core:2.0:User" def _resolve_db_path(request: Request) -> Path: path = getattr(request.app.state, "auth_db_path", None) - return Path(path) if path else DEFAULT_AUTH_DB_PATH + # API-3: 用 resolve_auth_db_path() 在调用时读取 env var， + # 让测试 monkeypatch.setenv("AGENTKIT_AUTH_DB", tmp) 可见。 + return Path(path) if path else _resolve_default_db_path() def _resolve_scim_token(request: Request) -> str | None: diff --git a/src/agentkit/server/routes/auth.py b/src/agentkit/server/routes/auth.py index a7b95bc..62871a4 100644 --- a/src/agentkit/server/routes/auth.py +++ b/src/agentkit/server/routes/auth.py @@ -796,6 +796,10 @@ class SSORedirectResponse(BaseModel): async def _sso_issue_token_pair(request: Request, user_dict: dict[str, object]) -> TokenResponse: """SSO 登录成功后签发 JWT pair + 创建 session（复用 login 流程）。""" + # SEC-1: SSO 流程必须与本地 login（auth.py:377）一致地校验 is_active， + # 否则已 deprovision 的用户可通过 IDP 回调重新拿到 JWT + session。 + if not bool(user_dict.get("is_active", True)): + raise HTTPException(status_code=403, detail="Account is disabled") db_path = await _ensure_db(request) secret = _resolve_jwt_secret(request) svc: SessionService = get_session_service()