Skip to content

Commit 290637f

Browse files
committed
[WIP] Refactor and simplify the implementation
1 parent 09443c3 commit 290637f

File tree

13 files changed

+226
-431
lines changed

13 files changed

+226
-431
lines changed

lib/.eslintrc.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,4 @@ globals:
9393
module: false
9494
internalBinding: false
9595
primordials: false
96+
runInPrivilegedScope: false

lib/internal/process/policy.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ module.exports = ObjectFreeze({
6868
if (typeof permissions !== 'string')
6969
throw new ERR_INVALID_ARG_TYPE('permissions', 'string', permissions);
7070
const ret = policy.deny(permissions);
71-
if (typeof ret === 'number')
71+
if (ret === undefined)
7272
throw new ERR_INVALID_ARG_VALUE('permissions', permissions);
7373
},
7474

@@ -83,7 +83,7 @@ module.exports = ObjectFreeze({
8383
if (typeof permissions !== 'string')
8484
throw new ERR_INVALID_ARG_TYPE('permission', 'string', permissions);
8585
const ret = policy.check(permissions);
86-
if (typeof ret === 'number')
86+
if (ret === undefined)
8787
throw new ERR_INVALID_ARG_VALUE('permissions', permissions);
8888
return ret;
8989
}

lib/internal/test/policy.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
'use strict';
2+
3+
process.emitWarning(
4+
'These APIs are for internal testing only. Do not use them.',
5+
'internal/test/policy');
6+
7+
module.exports = { runInPrivilegedScope };

node.gyp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@
220220
'lib/internal/source_map/source_map.js',
221221
'lib/internal/source_map/source_map_cache.js',
222222
'lib/internal/test/binding.js',
223+
'lib/internal/test/policy.js',
223224
'lib/internal/timers.js',
224225
'lib/internal/tls.js',
225226
'lib/internal/trace_events_async_hooks.js',
@@ -737,7 +738,6 @@
737738
'src/node_perf_common.h',
738739
'src/node_platform.h',
739740
'src/policy/policy.h',
740-
'src/policy/policy-inl.h',
741741
'src/node_process.h',
742742
'src/node_report.h',
743743
'src/node_revert.h',

src/env-inl.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
#include "env.h"
3030
#include "node.h"
3131
#include "util-inl.h"
32-
#include "policy/policy-inl.h"
3332
#include "uv.h"
3433
#include "v8.h"
3534
#include "node_perf_common.h"
@@ -247,8 +246,12 @@ inline size_t Environment::async_callback_scope_depth() const {
247246
return async_callback_scope_depth_;
248247
}
249248

250-
policy::PrivilegedAccessContext* Environment::privileged_access_context() {
251-
return &privileged_access_context_;
249+
inline void Environment::set_in_privileged_scope(bool on) {
250+
in_privileged_scope_ = on;
251+
}
252+
253+
inline bool Environment::in_privileged_scope() const {
254+
return in_privileged_scope_;
252255
}
253256

254257
inline void Environment::PushAsyncCallbackScope() {

src/env.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,6 @@ Environment::Environment(IsolateData* isolate_data,
313313
ThreadId thread_id)
314314
: isolate_(isolate),
315315
isolate_data_(isolate_data),
316-
privileged_access_context_(isolate_data->options()->per_env.get()),
317316
async_hooks_(isolate, MAYBE_FIELD_PTR(env_info, async_hooks)),
318317
immediate_info_(isolate, MAYBE_FIELD_PTR(env_info, immediate_info)),
319318
tick_info_(isolate, MAYBE_FIELD_PTR(env_info, tick_info)),

src/env.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
#include "node_main_instance.h"
3838
#include "node_options.h"
3939
#include "node_perf_common.h"
40-
#include "policy/policy.h"
4140
#include "req_wrap.h"
4241
#include "util.h"
4342
#include "uv.h"
@@ -1040,8 +1039,6 @@ class Environment : public MemoryRetainer {
10401039
inline const std::vector<std::string>& argv();
10411040
const std::string& exec_path() const;
10421041

1043-
inline policy::PrivilegedAccessContext* privileged_access_context();
1044-
10451042
typedef void (*HandleCleanupCb)(Environment* env,
10461043
uv_handle_t* handle,
10471044
void* arg);
@@ -1189,6 +1186,9 @@ class Environment : public MemoryRetainer {
11891186
inline node_module* extra_linked_bindings_head();
11901187
inline const Mutex& extra_linked_bindings_mutex() const;
11911188

1189+
inline void set_in_privileged_scope(bool on = true);
1190+
inline bool in_privileged_scope() const;
1191+
11921192
inline bool filehandle_close_warning() const;
11931193
inline void set_filehandle_close_warning(bool on);
11941194

@@ -1403,7 +1403,6 @@ class Environment : public MemoryRetainer {
14031403
std::list<binding::DLib> loaded_addons_;
14041404
v8::Isolate* const isolate_;
14051405
IsolateData* const isolate_data_;
1406-
policy::PrivilegedAccessContext privileged_access_context_;
14071406
uv_timer_t timer_handle_;
14081407
uv_check_t immediate_check_handle_;
14091408
uv_idle_t immediate_idle_handle_;
@@ -1425,6 +1424,8 @@ class Environment : public MemoryRetainer {
14251424
size_t async_callback_scope_depth_ = 0;
14261425
std::vector<double> destroy_async_id_list_;
14271426

1427+
bool in_privileged_scope_ = false;
1428+
14281429
#if HAVE_INSPECTOR
14291430
std::unique_ptr<profiler::V8CoverageConnection> coverage_connection_;
14301431
std::unique_ptr<profiler::V8CpuProfilerConnection> cpu_profiler_connection_;

src/node.cc

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,7 @@ void Environment::InitializeDiagnostics() {
298298
bool Environment::BootstrapPrivilegedAccessContext() {
299299
Local<Function> run_in_privileged_scope;
300300
MaybeLocal<Function> maybe_run_in_privileged_scope =
301-
Function::New(
302-
context(),
303-
policy::PrivilegedAccessContext::Run);
301+
Function::New(context(), policy::RunInPrivilegedScope);
304302
if (!maybe_run_in_privileged_scope.ToLocal(&run_in_privileged_scope))
305303
return false;
306304
set_run_in_privileged_scope(run_in_privileged_scope);
@@ -818,6 +816,14 @@ int ProcessGlobalArgs(std::vector<std::string>* args,
818816
}
819817
}
820818

819+
if (per_process::root_policy.Apply(
820+
per_process::cli_options->policy_deny,
821+
per_process::cli_options->policy_grant).IsNothing()) {
822+
errors->emplace_back(
823+
"invalid permissions passed to --policy-deny or --policy-grant");
824+
return 12;
825+
}
826+
821827
if (per_process::cli_options->disable_proto != "delete" &&
822828
per_process::cli_options->disable_proto != "throw" &&
823829
per_process::cli_options->disable_proto != "") {

src/node_options.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -322,14 +322,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
322322
&EnvironmentOptions::experimental_policy_integrity,
323323
kAllowedInEnvironment);
324324
Implies("--policy-integrity", "[has_policy_integrity_string]");
325-
AddOption("--policy-deny",
326-
"denied permissions",
327-
&EnvironmentOptions::policy_deny,
328-
kAllowedInEnvironment);
329-
AddOption("--policy-grant",
330-
"granted permissions",
331-
&EnvironmentOptions::policy_grant,
332-
kAllowedInEnvironment);
333325
AddOption("--experimental-repl-await",
334326
"experimental await keyword support in REPL",
335327
&EnvironmentOptions::experimental_repl_await,
@@ -712,6 +704,14 @@ PerProcessOptionsParser::PerProcessOptionsParser(
712704
"generate diagnostic report on fatal (internal) errors",
713705
&PerProcessOptions::report_on_fatalerror,
714706
kAllowedInEnvironment);
707+
AddOption("--policy-deny",
708+
"denied permissions",
709+
&PerProcessOptions::policy_deny,
710+
kAllowedInEnvironment);
711+
AddOption("--policy-grant",
712+
"granted permissions",
713+
&PerProcessOptions::policy_grant,
714+
kAllowedInEnvironment);
715715

716716
#ifdef NODE_HAVE_I18N_SUPPORT
717717
AddOption("--icu-data-dir",

src/node_options.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,6 @@ class EnvironmentOptions : public Options {
177177

178178
std::vector<std::string> preload_modules;
179179

180-
std::string policy_deny;
181-
std::string policy_grant;
182-
183180
std::vector<std::string> user_argv;
184181

185182
inline DebugOptions* get_debug_options() { return &debug_options_; }
@@ -263,6 +260,9 @@ class PerProcessOptions : public Options {
263260
bool trace_sigint = false;
264261
std::vector<std::string> cmdline;
265262

263+
std::string policy_grant;
264+
std::string policy_deny;
265+
266266
inline PerIsolateOptions* get_per_isolate_options();
267267
void CheckOptions(std::vector<std::string>* errors) override;
268268
};

0 commit comments

Comments
 (0)