Add MCP methods to applicable actions#900
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/actions/action_schema_search_v1.h">
<violation number="1" location="src/actions/action_schema_search_v1.h:178">
P1: Validate MCP `limit` range before converting to `size_t`; negative integers currently wrap to huge values and can cause pathological searches.</violation>
</file>
<file name="src/actions/action_serve_explorer_artifact_v1.h">
<violation number="1" location="src/actions/action_serve_explorer_artifact_v1.h:75">
P1: Validate `arguments.path` before appending it to `absolute_path`; current code allows absolute paths and parent-directory traversal to escape the explorer base directory.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
🤖 Augment PR SummarySummary: This PR adds Model Context Protocol (MCP) support to applicable actions so they can be invoked via JSON-RPC tool calls. Changes:
Technical Notes: MCP responses are returned as JSON-RPC 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| } | ||
| const auto path{arguments.at("path").to_string()}; | ||
| if (!path.empty()) { | ||
| absolute_path /= path; |
There was a problem hiding this comment.
src/actions/action_serve_explorer_artifact_v1.h:75: In the MCP "list" path, user-controlled arguments.path is appended directly to absolute_path, so an absolute path (or .. segments) can escape base()/explorer and read unexpected files via metapack_read_json. Consider constraining this input to a safe relative path before joining.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
|
|
||
| return this->base_ / "schemas" / uri_path / "%"; | ||
| return std::filesystem::path{uri_path}; |
There was a problem hiding this comment.
src/actions/schema_directory.cc:36: uri_to_path() returns a filesystem path built directly from the URI path without rejecting traversal segments (e.g. ..), which then flows into schema_directory() and various MCP handlers to access the filesystem. This can allow schema URIs to resolve outside the intended base()/schemas subtree.
Severity: high
Other Locations
src/actions/action_jsonschema_evaluate_v1.h:82src/actions/action_jsonschema_trace_v1.h:84src/actions/action_serve_schema_artifact_v1.h:75src/actions/action_serve_explorer_artifact_v1.h:83
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| if (!arguments.at("limit").is_integer()) { | ||
| return sourcemeta::one::jsonrpc_make_error_invalid_params(request_id); | ||
| } | ||
| limit = static_cast<std::size_t>(arguments.at("limit").to_integer()); |
There was a problem hiding this comment.
src/actions/action_schema_search_v1.h:178: MCP limit accepts any integer and is cast to std::size_t, so negative values can wrap to a huge limit and very large values bypass the REST-side bounds. This may lead to unexpected search sizes or resource usage; consider validating the range similarly to the REST handler.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Benchmark Index (community)
Details
| Benchmark suite | Current: fabf1f3 | Previous: ef38b58 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
240 ms |
235 ms |
1.02 |
Add one schema (100 existing) |
26 ms |
24 ms |
1.08 |
Add one schema (1000 existing) |
85 ms |
76 ms |
1.12 |
Add one schema (10000 existing) |
899 ms |
645 ms |
1.39 |
Update one schema (1 existing) |
19 ms |
17 ms |
1.12 |
Update one schema (101 existing) |
26 ms |
25 ms |
1.04 |
Update one schema (1001 existing) |
87 ms |
75 ms |
1.16 |
Update one schema (10001 existing) |
732 ms |
649 ms |
1.13 |
Cached rebuild (1 existing) |
6 ms |
5 ms |
1.20 |
Cached rebuild (101 existing) |
8 ms |
7 ms |
1.14 |
Cached rebuild (1001 existing) |
31 ms |
26 ms |
1.19 |
Cached rebuild (10001 existing) |
283 ms |
241 ms |
1.17 |
Index 100 schemas |
111 ms |
117 ms |
0.95 |
Index 1000 schemas |
888 ms |
969 ms |
0.92 |
Index 10000 schemas |
14074 ms |
12809 ms |
1.10 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Benchmark Index (enterprise)
Details
| Benchmark suite | Current: fabf1f3 | Previous: ef38b58 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
239 ms |
239 ms |
1 |
Add one schema (100 existing) |
27 ms |
29 ms |
0.93 |
Add one schema (1000 existing) |
85 ms |
87 ms |
0.98 |
Add one schema (10000 existing) |
817 ms |
730 ms |
1.12 |
Update one schema (1 existing) |
20 ms |
22 ms |
0.91 |
Update one schema (101 existing) |
27 ms |
32 ms |
0.84 |
Update one schema (1001 existing) |
84 ms |
89 ms |
0.94 |
Update one schema (10001 existing) |
786 ms |
744 ms |
1.06 |
Cached rebuild (1 existing) |
7 ms |
7 ms |
1 |
Cached rebuild (101 existing) |
9 ms |
10 ms |
0.90 |
Cached rebuild (1001 existing) |
31 ms |
34 ms |
0.91 |
Cached rebuild (10001 existing) |
303 ms |
278 ms |
1.09 |
Index 100 schemas |
119 ms |
123 ms |
0.97 |
Index 1000 schemas |
1127 ms |
1052 ms |
1.07 |
Index 10000 schemas |
13177 ms |
15181 ms |
0.87 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/actions/base.cc">
<violation number="1" location="src/actions/base.cc:38">
P2: The new string-based absolute-path check is weaker than `std::filesystem::path::is_absolute()` and can let platform-specific absolute paths bypass validation.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
There was a problem hiding this comment.
1 issue found across 21 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="enterprise/server/include/sourcemeta/one/enterprise_server_actions.h">
<violation number="1" location="enterprise/server/include/sourcemeta/one/enterprise_server_actions.h:34">
P1: Storing `allowed_origin_` and `registry_url_` as `std::string_view` introduces a lifetime bug. These values should be owned by `EnterpriseMCP` because they are used after construction and inside async callbacks.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 participant
Signed-off-by: Juan Cruz Viotti jv@jviotti.com