fix(security): inline redact into appendSessionTranscriptMessage by Ziy1-Tan · Pull Request #73563 · openclaw/openclaw

Ziy1-Tan · 2026-04-28T12:51:56Z

What

All callsites of appendSessionTranscriptMessage now redact sensitive content before writing to disk, closing the path-B leak identified in #64046. User-configured redact settings (logging.redactSensitive, redactPatterns) are now correctly threaded through all write paths.

Previously, only messages routed through guardSessionManager (the pi-embedded-runner path) were redacted. Direct callers — transcript.ts, attempt-execution.ts, chat-transcript-inject.ts, transcript-mirror.ts — wrote plaintext to the JSONL transcript. Additionally, transcript.ts and chat-transcript-inject.ts typed their config param as SessionWriteLockAcquireTimeoutConfig, which has no logging field, silently discarding user redact preferences on those paths.

Why

Sensitive data (API keys, tokens, PII matching configured patterns) could persist unredacted in session transcript files on disk. This fix makes redaction always-on at the write layer and correctly propagates user config through all write paths.

Changes

src/agents/transcript-redact.ts (new) — extract the 4 redact helpers from session-tool-result-guard-wrapper.ts into a shared module; zero logic change, verbatim move
src/agents/session-tool-result-guard-wrapper.ts — delete local copies, import from transcript-redact.ts
src/config/sessions/transcript-append.ts — widen config?: to OpenClawConfig; call redactTranscriptMessage before building the JSONL entry
src/config/sessions/transcript.ts — widen config param in appendExactAssistantMessageToSessionTranscript from SessionWriteLockAcquireTimeoutConfig to OpenClawConfig so user redact settings thread through correctly
src/gateway/server-methods/chat-transcript-inject.ts — widen config param in injectAssistantMessageToSessionTranscript to OpenClawConfig for the same reason
src/agents/transcript-redact.test.ts (new) — 9 unit tests for the shared redact module
src/config/sessions/transcript-append-redact.test.ts (new) — 4 integration tests asserting on-disk JSONL content (includes test for appendExactAssistantMessageToSessionTranscript path)
docs/.generated/plugin-sdk-api-baseline.sha256 — regenerated after appendSessionTranscriptMessage config type change (re-exported via agent-harness-runtime.ts)

Key property: always-on. When config is undefined, redactTranscriptMessage falls back to DEFAULT_REDACT_MODE + DEFAULT_REDACT_PATTERNS — not a noop. When the user sets logging.redactSensitive='off', that preference is now honored on all write paths.

Real behavior proof (required for external PRs)

Behavior or issue addressed: session transcript JSONL files on disk contained unredacted sensitive content (API keys, tokens matching configured patterns) for all callers of appendSessionTranscriptMessage that did not go through guardSessionManager.
Real environment tested: local macOS machine, Node 22.19.0, patched worktree code with OPENCLAW_STATE_DIR=/tmp/proof-openclaw.
Exact steps or command run after this patch: OPENCLAW_STATE_DIR=/tmp/proof-openclaw node scripts/run-node.mjs agent --local --to +10000000001 --message "my api key is sk-abcdef1234567890xyz, please just reply: noted" --model claude-cli/claude-sonnet-4-6, then read back the session JSONL from disk.
Evidence after fix: on-disk JSONL showed sk-abc…0xyz instead of sk-abcdef1234567890xyz. Screenshot:
Observed result after fix: secret token masked in written JSONL; safe surrounding text preserved.
What was not tested: full daemon with real channel messages. The fix is isolated to the appendSessionTranscriptMessage write path — no channel or agent behavior was changed.

Testing

pnpm check ✅ (exit 0 — typecheck, lint, policy guards all pass)
pnpm plugin-sdk:api:check ✅ (baseline hash matches after regeneration)
node scripts/check-src-extension-import-boundary.mjs --json → [] ✅
node scripts/run-vitest.mjs run src/agents/transcript-redact.test.ts → 9/9 ✅
node scripts/run-vitest.mjs run src/config/sessions/transcript-append-redact.test.ts → 4/4 ✅
Tested: fully tested

