[DO NOT MERGE] feat(core): add opt-in assertion instrumentation - #6592
[DO NOT MERGE] feat(core): add opt-in assertion instrumentation#6592antonis wants to merge 11 commits into
11 issues
High
Unprotected Number coercion in formatMessage can throw TypeError on the reporting path - `packages/core/src/js/assertion.ts:158-162`
Number(arg) in %d/%i/%f branches throws for Symbols and BigInts, crashing the assertion reporting path that is documented to never throw. Wrap these conversions in try-catch like the adjacent %j branch.
Error constructor capture breaks on top-level Error declarations - `packages/core/test/tools/sentryAssertionBabelPlugin.test.ts:39-45`
The hoisted var _Error = Error; alias at program top fails when a module declares a top-level let, const, or var named Error, causing a ReferenceError on load or a TypeError when the assertion fires.
Also found at:
packages/core/src/js/tools/sentryBabelTransformerUtils.ts:5
Medium
Hoisted `Error` alias breaks on module-level `Error` declarations - `packages/core/src/js/tools/sentryAssertionBabelPlugin.ts:544`
The var _Error = Error injected at program top captures module-level Error bindings (e.g. an import { Error } or let/var Error), so new _Error() later throws TypeError instead of creating the intended assertion error.
Tests leak `siteId` entries into module-level `reportedSites` set - `packages/core/test/assertion.test.ts:95`
Tests that pass siteId without jest.isolateModules permanently add entries to the shared module-level reportedSites set, making the suite order-dependent and brittle against future additions that reuse a call-site id.
formatMessage crashes reporting path on non-coercible format args - `packages/core/src/js/assertion.ts:158-162`
The %d, %i, and %f format specifiers call Number(arg) without a try-catch, so a Symbol, BigInt, or object with a throwing valueOf crashes the reporting path instead of reporting safely.
Also found at:
packages/core/src/js/assertion.ts:244packages/core/src/js/tools/sentryBabelTransformerUtils.ts:8packages/core/test/tools/sentryAssertionBabelPlugin.test.ts:51-54
Short-circuited TDZ identifiers crash on the report path - `packages/core/src/js/tools/sentryAssertionBabelPlugin.ts:493-500`
collectValueIdentifiers can emit identifiers that are in the temporal dead zone; evaluating them on the failure path throws a ReferenceError instead of reporting the assertion cleanly.
Assertion transform changes semantics when call is used as an expression - `packages/core/src/js/tools/sentryAssertionBabelPlugin.ts:570-571`
The Babel plugin unconditionally replaces every matched CallExpression with condition || reportCall() via path.replaceWith(). Because the original assertion pragmas (invariant, assert, warning, console.assert) return undefined (falsy), the replacement alters the resulting value and control flow when the call is not a standalone statement. For example, warning(cond) && next() becomes (cond || reportCall()) && next(); if cond is falsy, reportCall() returns a truthy event-id string, so next() runs even though the original warning would have returned undefined and skipped it. Similarly, assert(cond) && next() with a truthy cond evaluates to cond (truthy) after transformation, causing next() to run, whereas the original assert returns undefined (falsy) and would skip it. No parent-context check restricts the rewrite to ExpressionStatement nodes, and no tests cover expression-context usages.
Low
Dangling format specifier branch untested - `packages/core/test/assertion.test.ts:234-241`
This test's title and comment describe a dangling format specifier, but messageArgs provides four values for three specifiers so the i >= args.length branch in formatMessage is never exercised here.
captureAssertionViolation return type promises string but can return undefined - `packages/core/src/js/assertion.ts:322`
Return eventId ?? '' instead of raw eventId so the public API always returns a string as declared.
Error message backfill throws TypeError when error has a read-only message property - `packages/core/test/assertion.test.ts:133`
In packages/core/src/js/assertion.ts, the unguarded error.message = message assignment throws when options.error has a non-writable message property (e.g., a frozen or sealed Error). Because the file is an ES module and runs in strict mode, this can turn a report-only assertion into an unexpected crash and prevents hard preconditions from re-throwing their original error.
...and 1 more
4 skills analyzed
| Skill | Findings | Duration | Cost |
|---|---|---|---|
| security-review | 0 | 3m 8s | $1.48 |
| code-review | 4 | 20m 4s | $1.33 |
| find-bugs | 7 | 50m 3s | $4.07 |
| gha-security-review | 0 | 36m 13s | $0.07 |
⏱ 109m 28s · 12.9M in / 1.3M out · $6.95