Skip to content

Commit 8c815ac

Browse files
committed
Fix security vulnerabilities in Node.js driver
Fix security vulnerabilities in Node.js driver and harden input validation and query construction. Note: This PR was created with AI tools and a human. - Add input validation for graph names, label names, and column names to prevent SQL injection via string interpolation. Graph name rules are based on Apache AGE's naming conventions and Neo4j/openCypher compatibility (hyphens and dots permitted, min 3 chars, max 63 chars). Label names follow AGE's stricter rules (no hyphens or dots). - Add design documentation noting intentional ASCII-only restriction in driver-side regex validation as a security hardening measure (homoglyph/encoding attack surface reduction). AGE's server-side validation (name_validation.h) uses full Unicode ID_Start/ID_Continue and remains the authoritative check for Unicode names. - Add safe query helpers: queryCypher(), createGraph(), dropGraph() with graph name validation and dollar-quoting for Cypher strings - Add runtime typeof check on dropGraph cascade parameter to prevent injection from plain JavaScript consumers (TypeScript types are erased at runtime) - Use BigInt for integer values exceeding Number.MAX_SAFE_INTEGER to prevent silent precision loss with 64-bit AGE graph IDs - Make CREATE EXTENSION opt-in via SetAGETypesOptions.createExtension instead of running DDL automatically without user consent - Wrap LOAD/search_path setup in try/catch with actionable error message mentioning CREATE EXTENSION and { createExtension: true } - Improve agtype-not-found error message with installation guidance - Tighten pg dependency from >=6.0.0 to >=8.0.0 - Add comprehensive test suites covering validation, SQL injection prevention, cascade type safety, hyphenated graph name integration, BigInt parsing, and setAGETypes error handling - Add design note in tests documenting why createExtension: false is the correct default (CI image has AGE pre-installed, auto-creating extensions requires SUPERUSER and conflates concerns) modified: drivers/nodejs/package.json modified: drivers/nodejs/src/antlr4/CustomAgTypeListener.ts modified: drivers/nodejs/src/index.ts modified: drivers/nodejs/test/Agtype.test.ts modified: drivers/nodejs/test/index.test.ts
1 parent 5fe2121 commit 8c815ac

5 files changed

Lines changed: 539 additions & 82 deletions

File tree

drivers/nodejs/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
"author": "Alex Kwak <take-me-home@kakao.com>",
3131
"dependencies": {
3232
"antlr4ts": "^0.5.0-alpha.4",
33-
"pg": ">=6.0.0"
33+
"pg": ">=8.0.0"
3434
},
3535
"devDependencies": {
3636
"@types/jest": "^29.5.14",

drivers/nodejs/src/antlr4/CustomAgTypeListener.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,11 @@ class CustomAgTypeListener implements AgtypeListener, ParseTreeListener {
9292
}
9393

9494
exitIntegerValue (ctx: IntegerValueContext): void {
95-
const value = parseInt(ctx.text)
95+
// Use BigInt for values that exceed Number.MAX_SAFE_INTEGER to
96+
// prevent silent precision loss with large AGE graph IDs (64-bit).
97+
const text = ctx.text
98+
const num = Number(text)
99+
const value = Number.isSafeInteger(num) ? num : BigInt(text)
96100
if (!this.pushIfArray(value)) {
97101
this.lastValue = value
98102
}

drivers/nodejs/src/index.ts

Lines changed: 288 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,130 @@
1717
* under the License.
1818
*/
1919

20-
import { Client } from 'pg'
20+
import { Client, QueryResult, QueryResultRow } from 'pg'
2121
import pgTypes from 'pg-types'
2222
import { CharStreams, CommonTokenStream } from 'antlr4ts'
2323
import { AgtypeLexer } from './antlr4/AgtypeLexer'
2424
import { AgtypeParser } from './antlr4/AgtypeParser'
2525
import CustomAgTypeListener from './antlr4/CustomAgTypeListener'
2626
import { ParseTreeWalker } from 'antlr4ts/tree'
2727

28+
/**
29+
* Valid graph name pattern based on Apache AGE's naming conventions
30+
* (Neo4j/openCypher compatible). Allows ASCII letters, digits,
31+
* underscores, plus dots and hyphens in middle positions.
32+
*
33+
* DESIGN NOTE: AGE's server-side validation (name_validation.h) uses
34+
* full Unicode ID_Start/ID_Continue character classes, accepting names
35+
* like 'mydätabase' or 'mydঅtabase'. This driver intentionally restricts
36+
* to ASCII-only as a security hardening measure — it reduces the attack
37+
* surface for homoglyph and encoding-based injection vectors. Server-side
38+
* validation remains the authoritative check for Unicode names.
39+
*
40+
* Start: ASCII letter or underscore
41+
* Middle: ASCII letter, digit, underscore, dot, or hyphen
42+
* End: ASCII letter, digit, or underscore
43+
*/
44+
const VALID_GRAPH_NAME = /^[A-Za-z_][A-Za-z0-9_.\-]*[A-Za-z0-9_]$/
45+
46+
/**
47+
* Valid label name pattern based on Apache AGE's naming rules.
48+
* Labels follow stricter identifier rules than graph names — dots and
49+
* hyphens are NOT permitted in label names.
50+
*
51+
* Note: ASCII-only restriction (see VALID_GRAPH_NAME design note above).
52+
*/
53+
const VALID_LABEL_NAME = /^[A-Za-z_][A-Za-z0-9_]*$/
54+
55+
/**
56+
* Valid SQL identifier pattern for column names and types in the
57+
* query result AS clause. Standard PostgreSQL unquoted identifier rules.
58+
*
59+
* Note: ASCII-only restriction (see VALID_GRAPH_NAME design note above).
60+
*/
61+
const VALID_SQL_IDENTIFIER = /^[A-Za-z_][A-Za-z0-9_]*$/
62+
63+
/**
64+
* Validates that a graph name conforms to Apache AGE's naming conventions
65+
* and is safe for use in SQL queries.
66+
*
67+
* Graph names must:
68+
* - Be at least 3 characters and at most 63 characters
69+
* - Start with an ASCII letter or underscore
70+
* - Contain only ASCII letters, digits, underscores, dots, and hyphens
71+
* - End with an ASCII letter, digit, or underscore
72+
*
73+
* Note: This is intentionally stricter than AGE's server-side validation
74+
* (which accepts Unicode letters). See VALID_GRAPH_NAME design note.
75+
*
76+
* @param graphName - The graph name to validate
77+
* @throws Error if the graph name is invalid
78+
*/
79+
function validateGraphName (graphName: string): void {
80+
if (!graphName || typeof graphName !== 'string') {
81+
throw new Error('Graph name must be a non-empty string')
82+
}
83+
if (graphName.length < 3) {
84+
throw new Error(
85+
`Invalid graph name: '${graphName}'. Graph names must be at least 3 characters.`
86+
)
87+
}
88+
if (graphName.length > 63) {
89+
throw new Error('Graph name must not exceed 63 characters (PostgreSQL name limit)')
90+
}
91+
if (!VALID_GRAPH_NAME.test(graphName)) {
92+
throw new Error(
93+
`Invalid graph name: '${graphName}'. Graph names must start with a letter ` +
94+
'or underscore, may contain letters, digits, underscores, dots, and hyphens, ' +
95+
'and must end with a letter, digit, or underscore.'
96+
)
97+
}
98+
}
99+
100+
/**
101+
* Validates that a label name conforms to Apache AGE's naming rules.
102+
* Label names are stricter than graph names — only letters, digits,
103+
* and underscores are permitted (no dots or hyphens).
104+
*
105+
* @param labelName - The label name to validate
106+
* @throws Error if the label name is invalid
107+
*/
108+
function validateLabelName (labelName: string): void {
109+
if (!labelName || typeof labelName !== 'string') {
110+
throw new Error('Label name must be a non-empty string')
111+
}
112+
if (labelName.length > 63) {
113+
throw new Error('Label name must not exceed 63 characters (PostgreSQL name limit)')
114+
}
115+
if (!VALID_LABEL_NAME.test(labelName)) {
116+
throw new Error(
117+
`Invalid label name: '${labelName}'. Label names must start with a letter ` +
118+
'or underscore and contain only letters, digits, and underscores.'
119+
)
120+
}
121+
}
122+
123+
/**
124+
* Escapes a PostgreSQL dollar-quoted string literal by ensuring the
125+
* cypher query does not contain the dollar-quote delimiter. If the
126+
* default $$ delimiter conflicts, a unique tagged delimiter is used.
127+
*
128+
* @param cypher - The Cypher query string
129+
* @returns An object with the delimiter and safely quoted string
130+
*/
131+
function cypherDollarQuote (cypher: string): { delimiter: string; quoted: string } {
132+
if (!cypher.includes('$$')) {
133+
return { delimiter: '$$', quoted: `$$${cypher}$$` }
134+
}
135+
// Generate a unique tag that doesn't appear in the cypher query
136+
let tag = 'cypher'
137+
let counter = 0
138+
while (cypher.includes(`$${tag}$`)) {
139+
tag = `cypher${counter++}`
140+
}
141+
return { delimiter: `$${tag}$`, quoted: `$${tag}$${cypher}$${tag}$` }
142+
}
143+
28144
function AGTypeParse (input: string) {
29145
const chars = CharStreams.fromString(input)
30146
const lexer = new AgtypeLexer(chars)
@@ -36,21 +152,180 @@ function AGTypeParse (input: string) {
36152
return printer.getResult()
37153
}
38154

39-
async function setAGETypes (client: Client, types: typeof pgTypes) {
40-
await client.query(`
41-
CREATE EXTENSION IF NOT EXISTS age;
42-
LOAD 'age';
43-
SET search_path = ag_catalog, "$user", public;
44-
`)
155+
/**
156+
* Options for setAGETypes configuration.
157+
*/
158+
interface SetAGETypesOptions {
159+
/**
160+
* If true, will attempt to CREATE EXTENSION IF NOT EXISTS age.
161+
* Defaults to false. Set to true only if the connected user has
162+
* sufficient privileges.
163+
*/
164+
createExtension?: boolean
165+
}
166+
167+
/**
168+
* Configures the pg client to properly parse AGE agtype results.
169+
*
170+
* This function:
171+
* 1. Loads the AGE extension into the session
172+
* 2. Sets the search_path to include ag_catalog
173+
* 3. Registers the agtype type parser
174+
*
175+
* @param client - A connected pg Client instance
176+
* @param types - The pg types module for registering type parsers
177+
* @param options - Optional configuration settings
178+
* @throws Error if AGE extension is not installed or agtype is not found
179+
*/
180+
async function setAGETypes (client: Client, types: typeof pgTypes, options?: SetAGETypesOptions) {
181+
const createExtension = options?.createExtension ?? false
182+
183+
if (createExtension) {
184+
await client.query('CREATE EXTENSION IF NOT EXISTS age;')
185+
}
186+
187+
try {
188+
await client.query("LOAD 'age';")
189+
await client.query('SET search_path = ag_catalog, "$user", public;')
190+
} catch (err: unknown) {
191+
const msg = err instanceof Error ? err.message : String(err)
192+
throw new Error(
193+
`Failed to load AGE extension: ${msg}. ` +
194+
'Ensure the AGE extension is installed in the database. ' +
195+
'You may need to run CREATE EXTENSION age; first, or pass ' +
196+
'{ createExtension: true } to setAGETypes().'
197+
)
198+
}
45199

46-
const oidResults = await client.query(`
47-
select typelem
48-
from pg_type
49-
where typname = '_agtype';`)
200+
const oidResults = await client.query(
201+
"SELECT typelem FROM pg_type WHERE typname = '_agtype';"
202+
)
50203

51-
if (oidResults.rows.length < 1) { throw new Error() }
204+
if (oidResults.rows.length < 1) {
205+
throw new Error(
206+
'AGE agtype type not found. Ensure the AGE extension is installed ' +
207+
'and loaded in the current database. Run CREATE EXTENSION age; first, ' +
208+
'or pass { createExtension: true } to setAGETypes().'
209+
)
210+
}
52211

53212
types.setTypeParser(oidResults.rows[0].typelem, AGTypeParse)
54213
}
55214

56-
export { setAGETypes, AGTypeParse }
215+
/**
216+
* Column definition for Cypher query results.
217+
*/
218+
interface CypherColumn {
219+
/** Column alias name */
220+
name: string
221+
/** Column type (defaults to 'agtype') */
222+
type?: string
223+
}
224+
225+
/**
226+
* Executes a Cypher query safely against an AGE graph.
227+
*
228+
* This function validates the graph name to prevent SQL injection,
229+
* properly quotes the Cypher query using dollar-quoting, and builds
230+
* the required SQL wrapper.
231+
*
232+
* @param client - A connected pg Client instance (with AGE types set)
233+
* @param graphName - The target graph name (must be a valid AGE graph name)
234+
* @param cypher - The Cypher query string
235+
* @param columns - Column definitions for the result set
236+
* @returns The query result
237+
* @throws Error if graphName is invalid or query fails
238+
*
239+
* @example
240+
* ```typescript
241+
* const result = await queryCypher(
242+
* client,
243+
* 'my_graph',
244+
* 'MATCH (n:Person) WHERE n.name = $name RETURN n',
245+
* [{ name: 'n' }]
246+
* );
247+
* ```
248+
*/
249+
async function queryCypher<T extends QueryResultRow = any> (
250+
client: Client,
251+
graphName: string,
252+
cypher: string,
253+
columns: CypherColumn[] = [{ name: 'result' }]
254+
): Promise<QueryResult<T>> {
255+
// Validate graph name against injection
256+
validateGraphName(graphName)
257+
258+
// Validate column definitions
259+
if (!columns || columns.length === 0) {
260+
throw new Error('At least one column definition is required')
261+
}
262+
263+
for (const col of columns) {
264+
if (!col.name || typeof col.name !== 'string') {
265+
throw new Error('Column name must be a non-empty string')
266+
}
267+
// Column names must be valid SQL identifiers
268+
if (!VALID_SQL_IDENTIFIER.test(col.name)) {
269+
throw new Error(
270+
`Invalid column name: '${col.name}'. Column names must be valid SQL identifiers.`
271+
)
272+
}
273+
if (col.type && !VALID_SQL_IDENTIFIER.test(col.type)) {
274+
throw new Error(
275+
`Invalid column type: '${col.type}'. Column types must be valid SQL type identifiers.`
276+
)
277+
}
278+
}
279+
280+
// Build column list safely
281+
const columnList = columns
282+
.map(col => `${col.name} ${col.type ?? 'agtype'}`)
283+
.join(', ')
284+
285+
// Safely dollar-quote the cypher query
286+
const { quoted } = cypherDollarQuote(cypher)
287+
288+
const sql = `SELECT * FROM cypher('${graphName}', ${quoted}) AS (${columnList});`
289+
290+
return client.query<T>(sql)
291+
}
292+
293+
/**
294+
* Creates a new graph safely.
295+
*
296+
* @param client - A connected pg Client instance
297+
* @param graphName - Name for the new graph (must be a valid AGE graph name)
298+
* @throws Error if graphName is invalid
299+
*/
300+
async function createGraph (client: Client, graphName: string): Promise<void> {
301+
validateGraphName(graphName)
302+
await client.query(`SELECT * FROM ag_catalog.create_graph('${graphName}');`)
303+
}
304+
305+
/**
306+
* Drops an existing graph safely.
307+
*
308+
* @param client - A connected pg Client instance
309+
* @param graphName - Name of the graph to drop (must be a valid AGE graph name)
310+
* @param cascade - If true, drop dependent objects (default: false)
311+
* @throws Error if graphName is invalid
312+
*/
313+
async function dropGraph (client: Client, graphName: string, cascade: boolean = false): Promise<void> {
314+
validateGraphName(graphName)
315+
if (typeof cascade !== 'boolean') {
316+
throw new Error('cascade parameter must be a boolean')
317+
}
318+
await client.query(`SELECT * FROM ag_catalog.drop_graph('${graphName}', ${cascade});`)
319+
}
320+
321+
export {
322+
setAGETypes,
323+
AGTypeParse,
324+
queryCypher,
325+
createGraph,
326+
dropGraph,
327+
validateGraphName,
328+
validateLabelName,
329+
SetAGETypesOptions,
330+
CypherColumn
331+
}

drivers/nodejs/test/Agtype.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,31 @@ describe('Parsing', () => {
101101
}))
102102
})))
103103
})
104+
105+
it('Large integer uses BigInt when exceeding MAX_SAFE_INTEGER', () => {
106+
// 2^53 = 9007199254740992 exceeds MAX_SAFE_INTEGER (2^53 - 1)
107+
const result = AGTypeParse('{"id": 9007199254740993, "label": "test", "properties": {}}::vertex')
108+
const id = (result as Map<string, any>).get('id')
109+
expect(typeof id).toBe('bigint')
110+
expect(id).toBe(BigInt('9007199254740993'))
111+
})
112+
113+
it('Safe integers remain as Number type', () => {
114+
const result = AGTypeParse('{"id": 844424930131969, "label": "test", "properties": {}}::vertex')
115+
const id = (result as Map<string, any>).get('id')
116+
expect(typeof id).toBe('number')
117+
expect(id).toBe(844424930131969)
118+
})
119+
120+
it('Small integers remain as Number type', () => {
121+
const result = AGTypeParse('42')
122+
expect(typeof result).toBe('number')
123+
expect(result).toBe(42)
124+
})
125+
126+
it('Negative integers parsed correctly', () => {
127+
const result = AGTypeParse('-100')
128+
expect(typeof result).toBe('number')
129+
expect(result).toBe(-100)
130+
})
104131
})

0 commit comments

Comments
 (0)