Skip to content

Commit f7e5c08

Browse files
committed
Guard manage_asset search against broad scans
1 parent 2d2bdc5 commit f7e5c08

5 files changed

Lines changed: 199 additions & 16 deletions

File tree

MCPForUnity/Editor/Tools/ManageAsset.cs

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -627,37 +627,47 @@ private static object MoveOrRenameAsset(string path, string destinationPath)
627627

628628
private static object SearchAssets(JObject @params)
629629
{
630-
string searchPattern = @params["searchPattern"]?.ToString();
631-
string filterType = @params["filterType"]?.ToString();
632-
string pathScope = @params["path"]?.ToString(); // Use path as folder scope
633-
string filterDateAfterStr = @params["filterDateAfter"]?.ToString();
630+
string searchPattern = @params["searchPattern"]?.ToString()?.Trim();
631+
string filterType = @params["filterType"]?.ToString()?.Trim();
632+
string pathScope = @params["path"]?.ToString()?.Trim(); // Use path as folder scope
633+
string filterDateAfterStr = @params["filterDateAfter"]?.ToString()?.Trim();
634634
int pageSize = @params["pageSize"]?.ToObject<int?>() ?? 50; // Default page size
635635
int pageNumber = @params["pageNumber"]?.ToObject<int?>() ?? 1; // Default page number (1-based)
636636
bool generatePreview = @params["generatePreview"]?.ToObject<bool>() ?? false;
637637

638638
List<string> searchFilters = new List<string>();
639-
if (!string.IsNullOrEmpty(searchPattern))
639+
if (!string.IsNullOrWhiteSpace(searchPattern))
640640
searchFilters.Add(searchPattern);
641-
if (!string.IsNullOrEmpty(filterType))
641+
if (!string.IsNullOrWhiteSpace(filterType))
642642
searchFilters.Add($"t:{filterType}");
643643

644644
string[] folderScope = null;
645-
if (!string.IsNullOrEmpty(pathScope))
645+
if (!string.IsNullOrWhiteSpace(pathScope))
646646
{
647647
folderScope = new string[] { AssetPathUtility.SanitizeAssetPath(pathScope) };
648648
if (!AssetDatabase.IsValidFolder(folderScope[0]))
649649
{
650-
// Maybe the user provided a file path instead of a folder?
651-
// We could search in the containing folder, or return an error.
652-
McpLog.Warn(
653-
$"Search path '{folderScope[0]}' is not a valid folder. Searching entire project."
650+
return new ErrorResponse(
651+
$"Search path '{folderScope[0]}' is not a valid folder. "
652+
+ "Use a valid folder scope in 'path' or put the query in 'searchPattern'."
654653
);
655-
folderScope = null; // Search everywhere if path isn't a folder
656654
}
657655
}
658656

657+
if (searchFilters.Count == 0 && folderScope == null)
658+
{
659+
return new ErrorResponse(
660+
"manage_asset search requires a valid folder scope in 'path' or a search filter such as 'searchPattern' or 'filterType'."
661+
);
662+
}
663+
664+
if (pageSize <= 0)
665+
return new ErrorResponse("'pageSize' must be greater than zero.");
666+
if (pageNumber <= 0)
667+
return new ErrorResponse("'pageNumber' must be greater than zero.");
668+
659669
DateTime? filterDateAfter = null;
660-
if (!string.IsNullOrEmpty(filterDateAfterStr))
670+
if (!string.IsNullOrWhiteSpace(filterDateAfterStr))
661671
{
662672
if (
663673
DateTime.TryParse(
@@ -684,7 +694,7 @@ out DateTime parsedDate
684694
string.Join(" ", searchFilters),
685695
folderScope
686696
);
687-
List<object> results = new List<object>();
697+
List<string> matchingPaths = new List<string>();
688698
int totalFound = 0;
689699

690700
foreach (string guid in guids)
@@ -706,12 +716,16 @@ out DateTime parsedDate
706716
}
707717

708718
totalFound++; // Count matching assets before pagination
709-
results.Add(GetAssetData(assetPath, generatePreview));
719+
matchingPaths.Add(assetPath);
710720
}
711721

712722
// Apply pagination
713723
int startIndex = (pageNumber - 1) * pageSize;
714-
var pagedResults = results.Skip(startIndex).Take(pageSize).ToList();
724+
var pagedResults = matchingPaths
725+
.Skip(startIndex)
726+
.Take(pageSize)
727+
.Select(assetPath => GetAssetData(assetPath, generatePreview))
728+
.ToList();
715729

716730
return new SuccessResponse(
717731
$"Found {totalFound} asset(s). Returning page {pageNumber} ({pagedResults.Count} assets).",

Server/src/services/tools/manage_asset.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,18 +80,41 @@ async def manage_asset(
8080
# Handle case where path is not a string despite type annotation
8181
raw_path = ""
8282

83+
def _looks_like_asset_folder_scope(value: str) -> bool:
84+
normalized = value.replace("\\", "/").strip("/")
85+
return (
86+
normalized == "Assets"
87+
or normalized.startswith("Assets/")
88+
or normalized == "Packages"
89+
or normalized.startswith("Packages/")
90+
)
91+
8392
# If the caller put an AssetDatabase query into `path`, treat it as `search_pattern`.
8493
if (not search_pattern) and raw_path.startswith("t:"):
8594
search_pattern = raw_path
8695
path = "Assets"
8796
await ctx.info("manage_asset(search): normalized query from `path` into `search_pattern` and set path='Assets'")
97+
elif raw_path and not _looks_like_asset_folder_scope(raw_path):
98+
if not search_pattern:
99+
search_pattern = raw_path
100+
path = "Assets"
101+
await ctx.info("manage_asset(search): normalized non-folder `path` into search criteria and set path='Assets'")
88102

89103
# If the caller used `asset_type` to mean a search filter, map it to filter_type.
90104
# (In Unity, filterType becomes `t:<filterType>`.)
91105
if (not filter_type) and asset_type and isinstance(asset_type, str):
92106
filter_type = asset_type
93107
await ctx.info("manage_asset(search): mapped `asset_type` into `filter_type` for safer server-side filtering")
94108

109+
if not any((raw_path, search_pattern, filter_type)):
110+
return {
111+
"success": False,
112+
"message": (
113+
"manage_asset search requires a folder scope in `path` or at least one search criterion "
114+
"such as `search_pattern` or `filter_type`."
115+
),
116+
}
117+
95118
# Prepare parameters for the C# handler
96119
params_dict = {
97120
"action": action.lower(),
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import pytest
2+
3+
from .test_helpers import DummyContext
4+
from services.tools.manage_asset import manage_asset
5+
6+
7+
@pytest.mark.asyncio
8+
async def test_search_without_scope_or_filter_returns_error(monkeypatch):
9+
async def fail_if_dispatched(*args, **kwargs):
10+
raise AssertionError("empty asset search should not dispatch to Unity")
11+
12+
monkeypatch.setattr(
13+
"services.tools.manage_asset.send_with_unity_instance",
14+
fail_if_dispatched,
15+
)
16+
17+
result = await manage_asset(
18+
ctx=DummyContext(),
19+
action="search",
20+
path="",
21+
)
22+
23+
assert result["success"] is False
24+
assert "search_pattern" in result["message"]
25+
26+
27+
@pytest.mark.asyncio
28+
async def test_search_date_filter_without_scope_or_filter_returns_error(monkeypatch):
29+
async def fail_if_dispatched(*args, **kwargs):
30+
raise AssertionError("date-only asset search should not dispatch to Unity")
31+
32+
monkeypatch.setattr(
33+
"services.tools.manage_asset.send_with_unity_instance",
34+
fail_if_dispatched,
35+
)
36+
37+
result = await manage_asset(
38+
ctx=DummyContext(),
39+
action="search",
40+
path="",
41+
filter_date_after="2026-01-01T00:00:00Z",
42+
)
43+
44+
assert result["success"] is False
45+
assert "filter_type" in result["message"]
46+
47+
48+
@pytest.mark.asyncio
49+
async def test_search_treats_non_folder_path_as_search_pattern(monkeypatch):
50+
captured = {}
51+
52+
async def fake_send(send_fn, unity_instance, command_type, params, **kwargs):
53+
captured["command_type"] = command_type
54+
captured["params"] = params
55+
return {"success": True, "data": {"assets": []}}
56+
57+
monkeypatch.setattr(
58+
"services.tools.manage_asset.send_with_unity_instance",
59+
fake_send,
60+
)
61+
62+
result = await manage_asset(
63+
ctx=DummyContext(),
64+
action="search",
65+
path="PlayerController",
66+
)
67+
68+
assert result["success"] is True
69+
assert captured["command_type"] == "manage_asset"
70+
assert captured["params"]["path"] == "Assets"
71+
assert captured["params"]["searchPattern"] == "PlayerController"
72+
73+
74+
@pytest.mark.asyncio
75+
async def test_search_keeps_folder_scope_when_filter_is_present(monkeypatch):
76+
captured = {}
77+
78+
async def fake_send(send_fn, unity_instance, command_type, params, **kwargs):
79+
captured["params"] = params
80+
return {"success": True, "data": {"assets": []}}
81+
82+
monkeypatch.setattr(
83+
"services.tools.manage_asset.send_with_unity_instance",
84+
fake_send,
85+
)
86+
87+
result = await manage_asset(
88+
ctx=DummyContext(),
89+
action="search",
90+
path="Assets/Prefabs",
91+
filter_type="Prefab",
92+
)
93+
94+
assert result["success"] is True
95+
assert captured["params"]["path"] == "Assets/Prefabs"
96+
assert captured["params"]["filterType"] == "Prefab"
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
using MCPForUnity.Editor.Helpers;
2+
using MCPForUnity.Editor.Tools;
3+
using Newtonsoft.Json.Linq;
4+
using NUnit.Framework;
5+
6+
namespace MCPForUnityTests.Editor.Tools
7+
{
8+
public class ManageAssetSearchGuardTests
9+
{
10+
[Test]
11+
public void Search_WithoutScopeOrFilter_ReturnsError()
12+
{
13+
var result = ManageAsset.HandleCommand(new JObject
14+
{
15+
["action"] = "search",
16+
["path"] = ""
17+
});
18+
19+
var error = result as ErrorResponse;
20+
Assert.IsNotNull(error, "Unfiltered project-wide search should be rejected.");
21+
Assert.That(error.Error, Does.Contain("searchPattern"));
22+
}
23+
24+
[Test]
25+
public void Search_WithInvalidFolderScope_ReturnsError()
26+
{
27+
var result = ManageAsset.HandleCommand(new JObject
28+
{
29+
["action"] = "search",
30+
["path"] = "DefinitelyNotAUnityAssetFolder",
31+
["filterType"] = "Prefab"
32+
});
33+
34+
var error = result as ErrorResponse;
35+
Assert.IsNotNull(error, "Invalid search folder must not fall back to a full-project search.");
36+
Assert.That(error.Error, Does.Contain("valid folder"));
37+
}
38+
}
39+
}

TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageAssetSearchGuardTests.cs.meta

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)