From 9c5928b5aa3716f7441694da24982cecacb7a061 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sun, 17 Apr 2022 17:47:24 +0200 Subject: fix: panic when trying to pledge permissions before restoring previous pledge (#14306) This commit fixes and edge case, where testing/benching code could pledge new permission set before restoring the previous pledge. Appropriate panics were added and tests that assert that process is killed in case of "recursive pledge". --- cli/ops/bench.rs | 4 ++++ cli/ops/testing.rs | 3 +++ cli/tests/integration/bench_tests.rs | 19 +++++++++++++++++++ cli/tests/integration/test_tests.rs | 18 ++++++++++++++++++ .../testdata/bench/recursive_permissions_pledge.js | 8 ++++++++ .../testdata/test/recursive_permissions_pledge.js | 8 ++++++++ 6 files changed, 60 insertions(+) create mode 100644 cli/tests/testdata/bench/recursive_permissions_pledge.js create mode 100644 cli/tests/testdata/test/recursive_permissions_pledge.js (limited to 'cli') diff --git a/cli/ops/bench.rs b/cli/ops/bench.rs index ea040b4a5..6f4b80974 100644 --- a/cli/ops/bench.rs +++ b/cli/ops/bench.rs @@ -63,6 +63,10 @@ pub fn op_pledge_test_permissions( let worker_permissions = create_child_permissions(parent_permissions, args)?; let parent_permissions = parent_permissions.clone(); + if state.try_take::().is_some() { + panic!("pledge test permissions called before restoring previous pledge"); + } + state.put::(PermissionsHolder(token, parent_permissions)); // NOTE: This call overrides current permission set for the worker diff --git a/cli/ops/testing.rs b/cli/ops/testing.rs index 16544dd98..3a57d307b 100644 --- a/cli/ops/testing.rs +++ b/cli/ops/testing.rs @@ -122,6 +122,9 @@ pub fn op_pledge_test_permissions( let worker_permissions = create_child_permissions(parent_permissions, args)?; let parent_permissions = parent_permissions.clone(); + if state.try_take::().is_some() { + panic!("pledge test permissions called before restoring previous pledge"); + } state.put::(PermissionsHolder(token, parent_permissions)); // NOTE: This call overrides current permission set for the worker diff --git a/cli/tests/integration/bench_tests.rs b/cli/tests/integration/bench_tests.rs index 2df08bdb5..7b4fbb0a5 100644 --- a/cli/tests/integration/bench_tests.rs +++ b/cli/tests/integration/bench_tests.rs @@ -1,6 +1,7 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. use crate::itest; +use test_util as util; itest!(requires_unstable { args: "bench bench/requires_unstable.js", @@ -139,3 +140,21 @@ itest!(no_prompt_with_denied_perms { exit_code: 1, output: "bench/no_prompt_with_denied_perms.out", }); + +#[test] +fn recursive_permissions_pledge() { + let output = util::deno_cmd() + .current_dir(util::testdata_path()) + .arg("bench") + .arg("--unstable") + .arg("bench/recursive_permissions_pledge.js") + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(!output.status.success()); + assert!(String::from_utf8(output.stderr).unwrap().contains( + "pledge test permissions called before restoring previous pledge" + )); +} diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs index 6a0d5c1ab..bac50f16d 100644 --- a/cli/tests/integration/test_tests.rs +++ b/cli/tests/integration/test_tests.rs @@ -298,3 +298,21 @@ itest!(no_prompt_with_denied_perms { exit_code: 1, output: "test/no_prompt_with_denied_perms.out", }); + +#[test] +fn recursive_permissions_pledge() { + let output = util::deno_cmd() + .current_dir(util::testdata_path()) + .arg("test") + .arg("test/recursive_permissions_pledge.js") + .stderr(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(!output.status.success()); + assert!(String::from_utf8(output.stderr).unwrap().contains( + "pledge test permissions called before restoring previous pledge" + )); +} diff --git a/cli/tests/testdata/bench/recursive_permissions_pledge.js b/cli/tests/testdata/bench/recursive_permissions_pledge.js new file mode 100644 index 000000000..dcdcbf574 --- /dev/null +++ b/cli/tests/testdata/bench/recursive_permissions_pledge.js @@ -0,0 +1,8 @@ +Deno.core.opSync( + "op_pledge_test_permissions", + "none", +); +Deno.core.opSync( + "op_pledge_test_permissions", + "inherit", +); diff --git a/cli/tests/testdata/test/recursive_permissions_pledge.js b/cli/tests/testdata/test/recursive_permissions_pledge.js new file mode 100644 index 000000000..dcdcbf574 --- /dev/null +++ b/cli/tests/testdata/test/recursive_permissions_pledge.js @@ -0,0 +1,8 @@ +Deno.core.opSync( + "op_pledge_test_permissions", + "none", +); +Deno.core.opSync( + "op_pledge_test_permissions", + "inherit", +); -- cgit v1.2.3