-
Notifications
You must be signed in to change notification settings - Fork 4
fix(catchers): validate beforeSend return value to avoid sending invalid payload #150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9bd6a90
f20a748
da99f8a
17f43e2
14cbeab
e79e9c3
c1a81f2
5505090
e02ffd5
4279d77
7e087f7
93e0bfb
d2b0812
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import type { Breadcrumb, BreadcrumbLevel, BreadcrumbType, Json, JsonNode } from | |
| import Sanitizer from '../modules/sanitizer'; | ||
| import { buildElementSelector } from '../utils/selector'; | ||
| import log from '../utils/log'; | ||
| import { isValidBreadcrumb } from '../utils/validation'; | ||
|
|
||
| /** | ||
| * Default maximum number of breadcrumbs to store | ||
|
|
@@ -48,11 +49,13 @@ export interface BreadcrumbsOptions { | |
| maxBreadcrumbs?: number; | ||
|
|
||
| /** | ||
| * Hook called before each breadcrumb is stored | ||
| * Return null to discard the breadcrumb | ||
| * Return modified breadcrumb to store it | ||
| * Hook called before each breadcrumb is stored. | ||
| * - Return modified breadcrumb — it will be stored instead of the original. | ||
| * - Return `false` — the breadcrumb will be discarded. | ||
| * - Return nothing (`void` / `undefined` / `null`) — the original breadcrumb is stored as-is (a warning is logged). | ||
| * - If the hook returns an invalid value, a warning is logged and the original breadcrumb is stored. | ||
| */ | ||
| beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb | null; | ||
| beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb | false | void; | ||
|
|
||
| /** | ||
| * Enable automatic fetch/XHR breadcrumbs | ||
|
|
@@ -91,7 +94,7 @@ interface InternalBreadcrumbsOptions { | |
| trackFetch: boolean; | ||
| trackNavigation: boolean; | ||
| trackClicks: boolean; | ||
| beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb | null; | ||
| beforeBreadcrumb?: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => Breadcrumb | false | void; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -233,16 +236,30 @@ export class BreadcrumbManager { | |
| * Apply beforeBreadcrumb hook | ||
| */ | ||
| if (this.options.beforeBreadcrumb) { | ||
| const result = this.options.beforeBreadcrumb(bc, hint); | ||
| const breadcrumbClone = structuredClone(bc); | ||
| const result = this.options.beforeBreadcrumb(breadcrumbClone, hint); | ||
|
|
||
| if (result === null) { | ||
| /** | ||
| * Discard breadcrumb | ||
| */ | ||
| /** | ||
| * false means discard | ||
| */ | ||
| if (result === false) { | ||
| return; | ||
| } | ||
|
Comment on lines
+242
to
247
|
||
|
|
||
| Object.assign(bc, result); | ||
| /** | ||
| * Valid breadcrumb → apply changes from hook | ||
| */ | ||
| if (isValidBreadcrumb(result)) { | ||
| Object.assign(bc, result); | ||
| } else { | ||
| /** | ||
| * Anything else is invalid — warn, bc stays untouched (hook only received a clone) | ||
| */ | ||
| log( | ||
| 'Invalid beforeBreadcrumb value. It should return breadcrumb or false. Breadcrumb is stored without changes.', | ||
| 'warn' | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,23 +2,23 @@ import Socket from './modules/socket'; | |||||
| import Sanitizer from './modules/sanitizer'; | ||||||
| import log from './utils/log'; | ||||||
| import StackParser from './modules/stackParser'; | ||||||
| import type { CatcherMessage, HawkInitialSettings, BreadcrumbsAPI } from './types'; | ||||||
| import type { CatcherMessage, HawkInitialSettings, BreadcrumbsAPI, Transport } from './types'; | ||||||
| import { VueIntegration } from './integrations/vue'; | ||||||
| import { id } from './utils/id'; | ||||||
| import type { | ||||||
| AffectedUser, | ||||||
| EventContext, | ||||||
| JavaScriptAddons, | ||||||
| VueIntegrationAddons, | ||||||
| Json, EncodedIntegrationToken, DecodedIntegrationToken, | ||||||
| Json, EncodedIntegrationToken, DecodedIntegrationToken | ||||||
| } from '@hawk.so/types'; | ||||||
| import type { JavaScriptCatcherIntegrations } from './types/integrations'; | ||||||
| import { EventRejectedError } from './errors'; | ||||||
| import type { HawkJavaScriptEvent } from './types'; | ||||||
| import { isErrorProcessed, markErrorAsProcessed } from './utils/event'; | ||||||
| import { ConsoleCatcher } from './addons/consoleCatcher'; | ||||||
| import { BreadcrumbManager } from './addons/breadcrumbs'; | ||||||
| import { validateUser, validateContext } from './utils/validation'; | ||||||
| import { validateUser, validateContext, isValidEventPayload } from './utils/validation'; | ||||||
|
|
||||||
| /** | ||||||
| * Allow to use global VERSION, that will be overwritten by Webpack | ||||||
|
|
@@ -73,16 +73,18 @@ export default class Catcher { | |||||
| private context: EventContext | undefined; | ||||||
|
|
||||||
| /** | ||||||
| * This Method allows developer to filter any data you don't want sending to Hawk | ||||||
| * If method returns false, event will not be sent | ||||||
| * This Method allows developer to filter any data you don't want sending to Hawk. | ||||||
| * - Return modified event — it will be sent instead of the original. | ||||||
| * - Return `false` — the event will be dropped entirely. | ||||||
| * - Any other value is invalid — the original event is sent as-is (a warning is logged). | ||||||
| */ | ||||||
| private readonly beforeSend: undefined | ((event: HawkJavaScriptEvent) => HawkJavaScriptEvent | false); | ||||||
| private readonly beforeSend: undefined | ((event: HawkJavaScriptEvent) => HawkJavaScriptEvent | false | void); | ||||||
|
||||||
| private readonly beforeSend: undefined | ((event: HawkJavaScriptEvent) => HawkJavaScriptEvent | false | void); | |
| private readonly beforeSend: undefined | ((event: HawkJavaScriptEvent) => HawkJavaScriptEvent | false | void | null); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||
| import type { EventContext, AffectedUser } from '@hawk.so/types'; | ||||||
| import type { HawkJavaScriptEvent } from './event'; | ||||||
| import type { Transport } from './transport'; | ||||||
| import type { BreadcrumbsOptions } from '../addons/breadcrumbs'; | ||||||
|
|
||||||
| /** | ||||||
|
|
@@ -65,10 +66,11 @@ export interface HawkInitialSettings { | |||||
|
|
||||||
| /** | ||||||
| * This Method allows you to filter any data you don't want sending to Hawk. | ||||||
| * | ||||||
| * Return `false` to prevent the event from being sent to Hawk. | ||||||
| * - Return modified event — it will be sent instead of the original. | ||||||
| * - Return `false` — the event will be dropped entirely. | ||||||
| * - Any other value is invalid — the original event is sent as-is (a warning is logged). | ||||||
| */ | ||||||
| beforeSend?(event: HawkJavaScriptEvent): HawkJavaScriptEvent | false; | ||||||
| beforeSend?(event: HawkJavaScriptEvent): HawkJavaScriptEvent | false | void; | ||||||
|
||||||
| beforeSend?(event: HawkJavaScriptEvent): HawkJavaScriptEvent | false | void; | |
| beforeSend?(event: HawkJavaScriptEvent): HawkJavaScriptEvent | false | void | null; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import type { CatcherMessage } from './catcher-message'; | ||
|
|
||
| /** | ||
| * Transport interface — anything that can send a CatcherMessage | ||
| */ | ||
| export interface Transport { | ||
| send(message: CatcherMessage): Promise<void>; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSDoc says
beforeBreadcrumbcan returnnull, and the runtime path handlesnull, but the callback type isBreadcrumb | false | void(nonull). Align the public type signature with the documented/runtime behavior (either addnullor removenullfrom docs and handling).