Repository navigation
Conversation
…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>
Contributor
Author
|
Not correct approach |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
errserializer inspects errors withutil.inspect(err, false, 10). For axios/@lifeomic/alphaerrors, the error retains a liveClientRequestand its attached TLS socket; a depth-10 inspect walks into the socket and reaches the requestAuthorizationheader via a Node-internal socket property (Symbol(connect-options)), plusrequest._header/Symbol(kOutHeaders). Existing redaction (api/index.tsinterceptor,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 inlineBearer/Authorizationin strings.logger/index.ts: theerrserializer now sanitizes before inspect. The old guardif (!err || !err.stack) return errreturned 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 theerr.$responsefield, which the upload path logs separately and which bypasses theerrserializer.synchronization/index.ts:cleanAxiosErroradditionally detachesrequest.socket/connectionandresponse.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 realcreateIntegrationLogger; asserts the emitted record is clean. Verified to fail without the fix.credentialLeakE2E.test.ts— drives the realuploadDataChunkthrough the real@lifeomic/alphaclient 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 alphaAxiosErrors have no.stack, so the original guard returned them raw. Verified to fail without the fix; the serializer alone (without thecleanAxiosErrorchange) is sufficient.Full
integration-sdk-runtimetypecheck + logger/synchronization suites pass.Follow-ups (not in this PR)
err.$responseis defensive for AWS-SDK-v2-shaped errors; alpha never sets it today.@lifeomic/alpha/axios to undici (aligns with@jupiterone/http-client).🤖 Generated with Claude Code