Repository navigation
feat(services/cloudwalker): add socks5 proxy support - #559
Conversation
|
PR Title: refactor(services/cloudwalker): rename to cloudwal... Commit: 整体评估本次变更将 CloudWalker 服务重命名为 cloudwalker-cnapp，并同步更新入口、服务描述与注册清单，同时为配置新增代理和跳过 TLS 校验能力。重命名与注册链路整体一致，新增代理请求逻辑存在需要在合入前处理的风险。 总结重命名部分是自洽的，可以保留；但新增代理能力有三个问题需修复：配置声明的 HTTP 代理实际不可用、skipTlsVerify 在直连场景不生效、代理响应流错误会导致请求挂起或进程异常。建议修复后再合入。 Findings (3)
Prompt To Fix All With AI |
![monkeyscan[bot]](https://avatars.githubusercontent.com/u/231811742?s=60&v=4){kind=small}
eb1aaa7 to
30f88f9
Compare
|
PR Title: refactor(services/cloudwalker): rename to cloudwal... Commit: 整体评估本次变更在 chaitin__cloudwalker 服务中引入代理与跳过 TLS 校验的配置项，并新增一条基于 socks-proxy-agent 的请求路径。整体实现方向合理，但新增传输路径在配置语义和异常处理上仍需完善。 总结当前实现存在必须修复的功能与稳定性问题，最关键是 skipTlsVerify 仅在配置代理时才生效，非代理场景会静默失效；建议修复相关项后再合入。 Findings (3)
Prompt To Fix All With AI |
Daemonceltics
changed the title
| text: async () => bodyText, | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🟡 代理请求缺少响应流错误处理，连接中断时可能挂起或触发未处理异常
Category: 稳定性与可用性
proxiedFetch 只监听 res 的 data 和 end，没有监听 error/aborted；当响应体传输过程中连接被重置时 end 不会触发，Promise 既不 resolve 也不 reject，还可能在 IncomingMessage 上产生未处理的 error 事件。此时 socket 已断开，req.setTimeout 也不会再触发，调用方可能无限期挂起或导致进程异常。
Problem code:
OctoBus/services/chaitin__cloudwalker/src/cloudwalker.js
Lines 402 to 419 in 30f88f9
Recommendation:
为 res 增加 error/aborted 处理并 reject 该 Promise，同时在异常路径清理请求，并考虑用整体超时（如 AbortSignal）替代仅基于 socket 空闲的 setTimeout。
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/chaitin__cloudwalker/src/cloudwalker.js
Line: 402-419
Comment:
代理请求缺少响应流错误处理，连接中断时可能挂起或触发未处理异常
proxiedFetch 只监听 res 的 data 和 end，没有监听 error/aborted；当响应体传输过程中连接被重置时 end 不会触发，Promise 既不 resolve 也不 reject，还可能在 IncomingMessage 上产生未处理的 error 事件。此时 socket 已断开，req.setTimeout 也不会再触发，调用方可能无限期挂起或导致进程异常。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| headers, | ||
| redirect: 'manual', | ||
| signal: AbortSignal.timeout(30000), | ||
| }); |
There was a problem hiding this comment.
🟡 skipTlsVerify 在未配置代理时完全不生效
Category: 功能正确性
cloudwalker.js 的 get() 只在 proxyUrl 分支把 skipTlsVerify 传给 proxiedFetch，非代理分支（第 479 行）仍直接调用 this.fetchImpl，没有任何跳过证书校验的参数。用户按 config.schema.json 为自签名部署开启 skipTlsVerify 但没有配置代理时，请求仍会因证书校验失败而报错，该配置被静默忽略。
Problem code:
OctoBus/services/chaitin__cloudwalker/src/cloudwalker.js
Lines 477 to 484 in 30f88f9
Recommendation:
让 skipTlsVerify 对代理和非代理两条路径都生效（例如非代理路径改用可自定义 TLS Agent 的 undici/native 请求），或在 schema 与启动校验中明确要求必须同时配置代理。
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/chaitin__cloudwalker/src/cloudwalker.js
Line: 477-484
Comment:
skipTlsVerify 在未配置代理时完全不生效
cloudwalker.js 的 get() 只在 proxyUrl 分支把 skipTlsVerify 传给 proxiedFetch，非代理分支（第 479 行）仍直接调用 this.fetchImpl，没有任何跳过证书校验的参数。用户按 config.schema.json 为自签名部署开启 skipTlsVerify 但没有配置代理时，请求仍会因证书校验失败而报错，该配置被静默忽略。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
PR Title: feat(services/cloudwalker): add socks5 proxy suppo... Commit: 整体评估本轮延续上一轮的 socks5 代理支持，主要为代理传输与内部 helper 补充单元测试，并新增 总结本轮测试可以验证正常代理请求和错误码映射，但未覆盖真实超时回调与响应流中断，稳定性保障有限。上一轮指出的三个问题仍需在合入前处理：skipTlsVerify 在未配置代理时不生效、proxiedFetch 缺少响应流错误处理可能导致请求挂起、配置声明支持 HTTP 代理但实现仅支持 SOCKS。 Findings (2)
Prompt To Fix All With AI |
- route requests through an optional socks5h proxy via socks-proxy-agent - add skipTlsVerify config for self-signed deployments - accept proxy_url / proxyUrl bindings
- exercise proxiedFetch via mocked http request (success, network error, timeout) - unit-test toBoolean/normalize*/isJsonContentType/binding resolvers through _test
4eb8db4 to
22be7a0
Compare
|
PR Title: feat(services/cloudwalker): add socks5 proxy suppo... Commit: 整体评估本次变更给 CloudWalker 客户端新增 SOCKS 代理传输、skipTlsVerify 开关以及对应的配置 schema 和测试，整体方向合理，但代理实现的功能边界与异常处理仍不完整，尚不具备安全合入的条件。 总结代理与自签名部署相关的配置存在多处会导致功能失效或请求挂起的问题：skipTlsVerify 在未配置代理时完全不生效，配置声明的 HTTP 代理实际不受支持，代理路径的超时与响应流异常也可能使调用无限挂起。建议合入前修复这些关键项。 Findings (4)
Prompt To Fix All With AI |
| const proxiedFetch = (url, { headers = {} } = {}, { proxyUrl, skipTlsVerify, timeoutMs = 30000 } = {}) => new Promise((resolve, reject) => { | ||
| const target = new URL(url); | ||
| const mod = target.protocol === 'https:' ? https : http; | ||
| const agent = new SocksProxyAgent(proxyUrl); |
There was a problem hiding this comment.
🟠 配置声明支持 HTTP 代理，但实现只支持 SOCKS 代理
Category: 功能正确性
config.schema.json 将 proxy_url 描述为支持 HTTP/SOCKS 代理，但 proxiedFetch 对所有 proxyUrl 一律构造 SocksProxyAgent，而 socks-proxy-agent 只接受 socks、socks4、socks5、socks5h 协议。用户按文档填写 http:// 或 https:// 代理时会在构造阶段抛错，并被包装成笼统的 upstream unreachable，代理功能实际不可用。
Problem code:
OctoBus/services/chaitin__cloudwalker/src/cloudwalker.js
Lines 392 to 395 in 22be7a0
Recommendation:
根据代理 URL 的协议选择传输：socks 协议用 SocksProxyAgent，http/https 代理用 HttpProxyAgent/HttpsProxyAgent；或者修正 schema 描述只声明支持 SOCKS。
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/chaitin__cloudwalker/src/cloudwalker.js
Line: 392-395
Comment:
配置声明支持 HTTP 代理，但实现只支持 SOCKS 代理
config.schema.json 将 proxy_url 描述为支持 HTTP/SOCKS 代理，但 proxiedFetch 对所有 proxyUrl 一律构造 SocksProxyAgent，而 socks-proxy-agent 只接受 socks、socks4、socks5、socks5h 协议。用户按文档填写 http:// 或 https:// 代理时会在构造阶段抛错，并被包装成笼统的 upstream unreachable，代理功能实际不可用。
---
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 只为响应流注册了 data 和 end 处理器，没有监听 error/aborted/close；当响应体传输过程中连接被重置时 end 不会触发，Promise 既不 resolve 也不 reject，IncomingMessage 上的 error 事件还可能因无监听而抛出未处理异常。调用方会无限挂起或导致进程异常。
Problem code:
OctoBus/services/chaitin__cloudwalker/src/cloudwalker.js
Lines 402 to 419 in 22be7a0
Recommendation:
同时监听 res 的 error、aborted 和 close 事件，在未正常结束时 reject，并在 settle 后清理请求/定时器。
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/chaitin__cloudwalker/src/cloudwalker.js
Line: 402-419
Comment:
代理响应流缺少错误/中断处理，连接中断时请求会挂起或抛出未处理异常
proxiedFetch 只为响应流注册了 data 和 end 处理器，没有监听 error/aborted/close；当响应体传输过程中连接被重置时 end 不会触发，Promise 既不 resolve 也不 reject，IncomingMessage 上的 error 事件还可能因无监听而抛出未处理异常。调用方会无限挂起或导致进程异常。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| }); | ||
| }); | ||
|
|
||
| req.setTimeout(timeoutMs, () => req.destroy(Object.assign(new Error('timeout'), { name: 'TimeoutError', code: 'ETIMEDOUT' }))); |
There was a problem hiding this comment.
🟡 代理请求的 30 秒超时只覆盖已连接 socket 的空闲时间，无法限制整次请求
Category: 稳定性与可用性
proxiedFetch 使用 req.setTimeout(30000, ...) 实现超时，但 Node 的 request.setTimeout 只有在 socket 已分配并连接后才设置空闲超时，既不覆盖 SocksProxyAgent 的 SOCKS 建连阶段，也会被持续到达的数据不断重置。代理不可达或响应缓慢时请求可能远超 30 秒甚至无限挂起，而非代理路径的 AbortSignal.timeout(30000) 是整次请求的硬性截止时间。
Problem code:
Recommendation:
在 Promise 创建时启动一个独立的整体截止定时器（超时后调用 req.destroy() 并清理），并确保建连阶段也受该定时器约束；请求结束或失败时 clearTimeout。
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/chaitin__cloudwalker/src/cloudwalker.js
Line: 421
Comment:
代理请求的 30 秒超时只覆盖已连接 socket 的空闲时间，无法限制整次请求
proxiedFetch 使用 req.setTimeout(30000, ...) 实现超时，但 Node 的 request.setTimeout 只有在 socket 已分配并连接后才设置空闲超时，既不覆盖 SocksProxyAgent 的 SOCKS 建连阶段，也会被持续到达的数据不断重置。代理不可达或响应缓慢时请求可能远超 30 秒甚至无限挂起，而非代理路径的 AbortSignal.timeout(30000) 是整次请求的硬性截止时间。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| headers, | ||
| redirect: 'manual', | ||
| signal: AbortSignal.timeout(30000), | ||
| }); |
There was a problem hiding this comment.
🟠 未配置代理时 skipTlsVerify 完全不生效
Category: 功能正确性
新增的 skipTlsVerify 配置只在 proxyUrl 分支传给 proxiedFetch，非代理分支仍然原样调用 this.fetchImpl，没有把该选项传给 fetch 的 TLS 层。用户在自签名部署中按 config.schema.json 开启 skipTlsVerify 但未配置代理时，请求仍会因证书校验失败而报错，该选项被静默忽略。
Problem code:
OctoBus/services/chaitin__cloudwalker/src/cloudwalker.js
Lines 477 to 484 in 22be7a0
Recommendation:
非代理路径使用带 connect: { rejectUnauthorized: false } 的 undici Agent 作为 dispatcher，或在统一的自定义传输层中同时应用该选项。
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/chaitin__cloudwalker/src/cloudwalker.js
Line: 477-484
Comment:
未配置代理时 skipTlsVerify 完全不生效
新增的 skipTlsVerify 配置只在 proxyUrl 分支传给 proxiedFetch，非代理分支仍然原样调用 this.fetchImpl，没有把该选项传给 fetch 的 TLS 层。用户在自签名部署中按 config.schema.json 开启 skipTlsVerify 但未配置代理时，请求仍会因证书校验失败而报错，该选项被静默忽略。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
…ksProxyAgent resolveProxyUrl returned any proxy_url value verbatim, so the smoke harness synthesizing the mock upstream as an http base URL fed SocksProxyAgent a non-socks URL and every Connect call failed with "A socks protocol must be specified! Got: http:". Fall back to a direct connection for non-socks URLs.
|
PR Title: feat(services/cloudwalker): add socks5 proxy suppo... Commit: 整体评估本次变更继续完善 CloudWalker 客户端的 SOCKS 代理支持，修复了非 SOCKS 代理地址导致的调用失败，并顺带补充了代理路径的测试覆盖。整体方向与上轮一致，但代理与 TLS 相关配置的处理仍不完整，尚不具备安全合入的条件。 总结上轮指出的代理请求超时、响应流中断处理、skipTlsVerify 在未配置代理时不生效等问题本轮未涉及，仍然存在；本轮新增的核心风险是不受支持的代理地址被静默丢弃并退化为直连。合入前应让不支持的代理协议显式报错（或真正支持 HTTP 代理），而不是静默绕过代理。 Findings (1)
Prompt To Fix All With AI |
| const raw = String(bindings.proxyUrl ?? bindings.proxy_url ?? '').trim(); | ||
| // 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. | ||
| return /^socks(4a?|5h?)?:\/\//i.test(raw) ? raw : ''; |
There was a problem hiding this comment.
🟠 非 SOCKS 代理地址被静默丢弃，请求会绕过代理直连
Category: 安全与隐私
resolveProxyUrl 现在把任何非 socks 协议的 proxy_url（包括 config.schema.json 仍声明支持的 http/https 代理）直接返回空字符串，调用方据此走直连分支且不会收到任何错误。用户按文档配置 HTTP 代理时流量会绕过代理直接发出，既可能违反网络出口策略，也让代理配置错误完全无法被发现。
Problem code:
OctoBus/services/chaitin__cloudwalker/src/cloudwalker.js
Lines 805 to 809 in 9daed46
Recommendation:
对不支持的代理协议抛出明确的配置错误（或实现 HTTP 代理支持），并同步修正 config.schema.json 中 HTTP/SOCKS 的描述，避免静默降级为直连。
| return /^socks(4a?|5h?)?:\/\//i.test(raw) ? raw : ''; | |
| const resolveProxyUrl = (bindings = {}) => { | |
| const raw = String(bindings.proxyUrl ?? bindings.proxy_url ?? '').trim(); | |
| if (!raw) return ''; | |
| if (!/^socks(4a?|5h?)?:\/\//i.test(raw)) { | |
| throw new Error(`Unsupported proxy URL scheme: ${raw}; only socks:// proxies are supported`); | |
| } | |
| return raw; | |
| }; |
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/chaitin__cloudwalker/src/cloudwalker.js
Line: 805-809
Comment:
非 SOCKS 代理地址被静默丢弃，请求会绕过代理直连
resolveProxyUrl 现在把任何非 socks 协议的 proxy_url（包括 config.schema.json 仍声明支持的 http/https 代理）直接返回空字符串，调用方据此走直连分支且不会收到任何错误。用户按文档配置 HTTP 代理时流量会绕过代理直接发出，既可能违反网络出口策略，也让代理配置错误完全无法被发现。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Add optional socks5 proxy support to the CloudWalker service.
No directory or service-id rename.