Conversation
debajyoti0606
requested review from
devesh-juspay,
junaid-shirur,
kalpadhwaryu,
shivamashtikar and
zereraz
as code owners
WalkthroughThis PR introduces comprehensive script execution support to the workflow builder. Changes span Docker/infrastructure setup for Python, JavaScript, and R runtimes with sandboxing; new React components for code editing and script configuration; API endpoints for script language discovery; and server-side script execution with security validation and output formatting. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WorkflowBuilder as WorkflowBuilder UI
participant ScriptSidebar
participant CodeEditor
participant API as Backend API
participant Executor as Script Executor
User->>WorkflowBuilder: Click "Run Script" action
WorkflowBuilder->>API: GET /workflow/script/languages
API-->>WorkflowBuilder: Available languages + config
WorkflowBuilder->>ScriptSidebar: Open (creation mode)
User->>ScriptSidebar: Select language
ScriptSidebar->>CodeEditor: Open editor modal
User->>CodeEditor: Write/edit code
User->>CodeEditor: Click Save
CodeEditor-->>ScriptSidebar: Closed with code
User->>ScriptSidebar: Click Save configuration
ScriptSidebar->>API: Create/Update script tool
API->>Executor: Execute script on workflow run
Executor->>Executor: Validate security rules
Executor->>Executor: Sandbox with Bubblewrap (optional)
Executor->>Executor: Capture output/files
Executor-->>API: Structured execution result
API-->>WorkflowBuilder: Display result in execution view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @debajyoti0606, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the application's capabilities by enabling users to integrate and execute custom scripts written in Python, JavaScript, and R directly within their workflows. It provides a robust and secure environment for running these scripts, complete with a dedicated code editor and seamless integration into the workflow builder. These changes empower users with advanced automation and data processing functionalities, making the platform more versatile and extensible. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a powerful script execution feature, adding support for Python, JavaScript, and R within workflows. The implementation includes a new frontend UI with a Monaco-based code editor and a sandboxed execution environment on the backend using Bubblewrap. The changes are extensive and well-structured.
My review focuses on several key areas:
- Security: The use of
privileged: truecontainers andsetuidonbwrapintroduces significant security risks that need to be addressed. - Docker Best Practices: There are opportunities to optimize the Dockerfile for smaller image size and better build performance.
- Frontend Robustness: Some improvements can be made to the new
CodeEditorcomponent to make it more robust. - Code Duplication: There is some duplicated code in the frontend that can be refactored.
Overall, this is a great addition, but the security implications require careful consideration.
| # Copy and setup startup script | ||
| COPY --chown=bun:bun start.sh /usr/src/app/start.sh | ||
| RUN chmod +x /usr/src/app/start.sh | ||
| RUN chmod u+s $(which bwrap) |
There was a problem hiding this comment.
Setting the setuid bit on bwrap is a significant security decision. When combined with the privileged: true container setting, this could create potential privilege escalation vectors if any vulnerabilities exist in bwrap. This approach should be re-evaluated. Consider using more granular capabilities with cap_add in your Docker Compose file instead of running a privileged container, which may mitigate the need for this or at least reduce the associated risk.
| container_name: xyne-app | ||
| user: "${DOCKER_UID}:${DOCKER_GID}" | ||
| # Ports removed - NGINX handles external access | ||
| privileged: true |
There was a problem hiding this comment.
Running a container with privileged: true disables all security mechanisms and gives it full access to the host system, which is a critical security risk. This is likely to support bubblewrap sandboxing. To follow the principle of least privilege, you should grant only the necessary capabilities instead. For bubblewrap, this is typically SYS_ADMIN.
cap_add:
- SYS_ADMIN| RUN bun run build | ||
|
|
||
| # Set the environment as production | ||
| ENV NODE_ENV=production |
There was a problem hiding this comment.
The ENV NODE_ENV=production is set after the frontend build (RUN bun run build). The frontend build process relies on NODE_ENV=production to create an optimized production build. Without it, the resulting frontend assets will be larger and less performant. This should be set before the build command is run.
| RUN python3 -m pip install --break-system-packages -r /usr/src/app/requirements.txt | ||
| RUN npm install -g $(cat /usr/src/app/js-requirements.json | jq -r '.dependencies | keys[]') | ||
| RUN Rscript /usr/src/app/requirements.R |
There was a problem hiding this comment.
Each RUN command in a Dockerfile creates a new layer in the image. To optimize the image size and build time, it's a best practice to chain related commands into a single RUN instruction. These three package installation commands can be combined.
RUN python3 -m pip install --break-system-packages -r /usr/src/app/requirements.txt && \
npm install -g $(cat /usr/src/app/js-requirements.json | jq -r '.dependencies | keys[]') && \
Rscript /usr/src/app/requirements.R
| useEffect(() => { | ||
| if (isOpen) { | ||
| // Set loading to false immediately - let Monaco handle its own loading | ||
| const timer = setTimeout(() => { | ||
| setIsMonacoLoading(false) | ||
| }, 1000) | ||
|
|
||
| return () => clearTimeout(timer) | ||
| } | ||
| }, [isOpen]) |
There was a problem hiding this comment.
This useEffect hook uses a setTimeout to manage the loading state of the Monaco editor. This is a fragile approach, as the editor's actual load time can vary. The component already uses the onMount prop, which is the correct and more robust way to handle this. This effect is redundant and could lead to UI glitches if the editor loads faster or slower than the 1-second timeout.
| // Special rendering for python_script tools | ||
| const hasPythonScriptTool = | ||
| tools && tools.length > 0 && tools[0].type === "python_script" | ||
| if (step.type === "python_script" || hasPythonScriptTool) { | ||
| // Check if any associated tool execution has failed | ||
| const hasFailedToolExecution = | ||
| tools && tools.some((tool) => (tool as any).status === "failed") | ||
| const isFailed = step.status === "failed" || hasFailedToolExecution | ||
|
|
||
| return ( | ||
| <> | ||
| <div | ||
| className="relative cursor-pointer hover:shadow-lg transition-shadow" | ||
| style={{ | ||
| width: "320px", | ||
| minHeight: "122px", | ||
| borderRadius: "12px", | ||
| border: selected | ||
| ? isFailed | ||
| ? "2px solid #DC2626" | ||
| : isCompleted | ||
| ? "2px solid #059669" | ||
| : "2px solid #111827" | ||
| : isFailed | ||
| ? "2px solid #F87171" | ||
| : isCompleted | ||
| ? "2px solid #34D399" | ||
| : "2px solid #6B7280", | ||
| background: isFailed ? "#FEF2F2" : isCompleted ? "#F0FDF4" : "#FFF", | ||
| boxShadow: isFailed | ||
| ? "0 0 0 2px #FECACA" | ||
| : isCompleted | ||
| ? "0 0 0 2px #BBF7D0" | ||
| : "0 0 0 2px #E2E2E2", | ||
| }} | ||
| > | ||
| {/* Header with icon and title */} | ||
| <div className="flex items-center gap-3 text-left w-full px-4 pt-4 mb-3"> | ||
| {/* Bot icon with background */} | ||
| <div | ||
| className="flex justify-center items-center flex-shrink-0" | ||
| style={{ | ||
| display: "flex", | ||
| width: "24px", | ||
| height: "24px", | ||
| padding: "4px", | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| borderRadius: "4.8px", | ||
| background: "#EBF4FF", | ||
| }} | ||
| > | ||
| <Code width={16} height={16} color="#10B981" /> | ||
| </div> | ||
|
|
||
| <h3 | ||
| className="text-gray-800 truncate flex-1" | ||
| style={{ | ||
| fontFamily: "Inter", | ||
| fontSize: "14px", | ||
| fontStyle: "normal", | ||
| fontWeight: "600", | ||
| lineHeight: "normal", | ||
| letterSpacing: "-0.14px", | ||
| color: "#3B4145", | ||
| }} | ||
| > | ||
| {step.name || "Python Script"} | ||
| {/* Show execution status indicator */} | ||
| {(step as any).isExecution && isActive && ( | ||
| <span className="ml-2 text-xs bg-blue-100 text-blue-800 px-2 py-1 rounded-full"> | ||
| Running | ||
| </span> | ||
| )} | ||
| {(step as any).isExecution && | ||
| isFailed && | ||
| step.status !== "failed" && ( | ||
| <span className="ml-2 text-xs bg-red-100 text-red-800 px-2 py-1 rounded-full"> | ||
| Tool Failed | ||
| </span> | ||
| )} | ||
| </h3> | ||
| </div> | ||
|
|
||
| {/* Full-width horizontal divider */} | ||
| <div className="w-full h-px bg-gray-200 mb-3"></div> | ||
|
|
||
| {/* Description text */} | ||
| <div className="px-4 pb-4"> | ||
| <p className="text-gray-600 text-sm leading-relaxed text-left break-words overflow-hidden"> | ||
| {step.description || | ||
| "Execute Python script to process data and generate results."} | ||
| </p> | ||
| </div> | ||
|
|
||
| {/* ReactFlow Handles - invisible but functional */} | ||
| <Handle | ||
| type="target" | ||
| position={Position.Top} | ||
| id="top" | ||
| isConnectable={isConnectable} | ||
| className="opacity-0" | ||
| /> | ||
|
|
||
| <Handle | ||
| type="source" | ||
| position={Position.Bottom} | ||
| id="bottom" | ||
| isConnectable={isConnectable} | ||
| className="opacity-0" | ||
| /> | ||
|
|
||
| {/* Bottom center connection point - visual only */} | ||
| <div className="absolute -bottom-1.5 left-1/2 transform -translate-x-1/2"> | ||
| <div className="w-3 h-3 bg-gray-400 rounded-full border-2 border-white shadow-sm"></div> | ||
| </div> | ||
|
|
||
| </div> | ||
| </> | ||
| ) | ||
| } | ||
|
|
||
| // Special rendering for script nodes and steps with script tools | ||
| const hasScriptTool = tools && tools.length > 0 && tools[0].type === "script" | ||
| if (step.type === "script" || hasScriptTool) { | ||
| // Check if any associated tool execution has failed | ||
| const hasFailedToolExecution = | ||
| tools && tools.some((tool) => (tool as any).status === "failed") | ||
| const isFailed = step.status === "failed" || hasFailedToolExecution | ||
|
|
||
| // Extract title and description from script tool data | ||
| const scriptTool = hasScriptTool ? tools[0] : null | ||
| const scriptData = (scriptTool as any)?.result || (scriptTool as any)?.config || {} | ||
| const language = scriptData?.language || (scriptTool as any)?.toolType || "script" | ||
| const scriptTitle = `${language.charAt(0).toUpperCase() + language.slice(1)} Script` | ||
| const scriptDescription = step.description || `Execute ${language} script with custom code` | ||
|
|
||
| return ( | ||
| <> | ||
| <div | ||
| className="relative cursor-pointer hover:shadow-lg transition-shadow" | ||
| style={{ | ||
| width: "320px", | ||
| minHeight: "122px", | ||
| borderRadius: "12px", | ||
| border: selected | ||
| ? isFailed | ||
| ? "2px solid #DC2626" | ||
| : isCompleted | ||
| ? "2px solid #059669" | ||
| : "2px solid #111827" | ||
| : isFailed | ||
| ? "2px solid #F87171" | ||
| : isCompleted | ||
| ? "2px solid #34D399" | ||
| : "2px solid #6B7280", | ||
| background: isFailed ? "#FEF2F2" : isCompleted ? "#F0FDF4" : "#FFF", | ||
| boxShadow: isFailed | ||
| ? "0 0 0 2px #FECACA" | ||
| : isCompleted | ||
| ? "0 0 0 2px #BBF7D0" | ||
| : "0 0 0 2px #E2E2E2", | ||
| }} | ||
| > | ||
| {/* Header with icon and title */} | ||
| <div className="flex items-center gap-3 text-left w-full px-4 pt-4 mb-3"> | ||
| {/* Green code icon with background */} | ||
| <div | ||
| className="flex justify-center items-center flex-shrink-0" | ||
| style={{ | ||
| display: "flex", | ||
| width: "24px", | ||
| height: "24px", | ||
| padding: "4px", | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| borderRadius: "4.8px", | ||
| background: "#F0FDF4", | ||
| }} | ||
| > | ||
| <Code width={16} height={16} color="#10B981" /> | ||
| </div> | ||
|
|
||
| <h3 | ||
| className="text-gray-800 truncate flex-1" | ||
| style={{ | ||
| fontFamily: "Inter", | ||
| fontSize: "14px", | ||
| fontStyle: "normal", | ||
| fontWeight: "600", | ||
| lineHeight: "normal", | ||
| letterSpacing: "-0.14px", | ||
| color: "#3B4145", | ||
| }} | ||
| > | ||
| {step.name || scriptTitle} | ||
| {/* Show execution status indicator */} | ||
| {(step as any).isExecution && isActive && ( | ||
| <span className="ml-2 text-xs bg-blue-100 text-blue-800 px-2 py-1 rounded-full"> | ||
| Running | ||
| </span> | ||
| )} | ||
| {(step as any).isExecution && | ||
| isFailed && | ||
| step.status !== "failed" && ( | ||
| <span className="ml-2 text-xs bg-red-100 text-red-800 px-2 py-1 rounded-full"> | ||
| Tool Failed | ||
| </span> | ||
| )} | ||
| </h3> | ||
| </div> | ||
|
|
||
| {/* Full-width horizontal divider */} | ||
| <div className="w-full h-px bg-gray-200 mb-3"></div> | ||
|
|
||
| {/* Description text */} | ||
| <div className="px-4 pb-4"> | ||
| <p className="text-gray-600 text-sm leading-relaxed text-left break-words overflow-hidden"> | ||
| {scriptDescription} | ||
| </p> | ||
| </div> | ||
|
|
||
| {/* ReactFlow Handles - invisible but functional */} | ||
| <Handle | ||
| type="target" | ||
| position={Position.Top} | ||
| id="top" | ||
| isConnectable={isConnectable} | ||
| className="opacity-0" | ||
| /> | ||
|
|
||
| <Handle | ||
| type="source" | ||
| position={Position.Bottom} | ||
| id="bottom" | ||
| isConnectable={isConnectable} | ||
| className="opacity-0" | ||
| /> | ||
|
|
||
| {/* Bottom center connection point - visual only */} | ||
| <div className="absolute -bottom-1.5 left-1/2 transform -translate-x-1/2"> | ||
| <div className="w-3 h-3 bg-gray-400 rounded-full border-2 border-white shadow-sm"></div> | ||
| </div> | ||
|
|
||
| </div> | ||
| </> | ||
| ) | ||
| } |
There was a problem hiding this comment.
The rendering logic for python_script nodes (lines 753-872) and script nodes (lines 874-999) is nearly identical. This duplication makes the code harder to maintain, as any future UI changes would need to be applied in both places. This should be refactored into a single, reusable component that accepts language-specific details as props.
| ], | ||
| restrictedKeywords: [ | ||
| "import os","from os", "import sys", "import subprocess", "import socket", | ||
| "from sys", "from subprocess", "exec", "eval", "file(","dir(", |
There was a problem hiding this comment.
The list of restricted keywords for Python is missing open(). While the bubblewrap sandbox provides a layer of protection, it's a security best practice to implement defense in depth. Explicitly blocking file-opening functions at the script validation level adds an important security layer.
| "from sys", "from subprocess", "exec", "eval", "file(","dir(", | |
| "from sys", "from subprocess", "exec", "eval", "file(", "open(","dir(", |
| for (const keyword of config.restrictedKeywords) { | ||
| if (script.includes(keyword)) { | ||
| violations.push(`Restricted keyword detected: ${keyword}`) | ||
| } | ||
| } |
There was a problem hiding this comment.
The security validation starts with a weak keyword check using script.includes(), which can be easily bypassed (e.g., using string concatenation). The more robust, regex-based validation for imports that follows is a much better approach. This initial loop is redundant and should be removed to rely solely on the stronger validation logic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (17)
requirements.R (1)
1-10: Consider pinning R package versions for reproducible builds.The script installs packages without version pinning, meaning image contents can drift over time as CRAN updates. Current stable versions are jsonlite 2.0.0, httr 1.4.7, and curl 7.0.0.
For stricter reproducibility, you can pin these versions using
remotes::install_version():remotes::install_version(c("jsonlite=2.0.0", "httr=1.4.7", "curl=7.0.0"), dependencies=TRUE)Alternatively, wrap each package in a conditional check to avoid reinstalling on rebuild:
if (!requireNamespace("pkg", quietly = TRUE)) install.packages("pkg")frontend/package.json (1)
28-28: Align@monaco-editor/reactversion with React 19 support and configure Vite workers.You're on React 19.1.0 with
@monaco-editor/react^4.6.0. React 19 support was explicitly added in the 4.7.0 release, so bump to ^4.7.0. Yourmonaco-editor^0.52.2 is already compatible.Additionally, for Vite, the recommended pattern includes importing Monaco workers and configuring MonacoEnvironment, then calling loader.config({ monaco }). Your
frontend/src/components/CodeEditor.tsxcurrently uses a simple initialization without this setup. While the editor may work, add the worker configuration to align with best practices:- "@monaco-editor/react": "^4.6.0", + "@monaco-editor/react": "^4.7.0",In
CodeEditor.tsx, replace the basic initialization with the Vite worker pattern: importmonaco-editorworkers with the?workersyntax (e.g.,editor.worker?worker), configureself.MonacoEnvironment, and callloader.config({ monaco })before rendering the Editor component.Also applies to: 61-61
frontend/src/components/workflow/WhatHappensNextUI.tsx (1)
121-121: Refactor the long conditional for maintainability.The condition is checking six action IDs individually, making it hard to maintain. Consider refactoring to check against the availableActions array directly.
Apply this approach to simplify maintenance:
- if (action.id === "ai_agent" || action.id === "qna_agent" || action.id === "email" || action.id === "run_script" || action.id === "http_request" || action.id === "select_agents") { + if (!action.isComingSoon) { // For AI Agent, Q&A Agent, Email, HTTP Request, and Select Agents, trigger custom event to open respective ConfigUI onSelectAction(action.id) onClose() // Close WhatHappensNextUI }This way, any action in
availableActions(which doesn't haveisComingSoonset) will automatically work without modifying this condition.server/api/script.ts (1)
2-2: Remove unused import.
getLoggeris imported but never used in this file.Apply this diff:
-import { getLogger, getLoggerWithChild } from "@/logger" +import { getLoggerWithChild } from "@/logger"server/api/workflow.ts (1)
230-230: Script tool integration looks consistent; consider a couple of robustness tweaksThe overall flow (language mapping, latest-step-based
scriptInput, and normalized success/error payloads) aligns well with other tool cases and theexecuteScriptcontract.Two improvements worth considering:
- Defensive checks for
tool.valueandlanguage
Right nowtool.valueandtool.value.languageare assumed to exist; if either isnull/undefinedyou hitlanguage.toLowerCase()and fall into the catch with a generic"Script execution failed"message. A small guard would surface a clearer error and avoid relying on exception control flow:case "script":
- const scriptContent = tool.value.script
- const scriptConfig = tool.value.config
- const language = tool.value.language
- if (!tool?.value || typeof tool.value !== "object") {
- return {
status: "error",result: { error: "Invalid script tool configuration: missing value object" },- }
- }
- const scriptContent = tool.value.script
- const scriptConfig = tool.value.config
- const language = tool.value.language
- if (!language || typeof language !== "string") {
- return {
status: "error",result: { error: "Script language is required in tool value" },- }
- }
Logger.info(Executing script in language: ${language})
if (!scriptContent) {
return {
status: "error",
result: { error: "No script content found in tool value" },
}
}This keeps failures clearly attributable to misconfiguration rather than execution.
- Optional: expose more control over script input selection
Using the latest step’s.resultor.formSubmissionasscriptInputis a reasonable default, but some workflows may need a specific upstream step or the fullpreviousStepResults. In the future, you might allowtool.configto specify a preferred step key or a simple path (e.g.,config.inputSourceStep/config.inputPath) and fall back to the current heuristic when unset. This would make script behavior more predictable in multi-branch workflows without breaking existing flows.Also applies to: 3735-3833
frontend/src/components/CodeEditor.tsx (2)
1-5: Use a type-only import formonaco-editorto avoid extra bundle weightYou only use
editoras a type (for the ref andhandleEditorDidMountsignature), butimport { editor } from "monaco-editor"pulls the runtime module into the bundle even though it’s not used. Converting this to a type-only import keeps the types without impacting the runtime bundle:-import React, { useRef, useEffect } from "react" -import { Editor } from "@monaco-editor/react" -import { X, Save } from "lucide-react" -import { Button } from "@/components/ui/button" -import { editor } from "monaco-editor" +import React, { useRef, useEffect } from "react" +import { Editor } from "@monaco-editor/react" +import { X, Save } from "lucide-react" +import { Button } from "@/components/ui/button" +import type { editor } from "monaco-editor"This keeps type safety while preventing an unnecessary direct import of Monaco itself.
27-45: Simplify Monaco loading/error state; currently the error path is unreachableThe loading and error handling around Monaco can be tightened up:
isMonacoLoadingis driven by a 1s timeout onisOpenand also set tofalseinhandleEditorDidMount, but there’s no path that ever setsmonacoErrorto a non-null value. As a result, the “error” branch with the textarea fallback is effectively dead code.- You already pass a
loadingJSX to the<Editor />component, so the outerisMonacoLoading ? … : <Editor />guard is somewhat redundant;@monaco-editor/reactcan handle showing the loader until Monaco is ready.- The artificial 1s delay before flipping
isMonacoLoadingoff will keep showing the custom spinner even if Monaco is actually ready sooner.If you don’t specifically need the external spinner state, you can simplify to rely entirely on the Editor’s built-in loading and remove the unused error state, e.g.:
-const [isMonacoLoading, setIsMonacoLoading] = React.useState(true) -const [monacoError, setMonacoError] = React.useState<string | null>(null) +const [isMonacoLoading, setIsMonacoLoading] = React.useState(true) -useEffect(() => { - if (isOpen) { - const timer = setTimeout(() => { - setIsMonacoLoading(false) - }, 1000) - return () => clearTimeout(timer) - } -}, [isOpen]) +// Optional: drop this effect entirely and let onMount + Editor.loading handle UX, +// or just set isMonacoLoading(false) in onMount. -const handleEditorDidMount = (editor: editor.IStandaloneCodeEditor, monaco: typeof import('monaco-editor')) => { +const handleEditorDidMount = (editor: editor.IStandaloneCodeEditor, monaco: typeof import('monaco-editor')) => { editorRef.current = editor setIsMonacoLoading(false) - setMonacoError(null) … } … - <div className="flex-1 overflow-hidden"> - {isMonacoLoading ? ( - …custom spinner… - ) : monacoError ? ( - …textarea fallback… - ) : ( - <Editor + <div className="flex-1 overflow-hidden"> + <Editor height="100%" language={language} value={currentValue} onChange={handleEditorChange} onMount={handleEditorDidMount} loading={ <div className="flex items-center justify-center h-full"> … </div> } theme={monacoTheme} options={{ … }} - /> - )} + /> </div>This reduces state, removes dead branches, and lets Monaco’s own loading callback drive the UX.
Also applies to: 51-79, 124-179
frontend/src/components/workflow/executedWorkflowRenderer.tsx (3)
788-805: Fix the misleading comment.The comment says "Bot icon with background" but the code renders a Code icon. This should be corrected for maintainability.
- {/* Header with icon and title */} - <div className="flex items-center gap-3 text-left w-full px-4 pt-4 mb-3"> - {/* Bot icon with background */} + {/* Header with icon and title */} + <div className="flex items-center gap-3 text-left w-full px-4 pt-4 mb-3"> + {/* Code icon with background */}
752-999: Consider consolidating python_script and script node rendering.The
python_script(lines 752-872) andscript(lines 874-999) rendering branches share nearly identical structure and styling. This code duplication could be reduced by extracting a sharedScriptNodeRenderercomponent that accepts the node type as a parameter.This would improve maintainability and reduce the ~250 lines of duplicate code.
228-238: Extract repeated border style logic into a helper function.This nested ternary pattern for computing border colors based on
selected,isFailed, andisCompletedstates is repeated across all node type branches (lines 228-238, 428-438, 548-558, 769-779, 897-907, 1017-1027).Consider extracting this to a helper function for better maintainability:
const getNodeBorderStyle = (selected: boolean, isFailed: boolean, isCompleted: boolean) => { if (selected) { if (isFailed) return "2px solid #DC2626" if (isCompleted) return "2px solid #059669" return "2px solid #111827" } if (isFailed) return "2px solid #F87171" if (isCompleted) return "2px solid #34D399" return "2px solid #6B7280" }frontend/src/components/ScriptSidebar.tsx (2)
86-98: Add cleanup for the fetch operation to prevent memory leaks.The
useEffectfetches languages when the sidebar opens but doesn't handle component unmounting during the fetch. This could cause a "setState on unmounted component" warning.useEffect(() => { + let isMounted = true + if (isOpen) { - fetchLanguages() + const loadLanguages = async () => { + try { + setIsLoading(true) + setError(null) + const response = await api.workflow.script.languages.$get() + + if (!response.ok) { + throw new Error("Failed to fetch script languages") + } + + const data = await response.json() + if (isMounted) { + setLanguages(data.availableLanguages) + + if (data.availableLanguages.length > 0 && !initialData) { + const defaultLang = data.availableLanguages[0] + setSelectedLanguage(defaultLang.language) + setCode(defaultLang.codeWritingBlock) + } + } + } catch (err) { + if (isMounted) { + setError(err instanceof Error ? err.message : "Failed to load languages") + } + } finally { + if (isMounted) { + setIsLoading(false) + } + } + } + loadLanguages() } else { // Reset state when sidebar closes setSelectedLanguage("") setCode("") setConfig("{}") setSavedCodes({}) setSavedConfigs({}) setConfigError(null) } + + return () => { + isMounted = false + } }, [isOpen])
396-424: Add click-outside handler and keyboard navigation for the dropdown.The custom dropdown lacks:
- Click-outside-to-close behavior
- Keyboard navigation (Escape to close, arrow keys)
- ARIA attributes for accessibility
Consider using a library like Radix UI's Select component (which may already be in your design system) or adding these behaviors:
+// Add to component body: +useEffect(() => { + const handleClickOutside = (e: MouseEvent) => { + if (isLanguageDropdownOpen) { + setIsLanguageDropdownOpen(false) + } + } + const handleEscape = (e: KeyboardEvent) => { + if (e.key === 'Escape' && isLanguageDropdownOpen) { + setIsLanguageDropdownOpen(false) + } + } + document.addEventListener('mousedown', handleClickOutside) + document.addEventListener('keydown', handleEscape) + return () => { + document.removeEventListener('mousedown', handleClickOutside) + document.removeEventListener('keydown', handleEscape) + } +}, [isLanguageDropdownOpen]) // Add aria attributes to dropdown button: <button onClick={() => setIsLanguageDropdownOpen(!isLanguageDropdownOpen)} + aria-haspopup="listbox" + aria-expanded={isLanguageDropdownOpen} className="w-full h-10 px-3 py-2 ..." >server/workflowScriptExecutorTool.ts (2)
346-349: Consider moving require statements to top-level imports.Using
require()inside the function works but is unconventional. Moving these to top-level imports improves code clarity and follows Node.js best practices.+import * as fs from 'fs/promises' +import * as path from 'path' +import { randomUUID } from 'crypto' async function generateScript(script: string, language: ScriptLanguage, input: Record<string, any>, config?: Record<string, any>): Promise<{ success: boolean; filePath?: string; error?: string }> { - const fs = require('fs').promises - const path = require('path') - const { randomUUID } = require('crypto')
720-735: Command construction is safe but could be more robust.The command string is split by space on line 735, which works because
scriptPathis internally generated with a UUID-based filename (no spaces). However, this pattern is fragile if future changes introduce spaces in paths.Consider using array-based command construction from the start:
- const runCommand = `${langConfig.runCommand} ${scriptPath}`.trim() - - if (!runCommand) { + if (!langConfig.runCommand) { cleanTempFiles(scriptPath, exportDir) return { success: false, output: {}, error: `Unsupported language: ${executionInput.language}`, exitCode: -1 } } - // Parse command and arguments - let [command, ...args] = runCommand.split(' ') + let command = langConfig.runCommand + let args = [scriptPath]frontend/src/components/workflow/WorkflowBuilder.tsx (3)
168-168: ScriptSidebar import and state wiring look consistent, consider avoiding the"pending"sentinel.The
ScriptSidebarimport and the newshowScriptConfigUI/selectedScriptNodeIdstate follow the same pattern as other sidebars and node IDs, so the wiring is correct. The use of a"pending"string sentinel onselectedScriptNodeIdis functional but slightly opaque; if this grows, consider an explicit union type or a separateisScriptCreateModeflag for clarity.Also applies to: 2764-2778
1723-1891: Script StepNode rendering is aligned with other node types; minor robustness and reuse opportunities.The script node card mirrors the HTTP/email/form/webhook patterns and correctly derives title/description from
tools[0].val/tools[0].valueandstepfallbacks, so it should render existing and newly created script tools fine. Two small polish ideas:
- When deriving the language (
language.charAt(0)/slice(1)), you may want to guard withtypeof language === "string"to avoid surprises if upstream ever passes a non-string.- A lot of JSX (card container, handles, plus button) is duplicated from other node renderers; if this area evolves further, a shared “NodeShell” component could reduce duplication and risk of subtle styling drift.
5956-6132: ScriptSidebar integration works end‑to‑end; consider tightening selection behavior and tool-update robustness.The overall integration (open/close/back handlers,
selectedNodeId,initialData,onStepCreated,onToolUpdated) is sound and follows the patterns used for other tool config sidebars. A couple of concrete improvements would make it behave more like the existing nodes and be more robust:
New script node is never selected after creation.
InonStepCreated, the new node is added but not markedselected, and the follow‑upsetNodescall only removeshasNextfrom the source node without explicitly selecting the new node or deselecting others. All the other creation flows (AI, Q&A, Email, HTTP) select the newly created node and clear selection elsewhere. You can align behavior like this:- const newNode = { - id: newNodeId, - type: "stepNode", - position: { - x: 400, - y: sourceNode.position.y + 250, - }, - data: { + const newNode = { + id: newNodeId, + type: "stepNode", + position: { + x: 400, + y: sourceNode.position.y + 250, + },
data: { step: { id: newNodeId, name: stepData.name, description: stepData.description, type: stepData.type, status: "pending", contents: [], config: stepData.tool?.value || {}, }, tools: stepData.tool ? [stepData.tool] : [], isActive: false, isCompleted: false, hasNext: true, },
draggable: true,
draggable: true,selected: true, }...
setNodes((prevNodes) =>prevNodes.map((node) =>node.id === selectedNodeForNext? {...node,data: {...node.data,hasNext: false,},}: node,),)
setNodes((prevNodes) =>prevNodes.map((node) =>node.id === selectedNodeForNext? {...node,data: { ...node.data, hasNext: false },selected: false,}: node.id === newNodeId? { ...node, selected: true }: { ...node, selected: false },),)2. **Guard against empty `tools` when updating a script tool.** `onToolUpdated` special‑cases `updatedTool.type === "script"` by replacing the first tool only if `currentTools.length > 0`. If a script node ever has an empty `tools` array (due to future refactors or backend data quirks), this would silently drop the updated tool. A small adjustment makes it more defensive: ```diff - if (updatedTool.type === "script" && currentTools.length > 0) { - updatedTools = [updatedTool] - } else { + if (updatedTool.type === "script") { + // Always ensure there's exactly one script tool + updatedTools = [updatedTool] + } else { // Fallback to ID matching for other tool types updatedTools = currentTools.map((tool) => tool.id === updatedTool.id ? updatedTool : tool ) - // If no tool was updated by ID matching, replace the first tool - if (updatedTools.every(tool => tool !== updatedTool) && currentTools.length > 0) { - updatedTools[0] = updatedTool - } + // If no tool was updated by ID matching and we have tools, replace the first + if ( + currentTools.length > 0 && + !currentTools.some(tool => tool.id === updatedTool.id) + ) { + updatedTools[0] = updatedTool + } }
Skip
initialDatalookup in create‑mode to avoid noisy logs.
WhenselectedScriptNodeId === "pending", theinitialDataIIFE runs, logs a node lookup withnode/toolsundefined, and returnsundefinedanyway. That’s harmless but noisy; you can gate it to only run for real node IDs:- initialData={ - selectedScriptNodeId + initialData={ + selectedScriptNodeId && selectedScriptNodeId !== "pending" ? (() => { const node = nodes.find((n) => n.id === selectedScriptNodeId) const tools = node?.data?.tools as Tool[] | undefined const tool = tools && tools.length > 0 ? tools[0] : undefined console.log("WorkflowBuilder: Passing script tool data to sidebar:", { nodeId: selectedScriptNodeId, node: node, tools: tools, tool: tool }) return tool })() : undefined }Keep script config local to builder unless intentionally persisting.
From theScriptSidebardescription, it can optionally hitworkflowStepsAPI/workflowToolsAPIwhen a template exists. In this builder, all other tools (except Jira) are intentionally kept as local ReactFlow state and only persisted throughhandleSaveChanges. It’s worth double‑checking that for builder/create flows we rely solely ononStepCreated/onToolUpdatedand not on ScriptSidebar’s internal API calls, to avoid introducing a second persistence path. Based on learnings, this component should stay the single source of truth for saves.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
Dockerfile(2 hunks)deployment/portable/docker-compose.app-build.yml(1 hunks)deployment/portable/nginx.conf(1 hunks)frontend/package.json(2 hunks)frontend/src/components/CodeEditor.tsx(1 hunks)frontend/src/components/ScriptSidebar.tsx(1 hunks)frontend/src/components/workflow/WhatHappensNextUI.tsx(2 hunks)frontend/src/components/workflow/WorkflowBuilder.tsx(10 hunks)frontend/src/components/workflow/executedWorkflowRenderer.tsx(12 hunks)js-requirements.json(1 hunks)requirements.R(1 hunks)requirements.txt(1 hunks)server/.env.default(1 hunks)server/api/script.ts(1 hunks)server/api/workflow.ts(2 hunks)server/server.ts(2 hunks)server/types/workflowTypes.ts(1 hunks)server/workflowScriptExecutorTool.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-17T08:54:59.756Z
Learnt from: debajyoti0606
Repo: xynehq/xyne PR: 1075
File: server/workflowScriptExecutorTool.ts:36-77
Timestamp: 2025-10-17T08:54:59.756Z
Learning: In server/workflowScriptExecutorTool.ts, the validateJson function's escape sequence normalization logic (lines 41-52 that replace \n, \r, \t, \f, \b, \", and \\ before parsing) is intentionally designed to handle edge cases and should not be flagged as potentially corrupting valid JSON. The double-escaping behavior is expected.
Applied to files:
server/workflowScriptExecutorTool.tsserver/api/workflow.ts
📚 Learning: 2025-10-17T10:33:37.098Z
Learnt from: debajyoti0606
Repo: xynehq/xyne PR: 1075
File: server/workflowScriptExecutorTool.ts:480-532
Timestamp: 2025-10-17T10:33:37.098Z
Learning: In server/workflowScriptExecutorTool.ts, the formatJavaScriptOutput function's regex pattern for quoting property names (using `/([{,]\s*)([a-zA-Z_$][a-zA-Z0-9_$]*)\s*:/g`) is intentionally designed as a best-effort approach, even though it may match inside string values in edge cases. This is expected behavior with fallback handling via try-catch blocks.
Applied to files:
server/workflowScriptExecutorTool.tsserver/api/workflow.ts
📚 Learning: 2025-11-12T06:07:09.854Z
Learnt from: Sithaarth24
Repo: xynehq/xyne PR: 1204
File: frontend/src/components/workflow/WorkflowBuilder.tsx:3851-3938
Timestamp: 2025-11-12T06:07:09.854Z
Learning: In the WorkflowBuilder component, workflow node configurations (including Slack Trigger, AI Agent, Email, HTTP Request, Form, etc.) should only update local React state when users configure them. Backend persistence happens through a single batch save operation when the user clicks the "Save Changes" button (handleSaveChanges function). Only Jira currently has immediate backend persistence (will be changed to match this pattern).
Applied to files:
frontend/src/components/workflow/WorkflowBuilder.tsx
📚 Learning: 2025-06-23T07:43:59.494Z
Learnt from: rahul1841
Repo: xynehq/xyne PR: 582
File: frontend/src/routes/_authenticated/agent.tsx:1654-1654
Timestamp: 2025-06-23T07:43:59.494Z
Learning: In the agent test interface dropdown (frontend/src/routes/_authenticated/agent.tsx), the agent selection dropdown specifically uses `allAgentsList` only and should only check `allAgentsList.length > 0` for visibility. It does not need to check other agent lists like `madeByMeAgentsList` or `sharedToMeAgentsList` because the dropdown is designed to show agents from the "all agents" tab only.
Applied to files:
frontend/src/components/workflow/WorkflowBuilder.tsx
🧬 Code graph analysis (5)
server/server.ts (1)
server/api/script.ts (1)
GetScriptLanguagesApi(10-33)
frontend/src/components/CodeEditor.tsx (1)
frontend/src/components/ui/button.tsx (1)
Button(57-57)
frontend/src/components/ScriptSidebar.tsx (6)
frontend/src/components/ThemeContext.tsx (1)
useTheme(89-95)frontend/src/api.ts (1)
api(5-5)frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(396-1064)frontend/src/components/ui/button.tsx (1)
Button(57-57)frontend/src/components/ui/label.tsx (1)
Label(24-24)frontend/src/components/CodeEditor.tsx (1)
CodeEditor(17-195)
server/api/workflow.ts (1)
server/workflowScriptExecutorTool.ts (2)
ScriptLanguage(873-873)executeScript(679-871)
frontend/src/components/workflow/WorkflowBuilder.tsx (2)
frontend/src/components/ScriptSidebar.tsx (1)
ScriptSidebar(30-520)frontend/src/components/workflow/Types.ts (1)
Tool(58-63)
🔇 Additional comments (18)
server/types/workflowTypes.ts (1)
15-29:ToolType.SCRIPTenum addition looks consistent.The new
SCRIPT = "script"member is consistent with existingToolTypenaming and doesn’t introduce any breaking changes here. Just ensure all server/router logic and UI switch statements that depend onToolTypenow handle this new case (which the rest of the PR appears to be doing).server/server.ts (1)
330-330: LGTM!The import and route for script language discovery are correctly placed and follow the existing patterns in the codebase.
Also applies to: 1546-1546
frontend/src/components/workflow/WhatHappensNextUI.tsx (1)
55-60: LGTM!The "Run Script/Code" action is well-defined and follows the existing pattern.
server/api/script.ts (1)
8-8: Logger usage pattern is correct — no changes needed.The logger is intentionally designed as a factory function.
getLoggerWithChild()returns a callable that accepts optional context parameters. It can be invoked as:
loggerWithChild({ email: userEmail }).info(...)when email context is availableloggerWithChild().error(...)when context is not availableThis pattern is consistent throughout the codebase (e.g.,
server/db/agent.ts,server/api/knowledgeBase.ts) and aligns with the intentional design to handle different logging scenarios.frontend/src/components/workflow/executedWorkflowRenderer.tsx (3)
2-2: LGTM!Code icon import correctly added for the new script node types.
1489-1516: LGTM!The script node input handling correctly identifies script steps via both
step.typeand tool type checks, and appropriately renders the complete JSON input data for script nodes.
1759-1769: LGTM!Script tool output rendering appropriately displays formatted JSON with monospace font styling, consistent with the expected output format from script executions.
frontend/src/components/ScriptSidebar.tsx (3)
100-126: LGTM!The effect correctly handles initial data loading with backward compatibility for both
valueandvalfields, and properly populates the saved codes/configs cache.
210-338: LGTM!The save handler correctly validates JSON config before parsing, handles both update and create flows with a UI-first approach, and properly manages API persistence without blocking the user experience.
497-517: LGTM!CodeEditor integration is correctly implemented with separate instances for code and config, proper language settings, and theme synchronization.
server/workflowScriptExecutorTool.ts (5)
36-77: LGTM!The JSON validation function correctly handles both string and object inputs with intentional escape sequence normalization for edge cases. Based on learnings, this behavior is expected.
593-658: Network access is not restricted in the sandbox - verify this is intentional.The Bubblewrap configuration does not include
--unshare-net, meaning sandboxed scripts can make network requests. This appears intentional given the allowed imports include networking libraries (requests,http,fetch,axios,httr,curl).However, this means malicious scripts could potentially:
- Exfiltrate data to external servers
- Make requests to internal services
Confirm this is the intended behavior, or add
--unshare-netif network access should be restricted.
660-677: LGTM!The cleanup function correctly uses fire-and-forget async deletion with proper error handling. This ensures cleanup doesn't block execution while still logging any issues.
875-885: LGTM!The
getCodeWritingBlockfunction provides helpful language-specific templates with clear documentation for input/config access, allowed imports, and file I/O directories.
229-343: Security layering verified - review comment is accurate.Verification confirms the security architecture matches the review concern:
USE_BUBBLEWRAPdefaults totrue(line 33, opt-out via env var)- Bubblewrap sandbox is applied at runtime (lines 738, 742)
validateScriptSecurityviolations are enforced and block execution (lines 376-378)The code correctly implements defense-in-depth:
validateScriptSecurityprovides first-layer validation, and Bubblewrap serves as the primary runtime boundary. No changes needed.frontend/src/components/workflow/WorkflowBuilder.tsx (3)
3523-3536: Script node click behavior matches existing config-sidebars flow.On node click you now reset
showScriptConfigUI/selectedScriptNodeIdalong with the other sidebars, then branch ontoolType === "script"to openScriptSidebarwith the clicked node’s ID. This is consistent with how email/HTTP/form/webhook/Jira paths work and avoids sidebar overlap; there’s no obvious state or dependency issue here.Also applies to: 3649-3654
3855-3876: Closing script sidebar when switching to Triggers/WhatHappensNext avoids overlapping panels.The added
setShowScriptConfigUI(false)calls in theopenWhatHappensNext/openTriggersSidebarhandlers correctly ensure that the script config UI can’t remain open behind other high-level sidebars. This matches the mutual-exclusion behavior of the other tool sidebars.
4182-4190:run_scriptaction wiring is consistent with other “What happens next” actions.The new
actionId === "run_script"branch mirrors the email/AI flows by settingselectedScriptNodeIdto"pending"and openingScriptSidebarwhile keepingWhatHappensNextUIvisible in the background. This is the right place to hang script creation off the “+” button and reuses the existing “pending” create-mode convention cleanly.
| container_name: xyne-app | ||
| user: "${DOCKER_UID}:${DOCKER_GID}" | ||
| # Ports removed - NGINX handles external access | ||
| privileged: true |
There was a problem hiding this comment.
privileged: true seriously weakens isolation, especially with script execution and setuid bwrap.
Granting full privileged: true to the app container effectively disables most Docker isolation. Combined with:
- Untrusted user‑supplied Python/JS/R scripts, and
- A setuid‑root
bwrapbinary inside the image,
a compromise of the script executor or an argument‑validation bug in the bubblewrap wrapper can turn into near‑host‑level access.
Unless you have a very specific, documented requirement, it would be much safer to:
- Remove
privileged: true, and - If needed, add only narrowly scoped
cap_addcapabilities (or devices) required by bubblewrap, instead of the full privileged set.
If this is only for a local “power user” build flow, please clearly mark it as such and keep your production/staging compose files non‑privileged.
| add_header Cross-Origin-Opener-Policy "same-origin" always; | ||
| add_header Cross-Origin-Resource-Policy "same-origin" always; | ||
| add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; img-src 'self' data: https:; font-src 'self' https://fonts.gstatic.com; connect-src 'self' https: wss: ws:; frame-ancestors 'none';" always; | ||
| add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval' https://cdn.jsdelivr.net; worker-src 'self' blob:; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://cdn.jsdelivr.net; img-src 'self' data: https:; font-src 'self' https://fonts.gstatic.com; connect-src 'self' https: wss: ws:; frame-ancestors 'none';" always; |
There was a problem hiding this comment.
🧩 Analysis chain
Adding third-party CDN to CSP increases supply chain attack risk.
Allowing https://cdn.jsdelivr.net in script-src introduces a third-party dependency that could serve malicious code if compromised. jsdelivr is a public CDN hosting user-uploaded packages, creating a broader attack surface. While jsdelivr is reputable and likely required for Monaco Editor, consider these mitigations:
-
Use Subresource Integrity (SRI) hashes when loading resources from jsdelivr to ensure they haven't been tampered with:
<script src="https://cdn.jsdelivr.net/..." integrity="sha384-..." crossorigin="anonymous"></script>
-
Consider hosting Monaco Editor locally to eliminate third-party dependency entirely and reduce attack surface.
-
Document the security trade-off and establish a plan to periodically review the necessity of this CDN dependency.
Note: The CSP already includes 'unsafe-inline' and 'unsafe-eval', which significantly weaken protection against XSS. Consider replacing these with nonces or hashes in a future iteration.
Would you like me to generate a script to verify which Monaco Editor resources are being loaded from jsdelivr, or help create a local hosting alternative?
I'll verify the review comment by examining the actual code changes and context. Let me start with shell scripts to inspect the nginx.conf file and related Monaco Editor integration.
🏁 Script executed:
#!/bin/bash
# Examine the nginx.conf file around line 54
echo "=== nginx.conf content around line 54 ==="
sed -n '50,60p' deployment/portable/nginx.conf
echo -e "\n=== Full CSP header (if spanning multiple lines) ==="
sed -n '54p' deployment/portable/nginx.conf | head -c 500
echo -e "\n\n=== Search for Monaco Editor references ==="
rg -i "monaco" --type-list | head -5
rg -i "monaco" -n -C 3 2>/dev/null || echo "No Monaco references found"
echo -e "\n=== Search for jsdelivr references in codebase ==="
rg "jsdelivr" -n -C 2 2>/dev/null || echo "No jsdelivr references found"
echo -e "\n=== Check for web worker configuration ==="
rg "worker-src" -n -C 2 deployment/portable/nginx.confLength of output: 11425
🏁 Script executed:
#!/bin/bash
# Check HTML/template files for Monaco Editor loading
echo "=== Search for script tags loading Monaco or CDN resources ==="
find . -type f \( -name "*.html" -o -name "*.htm" -o -name "*.hjs" -o -name "*.ejs" \) -exec grep -l "monaco\|cdn.jsdelivr" {} \; 2>/dev/null | head -10
echo -e "\n=== Check HTML files for resource loading patterns ==="
rg "<script.*src" -n --type=html 2>/dev/null | grep -E "(monaco|jsdelivr|cdn)" | head -20Length of output: 118
🌐 Web query:
jsdelivr CDN supply chain security risks vulnerabilities 2024 2025
💡 Result:
Summary — jsDelivr CDN supply‑chain risks observed 2024–2025
Key incidents and findings
- Attackers have hosted trojanized JavaScript on jsDelivr that exfiltrated form data and harvested credentials; researchers found malicious scripts served directly from the official jsDelivr URLs. [1]
- Multiple 2025 npm supply‑chain compromises (phishing of maintainers) produced malicious packages whose browser‑side code was also mirrored to CDNs (including jsDelivr), enabling live exploitation in web pages using those CDN URLs. Those compromises affected high‑download packages and were live for short windows, magnifying risk to sites that hot‑link remote assets. [2][3]
- Earlier CDN supply‑chain compromises (e.g., polyfill.js) show attackers can replace or poison commonly included shared resources; CVE advisories and vendor notes documented real-world impact and remediation steps. [4]
Primary attack vectors and vulnerabilities
- Trust in remote, mutable CDN paths: pulling scripts by URL (no integrity checks) lets any updated/tainted file execute in site context. [1][2]
- Compromise of upstream package maintainers or repositories can propagate malicious code into CDN‑served assets. [2]
- Dynamic/obfuscated payloads served from CDNs evade casual review and can target specific victims or environments (conditional logic). [1][3]
- Lack of subresource integrity (SRI), no strict Content Security Policy (CSP), and wide reuse of popular URLs increase blast radius. [1][4]
Practical mitigations (concise)
- Serve third‑party libraries from pinned, internal copies (bundle in builds) instead of hot‑linking CDN URLs. [1][4]
- Use Subresource Integrity (SRI) for public CDN assets plus strict CSP to prevent unexpected script execution. [1]
- Monitor dependencies and CDN URLs (SCA, SRI mismatch alerts, file‑hash monitoring) and deploy rapid rollback/incident playbooks. [2][3]
- Enforce strong maintainer hygiene: 2FA, org‑level protections, phishing resistance training, and least‑privilege tokens for package publishing. [2]
- Prefer signed, immutable artifact delivery (artifact registries with supply‑chain provenance/SBOMs) and automate dependency scanning in CI. [3][4]
If you want, I can:
- list the exact compromised package names/IOCs from the 2025 incidents,
- produce SRI/CSP examples and a short remediation checklist for a web app.
Sources
[1] Cside blog: “Web supply chain attack through trojanized jQuery on npm, GitHub and CDNs” (jsDelivr evidence). (cside.com)
[2] Multiple reports on the Sept 2025 npm supply‑chain compromises (coverage & advisories). (sisainfosec.com)
[3] Technical writeups summarizing the 2025 npm campaign and package list. (foresiet.com)
[4] Posit / Sansec / CVE writeups on CDN polyfill compromise and CVE‑2024‑38526. (support.posit.co)
Citations:
- 1: https://cside.com/blog/web-supply-chain-attack-through-trojanized-jquery-on-npm-github-and-cdns?utm_source=openai
- 2: https://www.sisainfosec.com/blogs/npm-supply-chain-attack-hits-packages-with-billions-of-weekly-downloads-advisory-by-sisa-sappers/?utm_source=openai
- 3: https://foresiet.com/blog/npm-supply-chain-attack-2025/?utm_source=openai
- 4: https://support.posit.co/hc/en-us/articles/24767859071895-PolyFill-CDN-Supply-Chain-Attack-CVE-2024-38526?utm_source=openai
I need to verify how Monaco Editor loads its assets to determine if the SRI recommendations are feasible.
🏁 Script executed:
#!/bin/bash
# Check if Monaco Editor loads from CDN or is bundled
echo "=== Check package.json Monaco configuration ==="
jq '.dependencies | keys[] | select(. | contains("monaco"))' frontend/package.json
echo -e "\n=== Check CodeEditor.tsx for loader configuration or CDN references ==="
grep -n -i "cdn\|jsdelivr\|loader\|src=" frontend/src/components/CodeEditor.tsx | head -20
echo -e "\n=== Check public directory for Monaco files ==="
find . -name "*monaco*" -type f 2>/dev/null | grep -v node_modules | head -20
echo -e "\n=== Search for webpack/build config that might bundle Monaco ==="
find . -name "webpack.config.*" -o -name "vite.config.*" -o -name "tsconfig.json" 2>/dev/null | head -5
rg "monaco" webpack.config.* vite.config.* 2>/dev/null || echo "No Monaco refs in build config"
echo -e "\n=== Check if @monaco-editor/react uses default loader or custom CDN ==="
grep -r "monacoEditorModule\|getEditorModule\|loader" frontend/src/components/CodeEditor.tsx 2>/dev/null | head -10Length of output: 581
🌐 Web query:
@monaco-editor/react CDN loading subresource integrity SRI
💡 Result:
Short answer: you can use SRI when loading @monaco-editor/react from a CDN, but you must use fixed, single-file (versioned) URLs and provide matching integrity hashes for every cross‑origin resource; because Monaco also loads multiple worker and asset files at runtime, in practice it's usually easier to self‑host the Monaco assets (or pin & SRI every file) and add crossorigin="anonymous". [1][2][3]
What to do (concise steps)
- Use a versioned, single-file CDN URL (no aliases or dynamic/minified-on-demand URLs). Example provider: jsDelivr supports npm package URLs. [2][1]
- Compute the SRI hash for the exact file you will load (sha384 or sha512 recommended). You can generate this during your build (tools: sri-toolbox / node-sri or your build pipeline). [5][6]
- Add both integrity and crossorigin attributes to the <script> or tag. Example: <script src="https://cdn.jsdelivr.net/npm/@monaco-editor/react@4.7.0/dist/index.umd.js" integrity="sha384-BASE64_HASH_HERE" crossorigin="anonymous"></script>
- If Monaco will fetch additional runtime files (workers, language files, etc.), those must either be:
- Hosted locally (so you can control and SRI them), or
- Also referenced by exact versioned URLs with integrity hashes — otherwise SRI will break when those files are fetched/changed. jsDelivr explicitly warns not to use SRI with dynamically generated or version‑aliased files. [1]
Useful references
- jsDelivr: Using SRI with dynamic files (how/when to use SRI and warnings about dynamic URLs). [1]
- @monaco-editor/react docs / npm / GitHub: notes about CDN usage and loader/config (Monaco loads assets and can be reconfigured to load from local paths). [2][3]
- Tools to generate SRI hashes: sri-toolbox, node-sri (for CI/build). [5][6]
If you want, tell me:
- which CDN (jsDelivr / unpkg / unpkg-style URL) and exact package version you plan to use, and whether you can self‑host Monaco worker files — I’ll generate the exact script/link tags and show how to compute the integrity hashes for those files.
Sources:
[1] jsDelivr — Using SRI with dynamic files.
[2] @monaco-editor/react — npm / docs on CDN usage.
[3] monaco-react — GitHub README (loader.config details).
[5] sri-toolbox (generate SRI in build).
[6] node-sri (generate SRI).
CDN supply chain risk in CSP requires alternative mitigation approach.
Adding https://cdn.jsdelivr.net to script-src introduces legitimate supply chain risk. Recent 2024–2025 CDN attacks show malicious scripts served from jsDelivr URLs that exfiltrated credentials, and compromised npm packages were mirrored to CDNs enabling live exploitation. However, the recommended approach needs adjustment:
SRI hashes are not practical here. @monaco-editor/react uses a default dynamic loader that fetches multiple runtime assets (workers, language files) at startup. jsDelivr warns against using SRI with dynamically generated or version-aliased files, making SRI ineffective for this configuration.
Instead, consider:
-
Bundle Monaco locally — Package
@monaco-editor/reactandmonaco-editoras part of your build rather than loading from CDN. This eliminates the third-party dependency entirely and is the most robust mitigation. -
If CDN is necessary, accept the trade-off and document it as a calculated risk, noting the need for periodic supply chain reviews and a rollback plan in case of compromise.
-
Plan CSP improvements — The existing
'unsafe-inline'and'unsafe-eval'inscript-srcalready weaken protection significantly. Removing these should be prioritized once Monaco (or alternatives) can run with stricter CSP directives.
🤖 Prompt for AI Agents
In deployment/portable/nginx.conf around line 54, the CSP currently allows
scripts from https://cdn.jsdelivr.net which creates a supply-chain risk; fix by
removing the CDN from script-src and instead bundle
@monaco-editor/react/monaco-editor into the app build so runtime assets are
served locally (update build config to include Monaco assets and serve them from
your origin), or if you must keep the CDN, document it as an accepted risk with
a plan for periodic supply-chain reviews and rollback procedures; additionally,
plan and track a follow-up to remove 'unsafe-inline' and 'unsafe-eval' from
script-src once Monaco is running from local bundles and tighten
worker-src/connect-src as appropriate.
| python3 python3-pip nodejs npm r-base-core r-base-dev \ | ||
| build-essential libcurl4-openssl-dev libssl-dev libxml2-dev \ | ||
| jq bubblewrap \ |
There was a problem hiding this comment.
Setuid bwrap + script execution + privileged compose is a dangerous combination.
This line:
RUN chmod u+s $(which bwrap)turns bwrap into a setuid‑root helper. On its own, that’s expected for unprivileged bubblewrap sandboxing. However, in this PR it’s combined with:
- A workflow feature that runs user‑authored Python/JS/R scripts, and
- A
docker-composeconfig that setsprivileged: trueon the app service.
That means a bug in your script‑executor wrapper (e.g., insufficient validation of paths/flags passed to bwrap) could give an attacker a path from “run arbitrary script code” → “root inside container” → “root with near‑host capabilities”.
Recommendations:
- If at all possible, drop
privileged: trueindocker-compose.app-build.ymland rely on unprivileged bubblewrap + namespaces only. - Ensure the server‑side script executor strictly whitelists what can be mounted/exposed into the sandbox (no arbitrary
--bind /or host paths). - Consider separating a stricter “production” Dockerfile/compose profile where the threat model explicitly assumes untrusted scripts.
Also applies to: 94-94
🤖 Prompt for AI Agents
In Dockerfile around lines 51-53 (and also line 94), the build currently
installs bubblewrap and sets it setuid-root via "chmod u+s $(which bwrap)",
which combined with the repo's privileged docker-compose and server-side script
execution creates an escalation risk; remove the setuid change and avoid setting
privileged: true in docker-compose.app-build.yml, restrict the script executor
to a strict whitelist of allowed mounts/flags and disallow binding arbitrary
host paths, and add a separate production Dockerfile/compose profile (or build
arg) that enforces unprivileged sandboxing and stricter runtime constraints for
running untrusted scripts.
Language runtime setup works, but npm global install is non‑deterministic.
The overall pattern (install Python/Node/R + requirements, create script_executor_utils, chown to bun) fits the script‑executor goal. One issue is reproducibility of the JS runtime:
RUN npm install -g $(cat /usr/src/app/js-requirements.json | jq -r '.dependencies | keys[]')This installs only package names (no versions), so each build may pick different axios (and any future packages) depending on the current npm registry state, even though js-requirements.json declares versions.
To make builds deterministic and align with js-requirements.json, consider:
-RUN npm install -g $(cat /usr/src/app/js-requirements.json | jq -r '.dependencies | keys[]')
+RUN npm install -g $(jq -r '.dependencies | to_entries[] | "\(.key)@\(.value)"' /usr/src/app/js-requirements.json)You may also want to revisit whether you need global installs at all versus a dedicated tools directory on PATH, but that’s optional.
Also applies to: 63-71, 88-89
🤖 Prompt for AI Agents
In Dockerfile around lines 51-53 (also applies to 63-71 and 88-89), the RUN that
does npm install -g uses only package names, which makes builds
non-deterministic; change the install step to install packages with explicit
versions derived from js-requirements.json (e.g. use jq to emit "name@version"
for each dependency and pass that list to npm install -g), or alternatively
create a package.json/package-lock.json in a temp directory and run npm ci (or
install into a dedicated tools dir on PATH instead of global) so the exact
versions from js-requirements.json are installed deterministically.
| { | ||
| "dependencies": { | ||
| "axios": "^1.5.0" | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
axios ^1.5.0 pulls in known‑vulnerable versions; please bump and align with Docker build.
Security feeds list multiple issues affecting axios 1.x before recent patches (SSRF, CSRF token leakage, prototype pollution, ReDoS, data: URL DoS), with fixes spread across versions 1.6.0, 1.6.3, 1.6.4, 1.7.4, 1.8.2, and 1.12.0.(meterian.io) A range like ^1.5.0 allows many of the affected versions.
Suggestions:
- Bump to a fully patched series, e.g.
"axios": "^1.12.0"(or whatever your security scanner recommends today). - In the Dockerfile, you currently do
npm install -g $(... | jq -r '.dependencies | keys[]'), which ignores version specifiers. Once you update this file, also update that command to installname@versionpairs so the runtime actually matches these constraints (see Dockerfile comment).
Please run your JS dependency scanner (e.g. npm audit, Snyk, or GitHub Dependabot) against axios to confirm the currently recommended minimum patched version for all open CVEs:
🌐 Web query:
Latest security advisories and recommended fixed versions for the npm package "axios"
💡 Result:
Here are the latest public security advisories for the npm package axios and the recommended fixed versions (sources cited):
-
CVE-2025-27152 — SSRF and credential leakage when passing absolute URLs (impacts <0.30.0 and 1.0.0–1.8.1).
- Fixed in: 0.30.0 and 1.8.2. [1][2]
-
CVE-2025-58754 — Denial‑of‑Service: data: URI responses bypass maxContentLength/maxBodyLength (impacts <0.30.2 and >1.0.0 <1.12.0).
- Fixed in: 0.30.2 and 1.12.0. [3][4]
-
GHSA-rm8p-cx58-hcvx (withdrawn) — transitive critical issue via form-data impacting axios@1.10.0; advisory was withdrawn (users can instead use a patched form-data). Patched form-data / axios guidance referenced in the advisory. [5]
Recommended action: upgrade to the nearest fixed line at or above the versions listed (for example: 0.30.2 or 1.12.0+, or at minimum 1.8.2 for the SSRF fix). Verify your app’s axios major line (0.3x vs 1.x) and test compatibility before deploy.
Sources:
[1] GitLab advisory (CVE-2025-27152).
[2] NVD / GitHub advisory references for CVE-2025-27152.
[3] GitLab advisory (CVE-2025-58754 / release v1.12.0).
[4] ResolvedSecurity summary of CVE-2025-58754.
[5] GitHub Advisory (withdrawn GHSA for form-data / axios@1.10.0).
If you want, I can fetch the exact advisory pages and link each citation directly.
Upgrade axios to ^1.12.0 to resolve known security vulnerabilities; also update Dockerfile to enforce version pinning.
axios ^1.5.0 exposes your runtime to CVE-2025-27152 (SSRF/credential leakage affecting 1.0.0–1.8.1, fixed in 1.8.2) and CVE-2025-58754 (DoS via data: URI bypass affecting all versions >1.0.0 <1.12.0, fixed in 1.12.0). Since your range allows 1.5.0–1.11.x, both remain unpatched.
- Bump to
"axios": "^1.12.0"(or higher as recommended by your security tooling). - In the Dockerfile, update the npm install command from
npm install -g $(... | jq -r '.dependencies | keys[]')to installname@versionpairs so runtime versions match this file's constraints.
🤖 Prompt for AI Agents
In js-requirements.json lines 1–5, bump the axios dependency to a patched
release by changing the version to "^1.12.0" (or newer) to remediate
CVE-2025-27152 and CVE-2025-58754; additionally, update the Dockerfile npm
install step to install explicit name@version pairs from this file (rather than
only package names) so the runtime honors the pinned versions — e.g., generate
installs like "axios@1.12.0" from the dependencies object and use that list in
the npm install command to enforce exact versions at build time.
| @@ -0,0 +1 @@ | |||
| requests==2.31.0 No newline at end of file | |||
There was a problem hiding this comment.
🧩 Analysis chain
requests==2.31.0 has known vulnerabilities; please bump to a patched 2.32.x.
Security advisories now flag 2.31.0 as affected by at least one TLS-verification bypass issue (CVE-2024-35195) when Session is first used with verify=False; fixes are in 2.32.x (e.g. 2.32.2+ / 2.32.4+).(security.snyk.io)
Since this image will execute arbitrary scripts that may make outbound HTTP calls, please upgrade to a patched series (for example requests>=2.32.4,<2.33 or the latest 2.32.x) and run your integration tests against it.
Consider running your dependency scanner (e.g., Snyk, pip-audit, or GitHub Dependabot) to confirm the currently recommended fixed version and check for any additional advisories:
🌐 Web query:
Latest security advisories and recommended fixed versions for the Python package "requests"
💡 Result:
Here are the most recent security advisories for the python "requests" package and the recommended fixed versions:
- CVE-2024-35195 — Session verify=False persists (TLS verification bypass via connection pool). Fixed in requests 2.32.0. [1][2]
- CVE-2024-47081 — URL parsing may leak .netrc credentials to third parties. Fixed in requests 2.32.4. [3][4]
- CVE-2023-32681 — header parsing issue (information leak). Fixed in earlier 2.31.x / distribution updates (see distro advisories). [5]
Recommended action: upgrade to at least requests==2.32.4 (contains the fixes above). If you cannot upgrade immediately, mitigate CVE-2024-47081 by disabling .netrc usage (use Session(trust_env=False)) and mitigate CVE-2024-35195 by avoiding verify=False on shared Sessions / recreating Sessions per request.
Sources:
[1] CVE-2024-35195 details (CVE / NVD / CVE details).
[2] GHSA / requests upstream fix referencing 2.32.0.
[3] IBM security bulletin (CVE-2024-47081) — fixed in 2.32.4.
[4] Debian/Ubuntu advisories noting requests 2.32.4 fixes.
[5] Ubuntu security notice and historical advisories for CVE-2023-32681.
If you want, I can fetch and paste the exact advisory text and upstream GitHub security-advisory links for each CVE.
requests==2.31.0 has multiple known vulnerabilities; upgrade to requests==2.32.4 or later.
CVE-2024-35195 (TLS verification bypass via connection pool when verify=False) is fixed in requests 2.32.0, and CVE-2024-47081 (URL parsing may leak .netrc credentials) is fixed in requests 2.32.4. Since this image executes arbitrary scripts with outbound HTTP calls, upgrade to at least requests==2.32.4 to cover all known advisories (for example requests>=2.32.4,<2.33).
🤖 Prompt for AI Agents
In requirements.txt around lines 1 to 1, the pinned dependency requests==2.31.0
contains known vulnerabilities; update the requirement to at least
requests==2.32.4 (recommended: requests>=2.32.4,<2.33) to include fixes for
CVE-2024-35195 and CVE-2024-47081, then regenerate any lockfiles or dependency
caches and run your test suite/CI to ensure compatibility.
| #BUBBLEWRAP SUPPORT | ||
| USE_BUBBLEWRAP=false | ||
|
|
There was a problem hiding this comment.
Secure‑by‑default: consider enabling USE_BUBBLEWRAP in the default env.
Given the Docker image now installs bubblewrap and configures it for sandboxing, shipping .env.default with USE_BUBBLEWRAP=false means anyone copying this template will run scripts without the intended isolation.
For a feature that executes arbitrary Python/JS/R code, it would be safer to:
- Default
USE_BUBBLEWRAP=truein.env.default, and - Require explicit opt‑out in local/dev overrides if someone can’t or doesn’t want to use bubblewrap.
At minimum, please document clearly that production setups should turn this on.
🤖 Prompt for AI Agents
In server/.env.default around lines 24–26, the USE_BUBBLEWRAP flag is currently
set to false; update the default to USE_BUBBLEWRAP=true to enable sandboxing by
default, and add a short inline comment explaining this default (that bubblewrap
is installed in the Docker image and provides isolation for running untrusted
code, with instructions to set USE_BUBBLEWRAP=false only to explicitly opt out
for local/dev). Also add or update a one‑line note in the project README or env
documentation recommending that production deployments ensure
USE_BUBBLEWRAP=true.
| const languagesWithCodeBlocks = availableLanguages.map(language => { | ||
| const codeWritingBlock = getCodeWritingBlock(language, {}, {}) | ||
|
|
||
| return { | ||
| language, | ||
| codeWritingBlock | ||
| } | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify empty objects passed to getCodeWritingBlock.
The function is called with empty objects {} for what appear to be input and config parameters. Please verify these are appropriate default values.
Run this script to examine the getCodeWritingBlock function signature and usage:
🏁 Script executed:
#!/bin/bash
# Find the getCodeWritingBlock function definition and its usage patterns
echo "=== Searching for getCodeWritingBlock definition ==="
ast-grep --pattern $'function getCodeWritingBlock($$$) {
$$$
}'
echo -e "\n=== Alternative: searching with rg ==="
rg -A 10 'export.*getCodeWritingBlock' --type ts
echo -e "\n=== Searching for other usages of getCodeWritingBlock ==="
rg 'getCodeWritingBlock\(' --type ts -B 1 -A 1Length of output: 1876
Remove explicit empty objects; rely on optional parameters and default operator.
The function signature shows both input and config are optional parameters with defaults. Explicitly passing {} is redundant:
const codeWritingBlock = getCodeWritingBlock(language)Since the function already handles falsy values via || operators, omitting the arguments is the idiomatic approach.
🤖 Prompt for AI Agents
In server/api/script.ts around lines 14 to 21, the call to getCodeWritingBlock
passes explicit empty objects for optional parameters
(getCodeWritingBlock(language, {}, {})); remove the redundant {} arguments and
call getCodeWritingBlock(language) so the function can rely on its optional
params and default/falsy handling.
| allowedImports: [ | ||
| "json", "math", "datetime", "random", "re", "collections", | ||
| "itertools", "functools", "operator", "string", "uuid", | ||
| "hashlib", "base64", "urllib.parse", "statistics", "requests", | ||
| "urllib", "http", "urllib.request", "urllib.error", "http.client", | ||
| "csv","pandas","requests","time" | ||
| ], |
There was a problem hiding this comment.
Remove duplicate "requests" entry from Python allowed imports.
The requests module appears twice in the allowedImports array (once on line 100 and again on line 103).
allowedImports: [
"json", "math", "datetime", "random", "re", "collections",
- "itertools", "functools", "operator", "string", "uuid",
- "hashlib", "base64", "urllib.parse", "statistics", "requests",
+ "itertools", "functools", "operator", "string", "uuid",
+ "hashlib", "base64", "urllib.parse", "statistics",
"urllib", "http", "urllib.request", "urllib.error", "http.client",
"csv","pandas","requests","time"
],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| allowedImports: [ | |
| "json", "math", "datetime", "random", "re", "collections", | |
| "itertools", "functools", "operator", "string", "uuid", | |
| "hashlib", "base64", "urllib.parse", "statistics", "requests", | |
| "urllib", "http", "urllib.request", "urllib.error", "http.client", | |
| "csv","pandas","requests","time" | |
| ], | |
| allowedImports: [ | |
| "json", "math", "datetime", "random", "re", "collections", | |
| "itertools", "functools", "operator", "string", "uuid", | |
| "hashlib", "base64", "urllib.parse", "statistics", | |
| "urllib", "http", "urllib.request", "urllib.error", "http.client", | |
| "csv","pandas","requests","time" | |
| ], |
🤖 Prompt for AI Agents
In server/workflowScriptExecutorTool.ts around lines 98 to 104, the
allowedImports array contains a duplicate "requests" entry; remove the redundant
"requests" string so each module appears only once (or deduplicate the array
programmatically), ensuring the final allowedImports list contains unique module
names.
| allowedImports: [ | ||
| "jsonlite", "base", "stats", "utils", "graphics", "grDevices", | ||
| "methods", "datasets", "stringr", "lubridate", "dplyr", | ||
| "httr", "RCurl", "curl", "readr", "data.table", | ||
| "methods", "datasets", "stringr", "lubridate", "dplyr", | ||
| "httr", "RCurl", "curl","digest","tidyverse","glue","bigrquery", | ||
| "readr","readxl","googlesheets4","stringr","data.table", | ||
| "countrycode","foreach","doParallel","googledrive" | ||
| ], |
There was a problem hiding this comment.
Remove duplicate entries from R allowed imports.
Several packages appear twice: methods, datasets, stringr, lubridate, dplyr.
allowedImports: [
"jsonlite", "base", "stats", "utils", "graphics", "grDevices",
- "methods", "datasets", "stringr", "lubridate", "dplyr",
"httr", "RCurl", "curl", "readr", "data.table",
"methods", "datasets", "stringr", "lubridate", "dplyr",
- "httr", "RCurl", "curl","digest","tidyverse","glue","bigrquery",
- "readr","readxl","googlesheets4","stringr","data.table",
+ "httr", "RCurl", "curl", "digest", "tidyverse", "glue", "bigrquery",
+ "readr", "readxl", "googlesheets4", "data.table",
"countrycode","foreach","doParallel","googledrive"
],Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In server/workflowScriptExecutorTool.ts around lines 178 to 186, the
allowedImports array for R contains duplicate package entries (methods,
datasets, stringr, lubridate, dplyr). Remove the repeated package names so each
package appears only once in the array; ensure the final list preserves required
packages but de-duplicates any repeats and keep consistent formatting (commas
and quotes) after removal.
| return new Promise((resolve) => { | ||
| const child = spawn(command, args, { | ||
| timeout: timeout_value * 60 * 1000, // 1 minute timeout | ||
| killSignal: 'SIGKILL' | ||
| }) |
There was a problem hiding this comment.
Fix the incorrect timeout comment.
The comment says "1 minute timeout" but the actual timeout is 20 minutes (timeout_value * 60 * 1000 where timeout_value = 20).
const child = spawn(command, args, {
- timeout: timeout_value * 60 * 1000, // 1 minute timeout
+ timeout: timeout_value * 60 * 1000, // 20 minute timeout
killSignal: 'SIGKILL'
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return new Promise((resolve) => { | |
| const child = spawn(command, args, { | |
| timeout: timeout_value * 60 * 1000, // 1 minute timeout | |
| killSignal: 'SIGKILL' | |
| }) | |
| return new Promise((resolve) => { | |
| const child = spawn(command, args, { | |
| timeout: timeout_value * 60 * 1000, // 20 minute timeout | |
| killSignal: 'SIGKILL' | |
| }) |
🤖 Prompt for AI Agents
In server/workflowScriptExecutorTool.ts around lines 756 to 760, the inline
comment incorrectly states "1 minute timeout" while the timeout is set to
timeout_value * 60 * 1000 (with timeout_value = 20). Update the comment to
correctly reflect the actual timeout (e.g., "timeout_value minutes (20 minutes)"
or "20 minute timeout" or change expression/comment to match whichever value you
intend) so the comment and code are consistent.
1 participant
Description
script executor suppoprt
Testing
Additional Notes
Summary by CodeRabbit
Release Notes
New Features
Infrastructure
✏️ Tip: You can customize this high-level summary in your review settings.