test: Add critical missing tests for DISABLED_TOOLS feature

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>
This commit is contained in:
czlonkowski
2025-11-09 17:27:57 +01:00
parent 821ace310e
commit eb362febd6

View File

@@ -287,6 +287,33 @@ describe('Disabled Tools Additional Coverage (Issue #410)', () => {
expect(disabledTools.has('tool2')).toBe(true); expect(disabledTools.has('tool2')).toBe(true);
expect(disabledTools.has('tool3')).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', () => { 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'); 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', () => { describe('Real-World Deployment Verification', () => {