Repository navigation
feat(services/safeline-waf-login): add SafeLine WAF account/password service - #561
Conversation
|
PR Title: feat(services/safeline-waf-login): add SafeLine WA... Commit: 整体评估本次变更新增雷池 SafeLine WAF 账号密码登录版 OctoBus 服务,包含 5 个 gRPC 方法、内存会话管理、HTTP/SOCKS 传输、配置与密钥 schema、mock 上游及单元测试,并接入根包的 bin 与入口清单。整体结构完整,错误映射和测试覆盖设计较为清晰,但存在若干需要修复后再合入的问题。 总结整体方向合理,但配置能力与打包依赖存在缺口,部署后可能出现服务不可用或请求静默失败。最需要优先处理的是根包缺少新增运行时依赖,以及自定义 headers 未生效和 HTTP 代理不可用的问题;非代理请求的超时也未覆盖响应体读取。 Findings (4)
Prompt To Fix All With AI |
| clearTimeout(timer); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
🟡 非代理请求的超时未覆盖响应体读取
Category: 稳定性与可用性
fetchWithTimeout 在 fetch 收到响应头并返回后就在 finally 中清除定时器,而 readJsonResponse 此后才 await res.text() 读取响应体。如果上游先返回响应头、再长时间不发送响应体,配置的 timeoutMs 不会生效,RPC 会一直挂起到 undici 默认的体超时(约 300 秒),造成巡检卡死。
Problem code:
OctoBus/services/chaitin__safeline-waf-login/src/safeline-waf-login.js
Lines 232 to 245 in 53ad81b
Recommendation:
把响应体读取纳入超时中止范围,例如让 AbortController 保持到 res.text() 完成,或在 fetchWithTimeout 内部完成响应体读取。
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-login/src/safeline-waf-login.js
Line: 232-245
Comment:
非代理请求的超时未覆盖响应体读取
fetchWithTimeout 在 fetch 收到响应头并返回后就在 finally 中清除定时器,而 readJsonResponse 此后才 await res.text() 读取响应体。如果上游先返回响应头、再长时间不发送响应体,配置的 timeoutMs 不会生效,RPC 会一直挂起到 undici 默认的体超时(约 300 秒),造成巡检卡死。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| const proxiedFetch = (url, { method = 'GET', headers = {}, body } = {}, { timeoutMs, proxyUrl, skipTlsVerify } = {}) => 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.
🟡 proxy_url 配置成 HTTP 代理时会抛错导致所有请求失败
Category: 功能正确性
config.schema.json 与 README 都把 proxy_url 描述为支持 HTTP/SOCKS 代理,但实现无条件执行 new SocksProxyAgent(proxyUrl),该 Agent 只接受 socks/socks4/socks5/socks5h 协议。配置 http:// 代理时构造会抛错且未转换为 gRPC 错误,所有 RPC 都会失败。
Problem code:
Recommendation:
改用支持 HTTP 代理的 Agent(如 undici 的 ProxyAgent),或在配置 schema 与 README 中明确限定为 SOCKS,并补充 HTTP 代理用例。
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-login/src/safeline-waf-login.js
Line: 249
Comment:
proxy_url 配置成 HTTP 代理时会抛错导致所有请求失败
config.schema.json 与 README 都把 proxy_url 描述为支持 HTTP/SOCKS 代理,但实现无条件执行 new SocksProxyAgent(proxyUrl),该 Agent 只接受 socks/socks4/socks5/socks5h 协议。配置 http:// 代理时构造会抛错且未转换为 gRPC 错误,所有 RPC 都会失败。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| } | ||
|
|
||
| // 2. 登录(Django CSRF 需要 x-csrftoken 头 + csrftoken cookie + Referer 头匹配 Host) | ||
| const headers = { 'content-type': 'application/json', referer: `${baseUrl}/` }; |
There was a problem hiding this comment.
🟡 配置项 headers 被完全忽略
Category: 功能正确性
config.schema.json 和文件头注释都声明支持自定义 HTTP 头,但登录请求与只读请求各自硬编码 headers,合并后的 bindings.headers 在整个实现中从未被读取。用户按配置填写的额外请求头(例如反向代理鉴权头)会被静默丢弃,依赖这些头的部署会直接请求失败。
Problem code:
Recommendation:
在 doLogin 和 authenticatedGet 构造 headers 时合并 bindings.headers,并补充对应单元测试。
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-login/src/safeline-waf-login.js
Line: 404
Comment:
配置项 headers 被完全忽略
config.schema.json 和文件头注释都声明支持自定义 HTTP 头,但登录请求与只读请求各自硬编码 headers,合并后的 bindings.headers 在整个实现中从未被读取。用户按配置填写的额外请求头(例如反向代理鉴权头)会被静默丢弃,依赖这些头的部署会直接请求失败。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| "slack__group-robot", | ||
| "chaitin__safeline-waf-eliminate-false-positive", | ||
| "chaitin__safeline-waf", | ||
| "chaitin__safeline-waf-login", |
There was a problem hiding this comment.
🟠 根包清单未声明新增的 socks-proxy-agent 运行时依赖
Category: 稳定性与可用性
本次把新服务加入了根包的 files 与 bin,但该服务在模块顶层 import 的 'socks-proxy-agent' 只写在服务本地 package.json 中,根 services/package.json 的 dependencies 和 bundledDependencies 都没有加入该包。以仓库根包安装或打包方式运行(以及从根目录解析模块的测试)时,node_modules 中不存在该模块,服务启动即报 ERR_MODULE_NOT_FOUND,所有 RPC 不可用。
Problem code:
Line 376 in 53ad81b
Recommendation:
在 services/package.json 的 dependencies 与 bundledDependencies 中补充 socks-proxy-agent。
Prompt To Fix All With AI
This is a comment left during a code review.
Repository: https://github.com/chaitin/OctoBus
Path: services/package.json
Line: 376
Comment:
根包清单未声明新增的 socks-proxy-agent 运行时依赖
本次把新服务加入了根包的 files 与 bin,但该服务在模块顶层 import 的 'socks-proxy-agent' 只写在服务本地 package.json 中,根 services/package.json 的 dependencies 和 bundledDependencies 都没有加入该包。以仓库根包安装或打包方式运行(以及从根目录解析模块的测试)时,node_modules 中不存在该模块,服务启动即报 ERR_MODULE_NOT_FOUND,所有 RPC 不可用。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
…service - 雷池控制台无 OpenAPI 场景:Django CSRF + session 会话认证 - 方法:Login / GetProfile / GetOverview / SearchDetectLog / GetESIndices - 内存会话管理(crypto.randomUUID,TTL 30min),增加 socks5 代理支持
- export transport and error helpers via _test for unit testing - cover proxiedFetch socks5 transport with mocked http.request - cover error mapping, empty body, missing-sessionid and missing-endpoint branches
53ad81b to
e2800d8
Compare
…ate missing sessionid The smoke harness synthesizes the mock upstream as an http base URL, which was fed verbatim to SocksProxyAgent and failed every Connect call with "A socks protocol must be specified! Got: http:". Only route socks:// URLs through the agent and fall back to direct for the rest. Add a smoke fixture targeting Login (the only method that needs no prior session) and stop rejecting a login whose 200 response omits the sessionid cookie, so the generic smoke mock can complete the call. Authenticated reads still surface a missing session as PERMISSION_DENIED on their next call.
|
PR Title: feat(services/safeline-waf-login): add SafeLine WA... Commit: 整体评估本轮改动很小,是在上一轮新增 SafeLine WAF 登录服务的基础上,为跑通通用冒烟测试而调整代理 URL 解析、放宽登录 sessionid 校验并新增 Login 冒烟用例。整体结构未变,但这两处兼容性调整触及了代理配置语义与认证成功判定,需要谨慎评估。 总结本轮引入两项回归:HTTP 代理配置被静默忽略并回退直连,以及登录未拿到 sessionid 时仍返回成功;前者可能让流量绕过既定代理并导致请求失败,后者会掩盖真实的登录失败,建议合入前修复。上一轮仍未解决的 headers 配置被忽略、非代理请求超时未覆盖响应体读取两项问题也依然存在。 Findings (2)
Prompt To Fix All With AI |
| // 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.
🟠 HTTP 代理配置被静默忽略并回退直连
Category: 安全与隐私
resolveProxyUrl 现在只返回 socks 系协议,README 与 config.schema.json 明确支持的 http:// 或 https:// 代理会被解析为空字符串,fetchSafeline 随即走直连。用户配置的代理被静默绕过,既可能因网络隔离导致所有 RPC 失败,也会让本应经代理转发的登录凭据直接发往目标地址。
Problem code:
OctoBus/services/chaitin__safeline-waf-login/src/safeline-waf-login.js
Lines 163 to 168 in aa1c50e
Recommendation:
继续为 http(s) 代理提供支持(例如使用 undici 的 ProxyAgent),或在配置校验阶段对不支持的协议返回明确的配置错误,而不是静默丢弃;同时补充 http(s) 代理的单元测试。
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-login/src/safeline-waf-login.js
Line: 163-168
Comment:
HTTP 代理配置被静默忽略并回退直连
resolveProxyUrl 现在只返回 socks 系协议,README 与 config.schema.json 明确支持的 http:// 或 https:// 代理会被解析为空字符串,fetchSafeline 随即走直连。用户配置的代理被静默绕过,既可能因网络隔离导致所有 RPC 失败,也会让本应经代理转发的登录凭据直接发往目标地址。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| // A login that returns 200 with no business error is treated as success even | ||
| // when the upstream omits the sessionid cookie (e.g. the smoke harness mock). | ||
| // Authenticated reads surface the missing session on their next call, so a | ||
| // genuine login failure is still reported as PERMISSION_DENIED at read time. |
There was a problem hiding this comment.
🟡 登录未拿到 sessionid 时仍返回成功
Category: 功能正确性
doLogin 删除了 sessionid 缺失时的 PERMISSION_DENIED 校验,现在上游返回 200 但未下发 sessionid(包括空响应体命中默认值的情况)也会被当作登录成功,并把空 sessionid 写入会话。调用方会拿到看似有效却无法通过认证的 session_token,真实登录失败被推迟到后续读取才暴露,定位困难且返回语义错误。
Problem code:
OctoBus/services/chaitin__safeline-waf-login/src/safeline-waf-login.js
Lines 423 to 426 in aa1c50e
Recommendation:
保留 sessionid 为空时报 PERMISSION_DENIED 的校验;为跑通冒烟测试,应调整 smoke harness 或 mock 上游返回 sessionid,而不是放宽生产环境的登录成功判定。
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-login/src/safeline-waf-login.js
Line: 423-426
Comment:
登录未拿到 sessionid 时仍返回成功
doLogin 删除了 sessionid 缺失时的 PERMISSION_DENIED 校验,现在上游返回 200 但未下发 sessionid(包括空响应体命中默认值的情况)也会被当作登录成功,并把空 sessionid 写入会话。调用方会拿到看似有效却无法通过认证的 session_token,真实登录失败被推迟到后续读取才暴露,定位困难且返回语义错误。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary
新增雷池 SafeLine WAF 账号密码登录版只读查询 service:
Login/GetProfile/GetOverview/SearchDetectLog/GetESIndicesTest plan
node scripts/validate-service-package.mjs --service-dir chaitin__safeline-waf-loginnode --test单测(mock 上游)