[Nanobot] Task #spider_gh_bounty_19790: Title: Security Bug Report - Bun respons...#19792
Conversation
| } | ||
| } | ||
| } | ||
| return options.beforeEvent ? options.beforeEvent(event) : event; |
There was a problem hiding this comment.
Bug: The init function incorrectly references options.beforeEvent instead of the correct options.beforeSend, causing user-provided beforeSend callbacks to be silently ignored.
Severity: HIGH
Suggested Fix
In packages/bun/src/sdk.ts on line 21, change the reference from options.beforeEvent to options.beforeSend to correctly chain to the user-provided callback.
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/bun/src/sdk.ts#L21
Potential issue: In the `init` function, a custom `beforeSend` handler is defined to
sanitize headers. This handler attempts to chain to a user-provided callback by checking
for `options.beforeEvent`. However, the correct Sentry SDK option is
`options.beforeSend`. As `options.beforeEvent` will always be undefined, any
`beforeSend` callback provided by the user during initialization will be silently
ignored, preventing them from modifying or filtering events before they are sent.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| } | ||
| } | ||
| return options.beforeEvent ? options.beforeEvent(event) : event; | ||
| }, |
There was a problem hiding this comment.
User's beforeSend callback silently overwritten
High Severity
The init function defines its own beforeSend after spreading ...options, which silently overwrites any user-provided beforeSend callback. The code then attempts to chain to options.beforeEvent, but beforeEvent is not a valid Sentry option — the correct property name is beforeSend. Since the user's original beforeSend has already been replaced, it can never be invoked, effectively dropping user-defined event filtering/modification entirely.
| if (headers[header]) { | ||
| headers[header] = '[Filtered]'; | ||
| } | ||
| } |
There was a problem hiding this comment.
Case-sensitive header matching misses mixed-case headers
High Severity
The beforeSend sanitization in sdk.ts checks for headers using exact lowercase keys (e.g., set-cookie, authorization) via direct property access. HTTP headers commonly use mixed case like Set-Cookie or Authorization, so these won't match and sensitive values will leak unsanitized into events. The sanitizeHeaders function in http.ts correctly uses key.toLowerCase(), but the beforeSend logic does not.
| beforeSend(event) { | ||
| if (event.contexts?.response?.headers) { | ||
| const headers = event.contexts.response.headers as Record<string, string | string[] | undefined>; | ||
| const sensitiveHeaders = ['set-cookie', 'cookie', 'authorization']; |
There was a problem hiding this comment.
Inconsistent sensitive header lists between sanitization paths
Medium Severity
Two separate sanitization mechanisms exist with different header lists. The SENSITIVE_HEADERS set in http.ts includes six headers (www-authenticate, proxy-authorization, proxy-authenticate in addition to the base three), while the sensitiveHeaders array in sdk.ts beforeSend only lists three (set-cookie, cookie, authorization). This inconsistency means sensitive authentication-related headers may not be consistently sanitized depending on which path processes the event.
Additional Locations (1)
| 'proxy-authenticate', | ||
| ]); | ||
|
|
||
| function sanitizeHeaders(headers: Record<string, string | string[] | undefined> | undefined): Record<string, string | string[] | undefined> | undefined { |
There was a problem hiding this comment.
sanitizeHeaders not exported but imported by test
Medium Severity
The sanitizeHeaders function in http.ts is declared without the export keyword, but the test file insomnia__sanitization.test.ts imports it as a named export. This means all tests for header sanitization will fail, providing no verification for the security fix this PR introduces.
Additional Locations (1)
|
|
||
| export const httpIntegration = (): IntegrationFn => { | ||
| return { | ||
| name: 'Http', |
There was a problem hiding this comment.
Integration name collision replaces real HTTP instrumentation
High Severity
The new sanitization integration uses name: 'Http', which is identical to the existing node httpIntegration's INTEGRATION_NAME. Sentry's filterDuplicates deduplicates integrations by name, keeping the last one. When defaultIntegrations combines getDefaultIntegrations() (which includes the real Http integration) with this custom httpIntegration(), the sanitization-only stub replaces the real HTTP instrumentation integration, losing all HTTP request/response tracking.


自动化提交说明 (Automated PR)
Changed Files:
packages/bun/src/integrations/http.tspackages/bun/src/sdk.tspackages/bun/src/insomnia__sanitization.test.ts此 PR 由 AGI-Life-Engine 智能体生成,提供真实的业务逻辑代码交付。如有调整建议,可直接评论。