-
Notifications
You must be signed in to change notification settings - Fork 944
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
Feat: Added command center #2674
base: main
Are you sure you want to change the base?
Changes from all commits
f0da710
cba8279
7c105e3
b6a6304
df7d656
0d84846
a055d94
f8b89f0
775b835
023255f
7a8fe31
f1f0661
8645afb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,8 +23,13 @@ | |
|
||
from __future__ import annotations | ||
|
||
import ast | ||
import asyncio | ||
import builtins | ||
import contextlib | ||
import inspect | ||
import io | ||
import re | ||
from collections.abc import Callable | ||
from typing import TYPE_CHECKING, Literal | ||
|
||
|
@@ -57,6 +62,7 @@ def SolaraViz( | |
simulator: Simulator | None = None, | ||
model_params=None, | ||
name: str | None = None, | ||
additional_imports: dict | None = None, | ||
): | ||
"""Solara visualization component. | ||
|
||
|
@@ -80,6 +86,7 @@ def SolaraViz( | |
model_params (dict, optional): Parameters for (re-)instantiating a model. | ||
Can include user-adjustable parameters and fixed parameters. Defaults to None. | ||
name (str | None, optional): Name of the visualization. Defaults to the models class name. | ||
additional_imports (dict, optional): Dictionary of names to either import strings or objects | ||
|
||
Returns: | ||
solara.component: A Solara component that renders the visualization interface for the model. | ||
|
@@ -157,6 +164,8 @@ def SolaraViz( | |
) | ||
with solara.Card("Information"): | ||
ShowSteps(model.value) | ||
with solara.Card("Command Center"): | ||
CommandCenter(model.value, additional_imports) | ||
|
||
ComponentsView(components, model.value) | ||
|
||
|
@@ -586,3 +595,156 @@ def ShowSteps(model): | |
"""Display the current step of the model.""" | ||
update_counter.get() | ||
return solara.Text(f"Step: {model.steps}") | ||
|
||
|
||
class _SecureCodeValidator(ast.NodeVisitor): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what did you use as inspiration, if any for this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't find any source exactly resembling this, but after search a bit I found some open discussions on stackoverflow suggesting the use of |
||
"""Validates Python code for potentially dangerous operations.""" | ||
|
||
FORBIDDEN_BUILTINS = { | ||
"eval", | ||
"exec", | ||
"compile", # Code execution | ||
"open", # File system access | ||
"globals", | ||
"locals", # Global state manipulation | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. most of these make sense to me is the model itself by default available in the namespace? also, we can refine this based on experiences There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I don't think I exactly understand what you mean here. Can you add a little more context. |
||
|
||
FORBIDDEN_ATTRIBUTES = {"__code__", "__globals__", "__builtins__"} | ||
|
||
def __init__(self): | ||
self.errors = [] | ||
|
||
def visit_call(self, node): | ||
# Check function calls | ||
if isinstance(node.func, ast.Name): | ||
if node.func.id in self.FORBIDDEN_BUILTINS: | ||
self.errors.append( | ||
f"Use of forbidden built-in '{node.func.id}()' is not allowed for security reasons" | ||
) | ||
elif ( | ||
isinstance(node.func, ast.Attribute) | ||
and node.func.attr in self.FORBIDDEN_ATTRIBUTES | ||
): | ||
self.errors.append( | ||
f"Access to special attribute '{node.func.attr}' is not allowed for security reasons" | ||
) | ||
self.generic_visit(node) | ||
|
||
def visit_attribute(self, node): | ||
attrs = [] | ||
curr = node | ||
while isinstance(curr, ast.Attribute): | ||
attrs.append(curr.attr) | ||
curr = curr.value | ||
|
||
dangerous_patterns = [ | ||
r"(subprocess|shutil)", # System manipulation modules | ||
] | ||
|
||
for attr in attrs: | ||
for pattern in dangerous_patterns: | ||
if re.match(pattern, attr): | ||
self.errors.append( | ||
f"Access to system module '{attr}' is not allowed" | ||
) | ||
|
||
self.generic_visit(node) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is the overhead of this security validation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ran some tests, and most of the time, the overhead from the security validator is more than double the execution time of the code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that is a lot, it might be an argument to leave out validation at all and just give a very clear warning in the docs about security. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it's a lot, I didn't think it to be so performance heavy. But since it could be used for smaller models, I think a warning in the docs should do the job (personal view). |
||
|
||
|
||
@solara.component | ||
def CommandCenter(model, additional_imports=None): | ||
"""Interactive command center for executing Python code against the model. | ||
|
||
Args: | ||
model: The model instance to execute code against | ||
additional_imports (dict, optional): Dictionary of names to objects | ||
e.g. {'np': numpy, 'pd': pandas} | ||
""" | ||
code = solara.use_reactive("") | ||
output = solara.use_reactive("") | ||
error = solara.use_reactive("") | ||
|
||
def create_safe_globals( | ||
model: object, | ||
additional_imports: dict[str, str | object] | None = None, | ||
) -> dict: | ||
"""Creates a restricted globals dictionary for code execution.""" | ||
safe_globals = {"__builtins__": builtins.__dict__.copy()} | ||
|
||
# Remove forbidden builtins using the same set from _SecureCodeValidator | ||
for name in _SecureCodeValidator.FORBIDDEN_BUILTINS: | ||
safe_globals["__builtins__"].pop(name, None) | ||
|
||
safe_globals["model"] = model | ||
|
||
# Add additional imports if provided | ||
if additional_imports: | ||
for name, value in additional_imports.items(): | ||
safe_globals[name] = value | ||
|
||
return safe_globals | ||
|
||
solara.Style(""" | ||
.v-text-field__slot textarea { | ||
font-family: monospace !important; | ||
} | ||
""") | ||
|
||
def handle_code_change(new_value): | ||
code.set(new_value) | ||
error.set("") | ||
|
||
def handle_run(): | ||
output.set("") | ||
error.set("") | ||
|
||
try: | ||
tree = ast.parse(code.value) | ||
validator = _SecureCodeValidator() | ||
validator.visit(tree) | ||
|
||
if validator.errors: | ||
error.set("Security violations found:\n" + "\n".join(validator.errors)) | ||
return | ||
|
||
except SyntaxError as e: | ||
error.set(f"Syntax Error: {e!s}") | ||
return | ||
|
||
stdout = io.StringIO() | ||
try: | ||
exec_env = create_safe_globals(model, additional_imports) | ||
|
||
with contextlib.redirect_stdout(stdout): | ||
exec(code.value, exec_env, exec_env) # noqa: S102 | ||
|
||
# Force update to display any changes to the model | ||
from mesa.visualization.utils import force_update | ||
|
||
force_update() | ||
|
||
output.set(stdout.getvalue()) | ||
|
||
except Exception as e: | ||
error.set(f"Runtime Error: {e!s}") | ||
|
||
with solara.Column(): | ||
if error.value: | ||
solara.Error(error.value) | ||
|
||
solara.InputTextArea( | ||
label="Enter Python code:", | ||
value=code.value, | ||
on_value=handle_code_change, | ||
continuous_update=True, | ||
rows=6, | ||
) | ||
|
||
with solara.Row(justify="end"): | ||
solara.Button("Run Code", on_click=handle_run, color="primary") | ||
|
||
solara.Markdown("### Output") | ||
if output.value: | ||
solara.Markdown(f"```python\n{output.value}\n```") | ||
else: | ||
solara.Text("Output will appear here", style={"color": "#666"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want the command center to be allways there, or make it optional in some way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally would prefer it to be always be there, it will encourage the users to play around with it and know it better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a design point of view, I prefer giving users the choice but show it in all our examples.
Someone might want to built a UI with very limited possibility for interaction, or only show the animation to a client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, does a boolean parameter in
SolaraViz
work?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking more along the lines of treating it as a component so you could just at it to the list of components:
But I am open to having it as a boolean as well. I haven't thought about the layout anyway in great detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that works as well, in fact that is more intuitive. I like this idea more than that of a boolean.