.1616750161180714:ff310f34bb0b567aae0027c5c57012d3_69e6d82a59f68fc20e58829f.69e6e1e159f68fc20e588315.69e6e1e190f0ee6098651893:Trae CN.T(2026/4/21 10:33:05)#2933
Hidden character warning
Conversation
修改API调用失败时的日志级别以更准确反映问题严重性，同时在返回给客户端的错误信息中包含具体错误原因，便于问题排查
修改测试用例以匹配apiCallTransport函数新增的布尔参数，确保测试覆盖新功能
在API调用请求中新增insecure字段，用于控制是否跳过TLS证书验证。当设置为true时，将配置HTTP客户端不验证服务器证书，适用于测试环境或自签名证书场景。同时更新相关传输层逻辑以支持此功能。
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an 'Insecure' option to the management API call tool, allowing users to bypass TLS certificate verification. The changes include updating the request structure, the transport factory function, and existing OAuth token refresh flows. Feedback was provided regarding a potential information leak by returning raw error messages in the API response and a suggestion to refactor duplicated logic within the transport configuration function to improve maintainability.
| log.WithError(errDo).Error("management APICall request failed") | ||
| c.JSON(http.StatusBadGateway, gin.H{"error": "request failed: " + errDo.Error()}) |
There was a problem hiding this comment.
在 API 响应中直接返回 errDo.Error() 可能会泄露敏感的内部信息（如内部 IP 地址、主机名或具体的网络拓扑）。虽然这是管理接口，但出于安全最佳实践，建议在日志中记录详细错误，而向客户端返回更通用的错误消息。
| log.WithError(errDo).Error("management APICall request failed") | |
| c.JSON(http.StatusBadGateway, gin.H{"error": "request failed: " + errDo.Error()}) | |
| log.WithError(errDo).Error("management APICall request failed") | |
| c.JSON(http.StatusBadGateway, gin.H{"error": "request failed"}) |
| if insecure { | ||
| if transport.TLSClientConfig == nil { | ||
| transport.TLSClientConfig = &tls.Config{} | ||
| } | ||
| transport.TLSClientConfig.InsecureSkipVerify = true | ||
| } | ||
| return transport | ||
| } | ||
| } | ||
|
|
||
| transport, ok := http.DefaultTransport.(*http.Transport) | ||
| if !ok || transport == nil { | ||
| return &http.Transport{Proxy: nil} | ||
| t := &http.Transport{Proxy: nil} | ||
| if insecure { | ||
| t.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} | ||
| } | ||
| return t | ||
| } | ||
| clone := transport.Clone() | ||
| clone.Proxy = nil | ||
| if insecure { | ||
| if clone.TLSClientConfig == nil { | ||
| clone.TLSClientConfig = &tls.Config{} | ||
| } | ||
| clone.TLSClientConfig.InsecureSkipVerify = true | ||
| } |
There was a problem hiding this comment.
应用 insecure 标志的逻辑在函数中重复出现了三次。这种重复增加了维护成本，并可能导致未来修改时的不一致。建议重构此函数，先确定 http.RoundTripper 实例，然后在函数末尾统一进行类型断言并设置 TLSClientConfig.InsecureSkipVerify。
luispater
left a comment
luispater
left a comment
There was a problem hiding this comment.
Summary:
This PR adds an insecure flag to the management /v0/management/api-call endpoint to optionally skip TLS certificate verification, and tweaks APICall failure logging + error response messaging.
Blocking:
- The
auto-retarget-main-pr-to-devworkflow fails because there is already an open PR (#2932) for basedevand the same head branch. Please resolve the duplicate (close one PR or adjust base/head appropriately). - Please update the in-file endpoint docs for
/v0/management/api-callto include the newinsecurefield and a clear warning (intended for testing/self-signed cert scenarios only). - Add unit coverage for the new behavior (e.g.,
apiCallTransport(auth, true)results in a*http.TransportwithTLSClientConfig.InsecureSkipVerify == true).
Non-blocking:
- Re-evaluate exposing the raw
errDo.Error()string to clients to avoid accidental leakage and unstable error formats; logging may be sufficient. - The PR title looks auto-generated; renaming it would make future maintenance easier.
Test plan:
- Not run locally (per request: no checkout / no working tree changes).
- Suggested to run on the PR branch:
go test ./...andgo build -o test-output ./cmd/server && rm test-output.
CI notes (informational):
translator-path-guardandpr-test-buildare green (ignored for recommendation).auto-retarget-main-pr-to-devfails due to the existing PR targetingdev.
2 participants
test(management): 更新apiCallTransport测试以包含新参数
修改测试用例以匹配apiCallTransport函数新增的布尔参数，确保测试覆盖新功能
feat(api): 添加支持忽略TLS证书验证的选项
在API调用请求中新增insecure字段，用于控制是否跳过TLS证书验证。当设置为true时，将配置HTTP客户端不验证服务器证书，适用于测试环境或自签名证书场景。同时更新相关传输层逻辑以支持此功能。