Skip to content

fix: Sandboxing Scripts in Shared Contexts #357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions crates/bevy_mod_scripting_core/src/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use bevy::{
reflect::TypePath,
utils::HashMap,
};
use mlua::Table;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙅 This is introducing lua logic into the bms_core layer. The idea is to only keep language agnostic logic in here

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The crate does have mlua and rhai dependencies behind feature flags, but those are only there to implemenet Error conversions

use std::borrow::Cow;

/// Represents a scripting language. Languages which compile into another language should use the target language as their language.
Expand Down Expand Up @@ -169,6 +170,13 @@ impl Default for AssetPathToLanguageMapper {
}
}

/// stores environments for each script
#[derive(Default, Debug, Resource)]
pub struct ScriptEnvironmentStore {
/// The map of Script id's to their env
pub map: HashMap<ScriptId, Table>,
}

/// A cache of asset id's to their script id's. Necessary since when we drop an asset we won't have the ability to get the path from the asset.
#[derive(Default, Debug, Resource)]
pub struct ScriptMetadataStore {
Expand Down Expand Up @@ -349,6 +357,7 @@ pub(crate) fn configure_asset_systems(app: &mut App) -> &mut App {
.before(ScriptingSystemSet::ScriptMetadataRemoval),
),
)
.init_resource::<ScriptEnvironmentStore>()
.init_resource::<ScriptMetadataStore>()
.init_resource::<ScriptAssetSettings>()
.add_event::<ScriptAssetEvent>();
Expand Down
32 changes: 30 additions & 2 deletions crates/languages/bevy_mod_scripting_lua/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use bevy::{
ecs::{entity::Entity, world::World},
};
use bevy_mod_scripting_core::{
asset::{AssetPathToLanguageMapper, Language},
asset::{AssetPathToLanguageMapper, Language, ScriptEnvironmentStore},
bindings::{
function::namespace::Namespace, globals::AppScriptGlobalsRegistry,
script_value::ScriptValue, ThreadWorldContainer, WorldContainer,
Expand Down Expand Up @@ -172,11 +172,25 @@ fn load_lua_content_into_context(
.iter()
.try_for_each(|init| init(script_id, Entity::from_raw(0), context))?;

// isolate the script's globals into an environment
let metatable = context.create_table()?;
metatable.set("__index", context.globals())?;
let env = context.create_table()?;
env.set_metatable(Some(metatable));

context
.load(content)
.set_environment(env.clone())
.exec()
.map_err(ScriptError::from_mlua_error)?;

// store the env in the store so we can call BMS event handlers from them
let world = ThreadWorldContainer.try_get_world()?;
world.with_global_access(move |w| {
let map = &mut w.resource_mut::<ScriptEnvironmentStore>().map;
map.insert(script_id.clone(), env.clone());
})?;

Ok(())
}

Expand Down Expand Up @@ -240,7 +254,21 @@ pub fn lua_handler(
.iter()
.try_for_each(|init| init(script_id, entity, context))?;

let handler: Function = match context.globals().raw_get(callback_label.as_ref()) {
// we need the world to access the ScriptEnvironmentStore
// we will find the environment that belongs to this script to call the function
let world = ThreadWorldContainer.try_get_world()?;
let env = world.with_global_access(|w| {
w.resource::<ScriptEnvironmentStore>()
.map
.get(script_id)
.ok_or(ScriptError::new(format!(
"Could not find environment for script with ScriptId: {}",
script_id
)))
.cloned()
})?;

let handler: Function = match env?.raw_get(callback_label.as_ref()) {
Ok(handler) => handler,
// not subscribed to this event type
Err(_) => {
Expand Down
Loading