fix(auth): strip controls from masked API keys by lin-hongkuan · Pull Request #96445 · openclaw/openclaw

lin-hongkuan · 2026-06-24T13:30:53Z

What Problem This Solves

maskApiKey() trims credential-like values before redacting them, but embedded control characters were preserved in the masked diagnostic output. A copied value containing a newline or NUL could therefore make masked auth/setup text multiline or carry invisible control bytes even though the secret itself was redacted.

Why This Change Was Made

The masker now strips C0/C1/DEL control characters before applying the existing length-based redaction rules. This keeps the same visible masking format for printable values while making diagnostic output single-line/control-free.

A focused regression test covers newline, NUL, and all-control input.

User Impact

Onboarding, auth overview, and directive responses that display masked credentials no longer echo hidden control characters from malformed input.

Evidence

Direct behavior probe:

$ node --import tsx -e "import('./src/utils/mask-api-key.ts').then(({ maskApiKey }) => console.log(JSON.stringify({ newline: maskApiKey('abcd\nefghijklmnop'), nul: maskApiKey('abcd\u0000efghijklmnop'), delC1: maskApiKey('abcd\u007f\u0085efghijklmnop'), emptyControls: maskApiKey('\u0000\n\u0085') })))"
{"newline":"ab...op","nul":"ab...op","delC1":"ab...op","emptyControls":"missing"}

Targeted test:

$ node scripts/run-vitest.mjs run src/utils/mask-api-key.test.ts

 RUN  v4.1.8 C:/Users/lin20/Documents/New project 3/openclaw


 Test Files  1 passed (1)
      Tests  4 passed (4)
   Start at  21:29:22
   Duration  351ms (transform 30ms, setup 0ms, import 54ms, tests 5ms, environment 0ms)

[test] starting test/vitest/vitest.unit-fast.config.ts
[test] passed 1 Vitest shard in 7.09s

Formatting:

$ node_modules\.bin\oxfmt.cmd --check src/utils/mask-api-key.ts src/utils/mask-api-key.test.ts
Checking formatting...

All matched files use the correct format.
Finished in 1246ms on 2 files using 32 threads.

Whitespace:

$ git diff --check

AI-assisted

Prepared with Codex. I reviewed the change, understand the touched code path, and kept the PR focused on the bug described above.

lin-hongkuan · 2026-06-24T13:39:31Z

@clawsweeper re-review

clawsweeper · 2026-06-24T13:39:34Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

clawsweeper · 2026-06-24T13:47:02Z

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 5:16 PM ET / 21:16 UTC.

Summary
The PR strips C0/C1/DEL control characters from API-key diagnostic masking before applying the existing redaction rules and adds focused regression coverage.

PR surface: Source +12, Tests +7. Total +19 across 2 files.

Reproducibility: yes. from source inspection: current main trims only at the edges, and the shared display callers then slice the same string into diagnostic labels. I did not run tests locally because this review is read-only.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

none.

Risk before merge

[P1] Live PR status still had queued checks after the latest no-op retrigger commit during review; this is an ordinary merge-gate wait, not a code repair need.

Maintainer options:

Decide the mitigation before merge
Land the focused masker and regression test after normal maintainer merge gates, while keeping raw secret input normalization in the adjacent companion PR.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No repair lane is needed; the patch is correct and the remaining action is normal maintainer review plus merge-gate completion.

Security
Cleared: No security or supply-chain regression found; the diff tightens an in-repo credential masking helper and adds a colocated unit test.

Review details

Best possible solution:

Land the focused masker and regression test after normal maintainer merge gates, while keeping raw secret input normalization in the adjacent companion PR.

Do we have a high-confidence way to reproduce the issue?

Yes from source inspection: current main trims only at the edges, and the shared display callers then slice the same string into diagnostic labels. I did not run tests locally because this review is read-only.

Is this the best way to solve the issue?

Yes. Changing the shared diagnostic masker is the narrowest maintainable fix for the display problem; raw secret input normalization remains a separate boundary handled by #96444.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 13ecca5408cb.

Label changes

Label justifications:

P2: This fixes a real auth diagnostic-output bug with limited blast radius and no new config, migration, or product surface.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes copied terminal output from a direct after-fix behavior probe plus the targeted test run, which is sufficient for this non-visual utility change.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied terminal output from a direct after-fix behavior probe plus the targeted test run, which is sufficient for this non-visual utility change.

Evidence reviewed

PR surface:

Source +12, Tests +7. Total +19 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	13	1	+12
Tests	1	7	0	+7
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	20	1	+19

What I checked:

Repository policy read: Root review policy was read fully and applied because this PR touches auth diagnostics and requires caller, sibling, proof, and history checks. (AGENTS.md:1, 13ecca5408cb)
Maintainer notes check: The only maintainer note present was Telegram-specific, which does not match this auth utility change.
Current main behavior: Current main still calls value.trim() directly before slicing, so embedded controls inside a credential-like value can remain in masked output. (src/utils/mask-api-key.ts:3, 13ecca5408cb)
PR implementation: The patch changes maskApiKey() to strip controls before trimming and adds newline, NUL, DEL/C1, and all-control regression cases. (src/utils/mask-api-key.ts:3, 6a0f9845eaac)
Shared caller surface: Model auth overview, auth directives, and gateway setup confirmations all flow credential display through maskApiKey(), so the helper is the right shared display boundary. (src/commands/models/list.auth-overview.ts:23, 13ecca5408cb)
Adjacent sibling boundary: normalizeSecretInput() is a raw secret input-normalization boundary and remains separate from diagnostic masking; the companion PR at fix(secrets): strip control characters from secret input #96444 does not supersede this one. (src/utils/normalize-secret-input.ts:20, 13ecca5408cb)

Likely related people:

rosenlo: git blame attributes the current shared masker implementation and auth display callers to commit f9e0dce. (role: introduced behavior; confidence: high; commits: f9e0dce73109; files: src/utils/mask-api-key.ts, src/utils/mask-api-key.test.ts, src/commands/models/list.auth-overview.ts)
Ayaan Zaidi: git show --format=fuller lists Ayaan Zaidi as committer for the commit that introduced the current shared masker and auth display caller files in this checkout. (role: committer; confidence: medium; commits: f9e0dce73109; files: src/utils/mask-api-key.ts, src/commands/models/list.auth-overview.ts, src/auto-reply/reply/directive-handling.auth.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

* fix(auth): strip controls from masked API keys * test(auth): cover DEL and C1 masked key controls * chore: retrigger PR checks --------- Co-authored-by: lin-hongkuan <lin-hongkuan@users.noreply.github.com> (cherry picked from commit 2906d6c)

* fix(auth): strip controls from masked API keys * test(auth): cover DEL and C1 masked key controls * chore: retrigger PR checks --------- Co-authored-by: lin-hongkuan <lin-hongkuan@users.noreply.github.com>

* fix(auth): strip controls from masked API keys * test(auth): cover DEL and C1 masked key controls * chore: retrigger PR checks --------- Co-authored-by: lin-hongkuan <lin-hongkuan@users.noreply.github.com> (cherry picked from commit 2906d6c)

fix(auth): strip controls from masked API keys

37c1a43

openclaw-barnacle Bot added the size: XS label Jun 24, 2026

lin-hongkuan and others added 2 commits June 24, 2026 21:59

test(auth): cover DEL and C1 masked key controls

53bf03c

chore: retrigger PR checks

6a0f984

clawsweeper Bot mentioned this pull request Jun 26, 2026

fix(secrets): strip control characters from secret input #96444

Merged

vincentkoc merged commit 2906d6c into openclaw:main Jun 29, 2026
93 checks passed

github-actions Bot mentioned this pull request Jun 29, 2026

📡 Upstream Digest — 2026-06-29 19:45 UTC curtismercier/openclaw-mods#1175

Open

ooiuuii mentioned this pull request Jun 30, 2026

Masked secret display can preserve control characters NousResearch/hermes-agent#55319

Closed

clawsweeper Bot mentioned this pull request Jul 4, 2026

refactor(security): consolidate secret primitives #99746

Merged

clawsweeper Bot mentioned this pull request Jul 12, 2026

fix(security): keep secret mask from splitting UTF-16 surrogate pairs #104800

Merged

Navigation Menu

Uh oh!

fix(auth): strip controls from masked API keys - #96445

fix(auth): strip controls from masked API keys#96445
vincentkoc merged 3 commits into
openclaw:mainfrom
lin-hongkuan:codex/mask-api-key-strip-controls

lin-hongkuan commented Jun 24, 2026 •

edited

Loading

Uh oh!

lin-hongkuan commented Jun 24, 2026

Uh oh!

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

lin-hongkuan commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

AI-assisted

Uh oh!

lin-hongkuan commented Jun 24, 2026

Uh oh!

clawsweeper Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lin-hongkuan commented Jun 24, 2026 •

edited

Loading

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading