diff --git a/apps/server/src/routes/worktree/index.ts b/apps/server/src/routes/worktree/index.ts index 992a7b48..a7df37bb 100644 --- a/apps/server/src/routes/worktree/index.ts +++ b/apps/server/src/routes/worktree/index.ts @@ -101,7 +101,12 @@ export function createWorktreeRoutes( requireValidWorktree, createPullHandler() ); - router.post('/checkout-branch', requireValidWorktree, createCheckoutBranchHandler()); + router.post( + '/checkout-branch', + validatePathParams('worktreePath'), + requireValidWorktree, + createCheckoutBranchHandler() + ); router.post( '/list-branches', validatePathParams('worktreePath'), diff --git a/apps/server/src/routes/worktree/routes/checkout-branch.ts b/apps/server/src/routes/worktree/routes/checkout-branch.ts index 7ffee2c0..23963480 100644 --- a/apps/server/src/routes/worktree/routes/checkout-branch.ts +++ b/apps/server/src/routes/worktree/routes/checkout-branch.ts @@ -2,15 +2,15 @@ * POST /checkout-branch endpoint - Create and checkout a new branch * * Note: Git repository validation (isGitRepo, hasCommits) is handled by - * the requireValidWorktree middleware in index.ts + * the requireValidWorktree middleware in index.ts. + * Path validation (ALLOWED_ROOT_DIRECTORY) is handled by validatePathParams + * middleware in index.ts. */ import type { Request, Response } from 'express'; -import { exec } from 'child_process'; -import { promisify } from 'util'; -import { getErrorMessage, logError } from '../common.js'; - -const execAsync = promisify(exec); +import path from 'path'; +import { stat } from 'fs/promises'; +import { getErrorMessage, logError, isValidBranchName, execGitCommand } from '../common.js'; export function createCheckoutBranchHandler() { return async (req: Request, res: Response): Promise => { @@ -36,27 +36,47 @@ export function createCheckoutBranchHandler() { return; } - // Validate branch name (basic validation) - const invalidChars = /[\s~^:?*[\\]/; - if (invalidChars.test(branchName)) { + // Validate branch name using shared allowlist: /^[a-zA-Z0-9._\-/]+$/ + if (!isValidBranchName(branchName)) { res.status(400).json({ success: false, - error: 'Branch name contains invalid characters', + error: + 'Invalid branch name. Must contain only letters, numbers, dots, dashes, underscores, or slashes.', }); return; } - // Get current branch for reference - const { stdout: currentBranchOutput } = await execAsync('git rev-parse --abbrev-ref HEAD', { - cwd: worktreePath, - }); + // Resolve and validate worktreePath to prevent traversal attacks. + // The validatePathParams middleware checks against ALLOWED_ROOT_DIRECTORY, + // but we also resolve the path and verify it exists as a directory. + const resolvedPath = path.resolve(worktreePath); + try { + const stats = await stat(resolvedPath); + if (!stats.isDirectory()) { + res.status(400).json({ + success: false, + error: 'worktreePath is not a directory', + }); + return; + } + } catch { + res.status(400).json({ + success: false, + error: 'worktreePath does not exist or is not accessible', + }); + return; + } + + // Get current branch for reference (using argument array to avoid shell injection) + const currentBranchOutput = await execGitCommand( + ['rev-parse', '--abbrev-ref', 'HEAD'], + resolvedPath + ); const currentBranch = currentBranchOutput.trim(); // Check if branch already exists try { - await execAsync(`git rev-parse --verify ${branchName}`, { - cwd: worktreePath, - }); + await execGitCommand(['rev-parse', '--verify', branchName], resolvedPath); // Branch exists res.status(400).json({ success: false, @@ -67,10 +87,8 @@ export function createCheckoutBranchHandler() { // Branch doesn't exist, good to create } - // Create and checkout the new branch - await execAsync(`git checkout -b ${branchName}`, { - cwd: worktreePath, - }); + // Create and checkout the new branch (using argument array to avoid shell injection) + await execGitCommand(['checkout', '-b', branchName], resolvedPath); res.json({ success: true, diff --git a/apps/ui/src/components/views/board-view/components/kanban-card/card-header.tsx b/apps/ui/src/components/views/board-view/components/kanban-card/card-header.tsx index d69ebf8e..0f44ac21 100644 --- a/apps/ui/src/components/views/board-view/components/kanban-card/card-header.tsx +++ b/apps/ui/src/components/views/board-view/components/kanban-card/card-header.tsx @@ -1,4 +1,3 @@ -// @ts-nocheck - header component props with optional handlers and status variants import { memo, useState } from 'react'; import type { DraggableAttributes, DraggableSyntheticListeners } from '@dnd-kit/core'; import { Feature } from '@/store/app-store'; @@ -39,37 +38,53 @@ function DuplicateMenuItems({ onDuplicateAsChild?: () => void; }) { if (!onDuplicate) return null; + + // When there's no sub-child action, render a simple menu item (no DropdownMenuSub wrapper) + if (!onDuplicateAsChild) { + return ( + { + e.stopPropagation(); + onDuplicate(); + }} + className="text-xs" + > + + Duplicate + + ); + } + + // When sub-child action is available, render a proper DropdownMenuSub with + // DropdownMenuSubTrigger and DropdownMenuSubContent per Radix conventions return ( -
+ + + Duplicate + + { e.stopPropagation(); onDuplicate(); }} - className="text-xs flex-1 pr-0 rounded-r-none" + className="text-xs" > Duplicate - {onDuplicateAsChild && ( - - )} -
- {onDuplicateAsChild && ( - - { - e.stopPropagation(); - onDuplicateAsChild(); - }} - className="text-xs" - > - - Duplicate as Child - - - )} + { + e.stopPropagation(); + onDuplicateAsChild(); + }} + className="text-xs" + > + + Duplicate as Child + + ); } @@ -122,7 +137,7 @@ export const CardHeaderSection = memo(function CardHeaderSection({
- {feature.startedAt && ( + {typeof feature.startedAt === 'string' && ( - - - - - - - - + {/* Only render overflow menu when there are actionable items */} + {onDuplicate && ( + + + + + + + + + )}
)}