-
Notifications
You must be signed in to change notification settings - Fork 900
stuff #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
stuff #53
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,7 +2,7 @@ import type { RouterOutputs } from "@superset/api"; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Play } from "lucide-react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type React from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { Workspace } from "shared/types"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { Tab, Workspace } from "shared/types"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type Task = RouterOutputs["task"]["all"][number]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -12,7 +12,7 @@ interface TaskCardProps { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentWorkspace: Workspace | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| selectedWorktreeId: string | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onTabSelect: (worktreeId: string, tabId: string) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onReload: () => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onTabCreated: (worktreeId: string, tab: Tab) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onUpdateTask: ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| taskId: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| updates: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -41,7 +41,7 @@ export const TaskCard: React.FC<TaskCardProps> = ({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentWorkspace, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| selectedWorktreeId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onTabSelect, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onReload, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onTabCreated, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onUpdateTask, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const statusColor = statusColors[task.status] || "bg-neutral-500"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -79,33 +79,35 @@ export const TaskCard: React.FC<TaskCardProps> = ({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Create a new terminal with claude command | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const taskPrompt = `${task.title}\n\n${task.description || ""}`.trim(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Escape quotes and newlines for shell command | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const escapedPrompt = taskPrompt | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .replace(/\\/g, '\\\\') // Escape backslashes first | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .replace(/"/g, '\\"') // Escape double quotes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .replace(/\n/g, '\\n'); // Escape newlines | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await window.ipcRenderer.invoke("tab-create", { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workspaceId: currentWorkspace.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| worktreeId: targetWorktreeId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: `Task: ${task.slug}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: "terminal", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| command: `claude "hi"`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| command: `claude "${escapedPrompt}"`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+82
to
+93
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Incomplete shell escaping enables command injection. The current escaping logic only handles backslashes, double quotes, and newlines. It's missing critical shell metacharacters that are interpreted within double quotes:
If a task title or description contains these characters, they will be executed in the shell context, allowing arbitrary code execution. Recommended fix: Use a safer approach by passing the prompt as a separate argument or using proper shell escaping: - try {
- // Create a new terminal with claude command
- const taskPrompt = `${task.title}\n\n${task.description || ""}`.trim();
- // Escape quotes and newlines for shell command
- const escapedPrompt = taskPrompt
- .replace(/\\/g, '\\\\') // Escape backslashes first
- .replace(/"/g, '\\"') // Escape double quotes
- .replace(/\n/g, '\\n'); // Escape newlines
+ try {
+ // Create a new terminal with claude command
+ const taskPrompt = `${task.title}\n\n${task.description || ""}`.trim();
+ // Escape for shell: backslashes, quotes, dollar signs, backticks, and newlines
+ const escapedPrompt = taskPrompt
+ .replace(/\\/g, '\\\\') // Escape backslashes first
+ .replace(/"/g, '\\"') // Escape double quotes
+ .replace(/\$/g, '\\$') // Escape dollar signs
+ .replace(/`/g, '\\`') // Escape backticks
+ .replace(/!/g, '\\!') // Escape exclamation marks
+ .replace(/\n/g, '\\n'); // Escape newlines
const result = await window.ipcRenderer.invoke("tab-create", {Better alternative: If the IPC handler supports it, consider passing the prompt as a separate parameter rather than embedding it in the command string, avoiding shell escaping entirely. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (result.success) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (result.success && result.tab) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const newTabId = result.tab.id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Update task status to planning (pending) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onUpdateTask(task.id, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| title: task.title, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: task.description || "", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| status: "planning", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Reload workspace to get updated tab data | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await onReload(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Select the new tab after reload | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const newTabId = result.tab?.id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (newTabId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Small delay to ensure workspace is reloaded | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setTimeout(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onTabSelect(targetWorktreeId, newTabId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, 100); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Optimistically add the tab to state | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onTabCreated(targetWorktreeId, result.tab); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Select the new tab immediately | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onTabSelect(targetWorktreeId, newTabId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error("Error starting task:", error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify command sanitization in tmux-manager to prevent injection.
The
tab.commandstring is passed directly totmuxManager.create()without validation or sanitization in this layer. Based on the related changes in TaskCard.tsx (which constructs commands using incomplete shell escaping), there is a potential command injection vulnerability if:Run the following script to examine how tmux-manager handles the command parameter:
If tmux-manager passes the command to a shell without additional escaping, you must either:
tab.commandhere before passing it🏁 Script executed:
Length of output: 78
🏁 Script executed:
Length of output: 2356
🏁 Script executed:
Length of output: 3782
🏁 Script executed:
Length of output: 670
🏁 Script executed:
Length of output: 510
🏁 Script executed:
Length of output: 1963
🏁 Script executed:
Length of output: 99
🏁 Script executed:
Length of output: 1143
Verify command sanitization in tmux-manager to prevent injection — VULNERABILITY CONFIRMED.
The
tab.commandstring is passed directly totmuxManager.create()without validation or sanitization. The underlying vulnerability stems from incomplete shell escaping in TaskCard.tsx (lines 84-87), which only escapes backslashes, double quotes, and newlines but omits critical shell metacharacters like$, backticks (`),!, and command substitution syntax.Execution path:
Risks:
$(command)or`command`bypasses current escaping and executes arbitrary codeRequired fix: Either properly escape all shell metacharacters using a robust method (e.g., shell escaping library or printf %q equivalent) or refactor to pass commands as argument arrays rather than shell strings.
🤖 Prompt for AI Agents