fix(runtime): suggest --unstable-unsafe-proto after __proto__ access (#35192)

Deno disables the `Object.prototype.__proto__` accessor by default to
guard against prototype pollution. It did this by deleting the property, so
reads return `undefined` and writes silently create a useless own property
without changing the prototype. These quiet failures are hard to track down when
code accidentally relies on `__proto__`.

Making the accessor throw instead (#34730) surfaced those bugs but broke
real packages such as Playwright, and was reverted (#34772). This takes a
middle ground: it keeps the silent behavior, but installs an accessor that
reproduces the deleted semantics exactly (read returns `undefined`, write creates
an own data property, the prototype is unchanged) while recording the first
access in a process-global flag. When the program later crashes, the
uncaught-error formatter appends a hint to run again with `--unstable-unsafe-proto`.
Programs that never touch `__proto__`, and runs with the flag already enabled,
are unaffected.

Reads and writes are recorded under separate flags because they fail
differently. A write silently no-ops and the breakage surfaces
downstream at an unrelated-looking line, so any later crash is enough to show the hint. A
read instead returns `undefined` and crashes right at the access site (for
example pnpm 11's `who.__proto__.constructor`), so the hint for a read is only
shown when the crashing error itself mentions `__proto__`. That keeps programs
that merely probe `__proto__` for feature detection, and then crash for an
unrelated reason, from getting a spurious suggestion.

One consequence is that the accessor is now present in both modes (which
matches Node, where it always exists), so `Object.hasOwn(Object.prototype,
"__proto__")` is now `true` by default rather than `false`. The
detection in the existing tests and the unsafe-proto fixture therefore switches from
checking presence to checking behavior: reading `__proto__` returns
`undefined` only when the accessor is disabled.

These are the real-world `__proto__` crashes the hint now points at,
each of which runs correctly under `--unstable-unsafe-proto`:

Closes #34694
Closes #34337
Closes #26584
Closes #24986
Closes #24413
Closes #35131

Author: bartlomieju

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,24 @@ 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);
+
+/// Set to `true` when user code reads `Object.prototype.__proto__` while the
+/// accessor is disabled (the read returns `undefined`). Unlike a write, a read
+/// crashes right at the access site (e.g. `who.__proto__.constructor` throws on
+/// the same line), so on its own a read flag is too noisy to nudge on: programs
+/// routinely probe `__proto__` for feature detection without anything going
+/// wrong. The error formatter therefore only suggests the escape hatch when
+/// this flag is set *and* the crashing error mentions `__proto__`.
+/// Written by `op_proto_get_attempted`, read by `get_suggestions_for_terminal_errors`.
+pub static PROTO_GET_ATTEMPTED: AtomicBool = AtomicBool::new(false);
+
 #[derive(Debug, Clone)]
 struct ErrorReference<'a> {
   from: &'a JsError,
@@ -352,6 +372,46 @@ fn format_js_error_inner(
 }
 
 fn get_suggestions_for_terminal_errors(e: &JsError) -> Vec<FixSuggestion<'_>> {
+  let mut suggestions = get_message_suggestions(e);
+  // A `__proto__` *write* silently no-ops and the breakage surfaces downstream
+  // at an unrelated-looking line, so any later crash is reason enough to point
+  // at the escape hatch. A `__proto__` *read* instead returns `undefined` and
+  // blows up right at the access site, so we only nudge when `__proto__` is on
+  // the crashing line/message, to avoid bothering programs that merely probed
+  // `__proto__` once (feature detection) and then crashed for another reason.
+  let info = if PROTO_SET_ATTEMPTED.load(Ordering::Relaxed) {
+    Some(cstr!(
+      "This program assigned to <u>Object.prototype.__proto__</>, which Deno disables by default."
+    ))
+  } else if PROTO_GET_ATTEMPTED.load(Ordering::Relaxed)
+    && error_mentions_proto(e)
+  {
+    Some(cstr!(
+      "This program read <u>Object.prototype.__proto__</>, which Deno disables by default (it returns <i>undefined</>)."
+    ))
+  } else {
+    None
+  };
+  if let Some(info) = info {
+    suggestions.push(FixSuggestion::info(info));
+    suggestions.push(FixSuggestion::hint(cstr!(
+      "If this caused the error, run again with <u>--unstable-unsafe-proto</> to restore it."
+    )));
+  }
+  suggestions
+}
+
+fn error_mentions_proto(e: &JsError) -> bool {
+  e.source_line
+    .as_deref()
+    .is_some_and(|l| l.contains("__proto__"))
+    || e
+      .message
+      .as_deref()
+      .is_some_and(|m| m.contains("__proto__"))
+}
+
+fn get_message_suggestions(e: &JsError) -> Vec<FixSuggestion<'_>> {
   if let Some(msg) = &e.message {
     if msg.contains("module is not defined")
       || msg.contains("exports is not defined")

runtime/js/99_main.js

@@ -17,6 +17,8 @@ import {
   op_internal_log,
   op_main_module,
   op_ppid,
+  op_proto_get_attempted,
+  op_proto_set_attempted,
   op_set_format_exception_callback,
   op_snapshot_options,
   op_worker_close,
@@ -39,7 +41,9 @@ const {
   ObjectDefineProperty,
   ObjectGetOwnPropertyDescriptors,
   ObjectHasOwn,
+  ObjectIsExtensible,
   ObjectKeys,
+  ObjectPrototype,
   ObjectPrototypeIsPrototypeOf,
   ObjectSetPrototypeOf,
   PromisePrototypeThen,
@@ -730,6 +734,64 @@ const executionModes = {
   jupyter: 8,
 };
 
+let protoSetRecorded = false;
+let protoGetRecorded = 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 read and the first write. When
+// the program later crashes, the uncaught-error formatter (runtime/fmt_errors.rs)
+// reads those flags and suggests `--unstable-unsafe-proto`. A write surfaces
+// downstream so any later crash triggers the nudge; a read crashes at the
+// access site, so the formatter additionally requires `__proto__` on the
+// failing line before nudging. 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__() {
+      if (!protoGetRecorded) {
+        protoGetRecorded = true;
+        op_proto_get_attempted();
+      }
+      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) {
@@ -922,9 +984,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
@@ -1054,9 +1114,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

runtime/ops/bootstrap.rs

@@ -21,6 +21,8 @@ deno_core::extension!(
     op_bootstrap_stderr_no_color,
     op_bootstrap_unstable_args,
     op_bootstrap_is_from_unconfigured_runtime,
+    op_proto_set_attempted,
+    op_proto_get_attempted,
     op_snapshot_options,
   ],
   options = {
@@ -162,3 +164,24 @@ 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::<IsFromUnconfiguredRuntime>().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);
+}
+
+// Called (at most once) from the disabled `Object.prototype.__proto__` getter
+// in 99_main.js when user code reads `__proto__` (the read returns `undefined`).
+// Records a process-global flag; the uncaught-error formatter only acts on it
+// when the crashing error also mentions `__proto__`, since reads are common and
+// usually harmless. See `crate::fmt_errors::PROTO_GET_ATTEMPTED`.
+#[op2(fast)]
+pub fn op_proto_get_attempted() {
+  crate::fmt_errors::PROTO_GET_ATTEMPTED
+    .store(true, std::sync::atomic::Ordering::Relaxed);
+}

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 {

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",

tests/specs/run/unsafe_proto/worker.js

@@ -1,2 +1,2 @@
-console.log(Object.hasOwn(Object.prototype, "__proto__"));
+console.log(({}).__proto__ !== undefined);
 close();

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",

tests/specs/run/unsafe_proto_flag/worker.js

@@ -1,2 +1,2 @@
-console.log(Object.hasOwn(Object.prototype, "__proto__"));
+console.log(({}).__proto__ !== undefined);
 close();

tests/specs/run/unsafe_proto_suggestion/__test__.jsonc

@@ -0,0 +1,29 @@
+{
+  "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"
+    },
+    "suggests_flag_after_proto_read_on_failing_line": {
+      "args": "run read_and_throw.js",
+      "exitCode": 1,
+      "output": "read_and_throw.out"
+    },
+    "no_suggestion_when_proto_read_unrelated_to_crash": {
+      "args": "run read_unrelated_throw.js",
+      "exitCode": 1,
+      "output": "read_unrelated_throw.out"
+    }
+  }
+}

tests/specs/run/unsafe_proto_suggestion/read_and_throw.js

@@ -0,0 +1,7 @@
+// Reading `__proto__` returns `undefined` while the accessor is disabled (the
+// default), so `who.__proto__.constructor` throws right at the access site.
+// Because the crashing line itself mentions `__proto__`, the uncaught-error
+// formatter suggests `--unstable-unsafe-proto`. This mirrors the real-world
+// crash in pnpm 11 (denoland/deno#34694).
+const who = {};
+who.__proto__.constructor;