Implement tool-level error sanitization to prevent sensitive data leakage

[Claude]

Tool handlers were throwing raw exceptions that exposed stack traces, connection strings with credentials, internal file paths, and API tokens to MCP clients.

Changes

Added ErrorHandler.toMcpErrorResult() utility (src/errors/ErrorHandler.ts)

• Converts exceptions to sanitized MCP tool results
• Removes: stack traces, connection URLs with credentials, IP addresses/ports, file paths, password/token fields
• Preserves AppError messages (already user-safe)
• Returns consistent format with isError: true flag

Updated all tool handlers (src/mcp/tools.ts)

• Changed from throw error to return errorHandler.toMcpErrorResult(error)
• Maintains internal error logging while sanitizing client-facing messages
• Applies to: query_graph, query_graph_readonly, list_graphs, delete_graph, list_keys, set_key, get_key, delete_key

Test coverage (src/errors/ErrorHandler.test.ts)

• 17 test cases covering sanitization patterns
• Validates removal of credentials, stack traces, network info, secrets

Example

Before:

Error: Connection failed to redis://admin:secret123@192.168.1.100:6379
    at /home/app/src/db.ts:45
    at async connect (/home/app/index.js:12)

After:

Error: Connection failed to redis://<credentials>@<host>

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Implementation of Tool-Level Error Mapping</issue_title>
<issue_description>Description: Tool handlers currently throw raw exceptions or return unsanitized error strings that may include stack traces or internal connection details.

Impact: Leaking of internal implementation details and poor developer experience for LLM integrations.

Recommended Fix: Create a mapping utility (toMcpErrorResult) that scrubs sensitive data and returns consistent, user-safe error messages to the client.

Affected Files: src/mcp/tools.ts, src/errors/ErrorHandler.ts.</issue_description>

Comments on the Issue (you are @claude[agent] in this section)

• Fixes Implementation of Tool-Level Error Mapping #34

[Copilot]

Pull request overview

This pull request implements tool-level error sanitization to prevent sensitive data leakage in MCP (Model Context Protocol) tool responses. The PR adds a centralized sanitization mechanism that removes credentials, stack traces, file paths, and other sensitive information from error messages before they are returned to MCP clients.

Changes:

• Added ErrorHandler.toMcpErrorResult() method that sanitizes errors and returns them in MCP-compatible format with comprehensive regex-based scrubbing of sensitive patterns
• Updated all 8 tool handlers in tools.ts to return sanitized error results instead of throwing raw exceptions
• Added 17 test cases covering sanitization of various sensitive data patterns including credentials, paths, IPs, and tokens

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
src/errors/ErrorHandler.ts	Added McpErrorResult interface and toMcpErrorResult() public method with sanitizeErrorMessage() private helper that removes stack traces, connection strings with credentials, password/token fields, IP addresses, and file paths
src/mcp/tools.ts	Updated all 8 tool handlers (query_graph, query_graph_readonly, list_graphs, delete_graph, list_keys, set_key, get_key, delete_key) to catch errors and return sanitized results via errorHandler.toMcpErrorResult() instead of throwing; also cleaned up trailing whitespace
src/errors/ErrorHandler.test.ts	Added comprehensive test suite with 17 test cases covering AppError passthrough, stack trace removal, file path sanitization, connection string scrubbing, IP/port redaction, credential field sanitization, edge cases (null/undefined/empty), and complex multi-pattern scenarios

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The comment says "use negative lookahead" but the code uses negative lookbehind (?<!:). Update the comment to say "use negative lookbehind" for accuracy.

Suggested change

	// Remove file paths (absolute paths) - use negative lookahead to avoid matching :// URLs
	// Remove file paths (absolute paths) - use negative lookbehind to avoid matching :// URLs

[Copilot]

The regex patterns for connection strings only match when credentials are present (they require an @ symbol). Connection strings without credentials like redis://localhost:6379 won't be sanitized. While these don't contain secrets, they do reveal internal network topology (hostnames, ports). Consider whether this is acceptable or if you want to also sanitize connection strings without credentials for consistency.

Suggested change

	// Remove connection strings without credentials (sanitize host and path)
	sanitized = sanitized.replace(/redis:\/\/(?![^@\s]+@)[^\s]+/gi, 'redis://<host>');
	sanitized = sanitized.replace(/mongodb:\/\/(?![^@\s]+@)[^\s]+/gi, 'mongodb://<host>');
	sanitized = sanitized.replace(/postgresql:\/\/(?![^@\s]+@)[^\s]+/gi, 'postgresql://<host>');

[Copilot]

The sanitization is missing the falkordb:// protocol which is used throughout this codebase. Connection strings like falkordb://user:pass@host:port could leak credentials. Add a replacement pattern similar to the existing ones for redis, mongodb, and postgresql protocols.

Suggested change

	sanitized = sanitized.replace(/postgresql:\/\/[^@\s]+@[^\s]+/gi, 'postgresql://<credentials>@<host>');
	sanitized = sanitized.replace(/postgresql:\/\/[^@\s]+@[^\s]+/gi, 'postgresql://<credentials>@<host>');
	sanitized = sanitized.replace(/falkordb:\/\/[^@\s]+@[^\s]+/gi, 'falkordb://<credentials>@<host>');

[Copilot]

The character class [\w\-./] has ambiguous placement of the hyphen. In character classes, a hyphen between characters can be interpreted as a range. Move the hyphen to the start or end of the character class for clarity: [\w.\/-] or [\w./-] (where / is escaped for readability). While JavaScript typically handles this gracefully when the range is invalid, it's a maintainability issue and could cause confusion.

Suggested change

	sanitized = sanitized.replace(/(?<!:)\/[\w\-./]+/g, '<path>');
	sanitized = sanitized.replace(/\b[A-Z]:\\[\w\-\\]+/g, '<path>');
	sanitized = sanitized.replace(/(?<!:)\/[\w./-]+/g, '<path>');
	sanitized = sanitized.replace(/\b[A-Z]:\\[\w\\-]+/g, '<path>');

[codecov]

Codecov Report

❌ Patch coverage is 88.23529% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.56%. Comparing base (2b24b1d) to head (c90fa78).

Files with missing lines	Patch %	Lines
src/mcp/tools.ts	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
+ Coverage   48.32%   51.56%   +3.23%     
==========================================
  Files          11       11              
  Lines         449      479      +30     
  Branches      103      107       +4     
==========================================
+ Hits          217      247      +30     
  Misses        232      232              

Flag	Coverage Δ
unittests	51.56% <88.23%> (+3.23%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.