Skip to content
Merged
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
30 changes: 30 additions & 0 deletions .github/workflows/security-audit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
name: Security Audit

on:
pull_request:
branches:
- '*'
push:
branches:
- main
- master
schedule:
# Run weekly on Mondays at 9 AM UTC
- cron: '0 9 * * 1'

jobs:
audit:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Setup project
uses: ./.github/actions/setup-project
with:
check-lockfile: 'true'

- name: Run npm audit
run: npm audit --audit-level=moderate
continue-on-error: false
2 changes: 2 additions & 0 deletions apps/server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import { SettingsService } from './services/settings-service.js';
import { createSpecRegenerationRoutes } from './routes/app-spec/index.js';
import { createClaudeRoutes } from './routes/claude/index.js';
import { ClaudeUsageService } from './services/claude-usage-service.js';
import { createGitHubRoutes } from './routes/github/index.js';
import { createContextRoutes } from './routes/context/index.js';

// Load environment variables
Expand Down Expand Up @@ -146,6 +147,7 @@ app.use('/api/templates', createTemplatesRoutes());
app.use('/api/terminal', createTerminalRoutes());
app.use('/api/settings', createSettingsRoutes(settingsService));
app.use('/api/claude', createClaudeRoutes(claudeUsageService));
app.use('/api/github', createGitHubRoutes());
app.use('/api/context', createContextRoutes());

// Create HTTP server
Expand Down
18 changes: 18 additions & 0 deletions apps/server/src/routes/github/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* GitHub routes - HTTP API for GitHub integration
*/

import { Router } from 'express';
import { createCheckGitHubRemoteHandler } from './routes/check-github-remote.js';
import { createListIssuesHandler } from './routes/list-issues.js';
import { createListPRsHandler } from './routes/list-prs.js';

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

router.post('/check-remote', createCheckGitHubRemoteHandler());
router.post('/issues', createListIssuesHandler());
router.post('/prs', createListPRsHandler());

return router;
}
71 changes: 71 additions & 0 deletions apps/server/src/routes/github/routes/check-github-remote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* GET /check-github-remote endpoint - Check if project has a GitHub remote
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The comment indicates this is a GET endpoint, but it's registered as a POST route in apps/server/src/routes/github/index.ts. The documentation should be consistent with the implementation.

Suggested change
* GET /check-github-remote endpoint - Check if project has a GitHub remote
* POST /check-github-remote endpoint - Check if project has a GitHub remote

*/

import type { Request, Response } from 'express';
import { execAsync, execEnv, getErrorMessage, logError } from './common.js';

export interface GitHubRemoteStatus {
hasGitHubRemote: boolean;
remoteUrl: string | null;
owner: string | null;
repo: string | null;
}

export async function checkGitHubRemote(projectPath: string): Promise<GitHubRemoteStatus> {
const status: GitHubRemoteStatus = {
hasGitHubRemote: false,
remoteUrl: null,
owner: null,
repo: null,
};

try {
// Get the remote URL (origin by default)
const { stdout } = await execAsync('git remote get-url origin', {
cwd: projectPath,
env: execEnv,
});

const remoteUrl = stdout.trim();
status.remoteUrl = remoteUrl;

// Check if it's a GitHub URL
// Formats: https://github.com/owner/repo.git, git@github.com:owner/repo.git
const httpsMatch = remoteUrl.match(/https:\/\/github\.com\/([^/]+)\/([^/.]+)/);
const sshMatch = remoteUrl.match(/git@github\.com:([^/]+)\/([^/.]+)/);
Comment on lines +35 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The regular expressions used to capture the repository name are incorrect for repositories that contain a dot in their name. For example, for a URL like https://github.com/owner/my.repo.git, the current regex ([^/.]+) will only capture my. This should be changed to capture the full repository name.

Suggested change
const httpsMatch = remoteUrl.match(/https:\/\/github\.com\/([^/]+)\/([^/.]+)/);
const sshMatch = remoteUrl.match(/git@github\.com:([^/]+)\/([^/.]+)/);
const httpsMatch = remoteUrl.match(/https:\/\/github\.com\/([^/]+)\/(.+)/);
const sshMatch = remoteUrl.match(/git@github\.com:([^/]+)\/(.+)/);


const match = httpsMatch || sshMatch;
if (match) {
status.hasGitHubRemote = true;
status.owner = match[1];
status.repo = match[2].replace(/\.git$/, '');
}
} catch {
// No remote or not a git repo - that's okay
}

return status;
}

export function createCheckGitHubRemoteHandler() {
return async (req: Request, res: Response): Promise<void> => {
try {
const { projectPath } = req.body;

if (!projectPath) {
res.status(400).json({ success: false, error: 'projectPath is required' });
return;
}

const status = await checkGitHubRemote(projectPath);
res.json({
success: true,
...status,
});
} catch (error) {
logError(error, 'Check GitHub remote failed');
res.status(500).json({ success: false, error: getErrorMessage(error) });
}
};
}
35 changes: 35 additions & 0 deletions apps/server/src/routes/github/routes/common.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* Common utilities for GitHub routes
*/

import { exec } from 'child_process';
import { promisify } from 'util';

export const execAsync = promisify(exec);

// Extended PATH to include common tool installation locations
export const extendedPath = [
process.env.PATH,
'/opt/homebrew/bin',
'/usr/local/bin',
'/home/linuxbrew/.linuxbrew/bin',
`${process.env.HOME}/.local/bin`,
]
.filter(Boolean)
.join(':');

export const execEnv = {
...process.env,
PATH: extendedPath,
};

export function getErrorMessage(error: unknown): string {
if (error instanceof Error) {
return error.message;
}
return String(error);
}

export function logError(error: unknown, context: string): void {
console.error(`[GitHub] ${context}:`, error);
}
90 changes: 90 additions & 0 deletions apps/server/src/routes/github/routes/list-issues.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/**
* POST /list-issues endpoint - List GitHub issues for a project
*/

import type { Request, Response } from 'express';
import { execAsync, execEnv, getErrorMessage, logError } from './common.js';
import { checkGitHubRemote } from './check-github-remote.js';

export interface GitHubLabel {
name: string;
color: string;
}

export interface GitHubAuthor {
login: string;
}

export interface GitHubIssue {
number: number;
title: string;
state: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The state property is typed as a generic string. For better type safety and clarity, it should be a literal type representing the possible values returned by the GitHub CLI.

Suggested change
state: string;
state: 'OPEN' | 'CLOSED';

author: GitHubAuthor;
createdAt: string;
labels: GitHubLabel[];
url: string;
body: string;
}
Comment on lines +9 to +27
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Extract duplicated type definitions to a shared module.

The interfaces GitHubLabel, GitHubAuthor, and GitHubIssue are duplicated across multiple files:

  • apps/server/src/routes/github/routes/list-prs.ts
  • apps/ui/src/lib/electron.ts
  • This file

Centralizing these types in a shared module will prevent definition drift and reduce maintenance burden.

💡 Suggested approach

Consider creating a shared types file such as apps/server/src/routes/github/types.ts to export these common interfaces, then import them in both route handlers.

🤖 Prompt for AI Agents
In apps/server/src/routes/github/routes/list-issues.ts lines 9-27: the
interfaces GitHubLabel, GitHubAuthor, and GitHubIssue are duplicated across
multiple files; extract them into a single exported module (e.g.,
apps/server/src/routes/github/types.ts) that exports these three interfaces,
then replace the local definitions with an import from that new module in this
file and also update apps/server/src/routes/github/routes/list-prs.ts and
apps/ui/src/lib/electron.ts to import the shared types; ensure the new types
file uses named exports and adjust relative import paths in each file
accordingly so all references compile.


export interface ListIssuesResult {
success: boolean;
openIssues?: GitHubIssue[];
closedIssues?: GitHubIssue[];
error?: string;
}

export function createListIssuesHandler() {
return async (req: Request, res: Response): Promise<void> => {
try {
const { projectPath } = req.body;

if (!projectPath) {
res.status(400).json({ success: false, error: 'projectPath is required' });
return;
}

// First check if this is a GitHub repo
const remoteStatus = await checkGitHubRemote(projectPath);
if (!remoteStatus.hasGitHubRemote) {
res.status(400).json({
success: false,
error: 'Project does not have a GitHub remote',
});
return;
}

// Fetch open and closed issues in parallel
const [openResult, closedResult] = await Promise.all([
execAsync(
'gh issue list --state open --json number,title,state,author,createdAt,labels,url,body --limit 100',
{
cwd: projectPath,
env: execEnv,
}
),
execAsync(
'gh issue list --state closed --json number,title,state,author,createdAt,labels,url,body --limit 50',
{
cwd: projectPath,
env: execEnv,
}
),
Comment on lines +57 to +71
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

Add timeout to prevent indefinite blocking on CLI commands.

The GitHub CLI commands lack timeout protection. If the gh process hangs due to network issues, rate limits, or other failures, the request thread will block indefinitely, degrading service availability.

🔧 Recommended fix

Add a timeout to the execAsync options:

 const [openResult, closedResult] = await Promise.all([
   execAsync(
     'gh issue list --state open --json number,title,state,author,createdAt,labels,url,body --limit 100',
     {
       cwd: projectPath,
       env: execEnv,
+      timeout: 30000, // 30 second timeout
     }
   ),
   execAsync(
     'gh issue list --state closed --json number,title,state,author,createdAt,labels,url,body --limit 50',
     {
       cwd: projectPath,
       env: execEnv,
+      timeout: 30000, // 30 second timeout
     }
   ),
 ]);
🤖 Prompt for AI Agents
In apps/server/src/routes/github/routes/list-issues.ts around lines 57 to 71,
the two execAsync calls invoking the `gh` CLI lack a timeout and can block
forever; add a timeout option (e.g. timeout: 30000) to each execAsync options
object so the child process is killed after a reasonable period, and update the
surrounding error handling to catch timeout errors (or treat them as transient
failures) and return/throw an appropriate error (e.g. 504 or a descriptive log)
instead of allowing indefinite blocking.

]);

const { stdout: openStdout } = openResult;
const { stdout: closedStdout } = closedResult;

const openIssues: GitHubIssue[] = JSON.parse(openStdout || '[]');
const closedIssues: GitHubIssue[] = JSON.parse(closedStdout || '[]');

res.json({
success: true,
openIssues,
closedIssues,
});
} catch (error) {
logError(error, 'List GitHub issues failed');
res.status(500).json({ success: false, error: getErrorMessage(error) });
}
};
}
92 changes: 92 additions & 0 deletions apps/server/src/routes/github/routes/list-prs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/**
* POST /list-prs endpoint - List GitHub pull requests for a project
*/

import type { Request, Response } from 'express';
import { execAsync, execEnv, getErrorMessage, logError } from './common.js';
import { checkGitHubRemote } from './check-github-remote.js';

export interface GitHubLabel {
name: string;
color: string;
}

export interface GitHubAuthor {
login: string;
}

export interface GitHubPR {
number: number;
title: string;
state: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The state property is typed as a generic string. For better type safety and clarity, it should be a literal type representing the possible values.

Suggested change
state: string;
state: 'OPEN' | 'MERGED' | 'CLOSED';

author: GitHubAuthor;
createdAt: string;
labels: GitHubLabel[];
url: string;
isDraft: boolean;
headRefName: string;
reviewDecision: string | null;
mergeable: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The mergeable property is typed as a generic string. Using a literal type with possible values like 'MERGEABLE', 'CONFLICTING', and 'UNKNOWN' would improve type safety.

Suggested change
mergeable: string;
mergeable: 'MERGEABLE' | 'CONFLICTING' | 'UNKNOWN';

body: string;
}

export interface ListPRsResult {
success: boolean;
openPRs?: GitHubPR[];
mergedPRs?: GitHubPR[];
error?: string;
}

export function createListPRsHandler() {
return async (req: Request, res: Response): Promise<void> => {
try {
const { projectPath } = req.body;

if (!projectPath) {
res.status(400).json({ success: false, error: 'projectPath is required' });
return;
}

// First check if this is a GitHub repo
const remoteStatus = await checkGitHubRemote(projectPath);
if (!remoteStatus.hasGitHubRemote) {
res.status(400).json({
success: false,
error: 'Project does not have a GitHub remote',
});
return;
}

const [openResult, mergedResult] = await Promise.all([
execAsync(
'gh pr list --state open --json number,title,state,author,createdAt,labels,url,isDraft,headRefName,reviewDecision,mergeable,body --limit 100',
{
cwd: projectPath,
env: execEnv,
}
),
execAsync(
'gh pr list --state merged --json number,title,state,author,createdAt,labels,url,isDraft,headRefName,reviewDecision,mergeable,body --limit 50',
{
cwd: projectPath,
env: execEnv,
}
),
]);
const { stdout: openStdout } = openResult;
const { stdout: mergedStdout } = mergedResult;

const openPRs: GitHubPR[] = JSON.parse(openStdout || '[]');
const mergedPRs: GitHubPR[] = JSON.parse(mergedStdout || '[]');

res.json({
success: true,
openPRs,
mergedPRs,
});
} catch (error) {
logError(error, 'List GitHub PRs failed');
res.status(500).json({ success: false, error: getErrorMessage(error) });
}
};
}
12 changes: 3 additions & 9 deletions apps/server/src/routes/suggestions/generate-suggestions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,14 @@ const suggestionsSchema = {
id: { type: 'string' },
category: { type: 'string' },
description: { type: 'string' },
steps: {
type: 'array',
items: { type: 'string' },
},
priority: {
type: 'number',
minimum: 1,
maximum: 3,
},
reasoning: { type: 'string' },
},
required: ['category', 'description', 'steps', 'priority', 'reasoning'],
required: ['category', 'description', 'priority', 'reasoning'],
},
},
},
Expand Down Expand Up @@ -62,9 +58,8 @@ Look at the codebase and provide 3-5 concrete suggestions.
For each suggestion, provide:
1. A category (e.g., "User Experience", "Security", "Performance")
2. A clear description of what to implement
3. Concrete steps to implement it
4. Priority (1=high, 2=medium, 3=low)
5. Brief reasoning for why this would help
3. Priority (1=high, 2=medium, 3=low)
4. Brief reasoning for why this would help

The response will be automatically formatted as structured JSON.`;

Expand Down Expand Up @@ -164,7 +159,6 @@ The response will be automatically formatted as structured JSON.`;
id: `suggestion-${Date.now()}-0`,
category: 'Analysis',
description: 'Review the AI analysis output for insights',
steps: ['Review the generated analysis'],
priority: 1,
reasoning: 'The AI provided analysis but suggestions need manual review',
},
Expand Down
Loading