From c031adb906de1e071f44f6dbf27e555caff9c5ac Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 12:07:38 +0000 Subject: [PATCH 1/3] docs: backfill shared Bugbot review rules Align .cursor/BUGBOT.md with cross-SDK review guidance and develop.sentry.dev philosophy/principles (PII defaults, never self-capture, host safety, deps, what-not-to-flag). Co-Authored-By: Stephanie Anderson --- .cursor/BUGBOT.md | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/.cursor/BUGBOT.md b/.cursor/BUGBOT.md index fa7fd035d48c..ebdf8e5421b5 100644 --- a/.cursor/BUGBOT.md +++ b/.cursor/BUGBOT.md @@ -5,19 +5,28 @@ Flag any of the following indicators or missing requirements. If you find anything to flag, mention that you flagged this in the review because it was mentioned in this rules file. Unless explicitly noted (e.g. in the `Testing Conventions` section), only flag the issues below in production code — ignore them in test files. +These rules operationalize the Sentry SDK [philosophy](https://develop.sentry.dev/sdk/getting-started/philosophy/) and [principles](https://develop.sentry.dev/sdk/getting-started/principles/): protect customer apps and data, prefer safe defaults, keep the base SDK lean, stay compatible, and never let SDK or callback failures become Sentry traffic or host crashes. + +Keep reviews high-signal. Prefer actionable, high-confidence findings over speculative warnings or drive-by refactors unrelated to the diff. + ## Critical Issues to Flag ### Security Vulnerabilities -- Exposed secrets, API keys, tokens or credentials in code or comments +- Exposed secrets, API keys, tokens, DSNs, or credentials in code, comments, logs, configs, or examples - Unsafe use of `eval()`, `Function()`, or `innerHTML` - Unsafe regular expressions that could cause ReDoS attacks +- PII or sensitive data attached by auto-instrumentation without an explicit opt-in (`sendDefaultPii` or equivalent). Flag new default logging/sending of request/response bodies, full URLs with query secrets, file paths, or device identifiers. +- Large or sensitive attachments enabled by default, or attachments lacking size limits / backoff +- Diagnostics, sampling overrides, verbose logging, or feature flags accidentally enabled in production defaults ### Breaking Changes - Public API changes without proper deprecation notices - Removal of publicly exported functions, classes, or types. Internal removals are fine! - Changes to function signatures in public APIs +- Silent changes to defaults, sampling, or feature toggles that affect existing apps without migration notes +- Raising minimum runtime/browser/Node support, or dropping a supported platform, without an explicit docs/changelog/migration callout ## SDK-relevant issues @@ -28,6 +37,14 @@ Unless explicitly noted (e.g. in the `Testing Conventions` section), only flag t - Large bundle size increases in browser packages. Sometimes they're unavoidable but flag them anyway. - Flag top-level side effects (function calls, mutations of module-level state, IIFEs) in modules that are reachable from a package's public entry points. Side effects defeat tree-shaking and inflate bundles for users who don't import the affected code. Pure exports (constants, classes, factory functions, side-effect-free declarations) are fine. +### Trust, defaults, and dependencies + +- Never let SDK init or instrumentation failures crash or brick the host application. Prefer graceful degrade / no-op on unsupported or outdated environments; if that is impossible, the break must be obvious at install/build time, not at runtime. +- Flag new runtime dependencies on the base SDK path. Integration-only optional dependencies are fine when justified; baseline deps increase license, maintenance, and supply-chain surface. +- Flag new required configuration or integrations left disabled by default when auto-enable would give a better out-of-the-box experience without surprising side effects. +- Flag heavy in-SDK business logic that permanently reshapes wire-format data when collecting rawer data and leaving transformation to the server would do. +- SDKs must never `captureException` / `captureMessage` (or equivalent) for exceptions thrown inside the SDK itself or inside user callbacks such as `beforeSend` / `before_send`, `tracesSampler`, or similar hooks. Swallow gracefully and emit an error-level SDK log instead — capturing here can loop and take down the process. See [Never capture your own exceptions](https://develop.sentry.dev/sdk/getting-started/principles/#never-capture-your-own-exceptions). + ### Auto instrumentation, SDK integrations, Sentry-specific conventions - When calling any `startSpan` API (`startInactiveSpan`, `startSpanManual`, etc), always ensure that the following span attributes are set: @@ -40,6 +57,7 @@ Unless explicitly noted (e.g. in the `Testing Conventions` section), only flag t - When calling `captureException`, always make sure that the `mechanism` is set: - `handled`: must be set to `true` or `false` - `type`: must be a proper identifier (i.e. identify the integration and part in the integration that caught the exception). The value should follow the same naming convention as `SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN`, and align with the surrounding span's origin when one exists. +- For automated structured logs, always set `sentry.origin` with a proper origin (same character rules as span origins). - When calling any `startSpan` API, check how errors in the instrumented code are handled: - Generally, errors in instrumented code should be allowed to bubble up so the end user can handle them. If they remain unhandled, they will eventually be captured by Sentry through the SDK's global error handlers — so instrumentation code should typically **not** call `captureException` itself. - Only consider calling `captureException` if the instrumentation prevents errors from bubbling up (e.g. by swallowing them in a `try/catch` or an error event listener). Doing so is generally discouraged — prefer to let the error propagate instead. @@ -64,8 +82,9 @@ Unless explicitly noted (e.g. in the `Testing Conventions` section), only flag t - When reviewing a `feat` PR, check if the PR includes at least one integration or E2E test. If neither is present, flag it and recommend adding one. - When reviewing a `fix` PR, check if the PR includes at least one unit, integration or E2E test that covers the regression this PR fixes. The test should fail without the fix and pass with it. If you cannot tell from the diff whether this is the case, ask the author to confirm. If no tests are present, flag it and recommend adding one. -- Check that tests actually test the newly added behaviour. +- Check that tests actually test the newly added behaviour. Tests must prove user-visible or SDK behavior, not merely coverage or "did not throw". For instance, when checking on sent payloads by the SDK, ensure that the newly added data is asserted thoroughly. + Hollow tests that look correct but assert nothing meaningful should be flagged. - Flag usage of `expect.objectContaining` and other relaxed assertions, when a test expects something NOT to be included in a payload but there's no respective assertion. - Flag usage of conditionals in one test and recommend splitting up the test for the different paths. - Flag usage of loops testing multiple scenarios in one test and recommend using `(it)|(test).each` instead. @@ -81,3 +100,11 @@ Unless explicitly noted (e.g. in the `Testing Conventions` section), only flag t - When any `setTimeout` or `setInterval` timers are started in a code path that can end up in server runtime packages (e.g. `@sentry/core` or `@sentry/node`), flag if neither `timeout.unref()` nor `safeUnref()` are called. Not unref'ing a timer can keep CLI-like applications or node scripts from exiting immediately, due to the process waiting on timers started by the SDK. - Flag Node-only imports (`fs`, `path`, `child_process`, `os`, `net`, `http`, `https`, `node:*`, etc.) in code paths that ship to non-Node runtimes (`@sentry/browser`, `@sentry/cloudflare`, `@sentry/deno`, `@sentry/bun`, or shared code in `@sentry/core` / `@sentry/browser-utils` reachable from those entry points). When the dependency is unavoidable, isolate it behind a runtime check, dynamic `import()`, or a Node-only entry point — never at the top level of a cross-runtime module. + +## What NOT to Flag + +- Pure style or formatting that linters/formatters already own (Oxlint, Oxfmt, etc.) +- Speculative refactors or "improvements" with no clear user benefit or linked motivation +- Idiomatic monkeypatching / low-level hooks solely because they are brittle — only flag when unsafe, non-idempotent, or harmful to the host app +- Test-only issues unless covered by `Testing Conventions` +- Conventional commit / PR title format when CI already validates it From 05c38830fbc159ddb007dcfa51936bea05f9db58 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:13:45 +0000 Subject: [PATCH 2/3] docs: clarify Bugbot secrets and dataCollection rules Address review feedback: don't flag bogus test DSNs, and replace outdated sendDefaultPii guidance with dataCollection-aware wording. --- .cursor/BUGBOT.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.cursor/BUGBOT.md b/.cursor/BUGBOT.md index ebdf8e5421b5..f1f8070e4353 100644 --- a/.cursor/BUGBOT.md +++ b/.cursor/BUGBOT.md @@ -13,10 +13,10 @@ Keep reviews high-signal. Prefer actionable, high-confidence findings over specu ### Security Vulnerabilities -- Exposed secrets, API keys, tokens, DSNs, or credentials in code, comments, logs, configs, or examples +- Exposed **real** secrets, API keys, tokens, or credentials in production code, comments, logs, or committed configs. Do **not** flag test fixtures, unit/integration/E2E helpers, or docs/examples that use obviously fake / placeholder DSNs and credentials (bogus DSNs in tests are expected). - Unsafe use of `eval()`, `Function()`, or `innerHTML` - Unsafe regular expressions that could cause ReDoS attacks -- PII or sensitive data attached by auto-instrumentation without an explicit opt-in (`sendDefaultPii` or equivalent). Flag new default logging/sending of request/response bodies, full URLs with query secrets, file paths, or device identifiers. +- Sensitive data collection must go through the SDK's `dataCollection` options (and helpers like `filterCollectedUrl` / `filterCollectedUrlQuery`) where applicable. Flag new instrumentation that attaches request/response bodies, cookies, headers, URL query params, user info, DB query data, GraphQL variables, GenAI I/O, or similar without honoring the relevant `dataCollection.*` knobs — or that hard-codes collection on when those options would disable or filter it. Prefer linking reviewers to the existing URL-attribute filtering rules below rather than inventing parallel guidance. - Large or sensitive attachments enabled by default, or attachments lacking size limits / backoff - Diagnostics, sampling overrides, verbose logging, or feature flags accidentally enabled in production defaults From c7c8f0492c31da691e3df726afd309ea0b246907 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:15:27 +0000 Subject: [PATCH 3/3] docs: drop generic attachment and hook exceptions Omit JS-inapplicable attachment defaults guidance, and allow flagging brittle monkeypatches when a better approach exists. --- .cursor/BUGBOT.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/.cursor/BUGBOT.md b/.cursor/BUGBOT.md index f1f8070e4353..9bf2d8040791 100644 --- a/.cursor/BUGBOT.md +++ b/.cursor/BUGBOT.md @@ -17,7 +17,6 @@ Keep reviews high-signal. Prefer actionable, high-confidence findings over specu - Unsafe use of `eval()`, `Function()`, or `innerHTML` - Unsafe regular expressions that could cause ReDoS attacks - Sensitive data collection must go through the SDK's `dataCollection` options (and helpers like `filterCollectedUrl` / `filterCollectedUrlQuery`) where applicable. Flag new instrumentation that attaches request/response bodies, cookies, headers, URL query params, user info, DB query data, GraphQL variables, GenAI I/O, or similar without honoring the relevant `dataCollection.*` knobs — or that hard-codes collection on when those options would disable or filter it. Prefer linking reviewers to the existing URL-attribute filtering rules below rather than inventing parallel guidance. -- Large or sensitive attachments enabled by default, or attachments lacking size limits / backoff - Diagnostics, sampling overrides, verbose logging, or feature flags accidentally enabled in production defaults ### Breaking Changes @@ -105,6 +104,5 @@ Keep reviews high-signal. Prefer actionable, high-confidence findings over specu - Pure style or formatting that linters/formatters already own (Oxlint, Oxfmt, etc.) - Speculative refactors or "improvements" with no clear user benefit or linked motivation -- Idiomatic monkeypatching / low-level hooks solely because they are brittle — only flag when unsafe, non-idempotent, or harmful to the host app - Test-only issues unless covered by `Testing Conventions` - Conventional commit / PR title format when CI already validates it