Skip to content

[PR] fix(log): mask userinfo in non-http scheme URLs in logs/diagnostics/publish (#4191) - #4402

Closed
ranxianglei wants to merge 2 commits into
masterfrom
2026-10-23_log-mask-nonhttp-scheme-userinfo
Closed

[PR] fix(log): mask userinfo in non-http scheme URLs in logs/diagnostics/publish (#4191)#4402
ranxianglei wants to merge 2 commits into
masterfrom
2026-10-23_log-mask-nonhttp-scheme-userinfo

Conversation

@ranxianglei

@ranxianglei ranxianglei commented Oct 23, 2026

Copy link
Copy Markdown
Owner

What & why

maskUrlsInText, maskUrlsForPublish, maskUrlsForDiagnostics in src/log-mask.ts all shared one entry regex that only matched https?://. Any other scheme carrying inline credentials — socks5://user:pass@host (self-relay forwarding addresses), ws/wss, ftp, … — passed through verbatim into logs, diagnostic blobs, and public publish text. The single-URL maskers (maskUrlForLog etc.) already strip userinfo/query/hash via new URL for every scheme; only the entry find-regex was too narrow, so the leak was purely an entry-gate gap.

Fix

One shared const now drives all three fns:

const EMBEDDED_URL_RE = /[a-z][a-z0-9+.-]*:\/\/[^\s"'<>]+/gi;

matching any RFC-3986 scheme://… form. Extracted as a single constant (was triplicated inline) so the three surfaces can't drift apart again.

Deliberate deviation from the suggested fix

The issue proposed \b[a-z][a-z0-9+.-]*:\/\/…. I dropped the leading \b: a word-char-glued URL like xhttps://foo.com/bar must stay masked exactly as before — with \b, _/h are both word chars so no boundary forms and the match is lost → leak. Without it, coverage is a strict superset of the old pattern (over-matching a rare word://token is safe; under-matching leaks). Pinned by a regression test.

Behavior change (flagged per policy)

  • Old: non-http scheme URLs passed through unmasked in logs / diagnostics / publish blobs.
  • New: they are host-masked there too; userinfo/query/hash always dropped for every scheme.
  • The loopback exemption in maskUrlForDiagnostics is unchanged and remains per-scheme (a local ws://127.0.0.1:…/acp stays verbatim by design).

No config/env/version surface touched (pure internal regex behavior).

Tests

3 new regression tests in tests/log-mask.test.ts (socks5/ws/wss/ftp userinfo stripped; pathless non-special scheme keeps no trailing slash; word-char-glued URL still masked; diagnostics loopback-per-scheme preserved). Local targeted run: tests/log-mask.test.ts 24/24 green; npm run typecheck clean. Full-suite verdict = remote CI on this head.

Closes #4191

awork
fix(log): mask userinfo in non-http scheme URLs in logs/diagnostics/p…
5fcef58 
…ublish (#4191)

maskUrlsInText / maskUrlsForPublish / maskUrlsForDiagnostics all shared one
entry regex that only matched https?://, so socks5://user:pass@host (self-relay
forwarding addresses), ws/wss, ftp etc. carried their inline credentials
verbatim into logs, diagnostic blobs and public publish text. The single-URL
maskers already strip userinfo/query/hash via new URL for every scheme; only
the entry find-regex was too narrow.

Shared const EMBEDDED_URL_RE now matches any RFC-3986 scheme://… form in
all three fns. No leading word-boundary on purpose: a word-char-glued URL
(xhttps://…) must stay masked exactly as before - over-matching is safe,
under-matching leaks. 3 regression tests added. Behavior change: non-http
scheme URLs are now host-masked where they were previously passed through.
@ranxianglei ranxianglei mentioned this pull request Oct 23, 2026
Closed
@ranxianglei ranxianglei changed the title fix(log): mask userinfo in non-http scheme URLs in logs/diagnostics/publish (#4191) [PR] fix(log): mask userinfo in non-http scheme URLs in logs/diagnostics/publish (#4191) Oct 23, 2026
@github-actions

github-actions Bot commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-10-23_log-mask-nonhttp-scheme-userinfo (6b8a477)

Option A — Install this exact build (recommended)

npm install -g billion-context@0.0.5240-pr.4402.c9d72ba

This exact version is immutable and reproducible. The rolling pr tag tracks the newest test build across all PRs.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr4402.tgz
npm install -g package

Install instructions are refreshed on each push.

@ranxianglei

ranxianglei commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

查重:不重复。#4402 是仓库内唯一 open 的、针对源 issue #4191(日志脱敏漏非-http 协议 URL 的 userinfo)的修复 PR。相关历史项均为已关闭的不同机制,不同源:#3832(诊断 blob 掩码私有中继主机)、#1718/#1719(凭据形状扫描)、#2570(usage-vs-est 诊断绕过掩码)——都不涉及本次「入口 find-regex 只认 https?://」这一处缺口。

标签:bug + scope/other + p2(日志/诊断/publish 脱敏是自成一域的卫生问题,归不进 kernel/wiring/storage/ui/ci/docs;凭据要进公开 publish 面,需上游错误文本恰好携带带 inline 凭证的 socks5/ws URL 才触发,有绕行、无不可用,故 p2 而非 p1)。保留系统已有的 pr 标签。

这是 [PR] 镜像,分支 2026-10-23_log-mask-nonhttp-scheme-userinfo 已含完整实现(单 commit,仅动 src/log-mask.ts + tests/log-mask.test.ts)。按 PR review 规范我做独立审核(自行读 diff、判回归,不复述 issue 结论)+ 核对 CI。CI 现状:head 5fcef58 上 9 success / 11 skipped(门控 e2e 车道)/ 0 failure;且 master 自 base 起未改动本 PR 触碰的两个文件 → rebase 干净。

接下来我给八问结论,并顺手补一条 maskUrlsForPublish(公开边界)面的回归断言后推分支。

@ranxianglei ranxianglei added bug Something isn't working p2 scope/other labels Oct 23, 2026
awork
test(log-mask): pin maskUrlsForPublish on the public boundary for non…
6b8a477 
…-http schemes (#4191)

Adds the one surface the original three #4191 test blocks did not cover directly: maskUrlsForPublish. It uses different always-mask logic (isPublicApiHost, ignoring BILI_LOG_MASK_HOSTS=0), so it is pinned separately rather than relying on transitive coverage via maskUrlsInText/maskUrlsForDiagnostics.
@ranxianglei

ranxianglei commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

独立审核完成(自行读 diff、逐条判回归,未照抄 issue 结论)。结论:可合 —— 修复正确、测试有牙、前一 head CI 已绿;并顺手补了一条原本缺失的 publish 面回归测试(独立 commit 6b8a47718,已推分支)。

结构化八问

  1. 是什么问题:日志/诊断/发布三处脱敏只认 http(s) URL。其它带 inline 凭证的协议(socks5://user:pass@host 这类自中继转发地址、ws/wss/ftp)被原样写进日志、诊断 blob、以及公开 publish 文本——凭证泄露。
  2. 什么原因导致的:不是单条 URL 脱敏函数的 bug(它们早已用 new URL 对任意协议剥掉 userinfo/query/hash);是入口 find-regex 太窄——maskUrlsInText 等只匹配 https?://,非-http URL 根本没进到脱敏函数。纯入口门缺口,issue 定位准确。
  3. 影响范围与严重程度:受影响面 = 日志 + 诊断 blob + 公开 publish 文本;触发条件 = 上游错误/日志串里恰好嵌了带 inline userinfo 的非-http URL。p2(条件触发、有绕行、无不可用)。公开 publish 面是其中风险最高一环,但触发条件相同。
  4. 怎么修复的:抽出单一共享常量 EMBEDDED_URL_RE(匹配任意 RFC-3986 scheme://…)统一驱动三个函数(原先是三处各自内联的 http-only 正则)。刻意丢掉 issue 建议里的前导 \b:xhttps://foo.com/bar 这种词字符粘连的 URL 必须照旧被掩码——带 \b 会因"无边界"漏匹配而泄露。宁多掩(安全)勿漏掩(危险)。
  5. 是否有回归问题:审过的既有行为逐项——① http(s) 脱敏不变(新正则是旧模式严格超集);② 诊断面 loopback 豁免按协议保留(本就是既有行为,http 早适用,扩到 ws/socks5 一致、非新引入);③ 无可用性回归:单条 URL 函数内部 try/catch,解析失败返回 <unparseable-url>,更宽的正则只会"多掩码"、不可能把日志路径搞崩,方向安全;④ 噪声不误伤:裸 http:// (无 host)、mailto:(无 //)正确不匹配。除本意内的掩码面扩大外,无阈值/时序/输出格式漂移。
  6. 是否破坏缓存:不涉及——纯日志脱敏正则,未触碰缓存/重试/游标/去重键。
  7. 是否改了设计方向:不涉及——架构/边界/数据流不变,仅入口正则放宽 + 去重为单一常量。两种压缩模式(plugin/proxy)均不受影响(宿主侧日志卫生,非 wire 改动)。
  8. 是否引入了新的配置:不涉及——无新 env/config/默认值;BILI_LOG_MASK_HOSTS 语义不变(仍只约束本地日志/诊断路径,publish 路径本就无条件掩码)。

我独立验证了什么(不复述 issue)

  • 自读 diff,用纯 Node 内建(逐字复制真实函数体)对每条期望输出做 WHATWG URL 实测 → 17/17 通过,含边界:pathless 非特殊协议不留尾斜杠;publish 路径在 BILI_LOG_MASK_HOSTS=0 下仍无条件掩码。
  • 回归清点即上第 5 问。
  • 测试有效性(质量关):断言有牙(精确字符串 + 负例)。发现一处覆盖缺口:原有三个测试块覆盖 InText×2 + Diagnostics×1,唯独没直接测 maskUrlsForPublish——它走的是不同的"无条件掩码"逻辑(直接调 isPublicApiHost,忽略 opt-out),不能靠另两个函数间接覆盖。

顺手小修(独立标注 commit)

test(log-mask): pin maskUrlsForPublish on the public boundary for non-http schemes (#4191) —— 仅 1 个 import + 1 个测试块(+12 行),fast-forward 推到 PR 分支,head 现为 6b8a47718。

行为变更披露(按政策必报)

  • 旧:非-http 协议 URL 在日志/诊断/publish 中不掩码、原样透传。
  • 新:这些场景同样做 host 掩码;所有协议的 userinfo/query/hash 一律剥除。
  • loopback 豁免不变、按协议生效(bili 本地代理的 ws/socks5 地址仍逐字保留,符合设计)。
    此为本次修复的本意,特此标明。

CI

前 head 5fcef58 绿(9 success / 11 skipped 门控 e2e / 0 fail);新 head 6b8a47718(我只加 import+测试)CI 已重跑、当前 queued/in-progress,全量判定以远程 CI 为准。改动纯增量且已本地独立验证。

判定:可合。 代码正确、diff 干净(每行都服务于 #4191)、引用源 issue、无 kernel/config/version/wire 变更。待 6b8a47718 CI 转绿即可合并。

@ranxianglei ranxianglei mentioned this pull request Oct 24, 2026
Merged
@ranxianglei

ranxianglei commented Oct 24, 2026

Copy link
Copy Markdown
Owner Author

(ai: zhipuai-lb/glm-5.3)

已并入合并列车 #4525(master 6d7c587,合并方式 merge commit,提交保留原作者署名)。本 PR 关单收尾;若列车中对应提交出问题,回滚点为 #4525 单提交 revert。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug Something isn't working p2 scope/other

Projects

None yet

Milestone

No milestone

1 participant

@ranxianglei