Skip to content

fix(workbench): redact sensitive fields in run result JSON - #10

Closed
SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/workbench-redact-run-json
Closed

SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/workbench-redact-run-json

Conversation

@SebTardif

@SebTardif SebTardif commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Agent Workbench masks sensitive fields at the Result display boundary without changing session settings, SDK requests, or streamed assistant text. The helper stays local to preserve the copyable browser example.

Independently rebased onto main at 6deea84fc44138676bdf0d4b99dae98ec33d43ff. This PR has no changelog changes; #12 owns the complete Unreleased section. Contributor credit is preserved in the commit.

Before/after real-Chrome evidence for the unchanged renderer patch: #10 (comment)

@clawsweeper

clawsweeper Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 2, 2026
@clawsweeper

clawsweeper Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 5, 2026, 4:08 PM ET / 20:08 UTC.

ClawSweeper review

What this changes

The Agent Workbench masks sensitive fields before displaying run-result JSON, adds regression tests, and documents the masking boundary.

Merge readiness

✅ Ready for maintainer review

This PR remains necessary because current main still displays unredacted result JSON. The focused implementation and supplied browser verification support landing it; no blocking defect was found.

Priority: P2
Reviewed head: a5c6079027190f688b4b0020ad4658d4836dcb82

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, maintainable patch with relevant browser verification, regression coverage, and no blocking findings.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): Supplied exact-head Chrome observations exercise Run through the documented workspace shim to the changed Result renderer and report masked sessionKey with preserved completion, events, and assistant text; terminal output also demonstrates nested credential masking. Linked screenshots could not be retrieved independently, and no live Gateway coverage is claimed.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): Supplied exact-head Chrome observations exercise Run through the documented workspace shim to the changed Result renderer and report masked sessionKey with preserved completion, events, and assistant text; terminal output also demonstrates nested credential masking. Linked screenshots could not be retrieved independently, and no live Gateway coverage is claimed.
Evidence reviewed 8 items Current main still needs the fix: The fetched main revision stores run.wait() directly and renders JSON.stringify(result, null, 2). Its changelog contains no workbench redaction entry, and the local tag listing is empty; no shipped fix is established.
Bounded display-only implementation: The introduced formatter is called only by the Result panel. It creates a redacted representation without changing the stored result, session settings, SDK requests, or assistant stream. The three tests cover helper parity, nested credential fields, and a wait() result.
Existing pattern and standalone boundary: The recipe redactor uses the same key matching. Importing that module into the browser would also import node:url; README.md and CONTRIBUTING.md describe independently copyable SDK examples. The local helper therefore has a concrete packaging justification rather than creating a competing user-facing capability.
Findings None None.
Security None None.

How this fits together

Agent Workbench is a copyable browser example that sends prompts and session settings through the cookbook SDK shim. It displays streamed events and assistant text, then formats the completed run result in a separate panel.

flowchart TD
  A[Prompt and session settings] --> B[SDK run]
  B --> C[Events and assistant text]
  B --> D[Completed result]
  D --> E[Mask sensitive JSON fields]
  E --> F[Result panel]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +37/-1; tests +59/-0; documentation +6/-0 The small production addition keeps the browser example standalone, with three focused regression tests.

Technical review

Best possible solution:

Keep masking at the Result display boundary, preserve the reusable session value internally, and retain the documented limits on screenshot sanitization.

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

Yes: main directly stringifies the shim's session-bearing wait() result, and the supplied Chrome baseline confirms the visible value. This review verified the source path without executing the application.

Is this the best way to solve the issue?

Yes: formatting a redacted copy at the rendering boundary follows the existing recipe policy while preserving standalone browser packaging and runtime session behavior.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against afff52646498.

Labels

Label justifications:

  • P2: This repairs sensitive-field exposure in one example's Result panel with a limited, display-only change.
  • 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 (live_output): Supplied exact-head Chrome observations exercise Run through the documented workspace shim to the changed Result renderer and report masked sessionKey with preserved completion, events, and assistant text; terminal output also demonstrates nested credential masking. Linked screenshots could not be retrieved independently, and no live Gateway coverage is claimed.
  • proof: sufficient: Contributor real behavior proof is sufficient. Supplied exact-head Chrome observations exercise Run through the documented workspace shim to the changed Result renderer and report masked sessionKey with preserved completion, events, and assistant text; terminal output also demonstrates nested credential masking. Linked screenshots could not be retrieved independently, and no live Gateway coverage is claimed.

Evidence

What I checked:

  • Current main still needs the fix: The fetched main revision stores run.wait() directly and renders JSON.stringify(result, null, 2). Its changelog contains no workbench redaction entry, and the local tag listing is empty; no shipped fix is established. (sdk/agent-workbench/src/App.tsx:285, afff52646498)
  • Bounded display-only implementation: The introduced formatter is called only by the Result panel. It creates a redacted representation without changing the stored result, session settings, SDK requests, or assistant stream. The three tests cover helper parity, nested credential fields, and a wait() result. (sdk/agent-workbench/src/App.tsx:286, a5c607902719)
  • Existing pattern and standalone boundary: The recipe redactor uses the same key matching. Importing that module into the browser would also import node:url; README.md and CONTRIBUTING.md describe independently copyable SDK examples. The local helper therefore has a concrete packaging justification rather than creating a competing user-facing capability. (recipes/_shared/run-main.ts:1, a5c607902719)
  • Real browser verification at the reviewed head: The supplied comment fix(workbench): redact sensitive fields in run result JSON #10 (comment) records Chrome on macOS with Node 24.20.0: clicking Run with synthetic inputs completed with three events, unchanged assistant text, and a masked sessionKey at the exact reviewed head; the pinned main baseline displayed the value. The body additionally supplies formatter output for nested token/password/apiKey fields. Attachment retrieval returned cache misses, so the images were not independently viewed; this reviewer limitation does not invalidate the supplied runtime observations. (sdk/agent-workbench/src/App.tsx:286, a5c607902719)
  • Proof environment matches the owned example: The workbench depends on the workspace SDK shim, whose default wait() result contains sessionKey. This proves the demonstrated local example boundary without claiming live Gateway/provider coverage. (packages/openclaw-sdk-shim/src/index.ts:155, a5c607902719)
  • Re-review continuity and validation: The previous completed review reported no findings. Comparing its head with this head shows only README and changelog additions, including the explicit limit that controls and assistant text remain visible. The supplied comment reports pnpm check passing all ten tests and both browser builds, and the supplied current-head check result is successful. No tests or bundled review helpers were executed during this read-only review. (sdk/agent-workbench/README.md:25, a5c607902719)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (7 earlier review cycles)
  • reviewed 2026-09-02T18:53:44.657Z sha b6504bf :: needs maintainer review before merge. :: none
  • reviewed 2026-09-03T07:59:57.102Z sha b6504bf :: needs maintainer review before merge. :: none
  • reviewed 2026-09-03T10:39:41.363Z sha b6504bf :: needs maintainer review before merge. :: none
  • reviewed 2026-09-03T21:52:12.687Z sha b6504bf :: needs maintainer review before merge. :: none
  • reviewed 2026-09-04T02:53:14.467Z sha b6504bf :: needs maintainer review before merge. :: none
  • reviewed 2026-09-04T23:00:57.089Z sha b6504bf :: needs maintainer review before merge. :: none
  • reviewed 2026-09-05T10:59:52.784Z sha b6504bf :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Sep 4, 2026
@steipete

steipete commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Verified the change in real Chrome on macOS with Node 24.20.0 and the documented workspace SDK shim. At a5c6079027190f688b4b0020ad4658d4836dcb82, entered a synthetic session value and prompt, clicked Run, and observed a completed run, three timeline events, unchanged assistant text, and "sessionKey": "[REDACTED]" in Result. The baseline on afff526464987148d792fddea8c63626cd8f0d11 displayed the synthetic session value in Result. Both screenshots contain only synthetic local data.

pnpm check passed, including formatting, types, lint, all 10 tests, documentation, both compiled CLIs, and both browser production builds. Independent autoreview through P2 is clean. This verifies Result JSON masking; session controls and assistant text remain visible, and no live Gateway/provider behavior is claimed.

Added documentation of the output boundary and an Unreleased entry crediting @SebTardif. Prepared for maintainer approval; not merged.

Before: synthetic session visible in Result

After: Result session key is redacted

Mask sensitive fields at the Result display boundary without changing the stored result, session settings, SDK requests, or assistant stream.

Release notes are consolidated in PR openclaw#12.

Co-authored-by: Sebastien Tardif <sebtardif@ncf.ca>
@steipete

steipete commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Restacked independently onto main 6deea84fc44138676bdf0d4b99dae98ec33d43ff at 3a44cfe1ebe10dac0ea4e67b5eaeb7a8ffdf3642.

This PR has no changelog diff; the complete Unreleased section is in #12. The four PRs touch disjoint files and reconstruct the previously proven combined source tree exactly (56576a9cc5b03069b71700b776279ba19763d030).

The full cookbook gate passed (20 tests, formatting, types, lint, docs, all four starter builds, and both compiled CLI smokes). Branch autoreview through P2 is clean. Exact-head CI: https://github.com/openclaw/cookbook/actions/runs/33990197270

Local runs were serialized using short private macOS temp directories after concurrent subprocess timeouts and a too-long tsx socket path. No test timeouts or product code were weakened. No merge or release performed.

This exact-head CI ran from a task-owned mirror because this PR was unexpectedly closed before the rebase push; reopening is awaiting orchestrator clarification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants