Skip to content

feat(mcp): add input sanitization and test coverage#52

Open
CoderMungan wants to merge 1 commit intomainfrom
feat/mcp-hardening
Open

feat(mcp): add input sanitization and test coverage#52
CoderMungan wants to merge 1 commit intomainfrom
feat/mcp-hardening

Conversation

@CoderMungan
Copy link
Copy Markdown
Member

MCP-SAN (#49): Input sanitization for the MCP server layer.

  • Add sanitize package: Content (Markdown structure injection), Reflect (truncate + strip control chars for error messages), SessionID (path-safe session identifiers), StripControl, Truncate
  • Sanitize all reflected user inputs in dispatch error messages (tool names, prompt names, resource URIs) via sanitize.Reflect
  • Reject unknown entry types before writing to .context/ files
  • Enforce MaxContentLen (32KB) on entry content in extract.EntryArgs
  • Sanitize entry content and optional fields via sanitize.Content and extract.SanitizedOpts before writing
  • Cap journal source limit to MaxSourceLimit (100)
  • Sanitize caller identifiers in session events
  • Add input length constants to config/mcp/cfg
  • Add error message keys for input-too-long and unknown-entry-type

MCP-COV (#50): Comprehensive test coverage for MCP subsystem.

  • internal/mcp/proto: 22 schema round-trip and edge-case tests
  • internal/mcp/session: 7 state lifecycle tests (100% coverage)
  • internal/mcp/server: 4 integration tests (Serve edge cases, prompt add-learning)
  • internal/mcp/server/def/tool: 9 tool definition tests
  • internal/mcp/server/def/prompt: 9 prompt definition tests
  • internal/mcp/server/extract: 7 extraction and sanitization tests
  • internal/mcp/server/io: 3 WriteJSON tests (100% coverage)
  • internal/mcp/server/out: 8 response builder tests (100% coverage)
  • internal/mcp/server/parse: 3 request parsing tests (100% coverage)
  • internal/mcp/server/stat: 2 statistics tests (100% coverage)
  • internal/sanitize: 22 sanitization tests (Content, Reflect, SessionID, StripControl, Truncate + existing Filename)
  • Server package coverage: 73% -> 92%

Closes #49
Closes #50

MCP-SAN (#49): Input sanitization for the MCP server layer.

- Add sanitize package: Content (Markdown structure injection),
  Reflect (truncate + strip control chars for error messages),
  SessionID (path-safe session identifiers), StripControl, Truncate
- Sanitize all reflected user inputs in dispatch error messages
  (tool names, prompt names, resource URIs) via sanitize.Reflect
- Reject unknown entry types before writing to .context/ files
- Enforce MaxContentLen (32KB) on entry content in extract.EntryArgs
- Sanitize entry content and optional fields via sanitize.Content
  and extract.SanitizedOpts before writing
- Cap journal source limit to MaxSourceLimit (100)
- Sanitize caller identifiers in session events
- Add input length constants to config/mcp/cfg
- Add error message keys for input-too-long and unknown-entry-type

MCP-COV (#50): Comprehensive test coverage for MCP subsystem.

- internal/mcp/proto: 22 schema round-trip and edge-case tests
- internal/mcp/session: 7 state lifecycle tests (100% coverage)
- internal/mcp/server: 4 integration tests (Serve edge cases,
  prompt add-learning)
- internal/mcp/server/def/tool: 9 tool definition tests
- internal/mcp/server/def/prompt: 9 prompt definition tests
- internal/mcp/server/extract: 7 extraction and sanitization tests
- internal/mcp/server/io: 3 WriteJSON tests (100% coverage)
- internal/mcp/server/out: 8 response builder tests (100% coverage)
- internal/mcp/server/parse: 3 request parsing tests (100% coverage)
- internal/mcp/server/stat: 2 statistics tests (100% coverage)
- internal/sanitize: 22 sanitization tests (Content, Reflect,
  SessionID, StripControl, Truncate + existing Filename)
- Server package coverage: 73% -> 92%

Closes #49
Closes #50

Signed-off-by: CoderMungan <codermungan@gmail.com>
@josealekhine
Copy link
Copy Markdown
Member

Hey @CoderMungan ; I'm adding a few AST-based code quality tests.

Once done, this one is next.

Fair warning; your code at its current state "may" break tests.
(not sure, possible)

But it will be easy and mechanical to fix them if that happens.

Just a heads up.

Copy link
Copy Markdown
Member

@josealekhine josealekhine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @CoderMungan — thank you for this, the sanitization work and test coverage are solid.

Before we can merge, this branch needs a rebase onto current main.

It's 107 commits behind, and main has had significant structural changes since March 30th that will affect your code:

What changed on main:

  1. AST audit tests — We now enforce conventions via go test ./internal/audit/...: magic strings, line length (≤80 chars), doc comments, no raw file I/O, no inline regexp,
    import/variable shadowing, and more. Your new files will need to pass these checks.
  2. ctx hook → ctx setup rename — assets/hooks/ was split into assets/integrations/ + assets/hooks/messages/. If your branch touches anything in that area, paths have changed.
  3. Config constant cleanup — Magic strings were externalized to YAML and constants were reorganized. New constants in config/mcp/cfg/ should follow the established pattern (semantic
    prefixes, YAML-backed descriptions).
  4. File I/O migration — All os.ReadFile/os.WriteFile calls moved to internal/io/. New code should use ctxio.ReadFile/ctxio.WriteFile instead of raw os calls.

How to verify after rebase:

  make lint   # must be 0 issues
  make test   # all tests must pass, including audit checks

The audit tests in internal/audit/ will flag anything that doesn't match current conventions — they're designed to catch exactly this kind of drift.

Let me know if you hit any issues during the rebase — happy to help work through conflicts.

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.

MCP-COV: MCP Test Coverage MCP-SAN: MCP Server Input Sanitization

2 participants