diff --git a/backend/src/context/cli.rs b/backend/src/context/cli.rs index aa8ab59f4..25adecdde 100644 --- a/backend/src/context/cli.rs +++ b/backend/src/context/cli.rs @@ -81,7 +81,7 @@ impl CliContext { .chain(std::iter::once(Path::new(crate::util::config::CONFIG_PATH))), )?; let mut url = if let Some(host) = matches.value_of("host") { - Url::parse host.parse()? + host.parse()? } else if let Some(host) = base.host { host } else { diff --git a/backend/src/control.rs b/backend/src/control.rs index 56d4e6788..1922fdc81 100644 --- a/backend/src/control.rs +++ b/backend/src/control.rs @@ -173,10 +173,28 @@ pub async fn stop_dry( pub async fn stop_impl(ctx: RpcContext, id: PackageId) -> Result { let mut db = ctx.db.handle(); let mut tx = db.begin().await?; + let version = crate::db::DatabaseModel::new() + .package_data() + .idx_model(&id) + .expect(&mut tx) + .await? + .installed() + .expect(&mut tx) + .await? + .manifest() + .version() + .get(&mut tx) + .await? + .clone(); let last_statuts = stop_common(&mut tx, &id, &mut BTreeMap::new()).await?; tx.commit().await?; + ctx.managers + .get(&(id, version)) + .await + .ok_or_else(|| Error::new(eyre!("Manager not found"), crate::ErrorKind::InvalidRequest))? + .stop(); Ok(last_statuts) } @@ -185,23 +203,28 @@ pub async fn stop_impl(ctx: RpcContext, id: PackageId) -> Result Result<(), Error> { let mut db = ctx.db.handle(); let mut tx = db.begin().await?; - - let mut status = crate::db::DatabaseModel::new() + let version = crate::db::DatabaseModel::new() .package_data() .idx_model(&id) - .and_then(|pde| pde.installed()) - .map(|i| i.status().main()) - .get_mut(&mut tx) - .await?; - if !matches!(&*status, Some(MainStatus::Running { .. })) { - return Err(Error::new( - eyre!("{} is not running", id), - crate::ErrorKind::InvalidRequest, - )); - } - *status = Some(MainStatus::Restarting); - status.save(&mut tx).await?; + .expect(&mut tx) + .await? + .installed() + .expect(&mut tx) + .await? + .manifest() + .version() + .get(&mut tx) + .await? + .clone(); + tx.commit().await?; + ctx.managers + .get(&(id, version)) + .await + .ok_or_else(|| Error::new(eyre!("Manager not found"), crate::ErrorKind::InvalidRequest))? + .restart() + .await; + Ok(()) } diff --git a/backend/src/dependencies.rs b/backend/src/dependencies.rs index af56f50e2..6a79b4bce 100644 --- a/backend/src/dependencies.rs +++ b/backend/src/dependencies.rs @@ -1047,7 +1047,7 @@ pub fn heal_transitive<'a, Db: DbHandle>( pub async fn reconfigure_dependents_with_live_pointers( ctx: &RpcContext, - mut tx: impl DbHandle, + tx: impl DbHandle, receipts: &ConfigReceipts, pde: &InstalledPackageDataEntry, ) -> Result<(), Error> { diff --git a/backend/src/manager/health.rs b/backend/src/manager/health.rs index 58512ba89..1ec89c26c 100644 --- a/backend/src/manager/health.rs +++ b/backend/src/manager/health.rs @@ -1,5 +1,4 @@ use std::collections::BTreeMap; -use std::sync::atomic::{AtomicBool, Ordering}; use itertools::Itertools; use patch_db::{DbHandle, LockReceipt, LockType}; diff --git a/backend/src/manager/js_api.rs b/backend/src/manager/js_api.rs index 65174c7c3..d2bd7ba8b 100644 --- a/backend/src/manager/js_api.rs +++ b/backend/src/manager/js_api.rs @@ -147,7 +147,6 @@ impl OsApi for Manager { .create_service(self.seed.manifest.id.clone(), ip) .await .map_err(|e| eyre!("Could not get to net controller: {e:?}"))?; - let mut secrets = self.seed.ctx.secret_store.acquire().await?; svc.remove_lan(id, external) .await @@ -165,7 +164,6 @@ impl OsApi for Manager { .create_service(self.seed.manifest.id.clone(), ip) .await .map_err(|e| eyre!("Could not get to net controller: {e:?}"))?; - let mut secrets = self.seed.ctx.secret_store.acquire().await?; svc.remove_tor(id, external) .await @@ -173,30 +171,26 @@ impl OsApi for Manager { Ok(()) } - fn set_started(&self) -> Result<(), Report> { + fn set_started(&self) { self.manage_container .current_state .send(StartStop::Start) - .unwrap_or_default(); - Ok(()) + .unwrap_or_default() } - async fn restart(&self) -> Result<(), Report> { - self.perform_restart().await; - Ok(()) + async fn restart(&self) { + self.perform_restart().await } - async fn start(&self) -> Result<(), Report> { + async fn start(&self) { self.manage_container .wait_for_desired(StartStop::Start) - .await; - Ok(()) + .await } - async fn stop(&self) -> Result<(), Report> { + async fn stop(&self) { self.manage_container .wait_for_desired(StartStop::Stop) - .await; - Ok(()) + .await } } diff --git a/backend/src/manager/manager_container.rs b/backend/src/manager/manager_container.rs index e0eb153f0..afce929f0 100644 --- a/backend/src/manager/manager_container.rs +++ b/backend/src/manager/manager_container.rs @@ -1,12 +1,15 @@ use std::sync::Arc; use std::time::Duration; +use futures::FutureExt; use patch_db::PatchDbHandle; use tokio::sync::watch; use tokio::sync::watch::Sender; +use tracing::instrument; use super::start_stop::StartStop; use super::{manager_seed, run_main, ManagerPersistentContainer, RunMainResult}; +use crate::procedure::NoOutput; use crate::s9pk::manifest::Manifest; use crate::status::MainStatus; use crate::util::{GeneralBoxedGuard, NonDetachingJoinHandle}; @@ -15,10 +18,10 @@ use crate::Error; pub type ManageContainerOverride = Arc>>; pub struct ManageContainer { - current_state: Arc>, - desired_state: Arc>, - service: NonDetachingJoinHandle<()>, - save_state: NonDetachingJoinHandle<()>, + pub(super) current_state: Arc>, + pub(super) desired_state: Arc>, + _service: NonDetachingJoinHandle<()>, + _save_state: NonDetachingJoinHandle<()>, override_main_status: ManageContainerOverride, } @@ -30,7 +33,7 @@ impl ManageContainer { let mut db = seed.ctx.db.handle(); let current_state = Arc::new(watch::channel(StartStop::Stop).0); let desired_state = Arc::new( - watch::channel::(get_status(&mut db, &seed.manifest).await?.into()).0, + watch::channel::(get_status(&mut db, &seed.manifest).await.into()).0, ); let override_main_status: ManageContainerOverride = Arc::new(watch::channel(None).0); let service = tokio::spawn(create_service_manager( @@ -50,17 +53,19 @@ impl ManageContainer { Ok(ManageContainer { current_state, desired_state, - service, + _service: service, override_main_status, - save_state, + _save_state: save_state, }) } pub fn set_override(&self, override_status: Option) -> GeneralBoxedGuard { - self.override_main_status.send(override_status); + self.override_main_status + .send(override_status) + .unwrap_or_default(); let override_main_status = self.override_main_status.clone(); let guard = GeneralBoxedGuard::new(move || { - override_main_status.send(None); + override_main_status.send(None).unwrap_or_default(); }); guard } @@ -70,9 +75,9 @@ impl ManageContainer { } pub async fn wait_for_desired(&self, new_state: StartStop) { - let mut current_state = self.current_state(); + let current_state = self.current_state(); self.to_desired(new_state); - while *current_state.borrow() != new_state { + while current_state.borrow() != new_state { current_state.changed().await.unwrap_or_default(); } } @@ -99,14 +104,14 @@ async fn create_service_manager( let current: StartStop = current_state.borrow().clone(); let desired: StartStop = desired_state_receiver.borrow().clone(); match (current, desired) { - (StartStop::Start, StartStop::Start) => continue, + (StartStop::Start, StartStop::Start) => (), (StartStop::Start, StartStop::Stop) => { if let Err(err) = seed.stop_container().await { tracing::error!("Could not stop container"); tracing::debug!("{:?}", err) }; running_service = None; - current_state.send(StartStop::Stop); + current_state.send(StartStop::Stop).unwrap_or_default(); } (StartStop::Stop, StartStop::Start) => starting_service( current_state.clone(), @@ -115,8 +120,9 @@ async fn create_service_manager( persistent_container.clone(), &mut running_service, ), - (StartStop::Stop, StartStop::Stop) => continue, + (StartStop::Stop, StartStop::Stop) => (), } + if let Err(_) = desired_state_receiver.changed().await { tracing::error!("Desired state error"); break; @@ -188,7 +194,7 @@ fn starting_service( let set_running = { let current_state = current_state.clone(); Arc::new(move || { - current_state.send(StartStop::Start); + current_state.send(StartStop::Start).unwrap_or_default(); }) }; let set_stopped = { @@ -203,8 +209,8 @@ fn starting_service( set_running.clone(), ) .await; - run_main_log_result(result, seed.clone()); - set_stopped(); + set_stopped().unwrap_or_default(); + run_main_log_result(result, seed.clone()).await; } }; *running_service = Some(tokio::spawn(running_main_loop).into()); @@ -217,24 +223,23 @@ async fn run_main_log_result(result: RunMainResult, seed: Arc(|i| i.status().main()) - .get(db, false) + .get(&mut db) .await; match started.as_deref() { Ok(Some(MainStatus::Running { .. })) => { - let res = thread_shared.seed.ctx.notification_manager + let res = seed.ctx.notification_manager .notify( - db, - Some(thread_shared.seed.manifest.id.clone()), + &mut db, + Some(seed.manifest.id.clone()), NotificationLevel::Warning, String::from("Service Crashed"), - format!("The service {} has crashed with the following exit code: {}\nDetails: {}", thread_shared.seed.manifest.id.clone(), e.0, e.1), + format!("The service {} has crashed with the following exit code: {}\nDetails: {}", seed.manifest.id.clone(), e.0, e.1), (), Some(3600) // 1 hour ) @@ -249,6 +254,11 @@ async fn run_main_log_result(result: RunMainResult, seed: Arc Result { - Ok(crate::db::DatabaseModel::new() - .package_data() - .idx_model(&manifest.id) - .expect(db) - .await? - .installed() - .expect(db) - .await? - .status() - .main() - .get(db) - .await? - .clone()) +#[instrument(skip(db, manifest))] +pub(super) async fn get_status(db: &mut PatchDbHandle, manifest: &Manifest) -> MainStatus { + async move { + Ok::<_, Error>( + crate::db::DatabaseModel::new() + .package_data() + .idx_model(&manifest.id) + .expect(db) + .await? + .installed() + .expect(db) + .await? + .status() + .main() + .get(db) + .await? + .clone(), + ) + } + .map(|x| x.unwrap_or_else(|e| MainStatus::Stopped)) + .await } +#[instrument(skip(db, manifest))] async fn set_status( db: &mut PatchDbHandle, manifest: &Manifest, main_status: &MainStatus, ) -> Result<(), Error> { - crate::db::DatabaseModel::new() + if crate::db::DatabaseModel::new() .package_data() .idx_model(&manifest.id) .expect(db) .await? .installed() - .expect(db) + .exists(db) .await? - .status() - .main() - .put(db, main_status) - .await?; + { + crate::db::DatabaseModel::new() + .package_data() + .idx_model(&manifest.id) + .expect(db) + .await? + .installed() + .expect(db) + .await? + .status() + .main() + .put(db, main_status) + .await?; + } Ok(()) } diff --git a/backend/src/manager/mod.rs b/backend/src/manager/mod.rs index 357517a58..be9db06c7 100644 --- a/backend/src/manager/mod.rs +++ b/backend/src/manager/mod.rs @@ -7,7 +7,7 @@ use std::time::Duration; use color_eyre::{eyre::eyre, Report}; use embassy_container_init::ProcessGroupId; use futures::future::BoxFuture; -use futures::{FutureExt, TryFutureExt}; +use futures::{Future, FutureExt, TryFutureExt}; use helpers::UnixRpcClient; use models::{ErrorKind, PackageId}; use nix::sys::signal::Signal; @@ -168,19 +168,22 @@ impl Manager { self._transition_replace({ let manage_container = self.manage_container.clone(); - TransitionState::Configuring(tokio::spawn(async move { - let desired_state = manage_container.desired_state(); - let state_reverter = DesiredStateReverter::new(manage_container.clone()); - let mut current_state = manage_container.current_state(); - manage_container.to_desired(StartStop::Stop); - while current_state.borrow().is_start() { - current_state.changed().await.unwrap(); - } + TransitionState::Configuring( + tokio::spawn(async move { + let desired_state = manage_container.desired_state(); + let state_reverter = DesiredStateReverter::new(manage_container.clone()); + let mut current_state = manage_container.current_state(); + manage_container.to_desired(StartStop::Stop); + while current_state.borrow().is_start() { + current_state.changed().await.unwrap(); + } - transition_state.await; + transition_state.await; - state_reverter.revert().await; - })) + state_reverter.revert().await; + }) + .into(), + ) }); done.await } @@ -230,69 +233,76 @@ impl Manager { .send_replace(Arc::new(transition_state)) .abort(); } + + pub(super) fn perform_restart(&self) -> impl Future + 'static { + let manage_container = self.manage_container.clone(); + async move { + let _ = manage_container.set_override(Some(MainStatus::Restarting)); + manage_container.wait_for_desired(StartStop::Stop).await; + manage_container.wait_for_desired(StartStop::Start).await; + } + } fn _transition_restart(&self) -> TransitionState { let transition = self.transition.clone(); + let restart = self.perform_restart(); + TransitionState::Restarting( + tokio::spawn(async move { + restart.await; + transition.send_replace(Default::default()); + }) + .into(), + ) + } + fn perform_backup( + &self, + backup_guard: BackupGuard, + ) -> impl Future, Error>> + 'static { let manage_container = self.manage_container.clone(); - TransitionState::Restarting(tokio::spawn(async move { - let mut current_state = manage_container.current_state(); - let _ = manage_container.set_override(Some(MainStatus::Restarting)); - manage_container.to_desired(StartStop::Stop); - while current_state.borrow().is_start() { - current_state.changed().await.unwrap(); - } - manage_container.to_desired(StartStop::Start); - while current_state.borrow().is_stop() { - current_state.changed().await.unwrap(); - } - transition.send_replace(Default::default()); - })) + let seed = self.seed.clone(); + async move { + let state_reverter = DesiredStateReverter::new(manage_container.clone()); + let mut tx = seed.ctx.db.handle(); + let _ = manage_container + .set_override(Some(get_status(&mut tx, &seed.manifest).await.backing_up())); + manage_container.wait_for_desired(StartStop::Stop).await; + + let backup_guard = backup_guard.lock().await; + let guard = backup_guard.mount_package_backup(&seed.manifest.id).await?; + + let res = seed + .manifest + .backup + .create( + &seed.ctx, + &mut tx, + &seed.manifest.id, + &seed.manifest.title, + &seed.manifest.version, + &seed.manifest.interfaces, + &seed.manifest.volumes, + ) + .await; + guard.unmount().await?; + drop(backup_guard); + + let return_value = res; + state_reverter.revert().await; + Ok::<_, Error>(return_value) + } } fn _transition_backup( &self, backup_guard: BackupGuard, ) -> (TransitionState, BoxFuture) { - let manage_container = self.manage_container.clone(); - let seed = self.seed.clone(); let (send, done) = oneshot::channel(); ( - TransitionState::BackingUp(tokio::spawn( - async move { - let state_reverter = DesiredStateReverter::new(manage_container.clone()); - let mut current_state = manage_container.current_state(); - let mut tx = seed.ctx.db.handle(); - let _ = manage_container.set_override(Some( - get_status(&mut tx, &seed.manifest).await?.backing_up(), - )); - manage_container.to_desired(StartStop::Stop); - while current_state.borrow().is_start() { - current_state.changed().await.unwrap(); - } - - let backup_guard = backup_guard.lock().await; - let guard = backup_guard.mount_package_backup(&seed.manifest.id).await?; - - let res = seed - .manifest - .backup - .create( - &seed.ctx, - &mut tx, - &seed.manifest.id, - &seed.manifest.title, - &seed.manifest.version, - &seed.manifest.interfaces, - &seed.manifest.volumes, - ) - .await; - guard.unmount().await?; - drop(backup_guard); - - let return_value = res; - state_reverter.revert().await; - Ok::<_, Error>(return_value) - } - .then(finnish_up_backup_task(self.transition.clone(), send)), // - )), + TransitionState::BackingUp( + tokio::spawn( + self.perform_backup(backup_guard) + .then(finnish_up_backup_task(self.transition.clone(), send)), + ) + .into(), + ), done.map_err(|err| Error::new(eyre!("Oneshot error: {err:?}"), ErrorKind::Unknown)) .map(flatten_backup_error) .boxed(), @@ -683,11 +693,13 @@ async fn run_main( }; let svc = if let Some(ip) = ip { - Some(add_network_for_main(&*seed, ip).await?) + let net = add_network_for_main(&seed, ip).await?; + started(); + Some(net) } else { None }; - started(); + let health = main_health_check_daemon(seed.clone()); let res = tokio::select! { a = runtime => a.map_err(|_| Error::new(eyre!("Manager runtime panicked!"), crate::ErrorKind::Docker)).and_then(|a| a), @@ -774,6 +786,7 @@ async fn get_long_running_ip(seed: &ManagerSeed, runtime: &mut LongRunning) -> G } } +#[instrument(skip(seed))] async fn container_inspect( seed: &ManagerSeed, ) -> Result { @@ -783,6 +796,7 @@ async fn container_inspect( .await } +#[instrument(skip(seed))] async fn add_network_for_main( seed: &ManagerSeed, ip: std::net::Ipv4Addr, @@ -814,6 +828,7 @@ async fn add_network_for_main( Ok(svc) } +#[instrument(skip(svc))] async fn remove_network_for_main(svc: NetService) -> Result<(), Error> { svc.remove_all().await } @@ -848,6 +863,7 @@ async fn try_get_running_ip(seed: &ManagerSeed) -> Result, Repo .transpose()?) } +#[instrument(skip(seed, runtime))] async fn get_running_ip(seed: &ManagerSeed, mut runtime: &mut RuntimeOfCommand) -> GetRunningIp { loop { match container_inspect(seed).await { diff --git a/backend/src/manager/transition_state.rs b/backend/src/manager/transition_state.rs index 5b615e67e..bea191538 100644 --- a/backend/src/manager/transition_state.rs +++ b/backend/src/manager/transition_state.rs @@ -1,16 +1,14 @@ -use tokio::task::JoinHandle; +use helpers::NonDetachingJoinHandle; pub(crate) enum TransitionState { - // Starting(JoinHandle<()>), - // Stopping(JoinHandle<()>) - BackingUp(JoinHandle<()>), - Restarting(JoinHandle<()>), - Configuring(JoinHandle<()>), + BackingUp(NonDetachingJoinHandle<()>), + Restarting(NonDetachingJoinHandle<()>), + Configuring(NonDetachingJoinHandle<()>), None, } impl TransitionState { - pub(crate) fn join_handle(&self) -> Option<&JoinHandle<()>> { + pub(crate) fn join_handle(&self) -> Option<&NonDetachingJoinHandle<()>> { Some(match self { TransitionState::BackingUp(a) => a, TransitionState::Restarting(a) => a, diff --git a/backend/src/net/keys.rs b/backend/src/net/keys.rs index d406df9c0..70c041f0b 100644 --- a/backend/src/net/keys.rs +++ b/backend/src/net/keys.rs @@ -8,12 +8,14 @@ use p256::elliptic_curve::pkcs8::EncodePrivateKey; use sqlx::PgExecutor; use ssh_key::private::Ed25519PrivateKey; use torut::onion::{OnionAddressV3, TorSecretKeyV3}; +use tracing::instrument; use zeroize::Zeroize; use crate::net::ssl::CertPair; use crate::Error; // TODO: delete once we may change tor addresses +#[instrument(skip(secrets))] async fn compat( secrets: impl PgExecutor<'_>, interface: &Option<(PackageId, InterfaceId)>, @@ -182,6 +184,7 @@ impl Key { }) .collect() } + #[instrument(skip(secrets))] pub async fn for_interface( secrets: &mut Ex, interface: Option<(PackageId, InterfaceId)>, diff --git a/backend/src/net/net_controller.rs b/backend/src/net/net_controller.rs index c04707d37..db695f639 100644 --- a/backend/src/net/net_controller.rs +++ b/backend/src/net/net_controller.rs @@ -215,6 +215,7 @@ pub struct NetService { lan: BTreeMap<(InterfaceId, u16), (Key, Vec>)>, } impl NetService { + #[instrument(skip(self))] fn net_controller(&self) -> Result, Error> { Weak::upgrade(&self.controller).ok_or_else(|| { Error::new( @@ -223,6 +224,7 @@ impl NetService { ) }) } + #[instrument(skip(self, secrets))] pub async fn add_tor( &mut self, secrets: &mut Ex, diff --git a/backend/src/ssh.rs b/backend/src/ssh.rs index 486a75097..091f2ccbe 100644 --- a/backend/src/ssh.rs +++ b/backend/src/ssh.rs @@ -5,7 +5,6 @@ use clap::ArgMatches; use color_eyre::eyre::eyre; use rpc_toolkit::command; use sqlx::{Executor, Pool, Postgres}; -use ssh_key::private::Ed25519PrivateKey; use tracing::instrument; use crate::context::RpcContext; diff --git a/backend/test/js_action_execute/package-data/scripts/test-package/0.3.0.3/embassy.js b/backend/test/js_action_execute/package-data/scripts/test-package/0.3.0.3/embassy.js index c14017b1d..b9d9c009d 100644 --- a/backend/test/js_action_execute/package-data/scripts/test-package/0.3.0.3/embassy.js +++ b/backend/test/js_action_execute/package-data/scripts/test-package/0.3.0.3/embassy.js @@ -10,7 +10,7 @@ export async function getConfig(effects) { volumeId: "main", }); throw new Error( - "Expecting that the ../test.log should not be a valid path since we are breaking out of the parent" + "Expecting that the ../test.log should not be a valid path since we are breaking out of the parent", ); } catch (e) {} try { @@ -20,7 +20,7 @@ export async function getConfig(effects) { volumeId: "main", }); throw new Error( - "Expecting that using a symlink to break out of parent still fails for writing" + "Expecting that using a symlink to break out of parent still fails for writing", ); } catch (e) {} try { @@ -29,7 +29,7 @@ export async function getConfig(effects) { volumeId: "main", }); throw new Error( - "Expecting that using a symlink to break out of parent still fails for writing dir" + "Expecting that using a symlink to break out of parent still fails for writing dir", ); } catch (e) {} try { @@ -38,7 +38,7 @@ export async function getConfig(effects) { volumeId: "main", }); throw new Error( - "Expecting that using a symlink to break out of parent still fails for reading" + "Expecting that using a symlink to break out of parent still fails for reading", ); } catch (e) {} @@ -81,7 +81,7 @@ export async function getConfig(effects) { `Read results are ${await effects.readFile({ path: "./test.log", volumeId: "main", - })}` + })}`, ); // Testing loging effects.trace("trace"); @@ -730,48 +730,47 @@ export async function setConfig(effects) { const assert = (condition, message) => { if (!condition) { - throw ({error: message}); + throw ({ error: message }); } }; const ackermann = (m, n) => { if (m === 0) { - return n+1 + return n + 1; } if (n === 0) { - return ackermann((m - 1), 1); + return ackermann(m - 1, 1); } if (m !== 0 && n !== 0) { - return ackermann((m-1), ackermann(m, (n-1))) + return ackermann(m - 1, ackermann(m, n - 1)); } -} +}; export const action = { async slow(effects, _input) { - while(true) { + while (true) { effects.error("A"); - await ackermann(3,10); + await ackermann(3, 10); // await effects.sleep(100); - } }, async fetch(effects, _input) { const example = await effects.fetch( - "https://postman-echo.com/get?foo1=bar1&foo2=bar2" + "https://postman-echo.com/get?foo1=bar1&foo2=bar2", ); assert( Number(example.headers["content-length"]) > 0 && Number(example.headers["content-length"]) <= 1000000, - "Should have content length" + "Should have content length", ); assert( example.text() instanceof Promise, - "example.text() should be a promise" + "example.text() should be a promise", ); assert(example.body === undefined, "example.body should not be defined"); assert( JSON.parse(await example.text()).args.foo1 === "bar1", - "Body should be parsed" + "Body should be parsed", ); const message = `This worked @ ${new Date().toISOString()}`; const secondResponse = await effects.fetch( @@ -782,11 +781,11 @@ export const action = { headers: { test: "1234", }, - } + }, ); assert( (await secondResponse.json()).json.message === message, - "Body should be parsed from response" + "Body should be parsed from response", ); return { result: { @@ -844,7 +843,6 @@ export const action = { failed = true; } assert(failed, "Should not be able to remove file that doesn't exist"); - return { result: { @@ -857,12 +855,12 @@ export const action = { }, /** * Created this test because of issue - * https://github.com/Start9Labs/embassy-os/issues/1737 + * https://github.com/Start9Labs/embassy-os/issues/1737 * which that we couldn't create a dir that was deeply nested, and the parents where * not created yet. Found this out during the migrations, where the parent would die. - * @param {*} effects - * @param {*} _input - * @returns + * @param {*} effects + * @param {*} _input + * @returns */ async "test-deep-dir"(effects, _input) { await effects @@ -935,9 +933,9 @@ export const action = { * Created this test because of issue * https://github.com/Start9Labs/embassy-os/issues/2121 * That the empty in the create dies - * @param {*} effects - * @param {*} _input - * @returns + * @param {*} effects + * @param {*} _input + * @returns */ async "test-zero-dir"(effects, _input) { await effects.createDir({ @@ -955,9 +953,9 @@ export const action = { }, /** * Found case where we could escape with the new deeper dir fix. - * @param {*} effects - * @param {*} _input - * @returns + * @param {*} effects + * @param {*} _input + * @returns */ async "test-deep-dir-escape"(effects, _input) { await effects @@ -969,7 +967,9 @@ export const action = { await effects.createDir({ volumeId: "main", path: "test-deep-dir/../../test", - }).then(_ => {throw new Error("Should not be able to create sub")}, _ => {}); + }).then((_) => { + throw new Error("Should not be able to create sub"); + }, (_) => {}); return { result: { @@ -981,12 +981,11 @@ export const action = { }; }, - /** * Want to test that rsync works - * @param {*} effects - * @param {*} _input - * @returns + * @param {*} effects + * @param {*} _input + * @returns */ async "test-rsync"(effects, _input) { try { @@ -1005,17 +1004,22 @@ export const action = { delete: true, force: true, ignoreExisting: false, - } + }, }); assert(await runningRsync.id() >= 1, "Expect that we have an id"); - const progress = await runningRsync.progress() - assert(progress >= 0 && progress <= 1, `Expect progress to be 0 <= progress(${progress}) <= 1`); + const progress = await runningRsync.progress(); + assert( + progress >= 0 && progress <= 1, + `Expect progress to be 0 <= progress(${progress}) <= 1`, + ); await runningRsync.wait(); - assert((await effects.readFile({ - volumeId: "main", - path: "test-rsync-out/testing-rsync/someFile.txt", - })).length > 0, 'Asserting that we read in the file "test_rsync/test-package/0.3.0.3/embassy.js"'); - + assert( + (await effects.readFile({ + volumeId: "main", + path: "test-rsync-out/testing-rsync/someFile.txt", + })).length > 0, + 'Asserting that we read in the file "test_rsync/test-package/0.3.0.3/embassy.js"', + ); return { result: { @@ -1025,11 +1029,9 @@ export const action = { qr: false, }, }; - } - catch (e) { + } catch (e) { throw e; - } - finally { + } finally { await effects .removeDir({ volumeId: "main", @@ -1040,18 +1042,24 @@ export const action = { }, /** * Testing callbacks? - * @param {*} effects - * @param {*} _input - * @returns + * @param {*} effects + * @param {*} _input + * @returns */ async "test-callback"(effects, _input) { await Promise.race([ - new Promise(done => effects.getServiceConfig({serviceId: 'something', configPath: "string", onChange: done})), - new Promise (async () => { - await effects.sleep(100) - throw new Error("Currently in sleeping") - } - )]) + new Promise((done) => + effects.getServiceConfig({ + serviceId: "something", + configPath: "string", + onChange: done, + }) + ), + new Promise(async () => { + await effects.sleep(100); + throw new Error("Currently in sleeping"); + }), + ]); return { result: { @@ -1064,13 +1072,13 @@ export const action = { }, /** - * We wanted to change the permissions and the ownership during the + * We wanted to change the permissions and the ownership during the * backing up, there where cases where the ownership is weird and * broke for non root users. * Note: Test for the chmod is broken and turned off because it only works when ran by root - * @param {*} effects - * @param {*} _input - * @returns + * @param {*} effects + * @param {*} _input + * @returns */ async "test-permission-chown"(effects, _input) { await effects @@ -1090,30 +1098,39 @@ export const action = { }); const firstMetaData = await effects.metadata({ - volumeId: 'main', - path: 'pem-chown/deep/123/test.txt', - }) - assert(firstMetaData.readonly === false, `The readonly (${firstMetaData.readonly}) is wrong`); + volumeId: "main", + path: "pem-chown/deep/123/test.txt", + }); + assert( + firstMetaData.readonly === false, + `The readonly (${firstMetaData.readonly}) is wrong`, + ); const previousUid = firstMetaData.uid; - const expected = 1234 + const expected = 1234; - await effects.setPermissions({ - volumeId: 'main', - path: 'pem-chown/deep/123/test.txt', - readonly: true - }) + await effects.chmod({ + volumeId: "main", + path: "pem-chown/deep/123/test.txt", + mode: 0o444, + }); const chownError = await effects.chown({ - volumeId: 'main', - path: 'pem-chown/deep', - uid: expected - }).then(() => true, () => false) + volumeId: "main", + path: "pem-chown/deep", + uid: expected, + }).then(() => true, () => false); let metaData = await effects.metadata({ - volumeId: 'main', - path: 'pem-chown/deep/123/test.txt', - }) - assert(metaData.readonly === true, `The readonly (${metaData.readonly}) is wrong`); + volumeId: "main", + path: "pem-chown/deep/123/test.txt", + }); if (chownError) { - assert(metaData.uid === expected, `The uuid (${metaData.uid}) is wrong, should be more than ${previousUid}`); + assert( + metaData.mode === 0o444, + `The mode (${metaData.mode}) is wrong compared to ${0o444}}`, + ); + assert( + metaData.uid === expected, + `The uuid (${metaData.uid}) is wrong, should be more than ${previousUid}`, + ); } return { @@ -1132,4 +1149,3 @@ export const action = { } }; - diff --git a/libs/helpers/src/lib.rs b/libs/helpers/src/lib.rs index f4290b795..d8dcf6c6d 100644 --- a/libs/helpers/src/lib.rs +++ b/libs/helpers/src/lib.rs @@ -1,4 +1,5 @@ use std::future::Future; +use std::ops::{Deref, DerefMut}; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -93,6 +94,17 @@ impl Future for NonDetachingJoinHandle { this.0.poll(cx) } } +impl Deref for NonDetachingJoinHandle { + type Target = JoinHandle; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl DerefMut for NonDetachingJoinHandle { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} pub struct AtomicFile { tmp_path: PathBuf, diff --git a/libs/helpers/src/os_api.rs b/libs/helpers/src/os_api.rs index 47735332f..96754cb3c 100644 --- a/libs/helpers/src/os_api.rs +++ b/libs/helpers/src/os_api.rs @@ -76,8 +76,8 @@ pub trait OsApi: Send + Sync + 'static { async fn unbind_local(&self, id: InterfaceId, external: u16) -> Result<(), Report>; async fn unbind_onion(&self, id: InterfaceId, external: u16) -> Result<(), Report>; - fn set_started(&self) -> Result<(), Report>; - async fn restart(&self) -> Result<(), Report>; - async fn start(&self) -> Result<(), Report>; - async fn stop(&self) -> Result<(), Report>; + fn set_started(&self); + async fn restart(&self); + async fn start(&self); + async fn stop(&self); } diff --git a/libs/js_engine/src/artifacts/loadModule.js b/libs/js_engine/src/artifacts/loadModule.js index 1998f7cc1..2c3be8cba 100644 --- a/libs/js_engine/src/artifacts/loadModule.js +++ b/libs/js_engine/src/artifacts/loadModule.js @@ -1,8 +1,6 @@ import Deno from "/deno_global.js"; import * as mainModule from "/embassy.js"; -// throw new Error("I'm going crasy") - function requireParam(param) { throw new Error(`Missing required parameter ${param}`); } @@ -49,10 +47,8 @@ const writeFile = ( ) => Deno.core.opAsync("write_file", volumeId, path, toWrite); const readFile = ( - { - volumeId = requireParam("volumeId"), - path = requireParam("path"), - } = requireParam("options"), + { volumeId = requireParam("volumeId"), path = requireParam("path") } = + requireParam("options"), ) => Deno.core.opAsync("read_file", volumeId, path); const runDaemon = ( @@ -74,11 +70,8 @@ const runDaemon = ( }; }; const runCommand = async ( - { - command = requireParam("command"), - args = [], - timeoutMillis = 30000, - } = requireParam("options"), + { command = requireParam("command"), args = [], timeoutMillis = 30000 } = + requireParam("options"), ) => { let id = Deno.core.opAsync( "start_command", @@ -128,10 +121,8 @@ const rename = ( } = requireParam("options"), ) => Deno.core.opAsync("rename", srcVolume, srcPath, dstVolume, dstPath); const metadata = async ( - { - volumeId = requireParam("volumeId"), - path = requireParam("path"), - } = requireParam("options"), + { volumeId = requireParam("volumeId"), path = requireParam("path") } = + requireParam("options"), ) => { const data = await Deno.core.opAsync("metadata", volumeId, path); return { @@ -142,10 +133,8 @@ const metadata = async ( }; }; const removeFile = ( - { - volumeId = requireParam("volumeId"), - path = requireParam("path"), - } = requireParam("options"), + { volumeId = requireParam("volumeId"), path = requireParam("path") } = + requireParam("options"), ) => Deno.core.opAsync("remove_file", volumeId, path); const isSandboxed = () => Deno.core.opSync("is_sandboxed"); @@ -182,26 +171,20 @@ const chmod = async ( return await Deno.core.opAsync("chmod", volumeId, path, mode); }; const readJsonFile = async ( - { - volumeId = requireParam("volumeId"), - path = requireParam("path"), - } = requireParam("options"), + { volumeId = requireParam("volumeId"), path = requireParam("path") } = + requireParam("options"), ) => JSON.parse(await readFile({ volumeId, path })); const createDir = ( - { - volumeId = requireParam("volumeId"), - path = requireParam("path"), - } = requireParam("options"), + { volumeId = requireParam("volumeId"), path = requireParam("path") } = + requireParam("options"), ) => Deno.core.opAsync("create_dir", volumeId, path); const readDir = ( { volumeId = requireParam("volumeId"), path = requireParam("path") } = requireParam("options"), ) => Deno.core.opAsync("read_dir", volumeId, path); const removeDir = ( - { - volumeId = requireParam("volumeId"), - path = requireParam("path"), - } = requireParam("options"), + { volumeId = requireParam("volumeId"), path = requireParam("path") } = + requireParam("options"), ) => Deno.core.opAsync("remove_dir", volumeId, path); const trace = (whatToTrace = requireParam("whatToTrace")) => Deno.core.opAsync("log_trace", whatToTrace); @@ -276,15 +259,10 @@ const getServiceConfig = async ( ); }; -const setPermissions = async ( - { - volumeId = requireParam("volumeId"), - path = requireParam("path"), - readonly = requireParam("readonly"), - } = requireParam("options"), -) => { - return await Deno.core.opAsync("set_permissions", volumeId, path, readonly); -}; +const started = () => Deno.core.opSync("set_started"); +const restart = () => Deno.core.opAsync("restart"); +const start = () => Deno.core.opAsync("start"); +const stop = () => Deno.core.opAsync("stop"); const currentFunction = Deno.core.opSync("current_function"); const input = Deno.core.opSync("get_input"); @@ -314,13 +292,19 @@ const effects = { runCommand, runDaemon, runRsync, - setPermissions, + chmod, signalGroup, sleep, trace, warn, writeFile, writeJsonFile, + restart, + start, + stop, +}; +const fnSpecificArgs = { + main: { started }, }; const defaults = { @@ -337,8 +321,10 @@ function safeToString(fn, orValue = "") { } } +const apiVersion = mainModule?.version || defaults?.version || 0; const runFunction = jsonPointerValue(mainModule, currentFunction) || jsonPointerValue(defaults, currentFunction); +const extraArgs = jsonPointerValue(fnSpecificArgs, currentFunction) || {}; (async () => { const answer = await (async () => { if (typeof runFunction !== "function") { @@ -346,7 +332,21 @@ const runFunction = jsonPointerValue(mainModule, currentFunction) || throw new Error(`Expecting ${currentFunction} to be a function`); } })() - .then(() => runFunction(effects, input, ...variable_args)) + .then(() => { + switch (apiVersion) { + case 0: + return runFunction(effects, input, ...variable_args); + case 1: + return runFunction({ + effects, + input, + args: variable_args, + ...extraArgs, + }); + default: + return { error: `Unknown API version ${apiVersion}` }; + } + }) .catch((e) => { if ("error" in e) return e; if ("error-code" in e) return e; diff --git a/libs/js_engine/src/lib.rs b/libs/js_engine/src/lib.rs index ad639ef61..974d249c9 100644 --- a/libs/js_engine/src/lib.rs +++ b/libs/js_engine/src/lib.rs @@ -317,12 +317,16 @@ impl JsExecutionEnvironment { fns::wait_command::decl(), fns::sleep::decl(), fns::send_signal::decl(), - fns::set_permissions::decl(), + fns::chmod::decl(), fns::signal_group::decl(), fns::rsync::decl(), fns::rsync_wait::decl(), fns::rsync_progress::decl(), fns::get_service_config::decl(), + fns::set_started::decl(), + fns::restart::decl(), + fns::start::decl(), + fns::stop::decl(), ] } @@ -624,6 +628,16 @@ mod fns { path_in: PathBuf, write: String, ) -> Result<(), AnyError> { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run writeFile in sandboxed mode"); + } + let (volumes, volume_path) = { let state = state.borrow(); let ctx: &JsContext = state.borrow(); @@ -677,6 +691,16 @@ mod fns { dst_volume: VolumeId, dst_path: PathBuf, ) -> Result<(), AnyError> { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run rename in sandboxed mode"); + } + let (volumes, volume_path, volume_path_out) = { let state = state.borrow(); let ctx: &JsContext = state.borrow(); @@ -734,6 +758,16 @@ mod fns { dst_path: PathBuf, options: RsyncOptions, ) -> Result { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run rsync in sandboxed mode"); + } + let (volumes, volume_path, volume_path_out, rsyncs) = { let state = state.borrow(); let ctx: &JsContext = state.borrow(); @@ -828,12 +862,22 @@ mod fns { Ok(progress) } #[op] - async fn set_permissions( + async fn chown( state: Rc>, volume_id: VolumeId, path_in: PathBuf, - readonly: bool, + ownership: u32, ) -> Result<(), AnyError> { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run chown in sandboxed mode"); + } + let (volumes, volume_path) = { let state = state.borrow(); let ctx: &JsContext = state.borrow(); @@ -855,9 +899,56 @@ mod fns { volume_path.to_string_lossy(), ); } - let mut perms = tokio::fs::metadata(&new_file).await?.permissions(); - perms.set_readonly(readonly); - tokio::fs::set_permissions(new_file, perms).await?; + let output = tokio::process::Command::new("chown") + .arg("--recursive") + .arg(format!("{ownership}")) + .arg(new_file.as_os_str()) + .output() + .await?; + if !output.status.success() { + return Err(anyhow!("Chown Error")); + } + Ok(()) + } + #[op] + async fn chmod( + state: Rc>, + volume_id: VolumeId, + path_in: PathBuf, + mode: u32, + ) -> Result<(), AnyError> { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run chmod in sandboxed mode"); + } + + let (volumes, volume_path) = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + let volume_path = ctx + .volumes + .path_for(&ctx.datadir, &ctx.package_id, &ctx.version, &volume_id) + .ok_or_else(|| anyhow!("There is no {} in volumes", volume_id))?; + (ctx.volumes.clone(), volume_path) + }; + if volumes.readonly(&volume_id) { + bail!("Volume {} is readonly", volume_id); + } + let new_file = volume_path.join(path_in); + // With the volume check + if !is_subset(&volume_path, &new_file).await? { + bail!( + "Path '{}' has broken away from parent '{}'", + new_file.to_string_lossy(), + volume_path.to_string_lossy(), + ); + } + tokio::fs::set_permissions(new_file, Permissions::from_mode(mode)).await?; Ok(()) } #[op] @@ -866,6 +957,16 @@ mod fns { volume_id: VolumeId, path_in: PathBuf, ) -> Result<(), AnyError> { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run removeFile in sandboxed mode"); + } + let (volumes, volume_path) = { let state = state.borrow(); let ctx: &JsContext = state.borrow(); @@ -897,6 +998,16 @@ mod fns { volume_id: VolumeId, path_in: PathBuf, ) -> Result<(), AnyError> { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run removeDir in sandboxed mode"); + } + let (volumes, volume_path) = { let state = state.borrow(); let ctx: &JsContext = state.borrow(); @@ -928,6 +1039,16 @@ mod fns { volume_id: VolumeId, path_in: PathBuf, ) -> Result<(), AnyError> { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run createDir in sandboxed mode"); + } + let (volumes, volume_path) = { let state = state.borrow(); let ctx: &JsContext = state.borrow(); @@ -1251,6 +1372,16 @@ mod fns { pid: u32, signal: u32, ) -> Result<(), AnyError> { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run sendSignal in sandboxed mode"); + } + if let Some(rpc_client) = { let state = state.borrow(); let ctx = state.borrow::(); @@ -1279,6 +1410,16 @@ mod fns { gid: u32, signal: u32, ) -> Result<(), AnyError> { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run signalGroup in sandboxed mode"); + } + if let Some(rpc_client) = { let state = state.borrow(); let ctx = state.borrow::(); @@ -1315,6 +1456,16 @@ mod fns { output: OutputStrategy, timeout: Option, ) -> Result { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run command in sandboxed mode"); + } + if let (gid, Some(rpc_client)) = { let state = state.borrow(); let ctx = state.borrow::(); @@ -1390,99 +1541,6 @@ mod fns { Ok(()) } - #[op] - async fn chown( - state: Rc>, - volume_id: VolumeId, - path_in: PathBuf, - ownership: u32, - ) -> Result<(), AnyError> { - let sandboxed = { - let state = state.borrow(); - let ctx: &JsContext = state.borrow(); - ctx.sandboxed - }; - - if sandboxed { - bail!("Will not run chown in sandboxed mode"); - } - - let (volumes, volume_path) = { - let state = state.borrow(); - let ctx: &JsContext = state.borrow(); - let volume_path = ctx - .volumes - .path_for(&ctx.datadir, &ctx.package_id, &ctx.version, &volume_id) - .ok_or_else(|| anyhow!("There is no {} in volumes", volume_id))?; - (ctx.volumes.clone(), volume_path) - }; - if volumes.readonly(&volume_id) { - bail!("Volume {} is readonly", volume_id); - } - let path_in = path_in.strip_prefix("/").unwrap_or(&path_in); - let new_file = volume_path.join(path_in); - // With the volume check - if !is_subset(&volume_path, &new_file).await? { - bail!( - "Path '{}' has broken away from parent '{}'", - new_file.to_string_lossy(), - volume_path.to_string_lossy(), - ); - } - let output = tokio::process::Command::new("chown") - .arg("--recursive") - .arg(format!("{ownership}")) - .arg(new_file.as_os_str()) - .output() - .await?; - if !output.status.success() { - return Err(anyhow!("Chown Error")); - } - Ok(()) - } - #[op] - async fn chmod( - state: Rc>, - volume_id: VolumeId, - path_in: PathBuf, - mode: u32, - ) -> Result<(), AnyError> { - let sandboxed = { - let state = state.borrow(); - let ctx: &JsContext = state.borrow(); - ctx.sandboxed - }; - - if sandboxed { - bail!("Will not run chmod in sandboxed mode"); - } - - let (volumes, volume_path) = { - let state = state.borrow(); - let ctx: &JsContext = state.borrow(); - let volume_path = ctx - .volumes - .path_for(&ctx.datadir, &ctx.package_id, &ctx.version, &volume_id) - .ok_or_else(|| anyhow!("There is no {} in volumes", volume_id))?; - (ctx.volumes.clone(), volume_path) - }; - if volumes.readonly(&volume_id) { - bail!("Volume {} is readonly", volume_id); - } - let path_in = path_in.strip_prefix("/").unwrap_or(&path_in); - let new_file = volume_path.join(path_in); - // With the volume check - if !is_subset(&volume_path, &new_file).await? { - bail!( - "Path '{}' has broken away from parent '{}'", - new_file.to_string_lossy(), - volume_path.to_string_lossy(), - ); - } - tokio::fs::set_permissions(new_file, Permissions::from_mode(mode)).await?; - Ok(()) - } - #[op] async fn get_service_config( state: Rc>, @@ -1510,6 +1568,16 @@ mod fns { internal_port: u16, address_schema: AddressSchemaOnion, ) -> Result { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run bindOnion in sandboxed mode"); + } + let os = { let state = state.borrow(); let ctx = state.borrow::(); @@ -1525,6 +1593,16 @@ mod fns { internal_port: u16, address_schema: AddressSchemaLocal, ) -> Result { + let sandboxed = { + let state = state.borrow(); + let ctx: &JsContext = state.borrow(); + ctx.sandboxed + }; + + if sandboxed { + bail!("Will not run bindLocal in sandboxed mode"); + } + let os = { let state = state.borrow(); let ctx = state.borrow::(); @@ -1536,12 +1614,12 @@ mod fns { } #[op] - fn set_started(state: &mut OpState) -> Result<(), AnyError> { + fn set_started(state: &mut OpState) { let os = { let ctx = state.borrow::(); ctx.os.clone() }; - os.set_started().map_err(|e| anyhow!("{e:?}")) + os.set_started() } #[op] @@ -1561,7 +1639,9 @@ mod fns { let ctx = state.borrow::(); ctx.os.clone() }; - os.restart().await.map_err(|e| anyhow!("{e:?}")) + os.restart().await; + + Ok(()) } #[op] @@ -1581,7 +1661,9 @@ mod fns { let ctx = state.borrow::(); ctx.os.clone() }; - os.start().await.map_err(|e| anyhow!("{e:?}")) + os.start().await; + + Ok(()) } #[op] @@ -1601,7 +1683,9 @@ mod fns { let ctx = state.borrow::(); ctx.os.clone() }; - os.stop().await.map_err(|e| anyhow!("{e:?}")) + os.stop().await; + + Ok(()) } /// We need to make sure that during the file accessing, we don't reach beyond our scope of control