Skip to content

security(phase-3): sanitize error responses to MCP clients (M3)#20

Merged
rkm1 merged 1 commit into
mainfrom
security/phase-3-info-disclosure
May 11, 2026
Merged

security(phase-3): sanitize error responses to MCP clients (M3)#20
rkm1 merged 1 commit into
mainfrom
security/phase-3-info-disclosure

Conversation

@rkm1
Copy link
Copy Markdown
Member

@rkm1 rkm1 commented May 11, 2026

Phase 3 of plans/security-fixes-2026-05.md.

Summary

Add a tiny error-sanitization layer (`src/mcp/errors.ts`) and route every catch-handler in the MCP transport / tool surface through it. The helper logs the full error to stderr for the operator and returns the message with absolute paths (POSIX, Windows drive, UNC) replaced by ``.

Finding Site Fix
M3 `src/mcp/http.ts:113` (500 handler) `sanitizeForClient(err, "/mcp")` instead of `String(err)`
M3 `src/mcp/tools.ts` — every tool catch (`search_docs`, `list_docs`, `reindex_docs`, `get`, `multi_get`, `set_context`, `list_contexts`, `remove_context`) plus the per-file error in `multi_get`'s loop same — caught errors no longer leak abs paths to clients

Phase 1 already removed `absPath` interpolation from `indexer.resolveRef` error returns; this phase closes the catch-handler escape hatch.

Test plan

  • 349 tests pass (was 337; added 12 — 9 helper cases + 3 regression tests asserting catch-handler output omits `/Users/`, `secret-client`, `/var/log`)
  • `npm run lint` clean
  • `npx tsc --noEmit` clean

🤖 Generated with Claude Code

Add src/mcp/errors.ts with `sanitizeForClient` and `stripAbsolutePaths`.
Route every `catch (err) { ...return { error: String(err) } }` site
through the helper so caught exceptions can't leak absolute filesystem
paths (which embed the user's home dir, repo path, and often customer
or project names) to MCP / HTTP clients.

The helper:
  - Logs the full original error to stderr (operator-visible).
  - Returns the message with absolute paths (POSIX, Windows drive,
    UNC) replaced by `<path>`.

Wired into:
  - src/mcp/http.ts:113 — /mcp 500-handler
  - src/mcp/tools.ts — every tool's catch block (search_docs,
    list_docs, reindex_docs, get, multi_get, set_context,
    list_contexts, remove_context), plus the per-file error inside
    multi_get's loop.

Note: indexer.resolveRef error sites were already addressed in
Phase 1's refactor (they now return relFile-based errors that
never embed workspaceRoot). This phase covers the catch-handler
escape hatch and the OpenAI passthrough flagged in M1's notes.

Tests
  - mcp-errors.test.ts: 9 cases for stripAbsolutePaths (POSIX,
    Windows drive, UNC, multiple paths, path-only message,
    relative paths preserved) and sanitizeForClient (stderr log,
    non-Error throwables, context-hint prefix).
  - mcp-tools.test.ts: 3 regression tests that throw an Error
    embedding an absolute path from search_docs, reindex_docs, and
    get; assert the client response contains the error context but
    not `/Users/`, `secret-client`, or `/var/log`.

349 tests pass (was 337; added 12).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rkm1 rkm1 merged commit a054169 into main May 11, 2026
10 checks passed
@rkm1 rkm1 deleted the security/phase-3-info-disclosure branch May 11, 2026 17:05
@rkm1 rkm1 mentioned this pull request May 11, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant