fix(typescript): fail closed and add null-safety validation to McpSecurityScanner

agent-governance-typescript/src/mcp.ts

@@ -31,8 +31,8 @@ export interface McpScanResult {
 
 /** Minimal MCP tool definition accepted by the scanner. */
 export interface McpToolDefinition {
-  name: string;
-  description: string;
+  name?: string;
+  description?: string;
   parameters?: Record<string, unknown>;
 }
 
@@ -148,24 +148,40 @@ const SEVERITY_WEIGHT: Record<McpThreat['severity'], number> = {
 export class McpSecurityScanner {
   /** Scan a single tool definition. */
   scan(toolDefinition: McpToolDefinition): McpScanResult {
-    const threats: McpThreat[] = [];
-
-    this.detectToolPoisoning(toolDefinition, threats);
-    this.detectTyposquatting(toolDefinition, threats);
-    this.detectHiddenInstructions(toolDefinition, threats);
-    this.detectRugPull(toolDefinition, threats);
-
-    const risk_score = Math.min(
-      100,
-      threats.reduce((sum, t) => sum + SEVERITY_WEIGHT[t.severity], 0),
-    );
-
-    return {
-      tool_name: toolDefinition.name,
-      threats,
-      risk_score,
-      safe: threats.length === 0,
-    };
+    const toolName = toolDefinition?.name ?? 'unknown';
+    try {
+      const threats: McpThreat[] = [];
+
+      this.detectToolPoisoning(toolDefinition, threats);
+      this.detectTyposquatting(toolDefinition, threats);
+      this.detectHiddenInstructions(toolDefinition, threats);
+      this.detectRugPull(toolDefinition, threats);
+
+      const risk_score = Math.min(
+        100,
+        threats.reduce((sum, t) => sum + SEVERITY_WEIGHT[t.severity], 0),
+      );
+
+      return {
+        tool_name: toolName,
+        threats,
+        risk_score,
+        safe: threats.length === 0,
+      };
+    } catch (err) {
+      return {
+        tool_name: toolName,
+        threats: [
+          {
+            type: McpThreatType.ToolPoisoning,
+            severity: 'critical',
+            description: `Scan error — fail closed: ${err instanceof Error ? err.message : String(err)}`,
+          },
+        ],
+        risk_score: 100,
+        safe: false,
+      };
+    }
   }
 
   /** Scan multiple tool definitions. */
@@ -176,7 +192,7 @@ export class McpSecurityScanner {
   // ── Private detection methods ──
 
   private detectToolPoisoning(tool: McpToolDefinition, threats: McpThreat[]): void {
-    const text = tool.description;
+    const text = tool?.description ?? '';
     for (const pattern of POISONING_PATTERNS) {
       const match = pattern.exec(text);
       if (match) {
@@ -211,7 +227,9 @@ export class McpSecurityScanner {
   }
 
   private detectTyposquatting(tool: McpToolDefinition, threats: McpThreat[]): void {
-    const name = tool.name.toLowerCase();
+    const name = (tool?.name ?? '').toLowerCase();
+    if (!name) return;
+
     for (const known of KNOWN_TOOL_NAMES) {
       if (name === known) continue; // exact match is fine
       const dist = levenshtein(name, known);
@@ -222,12 +240,13 @@ export class McpSecurityScanner {
           description: `Tool name "${tool.name}" is suspiciously similar to known tool "${known}" (edit distance ${dist})`,
           evidence: known,
         });
+        break; // stop after first typosquatting match to prevent duplicate threat inflation
       }
     }
   }
 
   private detectHiddenInstructions(tool: McpToolDefinition, threats: McpThreat[]): void {
-    const text = tool.description;
+    const text = tool?.description ?? '';
 
     // Zero-width characters
     for (const zwc of ZERO_WIDTH_CHARS) {
@@ -257,7 +276,7 @@ export class McpSecurityScanner {
   }
 
   private detectRugPull(tool: McpToolDefinition, threats: McpThreat[]): void {
-    const text = tool.description;
+    const text = tool?.description ?? '';
     if (text.length <= RUG_PULL_DESCRIPTION_LENGTH) return;
 
     let instructionMatches = 0;

agent-governance-typescript/tests/mcp.test.ts

@@ -212,4 +212,54 @@ describe('McpSecurityScanner', () => {
       expect(result.risk_score).toBeGreaterThan(0);
     });
   });
+
+  // ── Edge cases and Fail-Closed ──
+
+  describe('null safety and fail-closed handling', () => {
+    it('handles tool definition with missing description without throwing', () => {
+      const tool: McpToolDefinition = {
+        name: 'read_file',
+      };
+      const result = scanner.scan(tool);
+      expect(result.safe).toBe(true);
+      expect(result.tool_name).toBe('read_file');
+    });
+
+    it('handles tool definition with missing name without throwing', () => {
+      const tool: McpToolDefinition = {
+        description: 'Reads a file.',
+      };
+      const result = scanner.scan(tool);
+      expect(result.safe).toBe(true);
+      expect(result.tool_name).toBe('unknown');
+    });
+
+    it('fails closed when scan method encounters an unexpected exception', () => {
+      const tool: McpToolDefinition = {
+        name: 'test_tool',
+        description: 'Test description',
+      };
+      // Force an error in internal detector
+      jest.spyOn(scanner as any, 'detectToolPoisoning').mockImplementation(() => {
+        throw new Error('Unexpected memory error');
+      });
+
+      const result = scanner.scan(tool);
+      expect(result.safe).toBe(false);
+      expect(result.risk_score).toBe(100);
+      expect(result.threats).toHaveLength(1);
+      expect(result.threats[0].severity).toBe('critical');
+      expect(result.threats[0].description).toContain('Scan error — fail closed');
+    });
+
+    it('emits at most one typosquatting threat per tool', () => {
+      const tool: McpToolDefinition = {
+        name: 'serch', // dist 1 from search, dist 2 from fetch
+        description: 'Web search tool',
+      };
+      const result = scanner.scan(tool);
+      const typosquats = result.threats.filter((t) => t.type === McpThreatType.Typosquatting);
+      expect(typosquats).toHaveLength(1);
+    });
+  });
 });