Sanitize tool_id in toolops responses (XSS/robustness)

Open Beginner friendly
#5,876 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
fastapi, python
Domain
api, backend, security

Research direction

Start in toolops_router.py at generate_testcases_for_tool and enrich_a_tool, especially the output assignment around lines 165-168 and the logger call. Trace existing SecurityValidator.sanitize_display_text() usage, add regression tests for sanitized tool_id responses, and run those tests plus lint on the changed files.

Written by the indexing model from the issue text.

Description

security

Description

Two security/robustness fixes were identified in the pr-5417-decomposition scope sweep. After triage, only Fix 1 remains actionable.

Fix 1 (actionable): XSS prevention in toolops_router.py

Apply SecurityValidator.sanitize_display_text() to tool_id at output sites in generate_testcases_for_tool and enrich_a_tool:

  • toolops_router.py:168: result["tool_id"] = tool_id — reflects raw value into response
  • toolops_router.py:165: string concatenation in logger call

Risk level: Low in practice — tool_id is already constrained by QueryIdentifierDotted300 (SecurityValidator.IDENTIFIER_PATTERN: ^[a-zA-Z0-9_\-\.]+$), which prevents HTML/JS injection at the input boundary. The output sanitization is defence-in-depth.

Fix 2 (superseded): int(hours) coercion in observability.py

Both get_stats and get_query_performance already declare hours: int = Query(24, ge=1, le=168). FastAPI enforces the type at the HTTP boundary — a non-integer value results in a 422 before the handler runs. Adding int(hours) inside the handler would be a no-op. This fix is not needed.

Acceptance Criteria

  • tool_id outputs in /toolops endpoints are sanitized via SecurityValidator.sanitize_display_text()
  • Regression tests added for the fix
  • Lint passes on changed files

Labels

bug, security

Related

Part of pr-5417-decomposition security family completion.

Dominant language
Python
Stars
4.5k
Forks
877
Avg merge
3d 12h
Merged PRs (30d)
66

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from IBM/mcp-context-forge

All issues in IBM/mcp-context-forge

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.