Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ export async function generateFeaturesFromSpec(
events: EventEmitter,
abortController: AbortController,
maxFeatures?: number,
settingsService?: SettingsService
settingsService?: SettingsService,
targetBranch?: string
): Promise<void> {
const featureCount = maxFeatures ?? DEFAULT_MAX_FEATURES;
logger.debug('========== generateFeaturesFromSpec() started ==========');
logger.debug('projectPath:', projectPath);
logger.debug('maxFeatures:', featureCount);
logger.debug('targetBranch:', targetBranch);

// Read existing spec from .automaker directory
const specPath = getAppSpecPath(projectPath);
Expand Down Expand Up @@ -233,7 +235,7 @@ CRITICAL INSTRUCTIONS:
logger.info(responseText);
logger.info('========== END RESPONSE TEXT ==========');

await parseAndCreateFeatures(projectPath, responseText, events);
await parseAndCreateFeatures(projectPath, responseText, events, targetBranch);

logger.debug('========== generateFeaturesFromSpec() completed ==========');
}
83 changes: 81 additions & 2 deletions apps/server/src/routes/app-spec/generate-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { generateFeaturesFromSpec } from './generate-features-from-spec.js';
import { ensureAutomakerDir, getAppSpecPath } from '@automaker/platform';
import type { SettingsService } from '../../services/settings-service.js';
import { getAutoLoadClaudeMdSetting } from '../../lib/settings-helpers.js';
import { validateBranchName } from '@automaker/git-utils';

const logger = createLogger('SpecRegeneration');

Expand All @@ -37,7 +38,8 @@ export async function generateSpec(
generateFeatures?: boolean,
analyzeProject?: boolean,
maxFeatures?: number,
settingsService?: SettingsService
settingsService?: SettingsService,
targetBranch?: string
): Promise<void> {
logger.info('========== generateSpec() started ==========');
logger.info('projectPath:', projectPath);
Expand All @@ -46,6 +48,7 @@ export async function generateSpec(
logger.info('generateFeatures:', generateFeatures);
logger.info('analyzeProject:', analyzeProject);
logger.info('maxFeatures:', maxFeatures);
logger.info('targetBranch:', targetBranch);

// Build the prompt based on whether we should analyze the project
let analysisInstructions = '';
Expand Down Expand Up @@ -360,6 +363,81 @@ Your entire response should be valid JSON starting with { and ending with }. No
});
}

// Create worktree for target branch if specified and doesn't exist
if (targetBranch && targetBranch !== 'main' && targetBranch !== 'master') {
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

security-critical critical

The targetBranch variable, which comes from the request body, is used directly in shell commands via execAsync. This creates a critical command injection vulnerability. A malicious user could provide a crafted branch name like my-branch; rm -rf / to execute arbitrary commands on the server.

To fix this, you must validate the targetBranch name to ensure it only contains characters safe for a branch name before using it in any shell commands. I've suggested adding a validation function that checks for invalid characters and path traversal attempts.

    const isValidGitBranchName = (branchName: string): boolean => {
      // Prevents command injection and path traversal.
      if (branchName.includes('..')) {
        return false;
      }
      // A simplified regex for valid git branch names. It disallows characters
      // often used in command injection attacks (;, &, |, $, `, etc.).
      return /^[a-zA-Z0-9_./-]+$/.test(branchName);
    };

    if (!isValidGitBranchName(targetBranch)) {
      const errorMessage = `Invalid target branch name provided: ${targetBranch}`;
      logger.error(errorMessage);
      // Throwing an error is important to stop execution and alert the caller.
      throw new Error(errorMessage);
    }

    try {

Comment on lines +366 to +368
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the file in question to understand the context better
cat -n apps/server/src/routes/app-spec/generate-spec.ts | sed -n '355,375p'

Repository: AutoMaker-Org/automaker

Length of output: 999


🏁 Script executed:

# Search for how the codebase determines or references primary/main branches
rg -n "primary.*branch|mainBranch|develop.*branch|master.*branch" apps/server/src --type ts

Repository: AutoMaker-Org/automaker

Length of output: 49


🏁 Script executed:

# Look for branch-related constants or configurations
rg -n "const.*branch|main.*master|develop" apps/server/src --type ts -B 1 -A 1 | head -n 100

Repository: AutoMaker-Org/automaker

Length of output: 7742


🏁 Script executed:

# Look for function documentation and more context around the worktree creation
cat -n apps/server/src/routes/app-spec/generate-spec.ts | sed -n '300,400p'

Repository: AutoMaker-Org/automaker

Length of output: 5011


🏁 Script executed:

# Search for any configuration or documentation about primary branches
rg -n "primary" apps/server/src --type ts -B 2 -A 2

Repository: AutoMaker-Org/automaker

Length of output: 49


🏁 Script executed:

# Check if there's documentation about the function or branch handling strategy
rg -n "develop" apps/server/src/routes/app-spec --type ts -B 1 -A 1

Repository: AutoMaker-Org/automaker

Length of output: 49


🏁 Script executed:

# Look for how baseBranch or similar concepts are handled
rg -n "baseBranch|base.*branch" apps/server/src --type ts -B 1 -A 1 | head -50

Repository: AutoMaker-Org/automaker

Length of output: 2541


🏁 Script executed:

# Get function signature and documentation
cat -n apps/server/src/routes/app-spec/generate-spec.ts | head -100

Repository: AutoMaker-Org/automaker

Length of output: 4632


🏁 Script executed:

# Search for where generateSpec function is called and how targetBranch is passed
rg -n "generateSpec|targetBranch" apps/server/src/routes --type ts -B 2 -A 2 | head -100

Repository: AutoMaker-Org/automaker

Length of output: 8523


🏁 Script executed:

# Look for any configuration or environment variables related to default/primary branches
rg -n "default.*branch|main.*master" apps/server/src --type ts | head -50

Repository: AutoMaker-Org/automaker

Length of output: 486


Add support for additional primary branches beyond just main and master.

The worktree creation logic currently skips only main and master, but doesn't account for develop or other primary branches a project might use. If a project designates develop as its primary branch, creating a worktree for it would be incorrect.

This pattern appears in multiple places (generate-spec.ts:366, parse-and-create-features.ts:123, and delete.ts:60). The worktree logic should detect or accept a configurable list of primary branches rather than hardcoding only two branch names.

🤖 Prompt for AI Agents
In @apps/server/src/routes/app-spec/generate-spec.ts around lines 365 - 367,
Replace the hardcoded check (targetBranch !== 'main' && targetBranch !==
'master') with a reusable primary-branch check using a configurable list: create
a helper (e.g., isPrimaryBranch(branch: string)) that reads PRIMARY_BRANCHES
from config/ENV (falling back to ['main','master']) and returns
PRIMARY_BRANCHES.includes(branch); then use if (targetBranch &&
!isPrimaryBranch(targetBranch)) when creating worktrees in generate-spec.ts (the
targetBranch conditional), parse-and-create-features.ts, and delete.ts so
additional primary branches like 'develop' are respected and the list can be
changed without code edits.

// Validate branch name to prevent command injection
validateBranchName(targetBranch);

logger.info(`Checking if worktree for branch '${targetBranch}' exists...`);
const { execFile } = await import('child_process');
const { promisify } = await import('util');
const fs = await import('fs');
const execFileAsync = promisify(execFile);

// Check if worktree already exists
const worktreePath = path.join(projectPath, '.worktrees', targetBranch);
const worktreeExists = fs.existsSync(worktreePath);

if (worktreeExists) {
logger.info(`Worktree for branch '${targetBranch}' already exists at ${worktreePath}`);
} else {
// Check if branch exists
let branchExists = false;
try {
await execFileAsync('git', ['rev-parse', '--verify', targetBranch], { cwd: projectPath });
branchExists = true;
logger.info(`Branch '${targetBranch}' already exists`);
} catch {
logger.info(`Branch '${targetBranch}' does not exist, will create with worktree`);
}

// Create .worktrees directory if it doesn't exist
const worktreesDir = path.join(projectPath, '.worktrees');
if (!fs.existsSync(worktreesDir)) {
fs.mkdirSync(worktreesDir, { recursive: true });
}

// Create worktree
logger.info(`Creating worktree for branch '${targetBranch}' at ${worktreePath}`);
if (branchExists) {
// Use existing branch
await execFileAsync('git', ['worktree', 'add', worktreePath, targetBranch], {
cwd: projectPath,
});
} else {
// Create new branch from HEAD
await execFileAsync(
'git',
['worktree', 'add', '-b', targetBranch, worktreePath, 'HEAD'],
{
cwd: projectPath,
}
);
}
logger.info(`✓ Created worktree for branch '${targetBranch}'`);

// Track the branch so it persists in the UI
try {
await execFileAsync('git', ['config', `branch.${targetBranch}.remote`, 'origin'], {
cwd: projectPath,
});
await execFileAsync(
'git',
['config', `branch.${targetBranch}.merge`, `refs/heads/${targetBranch}`],
{ cwd: projectPath }
);
logger.info(`✓ Configured tracking for branch '${targetBranch}'`);
} catch (trackError) {
logger.warn(`Failed to configure tracking for branch '${targetBranch}':`, trackError);
}
}
} catch (worktreeError) {
logger.warn(`Failed to create worktree for branch '${targetBranch}':`, worktreeError);
// Don't throw - this is not critical, continue with feature generation
}
}

// If generate features was requested, generate them from the spec
if (generateFeatures) {
logger.info('Starting feature generation from spec...');
Expand All @@ -371,7 +449,8 @@ Your entire response should be valid JSON starting with { and ending with }. No
events,
featureAbortController,
maxFeatures,
settingsService
settingsService,
targetBranch
);
// Final completion will be emitted by generateFeaturesFromSpec -> parseAndCreateFeatures
} catch (featureError) {
Expand Down
45 changes: 44 additions & 1 deletion apps/server/src/routes/app-spec/parse-and-create-features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ const logger = createLogger('SpecRegeneration');
export async function parseAndCreateFeatures(
projectPath: string,
content: string,
events: EventEmitter
events: EventEmitter,
targetBranch?: string
): Promise<void> {
logger.info('========== parseAndCreateFeatures() started ==========');
logger.info(`Content length: ${content.length} chars`);
logger.info('targetBranch:', targetBranch);
logger.info('========== CONTENT RECEIVED FOR PARSING ==========');
logger.info(content);
logger.info('========== END CONTENT ==========');
Comment on lines 14 to 25
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid logging raw content at info-level; include targetBranch but keep payload logs bounded.

Adding targetBranch logging is helpful, but this function currently logs the full content at info, which can be huge and may include sensitive data. Consider moving the full payload to debug (or truncating) and keeping info to metadata (length, branch, counts).

🤖 Prompt for AI Agents
In @apps/server/src/routes/app-spec/parse-and-create-features.ts around lines 14
- 25, The parseAndCreateFeatures function logs the entire content at info level
which can be large or sensitive; change logging so info-level retains only
metadata (keep logger.info('Content length: ...') and logger.info('targetBranch:
...') using the targetBranch variable) and move the full payload to logger.debug
or log a truncated preview (e.g., first N chars with an ellipsis) instead of
logger.info(content); update references in parseAndCreateFeatures to use
logger.debug for the full content and ensure any truncated preview is clearly
labeled to bound the payload size.

Expand Down Expand Up @@ -53,6 +55,38 @@ export async function parseAndCreateFeatures(
const featuresDir = getFeaturesDir(projectPath);
await secureFs.mkdir(featuresDir, { recursive: true });

// Delete existing features for this branch before creating new ones
if (targetBranch) {
logger.info(`Deleting existing features for branch '${targetBranch}'...`);
try {
const existingFeatures = await secureFs.readdir(featuresDir);
let deletedCount = 0;

for (const featureId of existingFeatures) {
const featureJsonPath = path.join(featuresDir, featureId, 'feature.json');
try {
const featureContent = (await secureFs.readFile(featureJsonPath, 'utf-8')) as string;
const featureData = JSON.parse(featureContent);

// Delete if it matches the target branch
if (featureData.branchName === targetBranch) {
logger.info(`Deleting feature ${featureId} from branch ${targetBranch}`);
await secureFs.rm(path.join(featuresDir, featureId), { recursive: true });
deletedCount++;
}
} catch (err) {
// Skip if feature.json doesn't exist or can't be read
logger.debug(`Skipping ${featureId}: ${err}`);
}
}

logger.info(`✓ Deleted ${deletedCount} existing features for branch '${targetBranch}'`);
} catch (err) {
logger.warn('Failed to delete existing features:', err);
// Continue anyway - not critical
}
}

const createdFeatures: Array<{ id: string; title: string }> = [];

for (const feature of parsed.features) {
Expand All @@ -69,6 +103,7 @@ export async function parseAndCreateFeatures(
priority: feature.priority || 2,
complexity: feature.complexity || 'moderate',
dependencies: feature.dependencies || [],
branchName: targetBranch || 'main',
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};
Expand All @@ -83,10 +118,18 @@ export async function parseAndCreateFeatures(

logger.info(`✓ Created ${createdFeatures.length} features successfully`);

// Calculate worktree path for the target branch
let worktreePath: string | undefined;
if (targetBranch && targetBranch !== 'main' && targetBranch !== 'master') {
worktreePath = path.join(projectPath, '.worktrees', targetBranch);
}

events.emit('spec-regeneration:event', {
type: 'spec_regeneration_complete',
message: `Spec regeneration complete! Created ${createdFeatures.length} features.`,
projectPath: projectPath,
targetBranch: targetBranch,
worktreePath: worktreePath,
});
} catch (error) {
logger.error('❌ parseAndCreateFeatures() failed:');
Expand Down
28 changes: 19 additions & 9 deletions apps/server/src/routes/app-spec/routes/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,29 @@ export function createCreateHandler(events: EventEmitter) {
logger.debug('Request body:', JSON.stringify(req.body, null, 2));

try {
const { projectPath, projectOverview, generateFeatures, analyzeProject, maxFeatures } =
req.body as {
projectPath: string;
projectOverview: string;
generateFeatures?: boolean;
analyzeProject?: boolean;
maxFeatures?: number;
};
const {
projectPath,
projectOverview,
generateFeatures,
analyzeProject,
maxFeatures,
targetBranch,
} = req.body as {
projectPath: string;
projectOverview: string;
generateFeatures?: boolean;
analyzeProject?: boolean;
maxFeatures?: number;
targetBranch?: string;
};

logger.debug('Parsed params:');
logger.debug(' projectPath:', projectPath);
logger.debug(' projectOverview length:', `${projectOverview?.length || 0} chars`);
logger.debug(' generateFeatures:', generateFeatures);
logger.debug(' analyzeProject:', analyzeProject);
logger.debug(' maxFeatures:', maxFeatures);
logger.debug(' targetBranch:', targetBranch);

if (!projectPath || !projectOverview) {
logger.error('Missing required parameters');
Expand Down Expand Up @@ -68,7 +76,9 @@ export function createCreateHandler(events: EventEmitter) {
abortController,
generateFeatures,
analyzeProject,
maxFeatures
maxFeatures,
undefined,
targetBranch
)
.catch((error) => {
logError(error, 'Generation failed with error');
Expand Down
27 changes: 18 additions & 9 deletions apps/server/src/routes/app-spec/routes/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,29 @@ export function createGenerateHandler(events: EventEmitter, settingsService?: Se
logger.debug('Request body:', JSON.stringify(req.body, null, 2));

try {
const { projectPath, projectDefinition, generateFeatures, analyzeProject, maxFeatures } =
req.body as {
projectPath: string;
projectDefinition: string;
generateFeatures?: boolean;
analyzeProject?: boolean;
maxFeatures?: number;
};
const {
projectPath,
projectDefinition,
generateFeatures,
analyzeProject,
maxFeatures,
targetBranch,
} = req.body as {
projectPath: string;
projectDefinition: string;
generateFeatures?: boolean;
analyzeProject?: boolean;
maxFeatures?: number;
targetBranch?: string;
};

logger.debug('Parsed params:');
logger.debug(' projectPath:', projectPath);
logger.debug(' projectDefinition length:', `${projectDefinition?.length || 0} chars`);
logger.debug(' generateFeatures:', generateFeatures);
logger.debug(' analyzeProject:', analyzeProject);
logger.debug(' maxFeatures:', maxFeatures);
logger.debug(' targetBranch:', targetBranch);

if (!projectPath || !projectDefinition) {
logger.error('Missing required parameters');
Expand Down Expand Up @@ -69,7 +77,8 @@ export function createGenerateHandler(events: EventEmitter, settingsService?: Se
generateFeatures,
analyzeProject,
maxFeatures,
settingsService
settingsService,
targetBranch
)
.catch((error) => {
logError(error, 'Generation failed with error');
Expand Down
6 changes: 6 additions & 0 deletions apps/server/src/routes/worktree/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,19 @@ import { createMigrateHandler } from './routes/migrate.js';
import { createStartDevHandler } from './routes/start-dev.js';
import { createStopDevHandler } from './routes/stop-dev.js';
import { createListDevServersHandler } from './routes/list-dev-servers.js';
import { createGetCurrentBranchHandler } from './routes/get-current-branch.js';

export function createWorktreeRoutes(): Router {
const router = Router();

router.post('/info', validatePathParams('projectPath'), createInfoHandler());
router.post('/status', validatePathParams('projectPath'), createStatusHandler());
router.post('/list', createListHandler());
router.post(
'/current-branch',
validatePathParams('projectPath'),
createGetCurrentBranchHandler()
);
router.post('/diffs', validatePathParams('projectPath'), createDiffsHandler());
router.post('/file-diff', validatePathParams('projectPath', 'filePath'), createFileDiffHandler());
router.post(
Expand Down
Loading
Loading