Skip to content

fix(runtime): stop bearer token leak in error logs (TD-9349) - #1210

Closed
VDubber wants to merge 3 commits into
mainfrom
TD-9349-sanitize-error-serializer
Closed

VDubber wants to merge 3 commits into
mainfrom
TD-9349-sanitize-error-serializer

Conversation

@VDubber

@VDubber VDubber commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the credential leak reported in TD-9349: during sustained 5xx errors from the persister/upload API, an integration job logged a live JupiterOne API bearer token in plaintext.

Root cause. The bunyan err serializer inspects errors with util.inspect(err, false, 10). For axios/@lifeomic/alpha errors, the error retains a live ClientRequest and its attached TLS socket; a depth-10 inspect walks into the socket and reaches the request Authorization header via a Node-internal socket property (Symbol(connect-options)), plus request._header / Symbol(kOutHeaders). Existing redaction (api/index.ts interceptor, cleanAxiosError) only patched known object paths and never reached the socket.

This is a recurrence of the class behind TD-8691 (FleetDM, gaxios) / TD-8711. Fixing it at the serialization choke point covers gaxios, axios, and alpha for every integration at once.

What changed

  • logger/sanitizeError.ts (new): cycle-guarded, depth-bounded, non-mutating clone that tags live transport objects (Socket/TLSSocket/Agent/ClientRequest/streams/Buffer) so they are never walked, redacts credential-bearing keys (string and symbol), and scrubs inline Bearer/Authorization in strings.
  • logger/index.ts: the err serializer now sanitizes before inspect. The old guard if (!err || !err.stack) return err returned real alpha errors raw (they have no .stack) — now it only passes through primitives and always sanitizes error-like objects. Also registers a serializer for the err.$response field, which the upload path logs separately and which bypasses the err serializer.
  • synchronization/index.ts: cleanAxiosError additionally detaches request.socket/connection and response.request (defense-in-depth).

Testing / evidence

  • sanitizeError.test.ts — unit coverage (key/symbol redaction, string scrub, cycle safety, Error field preservation, live-socket).
  • credentialLeak.test.ts — logs a live-socket error under { err } through a real createIntegrationLogger; asserts the emitted record is clean. Verified to fail without the fix.
  • credentialLeakE2E.test.ts — drives the real uploadDataChunk through the real @lifeomic/alpha client over TLS against a mock persister returning 503s, and asserts the bytes bunyan actually writes contain no token. This mock-API run caught a gap the unit tests missed: real alpha AxiosErrors have no .stack, so the original guard returned them raw. Verified to fail without the fix; the serializer alone (without the cleanAxiosError change) is sufficient.

Full integration-sdk-runtime typecheck + logger/synchronization suites pass.

Follow-ups (not in this PR)

  • err.$response is defensive for AWS-SDK-v2-shaped errors; alpha never sets it today.
  • Longer-term: migrate the SDK's internal client off @lifeomic/alpha/axios to undici (aligns with @jupiterone/http-client).

🤖 Generated with Claude Code

VDubber and others added 3 commits September 30, 2026 17:11
…en leak (TD-9349)

The bunyan `err` serializer inspected errors with util.inspect(err, false, 10).
For axios/gaxios-family errors, the error retains a live ClientRequest and its
attached TLS socket; a depth-10 inspect walks into the socket and reaches the
request Authorization header via Symbol(connect-options), leaking the JupiterOne
API bearer token into logs. Existing redaction only patched known object paths
(config.headers, request._header, kOutHeaders) and never reached the socket.

Fix at the serialization choke point every integration logs through:

- Add logger/sanitizeError.ts: a cycle-guarded, depth-bounded, non-mutating
  clone that tags live transport objects (Socket/TLSSocket/Agent/ClientRequest/
  streams/Buffer) so Symbol(connect-options) is never walked, redacts
  credential-bearing keys (string and symbol), and scrubs inline Bearer/
  Authorization occurrences in strings.
- Serialize sanitizeError(err) before inspect() in the err serializer.
- Defense-in-depth: cleanAxiosError also detaches request.socket/connection and
  response.request.

Closes the leak class across gaxios, axios, and @lifeomic/alpha at one place.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Builds a real logger via createIntegrationLogger, logs a live-socket axios-style
error under { err } exactly as handleUploadDataChunkError does, and asserts the
emitted RingBuffer record contains no bearer token. Verified to fail against the
pre-fix raw inspect(err, false, 10) serializer (token leaked via request._header
and Symbol(kOutHeaders)), and pass with sanitizeError in place.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The mock-API validation against the real @lifeomic/alpha client surfaced two
gaps the unit tests (which used stack-bearing Error objects) missed:

1. Real alpha AxiosErrors have no `.stack`. The err serializer's guard
   `if (!err || !err.stack) return err` returned the raw error untouched, so
   sanitizeError never ran and bunyan JSON-serialized the live socket, leaking
   the bearer token. The guard now only passes through primitives and always
   sanitizes error-like objects.
2. The synchronization upload path logs the raw response under the dedicated
   `err.$response` field, which bypasses the `err` serializer. Register a
   serializer for that field too.

Adds an end-to-end test that drives uploadDataChunk through the real alpha
client over TLS against a mock persister returning 503s, and asserts the bytes
bunyan actually writes contain no bearer token. Verified to fail without the fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@VDubber

VDubber commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Not correct approach

@VDubber VDubber closed this Oct 7, 2026
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