diff --git a/runtime/fmt_errors.rs b/runtime/fmt_errors.rs index bdeafaf723d1b5..49cf9d6ea7251b 100644 --- a/runtime/fmt_errors.rs +++ b/runtime/fmt_errors.rs @@ -3,6 +3,8 @@ use std::borrow::Cow; use std::fmt::Write as _; use std::sync::LazyLock; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; use color_print::cformat; use color_print::cstr; @@ -11,6 +13,14 @@ use deno_core::error::format_frame; use deno_core::url::Url; use deno_terminal::colors; +/// Set to `true` when user code assigns to `Object.prototype.__proto__` while +/// the accessor is disabled (the default, unless `--unstable-unsafe-proto`). +/// The assignment itself stays a silent no-op so fragile packages keep working +/// (see denoland/deno#34730 / #34772, where throwing broke Playwright); this +/// flag only lets the uncaught-error formatter nudge toward the escape hatch. +/// Written by `op_proto_set_attempted`, read by `get_suggestions_for_terminal_errors`. +pub static PROTO_SET_ATTEMPTED: AtomicBool = AtomicBool::new(false); + #[derive(Debug, Clone)] struct ErrorReference<'a> { from: &'a JsError, @@ -352,6 +362,23 @@ fn format_js_error_inner( } fn get_suggestions_for_terminal_errors(e: &JsError) -> Vec> { + let mut suggestions = get_message_suggestions(e); + // If the program assigned to the (disabled by default) `__proto__` accessor + // and then crashed, the two are frequently related, so point at the escape + // hatch. This is intentionally a soft nudge appended to any uncaught error + // rather than a hard failure at assignment time, which broke real packages. + if PROTO_SET_ATTEMPTED.load(Ordering::Relaxed) { + suggestions.push(FixSuggestion::info(cstr!( + "This program assigned to Object.prototype.__proto__, which Deno disables by default." + ))); + suggestions.push(FixSuggestion::hint(cstr!( + "If this caused the error, run again with --unstable-unsafe-proto to restore it." + ))); + } + suggestions +} + +fn get_message_suggestions(e: &JsError) -> Vec> { if let Some(msg) = &e.message { if msg.contains("module is not defined") || msg.contains("exports is not defined") diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js index 411db875a231a9..221be0460f1bc3 100644 --- a/runtime/js/99_main.js +++ b/runtime/js/99_main.js @@ -17,6 +17,7 @@ import { op_internal_log, op_main_module, op_ppid, + op_proto_set_attempted, op_set_format_exception_callback, op_snapshot_options, op_worker_close, @@ -39,7 +40,9 @@ const { ObjectDefineProperty, ObjectGetOwnPropertyDescriptors, ObjectHasOwn, + ObjectIsExtensible, ObjectKeys, + ObjectPrototype, ObjectPrototypeIsPrototypeOf, ObjectSetPrototypeOf, PromisePrototypeThen, @@ -718,6 +721,56 @@ const executionModes = { jupyter: 8, }; +let protoSetRecorded = false; + +// By default Deno disables the `Object.prototype.__proto__` accessor for +// security reasons (it enables prototype pollution), see +// https://tc39.es/ecma262/#sec-get-object.prototype.__proto__ +// +// Historically this was done with `delete Object.prototype.__proto__`, which +// makes `obj.__proto__` reads return `undefined` and writes silently create a +// useless own property. Making the accessor *throw* instead would surface those +// silent bugs, but it broke real packages such as Playwright (see +// denoland/deno#34730 / #34772), so we keep the silent behavior. +// +// Instead of deleting we install an accessor that reproduces the deleted +// semantics exactly (read -> `undefined`, write -> own data property, prototype +// unchanged) but additionally records the first write. When the program later +// crashes, the uncaught-error formatter (runtime/fmt_errors.rs) reads that flag +// and suggests `--unstable-unsafe-proto`. The `__proto__` key in object +// literals (e.g. `{ __proto__: null }`) is separate syntax and is unaffected. +function disableProtoAccessor() { + ObjectDefineProperty(ObjectPrototype, "__proto__", { + __proto__: null, + configurable: true, + enumerable: false, + // Distinct getter/setter function objects: the native accessor uses + // separate functions and WPT asserts accessor get/set are unique. + get: function __proto__() { + return undefined; + }, + set: function __proto__(value) { + if (!protoSetRecorded) { + protoSetRecorded = true; + op_proto_set_attempted(); + } + // Reproduce the previous `delete` behavior: a bare assignment created a + // normal own data property. Skip non-extensible receivers, where that + // assignment was a silent no-op in sloppy mode (and a throw in strict + // mode); keeping it silent here matches the "stay silent" goal above. + if (ObjectIsExtensible(this)) { + ObjectDefineProperty(this, "__proto__", { + __proto__: null, + value, + writable: true, + enumerable: true, + configurable: true, + }); + } + }, + }); +} + function bootstrapMainRuntime(runtimeOptions, warmup = false) { if (!warmup) { if (hasBootstrapped) { @@ -910,9 +963,7 @@ function bootstrapMainRuntime(runtimeOptions, warmup = false) { } if (!ArrayPrototypeIncludes(unstableFeatures, unstableIds.unsafeProto)) { - // Removes the `__proto__` for security reasons. - // https://tc39.es/ecma262/#sec-get-object.prototype.__proto__ - delete Object.prototype.__proto__; + disableProtoAccessor(); } // Setup `Deno` global - we're actually overriding already existing global @@ -1042,9 +1093,7 @@ function bootstrapWorkerRuntime( delete finalDenoNs.mainModule; if (!ArrayPrototypeIncludes(unstableFeatures, unstableIds.unsafeProto)) { - // Removes the `__proto__` for security reasons. - // https://tc39.es/ecma262/#sec-get-object.prototype.__proto__ - delete Object.prototype.__proto__; + disableProtoAccessor(); } // Setup `Deno` global - we're actually overriding already existing global diff --git a/runtime/ops/bootstrap.rs b/runtime/ops/bootstrap.rs index cb8fb060d55d5e..2e5f79cfddd3a6 100644 --- a/runtime/ops/bootstrap.rs +++ b/runtime/ops/bootstrap.rs @@ -21,6 +21,7 @@ deno_core::extension!( op_bootstrap_stderr_no_color, op_bootstrap_unstable_args, op_bootstrap_is_from_unconfigured_runtime, + op_proto_set_attempted, op_snapshot_options, ], options = { @@ -162,3 +163,13 @@ pub fn op_bootstrap_stderr_no_color(_state: &mut OpState) -> bool { pub fn op_bootstrap_is_from_unconfigured_runtime(state: &mut OpState) -> bool { state.borrow::().0 } + +// Called (at most once) from the disabled `Object.prototype.__proto__` setter +// in 99_main.js when user code assigns to `__proto__`. Records a process-global +// flag so that, if the program later crashes, the uncaught-error formatter can +// suggest `--unstable-unsafe-proto`. See `crate::fmt_errors::PROTO_SET_ATTEMPTED`. +#[op2(fast)] +pub fn op_proto_set_attempted() { + crate::fmt_errors::PROTO_SET_ATTEMPTED + .store(true, std::sync::atomic::Ordering::Relaxed); +} diff --git a/tests/registry/npm/@denotest/unsafe-proto-bin/1.0.0/cli.mjs b/tests/registry/npm/@denotest/unsafe-proto-bin/1.0.0/cli.mjs index 80a36ddf77edf9..0446abf2a64cdf 100644 --- a/tests/registry/npm/@denotest/unsafe-proto-bin/1.0.0/cli.mjs +++ b/tests/registry/npm/@denotest/unsafe-proto-bin/1.0.0/cli.mjs @@ -1,6 +1,9 @@ #!/usr/bin/env node -const hasUnsafeProto = Object.hasOwn(Object.prototype, "__proto__"); +// Detect by behavior, not presence: Deno keeps the `__proto__` accessor +// installed even when disabled (so it can capture assignments), so reading it +// is what actually distinguishes the modes — disabled returns `undefined`. +const hasUnsafeProto = ({}).__proto__ !== undefined; if (hasUnsafeProto) { console.log("unsafe proto enabled"); } else { diff --git a/tests/specs/run/unsafe_proto/main.js b/tests/specs/run/unsafe_proto/main.js index eb95c55a030058..c1f0bfed6ab741 100644 --- a/tests/specs/run/unsafe_proto/main.js +++ b/tests/specs/run/unsafe_proto/main.js @@ -1,4 +1,6 @@ -console.log(Object.hasOwn(Object.prototype, "__proto__")); +// Disabled by default: the accessor stays installed (so writes can be +// captured) but reads return `undefined`, so this prints `false`. +console.log(({}).__proto__ !== undefined); new Worker(import.meta.resolve("./worker.js"), { type: "module", diff --git a/tests/specs/run/unsafe_proto/worker.js b/tests/specs/run/unsafe_proto/worker.js index b22bc8713988f6..8e394246d20fe9 100644 --- a/tests/specs/run/unsafe_proto/worker.js +++ b/tests/specs/run/unsafe_proto/worker.js @@ -1,2 +1,2 @@ -console.log(Object.hasOwn(Object.prototype, "__proto__")); +console.log(({}).__proto__ !== undefined); close(); diff --git a/tests/specs/run/unsafe_proto_flag/main.js b/tests/specs/run/unsafe_proto_flag/main.js index eb95c55a030058..86a79788017173 100644 --- a/tests/specs/run/unsafe_proto_flag/main.js +++ b/tests/specs/run/unsafe_proto_flag/main.js @@ -1,4 +1,6 @@ -console.log(Object.hasOwn(Object.prototype, "__proto__")); +// With --unstable-unsafe-proto the native accessor is restored, so reading +// `__proto__` returns the prototype and this prints `true`. +console.log(({}).__proto__ !== undefined); new Worker(import.meta.resolve("./worker.js"), { type: "module", diff --git a/tests/specs/run/unsafe_proto_flag/worker.js b/tests/specs/run/unsafe_proto_flag/worker.js index b22bc8713988f6..8e394246d20fe9 100644 --- a/tests/specs/run/unsafe_proto_flag/worker.js +++ b/tests/specs/run/unsafe_proto_flag/worker.js @@ -1,2 +1,2 @@ -console.log(Object.hasOwn(Object.prototype, "__proto__")); +console.log(({}).__proto__ !== undefined); close(); diff --git a/tests/specs/run/unsafe_proto_suggestion/__test__.jsonc b/tests/specs/run/unsafe_proto_suggestion/__test__.jsonc new file mode 100644 index 00000000000000..0a2dad5e2fdba8 --- /dev/null +++ b/tests/specs/run/unsafe_proto_suggestion/__test__.jsonc @@ -0,0 +1,19 @@ +{ + "tests": { + "suggests_flag_after_proto_set": { + "args": "run set_and_throw.js", + "exitCode": 1, + "output": "set_and_throw.out" + }, + "no_suggestion_without_proto_set": { + "args": "run throw_only.js", + "exitCode": 1, + "output": "throw_only.out" + }, + "no_suggestion_with_unsafe_proto_flag": { + "args": "run --unstable-unsafe-proto set_and_throw.js", + "exitCode": 1, + "output": "set_and_throw_unsafe.out" + } + } +} diff --git a/tests/specs/run/unsafe_proto_suggestion/set_and_throw.js b/tests/specs/run/unsafe_proto_suggestion/set_and_throw.js new file mode 100644 index 00000000000000..6d259d9ccc158c --- /dev/null +++ b/tests/specs/run/unsafe_proto_suggestion/set_and_throw.js @@ -0,0 +1,7 @@ +// Assigning to `__proto__` is a silent no-op while the accessor is disabled +// (the default). Deno records that it happened so that, if the program then +// crashes, the uncaught-error formatter can suggest `--unstable-unsafe-proto`. +const obj = {}; +obj.__proto__ = { polluted: true }; + +throw new Error("boom"); diff --git a/tests/specs/run/unsafe_proto_suggestion/set_and_throw.out b/tests/specs/run/unsafe_proto_suggestion/set_and_throw.out new file mode 100644 index 00000000000000..f4fc3c0d5a4b33 --- /dev/null +++ b/tests/specs/run/unsafe_proto_suggestion/set_and_throw.out @@ -0,0 +1,7 @@ +error: Uncaught (in promise) Error: boom +throw new Error("boom"); + ^ + at [WILDCARD]set_and_throw.js:7:7 + + info: This program assigned to Object.prototype.__proto__, which Deno disables by default. + hint: If this caused the error, run again with --unstable-unsafe-proto to restore it. diff --git a/tests/specs/run/unsafe_proto_suggestion/set_and_throw_unsafe.out b/tests/specs/run/unsafe_proto_suggestion/set_and_throw_unsafe.out new file mode 100644 index 00000000000000..c9052d1169f2d7 --- /dev/null +++ b/tests/specs/run/unsafe_proto_suggestion/set_and_throw_unsafe.out @@ -0,0 +1,4 @@ +error: Uncaught (in promise) Error: boom +throw new Error("boom"); + ^ + at [WILDCARD]set_and_throw.js:7:7 diff --git a/tests/specs/run/unsafe_proto_suggestion/throw_only.js b/tests/specs/run/unsafe_proto_suggestion/throw_only.js new file mode 100644 index 00000000000000..c5d1175d7618ab --- /dev/null +++ b/tests/specs/run/unsafe_proto_suggestion/throw_only.js @@ -0,0 +1,3 @@ +// This program never touches `__proto__`, so the crash must NOT carry the +// `--unstable-unsafe-proto` suggestion. +throw new Error("boom"); diff --git a/tests/specs/run/unsafe_proto_suggestion/throw_only.out b/tests/specs/run/unsafe_proto_suggestion/throw_only.out new file mode 100644 index 00000000000000..ce5b7832029bae --- /dev/null +++ b/tests/specs/run/unsafe_proto_suggestion/throw_only.out @@ -0,0 +1,4 @@ +error: Uncaught (in promise) Error: boom +throw new Error("boom"); + ^ + at [WILDCARD]throw_only.js:3:7