Repository navigation
feat(services/safeline-waf): add socks5 proxy support and read methods - #563
Conversation
|
PR Title: feat(services/safeline-waf): add socks5 proxy supp... Commit: 整体评估本次变更在 SafeLine WAF 服务中新增了四个巡检只读 RPC,并引入可选的 SOCKS5 代理支持,同时统一了 protobuf Value 的 null 处理;整体结构与既有实现保持一致,接口、proto 与测试补充较完整。 总结功能主体可用,但新增代理支持存在需要修复的风险:HTTP(S) 代理配置会被静默忽略并回退直连、代理凭据只能保存在非 secret 配置中、代理请求未处理响应流错误可能触发未捕获异常。建议在合入前修正代理 scheme 校验与文档一致性、调整代理凭据的存放位置并补上响应错误监听,同时补充代理路径的测试。 Findings (3)
Prompt To Fix All With AI |
| "proxy_url": { | ||
| "type": "string", | ||
| "description": "Optional HTTP/SOCKS proxy URL for reaching the device, e.g. socks5h://user:pass@host:port." | ||
| }, |
There was a problem hiding this comment.
🟡 代理凭据只能保存在非 secret 配置中
Category: 安全与隐私
proxy_url 是唯一可配置代理凭据的字段,但它只存在于 config.schema.json,secret.schema.json 因 additionalProperties=false 不接受该字段,README 示例又直接把 user:pass 写进 config。这样 SOCKS 代理的用户名/密码只能以明文形式保存在非 secret 配置中,容易在配置展示、导出或日志中泄露。
Problem code:
OctoBus/services/chaitin__safeline-waf/config.schema.json
Lines 37 to 40 in 84e6d6c
Recommendation:
允许并优先从 secret(例如 secret.proxy_url 或独立的用户名/密码字段)读取代理凭据,config 中只保留不带凭据的代理地址。
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/chaitin__safeline-waf/config.schema.json
Line: 37-40
Comment:
代理凭据只能保存在非 secret 配置中
proxy_url 是唯一可配置代理凭据的字段,但它只存在于 config.schema.json,secret.schema.json 因 additionalProperties=false 不接受该字段,README 示例又直接把 user:pass 写进 config。这样 SOCKS 代理的用户名/密码只能以明文形式保存在非 secret 配置中,容易在配置展示、导出或日志中泄露。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| text: async () => bodyText, | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🟡 代理请求未处理响应流错误,可能触发未捕获异常
Category: 稳定性与可用性
proxiedFetch 只为 req 注册了 error 监听,响应对象 res 没有 error/aborted 处理;当上游在接收响应体期间重置连接或响应解析出错时,res 的 'error' 事件无人监听会抛出未捕获异常。这类异常发生在 Promise 之外,可能让 long-running 服务进程直接退出。
Problem code:
OctoBus/services/chaitin__safeline-waf/src/safeline-waf.js
Lines 195 to 212 in 84e6d6c
Recommendation:
在 res 上注册 res.on('error', reject) 并处理 'aborted',确保这些情况能 reject 并映射为 UNAVAILABLE。
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/chaitin__safeline-waf/src/safeline-waf.js
Line: 195-212
Comment:
代理请求未处理响应流错误,可能触发未捕获异常
proxiedFetch 只为 req 注册了 error 监听,响应对象 res 没有 error/aborted 处理;当上游在接收响应体期间重置连接或响应解析出错时,res 的 'error' 事件无人监听会抛出未捕获异常。这类异常发生在 Promise 之外,可能让 long-running 服务进程直接退出。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| // Only socks:// schemes are routable through SocksProxyAgent; a plain | ||
| // http(s) URL (e.g. the smoke harness synthesizing the mock upstream) is | ||
| // not a proxy, so fall back to a direct connection instead of throwing. | ||
| const proxyUrl = /^socks(4a?|5h?)?:\/\//i.test(rawProxyUrl) ? rawProxyUrl : ''; |
There was a problem hiding this comment.
🟡 HTTP(S) 代理配置被静默忽略并回退直连
Category: 功能正确性
config.schema.json 把 proxy_url 描述为 HTTP/SOCKS 代理,但这里只用 /^socks.../ 过滤,http:// 或 https:// 的代理地址会被置空并走直连且不报错。用户按 schema 文档配置 HTTP 代理时,请求会绕过代理直接连接 SafeLine,在只能经代理访问的私有部署中表现为连接失败或从错误网络出口访问。
Problem code:
Recommendation:
对不支持的代理 scheme 显式抛出 INVALID_ARGUMENT,或把 config schema 描述改为仅支持 socks://;若要支持 HTTP 代理需接入相应的 ProxyAgent。
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/chaitin__safeline-waf/src/safeline-waf.js
Line: 471
Comment:
HTTP(S) 代理配置被静默忽略并回退直连
config.schema.json 把 proxy_url 描述为 HTTP/SOCKS 代理,但这里只用 /^socks.../ 过滤,http:// 或 https:// 的代理地址会被置空并走直连且不报错。用户按 schema 文档配置 HTTP 代理时,请求会绕过代理直接连接 SafeLine,在只能经代理访问的私有部署中表现为连接失败或从错误网络出口访问。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
- 新增 GetProfile / GetOverview / SearchDetectLog / GetESIndices 只读方法 - src 增加 socks5 代理支持(socks-proxy-agent) - config 增加 proxy_url / proxyUrl / skipTlsVerify 支持
…Value
toValue returned undefined for null/undefined fields, so responses carrying
an empty err or msg serialized to an empty google.protobuf.Value and the
Connect RPC failed with "none of the oneof fields is set", hanging the smoke
test. Encode null as { nullValue: 'NULL_VALUE' } (matching safeline-waf-login)
and keep array elements instead of dropping them.
Also only route socks:// proxy URLs through SocksProxyAgent; the smoke harness
synthesizes the mock upstream as an http base URL that is not a proxy.
84e6d6c to
54ed43b
Compare
Summary
GetProfile/GetOverview/SearchDetectLog/GetESIndices只读方法src增加 socks5 代理支持(socks-proxy-agent)config增加proxy_url/proxyUrl/skipTlsVerify支持Test plan
node scripts/validate-service-package.mjs --service-dir chaitin__safeline-wafnode --test单测(mock 上游)