From eb362febd695b05d21acf9b5d64a1c2bb9ca1b87 Mon Sep 17 00:00:00 2001
From: czlonkowski <56956555+czlonkowski@users.noreply.github.com>
Date: Sun, 9 Nov 2025 17:27:57 +0100
Subject: [PATCH] test: Add critical missing tests for DISABLED_TOOLS feature
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add tests for two critical features identified by code review:

1. 10KB Safety Limit Test:
   - Verify DISABLED_TOOLS environment variable is truncated at 10KB
   - Test with 15KB input to ensure truncation works
   - Confirm first tools are parsed, last tools are excluded
   - Prevents DoS attacks from massive environment variables

2. Security Information Disclosure Test:
   - Verify error messages only reveal attempted tool name
   - Ensure full list of disabled tools is NOT leaked
   - Critical security test to prevent configuration disclosure
   - Tests defense against information leakage attacks

Test Coverage:
- Total tests: 47 (up from 45)
- Both tests passing
- Addresses critical gaps from code review

Files Modified:
- tests/unit/mcp/disabled-tools-additional.test.ts

Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 .../mcp/disabled-tools-additional.test.ts     | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/tests/unit/mcp/disabled-tools-additional.test.ts b/tests/unit/mcp/disabled-tools-additional.test.ts
index 044a9af..bd8d78d 100644
--- a/tests/unit/mcp/disabled-tools-additional.test.ts
+++ b/tests/unit/mcp/disabled-tools-additional.test.ts
@@ -287,6 +287,33 @@ describe('Disabled Tools Additional Coverage (Issue #410)', () => {
       expect(disabledTools.has('tool2')).toBe(true);
       expect(disabledTools.has('tool3')).toBe(true);
     });
+
+    it('should enforce 10KB limit on DISABLED_TOOLS environment variable', () => {
+      // Create a very long env var (15KB) by repeating tool names
+      const longTools = Array.from({ length: 1500 }, (_, i) => `tool_${i}`);
+      const longValue = longTools.join(',');
+
+      // Verify we created >10KB string
+      expect(longValue.length).toBeGreaterThan(10000);
+
+      process.env.DISABLED_TOOLS = longValue;
+      server = new TestableN8NMCPServer();
+
+      // Should succeed and truncate to 10KB
+      const disabledTools = server.testGetDisabledTools();
+
+      // Should have parsed some tools (at least the first ones)
+      expect(disabledTools.size).toBeGreaterThan(0);
+
+      // First few tools should be present (they're in the first 10KB)
+      expect(disabledTools.has('tool_0')).toBe(true);
+      expect(disabledTools.has('tool_1')).toBe(true);
+      expect(disabledTools.has('tool_2')).toBe(true);
+
+      // Last tools should NOT be present (they were truncated)
+      expect(disabledTools.has('tool_1499')).toBe(false);
+      expect(disabledTools.has('tool_1498')).toBe(false);
+    });
   });
 
   describe('Defense in Depth - Multiple Layers', () => {
@@ -332,6 +359,33 @@ describe('Disabled Tools Additional Coverage (Issue #410)', () => {
         expect(error.message).not.toContain('disabled via DISABLED_TOOLS');
       }
     });
+
+    it('should not leak list of disabled tools in error response', async () => {
+      // Set multiple disabled tools including some "secret" ones
+      process.env.DISABLED_TOOLS = 'secret_tool_1,secret_tool_2,secret_tool_3,attempted_tool';
+      server = new TestableN8NMCPServer();
+
+      // Try to execute one of the disabled tools
+      let errorMessage = '';
+      try {
+        await server.testExecuteTool('attempted_tool', {});
+      } catch (error: any) {
+        errorMessage = error.message;
+      }
+
+      // Error message should mention the attempted tool
+      expect(errorMessage).toContain('attempted_tool');
+      expect(errorMessage).toContain('disabled via DISABLED_TOOLS');
+
+      // Error message should NOT leak the other disabled tools
+      expect(errorMessage).not.toContain('secret_tool_1');
+      expect(errorMessage).not.toContain('secret_tool_2');
+      expect(errorMessage).not.toContain('secret_tool_3');
+
+      // Should not contain any arrays or lists
+      expect(errorMessage).not.toContain('[');
+      expect(errorMessage).not.toContain(']');
+    });
   });
 
   describe('Real-World Deployment Verification', () => {