Skip to content

Commit f904f47

Browse files
ja8zyjitsja8zyjits
committed
Fixed security issues in the logging and redundant test cases
Signed-off-by: Jitesh Nair <jiteshnair@ibm.com>
1 parent 7d9a2c9 commit f904f47

File tree

4 files changed

+49
-75
lines changed

4 files changed

+49
-75
lines changed

‎.secrets.baseline‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"files": "^.secrets.baseline|package-lock.json|Cargo.lock|scripts/sign_image.sh|scripts/zap|sonar-project.properties|uv.lock|go.sum|mcpgateway/sri_hashes.json|^.secrets.baseline$",
44
"lines": null
55
},
6-
"generated_at": "2026-04-14T15:58:17Z",
6+
"generated_at": "2026-04-14T21:47:51Z",
77
"plugins_used": [
88
{
99
"name": "AWSKeyDetector"
@@ -4876,23 +4876,23 @@
48764876
"hashed_secret": "f2b14f68eb995facb3a1c35287b778d5bd785511",
48774877
"is_secret": false,
48784878
"is_verified": false,
4879-
"line_number": 175,
4879+
"line_number": 176,
48804880
"type": "Secret Keyword",
48814881
"verified_result": null
48824882
},
48834883
{
48844884
"hashed_secret": "1073ab6cda4b991cd29f9e83a307f34004ae9327",
48854885
"is_secret": false,
48864886
"is_verified": false,
4887-
"line_number": 181,
4887+
"line_number": 182,
48884888
"type": "Secret Keyword",
48894889
"verified_result": null
48904890
},
48914891
{
48924892
"hashed_secret": "d4fe581561f18ee5006254a7e53f53cbed780bc2",
48934893
"is_secret": false,
48944894
"is_verified": false,
4895-
"line_number": 268,
4895+
"line_number": 269,
48964896
"type": "Secret Keyword",
48974897
"verified_result": null
48984898
}
@@ -8698,63 +8698,63 @@
86988698
"hashed_secret": "99834bc4eff3f1e1c1e4692d2476b593b501d045",
86998699
"is_secret": false,
87008700
"is_verified": false,
8701-
"line_number": 5738,
8701+
"line_number": 5711,
87028702
"type": "Secret Keyword",
87038703
"verified_result": null
87048704
},
87058705
{
87068706
"hashed_secret": "f2b14f68eb995facb3a1c35287b778d5bd785511",
87078707
"is_secret": false,
87088708
"is_verified": false,
8709-
"line_number": 5756,
8709+
"line_number": 5729,
87108710
"type": "Secret Keyword",
87118711
"verified_result": null
87128712
},
87138713
{
87148714
"hashed_secret": "b8cec023ad982a1355abb9e7c7700e42d7e6fac3",
87158715
"is_secret": false,
87168716
"is_verified": false,
8717-
"line_number": 5780,
8717+
"line_number": 5753,
87188718
"type": "Secret Keyword",
87198719
"verified_result": null
87208720
},
87218721
{
87228722
"hashed_secret": "0614eb27c6e0d2f4ed9a1cce2a5bcbbbc17aa556",
87238723
"is_secret": false,
87248724
"is_verified": false,
8725-
"line_number": 5844,
8725+
"line_number": 5817,
87268726
"type": "Secret Keyword",
87278727
"verified_result": null
87288728
},
87298729
{
87308730
"hashed_secret": "a38fea79a043f0c9a62f851659d459dc3b716ea9",
87318731
"is_secret": false,
87328732
"is_verified": false,
8733-
"line_number": 5855,
8733+
"line_number": 5828,
87348734
"type": "Secret Keyword",
87358735
"verified_result": null
87368736
},
87378737
{
87388738
"hashed_secret": "a50da1fb101595ac5158f2c9394a65a84061b56c",
87398739
"is_secret": false,
87408740
"is_verified": false,
8741-
"line_number": 5896,
8741+
"line_number": 5869,
87428742
"type": "Secret Keyword",
87438743
"verified_result": null
87448744
},
87458745
{
87468746
"hashed_secret": "ed3e0017cb8e4b06a59af1a441f62cbe58d2ef59",
87478747
"is_secret": false,
87488748
"is_verified": false,
8749-
"line_number": 5902,
8749+
"line_number": 5875,
87508750
"type": "Secret Keyword",
87518751
"verified_result": null
87528752
},
87538753
{
87548754
"hashed_secret": "9bdc408babb4c5740aa9ee42e65b9308bd01f5f9",
87558755
"is_secret": false,
87568756
"is_verified": false,
8757-
"line_number": 7025,
8757+
"line_number": 6998,
87588758
"type": "Secret Keyword",
87598759
"verified_result": null
87608760
}

‎mcpgateway/middleware/request_logging_middleware.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
# First-Party
6464
from mcpgateway.auth import get_current_user
6565
from mcpgateway.config import settings
66+
from mcpgateway.common.validators import SecurityValidator
6667
from mcpgateway.middleware.path_filter import should_skip_request_logging
6768
from mcpgateway.services.logging_service import LoggingService
6869
from mcpgateway.services.structured_logger import get_structured_logger
@@ -299,8 +300,8 @@ def _mask_json_payload_for_logging(payload: bytes, max_depth: int = 10) -> str:
299300
if isinstance(masked_payload, bytes):
300301
return masked_payload.decode("utf-8", errors="ignore")
301302
return str(masked_payload)
302-
except Exception:
303-
logger.exception("Failed in processing the payload")
303+
except Exception as exc:
304+
logger.warning("Failed in processing the payload %s", SecurityValidator.sanitize_log_message(str(exc)))
304305

305306
json_payload = orjson.loads(payload)
306307
payload_to_log = mask_sensitive_data(json_payload, max_depth)

‎mcpgateway/services/tool_service.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1298,7 +1298,7 @@ def _extract_and_validate_structured_content(self, tool: DbTool, tool_result: "T
12981298
# Reference: https://modelcontextprotocol.io/specification/2025-11-25/server/tools#error-handling
12991299
is_error = getattr(tool_result, "is_error", False) or getattr(tool_result, "isError", False)
13001300
if is_error:
1301-
logger.debug(f"Skipping output schema validation for error response from tool {getattr(tool, 'name', '<unknown>')}")
1301+
logger.debug(f"Skipping output schema validation for error response from tool {SecurityValidator.sanitize_log_message(getattr(tool, 'name', '<unknown>'))}")
13021302
return True
13031303

13041304
output_schema = getattr(tool, "output_schema", None)

‎tests/unit/mcpgateway/services/test_tool_service_coverage.py‎

Lines changed: 33 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -3546,6 +3546,39 @@ async def test_team_name_conflict(self, tool_service):
35463546
tool.tags = []
35473547
tool.team_id = "team-1"
35483548

3549+
existing = MagicMock()
3550+
existing.name = "my_tool"
3551+
existing.enabled = True
3552+
existing.id = "existing-id"
3553+
existing.visibility = "team"
3554+
3555+
db = MagicMock()
3556+
db.execute = MagicMock(return_value=MagicMock(scalar_one_or_none=MagicMock(return_value=existing)))
3557+
3558+
with pytest.raises(ToolNameConflictError):
3559+
await tool_service.register_tool(db, tool, visibility="team", team_id="team-1", owner_email="user@test.com")
3560+
3561+
@pytest.mark.asyncio
3562+
async def test_defaults_visibility_from_tool_object(self, tool_service):
3563+
"""When visibility is None, defaults from tool.visibility then checks name conflict."""
3564+
tool = MagicMock()
3565+
tool.name = "dup_tool_defaults"
3566+
tool.team_id = "team-99"
3567+
tool.owner_email = "default@test.com"
3568+
tool.visibility = "team" # will be used as default since visibility=None
3569+
3570+
existing = MagicMock()
3571+
existing.name = "dup_tool_defaults"
3572+
existing.enabled = True
3573+
existing.id = "existing-id"
3574+
existing.visibility = "team"
3575+
3576+
db = MagicMock()
3577+
db.execute = MagicMock(return_value=MagicMock(scalar_one_or_none=MagicMock(return_value=existing)))
3578+
3579+
# visibility=None => uses tool.visibility="team", team_id defaults from tool.team_id
3580+
with pytest.raises(ToolNameConflictError):
3581+
await tool_service.register_tool(db, tool, visibility=None, owner_email=None)
35493582

35503583
def test_skip_validation_for_error_response_is_error(self, tool_service):
35513584
"""Skip output schema validation when is_error=True per MCP spec."""
@@ -3646,66 +3679,6 @@ def test_validate_success_response_fails_validation(self, tool_service):
36463679
assert "recognitionId" in tool_result.content[0].text or "required" in tool_result.content[0].text.lower()
36473680

36483681

3649-
# ---------------------------------------------------------------------------
3650-
# register_tool — team name conflict (lines 1075-1078) + defaults (1059-1062)
3651-
# ---------------------------------------------------------------------------
3652-
3653-
3654-
class TestRegisterToolBranches:
3655-
@pytest.mark.asyncio
3656-
async def test_team_name_conflict(self, tool_service):
3657-
"""Raises ToolNameConflictError when team tool with same name exists."""
3658-
tool = MagicMock()
3659-
tool.name = "my_tool"
3660-
tool.displayName = "My Tool"
3661-
tool.url = "http://example.com"
3662-
tool.description = "desc"
3663-
tool.integration_type = "REST"
3664-
tool.request_type = "GET"
3665-
tool.headers = {}
3666-
tool.input_schema = {}
3667-
tool.output_schema = None
3668-
tool.annotations = {}
3669-
tool.jsonpath_filter = None
3670-
tool.auth = None
3671-
tool.tags = []
3672-
tool.team_id = "team-1"
3673-
3674-
existing = MagicMock()
3675-
existing.name = "my_tool"
3676-
existing.enabled = True
3677-
existing.id = "existing-id"
3678-
existing.visibility = "team"
3679-
3680-
db = MagicMock()
3681-
db.execute = MagicMock(return_value=MagicMock(scalar_one_or_none=MagicMock(return_value=existing)))
3682-
3683-
with pytest.raises(ToolNameConflictError):
3684-
await tool_service.register_tool(db, tool, visibility="team", team_id="team-1", owner_email="user@test.com")
3685-
3686-
@pytest.mark.asyncio
3687-
async def test_defaults_visibility_from_tool_object(self, tool_service):
3688-
"""When visibility is None, defaults from tool.visibility then checks name conflict."""
3689-
tool = MagicMock()
3690-
tool.name = "dup_tool_defaults"
3691-
tool.team_id = "team-99"
3692-
tool.owner_email = "default@test.com"
3693-
tool.visibility = "team" # will be used as default since visibility=None
3694-
3695-
existing = MagicMock()
3696-
existing.name = "dup_tool_defaults"
3697-
existing.enabled = True
3698-
existing.id = "existing-id"
3699-
existing.visibility = "team"
3700-
3701-
db = MagicMock()
3702-
db.execute = MagicMock(return_value=MagicMock(scalar_one_or_none=MagicMock(return_value=existing)))
3703-
3704-
# visibility=None => uses tool.visibility="team", team_id defaults from tool.team_id
3705-
with pytest.raises(ToolNameConflictError):
3706-
await tool_service.register_tool(db, tool, visibility=None, owner_email=None)
3707-
3708-
37093682
# ---------------------------------------------------------------------------
37103683
# _process_chunk — fail status + audit + exception (lines 1432-1434, 1449-1465)
37113684
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)