Skip to content

Commit

Permalink
src: fix permission inspector crash
Browse files Browse the repository at this point in the history
PR-URL: nodejs#53389
Fixes: nodejs#53385
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
  • Loading branch information
theanarkh authored and bmeck committed Jun 22, 2024
1 parent b15b3e9 commit 72eaff9
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/inspector_js_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
15 changes: 15 additions & 0 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1123,6 +1123,21 @@ 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
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/permission/inspector-brk.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("Hi!")
41 changes: 41 additions & 0 deletions test/parallel/test-permission-inspector-brk.js
Original file line number Diff line number Diff line change
@@ -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/);
}

0 comments on commit 72eaff9

Please sign in to comment.