Issue Station
Sign In
Tools hereToolbox

openclaw - ✅(Solved) Fix Security: Information leakage via unfiltered error object serialization [1 pull requests, 1 participants]

Official PRs (…)
ON THIS PAGE

Recommended Tools

×6

Utilities matched from this issue’s tags and category — try them while you read without losing context.

GitHub issue graph ai analysis

Paste a GitHub issue URL. We fetch that issue, discover linked issues from bodies/comments/timeline, collect linked pull requests, and produce a structured English report.

The report is written in English Markdown for sharing and archival.

Helpful · Quick feedback

Loading…
GitHub stats
openclaw/openclaw#54291•Fetched 2026-04-08 01:29:32
View on GitHub
Comments
0
Participants
1
Timeline
5
Reactions
0
Author
jinduwang1001-maxjinduwang1001-max
Participants
jinduwang1001-maxjinduwang1001-max
Timeline (top)
referenced ×4cross-referenced ×1

describeUnknownError() and similar error formatting functions use JSON.stringify(err) on arbitrary error objects without filtering sensitive fields. Error objects in Node.js often contain stack traces, internal file paths, database connection strings, or API keys in URL parameters.

The serialized output flows into user-facing error messages and gateway error handling.

Error Message

describeUnknownError() and similar error formatting functions use JSON.stringify(err) on arbitrary error objects without filtering sensitive fields. Error objects in Node.js often contain stack traces, internal file paths, database connection strings, or API keys in URL parameters. The serialized output flows into user-facing error messages and gateway error handling. Implement an allowlist of safe error properties to serialize (e.g., message, code, status). Strip stack traces, internal paths, and properties matching credential patterns before serialization.

Root Cause

describeUnknownError() and similar error formatting functions use JSON.stringify(err) on arbitrary error objects without filtering sensitive fields. Error objects in Node.js often contain stack traces, internal file paths, database connection strings, or API keys in URL parameters.

The serialized output flows into user-facing error messages and gateway error handling.

PR fix notes

PR #62189: fix(security): wire describeUnknownError through formatErrorMessage for credential redaction

Description (problem / solution / changelog)

Summary

  • Problem: describeUnknownError() in pi-embedded-runner/utils.ts and secrets/shared.ts serialises arbitrary error objects with raw JSON.stringify(). Error objects in Node.js regularly carry stack traces, internal file paths, DB connection strings, and API keys embedded in URL parameters.
  • Why it matters: the serialised output flows into user-facing error messages and gateway error handling, exposing credentials to end users and connected chat channels.
  • What changed: both describeUnknownError implementations now delegate to formatErrorMessage() from infra/errors.ts, which already applies redactSensitiveText() with 17+ regex patterns covering token prefixes (sk-, ghp_, xox*), Bearer headers, PEM blocks, and ENV-style credential assignments.
  • What did NOT change: formatErrorMessage itself is untouched. No new redaction patterns added. No changes to how errors are thrown or caught. Call sites that already used formatErrorMessage directly are unaffected.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

  • Closes #54291
  • This PR fixes a bug or regression

Root Cause (if applicable)

  • Root cause: describeUnknownError was written independently in two files (pi-embedded-runner/utils.ts and secrets/shared.ts) before formatErrorMessage + redactSensitiveText existed in infra/errors.ts. When the centralised redaction was added, these two call sites weren't updated to use it.
  • Missing detection / guardrail: no lint rule or test asserting that error serialisation passes through the redaction pipeline.
  • Contributing context: the formatErrorMessage function in infra/errors.ts already handles the full type-narrowing chain (Error instances, strings, numbers, booleans, unknown objects) with JSON.stringify as a last resort, then pipes everything through redactSensitiveText. The two describeUnknownError functions duplicated this logic minus the redaction step.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Existing coverage already sufficient
  • Target test or file: src/infra/errors.ts is exercised transitively by the pi-embedded-runner test suite (281 passing tests in vitest.infra.config.ts)
  • Scenario the test should lock in: error objects containing credential-like strings get redacted before reaching user-facing output
  • Why this is the smallest reliable guardrail: the existing redactSensitiveText regex suite already covers the credential patterns. Wiring through formatErrorMessage picks up that coverage automatically.
  • If no new test is added, why not: formatErrorMessage and redactSensitiveText both have their own test coverage. Adding a test for the delegation itself would just be testing that a function call works, not behaviour.

User-visible / Behavior Changes

Error messages returned to users and logged to channels will have credentials redacted where they weren't before. Some error detail (stack traces, internal paths) may be shorter than before. This is the intended fix.

Diagram (if applicable)

Before:
  catch (err) -> describeUnknownError(err) -> JSON.stringify(err) -> user sees raw credentials

After:
  catch (err) -> describeUnknownError(err) -> formatErrorMessage(err) -> redactSensitiveText() -> user sees [REDACTED]

Security Impact (required)

  • New permissions/capabilities? No
  • Touches auth, secrets, or tokens? Yes - reduces exposure of leaked credentials in error messages
  • Handles user input? No
  • New dependencies? No
  • Relevant CWE: CWE-209 (Generation of Error Message Containing Sensitive Information)

Repro + Verification

Environment: openclaw 2026.4.6, Node.js 22+, Ubuntu/macOS/Windows

Steps to reproduce the original issue:

  1. Trigger any error path that hits describeUnknownError (e.g. a provider auth failure with a token in the URL)
  2. The raw token appears in the error message shown to the user or sent to a chat channel

Expected after fix: tokens matching known patterns (sk-*, ghp_*, Bearer *, etc.) get replaced with [REDACTED]

Verification:

pnpm tsgo         # types pass (pre-existing failures only)
node scripts/run-vitest.mjs run --config vitest.infra.config.ts  # 281 pass, 3 pre-existing failures

Human Verification (required)

  • I've verified that formatErrorMessage in infra/errors.ts already handles all the type-narrowing branches that the two removed describeUnknownError bodies handled (Error, string, number, bigint, boolean, unknown object via JSON.stringify)
  • I've confirmed redactSensitiveText covers token prefixes, Bearer headers, PEM blocks, ENV assignments, and common secret patterns (17+ regex patterns)
  • I ran the full infra test suite before and after the change - identical results (281 pass, 3 pre-existing failures unrelated to this change)
  • I haven't verified every possible error object shape across all 15+ call sites of describeUnknownError end-to-end. The type signatures match and the delegation is straightforward, but a call site passing something unusual could theoretically behave differently with formatErrorMessage's cause-chain traversal.

Compatibility / Migration

No migration needed. The function signatures are unchanged. All existing callers continue to work. The only observable difference is shorter, redacted error messages.

Risks and Mitigations

RiskMitigation
formatErrorMessage traverses .cause chains, which the old describeUnknownError didn't. Could produce longer messages for nested errors.The cause-chain output is still piped through redactSensitiveText, so credentials won't leak even in longer messages. The extra context from cause chains is generally useful for debugging.
Circular import risk (secrets/shared.ts importing from infra/errors.ts)Verified: infra/errors.ts imports only from logging/redact.ts. No circular dependency.

Changed files

  • CHANGELOG.md (modified, +1/-0)
  • src/agents/pi-embedded-runner/utils.ts (modified, +8/-12)
  • src/secrets/shared.ts (modified, +5/-18)
RAW_BUFFERClick to expand / collapse

Summary

describeUnknownError() and similar error formatting functions use JSON.stringify(err) on arbitrary error objects without filtering sensitive fields. Error objects in Node.js often contain stack traces, internal file paths, database connection strings, or API keys in URL parameters.

The serialized output flows into user-facing error messages and gateway error handling.

Impact

  • Internal system information leaked to end users or logged to external services
  • Stack traces reveal file paths and dependency versions
  • Database errors may contain connection strings
  • HTTP errors may contain API keys in URL parameters

Location

  • dist/shared-DdWaoh8l.js:10-21
  • dist/channel-CqgJouqd.js:819
  • dist/probe-6CE-jr5k.js:390

Suggested Fix

Implement an allowlist of safe error properties to serialize (e.g., message, code, status). Strip stack traces, internal paths, and properties matching credential patterns before serialization.

extent analysis

Fix Plan

To address the issue, we will implement an error serialization function that filters sensitive fields. Here are the steps:

  • Create a function serializeError that takes an error object as input and returns a sanitized error object.
  • Define an allowlist of safe error properties to serialize (e.g., message, code, status).
  • Use a library like strip-ansi to remove ANSI escape codes from error messages.
  • Use a regular expression to detect and remove potential credential patterns.

Example Code

const stripAnsi = require('strip-ansi');

const safeProperties = ['message', 'code', 'status'];

function serializeError(err) {
  const sanitizedError = {};
  for (const property of safeProperties) {
    if (Object.prototype.hasOwnProperty.call(err, property)) {
      sanitizedError[property] = err[property];
    }
  }
  // Remove ANSI escape codes from error messages
  if (sanitizedError.message) {
    sanitizedError.message = stripAnsi(sanitizedError.message);
  }
  // Remove potential credential patterns
  for (const property in sanitizedError) {
    if (typeof sanitizedError[property] === 'string') {
      sanitizedError[property] = sanitizedError[property].replace(/(password|token|key)=[^&]*/gi, '');
    }
  }
  return sanitizedError;
}

// Usage
const error = new Error('Database connection failed');
error.code = 'ECONNREFUSED';
const serializedError = serializeError(error);
console.log(JSON.stringify(serializedError));

Verification

To verify that the fix worked, test the serializeError function with different error objects and verify that sensitive fields are removed. You can also log the serialized error objects to ensure they do not contain internal system information or credentials.

Extra Tips

  • Consider using a library like error-serializer to handle error serialization and deserialization.
  • Make sure to test the serializeError function thoroughly to ensure it handles different types of error objects and edge cases.
  • Review your codebase to ensure that all error objects are serialized using the serializeError function to prevent sensitive information from being leaked.

Vote matrix · Quick signals

Works
Did the solution work? Tap to confirm.
Easy Fix
Was it a quick fix?
Time Saver
Did it save you time?
Blocking
Was it severely blocking?
Common Issue
Are others likely hitting this too?
Flaky / Intermittent
Is it intermittent?
Verified / Reproducible
Can you reproduce it reliably?
Loading…
#api#prompt formatting#chain error#conversation history#database connection

Still need to ship something?

×6

Another batch ranked right after the header list — different links, same matching logic.

Back to top recommendations

TRENDING