Skip to content

Commit 162eefb

Browse files
antonisclaude
andcommitted
fix(core): harden assertion transform against Error shadowing, __proto__ keys, and @sentry-internal
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 92c1b77 commit 162eefb

2 files changed

Lines changed: 62 additions & 12 deletions

File tree

‎packages/core/src/js/tools/sentryAssertionBabelPlugin.ts‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,12 @@ const DEFAULT_ASSERTION_MODULES = ['invariant', 'tiny-invariant', 'warning', 'as
116116
* Path fragments identifying the Sentry SDK's own source. Files matching any of
117117
* these are never instrumented — the plugin injects a `require` of the SDK, so
118118
* rewriting the SDK's own asserts would create a self-referential require. The
119-
* second marker covers the monorepo dev symlink, whose path has no
120-
* `node_modules/@sentry` segment.
119+
* `@sentry-internal` scope holds packages the SDK depends on transitively, so
120+
* instrumenting them would create the same circular require. The last marker
121+
* covers the monorepo dev symlink, whose path has no `node_modules/@sentry`
122+
* segment.
121123
*/
122-
const SENTRY_SDK_PATH_MARKERS = ['/@sentry/', 'sentry-react-native/packages/'];
124+
const SENTRY_SDK_PATH_MARKERS = ['/@sentry/', '/@sentry-internal/', 'sentry-react-native/packages/'];
123125

124126
export interface SentryAssertionBabelPluginOptions {
125127
/**
@@ -471,9 +473,18 @@ function ensureErrorBinding(
471473
}
472474
uid = path.scope.generateUidIdentifier('Error');
473475
const program = path.scope.getProgramParent().path as NodePath<BabelTypes.Program>;
474-
program.unshiftContainer('body', [
475-
t.variableDeclaration('var', [t.variableDeclarator(t.cloneNode(uid), t.identifier('Error'))]),
476-
]);
476+
// `var _Error = typeof globalThis !== 'undefined' ? globalThis.Error : Error;`
477+
// Reading `globalThis.Error` (not the bare `Error` identifier) means a
478+
// module-level `const`/`let`/`class Error` — which would put the bare
479+
// identifier in its TDZ at program top, or a `var Error` shadow that reads
480+
// `undefined` — can't corrupt the alias. The bare-`Error` fallback is only
481+
// reached on ancient runtimes without `globalThis`, where the shadow is moot.
482+
const errorInit = t.conditionalExpression(
483+
t.binaryExpression('!==', t.unaryExpression('typeof', t.identifier('globalThis')), t.stringLiteral('undefined')),
484+
t.memberExpression(t.identifier('globalThis'), t.identifier('Error')),
485+
t.identifier('Error'),
486+
);
487+
program.unshiftContainer('body', [t.variableDeclaration('var', [t.variableDeclarator(t.cloneNode(uid), errorInit)])]);
477488
state.set(ERROR_UID_KEY, uid);
478489
return uid;
479490
}
@@ -505,7 +516,16 @@ function buildReportProperties(
505516
properties.push(
506517
t.objectProperty(
507518
t.identifier('values'),
508-
t.objectExpression(valueNames.map(name => t.objectProperty(t.identifier(name), t.identifier(name)))),
519+
t.objectExpression(
520+
valueNames.map(name =>
521+
// A bare `__proto__: v` key is the prototype-setter syntax, not a
522+
// data property, and throws for a non-object value. Emit it as a
523+
// computed key (`['__proto__']: v`) so it stays an own property.
524+
name === '__proto__'
525+
? t.objectProperty(t.stringLiteral('__proto__'), t.identifier('__proto__'), /* computed */ true)
526+
: t.objectProperty(t.identifier(name), t.identifier(name)),
527+
),
528+
),
509529
),
510530
);
511531
}

‎packages/core/test/tools/sentryAssertionBabelPlugin.test.ts‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,20 +43,38 @@ describe('sentryAssertionBabelPlugin', () => {
4343
// without relying on framesToPop or the in_app heuristic. It goes through a
4444
// hoisted global-`Error` alias so a call-site shadow can't break it.
4545
const out = transform(`invariant(total >= 0, 'bad total');`);
46-
expect(out).toMatch(/var _Error\d* = Error;/);
46+
expect(out).toMatch(/var _Error\d* = typeof globalThis !== ["']undefined["'] \? globalThis\.Error : Error;/);
4747
expect(out).toMatch(/error: new _Error\d*\(\)/);
4848
});
4949

5050
it('is immune to a call-site `Error` shadow (hoisted alias captures the global)', () => {
5151
// A parameter named `Error` shadows the global at the call site. The hoisted
52-
// `var _Error = Error;` at program top captured the real constructor first,
53-
// so the injected `new _Error()` never resolves to the shadow (which would
54-
// throw `TypeError: Error is not a constructor` when the assertion fires).
52+
// alias at program top captured the real constructor first, so the injected
53+
// `new _Error()` never resolves to the shadow (which would throw
54+
// `TypeError: Error is not a constructor` when the assertion fires).
5555
const out = transform(`function f(Error) {\n invariant(ok);\n}`);
56-
expect(out).toMatch(/var _Error\d* = Error;/);
56+
expect(out).toMatch(/var _Error\d* = typeof globalThis !== ["']undefined["'] \? globalThis\.Error : Error;/);
5757
expect(out).toMatch(/error: new _Error\d*\(\)/);
5858
});
5959

60+
it('is immune to a module-level `Error` shadow (alias reads globalThis.Error)', () => {
61+
// A module-scope `const Error` would put the bare `Error` identifier in its
62+
// TDZ at program top, so `var _Error = Error;` would throw at load time. The
63+
// alias reads `globalThis.Error` instead, which the lexical shadow can't
64+
// capture.
65+
const out = transform(`const Error = 1;\ninvariant(ok);`);
66+
expect(out).toMatch(/var _Error\d* = typeof globalThis !== ["']undefined["'] \? globalThis\.Error : Error;/);
67+
expect(out).toMatch(/error: new _Error\d*\(\)/);
68+
});
69+
70+
it('emits a `__proto__` value as a computed key, not a prototype setter', () => {
71+
// `{ __proto__: v }` sets the prototype and throws for a non-object value;
72+
// the computed form `{ ['__proto__']: v }` keeps it an own data property.
73+
const out = transform(`const __proto__ = 1;\ninvariant(__proto__ > 0);`);
74+
expect(out).toMatch(/\[["']__proto__["']\]: __proto__/);
75+
expect(out).not.toMatch(/\{\s*__proto__: __proto__/);
76+
});
77+
6078
it('forwards variadic substitution args as messageArgs for interpolation', () => {
6179
// RN's Dimensions invariant is `invariant(dims, 'No dimension set for key %s',
6280
// dimension)` — the extra arg must reach the reporter so `%s` interpolates.
@@ -250,6 +268,18 @@ describe('sentryAssertionBabelPlugin', () => {
250268
expect(out).toMatch(/invariant\(ok\)/);
251269
});
252270

271+
it('never instruments `@sentry-internal` packages (SDK transitive deps)', () => {
272+
// `@sentry-internal/*` packages are dependencies of `@sentry/react-native`;
273+
// instrumenting them injects a require of the SDK into its own dependency
274+
// graph, creating a circular require that can leave the reporter undefined.
275+
const out = transform(`import invariant from 'invariant';\ninvariant(ok);`, {
276+
filename: '/proj/node_modules/@sentry-internal/browser-utils/index.js',
277+
options: { includeNodeModules: true },
278+
});
279+
expect(out).not.toContain('_captureAssertionViolation');
280+
expect(out).toMatch(/invariant\(ok\)/);
281+
});
282+
253283
it('never instruments the Sentry SDK’s own source (monorepo symlink path)', () => {
254284
// The dev symlink resolves the SDK through a path with no node_modules/@sentry
255285
// segment, so it must be excluded by the packages/ marker too.

0 commit comments

Comments
 (0)