fix: add redaction patterns for JWTs, Basic auth, and custom security headers in logs.tail by Magicray1217 · Pull Request #67041 · openclaw/openclaw

Magicray1217 · 2026-04-15T07:10:06Z

Summary

\logs.tail\ redaction missed several credential formats that could leak secrets to \operator.read\ clients.

Changes

Added redaction patterns for:

Generic JWTs — \�yJ...\ three-segment base64url tokens
Basic auth headers — \Authorization: Basic \
Custom security headers — \X-OpenClaw-Token, \x-pomerium-jwt-assertion, \X-Api-Key, \X-Auth-Token\

Tests

Added 4 test cases in
edact.test.ts\ covering all new patterns.

Fixes #66832

… headers The logs.tail redaction missed several credential formats that could leak secrets to operator.read clients: - Generic JWTs (eyJ... three-segment base64url tokens) - Basic auth headers (Authorization: Basic ...) - Custom security headers (X-OpenClaw-Token, x-pomerium-jwt-assertion, X-Api-Key, X-Auth-Token) Added patterns and tests for all four cases. Fixes openclaw#66832

greptile-apps · 2026-04-15T07:13:41Z

Greptile Summary

Adds redaction patterns for generic JWTs, Basic auth headers, and custom security headers (X-OpenClaw-Token, x-pomerium-jwt-assertion, X-Api-Key, X-Auth-Token) to the logs.tail redaction pipeline, along with four new test cases.

