Repository navigation
fix(core): redact credential-bearing error text at the RUM sink - #11424
bluefateludi wants to merge 13 commits into
Conversation
|
Review follow-up, pushed in
New tests pin the two behaviours: a control character wedged between The masking-vs-fingerprinting policy question remains open for a maintainer, as the triage intended. |
…y value Review round 2 on QwenLM#11424, addressing the six Critical findings plus the two new ones from the latest pass: - R1-1: bound both key runs at {0,64} — the cubic-backtracking DoS on dash-dense input drops from 632.9s to ~26ms at the scheduler gate. - R1-2: strip ANSI/VT sequences first (newlines preserved) and drop the \b anchors so an escape sequence or word char before a key can no longer silently disable a mask. - R1-3: cover properties.error — hook error text carrying the full hook command line was uploaded neither masked nor capped. - R1-4: the Authorization/bearer value position now skips the scheme or label word instead of masking it, so token/Basic/ApiKey/Digest values and the 'bearer token: <jwt>' prose form are masked. - R1-18/R1-19: the value alternative is a whitespace-bounded optionally- unterminated quoted run or a bare run that never starts with a dash, so an empty value cannot eat the next flag's name and a broken quote cannot void the mask. - R2-1: neutralise control chars with a space instead of deleting them — deletion fused key and value into one token or glued a word onto the key, regressing masks that worked before. - R2-2: skip a shell line-continuation marker before the value so masks bind to the credential on the continuation line. - R1-5 (minimal): register the secrets the process holds (content generator API key, MCP server header values) and mask them by exact value — closed by construction; the design doc drops the class-closure claim and records the pattern-enumeration residual gap in Non-goals.
|
Thanks for the two review rounds — all seven open Criticals plus the two new ones from the latest pass are fixed in 3c9d095, along with the surrogate-test Suggestion (R2-3). Plan and rationale per finding: R1-1 (backtracking DoS) — Both key runs in every pattern are now bounded at R1-2 (ANSI + R1-3 ( R1-4 (scheme-word swallowing) — The Authorization value position now skips an optional scheme word ( R1-18 (leading-dash value) / R1-19 (unterminated quote) — Solved together by one shared value alternative: R2-1 (delete → neutralise) — Control chars are now replaced with a space, not deleted, per the measured fix: R2-2 (line continuation) — The value-group prefix R1-5 (class-level) — minimal value-masking, as discussed — Process-held secrets (the content-generator API key and MCP server header values) are now masked by exact value before the pattern passes: R2-3 (surrogate test) — Rewritten to the astral form ( Also fixed while porting: the space-separated flag mask is now restricted to Verification: 261/261 across |
3c9d095 to
a63390a
Compare
…y value Review round 2 on QwenLM#11424, addressing the six Critical findings plus the two new ones from the latest pass: - R1-1: bound both key runs at {0,64} — the cubic-backtracking DoS on dash-dense input drops from 632.9s to ~26ms at the scheduler gate. - R1-2: strip ANSI/VT sequences first (newlines preserved) and drop the \b anchors so an escape sequence or word char before a key can no longer silently disable a mask. - R1-3: cover properties.error — hook error text carrying the full hook command line was uploaded neither masked nor capped. - R1-4: the Authorization/bearer value position now skips the scheme or label word instead of masking it, so token/Basic/ApiKey/Digest values and the 'bearer token: <jwt>' prose form are masked. - R1-18/R1-19: the value alternative is a whitespace-bounded optionally- unterminated quoted run or a bare run that never starts with a dash, so an empty value cannot eat the next flag's name and a broken quote cannot void the mask. - R2-1: neutralise control chars with a space instead of deleting them — deletion fused key and value into one token or glued a word onto the key, regressing masks that worked before. - R2-2: skip a shell line-continuation marker before the value so masks bind to the credential on the continuation line. - R1-5 (minimal): register the secrets the process holds (content generator API key, MCP server header values) and mask them by exact value — closed by construction; the design doc drops the class-closure claim and records the pattern-enumeration residual gap in Non-goals.
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
|
The
A re-run of the failed job, or any future rebase, should go green against current main. Per the no-rebase note above I'm leaving the branch as-is; happy to rebase if a maintainer prefers. 中文说明
重跑失败的 job,或将来任何 rebase,对当前 main 都应当变绿。遵照上方"请勿 rebase"的提醒,分支保持不动;如 maintainer 希望rebase,我可以随时执行。 |
Tool error messages carry raw shell command lines, which reached the usage-statistics endpoint unredacted through error_message and message fields. Apply a sink-side pass in enqueueLogEvent, the choke point all log methods share, using a new redactErrorText helper that masks URL userinfo, Authorization headers, bearer tokens, and secret-looking flag/env assignments, then truncates to the same bound the OTel span path uses. Closes QwenLM#11198
…error redaction Address review findings on the first round: the truncation cap and its surrogate-pair guard are now the exported truncateErrorText from session-tracing, used by both the OTel span path and the RUM sink pass instead of a copied constant; and control characters are stripped before masking (newlines preserved, so the multi-line error block survives) so a C0 char cannot split a secret key from its separator and defeat a mask. The design doc now states the parity claim accurately and records the two free-text surfaces the choke point does not reach.
…y value Review round 2 on QwenLM#11424, addressing the six Critical findings plus the two new ones from the latest pass: - R1-1: bound both key runs at {0,64} — the cubic-backtracking DoS on dash-dense input drops from 632.9s to ~26ms at the scheduler gate. - R1-2: strip ANSI/VT sequences first (newlines preserved) and drop the \b anchors so an escape sequence or word char before a key can no longer silently disable a mask. - R1-3: cover properties.error — hook error text carrying the full hook command line was uploaded neither masked nor capped. - R1-4: the Authorization/bearer value position now skips the scheme or label word instead of masking it, so token/Basic/ApiKey/Digest values and the 'bearer token: <jwt>' prose form are masked. - R1-18/R1-19: the value alternative is a whitespace-bounded optionally- unterminated quoted run or a bare run that never starts with a dash, so an empty value cannot eat the next flag's name and a broken quote cannot void the mask. - R2-1: neutralise control chars with a space instead of deleting them — deletion fused key and value into one token or glued a word onto the key, regressing masks that worked before. - R2-2: skip a shell line-continuation marker before the value so masks bind to the credential on the continuation line. - R1-5 (minimal): register the secrets the process holds (content generator API key, MCP server header values) and mask them by exact value — closed by construction; the design doc drops the class-closure claim and records the pattern-enumeration residual gap in Non-goals.
a63390a to
2a2ec70
Compare
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at 2a2ec70. The regex hardening from the earlier rounds is real and I confirmed the bounded key runs, the value guard, the control-character stripping and the continuation prefix. One new Critical, and one policy question that still needs settling before this can land.
[Critical] registerProcessSecrets dereferences a getter that is undefined before auth initialization, and the resulting TypeError silently drops the event. packages/core/src/telemetry/qwen-logger/qwen-logger.ts:145-148:
function registerProcessSecrets(config: Config | undefined): void {
const secrets: Array<string | undefined> = [
config?.getContentGeneratorConfig().apiKey,
];The optional chain guards config, not the getter's result. getContentGeneratorConfig is (packages/core/src/config/config.ts:4942-4947):
getContentGeneratorConfig(): ContentGeneratorConfig {
return (
getRuntimeContentGenerator()?.contentGeneratorConfig ??
this.contentGeneratorConfig
);
}The backing field is declared private contentGeneratorConfig!: ContentGeneratorConfig (config.ts:2225) and assigned only in refreshAuth, while getRuntimeContentGenerator() reads an AsyncLocalStorage store that is empty at startup. The declared return type is non-nullable but the codebase does not believe it — every other call site guards with ?. (config.ts:4198, 4963, 8623). The call is inside enqueueLogEvent's try block (qwen-logger.ts:231) ahead of this.events.push(event) at :241, and the catch at :248-250 only debug-logs, so the throw costs the whole event. This is reachable on the default startup path, where session-start telemetry is emitted before auth is validated. It is not a leak — it is silent loss of session_start. CI does not catch it because the test mock always returns an object.
[Suggestion] Registering every MCP header value as a secret over-redacts. A server configured with Accept: application/json, text/event-stream makes the exact-value pass replace application/json throughout all error text, since the length floor is only 8 characters. That degrades diagnosability for no security gain; restrict this to auth-ish header names.
[Suggestion] Simplicity. knownSecretValues is module-global mutable state repopulated on every event, cleared only by a test-only export that ships in production. The leak-proof half of this feature is achievable by passing the secrets into redactErrorText as a parameter with no module state at all. As it stands the PR adds a second mechanism beyond what was asked.
[Nit] The idempotence claim does not hold for truncation, since the truncating helper appends a marker and a second pass re-truncates. And RumExceptionEvent.caused_by is a free-text surface that is neither in the key list nor recorded as a known limitation, while stack was covered defensively.
Previously raised, still standing: the CPU half of the round-1 performance concern — all passes run before truncation, and ToolCallEvent.error is unbounded at its producer. And the spelling-enumeration gap: ?key= / ?access_token= in URLs and JSON-shaped "api_key":"…" remain unmasked because every flag pattern requires a leading dash. Several other suggestions from those rounds are also unaddressed.
Decision needed from us, not the author. This PR defers the masking-versus-fingerprinting policy question, and it should not land undecided: either we accept that spelling-based patterns are best-effort and say so in the code, or we commit to the exact-value approach and drop the pattern layer. I would rather we choose than merge both.
Verification gaps: no tests run, so every finding here is read-verified only. The regex behaviour is static reasoning — I did not empirically confirm the ?key= and JSON misses, nor measure backtracking cost. I could not enumerate every producer of RumExceptionEvent.message and stack outside this file, which is the main reason I am not treating the redaction surface as closed. I verified the pre-auth ordering on the non-interactive path only.
|
Thanks — the Critical is real and is fixed in 16c4f99 (pushed). [Critical] pre-auth
[Nit] truncation idempotence. You're right that a second pass would re-truncate ( [Nit] [Suggestion] MCP header over-redaction. Kept register-all-headers this round by design: header values are secrets-by-convention (they ride on every request to that server), and we'd rather over-mask than under-mask at a telemetry sink. But the [Suggestion] module-global Still standing (CPU cost; Masking vs fingerprinting. Agreed it should be decided before this lands. Our recommendation: keep both layers and state in the code that the spelling-based patterns are best-effort — they cover disjoint failure modes (exact values catch our own credentials in any spelling; patterns catch credential-shaped text from external error output we hold no copy of). The doc comment on |
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is running in workflow run. A command-triggered review is not listed under the checks of this PR; the result is posted here as a review when it finishes. |
|
@qwen-code /review |
Pre-bound the mask passes at a working multiple of the shared cap so the output limit bounds the CPU too (dash-dense hostile error text was quadratic in the URL pass), neutralise VT runs instead of deleting them so a colour sequence between a flag and its value cannot fuse the token, unescape two-character JSON escapes before the masks so stringified producer output is masked in one text shape, prefer fully closed quoted runs so a quoted secret containing spaces is masked whole, let the Authorization scheme word and the bearer label fold across one newline, and mask registered secrets on the raw input so an incomplete escape sequence cannot consume the secret's first character. Register the credential half of scheme-prefixed MCP header values and fold second-session configs into the add-only secret registry, and narrow the design doc's exact-value claims to the registered set with the remaining carries recorded as an open residual gap.
|
Round 3 addressed — 4f4ac86 merged main, ec72635 closes the standing Criticals. Verified each mechanism against HEAD first (the round-3 review anchored at 2a2ec70, one commit before 16c4f99, so R3-10's pre-auth getter finding was already fixed there and is not re-done). Fixed in ec72635
Every fix has a red/green-verified test: each mutation (pre-bound removed, VT replace removed, JSON unescape removed, closed-quote branch removed, either fold removed, raw-input mask removed, envelope split removed, else-arm registration removed) was applied and confirmed to redden its test before being restored. Doc corrections (the consolidated pass R3-22 asked for): the "closed by construction / no shape at all" sentences are narrowed to the registered set; a named residual-gap list now records what the registry does NOT read (Qwen OAuth dynamic tokens, Not adopted this round — needs a maintainer decision, recorded in the doc
Deferred as Suggestion-level (per the ~5-round convergence posture; each is a behaviour change with a measured trade-off, none certifies falsely): R3-2 (raw-shell vs JSON modelling of One process note: the round-3 bot review anchored at 2a2ec70 while the PR head was already 16c4f99 — worth a re-run at ec72635 so the next round doesn't re-derive fixed findings. |
doudouOUC
left a comment
There was a problem hiding this comment.
Agent-assisted review at ec7263522b223a6e3f3f6377076ff1782584674e — one confirmed regression; residual privacy-policy decision remains. Comment only; no approval implied.
Critical — JSON-escaped registered credentials bypass the relocated exact-value pass
packages/core/src/telemetry/sanitize.ts:222-242 now performs its only maskKnownSecretValues pass BEFORE JSON unescaping. A registered MCP header credential containing a quote/backslash therefore does not occur literally in a JSON-stringified error; normalization subsequently reconstructs it, after the only value-based protection has finished. The spelling masks do not catch an ordinary prose echo. This is not the documented exclusion for unregistered credentials or unknown key spellings.
Verified synthetic example: register SYNTHETIC"SECRET_12345, then redact JSON.stringify([{text: 'upstream echoed SYNTHETIC"SECRET_12345'}]). The returned text still contains the entire registered credential. Real path: header values are registered at qwen-logger/qwen-logger.ts:155-168; non-image MCP failures JSON-stringify response parts at tools/mcp-tool.ts:870-883; telemetry/types.ts:225 → loggers.ts:173-192,302-319 → qwen-logger/qwen-logger.ts:606-628 carries that string to properties.error_message and the RUM queue. Header values are forwarded without a restriction excluding quotes (tools/mcp-client.ts:2516-2518). Usage statistics default on (config/config.ts:2813). Keep the raw-value pass needed for R3-22, but also protect the normalized representation before upload; add a registered-value regression exercising this actual JSON producer shape.
Historical Critical reassessment
- Fixed: our previous pre-auth crash / R3-1 (
getContentGeneratorConfig()?.apiKey, logger:153); hookproperties.erroromission / R1-3 (:133); secondary Config registration / R3-26 (:233-244); scheme-prefixed MCP credential registration / R3-27 (:164-165). - Earlier flag/value, unclosed-quote, auth-label, control-fusion, ANSI, continuation, escaped-quote, quoted-space and folded-header witnesses (R1-18/19/4/2, R2-1/2, R3-19/20/23/24) are addressed by the current patterns/normalization and passed isolated representative probes. R3-22's raw incomplete-CSI witness is fixed, but its reordered pass introduces the regression above.
- R1-1's unbounded-input work is bounded at 64 KiB before masking (:222), with bounded key runs. This is not a claim of linear regex runtime: synthetic dash-dense inputs took approximately 156 ms (28k chars) and 636 ms (1.4M input, prebounded).
- R1-5 and R3-18 are not behavioral closures: unregistered JSON token bodies,
curl -u, query tokens and the invisible-character Authorization witness still leak in helper probes. The design explicitly records these limitations (lines 26-36,71-82,137-143), satisfying the historical documentation/narrowed-guarantee alternative, not providing a fail-closed privacy boundary. Maintainers still own acceptance of that policy; I am not multiplying documented misses into new regex blockers. OAuth/custom-header/env/clientSecret registry gaps, payloadbase_url, and the unchanged narrower OTel redactor remain limitations, not certified safe surfaces.
Coverage / validation
Read all six changed files/changed functions and tests, the design, actual shell/MCP → ToolCallEvent → normalization → RUM queue/payload consumers, registration and truncation paths, and paginated review history. Non-test source diff is below the 500-line gate even before excluding comments; external-author fix, not a large refactor. No daemon route changes. Existing noncritical deferrals remain deferred; no new Suggestions.
Executed only an isolated scratch probe of the already-read pure helpers under Linux / Node v22.23.1 (stripTypeScriptTypes + VM), using synthetic values. No application imports, dependency installation, repository tests/build, provider requests, or RUM uploads. No end-to-end telemetry claim.
|
Thanks @doudouOUC — confirmed and fixed in d6165f5. The only Regression added in On the residual policy items (R1-5/R3-18): understood and agreed they remain documented limitations of the narrowed guarantee, as the design records — no new regex blockers from our side either. |
|
@qwen-code /resolve |
Merge origin/main into codex/issue-11198-redact-rum-errors. Main landed QwenLM#11649, which replaces the RUM enqueue boundary's `message`, `error_message`, and `error_excerpt` with a fixed marker outright. This branch masks only credential-bearing shapes in those same fields plus `stack` and `properties.error`. Both sides edited the one choke point, so the calls collided. Keep both passes rather than picking a side: the blanket one runs first and owns the three fields it replaces, then the targeted one covers what it leaves raw. Neither side's coverage is lost, and the stricter base-branch policy is not weakened. `redactErrorText` is a no-op on the marker, so the order is not output-load-bearing and only avoids re-scanning text that is about to be discarded.
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge resolution — PR #11424Root cause — duplicate fix, not adjacent edits. Semantic — same fields, incompatible policies. main is blanket ( this.redactEventErrorText(event); // blanket (#11649)
registerProcessSecrets(this.config);
redactErrorTextFields(event); // targeted (#11424)No coverage lost; the base branch's stricter policy is not weakened. Load-bearing.
Could not verify. No build/tests run. 中文说明根因:重复修复,非相邻改动。 main 的 语义冲突:同一批字段,策略不兼容。 main 全量覆盖( 关键约束: 不要调换顺序或删任一调用——当前输出一致仅因 未能验证: 未跑构建与测试。 |
|
The resolve run's one open item — the two superseded tests — is closed in a8e9e52 (test-only, on top of 44cedda). Exactly the two the resolution note predicted went red at the resolved head (
Mutations re-verified at the new head: removing the targeted pair ( The merge-resolution ordering itself (blanket first, |
The merge of main re-introduced loggers.test.ts configs that stub only the telemetry-relevant getters; registerProcessSecrets called getContentGeneratorConfig unguarded and threw, failing 29 tests and — because the throw happens inside enqueueLogEvent's catch-all path at getInstance time — masking it would also drop the event. Call both getters with optional chaining so partial Config objects register no generator/MCP secrets instead of crashing.
|
The ubuntu Fixed in 8a49a6b: both Verified locally: |
|
The two ubuntu Both failing tests ( No branch-side change can legitimately alter that outcome, so I'm leaving the code untouched here — this looks like a main-side flake or an ink patch interaction that will need to be addressed upstream of this PR. Happy to re-run or rebase again if main lands a fix. |
|
Follow-up with hard evidence on the two ubuntu I reproduced the same two failures locally with the ink patch correctly applied (the CI run log shows Same-base cross-check: #11842 / #11844 / #11845 share main merge-base e04f2ec with runs interleaved with ours and passed the identical lane. Filed #11850 upstream with the mechanism and suggested deterministic refill hooks. Nothing branch-side to change here. |
What this PR does
Every event the usage-statistics channel queues for the RUM endpoint now passes a redaction step at the single choke point all log methods share, before it is queued for upload. The pass masks credential-bearing shapes in error text: URL userinfo (reusing the existing URL redactor),
Authorizationheaders including their bearer prefixes, bare bearer tokens, and secret-looking flag or environment assignments such as--token=…,--api-key=…, orAWS_SECRET_ACCESS_KEY=…. After masking, control characters are stripped (newlines preserved, so the multi-line error block survives) so a control character cannot split a secret key from its separator and defeat a mask; the text is then truncated by the same shared helper the OpenTelemetry span path uses, bound and surrogate-pair guard alike, so both telemetry sinks apply one definition of the truncation bound. Non-error fields (model names, durations, counts, error type codes) are untouched. Two free-text surfaces the choke point does not reach (snapshotsand the payload-levelbase_url) are recorded as known limits in the design doc; neither carries a shell command line today.The redaction policy is pattern masking rather than fingerprinting, and that choice is recorded in a design doc alongside the code: masked text stays debuggable in the RUM feed, and any shape the mask misses is bounded by the truncation cap. A fingerprinted feed (tool name + exit code + hash) cannot leak by construction but loses the diagnostic value that makes the feed worth having. If maintainers prefer the fingerprint direction, the sink-side pass makes that swap a one-function change.
Why it's needed
Closes #11198. The usage-statistics channel is on by default and uploads tool error text verbatim. A failed shell command's error message starts with the full command line, so a failed
git clone https://x-access-token:…@github.com/…, acurlwith anAuthorizationheader, or a connection string with embedded database credentials is uploaded with the secret inline — no opt-in step, no redaction anywhere on the path. The same sink ships raw error text through the API-error, invalid-chunk, auth, and ripgrep-fallback events as well. The OpenTelemetry span path already redacts its copy of the same error text; this PR brings the RUM path to parity.Fixing this per call site is how the gap kept reopening — a recent PR added a second unredacted error field on this sink and review caught it. A pass at the choke point closes the class: future call sites inherit the redaction because every event funnels through it.
One decision from the issue's triage needs maintainer confirmation: the redaction policy. This PR implements masking; the design doc records why and what the alternative would trade away.
Reviewer Test Plan
How to verify
From the repository root, run the telemetry unit tests:
Expected: all pass, including the new cases. The sanitize tests pin each masked shape — the exact leaked example from the issue (
git clone https://x-access-token:ghs_abc@github.com/o/r), a database DSN, anAuthorizationheader inside acurl -Hargument, secret flags with both=and space separators, and secret env assignments — plus two negative cases proving ordinary text (error: connection refused for host db:5432,--verbose=2) passes through unchanged. The logger tests enqueue an event whose error text carries credentials and assert the queued copy is masked, and that a non-error event's properties are untouched.A broader regression check:
Expected: 30 files, 1011 tests, all passing.
To see the behaviour without the tests: build core, import the redaction helper from the built output, and pass it any failing shell command's error block — the command line comes back with credentials replaced by
***and the rest of the diagnostic text intact.Evidence (Before & After)
Non-UI change; no screenshots. Before: the chain verified in the issue's triage shows raw command lines reaching the RUM payload fields (
message,error_message) with no redaction anywhere on the path. After: the same fields, enqueued through the choke point, containhttps://***REDACTED***@github.com/o/rand"Authorization: ***"while preserving the surrounding diagnostic text.Tested on
Environment (optional)
Windows 11, Node.js 22, unit tests only. Full repo
npm run buildandnpm run typecheckclean.Risk & Scope
Linked Issues
Closes #11198
中文说明
本 PR 做了什么
usage-statistics 通道发往 RUM 端点的每个事件,现在都会在所有日志方法共用的唯一汇聚点入队前经过一步脱敏。该步骤掩码错误文本中携带凭据的形态:URL userinfo(复用现有 URL 脱敏器)、
Authorization头(含 bearer 前缀)、裸 bearer token,以及形如--token=…、--api-key=…、AWS_SECRET_ACCESS_KEY=…的 secret 形态 flag / 环境变量赋值。掩码后按 OpenTelemetry span 路径已在用的长度上限截断,两个遥测出口应用同一份"安全错误文本"定义。非错误字段(模型名、时长、计数、错误类型码)不受影响。脱敏策略选择的是模式掩码而非指纹化,这一取舍连同理由记录在代码旁的设计文档中:掩码后的文本在 RUM 流里仍可调试,掩码漏掉的形态被截断上限约束。指纹化(工具名 + 退出码 + 哈希)在构造上不可能泄露,但会失去这条数据流存在的诊断价值。若维护者更倾向指纹化方向,sink 侧的这层结构使替换只是改一个函数。
为什么需要
Closes #11198。usage-statistics 通道默认开启,且原样上传工具错误文本。shell 命令失败时的错误消息以完整命令行开头,因此失败的
git clone https://x-access-token:…@github.com/…、带Authorization头的curl、或内嵌数据库凭据的连接串,都会把 secret 原样上传——无需任何 opt-in,路径上没有任何脱敏。同一出口还通过 API 错误、invalid-chunk、auth、ripgrep-fallback 事件上传原始错误文本。OpenTelemetry span 路径对同一份错误文本早已脱敏;本 PR 让 RUM 路径与之对齐。逐调用点修复正是这个缺口反复重开的原因——最近一个 PR 在同一出口新增了第二个未脱敏的错误字段,被 review 抓住。在汇聚点做一次统一处理闭掉的是整类问题:未来的调用点自动继承脱敏,因为所有事件都经过这里。
issue triage 中有一个决定需要维护者确认:脱敏策略。本 PR 实现的是掩码;设计文档记录了理由以及备选方案的代价。
Reviewer Test Plan
如何验证
在仓库根目录运行遥测单测:
预期:全部通过,包括新增用例。sanitize 测试逐形态钉死:issue 中的原始泄露样例(
git clone https://x-access-token:ghs_abc@github.com/o/r)、数据库 DSN、curl -H参数里的Authorization头、=与空格两种分隔的 secret flag、secret 环境变量赋值;另有两个反向用例证明普通文本(error: connection refused for host db:5432、--verbose=2)原样通过。logger 测试入队一个错误文本携带凭据的事件,断言排队副本已掩码,且非错误事件的属性不受影响。更大范围的回归检查:
预期:30 个文件、1011 个测试全部通过。
不看测试直接观察行为:构建 core 后从产物导入脱敏函数,传入任一失败 shell 命令的错误块——命令行返回时凭据已替换为
***,其余诊断文本保留。证据(前后对比)
非 UI 改动,无截图。改前:issue triage 核实的链路显示原始命令行直达 RUM 载荷字段(
message、error_message),路径上无任何脱敏。改后:同样字段经汇聚点入队后内容为https://***REDACTED***@github.com/o/r与"Authorization: ***",周围诊断文本保留。测试环境
环境(可选)
Windows 11,Node.js 22,仅单元测试。全仓
npm run build与npm run typecheck干净。风险与范围
关联 Issue
Closes #11198