Address allowlist and enum fallback reviews

Author: HenkDz

src/server/connection-target.test.ts

@@ -32,6 +32,19 @@ describe('connection target allowlist helpers', () => {
     )).not.toThrow();
   });
 
+  it('rejects URL host override parameters before allowlist matching', () => {
+    const allowedTargets = normalizeAllowedConnectionTargets(['allowed.internal']);
+
+    expect(() => assertConnectionTargetAllowed(
+      'postgresql://allowed.internal/app?host=evil.internal',
+      allowedTargets
+    )).toThrow('host or hostaddr query parameters');
+    expect(() => assertConnectionTargetAllowed(
+      'postgresql://allowed.internal/app?hostaddr=10.0.0.1',
+      allowedTargets
+    )).toThrow('host or hostaddr query parameters');
+  });
+
   it('parses keyword-style connection targets', () => {
     expect(parseConnectionTarget("host=db.internal port=5432 dbname=app user='read only' password=secret")).toEqual({
       host: 'db.internal',

src/server/connection-target.ts

@@ -211,6 +211,9 @@ export function parseConnectionTarget(connectionString: string): ConnectionTarge
       if (!parsed.hostname) {
         throw new Error('Connection target allowlist requires URL connection strings to include an explicit host.');
       }
+      if (parsed.searchParams.has('host') || parsed.searchParams.has('hostaddr')) {
+        throw new Error('Connection target allowlist does not support URL host or hostaddr query parameters.');
+      }
 
       return {
         host: normalizeConnectionHost(parsed.hostname),
@@ -220,7 +223,7 @@ export function parseConnectionTarget(connectionString: string): ConnectionTarge
       };
     }
   } catch (error) {
-    if (error instanceof Error && error.message.includes('allowlist requires')) {
+    if (error instanceof Error && error.message.includes('allowlist')) {
       throw error;
     }
   }

src/tools/schema.test.ts

@@ -383,6 +383,27 @@ describe('manageSchemaTools', () => {
     expect(output).toContain('"active"');
   });
 
+  it('passes undefined connection strings through enum fallback paths', async () => {
+    const mockDb = {
+      connect: vi.fn().mockResolvedValue(undefined),
+      query: vi.fn()
+        .mockResolvedValueOnce([])
+        .mockResolvedValueOnce([]),
+      disconnect: vi.fn().mockResolvedValue(undefined)
+    };
+    vi.spyOn(DatabaseConnection, 'getInstance').mockReturnValue(mockDb as unknown as DatabaseConnection);
+
+    await manageSchemaTools.execute({ operation: 'get_enums' }, mockGetConnectionString);
+    await manageSchemaTools.execute({
+      operation: 'create_enum',
+      enumName: 'status',
+      values: ['active']
+    }, mockGetConnectionString);
+
+    expect(mockGetConnectionString).toHaveBeenNthCalledWith(1, undefined);
+    expect(mockGetConnectionString).toHaveBeenNthCalledWith(2, undefined);
+  });
+
   it('sanitizes schema database errors before returning them', async () => {
     const mockDb = {
       connect: vi.fn().mockResolvedValue(undefined),

src/tools/schema.ts

@@ -410,11 +410,11 @@ async function getTableInfo(db: DatabaseConnection, tableName: string, schema: s
 } 
 
 // Enum functions (adapted from enums.ts)
-async function executeGetEnumsInSchema(
-  connectionString: string,
-  schema = 'public',
-  enumName?: string,
-  getConnectionString?: GetConnectionStringFn
+async function executeGetEnumsInSchema(
+  connectionString: string | undefined,
+  schema = 'public',
+  enumName?: string,
+  getConnectionString?: GetConnectionStringFn
 ): Promise<EnumInfo[]> {
   const resolvedConnectionString = getConnectionString ? getConnectionString(connectionString) : connectionString;
   const db = DatabaseConnection.getInstance();
@@ -450,9 +450,9 @@ async function executeGetEnumsInSchema(
 }
 
 async function executeCreateEnumInSchema(
-  connectionString: string,
-  enumName: string,
-  values: string[],
+  connectionString: string | undefined,
+  enumName: string,
+  values: string[],
   schema = 'public',
   ifNotExists = false,
   getConnectionString?: GetConnectionStringFn
@@ -562,12 +562,12 @@ export const manageSchemaTools: PostgresTool = {
           return { content: [{ type: 'text', text: `Table ${result.tableName} altered successfully.` }, { type: 'text', text: JSON.stringify(result, null, 2) }] };
         }
 
-        case 'get_enums': {
-          const result = await executeGetEnumsInSchema(
-            connStringArg || '', 
-            schema || 'public', 
-            enumName, 
-            getConnectionStringVal
+        case 'get_enums': {
+          const result = await executeGetEnumsInSchema(
+            connStringArg,
+            schema || 'public', 
+            enumName, 
+            getConnectionStringVal
           );
           return { content: [{ type: 'text', text: `Fetched ${result.length} ENUM(s).` }, { type: 'text', text: JSON.stringify(result, null, 2) }] };
         }
@@ -578,12 +578,12 @@ export const manageSchemaTools: PostgresTool = {
               content: [{ type: 'text', text: 'Error: enumName and values are required for create_enum operation' }], 
               isError: true 
             };
-          }
-          const result = await executeCreateEnumInSchema(
-            connStringArg || '', 
-            enumName, 
-            values, 
-            schema || 'public', 
+          }
+          const result = await executeCreateEnumInSchema(
+            connStringArg,
+            enumName, 
+            values, 
+            schema || 'public', 
             ifNotExists || false, 
             getConnectionStringVal
           );