Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion emain/preload.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2025, Command Line Inc.
// SPDX-License-Identifier: Apache-2.0

import { contextBridge, ipcRenderer, Rectangle, WebviewTag } from "electron";
import { contextBridge, ipcRenderer, Rectangle, webUtils, WebviewTag } from "electron";

// update type in custom.d.ts (ElectronApi type)
contextBridge.exposeInMainWorld("api", {
Expand Down Expand Up @@ -68,6 +68,7 @@ contextBridge.exposeInMainWorld("api", {
openBuilder: (appId?: string) => ipcRenderer.send("open-builder", appId),
setBuilderWindowAppId: (appId: string) => ipcRenderer.send("set-builder-window-appid", appId),
doRefresh: () => ipcRenderer.send("do-refresh"),
getPathForFile: (file: File) => webUtils.getPathForFile(file),
});

// Custom event for "new-window"
Expand Down
74 changes: 72 additions & 2 deletions frontend/app/view/term/term.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { waveEventSubscribe } from "@/app/store/wps";
import { RpcApi } from "@/app/store/wshclientapi";
import { TabRpcClient } from "@/app/store/wshrpcutil";
import type { TermViewModel } from "@/app/view/term/term-model";
import { atoms, getOverrideConfigAtom, getSettingsPrefixAtom, globalStore, WOS } from "@/store/global";
import { atoms, getApi, getOverrideConfigAtom, getSettingsPrefixAtom, globalStore, WOS } from "@/store/global";
import { fireAndForget, useAtomValueSafe } from "@/util/util";
import { computeBgStyleFromMeta } from "@/util/waveutil";
import { ISearchOptions } from "@xterm/addon-search";
Expand Down Expand Up @@ -353,8 +353,78 @@ const TerminalView = ({ blockId, model }: ViewComponentProps<TermViewModel>) =>

const termBg = computeBgStyleFromMeta(blockData?.meta);

// Handle drag and drop
const handleDragOver = React.useCallback((e: React.DragEvent) => {
e.preventDefault();
e.stopPropagation();
// Indicate that we can accept the drop
e.dataTransfer.dropEffect = "copy";
}, []);

const handleDrop = React.useCallback((e: React.DragEvent) => {
e.preventDefault();
e.stopPropagation();

// Get files from the drop
const files = Array.from(e.dataTransfer.files);
if (files.length === 0) {
return;
}

console.log("Drop files:", files);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

// Get the file path(s) using the Electron API
const paths = files.map((file: File) => {
try {
// Use the exposed Electron API to get the full path
const fullPath = getApi().getPathForFile(file);
console.log("File:", file.name, "-> Full path:", fullPath);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Await getPathForFile before building drop input

Electron’s webUtils.getPathForFile returns a Promise, but the drop handler treats it as a synchronous string (getApi().getPathForFile(file) and later path.includes(...)). When a user drops a file, paths becomes an array of Promises, so the subsequent .includes/join logic throws and no path is sent to the terminal. Resolve the promise (e.g., with await/Promise.all) before quoting and sending the path string.

Useful? React with 👍 / 👎.

return fullPath;
} catch (err) {
console.error("Could not get path for file:", file.name, err);
return file.name;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
});

console.log("Paths to insert:", paths);

// Insert the path(s) into the terminal
// If multiple files, separate with spaces and quote if necessary
const pathString = paths.map(path => {
// Quote paths that contain spaces
if (path.includes(" ")) {
return `"${path}"`;
}
return path;
}).join(" ");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Path quoting is incomplete and may cause command injection risks.

The current implementation only quotes paths containing spaces, but shell commands require escaping many other special characters such as:

  • Single quotes (')
  • Double quotes already in the path (")
  • Dollar signs ($)
  • Backticks (`)
  • Backslashes (\)
  • Ampersands (&)
  • Semicolons (;)
  • Pipes (|)

Without proper escaping, a malicious filename could inject shell commands. Consider implementing proper shell escaping or using a library for this purpose.

🔎 Example of improved shell escaping

For POSIX shells, you could use single-quote escaping where internal single quotes are replaced with '\'':

         // Insert the path(s) into the terminal
         // If multiple files, separate with spaces and quote if necessary
         const pathString = paths.map(path => {
-            // Quote paths that contain spaces
-            if (path.includes(" ")) {
-                return `"${path}"`;
-            }
-            return path;
+            // Escape for POSIX shells: wrap in single quotes and escape any single quotes
+            return "'" + path.replace(/'/g, "'\\''") + "'";
         }).join(" ");

Note: This assumes a POSIX-compatible shell. For Windows Command Prompt or PowerShell, different escaping rules apply. You may need to detect the shell type or use a cross-platform escaping library.

📝 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.

Suggested change
const pathString = paths.map(path => {
// Quote paths that contain spaces
if (path.includes(" ")) {
return `"${path}"`;
}
return path;
}).join(" ");
const pathString = paths.map(path => {
// Escape for POSIX shells: wrap in single quotes and escape any single quotes
return "'" + path.replace(/'/g, "'\\''") + "'";
}).join(" ");
🤖 Prompt for AI Agents
In frontend/app/view/term/term.tsx around lines 393 to 399, the current logic
only quotes paths with spaces which leaves many shell-special characters
unescaped and risks command injection; replace this ad-hoc quoting with a safe
approach: either (A) stop concatenating into a shell string and pass paths as an
argv array to the child process API (preferred), or (B) if you must build a
shell string for a POSIX shell, wrap every path in single quotes and escape
internal single quotes by replacing ' with '\'' before joining, and for Windows
choose a platform-appropriate escaping library; ideally use a well-tested
cross-platform escaping library instead of hand-rolled logic.


console.log("Final path string:", pathString);

// Send the path to the terminal
if (model.termRef.current && pathString) {
model.sendDataToController(pathString);
}
}, [model]);

const handleDragEnter = React.useCallback((e: React.DragEvent) => {
e.preventDefault();
e.stopPropagation();
}, []);

const handleDragLeave = React.useCallback((e: React.DragEvent) => {
e.preventDefault();
e.stopPropagation();
}, []);

return (
<div className={clsx("view-term", "term-mode-" + termMode)} ref={viewRef}>
<div
className={clsx("view-term", "term-mode-" + termMode)}
ref={viewRef}
onDragOver={handleDragOver}
onDrop={handleDrop}
onDragEnter={handleDragEnter}
onDragLeave={handleDragLeave}
>
{termBg && <div className="absolute inset-0 z-0 pointer-events-none" style={termBg} />}
<TermResyncHandler blockId={blockId} model={model} />
<TermThemeUpdater blockId={blockId} model={model} termRef={model.termRef} />
Expand Down
1 change: 1 addition & 0 deletions frontend/types/custom.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ declare global {
openBuilder: (appId?: string) => void; // open-builder
setBuilderWindowAppId: (appId: string) => void; // set-builder-window-appid
doRefresh: () => void; // do-refresh
getPathForFile: (file: File) => string; // get-path-for-file
};

type ElectronContextMenuItem = {
Expand Down