mcp: add log and per-artifact issue query tools - #306
Conversation
Add three read-only dashboard tools, exposed both as MCP tools and on KernelCIClient: - get_log fetches a test's log_url and gzip-decompresses it, returning the content size-bounded to the tail by default, with total_bytes and a truncated flag so large logs stay within client limits - get_test_issues / get_build_issues list the issues tracked for a specific test or build, the inverse of the existing get_issue_tests and get_issue_builds get_log is new on KernelCIClient; the issue tools expose existing client methods that were not yet surfaced in the MCP. Signed-off-by: Ben Copeland <ben.copeland@linaro.org>
| total_bytes = len(content) | ||
| truncated = total_bytes > max_bytes | ||
| if truncated: | ||
| content = content[-max_bytes:] if tail else content[:max_bytes] |
There was a problem hiding this comment.
With max_bytes=0, content[-0:] returns the entire log. Negative values also return most of it. Validate that the value is positive, enforce a reasonable upper limit for MCP responses, and add boundary tests.
| content = response.content | ||
| if content[:2] == b"\x1f\x8b": | ||
| try: | ||
| content = gzip.decompress(content) |
There was a problem hiding this comment.
The download and decompression are not memory-bounded
A large log could exhaust the MCP process’s memory.
This should use streaming decompression with a bounded head/tail buffer and a hard size limit.
| except requests.exceptions.RequestException as exc: | ||
| raise KciDevError(f"Log download failed for test {test_id}: {exc}") from exc | ||
| content = response.content | ||
| if content[:2] == b"\x1f\x8b": |
There was a problem hiding this comment.
The code only catches OSError, so malformed or incomplete gzip content will escape as an internal exception instead of becoming KciDevError.
|
The trust boundary around log_url should be clarified. The code performs a direct GET, including redirects, against a URL obtained from dashboard data. If external submitters can control that field, this creates risk. The implementation should validate schemes, private/link-local addresses, DNS results, and redirect targets or document why all returned URLs are trusted. |
|
The happy paths have pytest coverage, but the security- and boundary-sensitive behavior is not covered. Please add tests for zero/negative/oversized max_bytes, truncated or malformed gzip input, download failures, bounded streaming/decompression, and URL/redirect validation. These tests should accompany the corresponding implementation fixes before approval. |
2 participants
Add three read-only dashboard tools, exposed both as MCP tools and on KernelCIClient:
get_log is new on KernelCIClient; the issue tools expose existing client methods that were not yet surfaced in the MCP.