fix(cli): reuse base URL sanitizer for provider warnings #8524
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -171,41 +171,117 @@ export function sanitizeProviderBaseUrl(baseUrl: string): string { | |||||
| const stripAt = (at: number) => | ||||||
| `${baseUrl.slice(0, authorityStart)}${baseUrl.slice(at + 1)}`; | ||||||
| const authorityEnd = findAuthorityEnd(baseUrl, authorityStart); | ||||||
| const authoritySlice = baseUrl.slice(authorityStart, authorityEnd); | ||||||
| const authorityAt = authoritySlice.lastIndexOf('@'); | ||||||
| const authorityAtIndex = | ||||||
| authorityAt === -1 ? -1 : authorityStart + authorityAt; | ||||||
|
|
||||||
| try { | ||||||
| const parsed = new URL(baseUrl); | ||||||
| if (!(parsed.username || parsed.password)) { | ||||||
| return baseUrl; | ||||||
| } | ||||||
| if (authorityAtIndex >= authorityStart) { | ||||||
| return stripAt(authorityAtIndex); | ||||||
| } | ||||||
| if (shouldExtendUserInfoSearch(authoritySlice, parsed)) { | ||||||
| const userInfoAt = findExtendedUserInfoAt(baseUrl, authorityEnd); | ||||||
| if (userInfoAt !== -1) { | ||||||
| return stripAt(userInfoAt); | ||||||
| } | ||||||
| } | ||||||
| return baseUrl; | ||||||
| } catch { | ||||||
| if (authorityAtIndex >= authorityStart) { | ||||||
| return stripAt(authorityAtIndex); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| const fallbackAt = findUnescapedUserInfoFallbackAt( | ||||||
| baseUrl, | ||||||
| authorityStart, | ||||||
| authorityEnd, | ||||||
| ); | ||||||
|
Comment on lines
+200
to
+204
Collaborator
There was a problem hiding this comment. [Critical] R2-2: The promoted fallback's unbounded Failure scenario (A/B-probed end-to-end through the status provider): warning Suggested fix: bound the fallback's search — strip at the first 中文说明被提升为公共路径的 fallback 中无界的 失败场景（已通过 status provider 端到端 A/B 探针验证）：warning 建议修复：限定 fallback 的搜索范围 —— 在 — qwen3.8-max via Qwen Code /review (v0.21.6) |
||||||
| return fallbackAt === -1 ? baseUrl : stripAt(fallbackAt); | ||||||
| } | ||||||
|
|
||||||
| function shouldExtendUserInfoSearch( | ||||||
| authoritySlice: string, | ||||||
| parsed: URL, | ||||||
| ): boolean { | ||||||
|
Comment on lines
+208
to
+211
Collaborator
There was a problem hiding this comment. [Critical] R5-2: [certifies-falsely] [regression] Over-strip: prose after a URL keeps being read as userinfo, so warnings are rewritten to the contact email's domain — an unbounded surface this family has exposed in every round since round 1. Warning-level A/B at the exact surface this diff changed: Witness: The fix must keep multi-word credentials stripping — 中文说明过度截取：URL 之后的散文仍被当作用户信息，warning 被改写为联系邮箱的域名 —— 该家族自第 1 轮起每轮都暴露新的入口，是无界表面。 在本 diff 改动的表面上做 warning 级 A/B： 修复必须保持多词凭据继续被剥离 —— — qwen3.8-max via Qwen Code /review (v0.22.3) |
||||||
| if (parsed.password) { | ||||||
| return true; | ||||||
| } | ||||||
|
Comment on lines
+212
to
+214
Collaborator
There was a problem hiding this comment. [Suggestion] R4-3: The Still stands from round 4. Mutation probe at this head: deleting Witness: 中文说明
第 4 轮的结论仍然成立。本 head 变异探针：从早退守卫中删除 — qwen3.8-max via Qwen Code /review (v0.22.3) |
||||||
| if (authoritySlice.includes(':')) { | ||||||
| return true; | ||||||
| } | ||||||
| // A dotted token without ':' is a complete host; trailing prose may follow. | ||||||
| return !(authoritySlice.includes('.') && !authoritySlice.includes(':')); | ||||||
|
Collaborator
There was a problem hiding this comment. [Suggestion] R5-4: The A maintainer investigating colon-bearing authorities (a truncated Witness:
Suggested change
中文说明最终返回里的 — qwen3.8-max via Qwen Code /review (v0.22.3) |
||||||
| } | ||||||
|
|
||||||
| function findExtendedUserInfoAt(baseUrl: string, authorityEnd: number): number { | ||||||
| const terminator = baseUrl.charAt(authorityEnd); | ||||||
| if (terminator === '/' || terminator === '?' || terminator === '#') { | ||||||
| return findLastAtAfter(baseUrl, authorityEnd); | ||||||
| } | ||||||
| return findHostAtBeforePathOrProse(baseUrl, authorityEnd); | ||||||
| } | ||||||
|
|
||||||
| function findHostAtBeforePathOrProse( | ||||||
| baseUrl: string, | ||||||
| authorityEnd: number, | ||||||
| ): number { | ||||||
| const pathStart = findPathDelimiterStart(baseUrl, authorityEnd); | ||||||
|
Collaborator
There was a problem hiding this comment. [Suggestion] R5-5: A maintainer reusing the helper as its name and sibling suggest — from a real authority start, e.g. when de-duplicating the delimiter block per R5-3 — gets back the authority terminator itself instead of the path start, so any span computed from that "end" collapses and credential stripping silently misbehaves or no-ops. Rename the parameter to Witness: 中文说明
— qwen3.8-max via Qwen Code /review (v0.22.3) |
||||||
| let proseEnd = baseUrl.length; | ||||||
| for (let i = authorityEnd + 1; i < baseUrl.length; i++) { | ||||||
| if ( | ||||||
| /\s/.test(baseUrl.charAt(i)) && | ||||||
| baseUrl.indexOf('@', authorityEnd) < i | ||||||
|
Comment on lines
+238
to
+239
Collaborator
There was a problem hiding this comment. [Suggestion] R5-6: A warning segment whose post-authority region holds k whitespace characters before the first const firstAtAfterAuthority = baseUrl.indexOf('@', authorityEnd);
for (let i = authorityEnd + 1; i < baseUrl.length; i++) {
if (/\s/.test(baseUrl.charAt(i)) && firstAtAfterAuthority < i) {
proseEnd = i;
break;
}
}Witness: 中文说明本循环中的 — qwen3.8-max via Qwen Code /review (v0.22.3) |
||||||
| ) { | ||||||
| proseEnd = i; | ||||||
| break; | ||||||
| } | ||||||
| } | ||||||
| const searchEnd = Math.min(pathStart, proseEnd); | ||||||
| return findLastAtBefore(baseUrl, authorityEnd, searchEnd); | ||||||
| } | ||||||
|
|
||||||
| function findPathDelimiterStart( | ||||||
| baseUrl: string, | ||||||
| authorityStart: number, | ||||||
| ): number { | ||||||
|
Comment on lines
+249
to
+252
Collaborator
There was a problem hiding this comment. [Suggestion] R5-3: A future authority-terminator change applied to one copy only makes function findAuthorityEnd(baseUrl: string, authorityStart: number): number {
const end = findPathDelimiterStart(baseUrl, authorityStart);
for (let i = authorityStart; i < end; i++) {
if (/\s/.test(baseUrl.charAt(i))) return i;
}
return end;
}Witness: The whitespace difference is deliberate and must survive the dedup: 中文说明
将来若只修改其中一份的 authority 终止符， 该空白差异是有意的，去重时必须保留： — qwen3.8-max via Qwen Code /review (v0.22.3) |
||||||
| const slash = baseUrl.indexOf('/', authorityStart); | ||||||
| const query = baseUrl.indexOf('?', authorityStart); | ||||||
| const hash = baseUrl.indexOf('#', authorityStart); | ||||||
| let end = baseUrl.length; | ||||||
| if (slash !== -1) end = Math.min(end, slash); | ||||||
| if (query !== -1) end = Math.min(end, query); | ||||||
| if (hash !== -1) end = Math.min(end, hash); | ||||||
| return end; | ||||||
| } | ||||||
|
|
||||||
| function findLastAtBefore(baseUrl: string, start: number, end: number): number { | ||||||
| const at = baseUrl.slice(start, end).lastIndexOf('@'); | ||||||
| return at === -1 ? -1 : start + at; | ||||||
| } | ||||||
|
|
||||||
| function findLastAtAfter(baseUrl: string, from: number): number { | ||||||
| let last = -1; | ||||||
| for (let i = from; i < baseUrl.length; i++) { | ||||||
| if (baseUrl.charAt(i) === '@') { | ||||||
| last = i; | ||||||
| } | ||||||
| } | ||||||
| return last; | ||||||
| } | ||||||
|
|
||||||
| function findUnescapedUserInfoFallbackAt( | ||||||
| baseUrl: string, | ||||||
| authorityStart: number, | ||||||
| authorityEnd: number, | ||||||
| ): number { | ||||||
| const at = findLastAtAfter(baseUrl, authorityEnd); | ||||||
| if (at === -1) { | ||||||
| return -1; | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -226,6 +302,14 @@ function findAuthorityEnd(baseUrl: string, authorityStart: number): number { | |||||
| if (slash !== -1) end = Math.min(end, slash); | ||||||
| if (query !== -1) end = Math.min(end, query); | ||||||
| if (hash !== -1) end = Math.min(end, hash); | ||||||
| // Raw whitespace is never valid in a URL authority. Treat it as a terminator | ||||||
| // so a pathless URL followed by prose (e.g. a contact email) does not pull | ||||||
| // the trailing text into the authority span. | ||||||
| for (let i = authorityStart; i < end; i++) { | ||||||
| if (/\s/.test(baseUrl.charAt(i))) { | ||||||
| return i; | ||||||
|
Comment on lines
+308
to
+310
Collaborator
There was a problem hiding this comment. [Critical] R3-6: This new whitespace terminator truncates 中文说明这个新增的空白终止符会截断 — qwen3.8-max via Qwen Code /review (v0.21.7) |
||||||
| } | ||||||
| } | ||||||
|
Comment on lines
+308
to
+312
Collaborator
There was a problem hiding this comment. [Critical] R5-1: [certifies-falsely] [regression] Credential leak: four probe-confirmed shapes share one root — the whitespace authority terminator truncates the authority span mid-userinfo, and every branch that re-derives the userinfo boundary afterwards fails on a different corner, so credentials survive sanitization. The probe matrix at this head, all against the merge base: (a) the in-span Witness: // Direction — in the parse-success arm, when credentials are proven, strip at the
// last '@' before the first '/ ? #' after authorityStart (WHATWG's own userinfo
// separator) instead of the truncated-span heuristics; on the catch arm, evaluate
// the guards against that same whitespace-agnostic extent.The fix must keep this diff's own prose-keep pins byte-identical — 中文说明凭据泄露：四个经探针确认的形态同根 —— 空白 authority 终止符把 authority 范围从 userinfo 中间截断，之后每个重新推导 userinfo 边界的分支各漏一角，凭据残留在“已清理”的输出里。 探针矩阵（本 head 对 merge base）：(a) span 内 修复必须保持本 PR 自己固定的“保留散文”用例逐字节不变 —— — qwen3.8-max via Qwen Code /review (v0.22.3) |
||||||
| return end; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] R2-3: The stripping side of the new fall-through is unpinned by tests. In the added row
'https://user:pass@host.io please contact admin@corp.io'the credential@sits inside the whitespace-truncated span, so the strip happens in the ordinary try-branch without ever reaching the fall-through;'https://api.example — contact admin@example.com'reaches the fall-through only to exercise its no-op bailout. (The pre-existing row'https://user:p ass@api.example/v1'does pin the path-bearing space-password strip — the pathless variant is what is missing.)Concrete cost: the documented recovery case in its pathless form (
'https://user:pa ss@host.io'→'https://host.io', confirmed working by probe) is pinned by nothing, and none of the R2-1/R2-2 broken shapes were ever put in front of the suite — the whole regression family ships green (58/58 pass while probes show wrong outputs for the unpinned shapes).Suggested fix: add rows such as
['https://user:pa ss@host.io', 'https://host.io'](pins the documented recovery) plus the R2-1/R2-2 shapes with their intended stripped/preserved expectations.中文说明
新 fall-through 的"执行剥离"一侧没有测试固定。新增用例
'https://user:pass@host.io please contact admin@corp.io'的凭据@位于空白截断范围内，剥离发生在普通 try 分支，根本不会到达 fall-through；'https://api.example — contact admin@example.com'到达 fall-through 只是触发其"原样返回"的退出分支。（已有用例'https://user:p ass@api.example/v1'确实固定了带路径的空白密码剥离 —— 缺的是无路径变体。）具体代价：注释中记录的恢复用例的无路径形态（
'https://user:pa ss@host.io'→'https://host.io'，探针确认当前行为正确）没有任何测试固定；R2-1/R2-2 的损坏形态也从未进入测试套件 —— 整组回退会在测试全绿的情况下合入（58/58 通过，而探针显示这些未固定形态的输出是错误的）。建议修复：补充用例行，例如
['https://user:pa ss@host.io', 'https://host.io']（固定注释记录的恢复行为），并为 R2-1/R2-2 的形态补上期望的剥离/保留断言。— qwen3.8-max via Qwen Code /review (v0.21.6)