fix(core): honour jitless config in allowsEval probe#5864
Conversation
|
Heads-up: the one failing CI job ( This PR only touches Happy to add |
|
@pullfrog review |
|
Reviewed PR #5864. The fix correctly short-circuits the Task list (5/5 completed)
|
![pullfrog[bot]](https://ghub.bbstale.com/avatars/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Correct fix — the early return before the new Function("") probe is the right approach. No circular dependency introduced (core.ts → util.ts is type-only at runtime), and the cached() memoization semantics are properly accounted for.
Task list (5/5 completed)
- Read the diff and understand the changes
- Read the changed source files for full context
- Verify correctness: check
cached()semantics andglobalConfigimport - Check for security concerns (external PR)
- Self-critique drafted comments and submit review
Claude Opus ｜ 𝕏
| // Users on strict Content-Security-Policy environments (no `unsafe-eval`) | ||
| // can opt out of the JIT fast-path with `z.config({ jitless: true })`. | ||
| // Honouring it here — before the `new Function("")` probe — prevents a | ||
| // one-shot `securitypolicyviolation` report at the probe site, which | ||
| // Chrome's DevTools surfaces as an Issue even though the error is caught. | ||
| // Callers must set `jitless` at application entry, before any schema is | ||
| // parsed, since this getter is memoised via `cached()`. |
There was a problem hiding this comment.
Nit: the rest of util.ts is nearly comment-free. Consider trimming this to 1–2 lines — the what is clear from the code; only the why (CSP violation fires even inside try/catch) needs explaining.
| // Users on strict Content-Security-Policy environments (no `unsafe-eval`) | |
| // can opt out of the JIT fast-path with `z.config({ jitless: true })`. | |
| // Honouring it here — before the `new Function("")` probe — prevents a | |
| // one-shot `securitypolicyviolation` report at the probe site, which | |
| // Chrome's DevTools surfaces as an Issue even though the error is caught. | |
| // Callers must set `jitless` at application entry, before any schema is | |
| // parsed, since this getter is memoised via `cached()`. | |
| // When jitless is set, skip the `new Function("")` probe entirely — | |
| // even a caught attempt fires a `securitypolicyviolation` report (#4461). |
There was a problem hiding this comment.
Reviewed — no issues found. The fix is correct, minimal, and well-tested.
Task list (5/5 completed)
- Read the diff and understand the changes
- Read the changed source files for full context
- Verify correctness: check
cached()semantics andglobalConfigimport - Check for security concerns (external PR)
- Self-critique drafted comments and submit review
Claude Opus ｜ 𝕏
Closes colinhacks#4461, closes colinhacks#5414. `allowsEval` (packages/zod/src/v4/core/util.ts) feature-probes for `new Function()` support to pick Zod's JIT fast-path. The probe runs inside try/catch so it never throws at runtime — but strict-CSP browsers (script-src without 'unsafe-eval') still log the caught attempt as a `securitypolicyviolation`, which Chrome DevTools surfaces as an Issue ("Content Security Policy of your site blocks the use of 'eval'"). Many users have reported this as noise in production error monitors; the documented workaround `z.config({ jitless: true })` currently doesn't help because the probe is memoised via `cached()` and runs lazily on first `.value` access regardless of config. This change makes `allowsEval` read `globalConfig.jitless` before attempting the probe. When `jitless` is true, return `false` early and skip `new Function("")` entirely — no CSP violation is generated. The existing `schemas.ts:2025` already gates the fast-path on `!jitless`, so users who opt in are already on the slow path; the probe was just dead weight on their page loads. - No default-behaviour change: when `jitless` is undefined (the default), `allowsEval` still probes as before. - Strictly more restrictive when `jitless=true`: no eval attempt, no cached path to it — safer on strict CSPs, no library-side regression. - Users must call `z.config({ jitless: true })` at application entry, before any schema is parsed, because `allowsEval.value` is memoised via `cached()` on first access. Tests: packages/zod/src/v4/classic/tests/jitless-allows-eval.test.ts stubs the `Function` global and asserts the probe is never invoked under `z.config({ jitless: true })`. Lives in its own file so vitest's per-file ESM isolation gives a fresh cached allowsEval.
dokson
force-pushed
the
fix/jitless-skips-allows-eval-probe
branch
from
c09540f to
13201be
Compare
|
TL;DR — When Key changes
Summary ｜ 2 files ｜ 2 commits ｜ base: CSP-safe
|
…s` (#5889) Mirror the dual-package hazard fix already applied to `globalRegistry` in #5452: `globalConfig` (packages/zod/src/v4/core/core.ts) is now read from and written to `globalThis.__zod_globalConfig` instead of being a fresh module-scope object on each load. Why this matters ---------------- Without this, every loaded copy of Zod has its OWN `globalConfig`. In a monorepo where some packages resolve `zod` as ESM and others as CJS — or where multiple bundles each include their own copy — a single `z.config({ jitless: true })` only updates one of them. #5789 is exactly this footgun in production: the user had to call `z.config` twice (once per instance) to suppress the CSP eval probe everywhere. Combined with #5864 (`allowsEval` already honours `globalConfig.jitless`), this also gives strict-CSP users a clean pre-import opt-out: <script>globalThis.__zod_globalConfig = { jitless: true };</script> <script type="module" src="app.js"></script> …which is strictly more general than the bespoke `window.ZOD_NO_EVAL` flag proposed in #5862, and falls out of an existing pattern instead of introducing a new one. Notes ----- - No default-behaviour change: `globalThis.__zod_globalConfig ??= {}` preserves any pre-populated object, and `Object.assign(globalConfig, …)` keeps mutating the same identity that `z.config()` and external pre-set scripts both observe. - Symmetric with the registry pattern (`__zod_globalRegistry`): no versioned key, so cross-major loads share state. That's the same trade-off #5452 already accepted; if it's wrong for config it's also wrong for the registry. - No public API change. `z.config()` returns the same object identity for a given realm; only the underlying storage moved. Tests: packages/zod/src/v4/classic/tests/global-config.test.ts asserts the singleton is on `globalThis`, that `z.config(...)` writes are observed via `globalThis.__zod_globalConfig`, and that pre-populating the global before import is preserved.
|
Landed in Zod 4.4 |
2 participants
Summary
Makes
z.config({ jitless: true })actually silence the Chrome DevTools "Content Security Policy of your site blocks the use of 'eval' in JavaScript" issue on strict-CSP pages, by short-circuiting thenew Function("")feature probe insideallowsEval.Closes #4461.
Closes #5414.
Problem
util.allowsEvalfeature-probes fornew Function()support so the JIT fast-path inschemas.tscan be enabled opportunistically. The probe is wrapped intry/catch, so it never throws at runtime — but browsers still log the caught attempt as asecuritypolicyviolation, which Chrome surfaces as a Developer Tools Issue:Users on strict CSPs (
script-srcwithout'unsafe-eval') have flagged this in production error monitors (Datadog, Sentry) — see #5414 where @zbauman3 reported "literally thousands of errors" from a single Zod v4 upgrade. The maintainer's documented workaround was:But this doesn't actually help today, because
allowsEvalis memoised viacached()and runs lazily on first.valueaccess, regardless of whether the user setjitlessfirst. Users reported back that settingjitlessdid nothing for the CSP violation, and the thread has stalled.Fix
allowsEvalnow consultsglobalConfig.jitlessbefore attempting the probe. Whenjitless === true, it returnsfalsewithout invokingnew Functionat all — no CSP violation is ever generated.The change is minimal, additive, and strictly more restrictive when opted into. The existing
jit = !core.globalConfig.jitlessgate atschemas.ts:2025already routesjitless=trueusers onto the slow path; the probe was just dead weight on their page loads.API / behaviour
jitlessisundefined(its default),allowsEvalprobes exactly as before — no behaviour shift for the 99% of users who never touch config.jitless=true, Zod never callsnew Function(""). Users on strict CSPs now have a clean devtools console.z.config({ jitless: true })must be called at application entry, before any schema is parsed, becauseallowsEval.valueis memoised viacached()on first access. Same contract thejitlessoption already implies — this PR doesn't change it.jitless(one extraif (undefined)branch on first probe).Tests
Added
packages/zod/src/v4/classic/tests/jitless-allows-eval.test.ts. It:z.config({ jitless: true })at the top of the test.globalThis.Functionwith a stub that throws if invoked.util.allowsEval.value === false.The test lives in its own file because vitest isolates ESM graphs per file, so the cached
allowsEval.valueis guaranteed fresh and is never accessed before the config mutation. Afinallyblock restoresglobalThis.Functionand clears the config flag so other test files in the same worker see the default.Regression check
Ran the full suite locally (
pnpm test). All existing tests pass exceptsrc/v4/classic/tests/datetime.test.ts > redos checker, which times out at 5000ms → 13800ms on my Windows/pnpm combo when run under parallel-file concurrency. Isolated run (pnpm test packages/zod/src/v4/classic/tests/datetime.test.ts) passes cleanly in 11.5s / 28 tests. The failure is environmental (ReDoS-check regex timing on a slower machine) and completely unrelated toutil.ts— same test passes onmainwithout this branch. CI on GitHub Actions (Linux) should be clean.Scope / risk
packages/zod/src/v4/core/util.ts(+12 lines, new runtime import + early return).packages/zod/src/v4/classic/tests/jitless-allows-eval.test.ts(+46 lines).$ZodConfig.jitlessalready exists onmainand is documented atcore.ts:129.Happy to iterate on the test style, the comment wording, or split the changelog/docs note into a separate PR if preferred.