Skip to content

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

Merged
steipete merged 2 commits into
openclaw:mainfrom
SebTardif:fix/workbench-redact-run-json
Sep 8, 2026
Merged

steipete merged 2 commits into
openclaw:mainfrom
SebTardif:fix/workbench-redact-run-json

Conversation

@SebTardif

@SebTardif SebTardif commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Agent Workbench renders the raw run.wait() result, exposing the session key in its Result JSON. Apply the cookbook's existing field-name redaction convention at the Result display boundary, matching recipes and the standalone CLIs.

The helper remains local so this browser starter stays independently copyable without importing Node-only recipe code. SDK state, session controls, and assistant text keep their existing behavior. The README explicitly describes the name-based matching, its overmasking, and its limits; this does not sanitize an entire page or recording.

This restores the fix from closed, unmerged #10 on current main. The cause of that earlier closure is unconfirmed. The implementation and regression tests are by @SebTardif; the maintainer clarified the documentation and captured fresh browser proof.

Validation

  • pnpm check: formatting, types, lint, all 23 tests, documentation for six recipes, all four starter builds, and both compiled CLI smokes pass.
  • Real Chrome on macOS, Node 24.20.0, pnpm 10.34.5, and the documented local workspace SDK shim: build the baseline and PR, serve using Vite preview, fill a synthetic session and prompt, then click Run. Baseline Result exposes the synthetic session; the PR shows [REDACTED]. Both complete with three events and hello from OpenClaw.
  • pnpm audit --json: zero vulnerabilities. Independent autoreview through P2 is clean; final head 8ab8dc14823d2da5b6a563252a56e3ddee6c12db passed Check. Maintainer proof includes the final-head browser rerun after the documentation clarification.
  • No live Gateway/provider behavior is claimed.

The changelog entry is in the separate final notes PR #14, to merge after this one. Merge and first-release decisions remain with the maintainer.

Before: baseline Result exposes the synthetic session

After: PR 13 Result redacts the synthetic session

The Agent Workbench Result panel stringified run.wait() output, so
sessionKey, token, password, and nested key fields appeared in the
browser. Recipes and starter CLIs already redact.

Copy the standalone redact helper into the workbench example and format
the Result JSON through it. Keep the helper local so the browser
example does not import recipes/_shared/run-main.ts.

Restores the closed unmerged PR 10 fix after a rebase-push race.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@clawsweeper

clawsweeper Bot commented Sep 7, 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. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: needs maintainer proof decision A ClawSweeper-authored PR needs a maintainer proof capture or override decision. labels Sep 7, 2026
@clawsweeper

clawsweeper Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 7, 2026, 10:44 PM ET / September 8, 2026, 02:44 UTC.

ClawSweeper review

What this changes

Agent Workbench masks sensitive field names in its Result JSON using a local helper, with regression tests and documentation of the masking limits.

Merge readiness

✅ Ready for maintainer review

The fix remains necessary on main, and the fresh final-head browser report resolves the previous proof concern. No blocking patch defects were found.

Priority: P2
Reviewed head: 8ab8dc14823d2da5b6a563252a56e3ddee6c12db

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused patch with relevant regression coverage and concrete final-head browser observations satisfies the earlier proof request.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The captured final-head Chrome report exercises the production Result renderer through the documented workspace shim and records masked sessionKey output with normal completion. This resolves the previous entrypoint-proof request; inaccessible attachments are a reviewer limitation, and no live-provider or whole-page sanitization claim is accepted.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The captured final-head Chrome report exercises the production Result renderer through the documented workspace shim and records masked sessionKey output with normal completion. This resolves the previous entrypoint-proof request; inaccessible attachments are a reviewer limitation, and no live-provider or whole-page sanitization claim is accepted.
Evidence reviewed 8 items Main still displays raw results: The fetched main revision renders JSON.stringify(result, null, 2) directly in the Result panel. The requested masking is absent there.
Introduced display boundary: The complete introduced source diff routes only Result rendering through the formatter. Requests, stored component settings, and assistant text are unchanged; three regression tests cover matching fields, nested keys, and a wait result.
Supported local browser environment: The starter depends on the repository workspace SDK shim, and sdk/README.md explicitly documents this setup. The shim returns the supplied sessionKey from wait(), so the reported browser scenario exercises the actual changed renderer with its documented local runtime.
Findings None None.
Security None None.

How this fits together

Agent Workbench is a copyable browser starter that sends run settings to the SDK and displays streamed events and final results. This change filters the final result for display without changing SDK requests or session controls.

flowchart LR
  A[Run controls] --> B[SDK run]
  B --> C[Assistant stream and events]
  B --> D[Final result]
  D --> E[Field-name masking]
  E --> F[Result JSON panel]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Source and test delta Production +37/-1; tests +59/-0; documentation +7/-0 The local helper preserves standalone copying, and three tests cover the display contract.

Root-cause cluster

Relationship: canonical
Canonical: #13
Summary: This is the active implementation restoring the closed-unmerged proposal; release notes are separate.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Keep masking at the Result display boundary, preserve independently copyable browser code, and retain the explicit limits on field-name masking.

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

Yes. The captured browser report reproduces raw session output on the pinned main baseline through Run → wait → Result, consistent with inspected source; this reviewer did not execute the app.

Is this the best way to solve the issue?

