Skip to content

security: redact mnemonic-shaped sequences from SDK default logger output#20

Merged
Sentinel-Bluebuilder merged 1 commit intomasterfrom
fix/redacting-logger
Apr 27, 2026
Merged

security: redact mnemonic-shaped sequences from SDK default logger output#20
Sentinel-Bluebuilder merged 1 commit intomasterfrom
fix/redacting-logger

Conversation

@Sentinel-Bluebuilder
Copy link
Copy Markdown
Owner

Summary

Severity: MEDIUM — Defense-in-depth against accidental mnemonic leaks through the SDK's default logger.

The SDK's default defaultLog is console.log and several inline fallbacks (opts.log || console.log) bypass it entirely. The SDK does not currently log the mnemonic. This PR is a safety net so a future bug doesn't.

Concrete leak scenarios this would catch:

  • log(derive failed: opts=${JSON.stringify(opts)})opts.mnemonic ends up in stdout.
  • A new contributor adds log(connecting with phrase ${opts.mnemonic.slice(0,4)}...) for "debugging".
  • A Sentry.breadcrumb({ msg: arg }) integration capturing the same arg the logger received.

Once a 12-word phrase touches stdout, it's in terminal scrollback, CI logs, log-aggregation pipelines (Datadog, Loki, Sentry), and shell history — and the wallet is permanently exposed.

Fix

New file connection/logger.js exports withMnemonicRedaction(logFn), which wraps any logger so 12/15/18/21/24-word lowercase BIP-39-shaped sequences in its arguments are replaced with [REDACTED MNEMONIC] before the underlying logger sees them.

const log = withMnemonicRedaction(console.log);
log('opts:', 'abandon ability able about above absent absorb abstract absurd abuse access accident');
//   → 'opts: [REDACTED MNEMONIC]'
log('hello world');
//   → 'hello world'
log('debug: connecting with phrase=abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon to node sentnode1xyz');
//   → 'debug: connecting with phrase=[REDACTED MNEMONIC] to node sentnode1xyz'

Wired into:

  • connection/connect.jsdefaultLog + 1 stale opts.log || console.log fallback
  • node-connect.jsdefaultLog + 2 stale fallbacks (recoverSession, connectAuto retry loop)
  • connection/disconnect.js — new module-local defaultLog for recoverSession
  • connection/resilience.js — new module-local defaultLog for tryFastReconnect

Design notes

  • Strings only. The redactor inspects typeof === 'string' arguments and returns everything else unchanged. We deliberately don't recurse into objects — that would risk triggering side-effecting custom getters and turn a logging utility into a footgun.
  • Cheap fast path. Strings shorter than 60 chars (shorter than the shortest 12-word phrase) skip the regex entirely.
  • User loggers are not wrapped. If you pass opts.log = mySpecialLogger, your logger gets the raw values. We only redact our own defaults — this is about catching our future bugs, not constraining your logging stack.
  • Pass-through for non-function input. withMnemonicRedaction(null) === null, so callers can still disable logging via defaultLog = null.

Test plan

Smoke-tested locally:

import { withMnemonicRedaction } from './connection/logger.js';
const buf = [];
const log = withMnemonicRedaction((...a) => buf.push(a.join(' ')));
log('opts:', 'abandon ability able about above absent absorb abstract absurd abuse access accident');
log('mnemonic:', Array(24).fill('abandon').join(' '));
log('hello world');
log(Array(11).fill('abandon').join(' '));      // below threshold — passes through
log('debug: connecting with phrase=' + Array(12).fill('abandon').join(' ') + ' to node sentnode1xyz');
console.log(buf);
// [
//   "opts: [REDACTED MNEMONIC]",
//   "mnemonic: [REDACTED MNEMONIC]",
//   "hello world",
//   "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon",
//   "debug: connecting with phrase=[REDACTED MNEMONIC] to node sentnode1xyz"
// ]
  • node --check passes on all 5 modified files
  • redactor handles 12/24-word phrases, leaves 11-word strings alone
  • redactor performs surgical replacement inside larger templates
  • non-function input returns unchanged (so defaultLog = null still works)
  • Reviewer: confirm no behavior change for callers passing their own opts.log
  • Reviewer: confirm short progress strings ('wallet', 'handshake', etc.) are unaffected

…tput

The SDK's default loggers (`defaultLog` in connection/connect.js and
node-connect.js, plus inline `opts.log || console.log` fallbacks in
recoverSession and tryFastReconnect) wrote directly to console.log. The SDK
itself never logs the mnemonic today, but a future template-string bug
(e.g. `log(`opts: ${JSON.stringify(opts)}`)` or `log(`derive failed for
${opts.mnemonic.slice(0,4)}...`)`) would leak the BIP-39 phrase to stdout —
and from there to terminal scrollback, CI logs, log aggregators, and shell
history.

Add connection/logger.js exporting `withMnemonicRedaction(logFn)`, which:
- runs the underlying logger after replacing any 12/15/18/21/24-word
  lowercase BIP-39-shaped sequence with `[REDACTED MNEMONIC]`,
- short-circuits on strings shorter than 60 characters (cheap fast path),
- only inspects string args (does not recurse into objects — avoids
  triggering side-effecting custom getters),
- pass-through for non-function input so callers can still disable
  logging by passing `null`.

Wire it into:
- connection/connect.js   defaultLog assignment + 1 stale fallback
- node-connect.js         defaultLog assignment + 2 stale fallbacks
- connection/disconnect.js  new module-local defaultLog (recoverSession)
- connection/resilience.js  new module-local defaultLog (tryFastReconnect)

User-supplied `opts.log` is still honored verbatim — we do not wrap external
loggers, only our own defaults. This is defense-in-depth against future bugs
in OUR code, not a constraint on the consumer's logging stack.
@Sentinel-Bluebuilder Sentinel-Bluebuilder merged commit 2064054 into master Apr 27, 2026
2 checks passed
@Sentinel-Bluebuilder Sentinel-Bluebuilder deleted the fix/redacting-logger branch April 27, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant