19 mirror parent databases and respect their id scope#32
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a "mirror" system that allows users to write and execute TypeScript code to automate database operations. The implementation includes a new Mirror database model with customizable auto-run scheduling, a Monaco-based code editor in the frontend, and backend execution using Node's vm module with esbuild compilation. Additionally, a customId field was added to the Container model to support custom identification in mirror operations.
Key Changes
- Added Mirror model to database schema with auto-run capabilities and execution tracking
- Implemented mirror execution service using vm.Script to run user-provided TypeScript code
- Added scheduler service (@nestjs/schedule) for automatic mirror execution at configurable intervals
- Created Monaco Editor integration in frontend for TypeScript code editing with type definitions
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/generics/prisma/schema.prisma | Added Mirror model and customId field to Container |
| packages/generics/prisma/migrations/20251211141209_init/migration.sql | Database migration for Mirror table and Container.customId |
| apps/api/src/modules/mirror/mirror.service.ts | Core service for mirror execution using vm.Script and esbuild |
| apps/api/src/modules/mirror/mirror.cron.ts | Cron scheduler for automatic mirror execution |
| apps/webapp/src/routes/mirror/MirrorPage.tsx | Main UI for mirror management |
| apps/webapp/src/routes/mirror/CodeEditor.tsx | Monaco Editor integration for code editing |
| pnpm-lock.yaml | Added dependencies: esbuild@0.27.1, monaco-editor@0.55.1, @nestjs/schedule@6.1.0 |
| package.json | Updated packageManager to pnpm@10.25.0 |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| private async getExecutionInstance(compiledCode: string, cb: Callback, dataUtils?: DataUtils): Promise<MirrorExecutor> { | ||
| const script = new vm.Script(compiledCode, { filename: 'mirror.js' }); | ||
| const context = vm.createContext({ | ||
| module: { exports: {} }, | ||
| exports: {}, | ||
| fetch, | ||
| dataUtils, | ||
| Math, | ||
| cb, | ||
| }); | ||
| script.runInContext(context); | ||
|
|
||
| const MirrorClass = context.module.exports.MyMirror; | ||
| return new MirrorClass(); | ||
| } |
There was a problem hiding this comment.
Executing user-provided code with vm.Script creates a significant security risk. The current sandbox is insufficient - it provides access to the fetch API which could be used to exfiltrate data or make unauthorized requests. Consider implementing more robust sandboxing using isolated-vm or similar libraries, and implement strict timeout mechanisms to prevent infinite loops or resource exhaustion.
| async deleteMirrors(ids: number[]) { | ||
| for (const id of ids) { | ||
| const mirror = await this.prisma.mirror.findUnique({ where: { id } }); | ||
| if (!mirror) continue; | ||
|
|
||
| if (mirror.code.trim().length != 0) { | ||
| const compiled = await this.compile(mirror.code); | ||
| const cb = (msg: string) => { | ||
| this.logger.log(`[MIRROR DELETE ${id}]: ${msg}`); | ||
| }; | ||
| const instance = await this.getExecutionInstance(compiled, cb, undefined); | ||
| await instance.delete(cb); | ||
| } | ||
|
|
||
| await this.prisma.mirror.delete({ where: { id } }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The deleteMirrors method silently continues if a mirror is not found or if code execution fails. This makes debugging difficult and could hide issues. Consider logging when mirrors are not found and properly handling errors during the delete callback execution.
| @Cron('* * * * *') // Runs every minute | ||
| async tick() { | ||
| const mirrors = await this.prisma.mirror.findMany({ where: { autoRun: true, NOT: { autoRunInterval: null } } }); | ||
|
|
||
| const now = Date.now(); | ||
| for (const mirror of mirrors) { | ||
| const msPassed = now - (mirror.lastRun?.getTime() ?? 0); | ||
| if (msPassed >= mirror.autoRunInterval! * 60 * 1000) { | ||
| // Execute mirror logic here | ||
| this.logger.log(`Executing mirror ${mirror.name} (ID: ${mirror.id})`); | ||
| const res = await this.mirrorService.runMirrorCode('run', mirror.id, (msg: string) => { | ||
| this.logger.debug(`Mirror ${mirror.name} says: ${msg}`); | ||
| }); | ||
|
|
||
| if (res != 'OK') this.logger.warn(`Mirror ${mirror.name} execution returned unexpected result: ${res}`); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The cron job runs every minute without any locking mechanism, which could lead to concurrent executions of the same mirror if a run takes longer than the interval. This could cause race conditions and duplicate processing. Implement a locking mechanism to prevent multiple simultaneous executions of the same mirror.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| let ran = false; | ||
|
|
||
| export const CodeEditor = ({ code, setCode }: { code: string; setCode: (code: string) => void }) => { | ||
| const theme = useTheme(); | ||
|
|
||
| const editorRef = useRef<HTMLDivElement | null>(null); | ||
| const monacoInstanceRef = useRef<monaco.editor.IStandaloneCodeEditor | null>(null); | ||
|
|
||
| const currentTheme = theme.theme === 'dark' ? 'vs-dark' : 'vs-light'; | ||
|
|
||
| useEffect(() => { | ||
| const run = async () => { | ||
| if (!editorRef.current || ran) return; | ||
| ran = true; | ||
|
|
||
| // load this from real file that is also accessible to backend | ||
| const res = await fetch('/mirror-types.txt'); | ||
|
|
||
| const text = await res.text(); | ||
| const globalText = text | ||
| .replace(/export declare /g, 'declare ') | ||
| .replace(/export type /g, 'type ') | ||
| .replace(/import .* from .*/g, ''); // Imports entfernen | ||
| monaco.typescript.typescriptDefaults.addExtraLib(globalText, 'file:///node_modules/@sigauth/generics/mirror.d.ts'); | ||
| self.MonacoEnvironment = { | ||
| getWorkerUrl: function (_, label) { | ||
| if (label === 'typescript' || label === 'javascript') { | ||
| return tsWorkerUrl; | ||
| } | ||
| return editorWorkerUrl; | ||
| }, | ||
| }; | ||
|
|
||
| const editor = monaco.editor.create(editorRef.current, { | ||
| value: | ||
| code.length > 0 | ||
| ? code | ||
| : `export class MyMirror extends MirrorExecutor { | ||
| async init(cb: Callback) { | ||
| // Your mirror initialization code here | ||
| // This function is called once when the mirror is created | ||
| } | ||
|
|
||
| async run(mirror: number, cb: Callback, dataUtils: DataUtils) { | ||
| // Your mirror logic here | ||
| } | ||
|
|
||
| async delete(cb: Callback) { | ||
| // Your mirror cleanup code here | ||
| // This function is called when the mirror is deleted | ||
| } | ||
| }`, | ||
| language: 'typescript', | ||
| theme: currentTheme, | ||
| automaticLayout: true, | ||
| }); | ||
| await new Promise(resolve => setTimeout(resolve, 100)); // wait a bit to ensure the editor is ready | ||
| editor.getAction('editor.action.formatDocument').run(editor.getModel()!.uri); | ||
|
|
||
| editor.onDidChangeModelContent(_ => { | ||
| setCode(editor.getValue()); | ||
| }); | ||
| monacoInstanceRef.current = editor; | ||
| ran = false; |
There was a problem hiding this comment.
Using a global ran variable to control editor initialization is not reliable in React's Strict Mode or when multiple editors are mounted. This could prevent the editor from initializing correctly. Use a ref or proper state management instead.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Added isRunning field to Mirror schema to track execution state - Updated cron job to check isRunning before starting execution - Set isRunning to true before execution, false after (with try-finally) - Prevents race conditions when mirror execution takes longer than interval Co-authored-by: Sigabiel <56195110+Sigabiel@users.noreply.github.com>
- Reverted Prisma schema changes (removed isRunning field) - Removed database migration for isRunning field - Added private Set<number> to track running mirrors in RAM - Check Set before execution, add ID when starting, remove when done - Execute mirrors asynchronously with finally block to ensure cleanup Co-authored-by: Sigabiel <56195110+Sigabiel@users.noreply.github.com>
- Added try-catch block in executeMirror for graceful error handling - Added void keyword to explicitly mark fire-and-forget promise behavior - Improved error logging with proper message extraction Co-authored-by: Sigabiel <56195110+Sigabiel@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add in-memory locking to prevent concurrent mirror executions
Implemented mirror and code editor feature