The Basic auth pattern ends with \\b, which conflicts with the +, /, = characters in the character class — backtracking will exclude trailing base64 padding characters (e.g. =) from the masked span.
The four new tests are nested inside describe(\"redactSensitiveLines\") but all call redactSensitiveText, and the JWT test exercises the pre-existing Bearer pattern rather than the newly added eyJ pattern.

Confidence Score: 5/5

Safe to merge; the new patterns work correctly in the common case and all remaining findings are P2.

All findings are P2. The \b boundary quirk in the Basic auth pattern only affects trailing base64 padding (not the credential itself), and the test organisation issue doesn't affect runtime behaviour. No P0 or P1 defects were found.

src/logging/redact.ts line 44 (Basic auth pattern \b); src/logging/redact.test.ts (test describe-block placement)

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/logging/redact.ts
Line: 44

Comment:
**`\b` prevents redacting trailing base64 padding**

The `\b` word-boundary at the end of this pattern causes greedy backtracking to stop before any trailing `=` or `/` characters, since those are non-word chars. For example, `dXNlcm5hbWU6cGFzc3dvcmQ=` would be matched as `dXNlcm5hbWU6cGFzc3dvcmQ` (23 chars), leaving the trailing `=` unredacted in the log output. The test passes because it only checks that the full original string is absent, not that the padding is masked too.

Since the character class `[A-Za-z0-9+/=]{18,}` already stops greedily at a non-matching character, the `\b` is not needed for boundary detection here and actively hurts correctness.

```suggestion
  String.raw`Authorization\s*[:=]\s*Basic\s+([A-Za-z0-9+/=]{18,})`,
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/logging/redact.test.ts
Line: 212-238

Comment:
**Tests placed in wrong describe block; JWT test doesn't isolate the new pattern**

All four new tests call `redactSensitiveText` but are nested inside `describe("redactSensitiveLines", ...)`. They should live in the `describe("redactSensitiveText", ...)` block above (or a new dedicated block).

Additionally, the `"masks generic JWT tokens"` test uses `Authorization: Bearer ${jwt}` as input. This string is already matched by the pre-existing Bearer pattern (`Authorization\s*[:=]\s*Bearer\s+...`), so the test never actually exercises the new `eyJ…` JWT pattern. A standalone JWT (e.g. in a log line without a `Bearer` prefix) would better isolate the new pattern:

```ts
// tests the new eyJ JWT pattern directly
const input = `x-pomerium-jwt-assertion: ${jwt}`;
// or simply
const input = `some log line: jwt=${jwt}`;
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: add redaction patterns for JWTs, Ba..." | Re-trigger Greptile}

greptile-apps · 2026-04-15T07:13:44Z

+  // Generic JWTs (three base64url segments separated by dots).
+  String.raw`\b(eyJ[A-Za-z0-9_-]{10,}\.eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,})\b`,
+  // Basic auth headers.
+  String.raw`Authorization\s*[:=]\s*Basic\s+([A-Za-z0-9+/=]{18,})\b`,


\b prevents redacting trailing base64 padding

The \b word-boundary at the end of this pattern causes greedy backtracking to stop before any trailing = or / characters, since those are non-word chars. For example, dXNlcm5hbWU6cGFzc3dvcmQ= would be matched as dXNlcm5hbWU6cGFzc3dvcmQ (23 chars), leaving the trailing = unredacted in the log output. The test passes because it only checks that the full original string is absent, not that the padding is masked too.

Since the character class [A-Za-z0-9+/=]{18,} already stops greedily at a non-matching character, the \b is not needed for boundary detection here and actively hurts correctness.

Suggested change

String.raw`Authorization\s*[:=]\s*Basic\s+([A-Za-z0-9+/=]{18,})\b`,

String.raw`Authorization\s*[:=]\s*Basic\s+([A-Za-z0-9+/=]{18,})`,

Prompt To Fix With AI

This is a comment left during a code review. Path: src/logging/redact.ts Line: 44 Comment: **`\b` prevents redacting trailing base64 padding** The `\b` word-boundary at the end of this pattern causes greedy backtracking to stop before any trailing `=` or `/` characters, since those are non-word chars. For example, `dXNlcm5hbWU6cGFzc3dvcmQ=` would be matched as `dXNlcm5hbWU6cGFzc3dvcmQ` (23 chars), leaving the trailing `=` unredacted in the log output. The test passes because it only checks that the full original string is absent, not that the padding is masked too. Since the character class `[A-Za-z0-9+/=]{18,}` already stops greedily at a non-matching character, the `\b` is not needed for boundary detection here and actively hurts correctness. ```suggestion String.raw`Authorization\s*[:=]\s*Basic\s+([A-Za-z0-9+/=]{18,})`, ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-04-15T07:13:45Z

+  it("masks generic JWT tokens", () => {
+    const jwt =
+      "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c";
+    const input = `Authorization: Bearer ${jwt}`;
+    const output = redactSensitiveText(input, { mode: "tools", patterns: defaults });
+    expect(output).not.toContain("eyJzdWIiOiIxMjM0NTY3ODkwIn0");
+  });
+
+  it("masks Basic auth headers", () => {
+    const input = "Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQ=";
+    const output = redactSensitiveText(input, { mode: "tools", patterns: defaults });
+    expect(output).not.toContain("dXNlcm5hbWU6cGFzc3dvcmQ=");
+  });
+
+  it("masks X-OpenClaw-Token headers", () => {
+    const input = 'X-OpenClaw-Token: oc_abcdef1234567890ghijklmn';
+    const output = redactSensitiveText(input, { mode: "tools", patterns: defaults });
+    expect(output).not.toContain("oc_abcdef1234567890ghijklmn");
+  });
+
+  it("masks x-pomerium-jwt-assertion headers", () => {
+    const jwt =
+      "eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ1c2VyQGV4YW1wbGUuY29tIn0.abc123def456ghi789jkl";
+    const input = `x-pomerium-jwt-assertion: ${jwt}`;
+    const output = redactSensitiveText(input, { mode: "tools", patterns: defaults });
+    expect(output).not.toContain("eyJzdWIiOiJ1c2VyQGV4YW1wbGUuY29tIn0");
+  });


Tests placed in wrong describe block; JWT test doesn't isolate the new pattern

All four new tests call redactSensitiveText but are nested inside describe("redactSensitiveLines", ...). They should live in the describe("redactSensitiveText", ...) block above (or a new dedicated block).

Additionally, the "masks generic JWT tokens" test uses Authorization: Bearer ${jwt} as input. This string is already matched by the pre-existing Bearer pattern (Authorization\s*[:=]\s*Bearer\s+...), so the test never actually exercises the new eyJ… JWT pattern. A standalone JWT (e.g. in a log line without a Bearer prefix) would better isolate the new pattern:

// tests the new eyJ JWT pattern directly const input = `x-pomerium-jwt-assertion: ${jwt}`; // or simply const input = `some log line: jwt=${jwt}`;

Prompt To Fix With AI

This is a comment left during a code review. Path: src/logging/redact.test.ts Line: 212-238 Comment: **Tests placed in wrong describe block; JWT test doesn't isolate the new pattern** All four new tests call `redactSensitiveText` but are nested inside `describe("redactSensitiveLines", ...)`. They should live in the `describe("redactSensitiveText", ...)` block above (or a new dedicated block). Additionally, the `"masks generic JWT tokens"` test uses `Authorization: Bearer ${jwt}` as input. This string is already matched by the pre-existing Bearer pattern (`Authorization\s*[:=]\s*Bearer\s+...`), so the test never actually exercises the new `eyJ…` JWT pattern. A standalone JWT (e.g. in a log line without a `Bearer` prefix) would better isolate the new pattern: ```ts // tests the new eyJ JWT pattern directly const input = `x-pomerium-jwt-assertion: ${jwt}`; // or simply const input = `some log line: jwt=${jwt}`; ``` How can I resolve this? If you propose a fix, please make it concise.

clawsweeper · 2026-04-27T04:47:16Z

Codex review: needs real behavior proof before merge.

Summary
The PR adds shared log redaction regexes and tests for JWT-shaped tokens, Basic auth headers, and selected security headers.

Reproducibility: yes. at source level with high confidence. logs.tail returns shared-redacted lines, and focused pattern probes show current main misses the reported Basic/custom-header samples while the PR still mishandles Basic credentials.

Real behavior proof
Needs real behavior proof before merge: The PR body/comments only show tests, CI, and bot review; the contributor should add redacted logs.tail or equivalent runtime terminal/log output, screenshot, recording, or linked artifact with private data redacted, then update the PR body for automatic re-review.

Next step before merge
Human or contributor follow-up is needed because this external security-sensitive PR lacks real behavior proof and has a concrete Basic auth redaction defect that should be amended before merge.

Security
Needs attention: The diff changes secret-redaction defaults, but the proposed Basic auth matcher is incomplete and can leave credentials visible.

Review findings

[P2] Redact short and padded Basic credentials — src/logging/redact.ts:44
[P3] Exercise standalone JWT redaction directly — src/logging/redact.test.ts:215-216

Review details

Best possible solution:

Amend this PR or land an equivalent narrow patch so shared defaults fully redact raw Basic auth and named security headers, preserve current JWT coverage, and prove the logs.tail path with focused tests plus runtime evidence.

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

Yes, at source level with high confidence. logs.tail returns shared-redacted lines, and focused pattern probes show current main misses the reported Basic/custom-header samples while the PR still mishandles Basic credentials.

Is this the best way to solve the issue?

No, not as written. The shared default redactor is the right layer, but the Basic auth pattern needs to cover valid short and padded Base64 credentials and the tests should isolate standalone JWT plus line/log-tail behavior.

Full review comments:

[P2] Redact short and padded Basic credentials — src/logging/redact.ts:44
The new matcher requires at least 18 Base64 characters and ends with a word boundary. The linked Authorization: Basic c2VjcmV0OnBhc3M= sample does not match, and longer padded credentials are captured without the trailing =, so returned logs can still expose auth material.
Confidence: 0.94
[P3] Exercise standalone JWT redaction directly — src/logging/redact.test.ts:215-216
This test uses Authorization: Bearer ${jwt}, which is already covered by the existing Bearer rule. Use a bare JWT or a non-Bearer header sample so the test fails if the new standalone JWT pattern is removed.
Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.92

Security concerns:

[medium] Basic auth redaction remains incomplete — src/logging/redact.ts:44
The proposed regex misses the linked short Basic credential sample and can leave trailing Base64 padding visible, weakening a log-redaction path intended to protect read-scoped clients.
Confidence: 0.94

Acceptance criteria:

node scripts/run-vitest.mjs src/logging/redact.test.ts src/logging/log-tail.test.ts src/gateway/server-methods/server-methods.test.ts
Provide redacted runtime proof from logs.tail or equivalent terminal/log output after the patch.

What I checked:

Current shared redaction gap: Current DEFAULT_REDACT_PATTERNS include quoted auth/header fields, Bearer auth, and a standalone JWT-shaped token pattern, but no raw Authorization: Basic ..., X-OpenClaw-Token, or x-pomerium-jwt-assertion header matcher. (src/logging/redact.ts:42, 2eee70e0a64b)
logs.tail uses shared redaction: readConfiguredLogTail resolves shared redaction options and returns redactSensitiveLines(result.lines, redaction) to callers. (src/logging/log-tail.ts:150, 2eee70e0a64b)
logs.tail is read-scoped: The core gateway method table assigns logs.tail to operator.read, matching the linked issue's read-scoped exposure concern. (src/gateway/methods/core-descriptors.ts:29, 2eee70e0a64b)
Focused pattern probe: Read-only probes showed current-main pattern shapes do not match the reported raw Basic auth or X-OpenClaw-Token samples, while the standalone JWT shape is already covered. (src/logging/redact.ts:54, 2eee70e0a64b)
PR Basic auth defect: The PR adds Authorization\s*[:=]\s*Basic\s+([A-Za-z0-9+/=]{18,})\b; probes showed it misses the linked short sample and captures a padded longer credential without the trailing =. (src/logging/redact.ts:44, bdf4fd937be6)
Open linked issue and PR state: Live PR metadata shows this PR is open, mergeable, maintainer-editable, and references the open linked bug with closing syntax; policy keeps the issue/PR pair open until a fix merges. (bdf4fd937be6)

Likely related people:

steipete: Recent commits touched logging redaction, support URL redaction, and the CLI/log-tail gateway surface around the affected path. (role: recent area contributor; confidence: high; commits: 306fe841f54b, a0023f4978a8, a4b97075aed3; files: src/logging/redact.ts, src/logging/redact.test.ts, src/logging/log-tail.ts)
eleqtrizit: Authored the merged change that connected shared redaction behavior to the logs.tail surface and nearby tests. (role: log-tail redaction contributor; confidence: high; commits: 851294126b30; files: src/logging/redact.ts, src/logging/redact.test.ts, src/logging/log-tail.ts)
liaoandi: Authored the merged HTTP client secret/header redaction expansion in the same shared default redaction list and tests. (role: adjacent redaction contributor; confidence: medium; commits: 21d758c644b0; files: src/logging/redact.ts, src/logging/redact.test.ts, src/infra/errors.test.ts)
pgondhi987: Recent current-main work touched shared redaction defaults and tests while expanding secret-shaped payload handling. (role: recent redaction contributor; confidence: medium; commits: 39bcd1e08834, 17ceca86d698; files: src/logging/redact.ts, src/logging/redact.test.ts, src/agents/session-tool-result-guard.ts)

Remaining risk / open question:

The proposed Basic auth matcher can still leave valid Basic credentials partially or fully visible.
The contributor has not provided after-fix real behavior proof from logs.tail or an equivalent runtime path.
The patch adds redaction defaults without focused redactSensitiveLines or logs.tail regression coverage for the affected endpoint path.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 2eee70e0a64b.

clawsweeper · 2026-04-29T16:13:48Z

Codex review: needs maintainer review before merge.

What this changes:

The PR adds default log redaction regexes and tests for generic JWTs, Basic auth headers, X-OpenClaw-Token, x-pomerium-jwt-assertion, X-Api-Key, and X-Auth-Token.

Maintainer follow-up before merge:

This is a valid open contributor PR for credential redaction, but it touches secret-leak prevention and needs maintainer review plus a small contributor or maintainer amendment rather than an autonomous replacement lane.

Review details

Best possible solution:

Keep this PR as the implementation candidate for the linked redaction bug, but amend it so the shared default patterns fully cover padded Basic auth, standalone JWTs, and the named security headers, with focused tests for redactSensitiveText, redactSensitiveLines, and the logs.tail path before merge.

Acceptance criteria:

pnpm test src/logging/redact.test.ts src/logging/log-tail.test.ts src/gateway/server-methods/server-methods.test.ts

What I checked:

Current default redaction gap: Current DEFAULT_REDACT_PATTERNS cover key/token assignments, URL query secrets, JSON fields, CLI flags, Bearer authorization, PEM blocks, common token prefixes, and Telegram bot tokens, but no default pattern for Authorization: Basic, standalone JWTs, or x-pomerium-jwt-assertion. (src/logging/redact.ts:13, acae48b790fa)
logs.tail uses shared redaction: readConfiguredLogTail reads the configured log file, resolves shared redaction options, and applies redactSensitiveLines to the returned log lines. (src/logging/log-tail.ts:150, acae48b790fa)
logs.tail is read-scoped: The gateway method scope map exposes logs.tail under READ_SCOPE, matching the report's operator.read exposure concern. (src/gateway/method-scopes.ts:69, acae48b790fa)
Existing tests do not cover requested formats: Current redaction tests cover existing token/key/Bearer/PEM behavior and redactSensitiveLines, but no Basic auth, standalone JWT, Pomerium assertion, or named custom security header regression cases. The log-tail test only mocks the redaction call-through. (src/logging/redact.test.ts:36, acae48b790fa)
Separate sanitizers are not the logs.tail default path: Support export and agent payload sanitizers already recognize Basic/JWT-like values, but those regexes live outside the shared DEFAULT_REDACT_PATTERNS path used by logs.tail. (src/logging/diagnostic-support-redaction.ts:18, acae48b790fa)
PR functional review: The provided PR diff is narrow, but the proposed Basic auth regex ends with a word boundary after a base64 character class, which can leave padding outside the masked span. The JWT test uses Authorization: Bearer, which is already covered by the pre-existing Bearer pattern rather than isolating the new standalone JWT pattern. (src/logging/redact.ts:44, bdf4fd937be6)

Likely related people:

steipete: Current-main blame and path history for the shared logging redaction defaults, log-tail implementation, log-tail tests, gateway log method handler, and method scope table all point to Peter Steinberger in the inspected local history. (role: recent maintainer and likely follow-up owner; confidence: high; commits: a972c9ec4547, be8c24633aaa; files: src/logging/redact.ts, src/logging/redact.test.ts, src/logging/log-tail.ts)

Remaining risk / open question:

Current main can still return unredacted Basic auth values, standalone JWTs, or Pomerium JWT assertion headers through shared logging/logs.tail redaction if those values reach file logs.
The PR's current Basic auth pattern can leave trailing base64 padding outside the masked span.
The PR's current tests do not isolate standalone JWT redaction and do not add a focused logs.tail/redactSensitiveLines regression for the affected endpoint path.

Codex review notes: model gpt-5.5, reasoning high; reviewed against acae48b790fa.

Fix logs.tail credential-header redaction and JSON-mode gateway transport errors.\n\nFixes #66832.\nFixes #79108.\nSupersedes #67041.\nSupersedes #79233.\n\nCo-authored-by: Mil Wang <mingjwan@microsoft.com>\nCo-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>

vincentkoc · 2026-05-16T18:06:10Z

Thanks for the focused fix. I landed a repaired equivalent on main in #82690 / e06782d, with contributor credit preserved.

What changed from this PR before landing:

Kept Basic auth redaction compatible with base64 padding and short tokens.
Added direct logs.tail coverage so the gateway/read path is proven, not only the raw redactor.
Kept the existing standalone JWT masking path intact.

Proof: focused Vitest passed after final rebase (8 files / 348 tests), and Blacksmith Testbox through Crabbox passed pnpm check:changed: tbx_01krryzqc6djxxnbrpea1n3n0t, Actions run https://github.com/openclaw/openclaw/actions/runs/25968977106, exit 0.

Fix logs.tail credential-header redaction and JSON-mode gateway transport errors.\n\nFixes openclaw#66832.\nFixes openclaw#79108.\nSupersedes openclaw#67041.\nSupersedes openclaw#79233.\n\nCo-authored-by: Mil Wang <mingjwan@microsoft.com>\nCo-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>

openclaw-barnacle Bot added the size: XS label Apr 15, 2026

greptile-apps Bot reviewed Apr 15, 2026

View reviewed changes

clawsweeper Bot mentioned this pull request Apr 27, 2026

[Bug]: logs.tail likely fails to redact several credential formats before returning log lines to operator.read clients. #66832

Closed

vincentkoc mentioned this pull request May 16, 2026

fix(gateway): redact credential-bearing diagnostics #82677

Merged

vincentkoc closed this in #82677 May 16, 2026

vincentkoc mentioned this pull request May 16, 2026

fix(gateway): land linked diagnostics fixes #82690

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: add redaction patterns for JWTs, Basic auth, and custom security headers in logs.tail#67041

fix: add redaction patterns for JWTs, Basic auth, and custom security headers in logs.tail#67041
Magicray1217 wants to merge 1 commit into
openclaw:mainfrom
Magicray1217:fix/logs-redact-jwt-basic-auth

Magicray1217 commented Apr 15, 2026

Uh oh!

greptile-apps Bot commented Apr 15, 2026

Uh oh!

greptile-apps Bot Apr 15, 2026

Uh oh!

greptile-apps Bot Apr 15, 2026

Uh oh!

clawsweeper Bot commented Apr 27, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Apr 29, 2026

Uh oh!

vincentkoc commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	String.raw`Authorization\s[:=]\sBasic\s+([A-Za-z0-9+/=]{18,})\b`,
	String.raw`Authorization\s[:=]\sBasic\s+([A-Za-z0-9+/=]{18,})`,

Uh oh!

Conversation

Magicray1217 commented Apr 15, 2026

Summary

Changes

Tests

Uh oh!

greptile-apps Bot commented Apr 15, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

greptile-apps Bot Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

clawsweeper Bot commented Apr 29, 2026

Uh oh!

vincentkoc commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Apr 27, 2026 •

edited

Loading