Skip to content

AUDIT-2025-01-13.md

Latest commit

 

History

History
500 lines (384 loc) · 14.5 KB

AUDIT-2025-01-13.md

File metadata and controls

500 lines (384 loc) · 14.5 KB

MCP Best Practices Audit Report

Date: 2025-01-13
Project: boilerplate-mcp-server v1.18.0
Auditor: AI Agent (based on official MCP SDK documentation and examples)
SDK Version: @modelcontextprotocol/sdk v1.25.3

Executive Summary

The boilerplate-mcp-server demonstrates strong architectural patterns and follows most modern MCP best practices. However, there are critical security gaps that must be addressed before production deployment.

Overall Compliance: 70% ✅ Good | 30% ⚠️ Needs Improvement

Priority Findings:

  • 🔴 CRITICAL: Missing DNS rebinding protection (no Origin header validation)
  • 🟡 HIGH: Missing explicit localhost binding
  • 🟡 HIGH: Missing isError field in tool error responses
  • 🟢 MEDIUM: Missing ResourceLink pattern implementation
  • 🟢 LOW: Missing prompt registration example

Detailed Findings

1. Security & Transport 🔒

1.1 DNS Rebinding Protection ❌ CRITICAL

Current State:

// src/index.ts (Line 70-72)
const app = express();
app.use(cors());
app.use(express.json());

Issue:
Using plain express() without DNS rebinding protection. The official MCP transport specification states:

"Servers MUST validate the Origin header on all incoming connections to prevent DNS rebinding attacks."

Best Practice:

import { createMcpExpressApp } from '@modelcontextprotocol/express';

// Built-in DNS rebinding protection
const app = createMcpExpressApp();

Impact:

  • Severity: HIGH
  • Risk: Malicious websites can potentially make requests to localhost MCP server
  • Attack Vector: DNS rebinding attack targeting user's localhost

Recommendation:
Add @modelcontextprotocol/express dependency and replace express() with createMcpExpressApp().

Reference:

1.2 Localhost Binding ⚠️ HIGH

Current State:

// src/index.ts (Line 107-113)
const PORT = Number(process.env.PORT ?? 3000);
await new Promise<void>((resolve) => {
    app.listen(PORT, () => {
        serverLogger.info(
            `HTTP transport listening on http://localhost:${PORT}${mcpEndpoint}`,
        );
        resolve();
    });
});

Issue:
app.listen(PORT) without explicit hostname binds to 0.0.0.0 (all interfaces) on some systems.

Best Practice:

app.listen(PORT, '127.0.0.1', () => {
    serverLogger.info(`HTTP transport listening on http://127.0.0.1:${PORT}${mcpEndpoint}`);
    resolve();
});

Impact:

  • Severity: MEDIUM-HIGH
  • Risk: Server accessible from network instead of localhost-only
  • Attack Vector: Remote access if firewall misconfigured

Recommendation:
Explicitly bind to 127.0.0.1 or localhost to prevent network exposure.

1.3 Authentication ⚠️ MEDIUM

Current State:
No authentication mechanism implemented in HTTP transport.

Issue:
MCP transport documentation recommends:

"Servers SHOULD implement authentication and authorization mechanisms appropriate for their deployment context."

Best Practice Examples:

  • Bearer token authentication
  • OAuth 2.0 integration
  • API key validation

Impact:

  • Severity: MEDIUM (depends on deployment)
  • Risk: Unauthorized access if server exposed beyond localhost
  • Mitigation: Low risk if DNS rebinding protection + localhost binding implemented

Recommendation:
Document authentication requirements in README and provide example implementation pattern.

2. Error Handling 🛠️

2.1 Missing isError Field ⚠️ HIGH

Current State:

// src/utils/error.util.ts (Line 129-166)
export function formatErrorForMcpTool(error: unknown): {
    content: Array<{ type: 'text'; text: string }>;
    metadata?: {
        errorType: ErrorType;
        statusCode?: number;
        errorDetails?: unknown;
    };
} {
    // ... returns content with metadata but no isError field
}

Issue:
According to MCP SDK examples and best practices, tool error responses should include isError: true at the top level:

// From typescript-sdk examples
return {
    content: [{ type: 'text', text: errorMessage }],
    isError: true  // ← Missing in our implementation
};

Best Practice:

export function formatErrorForMcpTool(error: unknown): {
    content: Array<{ type: 'text'; text: string }>;
    isError: true;  // ← Mark explicitly as error
    metadata?: {
        errorType: ErrorType;
        statusCode?: number;
        errorDetails?: unknown;
    };
} {
    const mcpError = ensureMcpError(error);
    return {
        content: [{ type: 'text', text: `Error: ${mcpError.message}` }],
        isError: true,  // ← Add this field
        metadata: { /* ... */ }
    };
}

Impact:

  • Severity: MEDIUM-HIGH
  • Risk: MCP clients may not properly detect error state
  • Behavior: Clients might treat errors as successful responses

Recommendation:
Add isError: true to error response type and all error formatters.

3. Modern API Usage ✅ EXCELLENT

3.1 Tool Registration ✅ COMPLIANT

Current Implementation:

// src/tools/ipaddress.tool.ts (Line 85-95)
server.registerTool(
    'ip_get_details',
    {
        title: 'IP Address Lookup',      // ✅ Display name
        description: IP_GET_DETAILS_DESCRIPTION,  // ✅ Detailed description
        inputSchema: GetIpDetailsToolSchema,      // ✅ Zod schema
    },
    handleGetIpDetails,
);

Assessment:
✅ EXCELLENT - Follows modern SDK v1.22.0+ API patterns perfectly:

  • Uses registerTool() instead of deprecated tool() method
  • Provides both title (for UI display) and description (detailed info)
  • Uses Zod for schema validation
  • Separates handler logic cleanly

Best Practice Alignment:
Matches official examples from modelcontextprotocol/typescript-sdk repository.

3.2 Resource Registration ✅ COMPLIANT

Current Implementation:

// src/resources/ipaddress.resource.ts (Line 17-50)
server.registerResource(
    'ip-lookup',
    new ResourceTemplate('ip://{ipAddress}', { list: undefined }),
    {
        title: 'IP Address Lookup',
        description: 'Retrieve geolocation and network information for a public IP address',
    },
    async (uri, variables) => {
        // Handler implementation
    },
);

Assessment:
✅ EXCELLENT - Modern resource registration with:

  • ResourceTemplate for parameterized URIs
  • Proper title + description metadata
  • Clean handler with URI variables extraction

3.3 Transport Implementation ✅ COMPLIANT

Current Implementation:

// src/index.ts (Line 77-80)
const transport = new StreamableHTTPServerTransport({
    sessionIdGenerator: undefined,  // ✅ Stateless pattern
});

Assessment:
✅ EXCELLENT - Uses modern transport:

  • StreamableHTTPServerTransport (not deprecated HTTP+SSE)
  • Explicit stateless configuration
  • Proper async/await patterns

Note:
Deprecated HTTP+SSE transport correctly avoided. Current implementation uses recommended Streamable HTTP protocol.

4. Missing Features 📋

4.1 ResourceLink Pattern ⚠️ MEDIUM

Current State:
Tools return full content inline, no support for ResourceLink references.

Best Practice:

// Tools can reference resources instead of embedding content
return {
    content: [
        {
            type: 'resource',
            resource: {
                uri: 'ip://8.8.8.8',
                text: 'Lookup result available as resource'
            }
        }
    ]
};

Benefit:

  • Reduces token usage for large responses
  • Enables resource caching
  • Allows clients to fetch details on-demand

Impact:

  • Severity: LOW-MEDIUM (optimization opportunity)
  • Risk: Higher token costs for large responses
  • Migration: Optional enhancement

Recommendation:
Document ResourceLink pattern in MODERNIZATION.md and provide example implementation.

4.2 Prompt Registration ⚠️ LOW

Current State:
No prompt registration examples in boilerplate.

Best Practice:

server.registerPrompt(
    'ip-analysis',
    {
        title: 'IP Analysis Template',
        description: 'Generate structured analysis for IP address',
        arguments: [
            { name: 'ip', description: 'IP address to analyze', required: true }
        ]
    },
    async (vars) => {
        const ipData = await getIpDetails(vars.ip);
        return {
            messages: [
                {
                    role: 'user',
                    content: {
                        type: 'text',
                        text: `Analyze this IP: ${JSON.stringify(ipData)}`
                    }
                }
            ]
        };
    }
);

Impact:

  • Severity: LOW (not essential for all servers)
  • Risk: None (prompts are optional MCP feature)
  • Use Case: Helpful for AI-driven analysis workflows

Recommendation:
Add example prompt registration to demonstrate MCP's three primitive types (tools, resources, prompts).

4.3 Protocol Version Header ⚠️ LOW

Current State:
No mention of MCP-Protocol-Version header handling in documentation.

Best Practice:
According to transport spec:

"Client MUST include MCP-Protocol-Version header on all requests"

Note:
SDK likely handles this automatically, but should be documented.

Recommendation:
Document protocol version requirements in README technical details section.

5. Strengths 💪

The following aspects demonstrate excellent MCP implementation:

5.1 Architecture ✅

  • 6-Layer Pattern: CLI → Tools → Resources → Controllers → Services → Utils
  • Clean Separation: Business logic isolated from MCP concerns
  • Type Safety: Full TypeScript with strict mode
  • DI Pattern: Controller dependencies clearly defined

5.2 Developer Experience ✅

  • Dual Transport: Supports both STDIO and HTTP seamlessly
  • CLI Mode: Built-in CLI for testing without MCP clients
  • Structured Logging: Context-aware logging throughout
  • Error Classification: Typed error system with meaningful categories

5.3 Quality ✅

  • Testing: 47 tests passing across 6 test suites
  • Graceful Shutdown: SIGINT/SIGTERM handlers implemented
  • Health Check: HTTP mode includes / endpoint for monitoring
  • Documentation: Comprehensive README, CHANGELOG, and MODERNIZATION guide

5.4 Modern Dependencies ✅

  • MCP SDK: v1.25.3 (latest stable)
  • Zod: v4.3.6 (latest with new features)
  • Express: v5.2.1 (latest major version)
  • All deps current: Updated 2025-01-13

Priority Recommendations

Immediate (Before Production)

  1. 🔴 Add DNS Rebinding Protection

    npm install @modelcontextprotocol/express

    Replace express() with createMcpExpressApp() in src/index.ts

  2. 🔴 Explicit Localhost Binding Update app.listen() to bind to 127.0.0.1 explicitly

  3. 🟡 Add isError: true Field Update src/utils/error.util.ts error formatters

Short Term (Next Release)

  1. 🟢 Document Authentication Requirements Add security section to README with authentication best practices

  2. 🟢 Add ResourceLink Example Create example tool that returns resource references instead of inline content

  3. 🟢 Add Prompt Registration Example Demonstrate all three MCP primitives (tools, resources, prompts)

Long Term (Future Enhancement)

  1. ⚪ MCP v2 Migration Plan SDK v2 expected Q1 2026 - prepare migration guide

  2. ⚪ Task Execution Pattern Add long-running task example with progress reporting

  3. ⚪ OAuth Integration Example Demonstrate secure authentication pattern for HTTP transport

Testing Validation ✅

All tests passing after dependency updates:

Test Suites: 6 passed, 6 total
Tests:       47 passed, 47 total
Snapshots:   0 total
Time:        8.204s

Coverage:

  • ✅ CLI commands functional
  • ✅ Tool handlers functional
  • ✅ Resource handlers functional
  • ✅ Controllers functional
  • ✅ Services functional
  • ✅ Utilities functional

Compliance Summary

Category Status Notes
Transport ✅ Compliant Modern Streamable HTTP
Tool Registration ✅ Compliant Modern registerTool API
Resource Registration ✅ Compliant ResourceTemplate pattern
DNS Rebinding Protection ❌ Missing Critical security gap
Localhost Binding ⚠️ Partial Needs explicit binding
Authentication ⚠️ Not Implemented Recommended for HTTP
Error Handling ⚠️ Partial Missing isError field
Session Management ✅ Compliant Stateless pattern
Graceful Shutdown ✅ Compliant SIGINT/SIGTERM handled
Documentation ✅ Excellent Comprehensive guides
Testing ✅ Excellent 47 tests passing
Dependencies ✅ Current All up-to-date (2025-01-13)

Conclusion

The boilerplate-mcp-server is a well-architected, well-documented project that follows most MCP best practices. The modern API usage, clean architecture, and comprehensive testing demonstrate high-quality engineering.

However, DNS rebinding protection is a critical security gap that must be addressed before production deployment. Once the security recommendations are implemented, this boilerplate will be production-ready and an excellent foundation for MCP server development.

Recommended Actions:

  1. Implement DNS rebinding protection (CRITICAL)
  2. Add explicit localhost binding (HIGH)
  3. Update error formatters with isError field (HIGH)
  4. Document authentication requirements (MEDIUM)
  5. Add ResourceLink and prompt examples (NICE-TO-HAVE)

References

  1. MCP Transport Specification
  2. MCP TypeScript SDK - Server Docs
  3. MCP TypeScript SDK - Examples
  4. MCP Security Best Practices
  5. Streamable HTTP Protocol

Audit Version: 1.0
Next Review: After MCP SDK v2 release (Q1 2026)