fix: auto-wrap Error args in the logger + diagnostic property allowlist (#1734)

[kriszyp]

Summary

Fixes the raw-Error logging leak class (#1734) at its root. Harper's logger extends Node's Console; passing a raw Error to a Console method formats it with util.inspect, which dumps every own-enumerable property after the stack. Libraries/app code stash secrets on thrown Errors (axios config/request headers, an hdb_secret used for an outbound Authorization header), so logger.<level>(error) landed those credentials verbatim in hdb.log at default level — retrievable via read_log.

Two mechanisms, matched to the two leak surfaces:

1. Logger auto-wrap (the invariant). The HarperLogger level methods — plus the inherited log/dir/table, which also bypass util.inspect's guard otherwise — now route every Error argument through errorForLog() (sanitizeErrorArgs) inside the level gate. This covers every logger call — all first-party code, future code, and component/app code holding a server.logger — with no per-site changes needed. Gated-out calls pay nothing beyond an arg scan; allocation only happens when an Error is present; a revoked-Proxy arg can't make the logger throw (isErrorLike try/catch).
2. Explicit wraps on console.* calls, scoped by error provenance (4 sites). harper_logger captures both stdout and stderr into hdb.log, but Node's Console runs util.inspect before the capture hook — so console.error(rawError) leaks identically. A sanitizing console subclass was rejected (overriding console methods makes the inspector report the override frame instead of the real call site), so wrapping is per-site — and scoped to where the error can actually originate in app/library code: JSONStream (serializes resource iterator output — the origin-fetch axios scenario of server/REST.ts logs the raw Error object to hdb.log — leaks secrets/sensitive data attached to escaping Errors #1734 flows through here), socketRouter's generic uncaughtException, componentLoader app-load failures, loadRootComponents' npm/pacote install of user packages. Sites with a closed error set (fs/LMDB/pm2/config/our own code) stay raw — those errors cannot carry stashed credentials.

Additionally, a small allowlist of diagnostic Error properties — code, status, statusCode, errno, syscall — is now rendered alongside the stack/cause chain. These are common triage fields (Node error codes, HTTP status) that libraries don't use to carry secrets, unlike arbitrary properties (axios' config/request), which stay excluded. path is deliberately not included — it can reveal internal filesystem layout and the message/stack already names the failing operation.

History

This is a clean transplant of kris/log-error-leak-sweep (formerly #1744) onto current main. #1744's base branch (kris/rest-log-error-leak-1734, the primitive-adding PR #1737) was squash-merged, so the original branch's commit history no longer rebases cleanly against main — reopening #1744 as-is would show REST.ts/http.ts/graphqlQuerying.ts as changed again, redundantly re-diffing #1737's already-merged content. This PR reapplies the sweep's net diff directly on top of the squashed main; #1744 is closed as superseded. It also folds in two #1737 review comments that were left unresolved when that PR merged (node:util import scope; move errorToString's null-guard into errorToString itself — see commit messages) and adds the diagnostic-property allowlist per a follow-up request after #1744 was mistakenly closed instead of merged.

Where to look

• utility/logging/harper_logger.ts — sanitizeErrorArgs/isErrorLike (the auto-wrap) and errorToLogString/loggablePropsSuffix (the allowlist) are the load-bearing changes. Detection is instanceof Error || util.types.isNativeError (cross-realm safe), wrapped in try/catch for exotic objects (revoked Proxy) where even instanceof can throw.
• The console-site wraps (JSONStream.ts, socketRouter.ts, componentLoader.ts, loadRootComponents.js) — mechanical; only the error arg gets errorForLog(...)/harperLogger.errorForLog(...), message strings/data args untouched. JSONStream's errorToString response pushes stay raw (client-facing body, not a log line) and are unaffected by the allowlist (allowlist only applies inside errorToLogString, used for log lines).
• Sanitization is deliberately shallow: an Error nested inside a logged object/array (logger.error({ err })) is not rewritten — deep-walking every logged structure isn't worth the per-call cost, and the server/REST.ts logs the raw Error object to hdb.log — leaks secrets/sensitive data attached to escaping Errors #1734 threat is raw thrown errors escaping directly to the logger.

Known gap (accepted)

Component/app code calling global console.error(err) directly (not through Harper's logger) still leaks via the stdio capture — only our own console sites are wrapped. Fixing that class requires the console subclass rejected above for inspector-ergonomics reasons, or an upstream-Node-level answer.

Testing

44 unit tests (was 9 before this work started): auto-wrap (sole-arg/any-position/cause-chain secret exclusion, non-Error objects untouched, level gating preserved, loggerWithTag coverage, log/dir/table bypass closed, revoked-Proxy no-throw) + the diagnostic-property allowlist (code/status/statusCode/errno/syscall surfaced, path excluded, siblings-still-excluded, cause-chain coverage). Built to dist and ran against it (mocha's --conditions=typestrip path can silently fall back to stale dist): 44 passing. oxlint + prettier clean.

Cross-model review

Thorough mode ran on an earlier iteration of this sweep (Codex found 3 missed sites + the log/dir/table/revoked-Proxy issues fixed in the auto-wrap design here; the Harper domain pass found the console-capture surface and non-standard logger bindings that informed this design). Gemini (agy) leg timed out on both attempts — no second-model coverage from Gemini on this iteration.

Generated by an LLM (Claude Sonnet 5 / Opus 4.8 / Fable 5 — multi-session).

[claude]

Reviewed; no blockers found.

[cb1kenobi]

Nit: diagnostic props are glued onto the last stack frame

base is the full multi-line error.stack, so base + loggablePropsSuffix(error) appends code=ENOENT errno=-2 syscall=open to the last stack-frame line, e.g. at open (node:fs:…) code=ENOENT errno=-2. Diagnostic fields read more clearly on their own line (they're the triage signal, and this keeps them off a frame that looks like part of the trace).

Suggested fix:

Suggested change

	return base + loggablePropsSuffix(error);
	const suffix = loggablePropsSuffix(error);
	return suffix ? `${base}
	${suffix.trimStart()}` : base;

—
Generated by Barber AI