Test Case	Expected	Actual	PASS
`redactTranscriptMessage` — text block with sk- token	Secret stripped, safe text kept	Unit test passes	✅
`redactTranscriptMessage` — `cfg=undefined`	Default patterns apply, not noop	Unit test passes	✅
`redactTranscriptMessage` — `redactSensitive:"off"`	Same object reference returned	Unit test passes	✅
`appendSessionTranscriptMessage` — secret in content	Secret absent from on-disk JSONL	Integration test passes	✅
`appendExactAssistantMessageToSessionTranscript` — `redactSensitive:'off'`	Secret preserved on disk	Integration test passes	✅
Real agent session via patched code	sk- token masked in JSONL on disk	Live run, screenshot above	✅

AI Disclosure

AI-assisted (Claude)
Tested: fully tested
I understand what the code does — the fix extracts existing redact helpers into a shared module and calls the exported function at the single write point in appendSessionTranscriptMessage, and widens config types on two additional write paths so user redact preferences are not silently discarded
ce:review run locally (correctness, maintainability, security, testing reviewers); all critical/high findings addressed
Session logs available on request

Closes #73565

greptile-apps · 2026-04-28T12:56:00Z

Greptile Summary

This PR correctly wraps all 4 bare SessionManager.open() call sites with guardSessionManager(), ensuring the beforeMessageWriteHook (redaction + tool-result guard) runs before every appendMessage. The follow-up fixes to both the idempotency-key asymmetry and the missing sessionKey in attempt-execution.ts are addressed in the diff.

updateMode: \"none\" silencing is broken: guardedAppend unconditionally calls emitSessionTranscriptUpdate; the default: break branch in the new switch adds no suppression, so updateMode: \"none\" now fires the event it was designed to suppress.
updateMode: \"file-only\" now double-emits: the guard fires a full-payload event (message + messageId) and then the switch fires a second bare-path event. Listeners see each message twice, and the "no-message-details" contract of \"file-only\" is violated. The code comment on line 287 explicitly notes that both modes need explicit handling, but only the extra emit for \"file-only\" was added — suppression was never implemented for either mode.

Confidence Score: 4/5

Safe to merge with awareness of the updateMode regression — currently only test-code callers use "file-only" or "none", so production impact is limited, but the contract is broken.

One confirmed P1: updateMode: "file-only" now double-emits (guard fires full-payload event + switch fires bare-path event) and updateMode: "none" silencing is lost. Both modes are only used in tests currently, so there is no confirmed production breakage, but the API contract change warrants follow-up before the modes are relied upon in production paths.

src/config/sessions/transcript.ts — the updateMode switch around lines 289-295 needs suppression logic for "none" and should not add a second emit for "file-only".

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/config/sessions/transcript.ts
Line: 286-295

Comment:
**`updateMode: "none"` no longer suppresses the event, and `"file-only"` now double-emits**

The comment on line 287 correctly states that `"file-only"` and `"none"` require explicit handling, but the switch only adds an *extra* emit for `"file-only"` and does nothing for `"none"` — neither case suppresses the guard's automatic emit.

Concrete effects:
- **`"none"`**: `guardedAppend` inside `installSessionToolResultGuard` unconditionally calls `emitSessionTranscriptUpdate({ sessionFile, sessionKey, message, messageId })`. The `default: break` branch never fires, so `updateMode: "none"` no longer silences the event.
- **`"file-only"`**: the guard fires a full-payload event (`message` + `messageId` included), then the switch fires a second bare `emitSessionTranscriptUpdate(sessionFile)`. Listeners receive the message twice: once with full details, once with only the path. The original semantics of "file-only" (no message details exposed to listeners) are violated.

The guard does not expose a mechanism to suppress its own emit, so the fix needs to be applied at the `guardSessionManager` call site or by threading an `onAfterAppend` callback through `guardSessionManager`'s options.

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

_{Reviews (2): Last reviewed commit: "test: remove stale eslint-disable from t..." | Re-trigger Greptile}

clawsweeper · 2026-04-28T12:56:57Z

Codex review: found issues before merge.

Summary
The PR extracts transcript redaction into a shared helper, applies it at appendSessionTranscriptMessage, widens transcript config typing, updates the SDK API baseline hash, and adds redaction tests.

Reproducibility: yes. A direct call to appendSessionTranscriptMessage with an sk- token and JSONL readback is a high-confidence path; current main source shows the raw params.message is persisted while the PR proof covers the patched path.

Real behavior proof
Sufficient (terminal): The PR body and inspected terminal screenshot show a patched local run plus on-disk JSONL readback with the submitted token masked after the fix.

Next step before merge
The remaining changelog fix is mechanical, but the PR branch currently reports dirty mergeability and rebaseable false, so contributor or maintainer branch-state repair is needed first.

Security
Cleared: The diff centralizes existing redaction logic at transcript append and does not add dependencies, workflow changes, install scripts, permissions, or new secret sources.

Review findings

[P3] Add the required changelog entry — src/config/sessions/transcript-append.ts:287

Review details

Best possible solution:

Land the append-layer redaction after resolving the branch conflicts, adding the active changelog entry, and rerunning the normal focused and changed-surface gates.

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

Yes. A direct call to appendSessionTranscriptMessage with an sk- token and JSONL readback is a high-confidence path; current main source shows the raw params.message is persisted while the PR proof covers the patched path.

Is this the best way to solve the issue?

Yes, with process blockers. Centralizing redaction at the locked append helper is the narrow durable fix for direct transcript-message writes, but the PR should add the changelog entry and resolve dirty mergeability before merge.

Full review comments:

[P3] Add the required changelog entry — src/config/sessions/transcript-append.ts:287
This changes user-facing security behavior for persisted session transcripts, but the PR does not update CHANGELOG.md. Repo policy requires user-facing fix/security changes to be recorded under the active release before merge.
Confidence: 0.93

Overall correctness: patch is correct
Overall confidence: 0.87

What I checked:

Current main persistence gap: Current origin/main still builds the transcript JSONL entry with message: params.message, so direct append callers persist the raw message payload at the append layer. (src/config/sessions/transcript-append.ts:293, 3e53b192842e)
Documented expected behavior: The logging docs say redaction applies to session transcript text sinks before JSONL lines or messages are written to disk. Public docs: docs/gateway/logging.md. (docs/gateway/logging.md:76, 3e53b192842e)
PR fix location: The PR computes finalMessage = redactTranscriptMessage(...) before constructing the persisted JSONL entry and writes finalMessage instead of the raw message. (src/config/sessions/transcript-append.ts:287, e869237847e5)
Regression coverage: The PR adds append-path tests for redaction-on, redaction-off, default config, and the exact assistant append path. (src/config/sessions/transcript-append-redact.test.ts:22, e869237847e5)
Real behavior proof: The inspected terminal screenshot shows a patched local openclaw agent --local run followed by JSONL readback where the submitted sk- token is stored as a masked value. (e869237847e5)
Missing changelog: The PR file list has eight changed files and no CHANGELOG.md entry, while this is a user-facing security fix for persisted transcripts. (CHANGELOG.md:5, e869237847e5)

Likely related people:

steipete: Recent history on the session transcript append helper and write-lock/concurrency behavior makes this the closest current-main owner for the append boundary touched by the PR. (role: recent maintainer; confidence: medium; commits: 3f6b481464bf, f7ed29e11812, b08220446a39; files: src/config/sessions/transcript-append.ts, src/config/sessions/transcript.ts)
vincentkoc: Recent commits connect this maintainer to persisted transcript redaction behavior and logging documentation that define the expected behavior this PR moves to the append boundary. (role: introduced adjacent redaction behavior; confidence: medium; commits: 406ae72fd278, 1fe15f230617; files: src/agents/session-tool-result-guard-wrapper.ts, src/logging/redact.ts, docs/gateway/logging.md)
hxy91819: The PR timeline assigns this maintainer for review, and recent redaction-pattern history includes them as a co-author/reviewer on adjacent logging redaction work. (role: adjacent reviewer; confidence: low; commits: 0260903f7f71; files: src/logging/redact.ts)

Remaining risk / open question:

The PR currently reports dirty mergeability and rebaseable: false, so the exact head needs branch-state repair before normal merge validation.
The PR still lacks the required active CHANGELOG.md entry for a user-facing security fix.

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

Ziy1-Tan · 2026-04-28T14:53:54Z

@greptile-apps

Ziy1-Tan · 2026-04-29T06:38:57Z

cc @hxy91819 .

AgentMessage is a union that includes BashExecutionMessage (from @mariozechner/pi-coding-agent module augmentation), which has no content field. Direct .content access on AgentMessage fails tsgo's strict union check. Changes: - Add msgContent() helper that casts through unknown to access content - Replace all result.content casts with msgContent(result) calls - Change as AgentMessage casts to as unknown as AgentMessage for object literals that don't fully satisfy AssistantMessage shape

…ranscript-inject.ts transcript.ts and chat-transcript-inject.ts passed SessionWriteLockAcquireTimeoutConfig (which has no logging field) to appendSessionTranscriptMessage, silently dropping logging.redactSensitive='off' and custom redactPatterns on those write paths. - Widen config param from SessionWriteLockAcquireTimeoutConfig to OpenClawConfig in appendExactAssistantMessageToSessionTranscript (transcript.ts) and injectAssistantMessageToSessionTranscript (chat-transcript-inject.ts) - Add integration test: appendExactAssistantMessageToSessionTranscript with redactSensitive='off' asserts the secret is NOT redacted on disk - Regenerate plugin-sdk-api-baseline.sha256 (appendSessionTranscriptMessage is re-exported via agent-harness-runtime; config type change drifts the hash)

Ziy1-Tan

4aff949 addresses this directly — appendInjectedAssistantMessageToTranscript no longer goes through guardSessionManager. It now calls appendSessionTranscriptMessage directly and passes config: params.config, so redactSensitive="off" is honored on this path too.
The guardSessionManager call you flagged is gone from this file entirely.
pnpm plugin-sdk:api:check passes on this branch — baseline was regenerated in docs/.generated/plugin-sdk-api-baseline.sha256.

PTAL @hxy91819

jesse-merhi · 2026-05-09T03:42:14Z

@hxy91819 and I both think this is good to go. Going to get this closed out with clawsweeper.

jesse-merhi · 2026-05-09T03:42:19Z

/clawsweeper automerge

clawsweeper · 2026-05-09T03:43:42Z

🦞🔧
ClawSweeper automerge is enabled.

Head: e869237847e5
Label: clawsweeper:automerge
Action: repair worker queued. Run: https://github.com/openclaw/clawsweeper/actions/runs/25590789778
Flow: review this head, repair/rebase only if needed, then re-review the exact repaired head before merge.

Draft PRs stay fix-only until GitHub marks them ready for review. Pause with /clawsweeper stop.

Automerge progress:

2026-05-09 03:42:19 UTC review queued e869237847e5 (queued)

2026-05-09 03:43:41 UTC repair queued e869237847e5 (autonomous) Run: https://github.com/openclaw/clawsweeper/actions/runs/25590789778

2026-05-09 03:47:31 UTC repair started (running) in 1s Run: https://github.com/openclaw/clawsweeper/actions/runs/25590789778 automerge-openclaw-openclaw-73563

2026-05-09 03:47:44 UTC validation plan (passed) in 14s Run: https://github.com/openclaw/clawsweeper/actions/runs/25590789778 pnpm check:changed

2026-05-09 03:47:55 UTC Codex write preflight (passed) in 25s Run: https://github.com/openclaw/clawsweeper/actions/runs/25590789778 danger-full-access

2026-05-09 03:54:46 UTC Codex edit 1 5effd83234e8 (complete) in 7m 16s Run: https://github.com/openclaw/clawsweeper/actions/runs/25590789778 exit 0

2026-05-09 04:05:28 UTC validation and review 1 c9600f19e117 (base moved) in 17m 58s Run: https://github.com/openclaw/clawsweeper/actions/runs/25590789778 rebased

2026-05-09 04:06:13 UTC repair finished c9600f19e117 (opened) in 18m 43s Run: https://github.com/openclaw/clawsweeper/actions/runs/25590789778 open_fix_pr

clawsweeper · 2026-05-09T04:06:11Z

ClawSweeper 🐠 reef update

Thanks for the contribution. The source branch was not safely writable by ClawSweeper, so it opened a replacement PR and kept the credit trail visible.

Replacement PR: #79645
This source PR is being closed only under the explicit source-close setting for this ClawSweeper run.
The original contribution stays credited in the replacement PR context.

fish notes: model gpt-5.5, reasoning high; reviewed against c9600f1.

openclaw-barnacle Bot added gateway Gateway runtime agents Agent runtime and tooling extensions: codex size: M labels Apr 28, 2026

greptile-apps Bot reviewed Apr 28, 2026

View reviewed changes

Comment thread extensions/codex/src/app-server/transcript-mirror.ts Outdated

Comment thread src/agents/command/attempt-execution.ts Outdated

clawsweeper Bot mentioned this pull request Apr 28, 2026

[Security]: session transcript persistence path missing redaction gate #73565

Closed

Ziy1-Tan force-pushed the fix/session-transcript-redaction branch from 18da68e to d86f58c Compare April 28, 2026 14:29

Ziy1-Tan mentioned this pull request Apr 28, 2026

fix: support explicit active-memory chat types #66285

Merged

2 tasks

greptile-apps Bot reviewed Apr 28, 2026

View reviewed changes

Comment thread src/config/sessions/transcript.ts Outdated

hxy91819 self-assigned this Apr 29, 2026

openclaw-barnacle Bot added docs Improvements or additions to documentation scripts Repository scripts size: L size: M and removed size: M scripts Repository scripts size: L labels Apr 29, 2026

Ziy1-Tan force-pushed the fix/session-transcript-redaction branch from 6c6108f to fc4cd78 Compare April 29, 2026 14:21

openclaw-barnacle Bot added size: L and removed size: M labels Apr 29, 2026

Ziy1-Tan force-pushed the fix/session-transcript-redaction branch from fc4cd78 to a264208 Compare April 29, 2026 14:36

Ziy1-Tan force-pushed the fix/session-transcript-redaction branch from a264208 to 8a61c8d Compare April 30, 2026 02:55

openclaw-barnacle Bot added the scripts Repository scripts label Apr 30, 2026

Ziy1-Tan force-pushed the fix/session-transcript-redaction branch from 8a61c8d to 3e2c175 Compare April 30, 2026 03:04

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 8, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime labels May 8, 2026

Ziy1-Tan force-pushed the fix/session-transcript-redaction branch from f297619 to 4aff949 Compare May 8, 2026 06:44

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

style: fix curly brace lint error in transcript-append-redact.test.ts

e869237

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

Ziy1-Tan commented May 8, 2026

View reviewed changes

jesse-merhi self-assigned this May 9, 2026

jesse-merhi self-requested a review May 9, 2026 03:42

clawsweeper Bot added the clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge label May 9, 2026

jesse-merhi approved these changes May 9, 2026

View reviewed changes

clawsweeper Bot mentioned this pull request May 9, 2026

fix(security): inline redact into appendSessionTranscriptMessage #79645

Merged

clawsweeper Bot closed this May 9, 2026

hxy91819 mentioned this pull request May 12, 2026

fix(security): centralize transcript append redaction #80950

Closed

Ziy1-Tan mentioned this pull request Jun 4, 2026

[Feature]: 希望支持敏感数据脱敏 #64046

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(security): inline redact into appendSessionTranscriptMessage#73563

fix(security): inline redact into appendSessionTranscriptMessage#73563
Ziy1-Tan wants to merge 6 commits into
openclaw:mainfrom
Ziy1-Tan:fix/session-transcript-redaction

Ziy1-Tan commented Apr 28, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

clawsweeper Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

Ziy1-Tan commented Apr 28, 2026

Uh oh!

Uh oh!

Ziy1-Tan commented Apr 29, 2026

Uh oh!

Ziy1-Tan left a comment •

edited

Loading

Uh oh!

jesse-merhi commented May 9, 2026

Uh oh!

jesse-merhi commented May 9, 2026

Uh oh!

clawsweeper Bot commented May 9, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Ziy1-Tan commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Changes

Real behavior proof (required for external PRs)

Testing

AI Disclosure

Uh oh!

greptile-apps Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Ziy1-Tan commented Apr 28, 2026

Uh oh!

Uh oh!

Ziy1-Tan commented Apr 29, 2026

Uh oh!

Ziy1-Tan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesse-merhi commented May 9, 2026

Uh oh!

jesse-merhi commented May 9, 2026

Uh oh!

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

Uh oh!

Uh oh!

clawsweeper Bot commented May 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Ziy1-Tan commented Apr 28, 2026 •

edited

Loading

greptile-apps Bot commented Apr 28, 2026 •

edited

Loading

clawsweeper Bot commented Apr 28, 2026 •

edited

Loading

Ziy1-Tan left a comment •

edited

Loading

clawsweeper Bot commented May 9, 2026 •

edited

Loading