diff --git a/.jules/bolt.md b/.jules/bolt.md index f1a8c146..97c2ca2e 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -77,3 +77,6 @@ Optimized metric route processing to O(N) by creating a mapping of routes direct ## 2024-07-13 - [Optimize Export Dictionary FK lookups] **Learning:** Found O(N * C * E) performance bottleneck in ERD export dictionaries due to repeated array searching with `edges.some()` inside a nested loop over nodes and columns. **Action:** Replace repeated linear array scans for edges by precomputing O(1) Set lookups of foreign key column handles per node before looping. +## 2026-08-07 - Optimize string-based node lookups using handle decoding +**Learning:** We can reduce (N)$ string encoding loops in FK edge column lookups to (1)$ directly by decoding the parsed handle id to extract column names directly without generating garbage. +**Action:** Always parse handles directly to resolve elements in edge loops if possible, rather than scanning the node lists to string encode. diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 1c145b3a..a59ee2a1 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -2,3 +2,7 @@ **Vulnerability:** User-provided string fields (like project and connection names) lacked strict validation against control characters, only relying on length constraints. **Learning:** This could potentially lead to Log Injection (CRLF injection), Null Byte Injection, or terminal escape injection if these strings are subsequently logged or rendered directly. **Prevention:** Use explicit regex validation `pattern=r'^[^\x00-\x1F\x7F]+$'` on Pydantic string fields to strictly reject control characters. +## 2026-08-07 - Mitigating DoS in String Split Operations +**Vulnerability:** Unbounded string allocations during parsing via `split` and `map` (like decoding handle strings into columns) can cause high memory allocation spikes and denial of service (DoS) when fed excessively long strings. +**Learning:** Functions that parse handles should implement reasonable length bounds on inputs before allocating intermediate arrays via `split` and `map`. +**Prevention:** Implement an explicit maximum length check (e.g. `if (str.length > 512) return null;`) before parsing string identifiers to prevent buffer exhaustion. diff --git a/frontend/src/erd/__tests__/handleLookupRegression.test.ts b/frontend/src/erd/__tests__/handleLookupRegression.test.ts new file mode 100644 index 00000000..84d600cf --- /dev/null +++ b/frontend/src/erd/__tests__/handleLookupRegression.test.ts @@ -0,0 +1,65 @@ +import type { Edge, Node } from '@xyflow/react'; +import { describe, expect, it } from 'vitest'; + +import type { TableNodeData } from '../convert'; +import { exportDDL } from '../export'; +import { parseColumnNameFromHandle, sourceColumnHandleId, targetColumnHandleId } from '../handleUtils'; + +function tableNode(id: string, title: string, columns: TableNodeData['columns']): Node { + return { + id, + type: 'tableNode', + position: { x: 0, y: 0 }, + data: { title, columns, badges: { pk: columns.some((column) => column.is_pk), fk: false } }, + }; +} + +describe('foreign-key handle lookup regressions', () => { + it('rejects malformed, oversized, and invalid-code-point handles', () => { + expect(parseColumnNameFromHandle('src-c-0069junk-0064')).toBeNull(); + expect(parseColumnNameFromHandle(`src-c-${'0069-'.repeat(150)}`)).toBeNull(); + expect(parseColumnNameFromHandle('src-c-110000')).toBeNull(); + }); + + it('indexes node columns once rather than rescanning for each edge', () => { + let reads = 0; + const sourceColumn = { + get column_name() { reads += 1; return 'user_id'; }, + data_type: 'integer', is_not_null: true, is_pk: false, + }; + const targetColumn = { + get column_name() { reads += 1; return 'id'; }, + data_type: 'integer', is_not_null: true, is_pk: true, + }; + const sourceNode = tableNode('posts', 'public.posts', [sourceColumn]); + const targetNode = tableNode('users', 'public.users', [targetColumn]); + const edges: Edge[] = Array.from({ length: 100 }, (_, index) => ({ + id: `fk_${index}`, + source: sourceNode.id, + target: targetNode.id, + sourceHandle: sourceColumnHandleId('user_id'), + targetHandle: targetColumnHandleId('id'), + })); + + const ddl = exportDDL([sourceNode, targetNode], edges); + + expect(ddl).toContain('FOREIGN KEY ("user_id")'); + expect(reads).toBeLessThan(20); + }); + + it('preserves the supported empty-column handle encoding', () => { + const sourceNode = tableNode('source', 'public.source', [ + { column_name: '', data_type: 'integer', is_not_null: true, is_pk: true }, + ]); + const targetNode = tableNode('target', 'public.target', [ + { column_name: 'id', data_type: 'integer', is_not_null: true, is_pk: true }, + ]); + const ddl = exportDDL([sourceNode, targetNode], [{ + id: 'fk_empty_handle', source: sourceNode.id, target: targetNode.id, + sourceHandle: 'src-c-empty', targetHandle: targetColumnHandleId('id'), + }]); + + expect(ddl).toContain('FOREIGN KEY ("unnamed")'); + expect(ddl).not.toContain('/* source columns */'); + }); +}); diff --git a/frontend/src/erd/export.ts b/frontend/src/erd/export.ts index 62ce7219..f0d8233b 100644 --- a/frontend/src/erd/export.ts +++ b/frontend/src/erd/export.ts @@ -2,7 +2,7 @@ import type { Node, Edge } from '@xyflow/react'; import { normalizeBusinessGroupColor } from './businessGroups'; import type { IndexRecommendation } from './cardinality'; import type { ForeignKeyEdgeData, TableNodeData } from './convert'; -import { sourceColumnHandleId, targetColumnHandleId } from './handleUtils'; +import { parseColumnNameFromHandle } from './handleUtils'; export * from './exportDataDictionary'; @@ -59,6 +59,8 @@ function fkColumnsForEdge( edge: Edge, sourceNode: Node, targetNode: Node, + sourceColumnNames: ReadonlySet, + targetColumnNames: ReadonlySet, ): { sourceColumns: string[]; targetColumns: string[] } | null { const data = edge.data as ForeignKeyEdgeData | undefined; const sourceColumns = data?.sourceColumns?.filter(Boolean) || []; @@ -67,13 +69,14 @@ function fkColumnsForEdge( return { sourceColumns, targetColumns }; } - const sourceHandleColumn = (sourceNode.data.columns || []) - .find((column) => sourceColumnHandleId(column.column_name) === edge.sourceHandle) - ?.column_name; - const targetHandleColumn = (targetNode.data.columns || []) - .find((column) => targetColumnHandleId(column.column_name) === edge.targetHandle) - ?.column_name; - if (sourceHandleColumn && targetHandleColumn) { + const sourceHandleColumn = parseColumnNameFromHandle(edge.sourceHandle); + const targetHandleColumn = parseColumnNameFromHandle(edge.targetHandle); + if ( + sourceHandleColumn !== null && + targetHandleColumn !== null && + sourceColumnNames.has(sourceHandleColumn) && + targetColumnNames.has(targetHandleColumn) + ) { return { sourceColumns: [sourceHandleColumn], targetColumns: [targetHandleColumn] }; } @@ -96,8 +99,13 @@ export function exportDDL(nodes: Node[], edges: Edge[]): string { // Bolt: Use map for O(1) node lookup instead of O(N) array find // Avoid Map(array.map) to prevent O(N) intermediate tuple array allocation overhead const nodesById = new Map>(); + const columnNamesByNodeId = new Map>(); for (const n of nodes) { nodesById.set(n.id, n); + columnNamesByNodeId.set( + n.id, + new Set((n.data.columns || []).map((column) => column.column_name)), + ); } // Export tables @@ -133,7 +141,13 @@ export function exportDDL(nodes: Node[], edges: Edge[]): string { const targetNode = nodesById.get(edge.target); if (sourceNode && targetNode) { - const fkColumns = fkColumnsForEdge(edge, sourceNode, targetNode); + const fkColumns = fkColumnsForEdge( + edge, + sourceNode, + targetNode, + columnNamesByNodeId.get(sourceNode.id)!, + columnNamesByNodeId.get(targetNode.id)!, + ); const constraintName = edge.label ? edge.label : `fk_${edge.source}_${edge.target}`; const sourceTable = quoteSqlIdentifier(sourceNode.data.title || sourceNode.id); const targetTable = quoteSqlIdentifier(targetNode.data.title || targetNode.id); diff --git a/frontend/src/erd/handleUtils.test.ts b/frontend/src/erd/handleUtils.test.ts index 0278739e..0f655bb6 100644 --- a/frontend/src/erd/handleUtils.test.ts +++ b/frontend/src/erd/handleUtils.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { sanitizeHandleId, sourceColumnHandleId, targetColumnHandleId } from './handleUtils'; +import { sanitizeHandleId, sourceColumnHandleId, targetColumnHandleId, parseColumnNameFromHandle } from './handleUtils'; describe('handleUtils', () => { describe('sanitizeHandleId', () => { @@ -22,6 +22,17 @@ describe('handleUtils', () => { it('should handle emojis', () => { expect(sanitizeHandleId('id_🚀')).toBe('c-0069-0064-005f-1f680'); }); + + it.each([ + ['id123', 'c-0069-0064-0031-0032-0033'], + ['user id', 'c-0075-0073-0065-0072-0020-0069-0064'], + ['!@#$%', 'c-0021-0040-0023-0024-0025'], + ['\n\t', 'c-000a-0009'], + ['e\u0301', 'c-0065-0301'], + ['👨‍👩‍👦', 'c-1f468-200d-1f469-200d-1f466'], + ])('encodes every Unicode scalar in %j', (input, expected) => { + expect(sanitizeHandleId(input)).toBe(expected); + }); }); describe('sourceColumnHandleId', () => { @@ -35,4 +46,21 @@ describe('handleUtils', () => { expect(targetColumnHandleId('id')).toBe('tgt-c-0069-0064'); }); }); + + describe('parseColumnNameFromHandle', () => { + it('decodes simple ascii', () => { + expect(parseColumnNameFromHandle('src-c-0069-0064')).toBe('id'); + }); + it('decodes empty', () => { + expect(parseColumnNameFromHandle('src-c-empty')).toBe(''); + }); + it('returns null for bad formats', () => { + expect(parseColumnNameFromHandle('bad-format')).toBe(null); + expect(parseColumnNameFromHandle(null as any)).toBe(null); + expect(parseColumnNameFromHandle(undefined as any)).toBe(null); + }); + it('returns null for excessively long strings to prevent DoS', () => { + expect(parseColumnNameFromHandle('src-c-' + '0069-'.repeat(150))).toBe(null); + }); + }); }); diff --git a/frontend/src/erd/handleUtils.ts b/frontend/src/erd/handleUtils.ts index 054d5ab2..d055e872 100644 --- a/frontend/src/erd/handleUtils.ts +++ b/frontend/src/erd/handleUtils.ts @@ -14,3 +14,24 @@ export function sourceColumnHandleId(columnName: string): string { export function targetColumnHandleId(columnName: string): string { return `tgt-${sanitizeHandleId(columnName)}` } + +export function parseColumnNameFromHandle(handleId: string | undefined | null): string | null { + if (!handleId || handleId.length > 512) return null; + + const encoded = handleId.startsWith('src-') || handleId.startsWith('tgt-') + ? handleId.slice(4) + : handleId; + if (!encoded.startsWith('c-')) return null; + + const codePoints = encoded.slice(2); + if (codePoints === 'empty') return ''; + + const parts = codePoints.split('-'); + if (!parts.every((part) => /^[0-9a-f]{4,6}$/i.test(part))) return null; + + try { + return parts.map((part) => String.fromCodePoint(Number.parseInt(part, 16))).join(''); + } catch { + return null; + } +}