diff --git a/apps/server/src/lib/worktree-metadata.ts b/apps/server/src/lib/worktree-metadata.ts index b8796fe4..199f61c9 100644 --- a/apps/server/src/lib/worktree-metadata.ts +++ b/apps/server/src/lib/worktree-metadata.ts @@ -20,12 +20,38 @@ export interface WorktreeMetadata { pr?: WorktreePRInfo; } +/** + * Sanitize branch name for cross-platform filesystem safety + */ +function sanitizeBranchName(branch: string): string { + // Replace characters that are invalid or problematic on various filesystems: + // - Forward and backslashes (path separators) + // - Windows invalid chars: : * ? " < > | + // - Other potentially problematic chars + let safeBranch = branch + .replace(/[/\\:*?"<>|]/g, "-") // Replace invalid chars with dash + .replace(/\s+/g, "_") // Replace spaces with underscores + .replace(/\.+$/g, "") // Remove trailing dots (Windows issue) + .replace(/-+/g, "-") // Collapse multiple dashes + .replace(/^-|-$/g, ""); // Remove leading/trailing dashes + + // Truncate to safe length (leave room for path components) + safeBranch = safeBranch.substring(0, 200); + + // Handle Windows reserved names (CON, PRN, AUX, NUL, COM1-9, LPT1-9) + const windowsReserved = /^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])$/i; + if (windowsReserved.test(safeBranch) || safeBranch.length === 0) { + safeBranch = `_${safeBranch || "branch"}`; + } + + return safeBranch; +} + /** * Get the path to the worktree metadata directory */ function getWorktreeMetadataDir(projectPath: string, branch: string): string { - // Sanitize branch name for filesystem (replace / with -) - const safeBranch = branch.replace(/\//g, "-"); + const safeBranch = sanitizeBranchName(branch); return path.join(projectPath, ".automaker", "worktrees", safeBranch); } diff --git a/apps/server/src/routes/worktree/routes/create-pr.ts b/apps/server/src/routes/worktree/routes/create-pr.ts index d55ef0c3..d2e22535 100644 --- a/apps/server/src/routes/worktree/routes/create-pr.ts +++ b/apps/server/src/routes/worktree/routes/create-pr.ts @@ -8,6 +8,24 @@ import { promisify } from "util"; import { getErrorMessage, logError } from "../common.js"; import { updateWorktreePRInfo } from "../../../lib/worktree-metadata.js"; +// Shell escaping utility to prevent command injection +function shellEscape(arg: string): string { + if (process.platform === "win32") { + // Windows CMD shell escaping + return `"${arg.replace(/"/g, '""')}"`; + } else { + // Unix shell escaping + return `'${arg.replace(/'/g, "'\\''")}'`; + } +} + +// Validate branch name to prevent command injection +function isValidBranchName(name: string): boolean { + // Git branch names cannot contain: space, ~, ^, :, ?, *, [, \, or control chars + // Also reject shell metacharacters for safety + return /^[a-zA-Z0-9._\-/]+$/.test(name) && name.length < 250; +} + const execAsync = promisify(exec); // Extended PATH to include common tool installation locations @@ -78,6 +96,15 @@ export function createCreatePRHandler() { ); const branchName = branchOutput.trim(); + // Validate branch name for security + if (!isValidBranchName(branchName)) { + res.status(400).json({ + success: false, + error: "Invalid branch name contains unsafe characters", + }); + return; + } + // Check for uncommitted changes const { stdout: status } = await execAsync("git status --porcelain", { cwd: worktreePath, diff --git a/apps/server/src/routes/worktree/routes/pr-info.ts b/apps/server/src/routes/worktree/routes/pr-info.ts index aa270466..179266be 100644 --- a/apps/server/src/routes/worktree/routes/pr-info.ts +++ b/apps/server/src/routes/worktree/routes/pr-info.ts @@ -42,6 +42,15 @@ const execEnv = { PATH: extendedPath, }; +/** + * Validate branch name to prevent command injection. + * Git branch names cannot contain: space, ~, ^, :, ?, *, [, \, or control chars. + * We also reject shell metacharacters for safety. + */ +function isValidBranchName(name: string): boolean { + return /^[a-zA-Z0-9._\-/]+$/.test(name) && name.length < 250; +} + export interface PRComment { id: number; author: string; @@ -79,6 +88,15 @@ export function createPRInfoHandler() { return; } + // Validate branch name to prevent command injection + if (!isValidBranchName(branchName)) { + res.status(400).json({ + success: false, + error: "Invalid branch name contains unsafe characters", + }); + return; + } + // Check if gh CLI is available let ghCliAvailable = false; try { @@ -226,35 +244,38 @@ export function createPRInfoHandler() { // Get review comments (inline code comments) let reviewComments: PRComment[] = []; - try { - const reviewsEndpoint = targetRepo - ? `repos/${targetRepo}/pulls/${prNumber}/comments` - : `repos/{owner}/{repo}/pulls/${prNumber}/comments`; - const reviewsCmd = `gh api ${reviewsEndpoint}`; - const { stdout: reviewsOutput } = await execAsync( - reviewsCmd, - { cwd: worktreePath, env: execEnv } - ); - const reviewsData = JSON.parse(reviewsOutput); - reviewComments = reviewsData.map((c: { - id: number; - user: { login: string }; - body: string; - path: string; - line?: number; - original_line?: number; - created_at: string; - }) => ({ - id: c.id, - author: c.user?.login || "unknown", - body: c.body, - path: c.path, - line: c.line || c.original_line, - createdAt: c.created_at, - isReviewComment: true, - })); - } catch (error) { - console.warn("[PRInfo] Failed to fetch review comments:", error); + // Only fetch review comments if we have repository info + if (targetRepo) { + try { + const reviewsEndpoint = `repos/${targetRepo}/pulls/${prNumber}/comments`; + const reviewsCmd = `gh api ${reviewsEndpoint}`; + const { stdout: reviewsOutput } = await execAsync( + reviewsCmd, + { cwd: worktreePath, env: execEnv } + ); + const reviewsData = JSON.parse(reviewsOutput); + reviewComments = reviewsData.map((c: { + id: number; + user: { login: string }; + body: string; + path: string; + line?: number; + original_line?: number; + created_at: string; + }) => ({ + id: c.id, + author: c.user?.login || "unknown", + body: c.body, + path: c.path, + line: c.line || c.original_line, + createdAt: c.created_at, + isReviewComment: true, + })); + } catch (error) { + console.warn("[PRInfo] Failed to fetch review comments:", error); + } + } else { + console.warn("[PRInfo] Cannot fetch review comments: repository info not available"); } const prInfo: PRInfo = { diff --git a/apps/ui/src/components/views/board-view.tsx b/apps/ui/src/components/views/board-view.tsx index add0693c..89540726 100644 --- a/apps/ui/src/components/views/board-view.tsx +++ b/apps/ui/src/components/views/board-view.tsx @@ -421,69 +421,21 @@ export function BoardView() { // Handler for addressing PR comments - creates a feature and starts it automatically const handleAddressPRComments = useCallback( async (worktree: WorktreeInfo, prInfo: PRInfo) => { - // If comments are empty, fetch them from GitHub - let fullPRInfo = prInfo; - if (prInfo.comments.length === 0 && prInfo.reviewComments.length === 0) { - try { - const api = getElectronAPI(); - if (api?.worktree?.getPRInfo) { - const result = await api.worktree.getPRInfo( - worktree.path, - worktree.branch - ); - if ( - result.success && - result.result?.hasPR && - result.result.prInfo - ) { - fullPRInfo = result.result.prInfo; - } - } - } catch (error) { - console.error("[Board] Failed to fetch PR comments:", error); - } - } - - // Format PR comments into a feature description - const allComments = [ - ...fullPRInfo.comments.map((c) => ({ - ...c, - type: "comment" as const, - })), - ...fullPRInfo.reviewComments.map((c) => ({ - ...c, - type: "review" as const, - })), - ].sort( - (a, b) => - new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime() - ); - - let description = `Address PR #${fullPRInfo.number} feedback: "${fullPRInfo.title}"\n\n`; - description += `PR URL: ${fullPRInfo.url}\n\n`; - - if (allComments.length === 0) { - description += `No comments found on this PR yet. Check the PR for any new feedback.\n`; - } else { - description += `## Feedback to address:\n\n`; - for (const comment of allComments) { - if (comment.type === "review" && comment.path) { - description += `### ${comment.path}${comment.line ? `:${comment.line}` : ""}\n`; - } - description += `**@${comment.author}:**\n${comment.body}\n\n`; - } - } + // Use a simple prompt that instructs the agent to read and address PR feedback + // The agent will fetch the PR comments directly, which is more reliable and up-to-date + const prNumber = prInfo.number; + const description = `Read the review requests on PR #${prNumber} and address any feedback the best you can.`; // Create the feature const featureData = { category: "PR Review", - description: description.trim(), + description, steps: [], images: [], imagePaths: [], skipTests: defaultSkipTests, - model: "sonnet" as const, - thinkingLevel: "medium" as const, + model: "opus" as const, + thinkingLevel: "none" as const, branchName: worktree.branch, priority: 1, // High priority for PR feedback planningMode: "skip" as const, @@ -500,7 +452,7 @@ export function BoardView() { (f) => f.branchName === worktree.branch && f.status === "backlog" && - f.description.includes(`PR #${fullPRInfo.number}`) + f.description.includes(`PR #${prNumber}`) ); if (newFeature) { @@ -1255,12 +1207,19 @@ export function BoardView() { // If a PR was created and we have the worktree branch, update all features on that branch with the PR URL if (prUrl && selectedWorktreeForAction?.branch) { const branchName = selectedWorktreeForAction.branch; - hookFeatures - .filter((f) => f.branchName === branchName) - .forEach((feature) => { - updateFeature(feature.id, { prUrl }); - persistFeatureUpdate(feature.id, { prUrl }); - }); + const featuresToUpdate = hookFeatures.filter((f) => f.branchName === branchName); + + // Update local state synchronously + featuresToUpdate.forEach((feature) => { + updateFeature(feature.id, { prUrl }); + }); + + // Persist changes asynchronously and in parallel + Promise.all( + featuresToUpdate.map((feature) => + persistFeatureUpdate(feature.id, { prUrl }) + ) + ).catch(console.error); } setWorktreeRefreshKey((k) => k + 1); setSelectedWorktreeForAction(null); diff --git a/apps/ui/src/components/views/board-view/components/kanban-card.tsx b/apps/ui/src/components/views/board-view/components/kanban-card.tsx index bf2ad33d..96ccb618 100644 --- a/apps/ui/src/components/views/board-view/components/kanban-card.tsx +++ b/apps/ui/src/components/views/board-view/components/kanban-card.tsx @@ -699,24 +699,30 @@ export const KanbanCard = memo(function KanbanCard({ )} {/* PR URL Display */} - {feature.prUrl && ( -
- e.stopPropagation()} - onPointerDown={(e) => e.stopPropagation()} - className="inline-flex items-center gap-1.5 text-[11px] text-purple-500 hover:text-purple-400 transition-colors" - title={feature.prUrl} - data-testid={`pr-url-${feature.id}`} - > - - Pull Request - - -
- )} + {typeof feature.prUrl === "string" && + /^https?:\/\//i.test(feature.prUrl) && (() => { + const prNumber = feature.prUrl.split('/').pop(); + return ( +
+ e.stopPropagation()} + onPointerDown={(e) => e.stopPropagation()} + className="inline-flex items-center gap-1.5 text-[11px] text-purple-500 hover:text-purple-400 transition-colors" + title={feature.prUrl} + data-testid={`pr-url-${feature.id}`} + > + + + {prNumber ? `Pull Request #${prNumber}` : 'Pull Request'} + + + +
+ ); + })()} {/* Steps Preview */} {showSteps && feature.steps && feature.steps.length > 0 && ( diff --git a/apps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsx index 17d66374..71d7bdf1 100644 --- a/apps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/create-pr-dialog.tsx @@ -29,12 +29,8 @@ interface CreatePRDialogProps { open: boolean; onOpenChange: (open: boolean) => void; worktree: WorktreeInfo | null; -<<<<<<< Updated upstream - onCreated: (prUrl?: string) => void; -======= projectPath: string | null; - onCreated: () => void; ->>>>>>> Stashed changes + onCreated: (prUrl?: string) => void; } export function CreatePRDialog({ diff --git a/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx b/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx index 492122ab..b3134191 100644 --- a/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx +++ b/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx @@ -1,7 +1,6 @@ import { Button } from "@/components/ui/button"; -<<<<<<< Updated upstream -import { RefreshCw, Globe, Loader2, CircleDot } from "lucide-react"; +import { RefreshCw, Globe, Loader2, CircleDot, GitPullRequest } from "lucide-react"; import { cn } from "@/lib/utils"; import { Tooltip, @@ -9,12 +8,7 @@ import { TooltipProvider, TooltipTrigger, } from "@/components/ui/tooltip"; -import type { WorktreeInfo, BranchInfo, DevServerInfo } from "../types"; -======= -import { RefreshCw, Globe, Loader2, GitPullRequest } from "lucide-react"; -import { cn } from "@/lib/utils"; import type { WorktreeInfo, BranchInfo, DevServerInfo, PRInfo } from "../types"; ->>>>>>> Stashed changes import { BranchSwitchDropdown } from "./branch-switch-dropdown"; import { WorktreeActionsDropdown } from "./worktree-actions-dropdown"; @@ -95,7 +89,6 @@ export function WorktreeTab({ onStopDevServer, onOpenDevServerUrl, }: WorktreeTabProps) { -<<<<<<< Updated upstream // Determine border color based on state: // - Running features: cyan border (high visibility, indicates active work) // - Uncommitted changes: amber border (warning state, needs attention) @@ -111,7 +104,7 @@ export function WorktreeTab({ }; const borderClasses = getBorderClasses(); -======= + let prBadge: JSX.Element | null = null; if (worktree.pr) { const prState = worktree.pr.state?.toLowerCase() ?? "open"; @@ -197,7 +190,6 @@ export function WorktreeTab({ ); } ->>>>>>> Stashed changes return (
@@ -225,7 +217,6 @@ export function WorktreeTab({ {cardCount} )} -<<<<<<< Updated upstream {hasChanges && ( @@ -246,9 +237,7 @@ export function WorktreeTab({ )} -======= {prBadge} ->>>>>>> Stashed changes )} -<<<<<<< Updated upstream {hasChanges && ( @@ -313,9 +301,7 @@ export function WorktreeTab({ )} -======= {prBadge} ->>>>>>> Stashed changes )}