Yes. A display-only helper follows the existing standalone CLI pattern, avoids importing Node-only recipe code into the browser, and leaves SDK state intact.

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The captured final-head Chrome report exercises the production Result renderer through the documented workspace shim and records masked sessionKey output with normal completion. This resolves the previous entrypoint-proof request; inaccessible attachments are a reviewer limitation, and no live-provider or whole-page sanitization claim is accepted.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit. Replaced prior rating: 🦐 gold shrimp.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: needs maintainer proof decision: Current PR status no longer selects a status label.

Label justifications:

  • P2: This is a focused improvement to sensitive-field handling in one starter's Result display.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit. Replaced prior rating: 🦐 gold shrimp.
  • proof: sufficient: Contributor real behavior proof is sufficient. The captured final-head Chrome report exercises the production Result renderer through the documented workspace shim and records masked sessionKey output with normal completion. This resolves the previous entrypoint-proof request; inaccessible attachments are a reviewer limitation, and no live-provider or whole-page sanitization claim is accepted.

Evidence

What I checked:

  • Main still displays raw results: The fetched main revision renders JSON.stringify(result, null, 2) directly in the Result panel. The requested masking is absent there. (sdk/agent-workbench/src/App.tsx:285, d4c2718b8a7c)
  • Introduced display boundary: The complete introduced source diff routes only Result rendering through the formatter. Requests, stored component settings, and assistant text are unchanged; three regression tests cover matching fields, nested keys, and a wait result. (sdk/agent-workbench/src/App.tsx:286, 8ab8dc14823d)
  • Supported local browser environment: The starter depends on the repository workspace SDK shim, and sdk/README.md explicitly documents this setup. The shim returns the supplied sessionKey from wait(), so the reported browser scenario exercises the actual changed renderer with its documented local runtime. (packages/openclaw-sdk-shim/src/index.ts:169, 8ab8dc14823d)
  • Fresh browser proof resolves prior review: The captured comment fix(workbench): redact sensitive fields in run result JSON #13 (comment) records real Chrome on macOS, baseline and final-head builds, Run submission, three events, preserved session controls, and Result output showing sessionKey as [REDACTED]. It explicitly records a final-head rerun with run_gckzhl. Attachment retrieval failed in this reviewer environment; the images were not independently inspected. The captured concrete browser observations remain valid evidence, without a live Gateway claim. (8ab8dc14823d)
  • Re-review continuity: The production implementation and regression test are unchanged from the previously reviewed head. The prior review had no findings and requested verification of browser evidence; the fresh final-head report supplies that verification. (sdk/agent-workbench/src/App.tsx:286, 8ab8dc14823d)
  • Existing standalone redaction precedent: Main already contains the same local helper in the quickstart starter. Blame points to 9e1866b; raw commit inspection identified its parent, and comparison against that parent confirms this helper was added there. This supports routing to SebTardif beyond authorship of this proposal. (sdk/quickstart/src/redact-sensitive-output.ts:14, 9e1866b365ec)

Likely related people:

  • Sebastien Tardif: Raw commit 9e1866b adds sdk/quickstart/src/redact-sensitive-output.ts:14 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 9e1866b365ec; files: sdk/quickstart/src/redact-sensitive-output.ts)
  • steipete: 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 (1 earlier review cycle)
  • reviewed 2026-09-07T21:06:17.295Z sha d2e3d5a :: needs real behavior proof before merge. :: none

@steipete

steipete commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Verified the exact PR head 8ab8dc14823d2da5b6a563252a56e3ddee6c12db in real Chrome on macOS with Node 24.20.0, pnpm 10.34.5, and the documented workspace SDK shim.

Built the baseline (d4c2718b8a7c9614bc0f03471ccbd1b07fe87fee) and this PR with pnpm --filter @openclaw/cookbook-agent-workbench build, served each using pnpm --filter @openclaw/cookbook-agent-workbench preview --host 127.0.0.1 --port 5183 --strictPort, entered synthetic-session-cookbook-13 and a synthetic prompt, then clicked Run. The baseline exposed the synthetic session value in Result. This PR completed the run with three timeline events and hello from OpenClaw, while Result displayed "sessionKey": "[REDACTED]". Session controls retained their value.

pnpm check passed: formatting, types, lint, 23 tests, six recipe documentation checks, all four starter builds, and both compiled CLI smokes. pnpm audit --json reported zero vulnerabilities. Independent branch autoreview through P2 is clean. Exact-head Check: https://github.com/openclaw/cookbook/actions/runs/34180923631 (success).

Both screenshots were inspected and contain only synthetic local data. This proves the Result display boundary using the supported local shim; it does not claim live Gateway/provider verification or whole-page redaction.

The implementation and tests were retained as submitted. A maintainer documentation commit clarifies the exact field-name policy and its limits. The screenshots show the same implementation before that documentation-only commit; rebuilding and repeating the browser run at the final head produced the same redacted result (run run_gckzhl). The maintainer changelog entry is being prepared separately so sibling PRs remain independent. Prepared for the orchestrator's merge decision; no merge or release performed.

Before: baseline Result exposes the synthetic session

After: PR 13 Result redacts the synthetic session

@steipete
steipete merged commit 34c2c26 into openclaw:main Sep 8, 2026
3 of 4 checks passed
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. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: needs maintainer proof decision A ClawSweeper-authored PR needs a maintainer proof capture or override decision.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants