Cap logged structured error details at 256K characters and mask credential-shaped fields in them

[kriszyp]

When an operation fails with a structured http_resp_msg, Harper logs it through inspectForLog at raised inspect limits (depth 8, 250 entries, 20,000-character strings). This PR bounds that render to 256 Ki characters in total — one log entry previously reached ~134M characters — and masks values stored under credential-shaped keys (password, authorization, *_secret_access_key, token, api_key, cookie, private_key, …) at any depth. The same renderer derives HdbError.message for structured non-report messages, so those are capped and masked too.

Not covered, by design: masking is structural, so a custom inspect hook that returns a secret as a plain string, a non-string Map key, and rawHeaders-style [name, value] arrays are not masked; HdbError's report flattening stays unmasked (entry 4).

For the human reviewer

1. The requirement. Warranted as specified, confirmed on main: a render of 199 objects × 249 fields of one shared 20 KB string produced 134,554,518 characters in ~850 ms, stopped only by util.inspect's own internal ~2^27-character limit, and a nested { headers: { Authorization } }, aws_secret_access_key or password rendered verbatim. One deviation from the task text: the budget counts characters, not bytes, because string length is what can be measured without an encoding pass; a capped entry is at most 768 KiB as UTF-8. No other in-flight work supersedes this.
2. Masking lives in the shared renderer, not only at the log call site. Because HdbError derives message through inspectForLog, a structured non-report error message now shows [redacted] and is capped in what API clients receive (errorToString, HTTP error bodies, get_job). The alternative was masking only at OperationFunctionCaller; chosen because that same message is also what gets logged. Reversible by moving masking to the call site.
3. One credential-key list, shared and extended. Reused the MCP audit log's substring regex rather than adding a third list; it is now exported from the logger as CREDENTIAL_KEY_PATTERN and gained cookie, private_key and passphrase, which also masks those in MCP audit records (an add_certificate-style private_key argument was previously logged). It over-masks by design: author, max_tokens and similar substrings are masked, numbers included; only null, undefined and booleans pass through, so token: undefined still shows. Rejected: UNLOGGABLE_OPERATION_FIELDS (exact names, including generic value/payload that a deep walk would erase) and decisionStore's hash-exclusion regex (its .*key matches primaryKey). A one-line change either way.
4. HdbError report reasons stay unmasked. messageText flattens { error, ...reasons } before inspectForLog sees it, and those values are validation reasons — often under a field literally named password ("Password must be at least 8 characters"). Masking there would erase them; the cost is that a caller putting a real secret into a report sends it to err.message. Pinned by a test; a follow-up issue if report objects can carry secrets.
5. Custom inspect hooks now run inside the sanitize walk. Own and inherited hooks on objects are called once, in walk order, where util.inspect would call them — own hooks with the original as this, inherited ones with the sanitized clone, both as before — and their output is walked, masked and charged like any value. This closes a pre-existing gap: a prototype hook's output was printed raw. Hooks carried by opaque built-ins and functions (a Date carrying a hook, a class's static hook, a Uint8Array subclass) are resolved the same way, so a hook that hides data keeps hiding it and one returning credentials is masked; Node's own Buffer hook renders natively. The alternative was never running application hooks on the log path, which would print class instances by their properties only.
6. The walk tracks util.inspect's layout. For the estimate to bound work, the walk charges per-entry overhead, charges repeat references again, walks only to the depth and maxArrayLength inspect prints, and does not enumerate a container inspect collapses to [ClassName] — so an empty object at that level now prints [Object] instead of {}. Properties walked only to serve as an inherited hook's this are refunded when its output replaces them, against a separate cap of four budgets; past that cap, further such objects print as an omission while other fields keep rendering. All of this assumes util.inspect's rendering rules; if Node changes them the estimate drifts, while the exact final cut still holds the 256 Ki cap. Simpler alternative rejected: charge only leaf lengths and rely on the final cut, which leaves the work unbounded (the 134M-character intermediate still gets built).
7. Accepted limit: one oversized leaf per budget — an Error with a short message but a huge stack, a huge function name, or a hook's huge thrown-error text — is built in full before it is cut; that is linear in data the value already holds, and the budget is spent after it. An Error whose message alone exceeds the budget is rendered from its message without reading its stack.
8. Planning review: Framing-Verdict: chosen-approach-sound (plan round, Codex). The 256 Ki cap is a constant rather than configuration; the largest known legitimate payload (deployComponent's install_output) is capped at 16 KB.

Product and architecture tour

Two-part bound on one log entry

Why both an estimate in the walk and an exact cut at the end?

Before	After
199 objects × 249 fields of one 20 KB string: 134,554,518 characters in one entry, ~850 ms, a nested Authorization printed verbatim.	≤ 262,144 characters, the walk stops each open container with one [N more … omitted (sanitize budget)] marker, credential-keyed values print [redacted].

The walk's estimate bounds the work, because it runs before util.inspect builds anything; the final cut bounds the length exactly, because inspect's layout and escaping cannot be predicted from the walk.

From payload to log line

Every value is sanitized and charged in the walk, in the order inspect prints it; inspect then formats only sanitized clones and pre-rendered text, and the result is cut to the cap.

flowchart LR
    A[Structured payload] --> B[Sanitize walk]
    B -->|charge per entry| C[Render budget]
    B -->|call hooks, walk output| B
    B --> D[util.inspect]
    D --> E[Exact cut to 256 Ki]
    E --> F[Log line]

Loading

What the walk treats as rendered

Which values are charged, and which are never walked?

Errors, opaque built-ins and functions are rendered to text in the walk and charged exactly. Custom inspect hooks, own or inherited, run in the walk and their output is walked like any value. Containers below the caller's depth + 1 are never walked, and a container at that level prints [ClassName] without being enumerated. Repeat references are charged again because inspect prints a shared sub-object at every occurrence.

Changes

• utility/logging/harper_logger.ts: the cap (MAX_LOG_RENDER_LENGTH) is applied on every path in inspectForLog, whose budget walks arrays only to the printed maxArrayLength. The walk skips what inspect never prints (below depth + 1, collapsed containers), charges repeat references, and stops before an oversized key. Hooks are resolved in the walk for objects (own, inherited) and leaves; properties walked for an inherited hook are refunded but capped by MAX_DISCARDED_RENDER. Masking is isCredentialEntry, applied to object keys and Map keys with the shared CREDENTIAL_KEY_PATTERN. errorToLogString skips the stack when the message alone exceeds the budget and stops appending causes past it. Look hardest at the hook resolution: it decides what application code runs during logging.
• components/mcp/audit.ts: imports the shared pattern and uses it in place of its local copy.
• utility/DESIGN.md — the invariant note; DESIGN.md — its index line.

Verification

• unitTests/utility/logging/harper_logger.test.js — 24 new tests in the inspectForLog block: the 134M-character shape renders within 256 Ki and ends as a whole structure (walk-truncated, not cut); escape-heavy output still meets the exact cap; hooks share one budget, including hooks nested in hook output, and resolve in walk order; inherited hook output is masked and charged, and a short summary survives large replaced properties; replaced properties across 250 siblings read 8 stacks, not 500, and fields after the cap still render; a hook on a built-in leaf is walked — masked when it returns credentials, kept when it hides data (SecretKey(***)), including a replaced Buffer hook; containers collapsed at inspect depth are not walked; arrays are walked only to the printed maxArrayLength; a huge-message Error's stack is never read; oversized keys stop their container; repeat references, Errors, typed arrays and Map keys are charged; masking at depth ≥ 6 in plain objects, class instances, Maps and symbol keys, with token: undefined and authorized: false still shown.
• unitTests/utility/OperationFunctionCaller.test.js — the real operation-caller log path emits a bounded, masked payload that keeps deployment_id.
• unitTests/utility/errors/hdbError.test.js — a non-report structured message is masked; a report reason under password is kept.
• Fails on base: 19 of the first 22 new tests failed against an origin/main build; the 3 that pass there guard behavior that must not change (report reasons, below-depth rendering, shared-object reuse). The ones added during review each target a finding and failed on the commit before its fix.
• Gates: test:unit:main 6322 passing, 0 failing (dfc88e3, run with the dispatch worker's GIT_CONFIG_GLOBAL/GIT_EDITOR/GIT_SSH_COMMAND unset — they fail gitCredentials.test.js locally only); test:unit:resources 3798 passing, 0 failing (133c30d; later commits touch only the logger); test:integration:all 2283 pass, 0 fail, 6 cancelled — all six the OllamaBackend suite, which needs models this machine lacks (0601c0b). Format, oxlint --deny-warnings on changed files, and check:design-docs clean.
• End-to-end route: the logged line is exactly util.inspect(inspectForLog(…)), which the unit tests call directly and through callOperationFunctionAsAwait; no separate integration test.

Signed: Claude Opus 5.5

🤖 Generated with Claude Code

https://claude.ai/code/session_013rVw4rPvxkKS6HkrR5z3xy

Release target: main — no backport evidence (8 changed lines predate this shallow clone's history) and no linked issue to carry a milestone from
Related PRs: #562 overlaps (its cached diff touches harper_logger.ts's export surface, which now carries CREDENTIAL_KEY_PATTERN), #3064 overlaps (adds agent.maxTokens, which this PR's substring pattern masks), #3078 overlaps (same utility/DESIGN.md "Read this when" line), 15 others independent
Complexity: complicated

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-composer,cursor-kimi,cursor-muse; rounds=12; full=3 @ e90c7d6}

_{Review-Attention: read ~6m (decisions: document-hook-gaps, substring-credential-match, shared-credential-list, mask-type-exemptions, hooks-in-walk, render-cap-size, do-less-alternative) @ e90c7d6}

[gemini-code-assist]

Code Review

This pull request refactors inspectForLog in harper_logger.ts to enforce a cumulative rendering budget (MAX_LOG_RENDER_LENGTH) across the entire object structure, preventing excessive log sizes. It also unifies credential masking using a shared CREDENTIAL_KEY_PATTERN and improves error logging by avoiding large stack materialization when the budget is exceeded. The feedback points out a potential TypeError in renderErrorLine when accessing error.constructor.name on objects with a null prototype, suggesting the use of optional chaining to safely fall back to a default name.