Conversation
size-limit report 📦
|
|
bugbot run |
Resolved conflicts in packages/core/src/utils/request.ts (kept develop's shouldFilterDataKey-based header filtering alongside the new body filter imports; filterKeyValueData import dropped as unused) and in the express tracing integration test (kept develop's span-streaming assertions with this branch's '[Filtered]' body expectations). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e93cb39. Configure here.
logaretm
left a comment
There was a problem hiding this comment.
Nice one, some minor stuff. I had the clanker look at the regex and it didn't like it.
|
|
||
| /** | ||
| * Scrubs the values of known-sensitive keys in an HTTP body the SDK collected itself, before it |
There was a problem hiding this comment.
Bug: The filterQueryParams function incorrectly adds =[Filtered] to sensitive keys that originally have no value, altering the request's structure and semantics.
Severity: LOW
Suggested Fix
Modify the logic in filterQueryParams to check if the original pair contains an = character. If a sensitive key is found and the original pair was valueless (no =), it should be replaced with just the encoded key or a value that preserves its valueless nature, not ${encodedKey}=${FILTERED_VALUE}.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/utils/data-collection/filterHttpBody.ts#L23-L25
Potential issue: The `filterQueryParams` function unconditionally appends
`=${FILTERED_VALUE}` to any key identified as sensitive. This logic does not account for
valueless keys, such as boolean flags in a form body (e.g.,
`sensitive_flag&other_param=value`). When `sensitive_flag` is filtered, it is
incorrectly transformed into `sensitive_flag=[Filtered]`, which changes the semantics of
the request. This contradicts the stated goal of preserving the request's structure
during data collection and could lead to incorrect server-side processing for APIs that
rely on valueless parameters.
| } | ||
|
|
||
| // `Object.fromEntries` instead of assigning `result[key]`, so user-controlled keys like | ||
| // `__proto__` never hit a computed property write (CodeQL js/remote-property-injection). | ||
| return Object.fromEntries( | ||
| Object.entries(value).map(([key, nested]) => [ | ||
| key, | ||
| shouldFilterDataKey(key, true) ? FILTERED_VALUE : filterBodyValue(nested), | ||
| ]), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Bug: The recursive function filterBodyValue lacks a depth limit and try/catch block, leading to potential stack overflow crashes when processing deeply nested request bodies.
Severity: HIGH
Suggested Fix
Wrap the call to filterBodyValue within the filterCollectedHttpBody function in a try/catch block to handle potential RangeError exceptions from deep recursion. Alternatively, implement a depth limit within filterBodyValue to prevent stack overflows.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/utils/data-collection/filterHttpBody.ts#L66-L83
Potential issue: The function `filterBodyValue` is called recursively on request bodies
without a depth limit or proper error handling. When `httpRequestToRequestData`
processes a request with a deeply nested JSON body, it calls `filterBodyValue` on the
parsed object. This leads to unbounded recursion, which can cause a stack overflow
(`RangeError`) and crash the request handler, resulting in a denial of service for that
endpoint.
Also affects:
packages/core/src/integrations/http/patch-request-to-capture-body.ts:93~100packages/core/src/utils/request.ts:161~192

Automatically captured HTTP request bodies went out raw, and the browser
graphqlClientattached GraphQL documents with inline literals intact.
Bodies now run through a filter at capture time, before truncation, since a truncated JSON
body no longer parses. JSON and form bodies keep their shape, and values of sensitive keys
become
[Filtered]. A body without key-value structure passes through unchanged: the SDKonly scrubs values it can attribute to a specific sensitive key, since Relay scrubs
server-side anyway and cannot tell an SDK-filtered value from a literal one. The browser
GraphQL document gets the same literal redaction the server-side integration applies to
the parsed AST.
The model follows OTel's sanitization of
db.query.text: keep the structure, replace the values.Relay scrubs as well, see this related PR: getsentry/sentry-conventions#625
Request bodies (
event.request.data/http.request.body.data)httpBodiesdefault (on)httpBodies: [](off){"color":"blue","password":"hunter2"}{"color":"blue","password":"[Filtered]"}color=blue&access_token=abc123color=blue&access_token=[Filtered]{"note":"xxx…...)"Not captured" means no attribute at all.
GraphQL document (
graphql.documenton span and breadcrumb)graphQL.documentdefault (on)document: false(off)query { user(email: "jane@example.com", age: 42) { name } }query { user(email: "*", age: *) { name } }query GetUser($id: ID!) { user(id: $id) { name } }Part of #24081