Skip to content

Conversation

@cbouy
Copy link

@cbouy cbouy commented Dec 18, 2025

First of, thanks for making this library 🙌

This PR fixes issue #1, and implements one of the suggestions from issue #4 (extracting the widget's state from traits) to simplify the code a tiny bit. While making this change I've formatted the file with ruff, happy to revert that and only keep the code change if you prefer.
I've tested it on one of my widget that specifies the _esm as a path and it worked beautifully!
I'm not sure if the same url blob pattern could be used for any of the other isModuleFormat and isClassFormat cases, it could make things a lot easier but I didn't have an example to play with so I haven't tried.
Edit: yup that works, I think it makes things a lot less error prone.
Also added an example in the relevant file.

@entelligence-ai-pr-reviews
Copy link

Entelligence AI Vulnerability Scanner

Status: No security vulnerabilities found

Your code passed our comprehensive security analysis.

Analyzed 2 files in total

@entelligence-ai-pr-reviews
Copy link

Review Summary

🏷️ Draft Comments (8)

Skipped posting 8 draft comments that were valid but scored below your review threshold (>=13/15). Feel free to update them here.

streamlit_anywidget/__init__.py (3)

71-71: widget_instance.get_state(key=traits) assumes the widget has a get_state method with this signature, but this is not a standard ipywidgets API and will raise AttributeError or return incorrect data for most widgets.

📊 Impact Scores:

  • Production Impact: 4/5
  • Fix Specificity: 5/5
  • Urgency Impact: 3/5
  • Total Score: 12/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In streamlit_anywidget/__init__.py, line 71, the code uses `widget_instance.get_state(key=traits)`, but most widget classes do not have a `get_state` method with this signature, which will cause an AttributeError or incorrect data extraction. Replace this line with a dictionary comprehension that collects the values for each trait in `traits`, e.g., `widget_data = {name: getattr(widget_instance, name) for name in traits}`.

85-91: The variable key is redefined in the for-loop (for key, value in component_value.items()), shadowing the function argument key and potentially causing incorrect widget updates or key mismatches.

📊 Impact Scores:

  • Production Impact: 3/5
  • Fix Specificity: 5/5
  • Urgency Impact: 3/5
  • Total Score: 11/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In streamlit_anywidget/__init__.py, lines 85-91, the for-loop uses `for key, value in component_value.items()`, which redefines the `key` argument of the function and can cause bugs. Change the loop variable from `key` to `attr_name` throughout this block to avoid shadowing and ensure correct attribute updates.

23-96: anywidget function is overly complex (12 > 10), making it hard to maintain and extend as the codebase grows.

📊 Impact Scores:

  • Production Impact: 2/5
  • Fix Specificity: 3/5
  • Urgency Impact: 2/5
  • Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

Refactor the `anywidget` function in streamlit_anywidget/__init__.py (lines 23-96) to reduce its cyclomatic complexity. Break out major logical sections (such as ESM/CSS extraction, widget state extraction, and frontend communication) into well-named helper functions. This will make the code easier to maintain and extend as the codebase grows.

streamlit_anywidget/frontend/src/StreamlitAnyWidget.tsx (5)

29-236: useEffect for loading the widget is missing widgetState and widget_class in its dependency array, causing stale closures and incorrect widget initialization when these values change.

📊 Impact Scores:

  • Production Impact: 3/5
  • Fix Specificity: 2/5
  • Urgency Impact: 2/5
  • Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In streamlit_anywidget/frontend/src/StreamlitAnyWidget.tsx, lines 29-236, the useEffect hook responsible for loading the widget is missing 'widgetState' and 'widget_class' in its dependency array. This can cause stale closures and incorrect widget initialization if these values change. Update the dependency array to include 'widgetState' and 'widget_class'.

94-96,175-176: new Function and dynamic code evaluation (lines 94, 175) are used to transform and execute widget code, which can cause major performance and security issues at scale due to lack of optimization, JIT deoptimization, and inability to statically analyze or cache code.

📊 Impact Scores:

  • Production Impact: 4/5
  • Fix Specificity: 3/5
  • Urgency Impact: 3/5
  • Total Score: 10/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In streamlit_anywidget/frontend/src/StreamlitAnyWidget.tsx, lines 94-96 and 175-176, the code uses `new Function` to dynamically evaluate widget code. This approach prevents JavaScript engines from optimizing the code, increases memory and CPU usage, and blocks static analysis and caching, especially as the number of widgets or reloads grows. Refactor to use safer, more performant dynamic import patterns (e.g., using Blob URLs and `import()` as done for bundled ESM), or pre-compile widget code where possible. Remove all uses of `new Function` for widget code execution.

44-51: The code creates a new <style> element for each widget class without cleanup, causing potential memory leaks and excessive DOM nodes if many widgets are loaded/unloaded dynamically.

📊 Impact Scores:

  • Production Impact: 3/5
  • Fix Specificity: 3/5
  • Urgency Impact: 2/5
  • Total Score: 8/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In streamlit_anywidget/frontend/src/StreamlitAnyWidget.tsx, lines 44-51, a new <style> element is created for each widget class if it doesn't already exist, but there is no cleanup when widgets are removed or updated. This can cause memory leaks and DOM bloat in long-running or dynamic widget scenarios. Refactor to remove the <style> element in the effect cleanup function when the widget is unmounted or the class changes.

213-215: Repeated use of setTimeout to update frame height after widget load (line 213-215) can cause unnecessary reflows and layout thrashing if widgets are loaded frequently or in rapid succession.

📊 Impact Scores:

  • Production Impact: 2/5
  • Fix Specificity: 2/5
  • Urgency Impact: 2/5
  • Total Score: 6/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In streamlit_anywidget/frontend/src/StreamlitAnyWidget.tsx, lines 213-215, `setTimeout` is used to update the Streamlit frame height after widget load. If widgets are loaded or updated frequently, this can cause layout thrashing and unnecessary reflows. Refactor to debounce or batch frame height updates, or use a more reliable event-driven approach (e.g., ResizeObserver) to minimize layout recalculations.

175-176: new Function is used to evaluate potentially untrusted widget module code from esm_content, enabling arbitrary code execution in the browser if the input is attacker-controlled.

📊 Impact Scores:

  • Production Impact: 5/5
  • Fix Specificity: 2/5
  • Urgency Impact: 5/5
  • Total Score: 12/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In streamlit_anywidget/frontend/src/StreamlitAnyWidget.tsx, lines 175-176, the code uses `new Function` to evaluate widget module code from `esm_content`, which is a critical security vulnerability (arbitrary code execution). Replace this with a secure, sandboxed loader or a safe import mechanism that does not use `new Function` or `eval`. If this cannot be safely implemented, throw an error to prevent execution.

🔍 Comments beyond diff scope (1)
streamlit_anywidget/frontend/src/StreamlitAnyWidget.tsx (1)

94-96: new Function is used to dynamically evaluate untrusted esm_content, allowing attackers to execute arbitrary JavaScript code in the browser context if they control widget source.
Category: security


@cbouy
Copy link
Author

cbouy commented Jan 10, 2026

@mdrazak2001 This is ready for comments and review when you have time!

@mdrazak2001
Copy link
Owner

hey @cbouy,

sorry for the delay, i will try to have a look at this change in the coming weekend.

Thanks for the commit btw!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants