🐛 Bugfix:fix aidp search tool params' save error#3296#3297
Conversation
… SSL verification in aidp_service and aidp_search_tool (#3280)
…d and frontend components. Ensure only defined values are used when merging and updating tool configurations.
… update unit tests to reflect changes in return type for tool instance creation and update.
| tool_info_dict = tool_info.__dict__ | { | ||
| "tenant_id": tenant_id, "user_id": user_id, "version_no": version_no} | ||
|
|
||
| # Filter out null values from params to avoid saving nulls to database |
There was a problem hiding this comment.
过滤 null 值的逻辑会丢失用户有意设置为 null 的默认参数。例如某个参数的 default 本身就是 None（表示"未配置"），这里会被过滤掉，导致前端无法区分"参数不存在"和"参数值为 null"。建议仅过滤显式传入的 null，而非所有 null 值。
| session.flush() # Flush to get the ID | ||
| tool_instance = new_tool_instance | ||
| return tool_instance | ||
| return as_dict(tool_instance) |
There was a problem hiding this comment.
返回类型从 ORM 对象改为 dict（as_dict(tool_instance)），这可能破坏依赖 ORM 属性的调用方。建议检查所有调用 create_or_update_tool_by_tool_info 的地方，确认它们都能正确处理 dict 返回值。
| params = (existing_instance or {}).get("params", {}) | ||
| # Safely get params, default to empty dict if None or not present | ||
| raw_params = (existing_instance or {}).get("params") | ||
| params = raw_params if raw_params is not None else {} |
There was a problem hiding this comment.
防御性 None 检查是好的，但当 raw_params 为 None 时静默回退到空 dict 可能掩盖数据问题。建议在此处添加 logger.warning 日志，记录 tool instance 的 params 字段为 None 的异常情况，便于排查数据一致性问题。
| instance_value = tool_info["params"].get(param_name) | ||
| # Only set default if instance value is not None | ||
| # This prevents null values from being saved to database and returned as defaults | ||
| if instance_value is not None: |
There was a problem hiding this comment.
instance_value is not None 的检查只过滤了 None 值，但不过滤空字符串、空列表等 falsy 值。如果用户有意将参数设置为空字符串（表示"清空"），这个逻辑是正确的。但如果参数值为 0 或 False（合法的 falsy 值），它们也会被保留。建议确认这种行为的意图，并在注释中说明。
| @router.get("/knowledge-bases-all") | ||
| async def fetch_all_aidp_knowledge_bases_api( | ||
| server_url: Annotated[str, Query(description="AIDP API server URL")], | ||
| api_key: Annotated[str, Query(description="AIDP API key")], |
There was a problem hiding this comment.
[P1] 这里把 api_key 放在 GET query 里传递，浏览器历史、反向代理 access log 和 APM 都很容易记录完整 URL。AIDP token 应该走 POST body 或后端保存的凭据引用，避免密钥进入 URL。
| timeout=20.0, | ||
| verify_ssl=True, | ||
| timeout=60.0, | ||
| verify_ssl=False, |
There was a problem hiding this comment.
[P1] 这里对外部 AIDP 请求关闭 TLS 校验，任何中间人都可以伪造知识库列表并窃取 Bearer token。除非有显式租户级开关和 CA 配置，否则生产路径不应默认 verify_ssl=False。
|
|
||
| while current_page <= max_pages and current_url: | ||
| logger.info( | ||
| "Fetching AIDP KBs — page %d from %s", |
There was a problem hiding this comment.
[P2] 这个日志打印完整 current_url，里面包含租户路径、分页参数，未来如果 next_link 携带签名参数也会直接落日志。建议只记录 host、page 和 request id，避免把外部 URL 原样写入日志。
| raw_next = result.get("next_link") or result.get("next") or "" | ||
| next_url_str = str(raw_next).strip() | ||
| if next_url_str: | ||
| current_url = urljoin(normalized_url + "/", next_url_str) |
There was a problem hiding this comment.
[P1] urljoin 会接受绝对 next_link；如果 AIDP 响应里返回 https://evil.example/...，后续请求会跟到攻击者控制的主机并带上 Authorization header。跟随 next_link 前需要校验 scheme/host 仍然等于 normalized_url。
| try { | ||
| const url = new URL(API_ENDPOINTS.aidp.knowledgeBasesAll, globalThis.location.origin); | ||
| url.searchParams.set("server_url", serverUrl); | ||
| url.searchParams.set("api_key", apiKey); |
There was a problem hiding this comment.
[P1] 前端同样把 apiKey 写进 query string，实际请求 URL 会进入浏览器、代理和服务端日志。这里应改成 POST body，或者只传后端侧保存的凭据 ID。
4 participants
🐛 Bugfix:fix aidp search tool params' save error
#3296