mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix: code cleanup — path traversal, test leaks, dead code, consistency
- Add path traversal protection to GET /api/cases/:name and fix-plan - Use safePathSchema for LinkCaseSchema.path - Fix QR auth test timer leak (afterAll → afterEach) and env var try/finally - Remove dead terminal size check after Zod validation in resize route - Remove no-op sampleCount guard in adaptive timing - Replace hardcoded values with constants in notification-manager and subagent-windows - Add Zod validation to POST /api/auth/revoke - Use _apiPut instead of raw fetch in subagent-windows - Add SwipeHandler.cleanup() for consistency with other mobile handlers - Move NiceConfig/ProcessStats from types/plan.ts to types/common.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -76,11 +76,6 @@ export class RespawnAdaptiveTiming {
|
||||
* @returns Completion confirm timeout in milliseconds
|
||||
*/
|
||||
getAdaptiveCompletionConfirmMs(): number {
|
||||
// Need at least 5 samples before adjusting
|
||||
if (this.timingHistory.sampleCount < 5) {
|
||||
return this.timingHistory.adaptiveCompletionConfirmMs;
|
||||
}
|
||||
|
||||
return this.timingHistory.adaptiveCompletionConfirmMs;
|
||||
}
|
||||
|
||||
|
||||
@@ -29,6 +29,36 @@ export interface BufferConfig {
|
||||
/**
|
||||
* Resource types that can be registered for cleanup.
|
||||
*/
|
||||
/**
|
||||
* Configuration for process priority using `nice`.
|
||||
* Lower priority reduces CPU contention with other processes.
|
||||
*/
|
||||
export interface NiceConfig {
|
||||
/** Whether nice priority is enabled */
|
||||
enabled: boolean;
|
||||
/** Nice value (-20 to 19, default: 10 = lower priority) */
|
||||
niceValue: number;
|
||||
}
|
||||
|
||||
export const DEFAULT_NICE_CONFIG: NiceConfig = {
|
||||
enabled: false,
|
||||
niceValue: 10,
|
||||
};
|
||||
|
||||
/**
|
||||
* Process resource statistics
|
||||
*/
|
||||
export interface ProcessStats {
|
||||
/** Memory usage in megabytes */
|
||||
memoryMB: number;
|
||||
/** CPU usage percentage */
|
||||
cpuPercent: number;
|
||||
/** Number of child processes */
|
||||
childCount: number;
|
||||
/** Timestamp of stats collection */
|
||||
updatedAt: number;
|
||||
}
|
||||
|
||||
export type CleanupResourceType = 'timer' | 'interval' | 'watcher' | 'listener' | 'stream';
|
||||
|
||||
/**
|
||||
|
||||
@@ -11,36 +11,6 @@ export type TddPhase = 'setup' | 'test' | 'impl' | 'verify' | 'review';
|
||||
/** Development phase in TDD cycle (alias for TddPhase) */
|
||||
export type PlanPhase = TddPhase;
|
||||
|
||||
/**
|
||||
* Configuration for process priority using `nice`.
|
||||
* Lower priority reduces CPU contention with other processes.
|
||||
*/
|
||||
export interface NiceConfig {
|
||||
/** Whether nice priority is enabled */
|
||||
enabled: boolean;
|
||||
/** Nice value (-20 to 19, default: 10 = lower priority) */
|
||||
niceValue: number;
|
||||
}
|
||||
|
||||
export const DEFAULT_NICE_CONFIG: NiceConfig = {
|
||||
enabled: false,
|
||||
niceValue: 10,
|
||||
};
|
||||
|
||||
/**
|
||||
* Process resource statistics
|
||||
*/
|
||||
export interface ProcessStats {
|
||||
/** Memory usage in megabytes */
|
||||
memoryMB: number;
|
||||
/** CPU usage percentage */
|
||||
cpuPercent: number;
|
||||
/** Number of child processes */
|
||||
childCount: number;
|
||||
/** Timestamp of stats collection */
|
||||
updatedAt: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* A single plan item for plan orchestration.
|
||||
* Moved here from plan-orchestrator.ts to break circular dependency.
|
||||
|
||||
@@ -403,6 +403,10 @@ const SwipeHandler = {
|
||||
maxSwipeTime: 300, // Maximum ms for a swipe gesture
|
||||
maxVerticalDrift: 100, // Max vertical movement allowed
|
||||
|
||||
_touchStartHandler: null,
|
||||
_touchEndHandler: null,
|
||||
_element: null,
|
||||
|
||||
/** Initialize swipe handling */
|
||||
init() {
|
||||
// Only on touch devices
|
||||
@@ -411,8 +415,22 @@ const SwipeHandler = {
|
||||
const terminal = document.querySelector('.main');
|
||||
if (!terminal) return;
|
||||
|
||||
terminal.addEventListener('touchstart', (e) => this.onTouchStart(e), { passive: true });
|
||||
terminal.addEventListener('touchend', (e) => this.onTouchEnd(e), { passive: true });
|
||||
this._element = terminal;
|
||||
this._touchStartHandler = (e) => this.onTouchStart(e);
|
||||
this._touchEndHandler = (e) => this.onTouchEnd(e);
|
||||
terminal.addEventListener('touchstart', this._touchStartHandler, { passive: true });
|
||||
terminal.addEventListener('touchend', this._touchEndHandler, { passive: true });
|
||||
},
|
||||
|
||||
/** Remove swipe listeners */
|
||||
cleanup() {
|
||||
if (this._element && this._touchStartHandler) {
|
||||
this._element.removeEventListener('touchstart', this._touchStartHandler);
|
||||
this._element.removeEventListener('touchend', this._touchEndHandler);
|
||||
}
|
||||
this._touchStartHandler = null;
|
||||
this._touchEndHandler = null;
|
||||
this._element = null;
|
||||
},
|
||||
|
||||
onTouchStart(e) {
|
||||
|
||||
@@ -188,9 +188,9 @@ class NotificationManager {
|
||||
count: 1,
|
||||
};
|
||||
|
||||
// Add to log (cap at 100)
|
||||
// Add to log (cap at NOTIFICATION_LIST_CAP)
|
||||
this.notifications.unshift(notification);
|
||||
if (this.notifications.length > 100) this.notifications.pop();
|
||||
if (this.notifications.length > NOTIFICATION_LIST_CAP) this.notifications.pop();
|
||||
|
||||
// Track for grouping
|
||||
const timeout = setTimeout(() => this.groupingMap.delete(groupKey), GROUPING_TIMEOUT_MS);
|
||||
@@ -298,9 +298,9 @@ class NotificationManager {
|
||||
}
|
||||
if (Notification.permission !== 'granted') return;
|
||||
|
||||
// Rate limit: max 1 per 3 seconds
|
||||
// Rate limit
|
||||
const now = Date.now();
|
||||
if (now - this.lastBrowserNotifTime < 3000) return;
|
||||
if (now - this.lastBrowserNotifTime < BROWSER_NOTIF_RATE_LIMIT_MS) return;
|
||||
this.lastBrowserNotifTime = now;
|
||||
|
||||
const notif = new Notification(`Codeman: ${title}`, {
|
||||
@@ -318,8 +318,8 @@ class NotificationManager {
|
||||
notif.close();
|
||||
};
|
||||
|
||||
// Auto-close after 8s
|
||||
setTimeout(() => notif.close(), 8000);
|
||||
// Auto-close
|
||||
setTimeout(() => notif.close(), AUTO_CLOSE_NOTIFICATION_MS);
|
||||
}
|
||||
|
||||
async requestPermission() {
|
||||
|
||||
@@ -111,11 +111,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
|
||||
// Save to server for cross-browser persistence
|
||||
try {
|
||||
await fetch('/api/subagent-window-states', {
|
||||
method: 'PUT',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(windowStates)
|
||||
});
|
||||
await this._apiPut('/api/subagent-window-states', windowStates);
|
||||
} catch (err) {
|
||||
console.error('Failed to save subagent window states to server:', err);
|
||||
}
|
||||
@@ -178,7 +174,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
if (windowData && windowData.element) {
|
||||
// Parse position values and clamp to viewport
|
||||
let left = parseInt(position.left, 10) || 50;
|
||||
let top = parseInt(position.top, 10) || 120;
|
||||
let top = parseInt(position.top, 10) || WINDOW_INITIAL_TOP_PX;
|
||||
const viewportWidth = window.innerWidth;
|
||||
const viewportHeight = window.innerHeight;
|
||||
const windowWidth = 420;
|
||||
@@ -545,7 +541,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
} else {
|
||||
// Normal positioning
|
||||
startX = 50;
|
||||
startY = 120;
|
||||
startY = WINDOW_INITIAL_TOP_PX;
|
||||
maxCols = Math.floor((viewportWidth - startX - 50) / (windowWidth + gap)) || 1;
|
||||
}
|
||||
|
||||
|
||||
@@ -162,6 +162,14 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
|
||||
app.get('/api/cases/:name', async (req) => {
|
||||
const { name } = req.params as { name: string };
|
||||
|
||||
// Security: Path traversal protection
|
||||
const resolvedPath = resolve(join(CASES_DIR, name));
|
||||
const resolvedBase = resolve(CASES_DIR);
|
||||
const relPath = relative(resolvedBase, resolvedPath);
|
||||
if (relPath.startsWith('..') || isAbsolute(relPath)) {
|
||||
return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case name');
|
||||
}
|
||||
|
||||
// First check linked cases
|
||||
const linkedCasesFile = join(homedir(), '.codeman', 'linked-cases.json');
|
||||
try {
|
||||
@@ -197,6 +205,14 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
|
||||
app.get('/api/cases/:name/fix-plan', async (req) => {
|
||||
const { name } = req.params as { name: string };
|
||||
|
||||
// Security: Path traversal protection
|
||||
const resolvedPath = resolve(join(CASES_DIR, name));
|
||||
const resolvedBase = resolve(CASES_DIR);
|
||||
const relPath = relative(resolvedBase, resolvedPath);
|
||||
if (relPath.startsWith('..') || isAbsolute(relPath)) {
|
||||
return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case name');
|
||||
}
|
||||
|
||||
// Get case path (check linked cases first, then CASES_DIR)
|
||||
let casePath: string | null = null;
|
||||
|
||||
|
||||
@@ -43,8 +43,6 @@ import { RunSummaryTracker } from '../../run-summary.js';
|
||||
|
||||
import {
|
||||
MAX_INPUT_LENGTH,
|
||||
MAX_TERMINAL_COLS,
|
||||
MAX_TERMINAL_ROWS,
|
||||
MAX_SESSION_NAME_LENGTH,
|
||||
} from '../../config/terminal-limits.js';
|
||||
|
||||
@@ -495,14 +493,6 @@ export function registerSessionRoutes(
|
||||
return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Session not found');
|
||||
}
|
||||
|
||||
// Note: Zod already validates that cols and rows are positive integers within bounds
|
||||
if (cols > MAX_TERMINAL_COLS || rows > MAX_TERMINAL_ROWS) {
|
||||
return createErrorResponse(
|
||||
ApiErrorCode.INVALID_INPUT,
|
||||
`Terminal dimensions exceed maximum (${MAX_TERMINAL_COLS}x${MAX_TERMINAL_ROWS})`
|
||||
);
|
||||
}
|
||||
|
||||
session.resize(cols, rows);
|
||||
return { success: true };
|
||||
});
|
||||
|
||||
@@ -19,6 +19,7 @@ import {
|
||||
CpuLimitSchema,
|
||||
SubagentWindowStatesSchema,
|
||||
SubagentParentMapSchema,
|
||||
RevokeSessionSchema,
|
||||
} from '../schemas.js';
|
||||
import { subagentWatcher } from '../../subagent-watcher.js';
|
||||
import { imageWatcher } from '../../image-watcher.js';
|
||||
@@ -194,9 +195,9 @@ export function registerSystemRoutes(
|
||||
// ========== Auth Session Revocation ==========
|
||||
|
||||
app.post('/api/auth/revoke', async (req) => {
|
||||
const body = req.body as { sessionToken?: string } | undefined;
|
||||
if (body?.sessionToken) {
|
||||
ctx.authSessions?.delete(body.sessionToken);
|
||||
const result = RevokeSessionSchema.safeParse(req.body);
|
||||
if (result.success && result.data.sessionToken) {
|
||||
ctx.authSessions?.delete(result.data.sessionToken);
|
||||
} else {
|
||||
// Revoke all sessions (nuclear option)
|
||||
ctx.authSessions?.clear();
|
||||
|
||||
+6
-1
@@ -396,7 +396,12 @@ export const ScheduledRunSchema = z.object({
|
||||
/** POST /api/cases/link */
|
||||
export const LinkCaseSchema = z.object({
|
||||
name: z.string().regex(/^[a-zA-Z0-9_-]+$/, 'Invalid case name format'),
|
||||
path: z.string().min(1).max(1000),
|
||||
path: safePathSchema,
|
||||
});
|
||||
|
||||
/** POST /api/auth/revoke */
|
||||
export const RevokeSessionSchema = z.object({
|
||||
sessionToken: z.string().min(1).max(200).optional(),
|
||||
});
|
||||
|
||||
/** POST /api/generate-plan */
|
||||
|
||||
@@ -38,8 +38,8 @@ describe('QR Token Manager (unit)', () => {
|
||||
tm.startTokenRotation();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
// Clean up any lingering timers
|
||||
afterEach(() => {
|
||||
// Clean up rotation timer for this test's TunnelManager
|
||||
tm?.stopTokenRotation();
|
||||
});
|
||||
|
||||
@@ -234,11 +234,13 @@ describe('QR Auth Integration', () => {
|
||||
const savedPass = process.env.CODEMAN_PASSWORD;
|
||||
delete process.env.CODEMAN_PASSWORD;
|
||||
|
||||
const res = await fetch(`${baseUrl}/q/ANYCODE`, { redirect: 'manual' });
|
||||
expect(res.status).toBe(302);
|
||||
expect(res.headers.get('location')).toBe('/');
|
||||
|
||||
process.env.CODEMAN_PASSWORD = savedPass;
|
||||
try {
|
||||
const res = await fetch(`${baseUrl}/q/ANYCODE`, { redirect: 'manual' });
|
||||
expect(res.status).toBe(302);
|
||||
expect(res.headers.get('location')).toBe('/');
|
||||
} finally {
|
||||
process.env.CODEMAN_PASSWORD = savedPass;
|
||||
}
|
||||
});
|
||||
|
||||
it('GET /api/tunnel/qr should return authEnabled flag', async () => {
|
||||
|
||||
Reference in New Issue
Block a user