src: fix permission inspector crash

nodejs · Jun 9, 2024 · 4a8cf55 · 4a8cf55
1 parent 2693f09
commit 4a8cf55
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 1 deletion.
diff --git a/deps/v8/src/wasm/wasm-engine.cc b/deps/v8/src/wasm/wasm-engine.cc
@@ -154,7 +154,7 @@ class WeakScriptHandle {
       : script_id_(script->id()), isolate_(isolate) {
     DCHECK(IsString(script->name()) || IsUndefined(script->name()));
     if (IsString(script->name())) {
-      source_url_ = String::cast(script->name())->ToCString();
+      // source_url_ = String::cast(script->name())->ToCString();
     }
     auto global_handle =
         script->GetIsolate()->global_handles()->Create(*script);

diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc
@@ -181,6 +181,9 @@ void SetConsoleExtensionInstaller(const FunctionCallbackInfo<Value>& info) {
 
 void CallAndPauseOnStart(const FunctionCallbackInfo<v8::Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(env,
+                                    permission::PermissionScope::kInspector,
+                                    "PauseOnNextJavascriptStatement");
   CHECK_GT(args.Length(), 1);
   CHECK(args[0]->IsFunction());
   SlicedArguments call_args(args, /* start */ 2);

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -1125,6 +1125,19 @@ bool ContextifyScript::EvalMachine(Local<Context> context,
 
 #if HAVE_INSPECTOR
   if (break_on_first_line) {
+    if (UNLIKELY(!env->permission()->is_granted(
+          env, permission::PermissionScope::kInspector, 
+          "PauseOnNextJavascriptStatement"))) {
+      node::permission::Permission::ThrowAccessDenied(env,
+        permission::PermissionScope::kInspector,
+        "PauseOnNextJavascriptStatement"); 
+      if (display_errors) {
+        // We should decorate non-termination exceptions
+        errors::DecorateErrorStack(env, try_catch);
+      }
+      try_catch.ReThrow();
+      return false;
+    }
     env->inspector_agent()->PauseOnNextJavascriptStatement("Break on start");
   }
 #endif

diff --git a/test/fixtures/permission/inspector-brk.js b/test/fixtures/permission/inspector-brk.js
@@ -0,0 +1 @@
+console.log("Hi!")
diff --git a/test/parallel/test-permission-inspector-brk.js b/test/parallel/test-permission-inspector-brk.js
@@ -0,0 +1,41 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+const fixtures = require('../common/fixtures');
+const file = fixtures.path('permission', 'inspector-brk.js');
+
+common.skipIfWorker();
+common.skipIfInspectorDisabled();
+
+// See https://github.com/nodejs/node/issues/53385
+{
+  const { status, stderr } = spawnSync(
+    process.execPath,
+    [
+      '--experimental-permission',
+      '--allow-fs-read=*',
+      '--inspect-brk',
+      file,
+    ],
+  );
+
+  assert.strictEqual(status, 1);
+  assert.match(stderr.toString(), /Error: Access to this API has been restricted/);
+}
+
+{
+  const { status, stderr } = spawnSync(
+    process.execPath,
+    [
+      '--experimental-permission',
+      '--inspect-brk',
+      '--eval',
+      'console.log("Hi!")',
+    ],
+  );
+
+  assert.strictEqual(status, 1);
+  assert.match(stderr.toString(), /Error: Access to this API has been restricted/);
+}