Skip to content

fix(core): shared redactors leave URL userinfo credentials intact #4593

Description

@yihanzhu

What happened

redactSecrets in packages/core/src/redaction.ts and the display redactor in packages/core/src/display-redaction.ts both handle URL query secrets (?token=…), but neither touches the URL userinfo section. A credential written as https://user:secret@host/… passes through both unchanged unless the secret itself happens to match one of the fixed token-prefix patterns.

Measured against the built @maka/core at eacfb46aa:

input redactSecrets (core) display redactor
https://myuser:glpat-AbCdEf12345XyZ@gitlab.com/team/repo.git unchanged unchanged
https://alice:hunter2@internal.example.com/repo.git unchanged unchanged
https://alice:ATBBxyz123abc456@bitbucket.org/team/repo.git unchanged unchanged
fatal: unable to access https://deploy:s3cretP@ss@git.corp.example/x.git/: 403 unchanged unchanged
control: https://api.example.com/v1?token=abc123 ?token=[redacted] ?token=<redacted>
control: https://ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789@github.com/o/r.git https://[redacted]@github.com/o/r.git https://<redacted>@github.com/o/r.git

The last row shows that today's only coverage is incidental: a GitHub PAT is caught because ghp_ is in SECRET_PATTERNS, not because the userinfo position is recognised. GitLab glpat-, Bitbucket app passwords and plain user:password are not.

Where it lands

git remote -v, git config --list and push/fetch error text are ordinary Bash output. packages/runtime/src/shell-run-manager.ts:953 redacts the command through this function, and packages/runtime/src/pty-screen-collector.ts:472,497 uses redactSecrets(line) !== line as the whole-line suppression test — so a credential-bearing remote URL is neither masked nor suppressed on either path.

The gap has already been worked around once, locally: apps/desktop/src/renderer/settings/provider-endpoint-presentation.ts:128-146 (#3639) clears parsed.username / parsed.password for the Settings display, with a comment noting that the shared redactor only sweeps "whatever remains".

Proposed fix

Add a structural userinfo step to both redactors, ahead of the query-secret step: for each https?:// run, if the authority (up to the first /, ? or #) contains @, replace everything from the authority start through the last @ with the redaction marker. Structural, so it needs no provider prefix list; and it removes only the credentials — host and path survive, unlike the assigned-key-value fallback, which swallows the rest of the URL.

For display-redaction.ts, the new PATTERNS entry must declare streamingTerminator and streamingValueGroup like its neighbours so the streaming-suffix redactors keep their contract.

Validation

  • packages/core/src/__tests__/redaction.test.ts: the four leaking rows above, plus https://user@host/… (no password) and a URL embedded in prose with trailing punctuation ((fetch), ., )), asserting host and path survive.
  • The same cases against the display redactor, including the stable/reversible streaming-suffix paths.

Precedent for single-gap redaction fixes: #295, #2042.

Environment

  • Commit: eacfb46aa
  • OS: macOS
  • Surface: Runtime Host / @maka/core
  • Node.js: 26.5.0
中文

packages/core 里的两个共享脱敏函数都处理 URL 的 query 参数(?token=…),但都不处理 URL 的 userinfo 段。https://用户名:密码@host/… 形式的凭据会原样穿过,除非密码本身碰巧命中某个固定的 token 前缀(如 ghp_)。GitLab 的 glpat-、Bitbucket app password、普通 user:password 都不在覆盖范围内。

git remote -vgit config --list、push/fetch 的报错文本都是普通的 Bash 输出,会经过 shell-run-manager.ts:953pty-screen-collector.ts:472,497 这两条路径,两者都依赖同一个函数,所以带凭据的远程 URL 既不会被掩码也不会被整行抑制。

#3639 已经在设置页的显示层本地绕过了一次同一个缺口,代码注释里写明共享脱敏函数"只清扫剩下的部分"。

建议加一步结构性的 userinfo 处理:对每段 https?://,若 authority 含 @,把 authority 起点到最后一个 @ 之间替换为脱敏标记。不依赖厂商前缀,且只删凭据、保留 host 和 path。

AI assistance disclosure: measurements and drafting assisted by Claude; reviewed by me.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions