Skip to content

Commit fe85ccf

Browse files
fix(security): address 5 critical code quality issues
- Replace `any` types with proper typed interfaces in frame-database.ts and frame-stack.ts (FrameRow, EventRow, AnchorRow, etc.) - Add safeJsonParse() to handle malformed JSON gracefully - Add depth limit to closeChildFrames() to prevent stack overflow - Fix ReDoS protection: pre-validate regex patterns before execution - Fix signature verification bypass: require TWILIO_AUTH_TOKEN in prod - Add parseCommandArgs() for proper shell argument parsing with quotes Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 3504654 commit fe85ccf

File tree

8 files changed

+316
-98
lines changed

8 files changed

+316
-98
lines changed

src/core/context/__tests__/frame-manager-cycle-detection.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,9 @@ describe('Frame Manager - Circular Reference Detection', () => {
195195
frameManager.updateParentFrame(frameB, null);
196196
}).not.toThrow();
197197

198-
// Verify B is now a root
198+
// Verify B is now a root (parent_frame_id is undefined for root frames)
199199
const updatedFrame = frameManager.getFrame(frameB);
200-
expect(updatedFrame?.parent_frame_id).toBeNull();
200+
expect(updatedFrame?.parent_frame_id).toBeUndefined();
201201
});
202202
});
203203

src/core/context/frame-database.ts

Lines changed: 104 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,65 @@ import { Frame, Event, Anchor } from './frame-types.js';
88
import { logger } from '../monitoring/logger.js';
99
import { DatabaseError, ErrorCode } from '../errors/index.js';
1010

11+
// Database row types for type-safe queries
12+
interface FrameRow {
13+
frame_id: string;
14+
run_id: string;
15+
project_id: string;
16+
parent_frame_id: string | null;
17+
depth: number;
18+
type: string;
19+
name: string;
20+
state: string;
21+
inputs: string;
22+
outputs: string;
23+
digest_text: string | null;
24+
digest_json: string;
25+
created_at: number;
26+
closed_at: number | null;
27+
}
28+
29+
interface EventRow {
30+
event_id: string;
31+
frame_id: string;
32+
run_id: string;
33+
seq: number;
34+
event_type: string;
35+
payload: string;
36+
ts: number;
37+
}
38+
39+
interface AnchorRow {
40+
anchor_id: string;
41+
frame_id: string;
42+
type: string;
43+
text: string;
44+
priority: number;
45+
metadata: string;
46+
created_at: number;
47+
}
48+
49+
interface CountRow {
50+
count: number;
51+
}
52+
53+
interface MaxSeqRow {
54+
max_seq: number | null;
55+
}
56+
57+
// Safe JSON parse with fallback
58+
function safeJsonParse<T>(json: string | null | undefined, fallback: T): T {
59+
if (!json) return fallback;
60+
try {
61+
return JSON.parse(json) as T;
62+
} catch {
63+
logger.warn('Failed to parse JSON, using fallback', {
64+
json: json.substring(0, 100),
65+
});
66+
return fallback;
67+
}
68+
}
69+
1170
export class FrameDatabase {
1271
constructor(private db: Database.Database) {}
1372

@@ -154,16 +213,20 @@ export class FrameDatabase {
154213
try {
155214
const row = this.db
156215
.prepare('SELECT * FROM frames WHERE frame_id = ?')
157-
.get(frameId) as any;
216+
.get(frameId) as FrameRow | undefined;
158217

159218
if (!row) return undefined;
160219

161220
return {
162221
...row,
163-
inputs: JSON.parse(row.inputs || '{}'),
164-
outputs: JSON.parse(row.outputs || '{}'),
165-
digest_json: JSON.parse(row.digest_json || '{}'),
166-
};
222+
parent_frame_id: row.parent_frame_id ?? undefined,
223+
inputs: safeJsonParse<Record<string, unknown>>(row.inputs, {}),
224+
outputs: safeJsonParse<Record<string, unknown>>(row.outputs, {}),
225+
digest_json: safeJsonParse<Record<string, unknown>>(
226+
row.digest_json,
227+
{}
228+
),
229+
} as Frame;
167230
} catch (error: unknown) {
168231
logger.warn(`Failed to get frame: ${frameId}`, { error });
169232
return undefined;
@@ -176,7 +239,7 @@ export class FrameDatabase {
176239
updateFrame(frameId: string, updates: Partial<Frame>): void {
177240
try {
178241
const setClauses: string[] = [];
179-
const values: any[] = [];
242+
const values: (string | number | null)[] = [];
180243

181244
if (updates.state !== undefined) {
182245
setClauses.push('state = ?');
@@ -252,14 +315,18 @@ export class FrameDatabase {
252315
: 'SELECT * FROM frames WHERE project_id = ? ORDER BY created_at';
253316

254317
const params = state ? [projectId, state] : [projectId];
255-
const rows = this.db.prepare(query).all(...params) as any[];
318+
const rows = this.db.prepare(query).all(...params) as FrameRow[];
256319

257-
return rows.map((row: any) => ({
320+
return rows.map((row) => ({
258321
...row,
259-
inputs: JSON.parse(row.inputs || '{}'),
260-
outputs: JSON.parse(row.outputs || '{}'),
261-
digest_json: JSON.parse(row.digest_json || '{}'),
262-
}));
322+
parent_frame_id: row.parent_frame_id ?? undefined,
323+
inputs: safeJsonParse<Record<string, unknown>>(row.inputs, {}),
324+
outputs: safeJsonParse<Record<string, unknown>>(row.outputs, {}),
325+
digest_json: safeJsonParse<Record<string, unknown>>(
326+
row.digest_json,
327+
{}
328+
),
329+
})) as Frame[];
263330
} catch (error: unknown) {
264331
throw new DatabaseError(
265332
`Failed to get frames for project: ${projectId}`,
@@ -304,12 +371,15 @@ export class FrameDatabase {
304371
// Return the created event with timestamp
305372
const createdEvent = this.db
306373
.prepare('SELECT * FROM events WHERE event_id = ?')
307-
.get(event.event_id) as any;
374+
.get(event.event_id) as EventRow;
308375

309376
return {
310377
...createdEvent,
311-
payload: JSON.parse(createdEvent.payload),
312-
};
378+
payload: safeJsonParse<Record<string, unknown>>(
379+
createdEvent.payload,
380+
{}
381+
),
382+
} as Event;
313383
} catch (error: unknown) {
314384
throw new DatabaseError(
315385
`Failed to insert event: ${event.event_id}`,
@@ -334,12 +404,12 @@ export class FrameDatabase {
334404
: 'SELECT * FROM events WHERE frame_id = ? ORDER BY seq ASC';
335405

336406
const params = limit ? [frameId, limit] : [frameId];
337-
const rows = this.db.prepare(query).all(...params) as any[];
407+
const rows = this.db.prepare(query).all(...params) as EventRow[];
338408

339-
return rows.map((row: any) => ({
409+
return rows.map((row) => ({
340410
...row,
341-
payload: JSON.parse(row.payload),
342-
}));
411+
payload: safeJsonParse<Record<string, unknown>>(row.payload, {}),
412+
})) as Event[];
343413
} catch (error: unknown) {
344414
throw new DatabaseError(
345415
`Failed to get events for frame: ${frameId}`,
@@ -357,7 +427,7 @@ export class FrameDatabase {
357427
try {
358428
const result = this.db
359429
.prepare('SELECT MAX(seq) as max_seq FROM events WHERE frame_id = ?')
360-
.get(frameId) as { max_seq: number | null };
430+
.get(frameId) as MaxSeqRow;
361431

362432
return (result.max_seq || 0) + 1;
363433
} catch (error: unknown) {
@@ -404,12 +474,15 @@ export class FrameDatabase {
404474
// Return the created anchor with timestamp
405475
const createdAnchor = this.db
406476
.prepare('SELECT * FROM anchors WHERE anchor_id = ?')
407-
.get(anchor.anchor_id) as any;
477+
.get(anchor.anchor_id) as AnchorRow;
408478

409479
return {
410480
...createdAnchor,
411-
metadata: JSON.parse(createdAnchor.metadata || '{}'),
412-
};
481+
metadata: safeJsonParse<Record<string, unknown>>(
482+
createdAnchor.metadata,
483+
{}
484+
),
485+
} as Anchor;
413486
} catch (error: unknown) {
414487
throw new DatabaseError(
415488
`Failed to insert anchor: ${anchor.anchor_id}`,
@@ -433,12 +506,12 @@ export class FrameDatabase {
433506
.prepare(
434507
'SELECT * FROM anchors WHERE frame_id = ? ORDER BY priority DESC, created_at ASC'
435508
)
436-
.all(frameId) as any[];
509+
.all(frameId) as AnchorRow[];
437510

438-
return rows.map((row: any) => ({
511+
return rows.map((row) => ({
439512
...row,
440-
metadata: JSON.parse(row.metadata || '{}'),
441-
}));
513+
metadata: safeJsonParse<Record<string, unknown>>(row.metadata, {}),
514+
})) as Anchor[];
442515
} catch (error: unknown) {
443516
throw new DatabaseError(
444517
`Failed to get anchors for frame: ${frameId}`,
@@ -477,16 +550,16 @@ export class FrameDatabase {
477550
try {
478551
const frameCount = this.db
479552
.prepare('SELECT COUNT(*) as count FROM frames')
480-
.get() as { count: number };
553+
.get() as CountRow;
481554
const eventCount = this.db
482555
.prepare('SELECT COUNT(*) as count FROM events')
483-
.get() as { count: number };
556+
.get() as CountRow;
484557
const anchorCount = this.db
485558
.prepare('SELECT COUNT(*) as count FROM anchors')
486-
.get() as { count: number };
559+
.get() as CountRow;
487560
const activeFrames = this.db
488561
.prepare("SELECT COUNT(*) as count FROM frames WHERE state = 'active'")
489-
.get() as { count: number };
562+
.get() as CountRow;
490563

491564
return {
492565
totalFrames: frameCount.count,

src/core/context/frame-stack.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -140,17 +140,17 @@ export class FrameStack {
140140
*/
141141
getStackFrames(): Frame[] {
142142
return this.activeStack
143-
.map((frameId: any) => this.frameDb.getFrame(frameId))
144-
.filter(Boolean) as Frame[];
143+
.map((frameId) => this.frameDb.getFrame(frameId))
144+
.filter((f): f is Frame => f !== undefined);
145145
}
146146

147147
/**
148148
* Get frame context for the hot stack
149149
*/
150150
getHotStackContext(maxEvents: number = 20): FrameContext[] {
151151
return this.activeStack
152-
.map((frameId: any) => this.buildFrameContext(frameId, maxEvents))
153-
.filter(Boolean) as FrameContext[];
152+
.map((frameId) => this.buildFrameContext(frameId, maxEvents))
153+
.filter((ctx): ctx is FrameContext => ctx !== null);
154154
}
155155

156156
/**
@@ -303,11 +303,11 @@ export class FrameStack {
303303
/**
304304
* Extract constraints from frame inputs
305305
*/
306-
private extractConstraints(inputs: Record<string, any>): string[] {
306+
private extractConstraints(inputs: Record<string, unknown>): string[] {
307307
const constraints: string[] = [];
308308

309309
if (inputs.constraints && Array.isArray(inputs.constraints)) {
310-
constraints.push(...inputs.constraints);
310+
constraints.push(...(inputs.constraints as string[]));
311311
}
312312

313313
return constraints;
@@ -316,12 +316,13 @@ export class FrameStack {
316316
/**
317317
* Extract active artifacts from events
318318
*/
319-
private extractActiveArtifacts(events: any[]): string[] {
319+
private extractActiveArtifacts(events: Event[]): string[] {
320320
const artifacts: string[] = [];
321321

322322
for (const event of events) {
323-
if (event.event_type === 'artifact' && event.payload?.path) {
324-
artifacts.push(event.payload.path);
323+
const payload = event.payload as Record<string, unknown>;
324+
if (event.event_type === 'artifact' && payload?.path) {
325+
artifacts.push(payload.path as string);
325326
}
326327
}
327328

@@ -350,7 +351,7 @@ export class FrameStack {
350351

351352
// Find root frames (no parent or parent not in active set)
352353
const rootFrames = frames.filter(
353-
(f: any) => !f.parent_frame_id || !frameMap.has(f.parent_frame_id)
354+
(f) => !f.parent_frame_id || !frameMap.has(f.parent_frame_id)
354355
);
355356

356357
if (rootFrames.length === 0) {
@@ -360,23 +361,22 @@ export class FrameStack {
360361

361362
if (rootFrames.length > 1) {
362363
logger.warn('Multiple root frames found, using most recent', {
363-
rootFrames: rootFrames.map((f: any) => f.frame_id),
364+
rootFrames: rootFrames.map((f) => f.frame_id),
364365
});
365366
}
366367

367368
// Build stack from root to leaves
368369
const stack: string[] = [];
369-
let currentFrame = rootFrames.sort(
370+
let currentFrame: Frame | undefined = rootFrames.sort(
370371
(a, b) => a.created_at - b.created_at
371372
)[0];
372373

373374
while (currentFrame) {
374375
stack.push(currentFrame.frame_id);
375376

376377
// Find child frame
377-
const childFrame = frames.find(
378-
(f: any) => f.parent_frame_id === currentFrame.frame_id
379-
);
378+
const parentId = currentFrame.frame_id;
379+
const childFrame = frames.find((f) => f.parent_frame_id === parentId);
380380
if (childFrame) {
381381
currentFrame = childFrame;
382382
} else {

0 commit comments

Comments
 (0)